Re: [PATCH V2] ASoC: fix hdmi codec driver contest in S3

From: Takashi Iwai <tiwai@suse.de>
To: "Yang, Libin" <libin.yang@intel.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"broonie@kernel.org" <broonie@kernel.org>
Subject: Re: [PATCH V2] ASoC: fix hdmi codec driver contest in S3
Date: Tue, 26 Mar 2019 08:36:50 +0100	[thread overview]
Message-ID: <s5hr2aum6i5.wl-tiwai@suse.de> (raw)
In-Reply-To: <96A12704CE18D347B625EE2D4A099D19527FB1A0@SHSMSX103.ccr.corp.intel.com>

On Tue, 26 Mar 2019 06:42:15 +0100,
Yang, Libin wrote:
(snip)
> Hi Takashi,
> Below is my draft patch, which doesn't work with pm_runtime_get_sync(). Is
> there anything wrong in my code?
(snip)
> And the dmesg is:
> [   36.087224] HDMI HDA Codec ehdaudio0D2: in hdmi_codec_resume 1890 ylb
> [   36.087230] HDMI HDA Codec ehdaudio0D2: in hdac_hdmi_runtime_resume 2122 ylb 1
> [   36.087335] HDMI HDA Codec ehdaudio0D2: in hdac_hdmi_cvt_output_widget_event 812 ylb
> [   40.097586] HDMI HDA Codec ehdaudio0D2: in hdac_hdmi_runtime_resume 2135 ylb 2
> [   40.097766] HDMI HDA Codec ehdaudio0D2: in hdac_hdmi_pin_output_widget_event 767 ylb
> [   45.108632] HDMI HDA Codec ehdaudio0D2: in hdac_hdmi_pin_mux_widget_event 857 ylb
> 
> >From the dmesg, hdac_hdmi_cvt_output_widget_event() 
> is called between "ylb 1" and "ylb 2" in runtime_resume().
> This means xxx_event() registers setting runs before display
> power is turned on.

OK, now I understood what went wrong.  It's a similar issue as we've
hit for the legacy HD-audio and figured out recently.

If my understanding is correct, the problem is the call of
pm_runtime_force_resume() in PM resume callback combined with an async
work.  pm_runtime_force_resume() sets the runtime state immediately to
RPM_ACTIVE even before finishing the task.  The code seems assuming
blindly that there is no other conflicting task while S3/S4 resume,
but this is a problem.  That's why the next pm_runtime_get_sync()
wasn't blocked but just went through; since the RPM flag was already
set to RPM_ACTIVE in pm_runtime_force_resume(), it thought as if it
were already active, while the real runtime resume code was still
processing the callback.

In the case of legacy HD-audio, we "fixed" the problem by avoiding the
trigger of async work at resume, but let it be triggered from runtime
resume.  In ASoC case, however, it's no option.

Maybe a possible solution in the sound driver side would be to move
from system suspend/resume to ASoC component suspend/resume.  The
runtime suspend/resume can be kept as is, and call
pm_runtime_force_suspend and resume from the component suspend/resume
callbacks.  Since the component callbacks are certainly processed
before DAPM events, this should assure the order.

thanks,

Takashi