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

[Thomas Zimmermann]

Hi

Am 13.10.26 um 15:07 schrieb Jie Feng:
> On Tue, 13 Oct 2026 at 20:32, Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:
> > Hi
> >
> > Am 13.10.26 um 14:17 schrieb Jie Feng:
> > > On Tue, 13 Oct 2026 at 15:11, Thomas Zimmermann <tzimmermann@xxxxxxx> 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@xxxxxxxxx>
> > > > > ---
> > > > > 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.

It's the callers responsibility to pass in enough pixels. You'll have to 
take care of it in the driver by rounding the damage area accordingly. 
It's not optimal but it's not a big deal either.

In the line-conversion helper, simply round down to the nearest multiple 
of 2.

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

Do you have a specific example?

> 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.

The current constants seem fine. With the 2-pixel alignment, there 
shouldn't be any other cases required.

Best regards
Thomas



--
--
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)