Re: [PATCH 2/3] gpu: ipu-v3: ipu-ic: Add support for BT.709 encoding

[Steve Longerbeam]

On 2/8/19 8:24 AM, Tim Harvey wrote:
On Sun, Feb 3, 2019 at 11:48 AM Steve Longerbeam <slongerb...@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 <thar...@gateworks.com>
Signed-off-by: Steve Longerbeam <slongerb...@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

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