| Message ID | 1465428739-26529-3-git-send-email-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 09, 2016 at 02:32:06AM +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_fourcc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++ > include/drm/drm_fourcc.h | 19 ++++++++++ > 2 files changed, 103 insertions(+) > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c > index 0645c85d5f95..47b9abd743be 100644 > --- a/drivers/gpu/drm/drm_fourcc.c > +++ b/drivers/gpu/drm/drm_fourcc.c > @@ -62,6 +62,90 @@ const char *drm_get_format_name(uint32_t format) > EXPORT_SYMBOL(drm_get_format_name); > > /** > + * drm_format_info - query 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) Bikeshed on your pixel format description table. I think the approach I've seen in gallium/mesa to describe pixel formats is a lot more generic, and we might as well adopt it when we change. Idea is to have a block size measure in pixels (both h and v), and then cpp is bytes_per_block. This is essentially what you have with hsub and vsub already, except confusing names, more ill-defined (since it only makes sense for yuv) and less generic. A few examples: h_blocksize v_blocksize bytes_per_block (per-plane) YUV410 4 4 {4, 1, 1} YUV411 4 1 {4, 1, 1} hsub = h_blocksize / bytes_per_block[U/V plane] vsub = v_blocksize / bytes_per_block[U/V plane] *_blocksize is in pixels [not entirely sure on the precise data, but that's kinda the point.] Ofc should maybe check that those helpers are only called on yuv planar formats, too. This way we could also remove some of the comments in drm_fourcc.h, or at least make the more clear. Another benefit is that with this it's possible to write entirely generic size/stride checking functions Oh and if we go with this, some asciiart for what's meant (in sphinx/rst, that will happen in 4.8) would be awesome. -Daniel > +{ > + static const struct drm_format_info formats[] = { > + { .format = DRM_FORMAT_C8, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_RGB332, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_BGR233, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_XRGB4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_XBGR4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_RGBX4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_BGRX4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_ARGB4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_ABGR4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_RGBA4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_BGRA4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_XRGB1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_XBGR1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_RGBX5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_BGRX5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_ARGB1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_ABGR1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_RGBA5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_BGRA5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_RGB565, .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_BGR565, .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_RGB888, .depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_BGR888, .depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_RGBX8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_BGRX8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_XRGB2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_XBGR2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_RGBX1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_BGRX1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_ARGB2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_ABGR2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_RGBA1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_BGRA1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_RGBA8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_BGRA8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_YUV410, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 }, > + { .format = DRM_FORMAT_YVU410, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 }, > + { .format = DRM_FORMAT_YUV411, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 }, > + { .format = DRM_FORMAT_YVU411, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 }, > + { .format = DRM_FORMAT_YUV420, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 }, > + { .format = DRM_FORMAT_YVU420, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 }, > + { .format = DRM_FORMAT_YUV422, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 }, > + { .format = DRM_FORMAT_YVU422, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 }, > + { .format = DRM_FORMAT_YUV444, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_YVU444, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_NV12, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 }, > + { .format = DRM_FORMAT_NV21, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 }, > + { .format = DRM_FORMAT_NV16, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 }, > + { .format = DRM_FORMAT_NV61, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 }, > + { .format = DRM_FORMAT_NV24, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_NV42, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 }, > + { .format = DRM_FORMAT_YUYV, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, > + { .format = DRM_FORMAT_YVYU, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, > + { .format = DRM_FORMAT_UYVY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, > + { .format = DRM_FORMAT_VYUY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, > + { .format = DRM_FORMAT_AYUV, .depth = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > + }; > + > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(formats); ++i) { > + if (formats[i].format == format) > + return &formats[i]; > + } > + > + return NULL; > +} > +EXPORT_SYMBOL(drm_format_info); > + > +/** > * 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_fourcc.h b/include/drm/drm_fourcc.h > index 7f90a396cf2b..b077df507b51 100644 > --- a/include/drm/drm_fourcc.h > +++ b/include/drm/drm_fourcc.h > @@ -25,6 +25,25 @@ > #include <linux/types.h> > #include <uapi/drm/drm_fourcc.h> > > +/** > + * 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) > + * @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; > + u8 depth; > + u8 num_planes; > + u8 cpp[3]; > + u8 hsub; > + u8 vsub; > +}; > + > +const struct drm_format_info *drm_format_info(u32 format); > void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp); > int drm_format_num_planes(uint32_t format); > int drm_format_plane_cpp(uint32_t format, int plane); > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jun 09, 2016 at 10:52:23AM +0200, Daniel Vetter wrote: > On Thu, Jun 09, 2016 at 02:32:06AM +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_fourcc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++ > > include/drm/drm_fourcc.h | 19 ++++++++++ > > 2 files changed, 103 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c > > index 0645c85d5f95..47b9abd743be 100644 > > --- a/drivers/gpu/drm/drm_fourcc.c > > +++ b/drivers/gpu/drm/drm_fourcc.c > > @@ -62,6 +62,90 @@ const char *drm_get_format_name(uint32_t format) > > EXPORT_SYMBOL(drm_get_format_name); > > > > /** > > + * drm_format_info - query 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) > > Bikeshed on your pixel format description table. I think the approach I've > seen in gallium/mesa to describe pixel formats is a lot more generic, and > we might as well adopt it when we change. Idea is to have a block size > measure in pixels (both h and v), and then cpp is bytes_per_block. This is > essentially what you have with hsub and vsub already, except confusing > names, more ill-defined (since it only makes sense for yuv) and less > generic. A few examples: I think you have your confusion backwards. Calling something a block in planar formats would be more confusing. The only thing that really matters is the relative position of the samples between the planes. So there really is no "block" in there. > h_blocksize v_blocksize bytes_per_block (per-plane) > YUV410 4 4 {4, 1, 1} > YUV411 4 1 {4, 1, 1} > > hsub = h_blocksize / bytes_per_block[U/V plane] > vsub = v_blocksize / bytes_per_block[U/V plane] > > *_blocksize is in pixels > > [not entirely sure on the precise data, but that's kinda the point.] > > Ofc should maybe check that those helpers are only called on yuv planar > formats, too. This way we could also remove some of the comments in > drm_fourcc.h, or at least make the more clear. Another benefit is that > with this it's possible to write entirely generic size/stride checking > functions > > Oh and if we go with this, some asciiart for what's meant (in sphinx/rst, > that will happen in 4.8) would be awesome. > -Daniel > > > +{ > > + static const struct drm_format_info formats[] = { > > + { .format = DRM_FORMAT_C8, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_RGB332, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_BGR233, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_XRGB4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_XBGR4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_RGBX4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_BGRX4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_ARGB4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_ABGR4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_RGBA4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_BGRA4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_XRGB1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_XBGR1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_RGBX5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_BGRX5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_ARGB1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_ABGR1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_RGBA5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_BGRA5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_RGB565, .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_BGR565, .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_RGB888, .depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_BGR888, .depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_RGBX8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_BGRX8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_XRGB2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_XBGR2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_RGBX1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_BGRX1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_ARGB2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_ABGR2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_RGBA1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_BGRA1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_RGBA8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_BGRA8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_YUV410, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 }, > > + { .format = DRM_FORMAT_YVU410, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 }, > > + { .format = DRM_FORMAT_YUV411, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 }, > > + { .format = DRM_FORMAT_YVU411, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 }, > > + { .format = DRM_FORMAT_YUV420, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 }, > > + { .format = DRM_FORMAT_YVU420, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 }, > > + { .format = DRM_FORMAT_YUV422, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 }, > > + { .format = DRM_FORMAT_YVU422, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 }, > > + { .format = DRM_FORMAT_YUV444, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_YVU444, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_NV12, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 }, > > + { .format = DRM_FORMAT_NV21, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 }, > > + { .format = DRM_FORMAT_NV16, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 }, > > + { .format = DRM_FORMAT_NV61, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 }, > > + { .format = DRM_FORMAT_NV24, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_NV42, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 }, > > + { .format = DRM_FORMAT_YUYV, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, > > + { .format = DRM_FORMAT_YVYU, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, > > + { .format = DRM_FORMAT_UYVY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, > > + { .format = DRM_FORMAT_VYUY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, > > + { .format = DRM_FORMAT_AYUV, .depth = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > + }; > > + > > + unsigned int i; > > + > > + for (i = 0; i < ARRAY_SIZE(formats); ++i) { > > + if (formats[i].format == format) > > + return &formats[i]; > > + } > > + > > + return NULL; > > +} > > +EXPORT_SYMBOL(drm_format_info); > > + > > +/** > > * 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_fourcc.h b/include/drm/drm_fourcc.h > > index 7f90a396cf2b..b077df507b51 100644 > > --- a/include/drm/drm_fourcc.h > > +++ b/include/drm/drm_fourcc.h > > @@ -25,6 +25,25 @@ > > #include <linux/types.h> > > #include <uapi/drm/drm_fourcc.h> > > > > +/** > > + * 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) > > + * @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; > > + u8 depth; > > + u8 num_planes; > > + u8 cpp[3]; > > + u8 hsub; > > + u8 vsub; > > +}; > > + > > +const struct drm_format_info *drm_format_info(u32 format); > > void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp); > > int drm_format_num_planes(uint32_t format); > > int drm_format_plane_cpp(uint32_t format, int plane); > > -- > > Regards, > > > > Laurent Pinchart > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Jun 09, 2016 at 03:23:17PM +0300, Ville Syrjälä wrote: > On Thu, Jun 09, 2016 at 10:52:23AM +0200, Daniel Vetter wrote: > > On Thu, Jun 09, 2016 at 02:32:06AM +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_fourcc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++ > > > include/drm/drm_fourcc.h | 19 ++++++++++ > > > 2 files changed, 103 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c > > > index 0645c85d5f95..47b9abd743be 100644 > > > --- a/drivers/gpu/drm/drm_fourcc.c > > > +++ b/drivers/gpu/drm/drm_fourcc.c > > > @@ -62,6 +62,90 @@ const char *drm_get_format_name(uint32_t format) > > > EXPORT_SYMBOL(drm_get_format_name); > > > > > > /** > > > + * drm_format_info - query 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) > > > > Bikeshed on your pixel format description table. I think the approach I've > > seen in gallium/mesa to describe pixel formats is a lot more generic, and > > we might as well adopt it when we change. Idea is to have a block size > > measure in pixels (both h and v), and then cpp is bytes_per_block. This is > > essentially what you have with hsub and vsub already, except confusing > > names, more ill-defined (since it only makes sense for yuv) and less > > generic. A few examples: > > I think you have your confusion backwards. Calling something a block in > planar formats would be more confusing. The only thing that really > matters is the relative position of the samples between the planes. > So there really is no "block" in there. Atm U/V planes have a cpp of 1, which is definitely not true. There's much less than 1 byte per visible pixel in those planes. And that's the part that annoys me. block here is an entirely free-standing concept that just means "group of pixels over which the bytes-per-group is counted in each group". It's a concept stolen from gallium and makes a lot more sense when talking about compressed formats. But I think it also makes sense when talking about yuv formats. Maybe if you object "block" we can call it "group_of_pixels" instead. It's _not_ meant to be a continuous block of bytes in memory at all. E.g. for YUYV formats the gropu size would be (2, 1), with 4 bytes-per-group for an interleaved format of Y8U8Y8V8. In a way the hsub/vsub stuff describes more the color data, the group-of-pixels stuff is more useful imo to describe how much space you need on each plane. -Daniel > > > h_blocksize v_blocksize bytes_per_block (per-plane) > > YUV410 4 4 {4, 1, 1} > > YUV411 4 1 {4, 1, 1} > > > > hsub = h_blocksize / bytes_per_block[U/V plane] > > vsub = v_blocksize / bytes_per_block[U/V plane] > > > > *_blocksize is in pixels > > > > [not entirely sure on the precise data, but that's kinda the point.] > > > > Ofc should maybe check that those helpers are only called on yuv planar > > formats, too. This way we could also remove some of the comments in > > drm_fourcc.h, or at least make the more clear. Another benefit is that > > with this it's possible to write entirely generic size/stride checking > > functions > > > > Oh and if we go with this, some asciiart for what's meant (in sphinx/rst, > > that will happen in 4.8) would be awesome. > > -Daniel > > > > > +{ > > > + static const struct drm_format_info formats[] = { > > > + { .format = DRM_FORMAT_C8, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_RGB332, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_BGR233, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_XRGB4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_XBGR4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_RGBX4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_BGRX4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_ARGB4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_ABGR4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_RGBA4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_BGRA4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_XRGB1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_XBGR1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_RGBX5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_BGRX5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_ARGB1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_ABGR1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_RGBA5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_BGRA5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_RGB565, .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_BGR565, .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_RGB888, .depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_BGR888, .depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_RGBX8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_BGRX8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_XRGB2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_XBGR2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_RGBX1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_BGRX1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_ARGB2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_ABGR2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_RGBA1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_BGRA1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_RGBA8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_BGRA8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_YUV410, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 }, > > > + { .format = DRM_FORMAT_YVU410, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 }, > > > + { .format = DRM_FORMAT_YUV411, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 }, > > > + { .format = DRM_FORMAT_YVU411, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 }, > > > + { .format = DRM_FORMAT_YUV420, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 }, > > > + { .format = DRM_FORMAT_YVU420, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 }, > > > + { .format = DRM_FORMAT_YUV422, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 }, > > > + { .format = DRM_FORMAT_YVU422, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 }, > > > + { .format = DRM_FORMAT_YUV444, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_YVU444, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_NV12, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 }, > > > + { .format = DRM_FORMAT_NV21, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 }, > > > + { .format = DRM_FORMAT_NV16, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 }, > > > + { .format = DRM_FORMAT_NV61, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 }, > > > + { .format = DRM_FORMAT_NV24, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_NV42, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 }, > > > + { .format = DRM_FORMAT_YUYV, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, > > > + { .format = DRM_FORMAT_YVYU, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, > > > + { .format = DRM_FORMAT_UYVY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, > > > + { .format = DRM_FORMAT_VYUY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, > > > + { .format = DRM_FORMAT_AYUV, .depth = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, > > > + }; > > > + > > > + unsigned int i; > > > + > > > + for (i = 0; i < ARRAY_SIZE(formats); ++i) { > > > + if (formats[i].format == format) > > > + return &formats[i]; > > > + } > > > + > > > + return NULL; > > > +} > > > +EXPORT_SYMBOL(drm_format_info); > > > + > > > +/** > > > * 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_fourcc.h b/include/drm/drm_fourcc.h > > > index 7f90a396cf2b..b077df507b51 100644 > > > --- a/include/drm/drm_fourcc.h > > > +++ b/include/drm/drm_fourcc.h > > > @@ -25,6 +25,25 @@ > > > #include <linux/types.h> > > > #include <uapi/drm/drm_fourcc.h> > > > > > > +/** > > > + * 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) > > > + * @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; > > > + u8 depth; > > > + u8 num_planes; > > > + u8 cpp[3]; > > > + u8 hsub; > > > + u8 vsub; > > > +}; > > > + > > > +const struct drm_format_info *drm_format_info(u32 format); > > > void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp); > > > int drm_format_num_planes(uint32_t format); > > > int drm_format_plane_cpp(uint32_t format, int plane); > > > -- > > > Regards, > > > > > > Laurent Pinchart > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel OTC
On Thu, Jun 09, 2016 at 02:40:28PM +0200, Daniel Vetter wrote: > On Thu, Jun 09, 2016 at 03:23:17PM +0300, Ville Syrjälä wrote: > > On Thu, Jun 09, 2016 at 10:52:23AM +0200, Daniel Vetter wrote: > > > On Thu, Jun 09, 2016 at 02:32:06AM +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_fourcc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++ > > > > include/drm/drm_fourcc.h | 19 ++++++++++ > > > > 2 files changed, 103 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c > > > > index 0645c85d5f95..47b9abd743be 100644 > > > > --- a/drivers/gpu/drm/drm_fourcc.c > > > > +++ b/drivers/gpu/drm/drm_fourcc.c > > > > @@ -62,6 +62,90 @@ const char *drm_get_format_name(uint32_t format) > > > > EXPORT_SYMBOL(drm_get_format_name); > > > > > > > > /** > > > > + * drm_format_info - query 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) > > > > > > Bikeshed on your pixel format description table. I think the approach I've > > > seen in gallium/mesa to describe pixel formats is a lot more generic, and > > > we might as well adopt it when we change. Idea is to have a block size > > > measure in pixels (both h and v), and then cpp is bytes_per_block. This is > > > essentially what you have with hsub and vsub already, except confusing > > > names, more ill-defined (since it only makes sense for yuv) and less > > > generic. A few examples: > > > > I think you have your confusion backwards. Calling something a block in > > planar formats would be more confusing. The only thing that really > > matters is the relative position of the samples between the planes. > > So there really is no "block" in there. > > Atm U/V planes have a cpp of 1, which is definitely not true. There's much > less than 1 byte per visible pixel in those planes. And that's the part > that annoys me. That's exactly as it should be. The cpp value isn't some average thing for the whole, it's per-plane. > > block here is an entirely free-standing concept that just means "group of > pixels over which the bytes-per-group is counted in each group". It's a > concept stolen from gallium and makes a lot more sense when talking about > compressed formats. But I think it also makes sense when talking about yuv > formats. For packed YUV formats the usual term I've heard is macropixel, and there it does make sense. I could live with calling it a block. So I guess eg. for 422 packed formats we'd have h_block_size=2 v_block_size=1, and bytes_per_block=4. For planar formats, each plane should be considered individually, and trying to come up with some kind of cpp value etc. for the whole thing is pointless. I think eg. with for all the NVxx formats the chroma plane should have h_block_size=2 v_block_size=1 bytes_per_block=2 regardless the sub-sampling factor. So if we start using the block size concept, I think that too should be per-plane. Anything else will just get really confusing.
On Thu, Jun 09, 2016 at 04:05:11PM +0300, Ville Syrjälä wrote: > On Thu, Jun 09, 2016 at 02:40:28PM +0200, Daniel Vetter wrote: > > On Thu, Jun 09, 2016 at 03:23:17PM +0300, Ville Syrjälä wrote: > > > On Thu, Jun 09, 2016 at 10:52:23AM +0200, Daniel Vetter wrote: > > > > On Thu, Jun 09, 2016 at 02:32:06AM +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_fourcc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++ > > > > > include/drm/drm_fourcc.h | 19 ++++++++++ > > > > > 2 files changed, 103 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c > > > > > index 0645c85d5f95..47b9abd743be 100644 > > > > > --- a/drivers/gpu/drm/drm_fourcc.c > > > > > +++ b/drivers/gpu/drm/drm_fourcc.c > > > > > @@ -62,6 +62,90 @@ const char *drm_get_format_name(uint32_t format) > > > > > EXPORT_SYMBOL(drm_get_format_name); > > > > > > > > > > /** > > > > > + * drm_format_info - query 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) > > > > > > > > Bikeshed on your pixel format description table. I think the approach I've > > > > seen in gallium/mesa to describe pixel formats is a lot more generic, and > > > > we might as well adopt it when we change. Idea is to have a block size > > > > measure in pixels (both h and v), and then cpp is bytes_per_block. This is > > > > essentially what you have with hsub and vsub already, except confusing > > > > names, more ill-defined (since it only makes sense for yuv) and less > > > > generic. A few examples: > > > > > > I think you have your confusion backwards. Calling something a block in > > > planar formats would be more confusing. The only thing that really > > > matters is the relative position of the samples between the planes. > > > So there really is no "block" in there. > > > > Atm U/V planes have a cpp of 1, which is definitely not true. There's much > > less than 1 byte per visible pixel in those planes. And that's the part > > that annoys me. > > That's exactly as it should be. The cpp value isn't some average thing > for the whole, it's per-plane. On a 4x subsampled U or V plane you have 1 byte for 4 pixels. That's just plain not 1 character-per-pixel, even when you just look at that plane. > > block here is an entirely free-standing concept that just means "group of > > pixels over which the bytes-per-group is counted in each group". It's a > > concept stolen from gallium and makes a lot more sense when talking about > > compressed formats. But I think it also makes sense when talking about yuv > > formats. > > For packed YUV formats the usual term I've heard is macropixel, and there > it does make sense. I could live with calling it a block. So I guess eg. > for 422 packed formats we'd have h_block_size=2 v_block_size=1, and > bytes_per_block=4. > > For planar formats, each plane should be considered individually, > and trying to come up with some kind of cpp value etc. for the whole > thing is pointless. I think eg. with for all the NVxx formats the > chroma plane should have h_block_size=2 v_block_size=1 bytes_per_block=2 > regardless the sub-sampling factor. Hm yeah NV12 is a case where 2 have 2 bytes per 2 pixel block (since they're together in 1 2ndary plane), but still a subsampling of 2x. Maybe we'd need both? And then perhaps define subsampling per color channel, but block size and bytes-per-block per plane? > So if we start using the block size concept, I think that too should be > per-plane. Anything else will just get really confusing. You can always merge blocks together by using the lcm. -Daniel
On Thu, Jun 09, 2016 at 03:29:10PM +0200, Daniel Vetter wrote: > On Thu, Jun 09, 2016 at 04:05:11PM +0300, Ville Syrjälä wrote: > > On Thu, Jun 09, 2016 at 02:40:28PM +0200, Daniel Vetter wrote: > > > On Thu, Jun 09, 2016 at 03:23:17PM +0300, Ville Syrjälä wrote: > > > > On Thu, Jun 09, 2016 at 10:52:23AM +0200, Daniel Vetter wrote: > > > > > On Thu, Jun 09, 2016 at 02:32:06AM +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_fourcc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++ > > > > > > include/drm/drm_fourcc.h | 19 ++++++++++ > > > > > > 2 files changed, 103 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c > > > > > > index 0645c85d5f95..47b9abd743be 100644 > > > > > > --- a/drivers/gpu/drm/drm_fourcc.c > > > > > > +++ b/drivers/gpu/drm/drm_fourcc.c > > > > > > @@ -62,6 +62,90 @@ const char *drm_get_format_name(uint32_t format) > > > > > > EXPORT_SYMBOL(drm_get_format_name); > > > > > > > > > > > > /** > > > > > > + * drm_format_info - query 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) > > > > > > > > > > Bikeshed on your pixel format description table. I think the approach I've > > > > > seen in gallium/mesa to describe pixel formats is a lot more generic, and > > > > > we might as well adopt it when we change. Idea is to have a block size > > > > > measure in pixels (both h and v), and then cpp is bytes_per_block. This is > > > > > essentially what you have with hsub and vsub already, except confusing > > > > > names, more ill-defined (since it only makes sense for yuv) and less > > > > > generic. A few examples: > > > > > > > > I think you have your confusion backwards. Calling something a block in > > > > planar formats would be more confusing. The only thing that really > > > > matters is the relative position of the samples between the planes. > > > > So there really is no "block" in there. > > > > > > Atm U/V planes have a cpp of 1, which is definitely not true. There's much > > > less than 1 byte per visible pixel in those planes. And that's the part > > > that annoys me. > > > > That's exactly as it should be. The cpp value isn't some average thing > > for the whole, it's per-plane. > > On a 4x subsampled U or V plane you have 1 byte for 4 pixels. That's just > plain not 1 character-per-pixel, even when you just look at that plane. OK. So let's stop calling it a pixel and call it a sample instead. It's 1 byte per sample, which is the only thing that should matter to anyone. > > > > block here is an entirely free-standing concept that just means "group of > > > pixels over which the bytes-per-group is counted in each group". It's a > > > concept stolen from gallium and makes a lot more sense when talking about > > > compressed formats. But I think it also makes sense when talking about yuv > > > formats. > > > > For packed YUV formats the usual term I've heard is macropixel, and there > > it does make sense. I could live with calling it a block. So I guess eg. > > for 422 packed formats we'd have h_block_size=2 v_block_size=1, and > > bytes_per_block=4. > > > > For planar formats, each plane should be considered individually, > > and trying to come up with some kind of cpp value etc. for the whole > > thing is pointless. I think eg. with for all the NVxx formats the > > chroma plane should have h_block_size=2 v_block_size=1 bytes_per_block=2 > > regardless the sub-sampling factor. Actually meant to write h_block_size=1 here obviously. 1 sample of each chroma component, 2 bytes in total. > > Hm yeah NV12 is a case where 2 have 2 bytes per 2 pixel block (since > they're together in 1 2ndary plane), but still a subsampling of 2x. Maybe > we'd need both? And then perhaps define subsampling per color channel, but > block size and bytes-per-block per plane? Well given all the formats we have today, it's always chroma that's sub-sampled. I guess if we were to have a more complicated mix of planes that may or may not be sub-sampled then it might make sense to define things in a more complicated way. Right now I don't see any need for that though.
Hello Daniel and Ville, On Thursday 09 Jun 2016 17:13:15 Ville Syrjälä wrote: > On Thu, Jun 09, 2016 at 03:29:10PM +0200, Daniel Vetter wrote: > > On Thu, Jun 09, 2016 at 04:05:11PM +0300, Ville Syrjälä wrote: > >> On Thu, Jun 09, 2016 at 02:40:28PM +0200, Daniel Vetter wrote: > >>> On Thu, Jun 09, 2016 at 03:23:17PM +0300, Ville Syrjälä wrote: > >>>> On Thu, Jun 09, 2016 at 10:52:23AM +0200, Daniel Vetter wrote: > >>>>> On Thu, Jun 09, 2016 at 02:32:06AM +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_fourcc.c | 84 ++++++++++++++++++++++++++++ > >>>>>> include/drm/drm_fourcc.h | 19 ++++++++++ > >>>>>> 2 files changed, 103 insertions(+) > >>>>>> > >>>>>> diff --git a/drivers/gpu/drm/drm_fourcc.c > >>>>>> b/drivers/gpu/drm/drm_fourcc.c > >>>>>> index 0645c85d5f95..47b9abd743be 100644 > >>>>>> --- a/drivers/gpu/drm/drm_fourcc.c > >>>>>> +++ b/drivers/gpu/drm/drm_fourcc.c > >>>>>> @@ -62,6 +62,90 @@ const char *drm_get_format_name(uint32_t > >>>>>> format) > >>>>>> > >>>>>> EXPORT_SYMBOL(drm_get_format_name); > >>>>>> > >>>>>> /** > >>>>>> + * drm_format_info - query 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) > >>>>> > >>>>> Bikeshed on your pixel format description table. As much as I like a good bikeshed myself, we haven't reached a conclusion here, so I'll keep the current approach until someone proposes something better :-) > >>>>> I think the > >>>>> approach I've seen in gallium/mesa to describe pixel formats is a > >>>>> lot more generic, and we might as well adopt it when we change. > >>>>> Idea is to have a block size measure in pixels (both h and v), and > >>>>> then cpp is bytes_per_block. This is essentially what you have > >>>>> with hsub and vsub already, except confusing names, more ill- > >>>>> defined (since it only makes sense for yuv) and less generic. A > >>>>> few examples: > >>>> > >>>> I think you have your confusion backwards. Calling something a block > >>>> in planar formats would be more confusing. The only thing that > >>>> really matters is the relative position of the samples between the > >>>> planes. So there really is no "block" in there. > >>> > >>> Atm U/V planes have a cpp of 1, which is definitely not true. There's > >>> much less than 1 byte per visible pixel in those planes. And that's > >>> the part that annoys me. > >> > >> That's exactly as it should be. The cpp value isn't some average thing > >> for the whole, it's per-plane. > > > > On a 4x subsampled U or V plane you have 1 byte for 4 pixels. That's just > > plain not 1 character-per-pixel, even when you just look at that plane. > > OK. So let's stop calling it a pixel and call it a sample instead. > It's 1 byte per sample, which is the only thing that should matter > to anyone. > > >>> block here is an entirely free-standing concept that just means "group > >>> of pixels over which the bytes-per-group is counted in each group". > >>> It's a concept stolen from gallium and makes a lot more sense when > >>> talking about compressed formats. But I think it also makes sense when > >>> talking about yuv formats. > >> > >> For packed YUV formats the usual term I've heard is macropixel, and > >> there it does make sense. I could live with calling it a block. So I > >> guess eg. for 422 packed formats we'd have h_block_size=2 > >> v_block_size=1, and bytes_per_block=4. > >> > >> For planar formats, each plane should be considered individually, > >> and trying to come up with some kind of cpp value etc. for the whole > >> thing is pointless. I think eg. with for all the NVxx formats the > >> chroma plane should have h_block_size=2 v_block_size=1 bytes_per_block=2 > >> regardless the sub-sampling factor. > > Actually meant to write h_block_size=1 here obviously. 1 sample of > each chroma component, 2 bytes in total. > > > Hm yeah NV12 is a case where 2 have 2 bytes per 2 pixel block (since > > they're together in 1 2ndary plane), but still a subsampling of 2x. Maybe > > we'd need both? And then perhaps define subsampling per color channel, but > > block size and bytes-per-block per plane? > > Well given all the formats we have today, it's always chroma that's > sub-sampled. I guess if we were to have a more complicated mix of > planes that may or may not be sub-sampled then it might make sense to > define things in a more complicated way. Right now I don't see any > need for that though.
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index 0645c85d5f95..47b9abd743be 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -62,6 +62,90 @@ const char *drm_get_format_name(uint32_t format) EXPORT_SYMBOL(drm_get_format_name); /** + * drm_format_info - query 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[] = { + { .format = DRM_FORMAT_C8, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_RGB332, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_BGR233, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_XRGB4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_XBGR4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_RGBX4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_BGRX4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_ARGB4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_ABGR4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_RGBA4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_BGRA4444, .depth = 12, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_XRGB1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_XBGR1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_RGBX5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_BGRX5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_ARGB1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_ABGR1555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_RGBA5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_BGRA5551, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_RGB565, .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_BGR565, .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_RGB888, .depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_BGR888, .depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_XRGB8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_XBGR8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_RGBX8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_BGRX8888, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_XRGB2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_XBGR2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_RGBX1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_BGRX1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_ARGB2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_ABGR2101010, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_RGBA1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_BGRA1010102, .depth = 30, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_ARGB8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_ABGR8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_RGBA8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_BGRA8888, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_YUV410, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 }, + { .format = DRM_FORMAT_YVU410, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 }, + { .format = DRM_FORMAT_YUV411, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 }, + { .format = DRM_FORMAT_YVU411, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 }, + { .format = DRM_FORMAT_YUV420, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 }, + { .format = DRM_FORMAT_YVU420, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 }, + { .format = DRM_FORMAT_YUV422, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 }, + { .format = DRM_FORMAT_YVU422, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 }, + { .format = DRM_FORMAT_YUV444, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_YVU444, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_NV12, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 }, + { .format = DRM_FORMAT_NV21, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 }, + { .format = DRM_FORMAT_NV16, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 }, + { .format = DRM_FORMAT_NV61, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 }, + { .format = DRM_FORMAT_NV24, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_NV42, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 }, + { .format = DRM_FORMAT_YUYV, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, + { .format = DRM_FORMAT_YVYU, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, + { .format = DRM_FORMAT_UYVY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, + { .format = DRM_FORMAT_VYUY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 }, + { .format = DRM_FORMAT_AYUV, .depth = 0, .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 }, + }; + + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(formats); ++i) { + if (formats[i].format == format) + return &formats[i]; + } + + return NULL; +} +EXPORT_SYMBOL(drm_format_info); + +/** * 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_fourcc.h b/include/drm/drm_fourcc.h index 7f90a396cf2b..b077df507b51 100644 --- a/include/drm/drm_fourcc.h +++ b/include/drm/drm_fourcc.h @@ -25,6 +25,25 @@ #include <linux/types.h> #include <uapi/drm/drm_fourcc.h> +/** + * 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) + * @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; + u8 depth; + u8 num_planes; + u8 cpp[3]; + u8 hsub; + u8 vsub; +}; + +const struct drm_format_info *drm_format_info(u32 format); void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp); int drm_format_num_planes(uint32_t format); int drm_format_plane_cpp(uint32_t format, int plane);
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_fourcc.c | 84 ++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_fourcc.h | 19 ++++++++++ 2 files changed, 103 insertions(+)