[PATCH 1/9] drm/i915: fix the FBC work allocation failure path

[PATCH 1/9] drm/i915: fix the FBC work allocation failure path

[Paulo Zanoni]

Always update the currrent crtc, fb and vertical offset after calling
enable_fbc. We were forgetting to do so along the failure paths when
enabling fbc synchronously. Fix this with a new helper to enable_fbc()
and update the state simultaneously.

v2: Improve commit message (Chris).
v3: Constify struct drm_framebuffer (Ville).

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 1f97fb5..9e42079 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -308,6 +308,18 @@ bool intel_fbc_enabled(struct drm_i915_private *dev_priv)
 	return dev_priv->fbc.enabled;
 }
 
+static void intel_fbc_enable(struct intel_crtc *crtc,
+			     const struct drm_framebuffer *fb)
+{
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+
+	dev_priv->fbc.enable_fbc(crtc);
+
+	dev_priv->fbc.crtc = crtc;
+	dev_priv->fbc.fb_id = fb->base.id;
+	dev_priv->fbc.y = crtc->base.y;
+}
+
 static void intel_fbc_work_fn(struct work_struct *__work)
 {
 	struct intel_fbc_work *work =
@@ -321,13 +333,8 @@ static void intel_fbc_work_fn(struct work_struct *__work)
 		/* Double check that we haven't switched fb without cancelling
 		 * the prior work.
 		 */
-		if (crtc_fb == work->fb) {
-			dev_priv->fbc.enable_fbc(work->crtc);
-
-			dev_priv->fbc.crtc = work->crtc;
-			dev_priv->fbc.fb_id = crtc_fb->base.id;
-			dev_priv->fbc.y = work->crtc->base.y;
-		}
+		if (crtc_fb == work->fb)
+			intel_fbc_enable(work->crtc, work->fb);
 
 		dev_priv->fbc.fbc_work = NULL;
 	}
@@ -361,7 +368,7 @@ static void intel_fbc_cancel_work(struct drm_i915_private *dev_priv)
 	dev_priv->fbc.fbc_work = NULL;
 }
 
-static void intel_fbc_enable(struct intel_crtc *crtc)
+static void intel_fbc_schedule_enable(struct intel_crtc *crtc)
 {
 	struct intel_fbc_work *work;
 	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
@@ -373,7 +380,7 @@ static void intel_fbc_enable(struct intel_crtc *crtc)
 	work = kzalloc(sizeof(*work), GFP_KERNEL);
 	if (work == NULL) {
 		DRM_ERROR("Failed to allocate FBC work structure\n");
-		dev_priv->fbc.enable_fbc(crtc);
+		intel_fbc_enable(crtc, crtc->base.primary->fb);
 		return;
 	}
 
@@ -824,7 +831,7 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv)
 		__intel_fbc_disable(dev_priv);
 	}
 
-	intel_fbc_enable(intel_crtc);
+	intel_fbc_schedule_enable(intel_crtc);
 	dev_priv->fbc.no_fbc_reason = FBC_OK;
 	return;
 
-- 
2.5.1

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

[PATCH 2/9] drm/i915: check for the supported strides on HSW+ FBC

[Paulo Zanoni]

Don't allow FBC for cases where the spec says we can't FBC.

v2:
  - Just WARN_ON() the strides that should have been caught earlier
    (Daniel)
  - Make it a new function since I expect this to grow more.
v3:
  - Document which IGT test is exercised by this.
v4:
  - Implement the restrictions for gens 2-6 too (Ville).
  - Fix off-by-one mistake (Ville).

Testcase: igt/kms_frontbuffer_tracking/fbc-badstride
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_fbc.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3bf8a9b..8cf2969 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -948,6 +948,7 @@ struct i915_fbc {
 		FBC_CHIP_DEFAULT, /* disabled by default on this chip */
 		FBC_ROTATION, /* rotation is not supported */
 		FBC_IN_DBG_MASTER, /* kernel debugger is active */
+		FBC_BAD_STRIDE, /* stride is not supported */
 	} no_fbc_reason;
 
 	bool (*fbc_enabled)(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 9e42079..db38091 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -480,6 +480,8 @@ const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
 		return "rotation unsupported";
 	case FBC_IN_DBG_MASTER:
 		return "Kernel debugger is active";
+	case FBC_BAD_STRIDE:
+		return "framebuffer stride not supported";
 	default:
 		MISSING_CASE(reason);
 		return "unknown reason";
@@ -671,6 +673,27 @@ static int intel_fbc_setup_cfb(struct drm_i915_private *dev_priv, int size,
 	return intel_fbc_alloc_cfb(dev_priv, size, fb_cpp);
 }
 
+static bool stride_is_valid(struct drm_i915_private *dev_priv,
+			    unsigned int stride)
+{
+	/* These should have been caught earlier. */
+	WARN_ON(stride < 512);
+	WARN_ON((stride & (64 - 1)) != 0);
+
+	/* Below are the additional FBC restrictions. */
+
+	if (IS_GEN2(dev_priv) || IS_GEN3(dev_priv))
+		return stride == 4096 || stride == 8192;
+
+	if (IS_GEN4(dev_priv) && !IS_G4X(dev_priv) && stride < 2048)
+		return false;
+
+	if (stride > 16384)
+		return false;
+
+	return true;
+}
+
 /**
  * __intel_fbc_update - enable/disable FBC as needed, unlocked
  * @dev_priv: i915 device instance
@@ -781,6 +804,11 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv)
 		goto out_disable;
 	}
 
+	if (!stride_is_valid(dev_priv, fb->pitches[0])) {
+		set_no_fbc_reason(dev_priv, FBC_BAD_STRIDE);
+		goto out_disable;
+	}
+
 	/* If the kernel debugger is active, always disable compression */
 	if (in_dbg_master()) {
 		set_no_fbc_reason(dev_priv, FBC_IN_DBG_MASTER);
-- 
2.5.1

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

[PATCH 3/9] drm/i915: avoid the last 8mb of stolen on BDW/SKL

[Paulo Zanoni]

The FBC hardware for these platforms doesn't have access to the
bios_reserved range, so it always assumes the maximum (8mb) is used.
So avoid this range while allocating.

This solves a bunch of FIFO underruns that happen if you end up
putting the CFB in that memory range. On my machine, with 32mb of
stolen, I need a 2560x1440 mode for that.

Testcase: igt/kms_frontbuffer_tracking/fbc-* (given the right setup)
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h        |  4 ++++
 drivers/gpu/drm/i915/i915_gem_gtt.h    |  1 +
 drivers/gpu/drm/i915/i915_gem_stolen.c | 26 +++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_fbc.c       | 16 ++++++++++++++--
 4 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8cf2969..1f02a5a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3173,6 +3173,10 @@ static inline void i915_gem_chipset_flush(struct drm_device *dev)
 int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
 				struct drm_mm_node *node, u64 size,
 				unsigned alignment);
+int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
+					 struct drm_mm_node *node, u64 size,
+					 unsigned alignment, u64 start,
+					 u64 end);
 void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
 				 struct drm_mm_node *node);
 int i915_gem_init_stolen(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 8275007..96ebb98 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -341,6 +341,7 @@ struct i915_gtt {
 	struct i915_address_space base;
 
 	size_t stolen_size;		/* Total size of stolen memory */
+	size_t stolen_usable_size;	/* Total size minus BIOS reserved */
 	u64 mappable_end;		/* End offset that we can CPU map */
 	struct io_mapping *mappable;	/* Mapping to our CPU mappable region */
 	phys_addr_t mappable_base;	/* PA of our GMADR */
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index bf26ecc..081ef6d 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -42,9 +42,9 @@
  * for is a boon.
  */
 
-int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
-				struct drm_mm_node *node, u64 size,
-				unsigned alignment)
+int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
+					 struct drm_mm_node *node, u64 size,
+					 unsigned alignment, u64 start, u64 end)
 {
 	int ret;
 
@@ -52,13 +52,23 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
 		return -ENODEV;
 
 	mutex_lock(&dev_priv->mm.stolen_lock);
-	ret = drm_mm_insert_node(&dev_priv->mm.stolen, node, size, alignment,
-				 DRM_MM_SEARCH_DEFAULT);
+	ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
+					  alignment, start, end,
+					  DRM_MM_SEARCH_DEFAULT);
 	mutex_unlock(&dev_priv->mm.stolen_lock);
 
 	return ret;
 }
 
