linux-kernel - Re: [PATCH] drm/panel: ilitek-ili9881c: Avoid unbalance prepare/unprepare

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPY8ntCvAnu9HS1WxWRkveXnQ_vD8EOdshX-ob8vGuGqOKp+RA@mail.gmail.com>
Date:   Thu, 9 Dec 2021 17:58:10 +0000
From:   Dave Stevenson <dave.stevenson@...pberrypi.com>
To:     Michael Nazzareno Trimarchi <michael@...rulasolutions.com>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Sam Ravnborg <sam@...nborg.org>,
        David Airlie <airlied@...ux.ie>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/panel: ilitek-ili9881c: Avoid unbalance prepare/unprepare

Hi Michael

On Thu, 9 Dec 2021 at 16:58, Michael Nazzareno Trimarchi
<michael@...rulasolutions.com> wrote:
>
> Hi all
>
> On Sat, Oct 16, 2021 at 4:58 PM Michael Trimarchi
> <michael@...rulasolutions.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


> > Signed-off-by: Michael Trimarchi <michael@...rulasolutions.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@...rulasolutions.com
> __________________________________
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info@...rulasolutions.com
> www.amarulasolutions.com