[PATCH 00/10] drm/i915: Some less complex FBC fixes

[PATCH 00/10] drm/i915: Some less complex FBC fixes

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Since fixing the FBC locking is a bigger task that will take a while,
I decided to pull all the simple fixes from my branch and post them
right away.

Some of these I've posted before, some others have seen a bit of action
by being in a public branch.

The FBC_FENCE_OFF change is just a guess at this point. The odd offset
just caught my eye while reading throguh i915_reg.h.

Ville Syrjälä (10):
  drm/i915: Don't write IVB_FBC_RT_BASE
  drm/i915: Don't set persistent FBC mode on ILK/SNB
  drm/i915: Don't set DPFC_HT_MODIFY bit on CTG/ILK/SNB
  drm/i915: Improve FBC plane defines a bit
  drm/i915: Use 1/2 compression ratio limit for 16bpp on FBC2
  drm/i915: Actually write the correct bits to DPFC_CONTROL on CTG
  drm/i915: Don't preserve DPFC_CONTROL bits ILK/SNB
  drm/i915: Kill most of the FBC register save/restore
  drm/i915: Fix FBC1 enable message
  drm/i915: Fix FBC_FENCE_OFF

 drivers/gpu/drm/i915/i915_drv.h     |  4 ----
 drivers/gpu/drm/i915/i915_reg.h     | 10 ++++-----
 drivers/gpu/drm/i915/i915_suspend.c | 32 +++++++----------------------
 drivers/gpu/drm/i915/intel_pm.c     | 41 ++++++++++++++++++++-----------------
 4 files changed, 33 insertions(+), 54 deletions(-)

-- 
1.8.3.2

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

[PATCH v3 01/10] drm/i915: Don't write IVB_FBC_RT_BASE

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We use nuking instead of render tracking on IVB+, so there's
no point in writing IVB_FBC_RT_BASE.

v2: Drop the IVB_FBC_RT_BASE write too
v3: Move the SNB stuff elsewhere, leaving only IVB+ here

Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b9b4fe4..55874d2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -283,8 +283,6 @@ static void gen7_enable_fbc(struct drm_crtc *crtc)
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	I915_WRITE(IVB_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj));
-
 	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
 		   IVB_DPFC_CTL_FENCE_EN |
 		   intel_crtc->plane << IVB_DPFC_CTL_PLANE_SHIFT);
-- 
1.8.3.2

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

[PATCH 02/10] drm/i915: Don't set persistent FBC mode on ILK/SNB

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The ILK/SNB docs are a bit unclear what the persistent mode does, but
the CTG docs clearly state that it was meant to be used when we're
tracking back buffer modifications. We never do that, so leave it in
non-persistent mode.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 55874d2..87ecf45 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -230,8 +230,6 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc)
 	dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
 	dpfc_ctl &= DPFC_RESERVED;
 	dpfc_ctl |= (plane | DPFC_CTL_LIMIT_1X);
-	/* Set persistent mode for front-buffer rendering, ala X. */
-	dpfc_ctl |= DPFC_CTL_PERSISTENT_MODE;
 	dpfc_ctl |= DPFC_CTL_FENCE_EN;
 	if (IS_GEN5(dev))
 		dpfc_ctl |= obj->fence_reg;
-- 
1.8.3.2

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

[PATCH 03/10] drm/i915: Don't set DPFC_HT_MODIFY bit on CTG/ILK/SNB

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The ILK/SNB docs don't really mention the the DPFC_HT_MODIFY bit.
CTG docs clearly state that it should be set only when tracking
back buffer modification in persistent mode. The bit is supposed
to be set by software after the first CPU modification to the
back buffer, and it would get automagically cleared by the hardware
on the next page flip.

Since we only track front buffer modification we don't need to set
this bit. GTT modification tracking still appears to work on ILK
and SNB with the bit unset. I don't have a CTG to verify how that
behaves.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 87ecf45..5fc1cc9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -159,7 +159,6 @@ static void g4x_enable_fbc(struct drm_crtc *crtc)
 
 	dpfc_ctl = plane | DPFC_SR_EN | DPFC_CTL_LIMIT_1X;
 	dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
-	I915_WRITE(DPFC_CHICKEN, DPFC_HT_MODIFY);
 
 	I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
 
@@ -233,7 +232,6 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc)
 	dpfc_ctl |= DPFC_CTL_FENCE_EN;
 	if (IS_GEN5(dev))
 		dpfc_ctl |= obj->fence_reg;
-	I915_WRITE(ILK_DPFC_CHICKEN, DPFC_HT_MODIFY);
 
 	I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->y);
 	I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
-- 
1.8.3.2

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

[PATCH 04/10] drm/i915: Improve FBC plane defines a bit

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make the FBC plane macros take the plane as a parameter.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  8 +++-----
 drivers/gpu/drm/i915/intel_pm.c | 13 +++++--------
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5d06ad6..8d952b2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1047,8 +1047,7 @@
 #define   FBC_CTL_IDLE_LINE	(2<<2)
 #define   FBC_CTL_IDLE_DEBUG	(3<<2)
 #define   FBC_CTL_CPU_FENCE	(1<<1)
-#define   FBC_CTL_PLANEA	(0<<0)
-#define   FBC_CTL_PLANEB	(1<<0)
+#define   FBC_CTL_PLANE(plane)	((plane)<<0)
 #define FBC_FENCE_OFF		0x0321b
 #define FBC_TAG			0x03300
 
@@ -1058,9 +1057,8 @@
 #define DPFC_CB_BASE		0x3200
 #define DPFC_CONTROL		0x3208
 #define   DPFC_CTL_EN		(1<<31)
-#define   DPFC_CTL_PLANEA	(0<<30)
-#define   DPFC_CTL_PLANEB	(1<<30)
-#define   IVB_DPFC_CTL_PLANE_SHIFT	(29)
+#define   DPFC_CTL_PLANE(plane)	((plane)<<30)
+#define   IVB_DPFC_CTL_PLANE(plane)	((plane)<<29)
 #define   DPFC_CTL_FENCE_EN	(1<<29)
 #define   IVB_DPFC_CTL_FENCE_EN	(1<<28)
 #define   DPFC_CTL_PERSISTENT_MODE	(1<<25)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5fc1cc9..c6e047e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -97,7 +97,7 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc)
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int cfb_pitch;
-	int plane, i;
+	int i;
 	u32 fbc_ctl;
 
 	cfb_pitch = dev_priv->fbc.size / FBC_LL_SIZE;
@@ -109,7 +109,6 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc)
 		cfb_pitch = (cfb_pitch / 32) - 1;
 	else
 		cfb_pitch = (cfb_pitch / 64) - 1;
-	plane = intel_crtc->plane == 0 ? FBC_CTL_PLANEA : FBC_CTL_PLANEB;
 
 	/* Clear old tags */
 	for (i = 0; i < (FBC_LL_SIZE / 32) + 1; i++)
@@ -120,7 +119,7 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc)
 
 		/* Set it up... */
 		fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
-		fbc_ctl2 |= plane;
+		fbc_ctl2 |= FBC_CTL_PLANE(intel_crtc->plane);
 		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
 		I915_WRITE(FBC_FENCE_OFF, crtc->y);
 	}
@@ -154,10 +153,9 @@ static void g4x_enable_fbc(struct drm_crtc *crtc)
 	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int plane = intel_crtc->plane == 0 ? DPFC_CTL_PLANEA : DPFC_CTL_PLANEB;
 	u32 dpfc_ctl;
 
-	dpfc_ctl = plane | DPFC_SR_EN | DPFC_CTL_LIMIT_1X;
+	dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_SR_EN | DPFC_CTL_LIMIT_1X;
 	dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
 
 	I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
@@ -223,12 +221,11 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc)
 	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int plane = intel_crtc->plane == 0 ? DPFC_CTL_PLANEA : DPFC_CTL_PLANEB;
 	u32 dpfc_ctl;
 
 	dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
 	dpfc_ctl &= DPFC_RESERVED;
-	dpfc_ctl |= (plane | DPFC_CTL_LIMIT_1X);
+	dpfc_ctl |= DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_CTL_LIMIT_1X;
 	dpfc_ctl |= DPFC_CTL_FENCE_EN;
 	if (IS_GEN5(dev))
 		dpfc_ctl |= obj->fence_reg;
@@ -281,7 +278,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc)
 
 	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
 		   IVB_DPFC_CTL_FENCE_EN |
-		   intel_crtc->plane << IVB_DPFC_CTL_PLANE_SHIFT);
+		   IVB_DPFC_CTL_PLANE(intel_crtc->plane));
 
 	if (IS_IVYBRIDGE(dev)) {
 		/* WaFbcAsynchFlipDisableFbcQueue:ivb */
-- 
1.8.3.2

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

[PATCH 05/10] drm/i915: Use 1/2 compression ratio limit for 16bpp on FBC2

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c6e047e..a7af5b4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -155,7 +155,11 @@ static void g4x_enable_fbc(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	u32 dpfc_ctl;
 
-	dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_SR_EN | DPFC_CTL_LIMIT_1X;
+	dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_SR_EN;
+	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
+		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
+	else
+		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
 	dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
 
 	I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
@@ -225,7 +229,11 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc)
 
 	dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
 	dpfc_ctl &= DPFC_RESERVED;
-	dpfc_ctl |= DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_CTL_LIMIT_1X;
+	dpfc_ctl |= DPFC_CTL_PLANE(intel_crtc->plane);
+	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
+		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
+	else
+		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
 	dpfc_ctl |= DPFC_CTL_FENCE_EN;
 	if (IS_GEN5(dev))
 		dpfc_ctl |= obj->fence_reg;
@@ -275,10 +283,16 @@ static void gen7_enable_fbc(struct drm_crtc *crtc)
 	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
 	struct drm_i915_gem_object *obj = intel_fb->obj;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	u32 dpfc_ctl;
 
-	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
-		   IVB_DPFC_CTL_FENCE_EN |
-		   IVB_DPFC_CTL_PLANE(intel_crtc->plane));
+	dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane);
+	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
+		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
+	else
+		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
+	dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN;
+
+	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
 
 	if (IS_IVYBRIDGE(dev)) {
 		/* WaFbcAsynchFlipDisableFbcQueue:ivb */
-- 
1.8.3.2

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

[PATCH 06/10] drm/i915: Actually write the correct bits to DPFC_CONTROL on CTG

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We set up all the bits for DPFC_CONTROL but forgot to actually
write them to the register. Oops.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a7af5b4..75aceaa 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -165,7 +165,7 @@ static void g4x_enable_fbc(struct drm_crtc *crtc)
 	I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
 
 	/* enable it... */
-	I915_WRITE(DPFC_CONTROL, I915_READ(DPFC_CONTROL) | DPFC_CTL_EN);
+	I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
 
 	DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
 }
