drm/atomic-helpers: Saner encoder/crtc callbacks
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.
v2: Fix kerneldoc, reported by Wu Fengguang.
Reviewed-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index d0c3611..379b6c8 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -599,7 +599,7 @@
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 @@
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 @@
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 @@
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 e76828d..4b19f7a 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -58,11 +58,19 @@
* @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
+ * @enable: 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 @@
/* 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 @@
* @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)
+ * @enable: 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 @@
/* 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);
};
/**