Re: [PATCH v2] drm/i915: Don't expose RGB/BGR 8888 formats on primary planes before SKL

From: Daniel Vetter <daniel@ffwll.ch>
To: Damien Lespiau <damien.lespiau@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2] drm/i915: Don't expose RGB/BGR 8888 formats on primary planes before SKL
Date: Tue, 19 May 2015 10:00:03 +0200	[thread overview]
Message-ID: <20150519080003.GD15256@phenom.ffwll.local> (raw)
In-Reply-To: <1431717342-25465-1-git-send-email-damien.lespiau@intel.com>

On Fri, May 15, 2015 at 08:15:42PM +0100, Damien Lespiau wrote:
> We don't actually do anything different for the A version of the 8888
> RGB formats before SKL. Don't let user space think we can support alpha
> blending.
> 
> v2: Fix the logic to forbid the creation ABGR2101010 fbs (Ville)
> 
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 35 ++++++++++++++++++++++++-----------
>  1 file changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5ebac76..f75505e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -52,7 +52,6 @@ static const uint32_t i8xx_primary_formats[] = {
>  	DRM_FORMAT_XRGB1555,
>  	DRM_FORMAT_ARGB1555,
>  	DRM_FORMAT_XRGB8888,
> -	DRM_FORMAT_ARGB8888,
>  };
>  
>  /* Primary plane formats for gen >= 4 */
> @@ -61,6 +60,15 @@ static const uint32_t i965_primary_formats[] = {
>  	DRM_FORMAT_RGB565,
>  	DRM_FORMAT_XRGB8888,
>  	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_XRGB2101010,
> +	DRM_FORMAT_XBGR2101010,
> +};
> +
> +static const uint32_t skl_primary_formats[] = {
> +	DRM_FORMAT_C8,
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_XBGR8888,
>  	DRM_FORMAT_ARGB8888,
>  	DRM_FORMAT_ABGR8888,
>  	DRM_FORMAT_XRGB2101010,
> @@ -2706,11 +2714,9 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  		dspcntr |= DISPPLANE_BGRX565;
>  		break;
>  	case DRM_FORMAT_XRGB8888:
> -	case DRM_FORMAT_ARGB8888:
>  		dspcntr |= DISPPLANE_BGRX888;
>  		break;
>  	case DRM_FORMAT_XBGR8888:
> -	case DRM_FORMAT_ABGR8888:
>  		dspcntr |= DISPPLANE_RGBX888;
>  		break;
>  	case DRM_FORMAT_XRGB2101010:
> @@ -2812,11 +2818,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  		dspcntr |= DISPPLANE_BGRX565;
>  		break;
>  	case DRM_FORMAT_XRGB8888:
> -	case DRM_FORMAT_ARGB8888:
>  		dspcntr |= DISPPLANE_BGRX888;
>  		break;
>  	case DRM_FORMAT_XBGR8888:
> -	case DRM_FORMAT_ABGR8888:
>  		dspcntr |= DISPPLANE_RGBX888;
>  		break;
>  	case DRM_FORMAT_XRGB2101010:
> @@ -13278,12 +13282,15 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
>  		primary->plane = !pipe;
>  
> -	if (INTEL_INFO(dev)->gen <= 3) {
> -		intel_primary_formats = i8xx_primary_formats;
> -		num_formats = ARRAY_SIZE(i8xx_primary_formats);
> -	} else {
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		intel_primary_formats = skl_primary_formats;
> +		num_formats = ARRAY_SIZE(skl_primary_formats);
> +	} else if (INTEL_INFO(dev)->gen >= 4) {
>  		intel_primary_formats = i965_primary_formats;
>  		num_formats = ARRAY_SIZE(i965_primary_formats);
> +	} else {
> +		intel_primary_formats = i8xx_primary_formats;
> +		num_formats = ARRAY_SIZE(i8xx_primary_formats);
>  	}
>  
>  	drm_universal_plane_init(dev, &primary->base, 0,
> @@ -13961,7 +13968,6 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  	case DRM_FORMAT_C8:
>  	case DRM_FORMAT_RGB565:
>  	case DRM_FORMAT_XRGB8888:
> -	case DRM_FORMAT_ARGB8888:
>  		break;
>  	case DRM_FORMAT_XRGB1555:
>  	case DRM_FORMAT_ARGB1555:
> @@ -13971,8 +13977,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  			return -EINVAL;
>  		}
>  		break;
> -	case DRM_FORMAT_XBGR8888:
> +	case DRM_FORMAT_ARGB8888:
>  	case DRM_FORMAT_ABGR8888:
> +		if (!IS_VALLEYVIEW(dev) && INTEL_INFO(dev)->gen < 9) {
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +				  drm_get_format_name(mode_cmd->pixel_format));
> +			return -EINVAL;
> +		}
> +		break;
> +	case DRM_FORMAT_XBGR8888:

These two hunks break cursor support, since with universal planes we
really want to be able to create rgba8888 for the cursor ...

I dropped the patch meanwhile, can you please resend?

Thanks, Daniel

>  	case DRM_FORMAT_XRGB2101010:
>  	case DRM_FORMAT_XBGR2101010:
>  		if (INTEL_INFO(dev)->gen < 4) {
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx