On 11/03/2014 04:31 AM, Philipp Zabel wrote: > 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? The color key passed to ipu_dp_set_chroma_key() has to be in one colorspace or the other, and the convention from elsewhere seems to be RGB24. The caller can't know what the colorspace of the Combining Unit in the DP is set to, so it can't be left to the caller to decide which colorspace to provide. So the point is, a choice has to be made and the best choice is an RGB24 color key, the DP can then convert to YUV444 if needed. > Regardless of the new feature, I like the move to static const > coefficient tables. Maybe split that into two patches? Sure, I'll do that. Steve > Signed-off-by: Steve Longerbeam <steve_longerbeam@xxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Steve Longerbeam | Senior Embedded Engineer, ESD Services Mentor Embedded(tm) | 46871 Bayside Parkway, Fremont, CA 94538 P 510.354.5838 | M 408.410.2735 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel