[PATCH] drm/malidp: Fix writeback in NV12

Brian Starkey brian.starkey at arm.com
Mon Aug 20 14:02:26 UTC 2018


Hi Alex,

On Mon, Aug 20, 2018 at 02:24:05PM +0100, Alexandru Gheorghe wrote:
>When we want to writeback to memory in NV12 format we need to program
>the RGB2YUV coefficients.
>Currently, we don't program the coefficients and the NV12 doesn't work
>at all.
>
>This patchset fixes that by programming a sane default(bt709, limited
>range) as rgb2yuv coefficients.
>
>In the long run, probably we need to think of a way for userspace to
>be able to program that, but for now I think this is better than not
>working at all or not advertising NV12 as a supported format for
>memwrite.
>
>Reviewed-by: Liviu Dudau <liviu.dudau at arm.com>
>Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe at arm.com>
>---
> drivers/gpu/drm/arm/malidp_hw.c   | 25 +++++++++++++++++++++++--
> drivers/gpu/drm/arm/malidp_hw.h   |  6 ++++--
> drivers/gpu/drm/arm/malidp_mw.c   | 12 +++++++++++-
> drivers/gpu/drm/arm/malidp_regs.h |  2 ++
> 4 files changed, 40 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
>index c94a4422e0e9..2781e462c1ed 100644
>--- a/drivers/gpu/drm/arm/malidp_hw.c
>+++ b/drivers/gpu/drm/arm/malidp_hw.c
>@@ -384,7 +384,8 @@ static long malidp500_se_calc_mclk(struct malidp_hw_device *hwdev,
>
> static int malidp500_enable_memwrite(struct malidp_hw_device *hwdev,
> 				     dma_addr_t *addrs, s32 *pitches,
>-				     int num_planes, u16 w, u16 h, u32 fmt_id)
>+				     int num_planes, u16 w, u16 h, u32 fmt_id,
>+				     const s16 *rgb2yuv_coeffs)
> {
> 	u32 base = MALIDP500_SE_MEMWRITE_BASE;
> 	u32 de_base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK);
>@@ -416,6 +417,16 @@ static int malidp500_enable_memwrite(struct malidp_hw_device *hwdev,
>
> 	malidp_hw_write(hwdev, MALIDP_DE_H_ACTIVE(w) | MALIDP_DE_V_ACTIVE(h),
> 			MALIDP500_SE_MEMWRITE_OUT_SIZE);
>+
>+	if (rgb2yuv_coeffs) {
>+		int i;
>+
>+		for (i = 0; i < MALIDP_COLORADJ_NUM_COEFFS; i++) {
>+			malidp_hw_write(hwdev, rgb2yuv_coeffs[i],
>+					MALIDP500_SE_RGB_YUV_COEFFS + i * 4);
>+		}
>+	}
>+
> 	malidp_hw_setbits(hwdev, MALIDP_SE_MEMWRITE_EN, MALIDP500_SE_CONTROL);
>
> 	return 0;
>@@ -658,7 +669,8 @@ static long malidp550_se_calc_mclk(struct malidp_hw_device *hwdev,
>
> static int malidp550_enable_memwrite(struct malidp_hw_device *hwdev,
> 				     dma_addr_t *addrs, s32 *pitches,
>-				     int num_planes, u16 w, u16 h, u32 fmt_id)
>+				     int num_planes, u16 w, u16 h, u32 fmt_id,
>+				     const s16 *rgb2yuv_coeffs)
> {
> 	u32 base = MALIDP550_SE_MEMWRITE_BASE;
> 	u32 de_base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK);
>@@ -689,6 +701,15 @@ static int malidp550_enable_memwrite(struct malidp_hw_device *hwdev,
> 	malidp_hw_setbits(hwdev, MALIDP550_SE_MEMWRITE_ONESHOT | MALIDP_SE_MEMWRITE_EN,
> 			  MALIDP550_SE_CONTROL);
>
>+	if (rgb2yuv_coeffs) {
>+		int i;
>+
>+		for (i = 0; i < MALIDP_COLORADJ_NUM_COEFFS; i++) {
>+			malidp_hw_write(hwdev, rgb2yuv_coeffs[i],
>+					MALIDP550_SE_RGB_YUV_COEFFS + i * 4);
>+		}
>+	}
>+
> 	return 0;
> }
>
>diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
>index ad2e96915d44..d2ceb70305ca 100644
>--- a/drivers/gpu/drm/arm/malidp_hw.h
>+++ b/drivers/gpu/drm/arm/malidp_hw.h
>@@ -190,8 +190,10 @@ struct malidp_hw {
> 	 * @param h - height of the output frame
> 	 * @param fmt_id - internal format ID of output buffer
> 	 */
>-	int (*enable_memwrite)(struct malidp_hw_device *hwdev, dma_addr_t *addrs,
>-			       s32 *pitches, int num_planes, u16 w, u16 h, u32 fmt_id);
>+	int (*enable_memwrite)(struct malidp_hw_device *hwdev,
>+			       dma_addr_t *addrs, s32 *pitches, int num_planes,
>+			       u16 w, u16 h, u32 fmt_id,
>+			       const s16 *rgb2yuv_coeffs);
>
> 	/*
> 	 * Disable the writing to memory of the next frame's content.
>diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
>index cfd718e7e97c..0523ba5d27dd 100644
>--- a/drivers/gpu/drm/arm/malidp_mw.c
>+++ b/drivers/gpu/drm/arm/malidp_mw.c
>@@ -213,6 +213,13 @@ int malidp_mw_connector_init(struct drm_device *drm)
> 	return 0;
> }
>
>+static const s16 rgb2yuv_coeffs_bt709_limited[MALIDP_COLORADJ_NUM_COEFFS] = {
>+	47,  157,   16,
>+	-26,  -87,  112,
>+	112, -102,  -10,
>+	16,  128,  128
>+};
>+
> void malidp_mw_atomic_commit(struct drm_device *drm,
> 			     struct drm_atomic_state *old_state)
> {
>@@ -242,7 +249,10 @@ void malidp_mw_atomic_commit(struct drm_device *drm,
>
> 		hwdev->hw->enable_memwrite(hwdev, mw_state->addrs,
> 					   mw_state->pitches, mw_state->n_planes,
>-					   fb->width, fb->height, mw_state->format);
>+					   fb->width, fb->height,
>+					   mw_state->format,
>+					   fb->format->is_yuv ?
>+					   rgb2yuv_coeffs_bt709_limited : NULL);

Given we only have one set of coefficients, it would be nice to either
program once at power-up, or to at least track in the connector state
whether the coeffs are already programmed instead of programming every
frame.

Cheers,
-Brian

> 	} else {
> 		DRM_DEV_DEBUG_DRIVER(drm->dev, "Disable memwrite\n");
> 		hwdev->hw->disable_memwrite(hwdev);
>diff --git a/drivers/gpu/drm/arm/malidp_regs.h b/drivers/gpu/drm/arm/malidp_regs.h
>index 3579d36b2a71..6ffe849774f2 100644
>--- a/drivers/gpu/drm/arm/malidp_regs.h
>+++ b/drivers/gpu/drm/arm/malidp_regs.h
>@@ -205,6 +205,7 @@
> #define MALIDP500_SE_BASE		0x00c00
> #define MALIDP500_SE_CONTROL		0x00c0c
> #define MALIDP500_SE_MEMWRITE_OUT_SIZE	0x00c2c
>+#define MALIDP500_SE_RGB_YUV_COEFFS	0x00C74
> #define MALIDP500_SE_MEMWRITE_BASE	0x00e00
> #define MALIDP500_DC_IRQ_BASE		0x00f00
> #define MALIDP500_CONFIG_VALID		0x00f00
>@@ -238,6 +239,7 @@
> #define MALIDP550_SE_CONTROL		0x08010
> #define   MALIDP550_SE_MEMWRITE_ONESHOT	(1 << 7)
> #define MALIDP550_SE_MEMWRITE_OUT_SIZE	0x08030
>+#define MALIDP550_SE_RGB_YUV_COEFFS	0x08078
> #define MALIDP550_SE_MEMWRITE_BASE	0x08100
> #define MALIDP550_DC_BASE		0x0c000
> #define MALIDP550_DC_CONTROL		0x0c010
>-- 
>2.18.0
>


More information about the dri-devel mailing list