-- 
1.8.3.2

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

[PATCH 07/10] drm/i915: Don't preserve DPFC_CONTROL bits ILK/SNB

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

On CTG and IVB+ we don't try to preserve any bits from the
DPFC_CONTROL register. Follow suit on ILK/SNB.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 75aceaa..cd031b6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -227,9 +227,7 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	u32 dpfc_ctl;
 
-	dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
-	dpfc_ctl &= DPFC_RESERVED;
-	dpfc_ctl |= DPFC_CTL_PLANE(intel_crtc->plane);
+	dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane);
 	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
 		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
 	else
-- 
1.8.3.2

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

[PATCH v2 08/10] drm/i915: Kill most of the FBC register save/restore

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We will anyway re-enable FBC normally after resume, so trying to save
and restore the register makes little sense.

We do need to preserve the FBC1 interval bits in FBC_CONTROL since
we only initialize them during driver load, and try to preserve them
after that.

v2: s/I915_HAS_FBC/HAS_FBC/ and fix the check for gen4

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |  4 ----
 drivers/gpu/drm/i915/i915_suspend.c | 32 +++++++-------------------------
 2 files changed, 7 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 56c720b..7b6a083 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -859,11 +859,7 @@ struct i915_suspend_saved_registers {
 	u32 savePFIT_CONTROL;
 	u32 save_palette_a[256];
 	u32 save_palette_b[256];
-	u32 saveDPFC_CB_BASE;
-	u32 saveFBC_CFB_BASE;
-	u32 saveFBC_LL_BASE;
 	u32 saveFBC_CONTROL;
-	u32 saveFBC_CONTROL2;
 	u32 saveIER;
 	u32 saveIIR;
 	u32 saveIMR;
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index e6c90d1..56785e8 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -236,19 +236,9 @@ static void i915_save_display(struct drm_device *dev)
 		dev_priv->regfile.savePP_DIVISOR = I915_READ(PP_DIVISOR);
 	}
 
-	/* Only regfile.save FBC state on the platform that supports FBC */
-	if (HAS_FBC(dev)) {
-		if (HAS_PCH_SPLIT(dev)) {
-			dev_priv->regfile.saveDPFC_CB_BASE = I915_READ(ILK_DPFC_CB_BASE);
-		} else if (IS_GM45(dev)) {
-			dev_priv->regfile.saveDPFC_CB_BASE = I915_READ(DPFC_CB_BASE);
-		} else {
-			dev_priv->regfile.saveFBC_CFB_BASE = I915_READ(FBC_CFB_BASE);
-			dev_priv->regfile.saveFBC_LL_BASE = I915_READ(FBC_LL_BASE);
-			dev_priv->regfile.saveFBC_CONTROL2 = I915_READ(FBC_CONTROL2);
-			dev_priv->regfile.saveFBC_CONTROL = I915_READ(FBC_CONTROL);
-		}
-	}
+	/* save FBC interval */
+	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev))
+		dev_priv->regfile.saveFBC_CONTROL = I915_READ(FBC_CONTROL);
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		i915_save_vga(dev);
@@ -300,18 +290,10 @@ static void i915_restore_display(struct drm_device *dev)
 
 	/* only restore FBC info on the platform that supports FBC*/
 	intel_disable_fbc(dev);
-	if (HAS_FBC(dev)) {
-		if (HAS_PCH_SPLIT(dev)) {
-			I915_WRITE(ILK_DPFC_CB_BASE, dev_priv->regfile.saveDPFC_CB_BASE);
-		} else if (IS_GM45(dev)) {
-			I915_WRITE(DPFC_CB_BASE, dev_priv->regfile.saveDPFC_CB_BASE);
-		} else {
-			I915_WRITE(FBC_CFB_BASE, dev_priv->regfile.saveFBC_CFB_BASE);
-			I915_WRITE(FBC_LL_BASE, dev_priv->regfile.saveFBC_LL_BASE);
-			I915_WRITE(FBC_CONTROL2, dev_priv->regfile.saveFBC_CONTROL2);
-			I915_WRITE(FBC_CONTROL, dev_priv->regfile.saveFBC_CONTROL);
-		}
-	}
+
+	/* restore FBC interval */
+	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev))
+		I915_WRITE(FBC_CONTROL, dev_priv->regfile.saveFBC_CONTROL);
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		i915_restore_vga(dev);
-- 
1.8.3.2

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

[PATCH 09/10] drm/i915: Fix FBC1 enable message

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The debug message telling FBC1 has been enabled is missing a newline.
Add it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index cd031b6..e6693f4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -134,7 +134,7 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc)
 	fbc_ctl |= obj->fence_reg;
 	I915_WRITE(FBC_CONTROL, fbc_ctl);
 
-	DRM_DEBUG_KMS("enabled FBC, pitch %d, yoff %d, plane %c, ",
+	DRM_DEBUG_KMS("enabled FBC, pitch %d, yoff %d, plane %c\n",
 		      cfb_pitch, crtc->y, plane_name(intel_crtc->plane));
 }
 
-- 
1.8.3.2

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

[PATCH 10/10] drm/i915: Fix FBC_FENCE_OFF

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Having a 4 byte register at 0x321b seems unlikely as that's not
4 byte aligned. Since later platforms have more or less the same FBC
registers with new names, assume that FBC_FENCE_OFF is at 0x3218 just
like DPFC_FENCE_YOFF.

This feels like a simple typo in BSpec. 321Bh looks a lot like 3218h
after all.

Should still be tested on real hardware of course. But I don't have
any mobile gen4 systems.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8d952b2..ade2a8e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1048,7 +1048,7 @@
 #define   FBC_CTL_IDLE_DEBUG	(3<<2)
 #define   FBC_CTL_CPU_FENCE	(1<<1)
 #define   FBC_CTL_PLANE(plane)	((plane)<<0)
-#define FBC_FENCE_OFF		0x0321b
+#define FBC_FENCE_OFF		0x03218 /* BSpec typo has 321Bh */
 #define FBC_TAG			0x03300
 
 #define FBC_LL_SIZE		(1536)
-- 
1.8.3.2

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

Re: [PATCH 00/10] drm/i915: Some less complex FBC fixes

[Chris Wilson]

On Thu, Jan 23, 2014 at 04:49:07PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since fixing the FBC locking is a bigger task that will take a while,
> I decided to pull all the simple fixes from my branch and post them
> right away.
> 
> Some of these I've posted before, some others have seen a bit of action
> by being in a public branch.
> 
> The FBC_FENCE_OFF change is just a guess at this point. The odd offset
> just caught my eye while reading throguh i915_reg.h.

I didn't spot anything offensive in the series and each patch only does
what it says on the tin. So I am going to stick my neck out and say

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

for the series. Being picky, I guess Fix FBC_FENCE_OFF should only be an
acked-by since we have no way to review it...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 05/10] drm/i915: Use 1/2 compression ratio limit for 16bpp on FBC2

[Daniel Vetter]

On Thu, Jan 23, 2014 at 04:49:12PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Hm, running the fbc tests with a 16bpp fb would be neat ...
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c6e047e..a7af5b4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -155,7 +155,11 @@ static void g4x_enable_fbc(struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	u32 dpfc_ctl;
>  
> -	dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_SR_EN | DPFC_CTL_LIMIT_1X;
> +	dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_SR_EN;
> +	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
> +		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
> +	else
> +		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
>  	dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
>  
>  	I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
> @@ -225,7 +229,11 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc)
>  
>  	dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
>  	dpfc_ctl &= DPFC_RESERVED;
> -	dpfc_ctl |= DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_CTL_LIMIT_1X;
> +	dpfc_ctl |= DPFC_CTL_PLANE(intel_crtc->plane);
> +	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
> +		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
> +	else
> +		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
>  	dpfc_ctl |= DPFC_CTL_FENCE_EN;
>  	if (IS_GEN5(dev))
>  		dpfc_ctl |= obj->fence_reg;
> @@ -275,10 +283,16 @@ static void gen7_enable_fbc(struct drm_crtc *crtc)
>  	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
>  	struct drm_i915_gem_object *obj = intel_fb->obj;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	u32 dpfc_ctl;
>  
> -	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
> -		   IVB_DPFC_CTL_FENCE_EN |
> -		   IVB_DPFC_CTL_PLANE(intel_crtc->plane));
> +	dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane);
> +	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
> +		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
> +	else
> +		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
> +	dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN;
> +
> +	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
>  
>  	if (IS_IVYBRIDGE(dev)) {
>  		/* WaFbcAsynchFlipDisableFbcQueue:ivb */
> -- 
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 00/10] drm/i915: Some less complex FBC fixes

[Daniel Vetter]

On Thu, Jan 23, 2014 at 07:47:59PM +0000, Chris Wilson wrote:
> On Thu, Jan 23, 2014 at 04:49:07PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Since fixing the FBC locking is a bigger task that will take a while,
> > I decided to pull all the simple fixes from my branch and post them
> > right away.
> > 
> > Some of these I've posted before, some others have seen a bit of action
> > by being in a public branch.
> > 
> > The FBC_FENCE_OFF change is just a guess at this point. The odd offset
> > just caught my eye while reading throguh i915_reg.h.
> 
> I didn't spot anything offensive in the series and each patch only does
> what it says on the tin. So I am going to stick my neck out and say
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> for the series. Being picky, I guess Fix FBC_FENCE_OFF should only be an
> acked-by since we have no way to review it...

Entire series merged, and I'll fire up my g4x here to see what happens ;-)

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

Re: [PATCH 00/10] drm/i915: Some less complex FBC fixes

[Daniel Vetter]

On Sat, Jan 25, 2014 at 08:59:49PM +0100, Daniel Vetter wrote:
> On Thu, Jan 23, 2014 at 07:47:59PM +0000, Chris Wilson wrote:
> > On Thu, Jan 23, 2014 at 04:49:07PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Since fixing the FBC locking is a bigger task that will take a while,
> > > I decided to pull all the simple fixes from my branch and post them
> > > right away.
> > > 
> > > Some of these I've posted before, some others have seen a bit of action
> > > by being in a public branch.
> > > 
> > > The FBC_FENCE_OFF change is just a guess at this point. The odd offset
> > > just caught my eye while reading throguh i915_reg.h.
> > 
> > I didn't spot anything offensive in the series and each patch only does
> > what it says on the tin. So I am going to stick my neck out and say
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > for the series. Being picky, I guess Fix FBC_FENCE_OFF should only be an
> > acked-by since we have no way to review it...
> 
> Entire series merged, and I'll fire up my g4x here to see what happens ;-)

Hm, seems to fail on my g4x when I enable fbc. This is on latest -nightly:


> IGT-Version: 1.5-gb5109e62cea1 (x86_64) (Linux: 3.13.0-rc8+ x86_64)
Subtest bad-pipe: SUCCESS
Subtest bad-source: SUCCESS
Subtest bad-nb-words-1: SUCCESS
Subtest bad-nb-words-3: SUCCESS
read-crc-pipe-A: Testing connector 5
Subtest read-crc-pipe-A: SUCCESS
read-crc-pipe-A-frame-sequence: Testing connector 5
Subtest read-crc-pipe-A-frame-sequence: SUCCESS
read-crc-pipe-B: Testing connector 5
Subtest read-crc-pipe-B: SUCCESS
read-crc-pipe-B-frame-sequence: Testing connector 5
Subtest read-crc-pipe-B-frame-sequence: SUCCESS
Test requirement not met in function test_read_crc, file kms_pipe_crc_basic.c:210:
Last errno: 0, Success
Test requirement: (!(valid_connectors))
No connector found for pipe 2
Subtest read-crc-pipe-C: SKIP
Test requirement not met in function test_read_crc, file kms_pipe_crc_basic.c:210:
Last errno: 0, Success
Test requirement: (!(valid_connectors))
No connector found for pipe 2
Subtest read-crc-pipe-C-frame-sequence: SKIP
root@gina:/home/daniel/xorg/intel-gpu-tools# tests/kms_fbc_crc
IGT-Version: 1.5-gb5109e62cea1 (x86_64) (Linux: 3.13.0-rc8+ x86_64)
Beginning page_flip on crtc 3, connector 5

page_flip on crtc 3, connector 5: PASSED

Beginning page_flip on crtc 4, connector 5

page_flip on crtc 4, connector 5: PASSED

Subtest page_flip: SUCCESS
Beginning mmap_cpu on crtc 3, connector 5

mmap_cpu on crtc 3, connector 5: PASSED

Beginning mmap_cpu on crtc 4, connector 5

mmap_cpu on crtc 4, connector 5: PASSED

Subtest mmap_cpu: SUCCESS
Beginning mmap_gtt on crtc 3, connector 5

mmap_gtt on crtc 3, connector 5: PASSED

Beginning mmap_gtt on crtc 4, connector 5

mmap_gtt on crtc 4, connector 5: PASSED

Subtest mmap_gtt: SUCCESS
Beginning blt on crtc 3, connector 5

blt on crtc 3, connector 5: PASSED

Beginning blt on crtc 4, connector 5

blt on crtc 4, connector 5: PASSED

Subtest blt: SUCCESS
Beginning render on crtc 3, connector 5
Test requirement not met in function fill_render, file kms_fbc_crc.c:212:
Last errno: 0, Success
Test requirement: (!rendercopy)
Subtest render: SKIP
Test requirement not met in function prepare_crtc, file kms_fbc_crc.c:398:
Last errno: 19, No such device
Test requirement: (!(data->ctx[0]))
Subtest context: SKIP
Beginning page_flip_and_mmap_cpu on crtc 3, connector 5
Test assertion failure function test_crc, file kms_fbc_crc.c:315:
Last errno: 0, Success
Failed assertion: !igt_crc_equal(&crcs[0], &data->ref_crc[1])
Subtest page_flip_and_mmap_cpu: FAIL
Beginning page_flip_and_mmap_gtt on crtc 3, connector 5
Test assertion failure function test_crc, file kms_fbc_crc.c:315:
Last errno: 0, Success
Failed assertion: !igt_crc_equal(&crcs[0], &data->ref_crc[1])
Subtest page_flip_and_mmap_gtt: FAIL
Beginning page_flip_and_blt on crtc 3, connector 5

page_flip_and_blt on crtc 3, connector 5: PASSED

Beginning page_flip_and_blt on crtc 4, connector 5

page_flip_and_blt on crtc 4, connector 5: PASSED

Subtest page_flip_and_blt: SUCCESS
Beginning page_flip_and_render on crtc 3, connector 5
Test requirement not met in function fill_render, file kms_fbc_crc.c:212:
Last errno: 0, Success
Test requirement: (!rendercopy)
Subtest page_flip_and_render: SKIP
Test requirement not met in function prepare_crtc, file kms_fbc_crc.c:398:
Last errno: 19, No such device
Test requirement: (!(data->ctx[0]))
Subtest page_flip_and_context: SKIP

So something with flip + frontbuffer access seems still busted.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 00/10] drm/i915: Some less complex FBC fixes

[Daniel Vetter]

