[PATCH] drm/format-helper: Add XRGB8888 to UYVY conversion

[PATCH] drm/format-helper: Add XRGB8888 to UYVY conversion

[Ho Jie Feng]

Add a helper for devices that use packed UYVY but accept XRGB8888
framebuffers.

Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
---
This is used for a driver, ms912x that I will be submitting. It only
supports uyvy framebuffers.

 drivers/gpu/drm/drm_format_helper.c   | 73 +++++++++++++++++++++++++++
 drivers/gpu/drm/drm_format_internal.h | 21 ++++++++
 include/drm/drm_format_helper.h       |  4 ++
 3 files changed, 98 insertions(+)

diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
index 6cddf05c493bd..606733ca43fb4 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -1379,3 +1379,76 @@ void drm_fb_xrgb8888_to_gray2(struct iosys_map *dst, const unsigned int *dst_pit
 }
 EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray2);
 
+static void drm_fb_xrgb8888_to_uyvy_line(void *dbuf, const void *sbuf, unsigned int width)
+{
+	u8 *transfer_buffer = dbuf;
+	const __le32 *temp_buffer = sbuf;
+	unsigned int i, dst_offset = 0;
+	unsigned int pixel1, pixel2;
+	unsigned int r1, g1, b1, r2, g2, b2;
+	unsigned int r, g, b;
+	unsigned int v, y1, u, y2;
+
+	for (i = 0; i < width; i += 2) {
+		pixel1 = le32_to_cpup(&temp_buffer[i]);
+		pixel2 = le32_to_cpup(&temp_buffer[i + 1]);
+
+		r1 = (pixel1 & 0x00ff0000) >> 16;
+		g1 = (pixel1 & 0x0000ff00) >> 8;
+		b1 =  pixel1 & 0x000000ff;
+		r2 = (pixel2 & 0x00ff0000) >> 16;
+		g2 = (pixel2 & 0x0000ff00) >> 8;
+		b2 =  pixel2 & 0x000000ff;
+
+		y1 = drm_pixel_rgb_to_y_bt601(r1, g1, b1);
+		y2 = drm_pixel_rgb_to_y_bt601(r2, g2, b2);
+
+		r = r1 + r2;
+		g = g1 + g2;
+		b = b1 + b2;
+
+		v = drm_pixel_rgb2_to_v_bt601(r, g, b);
+		u = drm_pixel_rgb2_to_u_bt601(r, g, b);
+
+		transfer_buffer[dst_offset++] = u;
+		transfer_buffer[dst_offset++] = y1;
+		transfer_buffer[dst_offset++] = v;
+		transfer_buffer[dst_offset++] = y2;
+	}
+}
+
+/**
+ * drm_fb_xrgb8888_to_uyvy - Convert XRGB8888 to UYVY clip buffer
+ * @dst: Array of UYVY destination buffers
+ * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
+ *             within @dst; can be NULL if scanlines are stored next to each other.
+ * @src: Array of XRGB8888 source buffers
+ * @fb: DRM framebuffer
+ * @clip: Clip rectangle area to copy; x1 and x2 must be even
+ * @state: Transform and conversion state
+ *
+ * This function copies parts of a framebuffer to display memory and converts the
+ * color format during the process. The parameters @dst, @dst_pitch and @src refer
+ * to arrays. Each array must have at least as many entries as there are planes in
+ * @fb's format. Each entry stores the value for the format's respective color plane
+ * at the same index.
+ *
+ * This function does not apply clipping on @dst (i.e. the destination is at the
+ * top-left corner).
+ *
+ * Drivers can use this function for UYVY devices that don't support XRGB8888
+ * natively. ITU BT.601 is being used for the RGB -> YUV conversion.
+ */
+void drm_fb_xrgb8888_to_uyvy(struct iosys_map *dst, const unsigned int *dst_pitch,
+			     const struct iosys_map *src, const struct drm_framebuffer *fb,
+			     const struct drm_rect *clip, struct drm_format_conv_state *state)
+{
+	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
+		2,
+	};
+
+	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, state,
+		    drm_fb_xrgb8888_to_uyvy_line);
+}
+EXPORT_SYMBOL(drm_fb_xrgb8888_to_uyvy);
+
diff --git a/drivers/gpu/drm/drm_format_internal.h b/drivers/gpu/drm/drm_format_internal.h
index ce29dd05bcc55..7a4cfb0d188cb 100644
--- a/drivers/gpu/drm/drm_format_internal.h
+++ b/drivers/gpu/drm/drm_format_internal.h
@@ -159,6 +159,27 @@ static inline u32 drm_pixel_xrgb8888_to_abgr2101010(u32 pix)
 	       drm_pixel_xrgb8888_to_xbgr2101010(pix);
 }
 
+static inline u32 drm_pixel_rgb_to_y_bt601(u32 r, u32 g, u32 b)
+{
+	const u32 luma = (16U << 16) + 16829 * r + 33039 * g + 6416 * b + 32768;
+
+	return luma >> 16;
+}
+
+static inline u32 drm_pixel_rgb2_to_u_bt601(u32 r2, u32 g2, u32 b2)
+{
+	const u32 u = (128U << 17) - 9714 * r2 - 19071 * g2 + 28784 * b2 + 65536;
+
+	return u >> 17;
+}
+
+static inline u32 drm_pixel_rgb2_to_v_bt601(u32 r2, u32 g2, u32 b2)
+{
+	const u32 v = (128U << 17) + 28784 * r2 - 24103 * g2 - 4681 * b2 + 65536;
+
+	return v >> 17;
+}
+
 /*
  * Conversion from ARGB8888
  */
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 2b5c1aef80b0e..e50f96e07484c 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -136,4 +136,8 @@ void drm_fb_xrgb8888_to_gray2(struct iosys_map *dst, const unsigned int *dst_pit
 			      const struct iosys_map *src, const struct drm_framebuffer *fb,
 			      const struct drm_rect *clip, struct drm_format_conv_state *state);
 
+void drm_fb_xrgb8888_to_uyvy(struct iosys_map *dst, const unsigned int *dst_pitch,
+			     const struct iosys_map *src, const struct drm_framebuffer *fb,
+			     const struct drm_rect *clip, struct drm_format_conv_state *state);
+
 #endif /* __LINUX_DRM_FORMAT_HELPER_H */
-- 
2.55.0

Re: [PATCH] drm/format-helper: Add XRGB8888 to UYVY conversion

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm: heap buffer overflow in drm_fb_xrgb8888_to_uyvy_line() on odd clip width
--

commit 4e505b4602c1fdbf4570529c2dce5fc08ba8b249
Author: Ho Jie Feng <hjf3108@gmail.com>
Subject: drm/format-helper: Add XRGB8888 to UYVY conversion

This commit adds a new format helper function, drm_fb_xrgb8888_to_uyvy(),
to convert XRGB8888 framebuffers to UYVY format. It is intended for devices
that use packed UYVY natively but need to support XRGB8888 framebuffers.

> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index 6cddf05c493bd..606733ca43fb4 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
[ ... ]
> +static void drm_fb_xrgb8888_to_uyvy_line(void *dbuf, const void *sbuf, unsigned int width)
> +{
> +	u8 *transfer_buffer = dbuf;
> +	const __le32 *temp_buffer = sbuf;
> +	unsigned int i, dst_offset = 0;
> +	unsigned int pixel1, pixel2;
> +	unsigned int r1, g1, b1, r2, g2, b2;
> +	unsigned int r, g, b;
> +	unsigned int v, y1, u, y2;
> +
> +	for (i = 0; i < width; i += 2) {
> +		pixel1 = le32_to_cpup(&temp_buffer[i]);
> +		pixel2 = le32_to_cpup(&temp_buffer[i + 1]);

[Severity: High]
Does this code perform an out-of-bounds read if the damage clip width is odd?

If userspace provides a damage clip where the width is an odd number, on the
final iteration of this loop, it appears we will unconditionally read
temp_buffer[i + 1] past the end of the source buffer.

> +
> +		r1 = (pixel1 & 0x00ff0000) >> 16;
> +		g1 = (pixel1 & 0x0000ff00) >> 8;
> +		b1 =  pixel1 & 0x000000ff;
> +		r2 = (pixel2 & 0x00ff0000) >> 16;
> +		g2 = (pixel2 & 0x0000ff00) >> 8;
> +		b2 =  pixel2 & 0x000000ff;
> +
> +		y1 = drm_pixel_rgb_to_y_bt601(r1, g1, b1);
> +		y2 = drm_pixel_rgb_to_y_bt601(r2, g2, b2);
> +
> +		r = r1 + r2;
> +		g = g1 + g2;
> +		b = b1 + b2;
> +
> +		v = drm_pixel_rgb2_to_v_bt601(r, g, b);
> +		u = drm_pixel_rgb2_to_u_bt601(r, g, b);
> +
> +		transfer_buffer[dst_offset++] = u;
> +		transfer_buffer[dst_offset++] = y1;
> +		transfer_buffer[dst_offset++] = v;
> +		transfer_buffer[dst_offset++] = y2;

[Severity: High]
Can this write overflow the destination buffer boundary?

Since the destination line buffer size is calculated based on dst_pixsize = 2,
it expects 2 bytes per pixel. For an odd width, writing a 4-byte UYVY
macropixel (u, y1, v, y2) on the final iteration writes 4 bytes when only
2 bytes are expected for that final odd pixel. 

This appears to overflow the destination buffer by 2 bytes per line,
potentially corrupting adjacent lines or causing a kernel heap buffer overflow
on the final line.

> +	}
> +}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012170857.199561-1-hjf3108@gmail.com?part=1

Re: [PATCH] drm/format-helper: Add XRGB8888 to UYVY conversion

[Thomas Zimmermann]

Hi

Am 12.10.26 um 19:08 schrieb Ho Jie Feng:
> Add a helper for devices that use packed UYVY but accept XRGB8888
> framebuffers.

Is there a description of UYVY available somewhere?

>
> Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
> ---
> This is used for a driver, ms912x that I will be submitting. It only
> supports uyvy framebuffers.

Please resubmit as part of the driver series. We're not going to merge 
it without callers anyway.

