From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752473AbbJJPgk (ORCPT ); Sat, 10 Oct 2015 11:36:40 -0400 Received: from smtp.domeneshop.no ([194.63.252.55]:48867 "EHLO smtp.domeneshop.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751472AbbJJPgh (ORCPT ); Sat, 10 Oct 2015 11:36:37 -0400 Subject: Re: [PATCH 1/2] staging: fbtft: add support for ST7789V display controller To: Dennis Menschel , Thomas Petazzoni References: <15d2be7d8dae6c8f7c399478db405479b01b698d.1444159111.git.menschel-d@posteo.de> Cc: Greg Kroah-Hartman , linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org From: =?UTF-8?Q?Noralf_Tr=c3=b8nnes?= Message-ID: <5619307D.8080009@tronnes.org> Date: Sat, 10 Oct 2015 17:36:29 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <15d2be7d8dae6c8f7c399478db405479b01b698d.1444159111.git.menschel-d@posteo.de> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- 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