State of affairs with Ilitek 9341 support

[Maxime Ripard]

On Mon, Dec 11, 2023 at 03:19:08PM +0200, Andy Shevchenko wrote:
> On Fri, Dec 08, 2023 at 09:18:20PM +0100, Noralf Trønnes wrote:
> > On 12/8/23 17:00, Andy Shevchenko wrote:
> > > Included authors and latest (non-white-space) contributors to the drivers
> > > in question along with relevant mailing list and respective (active in the
> > > area) maintainers.
> > > 
> > > I already had risen the question in times when 4th (sic!) driver for the same
> > > hardware was about to be pulled into upstream that we have to somehow reduce
> > > the code base and unify device properties.
> > > 
> > > So, the main question here "What is the plan and where are we now?"
> > > 
> > > I admit that fbtft case is special as it supports, in particular, platform
> > > device (parallel interface) and also well established in the embedded world.
> > > What about the rest?
> > > 
> > > N.B. Besides the fact that panel drivers are too OF-centric, which is bad
> > > practice for the new kernel code in general and has to be stopped. I.o.w.
> > > seeing of_property_*() or alike in the driver after ca. 2020 should be
> > > immediate NAK unless it's very well justified why it may not be used on
> > > non-OF systems.
> 
> Noralf, thanks for your response, my comments below.
> 
> TBH I would also like to hear from maintainers, because it seems they got
> an additional burden for no benefit.
> 
> > Last year drivers/gpu/drm/tiny/panel-mipi-dbi.c was added to support all
> > MIPI DBI compatible (ili9341) SPI displays.
> > It loads the initialisation commands from a firmware file. For more info
> > see https://github.com/notro/panel-mipi-dbi/wiki.
> > 
> > When I started on fbtft in 2013 I didn't know about MIPI DBI so I made
> > some common bus access functions and one driver per controller and that
> > driver had an initialisation sequence to match the panel I had. Then I
> > discovered that displays using the same controller could have different
> > init sequences so I added a Device Tree <init> property that could
> > override the driver init.
> > 
> > In 2015 fbtft was added to drivers/staging, but later that year fbdev
> > was closed for new drivers so it was a dead end.
> > 
> > I started to work on porting fbtft to DRM and almost 2 years later
> > support for the MI0283QT panel (ILI9341) was added.
> > I had now learned about MIPI DBI so a library to handle that was added.
> > I had asked on the Device Tree ML about the <init> property and I was
> > told that I couldn't have that which meant that I couldn't get away with
> > having just one driver for the MIPI DBI compatible display panels as I
> > was first hoping for.
> > 
> > I was aware that there was a challenge going from fbtft to DRM because
> > in fbtft there is support for all panel setups using the <init>
> > property, but in DRM every panel needed support in a driver. So I
> > started to look at adding Device Tree properties to describe the setup
> > for one controller. This would make it easy to describe a new panel in
> > Device Tree for a supported controller. Maxime Ripard came up with the
> > idea to have the controller initialisation commands in a firmware file
> > which meant that we could get away with having just one driver for all
> > MIPI DBI SPI panels (which is the vast majority of these SPI pixel
> > upload panels).
> > 
> > This meant that SPI support could be removed from all the MIPI DBI
> > compatible controllers in fbtft
> 
> I believe it can't. Otherwise we _must_ provide the DT (device property)
> parser that uses what is provided for fbtft SPI to be enabled in the other
> driver.

We never committed to supporting that binding in the first place, and
that code is in staging, so I don't see why we must do that.

> > since there's now a solution for them in DRM. The drivers themselves
> > must stay since they also have parallel bus support which is lacking
> > in DRM. My plan was to wait for panel-mipi-dbi to hit an LTS and
> > then either prepare patches to remove MIPI DBI SPI support from
> > fbtft or at least send an email to staging about the new driver.
> 
> > Unfortunately my health problems got worse and many plans went
> > out the window.
> 
> Oh, sad to hear this, hope you will get better sooner than later!
> 
> > ILI9341 DRM drivers
> > 
> > - drivers/gpu/drm/tiny/mi0283qt.c
> >   This was the first driver added for the MI0283QT panel series
> > 
> > - drivers/gpu/drm/tiny/ili9341.c
> >   Later ili9341 based panels was decided to be added to a controller
> >   specific driver.
> 
> Why was it appeared in the first place? :-(

It's probably an oversight. The name was far from obvious to begin with.

> > - drivers/gpu/drm/tiny/panel-mipi-dbi.c
> >   Generic MIPI DBI SPI driver that loads init commands from a firmware
> >   file. It uses of_property_read_string_index() and
> >   of_get_drm_panel_display_mode(). I don't know if it's possible to make
> >   device_property_*() versions of those.
> 
> Everything like this is possible, just somebody needs to fulfill that.
> And as I said, new OF-centric code, has to be NAKed by default.
> 
> > - drivers/gpu/drm/panel/panel-ilitek-ili9341.c
> >   This driver supports the MIPI DPI (RGB) interface on the controller.
> >   Controller init is done over MIPI DBI SPI. The driver does also for
> >   some reason support the same panel as the ili9341.c driver does.
> >   So 2 drivers for the same panel...
> >   Sidenote: It is possible to make a generic panel-mipi-dpi.c driver for
> >   panels that use DPI for pixels and DBI for init loaded from a firmware
> >   file.
> 
> I wonder who has enough experience and time to at least point out or do
> something about this...

I'm not sure it's worth it tbh. Panel drivers are fairly straightforward
and easy to maintain, and getting a driver that can support all the
variations of the power sequences those driver need would make a very
complicated driver.

And MIPI-DPI display are becoming more and more irrelevant. Small,
cheap, display will keep using MIPI-DBI, and middle and high-end screens
are using MIPI-DSI/eDP.

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20231211/cfc77032/attachment.sig>