[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAD=FV=UzVGE88k6kmN+BxO_SV4H9JDM=96E1Mco3K2mofRbnGA@mail.gmail.com>
Date: Tue, 30 Sep 2025 07:59:17 -0700
From: Doug Anderson <dianders@...omium.org>
To: Svyatoslav Ryhel <clamor95@...il.com>
Cc: Neil Armstrong <neil.armstrong@...aro.org>, Jessica Zhang <quic_jesszhan@...cinc.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Thierry Reding <thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>,
Sam Ravnborg <sam@...nborg.org>, dri-devel@...ts.freedesktop.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-tegra@...r.kernel.org
Subject: Re: [PATCH v1 2/8] gpu/drm: panel: add support for LG LD070WX3-SL01
MIPI DSI panel
Hi,
On Tue, Sep 30, 2025 at 7:48 AM Svyatoslav Ryhel <clamor95@...il.com> wrote:
>
> вт, 30 вер. 2025 р. о 17:34 Doug Anderson <dianders@...omium.org> пише:
> >
> > Hi,
> >
> > On Mon, Sep 29, 2025 at 10:13 PM Svyatoslav Ryhel <clamor95@...il.com> wrote:
> > >
> > > > > +static int lg_ld070wx3_unprepare(struct drm_panel *panel)
> > > > > +{
> > > > > + struct lg_ld070wx3 *priv = to_lg_ld070wx3(panel);
> > > > > + struct mipi_dsi_multi_context ctx = { .dsi = priv->dsi };
> > > > > +
> > > > > + mipi_dsi_dcs_enter_sleep_mode_multi(&ctx);
> > > > > +
> > > >
> > > > Maybe add some comment about ignoring the accumulated error in the
> > > > context and still doing the sleeps?
> > > >
> > >
> > > Isn't that obvious? Regardless of what command returns power supply
> > > should be turned off, preferably with a set amount of delays (delays
> > > are taken from datasheet) to avoid leaving panel in uncertain state
> > > with power on.
> >
> > I won't insist, though IMO any time an error return is purposely
> > ignored a comment about why can be justified.
> >
> >
> > > > > + msleep(50);
> > > > > +
> > > > > + regulator_bulk_disable(ARRAY_SIZE(priv->supplies), priv->supplies);
> > > > > +
> > > > > + /* power supply must be off for at least 1s after panel disable */
> > > > > + msleep(1000);
> > > >
> > > > Presumably it would be better to keep track of the time you turned it
> > > > off and then make sure you don't turn it on again before that time?
> > > > Otherwise you've got a pretty wasteful delay here.
> > > >
> > >
> > > And how do you propose to implement that? Should I use mutex?
> > > Datasheet is clear regarding this, after supply is turned off there
> > > MUST be AT LEAST 1 second of delay before supplies can be turned back
> > > on.
> >
> > You don't really need a mutex since the DRM core will make sure that
> > prepare and unprepare can't be called at the same time. panel-edp
> > implements this. See `unprepared_time` I believe.
> >
> > NOTE: if you want to get really deep into this, it's actually a bit of
> > a complicated topic and I would also encourage you to add an
>
> Please spare me of this, I have enough stuff to work with and have no
> capacity to delve into depth of drm any deeper. In case this panel had
> a reset I would not care about regulators too much, but it already
> gave me too much pain and caused partially reversible damage to itself
> that I am not willing to risk.
I won't insist. It's not much code, but it could always be done later.
-Doug
Powered by blists - more mailing lists