[PATCH v4 0/2] Add "link bpc" DRM property

[PATCH v4 0/2] Add "link bpc" DRM property

[Nicolas Frattaroli]

I'm taking over this series from Marius Vlad.

The series adds a new "link bpc" DRM property. It reflects the display
link's actual achieved output bits per component, considering any
degradation of the bit depth done by drivers for bandwidth or other
reasons. The property's value is updated during an atomic commit, which
is also when it fires an uevent if it changed to let userspace know.

There's a weston implementation at [1] which makes use of this new
property to warn when a user's requested bpc could not be reached.

[1]: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1850

---
Changes in v4:
- Throw out the workqueue and delayed work item
- Remove the drm_connector_update_link_bpc_state function
- Reimplement it by updating the property and firing the uevent in
  commit_tail
- Check that the provided max_bpc value in attach_link_bpc_property is
  within the expected range
- Clamp the connector state's link_bpc value between 8 and max_bpc so
  that no value outside the declared range is ever written to the drm
  property
- Update and reword doc strings
- Add an amdgpu implementation
- Link to v3: https://lore.kernel.org/r/20251022162843.1759-1-marius.vlad@collabora.com/T/

Changes in v3:
- remove VRR mention from commit description (Ville)
- add DRM_MODE_PROP_IMMUTABLE to flags (Ville)
- provide helpers functions for drivers to use (can be used by other
  types of connectors, not just HDMI)
- send uevent informating userspace when 'link bpc' connector state
  changed (Daniel @ https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1850)
- added missing doc entry
- Link to v2: https://lore.kernel.org/r/20251006083043.3115-1-marius.vlad@collabora.com/T/

Changes in v2:
- replace return with EBUSY if connector already exists (Dmitry)
- add i-g-t test and an implementation for Weston (Dmitry)
- re-wording patch description (Jani)
- Link to v1: https://lore.kernel.org/r/20250801101750.1726-1-marius.vlad@collabora.com/T/

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

---
Nicolas Frattaroli (2):
      drm/connector: hdmi: Add a 'link bpc' property
      drm/amd/display: Add support for 'link bpc' property

 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++-
 drivers/gpu/drm/display/drm_hdmi_state_helper.c   |  2 +
 drivers/gpu/drm/drm_atomic_helper.c               |  9 +++
 drivers/gpu/drm/drm_atomic_uapi.c                 |  2 +
 drivers/gpu/drm/drm_connector.c                   | 86 +++++++++++++++++++++++
 include/drm/drm_connector.h                       | 16 +++++
 6 files changed, 129 insertions(+), 1 deletion(-)
---
base-commit: 9a6bac4a4a289d3ac043f885758d208ccf07f149
change-id: 20260309-link-bpc-d0afc475ac49

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH v4 1/2] drm/connector: hdmi: Add a 'link bpc' property

[Nicolas Frattaroli]

Display drivers may degrade from higher bit depths to lower bit depths
for reasons such as bandwidth constraints. Userspace applications, such
as compositors, may wish to know that this has occurred transparently,
instead of assuming that the "max bpc" they requested could be reached.

Introduce a new immutable DRM property called "link bpc" that reflects
the current display link's bits-per-component. An uevent is fired when
the link bpc value changes.

Set the new link_bpc member in the HDMI state helper to its HDMI output
bpc, so that during the next commit, the property is updated accordingly
and the uevent is fired.

