Re: [PATCH 07/72] gpu: ipu-v3: Add ipu_dp_set_chroma_key()

[Philipp Zabel <p.zabel@pengutronix.de>]

Am Freitag, den 31.10.2014, 15:53 -0700 schrieb Steve Longerbeam:
> Adds the function ipu_dp_set_chroma_key(), which sets up a color key
> value for a DP foreground plane.
> 
> ipu_dp_set_chroma_key() accepts a color key value in RGB24 format.
> If the combiner unit colorspace is YUV, the key must be converted
> to YUV444, using the same CSC coefficients as programmed in the DP.
> So pull out the CSC coefficients from ipu_dp_csc_init() to make
> them available to rgb24_to_yuv444() that converts to color key.

What is the rationale to disallow specifying the color key in YUV?
Regardless of the new feature, I like the move to static const
coefficient tables. Maybe split that into two patches?

regards
Philipp

> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
>  drivers/gpu/ipu-v3/ipu-dp.c |  121 ++++++++++++++++++++++++++++++++++++-------
>  include/video/imx-ipu-v3.h  |    1 +
>  2 files changed, 103 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/ipu-v3/ipu-dp.c b/drivers/gpu/ipu-v3/ipu-dp.c
> index 98686ed..e4026f1 100644
> --- a/drivers/gpu/ipu-v3/ipu-dp.c
> +++ b/drivers/gpu/ipu-v3/ipu-dp.c
> @@ -84,6 +84,52 @@ static inline struct ipu_flow *to_flow(struct ipu_dp *dp)
>  		return container_of(dp, struct ipu_flow, background);
>  }
>  
> +static const int rgb2yuv_coeff[5][3] = {
> +	{ 0x0099, 0x012d, 0x003a },
> +	{ 0x03a9, 0x0356, 0x0100 },
> +	{ 0x0100, 0x0329, 0x03d6 },
> +	{ 0x0000, 0x0200, 0x0200 }, /* B0, B1, B2 */
> +	{ 0x2,    0x2,    0x2 },    /* S0, S1, S2 */
> +};
> +
> +static const int yuv2rgb_coeff[5][3] = {
> +	{ 0x0095, 0x0000, 0x00cc },
> +	{ 0x0095, 0x03ce, 0x0398 },
> +	{ 0x0095, 0x00ff, 0x0000 },
> +	{ 0x3e42, 0x010a, 0x3dd6 }, /* B0,B1,B2 */
> +	{ 0x1,    0x1,    0x1 },    /* S0,S1,S2 */
> +};
> +
> +/*
> + * This is used to convert an RGB24 color key to YUV444, using
> + * the same CSC coefficients as programmed in the DP.
> + */
> +static u32 rgb24_to_yuv444(u32 rgb24)
> +{
> +	u32 red, green, blue;
> +	int i, c[3];
> +
> +	red   = (rgb24 >> 16) & 0xff;
> +	green = (rgb24 >>  8) & 0xff;
> +	blue  = (rgb24 >>  0) & 0xff;
> +
> +	for (i = 0; i < 3; i++) {
> +		c[i] = red * rgb2yuv_coeff[i][0];
> +		c[i] += green * rgb2yuv_coeff[i][1];
> +		c[i] += blue * rgb2yuv_coeff[i][2];
> +		c[i] /= 16;
> +		c[i] += rgb2yuv_coeff[3][i] * 4;
> +		c[i] += 8;
> +		c[i] /= 16;
> +		if (c[i] < 0)
> +			c[i] = 0;
> +		if (c[i] > 255)
> +			c[i] = 255;
> +	}
> +
> +	return (c[0] << 16) | (c[1] << 8) | c[2];
> +}
> +
>  int ipu_dp_set_global_alpha(struct ipu_dp *dp, bool enable,
>  		u8 alpha, bool bg_chan)
>  {
> @@ -120,6 +166,48 @@ int ipu_dp_set_global_alpha(struct ipu_dp *dp, bool enable,
>  }
>  EXPORT_SYMBOL_GPL(ipu_dp_set_global_alpha);
>  
> +/*
> + * The input color_key must always be RGB24. It will be converted to
> + * YUV444 if the pixel format to the Combining unit is YUV space.
> + */
> +int ipu_dp_set_chroma_key(struct ipu_dp *dp, bool enable, u32 color_key)
> +{
> +	struct ipu_flow *flow = to_flow(dp);
> +	struct ipu_dp_priv *priv = flow->priv;
> +	enum ipu_color_space combiner_cs;
> +	u32 reg;
> +
> +	mutex_lock(&priv->mutex);
> +
> +	if (flow->foreground.in_cs == flow->background.in_cs)
> +		combiner_cs = flow->foreground.in_cs;
> +	else
> +		combiner_cs = flow->out_cs;
> +
> +	if (combiner_cs == IPUV3_COLORSPACE_YUV)
> +		color_key = rgb24_to_yuv444(color_key);
> +
> +	color_key &= 0x00ffffff;
> +
> +	if (enable) {
> +		reg = readl(flow->base + DP_GRAPH_WIND_CTRL) & ~0x00FFFFFFL;
> +		writel(reg | color_key, flow->base + DP_GRAPH_WIND_CTRL);
> +
> +		reg = readl(flow->base + DP_COM_CONF);
> +		writel(reg | DP_COM_CONF_GWCKE, flow->base + DP_COM_CONF);
> +	} else {
> +		reg = readl(flow->base + DP_COM_CONF);
> +		writel(reg & ~DP_COM_CONF_GWCKE, flow->base + DP_COM_CONF);
> +	}
> +
> +	ipu_srm_dp_sync_update(priv->ipu);
> +
> +	mutex_unlock(&priv->mutex);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ipu_dp_set_chroma_key);
> +
>  int ipu_dp_set_window_pos(struct ipu_dp *dp, u16 x_pos, u16 y_pos)
>  {
>  	struct ipu_flow *flow = to_flow(dp);
> @@ -138,6 +226,7 @@ static void ipu_dp_csc_init(struct ipu_flow *flow,
>  		enum ipu_color_space out,
>  		u32 place)
>  {
> +	const int (*c)[3];
>  	u32 reg;
>  
>  	reg = readl(flow->base + DP_COM_CONF);
> @@ -148,25 +237,19 @@ static void ipu_dp_csc_init(struct ipu_flow *flow,
>  		return;
>  	}
>  
> -	if (in == IPUV3_COLORSPACE_RGB && out == IPUV3_COLORSPACE_YUV) {
> -		writel(0x099 | (0x12d << 16), flow->base + DP_CSC_A_0);
> -		writel(0x03a | (0x3a9 << 16), flow->base + DP_CSC_A_1);
> -		writel(0x356 | (0x100 << 16), flow->base + DP_CSC_A_2);
> -		writel(0x100 | (0x329 << 16), flow->base + DP_CSC_A_3);
> -		writel(0x3d6 | (0x0000 << 16) | (2 << 30),
> -				flow->base + DP_CSC_0);
> -		writel(0x200 | (2 << 14) | (0x200 << 16) | (2 << 30),
> -				flow->base + DP_CSC_1);
> -	} else {
> -		writel(0x095 | (0x000 << 16), flow->base + DP_CSC_A_0);
> -		writel(0x0cc | (0x095 << 16), flow->base + DP_CSC_A_1);
> -		writel(0x3ce | (0x398 << 16), flow->base + DP_CSC_A_2);
> -		writel(0x095 | (0x0ff << 16), flow->base + DP_CSC_A_3);
> -		writel(0x000 | (0x3e42 << 16) | (1 << 30),
> -				flow->base + DP_CSC_0);
> -		writel(0x10a | (1 << 14) | (0x3dd6 << 16) | (1 << 30),
> -				flow->base + DP_CSC_1);
> -	}
> +	if (in == IPUV3_COLORSPACE_RGB && out == IPUV3_COLORSPACE_YUV)
> +		c = rgb2yuv_coeff;
> +	else
> +		c = yuv2rgb_coeff;
> +
> +	writel(c[0][0] | (c[0][1] << 16), flow->base + DP_CSC_A_0);
> +	writel(c[0][2] | (c[1][0] << 16), flow->base + DP_CSC_A_1);
> +	writel(c[1][1] | (c[1][2] << 16), flow->base + DP_CSC_A_2);
> +	writel(c[2][0] | (c[2][1] << 16), flow->base + DP_CSC_A_3);
> +	writel(c[2][2] | (c[3][0] << 16) | (c[4][0] << 30),
> +	       flow->base + DP_CSC_0);
> +	writel(c[3][1] | (c[4][1] << 14) | (c[3][2] << 16) | (c[4][2] << 30),
> +	       flow->base + DP_CSC_1);
>  
>  	reg |= place;
>  
> diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
> index 03cda50..e878343 100644
> --- a/include/video/imx-ipu-v3.h
> +++ b/include/video/imx-ipu-v3.h
> @@ -273,6 +273,7 @@ int ipu_dp_setup_channel(struct ipu_dp *dp,
>  int ipu_dp_set_window_pos(struct ipu_dp *, u16 x_pos, u16 y_pos);
>  int ipu_dp_set_global_alpha(struct ipu_dp *dp, bool enable, u8 alpha,
>  		bool bg_chan);
> +int ipu_dp_set_chroma_key(struct ipu_dp *dp, bool enable, u32 color_key);
>  
>  /*
>   * IPU CMOS Sensor Interface (csi) functions


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel