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

2019-01-10 Thread Heiko Stuebner
Am Dienstag, 8. Januar 2019, 22:46:59 CET schrieb Ezequiel Garcia:
> From: Daniele Castagna 
> 
> 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 (Tested on RK3399 and RK3288 boards 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 
> [ezequiel: rebase on linux-next and massage commit log]
> Signed-off-by: Ezequiel Garcia 
> ---
> v3: Add a check for non-null win_yuv2yuv as suggested by Heiko.

applied to drm-misc-next after making sure that display keeps
working on all rockchip socs now

Thanks
Heiko


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


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

2019-01-08 Thread Ezequiel Garcia
From: Daniele Castagna 

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 (Tested on RK3399 and RK3288 boards 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 
[ezequiel: rebase on linux-next and massage commit log]
Signed-off-by: Ezequiel Garcia 
---
v3: Add a check for non-null win_yuv2yuv as suggested by Heiko.

 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 40 -
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 13 +++
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 36 +++
 3 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index db8358e6d230..7ad9067b3110 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -53,6 +53,18 @@
vop_reg_set(vop, >phy->scl->ext->name, \
win->base, ~0, v, #name)
 
+#define VOP_WIN_YUV2YUV_SET(x, win_yuv2yuv, name, v) \
+   do { \
+   if (win_yuv2yuv && win_yuv2yuv->name.mask) \
+   vop_reg_set(vop, _yuv2yuv->name, 0, ~0, v, #name); \
+   } while (0)
+
+#define VOP_WIN_YUV2YUV_COEFFICIENT_SET(x, win_yuv2yuv, name, v) \
+   do { \
+   if (win_yuv2yuv && win_yuv2yuv->phy->name.mask) \
+   vop_reg_set(vop, _yuv2yuv->phy->name, 
win_yuv2yuv->base, ~0, v, #name); \
+   } while (0)
+
 #define VOP_INTR_SET_MASK(vop, name, mask, v) \
vop_reg_set(vop, >data->intr->name, 0, mask, v, #name)
 
@@ -85,6 +97,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,
 };
@@ -92,6 +116,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;
 };
 
@@ -713,6 +738,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;
@@ -728,6 +754,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.
@@ -768,7 +796,9 @@ 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];
@@ -782,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],
+   bt601_yuv2rgb[i]);
+   }
}
 
if (win->phy->scl)
@@