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 <[email protected]>

Reviewed-by: Harry Wentland <[email protected]>

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(
> 

Reply via email to