[PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Keith Packard]

RC6 should always work on IVB, and should work on SNB whenever IO
remapping is disabled. Make the default value for the parameter turn
it on in these cases. Setting the value to either 0 or 1 will force
the specified behavior.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |    4 ++--
 drivers/gpu/drm/i915/i915_drv.h      |    2 +-
 drivers/gpu/drm/i915/intel_display.c |   26 +++++++++++++++++++++++---
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 15bfa91..cf5c1bd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -63,10 +63,10 @@ module_param_named(semaphores, i915_semaphores, int, 0600);
 MODULE_PARM_DESC(semaphores,
 		"Use semaphores for inter-ring sync (default: false)");
 
-unsigned int i915_enable_rc6 __read_mostly = 0;
+int i915_enable_rc6 __read_mostly = -1;
 module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600);
 MODULE_PARM_DESC(i915_enable_rc6,
-		"Enable power-saving render C-state 6 (default: true)");
+		"Enable power-saving render C-state 6 (default: -1 (false when VT-d enabled)");
 
 int i915_enable_fbc __read_mostly = -1;
 module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4a9c1b9..172b022 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1002,7 +1002,7 @@ extern unsigned int i915_semaphores __read_mostly;
 extern unsigned int i915_lvds_downclock __read_mostly;
 extern int i915_panel_use_ssc __read_mostly;
 extern int i915_vbt_sdvo_panel_type __read_mostly;
-extern unsigned int i915_enable_rc6 __read_mostly;
+extern int i915_enable_rc6 __read_mostly;
 extern int i915_enable_fbc __read_mostly;
 extern bool i915_enable_hangcheck __read_mostly;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e77a863..13fd4c0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -38,8 +38,11 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "drm_dp_helper.h"
-
 #include "drm_crtc_helper.h"
+#ifdef CONFIG_INTEL_IOMMU
+#include <linux/dma_remapping.h>
+#include <linux/dmar.h>
+#endif
 
 #define HAS_eDP (intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))
 
@@ -7887,6 +7890,23 @@ void intel_init_emon(struct drm_device *dev)
 	dev_priv->corr = (lcfuse & LCFUSE_HIV_MASK);
 }
 
+static bool intel_enable_rc6(struct drm_device *dev)
+{
+	if (i915_enable_rc6 >= 0)
+		return i915_enable_rc6;
+	if (INTEL_INFO(dev)->gen >= 7)
+		return 1;
+#ifdef CONFIG_INTEL_IOMMU
+	/*
+	 * Only enable RC6 if all dma remapping is disabled, or if
+	 * there's no iommu present in the machine.
+	 */
+	if (INTEL_INFO(dev)->gen == 6)
+		return no_iommu || dmar_disabled;
+#endif
+	return 0;
+}
+
 void gen6_enable_rps(struct drm_i915_private *dev_priv)
 {
 	u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
@@ -7923,7 +7943,7 @@ void gen6_enable_rps(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN6_RC6p_THRESHOLD, 100000);
 	I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */
 
-	if (i915_enable_rc6)
+	if (intel_enable_rc6(dev_priv->dev))
 		rc6_mask = GEN6_RC_CTL_RC6p_ENABLE |
 			GEN6_RC_CTL_RC6_ENABLE;
 
@@ -8372,7 +8392,7 @@ void ironlake_enable_rc6(struct drm_device *dev)
 	/* rc6 disabled by default due to repeated reports of hanging during
 	 * boot and resume.
 	 */
-	if (!i915_enable_rc6)
+	if (!intel_enable_rc6(dev))
 		return;
 
 	mutex_lock(&dev->struct_mutex);
-- 
1.7.7.3

Re: [Intel-gfx] [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Kenneth Graunke]

On 11/18/2011 10:41 PM, Keith Packard wrote:
> RC6 should always work on IVB, and should work on SNB whenever IO
> remapping is disabled. Make the default value for the parameter turn
> it on in these cases. Setting the value to either 0 or 1 will force
> the specified behavior.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c      |    4 ++--
>  drivers/gpu/drm/i915/i915_drv.h      |    2 +-
>  drivers/gpu/drm/i915/intel_display.c |   26 +++++++++++++++++++++++---
>  3 files changed, 26 insertions(+), 6 deletions(-)

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Eugeni Dodonov]

On Sat, Nov 19, 2011 at 04:41, Keith Packard <keithp@keithp.com> wrote:
>
> +static bool intel_enable_rc6(struct drm_device *dev)
> +{
> +       if (i915_enable_rc6 >= 0)
> +               return i915_enable_rc6;
> +       if (INTEL_INFO(dev)->gen >= 7)
> +               return 1;
> +#ifdef CONFIG_INTEL_IOMMU
> +       /*
> +        * Only enable RC6 if all dma remapping is disabled, or if
> +        * there's no iommu present in the machine.
> +        */
> +       if (INTEL_INFO(dev)->gen == 6)
> +               return no_iommu || dmar_disabled;
> +#endif
> +       return 0;
> +}

Just one question I caught on 2nd read. Shouldn't we have #else within
this #ifdef block, to return 1? Otherwise, if CONFIG_INTEL_IOMMU is
not defined, we'll always disable rc6.

Or we just always intend to disable rc6 in case CONFIG_INTEL_IOMMU is not there?

--
Eugeni Dodonov

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

On Sat, 19 Nov 2011 07:25:09 -0200, Eugeni Dodonov <eugeni@dodonov.net> wrote:

> Just one question I caught on 2nd read. Shouldn't we have #else within
> this #ifdef block, to return 1? Otherwise, if CONFIG_INTEL_IOMMU is
> not defined, we'll always disable rc6.

Oops! Thanks for catching this. Here's a new version of that function
(the rest of the patch is the same). This one has explicit conditions
for Ironlake and Sandybridge (when CONFIG_INTEL_IOMMU is set), allowing
the Ivybridge and Sandybridge-without-IOMMU cases to take the default
path. This will also cause all future chips to enable rc6 by default.
 
+static bool intel_enable_rc6(struct drm_device *dev)
+{
+	/*
+	 * Respect the kernel parameter if it is set
+	 */
+	if (i915_enable_rc6 >= 0)
+		return i915_enable_rc6;
+
+	/*
+	 * Disable RC6 on Ironlake
+	 */
+	if (INTEL_INFO(dev)->gen == 5)
+		return 0;
+
+#ifdef CONFIG_INTEL_IOMMU
+	/*
+	 * Enable rc6 on Sandybridge if DMA remapping is disabled
+	 */
+	if (INTEL_INFO(dev)->gen == 6)
+		return no_iommu || dmar_disabled;
+#endif
+	return 1;
+}
+

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Eugeni Dodonov]

On Sat, Nov 19, 2011 at 16:32, Keith Packard <keithp@keithp.com> wrote:
> On Sat, 19 Nov 2011 07:25:09 -0200, Eugeni Dodonov <eugeni@dodonov.net> wrote:
>
>> Just one question I caught on 2nd read. Shouldn't we have #else within
>> this #ifdef block, to return 1? Otherwise, if CONFIG_INTEL_IOMMU is
>> not defined, we'll always disable rc6.
>
> Oops! Thanks for catching this. Here's a new version of that function
> (the rest of the patch is the same). This one has explicit conditions
> for Ironlake and Sandybridge (when CONFIG_INTEL_IOMMU is set), allowing
> the Ivybridge and Sandybridge-without-IOMMU cases to take the default
> path. This will also cause all future chips to enable rc6 by default.

Great, I think it catches all cases now.

Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

Thanks!

-- 
Eugeni Dodonov

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Matthew Garrett]

On Fri, Nov 18, 2011 at 10:41:29PM -0800, Keith Packard wrote:

> +	/*
> +	 * Only enable RC6 if all dma remapping is disabled, or if
> +	 * there's no iommu present in the machine.
> +	 */
> +	if (INTEL_INFO(dev)->gen == 6)
> +		return no_iommu || dmar_disabled;

So the user has to choose between 5W of power saving or having dmar? And 
we default to giving them dmar? I think that's going to come as a 
surprise to people.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Matthew Garrett]

On Tue, Nov 22, 2011 at 06:46:21PM -0200, Eugeni Dodonov wrote:
> On Tue, Nov 22, 2011 at 18:15, Matthew Garrett <mjg@redhat.com> wrote:
> 
> > On Fri, Nov 18, 2011 at 10:41:29PM -0800, Keith Packard wrote:
> >
> > > +     /*
> > > +      * Only enable RC6 if all dma remapping is disabled, or if
> > > +      * there's no iommu present in the machine.
> > > +      */
> > > +     if (INTEL_INFO(dev)->gen == 6)
> > > +             return no_iommu || dmar_disabled;
> >
> > So the user has to choose between 5W of power saving or having dmar? And
> > we default to giving them dmar?
> 
> 
> From the latest investigations, it is either this, or random gpu hangs and
> crashes when both are enabled :(.

So blacklist dmar on sandybridge. The power saving is going to be more 
important for most users.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 668 bytes --]

On Tue, 22 Nov 2011 20:15:31 +0000, Matthew Garrett <mjg@redhat.com> wrote:

> So the user has to choose between 5W of power saving or having dmar? And 
> we default to giving them dmar? I think that's going to come as a 
> surprise to people.

You'd have to go into the BIOS to turn this on for most machines at
least?

But, yeah, it seems like we should be turning DMAR off unless explicitly
requested; I can't understand how you'd ever need this running native on
the hardware. Not exactly an area I care about deeply; I've always
worked hard to make sure all virtualization garbage is disabled on every
machine I use.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Daniel Vetter]

On Tue, Nov 22, 2011 at 07:31:34PM -0800, Keith Packard wrote:
> On Tue, 22 Nov 2011 20:15:31 +0000, Matthew Garrett <mjg@redhat.com> wrote:
> 
> > So the user has to choose between 5W of power saving or having dmar? And 
> > we default to giving them dmar? I think that's going to come as a 
> > surprise to people.
> 
> You'd have to go into the BIOS to turn this on for most machines at
> least?
> 
> But, yeah, it seems like we should be turning DMAR off unless explicitly
> requested; I can't understand how you'd ever need this running native on
> the hardware. Not exactly an area I care about deeply; I've always
> worked hard to make sure all virtualization garbage is disabled on every
> machine I use.

Problem is that we need to disable dmar on the entire box, afaics. And I
assume that a bunch of people abusing desktop boards as servers will call
"regression" on that.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

