Hello,
Various pieces of information about DRM formats (number of planes, color depth, chroma subsampling, ...) are scattered across different helper functions in the DRM core. Callers of those functions often need to access more than a single parameter of the format, leading to inefficiencies due to multiple lookups.
This patch series addresses this issue by centralizing all format information in a single data structure (1/4). It reimplements the existing format helper functions based on that structure (3/4) and converts the DRM core code to use the new structure (4/4). Two unused format helper functions are removed in the process (2/4).
The new API is also useful for drivers. I will shortly post a patch series for the omapdrm driver that makes use of it.
Laurent Pinchart (4): drm: Centralize format information drm: Remove unused drm_format_plane_(width|height) helpers drm: Implement the drm_format_*() helpers as drm_format_info() wrappers drm: Use drm_format_info() in DRM core code
drivers/gpu/drm/drm_crtc.c | 391 +++++++++++------------------------- drivers/gpu/drm/drm_fb_cma_helper.c | 23 ++- include/drm/drm_crtc.h | 23 ++- 3 files changed, 153 insertions(+), 284 deletions(-)
Various pieces of information about DRM formats (number of planes, color depth, chroma subsampling, ...) are scattered across different helper functions in the DRM core. Callers of those functions often need to access more than a single parameter of the format, leading to inefficiencies due to multiple lookups.
Centralize all format information in a data structure and create a function to look up information based on the format 4CC.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/drm_crtc.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 21 ++++++++++++ 2 files changed, 104 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 0e3cc66aa8b7..74b0c6dd80cd 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5544,6 +5544,89 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev, }
/** + * drm_format_info - information for a given format + * @format: pixel format (DRM_FORMAT_*) + * + * Returns: + * The instance of struct drm_format_info that describes the pixel format, or + * NULL if the format is unsupported. + */ +const struct drm_format_info *drm_format_info(u32 format) +{ + static const struct drm_format_info formats[] = { + { DRM_FORMAT_C8, 8, 8, 1, { 1 }, 1, 1 }, + { DRM_FORMAT_RGB332, 8, 8, 1, { 1 }, 1, 1 }, + { DRM_FORMAT_BGR233, 8, 8, 1, { 1 }, 1, 1 }, + { DRM_FORMAT_XRGB4444, 12, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_XBGR4444, 12, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_RGBX4444, 12, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_BGRX4444, 12, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_ARGB4444, 12, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_ABGR4444, 12, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_RGBA4444, 12, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_BGRA4444, 12, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_XRGB1555, 15, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_XBGR1555, 15, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_RGBX5551, 15, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_BGRX5551, 15, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_ARGB1555, 15, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_ABGR1555, 15, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_RGBA5551, 15, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_BGRA5551, 15, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_RGB565, 16, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_BGR565, 16, 16, 1, { 2 }, 1, 1 }, + { DRM_FORMAT_RGB888, 24, 24, 1, { 3 }, 1, 1 }, + { DRM_FORMAT_BGR888, 24, 24, 1, { 3 }, 1, 1 }, + { DRM_FORMAT_XRGB8888, 24, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_XBGR8888, 24, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_RGBX8888, 24, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_BGRX8888, 24, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_XRGB2101010, 30, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_XBGR2101010, 30, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_RGBX1010102, 30, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_BGRX1010102, 30, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_ARGB2101010, 30, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_ABGR2101010, 30, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_RGBA1010102, 30, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_BGRA1010102, 30, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_ARGB8888, 32, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_ABGR8888, 32, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_RGBA8888, 32, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_BGRA8888, 32, 32, 1, { 4 }, 1, 1 }, + { DRM_FORMAT_YUV410, 0, 0, 3, { 1, 1, 1 }, 4, 4 }, + { DRM_FORMAT_YVU410, 0, 0, 3, { 1, 1, 1 }, 4, 4 }, + { DRM_FORMAT_YUV411, 0, 0, 3, { 1, 1, 1 }, 4, 1 }, + { DRM_FORMAT_YVU411, 0, 0, 3, { 1, 1, 1 }, 4, 1 }, + { DRM_FORMAT_YUV420, 0, 0, 3, { 1, 1, 1 }, 2, 2 }, + { DRM_FORMAT_YVU420, 0, 0, 3, { 1, 1, 1 }, 2, 2 }, + { DRM_FORMAT_YUV422, 0, 0, 3, { 1, 1, 1 }, 2, 1 }, + { DRM_FORMAT_YVU422, 0, 0, 3, { 1, 1, 1 }, 2, 1 }, + { DRM_FORMAT_YUV444, 0, 0, 3, { 1, 1, 1 }, 1, 1 }, + { DRM_FORMAT_YVU444, 0, 0, 3, { 1, 1, 1 }, 1, 1 }, + { DRM_FORMAT_NV12, 0, 0, 2, { 1, 2 }, 2, 2 }, + { DRM_FORMAT_NV21, 0, 0, 2, { 1, 2 }, 2, 2 }, + { DRM_FORMAT_NV16, 0, 0, 2, { 1, 2 }, 2, 1 }, + { DRM_FORMAT_NV61, 0, 0, 2, { 1, 2 }, 2, 1 }, + { DRM_FORMAT_NV24, 0, 0, 2, { 1, 2 }, 1, 1 }, + { DRM_FORMAT_NV42, 0, 0, 2, { 1, 2 }, 1, 1 }, + { DRM_FORMAT_YUYV, 0, 0, 1, { 2 }, 2, 1 }, + { DRM_FORMAT_YVYU, 0, 0, 1, { 2 }, 2, 1 }, + { DRM_FORMAT_UYVY, 0, 0, 1, { 2 }, 2, 1 }, + { DRM_FORMAT_VYUY, 0, 0, 1, { 2 }, 2, 1 }, + { DRM_FORMAT_AYUV, 0, 0, 1, { 4 }, 1, 1 }, + }; + + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(formats); ++i) { + if (formats[i].format == format) + return &formats[i]; + } + + return NULL; +} + +/** * drm_fb_get_bpp_depth - get the bpp/depth values for format * @format: pixel format (DRM_FORMAT_*) * @depth: storage for the depth value diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index d1559cd04e3d..4199794cc317 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -155,6 +155,26 @@ struct drm_display_info { u8 cea_rev; };
+/** + * struct drm_format_info - information about a DRM format + * @format: 4CC format identifier (DRM_FORMAT_*) + * @depth: color depth (number of bits per pixel excluding padding bits) + * @bpp: number of bits per pixel including padding + * @num_planes: number of color planes (1 to 3) + * @cpp: number of bytes per pixel (per plane) + * @hsub: horizontal chroma subsampling factor + * @vsub: vertical chroma subsampling factor + */ +struct drm_format_info { + u32 format; + unsigned int depth; + unsigned int bpp; + unsigned int num_planes; + unsigned int cpp[3]; + unsigned int hsub; + unsigned int vsub; +}; + /* data corresponds to displayid vend/prod/serial */ struct drm_tile_group { struct kref refcount; @@ -2540,6 +2560,7 @@ extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane, extern int drm_mode_atomic_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv);
+extern const struct drm_format_info *drm_format_info(u32 format); extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp); extern int drm_format_num_planes(uint32_t format);
On 07/06/16 02:33, Laurent Pinchart wrote:
+/**
- struct drm_format_info - information about a DRM format
- @format: 4CC format identifier (DRM_FORMAT_*)
- @depth: color depth (number of bits per pixel excluding padding bits)
- @bpp: number of bits per pixel including padding
- @num_planes: number of color planes (1 to 3)
- @cpp: number of bytes per pixel (per plane)
- @hsub: horizontal chroma subsampling factor
- @vsub: vertical chroma subsampling factor
- */
+struct drm_format_info {
- u32 format;
- unsigned int depth;
- unsigned int bpp;
- unsigned int num_planes;
- unsigned int cpp[3];
- unsigned int hsub;
- unsigned int vsub;
+};
Any reason not to pack this a bit? All those unsigned ints would fit easily into u8.
Tomi
Hi Tomi,
On Tuesday 07 Jun 2016 12:18:34 Tomi Valkeinen wrote:
On 07/06/16 02:33, Laurent Pinchart wrote:
+/**
- struct drm_format_info - information about a DRM format
- @format: 4CC format identifier (DRM_FORMAT_*)
- @depth: color depth (number of bits per pixel excluding padding bits)
- @bpp: number of bits per pixel including padding
- @num_planes: number of color planes (1 to 3)
- @cpp: number of bytes per pixel (per plane)
- @hsub: horizontal chroma subsampling factor
- @vsub: vertical chroma subsampling factor
- */
+struct drm_format_info {
- u32 format;
- unsigned int depth;
- unsigned int bpp;
- unsigned int num_planes;
- unsigned int cpp[3];
- unsigned int hsub;
- unsigned int vsub;
+};
Any reason not to pack this a bit? All those unsigned ints would fit easily into u8.
Good point, I'll do that.
On 07/06/16 02:33, Laurent Pinchart wrote:
Various pieces of information about DRM formats (number of planes, color depth, chroma subsampling, ...) are scattered across different helper functions in the DRM core. Callers of those functions often need to access more than a single parameter of the format, leading to inefficiencies due to multiple lookups.
Centralize all format information in a data structure and create a function to look up information based on the format 4CC.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/gpu/drm/drm_crtc.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 21 ++++++++++++ 2 files changed, 104 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 0e3cc66aa8b7..74b0c6dd80cd 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5544,6 +5544,89 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev, }
/**
- drm_format_info - information for a given format
- @format: pixel format (DRM_FORMAT_*)
- Returns:
- The instance of struct drm_format_info that describes the pixel format, or
- NULL if the format is unsupported.
- */
+const struct drm_format_info *drm_format_info(u32 format) +{
- static const struct drm_format_info formats[] = {
{ DRM_FORMAT_C8, 8, 8, 1, { 1 }, 1, 1 },{ DRM_FORMAT_RGB332, 8, 8, 1, { 1 }, 1, 1 },{ DRM_FORMAT_BGR233, 8, 8, 1, { 1 }, 1, 1 },{ DRM_FORMAT_XRGB4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_XBGR4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBX4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRX4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ARGB4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ABGR4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBA4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRA4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_XRGB1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_XBGR1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBX5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRX5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ARGB1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ABGR1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBA5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRA5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGB565, 16, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGR565, 16, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGB888, 24, 24, 1, { 3 }, 1, 1 },{ DRM_FORMAT_BGR888, 24, 24, 1, { 3 }, 1, 1 },{ DRM_FORMAT_XRGB8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_XBGR8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBX8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRX8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_XRGB2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_XBGR2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBX1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRX1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ARGB2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ABGR2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBA1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRA1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ARGB8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ABGR8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBA8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRA8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_YUV410, 0, 0, 3, { 1, 1, 1 }, 4, 4 },{ DRM_FORMAT_YVU410, 0, 0, 3, { 1, 1, 1 }, 4, 4 },{ DRM_FORMAT_YUV411, 0, 0, 3, { 1, 1, 1 }, 4, 1 },{ DRM_FORMAT_YVU411, 0, 0, 3, { 1, 1, 1 }, 4, 1 },{ DRM_FORMAT_YUV420, 0, 0, 3, { 1, 1, 1 }, 2, 2 },{ DRM_FORMAT_YVU420, 0, 0, 3, { 1, 1, 1 }, 2, 2 },{ DRM_FORMAT_YUV422, 0, 0, 3, { 1, 1, 1 }, 2, 1 },{ DRM_FORMAT_YVU422, 0, 0, 3, { 1, 1, 1 }, 2, 1 },{ DRM_FORMAT_YUV444, 0, 0, 3, { 1, 1, 1 }, 1, 1 },{ DRM_FORMAT_YVU444, 0, 0, 3, { 1, 1, 1 }, 1, 1 },{ DRM_FORMAT_NV12, 0, 0, 2, { 1, 2 }, 2, 2 },{ DRM_FORMAT_NV21, 0, 0, 2, { 1, 2 }, 2, 2 },{ DRM_FORMAT_NV16, 0, 0, 2, { 1, 2 }, 2, 1 },{ DRM_FORMAT_NV61, 0, 0, 2, { 1, 2 }, 2, 1 },{ DRM_FORMAT_NV24, 0, 0, 2, { 1, 2 }, 1, 1 },{ DRM_FORMAT_NV42, 0, 0, 2, { 1, 2 }, 1, 1 },{ DRM_FORMAT_YUYV, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_YVYU, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_UYVY, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_VYUY, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_AYUV, 0, 0, 1, { 4 }, 1, 1 },- };
- unsigned int i;
- for (i = 0; i < ARRAY_SIZE(formats); ++i) {
if (formats[i].format == format)return &formats[i];- }
- return NULL;
After looking at the third patch, I wonder if it would make sense to give a warning here if the format was not found. In the third patch many of the helpers will quietly return a valid value for unknown modes. Which is what they do at the moment too, but is there ever a valid reason to do that without something being wrong?
Tomi
Hi Tomi,
On Tuesday 07 Jun 2016 12:25:08 Tomi Valkeinen wrote:
On 07/06/16 02:33, Laurent Pinchart wrote:
Various pieces of information about DRM formats (number of planes, color depth, chroma subsampling, ...) are scattered across different helper functions in the DRM core. Callers of those functions often need to access more than a single parameter of the format, leading to inefficiencies due to multiple lookups.
Centralize all format information in a data structure and create a function to look up information based on the format 4CC.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/gpu/drm/drm_crtc.c | 83 +++++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 21 ++++++++++++ 2 files changed, 104 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 0e3cc66aa8b7..74b0c6dd80cd 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5544,6 +5544,89 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,> }
/**
- drm_format_info - information for a given format
- @format: pixel format (DRM_FORMAT_*)
- Returns:
- The instance of struct drm_format_info that describes the pixel
format, or + * NULL if the format is unsupported.
- */
+const struct drm_format_info *drm_format_info(u32 format) +{
- static const struct drm_format_info formats[] = {
{ DRM_FORMAT_C8, 8, 8, 1, { 1 }, 1, 1 },{ DRM_FORMAT_RGB332, 8, 8, 1, { 1 }, 1, 1 },{ DRM_FORMAT_BGR233, 8, 8, 1, { 1 }, 1, 1 },{ DRM_FORMAT_XRGB4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_XBGR4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBX4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRX4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ARGB4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ABGR4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBA4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRA4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_XRGB1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_XBGR1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBX5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRX5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ARGB1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ABGR1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBA5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRA5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGB565, 16, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGR565, 16, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGB888, 24, 24, 1, { 3 }, 1, 1 },{ DRM_FORMAT_BGR888, 24, 24, 1, { 3 }, 1, 1 },{ DRM_FORMAT_XRGB8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_XBGR8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBX8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRX8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_XRGB2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_XBGR2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBX1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRX1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ARGB2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ABGR2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBA1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRA1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ARGB8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ABGR8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBA8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRA8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_YUV410, 0, 0, 3, { 1, 1, 1 }, 4, 4 },{ DRM_FORMAT_YVU410, 0, 0, 3, { 1, 1, 1 }, 4, 4 },{ DRM_FORMAT_YUV411, 0, 0, 3, { 1, 1, 1 }, 4, 1 },{ DRM_FORMAT_YVU411, 0, 0, 3, { 1, 1, 1 }, 4, 1 },{ DRM_FORMAT_YUV420, 0, 0, 3, { 1, 1, 1 }, 2, 2 },{ DRM_FORMAT_YVU420, 0, 0, 3, { 1, 1, 1 }, 2, 2 },{ DRM_FORMAT_YUV422, 0, 0, 3, { 1, 1, 1 }, 2, 1 },{ DRM_FORMAT_YVU422, 0, 0, 3, { 1, 1, 1 }, 2, 1 },{ DRM_FORMAT_YUV444, 0, 0, 3, { 1, 1, 1 }, 1, 1 },{ DRM_FORMAT_YVU444, 0, 0, 3, { 1, 1, 1 }, 1, 1 },{ DRM_FORMAT_NV12, 0, 0, 2, { 1, 2 }, 2, 2 },{ DRM_FORMAT_NV21, 0, 0, 2, { 1, 2 }, 2, 2 },{ DRM_FORMAT_NV16, 0, 0, 2, { 1, 2 }, 2, 1 },{ DRM_FORMAT_NV61, 0, 0, 2, { 1, 2 }, 2, 1 },{ DRM_FORMAT_NV24, 0, 0, 2, { 1, 2 }, 1, 1 },{ DRM_FORMAT_NV42, 0, 0, 2, { 1, 2 }, 1, 1 },{ DRM_FORMAT_YUYV, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_YVYU, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_UYVY, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_VYUY, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_AYUV, 0, 0, 1, { 4 }, 1, 1 },- };
- unsigned int i;
- for (i = 0; i < ARRAY_SIZE(formats); ++i) {
if (formats[i].format == format)return &formats[i];- }
- return NULL;
After looking at the third patch, I wonder if it would make sense to give a warning here if the format was not found. In the third patch many of the helpers will quietly return a valid value for unknown modes. Which is what they do at the moment too, but is there ever a valid reason to do that without something being wrong?
We can't warn in this function due to its usage in framebuffer_check() that uses drm_format_info() to validate the format provided by userspace. However, I agree that all other call sites should not pass an unsupported format, that would be a bug in the caller.
I can rename the function to __drm_format_info(), call that in framebuffer_check(), and create a drm_format_info() wrapper with a WARN_ON if the format isn't found. Would you prefer that ?
On Tue, Jun 07, 2016 at 02:33:11AM +0300, Laurent Pinchart wrote:
Various pieces of information about DRM formats (number of planes, color depth, chroma subsampling, ...) are scattered across different helper functions in the DRM core. Callers of those functions often need to access more than a single parameter of the format, leading to inefficiencies due to multiple lookups.
Centralize all format information in a data structure and create a function to look up information based on the format 4CC.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/gpu/drm/drm_crtc.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 21 ++++++++++++ 2 files changed, 104 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 0e3cc66aa8b7..74b0c6dd80cd 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5544,6 +5544,89 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev, }
/**
- drm_format_info - information for a given format
- @format: pixel format (DRM_FORMAT_*)
- Returns:
- The instance of struct drm_format_info that describes the pixel format, or
- NULL if the format is unsupported.
- */
+const struct drm_format_info *drm_format_info(u32 format) +{
- static const struct drm_format_info formats[] = {
{ DRM_FORMAT_C8, 8, 8, 1, { 1 }, 1, 1 },
Named initializers please.
{ DRM_FORMAT_RGB332, 8, 8, 1, { 1 }, 1, 1 },{ DRM_FORMAT_BGR233, 8, 8, 1, { 1 }, 1, 1 },{ DRM_FORMAT_XRGB4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_XBGR4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBX4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRX4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ARGB4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ABGR4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBA4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRA4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_XRGB1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_XBGR1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBX5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRX5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ARGB1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ABGR1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBA5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRA5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGB565, 16, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGR565, 16, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGB888, 24, 24, 1, { 3 }, 1, 1 },{ DRM_FORMAT_BGR888, 24, 24, 1, { 3 }, 1, 1 },{ DRM_FORMAT_XRGB8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_XBGR8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBX8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRX8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_XRGB2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_XBGR2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBX1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRX1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ARGB2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ABGR2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBA1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRA1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ARGB8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ABGR8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBA8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRA8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_YUV410, 0, 0, 3, { 1, 1, 1 }, 4, 4 },{ DRM_FORMAT_YVU410, 0, 0, 3, { 1, 1, 1 }, 4, 4 },{ DRM_FORMAT_YUV411, 0, 0, 3, { 1, 1, 1 }, 4, 1 },{ DRM_FORMAT_YVU411, 0, 0, 3, { 1, 1, 1 }, 4, 1 },{ DRM_FORMAT_YUV420, 0, 0, 3, { 1, 1, 1 }, 2, 2 },{ DRM_FORMAT_YVU420, 0, 0, 3, { 1, 1, 1 }, 2, 2 },{ DRM_FORMAT_YUV422, 0, 0, 3, { 1, 1, 1 }, 2, 1 },{ DRM_FORMAT_YVU422, 0, 0, 3, { 1, 1, 1 }, 2, 1 },{ DRM_FORMAT_YUV444, 0, 0, 3, { 1, 1, 1 }, 1, 1 },{ DRM_FORMAT_YVU444, 0, 0, 3, { 1, 1, 1 }, 1, 1 },{ DRM_FORMAT_NV12, 0, 0, 2, { 1, 2 }, 2, 2 },{ DRM_FORMAT_NV21, 0, 0, 2, { 1, 2 }, 2, 2 },{ DRM_FORMAT_NV16, 0, 0, 2, { 1, 2 }, 2, 1 },{ DRM_FORMAT_NV61, 0, 0, 2, { 1, 2 }, 2, 1 },{ DRM_FORMAT_NV24, 0, 0, 2, { 1, 2 }, 1, 1 },{ DRM_FORMAT_NV42, 0, 0, 2, { 1, 2 }, 1, 1 },{ DRM_FORMAT_YUYV, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_YVYU, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_UYVY, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_VYUY, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_AYUV, 0, 0, 1, { 4 }, 1, 1 },- };
- unsigned int i;
- for (i = 0; i < ARRAY_SIZE(formats); ++i) {
if (formats[i].format == format)return &formats[i];- }
- return NULL;
+}
+/**
- drm_fb_get_bpp_depth - get the bpp/depth values for format
- @format: pixel format (DRM_FORMAT_*)
- @depth: storage for the depth value
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index d1559cd04e3d..4199794cc317 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -155,6 +155,26 @@ struct drm_display_info { u8 cea_rev; };
+/**
- struct drm_format_info - information about a DRM format
- @format: 4CC format identifier (DRM_FORMAT_*)
- @depth: color depth (number of bits per pixel excluding padding bits)
- @bpp: number of bits per pixel including padding
- @num_planes: number of color planes (1 to 3)
- @cpp: number of bytes per pixel (per plane)
- @hsub: horizontal chroma subsampling factor
- @vsub: vertical chroma subsampling factor
- */
+struct drm_format_info {
- u32 format;
- unsigned int depth;
- unsigned int bpp;
- unsigned int num_planes;
- unsigned int cpp[3];
- unsigned int hsub;
- unsigned int vsub;
+};
/* data corresponds to displayid vend/prod/serial */ struct drm_tile_group { struct kref refcount; @@ -2540,6 +2560,7 @@ extern int drm_mode_plane_set_obj_prop(struct drm_plane *plane, extern int drm_mode_atomic_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv);
+extern const struct drm_format_info *drm_format_info(u32 format); extern void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp); extern int drm_format_num_planes(uint32_t format); -- Regards,
Laurent Pinchart
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Ville,
On Tuesday 07 Jun 2016 16:20:17 Ville Syrjälä wrote:
On Tue, Jun 07, 2016 at 02:33:11AM +0300, Laurent Pinchart wrote:
Various pieces of information about DRM formats (number of planes, color depth, chroma subsampling, ...) are scattered across different helper functions in the DRM core. Callers of those functions often need to access more than a single parameter of the format, leading to inefficiencies due to multiple lookups.
Centralize all format information in a data structure and create a function to look up information based on the format 4CC.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/gpu/drm/drm_crtc.c | 83 +++++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 21 ++++++++++++ 2 files changed, 104 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 0e3cc66aa8b7..74b0c6dd80cd 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5544,6 +5544,89 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,> }
/**
- drm_format_info - information for a given format
- @format: pixel format (DRM_FORMAT_*)
- Returns:
- The instance of struct drm_format_info that describes the pixel
format, or + * NULL if the format is unsupported.
- */
+const struct drm_format_info *drm_format_info(u32 format) +{
- static const struct drm_format_info formats[] = {
{ DRM_FORMAT_C8, 8, 8, 1, { 1 }, 1, 1 },Named initializers please.
Do we really want to expand every entry to 7 lines ? I can do so, but it won't look pretty.
{ DRM_FORMAT_RGB332, 8, 8, 1, { 1 }, 1, 1 },{ DRM_FORMAT_BGR233, 8, 8, 1, { 1 }, 1, 1 },{ DRM_FORMAT_XRGB4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_XBGR4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBX4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRX4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ARGB4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ABGR4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBA4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRA4444, 12, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_XRGB1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_XBGR1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBX5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRX5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ARGB1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_ABGR1555, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGBA5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGRA5551, 15, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGB565, 16, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_BGR565, 16, 16, 1, { 2 }, 1, 1 },{ DRM_FORMAT_RGB888, 24, 24, 1, { 3 }, 1, 1 },{ DRM_FORMAT_BGR888, 24, 24, 1, { 3 }, 1, 1 },{ DRM_FORMAT_XRGB8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_XBGR8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBX8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRX8888, 24, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_XRGB2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_XBGR2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBX1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRX1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ARGB2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ABGR2101010, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBA1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRA1010102, 30, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ARGB8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_ABGR8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_RGBA8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_BGRA8888, 32, 32, 1, { 4 }, 1, 1 },{ DRM_FORMAT_YUV410, 0, 0, 3, { 1, 1, 1 }, 4, 4 },{ DRM_FORMAT_YVU410, 0, 0, 3, { 1, 1, 1 }, 4, 4 },{ DRM_FORMAT_YUV411, 0, 0, 3, { 1, 1, 1 }, 4, 1 },{ DRM_FORMAT_YVU411, 0, 0, 3, { 1, 1, 1 }, 4, 1 },{ DRM_FORMAT_YUV420, 0, 0, 3, { 1, 1, 1 }, 2, 2 },{ DRM_FORMAT_YVU420, 0, 0, 3, { 1, 1, 1 }, 2, 2 },{ DRM_FORMAT_YUV422, 0, 0, 3, { 1, 1, 1 }, 2, 1 },{ DRM_FORMAT_YVU422, 0, 0, 3, { 1, 1, 1 }, 2, 1 },{ DRM_FORMAT_YUV444, 0, 0, 3, { 1, 1, 1 }, 1, 1 },{ DRM_FORMAT_YVU444, 0, 0, 3, { 1, 1, 1 }, 1, 1 },{ DRM_FORMAT_NV12, 0, 0, 2, { 1, 2 }, 2, 2 },{ DRM_FORMAT_NV21, 0, 0, 2, { 1, 2 }, 2, 2 },{ DRM_FORMAT_NV16, 0, 0, 2, { 1, 2 }, 2, 1 },{ DRM_FORMAT_NV61, 0, 0, 2, { 1, 2 }, 2, 1 },{ DRM_FORMAT_NV24, 0, 0, 2, { 1, 2 }, 1, 1 },{ DRM_FORMAT_NV42, 0, 0, 2, { 1, 2 }, 1, 1 },{ DRM_FORMAT_YUYV, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_YVYU, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_UYVY, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_VYUY, 0, 0, 1, { 2 }, 2, 1 },{ DRM_FORMAT_AYUV, 0, 0, 1, { 4 }, 1, 1 },- };
- unsigned int i;
- for (i = 0; i < ARRAY_SIZE(formats); ++i) {
if (formats[i].format == format)return &formats[i];- }
- return NULL;
+}
The drm_format_plane_width() and drm_format_plane_height() helper functions are not used, remove them.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/drm_crtc.c | 42 ------------------------------------------ include/drm/drm_crtc.h | 2 -- 2 files changed, 44 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 74b0c6dd80cd..b405d4379e47 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5843,48 +5843,6 @@ int drm_format_vert_chroma_subsampling(uint32_t format) EXPORT_SYMBOL(drm_format_vert_chroma_subsampling);
/** - * drm_format_plane_width - width of the plane given the first plane - * @width: width of the first plane - * @format: pixel format - * @plane: plane index - * - * Returns: - * The width of @plane, given that the width of the first plane is @width. - */ -int drm_format_plane_width(int width, uint32_t format, int plane) -{ - if (plane >= drm_format_num_planes(format)) - return 0; - - if (plane == 0) - return width; - - return width / drm_format_horz_chroma_subsampling(format); -} -EXPORT_SYMBOL(drm_format_plane_width); - -/** - * drm_format_plane_height - height of the plane given the first plane - * @height: height of the first plane - * @format: pixel format - * @plane: plane index - * - * Returns: - * The height of @plane, given that the height of the first plane is @height. - */ -int drm_format_plane_height(int height, uint32_t format, int plane) -{ - if (plane >= drm_format_num_planes(format)) - return 0; - - if (plane == 0) - return height; - - return height / drm_format_vert_chroma_subsampling(format); -} -EXPORT_SYMBOL(drm_format_plane_height); - -/** * drm_rotation_simplify() - Try to simplify the rotation * @rotation: Rotation to be simplified * @supported_rotations: Supported rotations diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 4199794cc317..a45f57f32dca 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -2567,8 +2567,6 @@ extern int drm_format_num_planes(uint32_t format); extern int drm_format_plane_cpp(uint32_t format, int plane); extern int drm_format_horz_chroma_subsampling(uint32_t format); extern int drm_format_vert_chroma_subsampling(uint32_t format); -extern int drm_format_plane_width(int width, uint32_t format, int plane); -extern int drm_format_plane_height(int height, uint32_t format, int plane); extern const char *drm_get_format_name(uint32_t format); extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev, unsigned int supported_rotations);
On Tue, Jun 07, 2016 at 02:33:12AM +0300, Laurent Pinchart wrote:
The drm_format_plane_width() and drm_format_plane_height() helper functions are not used, remove them.
I have a user lined up, assuming I could get the dang thing reviewed.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/gpu/drm/drm_crtc.c | 42 ------------------------------------------ include/drm/drm_crtc.h | 2 -- 2 files changed, 44 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 74b0c6dd80cd..b405d4379e47 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5843,48 +5843,6 @@ int drm_format_vert_chroma_subsampling(uint32_t format) EXPORT_SYMBOL(drm_format_vert_chroma_subsampling);
/**
- drm_format_plane_width - width of the plane given the first plane
- @width: width of the first plane
- @format: pixel format
- @plane: plane index
- Returns:
- The width of @plane, given that the width of the first plane is @width.
- */
-int drm_format_plane_width(int width, uint32_t format, int plane) -{
- if (plane >= drm_format_num_planes(format))
return 0;- if (plane == 0)
return width;- return width / drm_format_horz_chroma_subsampling(format);
-} -EXPORT_SYMBOL(drm_format_plane_width);
-/**
- drm_format_plane_height - height of the plane given the first plane
- @height: height of the first plane
- @format: pixel format
- @plane: plane index
- Returns:
- The height of @plane, given that the height of the first plane is @height.
- */
-int drm_format_plane_height(int height, uint32_t format, int plane) -{
- if (plane >= drm_format_num_planes(format))
return 0;- if (plane == 0)
return height;- return height / drm_format_vert_chroma_subsampling(format);
-} -EXPORT_SYMBOL(drm_format_plane_height);
-/**
- drm_rotation_simplify() - Try to simplify the rotation
- @rotation: Rotation to be simplified
- @supported_rotations: Supported rotations
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 4199794cc317..a45f57f32dca 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -2567,8 +2567,6 @@ extern int drm_format_num_planes(uint32_t format); extern int drm_format_plane_cpp(uint32_t format, int plane); extern int drm_format_horz_chroma_subsampling(uint32_t format); extern int drm_format_vert_chroma_subsampling(uint32_t format); -extern int drm_format_plane_width(int width, uint32_t format, int plane); -extern int drm_format_plane_height(int height, uint32_t format, int plane); extern const char *drm_get_format_name(uint32_t format); extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev, unsigned int supported_rotations); -- Regards,
Laurent Pinchart
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Ville,
On Tuesday 07 Jun 2016 16:19:01 Ville Syrjälä wrote:
On Tue, Jun 07, 2016 at 02:33:12AM +0300, Laurent Pinchart wrote:
The drm_format_plane_width() and drm_format_plane_height() helper functions are not used, remove them.
I have a user lined up, assuming I could get the dang thing reviewed.
OK, I'll keep the functions and convert them to use drm_format_info(). If your user needs to call more than one format helper then I'd suggest using drm_format_info() directly and perform the width/height computation yourself, it will be more efficient.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/gpu/drm/drm_crtc.c | 42 ---------------------------------------- include/drm/drm_crtc.h | 2 -- 2 files changed, 44 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 74b0c6dd80cd..b405d4379e47 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5843,48 +5843,6 @@ int drm_format_vert_chroma_subsampling(uint32_t format)> EXPORT_SYMBOL(drm_format_vert_chroma_subsampling);
/**
- drm_format_plane_width - width of the plane given the first plane
- @width: width of the first plane
- @format: pixel format
- @plane: plane index
- Returns:
- The width of @plane, given that the width of the first plane is
@width.
- */
-int drm_format_plane_width(int width, uint32_t format, int plane) -{
- if (plane >= drm_format_num_planes(format))
return 0;- if (plane == 0)
return width;- return width / drm_format_horz_chroma_subsampling(format);
-} -EXPORT_SYMBOL(drm_format_plane_width);
-/**
- drm_format_plane_height - height of the plane given the first plane
- @height: height of the first plane
- @format: pixel format
- @plane: plane index
- Returns:
- The height of @plane, given that the height of the first plane is
@height. - */ -int drm_format_plane_height(int height, uint32_t format, int plane) -{
- if (plane >= drm_format_num_planes(format))
return 0;- if (plane == 0)
return height;- return height / drm_format_vert_chroma_subsampling(format);
-} -EXPORT_SYMBOL(drm_format_plane_height);
-/**
- drm_rotation_simplify() - Try to simplify the rotation
- @rotation: Rotation to be simplified
- @supported_rotations: Supported rotations
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 4199794cc317..a45f57f32dca 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -2567,8 +2567,6 @@ extern int drm_format_num_planes(uint32_t format);
extern int drm_format_plane_cpp(uint32_t format, int plane); extern int drm_format_horz_chroma_subsampling(uint32_t format); extern int drm_format_vert_chroma_subsampling(uint32_t format);
-extern int drm_format_plane_width(int width, uint32_t format, int plane); -extern int drm_format_plane_height(int height, uint32_t format, int plane);> extern const char *drm_get_format_name(uint32_t format); extern struct drm_property *drm_mode_create_rotation_property(struct drm_device *dev,> unsigned int supported_rotations);
Turn the drm_format_*() helpers into wrappers around the drm_format_info lookup function to centralize all format information in a single place.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/drm_crtc.c | 166 +++++++-------------------------------------- 1 file changed, 24 insertions(+), 142 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index b405d4379e47..d8215fdcaeb6 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5638,66 +5638,19 @@ const struct drm_format_info *drm_format_info(u32 format) void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp) { - switch (format) { - case DRM_FORMAT_C8: - case DRM_FORMAT_RGB332: - case DRM_FORMAT_BGR233: - *depth = 8; - *bpp = 8; - break; - case DRM_FORMAT_XRGB1555: - case DRM_FORMAT_XBGR1555: - case DRM_FORMAT_RGBX5551: - case DRM_FORMAT_BGRX5551: - case DRM_FORMAT_ARGB1555: - case DRM_FORMAT_ABGR1555: - case DRM_FORMAT_RGBA5551: - case DRM_FORMAT_BGRA5551: - *depth = 15; - *bpp = 16; - break; - case DRM_FORMAT_RGB565: - case DRM_FORMAT_BGR565: - *depth = 16; - *bpp = 16; - break; - case DRM_FORMAT_RGB888: - case DRM_FORMAT_BGR888: - *depth = 24; - *bpp = 24; - break; - case DRM_FORMAT_XRGB8888: - case DRM_FORMAT_XBGR8888: - case DRM_FORMAT_RGBX8888: - case DRM_FORMAT_BGRX8888: - *depth = 24; - *bpp = 32; - break; - case DRM_FORMAT_XRGB2101010: - case DRM_FORMAT_XBGR2101010: - case DRM_FORMAT_RGBX1010102: - case DRM_FORMAT_BGRX1010102: - case DRM_FORMAT_ARGB2101010: - case DRM_FORMAT_ABGR2101010: - case DRM_FORMAT_RGBA1010102: - case DRM_FORMAT_BGRA1010102: - *depth = 30; - *bpp = 32; - break; - case DRM_FORMAT_ARGB8888: - case DRM_FORMAT_ABGR8888: - case DRM_FORMAT_RGBA8888: - case DRM_FORMAT_BGRA8888: - *depth = 32; - *bpp = 32; - break; - default: + const struct drm_format_info *info; + + info = drm_format_info(format); + if (!info || !info->depth || !info->bpp) { DRM_DEBUG_KMS("unsupported pixel format %s\n", drm_get_format_name(format)); *depth = 0; *bpp = 0; - break; + return; } + + *depth = info->depth; + *bpp = info->bpp; } EXPORT_SYMBOL(drm_fb_get_bpp_depth);
@@ -5710,28 +5663,10 @@ EXPORT_SYMBOL(drm_fb_get_bpp_depth); */ int drm_format_num_planes(uint32_t format) { - switch (format) { - case DRM_FORMAT_YUV410: - case DRM_FORMAT_YVU410: - case DRM_FORMAT_YUV411: - case DRM_FORMAT_YVU411: - case DRM_FORMAT_YUV420: - case DRM_FORMAT_YVU420: - case DRM_FORMAT_YUV422: - case DRM_FORMAT_YVU422: - case DRM_FORMAT_YUV444: - case DRM_FORMAT_YVU444: - return 3; - case DRM_FORMAT_NV12: - case DRM_FORMAT_NV21: - case DRM_FORMAT_NV16: - case DRM_FORMAT_NV61: - case DRM_FORMAT_NV24: - case DRM_FORMAT_NV42: - return 2; - default: - return 1; - } + const struct drm_format_info *info; + + info = drm_format_info(format); + return info ? info->num_planes : 1; } EXPORT_SYMBOL(drm_format_num_planes);
@@ -5745,40 +5680,13 @@ EXPORT_SYMBOL(drm_format_num_planes); */ int drm_format_plane_cpp(uint32_t format, int plane) { - unsigned int depth; - int bpp; + const struct drm_format_info *info;
- if (plane >= drm_format_num_planes(format)) + info = drm_format_info(format); + if (!info || plane >= info->num_planes) return 0;
- switch (format) { - case DRM_FORMAT_YUYV: - case DRM_FORMAT_YVYU: - case DRM_FORMAT_UYVY: - case DRM_FORMAT_VYUY: - return 2; - case DRM_FORMAT_NV12: - case DRM_FORMAT_NV21: - case DRM_FORMAT_NV16: - case DRM_FORMAT_NV61: - case DRM_FORMAT_NV24: - case DRM_FORMAT_NV42: - return plane ? 2 : 1; - case DRM_FORMAT_YUV410: - case DRM_FORMAT_YVU410: - case DRM_FORMAT_YUV411: - case DRM_FORMAT_YVU411: - case DRM_FORMAT_YUV420: - case DRM_FORMAT_YVU420: - case DRM_FORMAT_YUV422: - case DRM_FORMAT_YVU422: - case DRM_FORMAT_YUV444: - case DRM_FORMAT_YVU444: - return 1; - default: - drm_fb_get_bpp_depth(format, &depth, &bpp); - return bpp >> 3; - } + return info->cpp[plane]; } EXPORT_SYMBOL(drm_format_plane_cpp);
@@ -5792,28 +5700,10 @@ EXPORT_SYMBOL(drm_format_plane_cpp); */ int drm_format_horz_chroma_subsampling(uint32_t format) { - switch (format) { - case DRM_FORMAT_YUV411: - case DRM_FORMAT_YVU411: - case DRM_FORMAT_YUV410: - case DRM_FORMAT_YVU410: - return 4; - case DRM_FORMAT_YUYV: - case DRM_FORMAT_YVYU: - case DRM_FORMAT_UYVY: - case DRM_FORMAT_VYUY: - case DRM_FORMAT_NV12: - case DRM_FORMAT_NV21: - case DRM_FORMAT_NV16: - case DRM_FORMAT_NV61: - case DRM_FORMAT_YUV422: - case DRM_FORMAT_YVU422: - case DRM_FORMAT_YUV420: - case DRM_FORMAT_YVU420: - return 2; - default: - return 1; - } + const struct drm_format_info *info; + + info = drm_format_info(format); + return info ? info->hsub : 1; } EXPORT_SYMBOL(drm_format_horz_chroma_subsampling);
@@ -5827,18 +5717,10 @@ EXPORT_SYMBOL(drm_format_horz_chroma_subsampling); */ int drm_format_vert_chroma_subsampling(uint32_t format) { - switch (format) { - case DRM_FORMAT_YUV410: - case DRM_FORMAT_YVU410: - return 4; - case DRM_FORMAT_YUV420: - case DRM_FORMAT_YVU420: - case DRM_FORMAT_NV12: - case DRM_FORMAT_NV21: - return 2; - default: - return 1; - } + const struct drm_format_info *info; + + info = drm_format_info(format); + return info ? info->vsub : 1; } EXPORT_SYMBOL(drm_format_vert_chroma_subsampling);
Replace calls to the drm_format_*() helper functions with direct use of the drm_format_info structure. This improves efficiency by removing duplicate lookups.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/gpu/drm/drm_crtc.c | 100 +++++------------------------------- drivers/gpu/drm/drm_fb_cma_helper.c | 23 +++++---- 2 files changed, 25 insertions(+), 98 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d8215fdcaeb6..2fa458d47c90 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3204,108 +3204,32 @@ int drm_mode_addfb(struct drm_device *dev, return 0; }
-static int format_check(const struct drm_mode_fb_cmd2 *r) -{ - uint32_t format = r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN; - - switch (format) { - case DRM_FORMAT_C8: - case DRM_FORMAT_RGB332: - case DRM_FORMAT_BGR233: - case DRM_FORMAT_XRGB4444: - case DRM_FORMAT_XBGR4444: - case DRM_FORMAT_RGBX4444: - case DRM_FORMAT_BGRX4444: - case DRM_FORMAT_ARGB4444: - case DRM_FORMAT_ABGR4444: - case DRM_FORMAT_RGBA4444: - case DRM_FORMAT_BGRA4444: - case DRM_FORMAT_XRGB1555: - case DRM_FORMAT_XBGR1555: - case DRM_FORMAT_RGBX5551: - case DRM_FORMAT_BGRX5551: - case DRM_FORMAT_ARGB1555: - case DRM_FORMAT_ABGR1555: - case DRM_FORMAT_RGBA5551: - case DRM_FORMAT_BGRA5551: - case DRM_FORMAT_RGB565: - case DRM_FORMAT_BGR565: - case DRM_FORMAT_RGB888: - case DRM_FORMAT_BGR888: - case DRM_FORMAT_XRGB8888: - case DRM_FORMAT_XBGR8888: - case DRM_FORMAT_RGBX8888: - case DRM_FORMAT_BGRX8888: - case DRM_FORMAT_ARGB8888: - case DRM_FORMAT_ABGR8888: - case DRM_FORMAT_RGBA8888: - case DRM_FORMAT_BGRA8888: - case DRM_FORMAT_XRGB2101010: - case DRM_FORMAT_XBGR2101010: - case DRM_FORMAT_RGBX1010102: - case DRM_FORMAT_BGRX1010102: - case DRM_FORMAT_ARGB2101010: - case DRM_FORMAT_ABGR2101010: - case DRM_FORMAT_RGBA1010102: - case DRM_FORMAT_BGRA1010102: - case DRM_FORMAT_YUYV: - case DRM_FORMAT_YVYU: - case DRM_FORMAT_UYVY: - case DRM_FORMAT_VYUY: - case DRM_FORMAT_AYUV: - case DRM_FORMAT_NV12: - case DRM_FORMAT_NV21: - case DRM_FORMAT_NV16: - case DRM_FORMAT_NV61: - case DRM_FORMAT_NV24: - case DRM_FORMAT_NV42: - case DRM_FORMAT_YUV410: - case DRM_FORMAT_YVU410: - case DRM_FORMAT_YUV411: - case DRM_FORMAT_YVU411: - case DRM_FORMAT_YUV420: - case DRM_FORMAT_YVU420: - case DRM_FORMAT_YUV422: - case DRM_FORMAT_YVU422: - case DRM_FORMAT_YUV444: - case DRM_FORMAT_YVU444: - return 0; - default: - DRM_DEBUG_KMS("invalid pixel format %s\n", - drm_get_format_name(r->pixel_format)); - return -EINVAL; - } -} - static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) { - int ret, hsub, vsub, num_planes, i; + const struct drm_format_info *info; + int i;
- ret = format_check(r); - if (ret) { + info = drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN); + if (!info) { DRM_DEBUG_KMS("bad framebuffer format %s\n", drm_get_format_name(r->pixel_format)); - return ret; + return -EINVAL; }
- hsub = drm_format_horz_chroma_subsampling(r->pixel_format); - vsub = drm_format_vert_chroma_subsampling(r->pixel_format); - num_planes = drm_format_num_planes(r->pixel_format); - - if (r->width == 0 || r->width % hsub) { + if (r->width == 0 || r->width % info->hsub) { DRM_DEBUG_KMS("bad framebuffer width %u\n", r->width); return -EINVAL; }
- if (r->height == 0 || r->height % vsub) { + if (r->height == 0 || r->height % info->vsub) { DRM_DEBUG_KMS("bad framebuffer height %u\n", r->height); return -EINVAL; }
- for (i = 0; i < num_planes; i++) { - unsigned int width = r->width / (i != 0 ? hsub : 1); - unsigned int height = r->height / (i != 0 ? vsub : 1); - unsigned int cpp = drm_format_plane_cpp(r->pixel_format, i); + for (i = 0; i < info->num_planes; i++) { + unsigned int width = r->width / (i != 0 ? info->hsub : 1); + unsigned int height = r->height / (i != 0 ? info->vsub : 1); + unsigned int cpp = info->cpp[i];
if (!r->handles[i]) { DRM_DEBUG_KMS("no buffer object handle for plane %d\n", i); @@ -3348,7 +3272,7 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) } }
- for (i = num_planes; i < 4; i++) { + for (i = info->num_planes; i < 4; i++) { if (r->modifier[i]) { DRM_DEBUG_KMS("non-zero modifier for unused plane %d\n", i); return -EINVAL; diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 5075fae3c4e2..81525a23e4b7 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -171,20 +171,20 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev, struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd, const struct drm_framebuffer_funcs *funcs) { + const struct drm_format_info *info; struct drm_fb_cma *fb_cma; struct drm_gem_cma_object *objs[4]; struct drm_gem_object *obj; - unsigned int hsub; - unsigned int vsub; int ret; int i;
- hsub = drm_format_horz_chroma_subsampling(mode_cmd->pixel_format); - vsub = drm_format_vert_chroma_subsampling(mode_cmd->pixel_format); + info = drm_format_info(mode_cmd->pixel_format); + if (!info) + return -EINVAL;
- for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) { - unsigned int width = mode_cmd->width / (i ? hsub : 1); - unsigned int height = mode_cmd->height / (i ? vsub : 1); + for (i = 0; i < info->num_planes; i++) { + unsigned int width = mode_cmd->width / (i ? info->hsub : 1); + unsigned int height = mode_cmd->height / (i ? info->vsub : 1); unsigned int min_size;
obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[i]); @@ -195,7 +195,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev, }
min_size = (height - 1) * mode_cmd->pitches[i] - + width * drm_format_plane_cpp(mode_cmd->pixel_format, i) + + width * info->cpp[i] + mode_cmd->offsets[i];
if (obj->size < min_size) { @@ -265,12 +265,15 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj); static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m) { struct drm_fb_cma *fb_cma = to_fb_cma(fb); - int i, n = drm_format_num_planes(fb->pixel_format); + const struct drm_format_info *info; + int i;
seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height, (char *)&fb->pixel_format);
- for (i = 0; i < n; i++) { + info = drm_format_info(fb->pixel_format); + + for (i = 0; i < info->num_planes; i++) { seq_printf(m, " %d: offset=%d pitch=%d, obj: ", i, fb->offsets[i], fb->pitches[i]); drm_gem_cma_describe(fb_cma->obj[i], m);
On Tue, Jun 07, 2016 at 02:33:10AM +0300, Laurent Pinchart wrote:
Hello,
Various pieces of information about DRM formats (number of planes, color depth, chroma subsampling, ...) are scattered across different helper functions in the DRM core. Callers of those functions often need to access more than a single parameter of the format, leading to inefficiencies due to multiple lookups.
This patch series addresses this issue by centralizing all format information in a single data structure (1/4). It reimplements the existing format helper functions based on that structure (3/4) and converts the DRM core code to use the new structure (4/4). Two unused format helper functions are removed in the process (2/4).
The new API is also useful for drivers. I will shortly post a patch series for the omapdrm driver that makes use of it.
I'm still meh on this, but you could convince me if you'd extract all the format related stuff into drm_fourcc.c. drm_crtc is a mess, and the abi docs are confusing since everything is in one bag. Splitting parts out would be awesome. Other stuff I think we could split out is all the framebuffer handling, basic property stuff, specialized properties (for zorder, blending, whatever) and maybe even a few more sub-topics. But let's start somewhere. -Daniel
Laurent Pinchart (4): drm: Centralize format information drm: Remove unused drm_format_plane_(width|height) helpers drm: Implement the drm_format_*() helpers as drm_format_info() wrappers drm: Use drm_format_info() in DRM core code
drivers/gpu/drm/drm_crtc.c | 391 +++++++++++------------------------- drivers/gpu/drm/drm_fb_cma_helper.c | 23 ++- include/drm/drm_crtc.h | 23 ++- 3 files changed, 153 insertions(+), 284 deletions(-)
-- Regards,
Laurent Pinchart
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Daniel,
On Tuesday 07 Jun 2016 15:27:09 Daniel Vetter wrote:
On Tue, Jun 07, 2016 at 02:33:10AM +0300, Laurent Pinchart wrote:
Hello,
Various pieces of information about DRM formats (number of planes, color depth, chroma subsampling, ...) are scattered across different helper functions in the DRM core. Callers of those functions often need to access more than a single parameter of the format, leading to inefficiencies due to multiple lookups.
This patch series addresses this issue by centralizing all format information in a single data structure (1/4). It reimplements the existing format helper functions based on that structure (3/4) and converts the DRM core code to use the new structure (4/4). Two unused format helper functions are removed in the process (2/4).
The new API is also useful for drivers. I will shortly post a patch series for the omapdrm driver that makes use of it.
I'm still meh on this, but you could convince me if you'd extract all the format related stuff into drm_fourcc.c. drm_crtc is a mess, and the abi docs are confusing since everything is in one bag. Splitting parts out would be awesome.
I think we're on the same page, I had the exact same thought today. I'll work on that.
Other stuff I think we could split out is all the framebuffer handling, basic property stuff, specialized properties (for zorder, blending, whatever) and maybe even a few more sub-topics. But let's start somewhere.
Deal, I'll start here :-)
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Laurent Pinchart -
Tomi Valkeinen -
Ville Syrjälä