On Wed, May 20, 2015 at 02:58:53PM +0200, Daniel Vetter wrote: > On Wed, May 20, 2015 at 12:50:35PM +0200, Philipp Zabel wrote: > > 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----- > > Yeah that looks perfect, please wrap with commit message&sob and I'll > apply. I think that's going to trigger a lot. IIRC we call this thing for any format when constructing FBs. -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel