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.
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