>-----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()? Regards, Libin > > >Takashi > >> >> diff --git a/sound/soc/codecs/hdac_hdmi.c >> b/sound/soc/codecs/hdac_hdmi.c index 5eeb0fe..505efd9 100644 >> --- a/sound/soc/codecs/hdac_hdmi.c >> +++ b/sound/soc/codecs/hdac_hdmi.c >> @@ -1900,9 +1900,31 @@ static int hdmi_codec_resume(struct device >> *dev) #define hdmi_codec_resume NULL #endif >> >> +static int hdmi_component_suspend(struct snd_soc_component >> +*component) { >> + struct hdac_hdmi_priv *hdmi = >snd_soc_component_get_drvdata(component); >> + struct hdac_device *hdev = hdmi->hdev; >> + >> + pm_runtime_force_suspend(&hdev->dev); >> + >> + return 0; >> +} >> + >> +static int hdmi_component_resume(struct snd_soc_component >*component) >> +{ >> + struct hdac_hdmi_priv *hdmi = >snd_soc_component_get_drvdata(component); >> + struct hdac_device *hdev = hdmi->hdev; >> + >> + pm_runtime_force_resume(&hdev->dev); >> + >> + return 0; >> +} >> + >> static const struct snd_soc_component_driver hdmi_hda_codec = { >> .probe = hdmi_codec_probe, >> .remove = hdmi_codec_remove, >> + .suspend = hdmi_component_suspend, >> + .resume = hdmi_component_resume, >> .use_pmdown_time = 1, >> .endianness = 1, >> .non_legacy_dai_naming = 1, >> >> > >> >> Regards, >> Libin >> _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
