[PATCH v2 1/2] drm/bridge: Fix improper bridge init order with pre_enable_prev_first

[Frieder Schrempf]

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 <jagan at amarulasolutions.com>

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