[PATCH v2] drm/rockchip: dw_hdmi_qp: attach the HDMI Colorspace connector property

[PATCH v2] drm/rockchip: dw_hdmi_qp: attach the HDMI Colorspace connector property

[Igor Paunovic]

The RK3588 HDMI QP output never exposes the "Colorspace" connector
property, so userspace has no way to request BT.2020 colorimetry.

Everything needed to signal it is already in place. The connector comes
from drm_bridge_connector_init(), which calls drmm_connector_hdmi_init()
and attaches "max bpc", "HDR_OUTPUT_METADATA" and the output color
format, and the HDMI state helper feeds conn_state->colorspace into the
AVI infoframe colorimetry bits via hdmi_generate_avi_infoframe() and
drm_hdmi_avi_infoframe_colorimetry(). dw-hdmi-qp then emits the
infoframe through its hdmi_write_avi_infoframe hook. Only the property
itself is missing, so conn_state->colorspace stays at its default and
the AVI infoframe always reports "no colorimetry data".

The user-visible effect is that HDR is unavailable on every RK3588 HDMI
output. KWin (Plasma 6.x) requires "Colorspace" alongside
"HDR_OUTPUT_METADATA" and "max bpc" before it treats an output as
wide-gamut capable, so it never offers the HDR toggle.

Create and attach the property right after the bridge connector is
initialised, as vc4_hdmi already does. Passing 0 to
drm_mode_create_hdmi_colorspace_property() selects the full HDMI
colorspace set defined by the core.

Tested on an Orange Pi 5 Plus driving a Samsung Odyssey G70B: KWin now
offers the HDR toggle, the connector property switches to BT2020_RGB
once HDR is enabled, and the output runs 4K@144 at 10 bpc.

Signed-off-by: Igor Paunovic <royalnet026@gmail.com>
---
Changes in v2:
- Regenerated with git format-patch. v1 was a hand-assembled diff: it
  carried no "diff --git" header, no diffstat, and no function context on
  the @@ hunk header. That is the most likely reason it never showed up in
  patchwork and got no review in three weeks.
- Rebased onto drm-misc-next (97c03b32b28a), the right tree for this
  change. The hunk is unchanged apart from its new offset and the function
  context that format-patch adds.
- Reworded the commit message to lead with the user-visible effect and to
  name the exact core path that consumes conn_state->colorspace.
- Trimmed the in-code comment to a single line; the reasoning now lives in
  the commit message instead. No functional change from v1: the two API
  calls and their placement are the same.
- An automated review of v1 flagged that returning an error from
  dw_hdmi_qp_rockchip_bind() leaves hdmi->hpd_work queued. That gap
  pre-dates this patch and is shared by every existing error path in
  bind() (drmm_encoder_init(), dw_hdmi_qp_bind(),
  drm_bridge_connector_init()), since cancel_delayed_work_sync() runs only
  in dw_hdmi_qp_rockchip_unbind(), which the component framework does not
  call for a failed bind(). It is already being addressed in the HDMI 2.0
  series below ("Cancel pending HPD work on suspend", 30/69, and "Control
  the HPD IRQ line via the bridge HPD ops", 37/69), so this patch adds
  nothing for it.

v1: https://lore.kernel.org/all/20260714202850.40999-1-royalnet026@gmail.com/
HDMI 2.0 series: https://lore.kernel.org/all/20260731-dw-hdmi-qp-scramb-v10-0-294364b2cf15@collabora.com/

 drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
index 9c4cf68d79d5..35d04680d011 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
@@ -695,6 +695,13 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
 		return dev_err_probe(hdmi->dev, PTR_ERR(connector),
 				     "Failed to init bridge connector\n");
 
+	/* Passing 0 selects the full HDMI colorspace set defined by the core. */
+	ret = drm_mode_create_hdmi_colorspace_property(connector, 0);
+	if (ret)
+		return ret;
+
+	drm_connector_attach_colorspace_property(connector);
+
 	return 0;
 }
 

base-commit: 97c03b32b28a9f7f13f768f2b06e1eaafe850e66
-- 
2.43.0

Re: [PATCH v2] drm/rockchip: dw_hdmi_qp: attach the HDMI Colorspace connector property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free in HPD interrupt handler and workqueue due to mismatched devm and drmm lifecycles on bind error paths.
--

--- Patch [1]: [PATCH v2] drm/rockchip: dw_hdmi_qp: attach the HDMI Colorspace connector property ---
commit 2bdde0ebdaa05708637a2fd7def9b7106821d2d5
Author: Igor Paunovic <royalnet026@gmail.com>

drm/rockchip: dw_hdmi_qp: attach the HDMI Colorspace connector property

This commit attaches the HDMI Colorspace connector property to the RK3588
HDMI QP output, which allows userspace to request BT.2020 colorimetry. This
enables HDR support for every RK3588 HDMI output.

Link: https://lore.kernel.org/all/20260714202850.40999-1-royalnet026@gmail.com/
Link: https://lore.kernel.org/all/20260731-dw-hdmi-qp-scramb-v10-0-294364b2cf15@collabora.com/

> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 9c4cf68d79d50..35d04680d0113 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
[ ... ]
> @@ -695,6 +695,13 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  		return dev_err_probe(hdmi->dev, PTR_ERR(connector),
>  				     "Failed to init bridge connector\n");
>  
> +	/* Passing 0 selects the full HDMI colorspace set defined by the core. */
> +	ret = drm_mode_create_hdmi_colorspace_property(connector, 0);
> +	if (ret)
> +		return ret;
[Severity: High]
This is a pre-existing issue, but does adding this new error return path
introduce a use-after-free risk with the HPD interrupt handler?

As noted in the commit message, the uncancelled hpd_work queue is a known
issue on bind error paths, but the same problem applies to the IRQ handler.

In dw_hdmi_qp_rockchip_bind(), the hdmi struct is allocated via
drmm_kzalloc(), tying its lifetime to the DRM device:

	hdmi = drmm_kzalloc(drm, sizeof(*hdmi), GFP_KERNEL);

However, the interrupt is registered using devm_request_threaded_irq()
against the component's platform device (pdev->dev):

	ret = devm_request_threaded_irq(hdmi->dev, irq,
					cfg->ctrl_ops->hardirq_callback,
					cfg->ctrl_ops->irq_callback,
					IRQF_SHARED, "dw-hdmi-qp-hpd",
					hdmi);

If drm_mode_create_hdmi_colorspace_property() fails, the component bind
returns an error. This causes the master bind to fail and destroy the DRM
device, which frees the hdmi struct.

Because the component device itself remains successfully probed, its devm
resources (including the IRQ handler) are not rolled back. If the shared
IRQ line fires after this bind failure, will the kernel invoke the handler
with the freed hdmi pointer?

> +
> +	drm_connector_attach_colorspace_property(connector);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805153416.8459-1-royalnet026@gmail.com?part=1

Re: [PATCH v2] drm/rockchip: dw_hdmi_qp: attach the HDMI Colorspace connector property

[Igor Paunovic]

Hi,

Thank you for the review.

The lifetime mismatch is real and worth spelling out precisely: hdmi is
allocated with drmm_kzalloc() against the DRM device, while the HPD
interrupt is registered with devm_request_threaded_irq() against the
component's platform device, with hdmi as the cookie and IRQF_SHARED
set. The bind error paths that follow the IRQ registration predate this
patch: encoder init, dw_hdmi_qp_bind() and drm_bridge_connector_init()
can all return with the handler still registered. This patch adds one
more such return, which can only trigger if the property allocation
fails under memory pressure.

So the issue is pre-existing, correctly flagged as such, and this change
does not alter the exposure in any practical way. The clean fix looks
orthogonal to this patch: register the IRQ with plain
request_threaded_irq() and tie its release to the DRM device with
drmm_add_action_or_reset(), so the handler cannot outlive the structure
it dereferences. Happy to send that as a separate follow-up if the
maintainers think it is worth doing.

Thanks,
Igor

Re: [PATCH v2] drm/rockchip: dw_hdmi_qp: attach the HDMI Colorspace connector property

[Diederik de Haas]