On Sun, Jan 26, 2014 at 03:33:32PM +0100, Daniel Vetter wrote:
> On Sat, Jan 25, 2014 at 08:59:49PM +0100, Daniel Vetter wrote:
> > On Thu, Jan 23, 2014 at 07:47:59PM +0000, Chris Wilson wrote:
> > > On Thu, Jan 23, 2014 at 04:49:07PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Since fixing the FBC locking is a bigger task that will take a while,
> > > > I decided to pull all the simple fixes from my branch and post them
> > > > right away.
> > > > 
> > > > Some of these I've posted before, some others have seen a bit of action
> > > > by being in a public branch.
> > > > 
> > > > The FBC_FENCE_OFF change is just a guess at this point. The odd offset
> > > > just caught my eye while reading throguh i915_reg.h.
> > > 
> > > I didn't spot anything offensive in the series and each patch only does
> > > what it says on the tin. So I am going to stick my neck out and say
> > > 
> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > 
> > > for the series. Being picky, I guess Fix FBC_FENCE_OFF should only be an
> > > acked-by since we have no way to review it...
> > 
> > Entire series merged, and I'll fire up my g4x here to see what happens ;-)
> 
> Hm, seems to fail on my g4x when I enable fbc. This is on latest -nightly:

Also, shouldn't we enable fbc now on all gen5+ machines? Or is there still
something outstanding to fix issues? Atm we only have it enabled on
haswell :(
-Daniel

> 
> 
> > IGT-Version: 1.5-gb5109e62cea1 (x86_64) (Linux: 3.13.0-rc8+ x86_64)
> Subtest bad-pipe: SUCCESS
> Subtest bad-source: SUCCESS
> Subtest bad-nb-words-1: SUCCESS
> Subtest bad-nb-words-3: SUCCESS
> read-crc-pipe-A: Testing connector 5
> Subtest read-crc-pipe-A: SUCCESS
> read-crc-pipe-A-frame-sequence: Testing connector 5
> Subtest read-crc-pipe-A-frame-sequence: SUCCESS
> read-crc-pipe-B: Testing connector 5
> Subtest read-crc-pipe-B: SUCCESS
> read-crc-pipe-B-frame-sequence: Testing connector 5
> Subtest read-crc-pipe-B-frame-sequence: SUCCESS
> Test requirement not met in function test_read_crc, file kms_pipe_crc_basic.c:210:
> Last errno: 0, Success
> Test requirement: (!(valid_connectors))
> No connector found for pipe 2
> Subtest read-crc-pipe-C: SKIP
> Test requirement not met in function test_read_crc, file kms_pipe_crc_basic.c:210:
> Last errno: 0, Success
> Test requirement: (!(valid_connectors))
> No connector found for pipe 2
> Subtest read-crc-pipe-C-frame-sequence: SKIP
> root@gina:/home/daniel/xorg/intel-gpu-tools# tests/kms_fbc_crc
> IGT-Version: 1.5-gb5109e62cea1 (x86_64) (Linux: 3.13.0-rc8+ x86_64)
> Beginning page_flip on crtc 3, connector 5
> 
> page_flip on crtc 3, connector 5: PASSED
> 
> Beginning page_flip on crtc 4, connector 5
> 
> page_flip on crtc 4, connector 5: PASSED
> 
> Subtest page_flip: SUCCESS
> Beginning mmap_cpu on crtc 3, connector 5
> 
> mmap_cpu on crtc 3, connector 5: PASSED
> 
> Beginning mmap_cpu on crtc 4, connector 5
> 
> mmap_cpu on crtc 4, connector 5: PASSED
> 
> Subtest mmap_cpu: SUCCESS
> Beginning mmap_gtt on crtc 3, connector 5
> 
> mmap_gtt on crtc 3, connector 5: PASSED
> 
> Beginning mmap_gtt on crtc 4, connector 5
> 
> mmap_gtt on crtc 4, connector 5: PASSED
> 
> Subtest mmap_gtt: SUCCESS
> Beginning blt on crtc 3, connector 5
> 
> blt on crtc 3, connector 5: PASSED
> 
> Beginning blt on crtc 4, connector 5
> 
> blt on crtc 4, connector 5: PASSED
> 
> Subtest blt: SUCCESS
> Beginning render on crtc 3, connector 5
> Test requirement not met in function fill_render, file kms_fbc_crc.c:212:
> Last errno: 0, Success
> Test requirement: (!rendercopy)
> Subtest render: SKIP
> Test requirement not met in function prepare_crtc, file kms_fbc_crc.c:398:
> Last errno: 19, No such device
> Test requirement: (!(data->ctx[0]))
> Subtest context: SKIP
> Beginning page_flip_and_mmap_cpu on crtc 3, connector 5
> Test assertion failure function test_crc, file kms_fbc_crc.c:315:
> Last errno: 0, Success
> Failed assertion: !igt_crc_equal(&crcs[0], &data->ref_crc[1])
> Subtest page_flip_and_mmap_cpu: FAIL
> Beginning page_flip_and_mmap_gtt on crtc 3, connector 5
> Test assertion failure function test_crc, file kms_fbc_crc.c:315:
> Last errno: 0, Success
> Failed assertion: !igt_crc_equal(&crcs[0], &data->ref_crc[1])
> Subtest page_flip_and_mmap_gtt: FAIL
> Beginning page_flip_and_blt on crtc 3, connector 5
> 
> page_flip_and_blt on crtc 3, connector 5: PASSED
> 
> Beginning page_flip_and_blt on crtc 4, connector 5
> 
> page_flip_and_blt on crtc 4, connector 5: PASSED
> 
> Subtest page_flip_and_blt: SUCCESS
> Beginning page_flip_and_render on crtc 3, connector 5
> Test requirement not met in function fill_render, file kms_fbc_crc.c:212:
> Last errno: 0, Success
> Test requirement: (!rendercopy)
> Subtest page_flip_and_render: SKIP
> Test requirement not met in function prepare_crtc, file kms_fbc_crc.c:398:
> Last errno: 19, No such device
> Test requirement: (!(data->ctx[0]))
> Subtest page_flip_and_context: SKIP
> 
> So something with flip + frontbuffer access seems still busted.
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

Re: [PATCH 00/10] drm/i915: Some less complex FBC fixes

[Ville Syrjälä]

On Sun, Jan 26, 2014 at 03:33:32PM +0100, Daniel Vetter wrote:
> On Sat, Jan 25, 2014 at 08:59:49PM +0100, Daniel Vetter wrote:
> > On Thu, Jan 23, 2014 at 07:47:59PM +0000, Chris Wilson wrote:
> > > On Thu, Jan 23, 2014 at 04:49:07PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Since fixing the FBC locking is a bigger task that will take a while,
> > > > I decided to pull all the simple fixes from my branch and post them
> > > > right away.
> > > > 
> > > > Some of these I've posted before, some others have seen a bit of action
> > > > by being in a public branch.
> > > > 
> > > > The FBC_FENCE_OFF change is just a guess at this point. The odd offset
> > > > just caught my eye while reading throguh i915_reg.h.
> > > 
> > > I didn't spot anything offensive in the series and each patch only does
> > > what it says on the tin. So I am going to stick my neck out and say
> > > 
> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > 
> > > for the series. Being picky, I guess Fix FBC_FENCE_OFF should only be an
> > > acked-by since we have no way to review it...
> > 
> > Entire series merged, and I'll fire up my g4x here to see what happens ;-)
> 
> Hm, seems to fail on my g4x when I enable fbc. This is on latest -nightly:
> 
> 
> > IGT-Version: 1.5-gb5109e62cea1 (x86_64) (Linux: 3.13.0-rc8+ x86_64)
> root@gina:/home/daniel/xorg/intel-gpu-tools# tests/kms_fbc_crc
> IGT-Version: 1.5-gb5109e62cea1 (x86_64) (Linux: 3.13.0-rc8+ x86_64)
> Beginning page_flip on crtc 3, connector 5
> 
> page_flip on crtc 3, connector 5: PASSED
> 
> Beginning page_flip on crtc 4, connector 5
> 
> page_flip on crtc 4, connector 5: PASSED
> 
> Subtest page_flip: SUCCESS
> Beginning mmap_cpu on crtc 3, connector 5
> 
> mmap_cpu on crtc 3, connector 5: PASSED
> 
> Beginning mmap_cpu on crtc 4, connector 5
> 
> mmap_cpu on crtc 4, connector 5: PASSED
> 
> Subtest mmap_cpu: SUCCESS

That's surprising considering the nuke stuff isn't there. It should
fail every time.

> Beginning mmap_gtt on crtc 3, connector 5
> 
> mmap_gtt on crtc 3, connector 5: PASSED
> 
> Beginning mmap_gtt on crtc 4, connector 5
> 
> mmap_gtt on crtc 4, connector 5: PASSED
> 
> Subtest mmap_gtt: SUCCESS
> Beginning blt on crtc 3, connector 5
> 
> blt on crtc 3, connector 5: PASSED
> 
> Beginning blt on crtc 4, connector 5
> 
> blt on crtc 4, connector 5: PASSED
> 
> Subtest blt: SUCCESS
> Beginning render on crtc 3, connector 5
> Test requirement not met in function fill_render, file kms_fbc_crc.c:212:
> Last errno: 0, Success
> Test requirement: (!rendercopy)
> Subtest render: SKIP
> Test requirement not met in function prepare_crtc, file kms_fbc_crc.c:398:
> Last errno: 19, No such device
> Test requirement: (!(data->ctx[0]))
> Subtest context: SKIP
> Beginning page_flip_and_mmap_cpu on crtc 3, connector 5
> Test assertion failure function test_crc, file kms_fbc_crc.c:315:
> Last errno: 0, Success
> Failed assertion: !igt_crc_equal(&crcs[0], &data->ref_crc[1])
> Subtest page_flip_and_mmap_cpu: FAIL
> Beginning page_flip_and_mmap_gtt on crtc 3, connector 5
> Test assertion failure function test_crc, file kms_fbc_crc.c:315:
> Last errno: 0, Success
> Failed assertion: !igt_crc_equal(&crcs[0], &data->ref_crc[1])
> Subtest page_flip_and_mmap_gtt: FAIL

And that's a bit weird since the fence is there. Although the FBC
locking is fubar, in this case there shouldn't be any issues since
the test doesn't try to do concurrent modesets or anything else
that would cause thing to go bad due to locking issues. So it's
not clear why it fails here.

Or could be my fence register fix was wrong after all. But then why
doesn't it fail for the non page flipped case?

In any case I think the test may need to be made harder. We should
really test the fence offset thing via panning, and probably write to
the first and last lines of the display to make sure the Y offset is
set up correctly.

> Beginning page_flip_and_blt on crtc 3, connector 5
> 
> page_flip_and_blt on crtc 3, connector 5: PASSED
> 
> Beginning page_flip_and_blt on crtc 4, connector 5
> 
> page_flip_and_blt on crtc 4, connector 5: PASSED
> 
> Subtest page_flip_and_blt: SUCCESS
> Beginning page_flip_and_render on crtc 3, connector 5
> Test requirement not met in function fill_render, file kms_fbc_crc.c:212:
> Last errno: 0, Success
> Test requirement: (!rendercopy)
> Subtest page_flip_and_render: SKIP
> Test requirement not met in function prepare_crtc, file kms_fbc_crc.c:398:
> Last errno: 19, No such device
> Test requirement: (!(data->ctx[0]))
> Subtest page_flip_and_context: SKIP
> 
> So something with flip + frontbuffer access seems still busted.
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 00/10] drm/i915: Some less complex FBC fixes

