Re: [PATCH 2/7] ALSA: hda: Refactor display power management

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

>>>>>      a codec was done indirectly via link_power bus ops.)
>>>>>
>>>>> - snd_hdac_display_power() receives the codec address index.  For
>>>>>      turning on/off from the controller, pass HDA_CODEC_IDX_CONTROLLER.
>>>> The need for this virtual index==16 isn't fully clear to me,
>>>> especially if you use the bitfields instead of reference counts.
>>>>
>>>> Isn't there a risk of the controller setting the bit16 to zero, but
>>>> you still have bit4 on (assuming the idx is 4). If you use this
>>>> virtual index, it should override the actual physical bits when
>>>> set/cleared.
>>> This is the index for a controller, i.e. we'd need bits for the max
>>> number of codecs + 1.
>>>
>>> Actually we do support only up to 8 codecs (HDA_MAX_CODECS), so it
>>> should be 8, instead of 16, too.  I'll update to be HDA_MAX_CODECS.
>>>
>>>> Or is this meant to actually implement a preemption mechanism, where
>>>> the display power remains on for as long as the controller wishes,
>>>> regardless of what the patch_hdmi and hdac_hdmi code requests?
>>> Right.  That's the mechanism at the initial phase, we need the display
>>> power on while probing the codec, i.e. before identifying the codec
>>> ID.
>>>
>>>> Also don't we already have the HDMI codec address already after the
>>>> probe, so couldn't we provide the address directly?
>>> The resume seemed requiring the controller to take the display power
>>> at first, so the same mechanism is used.
>> ok, makes sense, thanks for the explanations.
>>
>> So I guess for the SOF patches, the only change would be to add the
>> second argument HDA_CODEC_IDX_CONTROLLER to snd_hdac_display_power()
>> calls, the rest looks unchanged or hidden inside the hdac library or
>> hdac_hdmi parts.
> Yes, other than that, this change makes things easier.
>
> Since we don't manage with refcount, the only important point is to
> turn off/on properly at suspend/resume (also off at remove), no matter
> how many times it gets called.
Ah yes, we are missing this on remove since we assumed the refcount 
would already be zero. I guess we'll have to revalidate this part 
anyways once your patches are merged (already have an SOF issue filed to 
track this change).