From ac576d3158a230f90bdb08510cbe4b34405baf6f Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 22 Jan 2015 16:36:24 +0100 Subject: 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. Signed-off-by: Daniel Vetter --- 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 6c47acb93249..79a444cffddc 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); }; /** -- cgit v1.2.3