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
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 <[email protected]>
Signed-off-by: Jagan Teki <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> Signed-off-by: Jagan Teki <[email protected]>
> ---
> 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
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 <[email protected]>
> Signed-off-by: Jagan Teki <[email protected]>
> ---
> 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)
>