diff mbox

[04/13] drm: add atomic_set_property wrappers

Message ID 1418771141-16954-5-git-send-email-robdclark@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rob Clark Dec. 16, 2014, 11:05 p.m. UTC
As we add properties for all the standard plane/crtc/connector
attributes (in preperation for the atomic ioctl), we are going to want
to handle core state in core (rather than per driver).  Intercepting the
core properties will be easier if the atomic_set_property vfuncs are not
called directly, but instead have a mandatory wrapper function (which
will later serve as the point to intercept core properties).

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/drm_atomic.c        | 78 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_atomic_helper.c | 12 +++---
 include/drm/drm_atomic.h            |  9 +++++
 include/drm/drm_crtc.h              |  3 ++
 4 files changed, 96 insertions(+), 6 deletions(-)

Comments

Sean Paul Dec. 17, 2014, 12:49 p.m. UTC | #1
On Tue, Dec 16, 2014 at 06:05:32PM -0500, Rob Clark wrote:
> As we add properties for all the standard plane/crtc/connector
> attributes (in preperation for the atomic ioctl), we are going to want
> to handle core state in core (rather than per driver).  Intercepting the
> core properties will be easier if the atomic_set_property vfuncs are not
> called directly, but instead have a mandatory wrapper function (which
> will later serve as the point to intercept core properties).
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>

Tiny copypasta nit in one comment.

> ---
>  drivers/gpu/drm/drm_atomic.c        | 78 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_atomic_helper.c | 12 +++---
>  include/drm/drm_atomic.h            |  9 +++++
>  include/drm/drm_crtc.h              |  3 ++
>  4 files changed, 96 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index ff5f034..1261ade 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -217,6 +217,28 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
>  EXPORT_SYMBOL(drm_atomic_get_crtc_state);
>  
>  /**
> + * drm_atomic_crtc_set_property - set property on connector

s/connector/crtc/

> + * @crtc: the drm CRTC to set a property on
> + * @state: the state object to update with the new property value
> + * @property: the property to set
> + * @val: the new property value
> + *
> + * Use this instead of calling crtc->atomic_set_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> +		struct drm_crtc_state *state, struct drm_property *property,
> +		uint64_t val)
> +{
> +	if (crtc->funcs->atomic_set_property)
> +		return crtc->funcs->atomic_set_property(crtc, state, property, val);
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_atomic_crtc_set_property);
> +
> +/**
>   * drm_atomic_get_plane_state - get plane state
>   * @state: global atomic state object
>   * @plane: plane to get state object for
> @@ -272,6 +294,28 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,
>  EXPORT_SYMBOL(drm_atomic_get_plane_state);
>  
>  /**
> + * drm_atomic_plane_set_property - set property on plane
> + * @plane: the drm plane to set a property on
> + * @state: the state object to update with the new property value
> + * @property: the property to set
> + * @val: the new property value
> + *
> + * Use this instead of calling plane->atomic_set_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_plane_set_property(struct drm_plane *plane,
> +		struct drm_plane_state *state, struct drm_property *property,
> +		uint64_t val)
> +{
> +	if (plane->funcs->atomic_set_property)
> +		return plane->funcs->atomic_set_property(plane, state, property, val);
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_atomic_plane_set_property);
> +
> +/**
>   * drm_atomic_get_connector_state - get connector state
>   * @state: global atomic state object
>   * @connector: connector to get state object for
> @@ -343,6 +387,40 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
>  EXPORT_SYMBOL(drm_atomic_get_connector_state);
>  
>  /**
> + * drm_atomic_connector_set_property - set property on connector.
> + * @connector: the drm connector to set a property on
> + * @state: the state object to update with the new property value
> + * @property: the property to set
> + * @val: the new property value
> + *
> + * Use this instead of calling connector->atomic_set_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_connector_set_property(struct drm_connector *connector,
> +		struct drm_connector_state *state, struct drm_property *property,
> +		uint64_t val)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (property == config->dpms_property) {
> +		/* setting DPMS property requires special handling, which
> +		 * is done in legacy setprop path for us.  Disallow (for
> +		 * now?) atomic writes to DPMS property:
> +		 */
> +		return -EINVAL;
> +	} else if (connector->funcs->atomic_set_property) {
> +		return connector->funcs->atomic_set_property(connector,
> +				state, property, val);
> +	} else {
> +		return -EINVAL;
> +	}
> +}
> +EXPORT_SYMBOL(drm_atomic_connector_set_property);
> +
> +/**
>   * drm_atomic_set_crtc_for_plane - set crtc for plane
>   * @state: the incoming atomic state
>   * @plane: the plane whose incoming state to update
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 4a78a77..d42fdb1 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1558,8 +1558,8 @@ retry:
>  		goto fail;
>  	}
>  
> -	ret = crtc->funcs->atomic_set_property(crtc, crtc_state,
> -					       property, val);
> +	ret = drm_atomic_crtc_set_property(crtc, crtc_state,
> +			property, val);
>  	if (ret)
>  		goto fail;
>  
> @@ -1617,8 +1617,8 @@ retry:
>  		goto fail;
>  	}
>  
> -	ret = plane->funcs->atomic_set_property(plane, plane_state,
> -					       property, val);
> +	ret = drm_atomic_plane_set_property(plane, plane_state,
> +			property, val);
>  	if (ret)
>  		goto fail;
>  
> @@ -1676,8 +1676,8 @@ retry:
>  		goto fail;
>  	}
>  
> -	ret = connector->funcs->atomic_set_property(connector, connector_state,
> -					       property, val);
> +	ret = drm_atomic_connector_set_property(connector, connector_state,
> +			property, val);
>  	if (ret)
>  		goto fail;
>  
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index ad22295..b0834dc 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -38,12 +38,21 @@ void drm_atomic_state_free(struct drm_atomic_state *state);
>  struct drm_crtc_state * __must_check
>  drm_atomic_get_crtc_state(struct drm_atomic_state *state,
>  			  struct drm_crtc *crtc);
> +int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> +		struct drm_crtc_state *state, struct drm_property *property,
> +		uint64_t val);
>  struct drm_plane_state * __must_check
>  drm_atomic_get_plane_state(struct drm_atomic_state *state,
>  			   struct drm_plane *plane);
> +int drm_atomic_plane_set_property(struct drm_plane *plane,
> +		struct drm_plane_state *state, struct drm_property *property,
> +		uint64_t val);
>  struct drm_connector_state * __must_check
>  drm_atomic_get_connector_state(struct drm_atomic_state *state,
>  			       struct drm_connector *connector);
> +int drm_atomic_connector_set_property(struct drm_connector *connector,
> +		struct drm_connector_state *state, struct drm_property *property,
> +		uint64_t val);
>  
>  int __must_check
>  drm_atomic_set_crtc_for_plane(struct drm_atomic_state *state,
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 61ab3e5..1b71f60 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -300,6 +300,7 @@ struct drm_crtc_state {
>   * @atomic_duplicate_state: duplicate the atomic state for this CRTC
>   * @atomic_destroy_state: destroy an atomic state for this CRTC
>   * @atomic_set_property: set a property on an atomic state for this CRTC
> + *    (do not call directly, use drm_atomic_crtc_set_property())
>   *
>   * The drm_crtc_funcs structure is the central CRTC management structure
>   * in the DRM.  Each CRTC controls one or more connectors (note that the name
> @@ -483,6 +484,7 @@ struct drm_connector_state {
>   * @atomic_duplicate_state: duplicate the atomic state for this connector
>   * @atomic_destroy_state: destroy an atomic state for this connector
>   * @atomic_set_property: set a property on an atomic state for this connector
> + *    (do not call directly, use drm_atomic_connector_set_property())
>   *
>   * Each CRTC may have one or more connectors attached to it.  The functions
>   * below allow the core DRM code to control connectors, enumerate available modes,
> @@ -743,6 +745,7 @@ struct drm_plane_state {
>   * @atomic_duplicate_state: duplicate the atomic state for this plane
>   * @atomic_destroy_state: destroy an atomic state for this plane
>   * @atomic_set_property: set a property on an atomic state for this plane
> + *    (do not call directly, use drm_atomic_plane_set_property())
>   */
>  struct drm_plane_funcs {
>  	int (*update_plane)(struct drm_plane *plane,
> -- 
> 2.1.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Daniel Vetter Dec. 17, 2014, 1:43 p.m. UTC | #2
On Tue, Dec 16, 2014 at 06:05:32PM -0500, Rob Clark wrote:
> As we add properties for all the standard plane/crtc/connector
> attributes (in preperation for the atomic ioctl), we are going to want
> to handle core state in core (rather than per driver).  Intercepting the
> core properties will be easier if the atomic_set_property vfuncs are not
> called directly, but instead have a mandatory wrapper function (which
> will later serve as the point to intercept core properties).
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>

Two suggestions for comment improvements below. With that addressed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_atomic.c        | 78 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_atomic_helper.c | 12 +++---
>  include/drm/drm_atomic.h            |  9 +++++
>  include/drm/drm_crtc.h              |  3 ++
>  4 files changed, 96 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index ff5f034..1261ade 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -217,6 +217,28 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
>  EXPORT_SYMBOL(drm_atomic_get_crtc_state);
>  
>  /**
> + * drm_atomic_crtc_set_property - set property on connector
> + * @crtc: the drm CRTC to set a property on
> + * @state: the state object to update with the new property value
> + * @property: the property to set
> + * @val: the new property value
> + *
> + * Use this instead of calling crtc->atomic_set_property directly

Maybe clarify a bit more here. What about

"This function directly processes generic properties (both mandatory and
optional ones) and calls into the driver's ->atomic_set_property hook for
anything else. To esnure consistent behavior this wrapper must be used
instead of calling the driver hook directly, even for driver private
properties."

A bit much perhaps, but kerneldoc is cheap ;-)

> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> +		struct drm_crtc_state *state, struct drm_property *property,
> +		uint64_t val)
> +{
> +	if (crtc->funcs->atomic_set_property)
> +		return crtc->funcs->atomic_set_property(crtc, state, property, val);
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_atomic_crtc_set_property);
> +
> +/**
>   * drm_atomic_get_plane_state - get plane state
>   * @state: global atomic state object
>   * @plane: plane to get state object for
> @@ -272,6 +294,28 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,
>  EXPORT_SYMBOL(drm_atomic_get_plane_state);
>  
>  /**
> + * drm_atomic_plane_set_property - set property on plane
> + * @plane: the drm plane to set a property on
> + * @state: the state object to update with the new property value
> + * @property: the property to set
> + * @val: the new property value
> + *
> + * Use this instead of calling plane->atomic_set_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_plane_set_property(struct drm_plane *plane,
> +		struct drm_plane_state *state, struct drm_property *property,
> +		uint64_t val)
> +{
> +	if (plane->funcs->atomic_set_property)
> +		return plane->funcs->atomic_set_property(plane, state, property, val);
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(drm_atomic_plane_set_property);
> +
> +/**
>   * drm_atomic_get_connector_state - get connector state
>   * @state: global atomic state object
>   * @connector: connector to get state object for
> @@ -343,6 +387,40 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
>  EXPORT_SYMBOL(drm_atomic_get_connector_state);
>  
>  /**
> + * drm_atomic_connector_set_property - set property on connector.
> + * @connector: the drm connector to set a property on
> + * @state: the state object to update with the new property value
> + * @property: the property to set
> + * @val: the new property value
> + *
> + * Use this instead of calling connector->atomic_set_property directly
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int drm_atomic_connector_set_property(struct drm_connector *connector,
> +		struct drm_connector_state *state, struct drm_property *property,
> +		uint64_t val)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	if (property == config->dpms_property) {
> +		/* setting DPMS property requires special handling, which
> +		 * is done in legacy setprop path for us.  Disallow (for
> +		 * now?) atomic writes to DPMS property:
> +		 */

Argh, I still need to write those patches for dpms in atomic. But imo a
better comment would be:

	/*
	 * Per-connector is a legacy-only feature, disallow for atomic
	 * updates.
	 */

I'll extend the comment once the atomic dpms is there.

> +		return -EINVAL;
> +	} else if (connector->funcs->atomic_set_property) {
> +		return connector->funcs->atomic_set_property(connector,
> +				state, property, val);
> +	} else {
> +		return -EINVAL;
> +	}
> +}
> +EXPORT_SYMBOL(drm_atomic_connector_set_property);
> +
> +/**
>   * drm_atomic_set_crtc_for_plane - set crtc for plane
>   * @state: the incoming atomic state
>   * @plane: the plane whose incoming state to update
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 4a78a77..d42fdb1 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1558,8 +1558,8 @@ retry:
>  		goto fail;
>  	}
>  
> -	ret = crtc->funcs->atomic_set_property(crtc, crtc_state,
> -					       property, val);
> +	ret = drm_atomic_crtc_set_property(crtc, crtc_state,
> +			property, val);
>  	if (ret)
>  		goto fail;
>  
> @@ -1617,8 +1617,8 @@ retry:
>  		goto fail;
>  	}
>  
> -	ret = plane->funcs->atomic_set_property(plane, plane_state,
> -					       property, val);
> +	ret = drm_atomic_plane_set_property(plane, plane_state,
> +			property, val);
>  	if (ret)
>  		goto fail;
>  
> @@ -1676,8 +1676,8 @@ retry:
>  		goto fail;
>  	}
>  
> -	ret = connector->funcs->atomic_set_property(connector, connector_state,
> -					       property, val);
> +	ret = drm_atomic_connector_set_property(connector, connector_state,
> +			property, val);
>  	if (ret)
>  		goto fail;
>  
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index ad22295..b0834dc 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -38,12 +38,21 @@ void drm_atomic_state_free(struct drm_atomic_state *state);
>  struct drm_crtc_state * __must_check
>  drm_atomic_get_crtc_state(struct drm_atomic_state *state,
>  			  struct drm_crtc *crtc);
> +int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> +		struct drm_crtc_state *state, struct drm_property *property,
> +		uint64_t val);
>  struct drm_plane_state * __must_check
>  drm_atomic_get_plane_state(struct drm_atomic_state *state,
>  			   struct drm_plane *plane);
> +int drm_atomic_plane_set_property(struct drm_plane *plane,
> +		struct drm_plane_state *state, struct drm_property *property,
> +		uint64_t val);
>  struct drm_connector_state * __must_check
>  drm_atomic_get_connector_state(struct drm_atomic_state *state,
>  			       struct drm_connector *connector);
> +int drm_atomic_connector_set_property(struct drm_connector *connector,
> +		struct drm_connector_state *state, struct drm_property *property,
> +		uint64_t val);
>  
>  int __must_check
>  drm_atomic_set_crtc_for_plane(struct drm_atomic_state *state,
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 61ab3e5..1b71f60 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -300,6 +300,7 @@ struct drm_crtc_state {
>   * @atomic_duplicate_state: duplicate the atomic state for this CRTC
>   * @atomic_destroy_state: destroy an atomic state for this CRTC
>   * @atomic_set_property: set a property on an atomic state for this CRTC
> + *    (do not call directly, use drm_atomic_crtc_set_property())
>   *
>   * The drm_crtc_funcs structure is the central CRTC management structure
>   * in the DRM.  Each CRTC controls one or more connectors (note that the name
> @@ -483,6 +484,7 @@ struct drm_connector_state {
>   * @atomic_duplicate_state: duplicate the atomic state for this connector
>   * @atomic_destroy_state: destroy an atomic state for this connector
>   * @atomic_set_property: set a property on an atomic state for this connector
> + *    (do not call directly, use drm_atomic_connector_set_property())
>   *
>   * Each CRTC may have one or more connectors attached to it.  The functions
>   * below allow the core DRM code to control connectors, enumerate available modes,
> @@ -743,6 +745,7 @@ struct drm_plane_state {
>   * @atomic_duplicate_state: duplicate the atomic state for this plane
>   * @atomic_destroy_state: destroy an atomic state for this plane
>   * @atomic_set_property: set a property on an atomic state for this plane
> + *    (do not call directly, use drm_atomic_plane_set_property())
>   */
>  struct drm_plane_funcs {
>  	int (*update_plane)(struct drm_plane *plane,
> -- 
> 2.1.0
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index ff5f034..1261ade 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -217,6 +217,28 @@  drm_atomic_get_crtc_state(struct drm_atomic_state *state,
 EXPORT_SYMBOL(drm_atomic_get_crtc_state);
 
 /**
+ * drm_atomic_crtc_set_property - set property on connector
+ * @crtc: the drm CRTC to set a property on
+ * @state: the state object to update with the new property value
+ * @property: the property to set
+ * @val: the new property value
+ *
+ * Use this instead of calling crtc->atomic_set_property directly
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
+		struct drm_crtc_state *state, struct drm_property *property,
+		uint64_t val)
+{
+	if (crtc->funcs->atomic_set_property)
+		return crtc->funcs->atomic_set_property(crtc, state, property, val);
+	return -EINVAL;
+}
+EXPORT_SYMBOL(drm_atomic_crtc_set_property);
+
+/**
  * drm_atomic_get_plane_state - get plane state
  * @state: global atomic state object
  * @plane: plane to get state object for
@@ -272,6 +294,28 @@  drm_atomic_get_plane_state(struct drm_atomic_state *state,
 EXPORT_SYMBOL(drm_atomic_get_plane_state);
 
 /**
+ * drm_atomic_plane_set_property - set property on plane
+ * @plane: the drm plane to set a property on
+ * @state: the state object to update with the new property value
+ * @property: the property to set
+ * @val: the new property value
+ *
+ * Use this instead of calling plane->atomic_set_property directly
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+int drm_atomic_plane_set_property(struct drm_plane *plane,
+		struct drm_plane_state *state, struct drm_property *property,
+		uint64_t val)
+{
+	if (plane->funcs->atomic_set_property)
+		return plane->funcs->atomic_set_property(plane, state, property, val);
+	return -EINVAL;
+}
+EXPORT_SYMBOL(drm_atomic_plane_set_property);
+
+/**
  * drm_atomic_get_connector_state - get connector state
  * @state: global atomic state object
  * @connector: connector to get state object for
@@ -343,6 +387,40 @@  drm_atomic_get_connector_state(struct drm_atomic_state *state,
 EXPORT_SYMBOL(drm_atomic_get_connector_state);
 
 /**
+ * drm_atomic_connector_set_property - set property on connector.
+ * @connector: the drm connector to set a property on
+ * @state: the state object to update with the new property value
+ * @property: the property to set
+ * @val: the new property value
+ *
+ * Use this instead of calling connector->atomic_set_property directly
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+int drm_atomic_connector_set_property(struct drm_connector *connector,
+		struct drm_connector_state *state, struct drm_property *property,
+		uint64_t val)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	if (property == config->dpms_property) {
+		/* setting DPMS property requires special handling, which
+		 * is done in legacy setprop path for us.  Disallow (for
+		 * now?) atomic writes to DPMS property:
+		 */
+		return -EINVAL;
+	} else if (connector->funcs->atomic_set_property) {
+		return connector->funcs->atomic_set_property(connector,
+				state, property, val);
+	} else {
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL(drm_atomic_connector_set_property);
+
+/**
  * drm_atomic_set_crtc_for_plane - set crtc for plane
  * @state: the incoming atomic state
  * @plane: the plane whose incoming state to update
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 4a78a77..d42fdb1 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1558,8 +1558,8 @@  retry:
 		goto fail;
 	}
 
-	ret = crtc->funcs->atomic_set_property(crtc, crtc_state,
-					       property, val);
+	ret = drm_atomic_crtc_set_property(crtc, crtc_state,
+			property, val);
 	if (ret)
 		goto fail;
 
@@ -1617,8 +1617,8 @@  retry:
 		goto fail;
 	}
 
-	ret = plane->funcs->atomic_set_property(plane, plane_state,
-					       property, val);
+	ret = drm_atomic_plane_set_property(plane, plane_state,
+			property, val);
 	if (ret)
 		goto fail;
 
@@ -1676,8 +1676,8 @@  retry:
 		goto fail;
 	}
 
-	ret = connector->funcs->atomic_set_property(connector, connector_state,
-					       property, val);
+	ret = drm_atomic_connector_set_property(connector, connector_state,
+			property, val);
 	if (ret)
 		goto fail;
 
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index ad22295..b0834dc 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -38,12 +38,21 @@  void drm_atomic_state_free(struct drm_atomic_state *state);
 struct drm_crtc_state * __must_check
 drm_atomic_get_crtc_state(struct drm_atomic_state *state,
 			  struct drm_crtc *crtc);
+int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
+		struct drm_crtc_state *state, struct drm_property *property,
+		uint64_t val);
 struct drm_plane_state * __must_check
 drm_atomic_get_plane_state(struct drm_atomic_state *state,
 			   struct drm_plane *plane);
+int drm_atomic_plane_set_property(struct drm_plane *plane,
+		struct drm_plane_state *state, struct drm_property *property,
+		uint64_t val);
 struct drm_connector_state * __must_check
 drm_atomic_get_connector_state(struct drm_atomic_state *state,
 			       struct drm_connector *connector);
+int drm_atomic_connector_set_property(struct drm_connector *connector,
+		struct drm_connector_state *state, struct drm_property *property,
+		uint64_t val);
 
 int __must_check
 drm_atomic_set_crtc_for_plane(struct drm_atomic_state *state,
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 61ab3e5..1b71f60 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -300,6 +300,7 @@  struct drm_crtc_state {
  * @atomic_duplicate_state: duplicate the atomic state for this CRTC
  * @atomic_destroy_state: destroy an atomic state for this CRTC
  * @atomic_set_property: set a property on an atomic state for this CRTC
+ *    (do not call directly, use drm_atomic_crtc_set_property())
  *
  * The drm_crtc_funcs structure is the central CRTC management structure
  * in the DRM.  Each CRTC controls one or more connectors (note that the name
@@ -483,6 +484,7 @@  struct drm_connector_state {
  * @atomic_duplicate_state: duplicate the atomic state for this connector
  * @atomic_destroy_state: destroy an atomic state for this connector
  * @atomic_set_property: set a property on an atomic state for this connector
+ *    (do not call directly, use drm_atomic_connector_set_property())
  *
  * Each CRTC may have one or more connectors attached to it.  The functions
  * below allow the core DRM code to control connectors, enumerate available modes,
@@ -743,6 +745,7 @@  struct drm_plane_state {
  * @atomic_duplicate_state: duplicate the atomic state for this plane
  * @atomic_destroy_state: destroy an atomic state for this plane
  * @atomic_set_property: set a property on an atomic state for this plane
+ *    (do not call directly, use drm_atomic_plane_set_property())
  */
 struct drm_plane_funcs {
 	int (*update_plane)(struct drm_plane *plane,