On Wed, 2011-11-23 at 11:26 +0100, Daniel Vetter wrote:
> On Tue, Nov 22, 2011 at 07:31:34PM -0800, Keith Packard wrote:
> > On Tue, 22 Nov 2011 20:15:31 +0000, Matthew Garrett <mjg@redhat.com> wrote:
> > 
> > > So the user has to choose between 5W of power saving or having dmar? And 
> > > we default to giving them dmar? I think that's going to come as a 
> > > surprise to people.
> > 
> > You'd have to go into the BIOS to turn this on for most machines at
> > least?
> > 
> > But, yeah, it seems like we should be turning DMAR off unless explicitly
> > requested; I can't understand how you'd ever need this running native on
> > the hardware. Not exactly an area I care about deeply; I've always
> > worked hard to make sure all virtualization garbage is disabled on every
> > machine I use.
> 
> Problem is that we need to disable dmar on the entire box, afaics. And I
> assume that a bunch of people abusing desktop boards as servers will call
> "regression" on that.

Hm, do you really have to disable it for the entire box, or just the
graphics?

Do we have a coherent erratum from Intel for the issues mentioned above
with DMAR+gfx+RC6?

Keith, do you know if a sighting has been filed and the hardware folks
are working on it?

Rajesh, are you familiar with this issue?

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5818 bytes --]

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Daniel Vetter]

On Wed, Nov 23, 2011 at 02:01:54PM +0000, David Woodhouse wrote:
> On Wed, 2011-11-23 at 11:26 +0100, Daniel Vetter wrote:
> > On Tue, Nov 22, 2011 at 07:31:34PM -0800, Keith Packard wrote:
> > > On Tue, 22 Nov 2011 20:15:31 +0000, Matthew Garrett <mjg@redhat.com> wrote:
> > > 
> > > > So the user has to choose between 5W of power saving or having dmar? And 
> > > > we default to giving them dmar? I think that's going to come as a 
> > > > surprise to people.
> > > 
> > > You'd have to go into the BIOS to turn this on for most machines at
> > > least?
> > > 
> > > But, yeah, it seems like we should be turning DMAR off unless explicitly
> > > requested; I can't understand how you'd ever need this running native on
> > > the hardware. Not exactly an area I care about deeply; I've always
> > > worked hard to make sure all virtualization garbage is disabled on every
> > > machine I use.
> > 
> > Problem is that we need to disable dmar on the entire box, afaics. And I
> > assume that a bunch of people abusing desktop boards as servers will call
> > "regression" on that.
> 
> Hm, do you really have to disable it for the entire box, or just the
> graphics?

At least for the dmar+gfx+semaphores hang I can reproduce, just disabling
dmar with intel_iommu=igfx_off is not good enough and iirc the same holds
for the dmar+rc6 hangs reported.

> Do we have a coherent erratum from Intel for the issues mentioned above
> with DMAR+gfx+RC6?

Afaik no errata applies to our dmar related troubles on snb. I've hoped
that ppgtt would magically fix this, and it seems to help quite a bit for
the semaphore hangs (but not everywhere). Couldn't yet look more into
this.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

On Wed, 2011-11-23 at 15:39 +0100, Daniel Vetter wrote:
> At least for the dmar+gfx+semaphores hang I can reproduce, just disabling
> dmar with intel_iommu=igfx_off is not good enough and iirc the same holds
> for the dmar+rc6 hangs reported. 

Um... let me restate that for clarity (and partly for Rajesh's benefit).

The DMAR associated with the integrated graphics is *disabled*.
Turned off. Not active. Ever.

You have a problem when you enable the *other* DMAR units in the system,
which should not be affecting the graphics device in any way.

When you do this, you see 'hangs' with semaphores and RC6. Is there a
better description of these 'hangs' somewhere? Is the hardware
completely locked?

These hangs go away when you disable the DMAR units. Again, that is the
*other* DMAR units in the system that have nothing to do with graphics.

While I'm getting quite used to DMAR-related errata, this one does make
me stop and think 'wtf?'. It just seems so incongruous that disabling an
*unrelated* IOMMU would make the problem go away, and it makes me wonder
if it's actually a timing-related issue which is always there, but
something about the use of DMAR for network/disk/etc. makes it more
likely to trigger?

We definitely need the hardware folks to get to the bottom of this one.

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5818 bytes --]

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Daniel Vetter]

On Wed, Nov 23, 2011 at 03:03:43PM +0000, David Woodhouse wrote:
> On Wed, 2011-11-23 at 15:39 +0100, Daniel Vetter wrote:
> > At least for the dmar+gfx+semaphores hang I can reproduce, just disabling
> > dmar with intel_iommu=igfx_off is not good enough and iirc the same holds
> > for the dmar+rc6 hangs reported. 
> 
> Um... let me restate that for clarity (and partly for Rajesh's benefit).
> 
> The DMAR associated with the integrated graphics is *disabled*.
> Turned off. Not active. Ever.
> 
> You have a problem when you enable the *other* DMAR units in the system,
> which should not be affecting the graphics device in any way.
> 
> When you do this, you see 'hangs' with semaphores and RC6. Is there a
> better description of these 'hangs' somewhere? Is the hardware
> completely locked?
> 
> These hangs go away when you disable the DMAR units. Again, that is the
> *other* DMAR units in the system that have nothing to do with graphics.
> 
> While I'm getting quite used to DMAR-related errata, this one does make
> me stop and think 'wtf?'. It just seems so incongruous that disabling an
> *unrelated* IOMMU would make the problem go away, and it makes me wonder
> if it's actually a timing-related issue which is always there, but
> something about the use of DMAR for network/disk/etc. makes it more
> likely to trigger?
> 
> We definitely need the hardware folks to get to the bottom of this one.

Ok, let me document the recipe I use to hang my box here. It's about the
dmar+semaphores hang I can reproduce, so might be slightly different in
the actual cause than the dmar+rc6 bug (for that one we only have bug
reports talking about hard freezing requiring power cycling).

- Grab a GT2+ mobile snb (both my and the only other reporters machine
  fits this, so maybe it matters). pci rev 09 (i.e. first production
  silicon).
- Install fc15 with the kde4 spin. I can't reproduce it with any other
  userspace than kde4.
- Grab latest d-i-f from Keith and latest userspace graphics code (to
  avoid hitting any other snb hangs we've tracked down meanwhile).
- Compile kernel with dmar and enable VT-d in the bios.
- Login into the systems with gdm, the machine usually dies within a few
  seconds (while kde4 loads). If that's not good enough, a few minutes of
  light desktop usage will kill it.
- Wait 2 minutes for the stuck-in-atomic detection logic to kick in and
  grab the backtrace over netconsole. Notice that the kernel is stuck
  trying to flush the dmar tlb cache (that's how I managed to track it
  down to a dmar interaction). Backtrace almost identical to the dmar
  issue on ilk. I've lost the backtrace, if you want I can regrab it.

Things I've tried that don't work around the issue:
- Disable dmar for the igfx with intel_iommu=igfx_off
- Apply the ilk workaround (i.e. synchronous dmar tlb flushes + gpu idling
  while flushing).

Things that work:
- Disabling semaphores.
- Disabling dmar in either the bios or on the cmdline with intel_iommu=off

All reporters that tried confirmed that igfx_off is not good enough, only
fully disabling dmar (for both the semaphores and the rc6 related hangs).

Things that look interesting:
- ppgtt support (i.e. using per-proces pagetables on the gfx instead of
  the global gtt) seems to paper over the issue for the original reporter
  of the semaphore related hangs.  Unfortunately not for me, gpu still
  hangs (but doesn't take down the entire system with it). I've not yet
  investigated this one closely. Fyi, the windows driver uses ppgtt
  unconditionally on snb. Also, ppgtt seems to have no effect for at least
  one report of dmar related rc6 hangs.

Cheers, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 916 bytes --]

On Wed, 2011-11-23 at 16:31 +0100, Daniel Vetter wrote:
> 
> Things I've tried that don't work around the issue:
> - Disable dmar for the igfx with intel_iommu=igfx_off
> - Apply the ilk workaround (i.e. synchronous dmar tlb flushes + gpu
> idling while flushing).

Well, the ILK workaround was only ensuring that the gfx was idle while
the *graphics* IOTLB was flushed. We are still flushing the IOTLB for
*other* IOMMUs at arbitrary times. 

Might be interesting to go for *deferred* IOTLB flushes (i.e. the normal
mode without the workaround or 'intel_iommu=strict' on the command
line), *BUT* to make sure the batched flush (the flush_unmaps() call)
can *only* happen when the GPU is quiescent. You could do that with a
dirty hack for testing, and if *that* fixes the issue.... well, $DEITY
knows, someone still needs a good kicking, but at least we'll have
learned something :)
 
-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5818 bytes --]

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Daniel Vetter]

On Wed, Nov 23, 2011 at 03:03:43PM +0000, David Woodhouse wrote:
> On Wed, 2011-11-23 at 15:39 +0100, Daniel Vetter wrote:
> > At least for the dmar+gfx+semaphores hang I can reproduce, just disabling
> > dmar with intel_iommu=igfx_off is not good enough and iirc the same holds
> > for the dmar+rc6 hangs reported. 
> 
> Um... let me restate that for clarity (and partly for Rajesh's benefit).
> 
> The DMAR associated with the integrated graphics is *disabled*.
> Turned off. Not active. Ever.

Hm, that comment confuses me a bit. I've always thought that igfx_off only
instantiates a identity mapping and leaves the dmar unit on. Is that
wrong?
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

On Wed, 2011-11-23 at 16:41 +0100, Daniel Vetter wrote:
> Hm, that comment confuses me a bit. I've always thought that igfx_off only
> instantiates a identity mapping and leaves the dmar unit on. Is that
> wrong? 

It's completely off. If a DMAR unit has *only* graphics devices behind
it (as the one for the built-in graphics does, because it's a whole
boatload of speshul case for integration with the GTT), we just don't
enable it at all. See the second for_each_drhd_unit() loop in
init_no_remapping_devices().

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5818 bytes --]

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Daniel Vetter]

On Wed, Nov 23, 2011 at 04:31:54PM +0100, Daniel Vetter wrote:
> - Wait 2 minutes for the stuck-in-atomic detection logic to kick in and
>   grab the backtrace over netconsole. Notice that the kernel is stuck
>   trying to flush the dmar tlb cache (that's how I managed to track it
>   down to a dmar interaction). Backtrace almost identical to the dmar
>   issue on ilk. I've lost the backtrace, if you want I can regrab it.

Ok, I've recaptured the last words from my dying machine:

Listening for netconsole messages
[  136.897673] ------------[ cut here ]------------
[  136.897694] WARNING: at kernel/watchdog.c:241 watchdog_overflow_callback+0x9b/0xa6()
[  136.897701] Hardware name: HP EliteBook 8460p
[  136.897707] Watchdog detected hard LOCKUP on cpu 0
[  136.897713] Modules linked in: sunrpc 8021q garp stp llc cpufreq_ondemand acpi_cpufreq freq_table mperf snd_hda_codec_hdmi snd_hda_codec_idt arc4 iwlwifi mac80211 hp_wmi sparse_keymap ppdev uvcvideo videodev v4l2_compat_ioctl32 btusb microcode snd_hda_intel snd_hda_codec snd_hwdep snd_seq bluetooth snd_seq_device iTCO_wdt snd_pcm snd_timer snd cfg80211 serio_raw iTCO_vendor_support joydev xhci_hcd rfkill e1000e soundcore snd_page_alloc parport_pc parport tpm_infineon wmi intel_ips ipv6 firewire_ohci firewire_core crc_itu_t sdhci_pci sdhci mmc_core i915 drm_kms_helper drm i2c_algo_bit i2c_core video [last unloaded: scsi_wait_scan]
[  136.897967] Pid: 0, comm: swapper Not tainted 3.2.0-rc2+ #162
[  136.897972] Call Trace:
[  136.897978]  <NMI>  [<ffffffff8105679a>] warn_slowpath_common+0x83/0x9b
[  136.897998]  [<ffffffff81056855>] warn_slowpath_fmt+0x46/0x48
[  136.898007]  [<ffffffff810152b1>] ? native_sched_clock+0x34/0x36
[  136.898016]  [<ffffffff810ad68b>] watchdog_overflow_callback+0x9b/0xa6
[  136.898026]  [<ffffffff810d78c3>] __perf_event_overflow+0x100/0x17f
[  136.898035]  [<ffffffff810d5f53>] ? perf_event_update_userpage+0xf/0xa2
[  136.898045]  [<ffffffff8101be7b>] ? x86_perf_event_set_period+0x107/0x113
[  136.898053]  [<ffffffff810d7efc>] perf_event_overflow+0x14/0x16
[  136.898062]  [<ffffffff8101f4bc>] intel_pmu_handle_irq+0x211/0x271
[  136.898073]  [<ffffffff81476b65>] perf_event_nmi_handler+0x19/0x1b
[  136.898082]  [<ffffffff814764f7>] nmi_handle+0x42/0x67
[  136.898091]  [<ffffffff814765a8>] do_nmi+0x8c/0x26b
[  136.898099]  [<ffffffff81475db0>] nmi+0x20/0x30
[  136.898109]  [<ffffffff81083ccc>] ? do_raw_spin_lock+0x1/0x25
[  136.898115]  <<EOE>>  <IRQ>  [<ffffffff8147547e>] ? _raw_spin_lock+0xe/0x10
[  136.898135]  [<ffffffff813b712b>] qi_submit_sync+0x30d/0x312
[  136.898143]  [<ffffffff813b7222>] qi_flush_iotlb+0x7a/0x7c
[  136.898152]  [<ffffffff813b918f>] flush_unmaps+0x72/0x131
[  136.898161]  [<ffffffff813b926d>] flush_unmaps_timeout+0x1f/0x32
[  136.898169]  [<ffffffff81062d9d>] run_timer_softirq+0x19b/0x280
[  136.898177]  [<ffffffff81014e05>] ? paravirt_read_tsc+0x9/0xd
[  136.898186]  [<ffffffff813b924e>] ? flush_unmaps+0x131/0x131
[  136.898195]  [<ffffffff8105c477>] __do_softirq+0xc9/0x1b5
[  136.898203]  [<ffffffff81014e05>] ? paravirt_read_tsc+0x9/0xd
[  136.898212]  [<ffffffff8147de6c>] call_softirq+0x1c/0x30
[  136.898222]  [<ffffffff81010add>] do_softirq+0x46/0x81
[  136.898230]  [<ffffffff8105c73f>] irq_exit+0x57/0xb1
[  136.898238]  [<ffffffff8147e7e1>] smp_apic_timer_interrupt+0x7c/0x8a
[  136.898251]  [<ffffffff8147c6de>] apic_timer_interrupt+0x6e/0x80
[  136.898256]  <EOI>  [<ffffffff81014e05>] ? paravirt_read_tsc+0x9/0xd
[  136.898271]  [<ffffffff812766b3>] ? intel_idle+0xef/0x120
[  136.898279]  [<ffffffff81276695>] ? intel_idle+0xd1/0x120
[  136.898289]  [<ffffffff8139f10b>] cpuidle_idle_call+0xe2/0x181
[  136.898297]  [<ffffffff8100e2ed>] cpu_idle+0xa9/0xf0
[  136.898306]  [<ffffffff81456a1e>] rest_init+0x72/0x74
[  136.898316]  [<ffffffff81aceb71>] start_kernel+0x3b0/0x3bd
[  136.898324]  [<ffffffff81ace2c4>] x86_64_start_reservations+0xaf/0xb3
[  136.898332]  [<ffffffff81ace140>] ? early_idt_handlers+0x140/0x140
[  136.898340]  [<ffffffff81ace3ca>] x86_64_start_kernel+0x102/0x111
[  136.898348] ---[ end trace 2d22d2d9c3bfe5c8 ]---
[  161.821354] ------------[ cut here ]------------
[  161.821365] WARNING: at kernel/watchdog.c:241 watchdog_overflow_callback+0x9b/0xa6()
[  161.821370] Hardware name: HP EliteBook 8460p
[  161.821376] Watchdog detected hard LOCKUP on cpu 1
[  161.821381] Modules linked in: sunrpc 8021q garp stp llc cpufreq_ondemand acpi_cpufreq freq_table mperf snd_hda_codec_hdmi snd_hda_codec_idt arc4 iwlwifi mac80211 hp_wmi sparse_keymap ppdev uvcvideo videodev v4l2_compat_ioctl32 btusb microcode snd_hda_intel snd_hda_codec snd_hwdep snd_seq bluetooth snd_seq_device iTCO_wdt snd_pcm snd_timer snd cfg80211 serio_raw iTCO_vendor_support joydev xhci_hcd rfkill e1000e soundcore snd_page_alloc parport_pc parport tpm_infineon wmi intel_ips ipv6 firewire_ohci firewire_core crc_itu_t sdhci_pci sdhci mmc_core i915 drm_kms_helper drm i2c_algo_bit i2c_core video [last unloaded: scsi_wait_scan]
[  161.821609] Pid: 1134, comm: X Tainted: G        W    3.2.0-rc2+ #162
[  161.821615] Call Trace:
[  161.821619]  <NMI>  [<ffffffff8105679a>] warn_slowpath_common+0x83/0x9b
[  161.821633]  [<ffffffff81056855>] warn_slowpath_fmt+0x46/0x48
[  161.821640]  [<ffffffff810152b1>] ? native_sched_clock+0x34/0x36
[  161.821648]  [<ffffffff810ad68b>] watchdog_overflow_callback+0x9b/0xa6
[  161.821655]  [<ffffffff810d78c3>] __perf_event_overflow+0x100/0x17f
[  161.821663]  [<ffffffff810d5f53>] ? perf_event_update_userpage+0xf/0xa2
[  161.821669]  [<ffffffff8101be7b>] ? x86_perf_event_set_period+0x107/0x113
[  161.821677]  [<ffffffff810d7efc>] perf_event_overflow+0x14/0x16
[  161.821684]  [<ffffffff8101f4bc>] intel_pmu_handle_irq+0x211/0x271
[  161.821692]  [<ffffffff81476b65>] perf_event_nmi_handler+0x19/0x1b
[  161.821700]  [<ffffffff814764f7>] nmi_handle+0x42/0x67
[  161.821708]  [<ffffffff814765a8>] do_nmi+0x8c/0x26b
[  161.821715]  [<ffffffff81475db0>] nmi+0x20/0x30
[  161.821723]  [<ffffffff814754aa>] ? _raw_spin_lock_irqsave+0x2a/0x2f
[  161.821728]  <<EOE>>  [<ffffffff813b988f>] add_unmap+0x21/0xb8
[  161.821744]  [<ffffffff813bad66>] intel_unmap_sg+0x101/0x110
[  161.821753]  [<ffffffff811341bc>] ? __pollwait+0xcc/0xcc
[  161.821761]  [<ffffffff812df78f>] intel_gtt_unmap_memory+0x50/0x70
[  161.821784]  [<ffffffffa007dad1>] i915_gem_gtt_unbind_object+0x9c/0xc7 [i915]
[  161.821805]  [<ffffffffa0079377>] i915_gem_object_unbind+0x111/0x1cb [i915]
[  161.821822]  [<ffffffffa0079453>] i915_gem_free_object_tail+0x22/0xd3 [i915]
[  161.821839]  [<ffffffffa007b58c>] i915_gem_free_object+0x46/0x4b [i915]
[  161.821856]  [<ffffffffa001ffa1>] ? drm_gem_handle_create+0xcb/0xcb [drm]
[  161.821870]  [<ffffffffa001ffcc>] drm_gem_object_free+0x2b/0x2d [drm]
[  161.821877]  [<ffffffff8123057b>] kref_put+0x43/0x4d
[  161.821890]  [<ffffffffa001fca5>] drm_gem_object_unreference_unlocked+0x33/0x40 [drm]
[  161.821904]  [<ffffffffa001fdf8>] drm_gem_object_handle_unreference_unlocked.part.1+0x27/0x2c [drm]
[  161.821918]  [<ffffffffa001fec8>] drm_gem_handle_delete+0x84/0x92 [drm]
[  161.821933]  [<ffffffffa0020305>] drm_gem_close_ioctl+0x28/0x2a [drm]
[  161.821946]  [<ffffffffa001e7ae>] drm_ioctl+0x2c8/0x3a5 [drm]
[  161.821958]  [<ffffffffa00202dd>] ? drm_gem_destroy+0x43/0x43 [drm]
[  161.821966]  [<ffffffff810749a6>] ? __hrtimer_start_range_ns+0x2cd/0x2ed
[  161.821974]  [<ffffffff811337f4>] do_vfs_ioctl+0x45d/0x49e
[  161.821982]  [<ffffffff81124f86>] ? fsnotify_access+0x5f/0x67
[  161.821988]  [<ffffffff8113388b>] sys_ioctl+0x56/0x7b
[  161.821995]  [<ffffffff8147bc02>] system_call_fastpath+0x16/0x1b
[  161.822002] ---[ end trace 2d22d2d9c3bfe5c9 ]---
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

