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, the disable sequence for the display pipeline will look like:
bridge[n]_disable
...
bridge[1]_disable
encoder_disable
crtc_disable
bridge[1]_post_disable
...
bridge[n]_post_disable
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>
Signed-off-by: Aradhya Bhatia <aradhya.bha...@linux.dev>
---
drivers/gpu/drm/drm_atomic_helper.c | 300 +++++++++++++++++-----------
1 file changed, 181 insertions(+), 119 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 5186d2114a50..ad6290a4a528 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -74,6 +74,12 @@
* also shares the &struct drm_plane_helper_funcs function table with the plane
* helpers.
*/
+
+enum bridge_chain_operation_type {
+ DRM_BRIDGE_PRE_ENABLE_OR_POST_DISABLE,
+ DRM_BRIDGE_ENABLE_OR_DISABLE,
+};
+
static void
drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
struct drm_plane_state *old_plane_state,
@@ -1122,74 +1128,12 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
}
static void
-disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
+crtc_disable(struct drm_device *dev, struct drm_atomic_state *old_state)
{
- 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;
- for_each_oldnew_connector_in_state(old_state, connector,
old_conn_state, new_conn_state, i) {
- const struct drm_encoder_helper_funcs *funcs;
- struct drm_encoder *encoder;
- struct drm_bridge *bridge;
-
- /*
- * Shut down everything that's in the changeset and currently
- * still on. So need to check the old, saved state.
- */
- if (!old_conn_state->crtc)
- continue;
-
- old_crtc_state = drm_atomic_get_old_crtc_state(old_state,
old_conn_state->crtc);
-
- if (new_conn_state->crtc)
- new_crtc_state = drm_atomic_get_new_crtc_state(
- old_state,
- new_conn_state->crtc);
- else
- new_crtc_state = NULL;
-
- if (!crtc_needs_disable(old_crtc_state, new_crtc_state) ||
- !drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state))
- continue;
-
- encoder = old_conn_state->best_encoder;
-
- /* We shouldn't get this far if we didn't previously have
- * an encoder.. but WARN_ON() rather than explode.
- */
- 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);
- }
-
- drm_atomic_bridge_chain_post_disable(bridge, old_state);
- }
-
for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
new_crtc_state, i) {
const struct drm_crtc_helper_funcs *funcs;
int ret;
@@ -1206,7 +1150,6 @@ disable_outputs(struct drm_device *dev, struct
drm_atomic_state *old_state)
drm_dbg_atomic(dev, "disabling [CRTC:%d:%s]\n",
crtc->base.id, crtc->name);
-
/* Right function depends upon target state. */
if (new_crtc_state->enable && funcs->prepare)
funcs->prepare(crtc);
@@ -1236,6 +1179,97 @@ disable_outputs(struct drm_device *dev, struct
drm_atomic_state *old_state)
}
}
+static void
+encoder_bridge_chain_disable(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_state *old_crtc_state, *new_crtc_state;
+ int i;
+
+ for_each_oldnew_connector_in_state(old_state, connector,
old_conn_state, new_conn_state, i) {
+ const struct drm_encoder_helper_funcs *funcs;
+ struct drm_encoder *encoder;
+ struct drm_bridge *bridge;
+
+ /*
+ * Shut down everything that's in the changeset and currently
+ * still on. So need to check the old, saved state.
+ */
+ if (!old_conn_state->crtc)
+ continue;
+
+ old_crtc_state = drm_atomic_get_old_crtc_state(old_state,
old_conn_state->crtc);
+
+ if (new_conn_state->crtc)
+ new_crtc_state = drm_atomic_get_new_crtc_state(
+ old_state,
+ new_conn_state->crtc);
+ else
+ new_crtc_state = NULL;
+
+ if (!crtc_needs_disable(old_crtc_state, new_crtc_state) ||
+ !drm_atomic_crtc_needs_modeset(old_conn_state->crtc->state))
+ continue;
+
+ encoder = old_conn_state->best_encoder;
+
+ /* We shouldn't get this far if we didn't previously have
+ * an encoder.. but WARN_ON() rather than explode.
+ */
+ if (WARN_ON(!encoder))
+ continue;
+
+ /*
+ * 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);
+
+ switch (op_type) {
+ case DRM_BRIDGE_ENABLE_OR_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_PRE_ENABLE_OR_POST_DISABLE:
+ drm_atomic_bridge_chain_post_disable(bridge, old_state);
+ break;
+
+ default:
+ drm_err(dev, "Unrecognized Encoder/Bridge operation
(%d).\n", op_type);
+ }
+ }
+}
+
+static void
+disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
+{
+ encoder_bridge_chain_disable(dev, old_state,
DRM_BRIDGE_ENABLE_OR_DISABLE);
+
+ crtc_disable(dev, old_state);
+
+ encoder_bridge_chain_disable(dev, old_state,
DRM_BRIDGE_PRE_ENABLE_OR_POST_DISABLE);
+}
+
/**
* drm_atomic_helper_update_legacy_modeset_state - update legacy modeset state
* @dev: DRM device
@@ -1445,28 +1479,69 @@ static void drm_atomic_helper_commit_writebacks(struct
drm_device *dev,
}
}
-/**
- * drm_atomic_helper_commit_modeset_enables - modeset commit to enable outputs
- * @dev: DRM device
- * @old_state: atomic state object with old state structures
- *
- * This function enables all the outputs with the new configuration which had
to
- * be turned off for the update.
- *
- * For compatibility with legacy CRTC helpers this should be called after
- * drm_atomic_helper_commit_planes(), which is what the default commit function
- * does. But drivers with different needs can group the modeset commits
together
- * and do the plane commits at the end. This is useful for drivers doing
runtime
- * PM since planes updates then only happen when the CRTC is actually enabled.
- */
-void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
- struct drm_atomic_state
*old_state)
+static void
+encoder_bridge_chain_enable(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_OR_POST_DISABLE:
+ drm_atomic_bridge_chain_pre_enable(bridge, old_state);
+ break;
+
+ case DRM_BRIDGE_ENABLE_OR_DISABLE:
+ 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;
+ }
+ }
+}
+
+static void
+crtc_enable(struct drm_device *dev, struct drm_atomic_state *old_state)
{
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;
for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
new_crtc_state, i) {
@@ -1490,43 +1565,30 @@ void drm_atomic_helper_commit_modeset_enables(struct
drm_device *dev,
funcs->commit(crtc);
}
}
-
- 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);
- }
+}
+
+/**
+ * drm_atomic_helper_commit_modeset_enables - modeset commit to enable outputs
+ * @dev: DRM device
+ * @old_state: atomic state object with old state structures
+ *
+ * This function enables all the outputs with the new configuration which had
to
+ * be turned off for the update.
+ *
+ * For compatibility with legacy CRTC helpers this should be called after
+ * drm_atomic_helper_commit_planes(), which is what the default commit function
+ * does. But drivers with different needs can group the modeset commits
together
+ * and do the plane commits at the end. This is useful for drivers doing
runtime
+ * PM since planes updates then only happen when the CRTC is actually enabled.
+ */
+void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
+ struct drm_atomic_state
*old_state)
+{
+ encoder_bridge_chain_enable(dev, old_state,
DRM_BRIDGE_PRE_ENABLE_OR_POST_DISABLE);
+
+ crtc_enable(dev, old_state);
+
+ encoder_bridge_chain_enable(dev, old_state,
DRM_BRIDGE_ENABLE_OR_DISABLE);
drm_atomic_helper_commit_writebacks(dev, old_state);
}
--
2.34.1
