| Message ID | 20211124150757.17929-1-noralf@tronnes.org |
|---|---|
| Headers | show |
| Series | drm/tiny/st7735r: Match up with staging/fbtft driver | expand |
Den 06.12.2021 16.26, skrev David Lechner: > On 12/1/21 8:52 AM, Maxime Ripard wrote: >> Hi Noralf, >> >> On Tue, Nov 30, 2021 at 03:30:11PM +0100, Noralf Trønnes wrote: >>> Den 29.11.2021 10.39, skrev Maxime Ripard: >>>> On Wed, Nov 24, 2021 at 04:03:07PM -0600, David Lechner wrote: >>>>> On 11/24/21 9:07 AM, Noralf Trønnes wrote: >>>> I agree that it doesn't really fit in the DT either though. Noralf, >>>> what >>>> kind of data do we need to setup a display in fbtft? The init sequence, >>>> and maybe some enable/reset GPIO, plus some timing duration maybe? >>>> >>>> There's one similar situation I can think of: wifi chips. Those also >>>> need a few infos from the DT (like what bus it's connected to, enable >>>> GPIO, etc) and a different sequence (firmware), sometimes different >>>> from >>>> one board to the other. >>>> >>>> Could we have a binding that would be something like: >>>> >>>> panel@42 { >>>> compatible = "panel-spi"; >>>> model = "panel-from-random-place-42"; >>>> enable-gpios = <&...>; >>>> } >>>> >>>> And then, the driver would request the init sequence through the >>>> firmware mechanism using a name generated from the model property. >>>> >>>> It allows to support multiple devices in a given system, since the >>>> firmware name wouldn't conflict, it makes a decent binding, and users >>>> can adjust the init sequence easily (maybe with a bit of tooling) >>>> >>>> Would that work? >>> >>> I really like this idea. An added benefit is that one driver can handle >>> all MIPI DBI compatible controllers avoiding the need to do a patchset >>> like this for all the various MIPI DBI controllers. The firmware will >>> just contain numeric commands with parameters, so no need for different >>> controller drivers to handle the controller specific command names. >>> >>> The following is a list of the MIPI DBI compatible controllers currently >>> in staging/fbtft: ili9341, hx8357d, st7735r, ili9163, ili9163, ili9163, >>> ili9163, ili9486, ili9481, tinylcd, s6d02a1, s6d02a1, hx8340bn, ili9340. >>> >>> The compatible needs to be a bit more specific though since there are 2 >>> major SPI protocols for these display: MIPI DBI and the one used by >>> ILI9325 and others. >>> >>> The full binding would be something like this: >>> >>> panel@42 { >>> compatible = "panel-mipi-dbi-spi"; >>> model = "panel-from-random-place-42"; >>> >>> /* The MIPI DBI spec lists these powers supply pins */ >>> vdd-supply = <&...>; >>> vddi-supply = <&...>; >>> >>> /* Optional gpio to drive the RESX line */ >>> reset-gpios = <&...>; >>> >>> /* >>> * D/CX: Data/Command, Command is active low >>> * Abcense: Interface option 1 (D/C embedded in 9-bit word) >>> * Precense: Interface option 3 >>> */ >>> dc-gpios = <&...>; >>> >>> /* >>> * If set the driver won't try to read from the controller to see >>> * if it's already configured by the bootloader or previously by >>> * the driver. A readable controller avoids flicker and/or delay >>> * enabling the pipeline. >>> * >>> * This property might not be necessary if we are guaranteed to >>> * always read back all 1's or 0's when MISO is not connected. >>> * I don't know if all setups can guarantee that. >>> */ >>> write-only; >>> >>> /* Optional ref to backlight node */ >>> backlight = <&...>; >>> } >> >> It looks decent to me. We'll want Rob to give his opinion though, but it >> looks in a much better shape compared to what we usually have :) >> >>> Many of these controllers also have a RGB interface option for the >>> pixels and only do configuration over SPI. >>> Maybe the compatible should reflect these 2 options somehow? >> >> I think we'll want a "real" panel for RGB, with its own compatible >> though. We have a few of these drivers in tree already, so it's better >> to remain consistent. >> >> Maxime >> > > I'm on board with the idea of the init sequence as firmware as well. > > It looks like Rob might have missed this thread, so maybe just apply > the acked patches and submit a v2 with the firmware implementation? > Yes, that's my plan. Noralf.
Den 24.11.2021 16.07, skrev Noralf Trønnes: > Hi, > > This patchset adds a missing piece for decommissioning the > staging/fbtft/fb_st7735r.c driver namely a way to configure the > controller from Device Tree. > > All fbtft drivers have builtin support for one display panel and all > other panels using that controller are configured using the Device Tree > 'init' property. This property is supported by all fbtft drivers and > provides a generic way to set register values or issue commands > (depending on the type of controller). > > It is common for these types of displays to have a datasheet listing the > necessary controller settings/commands or some example code doing the > same. > > This is how the panel directly supported by the fb_st7735r staging > driver is described using Device Tree with that driver: > > width = <160>; > height = <128>; > > init = <0x1000001 > 0x2000096 > 0x1000011 > 0x20000ff > 0x10000B1 0x01 0x2C 0x2D > 0x10000B4 0x07 > 0x10000C0 0xA2 0x02 0x84 > 0x10000C1 0xC5 > 0x10000C2 0x0A 0x00 > 0x10000C5 0x0E > 0x100003a 0x55 > 0x1000036 0x60 > 0x10000E0 0x0F 0x1A 0x0F 0x18 0x2F 0x28 0x20 0x22 > 0x1F 0x1B 0x23 0x37 0x00 0x07 0x02 0x10 > 0x10000E1 0x0F 0x1B 0x0F 0x17 0x33 0x2C 0x29 0x2E > 0x30 0x30 0x39 0x3F 0x00 0x07 0x03 0x10 > 0x1000029 > 0x2000064>; > > > This is how the same panel is described using the st7735r drm driver and > this patchset: > > width = <160>; > height = <128>; > > frmctr1 = [ 01 2C 2D ]; > invctr = [ 07 ]; > pwctr1 = [ A2 02 84 ]; > pwctr2 = [ C5 ]; > pwctr3 = [ 0A 00 ]; > vmctr1 = [ 0E ]; > madctl = [ 60 ]; > gamctrp1 = [ 0F 1A 0F 18 2F 28 20 22 1F 1B 23 37 00 07 02 10 ]; > gamctrn1 = [ 0F 1B 0F 17 33 2C 29 2E 30 30 39 3F 00 07 03 10 ]; > > > Back when the fbtft drivers were added to staging I asked on the DT > mailinglist if it was OK to use the 'init' property but it was turned > down. In this patchset I'm trying the same approach used by the > solomon,ssd1307fb.yaml binding in describing the attached panel. That > binding prefixes the properties with the vendor name, not sure why that > is and I think it looks strange so I try without it. > > Noralf. > > > Noralf Trønnes (6): > dt-bindings: display: sitronix,st7735r: Fix backlight in example > dt-bindings: display: sitronix,st7735r: Make reset-gpios optional > dt-bindings: display: sitronix,st7735r: Remove spi-max-frequency limit Patches 1-3 applied, thanks for reviewing. The change to the driver has been replaced by a new generic driver panel-mipi-dbi. Noralf. > dt-bindings: display: sitronix,st7735r: Add initialization properties > drm/mipi-dbi: Add device property functions > drm: tiny: st7735r: Support DT initialization of controller > > .../bindings/display/sitronix,st7735r.yaml | 122 ++++++++++++++- > drivers/gpu/drm/drm_mipi_dbi.c | 139 ++++++++++++++++++ > drivers/gpu/drm/tiny/st7735r.c | 87 +++++++++-- > include/drm/drm_mipi_dbi.h | 3 + > 4 files changed, 334 insertions(+), 17 deletions(-) >
Hi, This patchset adds a missing piece for decommissioning the staging/fbtft/fb_st7735r.c driver namely a way to configure the controller from Device Tree. All fbtft drivers have builtin support for one display panel and all other panels using that controller are configured using the Device Tree 'init' property. This property is supported by all fbtft drivers and provides a generic way to set register values or issue commands (depending on the type of controller). It is common for these types of displays to have a datasheet listing the necessary controller settings/commands or some example code doing the same. This is how the panel directly supported by the fb_st7735r staging driver is described using Device Tree with that driver: width = <160>; height = <128>; init = <0x1000001 0x2000096 0x1000011 0x20000ff 0x10000B1 0x01 0x2C 0x2D 0x10000B4 0x07 0x10000C0 0xA2 0x02 0x84 0x10000C1 0xC5 0x10000C2 0x0A 0x00 0x10000C5 0x0E 0x100003a 0x55 0x1000036 0x60 0x10000E0 0x0F 0x1A 0x0F 0x18 0x2F 0x28 0x20 0x22 0x1F 0x1B 0x23 0x37 0x00 0x07 0x02 0x10 0x10000E1 0x0F 0x1B 0x0F 0x17 0x33 0x2C 0x29 0x2E 0x30 0x30 0x39 0x3F 0x00 0x07 0x03 0x10 0x1000029 0x2000064>; This is how the same panel is described using the st7735r drm driver and this patchset: width = <160>; height = <128>; frmctr1 = [ 01 2C 2D ]; invctr = [ 07 ]; pwctr1 = [ A2 02 84 ]; pwctr2 = [ C5 ]; pwctr3 = [ 0A 00 ]; vmctr1 = [ 0E ]; madctl = [ 60 ]; gamctrp1 = [ 0F 1A 0F 18 2F 28 20 22 1F 1B 23 37 00 07 02 10 ]; gamctrn1 = [ 0F 1B 0F 17 33 2C 29 2E 30 30 39 3F 00 07 03 10 ]; Back when the fbtft drivers were added to staging I asked on the DT mailinglist if it was OK to use the 'init' property but it was turned down. In this patchset I'm trying the same approach used by the solomon,ssd1307fb.yaml binding in describing the attached panel. That binding prefixes the properties with the vendor name, not sure why that is and I think it looks strange so I try without it. Noralf. Noralf Trønnes (6): dt-bindings: display: sitronix,st7735r: Fix backlight in example dt-bindings: display: sitronix,st7735r: Make reset-gpios optional dt-bindings: display: sitronix,st7735r: Remove spi-max-frequency limit dt-bindings: display: sitronix,st7735r: Add initialization properties drm/mipi-dbi: Add device property functions drm: tiny: st7735r: Support DT initialization of controller .../bindings/display/sitronix,st7735r.yaml | 122 ++++++++++++++- drivers/gpu/drm/drm_mipi_dbi.c | 139 ++++++++++++++++++ drivers/gpu/drm/tiny/st7735r.c | 87 +++++++++-- include/drm/drm_mipi_dbi.h | 3 + 4 files changed, 334 insertions(+), 17 deletions(-)