Sorry I meant "sleep out" not "sleep in" obviously
On Mon, Mar 21, 2022 at 3:39 PM Christophe Branchereau
<[email protected]> 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 <[email protected]> wrote:
> >
> > Hi Christophe,
> >
> > Le lun., mars 21 2022 at 14:36:51 +0100, Christophe Branchereau
> > <[email protected]> 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 <[email protected]>
> >
> > 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
> > >
> >
> >
Hi Christophe,
On Mon, Mar 21, 2022 at 03:42:08PM +0100, Christophe Branchereau wrote:
> Sorry I meant "sleep out" not "sleep in" obviously
>
> On Mon, Mar 21, 2022 at 3:39 PM Christophe Branchereau
> <[email protected]> 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 <[email protected]> wrote:
> > >
> > > Hi Christophe,
> > >
> > > Le lun., mars 21 2022 at 14:36:51 +0100, Christophe Branchereau
> > > <[email protected]> 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 <[email protected]>
> > >
> > > Didn't Sam acked it?
No, that was the new driver, already replied.
For these changes - with the updated changelog:
Acked-by: Sam Ravnborg <[email protected]>