Re: [PATCH 1/2] staging: fbtft: add support for ST7789V display controller

[Dennis Menschel <menschel-d@posteo.de>]

Am 10.10.2015 um 17:36 schrieb Noralf Trønnes:
> 
> Den 07.10.2015 22:15, skrev Dennis Menschel:
>> This patch adds support for the Sitronix ST7789V display controller.
>> The controller is intended for small color displays with a resolution
>> of up to 320x240 pixels.
>>
>> Signed-off-by: Dennis Menschel <menschel-d@posteo.de>
>> ---
> 
> The future of fbtft is uncertain after fbdev maintainer Tomi Valkeinen
> suggested that it be removed [1], since fbdev is a deprecated framework.
> I have started to look at a DRM version, but this will take some time pull
> through (fbtft is just one of my hobby projects). I suggest that we still
> accept new drivers and if necessary convert them to DRM drivers later.
> 
> If we do that, there's one change I would like to see and that is switching
> to display/panel drivers instead of controller drivers. The only common
> functionality between controllers, that we make use of, is the
> set_addr_win() function (I didn't know that when I started fbtft based on
> the 2 displays I had). This is a small function so I don't see a problem
> duplicating this in new drivers. It's also possible to export the function
> from a controller driver. fbtft has a default set_addr_win() implementation
> that matches the MIPI compatible controllers (0x2A, 0x2B, 0x2C).
> (blank() is used on OLED controllers and set_gamma() will be obsolete with
> display drivers since gamma can be set in init())
> 
> Whatever fbtft turns into, it will use display drivers instead of
> controller drivers. I have done some work on fbtft that gives a hint on
> how it might look [2] (I stopped that work when I understood that fbtft
> might not move out of staging after all).
> The main reason for switching to displays drivers is that most controllers
> have a quite large register set with lots of settings. This is quite
> impossible to turn into Device Tree properties. And many panels also use
> undocumented registers.
> 
> fbtft_device
> The fbtft_device module provides a way to add devices for the fbtft
> drivers.
> It was made to make life simple for Raspberry Pi users when it's kernel
> didn't support Device Tree. There are no other modules like it in the
> kernel,
> so it will not move out of staging. The reason I haven't removed it is that
> it's tightly coupled with the flexfb driver making it usable with panels
> that's not supported by the other drivers. For instance the set_par()
> function, in the drivers that implement it, hardcodes some rotation/mirror
> register values that are panel specific. flexfb doesn't have a set_par()
> function, so it won't overwrite values set in init(), making it useful
> with some panels. I suggest that we don't make any more additions to
> fbtft_device, but just keep it as it is for now. fbtft_device has many
> module parameters that should cover most if not all configs [3].
> 
> 
> So for this particular patch I suggest you turn it into a display driver
> and that we merge that (drop the fbtft_device patch).
> Please write the init sequence in an init() function using write_reg()
> since init_sequence will go away. There's an enum for the MIPI DCS
> commands in include/video/mipi_display.h
> 
> 
> Noralf.
> 
> [1] https://lkml.org/lkml/2015/9/24/253
> [2] https://github.com/notro/linux-staging/commits/next
> [3] https://github.com/notro/fbtft/wiki/fbtft_device
> 

Thank you for your detailed explanation about the current state of fbtft
and the future plans for a possible successor framework. As suggested by
you, I'll make the following changes in the next patch:

- Change the st7789v controller driver to a cberry28 display driver.
- If applicable, use the default fbtft implementation of set_addr_win().
- Remove the blank() function as it is only intended for OLED displays.
- Use an init() function instead of an init sequence.
- Integrate the gamma settings into the init() function.
- Use enum for MIPI DCS commands which can be found in
  include/video/mipi_display.h.
- Do not extend fbtft_device since the displays can now be loaded via
  device tree (overlays).

Best regards,
Dennis Menschel