dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/10] Add displays support for bsh-smm-s2/pro boards
@ 2023-12-05 10:52 Dario Binacchi
  2023-12-05 10:52 ` [PATCH v4 01/10] drm/bridge: Fix bridge disable logic Dario Binacchi
                   ` (8 more replies)
  0 siblings, 9 replies; 21+ messages in thread
From: Dario Binacchi @ 2023-12-05 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peng Fan, Krzysztof Kozlowski, Sam Ravnborg, dri-devel,
	Frieder Schrempf, Andrzej Hajda, Dario Binacchi,
	Marek Szyprowski, Robert Foss, Dave Stevenson, Jernej Skrabec,
	NXP Linux Team, Jessica Zhang, michael, Jagan Teki, devicetree,
	Conor Dooley, Pengutronix Kernel Team, Jonas Karlman,
	Sascha Hauer, Maxime Ripard, Rob Herring, linux-arm-kernel,
	Neil Armstrong, Thomas Zimmermann, Shawn Guo, Amarula patchwork,
	Laurent Pinchart

The series adds drivers for the displays used by bsh-smm-s2/pro boards.
This required applying some patches to the samsung-dsim driver and the
drm_bridge.c module.

Changes in v4:
- Set the reset gpio to low in a single operation
- Remove duplicated code for prepare/unprepare callbacks
- Add 'Reviewed-by; tag of Neil Armstrong

Changes in v3:
- Add 'Reviewed-by' tag of Krzysztof Kozlowski.
- Replace "synaptics,r63353" compatible with "syna,r63353", as
  required by vendor-prefixes.yaml.
- Drop power-supply
- Replace "synaptics,r63353" compatible with "syna,r63353", as
  required by vendor-prefixes.yaml.
- Squash patch [09/11] dt-bindings: ili9805: add compatible string for Tianma TM041XDHG01
  into [07/11] dt-bindings: display: panel: Add Ilitek ili9805 panel controller.

Changes in v2:
- Add $ref to panel-common.yaml
- Drop port, reset-gpios, and backlight
- Set port and backlight ad required
- Replace additionalProperties with unevaluatedProperties
- Adjust the timings of the panel reset
- Add $ref to panel-common.yaml
- Drop port, reset-gpios, and backlight
- Set port and backlight ad required
- Replace additionalProperties with unevaluatedProperties
- Adjust the mipi_dsi node based on the latest patches merged into
  the mainline in the dtsi files it includes.
- Added to the series the following patches:
  - 0001 drm/bridge: Fix bridge disable logic
  - 0002 drm/bridge: Fix a use case in the bridge disable logic
  - 0003 samsung-dsim: enter display mode in the enable() callback
  - 0004 drm: bridge: samsung-dsim: complete the CLKLANE_STOP setting

Dario Binacchi (4):
  drm/bridge: Fix bridge disable logic
  drm/bridge: Fix a use case in the bridge disable logic
  drm: bridge: samsung-dsim: enter display mode in the enable() callback
  drm: bridge: samsung-dsim: complete the CLKLANE_STOP setting

Michael Trimarchi (6):
  dt-bindings: display: panel: Add synaptics r63353 panel controller
  drm/panel: Add Synaptics R63353 panel driver
  dt-bindings: display: panel: Add Ilitek ili9805 panel controller
  drm/panel: Add Ilitek ILI9805 panel driver
  drm/panel: ilitek-ili9805: add support for Tianma TM041XDHG01 panel
  arm64: dts: imx8mn-bsh-smm-s2/pro: add display setup

 .../display/panel/ilitek,ili9805.yaml         |  62 +++
 .../display/panel/synaptics,r63353.yaml       |  61 +++
 MAINTAINERS                                   |  12 +
 .../freescale/imx8mn-bsh-smm-s2-common.dtsi   |   1 +
 .../freescale/imx8mn-bsh-smm-s2-display.dtsi  | 121 ++++++
 drivers/gpu/drm/bridge/samsung-dsim.c         |  14 +-
 drivers/gpu/drm/drm_bridge.c                  |   9 +-
 drivers/gpu/drm/panel/Kconfig                 |  18 +
 drivers/gpu/drm/panel/Makefile                |   2 +
 drivers/gpu/drm/panel/panel-ilitek-ili9805.c  | 406 ++++++++++++++++++
 .../gpu/drm/panel/panel-synaptics-r63353.c    | 363 ++++++++++++++++
 11 files changed, 1062 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9805.yaml
 create mode 100644 Documentation/devicetree/bindings/display/panel/synaptics,r63353.yaml
 create mode 100644 arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2-display.dtsi
 create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9805.c
 create mode 100644 drivers/gpu/drm/panel/panel-synaptics-r63353.c

-- 
2.43.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 01/10] drm/bridge: Fix bridge disable logic
  2023-12-05 10:52 [PATCH v4 00/10] Add displays support for bsh-smm-s2/pro boards Dario Binacchi
@ 2023-12-05 10:52 ` Dario Binacchi
  2023-12-05 10:52 ` [PATCH v4 02/10] drm/bridge: Fix a use case in the " Dario Binacchi
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Dario Binacchi @ 2023-12-05 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Neil Armstrong, Dario Binacchi, Robert Foss,
	Andrzej Hajda, Thomas Zimmermann, Dave Stevenson, dri-devel,
	Jonas Karlman, Frieder Schrempf, Jernej Skrabec, michael,
	Amarula patchwork, Laurent Pinchart

As explained by the comment of the fixed code, we need to find the next
bridge that hasn't set the "pre_enable_prev_first" flag to true. The code,
on the contrary, was doing the opposite.
So, the order of disabling the bridges couldn't be altered as required
by setting the "pre_enable_prev_first" flag to true.

Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to alter bridge init order")
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---

(no changes since v1)

 drivers/gpu/drm/drm_bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 30d66bee0ec6..f66bf4925dd8 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -686,7 +686,7 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
 				 */
 				list_for_each_entry_from(next, &encoder->bridge_chain,
 							 chain_node) {
-					if (next->pre_enable_prev_first) {
+					if (!next->pre_enable_prev_first) {
 						next = list_prev_entry(next, chain_node);
 						limit = next;
 						break;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic
  2023-12-05 10:52 [PATCH v4 00/10] Add displays support for bsh-smm-s2/pro boards Dario Binacchi
  2023-12-05 10:52 ` [PATCH v4 01/10] drm/bridge: Fix bridge disable logic Dario Binacchi
@ 2023-12-05 10:52 ` Dario Binacchi
  2023-12-05 15:39   ` Dave Stevenson
  2023-12-05 10:52 ` [PATCH v4 03/10] drm: bridge: samsung-dsim: enter display mode in the enable() callback Dario Binacchi
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Dario Binacchi @ 2023-12-05 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Neil Armstrong, Dario Binacchi, Robert Foss,
	Andrzej Hajda, Thomas Zimmermann, Dave Stevenson, dri-devel,
	Jonas Karlman, Frieder Schrempf, Jernej Skrabec, michael,
	Amarula patchwork, Laurent Pinchart

The patch fixes the code for finding the next bridge with the
"pre_enable_prev_first" flag set to false. In case this condition is
not verified, i. e. there is no subsequent bridge with the flag set to
false, the whole bridge list is traversed, invalidating the "next"
variable.

The use of a new iteration variable (i. e. "iter") ensures that the value
of the "next" variable is not invalidated.

Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to alter bridge init order")
Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---

(no changes since v1)

 drivers/gpu/drm/drm_bridge.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index f66bf4925dd8..2e5781bf192e 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -662,7 +662,7 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
 					  struct drm_atomic_state *old_state)
 {
 	struct drm_encoder *encoder;
-	struct drm_bridge *next, *limit;
+	struct drm_bridge *iter, *next, *limit;
 
 	if (!bridge)
 		return;
@@ -680,14 +680,15 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
 				 * was enabled first, so disabled last
 				 */
 				limit = next;
+				iter = next;
 
 				/* Find the next bridge that has NOT requested
 				 * prev to be enabled first / disabled last
 				 */
-				list_for_each_entry_from(next, &encoder->bridge_chain,
+				list_for_each_entry_from(iter, &encoder->bridge_chain,
 							 chain_node) {
-					if (!next->pre_enable_prev_first) {
-						next = list_prev_entry(next, chain_node);
+					if (!iter->pre_enable_prev_first) {
+						next = list_prev_entry(iter, chain_node);
 						limit = next;
 						break;
 					}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 03/10] drm: bridge: samsung-dsim: enter display mode in the enable() callback
  2023-12-05 10:52 [PATCH v4 00/10] Add displays support for bsh-smm-s2/pro boards Dario Binacchi
  2023-12-05 10:52 ` [PATCH v4 01/10] drm/bridge: Fix bridge disable logic Dario Binacchi
  2023-12-05 10:52 ` [PATCH v4 02/10] drm/bridge: Fix a use case in the " Dario Binacchi
@ 2023-12-05 10:52 ` Dario Binacchi
  2023-12-05 10:52 ` [PATCH v4 04/10] drm: bridge: samsung-dsim: complete the CLKLANE_STOP setting Dario Binacchi
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Dario Binacchi @ 2023-12-05 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Neil Armstrong, Dario Binacchi, Robert Foss,
	Andrzej Hajda, Thomas Zimmermann, Jonas Karlman,
	Amarula patchwork, dri-devel, Jernej Skrabec, Jagan Teki,
	Marek Szyprowski, michael, Laurent Pinchart

The synaptics-r63353 (panel-bridge) can only be configured in command mode.
So, samsung-dsim (bridge) must not be in display mode during the
prepare()/unprepare() of the panel-bridge. Setting the
"pre_enable_prev_first" flag to true allows the prepare() of the
panel-bridge to be called between the pre_enabled() and enabled() of the
bridge. So, the bridge can enter display mode only in the enabled().
The unprepare() of the panel-bridge is instead called between the disable()
and post_disable() of the bridge. So, the disable() must exit the display
mode (i .e. enter command mode) to allow the panel-bridge to receive DSI
commands.

samsung_dsim_atomic_pre_enable   -> command mode
r63353_panel_prepare             -> send DSI commands
samsung_dsim_atomic_enable       -> enter display mode

samsung_dsim_atomic_disable      -> exit display mode (command mode)
r63353_panel_unprepare           -> send DSI commands
samsung_dsim_atomic_post_disable

Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---

(no changes since v1)

 drivers/gpu/drm/bridge/samsung-dsim.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index be5914caa17d..15bf05b2bbe4 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1494,7 +1494,6 @@ static void samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
 			return;
 
 		samsung_dsim_set_display_mode(dsi);
-		samsung_dsim_set_display_enable(dsi, true);
 	}
 }
 
@@ -1507,6 +1506,7 @@ static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
 		samsung_dsim_set_display_mode(dsi);
 		samsung_dsim_set_display_enable(dsi, true);
 	} else {
+		samsung_dsim_set_display_enable(dsi, true);
 		samsung_dsim_set_stop_state(dsi, false);
 	}
 
@@ -1524,6 +1524,8 @@ static void samsung_dsim_atomic_disable(struct drm_bridge *bridge,
 	if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
 		samsung_dsim_set_stop_state(dsi, true);
 
+	samsung_dsim_set_display_enable(dsi, false);
+
 	dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
 }
 
@@ -1532,7 +1534,8 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
 {
 	struct samsung_dsim *dsi = bridge_to_dsi(bridge);
 
-	samsung_dsim_set_display_enable(dsi, false);
+	if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
+		samsung_dsim_set_stop_state(dsi, true);
 
 	dsi->state &= ~DSIM_STATE_ENABLED;
 	pm_runtime_put_sync(dsi->dev);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 04/10] drm: bridge: samsung-dsim: complete the CLKLANE_STOP setting
  2023-12-05 10:52 [PATCH v4 00/10] Add displays support for bsh-smm-s2/pro boards Dario Binacchi
                   ` (2 preceding siblings ...)
  2023-12-05 10:52 ` [PATCH v4 03/10] drm: bridge: samsung-dsim: enter display mode in the enable() callback Dario Binacchi
@ 2023-12-05 10:52 ` Dario Binacchi
  2023-12-05 10:52 ` [PATCH v4 05/10] dt-bindings: display: panel: Add synaptics r63353 panel controller Dario Binacchi
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Dario Binacchi @ 2023-12-05 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Maxime Ripard, Neil Armstrong, Dario Binacchi, Robert Foss,
	Andrzej Hajda, Thomas Zimmermann, Jonas Karlman,
	Amarula patchwork, dri-devel, Jernej Skrabec, Jagan Teki,
	Marek Szyprowski, michael, Laurent Pinchart

The patch completes the setting of CLKLANE_STOP for the imx8mn and imx8mp
platforms (i. e. not exynos).

Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---

(no changes since v1)

 drivers/gpu/drm/bridge/samsung-dsim.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 15bf05b2bbe4..13f181c99d7e 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -96,6 +96,7 @@
 #define DSIM_MFLUSH_VS			BIT(29)
 /* This flag is valid only for exynos3250/3472/5260/5430 */
 #define DSIM_CLKLANE_STOP		BIT(30)
+#define DSIM_NON_CONTINUOUS_CLKLANE	BIT(31)
 
 /* DSIM_ESCMODE */
 #define DSIM_TX_TRIGGER_RST		BIT(4)
@@ -945,8 +946,12 @@ static int samsung_dsim_init_link(struct samsung_dsim *dsi)
 	 * power consumption.
 	 */
 	if (driver_data->has_clklane_stop &&
-	    dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS)
+	    dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) {
+		if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
+			reg |= DSIM_NON_CONTINUOUS_CLKLANE;
+
 		reg |= DSIM_CLKLANE_STOP;
+	}
 	samsung_dsim_write(dsi, DSIM_CONFIG_REG, reg);
 
 	lanes_mask = BIT(dsi->lanes) - 1;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 05/10] dt-bindings: display: panel: Add synaptics r63353 panel controller
  2023-12-05 10:52 [PATCH v4 00/10] Add displays support for bsh-smm-s2/pro boards Dario Binacchi
                   ` (3 preceding siblings ...)
  2023-12-05 10:52 ` [PATCH v4 04/10] drm: bridge: samsung-dsim: complete the CLKLANE_STOP setting Dario Binacchi
@ 2023-12-05 10:52 ` Dario Binacchi
  2023-12-05 10:52 ` [PATCH v4 06/10] drm/panel: Add Synaptics R63353 panel driver Dario Binacchi
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Dario Binacchi @ 2023-12-05 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Neil Armstrong, Conor Dooley, Dario Binacchi,
	Krzysztof Kozlowski, Sam Ravnborg, devicetree, dri-devel,
	Maxime Ripard, Krzysztof Kozlowski, Rob Herring,
	Thomas Zimmermann, Jessica Zhang, michael, Amarula patchwork

From: Michael Trimarchi <michael@amarulasolutions.com>

Add documentation for "synaptics,r63353" panel.

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

(no changes since v3)

Changes in v3:
- Add 'Reviewed-by' tag of Krzysztof Kozlowski.
- Replace "synaptics,r63353" compatible with "syna,r63353", as
  required by vendor-prefixes.yaml.

Changes in v2:
- Add $ref to panel-common.yaml
- Drop port, reset-gpios, and backlight
- Set port and backlight ad required
- Replace additionalProperties with unevaluatedProperties

 .../display/panel/synaptics,r63353.yaml       | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/synaptics,r63353.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/synaptics,r63353.yaml b/Documentation/devicetree/bindings/display/panel/synaptics,r63353.yaml
new file mode 100644
index 000000000000..e5617d125567
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/synaptics,r63353.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/synaptics,r63353.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synaptics R63353 based MIPI-DSI panels
+
+maintainers:
+  - Michael Trimarchi <michael@amarulasolutions.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - sharp,ls068b3sx02
+      - const: syna,r63353
+
+  avdd-supply: true
+  dvdd-supply: true
+  reg: true
+
+required:
+  - compatible
+  - avdd-supply
+  - dvdd-supply
+  - reg
+  - reset-gpios
+  - port
+  - backlight
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    dsi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        panel@0 {
+            compatible = "sharp,ls068b3sx02", "syna,r63353";
+            reg = <0>;
+            avdd-supply = <&avdd_display>;
+            dvdd-supply = <&dvdd_display>;
+            reset-gpios = <&r_pio 0 5 GPIO_ACTIVE_LOW>; /* PL05 */
+            backlight = <&backlight>;
+
+            port {
+                panel_in: endpoint {
+                    remote-endpoint = <&mipi_dsi_out>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 06/10] drm/panel: Add Synaptics R63353 panel driver
  2023-12-05 10:52 [PATCH v4 00/10] Add displays support for bsh-smm-s2/pro boards Dario Binacchi
                   ` (4 preceding siblings ...)
  2023-12-05 10:52 ` [PATCH v4 05/10] dt-bindings: display: panel: Add synaptics r63353 panel controller Dario Binacchi
@ 2023-12-05 10:52 ` Dario Binacchi
  2023-12-07  5:26   ` Dan Carpenter
  2023-12-05 10:52 ` [PATCH v4 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller Dario Binacchi
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Dario Binacchi @ 2023-12-05 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Neil Armstrong, Dario Binacchi, Thomas Zimmermann, Sam Ravnborg,
	Amarula patchwork, Maxime Ripard, dri-devel, Jessica Zhang,
	michael

From: Michael Trimarchi <michael@amarulasolutions.com>

The LS068B3SX02 panel is based on the Synaptics R63353 Controller.
Add a driver for it.

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

---

Changes in v4:
- Set the reset gpio to low in a single operation
- Remove duplicated code for prepare/unprepare callbacks

Changes in v2:
- Adjust the timings of the panel reset

 MAINTAINERS                                   |   6 +
 drivers/gpu/drm/panel/Kconfig                 |   9 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 .../gpu/drm/panel/panel-synaptics-r63353.c    | 363 ++++++++++++++++++
 4 files changed, 379 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-synaptics-r63353.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 788be9ab5b73..b82dc141d209 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6874,6 +6874,12 @@ T:	git git://anongit.freedesktop.org/drm/drm-misc
 F:	Documentation/devicetree/bindings/display/ste,mcde.yaml
 F:	drivers/gpu/drm/mcde/
 
+DRM DRIVER FOR SYNAPTICS R63353 PANELS
+M:	Michael Trimarchi <michael@amarulasolutions.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/display/panel/synaptics,r63353.yaml
+F:	drivers/gpu/drm/panel/panel-synaptics-r63353.c
+
 DRM DRIVER FOR TI DLPC3433 MIPI DSI TO DMD BRIDGE
 M:	Jagan Teki <jagan@amarulasolutions.com>
 S:	Maintained
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 99e14dc212ec..d018702be3dc 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -735,6 +735,15 @@ config DRM_PANEL_SITRONIX_ST7789V
 	  Say Y here if you want to enable support for the Sitronix
 	  ST7789V controller for 240x320 LCD panels
 
+config DRM_PANEL_SYNAPTICS_R63353
+	tristate "Synaptics R63353-based panels"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y if you want to enable support for panels based on the
+	  Synaptics R63353 controller.
+
 config DRM_PANEL_SONY_ACX565AKM
 	tristate "Sony ACX565AKM panel"
 	depends on GPIOLIB && OF && SPI
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index d10c3de51c6d..f267d932c2b5 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_DRM_PANEL_SHARP_LS060T1SX01) += panel-sharp-ls060t1sx01.o
 obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7701) += panel-sitronix-st7701.o
 obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7703) += panel-sitronix-st7703.o
 obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
+obj-$(CONFIG_DRM_PANEL_SYNAPTICS_R63353) += panel-synaptics-r63353.o
 obj-$(CONFIG_DRM_PANEL_SONY_ACX565AKM) += panel-sony-acx565akm.o
 obj-$(CONFIG_DRM_PANEL_SONY_TD4353_JDI) += panel-sony-td4353-jdi.o
 obj-$(CONFIG_DRM_PANEL_SONY_TULIP_TRULY_NT35521) += panel-sony-tulip-truly-nt35521.o
diff --git a/drivers/gpu/drm/panel/panel-synaptics-r63353.c b/drivers/gpu/drm/panel/panel-synaptics-r63353.c
new file mode 100644
index 000000000000..0a3c2a3d5998
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-synaptics-r63353.c
@@ -0,0 +1,363 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Synaptics R63353 Controller driver
+ *
+ * Copyright (C) 2020 BSH Hausgerate GmbH
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/fb.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/media-bus-format.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+
+#include <video/mipi_display.h>
+
+#define R63353_INSTR(...) { \
+		.len = sizeof((u8[]) {__VA_ARGS__}), \
+		.data = (u8[]){__VA_ARGS__} \
+	}
+
+struct r63353_instr {
+	size_t len;
+	const u8 *data;
+};
+
+static const struct r63353_instr sharp_ls068b3sx02_init[] = {
+	R63353_INSTR(0x51, 0xff),
+	R63353_INSTR(0x53, 0x0c),
+	R63353_INSTR(0x55, 0x00),
+	R63353_INSTR(0x84, 0x00),
+	R63353_INSTR(0x29),
+};
+
+struct r63353_desc {
+	const char *name;
+	const struct r63353_instr *init;
+	const size_t init_length;
+	const struct drm_display_mode *mode;
+	u32 width_mm;
+	u32 height_mm;
+};
+
+struct r63353_panel {
+	struct drm_panel base;
+	struct mipi_dsi_device *dsi;
+
+	struct gpio_desc *reset_gpio;
+	struct regulator *dvdd;
+	struct regulator *avdd;
+
+	struct r63353_desc *pdata;
+};
+
+static inline struct r63353_panel *to_r63353_panel(struct drm_panel *panel)
+{
+	return container_of(panel, struct r63353_panel, base);
+}
+
+static int r63353_panel_power_on(struct r63353_panel *rpanel)
+{
+	struct mipi_dsi_device *dsi = rpanel->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	ret = regulator_enable(rpanel->avdd);
+	if (ret) {
+		dev_err(dev, "Failed to enable avdd regulator (%d)\n", ret);
+		return ret;
+	}
+
+	usleep_range(15000, 25000);
+
+	ret = regulator_enable(rpanel->dvdd);
+	if (ret) {
+		dev_err(dev, "Failed to enable dvdd regulator (%d)\n", ret);
+		regulator_disable(rpanel->avdd);
+		return ret;
+	}
+
+	usleep_range(300000, 350000);
+	gpiod_set_value(rpanel->reset_gpio, 1);
+	usleep_range(15000, 25000);
+
+	return 0;
+}
+
+static int r63353_panel_power_off(struct r63353_panel *rpanel)
+{
+	gpiod_set_value(rpanel->reset_gpio, 0);
+	regulator_disable(rpanel->dvdd);
+	regulator_disable(rpanel->avdd);
+
+	return 0;
+}
+
+static int r63353_panel_activate(struct r63353_panel *rpanel)
+{
+	struct mipi_dsi_device *dsi = rpanel->dsi;
+	struct device *dev = &dsi->dev;
+	int i, ret;
+
+	ret = mipi_dsi_dcs_soft_reset(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to do Software Reset (%d)\n", ret);
+		goto fail;
+	}
+
+	usleep_range(15000, 17000);
+
+	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enter sleep mode (%d)\n", ret);
+		goto fail;
+	}
+
+	for (i = 0; i < rpanel->pdata->init_length; i++) {
+		const struct r63353_instr *instr = &rpanel->pdata->init[i];
+
+		ret = mipi_dsi_dcs_write_buffer(dsi, instr->data, instr->len);
+		if (ret < 0)
+			return ret;
+	}
+
+	msleep(120);
+
+	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to exit sleep mode (%d)\n", ret);
+		goto fail;
+	}
+
+	usleep_range(5000, 10000);
+
+	ret = mipi_dsi_dcs_set_display_on(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set display ON (%d)\n", ret);
+		goto fail;
+	}
+
+	return 0;
+
+fail:
+	gpiod_set_value(rpanel->reset_gpio, 0);
+
+	return ret;
+}
+
+static int r63353_panel_prepare(struct drm_panel *panel)
+{
+	struct r63353_panel *rpanel = to_r63353_panel(panel);
+	struct mipi_dsi_device *dsi = rpanel->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	dev_dbg(dev, "Preparing\n");
+
+	ret = r63353_panel_power_on(rpanel);
+	if (ret)
+		return ret;
+
+	ret = r63353_panel_activate(rpanel);
+	if (ret) {
+		r63353_panel_power_off(rpanel);
+		return ret;
+	}
+
+	dev_dbg(dev, "Prepared\n");
+	return 0;
+}
+
+static int r63353_panel_deactivate(struct r63353_panel *rpanel)
+{
+	struct mipi_dsi_device *dsi = rpanel->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	ret = mipi_dsi_dcs_set_display_off(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set display OFF (%d)\n", ret);
+		return ret;
+	}
+
+	usleep_range(5000, 10000);
+
+	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enter sleep mode (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int r63353_panel_unprepare(struct drm_panel *panel)
+{
+	struct r63353_panel *rpanel = to_r63353_panel(panel);
+
+	r63353_panel_deactivate(rpanel);
+	r63353_panel_power_off(rpanel);
+
+	return 0;
+}
+
+static const struct drm_display_mode sharp_ls068b3sx02_timing = {
+	.clock = 70000,
+	.hdisplay = 640,
+	.hsync_start = 640 + 35,
+	.hsync_end = 640 + 35 + 2,
+	.htotal = 640 + 35 + 2 + 150,
+	.vdisplay = 1280,
+	.vsync_start = 1280 + 2,
+	.vsync_end = 1280 + 2 + 4,
+	.vtotal = 1280 + 2 + 4 + 0,
+};
+
+static int r63353_panel_get_modes(struct drm_panel *panel,
+				  struct drm_connector *connector)
+{
+	struct r63353_panel *rpanel = to_r63353_panel(panel);
+	struct drm_display_mode *mode;
+	static const u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+
+	mode = drm_mode_duplicate(connector->dev, rpanel->pdata->mode);
+	if (!mode)
+		return -ENOMEM;
+
+	drm_mode_set_name(mode);
+	drm_mode_probed_add(connector, mode);
+
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	connector->display_info.width_mm = rpanel->pdata->width_mm;
+	connector->display_info.height_mm = rpanel->pdata->height_mm;
+
+	drm_display_info_set_bus_formats(&connector->display_info,
+					 &bus_format, 1);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs r63353_panel_funcs = {
+	.prepare = r63353_panel_prepare,
+	.unprepare = r63353_panel_unprepare,
+	.get_modes = r63353_panel_get_modes,
+};
+
+static int r63353_panel_probe(struct mipi_dsi_device *dsi)
+{
+	int ret = 0;
+	struct device *dev = &dsi->dev;
+	struct r63353_panel *panel;
+
+	panel = devm_kzalloc(&dsi->dev, sizeof(*panel), GFP_KERNEL);
+	if (!panel)
+		return -ENOMEM;
+
+	mipi_dsi_set_drvdata(dsi, panel);
+	panel->dsi = dsi;
+	panel->pdata = (struct r63353_desc *)of_device_get_match_data(dev);
+
+	dev_info(dev, "Panel %s\n", panel->pdata->name);
+
+	dsi->lanes = 2;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO_HSE | MIPI_DSI_MODE_VIDEO |
+			  MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM |
+			  MIPI_DSI_MODE_VIDEO_SYNC_PULSE | MIPI_DSI_MODE_NO_EOT_PACKET;
+
+	panel->dvdd = devm_regulator_get(dev, "dvdd");
+	if (IS_ERR(panel->dvdd))
+		return PTR_ERR(panel->dvdd);
+	panel->avdd = devm_regulator_get(dev, "avdd");
+	if (IS_ERR(panel->avdd))
+		return PTR_ERR(panel->avdd);
+
+	panel->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(panel->reset_gpio)) {
+		dev_err(dev, "failed to get RESET GPIO\n");
+		return PTR_ERR(panel->reset_gpio);
+	}
+
+	drm_panel_init(&panel->base, dev, &r63353_panel_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+
+	panel->base.prepare_prev_first = true;
+	ret = drm_panel_of_backlight(&panel->base);
+	if (ret)
+		return ret;
+
+	drm_panel_add(&panel->base);
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		dev_err(dev, "mipi_dsi_attach failed: %d\n", ret);
+		drm_panel_remove(&panel->base);
+		return ret;
+	}
+
+	return ret;
+}
+
+static void r63353_panel_remove(struct mipi_dsi_device *dsi)
+{
+	struct r63353_panel *rpanel = mipi_dsi_get_drvdata(dsi);
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	ret = mipi_dsi_detach(dsi);
+	if (ret < 0)
+		dev_err(dev, "Failed to detach from host (%d)\n", ret);
+
+	drm_panel_remove(&rpanel->base);
+}
+
+static void r63353_panel_shutdown(struct mipi_dsi_device *dsi)
+{
+	struct r63353_panel *rpanel = mipi_dsi_get_drvdata(dsi);
+
+	r63353_panel_unprepare(&rpanel->base);
+}
+
+static const struct r63353_desc sharp_ls068b3sx02_data = {
+	.name = "Sharp LS068B3SX02",
+	.mode = &sharp_ls068b3sx02_timing,
+	.init = sharp_ls068b3sx02_init,
+	.init_length = ARRAY_SIZE(sharp_ls068b3sx02_init),
+	.width_mm = 68,
+	.height_mm = 159,
+};
+
+static const struct of_device_id r63353_of_match[] = {
+	{ .compatible = "sharp,ls068b3sx02", .data = &sharp_ls068b3sx02_data },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, r63353_of_match);
+
+static struct mipi_dsi_driver r63353_panel_driver = {
+	.driver = {
+		   .name = "r63353-dsi",
+		   .of_match_table = r63353_of_match,
+	},
+	.probe = r63353_panel_probe,
+	.remove = r63353_panel_remove,
+	.shutdown = r63353_panel_shutdown,
+};
+
+module_mipi_dsi_driver(r63353_panel_driver);
+
+MODULE_AUTHOR("Matthias Proske <Matthias.Proske@bshg.com>");
+MODULE_AUTHOR("Michael Trimarchi <michael@amarulasolutions.com>");
+MODULE_DESCRIPTION("Synaptics R63353 Controller Driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller
  2023-12-05 10:52 [PATCH v4 00/10] Add displays support for bsh-smm-s2/pro boards Dario Binacchi
                   ` (5 preceding siblings ...)
  2023-12-05 10:52 ` [PATCH v4 06/10] drm/panel: Add Synaptics R63353 panel driver Dario Binacchi
@ 2023-12-05 10:52 ` Dario Binacchi
  2023-12-06 14:33   ` Rob Herring
  2023-12-05 10:52 ` [PATCH v4 08/10] drm/panel: Add Ilitek ILI9805 panel driver Dario Binacchi
  2023-12-05 10:52 ` [PATCH v4 09/10] drm/panel: ilitek-ili9805: add support for Tianma TM041XDHG01 panel Dario Binacchi
  8 siblings, 1 reply; 21+ messages in thread
From: Dario Binacchi @ 2023-12-05 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Neil Armstrong, Conor Dooley, Dario Binacchi,
	Krzysztof Kozlowski, Sam Ravnborg, devicetree, Amarula patchwork,
	dri-devel, Maxime Ripard, Rob Herring, Thomas Zimmermann,
	Jessica Zhang, michael

From: Michael Trimarchi <michael@amarulasolutions.com>

Add documentation for "ilitek,ili9805" panel.

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

---

(no changes since v3)

Changes in v3:
- Drop power-supply

Changes in v2:
- Add $ref to panel-common.yaml
- Drop port, reset-gpios, and backlight
- Set port and backlight ad required
- Replace additionalProperties with unevaluatedProperties

 .../display/panel/ilitek,ili9805.yaml         | 62 +++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9805.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/ilitek,ili9805.yaml b/Documentation/devicetree/bindings/display/panel/ilitek,ili9805.yaml
new file mode 100644
index 000000000000..f4f91f93f490
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/ilitek,ili9805.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/ilitek,ili9805.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ilitek ILI9805 based MIPI-DSI panels
+
+maintainers:
+  - Michael Trimarchi <michael@amarulasolutions.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - giantplus,gpm1790a0
+          - tianma,tm041xdhg01
+      - const: ilitek,ili9805
+
+  avdd-supply: true
+  dvdd-supply: true
+  reg: true
+
+required:
+  - compatible
+  - avdd-supply
+  - dvdd-supply
+  - reg
+  - reset-gpios
+  - port
+  - backlight
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    dsi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        panel@0 {
+            compatible = "giantplus,gpm1790a0", "ilitek,ili9805";
+            reg = <0>;
+            avdd-supply = <&avdd_display>;
+            dvdd-supply = <&dvdd_display>;
+            reset-gpios = <&r_pio 0 5 GPIO_ACTIVE_LOW>; /* PL05 */
+            backlight = <&backlight>;
+
+            port {
+                panel_in: endpoint {
+                    remote-endpoint = <&mipi_dsi_out>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 08/10] drm/panel: Add Ilitek ILI9805 panel driver
  2023-12-05 10:52 [PATCH v4 00/10] Add displays support for bsh-smm-s2/pro boards Dario Binacchi
                   ` (6 preceding siblings ...)
  2023-12-05 10:52 ` [PATCH v4 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller Dario Binacchi
@ 2023-12-05 10:52 ` Dario Binacchi
  2023-12-05 10:52 ` [PATCH v4 09/10] drm/panel: ilitek-ili9805: add support for Tianma TM041XDHG01 panel Dario Binacchi
  8 siblings, 0 replies; 21+ messages in thread
From: Dario Binacchi @ 2023-12-05 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Neil Armstrong, Dario Binacchi, Thomas Zimmermann, Sam Ravnborg,
	Amarula patchwork, Maxime Ripard, dri-devel, Jessica Zhang,
	michael

From: Michael Trimarchi <michael@amarulasolutions.com>

The GPM1790A0 panel is based on the Ilitek ILI9805 Controller.
Add a driver for it.

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

---

Changes in v4:
- Remove duplicated code for prepare/unprepare callbacks

 MAINTAINERS                                  |   6 +
 drivers/gpu/drm/panel/Kconfig                |   9 +
 drivers/gpu/drm/panel/Makefile               |   1 +
 drivers/gpu/drm/panel/panel-ilitek-ili9805.c | 353 +++++++++++++++++++
 4 files changed, 369 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9805.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b82dc141d209..4dccc72a0ed6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6646,6 +6646,12 @@ T:	git git://anongit.freedesktop.org/drm/drm-misc
 F:	Documentation/devicetree/bindings/display/ilitek,ili9486.yaml
 F:	drivers/gpu/drm/tiny/ili9486.c
 
+DRM DRIVER FOR ILITEK ILI9805 PANELS
+M:	Michael Trimarchi <michael@amarulasolutions.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/display/panel/ilitek,ili9805.yaml
+F:	drivers/gpu/drm/panel/panel-ilitek-ili9805.c
+
 DRM DRIVER FOR JADARD JD9365DA-H3 MIPI-DSI LCD PANELS
 M:	Jagan Teki <jagan@edgeble.ai>
 S:	Maintained
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index d018702be3dc..dad938cf6dec 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -194,6 +194,15 @@ config DRM_PANEL_ILITEK_ILI9341
 	  QVGA (240x320) RGB panels. support serial & parallel rgb
 	  interface.
 
+config DRM_PANEL_ILITEK_ILI9805
+	tristate "Ilitek ILI9805-based panels"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y if you want to enable support for panels based on the
+	  Ilitek ILI9805 controller.
+
 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 f267d932c2b5..d94a644d0a6c 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_DRM_PANEL_FEIYANG_FY07024DI26A30D) += panel-feiyang-fy07024di26a30d
 obj-$(CONFIG_DRM_PANEL_HIMAX_HX8394) += panel-himax-hx8394.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_IL9322) += panel-ilitek-ili9322.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9341) += panel-ilitek-ili9341.o
+obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9805) += panel-ilitek-ili9805.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9881C) += panel-ilitek-ili9881c.o
 obj-$(CONFIG_DRM_PANEL_ILITEK_ILI9882T) += panel-ilitek-ili9882t.o
 obj-$(CONFIG_DRM_PANEL_INNOLUX_EJ030NA) += panel-innolux-ej030na.o
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9805.c b/drivers/gpu/drm/panel/panel-ilitek-ili9805.c
new file mode 100644
index 000000000000..e36984b46e14
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9805.c
@@ -0,0 +1,353 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 BSH Hausgerate GmbH
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/fb.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+
+#include <video/mipi_display.h>
+
+#define ILI9805_EXTCMD_CMD_SET_ENABLE_REG	(0xff)
+#define ILI9805_SETEXTC_PARAMETER1		(0xff)
+#define ILI9805_SETEXTC_PARAMETER2		(0x98)
+#define ILI9805_SETEXTC_PARAMETER3		(0x05)
+
+#define ILI9805_INSTR(_delay, ...) { \
+		.delay = (_delay), \
+		.len = sizeof((u8[]) {__VA_ARGS__}), \
+		.data = (u8[]){__VA_ARGS__} \
+	}
+
+struct ili9805_instr {
+	size_t len;
+	const u8 *data;
+	u32 delay;
+};
+
+struct ili9805_desc {
+	const char *name;
+	const struct ili9805_instr *init;
+	const size_t init_length;
+	const struct drm_display_mode *mode;
+	u32 width_mm;
+	u32 height_mm;
+};
+
+struct ili9805 {
+	struct drm_panel	panel;
+	struct mipi_dsi_device	*dsi;
+	const struct ili9805_desc	*desc;
+
+	struct regulator	*dvdd;
+	struct regulator	*avdd;
+	struct gpio_desc	*reset_gpio;
+};
+
+static const struct ili9805_instr gpm1780a0_init[] = {
+	ILI9805_INSTR(100, ILI9805_EXTCMD_CMD_SET_ENABLE_REG, ILI9805_SETEXTC_PARAMETER1,
+		      ILI9805_SETEXTC_PARAMETER2, ILI9805_SETEXTC_PARAMETER3),
+	ILI9805_INSTR(100, 0xFD, 0x0F, 0x10, 0x44, 0x00),
+	ILI9805_INSTR(0, 0xf8, 0x18, 0x02, 0x02, 0x18, 0x02, 0x02, 0x30, 0x00,
+		      0x00, 0x30, 0x00, 0x00, 0x30, 0x00, 0x00),
+	ILI9805_INSTR(0, 0xB8, 0x62),
+	ILI9805_INSTR(0, 0xF1, 0x00),
+	ILI9805_INSTR(0, 0xF2, 0x00, 0x58, 0x40),
+	ILI9805_INSTR(0, 0xF3, 0x60, 0x83, 0x04),
+	ILI9805_INSTR(0, 0xFC, 0x04, 0x0F, 0x01),
+	ILI9805_INSTR(0, 0xEB, 0x08, 0x0F),
+	ILI9805_INSTR(0, 0xe0, 0x00, 0x08, 0x0d, 0x0e, 0x0e, 0x0d, 0x0a, 0x08, 0x04,
+		      0x08, 0x0d, 0x0f, 0x0b, 0x1c, 0x14, 0x0a),
+	ILI9805_INSTR(0, 0xe1, 0x00, 0x08, 0x0d, 0x0e, 0x0e, 0x0d, 0x0a, 0x08, 0x04,
+		      0x08, 0x0d, 0x0f, 0x0b, 0x1c, 0x14, 0x0a),
+	ILI9805_INSTR(10, 0xc1, 0x13, 0x39, 0x19, 0x06),
+	ILI9805_INSTR(10, 0xc7, 0xe5),
+	ILI9805_INSTR(10, 0xB1, 0x00, 0x12, 0x14),
+	ILI9805_INSTR(10, 0xB4, 0x02),
+	ILI9805_INSTR(0, 0xBB, 0x14, 0x55),
+	ILI9805_INSTR(0, MIPI_DCS_SET_ADDRESS_MODE, 0x08),
+	ILI9805_INSTR(0, MIPI_DCS_SET_PIXEL_FORMAT, 0x77),
+	ILI9805_INSTR(0, 0x20),
+	ILI9805_INSTR(0, 0xB0, 0x01),
+	ILI9805_INSTR(0, 0xB6, 0x31, 0x00, 0xef),
+	ILI9805_INSTR(0, 0xDF, 0x23),
+	ILI9805_INSTR(0, 0xB9, 0x02, 0x00),
+};
+
+static inline struct ili9805 *panel_to_ili9805(struct drm_panel *panel)
+{
+	return container_of(panel, struct ili9805, panel);
+}
+
+static int ili9805_power_on(struct ili9805 *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	ret = regulator_enable(ctx->avdd);
+	if (ret) {
+		dev_err(dev, "Failed to enable avdd regulator (%d)\n", ret);
+		return ret;
+	}
+
+	ret = regulator_enable(ctx->dvdd);
+	if (ret) {
+		dev_err(dev, "Failed to enable dvdd regulator (%d)\n", ret);
+		regulator_disable(ctx->avdd);
+		return ret;
+	}
+
+	gpiod_set_value(ctx->reset_gpio, 0);
+	usleep_range(5000, 10000);
+	gpiod_set_value(ctx->reset_gpio, 1);
+	msleep(120);
+
+	return 0;
+}
+
+static int ili9805_power_off(struct ili9805 *ctx)
+{
+	gpiod_set_value(ctx->reset_gpio, 0);
+	regulator_disable(ctx->dvdd);
+	regulator_disable(ctx->avdd);
+
+	return 0;
+}
+
+static int ili9805_activate(struct ili9805 *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int i, ret;
+
+	for (i = 0; i < ctx->desc->init_length; i++) {
+		const struct ili9805_instr *instr = &ctx->desc->init[i];
+
+		ret = mipi_dsi_dcs_write_buffer(ctx->dsi, instr->data, instr->len);
+		if (ret < 0)
+			return ret;
+
+		if (instr->delay > 0)
+			msleep(instr->delay);
+	}
+
+	ret = mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
+	if (ret) {
+		dev_err(dev, "Failed to exit sleep mode (%d)\n", ret);
+		return ret;
+	}
+
+	usleep_range(5000, 6000);
+
+	ret = mipi_dsi_dcs_set_display_on(ctx->dsi);
+	if (ret) {
+		dev_err(dev, "Failed to set display ON (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ili9805_prepare(struct drm_panel *panel)
+{
+	struct ili9805 *ctx = panel_to_ili9805(panel);
+	int ret;
+
+	ret = ili9805_power_on(ctx);
+	if (ret)
+		return ret;
+
+	ret = ili9805_activate(ctx);
+	if (ret) {
+		ili9805_power_off(ctx);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ili9805_deactivate(struct ili9805 *ctx)
+{
+	struct mipi_dsi_device *dsi = ctx->dsi;
+	struct device *dev = &dsi->dev;
+	int ret;
+
+	ret = mipi_dsi_dcs_set_display_off(ctx->dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set display OFF (%d)\n", ret);
+		return ret;
+	}
+
+	usleep_range(5000, 10000);
+
+	ret = mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enter sleep mode (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ili9805_unprepare(struct drm_panel *panel)
+{
+	struct ili9805 *ctx = panel_to_ili9805(panel);
+
+	ili9805_deactivate(ctx);
+	ili9805_power_off(ctx);
+
+	return 0;
+}
+
+static const struct drm_display_mode gpm1780a0_timing = {
+	.clock = 26227,
+
+	.hdisplay = 480,
+	.hsync_start = 480 + 10,
+	.hsync_end = 480 + 10 + 2,
+	.htotal = 480 + 10 + 2 + 36,
+
+	.vdisplay = 480,
+	.vsync_start = 480 + 2,
+	.vsync_end = 480 + 10 + 4,
+	.vtotal = 480 + 2 + 4 + 10,
+};
+
+static int ili9805_get_modes(struct drm_panel *panel,
+			      struct drm_connector *connector)
+{
+	struct ili9805 *ctx = panel_to_ili9805(panel);
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_duplicate(connector->dev, ctx->desc->mode);
+	if (!mode) {
+		dev_err(&ctx->dsi->dev, "failed to add mode %ux%ux@%u\n",
+			ctx->desc->mode->hdisplay,
+			ctx->desc->mode->vdisplay,
+			drm_mode_vrefresh(ctx->desc->mode));
+		return -ENOMEM;
+	}
+
+	drm_mode_set_name(mode);
+
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_probed_add(connector, mode);
+
+	connector->display_info.width_mm = mode->width_mm;
+	connector->display_info.height_mm = mode->height_mm;
+
+	return 1;
+}
+
+static const struct drm_panel_funcs ili9805_funcs = {
+	.prepare	= ili9805_prepare,
+	.unprepare	= ili9805_unprepare,
+	.get_modes	= ili9805_get_modes,
+};
+
+static int ili9805_dsi_probe(struct mipi_dsi_device *dsi)
+{
+	struct ili9805 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	mipi_dsi_set_drvdata(dsi, ctx);
+	ctx->dsi = dsi;
+	ctx->desc = of_device_get_match_data(&dsi->dev);
+
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->mode_flags = MIPI_DSI_MODE_VIDEO_HSE | MIPI_DSI_MODE_VIDEO |
+		MIPI_DSI_CLOCK_NON_CONTINUOUS | MIPI_DSI_MODE_LPM |
+		MIPI_DSI_MODE_VIDEO_SYNC_PULSE | MIPI_DSI_MODE_NO_EOT_PACKET;
+	dsi->lanes = 2;
+
+	drm_panel_init(&ctx->panel, &dsi->dev, &ili9805_funcs,
+		       DRM_MODE_CONNECTOR_DSI);
+
+	ctx->dvdd = devm_regulator_get(&dsi->dev, "dvdd");
+	if (IS_ERR(ctx->dvdd))
+		return PTR_ERR(ctx->dvdd);
+	ctx->avdd = devm_regulator_get(&dsi->dev, "avdd");
+	if (IS_ERR(ctx->avdd))
+		return PTR_ERR(ctx->avdd);
+
+	ctx->reset_gpio = devm_gpiod_get(&dsi->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->reset_gpio)) {
+		dev_err(&dsi->dev, "Couldn't get our reset GPIO\n");
+		return PTR_ERR(ctx->reset_gpio);
+	}
+
+	ctx->panel.prepare_prev_first = true;
+	ret = drm_panel_of_backlight(&ctx->panel);
+	if (ret)
+		return ret;
+
+	drm_panel_add(&ctx->panel);
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret < 0) {
+		dev_err(&dsi->dev, "mipi_dsi_attach failed: %d\n", ret);
+		drm_panel_remove(&ctx->panel);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void ili9805_dsi_remove(struct mipi_dsi_device *dsi)
+{
+	struct ili9805 *ctx = mipi_dsi_get_drvdata(dsi);
+	int ret;
+
+	ret = mipi_dsi_detach(dsi);
+	if (ret < 0)
+		dev_err(&dsi->dev, "failed to detach from DSI host: %d\n",
+			ret);
+
+	drm_panel_remove(&ctx->panel);
+}
+
+static const struct ili9805_desc gpm1780a0_desc = {
+	.init = gpm1780a0_init,
+	.init_length = ARRAY_SIZE(gpm1780a0_init),
+	.mode = &gpm1780a0_timing,
+	.width_mm = 65,
+	.height_mm = 65,
+};
+
+static const struct of_device_id ili9805_of_match[] = {
+	{ .compatible = "giantplus,gpm1790a0", .data = &gpm1780a0_desc },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ili9805_of_match);
+
+static struct mipi_dsi_driver ili9805_dsi_driver = {
+	.probe		= ili9805_dsi_probe,
+	.remove		= ili9805_dsi_remove,
+	.driver = {
+		.name		= "ili9805-dsi",
+		.of_match_table	= ili9805_of_match,
+	},
+};
+module_mipi_dsi_driver(ili9805_dsi_driver);
+
+MODULE_AUTHOR("Matthias Proske <Matthias.Proske@bshg.com>");
+MODULE_AUTHOR("Michael Trimarchi <michael@amarulasolutions.com>");
+MODULE_DESCRIPTION("Ilitek ILI9805 Controller Driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 09/10] drm/panel: ilitek-ili9805: add support for Tianma TM041XDHG01 panel
  2023-12-05 10:52 [PATCH v4 00/10] Add displays support for bsh-smm-s2/pro boards Dario Binacchi
                   ` (7 preceding siblings ...)
  2023-12-05 10:52 ` [PATCH v4 08/10] drm/panel: Add Ilitek ILI9805 panel driver Dario Binacchi
@ 2023-12-05 10:52 ` Dario Binacchi
  8 siblings, 0 replies; 21+ messages in thread
From: Dario Binacchi @ 2023-12-05 10:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Neil Armstrong, Dario Binacchi, Thomas Zimmermann, Sam Ravnborg,
	Amarula patchwork, Maxime Ripard, dri-devel, Jessica Zhang,
	michael

From: Michael Trimarchi <michael@amarulasolutions.com>

Tianma TM041XDHG01 utilizes the Ilitek ILI9805 controller.

Add this panel's initialzation sequence and timing to ILI9805 driver.

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---

Changes in v4:
- Add Reviewed-by tag of Neil Armstrong

 drivers/gpu/drm/panel/panel-ilitek-ili9805.c | 53 ++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9805.c b/drivers/gpu/drm/panel/panel-ilitek-ili9805.c
index e36984b46e14..5054d1a2b2f5 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9805.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9805.c
@@ -87,6 +87,36 @@ static const struct ili9805_instr gpm1780a0_init[] = {
 	ILI9805_INSTR(0, 0xB9, 0x02, 0x00),
 };
 
+static const struct ili9805_instr tm041xdhg01_init[] = {
+	ILI9805_INSTR(100, ILI9805_EXTCMD_CMD_SET_ENABLE_REG, ILI9805_SETEXTC_PARAMETER1,
+		      ILI9805_SETEXTC_PARAMETER2, ILI9805_SETEXTC_PARAMETER3),
+	ILI9805_INSTR(100, 0xFD, 0x0F, 0x13, 0x44, 0x00),
+	ILI9805_INSTR(0, 0xf8, 0x18, 0x02, 0x02, 0x18, 0x02, 0x02, 0x30, 0x01,
+		      0x01, 0x30, 0x01, 0x01, 0x30, 0x01, 0x01),
+	ILI9805_INSTR(0, 0xB8, 0x74),
+	ILI9805_INSTR(0, 0xF1, 0x00),
+	ILI9805_INSTR(0, 0xF2, 0x00, 0x58, 0x40),
+	ILI9805_INSTR(0, 0xFC, 0x04, 0x0F, 0x01),
+	ILI9805_INSTR(0, 0xEB, 0x08, 0x0F),
+	ILI9805_INSTR(0, 0xe0, 0x01, 0x0d, 0x15, 0x0e, 0x0f, 0x0f, 0x0b, 0x08, 0x04,
+		      0x07, 0x0a, 0x0d, 0x0c, 0x15, 0x0f, 0x08),
+	ILI9805_INSTR(0, 0xe1, 0x01, 0x0d, 0x15, 0x0e, 0x0f, 0x0f, 0x0b, 0x08, 0x04,
+		      0x07, 0x0a, 0x0d, 0x0c, 0x15, 0x0f, 0x08),
+	ILI9805_INSTR(10, 0xc1, 0x15, 0x03, 0x03, 0x31),
+	ILI9805_INSTR(10, 0xB1, 0x00, 0x12, 0x14),
+	ILI9805_INSTR(10, 0xB4, 0x02),
+	ILI9805_INSTR(0, 0xBB, 0x14, 0x55),
+	ILI9805_INSTR(0, MIPI_DCS_SET_ADDRESS_MODE, 0x0a),
+	ILI9805_INSTR(0, MIPI_DCS_SET_PIXEL_FORMAT, 0x77),
+	ILI9805_INSTR(0, 0x20),
+	ILI9805_INSTR(0, 0xB0, 0x00),
+	ILI9805_INSTR(0, 0xB6, 0x01),
+	ILI9805_INSTR(0, 0xc2, 0x11),
+	ILI9805_INSTR(0, 0x51, 0xFF),
+	ILI9805_INSTR(0, 0x53, 0x24),
+	ILI9805_INSTR(0, 0x55, 0x00),
+};
+
 static inline struct ili9805 *panel_to_ili9805(struct drm_panel *panel)
 {
 	return container_of(panel, struct ili9805, panel);
@@ -227,6 +257,20 @@ static const struct drm_display_mode gpm1780a0_timing = {
 	.vtotal = 480 + 2 + 4 + 10,
 };
 
+static const struct drm_display_mode tm041xdhg01_timing = {
+	.clock = 26227,
+
+	.hdisplay = 480,
+	.hsync_start = 480 + 10,
+	.hsync_end = 480 + 10 + 2,
+	.htotal = 480 + 10 + 2 + 36,
+
+	.vdisplay = 768,
+	.vsync_start = 768 + 2,
+	.vsync_end = 768 + 10 + 4,
+	.vtotal = 768 + 2 + 4 + 10,
+};
+
 static int ili9805_get_modes(struct drm_panel *panel,
 			      struct drm_connector *connector)
 {
@@ -331,8 +375,17 @@ static const struct ili9805_desc gpm1780a0_desc = {
 	.height_mm = 65,
 };
 
+static const struct ili9805_desc tm041xdhg01_desc = {
+	.init = tm041xdhg01_init,
+	.init_length = ARRAY_SIZE(tm041xdhg01_init),
+	.mode = &tm041xdhg01_timing,
+	.width_mm = 42,
+	.height_mm = 96,
+};
+
 static const struct of_device_id ili9805_of_match[] = {
 	{ .compatible = "giantplus,gpm1790a0", .data = &gpm1780a0_desc },
+	{ .compatible = "tianma,tm041xdhg01", .data = &tm041xdhg01_desc },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ili9805_of_match);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic
  2023-12-05 10:52 ` [PATCH v4 02/10] drm/bridge: Fix a use case in the " Dario Binacchi
@ 2023-12-05 15:39   ` Dave Stevenson
  2023-12-06 13:26     ` Dario Binacchi
  0 siblings, 1 reply; 21+ messages in thread
From: Dave Stevenson @ 2023-12-05 15:39 UTC (permalink / raw)
  To: Dario Binacchi
  Cc: Maxime Ripard, Neil Armstrong, Robert Foss, Andrzej Hajda,
	Thomas Zimmermann, Jonas Karlman, Amarula patchwork, dri-devel,
	linux-kernel, Frieder Schrempf, Jernej Skrabec, michael,
	Laurent Pinchart

Hi Dario

On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
<dario.binacchi@amarulasolutions.com> wrote:
>
> The patch fixes the code for finding the next bridge with the
> "pre_enable_prev_first" flag set to false. In case this condition is
> not verified, i. e. there is no subsequent bridge with the flag set to
> false, the whole bridge list is traversed, invalidating the "next"
> variable.
>
> The use of a new iteration variable (i. e. "iter") ensures that the value
> of the "next" variable is not invalidated.

We already have https://patchwork.freedesktop.org/patch/529288/ that
has been reviewed (but not applied) to resolve this. What does this
version do differently and why?

  Dave

> Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to alter bridge init order")
> Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---
>
> (no changes since v1)
>
>  drivers/gpu/drm/drm_bridge.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index f66bf4925dd8..2e5781bf192e 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -662,7 +662,7 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
>                                           struct drm_atomic_state *old_state)
>  {
>         struct drm_encoder *encoder;
> -       struct drm_bridge *next, *limit;
> +       struct drm_bridge *iter, *next, *limit;
>
>         if (!bridge)
>                 return;
> @@ -680,14 +680,15 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
>                                  * was enabled first, so disabled last
>                                  */
>                                 limit = next;
> +                               iter = next;
>
>                                 /* Find the next bridge that has NOT requested
>                                  * prev to be enabled first / disabled last
>                                  */
> -                               list_for_each_entry_from(next, &encoder->bridge_chain,
> +                               list_for_each_entry_from(iter, &encoder->bridge_chain,
>                                                          chain_node) {
> -                                       if (!next->pre_enable_prev_first) {
> -                                               next = list_prev_entry(next, chain_node);
> +                                       if (!iter->pre_enable_prev_first) {
> +                                               next = list_prev_entry(iter, chain_node);
>                                                 limit = next;
>                                                 break;
>                                         }
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic
  2023-12-05 15:39   ` Dave Stevenson
@ 2023-12-06 13:26     ` Dario Binacchi
  2023-12-06 13:31       ` Jagan Teki
  0 siblings, 1 reply; 21+ messages in thread
From: Dario Binacchi @ 2023-12-06 13:26 UTC (permalink / raw)
  To: Dave Stevenson, Jagan Teki
  Cc: Maxime Ripard, Neil Armstrong, Robert Foss, Andrzej Hajda,
	Thomas Zimmermann, Jonas Karlman, Amarula patchwork, dri-devel,
	linux-kernel, Frieder Schrempf, Jernej Skrabec, michael,
	Laurent Pinchart

Hi Dave and Jagan,

On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson
<dave.stevenson@raspberrypi.com> wrote:
>
> Hi Dario
>
> On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
> <dario.binacchi@amarulasolutions.com> wrote:
> >
> > The patch fixes the code for finding the next bridge with the
> > "pre_enable_prev_first" flag set to false. In case this condition is
> > not verified, i. e. there is no subsequent bridge with the flag set to
> > false, the whole bridge list is traversed, invalidating the "next"
> > variable.
> >
> > The use of a new iteration variable (i. e. "iter") ensures that the value
> > of the "next" variable is not invalidated.
>
> We already have https://patchwork.freedesktop.org/patch/529288/ that
> has been reviewed (but not applied) to resolve this. What does this
> version do differently and why?

My patches only affect drm_atomic_bridge_chain_post_disable(), whereas
Jagan's patch affects both
drm_atomic_bridge_chain_post_disable() and drm_atomic_bridge_chain_pre_enable().
I tested Jagan's patch on my system with success and I reviewed with
Michael Trimarchi the
drm_atomic_bridge_chain_pre_enable() fixing and we think it's okay.
We also believe that our changes to post_disable() are better, as we
set the 'next' variable only when required,
and the code is more optimized since the list_is_last() is not called
within the loop.
Would it be possible to use Jagan's patch for fixing
drm_atomic_bridge_chain_pre_enable() and mine for
fixing drm_atomic_bridge_chain_post_disable()?

Thanks and regards,
Dario

>
>   Dave
>
> > Fixes: 4fb912e5e190 ("drm/bridge: Introduce pre_enable_prev_first to alter bridge init order")
> > Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > ---
> >
> > (no changes since v1)
> >
> >  drivers/gpu/drm/drm_bridge.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index f66bf4925dd8..2e5781bf192e 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c
> > @@ -662,7 +662,7 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
> >                                           struct drm_atomic_state *old_state)
> >  {
> >         struct drm_encoder *encoder;
> > -       struct drm_bridge *next, *limit;
> > +       struct drm_bridge *iter, *next, *limit;
> >
> >         if (!bridge)
> >                 return;
> > @@ -680,14 +680,15 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
> >                                  * was enabled first, so disabled last
> >                                  */
> >                                 limit = next;
> > +                               iter = next;
> >
> >                                 /* Find the next bridge that has NOT requested
> >                                  * prev to be enabled first / disabled last
> >                                  */
> > -                               list_for_each_entry_from(next, &encoder->bridge_chain,
> > +                               list_for_each_entry_from(iter, &encoder->bridge_chain,
> >                                                          chain_node) {
> > -                                       if (!next->pre_enable_prev_first) {
> > -                                               next = list_prev_entry(next, chain_node);
> > +                                       if (!iter->pre_enable_prev_first) {
> > +                                               next = list_prev_entry(iter, chain_node);
> >                                                 limit = next;
> >                                                 break;
> >                                         }
> > --
> > 2.43.0
> >

-- 

Dario Binacchi

Senior Embedded Linux Developer

dario.binacchi@amarulasolutions.com

__________________________________


Amarula Solutions SRL

Via Le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 042 243 5310
info@amarulasolutions.com

www.amarulasolutions.com

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic
  2023-12-06 13:26     ` Dario Binacchi
@ 2023-12-06 13:31       ` Jagan Teki
  2023-12-06 13:57         ` Michael Nazzareno Trimarchi
  0 siblings, 1 reply; 21+ messages in thread
From: Jagan Teki @ 2023-12-06 13:31 UTC (permalink / raw)
  To: Dario Binacchi
  Cc: Maxime Ripard, Neil Armstrong, Robert Foss, Andrzej Hajda,
	Thomas Zimmermann, Dave Stevenson, dri-devel, Jonas Karlman,
	linux-kernel, Frieder Schrempf, Jernej Skrabec, michael,
	Amarula patchwork, Laurent Pinchart

Hi Dario,

On Wed, Dec 6, 2023 at 6:57 PM Dario Binacchi
<dario.binacchi@amarulasolutions.com> wrote:
>
> Hi Dave and Jagan,
>
> On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson
> <dave.stevenson@raspberrypi.com> wrote:
> >
> > Hi Dario
> >
> > On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
> > <dario.binacchi@amarulasolutions.com> wrote:
> > >
> > > The patch fixes the code for finding the next bridge with the
> > > "pre_enable_prev_first" flag set to false. In case this condition is
> > > not verified, i. e. there is no subsequent bridge with the flag set to
> > > false, the whole bridge list is traversed, invalidating the "next"
> > > variable.
> > >
> > > The use of a new iteration variable (i. e. "iter") ensures that the value
> > > of the "next" variable is not invalidated.
> >
> > We already have https://patchwork.freedesktop.org/patch/529288/ that
> > has been reviewed (but not applied) to resolve this. What does this
> > version do differently and why?
>
> My patches only affect drm_atomic_bridge_chain_post_disable(), whereas
> Jagan's patch affects both
> drm_atomic_bridge_chain_post_disable() and drm_atomic_bridge_chain_pre_enable().
> I tested Jagan's patch on my system with success and I reviewed with
> Michael Trimarchi the
> drm_atomic_bridge_chain_pre_enable() fixing and we think it's okay.
> We also believe that our changes to post_disable() are better, as we
> set the 'next' variable only when required,
> and the code is more optimized since the list_is_last() is not called
> within the loop.
> Would it be possible to use Jagan's patch for fixing
> drm_atomic_bridge_chain_pre_enable() and mine for
> fixing drm_atomic_bridge_chain_post_disable()?
>

Can you please share the post-disabled bridge chain list with the
below example before and after your change?

Example:
- Panel
- Bridge 1
- Bridge 2 pre_enable_prev_first
- Bridge 3
- Bridge 4 pre_enable_prev_first
- Bridge 5 pre_enable_prev_first
- Bridge 6
- Encoder

Thanks,
Jagan.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic
  2023-12-06 13:31       ` Jagan Teki
@ 2023-12-06 13:57         ` Michael Nazzareno Trimarchi
  2023-12-13 11:59           ` Dario Binacchi
  0 siblings, 1 reply; 21+ messages in thread
From: Michael Nazzareno Trimarchi @ 2023-12-06 13:57 UTC (permalink / raw)
  To: Jagan Teki
  Cc: Maxime Ripard, Neil Armstrong, Robert Foss, Andrzej Hajda,
	Thomas Zimmermann, Dave Stevenson, dri-devel, Jonas Karlman,
	linux-kernel, Frieder Schrempf, Jernej Skrabec, Dario Binacchi,
	Amarula patchwork, Laurent Pinchart

Hi Jagan

On Wed, Dec 6, 2023 at 2:31 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Hi Dario,
>
> On Wed, Dec 6, 2023 at 6:57 PM Dario Binacchi
> <dario.binacchi@amarulasolutions.com> wrote:
> >
> > Hi Dave and Jagan,
> >
> > On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson
> > <dave.stevenson@raspberrypi.com> wrote:
> > >
> > > Hi Dario
> > >
> > > On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
> > > <dario.binacchi@amarulasolutions.com> wrote:
> > > >
> > > > The patch fixes the code for finding the next bridge with the
> > > > "pre_enable_prev_first" flag set to false. In case this condition is
> > > > not verified, i. e. there is no subsequent bridge with the flag set to
> > > > false, the whole bridge list is traversed, invalidating the "next"
> > > > variable.
> > > >
> > > > The use of a new iteration variable (i. e. "iter") ensures that the value
> > > > of the "next" variable is not invalidated.
> > >
> > > We already have https://patchwork.freedesktop.org/patch/529288/ that
> > > has been reviewed (but not applied) to resolve this. What does this
> > > version do differently and why?
> >
> > My patches only affect drm_atomic_bridge_chain_post_disable(), whereas
> > Jagan's patch affects both
> > drm_atomic_bridge_chain_post_disable() and drm_atomic_bridge_chain_pre_enable().
> > I tested Jagan's patch on my system with success and I reviewed with
> > Michael Trimarchi the
> > drm_atomic_bridge_chain_pre_enable() fixing and we think it's okay.
> > We also believe that our changes to post_disable() are better, as we
> > set the 'next' variable only when required,
> > and the code is more optimized since the list_is_last() is not called
> > within the loop.
> > Would it be possible to use Jagan's patch for fixing
> > drm_atomic_bridge_chain_pre_enable() and mine for
> > fixing drm_atomic_bridge_chain_post_disable()?
> >
>
> Can you please share the post-disabled bridge chain list with the
> below example before and after your change?

We have already git commit the description in how the patch affects
the post_disable. As Dario
reported your patch is ok even in our use case. We don't have a real
use case as the one you describe.

Can we know how you test it in this use case here? Can you test our
patches of post_disable?

Thanks
Michael

>
> Example:
> - Panel
> - Bridge 1
> - Bridge 2 pre_enable_prev_first
> - Bridge 3
> - Bridge 4 pre_enable_prev_first
> - Bridge 5 pre_enable_prev_first
> - Bridge 6
> - Encoder
>
> Thanks,
> Jagan.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller
  2023-12-05 10:52 ` [PATCH v4 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller Dario Binacchi
@ 2023-12-06 14:33   ` Rob Herring
  2023-12-07  9:29     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 21+ messages in thread
From: Rob Herring @ 2023-12-06 14:33 UTC (permalink / raw)
  To: Dario Binacchi
  Cc: Neil Armstrong, Conor Dooley, Krzysztof Kozlowski, Sam Ravnborg,
	devicetree, Amarula patchwork, dri-devel, linux-kernel,
	Maxime Ripard, Thomas Zimmermann, Jessica Zhang, michael

On Tue, Dec 05, 2023 at 11:52:54AM +0100, Dario Binacchi wrote:
> From: Michael Trimarchi <michael@amarulasolutions.com>
> 
> Add documentation for "ilitek,ili9805" panel.
> 
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

Where's Krzysztof's Reviewed-by?

Rob

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 06/10] drm/panel: Add Synaptics R63353 panel driver
  2023-12-05 10:52 ` [PATCH v4 06/10] drm/panel: Add Synaptics R63353 panel driver Dario Binacchi
@ 2023-12-07  5:26   ` Dan Carpenter
  0 siblings, 0 replies; 21+ messages in thread
From: Dan Carpenter @ 2023-12-07  5:26 UTC (permalink / raw)
  To: oe-kbuild, Dario Binacchi, linux-kernel
  Cc: Neil Armstrong, michael, lkp, Amarula patchwork, Maxime Ripard,
	dri-devel, Thomas Zimmermann, oe-kbuild-all, Jessica Zhang,
	Dario Binacchi, Sam Ravnborg

Hi Dario,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Dario-Binacchi/drm-bridge-Fix-bridge-disable-logic/20231205-185455
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20231205105341.4100896-7-dario.binacchi%40amarulasolutions.com
patch subject: [PATCH v4 06/10] drm/panel: Add Synaptics R63353 panel driver
config: i386-randconfig-r071-20231206 (https://download.01.org/0day-ci/archive/20231207/202312070214.Eyi9T4eq-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231207/202312070214.Eyi9T4eq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202312070214.Eyi9T4eq-lkp@intel.com/

smatch warnings:
drivers/gpu/drm/panel/panel-synaptics-r63353.c:132 r63353_panel_activate() warn: missing unwind goto?

vim +132 drivers/gpu/drm/panel/panel-synaptics-r63353.c

64f91a53613aea Michael Trimarchi 2023-12-05  107  static int r63353_panel_activate(struct r63353_panel *rpanel)
64f91a53613aea Michael Trimarchi 2023-12-05  108  {
64f91a53613aea Michael Trimarchi 2023-12-05  109  	struct mipi_dsi_device *dsi = rpanel->dsi;
64f91a53613aea Michael Trimarchi 2023-12-05  110  	struct device *dev = &dsi->dev;
64f91a53613aea Michael Trimarchi 2023-12-05  111  	int i, ret;
64f91a53613aea Michael Trimarchi 2023-12-05  112  
64f91a53613aea Michael Trimarchi 2023-12-05  113  	ret = mipi_dsi_dcs_soft_reset(dsi);
64f91a53613aea Michael Trimarchi 2023-12-05  114  	if (ret < 0) {
64f91a53613aea Michael Trimarchi 2023-12-05  115  		dev_err(dev, "Failed to do Software Reset (%d)\n", ret);
64f91a53613aea Michael Trimarchi 2023-12-05  116  		goto fail;
64f91a53613aea Michael Trimarchi 2023-12-05  117  	}
64f91a53613aea Michael Trimarchi 2023-12-05  118  
64f91a53613aea Michael Trimarchi 2023-12-05  119  	usleep_range(15000, 17000);
64f91a53613aea Michael Trimarchi 2023-12-05  120  
64f91a53613aea Michael Trimarchi 2023-12-05  121  	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
64f91a53613aea Michael Trimarchi 2023-12-05  122  	if (ret < 0) {
64f91a53613aea Michael Trimarchi 2023-12-05  123  		dev_err(dev, "Failed to enter sleep mode (%d)\n", ret);
64f91a53613aea Michael Trimarchi 2023-12-05  124  		goto fail;
64f91a53613aea Michael Trimarchi 2023-12-05  125  	}
64f91a53613aea Michael Trimarchi 2023-12-05  126  
64f91a53613aea Michael Trimarchi 2023-12-05  127  	for (i = 0; i < rpanel->pdata->init_length; i++) {
64f91a53613aea Michael Trimarchi 2023-12-05  128  		const struct r63353_instr *instr = &rpanel->pdata->init[i];
64f91a53613aea Michael Trimarchi 2023-12-05  129  
64f91a53613aea Michael Trimarchi 2023-12-05  130  		ret = mipi_dsi_dcs_write_buffer(dsi, instr->data, instr->len);
64f91a53613aea Michael Trimarchi 2023-12-05  131  		if (ret < 0)
64f91a53613aea Michael Trimarchi 2023-12-05 @132  			return ret;

goto fail?

64f91a53613aea Michael Trimarchi 2023-12-05  133  	}
64f91a53613aea Michael Trimarchi 2023-12-05  134  
64f91a53613aea Michael Trimarchi 2023-12-05  135  	msleep(120);
64f91a53613aea Michael Trimarchi 2023-12-05  136  
64f91a53613aea Michael Trimarchi 2023-12-05  137  	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
64f91a53613aea Michael Trimarchi 2023-12-05  138  	if (ret < 0) {
64f91a53613aea Michael Trimarchi 2023-12-05  139  		dev_err(dev, "Failed to exit sleep mode (%d)\n", ret);
64f91a53613aea Michael Trimarchi 2023-12-05  140  		goto fail;
64f91a53613aea Michael Trimarchi 2023-12-05  141  	}
64f91a53613aea Michael Trimarchi 2023-12-05  142  
64f91a53613aea Michael Trimarchi 2023-12-05  143  	usleep_range(5000, 10000);
64f91a53613aea Michael Trimarchi 2023-12-05  144  
64f91a53613aea Michael Trimarchi 2023-12-05  145  	ret = mipi_dsi_dcs_set_display_on(dsi);
64f91a53613aea Michael Trimarchi 2023-12-05  146  	if (ret < 0) {
64f91a53613aea Michael Trimarchi 2023-12-05  147  		dev_err(dev, "Failed to set display ON (%d)\n", ret);
64f91a53613aea Michael Trimarchi 2023-12-05  148  		goto fail;
64f91a53613aea Michael Trimarchi 2023-12-05  149  	}
64f91a53613aea Michael Trimarchi 2023-12-05  150  
64f91a53613aea Michael Trimarchi 2023-12-05  151  	return 0;
64f91a53613aea Michael Trimarchi 2023-12-05  152  
64f91a53613aea Michael Trimarchi 2023-12-05  153  fail:
64f91a53613aea Michael Trimarchi 2023-12-05  154  	gpiod_set_value(rpanel->reset_gpio, 0);
64f91a53613aea Michael Trimarchi 2023-12-05  155  
64f91a53613aea Michael Trimarchi 2023-12-05  156  	return ret;
64f91a53613aea Michael Trimarchi 2023-12-05  157  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller
  2023-12-06 14:33   ` Rob Herring
@ 2023-12-07  9:29     ` Krzysztof Kozlowski
  2023-12-07  9:41       ` Maxime Ripard
  0 siblings, 1 reply; 21+ messages in thread
From: Krzysztof Kozlowski @ 2023-12-07  9:29 UTC (permalink / raw)
  To: Rob Herring, Dario Binacchi
  Cc: Neil Armstrong, Conor Dooley, Krzysztof Kozlowski, Sam Ravnborg,
	devicetree, Amarula patchwork, dri-devel, linux-kernel,
	Maxime Ripard, Thomas Zimmermann, Jessica Zhang, michael

On 06/12/2023 15:33, Rob Herring wrote:
> On Tue, Dec 05, 2023 at 11:52:54AM +0100, Dario Binacchi wrote:
>> From: Michael Trimarchi <michael@amarulasolutions.com>
>>
>> Add documentation for "ilitek,ili9805" panel.
>>
>> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
>> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> 
> Where's Krzysztof's Reviewed-by?

If only there was a tool which would solve problem of collecting tags...
Easy to use one command for applying tags from mailing list, so one's
review will not be lost thus people will not waste effort doing review
second time. How was it? a4? b4? c4?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller
  2023-12-07  9:29     ` Krzysztof Kozlowski
@ 2023-12-07  9:41       ` Maxime Ripard
  0 siblings, 0 replies; 21+ messages in thread
From: Maxime Ripard @ 2023-12-07  9:41 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Conor Dooley, Dario Binacchi, Krzysztof Kozlowski, Sam Ravnborg,
	Neil Armstrong, linux-kernel, dri-devel, devicetree,
	Thomas Zimmermann, Jessica Zhang, michael, Amarula patchwork

[-- Attachment #1: Type: text/plain, Size: 881 bytes --]

On Thu, Dec 07, 2023 at 10:29:12AM +0100, Krzysztof Kozlowski wrote:
> On 06/12/2023 15:33, Rob Herring wrote:
> > On Tue, Dec 05, 2023 at 11:52:54AM +0100, Dario Binacchi wrote:
> >> From: Michael Trimarchi <michael@amarulasolutions.com>
> >>
> >> Add documentation for "ilitek,ili9805" panel.
> >>
> >> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> >> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > 
> > Where's Krzysztof's Reviewed-by?
> 
> If only there was a tool which would solve problem of collecting tags...
> Easy to use one command for applying tags from mailing list, so one's
> review will not be lost thus people will not waste effort doing review
> second time. How was it? a4? b4? c4?

FWIW, a lot of people still don't know about b4, or a relying on
features/workflows not supported by b4.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic
  2023-12-06 13:57         ` Michael Nazzareno Trimarchi
@ 2023-12-13 11:59           ` Dario Binacchi
  2023-12-13 12:02             ` Jagan Teki
  2023-12-13 12:17             ` Maxime Ripard
  0 siblings, 2 replies; 21+ messages in thread
From: Dario Binacchi @ 2023-12-13 11:59 UTC (permalink / raw)
  To: Jagan Teki, Dave Stevenson
  Cc: Maxime Ripard, Neil Armstrong, Robert Foss, Andrzej Hajda,
	Thomas Zimmermann, Jonas Karlman, Amarula patchwork, dri-devel,
	linux-kernel, Frieder Schrempf, Jernej Skrabec,
	Michael Nazzareno Trimarchi, Laurent Pinchart

Hi Jagan and Dave,

On Wed, Dec 6, 2023 at 2:57 PM Michael Nazzareno Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Hi Jagan
>
> On Wed, Dec 6, 2023 at 2:31 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Hi Dario,
> >
> > On Wed, Dec 6, 2023 at 6:57 PM Dario Binacchi
> > <dario.binacchi@amarulasolutions.com> wrote:
> > >
> > > Hi Dave and Jagan,
> > >
> > > On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson
> > > <dave.stevenson@raspberrypi.com> wrote:
> > > >
> > > > Hi Dario
> > > >
> > > > On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
> > > > <dario.binacchi@amarulasolutions.com> wrote:
> > > > >
> > > > > The patch fixes the code for finding the next bridge with the
> > > > > "pre_enable_prev_first" flag set to false. In case this condition is
> > > > > not verified, i. e. there is no subsequent bridge with the flag set to
> > > > > false, the whole bridge list is traversed, invalidating the "next"
> > > > > variable.
> > > > >
> > > > > The use of a new iteration variable (i. e. "iter") ensures that the value
> > > > > of the "next" variable is not invalidated.
> > > >
> > > > We already have https://patchwork.freedesktop.org/patch/529288/ that
> > > > has been reviewed (but not applied) to resolve this. What does this
> > > > version do differently and why?
> > >
> > > My patches only affect drm_atomic_bridge_chain_post_disable(), whereas
> > > Jagan's patch affects both
> > > drm_atomic_bridge_chain_post_disable() and drm_atomic_bridge_chain_pre_enable().
> > > I tested Jagan's patch on my system with success and I reviewed with
> > > Michael Trimarchi the
> > > drm_atomic_bridge_chain_pre_enable() fixing and we think it's okay.
> > > We also believe that our changes to post_disable() are better, as we
> > > set the 'next' variable only when required,
> > > and the code is more optimized since the list_is_last() is not called
> > > within the loop.
> > > Would it be possible to use Jagan's patch for fixing
> > > drm_atomic_bridge_chain_pre_enable() and mine for
> > > fixing drm_atomic_bridge_chain_post_disable()?
> > >
> >
> > Can you please share the post-disabled bridge chain list with the
> > below example before and after your change?
>
> We have already git commit the description in how the patch affects
> the post_disable. As Dario
> reported your patch is ok even in our use case. We don't have a real
> use case as the one you describe.
>
> Can we know how you test it in this use case here? Can you test our
> patches of post_disable?
>
> Thanks
> Michael
>
> >
> > Example:
> > - Panel
> > - Bridge 1
> > - Bridge 2 pre_enable_prev_first
> > - Bridge 3
> > - Bridge 4 pre_enable_prev_first
> > - Bridge 5 pre_enable_prev_first
> > - Bridge 6
> > - Encoder
> >
> > Thanks,
> > Jagan.

Starting from my use case:

# cat /sys/kernel/debug/dri/32e00000.lcdif/bridge_chains
encoder[36]
bridge[0] type: 16, ops: 0x0, OF:
/soc@0/bus@32c00000/dsi@32e10000:fsl,imx8mn-mipi-dsim
bridge[1] type: 16, ops: 0x8, OF:
/soc@0/bus@32c00000/dsi@32e10000/panel@0:sharp,ls068b3sx0

I developed a pass through MIPI-DSI bridge driver to try to test your case:
# cat /sys/kernel/debug/dri/32e00000.lcdif/bridge_chains
encoder[36]
bridge[0] type: 16, ops: 0x0, OF:
/soc@0/bus@32c00000/dsi@32e10000:fsl,imx8mn-mipi-dsim
bridge[1] type: 16, ops: 0x0, OF: /pt_mipi_dsi1:amarula,pt-mipi-dsi
bridge[2] type: 16, ops: 0x0, OF: /pt_mipi_dsi2:amarula,pt-mipi-dsi
bridge[3] type: 16, ops: 0x0, OF: /pt_mipi_dsi3:amarula,pt-mipi-dsi
bridge[4] type: 16, ops: 0x0, OF: /pt_mipi_dsi4:amarula,pt-mipi-dsi
bridge[5] type: 16, ops: 0x0, OF: /pt_mipi_dsi5:amarula,pt-mipi-dsi
bridge[6] type: 16, ops: 0x8, OF: /pt_mipi_dsi5/panel@0:sharp,ls068b3sx02

The pre_enable_prev_first flag is set through the
"amarula,pre_enable_prev_first" dts property I put
in my dts.
Your and my patches give the same results (result: OK) in both your
use case and mine.
But If I test my new "enlarged" use case:

- Encoder
- bridge[0] (samsung-dsim)
- bridge[1] pre_enable_prev_first
- bridge[2] pre_enable_prev_first
- bridge[3] pre_enable_prev_first
- bridge[4] pre_enable_prev_first
- bridge[5] pre_enable_prev_first
- bridge[6] pre_enable_prev_first (Panel)

the result is:
my patches: KO
your patch: OK

So, I will remove my patches from the series.

Can the driver I implemented to test the use cases (pass through
MIPI-DSI) be considered useful for testing these
bridge pipelines?
Does it make sense to send its patch?

Thanks and regards
Dario

Dario Binacchi

Senior Embedded Linux Developer

dario.binacchi@amarulasolutions.com

__________________________________


Amarula Solutions SRL

Via Le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 042 243 5310
info@amarulasolutions.com

www.amarulasolutions.com

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic
  2023-12-13 11:59           ` Dario Binacchi
@ 2023-12-13 12:02             ` Jagan Teki
  2023-12-13 12:17             ` Maxime Ripard
  1 sibling, 0 replies; 21+ messages in thread
From: Jagan Teki @ 2023-12-13 12:02 UTC (permalink / raw)
  To: Dario Binacchi
  Cc: Maxime Ripard, Neil Armstrong, Robert Foss, Andrzej Hajda,
	Thomas Zimmermann, Dave Stevenson, dri-devel, Jonas Karlman,
	linux-kernel, Frieder Schrempf, Jernej Skrabec,
	Michael Nazzareno Trimarchi, Amarula patchwork, Laurent Pinchart

On Wed, Dec 13, 2023 at 5:29 PM Dario Binacchi
<dario.binacchi@amarulasolutions.com> wrote:
>
> Hi Jagan and Dave,
>
> On Wed, Dec 6, 2023 at 2:57 PM Michael Nazzareno Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > Hi Jagan
> >
> > On Wed, Dec 6, 2023 at 2:31 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > Hi Dario,
> > >
> > > On Wed, Dec 6, 2023 at 6:57 PM Dario Binacchi
> > > <dario.binacchi@amarulasolutions.com> wrote:
> > > >
> > > > Hi Dave and Jagan,
> > > >
> > > > On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson
> > > > <dave.stevenson@raspberrypi.com> wrote:
> > > > >
> > > > > Hi Dario
> > > > >
> > > > > On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
> > > > > <dario.binacchi@amarulasolutions.com> wrote:
> > > > > >
> > > > > > The patch fixes the code for finding the next bridge with the
> > > > > > "pre_enable_prev_first" flag set to false. In case this condition is
> > > > > > not verified, i. e. there is no subsequent bridge with the flag set to
> > > > > > false, the whole bridge list is traversed, invalidating the "next"
> > > > > > variable.
> > > > > >
> > > > > > The use of a new iteration variable (i. e. "iter") ensures that the value
> > > > > > of the "next" variable is not invalidated.
> > > > >
> > > > > We already have https://patchwork.freedesktop.org/patch/529288/ that
> > > > > has been reviewed (but not applied) to resolve this. What does this
> > > > > version do differently and why?
> > > >
> > > > My patches only affect drm_atomic_bridge_chain_post_disable(), whereas
> > > > Jagan's patch affects both
> > > > drm_atomic_bridge_chain_post_disable() and drm_atomic_bridge_chain_pre_enable().
> > > > I tested Jagan's patch on my system with success and I reviewed with
> > > > Michael Trimarchi the
> > > > drm_atomic_bridge_chain_pre_enable() fixing and we think it's okay.
> > > > We also believe that our changes to post_disable() are better, as we
> > > > set the 'next' variable only when required,
> > > > and the code is more optimized since the list_is_last() is not called
> > > > within the loop.
> > > > Would it be possible to use Jagan's patch for fixing
> > > > drm_atomic_bridge_chain_pre_enable() and mine for
> > > > fixing drm_atomic_bridge_chain_post_disable()?
> > > >
> > >
> > > Can you please share the post-disabled bridge chain list with the
> > > below example before and after your change?
> >
> > We have already git commit the description in how the patch affects
> > the post_disable. As Dario
> > reported your patch is ok even in our use case. We don't have a real
> > use case as the one you describe.
> >
> > Can we know how you test it in this use case here? Can you test our
> > patches of post_disable?
> >
> > Thanks
> > Michael
> >
> > >
> > > Example:
> > > - Panel
> > > - Bridge 1
> > > - Bridge 2 pre_enable_prev_first
> > > - Bridge 3
> > > - Bridge 4 pre_enable_prev_first
> > > - Bridge 5 pre_enable_prev_first
> > > - Bridge 6
> > > - Encoder
> > >
> > > Thanks,
> > > Jagan.
>
> Starting from my use case:
>
> # cat /sys/kernel/debug/dri/32e00000.lcdif/bridge_chains
> encoder[36]
> bridge[0] type: 16, ops: 0x0, OF:
> /soc@0/bus@32c00000/dsi@32e10000:fsl,imx8mn-mipi-dsim
> bridge[1] type: 16, ops: 0x8, OF:
> /soc@0/bus@32c00000/dsi@32e10000/panel@0:sharp,ls068b3sx0
>
> I developed a pass through MIPI-DSI bridge driver to try to test your case:
> # cat /sys/kernel/debug/dri/32e00000.lcdif/bridge_chains
> encoder[36]
> bridge[0] type: 16, ops: 0x0, OF:
> /soc@0/bus@32c00000/dsi@32e10000:fsl,imx8mn-mipi-dsim
> bridge[1] type: 16, ops: 0x0, OF: /pt_mipi_dsi1:amarula,pt-mipi-dsi
> bridge[2] type: 16, ops: 0x0, OF: /pt_mipi_dsi2:amarula,pt-mipi-dsi
> bridge[3] type: 16, ops: 0x0, OF: /pt_mipi_dsi3:amarula,pt-mipi-dsi
> bridge[4] type: 16, ops: 0x0, OF: /pt_mipi_dsi4:amarula,pt-mipi-dsi
> bridge[5] type: 16, ops: 0x0, OF: /pt_mipi_dsi5:amarula,pt-mipi-dsi
> bridge[6] type: 16, ops: 0x8, OF: /pt_mipi_dsi5/panel@0:sharp,ls068b3sx02
>
> The pre_enable_prev_first flag is set through the
> "amarula,pre_enable_prev_first" dts property I put
> in my dts.
> Your and my patches give the same results (result: OK) in both your
> use case and mine.
> But If I test my new "enlarged" use case:
>
> - Encoder
> - bridge[0] (samsung-dsim)
> - bridge[1] pre_enable_prev_first
> - bridge[2] pre_enable_prev_first
> - bridge[3] pre_enable_prev_first
> - bridge[4] pre_enable_prev_first
> - bridge[5] pre_enable_prev_first
> - bridge[6] pre_enable_prev_first (Panel)
>
> the result is:
> my patches: KO
> your patch: OK
>
> So, I will remove my patches from the series.
>
> Can the driver I implemented to test the use cases (pass through
> MIPI-DSI) be considered useful for testing these
> bridge pipelines?
> Does it make sense to send its patch?

I don't think so, I have a similar test bench for chain of bridges. I
will try to re-create the chain and update the result.

Jagan.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic
  2023-12-13 11:59           ` Dario Binacchi
  2023-12-13 12:02             ` Jagan Teki
@ 2023-12-13 12:17             ` Maxime Ripard
  1 sibling, 0 replies; 21+ messages in thread
From: Maxime Ripard @ 2023-12-13 12:17 UTC (permalink / raw)
  To: Dario Binacchi
  Cc: Neil Armstrong, Jernej Skrabec, Robert Foss, Thomas Zimmermann,
	Dave Stevenson, dri-devel, Jonas Karlman, linux-kernel,
	Frieder Schrempf, Jagan Teki, Andrzej Hajda,
	Michael Nazzareno Trimarchi, Amarula patchwork, Laurent Pinchart

[-- Attachment #1: Type: text/plain, Size: 4958 bytes --]

On Wed, Dec 13, 2023 at 12:59:05PM +0100, Dario Binacchi wrote:
> Hi Jagan and Dave,
> 
> On Wed, Dec 6, 2023 at 2:57 PM Michael Nazzareno Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > Hi Jagan
> >
> > On Wed, Dec 6, 2023 at 2:31 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > Hi Dario,
> > >
> > > On Wed, Dec 6, 2023 at 6:57 PM Dario Binacchi
> > > <dario.binacchi@amarulasolutions.com> wrote:
> > > >
> > > > Hi Dave and Jagan,
> > > >
> > > > On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson
> > > > <dave.stevenson@raspberrypi.com> wrote:
> > > > >
> > > > > Hi Dario
> > > > >
> > > > > On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
> > > > > <dario.binacchi@amarulasolutions.com> wrote:
> > > > > >
> > > > > > The patch fixes the code for finding the next bridge with the
> > > > > > "pre_enable_prev_first" flag set to false. In case this condition is
> > > > > > not verified, i. e. there is no subsequent bridge with the flag set to
> > > > > > false, the whole bridge list is traversed, invalidating the "next"
> > > > > > variable.
> > > > > >
> > > > > > The use of a new iteration variable (i. e. "iter") ensures that the value
> > > > > > of the "next" variable is not invalidated.
> > > > >
> > > > > We already have https://patchwork.freedesktop.org/patch/529288/ that
> > > > > has been reviewed (but not applied) to resolve this. What does this
> > > > > version do differently and why?
> > > >
> > > > My patches only affect drm_atomic_bridge_chain_post_disable(), whereas
> > > > Jagan's patch affects both
> > > > drm_atomic_bridge_chain_post_disable() and drm_atomic_bridge_chain_pre_enable().
> > > > I tested Jagan's patch on my system with success and I reviewed with
> > > > Michael Trimarchi the
> > > > drm_atomic_bridge_chain_pre_enable() fixing and we think it's okay.
> > > > We also believe that our changes to post_disable() are better, as we
> > > > set the 'next' variable only when required,
> > > > and the code is more optimized since the list_is_last() is not called
> > > > within the loop.
> > > > Would it be possible to use Jagan's patch for fixing
> > > > drm_atomic_bridge_chain_pre_enable() and mine for
> > > > fixing drm_atomic_bridge_chain_post_disable()?
> > > >
> > >
> > > Can you please share the post-disabled bridge chain list with the
> > > below example before and after your change?
> >
> > We have already git commit the description in how the patch affects
> > the post_disable. As Dario
> > reported your patch is ok even in our use case. We don't have a real
> > use case as the one you describe.
> >
> > Can we know how you test it in this use case here? Can you test our
> > patches of post_disable?
> >
> > Thanks
> > Michael
> >
> > >
> > > Example:
> > > - Panel
> > > - Bridge 1
> > > - Bridge 2 pre_enable_prev_first
> > > - Bridge 3
> > > - Bridge 4 pre_enable_prev_first
> > > - Bridge 5 pre_enable_prev_first
> > > - Bridge 6
> > > - Encoder
> > >
> > > Thanks,
> > > Jagan.
> 
> Starting from my use case:
> 
> # cat /sys/kernel/debug/dri/32e00000.lcdif/bridge_chains
> encoder[36]
> bridge[0] type: 16, ops: 0x0, OF:
> /soc@0/bus@32c00000/dsi@32e10000:fsl,imx8mn-mipi-dsim
> bridge[1] type: 16, ops: 0x8, OF:
> /soc@0/bus@32c00000/dsi@32e10000/panel@0:sharp,ls068b3sx0
> 
> I developed a pass through MIPI-DSI bridge driver to try to test your case:
> # cat /sys/kernel/debug/dri/32e00000.lcdif/bridge_chains
> encoder[36]
> bridge[0] type: 16, ops: 0x0, OF:
> /soc@0/bus@32c00000/dsi@32e10000:fsl,imx8mn-mipi-dsim
> bridge[1] type: 16, ops: 0x0, OF: /pt_mipi_dsi1:amarula,pt-mipi-dsi
> bridge[2] type: 16, ops: 0x0, OF: /pt_mipi_dsi2:amarula,pt-mipi-dsi
> bridge[3] type: 16, ops: 0x0, OF: /pt_mipi_dsi3:amarula,pt-mipi-dsi
> bridge[4] type: 16, ops: 0x0, OF: /pt_mipi_dsi4:amarula,pt-mipi-dsi
> bridge[5] type: 16, ops: 0x0, OF: /pt_mipi_dsi5:amarula,pt-mipi-dsi
> bridge[6] type: 16, ops: 0x8, OF: /pt_mipi_dsi5/panel@0:sharp,ls068b3sx02
> 
> The pre_enable_prev_first flag is set through the
> "amarula,pre_enable_prev_first" dts property I put
> in my dts.
> Your and my patches give the same results (result: OK) in both your
> use case and mine.
> But If I test my new "enlarged" use case:
> 
> - Encoder
> - bridge[0] (samsung-dsim)
> - bridge[1] pre_enable_prev_first
> - bridge[2] pre_enable_prev_first
> - bridge[3] pre_enable_prev_first
> - bridge[4] pre_enable_prev_first
> - bridge[5] pre_enable_prev_first
> - bridge[6] pre_enable_prev_first (Panel)
> 
> the result is:
> my patches: KO
> your patch: OK
> 
> So, I will remove my patches from the series.
> 
> Can the driver I implemented to test the use cases (pass through
> MIPI-DSI) be considered useful for testing these
> bridge pipelines?
> Does it make sense to send its patch?

As it is, not really, but kunit tests would be very welcome

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2023-12-13 12:18 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-05 10:52 [PATCH v4 00/10] Add displays support for bsh-smm-s2/pro boards Dario Binacchi
2023-12-05 10:52 ` [PATCH v4 01/10] drm/bridge: Fix bridge disable logic Dario Binacchi
2023-12-05 10:52 ` [PATCH v4 02/10] drm/bridge: Fix a use case in the " Dario Binacchi
2023-12-05 15:39   ` Dave Stevenson
2023-12-06 13:26     ` Dario Binacchi
2023-12-06 13:31       ` Jagan Teki
2023-12-06 13:57         ` Michael Nazzareno Trimarchi
2023-12-13 11:59           ` Dario Binacchi
2023-12-13 12:02             ` Jagan Teki
2023-12-13 12:17             ` Maxime Ripard
2023-12-05 10:52 ` [PATCH v4 03/10] drm: bridge: samsung-dsim: enter display mode in the enable() callback Dario Binacchi
2023-12-05 10:52 ` [PATCH v4 04/10] drm: bridge: samsung-dsim: complete the CLKLANE_STOP setting Dario Binacchi
2023-12-05 10:52 ` [PATCH v4 05/10] dt-bindings: display: panel: Add synaptics r63353 panel controller Dario Binacchi
2023-12-05 10:52 ` [PATCH v4 06/10] drm/panel: Add Synaptics R63353 panel driver Dario Binacchi
2023-12-07  5:26   ` Dan Carpenter
2023-12-05 10:52 ` [PATCH v4 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller Dario Binacchi
2023-12-06 14:33   ` Rob Herring
2023-12-07  9:29     ` Krzysztof Kozlowski
2023-12-07  9:41       ` Maxime Ripard
2023-12-05 10:52 ` [PATCH v4 08/10] drm/panel: Add Ilitek ILI9805 panel driver Dario Binacchi
2023-12-05 10:52 ` [PATCH v4 09/10] drm/panel: ilitek-ili9805: add support for Tianma TM041XDHG01 panel Dario Binacchi

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).