Hi Michael,
On 11.09.2020 15:54, Michael Tretter wrote:
> Make the exynos_dsi driver a full drm bridge that can be found and used
> from other drivers.
>
> Other drivers can only attach to the bridge, if a mipi dsi device
> already attached to the bridge. This allows to defer the probe of the
> display pipe until the downstream bridges are available, too.
>
> Signed-off-by: Michael Tretter <m.tret...@pengutronix.de>
This one (and the whole series applied) still fails on Exynos boards:
[drm] Exynos DRM: using 11c00000.fimd device for DMA mapping operations
exynos-drm exynos-drm: bound 11c00000.fimd (ops fimd_component_ops)
OF: graph: no port node found in /soc/dsi@11c80000
8<--- cut here ---
Unable to handle kernel NULL pointer dereference at virtual address 00000084
pgd = (ptrval)
[00000084] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Not tainted
5.9.0-rc4-next-20200911-00010-g417dc70d70ec #1608
Hardware name: Samsung Exynos (Flattened Device Tree)
PC is at drm_bridge_attach+0x18/0x164
LR is at exynos_dsi_bind+0x88/0xa8
pc : [<c0628c08>] lr : [<c064d560>] psr: 20000013
sp : ef0dfca8 ip : 00000002 fp : c13190e0
r10: 00000000 r9 : ee46d580 r8 : c13190e0
r7 : ee438800 r6 : 00000018 r5 : ef253810 r4 : ef39e840
r3 : 00000000 r2 : 00000018 r1 : ef39e888 r0 : ef39e840
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 10c5387d Table: 4000404a DAC: 00000051
Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
Stack: (0xef0dfca8 to 0xef0e0000)
...
[<c0628c08>] (drm_bridge_attach) from [<c064d560>]
(exynos_dsi_bind+0x88/0xa8)
[<c064d560>] (exynos_dsi_bind) from [<c066a800>]
(component_bind_all+0xfc/0x290)
[<c066a800>] (component_bind_all) from [<c0649dc0>]
(exynos_drm_bind+0xe4/0x19c)
[<c0649dc0>] (exynos_drm_bind) from [<c066ad74>]
(try_to_bring_up_master+0x1e4/0x2c4)
[<c066ad74>] (try_to_bring_up_master) from [<c066b2b4>]
(component_master_add_with_match+0xd4/0x108)
[<c066b2b4>] (component_master_add_with_match) from [<c0649ae8>]
(exynos_drm_platform_probe+0xe4/0x110)
[<c0649ae8>] (exynos_drm_platform_probe) from [<c0674e6c>]
(platform_drv_probe+0x6c/0xa4)
[<c0674e6c>] (platform_drv_probe) from [<c067242c>]
(really_probe+0x200/0x4fc)
[<c067242c>] (really_probe) from [<c06728f0>]
(driver_probe_device+0x78/0x1fc)
[<c06728f0>] (driver_probe_device) from [<c0672cd8>]
(device_driver_attach+0x58/0x60)
[<c0672cd8>] (device_driver_attach) from [<c0672dbc>]
(__driver_attach+0xdc/0x174)
[<c0672dbc>] (__driver_attach) from [<c06701b4>]
(bus_for_each_dev+0x68/0xb4)
[<c06701b4>] (bus_for_each_dev) from [<c06714e8>]
(bus_add_driver+0x158/0x214)
[<c06714e8>] (bus_add_driver) from [<c0673c1c>] (driver_register+0x78/0x110)
[<c0673c1c>] (driver_register) from [<c0649ca8>]
(exynos_drm_init+0xe4/0x118)
[<c0649ca8>] (exynos_drm_init) from [<c0102484>]
(do_one_initcall+0x8c/0x42c)
[<c0102484>] (do_one_initcall) from [<c11011c0>]
(kernel_init_freeable+0x190/0x1dc)
[<c11011c0>] (kernel_init_freeable) from [<c0af7880>]
(kernel_init+0x8/0x118)
[<c0af7880>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20)
Exception stack(0xef0dffb0 to 0xef0dfff8)
...
---[ end trace ee27f313f9ed9da1 ]---
# arm-linux-gnueabi-addr2line -e vmlinux c0628c08
drivers/gpu/drm/drm_bridge.c:184 (discriminator 1)
I will try to debug it a bit more today.
> ---
> v2:
> - move attach of out_bridge to bridge_attach
> - add bridge_detach
> - don't cleanup encoder if create_connector failed
> ---
> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 103 +++++++++++++++++-------
> 1 file changed, 75 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 2d75f9877dc0..5e7c1a99a3ee 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -266,6 +266,7 @@ struct exynos_dsi_driver_data {
>
> struct exynos_dsi {
> struct drm_encoder encoder;
> + struct drm_bridge bridge;
> struct mipi_dsi_host dsi_host;
> struct drm_connector connector;
> struct drm_panel *panel;
> @@ -1602,6 +1603,60 @@ static const struct drm_encoder_helper_funcs
> exynos_dsi_encoder_helper_funcs = {
> .disable = exynos_dsi_disable,
> };
>
> +static int exynos_dsi_bridge_attach(struct drm_bridge *bridge,
> + enum drm_bridge_attach_flags flags)
> +{
> + struct exynos_dsi *dsi = bridge->driver_private;
> + struct drm_encoder *encoder = bridge->encoder;
> + int ret;
> +
> + if (!dsi->out_bridge && !dsi->panel)
> + return -EPROBE_DEFER;
> +
> + if (dsi->out_bridge) {
> + ret = drm_bridge_attach(encoder, dsi->out_bridge,
> + bridge, flags);
> + if (ret)
> + return ret;
> + list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain);
> + } else {
> + ret = exynos_dsi_create_connector(encoder);
> + if (ret)
> + return ret;
> +
> + if (dsi->panel) {
> + dsi->connector.status = connector_status_connected;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static void exynos_dsi_bridge_detach(struct drm_bridge *bridge)
> +{
> + struct exynos_dsi *dsi = bridge->driver_private;
> + struct drm_encoder *encoder = bridge->encoder;
> + struct drm_device *drm = encoder->dev;
> +
> + if (dsi->panel) {
> + mutex_lock(&drm->mode_config.mutex);
> + exynos_dsi_disable(&dsi->encoder);
> + dsi->panel = NULL;
> + dsi->connector.status = connector_status_disconnected;
> + mutex_unlock(&drm->mode_config.mutex);
> + } else {
> + if (dsi->out_bridge->funcs->detach)
> + dsi->out_bridge->funcs->detach(dsi->out_bridge);
> + dsi->out_bridge = NULL;
> + INIT_LIST_HEAD(&dsi->bridge_chain);
> + }
> +}
> +
> +static const struct drm_bridge_funcs exynos_dsi_bridge_funcs = {
> + .attach = exynos_dsi_bridge_attach,
> + .detach = exynos_dsi_bridge_detach,
> +};
> +
> MODULE_DEVICE_TABLE(of, exynos_dsi_of_match);
>
> static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
> @@ -1609,25 +1664,12 @@ static int exynos_dsi_host_attach(struct
> mipi_dsi_host *host,
> {
> struct exynos_dsi *dsi = host_to_dsi(host);
> const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops;
> - struct drm_encoder *encoder = &dsi->encoder;
> struct drm_bridge *out_bridge;
>
> - out_bridge = of_drm_find_bridge(device->dev.of_node);
> + out_bridge = of_drm_find_bridge(device->dev.of_node);
> if (out_bridge) {
> - drm_bridge_attach(encoder, out_bridge, NULL, 0);
> dsi->out_bridge = out_bridge;
> - list_splice_init(&encoder->bridge_chain, &dsi->bridge_chain);
> } else {
> - int ret = exynos_dsi_create_connector(encoder);
> -
> - if (ret) {
> - DRM_DEV_ERROR(dsi->dev,
> - "failed to create connector ret = %d\n",
> - ret);
> - drm_encoder_cleanup(encoder);
> - return ret;
> - }
> -
> dsi->panel = of_drm_find_panel(device->dev.of_node);
> if (IS_ERR(dsi->panel))
> dsi->panel = NULL;
> @@ -1662,20 +1704,6 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host
> *host,
> {
> struct exynos_dsi *dsi = host_to_dsi(host);
> const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops;
> - struct drm_device *drm = dsi->encoder.dev;
> -
> - if (dsi->panel) {
> - mutex_lock(&drm->mode_config.mutex);
> - exynos_dsi_disable(&dsi->encoder);
> - dsi->panel = NULL;
> - dsi->connector.status = connector_status_disconnected;
> - mutex_unlock(&drm->mode_config.mutex);
> - } else {
> - if (dsi->out_bridge->funcs->detach)
> - dsi->out_bridge->funcs->detach(dsi->out_bridge);
> - dsi->out_bridge = NULL;
> - INIT_LIST_HEAD(&dsi->bridge_chain);
> - }
>
> if (ops && ops->detach)
> ops->detach(dsi->dsi_host.dev, device);
> @@ -1786,7 +1814,15 @@ static int exynos_dsi_bind(struct device *dev, struct
> device *master,
> of_node_put(in_bridge_node);
> }
>
> + ret = drm_bridge_attach(encoder, &dsi->bridge, in_bridge, 0);
> + if (ret)
> + goto err;
> +
> return 0;
> +
> +err:
> + drm_encoder_cleanup(encoder);
> + return ret;
> }
>
> static void exynos_dsi_unbind(struct device *dev, struct device *master,
> @@ -1796,6 +1832,8 @@ static void exynos_dsi_unbind(struct device *dev,
> struct device *master,
> struct drm_encoder *encoder = &dsi->encoder;
>
> exynos_dsi_disable(encoder);
> +
> + drm_encoder_cleanup(encoder);
> }
>
> static const struct component_ops exynos_dsi_component_ops = {
> @@ -1806,6 +1844,7 @@ static const struct component_ops
> exynos_dsi_component_ops = {
> static struct exynos_dsi *__exynos_dsi_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> + struct drm_bridge *bridge;
> struct resource *res;
> struct exynos_dsi *dsi;
> int ret, i;
> @@ -1894,11 +1933,19 @@ static struct exynos_dsi *__exynos_dsi_probe(struct
> platform_device *pdev)
> if (ret)
> return ERR_PTR(ret);
>
> + bridge = &dsi->bridge;
> + bridge->driver_private = dsi;
> + bridge->funcs = &exynos_dsi_bridge_funcs;
> + bridge->of_node = dev->of_node;
> + drm_bridge_add(bridge);
> +
> return dsi;
> }
>
> static void __exynos_dsi_remove(struct exynos_dsi *dsi)
> {
> + drm_bridge_remove(&dsi->bridge);
> +
> mipi_dsi_host_unregister(&dsi->dsi_host);
> }
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
