Hi Tomi,

Thank you for the patch.

On Mon, Sep 02, 2019 at 03:53:57PM +0300, Tomi Valkeinen wrote:
> From: Jyri Sarha <jsa...@ti.com>
> 
> Adds support for COLOR_ENCODING and COLOR_RANGE properties to
> omap_plane.c and dispc.c. The supported encodings and ranges are:
> 
> For COLOR_ENCODING:
> - YCbCt BT.601 (default)

Did you mean YCbCr ?

> - YCbCt BT.709
> 
> For COLOR_RANGE:
> - YCbCt limited range
> - YCbCt full range (default)
> 
> Signed-off-by: Jyri Sarha <jsa...@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkei...@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c   | 119 ++++++++++++++++++++------
>  drivers/gpu/drm/omapdrm/dss/omapdss.h |   4 +
>  drivers/gpu/drm/omapdrm/omap_plane.c  |  30 +++++++
>  3 files changed, 127 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c 
> b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index 7d87d60edb80..40ddb28ee7aa 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -892,32 +892,91 @@ static void dispc_wb_write_color_conv_coef(struct 
> dispc_device *dispc,
>  #undef CVAL
>  }
>  
> -static void dispc_setup_color_conv_coef(struct dispc_device *dispc)
> +/* YUV -> RGB, ITU-R BT.601, full range */
> +const static struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_full = {

static const is usually preferred over const static, here and below.

> +     256,   0,  358,         /* ry, rcb, rcr |1.000  0.000  1.402|*/
> +     256, -88, -182,         /* gy, gcb, gcr |1.000 -0.344 -0.714|*/
> +     256, 452,    0,         /* by, bcb, bcr |1.000  1.772  0.000|*/
> +     true,                   /* full range */
> +};
> +
> +/* YUV -> RGB, ITU-R BT.601, limited range */
> +const static struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_lim = {
> +     298,    0,  409,        /* ry, rcb, rcr |1.164  0.000  1.596|*/
> +     298, -100, -208,        /* gy, gcb, gcr |1.164 -0.392 -0.813|*/
> +     298,  516,    0,        /* by, bcb, bcr |1.164  2.017  0.000|*/
> +     false,                  /* limited range */
> +};
> +
> +/* YUV -> RGB, ITU-R BT.709, full range */
> +const static struct csc_coef_yuv2rgb coefs_yuv2rgb_bt709_full = {
> +     256,    0,  402,        /* ry, rcb, rcr |1.000  0.000  1.570|*/
> +     256,  -48, -120,        /* gy, gcb, gcr |1.000 -0.187 -0.467|*/
> +     256,  475,    0,        /* by, bcb, bcr |1.000  1.856  0.000|*/
> +     true,                   /* full range */
> +};
> +
> +/* YUV -> RGB, ITU-R BT.709, limited range */
> +const static struct csc_coef_yuv2rgb coefs_yuv2rgb_bt709_lim = {
> +     298,    0,  459,        /* ry, rcb, rcr |1.164  0.000  1.793|*/
> +     298,  -55, -136,        /* gy, gcb, gcr |1.164 -0.213 -0.533|*/
> +     298,  541,    0,        /* by, bcb, bcr |1.164  2.112  0.000|*/
> +     false,                  /* limited range */
> +};
> +
> +/* RGB -> YUV, ITU-R BT.601, limited range */
> +const static struct csc_coef_rgb2yuv coefs_rgb2yuv_bt601_lim = {
> +      66, 129,  25,          /* yr,   yg,  yb | 0.257  0.504  0.098|*/
> +     -38, -74, 112,          /* cbr, cbg, cbb |-0.148 -0.291  0.439|*/
> +     112, -94, -18,          /* crr, crg, crb | 0.439 -0.368 -0.071|*/
> +     false,                  /* limited range */
> +};
> +
> +/* RGB -> YUV, ITU-R BT.601, full range */
> +const static struct csc_coef_rgb2yuv coefs_rgb2yuv_bt601_full = {
> +      77,  150,  29,         /* yr,   yg,  yb | 0.299  0.587  0.114|*/
> +     -43,  -85, 128,         /* cbr, cbg, cbb |-0.173 -0.339  0.511|*/
> +     128, -107, -21,         /* crr, crg, crb | 0.511 -0.428 -0.083|*/
> +     true,                   /* full range */
> +};
> +
> +/* RGB -> YUV, ITU-R BT.709, limited range */
> +const static struct csc_coef_rgb2yuv coefs_rgb2yuv_bt701_lim = {

That should be coefs_rgb2yuv_bt709_lim

> +      47,  157,   16,        /* yr,   yg,  yb | 0.1826  0.6142  0.0620|*/
> +     -26,  -87,  112,        /* cbr, cbg, cbb |-0.1006 -0.3386  0.4392|*/
> +     112, -102,  -10,        /* crr, crg, crb | 0.4392 -0.3989 -0.0403|*/
> +     false,                  /* limited range */
> +};

Why is coefs_rgb2yuv_bt709_full not supported ? Actually
coefs_rgb2yuv_bt601_lim and coefs_rgb2yuv_bt701_lim seem unused.

> +
> +static int dispc_ovl_set_csc(struct dispc_device *dispc,
> +                          enum omap_plane_id plane,
> +                          enum drm_color_encoding color_encoding,
> +                          enum drm_color_range color_range)
>  {
> -     int i;
> -     int num_ovl = dispc_get_num_ovls(dispc);
> -
> -     /* YUV -> RGB, ITU-R BT.601, limited range */
> -     const struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_lim = {
> -             298,    0,  409,        /* ry, rcb, rcr */
> -             298, -100, -208,        /* gy, gcb, gcr */
> -             298,  516,    0,        /* by, bcb, bcr */
> -             false,                  /* limited range */
> -     };
> +     const struct csc_coef_yuv2rgb *csc;
>  
> -     /* RGB -> YUV, ITU-R BT.601, limited range */
> -     const struct csc_coef_rgb2yuv coefs_rgb2yuv_bt601_lim = {
> -              66, 129,  25,          /* yr,   yg,  yb */
> -             -38, -74, 112,          /* cbr, cbg, cbb */
> -             112, -94, -18,          /* crr, crg, crb */
> -             false,                  /* limited range */
> -     };
> +     switch (color_encoding) {
> +     case DRM_COLOR_YCBCR_BT601:
> +             if (color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> +                     csc = &coefs_yuv2rgb_bt601_full;
> +             else
> +                     csc = &coefs_yuv2rgb_bt601_lim;
> +             break;
> +     case DRM_COLOR_YCBCR_BT709:
> +             if (color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> +                     csc = &coefs_yuv2rgb_bt709_full;
> +             else
> +                     csc = &coefs_yuv2rgb_bt709_lim;
> +             break;
> +     default:
> +             DSSERR("Unsupported CSC mode %d for plane %d\n",
> +                    color_encoding, plane);
> +             return -EINVAL;
> +     }
>  
> -     for (i = 1; i < num_ovl; i++)
> -             dispc_ovl_write_color_conv_coef(dispc, i, 
> &coefs_yuv2rgb_bt601_lim);
> +     dispc_ovl_write_color_conv_coef(dispc, plane, csc);
>  
> -     if (dispc->feat->has_writeback)
> -             dispc_wb_write_color_conv_coef(dispc, &coefs_rgb2yuv_bt601_lim);
> +     return 0;
>  }
>  
>  static void dispc_ovl_set_ba0(struct dispc_device *dispc,
> @@ -2598,7 +2657,9 @@ static int dispc_ovl_setup_common(struct dispc_device 
> *dispc,
>                                 u8 pre_mult_alpha, u8 global_alpha,
>                                 enum omap_dss_rotation_type rotation_type,
>                                 bool replication, const struct videomode *vm,
> -                               bool mem_to_mem)
> +                               bool mem_to_mem,
> +                               enum drm_color_encoding color_encoding,
> +                               enum drm_color_range color_range)
>  {
>       bool five_taps = true;
>       bool fieldmode = false;
> @@ -2747,6 +2808,9 @@ static int dispc_ovl_setup_common(struct dispc_device 
> *dispc,
>                                     fieldmode, fourcc, rotation);
>               dispc_ovl_set_output_size(dispc, plane, out_width, out_height);
>               dispc_ovl_set_vid_color_conv(dispc, plane, cconv);
> +
> +             if (plane != OMAP_DSS_WB)
> +                     dispc_ovl_set_csc(dispc, plane, color_encoding, 
> color_range);
>       }
>  
>       dispc_ovl_set_rotation_attrs(dispc, plane, rotation, rotation_type,
> @@ -2783,7 +2847,8 @@ static int dispc_ovl_setup(struct dispc_device *dispc,
>               oi->screen_width, oi->pos_x, oi->pos_y, oi->width, oi->height,
>               oi->out_width, oi->out_height, oi->fourcc, oi->rotation,
>               oi->zorder, oi->pre_mult_alpha, oi->global_alpha,
> -             oi->rotation_type, replication, vm, mem_to_mem);
> +             oi->rotation_type, replication, vm, mem_to_mem,
> +             oi->color_encoding, oi->color_range);
>  
>       return r;
>  }
> @@ -2816,7 +2881,8 @@ static int dispc_wb_setup(struct dispc_device *dispc,
>               wi->buf_width, pos_x, pos_y, in_width, in_height, wi->width,
>               wi->height, wi->fourcc, wi->rotation, zorder,
>               wi->pre_mult_alpha, global_alpha, wi->rotation_type,
> -             replication, vm, mem_to_mem);
> +             replication, vm, mem_to_mem, DRM_COLOR_YCBCR_BT601,
> +             DRM_COLOR_YCBCR_LIMITED_RANGE);
>       if (r)
>               return r;
>  
> @@ -3948,7 +4014,8 @@ static void _omap_dispc_initial_config(struct 
> dispc_device *dispc)
>           dispc->feat->has_gamma_table)
>               REG_FLD_MOD(dispc, DISPC_CONFIG, 1, 9, 9);
>  
> -     dispc_setup_color_conv_coef(dispc);
> +     if (dispc->feat->has_writeback)
> +             dispc_wb_write_color_conv_coef(dispc, 
> &coefs_rgb2yuv_bt601_full);
>  
>       dispc_set_loadmode(dispc, OMAP_DSS_LOAD_FRAME_ONLY);
>  
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h 
> b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> index 79f6b195c7cf..1430cab6b877 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -14,6 +14,7 @@
>  #include <linux/platform_data/omapdss.h>
>  #include <uapi/drm/drm_mode.h>
>  #include <drm/drm_crtc.h>
> +#include <drm/drm_color_mgmt.h>

Alphabetical order ? While at is you coulde remove uapi/

>  
>  #define DISPC_IRQ_FRAMEDONE          (1 << 0)
>  #define DISPC_IRQ_VSYNC                      (1 << 1)
> @@ -243,6 +244,9 @@ struct omap_overlay_info {
>       u8 global_alpha;
>       u8 pre_mult_alpha;
>       u8 zorder;
> +
> +     enum drm_color_encoding color_encoding;
> +     enum drm_color_range color_range;
>  };
>  
>  struct omap_overlay_manager_info {
> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c 
> b/drivers/gpu/drm/omapdrm/omap_plane.c
> index 73ec99819a3d..db8e917260df 100644
> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
> @@ -59,6 +59,8 @@ static void omap_plane_atomic_update(struct drm_plane 
> *plane,
>               info.pre_mult_alpha = 1;
>       else
>               info.pre_mult_alpha = 0;
> +     info.color_encoding = state->color_encoding;
> +     info.color_range = state->color_range;
>  
>       /* update scanout: */
>       omap_framebuffer_update_scanout(state->fb, state, &info);
> @@ -189,6 +191,8 @@ static void omap_plane_reset(struct drm_plane *plane)
>        */
>       plane->state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
>                          ? 0 : omap_plane->id;
> +     plane->state->color_encoding = DRM_COLOR_YCBCR_BT601;
> +     plane->state->color_range = DRM_COLOR_YCBCR_FULL_RANGE;
>  }
>  
>  static int omap_plane_atomic_set_property(struct drm_plane *plane,
> @@ -232,6 +236,23 @@ static const struct drm_plane_funcs omap_plane_funcs = {
>       .atomic_get_property = omap_plane_atomic_get_property,
>  };
>  
> +static bool omap_plane_supports_yuv(struct drm_plane *plane)
> +{
> +     struct omap_drm_private *priv = plane->dev->dev_private;
> +     struct omap_plane *omap_plane = to_omap_plane(plane);
> +     const u32 *formats =
> +             priv->dispc_ops->ovl_get_color_modes(priv->dispc, 
> omap_plane->id);
> +     int i;

unsigned int i ?

> +
> +     for (i = 0; formats[i]; i++)
> +             if (formats[i] == DRM_FORMAT_YUYV ||
> +                 formats[i] == DRM_FORMAT_UYVY ||
> +                 formats[i] == DRM_FORMAT_NV12)
> +                     return true;
> +
> +     return false;
> +}
> +
>  static const char *plane_id_to_name[] = {
>       [OMAP_DSS_GFX] = "gfx",
>       [OMAP_DSS_VIDEO1] = "vid1",
> @@ -293,6 +314,15 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
>       drm_plane_create_blend_mode_property(plane, 
> BIT(DRM_MODE_BLEND_PREMULTI) |
>                                            BIT(DRM_MODE_BLEND_COVERAGE));
>  
> +     if (omap_plane_supports_yuv(plane))
> +             drm_plane_create_color_properties(plane,
> +                                     BIT(DRM_COLOR_YCBCR_BT601) |
> +                                     BIT(DRM_COLOR_YCBCR_BT709),
> +                                     BIT(DRM_COLOR_YCBCR_FULL_RANGE) |
> +                                     BIT(DRM_COLOR_YCBCR_LIMITED_RANGE),
> +                                     DRM_COLOR_YCBCR_BT601,
> +                                     DRM_COLOR_YCBCR_FULL_RANGE);
> +
>       return plane;
>  
>  error:

-- 
Regards,

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

Reply via email to