Sorry I meant "sleep out" not "sleep in" obviously On Mon, Mar 21, 2022 at 3:39 PM Christophe Branchereau <cbranchereau@xxxxxxxxx> 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@xxxxxxxxxxxxxxx> wrote: > > > > Hi Christophe, > > > > Le lun., mars 21 2022 at 14:36:51 +0100, Christophe Branchereau > > <cbranchereau@xxxxxxxxx> 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@xxxxxxxxx> > > > > 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 > > > > > > >