notro / panel-mipi-dbi

13 stars 1 forks source link

Drop the need of command file for panels w/o vendor-specific commands #1

Open Icenowy opened 2 years ago

Icenowy commented 2 years ago

Sipeed MaixSense has a LCD panel that does not need any vendor-specific commands to initialize (because all configuration is programmed into OTP).

For this kind of panels, only MIPI DCS commands are needed.

Should we implement the generation of MIPI DCS control stream in panel-mipi-dbi and drop the need of command file?

See https://github.com/sipeed/linux/commit/b0038b7752e827fa76d80530d566d1e2ce21e837 for an example.

Icenowy commented 2 years ago

I am thinking using a special compatible string for these kinds of panels, and inject panel addressing details as DT properties.

notro commented 2 years ago

I knew it was possible for the controller to have the initialisation builtin, but in my 9 years of dealing with these SPI displays I have never actually heard of one, until now.

We can't have a generic compatible for these displays, the DT maintainer was very specific that the compatible should have 2 values, the first should be for a specific display and the second should be panel-mipi-dbi-spi.

I did consider making the firmware file optional, but decided against it since I hadn't seen such a display in the wild and having it mandatory would give the user an error message should it be forgotten. I made the driver for the maker community and the majority of those are not kernel savvy people. You know where to look (or someone tells you where) if the DRM device doesn't show up, it's harder for novices to troubleshoot if the display doesn't work but the DRM device is present. At least this is my thinking, I have seen people struggle getting their displays to work.

In your driver you do set INVERT_MODE and ADDRESS_MODE for the display, so it looks like not all configuration is pre-programmed?

I'm also not sure how PIXEL_FORMAT should have been handled, the DBI spec supports RGB666 so the driver should not block support for that in my opinion.