On Thu, Feb 15, 2018 at 12:32:51AM -0500, Daniele Castagna wrote:
> Currently NV12 hardware overlays scheduled with atomic interface are
> converted to RGB using a color space conversion different than BT.601.
>
> The result is that colors of NV12 buffers composited with Mali don't
> match colors of YUV hardware overlays.
>
> Running modetest with an NV12 plane also shows colors are incorrect
> if compared to an RGB overlay that represents the same color pattern.
>
> This CL adds support for YUV2YUV color space conversion (CSC) module on
> rockchip and enables the YUV2RGB part that allows to specify a 4x3
> matrix to convert the colors.
>
> The matrix is set to BT.601 coefficients to align to Mali and to what
> modetest expects.
Hey Daniele,
Apologies for missing this set.
>
> TEST=modetest -M rockchip -s 34@30:2400x1600 -P 30:640x480+650+10@NV12 -P
> 30:640x480+10+10@XR24
>
> Change-Id: Ia5a3b4793229e2c63f79f9f414d1cbe6ccc63fc1
It's usually good form to strip out the gerrit Change-Id from the commit
message.
> Tested-by: Daniele Castagna <dcasta...@chromium.org>
> Reviewed-by: Daniele Castagna <dcasta...@chromium.org>
> Reviewed-by: Kristian H. Kristensen <hoegsb...@chromium.org>
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 40 ++++++++++++++++++++-
> drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 9 +++++
> drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 39 ++++++++++++++++++--
> 3 files changed, 85 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index ba7505292b786..ea43ab797f555 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -50,6 +50,10 @@
> vop_reg_set(vop, &win->phy->scl->ext->name, \
> win->base, ~0, v, #name)
>
> +#define VOP_YUV2YUV_SET(x, name, v) \
> + if ((x)->data->yuv2yuv) \
> + vop_reg_set(vop, &vop->data->yuv2yuv->name, 0, ~0, v,
> #name)
> +
> #define VOP_INTR_SET_MASK(vop, name, mask, v) \
> vop_reg_set(vop, &vop->data->intr->name, 0, mask, v, #name)
>
> @@ -79,6 +83,18 @@
> #define to_vop(x) container_of(x, struct vop, crtc)
> #define to_vop_win(x) container_of(x, struct vop_win, base)
>
> +/*
> + * The coefficients of the following matrix are all fixed points.
> + * The format is S2.10 for the 3x3 part of the matrix, and S9.12 for the
> offsets.
> + * They are all represented in two's complement.
> + */
> +static const uint32_t bt601_yuv2rgb[] = {
> + 0x4A8, 0x0, 0x662,
> + 0x4A8, 0x1E6F, 0x1CBF,
> + 0x4A8, 0x812, 0x0,
> + 0x321168, 0x0877CF, 0x2EB127
> +};
> +
> enum vop_pending {
> VOP_PENDING_FB_UNREF,
> };
> @@ -722,6 +738,9 @@ static void vop_plane_atomic_update(struct drm_plane
> *plane,
> uint32_t val;
> bool rb_swap;
> int format;
> + int is_yuv = is_yuv_support(fb->format->format);
> + int win_index = VOP_WIN_TO_INDEX(vop_win);
> + int i;
>
> /*
> * can't update plane when vop is disabled.
> @@ -762,7 +781,14 @@ static void vop_plane_atomic_update(struct drm_plane
> *plane,
> VOP_WIN_SET(vop, win, 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);
> - if (is_yuv_support(fb->format->format)) {
> +
> + if (!win_index) {
> + VOP_YUV2YUV_SET(vop, win0_y2r_en, is_yuv);
> + } else if (win_index == 1) {
> + VOP_YUV2YUV_SET(vop, win1_y2r_en, is_yuv);
> + }
> +
> + if (is_yuv) {
> int hsub =
> drm_format_horz_chroma_subsampling(fb->format->format);
> int vsub =
> drm_format_vert_chroma_subsampling(fb->format->format);
> int bpp = fb->format->cpp[1];
> @@ -776,6 +802,18 @@ static void vop_plane_atomic_update(struct drm_plane
> *plane,
> 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));
> VOP_WIN_SET(vop, win, uv_mst, dma_addr);
> +
> + for (i = 0; i < 12; i++) {
> + if (!win_index) {
> + VOP_YUV2YUV_SET(vop,
> + win0_y2r_coefficients[i],
> + bt601_yuv2rgb[i]);
> + } else {
> + VOP_YUV2YUV_SET(vop,
> + win1_y2r_coefficients[i],
> + bt601_yuv2rgb[i]);
> + }
> + }
> }
>
> if (win->phy->scl)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index 56bbd2e2a8efb..aa8a5d2690376 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -79,6 +79,14 @@ struct vop_misc {
> struct vop_reg global_regdone_en;
> };
>
> +struct vop_yuv2yuv {
> + struct vop_reg win0_y2r_en;
> + struct vop_reg win0_y2r_coefficients[12];
> +
> + struct vop_reg win1_y2r_en;
> + struct vop_reg win1_y2r_coefficients[12];
> +};
struct vop_yuv2yuv {
struct {
struct vop_reg y2r_en;
struct vop_reg y2r_coefficients[12];
} regs[2];
};
Then add win_index to the macro above and you can avoid the if (!win_index)
checks everywhere.
Bonus points if you #define the '2' and '12' as something more meaningful
> +
> struct vop_intr {
> const int *intrs;
> uint32_t nintrs;
> @@ -157,6 +165,7 @@ struct vop_data {
> const struct vop_misc *misc;
> const struct vop_modeset *modeset;
> const struct vop_output *output;
> + const struct vop_yuv2yuv *yuv2yuv;
> const struct vop_win_data *win;
> unsigned int win_size;
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 2e4eea3459fe6..be9c414e2e4bc 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -422,6 +422,40 @@ static const struct vop_output rk3399_output = {
> .mipi_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 15),
> };
>
> +static const struct vop_yuv2yuv rk3399_vop_yuv2yuv = {
> + .win0_y2r_en = VOP_REG(RK3399_YUV2YUV_WIN, 0x1, 1),
> + .win0_y2r_coefficients = {
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 0, 0xffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 0, 0xffff, 16),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 4, 0xffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 4, 0xffff, 16),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 8, 0xffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 8, 0xffff, 16),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 12, 0xffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 12, 0xffff, 16),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 16, 0xffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 20, 0xffffffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 24, 0xffffffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 28, 0xffffffff, 0),
> + },
> +
> + .win1_y2r_en = VOP_REG(RK3399_YUV2YUV_WIN, 0x1, 9),
> + .win1_y2r_coefficients = {
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 0, 0xffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 0, 0xffff, 16),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 4, 0xffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 4, 0xffff, 16),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 8, 0xffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 8, 0xffff, 16),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 12, 0xffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 12, 0xffff, 16),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 16, 0xffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 20, 0xffffffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 24, 0xffffffff, 0),
> + VOP_REG(RK3399_WIN1_YUV2YUV_Y2R + 28, 0xffffffff, 0),
> + },
> +};
> +
> static const struct vop_data rk3399_vop_big = {
> .version = VOP_VERSION(3, 5),
> .feature = VOP_FEATURE_OUTPUT_RGB10,
> @@ -430,8 +464,9 @@ static const struct vop_data rk3399_vop_big = {
> .modeset = &rk3288_modeset,
> .output = &rk3399_output,
> .misc = &rk3368_misc,
> - .win = rk3368_vop_win_data,
> - .win_size = ARRAY_SIZE(rk3368_vop_win_data),
> + .yuv2yuv = &rk3399_vop_yuv2yuv,
> + .win = rk3399_vop_win_data,
> + .win_size = ARRAY_SIZE(rk3399_vop_win_data),
> };
>
> static const struct vop_win_data rk3399_vop_lit_win_data[] = {
> --
> 2.16.1.291.g4437f3f132-goog
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
