lkml.org 
[lkml]   [2021]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] drm/panel: ilitek-ili9881c: Avoid unbalance prepare/unprepare
Hi Dave

On Thu, Dec 9, 2021 at 6:58 PM Dave Stevenson
<dave.stevenson@raspberrypi.com> wrote:
>
> Hi Michael
>
> On Thu, 9 Dec 2021 at 16:58, Michael Nazzareno Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > Hi all
> >
> > On Sat, Oct 16, 2021 at 4:58 PM Michael Trimarchi
> > <michael@amarulasolutions.com> wrote:
> > >
> > > All the panel driver check the fact that their prepare/unprepare
> > > call was already called. It's not an ideal solution but fix
> > > for now the problem on ili9881c
> > >
> > > [ 9862.283296] ------------[ cut here ]------------
> > > [ 9862.288490] unbalanced disables for vcc3v3_lcd
> > > [ 9862.293555] WARNING: CPU: 0 PID: 1 at drivers/regulator/core.c:2851
> > > _regulator_disable+0xd4/0x190
> > >
> > > from:
> > >
> > > [ 9862.038619] drm_panel_unprepare+0x2c/0x4c
> > > [ 9862.043212] panel_bridge_post_disable+0x18/0x24
> > > [ 9862.048390] dw_mipi_dsi_bridge_post_disable+0x3c/0xf0
> > > [ 9862.054153] drm_atomic_bridge_chain_post_disable+0x8c/0xd0
> > >
> > > and:
> > >
> > > [ 9862.183103] drm_panel_unprepare+0x2c/0x4c
> > > [ 9862.187695] panel_bridge_post_disable+0x18/0x24
> > > [ 9862.192872] drm_atomic_bridge_chain_post_disable+0x8c/0xd0
> > > [ 9862.199117] disable_outputs+0x120/0x31c
>
> This is down to the dw-mipi-dsi driver calling the post_disable hook
> explicitly at [1], but then also allowing the framework to call it.
> The explicit call is down to limitations in the DSI support, so we
> can't control the DSI host state to a fine enough degree (an ongoing
> discussion [2] [3]). There shouldn't be a need to handle mismatched
> calling in individual panel drivers.
>
> Dave
>
> [1] https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L894
> [2] https://lists.freedesktop.org/archives/dri-devel/2021-November/332060.html
> [3] https://lists.freedesktop.org/archives/dri-devel/2021-December/334007.html

I'm in the second case. I need to enable HS mode after the panel is
initialized. Time to time I have timeout
on dsi command or I have wrong panel initialization. So I explicit call from
the bridge but I understand that is not correct in the design point of view.

So this patch can not be queued because it's a known problem that
people are discussing

Michael

>
>
> > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > ---
> > > drivers/gpu/drm/panel/panel-ilitek-ili9881c.c | 14 ++++++++++++++
> > > 1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> > > index 103a16018975..f75eecb0e65c 100644
> > > --- a/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> > > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9881c.c
> > > @@ -52,6 +52,8 @@ struct ili9881c {
> > >
> > > struct regulator *power;
> > > struct gpio_desc *reset;
> > > +
> > > + bool prepared;
> > > };
> > >
> >
> > I found that this can be a general problem. Should not mandatory to
> > track panel status
> >
> > DRM_PANEL_PREPARED
> > DRM_PANEL_ENABLED
> >
> > Michael
> > > #define ILI9881C_SWITCH_PAGE_INSTR(_page) \
> > > @@ -707,6 +709,10 @@ static int ili9881c_prepare(struct drm_panel *panel)
> > > unsigned int i;
> > > int ret;
> > >
> > > + /* Preparing when already prepared is a no-op */
> > > + if (ctx->prepared)
> > > + return 0;
> > > +
> > > /* Power the panel */
> > > ret = regulator_enable(ctx->power);
> > > if (ret)
> > > @@ -745,6 +751,8 @@ static int ili9881c_prepare(struct drm_panel *panel)
> > > if (ret)
> > > return ret;
> > >
> > > + ctx->prepared = true;
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -770,10 +778,16 @@ static int ili9881c_unprepare(struct drm_panel *panel)
> > > {
> > > struct ili9881c *ctx = panel_to_ili9881c(panel);
> > >
> > > + /* Unpreparing when already unprepared is a no-op */
> > > + if (!ctx->prepared)
> > > + return 0;
> > > +
> > > mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
> > > regulator_disable(ctx->power);
> > > gpiod_set_value(ctx->reset, 1);
> > >
> > > + ctx->prepared = false;
> > > +
> > > return 0;
> > > }
> > >
> > > --
> > > 2.25.1
> > >
> >
> >
> > --
> > Michael Nazzareno Trimarchi
> > Co-Founder & Chief Executive Officer
> > M. +39 347 913 2170
> > michael@amarulasolutions.com
> > __________________________________
> >
> > Amarula Solutions BV
> > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > T. +31 (0)85 111 9172
> > info@amarulasolutions.com
> > www.amarulasolutions.com



--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com

\
 
 \ /
  Last update: 2021-12-09 19:10    [W:0.096 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site