[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: [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]

[-- Attachment #1.1: Type: text/plain, Size: 468 bytes --]

On Sat, Nov 19, 2011 at 04:41, Keith Packard <keithp@keithp.com> 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>
>

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

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 865 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[PATCH] drm/i915: enable semaphores on per-device defaults

[Eugeni Dodonov]

We should enable semaphores on IVB by default, and on SNB in cases where
dma remapping is disabled or iommu is not enabled.

v2: adapt patch according to the feedback, and put it in line with Keith's
rc6 enabling patch.

CC: Daniel Vetter <daniel.vetter@ffwll.ch>
CC: Ben Widawsky <ben@bwidawsk.net>
CC: Keith Packard <keithp@keithp.com>
CC: Jesse Barnes <jbarnes@virtuousgeek.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42696
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40564
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41353
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38862
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c            |    4 ++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   24 +++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index cc531bb..83b2be4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -58,10 +58,10 @@ module_param_named(powersave, i915_powersave, int, 0600);
 MODULE_PARM_DESC(powersave,
 		"Enable powersavings, fbc, downclocking, etc. (default: true)");
 
-unsigned int i915_semaphores __read_mostly = 0;
+unsigned int i915_semaphores __read_mostly = -1;
 module_param_named(semaphores, i915_semaphores, int, 0600);
 MODULE_PARM_DESC(semaphores,
-		"Use semaphores for inter-ring sync (default: false)");
+		"Use semaphores for inter-ring sync (default: -1 (use per-chip defaults))");
 
 unsigned int i915_enable_rc6 __read_mostly = 0;
 module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600);
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3693e83..c19a063 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -33,6 +33,11 @@
 #include "i915_trace.h"
 #include "intel_drv.h"
 
