* [PATCH v3 0/4] drm: lcdif: Improve YUV support
@ 2022-09-29 20:42 Laurent Pinchart
2022-09-29 20:42 ` [PATCH v3 1/4] drm: lcdif: Fix indentation in lcdif_regs.h Laurent Pinchart
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Laurent Pinchart @ 2022-09-29 20:42 UTC (permalink / raw)
To: dri-devel
Cc: Marek Vasut, Peng Fan, Alexander Stein, Kieran Bingham, Liu Ying,
Daniel Scally, Robby Cai
Hello,
This small patch series improves YUV support in the i.MX8MP LCDIF
driver. After patches 1/4 and 2/4 that fix tiny cosmetic issues, patch
3/4 fixes YUV quantization range for the RGB to YUV conversion. Patch
4/4 addresses the other direction and adds support for YUV planes.
Please see individual patches for details.
Compared to v2, review comments have been taken into account, and the
YUV to RGB coefficients in patch 4/4 have been fixed (they were
blatantly wrong due to a stupid mistake).
The series has been tested on a Polyhex Debix board with the currently
out-of-tree HDMI encoder support patches developed by Lucas Stach, with
the kmstest and the libcamera 'cam' applications.
There is a know issue with the way the driver programs the format and
pitch, which produces incorrect output when testing YUV formats with the
legacy (non-atomic) KMS API, in particular with the modetest
application. The framebuffer is accessed from the plane state in
function called from the .atomic_enable() handler, which in some
circumstances results in the format and/or pitch of the old frame buffer
being used. This issue preexists, and can be triggered by selecting a
different RGB format with modetest (XR15 for instance). It should be
fixed separately, and I wouldn't consider it as a blocker for this
series as YUV formats can already be used correctly when using the KMS
atomic API.
Kieran Bingham (1):
drm: lcdif: Add support for YUV planes
Laurent Pinchart (3):
drm: lcdif: Fix indentation in lcdif_regs.h
drm: lcdif: Don't use BIT() for multi-bit register fields
drm: lcdif: Switch to limited range for RGB to YUV conversion
drivers/gpu/drm/mxsfb/lcdif_kms.c | 232 ++++++++++++++++++++++++++---
drivers/gpu/drm/mxsfb/lcdif_regs.h | 37 ++---
2 files changed, 229 insertions(+), 40 deletions(-)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v3 1/4] drm: lcdif: Fix indentation in lcdif_regs.h 2022-09-29 20:42 [PATCH v3 0/4] drm: lcdif: Improve YUV support Laurent Pinchart @ 2022-09-29 20:42 ` Laurent Pinchart 2022-09-29 20:42 ` [PATCH v3 2/4] drm: lcdif: Don't use BIT() for multi-bit register fields Laurent Pinchart ` (3 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Laurent Pinchart @ 2022-09-29 20:42 UTC (permalink / raw) To: dri-devel Cc: Marek Vasut, Peng Fan, Alexander Stein, Kieran Bingham, Liu Ying, Daniel Scally, Robby Cai A couple of the register macro values are incorrectly indented. Fix them. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Marek Vasut <marex@denx.de> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Liu Ying <victor.liu@nxp.com> --- drivers/gpu/drm/mxsfb/lcdif_regs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/lcdif_regs.h b/drivers/gpu/drm/mxsfb/lcdif_regs.h index 8e8bef175bf2..013f2cace2a0 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_regs.h +++ b/drivers/gpu/drm/mxsfb/lcdif_regs.h @@ -130,7 +130,7 @@ #define CTRL_FETCH_START_OPTION_BPV BIT(9) #define CTRL_FETCH_START_OPTION_RESV GENMASK(9, 8) #define CTRL_FETCH_START_OPTION_MASK GENMASK(9, 8) -#define CTRL_NEG BIT(4) +#define CTRL_NEG BIT(4) #define CTRL_INV_PXCK BIT(3) #define CTRL_INV_DE BIT(2) #define CTRL_INV_VS BIT(1) @@ -186,7 +186,7 @@ #define INT_ENABLE_D1_PLANE_PANIC_EN BIT(0) #define CTRLDESCL0_1_HEIGHT(n) (((n) & 0xffff) << 16) -#define CTRLDESCL0_1_HEIGHT_MASK GENMASK(31, 16) +#define CTRLDESCL0_1_HEIGHT_MASK GENMASK(31, 16) #define CTRLDESCL0_1_WIDTH(n) ((n) & 0xffff) #define CTRLDESCL0_1_WIDTH_MASK GENMASK(15, 0) -- Regards, Laurent Pinchart ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/4] drm: lcdif: Don't use BIT() for multi-bit register fields 2022-09-29 20:42 [PATCH v3 0/4] drm: lcdif: Improve YUV support Laurent Pinchart 2022-09-29 20:42 ` [PATCH v3 1/4] drm: lcdif: Fix indentation in lcdif_regs.h Laurent Pinchart @ 2022-09-29 20:42 ` Laurent Pinchart 2022-09-29 20:42 ` [PATCH v3 3/4] drm: lcdif: Switch to limited range for RGB to YUV conversion Laurent Pinchart ` (2 subsequent siblings) 4 siblings, 0 replies; 8+ messages in thread From: Laurent Pinchart @ 2022-09-29 20:42 UTC (permalink / raw) To: dri-devel Cc: Marek Vasut, Peng Fan, Alexander Stein, Kieran Bingham, Liu Ying, Daniel Scally, Robby Cai The BIT() macro is meant to represent a single bit. Don't use it for values of register fields that span multiple bits. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Marek Vasut <marex@denx.de> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Liu Ying <victor.liu@nxp.com> --- Changes since v1: - Use hex for field values --- drivers/gpu/drm/mxsfb/lcdif_regs.h | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/lcdif_regs.h b/drivers/gpu/drm/mxsfb/lcdif_regs.h index 013f2cace2a0..0d5d9bedd94a 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_regs.h +++ b/drivers/gpu/drm/mxsfb/lcdif_regs.h @@ -138,9 +138,9 @@ #define DISP_PARA_DISP_ON BIT(31) #define DISP_PARA_SWAP_EN BIT(30) -#define DISP_PARA_LINE_PATTERN_UYVY_H (GENMASK(29, 28) | BIT(26)) -#define DISP_PARA_LINE_PATTERN_RGB565 GENMASK(28, 26) -#define DISP_PARA_LINE_PATTERN_RGB888 0 +#define DISP_PARA_LINE_PATTERN_UYVY_H (0xd << 26) +#define DISP_PARA_LINE_PATTERN_RGB565 (0x7 << 26) +#define DISP_PARA_LINE_PATTERN_RGB888 (0x0 << 26) #define DISP_PARA_LINE_PATTERN_MASK GENMASK(29, 26) #define DISP_PARA_DISP_MODE_MASK GENMASK(25, 24) #define DISP_PARA_BGND_R_MASK GENMASK(23, 16) @@ -202,18 +202,18 @@ #define CTRLDESCL0_5_EN BIT(31) #define CTRLDESCL0_5_SHADOW_LOAD_EN BIT(30) -#define CTRLDESCL0_5_BPP_16_RGB565 BIT(26) -#define CTRLDESCL0_5_BPP_16_ARGB1555 (BIT(26) | BIT(24)) -#define CTRLDESCL0_5_BPP_16_ARGB4444 (BIT(26) | BIT(25)) -#define CTRLDESCL0_5_BPP_YCbCr422 (BIT(26) | BIT(25) | BIT(24)) -#define CTRLDESCL0_5_BPP_24_RGB888 BIT(27) -#define CTRLDESCL0_5_BPP_32_ARGB8888 (BIT(27) | BIT(24)) -#define CTRLDESCL0_5_BPP_32_ABGR8888 (BIT(27) | BIT(25)) +#define CTRLDESCL0_5_BPP_16_RGB565 (0x4 << 24) +#define CTRLDESCL0_5_BPP_16_ARGB1555 (0x5 << 24) +#define CTRLDESCL0_5_BPP_16_ARGB4444 (0x6 << 24) +#define CTRLDESCL0_5_BPP_YCbCr422 (0x7 << 24) +#define CTRLDESCL0_5_BPP_24_RGB888 (0x8 << 24) +#define CTRLDESCL0_5_BPP_32_ARGB8888 (0x9 << 24) +#define CTRLDESCL0_5_BPP_32_ABGR8888 (0xa << 24) #define CTRLDESCL0_5_BPP_MASK GENMASK(27, 24) -#define CTRLDESCL0_5_YUV_FORMAT_Y2VY1U 0 -#define CTRLDESCL0_5_YUV_FORMAT_Y2UY1V BIT(14) -#define CTRLDESCL0_5_YUV_FORMAT_VY2UY1 BIT(15) -#define CTRLDESCL0_5_YUV_FORMAT_UY2VY1 (BIT(15) | BIT(14)) +#define CTRLDESCL0_5_YUV_FORMAT_Y2VY1U (0x0 << 14) +#define CTRLDESCL0_5_YUV_FORMAT_Y2UY1V (0x1 << 14) +#define CTRLDESCL0_5_YUV_FORMAT_VY2UY1 (0x2 << 14) +#define CTRLDESCL0_5_YUV_FORMAT_UY2VY1 (0x3 << 14) #define CTRLDESCL0_5_YUV_FORMAT_MASK GENMASK(15, 14) #define CSC0_CTRL_CSC_MODE_RGB2YCbCr GENMASK(2, 1) -- Regards, Laurent Pinchart ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 3/4] drm: lcdif: Switch to limited range for RGB to YUV conversion 2022-09-29 20:42 [PATCH v3 0/4] drm: lcdif: Improve YUV support Laurent Pinchart 2022-09-29 20:42 ` [PATCH v3 1/4] drm: lcdif: Fix indentation in lcdif_regs.h Laurent Pinchart 2022-09-29 20:42 ` [PATCH v3 2/4] drm: lcdif: Don't use BIT() for multi-bit register fields Laurent Pinchart @ 2022-09-29 20:42 ` Laurent Pinchart 2022-09-29 20:42 ` [PATCH v3 4/4] drm: lcdif: Add support for YUV planes Laurent Pinchart 2022-10-16 3:10 ` [PATCH v3 0/4] drm: lcdif: Improve YUV support Marek Vasut 4 siblings, 0 replies; 8+ messages in thread From: Laurent Pinchart @ 2022-09-29 20:42 UTC (permalink / raw) To: dri-devel Cc: Marek Vasut, Peng Fan, Alexander Stein, Kieran Bingham, Liu Ying, Daniel Scally, Robby Cai Up to and including v1.3, HDMI supported limited quantization range only for YCbCr. HDMI v1.4 introduced selectable quantization ranges, but this feature isn't supported in the dw-hdmi driver that is used in conjunction with the LCDIF in the i.MX8MP. The HDMI YCbCr output is thus always advertised in the AVI infoframe as limited range. The LCDIF driver, on the other hand, configures the CSC to produce full range YCbCr. This mismatch results in loss of details and incorrect colours. Fix it by switching to limited range YCbCr. The coefficients are copied from drivers/media/platforms/nxp/imx-pxp.c for coherency, as the hardware is most likely identical. Fixes: 9db35bb349a0 ("drm: lcdif: Add support for i.MX8MP LCDIF variant") Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Marek Vasut <marex@denx.de> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Liu Ying <victor.liu@nxp.com> --- Changes since v2: - List floating point coefficient values in comment - Fix typo in the commit message Changes since v1: - Use coefficients from imx-pxp.c --- drivers/gpu/drm/mxsfb/lcdif_kms.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c index 1f22ea5896d5..08e4880ec6cf 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c @@ -53,16 +53,22 @@ static void lcdif_set_formats(struct lcdif_drm_private *lcdif, writel(DISP_PARA_LINE_PATTERN_UYVY_H, lcdif->base + LCDC_V8_DISP_PARA); - /* CSC: BT.601 Full Range RGB to YCbCr coefficients. */ - writel(CSC0_COEF0_A2(0x096) | CSC0_COEF0_A1(0x04c), + /* + * CSC: BT.601 Limited Range RGB to YCbCr coefficients. + * + * |Y | | 0.2568 0.5041 0.0979| |R| |16 | + * |Cb| = |-0.1482 -0.2910 0.4392| * |G| + |128| + * |Cr| | 0.4392 0.4392 -0.3678| |B| |128| + */ + writel(CSC0_COEF0_A2(0x081) | CSC0_COEF0_A1(0x041), lcdif->base + LCDC_V8_CSC0_COEF0); - writel(CSC0_COEF1_B1(0x7d5) | CSC0_COEF1_A3(0x01d), + writel(CSC0_COEF1_B1(0x7db) | CSC0_COEF1_A3(0x019), lcdif->base + LCDC_V8_CSC0_COEF1); - writel(CSC0_COEF2_B3(0x080) | CSC0_COEF2_B2(0x7ac), + writel(CSC0_COEF2_B3(0x070) | CSC0_COEF2_B2(0x7b6), lcdif->base + LCDC_V8_CSC0_COEF2); - writel(CSC0_COEF3_C2(0x795) | CSC0_COEF3_C1(0x080), + writel(CSC0_COEF3_C2(0x7a2) | CSC0_COEF3_C1(0x070), lcdif->base + LCDC_V8_CSC0_COEF3); - writel(CSC0_COEF4_D1(0x000) | CSC0_COEF4_C3(0x7ec), + writel(CSC0_COEF4_D1(0x010) | CSC0_COEF4_C3(0x7ee), lcdif->base + LCDC_V8_CSC0_COEF4); writel(CSC0_COEF5_D3(0x080) | CSC0_COEF5_D2(0x080), lcdif->base + LCDC_V8_CSC0_COEF5); -- Regards, Laurent Pinchart ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 4/4] drm: lcdif: Add support for YUV planes 2022-09-29 20:42 [PATCH v3 0/4] drm: lcdif: Improve YUV support Laurent Pinchart ` (2 preceding siblings ...) 2022-09-29 20:42 ` [PATCH v3 3/4] drm: lcdif: Switch to limited range for RGB to YUV conversion Laurent Pinchart @ 2022-09-29 20:42 ` Laurent Pinchart 2022-09-30 4:01 ` Liu Ying 2022-10-16 3:10 ` [PATCH v3 0/4] drm: lcdif: Improve YUV support Marek Vasut 4 siblings, 1 reply; 8+ messages in thread From: Laurent Pinchart @ 2022-09-29 20:42 UTC (permalink / raw) To: dri-devel Cc: Marek Vasut, Peng Fan, Alexander Stein, Kieran Bingham, Liu Ying, Daniel Scally, Robby Cai From: Kieran Bingham <kieran.bingham@ideasonboard.com> The LCDIF includes a color space converter that supports YUV input. Use it to support YUV planes, either through the converter if the output format is RGB, or in conversion bypass mode otherwise. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Marek Vasut <marex@denx.de> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- Changes since v2: - Fix YUV to RGB coefficients - List floating point coefficient values in comment - Express CSC coefficients with three hex digits - Move | to the end of the line - Add comment header before RGB formats Changes since v1: - Support all YCbCr encodings and quantization ranges - Drop incorrect comment --- drivers/gpu/drm/mxsfb/lcdif_kms.c | 260 ++++++++++++++++++++++++----- drivers/gpu/drm/mxsfb/lcdif_regs.h | 5 +- 2 files changed, 224 insertions(+), 41 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c index 08e4880ec6cf..81454b53936f 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c @@ -15,6 +15,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_bridge.h> +#include <drm/drm_color_mgmt.h> #include <drm/drm_crtc.h> #include <drm/drm_encoder.h> #include <drm/drm_framebuffer.h> @@ -32,13 +33,119 @@ /* ----------------------------------------------------------------------------- * CRTC */ + +/* + * Despite the reference manual stating the opposite, the D1, D2 and D3 offset + * values are added to Y, U and V, not subtracted. They must thus be programmed + * with negative values. + */ +static const u32 lcdif_yuv2rgb_coeffs[3][2][6] = { + [DRM_COLOR_YCBCR_BT601] = { + [DRM_COLOR_YCBCR_LIMITED_RANGE] = { + /* + * BT.601 limited range: + * + * |R| |1.1644 0.0000 1.5960| |Y - 16 | + * |G| = |1.1644 -0.3917 -0.8129| * |Cb - 128| + * |B| |1.1644 2.0172 0.0000| |Cr - 128| + */ + CSC0_COEF0_A1(0x12a) | CSC0_COEF0_A2(0x000), + CSC0_COEF1_A3(0x199) | CSC0_COEF1_B1(0x12a), + CSC0_COEF2_B2(0x79c) | CSC0_COEF2_B3(0x730), + CSC0_COEF3_C1(0x12a) | CSC0_COEF3_C2(0x204), + CSC0_COEF4_C3(0x000) | CSC0_COEF4_D1(0x1f0), + CSC0_COEF5_D2(0x180) | CSC0_COEF5_D3(0x180), + }, + [DRM_COLOR_YCBCR_FULL_RANGE] = { + /* + * BT.601 full range: + * + * |R| |1.0000 0.0000 1.4020| |Y - 0 | + * |G| = |1.0000 -0.3441 -0.7141| * |Cb - 128| + * |B| |1.0000 1.7720 0.0000| |Cr - 128| + */ + CSC0_COEF0_A1(0x100) | CSC0_COEF0_A2(0x000), + CSC0_COEF1_A3(0x167) | CSC0_COEF1_B1(0x100), + CSC0_COEF2_B2(0x7a8) | CSC0_COEF2_B3(0x749), + CSC0_COEF3_C1(0x100) | CSC0_COEF3_C2(0x1c6), + CSC0_COEF4_C3(0x000) | CSC0_COEF4_D1(0x000), + CSC0_COEF5_D2(0x180) | CSC0_COEF5_D3(0x180), + }, + }, + [DRM_COLOR_YCBCR_BT709] = { + [DRM_COLOR_YCBCR_LIMITED_RANGE] = { + /* + * Rec.709 limited range: + * + * |R| |1.1644 0.0000 1.7927| |Y - 16 | + * |G| = |1.1644 -0.2132 -0.5329| * |Cb - 128| + * |B| |1.1644 2.1124 0.0000| |Cr - 128| + */ + CSC0_COEF0_A1(0x12a) | CSC0_COEF0_A2(0x000), + CSC0_COEF1_A3(0x1cb) | CSC0_COEF1_B1(0x12a), + CSC0_COEF2_B2(0x7c9) | CSC0_COEF2_B3(0x778), + CSC0_COEF3_C1(0x12a) | CSC0_COEF3_C2(0x21d), + CSC0_COEF4_C3(0x000) | CSC0_COEF4_D1(0x1f0), + CSC0_COEF5_D2(0x180) | CSC0_COEF5_D3(0x180), + }, + [DRM_COLOR_YCBCR_FULL_RANGE] = { + /* + * Rec.709 full range: + * + * |R| |1.0000 0.0000 1.5748| |Y - 0 | + * |G| = |1.0000 -0.1873 -0.4681| * |Cb - 128| + * |B| |1.0000 1.8556 0.0000| |Cr - 128| + */ + CSC0_COEF0_A1(0x100) | CSC0_COEF0_A2(0x000), + CSC0_COEF1_A3(0x193) | CSC0_COEF1_B1(0x100), + CSC0_COEF2_B2(0x7d0) | CSC0_COEF2_B3(0x788), + CSC0_COEF3_C1(0x100) | CSC0_COEF3_C2(0x1db), + CSC0_COEF4_C3(0x000) | CSC0_COEF4_D1(0x000), + CSC0_COEF5_D2(0x180) | CSC0_COEF5_D3(0x180), + }, + }, + [DRM_COLOR_YCBCR_BT2020] = { + [DRM_COLOR_YCBCR_LIMITED_RANGE] = { + /* + * BT.2020 limited range: + * + * |R| |1.1644 0.0000 1.6787| |Y - 16 | + * |G| = |1.1644 -0.1874 -0.6505| * |Cb - 128| + * |B| |1.1644 2.1418 0.0000| |Cr - 128| + */ + CSC0_COEF0_A1(0x12a) | CSC0_COEF0_A2(0x000), + CSC0_COEF1_A3(0x1ae) | CSC0_COEF1_B1(0x12a), + CSC0_COEF2_B2(0x7d0) | CSC0_COEF2_B3(0x759), + CSC0_COEF3_C1(0x12a) | CSC0_COEF3_C2(0x224), + CSC0_COEF4_C3(0x000) | CSC0_COEF4_D1(0x1f0), + CSC0_COEF5_D2(0x180) | CSC0_COEF5_D3(0x180), + }, + [DRM_COLOR_YCBCR_FULL_RANGE] = { + /* + * BT.2020 full range: + * + * |R| |1.0000 0.0000 1.4746| |Y - 0 | + * |G| = |1.0000 -0.1646 -0.5714| * |Cb - 128| + * |B| |1.0000 1.8814 0.0000| |Cr - 128| + */ + CSC0_COEF0_A1(0x100) | CSC0_COEF0_A2(0x000), + CSC0_COEF1_A3(0x179) | CSC0_COEF1_B1(0x100), + CSC0_COEF2_B2(0x7d6) | CSC0_COEF2_B3(0x76e), + CSC0_COEF3_C1(0x100) | CSC0_COEF3_C2(0x1e2), + CSC0_COEF4_C3(0x000) | CSC0_COEF4_D1(0x000), + CSC0_COEF5_D2(0x180) | CSC0_COEF5_D3(0x180), + }, + }, +}; + static void lcdif_set_formats(struct lcdif_drm_private *lcdif, + struct drm_plane_state *plane_state, const u32 bus_format) { struct drm_device *drm = lcdif->drm; - const u32 format = lcdif->crtc.primary->state->fb->format->format; - - writel(CSC0_CTRL_BYPASS, lcdif->base + LCDC_V8_CSC0_CTRL); + const u32 format = plane_state->fb->format->format; + bool in_yuv = false; + bool out_yuv = false; switch (bus_format) { case MEDIA_BUS_FMT_RGB565_1X16: @@ -52,6 +159,78 @@ static void lcdif_set_formats(struct lcdif_drm_private *lcdif, case MEDIA_BUS_FMT_UYVY8_1X16: writel(DISP_PARA_LINE_PATTERN_UYVY_H, lcdif->base + LCDC_V8_DISP_PARA); + out_yuv = true; + break; + default: + dev_err(drm->dev, "Unknown media bus format 0x%x\n", bus_format); + break; + } + + switch (format) { + /* RGB Formats */ + case DRM_FORMAT_RGB565: + writel(CTRLDESCL0_5_BPP_16_RGB565, + lcdif->base + LCDC_V8_CTRLDESCL0_5); + break; + case DRM_FORMAT_RGB888: + writel(CTRLDESCL0_5_BPP_24_RGB888, + lcdif->base + LCDC_V8_CTRLDESCL0_5); + break; + case DRM_FORMAT_XRGB1555: + writel(CTRLDESCL0_5_BPP_16_ARGB1555, + lcdif->base + LCDC_V8_CTRLDESCL0_5); + break; + case DRM_FORMAT_XRGB4444: + writel(CTRLDESCL0_5_BPP_16_ARGB4444, + lcdif->base + LCDC_V8_CTRLDESCL0_5); + break; + case DRM_FORMAT_XBGR8888: + writel(CTRLDESCL0_5_BPP_32_ABGR8888, + lcdif->base + LCDC_V8_CTRLDESCL0_5); + break; + case DRM_FORMAT_XRGB8888: + writel(CTRLDESCL0_5_BPP_32_ARGB8888, + lcdif->base + LCDC_V8_CTRLDESCL0_5); + break; + + /* YUYV Formats */ + case DRM_FORMAT_YUYV: + writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_VY2UY1, + lcdif->base + LCDC_V8_CTRLDESCL0_5); + in_yuv = true; + break; + case DRM_FORMAT_YVYU: + writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_UY2VY1, + lcdif->base + LCDC_V8_CTRLDESCL0_5); + in_yuv = true; + break; + case DRM_FORMAT_UYVY: + writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_Y2VY1U, + lcdif->base + LCDC_V8_CTRLDESCL0_5); + in_yuv = true; + break; + case DRM_FORMAT_VYUY: + writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_Y2UY1V, + lcdif->base + LCDC_V8_CTRLDESCL0_5); + in_yuv = true; + break; + + default: + dev_err(drm->dev, "Unknown pixel format 0x%x\n", format); + break; + } + + /* + * The CSC differentiates between "YCbCr" and "YUV", but the reference + * manual doesn't detail how they differ. Experiments showed that the + * luminance value is unaffected, only the calculations involving chroma + * values differ. The YCbCr mode behaves as expected, with chroma values + * being offset by 128. The YUV mode isn't fully understood. + */ + if (!in_yuv && out_yuv) { + /* RGB -> YCbCr */ + writel(CSC0_CTRL_CSC_MODE_RGB2YCbCr, + lcdif->base + LCDC_V8_CSC0_CTRL); /* * CSC: BT.601 Limited Range RGB to YCbCr coefficients. @@ -72,44 +251,24 @@ static void lcdif_set_formats(struct lcdif_drm_private *lcdif, lcdif->base + LCDC_V8_CSC0_COEF4); writel(CSC0_COEF5_D3(0x080) | CSC0_COEF5_D2(0x080), lcdif->base + LCDC_V8_CSC0_COEF5); + } else if (in_yuv && !out_yuv) { + /* YCbCr -> RGB */ + const u32 *coeffs = + lcdif_yuv2rgb_coeffs[plane_state->color_encoding] + [plane_state->color_range]; - writel(CSC0_CTRL_CSC_MODE_RGB2YCbCr, + writel(CSC0_CTRL_CSC_MODE_YCbCr2RGB, lcdif->base + LCDC_V8_CSC0_CTRL); - break; - default: - dev_err(drm->dev, "Unknown media bus format 0x%x\n", bus_format); - break; - } - - switch (format) { - case DRM_FORMAT_RGB565: - writel(CTRLDESCL0_5_BPP_16_RGB565, - lcdif->base + LCDC_V8_CTRLDESCL0_5); - break; - case DRM_FORMAT_RGB888: - writel(CTRLDESCL0_5_BPP_24_RGB888, - lcdif->base + LCDC_V8_CTRLDESCL0_5); - break; - case DRM_FORMAT_XRGB1555: - writel(CTRLDESCL0_5_BPP_16_ARGB1555, - lcdif->base + LCDC_V8_CTRLDESCL0_5); - break; - case DRM_FORMAT_XRGB4444: - writel(CTRLDESCL0_5_BPP_16_ARGB4444, - lcdif->base + LCDC_V8_CTRLDESCL0_5); - break; - case DRM_FORMAT_XBGR8888: - writel(CTRLDESCL0_5_BPP_32_ABGR8888, - lcdif->base + LCDC_V8_CTRLDESCL0_5); - break; - case DRM_FORMAT_XRGB8888: - writel(CTRLDESCL0_5_BPP_32_ARGB8888, - lcdif->base + LCDC_V8_CTRLDESCL0_5); - break; - default: - dev_err(drm->dev, "Unknown pixel format 0x%x\n", format); - break; + writel(coeffs[0], lcdif->base + LCDC_V8_CSC0_COEF0); + writel(coeffs[1], lcdif->base + LCDC_V8_CSC0_COEF1); + writel(coeffs[2], lcdif->base + LCDC_V8_CSC0_COEF2); + writel(coeffs[3], lcdif->base + LCDC_V8_CSC0_COEF3); + writel(coeffs[4], lcdif->base + LCDC_V8_CSC0_COEF4); + writel(coeffs[5], lcdif->base + LCDC_V8_CSC0_COEF5); + } else { + /* RGB -> RGB, YCbCr -> YCbCr: bypass colorspace converter. */ + writel(CSC0_CTRL_BYPASS, lcdif->base + LCDC_V8_CSC0_CTRL); } } @@ -207,6 +366,7 @@ static void lcdif_reset_block(struct lcdif_drm_private *lcdif) } static void lcdif_crtc_mode_set_nofb(struct lcdif_drm_private *lcdif, + struct drm_plane_state *plane_state, struct drm_bridge_state *bridge_state, const u32 bus_format) { @@ -229,7 +389,7 @@ static void lcdif_crtc_mode_set_nofb(struct lcdif_drm_private *lcdif, /* Mandatory eLCDIF reset as per the Reference Manual */ lcdif_reset_block(lcdif); - lcdif_set_formats(lcdif, bus_format); + lcdif_set_formats(lcdif, plane_state, bus_format); lcdif_set_mode(lcdif, bus_flags); } @@ -312,7 +472,7 @@ static void lcdif_crtc_atomic_enable(struct drm_crtc *crtc, pm_runtime_get_sync(drm->dev); - lcdif_crtc_mode_set_nofb(lcdif, bridge_state, bus_format); + lcdif_crtc_mode_set_nofb(lcdif, new_pstate, bridge_state, bus_format); /* Write cur_buf as well to avoid an initial corrupt frame */ paddr = drm_fb_cma_get_gem_addr(new_pstate->fb, new_pstate, 0); @@ -456,12 +616,19 @@ static const struct drm_plane_funcs lcdif_plane_funcs = { }; static const u32 lcdif_primary_plane_formats[] = { + /* RGB */ DRM_FORMAT_RGB565, DRM_FORMAT_RGB888, DRM_FORMAT_XBGR8888, DRM_FORMAT_XRGB1555, DRM_FORMAT_XRGB4444, DRM_FORMAT_XRGB8888, + + /* Packed YCbCr */ + DRM_FORMAT_YUYV, + DRM_FORMAT_YVYU, + DRM_FORMAT_UYVY, + DRM_FORMAT_VYUY, }; static const u64 lcdif_modifiers[] = { @@ -475,6 +642,11 @@ static const u64 lcdif_modifiers[] = { int lcdif_kms_init(struct lcdif_drm_private *lcdif) { + const u32 supported_encodings = BIT(DRM_COLOR_YCBCR_BT601) | + BIT(DRM_COLOR_YCBCR_BT709) | + BIT(DRM_COLOR_YCBCR_BT2020); + const u32 supported_ranges = BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) | + BIT(DRM_COLOR_YCBCR_FULL_RANGE); struct drm_encoder *encoder = &lcdif->encoder; struct drm_crtc *crtc = &lcdif->crtc; int ret; @@ -490,6 +662,14 @@ int lcdif_kms_init(struct lcdif_drm_private *lcdif) if (ret) return ret; + ret = drm_plane_create_color_properties(&lcdif->planes.primary, + supported_encodings, + supported_ranges, + DRM_COLOR_YCBCR_BT601, + DRM_COLOR_YCBCR_LIMITED_RANGE); + if (ret) + return ret; + drm_crtc_helper_add(crtc, &lcdif_crtc_helper_funcs); ret = drm_crtc_init_with_planes(lcdif->drm, crtc, &lcdif->planes.primary, NULL, diff --git a/drivers/gpu/drm/mxsfb/lcdif_regs.h b/drivers/gpu/drm/mxsfb/lcdif_regs.h index 0d5d9bedd94a..fb74eb5ccbf1 100644 --- a/drivers/gpu/drm/mxsfb/lcdif_regs.h +++ b/drivers/gpu/drm/mxsfb/lcdif_regs.h @@ -216,7 +216,10 @@ #define CTRLDESCL0_5_YUV_FORMAT_UY2VY1 (0x3 << 14) #define CTRLDESCL0_5_YUV_FORMAT_MASK GENMASK(15, 14) -#define CSC0_CTRL_CSC_MODE_RGB2YCbCr GENMASK(2, 1) +#define CSC0_CTRL_CSC_MODE_YUV2RGB (0x0 << 1) +#define CSC0_CTRL_CSC_MODE_YCbCr2RGB (0x1 << 1) +#define CSC0_CTRL_CSC_MODE_RGB2YUV (0x2 << 1) +#define CSC0_CTRL_CSC_MODE_RGB2YCbCr (0x3 << 1) #define CSC0_CTRL_CSC_MODE_MASK GENMASK(2, 1) #define CSC0_CTRL_BYPASS BIT(0) -- Regards, Laurent Pinchart ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 4/4] drm: lcdif: Add support for YUV planes 2022-09-29 20:42 ` [PATCH v3 4/4] drm: lcdif: Add support for YUV planes Laurent Pinchart @ 2022-09-30 4:01 ` Liu Ying 2022-09-30 8:05 ` Laurent Pinchart 0 siblings, 1 reply; 8+ messages in thread From: Liu Ying @ 2022-09-30 4:01 UTC (permalink / raw) To: Laurent Pinchart, dri-devel Cc: Marek Vasut, Peng Fan, Alexander Stein, Kieran Bingham, Daniel Scally, Robby Cai Hi Laurent, On Thu, 2022-09-29 at 23:42 +0300, Laurent Pinchart wrote: > From: Kieran Bingham <kieran.bingham@ideasonboard.com> > > The LCDIF includes a color space converter that supports YUV input. Use > it to support YUV planes, either through the converter if the output > format is RGB, or in conversion bypass mode otherwise. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Marek Vasut <marex@denx.de> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > Changes since v2: > > - Fix YUV to RGB coefficients > - List floating point coefficient values in comment > - Express CSC coefficients with three hex digits > - Move | to the end of the line > - Add comment header before RGB formats > > Changes since v1: > > - Support all YCbCr encodings and quantization ranges > - Drop incorrect comment > --- > drivers/gpu/drm/mxsfb/lcdif_kms.c | 260 ++++++++++++++++++++++++----- > drivers/gpu/drm/mxsfb/lcdif_regs.h | 5 +- > 2 files changed, 224 insertions(+), 41 deletions(-) Hmmm, 'git am' fails to apply this one to drm-misc-next but it's ok with v6.0-rc1. I should have tried drm-misc-next with v2. > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c > index 08e4880ec6cf..81454b53936f 100644 > --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c > +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c > @@ -15,6 +15,7 @@ > #include <drm/drm_atomic.h> > #include <drm/drm_atomic_helper.h> > #include <drm/drm_bridge.h> > +#include <drm/drm_color_mgmt.h> > #include <drm/drm_crtc.h> > #include <drm/drm_encoder.h> > #include <drm/drm_framebuffer.h> > @@ -32,13 +33,119 @@ > /* ----------------------------------------------------------------------------- > * CRTC > */ > + > +/* > + * Despite the reference manual stating the opposite, the D1, D2 and D3 offset > + * values are added to Y, U and V, not subtracted. They must thus be programmed > + * with negative values. Can you add the generic conversion equations with [A-D][1-3] coefficient symbols as comments in code, like imx-pxp.c does? It will improve code readability better and tell which coefficient is which. Also for RGB to YCbCr conversion if you want to do that with this series. Sorry for not pointing this out in v2 review cycle. > + */ > +static const u32 lcdif_yuv2rgb_coeffs[3][2][6] = { [...] > + /* YUYV Formats */ 'YUV Formats' would be better? Again, failed to catch this in v2... Regards, Liu Ying > + case DRM_FORMAT_YUYV: > + writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_VY2UY1, > + lcdif->base + LCDC_V8_CTRLDESCL0_5); > + in_yuv = true; > + break; > + case DRM_FORMAT_YVYU: > + writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_UY2VY1, > + lcdif->base + LCDC_V8_CTRLDESCL0_5); > + in_yuv = true; > + break; > + case DRM_FORMAT_UYVY: > + writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_Y2VY1U, > + lcdif->base + LCDC_V8_CTRLDESCL0_5); > + in_yuv = true; > + break; > + case DRM_FORMAT_VYUY: > + writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_Y2UY1V, > + lcdif->base + LCDC_V8_CTRLDESCL0_5); > + in_yuv = true; > + break; > + > + default: > + dev_err(drm->dev, "Unknown pixel format 0x%x\n", format); > + break; > + } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 4/4] drm: lcdif: Add support for YUV planes 2022-09-30 4:01 ` Liu Ying @ 2022-09-30 8:05 ` Laurent Pinchart 0 siblings, 0 replies; 8+ messages in thread From: Laurent Pinchart @ 2022-09-30 8:05 UTC (permalink / raw) To: Liu Ying Cc: Marek Vasut, Peng Fan, Alexander Stein, Kieran Bingham, dri-devel, Daniel Scally, Robby Cai Hi Liu, On Fri, Sep 30, 2022 at 12:01:39PM +0800, Liu Ying wrote: > On Thu, 2022-09-29 at 23:42 +0300, Laurent Pinchart wrote: > > From: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > The LCDIF includes a color space converter that supports YUV input. Use > > it to support YUV planes, either through the converter if the output > > format is RGB, or in conversion bypass mode otherwise. > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Marek Vasut <marex@denx.de> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > > Changes since v2: > > > > - Fix YUV to RGB coefficients > > - List floating point coefficient values in comment > > - Express CSC coefficients with three hex digits > > - Move | to the end of the line > > - Add comment header before RGB formats > > > > Changes since v1: > > > > - Support all YCbCr encodings and quantization ranges > > - Drop incorrect comment > > --- > > drivers/gpu/drm/mxsfb/lcdif_kms.c | 260 ++++++++++++++++++++++++----- > > drivers/gpu/drm/mxsfb/lcdif_regs.h | 5 +- > > 2 files changed, 224 insertions(+), 41 deletions(-) > > Hmmm, 'git am' fails to apply this one to drm-misc-next but it's ok > with v6.0-rc1. I should have tried drm-misc-next with v2. Oops. I'll rebase on top of drm-misc-next. > > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c b/drivers/gpu/drm/mxsfb/lcdif_kms.c > > index 08e4880ec6cf..81454b53936f 100644 > > --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c > > +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c > > @@ -15,6 +15,7 @@ > > #include <drm/drm_atomic.h> > > #include <drm/drm_atomic_helper.h> > > #include <drm/drm_bridge.h> > > +#include <drm/drm_color_mgmt.h> > > #include <drm/drm_crtc.h> > > #include <drm/drm_encoder.h> > > #include <drm/drm_framebuffer.h> > > @@ -32,13 +33,119 @@ > > /* ----------------------------------------------------------------------------- > > * CRTC > > */ > > + > > +/* > > + * Despite the reference manual stating the opposite, the D1, D2 and D3 offset > > + * values are added to Y, U and V, not subtracted. They must thus be programmed > > + * with negative values. > > Can you add the generic conversion equations with [A-D][1-3] > coefficient symbols as comments in code, like imx-pxp.c does? It will > improve code readability better and tell which coefficient is which. > Also for RGB to YCbCr conversion if you want to do that with this > series. Sorry for not pointing this out in v2 review cycle. I'll fix that. > > + */ > > +static const u32 lcdif_yuv2rgb_coeffs[3][2][6] = { > > [...] > > > + /* YUYV Formats */ > > 'YUV Formats' would be better? Again, failed to catch this in v2... Will fix this too. > > + case DRM_FORMAT_YUYV: > > + writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_VY2UY1, > > + lcdif->base + LCDC_V8_CTRLDESCL0_5); > > + in_yuv = true; > > + break; > > + case DRM_FORMAT_YVYU: > > + writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_UY2VY1, > > + lcdif->base + LCDC_V8_CTRLDESCL0_5); > > + in_yuv = true; > > + break; > > + case DRM_FORMAT_UYVY: > > + writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_Y2VY1U, > > + lcdif->base + LCDC_V8_CTRLDESCL0_5); > > + in_yuv = true; > > + break; > > + case DRM_FORMAT_VYUY: > > + writel(CTRLDESCL0_5_BPP_YCbCr422 | CTRLDESCL0_5_YUV_FORMAT_Y2UY1V, > > + lcdif->base + LCDC_V8_CTRLDESCL0_5); > > + in_yuv = true; > > + break; > > + > > + default: > > + dev_err(drm->dev, "Unknown pixel format 0x%x\n", format); > > + break; > > + } -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/4] drm: lcdif: Improve YUV support 2022-09-29 20:42 [PATCH v3 0/4] drm: lcdif: Improve YUV support Laurent Pinchart ` (3 preceding siblings ...) 2022-09-29 20:42 ` [PATCH v3 4/4] drm: lcdif: Add support for YUV planes Laurent Pinchart @ 2022-10-16 3:10 ` Marek Vasut 4 siblings, 0 replies; 8+ messages in thread From: Marek Vasut @ 2022-10-16 3:10 UTC (permalink / raw) To: Laurent Pinchart, dri-devel Cc: Peng Fan, Alexander Stein, Kieran Bingham, Liu Ying, Daniel Scally, Robby Cai On 9/29/22 22:42, Laurent Pinchart wrote: > Hello, > > This small patch series improves YUV support in the i.MX8MP LCDIF > driver. After patches 1/4 and 2/4 that fix tiny cosmetic issues, patch > 3/4 fixes YUV quantization range for the RGB to YUV conversion. Patch > 4/4 addresses the other direction and adds support for YUV planes. > > Please see individual patches for details. > > Compared to v2, review comments have been taken into account, and the > YUV to RGB coefficients in patch 4/4 have been fixed (they were > blatantly wrong due to a stupid mistake). > > The series has been tested on a Polyhex Debix board with the currently > out-of-tree HDMI encoder support patches developed by Lucas Stach, with > the kmstest and the libcamera 'cam' applications. > > There is a know issue with the way the driver programs the format and > pitch, which produces incorrect output when testing YUV formats with the > legacy (non-atomic) KMS API, in particular with the modetest > application. The framebuffer is accessed from the plane state in > function called from the .atomic_enable() handler, which in some > circumstances results in the format and/or pitch of the old frame buffer > being used. This issue preexists, and can be triggered by selecting a > different RGB format with modetest (XR15 for instance). It should be > fixed separately, and I wouldn't consider it as a blocker for this > series as YUV formats can already be used correctly when using the KMS > atomic API. > > Kieran Bingham (1): > drm: lcdif: Add support for YUV planes > > Laurent Pinchart (3): > drm: lcdif: Fix indentation in lcdif_regs.h > drm: lcdif: Don't use BIT() for multi-bit register fields > drm: lcdif: Switch to limited range for RGB to YUV conversion > > drivers/gpu/drm/mxsfb/lcdif_kms.c | 232 ++++++++++++++++++++++++++--- > drivers/gpu/drm/mxsfb/lcdif_regs.h | 37 ++--- > 2 files changed, 229 insertions(+), 40 deletions(-) Applied all, thanks ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-10-16 3:10 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-29 20:42 [PATCH v3 0/4] drm: lcdif: Improve YUV support Laurent Pinchart 2022-09-29 20:42 ` [PATCH v3 1/4] drm: lcdif: Fix indentation in lcdif_regs.h Laurent Pinchart 2022-09-29 20:42 ` [PATCH v3 2/4] drm: lcdif: Don't use BIT() for multi-bit register fields Laurent Pinchart 2022-09-29 20:42 ` [PATCH v3 3/4] drm: lcdif: Switch to limited range for RGB to YUV conversion Laurent Pinchart 2022-09-29 20:42 ` [PATCH v3 4/4] drm: lcdif: Add support for YUV planes Laurent Pinchart 2022-09-30 4:01 ` Liu Ying 2022-09-30 8:05 ` Laurent Pinchart 2022-10-16 3:10 ` [PATCH v3 0/4] drm: lcdif: Improve YUV support Marek Vasut
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.