Linux-Media Archive on lore.kernel.org
 help / Atom feed
* [PATCH 0/3] media: imx: Add support for BT.709 encoding
@ 2019-02-03 19:47 Steve Longerbeam
  2019-02-03 19:47 ` [PATCH 1/3] gpu: ipu-v3: ipu-ic: Rename yuv2rgb encoding matrices Steve Longerbeam
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Steve Longerbeam @ 2019-02-03 19:47 UTC (permalink / raw)
  To: linux-media; +Cc: Tim Harvey, Philipp Zabel, Steve Longerbeam

This patchset adds support for the BT.709 encoding and inverse encoding
matrices to the ipu_ic task init functions. The imx-media driver can
now support both BT.601 and BT.709 encoding.

Steve Longerbeam (3):
  gpu: ipu-v3: ipu-ic: Rename yuv2rgb encoding matrices
  gpu: ipu-v3: ipu-ic: Add support for BT.709 encoding
  media: imx: Allow BT.709 encoding for IC routes

 drivers/gpu/ipu-v3/ipu-ic.c                 | 75 ++++++++++++++++++---
 drivers/gpu/ipu-v3/ipu-image-convert.c      |  1 +
 drivers/staging/media/imx/imx-ic-prpencvf.c |  4 +-
 drivers/staging/media/imx/imx-media-utils.c |  4 +-
 include/video/imx-ipu-v3.h                  |  5 +-
 5 files changed, 76 insertions(+), 13 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] gpu: ipu-v3: ipu-ic: Rename yuv2rgb encoding matrices
  2019-02-03 19:47 [PATCH 0/3] media: imx: Add support for BT.709 encoding Steve Longerbeam
@ 2019-02-03 19:47 ` Steve Longerbeam
  2019-02-03 19:47 ` [PATCH 2/3] gpu: ipu-v3: ipu-ic: Add support for BT.709 encoding Steve Longerbeam
  2019-02-03 19:47 ` [PATCH 3/3] media: imx: Allow BT.709 encoding for IC routes Steve Longerbeam
  2 siblings, 0 replies; 7+ messages in thread
From: Steve Longerbeam @ 2019-02-03 19:47 UTC (permalink / raw)
  To: linux-media
  Cc: Tim Harvey, Philipp Zabel, Steve Longerbeam, dri-devel, linux-kernel

The ycbcr2rgb and inverse rgb2ycbcr matrices define the BT.601 encoding
coefficients, so rename them to indicate that. And add some comments
to make clear these are BT.601 coefficients encoding between YUV limited
range and RGB full range. No functional changes.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/gpu/ipu-v3/ipu-ic.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/ipu-v3/ipu-ic.c b/drivers/gpu/ipu-v3/ipu-ic.c
index 594c3cbc8291..35ae86ff0585 100644
--- a/drivers/gpu/ipu-v3/ipu-ic.c
+++ b/drivers/gpu/ipu-v3/ipu-ic.c
@@ -183,11 +183,13 @@ struct ic_csc_params {
 };
 
 /*
+ * BT.601 encoding from RGB full range to YUV limited range:
+ *
  * Y = R *  .299 + G *  .587 + B *  .114;
  * U = R * -.169 + G * -.332 + B *  .500 + 128.;
  * V = R *  .500 + G * -.419 + B * -.0813 + 128.;
  */
-static const struct ic_csc_params ic_csc_rgb2ycbcr = {
+static const struct ic_csc_params ic_csc_rgb2ycbcr_bt601 = {
 	.coeff = {
 		{ 77, 150, 29 },
 		{ 469, 427, 128 },
@@ -208,11 +210,13 @@ static const struct ic_csc_params ic_csc_rgb2rgb = {
 };
 
 /*
+ * Inverse BT.601 encoding from YUV limited range to RGB full range:
+ *
  * R = (1.164 * (Y - 16)) + (1.596 * (Cr - 128));
  * G = (1.164 * (Y - 16)) - (0.392 * (Cb - 128)) - (0.813 * (Cr - 128));
  * B = (1.164 * (Y - 16)) + (2.017 * (Cb - 128);
  */
-static const struct ic_csc_params ic_csc_ycbcr2rgb = {
+static const struct ic_csc_params ic_csc_ycbcr2rgb_bt601 = {
 	.coeff = {
 		{ 149, 0, 204 },
 		{ 149, 462, 408 },
@@ -238,9 +242,9 @@ static int init_csc(struct ipu_ic *ic,
 		(priv->tpmem_base + ic->reg->tpmem_csc[csc_index]);
 
 	if (inf == IPUV3_COLORSPACE_YUV && outf == IPUV3_COLORSPACE_RGB)
-		params = &ic_csc_ycbcr2rgb;
+		params = &ic_csc_ycbcr2rgb_bt601;
 	else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_YUV)
-		params = &ic_csc_rgb2ycbcr;
+		params = &ic_csc_rgb2ycbcr_bt601;
 	else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_RGB)
 		params = &ic_csc_rgb2rgb;
 	else {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] gpu: ipu-v3: ipu-ic: Add support for BT.709 encoding
  2019-02-03 19:47 [PATCH 0/3] media: imx: Add support for BT.709 encoding Steve Longerbeam
  2019-02-03 19:47 ` [PATCH 1/3] gpu: ipu-v3: ipu-ic: Rename yuv2rgb encoding matrices Steve Longerbeam
@ 2019-02-03 19:47 ` Steve Longerbeam
  2019-02-08 16:24   ` Tim Harvey
  2019-02-03 19:47 ` [PATCH 3/3] media: imx: Allow BT.709 encoding for IC routes Steve Longerbeam
  2 siblings, 1 reply; 7+ messages in thread
From: Steve Longerbeam @ 2019-02-03 19:47 UTC (permalink / raw)
  To: linux-media
  Cc: Tim Harvey, Philipp Zabel, Steve Longerbeam,
	Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Bartlomiej Zolnierkiewicz, dri-devel, linux-kernel, devel,
	linux-fbdev

Pass v4l2 encoding enum to the ipu_ic task init functions, and add
support for the BT.709 encoding and inverse encoding matrices.

Reported-by: Tim Harvey <tharvey@gateworks.com>
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/gpu/ipu-v3/ipu-ic.c                 | 67 ++++++++++++++++++---
 drivers/gpu/ipu-v3/ipu-image-convert.c      |  1 +
 drivers/staging/media/imx/imx-ic-prpencvf.c |  4 +-
 include/video/imx-ipu-v3.h                  |  5 +-
 4 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/ipu-v3/ipu-ic.c b/drivers/gpu/ipu-v3/ipu-ic.c
index 35ae86ff0585..63362b4fff81 100644
--- a/drivers/gpu/ipu-v3/ipu-ic.c
+++ b/drivers/gpu/ipu-v3/ipu-ic.c
@@ -199,6 +199,23 @@ static const struct ic_csc_params ic_csc_rgb2ycbcr_bt601 = {
 	.scale = 1,
 };
 
+/*
+ * BT.709 encoding from RGB full range to YUV limited range:
+ *
+ * Y = R *  .2126 + G *  .7152 + B *  .0722;
+ * U = R * -.1146 + G * -.3854 + B *  .5000 + 128.;
+ * V = R *  .5000 + G * -.4542 + B * -.0458 + 128.;
+ */
+static const struct ic_csc_params ic_csc_rgb2ycbcr_bt709 = {
+	.coeff = {
+		{ 54, 183, 18 },
+		{ 483, 413, 128 },
+		{ 128, 396, 500 },
+	},
+	.offset = { 0, 512, 512 },
+	.scale = 1,
+};
+
 /* transparent RGB->RGB matrix for graphics combining */
 static const struct ic_csc_params ic_csc_rgb2rgb = {
 	.coeff = {
@@ -226,12 +243,31 @@ static const struct ic_csc_params ic_csc_ycbcr2rgb_bt601 = {
 	.scale = 2,
 };
 
+/*
+ * Inverse BT.709 encoding from YUV limited range to RGB full range:
+ *
+ * R = (1. * (Y - 16)) + (1.5748 * (Cr - 128));
+ * G = (1. * (Y - 16)) - (0.1873 * (Cb - 128)) - (0.4681 * (Cr - 128));
+ * B = (1. * (Y - 16)) + (1.8556 * (Cb - 128);
+ */
+static const struct ic_csc_params ic_csc_ycbcr2rgb_bt709 = {
+	.coeff = {
+		{ 128, 0, 202 },
+		{ 128, 488, 452 },
+		{ 128, 238, 0 },
+	},
+	.offset = { -435, 136, -507 },
+	.scale = 2,
+};
+
 static int init_csc(struct ipu_ic *ic,
 		    enum ipu_color_space inf,
 		    enum ipu_color_space outf,
+		    enum v4l2_ycbcr_encoding encoding,
 		    int csc_index)
 {
 	struct ipu_ic_priv *priv = ic->priv;
+	const struct ic_csc_params *params_rgb2yuv, *params_yuv2rgb;
 	const struct ic_csc_params *params;
 	u32 __iomem *base;
 	const u16 (*c)[3];
@@ -241,10 +277,24 @@ static int init_csc(struct ipu_ic *ic,
 	base = (u32 __iomem *)
 		(priv->tpmem_base + ic->reg->tpmem_csc[csc_index]);
 
+	switch (encoding) {
+	case V4L2_YCBCR_ENC_601:
+		params_rgb2yuv =  &ic_csc_rgb2ycbcr_bt601;
+		params_yuv2rgb = &ic_csc_ycbcr2rgb_bt601;
+		break;
+	case V4L2_YCBCR_ENC_709:
+		params_rgb2yuv =  &ic_csc_rgb2ycbcr_bt709;
+		params_yuv2rgb = &ic_csc_ycbcr2rgb_bt709;
+		break;
+	default:
+		dev_err(priv->ipu->dev, "Unsupported YCbCr encoding\n");
+		return -EINVAL;
+	}
+
 	if (inf == IPUV3_COLORSPACE_YUV && outf == IPUV3_COLORSPACE_RGB)
-		params = &ic_csc_ycbcr2rgb_bt601;
+		params = params_yuv2rgb;
 	else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_YUV)
-		params = &ic_csc_rgb2ycbcr_bt601;
+		params = params_rgb2yuv;
 	else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_RGB)
 		params = &ic_csc_rgb2rgb;
 	else {
@@ -391,6 +441,7 @@ EXPORT_SYMBOL_GPL(ipu_ic_task_disable);
 
 int ipu_ic_task_graphics_init(struct ipu_ic *ic,
 			      enum ipu_color_space in_g_cs,
+			      enum v4l2_ycbcr_encoding encoding,
 			      bool galpha_en, u32 galpha,
 			      bool colorkey_en, u32 colorkey)
 {
@@ -409,7 +460,7 @@ int ipu_ic_task_graphics_init(struct ipu_ic *ic,
 	if (!(ic_conf & ic->bit->ic_conf_csc1_en)) {
 		/* need transparent CSC1 conversion */
 		ret = init_csc(ic, IPUV3_COLORSPACE_RGB,
-			       IPUV3_COLORSPACE_RGB, 0);
+			       IPUV3_COLORSPACE_RGB, encoding, 0);
 		if (ret)
 			goto unlock;
 	}
@@ -417,7 +468,7 @@ int ipu_ic_task_graphics_init(struct ipu_ic *ic,
 	ic->g_in_cs = in_g_cs;
 
 	if (ic->g_in_cs != ic->out_cs) {
-		ret = init_csc(ic, ic->g_in_cs, ic->out_cs, 1);
+		ret = init_csc(ic, ic->g_in_cs, ic->out_cs, encoding, 1);
 		if (ret)
 			goto unlock;
 	}
@@ -451,6 +502,7 @@ int ipu_ic_task_init_rsc(struct ipu_ic *ic,
 			 int out_width, int out_height,
 			 enum ipu_color_space in_cs,
 			 enum ipu_color_space out_cs,
+			 enum v4l2_ycbcr_encoding encoding,
 			 u32 rsc)
 {
 	struct ipu_ic_priv *priv = ic->priv;
@@ -486,7 +538,7 @@ int ipu_ic_task_init_rsc(struct ipu_ic *ic,
 	ic->out_cs = out_cs;
 
 	if (ic->in_cs != ic->out_cs) {
-		ret = init_csc(ic, ic->in_cs, ic->out_cs, 0);
+		ret = init_csc(ic, ic->in_cs, ic->out_cs, encoding, 0);
 		if (ret)
 			goto unlock;
 	}
@@ -500,10 +552,11 @@ int ipu_ic_task_init(struct ipu_ic *ic,
 		     int in_width, int in_height,
 		     int out_width, int out_height,
 		     enum ipu_color_space in_cs,
-		     enum ipu_color_space out_cs)
+		     enum ipu_color_space out_cs,
+		     enum v4l2_ycbcr_encoding encoding)
 {
 	return ipu_ic_task_init_rsc(ic, in_width, in_height, out_width,
-				    out_height, in_cs, out_cs, 0);
+				    out_height, in_cs, out_cs, encoding, 0);
 }
 EXPORT_SYMBOL_GPL(ipu_ic_task_init);
 
diff --git a/drivers/gpu/ipu-v3/ipu-image-convert.c b/drivers/gpu/ipu-v3/ipu-image-convert.c
index 13103ab86050..8b37daa99f58 100644
--- a/drivers/gpu/ipu-v3/ipu-image-convert.c
+++ b/drivers/gpu/ipu-v3/ipu-image-convert.c
@@ -1358,6 +1358,7 @@ static int convert_start(struct ipu_image_convert_run *run, unsigned int tile)
 			       dest_width,
 			       dest_height,
 			       src_cs, dest_cs,
+			       d_image->base.pix.ycbcr_enc,
 			       rsc);
 	if (ret) {
 		dev_err(priv->ipu->dev, "ipu_ic_task_init failed, %d\n", ret);
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 3637693c2bc8..c86d5275db46 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -484,7 +484,7 @@ static int prp_setup_rotation(struct prp_priv *priv)
 	ret = ipu_ic_task_init(priv->ic,
 			       infmt->width, infmt->height,
 			       outfmt->height, outfmt->width,
-			       incc->cs, outcc->cs);
+			       incc->cs, outcc->cs, outfmt->ycbcr_enc);
 	if (ret) {
 		v4l2_err(&ic_priv->sd, "ipu_ic_task_init failed, %d\n", ret);
 		goto free_rot1;
@@ -587,7 +587,7 @@ static int prp_setup_norotation(struct prp_priv *priv)
 	ret = ipu_ic_task_init(priv->ic,
 			       infmt->width, infmt->height,
 			       outfmt->width, outfmt->height,
-			       incc->cs, outcc->cs);
+			       incc->cs, outcc->cs, outfmt->ycbcr_enc);
 	if (ret) {
 		v4l2_err(&ic_priv->sd, "ipu_ic_task_init failed, %d\n", ret);
 		return ret;
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index c887f4bee5f8..b19d1e23eece 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -391,15 +391,18 @@ int ipu_ic_task_init(struct ipu_ic *ic,
 		     int in_width, int in_height,
 		     int out_width, int out_height,
 		     enum ipu_color_space in_cs,
-		     enum ipu_color_space out_cs);
+		     enum ipu_color_space out_cs,
+		     enum v4l2_ycbcr_encoding encoding);
 int ipu_ic_task_init_rsc(struct ipu_ic *ic,
 			 int in_width, int in_height,
 			 int out_width, int out_height,
 			 enum ipu_color_space in_cs,
 			 enum ipu_color_space out_cs,
+			 enum v4l2_ycbcr_encoding encoding,
 			 u32 rsc);
 int ipu_ic_task_graphics_init(struct ipu_ic *ic,
 			      enum ipu_color_space in_g_cs,
+			      enum v4l2_ycbcr_encoding encoding,
 			      bool galpha_en, u32 galpha,
 			      bool colorkey_en, u32 colorkey);
 void ipu_ic_task_enable(struct ipu_ic *ic);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] media: imx: Allow BT.709 encoding for IC routes
  2019-02-03 19:47 [PATCH 0/3] media: imx: Add support for BT.709 encoding Steve Longerbeam
  2019-02-03 19:47 ` [PATCH 1/3] gpu: ipu-v3: ipu-ic: Rename yuv2rgb encoding matrices Steve Longerbeam
  2019-02-03 19:47 ` [PATCH 2/3] gpu: ipu-v3: ipu-ic: Add support for BT.709 encoding Steve Longerbeam
@ 2019-02-03 19:47 ` Steve Longerbeam
  2019-02-05  1:55   ` Steve Longerbeam
  2 siblings, 1 reply; 7+ messages in thread
From: Steve Longerbeam @ 2019-02-03 19:47 UTC (permalink / raw)
  To: linux-media
  Cc: Tim Harvey, Philipp Zabel, Steve Longerbeam,
	Mauro Carvalho Chehab, Greg Kroah-Hartman, devel, linux-kernel

The IC now supports BT.709 Y'CbCr encoding, in addition to existing BT.601
encoding, so allow both, for pipelines that route through the IC.

Reported-by: Tim Harvey <tharvey@gateworks.com>
Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
---
 drivers/staging/media/imx/imx-media-utils.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 5f110d90a4ef..3512f09fb226 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -571,7 +571,9 @@ void imx_media_fill_default_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
 		tryfmt->quantization = is_rgb ?
 			V4L2_QUANTIZATION_FULL_RANGE :
 			V4L2_QUANTIZATION_LIM_RANGE;
-		tryfmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
+		if (tryfmt->ycbcr_enc != V4L2_YCBCR_ENC_601 &&
+		    tryfmt->ycbcr_enc != V4L2_YCBCR_ENC_709)
+			tryfmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
 	}
 }
 EXPORT_SYMBOL_GPL(imx_media_fill_default_mbus_fields);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] media: imx: Allow BT.709 encoding for IC routes
  2019-02-03 19:47 ` [PATCH 3/3] media: imx: Allow BT.709 encoding for IC routes Steve Longerbeam
@ 2019-02-05  1:55   ` Steve Longerbeam
  0 siblings, 0 replies; 7+ messages in thread
From: Steve Longerbeam @ 2019-02-05  1:55 UTC (permalink / raw)
  To: linux-media
  Cc: Tim Harvey, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, open list:STAGING SUBSYSTEM, open list

Sorry this patch isn't working, it's not possible to set BT.709 
encoding, working on a fix for v2.

Steve


On 2/3/19 11:47 AM, Steve Longerbeam wrote:
> The IC now supports BT.709 Y'CbCr encoding, in addition to existing BT.601
> encoding, so allow both, for pipelines that route through the IC.
>
> Reported-by: Tim Harvey <tharvey@gateworks.com>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
>   drivers/staging/media/imx/imx-media-utils.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> index 5f110d90a4ef..3512f09fb226 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -571,7 +571,9 @@ void imx_media_fill_default_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
>   		tryfmt->quantization = is_rgb ?
>   			V4L2_QUANTIZATION_FULL_RANGE :
>   			V4L2_QUANTIZATION_LIM_RANGE;
> -		tryfmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
> +		if (tryfmt->ycbcr_enc != V4L2_YCBCR_ENC_601 &&
> +		    tryfmt->ycbcr_enc != V4L2_YCBCR_ENC_709)
> +			tryfmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
>   	}
>   }
>   EXPORT_SYMBOL_GPL(imx_media_fill_default_mbus_fields);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] gpu: ipu-v3: ipu-ic: Add support for BT.709 encoding
  2019-02-03 19:47 ` [PATCH 2/3] gpu: ipu-v3: ipu-ic: Add support for BT.709 encoding Steve Longerbeam
@ 2019-02-08 16:24   ` Tim Harvey
  2019-02-08 19:14     ` Steve Longerbeam
  0 siblings, 1 reply; 7+ messages in thread
From: Tim Harvey @ 2019-02-08 16:24 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: linux-media, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Bartlomiej Zolnierkiewicz,
	open list:DRM DRIVERS FOR FREESCALE IMX, open list,
	open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER

On Sun, Feb 3, 2019 at 11:48 AM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
> Pass v4l2 encoding enum to the ipu_ic task init functions, and add
> support for the BT.709 encoding and inverse encoding matrices.
>
> Reported-by: Tim Harvey <tharvey@gateworks.com>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
>  drivers/gpu/ipu-v3/ipu-ic.c                 | 67 ++++++++++++++++++---
>  drivers/gpu/ipu-v3/ipu-image-convert.c      |  1 +
>  drivers/staging/media/imx/imx-ic-prpencvf.c |  4 +-
>  include/video/imx-ipu-v3.h                  |  5 +-
>  4 files changed, 67 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-ic.c b/drivers/gpu/ipu-v3/ipu-ic.c
> index 35ae86ff0585..63362b4fff81 100644
> --- a/drivers/gpu/ipu-v3/ipu-ic.c
> +++ b/drivers/gpu/ipu-v3/ipu-ic.c
> @@ -199,6 +199,23 @@ static const struct ic_csc_params ic_csc_rgb2ycbcr_bt601 = {
>         .scale = 1,
>  };
>
> +/*
> + * BT.709 encoding from RGB full range to YUV limited range:
> + *
> + * Y = R *  .2126 + G *  .7152 + B *  .0722;
> + * U = R * -.1146 + G * -.3854 + B *  .5000 + 128.;
> + * V = R *  .5000 + G * -.4542 + B * -.0458 + 128.;
> + */
> +static const struct ic_csc_params ic_csc_rgb2ycbcr_bt709 = {
> +       .coeff = {
> +               { 54, 183, 18 },
> +               { 483, 413, 128 },
> +               { 128, 396, 500 },
> +       },
> +       .offset = { 0, 512, 512 },
> +       .scale = 1,
> +};
> +
>  /* transparent RGB->RGB matrix for graphics combining */
>  static const struct ic_csc_params ic_csc_rgb2rgb = {
>         .coeff = {
> @@ -226,12 +243,31 @@ static const struct ic_csc_params ic_csc_ycbcr2rgb_bt601 = {
>         .scale = 2,
>  };
>
> +/*
> + * Inverse BT.709 encoding from YUV limited range to RGB full range:
> + *
> + * R = (1. * (Y - 16)) + (1.5748 * (Cr - 128));
> + * G = (1. * (Y - 16)) - (0.1873 * (Cb - 128)) - (0.4681 * (Cr - 128));
> + * B = (1. * (Y - 16)) + (1.8556 * (Cb - 128);
> + */
> +static const struct ic_csc_params ic_csc_ycbcr2rgb_bt709 = {
> +       .coeff = {
> +               { 128, 0, 202 },
> +               { 128, 488, 452 },
> +               { 128, 238, 0 },
> +       },
> +       .offset = { -435, 136, -507 },
> +       .scale = 2,
> +};
> +
>  static int init_csc(struct ipu_ic *ic,
>                     enum ipu_color_space inf,
>                     enum ipu_color_space outf,
> +                   enum v4l2_ycbcr_encoding encoding,
>                     int csc_index)
>  {
>         struct ipu_ic_priv *priv = ic->priv;
> +       const struct ic_csc_params *params_rgb2yuv, *params_yuv2rgb;
>         const struct ic_csc_params *params;
>         u32 __iomem *base;
>         const u16 (*c)[3];
> @@ -241,10 +277,24 @@ static int init_csc(struct ipu_ic *ic,
>         base = (u32 __iomem *)
>                 (priv->tpmem_base + ic->reg->tpmem_csc[csc_index]);
>
> +       switch (encoding) {
> +       case V4L2_YCBCR_ENC_601:
> +               params_rgb2yuv =  &ic_csc_rgb2ycbcr_bt601;
> +               params_yuv2rgb = &ic_csc_ycbcr2rgb_bt601;
> +               break;
> +       case V4L2_YCBCR_ENC_709:
> +               params_rgb2yuv =  &ic_csc_rgb2ycbcr_bt709;
> +               params_yuv2rgb = &ic_csc_ycbcr2rgb_bt709;
> +               break;
> +       default:
> +               dev_err(priv->ipu->dev, "Unsupported YCbCr encoding\n");
> +               return -EINVAL;
> +       }
> +

Steve,

This will fail for RGB to RGB with 'Unsupported YCbCr encoding. We
need to account for the RGB->RGB case.

How about something like:

 static int init_csc(struct ipu_ic *ic,
                    enum ipu_color_space inf,
                    enum ipu_color_space outf,
+                   enum v4l2_ycbcr_encoding encoding,
                    int csc_index)
 {
        struct ipu_ic_priv *priv = ic->priv;
-       const struct ic_csc_params *params;
+       const struct ic_csc_params *params = NULL;
        u32 __iomem *base;
        const u16 (*c)[3];
        const u16 *a;
@@ -241,13 +276,18 @@ static int init_csc(struct ipu_ic *ic,
        base = (u32 __iomem *)
                (priv->tpmem_base + ic->reg->tpmem_csc[csc_index]);

-       if (inf == IPUV3_COLORSPACE_YUV && outf == IPUV3_COLORSPACE_RGB)
-               params = &ic_csc_ycbcr2rgb_bt601;
-       else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_YUV)
-               params = &ic_csc_rgb2ycbcr_bt601;
+       if (inf == IPUV3_COLORSPACE_YUV && outf == IPUV3_COLORSPACE_RGB) {
+               params = (encoding == V4L2_YCBCR_ENC_601) ?
+                       &ic_csc_ycbcr2rgb_bt601 : &ic_csc_ycbcr2rgb_bt709;
+       }
+       else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_YUV) {
+               params = (encoding == V4L2_YCBCR_ENC_601) ?
+                       &ic_csc_rgb2ycbcr_bt601 : &ic_csc_rgb2ycbcr_bt709;
+       }
        else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_RGB)
                params = &ic_csc_rgb2rgb;
-       else {
+
+       if (!params) {
                dev_err(priv->ipu->dev, "Unsupported color space conversion\n");
                return -EINVAL;
        }

Tim

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] gpu: ipu-v3: ipu-ic: Add support for BT.709 encoding
  2019-02-08 16:24   ` Tim Harvey
@ 2019-02-08 19:14     ` Steve Longerbeam
  0 siblings, 0 replies; 7+ messages in thread
From: Steve Longerbeam @ 2019-02-08 19:14 UTC (permalink / raw)
  To: Tim Harvey
  Cc: linux-media, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Bartlomiej Zolnierkiewicz,
	open list:DRM DRIVERS FOR FREESCALE IMX, open list,
	open list:STAGING SUBSYSTEM, open list:FRAMEBUFFER LAYER



On 2/8/19 8:24 AM, Tim Harvey wrote:
> On Sun, Feb 3, 2019 at 11:48 AM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>> Pass v4l2 encoding enum to the ipu_ic task init functions, and add
>> support for the BT.709 encoding and inverse encoding matrices.
>>
>> Reported-by: Tim Harvey <tharvey@gateworks.com>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>>   drivers/gpu/ipu-v3/ipu-ic.c                 | 67 ++++++++++++++++++---
>>   drivers/gpu/ipu-v3/ipu-image-convert.c      |  1 +
>>   drivers/staging/media/imx/imx-ic-prpencvf.c |  4 +-
>>   include/video/imx-ipu-v3.h                  |  5 +-
>>   4 files changed, 67 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/ipu-v3/ipu-ic.c b/drivers/gpu/ipu-v3/ipu-ic.c
>> index 35ae86ff0585..63362b4fff81 100644
>> --- a/drivers/gpu/ipu-v3/ipu-ic.c
>> +++ b/drivers/gpu/ipu-v3/ipu-ic.c
>> @@ -199,6 +199,23 @@ static const struct ic_csc_params ic_csc_rgb2ycbcr_bt601 = {
>>          .scale = 1,
>>   };
>>
>> +/*
>> + * BT.709 encoding from RGB full range to YUV limited range:
>> + *
>> + * Y = R *  .2126 + G *  .7152 + B *  .0722;
>> + * U = R * -.1146 + G * -.3854 + B *  .5000 + 128.;
>> + * V = R *  .5000 + G * -.4542 + B * -.0458 + 128.;
>> + */
>> +static const struct ic_csc_params ic_csc_rgb2ycbcr_bt709 = {
>> +       .coeff = {
>> +               { 54, 183, 18 },
>> +               { 483, 413, 128 },
>> +               { 128, 396, 500 },
>> +       },
>> +       .offset = { 0, 512, 512 },
>> +       .scale = 1,
>> +};
>> +
>>   /* transparent RGB->RGB matrix for graphics combining */
>>   static const struct ic_csc_params ic_csc_rgb2rgb = {
>>          .coeff = {
>> @@ -226,12 +243,31 @@ static const struct ic_csc_params ic_csc_ycbcr2rgb_bt601 = {
>>          .scale = 2,
>>   };
>>
>> +/*
>> + * Inverse BT.709 encoding from YUV limited range to RGB full range:
>> + *
>> + * R = (1. * (Y - 16)) + (1.5748 * (Cr - 128));
>> + * G = (1. * (Y - 16)) - (0.1873 * (Cb - 128)) - (0.4681 * (Cr - 128));
>> + * B = (1. * (Y - 16)) + (1.8556 * (Cb - 128);
>> + */
>> +static const struct ic_csc_params ic_csc_ycbcr2rgb_bt709 = {
>> +       .coeff = {
>> +               { 128, 0, 202 },
>> +               { 128, 488, 452 },
>> +               { 128, 238, 0 },
>> +       },
>> +       .offset = { -435, 136, -507 },
>> +       .scale = 2,
>> +};
>> +
>>   static int init_csc(struct ipu_ic *ic,
>>                      enum ipu_color_space inf,
>>                      enum ipu_color_space outf,
>> +                   enum v4l2_ycbcr_encoding encoding,
>>                      int csc_index)
>>   {
>>          struct ipu_ic_priv *priv = ic->priv;
>> +       const struct ic_csc_params *params_rgb2yuv, *params_yuv2rgb;
>>          const struct ic_csc_params *params;
>>          u32 __iomem *base;
>>          const u16 (*c)[3];
>> @@ -241,10 +277,24 @@ static int init_csc(struct ipu_ic *ic,
>>          base = (u32 __iomem *)
>>                  (priv->tpmem_base + ic->reg->tpmem_csc[csc_index]);
>>
>> +       switch (encoding) {
>> +       case V4L2_YCBCR_ENC_601:
>> +               params_rgb2yuv =  &ic_csc_rgb2ycbcr_bt601;
>> +               params_yuv2rgb = &ic_csc_ycbcr2rgb_bt601;
>> +               break;
>> +       case V4L2_YCBCR_ENC_709:
>> +               params_rgb2yuv =  &ic_csc_rgb2ycbcr_bt709;
>> +               params_yuv2rgb = &ic_csc_ycbcr2rgb_bt709;
>> +               break;
>> +       default:
>> +               dev_err(priv->ipu->dev, "Unsupported YCbCr encoding\n");
>> +               return -EINVAL;
>> +       }
>> +
> Steve,
>
> This will fail for RGB to RGB with 'Unsupported YCbCr encoding. We
> need to account for the RGB->RGB case.
>
> How about something like:


Thanks for reporting Tim

I rather keep the check for supported encoding, and instead get rid of 
"Unsupported color space conversion" error, because that is the YUV->YUV 
case which can be allowed using the identity matrix.

Steve

>
>   static int init_csc(struct ipu_ic *ic,
>                      enum ipu_color_space inf,
>                      enum ipu_color_space outf,
> +                   enum v4l2_ycbcr_encoding encoding,
>                      int csc_index)
>   {
>          struct ipu_ic_priv *priv = ic->priv;
> -       const struct ic_csc_params *params;
> +       const struct ic_csc_params *params = NULL;
>          u32 __iomem *base;
>          const u16 (*c)[3];
>          const u16 *a;
> @@ -241,13 +276,18 @@ static int init_csc(struct ipu_ic *ic,
>          base = (u32 __iomem *)
>                  (priv->tpmem_base + ic->reg->tpmem_csc[csc_index]);
>
> -       if (inf == IPUV3_COLORSPACE_YUV && outf == IPUV3_COLORSPACE_RGB)
> -               params = &ic_csc_ycbcr2rgb_bt601;
> -       else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_YUV)
> -               params = &ic_csc_rgb2ycbcr_bt601;
> +       if (inf == IPUV3_COLORSPACE_YUV && outf == IPUV3_COLORSPACE_RGB) {
> +               params = (encoding == V4L2_YCBCR_ENC_601) ?
> +                       &ic_csc_ycbcr2rgb_bt601 : &ic_csc_ycbcr2rgb_bt709;
> +       }
> +       else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_YUV) {
> +               params = (encoding == V4L2_YCBCR_ENC_601) ?
> +                       &ic_csc_rgb2ycbcr_bt601 : &ic_csc_rgb2ycbcr_bt709;
> +       }
>          else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_RGB)
>                  params = &ic_csc_rgb2rgb;
> -       else {
> +
> +       if (!params) {
>                  dev_err(priv->ipu->dev, "Unsupported color space conversion\n");
>                  return -EINVAL;
>          }
>
> Tim


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-03 19:47 [PATCH 0/3] media: imx: Add support for BT.709 encoding Steve Longerbeam
2019-02-03 19:47 ` [PATCH 1/3] gpu: ipu-v3: ipu-ic: Rename yuv2rgb encoding matrices Steve Longerbeam
2019-02-03 19:47 ` [PATCH 2/3] gpu: ipu-v3: ipu-ic: Add support for BT.709 encoding Steve Longerbeam
2019-02-08 16:24   ` Tim Harvey
2019-02-08 19:14     ` Steve Longerbeam
2019-02-03 19:47 ` [PATCH 3/3] media: imx: Allow BT.709 encoding for IC routes Steve Longerbeam
2019-02-05  1:55   ` Steve Longerbeam

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org linux-media@archiver.kernel.org
	public-inbox-index linux-media


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/ public-inbox