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@xxxxxxxxx>
> ---
> 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);
