[PATCH v6 0/6] drm: sun4i: dsi: Bridge support

104 views
Skip to first unread message

Jagan Teki

unread,
Dec 10, 2021, 6:17:28 AM12/10/21
to Maxime Ripard, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com, Jagan Teki
This series add bridge support for Allwinner DSI controller.

In addition to previous version, this series add bridge support
only not doing any bridge conversion at the moment.

Previous version changes [1].

Patch 1: Drop the DRM bind race while attaching bridges

Patch 2: Move component_add into sun6i_dsi_attach

Patch 3: Add Bridge driver

Patch 4: Add mode_set API

Patch 5: Enable DSI Panel

Patch 6: Enable DSI Bridge

Patch 7: Enable DSI Bridge (I2C)

[1] https://patchwork.kernel.org/project/dri-devel/cover/20211122065223...@amarulasolutions.com/

Any inputs?
Jagan.

Jagan Teki (6):
drm: sun4i: dsi: Drop DRM bind race with bridge attach
drm: sun4i: dsi: Add component only once DSI device attached
drm: sun4i: dsi: Add bridge support
[DO NOT MERGE] ARM: dts: sun8i: bananapi-m2m: Enable iS070WV20-CT16 DSI Panel
[DO NOT MERGE] ARM: dts: sun8i: bananapi-m2m: Enable ICN6211 DSI Bridge
[DO NOT MERGE] ARM: dts: sun8i: bananapi-m2m: Enable DLPC3433 Bridge (I2C)

.../boot/dts/sun8i-r16-bananapi-m2m-i2c.dts | 69 +++++++
.../boot/dts/sun8i-r16-bananapi-m2m-panel.dts | 41 ++++
arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts | 64 ++++++
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 187 ++++++++----------
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 10 +-
5 files changed, 256 insertions(+), 115 deletions(-)

--
2.25.1

Jagan Teki

unread,
Dec 10, 2021, 6:17:30 AM12/10/21
to Maxime Ripard, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com, Jagan Teki
Existing host driver will keep looking for DRM pointer in
sun6i_dsi_attach and defers even if the particular DSI device
is found for the first time. Meanwhile it triggers the bind
callback and gets the DRM pointer and then continues the
sun6i_dsi_attach.

This makes a deadlock situation if sun6i_dsi_attach is trying
to find the bridge.

If interface bridge is trying to call host attach, then host
sun6i_dsi_attach is trying to find bridge and defers the
interface bridge even if it found the bridge as bind callback
does not complete at the movement. So, this sun6i_dsi_attach
defers interface bridge and triggers the bind callback and
tries to attach the bridge with a bridge pointer which is not
available at the moment.

Eventually these callbacks are triggered recursively, as
sun6i_dsi_attach defers interface bridge and bind callback
defers sun6i_dsi_attach due to invalid bridge ponter.

This patch prevents this situation by probing all DSI devices
on the pipeline first and then triggers the bind callback by
dropping exing DRM binding logic.

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

drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 10 +---------
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 1 -
2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 527c7b2474da..4bdcce8f1d84 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -967,14 +967,10 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,

if (IS_ERR(panel))
return PTR_ERR(panel);
- if (!dsi->drm || !dsi->drm->registered)
- return -EPROBE_DEFER;

dsi->panel = panel;
dsi->device = device;

- drm_kms_helper_hotplug_event(dsi->drm);
-
dev_info(host->dev, "Attached device %s\n", device->name);

return 0;
@@ -988,8 +984,6 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host,
dsi->panel = NULL;
dsi->device = NULL;

- drm_kms_helper_hotplug_event(dsi->drm);
-
return 0;
}

@@ -1077,8 +1071,6 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,

drm_connector_attach_encoder(&dsi->connector, &dsi->encoder);

- dsi->drm = drm;
-
return 0;

err_cleanup_connector:
@@ -1091,7 +1083,7 @@ static void sun6i_dsi_unbind(struct device *dev, struct device *master,
{
struct sun6i_dsi *dsi = dev_get_drvdata(dev);

- dsi->drm = NULL;
+ drm_encoder_cleanup(&dsi->encoder);
}

static const struct component_ops sun6i_dsi_ops = {
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index c863900ae3b4..61e88ea6044d 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -29,7 +29,6 @@ struct sun6i_dsi {

struct device *dev;
struct mipi_dsi_device *device;
- struct drm_device *drm;
struct drm_panel *panel;
};

--
2.25.1

Jagan Teki

unread,
Dec 10, 2021, 6:17:39 AM12/10/21
to Maxime Ripard, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com, Jagan Teki
Having component_add for running all drm bind callbacks returns
error or unbound due to chain of DSI devices connected across
bridge topology on a display pipeline.

In a typical bridge oriented display pipeline where the host is
connected to the bridge converter and that indeed connected to
a panel.

DRM => SUN6I DSI Host => Chipone ICN6211 => BananaPi Panel

The bridge converter is looking for a panel to probe first and
then attach the host. The host attach is looking for a bridge
converter to probe and preserve bridge pointer, at this movement
the host is trying to bind the all callbacks and one of the bind
callback in the DSI host is trying to find the bridge using the
bridge pointer in sun6i_dsi_attach call.

chipone_probe().start
drm_of_find_panel_or_bridge
mipi_dsi_attach
sun6i_dsi_attach
drm_of_find_panel_or_bridge
chipone_probe().done

sun6i_dsi_probe().start
mipi_dsi_host_register
component_add
sun6i_dsi_probe().done

However, the movement when panel defers the probe, will make the
bridge converter defer the host attach call which eventually found
a NULL bridge pointer during DSI component bind callback.

So, in order to prevent this scenario of binding invalid bridge,
wait for DSI devices on the pipeline to probe first and start the
binding process by moving component_add in host probe to attach call.

chipone_probe().start
drm_of_find_panel_or_bridge
mipi_dsi_attach
sun6i_dsi_attach
drm_of_find_panel_or_bridge
component_add
chipone_probe().done

sun6i_dsi_probe().start
mipi_dsi_host_register
sun6i_dsi_probe().done

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

drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 120 +++++++++++++------------
1 file changed, 61 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 4bdcce8f1d84..9cf91dcac3f2 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -959,11 +959,63 @@ static int sun6i_dsi_dcs_read(struct sun6i_dsi *dsi,
return 1;
}

+static int sun6i_dsi_bind(struct device *dev, struct device *master,
+ void *data)
+{
+ struct drm_device *drm = data;
+ struct sun6i_dsi *dsi = dev_get_drvdata(dev);
+ int ret;
+
+ drm_encoder_helper_add(&dsi->encoder,
+ &sun6i_dsi_enc_helper_funcs);
+ ret = drm_simple_encoder_init(drm, &dsi->encoder,
+ DRM_MODE_ENCODER_DSI);
+ if (ret) {
+ dev_err(dsi->dev, "Couldn't initialise the DSI encoder\n");
+ return ret;
+ }
+ dsi->encoder.possible_crtcs = BIT(0);
+
+ drm_connector_helper_add(&dsi->connector,
+ &sun6i_dsi_connector_helper_funcs);
+ ret = drm_connector_init(drm, &dsi->connector,
+ &sun6i_dsi_connector_funcs,
+ DRM_MODE_CONNECTOR_DSI);
+ if (ret) {
+ dev_err(dsi->dev,
+ "Couldn't initialise the DSI connector\n");
+ goto err_cleanup_connector;
+ }
+
+ drm_connector_attach_encoder(&dsi->connector, &dsi->encoder);
+
+ return 0;
+
+err_cleanup_connector:
+ drm_encoder_cleanup(&dsi->encoder);
+ return ret;
+}
+
+static void sun6i_dsi_unbind(struct device *dev, struct device *master,
+ void *data)
+{
+ struct sun6i_dsi *dsi = dev_get_drvdata(dev);
+
+ drm_encoder_cleanup(&dsi->encoder);
+}
+
+static const struct component_ops sun6i_dsi_ops = {
+ .bind = sun6i_dsi_bind,
+ .unbind = sun6i_dsi_unbind,
+};
+
static int sun6i_dsi_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device *device)
{
struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
struct drm_panel *panel = of_drm_find_panel(device->dev.of_node);
+ struct device *dev = dsi->dev;
+ int ret;

if (IS_ERR(panel))
return PTR_ERR(panel);
@@ -973,6 +1025,13 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,

dev_info(host->dev, "Attached device %s\n", device->name);

+ ret = component_add(dev, &sun6i_dsi_ops);
+ if (ret) {
+ dev_err(dev, "Couldn't register our component\n");
+ mipi_dsi_host_unregister(&dsi->host);
+ return ret;
+ }
+
return 0;
}

@@ -984,6 +1043,8 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host,
dsi->panel = NULL;
dsi->device = NULL;

+ component_del(dsi->dev, &sun6i_dsi_ops);
+
return 0;
}

@@ -1041,56 +1102,6 @@ static const struct regmap_config sun6i_dsi_regmap_config = {
.name = "mipi-dsi",
};

-static int sun6i_dsi_bind(struct device *dev, struct device *master,
- void *data)
-{
- struct drm_device *drm = data;
- struct sun6i_dsi *dsi = dev_get_drvdata(dev);
- int ret;
-
- drm_encoder_helper_add(&dsi->encoder,
- &sun6i_dsi_enc_helper_funcs);
- ret = drm_simple_encoder_init(drm, &dsi->encoder,
- DRM_MODE_ENCODER_DSI);
- if (ret) {
- dev_err(dsi->dev, "Couldn't initialise the DSI encoder\n");
- return ret;
- }
- dsi->encoder.possible_crtcs = BIT(0);
-
- drm_connector_helper_add(&dsi->connector,
- &sun6i_dsi_connector_helper_funcs);
- ret = drm_connector_init(drm, &dsi->connector,
- &sun6i_dsi_connector_funcs,
- DRM_MODE_CONNECTOR_DSI);
- if (ret) {
- dev_err(dsi->dev,
- "Couldn't initialise the DSI connector\n");
- goto err_cleanup_connector;
- }
-
- drm_connector_attach_encoder(&dsi->connector, &dsi->encoder);
-
- return 0;
-
-err_cleanup_connector:
- drm_encoder_cleanup(&dsi->encoder);
- return ret;
-}
-
-static void sun6i_dsi_unbind(struct device *dev, struct device *master,
- void *data)
-{
- struct sun6i_dsi *dsi = dev_get_drvdata(dev);
-
- drm_encoder_cleanup(&dsi->encoder);
-}
-
-static const struct component_ops sun6i_dsi_ops = {
- .bind = sun6i_dsi_bind,
- .unbind = sun6i_dsi_unbind,
-};
-
static int sun6i_dsi_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1172,16 +1183,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
goto err_unprotect_clk;
}

- ret = component_add(&pdev->dev, &sun6i_dsi_ops);
- if (ret) {
- dev_err(dev, "Couldn't register our component\n");
- goto err_remove_dsi_host;
- }
-
return 0;

-err_remove_dsi_host:
- mipi_dsi_host_unregister(&dsi->host);
err_unprotect_clk:
clk_rate_exclusive_put(dsi->mod_clk);
err_attach_clk:
@@ -1195,7 +1198,6 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct sun6i_dsi *dsi = dev_get_drvdata(dev);

- component_del(&pdev->dev, &sun6i_dsi_ops);
mipi_dsi_host_unregister(&dsi->host);
clk_rate_exclusive_put(dsi->mod_clk);

--
2.25.1

Jagan Teki

unread,
Dec 10, 2021, 6:17:42 AM12/10/21
to Maxime Ripard, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com, Jagan Teki
Some display panels would come up with a non-DSI output, those
can have an option to connect the DSI host by means of interface
bridge converter.

This DSI to non-DSI interface bridge converter would require
DSI Host to handle drm bridge functionalities in order to
communicate interface bridge.

This patch adds support for bridge functionalities in Allwinner
DSI controller.

Supporting down-stream bridge makes few changes in the driver.

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

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

- It uses devm_drm_of_get_bridge for panel or bridge lookup. It uses
port 0 and endpoint 0 to support I2C-based bridges eventhough the
usual Allwinner DSI OF graph doesn't require this for panel and
non-I2C based downstream bridges.

Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
---
Changes for v6:
- support donwstream bridge
- drop bridge conversion
- devm_drm_of_get_bridge() require child lookup
https://patchwork.kernel.org/project/dri-devel/cover/20211207054747...@amarulasolutions.com/
Changes for v5:
- add atomic APIs
- find host and device variant DSI devices.
Changes for v4, v3:
- none

drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 83 ++++++++++----------------
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 9 +--
2 files changed, 33 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 9cf91dcac3f2..f1d612bf1a0b 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -21,6 +21,7 @@

#include <drm/drm_atomic_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>
@@ -720,6 +721,7 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
struct mipi_dsi_device *device = dsi->device;
union phy_configure_opts opts = { };
struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy;
+ struct drm_bridge *iter;
u16 delay;
int err;

@@ -769,8 +771,10 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
phy_configure(dsi->dphy, &opts);
phy_power_on(dsi->dphy);

- if (dsi->panel)
- drm_panel_prepare(dsi->panel);
+ list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
+ if (iter->funcs->pre_enable)
+ iter->funcs->pre_enable(iter);
+ }

/*
* FIXME: This should be moved after the switch to HS mode.
@@ -784,8 +788,10 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
* ordering on the panels I've tested it with, so I guess this
* will do for now, until that IP is better understood.
*/
- if (dsi->panel)
- drm_panel_enable(dsi->panel);
+ list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
+ if (iter->funcs->enable)
+ iter->funcs->enable(iter);
+ }

sun6i_dsi_start(dsi, DSI_START_HSC);

@@ -797,12 +803,16 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
{
struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
+ struct drm_bridge *iter;

DRM_DEBUG_DRIVER("Disabling DSI output\n");

- if (dsi->panel) {
- drm_panel_disable(dsi->panel);
- drm_panel_unprepare(dsi->panel);
+ list_for_each_entry(iter, &dsi->bridge_chain, chain_node) {
+ if (iter->funcs->disable)
+ iter->funcs->disable(iter);
+
+ if (iter->funcs->post_disable)
+ iter->funcs->post_disable(iter);
}

phy_power_off(dsi->dphy);
@@ -813,35 +823,6 @@ static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
regulator_disable(dsi->regulator);
}

-static int sun6i_dsi_get_modes(struct drm_connector *connector)
-{
- struct sun6i_dsi *dsi = connector_to_sun6i_dsi(connector);
-
- return drm_panel_get_modes(dsi->panel, connector);
-}
-
-static const struct drm_connector_helper_funcs sun6i_dsi_connector_helper_funcs = {
- .get_modes = sun6i_dsi_get_modes,
-};
-
-static enum drm_connector_status
-sun6i_dsi_connector_detect(struct drm_connector *connector, bool force)
-{
- struct sun6i_dsi *dsi = connector_to_sun6i_dsi(connector);
-
- return dsi->panel ? connector_status_connected :
- connector_status_disconnected;
-}
-
-static const struct drm_connector_funcs sun6i_dsi_connector_funcs = {
- .detect = sun6i_dsi_connector_detect,
- .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 const struct drm_encoder_helper_funcs sun6i_dsi_enc_helper_funcs = {
.disable = sun6i_dsi_encoder_disable,
.enable = sun6i_dsi_encoder_enable,
@@ -976,22 +957,17 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,
}
dsi->encoder.possible_crtcs = BIT(0);

- drm_connector_helper_add(&dsi->connector,
- &sun6i_dsi_connector_helper_funcs);
- ret = drm_connector_init(drm, &dsi->connector,
- &sun6i_dsi_connector_funcs,
- DRM_MODE_CONNECTOR_DSI);
+ ret = drm_bridge_attach(&dsi->encoder, dsi->next_bridge, NULL, 0);
if (ret) {
- dev_err(dsi->dev,
- "Couldn't initialise the DSI connector\n");
- goto err_cleanup_connector;
+ dev_err(dsi->dev, "Couldn't attach drm bridge\n");
+ goto err_cleanup_encoder;
}

- drm_connector_attach_encoder(&dsi->connector, &dsi->encoder);
+ list_splice_init(&dsi->encoder.bridge_chain, &dsi->bridge_chain);

return 0;

-err_cleanup_connector:
+err_cleanup_encoder:
drm_encoder_cleanup(&dsi->encoder);
return ret;
}
@@ -1013,17 +989,18 @@ static int sun6i_dsi_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device *device)
{
struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
- struct drm_panel *panel = of_drm_find_panel(device->dev.of_node);
struct device *dev = dsi->dev;
int ret;

- if (IS_ERR(panel))
- return PTR_ERR(panel);
+ dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0);
+ if (IS_ERR(dsi->next_bridge)) {
+ dev_err(dsi->dev, "Couldn't find the bridge\n");
+ return PTR_ERR(dsi->next_bridge);
+ }

