Hi Tomi,
Thank you for the patch.
On Monday, 12 February 2018 11:44:54 EET Tomi Valkeinen wrote:
> 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.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
> ---
> 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 ff09e2be470f..697274317f7c
> 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -345,11 +345,6 @@ static const struct {
> },
> };
>
> -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);
> @@ -841,9 +836,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;
If you made this a 3x3 matrix without explicit names for the fields I think
you wouldn't need two structure and two functions.
> + 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))
>
> @@ -853,7 +857,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);
true and false should be equal to 1 and 0 respectively, so the !! shouldn't be
needed.
> +
> +#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
> }
> @@ -862,20 +883,28 @@ static void dispc_setup_color_conv_coef(void)
> {
> int i;
> int num_ovl = dispc_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 */
The 517 turned into 516, was that intentional ?
> + 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)
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
