[PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[Jagan Teki]

Updated series about drm bridge conversion of exynos dsi.

Previous version can be accessible, here [1].

Patch 1: tc358764 panel_bridge API

Patch 2: connector reset

Patch 3: bridge attach in MIC

Patch 4: panel_bridge API

Patch 5: bridge conversion

Patch 6: atomic functions

[1] https://patchwork.amarulasolutions.com/cover/1839

Any inputs?
Jagan.

Jagan Teki (6):
  drm: bridge: tc358764: Use drm panel_bridge API
  drm: bridge: panel: Reset the connector state pointer
  exynos: drm: dsi: Attach in_bridge in MIC driver
  drm: exynos: dsi: Use drm panel_bridge API
  drm: exynos: dsi: Convert to bridge driver
  drm: exynos: dsi: Switch to atomic funcs

 drivers/gpu/drm/bridge/panel.c          |   3 +
 drivers/gpu/drm/bridge/tc358764.c       | 104 +---------
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 241 ++++++------------------
 drivers/gpu/drm/exynos/exynos_drm_mic.c |  22 +++
 4 files changed, 93 insertions(+), 277 deletions(-)

-- 
2.25.1

[PATCH v6 1/6] drm: bridge: tc358764: Use drm panel_bridge API

[Jagan Teki]

Replace the manual panel handling code by a drm panel_bridge via
devm_drm_of_get_bridge().

Adding panel_bridge handling,

- Drops drm_connector and related operations as drm_bridge_attach
  creates connector during attachment.

- Drops panel pointer and panel healpers.

This simplifies the driver and allows all components in the display
pipeline to be treated as bridges.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v6:
- none
Changes for v2:
- s/panel_bridge/next_bridge
- drop unneeded headers

 drivers/gpu/drm/bridge/tc358764.c | 104 ++----------------------------
 1 file changed, 6 insertions(+), 98 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c
index c1e35bdf9232..dca41ed32f8a 100644
--- a/drivers/gpu/drm/bridge/tc358764.c
+++ b/drivers/gpu/drm/bridge/tc358764.c
@@ -16,14 +16,9 @@
 #include <video/mipi_display.h>
 
 #include <drm/drm_atomic_helper.h>
-#include <drm/drm_bridge.h>
-#include <drm/drm_crtc.h>
-#include <drm/drm_fb_helper.h>
 #include <drm/drm_mipi_dsi.h>
 #include <drm/drm_of.h>
-#include <drm/drm_panel.h>
 #include <drm/drm_print.h>
-#include <drm/drm_probe_helper.h>
 
 #define FLD_MASK(start, end)    (((1 << ((start) - (end) + 1)) - 1) << (end))
 #define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
@@ -153,10 +148,9 @@ static const char * const tc358764_supplies[] = {
 struct tc358764 {
 	struct device *dev;
 	struct drm_bridge bridge;
-	struct drm_connector connector;
+	struct drm_bridge *next_bridge;
 	struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)];
 	struct gpio_desc *gpio_reset;
-	struct drm_panel *panel;
 	int error;
 };
 
@@ -210,12 +204,6 @@ static inline struct tc358764 *bridge_to_tc358764(struct drm_bridge *bridge)
 	return container_of(bridge, struct tc358764, bridge);
 }
 
-static inline
-struct tc358764 *connector_to_tc358764(struct drm_connector *connector)
-{
-	return container_of(connector, struct tc358764, connector);
-}
-
 static int tc358764_init(struct tc358764 *ctx)
 {
 	u32 v = 0;
@@ -278,43 +266,11 @@ static void tc358764_reset(struct tc358764 *ctx)
 	usleep_range(1000, 2000);
 }
 
-static int tc358764_get_modes(struct drm_connector *connector)
-{
-	struct tc358764 *ctx = connector_to_tc358764(connector);
-
-	return drm_panel_get_modes(ctx->panel, connector);
-}
-
-static const
-struct drm_connector_helper_funcs tc358764_connector_helper_funcs = {
-	.get_modes = tc358764_get_modes,
-};
-
-static const struct drm_connector_funcs tc358764_connector_funcs = {
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = drm_connector_cleanup,
-	.reset = drm_atomic_helper_connector_reset,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-static void tc358764_disable(struct drm_bridge *bridge)
-{
-	struct tc358764 *ctx = bridge_to_tc358764(bridge);
-	int ret = drm_panel_disable(bridge_to_tc358764(bridge)->panel);
-
-	if (ret < 0)
-		dev_err(ctx->dev, "error disabling panel (%d)\n", ret);
-}
-
 static void tc358764_post_disable(struct drm_bridge *bridge)
 {
 	struct tc358764 *ctx = bridge_to_tc358764(bridge);
 	int ret;
 
-	ret = drm_panel_unprepare(ctx->panel);
-	if (ret < 0)
-		dev_err(ctx->dev, "error unpreparing panel (%d)\n", ret);
 	tc358764_reset(ctx);
 	usleep_range(10000, 15000);
 	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
@@ -335,72 +291,25 @@ static void tc358764_pre_enable(struct drm_bridge *bridge)
 	ret = tc358764_init(ctx);
 	if (ret < 0)
 		dev_err(ctx->dev, "error initializing bridge (%d)\n", ret);
-	ret = drm_panel_prepare(ctx->panel);
-	if (ret < 0)
-		dev_err(ctx->dev, "error preparing panel (%d)\n", ret);
-}
-
-static void tc358764_enable(struct drm_bridge *bridge)
-{
-	struct tc358764 *ctx = bridge_to_tc358764(bridge);
-	int ret = drm_panel_enable(ctx->panel);
-
-	if (ret < 0)
-		dev_err(ctx->dev, "error enabling panel (%d)\n", ret);
 }
 
 static int tc358764_attach(struct drm_bridge *bridge,
 			   enum drm_bridge_attach_flags flags)
-{
-	struct tc358764 *ctx = bridge_to_tc358764(bridge);
-	struct drm_device *drm = bridge->dev;
-	int ret;
-
-	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
-		DRM_ERROR("Fix bridge driver to make connector optional!");
-		return -EINVAL;
-	}
-
-	ctx->connector.polled = DRM_CONNECTOR_POLL_HPD;
-	ret = drm_connector_init(drm, &ctx->connector,
-				 &tc358764_connector_funcs,
-				 DRM_MODE_CONNECTOR_LVDS);
-	if (ret) {
-		DRM_ERROR("Failed to initialize connector\n");
-		return ret;
-	}
-
-	drm_connector_helper_add(&ctx->connector,
-				 &tc358764_connector_helper_funcs);
-	drm_connector_attach_encoder(&ctx->connector, bridge->encoder);
-	ctx->connector.funcs->reset(&ctx->connector);
-	drm_connector_register(&ctx->connector);
-
-	return 0;
-}
-
-static void tc358764_detach(struct drm_bridge *bridge)
 {
 	struct tc358764 *ctx = bridge_to_tc358764(bridge);
 
-	drm_connector_unregister(&ctx->connector);
-	ctx->panel = NULL;
-	drm_connector_put(&ctx->connector);
+	return drm_bridge_attach(bridge->encoder, ctx->next_bridge, bridge, flags);
 }
 
 static const struct drm_bridge_funcs tc358764_bridge_funcs = {
-	.disable = tc358764_disable,
 	.post_disable = tc358764_post_disable,
-	.enable = tc358764_enable,
 	.pre_enable = tc358764_pre_enable,
 	.attach = tc358764_attach,
-	.detach = tc358764_detach,
 };
 
 static int tc358764_parse_dt(struct tc358764 *ctx)
 {
 	struct device *dev = ctx->dev;
-	int ret;
 
 	ctx->gpio_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
 	if (IS_ERR(ctx->gpio_reset)) {
@@ -408,12 +317,11 @@ static int tc358764_parse_dt(struct tc358764 *ctx)
 		return PTR_ERR(ctx->gpio_reset);
 	}
 
-	ret = drm_of_find_panel_or_bridge(ctx->dev->of_node, 1, 0, &ctx->panel,
-					  NULL);
-	if (ret && ret != -EPROBE_DEFER)
-		dev_err(dev, "cannot find panel (%d)\n", ret);
+	ctx->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+	if (IS_ERR(ctx->next_bridge))
+		return PTR_ERR(ctx->next_bridge);
 
-	return ret;
+	return 0;
 }
 
 static int tc358764_configure_regulators(struct tc358764 *ctx)
-- 
2.25.1

[PATCH v6 2/6] drm: bridge: panel: Reset the connector state pointer

[Jagan Teki]

Trigger hotplug event with drm_kms_helper_hotplug_event might fail if the
connector state pointer is NULL.

BUG observed in exynos dsi driver where drm_bridge_attach is trying to register
a connector in panel_bridge before the hotplug event is triggered,

WARNING: CPU: 1 PID: 1 at drivers/gpu/drm/drm_atomic_state_helper.c:494 drm_atomic_helper_connector_duplicate_state+0x94/0x9c
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W 5.16.0-rc1-00009-g704b1dbfa4c2 #11058
Hardware name: Samsung Exynos (Flattened Device Tree)
[<c0110b30>] (unwind_backtrace) from [<c010c618>] (show_stack+0x10/0x14)
[<c010c618>] (show_stack) from [<c0b657d4>] (dump_stack_lvl+0x58/0x70)
[<c0b657d4>] (dump_stack_lvl) from [<c01261dc>] (__warn+0xd0/0x134)
[<c01261dc>] (__warn) from [<c0b5f628>] (warn_slowpath_fmt+0x5c/0xb4)
[<c0b5f628>] (warn_slowpath_fmt) from [<c064bce4>] (drm_atomic_helper_connector_duplicate_state+0x94/0x9c)
[<c064bce4>] (drm_atomic_helper_connector_duplicate_state) from [<c0666b64>] (drm_atomic_get_connector_state+0xd4/0x190)
[<c0666b64>] (drm_atomic_get_connector_state) from [<c0667928>] (__drm_atomic_helper_set_config+0x314/0x368)
[<c0667928>] (__drm_atomic_helper_set_config) from [<c067e628>] (drm_client_modeset_commit_atomic+0x170/0x278)
[<c067e628>] (drm_client_modeset_commit_atomic) from [<c067e800>] (drm_client_modeset_commit_locked+0x60/0x1c8)
[<c067e800>] (drm_client_modeset_commit_locked) from [<c067e98c>] (drm_client_modeset_commit+0x24/0x40)
[<c067e98c>] (drm_client_modeset_commit) from [<c06509c0>] (drm_fb_helper_set_par+0xb8/0xf8)
[<c06509c0>] (drm_fb_helper_set_par) from [<c05b86d0>] (fbcon_init+0x2c0/0x518)
[<c05b86d0>] (fbcon_init) from [<c060636c>] (visual_init+0xc0/0x108)
[<c060636c>] (visual_init) from [<c06085e4>] (do_bind_con_driver+0x1b8/0x3a4)
[<c06085e4>] (do_bind_con_driver) from [<c0608b40>] (do_take_over_console+0x13c/0x1e8)
[<c0608b40>] (do_take_over_console) from [<c05b6854>] (do_fbcon_takeover+0x78/0xd8)
[<c05b6854>] (do_fbcon_takeover) from [<c05b1154>] (register_framebuffer+0x208/0x2e0)
[<c05b1154>] (register_framebuffer) from [<c064ead0>] (__drm_fb_helper_initial_config_and_unlock+0x400/0x63c)
[<c064ead0>] (__drm_fb_helper_initial_config_and_unlock) from [<c063a718>] (drm_kms_helper_hotplug_event+0x24/0x30)
[<c063a718>] (drm_kms_helper_hotplug_event) from [<c068f668>] (exynos_dsi_host_attach+0x174/0x1fc)
[<c068f668>] (exynos_dsi_host_attach) from [<c0699354>] (s6e8aa0_probe+0x1b4/0x218)

So reset the atomic state for a given connector by freeing the state pointer
and allocate a new empty state object. This can be done using connector
funcs->reset helper and has to be done before the hotplug even calls.

This patch calls the connector->funcs->reset in panel_bridge_attach.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v6, v5:
- none
Changes for v4:
- new patch

 drivers/gpu/drm/bridge/panel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index b32295abd9e7..f6eea194482a 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -83,6 +83,9 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
 	drm_connector_attach_encoder(&panel_bridge->connector,
 					  bridge->encoder);
 
+	if (connector->funcs->reset)
+		connector->funcs->reset(connector);
+
 	return 0;
 }
 
-- 
2.25.1

[PATCH v6 3/6] exynos: drm: dsi: Attach in_bridge in MIC driver

[Jagan Teki]

MIC drivers in the Exynos5433 display pipeline are already registered
as bridge drivers and it is more advisable to attach the downstream
bridge on the bridge attach call instead of doing the same in the
DSI driver.

This makes bridge attachment more meaningful and avoids the races
during bridge function calls.

So, move the bridge finding and drm_bridge_attach from DSI to MIC.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v6:
- new patch

 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 15 ---------------
 drivers/gpu/drm/exynos/exynos_drm_mic.c | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index b7d0a4aead0a..741c046513e8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1660,11 +1660,6 @@ static int exynos_dsi_of_read_u32(const struct device_node *np,
 	return ret;
 }
 
-enum {
-	DSI_PORT_IN,
-	DSI_PORT_OUT
-};
-
 static int exynos_dsi_parse_dt(struct exynos_dsi *dsi)
 {
 	struct device *dev = dsi->dev;
@@ -1695,8 +1690,6 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
 	struct exynos_dsi *dsi = dev_get_drvdata(dev);
 	struct drm_encoder *encoder = &dsi->encoder;
 	struct drm_device *drm_dev = data;
-	struct device_node *in_bridge_node;
-	struct drm_bridge *in_bridge;
 	int ret;
 
 	drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_TMDS);
@@ -1707,14 +1700,6 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
 	if (ret < 0)
 		return ret;
 
-	in_bridge_node = of_graph_get_remote_node(dev->of_node, DSI_PORT_IN, 0);
-	if (in_bridge_node) {
-		in_bridge = of_drm_find_bridge(in_bridge_node);
-		if (in_bridge)
-			drm_bridge_attach(encoder, in_bridge, NULL, 0);
-		of_node_put(in_bridge_node);
-	}
-
 	return mipi_dsi_host_register(&dsi->dsi_host);
 }
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index 32672bf8ae4a..9e06f8e2a863 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -102,6 +102,7 @@ struct exynos_mic {
 	struct videomode vm;
 	struct drm_encoder *encoder;
 	struct drm_bridge bridge;
+	struct drm_bridge *next_bridge;
 
 	bool enabled;
 };
@@ -298,12 +299,22 @@ static void mic_pre_enable(struct drm_bridge *bridge)
 
 static void mic_enable(struct drm_bridge *bridge) { }
 
+static int mic_attach(struct drm_bridge *bridge,
+		      enum drm_bridge_attach_flags flags)
+{
+	struct exynos_mic *mic = bridge->driver_private;
+
+	return drm_bridge_attach(bridge->encoder, mic->next_bridge,
+				 &mic->bridge, flags);
+}
+
 static const struct drm_bridge_funcs mic_bridge_funcs = {
 	.disable = mic_disable,
 	.post_disable = mic_post_disable,
 	.mode_set = mic_mode_set,
 	.pre_enable = mic_pre_enable,
 	.enable = mic_enable,
+	.attach = mic_attach,
 };
 
 static int exynos_mic_bind(struct device *dev, struct device *master,
@@ -377,6 +388,7 @@ static int exynos_mic_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct exynos_mic *mic;
+	struct device_node *remote;
 	struct resource res;
 	int ret, i;
 
@@ -420,6 +432,16 @@ static int exynos_mic_probe(struct platform_device *pdev)
 		}
 	}
 
+	remote = of_graph_get_remote_node(dev->of_node, 1, 0);
+	mic->next_bridge = of_drm_find_bridge(remote);
+	if (IS_ERR(mic->next_bridge)) {
+		DRM_DEV_ERROR(dev, "mic: Failed to find next bridge\n");
+		ret = PTR_ERR(mic->next_bridge);
+		goto err;
+	}
+
+	of_node_put(remote);
+
 	platform_set_drvdata(pdev, mic);
 
 	mic->bridge.funcs = &mic_bridge_funcs;
-- 
2.25.1

[PATCH v6 4/6] drm: exynos: dsi: Use drm panel_bridge API

[Jagan Teki]

Replace the manual panel handling code by a drm panel_bridge via
devm_drm_of_get_bridge().

Adding panel_bridge handling,

- Drops drm_connector and related operations as drm_bridge_attach
  creates connector during attachment.

- Drops panel pointer and iterate the bridge, so-that it can operate
  the normal bridge and panel_bridge in constitutive callbacks.

This simplifies the driver and allows all components in the display
pipeline to be treated as bridges.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v6, v5:
- none
Changes for v4:
- drop unneeded headers
Changes for v3:
- fix port number
- add print for attached device
Changes for v2:
- new patch

 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 157 ++++--------------------
 1 file changed, 23 insertions(+), 134 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 741c046513e8..953094133ed8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -24,9 +24,7 @@
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
-#include <drm/drm_fb_helper.h>
 #include <drm/drm_mipi_dsi.h>
-#include <drm/drm_panel.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_simple_kms_helper.h>
@@ -253,8 +251,6 @@ struct exynos_dsi_driver_data {
 struct exynos_dsi {
 	struct drm_encoder encoder;
 	struct mipi_dsi_host dsi_host;
-	struct drm_connector connector;
-	struct drm_panel *panel;
 	struct list_head bridge_chain;
 	struct drm_bridge *out_bridge;
 	struct device *dev;
@@ -285,7 +281,6 @@ struct exynos_dsi {
 };
 
 #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
-#define connector_to_dsi(c) container_of(c, struct exynos_dsi, connector)
 
 static inline struct exynos_dsi *encoder_to_dsi(struct drm_encoder *e)
 {
@@ -1380,42 +1375,21 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
 
 	dsi->state |= DSIM_STATE_ENABLED;
 
-	if (dsi->panel) {
-		ret = drm_panel_prepare(dsi->panel);
-		if (ret < 0)
-			goto err_put_sync;
-	} else {
-		list_for_each_entry_reverse(iter, &dsi->bridge_chain,
-					    chain_node) {
-			if (iter->funcs->pre_enable)
-				iter->funcs->pre_enable(iter);
-		}
+	list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) {
+		if (iter->funcs->pre_enable)
+			iter->funcs->pre_enable(iter);
 	}
 
 	exynos_dsi_set_display_mode(dsi);
 	exynos_dsi_set_display_enable(dsi, true);
 
-	if (dsi->panel) {
-		ret = drm_panel_enable(dsi->panel);
-		if (ret < 0)
-			goto err_display_disable;
-	} else {
-		list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
-			if (iter->funcs->enable)
-				iter->funcs->enable(iter);
-		}
+	list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
+		if (iter->funcs->enable)
+			iter->funcs->enable(iter);
 	}
 
 	dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
 	return;
-
-err_display_disable:
-	exynos_dsi_set_display_enable(dsi, false);
-	drm_panel_unprepare(dsi->panel);
-
-err_put_sync:
-	dsi->state &= ~DSIM_STATE_ENABLED;
-	pm_runtime_put(dsi->dev);
 }
 
 static void exynos_dsi_disable(struct drm_encoder *encoder)
@@ -1428,15 +1402,12 @@ static void exynos_dsi_disable(struct drm_encoder *encoder)
 
 	dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
 
-	drm_panel_disable(dsi->panel);
-
 	list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) {
 		if (iter->funcs->disable)
 			iter->funcs->disable(iter);
 	}
 
 	exynos_dsi_set_display_enable(dsi, false);
-	drm_panel_unprepare(dsi->panel);
 
 	list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
 		if (iter->funcs->post_disable)
@@ -1456,70 +1427,6 @@ static void exynos_dsi_mode_set(struct drm_encoder *encoder,
 	drm_mode_copy(&dsi->mode, adjusted_mode);
 }
 
-static enum drm_connector_status
-exynos_dsi_detect(struct drm_connector *connector, bool force)
-{
-	return connector->status;
-}
-
-static void exynos_dsi_connector_destroy(struct drm_connector *connector)
-{
-	drm_connector_unregister(connector);
-	drm_connector_cleanup(connector);
-	connector->dev = NULL;
-}
-
-static const struct drm_connector_funcs exynos_dsi_connector_funcs = {
-	.detect = exynos_dsi_detect,
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.destroy = exynos_dsi_connector_destroy,
-	.reset = drm_atomic_helper_connector_reset,
-	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-
-static int exynos_dsi_get_modes(struct drm_connector *connector)
-{
-	struct exynos_dsi *dsi = connector_to_dsi(connector);
-
-	if (dsi->panel)
-		return drm_panel_get_modes(dsi->panel, connector);
-
-	return 0;
-}
-
-static const struct drm_connector_helper_funcs exynos_dsi_connector_helper_funcs = {
-	.get_modes = exynos_dsi_get_modes,
-};
-
-static int exynos_dsi_create_connector(struct drm_encoder *encoder)
-{
-	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
-	struct drm_connector *connector = &dsi->connector;
-	struct drm_device *drm = encoder->dev;
-	int ret;
-
-	connector->polled = DRM_CONNECTOR_POLL_HPD;
-
-	ret = drm_connector_init(drm, connector, &exynos_dsi_connector_funcs,
-				 DRM_MODE_CONNECTOR_DSI);
-	if (ret) {
-		DRM_DEV_ERROR(dsi->dev,
-			      "Failed to initialize connector with drm\n");
-		return ret;
-	}
-
-	connector->status = connector_status_disconnected;
-	drm_connector_helper_add(connector, &exynos_dsi_connector_helper_funcs);
-	drm_connector_attach_encoder(connector, encoder);
-	if (!drm->registered)
-		return 0;
-
-	connector->funcs->reset(connector);
-	drm_connector_register(connector);
-	return 0;
-}
-
 static const struct drm_encoder_helper_funcs exynos_dsi_encoder_helper_funcs = {
 	.enable = exynos_dsi_enable,
 	.disable = exynos_dsi_disable,
@@ -1532,33 +1439,23 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 				  struct mipi_dsi_device *device)
 {
 	struct exynos_dsi *dsi = host_to_dsi(host);
+	struct device *dev = dsi->dev;
 	struct drm_encoder *encoder = &dsi->encoder;
 	struct drm_device *drm = encoder->dev;
-	struct drm_bridge *out_bridge;
-
-	out_bridge  = of_drm_find_bridge(device->dev.of_node);
-	if (out_bridge) {
-		drm_bridge_attach(encoder, out_bridge, NULL, 0);
-		dsi->out_bridge = out_bridge;
-		list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain);
-	} else {
-		int ret = exynos_dsi_create_connector(encoder);
-
-		if (ret) {
-			DRM_DEV_ERROR(dsi->dev,
-				      "failed to create connector ret = %d\n",
-				      ret);
-			drm_encoder_cleanup(encoder);
-			return ret;
-		}
+	int ret;
 
-		dsi->panel = of_drm_find_panel(device->dev.of_node);
-		if (IS_ERR(dsi->panel))
-			dsi->panel = NULL;
-		else
-			dsi->connector.status = connector_status_connected;
+	dsi->out_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
+	if (IS_ERR(dsi->out_bridge)) {
+		ret = PTR_ERR(dsi->out_bridge);
+		DRM_DEV_ERROR(dev, "failed to find the bridge: %d\n", ret);
+		return ret;
 	}
 
+	DRM_DEV_INFO(dev, "Attached %s device\n", device->name);
+
+	drm_bridge_attach(encoder, dsi->out_bridge, NULL, 0);
+	list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain);
+
 	/*
 	 * This is a temporary solution and should be made by more generic way.
 	 *
@@ -1566,7 +1463,7 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 	 * TE interrupt handler.
 	 */
 	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO)) {
-		int ret = exynos_dsi_register_te_irq(dsi, &device->dev);
+		ret = exynos_dsi_register_te_irq(dsi, &device->dev);
 		if (ret)
 			return ret;
 	}
@@ -1593,18 +1490,10 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
 	struct exynos_dsi *dsi = host_to_dsi(host);
 	struct drm_device *drm = dsi->encoder.dev;
 
-	if (dsi->panel) {
-		mutex_lock(&drm->mode_config.mutex);
-		exynos_dsi_disable(&dsi->encoder);
-		dsi->panel = NULL;
-		dsi->connector.status = connector_status_disconnected;
-		mutex_unlock(&drm->mode_config.mutex);
-	} else {
-		if (dsi->out_bridge->funcs->detach)
-			dsi->out_bridge->funcs->detach(dsi->out_bridge);
-		dsi->out_bridge = NULL;
-		INIT_LIST_HEAD(&dsi->bridge_chain);
-	}
+	if (dsi->out_bridge->funcs->detach)
+		dsi->out_bridge->funcs->detach(dsi->out_bridge);
+	dsi->out_bridge = NULL;
+	INIT_LIST_HEAD(&dsi->bridge_chain);
 
 	if (drm->mode_config.poll_enabled)
 		drm_kms_helper_hotplug_event(drm);
-- 
2.25.1

[PATCH v6 5/6] drm: exynos: dsi: Convert to bridge driver

[Jagan Teki]

Convert the encoders to bridge drivers in order to standardize on
a single API with built-in dumb encoder support for compatibility
with existing component drivers.

Driver bridge conversion will help to reuse the same bridge on
different platforms as exynos dsi driver can be used as a Samsung
DSIM and use it for i.MX8MM platform.

Bridge conversion,

- Drops drm_encoder_helper_funcs.

- Adds drm_bridge_funcs and register a drm bridge.

- Drops bridge_chain.

- Separate pre_enable from enable function.

- Separate post_disable from disable function.

Convert it.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v6, v5:
- none
Changes for v4:
- add pre_enable function
- add post_disable function
Changes for v3:
- move bridge add in host_attach
- move bridge remove in host_detach
- use flags, bridge in drm_bridge_attach in attch 
Changes for v2:
- drop bridge_chain

 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 88 +++++++++++++------------
 1 file changed, 45 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 953094133ed8..59a4f7f52180 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -251,7 +251,7 @@ struct exynos_dsi_driver_data {
 struct exynos_dsi {
 	struct drm_encoder encoder;
 	struct mipi_dsi_host dsi_host;
-	struct list_head bridge_chain;
+	struct drm_bridge bridge;
 	struct drm_bridge *out_bridge;
 	struct device *dev;
 	struct drm_display_mode mode;
@@ -282,9 +282,9 @@ struct exynos_dsi {
 
 #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
 
-static inline struct exynos_dsi *encoder_to_dsi(struct drm_encoder *e)
+static inline struct exynos_dsi *bridge_to_dsi(struct drm_bridge *b)
 {
-	return container_of(e, struct exynos_dsi, encoder);
+	return container_of(b, struct exynos_dsi, bridge);
 }
 
 enum reg_idx {
@@ -1358,10 +1358,9 @@ static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi)
 	}
 }
 
-static void exynos_dsi_enable(struct drm_encoder *encoder)
+static void exynos_dsi_pre_enable(struct drm_bridge *bridge)
 {
-	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
-	struct drm_bridge *iter;
+	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
 	int ret;
 
 	if (dsi->state & DSIM_STATE_ENABLED)
@@ -1374,63 +1373,64 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
 	}
 
 	dsi->state |= DSIM_STATE_ENABLED;
+}
 
-	list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) {
-		if (iter->funcs->pre_enable)
-			iter->funcs->pre_enable(iter);
-	}
+static void exynos_dsi_enable(struct drm_bridge *bridge)
+{
+	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
 
 	exynos_dsi_set_display_mode(dsi);
 	exynos_dsi_set_display_enable(dsi, true);
 
-	list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
-		if (iter->funcs->enable)
-			iter->funcs->enable(iter);
-	}
-
 	dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
+
 	return;
 }
 
-static void exynos_dsi_disable(struct drm_encoder *encoder)
+static void exynos_dsi_disable(struct drm_bridge *bridge)
 {
-	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
-	struct drm_bridge *iter;
+	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
 
 	if (!(dsi->state & DSIM_STATE_ENABLED))
 		return;
 
 	dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
+}
 
-	list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) {
-		if (iter->funcs->disable)
-			iter->funcs->disable(iter);
-	}
+static void exynos_dsi_post_disable(struct drm_bridge *bridge)
+{
+	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
 
 	exynos_dsi_set_display_enable(dsi, false);
 
-	list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
-		if (iter->funcs->post_disable)
-			iter->funcs->post_disable(iter);
-	}
-
 	dsi->state &= ~DSIM_STATE_ENABLED;
 	pm_runtime_put_sync(dsi->dev);
 }
 
-static void exynos_dsi_mode_set(struct drm_encoder *encoder,
-				struct drm_display_mode *mode,
-				struct drm_display_mode *adjusted_mode)
+static void exynos_dsi_mode_set(struct drm_bridge *bridge,
+				const struct drm_display_mode *mode,
+				const struct drm_display_mode *adjusted_mode)
 {
-	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
+	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
 
 	drm_mode_copy(&dsi->mode, adjusted_mode);
 }
 
-static const struct drm_encoder_helper_funcs exynos_dsi_encoder_helper_funcs = {
-	.enable = exynos_dsi_enable,
-	.disable = exynos_dsi_disable,
-	.mode_set = exynos_dsi_mode_set,
+static int exynos_dsi_attach(struct drm_bridge *bridge,
+			     enum drm_bridge_attach_flags flags)
+{
+	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
+
+	return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags);
+}
+
+static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {
+	.pre_enable			= exynos_dsi_pre_enable,
+	.enable				= exynos_dsi_enable,
+	.disable			= exynos_dsi_disable,
+	.post_disable			= exynos_dsi_post_disable,
+	.mode_set			= exynos_dsi_mode_set,
+	.attach				= exynos_dsi_attach,
 };
 
 MODULE_DEVICE_TABLE(of, exynos_dsi_of_match);
@@ -1453,8 +1453,9 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 
 	DRM_DEV_INFO(dev, "Attached %s device\n", device->name);
 
-	drm_bridge_attach(encoder, dsi->out_bridge, NULL, 0);
-	list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain);
+	drm_bridge_add(&dsi->bridge);
+
+	drm_bridge_attach(encoder, &dsi->bridge, NULL, 0);
 
 	/*
 	 * This is a temporary solution and should be made by more generic way.
@@ -1493,13 +1494,14 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
 	if (dsi->out_bridge->funcs->detach)
 		dsi->out_bridge->funcs->detach(dsi->out_bridge);
 	dsi->out_bridge = NULL;
-	INIT_LIST_HEAD(&dsi->bridge_chain);
 
 	if (drm->mode_config.poll_enabled)
 		drm_kms_helper_hotplug_event(drm);
 
 	exynos_dsi_unregister_te_irq(dsi);
 
+	drm_bridge_remove(&dsi->bridge);
+
 	return 0;
 }
 
@@ -1583,8 +1585,6 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
 
 	drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_TMDS);
 
-	drm_encoder_helper_add(encoder, &exynos_dsi_encoder_helper_funcs);
-
 	ret = exynos_drm_set_possible_crtcs(encoder, EXYNOS_DISPLAY_TYPE_LCD);
 	if (ret < 0)
 		return ret;
@@ -1596,9 +1596,8 @@ static void exynos_dsi_unbind(struct device *dev, struct device *master,
 				void *data)
 {
 	struct exynos_dsi *dsi = dev_get_drvdata(dev);
-	struct drm_encoder *encoder = &dsi->encoder;
 
-	exynos_dsi_disable(encoder);
+	exynos_dsi_disable(&dsi->bridge);
 
 	mipi_dsi_host_unregister(&dsi->dsi_host);
 }
@@ -1621,7 +1620,6 @@ static int exynos_dsi_probe(struct platform_device *pdev)
 	init_completion(&dsi->completed);
 	spin_lock_init(&dsi->transfer_lock);
 	INIT_LIST_HEAD(&dsi->transfer_list);
-	INIT_LIST_HEAD(&dsi->bridge_chain);
 
 	dsi->dsi_host.ops = &exynos_dsi_ops;
 	dsi->dsi_host.dev = dev;
@@ -1689,6 +1687,10 @@ static int exynos_dsi_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(dev);
 
+	dsi->bridge.funcs = &exynos_dsi_bridge_funcs;
+	dsi->bridge.of_node = dev->of_node;
+	dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
+
 	ret = component_add(dev, &exynos_dsi_component_ops);
 	if (ret)
 		goto err_disable_runtime;
-- 
2.25.1

[PATCH v6 6/6] drm: exynos: dsi: Switch to atomic funcs

[Jagan Teki]

The new support drm bridges are moving towards atomic functions.

Replace atomic version of functions to continue the transition
to the atomic API.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v6, v5, v4, v3:
- none

 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 59a4f7f52180..06130eee8df8 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1358,7 +1358,8 @@ static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi)
 	}
 }
 
-static void exynos_dsi_pre_enable(struct drm_bridge *bridge)
+static void exynos_dsi_atomic_pre_enable(struct drm_bridge *bridge,
+					 struct drm_bridge_state *old_bridge_state)
 {
 	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
 	int ret;
@@ -1375,7 +1376,8 @@ static void exynos_dsi_pre_enable(struct drm_bridge *bridge)
 	dsi->state |= DSIM_STATE_ENABLED;
 }
 
-static void exynos_dsi_enable(struct drm_bridge *bridge)
+static void exynos_dsi_atomic_enable(struct drm_bridge *bridge,
+				     struct drm_bridge_state *old_bridge_state)
 {
 	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
 
@@ -1387,7 +1389,8 @@ static void exynos_dsi_enable(struct drm_bridge *bridge)
 	return;
 }
 
-static void exynos_dsi_disable(struct drm_bridge *bridge)
+static void exynos_dsi_atomic_disable(struct drm_bridge *bridge,
+				      struct drm_bridge_state *old_bridge_state)
 {
 	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
 
@@ -1397,7 +1400,8 @@ static void exynos_dsi_disable(struct drm_bridge *bridge)
 	dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
 }
 
-static void exynos_dsi_post_disable(struct drm_bridge *bridge)
+static void exynos_dsi_atomic_post_disable(struct drm_bridge *bridge,
+					   struct drm_bridge_state *old_bridge_state)
 {
 	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
 
@@ -1425,10 +1429,13 @@ static int exynos_dsi_attach(struct drm_bridge *bridge,
 }
 
 static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {
-	.pre_enable			= exynos_dsi_pre_enable,
-	.enable				= exynos_dsi_enable,
-	.disable			= exynos_dsi_disable,
-	.post_disable			= exynos_dsi_post_disable,
+	.atomic_duplicate_state		= drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
+	.atomic_reset			= drm_atomic_helper_bridge_reset,
+	.atomic_pre_enable		= exynos_dsi_atomic_pre_enable,
+	.atomic_enable			= exynos_dsi_atomic_enable,
+	.atomic_disable			= exynos_dsi_atomic_disable,
+	.atomic_post_disable		= exynos_dsi_atomic_post_disable,
 	.mode_set			= exynos_dsi_mode_set,
 	.attach				= exynos_dsi_attach,
 };
@@ -1597,7 +1604,7 @@ static void exynos_dsi_unbind(struct device *dev, struct device *master,
 {
 	struct exynos_dsi *dsi = dev_get_drvdata(dev);
 
-	exynos_dsi_disable(&dsi->bridge);
+	exynos_dsi_atomic_disable(&dsi->bridge, NULL);
 
 	mipi_dsi_host_unregister(&dsi->dsi_host);
 }
-- 
2.25.1

Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[Frieder Schrempf]

Hi Jagan,

Am 03.03.22 um 17:36 schrieb Jagan Teki:
> Updated series about drm bridge conversion of exynos dsi.
> 
> Previous version can be accessible, here [1].
> 
> Patch 1: tc358764 panel_bridge API
> 
> Patch 2: connector reset
> 
> Patch 3: bridge attach in MIC
> 
> Patch 4: panel_bridge API
> 
> Patch 5: bridge conversion
> 
> Patch 6: atomic functions
> 
> [1] https://patchwork.amarulasolutions.com/cover/1839
> 
> Any inputs?

Thanks for your efforts. I didn't follow the whole history, but I'm
looking forward and hope to see upstream support for the i.MX8MM DSIM in
the not too distant future.

Can you give me a short update about the state of this patchset? Are
there still any major obstacles?

I can't help with testing on Exynos, but if you have the matching
follow-up patches for i.MX8MM support somewhere around I could do some
tests with those on i.MX8MM.

Thanks
Frieder

Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[Jagan Teki]

Hi Frieder,

On Wed, Mar 9, 2022 at 6:54 PM Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:
>
> Hi Jagan,
>
> Am 03.03.22 um 17:36 schrieb Jagan Teki:
> > Updated series about drm bridge conversion of exynos dsi.
> >
> > Previous version can be accessible, here [1].
> >
> > Patch 1: tc358764 panel_bridge API
> >
> > Patch 2: connector reset
> >
> > Patch 3: bridge attach in MIC
> >
> > Patch 4: panel_bridge API
> >
> > Patch 5: bridge conversion
> >
> > Patch 6: atomic functions
> >
> > [1] https://patchwork.amarulasolutions.com/cover/1839
> >
> > Any inputs?
>
> Thanks for your efforts. I didn't follow the whole history, but I'm
> looking forward and hope to see upstream support for the i.MX8MM DSIM in
> the not too distant future.
>
> Can you give me a short update about the state of this patchset? Are
> there still any major obstacles?
>
> I can't help with testing on Exynos, but if you have the matching
> follow-up patches for i.MX8MM support somewhere around I could do some
> tests with those on i.MX8MM.

Unfortunately, it is getting slow due to existing exynos dsi drivers.
Idea is to push exynos and then move the bridge as per Mailing-list
discussion. I have initial series to support i.MX8MM on linux-next [1]
which is working on my setup. However I'm waiting for this series to
move further to send those on the mailing list. Indeed I'm solely
relaying on Marek testing to move further as I too don't have Exynos
hardware to validate.

[1] https://github.com/openedev/kernel/tree/imx8mm-dsi

Thanks,
Jagan.

Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[Frieder Schrempf]

Hi Jagan,

Am 09.03.22 um 15:01 schrieb Jagan Teki:
> Hi Frieder,
> 
> On Wed, Mar 9, 2022 at 6:54 PM Frieder Schrempf
> <frieder.schrempf@kontron.de> wrote:
>>
>> Hi Jagan,
>>
>> Am 03.03.22 um 17:36 schrieb Jagan Teki:
>>> Updated series about drm bridge conversion of exynos dsi.
>>>
>>> Previous version can be accessible, here [1].
>>>
>>> Patch 1: tc358764 panel_bridge API
>>>
>>> Patch 2: connector reset
>>>
>>> Patch 3: bridge attach in MIC
>>>
>>> Patch 4: panel_bridge API
>>>
>>> Patch 5: bridge conversion
>>>
>>> Patch 6: atomic functions
>>>
>>> [1] https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.amarulasolutions.com%2Fcover%2F1839&amp;data=04%7C01%7Cfrieder.schrempf%40kontron.de%7Cc99f637dd67444dfc38208da01d55963%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637824313083236643%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=qF5bVwelZ35cQQygW3PvPUZkQlyFalUDsyDVjDnngiU%3D&amp;reserved=0
>>>
>>> Any inputs?
>>
>> Thanks for your efforts. I didn't follow the whole history, but I'm
>> looking forward and hope to see upstream support for the i.MX8MM DSIM in
>> the not too distant future.
>>
>> Can you give me a short update about the state of this patchset? Are
>> there still any major obstacles?
>>
>> I can't help with testing on Exynos, but if you have the matching
>> follow-up patches for i.MX8MM support somewhere around I could do some
>> tests with those on i.MX8MM.
> 
> Unfortunately, it is getting slow due to existing exynos dsi drivers.
> Idea is to push exynos and then move the bridge as per Mailing-list
> discussion. I have initial series to support i.MX8MM on linux-next [1]
> which is working on my setup. However I'm waiting for this series to
> move further to send those on the mailing list. Indeed I'm solely
> relaying on Marek testing to move further as I too don't have Exynos
> hardware to validate.

Thanks for the status update. Let's hope Marek or others with access to
the hardware can provide further testing.

And thanks for providing the git tree for i.MX8MM. I will try to do some
tests on our hardware.

Thanks
Frieder

Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[Frieder Schrempf]

Am 10.03.22 um 14:03 schrieb Frieder Schrempf:
> Hi Jagan,
> 
> Am 09.03.22 um 15:01 schrieb Jagan Teki:
>> Hi Frieder,
>>
>> On Wed, Mar 9, 2022 at 6:54 PM Frieder Schrempf
>> <frieder.schrempf@kontron.de> wrote:
>>>
>>> Hi Jagan,
>>>
>>> Am 03.03.22 um 17:36 schrieb Jagan Teki:
>>>> Updated series about drm bridge conversion of exynos dsi.
>>>>
>>>> Previous version can be accessible, here [1].
>>>>
>>>> Patch 1: tc358764 panel_bridge API
>>>>
>>>> Patch 2: connector reset
>>>>
>>>> Patch 3: bridge attach in MIC
>>>>
>>>> Patch 4: panel_bridge API
>>>>
>>>> Patch 5: bridge conversion
>>>>
>>>> Patch 6: atomic functions
>>>>
>>>> [1] https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.amarulasolutions.com%2Fcover%2F1839&amp;data=04%7C01%7Cfrieder.schrempf%40kontron.de%7Cc99f637dd67444dfc38208da01d55963%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637824313083236643%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=qF5bVwelZ35cQQygW3PvPUZkQlyFalUDsyDVjDnngiU%3D&amp;reserved=0
>>>>
>>>> Any inputs?
>>>
>>> Thanks for your efforts. I didn't follow the whole history, but I'm
>>> looking forward and hope to see upstream support for the i.MX8MM DSIM in
>>> the not too distant future.
>>>
>>> Can you give me a short update about the state of this patchset? Are
>>> there still any major obstacles?
>>>
>>> I can't help with testing on Exynos, but if you have the matching
>>> follow-up patches for i.MX8MM support somewhere around I could do some
>>> tests with those on i.MX8MM.
>>
>> Unfortunately, it is getting slow due to existing exynos dsi drivers.
>> Idea is to push exynos and then move the bridge as per Mailing-list
>> discussion. I have initial series to support i.MX8MM on linux-next [1]
>> which is working on my setup. However I'm waiting for this series to
>> move further to send those on the mailing list. Indeed I'm solely
>> relaying on Marek testing to move further as I too don't have Exynos
>> hardware to validate.
> 
> Thanks for the status update. Let's hope Marek or others with access to
> the hardware can provide further testing.
> 
> And thanks for providing the git tree for i.MX8MM. I will try to do some
> tests on our hardware.

Sorry, forgot to say that if you could cc me on future iterations of
this patchset and the upcoming i.MX8MM patches, that would be great, thanks!

Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[Tim Harvey]

On Wed, Mar 9, 2022 at 6:01 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Hi Frieder,
>
> On Wed, Mar 9, 2022 at 6:54 PM Frieder Schrempf
> <frieder.schrempf@kontron.de> wrote:
> >
> > Hi Jagan,
> >
> > Am 03.03.22 um 17:36 schrieb Jagan Teki:
> > > Updated series about drm bridge conversion of exynos dsi.
> > >
> > > Previous version can be accessible, here [1].
> > >
> > > Patch 1: tc358764 panel_bridge API
> > >
> > > Patch 2: connector reset
> > >
> > > Patch 3: bridge attach in MIC
> > >
> > > Patch 4: panel_bridge API
> > >
> > > Patch 5: bridge conversion
> > >
> > > Patch 6: atomic functions
> > >
> > > [1] https://patchwork.amarulasolutions.com/cover/1839
> > >
> > > Any inputs?
> >
> > Thanks for your efforts. I didn't follow the whole history, but I'm
> > looking forward and hope to see upstream support for the i.MX8MM DSIM in
> > the not too distant future.
> >
> > Can you give me a short update about the state of this patchset? Are
> > there still any major obstacles?
> >
> > I can't help with testing on Exynos, but if you have the matching
> > follow-up patches for i.MX8MM support somewhere around I could do some
> > tests with those on i.MX8MM.
>
> Unfortunately, it is getting slow due to existing exynos dsi drivers.
> Idea is to push exynos and then move the bridge as per Mailing-list
> discussion. I have initial series to support i.MX8MM on linux-next [1]
> which is working on my setup. However I'm waiting for this series to
> move further to send those on the mailing list. Indeed I'm solely
> relaying on Marek testing to move further as I too don't have Exynos
> hardware to validate.
>
> [1] https://github.com/openedev/kernel/tree/imx8mm-dsi
>
> Thanks,
> Jagan.

Marek,

Have you had any time to test Jagan's latest series? There are several
of us waiting for the exynos series so as to gain support for imx8m
MIPI DSI on top of it.

What hardware is required to test this and where can it be found?

Best regards,

Tim

Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[Marek Szyprowski]

Hi Jagan,

On 03.03.2022 17:36, Jagan Teki wrote:
> Updated series about drm bridge conversion of exynos dsi.
>
> Previous version can be accessible, here [1].
>
> Patch 1: tc358764 panel_bridge API
>
> Patch 2: connector reset
>
> Patch 3: bridge attach in MIC
>
> Patch 4: panel_bridge API
>
> Patch 5: bridge conversion
>
> Patch 6: atomic functions
>
>
>
> Any inputs?


I'm really sorry for the delay on my side. I was really busy with other 
things and I was not able to check the display of the boards with remote 
access.


Finally, this patchset works properly on all my Exynos-based test systems:

1. Exynos4210 Trats with Samsung s6e8aa0 DSI panel

2. Exynos4412 Trats2 with Samsung s6e8aa0 DSI panel

3. Exynos5250 Arndale with TC358764 DSI-LVDS bridge and LVDS panel

4. Exynos5433 TM2e with Samsung s6e3hf2 DSI panel and internal Exynos 
MIC bridge


I will post my acked-by and tested-by tags for each patch.


> Jagan.
>
> Jagan Teki (6):
>    drm: bridge: tc358764: Use drm panel_bridge API
>    drm: bridge: panel: Reset the connector state pointer
>    exynos: drm: dsi: Attach in_bridge in MIC driver
>    drm: exynos: dsi: Use drm panel_bridge API
>    drm: exynos: dsi: Convert to bridge driver
>    drm: exynos: dsi: Switch to atomic funcs
>
>   drivers/gpu/drm/bridge/panel.c          |   3 +
>   drivers/gpu/drm/bridge/tc358764.c       | 104 +---------
>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 241 ++++++------------------
>   drivers/gpu/drm/exynos/exynos_drm_mic.c |  22 +++
>   4 files changed, 93 insertions(+), 277 deletions(-)
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v6 1/6] drm: bridge: tc358764: Use drm panel_bridge API

[Marek Szyprowski]

On 03.03.2022 17:36, Jagan Teki wrote:
> Replace the manual panel handling code by a drm panel_bridge via
> devm_drm_of_get_bridge().
>
> Adding panel_bridge handling,
>
> - Drops drm_connector and related operations as drm_bridge_attach
>    creates connector during attachment.
>
> - Drops panel pointer and panel healpers.
>
> This simplifies the driver and allows all components in the display
> pipeline to be treated as bridges.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
> Changes for v6:
> - none
> Changes for v2:
> - s/panel_bridge/next_bridge
> - drop unneeded headers
>
>   drivers/gpu/drm/bridge/tc358764.c | 104 ++----------------------------
>   1 file changed, 6 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c
> index c1e35bdf9232..dca41ed32f8a 100644
> --- a/drivers/gpu/drm/bridge/tc358764.c
> +++ b/drivers/gpu/drm/bridge/tc358764.c
> @@ -16,14 +16,9 @@
>   #include <video/mipi_display.h>
>   
>   #include <drm/drm_atomic_helper.h>
> -#include <drm/drm_bridge.h>
> -#include <drm/drm_crtc.h>
> -#include <drm/drm_fb_helper.h>
>   #include <drm/drm_mipi_dsi.h>
>   #include <drm/drm_of.h>
> -#include <drm/drm_panel.h>
>   #include <drm/drm_print.h>
> -#include <drm/drm_probe_helper.h>
>   
>   #define FLD_MASK(start, end)    (((1 << ((start) - (end) + 1)) - 1) << (end))
>   #define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> @@ -153,10 +148,9 @@ static const char * const tc358764_supplies[] = {
>   struct tc358764 {
>   	struct device *dev;
>   	struct drm_bridge bridge;
> -	struct drm_connector connector;
> +	struct drm_bridge *next_bridge;
>   	struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)];
>   	struct gpio_desc *gpio_reset;
> -	struct drm_panel *panel;
>   	int error;
>   };
>   
> @@ -210,12 +204,6 @@ static inline struct tc358764 *bridge_to_tc358764(struct drm_bridge *bridge)
>   	return container_of(bridge, struct tc358764, bridge);
>   }
>   
> -static inline
> -struct tc358764 *connector_to_tc358764(struct drm_connector *connector)
> -{
> -	return container_of(connector, struct tc358764, connector);
> -}
> -
>   static int tc358764_init(struct tc358764 *ctx)
>   {
>   	u32 v = 0;
> @@ -278,43 +266,11 @@ static void tc358764_reset(struct tc358764 *ctx)
>   	usleep_range(1000, 2000);
>   }
>   
> -static int tc358764_get_modes(struct drm_connector *connector)
> -{
> -	struct tc358764 *ctx = connector_to_tc358764(connector);
> -
> -	return drm_panel_get_modes(ctx->panel, connector);
> -}
> -
> -static const
> -struct drm_connector_helper_funcs tc358764_connector_helper_funcs = {
> -	.get_modes = tc358764_get_modes,
> -};
> -
> -static const struct drm_connector_funcs tc358764_connector_funcs = {
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = drm_connector_cleanup,
> -	.reset = drm_atomic_helper_connector_reset,
> -	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> -	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> -};
> -
> -static void tc358764_disable(struct drm_bridge *bridge)
> -{
> -	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> -	int ret = drm_panel_disable(bridge_to_tc358764(bridge)->panel);
> -
> -	if (ret < 0)
> -		dev_err(ctx->dev, "error disabling panel (%d)\n", ret);
> -}
> -
>   static void tc358764_post_disable(struct drm_bridge *bridge)
>   {
>   	struct tc358764 *ctx = bridge_to_tc358764(bridge);
>   	int ret;
>   
> -	ret = drm_panel_unprepare(ctx->panel);
> -	if (ret < 0)
> -		dev_err(ctx->dev, "error unpreparing panel (%d)\n", ret);
>   	tc358764_reset(ctx);
>   	usleep_range(10000, 15000);
>   	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> @@ -335,72 +291,25 @@ static void tc358764_pre_enable(struct drm_bridge *bridge)
>   	ret = tc358764_init(ctx);
>   	if (ret < 0)
>   		dev_err(ctx->dev, "error initializing bridge (%d)\n", ret);
> -	ret = drm_panel_prepare(ctx->panel);
> -	if (ret < 0)
> -		dev_err(ctx->dev, "error preparing panel (%d)\n", ret);
> -}
> -
> -static void tc358764_enable(struct drm_bridge *bridge)
> -{
> -	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> -	int ret = drm_panel_enable(ctx->panel);
> -
> -	if (ret < 0)
> -		dev_err(ctx->dev, "error enabling panel (%d)\n", ret);
>   }
>   
>   static int tc358764_attach(struct drm_bridge *bridge,
>   			   enum drm_bridge_attach_flags flags)
> -{
> -	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> -	struct drm_device *drm = bridge->dev;
> -	int ret;
> -
> -	if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> -		DRM_ERROR("Fix bridge driver to make connector optional!");
> -		return -EINVAL;
> -	}
> -
> -	ctx->connector.polled = DRM_CONNECTOR_POLL_HPD;
> -	ret = drm_connector_init(drm, &ctx->connector,
> -				 &tc358764_connector_funcs,
> -				 DRM_MODE_CONNECTOR_LVDS);
> -	if (ret) {
> -		DRM_ERROR("Failed to initialize connector\n");
> -		return ret;
> -	}
> -
> -	drm_connector_helper_add(&ctx->connector,
> -				 &tc358764_connector_helper_funcs);
> -	drm_connector_attach_encoder(&ctx->connector, bridge->encoder);
> -	ctx->connector.funcs->reset(&ctx->connector);
> -	drm_connector_register(&ctx->connector);
> -
> -	return 0;
> -}
> -
> -static void tc358764_detach(struct drm_bridge *bridge)
>   {
>   	struct tc358764 *ctx = bridge_to_tc358764(bridge);
>   
> -	drm_connector_unregister(&ctx->connector);
> -	ctx->panel = NULL;
> -	drm_connector_put(&ctx->connector);
> +	return drm_bridge_attach(bridge->encoder, ctx->next_bridge, bridge, flags);
>   }
>   
>   static const struct drm_bridge_funcs tc358764_bridge_funcs = {
> -	.disable = tc358764_disable,
>   	.post_disable = tc358764_post_disable,
> -	.enable = tc358764_enable,
>   	.pre_enable = tc358764_pre_enable,
>   	.attach = tc358764_attach,
> -	.detach = tc358764_detach,
>   };
>   
>   static int tc358764_parse_dt(struct tc358764 *ctx)
>   {
>   	struct device *dev = ctx->dev;
> -	int ret;
>   
>   	ctx->gpio_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
>   	if (IS_ERR(ctx->gpio_reset)) {
> @@ -408,12 +317,11 @@ static int tc358764_parse_dt(struct tc358764 *ctx)
>   		return PTR_ERR(ctx->gpio_reset);
>   	}
>   
> -	ret = drm_of_find_panel_or_bridge(ctx->dev->of_node, 1, 0, &ctx->panel,
> -					  NULL);
> -	if (ret && ret != -EPROBE_DEFER)
> -		dev_err(dev, "cannot find panel (%d)\n", ret);
> +	ctx->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> +	if (IS_ERR(ctx->next_bridge))
> +		return PTR_ERR(ctx->next_bridge);
>   
> -	return ret;
> +	return 0;
>   }
>   
>   static int tc358764_configure_regulators(struct tc358764 *ctx)

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v6 2/6] drm: bridge: panel: Reset the connector state pointer

[Marek Szyprowski]

On 03.03.2022 17:36, Jagan Teki wrote:
> Trigger hotplug event with drm_kms_helper_hotplug_event might fail if the
> connector state pointer is NULL.
>
> BUG observed in exynos dsi driver where drm_bridge_attach is trying to register
> a connector in panel_bridge before the hotplug event is triggered,
>
> WARNING: CPU: 1 PID: 1 at drivers/gpu/drm/drm_atomic_state_helper.c:494 drm_atomic_helper_connector_duplicate_state+0x94/0x9c
> Modules linked in:
> CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W 5.16.0-rc1-00009-g704b1dbfa4c2 #11058
> Hardware name: Samsung Exynos (Flattened Device Tree)
> [<c0110b30>] (unwind_backtrace) from [<c010c618>] (show_stack+0x10/0x14)
> [<c010c618>] (show_stack) from [<c0b657d4>] (dump_stack_lvl+0x58/0x70)
> [<c0b657d4>] (dump_stack_lvl) from [<c01261dc>] (__warn+0xd0/0x134)
> [<c01261dc>] (__warn) from [<c0b5f628>] (warn_slowpath_fmt+0x5c/0xb4)
> [<c0b5f628>] (warn_slowpath_fmt) from [<c064bce4>] (drm_atomic_helper_connector_duplicate_state+0x94/0x9c)
> [<c064bce4>] (drm_atomic_helper_connector_duplicate_state) from [<c0666b64>] (drm_atomic_get_connector_state+0xd4/0x190)
> [<c0666b64>] (drm_atomic_get_connector_state) from [<c0667928>] (__drm_atomic_helper_set_config+0x314/0x368)
> [<c0667928>] (__drm_atomic_helper_set_config) from [<c067e628>] (drm_client_modeset_commit_atomic+0x170/0x278)
> [<c067e628>] (drm_client_modeset_commit_atomic) from [<c067e800>] (drm_client_modeset_commit_locked+0x60/0x1c8)
> [<c067e800>] (drm_client_modeset_commit_locked) from [<c067e98c>] (drm_client_modeset_commit+0x24/0x40)
> [<c067e98c>] (drm_client_modeset_commit) from [<c06509c0>] (drm_fb_helper_set_par+0xb8/0xf8)
> [<c06509c0>] (drm_fb_helper_set_par) from [<c05b86d0>] (fbcon_init+0x2c0/0x518)
> [<c05b86d0>] (fbcon_init) from [<c060636c>] (visual_init+0xc0/0x108)
> [<c060636c>] (visual_init) from [<c06085e4>] (do_bind_con_driver+0x1b8/0x3a4)
> [<c06085e4>] (do_bind_con_driver) from [<c0608b40>] (do_take_over_console+0x13c/0x1e8)
> [<c0608b40>] (do_take_over_console) from [<c05b6854>] (do_fbcon_takeover+0x78/0xd8)
> [<c05b6854>] (do_fbcon_takeover) from [<c05b1154>] (register_framebuffer+0x208/0x2e0)
> [<c05b1154>] (register_framebuffer) from [<c064ead0>] (__drm_fb_helper_initial_config_and_unlock+0x400/0x63c)
> [<c064ead0>] (__drm_fb_helper_initial_config_and_unlock) from [<c063a718>] (drm_kms_helper_hotplug_event+0x24/0x30)
> [<c063a718>] (drm_kms_helper_hotplug_event) from [<c068f668>] (exynos_dsi_host_attach+0x174/0x1fc)
> [<c068f668>] (exynos_dsi_host_attach) from [<c0699354>] (s6e8aa0_probe+0x1b4/0x218)
>
> So reset the atomic state for a given connector by freeing the state pointer
> and allocate a new empty state object. This can be done using connector
> funcs->reset helper and has to be done before the hotplug even calls.
>
> This patch calls the connector->funcs->reset in panel_bridge_attach.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
> Changes for v6, v5:
> - none
> Changes for v4:
> - new patch
>
>   drivers/gpu/drm/bridge/panel.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
> index b32295abd9e7..f6eea194482a 100644
> --- a/drivers/gpu/drm/bridge/panel.c
> +++ b/drivers/gpu/drm/bridge/panel.c
> @@ -83,6 +83,9 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
>   	drm_connector_attach_encoder(&panel_bridge->connector,
>   					  bridge->encoder);
>   
> +	if (connector->funcs->reset)
> +		connector->funcs->reset(connector);
> +
>   	return 0;
>   }
>   

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v6 3/6] exynos: drm: dsi: Attach in_bridge in MIC driver

[Marek Szyprowski]

On 03.03.2022 17:36, Jagan Teki wrote:
> MIC drivers in the Exynos5433 display pipeline are already registered
> as bridge drivers and it is more advisable to attach the downstream
> bridge on the bridge attach call instead of doing the same in the
> DSI driver.
>
> This makes bridge attachment more meaningful and avoids the races
> during bridge function calls.
>
> So, move the bridge finding and drm_bridge_attach from DSI to MIC.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
> Changes for v6:
> - new patch
>
>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 15 ---------------
>   drivers/gpu/drm/exynos/exynos_drm_mic.c | 22 ++++++++++++++++++++++
>   2 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index b7d0a4aead0a..741c046513e8 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1660,11 +1660,6 @@ static int exynos_dsi_of_read_u32(const struct device_node *np,
>   	return ret;
>   }
>   
> -enum {
> -	DSI_PORT_IN,
> -	DSI_PORT_OUT
> -};
> -
>   static int exynos_dsi_parse_dt(struct exynos_dsi *dsi)
>   {
>   	struct device *dev = dsi->dev;
> @@ -1695,8 +1690,6 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>   	struct exynos_dsi *dsi = dev_get_drvdata(dev);
>   	struct drm_encoder *encoder = &dsi->encoder;
>   	struct drm_device *drm_dev = data;
> -	struct device_node *in_bridge_node;
> -	struct drm_bridge *in_bridge;
>   	int ret;
>   
>   	drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_TMDS);
> @@ -1707,14 +1700,6 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>   	if (ret < 0)
>   		return ret;
>   
> -	in_bridge_node = of_graph_get_remote_node(dev->of_node, DSI_PORT_IN, 0);
> -	if (in_bridge_node) {
> -		in_bridge = of_drm_find_bridge(in_bridge_node);
> -		if (in_bridge)
> -			drm_bridge_attach(encoder, in_bridge, NULL, 0);
> -		of_node_put(in_bridge_node);
> -	}
> -
>   	return mipi_dsi_host_register(&dsi->dsi_host);
>   }
>   
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> index 32672bf8ae4a..9e06f8e2a863 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> @@ -102,6 +102,7 @@ struct exynos_mic {
>   	struct videomode vm;
>   	struct drm_encoder *encoder;
>   	struct drm_bridge bridge;
> +	struct drm_bridge *next_bridge;
>   
>   	bool enabled;
>   };
> @@ -298,12 +299,22 @@ static void mic_pre_enable(struct drm_bridge *bridge)
>   
>   static void mic_enable(struct drm_bridge *bridge) { }
>   
> +static int mic_attach(struct drm_bridge *bridge,
> +		      enum drm_bridge_attach_flags flags)
> +{
> +	struct exynos_mic *mic = bridge->driver_private;
> +
> +	return drm_bridge_attach(bridge->encoder, mic->next_bridge,
> +				 &mic->bridge, flags);
> +}
> +
>   static const struct drm_bridge_funcs mic_bridge_funcs = {
>   	.disable = mic_disable,
>   	.post_disable = mic_post_disable,
>   	.mode_set = mic_mode_set,
>   	.pre_enable = mic_pre_enable,
>   	.enable = mic_enable,
> +	.attach = mic_attach,
>   };
>   
>   static int exynos_mic_bind(struct device *dev, struct device *master,
> @@ -377,6 +388,7 @@ static int exynos_mic_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct exynos_mic *mic;
> +	struct device_node *remote;
>   	struct resource res;
>   	int ret, i;
>   
> @@ -420,6 +432,16 @@ static int exynos_mic_probe(struct platform_device *pdev)
>   		}
>   	}
>   
> +	remote = of_graph_get_remote_node(dev->of_node, 1, 0);
> +	mic->next_bridge = of_drm_find_bridge(remote);
> +	if (IS_ERR(mic->next_bridge)) {
> +		DRM_DEV_ERROR(dev, "mic: Failed to find next bridge\n");
> +		ret = PTR_ERR(mic->next_bridge);
> +		goto err;
> +	}
> +
> +	of_node_put(remote);
> +
>   	platform_set_drvdata(pdev, mic);
>   
>   	mic->bridge.funcs = &mic_bridge_funcs;

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v6 4/6] drm: exynos: dsi: Use drm panel_bridge API

[Marek Szyprowski]

On 03.03.2022 17:36, Jagan Teki wrote:
> Replace the manual panel handling code by a drm panel_bridge via
> devm_drm_of_get_bridge().
>
> Adding panel_bridge handling,
>
> - Drops drm_connector and related operations as drm_bridge_attach
>    creates connector during attachment.
>
> - Drops panel pointer and iterate the bridge, so-that it can operate
>    the normal bridge and panel_bridge in constitutive callbacks.
>
> This simplifies the driver and allows all components in the display
> pipeline to be treated as bridges.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
> Changes for v6, v5:
> - none
> Changes for v4:
> - drop unneeded headers
> Changes for v3:
> - fix port number
> - add print for attached device
> Changes for v2:
> - new patch
>
>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 157 ++++--------------------
>   1 file changed, 23 insertions(+), 134 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 741c046513e8..953094133ed8 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -24,9 +24,7 @@
>   
>   #include <drm/drm_atomic_helper.h>
>   #include <drm/drm_bridge.h>
> -#include <drm/drm_fb_helper.h>
>   #include <drm/drm_mipi_dsi.h>
> -#include <drm/drm_panel.h>
>   #include <drm/drm_print.h>
>   #include <drm/drm_probe_helper.h>
>   #include <drm/drm_simple_kms_helper.h>
> @@ -253,8 +251,6 @@ struct exynos_dsi_driver_data {
>   struct exynos_dsi {
>   	struct drm_encoder encoder;
>   	struct mipi_dsi_host dsi_host;
> -	struct drm_connector connector;
> -	struct drm_panel *panel;
>   	struct list_head bridge_chain;
>   	struct drm_bridge *out_bridge;
>   	struct device *dev;
> @@ -285,7 +281,6 @@ struct exynos_dsi {
>   };
>   
>   #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
> -#define connector_to_dsi(c) container_of(c, struct exynos_dsi, connector)
>   
>   static inline struct exynos_dsi *encoder_to_dsi(struct drm_encoder *e)
>   {
> @@ -1380,42 +1375,21 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
>   
>   	dsi->state |= DSIM_STATE_ENABLED;
>   
> -	if (dsi->panel) {
> -		ret = drm_panel_prepare(dsi->panel);
> -		if (ret < 0)
> -			goto err_put_sync;
> -	} else {
> -		list_for_each_entry_reverse(iter, &dsi->bridge_chain,
> -					    chain_node) {
> -			if (iter->funcs->pre_enable)
> -				iter->funcs->pre_enable(iter);
> -		}
> +	list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) {
> +		if (iter->funcs->pre_enable)
> +			iter->funcs->pre_enable(iter);
>   	}
>   
>   	exynos_dsi_set_display_mode(dsi);
>   	exynos_dsi_set_display_enable(dsi, true);
>   
> -	if (dsi->panel) {
> -		ret = drm_panel_enable(dsi->panel);
> -		if (ret < 0)
> -			goto err_display_disable;
> -	} else {
> -		list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
> -			if (iter->funcs->enable)
> -				iter->funcs->enable(iter);
> -		}
> +	list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
> +		if (iter->funcs->enable)
> +			iter->funcs->enable(iter);
>   	}
>   
>   	dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
>   	return;
> -
> -err_display_disable:
> -	exynos_dsi_set_display_enable(dsi, false);
> -	drm_panel_unprepare(dsi->panel);
> -
> -err_put_sync:
> -	dsi->state &= ~DSIM_STATE_ENABLED;
> -	pm_runtime_put(dsi->dev);
>   }
>   
>   static void exynos_dsi_disable(struct drm_encoder *encoder)
> @@ -1428,15 +1402,12 @@ static void exynos_dsi_disable(struct drm_encoder *encoder)
>   
>   	dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
>   
> -	drm_panel_disable(dsi->panel);
> -
>   	list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) {
>   		if (iter->funcs->disable)
>   			iter->funcs->disable(iter);
>   	}
>   
>   	exynos_dsi_set_display_enable(dsi, false);
> -	drm_panel_unprepare(dsi->panel);
>   
>   	list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
>   		if (iter->funcs->post_disable)
> @@ -1456,70 +1427,6 @@ static void exynos_dsi_mode_set(struct drm_encoder *encoder,
>   	drm_mode_copy(&dsi->mode, adjusted_mode);
>   }
>   
> -static enum drm_connector_status
> -exynos_dsi_detect(struct drm_connector *connector, bool force)
> -{
> -	return connector->status;
> -}
> -
> -static void exynos_dsi_connector_destroy(struct drm_connector *connector)
> -{
> -	drm_connector_unregister(connector);
> -	drm_connector_cleanup(connector);
> -	connector->dev = NULL;
> -}
> -
> -static const struct drm_connector_funcs exynos_dsi_connector_funcs = {
> -	.detect = exynos_dsi_detect,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.destroy = exynos_dsi_connector_destroy,
> -	.reset = drm_atomic_helper_connector_reset,
> -	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> -	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> -};
> -
> -static int exynos_dsi_get_modes(struct drm_connector *connector)
> -{
> -	struct exynos_dsi *dsi = connector_to_dsi(connector);
> -
> -	if (dsi->panel)
> -		return drm_panel_get_modes(dsi->panel, connector);
> -
> -	return 0;
> -}
> -
> -static const struct drm_connector_helper_funcs exynos_dsi_connector_helper_funcs = {
> -	.get_modes = exynos_dsi_get_modes,
> -};
> -
> -static int exynos_dsi_create_connector(struct drm_encoder *encoder)
> -{
> -	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
> -	struct drm_connector *connector = &dsi->connector;
> -	struct drm_device *drm = encoder->dev;
> -	int ret;
> -
> -	connector->polled = DRM_CONNECTOR_POLL_HPD;
> -
> -	ret = drm_connector_init(drm, connector, &exynos_dsi_connector_funcs,
> -				 DRM_MODE_CONNECTOR_DSI);
> -	if (ret) {
> -		DRM_DEV_ERROR(dsi->dev,
> -			      "Failed to initialize connector with drm\n");
> -		return ret;
> -	}
> -
> -	connector->status = connector_status_disconnected;
> -	drm_connector_helper_add(connector, &exynos_dsi_connector_helper_funcs);
> -	drm_connector_attach_encoder(connector, encoder);
> -	if (!drm->registered)
> -		return 0;
> -
> -	connector->funcs->reset(connector);
> -	drm_connector_register(connector);
> -	return 0;
> -}
> -
>   static const struct drm_encoder_helper_funcs exynos_dsi_encoder_helper_funcs = {
>   	.enable = exynos_dsi_enable,
>   	.disable = exynos_dsi_disable,
> @@ -1532,33 +1439,23 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>   				  struct mipi_dsi_device *device)
>   {
>   	struct exynos_dsi *dsi = host_to_dsi(host);
> +	struct device *dev = dsi->dev;
>   	struct drm_encoder *encoder = &dsi->encoder;
>   	struct drm_device *drm = encoder->dev;
> -	struct drm_bridge *out_bridge;
> -
> -	out_bridge  = of_drm_find_bridge(device->dev.of_node);
> -	if (out_bridge) {
> -		drm_bridge_attach(encoder, out_bridge, NULL, 0);
> -		dsi->out_bridge = out_bridge;
> -		list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain);
> -	} else {
> -		int ret = exynos_dsi_create_connector(encoder);
> -
> -		if (ret) {
> -			DRM_DEV_ERROR(dsi->dev,
> -				      "failed to create connector ret = %d\n",
> -				      ret);
> -			drm_encoder_cleanup(encoder);
> -			return ret;
> -		}
> +	int ret;
>   
> -		dsi->panel = of_drm_find_panel(device->dev.of_node);
> -		if (IS_ERR(dsi->panel))
> -			dsi->panel = NULL;
> -		else
> -			dsi->connector.status = connector_status_connected;
> +	dsi->out_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> +	if (IS_ERR(dsi->out_bridge)) {
> +		ret = PTR_ERR(dsi->out_bridge);
> +		DRM_DEV_ERROR(dev, "failed to find the bridge: %d\n", ret);
> +		return ret;
>   	}
>   
> +	DRM_DEV_INFO(dev, "Attached %s device\n", device->name);
> +
> +	drm_bridge_attach(encoder, dsi->out_bridge, NULL, 0);
> +	list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain);
> +
>   	/*
>   	 * This is a temporary solution and should be made by more generic way.
>   	 *
> @@ -1566,7 +1463,7 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>   	 * TE interrupt handler.
>   	 */
>   	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO)) {
> -		int ret = exynos_dsi_register_te_irq(dsi, &device->dev);
> +		ret = exynos_dsi_register_te_irq(dsi, &device->dev);
>   		if (ret)
>   			return ret;
>   	}
> @@ -1593,18 +1490,10 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
>   	struct exynos_dsi *dsi = host_to_dsi(host);
>   	struct drm_device *drm = dsi->encoder.dev;
>   
> -	if (dsi->panel) {
> -		mutex_lock(&drm->mode_config.mutex);
> -		exynos_dsi_disable(&dsi->encoder);
> -		dsi->panel = NULL;
> -		dsi->connector.status = connector_status_disconnected;
> -		mutex_unlock(&drm->mode_config.mutex);
> -	} else {
> -		if (dsi->out_bridge->funcs->detach)
> -			dsi->out_bridge->funcs->detach(dsi->out_bridge);
> -		dsi->out_bridge = NULL;
> -		INIT_LIST_HEAD(&dsi->bridge_chain);
> -	}
> +	if (dsi->out_bridge->funcs->detach)
> +		dsi->out_bridge->funcs->detach(dsi->out_bridge);
> +	dsi->out_bridge = NULL;
> +	INIT_LIST_HEAD(&dsi->bridge_chain);
>   
>   	if (drm->mode_config.poll_enabled)
>   		drm_kms_helper_hotplug_event(drm);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v6 5/6] drm: exynos: dsi: Convert to bridge driver

