Re: [PATCH 0/4] drm/tiny: Add driver for Solomon SSD1307 OLED displays

[Javier Martinez Canillas <javierm@redhat.com>]

Hello Geert,

On 2/1/22 15:14, Geert Uytterhoeven wrote:
> Hi Javier,
> 
> On Tue, Feb 1, 2022 at 2:09 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> On 2/1/22 12:38, Geert Uytterhoeven wrote:
>>>> Since the current binding has a compatible "ssd1305fb-i2c", we could make the
>>>> new one "ssd1305drm-i2c" or better, just "ssd1305-i2c".
>>>
>>> DT describes hardware, not software policy.
>>> If the hardware is the same, the DT bindings should stay the same.
>>>
>>
>> Yes I know that but the thing is that the current binding don't describe
>> the hardware correctly. For instance, don't use a backlight DT node as a
>> property of the panel and have this "fb" suffix in the compatible strings.
>>
>> Having said that, my opinion is that we should just keep with the existing
>> bindings and make compatible to that even if isn't completely correct.
>>
>> Since that will ease adoption of the new DRM driver and allow users to use
>> it without the need to update their DTBs.
> 
> To me it looks like the pwms property is not related to the backlight
> at all, and only needed for some variants?
>

I was reading the datasheets of the ssd1305, ssd1306 and ssd1307. Only the
first one mentions anything about a PWM and says:

  In phase 3, the OLED driver switches to use current source to drive the
  OLED pixels and this is the current drive stage. SSD1305 employs PWM
  (Pulse Width Modulation) method to control the brightness of area color
  A, B, C, D color individually. The longer the waveform in current drive
  stage is, the brighter is the pixel and vice versa.

  After finishing phase 3, the driver IC will go back to phase 1 to display
  the next row image data. This threestep cycle is run continuously to refresh
  image display on OLED panel. 

The way I understand this is that the PWM isn't used for the backlight
but instead to power the IC and allow to display the actual pixels ?

And this matches what Maxime mentioned in this patch:

https://linux-arm-kernel.infradead.narkive.com/5i44FnQ8/patch-1-2-video-ssd1307fb-add-support-for-ssd1306-oled-controller

  The Solomon SSD1306 OLED controller is very similar to the SSD1307,
  except for the fact that the power is given through an external PWM for
  the 1307, and while the 1306 can generate its own power without any PWM. 

> And the actual backlight code seems to be about internal contrast
> adjustment?
> 
> So if the pwms usage is OK, what other reasons are there to break
> DT compatibility? IMHO just the "fb" suffix is not a good reason.
>

Absolutely agreed with you on this. It seems we should just use the existing
binding and make the driver compatible with that. The only value is that the
drm_panel infrastructure could be used, but making it backward compatible is
more worthy IMO.

Best regards, -- 
Javier Martinez Canillas
Linux Engineering
Red Hat