DK put some nice docs into the commit introducing driver private state, but in the git history alone it'll be lost.
Also, since Ville remove the void* usage it's a good opportunity to give the driver private stuff some tlc on the doc front. Finally try to explain why the "let's just subclass drm_atomic_state" approach wasn't the greatest, and annotate all those functions as deprecated in favour of more standardized driver private states. Also note where we could/should extend driver private states going forward (atm neither locking nor synchronization is handled in core/helpers, which isn't really all that great). Cc: Harry Wentland <harry.wentl...@amd.com> Cc: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com> Cc: Rob Clark <robdcl...@gmail.com> Cc: Alex Deucher <alexander.deuc...@amd.com> Cc: Ben Skeggs <bske...@redhat.com> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com> --- Documentation/gpu/drm-kms.rst | 6 ++++++ drivers/gpu/drm/drm_atomic.c | 45 ++++++++++++++++++++++++++++++++++++++++--- include/drm/drm_atomic.h | 28 +++++++++++++++++++++++++++ include/drm/drm_mode_config.h | 9 +++++++++ 4 files changed, 85 insertions(+), 3 deletions(-) diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 307284125d7a..420025bd6a9b 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -271,6 +271,12 @@ Taken all together there's two consequences for the atomic design: Read on in this chapter, and also in :ref:`drm_atomic_helper` for more detailed coverage of specific topics. +Handling Driver Private State +----------------------------- + +.. kernel-doc:: drivers/gpu/drm/drm_atomic.c + :doc: handling driver private state + Atomic Mode Setting Function Reference -------------------------------------- diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 37445d50816a..15e1a35c74a8 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -50,7 +50,8 @@ EXPORT_SYMBOL(__drm_crtc_commit_free); * @state: atomic state * * Free all the memory allocated by drm_atomic_state_init. - * This is useful for drivers that subclass the atomic state. + * This should only be used by drivers which are still subclassing + * &drm_atomic_state and haven't switched to &drm_private_state yet. */ void drm_atomic_state_default_release(struct drm_atomic_state *state) { @@ -67,7 +68,8 @@ EXPORT_SYMBOL(drm_atomic_state_default_release); * @state: atomic state * * Default implementation for filling in a new atomic state. - * This is useful for drivers that subclass the atomic state. + * This should only be used by drivers which are still subclassing + * &drm_atomic_state and haven't switched to &drm_private_state yet. */ int drm_atomic_state_init(struct drm_device *dev, struct drm_atomic_state *state) @@ -132,7 +134,8 @@ EXPORT_SYMBOL(drm_atomic_state_alloc); * @state: atomic state * * Default implementation for clearing atomic state. - * This is useful for drivers that subclass the atomic state. + * This should only be used by drivers which are still subclassing + * &drm_atomic_state and haven't switched to &drm_private_state yet. */ void drm_atomic_state_default_clear(struct drm_atomic_state *state) { @@ -946,6 +949,42 @@ static void drm_atomic_plane_print_state(struct drm_printer *p, plane->funcs->atomic_print_state(p, state); } +/** + * DOC: handling driver private state + * + * Very often the DRM objects exposed to userspace in the atomic modeset api + * (&drm_connector, &drm_crtc and &drm_plane) do not map neatly to the + * underlying hardware. Especially for any kind of shared resources (e.g. shared + * clocks, scaler units, bandwidth and fifo limits shared among a group of + * planes or CRTCs, and so on) it makes sense to model these as independent + * objects. Drivers then need to similar state tracking and commit ordering for + * such private (since not exposed to userpace) objects as the atomic core and + * helpers already provide for connectors, planes and CRTCs. + * + * To make this easier on drivers the atomic core provides some support to track + * driver private state objects using struct &drm_private_obj, with the + * associated state struct &drm_private_state. + * + * Similar to userspace-exposed objects, state structures can be acquired by + * calling drm_atomic_get_private_obj_state(). Since this function does not take + * care of locking, drivers should wrap it for each type of private state object + * they have with the required call to drm_modeset_lock() for the corresponding + * &drm_modeset_lock. + * + * All private state structures contained in a &drm_atomic_state update can be + * iterated using for_each_oldnew_private_obj_in_state(), + * for_each_old_private_obj_in_state() and for_each_old_private_obj_in_state(). + * Drivers are recommend to wrap these for each type of driver private state + * object they have, filtering on &drm_private_obj.funcs using for_each_if(), at + * least if they want to iterate over all objects of a given type. + * + * An earlier way to handle driver private state was by subclassing struct + * &drm_atomic_state. But since that encourages non-standard ways to implement + * the check/commit split atomic requires (by using e.g. "check and rollback or + * commit instead" of "duplicate state, check, then either commit or release + * duplicated state) it is deprecated in favour of using &drm_private_state. + */ + /** * drm_atomic_private_obj_init - initialize private object * @obj: private object diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 5afd6e364fb6..8e4829a25c1b 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -189,12 +189,40 @@ struct drm_private_state_funcs { struct drm_private_state *state); }; +/** + * struct drm_private_obj - base struct for driver private atomic object + * + * A driver private object is initialized by calling + * drm_atomic_private_obj_init() and cleaned up by calling + * drm_atomic_private_obj_fini(). + * + * Currently only tracks the state update functions and the opaque driver + * private state itself, but in the future might also track which + * &drm_modeset_lock is required to duplicate and update this object's state. + */ struct drm_private_obj { + /** + * @state: Current atomic state for this driver private object. + */ struct drm_private_state *state; + /** + * @funcs: + * + * Functions to manipulate the state of this driver private object, see + * &drm_private_state_funcs. + */ const struct drm_private_state_funcs *funcs; }; +/** + * struct drm_private_state - base struct for driver private object state + * @state: backpointer to global drm_atomic_state + * + * Currently only contains a backpointer to the overall atomic upated, but in + * the future also might hold synchronization information similar to e.g. + * &drm_crtc.commit. + */ struct drm_private_state { struct drm_atomic_state *state; }; diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index c6639e8e5007..2cb6f02df64a 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -269,6 +269,9 @@ struct drm_mode_config_funcs { * state easily. If this hook is implemented, drivers must also * implement @atomic_state_clear and @atomic_state_free. * + * Subclassing of &drm_atomic_state is deprecated in favour of using + * &drm_private_state and &drm_private_obj. + * * RETURNS: * * A new &drm_atomic_state on success or NULL on failure. @@ -290,6 +293,9 @@ struct drm_mode_config_funcs { * * Drivers that implement this must call drm_atomic_state_default_clear() * to clear common state. + * + * Subclassing of &drm_atomic_state is deprecated in favour of using + * &drm_private_state and &drm_private_obj. */ void (*atomic_state_clear)(struct drm_atomic_state *state); @@ -302,6 +308,9 @@ struct drm_mode_config_funcs { * * Drivers that implement this must call * drm_atomic_state_default_release() to release common resources. + * + * Subclassing of &drm_atomic_state is deprecated in favour of using + * &drm_private_state and &drm_private_obj. */ void (*atomic_state_free)(struct drm_atomic_state *state); }; -- 2.15.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel