Hi Javier,
On Tue, Feb 01, 2022 at 04:03:30PM +0100, Javier Martinez Canillas wrote:
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.
Only if the bindings describe the HW in a correct way that is.
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-ssd1...
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.
I took a look at the datasheets - and all ssd1305, ssd1306 and ssd1307 are the same. They have timing constrains on the Vcc. The random schematic I found on the net showed me that a PWM was used to control the Vcc voltage - which again is used to control the brightness.
All the above has nothing to do with backlight - I had this mixed up in my head.
So my current understanding: - solomon,ssd1307fb.yaml should NOT include a backlight node - because the backlight is something included in the ssd130x device and not something separate. - 1305, 1306, and 1307 (I did not check 1309) all requires a Vcc supply that shall be turned on/off according to the datasheet. This implies that we need a regulaator for Vcc - and the regulator could be a pwm based regulator or something else - the HW do not care. - But I can see that several design connect Vcc to a fixed voltage, so I am not too sure about this part.
I think the correct binding would have
ssd1307 => regulator => pwm
So the ssd1307 binding references a regulator, and the regulator may use an pwm or may use something else.
The current binding references a vbat supply - but the datasheet do not mention any vbat. It is most likely modelling the Vdd supply.
Right now my take is to go the simple route: - Keep the binding as is and just use the pwm as already implemented - Likewise keep the backlight as is
Last I recommend to drop the fbdev variant - if the drm driver has any regressions we can fix them. And I do not see any other way to move users over. Unless their setup breaks then they do not change.
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.
Using drm_panel here would IMO just complicate things - it is not that we will see many different panels (I think).
Sam