Hi, On Tue, Oct 12, 2021 at 10:47:14PM +0200, Guido Günther wrote:
Hi Laurent, On Tue, Oct 12, 2021 at 11:17:07PM +0300, Laurent Pinchart wrote:
Hi Guido,
Thank you for the patch.
On Tue, Oct 12, 2021 at 09:58:58PM +0200, Guido Günther wrote:
Otherwise logs are filled with
[drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@30800000/mipi-dsi@30a0 0000 to encoder None-34: -517
when the bridge isn't ready yet.
Fixes: fb8d617f8fd6 ("drm/bridge: Centralize error message when bridge attach fails") Signed-off-by: Guido Günther agx@sigxcpu.org
drivers/gpu/drm/drm_bridge.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index a8ed66751c2d..f0508e85ae98 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -227,14 +227,15 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge, bridge->encoder = NULL; list_del(&bridge->chain_node);
- if (ret != -EPROBE_DEFER) {
#ifdef CONFIG_OF
- DRM_ERROR("failed to attach bridge %pOF to encoder %s: %d\n",
bridge->of_node, encoder->name, ret);
DRM_ERROR("failed to attach bridge %pOF to encoder %s: %d\n",
bridge->of_node, encoder->name, ret);
#else
- DRM_ERROR("failed to attach bridge to encoder %s: %d\n",
encoder->name, ret);
DRM_ERROR("failed to attach bridge to encoder %s: %d\n",
encoder->name, ret);
#endif
- }
This looks fine as such, but I'm concerned about the direction it's taking. Ideally, probe deferral should happen at probe time, way before the bridge is attached. Doing otherwise is a step in the wrong direction in my opinion, and something we'll end up regretting when we'll feel the pain it inflicts.
The particular case I'm seeing this is the nwl driver probe deferrals if the panel bridge isn't ready (which needs a bunch of components (dsi, panel, backlight wrapped led, ...) and it probes fine later on so I wonder where you see the actual error cause? That downstream of the bridge isn't ready or that the display controller is already attaching the bridge?
I should add that mxsfb does a `dev_err_probe()` already when checking the return value of `drm_bridge_attach()` so the error printed is triggered by the additional check added in the above function while the code path already ignored -EPROBE_DEFER before. This looks sensible to me since upper layers can't known when all the downstream bridges are done probing or am I missing something?
Cheers, -- Guido
Cheers, -- Guido
return ret; } EXPORT_SYMBOL(drm_bridge_attach);
-- Regards,
Laurent Pinchart