diff mbox

[v2,3/8] drm/bridge/synopsys: dsi: defer probing if panel not available in bridge-attach

Message ID 20180618102806.15650-4-heiko@sntech.de (mailing list archive)
State New, archived
Headers show

Commit Message

Heiko Stübner June 18, 2018, 10:28 a.m. UTC
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

So emit a -EPROBE_DEFER in that case to give the driver another
chance at getting the display later.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Philippe CORNU July 3, 2018, 11:57 a.m. UTC | #1
Hi Heiko,

On 06/18/2018 12:28 PM, 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
> 
> So emit a -EPROBE_DEFER in that case to give the driver another
> chance at getting the display later.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>   drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 3 +++
>   1 file changed, 3 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..bd503f000ed5 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -835,6 +835,9 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge)
>   		return -ENODEV;
>   	}
>   
> +	if (!dsi->panel_bridge)
> +		return -EPROBE_DEFER;
> +

Thank you for your patch,

Reviewed-by: Philippe Cornu <philippe.cornu@st.com>
Tested-by: Philippe Cornu <philippe.cornu@st.com>

Philippe :-)

>   	/* Set the encoder type as caller does not know it */
>   	bridge->encoder->encoder_type = DRM_MODE_ENCODER_DSI;
>   
>
Andrzej Hajda July 3, 2018, 12:42 p.m. UTC | #2
On 18.06.2018 12:28, 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
>
> So emit a -EPROBE_DEFER in that case to give the driver another
> chance at getting the display later.

Thats odd (at least for me), if the resource (panel in this case) is not
present, bridge should not be attached (drm_bridge_attach should not be
called). Ie __dw_mipi_dsi_probe should rather look for the panel and
defer if not found. Resource gathering is the task of probe.
drm_bridge_attach can be called after probe and it would be difficult
for the caller to react properly for -EPROBE_DEFER error.

Regards
Andrzej

>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 3 +++
>  1 file changed, 3 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..bd503f000ed5 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -835,6 +835,9 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge)
>  		return -ENODEV;
>  	}
>  
> +	if (!dsi->panel_bridge)
> +		return -EPROBE_DEFER;
> +
>  	/* Set the encoder type as caller does not know it */
>  	bridge->encoder->encoder_type = DRM_MODE_ENCODER_DSI;
>
Heiko Stübner July 4, 2018, 10:36 a.m. UTC | #3
Am Dienstag, 3. Juli 2018, 14:42:49 CEST schrieb Andrzej Hajda:
> On 18.06.2018 12:28, 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
> >
> > So emit a -EPROBE_DEFER in that case to give the driver another
> > chance at getting the display later.
> 
> Thats odd (at least for me), if the resource (panel in this case) is not
> present, bridge should not be attached (drm_bridge_attach should not be
> called). Ie __dw_mipi_dsi_probe should rather look for the panel and
> defer if not found. Resource gathering is the task of probe.
> drm_bridge_attach can be called after probe and it would be difficult
> for the caller to react properly for -EPROBE_DEFER error.

I think one of the problems comes from the fact that the panel only
gets probed (and even the panel-device created) after the dsi host bus
gets created in __dw_mipi_dsi_probe.


In hindsight, I think this is more of a concurrency issue between the
panel probing and calling dsi-attach and the dsi-controller looking for
the panel vs. panel-bridge.

So when I'm using drm_of_find_panel_or_bridge() as indicator of the
panel being, it probably makes things racy. I'll try to find a better
solution for the issue.


Heiko
diff mbox

Patch

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index bb4aeca5c0f9..bd503f000ed5 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -835,6 +835,9 @@  static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge)
 		return -ENODEV;
 	}
 
+	if (!dsi->panel_bridge)
+		return -EPROBE_DEFER;
+
 	/* Set the encoder type as caller does not know it */
 	bridge->encoder->encoder_type = DRM_MODE_ENCODER_DSI;