Co-developed-by: Derek Foreman <derek.foreman@collabora.com>
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Co-developed-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c |  2 +
 drivers/gpu/drm/drm_atomic_helper.c             |  9 +++
 drivers/gpu/drm/drm_atomic_uapi.c               |  2 +
 drivers/gpu/drm/drm_connector.c                 | 86 +++++++++++++++++++++++++
 include/drm/drm_connector.h                     | 16 +++++
 5 files changed, 115 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index a1d16762ac7a..40648574f5e5 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -865,6 +865,8 @@ int drm_atomic_helper_connector_hdmi_check(struct drm_connector *connector,
 		struct drm_crtc *crtc = new_conn_state->crtc;
 		struct drm_crtc_state *crtc_state;
 
+		new_conn_state->link_bpc = new_conn_state->hdmi.output_bpc;
+
 		crtc_state = drm_atomic_get_crtc_state(state, crtc);
 		if (IS_ERR(crtc_state))
 			return PTR_ERR(crtc_state);
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 26953ed6b53e..4a932f543ac7 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2033,9 +2033,11 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_tail_rpm);
 
 static void commit_tail(struct drm_atomic_state *state)
 {
+	struct drm_connector_state *old_conn_state, *new_conn_state;
 	struct drm_device *dev = state->dev;
 	const struct drm_mode_config_helper_funcs *funcs;
 	struct drm_crtc_state *new_crtc_state;
+	struct drm_connector *connector;
 	struct drm_crtc *crtc;
 	ktime_t start;
 	s64 commit_time_ms;
@@ -2059,6 +2061,13 @@ static void commit_tail(struct drm_atomic_state *state)
 
 	drm_atomic_helper_wait_for_dependencies(state);
 
+	for_each_oldnew_connector_in_state(state, connector, old_conn_state,
+					   new_conn_state, i) {
+		if (old_conn_state->link_bpc != new_conn_state->link_bpc)
+			drm_connector_update_link_bpc_property(connector,
+							       new_conn_state);
+	}
+
 	/*
 	 * We cannot safely access new_crtc_state after
 	 * drm_atomic_helper_commit_hw_done() so figure out which crtc's have
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 87de41fb4459..3e8b4b795512 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1016,6 +1016,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = state->privacy_screen_sw_state;
 	} else if (property == connector->broadcast_rgb_property) {
 		*val = state->hdmi.broadcast_rgb;
+	} else if (property == connector->link_bpc_property) {
+		*val = state->link_bpc;
 	} else if (connector->funcs->atomic_get_property) {
 		return connector->funcs->atomic_get_property(connector,
 				state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index e70699c59c43..572894dad4bf 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -542,6 +542,75 @@ int drmm_connector_init(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drmm_connector_init);
 
+/**
+ * drm_connector_attach_link_bpc_property - create and attach 'link bpc' property
+ * @connector: drm connector
+ * @max_bpc: specify the upper limit, matching  that of 'max bpc' property
+ *
+ * Create and attach the 'link bpc' DRM property on @connector with an upper
+ * limit of @max_bpc.
+ *
+ * Returns:
+ * 0 on success, negative errno on failure.
+ */
+int
+drm_connector_attach_link_bpc_property(struct drm_connector *connector,
+				       unsigned int max_bpc)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+
+	if (connector->link_bpc_property)
+		return -EBUSY;
+
+	if (max_bpc < 8 || max_bpc > U8_MAX)
+		return -EINVAL;
+
+	prop = drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE,
+					 "link bpc", 8, max_bpc);
+	if (!prop)
+		return -ENOMEM;
+
+	connector->link_bpc_property = prop;
+
+	drm_object_attach_property(&connector->base, prop, max_bpc);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_link_bpc_property);
+
+/**
+ * drm_connector_update_link_bpc_property - update the 'link bpc' property of a
+ *                                          connector and fire uevent
+ * @connector: pointer to the &struct drm_connector
+ * @state: pointer to the &struct drm_connector_state with the new value
+ *
+ * Update the 'link bpc' property of the given @connector to the
+ * &drm_connector_state.link_bpc member's value of @state and fire a uevent.
+ */
+void
+drm_connector_update_link_bpc_property(struct drm_connector *connector,
+				       struct drm_connector_state *state)
+{
+	u8 bpc = clamp(state->link_bpc, 8, state->max_bpc);
+
+	if (!connector->link_bpc_property)
+		return;
+
+	if (bpc != state->link_bpc)
+		drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] Clamping link bpc from %u to %u\n",
+			    connector->base.id, connector->name, state->link_bpc, bpc);
+
+	drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] Setting state link bpc %u\n",
+				     connector->base.id, connector->name, bpc);
+	drm_object_property_set_value(&connector->base, connector->link_bpc_property,
+				      bpc);
+
+	drm_sysfs_connector_property_event(connector,
+					   connector->link_bpc_property);
+}
+EXPORT_SYMBOL(drm_connector_update_link_bpc_property);
+
 /**
  * drmm_connector_hdmi_init - Init a preallocated HDMI connector
  * @dev: DRM device
@@ -624,6 +693,10 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
 	drm_connector_attach_max_bpc_property(connector, 8, max_bpc);
 	connector->max_bpc = max_bpc;
 
+	ret = drm_connector_attach_link_bpc_property(connector, max_bpc);
+	if (ret)
+		return ret;
+
 	if (max_bpc > 8)
 		drm_connector_attach_hdr_output_metadata_property(connector);
 
@@ -1713,6 +1786,19 @@ EXPORT_SYMBOL(drm_hdmi_connector_get_output_format_name);
  *	drm_connector_attach_max_bpc_property() to create and attach the
  *	property to the connector during initialization.
  *
+ * link bpc:
+ *	This immutable range property can be used by userspace to determine the
+ *	current display link's bit depth. Drivers can use
+ *	drm_connector_attach_link_bpc_property() to create and attach the
+ *	property to the connector during initialization. They can then set the
+ *	&drm_connector_state.link_bpc member to the actual output bit depth
+ *	after any degradation. The drm property will be updated to this member's
+ *	value on the next atomic commit, and if it changed, a uevent will be
+ *	fired.
+ *	Userspace can listen to the uevent to be notified of link bpc changes,
+ *	and compare the property's value to what userspace requested to
+ *	determine whether colour depth has been degraded.
+ *
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index c18be8c19de0..3c339a9840c9 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1132,6 +1132,11 @@ struct drm_connector_state {
 	 */
 	u8 max_bpc;
 
+	/**
+	 * @link_bpc: Current display link's bits-per-component.
+	 */
+	u8 link_bpc;
+
 	/**
 	 * @privacy_screen_sw_state: See :ref:`Standard Connector
 	 * Properties<standard_connector_properties>`
@@ -2124,6 +2129,12 @@ struct drm_connector {
 	 */
 	struct drm_property *max_bpc_property;
 
+	/**
+	 * @link_bpc_property: Connector property that reflects the current
+	 * output bits per component.
+	 */
+	struct drm_property *link_bpc_property;
+
 	/** @privacy_screen: drm_privacy_screen for this connector, or NULL. */
 	struct drm_privacy_screen *privacy_screen;
 
@@ -2534,6 +2545,11 @@ void drm_connector_attach_privacy_screen_provider(
 	struct drm_connector *connector, struct drm_privacy_screen *priv);
 void drm_connector_update_privacy_screen(const struct drm_connector_state *connector_state);
 
+int drm_connector_attach_link_bpc_property(struct drm_connector *connector,
+					   unsigned int max_bpc);
+void drm_connector_update_link_bpc_property(struct drm_connector *connector,
+					    struct drm_connector_state *state);
+
 /**
  * struct drm_tile_group - Tile group metadata
  * @refcount: reference count

-- 
2.53.0

Claude review: drm/connector: hdmi: Add a 'link bpc' property

[Claude Code Review Bot]

Patch Review

**Timing of property update in `commit_tail`** (medium severity):

```c
drm_atomic_helper_wait_for_dependencies(state);

for_each_oldnew_connector_in_state(state, connector, old_conn_state,
                                   new_conn_state, i) {
    if (old_conn_state->link_bpc != new_conn_state->link_bpc)
        drm_connector_update_link_bpc_property(connector,
                                               new_conn_state);
}
```

This fires the uevent before `funcs->atomic_commit_tail()` runs, meaning userspace is notified of a change before hardware is actually programmed. For drivers with custom `atomic_commit_tail` (like amdgpu), this is especially problematic. The update should be placed *after* the hw commit, e.g. after `drm_atomic_helper_commit_hw_done()`. However, the comment at line 2071 warns that `new_crtc_state` can't be accessed after `commit_hw_done()` — but `connector_state` should still be safe. Consider moving this loop after the `atomic_commit_tail` call.

**Double clamping** (minor):

```c
void
drm_connector_update_link_bpc_property(struct drm_connector *connector,
                                       struct drm_connector_state *state)
{
    u8 bpc = clamp(state->link_bpc, 8, state->max_bpc);
```

The clamping is done both here and in the amdgpu helper (`amdgpu_dm_update_link_bpc`). For HDMI connectors, `output_bpc` is assigned directly. The defensive clamping is fine but worth noting that drivers are expected to provide sane values, and the double-clamp in patch 2 is redundant.

**Hardcoded minimum of 8** (minor):

```c
if (max_bpc < 8 || max_bpc > U8_MAX)
    return -EINVAL;

prop = drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE,
                                 "link bpc", 8, max_bpc);
```

The 8 bpc lower bound is hardcoded. Some display links (eDP panels, for instance) legitimately operate at 6 bpc. If the intent is to make this generic for future non-HDMI connectors, consider either making the minimum a parameter or documenting why 8 is the universal floor.

**HDMI helper sets link_bpc only when output changes** (potential issue):

```c
if (old_conn_state->hdmi.broadcast_rgb != new_conn_state->hdmi.broadcast_rgb ||
    old_conn_state->hdmi.output_bpc != new_conn_state->hdmi.output_bpc ||
    old_conn_state->hdmi.output_format != new_conn_state->hdmi.output_format) {
        ...
        new_conn_state->link_bpc = new_conn_state->hdmi.output_bpc;
```

`link_bpc` is only set when one of the HDMI output parameters changed. On the very first modeset (when `old_conn_state->link_bpc` is 0 from `kzalloc` and `new_conn_state->link_bpc` is also 0 from `duplicate_state` if it's the initial state), if `output_bpc` hasn't changed between old and new, `link_bpc` will remain 0 and the property won't be updated. This seems like it could lead to the property reporting `max_bpc` (the initial attached value) while the actual link_bpc state is 0 until the first mode change triggers the conditional.

**Property documentation** looks good and is well-placed in the standard connector properties section.

---
Generated by Claude Code Patch Reviewer

[PATCH v4 2/2] drm/amd/display: Add support for 'link bpc' property

[Nicolas Frattaroli]

The 'link bpc' DRM property exposes the connector's current display link
bits per component value. This allows userspace to discover whether a
link has degraded from a higher bit depth to a lower one.

Add support for it in amdgpu.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index dfe95c9b8746..cca4dd93bb6d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7184,6 +7184,14 @@ static void apply_dsc_policy_for_stream(struct amdgpu_dm_connector *aconnector,
 }
 #endif
 
+static void amdgpu_dm_update_link_bpc(struct drm_connector_state *conn_state,
+				      enum dc_color_depth depth)
+{
+	/* 6 bpc is an experimental internal format only, use 8 as minimum */
+	conn_state->link_bpc = clamp(convert_dc_color_depth_into_bpc(depth), 8,
+				     conn_state->max_bpc);
+}
+
 static struct dc_stream_state *
 create_stream_for_sink(struct drm_connector *connector,
 		       const struct drm_display_mode *drm_mode,
@@ -8981,8 +8989,10 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm,
 				adev->mode_info.underscan_vborder_property,
 				0);
 
-	if (!aconnector->mst_root)
+	if (!aconnector->mst_root) {
 		drm_connector_attach_max_bpc_property(&aconnector->base, 8, 16);
+		drm_connector_attach_link_bpc_property(&aconnector->base, 16);
+	}
 
 	aconnector->base.state->max_bpc = 16;
 	aconnector->base.state->max_requested_bpc = aconnector->base.state->max_bpc;
@@ -11425,6 +11435,9 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
 			goto fail;
 		}
 
+		amdgpu_dm_update_link_bpc(drm_new_conn_state,
+					  new_stream->timing.display_color_depth);
+
 		/*
 		 * TODO: Check VSDB bits to decide whether this should
 		 * be enabled or not.

-- 
2.53.0

Claude review: drm/amd/display: Add support for 'link bpc' property

[Claude Code Review Bot]

Patch Review

**MST exclusion** (correct):

```c
if (!aconnector->mst_root) {
    drm_connector_attach_max_bpc_property(&aconnector->base, 8, 16);
    drm_connector_attach_link_bpc_property(&aconnector->base, 16);
}
```

MST connectors are correctly excluded, matching the existing `max_bpc` behavior. The return value of `drm_connector_attach_link_bpc_property` is silently ignored here, unlike in `drmm_connector_hdmi_init` where it's checked. This is inconsistent — if the property creation fails (e.g. ENOMEM), this silently continues. Consider adding error handling.

**link_bpc set only on stream creation path** (potential gap):

```c
amdgpu_dm_update_link_bpc(drm_new_conn_state,
                          new_stream->timing.display_color_depth);
```

This is called in `dm_update_crtc_state` only when `action == DM_COMMIT_ACTION_DPMS_ON || action == DM_COMMIT_ACTION_SET`. When a connector is disabled (`DPMS_OFF`), `link_bpc` is not reset to 0 or any sentinel, so the property will retain the last active value. This might confuse userspace checking the property on a disconnected output.

**Redundant clamping** (nit):

```c
static void amdgpu_dm_update_link_bpc(struct drm_connector_state *conn_state,
                                      enum dc_color_depth depth)
{
    /* 6 bpc is an experimental internal format only, use 8 as minimum */
    conn_state->link_bpc = clamp(convert_dc_color_depth_into_bpc(depth), 8,
                                 conn_state->max_bpc);
}
```

The clamping is also done in `drm_connector_update_link_bpc_property()`, making this double-clamped. While not harmful, it's worth noting. The comment about 6 bpc being experimental is helpful though.

**`convert_dc_color_depth_into_bpc` can return 0**: If `depth` is an unexpected `dc_color_depth` enum value, the function returns 0. After clamping with min=8, this would become 8, which is a reasonable fallback. However, it might be worth adding a `drm_dbg` when the conversion returns 0, as it would indicate an unexpected hardware state.

---
Generated by Claude Code Patch Reviewer

Claude review: Add "link bpc" DRM property

[Claude Code Review Bot]

Overall Series Review

Subject: Add "link bpc" DRM property
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Patches: 3
Reviewed: 2026-03-12T06:57:50.360536

---

This is a 2-patch series adding a new immutable "link bpc" DRM connector property that exposes the actual bits-per-component achieved on a display link, letting userspace detect when bit depth has been degraded (e.g. due to bandwidth constraints). Patch 1 adds the core DRM infrastructure and HDMI helper integration; patch 2 adds amdgpu support.

The design is clean and the approach is reasonable. There are a few issues worth discussing:

**Timing concern in `commit_tail`**: The link_bpc property update and uevent fire *before* `funcs->atomic_commit_tail()` runs. This means userspace is notified of a bpc change before the hardware has actually committed the new state. For amdgpu specifically (which has its own `atomic_commit_tail`), this means the uevent fires before the display hardware is programmed. Ideally this should happen *after* the hw commit completes.

**Clamping lower bound of 8**: The property range and clamping both use 8 as the minimum, but some displays legitimately run at 6 bpc (e.g. certain eDP panels). If 6 bpc is "experimental internal" for amdgpu, that's fine for patch 2's local helper, but hard-coding 8 as the universal lower bound in the core DRM property (patch 1) seems potentially limiting.

**Missing link_bpc reset on disconnect**: When a connector is disabled/disconnected, `link_bpc` is never explicitly reset. It will carry the stale value from the last active state via `memcpy` in `duplicate_state`. This means the property could report a stale bpc value on a disconnected connector.

---

---
Generated by Claude Code Patch Reviewer