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

[Paul Cercueil]

Hi Noralf,


Le dim. 14 juin 2020 à 18:36, Noralf Trønnes <noralf at tronnes.org> a 
écrit :
> 
> 
> 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 at 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.

That's a minimal drawback for a perfectly architectured design ;)

> 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
> 
> My suggestion is to have one panel driver module that can support all 
> of
> these like this:
> 
> 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).

That means basically having two entry points per DBI driver, one as DSI 
device and one as SPI device, the latter doing the job of the current 
DBI-SPI bridge. I think i would be cleaner to just have duplicated 
strings with the DBI-SPI bridge.

Cheers,
-Paul

> Note that the interface part of the controller initialization will
> differ between these, the panel side init will be the same I assume.

> Noralf.