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

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 8/9] drm/i915: reject invalid formats for FBC
Date: Mon, 21 Sep 2015 17:00:59 +0300	[thread overview]
Message-ID: <20150921140059.GH26517@intel.com> (raw)
In-Reply-To: <1442254803-20043-8-git-send-email-paulo.r.zanoni@intel.com>

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

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

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

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