Hi Alexander,
The FORCE_STOP_STATE bit is unsuitable to force the DSI link into
LP-11
mode. It seems the bridge internally queues DSI packets and when the
FORCE_STOP_STATE bit is cleared, they are sent in close succession
without any useful timing (this also means that the DSI lanes won't go
into LP-11 mode). The length of this gibberish varies between 1ms and
5ms. This sometimes breaks an attached bridge (TI SN65DSI84 in this
case). In our case, the bridge will fail in about 1 per 500 reboots.
The FORCE_STOP_STATE handling was introduced to have the DSI lanes in
LP-11 state during the .pre_enable phase. But as it turns out, none of
this is needed at all. Between samsung_dsim_init() and
samsung_dsim_set_display_enable() the lanes are already in LP-11 mode.
Apparently LP-11 is actually entered with the call to
samsung_dsim_enable_lane(), but I don't know about other requisites on
that
matter. Unfortunately documentation lacks a lot in that regard.
Which is called during samsung_dsim_atomic_pre_enable(). So I'm not sure
why the FORCE_STOP_STATE was needed at all. Lanes will be in LP-11 mode
if the video stream isn't enabled.
The code as it was before commit 20c827683de0 ("drm: bridge:
samsung-dsim: Fix init during host transfer") and 0c14d3130654 ("drm:
bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec") was
correct
in this regard.
This patch basically reverts both commits. It was tested on an i.MX8M
SoC with an SN65DSI84 bridge. The signals were probed and the DSI
packets were decoded during initialization and link start-up. After
this
patch the first DSI packet on the link is a VSYNC packet and the
timing
is correct.
At which point does SN65DSI84 require LP-11?
I guess I have the same requirement as Frieder (we use the same bridge).
According to the datasheet, the DSI data must be in LP-11 when releasing
EN. According to the init sequence:
- asserting EN
- configure CSRs
- enable video stream
Although not, stated explicitly, LP-11 should also be active during CSR
writes.
But after all, the DSIM driver should adhere to the linux requirement,
which Frieder cited [1].
You have access to a DSI/D-PHY analyzer?
A pretty fast oscilloscope with an DSI decoder. So we can analyze one
DSI
lane.
Command mode between .pre_enable and .enable was also briefly tested
by
a quick hack. There was no DSI link partner which would have
responded,
but it was made sure the DSI packet was send on the link. As a side
note, the command mode seems to just work in HS mode. I couldn't find
that the bridge will handle commands in LP mode.
AFAIK ti-sn65dsi83.c only uses I2C for communication. Did you send DSI
read/
writes instead?
Yeah the bridge only supports I2C. I just wanted to try that a DSI
command
will still work after this patch. As a quick hack, I just added an
mipi_dsi_generic_write() to sn65dsi83_atomic_pre_enable() and made sure
there is a DSI write packet on the actual link.
-michael
best regards,
Alexander
Fixes: 20c827683de0 ("drm: bridge: samsung-dsim: Fix init during host
transfer") Fixes: 0c14d3130654 ("drm: bridge: samsung-dsim: Fix i.MX8M
enable flow to meet spec") Signed-off-by: Michael Walle
<mwalle@xxxxxxxxxx>
---
Let me know wether this should be two commits each reverting one, but
both
commits appeared first in kernel 6.5.
[1]
https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
