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

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

[Marius Vlad]

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

This patch introduces a new property 'link bpc' that user-space can
use to get the current bpc value of a running link. In the same
time this would allow user-space set up bpc using 'max_bpc' property.

Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
---
 drivers/gpu/drm/drm_atomic_uapi.c |  5 +++++
 drivers/gpu/drm/drm_connector.c   | 26 ++++++++++++++++++++++++++
 include/drm/drm_connector.h       |  8 ++++++++
 3 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index ecc73d52bfae..3a2ffb957ade 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;
 	} 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;
 	} 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..7ed27aec0ccc 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -542,6 +542,28 @@ 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 min, int max)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+
+	prop = connector->link_bpc_property;
+	if (prop)
+		return 0;
+
+	prop = drm_property_create_range(dev, 0, "link bpc", min, 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 +640,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, 8, 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

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

[Jani Nikula]

On Fri, 01 Aug 2025, Marius Vlad <marius.vlad@collabora.com> 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. 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.
>
> This patch introduces a new property 'link bpc' that user-space can

Nitpick, s/This patch introduces/Introduce/.

> use to get the current bpc value of a running link. In the same
> time this would allow user-space set up bpc using 'max_bpc' property.
>
> Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  5 +++++
>  drivers/gpu/drm/drm_connector.c   | 26 ++++++++++++++++++++++++++
>  include/drm/drm_connector.h       |  8 ++++++++
>  3 files changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index ecc73d52bfae..3a2ffb957ade 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;
>  	} 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;
>  	} 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..7ed27aec0ccc 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -542,6 +542,28 @@ 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 min, int max)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *prop;
> +
> +	prop = connector->link_bpc_property;
> +	if (prop)
> +		return 0;
> +
> +	prop = drm_property_create_range(dev, 0, "link bpc", min, 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 +640,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, 8, 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

It doesn't have to be just HDMI, does it?

> +	 */
> +	struct drm_property *link_bpc_property;
> +
>  	/** @privacy_screen: drm_privacy_screen for this connector, or NULL. */
>  	struct drm_privacy_screen *privacy_screen;

-- 
Jani Nikula, Intel

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

[Marius Vlad]

[-- Attachment #1: Type: text/plain, Size: 5775 bytes --]

Hi Jani,

Thanks for the feedback, sorry the late reply.

On Fri, Aug 01, 2025 at 03:19:08PM +0300, Jani Nikula wrote:
> On Fri, 01 Aug 2025, Marius Vlad <marius.vlad@collabora.com> 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. 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.
> >
> > This patch introduces a new property 'link bpc' that user-space can
> 
> Nitpick, s/This patch introduces/Introduce/.
Sure, will mend.
> 
> > use to get the current bpc value of a running link. In the same
> > time this would allow user-space set up bpc using 'max_bpc' property.
> >
> > Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
> > Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  5 +++++
> >  drivers/gpu/drm/drm_connector.c   | 26 ++++++++++++++++++++++++++
> >  include/drm/drm_connector.h       |  8 ++++++++
> >  3 files changed, 39 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index ecc73d52bfae..3a2ffb957ade 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;
> >  	} 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;
> >  	} 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..7ed27aec0ccc 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -542,6 +542,28 @@ 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 min, int max)
> > +{
> > +	struct drm_device *dev = connector->dev;
> > +	struct drm_property *prop;
> > +
> > +	prop = connector->link_bpc_property;
> > +	if (prop)
> > +		return 0;
> > +
> > +	prop = drm_property_create_range(dev, 0, "link bpc", min, 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 +640,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, 8, 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
> 
> It doesn't have to be just HDMI, does it?
For sure it doesn't, just that I didn't found anything else that would
expose this independently of/for the connector type. 

I'm under the impression that only HDMI -- and those drivers that
actually call the available helpers update this value accordingly.

For code that was inspired from i915, i915 doesn't seem to use those
helpers (intel_hdmi_mode_clock_valid) nor it does actually update
connector_state::hdmi:output_bpc when picking the best available bpc.  I
suppose doing this now wouldn't be that hard, but having something 
similar for DP/MST seems like an quest on its own. 

Is that something you'd like to see pursued with this patch/series?
> 
> > +	 */
> > +	struct drm_property *link_bpc_property;
> > +
> >  	/** @privacy_screen: drm_privacy_screen for this connector, or NULL. */
> >  	struct drm_privacy_screen *privacy_screen;
> 
> -- 
> Jani Nikula, Intel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Dmitry Baryshkov]

On Fri, Aug 01, 2025 at 01:17:50PM +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. 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.
> 
> This patch introduces a new property 'link bpc' that user-space can
> use to get the current bpc value of a running link. In the same
> time this would allow user-space set up bpc using 'max_bpc' property.

Could you please point out the userspace implementation which uses this
property?

> 
> Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c |  5 +++++
>  drivers/gpu/drm/drm_connector.c   | 26 ++++++++++++++++++++++++++
>  include/drm/drm_connector.h       |  8 ++++++++
>  3 files changed, 39 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index ecc73d52bfae..3a2ffb957ade 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;
>  	} 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;
>  	} 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..7ed27aec0ccc 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -542,6 +542,28 @@ 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 min, int max)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *prop;
> +
> +	prop = connector->link_bpc_property;
> +	if (prop)
> +		return 0;

Shouldn't it be:

if (connector->link_bpc_property)
	return -EBUSY;

> +
> +	prop = drm_property_create_range(dev, 0, "link bpc", min, 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 +640,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, 8, max_bpc);
> +	if (ret)
> +		return ret;

Is there a code which sets this property?

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

-- 
With best wishes
Dmitry

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

[Marius Vlad]

[-- Attachment #1: Type: text/plain, Size: 5547 bytes --]

Hi Dmitry,
On Fri, Aug 01, 2025 at 05:09:06PM +0300, Dmitry Baryshkov wrote:
> On Fri, Aug 01, 2025 at 01:17:50PM +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. 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.
> > 
> > This patch introduces a new property 'link bpc' that user-space can
> > use to get the current bpc value of a running link. In the same
> > time this would allow user-space set up bpc using 'max_bpc' property.
> 
> Could you please point out the userspace implementation which uses this
> property?
I'll be adding a MR for Weston for retriving this property. It will compare
it with 'max bpc' and inform the users that we've noticed a link change.
> 
> > 
> > Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
> > Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  5 +++++
> >  drivers/gpu/drm/drm_connector.c   | 26 ++++++++++++++++++++++++++
> >  include/drm/drm_connector.h       |  8 ++++++++
> >  3 files changed, 39 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index ecc73d52bfae..3a2ffb957ade 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;
> >  	} 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;
> >  	} 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..7ed27aec0ccc 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -542,6 +542,28 @@ 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 min, int max)
> > +{
> > +	struct drm_device *dev = connector->dev;
> > +	struct drm_property *prop;
> > +
> > +	prop = connector->link_bpc_property;
> > +	if (prop)
> > +		return 0;
> 
> Shouldn't it be:
> 
> if (connector->link_bpc_property)
> 	return -EBUSY;
Yeah.
> 
> > +
> > +	prop = drm_property_create_range(dev, 0, "link bpc", min, 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 +640,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, 8, max_bpc);
> > +	if (ret)
> > +		return ret;
> 
> Is there a code which sets this property?
Hmm, the idea is/was that userspace will just read out this property and
compare with the 'max bpc' one. In my mind it should be immutable. Is
that what you're implying here?
> 
> > +
> >  	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
> > 
> 
> -- 
> With best wishes
> Dmitry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Dmitry Baryshkov]

On Tue, Sep 23, 2025 at 03:06:27PM +0300, Marius Vlad wrote:
> Hi Dmitry,
> On Fri, Aug 01, 2025 at 05:09:06PM +0300, Dmitry Baryshkov wrote:
> > On Fri, Aug 01, 2025 at 01:17:50PM +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. 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.
> > > 
> > > This patch introduces a new property 'link bpc' that user-space can
> > > use to get the current bpc value of a running link. In the same
> > > time this would allow user-space set up bpc using 'max_bpc' property.
> > 
> > Could you please point out the userspace implementation which uses this
> > property?
> I'll be adding a MR for Weston for retriving this property. It will compare
> it with 'max bpc' and inform the users that we've noticed a link change.

Thanks!

> > 
> > > 
> > > Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
> > > Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> > > ---
> > >  drivers/gpu/drm/drm_atomic_uapi.c |  5 +++++
> > >  drivers/gpu/drm/drm_connector.c   | 26 ++++++++++++++++++++++++++
> > >  include/drm/drm_connector.h       |  8 ++++++++
> > >  3 files changed, 39 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > > index ecc73d52bfae..3a2ffb957ade 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;
> > >  	} 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;
> > >  	} 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..7ed27aec0ccc 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -542,6 +542,28 @@ 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 min, int max)
> > > +{
> > > +	struct drm_device *dev = connector->dev;
> > > +	struct drm_property *prop;
> > > +
> > > +	prop = connector->link_bpc_property;
> > > +	if (prop)
> > > +		return 0;
> > 
> > Shouldn't it be:
> > 
> > if (connector->link_bpc_property)
> > 	return -EBUSY;
> Yeah.
> > 
> > > +
> > > +	prop = drm_property_create_range(dev, 0, "link bpc", min, 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 +640,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, 8, max_bpc);
> > > +	if (ret)
> > > +		return ret;
> > 
> > Is there a code which sets this property?
> Hmm, the idea is/was that userspace will just read out this property and
> compare with the 'max bpc' one. In my mind it should be immutable. Is
> that what you're implying here?

Yes. Also plese point out the IGT tests for the prop.

> > 
> > > +
> > >  	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
> > > 
> > 
> > -- 
> > With best wishes
> > Dmitry



-- 
With best wishes
Dmitry

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

[Michel Dänzer]

On 01.08.25 12:17, 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. 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.
> 
> This patch introduces a new property 'link bpc' that user-space can
> use to get the current bpc value of a running link. In the same
> time this would allow user-space set up bpc using 'max_bpc' property.

With my Wayland compositor developer hat on, what I want to know the most isn't the physical link bpc but the "effective" bpc, i.e. how many bits of information can actually be transferred to the display, taking things like dithering into account.

(Not saying the physical link bpc couldn't also be useful for something, I'm not aware of any use for it offhand though)


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