+int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
+				struct drm_mm_node *node, u64 size,
+				unsigned alignment)
+{
+	return i915_gem_stolen_insert_node_in_range(dev_priv, node, size,
+					alignment, 0,
+					dev_priv->gtt.stolen_usable_size);
+}
+
 void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
 				 struct drm_mm_node *node)
 {
@@ -355,9 +365,11 @@ int i915_gem_init_stolen(struct drm_device *dev)
 		      dev_priv->gtt.stolen_size >> 10,
 		      (dev_priv->gtt.stolen_size - reserved_total) >> 10);
 
+	dev_priv->gtt.stolen_usable_size = dev_priv->gtt.stolen_size -
+					   reserved_total;
+
 	/* Basic memrange allocator for stolen space */
-	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
-		    reserved_total);
+	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index db38091..6f3c2ea 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -551,6 +551,16 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
 {
 	int compression_threshold = 1;
 	int ret;
+	u64 end;
+
+	/* The FBC hardware for BDW/SKL doesn't have access to the stolen
+	 * reserved range size, so it always assumes the maximum (8mb) is used.
+	 * If we enable FBC using a CFB on that memory range we'll get FIFO
+	 * underruns, even if that range is not reserved by the BIOS. */
+	if (IS_BROADWELL(dev_priv) || IS_SKYLAKE(dev_priv))
+		end = dev_priv->gtt.stolen_size - 8 * 1024 * 1024;
+	else
+		end = dev_priv->gtt.stolen_usable_size;
 
 	/* HACK: This code depends on what we will do in *_enable_fbc. If that
 	 * code changes, this code needs to change as well.
@@ -560,7 +570,8 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
 	 */
 
 	/* Try to over-allocate to reduce reallocations and fragmentation. */
-	ret = i915_gem_stolen_insert_node(dev_priv, node, size <<= 1, 4096);
+	ret = i915_gem_stolen_insert_node_in_range(dev_priv, node, size <<= 1,
+						   4096, 0, end);
 	if (ret == 0)
 		return compression_threshold;
 
@@ -570,7 +581,8 @@ again:
 	    (fb_cpp == 2 && compression_threshold == 2))
 		return 0;
 
