Re: [PATCH v2] drm/connector: hdmi: Add a 'link bpc' property

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Oct 06, 2025 at 11:30:43AM +0300, Marius Vlad wrote:
> From: Derek Foreman <derek.foreman@collabora.com>
> 
> Add a way to know the actual bpc of a running link.
> 
> Drivers might change the current bpc link value due to changes in mode
> line or refresh rates. For example when enabling VRR the underlying
> hardware might not be able sustain the same bandwidth for a particular
> mode line, and it might attempt to lower the bpc.

Not sure what this VRR stuff is trying to say. Enabling VRR doesn't
itself change anything about the link bandwidth.

> Another example can be
> found when switching the color output format, part of YUV420 fallback.
> 
> This means we might be displaying a stale bpc value although it was
> modified for different reasons -- like a refresh rate or an output
> color format.
> 
> Introduces a new property 'link bpc' that user-space can use to get the
> current bpc value of a running link while in the same time allow
> user-space to set-up bpc using 'max bpc' property.
> 
> An implementation for Weston [1] and a simple test for i-g-t [2] have been added.
> 
> Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> 
> [1] https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1850
> [2] https://lists.freedesktop.org/archives/igt-dev/2025-October/097061.html
> 
> ---
> 
> v1: 
> - https://lore.kernel.org/dri-devel/20250801101750.1726-1-marius.vlad@collabora.com/T/#u
> 
> 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)
>     
> 
>  drivers/gpu/drm/drm_atomic_uapi.c |  5 +++++
>  drivers/gpu/drm/drm_connector.c   | 25 +++++++++++++++++++++++++
>  include/drm/drm_connector.h       |  8 ++++++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index 85dbdaa4a2e2..15c5ad7ddfb5 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -776,6 +776,9 @@ 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->link_bpc_property) {
> +		drm_dbg_kms(dev, "only drivers can set link bpc property. Use max bpc instead\n");
> +		return -EINVAL;

Is there a particular reason this isn't just an immutable prop?

>  	} else if (property == connector->privacy_screen_sw_state_property) {
>  		state->privacy_screen_sw_state = val;
>  	} else if (property == connector->broadcast_rgb_property) {
> @@ -861,6 +864,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->link_bpc_property) {
> +		*val = state->hdmi.output_bpc;

Assuming hdmi here doesn't seem good. What about all the other
connector types?

>  	} 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..7cc99cd16e20 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -542,6 +542,27 @@ int drmm_connector_init(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(drmm_connector_init);
>  
> +static int
> +drm_connector_attach_link_bpc_property(struct drm_connector *connector,
> +				       int max)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *prop;
> +
> +	if (connector->link_bpc_property)
> +		return -EBUSY;
> +
> +	prop = drm_property_create_range(dev, 0, "link bpc", 8, max);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	connector->link_bpc_property = prop;
> +
> +	drm_object_attach_property(&connector->base, prop, max);
> +
> +	return 0;
> +}
> +
>  /**
>   * drmm_connector_hdmi_init - Init a preallocated HDMI connector
>   * @dev: DRM device
> @@ -618,6 +639,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);
>  
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 8f34f4b8183d..4a50198aa7c0 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -2079,6 +2079,14 @@ struct drm_connector {
>  	 */
>  	struct drm_property *max_bpc_property;
>  
> +	/**
> +	 * @link_bpc_property: Current connector link bpc set by the driver
> +	 *
> +	 * This property can be used to retrieve the current link bpc from
> +	 * connector_state::hdmi:output_bpc
> +	 */
> +	struct drm_property *link_bpc_property;
> +
>  	/** @privacy_screen: drm_privacy_screen for this connector, or NULL. */
>  	struct drm_privacy_screen *privacy_screen;
>  
> -- 
> 2.47.2

-- 
Ville Syrjälä
Intel