Another pass on Workarounds, with a focus on HSW

Another pass on Workarounds, with a focus on HSW

[Damien Lespiau]

Hi,

Patches 1-7: Some comments to signal the Wa we're implementing for the next
person that goes through the list.

Patch 8: Random detail when reading that code

Patch 9-10: Two new workarounds. I don't remember at all the discussions around
RC6+ on GEN6+, but there's definitely a workaround that tells us to disable
RC6+.

-- 
Damien

[PATCH 01/10] drm/i915: We implement WaFbcWaitForVBlankBeforeEnable for ilk and snb

[Damien Lespiau]

We also wait for that blank on other platforms but the w/a doesn't
apply there. Not an issue at all.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 50fe3d7..57e99b1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -404,6 +404,8 @@ void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	 * following the termination of the page-flipping sequence
 	 * and indeed performing the enable as a co-routine and not
 	 * waiting synchronously upon the vblank.
+	 *
+	 * WaFbcWaitForVBlankBeforeEnable:ilk,snb
 	 */
 	schedule_delayed_work(&work->work, msecs_to_jiffies(50));
 }
-- 
1.8.1.4

[PATCH 02/10] drm/i915: We implement WaFbcAsynchFlipDisableFbcQueue on ilk and snb

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 57e99b1..eb3c2c4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4420,6 +4420,8 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
 	 * The bit 22 of 0x42000
 	 * The bit 22 of 0x42004
 	 * The bit 7,8,9 of 0x42020.
+	 *
+	 * WaFbcAsynchFlipDisableFbcQueue:ilk
 	 */
 	if (IS_IRONLAKE_M(dev)) {
 		I915_WRITE(ILK_DISPLAY_CHICKEN1,
@@ -4557,6 +4559,8 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 	 * The bit5 and bit7 of 0x42020
 	 * The bit14 of 0x70180
 	 * The bit14 of 0x71180
+	 *
+	 * WaFbcAsynchFlipDisableFbcQueue:snb
 	 */
 	I915_WRITE(ILK_DISPLAY_CHICKEN1,
 		   I915_READ(ILK_DISPLAY_CHICKEN1) |
-- 
1.8.1.4

[PATCH 03/10] drm/i915: We implement WaFbcDisableDpfcClockGating on ilk

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index eb3c2c4..2eb1846 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4385,7 +4385,10 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE;
 
-	/* Required for FBC */
+	/*
+	 * Required for FBC
+	 * WaFbcDisableDpfcClockGating:ilk
+	 */
 	dspclk_gate |= ILK_DPFCRUNIT_CLOCK_GATE_DISABLE |
 		   ILK_DPFCUNIT_CLOCK_GATE_DISABLE |
 		   ILK_DPFDUNIT_CLOCK_GATE_ENABLE;
-- 
1.8.1.4

[PATCH 04/10] drm/i915: We implement WaMPhyProgramming on Haswell

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 827d7ca..9e0d69c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5122,7 +5122,10 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
 	BUG_ON(val != final);
 }
 
-/* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */
+/*
+ * Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O.
+ * WaMPhyProgramming:hsw
+ */
 static void lpt_init_pch_refclk(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-- 
1.8.1.4

[PATCH 05/10] drm/i915: We implement the TLB invalidation mode workaounds

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 0e72da6..ce14594 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -523,11 +523,14 @@ static int init_render_ring(struct intel_ring_buffer *ring)
 	if (INTEL_INFO(dev)->gen >= 6)
 		I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(ASYNC_FLIP_PERF_DISABLE));
 
-	/* Required for the hardware to program scanline values for waiting */
+	/* Required for the hardware to program scanline values for waiting
+	 * WaEnableFlushTlbInvalidationMode:snb
+	 */
 	if (INTEL_INFO(dev)->gen == 6)
 		I915_WRITE(GFX_MODE,
 			   _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_ALWAYS));
 
+	/* WaBCSVCSTlbInvalidationMode:ivb,hsw,vlv */
 	if (IS_GEN7(dev))
 		I915_WRITE(GFX_MODE_GEN7,
 			   _MASKED_BIT_DISABLE(GFX_TLB_INVALIDATE_ALWAYS) |
-- 
1.8.1.4

[PATCH 06/10] drm/i915: We implement WaEDPModeSetSequenceChange

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 486c46b..bc7dd9a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1050,6 +1050,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
 		intel_dp_start_link_train(intel_dp);
 		intel_dp_complete_link_train(intel_dp);
+		/* WaEDPModeSetSequenceChange:hsw */
 		if (port != PORT_A)
 			intel_dp_stop_link_train(intel_dp);
 	}
@@ -1113,6 +1114,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
 	} else if (type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
+		/* WaEDPModeSetSequenceChange:hsw */
 		if (port == PORT_A)
 			intel_dp_stop_link_train(intel_dp);
 
-- 
1.8.1.4

[PATCH 07/10] drm/i915: We implement WaFlushOpOnCSStall

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ce14594..009e616 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -171,6 +171,8 @@ gen4_render_ring_flush(struct intel_ring_buffer *ring,
  * Post-sync nonzero is what triggered this second workaround, so we
  * can't use that one either.  Notify enable is IRQs, which aren't
  * really our business.  That leaves only stall at scoreboard.
+ *
+ * WaFlushOpOnCSStall:ivb
  */
 static int
 intel_emit_post_sync_nonzero_flush(struct intel_ring_buffer *ring)
@@ -296,6 +298,7 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
 	 * Workaround: 4th PIPE_CONTROL command (except the ones with only
 	 * read-cache invalidate bits set) must have the CS_STALL bit set. We
 	 * don't try to be clever and just set it unconditionally.
+	 * WaFlushOpOnCSStall:ivb,hsw,vlv
 	 */
 	flags |= PIPE_CONTROL_CS_STALL;
 
-- 
1.8.1.4

[PATCH 08/10] drm/i915: Don't try to calculate RC6 residency on GEN4 and before

[Damien Lespiau]

intel_enable_rc6() is used to check if we can compute the RC6 residency
in the sysfs code. Disable this for platforms older than Ironlake.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2eb1846..0465cd6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3154,6 +3154,10 @@ static void valleyview_disable_rps(struct drm_device *dev)
 
 int intel_enable_rc6(const struct drm_device *dev)
 {
+	/* No RC6 before Ironlake */
+	if (INTEL_INFO(dev)->gen < 5)
+		return 0;
+
 	/* Respect the kernel parameter if it is set */
 	if (i915_enable_rc6 >= 0)
 		return i915_enable_rc6;
-- 
1.8.1.4

[PATCH 09/10] drm/i915: Implement WaRsDisableRC6Plus

[Damien Lespiau]

Let's disable RC6+ by default. We still leave the possibility to
override this with the module parameter.

I've also shuffled the code around so we always log if we have RC6
enabled or disabled.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0465cd6..e19952f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3158,27 +3158,20 @@ int intel_enable_rc6(const struct drm_device *dev)
 	if (INTEL_INFO(dev)->gen < 5)
 		return 0;
 
-	/* 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)
+	if (i915_enable_rc6 == 0 || INTEL_INFO(dev)->gen == 5) {
+		DRM_DEBUG_DRIVER("RC6 disabled\n");
 		return 0;
-
-	if (IS_HASWELL(dev)) {
-		DRM_DEBUG_DRIVER("Haswell: only RC6 available\n");
-		return INTEL_RC6_ENABLE;
 	}
 
-	/* snb/ivb have more than one rc6 state. */
-	if (INTEL_INFO(dev)->gen == 6) {
-		DRM_DEBUG_DRIVER("Sandybridge: deep RC6 disabled\n");
-		return INTEL_RC6_ENABLE;
-	}
+	DRM_DEBUG_DRIVER("RC6 enabled\n");
+
+	if (i915_enable_rc6 > 0)
+		return i915_enable_rc6;
 
-	DRM_DEBUG_DRIVER("RC6 and deep RC6 enabled\n");
-	return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
+	/* snb/ivb have more than one rc6 state. However we only allow RC6
+	 * WaRsDisableRC6Plus:snb,ivb,vlv */
+	return INTEL_RC6_ENABLE;
 }
 
 static void gen6_enable_rps(struct drm_device *dev)
-- 
1.8.1.4

[PATCH 10/10] drm/i915: Implement WaBlockMsgChannelDuringGfxReset

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 9 +++++++++
 drivers/gpu/drm/i915/i915_reg.h | 9 +++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 58048d4..187a9a4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4063,6 +4063,15 @@ i915_gem_init_hw(struct drm_device *dev)
 		I915_WRITE(GEN7_MSG_CTL, temp);
 	}
 
+	/* WaBlockMsgChannelDuringGfxReset:hsw */
+	if (IS_HASWELL(dev)) {
+		u32 temp = I915_READ(GEN7_MISCCPCTL);
+		temp &= MISCCPCTL_BLOCK_MC_GFX_FULL_SR |
+			MISCCPCTL_BLOCK_MC_RENDER_SR |
+			MISCCPCTL_BLOCK_MC_FLR;
+		I915_WRITE(GEN7_MISCCPCTL, temp);
+	}
+
 	i915_gem_l3_remap(dev);
 
 	i915_gem_init_swizzling(dev);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 47a9de0..ff27c73 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4595,8 +4595,13 @@
 #define   GEN6_RC6			3
 #define   GEN6_RC7			4
 
-#define GEN7_MISCCPCTL			(0x9424)
-#define   GEN7_DOP_CLOCK_GATE_ENABLE	(1<<0)
+#define GEN7_MISCCPCTL				(0x9424)
+#define   MISCCPCTL_BLOCK_MC_GFX_FULL_SR	(1<<6)
+#define   MISCCPCTL_BLOCK_MC_BLITTER_SR		(1<<5)
+#define   MISCCPCTL_BLOCK_MC_MEDIA_SR		(1<<4)
+#define   MISCCPCTL_BLOCK_MC_RENDER_SR		(1<<3)
+#define   MISCCPCTL_BLOCK_MC_FLR		(1<<2)
+#define   GEN7_DOP_CLOCK_GATE_ENABLE		(1<<0)
 
 /* IVYBRIDGE DPF */
 #define GEN7_L3CDERRST1			0xB008 /* L3CD Error Status 1 */
-- 
1.8.1.4

Re: [PATCH 10/10] drm/i915: Implement WaBlockMsgChannelDuringGfxReset

[Chris Wilson]

On Fri, Jun 07, 2013 at 05:41:16PM +0100, Damien Lespiau wrote:

During GFX reset? This patch looks a little permanent to me, so please
explain.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 10/10] drm/i915: Implement WaBlockMsgChannelDuringGfxReset

