On 2025/09/11, Marius Vlad wrote:
> From: Derek Foreman <[email protected]>
>
> This adds YUV444 and Auto, which will fallback to RGB as per
> commit "drm: Pass supported color formats straight onto drm_bridge".
>
> Signed-off-by: Derek Foreman <[email protected]>
> Signed-off-by: Marius Vlad <[email protected]>
> ---
> .../gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 10 +++-
> drivers/gpu/drm/rockchip/inno_hdmi.c | 8 ++++
> drivers/gpu/drm/rockchip/rk3066_hdmi.c | 8 ++++
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 46 +++++++++++++++++++
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 2 +
> 5 files changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 58e24669ef34..794b8de9c9c5 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -427,6 +427,11 @@ static const struct of_device_id
> dw_hdmi_qp_rockchip_dt_ids[] = {
> };
> MODULE_DEVICE_TABLE(of, dw_hdmi_qp_rockchip_dt_ids);
>
> +static const u32 supported_colorformats =
> + DRM_COLOR_FORMAT_AUTO |
> + DRM_COLOR_FORMAT_RGB444 |
> + DRM_COLOR_FORMAT_YCBCR444;
> +
> static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device
> *master,
> void *data)
> {
> @@ -563,13 +568,16 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev,
> struct device *master,
> return ret;
> }
>
> - connector = drm_bridge_connector_init(drm, encoder,
> BIT(HDMI_COLORSPACE_RGB));
> + connector = drm_bridge_connector_init(drm, encoder,
> supported_colorformats);
The supported formats set to BIT(HDMI_COLORSPACE_RGB) initially as part
of drm_bridge_connector_init() and further moved out and passed as an
argument via "[PATCH 5/8] drm: Pass supported color formats straight
onto drm_bridge" is nothing but a default value to be used later on
drmm_connector_hdmi_init() invocation if the HDMI bridge in the display
pipeline (currently the framework allows only one bridge in a chain to
set DRM_BRIDGE_OP_HDMI) does not provide it's own supported_formats
before it gets attached to the encoder's chain. Otherwise it will be
simply overridden:
if (bridge->supported_formats)
supported_formats = bridge->supported_formats;
This is precisely the case with [1] handling platform supported formats
and color depth in DW HDMI QP library.
> if (IS_ERR(connector)) {
> ret = PTR_ERR(connector);
> dev_err(hdmi->dev, "failed to init bridge connector: %d\n",
> ret);
> return ret;
> }
>
> + if (!drm_mode_create_hdmi_color_format_property(connector,
> supported_colorformats))
> + drm_connector_attach_color_format_property(connector);
> +
> return drm_connector_attach_encoder(connector, encoder);
> }
[...]
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index 186f6452a7d3..5634e59a6412 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -1543,6 +1543,50 @@ static void vop2_dither_setup(struct drm_crtc *crtc,
> u32 *dsp_ctrl)
> DITHER_DOWN_ALLEGRO);
> }
>
> +static void vop2_bcsh_config(struct drm_crtc *crtc, struct vop2_video_port
> *vp)
> +{
> + struct drm_connector_list_iter conn_iter;
> + struct drm_connector *connector;
> + u32 format = 0;
> + enum drm_colorspace colorspace = 0;
> + u32 val = 0;
> +
> + drm_connector_list_iter_begin(crtc->dev, &conn_iter);
> + drm_for_each_connector_iter(connector, &conn_iter) {
> + if (!(crtc->state->connector_mask &
> drm_connector_mask(connector)))
> + continue;
> +
> + format = connector->state->color_format;
> + colorspace = connector->state->colorspace;
> + break;
> + }
> + drm_connector_list_iter_end(&conn_iter);
> +
> + if (format == DRM_COLOR_FORMAT_YCBCR420 ||
> + format == DRM_COLOR_FORMAT_YCBCR444 ||
> + format == DRM_COLOR_FORMAT_YCBCR422) {
> + val = RK3568_VP_BCSH_CTRL__BCSH_R2Y_EN | BIT(7);
> +
> + switch (colorspace) {
> + case DRM_MODE_COLORIMETRY_BT2020_RGB:
> + case DRM_MODE_COLORIMETRY_BT2020_YCC:
> + val |= BIT(7) | BIT(6);
> + break;
> + case DRM_MODE_COLORIMETRY_BT709_YCC:
> + val |= BIT(6);
> + break;
> + default:
> + break;
> + }
> + if (colorspace == DRM_MODE_COLORIMETRY_BT2020_RGB ||
> + colorspace == DRM_MODE_COLORIMETRY_BT2020_YCC)
While working on YUV420 output format support [2] I noticed VOP2 relies
on v4l2_colorspace instead of drm_colorspace, presumably that's just a
downstream inheritance. Moreover, it also defines a custom
vop_csc_format and provides the vop2_convert_csc_mode() helper to translate
the V4L2_COLORSPACE_* entries into the internal CSC_* representation, as
required for the actual HW programming.
I implemented the rockchip_drm_colorimetry_to_v4l_colorspace() utility
in [3] to convert drm_colorspace to v4l2_colorspace as a temporary
workaround to pass conn_state->colorspace via
dw_hdmi_qp_rockchip_encoder_atomic_check() in [2].
I think we should try first to clean this up before adding even more
colorspace related mess, i.e. if possible get rid of v4l2_colorspace and
rely exclusively on drm_colorspace while providing a conversion helper
to the internal vop_csc_format.
Alternatively, we could keep the rockchip changes in this series to the
bare minimum (i.e. RGB-only) and handle YUV separately, to avoid adding
here unnecessary and/or unrelated complexity. It's worth noting the
indicated YUV420 related work has been done on top of the high color
depth support series [1] for the required platform support, while in
turn it conflicts with the HDMI CEC series [4], hence added as a
dependency.
[1]
https://lore.kernel.org/all/[email protected]/
[2]
https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/6d78f42a5b88cf83cff771f7cccc44c8d67b9695
[3]
https://gitlab.collabora.com/hardware-enablement/rockchip-3588/linux/-/commit/9ce7c0c95ce024ee5799a3e5f776c98609683090
[4]
https://lore.kernel.org/all/[email protected]/
