Op 25-07-17 om 10:01 schreef Daniel Vetter:
> Atomic drivers only use the property value store for immutable (i.e.
> can't be set by userspace, but the kernel can still adjust it)
> properties. The only tricky part is the removal of the update in
> drm_atomic_helper_update_legacy_modeset_state().
>
> This was added in
>
> commit 8c10342cb48f3140d9abeadcfd2fa6625d447282 (tag: 
> topic/drm-misc-2015-07-28)
> Author: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Date:   Mon Jul 27 13:24:29 2015 +0200
>
>     drm/atomic: Update legacy DPMS state during modesets, v3.
>
> by copying it from the i915 code, where it was originally added in
>
> commit 68d3472047a572936551f8ff0b6f4016c5a1fdef
> Author: Daniel Vetter <daniel.vet...@ffwll.ch>
> Date:   Thu Sep 6 22:08:35 2012 +0200
>
>     drm/i915: update dpms property in set_mode
>
> for the legacy modeset code. The reason we needed this hack was that
> i915 didn't yet set DRIVER_ATOMIC, and we checked for that instead of
> the newer-ish drm_drv_uses_atomic_modeset(), which avoids such
> troubles. With the correct feature checks this isn't needed anymore at
> all.
>
> Also make sure that drivers don't accidentally get this wrong by
> making the exported version of drm_object_property_get_value() only
> work for legacy drivers. Only gma500 uses it anyway.
>
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c |  4 ---
>  drivers/gpu/drm/drm_connector.c     |  3 +--
>  drivers/gpu/drm/drm_crtc.c          |  2 +-
>  drivers/gpu/drm/drm_mode_object.c   | 49 
> +++++++++++++++++++++++--------------
>  drivers/gpu/drm/drm_plane.c         |  2 +-
>  5 files changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 7582bbc5decc..4a960c741e35 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -921,16 +921,12 @@ drm_atomic_helper_update_legacy_modeset_state(struct 
> drm_device *dev,
>               crtc = new_conn_state->crtc;
>               if ((!crtc && old_conn_state->crtc) ||
>                   (crtc && drm_atomic_crtc_needs_modeset(crtc->state))) {
> -                     struct drm_property *dpms_prop =
> -                             dev->mode_config.dpms_property;
>                       int mode = DRM_MODE_DPMS_OFF;
>  
>                       if (crtc && crtc->state->active)
>                               mode = DRM_MODE_DPMS_ON;
>  
>                       connector->dpms = mode;
> -                     drm_object_property_set_value(&connector->base,
> -                                                   dpms_prop, mode);
>               }
>       }
>  
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 8072e6e4c62c..349104eadefe 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1225,8 +1225,7 @@ int drm_mode_connector_set_obj_prop(struct 
> drm_mode_object *obj,
>       } else if (connector->funcs->set_property)
>               ret = connector->funcs->set_property(connector, property, 
> value);
>  
> -     /* store the property value if successful */
> -     if (!ret)
> +     if (!ret && drm_drv_uses_atomic_modeset(property->dev))
>               drm_object_property_set_value(&connector->base, property, 
> value);
>       return ret;
>  }
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 5af25ce5bf7c..7d4fcdd34342 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -736,7 +736,7 @@ int drm_mode_crtc_set_obj_prop(struct drm_mode_object 
> *obj,
>  
>       if (crtc->funcs->set_property)
>               ret = crtc->funcs->set_property(crtc, property, value);
> -     if (!ret)
> +     if (!ret && drm_drv_uses_atomic_modeset(property->dev))
>               drm_object_property_set_value(obj, property, value);
You've inverted the checks here for connector and crtc. :)

Otherwise looks sane, fix this and you can add my r-b.

On second thought, as penance you should add kms_properties to the 
fast-feedback testlist. ;)
>       return ret;
> diff --git a/drivers/gpu/drm/drm_mode_object.c 
> b/drivers/gpu/drm/drm_mode_object.c
> index da9a9adbcc98..92743a796bf0 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -233,6 +233,9 @@ int drm_object_property_set_value(struct drm_mode_object 
> *obj,
>  {
>       int i;
>  
> +     WARN_ON(drm_drv_uses_atomic_modeset(property->dev) &&
> +             !(property->flags & DRM_MODE_PROP_IMMUTABLE));
> +
>       for (i = 0; i < obj->properties->count; i++) {
>               if (obj->properties->properties[i] == property) {
>                       obj->properties->values[i] = val;
> @@ -244,24 +247,7 @@ int drm_object_property_set_value(struct drm_mode_object 
> *obj,
>  }
>  EXPORT_SYMBOL(drm_object_property_set_value);
>  
> -/**
> - * drm_object_property_get_value - retrieve the value of a property
> - * @obj: drm mode object to get property value from
> - * @property: property to retrieve
> - * @val: storage for the property value
> - *
> - * This function retrieves the softare state of the given property for the 
> given
> - * property. Since there is no driver callback to retrieve the current 
> property
> - * value this might be out of sync with the hardware, depending upon the 
> driver
> - * and property.
> - *
> - * Atomic drivers should never call this function directly, the core will 
> read
> - * out property values through the various ->atomic_get_property callbacks.
> - *
> - * Returns:
> - * Zero on success, error code on failure.
> - */
> -int drm_object_property_get_value(struct drm_mode_object *obj,
> +int __drm_object_property_get_value(struct drm_mode_object *obj,
>                                 struct drm_property *property, uint64_t *val)
>  {
>       int i;
> @@ -284,6 +270,31 @@ int drm_object_property_get_value(struct drm_mode_object 
> *obj,
>  
>       return -EINVAL;
>  }
> +
> +/**
> + * drm_object_property_get_value - retrieve the value of a property
> + * @obj: drm mode object to get property value from
> + * @property: property to retrieve
> + * @val: storage for the property value
> + *
> + * This function retrieves the softare state of the given property for the 
> given
> + * property. Since there is no driver callback to retrieve the current 
> property
> + * value this might be out of sync with the hardware, depending upon the 
> driver
> + * and property.
> + *
> + * Atomic drivers should never call this function directly, the core will 
> read
> + * out property values through the various ->atomic_get_property callbacks.
> + *
> + * Returns:
> + * Zero on success, error code on failure.
> + */
> +int drm_object_property_get_value(struct drm_mode_object *obj,
> +                               struct drm_property *property, uint64_t *val)
> +{
> +     WARN_ON(drm_drv_uses_atomic_modeset(property->dev));
> +
> +     return __drm_object_property_get_value(obj, property, val);
> +}
>  EXPORT_SYMBOL(drm_object_property_get_value);
>  
>  /* helper for getconnector and getproperties ioctls */
> @@ -302,7 +313,7 @@ int drm_mode_object_get_properties(struct drm_mode_object 
> *obj, bool atomic,
>                       continue;
>  
>               if (*arg_count_props > count) {
> -                     ret = drm_object_property_get_value(obj, prop, &val);
> +                     ret = __drm_object_property_get_value(obj, prop, &val);
>                       if (ret)
>                               return ret;
>  
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 5dc8c4350602..b4ffd0455200 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -330,7 +330,7 @@ int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
>  
>       if (plane->funcs->set_property)
>               ret = plane->funcs->set_property(plane, property, value);
> -     if (!ret)
> +     if (!ret && drm_drv_uses_atomic_modeset(property->dev))
>               drm_object_property_set_value(obj, property, value);
>  
>       return ret;


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to