- dsi->panel = panel;
dsi->device = device;

- dev_info(host->dev, "Attached device %s\n", device->name);
+ dev_info(host->dev, "Attached %s\n", device->name);

ret = component_add(dev, &sun6i_dsi_ops);
if (ret) {
@@ -1040,7 +1017,7 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host,
{
struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);

- dsi->panel = NULL;
+ dsi->next_bridge = NULL;
dsi->device = NULL;

component_del(dsi->dev, &sun6i_dsi_ops);
@@ -1118,6 +1095,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
dsi->host.ops = &sun6i_dsi_host_ops;
dsi->host.dev = dev;

+ INIT_LIST_HEAD(&dsi->bridge_chain);
+
if (of_device_is_compatible(dev->of_node,
"allwinner,sun6i-a31-mipi-dsi"))
bus_clk_name = "bus";
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index 61e88ea6044d..96a219b9c275 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -16,7 +16,6 @@
#define SUN6I_DSI_TCON_DIV 4

struct sun6i_dsi {
- struct drm_connector connector;
struct drm_encoder encoder;
struct mipi_dsi_host host;

@@ -29,7 +28,8 @@ struct sun6i_dsi {

struct device *dev;
struct mipi_dsi_device *device;
- struct drm_panel *panel;
+ struct drm_bridge *next_bridge;
+ struct list_head bridge_chain;
};

static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
@@ -37,11 +37,6 @@ static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
return container_of(host, struct sun6i_dsi, host);
};

-static inline struct sun6i_dsi *connector_to_sun6i_dsi(struct drm_connector *connector)
-{
- return container_of(connector, struct sun6i_dsi, connector);
-};
-
static inline struct sun6i_dsi *encoder_to_sun6i_dsi(const struct drm_encoder *encoder)
{
return container_of(encoder, struct sun6i_dsi, encoder);
--
2.25.1

Jagan Teki

unread,
Dec 10, 2021, 6:17:47 AM12/10/21
to Maxime Ripard, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com, Jagan Teki
Bananapi S070WV20-CT16 is a pure RGB output panel with ICN6211
DSI/RGB convertor bridge.

Enable support for it.

Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
---
.../boot/dts/sun8i-r16-bananapi-m2m-panel.dts | 41 +++++++++++++++++++
1 file changed, 41 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r16-bananapi-m2m-panel.dts b/arch/arm/boot/dts/sun8i-r16-bananapi-m2m-panel.dts
index bf5b5e2f6168..04392358b492 100644
--- a/arch/arm/boot/dts/sun8i-r16-bananapi-m2m-panel.dts
+++ b/arch/arm/boot/dts/sun8i-r16-bananapi-m2m-panel.dts
@@ -44,6 +44,7 @@
#include "sun8i-a33.dtsi"

#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pwm/pwm.h>

/ {
model = "BananaPi M2 Magic";
@@ -55,12 +56,21 @@ aliases {
i2c2 = &i2c2;
serial0 = &uart0;
serial1 = &uart1;
+ mmc0 = &mmc0;
};

chosen {
stdout-path = "serial0:115200n8";
};

+ backlight: backlight {
+ compatible = "pwm-backlight";
+ pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
+ brightness-levels = <1 2 4 8 16 32 64 128 255>;
+ default-brightness-level = <8>;
+ enable-gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* LCD-BL-EN: PL4 */
+ };
+
leds {
compatible = "gpio-leds";

@@ -122,6 +132,27 @@ &dai {
status = "okay";
};

+&de {
+ status = "okay";
+};
+
+&dphy {
+ status = "okay";
+};
+
+&dsi {
+ vcc-dsi-supply = <&reg_dcdc1>; /* VCC-DSI */
+ status = "okay";
+
+ panel@0 {
+ compatible = "bananapi,s070wv20-ct16-icn6211";
+ reg = <0>;
+ reset-gpios = <&r_pio 0 5 GPIO_ACTIVE_HIGH>; /* LCD-RST: PL5 */
+ enable-gpios = <&pio 1 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PB7 */
+ backlight = <&backlight>;
+ };
+};
+
&ehci0 {
status = "okay";
};
@@ -157,6 +188,12 @@ &ohci0 {
status = "okay";
};

+&pwm {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm0_pin>;
+ status = "okay";
+};
+
&r_rsb {
status = "okay";

@@ -269,6 +306,10 @@ &sound {
status = "okay";
};

+&tcon0 {
+ status = "okay";
+};
+
&uart0 {
pinctrl-names = "default";
pinctrl-0 = <&uart0_pb_pins>;
--
2.25.1

Jagan Teki

unread,
Dec 10, 2021, 6:17:50 AM12/10/21
to Maxime Ripard, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com, Jagan Teki
Bananapi S070WV20-CT16 is a pure RGB output panel with ICN6211
DSI/RGB convertor bridge.

Enable bridge along with associated panel.

Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
---
arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts | 64 ++++++++++++++++++++
1 file changed, 64 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts b/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts
index bf5b5e2f6168..501666dfb5ee 100644
--- a/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts
+++ b/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts
@@ -44,6 +44,7 @@
#include "sun8i-a33.dtsi"

#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pwm/pwm.h>

/ {
model = "BananaPi M2 Magic";
@@ -55,12 +56,21 @@ aliases {
i2c2 = &i2c2;
serial0 = &uart0;
serial1 = &uart1;
+ mmc0 = &mmc0;
};

chosen {
stdout-path = "serial0:115200n8";
};

+ backlight: backlight {
+ compatible = "pwm-backlight";
+ pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
+ brightness-levels = <1 2 4 8 16 32 64 128 255>;
+ default-brightness-level = <8>;
+ enable-gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* LCD-BL-EN: PL4 */
+ };
+
leds {
compatible = "gpio-leds";

@@ -81,6 +91,18 @@ led-2 {
};
};

+ panel {
+ compatible = "bananapi,s070wv20-ct16";
+ enable-gpios = <&pio 1 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PB7 */
+ backlight = <&backlight>;
+
+ port {
+ panel_out_bridge: endpoint {
+ remote-endpoint = <&bridge_out_panel>;
+ };
+ };
+ };
+
reg_vcc5v0: vcc5v0 {
compatible = "regulator-fixed";
regulator-name = "vcc5v0";
@@ -122,6 +144,38 @@ &dai {
status = "okay";
};

+&de {
+ status = "okay";
+};
+
+&dphy {
+ status = "okay";
+};
+
+&dsi {
+ vcc-dsi-supply = <&reg_dcdc1>; /* VCC-DSI */
+ status = "okay";
+
+ bridge@0 {
+ compatible = "chipone,icn6211";
+ reg = <0>;
+ enable-gpios = <&r_pio 0 5 GPIO_ACTIVE_HIGH>; /* LCD-RST: PL5 */
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ bridge_out: port@1 {
+ reg = <1>;
+
+ bridge_out_panel: endpoint {
+ remote-endpoint = <&panel_out_bridge>;
+ };
+ };
+ };
+ };
+};
+
&ehci0 {
status = "okay";
};
@@ -157,6 +211,12 @@ &ohci0 {
status = "okay";
};

+&pwm {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm0_pin>;
+ status = "okay";
+};
+
&r_rsb {
status = "okay";

@@ -269,6 +329,10 @@ &sound {

Jagan Teki

unread,
Dec 10, 2021, 6:17:54 AM12/10/21
to Maxime Ripard, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com, Jagan Teki
R16 board has DLCP3433 DSI bridge for connecting TI DMD Projectors.

Enable support for it.

Note: This is for testing purpose, DLPC3433 not available in BPI-M2M.

Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
---
.../boot/dts/sun8i-r16-bananapi-m2m-i2c.dts | 69 +++++++++++++++++++
1 file changed, 69 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r16-bananapi-m2m-i2c.dts b/arch/arm/boot/dts/sun8i-r16-bananapi-m2m-i2c.dts
index 7713cdaf6211..a96fbfd743f7 100644
--- a/arch/arm/boot/dts/sun8i-r16-bananapi-m2m-i2c.dts
+++ b/arch/arm/boot/dts/sun8i-r16-bananapi-m2m-i2c.dts
@@ -86,6 +86,16 @@ vol-down {
};
};

+ panel {
+ compatible = "panel-simple";
+
+ port {
+ panel_out_bridge: endpoint {
+ remote-endpoint = <&bridge_out_panel>;
+ };
+ };
+ };
+
reg_vcc5v0: vcc5v0 {
compatible = "regulator-fixed";
regulator-name = "vcc5v0";
@@ -125,6 +135,32 @@ &dai {
status = "okay";
};

+&de {
+ status = "okay";
+};
+
+&dphy {
+ status = "okay";
+};
+
+&dsi {
+ vcc-dsi-supply = <&reg_dcdc1>; /* VCC-DSI */
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ dsi_out_bridge: endpoint {
+ remote-endpoint = <&bridge_out_dsi>;
+ };
+ };
+ };
+};
+
&ehci0 {
status = "okay";
};
@@ -132,6 +168,35 @@ &ehci0 {
&i2c1 {
clock-frequency = <100000>;
status = "okay";
+
+ bridge@1d {
+ compatible = "ti,dlpc3433";
+ reg = <0x1d>;
+ enable-gpios = <&pio 4 15 GPIO_ACTIVE_HIGH>;
+ vcc_intf-supply = <&reg_dldo1>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ bridge_in: port@0 {
+ reg = <0>;
+
+ bridge_out_dsi: endpoint {
+ remote-endpoint = <&dsi_out_bridge>;
+ data-lanes = <0 1 2 3>;
+ };
+ };
+
+ bridge_out: port@1 {
+ reg = <1>;
+
+ bridge_out_panel: endpoint {
+ remote-endpoint = <&panel_out_bridge>;
+ };
+ };
+ };
+ };
};

&i2c2 {
@@ -289,6 +354,10 @@ &sound {
"Right DAC", "AIF1 Slot 0 Right";

Maxime Ripard

unread,
Dec 13, 2021, 12:00:56 PM12/13/21
to Jagan Teki, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com
Hi,

On Fri, Dec 10, 2021 at 04:47:05PM +0530, Jagan Teki wrote:
> This series add bridge support for Allwinner DSI controller.
>
> In addition to previous version, this series add bridge support
> only not doing any bridge conversion at the moment.
>
> Previous version changes [1].
>
> Patch 1: Drop the DRM bind race while attaching bridges
>
> Patch 2: Move component_add into sun6i_dsi_attach
>
> Patch 3: Add Bridge driver
>
> Patch 4: Add mode_set API
>
> Patch 5: Enable DSI Panel
>
> Patch 6: Enable DSI Bridge
>
> Patch 7: Enable DSI Bridge (I2C)

It would be worth mentioning that we can't apply this patch until the
one for drm_of_find_panel_or_bridge has landed? Otherwise, we're going
to break every user so far.

Maxime
signature.asc

Maxime Ripard

unread,
Dec 13, 2021, 12:02:28 PM12/13/21
to Jagan Teki, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com
On Fri, Dec 10, 2021 at 04:47:06PM +0530, Jagan Teki wrote:
> Existing host driver will keep looking for DRM pointer in
> sun6i_dsi_attach and defers even if the particular DSI device
> is found for the first time. Meanwhile it triggers the bind
> callback and gets the DRM pointer and then continues the
> sun6i_dsi_attach.
>
> This makes a deadlock situation if sun6i_dsi_attach is trying
> to find the bridge.

I'm not sure what you mean by deadlock here, there's no lock involved?

> If interface bridge is trying to call host attach, then host
> sun6i_dsi_attach is trying to find bridge and defers the
> interface bridge even if it found the bridge as bind callback
> does not complete at the movement. So, this sun6i_dsi_attach
> defers interface bridge and triggers the bind callback and
> tries to attach the bridge with a bridge pointer which is not
> available at the moment.
>
> Eventually these callbacks are triggered recursively, as
> sun6i_dsi_attach defers interface bridge and bind callback
> defers sun6i_dsi_attach due to invalid bridge ponter.

^ pointer

>
> This patch prevents this situation by probing all DSI devices
> on the pipeline first and then triggers the bind callback by
> dropping exing DRM binding logic.

^ existing I guess?
signature.asc

Maxime Ripard

unread,
Dec 13, 2021, 12:05:20 PM12/13/21
to Jagan Teki, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com
On Fri, Dec 10, 2021 at 04:47:07PM +0530, Jagan Teki wrote:
> Having component_add for running all drm bind callbacks returns
> error or unbound due to chain of DSI devices connected across
> bridge topology on a display pipeline.

I'm not sure what that means?

> In a typical bridge oriented display pipeline where the host is
> connected to the bridge converter and that indeed connected to
> a panel.
>
> DRM => SUN6I DSI Host => Chipone ICN6211 => BananaPi Panel
>
> The bridge converter is looking for a panel to probe first and
> then attach the host. The host attach is looking for a bridge
> converter to probe and preserve bridge pointer, at this movement

^ moment ?
Just use a forward declaration there, it will make the patch more
straightforward.

Maxime
signature.asc

Maxime Ripard

unread,
Dec 13, 2021, 12:18:03 PM12/13/21
to Jagan Teki, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com
Like we discussed in the previous version already, this is unnecessary,
just like the poking at bridge_chain in the encoder.

Maxime
signature.asc

Jagan Teki

unread,
Jan 17, 2022, 10:38:43 AM1/17/22
to Maxime Ripard, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com
Unlike previous patch, this patch not doing bridge conversion it is
supporting downstream bridge. Yes, it is possible to use bridge helper
for invoking downstream bridge enable when bridge functions added.

Thanks,
Jagan.

Jagan Teki

unread,
Jan 17, 2022, 10:42:51 AM1/17/22
to Maxime Ripard, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com
On Mon, Dec 13, 2021 at 10:32 PM Maxime Ripard <max...@cerno.tech> wrote:
>
> On Fri, Dec 10, 2021 at 04:47:06PM +0530, Jagan Teki wrote:
> > Existing host driver will keep looking for DRM pointer in
> > sun6i_dsi_attach and defers even if the particular DSI device
> > is found for the first time. Meanwhile it triggers the bind
> > callback and gets the DRM pointer and then continues the
> > sun6i_dsi_attach.
> >
> > This makes a deadlock situation if sun6i_dsi_attach is trying
> > to find the bridge.
>
> I'm not sure what you mean by deadlock here, there's no lock involved?

deadlock parse here for general understanding, where bind is trying to
attach bridge but drm pointer is not available that point and drm
pointer will available only when bind done. This is what I'm calling
as deadlock here.

Anyway, now I'm able to support both panel and bridge to support
hotplug so no need to drop the hotplug support.

Please let me know, if you have any questions so-that I can send next
version series.

Thanks,
Jagan.

Maxime Ripard

unread,
Feb 4, 2022, 1:20:12 PM2/4/22
to Jagan Teki, Chen-Yu Tsai, Laurent Pinchart, Neil Armstrong, Robert Foss, Sam Ravnborg, dri-...@lists.freedesktop.org, linux-ar...@lists.infradead.org, linux...@googlegroups.com, linux-...@amarulasolutions.com
On Mon, Jan 17, 2022 at 09:12:37PM +0530, Jagan Teki wrote:
> On Mon, Dec 13, 2021 at 10:32 PM Maxime Ripard <max...@cerno.tech> wrote:
> >
> > On Fri, Dec 10, 2021 at 04:47:06PM +0530, Jagan Teki wrote:
> > > Existing host driver will keep looking for DRM pointer in
> > > sun6i_dsi_attach and defers even if the particular DSI device
> > > is found for the first time. Meanwhile it triggers the bind
> > > callback and gets the DRM pointer and then continues the
> > > sun6i_dsi_attach.
> > >
> > > This makes a deadlock situation if sun6i_dsi_attach is trying
> > > to find the bridge.
> >
> > I'm not sure what you mean by deadlock here, there's no lock involved?
>
> deadlock parse here for general understanding, where bind is trying to
> attach bridge but drm pointer is not available that point and drm
> pointer will available only when bind done. This is what I'm calling
> as deadlock here.

Have you read:
https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html#special-care-with-mipi-dsi-bridges

Maxime
signature.asc
Reply all
Reply to author
Forward
0 new messages