Re: [RFC PATCH 0/4] DSI/DBI and TinyDRM driver

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Jul 07, 2020 at 04:32:25PM +0200, Noralf Trønnes wrote:
> (cc Dillon)
> 
> Den 03.07.2020 19.26, skrev Sam Ravnborg:
> > Hi Noralf/Paul.
> > 
> > Trying to stir up this discussion again.
> > 
> > On Sun, Jun 14, 2020 at 06:36:22PM +0200, Noralf Trønnes wrote:
> >>
> >>
> >> Den 07.06.2020 15.38, skrev Paul Cercueil:
> >>> Hi,
> >>>
> >>> Here's a follow-up on the previous discussion about the current state of
> >>> DSI/DBI panel drivers, TinyDRM, and the need of a cleanup.
> >>>
> >>> This patchset introduces the following:
> >>> * It slightly tweaks the MIPI DSI code so that it supports MIPI DBI over
> >>>   various buses. This patch has been tested with a non-upstream DRM
> >>>   panel driver for a ILI9331 DBI/8080 panel, written with the DSI
> >>>   framework (and doesn't include <drm/drm_mipi_dbi.h>), and non-upstream
> >>>   DSI/DBI host driver for the Ingenic SoCs.
> >>>
> >>> * A SPI DBI host driver, using the current MIPI DSI framework. It allows
> >>>   MIPI DSI/DBI drivers to be written with the DSI framework, even if
> >>>   they are connected over SPI, instead of registering as SPI device
> >>>   drivers. Since most of these panels can be connected over various
> >>>   buses, it permits to reuse the same driver independently of the bus
> >>>   used.
> >>>
> >>> * A TinyDRM driver for DSI/DBI panels, once again independent of the bus
> >>>   used; the only dependency (currently) being that the panel must
> >>>   understand DCS commands.
> >>>
> >>> * A DRM panel driver to test the stack. This driver controls Ilitek
> >>>   ILI9341 based DBI panels, like the Adafruit YX240QV29-T 320x240 2.4"
> >>>   TFT LCD panel. This panel was converted from
> >>>   drivers/gpu/drm/tiny/ili9341.c.
> >>>
> >>> I would like to emphasize that while it has been compile-tested, I did
> >>> not test it with real hardware since I do not have any DBI panel
> >>> connected over SPI. I did runtime-test the code, just without any panel
> >>> connected.
> >>>
> >>> Another thing to note, is that it does not break Device Tree ABI. The
> >>> display node stays the same:
> >>>
> >>> display@0 {
> >>> 	compatible = "adafruit,yx240qv29", "ilitek,ili9341";
> >>> 	reg = <0>;
> >>> 	spi-max-frequency = <32000000>;
> >>> 	dc-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
> >>> 	reset-gpios = <&gpio0 8 GPIO_ACTIVE_HIGH>;
> >>> 	rotation = <270>;
> >>> 	backlight = <&backlight>;
> >>> };
> >>>
> >>> The reason it works, is that the "adafruit,yx240qv29" device is probed
> >>> on the SPI bus, so it will match with the SPI/DBI host driver. This will
> >>> in turn register the very same node with the DSI bus, and the ILI9341
> >>> DRM panel driver will probe. The driver will detect that no controller
> >>> is linked to the panel, and eventually register the DBI/DSI TinyDRM
> >>> driver.
> >>>
> >>> I can't stress it enough that this is a RFC, so it still has very rough
> >>> edges.
> >>>
> >>
> >> I don't know bridge and dsi drivers so I can't comment on that, but one
> >> thing I didn't like is that the DT compatible string has to be added to
> >> 2 different modules.
> >>
> >> As an example, a MI0283QT panel (ILI9341) supports these interface options:
> >>
> >> 1. SPI
> >>    Panel setup/control and framebuffer upload over SPI
> >>
> >> 2. SPI + DPI
> >>    Panel setup/control over SPI, framebuffer scanout over DPI
> >>
> >> 3. Parallel bus
> >>    Panel setup/control and framebuffer upload over parallel bus
> > 
> > To continue the configurations we should support:
> > - Panels where the chip can be configured to SPI, SPI+DPI, Parallel bus
> >   (as detailed by Noralf above)
> > - Panels that supports only 6800 or 8080 - connected via GPIO pins or
> >   memory mapped (maybe behind some special IP to support this)
> >   Command set is often special.
> > 
> > We will see a number of chips with many different types of displays.
> > So the drivers should be chip specific with configuration depending on
> > the connected display.
> > 
> > What I hope we can find a solution for is a single file/driver that can
> > support all the relevant interface types for a chip.
> > So we would end up with a single file that included the necessary
> > support for ili9341 in all interface configurations with the necessary
> > support for the relevant displays.
> > 
> > I do not know how far we are from this as I have not dived into the
> > details of any of the proposals.
> 
> In an ideal world I would have liked to see the MIPI DBI parallel
> interface implemented using a new Linux parallel bus type. It could have
> drivers for gpio bitbanging and mmio in addition to other hw specific
> drivers. Now we could have a drm_mipi_dbi DRM driver that registers as a
> SPI client driver and a Parallel bus client driver. Or it can be a
> component driver for the existing DRM driver on the SoC.
> 
> I had plans to do this and made a prototype, but dropped it since it
> would probably require a lot of work getting in a new Linux bus type.

Channelling my inner Greg KH:

Please just create a new bus, it should be quite easy and boilerplate is
manageable.

Greg, did I get this right? Maybe any recommendations for a simple
parallel bus with perhaps different register access paths depending upon
how it's all wired up exactly?
-Daniel

> However if we're going to treat this parallel bus only as a MIPI DBI
> display interface but support gpio bitbanging and mmio as well, then we
> could add DRM drivers for each MIPI DBI bus (that don't have special
> parallel bus hw):
> - mipi-dbi-spi
> - mipi-dbi-gpio
> - mipi-dbi-mmio
> 
> These drivers will register as a mipi_dsi_host adapted like Paul suggested.
> 
> The panel drivers will be mipi_dsi_drivers. Now the panels should work
> regardless of bus type. They probably need to know about the bus type,
> at least whether the parallell bus is 8-bit or 16-bit wide.
> 
> The current MIPI DBI SPI drivers (drm/tiny) will need to be treated
> specially to keep working with old Device Trees when moved over to
> drm/panel.
> 
> Noralf.
> 
> 
> >>
> >> My suggestion is to have one panel driver module that can support all of
> >> these like this:
> > So I think we agree here.
> > 
> >>
> >> For 1. and 2. a SPI driver is registered and if I understand your
> >> example correctly of_graph_get_port_by_id() can be used during probe to
> >> distinguish between the 2 options and register a full DRM driver for 1.
> >> and add a DRM panel for 2.
> >>
> >> For 3. a DSI driver is registered (adapted for DBI use like you're
> >> suggesting).
> >>
> >> Note that the interface part of the controller initialization will
> >> differ between these, the panel side init will be the same I assume.
> > 
> > 	Sam
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel