diff mbox series

[V2,2/4] drm/bridge: Introduce pre_enable_upstream_first to alter bridge init order

Message ID ec19836b0fbbc1bb53a6ba6ce93eec2184676aae.1646406653.git.dave.stevenson@raspberrypi.com (mailing list archive)
State New, archived
Headers show
Series DSI host and peripheral initialisation ordering | expand

Commit Message

Dave Stevenson March 4, 2022, 3:17 p.m. UTC
DSI sink devices typically want the DSI host powered up and configured
before they are powered up. pre_enable is the place this would normally
happen, but they are called in reverse order from panel/connector towards
the encoder, which is the "wrong" order.

Add a new flag pre_enable_upstream_first that any bridge can set
to swap the order of pre_enable (and post_disable) for that and the
immediately upstream bridge.
Should the immediately upstream bridge also set the
pre_enable_upstream_first flag, the bridge upstream of that will be called
before either of those which requested pre_enable_upstream_first.

eg:
- Panel
- Bridge 1
- Bridge 2 pre_enable_upstream_first
- Bridge 3
- Bridge 4 pre_enable_upstream_first
- Bridge 5 pre_enable_upstream_first
- Bridge 6
- Encoder
Would result in pre_enable's being called as Panel, Bridge 1, Bridge 3,
Bridge 2, Bridge 6, Bridge 5, Bridge 4, Encoder.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 drivers/gpu/drm/drm_bridge.c | 116 +++++++++++++++++++++++++++++++++----------
 include/drm/drm_bridge.h     |   8 +++
 2 files changed, 98 insertions(+), 26 deletions(-)

Comments

Jagan Teki Sept. 14, 2022, 9:46 a.m. UTC | #1
Hi Dave,

On Fri, Mar 4, 2022 at 8:48 PM Dave Stevenson
<dave.stevenson@raspberrypi.com> wrote:
>
> DSI sink devices typically want the DSI host powered up and configured
> before they are powered up. pre_enable is the place this would normally
> happen, but they are called in reverse order from panel/connector towards
> the encoder, which is the "wrong" order.
>
> Add a new flag pre_enable_upstream_first that any bridge can set
> to swap the order of pre_enable (and post_disable) for that and the
> immediately upstream bridge.
> Should the immediately upstream bridge also set the
> pre_enable_upstream_first flag, the bridge upstream of that will be called
> before either of those which requested pre_enable_upstream_first.
>
> eg:
> - Panel
> - Bridge 1
> - Bridge 2 pre_enable_upstream_first
> - Bridge 3
> - Bridge 4 pre_enable_upstream_first
> - Bridge 5 pre_enable_upstream_first
> - Bridge 6
> - Encoder
> Would result in pre_enable's being called as Panel, Bridge 1, Bridge 3,
> Bridge 2, Bridge 6, Bridge 5, Bridge 4, Encoder.

Look like this is something that I've reproduced before, isn't it?
https://patchwork.kernel.org/project/dri-devel/patch/20210214194102.126146-6-jagan@amarulasolutions.com/

Jagan.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 198fd471a488..70b513f5ce0d 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -523,6 +523,10 @@  EXPORT_SYMBOL(drm_bridge_chain_disable);
  * encoder chain, starting from the first bridge to the last. These are called
  * after completing the encoder's prepare op.
  *
+ * If a bridge sets @pre_enable_upstream_first, then the @post_disable for that
+ * bridge will be called before the previous one to reverse the @pre_enable
+ * calling direction.
+ *
  * Note: the bridge passed should be the one closest to the encoder
  */
 void drm_bridge_chain_post_disable(struct drm_bridge *bridge)
@@ -569,6 +573,9 @@  EXPORT_SYMBOL(drm_bridge_chain_mode_set);
  * chain, starting from the last bridge to the first. These are called
  * before calling the encoder's commit op.
  *
+ * If a bridge sets @pre_enable_upstream_first, then the @pre_enable for the
+ * previous bridge will be called before @pre_enable of this bridge.
+ *
  * Note: the bridge passed should be the one closest to the encoder
  */
 void drm_bridge_chain_pre_enable(struct drm_bridge *bridge)
@@ -645,6 +652,25 @@  void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge,
 }
 EXPORT_SYMBOL(drm_atomic_bridge_chain_disable);
 
+static void drm_atomic_bridge_call_post_disable(struct drm_bridge *bridge,
+						struct drm_atomic_state *old_state)
+{
+	if (old_state && bridge->funcs->atomic_post_disable) {
+		struct drm_bridge_state *old_bridge_state;
+
+		old_bridge_state =
+			drm_atomic_get_old_bridge_state(old_state,
+							bridge);
+		if (WARN_ON(!old_bridge_state))
+			return;
+
+		bridge->funcs->atomic_post_disable(bridge,
+						   old_bridge_state);
+	} else if (bridge->funcs->post_disable) {
+		bridge->funcs->post_disable(bridge);
+	}
+}
+
 /**
  * drm_atomic_bridge_chain_post_disable - cleans up after disabling all bridges
  *					  in the encoder chain
@@ -655,6 +681,9 @@  EXPORT_SYMBOL(drm_atomic_bridge_chain_disable);
  * &drm_bridge_funcs.post_disable) op for all the bridges in the encoder chain,
  * starting from the first bridge to the last. These are called after completing
  * &drm_encoder_helper_funcs.atomic_disable
+ * If a bridge sets @pre_enable_upstream_first, then the @post_disable for that
+ * bridge will be called before the previous one to reverse the @pre_enable
+ * calling direction.
  *
  * Note: the bridge passed should be the one closest to the encoder
  */
@@ -662,30 +691,55 @@  void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
 					  struct drm_atomic_state *old_state)
 {
 	struct drm_encoder *encoder;
+	struct drm_bridge *prev, *tmp;
 
 	if (!bridge)
 		return;
 
 	encoder = bridge->encoder;
-	list_for_each_entry_from(bridge, &encoder->bridge_chain, chain_node) {
-		if (old_state && bridge->funcs->atomic_post_disable) {
-			struct drm_bridge_state *old_bridge_state;
 
-			old_bridge_state =
-				drm_atomic_get_old_bridge_state(old_state,
-								bridge);
-			if (WARN_ON(!old_bridge_state))
-				return;
+	list_for_each_entry_from(bridge, &encoder->bridge_chain, chain_node) {
+		if (!list_is_last(&bridge->chain_node, &encoder->bridge_chain) &&
+		    list_next_entry(bridge, chain_node)->pre_enable_upstream_first)
+			/* Skip bridges where the downstream bridge wanted
+			 * pre_enable after / post_disable before the upstream
+			 * bridge.
+			 */
+			continue;
 
-			bridge->funcs->atomic_post_disable(bridge,
-							   old_bridge_state);
-		} else if (bridge->funcs->post_disable) {
-			bridge->funcs->post_disable(bridge);
-		}
+		/* Call this bridge and any skipped ones in reverse order */
+		tmp = bridge;
+		do {
+			prev = tmp;
+			drm_atomic_bridge_call_post_disable(tmp, old_state);
+			if (tmp == list_first_entry(&encoder->bridge_chain,
+						    struct drm_bridge, chain_node))
+				tmp = NULL;
+			else
+				tmp = list_prev_entry(tmp, chain_node);
+		} while (tmp && prev->pre_enable_upstream_first);
 	}
 }
 EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable);
 
+static void drm_atomic_bridge_call_pre_enable(struct drm_bridge *bridge,
+					      struct drm_atomic_state *old_state)
+{
+	if (old_state && bridge->funcs->atomic_pre_enable) {
+		struct drm_bridge_state *old_bridge_state;
+
+		old_bridge_state =
+			drm_atomic_get_old_bridge_state(old_state,
+							bridge);
+		if (WARN_ON(!old_bridge_state))
+			return;
+
+		bridge->funcs->atomic_pre_enable(bridge, old_bridge_state);
+	} else if (bridge->funcs->pre_enable) {
+		bridge->funcs->pre_enable(bridge);
+	}
+}
+
 /**
  * drm_atomic_bridge_chain_pre_enable - prepares for enabling all bridges in
  *					the encoder chain
@@ -697,32 +751,42 @@  EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable);
  * starting from the last bridge to the first. These are called before calling
  * &drm_encoder_helper_funcs.atomic_enable
  *
+ * If a bridge sets @pre_enable_upstream_first, then the pre_enable for the
+ * upstream bridge will be called before pre_enable of this bridge.
+ *
  * Note: the bridge passed should be the one closest to the encoder
  */
 void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge,
 					struct drm_atomic_state *old_state)
 {
 	struct drm_encoder *encoder;
-	struct drm_bridge *iter;
+	struct drm_bridge *iter, *tmp;
 
 	if (!bridge)
 		return;
 
 	encoder = bridge->encoder;
-	list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) {
-		if (old_state && iter->funcs->atomic_pre_enable) {
-			struct drm_bridge_state *old_bridge_state;
 
-			old_bridge_state =
-				drm_atomic_get_old_bridge_state(old_state,
-								iter);
-			if (WARN_ON(!old_bridge_state))
-				return;
+	list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) {
+		if (iter->pre_enable_upstream_first && iter != bridge)
+			/* Skip bridges which want the upstream pre_enable
+			 * called before their pre_enable.
+			 */
+			continue;
 
-			iter->funcs->atomic_pre_enable(iter, old_bridge_state);
-		} else if (iter->funcs->pre_enable) {
-			iter->funcs->pre_enable(iter);
-		}
+		tmp = iter;
+		do {
+			/* Work forward through the current bridge, and any
+			 * that had been skipped.
+			 */
+			drm_atomic_bridge_call_pre_enable(tmp, old_state);
+			if (tmp == list_last_entry(&encoder->bridge_chain,
+						   struct drm_bridge,
+						   chain_node))
+				tmp = NULL;
+			else
+				tmp = list_next_entry(tmp, chain_node);
+		} while (tmp && tmp->pre_enable_upstream_first);
 
 		if (iter == bridge)
 			break;
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index f27b4060faa2..cf1fb3ad7054 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -769,6 +769,14 @@  struct drm_bridge {
 	 */
 	bool interlace_allowed;
 	/**
+	 * @pre_enable_upstream_first: The bridge requires that the upstream
+	 * bridge @pre_enable function is called before its @pre_enable,
+	 * and conversely for post_disable. This is most frequently a
+	 * requirement for DSI devices which need the host to be initialised
+	 * before the peripheral.
+	 */
+	bool pre_enable_upstream_first;
+	/**
 	 * @ddc: Associated I2C adapter for DDC access, if any.
 	 */
 	struct i2c_adapter *ddc;