| Message ID | c97024b97d3261dcf41aad3c8bc1c5d9906f33c9.1553032382.git-series.maxime.ripard@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Split out the formats API and move it to a common place | expand |
Le mardi 19 mars 2019 à 22:57 +0100, Maxime Ripard a écrit : > V4L2 uses different fourcc's than DRM, and has a different set of formats. > For now, let's add the v4l2 fourcc's for the already existing formats. > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > --- > include/linux/image-formats.h | 9 +++++- > lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- > 2 files changed, 76 insertions(+) > > diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h > index 53fd73a71b3d..fbc3a4501ebd 100644 > --- a/include/linux/image-formats.h > +++ b/include/linux/image-formats.h > @@ -26,6 +26,13 @@ struct image_format_info { > }; > > /** > + * @v4l2_fmt: > + * > + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) > + */ > + u32 v4l2_fmt; > + > + /** > * @depth: > * > * Color depth (number of bits per pixel excluding padding bits), > @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) > > const struct image_format_info *__image_format_drm_lookup(u32 drm); > const struct image_format_info *image_format_drm_lookup(u32 drm); > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); > unsigned int image_format_plane_cpp(const struct image_format_info *format, > int plane); > unsigned int image_format_plane_width(int width, > diff --git a/lib/image-formats.c b/lib/image-formats.c > index 9b9a73220c5d..39f1d38ae861 100644 > --- a/lib/image-formats.c > +++ b/lib/image-formats.c > @@ -8,6 +8,7 @@ > static const struct image_format_info formats[] = { > { > .drm_fmt = DRM_FORMAT_C8, > + .v4l2_fmt = V4L2_PIX_FMT_GREY, > .depth = 8, > .num_planes = 1, > .cpp = { 1, 0, 0 }, > @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_RGB332, > + .v4l2_fmt = V4L2_PIX_FMT_RGB332, > .depth = 8, > .num_planes = 1, > .cpp = { 1, 0, 0 }, > @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_XRGB4444, > + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_ARGB4444, > + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { > .has_alpha = true, > }, { > .drm_fmt = DRM_FORMAT_XRGB1555, > + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, > .depth = 15, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_ARGB1555, > + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, > .depth = 15, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { > .has_alpha = true, > }, { > .drm_fmt = DRM_FORMAT_RGB565, > + .v4l2_fmt = V4L2_PIX_FMT_RGB565, > .depth = 16, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_RGB888, > + .v4l2_fmt = V4L2_PIX_FMT_RGB24, > .depth = 24, > .num_planes = 1, > .cpp = { 3, 0, 0 }, > @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_BGR888, > + .v4l2_fmt = V4L2_PIX_FMT_BGR24, > .depth = 24, > .num_planes = 1, > .cpp = { 3, 0, 0 }, > @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_XRGB8888, > + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, All RGB mapping should be surrounded by ifdef, because many (not all) DRM formats represent the order of component when placed in a CPU register, unlike V4L2 which uses memory order. I've pick this one randomly, but this one on most system, little endian, will match V4L2_PIX_FMT_XBGR32. This type of complex mapping can be found in multiple places, notably in GStreamer: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/sys/kms/gstkmsutils.c#L45 > .depth = 24, > .num_planes = 1, > .cpp = { 4, 0, 0 }, > @@ -281,6 +291,7 @@ static const struct image_format_info formats[] = { > .has_alpha = true, > }, { > .drm_fmt = DRM_FORMAT_ARGB8888, > + .v4l2_fmt = V4L2_PIX_FMT_ARGB32, > .depth = 32, > .num_planes = 1, > .cpp = { 4, 0, 0 }, > @@ -361,6 +372,7 @@ static const struct image_format_info formats[] = { > .has_alpha = true, > }, { > .drm_fmt = DRM_FORMAT_YUV410, > + .v4l2_fmt = V4L2_PIX_FMT_YUV410, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -369,6 +381,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVU410, > + .v4l2_fmt = V4L2_PIX_FMT_YVU410, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -393,6 +406,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YUV420, > + .v4l2_fmt = V4L2_PIX_FMT_YUV420M, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -401,6 +415,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVU420, > + .v4l2_fmt = V4L2_PIX_FMT_YVU420M, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -409,6 +424,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YUV422, > + .v4l2_fmt = V4L2_PIX_FMT_YUV422M, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -417,6 +433,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVU422, > + .v4l2_fmt = V4L2_PIX_FMT_YVU422M, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -425,6 +442,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YUV444, > + .v4l2_fmt = V4L2_PIX_FMT_YUV444M, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -433,6 +451,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVU444, > + .v4l2_fmt = V4L2_PIX_FMT_YVU444M, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -441,6 +460,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV12, > + .v4l2_fmt = V4L2_PIX_FMT_NV12, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -449,6 +469,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV21, > + .v4l2_fmt = V4L2_PIX_FMT_NV21, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -457,6 +478,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV16, > + .v4l2_fmt = V4L2_PIX_FMT_NV16, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -465,6 +487,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV61, > + .v4l2_fmt = V4L2_PIX_FMT_NV61, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV24, > + .v4l2_fmt = V4L2_PIX_FMT_NV24, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -481,6 +505,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV42, > + .v4l2_fmt = V4L2_PIX_FMT_NV42, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -489,6 +514,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YUYV, > + .v4l2_fmt = V4L2_PIX_FMT_YUYV, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -497,6 +523,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVYU, > + .v4l2_fmt = V4L2_PIX_FMT_YVYU, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -505,6 +532,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_UYVY, > + .v4l2_fmt = V4L2_PIX_FMT_UYVY, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -513,6 +541,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_VYUY, > + .v4l2_fmt = V4L2_PIX_FMT_VYUY, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -632,6 +661,44 @@ const struct image_format_info *image_format_drm_lookup(u32 drm) > EXPORT_SYMBOL(image_format_drm_lookup); > > /** > + * __image_format_v4l2_lookup - query information for a given format > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > + * > + * The caller should only pass a supported pixel format to this function. > + * > + * Returns: > + * The instance of struct image_format_info that describes the pixel format, or > + * NULL if the format is unsupported. > + */ > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2) > +{ > + return __image_format_lookup(v4l2_fmt, v4l2); > +} > +EXPORT_SYMBOL(__image_format_v4l2_lookup); > + > +/** > + * image_format_v4l2_lookup - query information for a given format > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > + * > + * The caller should only pass a supported pixel format to this function. > + * Unsupported pixel formats will generate a warning in the kernel log. > + * > + * Returns: > + * The instance of struct image_format_info that describes the pixel format, or > + * NULL if the format is unsupported. > + */ > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2) > +{ > + const struct image_format_info *format; > + > + format = __image_format_v4l2_lookup(v4l2); > + > + WARN_ON(!format); > + return format; > +} > +EXPORT_SYMBOL(image_format_v4l2_lookup); > + > +/** > * image_format_plane_cpp - determine the bytes per pixel value > * @format: pointer to the image_format > * @plane: plane index
On Tue, Mar 19, 2019 at 07:29:18PM -0400, Nicolas Dufresne wrote: > Le mardi 19 mars 2019 à 22:57 +0100, Maxime Ripard a écrit : > > V4L2 uses different fourcc's than DRM, and has a different set of formats. > > For now, let's add the v4l2 fourcc's for the already existing formats. > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > --- > > include/linux/image-formats.h | 9 +++++- > > lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- > > 2 files changed, 76 insertions(+) > > > > diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h > > index 53fd73a71b3d..fbc3a4501ebd 100644 > > --- a/include/linux/image-formats.h > > +++ b/include/linux/image-formats.h > > @@ -26,6 +26,13 @@ struct image_format_info { > > }; > > > > /** > > + * @v4l2_fmt: > > + * > > + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) > > + */ > > + u32 v4l2_fmt; > > + > > + /** > > * @depth: > > * > > * Color depth (number of bits per pixel excluding padding bits), > > @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) > > > > const struct image_format_info *__image_format_drm_lookup(u32 drm); > > const struct image_format_info *image_format_drm_lookup(u32 drm); > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); > > unsigned int image_format_plane_cpp(const struct image_format_info *format, > > int plane); > > unsigned int image_format_plane_width(int width, > > diff --git a/lib/image-formats.c b/lib/image-formats.c > > index 9b9a73220c5d..39f1d38ae861 100644 > > --- a/lib/image-formats.c > > +++ b/lib/image-formats.c > > @@ -8,6 +8,7 @@ > > static const struct image_format_info formats[] = { > > { > > .drm_fmt = DRM_FORMAT_C8, > > + .v4l2_fmt = V4L2_PIX_FMT_GREY, > > .depth = 8, > > .num_planes = 1, > > .cpp = { 1, 0, 0 }, > > @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { > > .vsub = 1, > > }, { > > .drm_fmt = DRM_FORMAT_RGB332, > > + .v4l2_fmt = V4L2_PIX_FMT_RGB332, > > .depth = 8, > > .num_planes = 1, > > .cpp = { 1, 0, 0 }, > > @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { > > .vsub = 1, > > }, { > > .drm_fmt = DRM_FORMAT_XRGB4444, > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, > > .depth = 0, > > .num_planes = 1, > > .cpp = { 2, 0, 0 }, > > @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { > > .vsub = 1, > > }, { > > .drm_fmt = DRM_FORMAT_ARGB4444, > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, > > .depth = 0, > > .num_planes = 1, > > .cpp = { 2, 0, 0 }, > > @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { > > .has_alpha = true, > > }, { > > .drm_fmt = DRM_FORMAT_XRGB1555, > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, > > .depth = 15, > > .num_planes = 1, > > .cpp = { 2, 0, 0 }, > > @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { > > .vsub = 1, > > }, { > > .drm_fmt = DRM_FORMAT_ARGB1555, > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, > > .depth = 15, > > .num_planes = 1, > > .cpp = { 2, 0, 0 }, > > @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { > > .has_alpha = true, > > }, { > > .drm_fmt = DRM_FORMAT_RGB565, > > + .v4l2_fmt = V4L2_PIX_FMT_RGB565, > > .depth = 16, > > .num_planes = 1, > > .cpp = { 2, 0, 0 }, > > @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { > > .vsub = 1, > > }, { > > .drm_fmt = DRM_FORMAT_RGB888, > > + .v4l2_fmt = V4L2_PIX_FMT_RGB24, > > .depth = 24, > > .num_planes = 1, > > .cpp = { 3, 0, 0 }, > > @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { > > .vsub = 1, > > }, { > > .drm_fmt = DRM_FORMAT_BGR888, > > + .v4l2_fmt = V4L2_PIX_FMT_BGR24, > > .depth = 24, > > .num_planes = 1, > > .cpp = { 3, 0, 0 }, > > @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { > > .vsub = 1, > > }, { > > .drm_fmt = DRM_FORMAT_XRGB8888, > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, > > > All RGB mapping should be surrounded by ifdef, because many (not all) > DRM formats represent the order of component when placed in a CPU > register, unlike V4L2 which uses memory order. I've pick this one DRM formats are explicitly defined as little endian. > randomly, but this one on most system, little endian, will match > V4L2_PIX_FMT_XBGR32. This type of complex mapping can be found in > multiple places, notably in GStreamer: > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/sys/kms/gstkmsutils.c#L45 > > > .depth = 24, > > .num_planes = 1, > > .cpp = { 4, 0, 0 }, > > @@ -281,6 +291,7 @@ static const struct image_format_info formats[] = { > > .has_alpha = true, > > }, { > > .drm_fmt = DRM_FORMAT_ARGB8888, > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB32, > > .depth = 32, > > .num_planes = 1, > > .cpp = { 4, 0, 0 }, > > @@ -361,6 +372,7 @@ static const struct image_format_info formats[] = { > > .has_alpha = true, > > }, { > > .drm_fmt = DRM_FORMAT_YUV410, > > + .v4l2_fmt = V4L2_PIX_FMT_YUV410, > > .depth = 0, > > .num_planes = 3, > > .cpp = { 1, 1, 1 }, > > @@ -369,6 +381,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_YVU410, > > + .v4l2_fmt = V4L2_PIX_FMT_YVU410, > > .depth = 0, > > .num_planes = 3, > > .cpp = { 1, 1, 1 }, > > @@ -393,6 +406,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_YUV420, > > + .v4l2_fmt = V4L2_PIX_FMT_YUV420M, > > .depth = 0, > > .num_planes = 3, > > .cpp = { 1, 1, 1 }, > > @@ -401,6 +415,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_YVU420, > > + .v4l2_fmt = V4L2_PIX_FMT_YVU420M, > > .depth = 0, > > .num_planes = 3, > > .cpp = { 1, 1, 1 }, > > @@ -409,6 +424,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_YUV422, > > + .v4l2_fmt = V4L2_PIX_FMT_YUV422M, > > .depth = 0, > > .num_planes = 3, > > .cpp = { 1, 1, 1 }, > > @@ -417,6 +433,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_YVU422, > > + .v4l2_fmt = V4L2_PIX_FMT_YVU422M, > > .depth = 0, > > .num_planes = 3, > > .cpp = { 1, 1, 1 }, > > @@ -425,6 +442,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_YUV444, > > + .v4l2_fmt = V4L2_PIX_FMT_YUV444M, > > .depth = 0, > > .num_planes = 3, > > .cpp = { 1, 1, 1 }, > > @@ -433,6 +451,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_YVU444, > > + .v4l2_fmt = V4L2_PIX_FMT_YVU444M, > > .depth = 0, > > .num_planes = 3, > > .cpp = { 1, 1, 1 }, > > @@ -441,6 +460,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_NV12, > > + .v4l2_fmt = V4L2_PIX_FMT_NV12, > > .depth = 0, > > .num_planes = 2, > > .cpp = { 1, 2, 0 }, > > @@ -449,6 +469,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_NV21, > > + .v4l2_fmt = V4L2_PIX_FMT_NV21, > > .depth = 0, > > .num_planes = 2, > > .cpp = { 1, 2, 0 }, > > @@ -457,6 +478,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_NV16, > > + .v4l2_fmt = V4L2_PIX_FMT_NV16, > > .depth = 0, > > .num_planes = 2, > > .cpp = { 1, 2, 0 }, > > @@ -465,6 +487,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_NV61, > > + .v4l2_fmt = V4L2_PIX_FMT_NV61, > > .depth = 0, > > .num_planes = 2, > > .cpp = { 1, 2, 0 }, > > @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_NV24, > > + .v4l2_fmt = V4L2_PIX_FMT_NV24, > > .depth = 0, > > .num_planes = 2, > > .cpp = { 1, 2, 0 }, > > @@ -481,6 +505,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_NV42, > > + .v4l2_fmt = V4L2_PIX_FMT_NV42, > > .depth = 0, > > .num_planes = 2, > > .cpp = { 1, 2, 0 }, > > @@ -489,6 +514,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_YUYV, > > + .v4l2_fmt = V4L2_PIX_FMT_YUYV, > > .depth = 0, > > .num_planes = 1, > > .cpp = { 2, 0, 0 }, > > @@ -497,6 +523,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_YVYU, > > + .v4l2_fmt = V4L2_PIX_FMT_YVYU, > > .depth = 0, > > .num_planes = 1, > > .cpp = { 2, 0, 0 }, > > @@ -505,6 +532,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_UYVY, > > + .v4l2_fmt = V4L2_PIX_FMT_UYVY, > > .depth = 0, > > .num_planes = 1, > > .cpp = { 2, 0, 0 }, > > @@ -513,6 +541,7 @@ static const struct image_format_info formats[] = { > > .is_yuv = true, > > }, { > > .drm_fmt = DRM_FORMAT_VYUY, > > + .v4l2_fmt = V4L2_PIX_FMT_VYUY, > > .depth = 0, > > .num_planes = 1, > > .cpp = { 2, 0, 0 }, > > @@ -632,6 +661,44 @@ const struct image_format_info *image_format_drm_lookup(u32 drm) > > EXPORT_SYMBOL(image_format_drm_lookup); > > > > /** > > + * __image_format_v4l2_lookup - query information for a given format > > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > > + * > > + * The caller should only pass a supported pixel format to this function. > > + * > > + * Returns: > > + * The instance of struct image_format_info that describes the pixel format, or > > + * NULL if the format is unsupported. > > + */ > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2) > > +{ > > + return __image_format_lookup(v4l2_fmt, v4l2); > > +} > > +EXPORT_SYMBOL(__image_format_v4l2_lookup); > > + > > +/** > > + * image_format_v4l2_lookup - query information for a given format > > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > > + * > > + * The caller should only pass a supported pixel format to this function. > > + * Unsupported pixel formats will generate a warning in the kernel log. > > + * > > + * Returns: > > + * The instance of struct image_format_info that describes the pixel format, or > > + * NULL if the format is unsupported. > > + */ > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2) > > +{ > > + const struct image_format_info *format; > > + > > + format = __image_format_v4l2_lookup(v4l2); > > + > > + WARN_ON(!format); > > + return format; > > +} > > +EXPORT_SYMBOL(image_format_v4l2_lookup); > > + > > +/** > > * image_format_plane_cpp - determine the bytes per pixel value > > * @format: pointer to the image_format > > * @plane: plane index > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Le mercredi 20 mars 2019 à 16:27 +0200, Ville Syrjälä a écrit : > On Tue, Mar 19, 2019 at 07:29:18PM -0400, Nicolas Dufresne wrote: > > Le mardi 19 mars 2019 à 22:57 +0100, Maxime Ripard a écrit : > > > V4L2 uses different fourcc's than DRM, and has a different set of formats. > > > For now, let's add the v4l2 fourcc's for the already existing formats. > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > --- > > > include/linux/image-formats.h | 9 +++++- > > > lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- > > > 2 files changed, 76 insertions(+) > > > > > > diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h > > > index 53fd73a71b3d..fbc3a4501ebd 100644 > > > --- a/include/linux/image-formats.h > > > +++ b/include/linux/image-formats.h > > > @@ -26,6 +26,13 @@ struct image_format_info { > > > }; > > > > > > /** > > > + * @v4l2_fmt: > > > + * > > > + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) > > > + */ > > > + u32 v4l2_fmt; > > > + > > > + /** > > > * @depth: > > > * > > > * Color depth (number of bits per pixel excluding padding bits), > > > @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) > > > > > > const struct image_format_info *__image_format_drm_lookup(u32 drm); > > > const struct image_format_info *image_format_drm_lookup(u32 drm); > > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); > > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); > > > unsigned int image_format_plane_cpp(const struct image_format_info *format, > > > int plane); > > > unsigned int image_format_plane_width(int width, > > > diff --git a/lib/image-formats.c b/lib/image-formats.c > > > index 9b9a73220c5d..39f1d38ae861 100644 > > > --- a/lib/image-formats.c > > > +++ b/lib/image-formats.c > > > @@ -8,6 +8,7 @@ > > > static const struct image_format_info formats[] = { > > > { > > > .drm_fmt = DRM_FORMAT_C8, > > > + .v4l2_fmt = V4L2_PIX_FMT_GREY, > > > .depth = 8, > > > .num_planes = 1, > > > .cpp = { 1, 0, 0 }, > > > @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { > > > .vsub = 1, > > > }, { > > > .drm_fmt = DRM_FORMAT_RGB332, > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB332, > > > .depth = 8, > > > .num_planes = 1, > > > .cpp = { 1, 0, 0 }, > > > @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { > > > .vsub = 1, > > > }, { > > > .drm_fmt = DRM_FORMAT_XRGB4444, > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, > > > .depth = 0, > > > .num_planes = 1, > > > .cpp = { 2, 0, 0 }, > > > @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { > > > .vsub = 1, > > > }, { > > > .drm_fmt = DRM_FORMAT_ARGB4444, > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, > > > .depth = 0, > > > .num_planes = 1, > > > .cpp = { 2, 0, 0 }, > > > @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { > > > .has_alpha = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_XRGB1555, > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, > > > .depth = 15, > > > .num_planes = 1, > > > .cpp = { 2, 0, 0 }, > > > @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { > > > .vsub = 1, > > > }, { > > > .drm_fmt = DRM_FORMAT_ARGB1555, > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, > > > .depth = 15, > > > .num_planes = 1, > > > .cpp = { 2, 0, 0 }, > > > @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { > > > .has_alpha = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_RGB565, > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB565, > > > .depth = 16, > > > .num_planes = 1, > > > .cpp = { 2, 0, 0 }, > > > @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { > > > .vsub = 1, > > > }, { > > > .drm_fmt = DRM_FORMAT_RGB888, > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB24, > > > .depth = 24, > > > .num_planes = 1, > > > .cpp = { 3, 0, 0 }, > > > @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { > > > .vsub = 1, > > > }, { > > > .drm_fmt = DRM_FORMAT_BGR888, > > > + .v4l2_fmt = V4L2_PIX_FMT_BGR24, > > > .depth = 24, > > > .num_planes = 1, > > > .cpp = { 3, 0, 0 }, > > > @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { > > > .vsub = 1, > > > }, { > > > .drm_fmt = DRM_FORMAT_XRGB8888, > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, > > > > All RGB mapping should be surrounded by ifdef, because many (not all) > > DRM formats represent the order of component when placed in a CPU > > register, unlike V4L2 which uses memory order. I've pick this one > > DRM formats are explicitly defined as little endian. Yes, that means the same thing. The mapping has nothing to do with the buffer bytes order, unlike V4L2 (and most streaming stack) do. Though the mapping in DRM isn't consistent. Again, this mapping is not correct, it will result in swapped colors. > > > randomly, but this one on most system, little endian, will match > > V4L2_PIX_FMT_XBGR32. This type of complex mapping can be found in > > multiple places, notably in GStreamer: > > > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/sys/kms/gstkmsutils.c#L45 > > > > > .depth = 24, > > > .num_planes = 1, > > > .cpp = { 4, 0, 0 }, > > > @@ -281,6 +291,7 @@ static const struct image_format_info formats[] = { > > > .has_alpha = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_ARGB8888, > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB32, > > > .depth = 32, > > > .num_planes = 1, > > > .cpp = { 4, 0, 0 }, > > > @@ -361,6 +372,7 @@ static const struct image_format_info formats[] = { > > > .has_alpha = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_YUV410, > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV410, > > > .depth = 0, > > > .num_planes = 3, > > > .cpp = { 1, 1, 1 }, > > > @@ -369,6 +381,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_YVU410, > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU410, > > > .depth = 0, > > > .num_planes = 3, > > > .cpp = { 1, 1, 1 }, > > > @@ -393,6 +406,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_YUV420, > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV420M, > > > .depth = 0, > > > .num_planes = 3, > > > .cpp = { 1, 1, 1 }, > > > @@ -401,6 +415,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_YVU420, > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU420M, > > > .depth = 0, > > > .num_planes = 3, > > > .cpp = { 1, 1, 1 }, > > > @@ -409,6 +424,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_YUV422, > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV422M, > > > .depth = 0, > > > .num_planes = 3, > > > .cpp = { 1, 1, 1 }, > > > @@ -417,6 +433,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_YVU422, > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU422M, > > > .depth = 0, > > > .num_planes = 3, > > > .cpp = { 1, 1, 1 }, > > > @@ -425,6 +442,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_YUV444, > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV444M, > > > .depth = 0, > > > .num_planes = 3, > > > .cpp = { 1, 1, 1 }, > > > @@ -433,6 +451,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_YVU444, > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU444M, > > > .depth = 0, > > > .num_planes = 3, > > > .cpp = { 1, 1, 1 }, > > > @@ -441,6 +460,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_NV12, > > > + .v4l2_fmt = V4L2_PIX_FMT_NV12, > > > .depth = 0, > > > .num_planes = 2, > > > .cpp = { 1, 2, 0 }, > > > @@ -449,6 +469,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_NV21, > > > + .v4l2_fmt = V4L2_PIX_FMT_NV21, > > > .depth = 0, > > > .num_planes = 2, > > > .cpp = { 1, 2, 0 }, > > > @@ -457,6 +478,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_NV16, > > > + .v4l2_fmt = V4L2_PIX_FMT_NV16, > > > .depth = 0, > > > .num_planes = 2, > > > .cpp = { 1, 2, 0 }, > > > @@ -465,6 +487,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_NV61, > > > + .v4l2_fmt = V4L2_PIX_FMT_NV61, > > > .depth = 0, > > > .num_planes = 2, > > > .cpp = { 1, 2, 0 }, > > > @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_NV24, > > > + .v4l2_fmt = V4L2_PIX_FMT_NV24, > > > .depth = 0, > > > .num_planes = 2, > > > .cpp = { 1, 2, 0 }, > > > @@ -481,6 +505,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_NV42, > > > + .v4l2_fmt = V4L2_PIX_FMT_NV42, > > > .depth = 0, > > > .num_planes = 2, > > > .cpp = { 1, 2, 0 }, > > > @@ -489,6 +514,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_YUYV, > > > + .v4l2_fmt = V4L2_PIX_FMT_YUYV, > > > .depth = 0, > > > .num_planes = 1, > > > .cpp = { 2, 0, 0 }, > > > @@ -497,6 +523,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_YVYU, > > > + .v4l2_fmt = V4L2_PIX_FMT_YVYU, > > > .depth = 0, > > > .num_planes = 1, > > > .cpp = { 2, 0, 0 }, > > > @@ -505,6 +532,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_UYVY, > > > + .v4l2_fmt = V4L2_PIX_FMT_UYVY, > > > .depth = 0, > > > .num_planes = 1, > > > .cpp = { 2, 0, 0 }, > > > @@ -513,6 +541,7 @@ static const struct image_format_info formats[] = { > > > .is_yuv = true, > > > }, { > > > .drm_fmt = DRM_FORMAT_VYUY, > > > + .v4l2_fmt = V4L2_PIX_FMT_VYUY, > > > .depth = 0, > > > .num_planes = 1, > > > .cpp = { 2, 0, 0 }, > > > @@ -632,6 +661,44 @@ const struct image_format_info *image_format_drm_lookup(u32 drm) > > > EXPORT_SYMBOL(image_format_drm_lookup); > > > > > > /** > > > + * __image_format_v4l2_lookup - query information for a given format > > > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > > > + * > > > + * The caller should only pass a supported pixel format to this function. > > > + * > > > + * Returns: > > > + * The instance of struct image_format_info that describes the pixel format, or > > > + * NULL if the format is unsupported. > > > + */ > > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2) > > > +{ > > > + return __image_format_lookup(v4l2_fmt, v4l2); > > > +} > > > +EXPORT_SYMBOL(__image_format_v4l2_lookup); > > > + > > > +/** > > > + * image_format_v4l2_lookup - query information for a given format > > > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > > > + * > > > + * The caller should only pass a supported pixel format to this function. > > > + * Unsupported pixel formats will generate a warning in the kernel log. > > > + * > > > + * Returns: > > > + * The instance of struct image_format_info that describes the pixel format, or > > > + * NULL if the format is unsupported. > > > + */ > > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2) > > > +{ > > > + const struct image_format_info *format; > > > + > > > + format = __image_format_v4l2_lookup(v4l2); > > > + > > > + WARN_ON(!format); > > > + return format; > > > +} > > > +EXPORT_SYMBOL(image_format_v4l2_lookup); > > > + > > > +/** > > > * image_format_plane_cpp - determine the bytes per pixel value > > > * @format: pointer to the image_format > > > * @plane: plane index > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Mar 20, 2019 at 11:51:58AM -0400, Nicolas Dufresne wrote: > Le mercredi 20 mars 2019 à 16:27 +0200, Ville Syrjälä a écrit : > > On Tue, Mar 19, 2019 at 07:29:18PM -0400, Nicolas Dufresne wrote: > > > Le mardi 19 mars 2019 à 22:57 +0100, Maxime Ripard a écrit : > > > > V4L2 uses different fourcc's than DRM, and has a different set of formats. > > > > For now, let's add the v4l2 fourcc's for the already existing formats. > > > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > --- > > > > include/linux/image-formats.h | 9 +++++- > > > > lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- > > > > 2 files changed, 76 insertions(+) > > > > > > > > diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h > > > > index 53fd73a71b3d..fbc3a4501ebd 100644 > > > > --- a/include/linux/image-formats.h > > > > +++ b/include/linux/image-formats.h > > > > @@ -26,6 +26,13 @@ struct image_format_info { > > > > }; > > > > > > > > /** > > > > + * @v4l2_fmt: > > > > + * > > > > + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) > > > > + */ > > > > + u32 v4l2_fmt; > > > > + > > > > + /** > > > > * @depth: > > > > * > > > > * Color depth (number of bits per pixel excluding padding bits), > > > > @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) > > > > > > > > const struct image_format_info *__image_format_drm_lookup(u32 drm); > > > > const struct image_format_info *image_format_drm_lookup(u32 drm); > > > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); > > > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); > > > > unsigned int image_format_plane_cpp(const struct image_format_info *format, > > > > int plane); > > > > unsigned int image_format_plane_width(int width, > > > > diff --git a/lib/image-formats.c b/lib/image-formats.c > > > > index 9b9a73220c5d..39f1d38ae861 100644 > > > > --- a/lib/image-formats.c > > > > +++ b/lib/image-formats.c > > > > @@ -8,6 +8,7 @@ > > > > static const struct image_format_info formats[] = { > > > > { > > > > .drm_fmt = DRM_FORMAT_C8, > > > > + .v4l2_fmt = V4L2_PIX_FMT_GREY, > > > > .depth = 8, > > > > .num_planes = 1, > > > > .cpp = { 1, 0, 0 }, > > > > @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { > > > > .vsub = 1, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_RGB332, > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB332, > > > > .depth = 8, > > > > .num_planes = 1, > > > > .cpp = { 1, 0, 0 }, > > > > @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { > > > > .vsub = 1, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_XRGB4444, > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, > > > > .depth = 0, > > > > .num_planes = 1, > > > > .cpp = { 2, 0, 0 }, > > > > @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { > > > > .vsub = 1, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_ARGB4444, > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, > > > > .depth = 0, > > > > .num_planes = 1, > > > > .cpp = { 2, 0, 0 }, > > > > @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { > > > > .has_alpha = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_XRGB1555, > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, > > > > .depth = 15, > > > > .num_planes = 1, > > > > .cpp = { 2, 0, 0 }, > > > > @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { > > > > .vsub = 1, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_ARGB1555, > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, > > > > .depth = 15, > > > > .num_planes = 1, > > > > .cpp = { 2, 0, 0 }, > > > > @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { > > > > .has_alpha = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_RGB565, > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB565, > > > > .depth = 16, > > > > .num_planes = 1, > > > > .cpp = { 2, 0, 0 }, > > > > @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { > > > > .vsub = 1, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_RGB888, > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB24, > > > > .depth = 24, > > > > .num_planes = 1, > > > > .cpp = { 3, 0, 0 }, > > > > @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { > > > > .vsub = 1, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_BGR888, > > > > + .v4l2_fmt = V4L2_PIX_FMT_BGR24, > > > > .depth = 24, > > > > .num_planes = 1, > > > > .cpp = { 3, 0, 0 }, > > > > @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { > > > > .vsub = 1, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_XRGB8888, > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, > > > > > > All RGB mapping should be surrounded by ifdef, because many (not all) > > > DRM formats represent the order of component when placed in a CPU > > > register, unlike V4L2 which uses memory order. I've pick this one > > > > DRM formats are explicitly defined as little endian. > > Yes, that means the same thing. The mapping has nothing to do with the > buffer bytes order, unlike V4L2 (and most streaming stack) do. It has everything to do with byte order. Little endian means the least significant byte is stored in the first byte in memory. Based on https://www.kernel.org/doc/html/v4.15/media/uapi/v4l/pixfmt-packed-rgb.html drm XRGB888 is actually v4l XBGR32, not XRGB32 as claimed by this patch. > > > > > > randomly, but this one on most system, little endian, will match > > > V4L2_PIX_FMT_XBGR32. This type of complex mapping can be found in > > > multiple places, notably in GStreamer: > > > > > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/sys/kms/gstkmsutils.c#L45 > > > > > > > .depth = 24, > > > > .num_planes = 1, > > > > .cpp = { 4, 0, 0 }, > > > > @@ -281,6 +291,7 @@ static const struct image_format_info formats[] = { > > > > .has_alpha = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_ARGB8888, > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB32, > > > > .depth = 32, > > > > .num_planes = 1, > > > > .cpp = { 4, 0, 0 }, > > > > @@ -361,6 +372,7 @@ static const struct image_format_info formats[] = { > > > > .has_alpha = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_YUV410, > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV410, > > > > .depth = 0, > > > > .num_planes = 3, > > > > .cpp = { 1, 1, 1 }, > > > > @@ -369,6 +381,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_YVU410, > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU410, > > > > .depth = 0, > > > > .num_planes = 3, > > > > .cpp = { 1, 1, 1 }, > > > > @@ -393,6 +406,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_YUV420, > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV420M, > > > > .depth = 0, > > > > .num_planes = 3, > > > > .cpp = { 1, 1, 1 }, > > > > @@ -401,6 +415,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_YVU420, > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU420M, > > > > .depth = 0, > > > > .num_planes = 3, > > > > .cpp = { 1, 1, 1 }, > > > > @@ -409,6 +424,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_YUV422, > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV422M, > > > > .depth = 0, > > > > .num_planes = 3, > > > > .cpp = { 1, 1, 1 }, > > > > @@ -417,6 +433,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_YVU422, > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU422M, > > > > .depth = 0, > > > > .num_planes = 3, > > > > .cpp = { 1, 1, 1 }, > > > > @@ -425,6 +442,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_YUV444, > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV444M, > > > > .depth = 0, > > > > .num_planes = 3, > > > > .cpp = { 1, 1, 1 }, > > > > @@ -433,6 +451,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_YVU444, > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU444M, > > > > .depth = 0, > > > > .num_planes = 3, > > > > .cpp = { 1, 1, 1 }, > > > > @@ -441,6 +460,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_NV12, > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV12, > > > > .depth = 0, > > > > .num_planes = 2, > > > > .cpp = { 1, 2, 0 }, > > > > @@ -449,6 +469,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_NV21, > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV21, > > > > .depth = 0, > > > > .num_planes = 2, > > > > .cpp = { 1, 2, 0 }, > > > > @@ -457,6 +478,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_NV16, > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV16, > > > > .depth = 0, > > > > .num_planes = 2, > > > > .cpp = { 1, 2, 0 }, > > > > @@ -465,6 +487,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_NV61, > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV61, > > > > .depth = 0, > > > > .num_planes = 2, > > > > .cpp = { 1, 2, 0 }, > > > > @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_NV24, > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV24, > > > > .depth = 0, > > > > .num_planes = 2, > > > > .cpp = { 1, 2, 0 }, > > > > @@ -481,6 +505,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_NV42, > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV42, > > > > .depth = 0, > > > > .num_planes = 2, > > > > .cpp = { 1, 2, 0 }, > > > > @@ -489,6 +514,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_YUYV, > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUYV, > > > > .depth = 0, > > > > .num_planes = 1, > > > > .cpp = { 2, 0, 0 }, > > > > @@ -497,6 +523,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_YVYU, > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVYU, > > > > .depth = 0, > > > > .num_planes = 1, > > > > .cpp = { 2, 0, 0 }, > > > > @@ -505,6 +532,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_UYVY, > > > > + .v4l2_fmt = V4L2_PIX_FMT_UYVY, > > > > .depth = 0, > > > > .num_planes = 1, > > > > .cpp = { 2, 0, 0 }, > > > > @@ -513,6 +541,7 @@ static const struct image_format_info formats[] = { > > > > .is_yuv = true, > > > > }, { > > > > .drm_fmt = DRM_FORMAT_VYUY, > > > > + .v4l2_fmt = V4L2_PIX_FMT_VYUY, > > > > .depth = 0, > > > > .num_planes = 1, > > > > .cpp = { 2, 0, 0 }, > > > > @@ -632,6 +661,44 @@ const struct image_format_info *image_format_drm_lookup(u32 drm) > > > > EXPORT_SYMBOL(image_format_drm_lookup); > > > > > > > > /** > > > > + * __image_format_v4l2_lookup - query information for a given format > > > > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > > > > + * > > > > + * The caller should only pass a supported pixel format to this function. > > > > + * > > > > + * Returns: > > > > + * The instance of struct image_format_info that describes the pixel format, or > > > > + * NULL if the format is unsupported. > > > > + */ > > > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2) > > > > +{ > > > > + return __image_format_lookup(v4l2_fmt, v4l2); > > > > +} > > > > +EXPORT_SYMBOL(__image_format_v4l2_lookup); > > > > + > > > > +/** > > > > + * image_format_v4l2_lookup - query information for a given format > > > > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > > > > + * > > > > + * The caller should only pass a supported pixel format to this function. > > > > + * Unsupported pixel formats will generate a warning in the kernel log. > > > > + * > > > > + * Returns: > > > > + * The instance of struct image_format_info that describes the pixel format, or > > > > + * NULL if the format is unsupported. > > > > + */ > > > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2) > > > > +{ > > > > + const struct image_format_info *format; > > > > + > > > > + format = __image_format_v4l2_lookup(v4l2); > > > > + > > > > + WARN_ON(!format); > > > > + return format; > > > > +} > > > > +EXPORT_SYMBOL(image_format_v4l2_lookup); > > > > + > > > > +/** > > > > * image_format_plane_cpp - determine the bytes per pixel value > > > > * @format: pointer to the image_format > > > > * @plane: plane index > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Le mercredi 20 mars 2019 à 18:09 +0200, Ville Syrjälä a écrit : > On Wed, Mar 20, 2019 at 11:51:58AM -0400, Nicolas Dufresne wrote: > > Le mercredi 20 mars 2019 à 16:27 +0200, Ville Syrjälä a écrit : > > > On Tue, Mar 19, 2019 at 07:29:18PM -0400, Nicolas Dufresne wrote: > > > > Le mardi 19 mars 2019 à 22:57 +0100, Maxime Ripard a écrit : > > > > > V4L2 uses different fourcc's than DRM, and has a different set of formats. > > > > > For now, let's add the v4l2 fourcc's for the already existing formats. > > > > > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > --- > > > > > include/linux/image-formats.h | 9 +++++- > > > > > lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- > > > > > 2 files changed, 76 insertions(+) > > > > > > > > > > diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h > > > > > index 53fd73a71b3d..fbc3a4501ebd 100644 > > > > > --- a/include/linux/image-formats.h > > > > > +++ b/include/linux/image-formats.h > > > > > @@ -26,6 +26,13 @@ struct image_format_info { > > > > > }; > > > > > > > > > > /** > > > > > + * @v4l2_fmt: > > > > > + * > > > > > + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) > > > > > + */ > > > > > + u32 v4l2_fmt; > > > > > + > > > > > + /** > > > > > * @depth: > > > > > * > > > > > * Color depth (number of bits per pixel excluding padding bits), > > > > > @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) > > > > > > > > > > const struct image_format_info *__image_format_drm_lookup(u32 drm); > > > > > const struct image_format_info *image_format_drm_lookup(u32 drm); > > > > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); > > > > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); > > > > > unsigned int image_format_plane_cpp(const struct image_format_info *format, > > > > > int plane); > > > > > unsigned int image_format_plane_width(int width, > > > > > diff --git a/lib/image-formats.c b/lib/image-formats.c > > > > > index 9b9a73220c5d..39f1d38ae861 100644 > > > > > --- a/lib/image-formats.c > > > > > +++ b/lib/image-formats.c > > > > > @@ -8,6 +8,7 @@ > > > > > static const struct image_format_info formats[] = { > > > > > { > > > > > .drm_fmt = DRM_FORMAT_C8, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_GREY, > > > > > .depth = 8, > > > > > .num_planes = 1, > > > > > .cpp = { 1, 0, 0 }, > > > > > @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { > > > > > .vsub = 1, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_RGB332, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB332, > > > > > .depth = 8, > > > > > .num_planes = 1, > > > > > .cpp = { 1, 0, 0 }, > > > > > @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { > > > > > .vsub = 1, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_XRGB4444, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, > > > > > .depth = 0, > > > > > .num_planes = 1, > > > > > .cpp = { 2, 0, 0 }, > > > > > @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { > > > > > .vsub = 1, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_ARGB4444, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, > > > > > .depth = 0, > > > > > .num_planes = 1, > > > > > .cpp = { 2, 0, 0 }, > > > > > @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { > > > > > .has_alpha = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_XRGB1555, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, > > > > > .depth = 15, > > > > > .num_planes = 1, > > > > > .cpp = { 2, 0, 0 }, > > > > > @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { > > > > > .vsub = 1, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_ARGB1555, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, > > > > > .depth = 15, > > > > > .num_planes = 1, > > > > > .cpp = { 2, 0, 0 }, > > > > > @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { > > > > > .has_alpha = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_RGB565, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB565, > > > > > .depth = 16, > > > > > .num_planes = 1, > > > > > .cpp = { 2, 0, 0 }, > > > > > @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { > > > > > .vsub = 1, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_RGB888, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB24, > > > > > .depth = 24, > > > > > .num_planes = 1, > > > > > .cpp = { 3, 0, 0 }, > > > > > @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { > > > > > .vsub = 1, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_BGR888, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_BGR24, > > > > > .depth = 24, > > > > > .num_planes = 1, > > > > > .cpp = { 3, 0, 0 }, > > > > > @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { > > > > > .vsub = 1, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_XRGB8888, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, > > > > > > > > All RGB mapping should be surrounded by ifdef, because many (not all) > > > > DRM formats represent the order of component when placed in a CPU > > > > register, unlike V4L2 which uses memory order. I've pick this one > > > > > > DRM formats are explicitly defined as little endian. > > > > Yes, that means the same thing. The mapping has nothing to do with the > > buffer bytes order, unlike V4L2 (and most streaming stack) do. > > It has everything to do with byte order. Little endian means the least > significant byte is stored in the first byte in memory. > > Based on https://www.kernel.org/doc/html/v4.15/media/uapi/v4l/pixfmt-packed-rgb.html > drm XRGB888 is actually v4l XBGR32, not XRGB32 as claimed by this patch. That's basically what I said, as it's define for Little Endian rather then buffer order, you have to make the mapping conditional. It basically mean that in memory, the DRM format physically differ depending on CPU endian. Last time we have run this on PPC / Big Endian, the mapping was XRGB/XRGB, we checked that up multiple time with the DRM maintainers and was told this is exactly what it's suppose to do, hence this endian dependant mapping all over the place. If you make up that this isn't right, you are breaking userspace, and people don't like that. So the mapping should be: Little: DRM XRGB / V4L2 XBGR Big: DRM XRGB / V4L2 XRGB > > > > > randomly, but this one on most system, little endian, will match > > > > V4L2_PIX_FMT_XBGR32. This type of complex mapping can be found in > > > > multiple places, notably in GStreamer: > > > > > > > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/sys/kms/gstkmsutils.c#L45 > > > > > > > > > .depth = 24, > > > > > .num_planes = 1, > > > > > .cpp = { 4, 0, 0 }, > > > > > @@ -281,6 +291,7 @@ static const struct image_format_info formats[] = { > > > > > .has_alpha = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_ARGB8888, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB32, > > > > > .depth = 32, > > > > > .num_planes = 1, > > > > > .cpp = { 4, 0, 0 }, > > > > > @@ -361,6 +372,7 @@ static const struct image_format_info formats[] = { > > > > > .has_alpha = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_YUV410, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV410, > > > > > .depth = 0, > > > > > .num_planes = 3, > > > > > .cpp = { 1, 1, 1 }, > > > > > @@ -369,6 +381,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_YVU410, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU410, > > > > > .depth = 0, > > > > > .num_planes = 3, > > > > > .cpp = { 1, 1, 1 }, > > > > > @@ -393,6 +406,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_YUV420, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV420M, > > > > > .depth = 0, > > > > > .num_planes = 3, > > > > > .cpp = { 1, 1, 1 }, > > > > > @@ -401,6 +415,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_YVU420, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU420M, > > > > > .depth = 0, > > > > > .num_planes = 3, > > > > > .cpp = { 1, 1, 1 }, > > > > > @@ -409,6 +424,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_YUV422, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV422M, > > > > > .depth = 0, > > > > > .num_planes = 3, > > > > > .cpp = { 1, 1, 1 }, > > > > > @@ -417,6 +433,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_YVU422, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU422M, > > > > > .depth = 0, > > > > > .num_planes = 3, > > > > > .cpp = { 1, 1, 1 }, > > > > > @@ -425,6 +442,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_YUV444, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV444M, > > > > > .depth = 0, > > > > > .num_planes = 3, > > > > > .cpp = { 1, 1, 1 }, > > > > > @@ -433,6 +451,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_YVU444, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU444M, > > > > > .depth = 0, > > > > > .num_planes = 3, > > > > > .cpp = { 1, 1, 1 }, > > > > > @@ -441,6 +460,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_NV12, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV12, > > > > > .depth = 0, > > > > > .num_planes = 2, > > > > > .cpp = { 1, 2, 0 }, > > > > > @@ -449,6 +469,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_NV21, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV21, > > > > > .depth = 0, > > > > > .num_planes = 2, > > > > > .cpp = { 1, 2, 0 }, > > > > > @@ -457,6 +478,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_NV16, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV16, > > > > > .depth = 0, > > > > > .num_planes = 2, > > > > > .cpp = { 1, 2, 0 }, > > > > > @@ -465,6 +487,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_NV61, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV61, > > > > > .depth = 0, > > > > > .num_planes = 2, > > > > > .cpp = { 1, 2, 0 }, > > > > > @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_NV24, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV24, > > > > > .depth = 0, > > > > > .num_planes = 2, > > > > > .cpp = { 1, 2, 0 }, > > > > > @@ -481,6 +505,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_NV42, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV42, > > > > > .depth = 0, > > > > > .num_planes = 2, > > > > > .cpp = { 1, 2, 0 }, > > > > > @@ -489,6 +514,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_YUYV, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUYV, > > > > > .depth = 0, > > > > > .num_planes = 1, > > > > > .cpp = { 2, 0, 0 }, > > > > > @@ -497,6 +523,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_YVYU, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVYU, > > > > > .depth = 0, > > > > > .num_planes = 1, > > > > > .cpp = { 2, 0, 0 }, > > > > > @@ -505,6 +532,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_UYVY, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_UYVY, > > > > > .depth = 0, > > > > > .num_planes = 1, > > > > > .cpp = { 2, 0, 0 }, > > > > > @@ -513,6 +541,7 @@ static const struct image_format_info formats[] = { > > > > > .is_yuv = true, > > > > > }, { > > > > > .drm_fmt = DRM_FORMAT_VYUY, > > > > > + .v4l2_fmt = V4L2_PIX_FMT_VYUY, > > > > > .depth = 0, > > > > > .num_planes = 1, > > > > > .cpp = { 2, 0, 0 }, > > > > > @@ -632,6 +661,44 @@ const struct image_format_info *image_format_drm_lookup(u32 drm) > > > > > EXPORT_SYMBOL(image_format_drm_lookup); > > > > > > > > > > /** > > > > > + * __image_format_v4l2_lookup - query information for a given format > > > > > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > > > > > + * > > > > > + * The caller should only pass a supported pixel format to this function. > > > > > + * > > > > > + * Returns: > > > > > + * The instance of struct image_format_info that describes the pixel format, or > > > > > + * NULL if the format is unsupported. > > > > > + */ > > > > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2) > > > > > +{ > > > > > + return __image_format_lookup(v4l2_fmt, v4l2); > > > > > +} > > > > > +EXPORT_SYMBOL(__image_format_v4l2_lookup); > > > > > + > > > > > +/** > > > > > + * image_format_v4l2_lookup - query information for a given format > > > > > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > > > > > + * > > > > > + * The caller should only pass a supported pixel format to this function. > > > > > + * Unsupported pixel formats will generate a warning in the kernel log. > > > > > + * > > > > > + * Returns: > > > > > + * The instance of struct image_format_info that describes the pixel format, or > > > > > + * NULL if the format is unsupported. > > > > > + */ > > > > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2) > > > > > +{ > > > > > + const struct image_format_info *format; > > > > > + > > > > > + format = __image_format_v4l2_lookup(v4l2); > > > > > + > > > > > + WARN_ON(!format); > > > > > + return format; > > > > > +} > > > > > +EXPORT_SYMBOL(image_format_v4l2_lookup); > > > > > + > > > > > +/** > > > > > * image_format_plane_cpp - determine the bytes per pixel value > > > > > * @format: pointer to the image_format > > > > > * @plane: plane index > > > > > > > > _______________________________________________ > > > > dri-devel mailing list > > > > dri-devel@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > >
On Wed, Mar 20, 2019 at 12:30:25PM -0400, Nicolas Dufresne wrote: > Le mercredi 20 mars 2019 à 18:09 +0200, Ville Syrjälä a écrit : > > On Wed, Mar 20, 2019 at 11:51:58AM -0400, Nicolas Dufresne wrote: > > > Le mercredi 20 mars 2019 à 16:27 +0200, Ville Syrjälä a écrit : > > > > On Tue, Mar 19, 2019 at 07:29:18PM -0400, Nicolas Dufresne wrote: > > > > > Le mardi 19 mars 2019 à 22:57 +0100, Maxime Ripard a écrit : > > > > > > V4L2 uses different fourcc's than DRM, and has a different set of formats. > > > > > > For now, let's add the v4l2 fourcc's for the already existing formats. > > > > > > > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > --- > > > > > > include/linux/image-formats.h | 9 +++++- > > > > > > lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- > > > > > > 2 files changed, 76 insertions(+) > > > > > > > > > > > > diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h > > > > > > index 53fd73a71b3d..fbc3a4501ebd 100644 > > > > > > --- a/include/linux/image-formats.h > > > > > > +++ b/include/linux/image-formats.h > > > > > > @@ -26,6 +26,13 @@ struct image_format_info { > > > > > > }; > > > > > > > > > > > > /** > > > > > > + * @v4l2_fmt: > > > > > > + * > > > > > > + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) > > > > > > + */ > > > > > > + u32 v4l2_fmt; > > > > > > + > > > > > > + /** > > > > > > * @depth: > > > > > > * > > > > > > * Color depth (number of bits per pixel excluding padding bits), > > > > > > @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) > > > > > > > > > > > > const struct image_format_info *__image_format_drm_lookup(u32 drm); > > > > > > const struct image_format_info *image_format_drm_lookup(u32 drm); > > > > > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); > > > > > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); > > > > > > unsigned int image_format_plane_cpp(const struct image_format_info *format, > > > > > > int plane); > > > > > > unsigned int image_format_plane_width(int width, > > > > > > diff --git a/lib/image-formats.c b/lib/image-formats.c > > > > > > index 9b9a73220c5d..39f1d38ae861 100644 > > > > > > --- a/lib/image-formats.c > > > > > > +++ b/lib/image-formats.c > > > > > > @@ -8,6 +8,7 @@ > > > > > > static const struct image_format_info formats[] = { > > > > > > { > > > > > > .drm_fmt = DRM_FORMAT_C8, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_GREY, > > > > > > .depth = 8, > > > > > > .num_planes = 1, > > > > > > .cpp = { 1, 0, 0 }, > > > > > > @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { > > > > > > .vsub = 1, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_RGB332, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB332, > > > > > > .depth = 8, > > > > > > .num_planes = 1, > > > > > > .cpp = { 1, 0, 0 }, > > > > > > @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { > > > > > > .vsub = 1, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_XRGB4444, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, > > > > > > .depth = 0, > > > > > > .num_planes = 1, > > > > > > .cpp = { 2, 0, 0 }, > > > > > > @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { > > > > > > .vsub = 1, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_ARGB4444, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, > > > > > > .depth = 0, > > > > > > .num_planes = 1, > > > > > > .cpp = { 2, 0, 0 }, > > > > > > @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { > > > > > > .has_alpha = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_XRGB1555, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, > > > > > > .depth = 15, > > > > > > .num_planes = 1, > > > > > > .cpp = { 2, 0, 0 }, > > > > > > @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { > > > > > > .vsub = 1, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_ARGB1555, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, > > > > > > .depth = 15, > > > > > > .num_planes = 1, > > > > > > .cpp = { 2, 0, 0 }, > > > > > > @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { > > > > > > .has_alpha = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_RGB565, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB565, > > > > > > .depth = 16, > > > > > > .num_planes = 1, > > > > > > .cpp = { 2, 0, 0 }, > > > > > > @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { > > > > > > .vsub = 1, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_RGB888, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB24, > > > > > > .depth = 24, > > > > > > .num_planes = 1, > > > > > > .cpp = { 3, 0, 0 }, > > > > > > @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { > > > > > > .vsub = 1, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_BGR888, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_BGR24, > > > > > > .depth = 24, > > > > > > .num_planes = 1, > > > > > > .cpp = { 3, 0, 0 }, > > > > > > @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { > > > > > > .vsub = 1, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_XRGB8888, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, > > > > > > > > > > All RGB mapping should be surrounded by ifdef, because many (not all) > > > > > DRM formats represent the order of component when placed in a CPU > > > > > register, unlike V4L2 which uses memory order. I've pick this one > > > > > > > > DRM formats are explicitly defined as little endian. > > > > > > Yes, that means the same thing. The mapping has nothing to do with the > > > buffer bytes order, unlike V4L2 (and most streaming stack) do. > > > > It has everything to do with byte order. Little endian means the least > > significant byte is stored in the first byte in memory. > > > > Based on https://www.kernel.org/doc/html/v4.15/media/uapi/v4l/pixfmt-packed-rgb.html > > drm XRGB888 is actually v4l XBGR32, not XRGB32 as claimed by this patch. > > That's basically what I said, as it's define for Little Endian rather > then buffer order, you have to make the mapping conditional. It > basically mean that in memory, the DRM format physically differ > depending on CPU endian. No. It is always little endian no matter what the CPU is. > Last time we have run this on PPC / Big > Endian, the mapping was XRGB/XRGB, we checked that up multiple time > with the DRM maintainers and was told this is exactly what it's suppose > to do, hence this endian dependant mapping all over the place. If you > make up that this isn't right, you are breaking userspace, and people > don't like that. Someone recently added those DRM_FORMAT_HOST variants to allow the legacy addfb1 to create pick the format based on host endianness. I thought that was the only conclusion from the little vs. big endian drm fourcc wars. > > So the mapping should be: > Little: DRM XRGB / V4L2 XBGR > Big: DRM XRGB / V4L2 XRGB > > > > > > > > randomly, but this one on most system, little endian, will match > > > > > V4L2_PIX_FMT_XBGR32. This type of complex mapping can be found in > > > > > multiple places, notably in GStreamer: > > > > > > > > > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/sys/kms/gstkmsutils.c#L45 > > > > > > > > > > > .depth = 24, > > > > > > .num_planes = 1, > > > > > > .cpp = { 4, 0, 0 }, > > > > > > @@ -281,6 +291,7 @@ static const struct image_format_info formats[] = { > > > > > > .has_alpha = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_ARGB8888, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB32, > > > > > > .depth = 32, > > > > > > .num_planes = 1, > > > > > > .cpp = { 4, 0, 0 }, > > > > > > @@ -361,6 +372,7 @@ static const struct image_format_info formats[] = { > > > > > > .has_alpha = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_YUV410, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV410, > > > > > > .depth = 0, > > > > > > .num_planes = 3, > > > > > > .cpp = { 1, 1, 1 }, > > > > > > @@ -369,6 +381,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_YVU410, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU410, > > > > > > .depth = 0, > > > > > > .num_planes = 3, > > > > > > .cpp = { 1, 1, 1 }, > > > > > > @@ -393,6 +406,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_YUV420, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV420M, > > > > > > .depth = 0, > > > > > > .num_planes = 3, > > > > > > .cpp = { 1, 1, 1 }, > > > > > > @@ -401,6 +415,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_YVU420, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU420M, > > > > > > .depth = 0, > > > > > > .num_planes = 3, > > > > > > .cpp = { 1, 1, 1 }, > > > > > > @@ -409,6 +424,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_YUV422, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV422M, > > > > > > .depth = 0, > > > > > > .num_planes = 3, > > > > > > .cpp = { 1, 1, 1 }, > > > > > > @@ -417,6 +433,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_YVU422, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU422M, > > > > > > .depth = 0, > > > > > > .num_planes = 3, > > > > > > .cpp = { 1, 1, 1 }, > > > > > > @@ -425,6 +442,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_YUV444, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV444M, > > > > > > .depth = 0, > > > > > > .num_planes = 3, > > > > > > .cpp = { 1, 1, 1 }, > > > > > > @@ -433,6 +451,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_YVU444, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU444M, > > > > > > .depth = 0, > > > > > > .num_planes = 3, > > > > > > .cpp = { 1, 1, 1 }, > > > > > > @@ -441,6 +460,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_NV12, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV12, > > > > > > .depth = 0, > > > > > > .num_planes = 2, > > > > > > .cpp = { 1, 2, 0 }, > > > > > > @@ -449,6 +469,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_NV21, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV21, > > > > > > .depth = 0, > > > > > > .num_planes = 2, > > > > > > .cpp = { 1, 2, 0 }, > > > > > > @@ -457,6 +478,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_NV16, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV16, > > > > > > .depth = 0, > > > > > > .num_planes = 2, > > > > > > .cpp = { 1, 2, 0 }, > > > > > > @@ -465,6 +487,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_NV61, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV61, > > > > > > .depth = 0, > > > > > > .num_planes = 2, > > > > > > .cpp = { 1, 2, 0 }, > > > > > > @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_NV24, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV24, > > > > > > .depth = 0, > > > > > > .num_planes = 2, > > > > > > .cpp = { 1, 2, 0 }, > > > > > > @@ -481,6 +505,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_NV42, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV42, > > > > > > .depth = 0, > > > > > > .num_planes = 2, > > > > > > .cpp = { 1, 2, 0 }, > > > > > > @@ -489,6 +514,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_YUYV, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUYV, > > > > > > .depth = 0, > > > > > > .num_planes = 1, > > > > > > .cpp = { 2, 0, 0 }, > > > > > > @@ -497,6 +523,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_YVYU, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVYU, > > > > > > .depth = 0, > > > > > > .num_planes = 1, > > > > > > .cpp = { 2, 0, 0 }, > > > > > > @@ -505,6 +532,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_UYVY, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_UYVY, > > > > > > .depth = 0, > > > > > > .num_planes = 1, > > > > > > .cpp = { 2, 0, 0 }, > > > > > > @@ -513,6 +541,7 @@ static const struct image_format_info formats[] = { > > > > > > .is_yuv = true, > > > > > > }, { > > > > > > .drm_fmt = DRM_FORMAT_VYUY, > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_VYUY, > > > > > > .depth = 0, > > > > > > .num_planes = 1, > > > > > > .cpp = { 2, 0, 0 }, > > > > > > @@ -632,6 +661,44 @@ const struct image_format_info *image_format_drm_lookup(u32 drm) > > > > > > EXPORT_SYMBOL(image_format_drm_lookup); > > > > > > > > > > > > /** > > > > > > + * __image_format_v4l2_lookup - query information for a given format > > > > > > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > > > > > > + * > > > > > > + * The caller should only pass a supported pixel format to this function. > > > > > > + * > > > > > > + * Returns: > > > > > > + * The instance of struct image_format_info that describes the pixel format, or > > > > > > + * NULL if the format is unsupported. > > > > > > + */ > > > > > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2) > > > > > > +{ > > > > > > + return __image_format_lookup(v4l2_fmt, v4l2); > > > > > > +} > > > > > > +EXPORT_SYMBOL(__image_format_v4l2_lookup); > > > > > > + > > > > > > +/** > > > > > > + * image_format_v4l2_lookup - query information for a given format > > > > > > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > > > > > > + * > > > > > > + * The caller should only pass a supported pixel format to this function. > > > > > > + * Unsupported pixel formats will generate a warning in the kernel log. > > > > > > + * > > > > > > + * Returns: > > > > > > + * The instance of struct image_format_info that describes the pixel format, or > > > > > > + * NULL if the format is unsupported. > > > > > > + */ > > > > > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2) > > > > > > +{ > > > > > > + const struct image_format_info *format; > > > > > > + > > > > > > + format = __image_format_v4l2_lookup(v4l2); > > > > > > + > > > > > > + WARN_ON(!format); > > > > > > + return format; > > > > > > +} > > > > > > +EXPORT_SYMBOL(image_format_v4l2_lookup); > > > > > > + > > > > > > +/** > > > > > > * image_format_plane_cpp - determine the bytes per pixel value > > > > > > * @format: pointer to the image_format > > > > > > * @plane: plane index > > > > > > > > > > _______________________________________________ > > > > > dri-devel mailing list > > > > > dri-devel@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > >
On Wed, Mar 20, 2019 at 06:15:54PM +0000, Brian Starkey wrote: > On Tue, Mar 19, 2019 at 07:29:18PM -0400, Nicolas Dufresne wrote: > > All RGB mapping should be surrounded by ifdef, because many (not all) > > DRM formats represent the order of component when placed in a CPU > > register, unlike V4L2 which uses memory order. I've pick this one > > randomly, but this one on most system, little endian, will match > > V4L2_PIX_FMT_XBGR32. This type of complex mapping can be found in > > multiple places, notably in GStreamer: > > > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/sys/kms/gstkmsutils.c#L45 > > I do sort-of wonder if it's worth trying to switch to common fourccs > between DRM and V4L2 (and whatever else there is). > > The V4L2 formats list is quite incomplete and a little quirky in > places (V4L2_PIX_FORMAT_XBGR32 and V4L2_PIX_FORMAT_XRGB32 naming > inconsistency being one. 'X' isn't even next to 'B' in XBGR32). > > At least for newly-added formats, not using a common definition > doesn't make a lot of sense to me. Longer term, I also don't really > see any downsides to unification. Eventually, I agree that that his where we should be heading. Moving the existing formats support to a common place will help with that. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Hi, Le mercredi 20 mars 2019 à 20:39 +0200, Ville Syrjälä a écrit : > On Wed, Mar 20, 2019 at 02:27:59PM -0400, Nicolas Dufresne wrote: > > Le mercredi 20 mars 2019 à 18:41 +0200, Ville Syrjälä a écrit : > > > On Wed, Mar 20, 2019 at 12:30:25PM -0400, Nicolas Dufresne wrote: > > > > Le mercredi 20 mars 2019 à 18:09 +0200, Ville Syrjälä a écrit : > > > > > On Wed, Mar 20, 2019 at 11:51:58AM -0400, Nicolas Dufresne wrote: > > > > > > Le mercredi 20 mars 2019 à 16:27 +0200, Ville Syrjälä a écrit : > > > > > > > On Tue, Mar 19, 2019 at 07:29:18PM -0400, Nicolas Dufresne wrote: > > > > > > > > Le mardi 19 mars 2019 à 22:57 +0100, Maxime Ripard a écrit : > > > > > > > > > V4L2 uses different fourcc's than DRM, and has a different set of formats. > > > > > > > > > For now, let's add the v4l2 fourcc's for the already existing formats. > > > > > > > > > > > > > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > > > > --- > > > > > > > > > include/linux/image-formats.h | 9 +++++- > > > > > > > > > lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- > > > > > > > > > 2 files changed, 76 insertions(+) > > > > > > > > > > > > > > > > > > diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h > > > > > > > > > index 53fd73a71b3d..fbc3a4501ebd 100644 > > > > > > > > > --- a/include/linux/image-formats.h > > > > > > > > > +++ b/include/linux/image-formats.h > > > > > > > > > @@ -26,6 +26,13 @@ struct image_format_info { > > > > > > > > > }; > > > > > > > > > > > > > > > > > > /** > > > > > > > > > + * @v4l2_fmt: > > > > > > > > > + * > > > > > > > > > + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) > > > > > > > > > + */ > > > > > > > > > + u32 v4l2_fmt; > > > > > > > > > + > > > > > > > > > + /** > > > > > > > > > * @depth: > > > > > > > > > * > > > > > > > > > * Color depth (number of bits per pixel excluding padding bits), > > > > > > > > > @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) > > > > > > > > > > > > > > > > > > const struct image_format_info *__image_format_drm_lookup(u32 drm); > > > > > > > > > const struct image_format_info *image_format_drm_lookup(u32 drm); > > > > > > > > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); > > > > > > > > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); > > > > > > > > > unsigned int image_format_plane_cpp(const struct image_format_info *format, > > > > > > > > > int plane); > > > > > > > > > unsigned int image_format_plane_width(int width, > > > > > > > > > diff --git a/lib/image-formats.c b/lib/image-formats.c > > > > > > > > > index 9b9a73220c5d..39f1d38ae861 100644 > > > > > > > > > --- a/lib/image-formats.c > > > > > > > > > +++ b/lib/image-formats.c > > > > > > > > > @@ -8,6 +8,7 @@ > > > > > > > > > static const struct image_format_info formats[] = { > > > > > > > > > { > > > > > > > > > .drm_fmt = DRM_FORMAT_C8, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_GREY, > > > > > > > > > .depth = 8, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 1, 0, 0 }, > > > > > > > > > @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .vsub = 1, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_RGB332, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB332, > > > > > > > > > .depth = 8, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 1, 0, 0 }, > > > > > > > > > @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .vsub = 1, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_XRGB4444, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .vsub = 1, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_ARGB4444, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .has_alpha = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_XRGB1555, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, > > > > > > > > > .depth = 15, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .vsub = 1, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_ARGB1555, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, > > > > > > > > > .depth = 15, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .has_alpha = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_RGB565, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB565, > > > > > > > > > .depth = 16, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .vsub = 1, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_RGB888, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB24, > > > > > > > > > .depth = 24, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 3, 0, 0 }, > > > > > > > > > @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .vsub = 1, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_BGR888, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_BGR24, > > > > > > > > > .depth = 24, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 3, 0, 0 }, > > > > > > > > > @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .vsub = 1, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_XRGB8888, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, > > > > > > > > > > > > > > > > All RGB mapping should be surrounded by ifdef, because many (not all) > > > > > > > > DRM formats represent the order of component when placed in a CPU > > > > > > > > register, unlike V4L2 which uses memory order. I've pick this one > > > > > > > > > > > > > > DRM formats are explicitly defined as little endian. > > > > > > > > > > > > Yes, that means the same thing. The mapping has nothing to do with the > > > > > > buffer bytes order, unlike V4L2 (and most streaming stack) do. > > > > > > > > > > It has everything to do with byte order. Little endian means the least > > > > > significant byte is stored in the first byte in memory. > > > > > > > > > > Based on https://www.kernel.org/doc/html/v4.15/media/uapi/v4l/pixfmt-packed-rgb.html > > > > > drm XRGB888 is actually v4l XBGR32, not XRGB32 as claimed by this patch. > > > > > > > > That's basically what I said, as it's define for Little Endian rather > > > > then buffer order, you have to make the mapping conditional. It > > > > basically mean that in memory, the DRM format physically differ > > > > depending on CPU endian. > > > > > > No. It is always little endian no matter what the CPU is. > > > > I'm sorry, this is in your ABI, we don't add layers of ifdef in > > userspace code just for the fun of it. If you redefine this now you are > > breaking userspace. I agree there is very little to no Big Endian on > > DRM side anymore, but what historically was mapped per CPU cannot be > > changed by you now. > > It was always little endian. I'm not sure what it's worth, but there is a "pixel format guide" project that is all about matching formats from one API to another: https://afrantzis.com/pixel-format-guide/ (and it has an associated tool too). On the page about DRM, it seems that they got the word that DRM formats are the native pixel order in little-endian systems: https://afrantzis.com/pixel-format-guide/drm.html Perhaps some drivers have been abusing the format definitions, leading to the inconsistencies that Nicolas could witness? Cheers, Paul > > > > Last time we have run this on PPC / Big > > > > Endian, the mapping was XRGB/XRGB, we checked that up multiple time > > > > with the DRM maintainers and was told this is exactly what it's suppose > > > > to do, hence this endian dependant mapping all over the place. If you > > > > make up that this isn't right, you are breaking userspace, and people > > > > don't like that. > > > > > > Someone recently added those DRM_FORMAT_HOST variants to allow > > > the legacy addfb1 to create pick the format based on host > > > endianness. I thought that was the only conclusion from the > > > little vs. big endian drm fourcc wars. > > > > > > > So the mapping should be: > > > > Little: DRM XRGB / V4L2 XBGR > > > > Big: DRM XRGB / V4L2 XRGB > > > > > > > > > > > > randomly, but this one on most system, little endian, will match > > > > > > > > V4L2_PIX_FMT_XBGR32. This type of complex mapping can be found in > > > > > > > > multiple places, notably in GStreamer: > > > > > > > > > > > > > > > > https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/sys/kms/gstkmsutils.c#L45 > > > > > > > > > > > > > > > > > .depth = 24, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 4, 0, 0 }, > > > > > > > > > @@ -281,6 +291,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .has_alpha = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_ARGB8888, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB32, > > > > > > > > > .depth = 32, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 4, 0, 0 }, > > > > > > > > > @@ -361,6 +372,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .has_alpha = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_YUV410, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV410, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 3, > > > > > > > > > .cpp = { 1, 1, 1 }, > > > > > > > > > @@ -369,6 +381,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_YVU410, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU410, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 3, > > > > > > > > > .cpp = { 1, 1, 1 }, > > > > > > > > > @@ -393,6 +406,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_YUV420, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV420M, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 3, > > > > > > > > > .cpp = { 1, 1, 1 }, > > > > > > > > > @@ -401,6 +415,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_YVU420, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU420M, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 3, > > > > > > > > > .cpp = { 1, 1, 1 }, > > > > > > > > > @@ -409,6 +424,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_YUV422, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV422M, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 3, > > > > > > > > > .cpp = { 1, 1, 1 }, > > > > > > > > > @@ -417,6 +433,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_YVU422, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU422M, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 3, > > > > > > > > > .cpp = { 1, 1, 1 }, > > > > > > > > > @@ -425,6 +442,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_YUV444, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUV444M, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 3, > > > > > > > > > .cpp = { 1, 1, 1 }, > > > > > > > > > @@ -433,6 +451,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_YVU444, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVU444M, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 3, > > > > > > > > > .cpp = { 1, 1, 1 }, > > > > > > > > > @@ -441,6 +460,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_NV12, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV12, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 2, > > > > > > > > > .cpp = { 1, 2, 0 }, > > > > > > > > > @@ -449,6 +469,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_NV21, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV21, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 2, > > > > > > > > > .cpp = { 1, 2, 0 }, > > > > > > > > > @@ -457,6 +478,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_NV16, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV16, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 2, > > > > > > > > > .cpp = { 1, 2, 0 }, > > > > > > > > > @@ -465,6 +487,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_NV61, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV61, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 2, > > > > > > > > > .cpp = { 1, 2, 0 }, > > > > > > > > > @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_NV24, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV24, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 2, > > > > > > > > > .cpp = { 1, 2, 0 }, > > > > > > > > > @@ -481,6 +505,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_NV42, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_NV42, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 2, > > > > > > > > > .cpp = { 1, 2, 0 }, > > > > > > > > > @@ -489,6 +514,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_YUYV, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YUYV, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > @@ -497,6 +523,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_YVYU, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_YVYU, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > @@ -505,6 +532,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_UYVY, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_UYVY, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > @@ -513,6 +541,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > .is_yuv = true, > > > > > > > > > }, { > > > > > > > > > .drm_fmt = DRM_FORMAT_VYUY, > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_VYUY, > > > > > > > > > .depth = 0, > > > > > > > > > .num_planes = 1, > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > @@ -632,6 +661,44 @@ const struct image_format_info *image_format_drm_lookup(u32 drm) > > > > > > > > > EXPORT_SYMBOL(image_format_drm_lookup); > > > > > > > > > > > > > > > > > > /** > > > > > > > > > + * __image_format_v4l2_lookup - query information for a given format > > > > > > > > > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > > > > > > > > > + * > > > > > > > > > + * The caller should only pass a supported pixel format to this function. > > > > > > > > > + * > > > > > > > > > + * Returns: > > > > > > > > > + * The instance of struct image_format_info that describes the pixel format, or > > > > > > > > > + * NULL if the format is unsupported. > > > > > > > > > + */ > > > > > > > > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2) > > > > > > > > > +{ > > > > > > > > > + return __image_format_lookup(v4l2_fmt, v4l2); > > > > > > > > > +} > > > > > > > > > +EXPORT_SYMBOL(__image_format_v4l2_lookup); > > > > > > > > > + > > > > > > > > > +/** > > > > > > > > > + * image_format_v4l2_lookup - query information for a given format > > > > > > > > > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > > > > > > > > > + * > > > > > > > > > + * The caller should only pass a supported pixel format to this function. > > > > > > > > > + * Unsupported pixel formats will generate a warning in the kernel log. > > > > > > > > > + * > > > > > > > > > + * Returns: > > > > > > > > > + * The instance of struct image_format_info that describes the pixel format, or > > > > > > > > > + * NULL if the format is unsupported. > > > > > > > > > + */ > > > > > > > > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2) > > > > > > > > > +{ > > > > > > > > > + const struct image_format_info *format; > > > > > > > > > + > > > > > > > > > + format = __image_format_v4l2_lookup(v4l2); > > > > > > > > > + > > > > > > > > > + WARN_ON(!format); > > > > > > > > > + return format; > > > > > > > > > +} > > > > > > > > > +EXPORT_SYMBOL(image_format_v4l2_lookup); > > > > > > > > > + > > > > > > > > > +/** > > > > > > > > > * image_format_plane_cpp - determine the bytes per pixel value > > > > > > > > > * @format: pointer to the image_format > > > > > > > > > * @plane: plane index > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > dri-devel mailing list > > > > > > > > dri-devel@lists.freedesktop.org > > > > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > >
On Thu, Mar 21, 2019 at 05:04:19PM +0100, Paul Kocialkowski wrote: > Hi, > > Le mercredi 20 mars 2019 à 20:39 +0200, Ville Syrjälä a écrit : > > On Wed, Mar 20, 2019 at 02:27:59PM -0400, Nicolas Dufresne wrote: > > > Le mercredi 20 mars 2019 à 18:41 +0200, Ville Syrjälä a écrit : > > > > On Wed, Mar 20, 2019 at 12:30:25PM -0400, Nicolas Dufresne wrote: > > > > > Le mercredi 20 mars 2019 à 18:09 +0200, Ville Syrjälä a écrit : > > > > > > On Wed, Mar 20, 2019 at 11:51:58AM -0400, Nicolas Dufresne wrote: > > > > > > > Le mercredi 20 mars 2019 à 16:27 +0200, Ville Syrjälä a écrit : > > > > > > > > On Tue, Mar 19, 2019 at 07:29:18PM -0400, Nicolas Dufresne wrote: > > > > > > > > > Le mardi 19 mars 2019 à 22:57 +0100, Maxime Ripard a écrit : > > > > > > > > > > V4L2 uses different fourcc's than DRM, and has a different set of formats. > > > > > > > > > > For now, let's add the v4l2 fourcc's for the already existing formats. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > > > > > --- > > > > > > > > > > include/linux/image-formats.h | 9 +++++- > > > > > > > > > > lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- > > > > > > > > > > 2 files changed, 76 insertions(+) > > > > > > > > > > > > > > > > > > > > diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h > > > > > > > > > > index 53fd73a71b3d..fbc3a4501ebd 100644 > > > > > > > > > > --- a/include/linux/image-formats.h > > > > > > > > > > +++ b/include/linux/image-formats.h > > > > > > > > > > @@ -26,6 +26,13 @@ struct image_format_info { > > > > > > > > > > }; > > > > > > > > > > > > > > > > > > > > /** > > > > > > > > > > + * @v4l2_fmt: > > > > > > > > > > + * > > > > > > > > > > + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) > > > > > > > > > > + */ > > > > > > > > > > + u32 v4l2_fmt; > > > > > > > > > > + > > > > > > > > > > + /** > > > > > > > > > > * @depth: > > > > > > > > > > * > > > > > > > > > > * Color depth (number of bits per pixel excluding padding bits), > > > > > > > > > > @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) > > > > > > > > > > > > > > > > > > > > const struct image_format_info *__image_format_drm_lookup(u32 drm); > > > > > > > > > > const struct image_format_info *image_format_drm_lookup(u32 drm); > > > > > > > > > > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); > > > > > > > > > > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); > > > > > > > > > > unsigned int image_format_plane_cpp(const struct image_format_info *format, > > > > > > > > > > int plane); > > > > > > > > > > unsigned int image_format_plane_width(int width, > > > > > > > > > > diff --git a/lib/image-formats.c b/lib/image-formats.c > > > > > > > > > > index 9b9a73220c5d..39f1d38ae861 100644 > > > > > > > > > > --- a/lib/image-formats.c > > > > > > > > > > +++ b/lib/image-formats.c > > > > > > > > > > @@ -8,6 +8,7 @@ > > > > > > > > > > static const struct image_format_info formats[] = { > > > > > > > > > > { > > > > > > > > > > .drm_fmt = DRM_FORMAT_C8, > > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_GREY, > > > > > > > > > > .depth = 8, > > > > > > > > > > .num_planes = 1, > > > > > > > > > > .cpp = { 1, 0, 0 }, > > > > > > > > > > @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > > .vsub = 1, > > > > > > > > > > }, { > > > > > > > > > > .drm_fmt = DRM_FORMAT_RGB332, > > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB332, > > > > > > > > > > .depth = 8, > > > > > > > > > > .num_planes = 1, > > > > > > > > > > .cpp = { 1, 0, 0 }, > > > > > > > > > > @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > > .vsub = 1, > > > > > > > > > > }, { > > > > > > > > > > .drm_fmt = DRM_FORMAT_XRGB4444, > > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, > > > > > > > > > > .depth = 0, > > > > > > > > > > .num_planes = 1, > > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > > @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > > .vsub = 1, > > > > > > > > > > }, { > > > > > > > > > > .drm_fmt = DRM_FORMAT_ARGB4444, > > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, > > > > > > > > > > .depth = 0, > > > > > > > > > > .num_planes = 1, > > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > > @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > > .has_alpha = true, > > > > > > > > > > }, { > > > > > > > > > > .drm_fmt = DRM_FORMAT_XRGB1555, > > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, > > > > > > > > > > .depth = 15, > > > > > > > > > > .num_planes = 1, > > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > > @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > > .vsub = 1, > > > > > > > > > > }, { > > > > > > > > > > .drm_fmt = DRM_FORMAT_ARGB1555, > > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, > > > > > > > > > > .depth = 15, > > > > > > > > > > .num_planes = 1, > > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > > @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > > .has_alpha = true, > > > > > > > > > > }, { > > > > > > > > > > .drm_fmt = DRM_FORMAT_RGB565, > > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB565, > > > > > > > > > > .depth = 16, > > > > > > > > > > .num_planes = 1, > > > > > > > > > > .cpp = { 2, 0, 0 }, > > > > > > > > > > @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > > .vsub = 1, > > > > > > > > > > }, { > > > > > > > > > > .drm_fmt = DRM_FORMAT_RGB888, > > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_RGB24, > > > > > > > > > > .depth = 24, > > > > > > > > > > .num_planes = 1, > > > > > > > > > > .cpp = { 3, 0, 0 }, > > > > > > > > > > @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > > .vsub = 1, > > > > > > > > > > }, { > > > > > > > > > > .drm_fmt = DRM_FORMAT_BGR888, > > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_BGR24, > > > > > > > > > > .depth = 24, > > > > > > > > > > .num_planes = 1, > > > > > > > > > > .cpp = { 3, 0, 0 }, > > > > > > > > > > @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { > > > > > > > > > > .vsub = 1, > > > > > > > > > > }, { > > > > > > > > > > .drm_fmt = DRM_FORMAT_XRGB8888, > > > > > > > > > > + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, > > > > > > > > > > > > > > > > > > All RGB mapping should be surrounded by ifdef, because many (not all) > > > > > > > > > DRM formats represent the order of component when placed in a CPU > > > > > > > > > register, unlike V4L2 which uses memory order. I've pick this one > > > > > > > > > > > > > > > > DRM formats are explicitly defined as little endian. > > > > > > > > > > > > > > Yes, that means the same thing. The mapping has nothing to do with the > > > > > > > buffer bytes order, unlike V4L2 (and most streaming stack) do. > > > > > > > > > > > > It has everything to do with byte order. Little endian means the least > > > > > > significant byte is stored in the first byte in memory. > > > > > > > > > > > > Based on https://www.kernel.org/doc/html/v4.15/media/uapi/v4l/pixfmt-packed-rgb.html > > > > > > drm XRGB888 is actually v4l XBGR32, not XRGB32 as claimed by this patch. > > > > > > > > > > That's basically what I said, as it's define for Little Endian rather > > > > > then buffer order, you have to make the mapping conditional. It > > > > > basically mean that in memory, the DRM format physically differ > > > > > depending on CPU endian. > > > > > > > > No. It is always little endian no matter what the CPU is. > > > > > > I'm sorry, this is in your ABI, we don't add layers of ifdef in > > > userspace code just for the fun of it. If you redefine this now you are > > > breaking userspace. I agree there is very little to no Big Endian on > > > DRM side anymore, but what historically was mapped per CPU cannot be > > > changed by you now. > > > > It was always little endian. > > I'm not sure what it's worth, but there is a "pixel format guide" > project that is all about matching formats from one API to another: > https://afrantzis.com/pixel-format-guide/ (and it has an associated > tool too). > > On the page about DRM, it seems that they got the word that DRM formats > are the native pixel order in little-endian systems: > https://afrantzis.com/pixel-format-guide/drm.html Looks consistent with the official word in drm_fourcc.h. $ python3 -m pfg find-compatible V4L2_PIX_FMT_XBGR32 drm Format: V4L2_PIX_FMT_XBGR32 Is compatible on all systems with: DRM_FORMAT_XRGB8888 Is compatible on little-endian systems with: Is compatible on big-endian systems with: $ python3 -m pfg find-compatible DRM_FORMAT_XRGB8888 v4l2 Format: DRM_FORMAT_XRGB8888 Is compatible on all systems with: V4L2_PIX_FMT_XBGR32 Is compatible on little-endian systems with: Is compatible on big-endian systems with: Even works both ways :) > > Perhaps some drivers have been abusing the format definitions, leading > to the inconsistencies that Nicolas could witness? That is quite possible, perhaps even likely. No one really seems interested in making sure big endian systems actually work properly. I believe the usual approach is to hack around semi-rnadomly until the correct colors accidentally appear on the screen.
Le jeudi 21 mars 2019 à 18:35 +0200, Ville Syrjälä a écrit : > > I'm not sure what it's worth, but there is a "pixel format guide" > > project that is all about matching formats from one API to another: > > https://afrantzis.com/pixel-format-guide/ (and it has an associated > > tool too). > > > > On the page about DRM, it seems that they got the word that DRM formats > > are the native pixel order in little-endian systems: > > https://afrantzis.com/pixel-format-guide/drm.html > > Looks consistent with the official word in drm_fourcc.h. > > $ python3 -m pfg find-compatible V4L2_PIX_FMT_XBGR32 drm > Format: V4L2_PIX_FMT_XBGR32 > Is compatible on all systems with: > DRM_FORMAT_XRGB8888 > Is compatible on little-endian systems with: > Is compatible on big-endian systems with: > > $ python3 -m pfg find-compatible DRM_FORMAT_XRGB8888 v4l2 > Format: DRM_FORMAT_XRGB8888 > Is compatible on all systems with: > V4L2_PIX_FMT_XBGR32 > Is compatible on little-endian systems with: > Is compatible on big-endian systems with: > > Even works both ways :) > > > Perhaps some drivers have been abusing the format definitions, leading > > to the inconsistencies that Nicolas could witness? > > That is quite possible, perhaps even likely. No one really > seems interested in making sure big endian systems actually > work properly. I believe the usual approach is to hack > around semi-rnadomly until the correct colors accidentally > appear on the screen. We did not hack around randomly. The code in GStreamer is exactly what the DRM and Wayland dev told us to do (they provided the initial patches to make it work). These are initially patches from Intel for what it's worth (see the wlvideoformat.c header [0]). Even though I just notice that in this file, it seems that we ended up with a different mapping order for WL and DRM format in 24bit RGB (no padding), clearly is a bug. That being said, there is no logical meaning for little endian 24bit RGB, you can't load a pixel on CPU in a single op. Just saying since I would not know which one of the two mapping here is right. I would probably have to go testing what DRM drivers do, which may not mean anything. I would also ask and get contradictory answers. I have never myself tested these on big endian drivers though, as you say, nobody seems to care about graphics on those anymore. So the easy statement is to say they are little endian, like you just did, and ignore the legacy, but there is some catch. YUV formats has been added without applying this rules. So V4L2 YUYV match YUYV in DRM format name instead of little endian UYVY. (at least 4 tested drivers implements it this way). Same for NV12, for which little endian CPU representation would swap the UV paid on a 16bit word. To me, all the YUV stuff is the right way, because this is what the rest of the world is doing, it's not ambiguous. [0] https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/ext/wayland/wlvideoformat.c#L86 Nicolas
On Thu, Mar 21, 2019 at 03:14:06PM -0400, Nicolas Dufresne wrote: > Le jeudi 21 mars 2019 à 18:35 +0200, Ville Syrjälä a écrit : > > > I'm not sure what it's worth, but there is a "pixel format guide" > > > project that is all about matching formats from one API to another: > > > https://afrantzis.com/pixel-format-guide/ (and it has an associated > > > tool too). > > > > > > On the page about DRM, it seems that they got the word that DRM formats > > > are the native pixel order in little-endian systems: > > > https://afrantzis.com/pixel-format-guide/drm.html > > > > Looks consistent with the official word in drm_fourcc.h. > > > > $ python3 -m pfg find-compatible V4L2_PIX_FMT_XBGR32 drm > > Format: V4L2_PIX_FMT_XBGR32 > > Is compatible on all systems with: > > DRM_FORMAT_XRGB8888 > > Is compatible on little-endian systems with: > > Is compatible on big-endian systems with: > > > > $ python3 -m pfg find-compatible DRM_FORMAT_XRGB8888 v4l2 > > Format: DRM_FORMAT_XRGB8888 > > Is compatible on all systems with: > > V4L2_PIX_FMT_XBGR32 > > Is compatible on little-endian systems with: > > Is compatible on big-endian systems with: > > > > Even works both ways :) > > > > > Perhaps some drivers have been abusing the format definitions, leading > > > to the inconsistencies that Nicolas could witness? > > > > That is quite possible, perhaps even likely. No one really > > seems interested in making sure big endian systems actually > > work properly. I believe the usual approach is to hack > > around semi-rnadomly until the correct colors accidentally > > appear on the screen. > > We did not hack around randomly. The code in GStreamer is exactly what > the DRM and Wayland dev told us to do (they provided the initial > patches to make it work). These are initially patches from Intel for > what it's worth (see the wlvideoformat.c header [0]). Even though I > just notice that in this file, it seems that we ended up with a > different mapping order for WL and DRM format in 24bit RGB (no > padding), clearly is a bug. That being said, there is no logical > meaning for little endian 24bit RGB, you can't load a pixel on CPU in a > single op. To me little endian just means "little end comes first". So if you think of things as just a stream of bytes CPU word size etc. doesn't matter. And I guess if you really wanted to you could even make a CPU with 24bit word size. Anyways, to make things more clear drm_fourcc.h could probably document things better by showing explicitly which bits go into which byte. I don't know who did what patches for whatever project, but clearly something has been lost in translation at some point. > Just saying since I would not know which one of the two > mapping here is right. I would probably have to go testing what DRM > drivers do, which may not mean anything. I would also ask and get > contradictory answers. > > I have never myself tested these on big endian drivers though, as you > say, nobody seems to care about graphics on those anymore. So the easy > statement is to say they are little endian, like you just did, and > ignore the legacy, but there is some catch. YUV formats has been added > without applying this rules. All drm formats follow the same rule (ignoring the recently added non-byte aligned stuff which I guess don't really follow any rules). > So V4L2 YUYV match YUYV in DRM format name > instead of little endian UYVY. (at least 4 tested drivers implements it > this way). Same for NV12, for which little endian CPU representation > would swap the UV paid on a 16bit word. DRM NV12 and YUYV (YUY2) match the NV12 and YUY2 defined here https://docs.microsoft.com/en-us/windows/desktop/medfound/recommended-8-bit-yuv-formats-for-video-rendering > > To me, all the YUV stuff is the right way, because this is what the > rest of the world is doing, it's not ambiguous. > > [0] https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/ext/wayland/wlvideoformat.c#L86 > > > > Nicolas
On Thu, Mar 21, 2019 at 03:14:06PM -0400, Nicolas Dufresne wrote: > Le jeudi 21 mars 2019 à 18:35 +0200, Ville Syrjälä a écrit : > > > I'm not sure what it's worth, but there is a "pixel format guide" > > > project that is all about matching formats from one API to another: > > > https://afrantzis.com/pixel-format-guide/ (and it has an associated > > > tool too). > > > > > > On the page about DRM, it seems that they got the word that DRM formats > > > are the native pixel order in little-endian systems: > > > https://afrantzis.com/pixel-format-guide/drm.html > > > > Looks consistent with the official word in drm_fourcc.h. > > > > $ python3 -m pfg find-compatible V4L2_PIX_FMT_XBGR32 drm > > Format: V4L2_PIX_FMT_XBGR32 > > Is compatible on all systems with: > > DRM_FORMAT_XRGB8888 > > Is compatible on little-endian systems with: > > Is compatible on big-endian systems with: > > > > $ python3 -m pfg find-compatible DRM_FORMAT_XRGB8888 v4l2 > > Format: DRM_FORMAT_XRGB8888 > > Is compatible on all systems with: > > V4L2_PIX_FMT_XBGR32 > > Is compatible on little-endian systems with: > > Is compatible on big-endian systems with: > > > > Even works both ways :) > > > > > Perhaps some drivers have been abusing the format definitions, leading > > > to the inconsistencies that Nicolas could witness? > > > > That is quite possible, perhaps even likely. No one really > > seems interested in making sure big endian systems actually > > work properly. I believe the usual approach is to hack > > around semi-rnadomly until the correct colors accidentally > > appear on the screen. > > We did not hack around randomly. BTW I didn't mean to imply it was you who hacked around randomly. Sorry if you got that impression. What I was trying to convey is the following sequence of events: 1. random person X gets their hand on a big endian machine for a while 2. colors are wrong 3. they hack stuff until the colors are correct in their current use case 4. they move on to more interesting things
Le vendredi 22 mars 2019 à 16:42 +0200, Ville Syrjälä a écrit : > On Thu, Mar 21, 2019 at 03:14:06PM -0400, Nicolas Dufresne wrote: > > Le jeudi 21 mars 2019 à 18:35 +0200, Ville Syrjälä a écrit : > > > > I'm not sure what it's worth, but there is a "pixel format guide" > > > > project that is all about matching formats from one API to another: > > > > https://afrantzis.com/pixel-format-guide/ (and it has an associated > > > > tool too). > > > > > > > > On the page about DRM, it seems that they got the word that DRM formats > > > > are the native pixel order in little-endian systems: > > > > https://afrantzis.com/pixel-format-guide/drm.html > > > > > > Looks consistent with the official word in drm_fourcc.h. > > > > > > $ python3 -m pfg find-compatible V4L2_PIX_FMT_XBGR32 drm > > > Format: V4L2_PIX_FMT_XBGR32 > > > Is compatible on all systems with: > > > DRM_FORMAT_XRGB8888 > > > Is compatible on little-endian systems with: > > > Is compatible on big-endian systems with: > > > > > > $ python3 -m pfg find-compatible DRM_FORMAT_XRGB8888 v4l2 > > > Format: DRM_FORMAT_XRGB8888 > > > Is compatible on all systems with: > > > V4L2_PIX_FMT_XBGR32 > > > Is compatible on little-endian systems with: > > > Is compatible on big-endian systems with: > > > > > > Even works both ways :) > > > > > > > Perhaps some drivers have been abusing the format definitions, leading > > > > to the inconsistencies that Nicolas could witness? > > > > > > That is quite possible, perhaps even likely. No one really > > > seems interested in making sure big endian systems actually > > > work properly. I believe the usual approach is to hack > > > around semi-rnadomly until the correct colors accidentally > > > appear on the screen. > > > > We did not hack around randomly. > > BTW I didn't mean to imply it was you who hacked around randomly. > Sorry if you got that impression. > > What I was trying to convey is the following sequence of events: > 1. random person X gets their hand on a big endian machine for > a while > 2. colors are wrong > 3. they hack stuff until the colors are correct in their > current use case > 4. they move on to more interesting things Thanks for clarification. Nicolas
Le jeudi 21 mars 2019 à 23:44 +0200, Ville Syrjälä a écrit : > On Thu, Mar 21, 2019 at 03:14:06PM -0400, Nicolas Dufresne wrote: > > Le jeudi 21 mars 2019 à 18:35 +0200, Ville Syrjälä a écrit : > > > > I'm not sure what it's worth, but there is a "pixel format guide" > > > > project that is all about matching formats from one API to another: > > > > https://afrantzis.com/pixel-format-guide/ (and it has an associated > > > > tool too). > > > > > > > > On the page about DRM, it seems that they got the word that DRM formats > > > > are the native pixel order in little-endian systems: > > > > https://afrantzis.com/pixel-format-guide/drm.html > > > > > > Looks consistent with the official word in drm_fourcc.h. > > > > > > $ python3 -m pfg find-compatible V4L2_PIX_FMT_XBGR32 drm > > > Format: V4L2_PIX_FMT_XBGR32 > > > Is compatible on all systems with: > > > DRM_FORMAT_XRGB8888 > > > Is compatible on little-endian systems with: > > > Is compatible on big-endian systems with: > > > > > > $ python3 -m pfg find-compatible DRM_FORMAT_XRGB8888 v4l2 > > > Format: DRM_FORMAT_XRGB8888 > > > Is compatible on all systems with: > > > V4L2_PIX_FMT_XBGR32 > > > Is compatible on little-endian systems with: > > > Is compatible on big-endian systems with: > > > > > > Even works both ways :) > > > > > > > Perhaps some drivers have been abusing the format definitions, leading > > > > to the inconsistencies that Nicolas could witness? > > > > > > That is quite possible, perhaps even likely. No one really > > > seems interested in making sure big endian systems actually > > > work properly. I believe the usual approach is to hack > > > around semi-rnadomly until the correct colors accidentally > > > appear on the screen. > > > > We did not hack around randomly. The code in GStreamer is exactly what > > the DRM and Wayland dev told us to do (they provided the initial > > patches to make it work). These are initially patches from Intel for > > what it's worth (see the wlvideoformat.c header [0]). Even though I > > just notice that in this file, it seems that we ended up with a > > different mapping order for WL and DRM format in 24bit RGB (no > > padding), clearly is a bug. That being said, there is no logical > > meaning for little endian 24bit RGB, you can't load a pixel on CPU in a > > single op. > > To me little endian just means "little end comes first". So if > you think of things as just a stream of bytes CPU word size > etc. doesn't matter. And I guess if you really wanted to you > could even make a CPU with 24bit word size. > > Anyways, to make things more clear drm_fourcc.h could probably > document things better by showing explicitly which bits go into > which byte. > > I don't know who did what patches for whatever project, but > clearly something has been lost in translation at some point. > > > Just saying since I would not know which one of the two > > mapping here is right. I would probably have to go testing what DRM > > drivers do, which may not mean anything. I would also ask and get > > contradictory answers. > > > > I have never myself tested these on big endian drivers though, as you > > say, nobody seems to care about graphics on those anymore. So the easy > > statement is to say they are little endian, like you just did, and > > ignore the legacy, but there is some catch. YUV formats has been added > > without applying this rules. > > All drm formats follow the same rule (ignoring the recently added > non-byte aligned stuff which I guess don't really follow any rules). > > > So V4L2 YUYV match YUYV in DRM format name > > instead of little endian UYVY. (at least 4 tested drivers implements it > > this way). Same for NV12, for which little endian CPU representation > > would swap the UV paid on a 16bit word. > > DRM NV12 and YUYV (YUY2) match the NV12 and YUY2 defined here > https://docs.microsoft.com/en-us/windows/desktop/medfound/recommended-8-bit-yuv-formats-for-video-rendering Problem is that these are all expressed MSB first like (the way V4L2 and GStreamer do appart for the padding X, which is usually first in V4L2). Let's say you have 2 pixels of YUYV in a 32bit buffer. buf[0] = Y buf[1] = U buf[2] = Y buf[3] = V While with LSB first (was this what you wanted to say ?), this format would be VYUY as: buf[0] = V buf[1] = Y buf[2] = U buf[3] = Y But I tested this format on i915, xlnx, msm and imx drm drivers, and they are all mapped as MSB. The LSB version of NV12 is called NV21 in MS pixel formats. It would be really confusing if the LSB rule was to be applied to these format in my opinion, because the names don't explicitly express the placement for the components. Anyway, to cut short, as per currently tested implementation of DRM driver, we can keep the RGB mapping static (reversed) for now, but for Maxime, who probably have no clue about all this, the YUYV mapping is correct in this patch (in as of currently tested drivers). > > > To me, all the YUV stuff is the right way, because this is what the > > rest of the world is doing, it's not ambiguous. > > > > [0] https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/ext/wayland/wlvideoformat.c#L86 > > > > > > > > Nicolas
On Fri, Mar 22, 2019 at 02:24:29PM -0400, Nicolas Dufresne wrote: > Le jeudi 21 mars 2019 à 23:44 +0200, Ville Syrjälä a écrit : > > On Thu, Mar 21, 2019 at 03:14:06PM -0400, Nicolas Dufresne wrote: > > > Le jeudi 21 mars 2019 à 18:35 +0200, Ville Syrjälä a écrit : > > > > > I'm not sure what it's worth, but there is a "pixel format guide" > > > > > project that is all about matching formats from one API to another: > > > > > https://afrantzis.com/pixel-format-guide/ (and it has an associated > > > > > tool too). > > > > > > > > > > On the page about DRM, it seems that they got the word that DRM formats > > > > > are the native pixel order in little-endian systems: > > > > > https://afrantzis.com/pixel-format-guide/drm.html > > > > > > > > Looks consistent with the official word in drm_fourcc.h. > > > > > > > > $ python3 -m pfg find-compatible V4L2_PIX_FMT_XBGR32 drm > > > > Format: V4L2_PIX_FMT_XBGR32 > > > > Is compatible on all systems with: > > > > DRM_FORMAT_XRGB8888 > > > > Is compatible on little-endian systems with: > > > > Is compatible on big-endian systems with: > > > > > > > > $ python3 -m pfg find-compatible DRM_FORMAT_XRGB8888 v4l2 > > > > Format: DRM_FORMAT_XRGB8888 > > > > Is compatible on all systems with: > > > > V4L2_PIX_FMT_XBGR32 > > > > Is compatible on little-endian systems with: > > > > Is compatible on big-endian systems with: > > > > > > > > Even works both ways :) > > > > > > > > > Perhaps some drivers have been abusing the format definitions, leading > > > > > to the inconsistencies that Nicolas could witness? > > > > > > > > That is quite possible, perhaps even likely. No one really > > > > seems interested in making sure big endian systems actually > > > > work properly. I believe the usual approach is to hack > > > > around semi-rnadomly until the correct colors accidentally > > > > appear on the screen. > > > > > > We did not hack around randomly. The code in GStreamer is exactly what > > > the DRM and Wayland dev told us to do (they provided the initial > > > patches to make it work). These are initially patches from Intel for > > > what it's worth (see the wlvideoformat.c header [0]). Even though I > > > just notice that in this file, it seems that we ended up with a > > > different mapping order for WL and DRM format in 24bit RGB (no > > > padding), clearly is a bug. That being said, there is no logical > > > meaning for little endian 24bit RGB, you can't load a pixel on CPU in a > > > single op. > > > > To me little endian just means "little end comes first". So if > > you think of things as just a stream of bytes CPU word size > > etc. doesn't matter. And I guess if you really wanted to you > > could even make a CPU with 24bit word size. > > > > Anyways, to make things more clear drm_fourcc.h could probably > > document things better by showing explicitly which bits go into > > which byte. > > > > I don't know who did what patches for whatever project, but > > clearly something has been lost in translation at some point. > > > > > Just saying since I would not know which one of the two > > > mapping here is right. I would probably have to go testing what DRM > > > drivers do, which may not mean anything. I would also ask and get > > > contradictory answers. > > > > > > I have never myself tested these on big endian drivers though, as you > > > say, nobody seems to care about graphics on those anymore. So the easy > > > statement is to say they are little endian, like you just did, and > > > ignore the legacy, but there is some catch. YUV formats has been added > > > without applying this rules. > > > > All drm formats follow the same rule (ignoring the recently added > > non-byte aligned stuff which I guess don't really follow any rules). > > > > > So V4L2 YUYV match YUYV in DRM format name > > > instead of little endian UYVY. (at least 4 tested drivers implements it > > > this way). Same for NV12, for which little endian CPU representation > > > would swap the UV paid on a 16bit word. > > > > DRM NV12 and YUYV (YUY2) match the NV12 and YUY2 defined here > > https://docs.microsoft.com/en-us/windows/desktop/medfound/recommended-8-bit-yuv-formats-for-video-rendering > > Problem is that these are all expressed MSB first like (the way V4L2 > and GStreamer do appart for the padding X, which is usually first in > V4L2). Let's say you have 2 pixels of YUYV in a 32bit buffer. > > buf[0] = Y > buf[1] = U > buf[2] = Y > buf[3] = V This is drm YUYV (aka. YUY2). Well, assuming buf[] is made up of bytes. > > While with LSB first (was this what you wanted to say ?), this format > would be VYUY as: > > buf[0] = V > buf[1] = Y > buf[2] = U > buf[3] = Y This is VYUY as far as drm is concerned. > > But I tested this format on i915, xlnx, msm and imx drm drivers, and > they are all mapped as MSB. The LSB version of NV12 is called NV21 in > MS pixel formats. It would be really confusing if the LSB rule was to > be applied to these format in my opinion, because the names don't > explicitly express the placement for the components. The names don't really mean anything. Don't try to give any any special meaning. They're just that, names. The fact that we used fourccs for them was a mistake IMO. IIRC I objected but ended up going with them to get the bikeshed painted at all. And yes, the fact that we used a different naming scheme for packed YUV vs. RGB was probably a mistake by me. But it was done long ago and we have to live with it. Fortunately the mess has gotten much worse since then and we are even more inconsistent now. We now YUV formats where the A vs. X variants use totally different naming schemes. > > Anyway, to cut short, as per currently tested implementation of DRM > driver, we can keep the RGB mapping static (reversed) for now, but for > Maxime, who probably have no clue about all this, the YUYV mapping is > correct in this patch (in as of currently tested drivers). > > > > > > To me, all the YUV stuff is the right way, because this is what the > > > rest of the world is doing, it's not ambiguous. > > > > > > [0] https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/ext/wayland/wlvideoformat.c#L86 > > > > > > > > > > > > Nicolas
Le vendredi 22 mars 2019 à 20:44 +0200, Ville Syrjälä a écrit : > On Fri, Mar 22, 2019 at 02:24:29PM -0400, Nicolas Dufresne wrote: > > Le jeudi 21 mars 2019 à 23:44 +0200, Ville Syrjälä a écrit : > > > On Thu, Mar 21, 2019 at 03:14:06PM -0400, Nicolas Dufresne wrote: > > > > Le jeudi 21 mars 2019 à 18:35 +0200, Ville Syrjälä a écrit : > > > > > > I'm not sure what it's worth, but there is a "pixel format guide" > > > > > > project that is all about matching formats from one API to another: > > > > > > https://afrantzis.com/pixel-format-guide/ (and it has an associated > > > > > > tool too). > > > > > > > > > > > > On the page about DRM, it seems that they got the word that DRM formats > > > > > > are the native pixel order in little-endian systems: > > > > > > https://afrantzis.com/pixel-format-guide/drm.html > > > > > > > > > > Looks consistent with the official word in drm_fourcc.h. > > > > > > > > > > $ python3 -m pfg find-compatible V4L2_PIX_FMT_XBGR32 drm > > > > > Format: V4L2_PIX_FMT_XBGR32 > > > > > Is compatible on all systems with: > > > > > DRM_FORMAT_XRGB8888 > > > > > Is compatible on little-endian systems with: > > > > > Is compatible on big-endian systems with: > > > > > > > > > > $ python3 -m pfg find-compatible DRM_FORMAT_XRGB8888 v4l2 > > > > > Format: DRM_FORMAT_XRGB8888 > > > > > Is compatible on all systems with: > > > > > V4L2_PIX_FMT_XBGR32 > > > > > Is compatible on little-endian systems with: > > > > > Is compatible on big-endian systems with: > > > > > > > > > > Even works both ways :) > > > > > > > > > > > Perhaps some drivers have been abusing the format definitions, leading > > > > > > to the inconsistencies that Nicolas could witness? > > > > > > > > > > That is quite possible, perhaps even likely. No one really > > > > > seems interested in making sure big endian systems actually > > > > > work properly. I believe the usual approach is to hack > > > > > around semi-rnadomly until the correct colors accidentally > > > > > appear on the screen. > > > > > > > > We did not hack around randomly. The code in GStreamer is exactly what > > > > the DRM and Wayland dev told us to do (they provided the initial > > > > patches to make it work). These are initially patches from Intel for > > > > what it's worth (see the wlvideoformat.c header [0]). Even though I > > > > just notice that in this file, it seems that we ended up with a > > > > different mapping order for WL and DRM format in 24bit RGB (no > > > > padding), clearly is a bug. That being said, there is no logical > > > > meaning for little endian 24bit RGB, you can't load a pixel on CPU in a > > > > single op. > > > > > > To me little endian just means "little end comes first". So if > > > you think of things as just a stream of bytes CPU word size > > > etc. doesn't matter. And I guess if you really wanted to you > > > could even make a CPU with 24bit word size. > > > > > > Anyways, to make things more clear drm_fourcc.h could probably > > > document things better by showing explicitly which bits go into > > > which byte. > > > > > > I don't know who did what patches for whatever project, but > > > clearly something has been lost in translation at some point. > > > > > > > Just saying since I would not know which one of the two > > > > mapping here is right. I would probably have to go testing what DRM > > > > drivers do, which may not mean anything. I would also ask and get > > > > contradictory answers. > > > > > > > > I have never myself tested these on big endian drivers though, as you > > > > say, nobody seems to care about graphics on those anymore. So the easy > > > > statement is to say they are little endian, like you just did, and > > > > ignore the legacy, but there is some catch. YUV formats has been added > > > > without applying this rules. > > > > > > All drm formats follow the same rule (ignoring the recently added > > > non-byte aligned stuff which I guess don't really follow any rules). > > > > > > > So V4L2 YUYV match YUYV in DRM format name > > > > instead of little endian UYVY. (at least 4 tested drivers implements it > > > > this way). Same for NV12, for which little endian CPU representation > > > > would swap the UV paid on a 16bit word. > > > > > > DRM NV12 and YUYV (YUY2) match the NV12 and YUY2 defined here > > > https://docs.microsoft.com/en-us/windows/desktop/medfound/recommended-8-bit-yuv-formats-for-video-rendering > > > > Problem is that these are all expressed MSB first like (the way V4L2 > > and GStreamer do appart for the padding X, which is usually first in > > V4L2). Let's say you have 2 pixels of YUYV in a 32bit buffer. > > > > buf[0] = Y > > buf[1] = U > > buf[2] = Y > > buf[3] = V > > This is drm YUYV (aka. YUY2). Well, assuming buf[] is made up of bytes. > > > While with LSB first (was this what you wanted to say ?), this format > > would be VYUY as: > > > > buf[0] = V > > buf[1] = Y > > buf[2] = U > > buf[3] = Y > > This is VYUY as far as drm is concerned. > > > But I tested this format on i915, xlnx, msm and imx drm drivers, and > > they are all mapped as MSB. The LSB version of NV12 is called NV21 in > > MS pixel formats. It would be really confusing if the LSB rule was to > > be applied to these format in my opinion, because the names don't > > explicitly express the placement for the components. > > The names don't really mean anything. Don't try to give any any special > meaning. They're just that, names. The fact that we used fourccs for > them was a mistake IMO. IIRC I objected but ended up going with them > to get the bikeshed painted at all. I can only agree with you. Note, it was not obvious to me that when you said the format are little endian you refered to the description next to the format name inside drm_fourcc.h. > > And yes, the fact that we used a different naming scheme for packed YUV > vs. RGB was probably a mistake by me. But it was done long ago and we > have to live with it. Fortunately the mess has gotten much worse since > then and we are even more inconsistent now. We now YUV formats where > the A vs. X variants use totally different naming schemes. Ok, so now that we are set, I'll retake this patch and simply comment next to each badly mapped format. > > > Anyway, to cut short, as per currently tested implementation of DRM > > driver, we can keep the RGB mapping static (reversed) for now, but for > > Maxime, who probably have no clue about all this, the YUYV mapping is > > correct in this patch (in as of currently tested drivers). > > > > > > To me, all the YUV stuff is the right way, because this is what the > > > > rest of the world is doing, it's not ambiguous. > > > > > > > > [0] https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/blob/master/ext/wayland/wlvideoformat.c#L86 > > > > > > > > > > > > > > > > Nicolas > >
Le mardi 19 mars 2019 à 22:57 +0100, Maxime Ripard a écrit : > V4L2 uses different fourcc's than DRM, and has a different set of formats. > For now, let's add the v4l2 fourcc's for the already existing formats. Hopefully I get the fixup right this time, see inline. > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > --- > include/linux/image-formats.h | 9 +++++- > lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- > 2 files changed, 76 insertions(+) > > diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h > index 53fd73a71b3d..fbc3a4501ebd 100644 > --- a/include/linux/image-formats.h > +++ b/include/linux/image-formats.h > @@ -26,6 +26,13 @@ struct image_format_info { > }; > > /** > + * @v4l2_fmt: > + * > + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) > + */ > + u32 v4l2_fmt; > + > + /** > * @depth: > * > * Color depth (number of bits per pixel excluding padding bits), > @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) > > const struct image_format_info *__image_format_drm_lookup(u32 drm); > const struct image_format_info *image_format_drm_lookup(u32 drm); > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); > unsigned int image_format_plane_cpp(const struct image_format_info *format, > int plane); > unsigned int image_format_plane_width(int width, > diff --git a/lib/image-formats.c b/lib/image-formats.c > index 9b9a73220c5d..39f1d38ae861 100644 > --- a/lib/image-formats.c > +++ b/lib/image-formats.c > @@ -8,6 +8,7 @@ > static const struct image_format_info formats[] = { > { > .drm_fmt = DRM_FORMAT_C8, > + .v4l2_fmt = V4L2_PIX_FMT_GREY, > .depth = 8, > .num_planes = 1, > .cpp = { 1, 0, 0 }, > @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_RGB332, > + .v4l2_fmt = V4L2_PIX_FMT_RGB332, > .depth = 8, > .num_planes = 1, > .cpp = { 1, 0, 0 }, > @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_XRGB4444, > + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, Not completely sure, looks like in the V4L2 variant, the padding is on the 4 MSB of the second byte, which makes it incompatible. There is no other equivalent. > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_ARGB4444, > + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, Similarly. > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { > .has_alpha = true, > }, { > .drm_fmt = DRM_FORMAT_XRGB1555, > + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, Same swapping, can't find a match. > .depth = 15, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_ARGB1555, > + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, Same. > .depth = 15, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { > .has_alpha = true, > }, { > .drm_fmt = DRM_FORMAT_RGB565, > + .v4l2_fmt = V4L2_PIX_FMT_RGB565, -> V4L2_PIX_FMT_RGB565X Was added later, as what you expect is not compatible. > .depth = 16, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_RGB888, > + .v4l2_fmt = V4L2_PIX_FMT_RGB24, -> V4L2_PIX_FMT_BGR24 > .depth = 24, > .num_planes = 1, > .cpp = { 3, 0, 0 }, > @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_BGR888, > + .v4l2_fmt = V4L2_PIX_FMT_BGR24, -> V4L2_PIX_FMT_RGB24 > .depth = 24, > .num_planes = 1, > .cpp = { 3, 0, 0 }, > @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_XRGB8888, > + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, -> V4L2_PIX_FMT_XBGR32 > .depth = 24, > .num_planes = 1, > .cpp = { 4, 0, 0 }, > @@ -281,6 +291,7 @@ static const struct image_format_info formats[] = { > .has_alpha = true, > }, { > .drm_fmt = DRM_FORMAT_ARGB8888, > + .v4l2_fmt = V4L2_PIX_FMT_ARGB32, -> V4L2_PIX_FMT_ABGR32 > .depth = 32, > .num_planes = 1, > .cpp = { 4, 0, 0 }, > @@ -361,6 +372,7 @@ static const struct image_format_info formats[] = { > .has_alpha = true, > }, { > .drm_fmt = DRM_FORMAT_YUV410, > + .v4l2_fmt = V4L2_PIX_FMT_YUV410, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -369,6 +381,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVU410, > + .v4l2_fmt = V4L2_PIX_FMT_YVU410, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -393,6 +406,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YUV420, > + .v4l2_fmt = V4L2_PIX_FMT_YUV420M, There is two valid matches in V4L2 for this format, not sure how this will be handled. The M variant is to be used with MPLANE v4l2_buffer when num_planes is bigger then 1. > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -401,6 +415,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVU420, > + .v4l2_fmt = V4L2_PIX_FMT_YVU420M, Same. > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -409,6 +424,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YUV422, > + .v4l2_fmt = V4L2_PIX_FMT_YUV422M, Same. > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -417,6 +433,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVU422, > + .v4l2_fmt = V4L2_PIX_FMT_YVU422M, Same. > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -425,6 +442,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YUV444, > + .v4l2_fmt = V4L2_PIX_FMT_YUV444M, Same. > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -433,6 +451,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVU444, > + .v4l2_fmt = V4L2_PIX_FMT_YVU444M, Same. > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -441,6 +460,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV12, > + .v4l2_fmt = V4L2_PIX_FMT_NV12, V4L2_PIX_FMT_NV12M is the mplane variant. Depending on how you handle, which should be concistant picking one up. > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -449,6 +469,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV21, > + .v4l2_fmt = V4L2_PIX_FMT_NV21, Same, NV12M for mplane. > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -457,6 +478,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV16, > + .v4l2_fmt = V4L2_PIX_FMT_NV16, Same, NV16M. > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -465,6 +487,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV61, > + .v4l2_fmt = V4L2_PIX_FMT_NV61, Same, NV61M. > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV24, > + .v4l2_fmt = V4L2_PIX_FMT_NV24, For extra fun, the M variant has not been added yet. > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -481,6 +505,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV42, > + .v4l2_fmt = V4L2_PIX_FMT_NV42, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -489,6 +514,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YUYV, > + .v4l2_fmt = V4L2_PIX_FMT_YUYV, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -497,6 +523,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVYU, > + .v4l2_fmt = V4L2_PIX_FMT_YVYU, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -505,6 +532,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_UYVY, > + .v4l2_fmt = V4L2_PIX_FMT_UYVY, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -513,6 +541,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_VYUY, > + .v4l2_fmt = V4L2_PIX_FMT_VYUY, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -632,6 +661,44 @@ const struct image_format_info *image_format_drm_lookup(u32 drm) > EXPORT_SYMBOL(image_format_drm_lookup); > > /** > + * __image_format_v4l2_lookup - query information for a given format > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > + * > + * The caller should only pass a supported pixel format to this function. > + * > + * Returns: > + * The instance of struct image_format_info that describes the pixel format, or > + * NULL if the format is unsupported. > + */ > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2) > +{ > + return __image_format_lookup(v4l2_fmt, v4l2); > +} > +EXPORT_SYMBOL(__image_format_v4l2_lookup); > + > +/** > + * image_format_v4l2_lookup - query information for a given format > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > + * > + * The caller should only pass a supported pixel format to this function. > + * Unsupported pixel formats will generate a warning in the kernel log. > + * > + * Returns: > + * The instance of struct image_format_info that describes the pixel format, or > + * NULL if the format is unsupported. > + */ > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2) > +{ > + const struct image_format_info *format; > + > + format = __image_format_v4l2_lookup(v4l2); > + > + WARN_ON(!format); > + return format; > +} > +EXPORT_SYMBOL(image_format_v4l2_lookup); > + > +/** > * image_format_plane_cpp - determine the bytes per pixel value > * @format: pointer to the image_format > * @plane: plane index
Hi Nicolas, On Fri, Mar 22, 2019 at 03:55:19PM -0400, Nicolas Dufresne wrote: > Le mardi 19 mars 2019 à 22:57 +0100, Maxime Ripard a écrit : > > V4L2 uses different fourcc's than DRM, and has a different set of formats. > > For now, let's add the v4l2 fourcc's for the already existing formats. > > Hopefully I get the fixup right this time, see inline. Thanks a lot for that extensive review. About the single vs multi-planar issue, I'd be inclined with just supporting single-planar formats for now, and extend it later to deal with multiplanar formats. Would that work for everyone? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Hi Maxime, Some comments below... On 3/19/19 10:57 PM, Maxime Ripard wrote: > V4L2 uses different fourcc's than DRM, and has a different set of formats. > For now, let's add the v4l2 fourcc's for the already existing formats. > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> > --- > include/linux/image-formats.h | 9 +++++- > lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- > 2 files changed, 76 insertions(+) > > diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h > index 53fd73a71b3d..fbc3a4501ebd 100644 > --- a/include/linux/image-formats.h > +++ b/include/linux/image-formats.h > @@ -26,6 +26,13 @@ struct image_format_info { > }; > > /** > + * @v4l2_fmt: > + * > + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) > + */ > + u32 v4l2_fmt; > + > + /** > * @depth: > * > * Color depth (number of bits per pixel excluding padding bits), > @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) > > const struct image_format_info *__image_format_drm_lookup(u32 drm); > const struct image_format_info *image_format_drm_lookup(u32 drm); > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); > unsigned int image_format_plane_cpp(const struct image_format_info *format, > int plane); > unsigned int image_format_plane_width(int width, > diff --git a/lib/image-formats.c b/lib/image-formats.c > index 9b9a73220c5d..39f1d38ae861 100644 > --- a/lib/image-formats.c > +++ b/lib/image-formats.c > @@ -8,6 +8,7 @@ > static const struct image_format_info formats[] = { > { > .drm_fmt = DRM_FORMAT_C8, > + .v4l2_fmt = V4L2_PIX_FMT_GREY, > .depth = 8, > .num_planes = 1, > .cpp = { 1, 0, 0 }, > @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_RGB332, > + .v4l2_fmt = V4L2_PIX_FMT_RGB332, > .depth = 8, > .num_planes = 1, > .cpp = { 1, 0, 0 }, > @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_XRGB4444, > + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_ARGB4444, > + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { > .has_alpha = true, > }, { > .drm_fmt = DRM_FORMAT_XRGB1555, > + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, > .depth = 15, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_ARGB1555, > + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, > .depth = 15, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { > .has_alpha = true, > }, { > .drm_fmt = DRM_FORMAT_RGB565, > + .v4l2_fmt = V4L2_PIX_FMT_RGB565, > .depth = 16, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_RGB888, > + .v4l2_fmt = V4L2_PIX_FMT_RGB24, The V4L2 pixelformats describe the order of the components in memory, and as such are independent of the CPU endianness. How is that for the DRM formats? Will the order of DRM_FORMAT_RGB888 in memory differ between little and big endian systems? Mind you, V4L2 drivers are frankly never tested on big endian systems and I suspect many if not all will fail miserably on details like this. > .depth = 24, > .num_planes = 1, > .cpp = { 3, 0, 0 }, > @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_BGR888, > + .v4l2_fmt = V4L2_PIX_FMT_BGR24, > .depth = 24, > .num_planes = 1, > .cpp = { 3, 0, 0 }, > @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { > .vsub = 1, > }, { > .drm_fmt = DRM_FORMAT_XRGB8888, > + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, > .depth = 24, > .num_planes = 1, > .cpp = { 4, 0, 0 }, > @@ -281,6 +291,7 @@ static const struct image_format_info formats[] = { > .has_alpha = true, > }, { > .drm_fmt = DRM_FORMAT_ARGB8888, > + .v4l2_fmt = V4L2_PIX_FMT_ARGB32, > .depth = 32, > .num_planes = 1, > .cpp = { 4, 0, 0 }, > @@ -361,6 +372,7 @@ static const struct image_format_info formats[] = { > .has_alpha = true, > }, { > .drm_fmt = DRM_FORMAT_YUV410, > + .v4l2_fmt = V4L2_PIX_FMT_YUV410, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -369,6 +381,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVU410, > + .v4l2_fmt = V4L2_PIX_FMT_YVU410, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -393,6 +406,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YUV420, > + .v4l2_fmt = V4L2_PIX_FMT_YUV420M, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -401,6 +415,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVU420, > + .v4l2_fmt = V4L2_PIX_FMT_YVU420M, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -409,6 +424,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YUV422, > + .v4l2_fmt = V4L2_PIX_FMT_YUV422M, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -417,6 +433,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVU422, > + .v4l2_fmt = V4L2_PIX_FMT_YVU422M, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -425,6 +442,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YUV444, > + .v4l2_fmt = V4L2_PIX_FMT_YUV444M, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -433,6 +451,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVU444, > + .v4l2_fmt = V4L2_PIX_FMT_YVU444M, > .depth = 0, > .num_planes = 3, > .cpp = { 1, 1, 1 }, > @@ -441,6 +460,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV12, > + .v4l2_fmt = V4L2_PIX_FMT_NV12, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -449,6 +469,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV21, > + .v4l2_fmt = V4L2_PIX_FMT_NV21, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -457,6 +478,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV16, > + .v4l2_fmt = V4L2_PIX_FMT_NV16, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -465,6 +487,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV61, > + .v4l2_fmt = V4L2_PIX_FMT_NV61, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV24, > + .v4l2_fmt = V4L2_PIX_FMT_NV24, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -481,6 +505,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_NV42, > + .v4l2_fmt = V4L2_PIX_FMT_NV42, > .depth = 0, > .num_planes = 2, > .cpp = { 1, 2, 0 }, > @@ -489,6 +514,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YUYV, > + .v4l2_fmt = V4L2_PIX_FMT_YUYV, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -497,6 +523,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_YVYU, > + .v4l2_fmt = V4L2_PIX_FMT_YVYU, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -505,6 +532,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_UYVY, > + .v4l2_fmt = V4L2_PIX_FMT_UYVY, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -513,6 +541,7 @@ static const struct image_format_info formats[] = { > .is_yuv = true, > }, { > .drm_fmt = DRM_FORMAT_VYUY, > + .v4l2_fmt = V4L2_PIX_FMT_VYUY, > .depth = 0, > .num_planes = 1, > .cpp = { 2, 0, 0 }, > @@ -632,6 +661,44 @@ const struct image_format_info *image_format_drm_lookup(u32 drm) > EXPORT_SYMBOL(image_format_drm_lookup); > > /** > + * __image_format_v4l2_lookup - query information for a given format > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > + * > + * The caller should only pass a supported pixel format to this function. > + * > + * Returns: > + * The instance of struct image_format_info that describes the pixel format, or > + * NULL if the format is unsupported. > + */ > +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2) > +{ > + return __image_format_lookup(v4l2_fmt, v4l2); > +} > +EXPORT_SYMBOL(__image_format_v4l2_lookup); > + > +/** > + * image_format_v4l2_lookup - query information for a given format > + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) > + * > + * The caller should only pass a supported pixel format to this function. > + * Unsupported pixel formats will generate a warning in the kernel log. > + * > + * Returns: > + * The instance of struct image_format_info that describes the pixel format, or > + * NULL if the format is unsupported. > + */ > +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2) > +{ > + const struct image_format_info *format; > + > + format = __image_format_v4l2_lookup(v4l2); > + > + WARN_ON(!format); > + return format; > +} > +EXPORT_SYMBOL(image_format_v4l2_lookup); > + > +/** > * image_format_plane_cpp - determine the bytes per pixel value > * @format: pointer to the image_format > * @plane: plane index > Anyway, I think this is a great improvement! Regards, Hans
On 4/11/19 9:12 AM, Hans Verkuil wrote: > Hi Maxime, > > Some comments below... > > On 3/19/19 10:57 PM, Maxime Ripard wrote: >> V4L2 uses different fourcc's than DRM, and has a different set of formats. >> For now, let's add the v4l2 fourcc's for the already existing formats. >> >> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> >> --- >> include/linux/image-formats.h | 9 +++++- >> lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- >> 2 files changed, 76 insertions(+) >> >> diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h >> index 53fd73a71b3d..fbc3a4501ebd 100644 >> --- a/include/linux/image-formats.h >> +++ b/include/linux/image-formats.h >> @@ -26,6 +26,13 @@ struct image_format_info { >> }; >> >> /** >> + * @v4l2_fmt: >> + * >> + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) >> + */ >> + u32 v4l2_fmt; >> + >> + /** >> * @depth: >> * >> * Color depth (number of bits per pixel excluding padding bits), >> @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) >> >> const struct image_format_info *__image_format_drm_lookup(u32 drm); >> const struct image_format_info *image_format_drm_lookup(u32 drm); >> +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); >> +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); >> unsigned int image_format_plane_cpp(const struct image_format_info *format, >> int plane); >> unsigned int image_format_plane_width(int width, >> diff --git a/lib/image-formats.c b/lib/image-formats.c >> index 9b9a73220c5d..39f1d38ae861 100644 >> --- a/lib/image-formats.c >> +++ b/lib/image-formats.c >> @@ -8,6 +8,7 @@ >> static const struct image_format_info formats[] = { >> { >> .drm_fmt = DRM_FORMAT_C8, >> + .v4l2_fmt = V4L2_PIX_FMT_GREY, >> .depth = 8, >> .num_planes = 1, >> .cpp = { 1, 0, 0 }, >> @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_RGB332, >> + .v4l2_fmt = V4L2_PIX_FMT_RGB332, >> .depth = 8, >> .num_planes = 1, >> .cpp = { 1, 0, 0 }, >> @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_XRGB4444, >> + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, >> .depth = 0, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_ARGB4444, >> + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, >> .depth = 0, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { >> .has_alpha = true, >> }, { >> .drm_fmt = DRM_FORMAT_XRGB1555, >> + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, >> .depth = 15, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_ARGB1555, >> + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, >> .depth = 15, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { >> .has_alpha = true, >> }, { >> .drm_fmt = DRM_FORMAT_RGB565, >> + .v4l2_fmt = V4L2_PIX_FMT_RGB565, >> .depth = 16, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_RGB888, >> + .v4l2_fmt = V4L2_PIX_FMT_RGB24, > > The V4L2 pixelformats describe the order of the components in memory, and as > such are independent of the CPU endianness. How is that for the DRM formats? > > Will the order of DRM_FORMAT_RGB888 in memory differ between little and big > endian systems? > > Mind you, V4L2 drivers are frankly never tested on big endian systems and I > suspect many if not all will fail miserably on details like this. Never mind, I now see that there was a long discussion about this same topic. Hans > >> .depth = 24, >> .num_planes = 1, >> .cpp = { 3, 0, 0 }, >> @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_BGR888, >> + .v4l2_fmt = V4L2_PIX_FMT_BGR24, >> .depth = 24, >> .num_planes = 1, >> .cpp = { 3, 0, 0 }, >> @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_XRGB8888, >> + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, >> .depth = 24, >> .num_planes = 1, >> .cpp = { 4, 0, 0 }, >> @@ -281,6 +291,7 @@ static const struct image_format_info formats[] = { >> .has_alpha = true, >> }, { >> .drm_fmt = DRM_FORMAT_ARGB8888, >> + .v4l2_fmt = V4L2_PIX_FMT_ARGB32, >> .depth = 32, >> .num_planes = 1, >> .cpp = { 4, 0, 0 }, >> @@ -361,6 +372,7 @@ static const struct image_format_info formats[] = { >> .has_alpha = true, >> }, { >> .drm_fmt = DRM_FORMAT_YUV410, >> + .v4l2_fmt = V4L2_PIX_FMT_YUV410, >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -369,6 +381,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YVU410, >> + .v4l2_fmt = V4L2_PIX_FMT_YVU410, >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -393,6 +406,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YUV420, >> + .v4l2_fmt = V4L2_PIX_FMT_YUV420M, >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -401,6 +415,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YVU420, >> + .v4l2_fmt = V4L2_PIX_FMT_YVU420M, >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -409,6 +424,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YUV422, >> + .v4l2_fmt = V4L2_PIX_FMT_YUV422M, >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -417,6 +433,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YVU422, >> + .v4l2_fmt = V4L2_PIX_FMT_YVU422M, >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -425,6 +442,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YUV444, >> + .v4l2_fmt = V4L2_PIX_FMT_YUV444M, >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -433,6 +451,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YVU444, >> + .v4l2_fmt = V4L2_PIX_FMT_YVU444M, >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -441,6 +460,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_NV12, >> + .v4l2_fmt = V4L2_PIX_FMT_NV12, >> .depth = 0, >> .num_planes = 2, >> .cpp = { 1, 2, 0 }, >> @@ -449,6 +469,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_NV21, >> + .v4l2_fmt = V4L2_PIX_FMT_NV21, >> .depth = 0, >> .num_planes = 2, >> .cpp = { 1, 2, 0 }, >> @@ -457,6 +478,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_NV16, >> + .v4l2_fmt = V4L2_PIX_FMT_NV16, >> .depth = 0, >> .num_planes = 2, >> .cpp = { 1, 2, 0 }, >> @@ -465,6 +487,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_NV61, >> + .v4l2_fmt = V4L2_PIX_FMT_NV61, >> .depth = 0, >> .num_planes = 2, >> .cpp = { 1, 2, 0 }, >> @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_NV24, >> + .v4l2_fmt = V4L2_PIX_FMT_NV24, >> .depth = 0, >> .num_planes = 2, >> .cpp = { 1, 2, 0 }, >> @@ -481,6 +505,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_NV42, >> + .v4l2_fmt = V4L2_PIX_FMT_NV42, >> .depth = 0, >> .num_planes = 2, >> .cpp = { 1, 2, 0 }, >> @@ -489,6 +514,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YUYV, >> + .v4l2_fmt = V4L2_PIX_FMT_YUYV, >> .depth = 0, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -497,6 +523,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YVYU, >> + .v4l2_fmt = V4L2_PIX_FMT_YVYU, >> .depth = 0, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -505,6 +532,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_UYVY, >> + .v4l2_fmt = V4L2_PIX_FMT_UYVY, >> .depth = 0, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -513,6 +541,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_VYUY, >> + .v4l2_fmt = V4L2_PIX_FMT_VYUY, >> .depth = 0, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -632,6 +661,44 @@ const struct image_format_info *image_format_drm_lookup(u32 drm) >> EXPORT_SYMBOL(image_format_drm_lookup); >> >> /** >> + * __image_format_v4l2_lookup - query information for a given format >> + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) >> + * >> + * The caller should only pass a supported pixel format to this function. >> + * >> + * Returns: >> + * The instance of struct image_format_info that describes the pixel format, or >> + * NULL if the format is unsupported. >> + */ >> +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2) >> +{ >> + return __image_format_lookup(v4l2_fmt, v4l2); >> +} >> +EXPORT_SYMBOL(__image_format_v4l2_lookup); >> + >> +/** >> + * image_format_v4l2_lookup - query information for a given format >> + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) >> + * >> + * The caller should only pass a supported pixel format to this function. >> + * Unsupported pixel formats will generate a warning in the kernel log. >> + * >> + * Returns: >> + * The instance of struct image_format_info that describes the pixel format, or >> + * NULL if the format is unsupported. >> + */ >> +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2) >> +{ >> + const struct image_format_info *format; >> + >> + format = __image_format_v4l2_lookup(v4l2); >> + >> + WARN_ON(!format); >> + return format; >> +} >> +EXPORT_SYMBOL(image_format_v4l2_lookup); >> + >> +/** >> * image_format_plane_cpp - determine the bytes per pixel value >> * @format: pointer to the image_format >> * @plane: plane index >> > > Anyway, I think this is a great improvement! > > Regards, > > Hans >
On 4/1/19 4:44 PM, Maxime Ripard wrote: > Hi Nicolas, > > On Fri, Mar 22, 2019 at 03:55:19PM -0400, Nicolas Dufresne wrote: >> Le mardi 19 mars 2019 à 22:57 +0100, Maxime Ripard a écrit : >>> V4L2 uses different fourcc's than DRM, and has a different set of formats. >>> For now, let's add the v4l2 fourcc's for the already existing formats. >> >> Hopefully I get the fixup right this time, see inline. > > Thanks a lot for that extensive review. > > About the single vs multi-planar issue, I'd be inclined with just > supporting single-planar formats for now, and extend it later to deal > with multiplanar formats. > > Would that work for everyone? I'd say that is the right approach. Regards, Hans > > Maxime > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com >
On 3/22/19 8:55 PM, Nicolas Dufresne wrote: > Le mardi 19 mars 2019 à 22:57 +0100, Maxime Ripard a écrit : >> V4L2 uses different fourcc's than DRM, and has a different set of formats. >> For now, let's add the v4l2 fourcc's for the already existing formats. > > Hopefully I get the fixup right this time, see inline. > >> >> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> >> --- >> include/linux/image-formats.h | 9 +++++- >> lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- >> 2 files changed, 76 insertions(+) >> >> diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h >> index 53fd73a71b3d..fbc3a4501ebd 100644 >> --- a/include/linux/image-formats.h >> +++ b/include/linux/image-formats.h >> @@ -26,6 +26,13 @@ struct image_format_info { >> }; >> >> /** >> + * @v4l2_fmt: >> + * >> + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) >> + */ >> + u32 v4l2_fmt; >> + >> + /** >> * @depth: >> * >> * Color depth (number of bits per pixel excluding padding bits), >> @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) >> >> const struct image_format_info *__image_format_drm_lookup(u32 drm); >> const struct image_format_info *image_format_drm_lookup(u32 drm); >> +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); >> +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); >> unsigned int image_format_plane_cpp(const struct image_format_info *format, >> int plane); >> unsigned int image_format_plane_width(int width, >> diff --git a/lib/image-formats.c b/lib/image-formats.c >> index 9b9a73220c5d..39f1d38ae861 100644 >> --- a/lib/image-formats.c >> +++ b/lib/image-formats.c >> @@ -8,6 +8,7 @@ >> static const struct image_format_info formats[] = { >> { >> .drm_fmt = DRM_FORMAT_C8, >> + .v4l2_fmt = V4L2_PIX_FMT_GREY, >> .depth = 8, >> .num_planes = 1, >> .cpp = { 1, 0, 0 }, >> @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_RGB332, >> + .v4l2_fmt = V4L2_PIX_FMT_RGB332, >> .depth = 8, >> .num_planes = 1, >> .cpp = { 1, 0, 0 }, >> @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_XRGB4444, >> + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, > > Not completely sure, looks like in the V4L2 variant, the padding is on > the 4 MSB of the second byte, which makes it incompatible. There is no > other equivalent. > >> .depth = 0, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_ARGB4444, >> + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, > > Similarly. > >> .depth = 0, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { >> .has_alpha = true, >> }, { >> .drm_fmt = DRM_FORMAT_XRGB1555, >> + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, > > Same swapping, can't find a match. > >> .depth = 15, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_ARGB1555, >> + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, > > Same. > >> .depth = 15, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { >> .has_alpha = true, >> }, { >> .drm_fmt = DRM_FORMAT_RGB565, >> + .v4l2_fmt = V4L2_PIX_FMT_RGB565, > > -> V4L2_PIX_FMT_RGB565X > > Was added later, as what you expect is not compatible. All these 16-bit V4L2 pixelformats are ancient, but they are (to my knowledge) correct w.r.t. the documented layout of the bits in memory. I.e., that's what the hardware will give you. I think if there is no equivalence between the drm and v4l2 fourcc's, then you need two entries in this table, one for drm (v4l2_fmt == 0), one for v4l2 (drm_fmt == 0). > >> .depth = 16, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_RGB888, >> + .v4l2_fmt = V4L2_PIX_FMT_RGB24, > > -> V4L2_PIX_FMT_BGR24 > >> .depth = 24, >> .num_planes = 1, >> .cpp = { 3, 0, 0 }, >> @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_BGR888, >> + .v4l2_fmt = V4L2_PIX_FMT_BGR24, > > -> V4L2_PIX_FMT_RGB24 > >> .depth = 24, >> .num_planes = 1, >> .cpp = { 3, 0, 0 }, >> @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { >> .vsub = 1, >> }, { >> .drm_fmt = DRM_FORMAT_XRGB8888, >> + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, > > -> V4L2_PIX_FMT_XBGR32 > >> .depth = 24, >> .num_planes = 1, >> .cpp = { 4, 0, 0 }, >> @@ -281,6 +291,7 @@ static const struct image_format_info formats[] = { >> .has_alpha = true, >> }, { >> .drm_fmt = DRM_FORMAT_ARGB8888, >> + .v4l2_fmt = V4L2_PIX_FMT_ARGB32, > > -> V4L2_PIX_FMT_ABGR32 > >> .depth = 32, >> .num_planes = 1, >> .cpp = { 4, 0, 0 }, >> @@ -361,6 +372,7 @@ static const struct image_format_info formats[] = { >> .has_alpha = true, >> }, { >> .drm_fmt = DRM_FORMAT_YUV410, >> + .v4l2_fmt = V4L2_PIX_FMT_YUV410, >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -369,6 +381,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YVU410, >> + .v4l2_fmt = V4L2_PIX_FMT_YVU410, >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -393,6 +406,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YUV420, >> + .v4l2_fmt = V4L2_PIX_FMT_YUV420M, > > There is two valid matches in V4L2 for this format, not sure how this > will be handled. The M variant is to be used with MPLANE v4l2_buffer > when num_planes is bigger then 1. We should use the non-multiplanar variant (without the M). At least for now. > >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -401,6 +415,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YVU420, >> + .v4l2_fmt = V4L2_PIX_FMT_YVU420M, > > Same. > >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -409,6 +424,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YUV422, >> + .v4l2_fmt = V4L2_PIX_FMT_YUV422M, > > Same. > >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -417,6 +433,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YVU422, >> + .v4l2_fmt = V4L2_PIX_FMT_YVU422M, > > Same. > >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -425,6 +442,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YUV444, >> + .v4l2_fmt = V4L2_PIX_FMT_YUV444M, > > Same. > >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -433,6 +451,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YVU444, >> + .v4l2_fmt = V4L2_PIX_FMT_YVU444M, > > Same. > >> .depth = 0, >> .num_planes = 3, >> .cpp = { 1, 1, 1 }, >> @@ -441,6 +460,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_NV12, >> + .v4l2_fmt = V4L2_PIX_FMT_NV12, > > V4L2_PIX_FMT_NV12M is the mplane variant. Depending on how you handle, > which should be concistant picking one up. > >> .depth = 0, >> .num_planes = 2, >> .cpp = { 1, 2, 0 }, >> @@ -449,6 +469,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_NV21, >> + .v4l2_fmt = V4L2_PIX_FMT_NV21, > > Same, NV12M for mplane. > >> .depth = 0, >> .num_planes = 2, >> .cpp = { 1, 2, 0 }, >> @@ -457,6 +478,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_NV16, >> + .v4l2_fmt = V4L2_PIX_FMT_NV16, > > Same, NV16M. > >> .depth = 0, >> .num_planes = 2, >> .cpp = { 1, 2, 0 }, >> @@ -465,6 +487,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_NV61, >> + .v4l2_fmt = V4L2_PIX_FMT_NV61, > > Same, NV61M. > >> .depth = 0, >> .num_planes = 2, >> .cpp = { 1, 2, 0 }, >> @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_NV24, >> + .v4l2_fmt = V4L2_PIX_FMT_NV24, > > For extra fun, the M variant has not been added yet. Note that it has been the practice in V4L2 to avoid adding pixelformats unless they are in use by a V4L2 driver. So that's why some combinations do not exist. If we are creating a common library then I think we should change that rule to: "unless they are in use by a DRM or V4L2 driver". And when new formats are added, and they exists already for DRM or V4L2, then we should use the same fourcc for the other subsystem. I.e. if pixelformat V4L2_PIX_FMT_FOO was already defined, then add a: #define DRM_FORMAT_FOO V4L2_PIX_FMT_FOO rather than creating a new fourcc. We could even start looking at redoing the whole scheme in a unified way, but that's something for the (far) future. This is already a big step forward. Regards, Hans > >> .depth = 0, >> .num_planes = 2, >> .cpp = { 1, 2, 0 }, >> @@ -481,6 +505,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_NV42, >> + .v4l2_fmt = V4L2_PIX_FMT_NV42, >> .depth = 0, >> .num_planes = 2, >> .cpp = { 1, 2, 0 }, >> @@ -489,6 +514,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YUYV, >> + .v4l2_fmt = V4L2_PIX_FMT_YUYV, >> .depth = 0, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -497,6 +523,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_YVYU, >> + .v4l2_fmt = V4L2_PIX_FMT_YVYU, >> .depth = 0, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -505,6 +532,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_UYVY, >> + .v4l2_fmt = V4L2_PIX_FMT_UYVY, >> .depth = 0, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -513,6 +541,7 @@ static const struct image_format_info formats[] = { >> .is_yuv = true, >> }, { >> .drm_fmt = DRM_FORMAT_VYUY, >> + .v4l2_fmt = V4L2_PIX_FMT_VYUY, >> .depth = 0, >> .num_planes = 1, >> .cpp = { 2, 0, 0 }, >> @@ -632,6 +661,44 @@ const struct image_format_info *image_format_drm_lookup(u32 drm) >> EXPORT_SYMBOL(image_format_drm_lookup); >> >> /** >> + * __image_format_v4l2_lookup - query information for a given format >> + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) >> + * >> + * The caller should only pass a supported pixel format to this function. >> + * >> + * Returns: >> + * The instance of struct image_format_info that describes the pixel format, or >> + * NULL if the format is unsupported. >> + */ >> +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2) >> +{ >> + return __image_format_lookup(v4l2_fmt, v4l2); >> +} >> +EXPORT_SYMBOL(__image_format_v4l2_lookup); >> + >> +/** >> + * image_format_v4l2_lookup - query information for a given format >> + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) >> + * >> + * The caller should only pass a supported pixel format to this function. >> + * Unsupported pixel formats will generate a warning in the kernel log. >> + * >> + * Returns: >> + * The instance of struct image_format_info that describes the pixel format, or >> + * NULL if the format is unsupported. >> + */ >> +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2) >> +{ >> + const struct image_format_info *format; >> + >> + format = __image_format_v4l2_lookup(v4l2); >> + >> + WARN_ON(!format); >> + return format; >> +} >> +EXPORT_SYMBOL(image_format_v4l2_lookup); >> + >> +/** >> * image_format_plane_cpp - determine the bytes per pixel value >> * @format: pointer to the image_format >> * @plane: plane index
Hi, On Thu, Apr 11, 2019 at 09:38:06AM +0200, Hans Verkuil wrote: > >> @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { > >> .has_alpha = true, > >> }, { > >> .drm_fmt = DRM_FORMAT_RGB565, > >> + .v4l2_fmt = V4L2_PIX_FMT_RGB565, > > > > -> V4L2_PIX_FMT_RGB565X > > > > Was added later, as what you expect is not compatible. > > All these 16-bit V4L2 pixelformats are ancient, but they are (to my knowledge) > correct w.r.t. the documented layout of the bits in memory. I.e., that's what > the hardware will give you. > > I think if there is no equivalence between the drm and v4l2 fourcc's, then > you need two entries in this table, one for drm (v4l2_fmt == 0), one for v4l2 > (drm_fmt == 0). Yeah, that was my plan. I'd definitely expect some formats in V4L2 while not supported by DRM, or the other way around. > >> @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { > >> .is_yuv = true, > >> }, { > >> .drm_fmt = DRM_FORMAT_NV24, > >> + .v4l2_fmt = V4L2_PIX_FMT_NV24, > > > > For extra fun, the M variant has not been added yet. > > Note that it has been the practice in V4L2 to avoid adding pixelformats unless > they are in use by a V4L2 driver. So that's why some combinations do not exist. > > If we are creating a common library then I think we should change that rule > to: "unless they are in use by a DRM or V4L2 driver". And when new formats are > added, and they exists already for DRM or V4L2, then we should use the same > fourcc for the other subsystem. > > I.e. if pixelformat V4L2_PIX_FMT_FOO was already defined, then add a: > > #define DRM_FORMAT_FOO V4L2_PIX_FMT_FOO > > rather than creating a new fourcc. That makes sense > We could even start looking at redoing the whole scheme in a unified way, but > that's something for the (far) future. Yeah, my secret plan is to have eventually a single fourcc (or even #define) for both DRM and v4l2 for any new format. But don't tell anyone :) > This is already a big step forward. Great, I'll respin this then. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h index 53fd73a71b3d..fbc3a4501ebd 100644 --- a/include/linux/image-formats.h +++ b/include/linux/image-formats.h @@ -26,6 +26,13 @@ struct image_format_info { }; /** + * @v4l2_fmt: + * + * V4L2 4CC format identifier (V4L2_PIX_FMT_*) + */ + u32 v4l2_fmt; + + /** * @depth: * * Color depth (number of bits per pixel excluding padding bits), @@ -222,6 +229,8 @@ image_format_info_is_yuv_sampling_444(const struct image_format_info *info) const struct image_format_info *__image_format_drm_lookup(u32 drm); const struct image_format_info *image_format_drm_lookup(u32 drm); +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2); +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2); unsigned int image_format_plane_cpp(const struct image_format_info *format, int plane); unsigned int image_format_plane_width(int width, diff --git a/lib/image-formats.c b/lib/image-formats.c index 9b9a73220c5d..39f1d38ae861 100644 --- a/lib/image-formats.c +++ b/lib/image-formats.c @@ -8,6 +8,7 @@ static const struct image_format_info formats[] = { { .drm_fmt = DRM_FORMAT_C8, + .v4l2_fmt = V4L2_PIX_FMT_GREY, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, @@ -15,6 +16,7 @@ static const struct image_format_info formats[] = { .vsub = 1, }, { .drm_fmt = DRM_FORMAT_RGB332, + .v4l2_fmt = V4L2_PIX_FMT_RGB332, .depth = 8, .num_planes = 1, .cpp = { 1, 0, 0 }, @@ -29,6 +31,7 @@ static const struct image_format_info formats[] = { .vsub = 1, }, { .drm_fmt = DRM_FORMAT_XRGB4444, + .v4l2_fmt = V4L2_PIX_FMT_XRGB444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, @@ -57,6 +60,7 @@ static const struct image_format_info formats[] = { .vsub = 1, }, { .drm_fmt = DRM_FORMAT_ARGB4444, + .v4l2_fmt = V4L2_PIX_FMT_ARGB444, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, @@ -89,6 +93,7 @@ static const struct image_format_info formats[] = { .has_alpha = true, }, { .drm_fmt = DRM_FORMAT_XRGB1555, + .v4l2_fmt = V4L2_PIX_FMT_XRGB555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, @@ -117,6 +122,7 @@ static const struct image_format_info formats[] = { .vsub = 1, }, { .drm_fmt = DRM_FORMAT_ARGB1555, + .v4l2_fmt = V4L2_PIX_FMT_ARGB555, .depth = 15, .num_planes = 1, .cpp = { 2, 0, 0 }, @@ -149,6 +155,7 @@ static const struct image_format_info formats[] = { .has_alpha = true, }, { .drm_fmt = DRM_FORMAT_RGB565, + .v4l2_fmt = V4L2_PIX_FMT_RGB565, .depth = 16, .num_planes = 1, .cpp = { 2, 0, 0 }, @@ -163,6 +170,7 @@ static const struct image_format_info formats[] = { .vsub = 1, }, { .drm_fmt = DRM_FORMAT_RGB888, + .v4l2_fmt = V4L2_PIX_FMT_RGB24, .depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, @@ -170,6 +178,7 @@ static const struct image_format_info formats[] = { .vsub = 1, }, { .drm_fmt = DRM_FORMAT_BGR888, + .v4l2_fmt = V4L2_PIX_FMT_BGR24, .depth = 24, .num_planes = 1, .cpp = { 3, 0, 0 }, @@ -177,6 +186,7 @@ static const struct image_format_info formats[] = { .vsub = 1, }, { .drm_fmt = DRM_FORMAT_XRGB8888, + .v4l2_fmt = V4L2_PIX_FMT_XRGB32, .depth = 24, .num_planes = 1, .cpp = { 4, 0, 0 }, @@ -281,6 +291,7 @@ static const struct image_format_info formats[] = { .has_alpha = true, }, { .drm_fmt = DRM_FORMAT_ARGB8888, + .v4l2_fmt = V4L2_PIX_FMT_ARGB32, .depth = 32, .num_planes = 1, .cpp = { 4, 0, 0 }, @@ -361,6 +372,7 @@ static const struct image_format_info formats[] = { .has_alpha = true, }, { .drm_fmt = DRM_FORMAT_YUV410, + .v4l2_fmt = V4L2_PIX_FMT_YUV410, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, @@ -369,6 +381,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_YVU410, + .v4l2_fmt = V4L2_PIX_FMT_YVU410, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, @@ -393,6 +406,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_YUV420, + .v4l2_fmt = V4L2_PIX_FMT_YUV420M, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, @@ -401,6 +415,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_YVU420, + .v4l2_fmt = V4L2_PIX_FMT_YVU420M, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, @@ -409,6 +424,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_YUV422, + .v4l2_fmt = V4L2_PIX_FMT_YUV422M, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, @@ -417,6 +433,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_YVU422, + .v4l2_fmt = V4L2_PIX_FMT_YVU422M, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, @@ -425,6 +442,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_YUV444, + .v4l2_fmt = V4L2_PIX_FMT_YUV444M, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, @@ -433,6 +451,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_YVU444, + .v4l2_fmt = V4L2_PIX_FMT_YVU444M, .depth = 0, .num_planes = 3, .cpp = { 1, 1, 1 }, @@ -441,6 +460,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_NV12, + .v4l2_fmt = V4L2_PIX_FMT_NV12, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, @@ -449,6 +469,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_NV21, + .v4l2_fmt = V4L2_PIX_FMT_NV21, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, @@ -457,6 +478,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_NV16, + .v4l2_fmt = V4L2_PIX_FMT_NV16, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, @@ -465,6 +487,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_NV61, + .v4l2_fmt = V4L2_PIX_FMT_NV61, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, @@ -473,6 +496,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_NV24, + .v4l2_fmt = V4L2_PIX_FMT_NV24, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, @@ -481,6 +505,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_NV42, + .v4l2_fmt = V4L2_PIX_FMT_NV42, .depth = 0, .num_planes = 2, .cpp = { 1, 2, 0 }, @@ -489,6 +514,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_YUYV, + .v4l2_fmt = V4L2_PIX_FMT_YUYV, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, @@ -497,6 +523,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_YVYU, + .v4l2_fmt = V4L2_PIX_FMT_YVYU, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, @@ -505,6 +532,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_UYVY, + .v4l2_fmt = V4L2_PIX_FMT_UYVY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, @@ -513,6 +541,7 @@ static const struct image_format_info formats[] = { .is_yuv = true, }, { .drm_fmt = DRM_FORMAT_VYUY, + .v4l2_fmt = V4L2_PIX_FMT_VYUY, .depth = 0, .num_planes = 1, .cpp = { 2, 0, 0 }, @@ -632,6 +661,44 @@ const struct image_format_info *image_format_drm_lookup(u32 drm) EXPORT_SYMBOL(image_format_drm_lookup); /** + * __image_format_v4l2_lookup - query information for a given format + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) + * + * The caller should only pass a supported pixel format to this function. + * + * Returns: + * The instance of struct image_format_info that describes the pixel format, or + * NULL if the format is unsupported. + */ +const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2) +{ + return __image_format_lookup(v4l2_fmt, v4l2); +} +EXPORT_SYMBOL(__image_format_v4l2_lookup); + +/** + * image_format_v4l2_lookup - query information for a given format + * @v4l2: V4L2 fourcc pixel format (V4L2_PIX_FMT_*) + * + * The caller should only pass a supported pixel format to this function. + * Unsupported pixel formats will generate a warning in the kernel log. + * + * Returns: + * The instance of struct image_format_info that describes the pixel format, or + * NULL if the format is unsupported. + */ +const struct image_format_info *image_format_v4l2_lookup(u32 v4l2) +{ + const struct image_format_info *format; + + format = __image_format_v4l2_lookup(v4l2); + + WARN_ON(!format); + return format; +} +EXPORT_SYMBOL(image_format_v4l2_lookup); + +/** * image_format_plane_cpp - determine the bytes per pixel value * @format: pointer to the image_format * @plane: plane index
V4L2 uses different fourcc's than DRM, and has a different set of formats. For now, let's add the v4l2 fourcc's for the already existing formats. Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com> --- include/linux/image-formats.h | 9 +++++- lib/image-formats.c | 67 ++++++++++++++++++++++++++++++++++++- 2 files changed, 76 insertions(+)