On Thu, 28 Mar 2019 08:28:06 +0100, Yang, Libin wrote: > > > >-----Original Message----- > >From: Takashi Iwai [mailto:tiwai@xxxxxxx] > >Sent: Thursday, March 28, 2019 3:15 PM > >To: Yang, Libin <libin.yang@xxxxxxxxx> > >Cc: 'alsa-devel@xxxxxxxxxxxxxxxx' <alsa-devel@xxxxxxxxxxxxxxxx>; > >'broonie@xxxxxxxxxx' <broonie@xxxxxxxxxx> > >Subject: Re: [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: > >> >>> 1. 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 _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
