On Wed, Dec 17, 2014 at 9:02 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Tue, Dec 16, 2014 at 06:05:34PM -0500, Rob Clark wrote:
>> Once a driver is using atomic helpers for modeset, the next step is to
>> switch over to atomic properties. To do this, plug in the helper
>> function to your modeconfig funcs and be sure to implement the plane/
>> crtc/connector atomic_{get,set}_property vfuncs for any of your mode-
>> objects which have driver custom properties.
>>
>> Signed-off-by: Rob Clark <robdclark@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/drm_atomic_helper.c | 46 +++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/drm_crtc.c | 9 ++++++++
>> include/drm/drm_atomic_helper.h | 2 ++
>> include/drm/drm_crtc.h | 3 +++
>> 4 files changed, 60 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index d42fdb1..1a1ab34 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -1703,6 +1703,52 @@ backoff:
>> EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
>>
>> /**
>> + * drm_atomic_helper_get_property - helper to read atomic property
>> + * @obj: drm mode object whose property to read
>> + * @property: the property to read
>> + * @val: the read value, returned by reference
>> + *
>> + * RETURNS:
>> + * Zero on success, error code on failure
>> + */
>> +int drm_atomic_helper_get_property(struct drm_mode_object *obj,
>> + struct drm_property *property, uint64_t *val)
>> +{
>> + struct drm_device *dev = property->dev;
>> + int ret;
>> +
>> + switch (obj->type) {
>> + case DRM_MODE_OBJECT_CONNECTOR: {
>> + struct drm_connector *connector = obj_to_connector(obj);
>> + WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>> + ret = connector->funcs->atomic_get_property(connector,
>> + connector->state, property, val);
>
> Shouldn't we use the get helpers introduced in previous patches here? For
> legacy support we definitely want old core stuff like dpms to keep
> working. Of course that means all the new atomic properties won't get
> magically filtered out. But I think we need to hide them explicitly
> anyway, e.g. with a new DRM_MODE_PROP_ATOMIC and checking for that in
> legacy paths.
bleh, you are right.. and they did at one point use the helpers.. but
somehow in my squash/rebase/juggle act I lost that :-(
But I am still not a huge fan of hiding props for atomic drivers via
legacy getprop/setprop interfaces.. if nothing else it will prevent
them from showing up in modetest. And it seems kind of inconsistent
and unnecessary.
BR,
-R
> -Daniel
>
>> + break;
>> + }
>> + case DRM_MODE_OBJECT_CRTC: {
>> + struct drm_crtc *crtc = obj_to_crtc(obj);
>> + WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
>> + ret = crtc->funcs->atomic_get_property(crtc,
>> + crtc->state, property, val);
>> + break;
>> + }
>> + case DRM_MODE_OBJECT_PLANE: {
>> + struct drm_plane *plane = obj_to_plane(obj);
>> + WARN_ON(!drm_modeset_is_locked(&plane->mutex));
>> + ret = plane->funcs->atomic_get_property(plane,
>> + plane->state, property, val);
>> + break;
>> + }
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(drm_atomic_helper_get_property);
>> +
>> +/**
>> * drm_atomic_helper_page_flip - execute a legacy page flip
>> * @crtc: DRM crtc
>> * @fb: DRM framebuffer
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 481bb25..57cd950 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -3878,8 +3878,17 @@ EXPORT_SYMBOL(drm_object_property_set_value);
>> int drm_object_property_get_value(struct drm_mode_object *obj,
>> struct drm_property *property, uint64_t *val)
>> {
>> + struct drm_mode_config *config = &property->dev->mode_config;
>> int i;
>>
>> + /* read-only properties bypass atomic mechanism and still store
>> + * their value in obj->properties->values[].. mostly to avoid
>> + * having to deal w/ EDID and similar props in atomic paths:
>> + */
>> + if (config->funcs->atomic_get_property &&
>> + !(property->flags & DRM_MODE_PROP_IMMUTABLE))
>> + return config->funcs->atomic_get_property(obj, property, val);
>> +
>> for (i = 0; i < obj->properties->count; i++) {
>> if (obj->properties->properties[i] == property) {
>> *val = obj->properties->values[i];
>> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
>> index f956b41..5e72b82 100644
>> --- a/include/drm/drm_atomic_helper.h
>> +++ b/include/drm/drm_atomic_helper.h
>> @@ -74,6 +74,8 @@ int drm_atomic_helper_plane_set_property(struct drm_plane *plane,
>> int drm_atomic_helper_connector_set_property(struct drm_connector *connector,
>> struct drm_property *property,
>> uint64_t val);
>> +int drm_atomic_helper_get_property(struct drm_mode_object *obj,
>> + struct drm_property *property, uint64_t *val);
>> int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
>> struct drm_framebuffer *fb,
>> struct drm_pending_vblank_event *event,
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index e6cfdaf..ca8ba72 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -941,6 +941,7 @@ struct drm_mode_set {
>> * struct drm_mode_config_funcs - basic driver provided mode setting functions
>> * @fb_create: create a new framebuffer object
>> * @output_poll_changed: function to handle output configuration changes
>> + * @atomic_get_property: atomic way to read property value
>> * @atomic_check: check whether a give atomic state update is possible
>> * @atomic_commit: commit an atomic state update previously verified with
>> * atomic_check()
>> @@ -954,6 +955,8 @@ struct drm_mode_config_funcs {
>> struct drm_mode_fb_cmd2 *mode_cmd);
>> void (*output_poll_changed)(struct drm_device *dev);
>>
>> + int (*atomic_get_property)(struct drm_mode_object *obj,
>> + struct drm_property *property, uint64_t *val);
>> int (*atomic_check)(struct drm_device *dev,
>> struct drm_atomic_state *a);
>> int (*atomic_commit)(struct drm_device *dev,
>> --
>> 2.1.0
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
