* [PATCH] drm: bridge: dw-hdmi: Fix RGB to YUV color space conversion
@ 2021-11-06 13:00 Alex Bee
2021-11-10 20:20 ` Jernej Škrabec
0 siblings, 1 reply; 3+ messages in thread
From: Alex Bee @ 2021-11-06 13:00 UTC (permalink / raw)
To: Neil Armstrong, Andrzej Hajda, Robert Foss, Laurent Pinchart
Cc: Jonas Karlman, Jernej Skrabec, dri-devel, linux-kernel, Alex Bee
As per CEA-861 quantization range is always limited in case of YUV
output - indepentently which CEA mode it is or if it is an DMT mode.
This is already correctly setup in HDMI AVI inforame, but we always do
a RGB to YUV conversion which doesn't consider that RGB input can be
full range as well.
That leads to incorrect colors for all CEA modes except mode 1 for HDMI
and always for DVI.
To fix this, provide additional csc coefficents for converting from RGB
full range to EITU601/EITU709 limited range and rename the existing
arrays to clarify their meaning.
Signed-off-by: Alex Bee <knaerzche@gmail.com>
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 24 +++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 62ae63565d3a..1cba08b70091 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -80,13 +80,25 @@ static const u16 csc_coeff_rgb_out_eitu709[3][4] = {
{ 0x2000, 0x0000, 0x3b61, 0x7e25 }
};
-static const u16 csc_coeff_rgb_in_eitu601[3][4] = {
+static const u16 csc_coeff_rgb_full_in_eitu601[3][4] = {
+ { 0x2044, 0x106f, 0x0644, 0x0040 },
+ { 0xe677, 0x1c1c, 0xfd46, 0x0200 },
+ { 0xed60, 0xf685, 0x1c1c, 0x0200 }
+};
+
+static const u16 csc_coeff_rgb_limited_in_eitu601[3][4] = {
{ 0x2591, 0x1322, 0x074b, 0x0000 },
{ 0x6535, 0x2000, 0x7acc, 0x0200 },
{ 0x6acd, 0x7534, 0x2000, 0x0200 }
};
-static const u16 csc_coeff_rgb_in_eitu709[3][4] = {
+static const u16 csc_coeff_rgb_full_in_eitu709[3][4] = {
+ { 0x2750, 0x0baf, 0x03f8, 0x0040 },
+ { 0xe677, 0x1c1c, 0xfd6d, 0x0200 },
+ { 0xea55, 0xf98f, 0x1c1c, 0x0200 }
+};
+
+static const u16 csc_coeff_rgb_limted_in_eitu709[3][4] = {
{ 0x2dc5, 0x0d9b, 0x049e, 0x0000 },
{ 0x62f0, 0x2000, 0x7d11, 0x0200 },
{ 0x6756, 0x78ab, 0x2000, 0x0200 }
@@ -1023,9 +1035,13 @@ static void dw_hdmi_update_csc_coeffs(struct dw_hdmi *hdmi)
csc_coeff = &csc_coeff_rgb_out_eitu709;
} else if (is_input_rgb && !is_output_rgb) {
if (hdmi->hdmi_data.enc_out_encoding == V4L2_YCBCR_ENC_601)
- csc_coeff = &csc_coeff_rgb_in_eitu601;
+ csc_coeff = hdmi->hdmi_data.rgb_limited_range
+ ? &csc_coeff_rgb_limited_in_eitu601
+ : &csc_coeff_rgb_full_in_eitu601;
else
- csc_coeff = &csc_coeff_rgb_in_eitu709;
+ csc_coeff = hdmi->hdmi_data.rgb_limited_range
+ ? &csc_coeff_rgb_limted_in_eitu709
+ : &csc_coeff_rgb_full_in_eitu709;
csc_scale = 0;
} else if (is_input_rgb && is_output_rgb &&
hdmi->hdmi_data.rgb_limited_range) {
base-commit: 89636a06fa2ee7826a19c39c19a9bc99ab9340a9
--
2.30.2
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] drm: bridge: dw-hdmi: Fix RGB to YUV color space conversion
2021-11-06 13:00 [PATCH] drm: bridge: dw-hdmi: Fix RGB to YUV color space conversion Alex Bee
@ 2021-11-10 20:20 ` Jernej Škrabec
2021-11-10 21:26 ` Jernej Škrabec
0 siblings, 1 reply; 3+ messages in thread
From: Jernej Škrabec @ 2021-11-10 20:20 UTC (permalink / raw)
To: Neil Armstrong, Andrzej Hajda, Robert Foss, Laurent Pinchart, Alex Bee
Cc: Jonas Karlman, dri-devel, linux-kernel, Alex Bee
Hi Alex,
Dne sobota, 06. november 2021 ob 14:00:44 CET je Alex Bee napisal(a):
> As per CEA-861 quantization range is always limited in case of YUV
> output - indepentently which CEA mode it is or if it is an DMT mode.
>
> This is already correctly setup in HDMI AVI inforame, but we always do
> a RGB to YUV conversion which doesn't consider that RGB input can be
> full range as well.
> That leads to incorrect colors for all CEA modes except mode 1 for HDMI
> and always for DVI.
>
> To fix this, provide additional csc coefficents for converting from RGB
> full range to EITU601/EITU709 limited range and rename the existing
> arrays to clarify their meaning.
>
> Signed-off-by: Alex Bee <knaerzche@gmail.com>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 24 +++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/
bridge/synopsys/dw-hdmi.c
> index 62ae63565d3a..1cba08b70091 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -80,13 +80,25 @@ static const u16 csc_coeff_rgb_out_eitu709[3][4] = {
> { 0x2000, 0x0000, 0x3b61, 0x7e25 }
> };
>
> -static const u16 csc_coeff_rgb_in_eitu601[3][4] = {
> +static const u16 csc_coeff_rgb_full_in_eitu601[3][4] = {
I would rather move "full" and "limited" to the end, since RGB always has full
range and we want YUV to have full or limited range.
Just one observation - no other matrix sets bit 15 in any coefficient, but yours
do. I can't see anywhere documented if bit 15 is ignored or not. Can you try
with it set to 0? If it still works, I would set it to 0 for consistency.
Best regards,
Jernej
> + { 0x2044, 0x106f, 0x0644, 0x0040 },
> + { 0xe677, 0x1c1c, 0xfd46, 0x0200 },
> + { 0xed60, 0xf685, 0x1c1c, 0x0200 }
> +};
> +
> +static const u16 csc_coeff_rgb_limited_in_eitu601[3][4] = {
> { 0x2591, 0x1322, 0x074b, 0x0000 },
> { 0x6535, 0x2000, 0x7acc, 0x0200 },
> { 0x6acd, 0x7534, 0x2000, 0x0200 }
> };
>
> -static const u16 csc_coeff_rgb_in_eitu709[3][4] = {
> +static const u16 csc_coeff_rgb_full_in_eitu709[3][4] = {
> + { 0x2750, 0x0baf, 0x03f8, 0x0040 },
> + { 0xe677, 0x1c1c, 0xfd6d, 0x0200 },
> + { 0xea55, 0xf98f, 0x1c1c, 0x0200 }
> +};
> +
> +static const u16 csc_coeff_rgb_limted_in_eitu709[3][4] = {
> { 0x2dc5, 0x0d9b, 0x049e, 0x0000 },
> { 0x62f0, 0x2000, 0x7d11, 0x0200 },
> { 0x6756, 0x78ab, 0x2000, 0x0200 }
> @@ -1023,9 +1035,13 @@ static void dw_hdmi_update_csc_coeffs(struct dw_hdmi
*hdmi)
> csc_coeff = &csc_coeff_rgb_out_eitu709;
> } else if (is_input_rgb && !is_output_rgb) {
> if (hdmi->hdmi_data.enc_out_encoding ==
V4L2_YCBCR_ENC_601)
> - csc_coeff = &csc_coeff_rgb_in_eitu601;
> + csc_coeff = hdmi->hdmi_data.rgb_limited_range
> + ? &csc_coeff_rgb_limited_in_eitu601
> + : &csc_coeff_rgb_full_in_eitu601;
> else
> - csc_coeff = &csc_coeff_rgb_in_eitu709;
> + csc_coeff = hdmi->hdmi_data.rgb_limited_range
> + ? &csc_coeff_rgb_limted_in_eitu709
> + : &csc_coeff_rgb_full_in_eitu709;
> csc_scale = 0;
> } else if (is_input_rgb && is_output_rgb &&
> hdmi->hdmi_data.rgb_limited_range) {
>
> base-commit: 89636a06fa2ee7826a19c39c19a9bc99ab9340a9
> --
> 2.30.2
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] drm: bridge: dw-hdmi: Fix RGB to YUV color space conversion
2021-11-10 20:20 ` Jernej Škrabec
@ 2021-11-10 21:26 ` Jernej Škrabec
0 siblings, 0 replies; 3+ messages in thread
From: Jernej Škrabec @ 2021-11-10 21:26 UTC (permalink / raw)
To: Neil Armstrong, Andrzej Hajda, Robert Foss, Laurent Pinchart, Alex Bee
Cc: Jonas Karlman, dri-devel, linux-kernel, Alex Bee
Dne sreda, 10. november 2021 ob 21:20:46 CET je Jernej Škrabec napisal(a):
> Hi Alex,
>
> Dne sobota, 06. november 2021 ob 14:00:44 CET je Alex Bee napisal(a):
> > As per CEA-861 quantization range is always limited in case of YUV
> > output - indepentently which CEA mode it is or if it is an DMT mode.
> >
> > This is already correctly setup in HDMI AVI inforame, but we always do
> > a RGB to YUV conversion which doesn't consider that RGB input can be
> > full range as well.
> > That leads to incorrect colors for all CEA modes except mode 1 for HDMI
> > and always for DVI.
> >
> > To fix this, provide additional csc coefficents for converting from RGB
> > full range to EITU601/EITU709 limited range and rename the existing
> > arrays to clarify their meaning.
> >
> > Signed-off-by: Alex Bee <knaerzche@gmail.com>
> > ---
> >
> > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 24 +++++++++++++++++++----
> > 1 file changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/
>
> bridge/synopsys/dw-hdmi.c
>
> > index 62ae63565d3a..1cba08b70091 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -80,13 +80,25 @@ static const u16 csc_coeff_rgb_out_eitu709[3][4] = {
> >
> > { 0x2000, 0x0000, 0x3b61, 0x7e25 }
> >
> > };
> >
> > -static const u16 csc_coeff_rgb_in_eitu601[3][4] = {
> > +static const u16 csc_coeff_rgb_full_in_eitu601[3][4] = {
>
> I would rather move "full" and "limited" to the end, since RGB always has
> full range and we want YUV to have full or limited range.
>
> Just one observation - no other matrix sets bit 15 in any coefficient, but
> yours do. I can't see anywhere documented if bit 15 is ignored or not. Can
> you try with it set to 0? If it still works, I would set it to 0 for
> consistency.
>
> Best regards,
> Jernej
>
> > + { 0x2044, 0x106f, 0x0644, 0x0040 },
> > + { 0xe677, 0x1c1c, 0xfd46, 0x0200 },
By my calculations, above line should be:
{ 0xe876, 0x1c1c, 0xfb6e, 0x0200}
Can you check again?
> > + { 0xed60, 0xf685, 0x1c1c, 0x0200 }
> > +};
> > +
> > +static const u16 csc_coeff_rgb_limited_in_eitu601[3][4] = {
> >
> > { 0x2591, 0x1322, 0x074b, 0x0000 },
> > { 0x6535, 0x2000, 0x7acc, 0x0200 },
> > { 0x6acd, 0x7534, 0x2000, 0x0200 }
> >
> > };
> >
> > -static const u16 csc_coeff_rgb_in_eitu709[3][4] = {
> > +static const u16 csc_coeff_rgb_full_in_eitu709[3][4] = {
> > + { 0x2750, 0x0baf, 0x03f8, 0x0040 },
> > + { 0xe677, 0x1c1c, 0xfd6d, 0x0200 },
> > + { 0xea55, 0xf98f, 0x1c1c, 0x0200 }
> > +};
> > +
> > +static const u16 csc_coeff_rgb_limted_in_eitu709[3][4] = {
> >
> > { 0x2dc5, 0x0d9b, 0x049e, 0x0000 },
> > { 0x62f0, 0x2000, 0x7d11, 0x0200 },
> > { 0x6756, 0x78ab, 0x2000, 0x0200 }
> >
> > @@ -1023,9 +1035,13 @@ static void dw_hdmi_update_csc_coeffs(struct
> > dw_hdmi
>
> *hdmi)
>
> > csc_coeff = &csc_coeff_rgb_out_eitu709;
> >
> > } else if (is_input_rgb && !is_output_rgb) {
> >
> > if (hdmi->hdmi_data.enc_out_encoding ==
>
> V4L2_YCBCR_ENC_601)
>
> > - csc_coeff = &csc_coeff_rgb_in_eitu601;
> > + csc_coeff = hdmi->hdmi_data.rgb_limited_range
> > + ?
&csc_coeff_rgb_limited_in_eitu601
> > + : &csc_coeff_rgb_full_in_eitu601;
> >
> > else
> >
> > - csc_coeff = &csc_coeff_rgb_in_eitu709;
> > + csc_coeff = hdmi->hdmi_data.rgb_limited_range
> > + ? &csc_coeff_rgb_limted_in_eitu709
> > + : &csc_coeff_rgb_full_in_eitu709;
> >
> > csc_scale = 0;
> >
> > } else if (is_input_rgb && is_output_rgb &&
> >
> > hdmi->hdmi_data.rgb_limited_range) {
> >
> > base-commit: 89636a06fa2ee7826a19c39c19a9bc99ab9340a9
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-10 21:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-06 13:00 [PATCH] drm: bridge: dw-hdmi: Fix RGB to YUV color space conversion Alex Bee
2021-11-10 20:20 ` Jernej Škrabec
2021-11-10 21:26 ` Jernej Škrabec
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®