On Fri, Dec 04, 2015 at 09:45:48AM +0100, Daniel Vetter wrote:
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
[...]
> + /**
> + * @destroy:
> + *
> + * Clean up CRTC resources. This is only called at driver unload time
Perhaps drop "only" because there are more than a single potential
usage?
> + /**
> + * @set_property:
> + *
> + * This is the legacy entry point to update a property attach to the
"attached to"
> - /* atomic update handling */
> + /**
> + * @atomic_duplicate_state:
> + *
> + * Duplicate the current atomic state for this CRTC and return it.
> + * The core and helpers gurantee that any atomic state duplicated with
> + * this hook and still owned by the caller (i.e. not transferred to the
> + * driver by calling ->atomic_commit() from struct
> + * &drm_mode_config_funcs) will be cleaned up by calling the
> + * @atomic_destroy_state hook in this structure.
> + *
> + * Atomic drivers which don't subclass struct &drm_crtc should use
> + * drm_atomic_helper_crtc_duplicate_state(). Drivers that subclass the
> + * state structure to extend it with driver-private state should use
> + * __drm_atomic_helper_crtc_duplicate_state() to make sure shared state
> is
> + * duplicated in a consisten fashion across drivers.
"consistent"
> + /**
> + * @atomic_set_property:
> + *
> + * Decode a driver-private property value and store the decoded value
> + * into the passed-in state structure. Since the atomic core decodes all
> + * standardized properties (even for extensions beyond the core set of
> + * properties which might not be implemented by all drivers) this
> + * requires drivers to subclass the state structure.
> + *
> + * Such driver-private properties should really only implemented for
"be implemented"
> + * This function is called in the state assembly phase of atomic
> + * modesets, which can be aborted for any reason (including on
> + * userspace's request to just check whether a configuration would be
> + * possible). Drivers MUST NOT touch any persistent state (hardware or
> + * software) or data structures except the passed in adjusted_mode
Copy-paste error? Should be "... the passed in @state parameter."
> + *
> + * Also since userspace controls in which order properties are set this
> + * function must not do any input validation (since the state update is
> + * incompletely and hence likely inconsistent). Instead any such input
"incomplete"
> + * validation must be done in the various atomic_check callbacks.
> + *
> + * RETURNS:
> + *
> + * 0 if the property has been found, -EINVAL if the property isn't
> + * implemented by the driver (which shouldn't ever happen, the core only
s/shouldn't ever/should never/?
> + * asks for properties attached to this CRTC). No other
Seems like you could put more text on the above line.
> + * validation is allowed by the driver. The core checks that the
> + * property value is within the range (integer, valid enum value, ...)
> + * the driver set when registering the property already.
I'd put the "already" after "The core", otherwise it reads weird in my
opinion.
> + */
> int (*atomic_set_property)(struct drm_crtc *crtc,
> struct drm_crtc_state *state,
> struct drm_property *property,
> uint64_t val);
> + /**
> + * @atomic_get_property:
> + *
> + * Reads out the decoded driver-private property. This is used to
> + * implement the GETCRTC ioctl.
> + *
> + * Do not call this function directly, use
> + * drm_atomic_crtc_get_property() instead.
> + *
> + * This callback is optional if the driver does not support any
> + * driver-private atomic properties.
> + *
> + * RETURNS:
> + *
> + * 0 on success, -EINVAL if ther property isn't implemented by the
s/ther/the/
> + * driver (which shouldn't ever happen, the core only asks for
s/shouldn't ever/should never/?
> + * properties attached on this CRTC).
"attached to"
> @@ -539,19 +662,142 @@ struct drm_connector_funcs {
> enum drm_connector_status (*detect)(struct drm_connector *connector,
> bool force);
> int (*fill_modes)(struct drm_connector *connector, uint32_t max_width,
> uint32_t max_height);
> +
> + /**
> + * @set_property:
> + *
> + * This is the legacy entry point to update a property attach to the
"attached to"
> + /**
> + * @destroy:
> + *
> + * Clean up connector resources. This is only called at driver unload
> time
> + * through drm_mode_config_cleanup(), but can also be called at runtime
> + * when a connector is being hot-unplugged.
Again, perhaps drop the "only" because it's inconsistent when followed
by "but can also".
Most of the comments on the CRTC helpers do apply to the connector
helpers as well, so I haven't repeated them.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20151207/7833adbe/attachment.sig>