+#ifdef CONFIG_INTEL_IOMMU
+#include <linux/dma_remapping.h>
+#include <linux/dmar.h>
+#endif
+
 struct change_domains {
 	uint32_t invalidate_domains;
 	uint32_t flush_domains;
@@ -746,6 +751,23 @@ i915_gem_execbuffer_flush(struct drm_device *dev,
 	return 0;
 }
 
+static bool
+intel_enable_semaphores(struct drm_device *dev)
+{
+	if (i915_semaphores >= 0)
+		return i915_semaphores;
+	if (INTEL_INFO(dev)->gen >= 7)
+		return 1;
+#ifdef CONFIG_INTEL_IOMMU
+	/* On gen6, we only enable semaphores if dma remapping is disabled,
+	 * or if there is no iommu.
+	 */
+	if (INTEL_INFO(dev)->gen == 6)
+		return no_iommu || dmar_disabled;
+#endif
+	return 1;
+}
+
 static int
 i915_gem_execbuffer_sync_rings(struct drm_i915_gem_object *obj,
 			       struct intel_ring_buffer *to)
@@ -758,7 +780,7 @@ i915_gem_execbuffer_sync_rings(struct drm_i915_gem_object *obj,
 		return 0;
 
 	/* XXX gpu semaphores are implicated in various hard hangs on SNB */
-	if (INTEL_INFO(obj->base.dev)->gen < 6 || !i915_semaphores)
+	if (INTEL_INFO(obj->base.dev)->gen < 6 || !intel_enable_semaphores(obj->base.dev))
 		return i915_gem_object_wait_rendering(obj);
 
 	idx = intel_ring_sync_index(from, to);
-- 
1.7.7.4

Re: [PATCH] drm/i915: enable semaphores on per-device defaults

[Chris Wilson]

On Mon, 21 Nov 2011 11:03:49 -0200, Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:
> We should enable semaphores on IVB by default, and on SNB in cases where
> dma remapping is disabled or iommu is not enabled.
> 
> v2: adapt patch according to the feedback, and put it in line with Keith's
> rc6 enabling patch.
> ---
>  static int
>  i915_gem_execbuffer_sync_rings(struct drm_i915_gem_object *obj,
>  			       struct intel_ring_buffer *to)
> @@ -758,7 +780,7 @@ i915_gem_execbuffer_sync_rings(struct drm_i915_gem_object *obj,
>  		return 0;
>  
>  	/* XXX gpu semaphores are implicated in various hard hangs on SNB */
> -	if (INTEL_INFO(obj->base.dev)->gen < 6 || !i915_semaphores)
> +	if (INTEL_INFO(obj->base.dev)->gen < 6 || !intel_enable_semaphores(obj->base.dev))

Just merge the generation check into intel_enable_semaphores().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: enable semaphores on per-device defaults

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 379 bytes --]

On Mon, 21 Nov 2011 11:03:49 -0200, Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> -unsigned int i915_semaphores __read_mostly = 0;
> +unsigned int i915_semaphores __read_mostly = -1;

That better be signed or it's not going to compare well with -1

Otherwise, this looks good to me.

Reviewed-by: Keith Packard <keithp@keithp.com>

-- 
keith.packard@intel.com

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

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Eugeni Dodonov]

[-- Attachment #1.1: Type: text/plain, Size: 784 bytes --]

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 :(.

When the root cause will be discovered, we should allow both of course. But
so far, all the attempts on this weren't successful.

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 1162 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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.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 #1.2: Type: application/pgp-signature, Size: 827 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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 --]

[PATCH 0/8] misc fixes for 3.2

[Eugeni Dodonov]

Hi,

here comes my queue of patches for next 3.2 push.

patch1 removes CxSR on ILK, following the specifications
patch2 improved edid timings by reducing number of unnecessary retries
patch3 prevents a division-by-zero in kernel when querying for gpu power on ILK
patch4,5 fix typos in comments
patch6 adds the option to enable semaphores according to per-device default.
patch7 fixes an issue which was preventing default fbc value from working
patch8 adds the option to enable rc6 according to per-device default values.

Patch 6 and 8 are based on previously sent patches for semaphores and rc6
enablement. According to the discussion about their enablement on SNB according
to the status of VTd, it was decided to postpone such enablement until we
figure out what is going wrong with the hardware. But as no such issues exist
on IVB, and as those patches bring lots of benefits to the system, there is no
reason to not have them on by default there.

Eugeni Dodonov (8):
  drm/i915: there is no pipe CxSR on ironlake
  drm: give up on edid retries when i2c bus is not responding
  drm/i915: prevent division by zero when asking for chipset power
  drm/i915: fix typo in function name
  platform/x86: fix typos in function comments
  drm/i915: enable semaphores on per-device defaults
  drm/i915: allow per-device fbc to work with default settings
  drm/i915: enable rc6 by default on IVB onwards

 drivers/gpu/drm/drm_edid.c                 |    5 +++++
 drivers/gpu/drm/i915/i915_dma.c            |   10 ++++++++++
 drivers/gpu/drm/i915/i915_drv.c            |   14 +++++++-------
 drivers/gpu/drm/i915/i915_drv.h            |    7 ++++---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   18 +++++++++++++++++-
 drivers/gpu/drm/i915/intel_display.c       |   13 +++++++++++--
 drivers/platform/x86/intel_ips.c           |    6 +++---
 7 files changed, 57 insertions(+), 16 deletions(-)

-- 
1.7.7.4

[PATCH 1/8] drm/i915: there is no pipe CxSR on ironlake

[Eugeni Dodonov]

After checking the specs and discussing with Jesse, turns out CxSR is not
available on Ironlake and gen5, and its advertisement on the device
description is misleading.

Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e9c2cfe..ea0aa20 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -198,7 +198,7 @@ static const struct intel_device_info intel_pineview_info = {
 
 static const struct intel_device_info intel_ironlake_d_info = {
 	.gen = 5,
-	.need_gfx_hws = 1, .has_pipe_cxsr = 1, .has_hotplug = 1,
+	.need_gfx_hws = 1, .has_hotplug = 1,
 	.has_bsd_ring = 1,
 };
 
-- 
1.7.7.4

[PATCH 2/8] drm: give up on edid retries when i2c bus is not responding

[Eugeni Dodonov]

This allows to avoid talking to a non-responding bus repeatedly until we
finally timeout after 15 attempts. We can do this by catching the -ENXIO
error, provided by i2c_algo_bit:bit_doAddress call.

Within the bit_doAddress we already try 3 times to get the edid data, so
if the routine tells us that bus is not responding, it is mostly pointless
to keep re-trying those attempts over and over again until we reach final
number of retries.

This change should fix https://bugs.freedesktop.org/show_bug.cgi?id=41059
and improve overall edid detection timing by 10-30% in most cases, and by
a much larger margin in case of phantom outputs (up to 30x in one worst
case).

Timing results for i915-powered machines for 'time xrandr' command:
Machine 1: from 0.840s to 0.290s
Machine 2: from 0.315s to 0.280s
Machine 3: from +/- 4s to 0.184s

Timing results for HD5770 with 'time xrandr' command:
Machine 4: from 3.210s to 1.060s

Reviewed-by: Chris Wilson <chris@hchris-wilson.co.uk>
Reviewed-by: Keith Packard <keithp@keithp.com>
Tested-by: Sean Finney <seanius@seanius.net>
Tested-by: Soren Hansen <soren@linux2go.dk>
Tested-by: Hernando Torque <sirius@sonnenkinder.org>
Tested-by: Mike Lothian <mike@fireburn.co.uk>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41059
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/drm_edid.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3e927ce..fb6c26c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -266,6 +266,11 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf,
 			}
 		};
 		ret = i2c_transfer(adapter, msgs, 2);
+		if (ret == -ENXIO) {
+			DRM_DEBUG_KMS("drm: skipping non-existent adapter %s\n",
+					adapter->name);
+			break;
+		}
 	} while (ret != 2 && --retries);
 
 	return ret == 2 ? 0 : -1;
-- 
1.7.7.4

[PATCH 3/8] drm/i915: prevent division by zero when asking for chipset power

[Eugeni Dodonov]

This prevents an in-kernel division by zero which happens when we are
asking for i915_chipset_val too quickly, or within a race condition
between the power monitoring thread and userspace accesses via debugfs.

The issue can be reproduced easily via the following command:
while ``; do cat /sys/kernel/debug/dri/0/i915_emon_status; done

This is particularly dangerous because it can be triggered by
a non-privileged user by just reading the debugfs entry.

This issue was also found independently by Konstantin Belousov
<kostikbel@gmail.com>, who proposed a similar patch.

Reported-by: Konstantin Belousov <kostikbel@gmail.com>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Acked-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: <stable@vger.kernel.org>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c |   10 ++++++++++
 drivers/gpu/drm/i915/i915_drv.h |    1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a9533c5..a9ae374 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1454,6 +1454,14 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
 
 	diff1 = now - dev_priv->last_time1;
 
+	/* Prevent division-by-zero if we are asking too fast.
+	 * Also, we don't get interesting results if we are polling
+	 * faster than once in 10ms, so just return the saved value
+	 * in such cases.
+	 */
+	if (diff1 <= 10)
+		return dev_priv->chipset_power;
+
 	count1 = I915_READ(DMIEC);
 	count2 = I915_READ(DDREC);
 	count3 = I915_READ(CSIEC);
@@ -1484,6 +1492,8 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
 	dev_priv->last_count1 = total_count;
 	dev_priv->last_time1 = now;
 
+	dev_priv->chipset_power = ret;
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 06a37f4..ced0839 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -707,6 +707,7 @@ typedef struct drm_i915_private {
 
 	u64 last_count1;
 	unsigned long last_time1;
+	unsigned long chipset_power;
 	u64 last_count2;
 	struct timespec last_time2;
 	unsigned long gfx_power;
-- 
1.7.7.4

[PATCH 4/8] drm/i915: fix typo in function name

[Eugeni Dodonov]

Fix function name in comments, a left-over from when i965_reset was
renamed to i915_reset.

Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ea0aa20..cc752dd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -581,7 +581,7 @@ static int gen6_do_reset(struct drm_device *dev, u8 flags)
 }
 
 /**
- * i965_reset - reset chip after a hang
+ * i915_reset - reset chip after a hang
  * @dev: drm device to reset
  * @flags: reset domains
  *
-- 
1.7.7.4

[PATCH 5/8] platform/x86: fix typos in function comments

[Eugeni Dodonov]

Fix small typos in comments.

Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/platform/x86/intel_ips.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel_ips.c b/drivers/platform/x86/intel_ips.c
index 809a3ae..8dec469 100644
--- a/drivers/platform/x86/intel_ips.c
+++ b/drivers/platform/x86/intel_ips.c
@@ -33,7 +33,7 @@
  * performance by allocating more power or thermal budget to the CPU or GPU
  * based on available headroom and activity.
  *
- * The basic algorithm is driven by a 5s moving average of tempurature.  If
+ * The basic algorithm is driven by a 5s moving average of temperature.  If
  * thermal headroom is available, the CPU and/or GPU power clamps may be
  * adjusted upwards.  If we hit the thermal ceiling or a thermal trigger,
  * we scale back the clamp.  Aside from trigger events (when we're critically
@@ -970,9 +970,9 @@ static void monitor_timeout(unsigned long arg)
  * @data: ips driver structure
  *
  * This is the main function for the IPS driver.  It monitors power and
- * tempurature in the MCP and adjusts CPU and GPU power clams accordingly.
+ * temperature in the MCP and adjusts CPU and GPU power clams accordingly.
  *
- * We keep a 5s moving average of power consumption and tempurature.  Using
+ * We keep a 5s moving average of power consumption and temperature.  Using
  * that data, along with CPU vs GPU preference, we adjust the power clamps
  * up or down.
  */
-- 
1.7.7.4

[PATCH 6/8] drm/i915: enable semaphores on per-device defaults

[Eugeni Dodonov]

This adds a default setting for semaphores parameter, and enables
semaphores by default on IVB.

For now, as semaphores interaction with VTd causes random issues on SNB,
we do not enable them by default. But they can still be enabled via the
semaphores=1 kernel parameter.

CC: Daniel Vetter <daniel.vetter@ffwll.ch>
CC: Ben Widawsky <ben@bwidawsk.net>
CC: Keith Packard <keithp@keithp.com>
CC: Jesse Barnes <jbarnes@virtuousgeek.org>
CC: Chris Wilson <chris@chris-wilson.co.uk>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42696
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40564
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41353
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38862
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c            |    4 ++--
 drivers/gpu/drm/i915/i915_drv.h            |    2 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   18 +++++++++++++++++-
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index cc752dd..06635eb 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -58,10 +58,10 @@ module_param_named(powersave, i915_powersave, int, 0600);
 MODULE_PARM_DESC(powersave,
 		"Enable powersavings, fbc, downclocking, etc. (default: true)");
 
-unsigned int i915_semaphores __read_mostly = 0;
+int i915_semaphores __read_mostly = -1;
 module_param_named(semaphores, i915_semaphores, int, 0600);
 MODULE_PARM_DESC(semaphores,
-		"Use semaphores for inter-ring sync (default: false)");
+		"Use semaphores for inter-ring sync (default: -1 (use per-chip defaults))");
 
 unsigned int i915_enable_rc6 __read_mostly = 0;
 module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ced0839..82ee4bb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -998,7 +998,7 @@ extern int i915_max_ioctl;
 extern unsigned int i915_fbpercrtc __always_unused;
 extern int i915_panel_ignore_lid __read_mostly;
 extern unsigned int i915_powersave __read_mostly;