On Wed Aug 5, 2026 at 5:33 PM CEST, Igor Paunovic wrote:
> The RK3588 HDMI QP output never exposes the "Colorspace" connector
> property, so userspace has no way to request BT.2020 colorimetry.
>
> Everything needed to signal it is already in place. The connector comes
> from drm_bridge_connector_init(), which calls drmm_connector_hdmi_init()
> and attaches "max bpc", "HDR_OUTPUT_METADATA" and the output color
> format, and the HDMI state helper feeds conn_state->colorspace into the
> AVI infoframe colorimetry bits via hdmi_generate_avi_infoframe() and
> drm_hdmi_avi_infoframe_colorimetry(). dw-hdmi-qp then emits the
> infoframe through its hdmi_write_avi_infoframe hook. Only the property
> itself is missing, so conn_state->colorspace stays at its default and
> the AVI infoframe always reports "no colorimetry data".
>
> The user-visible effect is that HDR is unavailable on every RK3588 HDMI
> output. KWin (Plasma 6.x) requires "Colorspace" alongside
> "HDR_OUTPUT_METADATA" and "max bpc" before it treats an output as
> wide-gamut capable, so it never offers the HDR toggle.
>
> Create and attach the property right after the bridge connector is
> initialised, as vc4_hdmi already does. Passing 0 to
> drm_mode_create_hdmi_colorspace_property() selects the full HDMI
> colorspace set defined by the core.
>
> Tested on an Orange Pi 5 Plus driving a Samsung Odyssey G70B: KWin now
> offers the HDR toggle, the connector property switches to BT2020_RGB
> once HDR is enabled, and the output runs 4K@144 at 10 bpc.
>
> Signed-off-by: Igor Paunovic <royalnet026@gmail.com>
> ---
> Changes in v2:
> - Regenerated with git format-patch. v1 was a hand-assembled diff: it
>   carried no "diff --git" header, no diffstat, and no function context on
>   the @@ hunk header. That is the most likely reason it never showed up in
>   patchwork and got no review in three weeks.
> - Rebased onto drm-misc-next (97c03b32b28a), the right tree for this
>   change. The hunk is unchanged apart from its new offset and the function
>   context that format-patch adds.
> - Reworded the commit message to lead with the user-visible effect and to
>   name the exact core path that consumes conn_state->colorspace.
> - Trimmed the in-code comment to a single line; the reasoning now lives in
>   the commit message instead. No functional change from v1: the two API
>   calls and their placement are the same.
> - An automated review of v1 flagged that returning an error from
>   dw_hdmi_qp_rockchip_bind() leaves hdmi->hpd_work queued. That gap
>   pre-dates this patch and is shared by every existing error path in
>   bind() (drmm_encoder_init(), dw_hdmi_qp_bind(),
>   drm_bridge_connector_init()), since cancel_delayed_work_sync() runs only
>   in dw_hdmi_qp_rockchip_unbind(), which the component framework does not
>   call for a failed bind(). It is already being addressed in the HDMI 2.0
>   series below ("Cancel pending HPD work on suspend", 30/69, and "Control
>   the HPD IRQ line via the bridge HPD ops", 37/69), so this patch adds
>   nothing for it.
>
> v1: https://lore.kernel.org/all/20260714202850.40999-1-royalnet026@gmail.com/
> HDMI 2.0 series: https://lore.kernel.org/all/20260731-dw-hdmi-qp-scramb-v10-0-294364b2cf15@collabora.com/

Explicitly adding the author of that patch set into the loop.

Cheers,
  Diederik

>  drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 9c4cf68d79d5..35d04680d011 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -695,6 +695,13 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>  		return dev_err_probe(hdmi->dev, PTR_ERR(connector),
>  				     "Failed to init bridge connector\n");
>  
> +	/* Passing 0 selects the full HDMI colorspace set defined by the core. */
> +	ret = drm_mode_create_hdmi_colorspace_property(connector, 0);
> +	if (ret)
> +		return ret;
> +
> +	drm_connector_attach_colorspace_property(connector);
> +
>  	return 0;
>  }
>  
>
> base-commit: 97c03b32b28a9f7f13f768f2b06e1eaafe850e66