Re: [PATCH 1/3] drm/ssd130x: Fix reset timing for ssd132x

[John Keeping <jkeeping@inmusicbrands.com>]

Hi Javier,

Thanks for the review.

On Tue, Jan 14, 2025 at 11:21:25PM +0100, Javier Martinez Canillas wrote:
> John Keeping <jkeeping@inmusicbrands.com> writes:
> Thanks for your patches!
> 
> > The ssd132x family of chips require the result pulse to be at least
> > 100us in length.  Increase the reset time to meet this requirement.
> >
> 
> That's not what the datasheet says AFAIU. It says the following in the
> "8.9 Power ON and OFF sequence" section.
> 
> Power ON sequence:
> 
> 1. Power ON VDD.
> 2. After VDD become stable, set RES# pin LOW (logic LOW) for at least
>    3us (t1) and then HIGH (logic HIGH).
> 3. After set RES# pin LOW (logic LOW), wait for at least 3us (t2).
>    Then Power ON VCC.
> 4. After VCC become stable, send command AFh for display ON. SEG/COM
>    will be ON after 100ms (tAF).

The version of the datasheet I have for SD1322 says:

Power ON sequence:

1. Power ON VCI, VDDIO.
2. After VCI, V DDIO become stable, set wait time at least 1ms (t 0) for
   internal V DD become stable. Then set RES# pin LOW (logic low) for at
   least 100us (t1) (4) and then HIGH (logic high).
3. After set RES# pin LOW (logic low), wait for at least 100us (t2).
   Then Power ON V CC.(1)
4. After VCC become stable, send command AFh for display ON. SEG/COM
   will be ON after 200ms (t AF).

And on the hardware I have 4us seems to be too short.

However, having tested it again today it seems to be fine with the 4us
delay so I suspect this was a misleading change in the midst of other
debugging.

I will drop this patch from v2.

> > Signed-off-by: John Keeping <jkeeping@inmusicbrands.com>
> > ---
> >  drivers/gpu/drm/solomon/ssd130x.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
> > index b777690fd6607..2622172228361 100644
> > --- a/drivers/gpu/drm/solomon/ssd130x.c
> > +++ b/drivers/gpu/drm/solomon/ssd130x.c
> > @@ -363,7 +363,7 @@ static void ssd130x_reset(struct ssd130x_device *ssd130x)
> >  
> >  	/* Reset the screen */
> >  	gpiod_set_value_cansleep(ssd130x->reset, 1);
> > -	udelay(4);
> > +	usleep_range(100, 1000);
> >  	gpiod_set_value_cansleep(ssd130x->reset, 0);
> >  	udelay(4);
> 
> That's why I think that the udelay(4) are correct here, since that will
> make for the delay to be bigger than 3 usecs.
> 
> Now, is true that the mentioned 100ms (tAF) after sending an AFh command
> might not happen. Since I see there's no delay after sending a display ON
> command in ssd130x_encoder_atomic_enable():

I don't think this matters.  It is a delay before the user sees the
image, but that is not relevant to the timing of any commands


Regards,
John