notro / tinydrm

Discontinued. Out of tree tinydrm modules
https://github.com/notro/tinydrm/wiki
99 stars 27 forks source link

Adding support for ssd1306 i2c display [fbtft v/s tinydrm] #13

Closed v-thakkar closed 3 years ago

v-thakkar commented 6 years ago

Hi Noralf,

I was interested in adding support for ssd1306 I2C display in the mainline. I see that there is already fbtft driver for the same but it seems to be for the SPI only and moreover, after reading mailing list conversations and you wiki notes, I think it is preferable to have tinydrm driver.

Could you please tell me what is the standard procedure to do that? I was mainly confused as I wasn't sure if any such effort has been done previously and/or are there any discussions happened specifically for ssd1306 and related displays? Does submitting tinydrm driver in the mailing list directly works? And how staging drivers are moved? I mean I know that fbdev maintainer don't want to accept more drivers, so once we have full functional tinydrm driver accepted in the mainline, do we end up deleting the driver from fbtft subsystem?

Thanks!

notro commented 6 years ago

There is this fbdev driver: drivers/video/fbdev/ssd1307fb.c Device Tree binding: Documentation/devicetree/bindings/display/ssd1307fb.txt

v-thakkar commented 6 years ago

Yes, I know about it. But I was mostly interested in knowing if it is still preferable or worthy to add tinydrm driver for the same. Mainly because as mentioned by fbdev maintainer, it's a deprecated framework and new drivers should use DRM but I'm not sure if there is an interest in converting existing fbdev drivers to DRM and specifically as tinyDRM is something I'll need to use for SSD1306, I wasn't sure if you would be willing to have these drivers in your subsystem. Thanks.

notro commented 6 years ago

I doubt that a tinydrm version will see much use.

If I had such a display I would have used the fbdev driver if I could. It's been around for a long time and looking at the history, it's still being maintained and extended. Meaning it's very likely that corner cases have been covered and if a problem crops up, it's likely that someone fixes it before it becomes a problem for me.

I don't know what people (companies/products) use such tiny monochrome displays for, but I doubt it is with a modern graphic stacks where DRM is important. Maybe used in some legacy stuff? If you're not going to use the driver yourself, I'm afraid it will be dead weight.

But I could be wrong. You can ask on the fbdev mailinglist and cc some of the people that have contributed to the driver recently in form of fixes that have been detected when actually using the driver (not general fbdev maintanence patches).

v-thakkar commented 6 years ago

Thanks @notro! This was really helpful. I'm going to email today in the fbdev mailing list but I had one last question. Why are we still keeping drivers related to ssd1306 and related displays in the staging subsystem as we already have fbdev driver? Was there any specific reason? I wasn't able to find the specific reason from commit logs so asking. Wouldn't it makes sense to have a single driver? I am asking here so that I know if I also need to include staging subsystem mailing list in the email.

notro commented 6 years ago

The fb_ssd1306 driver in staging only supports the SPI interface whereas ssd1307fb only supports the I2C interface of the controller.

The reason it only supports i2c, is probably because many computer mainboards have i2c interfaces, and SPI is usually only found in the embedded world.

For a tinydrm driver it would make sense to support both interfaces. Before fbdev was closed for new drivers, I did some work to prepare for moving fbtft out of staging. I see that I split the driver in 2 parts: Display and controller: https://github.com/notro/fbdbi/blob/master/adafruit13fb.c https://github.com/notro/fbdbi/blob/master/ssd1306.c

The ssd1306 is so simple that it is possible to cover it with just a controller driver. Other display controller of this type have a lot of registers and need to be split up since it's not allowed to have register setup in the Device Tree. This is what fbtft does...