Re: [PATCH 6/7] drm: add RGB formats with separate alpha plane

[Philipp Zabel <p.zabel@pengutronix.de>]

Hi Daniel,

thank you for the comments.

Am Dienstag, den 19.05.2015, 18:52 +0200 schrieb Daniel Vetter:
[...]
> > @@ -5198,11 +5206,15 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> >  		break;
> >  	case DRM_FORMAT_RGB565:
> >  	case DRM_FORMAT_BGR565:
> > +	case DRM_FORMAT_RGB565_A8:
> > +	case DRM_FORMAT_BGR565_A8:
> >  		*depth = 16;
> >  		*bpp = 16;
> >  		break;
> >  	case DRM_FORMAT_RGB888:
> >  	case DRM_FORMAT_BGR888:
> > +	case DRM_FORMAT_RGB888_A8:
> > +	case DRM_FORMAT_BGR888_A8:
> >  		*depth = 24;
> >  		*bpp = 24;
> >  		break;
> > @@ -5210,6 +5222,10 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> >  	case DRM_FORMAT_XBGR8888:
> >  	case DRM_FORMAT_RGBX8888:
> >  	case DRM_FORMAT_BGRX8888:
> > +	case DRM_FORMAT_XRGB8888_A8:
> > +	case DRM_FORMAT_XBGR8888_A8:
> > +	case DRM_FORMAT_RGBX8888_A8:
> > +	case DRM_FORMAT_BGRX8888_A8:
> >  		*depth = 24;
> >  		*bpp = 32;
> >  		break;
> 
> Please drop the above two hunks, these functions are only for backwards
> compat with drivers from the addfb1 days. Modern drivers should only use
> the format tags directly. Extending the plane_cpp function like you do
> below is enough.

I'll leave drm_fb_get_bpp_depth untouched.

> Maybe we should add a WARN_ON(num_planes(format) != 0) to the top of this
> and a comment that this is for legacy stuff only.

Do you mean:

-----8<-----
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5186,6 +5186,12 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
 void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
 			  int *bpp)
 {
+	/*
+	 * This function is to be used for legacy drivers only, no new formats
+	 * should be added here.
+	 */
+	WARN_ON(drm_format_num_planes(format) != 1);
+
 	switch (format) {
 	case DRM_FORMAT_C8:
 	case DRM_FORMAT_RGB332:
----->8-----

> lgtm otherwise.
> -Daniel

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel