John Keeping <jkeeping@xxxxxxxxxxxxxxxxx> writes: > 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) Oh, that's interesting... I was looking at the datasheet for SSD1327 (the only SSD132x OLED I have). Maybe we could parameterize the delay values and be new members to the struct ssd130x_deviceinfo ? > 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. > Yeah, it says 100us in your datasheet while it says 3us in the one I've for the SSD1327. > 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. > Got it. > I will drop this patch from v2. > Ok. >> > 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 > Oh right, it's only about the time that the SEG/COM are ON indeed. > > Regards, > John > -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
