On Wed, May 11, 2022 at 2:49 PM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> wrote:
>
> On 11/05/2022 23:06, Doug Anderson wrote:
> > Hi,
> >
> > On Tue, Dec 7, 2021 at 2:29 PM Dmitry Baryshkov
> > <dmitry.baryshkov@xxxxxxxxxx> wrote:
> >>
> >> The DSI subsystem does not fully fall into the pre-enable/enable system
> >> of callbacks, since typically DSI device bridge drivers expect to be
> >> able to communicate with DSI devices at the pre-enable() callback. The
> >> reason is that for some DSI hosts enabling the video stream would
> >> prevent other drivers from sending DSI commands. For example see the
> >> panel-bridge driver, which does drm_panel_prepare() from the
> >> pre_enable() callback (which would be called before our pre_enable()
> >> callback, resulting in panel preparation failures as the link is not yet
> >> ready).
> >>
> >> Therewere several attempts to solve this issue, but currently the best
> >> approach is to power up the DSI link from the mode_set() callback,
> >> allowing next bridge/panel to use DSI transfers in the pre_enable()
> >> time. Follow this approach.
> >>
> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/msm/dsi/dsi_manager.c | 43 +++++++++++++++++++--------
> >> 1 file changed, 31 insertions(+), 12 deletions(-)
> >
> > I happened to be testing today on one of the sc7180-trogdor variants
> > that has a parade-ps8640 bridge chip in it and ran into problems. A
> > bisect pointed to this patch and, sure enough, reverting it fixes me
> > again.
> >
> > The Chromebook in question is able to power the screen on at bootup
> > but things don't work so well in other cases. Specifically, if I leave
> > the Chromebook idle then it will turn the screen off (but in this
> > case, not enter S3). Hitting a key should wake the screen up, but it
> > doesn't.
> >
> > None of the error prints in dsi_mgr_bridge_power_on() are hitting when
> > it fails and I even added extra error prints. It's not hitting any of
> > the early returns.
> >
> > I did a little bit more debugging and it appears that nothing funny is
> > going on. It's just the ordering difference that matters. With the
> > patch reverted, I see this and it all works:
> >
> > boot:
> > [ 9.653801] DOUG: dsi_mgr_bridge_mode_set
> > [ 9.658687] DOUG: ps8640_pre_enable
> > [ 9.664194] DOUG: dsi_mgr_bridge_pre_enable
> >
> > screen turns off:
> > [ 82.130038] DOUG: dsi_mgr_bridge_post_disable
> > [ 82.166817] DOUG: ps8640_post_disable
> >
> > screen turns on:
> > [ 92.611846] DOUG: dsi_mgr_bridge_mode_set
> > [ 92.617875] DOUG: ps8640_pre_enable
> > [ 92.920237] DOUG: dsi_mgr_bridge_pre_enable
> >
> > Without the patch reverted, I see this and it fails:
> >
> > boot:
> > [ 10.817810] DOUG: dsi_mgr_bridge_mode_set
> > [ 10.822128] DOUG: dsi_mgr_bridge_power_on
> > [ 10.852131] DOUG: ps8640_pre_enable
> > [ 10.857942] DOUG: dsi_mgr_bridge_pre_enable
> >
> > screen turns off:
> > [ 34.819953] DOUG: dsi_mgr_bridge_post_disable
> > [ 34.883777] DOUG: ps8640_post_disable
> >
> > screen should turn on, but doesn't:
> > [ 46.193589] DOUG: dsi_mgr_bridge_mode_set
> > [ 46.197951] DOUG: dsi_mgr_bridge_power_on
> > [ 46.248438] DOUG: ps8640_pre_enable
> > [ 46.541700] DOUG: dsi_mgr_bridge_pre_enable
> >
> > Unfortunately, ps8640 is a pretty big black box. The Linux kernel
> > driver does almost nothing at all and the parade bridge chip has a
> > bunch of magic firmware on it. Though I don't know for sure, I assume
> > that this magic firmware simply can't handle the fact that the MIPI
> > side is already setup / running when the bridge is powered on.
> >
> > Rather than this patch, maybe you can jump on Dave Stevenson's patch
> > series [1] which I believe would solve this problem in a more dynamic
> > way? Would you accept a revert of ${SUBJECT} patch to fix my problem?
>
> I'm fine with the revert, but it will depend on [1]. Otherwise other
> MIPI DSI bridges are broken (see the discussion at [2]).
heh, well the problem is that MIPI DSI bridges, or at least one of
them, is broken *without* the revert ;-)
[1] looks like a bit much for -fixes, so I'd be inclined to revert
this patch and at least go back to the broken/working state from
before for the time being..
BR,
-R
> > [1] https://lore.kernel.org/r/cover.1646406653.git.dave.stevenson@xxxxxxxxxxxxxxx
>
> [2]
> https://lore.kernel.org/all/CAPY8ntBrhYAmsraDqJGuTrSL6VjGXBAMVoN7xweV7E4qZv+v3Q@xxxxxxxxxxxxxx/
>
>
> --
> With best wishes
> Dmitry
