Re: [PATCH 06/13] drm: add atomic hook to read property plus helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux