linux-kernel - [PATCH] drm: atomic: Clarify documentation around drm_atomic_crtc_needs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 13 Oct 2016 10:47:08 +0100
From:   Brian Starkey <brian.starkey@....com>
To:     daniel@...ll.ch, dri-devel@...ts.freedesktop.org
Cc:     linux-kernel@...r.kernel.org
Subject: [PATCH] drm: atomic: Clarify documentation around drm_atomic_crtc_needs_modeset

Add some additional comments to more explicitly describe the meaning and
usage of the three CRTC modeset detection booleans: mode_changed,
connectors_changed and active_changed.

Suggested-by: Daniel Vetter <daniel.vetter@...ll.ch>
Signed-off-by: Brian Starkey <brian.starkey@....com>
---

Hi Daniel,

I guess I asked for this one :-), please just check my understanding
is correct.

Thanks,
Brian

 drivers/gpu/drm/drm_atomic_helper.c |    9 +++++----
 include/drm/drm_atomic.h            |   11 ++++++++++-
 include/drm/drm_crtc.h              |    5 +++++
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 78ea735..fb4071a 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -458,10 +458,11 @@ mode_fixup(struct drm_atomic_state *state)
  * removed from the crtc.
  * crtc_state->active_changed is set when crtc_state->active changes,
  * which is used for dpms.
+ * See also: drm_atomic_crtc_needs_modeset()
  *
  * IMPORTANT:
  *
- * Drivers which update ->mode_changed (e.g. in their ->atomic_check hooks if a
+ * Drivers which set ->mode_changed (e.g. in their ->atomic_check hooks if a
  * plane update can't be done without a full modeset) _must_ call this function
  * afterwards after that change. It is permitted to call this function multiple
  * times for the same update, e.g. when the ->atomic_check functions depend upon
@@ -510,9 +511,9 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
 
 	for_each_connector_in_state(state, connector, connector_state, i) {
 		/*
-		 * This only sets crtc->mode_changed for routing changes,
-		 * drivers must set crtc->mode_changed themselves when connector
-		 * properties need to be updated.
+		 * This only sets crtc->connectors_changed for routing changes,
+		 * drivers must set crtc->connectors_changed themselves when
+		 * connector properties need to be updated.
 		 */
 		ret = update_connector_routing(state, connector,
 					       connector_state);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index d9aff06..1ce255f 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -368,8 +368,17 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state);
  *
  * 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_change. This helper simply
+ * connectors_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
+ * change them appropriately to accurately represent whether a modeset is
+ * really needed. In general, drivers should avoid full modesets whenever
+ * possible.
+ *
+ * For example if the CRTC mode has changed, and the hardware is able to enact
+ * the requested mode change without going through a full modeset, the driver
+ * should clear mode_changed during its ->atomic_check.
  */
 static inline bool
 drm_atomic_crtc_needs_modeset(struct drm_crtc_state *state)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c4a3164..1f094d2 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -116,6 +116,11 @@ struct drm_plane_helper_funcs;
  * never return in a failure from the ->atomic_check callback. Userspace assumes
  * that a DPMS On will always succeed. In other words: @enable controls resource
  * assignment, @active controls the actual hardware state.
+ *
+ * The three booleans active_changed, connectors_changed and mode_changed are
+ * intended to indicate whether a full modeset is needed, rather than strictly
+ * describing what has changed in a commit.
+ * See also: drm_atomic_crtc_needs_modeset()
  */
 struct drm_crtc_state {
 	struct drm_crtc *crtc;
-- 
1.7.9.5