[Marek Szyprowski]

On 03.03.2022 17:36, Jagan Teki wrote:
> Convert the encoders to bridge drivers in order to standardize on
> a single API with built-in dumb encoder support for compatibility
> with existing component drivers.
>
> Driver bridge conversion will help to reuse the same bridge on
> different platforms as exynos dsi driver can be used as a Samsung
> DSIM and use it for i.MX8MM platform.
>
> Bridge conversion,
>
> - Drops drm_encoder_helper_funcs.
>
> - Adds drm_bridge_funcs and register a drm bridge.
>
> - Drops bridge_chain.
>
> - Separate pre_enable from enable function.
>
> - Separate post_disable from disable function.
>
> Convert it.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
> Changes for v6, v5:
> - none
> Changes for v4:
> - add pre_enable function
> - add post_disable function
> Changes for v3:
> - move bridge add in host_attach
> - move bridge remove in host_detach
> - use flags, bridge in drm_bridge_attach in attch
> Changes for v2:
> - drop bridge_chain
>
>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 88 +++++++++++++------------
>   1 file changed, 45 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 953094133ed8..59a4f7f52180 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -251,7 +251,7 @@ struct exynos_dsi_driver_data {
>   struct exynos_dsi {
>   	struct drm_encoder encoder;
>   	struct mipi_dsi_host dsi_host;
> -	struct list_head bridge_chain;
> +	struct drm_bridge bridge;
>   	struct drm_bridge *out_bridge;
>   	struct device *dev;
>   	struct drm_display_mode mode;
> @@ -282,9 +282,9 @@ struct exynos_dsi {
>   
>   #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
>   
> -static inline struct exynos_dsi *encoder_to_dsi(struct drm_encoder *e)
> +static inline struct exynos_dsi *bridge_to_dsi(struct drm_bridge *b)
>   {
> -	return container_of(e, struct exynos_dsi, encoder);
> +	return container_of(b, struct exynos_dsi, bridge);
>   }
>   
>   enum reg_idx {
> @@ -1358,10 +1358,9 @@ static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi)
>   	}
>   }
>   
> -static void exynos_dsi_enable(struct drm_encoder *encoder)
> +static void exynos_dsi_pre_enable(struct drm_bridge *bridge)
>   {
> -	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
> -	struct drm_bridge *iter;
> +	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
>   	int ret;
>   
>   	if (dsi->state & DSIM_STATE_ENABLED)
> @@ -1374,63 +1373,64 @@ static void exynos_dsi_enable(struct drm_encoder *encoder)
>   	}
>   
>   	dsi->state |= DSIM_STATE_ENABLED;
> +}
>   
> -	list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) {
> -		if (iter->funcs->pre_enable)
> -			iter->funcs->pre_enable(iter);
> -	}
> +static void exynos_dsi_enable(struct drm_bridge *bridge)
> +{
> +	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
>   
>   	exynos_dsi_set_display_mode(dsi);
>   	exynos_dsi_set_display_enable(dsi, true);
>   
> -	list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
> -		if (iter->funcs->enable)
> -			iter->funcs->enable(iter);
> -	}
> -
>   	dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
> +
>   	return;
>   }
>   
> -static void exynos_dsi_disable(struct drm_encoder *encoder)
> +static void exynos_dsi_disable(struct drm_bridge *bridge)
>   {
> -	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
> -	struct drm_bridge *iter;
> +	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
>   
>   	if (!(dsi->state & DSIM_STATE_ENABLED))
>   		return;
>   
>   	dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
> +}
>   
> -	list_for_each_entry_reverse(iter, &dsi->bridge_chain, chain_node) {
> -		if (iter->funcs->disable)
> -			iter->funcs->disable(iter);
> -	}
> +static void exynos_dsi_post_disable(struct drm_bridge *bridge)
> +{
> +	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
>   
>   	exynos_dsi_set_display_enable(dsi, false);
>   
> -	list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
> -		if (iter->funcs->post_disable)
> -			iter->funcs->post_disable(iter);
> -	}
> -
>   	dsi->state &= ~DSIM_STATE_ENABLED;
>   	pm_runtime_put_sync(dsi->dev);
>   }
>   
> -static void exynos_dsi_mode_set(struct drm_encoder *encoder,
> -				struct drm_display_mode *mode,
> -				struct drm_display_mode *adjusted_mode)
> +static void exynos_dsi_mode_set(struct drm_bridge *bridge,
> +				const struct drm_display_mode *mode,
> +				const struct drm_display_mode *adjusted_mode)
>   {
> -	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
> +	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
>   
>   	drm_mode_copy(&dsi->mode, adjusted_mode);
>   }
>   
> -static const struct drm_encoder_helper_funcs exynos_dsi_encoder_helper_funcs = {
> -	.enable = exynos_dsi_enable,
> -	.disable = exynos_dsi_disable,
> -	.mode_set = exynos_dsi_mode_set,
> +static int exynos_dsi_attach(struct drm_bridge *bridge,
> +			     enum drm_bridge_attach_flags flags)
> +{
> +	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
> +
> +	return drm_bridge_attach(bridge->encoder, dsi->out_bridge, NULL, flags);
> +}
> +
> +static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {
> +	.pre_enable			= exynos_dsi_pre_enable,
> +	.enable				= exynos_dsi_enable,
> +	.disable			= exynos_dsi_disable,
> +	.post_disable			= exynos_dsi_post_disable,
> +	.mode_set			= exynos_dsi_mode_set,
> +	.attach				= exynos_dsi_attach,
>   };
>   
>   MODULE_DEVICE_TABLE(of, exynos_dsi_of_match);
> @@ -1453,8 +1453,9 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>   
>   	DRM_DEV_INFO(dev, "Attached %s device\n", device->name);
>   
> -	drm_bridge_attach(encoder, dsi->out_bridge, NULL, 0);
> -	list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain);
> +	drm_bridge_add(&dsi->bridge);
> +
> +	drm_bridge_attach(encoder, &dsi->bridge, NULL, 0);
>   
>   	/*
>   	 * This is a temporary solution and should be made by more generic way.
> @@ -1493,13 +1494,14 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
>   	if (dsi->out_bridge->funcs->detach)
>   		dsi->out_bridge->funcs->detach(dsi->out_bridge);
>   	dsi->out_bridge = NULL;
> -	INIT_LIST_HEAD(&dsi->bridge_chain);
>   
>   	if (drm->mode_config.poll_enabled)
>   		drm_kms_helper_hotplug_event(drm);
>   
>   	exynos_dsi_unregister_te_irq(dsi);
>   
> +	drm_bridge_remove(&dsi->bridge);
> +
>   	return 0;
>   }
>   
> @@ -1583,8 +1585,6 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
>   
>   	drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_TMDS);
>   
> -	drm_encoder_helper_add(encoder, &exynos_dsi_encoder_helper_funcs);
> -
>   	ret = exynos_drm_set_possible_crtcs(encoder, EXYNOS_DISPLAY_TYPE_LCD);
>   	if (ret < 0)
>   		return ret;
> @@ -1596,9 +1596,8 @@ static void exynos_dsi_unbind(struct device *dev, struct device *master,
>   				void *data)
>   {
>   	struct exynos_dsi *dsi = dev_get_drvdata(dev);
> -	struct drm_encoder *encoder = &dsi->encoder;
>   
> -	exynos_dsi_disable(encoder);
> +	exynos_dsi_disable(&dsi->bridge);
>   
>   	mipi_dsi_host_unregister(&dsi->dsi_host);
>   }
> @@ -1621,7 +1620,6 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>   	init_completion(&dsi->completed);
>   	spin_lock_init(&dsi->transfer_lock);
>   	INIT_LIST_HEAD(&dsi->transfer_list);
> -	INIT_LIST_HEAD(&dsi->bridge_chain);
>   
>   	dsi->dsi_host.ops = &exynos_dsi_ops;
>   	dsi->dsi_host.dev = dev;
> @@ -1689,6 +1687,10 @@ static int exynos_dsi_probe(struct platform_device *pdev)
>   
>   	pm_runtime_enable(dev);
>   
> +	dsi->bridge.funcs = &exynos_dsi_bridge_funcs;
> +	dsi->bridge.of_node = dev->of_node;
> +	dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
> +
>   	ret = component_add(dev, &exynos_dsi_component_ops);
>   	if (ret)
>   		goto err_disable_runtime;

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v6 6/6] drm: exynos: dsi: Switch to atomic funcs

[Marek Szyprowski]

On 03.03.2022 17:36, Jagan Teki wrote:
> The new support drm bridges are moving towards atomic functions.
>
> Replace atomic version of functions to continue the transition
> to the atomic API.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
> Changes for v6, v5, v4, v3:
> - none
>
>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 25 ++++++++++++++++---------
>   1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 59a4f7f52180..06130eee8df8 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1358,7 +1358,8 @@ static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi)
>   	}
>   }
>   
> -static void exynos_dsi_pre_enable(struct drm_bridge *bridge)
> +static void exynos_dsi_atomic_pre_enable(struct drm_bridge *bridge,
> +					 struct drm_bridge_state *old_bridge_state)
>   {
>   	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
>   	int ret;
> @@ -1375,7 +1376,8 @@ static void exynos_dsi_pre_enable(struct drm_bridge *bridge)
>   	dsi->state |= DSIM_STATE_ENABLED;
>   }
>   
> -static void exynos_dsi_enable(struct drm_bridge *bridge)
> +static void exynos_dsi_atomic_enable(struct drm_bridge *bridge,
> +				     struct drm_bridge_state *old_bridge_state)
>   {
>   	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
>   
> @@ -1387,7 +1389,8 @@ static void exynos_dsi_enable(struct drm_bridge *bridge)
>   	return;
>   }
>   
> -static void exynos_dsi_disable(struct drm_bridge *bridge)
> +static void exynos_dsi_atomic_disable(struct drm_bridge *bridge,
> +				      struct drm_bridge_state *old_bridge_state)
>   {
>   	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
>   
> @@ -1397,7 +1400,8 @@ static void exynos_dsi_disable(struct drm_bridge *bridge)
>   	dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
>   }
>   
> -static void exynos_dsi_post_disable(struct drm_bridge *bridge)
> +static void exynos_dsi_atomic_post_disable(struct drm_bridge *bridge,
> +					   struct drm_bridge_state *old_bridge_state)
>   {
>   	struct exynos_dsi *dsi = bridge_to_dsi(bridge);
>   
> @@ -1425,10 +1429,13 @@ static int exynos_dsi_attach(struct drm_bridge *bridge,
>   }
>   
>   static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {
> -	.pre_enable			= exynos_dsi_pre_enable,
> -	.enable				= exynos_dsi_enable,
> -	.disable			= exynos_dsi_disable,
> -	.post_disable			= exynos_dsi_post_disable,
> +	.atomic_duplicate_state		= drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
> +	.atomic_reset			= drm_atomic_helper_bridge_reset,
> +	.atomic_pre_enable		= exynos_dsi_atomic_pre_enable,
> +	.atomic_enable			= exynos_dsi_atomic_enable,
> +	.atomic_disable			= exynos_dsi_atomic_disable,
> +	.atomic_post_disable		= exynos_dsi_atomic_post_disable,
>   	.mode_set			= exynos_dsi_mode_set,
>   	.attach				= exynos_dsi_attach,
>   };
> @@ -1597,7 +1604,7 @@ static void exynos_dsi_unbind(struct device *dev, struct device *master,
>   {
>   	struct exynos_dsi *dsi = dev_get_drvdata(dev);
>   
> -	exynos_dsi_disable(&dsi->bridge);
> +	exynos_dsi_atomic_disable(&dsi->bridge, NULL);
>   
>   	mipi_dsi_host_unregister(&dsi->dsi_host);
>   }

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[Adam Ford]

On Fri, Mar 25, 2022 at 10:00 AM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> Hi Jagan,
>
> On 03.03.2022 17:36, Jagan Teki wrote:
> > Updated series about drm bridge conversion of exynos dsi.
> >
> > Previous version can be accessible, here [1].
> >
> > Patch 1: tc358764 panel_bridge API
> >
> > Patch 2: connector reset
> >
> > Patch 3: bridge attach in MIC
> >
> > Patch 4: panel_bridge API
> >
> > Patch 5: bridge conversion
> >
> > Patch 6: atomic functions
> >
> >
> >
> > Any inputs?
>
>
> I'm really sorry for the delay on my side. I was really busy with other
> things and I was not able to check the display of the boards with remote
> access.
>
>
> Finally, this patchset works properly on all my Exynos-based test systems:
>
> 1. Exynos4210 Trats with Samsung s6e8aa0 DSI panel
>
> 2. Exynos4412 Trats2 with Samsung s6e8aa0 DSI panel
>
> 3. Exynos5250 Arndale with TC358764 DSI-LVDS bridge and LVDS panel
>
> 4. Exynos5433 TM2e with Samsung s6e3hf2 DSI panel and internal Exynos
> MIC bridge
>
>
> I will post my acked-by and tested-by tags for each patch.

Thank you so much!  I think a lot of people will celebrate when this
gets approved and merged.  ;-)


adam
>
>
> > Jagan.
> >
> > Jagan Teki (6):
> >    drm: bridge: tc358764: Use drm panel_bridge API
> >    drm: bridge: panel: Reset the connector state pointer
> >    exynos: drm: dsi: Attach in_bridge in MIC driver
> >    drm: exynos: dsi: Use drm panel_bridge API
> >    drm: exynos: dsi: Convert to bridge driver
> >    drm: exynos: dsi: Switch to atomic funcs
> >
> >   drivers/gpu/drm/bridge/panel.c          |   3 +
> >   drivers/gpu/drm/bridge/tc358764.c       | 104 +---------
> >   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 241 ++++++------------------
> >   drivers/gpu/drm/exynos/exynos_drm_mic.c |  22 +++
> >   4 files changed, 93 insertions(+), 277 deletions(-)
> >
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>

Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[Robert Foss]

On Fri, 25 Mar 2022 at 17:04, Adam Ford <aford173@gmail.com> wrote:
>
> On Fri, Mar 25, 2022 at 10:00 AM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
> >
> > Hi Jagan,
> >
> > On 03.03.2022 17:36, Jagan Teki wrote:
> > > Updated series about drm bridge conversion of exynos dsi.
> > >
> > > Previous version can be accessible, here [1].
> > >
> > > Patch 1: tc358764 panel_bridge API
> > >
> > > Patch 2: connector reset
> > >
> > > Patch 3: bridge attach in MIC
> > >
> > > Patch 4: panel_bridge API
> > >
> > > Patch 5: bridge conversion
> > >
> > > Patch 6: atomic functions
> > >
> > >
> > >
> > > Any inputs?
> >
> >
> > I'm really sorry for the delay on my side. I was really busy with other
> > things and I was not able to check the display of the boards with remote
> > access.
> >
> >
> > Finally, this patchset works properly on all my Exynos-based test systems:
> >
> > 1. Exynos4210 Trats with Samsung s6e8aa0 DSI panel
> >
> > 2. Exynos4412 Trats2 with Samsung s6e8aa0 DSI panel
> >
> > 3. Exynos5250 Arndale with TC358764 DSI-LVDS bridge and LVDS panel
> >
> > 4. Exynos5433 TM2e with Samsung s6e3hf2 DSI panel and internal Exynos
> > MIC bridge
> >
> >
> > I will post my acked-by and tested-by tags for each patch.
>
> Thank you so much!  I think a lot of people will celebrate when this
> gets approved and merged.  ;-)
>
>

Applied to drm-misc-next.

Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[Marek Szyprowski]

Dear All,

On 31.03.2022 16:22, Robert Foss wrote:
> On Fri, 25 Mar 2022 at 17:04, Adam Ford <aford173@gmail.com> wrote:
>> On Fri, Mar 25, 2022 at 10:00 AM Marek Szyprowski
>> <m.szyprowski@samsung.com> wrote:
>>> On 03.03.2022 17:36, Jagan Teki wrote:
>>>> Updated series about drm bridge conversion of exynos dsi.
>>>>
>>>> Previous version can be accessible, here [1].
>>>>
>>>> Patch 1: tc358764 panel_bridge API
>>>>
>>>> Patch 2: connector reset
>>>>
>>>> Patch 3: bridge attach in MIC
>>>>
>>>> Patch 4: panel_bridge API
>>>>
>>>> Patch 5: bridge conversion
>>>>
>>>> Patch 6: atomic functions
>>>>
>>>>
>>>>
>>>> Any inputs?
>>>
>>> I'm really sorry for the delay on my side. I was really busy with other
>>> things and I was not able to check the display of the boards with remote
>>> access.
>>>
>>>
>>> Finally, this patchset works properly on all my Exynos-based test systems:
>>>
>>> 1. Exynos4210 Trats with Samsung s6e8aa0 DSI panel
>>>
>>> 2. Exynos4412 Trats2 with Samsung s6e8aa0 DSI panel
>>>
>>> 3. Exynos5250 Arndale with TC358764 DSI-LVDS bridge and LVDS panel
>>>
>>> 4. Exynos5433 TM2e with Samsung s6e3hf2 DSI panel and internal Exynos
>>> MIC bridge
>>>
>>>
>>> I will post my acked-by and tested-by tags for each patch.
>> Thank you so much!  I think a lot of people will celebrate when this
>> gets approved and merged.  ;-)
>>
>>
> Applied to drm-misc-next.


Thanks for merging this. Today (once the patches landed in linux-next) I 
found that there is one more issue left to fix.

On the Exynos4210-based Trats board I get the following error:

# ./modetest -c -Mexynos
could not get connector 56: No such file or directory
Segmentation fault

#

Surprisingly, all other boards, even Exynos4412-based Trats2 with 
exactly the same DSI controller and panel works fine:

# ./modetest -c -Mexynos
Connectors:
id      encoder status          name            size (mm) modes   encoders
71      70      connected       DSI-1           58x103 1       70
   modes:
         name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot)
   720x1280 60 720 725 730 735 1280 1293 1295 1296 57153 flags: ; type: 
preferred, driver
   props:
         1 EDID:
                 flags: immutable blob
                 blobs:

                 value:
         2 DPMS:
                 flags: enum
                 enums: On=0 Standby=1 Suspend=2 Off=3
                 value: 0
         5 link-status:
                 flags: enum
                 enums: Good=0 Bad=1
                 value: 0
         6 non-desktop:
                 flags: immutable range
                 values: 0 1
                 value: 0
         4 TILE:
                 flags: immutable blob
                 blobs:

                 value:
         20 CRTC_ID:
                 flags: object
                 value: 54
73      0       connected       HDMI-A-1        0x0 0       72
   props:
         1 EDID:
                 flags: immutable blob
                 blobs:

                 value:
         2 DPMS:
                 flags: enum
                 enums: On=0 Standby=1 Suspend=2 Off=3
                 value: 0
         5 link-status:
                 flags: enum
                 enums: Good=0 Bad=1
                 value: 0
         6 non-desktop:
                 flags: immutable range
                 values: 0 1
                 value: 0
         4 TILE:
                 flags: immutable blob
                 blobs:

                 value:
         20 CRTC_ID:
                 flags: object
                 value: 0

(the only difference between Trats and Trats2 is the fact that Trats2 
has also HDMI output implemented).

It looks that something is missing in the connector initialization, but 
I didn't dig enough into it. The emulated framebuffer is properly 
registered and displayed on the panel.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[Jagan Teki]

Hi Marek,

On Thu, Apr 7, 2022 at 4:54 PM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> Dear All,
>
> On 31.03.2022 16:22, Robert Foss wrote:
> > On Fri, 25 Mar 2022 at 17:04, Adam Ford <aford173@gmail.com> wrote:
> >> On Fri, Mar 25, 2022 at 10:00 AM Marek Szyprowski
> >> <m.szyprowski@samsung.com> wrote:
> >>> On 03.03.2022 17:36, Jagan Teki wrote:
> >>>> Updated series about drm bridge conversion of exynos dsi.
> >>>>
> >>>> Previous version can be accessible, here [1].
> >>>>
> >>>> Patch 1: tc358764 panel_bridge API
> >>>>
> >>>> Patch 2: connector reset
> >>>>
> >>>> Patch 3: bridge attach in MIC
> >>>>
> >>>> Patch 4: panel_bridge API
> >>>>
> >>>> Patch 5: bridge conversion
> >>>>
> >>>> Patch 6: atomic functions
> >>>>
> >>>>
> >>>>
> >>>> Any inputs?
> >>>
> >>> I'm really sorry for the delay on my side. I was really busy with other
> >>> things and I was not able to check the display of the boards with remote
> >>> access.
> >>>
> >>>
> >>> Finally, this patchset works properly on all my Exynos-based test systems:
> >>>
> >>> 1. Exynos4210 Trats with Samsung s6e8aa0 DSI panel
> >>>
> >>> 2. Exynos4412 Trats2 with Samsung s6e8aa0 DSI panel
> >>>
> >>> 3. Exynos5250 Arndale with TC358764 DSI-LVDS bridge and LVDS panel
> >>>
> >>> 4. Exynos5433 TM2e with Samsung s6e3hf2 DSI panel and internal Exynos
> >>> MIC bridge
> >>>
> >>>
> >>> I will post my acked-by and tested-by tags for each patch.
> >> Thank you so much!  I think a lot of people will celebrate when this
> >> gets approved and merged.  ;-)
> >>
> >>
> > Applied to drm-misc-next.
>
>
> Thanks for merging this. Today (once the patches landed in linux-next) I
> found that there is one more issue left to fix.
>
> On the Exynos4210-based Trats board I get the following error:
>
> # ./modetest -c -Mexynos
> could not get connector 56: No such file or directory
> Segmentation fault
>
> #
>
> Surprisingly, all other boards, even Exynos4412-based Trats2 with
> exactly the same DSI controller and panel works fine:
>
> # ./modetest -c -Mexynos
> Connectors:
> id      encoder status          name            size (mm) modes   encoders
> 71      70      connected       DSI-1           58x103 1       70
>    modes:
>          name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot)
>    720x1280 60 720 725 730 735 1280 1293 1295 1296 57153 flags: ; type:
> preferred, driver
>    props:
>          1 EDID:
>                  flags: immutable blob
>                  blobs:
>
>                  value:
>          2 DPMS:
>                  flags: enum
>                  enums: On=0 Standby=1 Suspend=2 Off=3
>                  value: 0
>          5 link-status:
>                  flags: enum
>                  enums: Good=0 Bad=1
>                  value: 0
>          6 non-desktop:
>                  flags: immutable range
>                  values: 0 1
>                  value: 0
>          4 TILE:
>                  flags: immutable blob
>                  blobs:
>
>                  value:
>          20 CRTC_ID:
>                  flags: object
>                  value: 54
> 73      0       connected       HDMI-A-1        0x0 0       72
>    props:
>          1 EDID:
>                  flags: immutable blob
>                  blobs:
>
>                  value:
>          2 DPMS:
>                  flags: enum
>                  enums: On=0 Standby=1 Suspend=2 Off=3
>                  value: 0
>          5 link-status:
>                  flags: enum
>                  enums: Good=0 Bad=1
>                  value: 0
>          6 non-desktop:
>                  flags: immutable range
>                  values: 0 1
>                  value: 0
>          4 TILE:
>                  flags: immutable blob
>                  blobs:
>
>                  value:
>          20 CRTC_ID:
>                  flags: object
>                  value: 0
>
> (the only difference between Trats and Trats2 is the fact that Trats2
> has also HDMI output implemented).
>
> It looks that something is missing in the connector initialization, but
> I didn't dig enough into it. The emulated framebuffer is properly
> registered and displayed on the panel.

Can you please share the full dmesg?

Thanks,
Jagan.

Re: [PATCH v6 0/6] drm: exynos: dsi: Convert drm bridge

[Marek Szyprowski]

Dear All,

On 07.04.2022 13:24, Marek Szyprowski wrote:
> On 31.03.2022 16:22, Robert Foss wrote:
>> On Fri, 25 Mar 2022 at 17:04, Adam Ford <aford173@gmail.com> wrote:
>>> On Fri, Mar 25, 2022 at 10:00 AM Marek Szyprowski
>>> <m.szyprowski@samsung.com> wrote:
>>>> On 03.03.2022 17:36, Jagan Teki wrote:
>>>>> Updated series about drm bridge conversion of exynos dsi.
>>>>>
>>>>> Previous version can be accessible, here [1].
>>>>>
>>>>> Patch 1: tc358764 panel_bridge API
>>>>>
>>>>> Patch 2: connector reset
>>>>>
>>>>> Patch 3: bridge attach in MIC
>>>>>
>>>>> Patch 4: panel_bridge API
>>>>>
>>>>> Patch 5: bridge conversion
>>>>>
>>>>> Patch 6: atomic functions
>>>>>
>>>>>
>>>>>
>>>>> Any inputs?
>>>>
>>>> I'm really sorry for the delay on my side. I was really busy with 
>>>> other
>>>> things and I was not able to check the display of the boards with 
>>>> remote
>>>> access.
>>>>
>>>>
>>>> Finally, this patchset works properly on all my Exynos-based test 
>>>> systems:
>>>>
>>>> 1. Exynos4210 Trats with Samsung s6e8aa0 DSI panel
>>>>
>>>> 2. Exynos4412 Trats2 with Samsung s6e8aa0 DSI panel
>>>>
>>>> 3. Exynos5250 Arndale with TC358764 DSI-LVDS bridge and LVDS panel
>>>>
>>>> 4. Exynos5433 TM2e with Samsung s6e3hf2 DSI panel and internal Exynos
>>>> MIC bridge
>>>>
>>>>
>>>> I will post my acked-by and tested-by tags for each patch.
>>> Thank you so much!  I think a lot of people will celebrate when this
>>> gets approved and merged.  ;-)
>>>
>>>
>> Applied to drm-misc-next.
>
>
> Thanks for merging this. Today (once the patches landed in linux-next) 
> I found that there is one more issue left to fix.
>
> On the Exynos4210-based Trats board I get the following error:
>
> # ./modetest -c -Mexynos
> could not get connector 56: No such file or directory
> Segmentation fault
>
> #
>
> Surprisingly, all other boards, even Exynos4412-based Trats2 with 
> exactly the same DSI controller and panel works fine:
>
> # ./modetest -c -Mexynos
> Connectors:
> id      encoder status          name            size (mm) modes encoders
> 71      70      connected       DSI-1           58x103 1       70

This is related to the asynchronous DSI driver registration and DSI 
device probe.

If the DSI driver has been registered before the DRM component device 
bind, everything is fine: the DRM connector is created by 
panel_bridge_attach() and then that connector is registered to userspace 
by the drm_modeset_register_all() in the last steps of initializing the 
compound DRM device.

However, when DSI driver is not yet registered during the DRM component 
bind, the DRM device finishes registration without any connector 
('exynos-drm exynos-drm: [drm] Cannot find any crtc or sizes' message). 
Then, when DSI driver gets registered, the connector is created by 
panel_brige_attach(), but there is no code, which would call 
drm_connector_register() to make it available for userspace.

Exactly the same issue has been earlier fixed by the commit deee3284cba3 
("drm/exynos/dsi: register connector if it is created after drm bind").

The following patch fixes this with the current code:

diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c
index ff1c37b2e6e5..2165f38989f1 100644
--- a/drivers/gpu/drm/bridge/panel.c
+++ b/drivers/gpu/drm/bridge/panel.c
@@ -86,6 +86,9 @@ static int panel_bridge_attach(struct drm_bridge *bridge,
         if (connector->funcs->reset)
                 connector->funcs->reset(connector);

+       if (bridge->dev->registered)
+               drm_connector_register(connector);
+
         return 0;
  }

If this is okay, I will send it as a proper patch, tagged as a fix for 
934aef885f9d ("drm: bridge: panel: Reset the connector state pointer").

> modes:
>         name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot)
>   720x1280 60 720 725 730 735 1280 1293 1295 1296 57153 flags: ; type: 
> preferred, driver
>   props:
>         1 EDID:
>                 flags: immutable blob
>                 blobs:
>
>                 value:
>         2 DPMS:
>                 flags: enum
>                 enums: On=0 Standby=1 Suspend=2 Off=3
>                 value: 0
>         5 link-status:
>                 flags: enum
>                 enums: Good=0 Bad=1
>                 value: 0
>         6 non-desktop:
>                 flags: immutable range
>                 values: 0 1
>                 value: 0
>         4 TILE:
>                 flags: immutable blob
>                 blobs:
>
>                 value:
>         20 CRTC_ID:
>                 flags: object
>                 value: 54
> 73      0       connected       HDMI-A-1        0x0 0       72
>   props:
>         1 EDID:
>                 flags: immutable blob
>                 blobs:
>
>                 value:
>         2 DPMS:
>                 flags: enum
>                 enums: On=0 Standby=1 Suspend=2 Off=3
>                 value: 0
>         5 link-status:
>                 flags: enum
>                 enums: Good=0 Bad=1
>                 value: 0
>         6 non-desktop:
>                 flags: immutable range
>                 values: 0 1
>                 value: 0
>         4 TILE:
>                 flags: immutable blob
>                 blobs:
>
>                 value:
>         20 CRTC_ID:
>                 flags: object
>                 value: 0
>
> (the only difference between Trats and Trats2 is the fact that Trats2 
> has also HDMI output implemented).
>
> It looks that something is missing in the connector initialization, 
> but I didn't dig enough into it. The emulated framebuffer is properly 
> registered and displayed on the panel.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland