The series adds display support for the stm32f769-disco board. It has been
tested on hardware revisions MB1225-B03 and MB1166-A09. This required
modifications to the nt35510 driver. As I do not have the Hydis HVA40WV1
display, it would be better if someone tested the driver in that
configuration.
Changes in v4:
- Put the "enum" list in alphabetical order
Changes in v3:
- Use "enum" to have less code changed
Changes in v2:
- Add Acked-by tag of Conor Dooley
- Add a dash in front of each "items:"
- Change the status of panel_backlight node to "disabled"
- Delete backlight property from panel0 node.
- Re-write the patch [7/8] "drm/panel: nt35510: refactor panel initialization"
in the same style as the original driver in order to maintain the same
structure.
- Re-write the patch [8/8] "drm/panel: nt35510: support FRIDA FRD400B25025-A-CTK"
in the same style as the original driver.
Dario Binacchi (8):
dt-bindings: mfd: stm32f7: Add binding definition for DSI
ARM: dts: stm32: add DSI support on stm32f769
ARM: dts: stm32: rename mmc_vcard to vcc-3v3 on stm32f769-disco
ARM: dts: stm32: add display support on stm32f769-disco
dt-bindings: nt35510: add compatible for FRIDA FRD400B25025-A-CTK
ARM: dts: add stm32f769-disco-mb1225-revb03-mb1166-reva09
drm/panel: nt35510: move hardwired parameters to configuration
drm/panel: nt35510: support FRIDA FRD400B25025-A-CTK
.../display/panel/novatek,nt35510.yaml | 4 +-
arch/arm/boot/dts/st/Makefile | 1 +
...f769-disco-mb1225-revb03-mb1166-reva09.dts | 18 +
arch/arm/boot/dts/st/stm32f769-disco.dts | 78 +++-
arch/arm/boot/dts/st/stm32f769.dtsi | 21 +
drivers/gpu/drm/panel/panel-novatek-nt35510.c | 422 +++++++++++++++---
include/dt-bindings/mfd/stm32f7-rcc.h | 1 +
7 files changed, 484 insertions(+), 61 deletions(-)
create mode 100644 arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
create mode 100644 arch/arm/boot/dts/st/stm32f769.dtsi
--
2.43.0
Add support for MIPI DSI Host controller. Since MIPI DSI is not
available on stm32f746, the patch adds the "stm32f769.dtsi" file
containing the dsi node inside.
Signed-off-by: Dario Binacchi <[email protected]>
---
(no changes since v1)
arch/arm/boot/dts/st/stm32f769.dtsi | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 arch/arm/boot/dts/st/stm32f769.dtsi
diff --git a/arch/arm/boot/dts/st/stm32f769.dtsi b/arch/arm/boot/dts/st/stm32f769.dtsi
new file mode 100644
index 000000000000..e09184f7079c
--- /dev/null
+++ b/arch/arm/boot/dts/st/stm32f769.dtsi
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 Dario Binacchi <[email protected]>
+ */
+
+#include "stm32f746.dtsi"
+
+/ {
+ soc {
+ dsi: dsi@40016c00 {
+ compatible = "st,stm32-dsi";
+ reg = <0x40016c00 0x800>;
+ interrupts = <98>;
+ clocks = <&rcc 1 CLK_F769_DSI>, <&clk_hse>;
+ clock-names = "pclk", "ref";
+ resets = <&rcc STM32F7_APB2_RESET(DSI)>;
+ reset-names = "apb";
+ status = "disabled";
+ };
+ };
+};
--
2.43.0
Add binding definition for MIPI DSI Host controller.
Signed-off-by: Dario Binacchi <[email protected]>
Acked-by: Conor Dooley <[email protected]>
---
(no changes since v2)
Changes in v2:
- Add Acked-by tag of Conor Dooley
include/dt-bindings/mfd/stm32f7-rcc.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/dt-bindings/mfd/stm32f7-rcc.h b/include/dt-bindings/mfd/stm32f7-rcc.h
index 8d73a9c51e2b..a4e4f9271395 100644
--- a/include/dt-bindings/mfd/stm32f7-rcc.h
+++ b/include/dt-bindings/mfd/stm32f7-rcc.h
@@ -108,6 +108,7 @@
#define STM32F7_RCC_APB2_SAI1 22
#define STM32F7_RCC_APB2_SAI2 23
#define STM32F7_RCC_APB2_LTDC 26
+#define STM32F7_RCC_APB2_DSI 27
#define STM32F7_APB2_RESET(bit) (STM32F7_RCC_APB2_##bit + (0x24 * 8))
#define STM32F7_APB2_CLOCK(bit) (STM32F7_RCC_APB2_##bit + 0xA0)
--
2.43.0
In the schematics of document UM2033, the power supply for the micro SD
card is the same 3v3 voltage that is used to power other devices on the
board. By generalizing the name of the voltage regulator, it can be
referenced by other nodes in the device tree without creating
misunderstandings.
This patch is preparatory for future developments.
Signed-off-by: Dario Binacchi <[email protected]>
---
(no changes since v1)
arch/arm/boot/dts/st/stm32f769-disco.dts | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/st/stm32f769-disco.dts b/arch/arm/boot/dts/st/stm32f769-disco.dts
index 5d12ae25b327..8632bd866272 100644
--- a/arch/arm/boot/dts/st/stm32f769-disco.dts
+++ b/arch/arm/boot/dts/st/stm32f769-disco.dts
@@ -92,9 +92,9 @@ usbotg_hs_phy: usb-phy {
clock-names = "main_clk";
};
- mmc_vcard: mmc_vcard {
+ vcc_3v3: vcc_3v3 {
compatible = "regulator-fixed";
- regulator-name = "mmc_vcard";
+ regulator-name = "vcc_3v3";
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;
};
@@ -128,7 +128,7 @@ &rtc {
&sdio2 {
status = "okay";
- vmmc-supply = <&mmc_vcard>;
+ vmmc-supply = <&vcc_3v3>;
cd-gpios = <&gpioi 15 GPIO_ACTIVE_LOW>;
broken-cd;
pinctrl-names = "default", "opendrain", "sleep";
--
2.43.0
The patch adds display support on the stm32f769-disco board.
Signed-off-by: Dario Binacchi <[email protected]>
---
(no changes since v1)
arch/arm/boot/dts/st/stm32f769-disco.dts | 72 +++++++++++++++++++++++-
1 file changed, 71 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/st/stm32f769-disco.dts b/arch/arm/boot/dts/st/stm32f769-disco.dts
index 8632bd866272..d1eb5f9c78bf 100644
--- a/arch/arm/boot/dts/st/stm32f769-disco.dts
+++ b/arch/arm/boot/dts/st/stm32f769-disco.dts
@@ -41,7 +41,7 @@
*/
/dts-v1/;
-#include "stm32f746.dtsi"
+#include "stm32f769.dtsi"
#include "stm32f769-pinctrl.dtsi"
#include <dt-bindings/input/input.h>
#include <dt-bindings/gpio/gpio.h>
@@ -60,6 +60,19 @@ memory@c0000000 {
reg = <0xC0000000 0x1000000>;
};
+ reserved-memory {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ linux,dma {
+ compatible = "shared-dma-pool";
+ linux,dma-default;
+ no-map;
+ size = <0x100000>;
+ };
+ };
+
aliases {
serial0 = &usart1;
};
@@ -85,6 +98,13 @@ button-0 {
};
};
+ panel_backlight: panel-backlight {
+ compatible = "gpio-backlight";
+ gpios = <&gpioi 14 GPIO_ACTIVE_HIGH>;
+ default-on;
+ status = "okay";
+ };
+
usbotg_hs_phy: usb-phy {
#phy-cells = <0>;
compatible = "usb-nop-xceiv";
@@ -114,6 +134,46 @@ &clk_hse {
clock-frequency = <25000000>;
};
+&dsi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dsi_in: endpoint {
+ remote-endpoint = <<dc_out_dsi>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ dsi_out: endpoint {
+ remote-endpoint = <&dsi_panel_in>;
+ };
+ };
+ };
+
+ panel0: panel-dsi@0 {
+ compatible = "orisetech,otm8009a";
+ reg = <0>; /* dsi virtual channel (0..3) */
+ reset-gpios = <&gpioj 15 GPIO_ACTIVE_LOW>;
+ power-supply = <&vcc_3v3>;
+ backlight = <&panel_backlight>;
+ status = "okay";
+
+ port {
+ dsi_panel_in: endpoint {
+ remote-endpoint = <&dsi_out>;
+ };
+ };
+ };
+};
+
&i2c1 {
pinctrl-0 = <&i2c1_pins_b>;
pinctrl-names = "default";
@@ -122,6 +182,16 @@ &i2c1 {
status = "okay";
};
+<dc {
+ status = "okay";
+
+ port {
+ ltdc_out_dsi: endpoint@0 {
+ remote-endpoint = <&dsi_in>;
+ };
+ };
+};
+
&rtc {
status = "okay";
};
--
2.43.0
The patch adds the FRIDA FRD400B25025-A-CTK panel, which belongs to the
Novatek NT35510-based panel family.
Signed-off-by: Dario Binacchi <[email protected]>
---
Changes in v4:
- Put the "enum" list in alphabetical order
Changes in v3:
- Use "enum" to have less code changed
Changes in v2:
- Add a dash in front of each "items:"
.../devicetree/bindings/display/panel/novatek,nt35510.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/panel/novatek,nt35510.yaml b/Documentation/devicetree/bindings/display/panel/novatek,nt35510.yaml
index bc92928c805b..a4afaff483b7 100644
--- a/Documentation/devicetree/bindings/display/panel/novatek,nt35510.yaml
+++ b/Documentation/devicetree/bindings/display/panel/novatek,nt35510.yaml
@@ -15,7 +15,9 @@ allOf:
properties:
compatible:
items:
- - const: hydis,hva40wv1
+ - enum:
+ - frida,frd400b25025
+ - hydis,hva40wv1
- const: novatek,nt35510
description: This indicates the panel manufacturer of the panel
that is in turn using the NT35510 panel driver. The compatible
--
2.43.0
As reported in the section 8.3 (i. e. Board revision history) of document
UM2033 (i. e. Discovery kit with STM32F769NI MCU) these are the changes
related to the board revisions addressed by the patch:
- Board MB1225 revision B-03:
- Memory MICRON MT48LC4M32B2B5-6A replaced by ISSI IS42S32400F-6BL
- Board MB1166 revision A-09:
- LCD FRIDA FRD397B25009-D-CTK replaced by FRIDA FRD400B25025-A-CTK
The patch only adds the DTS support for the new display which belongs to
to the Novatek NT35510-based panel family.
Signed-off-by: Dario Binacchi <[email protected]>
---
(no changes since v2)
Changes in v2:
- Change the status of panel_backlight node to "disabled"
- Delete backlight property from panel0 node.
arch/arm/boot/dts/st/Makefile | 1 +
...2f769-disco-mb1225-revb03-mb1166-reva09.dts | 18 ++++++++++++++++++
2 files changed, 19 insertions(+)
create mode 100644 arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
diff --git a/arch/arm/boot/dts/st/Makefile b/arch/arm/boot/dts/st/Makefile
index 7892ad69b441..390dbd300a57 100644
--- a/arch/arm/boot/dts/st/Makefile
+++ b/arch/arm/boot/dts/st/Makefile
@@ -23,6 +23,7 @@ dtb-$(CONFIG_ARCH_STM32) += \
stm32f469-disco.dtb \
stm32f746-disco.dtb \
stm32f769-disco.dtb \
+ stm32f769-disco-mb1225-revb03-mb1166-reva09.dts \
stm32429i-eval.dtb \
stm32746g-eval.dtb \
stm32h743i-eval.dtb \
diff --git a/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts b/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
new file mode 100644
index 000000000000..014cac192375
--- /dev/null
+++ b/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 Dario Binacchi <[email protected]>
+ */
+
+#include "stm32f769-disco.dts"
+
+&panel_backlight {
+ status = "disabled";
+};
+
+&panel0 {
+ compatible = "frida,frd400b25025", "novatek,nt35510";
+ vddi-supply = <&vcc_3v3>;
+ vdd-supply = <&vcc_3v3>;
+ /delete-property/backlight;
+ /delete-property/power-supply;
+};
--
2.43.0
This patch, preparatory for future developments, move the hardwired
parameters to configuration data to allow the addition of new
NT35510-based panels.
Signed-off-by: Dario Binacchi <[email protected]>
---
(no changes since v2)
Changes in v2:
- Re-write the patch [7/8] "drm/panel: nt35510: refactor panel initialization"
in the same style as the original driver in order to maintain the same
structure.
drivers/gpu/drm/panel/panel-novatek-nt35510.c | 140 ++++++++++++++----
1 file changed, 115 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
index d6dceb858008..ce8969f48286 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
@@ -171,6 +171,10 @@ struct nt35510_config {
* timing in the display controller.
*/
const struct drm_display_mode mode;
+ /**
+ * @mode_flags: DSI operation mode related flags
+ */
+ unsigned long mode_flags;
/**
* @avdd: setting for AVDD ranging from 0x00 = 6.5V to 0x14 = 4.5V
* in 0.1V steps the default is 0x05 which means 6.0V
@@ -273,6 +277,100 @@ struct nt35510_config {
* same layout of bytes as @vgp.
*/
u8 vgn[NT35510_P1_VGN_LEN];
+ /**
+ * @dopctr: setting optional control for display
+ * ERR bits 0..1 in the first byte is the ERR pin output signal setting.
+ * 0 = Disable, ERR pin output low
+ * 1 = ERR pin output CRC error only
+ * 2 = ERR pin output ECC error only
+ * 3 = ERR pin output CRC and ECC error
+ * The default is 0.
+ * N565 bit 2 in the first byte is the 16-bit/pixel format selection.
+ * 0 = R[4:0] + G[5:3] & G[2:0] + B[4:0]
+ * 1 = G[2:0] + R[4:0] & B[4:0] + G[5:3]
+ * The default is 0.
+ * DIS_EoTP_HS bit 3 in the first byte is "DSI protocol violation" error
+ * reporting.
+ * 0 = reporting when error
+ * 1 = not reporting when error
+ * DSIM bit 4 in the first byte is the video mode data type enable
+ * 0 = Video mode data type disable
+ * 1 = Video mode data type enable
+ * The default is 0.
+ * DSIG bit 5 int the first byte is the generic r/w data type enable
+ * 0 = Generic r/w disable
+ * 1 = Generic r/w enable
+ * The default is 0.
+ * DSITE bit 6 in the first byte is TE line enable
+ * 0 = TE line is disabled
+ * 1 = TE line is enabled
+ * The default is 0.
+ * RAMKP bit 7 in the first byte is the frame memory keep/loss in
+ * sleep-in mode
+ * 0 = contents loss in sleep-in
+ * 1 = contents keep in sleep-in
+ * The default is 0.
+ * CRL bit 1 in the second byte is the source driver data shift
+ * direction selection. This bit is XOR operation with bit RSMX
+ * of 3600h command.
+ * 0 (RMSX = 0) = S1 -> S1440
+ * 0 (RMSX = 1) = S1440 -> S1
+ * 1 (RMSX = 0) = S1440 -> S1
+ * 1 (RMSX = 1) = S1 -> S1440
+ * The default is 0.
+ * CTB bit 2 in the second byte is the vertical scanning direction
+ * selection for gate control signals. This bit is XOR operation
+ * with bit ML of 3600h command.
+ * 0 (ML = 0) = Forward (top -> bottom)
+ * 0 (ML = 1) = Reverse (bottom -> top)
+ * 1 (ML = 0) = Reverse (bottom -> top)
+ * 1 (ML = 1) = Forward (top -> bottom)
+ * The default is 0.
+ * CRGB bit 3 in the second byte is RGB-BGR order selection. This
+ * bit is XOR operation with bit RGB of 3600h command.
+ * 0 (RGB = 0) = RGB/Normal
+ * 0 (RGB = 1) = BGR/RB swap
+ * 1 (RGB = 0) = BGR/RB swap
+ * 1 (RGB = 1) = RGB/Normal
+ * The default is 0.
+ * TE_PWR_SEL bit 4 in the second byte is the TE output voltage
+ * level selection (only valid when DSTB_SEL = 0 or DSTB_SEL = 1,
+ * VSEL = High and VDDI = 1.665~3.3V).
+ * 0 = TE output voltage level is VDDI
+ * 1 = TE output voltage level is VDDA
+ * The default is 0.
+ */
+ u8 dopctr[NT35510_P0_DOPCTR_LEN];
+ /**
+ * @madctl: Memory data access control
+ * RSMY bit 0 is flip vertical. Flips the display image top to down.
+ * RSMX bit 1 is flip horizontal. Flips the display image left to right.
+ * MH bit 2 is the horizontal refresh order.
+ * RGB bit 3 is the RGB-BGR order.
+ * 0 = RGB color sequence
+ * 1 = BGR color sequence
+ * ML bit 4 is the vertical refresh order.
+ * MV bit 5 is the row/column exchange.
+ * MX bit 6 is the column address order.
+ * MY bit 7 is the row address order.
+ */
+ u8 madctl;
+ /**
+ * @sdhdtctr: source output data hold time
+ * 0x00..0x3F = 0..31.5us in steps of 0.5us
+ * The default is 0x05 = 2.5us.
+ */
+ u8 sdhdtctr;
+ /**
+ * @gseqctr: EQ control for gate signals
+ * GFEQ_XX[3:0]: time setting of EQ step for falling edge in steps
+ * of 0.5us.
+ * The default is 0x07 = 3.5us
+ * GREQ_XX[7:4]: time setting of EQ step for rising edge in steps
+ * of 0.5us.
+ * The default is 0x07 = 3.5us
+ */
+ u8 gseqctr[NT35510_P0_GSEQCTR_LEN];
/**
* @sdeqctr: Source driver control settings, first byte is
* 0 for mode 1 and 1 for mode 2. Mode 1 uses two steps and
@@ -536,46 +634,28 @@ static int nt35510_setup_display(struct nt35510 *nt)
{
struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
const struct nt35510_config *conf = nt->conf;
- u8 dopctr[NT35510_P0_DOPCTR_LEN];
- u8 gseqctr[NT35510_P0_GSEQCTR_LEN];
u8 dpfrctr[NT35510_P0_DPFRCTR1_LEN];
- /* FIXME: set up any rotation (assume none for now) */
- u8 addr_mode = NT35510_ROTATE_0_SETTING;
- u8 val;
int ret;
- /* Enable TE, EoTP and RGB pixel format */
- dopctr[0] = NT35510_DOPCTR_0_DSITE | NT35510_DOPCTR_0_EOTP |
- NT35510_DOPCTR_0_N565;
- dopctr[1] = NT35510_DOPCTR_1_CTB;
ret = nt35510_send_long(nt, dsi, NT35510_P0_DOPCTR,
NT35510_P0_DOPCTR_LEN,
- dopctr);
+ conf->dopctr);
if (ret)
return ret;
- ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_ADDRESS_MODE, &addr_mode,
- sizeof(addr_mode));
+ ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_ADDRESS_MODE, &conf->madctl,
+ sizeof(conf->madctl));
if (ret < 0)
return ret;
- /*
- * Source data hold time, default 0x05 = 2.5us
- * 0x00..0x3F = 0 .. 31.5us in steps of 0.5us
- * 0x0A = 5us
- */
- val = 0x0A;
- ret = mipi_dsi_dcs_write(dsi, NT35510_P0_SDHDTCTR, &val,
- sizeof(val));
+ ret = mipi_dsi_dcs_write(dsi, NT35510_P0_SDHDTCTR, &conf->sdhdtctr,
+ sizeof(conf->sdhdtctr));
if (ret < 0)
return ret;
- /* EQ control for gate signals, 0x00 = 0 us */
- gseqctr[0] = 0x00;
- gseqctr[1] = 0x00;
ret = nt35510_send_long(nt, dsi, NT35510_P0_GSEQCTR,
NT35510_P0_GSEQCTR_LEN,
- gseqctr);
+ conf->gseqctr);
if (ret)
return ret;
@@ -896,7 +976,6 @@ static int nt35510_probe(struct mipi_dsi_device *dsi)
*/
dsi->hs_rate = 349440000;
dsi->lp_rate = 9600000;
- dsi->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS;
/*
* Every new incarnation of this display must have a unique
@@ -908,6 +987,8 @@ static int nt35510_probe(struct mipi_dsi_device *dsi)
return -ENODEV;
}
+ dsi->mode_flags = nt->conf->mode_flags;
+
nt->supplies[0].supply = "vdd"; /* 2.3-4.8 V */
nt->supplies[1].supply = "vddi"; /* 1.65-3.3V */
ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(nt->supplies),
@@ -1030,6 +1111,7 @@ static const struct nt35510_config nt35510_hydis_hva40wv1 = {
.vtotal = 800 + 2 + 0 + 5, /* VBP = 5 */
.flags = 0,
},
+ .mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS,
/* 0x09: AVDD = 5.6V */
.avdd = { 0x09, 0x09, 0x09 },
/* 0x34: PCK = Hsync/2, BTP = 2 x VDDB */
@@ -1050,6 +1132,14 @@ static const struct nt35510_config nt35510_hydis_hva40wv1 = {
.vgp = { 0x00, 0xA3, 0x00 },
/* VGMP: 0x0A3 = 5.0375V, VGSP = 0V */
.vgn = { 0x00, 0xA3, 0x00 },
+ /* Enable TE, EoTP and RGB pixel format */
+ .dopctr = { NT35510_DOPCTR_0_DSITE | NT35510_DOPCTR_0_EOTP |
+ NT35510_DOPCTR_0_N565, NT35510_DOPCTR_1_CTB },
+ .madctl = NT35510_ROTATE_180_SETTING,
+ /* 0x0A: SDT = 5 us */
+ .sdhdtctr = 0x0A,
+ /* EQ control for gate signals, 0x00 = 0 us */
+ .gseqctr = { 0x00, 0x00 },
/* SDEQCTR: source driver EQ mode 2, 2.5 us rise time on each step */
.sdeqctr = { 0x01, 0x05, 0x05, 0x05 },
/* SDVPCTR: Normal operation off color during v porch */
--
2.43.0
The initialization commands are taken from the STMicroelectronics driver
found at [1].
To ensure backward compatibility, flags have been added to enable gamma
correction setting and display control. In other cases, registers have
been set to their default values according to the specifications found
in the datasheet.
[1] https://github.com/STMicroelectronics/STM32CubeF7/blob/master/Drivers/BSP/Components/nt35510/
Signed-off-by: Dario Binacchi <[email protected]>
---
(no changes since v2)
Changes in v2:
- Re-write the patch [8/8] "drm/panel: nt35510: support FRIDA FRD400B25025-A-CTK"
in the same style as the original driver.
drivers/gpu/drm/panel/panel-novatek-nt35510.c | 282 ++++++++++++++++--
1 file changed, 251 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
index ce8969f48286..c85dd0d0829d 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
@@ -36,6 +36,9 @@
#include <drm/drm_modes.h>
#include <drm/drm_panel.h>
+#define NT35510_CMD_CORRECT_GAMMA BIT(0)
+#define NT35510_CMD_CONTROL_DISPLAY BIT(1)
+
#define MCS_CMD_MAUCCTR 0xF0 /* Manufacturer command enable */
#define MCS_CMD_READ_ID1 0xDA
#define MCS_CMD_READ_ID2 0xDB
@@ -112,18 +115,33 @@
/* AVDD and AVEE setting 3 bytes */
#define NT35510_P1_AVDD_LEN 3
#define NT35510_P1_AVEE_LEN 3
+#define NT35510_P1_VCL_LEN 3
#define NT35510_P1_VGH_LEN 3
#define NT35510_P1_VGL_LEN 3
#define NT35510_P1_VGP_LEN 3
#define NT35510_P1_VGN_LEN 3
+#define NT35510_P1_VCMOFF_LEN 2
/* BT1CTR thru BT5CTR setting 3 bytes */
#define NT35510_P1_BT1CTR_LEN 3
#define NT35510_P1_BT2CTR_LEN 3
+#define NT35510_P1_BT3CTR_LEN 3
#define NT35510_P1_BT4CTR_LEN 3
#define NT35510_P1_BT5CTR_LEN 3
/* 52 gamma parameters times two per color: positive and negative */
#define NT35510_P1_GAMMA_LEN 52
+#define NT35510_WRCTRLD_BCTRL BIT(5)
+#define NT35510_WRCTRLD_A BIT(4)
+#define NT35510_WRCTRLD_DD BIT(3)
+#define NT35510_WRCTRLD_BL BIT(2)
+#define NT35510_WRCTRLD_DB BIT(1)
+#define NT35510_WRCTRLD_G BIT(0)
+
+#define NT35510_WRCABC_OFF 0
+#define NT35510_WRCABC_UI_MODE 1
+#define NT35510_WRCABC_STILL_MODE 2
+#define NT35510_WRCABC_MOVING_MODE 3
+
/**
* struct nt35510_config - the display-specific NT35510 configuration
*
@@ -175,6 +193,10 @@ struct nt35510_config {
* @mode_flags: DSI operation mode related flags
*/
unsigned long mode_flags;
+ /**
+ * @cmds: enable DSI commands
+ */
+ u32 cmds;
/**
* @avdd: setting for AVDD ranging from 0x00 = 6.5V to 0x14 = 4.5V
* in 0.1V steps the default is 0x05 which means 6.0V
@@ -224,6 +246,25 @@ struct nt35510_config {
* The defaults are 4 and 3 yielding 0x34
*/
u8 bt2ctr[NT35510_P1_BT2CTR_LEN];
+ /**
+ * @vcl: setting for VCL ranging from 0x00 = -2.5V to 0x11 = -4.0V
+ * in 1V steps, the default is 0x00 which means -2.5V
+ */
+ u8 vcl[NT35510_P1_VCL_LEN];
+ /**
+ * @bt3ctr: setting for boost power control for the VCL step-up
+ * circuit (3)
+ * bits 0..2 in the lower nibble controls CLCK, the booster clock
+ * frequency, the values are the same as for PCK in @bt1ctr.
+ * bits 4..5 in the upper nibble controls BTCL, the boosting
+ * amplification for the step-up circuit.
+ * 0 = Disable
+ * 1 = -0.5 x VDDB
+ * 2 = -1 x VDDB
+ * 3 = -2 x VDDB
+ * The defaults are 4 and 2 yielding 0x24
+ */
+ u8 bt3ctr[NT35510_P1_BT3CTR_LEN];
/**
* @vgh: setting for VGH ranging from 0x00 = 7.0V to 0x0B = 18.0V
* in 1V steps, the default is 0x08 which means 15V
@@ -277,6 +318,19 @@ struct nt35510_config {
* same layout of bytes as @vgp.
*/
u8 vgn[NT35510_P1_VGN_LEN];
+ /**
+ * @vcmoff: setting the DC VCOM offset voltage
+ * The first byte contains bit 8 of VCM in bit 0 and VCMOFFSEL in bit 4.
+ * The second byte contains bits 0..7 of VCM.
+ * VCMOFFSEL the common voltage offset mode.
+ * VCMOFFSEL 0x00 = VCOM .. 0x01 Gamma.
+ * The default is 0x00.
+ * VCM the VCOM output voltage (VCMOFFSEL = 0) or the internal register
+ * offset for gamma voltage (VCMOFFSEL = 1).
+ * VCM 0x00 = 0V/0 .. 0x118 = 3.5V/280 in steps of 12.5mV/1step
+ * The default is 0x00 = 0V/0.
+ */
+ u8 vcmoff[NT35510_P1_VCMOFF_LEN];
/**
* @dopctr: setting optional control for display
* ERR bits 0..1 in the first byte is the ERR pin output signal setting.
@@ -441,6 +495,43 @@ struct nt35510_config {
* @gamma_corr_neg_b: Blue gamma correction parameters, negative
*/
u8 gamma_corr_neg_b[NT35510_P1_GAMMA_LEN];
+ /**
+ * @wrdisbv: write display brightness
+ * 0x00 value means the lowest brightness and 0xff value means
+ * the highest brightness.
+ * The default is 0x00.
+ */
+ u8 wrdisbv;
+ /**
+ * @wrctrld: write control display
+ * G bit 0 selects gamma curve: 0 = Manual, 1 = Automatic
+ * DB bit 1 selects display brightness: 0 = Manual, 1 = Automatic
+ * BL bit 2 controls backlight control: 0 = Off, 1 = On
+ * DD bit 3 controls display dimming: 0 = Off, 1 = On
+ * A bit 4 controls LABC block: 0 = Off, 1 = On
+ * BCTRL bit 5 controls brightness block: 0 = Off, 1 = On
+ */
+ u8 wrctrld;
+ /**
+ * @wrcabc: write content adaptive brightness control
+ * There is possible to use 4 different modes for content adaptive
+ * image functionality:
+ * 0: Off
+ * 1: User Interface Image (UI-Mode)
+ * 2: Still Picture Image (Still-Mode)
+ * 3: Moving Picture Image (Moving-Mode)
+ * The default is 0
+ */
+ u8 wrcabc;
+ /**
+ * @wrcabcmb: write CABC minimum brightness
+ * Set the minimum brightness value of the display for CABC
+ * function.
+ * 0x00 value means the lowest brightness for CABC and 0xff
+ * value means the highest brightness for CABC.
+ * The default is 0x00.
+ */
+ u8 wrcabcmb;
};
/**
@@ -584,6 +675,16 @@ static int nt35510_setup_power(struct nt35510 *nt)
nt->conf->bt2ctr);
if (ret)
return ret;
+ ret = nt35510_send_long(nt, dsi, NT35510_P1_SETVCL,
+ NT35510_P1_VCL_LEN,
+ nt->conf->vcl);
+ if (ret)
+ return ret;
+ ret = nt35510_send_long(nt, dsi, NT35510_P1_BT3CTR,
+ NT35510_P1_BT3CTR_LEN,
+ nt->conf->bt3ctr);
+ if (ret)
+ return ret;
ret = nt35510_send_long(nt, dsi, NT35510_P1_SETVGH,
NT35510_P1_VGH_LEN,
nt->conf->vgh);
@@ -620,6 +721,12 @@ static int nt35510_setup_power(struct nt35510 *nt)
if (ret)
return ret;
+ ret = nt35510_send_long(nt, dsi, NT35510_P1_SETVCMOFF,
+ NT35510_P1_VCMOFF_LEN,
+ nt->conf->vcmoff);
+ if (ret)
+ return ret;
+
/* Typically 10 ms */
usleep_range(10000, 20000);
@@ -799,36 +906,38 @@ static int nt35510_power_on(struct nt35510 *nt)
if (ret)
return ret;
- ret = nt35510_send_long(nt, dsi, NT35510_P1_SET_GAMMA_RED_POS,
- NT35510_P1_GAMMA_LEN,
- nt->conf->gamma_corr_pos_r);
- if (ret)
- return ret;
- ret = nt35510_send_long(nt, dsi, NT35510_P1_SET_GAMMA_GREEN_POS,
- NT35510_P1_GAMMA_LEN,
- nt->conf->gamma_corr_pos_g);
- if (ret)
- return ret;
- ret = nt35510_send_long(nt, dsi, NT35510_P1_SET_GAMMA_BLUE_POS,
- NT35510_P1_GAMMA_LEN,
- nt->conf->gamma_corr_pos_b);
- if (ret)
- return ret;
- ret = nt35510_send_long(nt, dsi, NT35510_P1_SET_GAMMA_RED_NEG,
- NT35510_P1_GAMMA_LEN,
- nt->conf->gamma_corr_neg_r);
- if (ret)
- return ret;
- ret = nt35510_send_long(nt, dsi, NT35510_P1_SET_GAMMA_GREEN_NEG,
- NT35510_P1_GAMMA_LEN,
- nt->conf->gamma_corr_neg_g);
- if (ret)
- return ret;
- ret = nt35510_send_long(nt, dsi, NT35510_P1_SET_GAMMA_BLUE_NEG,
- NT35510_P1_GAMMA_LEN,
- nt->conf->gamma_corr_neg_b);
- if (ret)
- return ret;
+ if (nt->conf->cmds & NT35510_CMD_CORRECT_GAMMA) {
+ ret = nt35510_send_long(nt, dsi, NT35510_P1_SET_GAMMA_RED_POS,
+ NT35510_P1_GAMMA_LEN,
+ nt->conf->gamma_corr_pos_r);
+ if (ret)
+ return ret;
+ ret = nt35510_send_long(nt, dsi, NT35510_P1_SET_GAMMA_GREEN_POS,
+ NT35510_P1_GAMMA_LEN,
+ nt->conf->gamma_corr_pos_g);
+ if (ret)
+ return ret;
+ ret = nt35510_send_long(nt, dsi, NT35510_P1_SET_GAMMA_BLUE_POS,
+ NT35510_P1_GAMMA_LEN,
+ nt->conf->gamma_corr_pos_b);
+ if (ret)
+ return ret;
+ ret = nt35510_send_long(nt, dsi, NT35510_P1_SET_GAMMA_RED_NEG,
+ NT35510_P1_GAMMA_LEN,
+ nt->conf->gamma_corr_neg_r);
+ if (ret)
+ return ret;
+ ret = nt35510_send_long(nt, dsi, NT35510_P1_SET_GAMMA_GREEN_NEG,
+ NT35510_P1_GAMMA_LEN,
+ nt->conf->gamma_corr_neg_g);
+ if (ret)
+ return ret;
+ ret = nt35510_send_long(nt, dsi, NT35510_P1_SET_GAMMA_BLUE_NEG,
+ NT35510_P1_GAMMA_LEN,
+ nt->conf->gamma_corr_neg_b);
+ if (ret)
+ return ret;
+ }
/* Set up stuff in manufacturer control, page 0 */
ret = nt35510_send_long(nt, dsi, MCS_CMD_MAUCCTR,
@@ -907,6 +1016,26 @@ static int nt35510_prepare(struct drm_panel *panel)
/* Up to 120 ms */
usleep_range(120000, 150000);
+ if (nt->conf->cmds & NT35510_CMD_CONTROL_DISPLAY) {
+ ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_WRITE_CONTROL_DISPLAY,
+ &nt->conf->wrctrld,
+ sizeof(nt->conf->wrctrld));
+ if (ret < 0)
+ return ret;
+
+ ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_WRITE_POWER_SAVE,
+ &nt->conf->wrcabc,
+ sizeof(nt->conf->wrcabc));
+ if (ret < 0)
+ return ret;
+
+ ret = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_CABC_MIN_BRIGHTNESS,
+ &nt->conf->wrcabcmb,
+ sizeof(nt->conf->wrcabcmb));
+ if (ret < 0)
+ return ret;
+ }
+
ret = mipi_dsi_dcs_set_display_on(dsi);
if (ret) {
dev_err(nt->dev, "failed to turn display on (%d)\n", ret);
@@ -1033,7 +1162,10 @@ static int nt35510_probe(struct mipi_dsi_device *dsi)
return PTR_ERR(bl);
}
bl->props.max_brightness = 255;
- bl->props.brightness = 255;
+ if (nt->conf->cmds & NT35510_CMD_CONTROL_DISPLAY)
+ bl->props.brightness = nt->conf->wrdisbv;
+ else
+ bl->props.brightness = 255;
bl->props.power = FB_BLANK_POWERDOWN;
nt->panel.backlight = bl;
}
@@ -1112,6 +1244,7 @@ static const struct nt35510_config nt35510_hydis_hva40wv1 = {
.flags = 0,
},
.mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS,
+ .cmds = NT35510_CMD_CORRECT_GAMMA,
/* 0x09: AVDD = 5.6V */
.avdd = { 0x09, 0x09, 0x09 },
/* 0x34: PCK = Hsync/2, BTP = 2 x VDDB */
@@ -1120,6 +1253,10 @@ static const struct nt35510_config nt35510_hydis_hva40wv1 = {
.avee = { 0x09, 0x09, 0x09 },
/* 0x24: NCK = Hsync/2, BTN = -2 x VDDB */
.bt2ctr = { 0x24, 0x24, 0x24 },
+ /* VBCLA: -2.5V, VBCLB: -2.5V, VBCLC: -2.5V */
+ .vcl = { 0x00, 0x00, 0x00 },
+ /* 0x24: CLCK = Hsync/2, BTN = -1 x VDDB */
+ .bt3ctr = { 0x24, 0x24, 0x24 },
/* 0x05 = 12V */
.vgh = { 0x05, 0x05, 0x05 },
/* 0x24: NCKA = Hsync/2, VGH = 2 x AVDD - AVEE */
@@ -1132,6 +1269,8 @@ static const struct nt35510_config nt35510_hydis_hva40wv1 = {
.vgp = { 0x00, 0xA3, 0x00 },
/* VGMP: 0x0A3 = 5.0375V, VGSP = 0V */
.vgn = { 0x00, 0xA3, 0x00 },
+ /* VCMOFFSEL = VCOM voltage offset mode, VCM = 0V */
+ .vcmoff = { 0x00, 0x00 },
/* Enable TE, EoTP and RGB pixel format */
.dopctr = { NT35510_DOPCTR_0_DSITE | NT35510_DOPCTR_0_EOTP |
NT35510_DOPCTR_0_N565, NT35510_DOPCTR_1_CTB },
@@ -1163,7 +1302,88 @@ static const struct nt35510_config nt35510_hydis_hva40wv1 = {
.gamma_corr_neg_b = { NT35510_GAMMA_NEG_DEFAULT },
};
+static const struct nt35510_config nt35510_frida_frd400b25025 = {
+ .width_mm = 52,
+ .height_mm = 86,
+ .mode = {
+ .clock = 23000,
+ .hdisplay = 480,
+ .hsync_start = 480 + 34, /* HFP = 34 */
+ .hsync_end = 480 + 34 + 2, /* HSync = 2 */
+ .htotal = 480 + 34 + 2 + 34, /* HBP = 34 */
+ .vdisplay = 800,
+ .vsync_start = 800 + 15, /* VFP = 15 */
+ .vsync_end = 800 + 15 + 12, /* VSync = 12 */
+ .vtotal = 800 + 15 + 12 + 15, /* VBP = 15 */
+ .flags = 0,
+ },
+ .mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST |
+ MIPI_DSI_MODE_LPM,
+ .cmds = NT35510_CMD_CONTROL_DISPLAY,
+ /* 0x03: AVDD = 6.2V */
+ .avdd = { 0x03, 0x03, 0x03 },
+ /* 0x46: PCK = 2 x Hsync, BTP = 2.5 x VDDB */
+ .bt1ctr = { 0x46, 0x46, 0x46 },
+ /* 0x03: AVEE = -6.2V */
+ .avee = { 0x03, 0x03, 0x03 },
+ /* 0x36: PCK = 2 x Hsync, BTP = 2 x VDDB */
+ .bt2ctr = { 0x36, 0x36, 0x36 },
+ /* VBCLA: -2.5V, VBCLB: -2.5V, VBCLC: -3.5V */
+ .vcl = { 0x00, 0x00, 0x02 },
+ /* 0x26: CLCK = 2 x Hsync, BTN = -1 x VDDB */
+ .bt3ctr = { 0x26, 0x26, 0x26 },
+ /* 0x09 = 16V */
+ .vgh = { 0x09, 0x09, 0x09 },
+ /* 0x36: HCK = 2 x Hsync, VGH = 2 x AVDD - AVEE */
+ .bt4ctr = { 0x36, 0x36, 0x36 },
+ /* 0x08 = -10V */
+ .vgl = { 0x08, 0x08, 0x08 },
+ /* 0x26: LCK = 2 x Hsync, VGL = AVDD + VCL - AVDD */
+ .bt5ctr = { 0x26, 0x26, 0x26 },
+ /* VGMP: 0x080 = 4.6V, VGSP = 0V */
+ .vgp = { 0x00, 0x80, 0x00 },
+ /* VGMP: 0x080 = 4.6V, VGSP = 0V */
+ .vgn = { 0x00, 0x80, 0x00 },
+ /* VCMOFFSEL = VCOM voltage offset mode, VCM = -1V */
+ .vcmoff = { 0x00, 0x50 },
+ .dopctr = { NT35510_DOPCTR_0_RAMKP | NT35510_DOPCTR_0_DSITE |
+ NT35510_DOPCTR_0_DSIG | NT35510_DOPCTR_0_DSIM |
+ NT35510_DOPCTR_0_EOTP | NT35510_DOPCTR_0_N565, 0 },
+ .madctl = NT35510_ROTATE_180_SETTING,
+ /* 0x03: SDT = 1.5 us */
+ .sdhdtctr = 0x03,
+ /* EQ control for gate signals, 0x00 = 0 us */
+ .gseqctr = { 0x00, 0x00 },
+ /* SDEQCTR: source driver EQ mode 2, 1 us rise time on each step */
+ .sdeqctr = { 0x01, 0x02, 0x02, 0x02 },
+ /* SDVPCTR: Normal operation off color during v porch */
+ .sdvpctr = 0x01,
+ /* T1: number of pixel clocks on one scanline: 0x184 = 389 clocks */
+ .t1 = 0x0184,
+ /* VBP: vertical back porch toward the panel */
+ .vbp = 0x1C,
+ /* VFP: vertical front porch toward the panel */
+ .vfp = 0x1C,
+ /* PSEL: divide pixel clock 23MHz with 1 (no clock downscaling) */
+ .psel = 0,
+ /* DPTMCTR12: 0x03: LVGL = VGLX, overlap mode, swap R->L O->E */
+ .dpmctr12 = { 0x03, 0x00, 0x00, },
+ /* write display brightness */
+ .wrdisbv = 0x7f,
+ /* write control display */
+ .wrctrld = NT35510_WRCTRLD_BCTRL | NT35510_WRCTRLD_DD |
+ NT35510_WRCTRLD_BL,
+ /* write content adaptive brightness control */
+ .wrcabc = NT35510_WRCABC_STILL_MODE,
+ /* write CABC minimum brightness */
+ .wrcabcmb = 0xff,
+};
+
static const struct of_device_id nt35510_of_match[] = {
+ {
+ .compatible = "frida,frd400b25025",
+ .data = &nt35510_frida_frd400b25025,
+ },
{
.compatible = "hydis,hva40wv1",
.data = &nt35510_hydis_hva40wv1,
--
2.43.0
On 04/01/2024 09:41, Dario Binacchi wrote:
> The patch adds the FRIDA FRD400B25025-A-CTK panel, which belongs to the
> Novatek NT35510-based panel family.
>
> Signed-off-by: Dario Binacchi <[email protected]>
>
> ---
>
> Changes in v4:
> - Put the "enum" list in alphabetical order
>
Acked-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On Thu, Jan 4, 2024 at 9:42 AM Dario Binacchi
<[email protected]> wrote:
> The patch adds the FRIDA FRD400B25025-A-CTK panel, which belongs to the
> Novatek NT35510-based panel family.
>
> Signed-off-by: Dario Binacchi <[email protected]>
v4 looks very nice, thanks!
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Thu, Jan 4, 2024 at 9:42 AM Dario Binacchi
<[email protected]> wrote:
> This patch, preparatory for future developments, move the hardwired
> parameters to configuration data to allow the addition of new
> NT35510-based panels.
>
> Signed-off-by: Dario Binacchi <[email protected]>
I tested this patch on the NT35510-based Skomer and it makes the
whole display mirrored around the Y-axis (text from right to left instead
of the other way around, penguins appear mirrored upper right
etc).
+ /* Enable TE, EoTP and RGB pixel format */
+ .dopctr = { NT35510_DOPCTR_0_DSITE | NT35510_DOPCTR_0_EOTP |
+ NT35510_DOPCTR_0_N565, NT35510_DOPCTR_1_CTB },
+ .madctl = NT35510_ROTATE_180_SETTING,
Changing this to NT35510_ROTATE_0_SETTING makes it work
fine again.
With that change:
Reviewed-by: Linus Walleij <[email protected]>
Tested-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On Thu, Jan 4, 2024 at 9:42 AM Dario Binacchi
<[email protected]> wrote:
> The initialization commands are taken from the STMicroelectronics driver
> found at [1].
> To ensure backward compatibility, flags have been added to enable gamma
> correction setting and display control. In other cases, registers have
> been set to their default values according to the specifications found
> in the datasheet.
>
> [1] https://github.com/STMicroelectronics/STM32CubeF7/blob/master/Drivers/BSP/Components/nt35510/
> Signed-off-by: Dario Binacchi <[email protected]>
>
> ---
>
> (no changes since v2)
Reviewed-by: Linus Walleij <[email protected]>
(also tested to not regress my hardware)
Yours,
Linus Walleij
On Fri, Jan 5, 2024 at 8:08 PM Linus Walleij <[email protected]> wrote:
>
> On Thu, Jan 4, 2024 at 9:42 AM Dario Binacchi
> <[email protected]> wrote:
>
> > This patch, preparatory for future developments, move the hardwired
> > parameters to configuration data to allow the addition of new
> > NT35510-based panels.
> >
> > Signed-off-by: Dario Binacchi <[email protected]>
>
> I tested this patch on the NT35510-based Skomer and it makes the
> whole display mirrored around the Y-axis (text from right to left instead
> of the other way around, penguins appear mirrored upper right
> etc).
>
> + /* Enable TE, EoTP and RGB pixel format */
> + .dopctr = { NT35510_DOPCTR_0_DSITE | NT35510_DOPCTR_0_EOTP |
> + NT35510_DOPCTR_0_N565, NT35510_DOPCTR_1_CTB },
> + .madctl = NT35510_ROTATE_180_SETTING,
>
> Changing this to NT35510_ROTATE_0_SETTING makes it work
> fine again.
Sorry for the mistake
Thanks and regards,
Dario Binacchi
>
> With that change:
> Reviewed-by: Linus Walleij <[email protected]>
> Tested-by: Linus Walleij <[email protected]>
>
> Yours,
> Linus Walleij
--
Dario Binacchi
Senior Embedded Linux Developer
[email protected]
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
[email protected]
http://www.amarulasolutions.com
On Fri, Jan 5, 2024 at 8:09 PM Linus Walleij <[email protected]> wrote:
>
> On Thu, Jan 4, 2024 at 9:42 AM Dario Binacchi
> <[email protected]> wrote:
>
> > The initialization commands are taken from the STMicroelectronics driver
> > found at [1].
> > To ensure backward compatibility, flags have been added to enable gamma
> > correction setting and display control. In other cases, registers have
> > been set to their default values according to the specifications found
> > in the datasheet.
> >
> > [1] https://github.com/STMicroelectronics/STM32CubeF7/blob/master/Drivers/BSP/Components/nt35510/
> > Signed-off-by: Dario Binacchi <[email protected]>
> >
> > ---
> >
> > (no changes since v2)
>
> Reviewed-by: Linus Walleij <[email protected]>
> (also tested to not regress my hardware)
After submitting v4, I tested the driver under different conditions,
i. e. without enabling display support in
U-Boot (I also implemented a version for U-Boot, which I will send
only after this series is merged into
the Linux kernel). In that condition I encountered an issue with the reset pin.
The minimal fix, would be this:
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
index c85dd0d0829d..89ba99763468 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
@@ -1133,7 +1133,7 @@ static int nt35510_probe(struct mipi_dsi_device *dsi)
if (ret)
return ret;
- nt->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
+ nt->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(nt->reset_gpio)) {
dev_err(dev, "error getting RESET GPIO\n");
return PTR_ERR(nt->reset_gpio);
With the doubt that this might cause a regression in your use case.
I then tried modifying the device tree instead of the nt35510 driver.
In the end, I arrived
at this patch that leaves me with some doubts, especially regarding
the strict option.
diff --git a/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
b/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-m>
index 014cac192375..32ed420a6cbf 100644
--- a/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
+++ b/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
@@ -13,6 +13,17 @@ &panel0 {
compatible = "frida,frd400b25025", "novatek,nt35510";
vddi-supply = <&vcc_3v3>;
vdd-supply = <&vcc_3v3>;
+ pinctrl-0 = <&panel_reset>;
+ pinctrl-names = "default";
/delete-property/backlight;
/delete-property/power-supply;
};
+
+&pinctrl {
+ panel_reset: panel-reset {
+ pins1 {
+ pinmux = <STM32_PINMUX('J', 15, GPIO)>;
+ output-high;
+ };
+ };
+};
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c
b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 346a31f31bba..6f055f7f96a2 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -895,7 +895,6 @@ static const struct pinmux_ops stm32_pmx_ops = {
.set_mux = stm32_pmx_set_mux,
.gpio_set_direction = stm32_pmx_gpio_set_direction,
.request = stm32_pmx_request,
- .strict = true,
};
Another option to fix my use case without introducing regressions
could be to add a
new property to the device tree that suggests whether to call
devm_gpiod_get_optional()
with the GPIOD_ASIS or GPIOD_OUT_HIGH parameter.
What is your opinion?
Thanks and regards,
Dario Binacchi
>
> Yours,
> Linus Walleij
--
Dario Binacchi
Senior Embedded Linux Developer
[email protected]
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
[email protected]
http://www.amarulasolutions.com
On Sat, Jan 6, 2024 at 12:07 PM Dario Binacchi
<[email protected]> wrote:
> After submitting v4, I tested the driver under different conditions,
> i. e. without enabling display support in
> U-Boot (I also implemented a version for U-Boot, which I will send
> only after this series is merged into
> the Linux kernel). In that condition I encountered an issue with the reset pin.
>
> The minimal fix, would be this:
>
> diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> index c85dd0d0829d..89ba99763468 100644
> --- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> +++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> @@ -1133,7 +1133,7 @@ static int nt35510_probe(struct mipi_dsi_device *dsi)
> if (ret)
> return ret;
>
> - nt->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
> + nt->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
This is fine, because we later on toggle reset in nt35510_power_on(),
this just asserts the reset signal already in probe.
I don't see why this would make a difference though?
Is it to make the reset delete artifacts from the screen?
Just explain it in the commit message.
It is a bit confusing when I look at your DTS patch:
https://lore.kernel.org/linux-arm-kernel/[email protected]/
this doesn't even contain a reset GPIO, so nothing will happen
at all?
> I then tried modifying the device tree instead of the nt35510 driver.
> In the end, I arrived
> at this patch that leaves me with some doubts, especially regarding
> the strict option.
>
> diff --git a/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
> b/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-m>
> index 014cac192375..32ed420a6cbf 100644
> --- a/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
> +++ b/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
> @@ -13,6 +13,17 @@ &panel0 {
> compatible = "frida,frd400b25025", "novatek,nt35510";
> vddi-supply = <&vcc_3v3>;
> vdd-supply = <&vcc_3v3>;
> + pinctrl-0 = <&panel_reset>;
> + pinctrl-names = "default";
> /delete-property/backlight;
> /delete-property/power-supply;
> };
> +
> +&pinctrl {
> + panel_reset: panel-reset {
> + pins1 {
> + pinmux = <STM32_PINMUX('J', 15, GPIO)>;
> + output-high;
But this achieves the *opposite* of what you do in the
other patch. This sets the reset line de-asserted since it is
active low.
Did you add the reset line to your device tree and forgot to
set it as GPIO_ACTIVE_LOW perhaps?
> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
> @@ -895,7 +895,6 @@ static const struct pinmux_ops stm32_pmx_ops = {
> .set_mux = stm32_pmx_set_mux,
> .gpio_set_direction = stm32_pmx_gpio_set_direction,
> .request = stm32_pmx_request,
> - .strict = true,
To be honest this is what I use a lot of the time, with non-strict
pin control you can set up default GPIO values using the pin control
device tree, and it's really simple and easy to read like that since e.g.
in this case you set it from the panel device node which is what
is also consuming the GPIO, so very logical. So I
would certainly remove this .strict setting, but maybe Alexandre
et al have a strong opinion about it.
> Another option to fix my use case without introducing regressions
> could be to add a
> new property to the device tree that suggests whether to call
> devm_gpiod_get_optional()
> with the GPIOD_ASIS or GPIOD_OUT_HIGH parameter.
>
> What is your opinion?
It's fine either way, but just use GPIOD_OUT_HIGH and I can test
it on my system as well, I think it's fine.
Yours,
Linus Walleij
On Sun, Jan 7, 2024 at 9:02 PM Linus Walleij <[email protected]> wrote:
>
> On Sat, Jan 6, 2024 at 12:07 PM Dario Binacchi
> <[email protected]> wrote:
>
> > After submitting v4, I tested the driver under different conditions,
> > i. e. without enabling display support in
> > U-Boot (I also implemented a version for U-Boot, which I will send
> > only after this series is merged into
> > the Linux kernel). In that condition I encountered an issue with the reset pin.
> >
> > The minimal fix, would be this:
> >
> > diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> > b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> > index c85dd0d0829d..89ba99763468 100644
> > --- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> > +++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
> > @@ -1133,7 +1133,7 @@ static int nt35510_probe(struct mipi_dsi_device *dsi)
> > if (ret)
> > return ret;
> >
> > - nt->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
> > + nt->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>
>
> This is fine, because we later on toggle reset in nt35510_power_on(),
> this just asserts the reset signal already in probe.
>
> I don't see why this would make a difference though?
>
> Is it to make the reset delete artifacts from the screen?
>
> Just explain it in the commit message.
>
> It is a bit confusing when I look at your DTS patch:
>
> https://lore.kernel.org/linux-arm-kernel/[email protected]/
>
> this doesn't even contain a reset GPIO, so nothing will happen
> at all?
+++ b/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 Dario Binacchi <[email protected]>
+ */
+
+#include "stm32f769-disco.dts"
+
The GPIO is contained in the stm32f769-disco.dts:
panel0: panel-dsi@0 {
compatible = "orisetech,otm8009a";
reg = <0>; /* dsi virtual channel (0..3) */
reset-gpios = <&gpioj 15 GPIO_ACTIVE_LOW>;
power-supply = <&vcc_3v3>;
backlight = <&panel_backlight>;
status = "okay";
...
};
>
> > I then tried modifying the device tree instead of the nt35510 driver.
> > In the end, I arrived
> > at this patch that leaves me with some doubts, especially regarding
> > the strict option.
> >
> > diff --git a/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
> > b/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-m>
> > index 014cac192375..32ed420a6cbf 100644
> > --- a/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
> > +++ b/arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts
> > @@ -13,6 +13,17 @@ &panel0 {
> > compatible = "frida,frd400b25025", "novatek,nt35510";
> > vddi-supply = <&vcc_3v3>;
> > vdd-supply = <&vcc_3v3>;
> > + pinctrl-0 = <&panel_reset>;
> > + pinctrl-names = "default";
> > /delete-property/backlight;
> > /delete-property/power-supply;
> > };
> > +
> > +&pinctrl {
> > + panel_reset: panel-reset {
> > + pins1 {
> > + pinmux = <STM32_PINMUX('J', 15, GPIO)>;
> > + output-high;
>
> But this achieves the *opposite* of what you do in the
> other patch. This sets the reset line de-asserted since it is
> active low.
>
> Did you add the reset line to your device tree and forgot to
> set it as GPIO_ACTIVE_LOW perhaps?
panel0: panel-dsi@0 {
compatible = "orisetech,otm8009a";
reg = <0>; /* dsi virtual channel (0..3) */
reset-gpios = <&gpioj 15 GPIO_ACTIVE_LOW>;
>
> > --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
> > +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
> > @@ -895,7 +895,6 @@ static const struct pinmux_ops stm32_pmx_ops = {
> > .set_mux = stm32_pmx_set_mux,
> > .gpio_set_direction = stm32_pmx_gpio_set_direction,
> > .request = stm32_pmx_request,
> > - .strict = true,
>
> To be honest this is what I use a lot of the time, with non-strict
> pin control you can set up default GPIO values using the pin control
> device tree, and it's really simple and easy to read like that since e.g.
> in this case you set it from the panel device node which is what
> is also consuming the GPIO, so very logical. So I
> would certainly remove this .strict setting, but maybe Alexandre
> et al have a strong opinion about it.
I will send a separate RFC PATCH.
Thanks and regards,
Dario Binacchi
>
> > Another option to fix my use case without introducing regressions
> > could be to add a
> > new property to the device tree that suggests whether to call
> > devm_gpiod_get_optional()
> > with the GPIOD_ASIS or GPIOD_OUT_HIGH parameter.
> >
> > What is your opinion?
>
> It's fine either way, but just use GPIOD_OUT_HIGH and I can test
> it on my system as well, I think it's fine.
>
> Yours,
> Linus Walleij
--
Dario Binacchi
Senior Embedded Linux Developer
[email protected]
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
[email protected]
http://www.amarulasolutions.com