| Message ID | 1418771141-16954-7-git-send-email-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@gmail.com> > --- > 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. -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 >
On Wed, Dec 17, 2014 at 03:02:41PM +0100, Daniel Vetter 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@gmail.com> > > --- > > 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. Also I'm not sure if there's a benefit to make this a vfunc+helper instead of just core code called unconditionally. Meaning I can't think of any case where drivers want to overwrite this, ever. They can do all the fancy they want for their private properties in the per-object hooks. And for core/shared stuff we better not allow them to play with fire. Or do I miss an important case here? -Daniel
On Wed, Dec 17, 2014 at 9:02 AM, Daniel Vetter <daniel@ffwll.ch> 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@gmail.com> >> --- >> 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
On Wed, Dec 17, 2014 at 9:06 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Wed, Dec 17, 2014 at 03:02:41PM +0100, Daniel Vetter 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@gmail.com> >> > --- >> > 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. > > Also I'm not sure if there's a benefit to make this a vfunc+helper instead > of just core code called unconditionally. Meaning I can't think of any > case where drivers want to overwrite this, ever. They can do all the fancy > they want for their private properties in the per-object hooks. And for > core/shared stuff we better not allow them to play with fire. > > Or do I miss an important case here? drivers can have their own custom properties too :-) umm, or are you talking about the vfunc in config->funcs? (In which case, it was at least a convenient way to for core to know whether to take legacy path or atomic path) BR, -R > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Wed, Dec 17, 2014 at 09:15:12AM -0500, Rob Clark wrote: > On Wed, Dec 17, 2014 at 9:06 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Wed, Dec 17, 2014 at 03:02:41PM +0100, Daniel Vetter 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@gmail.com> > >> > --- > >> > 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. > > > > Also I'm not sure if there's a benefit to make this a vfunc+helper instead > > of just core code called unconditionally. Meaning I can't think of any > > case where drivers want to overwrite this, ever. They can do all the fancy > > they want for their private properties in the per-object hooks. And for > > core/shared stuff we better not allow them to play with fire. > > > > Or do I miss an important case here? > > drivers can have their own custom properties too :-) > > umm, or are you talking about the vfunc in config->funcs? (In which > case, it was at least a convenient way to for core to know whether to > take legacy path or atomic path) Yeah I only meant the config->func hook. Is there no other way to handle this in the core? It looks funny if we use a func pointer essentially as a boolean and don't allow drivers to set anything else than the defaults. Essentially makes the helpers mandatory. Maybe just have bool config->use_atomic_properties instead? -Daniel
On Wed, Dec 17, 2014 at 09:12:31AM -0500, Rob Clark wrote: > 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. By default at least the intel X driver reads/writes _all_ props it finds. Maybe just on connectors, but even there we have the crtc prop. Which means with the current legacy set_prop helpers we'll end up doing tons of modesets, and that will piss of users. So I think we don't have a choice but hide them. In i915 all the legacy props have careful checks to short-circuit such noop changes out, exactly for this reasons. Adding all these short-circuit checks would pretty badly complicate helpers and hence is something I've explicitly left out (for now at least). And the PROP_ATOMIC flag is a useful hint to userspace to maybe not do stupid things with that property just for fun. -Daniel
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); + 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,
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@gmail.com> --- 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(+)