[PATCH 0/6] Enabling Frame Buffer Compression (FBC) for IVB and HSW

[PATCH 0/6] Enabling Frame Buffer Compression (FBC) for IVB and HSW

[Rodrigo Vivi]

Hi all, this series enable Frame Buffer Compression at IVB and HSW.

I decided to create a new function gen7_enable_fbc to avoid getting old
function messed with many IS_IVYBRIDGE and IS_HASWELL checks.

Also I decided to split the needed workarounds in separated patches to be
easy to revert at any time if needed.

On my local tests using a HSW ULT machine I could save around 0.1/0.2W.
Up to 0.3W. And apparently stable enough.

Zhang, Ouping <ouping.zhang@intel.com> confirmed "FBC saved more 0.3w on idle, 0.2w on video and 0.2w on game workload" on HSW.

We decided to leave it disabled by default on IVB for now before a careful
validation in order to avoid issues we faced with SNB previously. On HSW it is
enabled by default.

Chris Wilson required more interection with user space and Daniel Vetter has
already giving some suggestions. This work still need to be done, but first
lets enable fbc support using the current structure.

As always, reviews, comments, bikeshedings, tests, etc are welcome.

Thanks in advance,
Rodrigo.

Rodrigo Vivi (6):
  drm/i915: Enable FBC at Ivybridge.
  drm/i915: IVB FBC WaFbcAsynchFlipDisableFbcQueue
  drm/i915: IVB FBC WaFbcDisableDpfcClockGating
  drm/i915: Enable FBC at Haswell.
  drm/i915: HSW FBC WaFbcAsynchFlipDisableFbcQueue
  drm/i915: HSW FBC WaFbcDisableDpfcClockGating

 drivers/gpu/drm/i915/i915_drv.c |  2 ++
 drivers/gpu/drm/i915/i915_reg.h | 16 +++++++++++
 drivers/gpu/drm/i915/intel_pm.c | 61 +++++++++++++++++++++++++++++++++++++++--
 3 files changed, 76 insertions(+), 3 deletions(-)

-- 
1.8.1.4

[PATCH 1/6] drm/i915: Enable FBC at Ivybridge.

[Rodrigo Vivi]

This patch introduce Frame Buffer Compression (FBC) support for IVB,
without enabling it by default.
It adds a new function gen7_enable_fbc to avoid getting
ironlake_enable_fbc messed with many IS_IVYBRIDGE checks.

v2: Fixes from Ville.
     	*  Fix Plane. FBC is tied to primary plane A in HSW
    	*  Fix DPFC initial write to avoid let trash on the register.
v3: Checking for bad plane on intel_update_fbc() as Chris suggested.
v4: Ville pointed out that according to BSpec FBC_CTL bits 0:3 must be 0.
v5: Up to v4 this work was entirely focused on Haswell. However Ville
    noticed I could reuse the FBC work done for HSW and get FBC for free
    at Ivybridge. So it makes more sense enable FBC for IVB first.
    FBC for HSW comming on next patches. We are just not enabling it by
    default on IVB.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_drv.c |  1 +
 drivers/gpu/drm/i915/i915_reg.h |  6 ++++++
 drivers/gpu/drm/i915/intel_pm.c | 35 +++++++++++++++++++++++++++++++++--
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9ebe895..a073b4c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -280,6 +280,7 @@ static const struct intel_device_info intel_ivybridge_m_info = {
 	GEN7_FEATURES,
 	.is_ivybridge = 1,
 	.is_mobile = 1,
+	.has_fbc = 1,
 };
 
 static const struct intel_device_info intel_ivybridge_q_info = {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 077d40f..f64f118 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -809,7 +809,9 @@
 #define   DPFC_CTL_EN		(1<<31)
 #define   DPFC_CTL_PLANEA	(0<<30)
 #define   DPFC_CTL_PLANEB	(1<<30)
+#define   IVB_DPFC_CTL_PLANE_SHIFT	(29)
 #define   DPFC_CTL_FENCE_EN	(1<<29)
+#define   IVB_DPFC_CTL_FENCE_EN	(1<<28)
 #define   DPFC_CTL_PERSISTENT_MODE	(1<<25)
 #define   DPFC_SR_EN		(1<<10)
 #define   DPFC_CTL_LIMIT_1X	(0<<6)
@@ -857,6 +859,10 @@
 #define   SNB_CPU_FENCE_ENABLE	(1<<29)
 #define DPFC_CPU_FENCE_OFFSET	0x100104
 
+/* Framebuffer compression for Ivybridge */
+#define IVB_FBC_RT_BASE			0x7020
+#define   IVB_FBC_RT_BASE_ADDR_SHIFT	12
+
 
 /*
  * GPIO regs
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f747cb0..86a941a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -253,6 +253,32 @@ 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)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_framebuffer *fb = crtc->fb;
+	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+	struct drm_i915_gem_object *obj = intel_fb->obj;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	I915_WRITE(IVB_FBC_RT_BASE,
+		   obj->gtt_offset << IVB_FBC_RT_BASE_ADDR_SHIFT |
+		   ILK_FBC_RT_VALID);
+
+	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
+		   IVB_DPFC_CTL_FENCE_EN |
+		   intel_crtc->plane << IVB_DPFC_CTL_PLANE_SHIFT);
+
+	I915_WRITE(SNB_DPFC_CTL_SA,
+		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
+	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
+
+	sandybridge_blit_fbc_update(dev);
+
+	DRM_DEBUG_KMS("enabled fbc on plane %d\n", intel_crtc->plane);
+}
+
 bool intel_fbc_enabled(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -439,7 +465,7 @@ void intel_update_fbc(struct drm_device *dev)
 	if (enable_fbc < 0) {
 		DRM_DEBUG_KMS("fbc set to per-chip default\n");
 		enable_fbc = 1;
-		if (INTEL_INFO(dev)->gen <= 6)
+		if (INTEL_INFO(dev)->gen <= 7)
 			enable_fbc = 0;
 	}
 	if (!enable_fbc) {
@@ -4180,7 +4206,12 @@ void intel_init_pm(struct drm_device *dev)
 	if (I915_HAS_FBC(dev)) {
 		if (HAS_PCH_SPLIT(dev)) {
 			dev_priv->display.fbc_enabled = ironlake_fbc_enabled;
-			dev_priv->display.enable_fbc = ironlake_enable_fbc;
+			if (IS_IVYBRIDGE(dev))
+				dev_priv->display.enable_fbc =
+					gen7_enable_fbc;
+			else
+				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;
-- 
1.8.1.4

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

Re: [PATCH 1/6] drm/i915: Enable FBC at Ivybridge.

[Matt Turner]

On Tue, Apr 23, 2013 at 10:52 AM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> This patch introduce Frame Buffer Compression (FBC) support for IVB,
> without enabling it by default.

The summary is kind of confusing, since FBC isn't being enabled by default.

Maybe change it to "drm/i915: Add support for FBC on Ivybridge."?

Re: [PATCH 1/6] drm/i915: Enable FBC at Ivybridge.

[Rodrigo Vivi]

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

Thanks Matt.

Reviewers and Daniel, please consider "[PATCH] drm/i915: Add support for
FBC on Ivybridge"
instead of this one.



On Tue, Apr 23, 2013 at 4:04 PM, Matt Turner <mattst88@gmail.com> wrote:

> On Tue, Apr 23, 2013 at 10:52 AM, Rodrigo Vivi <rodrigo.vivi@gmail.com>
> wrote:
> > This patch introduce Frame Buffer Compression (FBC) support for IVB,
> > without enabling it by default.
>
> The summary is kind of confusing, since FBC isn't being enabled by default.
>
> Maybe change it to "drm/i915: Add support for FBC on Ivybridge."?
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

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

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

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

[PATCH] drm/i915: Add support for FBC on Ivybridge.

[Rodrigo Vivi]

This patch introduce Frame Buffer Compression (FBC) support for IVB,
without enabling it by default.
It adds a new function gen7_enable_fbc to avoid getting
ironlake_enable_fbc messed with many IS_IVYBRIDGE checks.

v2: Fixes from Ville.
     	*  Fix Plane. FBC is tied to primary plane A in HSW
    	*  Fix DPFC initial write to avoid let trash on the register.
v3: Checking for bad plane on intel_update_fbc() as Chris suggested.
v4: Ville pointed out that according to BSpec FBC_CTL bits 0:3 must be 0.
v5: Up to v4 this work was entirely focused on Haswell. However Ville
    noticed I could reuse the FBC work done for HSW and get FBC for free
    at Ivybridge. So it makes more sense enable FBC for IVB first.
    FBC for HSW comming on next patches. We are just not enabling it by
    default on IVB.
v6: Fix confused commit name (by Matt Turner).

Cc: Matt Turner <mattst88@gmail.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_drv.c |  1 +
 drivers/gpu/drm/i915/i915_reg.h |  6 ++++++
 drivers/gpu/drm/i915/intel_pm.c | 35 +++++++++++++++++++++++++++++++++--
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9ebe895..a073b4c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -280,6 +280,7 @@ static const struct intel_device_info intel_ivybridge_m_info = {
 	GEN7_FEATURES,
 	.is_ivybridge = 1,
 	.is_mobile = 1,
+	.has_fbc = 1,
 };
 
 static const struct intel_device_info intel_ivybridge_q_info = {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 077d40f..f64f118 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -809,7 +809,9 @@
 #define   DPFC_CTL_EN		(1<<31)
 #define   DPFC_CTL_PLANEA	(0<<30)
 #define   DPFC_CTL_PLANEB	(1<<30)
+#define   IVB_DPFC_CTL_PLANE_SHIFT	(29)
 #define   DPFC_CTL_FENCE_EN	(1<<29)
+#define   IVB_DPFC_CTL_FENCE_EN	(1<<28)
 #define   DPFC_CTL_PERSISTENT_MODE	(1<<25)
 #define   DPFC_SR_EN		(1<<10)
 #define   DPFC_CTL_LIMIT_1X	(0<<6)
@@ -857,6 +859,10 @@
 #define   SNB_CPU_FENCE_ENABLE	(1<<29)
 #define DPFC_CPU_FENCE_OFFSET	0x100104
 
+/* Framebuffer compression for Ivybridge */
+#define IVB_FBC_RT_BASE			0x7020
+#define   IVB_FBC_RT_BASE_ADDR_SHIFT	12
+
 
 /*
  * GPIO regs
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f747cb0..86a941a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -253,6 +253,32 @@ 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)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_framebuffer *fb = crtc->fb;
+	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+	struct drm_i915_gem_object *obj = intel_fb->obj;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	I915_WRITE(IVB_FBC_RT_BASE,
+		   obj->gtt_offset << IVB_FBC_RT_BASE_ADDR_SHIFT |
+		   ILK_FBC_RT_VALID);
+
+	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
+		   IVB_DPFC_CTL_FENCE_EN |
+		   intel_crtc->plane << IVB_DPFC_CTL_PLANE_SHIFT);
+
+	I915_WRITE(SNB_DPFC_CTL_SA,
+		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
+	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
+
+	sandybridge_blit_fbc_update(dev);
+
+	DRM_DEBUG_KMS("enabled fbc on plane %d\n", intel_crtc->plane);
+}
+
 bool intel_fbc_enabled(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -439,7 +465,7 @@ void intel_update_fbc(struct drm_device *dev)
 	if (enable_fbc < 0) {
 		DRM_DEBUG_KMS("fbc set to per-chip default\n");
 		enable_fbc = 1;
-		if (INTEL_INFO(dev)->gen <= 6)
+		if (INTEL_INFO(dev)->gen <= 7)
 			enable_fbc = 0;
 	}
 	if (!enable_fbc) {
@@ -4180,7 +4206,12 @@ void intel_init_pm(struct drm_device *dev)
 	if (I915_HAS_FBC(dev)) {
 		if (HAS_PCH_SPLIT(dev)) {
 			dev_priv->display.fbc_enabled = ironlake_fbc_enabled;
-			dev_priv->display.enable_fbc = ironlake_enable_fbc;
+			if (IS_IVYBRIDGE(dev))
+				dev_priv->display.enable_fbc =
+					gen7_enable_fbc;
+			else
+				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;
-- 
1.8.1.4

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

Re: [PATCH] drm/i915: Add support for FBC on Ivybridge.

[Ville Syrjälä]

On Tue, Apr 23, 2013 at 05:55:04PM -0300, Rodrigo Vivi wrote:
> This patch introduce Frame Buffer Compression (FBC) support for IVB,
> without enabling it by default.
> It adds a new function gen7_enable_fbc to avoid getting
> ironlake_enable_fbc messed with many IS_IVYBRIDGE checks.
> 
> v2: Fixes from Ville.
>      	*  Fix Plane. FBC is tied to primary plane A in HSW
>     	*  Fix DPFC initial write to avoid let trash on the register.
> v3: Checking for bad plane on intel_update_fbc() as Chris suggested.
> v4: Ville pointed out that according to BSpec FBC_CTL bits 0:3 must be 0.
> v5: Up to v4 this work was entirely focused on Haswell. However Ville
>     noticed I could reuse the FBC work done for HSW and get FBC for free
>     at Ivybridge. So it makes more sense enable FBC for IVB first.
>     FBC for HSW comming on next patches. We are just not enabling it by
>     default on IVB.
> v6: Fix confused commit name (by Matt Turner).
> 
> Cc: Matt Turner <mattst88@gmail.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |  1 +
>  drivers/gpu/drm/i915/i915_reg.h |  6 ++++++
>  drivers/gpu/drm/i915/intel_pm.c | 35 +++++++++++++++++++++++++++++++++--
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9ebe895..a073b4c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -280,6 +280,7 @@ static const struct intel_device_info intel_ivybridge_m_info = {
>  	GEN7_FEATURES,
>  	.is_ivybridge = 1,
>  	.is_mobile = 1,
> +	.has_fbc = 1,
>  };
>  
>  static const struct intel_device_info intel_ivybridge_q_info = {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 077d40f..f64f118 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -809,7 +809,9 @@
>  #define   DPFC_CTL_EN		(1<<31)
>  #define   DPFC_CTL_PLANEA	(0<<30)
>  #define   DPFC_CTL_PLANEB	(1<<30)
> +#define   IVB_DPFC_CTL_PLANE_SHIFT	(29)
>  #define   DPFC_CTL_FENCE_EN	(1<<29)
> +#define   IVB_DPFC_CTL_FENCE_EN	(1<<28)
>  #define   DPFC_CTL_PERSISTENT_MODE	(1<<25)
>  #define   DPFC_SR_EN		(1<<10)
>  #define   DPFC_CTL_LIMIT_1X	(0<<6)
> @@ -857,6 +859,10 @@
>  #define   SNB_CPU_FENCE_ENABLE	(1<<29)
>  #define DPFC_CPU_FENCE_OFFSET	0x100104
>  
> +/* Framebuffer compression for Ivybridge */
> +#define IVB_FBC_RT_BASE			0x7020
> +#define   IVB_FBC_RT_BASE_ADDR_SHIFT	12
> +
>  
>  /*
>   * GPIO regs
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f747cb0..86a941a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -253,6 +253,32 @@ 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)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_framebuffer *fb = crtc->fb;
> +	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> +	struct drm_i915_gem_object *obj = intel_fb->obj;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +
> +	I915_WRITE(IVB_FBC_RT_BASE,
> +		   obj->gtt_offset << IVB_FBC_RT_BASE_ADDR_SHIFT |

gtt_offset is a page aligned byte offset, so the shift shouldn't be
here.

Also I'm thinking we should probably set the front buffer target bit,
since we're not using GFDT (yet at least) and our scanout buffers aren't
cached in LLC. I'm not sure what will happen if we leave the bit unset
and don't issue GFDT flushes.

> +		   ILK_FBC_RT_VALID);

BTW Bspec seems to tell me that we shouldn't even enable this render
tracking stuff, and instead we should using LRIs to nuke the FBC state
after flushing.

It's also saying something that we shouldn't write this more than once
per context. That doesn't sound quite right. What if we flip to another
buffer? Oh and what happens when we switch to another context? Is it
going to overwrite this register with a stale value? Should we maybe
reload this register with the latest value after each context switch?
Maybe this context mess is the reason we're not supposed to use this
register.

> +
> +	I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |

Apparently we should use 2x for 16bpp formats. BTW the spec doesn't
say anything about 8bpp. Maybe we need to disable fbc w/ 8bpp?

> +		   IVB_DPFC_CTL_FENCE_EN |
> +		   intel_crtc->plane << IVB_DPFC_CTL_PLANE_SHIFT);
> +
> +	I915_WRITE(SNB_DPFC_CTL_SA,
> +		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> +	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
> +
> +	sandybridge_blit_fbc_update(dev);
> +
> +	DRM_DEBUG_KMS("enabled fbc on plane %d\n", intel_crtc->plane);
> +}
> +
>  bool intel_fbc_enabled(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -439,7 +465,7 @@ void intel_update_fbc(struct drm_device *dev)
>  	if (enable_fbc < 0) {
>  		DRM_DEBUG_KMS("fbc set to per-chip default\n");
>  		enable_fbc = 1;
> -		if (INTEL_INFO(dev)->gen <= 6)
> +		if (INTEL_INFO(dev)->gen <= 7)
>  			enable_fbc = 0;
>  	}
>  	if (!enable_fbc) {
> @@ -4180,7 +4206,12 @@ void intel_init_pm(struct drm_device *dev)
>  	if (I915_HAS_FBC(dev)) {
>  		if (HAS_PCH_SPLIT(dev)) {
>  			dev_priv->display.fbc_enabled = ironlake_fbc_enabled;
> -			dev_priv->display.enable_fbc = ironlake_enable_fbc;
> +			if (IS_IVYBRIDGE(dev))
> +				dev_priv->display.enable_fbc =
> +					gen7_enable_fbc;
> +			else
> +				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;
> -- 
> 1.8.1.4

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Add support for FBC on Ivybridge.

[Rodrigo Vivi]

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

Hi Ville, thanks for the careful review.

You are right on most cases, but on of you suggestions made fbc stopping
work here, after a while trying to figure out what it was and testing case
by case I found out we must continue using ILK_FBC_RT_VALID or it doesn't
work on IVB neither on HSW. Maybe because we don't implement that other LRI
WA. So a necessary evil for now.

It saves less power with front target bit set:

// This works:
// I915_WRITE(IVB_FBC_RT_BASE,
//    obj->gtt_offset << 12); |
//    ILK_FBC_RT_VALID);

// This doesn't work:
// I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset << 12 |
SNB_FBC_FRONT_BUFFER);

// This works:
// I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset |
//    ILK_FBC_RT_VALID | SNB_FBC_FRONT_BUFFER);
// min en 20.1 / av en 20.3 / av dis 20.8

// This works better:
I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID);
// min en 19.9 / av en 20.2 / av dis 20.8

new version coming soon...


On Wed, Apr 24, 2013 at 1:47 PM, Ville Syrjälä <
ville.syrjala@linux.intel.com> wrote:

> On Tue, Apr 23, 2013 at 05:55:04PM -0300, Rodrigo Vivi wrote:
> > This patch introduce Frame Buffer Compression (FBC) support for IVB,
> > without enabling it by default.
> > It adds a new function gen7_enable_fbc to avoid getting
> > ironlake_enable_fbc messed with many IS_IVYBRIDGE checks.
> >
> > v2: Fixes from Ville.
> >       *  Fix Plane. FBC is tied to primary plane A in HSW
> >       *  Fix DPFC initial write to avoid let trash on the register.
> > v3: Checking for bad plane on intel_update_fbc() as Chris suggested.
> > v4: Ville pointed out that according to BSpec FBC_CTL bits 0:3 must be 0.
> > v5: Up to v4 this work was entirely focused on Haswell. However Ville
> >     noticed I could reuse the FBC work done for HSW and get FBC for free
> >     at Ivybridge. So it makes more sense enable FBC for IVB first.
> >     FBC for HSW comming on next patches. We are just not enabling it by
> >     default on IVB.
> > v6: Fix confused commit name (by Matt Turner).
> >
> > Cc: Matt Turner <mattst88@gmail.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c |  1 +
> >  drivers/gpu/drm/i915/i915_reg.h |  6 ++++++
> >  drivers/gpu/drm/i915/intel_pm.c | 35 +++++++++++++++++++++++++++++++++--
> >  3 files changed, 40 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> > index 9ebe895..a073b4c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -280,6 +280,7 @@ static const struct intel_device_info
> intel_ivybridge_m_info = {
> >       GEN7_FEATURES,
> >       .is_ivybridge = 1,
> >       .is_mobile = 1,
> > +     .has_fbc = 1,
> >  };
> >
> >  static const struct intel_device_info intel_ivybridge_q_info = {
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> > index 077d40f..f64f118 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -809,7 +809,9 @@
> >  #define   DPFC_CTL_EN                (1<<31)
> >  #define   DPFC_CTL_PLANEA    (0<<30)
> >  #define   DPFC_CTL_PLANEB    (1<<30)
> > +#define   IVB_DPFC_CTL_PLANE_SHIFT   (29)
> >  #define   DPFC_CTL_FENCE_EN  (1<<29)
> > +#define   IVB_DPFC_CTL_FENCE_EN      (1<<28)
> >  #define   DPFC_CTL_PERSISTENT_MODE   (1<<25)
> >  #define   DPFC_SR_EN         (1<<10)
> >  #define   DPFC_CTL_LIMIT_1X  (0<<6)
> > @@ -857,6 +859,10 @@
> >  #define   SNB_CPU_FENCE_ENABLE       (1<<29)
> >  #define DPFC_CPU_FENCE_OFFSET        0x100104
> >
> > +/* Framebuffer compression for Ivybridge */
> > +#define IVB_FBC_RT_BASE                      0x7020
> > +#define   IVB_FBC_RT_BASE_ADDR_SHIFT 12
> > +
> >
> >  /*
> >   * GPIO regs
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> > index f747cb0..86a941a 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -253,6 +253,32 @@ 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)
> > +{
> > +     struct drm_device *dev = crtc->dev;
> > +     struct drm_i915_private *dev_priv = dev->dev_private;
> > +     struct drm_framebuffer *fb = crtc->fb;
> > +     struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> > +     struct drm_i915_gem_object *obj = intel_fb->obj;
> > +     struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > +
> > +     I915_WRITE(IVB_FBC_RT_BASE,
> > +                obj->gtt_offset << IVB_FBC_RT_BASE_ADDR_SHIFT |
>
> gtt_offset is a page aligned byte offset, so the shift shouldn't be
> here.
>
> Also I'm thinking we should probably set the front buffer target bit,
> since we're not using GFDT (yet at least) and our scanout buffers aren't
> cached in LLC. I'm not sure what will happen if we leave the bit unset
> and don't issue GFDT flushes.
>
> > +                ILK_FBC_RT_VALID);
>
> BTW Bspec seems to tell me that we shouldn't even enable this render
> tracking stuff, and instead we should using LRIs to nuke the FBC state
> after flushing.
>
> It's also saying something that we shouldn't write this more than once
> per context. That doesn't sound quite right. What if we flip to another
> buffer? Oh and what happens when we switch to another context? Is it
> going to overwrite this register with a stale value? Should we maybe
> reload this register with the latest value after each context switch?
> Maybe this context mess is the reason we're not supposed to use this
> register.
>
> > +
> > +     I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
>
> Apparently we should use 2x for 16bpp formats. BTW the spec doesn't
> say anything about 8bpp. Maybe we need to disable fbc w/ 8bpp?
>
> > +                IVB_DPFC_CTL_FENCE_EN |
> > +                intel_crtc->plane << IVB_DPFC_CTL_PLANE_SHIFT);
> > +
> > +     I915_WRITE(SNB_DPFC_CTL_SA,
> > +                SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> > +     I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
> > +
> > +     sandybridge_blit_fbc_update(dev);
> > +
> > +     DRM_DEBUG_KMS("enabled fbc on plane %d\n", intel_crtc->plane);
> > +}
> > +
> >  bool intel_fbc_enabled(struct drm_device *dev)
> >  {
> >       struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -439,7 +465,7 @@ void intel_update_fbc(struct drm_device *dev)
> >       if (enable_fbc < 0) {
> >               DRM_DEBUG_KMS("fbc set to per-chip default\n");
> >               enable_fbc = 1;
> > -             if (INTEL_INFO(dev)->gen <= 6)
> > +             if (INTEL_INFO(dev)->gen <= 7)
> >                       enable_fbc = 0;
> >       }
> >       if (!enable_fbc) {
> > @@ -4180,7 +4206,12 @@ void intel_init_pm(struct drm_device *dev)
> >       if (I915_HAS_FBC(dev)) {
> >               if (HAS_PCH_SPLIT(dev)) {
> >                       dev_priv->display.fbc_enabled =
> ironlake_fbc_enabled;
> > -                     dev_priv->display.enable_fbc = ironlake_enable_fbc;
> > +                     if (IS_IVYBRIDGE(dev))
> > +                             dev_priv->display.enable_fbc =
> > +                                     gen7_enable_fbc;
> > +                     else
> > +                             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;
> > --
> > 1.8.1.4
>
> --
> Ville Syrjälä
> Intel OTC
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

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

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

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

[PATCH 2/6] drm/i915: IVB FBC WaFbcAsynchFlipDisableFbcQueue

[Rodrigo Vivi]

Display register 42000h bit 22 must be set to 1b for the entire time that
Frame Buffer Compression is enabled.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 86a941a..6315627 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -270,6 +270,8 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		   IVB_DPFC_CTL_FENCE_EN |
 		   intel_crtc->plane << IVB_DPFC_CTL_PLANE_SHIFT);
 
+	/* WaFbcAsynchFlipDisableFbcQueue */
+	I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
 	I915_WRITE(SNB_DPFC_CTL_SA,
 		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
 	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
-- 
1.8.1.4

Re: [PATCH 2/6] drm/i915: IVB FBC WaFbcAsynchFlipDisableFbcQueue

[Ville Syrjälä]

On Tue, Apr 23, 2013 at 02:52:17PM -0300, Rodrigo Vivi wrote:
> Display register 42000h bit 22 must be set to 1b for the entire time that
> Frame Buffer Compression is enabled.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

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

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 86a941a..6315627 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -270,6 +270,8 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>  		   IVB_DPFC_CTL_FENCE_EN |
>  		   intel_crtc->plane << IVB_DPFC_CTL_PLANE_SHIFT);
>  
> +	/* WaFbcAsynchFlipDisableFbcQueue */
> +	I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
>  	I915_WRITE(SNB_DPFC_CTL_SA,
>  		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
>  	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 3/6] drm/i915: IVB FBC WaFbcDisableDpfcClockGating

[Rodrigo Vivi]

Display register 42020h bit 9 must be set to 1b for the entire time that
Frame Buffer Compression is enabled.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6315627..a33490c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -242,6 +242,11 @@ static void ironlake_disable_fbc(struct drm_device *dev)
 		dpfc_ctl &= ~DPFC_CTL_EN;
 		I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl);
 
+		if (IS_IVYBRIDGE(dev))
+			/* WaFbcDisableDpfcClockGating */
+			I915_WRITE(ILK_DSPCLK_GATE_D,
+				   ~ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
+
 		DRM_DEBUG_KMS("disabled FBC\n");
 	}
 }
@@ -272,6 +277,9 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 
 	/* WaFbcAsynchFlipDisableFbcQueue */
 	I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
+	/* WaFbcDisableDpfcClockGating */
+	I915_WRITE(ILK_DSPCLK_GATE_D, ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
+
 	I915_WRITE(SNB_DPFC_CTL_SA,
 		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
 	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
-- 
1.8.1.4

Re: [PATCH 3/6] drm/i915: IVB FBC WaFbcDisableDpfcClockGating

[Ville Syrjälä]

On Tue, Apr 23, 2013 at 02:52:18PM -0300, Rodrigo Vivi wrote:
> Display register 42020h bit 9 must be set to 1b for the entire time that
> Frame Buffer Compression is enabled.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6315627..a33490c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -242,6 +242,11 @@ static void ironlake_disable_fbc(struct drm_device *dev)
>  		dpfc_ctl &= ~DPFC_CTL_EN;
>  		I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl);
>  
> +		if (IS_IVYBRIDGE(dev))
> +			/* WaFbcDisableDpfcClockGating */
> +			I915_WRITE(ILK_DSPCLK_GATE_D,
> +				   ~ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
> +
>  		DRM_DEBUG_KMS("disabled FBC\n");
>  	}
>  }
> @@ -272,6 +277,9 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>  
>  	/* WaFbcAsynchFlipDisableFbcQueue */
>  	I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
> +	/* WaFbcDisableDpfcClockGating */
> +	I915_WRITE(ILK_DSPCLK_GATE_D, ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
> +

You need to preserve the other bits, mainly ILK_VRHUNIT_CLOCK_GATE_DISABLE.

>  	I915_WRITE(SNB_DPFC_CTL_SA,
>  		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
>  	I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 4/6] drm/i915: Enable FBC at Haswell.

[Rodrigo Vivi]

This patch introduce Frame Buffer Compression (FBC) support for HSW.
FBC is tied to primary plane A in HSW.
---
 drivers/gpu/drm/i915/i915_drv.c |  1 +
 drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a073b4c..6bf7ab4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -315,6 +315,7 @@ static const struct intel_device_info intel_haswell_m_info = {
 	GEN7_FEATURES,
 	.is_haswell = 1,
 	.is_mobile = 1,
+	.has_fbc = 1,
 };
 
 static const struct pci_device_id pciidlist[] = {		/* aka */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a33490c..972a1a3 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -275,10 +275,12 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		   IVB_DPFC_CTL_FENCE_EN |
 		   intel_crtc->plane << IVB_DPFC_CTL_PLANE_SHIFT);
 
-	/* WaFbcAsynchFlipDisableFbcQueue */
-	I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
-	/* WaFbcDisableDpfcClockGating */
-	I915_WRITE(ILK_DSPCLK_GATE_D, ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
+	if (IS_IVYBRIDGE(dev)) {
+		/* WaFbcAsynchFlipDisableFbcQueue */
+		I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
+		/* WaFbcDisableDpfcClockGating */
+		I915_WRITE(ILK_DSPCLK_GATE_D, ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
+	}
 
 	I915_WRITE(SNB_DPFC_CTL_SA,
 		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
@@ -496,7 +498,8 @@ void intel_update_fbc(struct drm_device *dev)
 		dev_priv->no_fbc_reason = FBC_MODE_TOO_LARGE;
 		goto out_disable;
 	}
-	if ((IS_I915GM(dev) || IS_I945GM(dev)) && intel_crtc->plane != 0) {
+	if ((IS_I915GM(dev) || IS_I945GM(dev) || IS_HASWELL(dev)) &&
+	    intel_crtc->plane != 0) {
 		DRM_DEBUG_KMS("plane not 0, disabling compression\n");
 		dev_priv->no_fbc_reason = FBC_BAD_PLANE;
 		goto out_disable;
@@ -4216,7 +4219,7 @@ void intel_init_pm(struct drm_device *dev)
 	if (I915_HAS_FBC(dev)) {
 		if (HAS_PCH_SPLIT(dev)) {
 			dev_priv->display.fbc_enabled = ironlake_fbc_enabled;
-			if (IS_IVYBRIDGE(dev))
+			if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
 				dev_priv->display.enable_fbc =
 					gen7_enable_fbc;
 			else
-- 
1.8.1.4

Re: [PATCH 4/6] drm/i915: Enable FBC at Haswell.

[Ville Syrjälä]

On Tue, Apr 23, 2013 at 02:52:19PM -0300, Rodrigo Vivi wrote:
> This patch introduce Frame Buffer Compression (FBC) support for HSW.
> FBC is tied to primary plane A in HSW.

The docs say FBC must be disabled before disabling the plane on HSW.
We're doing these steps in the opposite order on ILK+. Although maybe
it's not a big deal when were disabling the pipe too.

But I'd just change the order of these operations in both
ironlake_crtc_disable() and haswell_crtc_disable(). Otherwise people
will get confused every time they try to figure out why the order of
operations is different.

> ---
>  drivers/gpu/drm/i915/i915_drv.c |  1 +
>  drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++------
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a073b4c..6bf7ab4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -315,6 +315,7 @@ static const struct intel_device_info intel_haswell_m_info = {
>  	GEN7_FEATURES,
>  	.is_haswell = 1,
>  	.is_mobile = 1,
> +	.has_fbc = 1,
>  };
>  
>  static const struct pci_device_id pciidlist[] = {		/* aka */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a33490c..972a1a3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -275,10 +275,12 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>  		   IVB_DPFC_CTL_FENCE_EN |
>  		   intel_crtc->plane << IVB_DPFC_CTL_PLANE_SHIFT);
>  
> -	/* WaFbcAsynchFlipDisableFbcQueue */
> -	I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
> -	/* WaFbcDisableDpfcClockGating */
> -	I915_WRITE(ILK_DSPCLK_GATE_D, ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
> +	if (IS_IVYBRIDGE(dev)) {
> +		/* WaFbcAsynchFlipDisableFbcQueue */
> +		I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
> +		/* WaFbcDisableDpfcClockGating */
> +		I915_WRITE(ILK_DSPCLK_GATE_D, ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
> +	}
>  
>  	I915_WRITE(SNB_DPFC_CTL_SA,
>  		   SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> @@ -496,7 +498,8 @@ void intel_update_fbc(struct drm_device *dev)
>  		dev_priv->no_fbc_reason = FBC_MODE_TOO_LARGE;
>  		goto out_disable;
>  	}
> -	if ((IS_I915GM(dev) || IS_I945GM(dev)) && intel_crtc->plane != 0) {
> +	if ((IS_I915GM(dev) || IS_I945GM(dev) || IS_HASWELL(dev)) &&
> +	    intel_crtc->plane != 0) {
>  		DRM_DEBUG_KMS("plane not 0, disabling compression\n");
>  		dev_priv->no_fbc_reason = FBC_BAD_PLANE;
>  		goto out_disable;
> @@ -4216,7 +4219,7 @@ void intel_init_pm(struct drm_device *dev)
>  	if (I915_HAS_FBC(dev)) {
>  		if (HAS_PCH_SPLIT(dev)) {
>  			dev_priv->display.fbc_enabled = ironlake_fbc_enabled;
> -			if (IS_IVYBRIDGE(dev))
> +			if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
>  				dev_priv->display.enable_fbc =
>  					gen7_enable_fbc;
>  			else
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 5/6] drm/i915: HSW FBC WaFbcAsynchFlipDisableFbcQueue

[Rodrigo Vivi]

Display register 420B0h bit 22 must be set to 1b for the entire time that
Frame Buffer Compression is enabled.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 7 +++++++
 drivers/gpu/drm/i915/intel_pm.c | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f64f118..cb2d74c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -864,6 +864,13 @@
 #define   IVB_FBC_RT_BASE_ADDR_SHIFT	12
 
 
+#define _HSW_PIPE_SLICE_CHICKEN_1_A	0x420B0
+#define _HSW_PIPE_SLICE_CHICKEN_1_B	0x420B4
+#define   HSW_BYPASS_FBC_QUEUE		(1<<22)
+#define HSW_PIPE_SLICE_CHICKEN_1(pipe) _PIPE(pipe, + \
+					     _HSW_PIPE_SLICE_CHICKEN_1_A, + \
+					     _HSW_PIPE_SLICE_CHICKEN_1_B)
+
 /*
  * GPIO regs
  */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 972a1a3..f81b25f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -280,6 +280,10 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
 		/* WaFbcDisableDpfcClockGating */
 		I915_WRITE(ILK_DSPCLK_GATE_D, ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
+	} else {
+		/* WaFbcAsynchFlipDisableFbcQueue */
+		I915_WRITE(HSW_PIPE_SLICE_CHICKEN_1(intel_crtc->pipe),
+			   HSW_BYPASS_FBC_QUEUE);
 	}
 
 	I915_WRITE(SNB_DPFC_CTL_SA,
-- 
1.8.1.4

Re: [PATCH 5/6] drm/i915: HSW FBC WaFbcAsynchFlipDisableFbcQueue

[Ville Syrjälä]

On Tue, Apr 23, 2013 at 02:52:20PM -0300, Rodrigo Vivi wrote:
> Display register 420B0h bit 22 must be set to 1b for the entire time that
> Frame Buffer Compression is enabled.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>

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

> ---
>  drivers/gpu/drm/i915/i915_reg.h | 7 +++++++
>  drivers/gpu/drm/i915/intel_pm.c | 4 ++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f64f118..cb2d74c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -864,6 +864,13 @@
>  #define   IVB_FBC_RT_BASE_ADDR_SHIFT	12
>  
>  
> +#define _HSW_PIPE_SLICE_CHICKEN_1_A	0x420B0
> +#define _HSW_PIPE_SLICE_CHICKEN_1_B	0x420B4
> +#define   HSW_BYPASS_FBC_QUEUE		(1<<22)
> +#define HSW_PIPE_SLICE_CHICKEN_1(pipe) _PIPE(pipe, + \
> +					     _HSW_PIPE_SLICE_CHICKEN_1_A, + \
> +					     _HSW_PIPE_SLICE_CHICKEN_1_B)
> +
>  /*
>   * GPIO regs
>   */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 972a1a3..f81b25f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -280,6 +280,10 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>  		I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
>  		/* WaFbcDisableDpfcClockGating */
>  		I915_WRITE(ILK_DSPCLK_GATE_D, ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
> +	} else {
> +		/* WaFbcAsynchFlipDisableFbcQueue */
> +		I915_WRITE(HSW_PIPE_SLICE_CHICKEN_1(intel_crtc->pipe),
> +			   HSW_BYPASS_FBC_QUEUE);
>  	}
>  
>  	I915_WRITE(SNB_DPFC_CTL_SA,
> -- 
> 1.8.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 6/6] drm/i915: HSW FBC WaFbcDisableDpfcClockGating

[Rodrigo Vivi]

Display register 46500h bit 23 must be set to 1b for the entire time that
Frame Buffer Compression is enabled.

v2: Ville suggested to enable it back when disabling fbc to avoid wasting
    power.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 3 +++
 drivers/gpu/drm/i915/intel_pm.c | 7 +++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cb2d74c..bd2f64d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -871,6 +871,9 @@
 					     _HSW_PIPE_SLICE_CHICKEN_1_A, + \
 					     _HSW_PIPE_SLICE_CHICKEN_1_B)
 
+#define HSW_CLKGATE_DISABLE_PART_1	0x46500
+#define   HSW_DPFC_GATING_DISABLE	(1<<23)
+
 /*
  * GPIO regs
  */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f81b25f..7e8caba 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -247,6 +247,11 @@ static void ironlake_disable_fbc(struct drm_device *dev)
 			I915_WRITE(ILK_DSPCLK_GATE_D,
 				   ~ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
 
+		if(IS_HASWELL(dev))
+			/* WaFbcDisableDpfcClockGating */
+			I915_WRITE(HSW_CLKGATE_DISABLE_PART_1,
+				   ~HSW_DPFC_GATING_DISABLE);
+
 		DRM_DEBUG_KMS("disabled FBC\n");
 	}
 }
@@ -284,6 +289,8 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 		/* WaFbcAsynchFlipDisableFbcQueue */
 		I915_WRITE(HSW_PIPE_SLICE_CHICKEN_1(intel_crtc->pipe),
 			   HSW_BYPASS_FBC_QUEUE);
+		/* WaFbcDisableDpfcClockGating */
+		I915_WRITE(HSW_CLKGATE_DISABLE_PART_1, HSW_DPFC_GATING_DISABLE);
 	}
 
 	I915_WRITE(SNB_DPFC_CTL_SA,
-- 
1.8.1.4

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

Re: [PATCH 6/6] drm/i915: HSW FBC WaFbcDisableDpfcClockGating

[Ville Syrjälä]

On Tue, Apr 23, 2013 at 02:52:21PM -0300, Rodrigo Vivi wrote:
> Display register 46500h bit 23 must be set to 1b for the entire time that
> Frame Buffer Compression is enabled.
> 
> v2: Ville suggested to enable it back when disabling fbc to avoid wasting
>     power.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 3 +++
>  drivers/gpu/drm/i915/intel_pm.c | 7 +++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cb2d74c..bd2f64d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -871,6 +871,9 @@
>  					     _HSW_PIPE_SLICE_CHICKEN_1_A, + \
>  					     _HSW_PIPE_SLICE_CHICKEN_1_B)
>  
> +#define HSW_CLKGATE_DISABLE_PART_1	0x46500
> +#define   HSW_DPFC_GATING_DISABLE	(1<<23)
> +
>  /*
>   * GPIO regs
>   */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f81b25f..7e8caba 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -247,6 +247,11 @@ static void ironlake_disable_fbc(struct drm_device *dev)
>  			I915_WRITE(ILK_DSPCLK_GATE_D,
>  				   ~ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
>  
> +		if(IS_HASWELL(dev))
> +			/* WaFbcDisableDpfcClockGating */
> +			I915_WRITE(HSW_CLKGATE_DISABLE_PART_1,
> +				   ~HSW_DPFC_GATING_DISABLE);

Should be '0' or maybe better do RMW instead in case someone has to
touch this register from somwhere else.

Same issue in IVB patch btw, just didn't spot it then.

> +
>  		DRM_DEBUG_KMS("disabled FBC\n");
>  	}
>  }
> @@ -284,6 +289,8 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
>  		/* WaFbcAsynchFlipDisableFbcQueue */
>  		I915_WRITE(HSW_PIPE_SLICE_CHICKEN_1(intel_crtc->pipe),
>  			   HSW_BYPASS_FBC_QUEUE);
> +		/* WaFbcDisableDpfcClockGating */
> +		I915_WRITE(HSW_CLKGATE_DISABLE_PART_1, HSW_DPFC_GATING_DISABLE);
>  	}
>  
>  	I915_WRITE(SNB_DPFC_CTL_SA,
> -- 
> 1.8.1.4

-- 
Ville Syrjälä
Intel OTC