Hi Dave, On 02.12.22 15:28, Dave Stevenson 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_prev_first that any bridge can set > to swap the order of pre_enable (and post_disable) for that and the > immediately previous bridge. > Should the immediately previous bridge also set the > pre_enable_prev_first flag, the previous bridge to that will be called > before either of those which requested pre_enable_prev_first. > > eg: > - 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 > 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@xxxxxxxxxxxxxxx> I tested this patchset together with the pending Samsung DSIM bridge conversion for i.MX8MM [1] and additional patches to fix the enable/disable sequence for my setup [2] on a Kontron DL i.MX8MM board featuring a TI SN65DSI84 and a 7" LVDS panel. Using this the initialization sequence of the TI SN65DSI84 specified in the datasheet can be achieved. The correct order was verified by tracing the function calls and capturing the DSI data signal and the SN65DSI84 enable GPIO (EN) on a scope. I've also gone through the code of this patch and it all makes sense to me. There is only one small nitpick (see below). Tested-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx> Reviewed-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx> Thanks Frieder [1] https://patchwork.kernel.org/project/dri-devel/cover/20221110183853.3678209-1-jagan@xxxxxxxxxxxxxxxxxxxx/ [2] https://git.kontron-electronics.de/sw/misc/linux/-/commits/v6.1-dsim-mx8mm > --- > drivers/gpu/drm/drm_bridge.c | 144 +++++++++++++++++++++++++++++------ > include/drm/drm_bridge.h | 8 ++ > 2 files changed, 128 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index bb7fc09267af..41051869d6bf 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -581,6 +581,25 @@ void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge, [...]> > - bridge->funcs->atomic_post_disable(bridge, > - old_bridge_state); > - } else if (bridge->funcs->post_disable) { > - bridge->funcs->post_disable(bridge); > - } > + if (limit) > + bridge = limit; Nit: Maybe add a comment for the code above to explain that this is for skipping the already post_disabled bridge. Just like for pre_enable below? > } > } > EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable); > [...]
