Re: [PATCH 21/21] OMAPDSS: DISPC: Configure color conversion coefficients for writeback

Tomi Valkeinen
2012-09-14T09:07:29Z

[-- Attachment #1: Type: text/plain, Size: 2648 bytes --]

On Thu, 2012-09-13 at 17:45 +0530, Archit Taneja wrote:
> Writeback pipeline receives RGB data from one of the overlays or one of the
> overlay managers. If the target color mode is YUV422 or NV12, we need to convert
> the RGB pixels to YUV. The scalar in WB then converts it to the target color
> mode.
>
> Hence, the color conversion coefficients that need to be programmed are the ones
> which convert a RGB24 pixel to YUV444. Program these coefficients for writeback
> pipeline.
>
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
> drivers/video/omap2/dss/dispc.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 1623c9b..5811a18 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -681,20 +681,30 @@ static void dispc_plane_set_scale_coef(enum omap_plane plane, int fir_hinc,
> static void _dispc_setup_color_conv_coef(void)
> {
> int i;
> + int num_ovl = dss_feat_get_num_ovls();
> + int num_wb = dss_feat_get_num_wbs();
> +
> const struct color_conv_coef {
> int ry, rcr, rcb, gy, gcr, gcb, by, bcr, bcb;
> int full_range;
> - } ctbl_bt601_5 = {
> - 298, 409, 0, 298, -208, -100, 298, 0, 517, 0,
> + } ctbl_bt601_5[2] = {
> + { 298, 409, 0, 298, -208, -100, 298, 0, 517, 0, },
> + { 66, 112, -38, 129, -94, -74, 25, -18, 112, 0, },
> };
>
> const struct color_conv_coef *ct;
>
> #define CVAL(x, y) (FLD_VAL(x, 26, 16) | FLD_VAL(y, 10, 0))
> +#define YUVTORGB 0
> +#define RGBTOYUV 1
>
> - ct = &ctbl_bt601_5;
> + ct = &ctbl_bt601_5[YUVTORGB];
> +
> + for (i = 1; i < num_ovl + num_wb; i++) {
> +
> + if (i >= num_ovl)
> + ct = &ctbl_bt601_5[RGBTOYUV];
>
> - for (i = 1; i < dss_feat_get_num_ovls(); i++) {
> dispc_write_reg(DISPC_OVL_CONV_COEF(i, 0),
> CVAL(ct->rcr, ct->ry));
> dispc_write_reg(DISPC_OVL_CONV_COEF(i, 1),

This looks a bit funny. I'd suggest to take the actual register writes
to a separate function, and have a separate tables for ovls and wb, and
have two for loops, first for ovls and then for wbs.

Btw, I wonder if we could consider WB as a single special entity, i.e.
no need for "num_wbs" or such. I know things may change in the future
HW, but I got the impression that adding overlays to DSS is costly, and
single WB is enough.

Then again, I'm not sure if handling only single WB would simplify much.
If you think it's no issue to have support for multiple WBs, perhaps we
can have it, just in case.

Tomi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]