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

2019-02-09 Thread Steve Longerbeam



On 2/8/19 4:20 PM, Tim Harvey wrote:

On Fri, Feb 8, 2019 at 11:28 AM Steve Longerbeam  wrote:

 if (inf == outf)
 params = _csc_identity;
 else if (inf == IPUV3_COLORSPACE_YUV)
-   params = _csc_ycbcr2rgb_bt601;
+   params = _csc_ycbcr2rgb;


Steve,

compile issue...

params = params_yuv2rgb;


 else
-   params = _csc_rgb2ycbcr_bt601;
+   params = _csc_rgb2ycbcr;

params = params_rgb2yuv;


Wow, did I not even compile test that? Must be my head cold :-/
Sending v4.



But, I'm still failing when using the mem2mem element (gst-launch-1.0
v4l2src device=/dev/video4 ! v4l2video8convert
output-io-mode=dmabuf-import ! fbdevsink) with 'Unsupported YCbCr
encoding' because of inf=IPU_COLORSPACE_YCBCR outf=IPU_COLORSPACE_RGB
and a seemingly unset encoding being passed in.

It looks like maybe something in the mem2mem driver isn't defaulting
encoding. The call path is (v4l2_m2m_streamon -> device_run ->
ipu_image_convert_queue -> convert_start -> ipu_ic_task_init_rsc ->
init_csc).


Looking at v7 of the mem2mem driver, it will set ycbcr_enc at the output 
side to V4L2_YCBCR_ENC_DEFAULT if colorspace is default. So colorspace 
will need to be set to something non-default in addition to setting 
ycbcr_enc, at the output side. I don't know whether gstreamer 
v4l2videoNconvertelement will do this, but you could hack the driver for 
now to get around it, and let Philipp know this may need a workaround in 
mem2mem for v8.


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


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

2019-02-09 Thread Steve Longerbeam
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 
Signed-off-by: Steve Longerbeam 
---
Changes in v2:
- only return "Unsupported YCbCr encoding" error if inf != outf,
  since if inf == outf, the identity matrix can be used. Reported
  by Tim Harvey.
---
 drivers/gpu/ipu-v3/ipu-ic.c | 71 +++--
 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, 71 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/ipu-v3/ipu-ic.c b/drivers/gpu/ipu-v3/ipu-ic.c
index e459615a49a1..0d57ca7ba18e 100644
--- a/drivers/gpu/ipu-v3/ipu-ic.c
+++ b/drivers/gpu/ipu-v3/ipu-ic.c
@@ -212,6 +212,23 @@ static const struct ic_csc_params ic_csc_identity = {
.scale = 2,
 };
 
+/*
+ * 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,
+};
+
 /*
  * Inverse BT.601 encoding from YUV limited range to RGB full range:
  *
@@ -229,12 +246,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];
@@ -244,12 +280,30 @@ 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 =  _csc_rgb2ycbcr_bt601;
+   params_yuv2rgb = _csc_ycbcr2rgb_bt601;
+   break;
+   case V4L2_YCBCR_ENC_709:
+   params_rgb2yuv =  _csc_rgb2ycbcr_bt709;
+   params_yuv2rgb = _csc_ycbcr2rgb_bt709;
+   break;
+   default:
+   if (inf != outf) {
+   dev_err(priv->ipu->dev,
+   "Unsupported YCbCr encoding\n");
+   return -EINVAL;
+   }
+   break;
+   }
+
if (inf == outf)
params = _csc_identity;
else if (inf == IPUV3_COLORSPACE_YUV)
-   params = _csc_ycbcr2rgb_bt601;
+   params = _csc_ycbcr2rgb;
else
-   params = _csc_rgb2ycbcr_bt601;
+   params = _csc_rgb2ycbcr;
 
/* Cast to unsigned */
c = (const u16 (*)[3])params->coeff;
@@ -390,6 +444,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)
 {
@@ -408,7 +463,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;
}
@@ -416,7 +471,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;
}
@@ -450,6 +505,7 @@ int ipu_ic_task_init_rsc(struct ipu_ic *ic,
 int out_width, int out_height,
 enum 

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

2019-02-08 Thread Tim Harvey
On Fri, Feb 8, 2019 at 11:28 AM Steve Longerbeam  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 
> Signed-off-by: Steve Longerbeam 
> ---
> Changes in v2:
> - only return "Unsupported YCbCr encoding" error if inf != outf,
>   since if inf == outf, the identity matrix can be used. Reported
>   by Tim Harvey.
> ---
>  drivers/gpu/ipu-v3/ipu-ic.c | 71 +++--
>  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, 71 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/ipu-v3/ipu-ic.c b/drivers/gpu/ipu-v3/ipu-ic.c
> index e459615a49a1..0d57ca7ba18e 100644
> --- a/drivers/gpu/ipu-v3/ipu-ic.c
> +++ b/drivers/gpu/ipu-v3/ipu-ic.c
> @@ -212,6 +212,23 @@ static const struct ic_csc_params ic_csc_identity = {
> .scale = 2,
>  };
>
> +/*
> + * 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,
> +};
> +
>  /*
>   * Inverse BT.601 encoding from YUV limited range to RGB full range:
>   *
> @@ -229,12 +246,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];
> @@ -244,12 +280,30 @@ 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 =  _csc_rgb2ycbcr_bt601;
> +   params_yuv2rgb = _csc_ycbcr2rgb_bt601;
> +   break;
> +   case V4L2_YCBCR_ENC_709:
> +   params_rgb2yuv =  _csc_rgb2ycbcr_bt709;
> +   params_yuv2rgb = _csc_ycbcr2rgb_bt709;
> +   break;
> +   default:
> +   if (inf != outf) {
> +   dev_err(priv->ipu->dev,
> +   "Unsupported YCbCr encoding\n");
> +   return -EINVAL;
> +   }
> +   break;
> +   }
> +
> if (inf == outf)
> params = _csc_identity;
> else if (inf == IPUV3_COLORSPACE_YUV)
> -   params = _csc_ycbcr2rgb_bt601;
> +   params = _csc_ycbcr2rgb;


Steve,

compile issue...

params = params_yuv2rgb;

> else
> -   params = _csc_rgb2ycbcr_bt601;
> +   params = _csc_rgb2ycbcr;

params = params_rgb2yuv;

But, I'm still failing when using the mem2mem element (gst-launch-1.0
v4l2src device=/dev/video4 ! v4l2video8convert
output-io-mode=dmabuf-import ! fbdevsink) with 'Unsupported YCbCr
encoding' because of inf=IPU_COLORSPACE_YCBCR outf=IPU_COLORSPACE_RGB
and a seemingly unset encoding being passed in.

It looks like maybe something in the mem2mem driver isn't defaulting
encoding. The call path is (v4l2_m2m_streamon -> device_run ->
ipu_image_convert_queue -> convert_start -> ipu_ic_task_init_rsc ->
init_csc).

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