All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiko Stuebner <heiko@sntech.de>
To: Andrzej Hajda <a.hajda@samsung.com>
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	briannorris@chromium.org, hoegsberg@gmail.com,
	philippe.cornu@st.com, dri-devel@lists.freedesktop.org,
	yannick.fertre@st.com, linux-rockchip@lists.infradead.org,
	nickey.yang@rock-chips.com, robh+dt@kernel.org,
	thierry.reding@gmail.com, laurent.pinchart@ideasonboard.com,
	mka@chromium.org
Subject: Re: [PATCH v3 3/8] drm/bridge/synopsys: dsi: add ability to check dsi-device attachment
Date: Mon, 13 Aug 2018 10:44:22 +0200	[thread overview]
Message-ID: <8621122.S3a5QVICDx@phil> (raw)
In-Reply-To: <20180813082842eucas1p1e45c0bf5d61b9d0015a4795f54c565f1~KZFD69J9n0732107321eucas1p1Q@eucas1p1.samsung.com>

Hi Andrzej,

Am Montag, 13. August 2018, 10:28:39 CEST schrieb 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@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

  reply	other threads:[~2018-08-13  8:44 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-09 13:48 [PATCH v3 0/8] drm/rockchip: migrate to common dw-mipi-dsi bridge and dual-dsi Heiko Stuebner
2018-07-09 13:48 ` [PATCH v3 1/8] drm/bridge/synopsys: dsi: move mipi_dsi_host_unregister to __dw_mipi_dsi_remove Heiko Stuebner
2018-07-09 13:48 ` [PATCH v3 2/8] drm/bridge/synopsys: dsi: don't call __dw_mipi_dsi_probe from dw_mipi_dsi_bind Heiko Stuebner
2018-08-13  8:07   ` Andrzej Hajda
2018-07-09 13:48 ` [PATCH v3 3/8] drm/bridge/synopsys: dsi: add ability to check dsi-device attachment Heiko Stuebner
2018-08-13  8:28   ` Andrzej Hajda
2018-08-13  8:44     ` Heiko Stuebner [this message]
2018-08-13 10:23       ` Andrzej Hajda
2018-08-14  7:49         ` Heiko Stuebner
2018-07-09 13:48 ` [PATCH v3 4/8] dt-bindings: display: rockchip: update DSI controller Heiko Stuebner
2018-07-09 13:48 ` [PATCH v3 5/8] drm/rockchip: dsi: migrate to use dw-mipi-dsi bridge driver Heiko Stuebner
2018-07-09 13:48 ` [PATCH v3 6/8] drm/dsi: add helper function to find the second host in a dual-dsi setup Heiko Stuebner
     [not found] ` <20180709134834.11035-1-heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
2018-07-09 13:48   ` [PATCH v3 7/8] drm/bridge/synopsys: dsi: add dual-dsi support Heiko Stuebner
2018-07-09 13:48   ` [PATCH v3 8/8] drm/rockchip: dsi: add dual mipi support Heiko Stuebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8621122.S3a5QVICDx@phil \
    --to=heiko@sntech.de \
    --cc=a.hajda@samsung.com \
    --cc=briannorris@chromium.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hoegsberg@gmail.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=mka@chromium.org \
    --cc=nickey.yang@rock-chips.com \
    --cc=philippe.cornu@st.com \
    --cc=robh+dt@kernel.org \
    --cc=thierry.reding@gmail.com \
    --cc=yannick.fertre@st.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.