>
>   drivers/gpu/drm/drm_format_helper.c   | 73 +++++++++++++++++++++++++++
>   drivers/gpu/drm/drm_format_internal.h | 21 ++++++++
>   include/drm/drm_format_helper.h       |  4 ++
>   3 files changed, 98 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c
> index 6cddf05c493bd..606733ca43fb4 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -1379,3 +1379,76 @@ void drm_fb_xrgb8888_to_gray2(struct iosys_map *dst, const unsigned int *dst_pit
>   }
>   EXPORT_SYMBOL(drm_fb_xrgb8888_to_gray2);
>   
> +static void drm_fb_xrgb8888_to_uyvy_line(void *dbuf, const void *sbuf, unsigned int width)
> +{
> +	u8 *transfer_buffer = dbuf;
> +	const __le32 *temp_buffer = sbuf;

For naming, please follow the conventions used by the other helpers in 
this file.

> +	unsigned int i, dst_offset = 0;
> +	unsigned int pixel1, pixel2;
> +	unsigned int r1, g1, b1, r2, g2, b2;
> +	unsigned int r, g, b;
> +	unsigned int v, y1, u, y2;
> +
> +	for (i = 0; i < width; i += 2) {
> +		pixel1 = le32_to_cpup(&temp_buffer[i]);
> +		pixel2 = le32_to_cpup(&temp_buffer[i + 1]);
> +
> +		r1 = (pixel1 & 0x00ff0000) >> 16;
> +		g1 = (pixel1 & 0x0000ff00) >> 8;
> +		b1 =  pixel1 & 0x000000ff;
> +		r2 = (pixel2 & 0x00ff0000) >> 16;
> +		g2 = (pixel2 & 0x0000ff00) >> 8;
> +		b2 =  pixel2 & 0x000000ff;
> +
> +		y1 = drm_pixel_rgb_to_y_bt601(r1, g1, b1);
> +		y2 = drm_pixel_rgb_to_y_bt601(r2, g2, b2);
> +
> +		r = r1 + r2;
> +		g = g1 + g2;
> +		b = b1 + b2;
> +
> +		v = drm_pixel_rgb2_to_v_bt601(r, g, b);
> +		u = drm_pixel_rgb2_to_u_bt601(r, g, b);
> +
> +		transfer_buffer[dst_offset++] = u;
> +		transfer_buffer[dst_offset++] = y1;
> +		transfer_buffer[dst_offset++] = v;
> +		transfer_buffer[dst_offset++] = y2;

Rather that writing bytes individually, use put_unaligned_le32() or a 
similar helper.  Byte order within a pixel is little endian, although 
I'm not sure what this means for UYVY.

> +	}
> +}
> +
> +/**
> + * drm_fb_xrgb8888_to_uyvy - Convert XRGB8888 to UYVY clip buffer
> + * @dst: Array of UYVY destination buffers
> + * @dst_pitch: Array of numbers of bytes between the start of two consecutive scanlines
> + *             within @dst; can be NULL if scanlines are stored next to each other.
> + * @src: Array of XRGB8888 source buffers
> + * @fb: DRM framebuffer
> + * @clip: Clip rectangle area to copy; x1 and x2 must be even
> + * @state: Transform and conversion state
> + *
> + * This function copies parts of a framebuffer to display memory and converts the
> + * color format during the process. The parameters @dst, @dst_pitch and @src refer
> + * to arrays. Each array must have at least as many entries as there are planes in
> + * @fb's format. Each entry stores the value for the format's respective color plane
> + * at the same index.
> + *
> + * This function does not apply clipping on @dst (i.e. the destination is at the
> + * top-left corner).
> + *
> + * Drivers can use this function for UYVY devices that don't support XRGB8888
> + * natively. ITU BT.601 is being used for the RGB -> YUV conversion.
> + */
> +void drm_fb_xrgb8888_to_uyvy(struct iosys_map *dst, const unsigned int *dst_pitch,
> +			     const struct iosys_map *src, const struct drm_framebuffer *fb,
> +			     const struct drm_rect *clip, struct drm_format_conv_state *state)
> +{
> +	static const u8 dst_pixsize[DRM_FORMAT_MAX_PLANES] = {
> +		2,
> +	};
> +
> +	drm_fb_xfrm(dst, dst_pitch, dst_pixsize, src, fb, clip, false, state,
> +		    drm_fb_xrgb8888_to_uyvy_line);
> +}
> +EXPORT_SYMBOL(drm_fb_xrgb8888_to_uyvy);
> +
> diff --git a/drivers/gpu/drm/drm_format_internal.h b/drivers/gpu/drm/drm_format_internal.h
> index ce29dd05bcc55..7a4cfb0d188cb 100644
> --- a/drivers/gpu/drm/drm_format_internal.h
> +++ b/drivers/gpu/drm/drm_format_internal.h
> @@ -159,6 +159,27 @@ static inline u32 drm_pixel_xrgb8888_to_abgr2101010(u32 pix)
>   	       drm_pixel_xrgb8888_to_xbgr2101010(pix);
>   }
>   
> +static inline u32 drm_pixel_rgb_to_y_bt601(u32 r, u32 g, u32 b)
> +{
> +	const u32 luma = (16U << 16) + 16829 * r + 33039 * g + 6416 * b + 32768;
> +
> +	return luma >> 16;
> +}
> +
> +static inline u32 drm_pixel_rgb2_to_u_bt601(u32 r2, u32 g2, u32 b2)
> +{
> +	const u32 u = (128U << 17) - 9714 * r2 - 19071 * g2 + 28784 * b2 + 65536;
> +
> +	return u >> 17;
> +}
> +
> +static inline u32 drm_pixel_rgb2_to_v_bt601(u32 r2, u32 g2, u32 b2)
> +{
> +	const u32 v = (128U << 17) + 28784 * r2 - 24103 * g2 - 4681 * b2 + 65536;
> +
> +	return v >> 17;
> +}
> +

These helpers seem too fine-grained. IMHO it would be better to have a 
helper like  drm_pixel_rgb_to_uyvy() that takes two RGB values and 
returns the assembled 16-bit UYVY value.

Best regards
Thomas

>   /*
>    * Conversion from ARGB8888
>    */
> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
> index 2b5c1aef80b0e..e50f96e07484c 100644
> --- a/include/drm/drm_format_helper.h
> +++ b/include/drm/drm_format_helper.h
> @@ -136,4 +136,8 @@ void drm_fb_xrgb8888_to_gray2(struct iosys_map *dst, const unsigned int *dst_pit
>   			      const struct iosys_map *src, const struct drm_framebuffer *fb,
>   			      const struct drm_rect *clip, struct drm_format_conv_state *state);
>   
> +void drm_fb_xrgb8888_to_uyvy(struct iosys_map *dst, const unsigned int *dst_pitch,
> +			     const struct iosys_map *src, const struct drm_framebuffer *fb,
> +			     const struct drm_rect *clip, struct drm_format_conv_state *state);
> +
>   #endif /* __LINUX_DRM_FORMAT_HELPER_H */

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)

Re: [PATCH] drm/format-helper: Add XRGB8888 to UYVY conversion

[Jie Feng]

On Tue, 13 Oct 2026 at 15:11, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 12.10.26 um 19:08 schrieb Ho Jie Feng:
> > Add a helper for devices that use packed UYVY but accept XRGB8888
> > framebuffers.
>
> Is there a description of UYVY available somewhere?
This is for a reverse engineered usb display adapter, which accepts
input in bytes of U, Y, V, Y in that order. I'm not sure what
counts as a description, apart from it matching DRM_FORMAT_UYVY?

This patch is meant to move this out of the driver:
https://github.com/rhgndf/ms912x/blob/main/ms912x_transfer.c#L153

> >
> > Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
> > ---
> > This is used for a driver, ms912x that I will be submitting. It only
> > supports uyvy framebuffers.
>
> Please resubmit as part of the driver series. We're not going to merge
> it without callers anyway.
> ...
> For naming, please follow the conventions used by the other helpers in
> this file.
Will send the driver in with this patch and its fixes, thanks for the review.

> ...
> > +             v = drm_pixel_rgb2_to_v_bt601(r, g, b);
> > +             u = drm_pixel_rgb2_to_u_bt601(r, g, b);
> > +
> > +             transfer_buffer[dst_offset++] = u;
> > +             transfer_buffer[dst_offset++] = y1;
> > +             transfer_buffer[dst_offset++] = v;
> > +             transfer_buffer[dst_offset++] = y2;
>
> Rather that writing bytes individually, use put_unaligned_le32() or a
> similar helper.  Byte order within a pixel is little endian, although
> I'm not sure what this means for UYVY.
This is meant to be sent over a usb transfer, so byte order needs to be
the same on both little and big endian systems.

> > ...
> > +static inline u32 drm_pixel_rgb_to_y_bt601(u32 r, u32 g, u32 b)
> > +{
> > +     const u32 luma = (16U << 16) + 16829 * r + 33039 * g + 6416 * b + 32768;
> > +
> > +     return luma >> 16;
> > +}
> > +
> > +static inline u32 drm_pixel_rgb2_to_u_bt601(u32 r2, u32 g2, u32 b2)
> > +{
> > +     const u32 u = (128U << 17) - 9714 * r2 - 19071 * g2 + 28784 * b2 + 65536;
> > +
> > +     return u >> 17;
> > +}
> > +
> > +static inline u32 drm_pixel_rgb2_to_v_bt601(u32 r2, u32 g2, u32 b2)
> > +{
> > +     const u32 v = (128U << 17) + 28784 * r2 - 24103 * g2 - 4681 * b2 + 65536;
> > +
> > +     return v >> 17;
> > +}
> > +
>
> These helpers seem too fine-grained. IMHO it would be better to have a
> helper like  drm_pixel_rgb_to_uyvy() that takes two RGB values and
> returns the assembled 16-bit UYVY value.
Makes sense.

> Best regards
> Thomas
>
> >   /*
> >    * Conversion from ARGB8888
> >    */
> > diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
> > index 2b5c1aef80b0e..e50f96e07484c 100644
> > --- a/include/drm/drm_format_helper.h
> > +++ b/include/drm/drm_format_helper.h
> > @@ -136,4 +136,8 @@ void drm_fb_xrgb8888_to_gray2(struct iosys_map *dst, const unsigned int *dst_pit
> >                             const struct iosys_map *src, const struct drm_framebuffer *fb,
> >                             const struct drm_rect *clip, struct drm_format_conv_state *state);
> >
> > +void drm_fb_xrgb8888_to_uyvy(struct iosys_map *dst, const unsigned int *dst_pitch,
> > +                          const struct iosys_map *src, const struct drm_framebuffer *fb,
> > +                          const struct drm_rect *clip, struct drm_format_conv_state *state);
> > +
> >   #endif /* __LINUX_DRM_FORMAT_HELPER_H */
>
> --
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
> GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)
>
>

Re: [PATCH] drm/format-helper: Add XRGB8888 to UYVY conversion

[Thomas Zimmermann]

Hi

Am 13.10.26 um 14:17 schrieb Jie Feng:
> On Tue, 13 Oct 2026 at 15:11, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Hi
>>
>> Am 12.10.26 um 19:08 schrieb Ho Jie Feng:
>>> Add a helper for devices that use packed UYVY but accept XRGB8888
>>> framebuffers.
>> Is there a description of UYVY available somewhere?
> This is for a reverse engineered usb display adapter, which accepts
> input in bytes of U, Y, V, Y in that order. I'm not sure what
> counts as a description, apart from it matching DRM_FORMAT_UYVY?

I see. The format exists already in DRM.

>
> This patch is meant to move this out of the driver:
> https://github.com/rhgndf/ms912x/blob/main/ms912x_transfer.c#L153
>
>>> Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
>>> ---
>>> This is used for a driver, ms912x that I will be submitting. It only
>>> supports uyvy framebuffers.
>> Please resubmit as part of the driver series. We're not going to merge
>> it without callers anyway.
>> ...
>> For naming, please follow the conventions used by the other helpers in
>> this file.
> Will send the driver in with this patch and its fixes, thanks for the review.
>
>> ...
>>> +             v = drm_pixel_rgb2_to_v_bt601(r, g, b);
>>> +             u = drm_pixel_rgb2_to_u_bt601(r, g, b);
>>> +
>>> +             transfer_buffer[dst_offset++] = u;
>>> +             transfer_buffer[dst_offset++] = y1;
>>> +             transfer_buffer[dst_offset++] = v;
>>> +             transfer_buffer[dst_offset++] = y2;
>> Rather that writing bytes individually, use put_unaligned_le32() or a
>> similar helper.  Byte order within a pixel is little endian, although
>> I'm not sure what this means for UYVY.
> This is meant to be sent over a usb transfer, so byte order needs to be
> the same on both little and big endian systems.

DRM formats always refer to little endian. As it is implemented now, 
it's not clear what the endianess of your output is. Please duplicate 
the other helpers' logic. They all pack the output with appropriate 
helpers. If the USB device requires BE, you have to write a separate 
conversion helper for that.

BTW, you have an OOB access if the line width is not a multiple of 2.

Best regards
Thomas

>
>>> ...
>>> +static inline u32 drm_pixel_rgb_to_y_bt601(u32 r, u32 g, u32 b)
>>> +{
>>> +     const u32 luma = (16U << 16) + 16829 * r + 33039 * g + 6416 * b + 32768;
>>> +
>>> +     return luma >> 16;
>>> +}
>>> +
>>> +static inline u32 drm_pixel_rgb2_to_u_bt601(u32 r2, u32 g2, u32 b2)
>>> +{
>>> +     const u32 u = (128U << 17) - 9714 * r2 - 19071 * g2 + 28784 * b2 + 65536;
>>> +
>>> +     return u >> 17;
>>> +}
>>> +
>>> +static inline u32 drm_pixel_rgb2_to_v_bt601(u32 r2, u32 g2, u32 b2)
>>> +{
>>> +     const u32 v = (128U << 17) + 28784 * r2 - 24103 * g2 - 4681 * b2 + 65536;
>>> +
>>> +     return v >> 17;
>>> +}
>>> +
>> These helpers seem too fine-grained. IMHO it would be better to have a
>> helper like  drm_pixel_rgb_to_uyvy() that takes two RGB values and
>> returns the assembled 16-bit UYVY value.
> Makes sense.
>
>> Best regards
>> Thomas
>>
>>>    /*
>>>     * Conversion from ARGB8888
>>>     */
>>> diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
>>> index 2b5c1aef80b0e..e50f96e07484c 100644
>>> --- a/include/drm/drm_format_helper.h
>>> +++ b/include/drm/drm_format_helper.h
>>> @@ -136,4 +136,8 @@ void drm_fb_xrgb8888_to_gray2(struct iosys_map *dst, const unsigned int *dst_pit
>>>                              const struct iosys_map *src, const struct drm_framebuffer *fb,
>>>                              const struct drm_rect *clip, struct drm_format_conv_state *state);
>>>
>>> +void drm_fb_xrgb8888_to_uyvy(struct iosys_map *dst, const unsigned int *dst_pitch,
>>> +                          const struct iosys_map *src, const struct drm_framebuffer *fb,
>>> +                          const struct drm_rect *clip, struct drm_format_conv_state *state);
>>> +
>>>    #endif /* __LINUX_DRM_FORMAT_HELPER_H */
>> --
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
>> GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)
>>
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)

Re: [PATCH] drm/format-helper: Add XRGB8888 to UYVY conversion

[Jie Feng]

On Tue, 13 Oct 2026 at 20:32, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 13.10.26 um 14:17 schrieb Jie Feng:
> > On Tue, 13 Oct 2026 at 15:11, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >> Hi
> >>
> >> Am 12.10.26 um 19:08 schrieb Ho Jie Feng:
> >>> Add a helper for devices that use packed UYVY but accept XRGB8888
> >>> framebuffers.
> >> Is there a description of UYVY available somewhere?
> > This is for a reverse engineered usb display adapter, which accepts
> > input in bytes of U, Y, V, Y in that order. I'm not sure what
> > counts as a description, apart from it matching DRM_FORMAT_UYVY?
>
> I see. The format exists already in DRM.
>
> >
> > This patch is meant to move this out of the driver:
> > https://github.com/rhgndf/ms912x/blob/main/ms912x_transfer.c#L153
> >
> >>> Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
> >>> ---
> >>> This is used for a driver, ms912x that I will be submitting. It only
> >>> supports uyvy framebuffers.
> >> Please resubmit as part of the driver series. We're not going to merge
> >> it without callers anyway.
> >> ...
> >> For naming, please follow the conventions used by the other helpers in
> >> this file.
> > Will send the driver in with this patch and its fixes, thanks for the review.
> >
> >> ...
> >>> +             v = drm_pixel_rgb2_to_v_bt601(r, g, b);
> >>> +             u = drm_pixel_rgb2_to_u_bt601(r, g, b);
> >>> +
> >>> +             transfer_buffer[dst_offset++] = u;
> >>> +             transfer_buffer[dst_offset++] = y1;
> >>> +             transfer_buffer[dst_offset++] = v;
> >>> +             transfer_buffer[dst_offset++] = y2;
> >> Rather that writing bytes individually, use put_unaligned_le32() or a
> >> similar helper.  Byte order within a pixel is little endian, although
> >> I'm not sure what this means for UYVY.
> > This is meant to be sent over a usb transfer, so byte order needs to be
> > the same on both little and big endian systems.
>
> DRM formats always refer to little endian. As it is implemented now,
> it's not clear what the endianess of your output is. Please duplicate
> the other helpers' logic. They all pack the output with appropriate
> helpers. If the USB device requires BE, you have to write a separate
> conversion helper for that.
>
> BTW, you have an OOB access if the line width is not a multiple of 2.
How should I handle this? Rounding it within the helper doesn't seem
to make sense and the function return is a void, so it can't return -EINVAL.

Other helpers ignore the -ENOMEM error when drm_format_conv_state_reserve
fails, so should it follow this convention?

Alternatively, drm_fb_xfrm can be exported, and let the driver define
the rgb->yuv coefficients? But I think it is fine since _to_gray8 also
cites BT.601.