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

From: "Yang, Libin" <libin.yang@intel.com>
To: Takashi Iwai <tiwai@suse.de>
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 11:15:06 +0000	[thread overview]
Message-ID: <96A12704CE18D347B625EE2D4A099D19527FB419@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <s5hlg12m2xr.wl-tiwai@suse.de>

Hi Takashi,

>-----Original Message-----
>From: Takashi Iwai [mailto:tiwai@suse.de]
>Sent: Tuesday, March 26, 2019 4:54 PM
>To: Yang, Libin <libin.yang@intel.com>
>Cc: alsa-devel@alsa-project.org; broonie@kernel.org
>Subject: Re: [alsa-devel] [PATCH V2] ASoC: fix hdmi codec driver contest in S3
>
>On Tue, 26 Mar 2019 09:46:31 +0100,
>Yang, Libin wrote:
>>
>> Hi Takashi
>>
>> >
>> >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.
>>
>> I tried to move display power setting from runtime suspend/resume to
>> component suspend/resume. I found there may be another issue:
>> playback will NOT call component suspend/resume.
>> This  means we will never have chance to set the display power if we
>> don't do the S3.
>
>The runtime suspend/resume are fine and need to be kept, I suppose.
>The only problem is the system suspend/resume callbacks that call
>pm_runtime_force_*().  Just moving these two to component would work?
>

Let's see the flow of the suspend:
1. Component suspend is called. It will call snd_hdac_display_power()
to turn off the display power.
2. runtime suspend is called. It will operate on the registers. This
is wrong. We can't operate on registers when the power is off.

So I think we can only call snd_hdac_display_power() either in
component suspend/resume or runtime suspend/resume.
We can't call snd_hdac_display_power() in both.

And based on the previous test, component suspend/resume
will not be called when playback. So we can only call
snd_hdac_display_power() in runtime suspend/resume?

Regards,
Libin

>
>Takashi