drm: Document kms locking a bit better
authorDaniel Vetter <daniel.vetter@ffwll.ch>
Tue, 28 Mar 2017 15:53:48 +0000 (17:53 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Wed, 29 Mar 2017 06:34:28 +0000 (08:34 +0200)
The rules are getting real hard, better to dump my brain into text a
bit. This is by far not complete, but I think I reasonable start at
least.

Some of the older kms structures would need a full doc review anyway
...

Cc: Harry Wentland <harry.wentland@amd.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170328155349.5972-2-daniel.vetter@ffwll.ch
include/drm/drm_connector.h
include/drm/drm_crtc.h
include/drm/drm_mode_config.h
include/drm/drm_plane.h

index 6d3f7ba1c797fe01ce3edbf9f894b256243e9f38..941f57f311aa2221bea207958225e4e1e894a358 100644 (file)
@@ -656,7 +656,6 @@ struct drm_cmdline_mode {
  * @bad_edid_counter: track sinks that give us an EDID with invalid checksum
  * @edid_corrupt: indicates whether the last read EDID was corrupt
  * @debugfs_entry: debugfs directory for this connector
- * @state: current atomic state for this connector
  * @has_tile: is this connector connected to a tiled monitor
  * @tile_group: tile group for the connected monitor
  * @tile_is_single_monitor: whether the tile is one monitor housing
@@ -824,6 +823,21 @@ struct drm_connector {
 
        struct dentry *debugfs_entry;
 
+       /**
+        * @state:
+        *
+        * Current atomic state for this connector.
+        *
+        * This is protected by @drm_mode_config.connection_mutex. Note that
+        * nonblocking atomic commits access the current connector state without
+        * taking locks. Either by going through the &struct drm_atomic_state
+        * pointers, see for_each_connector_in_state(),
+        * for_each_oldnew_connector_in_state(),
+        * for_each_old_connector_in_state() and
+        * for_each_new_connector_in_state(). Or through careful ordering of
+        * atomic commit operations as implemented in the atomic helpers, see
+        * &struct drm_crtc_commit.
+        */
        struct drm_connector_state *state;
 
        /* DisplayID bits */
index 24dcb121bad4694db31b85bceab71fdf50536f93..c1a75a9c81c287dd9b1ad80205f669be57c4a92a 100644 (file)
@@ -699,10 +699,12 @@ struct drm_crtc {
        /**
         * @mutex:
         *
-        * This provides a read lock for the overall crtc state (mode, dpms
+        * This provides a read lock for the overall CRTC state (mode, dpms
         * state, ...) and a write lock for everything which can be update
-        * without a full modeset (fb, cursor data, crtc properties ...). A full
+        * without a full modeset (fb, cursor data, CRTC properties ...). A full
         * modeset also need to grab &drm_mode_config.connection_mutex.
+        *
+        * For atomic drivers specifically this protects @state.
         */
        struct drm_modeset_lock mutex;
 
@@ -748,6 +750,14 @@ struct drm_crtc {
         * @state:
         *
         * Current atomic state for this CRTC.
+        *
+        * This is protected by @mutex. Note that nonblocking atomic commits
+        * access the current CRTC state without taking locks. Either by going
+        * through the &struct drm_atomic_state pointers, see
+        * for_each_crtc_in_state(), for_each_oldnew_crtc_in_state(),
+        * for_each_old_crtc_in_state() and for_each_new_crtc_in_state(). Or
+        * through careful ordering of atomic commit operations as implemented
+        * in the atomic helpers, see &struct drm_crtc_commit.
         */
        struct drm_crtc_state *state;
 
index 579070ff06ef03cca6c4d550030f8f96aacff403..42981711189bda6d93d282ede8d5607f88b1d434 100644 (file)
@@ -307,21 +307,6 @@ struct drm_mode_config_funcs {
 
 /**
  * struct drm_mode_config - Mode configuration control structure
- * @mutex: mutex protecting KMS related lists and structures
- * @connection_mutex: ww mutex protecting connector state and routing
- * @acquire_ctx: global implicit acquire context used by atomic drivers for
- *     legacy IOCTLs
- * @fb_lock: mutex to protect fb state and lists
- * @num_fb: number of fbs available
- * @fb_list: list of framebuffers available
- * @num_encoder: number of encoders on this device
- * @encoder_list: list of encoder objects
- * @num_overlay_plane: number of overlay planes on this device
- * @num_total_plane: number of universal (i.e. with primary/curso) planes on this device
- * @plane_list: list of plane objects
- * @num_crtc: number of CRTCs on this device
- * @crtc_list: list of CRTC objects
- * @property_list: list of property objects
  * @min_width: minimum pixel width on this device
  * @min_height: minimum pixel height on this device
  * @max_width: maximum pixel width on this device
@@ -332,9 +317,6 @@ struct drm_mode_config_funcs {
  * @poll_running: track polling status for this device
  * @delayed_event: track delayed poll uevent deliver for this device
  * @output_poll_work: delayed work for polling in process context
- * @property_blob_list: list of all the blob property objects
- * @blob_lock: mutex for blob property allocation and management
- * @*_property: core property tracking
  * @preferred_depth: preferred RBG pixel depth, used by fb helpers
  * @prefer_shadow: hint to userspace to prefer shadow-fb rendering
  * @cursor_width: hint to userspace for max cursor width
@@ -346,9 +328,37 @@ struct drm_mode_config_funcs {
  * global restrictions are also here, e.g. dimension restrictions.
  */
 struct drm_mode_config {
-       struct mutex mutex; /* protects configuration (mode lists etc.) */
-       struct drm_modeset_lock connection_mutex; /* protects connector->encoder and encoder->crtc links */
-       struct drm_modeset_acquire_ctx *acquire_ctx; /* for legacy _lock_all() / _unlock_all() */
+       /**
+        * @mutex:
+        *
+        * This is the big scary modeset BKL which protects everything that
+        * isn't protect otherwise. Scope is unclear and fuzzy, try to remove
+        * anything from under it's protection and move it into more well-scoped
+        * locks.
+        *
+        * The one important thing this protects is the use of @acquire_ctx.
+        */
+       struct mutex mutex;
+
+       /**
+        * @connection_mutex:
+        *
+        * This protects connector state and the connector to encoder to CRTC
+        * routing chain.
+        *
+        * For atomic drivers specifically this protects &drm_connector.state.
+        */
+       struct drm_modeset_lock connection_mutex;
+
+       /**
+        * @acquire_ctx:
+        *
+        * Global implicit acquire context used by atomic drivers for legacy
+        * IOCTLs. Deprecated, since implicit locking contexts make it
+        * impossible to use driver-private &struct drm_modeset_lock. Users of
+        * this must hold @mutex.
+        */
+       struct drm_modeset_acquire_ctx *acquire_ctx;
 
        /**
         * @idr_mutex:
@@ -374,8 +384,11 @@ struct drm_mode_config {
         */
        struct idr tile_idr;
 
-       struct mutex fb_lock; /* proctects global and per-file fb lists */
+       /** @fb_lock: Mutex to protect fb the global @fb_list and @num_fb. */
+       struct mutex fb_lock;
+       /** @num_fb: Number of entries on @fb_list. */
        int num_fb;
+       /** @fb_list: List of all &struct drm_framebuffer. */
        struct list_head fb_list;
 
        /**
@@ -393,27 +406,80 @@ struct drm_mode_config {
         */
        struct ida connector_ida;
        /**
-        * @connector_list: List of connector objects. Protected by
-        * @connector_list_lock. Only use drm_for_each_connector_iter() and
+        * @connector_list:
+        *
+        * List of connector objects linked with &drm_connector.head. Protected
+        * by @connector_list_lock. Only use drm_for_each_connector_iter() and
         * &struct drm_connector_list_iter to walk this list.
         */
        struct list_head connector_list;
+       /**
+        * @num_encoder:
+        *
+        * Number of encoders on this device. This is invariant over the
+        * lifetime of a device and hence doesn't need any locks.
+        */
        int num_encoder;
+       /**
+        * @encoder_list:
+        *
+        * List of encoder objects linked with &drm_encoder.head. This is
+        * invariant over the lifetime of a device and hence doesn't need any
+        * locks.
+        */
        struct list_head encoder_list;
 
-       /*
-        * Track # of overlay planes separately from # of total planes.  By
-        * default we only advertise overlay planes to userspace; if userspace
-        * sets the "universal plane" capability bit, we'll go ahead and
-        * expose all planes.
+       /**
+        * @num_overlay_plane:
+        *
+        * Number of overlay planes on this device, excluding primary and cursor
+        * planes.
+        *
+        * Track number of overlay planes separately from number of total
+        * planes.  By default we only advertise overlay planes to userspace; if
+        * userspace sets the "universal plane" capability bit, we'll go ahead
+        * and expose all planes. This is invariant over the lifetime of a
+        * device and hence doesn't need any locks.
         */
        int num_overlay_plane;
+       /**
+        * @num_total_plane:
+        *
+        * Number of universal (i.e. with primary/curso) planes on this device.
+        * This is invariant over the lifetime of a device and hence doesn't
+        * need any locks.
+        */
        int num_total_plane;
+       /**
+        * @plane_list:
+        *
+        * List of plane objects linked with &drm_plane.head. This is invariant
+        * over the lifetime of a device and hence doesn't need any locks.
+        */
        struct list_head plane_list;
 
+       /**
+        * @num_crtc:
+        *
+        * Number of CRTCs on this device linked with &drm_crtc.head. This is invariant over the lifetime
+        * of a device and hence doesn't need any locks.
+        */
        int num_crtc;
+       /**
+        * @crtc_list:
+        *
+        * List of CRTC objects linked with &drm_crtc.head. This is invariant
+        * over the lifetime of a device and hence doesn't need any locks.
+        */
        struct list_head crtc_list;
 
+       /**
+        * @property_list:
+        *
+        * List of property type objects linked with &drm_property.head. This is
+        * invariant over the lifetime of a device and hence doesn't need any
+        * locks.
+        */
        struct list_head property_list;
 
        int min_width, min_height;
@@ -427,10 +493,24 @@ struct drm_mode_config {
        bool delayed_event;
        struct delayed_work output_poll_work;
 
+       /**
+        * @blob_lock:
+        *
+        * Mutex for blob property allocation and management, protects
+        * @property_blob_list and &drm_file.blobs.
+        */
        struct mutex blob_lock;
 
-       /* pointers to standard properties */
+       /**
+        * @property_blob_list:
+        *
+        * List of all the blob property objects linked with
+        * &drm_property_blob.head. Protected by @blob_lock.
+        */
        struct list_head property_blob_list;
+
+       /* pointers to standard properties */
+
        /**
         * @edid_property: Default connector property to hold the EDID of the
         * currently connected sink, if any.
index 31da9f0c4ad252bfcf89c8516aa8ffbc4d39a4e6..7aedbf34f3a6a6b5163249c3ba2afdc8103f0378 100644 (file)
@@ -456,7 +456,6 @@ enum drm_plane_type {
  * @funcs: helper functions
  * @properties: property tracking for this plane
  * @type: type of plane (overlay, primary, cursor)
- * @state: current atomic state for this plane
  * @zpos_property: zpos property for this plane
  * @rotation_property: rotation property for this plane
  * @helper_private: mid-layer private data
@@ -473,6 +472,8 @@ struct drm_plane {
         * Protects modeset plane state, together with the &drm_crtc.mutex of
         * CRTC this plane is linked to (when active, getting activated or
         * getting disabled).
+        *
+        * For atomic drivers specifically this protects @state.
         */
        struct drm_modeset_lock mutex;
 
@@ -502,6 +503,19 @@ struct drm_plane {
 
        const struct drm_plane_helper_funcs *helper_private;
 
+       /**
+        * @state:
+        *
+        * Current atomic state for this plane.
+        *
+        * This is protected by @mutex. Note that nonblocking atomic commits
+        * access the current plane state without taking locks. Either by going
+        * through the &struct drm_atomic_state pointers, see
+        * for_each_plane_in_state(), for_each_oldnew_plane_in_state(),
+        * for_each_old_plane_in_state() and for_each_new_plane_in_state(). Or
+        * through careful ordering of atomic commit operations as implemented
+        * in the atomic helpers, see &struct drm_crtc_commit.
+        */
        struct drm_plane_state *state;
 
        struct drm_property *zpos_property;