[PATCH 4/5] drm/atomic-helpers: Saner encoder/crtc callbacks

[Daniel Vetter <daniel.vetter@ffwll.ch>]

For historical reasons going all the way back to how the Xrandr code
was implemented the semantics of the callbacks used to enable/disable
crtcs and encoders are ... interesting.

But with atomic helpers all that complexity has been binned, with only
a well-defined on/off action left. Unfortunately the names stuck.

Let's fix that by adding enable/disable hooks every, make them the
preferred variant for atomic and update documentations.

Later on we add debug warnings when drivers have deprecated hooks. But
while everything is in-flight with lots of drivers converting to
atomic that's a bit too much - better wait for things to settle a bit
first.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 17 ++++++++++++-----
 include/drm/drm_crtc_helper.h       | 20 ++++++++++++++++++--
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index a74a22db4dea..b67754035fec 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -599,7 +599,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 			encoder->bridge->funcs->disable(encoder->bridge);
 
 		/* Right function depends upon target state. */
-		if (connector->state->crtc)
+		if (connector->state->crtc && funcs->prepare)
 			funcs->prepare(encoder);
 		else if (funcs->disable)
 			funcs->disable(encoder);
@@ -628,7 +628,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
 		funcs = crtc->helper_private;
 
 		/* Right function depends upon target state. */
-		if (crtc->state->enable)
+		if (crtc->state->enable && funcs->prepare)
 			funcs->prepare(crtc);
 		else if (funcs->disable)
 			funcs->disable(crtc);
@@ -792,8 +792,12 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev,
 
 		funcs = crtc->helper_private;
 
-		if (crtc->state->enable)
-			funcs->commit(crtc);
+		if (crtc->state->enable) {
+			if (funcs->enable)
+				funcs->enable(crtc);
+			else
+				funcs->commit(crtc);
+		}
 	}
 
 	for (i = 0; i < old_state->num_connector; i++) {
@@ -819,7 +823,10 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev,
 		if (encoder->bridge)
 			encoder->bridge->funcs->pre_enable(encoder->bridge);
 
-		funcs->commit(encoder);
+		if (funcs->enable)
+			funcs->enable(encoder);
+		else
+			funcs->commit(encoder);
 
 		if (encoder->bridge)
 			encoder->bridge->funcs->enable(encoder->bridge);
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index e76828d81a8b..c36f1bc26837 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -58,11 +58,19 @@ enum mode_set_atomic {
  * @mode_set_base_atomic: non-blocking mode set (used for kgdb support)
  * @load_lut: load color palette
  * @disable: disable CRTC when no longer in use
+ * @disable: enable CRTC
  * @atomic_check: check for validity of an atomic state
  * @atomic_begin: begin atomic update
  * @atomic_flush: flush atomic update
  *
  * The helper operations are called by the mid-layer CRTC helper.
+ *
+ * Note that with atomic helpers @dpms, @prepare and @commit hooks are
+ * deprecated. Used @enable and @disable instead exclusively.
+ *
+ * With legacy crtc helpers there's a big semantic difference between @disable
+ * and the other hooks: @disable also needs to release any resources acquired in
+ * @mode_set (like shared PLLs).
  */
 struct drm_crtc_helper_funcs {
 	/*
@@ -93,8 +101,8 @@ struct drm_crtc_helper_funcs {
 	/* reload the current crtc LUT */
 	void (*load_lut)(struct drm_crtc *crtc);
 
-	/* disable crtc when not in use - more explicit than dpms off */
 	void (*disable)(struct drm_crtc *crtc);
+	void (*enable)(struct drm_crtc *crtc);
 
 	/* atomic helpers */
 	int (*atomic_check)(struct drm_crtc *crtc,
@@ -115,8 +123,16 @@ struct drm_crtc_helper_funcs {
  * @get_crtc: return CRTC that the encoder is currently attached to
  * @detect: connection status detection
  * @disable: disable encoder when not in use (overrides DPMS off)
+ * @disable: enable encoder
  *
  * The helper operations are called by the mid-layer CRTC helper.
+ *
+ * Note that with atomic helpers @dpms, @prepare and @commit hooks are
+ * deprecated. Used @enable and @disable instead exclusively.
+ *
+ * With legacy crtc helpers there's a big semantic difference between @disable
+ * and the other hooks: @disable also needs to release any resources acquired in
+ * @mode_set (like shared PLLs).
  */
 struct drm_encoder_helper_funcs {
 	void (*dpms)(struct drm_encoder *encoder, int mode);
@@ -135,8 +151,8 @@ struct drm_encoder_helper_funcs {
 	/* detect for DAC style encoders */
 	enum drm_connector_status (*detect)(struct drm_encoder *encoder,
 					    struct drm_connector *connector);
-	/* disable encoder when not in use - more explicit than dpms off */
 	void (*disable)(struct drm_encoder *encoder);
+	void (*enable)(struct drm_encoder *encoder);
 };
 
 /**
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx