Hi Javier, Thanks for the review. On Tue, Jan 14, 2025 at 11:21:25PM +0100, Javier Martinez Canillas wrote: > John Keeping <jkeeping@xxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxx> > > --- > > 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
