Message-ID: <87jzawwdbx.fsf@minerva.mail-host-address-is-not-set>
Date: Wed, 15 Jan 2025 12:01:22 +0100
From: Javier Martinez Canillas <javierm@...hat.com>
To: John Keeping <jkeeping@...usicbrands.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
 <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David
 Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] drm/ssd130x: Fix reset timing for ssd132x

John Keeping <jkeeping@...usicbrands.com> writes:

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

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@...usicbrands.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
>

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

Powered by blists - more mailing lists