[PATCH] iommu: export no_iommu and dmar_disabled symbols

[Eugeni Dodonov]

Those are needed by the rc6 and semaphores support in i915 driver.

In i915 driver, we do not enable either rc6 or semaphores on SNB when dmar
is enabled. So we use those variables to check the io remapping and iommu
status.

CC: Keith Packard <keithp@keithp.com>
CC: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 arch/x86/kernel/pci-dma.c   |    1 +
 drivers/iommu/intel-iommu.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 80dc793..4c9191e 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -33,6 +33,7 @@ int force_iommu __read_mostly = 0;
 int iommu_merge __read_mostly = 0;
 
 int no_iommu __read_mostly;
+EXPORT_SYMBOL_GPL(no_iommu);
 /* Set this to 1 if there is a HW IOMMU in the system */
 int iommu_detected __read_mostly = 0;
 
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index c0c7820..dfe5fd3 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -404,6 +404,7 @@ int dmar_disabled = 0;
 #else
 int dmar_disabled = 1;
 #endif /*CONFIG_INTEL_IOMMU_DEFAULT_ON*/
+EXPORT_SYMBOL_GPL(dmar_disabled);
 
 static int dmar_map_gfx = 1;
 static int dmar_forcedac;
-- 
1.7.7.4

Re: [PATCH] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Daniel Vetter]

