Re: [PATCH] drm/i915: fixup fb bpp computation in pipe_config_set_bpp

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Mar 28, 2013 at 04:38:08PM +0100, Daniel Vetter wrote:
> Ville pointed out that my assumption that no unsupported pixel format
> can get past the pipe config computation stage to the platform
> update_plane callbacks is wrong. The reason is that this function
> still checks the old fb->depth value instead of the new pixel_format.
> 
> While checking with all the other places that use this I've noticed
> that intel_framebuffer_init already has all the platform checks we
> need, so replace those checks with a WARN_ON.
> 
> Since fb->depth isn't set for YUV pixel formats and since we already
> can't create an fb with an rgb layout not support on the running
> platform I /think/ this patch doesn't fix any bug.
> 
> But it surely looks better!
> 
> v2: BGR formats are also only gen4+, so add the corresponding WARN_ON,
> too (Ville).
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

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

> ---
>  drivers/gpu/drm/i915/intel_display.c |   31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3f3a3dc..5e8b91f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7394,23 +7394,34 @@ pipe_config_set_bpp(struct drm_crtc *crtc,
>  	struct drm_connector *connector;
>  	int bpp;
>  
> -	switch (fb->depth) {
> -	case 8:
> +	switch (fb->pixel_format) {
> +	case DRM_FORMAT_C8:
>  		bpp = 8*3; /* since we go through a colormap */
>  		break;
> -	case 15:
> -	case 16:
> +	case DRM_FORMAT_XRGB1555:
> +	case DRM_FORMAT_ARGB1555:
> +		/* checked in intel_framebuffer_init already */
> +		if (WARN_ON(INTEL_INFO(dev)->gen > 3))
> +			return -EINVAL;
> +	case DRM_FORMAT_RGB565:
>  		bpp = 6*3; /* min is 18bpp */
>  		break;
> -	case 24:
> +	case DRM_FORMAT_XBGR8888:
> +	case DRM_FORMAT_ABGR8888:
> +		/* checked in intel_framebuffer_init already */
> +		if (WARN_ON(INTEL_INFO(dev)->gen < 4))
> +			return -EINVAL;
> +	case DRM_FORMAT_XRGB8888:
> +	case DRM_FORMAT_ARGB8888:
>  		bpp = 8*3;
>  		break;
> -	case 30:
> -		if (INTEL_INFO(dev)->gen < 4) {
> -			DRM_DEBUG_KMS("10 bpc not supported on gen2/3\n");
> +	case DRM_FORMAT_XRGB2101010:
> +	case DRM_FORMAT_ARGB2101010:
> +	case DRM_FORMAT_XBGR2101010:
> +	case DRM_FORMAT_ABGR2101010:
> +		/* checked in intel_framebuffer_init already */
> +		if (WARN_ON(INTEL_INFO(dev)->gen < 4))
>  			return -EINVAL;
> -		}
> -
>  		bpp = 10*3;
>  		break;
>  	/* TODO: gen4+ supports 16 bpc floating point, too. */
> -- 
> 1.7.10.4

-- 
Ville Syrjälä
Intel OTC