In function drm_atomic_bridge_chain_post_disable handling of
pre_enable_prev_first flag is broken because "next" variable will always
end up set to value of "bridge". This breaks loop which should disable
bridges in reverse:
next = list_next_entry(bridge, chain_node);
if (next->pre_enable_prev_first) {
/* next bridge had requested that prev
* was enabled first, so disabled last
*/
limit = next;
/* Find the next bridge that has NOT requested
* prev to be enabled first / disabled last
*/
list_for_each_entry_from(next, &encoder->bridge_chain,
chain_node) {
// Next condition is always true. It is likely meant to be inversed
// according to comment above. But doing this uncovers another problem:
// it won't work if there are few bridges with this flag set at the end.
if (next->pre_enable_prev_first) {
next = list_prev_entry(next, chain_node);
limit = next;
// Here we always set next = limit = branch at first iteration.
break;
}
}
/* Call these bridges in reverse order */
list_for_each_entry_from_reverse(next, &encoder->bridge_chain,
chain_node) {
// This loop never executes past this branch.
if (next == bridge)
break;
drm_atomic_bridge_call_post_disable(next, old_state);
In this patch logic for handling the flag is simplified. Temporary
"iter" variable is introduced instead of "next" which is used only
inside inner loops.
Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to alter
bridge init order")
Signed-off-by: Vladimir Lypak <vladimir.ly...@gmail.com>
---
drivers/gpu/drm/drm_bridge.c | 46 ++++++++++++++----------------------
1 file changed, 18 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index c3d69af02e79..7a5b39a46325 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -660,7 +660,7 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge
*bridge,
struct drm_atomic_state *old_state)
{
struct drm_encoder *encoder;
- struct drm_bridge *next, *limit;
+ struct drm_bridge *iter, *limit;
if (!bridge)
return;
@@ -670,36 +670,26 @@ void drm_atomic_bridge_chain_post_disable(struct
drm_bridge *bridge,
list_for_each_entry_from(bridge, &encoder->bridge_chain, chain_node) {
limit = NULL;
- if (!list_is_last(&bridge->chain_node, &encoder->bridge_chain))
{
- next = list_next_entry(bridge, chain_node);
-
- if (next->pre_enable_prev_first) {
- /* next bridge had requested that prev
- * was enabled first, so disabled last
- */
- limit = next;
-
- /* Find the next bridge that has NOT requested
- * prev to be enabled first / disabled last
- */
- list_for_each_entry_from(next,
&encoder->bridge_chain,
- chain_node) {
- if (next->pre_enable_prev_first) {
- next = list_prev_entry(next,
chain_node);
- limit = next;
- break;
- }
- }
+ /* Find sequence of bridges (bridge, limit] which requested
prev to
+ * be enabled first and disabled last
+ */
+ iter = list_next_entry(bridge, chain_node);
+ list_for_each_entry_from(iter, &encoder->bridge_chain,
chain_node) {
+ if (!iter->pre_enable_prev_first)
+ break;
+
+ limit = iter;
+ }
- /* Call these bridges in reverse order */
- list_for_each_entry_from_reverse(next,
&encoder->bridge_chain,
- chain_node) {
- if (next == bridge)
- break;
-
-
drm_atomic_bridge_call_post_disable(next,
-
old_state);
- }
+ if (limit) {
+ /* Call these bridges in reverse order */
+ iter = limit;
+ list_for_each_entry_from_reverse(iter,
+ &encoder->bridge_chain, chain_node) {
+ if (iter == bridge)
+ break;
+
+ drm_atomic_bridge_call_post_disable(iter,
old_state);
}
}
--
2.41.0
