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