| Message ID | aa2fb5c8681d87c489ebc471afd5e60133d8bbbb.1492768073.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jyri, Thank you for the patch. On Friday 21 Apr 2017 12:51:15 Jyri Sarha wrote: > From: Tomi Valkeinen <tomi.valkeinen@ti.com> > > The setup code for color space conversion is a bit messy. This patch > cleans it up. > > For some reason the TRM uses values in YCrCb order, which is also used > in the current driver, whereas everywhere else it's YCbCr (which also > matches YUV order). This patch changes the tables to use the common > order to avoid confusion. > > The tables are split into separate lines, and comments added for > clarity. > > WB color conversion registers are similar but different than non-WB, but > the same function was used to write both. It worked fine because the > coef table was adjusted accordingly, but that was rather confusing. This > patch adds a separate function to write the WB values so that the coef > table can be written in an understandable way. That's quite a few changes for a single patch. I might have split the last one in a separate patch. > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > drivers/gpu/drm/omapdrm/dss/dispc.c | 59 +++++++++++++++++++++++---------- > 1 file changed, 44 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c > b/drivers/gpu/drm/omapdrm/dss/dispc.c index 5ac0145..b53e63d 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dispc.c > +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c > @@ -293,11 +293,6 @@ struct dispc_gamma_desc { > }, > }; > > -struct color_conv_coef { > - int ry, rcr, rcb, gy, gcr, gcb, by, bcr, bcb; > - int full_range; > -}; > - > static unsigned long dispc_fclk_rate(void); > static unsigned long dispc_core_clk_rate(void); > static unsigned long dispc_mgr_lclk_rate(enum omap_channel channel); > @@ -757,9 +752,18 @@ static void dispc_ovl_set_scale_coef(enum omap_plane_id > plane, int fir_hinc, } > } > > +struct csc_coef_yuv2rgb { > + int ry, rcb, rcr, gy, gcb, gcr, by, bcb, bcr; > + bool full_range; > +}; > + > +struct csc_coef_rgb2yuv { > + int yr, yg, yb, cbr, cbg, cbb, crr, crg, crb; > + bool full_range; > +}; > > static void dispc_ovl_write_color_conv_coef(enum omap_plane_id plane, > - const struct color_conv_coef *ct) > + const struct csc_coef_yuv2rgb *ct) > { > #define CVAL(x, y) (FLD_VAL(x, 26, 16) | FLD_VAL(y, 10, 0)) > > @@ -769,7 +773,24 @@ static void dispc_ovl_write_color_conv_coef(enum > omap_plane_id plane, dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 3), > CVAL(ct->bcr, ct->by)); dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 4), > CVAL(0, ct->bcb)); > > - REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), ct->full_range, 11, 11); > + REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), !!ct->full_range, 11, 11); > + > +#undef CVAL > +} > + > +static void dispc_wb_write_color_conv_coef(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)) > + > + dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 0), CVAL(ct->yg, ct->yr)); > + dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 1), CVAL(ct->crr, ct->yb)); > + dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 2), CVAL(ct->crb, ct- >crg)); > + dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 3), CVAL(ct->cbg, ct- >cbr)); > + dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 4), CVAL(0, ct->cbb)); > + > + REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), !!ct->full_range, 11, 11); > > #undef CVAL > } > @@ -778,20 +799,28 @@ static void dispc_setup_color_conv_coef(void) > { > int i; > int num_ovl = dss_feat_get_num_ovls(); > - const struct color_conv_coef ctbl_bt601_5_ovl = { > - /* YUV -> RGB */ > - 298, 409, 0, 298, -208, -100, 298, 0, 517, 0, > + > + /* 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 */ You changed 517 to 516, was it intentional ? The commit message doesn't mention that modification. > + false, /* limited range */ > }; > - const struct color_conv_coef ctbl_bt601_5_wb = { > - /* RGB -> YUV */ > - 66, 129, 25, 112, -94, -18, -38, -74, 112, 0, > + > + /* 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 */ > }; > > for (i = 1; i < num_ovl; i++) > - dispc_ovl_write_color_conv_coef(i, &ctbl_bt601_5_ovl); > + dispc_ovl_write_color_conv_coef(i, &coefs_yuv2rgb_bt601_lim); > > if (dispc.feat->has_writeback) > - dispc_ovl_write_color_conv_coef(OMAP_DSS_WB, &ctbl_bt601_5_wb); > + dispc_wb_write_color_conv_coef(&coefs_rgb2yuv_bt601_lim); > } > > static void dispc_ovl_set_ba0(enum omap_plane_id plane, u32 paddr)
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c index 5ac0145..b53e63d 100644 --- a/drivers/gpu/drm/omapdrm/dss/dispc.c +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c @@ -293,11 +293,6 @@ struct dispc_gamma_desc { }, }; -struct color_conv_coef { - int ry, rcr, rcb, gy, gcr, gcb, by, bcr, bcb; - int full_range; -}; - static unsigned long dispc_fclk_rate(void); static unsigned long dispc_core_clk_rate(void); static unsigned long dispc_mgr_lclk_rate(enum omap_channel channel); @@ -757,9 +752,18 @@ static void dispc_ovl_set_scale_coef(enum omap_plane_id plane, int fir_hinc, } } +struct csc_coef_yuv2rgb { + int ry, rcb, rcr, gy, gcb, gcr, by, bcb, bcr; + bool full_range; +}; + +struct csc_coef_rgb2yuv { + int yr, yg, yb, cbr, cbg, cbb, crr, crg, crb; + bool full_range; +}; static void dispc_ovl_write_color_conv_coef(enum omap_plane_id plane, - const struct color_conv_coef *ct) + const struct csc_coef_yuv2rgb *ct) { #define CVAL(x, y) (FLD_VAL(x, 26, 16) | FLD_VAL(y, 10, 0)) @@ -769,7 +773,24 @@ static void dispc_ovl_write_color_conv_coef(enum omap_plane_id plane, dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 3), CVAL(ct->bcr, ct->by)); dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 4), CVAL(0, ct->bcb)); - REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), ct->full_range, 11, 11); + REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), !!ct->full_range, 11, 11); + +#undef CVAL +} + +static void dispc_wb_write_color_conv_coef(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)) + + dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 0), CVAL(ct->yg, ct->yr)); + dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 1), CVAL(ct->crr, ct->yb)); + dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 2), CVAL(ct->crb, ct->crg)); + dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 3), CVAL(ct->cbg, ct->cbr)); + dispc_write_reg(DISPC_OVL_CONV_COEF(plane, 4), CVAL(0, ct->cbb)); + + REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), !!ct->full_range, 11, 11); #undef CVAL } @@ -778,20 +799,28 @@ static void dispc_setup_color_conv_coef(void) { int i; int num_ovl = dss_feat_get_num_ovls(); - const struct color_conv_coef ctbl_bt601_5_ovl = { - /* YUV -> RGB */ - 298, 409, 0, 298, -208, -100, 298, 0, 517, 0, + + /* 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 color_conv_coef ctbl_bt601_5_wb = { - /* RGB -> YUV */ - 66, 129, 25, 112, -94, -18, -38, -74, 112, 0, + + /* 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 */ }; for (i = 1; i < num_ovl; i++) - dispc_ovl_write_color_conv_coef(i, &ctbl_bt601_5_ovl); + dispc_ovl_write_color_conv_coef(i, &coefs_yuv2rgb_bt601_lim); if (dispc.feat->has_writeback) - dispc_ovl_write_color_conv_coef(OMAP_DSS_WB, &ctbl_bt601_5_wb); + dispc_wb_write_color_conv_coef(&coefs_rgb2yuv_bt601_lim); } static void dispc_ovl_set_ba0(enum omap_plane_id plane, u32 paddr)