Re: [PATCH 04/11] drm/rockchip: inno_hdmi: Correctly setup HDMI quantization range
From: Alex Bee
Date: Thu Dec 14 2023 - 11:35:07 EST
Hi Heiko, Hi Maxime,
Am 14.12.23 um 12:36 schrieb Maxime Ripard:
On Thu, Dec 14, 2023 at 12:17:34PM +0100, Heiko Stübner wrote:
Am Donnerstag, 14. Dezember 2023, 12:12:08 CET schrieb Alex Bee:
Hi Maxime
Am 14.12.23 um 08:56 schrieb Maxime Ripard:
Hi Alex,
Thanks for working on this!
On Wed, Dec 13, 2023 at 08:51:18PM +0100, Alex Bee wrote:
The display controller will always give full range RGB regardless of the
mode set, but HDMI requires certain modes to be transmitted in limited
range RGB. This is especially required for HDMI sinks which do not support
non-standard quantization ranges.
This enables color space conversion for those modes and sets the
quantization range accordingly in the AVI infoframe.
Fixes: 412d4ae6b7a5 ("drm/rockchip: hdmi: add Innosilicon HDMI support")
Signed-off-by: Alex Bee <knaerzche@xxxxxxxxx>
---
drivers/gpu/drm/rockchip/inno_hdmi.c | 40 ++++++++++++++++++++++------
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 345253e033c5..32626a75723c 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -33,6 +33,7 @@ struct hdmi_data_info {
unsigned int enc_in_format;
unsigned int enc_out_format;
unsigned int colorimetry;
+ bool rgb_limited_range;
};
struct inno_hdmi_i2c {
@@ -308,6 +309,18 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
else
frame.avi.colorspace = HDMI_COLORSPACE_RGB;
+ if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_RGB) {
+ drm_hdmi_avi_infoframe_quant_range(&frame.avi,
+ &hdmi->connector, mode,
+ hdmi->hdmi_data.rgb_limited_range ?
+ HDMI_QUANTIZATION_RANGE_LIMITED :
+ HDMI_QUANTIZATION_RANGE_FULL);
+ } else {
+ frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
+ frame.avi.ycc_quantization_range =
+ HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
+ }
+
return inno_hdmi_upload_frame(hdmi, rc, &frame, INFOFRAME_AVI, 0, 0, 0);
}
@@ -334,14 +347,22 @@ static int inno_hdmi_config_video_csc(struct inno_hdmi *hdmi)
if (data->enc_in_format == data->enc_out_format) {
if ((data->enc_in_format == HDMI_COLORSPACE_RGB) ||
(data->enc_in_format >= HDMI_COLORSPACE_YUV444)) {
- value = v_SOF_DISABLE | v_COLOR_DEPTH_NOT_INDICATED(1);
- hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL3, value);
-
- hdmi_modb(hdmi, HDMI_VIDEO_CONTRL,
- m_VIDEO_AUTO_CSC | m_VIDEO_C0_C2_SWAP,
- v_VIDEO_AUTO_CSC(AUTO_CSC_DISABLE) |
- v_VIDEO_C0_C2_SWAP(C0_C2_CHANGE_DISABLE));
- return 0;
+ if (data->enc_in_format == HDMI_COLORSPACE_RGB &&
+ data->enc_out_format == HDMI_COLORSPACE_RGB &&
+ hdmi->hdmi_data.rgb_limited_range) {
+ csc_mode = CSC_RGB_0_255_TO_RGB_16_235_8BIT;
+ auto_csc = AUTO_CSC_DISABLE;
+ c0_c2_change = C0_C2_CHANGE_DISABLE;
+ csc_enable = v_CSC_ENABLE;
+ } else {
+ value = v_SOF_DISABLE | v_COLOR_DEPTH_NOT_INDICATED(1);
+ hdmi_writeb(hdmi, HDMI_VIDEO_CONTRL3, value);
+ hdmi_modb(hdmi, HDMI_VIDEO_CONTRL,
+ m_VIDEO_AUTO_CSC | m_VIDEO_C0_C2_SWAP,
+ v_VIDEO_AUTO_CSC(AUTO_CSC_DISABLE) |
+ v_VIDEO_C0_C2_SWAP(C0_C2_CHANGE_DISABLE));
+ return 0;
+ }
}
}
@@ -458,6 +479,9 @@ static int inno_hdmi_setup(struct inno_hdmi *hdmi,
else
hdmi->hdmi_data.colorimetry = HDMI_COLORIMETRY_ITU_709;
+ hdmi->hdmi_data.rgb_limited_range =
+ drm_default_rgb_quant_range(mode) == HDMI_QUANTIZATION_RANGE_LIMITED;
+
This patch conflicts heavily with my inno_hdmi patches here (patches 22 to 38):
https://lore.kernel.org/dri-devel/20231207-kms-hdmi-connector-state-v5-0-6538e19d634d@xxxxxxxxxx/
I'm aware of that and I mentioned it in the cover letter. Your series is
not merged yet and it didn't get much feedback so far. What is the
status there? Especially because you are removing things from inno-hdmi
driver (which aren't really required to remove there) which will I have
to reintroduce.
Sadly I haven't found the time to look closer at Maxime's series so far,
but I got the impression that it separates into multiple cleanup steps
for a number of controllers.
Yeah, one of the previous version comment was to support more
controllers than vc4, which is fair. So I ended up reworking and
converting multiple controllers, but most of the clean up changes can be
applied outside of that series just fine.
I just didn't find someone to test / review them yet :)
I'm not exactly sure how to proceed here. Do you want me to:
- base my patches on top of Maxime's series and reintroduce csc things
(of course only those which touch inno-hdmi w/o the framwork changes)
- only apply the patches that do not touch csc things and base my
series on top of that
- adapt Maxime's patches so that RGB full to RGB limited stays in there
- wait with resend until Maxime's series is merged and reintroduce csc
after that
- something else
?
Please advise.
Thanks,
Alex
Maxime