Re: [PATCH v2 1/3] drm: Add variable refresh rate properties to connector

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Sep 24, 2018 at 09:32:11PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 24, 2018 at 02:15:35PM -0400, Nicholas Kazlauskas wrote:
> > Modern display hardware is capable of supporting variable refresh rates
> > and adaptive sync technologies. The properties for querying and
> > controlling these features should be exposed on the DRM connector.
> > 
> > This patch introduces two new properties for variable refresh rate
> > support:
> > 
> > - variable_refresh_capable
> > - variable_refresh_enabled
> > 
> > These are optional properties that can be added to a DRM connector
> > dynamically by using drm_connector_attach_variable_refresh_properties.
> > 
> > DRM drivers should set variable_refresh_capable as applicable for
> > their hardware. The property variable_refresh_enabled is a userspace
> > controlled option.
> > 
> > Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> > ---
> >  drivers/gpu/drm/drm_atomic_uapi.c |  6 ++++++
> >  drivers/gpu/drm/drm_connector.c   | 35 +++++++++++++++++++++++++++++++
> >  include/drm/drm_connector.h       | 27 ++++++++++++++++++++++++
> >  3 files changed, 68 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> > index d5b7f315098c..0bb27a24a55c 100644
> > --- a/drivers/gpu/drm/drm_atomic_uapi.c
> > +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> > @@ -723,6 +723,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> >  		state->content_type = val;
> >  	} else if (property == connector->scaling_mode_property) {
> >  		state->scaling_mode = val;
> > +	} else if (property == connector->variable_refresh_enabled_property) {
> > +		state->variable_refresh_enabled = val;
> >  	} else if (property == connector->content_protection_property) {
> >  		if (val == DRM_MODE_CONTENT_PROTECTION_ENABLED) {
> >  			DRM_DEBUG_KMS("only drivers can set CP Enabled\n");
> > @@ -797,6 +799,10 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> >  		*val = state->content_type;
> >  	} else if (property == connector->scaling_mode_property) {
> >  		*val = state->scaling_mode;
> > +	} else if (property == connector->variable_refresh_capable_property) {
> > +		*val = state->variable_refresh_capable;
> 
> Immutable props don't take this path. See
> __drm_object_property_get_value().

Yeah I think an explicit set function (like we have for edid, link_status,
...) would be good here. Gives us at least a neat way to attach some more
kernel-doc.
-Daniel

> 
> > +	} else if (property == connector->variable_refresh_enabled_property) {
> > +		*val = state->variable_refresh_enabled;
> >  	} else if (property == connector->content_protection_property) {
> >  		*val = state->content_protection;
> >  	} else if (property == config->writeback_fb_id_property) {
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 1e40e5decbe9..fc1732639bd3 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -1254,6 +1254,41 @@ int drm_mode_create_scaling_mode_property(struct drm_device *dev)
> >  }
> >  EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
> >  
> > +/**
> > + * drm_connector_attach_variable_refresh_properties - creates and attaches
> > + * properties for connectors that support adaptive refresh
> > + * @connector: connector to create adaptive refresh properties on
> > + */
> > +int drm_connector_attach_variable_refresh_properties(
> > +	struct drm_connector *connector)
> > +{
> > +	struct drm_device *dev = connector->dev;
> > +	struct drm_property *prop;
> > +
> > +	if (!connector->variable_refresh_capable_property) {
> > +		prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE,
> > +			"variable_refresh_capable");
> > +		if (!prop)
> > +			return -ENOMEM;
> > +
> > +		connector->variable_refresh_capable_property = prop;
> > +		drm_object_attach_property(&connector->base, prop, 0);
> > +	}
> > +
> > +	if (!connector->variable_refresh_enabled_property) {
> > +		prop = drm_property_create_bool(dev, 0,
> > +			"variable_refresh_enabled");
> > +		if (!prop)
> > +			return -ENOMEM;
> > +
> > +		connector->variable_refresh_enabled_property = prop;
> > +		drm_object_attach_property(&connector->base, prop, 0);
> > +	}
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(drm_connector_attach_variable_refresh_properties);
> > +
> >  /**
> >   * drm_connector_attach_scaling_mode_property - attach atomic scaling mode property
> >   * @connector: connector to attach scaling mode property on.
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 91a877fa00cb..e2c26842bd50 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -449,6 +449,18 @@ struct drm_connector_state {
> >  	 */
> >  	unsigned int content_protection;
> >  
> > +	/**
> > +	 * @variable_refresh_enabled: Connector property used to check
> > +	 * if variable refresh is supported on the device.
> > +	 */
> > +	bool variable_refresh_capable;
> > +
> > +	/**
> > +	 * @variable_refresh_enabled: Connector property used to check
> > +	 * if variable refresh is enabled.
> > +	 */
> > +	bool variable_refresh_enabled;
> > +
> >  	/**
> >  	 * @writeback_job: Writeback job for writeback connectors
> >  	 *
> > @@ -910,6 +922,19 @@ struct drm_connector {
> >  	 */
> >  	struct drm_property *scaling_mode_property;
> >  
> > +	/**
> > +	 * @variable_refresh_capable_property: Optional property for
> > +	 * querying hardware support for variable refresh.
> > +	 */
> > +	struct drm_property *variable_refresh_capable_property;
> > +
> > +	/**
> > +	 * @variable_refresh_enabled_property: Optional property for
> > +	 * enabling or disabling support for variable refresh
> > +	 * on the connector.
> > +	 */
> > +	struct drm_property *variable_refresh_enabled_property;
> > +
> >  	/**
> >  	 * @content_protection_property: DRM ENUM property for content
> >  	 * protection. See drm_connector_attach_content_protection_property().
> > @@ -1183,6 +1208,8 @@ int drm_mode_create_scaling_mode_property(struct drm_device *dev);
> >  int drm_connector_attach_content_type_property(struct drm_connector *dev);
> >  int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
> >  					       u32 scaling_mode_mask);
> > +int drm_connector_attach_variable_refresh_properties(
> > +	struct drm_connector *connector);
> >  int drm_connector_attach_content_protection_property(
> >  		struct drm_connector *connector);
> >  int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
> > -- 
> > 2.19.0
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel