Hi Andy, On Friday, 23 January 2026 02:29:02 Central European Standard Time Andy Yan wrote: > > Hello Nicolas, > > 在 2026-01-22 20:59:41，"Nicolas Frattaroli" <[email protected]> > 写道： > >On Thursday, 22 January 2026 09:28:54 Central European Standard Time Andy > >Yan wrote: > >> > >> Hello Nicolas， > >> > >> At 2026-01-21 22:45:17, "Nicolas Frattaroli" > >> <[email protected]> wrote: > >> >YUV444 (aka YCbCr444) output isn't working quite right on RK3588. The > >> >resulting image on the display, while identifying itself as YUV444, has > >> >some components swapped, even after adding the necessary DRM formats to > >> >the conversion functions. > >> > > >> >Judging by downstream, this is because YUV444 also needs an rb swap > >> >performed in the AFBC case. > >> > > >> >Add the DRM formats to the appropriate switch statements, and add a > >> >function for checking whether an rb swap needs to be performed in the > >> >AFBC case. > >> > > >> >Fixes: 604be85547ce ("drm/rockchip: Add VOP2 driver") > >> >Signed-off-by: Nicolas Frattaroli <[email protected]> > >> >--- > >> > drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 19 +++++++++++++++++++ > >> > 1 file changed, 19 insertions(+) > >> > > >> >diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > >> >b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > >> >index ec3b4fde10db..469c63dd97d5 100644 > >> >--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > >> >+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c > >> >@@ -176,6 +176,7 @@ static enum vop2_data_format vop2_convert_format(u32 > >> >format) > >> > case DRM_FORMAT_ARGB2101010: > >> > case DRM_FORMAT_XBGR2101010: > >> > case DRM_FORMAT_ABGR2101010: > >> >+ case DRM_FORMAT_VUY101010: > >> > return VOP2_FMT_XRGB101010; > >> > case DRM_FORMAT_XRGB8888: > >> > case DRM_FORMAT_ARGB8888: > >> >@@ -184,6 +185,7 @@ static enum vop2_data_format vop2_convert_format(u32 > >> >format) > >> > return VOP2_FMT_ARGB8888; > >> > case DRM_FORMAT_RGB888: > >> > case DRM_FORMAT_BGR888: > >> >+ case DRM_FORMAT_VUY888: > >> > return VOP2_FMT_RGB888; > >> > case DRM_FORMAT_RGB565: > >> > case DRM_FORMAT_BGR565: > >> >@@ -225,6 +227,7 @@ static enum vop2_afbc_format > >> >vop2_convert_afbc_format(u32 format) > >> > case DRM_FORMAT_ARGB2101010: > >> > case DRM_FORMAT_XBGR2101010: > >> > case DRM_FORMAT_ABGR2101010: > >> >+ case DRM_FORMAT_VUY101010: > >> > return VOP2_AFBC_FMT_ARGB2101010; > >> > case DRM_FORMAT_XRGB8888: > >> > case DRM_FORMAT_ARGB8888: > >> >@@ -233,6 +236,7 @@ static enum vop2_afbc_format > >> >vop2_convert_afbc_format(u32 format) > >> > return VOP2_AFBC_FMT_ARGB8888; > >> > case DRM_FORMAT_RGB888: > >> > case DRM_FORMAT_BGR888: > >> >+ case DRM_FORMAT_VUY888: > >> > >> How did you test this format? It seems tools like modetest don’t support > >> testing this pattern. > >> > > > >Hi Andy, > > > >using the rest of this series, which implements the "color format" > >DRM property, and the corresponding weston MR that makes use of it[1]. > > > >I create a ~/.config/weston.ini with the following contents: > > > > [output] > > name=HDMI-A-1 > > color-format=yuv444 > > > >This will make Weston try to set the output format to 10-bit YUV444. To > >limit it to 8-bit, you can add `max-bpc=8`. The monitor's EDID needs to > >report YUV444 support, otherwise that Weston version won't let you set > >this property. > > > > > This looks a bit strange. Your commit message and the Weston configuration > here both target the output format, > but the patch modifies the functions vop2_convert_format and > vop2_convert_afbc_format, which are responsible for > converting the data formats of planes/framebuffers (fb)—these have nothing to > do with the output format.
Yep, I've now re-tested this in various ways and this commit doesn't do what I thought it did. I think when I authored it, this was still doing BCSH based conversion and may have depended on this at some stage. Also possible that I didn't do a clean test run of solely these changes to come to my conclusions. YUV444 primary planes aren't supported by RK3588 at all from what I gather, so I have no clue where I ran into this and how this fixed it. Testing on RK3576 without this, and also playing around with gbm-format, I also don't ever get into the situation where this is needed for correct output; it seems like EGLConfig always only exposes RGBA formats anyway, so Panfrost may still lack YUV format support for the buffers. I'll drop this patch on the next revision, but I'll keep the changes in mind if an atomic modesetting test workload that sets YUV plane formats ever comes to be. Thanks for the reviews! Kind regards, Nicolas Frattaroli > > > >Link: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1859 [1] > > > >Kind regards, > >Nicolas Frattaroli > > > >> > >> > >> > return VOP2_AFBC_FMT_RGB888; > >> > case DRM_FORMAT_RGB565: > >> > case DRM_FORMAT_BGR565: > >> >@@ -270,6 +274,19 @@ static bool vop2_win_rb_swap(u32 format) > >> > } > >> > } > >> > > >> >+static bool vop2_afbc_rb_swap(u32 format) > >> >+{ > >> >+ switch (format) { > >> >+ case DRM_FORMAT_NV24: > >> >+ case DRM_FORMAT_NV30: > >> >+ case DRM_FORMAT_VUY888: > >> >+ case DRM_FORMAT_VUY101010: > >> >+ return true; > >> >+ default: > >> >+ return false; > >> >+ } > >> >+} > >> >+ > >> > static bool vop2_afbc_uv_swap(u32 format) > >> > { > >> > switch (format) { > >> >@@ -1291,6 +1308,7 @@ static void vop2_plane_atomic_update(struct > >> >drm_plane *plane, > >> > /* It's for head stride, each head size is 16 byte */ > >> > stride = ALIGN(stride, block_w) / block_w * 16; > >> > > >> >+ rb_swap = vop2_afbc_rb_swap(fb->format->format); > >> > uv_swap = vop2_afbc_uv_swap(fb->format->format); > >> > /* > >> > * This is a workaround for crazy IC design, Cluster > >> >@@ -1308,6 +1326,7 @@ static void vop2_plane_atomic_update(struct > >> >drm_plane *plane, > >> > vop2_win_write(win, VOP2_WIN_AFBC_ENABLE, 1); > >> > vop2_win_write(win, VOP2_WIN_AFBC_FORMAT, afbc_format); > >> > vop2_win_write(win, VOP2_WIN_AFBC_UV_SWAP, uv_swap); > >> >+ vop2_win_write(win, VOP2_WIN_AFBC_RB_SWAP, rb_swap); > >> > /* > >> > * On rk3566/8, this bit is auto gating enable, > >> > * but this function is not work well so we need > >> > > >> > > > > > > > > >
