[PATCH] drm/i915: Switch off FBC when disabling the primary plane when obscured

[PATCH] drm/i915: Switch off FBC when disabling the primary plane when obscured

[Chris Wilson]

As we switch on/off the primary plane if it is completely obscured by an
overlapping video sprite, we also nee to make sure that we update the
FBC configuration at the same time.

References: https://bugs.freedesktop.org/show_bug.cgi?id=50238
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_drv.h    |    2 +-
 drivers/gpu/drm/i915/intel_pm.c     |    4 +++-
 drivers/gpu/drm/i915/intel_sprite.c |   26 +++++++++++++++-----------
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 11eafbd..d5cc0a2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -172,6 +172,7 @@ struct intel_crtc {
 	u8 lut_r[256], lut_g[256], lut_b[256];
 	int dpms_mode;
 	bool active; /* is the crtc on? independent of the dpms mode */
+	bool primary_disabled; /* is the crtc obscured by a plane? */
 	bool lowfreq_avail;
 	struct intel_overlay *overlay;
 	struct intel_unpin_work *unpin_work;
@@ -199,7 +200,6 @@ struct intel_plane {
 	struct drm_plane base;
 	enum pipe pipe;
 	struct drm_i915_gem_object *obj;
-	bool primary_disabled;
 	int max_downscale;
 	u32 lut_r[1024], lut_g[1024], lut_b[1024];
 	void (*update_plane)(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 51b5872..2d5a322 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -405,7 +405,9 @@ void intel_update_fbc(struct drm_device *dev)
 	 *   - going to an unsupported config (interlace, pixel multiply, etc.)
 	 */
 	list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, head) {
-		if (tmp_crtc->enabled && tmp_crtc->fb) {
+		if (tmp_crtc->enabled &&
+		    !tmp_crtc->primary_disabled &&
+		    tmp_crtc->fb) {
 			if (crtc) {
 				DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
 				dev_priv->no_fbc_reason = FBC_MULTIPLE_PIPES;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 5369a4c..e8abb8e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -336,6 +336,12 @@ intel_enable_primary(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int reg = DSPCNTR(intel_crtc->plane);
 
+	if (!intel_crtc->primary_disabled)
+		return;
+
+	intel_crtc->primary_disabled = false;
+	intel_update_fbc(dev);
+
 	I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE);
 }
 
@@ -347,7 +353,13 @@ intel_disable_primary(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int reg = DSPCNTR(intel_crtc->plane);
 
+	if (intel_crtc->primary_disabled)
+		return;
+
 	I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE);
+
+	intel_crtc->primary_disabled = true;
+	intel_update_fbc(dev);
 }
 
 static int
@@ -524,18 +536,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	 * Be sure to re-enable the primary before the sprite is no longer
 	 * covering it fully.
 	 */
-	if (!disable_primary && intel_plane->primary_disabled) {
+	if (!disable_primary)
 		intel_enable_primary(crtc);
-		intel_plane->primary_disabled = false;
-	}
 
 	intel_plane->update_plane(plane, fb, obj, crtc_x, crtc_y,
 				  crtc_w, crtc_h, x, y, src_w, src_h);
 
-	if (disable_primary) {
+	if (disable_primary)
 		intel_disable_primary(crtc);
-		intel_plane->primary_disabled = true;
-	}
 
 	/* Unpin old obj after new one is active to avoid ugliness */
 	if (old_obj)
@@ -554,11 +562,7 @@ intel_disable_plane(struct drm_plane *plane)
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	int ret = 0;
 
-	if (intel_plane->primary_disabled) {
-		intel_enable_primary(plane->crtc);
-		intel_plane->primary_disabled = false;
-	}
-
+	intel_enable_primary(plane->crtc);
 	intel_plane->disable_plane(plane);
 
 	mutex_lock(&dev->struct_mutex);
-- 
1.7.10

Re: [PATCH] drm/i915: Switch off FBC when disabling the primary plane when obscured

[Jani Nikula]

On Tue, 12 Jun 2012, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> As we switch on/off the primary plane if it is completely obscured by an
> overlapping video sprite, we also nee to make sure that we update the
> FBC configuration at the same time.

I had a patch to this effect which I didn't get around to sending
yet. Oh well.

Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com>

>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=50238
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_drv.h    |    2 +-
>  drivers/gpu/drm/i915/intel_pm.c     |    4 +++-
>  drivers/gpu/drm/i915/intel_sprite.c |   26 +++++++++++++++-----------
>  3 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 11eafbd..d5cc0a2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -172,6 +172,7 @@ struct intel_crtc {
>  	u8 lut_r[256], lut_g[256], lut_b[256];
>  	int dpms_mode;
>  	bool active; /* is the crtc on? independent of the dpms mode */
> +	bool primary_disabled; /* is the crtc obscured by a plane? */
>  	bool lowfreq_avail;
>  	struct intel_overlay *overlay;
>  	struct intel_unpin_work *unpin_work;
> @@ -199,7 +200,6 @@ struct intel_plane {
>  	struct drm_plane base;
>  	enum pipe pipe;
>  	struct drm_i915_gem_object *obj;
> -	bool primary_disabled;
>  	int max_downscale;
>  	u32 lut_r[1024], lut_g[1024], lut_b[1024];
>  	void (*update_plane)(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 51b5872..2d5a322 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -405,7 +405,9 @@ void intel_update_fbc(struct drm_device *dev)
>  	 *   - going to an unsupported config (interlace, pixel multiply, etc.)
>  	 */
>  	list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, head) {
> -		if (tmp_crtc->enabled && tmp_crtc->fb) {
> +		if (tmp_crtc->enabled &&
> +		    !tmp_crtc->primary_disabled &&
> +		    tmp_crtc->fb) {
>  			if (crtc) {
>  				DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
>  				dev_priv->no_fbc_reason = FBC_MULTIPLE_PIPES;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 5369a4c..e8abb8e 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -336,6 +336,12 @@ intel_enable_primary(struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int reg = DSPCNTR(intel_crtc->plane);
>  
> +	if (!intel_crtc->primary_disabled)
> +		return;
> +
> +	intel_crtc->primary_disabled = false;
> +	intel_update_fbc(dev);
> +
>  	I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE);
>  }
>  
> @@ -347,7 +353,13 @@ intel_disable_primary(struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int reg = DSPCNTR(intel_crtc->plane);
>  
> +	if (intel_crtc->primary_disabled)
> +		return;
> +
>  	I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE);
> +
> +	intel_crtc->primary_disabled = true;
> +	intel_update_fbc(dev);
>  }
>  
>  static int
> @@ -524,18 +536,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	 * Be sure to re-enable the primary before the sprite is no longer
>  	 * covering it fully.
>  	 */
> -	if (!disable_primary && intel_plane->primary_disabled) {
> +	if (!disable_primary)
>  		intel_enable_primary(crtc);
> -		intel_plane->primary_disabled = false;
> -	}
>  
>  	intel_plane->update_plane(plane, fb, obj, crtc_x, crtc_y,
>  				  crtc_w, crtc_h, x, y, src_w, src_h);
>  
> -	if (disable_primary) {
> +	if (disable_primary)
>  		intel_disable_primary(crtc);
> -		intel_plane->primary_disabled = true;
> -	}
>  
>  	/* Unpin old obj after new one is active to avoid ugliness */
>  	if (old_obj)
> @@ -554,11 +562,7 @@ intel_disable_plane(struct drm_plane *plane)
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	int ret = 0;
>  
> -	if (intel_plane->primary_disabled) {
> -		intel_enable_primary(plane->crtc);
> -		intel_plane->primary_disabled = false;
> -	}
> -
> +	intel_enable_primary(plane->crtc);
>  	intel_plane->disable_plane(plane);
>  
>  	mutex_lock(&dev->struct_mutex);
> -- 
> 1.7.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Switch off FBC when disabling the primary plane when obscured

[Jesse Barnes]

On Tue, 12 Jun 2012 12:46:00 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> As we switch on/off the primary plane if it is completely obscured by an
> overlapping video sprite, we also nee to make sure that we update the
> FBC configuration at the same time.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=50238
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---

Yeah looks good.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Though now we have 3 "active" state variables for the crtc don't we?
active, enabled, and now primary_disabled?  Oh and dpms state.  I
really wish we could get rid of 2 or 3 of those...</dreaming>

Jesse

Re: [PATCH] drm/i915: Switch off FBC when disabling the primary plane when obscured

[Chris Wilson]

On Tue, 12 Jun 2012 07:28:04 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Though now we have 3 "active" state variables for the crtc don't we?
> active, enabled, and now primary_disabled?  Oh and dpms state.  I
> really wish we could get rid of 2 or 3 of those...</dreaming>

That crossed my mind as well. This felt like it was an orthogonal
property of the crtc, though in reality if we disable the crtc we will
disable the sprites as well so there is a dependence. Given how quickly
the dream of reducing the number of active/enabled/disabled/dpms_mode
states turns into a nightmare, I felt a certain reticient to attempting
it. Perhaps our in-house dragon tamer may like to give it a shot?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Switch off FBC when disabling the primary plane when obscured

[Daniel Vetter]

On Tue, Jun 12, 2012 at 12:46:00PM +0100, Chris Wilson wrote:
> As we switch on/off the primary plane if it is completely obscured by an
> overlapping video sprite, we also nee to make sure that we update the
> FBC configuration at the same time.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=50238
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_drv.h    |    2 +-
>  drivers/gpu/drm/i915/intel_pm.c     |    4 +++-
>  drivers/gpu/drm/i915/intel_sprite.c |   26 +++++++++++++++-----------
>  3 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 11eafbd..d5cc0a2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -172,6 +172,7 @@ struct intel_crtc {
>  	u8 lut_r[256], lut_g[256], lut_b[256];
>  	int dpms_mode;
>  	bool active; /* is the crtc on? independent of the dpms mode */
> +	bool primary_disabled; /* is the crtc obscured by a plane? */
>  	bool lowfreq_avail;
>  	struct intel_overlay *overlay;
>  	struct intel_unpin_work *unpin_work;
> @@ -199,7 +200,6 @@ struct intel_plane {
>  	struct drm_plane base;
>  	enum pipe pipe;
>  	struct drm_i915_gem_object *obj;
> -	bool primary_disabled;
>  	int max_downscale;
>  	u32 lut_r[1024], lut_g[1024], lut_b[1024];
>  	void (*update_plane)(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 51b5872..2d5a322 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -405,7 +405,9 @@ void intel_update_fbc(struct drm_device *dev)
>  	 *   - going to an unsupported config (interlace, pixel multiply, etc.)
>  	 */
>  	list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, head) {
> -		if (tmp_crtc->enabled && tmp_crtc->fb) {
> +		if (tmp_crtc->enabled &&
> +		    !tmp_crtc->primary_disabled &&

At least in my tree here tmp_crtc is a struct drm_crtc pointer and hence
can't have a primary_disabled member ...

Care to rebase this onto dinq?
-Daniel

> +		    tmp_crtc->fb) {
>  			if (crtc) {
>  				DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
>  				dev_priv->no_fbc_reason = FBC_MULTIPLE_PIPES;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 5369a4c..e8abb8e 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -336,6 +336,12 @@ intel_enable_primary(struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int reg = DSPCNTR(intel_crtc->plane);
>  
> +	if (!intel_crtc->primary_disabled)
> +		return;
> +
> +	intel_crtc->primary_disabled = false;
> +	intel_update_fbc(dev);
> +
>  	I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE);
>  }
>  
> @@ -347,7 +353,13 @@ intel_disable_primary(struct drm_crtc *crtc)
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int reg = DSPCNTR(intel_crtc->plane);
>  
> +	if (intel_crtc->primary_disabled)
> +		return;
> +
>  	I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE);
> +
> +	intel_crtc->primary_disabled = true;
> +	intel_update_fbc(dev);
>  }
>  
>  static int
> @@ -524,18 +536,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	 * Be sure to re-enable the primary before the sprite is no longer
>  	 * covering it fully.
>  	 */
> -	if (!disable_primary && intel_plane->primary_disabled) {
> +	if (!disable_primary)
>  		intel_enable_primary(crtc);
> -		intel_plane->primary_disabled = false;
> -	}
>  
>  	intel_plane->update_plane(plane, fb, obj, crtc_x, crtc_y,
>  				  crtc_w, crtc_h, x, y, src_w, src_h);
>  
> -	if (disable_primary) {
> +	if (disable_primary)
>  		intel_disable_primary(crtc);
> -		intel_plane->primary_disabled = true;
> -	}
>  
>  	/* Unpin old obj after new one is active to avoid ugliness */
>  	if (old_obj)
> @@ -554,11 +562,7 @@ intel_disable_plane(struct drm_plane *plane)
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	int ret = 0;
>  
> -	if (intel_plane->primary_disabled) {
> -		intel_enable_primary(plane->crtc);
> -		intel_plane->primary_disabled = false;
> -	}
> -
> +	intel_enable_primary(plane->crtc);
>  	intel_plane->disable_plane(plane);
>  
>  	mutex_lock(&dev->struct_mutex);
> -- 
> 1.7.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

[PATCH] drm/i915: Switch off FBC when disabling the primary plane when obscured

[Chris Wilson]

As we switch on/off the primary plane if it is completely obscured by an
overlapping video sprite, we also nee to make sure that we update the
FBC configuration at the same time.

v2: Not all crtcs are intel_crtcs, as spotted by Daniel.

References: https://bugs.freedesktop.org/show_bug.cgi?id=50238
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    |    2 +-
 drivers/gpu/drm/i915/intel_pm.c     |    4 +++-
 drivers/gpu/drm/i915/intel_sprite.c |   26 +++++++++++++++-----------
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1a1fdb0..5290e9d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -169,6 +169,7 @@ struct intel_crtc {
 	u8 lut_r[256], lut_g[256], lut_b[256];
 	int dpms_mode;
 	bool active; /* is the crtc on? independent of the dpms mode */
+	bool primary_disabled; /* is the crtc obscured by a plane? */
 	bool busy; /* is scanout buffer being updated frequently? */
 	struct timer_list idle_timer;
 	bool lowfreq_avail;
@@ -191,7 +192,6 @@ struct intel_plane {
 	struct drm_plane base;
 	enum pipe pipe;
 	struct drm_i915_gem_object *obj;
-	bool primary_disabled;
 	int max_downscale;
 	u32 lut_r[1024], lut_g[1024], lut_b[1024];
 	void (*update_plane)(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d0ce2a5..b7de5ea 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -405,7 +405,9 @@ void intel_update_fbc(struct drm_device *dev)
 	 *   - going to an unsupported config (interlace, pixel multiply, etc.)
 	 */
 	list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, head) {
-		if (tmp_crtc->enabled && tmp_crtc->fb) {
+		if (tmp_crtc->enabled &&
+		    !to_intel_crtc(tmp_crtc)->primary_disabled &&
+		    tmp_crtc->fb) {
 			if (crtc) {
 				DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
 				dev_priv->no_fbc_reason = FBC_MULTIPLE_PIPES;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 2a20fb0..4cedb37 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -326,6 +326,12 @@ intel_enable_primary(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int reg = DSPCNTR(intel_crtc->plane);
 
+	if (!intel_crtc->primary_disabled)
+		return;
+
+	intel_crtc->primary_disabled = false;
+	intel_update_fbc(dev);
+
 	I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE);
 }
 
@@ -337,7 +343,13 @@ intel_disable_primary(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int reg = DSPCNTR(intel_crtc->plane);
 
+	if (intel_crtc->primary_disabled)
+		return;
+
 	I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE);
+
+	intel_crtc->primary_disabled = true;
+	intel_update_fbc(dev);
 }
 
 static int
@@ -485,18 +497,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	 * Be sure to re-enable the primary before the sprite is no longer
 	 * covering it fully.
 	 */
-	if (!disable_primary && intel_plane->primary_disabled) {
+	if (!disable_primary)
 		intel_enable_primary(crtc);
-		intel_plane->primary_disabled = false;
-	}
 
 	intel_plane->update_plane(plane, fb, obj, crtc_x, crtc_y,
 				  crtc_w, crtc_h, x, y, src_w, src_h);
 
-	if (disable_primary) {
+	if (disable_primary)
 		intel_disable_primary(crtc);
-		intel_plane->primary_disabled = true;
-	}
 
 	/* Unpin old obj after new one is active to avoid ugliness */
 	if (old_obj) {
@@ -527,11 +535,7 @@ intel_disable_plane(struct drm_plane *plane)
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	int ret = 0;
 
-	if (intel_plane->primary_disabled) {
-		intel_enable_primary(plane->crtc);
-		intel_plane->primary_disabled = false;
-	}
-
+	intel_enable_primary(plane->crtc);
 	intel_plane->disable_plane(plane);
 
 	if (!intel_plane->obj)
-- 
1.7.10

Re: [PATCH] drm/i915: Switch off FBC when disabling the primary plane when obscured

[Daniel Vetter]

On Wed, Jun 13, 2012 at 12:18:46PM +0100, Chris Wilson wrote:
> As we switch on/off the primary plane if it is completely obscured by an
> overlapping video sprite, we also nee to make sure that we update the
> FBC configuration at the same time.
> 
> v2: Not all crtcs are intel_crtcs, as spotted by Daniel.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=50238
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com>
Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: Switch off FBC when disabling the primary plane when obscured

[Daniel Vetter]

On Wed, Jun 13, 2012 at 01:40:24PM +0200, Daniel Vetter wrote:
> On Wed, Jun 13, 2012 at 12:18:46PM +0100, Chris Wilson wrote:
> > As we switch on/off the primary plane if it is completely obscured by an
> > overlapping video sprite, we also nee to make sure that we update the
> > FBC configuration at the same time.
> > 
> > v2: Not all crtcs are intel_crtcs, as spotted by Daniel.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=50238
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com>
> Queued for -next, thanks for the patch.
Blows up on my snb, dropped again.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

[PATCH] drm/i915: Switch off FBC when disabling the primary plane when obscured

[Chris Wilson]

As we switch on/off the primary plane if it is completely obscured by an
overlapping video sprite, we also nee to make sure that we update the
FBC configuration at the same time.

v2: Not all crtcs are intel_crtcs, as spotted by Daniel.
v3: Boot testing rules.

References: https://bugs.freedesktop.org/show_bug.cgi?id=50238
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_drv.h    |    2 +-
 drivers/gpu/drm/i915/intel_pm.c     |    4 +++-
 drivers/gpu/drm/i915/intel_sprite.c |   25 +++++++++++++++----------
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1a1fdb0..5290e9d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -169,6 +169,7 @@ struct intel_crtc {
 	u8 lut_r[256], lut_g[256], lut_b[256];
 	int dpms_mode;
 	bool active; /* is the crtc on? independent of the dpms mode */
+	bool primary_disabled; /* is the crtc obscured by a plane? */
 	bool busy; /* is scanout buffer being updated frequently? */
 	struct timer_list idle_timer;
 	bool lowfreq_avail;
@@ -191,7 +192,6 @@ struct intel_plane {
 	struct drm_plane base;
 	enum pipe pipe;
 	struct drm_i915_gem_object *obj;
-	bool primary_disabled;
 	int max_downscale;
 	u32 lut_r[1024], lut_g[1024], lut_b[1024];
 	void (*update_plane)(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d0ce2a5..b7de5ea 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -405,7 +405,9 @@ void intel_update_fbc(struct drm_device *dev)
 	 *   - going to an unsupported config (interlace, pixel multiply, etc.)
 	 */
 	list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, head) {
-		if (tmp_crtc->enabled && tmp_crtc->fb) {
+		if (tmp_crtc->enabled &&
+		    !to_intel_crtc(tmp_crtc)->primary_disabled &&
+		    tmp_crtc->fb) {
 			if (crtc) {
 				DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
 				dev_priv->no_fbc_reason = FBC_MULTIPLE_PIPES;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 2a20fb0..9d7777b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -326,6 +326,12 @@ intel_enable_primary(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int reg = DSPCNTR(intel_crtc->plane);
 
+	if (!intel_crtc->primary_disabled)
+		return;
+
+	intel_crtc->primary_disabled = false;
+	intel_update_fbc(dev);
+
 	I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE);
 }
 
@@ -337,7 +343,13 @@ intel_disable_primary(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int reg = DSPCNTR(intel_crtc->plane);
 
+	if (intel_crtc->primary_disabled)
+		return;
+
 	I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE);
+
+	intel_crtc->primary_disabled = true;
+	intel_update_fbc(dev);
 }
 
 static int
@@ -485,18 +497,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	 * Be sure to re-enable the primary before the sprite is no longer
 	 * covering it fully.
 	 */
-	if (!disable_primary && intel_plane->primary_disabled) {
+	if (!disable_primary)
 		intel_enable_primary(crtc);
-		intel_plane->primary_disabled = false;
-	}
 
 	intel_plane->update_plane(plane, fb, obj, crtc_x, crtc_y,
 				  crtc_w, crtc_h, x, y, src_w, src_h);
 
-	if (disable_primary) {
+	if (disable_primary)
 		intel_disable_primary(crtc);
-		intel_plane->primary_disabled = true;
-	}
 
 	/* Unpin old obj after new one is active to avoid ugliness */
 	if (old_obj) {
@@ -527,11 +535,8 @@ intel_disable_plane(struct drm_plane *plane)
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	int ret = 0;
 
-	if (intel_plane->primary_disabled) {
+	if (plane->crtc)
 		intel_enable_primary(plane->crtc);
-		intel_plane->primary_disabled = false;
-	}
-
 	intel_plane->disable_plane(plane);
 
 	if (!intel_plane->obj)
-- 
1.7.10

Re: [PATCH] drm/i915: Switch off FBC when disabling the primary plane when obscured

[Daniel Vetter]

On Wed, Jun 13, 2012 at 05:36:55PM +0100, Chris Wilson wrote:
> As we switch on/off the primary plane if it is completely obscured by an
> overlapping video sprite, we also nee to make sure that we update the
> FBC configuration at the same time.
> 
> v2: Not all crtcs are intel_crtcs, as spotted by Daniel.
> v3: Boot testing rules.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=50238
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48