[alsa-devel] [PATCH 1/2] ALSA: x86: Fix runtime PM for hdmi-lpe-audio
From: Ville Syrjälä ville.syrjala@linux.intel.com
Commit 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") broke runtime PM with lpe audio. We can no longer runtime suspend the GPU since the sysfs power/control for the lpe-audio device no longer exists and the device is considered always active. We can fix this by not marking the device as active.
Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Fixes: 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- sound/x86/intel_hdmi_audio.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 83d76c345940..bbed4acaafd1 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1877,7 +1877,6 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
pm_runtime_use_autosuspend(&pdev->dev); pm_runtime_mark_last_busy(&pdev->dev); - pm_runtime_set_active(&pdev->dev);
dev_dbg(&pdev->dev, "%s: handle pending notification\n", __func__); for_each_port(card_ctx, port) {
From: Ville Syrjälä ville.syrjala@linux.intel.com
Ever since commit 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") the runtime suspend/resume hooks are no longer used. Inline them into the system suspend hooks.
Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- sound/x86/intel_hdmi_audio.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index bbed4acaafd1..00c92eb854ce 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1648,7 +1648,7 @@ static int had_create_jack(struct snd_intelhad *ctx, * PM callbacks */
-static int hdmi_lpe_audio_runtime_suspend(struct device *dev) +static int __maybe_unused hdmi_lpe_audio_suspend(struct device *dev) { struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev); int port; @@ -1664,23 +1664,8 @@ static int hdmi_lpe_audio_runtime_suspend(struct device *dev) } }
- return 0; -} - -static int __maybe_unused hdmi_lpe_audio_suspend(struct device *dev) -{ - struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev); - int err; + snd_power_change_state(card_ctx->card, SNDRV_CTL_POWER_D3hot);
- err = hdmi_lpe_audio_runtime_suspend(dev); - if (!err) - snd_power_change_state(card_ctx->card, SNDRV_CTL_POWER_D3hot); - return err; -} - -static int hdmi_lpe_audio_runtime_resume(struct device *dev) -{ - pm_runtime_mark_last_busy(dev); return 0; }
@@ -1688,8 +1673,10 @@ static int __maybe_unused hdmi_lpe_audio_resume(struct device *dev) { struct snd_intelhad_card *card_ctx = dev_get_drvdata(dev);
- hdmi_lpe_audio_runtime_resume(dev); + pm_runtime_mark_last_busy(dev); + snd_power_change_state(card_ctx->card, SNDRV_CTL_POWER_D0); + return 0; }
@@ -1907,8 +1894,6 @@ static int hdmi_lpe_audio_remove(struct platform_device *pdev)
static const struct dev_pm_ops hdmi_lpe_audio_pm = { SET_SYSTEM_SLEEP_PM_OPS(hdmi_lpe_audio_suspend, hdmi_lpe_audio_resume) - SET_RUNTIME_PM_OPS(hdmi_lpe_audio_runtime_suspend, - hdmi_lpe_audio_runtime_resume, NULL) };
static struct platform_driver hdmi_lpe_audio_driver = {
Quoting Ville Syrjala (2018-10-24 16:48:25)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Ever since commit 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") the runtime suspend/resume hooks are no longer used. Inline them into the system suspend hooks.
Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
All tallies up, and rpm continues to function as desired. Reviewed-by: Chris Wilson chris@chris-wilson.co.uk -Chris
Quoting Ville Syrjala (2018-10-24 16:48:24)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Commit 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") broke runtime PM with lpe audio. We can no longer runtime suspend the GPU since the sysfs power/control for the lpe-audio device no longer exists and the device is considered always active. We can fix this by not marking the device as active.
Do you also want to send with a HAX patch to force selection of SND_X86 for CI? -Chris
On Wed, Oct 24, 2018 at 04:52:22PM +0100, Chris Wilson wrote:
Quoting Ville Syrjala (2018-10-24 16:48:24)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Commit 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") broke runtime PM with lpe audio. We can no longer runtime suspend the GPU since the sysfs power/control for the lpe-audio device no longer exists and the device is considered always active. We can fix this by not marking the device as active.
Do you also want to send with a HAX patch to force selection of SND_X86 for CI?
Hmm. Would have been a good idea a few minutes ago. Not sure I want to spam the list again. Depends how long it'll take to adjust the ci .config I suppose.
Quoting Ville Syrjälä (2018-10-24 17:28:45)
On Wed, Oct 24, 2018 at 04:52:22PM +0100, Chris Wilson wrote:
Quoting Ville Syrjala (2018-10-24 16:48:24)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Commit 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") broke runtime PM with lpe audio. We can no longer runtime suspend the GPU since the sysfs power/control for the lpe-audio device no longer exists and the device is considered always active. We can fix this by not marking the device as active.
Do you also want to send with a HAX patch to force selection of SND_X86 for CI?
Hmm. Would have been a good idea a few minutes ago. Not sure I want to spam the list again. Depends how long it'll take to adjust the ci .config I suppose.
Trybot and link to the results? -Chris
Quoting Chris Wilson (2018-10-24 17:32:04)
Quoting Ville Syrjälä (2018-10-24 17:28:45)
On Wed, Oct 24, 2018 at 04:52:22PM +0100, Chris Wilson wrote:
Quoting Ville Syrjala (2018-10-24 16:48:24)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Commit 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") broke runtime PM with lpe audio. We can no longer runtime suspend the GPU since the sysfs power/control for the lpe-audio device no longer exists and the device is considered always active. We can fix this by not marking the device as active.
Do you also want to send with a HAX patch to force selection of SND_X86 for CI?
Hmm. Would have been a good idea a few minutes ago. Not sure I want to spam the list again. Depends how long it'll take to adjust the ci .config I suppose.
Trybot and link to the results?
https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_3118/issues.html
The fi-byt-clapper pm tests passed which is the most significant result. For enabling LPE audio in CI, it appears we need to trick snd_hdmi_lpe_audio.ko into unloading on demand. -Chris
On Thu, Oct 25, 2018 at 07:50:29AM +0100, Chris Wilson wrote:
Quoting Chris Wilson (2018-10-24 17:32:04)
Quoting Ville Syrjälä (2018-10-24 17:28:45)
On Wed, Oct 24, 2018 at 04:52:22PM +0100, Chris Wilson wrote:
Quoting Ville Syrjala (2018-10-24 16:48:24)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Commit 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") broke runtime PM with lpe audio. We can no longer runtime suspend the GPU since the sysfs power/control for the lpe-audio device no longer exists and the device is considered always active. We can fix this by not marking the device as active.
Do you also want to send with a HAX patch to force selection of SND_X86 for CI?
Hmm. Would have been a good idea a few minutes ago. Not sure I want to spam the list again. Depends how long it'll take to adjust the ci .config I suppose.
Trybot and link to the results?
https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_3118/issues.html
The fi-byt-clapper pm tests passed which is the most significant result. For enabling LPE audio in CI, it appears we need to trick snd_hdmi_lpe_audio.ko into unloading on demand.
Re-ran the test for good measure after fixing the module unload in igt. Now all green.
https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_3140/issues.html
Quoting Ville Syrjala (2018-10-24 16:48:24)
From: Ville Syrjälä ville.syrjala@linux.intel.com
Commit 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") broke runtime PM with lpe audio. We can no longer runtime suspend the GPU since the sysfs power/control for the lpe-audio device no longer exists and the device is considered always active. We can fix this by not marking the device as active.
Seems like this use is covered by runtime_pm.txt:
However, if the device has a parent and the parent's runtime PM is enabled, calling pm_runtime_set_active() for the device will affect the parent, unless the parent's 'power.ignore_children' flag is set. Namely, in that case the parent won't be able to suspend at run time, using the PM core's helper functions, as long as the child's status is 'active', even if the child's runtime PM is still disabled (i.e. pm_runtime_enable() hasn't been called for the child yet or pm_runtime_disable() has been called for it). For this reason, once pm_runtime_set_active() has been called for the device, pm_runtime_enable() should be called for it too as soon as reasonably possible or its runtime PM status should be changed back to 'suspended' with the help of pm_runtime_set_suspended().
Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Fixes: 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
So according to that the alternative is to add a call to pm_runtime_enable(). Seems like the sensible course of action is to merely mark it as busy and not set it as active.
With the caveat of checking with CI + SND_X86 :) Reviewed-by: Chris Wilson chris@chris-wilson.co.uk -Chris
On Wed, Oct 24, 2018 at 06:48:24PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Commit 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") broke runtime PM with lpe audio. We can no longer runtime suspend the GPU since the sysfs power/control for the lpe-audio device no longer exists and the device is considered always active. We can fix this by not marking the device as active.
Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Fixes: 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Takashi, do you want to take these or should I just smash them into drm-intel?
sound/x86/intel_hdmi_audio.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 83d76c345940..bbed4acaafd1 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1877,7 +1877,6 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
pm_runtime_use_autosuspend(&pdev->dev); pm_runtime_mark_last_busy(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
dev_dbg(&pdev->dev, "%s: handle pending notification\n", __func__); for_each_port(card_ctx, port) {
-- 2.18.1
On Thu, 01 Nov 2018 16:24:36 +0100, Ville Syrjälä wrote:
On Wed, Oct 24, 2018 at 06:48:24PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Commit 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") broke runtime PM with lpe audio. We can no longer runtime suspend the GPU since the sysfs power/control for the lpe-audio device no longer exists and the device is considered always active. We can fix this by not marking the device as active.
Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Fixes: 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Takashi, do you want to take these or should I just smash them into drm-intel?
Feel free to go through drm-intel. Acked-by: Takashi Iwai tiwai@suse.de
thanks,
Takashi
sound/x86/intel_hdmi_audio.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/x86/intel_hdmi_audio.c b/sound/x86/intel_hdmi_audio.c index 83d76c345940..bbed4acaafd1 100644 --- a/sound/x86/intel_hdmi_audio.c +++ b/sound/x86/intel_hdmi_audio.c @@ -1877,7 +1877,6 @@ static int hdmi_lpe_audio_probe(struct platform_device *pdev)
pm_runtime_use_autosuspend(&pdev->dev); pm_runtime_mark_last_busy(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
dev_dbg(&pdev->dev, "%s: handle pending notification\n", __func__); for_each_port(card_ctx, port) {
-- 2.18.1
-- Ville Syrjälä Intel
On Fri, Nov 02, 2018 at 10:31:37AM +0100, Takashi Iwai wrote:
On Thu, 01 Nov 2018 16:24:36 +0100, Ville Syrjälä wrote:
On Wed, Oct 24, 2018 at 06:48:24PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Commit 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") broke runtime PM with lpe audio. We can no longer runtime suspend the GPU since the sysfs power/control for the lpe-audio device no longer exists and the device is considered always active. We can fix this by not marking the device as active.
Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Takashi Iwai tiwai@suse.de Cc: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Fixes: 46e831abe864 ("drm/i915/lpe: Mark LPE audio runtime pm as "no callbacks"") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Takashi, do you want to take these or should I just smash them into drm-intel?
Feel free to go through drm-intel. Acked-by: Takashi Iwai tiwai@suse.de
Thanks. Series pushed.
participants (3)
-
Chris Wilson -
Takashi Iwai -
Ville Syrjala