Re: [PATCH v2] drm/rockchip: vop: Fix color for RGB888/BGR888 format on VOP full
Hi Jonas,
On Thu, 2023-10-26 at 19:14 +, Jonas Karlman wrote:
> Use of DRM_FORMAT_RGB888 and DRM_FORMAT_BGR888 on e.g. RK3288, RK3328
> and RK3399 result in wrong colors being displayed.
>
> The issue can be observed using modetest:
>
> modetest -s @:1920x1080-60@RG24
> modetest -s @:1920x1080-60@BG24
>
> Vendor 4.4 kernel apply an inverted rb swap for these formats on VOP
> full framework (IP version 3.x) compared to VOP little framework (2.x).
>
> Fix colors by applying different rb swap for VOP full framework (3.x)
> and VOP little framework (2.x) similar to vendor 4.4 kernel.
>
> Fixes: 85a359f25388 ("drm/rockchip: Add BGR formats to VOP")
> Signed-off-by: Jonas Karlman
Reviewed-by: Christopher Obbard
Tested-by: Christopher Obbard
Since you missed adding my *-by tags in v2.
> ---
> Changes in v2:
> - Add comment about different rb swap for IP version 3.x and 2.x
> - Add fixes tag
>
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index b3d0b6ae9294..ed2ed25959a2 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -247,14 +247,22 @@ static inline void vop_cfg_done(struct vop *vop)
> VOP_REG_SET(vop, common, cfg_done, 1);
> }
>
> -static bool has_rb_swapped(uint32_t format)
> +static bool has_rb_swapped(uint32_t version, uint32_t format)
> {
> switch (format) {
> case DRM_FORMAT_XBGR:
> case DRM_FORMAT_ABGR:
> - case DRM_FORMAT_BGR888:
> case DRM_FORMAT_BGR565:
> return true;
> + /*
> + * full framework (IP version 3.x) only need rb swapped for RGB888
> and
> + * little framework (IP version 2.x) only need rb swapped for
> BGR888,
> + * check for 3.x to also only rb swap BGR888 for unknown vop
> version
> + */
> + case DRM_FORMAT_RGB888:
> + return VOP_MAJOR(version) == 3;
> + case DRM_FORMAT_BGR888:
> + return VOP_MAJOR(version) != 3;
> default:
> return false;
> }
> @@ -1035,7 +1043,7 @@ static void vop_plane_atomic_update(struct drm_plane
> *plane,
> VOP_WIN_SET(vop, win, dsp_info, dsp_info);
> VOP_WIN_SET(vop, win, dsp_st, dsp_st);
>
> - rb_swap = has_rb_swapped(fb->format->format);
> + rb_swap = has_rb_swapped(vop->data->version, fb->format->format);
> VOP_WIN_SET(vop, win, rb_swap, rb_swap);
>
> /*
Re: [PATCH] drm/rockchip: vop2: Add NV20 and NV30 support
Hi Jonas,
On Wed, 2023-10-25 at 21:32 +, Jonas Karlman wrote:
> Add support for the 10-bit 4:2:2 and 4:4:4 formats NV20 and NV30.
>
> These formats can be tested using modetest [1]:
>
> modetest -P @:1920x1080@
>
> e.g. on a ROCK 3 Model A (rk3568):
>
> modetest -P 43@67:1920x1080@NV20 -F tiles,tiles
> modetest -P 43@67:1920x1080@NV30 -F smpte,smpte
>
> [1] https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/329
>
> Signed-off-by: Jonas Karlman
Reviewed-by: Christopher Obbard
Tested-by: Christopher Obbard
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 5 +
> drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 2 ++
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index ab944010fe14..592f9d726f2e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -326,11 +326,14 @@ static enum vop2_data_format vop2_convert_format(u32
> format)
> case DRM_FORMAT_NV16:
> case DRM_FORMAT_NV61:
> return VOP2_FMT_YUV422SP;
> + case DRM_FORMAT_NV20:
> case DRM_FORMAT_Y210:
> return VOP2_FMT_YUV422SP_10;
> case DRM_FORMAT_NV24:
> case DRM_FORMAT_NV42:
> return VOP2_FMT_YUV444SP;
> + case DRM_FORMAT_NV30:
> + return VOP2_FMT_YUV444SP_10;
> case DRM_FORMAT_YUYV:
> case DRM_FORMAT_YVYU:
> return VOP2_FMT_VYUY422;
> @@ -415,6 +418,8 @@ static bool vop2_win_uv_swap(u32 format)
> case DRM_FORMAT_NV16:
> case DRM_FORMAT_NV24:
> case DRM_FORMAT_NV15:
> + case DRM_FORMAT_NV20:
> + case DRM_FORMAT_NV30:
> case DRM_FORMAT_YUYV:
> case DRM_FORMAT_UYVY:
> return true;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> index fdb48571efce..0b4280218a59 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop2_reg.c
> @@ -48,8 +48,10 @@ static const uint32_t formats_rk356x_esmart[] = {
> DRM_FORMAT_NV15, /* yuv420_10bit linear mode, 2 plane, no padding
> */
> DRM_FORMAT_NV16, /* yuv422_8bit linear mode, 2 plane */
> DRM_FORMAT_NV61, /* yuv422_8bit linear mode, 2 plane */
> + DRM_FORMAT_NV20, /* yuv422_10bit linear mode, 2 plane, no padding
> */
> DRM_FORMAT_NV24, /* yuv444_8bit linear mode, 2 plane */
> DRM_FORMAT_NV42, /* yuv444_8bit linear mode, 2 plane */
> + DRM_FORMAT_NV30, /* yuv444_10bit linear mode, 2 plane, no padding
> */
> DRM_FORMAT_YVYU, /* yuv422_8bit[YVYU] linear mode */
> DRM_FORMAT_VYUY, /* yuv422_8bit[VYUY] linear mode */
> };
Re: [PATCH] drm/rockchip: vop: Fix color for RGB888/BGR888 format on VOP full
Hi Jonas,
On Mon, 2023-10-23 at 21:11 +, Jonas Karlman wrote:
> Use of DRM_FORMAT_RGB888 and DRM_FORMAT_BGR888 on e.g. RK3288, RK3328
> and RK3399 result in wrong colors being displayed.
>
> The issue can be observed using modetest:
>
> modetest -s @:1920x1080-60@RG24
> modetest -s @:1920x1080-60@BG24
>
> Vendor 4.4 kernel apply an inverted rb swap for these formats on VOP
> full (major = 3).
>
> Fix colors by applying rb swap similar to vendor 4.4 kernel.
>
> Signed-off-by: Jonas Karlman
How about a fixes tag? Seems like this was introduced in commit 85a359f25388
("drm/rockchip: Add BGR formats to VOP")
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index b3d0b6ae9294..a1ce09a22f83 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -247,14 +247,17 @@ static inline void vop_cfg_done(struct vop *vop)
> VOP_REG_SET(vop, common, cfg_done, 1);
> }
>
> -static bool has_rb_swapped(uint32_t format)
> +static bool has_rb_swapped(uint32_t version, uint32_t format)
> {
> switch (format) {
> case DRM_FORMAT_XBGR:
> case DRM_FORMAT_ABGR:
> - case DRM_FORMAT_BGR888:
> case DRM_FORMAT_BGR565:
> return true;
> + case DRM_FORMAT_RGB888:
> + return VOP_MAJOR(version) == 3;
> + case DRM_FORMAT_BGR888:
> + return VOP_MAJOR(version) != 3;
The hardcoded bits are quite scary as it applies to all hardware variants ;-).
Is it worth adding an inline comment to describe what is going on and how it
relates to VOPL and VOPH? Or would it be even better to add this as a quirk in
the various vop_data structs?
Cheers!
Chris
> default:
> return false;
> }
> @@ -1035,7 +1038,7 @@ static void vop_plane_atomic_update(struct drm_plane
> *plane,
> VOP_WIN_SET(vop, win, dsp_info, dsp_info);
> VOP_WIN_SET(vop, win, dsp_st, dsp_st);
>
> - rb_swap = has_rb_swapped(fb->format->format);
> + rb_swap = has_rb_swapped(vop->data->version, fb->format->format);
> VOP_WIN_SET(vop, win, rb_swap, rb_swap);
>
> /*
Re: [PATCH v5 2/2] drm/rockchip: vop: Add NV15, NV20 and NV30 support
Hi Jonas,
On Mon, 2023-10-23 at 17:37 +, Jonas Karlman wrote:
> Add support for displaying 10-bit 4:2:0 and 4:2:2 formats produced by
> the Rockchip Video Decoder on RK322X, RK3288, RK3328 and RK3399.
> Also add support for 10-bit 4:4:4 format while at it.
>
> V5: Use drm_format_info_min_pitch() for correct bpp
> Add missing NV21, NV61 and NV42 formats
> V4: Rework RK3328/RK3399 win0/1 data to not affect RK3368
> V2: Added NV30 support
>
> Signed-off-by: Jonas Karlman
> Reviewed-by: Sandy Huang
Reviewed-by: Christopher Obbard
Tested-by: Christopher Obbard
Cheers!
Chris
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 36 ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 +
> drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 66 +
> 3 files changed, 86 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 14320bc73e5b..b3d0b6ae9294 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -272,6 +272,18 @@ static bool has_uv_swapped(uint32_t format)
> }
> }
>
> +static bool is_fmt_10(uint32_t format)
> +{
> + switch (format) {
> + case DRM_FORMAT_NV15:
> + case DRM_FORMAT_NV20:
> + case DRM_FORMAT_NV30:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> static enum vop_data_format vop_convert_format(uint32_t format)
> {
> switch (format) {
> @@ -287,12 +299,15 @@ static enum vop_data_format
> vop_convert_format(uint32_t format)
> case DRM_FORMAT_BGR565:
> return VOP_FMT_RGB565;
> case DRM_FORMAT_NV12:
> + case DRM_FORMAT_NV15:
> case DRM_FORMAT_NV21:
> return VOP_FMT_YUV420SP;
> case DRM_FORMAT_NV16:
> + case DRM_FORMAT_NV20:
> case DRM_FORMAT_NV61:
> return VOP_FMT_YUV422SP;
> case DRM_FORMAT_NV24:
> + case DRM_FORMAT_NV30:
> case DRM_FORMAT_NV42:
> return VOP_FMT_YUV444SP;
> default:
> @@ -944,7 +959,12 @@ static void vop_plane_atomic_update(struct drm_plane
> *plane,
> dsp_sty = dest->y1 + crtc->mode.vtotal - crtc->mode.vsync_start;
> dsp_st = dsp_sty << 16 | (dsp_stx & 0x);
>
> - offset = (src->x1 >> 16) * fb->format->cpp[0];
> + if (fb->format->char_per_block[0])
> + offset = drm_format_info_min_pitch(fb->format, 0,
> + src->x1 >> 16);
> + else
> + offset = (src->x1 >> 16) * fb->format->cpp[0];
> +
> offset += (src->y1 >> 16) * fb->pitches[0];
> dma_addr = rk_obj->dma_addr + offset + fb->offsets[0];
>
> @@ -970,6 +990,7 @@ static void vop_plane_atomic_update(struct drm_plane
> *plane,
> }
>
> VOP_WIN_SET(vop, win, format, format);
> + VOP_WIN_SET(vop, win, fmt_10, is_fmt_10(fb->format->format));
> VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 4));
> VOP_WIN_SET(vop, win, yrgb_mst, dma_addr);
> VOP_WIN_YUV2YUV_SET(vop, win_yuv2yuv, y2r_en, is_yuv);
> @@ -979,15 +1000,16 @@ static void vop_plane_atomic_update(struct drm_plane
> *plane,
> (new_state->rotation & DRM_MODE_REFLECT_X) ? 1 : 0);
>
> if (is_yuv) {
> - int hsub = fb->format->hsub;
> - int vsub = fb->format->vsub;
> - int bpp = fb->format->cpp[1];
> -
> uv_obj = fb->obj[1];
> rk_uv_obj = to_rockchip_obj(uv_obj);
>
> - offset = (src->x1 >> 16) * bpp / hsub;
> - offset += (src->y1 >> 16) * fb->pitches[1] / vsub;
> + if (fb->format->char_per_block[1])
> + offset = drm_format_info_min_pitch(fb->format, 1,
> + src->x1 >> 16);
> + else
> + offset = (src->x1 >> 16) * fb->format->cpp[1];
> + offset /= fb->format->hsub;
> + offset += (src->y1 >> 16) * fb->pitches[1] / fb->format-
> >vsub;
>
> dma_addr = rk_uv_obj->dma_addr + offset + fb->offsets[1];
> VOP_WIN_SET(vop, win, uv_vir, DIV_ROUND_UP(fb->pitches[1],
> 4));
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index 5f56e0597df8..4b2daefeb8c1 100644
>
Re: [PATCH v5 1/2] drm/fourcc: Add NV20 and NV30 YUV formats
Hi Jonas,
On Mon, 2023-10-23 at 17:37 +, Jonas Karlman wrote:
> DRM_FORMAT_NV20 and DRM_FORMAT_NV30 formats is the 2x1 and non-subsampled
> variant of NV15, a 10-bit 2-plane YUV format that has no padding between
> components. Instead, luminance and chrominance samples are grouped into 4s
> so that each group is packed into an integer number of bytes:
>
> = UVUV = 4 * 10 bits = 40 bits = 5 bytes
>
> The '20' and '30' suffix refers to the optimum effective bits per pixel
> which is achieved when the total number of luminance samples is a multiple
> of 4.
>
> V2: Added NV30 format
>
> Signed-off-by: Jonas Karlman
> Reviewed-by: Sandy Huang
Reviewed-by: Christopher Obbard
Tested-by: Christopher Obbard
Cheers!
Chris
> ---
> drivers/gpu/drm/drm_fourcc.c | 8
> include/uapi/drm/drm_fourcc.h | 2 ++
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index 0f17dfa8702b..193cf8ed7912 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -299,6 +299,14 @@ const struct drm_format_info *__drm_format_info(u32
> format)
> .num_planes = 2, .char_per_block = { 5, 5, 0 },
> .block_w = { 4, 2, 0 }, .block_h = { 1, 1, 0 }, .hsub =
> 2,
> .vsub = 2, .is_yuv = true },
> + { .format = DRM_FORMAT_NV20,.depth = 0,
> + .num_planes = 2, .char_per_block = { 5, 5, 0 },
> + .block_w = { 4, 2, 0 }, .block_h = { 1, 1, 0 }, .hsub =
> 2,
> + .vsub = 1, .is_yuv = true },
> + { .format = DRM_FORMAT_NV30,.depth = 0,
> + .num_planes = 2, .char_per_block = { 5, 5, 0 },
> + .block_w = { 4, 2, 0 }, .block_h = { 1, 1, 0 }, .hsub =
> 1,
> + .vsub = 1, .is_yuv = true },
> { .format = DRM_FORMAT_Q410,.depth = 0,
> .num_planes = 3, .char_per_block = { 2, 2, 2 },
> .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub =
> 1,
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 8db7fd3f743e..3151f1fc7ebb 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -323,6 +323,8 @@ extern "C" {
> * index 1 = Cr:Cb plane, [39:0] Cr1:Cb1:Cr0:Cb0 little endian
> */
> #define DRM_FORMAT_NV15 fourcc_code('N', 'V', '1', '5') /*
> 2x2 subsampled Cr:Cb plane */
> +#define DRM_FORMAT_NV20 fourcc_code('N', 'V', '2', '0') /*
> 2x1 subsampled Cr:Cb plane */
> +#define DRM_FORMAT_NV30 fourcc_code('N', 'V', '3', '0') /*
> non-subsampled Cr:Cb plane */
>
> /*
> * 2 plane YCbCr MSB aligned
Re: [PATCH v5 0/2] drm/rockchip: vop: Add NV15, NV20 and NV30 support
Hi Jonas, On Mon, 2023-10-23 at 17:37 +, Jonas Karlman wrote: > This series add support for displaying 10-bit 4:2:0 and 4:2:2 formats > produced > by the Rockchip Video Decoder on RK322X, RK3288, RK3328, RK3368 and RK3399. > Also include 10-bit 4:4:4 support since VOP can support that also. > > First patch adds new fourcc 10-bit YUV formats with 4:2:2/4:4:4 sub- > sampling. > Second patch adds support for displaying the new fourcc formats. > > These patches have been in use by LibreELEC and other distros for the > past 3+ years, hoping they can be merged this time around. > > A rough libdrm/modetest patch [2] have been used to validate use of > NV15, NV20 and NV30 formats on RK3288, RK3328 and RK3399 boards. > > modetest -s @:-@ > > Tinker Board R2.0 (rk3288w): > modetest -s 50:1920x1080-60@NV15 > > Rock Pi 4 (rk3399): > modetest -s 52@44:1920x1080-60@NV15 > > Rock64 (rk3328): > modetest -s 42:1920x1080-60@NV15 > > Changes in v5: > - Use drm_format_info_min_pitch() for correct bpp > - Add missing NV21, NV61 and NV42 formats > > Changes in v4: > - Rework RK3328/RK3399 win0/1 data to not affect RK3368 > > Changes in v3: > - No changes, rebased on next-20230616 > - R-B tags was collected > > Changes in v2: > - Add NV30 format > - R-B tags was not collected due to NV30 changes > > This series is also available at [1] and libdrm/modetest patch at [2]. > > [1] https://github.com/Kwiboo/linux-rockchip/commits/v6.6-rc7-vop-nv15 > [2] https://github.com/Kwiboo/libdrm/commits/nv15 Could you open a draft merge request for libdrm upstream at https://gitlab.freedesktop.org/mesa/drm pointing to this series? If there are subsequent revisions of this series, it'd be great to link that merge request in the cover letter. Cheers! Chris > > Jonas Karlman (2): > drm/fourcc: Add NV20 and NV30 YUV formats > drm/rockchip: vop: Add NV15, NV20 and NV30 support > > drivers/gpu/drm/drm_fourcc.c | 8 +++ > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 36 --- > drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 + > drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 66 + > include/uapi/drm/drm_fourcc.h | 2 + > 5 files changed, 96 insertions(+), 17 deletions(-) >
