Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
Hi Robert On Tue, Mar 5, 2024 at 3:54 PM Robert Foss wrote: > > On Tue, 28 Mar 2023 22:37:51 +0530, Jagan Teki wrote: > > For a given bridge pipeline if any bridge sets pre_enable_prev_first > > flag then the pre_enable for the previous bridge will be called before > > pre_enable of this bridge and opposite is done for post_disable. > > > > These are the potential bridge flags to alter bridge init order in order > > to satisfy the MIPI DSI host and downstream panel or bridge to function. > > However the existing pre_enable_prev_first logic with associated bridge > > ordering has broken for both pre_enable and post_disable calls. > > > > [...] > > Please excuse the delay, patches touching the core bridge code are a little > bit tougher to merge due to increased risks of breaking unrelated things. > > Applied, thanks! > I have a question about this prev_first flag. Can we map the order in the connector in dts? Michael > [1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e18aeeda0b69 > [2/2] drm/bridge: Document bridge init order with pre_enable_prev_first > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=113cc3ad8566 > > > > Rob > > -- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 mich...@amarulasolutions.com __ Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 i...@amarulasolutions.com www.amarulasolutions.com
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
On Tue, 28 Mar 2023 22:37:51 +0530, Jagan Teki wrote: > For a given bridge pipeline if any bridge sets pre_enable_prev_first > flag then the pre_enable for the previous bridge will be called before > pre_enable of this bridge and opposite is done for post_disable. > > These are the potential bridge flags to alter bridge init order in order > to satisfy the MIPI DSI host and downstream panel or bridge to function. > However the existing pre_enable_prev_first logic with associated bridge > ordering has broken for both pre_enable and post_disable calls. > > [...] Please excuse the delay, patches touching the core bridge code are a little bit tougher to merge due to increased risks of breaking unrelated things. Applied, thanks! [1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e18aeeda0b69 [2/2] drm/bridge: Document bridge init order with pre_enable_prev_first https://cgit.freedesktop.org/drm/drm-misc/commit/?id=113cc3ad8566 Rob
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
On Thu, Feb 29, 2024 at 12:39 PM Frieder Schrempf
wrote:
>
> Hi,
>
> On 28.03.23 19:07, Jagan Teki wrote:
> > For a given bridge pipeline if any bridge sets pre_enable_prev_first
> > flag then the pre_enable for the previous bridge will be called before
> > pre_enable of this bridge and opposite is done for post_disable.
> >
> > These are the potential bridge flags to alter bridge init order in order
> > to satisfy the MIPI DSI host and downstream panel or bridge to function.
> > However the existing pre_enable_prev_first logic with associated bridge
> > ordering has broken for both pre_enable and post_disable calls.
> >
> > [pre_enable]
> >
> > The altered bridge ordering has failed if two consecutive bridges on a
> > given pipeline enables the pre_enable_prev_first flag.
> >
> > Example:
> > - Panel
> > - Bridge 1
> > - Bridge 2 pre_enable_prev_first
> > - Bridge 3
> > - Bridge 4 pre_enable_prev_first
> > - Bridge 5 pre_enable_prev_first
> > - Bridge 6
> > - Encoder
> >
> > In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first.
> >
> > The logic looks for a bridge which enabled pre_enable_prev_first flag
> > on each iteration and assigned the previou bridge to limit pointer
> > if the bridge doesn't enable pre_enable_prev_first flags.
> >
> > If control found Bridge 2 is pre_enable_prev_first then the iteration
> > looks for Bridge 3 and found it is not pre_enable_prev_first and assigns
> > it's previous Bridge 4 to limit pointer and calls pre_enable of Bridge 3
> > and Bridge 2 and assign iter pointer with limit which is Bridge 4.
> >
> > Here is the actual problem, for the next iteration control look for
> > Bridge 5 instead of Bridge 4 has iter pointer in previous iteration
> > moved to Bridge 4 so this iteration skips the Bridge 4. The iteration
> > found Bridge 6 doesn't pre_enable_prev_first flags so the limit assigned
> > to Encoder. From next iteration Encoder skips as it is the last bridge
> > for reverse order pipeline.
> >
> > So, the resulting pre_enable bridge order would be,
> > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5.
> >
> > This patch fixes this by assigning limit to next pointer instead of
> > previous bridge since the iteration always looks for bridge that does
> > NOT request prev so assigning next makes sure the last bridge on a
> > given iteration what exactly the limit bridge is.
> >
> > So, the resulting pre_enable bridge order with fix would be,
> > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4,
> > Encoder.
> >
> > [post_disable]
> >
> > The altered bridge ordering has failed if two consecutive bridges on a
> > given pipeline enables the pre_enable_prev_first flag.
> >
> > Example:
> > - Panel
> > - Bridge 1
> > - Bridge 2 pre_enable_prev_first
> > - Bridge 3
> > - Bridge 4 pre_enable_prev_first
> > - Bridge 5 pre_enable_prev_first
> > - Bridge 6
> > - Encoder
> >
> > In this example Bridge 5 and Bridge 4 have pre_enable_prev_first.
> >
> > The logic looks for a bridge which enabled pre_enable_prev_first flags
> > on each iteration and assigned the previou bridge to next and next to
> > limit pointer if the bridge does enable pre_enable_prev_first flag.
> >
> > If control starts from Bridge 6 then it found next Bridge 5 is
> > pre_enable_prev_first and immediately the next assigned to previous
> > Bridge 6 and limit assignments to next Bridge 6 and call post_enable
> > of Bridge 6 even though the next consecutive Bridge 5 is enabled with
> > pre_enable_prev_first. This clearly misses the logic to find the state
> > of next conducive bridge as everytime the next and limit assigns
> > previous bridge if given bridge enabled pre_enable_prev_first.
> >
> > So, the resulting post_disable bridge order would be,
> > - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1,
> > Panel.
> >
> > This patch fixes this by assigning next with previou bridge only if the
> > bridge doesn't enable pre_enable_prev_first flag and the next further
> > assign it to limit. This way we can find the bridge that NOT requested
> > prev to disable last.
> >
> > So, the resulting pre_enable bridge order with fix would be,
> > - Encoder, Bridge 4, Bridge 5, Bridge 6, Bridge 2, Bridge 3, Bridge 1,
> > Panel.
> >
> > Validated the bridge init ordering by incorporating dummy bridges in
> > the sun6i-mipi-dsi pipeline
> >
> > Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to
> > alter bridge init order")
> > Signed-off-by: Jagan Teki
>
> This patch is now almost 1 year old and it has been tested and reviewed
> and there have been multiple pings.
>
> Is there anything missing? Why is it not applied yet?
Sorry about the delay. This has been tested and reviewed properly, so
I will apply it now.
>
> Andrzej, Neil, Robert: As DRM bridge maintainers, can you take care of this?
>
> Thanks
> Frieder
>
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
Hi,
On 28.03.23 19:07, Jagan Teki wrote:
> For a given bridge pipeline if any bridge sets pre_enable_prev_first
> flag then the pre_enable for the previous bridge will be called before
> pre_enable of this bridge and opposite is done for post_disable.
>
> These are the potential bridge flags to alter bridge init order in order
> to satisfy the MIPI DSI host and downstream panel or bridge to function.
> However the existing pre_enable_prev_first logic with associated bridge
> ordering has broken for both pre_enable and post_disable calls.
>
> [pre_enable]
>
> The altered bridge ordering has failed if two consecutive bridges on a
> given pipeline enables the pre_enable_prev_first flag.
>
> Example:
> - Panel
> - Bridge 1
> - Bridge 2 pre_enable_prev_first
> - Bridge 3
> - Bridge 4 pre_enable_prev_first
> - Bridge 5 pre_enable_prev_first
> - Bridge 6
> - Encoder
>
> In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first.
>
> The logic looks for a bridge which enabled pre_enable_prev_first flag
> on each iteration and assigned the previou bridge to limit pointer
> if the bridge doesn't enable pre_enable_prev_first flags.
>
> If control found Bridge 2 is pre_enable_prev_first then the iteration
> looks for Bridge 3 and found it is not pre_enable_prev_first and assigns
> it's previous Bridge 4 to limit pointer and calls pre_enable of Bridge 3
> and Bridge 2 and assign iter pointer with limit which is Bridge 4.
>
> Here is the actual problem, for the next iteration control look for
> Bridge 5 instead of Bridge 4 has iter pointer in previous iteration
> moved to Bridge 4 so this iteration skips the Bridge 4. The iteration
> found Bridge 6 doesn't pre_enable_prev_first flags so the limit assigned
> to Encoder. From next iteration Encoder skips as it is the last bridge
> for reverse order pipeline.
>
> So, the resulting pre_enable bridge order would be,
> - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5.
>
> This patch fixes this by assigning limit to next pointer instead of
> previous bridge since the iteration always looks for bridge that does
> NOT request prev so assigning next makes sure the last bridge on a
> given iteration what exactly the limit bridge is.
>
> So, the resulting pre_enable bridge order with fix would be,
> - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4,
> Encoder.
>
> [post_disable]
>
> The altered bridge ordering has failed if two consecutive bridges on a
> given pipeline enables the pre_enable_prev_first flag.
>
> Example:
> - Panel
> - Bridge 1
> - Bridge 2 pre_enable_prev_first
> - Bridge 3
> - Bridge 4 pre_enable_prev_first
> - Bridge 5 pre_enable_prev_first
> - Bridge 6
> - Encoder
>
> In this example Bridge 5 and Bridge 4 have pre_enable_prev_first.
>
> The logic looks for a bridge which enabled pre_enable_prev_first flags
> on each iteration and assigned the previou bridge to next and next to
> limit pointer if the bridge does enable pre_enable_prev_first flag.
>
> If control starts from Bridge 6 then it found next Bridge 5 is
> pre_enable_prev_first and immediately the next assigned to previous
> Bridge 6 and limit assignments to next Bridge 6 and call post_enable
> of Bridge 6 even though the next consecutive Bridge 5 is enabled with
> pre_enable_prev_first. This clearly misses the logic to find the state
> of next conducive bridge as everytime the next and limit assigns
> previous bridge if given bridge enabled pre_enable_prev_first.
>
> So, the resulting post_disable bridge order would be,
> - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1,
> Panel.
>
> This patch fixes this by assigning next with previou bridge only if the
> bridge doesn't enable pre_enable_prev_first flag and the next further
> assign it to limit. This way we can find the bridge that NOT requested
> prev to disable last.
>
> So, the resulting pre_enable bridge order with fix would be,
> - Encoder, Bridge 4, Bridge 5, Bridge 6, Bridge 2, Bridge 3, Bridge 1,
> Panel.
>
> Validated the bridge init ordering by incorporating dummy bridges in
> the sun6i-mipi-dsi pipeline
>
> Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to
> alter bridge init order")
> Signed-off-by: Jagan Teki
This patch is now almost 1 year old and it has been tested and reviewed
and there have been multiple pings.
Is there anything missing? Why is it not applied yet?
Andrzej, Neil, Robert: As DRM bridge maintainers, can you take care of this?
Thanks
Frieder
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
On Sun, Nov 26, 2023 at 9:41 PM Jagan Teki wrote: > > On Mon, Nov 13, 2023 at 6:45 PM Jagan Teki wrote: > > > > On Tue, Aug 1, 2023 at 11:50 AM Dave Stevenson > > wrote: > > > > > > Hi Jagan > > > > > > My apologies for dropping the ball on this one, and thanks to Frieder > > > for the nudge. > > > > > > On Wed, 12 Apr 2023 at 07:25, Jagan Teki > > > wrote: > > > > > > > > Hi Dave, > > > > > > > > Added Maxime, Laurent [which I thought I added before] > > > > > > > > On Tue, Mar 28, 2023 at 10:38 PM Jagan Teki > > > > wrote: > > > > > > > > > > For a given bridge pipeline if any bridge sets pre_enable_prev_first > > > > > flag then the pre_enable for the previous bridge will be called before > > > > > pre_enable of this bridge and opposite is done for post_disable. > > > > > > > > > > These are the potential bridge flags to alter bridge init order in > > > > > order > > > > > to satisfy the MIPI DSI host and downstream panel or bridge to > > > > > function. > > > > > However the existing pre_enable_prev_first logic with associated > > > > > bridge > > > > > ordering has broken for both pre_enable and post_disable calls. > > > > > > > > > > [pre_enable] > > > > > > > > > > The altered bridge ordering has failed if two consecutive bridges on a > > > > > given pipeline enables the pre_enable_prev_first flag. > > > > > > > > > > Example: > > > > > - Panel > > > > > - Bridge 1 > > > > > - Bridge 2 pre_enable_prev_first > > > > > - Bridge 3 > > > > > - Bridge 4 pre_enable_prev_first > > > > > - Bridge 5 pre_enable_prev_first > > > > > - Bridge 6 > > > > > - Encoder > > > > > > > > > > In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first. > > > > > > > > > > The logic looks for a bridge which enabled pre_enable_prev_first flag > > > > > on each iteration and assigned the previou bridge to limit pointer > > > > > if the bridge doesn't enable pre_enable_prev_first flags. > > > > > > > > > > If control found Bridge 2 is pre_enable_prev_first then the iteration > > > > > looks for Bridge 3 and found it is not pre_enable_prev_first and > > > > > assigns > > > > > it's previous Bridge 4 to limit pointer and calls pre_enable of > > > > > Bridge 3 > > > > > and Bridge 2 and assign iter pointer with limit which is Bridge 4. > > > > > > > > > > Here is the actual problem, for the next iteration control look for > > > > > Bridge 5 instead of Bridge 4 has iter pointer in previous iteration > > > > > moved to Bridge 4 so this iteration skips the Bridge 4. The iteration > > > > > found Bridge 6 doesn't pre_enable_prev_first flags so the limit > > > > > assigned > > > > > to Encoder. From next iteration Encoder skips as it is the last bridge > > > > > for reverse order pipeline. > > > > > > > > > > So, the resulting pre_enable bridge order would be, > > > > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5. > > > > > > > > > > This patch fixes this by assigning limit to next pointer instead of > > > > > previous bridge since the iteration always looks for bridge that does > > > > > NOT request prev so assigning next makes sure the last bridge on a > > > > > given iteration what exactly the limit bridge is. > > > > > > > > > > So, the resulting pre_enable bridge order with fix would be, > > > > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4, > > > > > Encoder. > > > > > > > > > > [post_disable] > > > > > > > > > > The altered bridge ordering has failed if two consecutive bridges on a > > > > > given pipeline enables the pre_enable_prev_first flag. > > > > > > > > > > Example: > > > > > - Panel > > > > > - Bridge 1 > > > > > - Bridge 2 pre_enable_prev_first > > > > > - Bridge 3 > > > > > - Bridge 4 pre_enable_prev_first > > > > > - Bridge 5 pre_enable_prev_first > > > > > - Bridge 6 > > > > > - Encoder > > > > > > > > > > In this example Bridge 5 and Bridge 4 have pre_enable_prev_first. > > > > > > > > > > The logic looks for a bridge which enabled pre_enable_prev_first flags > > > > > on each iteration and assigned the previou bridge to next and next to > > > > > limit pointer if the bridge does enable pre_enable_prev_first flag. > > > > > > > > > > If control starts from Bridge 6 then it found next Bridge 5 is > > > > > pre_enable_prev_first and immediately the next assigned to previous > > > > > Bridge 6 and limit assignments to next Bridge 6 and call post_enable > > > > > of Bridge 6 even though the next consecutive Bridge 5 is enabled with > > > > > pre_enable_prev_first. This clearly misses the logic to find the state > > > > > of next conducive bridge as everytime the next and limit assigns > > > > > previous bridge if given bridge enabled pre_enable_prev_first. > > > > > > > > > > So, the resulting post_disable bridge order would be, > > > > > - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1, > > > > > Panel. > > > > > > > > > > This patch fixes this by assigning next with previou bridge only if > > > > > the > > > > > bridge doesn't enab
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
On Mon, Dec 18, 2023 at 11:28 AM Jagan Teki wrote: > > On Sun, Nov 26, 2023 at 9:41 PM Jagan Teki wrote: > > > > On Mon, Nov 13, 2023 at 6:45 PM Jagan Teki > > wrote: > > > > > > On Tue, Aug 1, 2023 at 11:50 AM Dave Stevenson > > > wrote: > > > > > > > > Hi Jagan > > > > > > > > My apologies for dropping the ball on this one, and thanks to Frieder > > > > for the nudge. > > > > > > > > On Wed, 12 Apr 2023 at 07:25, Jagan Teki > > > > wrote: > > > > > > > > > > Hi Dave, > > > > > > > > > > Added Maxime, Laurent [which I thought I added before] > > > > > > > > > > On Tue, Mar 28, 2023 at 10:38 PM Jagan Teki > > > > > wrote: > > > > > > > > > > > > For a given bridge pipeline if any bridge sets pre_enable_prev_first > > > > > > flag then the pre_enable for the previous bridge will be called > > > > > > before > > > > > > pre_enable of this bridge and opposite is done for post_disable. > > > > > > > > > > > > These are the potential bridge flags to alter bridge init order in > > > > > > order > > > > > > to satisfy the MIPI DSI host and downstream panel or bridge to > > > > > > function. > > > > > > However the existing pre_enable_prev_first logic with associated > > > > > > bridge > > > > > > ordering has broken for both pre_enable and post_disable calls. > > > > > > > > > > > > [pre_enable] > > > > > > > > > > > > The altered bridge ordering has failed if two consecutive bridges > > > > > > on a > > > > > > given pipeline enables the pre_enable_prev_first flag. > > > > > > > > > > > > Example: > > > > > > - Panel > > > > > > - Bridge 1 > > > > > > - Bridge 2 pre_enable_prev_first > > > > > > - Bridge 3 > > > > > > - Bridge 4 pre_enable_prev_first > > > > > > - Bridge 5 pre_enable_prev_first > > > > > > - Bridge 6 > > > > > > - Encoder > > > > > > > > > > > > In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first. > > > > > > > > > > > > The logic looks for a bridge which enabled pre_enable_prev_first > > > > > > flag > > > > > > on each iteration and assigned the previou bridge to limit pointer > > > > > > if the bridge doesn't enable pre_enable_prev_first flags. > > > > > > > > > > > > If control found Bridge 2 is pre_enable_prev_first then the > > > > > > iteration > > > > > > looks for Bridge 3 and found it is not pre_enable_prev_first and > > > > > > assigns > > > > > > it's previous Bridge 4 to limit pointer and calls pre_enable of > > > > > > Bridge 3 > > > > > > and Bridge 2 and assign iter pointer with limit which is Bridge 4. > > > > > > > > > > > > Here is the actual problem, for the next iteration control look for > > > > > > Bridge 5 instead of Bridge 4 has iter pointer in previous iteration > > > > > > moved to Bridge 4 so this iteration skips the Bridge 4. The > > > > > > iteration > > > > > > found Bridge 6 doesn't pre_enable_prev_first flags so the limit > > > > > > assigned > > > > > > to Encoder. From next iteration Encoder skips as it is the last > > > > > > bridge > > > > > > for reverse order pipeline. > > > > > > > > > > > > So, the resulting pre_enable bridge order would be, > > > > > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5. > > > > > > > > > > > > This patch fixes this by assigning limit to next pointer instead of > > > > > > previous bridge since the iteration always looks for bridge that > > > > > > does > > > > > > NOT request prev so assigning next makes sure the last bridge on a > > > > > > given iteration what exactly the limit bridge is. > > > > > > > > > > > > So, the resulting pre_enable bridge order with fix would be, > > > > > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4, > > > > > > Encoder. > > > > > > > > > > > > [post_disable] > > > > > > > > > > > > The altered bridge ordering has failed if two consecutive bridges > > > > > > on a > > > > > > given pipeline enables the pre_enable_prev_first flag. > > > > > > > > > > > > Example: > > > > > > - Panel > > > > > > - Bridge 1 > > > > > > - Bridge 2 pre_enable_prev_first > > > > > > - Bridge 3 > > > > > > - Bridge 4 pre_enable_prev_first > > > > > > - Bridge 5 pre_enable_prev_first > > > > > > - Bridge 6 > > > > > > - Encoder > > > > > > > > > > > > In this example Bridge 5 and Bridge 4 have pre_enable_prev_first. > > > > > > > > > > > > The logic looks for a bridge which enabled pre_enable_prev_first > > > > > > flags > > > > > > on each iteration and assigned the previou bridge to next and next > > > > > > to > > > > > > limit pointer if the bridge does enable pre_enable_prev_first flag. > > > > > > > > > > > > If control starts from Bridge 6 then it found next Bridge 5 is > > > > > > pre_enable_prev_first and immediately the next assigned to previous > > > > > > Bridge 6 and limit assignments to next Bridge 6 and call post_enable > > > > > > of Bridge 6 even though the next consecutive Bridge 5 is enabled > > > > > > with > > > > > > pre_enable_prev_first. This clearly misses the logic to find the > > > > > > state > > > > > > of next con
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
Hi Jagan On Sun, Nov 26, 2023 at 5:11 PM Jagan Teki wrote: > > On Mon, Nov 13, 2023 at 6:45 PM Jagan Teki wrote: > > > > On Tue, Aug 1, 2023 at 11:50 AM Dave Stevenson > > wrote: > > > > > > Hi Jagan > > > > > > My apologies for dropping the ball on this one, and thanks to Frieder > > > for the nudge. > > > > > > On Wed, 12 Apr 2023 at 07:25, Jagan Teki > > > wrote: > > > > > > > > Hi Dave, > > > > > > > > Added Maxime, Laurent [which I thought I added before] > > > > > > > > On Tue, Mar 28, 2023 at 10:38 PM Jagan Teki > > > > wrote: > > > > > > > > > > For a given bridge pipeline if any bridge sets pre_enable_prev_first > > > > > flag then the pre_enable for the previous bridge will be called before > > > > > pre_enable of this bridge and opposite is done for post_disable. > > > > > > > > > > These are the potential bridge flags to alter bridge init order in > > > > > order > > > > > to satisfy the MIPI DSI host and downstream panel or bridge to > > > > > function. > > > > > However the existing pre_enable_prev_first logic with associated > > > > > bridge > > > > > ordering has broken for both pre_enable and post_disable calls. > > > > > > > > > > [pre_enable] > > > > > > > > > > The altered bridge ordering has failed if two consecutive bridges on a > > > > > given pipeline enables the pre_enable_prev_first flag. > > > > > > > > > > Example: > > > > > - Panel > > > > > - Bridge 1 > > > > > - Bridge 2 pre_enable_prev_first > > > > > - Bridge 3 > > > > > - Bridge 4 pre_enable_prev_first > > > > > - Bridge 5 pre_enable_prev_first > > > > > - Bridge 6 > > > > > - Encoder > > > > > > > > > > In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first. > > > > > > > > > > The logic looks for a bridge which enabled pre_enable_prev_first flag > > > > > on each iteration and assigned the previou bridge to limit pointer > > > > > if the bridge doesn't enable pre_enable_prev_first flags. > > > > > > > > > > If control found Bridge 2 is pre_enable_prev_first then the iteration > > > > > looks for Bridge 3 and found it is not pre_enable_prev_first and > > > > > assigns > > > > > it's previous Bridge 4 to limit pointer and calls pre_enable of > > > > > Bridge 3 > > > > > and Bridge 2 and assign iter pointer with limit which is Bridge 4. > > > > > > > > > > Here is the actual problem, for the next iteration control look for > > > > > Bridge 5 instead of Bridge 4 has iter pointer in previous iteration > > > > > moved to Bridge 4 so this iteration skips the Bridge 4. The iteration > > > > > found Bridge 6 doesn't pre_enable_prev_first flags so the limit > > > > > assigned > > > > > to Encoder. From next iteration Encoder skips as it is the last bridge > > > > > for reverse order pipeline. > > > > > > > > > > So, the resulting pre_enable bridge order would be, > > > > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5. > > > > > > > > > > This patch fixes this by assigning limit to next pointer instead of > > > > > previous bridge since the iteration always looks for bridge that does > > > > > NOT request prev so assigning next makes sure the last bridge on a > > > > > given iteration what exactly the limit bridge is. > > > > > > > > > > So, the resulting pre_enable bridge order with fix would be, > > > > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4, > > > > > Encoder. > > > > > > > > > > [post_disable] > > > > > > > > > > The altered bridge ordering has failed if two consecutive bridges on a > > > > > given pipeline enables the pre_enable_prev_first flag. > > > > > > > > > > Example: > > > > > - Panel > > > > > - Bridge 1 > > > > > - Bridge 2 pre_enable_prev_first > > > > > - Bridge 3 > > > > > - Bridge 4 pre_enable_prev_first > > > > > - Bridge 5 pre_enable_prev_first > > > > > - Bridge 6 > > > > > - Encoder > > > > > > > > > > In this example Bridge 5 and Bridge 4 have pre_enable_prev_first. > > > > > > > > > > The logic looks for a bridge which enabled pre_enable_prev_first flags > > > > > on each iteration and assigned the previou bridge to next and next to > > > > > limit pointer if the bridge does enable pre_enable_prev_first flag. > > > > > > > > > > If control starts from Bridge 6 then it found next Bridge 5 is > > > > > pre_enable_prev_first and immediately the next assigned to previous > > > > > Bridge 6 and limit assignments to next Bridge 6 and call post_enable > > > > > of Bridge 6 even though the next consecutive Bridge 5 is enabled with > > > > > pre_enable_prev_first. This clearly misses the logic to find the state > > > > > of next conducive bridge as everytime the next and limit assigns > > > > > previous bridge if given bridge enabled pre_enable_prev_first. > > > > > > > > > > So, the resulting post_disable bridge order would be, > > > > > - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1, > > > > > Panel. > > > > > > > > > > This patch fixes this by assigning next with previou bridge only if > > > > > the > > > > > bridge do
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
On Mon, Nov 13, 2023 at 6:45 PM Jagan Teki wrote: > > On Tue, Aug 1, 2023 at 11:50 AM Dave Stevenson > wrote: > > > > Hi Jagan > > > > My apologies for dropping the ball on this one, and thanks to Frieder > > for the nudge. > > > > On Wed, 12 Apr 2023 at 07:25, Jagan Teki wrote: > > > > > > Hi Dave, > > > > > > Added Maxime, Laurent [which I thought I added before] > > > > > > On Tue, Mar 28, 2023 at 10:38 PM Jagan Teki > > > wrote: > > > > > > > > For a given bridge pipeline if any bridge sets pre_enable_prev_first > > > > flag then the pre_enable for the previous bridge will be called before > > > > pre_enable of this bridge and opposite is done for post_disable. > > > > > > > > These are the potential bridge flags to alter bridge init order in order > > > > to satisfy the MIPI DSI host and downstream panel or bridge to function. > > > > However the existing pre_enable_prev_first logic with associated bridge > > > > ordering has broken for both pre_enable and post_disable calls. > > > > > > > > [pre_enable] > > > > > > > > The altered bridge ordering has failed if two consecutive bridges on a > > > > given pipeline enables the pre_enable_prev_first flag. > > > > > > > > Example: > > > > - Panel > > > > - Bridge 1 > > > > - Bridge 2 pre_enable_prev_first > > > > - Bridge 3 > > > > - Bridge 4 pre_enable_prev_first > > > > - Bridge 5 pre_enable_prev_first > > > > - Bridge 6 > > > > - Encoder > > > > > > > > In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first. > > > > > > > > The logic looks for a bridge which enabled pre_enable_prev_first flag > > > > on each iteration and assigned the previou bridge to limit pointer > > > > if the bridge doesn't enable pre_enable_prev_first flags. > > > > > > > > If control found Bridge 2 is pre_enable_prev_first then the iteration > > > > looks for Bridge 3 and found it is not pre_enable_prev_first and assigns > > > > it's previous Bridge 4 to limit pointer and calls pre_enable of Bridge 3 > > > > and Bridge 2 and assign iter pointer with limit which is Bridge 4. > > > > > > > > Here is the actual problem, for the next iteration control look for > > > > Bridge 5 instead of Bridge 4 has iter pointer in previous iteration > > > > moved to Bridge 4 so this iteration skips the Bridge 4. The iteration > > > > found Bridge 6 doesn't pre_enable_prev_first flags so the limit assigned > > > > to Encoder. From next iteration Encoder skips as it is the last bridge > > > > for reverse order pipeline. > > > > > > > > So, the resulting pre_enable bridge order would be, > > > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5. > > > > > > > > This patch fixes this by assigning limit to next pointer instead of > > > > previous bridge since the iteration always looks for bridge that does > > > > NOT request prev so assigning next makes sure the last bridge on a > > > > given iteration what exactly the limit bridge is. > > > > > > > > So, the resulting pre_enable bridge order with fix would be, > > > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4, > > > > Encoder. > > > > > > > > [post_disable] > > > > > > > > The altered bridge ordering has failed if two consecutive bridges on a > > > > given pipeline enables the pre_enable_prev_first flag. > > > > > > > > Example: > > > > - Panel > > > > - Bridge 1 > > > > - Bridge 2 pre_enable_prev_first > > > > - Bridge 3 > > > > - Bridge 4 pre_enable_prev_first > > > > - Bridge 5 pre_enable_prev_first > > > > - Bridge 6 > > > > - Encoder > > > > > > > > In this example Bridge 5 and Bridge 4 have pre_enable_prev_first. > > > > > > > > The logic looks for a bridge which enabled pre_enable_prev_first flags > > > > on each iteration and assigned the previou bridge to next and next to > > > > limit pointer if the bridge does enable pre_enable_prev_first flag. > > > > > > > > If control starts from Bridge 6 then it found next Bridge 5 is > > > > pre_enable_prev_first and immediately the next assigned to previous > > > > Bridge 6 and limit assignments to next Bridge 6 and call post_enable > > > > of Bridge 6 even though the next consecutive Bridge 5 is enabled with > > > > pre_enable_prev_first. This clearly misses the logic to find the state > > > > of next conducive bridge as everytime the next and limit assigns > > > > previous bridge if given bridge enabled pre_enable_prev_first. > > > > > > > > So, the resulting post_disable bridge order would be, > > > > - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1, > > > > Panel. > > > > > > > > This patch fixes this by assigning next with previou bridge only if the > > > > bridge doesn't enable pre_enable_prev_first flag and the next further > > > > assign it to limit. This way we can find the bridge that NOT requested > > > > prev to disable last. > > > > > > > > So, the resulting pre_enable bridge order with fix would be, > > > > - Encoder, Bridge 4, Bridge 5, Bridge 6, Bridge 2, Bridge 3, Bridge 1, > > > > Panel. > > > > > > > > Vali
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
On Tue, Aug 1, 2023 at 11:50 AM Dave Stevenson
wrote:
>
> Hi Jagan
>
> My apologies for dropping the ball on this one, and thanks to Frieder
> for the nudge.
>
> On Wed, 12 Apr 2023 at 07:25, Jagan Teki wrote:
> >
> > Hi Dave,
> >
> > Added Maxime, Laurent [which I thought I added before]
> >
> > On Tue, Mar 28, 2023 at 10:38 PM Jagan Teki
> > wrote:
> > >
> > > For a given bridge pipeline if any bridge sets pre_enable_prev_first
> > > flag then the pre_enable for the previous bridge will be called before
> > > pre_enable of this bridge and opposite is done for post_disable.
> > >
> > > These are the potential bridge flags to alter bridge init order in order
> > > to satisfy the MIPI DSI host and downstream panel or bridge to function.
> > > However the existing pre_enable_prev_first logic with associated bridge
> > > ordering has broken for both pre_enable and post_disable calls.
> > >
> > > [pre_enable]
> > >
> > > The altered bridge ordering has failed if two consecutive bridges on a
> > > given pipeline enables the pre_enable_prev_first flag.
> > >
> > > Example:
> > > - Panel
> > > - Bridge 1
> > > - Bridge 2 pre_enable_prev_first
> > > - Bridge 3
> > > - Bridge 4 pre_enable_prev_first
> > > - Bridge 5 pre_enable_prev_first
> > > - Bridge 6
> > > - Encoder
> > >
> > > In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first.
> > >
> > > The logic looks for a bridge which enabled pre_enable_prev_first flag
> > > on each iteration and assigned the previou bridge to limit pointer
> > > if the bridge doesn't enable pre_enable_prev_first flags.
> > >
> > > If control found Bridge 2 is pre_enable_prev_first then the iteration
> > > looks for Bridge 3 and found it is not pre_enable_prev_first and assigns
> > > it's previous Bridge 4 to limit pointer and calls pre_enable of Bridge 3
> > > and Bridge 2 and assign iter pointer with limit which is Bridge 4.
> > >
> > > Here is the actual problem, for the next iteration control look for
> > > Bridge 5 instead of Bridge 4 has iter pointer in previous iteration
> > > moved to Bridge 4 so this iteration skips the Bridge 4. The iteration
> > > found Bridge 6 doesn't pre_enable_prev_first flags so the limit assigned
> > > to Encoder. From next iteration Encoder skips as it is the last bridge
> > > for reverse order pipeline.
> > >
> > > So, the resulting pre_enable bridge order would be,
> > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5.
> > >
> > > This patch fixes this by assigning limit to next pointer instead of
> > > previous bridge since the iteration always looks for bridge that does
> > > NOT request prev so assigning next makes sure the last bridge on a
> > > given iteration what exactly the limit bridge is.
> > >
> > > So, the resulting pre_enable bridge order with fix would be,
> > > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4,
> > > Encoder.
> > >
> > > [post_disable]
> > >
> > > The altered bridge ordering has failed if two consecutive bridges on a
> > > given pipeline enables the pre_enable_prev_first flag.
> > >
> > > Example:
> > > - Panel
> > > - Bridge 1
> > > - Bridge 2 pre_enable_prev_first
> > > - Bridge 3
> > > - Bridge 4 pre_enable_prev_first
> > > - Bridge 5 pre_enable_prev_first
> > > - Bridge 6
> > > - Encoder
> > >
> > > In this example Bridge 5 and Bridge 4 have pre_enable_prev_first.
> > >
> > > The logic looks for a bridge which enabled pre_enable_prev_first flags
> > > on each iteration and assigned the previou bridge to next and next to
> > > limit pointer if the bridge does enable pre_enable_prev_first flag.
> > >
> > > If control starts from Bridge 6 then it found next Bridge 5 is
> > > pre_enable_prev_first and immediately the next assigned to previous
> > > Bridge 6 and limit assignments to next Bridge 6 and call post_enable
> > > of Bridge 6 even though the next consecutive Bridge 5 is enabled with
> > > pre_enable_prev_first. This clearly misses the logic to find the state
> > > of next conducive bridge as everytime the next and limit assigns
> > > previous bridge if given bridge enabled pre_enable_prev_first.
> > >
> > > So, the resulting post_disable bridge order would be,
> > > - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1,
> > > Panel.
> > >
> > > This patch fixes this by assigning next with previou bridge only if the
> > > bridge doesn't enable pre_enable_prev_first flag and the next further
> > > assign it to limit. This way we can find the bridge that NOT requested
> > > prev to disable last.
> > >
> > > So, the resulting pre_enable bridge order with fix would be,
> > > - Encoder, Bridge 4, Bridge 5, Bridge 6, Bridge 2, Bridge 3, Bridge 1,
> > > Panel.
> > >
> > > Validated the bridge init ordering by incorporating dummy bridges in
> > > the sun6i-mipi-dsi pipeline
> > >
> > > Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to
> > > alter bridge init order")
> > > Signed-off-by: Jagan Teki
>
> Thanks for investigating and sor
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
Hi Jagan
My apologies for dropping the ball on this one, and thanks to Frieder
for the nudge.
On Wed, 12 Apr 2023 at 07:25, Jagan Teki wrote:
>
> Hi Dave,
>
> Added Maxime, Laurent [which I thought I added before]
>
> On Tue, Mar 28, 2023 at 10:38 PM Jagan Teki
> wrote:
> >
> > For a given bridge pipeline if any bridge sets pre_enable_prev_first
> > flag then the pre_enable for the previous bridge will be called before
> > pre_enable of this bridge and opposite is done for post_disable.
> >
> > These are the potential bridge flags to alter bridge init order in order
> > to satisfy the MIPI DSI host and downstream panel or bridge to function.
> > However the existing pre_enable_prev_first logic with associated bridge
> > ordering has broken for both pre_enable and post_disable calls.
> >
> > [pre_enable]
> >
> > The altered bridge ordering has failed if two consecutive bridges on a
> > given pipeline enables the pre_enable_prev_first flag.
> >
> > Example:
> > - Panel
> > - Bridge 1
> > - Bridge 2 pre_enable_prev_first
> > - Bridge 3
> > - Bridge 4 pre_enable_prev_first
> > - Bridge 5 pre_enable_prev_first
> > - Bridge 6
> > - Encoder
> >
> > In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first.
> >
> > The logic looks for a bridge which enabled pre_enable_prev_first flag
> > on each iteration and assigned the previou bridge to limit pointer
> > if the bridge doesn't enable pre_enable_prev_first flags.
> >
> > If control found Bridge 2 is pre_enable_prev_first then the iteration
> > looks for Bridge 3 and found it is not pre_enable_prev_first and assigns
> > it's previous Bridge 4 to limit pointer and calls pre_enable of Bridge 3
> > and Bridge 2 and assign iter pointer with limit which is Bridge 4.
> >
> > Here is the actual problem, for the next iteration control look for
> > Bridge 5 instead of Bridge 4 has iter pointer in previous iteration
> > moved to Bridge 4 so this iteration skips the Bridge 4. The iteration
> > found Bridge 6 doesn't pre_enable_prev_first flags so the limit assigned
> > to Encoder. From next iteration Encoder skips as it is the last bridge
> > for reverse order pipeline.
> >
> > So, the resulting pre_enable bridge order would be,
> > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5.
> >
> > This patch fixes this by assigning limit to next pointer instead of
> > previous bridge since the iteration always looks for bridge that does
> > NOT request prev so assigning next makes sure the last bridge on a
> > given iteration what exactly the limit bridge is.
> >
> > So, the resulting pre_enable bridge order with fix would be,
> > - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4,
> > Encoder.
> >
> > [post_disable]
> >
> > The altered bridge ordering has failed if two consecutive bridges on a
> > given pipeline enables the pre_enable_prev_first flag.
> >
> > Example:
> > - Panel
> > - Bridge 1
> > - Bridge 2 pre_enable_prev_first
> > - Bridge 3
> > - Bridge 4 pre_enable_prev_first
> > - Bridge 5 pre_enable_prev_first
> > - Bridge 6
> > - Encoder
> >
> > In this example Bridge 5 and Bridge 4 have pre_enable_prev_first.
> >
> > The logic looks for a bridge which enabled pre_enable_prev_first flags
> > on each iteration and assigned the previou bridge to next and next to
> > limit pointer if the bridge does enable pre_enable_prev_first flag.
> >
> > If control starts from Bridge 6 then it found next Bridge 5 is
> > pre_enable_prev_first and immediately the next assigned to previous
> > Bridge 6 and limit assignments to next Bridge 6 and call post_enable
> > of Bridge 6 even though the next consecutive Bridge 5 is enabled with
> > pre_enable_prev_first. This clearly misses the logic to find the state
> > of next conducive bridge as everytime the next and limit assigns
> > previous bridge if given bridge enabled pre_enable_prev_first.
> >
> > So, the resulting post_disable bridge order would be,
> > - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1,
> > Panel.
> >
> > This patch fixes this by assigning next with previou bridge only if the
> > bridge doesn't enable pre_enable_prev_first flag and the next further
> > assign it to limit. This way we can find the bridge that NOT requested
> > prev to disable last.
> >
> > So, the resulting pre_enable bridge order with fix would be,
> > - Encoder, Bridge 4, Bridge 5, Bridge 6, Bridge 2, Bridge 3, Bridge 1,
> > Panel.
> >
> > Validated the bridge init ordering by incorporating dummy bridges in
> > the sun6i-mipi-dsi pipeline
> >
> > Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to
> > alter bridge init order")
> > Signed-off-by: Jagan Teki
Thanks for investigating and sorting this.
Reviewed-by: Dave Stevenson
> > ---
> > Changes for v2:
> > - add missing dri-devel in CC
>
> Would you please look into this issue?
>
> Thanks,
> Jagan.
Re: [PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
Hi Dave,
Added Maxime, Laurent [which I thought I added before]
On Tue, Mar 28, 2023 at 10:38 PM Jagan Teki wrote:
>
> For a given bridge pipeline if any bridge sets pre_enable_prev_first
> flag then the pre_enable for the previous bridge will be called before
> pre_enable of this bridge and opposite is done for post_disable.
>
> These are the potential bridge flags to alter bridge init order in order
> to satisfy the MIPI DSI host and downstream panel or bridge to function.
> However the existing pre_enable_prev_first logic with associated bridge
> ordering has broken for both pre_enable and post_disable calls.
>
> [pre_enable]
>
> The altered bridge ordering has failed if two consecutive bridges on a
> given pipeline enables the pre_enable_prev_first flag.
>
> Example:
> - Panel
> - Bridge 1
> - Bridge 2 pre_enable_prev_first
> - Bridge 3
> - Bridge 4 pre_enable_prev_first
> - Bridge 5 pre_enable_prev_first
> - Bridge 6
> - Encoder
>
> In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first.
>
> The logic looks for a bridge which enabled pre_enable_prev_first flag
> on each iteration and assigned the previou bridge to limit pointer
> if the bridge doesn't enable pre_enable_prev_first flags.
>
> If control found Bridge 2 is pre_enable_prev_first then the iteration
> looks for Bridge 3 and found it is not pre_enable_prev_first and assigns
> it's previous Bridge 4 to limit pointer and calls pre_enable of Bridge 3
> and Bridge 2 and assign iter pointer with limit which is Bridge 4.
>
> Here is the actual problem, for the next iteration control look for
> Bridge 5 instead of Bridge 4 has iter pointer in previous iteration
> moved to Bridge 4 so this iteration skips the Bridge 4. The iteration
> found Bridge 6 doesn't pre_enable_prev_first flags so the limit assigned
> to Encoder. From next iteration Encoder skips as it is the last bridge
> for reverse order pipeline.
>
> So, the resulting pre_enable bridge order would be,
> - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5.
>
> This patch fixes this by assigning limit to next pointer instead of
> previous bridge since the iteration always looks for bridge that does
> NOT request prev so assigning next makes sure the last bridge on a
> given iteration what exactly the limit bridge is.
>
> So, the resulting pre_enable bridge order with fix would be,
> - Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4,
> Encoder.
>
> [post_disable]
>
> The altered bridge ordering has failed if two consecutive bridges on a
> given pipeline enables the pre_enable_prev_first flag.
>
> Example:
> - Panel
> - Bridge 1
> - Bridge 2 pre_enable_prev_first
> - Bridge 3
> - Bridge 4 pre_enable_prev_first
> - Bridge 5 pre_enable_prev_first
> - Bridge 6
> - Encoder
>
> In this example Bridge 5 and Bridge 4 have pre_enable_prev_first.
>
> The logic looks for a bridge which enabled pre_enable_prev_first flags
> on each iteration and assigned the previou bridge to next and next to
> limit pointer if the bridge does enable pre_enable_prev_first flag.
>
> If control starts from Bridge 6 then it found next Bridge 5 is
> pre_enable_prev_first and immediately the next assigned to previous
> Bridge 6 and limit assignments to next Bridge 6 and call post_enable
> of Bridge 6 even though the next consecutive Bridge 5 is enabled with
> pre_enable_prev_first. This clearly misses the logic to find the state
> of next conducive bridge as everytime the next and limit assigns
> previous bridge if given bridge enabled pre_enable_prev_first.
>
> So, the resulting post_disable bridge order would be,
> - Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1,
> Panel.
>
> This patch fixes this by assigning next with previou bridge only if the
> bridge doesn't enable pre_enable_prev_first flag and the next further
> assign it to limit. This way we can find the bridge that NOT requested
> prev to disable last.
>
> So, the resulting pre_enable bridge order with fix would be,
> - Encoder, Bridge 4, Bridge 5, Bridge 6, Bridge 2, Bridge 3, Bridge 1,
> Panel.
>
> Validated the bridge init ordering by incorporating dummy bridges in
> the sun6i-mipi-dsi pipeline
>
> Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to
> alter bridge init order")
> Signed-off-by: Jagan Teki
> ---
> Changes for v2:
> - add missing dri-devel in CC
Would you please look into this issue?
Thanks,
Jagan.
[PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first
For a given bridge pipeline if any bridge sets pre_enable_prev_first
flag then the pre_enable for the previous bridge will be called before
pre_enable of this bridge and opposite is done for post_disable.
These are the potential bridge flags to alter bridge init order in order
to satisfy the MIPI DSI host and downstream panel or bridge to function.
However the existing pre_enable_prev_first logic with associated bridge
ordering has broken for both pre_enable and post_disable calls.
[pre_enable]
The altered bridge ordering has failed if two consecutive bridges on a
given pipeline enables the pre_enable_prev_first flag.
Example:
- Panel
- Bridge 1
- Bridge 2 pre_enable_prev_first
- Bridge 3
- Bridge 4 pre_enable_prev_first
- Bridge 5 pre_enable_prev_first
- Bridge 6
- Encoder
In this example, Bridge 4 and Bridge 5 have pre_enable_prev_first.
The logic looks for a bridge which enabled pre_enable_prev_first flag
on each iteration and assigned the previou bridge to limit pointer
if the bridge doesn't enable pre_enable_prev_first flags.
If control found Bridge 2 is pre_enable_prev_first then the iteration
looks for Bridge 3 and found it is not pre_enable_prev_first and assigns
it's previous Bridge 4 to limit pointer and calls pre_enable of Bridge 3
and Bridge 2 and assign iter pointer with limit which is Bridge 4.
Here is the actual problem, for the next iteration control look for
Bridge 5 instead of Bridge 4 has iter pointer in previous iteration
moved to Bridge 4 so this iteration skips the Bridge 4. The iteration
found Bridge 6 doesn't pre_enable_prev_first flags so the limit assigned
to Encoder. From next iteration Encoder skips as it is the last bridge
for reverse order pipeline.
So, the resulting pre_enable bridge order would be,
- Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5.
This patch fixes this by assigning limit to next pointer instead of
previous bridge since the iteration always looks for bridge that does
NOT request prev so assigning next makes sure the last bridge on a
given iteration what exactly the limit bridge is.
So, the resulting pre_enable bridge order with fix would be,
- Panel, Bridge 1, Bridge 3, Bridge 2, Bridge 6, Bridge 5, Bridge 4,
Encoder.
[post_disable]
The altered bridge ordering has failed if two consecutive bridges on a
given pipeline enables the pre_enable_prev_first flag.
Example:
- Panel
- Bridge 1
- Bridge 2 pre_enable_prev_first
- Bridge 3
- Bridge 4 pre_enable_prev_first
- Bridge 5 pre_enable_prev_first
- Bridge 6
- Encoder
In this example Bridge 5 and Bridge 4 have pre_enable_prev_first.
The logic looks for a bridge which enabled pre_enable_prev_first flags
on each iteration and assigned the previou bridge to next and next to
limit pointer if the bridge does enable pre_enable_prev_first flag.
If control starts from Bridge 6 then it found next Bridge 5 is
pre_enable_prev_first and immediately the next assigned to previous
Bridge 6 and limit assignments to next Bridge 6 and call post_enable
of Bridge 6 even though the next consecutive Bridge 5 is enabled with
pre_enable_prev_first. This clearly misses the logic to find the state
of next conducive bridge as everytime the next and limit assigns
previous bridge if given bridge enabled pre_enable_prev_first.
So, the resulting post_disable bridge order would be,
- Encoder, Bridge 6, Bridge 5, Bridge 4, Bridge 3, Bridge 2, Bridge 1,
Panel.
This patch fixes this by assigning next with previou bridge only if the
bridge doesn't enable pre_enable_prev_first flag and the next further
assign it to limit. This way we can find the bridge that NOT requested
prev to disable last.
So, the resulting pre_enable bridge order with fix would be,
- Encoder, Bridge 4, Bridge 5, Bridge 6, Bridge 2, Bridge 3, Bridge 1,
Panel.
Validated the bridge init ordering by incorporating dummy bridges in
the sun6i-mipi-dsi pipeline
Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to
alter bridge init order")
Signed-off-by: Jagan Teki
---
Changes for v2:
- add missing dri-devel in CC
drivers/gpu/drm/drm_bridge.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index c3d69af02e79..052a8e6c9961 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -684,11 +684,17 @@ void drm_atomic_bridge_chain_post_disable(struct
drm_bridge *bridge,
*/
list_for_each_entry_from(next,
&encoder->bridge_chain,
chain_node) {
- if (next->pre_enable_prev_first) {
+ if (!next->pre_enable_prev_first) {
next = list_prev_entry(next,
chain_node);
limit = next;
break;
