[Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers
On Haswell/Broadwell, the HD-Audio block is inside the HDMI/display power well and so the sna-hda audio codec acquires the display power well while it is operational. However, Skylake separates the powerwells again, and so we must remember to acquire the rpm wakeref for ourselves whilst tweaking the registers. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96214 Fixes: 03b135cebc47 "ALSA: hda - remove dependency on i915 power well for SKL") Signed-off-by: Chris Wilson Cc: Libin Yang Cc: Takashi Iwai Cc: Marius Vlad --- drivers/gpu/drm/i915/intel_audio.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c index 5d5f6bc10e85..eedcce6478ef 100644 --- a/drivers/gpu/drm/i915/intel_audio.c +++ b/drivers/gpu/drm/i915/intel_audio.c @@ -600,6 +600,8 @@ static void i915_audio_component_codec_wake_override(struct device *dev, if (!IS_SKYLAKE(dev_priv) && !IS_KABYLAKE(dev_priv)) return; + intel_runtime_pm_get(dev_priv); + /* * Enable/disable generating the codec wake signal, overriding the * internal logic to generate the codec wake to controller. @@ -615,6 +617,8 @@ static void i915_audio_component_codec_wake_override(struct device *dev, I915_WRITE(HSW_AUD_CHICKENBIT, tmp); usleep_range(1000, 1500); } + + intel_runtime_pm_put(dev_priv); } /* Get CDCLK in kHz */ @@ -648,6 +652,8 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, !IS_HASWELL(dev_priv)) return 0; + intel_runtime_pm_get(dev_priv); + mutex_lock(&dev_priv->av_mutex); /* 1. get the pipe */ intel_encoder = dev_priv->dig_port_map[port]; @@ -698,6 +704,7 @@ static int i915_audio_component_sync_audio_rate(struct device *dev, unlock: mutex_unlock(&dev_priv->av_mutex); + intel_runtime_pm_put(dev_priv); return err; } -- 2.8.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers
Chris Wilson writes: > On Haswell/Broadwell, the HD-Audio block is inside the HDMI/display > power well and so the sna-hda audio codec acquires the display power > well while it is operational. However, Skylake separates the powerwells > again, and so we must remember to acquire the rpm wakeref for ourselves > whilst tweaking the registers. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96214 > Fixes: 03b135cebc47 "ALSA: hda - remove dependency on i915 power well for > SKL") > Signed-off-by: Chris Wilson Reviewed-by: Mika Kuoppala > Cc: Libin Yang > Cc: Takashi Iwai > Cc: Marius Vlad > --- > drivers/gpu/drm/i915/intel_audio.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_audio.c > b/drivers/gpu/drm/i915/intel_audio.c > index 5d5f6bc10e85..eedcce6478ef 100644 > --- a/drivers/gpu/drm/i915/intel_audio.c > +++ b/drivers/gpu/drm/i915/intel_audio.c > @@ -600,6 +600,8 @@ static void > i915_audio_component_codec_wake_override(struct device *dev, > if (!IS_SKYLAKE(dev_priv) && !IS_KABYLAKE(dev_priv)) > return; > > + intel_runtime_pm_get(dev_priv); > + > /* >* Enable/disable generating the codec wake signal, overriding the >* internal logic to generate the codec wake to controller. > @@ -615,6 +617,8 @@ static void > i915_audio_component_codec_wake_override(struct device *dev, > I915_WRITE(HSW_AUD_CHICKENBIT, tmp); > usleep_range(1000, 1500); > } > + > + intel_runtime_pm_put(dev_priv); > } > > /* Get CDCLK in kHz */ > @@ -648,6 +652,8 @@ static int i915_audio_component_sync_audio_rate(struct > device *dev, > !IS_HASWELL(dev_priv)) > return 0; > > + intel_runtime_pm_get(dev_priv); > + > mutex_lock(&dev_priv->av_mutex); > /* 1. get the pipe */ > intel_encoder = dev_priv->dig_port_map[port]; > @@ -698,6 +704,7 @@ static int i915_audio_component_sync_audio_rate(struct > device *dev, > > unlock: > mutex_unlock(&dev_priv->av_mutex); > + intel_runtime_pm_put(dev_priv); > return err; > } > > -- > 2.8.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers
On Tue, Jul 12, 2016 at 04:10:22PM +0300, Mika Kuoppala wrote: > Chris Wilson writes: > > > On Haswell/Broadwell, the HD-Audio block is inside the HDMI/display > > power well and so the sna-hda audio codec acquires the display power > > well while it is operational. However, Skylake separates the powerwells > > again, and so we must remember to acquire the rpm wakeref for ourselves > > whilst tweaking the registers. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96214 > > Fixes: 03b135cebc47 "ALSA: hda - remove dependency on i915 power well for > > SKL") > > Signed-off-by: Chris Wilson > > Reviewed-by: Mika Kuoppala > > > Cc: Libin Yang > > Cc: Takashi Iwai > > Cc: Marius Vlad Marius, could you provide a tested by? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers
Did try when you submitted the patch...but can't seem to replicate with latest nightly on other SKLs, and currently do not have access on the machine that caused it. On Tue, Jul 12, 2016 at 02:16:50PM +0100, Chris Wilson wrote: > On Tue, Jul 12, 2016 at 04:10:22PM +0300, Mika Kuoppala wrote: > > Chris Wilson writes: > > > > > On Haswell/Broadwell, the HD-Audio block is inside the HDMI/display > > > power well and so the sna-hda audio codec acquires the display power > > > well while it is operational. However, Skylake separates the powerwells > > > again, and so we must remember to acquire the rpm wakeref for ourselves > > > whilst tweaking the registers. > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96214 > > > Fixes: 03b135cebc47 "ALSA: hda - remove dependency on i915 power well for > > > SKL") > > > Signed-off-by: Chris Wilson > > > > Reviewed-by: Mika Kuoppala > > > > > Cc: Libin Yang > > > Cc: Takashi Iwai > > > Cc: Marius Vlad > > Marius, could you provide a tested by? > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre signature.asc Description: Digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers
On Wed, Jul 13, 2016 at 05:01:02PM +0300, Marius Vlad wrote: > Did try when you submitted the patch...but can't seem to replicate with > latest nightly on other SKLs, and currently do not have access on > the machine that caused it. So fwiw, Hans de Geode confirmed that only reverting 03b135cebc47 (which enables the display powerwell as well) was enough. Libin, do you want to handle the revert or explain the mistake in commit 03b135cebc47d75ea2dc346770374ab741966955 Author: Libin Yang Date: Wed Jun 3 09:30:15 2015 +0800 ALSA: hda - remove controller dependency on i915 power well for SKL -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers
Hi Wilson, > -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Thursday, July 14, 2016 10:34 PM > To: Mika Kuoppala ; intel- > g...@lists.freedesktop.org; Takashi Iwai ; Yang, Libin > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD- > Audio registers > > On Wed, Jul 13, 2016 at 05:01:02PM +0300, Marius Vlad wrote: > > Did try when you submitted the patch...but can't seem to replicate > > with latest nightly on other SKLs, and currently do not have access on > > the machine that caused it. > > So fwiw, Hans de Geode confirmed that only reverting 03b135cebc47 (which > enables the display powerwell as well) was enough. > > Libin, do you want to handle the revert or explain the mistake in What's the problem with the patch? I can find the details from the email thread. HSW/BSW need acquire the power because it is inside the gpu. So we limited need i915 power for controller to HSW/BDW. > > commit 03b135cebc47d75ea2dc346770374ab741966955 > Author: Libin Yang > Date: Wed Jun 3 09:30:15 2015 +0800 > > ALSA: hda - remove controller dependency on i915 power well for SKL > > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers
On Fri, Jul 15, 2016 at 02:40:42AM +, Yang, Libin wrote: > Hi Wilson, > > > -Original Message- > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > Sent: Thursday, July 14, 2016 10:34 PM > > To: Mika Kuoppala ; intel- > > g...@lists.freedesktop.org; Takashi Iwai ; Yang, Libin > > > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD- > > Audio registers > > > > On Wed, Jul 13, 2016 at 05:01:02PM +0300, Marius Vlad wrote: > > > Did try when you submitted the patch...but can't seem to replicate > > > with latest nightly on other SKLs, and currently do not have access on > > > the machine that caused it. > > > > So fwiw, Hans de Geode confirmed that only reverting 03b135cebc47 (which > > enables the display powerwell as well) was enough. > > > > Libin, do you want to handle the revert or explain the mistake in > > What's the problem with the patch? I can find the details from the email > thread. > > HSW/BSW need acquire the power because it is inside the gpu. So we > limited need i915 power for controller to HSW/BDW. Please see Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96214 Judging from the reports, Skylake uses the same display powerwell as HSW/BDW. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers
Hi Wilson, Thanks for tell the details. SKL is different from HSW/BDW. HSW/BDW has 2 audio controllers. One is in PCH and another is in gfx. SKL has one controller, which is in PCH. Both HSW/BDW and SKL has display audio codec in gfx. For HSW/BDW, to use the controller inside gfx, we must get the gfx power. For SKL, to use the controller, we don't need to get the gfx power. For both HSW/BDW and SKL, to use display audio codec, we must get the gfx power. Without the patch, all the audio controllers except BYT/BSW will get the gfx power. However this is not true, SKL and APL audio controller don't need this power. - /* Baytral/Braswell controllers don't need this power */ - if (pci->device != 0x0f04 && pci->device != 0x2284) + /* HSW/BDW controllers need this power */ + if (CONTROLLER_IN_GPU(pci)) hda->need_i915_power = 1; For skl, it need to get gfx power only when display audio codec is used, this is set in the patch: + if (is_valleyview_plus(codec) || is_skylake(codec)) codec->core.link_power_control = 1; Without the patch, SKL may consume gfx power even it doesn't need. This patch has been tested by our full test cases for about one year, it shows the right result. Do you think maybe there is another recent patch cause this issue? I will debug the Bugzilla next week on audio side. Regards, Libin > -Original Message- > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > Sent: Friday, July 15, 2016 4:45 PM > To: Yang, Libin > Cc: Mika Kuoppala ; intel- > g...@lists.freedesktop.org; Takashi Iwai > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD- > Audio registers > > On Fri, Jul 15, 2016 at 02:40:42AM +, Yang, Libin wrote: > > Hi Wilson, > > > > > -Original Message- > > > From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] > > > Sent: Thursday, July 14, 2016 10:34 PM > > > To: Mika Kuoppala ; intel- > > > g...@lists.freedesktop.org; Takashi Iwai ; Yang, Libin > > > > > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm > > > for HD- Audio registers > > > > > > On Wed, Jul 13, 2016 at 05:01:02PM +0300, Marius Vlad wrote: > > > > Did try when you submitted the patch...but can't seem to replicate > > > > with latest nightly on other SKLs, and currently do not have > > > > access on the machine that caused it. > > > > > > So fwiw, Hans de Geode confirmed that only reverting 03b135cebc47 > > > (which enables the display powerwell as well) was enough. > > > > > > Libin, do you want to handle the revert or explain the mistake in > > > > What's the problem with the patch? I can find the details from the email > thread. > > > > HSW/BSW need acquire the power because it is inside the gpu. So we > > limited need i915 power for controller to HSW/BDW. > > Please see > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96214 > > Judging from the reports, Skylake uses the same display powerwell as > HSW/BDW. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
