On 9/7/21 9:31 AM, Andrzej Hajda wrote:
On 07.09.2021 04:39, Marek Vasut wrote:
In rare cases, the bridge may not start up correctly, which usually
leads to no display output. In case this happens, warn about it in
the kernel log.
Signed-off-by: Marek Vasut <marex@xxxxxxx>
Cc: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
Cc: Robert Foss <robert.foss@xxxxxxxxxx>
Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
---
NOTE: See the following:
https://e2e.ti.com/support/interface-group/interface/f/interface-forum/942005/sn65dsi83-dsi83-lvds-bridge---sporadic-behavior---no-video
https://community.nxp.com/t5/i-MX-Processors/i-MX8M-MIPI-DSI-Interface-LVDS-Bridge-Initialization/td-p/1156533
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index a32f70bc68ea4..4ea71d7f0bfbc 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -520,6 +520,11 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
/* Clear all errors that got asserted during initialization. */
regmap_read(ctx->regmap, REG_IRQ_STAT, &pval);
regmap_write(ctx->regmap, REG_IRQ_STAT, pval);
It does not look as correct error handling, maybe it would be good to
analyze and optionally report 'unexpected' errors here as well.
The above is correct -- it clears the status register because the setup
might've set random bits in that register. Then we wait a bit, let the
link run, and read them again to get the real link status in this new
piece of code below, hence the usleep_range there. And then if the link
indicates a problem, we know it is a problem.
+
+ usleep_range(10000, 12000);
+ regmap_read(ctx->regmap, REG_IRQ_STAT, &pval);
+ if (pval)
+ dev_err(ctx->dev, "Unexpected link status 0x%02x\n", pval);
I am not sure what is the case here but it looks like 'we do not know
what is going on, so let's add some diagnostic messages to gather info
and figure it out later'.
That's pretty much the case, see the two links above in the NOTE
section. If something goes wrong, we print the value for the user
(usually developer) so they can fix their problems. We cannot do much
better in the attach callback.
The issue I ran into (and where this would be helpful information to me
during debugging, since the issue happened real seldom, see also the
NOTE links above) is that the DSI controller driver started streaming
video on the data lanes before the DSI83 had a chance to initialize.
This worked most of the time, except for a few exceptions here and
there, where the video didn't start. This does set link status bits
consistently. In the meantime, I fixed the controller driver (so far
downstream, due to ongoing discussion).
Whole driver lacks IRQ handler which IMO could perform better diagnosis,
and I guess it could also help in recovery, but this is just my guess.
So if this patch is enough for now you can add:
No, IRQ won't help you here, because by the time you get the IRQ, the
DSI host already started streaming video on data lanes and you won't be
able to correctly reinit the DSI83 unless you communicate to the DSI
host that it should switch the data lanes back to LP11.
And for that, there is a bigger chunk missing really. What needs to be
added is a way for the DSI bridge / panel to communicate its needs to
the DSI host -- things like "I need DSI clock lane frequency f MHz, I
need clock lane in HS mode and data lanes in LP11 mode". If you look at
the way DSI hosts and bridges/panels work out the DSI link parameters,
you will notice they basically do it each on their own, there is no such
API or communication channel.
With the above in place, you could have an interrupt which would use it,
notify the host to configure the DSI bus in required way, then you would
be able to reinit the DSI83, and then again notify the DSI host to start
streaming video.
Reviewed-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
Regards
Andrzej
}
static void sn65dsi83_atomic_disable(struct drm_bridge *bridge,
--
Best regards,
Marek Vasut
