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

Reply via email to