Re: [PATCH v3 4/7] drm: exynos: dsi: Separate pre_enable, post_disable code

On 12.12.2021 19:14, Jagan Teki wrote:

Existing driver is calling manual bridge pre_enable, enable, disable and post_disable helpers with their enable and disable functions.

Separate the enable code with pre_enable and enable helpers like enable the DSI in pre_enable and set the display in enable.

Separate the disable code with disable and post_disable helpers like disable the DSI in disable and reset the display in post_disable.

This way the bridge functions are compatible with respective downstream bridge and panel_bridge drivers.

Example of enable bridge function calls with panel_bridge is,

[ 2.079030] panel_bridge_pre_enable: start [ 2.079043] panel_bridge_pre_enable: end! [ 2.079045] exynos_dsi_atomic_pre_enable: start [ 2.079723] exynos_dsi_atomic_pre_enable: end! [ 2.079728] exynos_dsi_atomic_enable: start [ 2.102500] exynos_dsi_atomic_enable: end [ 2.146505] panel_bridge_enable: start [ 2.148547] panel_bridge_enable: enable Signed-off-by: Jagan Teki jagan@amarulasolutions.com

---

Changes for v3:

• new patch

  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c index 1450187c1edc..07083a545948 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c @@ -1377,10 +1377,9 @@ static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi) } }

-static void exynos_dsi_enable(struct drm_bridge *bridge) +static void exynos_dsi_pre_enable(struct drm_bridge *bridge) { struct exynos_dsi *dsi = bridge_to_dsi(bridge);

• const struct drm_bridge_funcs *funcs = dsi->out_bridge->funcs; int ret;

  if (dsi->state & DSIM_STATE_ENABLED)

@@ -1393,38 +1392,36 @@ static void exynos_dsi_enable(struct drm_bridge *bridge) }

dsi->state |= DSIM_STATE_ENABLED; +}

• if (dsi->out_bridge)

• funcs->pre_enable(dsi->out_bridge);
  

+static void exynos_dsi_enable(struct drm_bridge *bridge) +{

• struct exynos_dsi *dsi = bridge_to_dsi(bridge);

  exynos_dsi_set_display_mode(bridge); exynos_dsi_set_display_enable(dsi, true);

• if (dsi->out_bridge)

• funcs->enable(dsi->out_bridge);
  

Ok, apparently I haven't catch that in previous patch you have left out bridge attached to encoder->bridge_chain, before the previous patch out bridge was detached from bridge_chain, which assured exynos_dsi has full control about callbacks.

Does it mean that after prev patch all bridge calls were called twice, I think it is incorrect.

Regards

Andrzej

dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;

• return; }

  static void exynos_dsi_disable(struct drm_bridge *bridge) { struct exynos_dsi *dsi = bridge_to_dsi(bridge);

• const struct drm_bridge_funcs *funcs = dsi->out_bridge->funcs;

  if (!(dsi->state & DSIM_STATE_ENABLED)) return;

  dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;

+}

• if (dsi->out_bridge)

• funcs->disable(dsi->out_bridge);
  

+static void exynos_dsi_post_disable(struct drm_bridge *bridge) +{

• struct exynos_dsi *dsi = bridge_to_dsi(bridge);

  exynos_dsi_set_display_enable(dsi, false);

• if (dsi->out_bridge)

• funcs->post_disable(dsi->out_bridge);
  

• dsi->state &= ~DSIM_STATE_ENABLED; pm_runtime_put_sync(dsi->dev); }

@@ -1438,8 +1435,10 @@ static int exynos_dsi_attach(struct drm_bridge *bridge, }

static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {

• .pre_enable = exynos_dsi_pre_enable, .enable = exynos_dsi_enable, .disable = exynos_dsi_disable,
• .post_disable = exynos_dsi_post_disable, .attach = exynos_dsi_attach, };