| Message ID | 20211212181416.3312656-5-jagan@amarulasolutions.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
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, > }; >
On Mon, Dec 13, 2021 at 3:27 PM Andrzej Hajda <andrzej.hajda@intel.com> wrote: > > > 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. I think squash this to previous patch make sense. let me know if you are fine with it? Thanks, Jagan.
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); - 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, };
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(-)