Re: [PATCH] ALSA: hda - delay resume haswell hdmi codec in system resume

[David Henningsson <david.henningsson@canonical.com>]

On 03/27/2013 11:01 AM, Lin, Mengdong wrote:
>> -----Original Message-----
>> From: David Henningsson [mailto:david.henningsson@canonical.com]
>> Sent: Wednesday, March 27, 2013 4:19 PM
>
>>> The Haswell codec set_power_state ops (intel_haswell_set_power_state) will
>> only wait if this is a delayed resume and clear this flag after waiting. And
>> actually, there is no waiting even in this case. Because when 1st user operation
>> after system resume happens, Gfx already finishes resuming and audio
>> initialization, so as long as intel_haswell_wait_ready_to_resume() enable the
>> unsol event, the unsol event comes and so so waiting finishes. The 300ms time
>> out is set for safety consideration in case unsol event is lost. I've not observed
>> any unsol event lost till now.
>>
>> As for the timeout, I suggest you use the codec->bus->workq instead of
>> creating a new workq. I think that will also give us some serialisation, i e,
>> protection against race conditions if the timeout happen at the same time as
>> the unsol event.
>
> Hi David,
>
> The new added "resume_wq" for hdmi codec is a wait queue, not a work queue like codec->bus->workq.
> It's expected to wake up as soon as the unsol event is got.

Sure; but I don't see why you need a wait queue for that? Why don't you 
just call the resume path from the unsol event handler 
(hdmi_present_sense, or its caller), and then also cancel the timeout 
handler (which can then be in the normal workq)?


> And I think there will not be race between unsol event and idle time out.
> Because until the first user operation trigger the delayed resume, the codec keeps in D3.
> And only until intel_haswell_set_power_state(), which is called by hda_call_codec_resume(), finishes waiting and programing the codec to D0,
> __snd_hda_power_up() will keep 'codec->power_transition' to '1'. Thus __snd_hda_power_down() will not schedule idle time out.
>
>
>>> BTW, would you please tell us how Ubuntu decides whether to enable the
>> normal idle power down (parameter "power_save")?
>>> We found this bug on a Haswell mobile machine with "power_save" disabled,
>> which means only system resume will program the codec back to D0. But on a
>> another machine with "power_save" enabled, this bug is not visible because
>> later runtime codec power up can program the codec to D0 and unmute the pin
>> again.
>>
>> Hmm. I don't think there is any difference between the upstream default and
>> Ubuntu in this case. I remember we had to turn off power_save for
>> Pantherpoint once, before keep_power_link_on for PantherPoint was
>> discovered and upstreamed. But that was quite a while ago.
>>
>> Then there is of course people playing with powertop and other tools, to
>> override the defaults.
>
> The default power_value is 0 in Kconfig, which means off.
> But on some HSW machines with ALC889 codec and Haswell display codec, both with EPSS support, I found that power_save is 1 after boot.
> On another HSW machine with ALC262, no EPSS, power_save is 0.
>
> So I suspect the rule is that if all codecs support EPSS, power_save will be enabled. But who enables this?

That does not seem to make sense. It might be something different?


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic