Re: [PATCH v5 3/3] drm/panel : innolux-ej030na and abt-y030xx067a : add .enable and .disable

[Christophe Branchereau <cbranchereau@gmail.com>]

Sorry I meant "sleep out" not "sleep in" obviously

On Mon, Mar 21, 2022 at 3:39 PM Christophe Branchereau
<cbranchereau@gmail.com> wrote:
>
> Following the introduction of bridge_atomic_enable in the ingenic
> drm driver, the crtc is enabled between .prepare and .enable, if
> it exists. Add it so the backlight is only enabled after the crtc is, to
> avoid graphical issues.
>
> As we're moving the "sleep in" command out of the init sequence
> into .enable for the ABT, we need to switch the regmap cache
> to REGCACHE_FLAT to be able to use regmap_set_bits, given this
> panel registers are write-ony and read as 0.
>
> On Mon, Mar 21, 2022 at 3:21 PM Paul Cercueil <paul@crapouillou.net> wrote:
> >
> > Hi Christophe,
> >
> > Le lun., mars 21 2022 at 14:36:51 +0100, Christophe Branchereau
> > <cbranchereau@gmail.com> a écrit :
> > > Following the introduction of bridge_atomic_enable in the ingenic
> > > drm driver, the crtc is enabled between .prepare and .enable, if
> > > it exists.
> > >
> > > Add it so the backlight is only enabled after the crtc is, to avoid
> > > graphical issues.
> > >
> > > Signed-off-by: Christophe Branchereau <cbranchereau@gmail.com>
> >
> > Didn't Sam acked it?
> >
> > > ---
> > >  drivers/gpu/drm/panel/panel-abt-y030xx067a.c  | 31
> > > +++++++++++++++++--
> > >  drivers/gpu/drm/panel/panel-innolux-ej030na.c | 31
> > > ++++++++++++++++---
> > >  2 files changed, 55 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/panel/panel-abt-y030xx067a.c
> > > b/drivers/gpu/drm/panel/panel-abt-y030xx067a.c
> > > index f043b484055b..ddfacaeac1d4 100644
> > > --- a/drivers/gpu/drm/panel/panel-abt-y030xx067a.c
> > > +++ b/drivers/gpu/drm/panel/panel-abt-y030xx067a.c
> > > @@ -140,7 +140,7 @@ static const struct reg_sequence
> > > y030xx067a_init_sequence[] = {
> > >       { 0x03, REG03_VPOSITION(0x0a) },
> > >       { 0x04, REG04_HPOSITION1(0xd2) },
> > >       { 0x05, REG05_CLIP | REG05_NVM_VREFRESH | REG05_SLBRCHARGE(0x2) },
> > > -     { 0x06, REG06_XPSAVE | REG06_NT },
> > > +     { 0x06, REG06_NT },
> > >       { 0x07, 0 },
> > >       { 0x08, REG08_PANEL(0x1) | REG08_CLOCK_DIV(0x2) },
> > >       { 0x09, REG09_SUB_BRIGHT_R(0x20) },
> > > @@ -183,8 +183,6 @@ static int y030xx067a_prepare(struct drm_panel
> > > *panel)
> > >               goto err_disable_regulator;
> > >       }
> > >
> > > -     msleep(120);
> > > -
> > >       return 0;
> > >
> > >  err_disable_regulator:
> > > @@ -202,6 +200,30 @@ static int y030xx067a_unprepare(struct drm_panel
> > > *panel)
> > >       return 0;
> > >  }
> > >
> > > +static int y030xx067a_enable(struct drm_panel *panel)
> > > +{
> > > +
> > > +     struct y030xx067a *priv = to_y030xx067a(panel);
> > > +
> > > +     regmap_set_bits(priv->map, 0x06, REG06_XPSAVE);
> > > +
> > > +     if (panel->backlight) {
> > > +             /* Wait for the picture to be ready before enabling backlight */
> > > +             msleep(120);
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int y030xx067a_disable(struct drm_panel *panel)
> > > +{
> > > +     struct y030xx067a *priv = to_y030xx067a(panel);
> > > +
> > > +     regmap_clear_bits(priv->map, 0x06, REG06_XPSAVE);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > >  static int y030xx067a_get_modes(struct drm_panel *panel,
> > >                               struct drm_connector *connector)
> > >  {
> > > @@ -239,6 +261,8 @@ static int y030xx067a_get_modes(struct drm_panel
> > > *panel,
> > >  static const struct drm_panel_funcs y030xx067a_funcs = {
> > >       .prepare        = y030xx067a_prepare,
> > >       .unprepare      = y030xx067a_unprepare,
> > > +     .enable         = y030xx067a_enable,
> > > +     .disable        = y030xx067a_disable,
> > >       .get_modes      = y030xx067a_get_modes,
> > >  };
> > >
> > > @@ -246,6 +270,7 @@ static const struct regmap_config
> > > y030xx067a_regmap_config = {
> > >       .reg_bits = 8,
> > >       .val_bits = 8,
> > >       .max_register = 0x15,
> > > +     .cache_type = REGCACHE_FLAT,
> >
> > I understand that this is added because the panel registers are
> > write-only and read as zero, and it is needed for
> > regmap_{clear,set}_bits to work.
> >
> > This information should definitely be added to the commit message.
> >
> > If you can add it inline here, and I'll update the commit message when
> > applying the patch.
> >
> > Cheers,
> > -Paul
> >
> > >  };
> > >
> > >  static int y030xx067a_probe(struct spi_device *spi)
> > > diff --git a/drivers/gpu/drm/panel/panel-innolux-ej030na.c
> > > b/drivers/gpu/drm/panel/panel-innolux-ej030na.c
> > > index c558de3f99be..6de7370185cd 100644
> > > --- a/drivers/gpu/drm/panel/panel-innolux-ej030na.c
> > > +++ b/drivers/gpu/drm/panel/panel-innolux-ej030na.c
> > > @@ -80,8 +80,6 @@ static const struct reg_sequence
> > > ej030na_init_sequence[] = {
> > >       { 0x47, 0x08 },
> > >       { 0x48, 0x0f },
> > >       { 0x49, 0x0f },
> > > -
> > > -     { 0x2b, 0x01 },
> > >  };
> > >
> > >  static int ej030na_prepare(struct drm_panel *panel)
> > > @@ -109,8 +107,6 @@ static int ej030na_prepare(struct drm_panel
> > > *panel)
> > >               goto err_disable_regulator;
> > >       }
> > >
> > > -     msleep(120);
> > > -
> > >       return 0;
> > >
> > >  err_disable_regulator:
> > > @@ -128,6 +124,31 @@ static int ej030na_unprepare(struct drm_panel
> > > *panel)
> > >       return 0;
> > >  }
> > >
> > > +static int ej030na_enable(struct drm_panel *panel)
> > > +{
> > > +     struct ej030na *priv = to_ej030na(panel);
> > > +
> > > +     /* standby off */
> > > +     regmap_write(priv->map, 0x2b, 0x01);
> > > +
> > > +     if (panel->backlight) {
> > > +             /* Wait for the picture to be ready before enabling backlight */
> > > +             msleep(120);
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int ej030na_disable(struct drm_panel *panel)
> > > +{
> > > +     struct ej030na *priv = to_ej030na(panel);
> > > +
> > > +     /* standby on */
> > > +     regmap_write(priv->map, 0x2b, 0x00);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > >  static int ej030na_get_modes(struct drm_panel *panel,
> > >                            struct drm_connector *connector)
> > >  {
> > > @@ -165,6 +186,8 @@ static int ej030na_get_modes(struct drm_panel
> > > *panel,
> > >  static const struct drm_panel_funcs ej030na_funcs = {
> > >       .prepare        = ej030na_prepare,
> > >       .unprepare      = ej030na_unprepare,
> > > +     .enable         = ej030na_enable,
> > > +     .disable        = ej030na_disable,
> > >       .get_modes      = ej030na_get_modes,
> > >  };
> > >
> > > --
> > > 2.35.1
> > >
> >
> >