-	ret = i915_gem_stolen_insert_node(dev_priv, node, size >>= 1, 4096);
+	ret = i915_gem_stolen_insert_node_in_range(dev_priv, node, size >>= 1,
+						   4096, 0, end);
 	if (ret && INTEL_INFO(dev_priv)->gen <= 4) {
 		return 0;
 	} else if (ret) {
-- 
2.5.1

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

[PATCH 4/9] drm/i915: print the correct amount of bytes allocated for the CFB

[Paulo Zanoni]

And also print the threshold. I was surprised to see a log message
claiming the CFB size was 32mb when there was less than 24mb available
for it.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 6f3c2ea..69726a7 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -634,8 +634,9 @@ static int intel_fbc_alloc_cfb(struct drm_i915_private *dev_priv, int size,
 
 	dev_priv->fbc.uncompressed_size = size;
 
-	DRM_DEBUG_KMS("reserved %d bytes of contiguous stolen space for FBC\n",
-		      size);
+	DRM_DEBUG_KMS("reserved %llu bytes of contiguous stolen space for FBC, threshold: %d\n",
+		      dev_priv->fbc.compressed_fb.size,
+		      dev_priv->fbc.threshold);
 
 	return 0;
 
-- 
2.5.1

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

[PATCH 5/9] drm/i915: don't enable FBC when pixel rate exceeds 95% on HSW/BDW

[Paulo Zanoni]

BSpec says we shouldn't enable FBC on HSW/BDW when the pipe pixel rate
exceeds 95% of the core display clock.

v2:
  - HSW also needs the WA (Ville).
  - Add the WA name (Ville).
  - Use the current cdclk (Ville).

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_fbc.c | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1f02a5a..d22120f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -949,6 +949,7 @@ struct i915_fbc {
 		FBC_ROTATION, /* rotation is not supported */
 		FBC_IN_DBG_MASTER, /* kernel debugger is active */
 		FBC_BAD_STRIDE, /* stride is not supported */
+		FBC_PIXEL_RATE, /* pixel rate is too big */
 	} no_fbc_reason;
 
 	bool (*fbc_enabled)(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 69726a7..1c4536a 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -482,6 +482,8 @@ const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
 		return "Kernel debugger is active";
 	case FBC_BAD_STRIDE:
 		return "framebuffer stride not supported";
+	case FBC_PIXEL_RATE:
+		return "pixel rate is too big";
 	default:
 		MISSING_CASE(reason);
 		return "unknown reason";
@@ -828,6 +830,14 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv)
 		goto out_disable;
 	}
 
+	/* WaFbcExceedCdClockThreshold:hsw,bdw */
+	if ((IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) &&
+	    ilk_pipe_pixel_rate(intel_crtc->config) >=
+	    dev_priv->cdclk_freq * 95 / 100) {
+		set_no_fbc_reason(dev_priv, FBC_PIXEL_RATE);
+		goto out_disable;
+	}
+
 	if (intel_fbc_setup_cfb(dev_priv, obj->base.size,
 				drm_format_plane_cpp(fb->pixel_format, 0))) {
 		set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL);
-- 
2.5.1

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

[PATCH 6/9] drm/i915: apply WaFbcAsynchFlipDisableFbcQueue earlier

[Paulo Zanoni]

The spec says the register should have that value for the entire time
that FBC is enabled, so apply the WA before we enable FBC.

Notice that we also have this WA for ILK/SNB, but it is implemented at
init_clock_gating(). I could move the IVB/HSW/BDW WA code to
init_clock_gating() too, but since we recently had some complaints
about WAs not staying after being set, I'm going to play safe and keep
this here for now.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 1c4536a..2b75003 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -272,8 +272,6 @@ static void gen7_fbc_enable(struct intel_crtc *crtc)
 	if (dev_priv->fbc.false_color)
 		dpfc_ctl |= FBC_CTL_FALSE_COLOR;
 
-	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
-
 	if (IS_IVYBRIDGE(dev_priv)) {
 		/* WaFbcAsynchFlipDisableFbcQueue:ivb */
 		I915_WRITE(ILK_DISPLAY_CHICKEN1,
@@ -286,6 +284,8 @@ static void gen7_fbc_enable(struct intel_crtc *crtc)
 			   HSW_FBCQ_DIS);
 	}
 
+	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
+
 	I915_WRITE(SNB_DPFC_CTL_SA,
 		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
 	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y);
-- 
2.5.1

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

[PATCH 7/9] drm/i915: don't apply WaFbcAsynchFlipDisableFbcQueue on SKL

[Paulo Zanoni]

This WA is only for HSW/BDW.

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

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 2b75003..56cf110 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -277,7 +277,7 @@ static void gen7_fbc_enable(struct intel_crtc *crtc)
 		I915_WRITE(ILK_DISPLAY_CHICKEN1,
 			   I915_READ(ILK_DISPLAY_CHICKEN1) |
 			   ILK_FBCQ_DIS);
-	} else {
+	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		/* WaFbcAsynchFlipDisableFbcQueue:hsw,bdw */
 		I915_WRITE(CHICKEN_PIPESL_1(crtc->pipe),
 			   I915_READ(CHICKEN_PIPESL_1(crtc->pipe)) |
-- 
2.5.1

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

[PATCH 8/9] drm/i915: reject invalid formats for FBC

[Paulo Zanoni]

This commit is essentially a rewrite of "drm/i915: Check pixel format
for fbc" from Ville Syrjälä. The idea is the same, but the code is
different due to all the changes that happened since his original
patch. So any bugs are due to my bad rewrite.

v2:
  - Drop the alpha formats (Ville).

Testcases: igt/kms_frontbuffer_tracking/*fbc*-${format_name}-draw-*
Credits-to: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_fbc.c | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d22120f..4e44352 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -950,6 +950,7 @@ struct i915_fbc {
 		FBC_IN_DBG_MASTER, /* kernel debugger is active */
 		FBC_BAD_STRIDE, /* stride is not supported */
 		FBC_PIXEL_RATE, /* pixel rate is too big */
+		FBC_PIXEL_FORMAT /* pixel format is invalid */
 	} no_fbc_reason;
 
 	bool (*fbc_enabled)(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 56cf110..a3a97f2 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -484,6 +484,8 @@ const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
 		return "framebuffer stride not supported";
 	case FBC_PIXEL_RATE:
 		return "pixel rate is too big";
+	case FBC_PIXEL_FORMAT:
+		return "pixel format is invalid";
 	default:
 		MISSING_CASE(reason);
 		return "unknown reason";
@@ -709,6 +711,31 @@ static bool stride_is_valid(struct drm_i915_private *dev_priv,
 	return true;
 }
 
+static bool pixel_format_is_valid(struct drm_framebuffer *fb)
+{
+	struct drm_device *dev = fb->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	/* Primary planes don't support alpha, so the "A" formats and "X"
+	 * formats are one and the same. */
+	switch (fb->pixel_format) {
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_XBGR8888:
+		return true;
+	case DRM_FORMAT_XRGB1555:
+	case DRM_FORMAT_RGB565:
+		/* 16bpp not supported on gen2 */
+		if (IS_GEN2(dev))
+			return false;
+		/* WaFbcOnly1to1Ratio:ctg */
+		if (IS_G4X(dev_priv))
+			return false;
+		return true;
+	default:
+		return false;
+	}
+}
+
 /**
  * __intel_fbc_update - enable/disable FBC as needed, unlocked
  * @dev_priv: i915 device instance
@@ -824,6 +851,11 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv)
 		goto out_disable;
 	}
 
+	if (!pixel_format_is_valid(fb)) {
+		set_no_fbc_reason(dev_priv, FBC_PIXEL_FORMAT);
+		goto out_disable;
+	}
+
 	/* If the kernel debugger is active, always disable compression */
 	if (in_dbg_master()) {
 		set_no_fbc_reason(dev_priv, FBC_IN_DBG_MASTER);
-- 
2.5.1

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

[PATCH 9/9] drm/i915: fix FBC for cases where crtc->base.y is non-zero

[Paulo Zanoni]

I only tested this on BDW and SKL, but since the register description
is the same ever since gen4, let's assume that all gens take the same
register format. If that's not true, then hopefully someone will
bisect a bug to this patch and we'll fix it.

Notice that the wrong fence offset register just means that the
hardware tracking will be wrong.

Testcases:
 - igt/kms_frontbuffer_tracking/fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt
 - igt/kms_frontbuffer_tracking/fbc-2p-primscrn-pri-shrfb-draw-mmap-gtt

v2:
  - Add intel_crtc->adjusted_{x,y} so this code can work independently
    of intel_gen4_compute_page_offset(). (Ville).
  - This version also works on SKL.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  9 +++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_fbc.c     | 25 ++++++++++++++++++++-----
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fc00867..c27a636 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2730,6 +2730,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 			(intel_crtc->config->pipe_src_w - 1) * pixel_size;
 	}
 
+	intel_crtc->adjusted_x = x;
+	intel_crtc->adjusted_y = y;
+
 	I915_WRITE(reg, dspcntr);
 
 	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
@@ -2830,6 +2833,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 		}
 	}
 
