mbox series

[v4,00/24] drm/bridge: Make panel and bridge probe order consistent

Message ID 20210910101218.1632297-1-maxime@cerno.tech (mailing list archive)
Headers show
Series drm/bridge: Make panel and bridge probe order consistent | expand

Message

Maxime Ripard Sept. 10, 2021, 10:11 a.m. UTC
Hi,

We've encountered an issue with the RaspberryPi DSI panel that prevented the
whole display driver from probing.

The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
Only register our component once a DSI device is attached"), but the basic idea
is that since the panel is probed through i2c, there's no synchronization
between its probe and the registration of the MIPI-DSI host it's attached to.

We initially moved the component framework registration to the MIPI-DSI Host
attach hook to make sure we register our component only when we have a DSI
device attached to our MIPI-DSI host, and then use lookup our DSI device in our
bind hook.

However, all the DSI bridges controlled through i2c are only registering their
associated DSI device in their bridge attach hook, meaning with our change
above, we never got that far, and therefore ended up in the same situation than
the one we were trying to fix for panels.

The best practice to avoid those issues is to register its functions only after
all its dependencies are live. We also shouldn't wait any longer than we should
to play nice with the other components that are waiting for us, so in our case
that would mean moving the DSI device registration to the bridge probe.

I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
would be affected by this and wouldn't probe anymore after those changes.
Exynos and kirin seems to be simple enough for a mechanical change (that still
requires to be tested), but the changes in msm seemed to be far more important
and I wasn't confortable doing them.

Let me know what you think,
Maxime

---

Changes from v3:
  - Converted exynos and kirin
  - Converted all the affected bridge drivers
  - Reworded the documentation a bit

Changes from v2:
  - Changed the approach as suggested by Andrzej, and aligned the bridge on the
    panel this time.
  - Fixed some typos

Changes from v1:
  - Change the name of drm_of_get_next function to drm_of_get_bridge
  - Mention the revert of 87154ff86bf6 and squash the two patches that were
    reverting that commit
  - Add some documentation
  - Make drm_panel_attach and _detach succeed when no callback is there

Maxime Ripard (24):
  drm/bridge: Add documentation sections
  drm/bridge: Document the probe issue with MIPI-DSI bridges
  drm/mipi-dsi: Create devm device registration
  drm/mipi-dsi: Create devm device attachment
  drm/bridge: adv7533: Switch to devm MIPI-DSI helpers
  drm/bridge: adv7511: Register and attach our DSI device at probe
  drm/bridge: anx7625: Switch to devm MIPI-DSI helpers
  drm/bridge: anx7625: Register and attach our DSI device at probe
  drm/bridge: lt8912b: Switch to devm MIPI-DSI helpers
  drm/bridge: lt8912b: Register and attach our DSI device at probe
  drm/bridge: lt9611: Switch to devm MIPI-DSI helpers
  drm/bridge: lt9611: Register and attach our DSI device at probe
  drm/bridge: lt9611uxc: Switch to devm MIPI-DSI helpers
  drm/bridge: lt9611uxc: Register and attach our DSI device at probe
  drm/bridge: ps8640: Switch to devm MIPI-DSI helpers
  drm/bridge: ps8640: Register and attach our DSI device at probe
  drm/bridge: sn65dsi83: Switch to devm MIPI-DSI helpers
  drm/bridge: sn65dsi83: Register and attach our DSI device at probe
  drm/bridge: sn65dsi86: Switch to devm MIPI-DSI helpers
  drm/bridge: sn65dsi86: Register and attach our DSI device at probe
  drm/bridge: tc358775: Switch to devm MIPI-DSI helpers
  drm/bridge: tc358775: Register and attach our DSI device at probe
  drm/kirin: dsi: Adjust probe order
  drm/exynos: dsi: Adjust probe order

 Documentation/gpu/drm-kms-helpers.rst        |  12 +++
 drivers/gpu/drm/bridge/adv7511/adv7511.h     |   1 -
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c |  15 ++-
 drivers/gpu/drm/bridge/adv7511/adv7533.c     |  20 +---
 drivers/gpu/drm/bridge/analogix/anx7625.c    |  40 ++++----
 drivers/gpu/drm/bridge/lontium-lt8912b.c     |  31 ++----
 drivers/gpu/drm/bridge/lontium-lt9611.c      |  62 +++++-------
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c   |  65 +++++-------
 drivers/gpu/drm/bridge/parade-ps8640.c       | 101 ++++++++++---------
 drivers/gpu/drm/bridge/tc358775.c            |  50 +++++----
 drivers/gpu/drm/bridge/ti-sn65dsi83.c        |  86 ++++++++--------
 drivers/gpu/drm/bridge/ti-sn65dsi86.c        |  94 ++++++++---------
 drivers/gpu/drm/drm_bridge.c                 |  69 ++++++++++++-
 drivers/gpu/drm/drm_mipi_dsi.c               |  81 +++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_dsi.c      |  19 ++--
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c |  27 +++--
 include/drm/drm_mipi_dsi.h                   |   4 +
 17 files changed, 460 insertions(+), 317 deletions(-)

Comments

John Stultz Sept. 29, 2021, 9:27 p.m. UTC | #1
On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> We've encountered an issue with the RaspberryPi DSI panel that prevented the
> whole display driver from probing.
>
> The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
> Only register our component once a DSI device is attached"), but the basic idea
> is that since the panel is probed through i2c, there's no synchronization
> between its probe and the registration of the MIPI-DSI host it's attached to.
>
> We initially moved the component framework registration to the MIPI-DSI Host
> attach hook to make sure we register our component only when we have a DSI
> device attached to our MIPI-DSI host, and then use lookup our DSI device in our
> bind hook.
>
> However, all the DSI bridges controlled through i2c are only registering their
> associated DSI device in their bridge attach hook, meaning with our change
> above, we never got that far, and therefore ended up in the same situation than
> the one we were trying to fix for panels.
>
> The best practice to avoid those issues is to register its functions only after
> all its dependencies are live. We also shouldn't wait any longer than we should
> to play nice with the other components that are waiting for us, so in our case
> that would mean moving the DSI device registration to the bridge probe.
>
> I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> would be affected by this and wouldn't probe anymore after those changes.
> Exynos and kirin seems to be simple enough for a mechanical change (that still
> requires to be tested), but the changes in msm seemed to be far more important
> and I wasn't confortable doing them.


Hey Maxime,
  Sorry for taking so long to get to this, but now that plumbers is
over I've had a chance to check it out on kirin

Rob Clark pointed me to his branch with some fixups here:
   https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework

But trying to boot hikey with that, I see the following loop indefinitely:
[    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
[    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
[    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
[    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
[    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
[    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
[    4.681898] adv7511 2-0039: failed to find dsi host
[    4.688836] adv7511 2-0039: supply avdd not found, using dummy regulator
[    4.695724] adv7511 2-0039: supply dvdd not found, using dummy regulator
[    4.702583] adv7511 2-0039: supply pvdd not found, using dummy regulator
[    4.709369] adv7511 2-0039: supply a2vdd not found, using dummy regulator
[    4.716232] adv7511 2-0039: supply v3p3 not found, using dummy regulator
[    4.722972] adv7511 2-0039: supply v1p2 not found, using dummy regulator
[    4.738720] adv7511 2-0039: failed to find dsi host

I'll have to dig a bit to figure out what's going wrong, but wanted to
give you the heads up that there seems to be a problem

thanks
-john
John Stultz Sept. 29, 2021, 9:32 p.m. UTC | #2
On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
>
> On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > We've encountered an issue with the RaspberryPi DSI panel that prevented the
> > whole display driver from probing.
> >
> > The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
> > Only register our component once a DSI device is attached"), but the basic idea
> > is that since the panel is probed through i2c, there's no synchronization
> > between its probe and the registration of the MIPI-DSI host it's attached to.
> >
> > We initially moved the component framework registration to the MIPI-DSI Host
> > attach hook to make sure we register our component only when we have a DSI
> > device attached to our MIPI-DSI host, and then use lookup our DSI device in our
> > bind hook.
> >
> > However, all the DSI bridges controlled through i2c are only registering their
> > associated DSI device in their bridge attach hook, meaning with our change
> > above, we never got that far, and therefore ended up in the same situation than
> > the one we were trying to fix for panels.
> >
> > The best practice to avoid those issues is to register its functions only after
> > all its dependencies are live. We also shouldn't wait any longer than we should
> > to play nice with the other components that are waiting for us, so in our case
> > that would mean moving the DSI device registration to the bridge probe.
> >
> > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > would be affected by this and wouldn't probe anymore after those changes.
> > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > requires to be tested), but the changes in msm seemed to be far more important
> > and I wasn't confortable doing them.
>
>
> Hey Maxime,
>   Sorry for taking so long to get to this, but now that plumbers is
> over I've had a chance to check it out on kirin
>
> Rob Clark pointed me to his branch with some fixups here:
>    https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
>
> But trying to boot hikey with that, I see the following loop indefinitely:
> [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> [    4.681898] adv7511 2-0039: failed to find dsi host

I just realized Rob's tree is missing the kirin patch. My apologies!
I'll retest and let you know.

thanks
-john
John Stultz Sept. 29, 2021, 9:51 p.m. UTC | #3
On Wed, Sep 29, 2021 at 2:32 PM John Stultz <john.stultz@linaro.org> wrote:
> On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
> > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > > The best practice to avoid those issues is to register its functions only after
> > > all its dependencies are live. We also shouldn't wait any longer than we should
> > > to play nice with the other components that are waiting for us, so in our case
> > > that would mean moving the DSI device registration to the bridge probe.
> > >
> > > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > > would be affected by this and wouldn't probe anymore after those changes.
> > > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > > requires to be tested), but the changes in msm seemed to be far more important
> > > and I wasn't confortable doing them.
> >
> >
> > Hey Maxime,
> >   Sorry for taking so long to get to this, but now that plumbers is
> > over I've had a chance to check it out on kirin
> >
> > Rob Clark pointed me to his branch with some fixups here:
> >    https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> >
> > But trying to boot hikey with that, I see the following loop indefinitely:
> > [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > [    4.681898] adv7511 2-0039: failed to find dsi host
>
> I just realized Rob's tree is missing the kirin patch. My apologies!
> I'll retest and let you know.

Ok, just retested including the kirin patch and unfortunately I'm
still seeing the same thing.  :(

Will dig a bit and let you know when I find more.

thanks
-john
Laurent Pinchart Sept. 29, 2021, 9:56 p.m. UTC | #4
Hi Maxime,

(CC'ing Kieran)

On Fri, Sep 10, 2021 at 12:11:54PM +0200, Maxime Ripard wrote:
> Hi,
> 
> We've encountered an issue with the RaspberryPi DSI panel that prevented the
> whole display driver from probing.
> 
> The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
> Only register our component once a DSI device is attached"), but the basic idea
> is that since the panel is probed through i2c, there's no synchronization
> between its probe and the registration of the MIPI-DSI host it's attached to.
> 
> We initially moved the component framework registration to the MIPI-DSI Host
> attach hook to make sure we register our component only when we have a DSI
> device attached to our MIPI-DSI host, and then use lookup our DSI device in our
> bind hook.
> 
> However, all the DSI bridges controlled through i2c are only registering their
> associated DSI device in their bridge attach hook, meaning with our change
> above, we never got that far, and therefore ended up in the same situation than
> the one we were trying to fix for panels.
> 
> The best practice to avoid those issues is to register its functions only after
> all its dependencies are live. We also shouldn't wait any longer than we should
> to play nice with the other components that are waiting for us, so in our case
> that would mean moving the DSI device registration to the bridge probe.
> 
> I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> would be affected by this and wouldn't probe anymore after those changes.
> Exynos and kirin seems to be simple enough for a mechanical change (that still
> requires to be tested), but the changes in msm seemed to be far more important
> and I wasn't confortable doing them.
> 
> Let me know what you think,

I've tested this series on my RPi CM4-based board, and there's a clear
improvement: the sn65dsi83 now probes successfully !

The downside is that I can now look at a panel that desperately refuses
to display anything. That's a separate issue, but it prevents me from
telling whether this series introduces regressions :-S I'll try to debug
that separately.

Also, Kieran, would you be able to test this with the SN65DSI86 ?

> ---
> 
> Changes from v3:
>   - Converted exynos and kirin
>   - Converted all the affected bridge drivers
>   - Reworded the documentation a bit
> 
> Changes from v2:
>   - Changed the approach as suggested by Andrzej, and aligned the bridge on the
>     panel this time.
>   - Fixed some typos
> 
> Changes from v1:
>   - Change the name of drm_of_get_next function to drm_of_get_bridge
>   - Mention the revert of 87154ff86bf6 and squash the two patches that were
>     reverting that commit
>   - Add some documentation
>   - Make drm_panel_attach and _detach succeed when no callback is there
> 
> Maxime Ripard (24):
>   drm/bridge: Add documentation sections
>   drm/bridge: Document the probe issue with MIPI-DSI bridges
>   drm/mipi-dsi: Create devm device registration
>   drm/mipi-dsi: Create devm device attachment
>   drm/bridge: adv7533: Switch to devm MIPI-DSI helpers
>   drm/bridge: adv7511: Register and attach our DSI device at probe
>   drm/bridge: anx7625: Switch to devm MIPI-DSI helpers
>   drm/bridge: anx7625: Register and attach our DSI device at probe
>   drm/bridge: lt8912b: Switch to devm MIPI-DSI helpers
>   drm/bridge: lt8912b: Register and attach our DSI device at probe
>   drm/bridge: lt9611: Switch to devm MIPI-DSI helpers
>   drm/bridge: lt9611: Register and attach our DSI device at probe
>   drm/bridge: lt9611uxc: Switch to devm MIPI-DSI helpers
>   drm/bridge: lt9611uxc: Register and attach our DSI device at probe
>   drm/bridge: ps8640: Switch to devm MIPI-DSI helpers
>   drm/bridge: ps8640: Register and attach our DSI device at probe
>   drm/bridge: sn65dsi83: Switch to devm MIPI-DSI helpers
>   drm/bridge: sn65dsi83: Register and attach our DSI device at probe
>   drm/bridge: sn65dsi86: Switch to devm MIPI-DSI helpers
>   drm/bridge: sn65dsi86: Register and attach our DSI device at probe
>   drm/bridge: tc358775: Switch to devm MIPI-DSI helpers
>   drm/bridge: tc358775: Register and attach our DSI device at probe
>   drm/kirin: dsi: Adjust probe order
>   drm/exynos: dsi: Adjust probe order
> 
>  Documentation/gpu/drm-kms-helpers.rst        |  12 +++
>  drivers/gpu/drm/bridge/adv7511/adv7511.h     |   1 -
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c |  15 ++-
>  drivers/gpu/drm/bridge/adv7511/adv7533.c     |  20 +---
>  drivers/gpu/drm/bridge/analogix/anx7625.c    |  40 ++++----
>  drivers/gpu/drm/bridge/lontium-lt8912b.c     |  31 ++----
>  drivers/gpu/drm/bridge/lontium-lt9611.c      |  62 +++++-------
>  drivers/gpu/drm/bridge/lontium-lt9611uxc.c   |  65 +++++-------
>  drivers/gpu/drm/bridge/parade-ps8640.c       | 101 ++++++++++---------
>  drivers/gpu/drm/bridge/tc358775.c            |  50 +++++----
>  drivers/gpu/drm/bridge/ti-sn65dsi83.c        |  86 ++++++++--------
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c        |  94 ++++++++---------
>  drivers/gpu/drm/drm_bridge.c                 |  69 ++++++++++++-
>  drivers/gpu/drm/drm_mipi_dsi.c               |  81 +++++++++++++++
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c      |  19 ++--
>  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c |  27 +++--
>  include/drm/drm_mipi_dsi.h                   |   4 +
>  17 files changed, 460 insertions(+), 317 deletions(-)
Rob Clark Sept. 29, 2021, 11:25 p.m. UTC | #5
On Wed, Sep 29, 2021 at 2:51 PM John Stultz <john.stultz@linaro.org> wrote:
>
> On Wed, Sep 29, 2021 at 2:32 PM John Stultz <john.stultz@linaro.org> wrote:
> > On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
> > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > The best practice to avoid those issues is to register its functions only after
> > > > all its dependencies are live. We also shouldn't wait any longer than we should
> > > > to play nice with the other components that are waiting for us, so in our case
> > > > that would mean moving the DSI device registration to the bridge probe.
> > > >
> > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > > > would be affected by this and wouldn't probe anymore after those changes.
> > > > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > > > requires to be tested), but the changes in msm seemed to be far more important
> > > > and I wasn't confortable doing them.
> > >
> > >
> > > Hey Maxime,
> > >   Sorry for taking so long to get to this, but now that plumbers is
> > > over I've had a chance to check it out on kirin
> > >
> > > Rob Clark pointed me to his branch with some fixups here:
> > >    https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > >
> > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > > [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > > [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > > [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > > [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > > [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > > [    4.681898] adv7511 2-0039: failed to find dsi host
> >
> > I just realized Rob's tree is missing the kirin patch. My apologies!
> > I'll retest and let you know.
>
> Ok, just retested including the kirin patch and unfortunately I'm
> still seeing the same thing.  :(
>
> Will dig a bit and let you know when I find more.

Did you have a chance to test it on anything using drm/msm with DSI
panels?  That would at least confirm that I didn't miss anything in
the drm/msm patch to swap the dsi-host vs bridge ordering..

BR,
-R
John Stultz Sept. 29, 2021, 11:25 p.m. UTC | #6
On Wed, Sep 29, 2021 at 4:20 PM Rob Clark <robdclark@gmail.com> wrote:
> On Wed, Sep 29, 2021 at 2:51 PM John Stultz <john.stultz@linaro.org> wrote:
> > On Wed, Sep 29, 2021 at 2:32 PM John Stultz <john.stultz@linaro.org> wrote:
> > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > The best practice to avoid those issues is to register its functions only after
> > > > > all its dependencies are live. We also shouldn't wait any longer than we should
> > > > > to play nice with the other components that are waiting for us, so in our case
> > > > > that would mean moving the DSI device registration to the bridge probe.
> > > > >
> > > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > > > > would be affected by this and wouldn't probe anymore after those changes.
> > > > > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > > > > requires to be tested), but the changes in msm seemed to be far more important
> > > > > and I wasn't confortable doing them.
> > > >
> > > >
> > > > Hey Maxime,
> > > >   Sorry for taking so long to get to this, but now that plumbers is
> > > > over I've had a chance to check it out on kirin
> > > >
> > > > Rob Clark pointed me to his branch with some fixups here:
> > > >    https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > >
> > > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > > [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > > > [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > > > [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > > > [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > > > [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > > > [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > > > [    4.681898] adv7511 2-0039: failed to find dsi host
> > >
> > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > I'll retest and let you know.
> >
> > Ok, just retested including the kirin patch and unfortunately I'm
> > still seeing the same thing.  :(
> >
> > Will dig a bit and let you know when I find more.
>
> Did you have a chance to test it on anything using drm/msm with DSI
> panels?  That would at least confirm that I didn't miss anything in
> the drm/msm patch to swap the dsi-host vs bridge ordering..

I believe Amit(cc'ed) tried to give it a run on his pocof1, but had
some troubles getting it working against a kernel that wasn't
suffering other regressions at the moment.

Amit/Caleb: Any chance one of you could try again w/ these merged to 5.15-rc3?

thanks
-john
John Stultz Sept. 29, 2021, 11:29 p.m. UTC | #7
On Wed, Sep 29, 2021 at 2:51 PM John Stultz <john.stultz@linaro.org> wrote:
>
> On Wed, Sep 29, 2021 at 2:32 PM John Stultz <john.stultz@linaro.org> wrote:
> > On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
> > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > The best practice to avoid those issues is to register its functions only after
> > > > all its dependencies are live. We also shouldn't wait any longer than we should
> > > > to play nice with the other components that are waiting for us, so in our case
> > > > that would mean moving the DSI device registration to the bridge probe.
> > > >
> > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > > > would be affected by this and wouldn't probe anymore after those changes.
> > > > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > > > requires to be tested), but the changes in msm seemed to be far more important
> > > > and I wasn't confortable doing them.
> > >
> > >
> > > Hey Maxime,
> > >   Sorry for taking so long to get to this, but now that plumbers is
> > > over I've had a chance to check it out on kirin
> > >
> > > Rob Clark pointed me to his branch with some fixups here:
> > >    https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > >
> > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > > [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > > [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > > [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > > [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > > [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > > [    4.681898] adv7511 2-0039: failed to find dsi host
> >
> > I just realized Rob's tree is missing the kirin patch. My apologies!
> > I'll retest and let you know.
>
> Ok, just retested including the kirin patch and unfortunately I'm
> still seeing the same thing.  :(
>
> Will dig a bit and let you know when I find more.

Hey Maxime!
  I chased down the issue. The dsi probe code was still calling
drm_of_find_panel_or_bridge() in order to succeed.

I've moved the logic that looks for the bridge into the bridge_init
and with that it seems to work.

Feel free (assuming it looks ok) to fold this change into your kirin patch:
  https://git.linaro.org/people/john.stultz/android-dev.git/commit/?id=4a35ccc4d7a53f68d6d93da3b47e232a7c75b91d

thanks
-john
John Stultz Sept. 30, 2021, 2:30 a.m. UTC | #8
On Wed, Sep 29, 2021 at 4:20 PM Rob Clark <robdclark@gmail.com> wrote:
> On Wed, Sep 29, 2021 at 2:51 PM John Stultz <john.stultz@linaro.org> wrote:
> > On Wed, Sep 29, 2021 at 2:32 PM John Stultz <john.stultz@linaro.org> wrote:
> > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > The best practice to avoid those issues is to register its functions only after
> > > > > all its dependencies are live. We also shouldn't wait any longer than we should
> > > > > to play nice with the other components that are waiting for us, so in our case
> > > > > that would mean moving the DSI device registration to the bridge probe.
> > > > >
> > > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > > > > would be affected by this and wouldn't probe anymore after those changes.
> > > > > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > > > > requires to be tested), but the changes in msm seemed to be far more important
> > > > > and I wasn't confortable doing them.
> > > >
> > > >
> > > > Hey Maxime,
> > > >   Sorry for taking so long to get to this, but now that plumbers is
> > > > over I've had a chance to check it out on kirin
> > > >
> > > > Rob Clark pointed me to his branch with some fixups here:
> > > >    https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > >
> > > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > > [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > > > [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > > > [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > > > [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > > > [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > > > [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > > > [    4.681898] adv7511 2-0039: failed to find dsi host
> > >
> > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > I'll retest and let you know.
> >
> > Ok, just retested including the kirin patch and unfortunately I'm
> > still seeing the same thing.  :(
> >
> > Will dig a bit and let you know when I find more.
>
> Did you have a chance to test it on anything using drm/msm with DSI
> panels?  That would at least confirm that I didn't miss anything in
> the drm/msm patch to swap the dsi-host vs bridge ordering..

Not a dsi panel, but for what it's worth, I did just test the series
with the db845c w/ HDMI using the lt9611 bridge.
So at least that is looking ok.

thanks
-john
Amit Pundir Sept. 30, 2021, 7:49 p.m. UTC | #9
On Thu, 30 Sept 2021 at 04:50, Rob Clark <robdclark@gmail.com> wrote:
>
> On Wed, Sep 29, 2021 at 2:51 PM John Stultz <john.stultz@linaro.org> wrote:
> >
> > On Wed, Sep 29, 2021 at 2:32 PM John Stultz <john.stultz@linaro.org> wrote:
> > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > The best practice to avoid those issues is to register its functions only after
> > > > > all its dependencies are live. We also shouldn't wait any longer than we should
> > > > > to play nice with the other components that are waiting for us, so in our case
> > > > > that would mean moving the DSI device registration to the bridge probe.
> > > > >
> > > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > > > > would be affected by this and wouldn't probe anymore after those changes.
> > > > > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > > > > requires to be tested), but the changes in msm seemed to be far more important
> > > > > and I wasn't confortable doing them.
> > > >
> > > >
> > > > Hey Maxime,
> > > >   Sorry for taking so long to get to this, but now that plumbers is
> > > > over I've had a chance to check it out on kirin
> > > >
> > > > Rob Clark pointed me to his branch with some fixups here:
> > > >    https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > >
> > > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > > [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > > > [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > > > [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > > > [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > > > [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > > > [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > > > [    4.681898] adv7511 2-0039: failed to find dsi host
> > >
> > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > I'll retest and let you know.
> >
> > Ok, just retested including the kirin patch and unfortunately I'm
> > still seeing the same thing.  :(
> >
> > Will dig a bit and let you know when I find more.
>
> Did you have a chance to test it on anything using drm/msm with DSI
> panels?  That would at least confirm that I didn't miss anything in
> the drm/msm patch to swap the dsi-host vs bridge ordering..

Hi, smoke tested
https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
on Pocophone F1 (sdm845 / A630) with v5.15-rc3. I see no obvious
regressions in my limited testing so far including video (youtube)
playback.

>
> BR,
> -R
Caleb Connolly Sept. 30, 2021, 8:20 p.m. UTC | #10
Hi,

On 30/09/2021 20:49, Amit Pundir wrote:
> On Thu, 30 Sept 2021 at 04:50, Rob Clark <robdclark@gmail.com> wrote:
>>
>> On Wed, Sep 29, 2021 at 2:51 PM John Stultz <john.stultz@linaro.org> wrote:
>>>
>>> On Wed, Sep 29, 2021 at 2:32 PM John Stultz <john.stultz@linaro.org> wrote:
>>>> On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
>>>>> On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
>>>>>> The best practice to avoid those issues is to register its functions only after
>>>>>> all its dependencies are live. We also shouldn't wait any longer than we should
>>>>>> to play nice with the other components that are waiting for us, so in our case
>>>>>> that would mean moving the DSI device registration to the bridge probe.
>>>>>>
>>>>>> I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
>>>>>> would be affected by this and wouldn't probe anymore after those changes.
>>>>>> Exynos and kirin seems to be simple enough for a mechanical change (that still
>>>>>> requires to be tested), but the changes in msm seemed to be far more important
>>>>>> and I wasn't confortable doing them.
>>>>>
>>>>>
>>>>> Hey Maxime,
>>>>>    Sorry for taking so long to get to this, but now that plumbers is
>>>>> over I've had a chance to check it out on kirin
>>>>>
>>>>> Rob Clark pointed me to his branch with some fixups here:
>>>>>     https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
>>>>>
>>>>> But trying to boot hikey with that, I see the following loop indefinitely:
>>>>> [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
>>>>> [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
>>>>> [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
>>>>> [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
>>>>> [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
>>>>> [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
>>>>> [    4.681898] adv7511 2-0039: failed to find dsi host
>>>>
>>>> I just realized Rob's tree is missing the kirin patch. My apologies!
>>>> I'll retest and let you know.
>>>
>>> Ok, just retested including the kirin patch and unfortunately I'm
>>> still seeing the same thing.  :(
>>>
>>> Will dig a bit and let you know when I find more.
>>
>> Did you have a chance to test it on anything using drm/msm with DSI
>> panels?  That would at least confirm that I didn't miss anything in
>> the drm/msm patch to swap the dsi-host vs bridge ordering..
> 
> Hi, smoke tested
> https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> on Pocophone F1 (sdm845 / A630) with v5.15-rc3. I see no obvious
> regressions in my limited testing so far including video (youtube)
> playback.
Tested on the OnePlus 6 too booting AOSP, works fine. This *fixes* FBDEV_EMULATION (so we can get a working framebuffer 
console) which was otherwise broken on 5.15.

However it spits out some warnings during boot: https://p.calebs.dev/gucysowyna.yaml


> 
>>
>> BR,
>> -R
Laurent Pinchart Oct. 3, 2021, 1:25 p.m. UTC | #11
Hi Maxime,

On Thu, Sep 30, 2021 at 12:56:02AM +0300, Laurent Pinchart wrote:
> On Fri, Sep 10, 2021 at 12:11:54PM +0200, Maxime Ripard wrote:
> > Hi,
> > 
> > We've encountered an issue with the RaspberryPi DSI panel that prevented the
> > whole display driver from probing.
> > 
> > The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
> > Only register our component once a DSI device is attached"), but the basic idea
> > is that since the panel is probed through i2c, there's no synchronization
> > between its probe and the registration of the MIPI-DSI host it's attached to.
> > 
> > We initially moved the component framework registration to the MIPI-DSI Host
> > attach hook to make sure we register our component only when we have a DSI
> > device attached to our MIPI-DSI host, and then use lookup our DSI device in our
> > bind hook.
> > 
> > However, all the DSI bridges controlled through i2c are only registering their
> > associated DSI device in their bridge attach hook, meaning with our change
> > above, we never got that far, and therefore ended up in the same situation than
> > the one we were trying to fix for panels.
> > 
> > The best practice to avoid those issues is to register its functions only after
> > all its dependencies are live. We also shouldn't wait any longer than we should
> > to play nice with the other components that are waiting for us, so in our case
> > that would mean moving the DSI device registration to the bridge probe.
> > 
> > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > would be affected by this and wouldn't probe anymore after those changes.
> > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > requires to be tested), but the changes in msm seemed to be far more important
> > and I wasn't confortable doing them.
> > 
> > Let me know what you think,
> 
> I've tested this series on my RPi CM4-based board, and there's a clear
> improvement: the sn65dsi83 now probes successfully !
> 
> The downside is that I can now look at a panel that desperately refuses
> to display anything. That's a separate issue, but it prevents me from
> telling whether this series introduces regressions :-S I'll try to debug
> that separately.

I managed to (partly) fix that issue with a few backports from the RPi
kernel, making me confident enough to say

Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

for

drivers/gpu/drm/bridge/ti-sn65dsi83.c
drivers/gpu/drm/drm_bridge.c
drivers/gpu/drm/drm_mipi_dsi.c
include/drm/drm_mipi_dsi.h

> Also, Kieran, would you be able to test this with the SN65DSI86 ?
> 
> > ---
> > 
> > Changes from v3:
> >   - Converted exynos and kirin
> >   - Converted all the affected bridge drivers
> >   - Reworded the documentation a bit
> > 
> > Changes from v2:
> >   - Changed the approach as suggested by Andrzej, and aligned the bridge on the
> >     panel this time.
> >   - Fixed some typos
> > 
> > Changes from v1:
> >   - Change the name of drm_of_get_next function to drm_of_get_bridge
> >   - Mention the revert of 87154ff86bf6 and squash the two patches that were
> >     reverting that commit
> >   - Add some documentation
> >   - Make drm_panel_attach and _detach succeed when no callback is there
> > 
> > Maxime Ripard (24):
> >   drm/bridge: Add documentation sections
> >   drm/bridge: Document the probe issue with MIPI-DSI bridges
> >   drm/mipi-dsi: Create devm device registration
> >   drm/mipi-dsi: Create devm device attachment
> >   drm/bridge: adv7533: Switch to devm MIPI-DSI helpers
> >   drm/bridge: adv7511: Register and attach our DSI device at probe
> >   drm/bridge: anx7625: Switch to devm MIPI-DSI helpers
> >   drm/bridge: anx7625: Register and attach our DSI device at probe
> >   drm/bridge: lt8912b: Switch to devm MIPI-DSI helpers
> >   drm/bridge: lt8912b: Register and attach our DSI device at probe
> >   drm/bridge: lt9611: Switch to devm MIPI-DSI helpers
> >   drm/bridge: lt9611: Register and attach our DSI device at probe
> >   drm/bridge: lt9611uxc: Switch to devm MIPI-DSI helpers
> >   drm/bridge: lt9611uxc: Register and attach our DSI device at probe
> >   drm/bridge: ps8640: Switch to devm MIPI-DSI helpers
> >   drm/bridge: ps8640: Register and attach our DSI device at probe
> >   drm/bridge: sn65dsi83: Switch to devm MIPI-DSI helpers
> >   drm/bridge: sn65dsi83: Register and attach our DSI device at probe
> >   drm/bridge: sn65dsi86: Switch to devm MIPI-DSI helpers
> >   drm/bridge: sn65dsi86: Register and attach our DSI device at probe
> >   drm/bridge: tc358775: Switch to devm MIPI-DSI helpers
> >   drm/bridge: tc358775: Register and attach our DSI device at probe
> >   drm/kirin: dsi: Adjust probe order
> >   drm/exynos: dsi: Adjust probe order
> > 
> >  Documentation/gpu/drm-kms-helpers.rst        |  12 +++
> >  drivers/gpu/drm/bridge/adv7511/adv7511.h     |   1 -
> >  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c |  15 ++-
> >  drivers/gpu/drm/bridge/adv7511/adv7533.c     |  20 +---
> >  drivers/gpu/drm/bridge/analogix/anx7625.c    |  40 ++++----
> >  drivers/gpu/drm/bridge/lontium-lt8912b.c     |  31 ++----
> >  drivers/gpu/drm/bridge/lontium-lt9611.c      |  62 +++++-------
> >  drivers/gpu/drm/bridge/lontium-lt9611uxc.c   |  65 +++++-------
> >  drivers/gpu/drm/bridge/parade-ps8640.c       | 101 ++++++++++---------
> >  drivers/gpu/drm/bridge/tc358775.c            |  50 +++++----
> >  drivers/gpu/drm/bridge/ti-sn65dsi83.c        |  86 ++++++++--------
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c        |  94 ++++++++---------
> >  drivers/gpu/drm/drm_bridge.c                 |  69 ++++++++++++-
> >  drivers/gpu/drm/drm_mipi_dsi.c               |  81 +++++++++++++++
> >  drivers/gpu/drm/exynos/exynos_drm_dsi.c      |  19 ++--
> >  drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c |  27 +++--
> >  include/drm/drm_mipi_dsi.h                   |   4 +
> >  17 files changed, 460 insertions(+), 317 deletions(-)
Maxime Ripard Oct. 13, 2021, 1:45 p.m. UTC | #12
Hi John,

On Wed, Sep 29, 2021 at 04:29:42PM -0700, John Stultz wrote:
> On Wed, Sep 29, 2021 at 2:51 PM John Stultz <john.stultz@linaro.org> wrote:
> >
> > On Wed, Sep 29, 2021 at 2:32 PM John Stultz <john.stultz@linaro.org> wrote:
> > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > The best practice to avoid those issues is to register its functions only after
> > > > > all its dependencies are live. We also shouldn't wait any longer than we should
> > > > > to play nice with the other components that are waiting for us, so in our case
> > > > > that would mean moving the DSI device registration to the bridge probe.
> > > > >
> > > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > > > > would be affected by this and wouldn't probe anymore after those changes.
> > > > > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > > > > requires to be tested), but the changes in msm seemed to be far more important
> > > > > and I wasn't confortable doing them.
> > > >
> > > >
> > > > Hey Maxime,
> > > >   Sorry for taking so long to get to this, but now that plumbers is
> > > > over I've had a chance to check it out on kirin
> > > >
> > > > Rob Clark pointed me to his branch with some fixups here:
> > > >    https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > >
> > > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > > [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > > > [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > > > [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > > > [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > > > [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > > > [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > > > [    4.681898] adv7511 2-0039: failed to find dsi host
> > >
> > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > I'll retest and let you know.
> >
> > Ok, just retested including the kirin patch and unfortunately I'm
> > still seeing the same thing.  :(
> >
> > Will dig a bit and let you know when I find more.
> 
> Hey Maxime!
>   I chased down the issue. The dsi probe code was still calling
> drm_of_find_panel_or_bridge() in order to succeed.
> 
> I've moved the logic that looks for the bridge into the bridge_init
> and with that it seems to work.
> 
> Feel free (assuming it looks ok) to fold this change into your kirin patch:
>   https://git.linaro.org/people/john.stultz/android-dev.git/commit/?id=4a35ccc4d7a53f68d6d93da3b47e232a7c75b91d

Thanks for testing, I've picked and squashed your fixup

Maxime
Maxime Ripard Oct. 13, 2021, 2:16 p.m. UTC | #13
Hi Caleb,

On Thu, Sep 30, 2021 at 09:20:52PM +0100, Caleb Connolly wrote:
> Hi,
> 
> On 30/09/2021 20:49, Amit Pundir wrote:
> > On Thu, 30 Sept 2021 at 04:50, Rob Clark <robdclark@gmail.com> wrote:
> > > 
> > > On Wed, Sep 29, 2021 at 2:51 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > 
> > > > On Wed, Sep 29, 2021 at 2:32 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > > > The best practice to avoid those issues is to register its functions only after
> > > > > > > all its dependencies are live. We also shouldn't wait any longer than we should
> > > > > > > to play nice with the other components that are waiting for us, so in our case
> > > > > > > that would mean moving the DSI device registration to the bridge probe.
> > > > > > > 
> > > > > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > > > > > > would be affected by this and wouldn't probe anymore after those changes.
> > > > > > > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > > > > > > requires to be tested), but the changes in msm seemed to be far more important
> > > > > > > and I wasn't confortable doing them.
> > > > > > 
> > > > > > 
> > > > > > Hey Maxime,
> > > > > >    Sorry for taking so long to get to this, but now that plumbers is
> > > > > > over I've had a chance to check it out on kirin
> > > > > > 
> > > > > > Rob Clark pointed me to his branch with some fixups here:
> > > > > >     https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > > > > 
> > > > > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > > > > [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > > > > > [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > > > > > [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > > > > > [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > > > > > [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > > > > > [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > > > > > [    4.681898] adv7511 2-0039: failed to find dsi host
> > > > > 
> > > > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > > > I'll retest and let you know.
> > > > 
> > > > Ok, just retested including the kirin patch and unfortunately I'm
> > > > still seeing the same thing.  :(
> > > > 
> > > > Will dig a bit and let you know when I find more.
> > > 
> > > Did you have a chance to test it on anything using drm/msm with DSI
> > > panels?  That would at least confirm that I didn't miss anything in
> > > the drm/msm patch to swap the dsi-host vs bridge ordering..
> > 
> > Hi, smoke tested
> > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > on Pocophone F1 (sdm845 / A630) with v5.15-rc3. I see no obvious
> > regressions in my limited testing so far including video (youtube)
> > playback.
> Tested on the OnePlus 6 too booting AOSP, works fine. This *fixes*
> FBDEV_EMULATION (so we can get a working framebuffer console) which was
> otherwise broken on 5.15.
> 
> However it spits out some warnings during boot: https://p.calebs.dev/gucysowyna.yaml

Thanks for testing. It looks like the runtime_pm ordering between the
msm devices changed a bit with the conversion Rob did.

Rob, do you know what could be going on?

Thanks!
Maxime
Rob Clark Oct. 14, 2021, 12:16 a.m. UTC | #14
On Wed, Oct 13, 2021 at 7:16 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi Caleb,
>
> On Thu, Sep 30, 2021 at 09:20:52PM +0100, Caleb Connolly wrote:
> > Hi,
> >
> > On 30/09/2021 20:49, Amit Pundir wrote:
> > > On Thu, 30 Sept 2021 at 04:50, Rob Clark <robdclark@gmail.com> wrote:
> > > >
> > > > On Wed, Sep 29, 2021 at 2:51 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > >
> > > > > On Wed, Sep 29, 2021 at 2:32 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > > > > The best practice to avoid those issues is to register its functions only after
> > > > > > > > all its dependencies are live. We also shouldn't wait any longer than we should
> > > > > > > > to play nice with the other components that are waiting for us, so in our case
> > > > > > > > that would mean moving the DSI device registration to the bridge probe.
> > > > > > > >
> > > > > > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > > > > > > > would be affected by this and wouldn't probe anymore after those changes.
> > > > > > > > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > > > > > > > requires to be tested), but the changes in msm seemed to be far more important
> > > > > > > > and I wasn't confortable doing them.
> > > > > > >
> > > > > > >
> > > > > > > Hey Maxime,
> > > > > > >    Sorry for taking so long to get to this, but now that plumbers is
> > > > > > > over I've had a chance to check it out on kirin
> > > > > > >
> > > > > > > Rob Clark pointed me to his branch with some fixups here:
> > > > > > >     https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > > > > >
> > > > > > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > > > > > [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > > > > > > [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > > > > > > [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > > > > > > [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > > > > > > [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > > > > > > [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > > > > > > [    4.681898] adv7511 2-0039: failed to find dsi host
> > > > > >
> > > > > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > > > > I'll retest and let you know.
> > > > >
> > > > > Ok, just retested including the kirin patch and unfortunately I'm
> > > > > still seeing the same thing.  :(
> > > > >
> > > > > Will dig a bit and let you know when I find more.
> > > >
> > > > Did you have a chance to test it on anything using drm/msm with DSI
> > > > panels?  That would at least confirm that I didn't miss anything in
> > > > the drm/msm patch to swap the dsi-host vs bridge ordering..
> > >
> > > Hi, smoke tested
> > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > on Pocophone F1 (sdm845 / A630) with v5.15-rc3. I see no obvious
> > > regressions in my limited testing so far including video (youtube)
> > > playback.
> > Tested on the OnePlus 6 too booting AOSP, works fine. This *fixes*
> > FBDEV_EMULATION (so we can get a working framebuffer console) which was
> > otherwise broken on 5.15.
> >
> > However it spits out some warnings during boot: https://p.calebs.dev/gucysowyna.yaml
>
> Thanks for testing. It looks like the runtime_pm ordering between the
> msm devices changed a bit with the conversion Rob did.
>
> Rob, do you know what could be going on?
>

Not entirely sure.. I didn't see that first splat, but maybe I was
missing some debug config? (The 2nd one is kind of "normal", I think
related to bootloader leaving the display on)

BR,
-R
Maxime Ripard Oct. 18, 2021, 12:34 p.m. UTC | #15
Hi Rob,

On Wed, Oct 13, 2021 at 05:16:58PM -0700, Rob Clark wrote:
> On Wed, Oct 13, 2021 at 7:16 AM Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi Caleb,
> >
> > On Thu, Sep 30, 2021 at 09:20:52PM +0100, Caleb Connolly wrote:
> > > Hi,
> > >
> > > On 30/09/2021 20:49, Amit Pundir wrote:
> > > > On Thu, 30 Sept 2021 at 04:50, Rob Clark <robdclark@gmail.com> wrote:
> > > > >
> > > > > On Wed, Sep 29, 2021 at 2:51 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > > >
> > > > > > On Wed, Sep 29, 2021 at 2:32 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > > > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > > > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > > > > > The best practice to avoid those issues is to register its functions only after
> > > > > > > > > all its dependencies are live. We also shouldn't wait any longer than we should
> > > > > > > > > to play nice with the other components that are waiting for us, so in our case
> > > > > > > > > that would mean moving the DSI device registration to the bridge probe.
> > > > > > > > >
> > > > > > > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > > > > > > > > would be affected by this and wouldn't probe anymore after those changes.
> > > > > > > > > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > > > > > > > > requires to be tested), but the changes in msm seemed to be far more important
> > > > > > > > > and I wasn't confortable doing them.
> > > > > > > >
> > > > > > > >
> > > > > > > > Hey Maxime,
> > > > > > > >    Sorry for taking so long to get to this, but now that plumbers is
> > > > > > > > over I've had a chance to check it out on kirin
> > > > > > > >
> > > > > > > > Rob Clark pointed me to his branch with some fixups here:
> > > > > > > >     https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > > > > > >
> > > > > > > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > > > > > > [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > > > > > > > [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > > > > > > > [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > > > > > > > [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > > > > > > > [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > > > > > > > [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > > > > > > > [    4.681898] adv7511 2-0039: failed to find dsi host
> > > > > > >
> > > > > > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > > > > > I'll retest and let you know.
> > > > > >
> > > > > > Ok, just retested including the kirin patch and unfortunately I'm
> > > > > > still seeing the same thing.  :(
> > > > > >
> > > > > > Will dig a bit and let you know when I find more.
> > > > >
> > > > > Did you have a chance to test it on anything using drm/msm with DSI
> > > > > panels?  That would at least confirm that I didn't miss anything in
> > > > > the drm/msm patch to swap the dsi-host vs bridge ordering..
> > > >
> > > > Hi, smoke tested
> > > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > > on Pocophone F1 (sdm845 / A630) with v5.15-rc3. I see no obvious
> > > > regressions in my limited testing so far including video (youtube)
> > > > playback.
> > > Tested on the OnePlus 6 too booting AOSP, works fine. This *fixes*
> > > FBDEV_EMULATION (so we can get a working framebuffer console) which was
> > > otherwise broken on 5.15.
> > >
> > > However it spits out some warnings during boot: https://p.calebs.dev/gucysowyna.yaml
> >
> > Thanks for testing. It looks like the runtime_pm ordering between the
> > msm devices changed a bit with the conversion Rob did.
> >
> > Rob, do you know what could be going on?
> >
> 
> Not entirely sure.. I didn't see that first splat, but maybe I was
> missing some debug config? (The 2nd one is kind of "normal", I think
> related to bootloader leaving the display on)

So do you feel like this is a blocker or do you expect it to be fixed
sometime down the road?

Maxime
Rob Clark Oct. 18, 2021, 3:55 p.m. UTC | #16
On Mon, Oct 18, 2021 at 5:34 AM Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi Rob,
>
> On Wed, Oct 13, 2021 at 05:16:58PM -0700, Rob Clark wrote:
> > On Wed, Oct 13, 2021 at 7:16 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > >
> > > Hi Caleb,
> > >
> > > On Thu, Sep 30, 2021 at 09:20:52PM +0100, Caleb Connolly wrote:
> > > > Hi,
> > > >
> > > > On 30/09/2021 20:49, Amit Pundir wrote:
> > > > > On Thu, 30 Sept 2021 at 04:50, Rob Clark <robdclark@gmail.com> wrote:
> > > > > >
> > > > > > On Wed, Sep 29, 2021 at 2:51 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > > > >
> > > > > > > On Wed, Sep 29, 2021 at 2:32 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > > > > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz <john.stultz@linaro.org> wrote:
> > > > > > > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard <maxime@cerno.tech> wrote:
> > > > > > > > > > The best practice to avoid those issues is to register its functions only after
> > > > > > > > > > all its dependencies are live. We also shouldn't wait any longer than we should
> > > > > > > > > > to play nice with the other components that are waiting for us, so in our case
> > > > > > > > > > that would mean moving the DSI device registration to the bridge probe.
> > > > > > > > > >
> > > > > > > > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin and msm
> > > > > > > > > > would be affected by this and wouldn't probe anymore after those changes.
> > > > > > > > > > Exynos and kirin seems to be simple enough for a mechanical change (that still
> > > > > > > > > > requires to be tested), but the changes in msm seemed to be far more important
> > > > > > > > > > and I wasn't confortable doing them.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Hey Maxime,
> > > > > > > > >    Sorry for taking so long to get to this, but now that plumbers is
> > > > > > > > > over I've had a chance to check it out on kirin
> > > > > > > > >
> > > > > > > > > Rob Clark pointed me to his branch with some fixups here:
> > > > > > > > >     https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > > > > > > >
> > > > > > > > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > > > > > > > [    4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > > > > > > > > [    4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > > > > > > > > [    4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > > > > > > > > [    4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > > > > > > > > [    4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > > > > > > > > [    4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > > > > > > > > [    4.681898] adv7511 2-0039: failed to find dsi host
> > > > > > > >
> > > > > > > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > > > > > > I'll retest and let you know.
> > > > > > >
> > > > > > > Ok, just retested including the kirin patch and unfortunately I'm
> > > > > > > still seeing the same thing.  :(
> > > > > > >
> > > > > > > Will dig a bit and let you know when I find more.
> > > > > >
> > > > > > Did you have a chance to test it on anything using drm/msm with DSI
> > > > > > panels?  That would at least confirm that I didn't miss anything in
> > > > > > the drm/msm patch to swap the dsi-host vs bridge ordering..
> > > > >
> > > > > Hi, smoke tested
> > > > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > > > on Pocophone F1 (sdm845 / A630) with v5.15-rc3. I see no obvious
> > > > > regressions in my limited testing so far including video (youtube)
> > > > > playback.
> > > > Tested on the OnePlus 6 too booting AOSP, works fine. This *fixes*
> > > > FBDEV_EMULATION (so we can get a working framebuffer console) which was
> > > > otherwise broken on 5.15.
> > > >
> > > > However it spits out some warnings during boot: https://p.calebs.dev/gucysowyna.yaml
> > >
> > > Thanks for testing. It looks like the runtime_pm ordering between the
> > > msm devices changed a bit with the conversion Rob did.
> > >
> > > Rob, do you know what could be going on?
> > >
> >
> > Not entirely sure.. I didn't see that first splat, but maybe I was
> > missing some debug config? (The 2nd one is kind of "normal", I think
> > related to bootloader leaving the display on)
>
> So do you feel like this is a blocker or do you expect it to be fixed
> sometime down the road?

No

I can try and take a look at the 2nd splat, but shouldn't block your series

BR,
-R