diff mbox series

[1/2] ALSA: x86: Fix runtime PM for hdmi-lpe-audio

Message ID 20181024154825.18185-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] ALSA: x86: Fix runtime PM for hdmi-lpe-audio | expand

Commit Message

Ville Syrjälä Oct. 24, 2018, 3:48 p.m. UTC 
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(-)

Comments

Chris Wilson Oct. 24, 2018, 3:52 p.m. UTC | #1 
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
Chris Wilson Oct. 24, 2018, 4:09 p.m. UTC | #2 
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
Ville Syrjälä Oct. 24, 2018, 4:28 p.m. UTC | #3 
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.
Chris Wilson Oct. 24, 2018, 4:32 p.m. UTC | #4 
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
Chris Wilson Oct. 25, 2018, 6:50 a.m. UTC | #5 
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
Ville Syrjälä Oct. 26, 2018, 4:56 p.m. UTC | #6 
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
Ville Syrjälä Nov. 1, 2018, 3:24 p.m. UTC | #7 
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
Takashi Iwai Nov. 2, 2018, 9:31 a.m. UTC | #8 
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
>
Ville Syrjälä Nov. 2, 2018, 4:20 p.m. UTC | #9 
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.
diff mbox series

Patch

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) {