+	intel_crtc->adjusted_x = x;
+	intel_crtc->adjusted_y = y;
+
 	I915_WRITE(reg, dspcntr);
 
 	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
@@ -3082,6 +3088,9 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	}
 	plane_offset = y_offset << 16 | x_offset;
 
+	intel_crtc->adjusted_x = x_offset;
+	intel_crtc->adjusted_y = y_offset;
+
 	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
 	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
 	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 02a755a..6054bcb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -537,6 +537,8 @@ struct intel_crtc {
 	 * gen4+ this only adjusts up to a tile, offsets within a tile are
 	 * handled in the hw itself (with the TILEOFF register). */
 	unsigned long dspaddr_offset;
+	int adjusted_x;
+	int adjusted_y;
 
 	struct drm_i915_gem_object *cursor_bo;
 	uint32_t cursor_addr;
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index a3a97f2..2319dc5 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -41,6 +41,19 @@
 #include "intel_drv.h"
 #include "i915_drv.h"
 
+/*
+ * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the
+ * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's
+ * origin so the x and y offsets can actually fit the registers. As a
+ * consequence, the fence doesn't really start exactly at the display plane
+ * address we program because it starts at the real start of the buffer, so we
+ * have to take this into consideration here.
+ */
+static unsigned int get_crtc_fence_y_offset(struct intel_crtc *crtc)
+{
+	return crtc->base.y - crtc->adjusted_y;
+}
+
 static void i8xx_fbc_disable(struct drm_i915_private *dev_priv)
 {
 	u32 fbc_ctl;
@@ -97,7 +110,7 @@ static void i8xx_fbc_enable(struct intel_crtc *crtc)
 		fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
 		fbc_ctl2 |= FBC_CTL_PLANE(crtc->plane);
 		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
-		I915_WRITE(FBC_FENCE_OFF, crtc->base.y);
+		I915_WRITE(FBC_FENCE_OFF, get_crtc_fence_y_offset(crtc));
 	}
 
 	/* enable it... */
@@ -135,7 +148,7 @@ static void g4x_fbc_enable(struct intel_crtc *crtc)
 		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
 	dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
 
-	I915_WRITE(DPFC_FENCE_YOFF, crtc->base.y);
+	I915_WRITE(DPFC_FENCE_YOFF, get_crtc_fence_y_offset(crtc));
 
 	/* enable it... */
 	I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
@@ -177,6 +190,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	u32 dpfc_ctl;
 	int threshold = dev_priv->fbc.threshold;
+	unsigned int y_offset;
 
 	dev_priv->fbc.enabled = true;
 
@@ -200,7 +214,8 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
 	if (IS_GEN5(dev_priv))
 		dpfc_ctl |= obj->fence_reg;
 
-	I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->base.y);
+	y_offset = get_crtc_fence_y_offset(crtc);
+	I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);
 	I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
 	/* enable it... */
 	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
@@ -208,7 +223,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
 	if (IS_GEN6(dev_priv)) {
 		I915_WRITE(SNB_DPFC_CTL_SA,
 			   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
-		I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y);
+		I915_WRITE(DPFC_CPU_FENCE_OFFSET, y_offset);
 	}
 
 	intel_fbc_nuke(dev_priv);
@@ -288,7 +303,7 @@ static void gen7_fbc_enable(struct intel_crtc *crtc)
 
 	I915_WRITE(SNB_DPFC_CTL_SA,
 		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
-	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y);
+	I915_WRITE(DPFC_CPU_FENCE_OFFSET, get_crtc_fence_y_offset(crtc));
 
 	intel_fbc_nuke(dev_priv);
 
-- 
2.5.1

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

Re: [PATCH 2/9] drm/i915: check for the supported strides on HSW+ FBC

[Ville Syrjälä]

