On 8/24/19 4:34 AM, Cezary Rojewski wrote:
On 2019-08-23 20:40, Pierre-Louis Bossart wrote:
-int skl_sst_init_fw(struct device *dev, struct skl_dev *skl) +int skl_sst_init_fw(struct skl_dev *skl) { - int ret; struct sst_dsp *sst = skl->dsp; + struct device *dev = skl->dev; + int (*lp_check)(struct sst_dsp *dsp, bool state); + int ret;
+ lp_check = skl->ipc.ops.check_dsp_lp_on; + skl->enable_miscbdcge(dev, false); + skl->clock_power_gating(dev, false); ret = sst->fw_ops.load_fw(sst); if (ret < 0) { dev_err(dev, "Load base fw failed : %d\n", ret); - return ret; + goto exit; + }
+ if (!skl->is_first_boot) + goto library_load; + /* Disable power check during cfg setup */ + skl->ipc.ops.check_dsp_lp_on = NULL;
It's very odd to play with .ops callback dynamically. Usually ops are constant, and if you want to disable them you add a flag.
Yeye, keen eye! Can't do everything at once though :/ The power check is APL+ specific and should not be part of generic ipc framework at all (found in /sound/soc/intel/common/sst-ipc.c). Different fate awaits said check. For now, in this single case it seems best to simply disable the check and reapply it once setup is done.
What's the difference with having this callback do nothing for APL-?
+ ret = skl_ipc_fw_cfg_get(&skl->ipc, &skl->fw_cfg); + if (ret < 0) { + dev_err(dev, "Failed to get fw cfg: %d\n", ret); + goto exit; + }
+ ret = skl_ipc_hw_cfg_get(&skl->ipc, &skl->hw_cfg); + if (ret < 0) { + dev_err(dev, "Failed to get hw cfg: %d\n", ret); + goto exit; } skl_dsp_init_core_state(sst); +library_load: if (skl->lib_count > 1) { ret = sst->fw_ops.load_library(sst, skl->lib_info, skl->lib_count); if (ret < 0) { - dev_err(dev, "Load Library failed : %x\n", ret); - return ret; + dev_err(dev, "Load library failed : %x\n", ret); + goto exit; } }
skl->is_first_boot = false; +exit: + skl->ipc.ops.check_dsp_lp_on = lp_check; + skl->enable_miscbdcge(dev, true); + skl->clock_power_gating(dev, true); - return 0; + return ret; } EXPORT_SYMBOL_GPL(skl_sst_init_fw);