On Wed, Nov 23, 2011 at 03:43:05PM +0000, David Woodhouse wrote:
> On Wed, 2011-11-23 at 16:41 +0100, Daniel Vetter wrote:
> > Hm, that comment confuses me a bit. I've always thought that igfx_off only
> > instantiates a identity mapping and leaves the dmar unit on. Is that
> > wrong? 
> 
> It's completely off. If a DMAR unit has *only* graphics devices behind
> it (as the one for the built-in graphics does, because it's a whole
> boatload of speshul case for integration with the GTT), we just don't
> enable it at all. See the second for_each_drhd_unit() loop in
> init_no_remapping_devices().

That explanation confused me even more. So I've rechecked with
intel_iommu=igfx_off and already thought I've made a decent fool of
myself because I couldn't readily hang it. Turns out it's just much harder
to hang with that. So I think moving around the tlb flushing for other
dmar nodes to align with the idled igfx isn't a great solution, simply
because I can't reliably tell whether it fixes anything.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [Intel-gfx] [PATCH] iommu: export no_iommu and dmar_disabled symbols

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

On Wed, 2011-11-23 at 16:42 -0200, Eugeni Dodonov wrote:
> Those are needed by the rc6 and semaphores support in i915 driver.
> 
> In i915 driver, we do not enable either rc6 or semaphores on SNB when dmar
> is enabled. So we use those variables to check the io remapping and iommu
> status.

This is horrid. In the general case, drivers have no business knowing
this. We need to properly identify what is wrong with this hardware, and
put in a quirk for it — perhaps refusing to enable the IOMMU at all on
the broken chipsets.

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5818 bytes --]

Re: [Intel-gfx] [PATCH] iommu: export no_iommu and dmar_disabled symbols

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 787 bytes --]

On Wed, 23 Nov 2011 20:36:00 +0000, David Woodhouse <dwmw2@infradead.org> wrote:

> This is horrid. In the general case, drivers have no business knowing
> this. We need to properly identify what is wrong with this hardware, and
> put in a quirk for it — perhaps refusing to enable the IOMMU at all on
> the broken chipsets.

That'd be fine with me, right now we're disabling RC6 and semaphores --
semaphores aren't all that important, although they do improve
performance a bit. RC6 is important, saving a huge amount of power.

I'd also be OK with requiring special options to enable DMAR and having
that also disable RC6/semaphores, if you'd rather expose that. In either
case, we need something that works and avoids hanging machines.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --]

drm/i915: Enabling RC6 where possible

[Keith Packard]

Ok, here's a "final" patch set to enable RC6 where possible on SNB and IVB
machines.

The first patch creates a new variable, intel_iommu_enabled, that is
exported by the intel iommu code and set when that code has
successfully initialized itself. The old plan of using no_iommu ||
dmar_disabled would work -- those variables are set only by kernel
parameters and don't reflect what the system is actually doing about
virtualization.


The second patch uses that value on SNB to tell whether RC6 can be
enabled by default. On IVB, RC6 is always enabled.


Of course, in all cases, you can override the RC6 setting with the
i915 module parameter.

For those of you who have experienced the delights of RC6 crashing
your machines, please test as this will be heading to 3.2 unless you
find something wrong with it.

-keith

[PATCH 1/2] iommu: Export intel_iommu_enabled to signal when iommu is in use

[Keith Packard]

From: Eugeni Dodonov <eugeni.dodonov@intel.com>

In i915 driver, we do not enable either rc6 or semaphores on SNB when dmar
is enabled. The new 'intel_iommu_enabled' variable signals when the
iommu code is in operation.

Cc: Ted Phelps <phelps@gnusto.com>
Cc: Peter <pab1612@gmail.com>
Cc: Lukas Hejtmanek <xhejtman@fi.muni.cz>
Cc: Andrew Lutomirski <luto@mit.edu>
CC: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Eugeni Dodonov <eugeni.dodonov@intel.com>
Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/iommu/intel-iommu.c   |    5 +++++
 include/linux/dma_remapping.h |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index c0c7820..8dc19b8 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -405,6 +405,9 @@ int dmar_disabled = 0;
 int dmar_disabled = 1;
 #endif /*CONFIG_INTEL_IOMMU_DEFAULT_ON*/
 
+int intel_iommu_enabled = 0;
+EXPORT_SYMBOL_GPL(intel_iommu_enabled);
+
 static int dmar_map_gfx = 1;
 static int dmar_forcedac;
 static int intel_iommu_strict;
@@ -3647,6 +3650,8 @@ int __init intel_iommu_init(void)
 
 	bus_register_notifier(&pci_bus_type, &device_nb);
 
+	intel_iommu_enabled = 1;
+
 	return 0;
 }
 
diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index ef90cbd..57c9a8a 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -31,6 +31,7 @@ extern void free_dmar_iommu(struct intel_iommu *iommu);
 extern int iommu_calculate_agaw(struct intel_iommu *iommu);
 extern int iommu_calculate_max_sagaw(struct intel_iommu *iommu);
 extern int dmar_disabled;
+extern int intel_iommu_enabled;
 #else
 static inline int iommu_calculate_agaw(struct intel_iommu *iommu)
 {
@@ -44,6 +45,7 @@ static inline void free_dmar_iommu(struct intel_iommu *iommu)
 {
 }
 #define dmar_disabled	(1)
+#define intel_iommu_enabled (0)
 #endif
 
 
-- 
1.7.7.3

[PATCH 2/2] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Keith Packard]

RC6 should always work on IVB, and should work on SNB whenever IO
remapping is disabled. RC6 never works on Ironlake. Make the default
value for the parameter follow these guidelines. Setting the value
to either 0 or 1 will force the specified behavior.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38567
Cc: Ted Phelps <phelps@gnusto.com>
Cc: Peter <pab1612@gmail.com>
Cc: Lukas Hejtmanek <xhejtman@fi.muni.cz>
Cc: Andrew Lutomirski <luto@mit.edu>
---
 drivers/gpu/drm/i915/i915_drv.c      |    4 ++--
 drivers/gpu/drm/i915/i915_drv.h      |    2 +-
 drivers/gpu/drm/i915/intel_display.c |   33 ++++++++++++++++++++++++++++++---
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 28836fe..47a42eb 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -63,10 +63,10 @@ module_param_named(semaphores, i915_semaphores, int, 0600);
 MODULE_PARM_DESC(semaphores,
 		"Use semaphores for inter-ring sync (default: false)");
 
-unsigned int i915_enable_rc6 __read_mostly = 0;
+int i915_enable_rc6 __read_mostly = -1;
 module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600);
 MODULE_PARM_DESC(i915_enable_rc6,
-		"Enable power-saving render C-state 6 (default: true)");
+		"Enable power-saving render C-state 6 (default: -1 (use per-chip default)");
 
 int i915_enable_fbc __read_mostly = -1;
 module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1421118..bb82ef8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1008,7 +1008,7 @@ extern unsigned int i915_semaphores __read_mostly;
 extern unsigned int i915_lvds_downclock __read_mostly;
 extern int i915_panel_use_ssc __read_mostly;
 extern int i915_vbt_sdvo_panel_type __read_mostly;
-extern unsigned int i915_enable_rc6 __read_mostly;
+extern int i915_enable_rc6 __read_mostly;
 extern int i915_enable_fbc __read_mostly;
 extern bool i915_enable_hangcheck __read_mostly;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a03ebf6..3097104 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -38,8 +38,8 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "drm_dp_helper.h"
-
 #include "drm_crtc_helper.h"
+#include <linux/dma_remapping.h>
 
 #define HAS_eDP (intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))
 
@@ -7928,6 +7928,33 @@ void intel_init_emon(struct drm_device *dev)
 	dev_priv->corr = (lcfuse & LCFUSE_HIV_MASK);
 }
 
+static bool intel_enable_rc6(struct drm_device *dev)
+{
+	/*
+	 * Respect the kernel parameter if it is set
+	 */
+	if (i915_enable_rc6 >= 0)
+		return i915_enable_rc6;
+
+	/*
+	 * Disable RC6 on Ironlake
+	 */
+	if (INTEL_INFO(dev)->gen == 5)
+		return 0;
+
+	/*
+	 * Enable rc6 on Sandybridge if DMA remapping is disabled
+	 */
+	if (INTEL_INFO(dev)->gen == 6) {
+		DRM_DEBUG_DRIVER("Sandybridge: intel_iommu_enabled %s -- RC6 %sabled\n",
+				 intel_iommu_enabled ? "true" : "false",
+				 !intel_iommu_enabled ? "en" : "dis");
+		return !intel_iommu_enabled;
+	}
+	DRM_DEBUG_DRIVER("RC6 enabled\n");
+	return 1;
+}
+
 void gen6_enable_rps(struct drm_i915_private *dev_priv)
 {
 	u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
@@ -7964,7 +7991,7 @@ void gen6_enable_rps(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN6_RC6p_THRESHOLD, 100000);
 	I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */
 
-	if (i915_enable_rc6)
+	if (intel_enable_rc6(dev_priv->dev))
 		rc6_mask = GEN6_RC_CTL_RC6p_ENABLE |
 			GEN6_RC_CTL_RC6_ENABLE;
 
@@ -8413,7 +8440,7 @@ void ironlake_enable_rc6(struct drm_device *dev)
 	/* rc6 disabled by default due to repeated reports of hanging during
 	 * boot and resume.
 	 */
-	if (!i915_enable_rc6)
+	if (!intel_enable_rc6(dev))
 		return;
 
 	mutex_lock(&dev->struct_mutex);
-- 
1.7.7.3

Re: [PATCH 2/2] drm/i915: By default, enable RC6 on IVB and SNB when reasonable

[Matthew Garrett]

On Fri, Dec 09, 2011 at 03:53:49PM -0800, Keith Packard wrote:
> RC6 should always work on IVB, and should work on SNB whenever IO
> remapping is disabled. RC6 never works on Ironlake. Make the default
> value for the parameter follow these guidelines. Setting the value
> to either 0 or 1 will force the specified behavior.

I still don't like this. We're in a situation where we clearly have to 
disable one feature or the other on SNB - but we're disabling the one 
that's going to be useful to a large number of people, and leaving the 
niche feature enabled. If we're going to merge this then let's turn off 
iommu on SNB by default.

-- 
Matthew Garrett | mjg59@srcf.ucam.org