Move the bridge pre_enable call before crtc enable, and the bridge post_disable call after the crtc disable.
The sequence of enable after this patch will look like: bridge[n]_pre_enable ... bridge[1]_pre_enable crtc_enable encoder_enable bridge[1]_enable ... bridge[n]__enable and vice-versa for the bridge chain disable sequence. The definition of bridge pre_enable hook says that, "The display pipe (i.e. clocks and timing signals) feeding this bridge will not yet be running when this callback is called". Since CRTC is also a source feeding the bridge, it should not be enabled before the bridges in the pipeline are pre_enabled. Fix that by re-ordering the sequence of bridge pre_enable and bridge post_disable. Signed-off-by: Aradhya Bhatia <a-bhat...@ti.com> --- drivers/gpu/drm/drm_atomic_helper.c | 165 ++++++++++++++++++---------- include/drm/drm_atomic_helper.h | 7 ++ 2 files changed, 114 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index fb97b51b38f1..e8ad08634f58 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -74,6 +74,7 @@ * also shares the &struct drm_plane_helper_funcs function table with the plane * helpers. */ + static void drm_atomic_helper_plane_changed(struct drm_atomic_state *state, struct drm_plane_state *old_plane_state, @@ -1122,11 +1123,11 @@ crtc_needs_disable(struct drm_crtc_state *old_state, } static void -disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) +disable_encoder_brige_chain(struct drm_device *dev, struct drm_atomic_state *old_state, + enum bridge_chain_operation_type op_type) { struct drm_connector *connector; struct drm_connector_state *old_conn_state, *new_conn_state; - struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state, *new_crtc_state; int i; @@ -1163,32 +1164,55 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) if (WARN_ON(!encoder)) continue; - funcs = encoder->helper_private; - - drm_dbg_atomic(dev, "disabling [ENCODER:%d:%s]\n", - encoder->base.id, encoder->name); - /* * Each encoder has at most one connector (since we always steal * it away), so we won't call disable hooks twice. */ bridge = drm_bridge_chain_get_first_bridge(encoder); - drm_atomic_bridge_chain_disable(bridge, old_state); - /* Right function depends upon target state. */ - if (funcs) { - if (funcs->atomic_disable) - funcs->atomic_disable(encoder, old_state); - else if (new_conn_state->crtc && funcs->prepare) - funcs->prepare(encoder); - else if (funcs->disable) - funcs->disable(encoder); - else if (funcs->dpms) - funcs->dpms(encoder, DRM_MODE_DPMS_OFF); - } + switch (op_type) { + case DRM_ENCODER_BRIDGE_DISABLE: + funcs = encoder->helper_private; + + drm_dbg_atomic(dev, "disabling [ENCODER:%d:%s]\n", + encoder->base.id, encoder->name); + + drm_atomic_bridge_chain_disable(bridge, old_state); + + /* Right function depends upon target state. */ + if (funcs) { + if (funcs->atomic_disable) + funcs->atomic_disable(encoder, old_state); + else if (new_conn_state->crtc && funcs->prepare) + funcs->prepare(encoder); + else if (funcs->disable) + funcs->disable(encoder); + else if (funcs->dpms) + funcs->dpms(encoder, DRM_MODE_DPMS_OFF); + } + + break; + + case DRM_BRIDGE_POST_DISABLE: + drm_atomic_bridge_chain_post_disable(bridge, old_state); - drm_atomic_bridge_chain_post_disable(bridge, old_state); + break; + + default: + drm_err(dev, "Unrecognized Encoder/Bridge Operation (%d).\n", op_type); + break; + } } +} + +static void +disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *old_crtc_state, *new_crtc_state; + int i; + + disable_encoder_brige_chain(dev, old_state, DRM_ENCODER_BRIDGE_DISABLE); for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; @@ -1234,6 +1258,8 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) if (ret == 0) drm_crtc_vblank_put(crtc); } + + disable_encoder_brige_chain(dev, old_state, DRM_BRIDGE_POST_DISABLE); } /** @@ -1445,6 +1471,64 @@ static void drm_atomic_helper_commit_writebacks(struct drm_device *dev, } } +static void +enable_encoder_brige_chain(struct drm_device *dev, struct drm_atomic_state *old_state, + enum bridge_chain_operation_type op_type) +{ + struct drm_connector *connector; + struct drm_connector_state *new_conn_state; + int i; + + for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { + const struct drm_encoder_helper_funcs *funcs; + struct drm_encoder *encoder; + struct drm_bridge *bridge; + + if (!new_conn_state->best_encoder) + continue; + + if (!new_conn_state->crtc->state->active || + !drm_atomic_crtc_needs_modeset(new_conn_state->crtc->state)) + continue; + + encoder = new_conn_state->best_encoder; + + /* + * Each encoder has at most one connector (since we always steal + * it away), so we won't call enable hooks twice. + */ + bridge = drm_bridge_chain_get_first_bridge(encoder); + + switch (op_type) { + case DRM_BRIDGE_PRE_ENABLE: + drm_atomic_bridge_chain_pre_enable(bridge, old_state); + break; + + case DRM_ENCODER_BRIDGE_ENABLE: + funcs = encoder->helper_private; + + drm_dbg_atomic(dev, "enabling [ENCODER:%d:%s]\n", + encoder->base.id, encoder->name); + + if (funcs) { + if (funcs->atomic_enable) + funcs->atomic_enable(encoder, old_state); + else if (funcs->enable) + funcs->enable(encoder); + else if (funcs->commit) + funcs->commit(encoder); + } + + drm_atomic_bridge_chain_enable(bridge, old_state); + break; + + default: + drm_err(dev, "Unrecognized Encoder/Bridge Operation (%d).\n", op_type); + break; + } + } +} + /** * drm_atomic_helper_commit_modeset_enables - modeset commit to enable outputs * @dev: DRM device @@ -1465,10 +1549,10 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, struct drm_crtc *crtc; struct drm_crtc_state *old_crtc_state; struct drm_crtc_state *new_crtc_state; - struct drm_connector *connector; - struct drm_connector_state *new_conn_state; int i; + enable_encoder_brige_chain(dev, old_state, DRM_BRIDGE_PRE_ENABLE); + for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state, new_crtc_state, i) { const struct drm_crtc_helper_funcs *funcs; @@ -1491,42 +1575,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev, } } - for_each_new_connector_in_state(old_state, connector, new_conn_state, i) { - const struct drm_encoder_helper_funcs *funcs; - struct drm_encoder *encoder; - struct drm_bridge *bridge; - - if (!new_conn_state->best_encoder) - continue; - - if (!new_conn_state->crtc->state->active || - !drm_atomic_crtc_needs_modeset(new_conn_state->crtc->state)) - continue; - - encoder = new_conn_state->best_encoder; - funcs = encoder->helper_private; - - drm_dbg_atomic(dev, "enabling [ENCODER:%d:%s]\n", - encoder->base.id, encoder->name); - - /* - * Each encoder has at most one connector (since we always steal - * it away), so we won't call enable hooks twice. - */ - bridge = drm_bridge_chain_get_first_bridge(encoder); - drm_atomic_bridge_chain_pre_enable(bridge, old_state); - - if (funcs) { - if (funcs->atomic_enable) - funcs->atomic_enable(encoder, old_state); - else if (funcs->enable) - funcs->enable(encoder); - else if (funcs->commit) - funcs->commit(encoder); - } - - drm_atomic_bridge_chain_enable(bridge, old_state); - } + enable_encoder_brige_chain(dev, old_state, DRM_ENCODER_BRIDGE_ENABLE); drm_atomic_helper_commit_writebacks(dev, old_state); } diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 9aa0a05aa072..b45a175a9f8a 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -43,6 +43,13 @@ */ #define DRM_PLANE_NO_SCALING (1<<16) +enum bridge_chain_operation_type { + DRM_BRIDGE_PRE_ENABLE, + DRM_BRIDGE_POST_DISABLE, + DRM_ENCODER_BRIDGE_ENABLE, + DRM_ENCODER_BRIDGE_DISABLE, +}; + struct drm_atomic_state; struct drm_private_obj; struct drm_private_state; -- 2.34.1