-extern unsigned int i915_semaphores __read_mostly;
+extern int i915_semaphores __read_mostly;
 extern unsigned int i915_lvds_downclock __read_mostly;
 extern unsigned int i915_panel_use_ssc __read_mostly;
 extern int i915_vbt_sdvo_panel_type __read_mostly;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3693e83..f037bca 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -746,6 +746,22 @@ i915_gem_execbuffer_flush(struct drm_device *dev,
 	return 0;
 }
 
+static bool
+intel_enable_semaphores(struct drm_device *dev)
+{
+	if (INTEL_INFO(dev)->gen < 6)
+		return 0;
+
+	if (i915_semaphores >= 0)
+		return i915_semaphores;
+
+	/* Enable semaphores by default on IVB onwards */
+	if (INTEL_INFO(dev)->gen >= 7)
+		return 1;
+
+	return 0;
+}
+
 static int
 i915_gem_execbuffer_sync_rings(struct drm_i915_gem_object *obj,
 			       struct intel_ring_buffer *to)
@@ -758,7 +774,7 @@ i915_gem_execbuffer_sync_rings(struct drm_i915_gem_object *obj,
 		return 0;
 
 	/* XXX gpu semaphores are implicated in various hard hangs on SNB */
-	if (INTEL_INFO(obj->base.dev)->gen < 6 || !i915_semaphores)
+	if (!intel_enable_semaphores(obj->base.dev))
 		return i915_gem_object_wait_rendering(obj);
 
 	idx = intel_ring_sync_index(from, to);
-- 
1.7.7.4

[PATCH 7/8] drm/i915: allow per-device fbc to work with default settings

[Eugeni Dodonov]

The change which added support for default settings for fbc has not
changed the variable type of i915_enable_fbc from unsigned int to int. So
we were unable to get into the default ('-1') state with the default
settings.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |    2 +-
 drivers/gpu/drm/i915/i915_drv.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 06635eb..a472851 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -68,7 +68,7 @@ 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)");
 
-unsigned int i915_enable_fbc __read_mostly = -1;
+int i915_enable_fbc __read_mostly = -1;
 module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600);
 MODULE_PARM_DESC(i915_enable_fbc,
 		"Enable frame buffer compression for power savings "
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 82ee4bb..14e8307 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1003,7 +1003,7 @@ extern unsigned int i915_lvds_downclock __read_mostly;
 extern unsigned 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 unsigned int i915_enable_fbc __read_mostly;
+extern int i915_enable_fbc __read_mostly;
 extern bool i915_enable_hangcheck __read_mostly;
 
 extern int i915_suspend(struct drm_device *dev, pm_message_t state);
-- 
1.7.7.4

[PATCH 8/8] drm/i915: enable rc6 by default on IVB onwards

[Eugeni Dodonov]

This is based on original Keith Packard's patch for enabling rc6 by
default, with the only change that we do only enable RC6 by default on IVB
onwards.

Due to the random issues related to RC6 on SNB when VTd is enabled, we'll
not enable them by default on this platform yet. It can always be enabled
by the i915.i915_enable_rc6=1 kernel parameter though.

CC: Daniel Vetter <daniel.vetter@ffwll.ch>
CC: Ben Widawsky <ben@bwidawsk.net>
CC: Keith Packard <keithp@keithp.com>
CC: Jesse Barnes <jbarnes@virtuousgeek.org>
CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |    4 ++--
 drivers/gpu/drm/i915/i915_drv.h      |    2 +-
 drivers/gpu/drm/i915/intel_display.c |   13 +++++++++++--
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a472851..9b16843 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: -1 (use per-chip defaults))");
 
-unsigned int i915_enable_rc6 __read_mostly = 0;
+int i915_enable_rc6 __read_mostly = 0;
 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 defaults))");
 
 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 14e8307..e283a1d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1002,7 +1002,7 @@ extern int i915_semaphores __read_mostly;
 extern unsigned int i915_lvds_downclock __read_mostly;
 extern unsigned 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 981b1f1..8d392b7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7886,6 +7886,15 @@ 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;
+	return 0;
+}
+
 void gen6_enable_rps(struct drm_i915_private *dev_priv)
 {
 	u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
@@ -7922,7 +7931,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;
 
@@ -8357,7 +8366,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.4

Re: [PATCH 6/8] drm/i915: enable semaphores on per-device defaults

[Daniel Vetter]

On Mon, Nov 28, 2011 at 04:15:19PM -0200, Eugeni Dodonov wrote:
> This adds a default setting for semaphores parameter, and enables
> semaphores by default on IVB.
> 
> For now, as semaphores interaction with VTd causes random issues on SNB,
> we do not enable them by default. But they can still be enabled via the
> semaphores=1 kernel parameter.
> 
> CC: Daniel Vetter <daniel.vetter@ffwll.ch>
> CC: Ben Widawsky <ben@bwidawsk.net>
> CC: Keith Packard <keithp@keithp.com>
> CC: Jesse Barnes <jbarnes@virtuousgeek.org>
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42696
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40564
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41353
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38862
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 8/8] drm/i915: enable rc6 by default on IVB onwards

[Daniel Vetter]

On Mon, Nov 28, 2011 at 04:15:21PM -0200, Eugeni Dodonov wrote:
> This is based on original Keith Packard's patch for enabling rc6 by
> default, with the only change that we do only enable RC6 by default on IVB
> onwards.
> 
> Due to the random issues related to RC6 on SNB when VTd is enabled, we'll
> not enable them by default on this platform yet. It can always be enabled
> by the i915.i915_enable_rc6=1 kernel parameter though.
> 
> CC: Daniel Vetter <daniel.vetter@ffwll.ch>
> CC: Ben Widawsky <ben@bwidawsk.net>
> CC: Keith Packard <keithp@keithp.com>
> CC: Jesse Barnes <jbarnes@virtuousgeek.org>
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c      |    4 ++--
>  drivers/gpu/drm/i915/i915_drv.h      |    2 +-
>  drivers/gpu/drm/i915/intel_display.c |   13 +++++++++++--
>  3 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a472851..9b16843 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: -1 (use per-chip defaults))");
>  
> -unsigned int i915_enable_rc6 __read_mostly = 0;
> +int i915_enable_rc6 __read_mostly = 0;

Looks like the -1 default value got lost ...

>  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 defaults))");
>  
>  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 14e8307..e283a1d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1002,7 +1002,7 @@ extern int i915_semaphores __read_mostly;
>  extern unsigned int i915_lvds_downclock __read_mostly;
>  extern unsigned 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 981b1f1..8d392b7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7886,6 +7886,15 @@ 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;
> +	return 0;
> +}
> +
>  void gen6_enable_rps(struct drm_i915_private *dev_priv)
>  {
>  	u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> @@ -7922,7 +7931,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;
>  
> @@ -8357,7 +8366,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.4
> 

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

[PATCH] drm/i915: enable rc6 by default on IVB onwards

[Eugeni Dodonov]

This is based on original Keith Packard's patch for enabling rc6 by
default, with the only change that we do only enable RC6 by default on IVB
onwards.

Due to the random issues related to RC6 on SNB when VTd is enabled, we'll
not enable them by default on this platform yet. It can always be enabled
by the i915.i915_enable_rc6=1 kernel parameter though.

CC: Daniel Vetter <daniel.vetter@ffwll.ch>
CC: Ben Widawsky <ben@bwidawsk.net>
CC: Keith Packard <keithp@keithp.com>
CC: Jesse Barnes <jbarnes@virtuousgeek.org>
CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |    4 ++--
 drivers/gpu/drm/i915/i915_drv.h      |    2 +-
 drivers/gpu/drm/i915/intel_display.c |   13 +++++++++++--
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a472851..e08d72f 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: -1 (use per-chip defaults))");
 
-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 defaults))");
 
 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 14e8307..e283a1d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1002,7 +1002,7 @@ extern int i915_semaphores __read_mostly;
 extern unsigned int i915_lvds_downclock __read_mostly;
 extern unsigned 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 981b1f1..8d392b7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7886,6 +7886,15 @@ 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;
+	return 0;
+}
+
 void gen6_enable_rps(struct drm_i915_private *dev_priv)
 {
 	u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
@@ -7922,7 +7931,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;
 
@@ -8357,7 +8366,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.4

Re: [PATCH] drm/i915: enable rc6 by default on IVB onwards

[Daniel Vetter]

On Mon, Nov 28, 2011 at 10:24:58PM -0200, Eugeni Dodonov wrote:
> This is based on original Keith Packard's patch for enabling rc6 by
> default, with the only change that we do only enable RC6 by default on IVB
> onwards.
> 
> Due to the random issues related to RC6 on SNB when VTd is enabled, we'll
> not enable them by default on this platform yet. It can always be enabled
> by the i915.i915_enable_rc6=1 kernel parameter though.
> 
> CC: Daniel Vetter <daniel.vetter@ffwll.ch>
> CC: Ben Widawsky <ben@bwidawsk.net>
> CC: Keith Packard <keithp@keithp.com>
> CC: Jesse Barnes <jbarnes@virtuousgeek.org>
> CC: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

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 6/8] drm/i915: enable semaphores on per-device defaults

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 405 bytes --]

On Mon, 28 Nov 2011 16:15:19 -0200, Eugeni Dodonov <eugeni.dodonov@intel.com> wrote:

> For now, as semaphores interaction with VTd causes random issues on SNB,
> we do not enable them by default. But they can still be enabled via the
> semaphores=1 kernel parameter.

Could we enable semaphores on SNB when VTd is not running, just like
we're going to do for RC6?

-- 
keith.packard@intel.com

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

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: enable semaphores on per-device defaults

[Eugeni Dodonov]

This adds a default setting for semaphores parameter, and enables
semaphores by default on IVB.

For now, as semaphores interaction with VTd causes random issues on SNB,
we do not enable them by default. But they can still be enabled via the
semaphores=1 kernel parameter.

v2: enables semaphores on SNB when IO remapping is disabled, with base on
Keith Packard patch.

CC: Daniel Vetter <daniel.vetter@ffwll.ch>
CC: Ben Widawsky <ben@bwidawsk.net>
CC: Keith Packard <keithp@keithp.com>
CC: Jesse Barnes <jbarnes@virtuousgeek.org>
CC: Chris Wilson <chris@chris-wilson.co.uk>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42696
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=40564
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41353
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38862
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c            |    4 ++--
 drivers/gpu/drm/i915/i915_drv.h            |    2 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   19 ++++++++++++++++++-
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7e7b4b2..cd0a625 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -58,10 +58,10 @@ module_param_named(powersave, i915_powersave, int, 0600);
 MODULE_PARM_DESC(powersave,
 		"Enable powersavings, fbc, downclocking, etc. (default: true)");
 
-unsigned int i915_semaphores __read_mostly = 0;
+int i915_semaphores __read_mostly = -1;
 module_param_named(semaphores, i915_semaphores, int, 0600);
 MODULE_PARM_DESC(semaphores,
-		"Use semaphores for inter-ring sync (default: false)");
+		"Use semaphores for inter-ring sync (default: -1 (use per-chip defaults))");
 
 int i915_enable_rc6 __read_mostly = -1;
 module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0600);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 42de1ba..939a8bc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -997,7 +997,7 @@ extern int i915_max_ioctl;
 extern unsigned int i915_fbpercrtc __always_unused;
 extern int i915_panel_ignore_lid __read_mostly;
 extern unsigned int i915_powersave __read_mostly;
-extern unsigned int i915_semaphores __read_mostly;
+extern int i915_semaphores __read_mostly;
 extern unsigned int i915_lvds_downclock __read_mostly;
 extern unsigned int i915_panel_use_ssc __read_mostly;
 extern int i915_vbt_sdvo_panel_type __read_mostly;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3693e83..c681dc1 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -32,6 +32,7 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
+#include <linux/dma_remapping.h>
 
 struct change_domains {
 	uint32_t invalidate_domains;
@@ -746,6 +747,22 @@ i915_gem_execbuffer_flush(struct drm_device *dev,
 	return 0;
 }
 
+static bool
+intel_enable_semaphores(struct drm_device *dev)
+{
+	if (INTEL_INFO(dev)->gen < 6)
+		return 0;
+
+	if (i915_semaphores >= 0)
+		return i915_semaphores;
+
+	/* Enable semaphores on SNB when IO remapping is off */
+	if (INTEL_INFO(dev)->gen == 6)
+		return !intel_iommu_enabled;
+
+	return 1;
+}
+
 static int
 i915_gem_execbuffer_sync_rings(struct drm_i915_gem_object *obj,
 			       struct intel_ring_buffer *to)
@@ -758,7 +775,7 @@ i915_gem_execbuffer_sync_rings(struct drm_i915_gem_object *obj,
 		return 0;
 
 	/* XXX gpu semaphores are implicated in various hard hangs on SNB */
-	if (INTEL_INFO(obj->base.dev)->gen < 6 || !i915_semaphores)
+	if (!intel_enable_semaphores(obj->base.dev))
 		return i915_gem_object_wait_rendering(obj);
 
 	idx = intel_ring_sync_index(from, to);
-- 
1.7.7.4

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

Re: [PATCH 4/8] drm/i915: fix typo in function name

[Daniel Vetter]

On Mon, Nov 28, 2011 at 04:15:17PM -0200, Eugeni Dodonov wrote:
> Fix function name in comments, a left-over from when i965_reset was
> renamed to i915_reset.
> 
> Signed-off-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

Queued for -next, thanks for the patch. The drm core i2c is pending an ack
from Dave, everything else should be merged already afaics.
-Daniel
> ---
>  drivers/gpu/drm/i915/i915_drv.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ea0aa20..cc752dd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -581,7 +581,7 @@ static int gen6_do_reset(struct drm_device *dev, u8 flags)
>  }
>  
>  /**
> - * i965_reset - reset chip after a hang
> + * i915_reset - reset chip after a hang
>   * @dev: drm device to reset
>   * @flags: reset domains
>   *
> -- 
> 1.7.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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