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 <[email protected]> > > > > 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 <[email protected]> > > Signed-off-by: Marius Vlad <[email protected]> > > --- > > 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
signature.asc
Description: PGP signature
