Re: [PATCH] drm/panel: simple: Support reset GPIOs

[Maxime Ripard]

On Mon, Dec 16, 2019 at 02:10:36PM +0100, Miquel Raynal wrote:
> > >  drivers/gpu/drm/panel/panel-simple.c | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> > > index 5d487686d25c..15dd495c347d 100644
> > > --- a/drivers/gpu/drm/panel/panel-simple.c
> > > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > > @@ -110,6 +110,7 @@ struct panel_simple {
> > >  	struct i2c_adapter *ddc;
> > >
> > >  	struct gpio_desc *enable_gpio;
> > > +	struct gpio_desc *reset_gpio;
> > >
> > >  	struct drm_display_mode override_mode;
> > >  };
> > > @@ -433,12 +434,21 @@ static int panel_simple_probe(struct device *dev, const struct panel_desc *desc)
> > >  	if (IS_ERR(panel->supply))
> > >  		return PTR_ERR(panel->supply);
> > >
> > > +	panel->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> > > +						    GPIOD_OUT_LOW);
> > > +	if (IS_ERR(panel->reset_gpio)) {
> > > +		err = PTR_ERR(panel->reset_gpio);
> > > +		if (err != -EPROBE_DEFER)
> > > +			dev_err(dev, "failed to request reset pin: %d\n", err);
> > > +		return err;
> > > +	}
> > > +
> >
> > However, I'm wondering if it wouldn't be better to just have the
> > device maintained in reset at probe (so OUT_HIGH) and moved out of
> > reset during either the prepare or enable callbacks.
> >
> > This is pretty much what is happening with the enable-gpios already.
> >
> > Also, panels usually need to wait for a minimum time after you
> > deassert the reset line. How is that dealt with?
> >
> > I guess a good way to do that would be to add that duration to the
> > panel description, since this is pretty much device specific.
>
> What about the case were your Bootloader displays something and you
> don't want the panel to blink ?

The Bootloader to Linux transition will make the panel blink already,
since the display engine is going to be reset / reconfigured during
the transition.

The only way to implement this would be to implement properly the
reset callbacks in all you display drivers to recreate the DRM state
from the hardware state, and then you'll be able to just switch to the
new buffer.

Only Intel does this at the time though, and that's way outside of the
scope of this patch...

> Right now I am just forcing the reset to be deasserted.

... Especially since the very next line after your patch forces the
panel to be disabled.

Maxime

Attachment: signature.asc
Description: PGP signature