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@xxxxxxxxxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel OTC
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
