On Thu, 28 Mar 2019 09:40:05 +0100,
libin.yang@xxxxxxxxx wrote:
>
> From: Libin Yang <libin.yang@xxxxxxxxx>
>
> Implement the hdmi codec component driver's suspend/resume callback.
> This can make sure that the dapm event is triggered after the display
> audio power domain is turned on if bus_control is set.
>
> Also this patch removes the codec device PM suspend/resume.
>
> Signed-off-by: Libin Yang <libin.yang@xxxxxxxxx>
> ---
> sound/soc/codecs/hdac_hdmi.c | 51 +++++++++++++++++++++++---------------------
> 1 file changed, 27 insertions(+), 24 deletions(-)
>
> diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
> index 5eeb0fe..478c6f2 100644
> --- a/sound/soc/codecs/hdac_hdmi.c
> +++ b/sound/soc/codecs/hdac_hdmi.c
> @@ -1873,36 +1873,27 @@ static void hdmi_codec_remove(struct snd_soc_component *component)
> pm_runtime_disable(&hdev->dev);
> }
>
> -#ifdef CONFIG_PM_SLEEP
> -static int hdmi_codec_resume(struct device *dev)
> +static int hdmi_component_suspend(struct snd_soc_component *component)
> {
> - struct hdac_device *hdev = dev_to_hdac_dev(dev);
> - struct hdac_hdmi_priv *hdmi = hdev_to_hdmi_priv(hdev);
> - int ret;
> + struct hdac_hdmi_priv *hdmi = snd_soc_component_get_drvdata(component);
> + struct hdac_device *hdev = hdmi->hdev;
>
> - ret = pm_runtime_force_resume(dev);
> - if (ret < 0)
> - return ret;
> - /*
> - * As the ELD notify callback request is not entertained while the
> - * device is in suspend state. Need to manually check detection of
> - * all pins here. pin capablity change is not support, so use the
> - * already set pin caps.
> - *
> - * NOTE: this is safe to call even if the codec doesn't actually resume.
> - * The pin check involves only with DRM audio component hooks, so it
> - * works even if the HD-audio side is still dreaming peacefully.
> - */
> - hdac_hdmi_present_sense_all_pins(hdev, hdmi, false);
> - return 0;
> + return pm_runtime_force_suspend(&hdev->dev);
> +}
> +
> +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;
> +
> + return pm_runtime_force_resume(&hdev->dev);
> }
> -#else
> -#define hdmi_codec_resume NULL
> -#endif
>
> 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,
> @@ -2104,6 +2095,7 @@ static int hdac_hdmi_runtime_resume(struct device *dev)
> struct hdac_device *hdev = dev_to_hdac_dev(dev);
> struct hdac_bus *bus = hdev->bus;
> struct hdac_ext_link *hlink = NULL;
> + struct hdac_hdmi_priv *hdmi = hdev_to_hdmi_priv(hdev);
>
> dev_dbg(dev, "Enter: %s\n", __func__);
>
> @@ -2128,6 +2120,18 @@ static int hdac_hdmi_runtime_resume(struct device *dev)
> snd_hdac_codec_read(hdev, hdev->afg, 0, AC_VERB_SET_POWER_STATE,
> AC_PWRST_D0);
>
> + /*
> + * As the ELD notify callback request is not entertained while the
> + * device is in suspend state. Need to manually check detection of
> + * all pins here. pin capablity change is not support, so use the
> + * already set pin caps.
> + *
> + * NOTE: this is safe to call even if the codec doesn't actually resume.
> + * The pin check involves only with DRM audio component hooks, so it
> + * works even if the HD-audio side is still dreaming peacefully.
> + */
> + hdac_hdmi_present_sense_all_pins(hdev, hdmi, false);
Do we need to move this into runtime_resume? The forced detection is
needed basically only for the system resume, i.e. the detection is
performed while the system is running.
Other than that, one thing I overlooked is that this approach might
hit another race when another problem is calling a path that includes
pm_runtime_get*(). Since the whole ASoC resume is done in an async
work, this would conflict. That's rather a fundamental bug in ASoC
resume framework, though...
So, I'm interested in whether another approach really works: namely
defining the PM dependency via device_link_add(). This should be
safer.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
