[Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-Audio registers

[Yang, Libin]

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:chris at chris-wilson.co.uk]
> Sent: Friday, July 15, 2016 4:45 PM
> To: Yang, Libin <libin.yang at intel.com>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>; intel-
> gfx at lists.freedesktop.org; Takashi Iwai <tiwai at suse.de>
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Acquire intel_runtime_pm for HD-
> Audio registers
> 
> On Fri, Jul 15, 2016 at 02:40:42AM +0000, Yang, Libin wrote:
> > Hi Wilson,
> >
> > > -----Original Message-----
> > > From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> > > Sent: Thursday, July 14, 2016 10:34 PM
> > > To: Mika Kuoppala <mika.kuoppala at linux.intel.com>; intel-
> > > gfx at lists.freedesktop.org; Takashi Iwai <tiwai at suse.de>; Yang, Libin
> > > <libin.yang at intel.com>
> > > 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