* [PATCH 1/3] drm/doc: remove standard connector props from the csv file
@ 2017-03-28 15:53 Daniel Vetter
2017-03-28 15:53 ` [PATCH 2/3] drm: Document kms locking a bit better Daniel Vetter
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Daniel Vetter @ 2017-03-28 15:53 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, DRI Development
They're properly documented in drm_connector.c now, and this
csv file is a horrible mess. Better to remove it.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
Documentation/gpu/kms-properties.csv | 5 -----
1 file changed, 5 deletions(-)
diff --git a/Documentation/gpu/kms-properties.csv b/Documentation/gpu/kms-properties.csv
index 981873a05d14..927b65e14219 100644
--- a/Documentation/gpu/kms-properties.csv
+++ b/Documentation/gpu/kms-properties.csv
@@ -1,10 +1,5 @@
Owner Module/Drivers,Group,Property Name,Type,Property Values,Object attached,Description/Restrictions
,,“scaling mode”,ENUM,"{ ""None"", ""Full"", ""Center"", ""Full aspect"" }",Connector,"Supported by: amdgpu, gma500, i915, nouveau and radeon."
-,Connector,“EDID”,BLOB | IMMUTABLE,0,Connector,Contains id of edid blob ptr object.
-,,“DPMS”,ENUM,"{ “On”, “Standby”, “Suspend”, “Off” }",Connector,Contains DPMS operation mode value.
-,,“PATH”,BLOB | IMMUTABLE,0,Connector,Contains topology path to a connector.
-,,“TILE”,BLOB | IMMUTABLE,0,Connector,Contains tiling information for a connector.
-,,“CRTC_ID”,OBJECT,DRM_MODE_OBJECT_CRTC,Connector,CRTC that connector is attached to (atomic)
,DVI-I,“subconnector”,ENUM,"{ “Unknown”, “DVI-D”, “DVI-A” }",Connector,TBD
,,“select subconnector”,ENUM,"{ “Automatic”, “DVI-D”, “DVI-A” }",Connector,TBD
,TV,“subconnector”,ENUM,"{ ""Unknown"", ""Composite"", ""SVIDEO"", ""Component"", ""SCART"" }",Connector,TBD
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] drm: Document kms locking a bit better 2017-03-28 15:53 [PATCH 1/3] drm/doc: remove standard connector props from the csv file Daniel Vetter @ 2017-03-28 15:53 ` Daniel Vetter 2017-03-28 15:53 ` [PATCH 3/3] drm: document the all the atomic iterators Daniel Vetter 2017-03-28 17:23 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/doc: remove standard connector props from the csv file Patchwork 2 siblings, 0 replies; 7+ messages in thread From: Daniel Vetter @ 2017-03-28 15:53 UTC (permalink / raw) To: Intel Graphics Development Cc: Daniel Vetter, Harry Wentland, DRI Development, Daniel Vetter 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> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- include/drm/drm_connector.h | 16 ++++- include/drm/drm_crtc.h | 14 ++++- include/drm/drm_mode_config.h | 140 +++++++++++++++++++++++++++++++++--------- include/drm/drm_plane.h | 16 ++++- 4 files changed, 152 insertions(+), 34 deletions(-) diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index f8b766d70a46..90e0e0e4e9d6 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -655,7 +655,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 @@ -823,6 +822,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 */ diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 352558e62cfa..ede60d67976f 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -702,10 +702,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; @@ -751,6 +753,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; diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 579070ff06ef..42981711189b 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -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. diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index c2eae851fa30..9ab3e7044812 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -459,7 +459,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 @@ -476,6 +475,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; @@ -505,6 +506,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; -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] drm: document the all the atomic iterators 2017-03-28 15:53 [PATCH 1/3] drm/doc: remove standard connector props from the csv file Daniel Vetter 2017-03-28 15:53 ` [PATCH 2/3] drm: Document kms locking a bit better Daniel Vetter @ 2017-03-28 15:53 ` Daniel Vetter 2017-03-28 16:10 ` Alex Deucher 2017-03-28 16:15 ` Harry Wentland 2017-03-28 17:23 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/doc: remove standard connector props from the csv file Patchwork 2 siblings, 2 replies; 7+ messages in thread From: Daniel Vetter @ 2017-03-28 15:53 UTC (permalink / raw) To: Intel Graphics Development Cc: Daniel Vetter, Harry Wentland, DRI Development, Daniel Vetter Mostly because I want the links from the newly-added @state functions to work. But I think explaining when they're useful and that the implicit one is deprecated is good either way. Slightly repetitive unfortunately. Cc: Harry Wentland <harry.wentland@amd.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- include/drm/drm_atomic.h | 159 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 158 insertions(+), 1 deletion(-) diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 0147a047878d..fd33ed5eaeb4 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -498,6 +498,23 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state); void drm_state_dump(struct drm_device *dev, struct drm_printer *p); +/** + * for_each_connector_in_state - iterate over all connectors in an atomic update + * @__state: &struct drm_atomic_state pointer + * @connector: &struct drm_connector iteration cursor + * @connector_state: &struct drm_connector_state iteration cursor + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all connectors in an atomic update. Note that before the + * software state is committed (by calling drm_atomic_helper_swap_state(), this + * points to the new state, while afterwards it points to the old state. Due to + * this tricky confusion this macro is deprecated. + * + * FIXME: + * + * Replace all usage of this with one of the explicit iterators below and then + * remove this macro. + */ #define for_each_connector_in_state(__state, connector, connector_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->num_connector && \ @@ -506,6 +523,20 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (connector) +/** + * for_each_oldnew_connector_in_state - iterate over all connectors in an atomic update + * @__state: &struct drm_atomic_state pointer + * @connector: &struct drm_connector iteration cursor + * @old_connector_state: &struct drm_connector_state iteration cursor for the + * old state + * @new_connector_state: &struct drm_connector_state iteration cursor for the + * new state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all connectors in an atomic update, tracking both old and + * new state. This is useful in places where the state delta needs to be + * considered, for example in atomic check functions. + */ #define for_each_oldnew_connector_in_state(__state, connector, old_connector_state, new_connector_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->num_connector && \ @@ -515,6 +546,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (connector) +/** + * for_each_old_connector_in_state - iterate over all connectors in an atomic update + * @__state: &struct drm_atomic_state pointer + * @connector: &struct drm_connector iteration cursor + * @old_connector_state: &struct drm_connector_state iteration cursor for the + * old state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all connectors in an atomic update, tracking only the old + * state. This is useful in disable functions, where we need the old state the + * hardware is still in. + */ #define for_each_old_connector_in_state(__state, connector, old_connector_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->num_connector && \ @@ -523,6 +566,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (connector) +/** + * for_each_new_connector_in_state - iterate over all connectors in an atomic update + * @__state: &struct drm_atomic_state pointer + * @connector: &struct drm_connector iteration cursor + * @new_connector_state: &struct drm_connector_state iteration cursor for the + * new state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all connectors in an atomic update, tracking only the new + * state. This is useful in enable functions, where we need the new state the + * hardware should be in when the atomic commit operation has completed. + */ #define for_each_new_connector_in_state(__state, connector, new_connector_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->num_connector && \ @@ -531,6 +586,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (connector) +/** + * for_each_crtc_in_state - iterate over all connectors in an atomic update + * @__state: &struct drm_atomic_state pointer + * @crtc: &struct drm_crtc iteration cursor + * @crtc_state: &struct drm_crtc_state iteration cursor + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all CRTCs in an atomic update. Note that before the + * software state is committed (by calling drm_atomic_helper_swap_state(), this + * points to the new state, while afterwards it points to the old state. Due to + * this tricky confusion this macro is deprecated. + * + * FIXME: + * + * Replace all usage of this with one of the explicit iterators below and then + * remove this macro. + */ #define for_each_crtc_in_state(__state, crtc, crtc_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_crtc && \ @@ -539,6 +611,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (crtc_state) +/** + * for_each_oldnew_crtc_in_state - iterate over all CRTCs in an atomic update + * @__state: &struct drm_atomic_state pointer + * @crtc: &struct drm_crtc iteration cursor + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all CRTCs in an atomic update, tracking both old and + * new state. This is useful in places where the state delta needs to be + * considered, for example in atomic check functions. + */ #define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_crtc && \ @@ -548,6 +632,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (crtc) +/** + * for_each_old_crtc_in_state - iterate over all CRTCs in an atomic update + * @__state: &struct drm_atomic_state pointer + * @crtc: &struct drm_crtc iteration cursor + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all CRTCs in an atomic update, tracking only the old + * state. This is useful in disable functions, where we need the old state the + * hardware is still in. + */ #define for_each_old_crtc_in_state(__state, crtc, old_crtc_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_crtc && \ @@ -556,6 +651,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (crtc) +/** + * for_each_new_crtc_in_state - iterate over all CRTCs in an atomic update + * @__state: &struct drm_atomic_state pointer + * @crtc: &struct drm_crtc iteration cursor + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all CRTCs in an atomic update, tracking only the new + * state. This is useful in enable functions, where we need the new state the + * hardware should be in when the atomic commit operation has completed. + */ #define for_each_new_crtc_in_state(__state, crtc, new_crtc_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_crtc && \ @@ -564,6 +670,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (crtc) +/** + * for_each_plane_in_state - iterate over all planes in an atomic update + * @__state: &struct drm_atomic_state pointer + * @plane: &struct drm_plane iteration cursor + * @plane_state: &struct drm_plane_state iteration cursor + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all planes in an atomic update. Note that before the + * software state is committed (by calling drm_atomic_helper_swap_state(), this + * points to the new state, while afterwards it points to the old state. Due to + * this tricky confusion this macro is deprecated. + * + * FIXME: + * + * Replace all usage of this with one of the explicit iterators below and then + * remove this macro. + */ #define for_each_plane_in_state(__state, plane, plane_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_total_plane && \ @@ -572,6 +695,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (plane_state) +/** + * for_each_oldnew_plane_in_state - iterate over all planes in an atomic update + * @__state: &struct drm_atomic_state pointer + * @plane: &struct drm_plane iteration cursor + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all planes in an atomic update, tracking both old and + * new state. This is useful in places where the state delta needs to be + * considered, for example in atomic check functions. + */ #define for_each_oldnew_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_total_plane && \ @@ -581,6 +716,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (plane) +/** + * for_each_old_plane_in_state - iterate over all planes in an atomic update + * @__state: &struct drm_atomic_state pointer + * @plane: &struct drm_plane iteration cursor + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all planes in an atomic update, tracking only the old + * state. This is useful in disable functions, where we need the old state the + * hardware is still in. + */ #define for_each_old_plane_in_state(__state, plane, old_plane_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_total_plane && \ @@ -589,6 +735,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (plane) +/** + * for_each_new_plane_in_state - iterate over all planes in an atomic update + * @__state: &struct drm_atomic_state pointer + * @plane: &struct drm_plane iteration cursor + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all planes in an atomic update, tracking only the new + * state. This is useful in enable functions, where we need the new state the + * hardware should be in when the atomic commit operation has completed. + */ #define for_each_new_plane_in_state(__state, plane, new_plane_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_total_plane && \ @@ -603,7 +760,7 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); * * To give drivers flexibility &struct drm_crtc_state has 3 booleans to track * whether the state CRTC changed enough to need a full modeset cycle: - * connectors_changed, mode_changed and active_changed. This helper simply + * planes_changed, mode_changed and active_changed. This helper simply * combines these three to compute the overall need for a modeset for @state. * * The atomic helper code sets these booleans, but drivers can and should -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] drm: document the all the atomic iterators 2017-03-28 15:53 ` [PATCH 3/3] drm: document the all the atomic iterators Daniel Vetter @ 2017-03-28 16:10 ` Alex Deucher 2017-03-29 6:35 ` Daniel Vetter 2017-03-28 16:15 ` Harry Wentland 1 sibling, 1 reply; 7+ messages in thread From: Alex Deucher @ 2017-03-28 16:10 UTC (permalink / raw) To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development On Tue, Mar 28, 2017 at 11:53 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > Mostly because I want the links from the newly-added @state functions > to work. But I think explaining when they're useful and that the > implicit one is deprecated is good either way. Slightly repetitive > unfortunately. > > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Series is: Reviewed-by: Alex Deucher <alexander.deucher@amd.com> > --- > include/drm/drm_atomic.h | 159 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 158 insertions(+), 1 deletion(-) > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 0147a047878d..fd33ed5eaeb4 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -498,6 +498,23 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state); > > void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > +/** > + * for_each_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @connector_state: &struct drm_connector_state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update. Note that before the > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > + * points to the new state, while afterwards it points to the old state. Due to > + * this tricky confusion this macro is deprecated. > + * > + * FIXME: > + * > + * Replace all usage of this with one of the explicit iterators below and then > + * remove this macro. > + */ > #define for_each_connector_in_state(__state, connector, connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -506,6 +523,20 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_oldnew_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @old_connector_state: &struct drm_connector_state iteration cursor for the > + * old state > + * @new_connector_state: &struct drm_connector_state iteration cursor for the > + * new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update, tracking both old and > + * new state. This is useful in places where the state delta needs to be > + * considered, for example in atomic check functions. > + */ > #define for_each_oldnew_connector_in_state(__state, connector, old_connector_state, new_connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -515,6 +546,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_old_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @old_connector_state: &struct drm_connector_state iteration cursor for the > + * old state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update, tracking only the old > + * state. This is useful in disable functions, where we need the old state the > + * hardware is still in. > + */ > #define for_each_old_connector_in_state(__state, connector, old_connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -523,6 +566,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_new_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @new_connector_state: &struct drm_connector_state iteration cursor for the > + * new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update, tracking only the new > + * state. This is useful in enable functions, where we need the new state the > + * hardware should be in when the atomic commit operation has completed. > + */ > #define for_each_new_connector_in_state(__state, connector, new_connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -531,6 +586,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_crtc_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @crtc_state: &struct drm_crtc_state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update. Note that before the > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > + * points to the new state, while afterwards it points to the old state. Due to > + * this tricky confusion this macro is deprecated. > + * > + * FIXME: > + * > + * Replace all usage of this with one of the explicit iterators below and then > + * remove this macro. > + */ > #define for_each_crtc_in_state(__state, crtc, crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -539,6 +611,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc_state) > > +/** > + * for_each_oldnew_crtc_in_state - iterate over all CRTCs in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state > + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update, tracking both old and > + * new state. This is useful in places where the state delta needs to be > + * considered, for example in atomic check functions. > + */ > #define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -548,6 +632,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc) > > +/** > + * for_each_old_crtc_in_state - iterate over all CRTCs in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update, tracking only the old > + * state. This is useful in disable functions, where we need the old state the > + * hardware is still in. > + */ > #define for_each_old_crtc_in_state(__state, crtc, old_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -556,6 +651,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc) > > +/** > + * for_each_new_crtc_in_state - iterate over all CRTCs in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update, tracking only the new > + * state. This is useful in enable functions, where we need the new state the > + * hardware should be in when the atomic commit operation has completed. > + */ > #define for_each_new_crtc_in_state(__state, crtc, new_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -564,6 +670,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc) > > +/** > + * for_each_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @plane_state: &struct drm_plane_state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update. Note that before the > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > + * points to the new state, while afterwards it points to the old state. Due to > + * this tricky confusion this macro is deprecated. > + * > + * FIXME: > + * > + * Replace all usage of this with one of the explicit iterators below and then > + * remove this macro. > + */ > #define for_each_plane_in_state(__state, plane, plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -572,6 +695,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (plane_state) > > +/** > + * for_each_oldnew_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state > + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update, tracking both old and > + * new state. This is useful in places where the state delta needs to be > + * considered, for example in atomic check functions. > + */ > #define for_each_oldnew_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -581,6 +716,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (plane) > > +/** > + * for_each_old_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update, tracking only the old > + * state. This is useful in disable functions, where we need the old state the > + * hardware is still in. > + */ > #define for_each_old_plane_in_state(__state, plane, old_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -589,6 +735,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (plane) > > +/** > + * for_each_new_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update, tracking only the new > + * state. This is useful in enable functions, where we need the new state the > + * hardware should be in when the atomic commit operation has completed. > + */ > #define for_each_new_plane_in_state(__state, plane, new_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -603,7 +760,7 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > * > * To give drivers flexibility &struct drm_crtc_state has 3 booleans to track > * whether the state CRTC changed enough to need a full modeset cycle: > - * connectors_changed, mode_changed and active_changed. This helper simply > + * planes_changed, mode_changed and active_changed. This helper simply > * combines these three to compute the overall need for a modeset for @state. > * > * The atomic helper code sets these booleans, but drivers can and should > -- > 2.11.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] drm: document the all the atomic iterators 2017-03-28 16:10 ` Alex Deucher @ 2017-03-29 6:35 ` Daniel Vetter 0 siblings, 0 replies; 7+ messages in thread From: Daniel Vetter @ 2017-03-29 6:35 UTC (permalink / raw) To: Alex Deucher Cc: Daniel Vetter, Intel Graphics Development, DRI Development, Daniel Vetter On Tue, Mar 28, 2017 at 12:10:37PM -0400, Alex Deucher wrote: > On Tue, Mar 28, 2017 at 11:53 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > Mostly because I want the links from the newly-added @state functions > > to work. But I think explaining when they're useful and that the > > implicit one is deprecated is good either way. Slightly repetitive > > unfortunately. > > > > Cc: Harry Wentland <harry.wentland@amd.com> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Series is: > Reviewed-by: Alex Deucher <alexander.deucher@amd.com> Thanks a lot for your reviews, all applied to -misc. -Daniel > > > --- > > include/drm/drm_atomic.h | 159 ++++++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 158 insertions(+), 1 deletion(-) > > > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > > index 0147a047878d..fd33ed5eaeb4 100644 > > --- a/include/drm/drm_atomic.h > > +++ b/include/drm/drm_atomic.h > > @@ -498,6 +498,23 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state); > > > > void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > > > +/** > > + * for_each_connector_in_state - iterate over all connectors in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @connector: &struct drm_connector iteration cursor > > + * @connector_state: &struct drm_connector_state iteration cursor > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all connectors in an atomic update. Note that before the > > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > > + * points to the new state, while afterwards it points to the old state. Due to > > + * this tricky confusion this macro is deprecated. > > + * > > + * FIXME: > > + * > > + * Replace all usage of this with one of the explicit iterators below and then > > + * remove this macro. > > + */ > > #define for_each_connector_in_state(__state, connector, connector_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->num_connector && \ > > @@ -506,6 +523,20 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (connector) > > > > +/** > > + * for_each_oldnew_connector_in_state - iterate over all connectors in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @connector: &struct drm_connector iteration cursor > > + * @old_connector_state: &struct drm_connector_state iteration cursor for the > > + * old state > > + * @new_connector_state: &struct drm_connector_state iteration cursor for the > > + * new state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all connectors in an atomic update, tracking both old and > > + * new state. This is useful in places where the state delta needs to be > > + * considered, for example in atomic check functions. > > + */ > > #define for_each_oldnew_connector_in_state(__state, connector, old_connector_state, new_connector_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->num_connector && \ > > @@ -515,6 +546,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (connector) > > > > +/** > > + * for_each_old_connector_in_state - iterate over all connectors in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @connector: &struct drm_connector iteration cursor > > + * @old_connector_state: &struct drm_connector_state iteration cursor for the > > + * old state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all connectors in an atomic update, tracking only the old > > + * state. This is useful in disable functions, where we need the old state the > > + * hardware is still in. > > + */ > > #define for_each_old_connector_in_state(__state, connector, old_connector_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->num_connector && \ > > @@ -523,6 +566,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (connector) > > > > +/** > > + * for_each_new_connector_in_state - iterate over all connectors in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @connector: &struct drm_connector iteration cursor > > + * @new_connector_state: &struct drm_connector_state iteration cursor for the > > + * new state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all connectors in an atomic update, tracking only the new > > + * state. This is useful in enable functions, where we need the new state the > > + * hardware should be in when the atomic commit operation has completed. > > + */ > > #define for_each_new_connector_in_state(__state, connector, new_connector_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->num_connector && \ > > @@ -531,6 +586,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (connector) > > > > +/** > > + * for_each_crtc_in_state - iterate over all connectors in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @crtc: &struct drm_crtc iteration cursor > > + * @crtc_state: &struct drm_crtc_state iteration cursor > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all CRTCs in an atomic update. Note that before the > > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > > + * points to the new state, while afterwards it points to the old state. Due to > > + * this tricky confusion this macro is deprecated. > > + * > > + * FIXME: > > + * > > + * Replace all usage of this with one of the explicit iterators below and then > > + * remove this macro. > > + */ > > #define for_each_crtc_in_state(__state, crtc, crtc_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_crtc && \ > > @@ -539,6 +611,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (crtc_state) > > > > +/** > > + * for_each_oldnew_crtc_in_state - iterate over all CRTCs in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @crtc: &struct drm_crtc iteration cursor > > + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state > > + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all CRTCs in an atomic update, tracking both old and > > + * new state. This is useful in places where the state delta needs to be > > + * considered, for example in atomic check functions. > > + */ > > #define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_crtc && \ > > @@ -548,6 +632,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (crtc) > > > > +/** > > + * for_each_old_crtc_in_state - iterate over all CRTCs in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @crtc: &struct drm_crtc iteration cursor > > + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all CRTCs in an atomic update, tracking only the old > > + * state. This is useful in disable functions, where we need the old state the > > + * hardware is still in. > > + */ > > #define for_each_old_crtc_in_state(__state, crtc, old_crtc_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_crtc && \ > > @@ -556,6 +651,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (crtc) > > > > +/** > > + * for_each_new_crtc_in_state - iterate over all CRTCs in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @crtc: &struct drm_crtc iteration cursor > > + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all CRTCs in an atomic update, tracking only the new > > + * state. This is useful in enable functions, where we need the new state the > > + * hardware should be in when the atomic commit operation has completed. > > + */ > > #define for_each_new_crtc_in_state(__state, crtc, new_crtc_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_crtc && \ > > @@ -564,6 +670,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (crtc) > > > > +/** > > + * for_each_plane_in_state - iterate over all planes in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @plane: &struct drm_plane iteration cursor > > + * @plane_state: &struct drm_plane_state iteration cursor > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all planes in an atomic update. Note that before the > > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > > + * points to the new state, while afterwards it points to the old state. Due to > > + * this tricky confusion this macro is deprecated. > > + * > > + * FIXME: > > + * > > + * Replace all usage of this with one of the explicit iterators below and then > > + * remove this macro. > > + */ > > #define for_each_plane_in_state(__state, plane, plane_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_total_plane && \ > > @@ -572,6 +695,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (plane_state) > > > > +/** > > + * for_each_oldnew_plane_in_state - iterate over all planes in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @plane: &struct drm_plane iteration cursor > > + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state > > + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all planes in an atomic update, tracking both old and > > + * new state. This is useful in places where the state delta needs to be > > + * considered, for example in atomic check functions. > > + */ > > #define for_each_oldnew_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_total_plane && \ > > @@ -581,6 +716,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (plane) > > > > +/** > > + * for_each_old_plane_in_state - iterate over all planes in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @plane: &struct drm_plane iteration cursor > > + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all planes in an atomic update, tracking only the old > > + * state. This is useful in disable functions, where we need the old state the > > + * hardware is still in. > > + */ > > #define for_each_old_plane_in_state(__state, plane, old_plane_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_total_plane && \ > > @@ -589,6 +735,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (plane) > > > > +/** > > + * for_each_new_plane_in_state - iterate over all planes in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @plane: &struct drm_plane iteration cursor > > + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all planes in an atomic update, tracking only the new > > + * state. This is useful in enable functions, where we need the new state the > > + * hardware should be in when the atomic commit operation has completed. > > + */ > > #define for_each_new_plane_in_state(__state, plane, new_plane_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_total_plane && \ > > @@ -603,7 +760,7 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > * > > * To give drivers flexibility &struct drm_crtc_state has 3 booleans to track > > * whether the state CRTC changed enough to need a full modeset cycle: > > - * connectors_changed, mode_changed and active_changed. This helper simply > > + * planes_changed, mode_changed and active_changed. This helper simply > > * combines these three to compute the overall need for a modeset for @state. > > * > > * The atomic helper code sets these booleans, but drivers can and should > > -- > > 2.11.0 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] drm: document the all the atomic iterators 2017-03-28 15:53 ` [PATCH 3/3] drm: document the all the atomic iterators Daniel Vetter 2017-03-28 16:10 ` Alex Deucher @ 2017-03-28 16:15 ` Harry Wentland 1 sibling, 0 replies; 7+ messages in thread From: Harry Wentland @ 2017-03-28 16:15 UTC (permalink / raw) To: Daniel Vetter, Intel Graphics Development; +Cc: Daniel Vetter, DRI Development Patches 1-3 are Reviewed-by: Harry Wentland <harry.wentland@amd.com> Harry On 2017-03-28 11:53 AM, Daniel Vetter wrote: > Mostly because I want the links from the newly-added @state functions > to work. But I think explaining when they're useful and that the > implicit one is deprecated is good either way. Slightly repetitive > unfortunately. > > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > include/drm/drm_atomic.h | 159 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 158 insertions(+), 1 deletion(-) > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 0147a047878d..fd33ed5eaeb4 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -498,6 +498,23 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state); > > void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > +/** > + * for_each_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @connector_state: &struct drm_connector_state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update. Note that before the > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > + * points to the new state, while afterwards it points to the old state. Due to > + * this tricky confusion this macro is deprecated. > + * > + * FIXME: > + * > + * Replace all usage of this with one of the explicit iterators below and then > + * remove this macro. > + */ > #define for_each_connector_in_state(__state, connector, connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -506,6 +523,20 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_oldnew_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @old_connector_state: &struct drm_connector_state iteration cursor for the > + * old state > + * @new_connector_state: &struct drm_connector_state iteration cursor for the > + * new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update, tracking both old and > + * new state. This is useful in places where the state delta needs to be > + * considered, for example in atomic check functions. > + */ > #define for_each_oldnew_connector_in_state(__state, connector, old_connector_state, new_connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -515,6 +546,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_old_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @old_connector_state: &struct drm_connector_state iteration cursor for the > + * old state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update, tracking only the old > + * state. This is useful in disable functions, where we need the old state the > + * hardware is still in. > + */ > #define for_each_old_connector_in_state(__state, connector, old_connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -523,6 +566,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_new_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @new_connector_state: &struct drm_connector_state iteration cursor for the > + * new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update, tracking only the new > + * state. This is useful in enable functions, where we need the new state the > + * hardware should be in when the atomic commit operation has completed. > + */ > #define for_each_new_connector_in_state(__state, connector, new_connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -531,6 +586,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_crtc_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @crtc_state: &struct drm_crtc_state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update. Note that before the > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > + * points to the new state, while afterwards it points to the old state. Due to > + * this tricky confusion this macro is deprecated. > + * > + * FIXME: > + * > + * Replace all usage of this with one of the explicit iterators below and then > + * remove this macro. > + */ > #define for_each_crtc_in_state(__state, crtc, crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -539,6 +611,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc_state) > > +/** > + * for_each_oldnew_crtc_in_state - iterate over all CRTCs in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state > + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update, tracking both old and > + * new state. This is useful in places where the state delta needs to be > + * considered, for example in atomic check functions. > + */ > #define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -548,6 +632,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc) > > +/** > + * for_each_old_crtc_in_state - iterate over all CRTCs in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update, tracking only the old > + * state. This is useful in disable functions, where we need the old state the > + * hardware is still in. > + */ > #define for_each_old_crtc_in_state(__state, crtc, old_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -556,6 +651,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc) > > +/** > + * for_each_new_crtc_in_state - iterate over all CRTCs in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update, tracking only the new > + * state. This is useful in enable functions, where we need the new state the > + * hardware should be in when the atomic commit operation has completed. > + */ > #define for_each_new_crtc_in_state(__state, crtc, new_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -564,6 +670,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc) > > +/** > + * for_each_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @plane_state: &struct drm_plane_state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update. Note that before the > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > + * points to the new state, while afterwards it points to the old state. Due to > + * this tricky confusion this macro is deprecated. > + * > + * FIXME: > + * > + * Replace all usage of this with one of the explicit iterators below and then > + * remove this macro. > + */ > #define for_each_plane_in_state(__state, plane, plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -572,6 +695,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (plane_state) > > +/** > + * for_each_oldnew_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state > + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update, tracking both old and > + * new state. This is useful in places where the state delta needs to be > + * considered, for example in atomic check functions. > + */ > #define for_each_oldnew_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -581,6 +716,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (plane) > > +/** > + * for_each_old_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update, tracking only the old > + * state. This is useful in disable functions, where we need the old state the > + * hardware is still in. > + */ > #define for_each_old_plane_in_state(__state, plane, old_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -589,6 +735,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (plane) > > +/** > + * for_each_new_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update, tracking only the new > + * state. This is useful in enable functions, where we need the new state the > + * hardware should be in when the atomic commit operation has completed. > + */ > #define for_each_new_plane_in_state(__state, plane, new_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -603,7 +760,7 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > * > * To give drivers flexibility &struct drm_crtc_state has 3 booleans to track > * whether the state CRTC changed enough to need a full modeset cycle: > - * connectors_changed, mode_changed and active_changed. This helper simply > + * planes_changed, mode_changed and active_changed. This helper simply > * combines these three to compute the overall need for a modeset for @state. > * > * The atomic helper code sets these booleans, but drivers can and should > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/doc: remove standard connector props from the csv file 2017-03-28 15:53 [PATCH 1/3] drm/doc: remove standard connector props from the csv file Daniel Vetter 2017-03-28 15:53 ` [PATCH 2/3] drm: Document kms locking a bit better Daniel Vetter 2017-03-28 15:53 ` [PATCH 3/3] drm: document the all the atomic iterators Daniel Vetter @ 2017-03-28 17:23 ` Patchwork 2 siblings, 0 replies; 7+ messages in thread From: Patchwork @ 2017-03-28 17:23 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx == Series Details == Series: series starting with [1/3] drm/doc: remove standard connector props from the csv file URL : https://patchwork.freedesktop.org/series/22023/ State : failure == Summary == Series 22023v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/22023/revisions/1/mbox/ Test gem_exec_suspend: Subgroup basic-s3: incomplete -> PASS (fi-skl-6260u) Subgroup basic-s4-devices: dmesg-warn -> PASS (fi-kbl-7560u) fdo#100428 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: pass -> FAIL (fi-snb-2600) fdo#100428 https://bugs.freedesktop.org/show_bug.cgi?id=100428 fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 455s fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time: 466s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 584s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 542s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 567s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 506s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 508s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 439s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 442s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 440s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 515s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 492s fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 489s fi-kbl-7560u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 596s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 482s fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 606s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 486s fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 517s fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time: 457s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 552s fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time: 418s dabd992961047cf26698036f563aa86a083284ac drm-tip: 2017y-03m-28d-15h-25m-54s UTC integration manifest 8bb1098 drm: document the all the atomic iterators b113c9f drm: Document kms locking a bit better 3d8cbdb drm/doc: remove standard connector props from the csv file == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4328/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-03-29 6:35 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-28 15:53 [PATCH 1/3] drm/doc: remove standard connector props from the csv file Daniel Vetter 2017-03-28 15:53 ` [PATCH 2/3] drm: Document kms locking a bit better Daniel Vetter 2017-03-28 15:53 ` [PATCH 3/3] drm: document the all the atomic iterators Daniel Vetter 2017-03-28 16:10 ` Alex Deucher 2017-03-29 6:35 ` Daniel Vetter 2017-03-28 16:15 ` Harry Wentland 2017-03-28 17:23 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/doc: remove standard connector props from the csv file Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.