From 687ae9e287b3a1a71e5e1c2a9c96b23d70768821 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Tue, 8 Jan 2019 11:37:31 +0100
Subject: [PATCH] ASoC: intel: skl: Fix display power regression
Git-commit: 687ae9e287b3a1a71e5e1c2a9c96b23d70768821
Patch-mainline: v5.0-rc4
References: bsc#1111666
Since the refactoring of HD-audio display power management, the
display power status is managed per domain. Meanwhile the ASoC
hdac_hdmi driver still keeps and relies (incorrectly) on the
refcounting together with ASoC skl driver, and this leads to the
display state always on.
This patch is an attempt to address the regression by simplifying the
PM code of ASoC skl and hdac_hdmi drivers. Basically, since the
refactoring, we don't have to manage the display power at HD-audio
controller suspend / resume but only at HD-audio HDMI codec suspend /
resume. So the patch drops the superfluous snd_hdac_display_power()
calls in skl driver.
Meanwhile, in hdac_hdmi side, we rewrite the PM call just to re-use
the runtime PM callbacks like other drivers do. Now the logic is
Simple: turn off at suspend and turn on at resume.
The patch also fixes the possibly missing display-power off at skl
driver removal as well as some error paths at probe.
Fixes: 029d92c289bd ("ALSA: hda: Refactor display power management")
Reported-by: Libin Yang <libin.yang@intel.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
sound/soc/codecs/hdac_hdmi.c | 49 +++++++++++-------------------------------
sound/soc/intel/skylake/skl.c | 44 +++++++++----------------------------
2 files changed, 24 insertions(+), 69 deletions(-)
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -1815,53 +1815,31 @@ static int hdmi_codec_remove(struct snd_
return 0;
}
-#ifdef CONFIG_PM
-static int hdmi_codec_prepare(struct device *dev)
-{
- struct hdac_ext_device *edev = to_hda_ext_device(dev);
- struct hdac_device *hdac = &edev->hdac;
-
- pm_runtime_get_sync(&edev->hdac.dev);
-
- /*
- * Power down afg.
- * codec_read is preferred over codec_write to set the power state.
- * This way verb is send to set the power state and response
- * is received. So setting power state is ensured without using loop
- * to read the state.
- */
- snd_hdac_codec_read(hdac, hdac->afg, 0, AC_VERB_SET_POWER_STATE,
- AC_PWRST_D3);
-
- return 0;
-}
-
-static void hdmi_codec_complete(struct device *dev)
+#ifdef CONFIG_PM_SLEEP
+static int hdmi_codec_resume(struct device *dev)
{
struct hdac_ext_device *edev = to_hda_ext_device(dev);
struct hdac_hdmi_priv *hdmi = edev->private_data;
- struct hdac_device *hdac = &edev->hdac;
-
- /* Power up afg */
- snd_hdac_codec_read(hdac, hdac->afg, 0, AC_VERB_SET_POWER_STATE,
- AC_PWRST_D0);
-
- hdac_hdmi_skl_enable_all_pins(&edev->hdac);
- hdac_hdmi_skl_enable_dp12(&edev->hdac);
+ int ret;
+ 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(edev, hdmi, false);
-
- pm_runtime_put_sync(&edev->hdac.dev);
+ return 0;
}
#else
-#define hdmi_codec_prepare NULL
-#define hdmi_codec_complete NULL
+#define hdmi_codec_resume NULL
#endif
static struct snd_soc_codec_driver hdmi_hda_codec = {
@@ -2125,8 +2103,7 @@ static int hdac_hdmi_runtime_resume(stru
static const struct dev_pm_ops hdac_hdmi_pm = {
SET_RUNTIME_PM_OPS(hdac_hdmi_runtime_suspend, hdac_hdmi_runtime_resume, NULL)
- .prepare = hdmi_codec_prepare,
- .complete = hdmi_codec_complete,
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, hdmi_codec_resume)
};
static const struct hda_device_id hdmi_list[] = {
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -306,14 +306,7 @@ static int skl_suspend(struct device *de
skl->skl_sst->fw_loaded = false;
}
- if (IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI)) {
- ret = snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, false);
- if (ret < 0)
- dev_err(bus->dev,
- "Cannot turn OFF display power on i915\n");
- }
-
- return ret;
+ return 0;
}
static int skl_resume(struct device *dev)
@@ -325,16 +318,6 @@ static int skl_resume(struct device *dev
struct hdac_ext_link *hlink = NULL;
int ret;
- /* Turned OFF in HDMI codec driver after codec reconfiguration */
- if (IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI)) {
- ret = snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, true);
- if (ret < 0) {
- dev_err(bus->dev,
- "Cannot turn on display power on i915\n");
- return ret;
- }
- }
-
/*
* resume only when we are not in suspend active, otherwise need to
* restore the device
@@ -368,7 +351,7 @@ static int skl_resume(struct device *dev
snd_hdac_bus_stop_cmd_io(&ebus->bus);
}
- return ret;
+ return 0;
}
#endif /* CONFIG_PM_SLEEP */
@@ -429,8 +412,10 @@ static int skl_free(struct hdac_ext_bus
snd_hdac_ext_bus_exit(ebus);
cancel_work_sync(&skl->probe_work);
- if (IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI))
+ if (IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI)) {
+ snd_hdac_display_power(&ebus->bus, HDA_CODEC_IDX_CONTROLLER, false);
snd_hdac_i915_exit(&ebus->bus);
+ }
return 0;
}
@@ -580,11 +565,9 @@ static int skl_i915_init(struct hdac_bus
if (err < 0)
return err;
- err = snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, true);
- if (err < 0)
- dev_err(bus->dev, "Cannot turn on display power on i915\n");
+ snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, true);
- return err;
+ return 0;
}
static void skl_probe_work(struct work_struct *work)
@@ -616,14 +599,6 @@ static void skl_probe_work(struct work_s
if (err < 0)
goto out_err;
- if (IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI)) {
- err = snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, false);
- if (err < 0) {
- dev_err(bus->dev, "Cannot turn off display power on i915\n");
- return;
- }
- }
-
/* register platform dai and controls */
err = skl_platform_register(bus->dev);
if (err < 0)
@@ -634,6 +609,9 @@ static void skl_probe_work(struct work_s
list_for_each_entry(hlink, &ebus->hlink_list, list)
snd_hdac_ext_bus_link_put(ebus, hlink);
+ if (IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI))
+ snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, false);
+
/* configure PM */
pm_runtime_put_noidle(bus->dev);
pm_runtime_allow(bus->dev);
@@ -643,7 +621,7 @@ static void skl_probe_work(struct work_s
out_err:
if (IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI))
- err = snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, false);
+ snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, false);
}
/*