All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] FBC support for gen2/3
@ 2013-11-28 15:29 ville.syrjala
  2013-11-28 15:29 ` [PATCH 1/8] drm/i915: Fix bogus FBC1 defines ville.syrjala
                   ` (7 more replies)
  0 siblings, 8 replies; 31+ messages in thread
From: ville.syrjala @ 2013-11-28 15:29 UTC (permalink / raw)
  To: intel-gfx

This series enables FBC on gen2 and gen3 platforms. Only tested on 855GM.

I have a bunch of other FBC patches still in the pipeline, but this should
have all the basics for gen2/3.

Ville Syrjälä (8):
      drm/i915: Fix bogus FBC1 defines
      drm/i915: Gen2 FBC1 CFB pitch wants 32B units
      drm/i915: FBC_CONTROL2 is gen4 only
      drm/i915: Fix FBC1 plane checks for gen2
      drm/i915: Reorganize FBC function pointer initializaition
      drm/i915: Rework the FBC interval/stall stuff a bit
      drm/i915: Swap primary planes on gen2 for FBC
      drm/i915: Enable FBC for all mobile gen2 and gen3 platforms

 drivers/gpu/drm/i915/i915_drv.c      |  3 ++
 drivers/gpu/drm/i915/i915_drv.h      |  4 +-
 drivers/gpu/drm/i915/i915_irq.c      | 14 ++++--
 drivers/gpu/drm/i915/i915_reg.h      |  4 +-
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 drivers/gpu/drm/i915/intel_pm.c      | 91 +++++++++++++++++-------------------
 6 files changed, 58 insertions(+), 60 deletions(-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 1/8] drm/i915: Fix bogus FBC1 defines
  2013-11-28 15:29 [PATCH 0/8] FBC support for gen2/3 ville.syrjala
@ 2013-11-28 15:29 ` ville.syrjala
  2013-11-29 13:59   ` Chris Wilson
  2013-11-28 15:29 ` [PATCH 2/8] drm/i915: Gen2 FBC1 CFB pitch wants 32B units ville.syrjala
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: ville.syrjala @ 2013-11-28 15:29 UTC (permalink / raw)
  To: intel-gfx

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

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6de26eb..62f3a30 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1028,14 +1028,14 @@
 #define   FBC_CTL_UNCOMPRESSIBLE (1<<14)
 #define   FBC_CTL_C3_IDLE	(1<<13)
 #define   FBC_CTL_STRIDE_SHIFT	(5)
-#define   FBC_CTL_FENCENO	(1<<0)
+#define   FBC_CTL_FENCENO_SHIFT	(0)
 #define FBC_COMMAND		0x0320c
 #define   FBC_CMD_COMPRESS	(1<<0)
 #define FBC_STATUS		0x03210
 #define   FBC_STAT_COMPRESSING	(1<<31)
 #define   FBC_STAT_COMPRESSED	(1<<30)
 #define   FBC_STAT_MODIFIED	(1<<29)
-#define   FBC_STAT_CURRENT_LINE	(1<<0)
+#define   FBC_STAT_CURRENT_LINE_SHIFT	(0)
 #define FBC_CONTROL2		0x03214
 #define   FBC_CTL_FENCE_DBL	(0<<4)
 #define   FBC_CTL_IDLE_IMM	(0<<2)
-- 
1.8.3.2

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

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 2/8] drm/i915: Gen2 FBC1 CFB pitch wants 32B units
  2013-11-28 15:29 [PATCH 0/8] FBC support for gen2/3 ville.syrjala
  2013-11-28 15:29 ` [PATCH 1/8] drm/i915: Fix bogus FBC1 defines ville.syrjala
@ 2013-11-28 15:29 ` ville.syrjala
  2013-12-12 12:54   ` Imre Deak
  2013-11-28 15:29 ` [PATCH 3/8] drm/i915: FBC_CONTROL2 is gen4 only ville.syrjala
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: ville.syrjala @ 2013-11-28 15:29 UTC (permalink / raw)
  To: intel-gfx

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

On gen2 the compressed frame buffer pitch is specified in 32B units
rather than the 64B units used on gen3+.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 614549b..1dee34c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -103,8 +103,11 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc,
 	if (fb->pitches[0] < cfb_pitch)
 		cfb_pitch = fb->pitches[0];
 
-	/* FBC_CTL wants 64B units */
-	cfb_pitch = (cfb_pitch / 64) - 1;
+	/* FBC_CTL wants 32B or 64B units */
+	if (IS_GEN2(dev))
+		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 */
-- 
1.8.3.2

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

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 3/8] drm/i915: FBC_CONTROL2 is gen4 only
  2013-11-28 15:29 [PATCH 0/8] FBC support for gen2/3 ville.syrjala
  2013-11-28 15:29 ` [PATCH 1/8] drm/i915: Fix bogus FBC1 defines ville.syrjala
  2013-11-28 15:29 ` [PATCH 2/8] drm/i915: Gen2 FBC1 CFB pitch wants 32B units ville.syrjala
@ 2013-11-28 15:29 ` ville.syrjala
  2013-11-29 14:01   ` Chris Wilson
  2013-11-28 15:29 ` [PATCH 4/8] drm/i915: Fix FBC1 plane checks for gen2 ville.syrjala
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: ville.syrjala @ 2013-11-28 15:29 UTC (permalink / raw)
  To: intel-gfx

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

Gen2 and gen3 don't have the FBC_CONTROL2 register, so don't
touch it.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1dee34c..3d4a47c 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 intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int cfb_pitch;
 	int plane, i;
-	u32 fbc_ctl, fbc_ctl2;
+	u32 fbc_ctl;
 
 	cfb_pitch = dev_priv->fbc.size / FBC_LL_SIZE;
 	if (fb->pitches[0] < cfb_pitch)
@@ -114,11 +114,15 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc,
 	for (i = 0; i < (FBC_LL_SIZE / 32) + 1; i++)
 		I915_WRITE(FBC_TAG + (i * 4), 0);
 
-	/* Set it up... */
-	fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
-	fbc_ctl2 |= plane;
-	I915_WRITE(FBC_CONTROL2, fbc_ctl2);
-	I915_WRITE(FBC_FENCE_OFF, crtc->y);
+	if (IS_GEN4(dev)) {
+		u32 fbc_ctl2;
+
+		/* Set it up... */
+		fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
+		fbc_ctl2 |= plane;
+		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
+		I915_WRITE(FBC_FENCE_OFF, crtc->y);
+	}
 
 	/* enable it... */
 	fbc_ctl = FBC_CTL_EN | FBC_CTL_PERIODIC;
-- 
1.8.3.2

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

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 4/8] drm/i915: Fix FBC1 plane checks for gen2
  2013-11-28 15:29 [PATCH 0/8] FBC support for gen2/3 ville.syrjala
                   ` (2 preceding siblings ...)
  2013-11-28 15:29 ` [PATCH 3/8] drm/i915: FBC_CONTROL2 is gen4 only ville.syrjala
@ 2013-11-28 15:29 ` ville.syrjala
  2013-11-29 13:57   ` Chris Wilson
  2013-11-28 15:29 ` [PATCH 5/8] drm/i915: Reorganize FBC function pointer initializaition ville.syrjala
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: ville.syrjala @ 2013-11-28 15:29 UTC (permalink / raw)
  To: intel-gfx

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

On gen2 and gen3 chipsets FBC is supported only on plane A. Fix (and
simplify) the plane checks in intel_update_fbc() accordingly.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3d4a47c..614c09a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -567,10 +567,10 @@ void intel_update_fbc(struct drm_device *dev)
 			DRM_DEBUG_KMS("mode too large for compression, disabling\n");
 		goto out_disable;
 	}
-	if ((IS_I915GM(dev) || IS_I945GM(dev) || IS_HASWELL(dev)) &&
-	    intel_crtc->plane != 0) {
+	if ((INTEL_INFO(dev)->gen < 4 || IS_HASWELL(dev)) &&
+	    intel_crtc->plane != PLANE_A) {
 		if (set_no_fbc_reason(dev_priv, FBC_BAD_PLANE))
-			DRM_DEBUG_KMS("plane not 0, disabling compression\n");
+			DRM_DEBUG_KMS("plane not A, disabling compression\n");
 		goto out_disable;
 	}
 
-- 
1.8.3.2

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

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 5/8] drm/i915: Reorganize FBC function pointer initializaition
  2013-11-28 15:29 [PATCH 0/8] FBC support for gen2/3 ville.syrjala
                   ` (3 preceding siblings ...)
  2013-11-28 15:29 ` [PATCH 4/8] drm/i915: Fix FBC1 plane checks for gen2 ville.syrjala
@ 2013-11-28 15:29 ` ville.syrjala
  2013-11-29 13:59   ` Chris Wilson
  2013-11-28 15:30 ` [PATCH 6/8] drm/i915: Rework the FBC interval/stall stuff a bit ville.syrjala
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: ville.syrjala @ 2013-11-28 15:29 UTC (permalink / raw)
  To: intel-gfx

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

Initialize the FBC vfuncs on gen2 and gen3 chipsets. Also make
a clean split for gen7+ vs. gen5+ vfunc initialization.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 614c09a..b296438 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5977,25 +5977,23 @@ void intel_init_pm(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (I915_HAS_FBC(dev)) {
-		if (HAS_PCH_SPLIT(dev)) {
+		if (INTEL_INFO(dev)->gen >= 7) {
 			dev_priv->display.fbc_enabled = ironlake_fbc_enabled;
-			if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
-				dev_priv->display.enable_fbc =
-					gen7_enable_fbc;
-			else
-				dev_priv->display.enable_fbc =
-					ironlake_enable_fbc;
+			dev_priv->display.enable_fbc = gen7_enable_fbc;
+			dev_priv->display.disable_fbc = ironlake_disable_fbc;
+		} else if (INTEL_INFO(dev)->gen >= 5) {
+			dev_priv->display.fbc_enabled = ironlake_fbc_enabled;
+			dev_priv->display.enable_fbc = ironlake_enable_fbc;
 			dev_priv->display.disable_fbc = ironlake_disable_fbc;
 		} else if (IS_GM45(dev)) {
 			dev_priv->display.fbc_enabled = g4x_fbc_enabled;
 			dev_priv->display.enable_fbc = g4x_enable_fbc;
 			dev_priv->display.disable_fbc = g4x_disable_fbc;
-		} else if (IS_CRESTLINE(dev)) {
+		} else {
 			dev_priv->display.fbc_enabled = i8xx_fbc_enabled;
 			dev_priv->display.enable_fbc = i8xx_enable_fbc;
 			dev_priv->display.disable_fbc = i8xx_disable_fbc;
 		}
-		/* 855GM needs testing */
 	}
 
 	/* For cxsr */
-- 
1.8.3.2

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

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 6/8] drm/i915: Rework the FBC interval/stall stuff a bit
  2013-11-28 15:29 [PATCH 0/8] FBC support for gen2/3 ville.syrjala
                   ` (4 preceding siblings ...)
  2013-11-28 15:29 ` [PATCH 5/8] drm/i915: Reorganize FBC function pointer initializaition ville.syrjala
@ 2013-11-28 15:30 ` ville.syrjala
  2013-11-29 13:53   ` Chris Wilson
  2013-12-12 14:04   ` Imre Deak
  2013-11-28 15:30 ` [PATCH 7/8] drm/i915: Swap primary planes on gen2 for FBC ville.syrjala
  2013-11-28 15:30 ` [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms ville.syrjala
  7 siblings, 2 replies; 31+ messages in thread
From: ville.syrjala @ 2013-11-28 15:30 UTC (permalink / raw)
  To: intel-gfx

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

Don't touch DPFC_RECOMP_CTL on FBC2, use RMW to update
the FBC_CONTROL on FBC1 to make it easier for people to
experiment with different numbers. Also fix the interval
mask for FBC1.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c5cbbc2..20a9811 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -373,8 +373,7 @@ struct dpll;
 struct drm_i915_display_funcs {
 	bool (*fbc_enabled)(struct drm_device *dev);
 	void (*enable_fbc)(struct drm_crtc *crtc,
-			   struct drm_framebuffer *fb,
-			   unsigned long interval);
+			   struct drm_framebuffer *fb);
 	void (*disable_fbc)(struct drm_device *dev);
 	int (*get_display_clock_speed)(struct drm_device *dev);
 	int (*get_fifo_size)(struct drm_device *dev, int plane);
@@ -692,7 +691,6 @@ struct i915_fbc {
 		struct delayed_work work;
 		struct drm_crtc *crtc;
 		struct drm_framebuffer *fb;
-		int interval;
 	} *fbc_work;
 
 	enum no_fbc_reason {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b296438..af2bc2b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -87,8 +87,7 @@ static void i8xx_disable_fbc(struct drm_device *dev)
 }
 
 static void i8xx_enable_fbc(struct drm_crtc *crtc,
-			    struct drm_framebuffer *fb,
-			    unsigned long interval)
+			    struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -125,11 +124,12 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc,
 	}
 
 	/* enable it... */
-	fbc_ctl = FBC_CTL_EN | FBC_CTL_PERIODIC;
+	fbc_ctl = I915_READ(FBC_CONTROL);
+	fbc_ctl &= 0x3fff << FBC_CTL_INTERVAL_SHIFT;
+	fbc_ctl |= FBC_CTL_EN | FBC_CTL_PERIODIC;
 	if (IS_I945GM(dev))
 		fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
 	fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
-	fbc_ctl |= (interval & 0x2fff) << FBC_CTL_INTERVAL_SHIFT;
 	fbc_ctl |= obj->fence_reg;
 	I915_WRITE(FBC_CONTROL, fbc_ctl);
 
@@ -145,8 +145,7 @@ static bool i8xx_fbc_enabled(struct drm_device *dev)
 }
 
 static void g4x_enable_fbc(struct drm_crtc *crtc,
-			   struct drm_framebuffer *fb,
-			   unsigned long interval)
+			   struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -154,15 +153,10 @@ static void g4x_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 plane = intel_crtc->plane == 0 ? DPFC_CTL_PLANEA : DPFC_CTL_PLANEB;
-	unsigned long stall_watermark = 200;
 	u32 dpfc_ctl;
 
 	dpfc_ctl = plane | DPFC_SR_EN | DPFC_CTL_LIMIT_1X;
 	dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
-
-	I915_WRITE(DPFC_RECOMP_CTL, DPFC_RECOMP_STALL_EN |
-		   (stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) |
-		   (interval << DPFC_RECOMP_TIMER_COUNT_SHIFT));
 	I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
 
 	/* enable it... */
@@ -194,8 +188,7 @@ static bool g4x_fbc_enabled(struct drm_device *dev)
 }
 
 static void ironlake_enable_fbc(struct drm_crtc *crtc,
-				struct drm_framebuffer *fb,
-				unsigned long interval)
+				struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -203,7 +196,6 @@ static void ironlake_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 plane = intel_crtc->plane == 0 ? DPFC_CTL_PLANEA : DPFC_CTL_PLANEB;
-	unsigned long stall_watermark = 200;
 	u32 dpfc_ctl;
 
 	dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
@@ -212,10 +204,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_RECOMP_CTL, DPFC_RECOMP_STALL_EN |
-		   (stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) |
-		   (interval << DPFC_RECOMP_TIMER_COUNT_SHIFT));
 	I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->y);
 	/* enable it... */
 	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
@@ -252,8 +240,7 @@ static bool ironlake_fbc_enabled(struct drm_device *dev)
 }
 
 static void gen7_enable_fbc(struct drm_crtc *crtc,
-			    struct drm_framebuffer *fb,
-			    unsigned long interval)
+			    struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -292,12 +279,11 @@ bool intel_fbc_enabled(struct drm_device *dev)
 }
 
 static void intel_setup_fbc(struct drm_crtc *crtc,
-			    struct drm_framebuffer *fb,
-			    unsigned long interval)
+			    struct drm_framebuffer *fb)
 {
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
 
-	dev_priv->display.enable_fbc(crtc, fb, interval);
+	dev_priv->display.enable_fbc(crtc, fb);
 
 	dev_priv->fbc.plane = to_intel_crtc(crtc)->plane;
 	drm_framebuffer_reference(fb);
@@ -324,7 +310,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
 		 * the prior work.
 		 */
 		if (work->crtc->fb == work->fb)
-			intel_setup_fbc(work->crtc, work->fb, work->interval);
+			intel_setup_fbc(work->crtc, work->fb);
 		dev_priv->fbc.fbc_work = NULL;
 	}
 	mutex_unlock(&dev->struct_mutex);
@@ -361,7 +347,7 @@ static void intel_cancel_fbc_work(struct drm_i915_private *dev_priv)
 	dev_priv->fbc.fbc_work = NULL;
 }
 
-static void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
+static void intel_enable_fbc(struct drm_crtc *crtc)
 {
 	struct intel_fbc_work *work;
 	struct drm_device *dev = crtc->dev;
@@ -375,13 +361,12 @@ static void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	work = kzalloc(sizeof(*work), GFP_KERNEL);
 	if (work == NULL) {
 		DRM_ERROR("Failed to allocate FBC work structure\n");
-		intel_setup_fbc(crtc, crtc->fb, interval);
+		intel_setup_fbc(crtc, crtc->fb);
 		return;
 	}
 
 	work->crtc = crtc;
 	work->fb = crtc->fb;
-	work->interval = interval;
 	INIT_DELAYED_WORK(&work->work, intel_fbc_work_fn);
 
 	drm_framebuffer_reference(work->fb);
@@ -448,7 +433,7 @@ void intel_fbc_nuke(struct drm_i915_gem_object *obj)
 	 * Must wait until the next vblank before re-enabling
 	 * otherwise the nuking won't actually happen.
 	 */
-	intel_enable_fbc(crtc, 500);
+	intel_enable_fbc(crtc);
 }
 
 static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
@@ -632,7 +617,7 @@ void intel_update_fbc(struct drm_device *dev)
 		intel_disable_fbc(dev);
 	}
 
-	intel_enable_fbc(crtc, 500);
+	intel_enable_fbc(crtc);
 	dev_priv->fbc.no_fbc_reason = FBC_OK;
 	return;
 
@@ -5993,6 +5978,9 @@ void intel_init_pm(struct drm_device *dev)
 			dev_priv->display.fbc_enabled = i8xx_fbc_enabled;
 			dev_priv->display.enable_fbc = i8xx_enable_fbc;
 			dev_priv->display.disable_fbc = i8xx_disable_fbc;
+
+			/* This value was pulled out of someone's hat */
+			I915_WRITE(FBC_CONTROL, 500 << FBC_CTL_INTERVAL_SHIFT);
 		}
 	}
 
-- 
1.8.3.2

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

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 7/8] drm/i915: Swap primary planes on gen2 for FBC
  2013-11-28 15:29 [PATCH 0/8] FBC support for gen2/3 ville.syrjala
                   ` (5 preceding siblings ...)
  2013-11-28 15:30 ` [PATCH 6/8] drm/i915: Rework the FBC interval/stall stuff a bit ville.syrjala
@ 2013-11-28 15:30 ` ville.syrjala
  2013-11-29 13:55   ` Chris Wilson
  2013-11-28 15:30 ` [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms ville.syrjala
  7 siblings, 1 reply; 31+ messages in thread
From: ville.syrjala @ 2013-11-28 15:30 UTC (permalink / raw)
  To: intel-gfx

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

Only plane A is FBC capable on gen2 (like gen3), but the panel fitter
is hooked up to pipe B, so we want to prefer pipe B + plane A.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c      | 14 +++++++++-----
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2715600..74918e7 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3139,10 +3139,10 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
  * Returns true when a page flip has completed.
  */
 static bool i8xx_handle_vblank(struct drm_device *dev,
-			       int pipe, u16 iir)
+			       int plane, int pipe, u32 iir)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(pipe);
+	u16 flip_pending = DISPLAY_PLANE_FLIP_PENDING(plane);
 
 	if (!drm_handle_vblank(dev, pipe))
 		return false;
@@ -3150,7 +3150,7 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
 	if ((iir & flip_pending) == 0)
 		return false;
 
-	intel_prepare_page_flip(dev, pipe);
+	intel_prepare_page_flip(dev, plane);
 
 	/* We detect FlipDone by looking for the change in PendingFlip from '1'
 	 * to '0' on the following vblank, i.e. IIR has the Pendingflip
@@ -3219,9 +3219,13 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 			notify_ring(dev, &dev_priv->ring[RCS]);
 
 		for_each_pipe(pipe) {
+			int plane = pipe;
+			if (IS_MOBILE(dev))
+				plane = !plane;
+
 			if (pipe_stats[pipe] & PIPE_VBLANK_INTERRUPT_STATUS &&
-			    i8xx_handle_vblank(dev, pipe, iir))
-				flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(pipe);
+			    i8xx_handle_vblank(dev, plane, pipe, iir))
+				flip_mask &= ~DISPLAY_PLANE_FLIP_PENDING(plane);
 
 			if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
 				i9xx_pipe_crc_irq_handler(dev, pipe);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1400df2..f70c392 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10149,7 +10149,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	/* Swap pipes & planes for FBC on pre-965 */
 	intel_crtc->pipe = pipe;
 	intel_crtc->plane = pipe;
-	if (IS_MOBILE(dev) && IS_GEN3(dev)) {
+	if (IS_MOBILE(dev) && INTEL_INFO(dev)->gen < 4) {
 		DRM_DEBUG_KMS("swapping pipes & planes for FBC\n");
 		intel_crtc->plane = !pipe;
 	}
-- 
1.8.3.2

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

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms
  2013-11-28 15:29 [PATCH 0/8] FBC support for gen2/3 ville.syrjala
                   ` (6 preceding siblings ...)
  2013-11-28 15:30 ` [PATCH 7/8] drm/i915: Swap primary planes on gen2 for FBC ville.syrjala
@ 2013-11-28 15:30 ` ville.syrjala
  2013-11-29 13:56   ` Chris Wilson
  2013-12-12 14:19   ` Imre Deak
  7 siblings, 2 replies; 31+ messages in thread
From: ville.syrjala @ 2013-11-28 15:30 UTC (permalink / raw)
  To: intel-gfx

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

All mobile gen2 and gen3 chipsets should have FBC1, and the code
should now handle them all. So just set has_fbc=true for all such
chipsets.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 0ec0fb3..923c8b6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -172,6 +172,7 @@ static const struct intel_device_info intel_i85x_info = {
 	.gen = 2, .is_i85x = 1, .is_mobile = 1, .num_pipes = 2,
 	.cursor_needs_physical = 1,
 	.has_overlay = 1, .overlay_needs_physical = 1,
+	.has_fbc = 1,
 	.ring_mask = RENDER_RING,
 };
 
@@ -191,6 +192,7 @@ static const struct intel_device_info intel_i915gm_info = {
 	.cursor_needs_physical = 1,
 	.has_overlay = 1, .overlay_needs_physical = 1,
 	.supports_tv = 1,
+	.has_fbc = 1,
 	.ring_mask = RENDER_RING,
 };
 static const struct intel_device_info intel_i945g_info = {
@@ -203,6 +205,7 @@ static const struct intel_device_info intel_i945gm_info = {
 	.has_hotplug = 1, .cursor_needs_physical = 1,
 	.has_overlay = 1, .overlay_needs_physical = 1,
 	.supports_tv = 1,
+	.has_fbc = 1,
 	.ring_mask = RENDER_RING,
 };
 
-- 
1.8.3.2

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

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/8] drm/i915: Rework the FBC interval/stall stuff a bit
  2013-11-28 15:30 ` [PATCH 6/8] drm/i915: Rework the FBC interval/stall stuff a bit ville.syrjala
@ 2013-11-29 13:53   ` Chris Wilson
  2013-12-12 14:04   ` Imre Deak
  1 sibling, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2013-11-29 13:53 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Nov 28, 2013 at 05:30:00PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Don't touch DPFC_RECOMP_CTL on FBC2, use RMW to update
> the FBC_CONTROL on FBC1 to make it easier for people to
> experiment with different numbers. Also fix the interval
> mask for FBC1.

I don't feel this is adequate to understand the changes in this patch.
In particular I see the non-i8xx paths being altered in a way that
doesn't seem to preserve the invariant register values.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 7/8] drm/i915: Swap primary planes on gen2 for FBC
  2013-11-28 15:30 ` [PATCH 7/8] drm/i915: Swap primary planes on gen2 for FBC ville.syrjala
@ 2013-11-29 13:55   ` Chris Wilson
  2013-12-03 21:35     ` Daniel Vetter
  0 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2013-11-29 13:55 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Nov 28, 2013 at 05:30:01PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Only plane A is FBC capable on gen2 (like gen3), but the panel fitter
> is hooked up to pipe B, so we want to prefer pipe B + plane A.

Ah, so that's the explanation!

Pretty please can I have this as a comment next to the pipe/plane switcheroo.
 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms
  2013-11-28 15:30 ` [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms ville.syrjala
@ 2013-11-29 13:56   ` Chris Wilson
  2013-11-29 14:10     ` Daniel Vetter
  2013-12-12 14:19   ` Imre Deak
  1 sibling, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2013-11-29 13:56 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Nov 28, 2013 at 05:30:02PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> All mobile gen2 and gen3 chipsets should have FBC1, and the code
> should now handle them all. So just set has_fbc=true for all such
> chipsets.

Just comment here that the default value for i915_enable_fbc is still
off for gen2, so for the user to actually enable FBC requires opting-in.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 4/8] drm/i915: Fix FBC1 plane checks for gen2
  2013-11-28 15:29 ` [PATCH 4/8] drm/i915: Fix FBC1 plane checks for gen2 ville.syrjala
@ 2013-11-29 13:57   ` Chris Wilson
  0 siblings, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2013-11-29 13:57 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Nov 28, 2013 at 05:29:58PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On gen2 and gen3 chipsets FBC is supported only on plane A. Fix (and
> simplify) the plane checks in intel_update_fbc() accordingly.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 5/8] drm/i915: Reorganize FBC function pointer initializaition
  2013-11-28 15:29 ` [PATCH 5/8] drm/i915: Reorganize FBC function pointer initializaition ville.syrjala
@ 2013-11-29 13:59   ` Chris Wilson
  0 siblings, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2013-11-29 13:59 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Nov 28, 2013 at 05:29:59PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Initialize the FBC vfuncs on gen2 and gen3 chipsets,
which are the same as currently used for Crestline (early mobile gen4
platform).

> Also make
> a clean split for gen7+ vs. gen5+ vfunc initialization.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviwed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/8] drm/i915: Fix bogus FBC1 defines
  2013-11-28 15:29 ` [PATCH 1/8] drm/i915: Fix bogus FBC1 defines ville.syrjala
@ 2013-11-29 13:59   ` Chris Wilson
  0 siblings, 0 replies; 31+ messages in thread
From: Chris Wilson @ 2013-11-29 13:59 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Nov 28, 2013 at 05:29:55PM +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>
Reviewed-by: Chris Wilons <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/8] drm/i915: FBC_CONTROL2 is gen4 only
  2013-11-28 15:29 ` [PATCH 3/8] drm/i915: FBC_CONTROL2 is gen4 only ville.syrjala
@ 2013-11-29 14:01   ` Chris Wilson
  2013-12-12 13:00     ` Imre Deak
  0 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2013-11-29 14:01 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Nov 28, 2013 at 05:29:57PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Gen2 and gen3 don't have the FBC_CONTROL2 register, so don't
> touch it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Hmm, another instance in i915_suspend.c
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms
  2013-11-29 13:56   ` Chris Wilson
@ 2013-11-29 14:10     ` Daniel Vetter
  2013-11-29 14:15       ` Chris Wilson
  0 siblings, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2013-11-29 14:10 UTC (permalink / raw)
  To: Chris Wilson, ville.syrjala, intel-gfx

On Fri, Nov 29, 2013 at 01:56:19PM +0000, Chris Wilson wrote:
> On Thu, Nov 28, 2013 at 05:30:02PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > All mobile gen2 and gen3 chipsets should have FBC1, and the code
> > should now handle them all. So just set has_fbc=true for all such
> > chipsets.
> 
> Just comment here that the default value for i915_enable_fbc is still
> off for gen2, so for the user to actually enable FBC requires opting-in.

Can we please dare and just enable it? Imo going through all this trouble
and not trying isn't really worth it ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms
  2013-11-29 14:10     ` Daniel Vetter
@ 2013-11-29 14:15       ` Chris Wilson
  2013-11-29 14:39         ` Ville Syrjälä
  0 siblings, 1 reply; 31+ messages in thread
From: Chris Wilson @ 2013-11-29 14:15 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Fri, Nov 29, 2013 at 03:10:17PM +0100, Daniel Vetter wrote:
> On Fri, Nov 29, 2013 at 01:56:19PM +0000, Chris Wilson wrote:
> > On Thu, Nov 28, 2013 at 05:30:02PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > All mobile gen2 and gen3 chipsets should have FBC1, and the code
> > > should now handle them all. So just set has_fbc=true for all such
> > > chipsets.
> > 
> > Just comment here that the default value for i915_enable_fbc is still
> > off for gen2, so for the user to actually enable FBC requires opting-in.
> 
> Can we please dare and just enable it? Imo going through all this trouble
> and not trying isn't really worth it ...

But as a separate patch with a host of tested-by and how much battery
life is improved... Ah, the joy of 10 year old batteries!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms
  2013-11-29 14:15       ` Chris Wilson
@ 2013-11-29 14:39         ` Ville Syrjälä
  0 siblings, 0 replies; 31+ messages in thread
From: Ville Syrjälä @ 2013-11-29 14:39 UTC (permalink / raw)
  To: Chris Wilson, Daniel Vetter, intel-gfx

On Fri, Nov 29, 2013 at 02:15:52PM +0000, Chris Wilson wrote:
> On Fri, Nov 29, 2013 at 03:10:17PM +0100, Daniel Vetter wrote:
> > On Fri, Nov 29, 2013 at 01:56:19PM +0000, Chris Wilson wrote:
> > > On Thu, Nov 28, 2013 at 05:30:02PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > All mobile gen2 and gen3 chipsets should have FBC1, and the code
> > > > should now handle them all. So just set has_fbc=true for all such
> > > > chipsets.
> > > 
> > > Just comment here that the default value for i915_enable_fbc is still
> > > off for gen2, so for the user to actually enable FBC requires opting-in.
> > 
> > Can we please dare and just enable it? Imo going through all this trouble
> > and not trying isn't really worth it ...
> 
> But as a separate patch with a host of tested-by and how much battery
> life is improved... Ah, the joy of 10 year old batteries!

Let's wait until I get all the fixes posted shall we :) There are still
some left.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 7/8] drm/i915: Swap primary planes on gen2 for FBC
  2013-11-29 13:55   ` Chris Wilson
@ 2013-12-03 21:35     ` Daniel Vetter
  0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2013-12-03 21:35 UTC (permalink / raw)
  To: Chris Wilson, ville.syrjala, intel-gfx

On Fri, Nov 29, 2013 at 01:55:04PM +0000, Chris Wilson wrote:
> On Thu, Nov 28, 2013 at 05:30:01PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Only plane A is FBC capable on gen2 (like gen3), but the panel fitter
> > is hooked up to pipe B, so we want to prefer pipe B + plane A.
> 
> Ah, so that's the explanation!
> 
> Pretty please can I have this as a comment next to the pipe/plane switcheroo.

Added.
>  
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Merged the patches from this series that scored an r-b from Chris, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/8] drm/i915: Gen2 FBC1 CFB pitch wants 32B units
  2013-11-28 15:29 ` [PATCH 2/8] drm/i915: Gen2 FBC1 CFB pitch wants 32B units ville.syrjala
@ 2013-12-12 12:54   ` Imre Deak
  0 siblings, 0 replies; 31+ messages in thread
From: Imre Deak @ 2013-12-12 12:54 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx


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

On Thu, 2013-11-28 at 17:29 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On gen2 the compressed frame buffer pitch is specified in 32B units
> rather than the 64B units used on gen3+.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 614549b..1dee34c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -103,8 +103,11 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc,
>  	if (fb->pitches[0] < cfb_pitch)
>  		cfb_pitch = fb->pitches[0];
>  
> -	/* FBC_CTL wants 64B units */
> -	cfb_pitch = (cfb_pitch / 64) - 1;
> +	/* FBC_CTL wants 32B or 64B units */
> +	if (IS_GEN2(dev))
> +		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 */


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

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

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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/8] drm/i915: FBC_CONTROL2 is gen4 only
  2013-11-29 14:01   ` Chris Wilson
@ 2013-12-12 13:00     ` Imre Deak
  2013-12-12 14:59       ` Daniel Vetter
  0 siblings, 1 reply; 31+ messages in thread
From: Imre Deak @ 2013-12-12 13:00 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx


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

On Fri, 2013-11-29 at 14:01 +0000, Chris Wilson wrote:
> On Thu, Nov 28, 2013 at 05:29:57PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Gen2 and gen3 don't have the FBC_CONTROL2 register, so don't
> > touch it.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Hmm, another instance in i915_suspend.c

Ville said he'll follow up with a separate fix for that, so:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> -Chris
> 


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

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

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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/8] drm/i915: Rework the FBC interval/stall stuff a bit
  2013-11-28 15:30 ` [PATCH 6/8] drm/i915: Rework the FBC interval/stall stuff a bit ville.syrjala
  2013-11-29 13:53   ` Chris Wilson
@ 2013-12-12 14:04   ` Imre Deak
  2013-12-12 15:03     ` Daniel Vetter
  1 sibling, 1 reply; 31+ messages in thread
From: Imre Deak @ 2013-12-12 14:04 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx


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

On Thu, 2013-11-28 at 17:30 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Don't touch DPFC_RECOMP_CTL on FBC2, use RMW to update
> the FBC_CONTROL on FBC1 to make it easier for people to
> experiment with different numbers. Also fix the interval
> mask for FBC1.

As I understood the reason for removing DPFC_RECOMP support is that we
need to understand better how it affects performance etc. before
enabling it. That's something for the future.

We could still zero that register in case BIOS leaves there something
else but that can be done as a follow-up too. So:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  4 +---
>  drivers/gpu/drm/i915/intel_pm.c | 46 +++++++++++++++--------------------------
>  2 files changed, 18 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c5cbbc2..20a9811 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -373,8 +373,7 @@ struct dpll;
>  struct drm_i915_display_funcs {
>  	bool (*fbc_enabled)(struct drm_device *dev);
>  	void (*enable_fbc)(struct drm_crtc *crtc,
> -			   struct drm_framebuffer *fb,
> -			   unsigned long interval);
> +			   struct drm_framebuffer *fb);
>  	void (*disable_fbc)(struct drm_device *dev);
>  	int (*get_display_clock_speed)(struct drm_device *dev);
>  	int (*get_fifo_size)(struct drm_device *dev, int plane);
> @@ -692,7 +691,6 @@ struct i915_fbc {
>  		struct delayed_work work;
>  		struct drm_crtc *crtc;
>  		struct drm_framebuffer *fb;
> -		int interval;
>  	} *fbc_work;
>  
>  	enum no_fbc_reason {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b296438..af2bc2b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -87,8 +87,7 @@ static void i8xx_disable_fbc(struct drm_device *dev)
>  }
>  
>  static void i8xx_enable_fbc(struct drm_crtc *crtc,
> -			    struct drm_framebuffer *fb,
> -			    unsigned long interval)
> +			    struct drm_framebuffer *fb)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -125,11 +124,12 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc,
>  	}
>  
>  	/* enable it... */
> -	fbc_ctl = FBC_CTL_EN | FBC_CTL_PERIODIC;
> +	fbc_ctl = I915_READ(FBC_CONTROL);
> +	fbc_ctl &= 0x3fff << FBC_CTL_INTERVAL_SHIFT;
> +	fbc_ctl |= FBC_CTL_EN | FBC_CTL_PERIODIC;
>  	if (IS_I945GM(dev))
>  		fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
>  	fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
> -	fbc_ctl |= (interval & 0x2fff) << FBC_CTL_INTERVAL_SHIFT;
>  	fbc_ctl |= obj->fence_reg;
>  	I915_WRITE(FBC_CONTROL, fbc_ctl);
>  
> @@ -145,8 +145,7 @@ static bool i8xx_fbc_enabled(struct drm_device *dev)
>  }
>  
>  static void g4x_enable_fbc(struct drm_crtc *crtc,
> -			   struct drm_framebuffer *fb,
> -			   unsigned long interval)
> +			   struct drm_framebuffer *fb)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -154,15 +153,10 @@ static void g4x_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 plane = intel_crtc->plane == 0 ? DPFC_CTL_PLANEA : DPFC_CTL_PLANEB;
> -	unsigned long stall_watermark = 200;
>  	u32 dpfc_ctl;
>  
>  	dpfc_ctl = plane | DPFC_SR_EN | DPFC_CTL_LIMIT_1X;
>  	dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
> -
> -	I915_WRITE(DPFC_RECOMP_CTL, DPFC_RECOMP_STALL_EN |
> -		   (stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) |
> -		   (interval << DPFC_RECOMP_TIMER_COUNT_SHIFT));
>  	I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
>  
>  	/* enable it... */
> @@ -194,8 +188,7 @@ static bool g4x_fbc_enabled(struct drm_device *dev)
>  }
>  
>  static void ironlake_enable_fbc(struct drm_crtc *crtc,
> -				struct drm_framebuffer *fb,
> -				unsigned long interval)
> +				struct drm_framebuffer *fb)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -203,7 +196,6 @@ static void ironlake_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 plane = intel_crtc->plane == 0 ? DPFC_CTL_PLANEA : DPFC_CTL_PLANEB;
> -	unsigned long stall_watermark = 200;
>  	u32 dpfc_ctl;
>  
>  	dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
> @@ -212,10 +204,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_RECOMP_CTL, DPFC_RECOMP_STALL_EN |
> -		   (stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) |
> -		   (interval << DPFC_RECOMP_TIMER_COUNT_SHIFT));
>  	I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->y);
>  	/* enable it... */
>  	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
> @@ -252,8 +240,7 @@ static bool ironlake_fbc_enabled(struct drm_device *dev)
>  }
>  
>  static void gen7_enable_fbc(struct drm_crtc *crtc,
> -			    struct drm_framebuffer *fb,
> -			    unsigned long interval)
> +			    struct drm_framebuffer *fb)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -292,12 +279,11 @@ bool intel_fbc_enabled(struct drm_device *dev)
>  }
>  
>  static void intel_setup_fbc(struct drm_crtc *crtc,
> -			    struct drm_framebuffer *fb,
> -			    unsigned long interval)
> +			    struct drm_framebuffer *fb)
>  {
>  	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
>  
> -	dev_priv->display.enable_fbc(crtc, fb, interval);
> +	dev_priv->display.enable_fbc(crtc, fb);
>  
>  	dev_priv->fbc.plane = to_intel_crtc(crtc)->plane;
>  	drm_framebuffer_reference(fb);
> @@ -324,7 +310,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
>  		 * the prior work.
>  		 */
>  		if (work->crtc->fb == work->fb)
> -			intel_setup_fbc(work->crtc, work->fb, work->interval);
> +			intel_setup_fbc(work->crtc, work->fb);
>  		dev_priv->fbc.fbc_work = NULL;
>  	}
>  	mutex_unlock(&dev->struct_mutex);
> @@ -361,7 +347,7 @@ static void intel_cancel_fbc_work(struct drm_i915_private *dev_priv)
>  	dev_priv->fbc.fbc_work = NULL;
>  }
>  
> -static void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
> +static void intel_enable_fbc(struct drm_crtc *crtc)
>  {
>  	struct intel_fbc_work *work;
>  	struct drm_device *dev = crtc->dev;
> @@ -375,13 +361,12 @@ static void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>  	work = kzalloc(sizeof(*work), GFP_KERNEL);
>  	if (work == NULL) {
>  		DRM_ERROR("Failed to allocate FBC work structure\n");
> -		intel_setup_fbc(crtc, crtc->fb, interval);
> +		intel_setup_fbc(crtc, crtc->fb);
>  		return;
>  	}
>  
>  	work->crtc = crtc;
>  	work->fb = crtc->fb;
> -	work->interval = interval;
>  	INIT_DELAYED_WORK(&work->work, intel_fbc_work_fn);
>  
>  	drm_framebuffer_reference(work->fb);
> @@ -448,7 +433,7 @@ void intel_fbc_nuke(struct drm_i915_gem_object *obj)
>  	 * Must wait until the next vblank before re-enabling
>  	 * otherwise the nuking won't actually happen.
>  	 */
> -	intel_enable_fbc(crtc, 500);
> +	intel_enable_fbc(crtc);
>  }
>  
>  static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
> @@ -632,7 +617,7 @@ void intel_update_fbc(struct drm_device *dev)
>  		intel_disable_fbc(dev);
>  	}
>  
> -	intel_enable_fbc(crtc, 500);
> +	intel_enable_fbc(crtc);
>  	dev_priv->fbc.no_fbc_reason = FBC_OK;
>  	return;
>  
> @@ -5993,6 +5978,9 @@ void intel_init_pm(struct drm_device *dev)
>  			dev_priv->display.fbc_enabled = i8xx_fbc_enabled;
>  			dev_priv->display.enable_fbc = i8xx_enable_fbc;
>  			dev_priv->display.disable_fbc = i8xx_disable_fbc;
> +
> +			/* This value was pulled out of someone's hat */
> +			I915_WRITE(FBC_CONTROL, 500 << FBC_CTL_INTERVAL_SHIFT);
>  		}
>  	}
>  


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

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

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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms
  2013-11-28 15:30 ` [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms ville.syrjala
  2013-11-29 13:56   ` Chris Wilson
@ 2013-12-12 14:19   ` Imre Deak
  2013-12-12 14:32     ` Ville Syrjälä
  1 sibling, 1 reply; 31+ messages in thread
From: Imre Deak @ 2013-12-12 14:19 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx


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

On Thu, 2013-11-28 at 17:30 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> All mobile gen2 and gen3 chipsets should have FBC1, and the code
> should now handle them all. So just set has_fbc=true for all such
> chipsets.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Based on the above we could also enable fbc for i830. In any case:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 0ec0fb3..923c8b6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -172,6 +172,7 @@ static const struct intel_device_info intel_i85x_info = {
>  	.gen = 2, .is_i85x = 1, .is_mobile = 1, .num_pipes = 2,
>  	.cursor_needs_physical = 1,
>  	.has_overlay = 1, .overlay_needs_physical = 1,
> +	.has_fbc = 1,
>  	.ring_mask = RENDER_RING,
>  };
>  
> @@ -191,6 +192,7 @@ static const struct intel_device_info intel_i915gm_info = {
>  	.cursor_needs_physical = 1,
>  	.has_overlay = 1, .overlay_needs_physical = 1,
>  	.supports_tv = 1,
> +	.has_fbc = 1,
>  	.ring_mask = RENDER_RING,
>  };
>  static const struct intel_device_info intel_i945g_info = {
> @@ -203,6 +205,7 @@ static const struct intel_device_info intel_i945gm_info = {
>  	.has_hotplug = 1, .cursor_needs_physical = 1,
>  	.has_overlay = 1, .overlay_needs_physical = 1,
>  	.supports_tv = 1,
> +	.has_fbc = 1,
>  	.ring_mask = RENDER_RING,
>  };
>  


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

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

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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms
  2013-12-12 14:19   ` Imre Deak
@ 2013-12-12 14:32     ` Ville Syrjälä
  2013-12-12 14:38       ` Daniel Vetter
  0 siblings, 1 reply; 31+ messages in thread
From: Ville Syrjälä @ 2013-12-12 14:32 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Thu, Dec 12, 2013 at 04:19:34PM +0200, Imre Deak wrote:
> On Thu, 2013-11-28 at 17:30 +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > All mobile gen2 and gen3 chipsets should have FBC1, and the code
> > should now handle them all. So just set has_fbc=true for all such
> > chipsets.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Based on the above we could also enable fbc for i830. In any case:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

According to the spec there's no FBC on 830, so I guess my commit
message was a tad incorrect. Some part of the spec hints that FBC
might be avilable on 865 too, but in another part only 85x is
mentioned.

> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 0ec0fb3..923c8b6 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -172,6 +172,7 @@ static const struct intel_device_info intel_i85x_info = {
> >  	.gen = 2, .is_i85x = 1, .is_mobile = 1, .num_pipes = 2,
> >  	.cursor_needs_physical = 1,
> >  	.has_overlay = 1, .overlay_needs_physical = 1,
> > +	.has_fbc = 1,
> >  	.ring_mask = RENDER_RING,
> >  };
> >  
> > @@ -191,6 +192,7 @@ static const struct intel_device_info intel_i915gm_info = {
> >  	.cursor_needs_physical = 1,
> >  	.has_overlay = 1, .overlay_needs_physical = 1,
> >  	.supports_tv = 1,
> > +	.has_fbc = 1,
> >  	.ring_mask = RENDER_RING,
> >  };
> >  static const struct intel_device_info intel_i945g_info = {
> > @@ -203,6 +205,7 @@ static const struct intel_device_info intel_i945gm_info = {
> >  	.has_hotplug = 1, .cursor_needs_physical = 1,
> >  	.has_overlay = 1, .overlay_needs_physical = 1,
> >  	.supports_tv = 1,
> > +	.has_fbc = 1,
> >  	.ring_mask = RENDER_RING,
> >  };
> >  
> 



-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms
  2013-12-12 14:32     ` Ville Syrjälä
@ 2013-12-12 14:38       ` Daniel Vetter
  2013-12-12 15:04         ` Daniel Vetter
  0 siblings, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2013-12-12 14:38 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Thu, Dec 12, 2013 at 3:32 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Dec 12, 2013 at 04:19:34PM +0200, Imre Deak wrote:
>> On Thu, 2013-11-28 at 17:30 +0200, ville.syrjala@linux.intel.com wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > All mobile gen2 and gen3 chipsets should have FBC1, and the code
>> > should now handle them all. So just set has_fbc=true for all such
>> > chipsets.
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Based on the above we could also enable fbc for i830. In any case:
>> Reviewed-by: Imre Deak <imre.deak@intel.com>
>
> According to the spec there's no FBC on 830, so I guess my commit
> message was a tad incorrect. Some part of the spec hints that FBC
> might be avilable on 865 too, but in another part only 85x is
> mentioned.

Iirc fbc was mobile-only until ilk happened. So by that pattern I
wouldn't expect 865 to have it, since it's a desktop part.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/8] drm/i915: FBC_CONTROL2 is gen4 only
  2013-12-12 13:00     ` Imre Deak
@ 2013-12-12 14:59       ` Daniel Vetter
  0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2013-12-12 14:59 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Thu, Dec 12, 2013 at 03:00:42PM +0200, Imre Deak wrote:
> On Fri, 2013-11-29 at 14:01 +0000, Chris Wilson wrote:
> > On Thu, Nov 28, 2013 at 05:29:57PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Gen2 and gen3 don't have the FBC_CONTROL2 register, so don't
> > > touch it.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Hmm, another instance in i915_suspend.c
> 
> Ville said he'll follow up with a separate fix for that, so:

Imo the right fix for that code is to disable it for modeset drivers and
make sure we restore everything in the normal modeset paths. There's very
little gunk left in there that we actually need, so every time we touch
this we should make some forward progress to it's ultimate demise.
-Daniel

> Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> > -Chris
> > 
> 



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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/8] drm/i915: Rework the FBC interval/stall stuff a bit
  2013-12-12 14:04   ` Imre Deak
@ 2013-12-12 15:03     ` Daniel Vetter
  2013-12-12 15:27       ` [PATCH v2 " ville.syrjala
  0 siblings, 1 reply; 31+ messages in thread
From: Daniel Vetter @ 2013-12-12 15:03 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Thu, Dec 12, 2013 at 04:04:49PM +0200, Imre Deak wrote:
> On Thu, 2013-11-28 at 17:30 +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Don't touch DPFC_RECOMP_CTL on FBC2, use RMW to update
> > the FBC_CONTROL on FBC1 to make it easier for people to
> > experiment with different numbers. Also fix the interval
> > mask for FBC1.
> 
> As I understood the reason for removing DPFC_RECOMP support is that we
> need to understand better how it affects performance etc. before
> enabling it. That's something for the future.
> 
> We could still zero that register in case BIOS leaves there something
> else but that can be done as a follow-up too. So:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

This patch here doesn't really apply on dinq. Ville, can you pls resend?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms
  2013-12-12 14:38       ` Daniel Vetter
@ 2013-12-12 15:04         ` Daniel Vetter
  0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2013-12-12 15:04 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Thu, Dec 12, 2013 at 03:38:38PM +0100, Daniel Vetter wrote:
> On Thu, Dec 12, 2013 at 3:32 PM, Ville Syrjälä
> <ville.syrjala@linux.intel.com> wrote:
> > On Thu, Dec 12, 2013 at 04:19:34PM +0200, Imre Deak wrote:
> >> On Thu, 2013-11-28 at 17:30 +0200, ville.syrjala@linux.intel.com wrote:
> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >
> >> > All mobile gen2 and gen3 chipsets should have FBC1, and the code
> >> > should now handle them all. So just set has_fbc=true for all such
> >> > chipsets.
> >> >
> >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> Based on the above we could also enable fbc for i830. In any case:
> >> Reviewed-by: Imre Deak <imre.deak@intel.com>
> >
> > According to the spec there's no FBC on 830, so I guess my commit
> > message was a tad incorrect. Some part of the spec hints that FBC
> > might be avilable on 865 too, but in another part only 85x is
> > mentioned.
> 
> Iirc fbc was mobile-only until ilk happened. So by that pattern I
> wouldn't expect 865 to have it, since it's a desktop part.

Ok I've merged 3 more patches from this series, but the last one kinda
refused to apply on dinq. Can you please rebase?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v2 6/8] drm/i915: Rework the FBC interval/stall stuff a bit
  2013-12-12 15:03     ` Daniel Vetter
@ 2013-12-12 15:27       ` ville.syrjala
  2013-12-12 16:45         ` Daniel Vetter
  0 siblings, 1 reply; 31+ messages in thread
From: ville.syrjala @ 2013-12-12 15:27 UTC (permalink / raw)
  To: intel-gfx

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

Don't touch DPFC_RECOMP_CTL on FBC2, use RMW to update
the FBC_CONTROL on FBC1 to make it easier for people to
experiment with different numbers. Also fix the interval
mask for FBC1.

v2: Rebased

Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  3 +--
 drivers/gpu/drm/i915/intel_pm.c | 34 ++++++++++++++--------------------
 2 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 46072b9..fca2eb6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -372,7 +372,7 @@ struct dpll;
 
 struct drm_i915_display_funcs {
 	bool (*fbc_enabled)(struct drm_device *dev);
-	void (*enable_fbc)(struct drm_crtc *crtc, unsigned long interval);
+	void (*enable_fbc)(struct drm_crtc *crtc);
 	void (*disable_fbc)(struct drm_device *dev);
 	int (*get_display_clock_speed)(struct drm_device *dev);
 	int (*get_fifo_size)(struct drm_device *dev, int plane);
@@ -695,7 +695,6 @@ struct i915_fbc {
 		struct delayed_work work;
 		struct drm_crtc *crtc;
 		struct drm_framebuffer *fb;
-		int interval;
 	} *fbc_work;
 
 	enum no_fbc_reason {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ac9dd46..013c522 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -88,7 +88,7 @@ static void i8xx_disable_fbc(struct drm_device *dev)
 	DRM_DEBUG_KMS("disabled FBC\n");
 }
 
-static void i8xx_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
+static void i8xx_enable_fbc(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -126,11 +126,12 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	}
 
 	/* enable it... */
-	fbc_ctl = FBC_CTL_EN | FBC_CTL_PERIODIC;
+	fbc_ctl = I915_READ(FBC_CONTROL);
+	fbc_ctl &= 0x3fff << FBC_CTL_INTERVAL_SHIFT;
+	fbc_ctl |= FBC_CTL_EN | FBC_CTL_PERIODIC;
 	if (IS_I945GM(dev))
 		fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
 	fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
-	fbc_ctl |= (interval & 0x2fff) << FBC_CTL_INTERVAL_SHIFT;
 	fbc_ctl |= obj->fence_reg;
 	I915_WRITE(FBC_CONTROL, fbc_ctl);
 
@@ -145,7 +146,7 @@ static bool i8xx_fbc_enabled(struct drm_device *dev)
 	return I915_READ(FBC_CONTROL) & FBC_CTL_EN;
 }
 
-static void g4x_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
+static void g4x_enable_fbc(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -154,16 +155,12 @@ static void g4x_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	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;
-	unsigned long stall_watermark = 200;
 	u32 dpfc_ctl;
 
 	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_RECOMP_CTL, DPFC_RECOMP_STALL_EN |
-		   (stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) |
-		   (interval << DPFC_RECOMP_TIMER_COUNT_SHIFT));
 	I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
 
 	/* enable it... */
@@ -219,7 +216,7 @@ static void sandybridge_blit_fbc_update(struct drm_device *dev)
 	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_MEDIA);
 }
 
-static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
+static void ironlake_enable_fbc(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -228,7 +225,6 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	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;
-	unsigned long stall_watermark = 200;
 	u32 dpfc_ctl;
 
 	dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
@@ -241,9 +237,6 @@ static void ironlake_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		dpfc_ctl |= obj->fence_reg;
 	I915_WRITE(ILK_DPFC_CHICKEN, DPFC_HT_MODIFY);
 
-	I915_WRITE(ILK_DPFC_RECOMP_CTL, DPFC_RECOMP_STALL_EN |
-		   (stall_watermark << DPFC_RECOMP_STALL_WM_SHIFT) |
-		   (interval << DPFC_RECOMP_TIMER_COUNT_SHIFT));
 	I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->y);
 	I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
 	/* enable it... */
@@ -281,7 +274,7 @@ static bool ironlake_fbc_enabled(struct drm_device *dev)
 	return I915_READ(ILK_DPFC_CONTROL) & DPFC_CTL_EN;
 }
 
-static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
+static void gen7_enable_fbc(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -338,8 +331,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
 		 * the prior work.
 		 */
 		if (work->crtc->fb == work->fb) {
-			dev_priv->display.enable_fbc(work->crtc,
-						     work->interval);
+			dev_priv->display.enable_fbc(work->crtc);
 
 			dev_priv->fbc.plane = to_intel_crtc(work->crtc)->plane;
 			dev_priv->fbc.fb_id = work->crtc->fb->base.id;
@@ -376,7 +368,7 @@ static void intel_cancel_fbc_work(struct drm_i915_private *dev_priv)
 	dev_priv->fbc.fbc_work = NULL;
 }
 
-static void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
+static void intel_enable_fbc(struct drm_crtc *crtc)
 {
 	struct intel_fbc_work *work;
 	struct drm_device *dev = crtc->dev;
@@ -390,13 +382,12 @@ static void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	work = kzalloc(sizeof(*work), GFP_KERNEL);
 	if (work == NULL) {
 		DRM_ERROR("Failed to allocate FBC work structure\n");
-		dev_priv->display.enable_fbc(crtc, interval);
+		dev_priv->display.enable_fbc(crtc);
 		return;
 	}
 
 	work->crtc = crtc;
 	work->fb = crtc->fb;
-	work->interval = interval;
 	INIT_DELAYED_WORK(&work->work, intel_fbc_work_fn);
 
 	dev_priv->fbc.fbc_work = work;
@@ -611,7 +602,7 @@ void intel_update_fbc(struct drm_device *dev)
 		intel_disable_fbc(dev);
 	}
 
-	intel_enable_fbc(crtc, 500);
+	intel_enable_fbc(crtc);
 	dev_priv->fbc.no_fbc_reason = FBC_OK;
 	return;
 
@@ -6074,6 +6065,9 @@ void intel_init_pm(struct drm_device *dev)
 			dev_priv->display.fbc_enabled = i8xx_fbc_enabled;
 			dev_priv->display.enable_fbc = i8xx_enable_fbc;
 			dev_priv->display.disable_fbc = i8xx_disable_fbc;
+
+			/* This value was pulled out of someone's hat */
+			I915_WRITE(FBC_CONTROL, 500 << FBC_CTL_INTERVAL_SHIFT);
 		}
 	}
 
-- 
1.8.3.2

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

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH v2 6/8] drm/i915: Rework the FBC interval/stall stuff a bit
  2013-12-12 15:27       ` [PATCH v2 " ville.syrjala
@ 2013-12-12 16:45         ` Daniel Vetter
  0 siblings, 0 replies; 31+ messages in thread
From: Daniel Vetter @ 2013-12-12 16:45 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Thu, Dec 12, 2013 at 05:27:40PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Don't touch DPFC_RECOMP_CTL on FBC2, use RMW to update
> the FBC_CONTROL on FBC1 to make it easier for people to
> experiment with different numbers. Also fix the interval
> mask for FBC1.
> 
> v2: Rebased
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2013-12-12 16:44 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-28 15:29 [PATCH 0/8] FBC support for gen2/3 ville.syrjala
2013-11-28 15:29 ` [PATCH 1/8] drm/i915: Fix bogus FBC1 defines ville.syrjala
2013-11-29 13:59   ` Chris Wilson
2013-11-28 15:29 ` [PATCH 2/8] drm/i915: Gen2 FBC1 CFB pitch wants 32B units ville.syrjala
2013-12-12 12:54   ` Imre Deak
2013-11-28 15:29 ` [PATCH 3/8] drm/i915: FBC_CONTROL2 is gen4 only ville.syrjala
2013-11-29 14:01   ` Chris Wilson
2013-12-12 13:00     ` Imre Deak
2013-12-12 14:59       ` Daniel Vetter
2013-11-28 15:29 ` [PATCH 4/8] drm/i915: Fix FBC1 plane checks for gen2 ville.syrjala
2013-11-29 13:57   ` Chris Wilson
2013-11-28 15:29 ` [PATCH 5/8] drm/i915: Reorganize FBC function pointer initializaition ville.syrjala
2013-11-29 13:59   ` Chris Wilson
2013-11-28 15:30 ` [PATCH 6/8] drm/i915: Rework the FBC interval/stall stuff a bit ville.syrjala
2013-11-29 13:53   ` Chris Wilson
2013-12-12 14:04   ` Imre Deak
2013-12-12 15:03     ` Daniel Vetter
2013-12-12 15:27       ` [PATCH v2 " ville.syrjala
2013-12-12 16:45         ` Daniel Vetter
2013-11-28 15:30 ` [PATCH 7/8] drm/i915: Swap primary planes on gen2 for FBC ville.syrjala
2013-11-29 13:55   ` Chris Wilson
2013-12-03 21:35     ` Daniel Vetter
2013-11-28 15:30 ` [PATCH 8/8] drm/i915: Enable FBC for all mobile gen2 and gen3 platforms ville.syrjala
2013-11-29 13:56   ` Chris Wilson
2013-11-29 14:10     ` Daniel Vetter
2013-11-29 14:15       ` Chris Wilson
2013-11-29 14:39         ` Ville Syrjälä
2013-12-12 14:19   ` Imre Deak
2013-12-12 14:32     ` Ville Syrjälä
2013-12-12 14:38       ` Daniel Vetter
2013-12-12 15:04         ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.