Hi Dave and Jagan, On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> wrote: > > Hi Dario > > On Tue, 5 Dec 2023 at 10:54, Dario Binacchi > <dario.binacchi@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > The patch fixes the code for finding the next bridge with the > > "pre_enable_prev_first" flag set to false. In case this condition is > > not verified, i. e. there is no subsequent bridge with the flag set to > > false, the whole bridge list is traversed, invalidating the "next" > > variable. > > > > The use of a new iteration variable (i. e. "iter") ensures that the value > > of the "next" variable is not invalidated. > > We already have https://patchwork.freedesktop.org/patch/529288/ that > has been reviewed (but not applied) to resolve this. What does this > version do differently and why? My patches only affect drm_atomic_bridge_chain_post_disable(), whereas Jagan's patch affects both drm_atomic_bridge_chain_post_disable() and drm_atomic_bridge_chain_pre_enable(). I tested Jagan's patch on my system with success and I reviewed with Michael Trimarchi the drm_atomic_bridge_chain_pre_enable() fixing and we think it's okay. We also believe that our changes to post_disable() are better, as we set the 'next' variable only when required, and the code is more optimized since the list_is_last() is not called within the loop. Would it be possible to use Jagan's patch for fixing drm_atomic_bridge_chain_pre_enable() and mine for fixing drm_atomic_bridge_chain_post_disable()? Thanks and regards, Dario > > Dave > > > Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to alter bridge init order") > > Co-developed-by: Michael Trimarchi <michael@xxxxxxxxxxxxxxxxxxxx> > > Signed-off-by: Michael Trimarchi <michael@xxxxxxxxxxxxxxxxxxxx> > > Signed-off-by: Dario Binacchi <dario.binacchi@xxxxxxxxxxxxxxxxxxxx> > > --- > > > > (no changes since v1) > > > > drivers/gpu/drm/drm_bridge.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > > index f66bf4925dd8..2e5781bf192e 100644 > > --- a/drivers/gpu/drm/drm_bridge.c > > +++ b/drivers/gpu/drm/drm_bridge.c > > @@ -662,7 +662,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, *next, *limit; > > > > if (!bridge) > > return; > > @@ -680,14 +680,15 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge, > > * was enabled first, so disabled last > > */ > > limit = next; > > + iter = 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, > > + list_for_each_entry_from(iter, &encoder->bridge_chain, > > chain_node) { > > - if (!next->pre_enable_prev_first) { > > - next = list_prev_entry(next, chain_node); > > + if (!iter->pre_enable_prev_first) { > > + next = list_prev_entry(iter, chain_node); > > limit = next; > > break; > > } > > -- > > 2.43.0 > > -- Dario Binacchi Senior Embedded Linux Developer dario.binacchi@xxxxxxxxxxxxxxxxxxxx __________________________________ Amarula Solutions SRL Via Le Canevare 30, 31100 Treviso, Veneto, IT T. +39 042 243 5310 info@xxxxxxxxxxxxxxxxxxxx www.amarulasolutions.com
