On Thu, 28 Mar 2019 08:28:06 +0100, Yang, Libin wrote:
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Thursday, March 28, 2019 3:15 PM 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: [alsa-devel] [PATCH V2] ASoC: fix hdmi codec driver contest in S3
On Thu, 28 Mar 2019 08:10:34 +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:
- Component suspend is called. It will call
snd_hdac_display_power() to turn off the display power.
No, what I meant was to make the component suspend calling pm_runtime_force_suspend() instead of the system suspend. Ditto for the component resume calling pm_runtime_force_resume().
It seems this component suspend/resume method works well. I will do more test and send out the patch later.
I did some stress test. And I found if I remove the debug message, the issue can still be reproduced with component suspend/resume added. If I add the debug message, it will be much better. I'm not sure whether my patch is wrong or not. I've set the bus_control = 1 in the dai driver.
I guess you forgot to remove the suspend/resume call from the codec device PM? Otherwise the codec device resume is still executed concurrently.
... Yes, I forgot it. Thanks for reminder. By the way, do you think it is better to use pm_runtime_force_resume() or pm_runtime_get_sync()?
It needs pm_runtime_force_resume() if it's suspended by pm_runtime_force_suspend(), I suppose.
Takashi