diff mbox series

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

Message ID 20211212181416.3312656-5-jagan@amarulasolutions.com
State New
Headers
Series drm: exynos: dsi: Convert drm bridge |

Commit Message

Jagan Teki Dec. 12, 2021, 6:14 p.m. UTC 
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(-)

Comments

Andrzej Hajda Dec. 13, 2021, 9:57 a.m. UTC | #1 
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,
>   };
>
Jagan Teki Dec. 14, 2021, 7:01 p.m. UTC | #2 
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 mbox series

Patch

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,
 };