2023-12-05 10:54:28

by Dario Binacchi

[permalink] [raw]
Subject: [PATCH v4 00/10] Add displays support for bsh-smm-s2/pro boards

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


2023-12-05 10:54:30

by Dario Binacchi

[permalink] [raw]
Subject: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic

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 <[email protected]>
Signed-off-by: Michael Trimarchi <[email protected]>
Signed-off-by: Dario Binacchi <[email protected]>
---

(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

2023-12-05 10:54:35

by Dario Binacchi

[permalink] [raw]
Subject: [PATCH v4 01/10] drm/bridge: Fix bridge disable logic

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 <[email protected]>
---

(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

2023-12-05 10:54:42

by Dario Binacchi

[permalink] [raw]
Subject: [PATCH v4 08/10] drm/panel: Add Ilitek ILI9805 panel driver

From: Michael Trimarchi <[email protected]>

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

Signed-off-by: Michael Trimarchi <[email protected]>
Signed-off-by: Dario Binacchi <[email protected]>

---

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 <[email protected]>
+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 <[email protected]>
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 <[email protected]>");
+MODULE_AUTHOR("Michael Trimarchi <[email protected]>");
+MODULE_DESCRIPTION("Ilitek ILI9805 Controller Driver");
+MODULE_LICENSE("GPL");
--
2.43.0

2023-12-05 10:54:43

by Dario Binacchi

[permalink] [raw]
Subject: [PATCH v4 09/10] drm/panel: ilitek-ili9805: add support for Tianma TM041XDHG01 panel

From: Michael Trimarchi <[email protected]>

Tianma TM041XDHG01 utilizes the Ilitek ILI9805 controller.

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

Signed-off-by: Michael Trimarchi <[email protected]>
Reviewed-by: Neil Armstrong <[email protected]>

Signed-off-by: Dario Binacchi <[email protected]>
---

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

2023-12-05 10:54:45

by Dario Binacchi

[permalink] [raw]
Subject: [PATCH v4 04/10] drm: bridge: samsung-dsim: complete the CLKLANE_STOP setting

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

Co-developed-by: Michael Trimarchi <[email protected]>
Signed-off-by: Michael Trimarchi <[email protected]>
Signed-off-by: Dario Binacchi <[email protected]>
---

(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

2023-12-05 10:54:50

by Dario Binacchi

[permalink] [raw]
Subject: [PATCH v4 10/10] arm64: dts: imx8mn-bsh-smm-s2/pro: add display setup

From: Michael Trimarchi <[email protected]>

Add the display and nodes required for its operation.

Signed-off-by: Michael Trimarchi <[email protected]>
Signed-off-by: Dario Binacchi <[email protected]>

---

(no changes since v3)

Changes in v3:
- 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:
- 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

.../freescale/imx8mn-bsh-smm-s2-common.dtsi | 1 +
.../freescale/imx8mn-bsh-smm-s2-display.dtsi | 121 ++++++++++++++++++
2 files changed, 122 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2-display.dtsi

diff --git a/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2-common.dtsi b/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2-common.dtsi
index 22a754d438f1..bbb07c650da9 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2-common.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2-common.dtsi
@@ -7,6 +7,7 @@
/dts-v1/;

#include "imx8mn.dtsi"
+#include "imx8mn-bsh-smm-s2-display.dtsi"

/ {
chosen {
diff --git a/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2-display.dtsi b/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2-display.dtsi
new file mode 100644
index 000000000000..f0a924cbe548
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2-display.dtsi
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2021 BSH
+ */
+
+/ {
+ backlight: backlight {
+ compatible = "pwm-backlight";
+ pwms = <&pwm1 0 700000 0>; /* 700000 ns = 1337Hz */
+ brightness-levels = <0 100>;
+ num-interpolated-steps = <100>;
+ default-brightness-level = <50>;
+ status = "okay";
+ };
+
+ reg_3v3_dvdd: regulator-3v3-O3 {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_dvdd>;
+ regulator-name = "3v3-dvdd-supply";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio1 7 GPIO_ACTIVE_LOW>;
+ };
+
+ reg_v3v3_avdd: regulator-3v3-O2 {
+ compatible = "regulator-fixed";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_avdd>;
+ regulator-name = "3v3-avdd-supply";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio1 5 GPIO_ACTIVE_LOW>;
+ };
+};
+
+&pwm1 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_bl>;
+};
+
+&lcdif {
+ status = "okay";
+ assigned-clocks = <&clk IMX8MN_VIDEO_PLL1>;
+ assigned-clock-rates = <594000000>;
+};
+
+&pgc_dispmix {
+ assigned-clocks = <&clk IMX8MN_CLK_DISP_AXI>, <&clk IMX8MN_CLK_DISP_APB>;
+ assigned-clock-parents = <&clk IMX8MN_SYS_PLL2_1000M>, <&clk IMX8MN_SYS_PLL1_800M>;
+ assigned-clock-rates = <500000000>, <200000000>;
+};
+
+&mipi_dsi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "okay";
+ samsung,esc-clock-frequency = <20000000>;
+ samsung,pll-clock-frequency = <12000000>;
+
+ panel@0 {
+ compatible = "sharp,ls068b3sx02", "syna,r63353";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_panel>;
+ reg = <0>;
+
+ backlight = <&backlight>;
+ dvdd-supply = <&reg_3v3_dvdd>;
+ avdd-supply = <&reg_v3v3_avdd>;
+ reset-gpios = <&gpio4 29 GPIO_ACTIVE_HIGH>;
+
+ port {
+ panel_in: endpoint {
+ remote-endpoint = <&mipi_dsi_out>;
+ };
+ };
+
+ };
+
+ ports {
+ port@1 {
+ reg = <1>;
+ mipi_dsi_out: endpoint {
+ remote-endpoint = <&panel_in>;
+ };
+ };
+ };
+};
+
+&gpu {
+ status = "okay";
+};
+
+&iomuxc {
+
+ /* This is for both PWM and voltage regulators for display */
+ pinctrl_bl: pwm1grp {
+ fsl,pins = <
+ MX8MN_IOMUXC_GPIO1_IO01_PWM1_OUT 0x16
+ >;
+ };
+
+ pinctrl_panel: panelgrp {
+ fsl,pins = <
+ MX8MN_IOMUXC_SAI3_RXC_GPIO4_IO29 0x16 /* panel reset */
+ >;
+ };
+
+ pinctrl_dvdd: dvddgrp {
+ fsl,pins = <
+ MX8MN_IOMUXC_GPIO1_IO07_GPIO1_IO7 0x16 /* VDD 3V3_VO3 */
+ >;
+ };
+
+ pinctrl_avdd: avddgrp {
+ fsl,pins = <
+ MX8MN_IOMUXC_GPIO1_IO05_GPIO1_IO5 0x16 /* VDD 3V3_VO2 */
+ >;
+ };
+};
--
2.43.0

2023-12-05 10:54:50

by Dario Binacchi

[permalink] [raw]
Subject: [PATCH v4 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller

From: Michael Trimarchi <[email protected]>

Add documentation for "ilitek,ili9805" panel.

Signed-off-by: Michael Trimarchi <[email protected]>
Signed-off-by: Dario Binacchi <[email protected]>

---

(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 <[email protected]>
+
+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

2023-12-05 10:54:59

by Dario Binacchi

[permalink] [raw]
Subject: [PATCH v4 05/10] dt-bindings: display: panel: Add synaptics r63353 panel controller

From: Michael Trimarchi <[email protected]>

Add documentation for "synaptics,r63353" panel.

Signed-off-by: Michael Trimarchi <[email protected]>
Signed-off-by: Dario Binacchi <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>

---

(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 <[email protected]>
+
+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

2023-12-05 10:55:05

by Dario Binacchi

[permalink] [raw]
Subject: [PATCH v4 06/10] drm/panel: Add Synaptics R63353 panel driver

From: Michael Trimarchi <[email protected]>

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

Signed-off-by: Michael Trimarchi <[email protected]>
Signed-off-by: Dario Binacchi <[email protected]>

---

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 <[email protected]>
+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 <[email protected]>
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 <[email protected]>");
+MODULE_AUTHOR("Michael Trimarchi <[email protected]>");
+MODULE_DESCRIPTION("Synaptics R63353 Controller Driver");
+MODULE_LICENSE("GPL");
--
2.43.0

2023-12-05 10:55:59

by Dario Binacchi

[permalink] [raw]
Subject: [PATCH v4 03/10] drm: bridge: samsung-dsim: enter display mode in the enable() callback

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 <[email protected]>
Signed-off-by: Michael Trimarchi <[email protected]>
Signed-off-by: Dario Binacchi <[email protected]>
---

(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

2023-12-05 15:39:38

by Dave Stevenson

[permalink] [raw]
Subject: Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic

Hi Dario

On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
<[email protected]> 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 <[email protected]>
> Signed-off-by: Michael Trimarchi <[email protected]>
> Signed-off-by: Dario Binacchi <[email protected]>
> ---
>
> (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
>

2023-12-06 13:27:23

by Dario Binacchi

[permalink] [raw]
Subject: Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic

Hi Dave and Jagan,

On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson
<[email protected]> wrote:
>
> Hi Dario
>
> On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
> <[email protected]> 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 <[email protected]>
> > Signed-off-by: Michael Trimarchi <[email protected]>
> > Signed-off-by: Dario Binacchi <[email protected]>
> > ---
> >
> > (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

[email protected]

__________________________________


Amarula Solutions SRL

Via Le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 042 243 5310
[email protected]

http://www.amarulasolutions.com

2023-12-06 13:31:29

by Jagan Teki

[permalink] [raw]
Subject: Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic

Hi Dario,

On Wed, Dec 6, 2023 at 6:57 PM Dario Binacchi
<[email protected]> wrote:
>
> Hi Dave and Jagan,
>
> On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson
> <[email protected]> wrote:
> >
> > Hi Dario
> >
> > On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
> > <[email protected]> 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.

Subject: Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic

Hi Jagan

On Wed, Dec 6, 2023 at 2:31 PM Jagan Teki <[email protected]> wrote:
>
> Hi Dario,
>
> On Wed, Dec 6, 2023 at 6:57 PM Dario Binacchi
> <[email protected]> wrote:
> >
> > Hi Dave and Jagan,
> >
> > On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson
> > <[email protected]> wrote:
> > >
> > > Hi Dario
> > >
> > > On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
> > > <[email protected]> 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.

2023-12-06 14:34:00

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller

On Tue, Dec 05, 2023 at 11:52:54AM +0100, Dario Binacchi wrote:
> From: Michael Trimarchi <[email protected]>
>
> Add documentation for "ilitek,ili9805" panel.
>
> Signed-off-by: Michael Trimarchi <[email protected]>
> Signed-off-by: Dario Binacchi <[email protected]>

Where's Krzysztof's Reviewed-by?

Rob

2023-12-07 05:26:38

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v4 06/10] drm/panel: Add Synaptics R63353 panel driver

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/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231207/[email protected]/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 <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

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

2023-12-07 09:30:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller

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 <[email protected]>
>>
>> Add documentation for "ilitek,ili9805" panel.
>>
>> Signed-off-by: Michael Trimarchi <[email protected]>
>> Signed-off-by: Dario Binacchi <[email protected]>
>
> 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

2023-12-07 09:41:41

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v4 07/10] dt-bindings: display: panel: Add Ilitek ili9805 panel controller

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 <[email protected]>
> >>
> >> Add documentation for "ilitek,ili9805" panel.
> >>
> >> Signed-off-by: Michael Trimarchi <[email protected]>
> >> Signed-off-by: Dario Binacchi <[email protected]>
> >
> > 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


Attachments:
(No filename) (881.00 B)
signature.asc (235.00 B)
Download all attachments

2023-12-13 11:59:24

by Dario Binacchi

[permalink] [raw]
Subject: Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic

Hi Jagan and Dave,

On Wed, Dec 6, 2023 at 2:57 PM Michael Nazzareno Trimarchi
<[email protected]> wrote:
>
> Hi Jagan
>
> On Wed, Dec 6, 2023 at 2:31 PM Jagan Teki <[email protected]> wrote:
> >
> > Hi Dario,
> >
> > On Wed, Dec 6, 2023 at 6:57 PM Dario Binacchi
> > <[email protected]> wrote:
> > >
> > > Hi Dave and Jagan,
> > >
> > > On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson
> > > <[email protected]> wrote:
> > > >
> > > > Hi Dario
> > > >
> > > > On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
> > > > <[email protected]> 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

[email protected]

__________________________________


Amarula Solutions SRL

Via Le Canevare 30, 31100 Treviso, Veneto, IT

T. +39 042 243 5310
[email protected]

http://www.amarulasolutions.com

2023-12-13 12:02:42

by Jagan Teki

[permalink] [raw]
Subject: Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic

On Wed, Dec 13, 2023 at 5:29 PM Dario Binacchi
<[email protected]> wrote:
>
> Hi Jagan and Dave,
>
> On Wed, Dec 6, 2023 at 2:57 PM Michael Nazzareno Trimarchi
> <[email protected]> wrote:
> >
> > Hi Jagan
> >
> > On Wed, Dec 6, 2023 at 2:31 PM Jagan Teki <[email protected]> wrote:
> > >
> > > Hi Dario,
> > >
> > > On Wed, Dec 6, 2023 at 6:57 PM Dario Binacchi
> > > <[email protected]> wrote:
> > > >
> > > > Hi Dave and Jagan,
> > > >
> > > > On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson
> > > > <[email protected]> wrote:
> > > > >
> > > > > Hi Dario
> > > > >
> > > > > On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
> > > > > <[email protected]> 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.

2023-12-13 12:18:12

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v4 02/10] drm/bridge: Fix a use case in the bridge disable logic

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
> <[email protected]> wrote:
> >
> > Hi Jagan
> >
> > On Wed, Dec 6, 2023 at 2:31 PM Jagan Teki <[email protected]> wrote:
> > >
> > > Hi Dario,
> > >
> > > On Wed, Dec 6, 2023 at 6:57 PM Dario Binacchi
> > > <[email protected]> wrote:
> > > >
> > > > Hi Dave and Jagan,
> > > >
> > > > On Tue, Dec 5, 2023 at 4:39 PM Dave Stevenson
> > > > <[email protected]> wrote:
> > > > >
> > > > > Hi Dario
> > > > >
> > > > > On Tue, 5 Dec 2023 at 10:54, Dario Binacchi
> > > > > <[email protected]> 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


Attachments:
(No filename) (4.84 kB)
signature.asc (235.00 B)
Download all attachments