-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Saturday, April 06, 2013 1:05 AM
Immediate playback:
- System skips hda_call_codec_resume and sets
codec->resume_delayed.
- Process context wants to start playback, which powers up the
codec and calls intel_haswell_set_power_state. 3. intel_haswell_wait_ready_to_resume enables unsol events and starts to wait on ready_to_resume 4. Gfx init finishes, and workq context fires the unsol event, which calls hdmi_intrinsic_event, which triggers the resume_wq. 5. Process context and workq context now continues in parallel, potentially (but hopefully not) leading to race conditions?
The current patch has a small race, but it can be avoided by clearing spec->ready_to_resume early enough, e.g. in suspend callback or in init callback, like the patch below.
I think there is no race for Haswell. In my test I observed that the unsol evnet will not be received until intel_haswell_wait_ready_to_resume()
enables the unsol event on the pins.
So I put "spec->ready_to_resume = 0" before enabling the unsol event.
Practically your patch would work well, but theoretically the unsol event might be issued and handled by hdmi_intrinsic_event() before you go into intel_haswell_wait_ready_to_resume(). That is, you _clear_ spec->ready_to_resume again and wait for the unsol event that had been already processed. That's the race David pointed.
The fix is simply not to clear spec->ready_to_resume in intel_haswell_wait_ready_to_resume() but clear it much earlier already before the resume path as my patch does. Then wait_event() passes immediately (and the unsol enablement of is anyway harmless).
Hi Takashi,
I see. Thanks for clarifying!
I'll revise the patch and test it again when I'm in office on Monday. Also I'll rebase the patch to for-next branch of sound git tree and fix the indentations and spaces errors.
Thanks Mengdong