[Ville Syrjälä]

On Sun, Jan 26, 2014 at 03:35:42PM +0100, Daniel Vetter wrote:
> On Sun, Jan 26, 2014 at 03:33:32PM +0100, Daniel Vetter wrote:
> > On Sat, Jan 25, 2014 at 08:59:49PM +0100, Daniel Vetter wrote:
> > > On Thu, Jan 23, 2014 at 07:47:59PM +0000, Chris Wilson wrote:
> > > > On Thu, Jan 23, 2014 at 04:49:07PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > Since fixing the FBC locking is a bigger task that will take a while,
> > > > > I decided to pull all the simple fixes from my branch and post them
> > > > > right away.
> > > > > 
> > > > > Some of these I've posted before, some others have seen a bit of action
> > > > > by being in a public branch.
> > > > > 
> > > > > The FBC_FENCE_OFF change is just a guess at this point. The odd offset
> > > > > just caught my eye while reading throguh i915_reg.h.
> > > > 
> > > > I didn't spot anything offensive in the series and each patch only does
> > > > what it says on the tin. So I am going to stick my neck out and say
> > > > 
> > > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > 
> > > > for the series. Being picky, I guess Fix FBC_FENCE_OFF should only be an
> > > > acked-by since we have no way to review it...
> > > 
> > > Entire series merged, and I'll fire up my g4x here to see what happens ;-)
> > 
> > Hm, seems to fail on my g4x when I enable fbc. This is on latest -nightly:
> 
> Also, shouldn't we enable fbc now on all gen5+ machines? Or is there still
> something outstanding to fix issues? Atm we only have it enabled on
> haswell :(

The render tracking bits are still broken. And the locking is crap. I
started to fix it for real, but it turned out to be a fair amount of
work, so I figured I'll get back to it after I've cleared out some other
stuff from my desk.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 05/10] drm/i915: Use 1/2 compression ratio limit for 16bpp on FBC2

[Ville Syrjälä]

On Sat, Jan 25, 2014 at 08:57:34PM +0100, Daniel Vetter wrote:
> On Thu, Jan 23, 2014 at 04:49:12PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Hm, running the fbc tests with a 16bpp fb would be neat ...

Indeed. I'll put it on the list.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 24 +++++++++++++++++++-----
> >  1 file changed, 19 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index c6e047e..a7af5b4 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -155,7 +155,11 @@ static void g4x_enable_fbc(struct drm_crtc *crtc)
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >  	u32 dpfc_ctl;
> >  
> > -	dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_SR_EN | DPFC_CTL_LIMIT_1X;
> > +	dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_SR_EN;
> > +	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
> > +		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
> > +	else
> > +		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
> >  	dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
> >  
> >  	I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
> > @@ -225,7 +229,11 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc)
> >  
> >  	dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
> >  	dpfc_ctl &= DPFC_RESERVED;
> > -	dpfc_ctl |= DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_CTL_LIMIT_1X;
> > +	dpfc_ctl |= DPFC_CTL_PLANE(intel_crtc->plane);
> > +	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
> > +		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
> > +	else
> > +		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
> >  	dpfc_ctl |= DPFC_CTL_FENCE_EN;
> >  	if (IS_GEN5(dev))
> >  		dpfc_ctl |= obj->fence_reg;
> > @@ -275,10 +283,16 @@ static void gen7_enable_fbc(struct drm_crtc *crtc)
> >  	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> >  	struct drm_i915_gem_object *obj = intel_fb->obj;
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > +	u32 dpfc_ctl;
> >  
> > -	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
> > -		   IVB_DPFC_CTL_FENCE_EN |
> > -		   IVB_DPFC_CTL_PLANE(intel_crtc->plane));
> > +	dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane);
> > +	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
> > +		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
> > +	else
> > +		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
> > +	dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN;
> > +
> > +	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> >  
> >  	if (IS_IVYBRIDGE(dev)) {
> >  		/* WaFbcAsynchFlipDisableFbcQueue:ivb */
> > -- 
> > 1.8.3.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC