* [PATCH v4 0/4] drm: sun4i: dsi: Convert drm bridge
@ 2021-03-22 14:01 Jagan Teki
2021-03-22 14:01 ` [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge Jagan Teki
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Jagan Teki @ 2021-03-22 14:01 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, Laurent Pinchart,
Samuel Holland
Cc: dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi, Jagan Teki
This series convert Allwinner DSI controller to full functional
drm bridge driver for supporting slave panel, bridges.
Here, are the previous version changes[1].
Patch 1: use drm_of_find_panel_or_bridge API
Patch 2: Adding DRM Bridge support
Patch 3: Convert to bridge driver, that indeed drop
encoder API's and support bridge API's
Patch 4: Overlay patch for bridge enablement in BPI-M2M
Note: Only nit on this series is kms hotplug, added Samuel Holland
for reviews and comments as he is authorized the code before.
[1] https://lkml.org/lkml/2021/2/14/173
Any inputs on this would be appreciated!
Jagan.
Jagan Teki (4):
drm: sun4i: dsi: Use drm_of_find_panel_or_bridge
drm: sun4i: dsi: Add bridge support
drm: sun4i: dsi: Convert to bridge driver
[DO NOT MERGE] ARM: dts: sun8i: bananapi-m2m: Enable S070WV20-CT16 panel
arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts | 85 ++++++++++++++++
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 100 +++++++++++++------
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 8 +-
3 files changed, 160 insertions(+), 33 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge
2021-03-22 14:01 [PATCH v4 0/4] drm: sun4i: dsi: Convert drm bridge Jagan Teki
@ 2021-03-22 14:01 ` Jagan Teki
2021-03-23 22:53 ` Laurent Pinchart
2021-03-22 14:01 ` [PATCH v4 2/4] drm: sun4i: dsi: Add bridge support Jagan Teki
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Jagan Teki @ 2021-03-22 14:01 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, Laurent Pinchart,
Samuel Holland
Cc: dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi, Jagan Teki
Replace of_drm_find_panel with drm_of_find_panel_or_bridge
for finding panel, this indeed help to find the bridge if
bridge support added.
Added NULL in bridge argument, same will replace with bridge
parameter once bridge supported.
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v4, v3:
- none
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 4f5efcace68e..2e9e7b2d4145 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>
@@ -963,10 +964,14 @@ 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 drm_panel *panel;
+ int ret;
+
+ ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
+ &panel, NULL);
+ if (ret)
+ return ret;
- if (IS_ERR(panel))
- return PTR_ERR(panel);
if (!dsi->drm || !dsi->drm->registered)
return -EPROBE_DEFER;
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v4 2/4] drm: sun4i: dsi: Add bridge support
2021-03-22 14:01 [PATCH v4 0/4] drm: sun4i: dsi: Convert drm bridge Jagan Teki
2021-03-22 14:01 ` [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge Jagan Teki
@ 2021-03-22 14:01 ` Jagan Teki
2021-03-24 1:10 ` Laurent Pinchart
2021-03-24 3:01 ` Samuel Holland
2021-03-22 14:01 ` [PATCH v4 3/4] drm: sun4i: dsi: Convert to bridge driver Jagan Teki
2021-03-22 14:01 ` [DO NOT MERGE] [PATCH v4 4/4] ARM: dts: sun8i: bananapi-m2m: Enable S070WV20-CT16 panel Jagan Teki
3 siblings, 2 replies; 15+ messages in thread
From: Jagan Teki @ 2021-03-22 14:01 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, Laurent Pinchart,
Samuel Holland
Cc: dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi, Jagan Teki
Some display panels would come up with a non-DSI output which
can have an option to connect DSI interface by means of bridge
converter.
This DSI to non-DSI bridge converter would require a bridge
driver that would communicate the DSI controller for bridge
functionalities.
So, add support for bridge functionalities in Allwinner DSI
controller.
Cc: Samuel Holland <samuel@sholland.org>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Note:
Samuel Holland, The existing kms hotplug dropped in order to
attach the bridge properly.
However, I did try several ways to support hotplug with the
bridge but it's resulting in a deadlock where bind never attach
bridge until bridge pointer found and bridge pointer cannot
found until bind finishes. Any inputs on this would be appreciated.
Changes for v4:
- none
Changes for v3:
- updated with new API's
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 34 +++++++++++++++++---------
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 +-
2 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 2e9e7b2d4145..39321299dc27 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -773,6 +773,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
if (dsi->panel)
drm_panel_prepare(dsi->panel);
+ if (dsi->panel_bridge)
+ dsi->panel_bridge->funcs->pre_enable(dsi->panel_bridge);
+
/*
* FIXME: This should be moved after the switch to HS mode.
*
@@ -788,6 +791,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
if (dsi->panel)
drm_panel_enable(dsi->panel);
+ if (dsi->panel_bridge)
+ dsi->panel_bridge->funcs->enable(dsi->panel_bridge);
+
sun6i_dsi_start(dsi, DSI_START_HSC);
udelay(1000);
@@ -804,6 +810,9 @@ static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
if (dsi->panel) {
drm_panel_disable(dsi->panel);
drm_panel_unprepare(dsi->panel);
+ } else if (dsi->panel_bridge) {
+ dsi->panel_bridge->funcs->disable(dsi->panel_bridge);
+ dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge);
}
phy_power_off(dsi->dphy);
@@ -964,23 +973,17 @@ 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;
int ret;
ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
- &panel, NULL);
+ &dsi->panel, &dsi->panel_bridge);
if (ret)
return ret;
- 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);
+ dev_info(host->dev, "Attached %s %s\n",
+ device->name, dsi->panel ? "panel" : "bridge");
return 0;
}
@@ -991,9 +994,10 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host,
struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
dsi->panel = NULL;
+ dsi->panel_bridge = NULL;
dsi->device = NULL;
- drm_kms_helper_hotplug_event(dsi->drm);
+ drm_of_panel_bridge_remove(dsi->dev->of_node, 0, 0);
return 0;
}
@@ -1082,7 +1086,13 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,
drm_connector_attach_encoder(&dsi->connector, &dsi->encoder);
- dsi->drm = drm;
+ if (dsi->panel_bridge) {
+ ret = drm_bridge_attach(&dsi->encoder, dsi->panel_bridge, NULL, 0);
+ if (ret) {
+ dev_err(dsi->dev, "Couldn't attach drm bridge\n");
+ goto err_cleanup_connector;
+ }
+ }
return 0;
@@ -1096,7 +1106,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..370ecb356a63 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -29,8 +29,8 @@ struct sun6i_dsi {
struct device *dev;
struct mipi_dsi_device *device;
- struct drm_device *drm;
struct drm_panel *panel;
+ struct drm_bridge *panel_bridge;
};
static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v4 3/4] drm: sun4i: dsi: Convert to bridge driver
2021-03-22 14:01 [PATCH v4 0/4] drm: sun4i: dsi: Convert drm bridge Jagan Teki
2021-03-22 14:01 ` [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge Jagan Teki
2021-03-22 14:01 ` [PATCH v4 2/4] drm: sun4i: dsi: Add bridge support Jagan Teki
@ 2021-03-22 14:01 ` Jagan Teki
2021-03-24 1:11 ` Laurent Pinchart
2021-03-22 14:01 ` [DO NOT MERGE] [PATCH v4 4/4] ARM: dts: sun8i: bananapi-m2m: Enable S070WV20-CT16 panel Jagan Teki
3 siblings, 1 reply; 15+ messages in thread
From: Jagan Teki @ 2021-03-22 14:01 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, Laurent Pinchart,
Samuel Holland
Cc: dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi, Jagan Teki
DRM bridge drivers have build-in handling of treating all display
pipeline components as bridges.
So, convert the existing to a drm bridge driver with a built-in
encoder support for compatibility with existing component drivers.
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v4:
- none
Changes for v3:
- new patch
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 75 ++++++++++++++++----------
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 6 +++
2 files changed, 54 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 39321299dc27..6f3c5330a468 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -714,10 +714,10 @@ static int sun6i_dsi_start(struct sun6i_dsi *dsi,
return 0;
}
-static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
+static void sun6i_dsi_bridge_enable(struct drm_bridge *bridge)
{
- struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
- struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
+ struct drm_display_mode *mode = &bridge->encoder->crtc->state->adjusted_mode;
+ struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
struct mipi_dsi_device *device = dsi->device;
union phy_configure_opts opts = { };
struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy;
@@ -801,9 +801,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
sun6i_dsi_start(dsi, DSI_START_HSD);
}
-static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
+static void sun6i_dsi_bridge_disable(struct drm_bridge *bridge)
{
- struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
+ struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
DRM_DEBUG_DRIVER("Disabling DSI output\n");
@@ -852,9 +852,40 @@ static const struct drm_connector_funcs sun6i_dsi_connector_funcs = {
.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,
+static int sun6i_dsi_bridge_attach(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
+ int ret;
+
+ if (dsi->panel_bridge)
+ return drm_bridge_attach(bridge->encoder, dsi->panel_bridge, NULL, 0);
+
+ if (dsi->panel) {
+ drm_connector_helper_add(&dsi->connector,
+ &sun6i_dsi_connector_helper_funcs);
+ ret = drm_connector_init(bridge->dev, &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 const struct drm_bridge_funcs sun6i_dsi_bridge_funcs = {
+ .enable = sun6i_dsi_bridge_enable,
+ .disable = sun6i_dsi_bridge_disable,
+ .attach = sun6i_dsi_bridge_attach,
};
static u32 sun6i_dsi_dcs_build_pkt_hdr(struct sun6i_dsi *dsi,
@@ -1063,8 +1094,6 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,
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) {
@@ -1073,27 +1102,12 @@ 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->bridge, NULL, 0);
if (ret) {
- dev_err(dsi->dev,
- "Couldn't initialise the DSI connector\n");
+ dev_err(dsi->dev, "Couldn't attach drm bridge\n");
goto err_cleanup_connector;
}
- drm_connector_attach_encoder(&dsi->connector, &dsi->encoder);
-
- if (dsi->panel_bridge) {
- ret = drm_bridge_attach(&dsi->encoder, dsi->panel_bridge, NULL, 0);
- if (ret) {
- dev_err(dsi->dev, "Couldn't attach drm bridge\n");
- goto err_cleanup_connector;
- }
- }
-
return 0;
err_cleanup_connector:
@@ -1199,6 +1213,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
goto err_unprotect_clk;
}
+ dsi->bridge.funcs = &sun6i_dsi_bridge_funcs;
+ dsi->bridge.of_node = dev->of_node;
+ dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
+
+ drm_bridge_add(&dsi->bridge);
+
ret = component_add(&pdev->dev, &sun6i_dsi_ops);
if (ret) {
dev_err(dev, "Couldn't register our component\n");
@@ -1222,6 +1242,7 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct sun6i_dsi *dsi = dev_get_drvdata(dev);
+ drm_bridge_remove(&dsi->bridge);
component_del(&pdev->dev, &sun6i_dsi_ops);
mipi_dsi_host_unregister(&dsi->host);
clk_rate_exclusive_put(dsi->mod_clk);
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index 370ecb356a63..5e70666089ad 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -16,6 +16,7 @@
#define SUN6I_DSI_TCON_DIV 4
struct sun6i_dsi {
+ struct drm_bridge bridge;
struct drm_connector connector;
struct drm_encoder encoder;
struct mipi_dsi_host host;
@@ -38,6 +39,11 @@ 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 *bridge_to_sun6i_dsi(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct sun6i_dsi, bridge);
+}
+
static inline struct sun6i_dsi *connector_to_sun6i_dsi(struct drm_connector *connector)
{
return container_of(connector, struct sun6i_dsi, connector);
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [DO NOT MERGE] [PATCH v4 4/4] ARM: dts: sun8i: bananapi-m2m: Enable S070WV20-CT16 panel
2021-03-22 14:01 [PATCH v4 0/4] drm: sun4i: dsi: Convert drm bridge Jagan Teki
` (2 preceding siblings ...)
2021-03-22 14:01 ` [PATCH v4 3/4] drm: sun4i: dsi: Convert to bridge driver Jagan Teki
@ 2021-03-22 14:01 ` Jagan Teki
3 siblings, 0 replies; 15+ messages in thread
From: Jagan Teki @ 2021-03-22 14:01 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, Laurent Pinchart,
Samuel Holland
Cc: dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi, Jagan Teki
This patch add support for Bananapi S070WV20-CT16 panel to
BPI-M2M board.
Bananapi S070WV20-CT16 is a pure RGB output panel with ICN6211 DSI/RGB
converter bridge, so enable bridge along with associated panel.
DSI panel connected via board DSI port with,
- DCDC1 as VCC-DSI supply
- PL5 gpio for bridge enable gpio pin
- PB7 gpio for lcd enable gpio pin
- PL4 gpio for backlight enable pin
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
Changes for v4:
- replace reset with enable-gpios
Changes for v3:
- none
arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts | 85 ++++++++++++++++++++
1 file changed, 85 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts b/arch/arm/boot/dts/sun8i-r16-bananapi-m2m.dts
index 293016d081cd..6f33e1ae8ffc 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,59 @@ &dai {
status = "okay";
};
+&de {
+ status = "okay";
+};
+
+&dphy {
+ status = "okay";
+};
+
+&dsi {
+ vcc-dsi-supply = <®_dcdc1>; /* VCC-DSI */
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dsi_out: port@0 {
+ reg = <0>;
+
+ dsi_out_bridge: endpoint {
+ remote-endpoint = <&bridge_out_dsi>;
+ };
+ };
+ };
+
+ 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_in: port@0 {
+ reg = <0>;
+
+ bridge_out_dsi: endpoint {
+ remote-endpoint = <&dsi_out_bridge>;
+ };
+ };
+
+ bridge_out: port@1 {
+ reg = <1>;
+
+ bridge_out_panel: endpoint {
+ remote-endpoint = <&panel_out_bridge>;
+ };
+ };
+ };
+ };
+};
+
&ehci0 {
status = "okay";
};
@@ -157,6 +232,12 @@ &ohci0 {
status = "okay";
};
+&pwm {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm0_pin>;
+ status = "okay";
+};
+
&r_rsb {
status = "okay";
@@ -269,6 +350,10 @@ &sound {
status = "okay";
};
+&tcon0 {
+ status = "okay";
+};
+
&uart0 {
pinctrl-names = "default";
pinctrl-0 = <&uart0_pb_pins>;
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge
2021-03-22 14:01 ` [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge Jagan Teki
@ 2021-03-23 22:53 ` Laurent Pinchart
2021-03-24 2:48 ` Samuel Holland
0 siblings, 1 reply; 15+ messages in thread
From: Laurent Pinchart @ 2021-03-23 22:53 UTC (permalink / raw)
To: Jagan Teki
Cc: Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi
Hi Jagan,
Thank you for the patch.
On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
> for finding panel, this indeed help to find the bridge if
> bridge support added.
>
> Added NULL in bridge argument, same will replace with bridge
> parameter once bridge supported.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Looks good, there should be no functional change.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes for v4, v3:
> - none
>
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 4f5efcace68e..2e9e7b2d4145 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>
> @@ -963,10 +964,14 @@ 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 drm_panel *panel;
> + int ret;
> +
> + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> + &panel, NULL);
> + if (ret)
> + return ret;
>
> - if (IS_ERR(panel))
> - return PTR_ERR(panel);
> if (!dsi->drm || !dsi->drm->registered)
> return -EPROBE_DEFER;
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 2/4] drm: sun4i: dsi: Add bridge support
2021-03-22 14:01 ` [PATCH v4 2/4] drm: sun4i: dsi: Add bridge support Jagan Teki
@ 2021-03-24 1:10 ` Laurent Pinchart
2021-03-24 3:01 ` Samuel Holland
1 sibling, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2021-03-24 1:10 UTC (permalink / raw)
To: Jagan Teki
Cc: Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi
Hi Jagan,
Thank you for the patch.
On Mon, Mar 22, 2021 at 07:31:50PM +0530, Jagan Teki wrote:
> Some display panels would come up with a non-DSI output which
Did you mean input instead of output ?
> can have an option to connect DSI interface by means of bridge
> converter.
>
> This DSI to non-DSI bridge converter would require a bridge
> driver that would communicate the DSI controller for bridge
> functionalities.
>
> So, add support for bridge functionalities in Allwinner DSI
> controller.
>
> Cc: Samuel Holland <samuel@sholland.org>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Note:
> Samuel Holland, The existing kms hotplug dropped in order to
> attach the bridge properly.
>
> However, I did try several ways to support hotplug with the
> bridge but it's resulting in a deadlock where bind never attach
> bridge until bridge pointer found and bridge pointer cannot
> found until bind finishes. Any inputs on this would be appreciated.
>
> Changes for v4:
> - none
> Changes for v3:
> - updated with new API's
>
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 34 +++++++++++++++++---------
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 +-
> 2 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 2e9e7b2d4145..39321299dc27 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -773,6 +773,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
> if (dsi->panel)
> drm_panel_prepare(dsi->panel);
>
> + if (dsi->panel_bridge)
> + dsi->panel_bridge->funcs->pre_enable(dsi->panel_bridge);
> +
> /*
> * FIXME: This should be moved after the switch to HS mode.
> *
> @@ -788,6 +791,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
> if (dsi->panel)
> drm_panel_enable(dsi->panel);
>
> + if (dsi->panel_bridge)
> + dsi->panel_bridge->funcs->enable(dsi->panel_bridge);
> +
> sun6i_dsi_start(dsi, DSI_START_HSC);
>
> udelay(1000);
> @@ -804,6 +810,9 @@ static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
> if (dsi->panel) {
> drm_panel_disable(dsi->panel);
> drm_panel_unprepare(dsi->panel);
> + } else if (dsi->panel_bridge) {
> + dsi->panel_bridge->funcs->disable(dsi->panel_bridge);
> + dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge);
> }
Instead of having code paths that depend on whether you have a panel or
a bridge, it would be better to wrap the panel a bridge (using
drivers/gpu/drm/bridge/panel.c). The dsi->panel_bridge pointer should be
renamed to next_bridge, and all the code (except in probe) can the use
next_bridge without caring if it's a direct connection to a panel or
another bridge.
Furthermore, the encoder should call bridge functions explicitly, this
should be handled by the DRM core.
>
> phy_power_off(dsi->dphy);
> @@ -964,23 +973,17 @@ 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;
> int ret;
>
> ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> - &panel, NULL);
> + &dsi->panel, &dsi->panel_bridge);
> if (ret)
> return ret;
>
> - 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);
> + dev_info(host->dev, "Attached %s %s\n",
> + device->name, dsi->panel ? "panel" : "bridge");
>
> return 0;
> }
> @@ -991,9 +994,10 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host,
> struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
>
> dsi->panel = NULL;
> + dsi->panel_bridge = NULL;
> dsi->device = NULL;
>
> - drm_kms_helper_hotplug_event(dsi->drm);
> + drm_of_panel_bridge_remove(dsi->dev->of_node, 0, 0);
>
> return 0;
> }
> @@ -1082,7 +1086,13 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,
>
> drm_connector_attach_encoder(&dsi->connector, &dsi->encoder);
>
> - dsi->drm = drm;
> + if (dsi->panel_bridge) {
> + ret = drm_bridge_attach(&dsi->encoder, dsi->panel_bridge, NULL, 0);
> + if (ret) {
> + dev_err(dsi->dev, "Couldn't attach drm bridge\n");
> + goto err_cleanup_connector;
> + }
> + }
>
> return 0;
>
> @@ -1096,7 +1106,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..370ecb356a63 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> @@ -29,8 +29,8 @@ struct sun6i_dsi {
>
> struct device *dev;
> struct mipi_dsi_device *device;
> - struct drm_device *drm;
> struct drm_panel *panel;
> + struct drm_bridge *panel_bridge;
> };
>
> static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 3/4] drm: sun4i: dsi: Convert to bridge driver
2021-03-22 14:01 ` [PATCH v4 3/4] drm: sun4i: dsi: Convert to bridge driver Jagan Teki
@ 2021-03-24 1:11 ` Laurent Pinchart
0 siblings, 0 replies; 15+ messages in thread
From: Laurent Pinchart @ 2021-03-24 1:11 UTC (permalink / raw)
To: Jagan Teki
Cc: Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi
Hi Jagan,
Thank you for the patch.
On Mon, Mar 22, 2021 at 07:31:51PM +0530, Jagan Teki wrote:
> DRM bridge drivers have build-in handling of treating all display
> pipeline components as bridges.
>
> So, convert the existing to a drm bridge driver with a built-in
> encoder support for compatibility with existing component drivers.
It would be best if possible to move this patch before 2/4, to first
convert to the bridge model, and then build on top of it.
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Changes for v4:
> - none
> Changes for v3:
> - new patch
>
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 75 ++++++++++++++++----------
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 6 +++
> 2 files changed, 54 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 39321299dc27..6f3c5330a468 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -714,10 +714,10 @@ static int sun6i_dsi_start(struct sun6i_dsi *dsi,
> return 0;
> }
>
> -static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
> +static void sun6i_dsi_bridge_enable(struct drm_bridge *bridge)
> {
> - struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
> - struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
> + struct drm_display_mode *mode = &bridge->encoder->crtc->state->adjusted_mode;
> + struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
> struct mipi_dsi_device *device = dsi->device;
> union phy_configure_opts opts = { };
> struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy;
> @@ -801,9 +801,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
> sun6i_dsi_start(dsi, DSI_START_HSD);
> }
>
> -static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
> +static void sun6i_dsi_bridge_disable(struct drm_bridge *bridge)
> {
> - struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
> + struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
>
> DRM_DEBUG_DRIVER("Disabling DSI output\n");
>
> @@ -852,9 +852,40 @@ static const struct drm_connector_funcs sun6i_dsi_connector_funcs = {
> .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,
> +static int sun6i_dsi_bridge_attach(struct drm_bridge *bridge,
> + enum drm_bridge_attach_flags flags)
> +{
> + struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
> + int ret;
> +
> + if (dsi->panel_bridge)
> + return drm_bridge_attach(bridge->encoder, dsi->panel_bridge, NULL, 0);
> +
> + if (dsi->panel) {
> + drm_connector_helper_add(&dsi->connector,
> + &sun6i_dsi_connector_helper_funcs);
> + ret = drm_connector_init(bridge->dev, &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 const struct drm_bridge_funcs sun6i_dsi_bridge_funcs = {
> + .enable = sun6i_dsi_bridge_enable,
> + .disable = sun6i_dsi_bridge_disable,
> + .attach = sun6i_dsi_bridge_attach,
> };
>
> static u32 sun6i_dsi_dcs_build_pkt_hdr(struct sun6i_dsi *dsi,
> @@ -1063,8 +1094,6 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,
> 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) {
> @@ -1073,27 +1102,12 @@ 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->bridge, NULL, 0);
> if (ret) {
> - dev_err(dsi->dev,
> - "Couldn't initialise the DSI connector\n");
> + dev_err(dsi->dev, "Couldn't attach drm bridge\n");
> goto err_cleanup_connector;
> }
>
> - drm_connector_attach_encoder(&dsi->connector, &dsi->encoder);
> -
> - if (dsi->panel_bridge) {
> - ret = drm_bridge_attach(&dsi->encoder, dsi->panel_bridge, NULL, 0);
> - if (ret) {
> - dev_err(dsi->dev, "Couldn't attach drm bridge\n");
> - goto err_cleanup_connector;
> - }
> - }
> -
> return 0;
>
> err_cleanup_connector:
> @@ -1199,6 +1213,12 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
> goto err_unprotect_clk;
> }
>
> + dsi->bridge.funcs = &sun6i_dsi_bridge_funcs;
> + dsi->bridge.of_node = dev->of_node;
> + dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
> +
> + drm_bridge_add(&dsi->bridge);
> +
> ret = component_add(&pdev->dev, &sun6i_dsi_ops);
> if (ret) {
> dev_err(dev, "Couldn't register our component\n");
> @@ -1222,6 +1242,7 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct sun6i_dsi *dsi = dev_get_drvdata(dev);
>
> + drm_bridge_remove(&dsi->bridge);
> component_del(&pdev->dev, &sun6i_dsi_ops);
> mipi_dsi_host_unregister(&dsi->host);
> clk_rate_exclusive_put(dsi->mod_clk);
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> index 370ecb356a63..5e70666089ad 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> @@ -16,6 +16,7 @@
> #define SUN6I_DSI_TCON_DIV 4
>
> struct sun6i_dsi {
> + struct drm_bridge bridge;
> struct drm_connector connector;
> struct drm_encoder encoder;
The drm_encoder should be dropped from this driver, the encoder should
be created by the main display driver.
> struct mipi_dsi_host host;
> @@ -38,6 +39,11 @@ 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 *bridge_to_sun6i_dsi(struct drm_bridge *bridge)
> +{
> + return container_of(bridge, struct sun6i_dsi, bridge);
> +}
> +
> static inline struct sun6i_dsi *connector_to_sun6i_dsi(struct drm_connector *connector)
> {
> return container_of(connector, struct sun6i_dsi, connector);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge
2021-03-23 22:53 ` Laurent Pinchart
@ 2021-03-24 2:48 ` Samuel Holland
2021-03-24 9:14 ` Jagan Teki
0 siblings, 1 reply; 15+ messages in thread
From: Samuel Holland @ 2021-03-24 2:48 UTC (permalink / raw)
To: Laurent Pinchart, Jagan Teki
Cc: Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec, dri-devel,
linux-arm-kernel, linux-kernel, linux-amarula, linux-sunxi
On 3/23/21 5:53 PM, Laurent Pinchart wrote:
> Hi Jagan,
>
> Thank you for the patch.
>
> On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
>> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
>> for finding panel, this indeed help to find the bridge if
>> bridge support added.
>>
>> Added NULL in bridge argument, same will replace with bridge
>> parameter once bridge supported.
>>
>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>
> Looks good, there should be no functional change.
Actually this breaks all existing users of this driver, see below.
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
>> ---
>> Changes for v4, v3:
>> - none
>>
>> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>> index 4f5efcace68e..2e9e7b2d4145 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>
>> @@ -963,10 +964,14 @@ 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);
This is using the OF node of the DSI device, which is a direct child of
the DSI host's OF node. There is no OF graph involved.
>> + struct drm_panel *panel;
>> + int ret;
>> +
>> + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
>> + &panel, NULL);
However, this function expects to find the panel using OF graph. This
does not work with existing device trees (PinePhone, PineTab) which do
not use OF graph to connect the panel. And it cannot work, because the
DSI host's binding specifies a single port: the input port from the
display engine.
Regards,
Samuel
>> + if (ret)
>> + return ret;
>>
>> - if (IS_ERR(panel))
>> - return PTR_ERR(panel);
>> if (!dsi->drm || !dsi->drm->registered)
>> return -EPROBE_DEFER;
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 2/4] drm: sun4i: dsi: Add bridge support
2021-03-22 14:01 ` [PATCH v4 2/4] drm: sun4i: dsi: Add bridge support Jagan Teki
2021-03-24 1:10 ` Laurent Pinchart
@ 2021-03-24 3:01 ` Samuel Holland
1 sibling, 0 replies; 15+ messages in thread
From: Samuel Holland @ 2021-03-24 3:01 UTC (permalink / raw)
To: Jagan Teki, Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec,
Laurent Pinchart
Cc: dri-devel, linux-arm-kernel, linux-kernel, linux-amarula, linux-sunxi
On 3/22/21 9:01 AM, Jagan Teki wrote:
> Some display panels would come up with a non-DSI output which
> can have an option to connect DSI interface by means of bridge
> converter.
>
> This DSI to non-DSI bridge converter would require a bridge
> driver that would communicate the DSI controller for bridge
> functionalities.
>
> So, add support for bridge functionalities in Allwinner DSI
> controller.
>
> Cc: Samuel Holland <samuel@sholland.org>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
> Note:
> Samuel Holland, The existing kms hotplug dropped in order to
> attach the bridge properly.
>
> However, I did try several ways to support hotplug with the
> bridge but it's resulting in a deadlock where bind never attach
> bridge until bridge pointer found and bridge pointer cannot
> found until bind finishes. Any inputs on this would be appreciated.
The intended behavior is that sun6i_dsi_bind() is independent of any DSI
device. And sun6i_dsi_attach() must only be called after bind completes
and the DRM device is registered. This design allows the rest of the
display engine (such as the HDMI output) to work even if no panel is
listed in the device tree, or if a panel driver is missing.
> Changes for v4:
> - none
> Changes for v3:
> - updated with new API's
>
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 34 +++++++++++++++++---------
> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 +-
> 2 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> index 2e9e7b2d4145..39321299dc27 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> @@ -773,6 +773,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
> if (dsi->panel)
> drm_panel_prepare(dsi->panel);
>
> + if (dsi->panel_bridge)
> + dsi->panel_bridge->funcs->pre_enable(dsi->panel_bridge);
> +
> /*
> * FIXME: This should be moved after the switch to HS mode.
> *
> @@ -788,6 +791,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
> if (dsi->panel)
> drm_panel_enable(dsi->panel);
>
> + if (dsi->panel_bridge)
> + dsi->panel_bridge->funcs->enable(dsi->panel_bridge);
> +
> sun6i_dsi_start(dsi, DSI_START_HSC);
>
> udelay(1000);
> @@ -804,6 +810,9 @@ static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
> if (dsi->panel) {
> drm_panel_disable(dsi->panel);
> drm_panel_unprepare(dsi->panel);
> + } else if (dsi->panel_bridge) {
> + dsi->panel_bridge->funcs->disable(dsi->panel_bridge);
> + dsi->panel_bridge->funcs->post_disable(dsi->panel_bridge);
> }
>
> phy_power_off(dsi->dphy);
> @@ -964,23 +973,17 @@ 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;
> int ret;
>
> ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> - &panel, NULL);
> + &dsi->panel, &dsi->panel_bridge);
> if (ret)
> return ret;
>
> - 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);
> + dev_info(host->dev, "Attached %s %s\n",
> + device->name, dsi->panel ? "panel" : "bridge");
>
> return 0;
> }
> @@ -991,9 +994,10 @@ static int sun6i_dsi_detach(struct mipi_dsi_host *host,
> struct sun6i_dsi *dsi = host_to_sun6i_dsi(host);
>
> dsi->panel = NULL;
> + dsi->panel_bridge = NULL;
> dsi->device = NULL;
>
> - drm_kms_helper_hotplug_event(dsi->drm);
> + drm_of_panel_bridge_remove(dsi->dev->of_node, 0, 0);
>
> return 0;
> }
> @@ -1082,7 +1086,13 @@ static int sun6i_dsi_bind(struct device *dev, struct device *master,
>
> drm_connector_attach_encoder(&dsi->connector, &dsi->encoder);
>
> - dsi->drm = drm;
> + if (dsi->panel_bridge) {
> + ret = drm_bridge_attach(&dsi->encoder, dsi->panel_bridge, NULL, 0);
> + if (ret) {
> + dev_err(dsi->dev, "Couldn't attach drm bridge\n");
> + goto err_cleanup_connector;
> + }
> + }
You initialize dsi->panel_bridge in sun6i_dsi_attach() above, but that
function may run long after sun6i_dsi_bind() has completed -- for
example if this driver is built in, but the panel/bridge driver is built
as a module. So you cannot reference dsi->panel_bridge from this function.
Regards,
Samuel
>
> return 0;
>
> @@ -1096,7 +1106,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..370ecb356a63 100644
> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> @@ -29,8 +29,8 @@ struct sun6i_dsi {
>
> struct device *dev;
> struct mipi_dsi_device *device;
> - struct drm_device *drm;
> struct drm_panel *panel;
> + struct drm_bridge *panel_bridge;
> };
>
> static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge
2021-03-24 2:48 ` Samuel Holland
@ 2021-03-24 9:14 ` Jagan Teki
2021-03-24 9:38 ` Laurent Pinchart
0 siblings, 1 reply; 15+ messages in thread
From: Jagan Teki @ 2021-03-24 9:14 UTC (permalink / raw)
To: Samuel Holland
Cc: Laurent Pinchart, Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec,
dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi
On Wed, Mar 24, 2021 at 8:18 AM Samuel Holland <samuel@sholland.org> wrote:
>
> On 3/23/21 5:53 PM, Laurent Pinchart wrote:
> > Hi Jagan,
> >
> > Thank you for the patch.
> >
> > On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
> >> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
> >> for finding panel, this indeed help to find the bridge if
> >> bridge support added.
> >>
> >> Added NULL in bridge argument, same will replace with bridge
> >> parameter once bridge supported.
> >>
> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >
> > Looks good, there should be no functional change.
>
> Actually this breaks all existing users of this driver, see below.
>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> >> ---
> >> Changes for v4, v3:
> >> - none
> >>
> >> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
> >> 1 file changed, 8 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> >> index 4f5efcace68e..2e9e7b2d4145 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>
> >> @@ -963,10 +964,14 @@ 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);
>
> This is using the OF node of the DSI device, which is a direct child of
> the DSI host's OF node. There is no OF graph involved.
>
> >> + struct drm_panel *panel;
> >> + int ret;
> >> +
> >> + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> >> + &panel, NULL);
>
> However, this function expects to find the panel using OF graph. This
> does not work with existing device trees (PinePhone, PineTab) which do
> not use OF graph to connect the panel. And it cannot work, because the
> DSI host's binding specifies a single port: the input port from the
> display engine.
Thanks for noticing this. I did understand your point and yes, I did
mention the updated pipeline in previous versions and forgot to add it
to this series.
Here is the updated pipeline to make it work:
https://patchwork.kernel.org/project/dri-devel/patch/20190524104252.20236-1-jagan@amarulasolutions.com/
Let me know your comments on this, so I will add a patch for the
above-affected DTS files.
Jagan.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge
2021-03-24 9:14 ` Jagan Teki
@ 2021-03-24 9:38 ` Laurent Pinchart
2021-03-24 9:49 ` Jagan Teki
0 siblings, 1 reply; 15+ messages in thread
From: Laurent Pinchart @ 2021-03-24 9:38 UTC (permalink / raw)
To: Jagan Teki
Cc: Samuel Holland, Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec,
dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi
Hi Jagan,
On Wed, Mar 24, 2021 at 02:44:57PM +0530, Jagan Teki wrote:
> On Wed, Mar 24, 2021 at 8:18 AM Samuel Holland wrote:
> > On 3/23/21 5:53 PM, Laurent Pinchart wrote:
> > > On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
> > >> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
> > >> for finding panel, this indeed help to find the bridge if
> > >> bridge support added.
> > >>
> > >> Added NULL in bridge argument, same will replace with bridge
> > >> parameter once bridge supported.
> > >>
> > >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > >
> > > Looks good, there should be no functional change.
> >
> > Actually this breaks all existing users of this driver, see below.
> >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > >> ---
> > >> Changes for v4, v3:
> > >> - none
> > >>
> > >> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
> > >> 1 file changed, 8 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > >> index 4f5efcace68e..2e9e7b2d4145 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>
> > >> @@ -963,10 +964,14 @@ 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);
> >
> > This is using the OF node of the DSI device, which is a direct child of
> > the DSI host's OF node. There is no OF graph involved.
> >
> > >> + struct drm_panel *panel;
> > >> + int ret;
> > >> +
> > >> + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> > >> + &panel, NULL);
> >
> > However, this function expects to find the panel using OF graph. This
> > does not work with existing device trees (PinePhone, PineTab) which do
> > not use OF graph to connect the panel. And it cannot work, because the
> > DSI host's binding specifies a single port: the input port from the
> > display engine.
>
> Thanks for noticing this. I did understand your point and yes, I did
> mention the updated pipeline in previous versions and forgot to add it
> to this series.
>
> Here is the updated pipeline to make it work:
>
> https://patchwork.kernel.org/project/dri-devel/patch/20190524104252.20236-1-jagan@amarulasolutions.com/
>
> Let me know your comments on this, so I will add a patch for the
> above-affected DTS files.
DT is an ABI, we need to ensure backward compatibility. Changes in
kernel drivers can't break devices that have an old DT.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge
2021-03-24 9:38 ` Laurent Pinchart
@ 2021-03-24 9:49 ` Jagan Teki
2021-03-24 9:55 ` Laurent Pinchart
0 siblings, 1 reply; 15+ messages in thread
From: Jagan Teki @ 2021-03-24 9:49 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Samuel Holland, Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec,
dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi
Hi Laurent,
On Wed, Mar 24, 2021 at 3:09 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Jagan,
>
> On Wed, Mar 24, 2021 at 02:44:57PM +0530, Jagan Teki wrote:
> > On Wed, Mar 24, 2021 at 8:18 AM Samuel Holland wrote:
> > > On 3/23/21 5:53 PM, Laurent Pinchart wrote:
> > > > On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
> > > >> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
> > > >> for finding panel, this indeed help to find the bridge if
> > > >> bridge support added.
> > > >>
> > > >> Added NULL in bridge argument, same will replace with bridge
> > > >> parameter once bridge supported.
> > > >>
> > > >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > >
> > > > Looks good, there should be no functional change.
> > >
> > > Actually this breaks all existing users of this driver, see below.
> > >
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > >> ---
> > > >> Changes for v4, v3:
> > > >> - none
> > > >>
> > > >> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
> > > >> 1 file changed, 8 insertions(+), 3 deletions(-)
> > > >>
> > > >> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > >> index 4f5efcace68e..2e9e7b2d4145 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>
> > > >> @@ -963,10 +964,14 @@ 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);
> > >
> > > This is using the OF node of the DSI device, which is a direct child of
> > > the DSI host's OF node. There is no OF graph involved.
> > >
> > > >> + struct drm_panel *panel;
> > > >> + int ret;
> > > >> +
> > > >> + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> > > >> + &panel, NULL);
> > >
> > > However, this function expects to find the panel using OF graph. This
> > > does not work with existing device trees (PinePhone, PineTab) which do
> > > not use OF graph to connect the panel. And it cannot work, because the
> > > DSI host's binding specifies a single port: the input port from the
> > > display engine.
> >
> > Thanks for noticing this. I did understand your point and yes, I did
> > mention the updated pipeline in previous versions and forgot to add it
> > to this series.
> >
> > Here is the updated pipeline to make it work:
> >
> > https://patchwork.kernel.org/project/dri-devel/patch/20190524104252.20236-1-jagan@amarulasolutions.com/
> >
> > Let me know your comments on this, so I will add a patch for the
> > above-affected DTS files.
>
> DT is an ABI, we need to ensure backward compatibility. Changes in
> kernel drivers can't break devices that have an old DT.
Thanks for your point.
So, we need to choose APIs that would compatible with the old DT and
new DT changes. Am I correct?
Jagan.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge
2021-03-24 9:49 ` Jagan Teki
@ 2021-03-24 9:55 ` Laurent Pinchart
2021-03-24 10:11 ` Maxime Ripard
0 siblings, 1 reply; 15+ messages in thread
From: Laurent Pinchart @ 2021-03-24 9:55 UTC (permalink / raw)
To: Jagan Teki
Cc: Samuel Holland, Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec,
dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi
Hi Jagan,
On Wed, Mar 24, 2021 at 03:19:10PM +0530, Jagan Teki wrote:
> On Wed, Mar 24, 2021 at 3:09 PM Laurent Pinchart wrote:
> > On Wed, Mar 24, 2021 at 02:44:57PM +0530, Jagan Teki wrote:
> > > On Wed, Mar 24, 2021 at 8:18 AM Samuel Holland wrote:
> > > > On 3/23/21 5:53 PM, Laurent Pinchart wrote:
> > > > > On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
> > > > >> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
> > > > >> for finding panel, this indeed help to find the bridge if
> > > > >> bridge support added.
> > > > >>
> > > > >> Added NULL in bridge argument, same will replace with bridge
> > > > >> parameter once bridge supported.
> > > > >>
> > > > >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > >
> > > > > Looks good, there should be no functional change.
> > > >
> > > > Actually this breaks all existing users of this driver, see below.
> > > >
> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > >
> > > > >> ---
> > > > >> Changes for v4, v3:
> > > > >> - none
> > > > >>
> > > > >> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
> > > > >> 1 file changed, 8 insertions(+), 3 deletions(-)
> > > > >>
> > > > >> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > >> index 4f5efcace68e..2e9e7b2d4145 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>
> > > > >> @@ -963,10 +964,14 @@ 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);
> > > >
> > > > This is using the OF node of the DSI device, which is a direct child of
> > > > the DSI host's OF node. There is no OF graph involved.
> > > >
> > > > >> + struct drm_panel *panel;
> > > > >> + int ret;
> > > > >> +
> > > > >> + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> > > > >> + &panel, NULL);
> > > >
> > > > However, this function expects to find the panel using OF graph. This
> > > > does not work with existing device trees (PinePhone, PineTab) which do
> > > > not use OF graph to connect the panel. And it cannot work, because the
> > > > DSI host's binding specifies a single port: the input port from the
> > > > display engine.
> > >
> > > Thanks for noticing this. I did understand your point and yes, I did
> > > mention the updated pipeline in previous versions and forgot to add it
> > > to this series.
> > >
> > > Here is the updated pipeline to make it work:
> > >
> > > https://patchwork.kernel.org/project/dri-devel/patch/20190524104252.20236-1-jagan@amarulasolutions.com/
> > >
> > > Let me know your comments on this, so I will add a patch for the
> > > above-affected DTS files.
> >
> > DT is an ABI, we need to ensure backward compatibility. Changes in
> > kernel drivers can't break devices that have an old DT.
>
> Thanks for your point.
>
> So, we need to choose APIs that would compatible with the old DT and
> new DT changes. Am I correct?
Yes, that's correct.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge
2021-03-24 9:55 ` Laurent Pinchart
@ 2021-03-24 10:11 ` Maxime Ripard
0 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2021-03-24 10:11 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Jagan Teki, Samuel Holland, Chen-Yu Tsai, Jernej Skrabec,
dri-devel, linux-arm-kernel, linux-kernel, linux-amarula,
linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 4206 bytes --]
On Wed, Mar 24, 2021 at 11:55:35AM +0200, Laurent Pinchart wrote:
> Hi Jagan,
>
> On Wed, Mar 24, 2021 at 03:19:10PM +0530, Jagan Teki wrote:
> > On Wed, Mar 24, 2021 at 3:09 PM Laurent Pinchart wrote:
> > > On Wed, Mar 24, 2021 at 02:44:57PM +0530, Jagan Teki wrote:
> > > > On Wed, Mar 24, 2021 at 8:18 AM Samuel Holland wrote:
> > > > > On 3/23/21 5:53 PM, Laurent Pinchart wrote:
> > > > > > On Mon, Mar 22, 2021 at 07:31:49PM +0530, Jagan Teki wrote:
> > > > > >> Replace of_drm_find_panel with drm_of_find_panel_or_bridge
> > > > > >> for finding panel, this indeed help to find the bridge if
> > > > > >> bridge support added.
> > > > > >>
> > > > > >> Added NULL in bridge argument, same will replace with bridge
> > > > > >> parameter once bridge supported.
> > > > > >>
> > > > > >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > >
> > > > > > Looks good, there should be no functional change.
> > > > >
> > > > > Actually this breaks all existing users of this driver, see below.
> > > > >
> > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > > >
> > > > > >> ---
> > > > > >> Changes for v4, v3:
> > > > > >> - none
> > > > > >>
> > > > > >> drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 11 ++++++++---
> > > > > >> 1 file changed, 8 insertions(+), 3 deletions(-)
> > > > > >>
> > > > > >> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
> > > > > >> index 4f5efcace68e..2e9e7b2d4145 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>
> > > > > >> @@ -963,10 +964,14 @@ 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);
> > > > >
> > > > > This is using the OF node of the DSI device, which is a direct child of
> > > > > the DSI host's OF node. There is no OF graph involved.
> > > > >
> > > > > >> + struct drm_panel *panel;
> > > > > >> + int ret;
> > > > > >> +
> > > > > >> + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 0, 0,
> > > > > >> + &panel, NULL);
> > > > >
> > > > > However, this function expects to find the panel using OF graph. This
> > > > > does not work with existing device trees (PinePhone, PineTab) which do
> > > > > not use OF graph to connect the panel. And it cannot work, because the
> > > > > DSI host's binding specifies a single port: the input port from the
> > > > > display engine.
> > > >
> > > > Thanks for noticing this. I did understand your point and yes, I did
> > > > mention the updated pipeline in previous versions and forgot to add it
> > > > to this series.
> > > >
> > > > Here is the updated pipeline to make it work:
> > > >
> > > > https://patchwork.kernel.org/project/dri-devel/patch/20190524104252.20236-1-jagan@amarulasolutions.com/
> > > >
> > > > Let me know your comments on this, so I will add a patch for the
> > > > above-affected DTS files.
> > >
> > > DT is an ABI, we need to ensure backward compatibility. Changes in
> > > kernel drivers can't break devices that have an old DT.
> >
> > Thanks for your point.
> >
> > So, we need to choose APIs that would compatible with the old DT and
> > new DT changes. Am I correct?
>
> Yes, that's correct.
However, I see no particular reason to change the DT binding in this
case. The DSI devices are supposed to be described through a subnode of
their DSI controller, that's the generic binding and except for very odd
devices (and a bridge like this one is certainly not one), I see no
reason to deviate from that.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-03-24 10:12 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 14:01 [PATCH v4 0/4] drm: sun4i: dsi: Convert drm bridge Jagan Teki
2021-03-22 14:01 ` [PATCH v4 1/4] drm: sun4i: dsi: Use drm_of_find_panel_or_bridge Jagan Teki
2021-03-23 22:53 ` Laurent Pinchart
2021-03-24 2:48 ` Samuel Holland
2021-03-24 9:14 ` Jagan Teki
2021-03-24 9:38 ` Laurent Pinchart
2021-03-24 9:49 ` Jagan Teki
2021-03-24 9:55 ` Laurent Pinchart
2021-03-24 10:11 ` Maxime Ripard
2021-03-22 14:01 ` [PATCH v4 2/4] drm: sun4i: dsi: Add bridge support Jagan Teki
2021-03-24 1:10 ` Laurent Pinchart
2021-03-24 3:01 ` Samuel Holland
2021-03-22 14:01 ` [PATCH v4 3/4] drm: sun4i: dsi: Convert to bridge driver Jagan Teki
2021-03-24 1:11 ` Laurent Pinchart
2021-03-22 14:01 ` [DO NOT MERGE] [PATCH v4 4/4] ARM: dts: sun8i: bananapi-m2m: Enable S070WV20-CT16 panel Jagan Teki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).