Re: [PATCH 1/2] drm/connector: Introduce "min bpc" connector property

[Harry Wentland <harry.wentland@amd.com>]

On 2026-06-12 11:34, Nicolas Frattaroli wrote:
> Add a new KMS connector property called "min bpc". Its purpose is to
> allow userspace to mandate that the output bit depth ends up greater
> than or equal to a specified depth, or else have the commit fail. This
> is useful for checking for output depth degradation in combination with
> atomic test commits.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Despite your statement in the cover letter, this does still need a
userspace implementation before merging, as well as an IGT test.

But I like this definition and your documentation and think we have
something quite workable, even long-term.

Harry

> ---
>  drivers/gpu/drm/drm_atomic_helper.c |  4 ++
>  drivers/gpu/drm/drm_atomic_uapi.c   |  4 ++
>  drivers/gpu/drm/drm_connector.c     | 77 +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h         | 13 +++++++
>  4 files changed, 98 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 285aac3554df..613a674ace87 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -739,6 +739,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>  			    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 (old_connector_state->color_format !=
>  			    new_connector_state->color_format)
>  				new_crtc_state->connectors_changed = true;
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 1050dddadb17..0440cb63f786 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -956,6 +956,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) {
> @@ -1043,6 +1045,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 cbb067d02cb9..dcf53cc113b0 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1738,6 +1738,46 @@ 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 minimum bit depth that
> + *	the driver must reach on the connector for a given atomic state. If it
> + *	cannot do so, the atomic commit fails.
> + *
> + *	The precise definition of minimum bit depth used for the "min bpc"
> + *	property must align with the definition a given driver uses for the
> + *	"max bpc" property, such that when "min bpc" and "max bpc" are set to
> + *	the same value, the allowable range of bit depths is a single set point.
> + *
> + *	Drivers should ideally use a definition of bit depth that refers to a
> + *	perceptual optical bit depth, meaning it is equivalent to an
> + *	uncompressed optical bit depth for a variety of content to the median
> + *	human eyeball in a variety of viewing environments. The display link's
> + *	actual optical bit depth to represent the data on the wire may be lower
> + *	due to compression techniques, or even higher due to padding. In
> + *	concrete terms, userspace should not need to know how to implement DSC
> + *	compression in order to set a minimum bit depth to the appropriate
> + *	value, and should not need to perform display-protocol-specific
> + *	calculations to set the right on-the-wire format.
> + *
> + *	Userspace may use the "min bpc" property in combination with performing
> + *	atomic test commits in order to enumerate the possible output bit depths
> + *	for a given configuration. This way, userspace compositors can make
> + *	use-case and content-specific tradeoffs to reach a desired output bit
> + *	depth, such as by using chroma subsampling, or by choosing a lower
> + *	refresh rate mode.
> + *
> + *	Userspace may use the "min bpc" property in combination with the
> + *	"max bpc" property to know what output bit depth a successful atomic
> + *	commit is using. By setting "min bpc" and "max bpc" to the same value,
> + *	a successful commit will be using an output bit depth of said value.
> + *
> + *	When "min bpc" is set to a non-zero value, any bit depth degradation
> + *	logic below "min bpc" must be implemented by userspace (through lowering
> + *	"min bpc"), rather than by kernel drivers. For a driver to degrade to an
> + *	uncompressed optical output bit depth below "min bpc" is always a bug no
> + *	matter how good the intentions are, since it renders the property
> + *	useless as an oracle.
> + *
>   * Connectors also have one standardized atomic property:
>   *
>   * CRTC_ID:
> @@ -2892,6 +2932,43 @@ 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.
> + * @max: The maximum bit depth supported by the connector.
> + *
> + * Create and attach the "min bpc" connector property, which is used by
> + * userspace to require that a certain minimum bit depth is reached by the
> + * connector on atomic commit.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_attach_min_bpc_property(struct drm_connector *connector,
> +					  unsigned int max)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *prop;
> +
> +	if (max > U8_MAX)
> +		return -ERANGE;
> +
> +	prop = connector->min_bpc_property;
> +	if (!prop) {
> +		prop = drm_property_create_range(dev, 0, "min bpc", 0, max);
> +		if (!prop)
> +			return -ENOMEM;
> +
> +		connector->min_bpc_property = prop;
> +	}
> +
> +	drm_object_attach_property(&connector->base, prop, 0);
> +	connector->state->min_requested_bpc = 0;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_attach_min_bpc_property);
> +
>  /**
>   * drm_connector_attach_hdr_output_metadata_property - attach "HDR_OUTPUT_METADATA" property
>   * @connector: connector to attach the property on.
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 4317166562cf..63d6ac85b3ec 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1261,6 +1261,11 @@ struct drm_connector_state {
>  	 */
>  	u8 max_requested_bpc;
>  
> +	/**
> +	 * @min_requested_bpc: Minimum output bit depth requested by userspace
> +	 */
> +	u8 min_requested_bpc;
> +
>  	/**
>  	 * @max_bpc: Connector max_bpc based on the requested max_bpc property
>  	 * and the connector bpc limitations obtained from edid.
> @@ -2291,6 +2296,12 @@ struct drm_connector {
>  	 */
>  	struct drm_property *max_bpc_property;
>  
> +	/**
> +	 * @min_bpc_property: Default connector property for the minimum bit
> +	 * depth for the output to reach, or the commit to fail otherwise.
> +	 */
> +	struct drm_property *min_bpc_property;
> +
>  	/** @privacy_screen: drm_privacy_screen for this connector, or NULL. */
>  	struct drm_privacy_screen *privacy_screen;
>  
> @@ -2697,6 +2708,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,
> +					  unsigned 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(
>