On Wed, May 20, 2015 at 04:54:01PM +0200, Philipp Zabel wrote:
Am Mittwoch, den 20.05.2015, 17:07 +0300 schrieb Ville Syrjälä:
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.
Yes I just noticed that too, for example via:
drm_fb_cma_create -> drm_fb_cma_alloc -> drm_helper_mode_fill_fb_struct -> drm_fb_get_bpp_depth
Hm right, so perhaps just a comment then? -Daniel