[Damien Lespiau]

On Fri, Jun 07, 2013 at 05:51:57PM +0100, Chris Wilson wrote:
> On Fri, Jun 07, 2013 at 05:41:16PM +0100, Damien Lespiau wrote:
> 
> During GFX reset? This patch looks a little permanent to me, so please
> explain.

The description of the bits is "Enable msg channel blockreq/ack
mechanism for [GFX full SR|FLR|Render SR]", so I'm assuming that it's
only needed to program it once and it's taken into account when doing
the reset.

I could also test it I guess...

-- 
Damien

Re: [PATCH 10/10] drm/i915: Implement WaBlockMsgChannelDuringGfxReset

[Damien Lespiau]

On Fri, Jun 07, 2013 at 05:59:45PM +0100, Damien Lespiau wrote:
> I could also test it I guess...

Well, it does seem to behave. The confusion may be from the WA name, the
field is called "Block Msg channel during resets". This is still mostly
guess work though.

-- 
Damien

Re: [PATCH 01/10] drm/i915: We implement WaFbcWaitForVBlankBeforeEnable for ilk and snb

[Paulo Zanoni]

2013/6/7 Damien Lespiau <damien.lespiau@intel.com>:
> We also wait for that blank on other platforms but the w/a doesn't
> apply there. Not an issue at all.
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 50fe3d7..57e99b1 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -404,6 +404,8 @@ void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>          * following the termination of the page-flipping sequence
>          * and indeed performing the enable as a co-routine and not
>          * waiting synchronously upon the vblank.
> +        *
> +        * WaFbcWaitForVBlankBeforeEnable:ilk,snb
>          */
>         schedule_delayed_work(&work->work, msecs_to_jiffies(50));
>  }
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 02/10] drm/i915: We implement WaFbcAsynchFlipDisableFbcQueue on ilk and snb

[Paulo Zanoni]

2013/6/7 Damien Lespiau <damien.lespiau@intel.com>:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 57e99b1..eb3c2c4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4420,6 +4420,8 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
>          * The bit 22 of 0x42000
>          * The bit 22 of 0x42004
>          * The bit 7,8,9 of 0x42020.
> +        *
> +        * WaFbcAsynchFlipDisableFbcQueue:ilk
>          */
>         if (IS_IRONLAKE_M(dev)) {
>                 I915_WRITE(ILK_DISPLAY_CHICKEN1,

Inside the IS_IRONLAKE_M check we set 2 chicken bits. Does the WA
apply to both? It seem it only applies to ILK_DISPLAY_CHICKEN1, so my
suggestion would be to move the comment down to inside the "if"
statement, just above the ILK_DISPLAY_CHICKEN1 line. With the WA
comment above the "if" statement it seems the WA applies to both
i915_writes. Anyway, Reviewed-by: Paulo Zanoni
<paulo.r.zanoni@intel.com>

> @@ -4557,6 +4559,8 @@ static void gen6_init_clock_gating(struct drm_device *dev)
>          * The bit5 and bit7 of 0x42020
>          * The bit14 of 0x70180
>          * The bit14 of 0x71180
> +        *
> +        * WaFbcAsynchFlipDisableFbcQueue:snb
>          */
>         I915_WRITE(ILK_DISPLAY_CHICKEN1,
>                    I915_READ(ILK_DISPLAY_CHICKEN1) |
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 03/10] drm/i915: We implement WaFbcDisableDpfcClockGating on ilk

[Paulo Zanoni]

2013/6/7 Damien Lespiau <damien.lespiau@intel.com>:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

If you search for the WaFbcDisableDpfcClockGating string on that file,
you'll see that this workaround is implemented differently on IVB/HSW:
on ILK the bits are turned on at init_clock_gating and stay like that.
On IVB/HSW we enable the bits at gen7_enable_fbc and disable them at
ironlake_disable_fbc. Can't se convert these ILK/SNB FBC workarounds
to be implemented like IVB/HSW? Of course, this would be a separate
patch. And we would have to re-check all the workarounds that only
need to be enabled while FBC is enabled. Volunteers?

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index eb3c2c4..2eb1846 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4385,7 +4385,10 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE;
>
> -       /* Required for FBC */
> +       /*
> +        * Required for FBC
> +        * WaFbcDisableDpfcClockGating:ilk
> +        */
>         dspclk_gate |= ILK_DPFCRUNIT_CLOCK_GATE_DISABLE |
>                    ILK_DPFCUNIT_CLOCK_GATE_DISABLE |
>                    ILK_DPFDUNIT_CLOCK_GATE_ENABLE;
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 04/10] drm/i915: We implement WaMPhyProgramming on Haswell

[Paulo Zanoni]

2013/6/7 Damien Lespiau <damien.lespiau@intel.com>:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 827d7ca..9e0d69c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5122,7 +5122,10 @@ static void ironlake_init_pch_refclk(struct drm_device *dev)
>         BUG_ON(val != final);
>  }
>
> -/* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */
> +/*
> + * Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O.
> + * WaMPhyProgramming:hsw
> + */
>  static void lpt_init_pch_refclk(struct drm_device *dev)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 09/10] drm/i915: Implement WaRsDisableRC6Plus

[Paulo Zanoni]

2013/6/7 Damien Lespiau <damien.lespiau@intel.com>:
> Let's disable RC6+ by default. We still leave the possibility to
> override this with the module parameter.
>
> I've also shuffled the code around so we always log if we have RC6
> enabled or disabled.

Why disable RC6+ on IVB? My docs tells me it should only be disabled
on the very first pre-production machines that no one has. On the
other hand, it looks like that for VLV we should disable RC6+.

The "code shuffle" looks like a nice improvement.

>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 25 +++++++++----------------
>  1 file changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0465cd6..e19952f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3158,27 +3158,20 @@ int intel_enable_rc6(const struct drm_device *dev)
>         if (INTEL_INFO(dev)->gen < 5)
>                 return 0;
>
> -       /* 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)
> +       if (i915_enable_rc6 == 0 || INTEL_INFO(dev)->gen == 5) {
> +               DRM_DEBUG_DRIVER("RC6 disabled\n");
>                 return 0;
> -
> -       if (IS_HASWELL(dev)) {
> -               DRM_DEBUG_DRIVER("Haswell: only RC6 available\n");
> -               return INTEL_RC6_ENABLE;
>         }
>
> -       /* snb/ivb have more than one rc6 state. */
> -       if (INTEL_INFO(dev)->gen == 6) {
> -               DRM_DEBUG_DRIVER("Sandybridge: deep RC6 disabled\n");
> -               return INTEL_RC6_ENABLE;
> -       }
> +       DRM_DEBUG_DRIVER("RC6 enabled\n");
> +
> +       if (i915_enable_rc6 > 0)
> +               return i915_enable_rc6;
>
> -       DRM_DEBUG_DRIVER("RC6 and deep RC6 enabled\n");
> -       return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE);
> +       /* snb/ivb have more than one rc6 state. However we only allow RC6
> +        * WaRsDisableRC6Plus:snb,ivb,vlv */
> +       return INTEL_RC6_ENABLE;
>  }
>
>  static void gen6_enable_rps(struct drm_device *dev)
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 08/10] drm/i915: Don't try to calculate RC6 residency on GEN4 and before

[Paulo Zanoni]

2013/6/7 Damien Lespiau <damien.lespiau@intel.com>:
> intel_enable_rc6() is used to check if we can compute the RC6 residency
> in the sysfs code. Disable this for platforms older than Ironlake.
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2eb1846..0465cd6 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3154,6 +3154,10 @@ static void valleyview_disable_rps(struct drm_device *dev)
>
>  int intel_enable_rc6(const struct drm_device *dev)
>  {
> +       /* No RC6 before Ironlake */
> +       if (INTEL_INFO(dev)->gen < 5)
> +               return 0;

Daniel recently complained about a similar check on one of my patches.
Looks like the preferred way would be "if (INTEL_INFO(dev)->gen <=
4)". I guess Daniel can do this change while applying the patch.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> +
>         /* Respect the kernel parameter if it is set */
>         if (i915_enable_rc6 >= 0)
>                 return i915_enable_rc6;
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 08/10] drm/i915: Don't try to calculate RC6 residency on GEN4 and before

[Daniel Vetter]

On Tue, Jun 11, 2013 at 04:33:24PM -0300, Paulo Zanoni wrote:
> 2013/6/7 Damien Lespiau <damien.lespiau@intel.com>:
> > intel_enable_rc6() is used to check if we can compute the RC6 residency
> > in the sysfs code. Disable this for platforms older than Ironlake.
> >
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 2eb1846..0465cd6 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3154,6 +3154,10 @@ static void valleyview_disable_rps(struct drm_device *dev)
> >
> >  int intel_enable_rc6(const struct drm_device *dev)
> >  {
> > +       /* No RC6 before Ironlake */
> > +       if (INTEL_INFO(dev)->gen < 5)
> > +               return 0;
> 
> Daniel recently complained about a similar check on one of my patches.
> Looks like the preferred way would be "if (INTEL_INFO(dev)->gen <=
> 4)". I guess Daniel can do this change while applying the patch.

Actually that way round is imo ok, since we tend to say pre-gen5 and mean
that it excludes gen5. It's kinda like iterating over 0-based C arrays
where you check i >= start && i < end ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 2/10 v2] drm/i915: We implement WaFbcAsynchFlipDisableFbcQueue on ilk and snb

[Damien Lespiau]

v2: Put the comment a bit closer to the actual write (Paulo Zanoni)

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 57e99b1..a3c4e06 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4422,6 +4422,7 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
 	 * The bit 7,8,9 of 0x42020.
 	 */
 	if (IS_IRONLAKE_M(dev)) {
+	 	/* WaFbcAsynchFlipDisableFbcQueue:ilk */
 		I915_WRITE(ILK_DISPLAY_CHICKEN1,
 			   I915_READ(ILK_DISPLAY_CHICKEN1) |
 			   ILK_FBCQ_DIS);
@@ -4557,6 +4558,8 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 	 * The bit5 and bit7 of 0x42020
 	 * The bit14 of 0x70180
 	 * The bit14 of 0x71180
+	 *
+	 * WaFbcAsynchFlipDisableFbcQueue:snb
 	 */
 	I915_WRITE(ILK_DISPLAY_CHICKEN1,
 		   I915_READ(ILK_DISPLAY_CHICKEN1) |
-- 
1.8.1.4

Re: [PATCH 08/10] drm/i915: Don't try to calculate RC6 residency on GEN4 and before

[Daniel Vetter]

On Tue, Jun 11, 2013 at 11:03:40PM +0200, Daniel Vetter wrote:
> On Tue, Jun 11, 2013 at 04:33:24PM -0300, Paulo Zanoni wrote:
> > 2013/6/7 Damien Lespiau <damien.lespiau@intel.com>:
> > > intel_enable_rc6() is used to check if we can compute the RC6 residency
> > > in the sysfs code. Disable this for platforms older than Ironlake.
> > >
> > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_pm.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > index 2eb1846..0465cd6 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -3154,6 +3154,10 @@ static void valleyview_disable_rps(struct drm_device *dev)
> > >
> > >  int intel_enable_rc6(const struct drm_device *dev)
> > >  {
> > > +       /* No RC6 before Ironlake */
> > > +       if (INTEL_INFO(dev)->gen < 5)
> > > +               return 0;
> > 
> > Daniel recently complained about a similar check on one of my patches.
> > Looks like the preferred way would be "if (INTEL_INFO(dev)->gen <=
> > 4)". I guess Daniel can do this change while applying the patch.
> 
> Actually that way round is imo ok, since we tend to say pre-gen5 and mean
> that it excludes gen5. It's kinda like iterating over 0-based C arrays
> where you check i >= start && i < end ;-)

Ok, I've merged the patches in this series for which I've found an r-b
tag. Some are still in limbo though.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 08/10] drm/i915: Don't try to calculate RC6 residency on GEN4 and before

[Damien Lespiau]

On Tue, Jul 16, 2013 at 10:34:06AM +0200, Daniel Vetter wrote:
> Ok, I've merged the patches in this series for which I've found an r-b
> tag. Some are still in limbo though.

Thanks for doing that, I still need to answer questions from Paulo and
the last two of them are 'hard' because we don't know the underlying
cause and are mostly guess work, so let me chase that internally.

-- 
Damien