Re: [PATCH v3 6/6] drm/rockchip: dw_hdmi_qp: Advertise BT.709 and BT.2020 colorimetries

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/rockchip: missing s->color_space update in dw_hdmi_qp atomic_check()
--

commit 43d044058730a347faa3ac82bf806a1a71fe7cd1
Author: Jan Claußen <jan.claussen10@web.de>

drm/rockchip: dw_hdmi_qp: Advertise BT.709 and BT.2020 colorimetries

This commit populates plat_data.supported_colorspaces in
dw_hdmi_qp_rockchip_bind() to advertise BT.709 YCC, BT.2020 RGB, and
BT.2020 YCC colorimetries on the Rockchip DW HDMI QP connector.

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 9c4cf68d79d50..dbb87dfd000c2 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -605,6 +605,10 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  				      BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) |
>  				      BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422);
>  
> +	plat_data.supported_colorspaces = BIT(DRM_MODE_COLORIMETRY_BT709_YCC) |
> +				      BIT(DRM_MODE_COLORIMETRY_BT2020_RGB) |
> +				      BIT(DRM_MODE_COLORIMETRY_BT2020_YCC);
> +

[Severity: Medium]
When userspace selects DRM_MODE_COLORIMETRY_BT2020_YCC or
DRM_MODE_COLORIMETRY_BT2020_RGB while the HDMI output format is YCbCr
(DRM_OUTPUT_COLOR_FORMAT_YCBCR444 or DRM_OUTPUT_COLOR_FORMAT_YCBCR422),
should dw_hdmi_qp_rockchip_encoder_atomic_check() also update s->color_space
from conn_state->colorspace?

On RK3588 and RK3576, RGB-to-YCbCr conversion is performed by VOP2 when the
bridge requests a YUV bus format (such as MEDIA_BUS_FMT_YUV8_1X24,
MEDIA_BUS_FMT_YUV10_1X30, or MEDIA_BUS_FMT_UYVY8_1X16). Both
vop2_setup_csc_mode() and vop2_bgcolor_setup() in
drivers/gpu/drm/rockchip/rockchip_drm_vop2.c rely on vcstate->color_space to
choose between CSC_BT2020L (for V4L2_COLORSPACE_BT2020) and CSC_BT709L:

drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:vop2_setup_csc_mode() {
	int output_csc = vcstate->color_space;
	...
	} else if (!is_input_yuv && is_output_yuv) {
		y2r_en = false;
		r2y_en = true;
		csc_mode = vop2_convert_csc_mode(output_csc);
	}
}

drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:vop2_bgcolor_setup() {
	if (new_vcstate->yuv_overlay) {
		vop2_rgb16_to_yuv16(new_vcstate->color_space,
				    DRM_ARGB64_GETR(bgcolor),
				    DRM_ARGB64_GETG(bgcolor),
				    DRM_ARGB64_GETB(bgcolor),
				    &y, &cb, &cr);
}

Currently, dw_hdmi_qp_rockchip_encoder_atomic_check() never sets
s->color_space, and its early return check only compares tmds_char_rate,
output_bpc, and bus_format:

drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rockchip_encoder_atomic_check() {
	if (hdmi->tmds_char_rate == conn_state->hdmi.tmds_char_rate &&
	    s->output_bpc == conn_state->hdmi.output_bpc &&
	    s->bus_format == ingest_fmt)
		return 0;
	...
	if (!ret) {
		hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
		s->output_type = DRM_MODE_CONNECTOR_HDMIA;
		s->output_bpc = conn_state->hdmi.output_bpc;
		s->bus_format = ingest_fmt;
	}
}

Without updating s->color_space, won't VOP2 leave vcstate->color_space at 0
(V4L2_COLORSPACE_DEFAULT) and convert RGB planes and the background color to
YCbCr using the Rec. 709 matrix (CSC_BT709L) while the HDMI AVI InfoFrame
signals BT.2020 colorimetry to the sink?

>  	encoder = &hdmi->encoder.encoder;
>  	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018123037.19560-1-jan.claussen10@web.de?part=6