Hi Andrzej,

Am Montag, 13. August 2018, 10:28:39 CEST schrieb Andrzej Hajda:

On 09.07.2018 15:48, Heiko Stuebner wrote:

quoted

When the panel-driver is build as a module it currently fails hard as the
panel cannot be probed directly:

dw_mipi_dsi_bind()
  __dw_mipi_dsi_probe()
    creates dsi bus
    creates panel device
    triggers panel module load
    panel not probed (module not loaded or panel probe slow)
  drm_bridge_attach
    fails with -EINVAL due to empty panel_bridge

Additionally the panel probing can run concurrently with dsi bringup
making it possible that the panel can already be found but dsi-attach
hasn't finished running.

The newly added function provides the ability for glue drivers to
check if a dsi device was actually attached and also protects
the attach part to prevent concurrency issues from panel-assignment
and drm_bridge_create.

Using that check glue drivers are able to for example defer probe/bind
in the case that the panel is not completely set up yet.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 25 +++++++++++++++++++
 include/drm/bridge/dw_mipi_dsi.h              |  1 +
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index bb4aeca5c0f9..88fed22ff3f6 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c

@@ -12,6 +12,7 @@
 #include <linux/component.h>
 #include <linux/iopoll.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>

@@ -219,6 +220,7 @@ struct dw_mipi_dsi {
 	struct drm_bridge bridge;
 	struct mipi_dsi_host dsi_host;
 	struct drm_bridge *panel_bridge;
+	struct mutex panel_mutex;
 	struct device *dev;
 	void __iomem *base;
 

@@ -296,10 +298,14 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
 			return PTR_ERR(bridge);
 	}
 
+	mutex_lock(&dsi->panel_mutex);
+
 	dsi->panel_bridge = bridge;
 
 	drm_bridge_add(&dsi->bridge);
 
+	mutex_unlock(&dsi->panel_mutex);
+
 	return 0;
 }
 

@@ -308,13 +314,30 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host *host,
 {
 	struct dw_mipi_dsi *dsi = host_to_dsi(host);
 
+	mutex_lock(&dsi->panel_mutex);
+
+	dsi->panel_bridge = NULL;
 	drm_of_panel_bridge_remove(host->dev->of_node, 1, 0);
 
 	drm_bridge_remove(&dsi->bridge);
 
+	mutex_unlock(&dsi->panel_mutex);
+
 	return 0;
 }
 
+bool dw_mipi_dsi_device_attached(struct dw_mipi_dsi *dsi)
+{
+	bool output;
+
+	mutex_lock(&dsi->panel_mutex);
+	output = !!dsi->panel_bridge;
+	mutex_unlock(&dsi->panel_mutex);
+
+	return output;
+}
+EXPORT_SYMBOL_GPL(dw_mipi_dsi_device_attached);

The function does not make sense. After releasing panel_mutex device can
be detached/(re-)attached multiple times. Ie it reports useless
information. Of course in most cases it will work as expected, but for
sure it is not bulletproof.

Ok. Can you suggest how we should check for the described case?
I.e. initially I tried to simply defer in dw_mipi_dsi_bridge_attach [0]
where you suggested to do that in probe.

I moved the check to bind - see patch 5 - to fix the issue regarding
panel only probing after the dsi bus gets created, so this function
is a means to check if the panel has finished attaching, or to defer
binding - see dw_mipi_dsi_rockchip_bind in patch 5.

So I'm somewhat out of ideas right now, how to do this right.


Thanks
Heiko

[0] https://patchwork.kernel.org/patch/10470821/


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel