diff mbox

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

Message ID 1442254803-20043-8-git-send-email-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

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

v2:
  - Drop the alpha formats (Ville).

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

Comments

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

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

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

Patch

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