| Message ID | 20201103080310.164453-6-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/omap: add color mgmt support | expand |
Hi Tomi and Jyri, Thank you for the patch. On Tue, Nov 03, 2020 at 10:03:10AM +0200, Tomi Valkeinen wrote: > From: Jyri Sarha <jsarha@ti.com> > > Adds support for COLOR_ENCODING and COLOR_RANGE properties to > omap_plane.c and dispc.c. The supported encodings and ranges are > presets are: > > For COLOR_ENCODING: > - YCbCr BT.601 (default) > - YCbCr BT.709 > > For COLOR_RANGE: > - YCbCr limited range > - YCbCr full range (default) > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/omapdrm/dss/dispc.c | 104 ++++++++++++++++---------- > drivers/gpu/drm/omapdrm/dss/omapdss.h | 4 + > drivers/gpu/drm/omapdrm/omap_plane.c | 30 ++++++++ > 3 files changed, 97 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c > index 48593932bddf..bf0c9d293077 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dispc.c > +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c > @@ -874,50 +874,67 @@ static void dispc_ovl_write_color_conv_coef(struct dispc_device *dispc, > #undef CVAL > } > > -static void dispc_wb_write_color_conv_coef(struct dispc_device *dispc, > - const struct csc_coef_rgb2yuv *ct) > -{ > - const enum omap_plane_id plane = OMAP_DSS_WB; > - > -#define CVAL(x, y) (FLD_VAL(x, 26, 16) | FLD_VAL(y, 10, 0)) > +/* YUV -> RGB, ITU-R BT.601, full range */ > +static const struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_full = { > + 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 */ > +}; > > - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 0), CVAL(ct->yg, ct->yr)); > - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 1), CVAL(ct->crr, ct->yb)); > - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 2), CVAL(ct->crb, ct->crg)); > - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 3), CVAL(ct->cbg, ct->cbr)); > - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 4), CVAL(0, ct->cbb)); > +/* YUV -> RGB, ITU-R BT.601, limited range */ > +static const 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 */ > +}; > > - REG_FLD_MOD(dispc, DISPC_OVL_ATTRIBUTES(plane), ct->full_range, 11, 11); > +/* YUV -> RGB, ITU-R BT.709, full range */ > +static const 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 */ > +}; > > -#undef CVAL > -} > +/* YUV -> RGB, ITU-R BT.709, limited range */ > +static const 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 */ > +}; > > -static void dispc_setup_color_conv_coef(struct dispc_device *dispc) > +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; Can this happen, given that the properties are created with only the above four combinations being allowed ? > + } > > - 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); Unless I'm mistaken, the writeback plane doesn't have the CSC matrix configured anymore. Is that intentional ? > + return 0; > } > > static void dispc_ovl_set_ba0(struct dispc_device *dispc, > @@ -2598,7 +2615,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 +2766,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 +2805,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 +2839,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; > > @@ -3927,8 +3951,6 @@ 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); > - > dispc_set_loadmode(dispc, OMAP_DSS_LOAD_FRAME_ONLY); > > dispc_init_fifos(dispc); > diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h > index 8e9a2019f173..816424eb2d41 100644 > --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h > +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h > @@ -7,6 +7,7 @@ > #ifndef __OMAP_DRM_DSS_H > #define __OMAP_DRM_DSS_H > > +#include <drm/drm_color_mgmt.h> > #include <drm/drm_crtc.h> > #include <drm/drm_mode.h> > #include <linux/device.h> > @@ -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..1f433fb5f207 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); > + u32 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:
On 30/11/2020 12:50, Laurent Pinchart wrote: > Hi Tomi and Jyri, > > Thank you for the patch. > > On Tue, Nov 03, 2020 at 10:03:10AM +0200, Tomi Valkeinen wrote: >> From: Jyri Sarha <jsarha@ti.com> >> >> Adds support for COLOR_ENCODING and COLOR_RANGE properties to >> omap_plane.c and dispc.c. The supported encodings and ranges are >> presets are: >> >> For COLOR_ENCODING: >> - YCbCr BT.601 (default) >> - YCbCr BT.709 >> >> For COLOR_RANGE: >> - YCbCr limited range >> - YCbCr full range (default) >> >> Signed-off-by: Jyri Sarha <jsarha@ti.com> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> drivers/gpu/drm/omapdrm/dss/dispc.c | 104 ++++++++++++++++---------- >> drivers/gpu/drm/omapdrm/dss/omapdss.h | 4 + >> drivers/gpu/drm/omapdrm/omap_plane.c | 30 ++++++++ >> 3 files changed, 97 insertions(+), 41 deletions(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c >> index 48593932bddf..bf0c9d293077 100644 >> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c >> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c >> @@ -874,50 +874,67 @@ static void dispc_ovl_write_color_conv_coef(struct dispc_device *dispc, >> #undef CVAL >> } >> >> -static void dispc_wb_write_color_conv_coef(struct dispc_device *dispc, >> - const struct csc_coef_rgb2yuv *ct) >> -{ >> - const enum omap_plane_id plane = OMAP_DSS_WB; >> - >> -#define CVAL(x, y) (FLD_VAL(x, 26, 16) | FLD_VAL(y, 10, 0)) >> +/* YUV -> RGB, ITU-R BT.601, full range */ >> +static const struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_full = { >> + 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 */ >> +}; >> >> - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 0), CVAL(ct->yg, ct->yr)); >> - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 1), CVAL(ct->crr, ct->yb)); >> - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 2), CVAL(ct->crb, ct->crg)); >> - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 3), CVAL(ct->cbg, ct->cbr)); >> - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 4), CVAL(0, ct->cbb)); >> +/* YUV -> RGB, ITU-R BT.601, limited range */ >> +static const 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 */ >> +}; >> >> - REG_FLD_MOD(dispc, DISPC_OVL_ATTRIBUTES(plane), ct->full_range, 11, 11); >> +/* YUV -> RGB, ITU-R BT.709, full range */ >> +static const 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 */ >> +}; >> >> -#undef CVAL >> -} >> +/* YUV -> RGB, ITU-R BT.709, limited range */ >> +static const 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 */ >> +}; >> >> -static void dispc_setup_color_conv_coef(struct dispc_device *dispc) >> +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; > > Can this happen, given that the properties are created with only the > above four combinations being allowed ? No, it shouldn't happen. Are you just asking, or are you suggesting that we shouldn't check if color_encoding is valid here? I don't usually check parameters which we can expect to be correct, but with we use switch/if with the parameter, we have to deal with the "else" case too. >> + } >> >> - 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); > > Unless I'm mistaken, the writeback plane doesn't have the CSC matrix > configured anymore. Is that intentional ? It's intentional. I should add it to the description. The driver doesn't support writeback, even if we have bits and pieces of writeback code in the dispc.c. I've been hoping to add WB support, so I haven't just removed them. However, here I didn't want to add new code for WB that I can't test, so I decided to just drop the WB case. Tomi
Hi Tomi, On Mon, Nov 30, 2020 at 01:53:25PM +0200, Tomi Valkeinen wrote: > On 30/11/2020 12:50, Laurent Pinchart wrote: > > On Tue, Nov 03, 2020 at 10:03:10AM +0200, Tomi Valkeinen wrote: > >> From: Jyri Sarha <jsarha@ti.com> > >> > >> Adds support for COLOR_ENCODING and COLOR_RANGE properties to > >> omap_plane.c and dispc.c. The supported encodings and ranges are > >> presets are: > >> > >> For COLOR_ENCODING: > >> - YCbCr BT.601 (default) > >> - YCbCr BT.709 > >> > >> For COLOR_RANGE: > >> - YCbCr limited range > >> - YCbCr full range (default) > >> > >> Signed-off-by: Jyri Sarha <jsarha@ti.com> > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > >> --- > >> drivers/gpu/drm/omapdrm/dss/dispc.c | 104 ++++++++++++++++---------- > >> drivers/gpu/drm/omapdrm/dss/omapdss.h | 4 + > >> drivers/gpu/drm/omapdrm/omap_plane.c | 30 ++++++++ > >> 3 files changed, 97 insertions(+), 41 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c > >> index 48593932bddf..bf0c9d293077 100644 > >> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c > >> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c > >> @@ -874,50 +874,67 @@ static void dispc_ovl_write_color_conv_coef(struct dispc_device *dispc, > >> #undef CVAL > >> } > >> > >> -static void dispc_wb_write_color_conv_coef(struct dispc_device *dispc, > >> - const struct csc_coef_rgb2yuv *ct) > >> -{ > >> - const enum omap_plane_id plane = OMAP_DSS_WB; > >> - > >> -#define CVAL(x, y) (FLD_VAL(x, 26, 16) | FLD_VAL(y, 10, 0)) > >> +/* YUV -> RGB, ITU-R BT.601, full range */ > >> +static const struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_full = { > >> + 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 */ > >> +}; > >> > >> - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 0), CVAL(ct->yg, ct->yr)); > >> - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 1), CVAL(ct->crr, ct->yb)); > >> - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 2), CVAL(ct->crb, ct->crg)); > >> - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 3), CVAL(ct->cbg, ct->cbr)); > >> - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 4), CVAL(0, ct->cbb)); > >> +/* YUV -> RGB, ITU-R BT.601, limited range */ > >> +static const 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 */ > >> +}; > >> > >> - REG_FLD_MOD(dispc, DISPC_OVL_ATTRIBUTES(plane), ct->full_range, 11, 11); > >> +/* YUV -> RGB, ITU-R BT.709, full range */ > >> +static const 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 */ > >> +}; > >> > >> -#undef CVAL > >> -} > >> +/* YUV -> RGB, ITU-R BT.709, limited range */ > >> +static const 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 */ > >> +}; > >> > >> -static void dispc_setup_color_conv_coef(struct dispc_device *dispc) > >> +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; > > > > Can this happen, given that the properties are created with only the > > above four combinations being allowed ? > > No, it shouldn't happen. Are you just asking, or are you suggesting that we shouldn't check if > color_encoding is valid here? > > I don't usually check parameters which we can expect to be correct, but with we use switch/if with > the parameter, we have to deal with the "else" case too. I use a default in that case, especially if it can avoid turning the function from void to int. > >> + } > >> > >> - 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); > > > > Unless I'm mistaken, the writeback plane doesn't have the CSC matrix > > configured anymore. Is that intentional ? > > It's intentional. I should add it to the description. > > The driver doesn't support writeback, even if we have bits and pieces of writeback code in the > dispc.c. I've been hoping to add WB support, so I haven't just removed them. However, here I didn't > want to add new code for WB that I can't test, so I decided to just drop the WB case. Sounds fair. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
On 30/11/2020 16:19, Laurent Pinchart wrote: >>>> + 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; >>> >>> Can this happen, given that the properties are created with only the >>> above four combinations being allowed ? >> >> No, it shouldn't happen. Are you just asking, or are you suggesting that we shouldn't check if >> color_encoding is valid here? >> >> I don't usually check parameters which we can expect to be correct, but with we use switch/if with >> the parameter, we have to deal with the "else" case too. > > I use a default in that case, especially if it can avoid turning the > function from void to int. Yes, that's true. I'll do the change. Tomi
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c index 48593932bddf..bf0c9d293077 100644 --- a/drivers/gpu/drm/omapdrm/dss/dispc.c +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c @@ -874,50 +874,67 @@ static void dispc_ovl_write_color_conv_coef(struct dispc_device *dispc, #undef CVAL } -static void dispc_wb_write_color_conv_coef(struct dispc_device *dispc, - const struct csc_coef_rgb2yuv *ct) -{ - const enum omap_plane_id plane = OMAP_DSS_WB; - -#define CVAL(x, y) (FLD_VAL(x, 26, 16) | FLD_VAL(y, 10, 0)) +/* YUV -> RGB, ITU-R BT.601, full range */ +static const struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_full = { + 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 */ +}; - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 0), CVAL(ct->yg, ct->yr)); - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 1), CVAL(ct->crr, ct->yb)); - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 2), CVAL(ct->crb, ct->crg)); - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 3), CVAL(ct->cbg, ct->cbr)); - dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 4), CVAL(0, ct->cbb)); +/* YUV -> RGB, ITU-R BT.601, limited range */ +static const 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 */ +}; - REG_FLD_MOD(dispc, DISPC_OVL_ATTRIBUTES(plane), ct->full_range, 11, 11); +/* YUV -> RGB, ITU-R BT.709, full range */ +static const 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 */ +}; -#undef CVAL -} +/* YUV -> RGB, ITU-R BT.709, limited range */ +static const 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 */ +}; -static void dispc_setup_color_conv_coef(struct dispc_device *dispc) +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 +2615,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 +2766,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 +2805,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 +2839,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; @@ -3927,8 +3951,6 @@ 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); - dispc_set_loadmode(dispc, OMAP_DSS_LOAD_FRAME_ONLY); dispc_init_fifos(dispc); diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h index 8e9a2019f173..816424eb2d41 100644 --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h @@ -7,6 +7,7 @@ #ifndef __OMAP_DRM_DSS_H #define __OMAP_DRM_DSS_H +#include <drm/drm_color_mgmt.h> #include <drm/drm_crtc.h> #include <drm/drm_mode.h> #include <linux/device.h> @@ -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..1f433fb5f207 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); + u32 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: