Re: [PATCH v4 1/4] gpu: ipu-v3: ipu-ic: Rename yuv2rgb encoding matrices
From: Steve Longerbeam
Date: Mon Feb 11 2019 - 18:33:31 EST
On 2/11/19 1:58 AM, Philipp Zabel wrote:
On Fri, 2019-02-08 at 17:47 -0800, Steve Longerbeam wrote:
The ycbcr2rgb and inverse rgb2ycbcr matrices define the BT.601 encoding
coefficients, so rename them to indicate that. And add some comments
to make clear these are BT.601 coefficients encoding between YUV limited
range and RGB full range. The ic_csc_rgb2rgb matrix is just an identity
matrix, so rename to ic_csc_identity. No functional changes.
Signed-off-by: Steve Longerbeam <slongerbeam@xxxxxxxxx>
---
Changes in v2:
- rename ic_csc_rgb2rgb matrix to ic_csc_identity.
---
drivers/gpu/ipu-v3/ipu-ic.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-ic.c b/drivers/gpu/ipu-v3/ipu-ic.c
index 594c3cbc8291..3ef61f0b509b 100644
--- a/drivers/gpu/ipu-v3/ipu-ic.c
+++ b/drivers/gpu/ipu-v3/ipu-ic.c
@@ -183,11 +183,13 @@ struct ic_csc_params {
};
/*
+ * BT.601 encoding from RGB full range to YUV limited range:
+ *
* Y = R * .299 + G * .587 + B * .114;
* U = R * -.169 + G * -.332 + B * .500 + 128.;
* V = R * .500 + G * -.419 + B * -.0813 + 128.;
Hm, this is a conversion to full range BT.601. For limited range, the
matrix coefficients
0.2990ÂÂ0.5870ÂÂ0.1140
-0.1687 -0.3313ÂÂ0.5000
0.5000 -0.4187 -0.0813
should be multiplied with 219/255 (Y) and 224/255 (U,V), respectively:
Y = R * .2568Â+ G * .5041 + B * .0979 + 16;
U = R * -.1482 + G * -.2910Â+ B *Â .4392 + 128;
V = R * .4392 + G * -.3678 + B * -.0714 + 128;
*/
-static const struct ic_csc_params ic_csc_rgb2ycbcr = {
+static const struct ic_csc_params ic_csc_rgb2ycbcr_bt601 = {
.coeff = {
{ 77, 150, 29 },
{ 469, 427, 128 },
@@ -197,8 +199,11 @@ static const struct ic_csc_params ic_csc_rgb2ycbcr = {
.scale = 1,
};
-/* transparent RGB->RGB matrix for graphics combining */
-static const struct ic_csc_params ic_csc_rgb2rgb = {
+/*
+ * identity matrix, used for transparent RGB->RGB graphics
+ * combining.
+ */
+static const struct ic_csc_params ic_csc_identity = {
.coeff = {
{ 128, 0, 0 },
{ 0, 128, 0 },
@@ -208,11 +213,13 @@ static const struct ic_csc_params ic_csc_rgb2rgb = {
};
/*
+ * Inverse BT.601 encoding from YUV limited range to RGB full range:
+ *
* R = (1.164 * (Y - 16)) + (1.596 * (Cr - 128));
* G = (1.164 * (Y - 16)) - (0.392 * (Cb - 128)) - (0.813 * (Cr - 128));
* B = (1.164 * (Y - 16)) + (2.017 * (Cb - 128);
*/
This looks correct.
Agreed, the coefficients in the comments are correct, but the actual
table values were a bit off. I will correct them for v5 (Green offset
should be 272 in the table, not 266).
Steve
-static const struct ic_csc_params ic_csc_ycbcr2rgb = {
+static const struct ic_csc_params ic_csc_ycbcr2rgb_bt601 = {
.coeff = {
{ 149, 0, 204 },
{ 149, 462, 408 },
@@ -238,11 +245,11 @@ static int init_csc(struct ipu_ic *ic,
(priv->tpmem_base + ic->reg->tpmem_csc[csc_index]);
if (inf == IPUV3_COLORSPACE_YUV && outf == IPUV3_COLORSPACE_RGB)
- params = &ic_csc_ycbcr2rgb;
+ params = &ic_csc_ycbcr2rgb_bt601;
else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_YUV)
- params = &ic_csc_rgb2ycbcr;
+ params = &ic_csc_rgb2ycbcr_bt601;
else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_RGB)
- params = &ic_csc_rgb2rgb;
+ params = &ic_csc_identity;
else {
dev_err(priv->ipu->dev, "Unsupported color space conversion\n");
return -EINVAL;
regards
Philipp