[alsa-devel] [PATCH 04/35] ASoC: Intel: Skylake: Unify firmware loading mechanism

[Cezary Rojewski]

On 2019-08-26 18:31, Pierre-Louis Bossart wrote:
> 
> 
> 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-?
> 

The entire check_dsp_lp_on is actually D0ix thingy.
-- The power-management is being addressed in following segment. --

D0ix is a power-optimization feature and implemented in APL and onward. 
So for SKL/ KBL there is no check_dsp_lp_on. You can see the difference 
in /skylake/skl-sst.c: static const struct skl_dsp_fw_ops skl_fw_ops 
(lack of set_state_D0ix handlers)

Once dsp-host interaction can be described as idle (e.g. ongoing 
playback with no IPC traffic), host may enable FW to power gate some 
components and thus reduce power consumption.

D0ix is abbrev for D0i0 <-> D0i3 transitions. Once dsp enters D0i3 
(power gated), no IPCs can be consumed apart from one and only SetD0ix - 
to wake dsp back to D0i0 state. In general you can think of D0i0 as D0.
Again, since SKL/ KBL have no D0ix, there is no need to wake anything, 
no checks are required.