On Tue, May 16, 2023 at 1:47 PM Marek Vasut <marex@xxxxxxx> wrote: > > On 5/16/23 10:12, Jagan Teki wrote: > > Hi Marek and Neil, > > > > On Sun, May 14, 2023 at 1:40 AM Marek Vasut <marex@xxxxxxx> wrote: > >> > >> This panel_bridge post_disable callback is called from the bridge chain now, > >> so drop the explicit call here. This fixes call imbalance, where this driver > >> does not call ->pre_enable, but does call ->post_disable . In case either of > >> the two callbacks implemented in one of the panel or bridge drivers contains > >> any operation with refcounted object, like regulator, this would make kernel > >> complain about the imbalance. > >> > >> This can be triggered e.g. with ST7701 driver, which operates on regulators > >> in its prepare/unprepare callbacks, which are called from pre_enable/post_disable > >> callbacks respectively. The former is called once, the later twice, during > >> entry to suspend. > >> > >> Drop the post_disable call to fix the imbalance. > >> > >> Signed-off-by: Marek Vasut <marex@xxxxxxx> > >> --- > >> Cc: Andrzej Hajda <andrzej.hajda@xxxxxxxxx> > >> Cc: Antonio Borneo <antonio.borneo@xxxxxxxxxxx> > >> Cc: Daniel Vetter <daniel@xxxxxxxx> > >> Cc: David Airlie <airlied@xxxxxxxxx> > >> Cc: Jernej Skrabec <jernej.skrabec@xxxxxxxxx> > >> Cc: Jonas Karlman <jonas@xxxxxxxxx> > >> Cc: Laurent Pinchart <Laurent.pinchart@xxxxxxxxxxxxxxxx> > >> Cc: Marek Vasut <marex@xxxxxxx> > >> Cc: Neil Armstrong <neil.armstrong@xxxxxxxxxx> > >> Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > >> Cc: Philippe Cornu <philippe.cornu@xxxxxxxxxxx> > >> Cc: Robert Foss <robert.foss@xxxxxxxxxx> > >> Cc: Vincent Abriou <vincent.abriou@xxxxxx> > >> Cc: Yannick Fertre <yannick.fertre@xxxxxxxxxxx> > >> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > >> --- > >> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 9 --------- > >> 1 file changed, 9 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > >> index b2efecf7d1603..63ac972547361 100644 > >> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > >> @@ -859,15 +859,6 @@ static void dw_mipi_dsi_bridge_post_atomic_disable(struct drm_bridge *bridge, > >> */ > >> dw_mipi_dsi_set_mode(dsi, 0); > >> > >> - /* > >> - * TODO Only way found to call panel-bridge post_disable & > >> - * panel unprepare before the dsi "final" disable... > >> - * This needs to be fixed in the drm_bridge framework and the API > >> - * needs to be updated to manage our own call chains... > >> - */ > >> - if (dsi->panel_bridge->funcs->post_disable) > >> - dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge); > >> - > > > > If my understanding was correct, the controller set the low-speed DCS > > in pre_enable and high-speed DCS in enable. So I'm thinking this > > explicit post_disable still needs to revert the operation within the > > bridge chain. I didn't test this but trying to understand how the > > existing behaviour is satisfied if we drop this. > > Did I miss a panel_bridge pre_enable call somewhere in the driver ? > Where is it ? Haa, sorry the next bridge pre_enable. driver setting the command-mode (low-speed) in mode_set so when the next bridge pre_enable is called, low-speed DCS can be sent, then the bridge enable() sets video mode (high-speed). This is where an explicit post_disable would be required, this is what I understood so far. Jagan.