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