Re: [PATCH v2] drm/rockchip: Fix YUV buffers color rendering

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2018-12-14 at 13:29 -0300, Ezequiel Garcia wrote:
> From: Daniele Castagna <dcastagna@xxxxxxxxxxxx>
> 
> Currently, YUV hardware overlays are converted to RGB using
> a color space conversion different than BT.601.
> 
> The result is that colors of e.g. NV12 buffers don't match
> colors of YUV hardware overlays.
> 
> In order to fix this, enable YUV2YUV and set appropriate coefficients
> for formats such as NV12 to be displayed correctly.
> 
> This commit was tested using modetest, gstreamer and chromeos (hardware
> accelerated video playback). Before the commit, tests rendering
> with NV12 format resulted in colors not displayed correctly.
> 
> Test examples (RK3399 Ficus board connected to HDMI monitor):
> 
>   $ modetest 39@32:1920x1080@NV12
>   $ gst-launch-1.0 videotestrc ! video/x-raw,format=NV12 ! kmssink
> 
> Signed-off-by: Daniele Castagna <dcastagna@xxxxxxxxxxxx>
> [ezequiel: rebase on linux-next and massage commit log]
> Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> ---
> v2:
>   * Addressed feedback from Sean Paul
>   * Rebased on linux-next
> 

Any feedback on this one?

Thanks!
Ezequiel

>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 41 ++++++++++++++++++++-
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 13 +++++++
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 36 ++++++++++++++++++
>  3 files changed, 89 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fb70fb486fbf..78c7f63a60c0 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -52,6 +52,18 @@
>  		vop_reg_set(vop, &win->phy->scl->ext->name, \
>  			    win->base, ~0, v, #name)
>  
> +#define VOP_WIN_YUV2YUV_SET(x, win_yuv2yuv, name, v) \
> +	do { \
> +		if (win_yuv2yuv->name.mask) \
> +			vop_reg_set(vop, &win_yuv2yuv->name, 0, ~0, v, #name); \
> +	} while (0)
> +
> +#define VOP_WIN_YUV2YUV_COEFFICIENT_SET(x, win_yuv2yuv, name, v) \
> +	do { \
> +		if (win_yuv2yuv->phy->name.mask) \
> +			vop_reg_set(vop, &win_yuv2yuv->phy->name, win_yuv2yuv->base, ~0, v, #name); \
> +	} while (0)
> +
>  #define VOP_INTR_SET_MASK(vop, name, mask, v) \
>  		vop_reg_set(vop, &vop->data->intr->name, 0, mask, v, #name)
>  
> @@ -84,6 +96,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 rockchip_bt601_yuv2rgb[] = {
> +	0x000004a8, 0x00000000, 0x00000662,
> +	0x000004a8, 0x00001e6f, 0x00001cbf,
> +	0x000004a8, 0x00000812, 0x00000000,
> +	0x00321168, 0x000877cf, 0x002eb127
> +};
> +
>  enum vop_pending {
>  	VOP_PENDING_FB_UNREF,
>  };
> @@ -91,6 +115,7 @@ enum vop_pending {
>  struct vop_win {
>  	struct drm_plane base;
>  	const struct vop_win_data *data;
> +	const struct vop_win_yuv2yuv_data *yuv2yuv_data;
>  	struct vop *vop;
>  };
>  
> @@ -712,6 +737,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
>  	struct drm_crtc *crtc = state->crtc;
>  	struct vop_win *vop_win = to_vop_win(plane);
>  	const struct vop_win_data *win = vop_win->data;
> +	const struct vop_win_yuv2yuv_data *win_yuv2yuv = vop_win->yuv2yuv_data;
>  	struct vop *vop = to_vop(state->crtc);
>  	struct drm_framebuffer *fb = state->fb;
>  	unsigned int actual_w, actual_h;
> @@ -727,6 +753,8 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
>  	bool rb_swap;
>  	int win_index = VOP_WIN_TO_INDEX(vop_win);
>  	int format;
> +	int is_yuv = fb->format->is_yuv;
> +	int i;
>  
>  	/*
>  	 * can't update plane when vop is disabled.
> @@ -767,7 +795,10 @@ 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 (fb->format->is_yuv) {
> +
> +	VOP_WIN_YUV2YUV_SET(vop, win_yuv2yuv, 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];
> @@ -781,6 +812,13 @@ 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 < NUM_YUV2YUV_COEFFICIENTS; i++) {
> +			VOP_WIN_YUV2YUV_COEFFICIENT_SET(vop,
> +							win_yuv2yuv,
> +							y2r_coefficients[i],
> +							rockchip_bt601_yuv2rgb[i]);
> +		}
>  	}
>  
>  	if (win->phy->scl)
> @@ -1529,6 +1567,7 @@ static void vop_win_init(struct vop *vop)
>  
>  		vop_win->data = win_data;
>  		vop_win->vop = vop;
> +		vop_win->yuv2yuv_data = &vop_data->win_yuv2yuv[i];
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index 0fe40e1983d9..aed467cd81b9 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -23,6 +23,8 @@
>  #define VOP_MAJOR(version)		((version) >> 8)
>  #define VOP_MINOR(version)		((version) & 0xff)
>  
> +#define NUM_YUV2YUV_COEFFICIENTS 12
> +
>  enum vop_data_format {
>  	VOP_FMT_ARGB8888 = 0,
>  	VOP_FMT_RGB888,
> @@ -124,6 +126,10 @@ struct vop_scl_regs {
>  	struct vop_reg scale_cbcr_y;
>  };
>  
> +struct vop_yuv2yuv_phy {
> +	struct vop_reg y2r_coefficients[NUM_YUV2YUV_COEFFICIENTS];
> +};
> +
>  struct vop_win_phy {
>  	const struct vop_scl_regs *scl;
>  	const uint32_t *data_formats;
> @@ -146,6 +152,12 @@ struct vop_win_phy {
>  	struct vop_reg channel;
>  };
>  
> +struct vop_win_yuv2yuv_data {
> +	uint32_t base;
> +	const struct vop_yuv2yuv_phy *phy;
> +	struct vop_reg y2r_en;
> +};
> +
>  struct vop_win_data {
>  	uint32_t base;
>  	const struct vop_win_phy *phy;
> @@ -159,6 +171,7 @@ struct vop_data {
>  	const struct vop_misc *misc;
>  	const struct vop_modeset *modeset;
>  	const struct vop_output *output;
> +	const struct vop_win_yuv2yuv_data *win_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 08fc40af52c8..fe752df4e038 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -637,6 +637,34 @@ static const struct vop_output rk3399_output = {
>  	.mipi_dual_channel_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 3),
>  };
>  
> +static const struct vop_yuv2yuv_phy rk3399_yuv2yuv_win01_data = {
> +	.y2r_coefficients = {
> +		VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 0, 0xffff, 0),
> +		VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 0, 0xffff, 16),
> +		VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 4, 0xffff, 0),
> +		VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 4, 0xffff, 16),
> +		VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 8, 0xffff, 0),
> +		VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 8, 0xffff, 16),
> +		VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 12, 0xffff, 0),
> +		VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 12, 0xffff, 16),
> +		VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 16, 0xffff, 0),
> +		VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 20, 0xffffffff, 0),
> +		VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 24, 0xffffffff, 0),
> +		VOP_REG(RK3399_WIN0_YUV2YUV_Y2R + 28, 0xffffffff, 0),
> +	},
> +};
> +
> +static const struct vop_yuv2yuv_phy rk3399_yuv2yuv_win23_data = { };
> +
> +static const struct vop_win_yuv2yuv_data rk3399_vop_big_win_yuv2yuv_data[] = {
> +	{ .base = 0x00, .phy = &rk3399_yuv2yuv_win01_data,
> +	  .y2r_en = VOP_REG(RK3399_YUV2YUV_WIN, 0x1, 1) },
> +	{ .base = 0x60, .phy = &rk3399_yuv2yuv_win01_data,
> +	  .y2r_en = VOP_REG(RK3399_YUV2YUV_WIN, 0x1, 9) },
> +	{ .base = 0xC0, .phy = &rk3399_yuv2yuv_win23_data },
> +	{ .base = 0x120, .phy = &rk3399_yuv2yuv_win23_data },
> +};
> +
>  static const struct vop_data rk3399_vop_big = {
>  	.version = VOP_VERSION(3, 5),
>  	.feature = VOP_FEATURE_OUTPUT_RGB10,
> @@ -647,6 +675,7 @@ static const struct vop_data rk3399_vop_big = {
>  	.misc = &rk3368_misc,
>  	.win = rk3368_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
> +	.win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
>  };
>  
>  static const struct vop_win_data rk3399_vop_lit_win_data[] = {
> @@ -656,6 +685,12 @@ static const struct vop_win_data rk3399_vop_lit_win_data[] = {
>  	  .type = DRM_PLANE_TYPE_CURSOR},
>  };
>  
> +static const struct vop_win_yuv2yuv_data rk3399_vop_lit_win_yuv2yuv_data[] = {
> +	{ .base = 0x00, .phy = &rk3399_yuv2yuv_win01_data,
> +	  .y2r_en = VOP_REG(RK3399_YUV2YUV_WIN, 0x1, 1)},
> +	{ .base = 0x60, .phy = &rk3399_yuv2yuv_win23_data },
> +};
> +
>  static const struct vop_data rk3399_vop_lit = {
>  	.version = VOP_VERSION(3, 6),
>  	.intr = &rk3366_vop_intr,
> @@ -665,6 +700,7 @@ static const struct vop_data rk3399_vop_lit = {
>  	.misc = &rk3368_misc,
>  	.win = rk3399_vop_lit_win_data,
>  	.win_size = ARRAY_SIZE(rk3399_vop_lit_win_data),
> +	.win_yuv2yuv = rk3399_vop_lit_win_yuv2yuv_data,
>  };
>  
>  static const struct vop_win_data rk3228_vop_win_data[] = {


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux