[PATCH] drm: Add "min bpc" connector property

Sasha McIntosh posted 1 patch 3 months, 1 week ago
drivers/gpu/drm/drm_atomic.c        | 12 +++++++++
drivers/gpu/drm/drm_atomic_helper.c |  4 +++
drivers/gpu/drm/drm_atomic_uapi.c   |  4 +++
drivers/gpu/drm/drm_connector.c     | 41 +++++++++++++++++++++++++++++
include/drm/drm_connector.h         | 20 ++++++++++++++
5 files changed, 81 insertions(+)
[PATCH] drm: Add "min bpc" connector property
Posted by Sasha McIntosh 3 months, 1 week ago
[Why]
When playing HDR or WCG content, bandwidth constraints may force the
driver to downgrade to 6bpc, resulting in visual artifacts like banding.

Userspace should be able to configure a minimum allowed bpc.

[How]
Introduce the "min bpc" connector property so the user can limit the
minimum bpc. Mirror the "mac bpc" implementation.

Signed-off-by: Sasha McIntosh <sashamcintosh@google.com>
---
 drivers/gpu/drm/drm_atomic.c        | 12 +++++++++
 drivers/gpu/drm/drm_atomic_helper.c |  4 +++
 drivers/gpu/drm/drm_atomic_uapi.c   |  4 +++
 drivers/gpu/drm/drm_connector.c     | 41 +++++++++++++++++++++++++++++
 include/drm/drm_connector.h         | 20 ++++++++++++++
 5 files changed, 81 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index be2cb6e43cb0..f85ad9c55e69 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -468,6 +468,17 @@ static int drm_atomic_connector_check(struct drm_connector *connector,
 	state->max_bpc = info->bpc ? info->bpc : 8;
 	if (connector->max_bpc_property)
 		state->max_bpc = min(state->max_bpc, state->max_requested_bpc);
+	if (connector->min_bpc_property)
+		state->min_bpc = state->min_requested_bpc;
+	if (connector->max_bpc_property && connector->min_bpc_property &&
+	    state->max_requested_bpc < state->min_requested_bpc) {
+		drm_dbg_atomic(connector->dev,
+			       "[CONNECTOR:%d:%s] max bpc %d < min bpc %d\n",
+			       connector->base.id, connector->name,
+			       state->max_requested_bpc,
+			       state->min_requested_bpc);
+		return -EINVAL;
+	}
 
 	if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) || !writeback_job)
 		return 0;
@@ -1195,6 +1206,7 @@ static void drm_atomic_connector_print_state(struct drm_printer *p,
 	drm_printf(p, "\tinterlace_allowed=%d\n", connector->interlace_allowed);
 	drm_printf(p, "\tycbcr_420_allowed=%d\n", connector->ycbcr_420_allowed);
 	drm_printf(p, "\tmax_requested_bpc=%d\n", state->max_requested_bpc);
+	drm_printf(p, "\tmin_requested_bpc=%d\n", state->min_requested_bpc);
 	drm_printf(p, "\tcolorspace=%s\n", drm_get_colorspace_name(state->colorspace));
 
 	if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 5a473a274ff0..75659d46c6fe 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -736,6 +736,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 			if (old_connector_state->max_requested_bpc !=
 			    new_connector_state->max_requested_bpc)
 				new_crtc_state->connectors_changed = true;
+
+			if (old_connector_state->min_requested_bpc !=
+			    new_connector_state->min_requested_bpc)
+				new_crtc_state->connectors_changed = true;
 		}
 
 		if (funcs->atomic_check)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 85dbdaa4a2e2..f99649f9c51f 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -776,6 +776,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
 						   fence_ptr);
 	} else if (property == connector->max_bpc_property) {
 		state->max_requested_bpc = val;
+	} else if (property == connector->min_bpc_property) {
+		state->min_requested_bpc = val;
 	} else if (property == connector->privacy_screen_sw_state_property) {
 		state->privacy_screen_sw_state = val;
 	} else if (property == connector->broadcast_rgb_property) {
@@ -861,6 +863,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = 0;
 	} else if (property == connector->max_bpc_property) {
 		*val = state->max_requested_bpc;
+	} else if (property == connector->min_bpc_property) {
+		*val = state->min_requested_bpc;
 	} else if (property == connector->privacy_screen_sw_state_property) {
 		*val = state->privacy_screen_sw_state;
 	} else if (property == connector->broadcast_rgb_property) {
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 272d6254ea47..2d9cfd4f5118 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1699,6 +1699,13 @@ 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.
  *
+ * min bpc:
+ *	This range property is used by userspace to set a lower bound for the bit
+ *	depth. When used the driver would set the bpc in accordance with the
+ *	valid range supported by the hardware and sink. Drivers to use the function
+ *	drm_connector_attach_min_bpc_property() to create and attach the
+ *	property to the connector during initialization.
+ *
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
@@ -2845,6 +2852,40 @@ int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
 
+/**
+ * drm_connector_attach_min_bpc_property - attach "min bpc" property
+ * @connector: connector to attach min bpc property on.
+ * @min: The minimum bit depth supported by the connector.
+ * @max: The maximum bit depth supported by the connector.
+ *
+ * This is used to add support for limiting the bit depth on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_min_bpc_property(struct drm_connector *connector,
+					  int min, int max)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+
+	prop = connector->min_bpc_property;
+	if (!prop) {
+		prop = drm_property_create_range(dev, 0, "min bpc", min, max);
+		if (!prop)
+			return -ENOMEM;
+
+		connector->min_bpc_property = prop;
+	}
+
+	drm_object_attach_property(&connector->base, prop, min);
+	connector->state->min_requested_bpc = min;
+	connector->state->min_bpc = min;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_min_bpc_property);
+
 /**
  * drm_connector_attach_hdr_output_metadata_property - attach "HDR_OUTPUT_METADA" property
  * @connector: connector to attach the property on.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 8f34f4b8183d..7581f965b015 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1126,12 +1126,24 @@ struct drm_connector_state {
 	 */
 	u8 max_requested_bpc;
 
+	/**
+	 * @min_requested_bpc: Connector property to limit the minimum bit
+	 * depth of the pixels.
+	 */
+	u8 min_requested_bpc;
+
 	/**
 	 * @max_bpc: Connector max_bpc based on the requested max_bpc property
 	 * and the connector bpc limitations obtained from edid.
 	 */
 	u8 max_bpc;
 
+	/**
+	 * @min_bpc: Connector min_bpc based on the requested min_bpc property
+	 * and the connector bpc limitations obtained from edid.
+	 */
+	u8 min_bpc;
+
 	/**
 	 * @privacy_screen_sw_state: See :ref:`Standard Connector
 	 * Properties<standard_connector_properties>`
@@ -2079,6 +2091,12 @@ struct drm_connector {
 	 */
 	struct drm_property *max_bpc_property;
 
+	/**
+	 * @min_bpc_property: Default connector property for the min bpc to be
+	 * driven out of the connector.
+	 */
+	struct drm_property *min_bpc_property;
+
 	/** @privacy_screen: drm_privacy_screen for this connector, or NULL. */
 	struct drm_privacy_screen *privacy_screen;
 
@@ -2482,6 +2500,8 @@ int drm_connector_set_orientation_from_panel(
 	struct drm_panel *panel);
 int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
 					  int min, int max);
+int drm_connector_attach_min_bpc_property(struct drm_connector *connector,
+					  int min, int max);
 void drm_connector_create_privacy_screen_properties(struct drm_connector *conn);
 void drm_connector_attach_privacy_screen_properties(struct drm_connector *conn);
 void drm_connector_attach_privacy_screen_provider(

base-commit: 098456f3141bf9e0c0d8973695ca38a03465ccd6
-- 
2.51.1.851.g4ebd6896fd-goog
Re: [PATCH] drm: Add "min bpc" connector property
Posted by Marius Vlad 3 months ago
Hi,
On Fri, Oct 31, 2025 at 04:45:34PM -0400, Sasha McIntosh wrote:
> [Why]
> When playing HDR or WCG content, bandwidth constraints may force the
> driver to downgrade to 6bpc, resulting in visual artifacts like banding.
Bit confused by this. How exactly would they reach 6bpc? The lower limit
seems to be 8 [1].
> 
> Userspace should be able to configure a minimum allowed bpc.
> 
> [How]
> Introduce the "min bpc" connector property so the user can limit the
> minimum bpc. Mirror the "mac bpc" implementation.

[1] https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/display/drm_hdmi_state_helper.c#L620
> 
> Signed-off-by: Sasha McIntosh <sashamcintosh@google.com>
> ---
>  drivers/gpu/drm/drm_atomic.c        | 12 +++++++++
>  drivers/gpu/drm/drm_atomic_helper.c |  4 +++
>  drivers/gpu/drm/drm_atomic_uapi.c   |  4 +++
>  drivers/gpu/drm/drm_connector.c     | 41 +++++++++++++++++++++++++++++
>  include/drm/drm_connector.h         | 20 ++++++++++++++
>  5 files changed, 81 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index be2cb6e43cb0..f85ad9c55e69 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -468,6 +468,17 @@ static int drm_atomic_connector_check(struct drm_connector *connector,
>  	state->max_bpc = info->bpc ? info->bpc : 8;
>  	if (connector->max_bpc_property)
>  		state->max_bpc = min(state->max_bpc, state->max_requested_bpc);
> +	if (connector->min_bpc_property)
> +		state->min_bpc = state->min_requested_bpc;
> +	if (connector->max_bpc_property && connector->min_bpc_property &&
> +	    state->max_requested_bpc < state->min_requested_bpc) {
> +		drm_dbg_atomic(connector->dev,
> +			       "[CONNECTOR:%d:%s] max bpc %d < min bpc %d\n",
> +			       connector->base.id, connector->name,
> +			       state->max_requested_bpc,
> +			       state->min_requested_bpc);
> +		return -EINVAL;
> +	}
>  
>  	if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) || !writeback_job)
>  		return 0;
> @@ -1195,6 +1206,7 @@ static void drm_atomic_connector_print_state(struct drm_printer *p,
>  	drm_printf(p, "\tinterlace_allowed=%d\n", connector->interlace_allowed);
>  	drm_printf(p, "\tycbcr_420_allowed=%d\n", connector->ycbcr_420_allowed);
>  	drm_printf(p, "\tmax_requested_bpc=%d\n", state->max_requested_bpc);
> +	drm_printf(p, "\tmin_requested_bpc=%d\n", state->min_requested_bpc);
>  	drm_printf(p, "\tcolorspace=%s\n", drm_get_colorspace_name(state->colorspace));
>  
>  	if (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 5a473a274ff0..75659d46c6fe 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -736,6 +736,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>  			if (old_connector_state->max_requested_bpc !=
>  			    new_connector_state->max_requested_bpc)
>  				new_crtc_state->connectors_changed = true;
> +
> +			if (old_connector_state->min_requested_bpc !=
> +			    new_connector_state->min_requested_bpc)
> +				new_crtc_state->connectors_changed = true;
>  		}
>  
>  		if (funcs->atomic_check)
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 85dbdaa4a2e2..f99649f9c51f 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -776,6 +776,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  						   fence_ptr);
>  	} else if (property == connector->max_bpc_property) {
>  		state->max_requested_bpc = val;
> +	} else if (property == connector->min_bpc_property) {
> +		state->min_requested_bpc = val;
>  	} else if (property == connector->privacy_screen_sw_state_property) {
>  		state->privacy_screen_sw_state = val;
>  	} else if (property == connector->broadcast_rgb_property) {
> @@ -861,6 +863,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  		*val = 0;
>  	} else if (property == connector->max_bpc_property) {
>  		*val = state->max_requested_bpc;
> +	} else if (property == connector->min_bpc_property) {
> +		*val = state->min_requested_bpc;
>  	} else if (property == connector->privacy_screen_sw_state_property) {
>  		*val = state->privacy_screen_sw_state;
>  	} else if (property == connector->broadcast_rgb_property) {
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 272d6254ea47..2d9cfd4f5118 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1699,6 +1699,13 @@ 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.
>   *
> + * min bpc:
> + *	This range property is used by userspace to set a lower bound for the bit
> + *	depth. When used the driver would set the bpc in accordance with the
> + *	valid range supported by the hardware and sink. Drivers to use the function
> + *	drm_connector_attach_min_bpc_property() to create and attach the
> + *	property to the connector during initialization.
> + *
>   * Connectors also have one standardized atomic property:
>   *
>   * CRTC_ID:
> @@ -2845,6 +2852,40 @@ int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>  }
>  EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
>  
> +/**
> + * drm_connector_attach_min_bpc_property - attach "min bpc" property
> + * @connector: connector to attach min bpc property on.
> + * @min: The minimum bit depth supported by the connector.
> + * @max: The maximum bit depth supported by the connector.
> + *
> + * This is used to add support for limiting the bit depth on a connector.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_attach_min_bpc_property(struct drm_connector *connector,
> +					  int min, int max)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *prop;
> +
> +	prop = connector->min_bpc_property;
> +	if (!prop) {
> +		prop = drm_property_create_range(dev, 0, "min bpc", min, max);
> +		if (!prop)
> +			return -ENOMEM;
> +
> +		connector->min_bpc_property = prop;
> +	}
> +
> +	drm_object_attach_property(&connector->base, prop, min);
> +	connector->state->min_requested_bpc = min;
> +	connector->state->min_bpc = min;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_attach_min_bpc_property);
> +
>  /**
>   * drm_connector_attach_hdr_output_metadata_property - attach "HDR_OUTPUT_METADA" property
>   * @connector: connector to attach the property on.
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 8f34f4b8183d..7581f965b015 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1126,12 +1126,24 @@ struct drm_connector_state {
>  	 */
>  	u8 max_requested_bpc;
>  
> +	/**
> +	 * @min_requested_bpc: Connector property to limit the minimum bit
> +	 * depth of the pixels.
> +	 */
> +	u8 min_requested_bpc;
> +
>  	/**
>  	 * @max_bpc: Connector max_bpc based on the requested max_bpc property
>  	 * and the connector bpc limitations obtained from edid.
>  	 */
>  	u8 max_bpc;
>  
> +	/**
> +	 * @min_bpc: Connector min_bpc based on the requested min_bpc property
> +	 * and the connector bpc limitations obtained from edid.
> +	 */
> +	u8 min_bpc;
> +
>  	/**
>  	 * @privacy_screen_sw_state: See :ref:`Standard Connector
>  	 * Properties<standard_connector_properties>`
> @@ -2079,6 +2091,12 @@ struct drm_connector {
>  	 */
>  	struct drm_property *max_bpc_property;
>  
> +	/**
> +	 * @min_bpc_property: Default connector property for the min bpc to be
> +	 * driven out of the connector.
> +	 */
> +	struct drm_property *min_bpc_property;
> +
>  	/** @privacy_screen: drm_privacy_screen for this connector, or NULL. */
>  	struct drm_privacy_screen *privacy_screen;
>  
> @@ -2482,6 +2500,8 @@ int drm_connector_set_orientation_from_panel(
>  	struct drm_panel *panel);
>  int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>  					  int min, int max);
> +int drm_connector_attach_min_bpc_property(struct drm_connector *connector,
> +					  int min, int max);
>  void drm_connector_create_privacy_screen_properties(struct drm_connector *conn);
>  void drm_connector_attach_privacy_screen_properties(struct drm_connector *conn);
>  void drm_connector_attach_privacy_screen_provider(
> 
> base-commit: 098456f3141bf9e0c0d8973695ca38a03465ccd6
> -- 
> 2.51.1.851.g4ebd6896fd-goog
> 
Re: [PATCH] drm: Add "min bpc" connector property
Posted by Michel Dänzer 3 months ago
On 10/31/25 21:45, Sasha McIntosh wrote:
> [Why]
> When playing HDR or WCG content, bandwidth constraints may force the
> driver to downgrade to 6bpc, resulting in visual artifacts like banding.
> 
> Userspace should be able to configure a minimum allowed bpc.
> 
> [How]
> Introduce the "min bpc" connector property so the user can limit the
> minimum bpc. Mirror the "mac bpc" implementation.

One issue with this is that the effective bpc of the link (as observed by the user) can be higher than the physical bpc, due to things like DSC or various dithering techniques. So requiring a minimum physical bpc could artificially exclude configurations which would otherwise work fine.


-- 
Earthling Michel Dänzer       \        GNOME / Xwayland / Mesa developer
https://redhat.com             \               Libre software enthusiast