On Mon, Sep 14, 2015 at 03:19:56PM -0300, Paulo Zanoni wrote:
> Don't allow FBC for cases where the spec says we can't FBC.
> 
> v2:
>   - Just WARN_ON() the strides that should have been caught earlier
>     (Daniel)
>   - Make it a new function since I expect this to grow more.
> v3:
>   - Document which IGT test is exercised by this.
> v4:
>   - Implement the restrictions for gens 2-6 too (Ville).
>   - Fix off-by-one mistake (Ville).
> 
> Testcase: igt/kms_frontbuffer_tracking/fbc-badstride
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_fbc.c | 28 ++++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3bf8a9b..8cf2969 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -948,6 +948,7 @@ struct i915_fbc {
>  		FBC_CHIP_DEFAULT, /* disabled by default on this chip */
>  		FBC_ROTATION, /* rotation is not supported */
>  		FBC_IN_DBG_MASTER, /* kernel debugger is active */
> +		FBC_BAD_STRIDE, /* stride is not supported */
>  	} no_fbc_reason;
>  
>  	bool (*fbc_enabled)(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 9e42079..db38091 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -480,6 +480,8 @@ const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
>  		return "rotation unsupported";
>  	case FBC_IN_DBG_MASTER:
>  		return "Kernel debugger is active";
> +	case FBC_BAD_STRIDE:
> +		return "framebuffer stride not supported";
>  	default:
>  		MISSING_CASE(reason);
>  		return "unknown reason";
> @@ -671,6 +673,27 @@ static int intel_fbc_setup_cfb(struct drm_i915_private *dev_priv, int size,
>  	return intel_fbc_alloc_cfb(dev_priv, size, fb_cpp);
>  }
>  
> +static bool stride_is_valid(struct drm_i915_private *dev_priv,
> +			    unsigned int stride)
> +{
> +	/* These should have been caught earlier. */
> +	WARN_ON(stride < 512);
> +	WARN_ON((stride & (64 - 1)) != 0);
> +
> +	/* Below are the additional FBC restrictions. */
> +
> +	if (IS_GEN2(dev_priv) || IS_GEN3(dev_priv))
> +		return stride == 4096 || stride == 8192;
> +
> +	if (IS_GEN4(dev_priv) && !IS_G4X(dev_priv) && stride < 2048)
> +		return false;
> +
> +	if (stride > 16384)
> +		return false;

Looks to match what I caught from my spec trawling trip last time.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
> +	return true;
> +}
> +
>  /**
>   * __intel_fbc_update - enable/disable FBC as needed, unlocked
>   * @dev_priv: i915 device instance
> @@ -781,6 +804,11 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv)
>  		goto out_disable;
>  	}
>  
> +	if (!stride_is_valid(dev_priv, fb->pitches[0])) {
> +		set_no_fbc_reason(dev_priv, FBC_BAD_STRIDE);
> +		goto out_disable;
> +	}
> +
>  	/* If the kernel debugger is active, always disable compression */
>  	if (in_dbg_master()) {
>  		set_no_fbc_reason(dev_priv, FBC_IN_DBG_MASTER);
> -- 
> 2.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/9] drm/i915: avoid the last 8mb of stolen on BDW/SKL

[Ville Syrjälä]

On Mon, Sep 14, 2015 at 03:19:57PM -0300, Paulo Zanoni wrote:
> The FBC hardware for these platforms doesn't have access to the
> bios_reserved range, so it always assumes the maximum (8mb) is used.
> So avoid this range while allocating.
> 
> This solves a bunch of FIFO underruns that happen if you end up
> putting the CFB in that memory range. On my machine, with 32mb of
> stolen, I need a 2560x1440 mode for that.
> 
> Testcase: igt/kms_frontbuffer_tracking/fbc-* (given the right setup)
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Thwe only real spec quote I was able to find is in FBC_CFB_BASE:
"The buffer must not overlap the top 8 MB of stolen memory. | BDW,SKL,EXCLUDE(BXT)"

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h        |  4 ++++
>  drivers/gpu/drm/i915/i915_gem_gtt.h    |  1 +
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 26 +++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_fbc.c       | 16 ++++++++++++++--
>  4 files changed, 38 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8cf2969..1f02a5a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3173,6 +3173,10 @@ static inline void i915_gem_chipset_flush(struct drm_device *dev)
>  int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
>  				struct drm_mm_node *node, u64 size,
>  				unsigned alignment);
> +int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
> +					 struct drm_mm_node *node, u64 size,
> +					 unsigned alignment, u64 start,
> +					 u64 end);
>  void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
>  				 struct drm_mm_node *node);
>  int i915_gem_init_stolen(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 8275007..96ebb98 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -341,6 +341,7 @@ struct i915_gtt {
>  	struct i915_address_space base;
>  
>  	size_t stolen_size;		/* Total size of stolen memory */
> +	size_t stolen_usable_size;	/* Total size minus BIOS reserved */
>  	u64 mappable_end;		/* End offset that we can CPU map */
>  	struct io_mapping *mappable;	/* Mapping to our CPU mappable region */
>  	phys_addr_t mappable_base;	/* PA of our GMADR */
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index bf26ecc..081ef6d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -42,9 +42,9 @@
>   * for is a boon.
>   */
>  
> -int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> -				struct drm_mm_node *node, u64 size,
> -				unsigned alignment)
> +int i915_gem_stolen_insert_node_in_range(struct drm_i915_private *dev_priv,
> +					 struct drm_mm_node *node, u64 size,
> +					 unsigned alignment, u64 start, u64 end)
>  {
>  	int ret;
>  
> @@ -52,13 +52,23 @@ int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
>  		return -ENODEV;
>  
>  	mutex_lock(&dev_priv->mm.stolen_lock);
> -	ret = drm_mm_insert_node(&dev_priv->mm.stolen, node, size, alignment,
> -				 DRM_MM_SEARCH_DEFAULT);
> +	ret = drm_mm_insert_node_in_range(&dev_priv->mm.stolen, node, size,
> +					  alignment, start, end,
> +					  DRM_MM_SEARCH_DEFAULT);
>  	mutex_unlock(&dev_priv->mm.stolen_lock);
>  
>  	return ret;
>  }
>  
> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> +				struct drm_mm_node *node, u64 size,
> +				unsigned alignment)
> +{
> +	return i915_gem_stolen_insert_node_in_range(dev_priv, node, size,
> +					alignment, 0,
> +					dev_priv->gtt.stolen_usable_size);
> +}
> +
>  void i915_gem_stolen_remove_node(struct drm_i915_private *dev_priv,
>  				 struct drm_mm_node *node)
>  {
> @@ -355,9 +365,11 @@ int i915_gem_init_stolen(struct drm_device *dev)
>  		      dev_priv->gtt.stolen_size >> 10,
>  		      (dev_priv->gtt.stolen_size - reserved_total) >> 10);
>  
> +	dev_priv->gtt.stolen_usable_size = dev_priv->gtt.stolen_size -
> +					   reserved_total;
> +
>  	/* Basic memrange allocator for stolen space */
> -	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_size -
> -		    reserved_total);
> +	drm_mm_init(&dev_priv->mm.stolen, 0, dev_priv->gtt.stolen_usable_size);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index db38091..6f3c2ea 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -551,6 +551,16 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
>  {
>  	int compression_threshold = 1;
>  	int ret;
> +	u64 end;
> +
> +	/* The FBC hardware for BDW/SKL doesn't have access to the stolen
> +	 * reserved range size, so it always assumes the maximum (8mb) is used.
> +	 * If we enable FBC using a CFB on that memory range we'll get FIFO
> +	 * underruns, even if that range is not reserved by the BIOS. */
> +	if (IS_BROADWELL(dev_priv) || IS_SKYLAKE(dev_priv))
> +		end = dev_priv->gtt.stolen_size - 8 * 1024 * 1024;
> +	else
> +		end = dev_priv->gtt.stolen_usable_size;
>  
>  	/* HACK: This code depends on what we will do in *_enable_fbc. If that
>  	 * code changes, this code needs to change as well.
> @@ -560,7 +570,8 @@ static int find_compression_threshold(struct drm_i915_private *dev_priv,
>  	 */
>  
>  	/* Try to over-allocate to reduce reallocations and fragmentation. */
> -	ret = i915_gem_stolen_insert_node(dev_priv, node, size <<= 1, 4096);
> +	ret = i915_gem_stolen_insert_node_in_range(dev_priv, node, size <<= 1,
> +						   4096, 0, end);
>  	if (ret == 0)
>  		return compression_threshold;
>  
> @@ -570,7 +581,8 @@ again:
>  	    (fb_cpp == 2 && compression_threshold == 2))
>  		return 0;
>  
> -	ret = i915_gem_stolen_insert_node(dev_priv, node, size >>= 1, 4096);
> +	ret = i915_gem_stolen_insert_node_in_range(dev_priv, node, size >>= 1,
> +						   4096, 0, end);
>  	if (ret && INTEL_INFO(dev_priv)->gen <= 4) {
>  		return 0;
>  	} else if (ret) {
> -- 
> 2.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/9] drm/i915: don't enable FBC when pixel rate exceeds 95% on HSW/BDW

[Ville Syrjälä]

On Mon, Sep 14, 2015 at 03:19:59PM -0300, Paulo Zanoni wrote:
> BSpec says we shouldn't enable FBC on HSW/BDW when the pipe pixel rate
> exceeds 95% of the core display clock.
> 
> v2:
>   - HSW also needs the WA (Ville).
>   - Add the WA name (Ville).
>   - Use the current cdclk (Ville).
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_fbc.c | 10 ++++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1f02a5a..d22120f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -949,6 +949,7 @@ struct i915_fbc {
>  		FBC_ROTATION, /* rotation is not supported */
>  		FBC_IN_DBG_MASTER, /* kernel debugger is active */
>  		FBC_BAD_STRIDE, /* stride is not supported */
> +		FBC_PIXEL_RATE, /* pixel rate is too big */
>  	} no_fbc_reason;
>  
>  	bool (*fbc_enabled)(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 69726a7..1c4536a 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -482,6 +482,8 @@ const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
>  		return "Kernel debugger is active";
>  	case FBC_BAD_STRIDE:
>  		return "framebuffer stride not supported";
> +	case FBC_PIXEL_RATE:
> +		return "pixel rate is too big";
>  	default:
>  		MISSING_CASE(reason);
>  		return "unknown reason";
> @@ -828,6 +830,14 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv)
>  		goto out_disable;
>  	}
>  
> +	/* WaFbcExceedCdClockThreshold:hsw,bdw */
> +	if ((IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) &&
> +	    ilk_pipe_pixel_rate(intel_crtc->config) >=
> +	    dev_priv->cdclk_freq * 95 / 100) {
> +		set_no_fbc_reason(dev_priv, FBC_PIXEL_RATE);
> +		goto out_disable;
> +	}
> +
>  	if (intel_fbc_setup_cfb(dev_priv, obj->base.size,
>  				drm_format_plane_cpp(fb->pixel_format, 0))) {
>  		set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL);
> -- 
> 2.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/9] drm/i915: reject invalid formats for FBC

[Ville Syrjälä]

On Mon, Sep 14, 2015 at 03:20:02PM -0300, Paulo Zanoni wrote:
> This commit is essentially a rewrite of "drm/i915: Check pixel format
> for fbc" from Ville Syrjälä. The idea is the same, but the code is
> different due to all the changes that happened since his original
> patch. So any bugs are due to my bad rewrite.
> 
> v2:
>   - Drop the alpha formats (Ville).
> 
> Testcases: igt/kms_frontbuffer_tracking/*fbc*-${format_name}-draw-*
> Credits-to: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_fbc.c | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d22120f..4e44352 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -950,6 +950,7 @@ struct i915_fbc {
>  		FBC_IN_DBG_MASTER, /* kernel debugger is active */
>  		FBC_BAD_STRIDE, /* stride is not supported */
>  		FBC_PIXEL_RATE, /* pixel rate is too big */
> +		FBC_PIXEL_FORMAT /* pixel format is invalid */
>  	} no_fbc_reason;
>  
>  	bool (*fbc_enabled)(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 56cf110..a3a97f2 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -484,6 +484,8 @@ const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
>  		return "framebuffer stride not supported";
>  	case FBC_PIXEL_RATE:
>  		return "pixel rate is too big";
> +	case FBC_PIXEL_FORMAT:
> +		return "pixel format is invalid";
>  	default:
>  		MISSING_CASE(reason);
>  		return "unknown reason";
> @@ -709,6 +711,31 @@ static bool stride_is_valid(struct drm_i915_private *dev_priv,
>  	return true;
>  }
>  
> +static bool pixel_format_is_valid(struct drm_framebuffer *fb)
> +{
> +	struct drm_device *dev = fb->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	/* Primary planes don't support alpha, so the "A" formats and "X"
> +	 * formats are one and the same. */

Stale comment. With that removed:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	switch (fb->pixel_format) {
> +	case DRM_FORMAT_XRGB8888:
> +	case DRM_FORMAT_XBGR8888:
> +		return true;
> +	case DRM_FORMAT_XRGB1555:
> +	case DRM_FORMAT_RGB565:
> +		/* 16bpp not supported on gen2 */
> +		if (IS_GEN2(dev))
> +			return false;
> +		/* WaFbcOnly1to1Ratio:ctg */
> +		if (IS_G4X(dev_priv))
> +			return false;
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  /**
>   * __intel_fbc_update - enable/disable FBC as needed, unlocked
>   * @dev_priv: i915 device instance
> @@ -824,6 +851,11 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv)
>  		goto out_disable;
>  	}
>  
> +	if (!pixel_format_is_valid(fb)) {
> +		set_no_fbc_reason(dev_priv, FBC_PIXEL_FORMAT);
> +		goto out_disable;
> +	}
> +
>  	/* If the kernel debugger is active, always disable compression */
>  	if (in_dbg_master()) {
>  		set_no_fbc_reason(dev_priv, FBC_IN_DBG_MASTER);
> -- 
> 2.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 9/9] drm/i915: fix FBC for cases where crtc->base.y is non-zero

[Ville Syrjälä]

On Mon, Sep 14, 2015 at 03:20:03PM -0300, Paulo Zanoni wrote:
> I only tested this on BDW and SKL, but since the register description
> is the same ever since gen4, let's assume that all gens take the same
> register format. If that's not true, then hopefully someone will
> bisect a bug to this patch and we'll fix it.
> 
> Notice that the wrong fence offset register just means that the
> hardware tracking will be wrong.
> 
> Testcases:
>  - igt/kms_frontbuffer_tracking/fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt
>  - igt/kms_frontbuffer_tracking/fbc-2p-primscrn-pri-shrfb-draw-mmap-gtt
> 
> v2:
>   - Add intel_crtc->adjusted_{x,y} so this code can work independently
>     of intel_gen4_compute_page_offset(). (Ville).
>   - This version also works on SKL.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  9 +++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_fbc.c     | 25 ++++++++++++++++++++-----
>  3 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fc00867..c27a636 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2730,6 +2730,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  			(intel_crtc->config->pipe_src_w - 1) * pixel_size;
>  	}
>  
> +	intel_crtc->adjusted_x = x;
> +	intel_crtc->adjusted_y = y;
> +
>  	I915_WRITE(reg, dspcntr);
>  
>  	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
> @@ -2830,6 +2833,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  		}
>  	}
>  
> +	intel_crtc->adjusted_x = x;
> +	intel_crtc->adjusted_y = y;
> +
>  	I915_WRITE(reg, dspcntr);
>  
>  	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
> @@ -3082,6 +3088,9 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  	}
>  	plane_offset = y_offset << 16 | x_offset;
>  
> +	intel_crtc->adjusted_x = x_offset;
> +	intel_crtc->adjusted_y = y_offset;
> +
>  	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
>  	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
>  	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 02a755a..6054bcb 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -537,6 +537,8 @@ struct intel_crtc {
>  	 * gen4+ this only adjusts up to a tile, offsets within a tile are
>  	 * handled in the hw itself (with the TILEOFF register). */
>  	unsigned long dspaddr_offset;
> +	int adjusted_x;
> +	int adjusted_y;
>  
>  	struct drm_i915_gem_object *cursor_bo;
>  	uint32_t cursor_addr;
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index a3a97f2..2319dc5 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -41,6 +41,19 @@
>  #include "intel_drv.h"
>  #include "i915_drv.h"
>  
> +/*
> + * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the
> + * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's
> + * origin so the x and y offsets can actually fit the registers. As a
> + * consequence, the fence doesn't really start exactly at the display plane
> + * address we program because it starts at the real start of the buffer, so we
> + * have to take this into consideration here.
> + */
> +static unsigned int get_crtc_fence_y_offset(struct intel_crtc *crtc)
> +{
> +	return crtc->base.y - crtc->adjusted_y;
> +}

We may want to move over to using the plane state here in the future.
But for now it looks OK, so:

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>


> +
>  static void i8xx_fbc_disable(struct drm_i915_private *dev_priv)
>  {
>  	u32 fbc_ctl;
> @@ -97,7 +110,7 @@ static void i8xx_fbc_enable(struct intel_crtc *crtc)
>  		fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
>  		fbc_ctl2 |= FBC_CTL_PLANE(crtc->plane);
>  		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
> -		I915_WRITE(FBC_FENCE_OFF, crtc->base.y);
> +		I915_WRITE(FBC_FENCE_OFF, get_crtc_fence_y_offset(crtc));
>  	}
>  
>  	/* enable it... */
> @@ -135,7 +148,7 @@ static void g4x_fbc_enable(struct intel_crtc *crtc)
>  		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
>  	dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
>  
> -	I915_WRITE(DPFC_FENCE_YOFF, crtc->base.y);
> +	I915_WRITE(DPFC_FENCE_YOFF, get_crtc_fence_y_offset(crtc));
>  
>  	/* enable it... */
>  	I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> @@ -177,6 +190,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	u32 dpfc_ctl;
>  	int threshold = dev_priv->fbc.threshold;
> +	unsigned int y_offset;
>  
>  	dev_priv->fbc.enabled = true;
>  
> @@ -200,7 +214,8 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
>  	if (IS_GEN5(dev_priv))
>  		dpfc_ctl |= obj->fence_reg;
>  
> -	I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->base.y);
> +	y_offset = get_crtc_fence_y_offset(crtc);
> +	I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);
>  	I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
>  	/* enable it... */
>  	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> @@ -208,7 +223,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
>  	if (IS_GEN6(dev_priv)) {
>  		I915_WRITE(SNB_DPFC_CTL_SA,
>  			   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> -		I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y);
> +		I915_WRITE(DPFC_CPU_FENCE_OFFSET, y_offset);
>  	}
>  
>  	intel_fbc_nuke(dev_priv);
> @@ -288,7 +303,7 @@ static void gen7_fbc_enable(struct intel_crtc *crtc)
>  
>  	I915_WRITE(SNB_DPFC_CTL_SA,
>  		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> -	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y);
> +	I915_WRITE(DPFC_CPU_FENCE_OFFSET, get_crtc_fence_y_offset(crtc));
>  
>  	intel_fbc_nuke(dev_priv);
>  
> -- 
> 2.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 8/9] drm/i915: reject invalid formats for FBC

[Paulo Zanoni]

This commit is essentially a rewrite of "drm/i915: Check pixel format
for fbc" from Ville Syrjälä. The idea is the same, but the code is
different due to all the changes that happened since his original
patch. So any bugs are due to my bad rewrite.

v2:
  - Drop the alpha formats (Ville).
v3:
  - Drop the stale comment (Ville).

Testcases: igt/kms_frontbuffer_tracking/*fbc*-${format_name}-draw-*
Credits-to: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_fbc.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d22120f..4e44352 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -950,6 +950,7 @@ struct i915_fbc {
 		FBC_IN_DBG_MASTER, /* kernel debugger is active */
 		FBC_BAD_STRIDE, /* stride is not supported */
 		FBC_PIXEL_RATE, /* pixel rate is too big */
+		FBC_PIXEL_FORMAT /* pixel format is invalid */
 	} no_fbc_reason;
 
 	bool (*fbc_enabled)(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 56cf110..9d2f56e 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -484,6 +484,8 @@ const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
 		return "framebuffer stride not supported";
 	case FBC_PIXEL_RATE:
 		return "pixel rate is too big";
+	case FBC_PIXEL_FORMAT:
+		return "pixel format is invalid";
 	default:
 		MISSING_CASE(reason);
 		return "unknown reason";
@@ -709,6 +711,29 @@ static bool stride_is_valid(struct drm_i915_private *dev_priv,
 	return true;
 }
 
+static bool pixel_format_is_valid(struct drm_framebuffer *fb)
+{
+	struct drm_device *dev = fb->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	switch (fb->pixel_format) {
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_XBGR8888:
+		return true;
+	case DRM_FORMAT_XRGB1555:
+	case DRM_FORMAT_RGB565:
+		/* 16bpp not supported on gen2 */
+		if (IS_GEN2(dev))
+			return false;
+		/* WaFbcOnly1to1Ratio:ctg */
+		if (IS_G4X(dev_priv))
+			return false;
+		return true;
+	default:
+		return false;
+	}
+}
+
 /**
  * __intel_fbc_update - enable/disable FBC as needed, unlocked
  * @dev_priv: i915 device instance
@@ -824,6 +849,11 @@ static void __intel_fbc_update(struct drm_i915_private *dev_priv)
 		goto out_disable;
 	}
 
+	if (!pixel_format_is_valid(fb)) {
+		set_no_fbc_reason(dev_priv, FBC_PIXEL_FORMAT);
+		goto out_disable;
+	}
+
 	/* If the kernel debugger is active, always disable compression */
 	if (in_dbg_master()) {
 		set_no_fbc_reason(dev_priv, FBC_IN_DBG_MASTER);
-- 
2.5.1

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

Re: [PATCH 9/9] drm/i915: fix FBC for cases where crtc->base.y is non-zero

[Daniel Vetter]

Pulled in the entire series to dinq, thanks.
-Daniel

On Mon, Sep 21, 2015 at 05:04:49PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 14, 2015 at 03:20:03PM -0300, Paulo Zanoni wrote:
> > I only tested this on BDW and SKL, but since the register description
> > is the same ever since gen4, let's assume that all gens take the same
> > register format. If that's not true, then hopefully someone will
> > bisect a bug to this patch and we'll fix it.
> > 
> > Notice that the wrong fence offset register just means that the
> > hardware tracking will be wrong.
> > 
> > Testcases:
> >  - igt/kms_frontbuffer_tracking/fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt
> >  - igt/kms_frontbuffer_tracking/fbc-2p-primscrn-pri-shrfb-draw-mmap-gtt
> > 
> > v2:
> >   - Add intel_crtc->adjusted_{x,y} so this code can work independently
> >     of intel_gen4_compute_page_offset(). (Ville).
> >   - This version also works on SKL.
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c |  9 +++++++++
> >  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
> >  drivers/gpu/drm/i915/intel_fbc.c     | 25 ++++++++++++++++++++-----
> >  3 files changed, 31 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index fc00867..c27a636 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2730,6 +2730,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
> >  			(intel_crtc->config->pipe_src_w - 1) * pixel_size;
> >  	}
> >  
> > +	intel_crtc->adjusted_x = x;
> > +	intel_crtc->adjusted_y = y;
> > +
> >  	I915_WRITE(reg, dspcntr);
> >  
> >  	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
> > @@ -2830,6 +2833,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
> >  		}
> >  	}
> >  
> > +	intel_crtc->adjusted_x = x;
> > +	intel_crtc->adjusted_y = y;
> > +
> >  	I915_WRITE(reg, dspcntr);
> >  
> >  	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
> > @@ -3082,6 +3088,9 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
> >  	}
> >  	plane_offset = y_offset << 16 | x_offset;
> >  
> > +	intel_crtc->adjusted_x = x_offset;
> > +	intel_crtc->adjusted_y = y_offset;
> > +
> >  	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> >  	I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset);
> >  	I915_WRITE(PLANE_SIZE(pipe, 0), plane_size);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 02a755a..6054bcb 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -537,6 +537,8 @@ struct intel_crtc {
> >  	 * gen4+ this only adjusts up to a tile, offsets within a tile are
> >  	 * handled in the hw itself (with the TILEOFF register). */
> >  	unsigned long dspaddr_offset;
> > +	int adjusted_x;
> > +	int adjusted_y;
> >  
> >  	struct drm_i915_gem_object *cursor_bo;
> >  	uint32_t cursor_addr;
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > index a3a97f2..2319dc5 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -41,6 +41,19 @@
> >  #include "intel_drv.h"
> >  #include "i915_drv.h"
> >  
> > +/*
> > + * In some platforms where the CRTC's x:0/y:0 coordinates doesn't match the
> > + * frontbuffer's x:0/y:0 coordinates we lie to the hardware about the plane's
> > + * origin so the x and y offsets can actually fit the registers. As a
> > + * consequence, the fence doesn't really start exactly at the display plane
> > + * address we program because it starts at the real start of the buffer, so we
> > + * have to take this into consideration here.
> > + */
> > +static unsigned int get_crtc_fence_y_offset(struct intel_crtc *crtc)
> > +{
> > +	return crtc->base.y - crtc->adjusted_y;
> > +}
> 
> We may want to move over to using the plane state here in the future.
> But for now it looks OK, so:
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> 
> > +
> >  static void i8xx_fbc_disable(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 fbc_ctl;
> > @@ -97,7 +110,7 @@ static void i8xx_fbc_enable(struct intel_crtc *crtc)
> >  		fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
> >  		fbc_ctl2 |= FBC_CTL_PLANE(crtc->plane);
> >  		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
> > -		I915_WRITE(FBC_FENCE_OFF, crtc->base.y);
> > +		I915_WRITE(FBC_FENCE_OFF, get_crtc_fence_y_offset(crtc));
> >  	}
> >  
> >  	/* enable it... */
> > @@ -135,7 +148,7 @@ static void g4x_fbc_enable(struct intel_crtc *crtc)
> >  		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
> >  	dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
> >  
> > -	I915_WRITE(DPFC_FENCE_YOFF, crtc->base.y);
> > +	I915_WRITE(DPFC_FENCE_YOFF, get_crtc_fence_y_offset(crtc));
> >  
> >  	/* enable it... */
> >  	I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> > @@ -177,6 +190,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
> >  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> >  	u32 dpfc_ctl;
> >  	int threshold = dev_priv->fbc.threshold;
> > +	unsigned int y_offset;
> >  
> >  	dev_priv->fbc.enabled = true;
> >  
> > @@ -200,7 +214,8 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
> >  	if (IS_GEN5(dev_priv))
> >  		dpfc_ctl |= obj->fence_reg;
> >  
> > -	I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->base.y);
> > +	y_offset = get_crtc_fence_y_offset(crtc);
> > +	I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset);
> >  	I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
> >  	/* enable it... */
> >  	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> > @@ -208,7 +223,7 @@ static void ilk_fbc_enable(struct intel_crtc *crtc)
> >  	if (IS_GEN6(dev_priv)) {
> >  		I915_WRITE(SNB_DPFC_CTL_SA,
> >  			   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> > -		I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y);
> > +		I915_WRITE(DPFC_CPU_FENCE_OFFSET, y_offset);
> >  	}
> >  
> >  	intel_fbc_nuke(dev_priv);
> > @@ -288,7 +303,7 @@ static void gen7_fbc_enable(struct intel_crtc *crtc)
> >  
> >  	I915_WRITE(SNB_DPFC_CTL_SA,
> >  		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> > -	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->base.y);
> > +	I915_WRITE(DPFC_CPU_FENCE_OFFSET, get_crtc_fence_y_offset(crtc));
> >  
> >  	intel_fbc_nuke(dev_priv);
> >  
> > -- 
> > 2.5.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx