* [PATCH v2 0/5] Enable ilitek ili9341 on stm32f429-disco board
@ 2020-05-08 4:13 dillon.minfei
2020-05-08 4:13 ` [PATCH v2 1/5] ARM: dts: stm32: Add pin map for ltdc, spi5 " dillon.minfei
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: dillon.minfei @ 2020-05-08 4:13 UTC (permalink / raw)
To: robh+dt, mcoquelin.stm32, alexandre.torgue, thierry.reding, sam,
airlied, daniel, mturquette, sboyd
Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel,
dri-devel, linux-clk, dillon min
From: dillon min <dillon.minfei@gmail.com>
This patchset have following changes
V2: verify ilitek,ili9341.yaml with make O=../linux-stm32 dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/panel/ilitek,ili9341.yaml
V1:
add ili9341 drm panel driver
add ltdc, spi5 controller for stm32f429-disco
add ltdc, spi5 pin map for stm32f429-disco
add docs about ili9341
fix ltdc driver loading hang in clk set rate bug
dillon min (5):
ARM: dts: stm32: Add pin map for ltdc, spi5 on stm32f429-disco board
ARM: dts: stm32: enable ltdc binding with ili9341 on stm32429-disco
board
dt-bindings: display: panel: Add ilitek ili9341 panel bindings
clk: stm32: Fix stm32f429 ltdc driver loading hang in clk set rate.
keep ltdc clk running after kernel startup
drm/panel: add panel driver for Ilitek ili9341 panels
.../bindings/display/panel/ilitek,ili9341.yaml | 68 +++
arch/arm/boot/dts/stm32f4-pinctrl.dtsi | 67 +++
arch/arm/boot/dts/stm32f429-disco.dts | 40 ++
drivers/clk/clk-stm32f4.c | 5 +-
drivers/gpu/drm/panel/Kconfig | 8 +
drivers/gpu/drm/panel/Makefile | 1 +
drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 561 +++++++++++++++++++++
7 files changed, 748 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9341.yaml
create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 1/5] ARM: dts: stm32: Add pin map for ltdc, spi5 on stm32f429-disco board 2020-05-08 4:13 [PATCH v2 0/5] Enable ilitek ili9341 on stm32f429-disco board dillon.minfei @ 2020-05-08 4:13 ` dillon.minfei 2020-05-08 4:13 ` [PATCH v2 2/5] ARM: dts: stm32: enable ltdc binding with ili9341 on stm32429-disco board dillon.minfei ` (3 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: dillon.minfei @ 2020-05-08 4:13 UTC (permalink / raw) To: robh+dt, mcoquelin.stm32, alexandre.torgue, thierry.reding, sam, airlied, daniel, mturquette, sboyd Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel, dri-devel, linux-clk, dillon min From: dillon min <dillon.minfei@gmail.com> This patch adds the pin configuration for ltdc, spi5 controller on stm32f429-disco board. Signed-off-by: dillon min <dillon.minfei@gmail.com> --- arch/arm/boot/dts/stm32f4-pinctrl.dtsi | 67 ++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/arch/arm/boot/dts/stm32f4-pinctrl.dtsi b/arch/arm/boot/dts/stm32f4-pinctrl.dtsi index 392fa14..0eb107f 100644 --- a/arch/arm/boot/dts/stm32f4-pinctrl.dtsi +++ b/arch/arm/boot/dts/stm32f4-pinctrl.dtsi @@ -316,6 +316,73 @@ }; }; + ltdc_pins_f429_disco: ltdc-1 { + pins { + pinmux = <STM32_PINMUX('C', 6, AF14)>, + /* LCD_HSYNC */ + <STM32_PINMUX('A', 4, AF14)>, + /* LCD_VSYNC */ + <STM32_PINMUX('G', 7, AF14)>, + /* LCD_CLK */ + <STM32_PINMUX('C', 10, AF14)>, + /* LCD_R2 */ + <STM32_PINMUX('B', 0, AF9)>, + /* LCD_R3 */ + <STM32_PINMUX('A', 11, AF14)>, + /* LCD_R4 */ + <STM32_PINMUX('A', 12, AF14)>, + /* LCD_R5 */ + <STM32_PINMUX('B', 1, AF9)>, + /* LCD_R6*/ + <STM32_PINMUX('G', 6, AF14)>, + /* LCD_R7 */ + <STM32_PINMUX('A', 6, AF14)>, + /* LCD_G2 */ + <STM32_PINMUX('G', 10, AF9)>, + /* LCD_G3 */ + <STM32_PINMUX('B', 10, AF14)>, + /* LCD_G4 */ + <STM32_PINMUX('D', 6, AF14)>, + /* LCD_B2 */ + <STM32_PINMUX('G', 11, AF14)>, + /* LCD_B3*/ + <STM32_PINMUX('B', 11, AF14)>, + /* LCD_G5 */ + <STM32_PINMUX('C', 7, AF14)>, + /* LCD_G6 */ + <STM32_PINMUX('D', 3, AF14)>, + /* LCD_G7 */ + <STM32_PINMUX('G', 12, AF9)>, + /* LCD_B4 */ + <STM32_PINMUX('A', 3, AF14)>, + /* LCD_B5 */ + <STM32_PINMUX('B', 8, AF14)>, + /* LCD_B6 */ + <STM32_PINMUX('B', 9, AF14)>, + /* LCD_B7 */ + <STM32_PINMUX('F', 10, AF14)>; + /* LCD_DE */ + slew-rate = <2>; + }; + }; + + spi5_pins: spi5-0 { + pins1 { + pinmux = <STM32_PINMUX('F', 7, AF5)>, + /* SPI5_CLK */ + <STM32_PINMUX('F', 9, AF5)>; + /* SPI5_MOSI */ + bias-disable; + drive-push-pull; + slew-rate = <0>; + }; + pins2 { + pinmux = <STM32_PINMUX('F', 8, AF5)>; + /* SPI5_MISO */ + bias-disable; + }; + }; + dcmi_pins: dcmi-0 { pins { pinmux = <STM32_PINMUX('A', 4, AF13)>, /* DCMI_HSYNC */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/5] ARM: dts: stm32: enable ltdc binding with ili9341 on stm32429-disco board 2020-05-08 4:13 [PATCH v2 0/5] Enable ilitek ili9341 on stm32f429-disco board dillon.minfei 2020-05-08 4:13 ` [PATCH v2 1/5] ARM: dts: stm32: Add pin map for ltdc, spi5 " dillon.minfei @ 2020-05-08 4:13 ` dillon.minfei 2020-05-08 4:13 ` [PATCH v2 3/5] dt-bindings: display: panel: Add ilitek ili9341 panel bindings dillon.minfei ` (2 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: dillon.minfei @ 2020-05-08 4:13 UTC (permalink / raw) To: robh+dt, mcoquelin.stm32, alexandre.torgue, thierry.reding, sam, airlied, daniel, mturquette, sboyd Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel, dri-devel, linux-clk, dillon min From: dillon min <dillon.minfei@gmail.com> Enable the ltdc & ili9341 on stm32429-disco board. Signed-off-by: dillon min <dillon.minfei@gmail.com> --- arch/arm/boot/dts/stm32f429-disco.dts | 40 +++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/arch/arm/boot/dts/stm32f429-disco.dts b/arch/arm/boot/dts/stm32f429-disco.dts index 30c0f67..2d9637a 100644 --- a/arch/arm/boot/dts/stm32f429-disco.dts +++ b/arch/arm/boot/dts/stm32f429-disco.dts @@ -49,6 +49,8 @@ #include "stm32f429.dtsi" #include "stm32f429-pinctrl.dtsi" #include <dt-bindings/input/input.h> +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/gpio/gpio.h> / { model = "STMicroelectronics STM32F429i-DISCO board"; @@ -127,3 +129,41 @@ pinctrl-names = "default"; status = "okay"; }; + +<dc { + status = "okay"; + pinctrl-0 = <<dc_pins_f429_disco>; + pinctrl-names = "default"; + + port { + ltdc_out_rgb: endpoint { + remote-endpoint = <&panel_in_rgb>; + }; + }; +}; + +&spi5 { + status = "okay"; + pinctrl-0 = <&spi5_pins>; + pinctrl-names = "default"; + #address-cells = <1>; + #size-cells = <0>; + cs-gpios = <&gpioc 2 GPIO_ACTIVE_LOW>; + dmas = <&dma2 3 2 0x400 0x0>, + <&dma2 4 2 0x400 0x0>; + dma-names = "rx", "tx"; + display: display@0{ + /* Connect panel-ilitek-9341 to ltdc */ + compatible = "stm32f429,ltdc-panel", "ilitek,ili9341"; + reg = <0>; + spi-3wire; + spi-max-frequency = <10000000>; + dc-gpios = <&gpiod 13 0>; + port { + panel_in_rgb: endpoint { + remote-endpoint = <<dc_out_rgb>; + }; + }; + }; +}; + -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/5] dt-bindings: display: panel: Add ilitek ili9341 panel bindings 2020-05-08 4:13 [PATCH v2 0/5] Enable ilitek ili9341 on stm32f429-disco board dillon.minfei 2020-05-08 4:13 ` [PATCH v2 1/5] ARM: dts: stm32: Add pin map for ltdc, spi5 " dillon.minfei 2020-05-08 4:13 ` [PATCH v2 2/5] ARM: dts: stm32: enable ltdc binding with ili9341 on stm32429-disco board dillon.minfei @ 2020-05-08 4:13 ` dillon.minfei 2020-05-08 4:13 ` [PATCH v2 4/5] clk: stm32: Fix stm32f429 ltdc driver loading hang in clk set rate. keep ltdc clk running after kernel startup dillon.minfei 2020-05-08 4:13 ` [PATCH v2 5/5] drm/panel: add panel driver for Ilitek ili9341 panels dillon.minfei 4 siblings, 0 replies; 10+ messages in thread From: dillon.minfei @ 2020-05-08 4:13 UTC (permalink / raw) To: robh+dt, mcoquelin.stm32, alexandre.torgue, thierry.reding, sam, airlied, daniel, mturquette, sboyd Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel, dri-devel, linux-clk, dillon min From: dillon min <dillon.minfei@gmail.com> Add documentation for "ilitek,ili9341" panel. Signed-off-by: dillon min <dillon.minfei@gmail.com> --- Hi Rob Herring, This patch [PATCH V2 3/5] about ilitek,ili9341.yaml was verifyed with make dt_binding_check thanks. best regards, dillon, .../bindings/display/panel/ilitek,ili9341.yaml | 68 ++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9341.yaml diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9341.yaml b/Documentation/devicetree/bindings/display/panel/ilitek,ili9341.yaml new file mode 100644 index 0000000..94c2b15 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9341.yaml @@ -0,0 +1,68 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/ilitek,ili9341.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Ilitek-9341 Display Panel + +maintainers: + - Dillon Min <dillon.minfei@gmail.com> + +description: | + Ilitek ILI9341 TFT panel driver with SPI control bus + This is a driver for 320x240 TFT panels, accepting a rgb input + streams that get adapted and scaled to the panel. + +allOf: + - $ref: panel-common.yaml# + +properties: + compatible: + items: + - const: stm32f429,ltdc-panel + - {} # ilitek,ili9341, but not listed here to avoid false select + + reg: true + + dc-gpios: + maxItems: 1 + description: panel spi dc gpio + + spi-3wire: true + + spi-max-frequency: + const: 10000000 + + port: true + +additionalProperties: false + +required: + - compatible + - reg + - dc-gpios + - spi-3wire + - spi-max-frequency + - port + +examples: + - |+ + spi { + #address-cells = <1>; + #size-cells = <0>; + panel: display@0 { + compatible = "stm32f429,ltdc-panel", "ilitek,ili9341"; + reg = <0>; + spi-3wire; + spi-max-frequency = <10000000>; + dc-gpios = <&gpiod 13 0>; + port { + panel_in: endpoint { + remote-endpoint = <&display_out>; + }; + }; + }; + }; +... + -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 4/5] clk: stm32: Fix stm32f429 ltdc driver loading hang in clk set rate. keep ltdc clk running after kernel startup 2020-05-08 4:13 [PATCH v2 0/5] Enable ilitek ili9341 on stm32f429-disco board dillon.minfei ` (2 preceding siblings ...) 2020-05-08 4:13 ` [PATCH v2 3/5] dt-bindings: display: panel: Add ilitek ili9341 panel bindings dillon.minfei @ 2020-05-08 4:13 ` dillon.minfei 2020-05-08 4:13 ` [PATCH v2 5/5] drm/panel: add panel driver for Ilitek ili9341 panels dillon.minfei 4 siblings, 0 replies; 10+ messages in thread From: dillon.minfei @ 2020-05-08 4:13 UTC (permalink / raw) To: robh+dt, mcoquelin.stm32, alexandre.torgue, thierry.reding, sam, airlied, daniel, mturquette, sboyd Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel, dri-devel, linux-clk, dillon min From: dillon min <dillon.minfei@gmail.com> as store stm32f4_rcc_register_pll return to the wrong offset of clks, so ltdc gate clk is null. need change clks[PLL_VCO_SAI] to clks[PLL_SAI] add CLK_IGNORE_UNUSED for ltdc to make sure clk not be freed by clk_disable_unused Signed-off-by: dillon min <dillon.minfei@gmail.com> --- drivers/clk/clk-stm32f4.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c index 18117ce..0ba73de 100644 --- a/drivers/clk/clk-stm32f4.c +++ b/drivers/clk/clk-stm32f4.c @@ -129,7 +129,8 @@ static const struct stm32f4_gate_data stm32f429_gates[] __initconst = { { STM32F4_RCC_APB2ENR, 20, "spi5", "apb2_div" }, { STM32F4_RCC_APB2ENR, 21, "spi6", "apb2_div" }, { STM32F4_RCC_APB2ENR, 22, "sai1", "apb2_div" }, - { STM32F4_RCC_APB2ENR, 26, "ltdc", "apb2_div" }, + { STM32F4_RCC_APB2ENR, 26, "ltdc", "apb2_div", + CLK_IGNORE_UNUSED }, }; static const struct stm32f4_gate_data stm32f469_gates[] __initconst = { @@ -1757,7 +1758,7 @@ static void __init stm32f4_rcc_init(struct device_node *np) clks[PLL_VCO_I2S] = stm32f4_rcc_register_pll("vco_in", &data->pll_data[1], &stm32f4_clk_lock); - clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll("vco_in", + clks[PLL_SAI] = stm32f4_rcc_register_pll("vco_in", &data->pll_data[2], &stm32f4_clk_lock); for (n = 0; n < MAX_POST_DIV; n++) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 5/5] drm/panel: add panel driver for Ilitek ili9341 panels 2020-05-08 4:13 [PATCH v2 0/5] Enable ilitek ili9341 on stm32f429-disco board dillon.minfei ` (3 preceding siblings ...) 2020-05-08 4:13 ` [PATCH v2 4/5] clk: stm32: Fix stm32f429 ltdc driver loading hang in clk set rate. keep ltdc clk running after kernel startup dillon.minfei @ 2020-05-08 4:13 ` dillon.minfei 2020-05-08 9:02 ` Sam Ravnborg 4 siblings, 1 reply; 10+ messages in thread From: dillon.minfei @ 2020-05-08 4:13 UTC (permalink / raw) To: robh+dt, mcoquelin.stm32, alexandre.torgue, thierry.reding, sam, airlied, daniel, mturquette, sboyd Cc: devicetree, linux-stm32, linux-arm-kernel, linux-kernel, dri-devel, linux-clk, dillon min From: dillon min <dillon.minfei@gmail.com> This is a driver for 320x240 TFT panels, accepting a rgb input streams that get adapted and scaled to the panel. Signed-off-by: dillon min <dillon.minfei@gmail.com> --- drivers/gpu/drm/panel/Kconfig | 8 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 561 +++++++++++++++++++++++++++ 3 files changed, 570 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index a1723c1..e42692c 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -95,6 +95,14 @@ config DRM_PANEL_ILITEK_IL9322 Say Y here if you want to enable support for Ilitek IL9322 QVGA (320x240) RGB, YUV and ITU-T BT.656 panels. +config DRM_PANEL_ILITEK_IL9341 + tristate "Ilitek ILI9341 240x320 QVGA panels" + depends on OF && SPI + select REGMAP + help + Say Y here if you want to enable support for Ilitek IL9341 + QVGA (240x320) RGB panels. + config DRM_PANEL_ILITEK_ILI9881C tristate "Ilitek ILI9881C-based panels" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index 96a883c..d123543 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) += panel-elida-kd35t133.o obj-$(CONFIG_DRM_PANEL_FEIXIN_K101_IM2BA02) += panel-feixin-k101-im2ba02.o obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d.o obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o +obj-$(CONFIG_DRM_PANEL_ILITEK_IL9341) += panel-ilitek-ili9341.o obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c new file mode 100644 index 0000000..ec22d80 --- /dev/null +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c @@ -0,0 +1,561 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Ilitek ILI9341 TFT LCD drm_panel driver. + * + * This panel can be configured to support: + * - 16-bit parallel RGB interface + * + * Copyright (C) 2020 Dillon Min <dillon.minfei@gmail.com> + * Derived from drivers/drm/gpu/panel/panel-ilitek-ili9322.c + */ + +#include <linux/bitops.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/regmap.h> +#include <linux/regulator/consumer.h> +#include <linux/spi/spi.h> + +#include <video/mipi_display.h> +#include <video/of_videomode.h> +#include <video/videomode.h> + +#include <drm/drm_modes.h> +#include <drm/drm_panel.h> +#include <drm/drm_print.h> + +#define DEFAULT_SPI_SPEED 10000000 + +#define ILI9341_SLEEP_OUT 0x11 /* Sleep out register */ +#define ILI9341_GAMMA 0x26 /* Gamma register */ +#define ILI9341_DISPLAY_OFF 0x28 /* Display off register */ +#define ILI9341_DISPLAY_ON 0x29 /* Display on register */ +#define ILI9341_COLUMN_ADDR 0x2A /* Colomn address register */ +#define ILI9341_PAGE_ADDR 0x2B /* Page address register */ +#define ILI9341_GRAM 0x2C /* GRAM register */ +#define ILI9341_MAC 0x36 /* Memory Access Control register*/ +#define ILI9341_PIXEL_FORMAT 0x3A /* Pixel Format register */ +#define ILI9341_WDB 0x51 /* Write Brightness Display + * register + */ +#define ILI9341_WCD 0x53 /* Write Control Display + * register + */ +#define ILI9341_RGB_INTERFACE 0xB0 /* RGB Interface Signal Control */ +#define ILI9341_FRC 0xB1 /* Frame Rate Control register */ +#define ILI9341_BPC 0xB5 /* Blanking Porch Control + * register + */ +#define ILI9341_DFC 0xB6 /* Display Function Control + * register + */ +#define ILI9341_POWER1 0xC0 /* Power Control 1 register */ +#define ILI9341_POWER2 0xC1 /* Power Control 2 register */ +#define ILI9341_VCOM1 0xC5 /* VCOM Control 1 register */ +#define ILI9341_VCOM2 0xC7 /* VCOM Control 2 register */ +#define ILI9341_POWERA 0xCB /* Power control A register */ +#define ILI9341_POWERB 0xCF /* Power control B register */ +#define ILI9341_PGAMMA 0xE0 /* Positive Gamma Correction + * register + */ +#define ILI9341_NGAMMA 0xE1 /* Negative Gamma Correction + * register + */ +#define ILI9341_DTCA 0xE8 /* Driver timing control A */ +#define ILI9341_DTCB 0xEA /* Driver timing control B */ +#define ILI9341_POWER_SEQ 0xED /* Power on sequence register */ +#define ILI9341_3GAMMA_EN 0xF2 /* 3 Gamma enable register */ +#define ILI9341_INTERFACE 0xF6 /* Interface control register */ +#define ILI9341_PRC 0xF7 /* Pump ratio control register */ + +static u8 ili9341_cmd0[] = {0xc3, 0x08, 0x50}; +static u8 ili9341_powerb[] = {0x00, 0xc1, 0x30}; +static u8 ili9341_power_seq[] = {0x64, 0x03, 0x12, 0x81}; +static u8 ili9341_dtca[] = {0x85, 0x00, 0x78}; +static u8 ili9341_powera[] = {0x39, 0x2c, 0x00, 0x34, 0x02}; +static u8 ili9341_prc[] = {0x20}; +static u8 ili9341_dtcb[] = {0x00, 0x00}; +static u8 ili9341_frc[] = {0x00, 0x1b}; +static u8 ili9341_dfc1[] = {0x0a, 0xa2}; +static u8 ili9341_power1[] = {0x10}; +static u8 ili9341_power2[] = {0x10}; +static u8 ili9341_vcom1[] = {0x45, 0x15}; +static u8 ili9341_vcom2[] = {0x90}; +static u8 ili9341_mac[] = {0xc8}; +static u8 ili9341_gamma_en[] = {0x00}; +static u8 ili9341_rgb_intr[] = {0xc2}; +static u8 ili9341_dfc2[] = {0x0a, 0xa7, 0x27, 0x04}; +static u8 ili9341_column_addr[] = {0x00, 0x00, 0x00, 0xef}; +static u8 ili9341_page_addr[] = {0x00, 0x00, 0x01, 0x3f}; +static u8 ili9341_intr[] = {0x01, 0x00, 0x06}; +static u8 ili9341_gamma[] = {0x01}; +static u8 ili9341_pgamma[] = {0x0f, 0x29, 0x24, 0x0c, 0x0e, 0x09, 0x4e, 0x78, + 0x3c, 0x09, 0x13, 0x05, 0x17, 0x11, 0x00}; +static u8 ili9341_ngamma[] = {0x00, 0x16, 0x1b, 0x04, 0x11, 0x07, 0x31, 0x33, + 0x42, 0x05, 0x0c, 0x0a, 0x28, 0x2f, 0x0f}; + +/** + * enum ili9341_input - the format of the incoming signal to the panel + * + * The panel can be connected to various input streams and four of them can + * be selected by electronic straps on the display. However it is possible + * to select another mode or override the electronic default with this + * setting. + */ +enum ili9341_input { + ILI9341_INPUT_PRGB_THROUGH = 0x0, + ILI9341_INPUT_PRGB_ALIGNED = 0x1, + ILI9341_INPUT_UNKNOWN = 0xf, +}; + +/** + * struct ili9341_config - the system specific ILI9341 configuration + * @width_mm: physical panel width [mm] + * @height_mm: physical panel height [mm] + * @input: the input/entry type used in this system, if this is set to + * ILI9341_INPUT_UNKNOWN the driver will try to figure it out by probing + * the hardware + * @dclk_active_high: data/pixel clock active high, data will be clocked + * in on the rising edge of the DCLK (this is usually the case). + * @de_active_high: DE (data entry) is active high + * @hsync_active_high: HSYNC is active high + * @vsync_active_high: VSYNC is active high + */ +struct ili9341_config { + u32 width_mm; + u32 height_mm; + enum ili9341_input input; + bool dclk_active_high; + bool de_active_high; + bool hsync_active_high; + bool vsync_active_high; +}; + +struct ili9341 { + struct device *dev; + const struct ili9341_config *conf; + struct drm_panel panel; + struct regmap *regmap; + struct gpio_desc *reset_gpio; + struct gpio_desc *dc_gpio; + enum ili9341_input input; + struct videomode vm; +}; + +static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel) +{ + return container_of(panel, struct ili9341, panel); +} + +int ili9341_spi_transfer(struct spi_device *spi, u32 speed_hz, + u8 bpw, const void *buf, size_t len) +{ + size_t max_chunk = spi_max_transfer_size(spi); + struct spi_transfer tr = { + .bits_per_word = bpw, + .speed_hz = speed_hz, + .len = len, + }; + struct spi_message m; + size_t chunk; + int ret; + + spi_message_init_with_transfers(&m, &tr, 1); + + while (len) { + chunk = min(len, max_chunk); + + tr.tx_buf = buf; + tr.len = chunk; + buf += chunk; + len -= chunk; + + ret = spi_sync(spi, &m); + if (ret) + return ret; + } + return 0; +} +static int ili9341_regmap_spi_write(void *context, const void *data, + size_t count) +{ + struct device *dev = context; + struct spi_device *spi = to_spi_device(dev); + struct ili9341 *ili = spi_get_drvdata(spi); + int ret = 0; + + gpiod_set_value_cansleep(ili->dc_gpio, 0); + + ret = ili9341_spi_transfer(spi, DEFAULT_SPI_SPEED, 8, data+0, 1); + if (ret || count == 1 || + ((u8 *)data)[0] == ILI9341_GRAM || + ((u8 *)data)[0] == ILI9341_DISPLAY_ON || + ((u8 *)data)[0] == ILI9341_SLEEP_OUT || + ((u8 *)data)[0] == ILI9341_DISPLAY_OFF) + return ret; + + gpiod_set_value_cansleep(ili->dc_gpio, 1); + + return ili9341_spi_transfer(spi, DEFAULT_SPI_SPEED, 8, data+1, count-1); +} + +static int ili9341_regmap_spi_read(void *context, const void *reg, + size_t reg_size, void *val, size_t val_size) +{ + return 0; +} + +static struct regmap_bus ili9341_regmap_bus = { + .write = ili9341_regmap_spi_write, + .read = ili9341_regmap_spi_read, + .reg_format_endian_default = REGMAP_ENDIAN_BIG, + .val_format_endian_default = REGMAP_ENDIAN_BIG, +}; + +static bool ili9341_volatile_reg(struct device *dev, unsigned int reg) +{ + return false; +} + +static bool ili9341_writeable_reg(struct device *dev, unsigned int reg) +{ + /* Just register 0 is read-only */ + if (reg == 0x00) + return false; + return true; +} + +static const struct regmap_config ili9341_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = 0xff, + .cache_type = REGCACHE_RBTREE, + .volatile_reg = ili9341_volatile_reg, + .writeable_reg = ili9341_writeable_reg, +}; + +static int ili9341_init(struct drm_panel *panel, struct ili9341 *ili) +{ + regmap_bulk_write(ili->regmap, 0xca, + ili9341_cmd0, sizeof(ili9341_cmd0)); + regmap_bulk_write(ili->regmap, ILI9341_POWERB, + ili9341_powerb, sizeof(ili9341_powerb)); + regmap_bulk_write(ili->regmap, ILI9341_POWER_SEQ, + ili9341_power_seq, sizeof(ili9341_power_seq)); + regmap_bulk_write(ili->regmap, ILI9341_DTCA, + ili9341_dtca, sizeof(ili9341_dtca)); + regmap_bulk_write(ili->regmap, ILI9341_POWERA, + ili9341_powera, sizeof(ili9341_powera)); + regmap_write(ili->regmap, ILI9341_PRC, ili9341_prc[0]); + regmap_bulk_write(ili->regmap, ILI9341_DTCB, + ili9341_dtcb, sizeof(ili9341_dtcb)); + regmap_bulk_write(ili->regmap, ILI9341_FRC, + ili9341_frc, sizeof(ili9341_frc)); + regmap_bulk_write(ili->regmap, ILI9341_DFC, + ili9341_dfc1, sizeof(ili9341_dfc1)); + regmap_write(ili->regmap, ILI9341_POWER1, ili9341_power1[0]); + regmap_write(ili->regmap, ILI9341_POWER2, ili9341_power2[0]); + regmap_bulk_write(ili->regmap, ILI9341_VCOM1, + ili9341_vcom1, sizeof(ili9341_vcom1)); + regmap_write(ili->regmap, ILI9341_VCOM2, ili9341_vcom2[0]); + regmap_write(ili->regmap, ILI9341_MAC, ili9341_mac[0]); + regmap_write(ili->regmap, ILI9341_3GAMMA_EN, ili9341_gamma_en[0]); + regmap_write(ili->regmap, ILI9341_RGB_INTERFACE, ili9341_rgb_intr[0]); + regmap_bulk_write(ili->regmap, ILI9341_DFC, + ili9341_dfc2, sizeof(ili9341_dfc2)); + + /* colomn address set */ + regmap_bulk_write(ili->regmap, ILI9341_COLUMN_ADDR, + ili9341_column_addr, sizeof(ili9341_column_addr)); + + /* Page Address Set */ + regmap_bulk_write(ili->regmap, ILI9341_PAGE_ADDR, + ili9341_page_addr, sizeof(ili9341_page_addr)); + regmap_bulk_write(ili->regmap, ILI9341_INTERFACE, + ili9341_intr, sizeof(ili9341_intr)); + regmap_write(ili->regmap, ILI9341_GRAM, 0); + msleep(200); + + regmap_write(ili->regmap, ILI9341_GAMMA, ili9341_gamma[0]); + regmap_bulk_write(ili->regmap, ILI9341_PGAMMA, + ili9341_pgamma, sizeof(ili9341_pgamma)); + regmap_bulk_write(ili->regmap, ILI9341_NGAMMA, + ili9341_ngamma, sizeof(ili9341_ngamma)); + regmap_write(ili->regmap, ILI9341_SLEEP_OUT, 0); + msleep(200); + + regmap_write(ili->regmap, ILI9341_DISPLAY_ON, 0); + + /* GRAM start writing */ + regmap_write(ili->regmap, ILI9341_GRAM, 0); + + dev_info(ili->dev, "initialized display\n"); + + return 0; +} + +/* + * This power-on sequence if from the datasheet, page 57. + */ +static int ili9341_power_on(struct ili9341 *ili) +{ + /* Assert RESET */ + gpiod_set_value(ili->reset_gpio, 1); + + msleep(20); + + /* De-assert RESET */ + gpiod_set_value(ili->reset_gpio, 0); + + msleep(10); + + return 0; +} + +static int ili9341_power_off(struct ili9341 *ili) +{ + return 0; +} + +static int ili9341_disable(struct drm_panel *panel) +{ + struct ili9341 *ili = panel_to_ili9341(panel); + int ret; + + ret = regmap_write(ili->regmap, ILI9341_DISPLAY_OFF, 0); + if (ret) { + dev_err(ili->dev, "unable to go to standby mode\n"); + return ret; + } + + return 0; +} + +static int ili9341_unprepare(struct drm_panel *panel) +{ + struct ili9341 *ili = panel_to_ili9341(panel); + + return ili9341_power_off(ili); +} + +static int ili9341_prepare(struct drm_panel *panel) +{ + struct ili9341 *ili = panel_to_ili9341(panel); + int ret; + + ret = ili9341_power_on(ili); + if (ret < 0) + return ret; + + ret = ili9341_init(panel, ili); + if (ret < 0) + ili9341_unprepare(panel); + + return ret; +} + +static int ili9341_enable(struct drm_panel *panel) +{ + struct ili9341 *ili = panel_to_ili9341(panel); + int ret; + + ret = regmap_write(ili->regmap, ILI9341_DISPLAY_ON, 0); + if (ret) { + dev_err(ili->dev, "unable to enable panel\n"); + return ret; + } + + return 0; +} + +/* This is the only mode listed for parallel RGB in the datasheet */ +static const struct drm_display_mode prgb_320x240_mode = { + .clock = 6100, + .hdisplay = 240, + .hsync_start = 240 + 10, + .hsync_end = 240 + 10 + 10, + .htotal = 280, + .vdisplay = 320, + .vsync_start = 320 + 4, + .vsync_end = 320 + 4 + 2, + .vtotal = 328, + .vrefresh = 60, + .flags = 0, +}; + +static int ili9341_get_modes(struct drm_panel *panel, + struct drm_connector *connector) +{ + struct ili9341 *ili = panel_to_ili9341(panel); + struct drm_device *drm = connector->dev; + struct drm_display_mode *mode; + struct drm_display_info *info; + + info = &connector->display_info; + info->width_mm = ili->conf->width_mm; + info->height_mm = ili->conf->height_mm; + if (ili->conf->dclk_active_high) + info->bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE; + else + info->bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE; + + if (ili->conf->de_active_high) + info->bus_flags |= DRM_BUS_FLAG_DE_HIGH; + else + info->bus_flags |= DRM_BUS_FLAG_DE_LOW; + + switch (ili->input) { + case ILI9341_INPUT_PRGB_THROUGH: + case ILI9341_INPUT_PRGB_ALIGNED: + mode = drm_mode_duplicate(drm, &prgb_320x240_mode); + break; + default: + mode = NULL; + break; + } + if (!mode) { + DRM_ERROR("bad mode or failed to add mode\n"); + return -EINVAL; + } + drm_mode_set_name(mode); + /* + * This is the preferred mode because most people are going + * to want to use the display with VGA type graphics. + */ + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; + + /* Set up the polarity */ + if (ili->conf->hsync_active_high) + mode->flags |= DRM_MODE_FLAG_PHSYNC; + else + mode->flags |= DRM_MODE_FLAG_NHSYNC; + if (ili->conf->vsync_active_high) + mode->flags |= DRM_MODE_FLAG_PVSYNC; + else + mode->flags |= DRM_MODE_FLAG_NVSYNC; + + mode->width_mm = ili->conf->width_mm; + mode->height_mm = ili->conf->height_mm; + drm_mode_probed_add(connector, mode); + + return 1; /* Number of modes */ +} + +static const struct drm_panel_funcs ili9341_drm_funcs = { + .disable = ili9341_disable, + .unprepare = ili9341_unprepare, + .prepare = ili9341_prepare, + .enable = ili9341_enable, + .get_modes = ili9341_get_modes, +}; + +static int ili9341_probe(struct spi_device *spi) +{ + struct device *dev = &spi->dev; + struct ili9341 *ili; + const struct regmap_config *regmap_config; + int ret; + + ili = devm_kzalloc(dev, sizeof(struct ili9341), GFP_KERNEL); + if (!ili) + return -ENOMEM; + + spi_set_drvdata(spi, ili); + + ili->dev = dev; + /* + * Every new incarnation of this display must have a unique + * data entry for the system in this driver. + */ + ili->conf = of_device_get_match_data(dev); + if (!ili->conf) { + dev_err(dev, "missing device configuration\n"); + return -ENODEV; + } + + ili->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(ili->reset_gpio)) { + dev_err(dev, "failed to get RESET GPIO\n"); + return PTR_ERR(ili->reset_gpio); + } + + ili->dc_gpio = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); + if (IS_ERR(ili->dc_gpio)) { + dev_err(dev, "failed to get DC GPIO\n"); + return PTR_ERR(ili->dc_gpio); + } + + spi->bits_per_word = 8; + ret = spi_setup(spi); + if (ret < 0) { + dev_err(dev, "spi setup failed.\n"); + return ret; + } + + regmap_config = &ili9341_regmap_config; + + ili->regmap = devm_regmap_init(dev, &ili9341_regmap_bus, dev, + regmap_config); + if (IS_ERR(ili->regmap)) { + dev_err(dev, "failed to allocate register map\n"); + return PTR_ERR(ili->regmap); + } + + ili->input = ili->conf->input; + + drm_panel_init(&ili->panel, dev, &ili9341_drm_funcs, + DRM_MODE_CONNECTOR_DPI); + + return drm_panel_add(&ili->panel); +} + +static int ili9341_remove(struct spi_device *spi) +{ + struct ili9341 *ili = spi_get_drvdata(spi); + + ili9341_power_off(ili); + drm_panel_remove(&ili->panel); + + return 0; +} + +/* + * The Stm32f429-disco board has a panel ili9341 connected to ltdc controller + */ +static const struct ili9341_config ili9341_data = { + .width_mm = 65, + .height_mm = 50, + .input = ILI9341_INPUT_PRGB_THROUGH, + .dclk_active_high = true, + .de_active_high = false, + .hsync_active_high = false, + .vsync_active_high = false, +}; + +static const struct of_device_id ili9341_of_match[] = { + { + .compatible = "stm32f429,ltdc-panel", + .data = &ili9341_data, + }, + { + .compatible = "ilitek,ili9341", + .data = NULL, + }, + { } +}; +MODULE_DEVICE_TABLE(of, ili9341_of_match); + +static struct spi_driver ili9341_driver = { + .probe = ili9341_probe, + .remove = ili9341_remove, + .driver = { + .name = "panel-ilitek-ili9341", + .of_match_table = ili9341_of_match, + }, +}; +module_spi_driver(ili9341_driver); + +MODULE_AUTHOR("Dillon Min <dillon.minfei@gmail.com>"); +MODULE_DESCRIPTION("ILI9341 LCD panel driver"); +MODULE_LICENSE("GPL v2"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 5/5] drm/panel: add panel driver for Ilitek ili9341 panels 2020-05-08 4:13 ` [PATCH v2 5/5] drm/panel: add panel driver for Ilitek ili9341 panels dillon.minfei @ 2020-05-08 9:02 ` Sam Ravnborg [not found] ` <CAL9mu0LcT3pPiM6PdJRvAJRMbXY6KACF1wNg8yOS-6D8aq6AeA@mail.gmail.com> 0 siblings, 1 reply; 10+ messages in thread From: Sam Ravnborg @ 2020-05-08 9:02 UTC (permalink / raw) To: dillon.minfei Cc: robh+dt, mcoquelin.stm32, alexandre.torgue, thierry.reding, airlied, daniel, mturquette, sboyd, devicetree, linux-stm32, linux-arm-kernel, linux-kernel, dri-devel, linux-clk Hi Dillon. Patch submissions starts to look fine. On Fri, May 08, 2020 at 12:13:14PM +0800, dillon.minfei@gmail.com wrote: > From: dillon min <dillon.minfei@gmail.com> > > This is a driver for 320x240 TFT panels, accepting a rgb input > streams that get adapted and scaled to the panel. This driver is, I suppose, prepared to be a driver for ILI9341 based panles, and as such not for a fixed resolution. I expect (hope) we in the future will see more panels added. Some things to fix, see comments in the follwoing. Sam > > Signed-off-by: dillon min <dillon.minfei@gmail.com> > --- > drivers/gpu/drm/panel/Kconfig | 8 + > drivers/gpu/drm/panel/Makefile | 1 + > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 561 +++++++++++++++++++++++++++ > 3 files changed, 570 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig > index a1723c1..e42692c 100644 > --- a/drivers/gpu/drm/panel/Kconfig > +++ b/drivers/gpu/drm/panel/Kconfig > @@ -95,6 +95,14 @@ config DRM_PANEL_ILITEK_IL9322 > Say Y here if you want to enable support for Ilitek IL9322 > QVGA (320x240) RGB, YUV and ITU-T BT.656 panels. > > +config DRM_PANEL_ILITEK_IL9341 ILI9341 - so the config name matches the name of the driver IC. > + tristate "Ilitek ILI9341 240x320 QVGA panels" > + depends on OF && SPI > + select REGMAP > + help > + Say Y here if you want to enable support for Ilitek IL9341 > + QVGA (240x320) RGB panels. See comment to the changelog, the driver is more generic - I assume. So the wording here can be improved to express this. > + > config DRM_PANEL_ILITEK_ILI9881C > tristate "Ilitek ILI9881C-based panels" > depends on OF > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile > index 96a883c..d123543 100644 > --- a/drivers/gpu/drm/panel/Makefile > +++ b/drivers/gpu/drm/panel/Makefile > @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) += panel-elida-kd35t133.o > obj-$(CONFIG_DRM_PANEL_FEIXIN_K101_IM2BA02) += panel-feixin-k101-im2ba02.o > obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d.o > obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o > +obj-$(CONFIG_DRM_PANEL_ILITEK_IL9341) += panel-ilitek-ili9341.o > obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o > obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o > obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > new file mode 100644 > index 0000000..ec22d80 > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > @@ -0,0 +1,561 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Ilitek ILI9341 TFT LCD drm_panel driver. > + * > + * This panel can be configured to support: > + * - 16-bit parallel RGB interface The interface to ILI9341 is SPI, and the interface between the ILI9341 and the panel is more of an itnernal thing. Or did I get this worng? > + * > + * Copyright (C) 2020 Dillon Min <dillon.minfei@gmail.com> > + * Derived from drivers/drm/gpu/panel/panel-ilitek-ili9322.c > + */ > + > +#include <linux/bitops.h> > +#include <linux/gpio/consumer.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/regmap.h> > +#include <linux/regulator/consumer.h> > +#include <linux/spi/spi.h> > + > +#include <video/mipi_display.h> > +#include <video/of_videomode.h> > +#include <video/videomode.h> > + > +#include <drm/drm_modes.h> > +#include <drm/drm_panel.h> > +#include <drm/drm_print.h> > + > +#define DEFAULT_SPI_SPEED 10000000 > + Please use same case for hex numbers in the driver. My personal preferences is lower-case. > +#define ILI9341_SLEEP_OUT 0x11 /* Sleep out register */ > +#define ILI9341_GAMMA 0x26 /* Gamma register */ > +#define ILI9341_DISPLAY_OFF 0x28 /* Display off register */ > +#define ILI9341_DISPLAY_ON 0x29 /* Display on register */ > +#define ILI9341_COLUMN_ADDR 0x2A /* Colomn address register */ > +#define ILI9341_PAGE_ADDR 0x2B /* Page address register */ > +#define ILI9341_GRAM 0x2C /* GRAM register */ > +#define ILI9341_MAC 0x36 /* Memory Access Control register*/ > +#define ILI9341_PIXEL_FORMAT 0x3A /* Pixel Format register */ > +#define ILI9341_WDB 0x51 /* Write Brightness Display > + * register > + */ > +#define ILI9341_WCD 0x53 /* Write Control Display > + * register > + */ > +#define ILI9341_RGB_INTERFACE 0xB0 /* RGB Interface Signal Control */ > +#define ILI9341_FRC 0xB1 /* Frame Rate Control register */ > +#define ILI9341_BPC 0xB5 /* Blanking Porch Control > + * register > + */ > +#define ILI9341_DFC 0xB6 /* Display Function Control > + * register > + */ > +#define ILI9341_POWER1 0xC0 /* Power Control 1 register */ > +#define ILI9341_POWER2 0xC1 /* Power Control 2 register */ > +#define ILI9341_VCOM1 0xC5 /* VCOM Control 1 register */ > +#define ILI9341_VCOM2 0xC7 /* VCOM Control 2 register */ > +#define ILI9341_POWERA 0xCB /* Power control A register */ > +#define ILI9341_POWERB 0xCF /* Power control B register */ > +#define ILI9341_PGAMMA 0xE0 /* Positive Gamma Correction > + * register > + */ > +#define ILI9341_NGAMMA 0xE1 /* Negative Gamma Correction > + * register > + */ > +#define ILI9341_DTCA 0xE8 /* Driver timing control A */ > +#define ILI9341_DTCB 0xEA /* Driver timing control B */ > +#define ILI9341_POWER_SEQ 0xED /* Power on sequence register */ > +#define ILI9341_3GAMMA_EN 0xF2 /* 3 Gamma enable register */ > +#define ILI9341_INTERFACE 0xF6 /* Interface control register */ > +#define ILI9341_PRC 0xF7 /* Pump ratio control register */ > + All the following should be const. Can any of the below be replaces by DEFINED constants? > +static u8 ili9341_cmd0[] = {0xc3, 0x08, 0x50}; > +static u8 ili9341_powerb[] = {0x00, 0xc1, 0x30}; > +static u8 ili9341_power_seq[] = {0x64, 0x03, 0x12, 0x81}; > +static u8 ili9341_dtca[] = {0x85, 0x00, 0x78}; > +static u8 ili9341_powera[] = {0x39, 0x2c, 0x00, 0x34, 0x02}; > +static u8 ili9341_prc[] = {0x20}; > +static u8 ili9341_dtcb[] = {0x00, 0x00}; > +static u8 ili9341_frc[] = {0x00, 0x1b}; > +static u8 ili9341_dfc1[] = {0x0a, 0xa2}; > +static u8 ili9341_power1[] = {0x10}; > +static u8 ili9341_power2[] = {0x10}; > +static u8 ili9341_vcom1[] = {0x45, 0x15}; > +static u8 ili9341_vcom2[] = {0x90}; > +static u8 ili9341_mac[] = {0xc8}; > +static u8 ili9341_gamma_en[] = {0x00}; > +static u8 ili9341_rgb_intr[] = {0xc2}; > +static u8 ili9341_dfc2[] = {0x0a, 0xa7, 0x27, 0x04}; > +static u8 ili9341_column_addr[] = {0x00, 0x00, 0x00, 0xef}; > +static u8 ili9341_page_addr[] = {0x00, 0x00, 0x01, 0x3f}; > +static u8 ili9341_intr[] = {0x01, 0x00, 0x06}; > +static u8 ili9341_gamma[] = {0x01}; > +static u8 ili9341_pgamma[] = {0x0f, 0x29, 0x24, 0x0c, 0x0e, 0x09, 0x4e, 0x78, > + 0x3c, 0x09, 0x13, 0x05, 0x17, 0x11, 0x00}; > +static u8 ili9341_ngamma[] = {0x00, 0x16, 0x1b, 0x04, 0x11, 0x07, 0x31, 0x33, > + 0x42, 0x05, 0x0c, 0x0a, 0x28, 0x2f, 0x0f}; > + > +/** > + * enum ili9341_input - the format of the incoming signal to the panel > + * > + * The panel can be connected to various input streams and four of them can > + * be selected by electronic straps on the display. However it is possible > + * to select another mode or override the electronic default with this > + * setting. > + */ > +enum ili9341_input { > + ILI9341_INPUT_PRGB_THROUGH = 0x0, > + ILI9341_INPUT_PRGB_ALIGNED = 0x1, > + ILI9341_INPUT_UNKNOWN = 0xf, > +}; > + > +/** > + * struct ili9341_config - the system specific ILI9341 configuration > + * @width_mm: physical panel width [mm] > + * @height_mm: physical panel height [mm] > + * @input: the input/entry type used in this system, if this is set to > + * ILI9341_INPUT_UNKNOWN the driver will try to figure it out by probing > + * the hardware > + * @dclk_active_high: data/pixel clock active high, data will be clocked > + * in on the rising edge of the DCLK (this is usually the case). > + * @de_active_high: DE (data entry) is active high > + * @hsync_active_high: HSYNC is active high > + * @vsync_active_high: VSYNC is active high > + */ > +struct ili9341_config { > + u32 width_mm; > + u32 height_mm; > + enum ili9341_input input; > + bool dclk_active_high; > + bool de_active_high; > + bool hsync_active_high; > + bool vsync_active_high; > +}; > + > +struct ili9341 { > + struct device *dev; > + const struct ili9341_config *conf; > + struct drm_panel panel; > + struct regmap *regmap; > + struct gpio_desc *reset_gpio; > + struct gpio_desc *dc_gpio; > + enum ili9341_input input; > + struct videomode vm; videomode is not used. So drop this field and drop the include files that are no logner needed. > +}; > + > +static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel) > +{ > + return container_of(panel, struct ili9341, panel); > +} > + > +int ili9341_spi_transfer(struct spi_device *spi, u32 speed_hz, > + u8 bpw, const void *buf, size_t len) > +{ > + size_t max_chunk = spi_max_transfer_size(spi); > + struct spi_transfer tr = { const? > + .bits_per_word = bpw, > + .speed_hz = speed_hz, > + .len = len, > + }; > + struct spi_message m; > + size_t chunk; > + int ret; > + > + spi_message_init_with_transfers(&m, &tr, 1); > + > + while (len) { > + chunk = min(len, max_chunk); > + > + tr.tx_buf = buf; > + tr.len = chunk; > + buf += chunk; > + len -= chunk; > + > + ret = spi_sync(spi, &m); > + if (ret) > + return ret; > + } > + return 0; > +} > +static int ili9341_regmap_spi_write(void *context, const void *data, > + size_t count) > +{ > + struct device *dev = context; > + struct spi_device *spi = to_spi_device(dev); > + struct ili9341 *ili = spi_get_drvdata(spi); > + int ret = 0; > + > + gpiod_set_value_cansleep(ili->dc_gpio, 0); > + > + ret = ili9341_spi_transfer(spi, DEFAULT_SPI_SPEED, 8, data+0, 1); > + if (ret || count == 1 || > + ((u8 *)data)[0] == ILI9341_GRAM || > + ((u8 *)data)[0] == ILI9341_DISPLAY_ON || > + ((u8 *)data)[0] == ILI9341_SLEEP_OUT || > + ((u8 *)data)[0] == ILI9341_DISPLAY_OFF) > + return ret; > + > + gpiod_set_value_cansleep(ili->dc_gpio, 1); > + > + return ili9341_spi_transfer(spi, DEFAULT_SPI_SPEED, 8, data+1, count-1); > +} > + > +static int ili9341_regmap_spi_read(void *context, const void *reg, > + size_t reg_size, void *val, size_t val_size) > +{ > + return 0; > +} Is this function really needed? If not delete it. > + > +static struct regmap_bus ili9341_regmap_bus = { > + .write = ili9341_regmap_spi_write, > + .read = ili9341_regmap_spi_read, > + .reg_format_endian_default = REGMAP_ENDIAN_BIG, > + .val_format_endian_default = REGMAP_ENDIAN_BIG, > +}; > + > +static bool ili9341_volatile_reg(struct device *dev, unsigned int reg) > +{ > + return false; > +} Is this function really nedded? If not delete it. > + > +static bool ili9341_writeable_reg(struct device *dev, unsigned int reg) > +{ > + /* Just register 0 is read-only */ > + if (reg == 0x00) > + return false; > + return true; > +} > + > +static const struct regmap_config ili9341_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = 0xff, > + .cache_type = REGCACHE_RBTREE, > + .volatile_reg = ili9341_volatile_reg, > + .writeable_reg = ili9341_writeable_reg, > +}; > + No error checks - consider something like: static int bulk_write(struct ili9341 *ili, u8 reg, const u8[] data, int len) { int err = ili->err; if (!err) { err = regmap_bulk_write(ili->regmap, reg, data, len); if (err) { dev_err(...); ili->err = err; } } return err; } Then you can use this in the following, and make this more readable. > +static int ili9341_init(struct drm_panel *panel, struct ili9341 *ili) > +{ > + regmap_bulk_write(ili->regmap, 0xca, > + ili9341_cmd0, sizeof(ili9341_cmd0)); > + regmap_bulk_write(ili->regmap, ILI9341_POWERB, > + ili9341_powerb, sizeof(ili9341_powerb)); > + regmap_bulk_write(ili->regmap, ILI9341_POWER_SEQ, > + ili9341_power_seq, sizeof(ili9341_power_seq)); > + regmap_bulk_write(ili->regmap, ILI9341_DTCA, > + ili9341_dtca, sizeof(ili9341_dtca)); > + regmap_bulk_write(ili->regmap, ILI9341_POWERA, > + ili9341_powera, sizeof(ili9341_powera)); > + regmap_write(ili->regmap, ILI9341_PRC, ili9341_prc[0]); > + regmap_bulk_write(ili->regmap, ILI9341_DTCB, > + ili9341_dtcb, sizeof(ili9341_dtcb)); > + regmap_bulk_write(ili->regmap, ILI9341_FRC, > + ili9341_frc, sizeof(ili9341_frc)); > + regmap_bulk_write(ili->regmap, ILI9341_DFC, > + ili9341_dfc1, sizeof(ili9341_dfc1)); > + regmap_write(ili->regmap, ILI9341_POWER1, ili9341_power1[0]); > + regmap_write(ili->regmap, ILI9341_POWER2, ili9341_power2[0]); > + regmap_bulk_write(ili->regmap, ILI9341_VCOM1, > + ili9341_vcom1, sizeof(ili9341_vcom1)); > + regmap_write(ili->regmap, ILI9341_VCOM2, ili9341_vcom2[0]); > + regmap_write(ili->regmap, ILI9341_MAC, ili9341_mac[0]); > + regmap_write(ili->regmap, ILI9341_3GAMMA_EN, ili9341_gamma_en[0]); > + regmap_write(ili->regmap, ILI9341_RGB_INTERFACE, ili9341_rgb_intr[0]); > + regmap_bulk_write(ili->regmap, ILI9341_DFC, > + ili9341_dfc2, sizeof(ili9341_dfc2)); > + > + /* colomn address set */ > + regmap_bulk_write(ili->regmap, ILI9341_COLUMN_ADDR, > + ili9341_column_addr, sizeof(ili9341_column_addr)); > + > + /* Page Address Set */ > + regmap_bulk_write(ili->regmap, ILI9341_PAGE_ADDR, > + ili9341_page_addr, sizeof(ili9341_page_addr)); > + regmap_bulk_write(ili->regmap, ILI9341_INTERFACE, > + ili9341_intr, sizeof(ili9341_intr)); > + regmap_write(ili->regmap, ILI9341_GRAM, 0); > + msleep(200); > + > + regmap_write(ili->regmap, ILI9341_GAMMA, ili9341_gamma[0]); > + regmap_bulk_write(ili->regmap, ILI9341_PGAMMA, > + ili9341_pgamma, sizeof(ili9341_pgamma)); > + regmap_bulk_write(ili->regmap, ILI9341_NGAMMA, > + ili9341_ngamma, sizeof(ili9341_ngamma)); > + regmap_write(ili->regmap, ILI9341_SLEEP_OUT, 0); > + msleep(200); > + > + regmap_write(ili->regmap, ILI9341_DISPLAY_ON, 0); > + > + /* GRAM start writing */ > + regmap_write(ili->regmap, ILI9341_GRAM, 0); > + > + dev_info(ili->dev, "initialized display\n"); > + > + return 0; > +} > + > +/* > + * This power-on sequence if from the datasheet, page 57. > + */ > +static int ili9341_power_on(struct ili9341 *ili) > +{ > + /* Assert RESET */ > + gpiod_set_value(ili->reset_gpio, 1); > + > + msleep(20); > + > + /* De-assert RESET */ > + gpiod_set_value(ili->reset_gpio, 0); > + > + msleep(10); > + > + return 0; > +} > + > +static int ili9341_power_off(struct ili9341 *ili) > +{ Assert reset? > + return 0; > +} > + > +static int ili9341_disable(struct drm_panel *panel) > +{ > + struct ili9341 *ili = panel_to_ili9341(panel); > + int ret; > + > + ret = regmap_write(ili->regmap, ILI9341_DISPLAY_OFF, 0); > + if (ret) { > + dev_err(ili->dev, "unable to go to standby mode\n"); > + return ret; > + } > + > + return 0; > +} > + > +static int ili9341_unprepare(struct drm_panel *panel) > +{ > + struct ili9341 *ili = panel_to_ili9341(panel); > + > + return ili9341_power_off(ili); > +} > + > +static int ili9341_prepare(struct drm_panel *panel) > +{ > + struct ili9341 *ili = panel_to_ili9341(panel); > + int ret; > + > + ret = ili9341_power_on(ili); > + if (ret < 0) > + return ret; > + > + ret = ili9341_init(panel, ili); > + if (ret < 0) > + ili9341_unprepare(panel); > + > + return ret; > +} > + > +static int ili9341_enable(struct drm_panel *panel) > +{ > + struct ili9341 *ili = panel_to_ili9341(panel); > + int ret; > + > + ret = regmap_write(ili->regmap, ILI9341_DISPLAY_ON, 0); > + if (ret) { > + dev_err(ili->dev, "unable to enable panel\n"); > + return ret; > + } > + > + return 0; > +} > + > +/* This is the only mode listed for parallel RGB in the datasheet */ > +static const struct drm_display_mode prgb_320x240_mode = { > + .clock = 6100, > + .hdisplay = 240, > + .hsync_start = 240 + 10, > + .hsync_end = 240 + 10 + 10, > + .htotal = 280, > + .vdisplay = 320, > + .vsync_start = 320 + 4, > + .vsync_end = 320 + 4 + 2, > + .vtotal = 328, > + .vrefresh = 60, > + .flags = 0, > +}; > + > +static int ili9341_get_modes(struct drm_panel *panel, > + struct drm_connector *connector) > +{ > + struct ili9341 *ili = panel_to_ili9341(panel); > + struct drm_device *drm = connector->dev; > + struct drm_display_mode *mode; > + struct drm_display_info *info; > + > + info = &connector->display_info; > + info->width_mm = ili->conf->width_mm; > + info->height_mm = ili->conf->height_mm; > + if (ili->conf->dclk_active_high) > + info->bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE; > + else > + info->bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE; > + > + if (ili->conf->de_active_high) > + info->bus_flags |= DRM_BUS_FLAG_DE_HIGH; > + else > + info->bus_flags |= DRM_BUS_FLAG_DE_LOW; > + > + switch (ili->input) { > + case ILI9341_INPUT_PRGB_THROUGH: > + case ILI9341_INPUT_PRGB_ALIGNED: > + mode = drm_mode_duplicate(drm, &prgb_320x240_mode); > + break; > + default: > + mode = NULL; > + break; > + } > + if (!mode) { > + DRM_ERROR("bad mode or failed to add mode\n"); > + return -EINVAL; > + } > + drm_mode_set_name(mode); > + /* > + * This is the preferred mode because most people are going > + * to want to use the display with VGA type graphics. > + */ > + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > + > + /* Set up the polarity */ > + if (ili->conf->hsync_active_high) > + mode->flags |= DRM_MODE_FLAG_PHSYNC; > + else > + mode->flags |= DRM_MODE_FLAG_NHSYNC; > + if (ili->conf->vsync_active_high) > + mode->flags |= DRM_MODE_FLAG_PVSYNC; > + else > + mode->flags |= DRM_MODE_FLAG_NVSYNC; > + > + mode->width_mm = ili->conf->width_mm; > + mode->height_mm = ili->conf->height_mm; > + drm_mode_probed_add(connector, mode); > + > + return 1; /* Number of modes */ > +} > + > +static const struct drm_panel_funcs ili9341_drm_funcs = { > + .disable = ili9341_disable, > + .unprepare = ili9341_unprepare, > + .prepare = ili9341_prepare, > + .enable = ili9341_enable, > + .get_modes = ili9341_get_modes, > +}; > + > +static int ili9341_probe(struct spi_device *spi) > +{ > + struct device *dev = &spi->dev; > + struct ili9341 *ili; > + const struct regmap_config *regmap_config; > + int ret; > + > + ili = devm_kzalloc(dev, sizeof(struct ili9341), GFP_KERNEL); > + if (!ili) > + return -ENOMEM; > + > + spi_set_drvdata(spi, ili); > + > + ili->dev = dev; > + /* > + * Every new incarnation of this display must have a unique > + * data entry for the system in this driver. > + */ > + ili->conf = of_device_get_match_data(dev); > + if (!ili->conf) { > + dev_err(dev, "missing device configuration\n"); > + return -ENODEV; > + } > + > + ili->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(ili->reset_gpio)) { > + dev_err(dev, "failed to get RESET GPIO\n"); > + return PTR_ERR(ili->reset_gpio); > + } > + > + ili->dc_gpio = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); > + if (IS_ERR(ili->dc_gpio)) { > + dev_err(dev, "failed to get DC GPIO\n"); > + return PTR_ERR(ili->dc_gpio); > + } > + > + spi->bits_per_word = 8; > + ret = spi_setup(spi); > + if (ret < 0) { > + dev_err(dev, "spi setup failed.\n"); > + return ret; > + } > + > + regmap_config = &ili9341_regmap_config; > + > + ili->regmap = devm_regmap_init(dev, &ili9341_regmap_bus, dev, > + regmap_config); > + if (IS_ERR(ili->regmap)) { > + dev_err(dev, "failed to allocate register map\n"); > + return PTR_ERR(ili->regmap); > + } > + > + ili->input = ili->conf->input; > + > + drm_panel_init(&ili->panel, dev, &ili9341_drm_funcs, > + DRM_MODE_CONNECTOR_DPI); > + > + return drm_panel_add(&ili->panel); > +} > + > +static int ili9341_remove(struct spi_device *spi) > +{ > + struct ili9341 *ili = spi_get_drvdata(spi); > + > + ili9341_power_off(ili); > + drm_panel_remove(&ili->panel); > + > + return 0; > +} > + > +/* > + * The Stm32f429-disco board has a panel ili9341 connected to ltdc controller > + */ > +static const struct ili9341_config ili9341_data = { This should be named "disco" something as this is m32f429-disco specific. > + .width_mm = 65, > + .height_mm = 50, > + .input = ILI9341_INPUT_PRGB_THROUGH, > + .dclk_active_high = true, > + .de_active_high = false, > + .hsync_active_high = false, > + .vsync_active_high = false, > +}; > + > +static const struct of_device_id ili9341_of_match[] = { > + { > + .compatible = "stm32f429,ltdc-panel", > + .data = &ili9341_data, > + }, > + { > + .compatible = "ilitek,ili9341", > + .data = NULL, This part is wrong, as ilitek,ili9341 is just the generic part. Only the first entry is relevant. > + }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ili9341_of_match); > + > +static struct spi_driver ili9341_driver = { > + .probe = ili9341_probe, > + .remove = ili9341_remove, > + .driver = { > + .name = "panel-ilitek-ili9341", > + .of_match_table = ili9341_of_match, > + }, > +}; > +module_spi_driver(ili9341_driver); > + > +MODULE_AUTHOR("Dillon Min <dillon.minfei@gmail.com>"); > +MODULE_DESCRIPTION("ILI9341 LCD panel driver"); > +MODULE_LICENSE("GPL v2"); > -- > 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CAL9mu0LcT3pPiM6PdJRvAJRMbXY6KACF1wNg8yOS-6D8aq6AeA@mail.gmail.com>]
* Re: [PATCH v2 5/5] drm/panel: add panel driver for Ilitek ili9341 panels [not found] ` <CAL9mu0LcT3pPiM6PdJRvAJRMbXY6KACF1wNg8yOS-6D8aq6AeA@mail.gmail.com> @ 2020-05-09 8:31 ` dillon min 2020-05-09 20:06 ` Sam Ravnborg 1 sibling, 0 replies; 10+ messages in thread From: dillon min @ 2020-05-09 8:31 UTC (permalink / raw) To: Sam Ravnborg Cc: robh+dt, mcoquelin.stm32, Alexandre Torgue, thierry.reding, airlied, Daniel Vetter, Michael Turquette, sboyd, devicetree, linux-stm32, linux-arm-kernel, linux-kernel, dri-devel, linux-clk Hi sam, all, i'm not sure you had receive this mail , as gmail's html encoded messages blocked by linux arm kernel mailing list. so, resent it in plain text again. with my name "dillon: " at left sorry for trouble. dillon best regards dillon min <dillon.minfei@gmail.com> 于2020年5月8日周五 下午6:13写道： > Hi Sam, Thanks for your comments, i will rework this panel driver after l3gd20 patch submission. > Sam Ravnborg <sam@ravnborg.org> 于2020年5月8日周五 下午5:02写道： >> >> Hi Dillon. >> >> Patch submissions starts to look fine. >> >> On Fri, May 08, 2020 at 12:13:14PM +0800, dillon.minfei@gmail.com wrote: >> > From: dillon min <dillon.minfei@gmail.com> >> > >> > This is a driver for 320x240 TFT panels, accepting a rgb input >> > streams that get adapted and scaled to the panel. > This driver is, I suppose, prepared to be a driver for ILI9341 based > panles, and as such not for a fixed resolution. > I expect (hope) we in the future will see more panels added. > dillon: As i checked ili9341 datasheets, this panel just support 240x320 resolution only. if i'm wrong , please correct me. thanks https://cdn-shop.adafruit.com/datasheets/ILI9341.pdf This panel can support 9 different kinds of interface , "3/4-line Serial Interface" have been supported by tiny/ili9341.c. i was verified it but the performance on stm32f4 is very low. currently, i just have stm32f429-disco in hands, with 18-bit parallel rgb bus connected on this board. reference to panel-ilitek-ili9322 and panel-ilitek-ili9881 driver, i have some plan to rewrite this driver. 1 add your below comments in. 2 use dc-gpio, reset-gpio, rgb-interface-bits from dts to config panel interface. 3 drop regmap, porting drm_mipi_dbi's mipi_dbi_command to config panel paramter. like tiny/ili9341.c 4 support rgb-16, rgb-18 interface. 5 use optional regulator or power gpio to control panel's power, as panel power is always on for my board, so i can't test this part. could i add the code which can't be tested? 6 support rotation in panel config (currently , i rotate the screen by kernel cmdline paramter) if you have any other common panel configuration should be add , please inform me. thanks. >> >> >> Some things to fix, see comments in the follwoing. >> >> Sam >> >> > >> > Signed-off-by: dillon min <dillon.minfei@gmail.com> >> > --- >> > drivers/gpu/drm/panel/Kconfig | 8 + >> > drivers/gpu/drm/panel/Makefile | 1 + >> > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 561 +++++++++++++++++++++++++++ >> > 3 files changed, 570 insertions(+) >> > create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c >> > >> > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig >> > index a1723c1..e42692c 100644 >> > --- a/drivers/gpu/drm/panel/Kconfig >> > +++ b/drivers/gpu/drm/panel/Kconfig >> > @@ -95,6 +95,14 @@ config DRM_PANEL_ILITEK_IL9322 >> > Say Y here if you want to enable support for Ilitek IL9322 >> > QVGA (320x240) RGB, YUV and ITU-T BT.656 panels. >> > >> > +config DRM_PANEL_ILITEK_IL9341 >> ILI9341 - so the config name matches the name of the driver IC. >> >> > + tristate "Ilitek ILI9341 240x320 QVGA panels" >> > + depends on OF && SPI >> > + select REGMAP >> > + help >> > + Say Y here if you want to enable support for Ilitek IL9341 >> > + QVGA (240x320) RGB panels. >> See comment to the changelog, the driver is more generic - I assume. >> So the wording here can be improved to express this. >> dillon: Add support RGB 16bits and RGB 18bits bus only ? >> >> > + >> > config DRM_PANEL_ILITEK_ILI9881C >> > tristate "Ilitek ILI9881C-based panels" >> > depends on OF >> > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile >> > index 96a883c..d123543 100644 >> > --- a/drivers/gpu/drm/panel/Makefile >> > +++ b/drivers/gpu/drm/panel/Makefile >> > @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) += panel-elida-kd35t133.o >> > obj-$(CONFIG_DRM_PANEL_FEIXIN_K101_IM2BA02) += panel-feixin-k101-im2ba02.o >> > obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d.o >> > obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o >> > +obj-$(CONFIG_DRM_PANEL_ILITEK_IL9341) += panel-ilitek-ili9341.o >> > obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o >> > obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o >> > obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o >> > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c >> > new file mode 100644 >> > index 0000000..ec22d80 >> > --- /dev/null >> > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c >> > @@ -0,0 +1,561 @@ >> > +// SPDX-License-Identifier: GPL-2.0-only >> > +/* >> > + * Ilitek ILI9341 TFT LCD drm_panel driver. >> > + * >> > + * This panel can be configured to support: >> > + * - 16-bit parallel RGB interface >> The interface to ILI9341 is SPI, and the interface between the ILI9341 >> and the panel is more of an itnernal thing. Or did I get this worng? >> dillon: SPI is for register configuration. RGB parallel for data transfer > >> > + * >> > + * Copyright (C) 2020 Dillon Min <dillon.minfei@gmail.com> >> > + * Derived from drivers/drm/gpu/panel/panel-ilitek-ili9322.c >> > + */ >> > + >> > +#include <linux/bitops.h> >> > +#include <linux/gpio/consumer.h> >> > +#include <linux/module.h> >> > +#include <linux/of_device.h> >> > +#include <linux/regmap.h> >> > +#include <linux/regulator/consumer.h> >> > +#include <linux/spi/spi.h> >> > + >> > +#include <video/mipi_display.h> >> > +#include <video/of_videomode.h> >> > +#include <video/videomode.h> >> > + >> > +#include <drm/drm_modes.h> >> > +#include <drm/drm_panel.h> >> > +#include <drm/drm_print.h> >> > + >> > +#define DEFAULT_SPI_SPEED 10000000 >> > + >> >> Please use same case for hex numbers in the driver. >> My personal preferences is lower-case. >> dillon: in next patch, spi speed will be configured by dts, not hardcode here. below register address will change to lower-case hex numbers. > >> >> > +#define ILI9341_SLEEP_OUT 0x11 /* Sleep out register */ >> > +#define ILI9341_GAMMA 0x26 /* Gamma register */ >> > +#define ILI9341_DISPLAY_OFF 0x28 /* Display off register */ >> > +#define ILI9341_DISPLAY_ON 0x29 /* Display on register */ >> > +#define ILI9341_COLUMN_ADDR 0x2A /* Colomn address register */ >> > +#define ILI9341_PAGE_ADDR 0x2B /* Page address register */ >> > +#define ILI9341_GRAM 0x2C /* GRAM register */ >> > +#define ILI9341_MAC 0x36 /* Memory Access Control register*/ >> > +#define ILI9341_PIXEL_FORMAT 0x3A /* Pixel Format register */ >> > +#define ILI9341_WDB 0x51 /* Write Brightness Display >> > + * register >> > + */ >> > +#define ILI9341_WCD 0x53 /* Write Control Display >> > + * register >> > + */ >> > +#define ILI9341_RGB_INTERFACE 0xB0 /* RGB Interface Signal Control */ >> > +#define ILI9341_FRC 0xB1 /* Frame Rate Control register */ >> > +#define ILI9341_BPC 0xB5 /* Blanking Porch Control >> > + * register >> > + */ >> > +#define ILI9341_DFC 0xB6 /* Display Function Control >> > + * register >> > + */ >> > +#define ILI9341_POWER1 0xC0 /* Power Control 1 register */ >> > +#define ILI9341_POWER2 0xC1 /* Power Control 2 register */ >> > +#define ILI9341_VCOM1 0xC5 /* VCOM Control 1 register */ >> > +#define ILI9341_VCOM2 0xC7 /* VCOM Control 2 register */ >> > +#define ILI9341_POWERA 0xCB /* Power control A register */ >> > +#define ILI9341_POWERB 0xCF /* Power control B register */ >> > +#define ILI9341_PGAMMA 0xE0 /* Positive Gamma Correction >> > + * register >> > + */ >> > +#define ILI9341_NGAMMA 0xE1 /* Negative Gamma Correction >> > + * register >> > + */ >> > +#define ILI9341_DTCA 0xE8 /* Driver timing control A */ >> > +#define ILI9341_DTCB 0xEA /* Driver timing control B */ >> > +#define ILI9341_POWER_SEQ 0xED /* Power on sequence register */ >> > +#define ILI9341_3GAMMA_EN 0xF2 /* 3 Gamma enable register */ >> > +#define ILI9341_INTERFACE 0xF6 /* Interface control register */ >> > +#define ILI9341_PRC 0xF7 /* Pump ratio control register */ >> > + >> >> All the following should be const. > dillon: ok > >> >> Can any of the below be replaces by DEFINED constants? >> > +static u8 ili9341_cmd0[] = {0xc3, 0x08, 0x50}; >> > +static u8 ili9341_powerb[] = {0x00, 0xc1, 0x30}; >> > +static u8 ili9341_power_seq[] = {0x64, 0x03, 0x12, 0x81}; >> > +static u8 ili9341_dtca[] = {0x85, 0x00, 0x78}; >> > +static u8 ili9341_powera[] = {0x39, 0x2c, 0x00, 0x34, 0x02}; >> > +static u8 ili9341_prc[] = {0x20}; >> > +static u8 ili9341_dtcb[] = {0x00, 0x00}; >> > +static u8 ili9341_frc[] = {0x00, 0x1b}; >> > +static u8 ili9341_dfc1[] = {0x0a, 0xa2}; >> > +static u8 ili9341_power1[] = {0x10}; >> > +static u8 ili9341_power2[] = {0x10}; >> > +static u8 ili9341_vcom1[] = {0x45, 0x15}; >> > +static u8 ili9341_vcom2[] = {0x90}; >> > +static u8 ili9341_mac[] = {0xc8}; >> > +static u8 ili9341_gamma_en[] = {0x00}; >> > +static u8 ili9341_rgb_intr[] = {0xc2}; >> > +static u8 ili9341_dfc2[] = {0x0a, 0xa7, 0x27, 0x04}; >> > +static u8 ili9341_column_addr[] = {0x00, 0x00, 0x00, 0xef}; >> > +static u8 ili9341_page_addr[] = {0x00, 0x00, 0x01, 0x3f}; >> > +static u8 ili9341_intr[] = {0x01, 0x00, 0x06}; >> > +static u8 ili9341_gamma[] = {0x01}; >> > +static u8 ili9341_pgamma[] = {0x0f, 0x29, 0x24, 0x0c, 0x0e, 0x09, 0x4e, 0x78, >> > + 0x3c, 0x09, 0x13, 0x05, 0x17, 0x11, 0x00}; >> > +static u8 ili9341_ngamma[] = {0x00, 0x16, 0x1b, 0x04, 0x11, 0x07, 0x31, 0x33, >> > + 0x42, 0x05, 0x0c, 0x0a, 0x28, 0x2f, 0x0f}; >> > + >> > +/** >> > + * enum ili9341_input - the format of the incoming signal to the panel >> > + * >> > + * The panel can be connected to various input streams and four of them can >> > + * be selected by electronic straps on the display. However it is possible >> > + * to select another mode or override the electronic default with this >> > + * setting. >> > + */ >> > +enum ili9341_input { >> > + ILI9341_INPUT_PRGB_THROUGH = 0x0, >> > + ILI9341_INPUT_PRGB_ALIGNED = 0x1, >> > + ILI9341_INPUT_UNKNOWN = 0xf, >> > +}; >> > + >> > +/** >> > + * struct ili9341_config - the system specific ILI9341 configuration >> > + * @width_mm: physical panel width [mm] >> > + * @height_mm: physical panel height [mm] >> > + * @input: the input/entry type used in this system, if this is set to >> > + * ILI9341_INPUT_UNKNOWN the driver will try to figure it out by probing >> > + * the hardware >> > + * @dclk_active_high: data/pixel clock active high, data will be clocked >> > + * in on the rising edge of the DCLK (this is usually the case). >> > + * @de_active_high: DE (data entry) is active high >> > + * @hsync_active_high: HSYNC is active high >> > + * @vsync_active_high: VSYNC is active high >> > + */ >> > +struct ili9341_config { >> > + u32 width_mm; >> > + u32 height_mm; >> > + enum ili9341_input input; >> > + bool dclk_active_high; >> > + bool de_active_high; >> > + bool hsync_active_high; >> > + bool vsync_active_high; >> > +}; >> > + >> > +struct ili9341 { >> > + struct device *dev; >> > + const struct ili9341_config *conf; >> > + struct drm_panel panel; >> > + struct regmap *regmap; >> > + struct gpio_desc *reset_gpio; >> > + struct gpio_desc *dc_gpio; >> > + enum ili9341_input input; >> >> > + struct videomode vm; >> videomode is not used. So drop this field and drop the include files >> that are no logner needed. >> dillon: ok >> >> > +}; >> > + >> > +static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel) >> > +{ >> > + return container_of(panel, struct ili9341, panel); >> > +} >> > + >> > +int ili9341_spi_transfer(struct spi_device *spi, u32 speed_hz, >> > + u8 bpw, const void *buf, size_t len) >> > +{ >> > + size_t max_chunk = spi_max_transfer_size(spi); >> > + struct spi_transfer tr = { >> const? >> dillon: ok >> >> > + .bits_per_word = bpw, >> > + .speed_hz = speed_hz, >> > + .len = len, >> > + }; >> > + struct spi_message m; >> > + size_t chunk; >> > + int ret; >> > + >> > + spi_message_init_with_transfers(&m, &tr, 1); >> > + >> > + while (len) { >> > + chunk = min(len, max_chunk); >> > + >> > + tr.tx_buf = buf; >> > + tr.len = chunk; >> > + buf += chunk; >> > + len -= chunk; >> > + >> > + ret = spi_sync(spi, &m); >> > + if (ret) >> > + return ret; >> > + } >> > + return 0; >> > +} >> > +static int ili9341_regmap_spi_write(void *context, const void *data, >> > + size_t count) >> > +{ >> > + struct device *dev = context; >> > + struct spi_device *spi = to_spi_device(dev); >> > + struct ili9341 *ili = spi_get_drvdata(spi); >> > + int ret = 0; >> > + >> > + gpiod_set_value_cansleep(ili->dc_gpio, 0); >> > + >> > + ret = ili9341_spi_transfer(spi, DEFAULT_SPI_SPEED, 8, data+0, 1); >> > + if (ret || count == 1 || >> > + ((u8 *)data)[0] == ILI9341_GRAM || >> > + ((u8 *)data)[0] == ILI9341_DISPLAY_ON || >> > + ((u8 *)data)[0] == ILI9341_SLEEP_OUT || >> > + ((u8 *)data)[0] == ILI9341_DISPLAY_OFF) >> > + return ret; >> > + >> > + gpiod_set_value_cansleep(ili->dc_gpio, 1); >> > + >> > + return ili9341_spi_transfer(spi, DEFAULT_SPI_SPEED, 8, data+1, count-1); >> > +} >> > + >> > +static int ili9341_regmap_spi_read(void *context, const void *reg, >> > + size_t reg_size, void *val, size_t val_size) >> > +{ >> > + return 0; >> > +} >> Is this function really needed? If not delete it. >> > regmap will be drop in next patch. >> >> > + >> > +static struct regmap_bus ili9341_regmap_bus = { >> > + .write = ili9341_regmap_spi_write, >> > + .read = ili9341_regmap_spi_read, >> > + .reg_format_endian_default = REGMAP_ENDIAN_BIG, >> > + .val_format_endian_default = REGMAP_ENDIAN_BIG, >> > +}; >> > + >> > +static bool ili9341_volatile_reg(struct device *dev, unsigned int reg) >> > +{ >> > + return false; >> > +} >> Is this function really nedded? If not delete it. >> >> > + >> > +static bool ili9341_writeable_reg(struct device *dev, unsigned int reg) >> > +{ >> > + /* Just register 0 is read-only */ >> > + if (reg == 0x00) >> > + return false; >> > + return true; >> > +} >> > + >> > +static const struct regmap_config ili9341_regmap_config = { >> > + .reg_bits = 8, >> > + .val_bits = 8, >> > + .max_register = 0xff, >> > + .cache_type = REGCACHE_RBTREE, >> > + .volatile_reg = ili9341_volatile_reg, >> > + .writeable_reg = ili9341_writeable_reg, >> > +}; >> > + >> >> No error checks - consider something like: >> >> static int bulk_write(struct ili9341 *ili, u8 reg, const u8[] data, int len) >> { >> int err = ili->err; >> >> if (!err) { >> err = regmap_bulk_write(ili->regmap, reg, data, len); >> if (err) { >> dev_err(...); >> ili->err = err; >> } >> } >> >> return err; >> } >> >> Then you can use this in the following, and make this more readable. >> dillon: ok, thanks for detail guide. > >> >> > +static int ili9341_init(struct drm_panel *panel, struct ili9341 *ili) >> > +{ >> > + regmap_bulk_write(ili->regmap, 0xca, >> > + ili9341_cmd0, sizeof(ili9341_cmd0)); >> > + regmap_bulk_write(ili->regmap, ILI9341_POWERB, >> > + ili9341_powerb, sizeof(ili9341_powerb)); >> > + regmap_bulk_write(ili->regmap, ILI9341_POWER_SEQ, >> > + ili9341_power_seq, sizeof(ili9341_power_seq)); >> > + regmap_bulk_write(ili->regmap, ILI9341_DTCA, >> > + ili9341_dtca, sizeof(ili9341_dtca)); >> > + regmap_bulk_write(ili->regmap, ILI9341_POWERA, >> > + ili9341_powera, sizeof(ili9341_powera)); >> > + regmap_write(ili->regmap, ILI9341_PRC, ili9341_prc[0]); >> > + regmap_bulk_write(ili->regmap, ILI9341_DTCB, >> > + ili9341_dtcb, sizeof(ili9341_dtcb)); >> > + regmap_bulk_write(ili->regmap, ILI9341_FRC, >> > + ili9341_frc, sizeof(ili9341_frc)); >> > + regmap_bulk_write(ili->regmap, ILI9341_DFC, >> > + ili9341_dfc1, sizeof(ili9341_dfc1)); >> > + regmap_write(ili->regmap, ILI9341_POWER1, ili9341_power1[0]); >> > + regmap_write(ili->regmap, ILI9341_POWER2, ili9341_power2[0]); >> > + regmap_bulk_write(ili->regmap, ILI9341_VCOM1, >> > + ili9341_vcom1, sizeof(ili9341_vcom1)); >> > + regmap_write(ili->regmap, ILI9341_VCOM2, ili9341_vcom2[0]); >> > + regmap_write(ili->regmap, ILI9341_MAC, ili9341_mac[0]); >> > + regmap_write(ili->regmap, ILI9341_3GAMMA_EN, ili9341_gamma_en[0]); >> > + regmap_write(ili->regmap, ILI9341_RGB_INTERFACE, ili9341_rgb_intr[0]); >> > + regmap_bulk_write(ili->regmap, ILI9341_DFC, >> > + ili9341_dfc2, sizeof(ili9341_dfc2)); >> > + >> > + /* colomn address set */ >> > + regmap_bulk_write(ili->regmap, ILI9341_COLUMN_ADDR, >> > + ili9341_column_addr, sizeof(ili9341_column_addr)); >> > + >> > + /* Page Address Set */ >> > + regmap_bulk_write(ili->regmap, ILI9341_PAGE_ADDR, >> > + ili9341_page_addr, sizeof(ili9341_page_addr)); >> > + regmap_bulk_write(ili->regmap, ILI9341_INTERFACE, >> > + ili9341_intr, sizeof(ili9341_intr)); >> > + regmap_write(ili->regmap, ILI9341_GRAM, 0); >> > + msleep(200); >> > + >> > + regmap_write(ili->regmap, ILI9341_GAMMA, ili9341_gamma[0]); >> > + regmap_bulk_write(ili->regmap, ILI9341_PGAMMA, >> > + ili9341_pgamma, sizeof(ili9341_pgamma)); >> > + regmap_bulk_write(ili->regmap, ILI9341_NGAMMA, >> > + ili9341_ngamma, sizeof(ili9341_ngamma)); >> > + regmap_write(ili->regmap, ILI9341_SLEEP_OUT, 0); >> > + msleep(200); >> > + >> > + regmap_write(ili->regmap, ILI9341_DISPLAY_ON, 0); >> > + >> > + /* GRAM start writing */ >> > + regmap_write(ili->regmap, ILI9341_GRAM, 0); >> > + >> > + dev_info(ili->dev, "initialized display\n"); >> > + >> > + return 0; >> > +} >> > + >> > +/* >> > + * This power-on sequence if from the datasheet, page 57. >> > + */ >> > +static int ili9341_power_on(struct ili9341 *ili) >> > +{ >> > + /* Assert RESET */ >> > + gpiod_set_value(ili->reset_gpio, 1); >> > + >> > + msleep(20); >> > + >> > + /* De-assert RESET */ >> > + gpiod_set_value(ili->reset_gpio, 0); >> > + >> > + msleep(10); >> > + >> > + return 0; >> > +} >> > + >> > +static int ili9341_power_off(struct ili9341 *ili) >> > +{ >> >> Assert reset? dillon: will add reset-pin control here. > > >> >> > + return 0; >> > +} >> > + >> > +static int ili9341_disable(struct drm_panel *panel) >> > +{ >> > + struct ili9341 *ili = panel_to_ili9341(panel); >> > + int ret; >> > + >> > + ret = regmap_write(ili->regmap, ILI9341_DISPLAY_OFF, 0); >> > + if (ret) { >> > + dev_err(ili->dev, "unable to go to standby mode\n"); >> > + return ret; >> > + } >> > + >> > + return 0; >> > +} >> > + >> > +static int ili9341_unprepare(struct drm_panel *panel) >> > +{ >> > + struct ili9341 *ili = panel_to_ili9341(panel); >> > + >> > + return ili9341_power_off(ili); >> > +} >> > + >> > +static int ili9341_prepare(struct drm_panel *panel) >> > +{ >> > + struct ili9341 *ili = panel_to_ili9341(panel); >> > + int ret; >> > + >> > + ret = ili9341_power_on(ili); >> > + if (ret < 0) >> > + return ret; >> > + >> > + ret = ili9341_init(panel, ili); >> > + if (ret < 0) >> > + ili9341_unprepare(panel); >> > + >> > + return ret; >> > +} >> > + >> > +static int ili9341_enable(struct drm_panel *panel) >> > +{ >> > + struct ili9341 *ili = panel_to_ili9341(panel); >> > + int ret; >> > + >> > + ret = regmap_write(ili->regmap, ILI9341_DISPLAY_ON, 0); >> > + if (ret) { >> > + dev_err(ili->dev, "unable to enable panel\n"); >> > + return ret; >> > + } >> > + >> > + return 0; >> > +} >> > + >> > +/* This is the only mode listed for parallel RGB in the datasheet */ >> > +static const struct drm_display_mode prgb_320x240_mode = { >> > + .clock = 6100, >> > + .hdisplay = 240, >> > + .hsync_start = 240 + 10, >> > + .hsync_end = 240 + 10 + 10, >> > + .htotal = 280, >> > + .vdisplay = 320, >> > + .vsync_start = 320 + 4, >> > + .vsync_end = 320 + 4 + 2, >> > + .vtotal = 328, >> > + .vrefresh = 60, >> > + .flags = 0, >> > +}; >> > + >> > +static int ili9341_get_modes(struct drm_panel *panel, >> > + struct drm_connector *connector) >> > +{ >> > + struct ili9341 *ili = panel_to_ili9341(panel); >> > + struct drm_device *drm = connector->dev; >> > + struct drm_display_mode *mode; >> > + struct drm_display_info *info; >> > + >> > + info = &connector->display_info; >> > + info->width_mm = ili->conf->width_mm; >> > + info->height_mm = ili->conf->height_mm; >> > + if (ili->conf->dclk_active_high) >> > + info->bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE; >> > + else >> > + info->bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE; >> > + >> > + if (ili->conf->de_active_high) >> > + info->bus_flags |= DRM_BUS_FLAG_DE_HIGH; >> > + else >> > + info->bus_flags |= DRM_BUS_FLAG_DE_LOW; >> > + >> > + switch (ili->input) { >> > + case ILI9341_INPUT_PRGB_THROUGH: >> > + case ILI9341_INPUT_PRGB_ALIGNED: >> > + mode = drm_mode_duplicate(drm, &prgb_320x240_mode); >> > + break; >> > + default: >> > + mode = NULL; >> > + break; >> > + } >> > + if (!mode) { >> > + DRM_ERROR("bad mode or failed to add mode\n"); >> > + return -EINVAL; >> > + } >> > + drm_mode_set_name(mode); >> > + /* >> > + * This is the preferred mode because most people are going >> > + * to want to use the display with VGA type graphics. >> > + */ >> > + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; >> > + >> > + /* Set up the polarity */ >> > + if (ili->conf->hsync_active_high) >> > + mode->flags |= DRM_MODE_FLAG_PHSYNC; >> > + else >> > + mode->flags |= DRM_MODE_FLAG_NHSYNC; >> > + if (ili->conf->vsync_active_high) >> > + mode->flags |= DRM_MODE_FLAG_PVSYNC; >> > + else >> > + mode->flags |= DRM_MODE_FLAG_NVSYNC; >> > + >> > + mode->width_mm = ili->conf->width_mm; >> > + mode->height_mm = ili->conf->height_mm; >> > + drm_mode_probed_add(connector, mode); >> > + >> > + return 1; /* Number of modes */ >> > +} >> > + >> > +static const struct drm_panel_funcs ili9341_drm_funcs = { >> > + .disable = ili9341_disable, >> > + .unprepare = ili9341_unprepare, >> > + .prepare = ili9341_prepare, >> > + .enable = ili9341_enable, >> > + .get_modes = ili9341_get_modes, >> > +}; >> > + >> > +static int ili9341_probe(struct spi_device *spi) >> > +{ >> > + struct device *dev = &spi->dev; >> > + struct ili9341 *ili; >> > + const struct regmap_config *regmap_config; >> > + int ret; >> > + >> > + ili = devm_kzalloc(dev, sizeof(struct ili9341), GFP_KERNEL); >> > + if (!ili) >> > + return -ENOMEM; >> > + >> > + spi_set_drvdata(spi, ili); >> > + >> > + ili->dev = dev; >> > + /* >> > + * Every new incarnation of this display must have a unique >> > + * data entry for the system in this driver. >> > + */ >> > + ili->conf = of_device_get_match_data(dev); >> > + if (!ili->conf) { >> > + dev_err(dev, "missing device configuration\n"); >> > + return -ENODEV; >> > + } >> > + >> > + ili->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); >> > + if (IS_ERR(ili->reset_gpio)) { >> > + dev_err(dev, "failed to get RESET GPIO\n"); >> > + return PTR_ERR(ili->reset_gpio); >> > + } >> > + >> > + ili->dc_gpio = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); >> > + if (IS_ERR(ili->dc_gpio)) { >> > + dev_err(dev, "failed to get DC GPIO\n"); >> > + return PTR_ERR(ili->dc_gpio); >> > + } >> > + >> > + spi->bits_per_word = 8; >> > + ret = spi_setup(spi); >> > + if (ret < 0) { >> > + dev_err(dev, "spi setup failed.\n"); >> > + return ret; >> > + } >> > + >> > + regmap_config = &ili9341_regmap_config; >> > + >> > + ili->regmap = devm_regmap_init(dev, &ili9341_regmap_bus, dev, >> > + regmap_config); >> > + if (IS_ERR(ili->regmap)) { >> > + dev_err(dev, "failed to allocate register map\n"); >> > + return PTR_ERR(ili->regmap); >> > + } >> > + >> > + ili->input = ili->conf->input; >> > + >> > + drm_panel_init(&ili->panel, dev, &ili9341_drm_funcs, >> > + DRM_MODE_CONNECTOR_DPI); >> > + >> > + return drm_panel_add(&ili->panel); >> > +} >> > + >> > +static int ili9341_remove(struct spi_device *spi) >> > +{ >> > + struct ili9341 *ili = spi_get_drvdata(spi); >> > + >> > + ili9341_power_off(ili); >> > + drm_panel_remove(&ili->panel); >> > + >> > + return 0; >> > +} >> > + >> > +/* >> > + * The Stm32f429-disco board has a panel ili9341 connected to ltdc controller >> > + */ >> > +static const struct ili9341_config ili9341_data = { >> This should be named "disco" something as this is m32f429-disco >> specific. >> dillon: ok >> >> > + .width_mm = 65, >> > + .height_mm = 50, >> > + .input = ILI9341_INPUT_PRGB_THROUGH, >> > + .dclk_active_high = true, >> > + .de_active_high = false, >> > + .hsync_active_high = false, >> > + .vsync_active_high = false, >> > +}; >> > + >> > +static const struct of_device_id ili9341_of_match[] = { >> > + { >> > + .compatible = "stm32f429,ltdc-panel", >> > + .data = &ili9341_data, >> > + }, >> >> >> > + { >> > + .compatible = "ilitek,ili9341", >> > + .data = NULL, >> This part is wrong, as ilitek,ili9341 is just the generic part. >> Only the first entry is relevant. >> >> dillon: ok >> >> > + }, >> > + { } >> > +}; >> > +MODULE_DEVICE_TABLE(of, ili9341_of_match); >> > + >> > +static struct spi_driver ili9341_driver = { >> > + .probe = ili9341_probe, >> > + .remove = ili9341_remove, >> > + .driver = { >> > + .name = "panel-ilitek-ili9341", >> > + .of_match_table = ili9341_of_match, >> > + }, >> > +}; >> > +module_spi_driver(ili9341_driver); >> > + >> > +MODULE_AUTHOR("Dillon Min <dillon.minfei@gmail.com>"); >> > +MODULE_DESCRIPTION("ILI9341 LCD panel driver"); >> > +MODULE_LICENSE("GPL v2"); >> > -- >> > 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 5/5] drm/panel: add panel driver for Ilitek ili9341 panels [not found] ` <CAL9mu0LcT3pPiM6PdJRvAJRMbXY6KACF1wNg8yOS-6D8aq6AeA@mail.gmail.com> 2020-05-09 8:31 ` dillon min @ 2020-05-09 20:06 ` Sam Ravnborg 2020-05-10 4:04 ` dillon min 1 sibling, 1 reply; 10+ messages in thread From: Sam Ravnborg @ 2020-05-09 20:06 UTC (permalink / raw) To: dillon min Cc: mcoquelin.stm32, Alexandre Torgue, devicetree, airlied, Michael Turquette, linux-kernel, dri-devel, linux-clk, sboyd, robh+dt, thierry.reding, linux-stm32, linux-arm-kernel Hi Dillon. On Fri, May 08, 2020 at 06:13:43PM +0800, dillon min wrote: > Hi Sam, > > Thanks for your comments, i will rework this panel driver after l3gd20 > patch submission. > > Sam Ravnborg <sam@ravnborg.org> 于2020年5月8日周五 下午5:02写道： > > > Hi Dillon. > > > > Patch submissions starts to look fine. > > > > On Fri, May 08, 2020 at 12:13:14PM +0800, dillon.minfei@gmail.com wrote: > > > From: dillon min <dillon.minfei@gmail.com> > > > > > > This is a driver for 320x240 TFT panels, accepting a rgb input > > > streams that get adapted and scaled to the panel. > > This driver is, I suppose, prepared to be a driver for ILI9341 based > > panles, and as such not for a fixed resolution. > > I expect (hope) we in the future will see more panels added. > > > > As i checked ili9341 datasheets, this panel just support 240x320 > resolution only. if i'm wrong , please correct me. thanks > https://cdn-shop.adafruit.com/datasheets/ILI9341.pdf > > This panel can support 9 different kinds of interface , "3/4-line Serial > Interface" have been supported by tiny/ili9341.c. i was verified it > but the performance on stm32f4 is very low. It had somehow escaped my mind we already have a driver for ili9341 based panels. And we do not want onyl one driver for this IC. So please extent the current driver to match your panel as well as the original panel. And then also extend the current binding for ili9341, which you may have to convert to DT Schema in the process. This is more work now but when it is done we end up with a much better solution than if we introduce an additional driver for ili9341. Sorry for not catching this in the inital review. Sam > > currently, i just have stm32f429-disco in hands, with 18-bit parallel rgb > bus connected on this board. reference to panel-ilitek-ili9322 and > panel-ilitek-ili9881 driver, i have some plan to rewrite this driver. > > 1 add your below comments in. > 2 use dc-gpio, reset-gpio, rgb-interface-bits from dts to config panel > interface. > 3 drop regmap, porting drm_mipi_dbi's mipi_dbi_command to config panel > paramter. like tiny/ili9341.c > 4 support rgb-16, rgb-18 interface. > 5 use optional regulator or power gpio to control panel's power, as panel > power is always on for my board, so i can't test this part. could i add the > code which can't be tested? > 6 support rotation in panel config (currently , i rotate the screen by > kernel cmdline paramter) > > if you have any other common panel configuration should be add , please > inform me. > > thanks. > > > > > Some things to fix, see comments in the follwoing. > > > > Sam > > > > > > > > Signed-off-by: dillon min <dillon.minfei@gmail.com> > > > --- > > > drivers/gpu/drm/panel/Kconfig | 8 + > > > drivers/gpu/drm/panel/Makefile | 1 + > > > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 561 > > +++++++++++++++++++++++++++ > > > 3 files changed, 570 insertions(+) > > > create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > > > > > diff --git a/drivers/gpu/drm/panel/Kconfig > > b/drivers/gpu/drm/panel/Kconfig > > > index a1723c1..e42692c 100644 > > > --- a/drivers/gpu/drm/panel/Kconfig > > > +++ b/drivers/gpu/drm/panel/Kconfig > > > @@ -95,6 +95,14 @@ config DRM_PANEL_ILITEK_IL9322 > > > Say Y here if you want to enable support for Ilitek IL9322 > > > QVGA (320x240) RGB, YUV and ITU-T BT.656 panels. > > > > > > +config DRM_PANEL_ILITEK_IL9341 > > ILI9341 - so the config name matches the name of the driver IC. > > > > > + tristate "Ilitek ILI9341 240x320 QVGA panels" > > > + depends on OF && SPI > > > + select REGMAP > > > + help > > > + Say Y here if you want to enable support for Ilitek IL9341 > > > + QVGA (240x320) RGB panels. > > See comment to the changelog, the driver is more generic - I assume. > > So the wording here can be improved to express this. > > > > Add support RGB 16bits and RGB 18bits bus only ? > > > > + > > > config DRM_PANEL_ILITEK_ILI9881C > > > tristate "Ilitek ILI9881C-based panels" > > > depends on OF > > > diff --git a/drivers/gpu/drm/panel/Makefile > > b/drivers/gpu/drm/panel/Makefile > > > index 96a883c..d123543 100644 > > > --- a/drivers/gpu/drm/panel/Makefile > > > +++ b/drivers/gpu/drm/panel/Makefile > > > @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) += > > panel-elida-kd35t133.o > > > obj-$(CONFIG_DRM_PANEL_FEIXIN_K101_IM2BA02) += > > panel-feixin-k101-im2ba02.o > > > obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += > > panel-feiyang-fy07024di26a30d.o > > > obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o > > > +obj-$(CONFIG_DRM_PANEL_ILITEK_IL9341) += panel-ilitek-ili9341.o > > > obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o > > > obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o > > > obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o > > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > > new file mode 100644 > > > index 0000000..ec22d80 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > > @@ -0,0 +1,561 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * Ilitek ILI9341 TFT LCD drm_panel driver. > > > + * > > > + * This panel can be configured to support: > > > + * - 16-bit parallel RGB interface > > The interface to ILI9341 is SPI, and the interface between the ILI9341 > > and the panel is more of an itnernal thing. Or did I get this worng? > > > > SPI is for register configuration. RGB parallel for data transfer > > > + * > > > + * Copyright (C) 2020 Dillon Min <dillon.minfei@gmail.com> > > > + * Derived from drivers/drm/gpu/panel/panel-ilitek-ili9322.c > > > + */ > > > + > > > +#include <linux/bitops.h> > > > +#include <linux/gpio/consumer.h> > > > +#include <linux/module.h> > > > +#include <linux/of_device.h> > > > +#include <linux/regmap.h> > > > +#include <linux/regulator/consumer.h> > > > +#include <linux/spi/spi.h> > > > + > > > +#include <video/mipi_display.h> > > > +#include <video/of_videomode.h> > > > +#include <video/videomode.h> > > > + > > > +#include <drm/drm_modes.h> > > > +#include <drm/drm_panel.h> > > > +#include <drm/drm_print.h> > > > + > > > +#define DEFAULT_SPI_SPEED 10000000 > > > + > > > > Please use same case for hex numbers in the driver. > > My personal preferences is lower-case. > > > > in next patch, spi speed will be configured by dts, not hardcode here. > below register address will change to lower-case hex numbers. > > > > > +#define ILI9341_SLEEP_OUT 0x11 /* Sleep out register */ > > > +#define ILI9341_GAMMA 0x26 /* Gamma register */ > > > +#define ILI9341_DISPLAY_OFF 0x28 /* Display off register */ > > > +#define ILI9341_DISPLAY_ON 0x29 /* Display on register */ > > > +#define ILI9341_COLUMN_ADDR 0x2A /* Colomn address register > > */ > > > +#define ILI9341_PAGE_ADDR 0x2B /* Page address register */ > > > +#define ILI9341_GRAM 0x2C /* GRAM register */ > > > +#define ILI9341_MAC 0x36 /* Memory Access Control > > register*/ > > > +#define ILI9341_PIXEL_FORMAT 0x3A /* Pixel Format register */ > > > +#define ILI9341_WDB 0x51 /* Write Brightness Display > > > + * register > > > + */ > > > +#define ILI9341_WCD 0x53 /* Write Control Display > > > + * register > > > + */ > > > +#define ILI9341_RGB_INTERFACE 0xB0 /* RGB Interface Signal > > Control */ > > > +#define ILI9341_FRC 0xB1 /* Frame Rate Control > > register */ > > > +#define ILI9341_BPC 0xB5 /* Blanking Porch Control > > > + * register > > > + */ > > > +#define ILI9341_DFC 0xB6 /* Display Function Control > > > + * register > > > + */ > > > +#define ILI9341_POWER1 0xC0 /* Power Control 1 register > > */ > > > +#define ILI9341_POWER2 0xC1 /* Power Control 2 register > > */ > > > +#define ILI9341_VCOM1 0xC5 /* VCOM Control 1 register > > */ > > > +#define ILI9341_VCOM2 0xC7 /* VCOM Control 2 register > > */ > > > +#define ILI9341_POWERA 0xCB /* Power control A register > > */ > > > +#define ILI9341_POWERB 0xCF /* Power control B register > > */ > > > +#define ILI9341_PGAMMA 0xE0 /* Positive Gamma Correction > > > + * register > > > + */ > > > +#define ILI9341_NGAMMA 0xE1 /* Negative Gamma Correction > > > + * register > > > + */ > > > +#define ILI9341_DTCA 0xE8 /* Driver timing control A > > */ > > > +#define ILI9341_DTCB 0xEA /* Driver timing control B > > */ > > > +#define ILI9341_POWER_SEQ 0xED /* Power on sequence > > register */ > > > +#define ILI9341_3GAMMA_EN 0xF2 /* 3 Gamma enable register > > */ > > > +#define ILI9341_INTERFACE 0xF6 /* Interface control > > register */ > > > +#define ILI9341_PRC 0xF7 /* Pump ratio control > > register */ > > > + > > > > All the following should be const. > > > ok > > > > Can any of the below be replaces by DEFINED constants? > > > +static u8 ili9341_cmd0[] = {0xc3, 0x08, 0x50}; > > > +static u8 ili9341_powerb[] = {0x00, 0xc1, 0x30}; > > > +static u8 ili9341_power_seq[] = {0x64, 0x03, 0x12, 0x81}; > > > +static u8 ili9341_dtca[] = {0x85, 0x00, 0x78}; > > > +static u8 ili9341_powera[] = {0x39, 0x2c, 0x00, 0x34, 0x02}; > > > +static u8 ili9341_prc[] = {0x20}; > > > +static u8 ili9341_dtcb[] = {0x00, 0x00}; > > > +static u8 ili9341_frc[] = {0x00, 0x1b}; > > > +static u8 ili9341_dfc1[] = {0x0a, 0xa2}; > > > +static u8 ili9341_power1[] = {0x10}; > > > +static u8 ili9341_power2[] = {0x10}; > > > +static u8 ili9341_vcom1[] = {0x45, 0x15}; > > > +static u8 ili9341_vcom2[] = {0x90}; > > > +static u8 ili9341_mac[] = {0xc8}; > > > +static u8 ili9341_gamma_en[] = {0x00}; > > > +static u8 ili9341_rgb_intr[] = {0xc2}; > > > +static u8 ili9341_dfc2[] = {0x0a, 0xa7, 0x27, 0x04}; > > > +static u8 ili9341_column_addr[] = {0x00, 0x00, 0x00, 0xef}; > > > +static u8 ili9341_page_addr[] = {0x00, 0x00, 0x01, 0x3f}; > > > +static u8 ili9341_intr[] = {0x01, 0x00, 0x06}; > > > +static u8 ili9341_gamma[] = {0x01}; > > > +static u8 ili9341_pgamma[] = {0x0f, 0x29, 0x24, 0x0c, 0x0e, 0x09, 0x4e, > > 0x78, > > > + 0x3c, 0x09, 0x13, 0x05, 0x17, 0x11, 0x00}; > > > +static u8 ili9341_ngamma[] = {0x00, 0x16, 0x1b, 0x04, 0x11, 0x07, 0x31, > > 0x33, > > > + 0x42, 0x05, 0x0c, 0x0a, 0x28, 0x2f, 0x0f}; > > > + > > > +/** > > > + * enum ili9341_input - the format of the incoming signal to the panel > > > + * > > > + * The panel can be connected to various input streams and four of them > > can > > > + * be selected by electronic straps on the display. However it is > > possible > > > + * to select another mode or override the electronic default with this > > > + * setting. > > > + */ > > > +enum ili9341_input { > > > + ILI9341_INPUT_PRGB_THROUGH = 0x0, > > > + ILI9341_INPUT_PRGB_ALIGNED = 0x1, > > > + ILI9341_INPUT_UNKNOWN = 0xf, > > > +}; > > > + > > > +/** > > > + * struct ili9341_config - the system specific ILI9341 configuration > > > + * @width_mm: physical panel width [mm] > > > + * @height_mm: physical panel height [mm] > > > + * @input: the input/entry type used in this system, if this is set to > > > + * ILI9341_INPUT_UNKNOWN the driver will try to figure it out by probing > > > + * the hardware > > > + * @dclk_active_high: data/pixel clock active high, data will be clocked > > > + * in on the rising edge of the DCLK (this is usually the case). > > > + * @de_active_high: DE (data entry) is active high > > > + * @hsync_active_high: HSYNC is active high > > > + * @vsync_active_high: VSYNC is active high > > > + */ > > > +struct ili9341_config { > > > + u32 width_mm; > > > + u32 height_mm; > > > + enum ili9341_input input; > > > + bool dclk_active_high; > > > + bool de_active_high; > > > + bool hsync_active_high; > > > + bool vsync_active_high; > > > +}; > > > + > > > +struct ili9341 { > > > + struct device *dev; > > > + const struct ili9341_config *conf; > > > + struct drm_panel panel; > > > + struct regmap *regmap; > > > + struct gpio_desc *reset_gpio; > > > + struct gpio_desc *dc_gpio; > > > + enum ili9341_input input; > > > > > + struct videomode vm; > > videomode is not used. So drop this field and drop the include files > > that are no logner needed. > > > > ok > > > > +}; > > > + > > > +static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel) > > > +{ > > > + return container_of(panel, struct ili9341, panel); > > > +} > > > + > > > +int ili9341_spi_transfer(struct spi_device *spi, u32 speed_hz, > > > + u8 bpw, const void *buf, size_t len) > > > +{ > > > + size_t max_chunk = spi_max_transfer_size(spi); > > > + struct spi_transfer tr = { > > const? > > > > ok > > > > + .bits_per_word = bpw, > > > + .speed_hz = speed_hz, > > > + .len = len, > > > + }; > > > + struct spi_message m; > > > + size_t chunk; > > > + int ret; > > > + > > > + spi_message_init_with_transfers(&m, &tr, 1); > > > + > > > + while (len) { > > > + chunk = min(len, max_chunk); > > > + > > > + tr.tx_buf = buf; > > > + tr.len = chunk; > > > + buf += chunk; > > > + len -= chunk; > > > + > > > + ret = spi_sync(spi, &m); > > > + if (ret) > > > + return ret; > > > + } > > > + return 0; > > > +} > > > +static int ili9341_regmap_spi_write(void *context, const void *data, > > > + size_t count) > > > +{ > > > + struct device *dev = context; > > > + struct spi_device *spi = to_spi_device(dev); > > > + struct ili9341 *ili = spi_get_drvdata(spi); > > > + int ret = 0; > > > + > > > + gpiod_set_value_cansleep(ili->dc_gpio, 0); > > > + > > > + ret = ili9341_spi_transfer(spi, DEFAULT_SPI_SPEED, 8, data+0, 1); > > > + if (ret || count == 1 || > > > + ((u8 *)data)[0] == ILI9341_GRAM || > > > + ((u8 *)data)[0] == ILI9341_DISPLAY_ON || > > > + ((u8 *)data)[0] == ILI9341_SLEEP_OUT || > > > + ((u8 *)data)[0] == ILI9341_DISPLAY_OFF) > > > + return ret; > > > + > > > + gpiod_set_value_cansleep(ili->dc_gpio, 1); > > > + > > > + return ili9341_spi_transfer(spi, DEFAULT_SPI_SPEED, 8, data+1, > > count-1); > > > +} > > > + > > > +static int ili9341_regmap_spi_read(void *context, const void *reg, > > > + size_t reg_size, void *val, size_t > > val_size) > > > +{ > > > + return 0; > > > +} > > Is this function really needed? If not delete it. > > > > regmap will be drop in next patch. > > > > + > > > +static struct regmap_bus ili9341_regmap_bus = { > > > + .write = ili9341_regmap_spi_write, > > > + .read = ili9341_regmap_spi_read, > > > + .reg_format_endian_default = REGMAP_ENDIAN_BIG, > > > + .val_format_endian_default = REGMAP_ENDIAN_BIG, > > > +}; > > > + > > > +static bool ili9341_volatile_reg(struct device *dev, unsigned int reg) > > > +{ > > > + return false; > > > +} > > Is this function really nedded? If not delete it. > > > > > + > > > +static bool ili9341_writeable_reg(struct device *dev, unsigned int reg) > > > +{ > > > + /* Just register 0 is read-only */ > > > + if (reg == 0x00) > > > + return false; > > > + return true; > > > +} > > > + > > > +static const struct regmap_config ili9341_regmap_config = { > > > + .reg_bits = 8, > > > + .val_bits = 8, > > > + .max_register = 0xff, > > > + .cache_type = REGCACHE_RBTREE, > > > + .volatile_reg = ili9341_volatile_reg, > > > + .writeable_reg = ili9341_writeable_reg, > > > +}; > > > + > > > > No error checks - consider something like: > > > > static int bulk_write(struct ili9341 *ili, u8 reg, const u8[] data, int > > len) > > { > > int err = ili->err; > > > > if (!err) { > > err = regmap_bulk_write(ili->regmap, reg, data, len); > > if (err) { > > dev_err(...); > > ili->err = err; > > } > > } > > > > return err; > > } > > > > Then you can use this in the following, and make this more readable. > > > > ok, thanks for detail guide. > > > > > +static int ili9341_init(struct drm_panel *panel, struct ili9341 *ili) > > > +{ > > > + regmap_bulk_write(ili->regmap, 0xca, > > > + ili9341_cmd0, > > sizeof(ili9341_cmd0)); > > > + regmap_bulk_write(ili->regmap, ILI9341_POWERB, > > > + ili9341_powerb, sizeof(ili9341_powerb)); > > > + regmap_bulk_write(ili->regmap, ILI9341_POWER_SEQ, > > > + ili9341_power_seq, > > sizeof(ili9341_power_seq)); > > > + regmap_bulk_write(ili->regmap, ILI9341_DTCA, > > > + ili9341_dtca, sizeof(ili9341_dtca)); > > > + regmap_bulk_write(ili->regmap, ILI9341_POWERA, > > > + ili9341_powera, sizeof(ili9341_powera)); > > > + regmap_write(ili->regmap, ILI9341_PRC, ili9341_prc[0]); > > > + regmap_bulk_write(ili->regmap, ILI9341_DTCB, > > > + ili9341_dtcb, sizeof(ili9341_dtcb)); > > > + regmap_bulk_write(ili->regmap, ILI9341_FRC, > > > + ili9341_frc, sizeof(ili9341_frc)); > > > + regmap_bulk_write(ili->regmap, ILI9341_DFC, > > > + ili9341_dfc1, sizeof(ili9341_dfc1)); > > > + regmap_write(ili->regmap, ILI9341_POWER1, ili9341_power1[0]); > > > + regmap_write(ili->regmap, ILI9341_POWER2, ili9341_power2[0]); > > > + regmap_bulk_write(ili->regmap, ILI9341_VCOM1, > > > + ili9341_vcom1, sizeof(ili9341_vcom1)); > > > + regmap_write(ili->regmap, ILI9341_VCOM2, ili9341_vcom2[0]); > > > + regmap_write(ili->regmap, ILI9341_MAC, ili9341_mac[0]); > > > + regmap_write(ili->regmap, ILI9341_3GAMMA_EN, ili9341_gamma_en[0]); > > > + regmap_write(ili->regmap, ILI9341_RGB_INTERFACE, > > ili9341_rgb_intr[0]); > > > + regmap_bulk_write(ili->regmap, ILI9341_DFC, > > > + ili9341_dfc2, sizeof(ili9341_dfc2)); > > > + > > > + /* colomn address set */ > > > + regmap_bulk_write(ili->regmap, ILI9341_COLUMN_ADDR, > > > + ili9341_column_addr, sizeof(ili9341_column_addr)); > > > + > > > + /* Page Address Set */ > > > + regmap_bulk_write(ili->regmap, ILI9341_PAGE_ADDR, > > > + ili9341_page_addr, > > sizeof(ili9341_page_addr)); > > > + regmap_bulk_write(ili->regmap, ILI9341_INTERFACE, > > > + ili9341_intr, sizeof(ili9341_intr)); > > > + regmap_write(ili->regmap, ILI9341_GRAM, 0); > > > + msleep(200); > > > + > > > + regmap_write(ili->regmap, ILI9341_GAMMA, ili9341_gamma[0]); > > > + regmap_bulk_write(ili->regmap, ILI9341_PGAMMA, > > > + ili9341_pgamma, sizeof(ili9341_pgamma)); > > > + regmap_bulk_write(ili->regmap, ILI9341_NGAMMA, > > > + ili9341_ngamma, sizeof(ili9341_ngamma)); > > > + regmap_write(ili->regmap, ILI9341_SLEEP_OUT, 0); > > > + msleep(200); > > > + > > > + regmap_write(ili->regmap, ILI9341_DISPLAY_ON, 0); > > > + > > > + /* GRAM start writing */ > > > + regmap_write(ili->regmap, ILI9341_GRAM, 0); > > > + > > > + dev_info(ili->dev, "initialized display\n"); > > > + > > > + return 0; > > > +} > > > + > > > +/* > > > + * This power-on sequence if from the datasheet, page 57. > > > + */ > > > +static int ili9341_power_on(struct ili9341 *ili) > > > +{ > > > + /* Assert RESET */ > > > + gpiod_set_value(ili->reset_gpio, 1); > > > + > > > + msleep(20); > > > + > > > + /* De-assert RESET */ > > > + gpiod_set_value(ili->reset_gpio, 0); > > > + > > > + msleep(10); > > > + > > > + return 0; > > > +} > > > + > > > +static int ili9341_power_off(struct ili9341 *ili) > > > +{ > > > > Assert reset? > > will add reset-pin control here. > > > > > > > + return 0; > > > +} > > > + > > > +static int ili9341_disable(struct drm_panel *panel) > > > +{ > > > + struct ili9341 *ili = panel_to_ili9341(panel); > > > + int ret; > > > + > > > + ret = regmap_write(ili->regmap, ILI9341_DISPLAY_OFF, 0); > > > + if (ret) { > > > + dev_err(ili->dev, "unable to go to standby mode\n"); > > > + return ret; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static int ili9341_unprepare(struct drm_panel *panel) > > > +{ > > > + struct ili9341 *ili = panel_to_ili9341(panel); > > > + > > > + return ili9341_power_off(ili); > > > +} > > > + > > > +static int ili9341_prepare(struct drm_panel *panel) > > > +{ > > > + struct ili9341 *ili = panel_to_ili9341(panel); > > > + int ret; > > > + > > > + ret = ili9341_power_on(ili); > > > + if (ret < 0) > > > + return ret; > > > + > > > + ret = ili9341_init(panel, ili); > > > + if (ret < 0) > > > + ili9341_unprepare(panel); > > > + > > > + return ret; > > > +} > > > + > > > +static int ili9341_enable(struct drm_panel *panel) > > > +{ > > > + struct ili9341 *ili = panel_to_ili9341(panel); > > > + int ret; > > > + > > > + ret = regmap_write(ili->regmap, ILI9341_DISPLAY_ON, 0); > > > + if (ret) { > > > + dev_err(ili->dev, "unable to enable panel\n"); > > > + return ret; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +/* This is the only mode listed for parallel RGB in the datasheet */ > > > +static const struct drm_display_mode prgb_320x240_mode = { > > > + .clock = 6100, > > > + .hdisplay = 240, > > > + .hsync_start = 240 + 10, > > > + .hsync_end = 240 + 10 + 10, > > > + .htotal = 280, > > > + .vdisplay = 320, > > > + .vsync_start = 320 + 4, > > > + .vsync_end = 320 + 4 + 2, > > > + .vtotal = 328, > > > + .vrefresh = 60, > > > + .flags = 0, > > > +}; > > > + > > > +static int ili9341_get_modes(struct drm_panel *panel, > > > + struct drm_connector *connector) > > > +{ > > > + struct ili9341 *ili = panel_to_ili9341(panel); > > > + struct drm_device *drm = connector->dev; > > > + struct drm_display_mode *mode; > > > + struct drm_display_info *info; > > > + > > > + info = &connector->display_info; > > > + info->width_mm = ili->conf->width_mm; > > > + info->height_mm = ili->conf->height_mm; > > > + if (ili->conf->dclk_active_high) > > > + info->bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE; > > > + else > > > + info->bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE; > > > + > > > + if (ili->conf->de_active_high) > > > + info->bus_flags |= DRM_BUS_FLAG_DE_HIGH; > > > + else > > > + info->bus_flags |= DRM_BUS_FLAG_DE_LOW; > > > + > > > + switch (ili->input) { > > > + case ILI9341_INPUT_PRGB_THROUGH: > > > + case ILI9341_INPUT_PRGB_ALIGNED: > > > + mode = drm_mode_duplicate(drm, &prgb_320x240_mode); > > > + break; > > > + default: > > > + mode = NULL; > > > + break; > > > + } > > > + if (!mode) { > > > + DRM_ERROR("bad mode or failed to add mode\n"); > > > + return -EINVAL; > > > + } > > > + drm_mode_set_name(mode); > > > + /* > > > + * This is the preferred mode because most people are going > > > + * to want to use the display with VGA type graphics. > > > + */ > > > + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > > > + > > > + /* Set up the polarity */ > > > + if (ili->conf->hsync_active_high) > > > + mode->flags |= DRM_MODE_FLAG_PHSYNC; > > > + else > > > + mode->flags |= DRM_MODE_FLAG_NHSYNC; > > > + if (ili->conf->vsync_active_high) > > > + mode->flags |= DRM_MODE_FLAG_PVSYNC; > > > + else > > > + mode->flags |= DRM_MODE_FLAG_NVSYNC; > > > + > > > + mode->width_mm = ili->conf->width_mm; > > > + mode->height_mm = ili->conf->height_mm; > > > + drm_mode_probed_add(connector, mode); > > > + > > > + return 1; /* Number of modes */ > > > +} > > > + > > > +static const struct drm_panel_funcs ili9341_drm_funcs = { > > > + .disable = ili9341_disable, > > > + .unprepare = ili9341_unprepare, > > > + .prepare = ili9341_prepare, > > > + .enable = ili9341_enable, > > > + .get_modes = ili9341_get_modes, > > > +}; > > > + > > > +static int ili9341_probe(struct spi_device *spi) > > > +{ > > > + struct device *dev = &spi->dev; > > > + struct ili9341 *ili; > > > + const struct regmap_config *regmap_config; > > > + int ret; > > > + > > > + ili = devm_kzalloc(dev, sizeof(struct ili9341), GFP_KERNEL); > > > + if (!ili) > > > + return -ENOMEM; > > > + > > > + spi_set_drvdata(spi, ili); > > > + > > > + ili->dev = dev; > > > + /* > > > + * Every new incarnation of this display must have a unique > > > + * data entry for the system in this driver. > > > + */ > > > + ili->conf = of_device_get_match_data(dev); > > > + if (!ili->conf) { > > > + dev_err(dev, "missing device configuration\n"); > > > + return -ENODEV; > > > + } > > > + > > > + ili->reset_gpio = devm_gpiod_get_optional(dev, "reset", > > GPIOD_OUT_HIGH); > > > + if (IS_ERR(ili->reset_gpio)) { > > > + dev_err(dev, "failed to get RESET GPIO\n"); > > > + return PTR_ERR(ili->reset_gpio); > > > + } > > > + > > > + ili->dc_gpio = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); > > > + if (IS_ERR(ili->dc_gpio)) { > > > + dev_err(dev, "failed to get DC GPIO\n"); > > > + return PTR_ERR(ili->dc_gpio); > > > + } > > > + > > > + spi->bits_per_word = 8; > > > + ret = spi_setup(spi); > > > + if (ret < 0) { > > > + dev_err(dev, "spi setup failed.\n"); > > > + return ret; > > > + } > > > + > > > + regmap_config = &ili9341_regmap_config; > > > + > > > + ili->regmap = devm_regmap_init(dev, &ili9341_regmap_bus, dev, > > > + regmap_config); > > > + if (IS_ERR(ili->regmap)) { > > > + dev_err(dev, "failed to allocate register map\n"); > > > + return PTR_ERR(ili->regmap); > > > + } > > > + > > > + ili->input = ili->conf->input; > > > + > > > + drm_panel_init(&ili->panel, dev, &ili9341_drm_funcs, > > > + DRM_MODE_CONNECTOR_DPI); > > > + > > > + return drm_panel_add(&ili->panel); > > > +} > > > + > > > +static int ili9341_remove(struct spi_device *spi) > > > +{ > > > + struct ili9341 *ili = spi_get_drvdata(spi); > > > + > > > + ili9341_power_off(ili); > > > + drm_panel_remove(&ili->panel); > > > + > > > + return 0; > > > +} > > > + > > > +/* > > > + * The Stm32f429-disco board has a panel ili9341 connected to ltdc > > controller > > > + */ > > > +static const struct ili9341_config ili9341_data = { > > This should be named "disco" something as this is m32f429-disco > > specific. > > > > ok > > > > + .width_mm = 65, > > > + .height_mm = 50, > > > + .input = ILI9341_INPUT_PRGB_THROUGH, > > > + .dclk_active_high = true, > > > + .de_active_high = false, > > > + .hsync_active_high = false, > > > + .vsync_active_high = false, > > > +}; > > > + > > > +static const struct of_device_id ili9341_of_match[] = { > > > + { > > > + .compatible = "stm32f429,ltdc-panel", > > > + .data = &ili9341_data, > > > + }, > > > > > > > + { > > > + .compatible = "ilitek,ili9341", > > > + .data = NULL, > > This part is wrong, as ilitek,ili9341 is just the generic part. > > Only the first entry is relevant. > > > > > > ok > > > > + }, > > > + { } > > > +}; > > > +MODULE_DEVICE_TABLE(of, ili9341_of_match); > > > + > > > +static struct spi_driver ili9341_driver = { > > > + .probe = ili9341_probe, > > > + .remove = ili9341_remove, > > > + .driver = { > > > + .name = "panel-ilitek-ili9341", > > > + .of_match_table = ili9341_of_match, > > > + }, > > > +}; > > > +module_spi_driver(ili9341_driver); > > > + > > > +MODULE_AUTHOR("Dillon Min <dillon.minfei@gmail.com>"); > > > +MODULE_DESCRIPTION("ILI9341 LCD panel driver"); > > > +MODULE_LICENSE("GPL v2"); > > > -- > > > 2.7.4 > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 5/5] drm/panel: add panel driver for Ilitek ili9341 panels 2020-05-09 20:06 ` Sam Ravnborg @ 2020-05-10 4:04 ` dillon min 0 siblings, 0 replies; 10+ messages in thread From: dillon min @ 2020-05-10 4:04 UTC (permalink / raw) To: Sam Ravnborg Cc: mcoquelin.stm32, Alexandre Torgue, devicetree, airlied, Michael Turquette, linux-kernel, dri-devel, linux-clk, sboyd, robh+dt, thierry.reding, linux-stm32, linux-arm-kernel Sam Ravnborg <sam@ravnborg.org> 于2020年5月10日周日 上午4:06写道： > > Hi Dillon. > > On Fri, May 08, 2020 at 06:13:43PM +0800, dillon min wrote: > > Hi Sam, > > > > Thanks for your comments, i will rework this panel driver after l3gd20 > > patch submission. > > > > Sam Ravnborg <sam@ravnborg.org> 于2020年5月8日周五 下午5:02写道： > > > > > Hi Dillon. > > > > > > Patch submissions starts to look fine. > > > > > > On Fri, May 08, 2020 at 12:13:14PM +0800, dillon.minfei@gmail.com wrote: > > > > From: dillon min <dillon.minfei@gmail.com> > > > > > > > > This is a driver for 320x240 TFT panels, accepting a rgb input > > > > streams that get adapted and scaled to the panel. > > > This driver is, I suppose, prepared to be a driver for ILI9341 based > > > panles, and as such not for a fixed resolution. > > > I expect (hope) we in the future will see more panels added. > > > > > > As i checked ili9341 datasheets, this panel just support 240x320 > > resolution only. if i'm wrong , please correct me. thanks > > https://cdn-shop.adafruit.com/datasheets/ILI9341.pdf > > > > This panel can support 9 different kinds of interface , "3/4-line Serial > > Interface" have been supported by tiny/ili9341.c. i was verified it > > but the performance on stm32f4 is very low. > It had somehow escaped my mind we already have a driver for ili9341 > based panels. > And we do not want onyl one driver for this IC. > So please extent the current driver to match your panel as well as > the original panel. > And then also extend the current binding for ili9341, which > you may have to convert to DT Schema in the process. > > This is more work now but when it is done we end up with a much better > solution than if we introduce an additional driver for ili9341. > > Sorry for not catching this in the inital review. > > Sam > > Hi Sam, Yes, it's should be more reasonable to support different interface of a panel in one driver. i will add mipi dpi & dbi support to this panel in one driver just like panel-simple.c(dpi & dsi) one question, what to do next: 1 should i just add code to tiny/ili9341.c to support mipi dpi. which means remove panel/panel-ilitek-ili9341.c 2 merge tiny/ili9341.c mipi dbi support to panel/panel-ilitek-ili9341.c , which means remove tiny/ili9341.c after this work done. to extend dts binding for different interface. Ie. original: 164 static const struct of_device_id ili9341_of_match[] = { 165 { .compatible = "adafruit,yx240qv29" }, 166 { } 167 }; 168 MODULE_DEVICE_TABLE(of, ili9341_of_match); 169 170 static const struct spi_device_id ili9341_id[] = { 171 { "yx240qv29", 0 }, 172 { } 173 }; 174 MODULE_DEVICE_TABLE(spi, ili9341_id); i want to extend of_device_id and spi_device_id for driver to identify dpi or dbi in probe. "adafruit,yx240qv29" is for users who use original tiny drm driver mipi dbi compatible for panel's parallel mcu and parallel rgb interface. static const struct of_device_id ili9341_of_match[] = { { .compatible = "adafruit,yx240qv29" }, { .compatible = "ili9341,parallel-mcu" }, { .compatible = "ili9341,parallel-rgb" }, { } }; static const struct spi_device_id ili9341_id[] = { { "yx240qv29", 0 }, { "parallel-mcu", 0 }, { "parallel-rgb", 0 }, { } }; static int ili9341_probe(struct spi_device *spi) { ... const struct spi_device_id *id = spi_get_device_id(spi); if (!strcmp(id->name, "yx240qv29")) { mipi dbi register } else if (!strcmp(id->name, "parallel-rgb") || !strcmp(id->name, "parallel-mcu") { mipi dpi register } else { failed handle } .... } if this is a good idea to extend dts binding ? thanks, best regards. Dillon > > > > currently, i just have stm32f429-disco in hands, with 18-bit parallel rgb > > bus connected on this board. reference to panel-ilitek-ili9322 and > > panel-ilitek-ili9881 driver, i have some plan to rewrite this driver. > > > > 1 add your below comments in. > > 2 use dc-gpio, reset-gpio, rgb-interface-bits from dts to config panel > > interface. > > 3 drop regmap, porting drm_mipi_dbi's mipi_dbi_command to config panel > > paramter. like tiny/ili9341.c > > 4 support rgb-16, rgb-18 interface. > > 5 use optional regulator or power gpio to control panel's power, as panel > > power is always on for my board, so i can't test this part. could i add the > > code which can't be tested? > > 6 support rotation in panel config (currently , i rotate the screen by > > kernel cmdline paramter) > > > > if you have any other common panel configuration should be add , please > > inform me. > > > > thanks. > > > > > > > > Some things to fix, see comments in the follwoing. > > > > > > Sam > > > > > > > > > > > Signed-off-by: dillon min <dillon.minfei@gmail.com> > > > > --- > > > > drivers/gpu/drm/panel/Kconfig | 8 + > > > > drivers/gpu/drm/panel/Makefile | 1 + > > > > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 561 > > > +++++++++++++++++++++++++++ > > > > 3 files changed, 570 insertions(+) > > > > create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > > > > > > > diff --git a/drivers/gpu/drm/panel/Kconfig > > > b/drivers/gpu/drm/panel/Kconfig > > > > index a1723c1..e42692c 100644 > > > > --- a/drivers/gpu/drm/panel/Kconfig > > > > +++ b/drivers/gpu/drm/panel/Kconfig > > > > @@ -95,6 +95,14 @@ config DRM_PANEL_ILITEK_IL9322 > > > > Say Y here if you want to enable support for Ilitek IL9322 > > > > QVGA (320x240) RGB, YUV and ITU-T BT.656 panels. > > > > > > > > +config DRM_PANEL_ILITEK_IL9341 > > > ILI9341 - so the config name matches the name of the driver IC. > > > > > > > + tristate "Ilitek ILI9341 240x320 QVGA panels" > > > > + depends on OF && SPI > > > > + select REGMAP > > > > + help > > > > + Say Y here if you want to enable support for Ilitek IL9341 > > > > + QVGA (240x320) RGB panels. > > > See comment to the changelog, the driver is more generic - I assume. > > > So the wording here can be improved to express this. > > > > > > Add support RGB 16bits and RGB 18bits bus only ? > > > > > > + > > > > config DRM_PANEL_ILITEK_ILI9881C > > > > tristate "Ilitek ILI9881C-based panels" > > > > depends on OF > > > > diff --git a/drivers/gpu/drm/panel/Makefile > > > b/drivers/gpu/drm/panel/Makefile > > > > index 96a883c..d123543 100644 > > > > --- a/drivers/gpu/drm/panel/Makefile > > > > +++ b/drivers/gpu/drm/panel/Makefile > > > > @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PANEL_ELIDA_KD35T133) += > > > panel-elida-kd35t133.o > > > > obj-$(CONFIG_DRM_PANEL_FEIXIN_K101_IM2BA02) += > > > panel-feixin-k101-im2ba02.o > > > > obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += > > > panel-feiyang-fy07024di26a30d.o > > > > obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o > > > > +obj-$(CONFIG_DRM_PANEL_ILITEK_IL9341) += panel-ilitek-ili9341.o > > > > obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o > > > > obj-$(CONFIG_DRM_PANEL_INNOLUX_P079ZCA) += panel-innolux-p079zca.o > > > > obj-$(CONFIG_DRM_PANEL_JDI_LT070ME05000) += panel-jdi-lt070me05000.o > > > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > > b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > > > new file mode 100644 > > > > index 0000000..ec22d80 > > > > --- /dev/null > > > > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c > > > > @@ -0,0 +1,561 @@ > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > +/* > > > > + * Ilitek ILI9341 TFT LCD drm_panel driver. > > > > + * > > > > + * This panel can be configured to support: > > > > + * - 16-bit parallel RGB interface > > > The interface to ILI9341 is SPI, and the interface between the ILI9341 > > > and the panel is more of an itnernal thing. Or did I get this worng? > > > > > > SPI is for register configuration. RGB parallel for data transfer > > > > > + * > > > > + * Copyright (C) 2020 Dillon Min <dillon.minfei@gmail.com> > > > > + * Derived from drivers/drm/gpu/panel/panel-ilitek-ili9322.c > > > > + */ > > > > + > > > > +#include <linux/bitops.h> > > > > +#include <linux/gpio/consumer.h> > > > > +#include <linux/module.h> > > > > +#include <linux/of_device.h> > > > > +#include <linux/regmap.h> > > > > +#include <linux/regulator/consumer.h> > > > > +#include <linux/spi/spi.h> > > > > + > > > > +#include <video/mipi_display.h> > > > > +#include <video/of_videomode.h> > > > > +#include <video/videomode.h> > > > > + > > > > +#include <drm/drm_modes.h> > > > > +#include <drm/drm_panel.h> > > > > +#include <drm/drm_print.h> > > > > + > > > > +#define DEFAULT_SPI_SPEED 10000000 > > > > + > > > > > > Please use same case for hex numbers in the driver. > > > My personal preferences is lower-case. > > > > > > in next patch, spi speed will be configured by dts, not hardcode here. > > below register address will change to lower-case hex numbers. > > > > > > > > +#define ILI9341_SLEEP_OUT 0x11 /* Sleep out register */ > > > > +#define ILI9341_GAMMA 0x26 /* Gamma register */ > > > > +#define ILI9341_DISPLAY_OFF 0x28 /* Display off register */ > > > > +#define ILI9341_DISPLAY_ON 0x29 /* Display on register */ > > > > +#define ILI9341_COLUMN_ADDR 0x2A /* Colomn address register > > > */ > > > > +#define ILI9341_PAGE_ADDR 0x2B /* Page address register */ > > > > +#define ILI9341_GRAM 0x2C /* GRAM register */ > > > > +#define ILI9341_MAC 0x36 /* Memory Access Control > > > register*/ > > > > +#define ILI9341_PIXEL_FORMAT 0x3A /* Pixel Format register */ > > > > +#define ILI9341_WDB 0x51 /* Write Brightness Display > > > > + * register > > > > + */ > > > > +#define ILI9341_WCD 0x53 /* Write Control Display > > > > + * register > > > > + */ > > > > +#define ILI9341_RGB_INTERFACE 0xB0 /* RGB Interface Signal > > > Control */ > > > > +#define ILI9341_FRC 0xB1 /* Frame Rate Control > > > register */ > > > > +#define ILI9341_BPC 0xB5 /* Blanking Porch Control > > > > + * register > > > > + */ > > > > +#define ILI9341_DFC 0xB6 /* Display Function Control > > > > + * register > > > > + */ > > > > +#define ILI9341_POWER1 0xC0 /* Power Control 1 register > > > */ > > > > +#define ILI9341_POWER2 0xC1 /* Power Control 2 register > > > */ > > > > +#define ILI9341_VCOM1 0xC5 /* VCOM Control 1 register > > > */ > > > > +#define ILI9341_VCOM2 0xC7 /* VCOM Control 2 register > > > */ > > > > +#define ILI9341_POWERA 0xCB /* Power control A register > > > */ > > > > +#define ILI9341_POWERB 0xCF /* Power control B register > > > */ > > > > +#define ILI9341_PGAMMA 0xE0 /* Positive Gamma Correction > > > > + * register > > > > + */ > > > > +#define ILI9341_NGAMMA 0xE1 /* Negative Gamma Correction > > > > + * register > > > > + */ > > > > +#define ILI9341_DTCA 0xE8 /* Driver timing control A > > > */ > > > > +#define ILI9341_DTCB 0xEA /* Driver timing control B > > > */ > > > > +#define ILI9341_POWER_SEQ 0xED /* Power on sequence > > > register */ > > > > +#define ILI9341_3GAMMA_EN 0xF2 /* 3 Gamma enable register > > > */ > > > > +#define ILI9341_INTERFACE 0xF6 /* Interface control > > > register */ > > > > +#define ILI9341_PRC 0xF7 /* Pump ratio control > > > register */ > > > > + > > > > > > All the following should be const. > > > > > ok > > > > > > > Can any of the below be replaces by DEFINED constants? > > > > +static u8 ili9341_cmd0[] = {0xc3, 0x08, 0x50}; > > > > +static u8 ili9341_powerb[] = {0x00, 0xc1, 0x30}; > > > > +static u8 ili9341_power_seq[] = {0x64, 0x03, 0x12, 0x81}; > > > > +static u8 ili9341_dtca[] = {0x85, 0x00, 0x78}; > > > > +static u8 ili9341_powera[] = {0x39, 0x2c, 0x00, 0x34, 0x02}; > > > > +static u8 ili9341_prc[] = {0x20}; > > > > +static u8 ili9341_dtcb[] = {0x00, 0x00}; > > > > +static u8 ili9341_frc[] = {0x00, 0x1b}; > > > > +static u8 ili9341_dfc1[] = {0x0a, 0xa2}; > > > > +static u8 ili9341_power1[] = {0x10}; > > > > +static u8 ili9341_power2[] = {0x10}; > > > > +static u8 ili9341_vcom1[] = {0x45, 0x15}; > > > > +static u8 ili9341_vcom2[] = {0x90}; > > > > +static u8 ili9341_mac[] = {0xc8}; > > > > +static u8 ili9341_gamma_en[] = {0x00}; > > > > +static u8 ili9341_rgb_intr[] = {0xc2}; > > > > +static u8 ili9341_dfc2[] = {0x0a, 0xa7, 0x27, 0x04}; > > > > +static u8 ili9341_column_addr[] = {0x00, 0x00, 0x00, 0xef}; > > > > +static u8 ili9341_page_addr[] = {0x00, 0x00, 0x01, 0x3f}; > > > > +static u8 ili9341_intr[] = {0x01, 0x00, 0x06}; > > > > +static u8 ili9341_gamma[] = {0x01}; > > > > +static u8 ili9341_pgamma[] = {0x0f, 0x29, 0x24, 0x0c, 0x0e, 0x09, 0x4e, > > > 0x78, > > > > + 0x3c, 0x09, 0x13, 0x05, 0x17, 0x11, 0x00}; > > > > +static u8 ili9341_ngamma[] = {0x00, 0x16, 0x1b, 0x04, 0x11, 0x07, 0x31, > > > 0x33, > > > > + 0x42, 0x05, 0x0c, 0x0a, 0x28, 0x2f, 0x0f}; > > > > + > > > > +/** > > > > + * enum ili9341_input - the format of the incoming signal to the panel > > > > + * > > > > + * The panel can be connected to various input streams and four of them > > > can > > > > + * be selected by electronic straps on the display. However it is > > > possible > > > > + * to select another mode or override the electronic default with this > > > > + * setting. > > > > + */ > > > > +enum ili9341_input { > > > > + ILI9341_INPUT_PRGB_THROUGH = 0x0, > > > > + ILI9341_INPUT_PRGB_ALIGNED = 0x1, > > > > + ILI9341_INPUT_UNKNOWN = 0xf, > > > > +}; > > > > + > > > > +/** > > > > + * struct ili9341_config - the system specific ILI9341 configuration > > > > + * @width_mm: physical panel width [mm] > > > > + * @height_mm: physical panel height [mm] > > > > + * @input: the input/entry type used in this system, if this is set to > > > > + * ILI9341_INPUT_UNKNOWN the driver will try to figure it out by probing > > > > + * the hardware > > > > + * @dclk_active_high: data/pixel clock active high, data will be clocked > > > > + * in on the rising edge of the DCLK (this is usually the case). > > > > + * @de_active_high: DE (data entry) is active high > > > > + * @hsync_active_high: HSYNC is active high > > > > + * @vsync_active_high: VSYNC is active high > > > > + */ > > > > +struct ili9341_config { > > > > + u32 width_mm; > > > > + u32 height_mm; > > > > + enum ili9341_input input; > > > > + bool dclk_active_high; > > > > + bool de_active_high; > > > > + bool hsync_active_high; > > > > + bool vsync_active_high; > > > > +}; > > > > + > > > > +struct ili9341 { > > > > + struct device *dev; > > > > + const struct ili9341_config *conf; > > > > + struct drm_panel panel; > > > > + struct regmap *regmap; > > > > + struct gpio_desc *reset_gpio; > > > > + struct gpio_desc *dc_gpio; > > > > + enum ili9341_input input; > > > > > > > + struct videomode vm; > > > videomode is not used. So drop this field and drop the include files > > > that are no logner needed. > > > > > > ok > > > > > > +}; > > > > + > > > > +static inline struct ili9341 *panel_to_ili9341(struct drm_panel *panel) > > > > +{ > > > > + return container_of(panel, struct ili9341, panel); > > > > +} > > > > + > > > > +int ili9341_spi_transfer(struct spi_device *spi, u32 speed_hz, > > > > + u8 bpw, const void *buf, size_t len) > > > > +{ > > > > + size_t max_chunk = spi_max_transfer_size(spi); > > > > + struct spi_transfer tr = { > > > const? > > > > > > ok > > > > > > + .bits_per_word = bpw, > > > > + .speed_hz = speed_hz, > > > > + .len = len, > > > > + }; > > > > + struct spi_message m; > > > > + size_t chunk; > > > > + int ret; > > > > + > > > > + spi_message_init_with_transfers(&m, &tr, 1); > > > > + > > > > + while (len) { > > > > + chunk = min(len, max_chunk); > > > > + > > > > + tr.tx_buf = buf; > > > > + tr.len = chunk; > > > > + buf += chunk; > > > > + len -= chunk; > > > > + > > > > + ret = spi_sync(spi, &m); > > > > + if (ret) > > > > + return ret; > > > > + } > > > > + return 0; > > > > +} > > > > +static int ili9341_regmap_spi_write(void *context, const void *data, > > > > + size_t count) > > > > +{ > > > > + struct device *dev = context; > > > > + struct spi_device *spi = to_spi_device(dev); > > > > + struct ili9341 *ili = spi_get_drvdata(spi); > > > > + int ret = 0; > > > > + > > > > + gpiod_set_value_cansleep(ili->dc_gpio, 0); > > > > + > > > > + ret = ili9341_spi_transfer(spi, DEFAULT_SPI_SPEED, 8, data+0, 1); > > > > + if (ret || count == 1 || > > > > + ((u8 *)data)[0] == ILI9341_GRAM || > > > > + ((u8 *)data)[0] == ILI9341_DISPLAY_ON || > > > > + ((u8 *)data)[0] == ILI9341_SLEEP_OUT || > > > > + ((u8 *)data)[0] == ILI9341_DISPLAY_OFF) > > > > + return ret; > > > > + > > > > + gpiod_set_value_cansleep(ili->dc_gpio, 1); > > > > + > > > > + return ili9341_spi_transfer(spi, DEFAULT_SPI_SPEED, 8, data+1, > > > count-1); > > > > +} > > > > + > > > > +static int ili9341_regmap_spi_read(void *context, const void *reg, > > > > + size_t reg_size, void *val, size_t > > > val_size) > > > > +{ > > > > + return 0; > > > > +} > > > Is this function really needed? If not delete it. > > > > > > regmap will be drop in next patch. > > > > > > + > > > > +static struct regmap_bus ili9341_regmap_bus = { > > > > + .write = ili9341_regmap_spi_write, > > > > + .read = ili9341_regmap_spi_read, > > > > + .reg_format_endian_default = REGMAP_ENDIAN_BIG, > > > > + .val_format_endian_default = REGMAP_ENDIAN_BIG, > > > > +}; > > > > + > > > > +static bool ili9341_volatile_reg(struct device *dev, unsigned int reg) > > > > +{ > > > > + return false; > > > > +} > > > Is this function really nedded? If not delete it. > > > > > > > + > > > > +static bool ili9341_writeable_reg(struct device *dev, unsigned int reg) > > > > +{ > > > > + /* Just register 0 is read-only */ > > > > + if (reg == 0x00) > > > > + return false; > > > > + return true; > > > > +} > > > > + > > > > +static const struct regmap_config ili9341_regmap_config = { > > > > + .reg_bits = 8, > > > > + .val_bits = 8, > > > > + .max_register = 0xff, > > > > + .cache_type = REGCACHE_RBTREE, > > > > + .volatile_reg = ili9341_volatile_reg, > > > > + .writeable_reg = ili9341_writeable_reg, > > > > +}; > > > > + > > > > > > No error checks - consider something like: > > > > > > static int bulk_write(struct ili9341 *ili, u8 reg, const u8[] data, int > > > len) > > > { > > > int err = ili->err; > > > > > > if (!err) { > > > err = regmap_bulk_write(ili->regmap, reg, data, len); > > > if (err) { > > > dev_err(...); > > > ili->err = err; > > > } > > > } > > > > > > return err; > > > } > > > > > > Then you can use this in the following, and make this more readable. > > > > > > ok, thanks for detail guide. > > > > > > > > +static int ili9341_init(struct drm_panel *panel, struct ili9341 *ili) > > > > +{ > > > > + regmap_bulk_write(ili->regmap, 0xca, > > > > + ili9341_cmd0, > > > sizeof(ili9341_cmd0)); > > > > + regmap_bulk_write(ili->regmap, ILI9341_POWERB, > > > > + ili9341_powerb, sizeof(ili9341_powerb)); > > > > + regmap_bulk_write(ili->regmap, ILI9341_POWER_SEQ, > > > > + ili9341_power_seq, > > > sizeof(ili9341_power_seq)); > > > > + regmap_bulk_write(ili->regmap, ILI9341_DTCA, > > > > + ili9341_dtca, sizeof(ili9341_dtca)); > > > > + regmap_bulk_write(ili->regmap, ILI9341_POWERA, > > > > + ili9341_powera, sizeof(ili9341_powera)); > > > > + regmap_write(ili->regmap, ILI9341_PRC, ili9341_prc[0]); > > > > + regmap_bulk_write(ili->regmap, ILI9341_DTCB, > > > > + ili9341_dtcb, sizeof(ili9341_dtcb)); > > > > + regmap_bulk_write(ili->regmap, ILI9341_FRC, > > > > + ili9341_frc, sizeof(ili9341_frc)); > > > > + regmap_bulk_write(ili->regmap, ILI9341_DFC, > > > > + ili9341_dfc1, sizeof(ili9341_dfc1)); > > > > + regmap_write(ili->regmap, ILI9341_POWER1, ili9341_power1[0]); > > > > + regmap_write(ili->regmap, ILI9341_POWER2, ili9341_power2[0]); > > > > + regmap_bulk_write(ili->regmap, ILI9341_VCOM1, > > > > + ili9341_vcom1, sizeof(ili9341_vcom1)); > > > > + regmap_write(ili->regmap, ILI9341_VCOM2, ili9341_vcom2[0]); > > > > + regmap_write(ili->regmap, ILI9341_MAC, ili9341_mac[0]); > > > > + regmap_write(ili->regmap, ILI9341_3GAMMA_EN, ili9341_gamma_en[0]); > > > > + regmap_write(ili->regmap, ILI9341_RGB_INTERFACE, > > > ili9341_rgb_intr[0]); > > > > + regmap_bulk_write(ili->regmap, ILI9341_DFC, > > > > + ili9341_dfc2, sizeof(ili9341_dfc2)); > > > > + > > > > + /* colomn address set */ > > > > + regmap_bulk_write(ili->regmap, ILI9341_COLUMN_ADDR, > > > > + ili9341_column_addr, sizeof(ili9341_column_addr)); > > > > + > > > > + /* Page Address Set */ > > > > + regmap_bulk_write(ili->regmap, ILI9341_PAGE_ADDR, > > > > + ili9341_page_addr, > > > sizeof(ili9341_page_addr)); > > > > + regmap_bulk_write(ili->regmap, ILI9341_INTERFACE, > > > > + ili9341_intr, sizeof(ili9341_intr)); > > > > + regmap_write(ili->regmap, ILI9341_GRAM, 0); > > > > + msleep(200); > > > > + > > > > + regmap_write(ili->regmap, ILI9341_GAMMA, ili9341_gamma[0]); > > > > + regmap_bulk_write(ili->regmap, ILI9341_PGAMMA, > > > > + ili9341_pgamma, sizeof(ili9341_pgamma)); > > > > + regmap_bulk_write(ili->regmap, ILI9341_NGAMMA, > > > > + ili9341_ngamma, sizeof(ili9341_ngamma)); > > > > + regmap_write(ili->regmap, ILI9341_SLEEP_OUT, 0); > > > > + msleep(200); > > > > + > > > > + regmap_write(ili->regmap, ILI9341_DISPLAY_ON, 0); > > > > + > > > > + /* GRAM start writing */ > > > > + regmap_write(ili->regmap, ILI9341_GRAM, 0); > > > > + > > > > + dev_info(ili->dev, "initialized display\n"); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +/* > > > > + * This power-on sequence if from the datasheet, page 57. > > > > + */ > > > > +static int ili9341_power_on(struct ili9341 *ili) > > > > +{ > > > > + /* Assert RESET */ > > > > + gpiod_set_value(ili->reset_gpio, 1); > > > > + > > > > + msleep(20); > > > > + > > > > + /* De-assert RESET */ > > > > + gpiod_set_value(ili->reset_gpio, 0); > > > > + > > > > + msleep(10); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int ili9341_power_off(struct ili9341 *ili) > > > > +{ > > > > > > Assert reset? > > > will add reset-pin control here. > > > > > > > > > > > + return 0; > > > > +} > > > > + > > > > +static int ili9341_disable(struct drm_panel *panel) > > > > +{ > > > > + struct ili9341 *ili = panel_to_ili9341(panel); > > > > + int ret; > > > > + > > > > + ret = regmap_write(ili->regmap, ILI9341_DISPLAY_OFF, 0); > > > > + if (ret) { > > > > + dev_err(ili->dev, "unable to go to standby mode\n"); > > > > + return ret; > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int ili9341_unprepare(struct drm_panel *panel) > > > > +{ > > > > + struct ili9341 *ili = panel_to_ili9341(panel); > > > > + > > > > + return ili9341_power_off(ili); > > > > +} > > > > + > > > > +static int ili9341_prepare(struct drm_panel *panel) > > > > +{ > > > > + struct ili9341 *ili = panel_to_ili9341(panel); > > > > + int ret; > > > > + > > > > + ret = ili9341_power_on(ili); > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > + ret = ili9341_init(panel, ili); > > > > + if (ret < 0) > > > > + ili9341_unprepare(panel); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static int ili9341_enable(struct drm_panel *panel) > > > > +{ > > > > + struct ili9341 *ili = panel_to_ili9341(panel); > > > > + int ret; > > > > + > > > > + ret = regmap_write(ili->regmap, ILI9341_DISPLAY_ON, 0); > > > > + if (ret) { > > > > + dev_err(ili->dev, "unable to enable panel\n"); > > > > + return ret; > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +/* This is the only mode listed for parallel RGB in the datasheet */ > > > > +static const struct drm_display_mode prgb_320x240_mode = { > > > > + .clock = 6100, > > > > + .hdisplay = 240, > > > > + .hsync_start = 240 + 10, > > > > + .hsync_end = 240 + 10 + 10, > > > > + .htotal = 280, > > > > + .vdisplay = 320, > > > > + .vsync_start = 320 + 4, > > > > + .vsync_end = 320 + 4 + 2, > > > > + .vtotal = 328, > > > > + .vrefresh = 60, > > > > + .flags = 0, > > > > +}; > > > > + > > > > +static int ili9341_get_modes(struct drm_panel *panel, > > > > + struct drm_connector *connector) > > > > +{ > > > > + struct ili9341 *ili = panel_to_ili9341(panel); > > > > + struct drm_device *drm = connector->dev; > > > > + struct drm_display_mode *mode; > > > > + struct drm_display_info *info; > > > > + > > > > + info = &connector->display_info; > > > > + info->width_mm = ili->conf->width_mm; > > > > + info->height_mm = ili->conf->height_mm; > > > > + if (ili->conf->dclk_active_high) > > > > + info->bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE; > > > > + else > > > > + info->bus_flags |= DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE; > > > > + > > > > + if (ili->conf->de_active_high) > > > > + info->bus_flags |= DRM_BUS_FLAG_DE_HIGH; > > > > + else > > > > + info->bus_flags |= DRM_BUS_FLAG_DE_LOW; > > > > + > > > > + switch (ili->input) { > > > > + case ILI9341_INPUT_PRGB_THROUGH: > > > > + case ILI9341_INPUT_PRGB_ALIGNED: > > > > + mode = drm_mode_duplicate(drm, &prgb_320x240_mode); > > > > + break; > > > > + default: > > > > + mode = NULL; > > > > + break; > > > > + } > > > > + if (!mode) { > > > > + DRM_ERROR("bad mode or failed to add mode\n"); > > > > + return -EINVAL; > > > > + } > > > > + drm_mode_set_name(mode); > > > > + /* > > > > + * This is the preferred mode because most people are going > > > > + * to want to use the display with VGA type graphics. > > > > + */ > > > > + mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED; > > > > + > > > > + /* Set up the polarity */ > > > > + if (ili->conf->hsync_active_high) > > > > + mode->flags |= DRM_MODE_FLAG_PHSYNC; > > > > + else > > > > + mode->flags |= DRM_MODE_FLAG_NHSYNC; > > > > + if (ili->conf->vsync_active_high) > > > > + mode->flags |= DRM_MODE_FLAG_PVSYNC; > > > > + else > > > > + mode->flags |= DRM_MODE_FLAG_NVSYNC; > > > > + > > > > + mode->width_mm = ili->conf->width_mm; > > > > + mode->height_mm = ili->conf->height_mm; > > > > + drm_mode_probed_add(connector, mode); > > > > + > > > > + return 1; /* Number of modes */ > > > > +} > > > > + > > > > +static const struct drm_panel_funcs ili9341_drm_funcs = { > > > > + .disable = ili9341_disable, > > > > + .unprepare = ili9341_unprepare, > > > > + .prepare = ili9341_prepare, > > > > + .enable = ili9341_enable, > > > > + .get_modes = ili9341_get_modes, > > > > +}; > > > > + > > > > +static int ili9341_probe(struct spi_device *spi) > > > > +{ > > > > + struct device *dev = &spi->dev; > > > > + struct ili9341 *ili; > > > > + const struct regmap_config *regmap_config; > > > > + int ret; > > > > + > > > > + ili = devm_kzalloc(dev, sizeof(struct ili9341), GFP_KERNEL); > > > > + if (!ili) > > > > + return -ENOMEM; > > > > + > > > > + spi_set_drvdata(spi, ili); > > > > + > > > > + ili->dev = dev; > > > > + /* > > > > + * Every new incarnation of this display must have a unique > > > > + * data entry for the system in this driver. > > > > + */ > > > > + ili->conf = of_device_get_match_data(dev); > > > > + if (!ili->conf) { > > > > + dev_err(dev, "missing device configuration\n"); > > > > + return -ENODEV; > > > > + } > > > > + > > > > + ili->reset_gpio = devm_gpiod_get_optional(dev, "reset", > > > GPIOD_OUT_HIGH); > > > > + if (IS_ERR(ili->reset_gpio)) { > > > > + dev_err(dev, "failed to get RESET GPIO\n"); > > > > + return PTR_ERR(ili->reset_gpio); > > > > + } > > > > + > > > > + ili->dc_gpio = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW); > > > > + if (IS_ERR(ili->dc_gpio)) { > > > > + dev_err(dev, "failed to get DC GPIO\n"); > > > > + return PTR_ERR(ili->dc_gpio); > > > > + } > > > > + > > > > + spi->bits_per_word = 8; > > > > + ret = spi_setup(spi); > > > > + if (ret < 0) { > > > > + dev_err(dev, "spi setup failed.\n"); > > > > + return ret; > > > > + } > > > > + > > > > + regmap_config = &ili9341_regmap_config; > > > > + > > > > + ili->regmap = devm_regmap_init(dev, &ili9341_regmap_bus, dev, > > > > + regmap_config); > > > > + if (IS_ERR(ili->regmap)) { > > > > + dev_err(dev, "failed to allocate register map\n"); > > > > + return PTR_ERR(ili->regmap); > > > > + } > > > > + > > > > + ili->input = ili->conf->input; > > > > + > > > > + drm_panel_init(&ili->panel, dev, &ili9341_drm_funcs, > > > > + DRM_MODE_CONNECTOR_DPI); > > > > + > > > > + return drm_panel_add(&ili->panel); > > > > +} > > > > + > > > > +static int ili9341_remove(struct spi_device *spi) > > > > +{ > > > > + struct ili9341 *ili = spi_get_drvdata(spi); > > > > + > > > > + ili9341_power_off(ili); > > > > + drm_panel_remove(&ili->panel); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +/* > > > > + * The Stm32f429-disco board has a panel ili9341 connected to ltdc > > > controller > > > > + */ > > > > +static const struct ili9341_config ili9341_data = { > > > This should be named "disco" something as this is m32f429-disco > > > specific. > > > > > > ok > > > > > > + .width_mm = 65, > > > > + .height_mm = 50, > > > > + .input = ILI9341_INPUT_PRGB_THROUGH, > > > > + .dclk_active_high = true, > > > > + .de_active_high = false, > > > > + .hsync_active_high = false, > > > > + .vsync_active_high = false, > > > > +}; > > > > + > > > > +static const struct of_device_id ili9341_of_match[] = { > > > > + { > > > > + .compatible = "stm32f429,ltdc-panel", > > > > + .data = &ili9341_data, > > > > + }, > > > > > > > > > > + { > > > > + .compatible = "ilitek,ili9341", > > > > + .data = NULL, > > > This part is wrong, as ilitek,ili9341 is just the generic part. > > > Only the first entry is relevant. > > > > > > > > > ok > > > > > > + }, > > > > + { } > > > > +}; > > > > +MODULE_DEVICE_TABLE(of, ili9341_of_match); > > > > + > > > > +static struct spi_driver ili9341_driver = { > > > > + .probe = ili9341_probe, > > > > + .remove = ili9341_remove, > > > > + .driver = { > > > > + .name = "panel-ilitek-ili9341", > > > > + .of_match_table = ili9341_of_match, > > > > + }, > > > > +}; > > > > +module_spi_driver(ili9341_driver); > > > > + > > > > +MODULE_AUTHOR("Dillon Min <dillon.minfei@gmail.com>"); > > > > +MODULE_DESCRIPTION("ILI9341 LCD panel driver"); > > > > +MODULE_LICENSE("GPL v2"); > > > > -- > > > > 2.7.4 > > > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-05-10 4:04 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-08 4:13 [PATCH v2 0/5] Enable ilitek ili9341 on stm32f429-disco board dillon.minfei
2020-05-08 4:13 ` [PATCH v2 1/5] ARM: dts: stm32: Add pin map for ltdc, spi5 " dillon.minfei
2020-05-08 4:13 ` [PATCH v2 2/5] ARM: dts: stm32: enable ltdc binding with ili9341 on stm32429-disco board dillon.minfei
2020-05-08 4:13 ` [PATCH v2 3/5] dt-bindings: display: panel: Add ilitek ili9341 panel bindings dillon.minfei
2020-05-08 4:13 ` [PATCH v2 4/5] clk: stm32: Fix stm32f429 ltdc driver loading hang in clk set rate. keep ltdc clk running after kernel startup dillon.minfei
2020-05-08 4:13 ` [PATCH v2 5/5] drm/panel: add panel driver for Ilitek ili9341 panels dillon.minfei
2020-05-08 9:02 ` Sam Ravnborg
[not found] ` <CAL9mu0LcT3pPiM6PdJRvAJRMbXY6KACF1wNg8yOS-6D8aq6AeA@mail.gmail.com>
2020-05-09 8:31 ` dillon min
2020-05-09 20:06 ` Sam Ravnborg
2020-05-10 4:04 ` dillon min
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).