[PATCH 5/5] drm/stm: ltdc: add support of ycbcr pixel formats

[PATCH 5/5] drm/stm: ltdc: add support of ycbcr pixel formats

[Yannick Fertre]

This patch adds the following YCbCr input pixel formats on the latest
LTDC hardware version:

1 plane  (co-planar)  : YUYV, YVYU, UYVY, VYUY
2 planes (semi-planar): NV12, NV21
3 planes (full-planar): YU12=I420=DRM YUV420, YV12=DRM YVU420

Signed-off-by: Yannick Fertre <yannick.fertre@foss.st.com>
---
 drivers/gpu/drm/stm/ltdc.c | 251 +++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/stm/ltdc.h |   1 +
 2 files changed, 245 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 4d249bc99894..7fd173390b9f 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -198,6 +198,21 @@
 
 #define LXCFBLNR_CFBLN	GENMASK(10, 0)	/* Color Frame Buffer Line Number */
 
+#define LXCR_C1R_YIA	BIT(0)		/* Ycbcr 422 Interleaved Ability */
+#define LXCR_C1R_YSPA	BIT(1)		/* Ycbcr 420 Semi-Planar Ability */
+#define LXCR_C1R_YFPA	BIT(2)		/* Ycbcr 420 Full-Planar Ability */
+#define LXCR_C1R_SCA	BIT(31)		/* SCaling Ability*/
+
+#define LxPCR_YREN	BIT(9)		/* Y Rescale Enable for the color dynamic range */
+#define LxPCR_OF	BIT(8)		/* Odd pixel First */
+#define LxPCR_CBF	BIT(7)		/* CB component First */
+#define LxPCR_YF	BIT(6)		/* Y component First */
+#define LxPCR_YCM	GENMASK(5, 4)	/* Ycbcr Conversion Mode */
+#define YCM_I		0x0		/* Interleaved 422 */
+#define YCM_SP		0x1		/* Semi-Planar 420 */
+#define YCM_FP		0x2		/* Full-Planar 420 */
+#define LxPCR_YCEN	BIT(3)		/* YCbCr-to-RGB Conversion Enable */
+
 #define LXRCR_IMR	BIT(0)		/* IMmediate Reload */
 #define LXRCR_VBR	BIT(1)		/* Vertical Blanking Reload */
 #define LXRCR_GRMSK	BIT(2)		/* Global (centralized) Reload MaSKed */
@@ -311,6 +326,23 @@ static const u32 ltdc_drm_fmt_a2[] = {
 	DRM_FORMAT_C8
 };
 
+static const u32 ltdc_drm_fmt_ycbcr_cp[] = {
+	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVYU,
+	DRM_FORMAT_UYVY,
+	DRM_FORMAT_VYUY
+};
+
+static const u32 ltdc_drm_fmt_ycbcr_sp[] = {
+	DRM_FORMAT_NV12,
+	DRM_FORMAT_NV21
+};
+
+static const u32 ltdc_drm_fmt_ycbcr_fp[] = {
+	DRM_FORMAT_YUV420,
+	DRM_FORMAT_YVU420
+};
+
 /* Layer register offsets */
 static const u32 ltdc_layer_regs_a0[] = {
 	0x80,	/* L1 configuration 0 */
@@ -410,6 +442,26 @@ static const struct regmap_config stm32_ltdc_regmap_cfg = {
 	.cache_type = REGCACHE_NONE,
 };
 
+static const u32 ltdc_ycbcr2rgb_coeffs[DRM_COLOR_ENCODING_MAX][DRM_COLOR_RANGE_MAX][2] = {
+	[DRM_COLOR_YCBCR_BT601][DRM_COLOR_YCBCR_LIMITED_RANGE] = {
+		0x02040199,	/* (b_cb = 516 / r_cr = 409) */
+		0x006400D0	/* (g_cb = 100 / g_cr = 208) */
+	},
+	[DRM_COLOR_YCBCR_BT601][DRM_COLOR_YCBCR_FULL_RANGE] = {
+		0x01C60167,	/* (b_cb = 454 / r_cr = 359) */
+		0x005800B7	/* (g_cb = 88 / g_cr = 183) */
+	},
+	[DRM_COLOR_YCBCR_BT709][DRM_COLOR_YCBCR_LIMITED_RANGE] = {
+		0x021D01CB,	/* (b_cb = 541 / r_cr = 459) */
+		0x00370089	/* (g_cb = 55 / g_cr = 137) */
+	},
+	[DRM_COLOR_YCBCR_BT709][DRM_COLOR_YCBCR_FULL_RANGE] = {
+		0x01DB0193,	/* (b_cb = 475 / r_cr = 403) */
+		0x00300078	/* (g_cb = 48 / g_cr = 120) */
+	}
+	/* BT2020 not supported */
+};
+
 static inline struct ltdc_device *crtc_to_ltdc(struct drm_crtc *crtc)
 {
 	return (struct ltdc_device *)crtc->dev->dev_private;
@@ -540,6 +592,78 @@ static inline u32 is_xrgb(u32 drm)
 	return ((drm & 0xFF) == 'X' || ((drm >> 8) & 0xFF) == 'X');
 }
 
+static inline void ltdc_set_ycbcr_config(struct drm_plane *plane, u32 drm_pix_fmt)
+{
+	struct ltdc_device *ldev = plane_to_ltdc(plane);
+	struct drm_plane_state *state = plane->state;
+	u32 lofs = plane->index * LAY_OFS;
+	u32 val;
+
+	switch (drm_pix_fmt) {
+	case DRM_FORMAT_YUYV:
+		val = (YCM_I << 4) | LxPCR_YF | LxPCR_CBF;
+		break;
+	case DRM_FORMAT_YVYU:
+		val = (YCM_I << 4) | LxPCR_YF;
+		break;
+	case DRM_FORMAT_UYVY:
+		val = (YCM_I << 4) | LxPCR_CBF;
+		break;
+	case DRM_FORMAT_VYUY:
+		val = (YCM_I << 4);
+		break;
+	case DRM_FORMAT_NV12:
+		val = (YCM_SP << 4) | LxPCR_CBF;
+		break;
+	case DRM_FORMAT_NV21:
+		val = (YCM_SP << 4);
+		break;
+	case DRM_FORMAT_YUV420:
+	case DRM_FORMAT_YVU420:
+		val = (YCM_FP << 4);
+		break;
+	default:
+		/* RGB or not a YCbCr supported format */
+		break;
+	}
+
+	/* Enable limited range */
+	if (state->color_range == DRM_COLOR_YCBCR_LIMITED_RANGE)
+		val |= LxPCR_YREN;
+
+	/* enable ycbcr conversion */
+	val |= LxPCR_YCEN;
+
+	regmap_write(ldev->regmap, LTDC_L1PCR + lofs, val);
+}
+
+static inline void ltdc_set_ycbcr_coeffs(struct drm_plane *plane)
+{
+	struct ltdc_device *ldev = plane_to_ltdc(plane);
+	struct drm_plane_state *state = plane->state;
+	enum drm_color_encoding enc = state->color_encoding;
+	enum drm_color_range ran = state->color_range;
+	u32 lofs = plane->index * LAY_OFS;
+
+	if (enc != DRM_COLOR_YCBCR_BT601 && enc != DRM_COLOR_YCBCR_BT709) {
+		DRM_ERROR("color encoding %d not supported, use bt601 by default\n", enc);
+		/* set by default color encoding to DRM_COLOR_YCBCR_BT601 */
+		enc = DRM_COLOR_YCBCR_BT601;
+	}
+
+	if (ran != DRM_COLOR_YCBCR_LIMITED_RANGE && ran != DRM_COLOR_YCBCR_FULL_RANGE) {
+		DRM_ERROR("color range %d not supported, use limited range by default\n", ran);
+		/* set by default color range to DRM_COLOR_YCBCR_LIMITED_RANGE */
+		ran = DRM_COLOR_YCBCR_LIMITED_RANGE;
+	}
+
+	DRM_DEBUG_DRIVER("Color encoding=%d, range=%d\n", enc, ran);
+	regmap_write(ldev->regmap, LTDC_L1CYR0R + lofs,
+		     ltdc_ycbcr2rgb_coeffs[enc][ran][0]);
+	regmap_write(ldev->regmap, LTDC_L1CYR1R + lofs,
+		     ltdc_ycbcr2rgb_coeffs[enc][ran][1]);
+}
+
 static irqreturn_t ltdc_irq_thread(int irq, void *arg)
 {
 	struct drm_device *ddev = arg;
@@ -1010,7 +1134,7 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
 	u32 y0 = newstate->crtc_y;
 	u32 y1 = newstate->crtc_y + newstate->crtc_h - 1;
 	u32 src_x, src_y, src_w, src_h;
-	u32 val, pitch_in_bytes, line_length, paddr, ahbp, avbp, bpcr;
+	u32 val, pitch_in_bytes, line_length, line_number, paddr, ahbp, avbp, bpcr;
 	enum ltdc_pix_fmt pf;
 
 	if (!newstate->crtc || !fb) {
@@ -1086,8 +1210,8 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
 	regmap_write_bits(ldev->regmap, LTDC_L1BFCR + lofs, LXBFCR_BF2 | LXBFCR_BF1, val);
 
 	/* Configures the frame buffer line number */
-	val = y1 - y0 + 1;
-	regmap_write_bits(ldev->regmap, LTDC_L1CFBLNR + lofs, LXCFBLNR_CFBLN, val);
+	line_number = y1 - y0 + 1;
+	regmap_write_bits(ldev->regmap, LTDC_L1CFBLNR + lofs, LXCFBLNR_CFBLN, line_number);
 
 	/* Sets the FB address */
 	paddr = (u32)drm_fb_cma_get_gem_addr(fb, newstate, 0);
@@ -1095,6 +1219,77 @@ static void ltdc_plane_atomic_update(struct drm_plane *plane,
 	DRM_DEBUG_DRIVER("fb: phys 0x%08x", paddr);
 	regmap_write(ldev->regmap, LTDC_L1CFBAR + lofs, paddr);
 
+	if (ldev->caps.ycbcr_input) {
+		if (fb->format->is_yuv) {
+			switch (fb->format->format) {
+			case DRM_FORMAT_NV12:
+			case DRM_FORMAT_NV21:
+			/* Configure the auxiliary frame buffer address 0 & 1 */
+			paddr = (u32)drm_fb_cma_get_gem_addr(fb, newstate, 1);
+			regmap_write(ldev->regmap, LTDC_L1AFBA0R + lofs, paddr);
+			regmap_write(ldev->regmap, LTDC_L1AFBA1R + lofs, paddr + 1);
+
+			/* Configure the buffer length */
+			val = ((pitch_in_bytes << 16) | line_length);
+			regmap_write(ldev->regmap, LTDC_L1AFBLR + lofs, val);
+
+			/* Configure the frame buffer line number */
+			val = (line_number >> 1);
+			regmap_write(ldev->regmap, LTDC_L1AFBLNR + lofs, val);
+			break;
+			case DRM_FORMAT_YUV420:
+			/* Configure the auxiliary frame buffer address 0 */
+			paddr = (u32)drm_fb_cma_get_gem_addr(fb, newstate, 1);
+			regmap_write(ldev->regmap, LTDC_L1AFBA0R + lofs, paddr);
+
+			/* Configure the auxiliary frame buffer address 1 */
+			paddr = (u32)drm_fb_cma_get_gem_addr(fb, newstate, 2);
+			regmap_write(ldev->regmap, LTDC_L1AFBA1R + lofs, paddr);
+
+			line_length = ((fb->format->cpp[0] * (x1 - x0 + 1)) >> 1) +
+				      (ldev->caps.bus_width >> 3) - 1;
+
+			/* Configure the buffer length */
+			val = (((pitch_in_bytes >> 1) << 16) | line_length);
+			regmap_write(ldev->regmap, LTDC_L1AFBLR + lofs, val);
+
+			/* Configure the frame buffer line number */
+			val = (line_number >> 1);
+			regmap_write(ldev->regmap, LTDC_L1AFBLNR + lofs, val);
+			break;
+			case DRM_FORMAT_YVU420:
+			/* Configure the auxiliary frame buffer address 0 */
+			paddr = (u32)drm_fb_cma_get_gem_addr(fb, newstate, 2);
+			regmap_write(ldev->regmap, LTDC_L1AFBA0R + lofs, paddr);
+
+			/* Configure the auxiliary frame buffer address 1 */
+			paddr = (u32)drm_fb_cma_get_gem_addr(fb, newstate, 1);
+			regmap_write(ldev->regmap, LTDC_L1AFBA1R + lofs, paddr);
+
+			line_length = ((fb->format->cpp[0] * (x1 - x0 + 1)) >> 1) +
+				      (ldev->caps.bus_width >> 3) - 1;
+
+			/* Configure the buffer length */
+			val = (((pitch_in_bytes >> 1) << 16) | line_length);
+			regmap_write(ldev->regmap, LTDC_L1AFBLR + lofs, val);
+
+			/* Configure the frame buffer line number */
+			val = (line_number >> 1);
+			regmap_write(ldev->regmap, LTDC_L1AFBLNR + lofs, val);
+			break;
+			}
+
+			/* Configure YCbC conversion coefficient */
+			ltdc_set_ycbcr_coeffs(plane);
+
+			/* Configure YCbCr format and enable/disable conversion */
+			ltdc_set_ycbcr_config(plane, fb->format->format);
+		} else {
+			/* disable ycbcr conversion */
+			regmap_write(ldev->regmap, LTDC_L1PCR + lofs, 0);
+		}
+	}
+
 	/* Enable layer and CLUT if needed */
 	val = fb->format->format == DRM_FORMAT_C8 ? LXCR_CLUTEN : 0;
 	val |= LXCR_LEN;
@@ -1186,7 +1381,8 @@ static const struct drm_plane_helper_funcs ltdc_plane_helper_funcs = {
 };
 
 static struct drm_plane *ltdc_plane_create(struct drm_device *ddev,
-					   enum drm_plane_type type)
+					   enum drm_plane_type type,
+					   int index)
 {
 	unsigned long possible_crtcs = CRTC_MASK;
 	struct ltdc_device *ldev = ddev->dev_private;
@@ -1196,9 +1392,16 @@ static struct drm_plane *ltdc_plane_create(struct drm_device *ddev,
 	u32 *formats;
 	u32 drm_fmt;
 	const u64 *modifiers = ltdc_format_modifiers;
+	u32 lofs = index * LAY_OFS;
+	u32 val;
 	int ret;
 
-	formats = devm_kzalloc(dev, ldev->caps.pix_fmt_nb * sizeof(*formats), GFP_KERNEL);
+	/* Allocate the biggest size according to supported color formats */
+	formats = devm_kzalloc(dev, (ldev->caps.pix_fmt_nb +
+			       ARRAY_SIZE(ltdc_drm_fmt_ycbcr_cp) +
+			       ARRAY_SIZE(ltdc_drm_fmt_ycbcr_sp) +
+			       ARRAY_SIZE(ltdc_drm_fmt_ycbcr_fp)) *
+			       sizeof(*formats), GFP_KERNEL);
 
 	for (i = 0; i < ldev->caps.pix_fmt_nb; i++) {
 		drm_fmt = ldev->caps.pix_fmt_drm[i];
@@ -1212,6 +1415,26 @@ static struct drm_plane *ltdc_plane_create(struct drm_device *ddev,
 		formats[nb_fmt++] = drm_fmt;
 	}
 
+	/* Add YCbCr supported pixel formats */
+	if (ldev->caps.ycbcr_input) {
+		regmap_read(ldev->regmap, LTDC_L1C1R + lofs, &val);
+		if (val & LXCR_C1R_YIA) {
+			memcpy(&formats[nb_fmt], ltdc_drm_fmt_ycbcr_cp,
+			       ARRAY_SIZE(ltdc_drm_fmt_ycbcr_cp) * sizeof(*formats));
+			nb_fmt += ARRAY_SIZE(ltdc_drm_fmt_ycbcr_cp);
+		}
+		if (val & LXCR_C1R_YSPA) {
+			memcpy(&formats[nb_fmt], ltdc_drm_fmt_ycbcr_sp,
+			       ARRAY_SIZE(ltdc_drm_fmt_ycbcr_sp) * sizeof(*formats));
+			nb_fmt += ARRAY_SIZE(ltdc_drm_fmt_ycbcr_sp);
+		}
+		if (val & LXCR_C1R_YFPA) {
+			memcpy(&formats[nb_fmt], ltdc_drm_fmt_ycbcr_fp,
+			       ARRAY_SIZE(ltdc_drm_fmt_ycbcr_fp) * sizeof(*formats));
+			nb_fmt += ARRAY_SIZE(ltdc_drm_fmt_ycbcr_fp);
+		}
+	}
+
 	plane = devm_kzalloc(dev, sizeof(*plane), GFP_KERNEL);
 	if (!plane)
 		return NULL;
@@ -1222,6 +1445,17 @@ static struct drm_plane *ltdc_plane_create(struct drm_device *ddev,
 	if (ret < 0)
 		return NULL;
 
+	if (ldev->caps.ycbcr_input) {
+		if (val & (LXCR_C1R_YIA | LXCR_C1R_YSPA | LXCR_C1R_YFPA))
+			drm_plane_create_color_properties(plane,
+							  BIT(DRM_COLOR_YCBCR_BT601) |
+							  BIT(DRM_COLOR_YCBCR_BT709),
+							  BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
+							  BIT(DRM_COLOR_YCBCR_FULL_RANGE),
+							  DRM_COLOR_YCBCR_BT601,
+							  DRM_COLOR_YCBCR_LIMITED_RANGE);
+	}
+
 	drm_plane_helper_add(plane, &ltdc_plane_helper_funcs);
 
 	drm_plane_create_alpha_property(plane);
@@ -1247,7 +1481,7 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc)
 	unsigned int i;
 	int ret;
 
-	primary = ltdc_plane_create(ddev, DRM_PLANE_TYPE_PRIMARY);
+	primary = ltdc_plane_create(ddev, DRM_PLANE_TYPE_PRIMARY, 0);
 	if (!primary) {
 		DRM_ERROR("Can not create primary plane\n");
 		return -EINVAL;
@@ -1271,7 +1505,7 @@ static int ltdc_crtc_init(struct drm_device *ddev, struct drm_crtc *crtc)
 
 	/* Add planes. Note : the first layer is used by primary plane */
 	for (i = 1; i < ldev->caps.nb_layers; i++) {
-		overlay = ltdc_plane_create(ddev, DRM_PLANE_TYPE_OVERLAY);
+		overlay = ltdc_plane_create(ddev, DRM_PLANE_TYPE_OVERLAY, i);
 		if (!overlay) {
 			ret = -ENOMEM;
 			DRM_ERROR("Can not create overlay plane %d\n", i);
@@ -1403,6 +1637,7 @@ static int ltdc_get_caps(struct drm_device *ddev)
 		if (ldev->caps.hw_version == HWVER_10200)
 			ldev->caps.pad_max_freq_hz = 65000000;
 		ldev->caps.nb_irq = 2;
+		ldev->caps.ycbcr_input = false;
 		ldev->caps.ycbcr_output = false;
 		ldev->caps.plane_reg_shadow = false;
 		break;
@@ -1416,6 +1651,7 @@ static int ltdc_get_caps(struct drm_device *ddev)
 		ldev->caps.non_alpha_only_l1 = false;
 		ldev->caps.pad_max_freq_hz = 150000000;
 		ldev->caps.nb_irq = 4;
+		ldev->caps.ycbcr_input = false;
 		ldev->caps.ycbcr_output = false;
 		ldev->caps.plane_reg_shadow = false;
 		break;
@@ -1429,6 +1665,7 @@ static int ltdc_get_caps(struct drm_device *ddev)
 		ldev->caps.non_alpha_only_l1 = false;
 		ldev->caps.pad_max_freq_hz = 90000000;
 		ldev->caps.nb_irq = 2;
+		ldev->caps.ycbcr_input = true;
 		ldev->caps.ycbcr_output = true;
 		ldev->caps.plane_reg_shadow = true;
 		break;
diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
index adc4f9cf7f95..6968d1ca5149 100644
--- a/drivers/gpu/drm/stm/ltdc.h
+++ b/drivers/gpu/drm/stm/ltdc.h
@@ -24,6 +24,7 @@ struct ltdc_caps {
 	bool non_alpha_only_l1; /* non-native no-alpha formats on layer 1 */
 	int pad_max_freq_hz;	/* max frequency supported by pad */
 	int nb_irq;		/* number of hardware interrupts */
+	bool ycbcr_input;	/* ycbcr input converter supported */
 	bool ycbcr_output;	/* ycbcr output converter supported */
 	bool plane_reg_shadow;	/* plane shadow registers ability */
 };
-- 
2.17.1

Re: [PATCH 5/5] drm/stm: ltdc: add support of ycbcr pixel formats

[Philippe CORNU]

On 12/15/21 10:48 PM, Yannick Fertre wrote:
> This patch adds the following YCbCr input pixel formats on the latest
> LTDC hardware version:
> 
> 1 plane  (co-planar)  : YUYV, YVYU, UYVY, VYUY
> 2 planes (semi-planar): NV12, NV21
> 3 planes (full-planar): YU12=I420=DRM YUV420, YV12=DRM YVU420
> 
> Signed-off-by: Yannick Fertre <yannick.fertre@foss.st.com>
> ---
>   drivers/gpu/drm/stm/ltdc.c | 251 +++++++++++++++++++++++++++++++++++--
>   drivers/gpu/drm/stm/ltdc.h |   1 +
>   2 files changed, 245 insertions(+), 7 deletions(-)
> 

Hi Yannick,
many thanks for your patch.
Nice hw features!
Acked-by: Philippe Cornu <philippe.cornu@foss.st.com>
Reviewed-by: Philippe Cornu <philippe.cornu@foss.st.com>
Philippe :-)

Re: [PATCH 5/5] drm/stm: ltdc: add support of ycbcr pixel formats

[Nathan Chancellor]

Hi Yannick,

On Wed, Dec 15, 2021 at 10:48:43PM +0100, Yannick Fertre wrote:
> This patch adds the following YCbCr input pixel formats on the latest
> LTDC hardware version:
> 
> 1 plane  (co-planar)  : YUYV, YVYU, UYVY, VYUY
> 2 planes (semi-planar): NV12, NV21
> 3 planes (full-planar): YU12=I420=DRM YUV420, YV12=DRM YVU420
> 
> Signed-off-by: Yannick Fertre <yannick.fertre@foss.st.com>

<snip>

> +static inline void ltdc_set_ycbcr_config(struct drm_plane *plane, u32 drm_pix_fmt)
> +{
> +	struct ltdc_device *ldev = plane_to_ltdc(plane);
> +	struct drm_plane_state *state = plane->state;
> +	u32 lofs = plane->index * LAY_OFS;
> +	u32 val;
> +
> +	switch (drm_pix_fmt) {
> +	case DRM_FORMAT_YUYV:
> +		val = (YCM_I << 4) | LxPCR_YF | LxPCR_CBF;
> +		break;
> +	case DRM_FORMAT_YVYU:
> +		val = (YCM_I << 4) | LxPCR_YF;
> +		break;
> +	case DRM_FORMAT_UYVY:
> +		val = (YCM_I << 4) | LxPCR_CBF;
> +		break;
> +	case DRM_FORMAT_VYUY:
> +		val = (YCM_I << 4);
> +		break;
> +	case DRM_FORMAT_NV12:
> +		val = (YCM_SP << 4) | LxPCR_CBF;
> +		break;
> +	case DRM_FORMAT_NV21:
> +		val = (YCM_SP << 4);
> +		break;
> +	case DRM_FORMAT_YUV420:
> +	case DRM_FORMAT_YVU420:
> +		val = (YCM_FP << 4);
> +		break;
> +	default:
> +		/* RGB or not a YCbCr supported format */
> +		break;
> +	}
> +
> +	/* Enable limited range */
> +	if (state->color_range == DRM_COLOR_YCBCR_LIMITED_RANGE)
> +		val |= LxPCR_YREN;
> +
> +	/* enable ycbcr conversion */
> +	val |= LxPCR_YCEN;
> +
> +	regmap_write(ldev->regmap, LTDC_L1PCR + lofs, val);
> +}

This patch as commit 484e72d3146b ("drm/stm: ltdc: add support of ycbcr
pixel formats") in -next introduced the following clang warning:

drivers/gpu/drm/stm/ltdc.c:625:2: warning: variable 'val' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
        default:
        ^~~~~~~
drivers/gpu/drm/stm/ltdc.c:635:2: note: uninitialized use occurs here
        val |= LxPCR_YCEN;
        ^~~
drivers/gpu/drm/stm/ltdc.c:600:9: note: initialize the variable 'val' to silence this warning
        u32 val;
               ^
                = 0
1 warning generated.

Would it be okay to just return in the default case (maybe with a
message about an unsupported format?) or should there be another fix?

Cheers,

Re: [PATCH 5/5] drm/stm: ltdc: add support of ycbcr pixel formats

[yannick Fertre]

Hi Nathan,

On 2/2/22 17:54, Nathan Chancellor wrote:
> Hi Yannick,
> 
> On Wed, Dec 15, 2021 at 10:48:43PM +0100, Yannick Fertre wrote:
>> This patch adds the following YCbCr input pixel formats on the latest
>> LTDC hardware version:
>>
>> 1 plane  (co-planar)  : YUYV, YVYU, UYVY, VYUY
>> 2 planes (semi-planar): NV12, NV21
>> 3 planes (full-planar): YU12=I420=DRM YUV420, YV12=DRM YVU420
>>
>> Signed-off-by: Yannick Fertre <yannick.fertre@foss.st.com>
> 
> <snip>
> 
>> +static inline void ltdc_set_ycbcr_config(struct drm_plane *plane, u32 drm_pix_fmt)
>> +{
>> +	struct ltdc_device *ldev = plane_to_ltdc(plane);
>> +	struct drm_plane_state *state = plane->state;
>> +	u32 lofs = plane->index * LAY_OFS;
>> +	u32 val;
>> +
>> +	switch (drm_pix_fmt) {
>> +	case DRM_FORMAT_YUYV:
>> +		val = (YCM_I << 4) | LxPCR_YF | LxPCR_CBF;
>> +		break;
>> +	case DRM_FORMAT_YVYU:
>> +		val = (YCM_I << 4) | LxPCR_YF;
>> +		break;
>> +	case DRM_FORMAT_UYVY:
>> +		val = (YCM_I << 4) | LxPCR_CBF;
>> +		break;
>> +	case DRM_FORMAT_VYUY:
>> +		val = (YCM_I << 4);
>> +		break;
>> +	case DRM_FORMAT_NV12:
>> +		val = (YCM_SP << 4) | LxPCR_CBF;
>> +		break;
>> +	case DRM_FORMAT_NV21:
>> +		val = (YCM_SP << 4);
>> +		break;
>> +	case DRM_FORMAT_YUV420:
>> +	case DRM_FORMAT_YVU420:
>> +		val = (YCM_FP << 4);
>> +		break;
>> +	default:
>> +		/* RGB or not a YCbCr supported format */
>> +		break;
>> +	}
>> +
>> +	/* Enable limited range */
>> +	if (state->color_range == DRM_COLOR_YCBCR_LIMITED_RANGE)
>> +		val |= LxPCR_YREN;
>> +
>> +	/* enable ycbcr conversion */
>> +	val |= LxPCR_YCEN;
>> +
>> +	regmap_write(ldev->regmap, LTDC_L1PCR + lofs, val);
>> +}
> 
> This patch as commit 484e72d3146b ("drm/stm: ltdc: add support of ycbcr
> pixel formats") in -next introduced the following clang warning:
> 
> drivers/gpu/drm/stm/ltdc.c:625:2: warning: variable 'val' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
>          default:
>          ^~~~~~~
> drivers/gpu/drm/stm/ltdc.c:635:2: note: uninitialized use occurs here
>          val |= LxPCR_YCEN;
>          ^~~
> drivers/gpu/drm/stm/ltdc.c:600:9: note: initialize the variable 'val' to silence this warning
>          u32 val;
>                 ^
>                  = 0
> 1 warning generated.
> 
> Would it be okay to just return in the default case (maybe with a
> message about an unsupported format?) or should there be another fix?
> 
> Cheers,


Thanks for your help.
It'okay for a message for unsupported format with a return in the 
default case.
Do you want create & push the patch?

Best regards

Re: [PATCH 5/5] drm/stm: ltdc: add support of ycbcr pixel formats

[Nathan Chancellor]

On Mon, Feb 07, 2022 at 11:00:34AM +0100, yannick Fertre wrote:
> Hi Nathan,
> 
> On 2/2/22 17:54, Nathan Chancellor wrote:
> > Hi Yannick,
> > 
> > On Wed, Dec 15, 2021 at 10:48:43PM +0100, Yannick Fertre wrote:
> > > This patch adds the following YCbCr input pixel formats on the latest
> > > LTDC hardware version:
> > > 
> > > 1 plane  (co-planar)  : YUYV, YVYU, UYVY, VYUY
> > > 2 planes (semi-planar): NV12, NV21
> > > 3 planes (full-planar): YU12=I420=DRM YUV420, YV12=DRM YVU420
> > > 
> > > Signed-off-by: Yannick Fertre <yannick.fertre@foss.st.com>
> > 
> > <snip>
> > 
> > > +static inline void ltdc_set_ycbcr_config(struct drm_plane *plane, u32 drm_pix_fmt)
> > > +{
> > > +	struct ltdc_device *ldev = plane_to_ltdc(plane);
> > > +	struct drm_plane_state *state = plane->state;
> > > +	u32 lofs = plane->index * LAY_OFS;
> > > +	u32 val;
> > > +
> > > +	switch (drm_pix_fmt) {
> > > +	case DRM_FORMAT_YUYV:
> > > +		val = (YCM_I << 4) | LxPCR_YF | LxPCR_CBF;
> > > +		break;
> > > +	case DRM_FORMAT_YVYU:
> > > +		val = (YCM_I << 4) | LxPCR_YF;
> > > +		break;
> > > +	case DRM_FORMAT_UYVY:
> > > +		val = (YCM_I << 4) | LxPCR_CBF;
> > > +		break;
> > > +	case DRM_FORMAT_VYUY:
> > > +		val = (YCM_I << 4);
> > > +		break;
> > > +	case DRM_FORMAT_NV12:
> > > +		val = (YCM_SP << 4) | LxPCR_CBF;
> > > +		break;
> > > +	case DRM_FORMAT_NV21:
> > > +		val = (YCM_SP << 4);
> > > +		break;
> > > +	case DRM_FORMAT_YUV420:
> > > +	case DRM_FORMAT_YVU420:
> > > +		val = (YCM_FP << 4);
> > > +		break;
> > > +	default:
> > > +		/* RGB or not a YCbCr supported format */
> > > +		break;
> > > +	}
> > > +
> > > +	/* Enable limited range */
> > > +	if (state->color_range == DRM_COLOR_YCBCR_LIMITED_RANGE)
> > > +		val |= LxPCR_YREN;
> > > +
> > > +	/* enable ycbcr conversion */
> > > +	val |= LxPCR_YCEN;
> > > +
> > > +	regmap_write(ldev->regmap, LTDC_L1PCR + lofs, val);
> > > +}
> > 
> > This patch as commit 484e72d3146b ("drm/stm: ltdc: add support of ycbcr
> > pixel formats") in -next introduced the following clang warning:
> > 
> > drivers/gpu/drm/stm/ltdc.c:625:2: warning: variable 'val' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
> >          default:
> >          ^~~~~~~
> > drivers/gpu/drm/stm/ltdc.c:635:2: note: uninitialized use occurs here
> >          val |= LxPCR_YCEN;
> >          ^~~
> > drivers/gpu/drm/stm/ltdc.c:600:9: note: initialize the variable 'val' to silence this warning
> >          u32 val;
> >                 ^
> >                  = 0
> > 1 warning generated.
> > 
> > Would it be okay to just return in the default case (maybe with a
> > message about an unsupported format?) or should there be another fix?
> > 
> > Cheers,
> 
> 
> Thanks for your help.
> It'okay for a message for unsupported format with a return in the default
> case.
> Do you want create & push the patch?

Thank you for the input! I have sent a fix now, please take a look.

https://lore.kernel.org/r/20220207165304.1046867-1-nathan@kernel.org/

Cheers,
Nathan