[PATCH v3 3/8] drm/bridge/synopsys: dsi: add ability to check dsi-device attachment

[Andrzej Hajda]

On 09.07.2018 15:48, Heiko Stuebner wrote:
> 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 at 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.

Regards
Andrzej

> +
>  static void dw_mipi_message_config(struct dw_mipi_dsi *dsi,
>  				   const struct mipi_dsi_msg *msg)
>  {
> @@ -867,6 +890,8 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
>  	dsi->dev = dev;
>  	dsi->plat_data = plat_data;
>  
> +	mutex_init(&dsi->panel_mutex);
> +
>  	if (!plat_data->phy_ops->init || !plat_data->phy_ops->get_lane_mbps) {
>  		DRM_ERROR("Phy not properly configured\n");
>  		return ERR_PTR(-ENODEV);
> diff --git a/include/drm/bridge/dw_mipi_dsi.h b/include/drm/bridge/dw_mipi_dsi.h
> index 6d7f8eb5d9f2..131ff2569ed4 100644
> --- a/include/drm/bridge/dw_mipi_dsi.h
> +++ b/include/drm/bridge/dw_mipi_dsi.h
> @@ -31,6 +31,7 @@ struct dw_mipi_dsi_plat_data {
>  	void *priv_data;
>  };
>  
> +bool dw_mipi_dsi_device_attached(struct dw_mipi_dsi *dsi);
>  struct dw_mipi_dsi *dw_mipi_dsi_probe(struct platform_device *pdev,
>  				      const struct dw_mipi_dsi_plat_data
>  				      *plat_data);