drm_bridges on fairphone-fp3 are enabled in the wrong order

drm_bridges on fairphone-fp3 are enabled in the wrong order

[Bert Karwatzki]

The fairphone-fp3 has a drm_panel attached to a dsi bridge. This are added to
the bridge_chain in struct drm_encoder in the order dsi, panel. When the
drm_atomic_bridge_chain_pre_enable these get enabled in the order panel, dsi
because of the list_for_each_entry_reverse. But the drm_panel of the fairphone-
fp3 is enabled using mipi_dsi_dcs_write_buffer which only works if the dsi is
enabled before the panel.
 To work around this one can revert

commit 9e15123eca7942caa8a3e1f58ec0df7d088df149
Author: Douglas Anderson <dianders@chromium.org>
Date:   Tue Jan 31 14:18:25 2023 -0800

    drm/msm/dsi: Stop unconditionally powering up DSI hosts at modeset

This leads to a working panel on startup. But when suspending one runs again
into a similar problem. When the bridges are disabled the dsi is disabled first
which leads to a failure in disabling the panel again because
mipi_dsi_dcs_write_buffer fails when the dsi is already switched of.
 As a simple workarund for both problems I have found it works to exchange the
order of the bridge chain in drm_endcoder:

diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 28b8012a21f2..990f7c68a27c 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -550,6 +555,8 @@ int msm_dsi_manager_ext_bridge_init(u8 id)
 		if (ret < 0)
 			return ret;
 	}
+	/* swap bridges in list */
+	list_swap(&encoder->bridge_chain, encoder->bridge_chain.next);

 	/* The pipeline is ready, ping encoders if necessary */
 	msm_dsi_manager_set_split_display(id);

But this does not look like a portable solution so I wonder if there is a better
way to do this.

The linux kernel used here is a linux-next-20220630, with several out-of-tree
patches which are needed for the msm8953 gpu and the display used in the
fairphone-fp3 located here: https://github.com/spasswolf/msm8953-linux.git in
branch msm8953_iommu_rebase_v2_wlan_modem_ipa_cpufreq_display_debug.

Bert Karwatzki

Re: drm_bridges on fairphone-fp3 are enabled in the wrong order

[Doug Anderson]

Hi,

On Wed, Jul 5, 2023 at 12:51 AM Bert Karwatzki <spasswolf@web.de> wrote:
>
> The fairphone-fp3 has a drm_panel attached to a dsi bridge. This are added to
> the bridge_chain in struct drm_encoder in the order dsi, panel. When the
> drm_atomic_bridge_chain_pre_enable these get enabled in the order panel, dsi
> because of the list_for_each_entry_reverse. But the drm_panel of the fairphone-
> fp3 is enabled using mipi_dsi_dcs_write_buffer which only works if the dsi is
> enabled before the panel.
>  To work around this one can revert
>
> commit 9e15123eca7942caa8a3e1f58ec0df7d088df149
> Author: Douglas Anderson <dianders@chromium.org>
> Date:   Tue Jan 31 14:18:25 2023 -0800
>
>     drm/msm/dsi: Stop unconditionally powering up DSI hosts at modeset
>
> This leads to a working panel on startup. But when suspending one runs again
> into a similar problem. When the bridges are disabled the dsi is disabled first
> which leads to a failure in disabling the panel again because
> mipi_dsi_dcs_write_buffer fails when the dsi is already switched of.
>  As a simple workarund for both problems I have found it works to exchange the
> order of the bridge chain in drm_endcoder:
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> index 28b8012a21f2..990f7c68a27c 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> @@ -550,6 +555,8 @@ int msm_dsi_manager_ext_bridge_init(u8 id)
>                 if (ret < 0)
>                         return ret;
>         }
> +       /* swap bridges in list */
> +       list_swap(&encoder->bridge_chain, encoder->bridge_chain.next);
>
>         /* The pipeline is ready, ping encoders if necessary */
>         msm_dsi_manager_set_split_display(id);
>
> But this does not look like a portable solution so I wonder if there is a better
> way to do this.
>
> The linux kernel used here is a linux-next-20220630, with several out-of-tree
> patches which are needed for the msm8953 gpu and the display used in the
> fairphone-fp3 located here: https://github.com/spasswolf/msm8953-linux.git in
> branch msm8953_iommu_rebase_v2_wlan_modem_ipa_cpufreq_display_debug.

Any chance that "pre_enable_prev_first" works for you? For the best
summary I'm aware of this issue, see:

https://lore.kernel.org/r/CAD=FV=X_xonf1Dz0BsNTKm4-zBm+ccKvPO+wEWFVMUVY_2=h3Q@mail.gmail.com

-Doug

Re: drm_bridges on fairphone-fp3 are enabled in the wrong order

[Bert Karwatzki]

Am Donnerstag, dem 06.07.2023 um 09:22 -0700 schrieb Doug Anderson:
> Hi,
>
> On Wed, Jul 5, 2023 at 12:51 AM Bert Karwatzki <spasswolf@web.de> wrote:
> >
> > The fairphone-fp3 has a drm_panel attached to a dsi bridge. This are added
> > to
> > the bridge_chain in struct drm_encoder in the order dsi, panel. When the
> > drm_atomic_bridge_chain_pre_enable these get enabled in the order panel, dsi
> > because of the list_for_each_entry_reverse. But the drm_panel of the
> > fairphone-
> > fp3 is enabled using mipi_dsi_dcs_write_buffer which only works if the dsi
> > is
> > enabled before the panel.
> >  To work around this one can revert
> >
> > commit 9e15123eca7942caa8a3e1f58ec0df7d088df149
> > Author: Douglas Anderson <dianders@chromium.org>
> > Date:   Tue Jan 31 14:18:25 2023 -0800
> >
> >     drm/msm/dsi: Stop unconditionally powering up DSI hosts at modeset
> >
> > This leads to a working panel on startup. But when suspending one runs again
> > into a similar problem. When the bridges are disabled the dsi is disabled
> > first
> > which leads to a failure in disabling the panel again because
> > mipi_dsi_dcs_write_buffer fails when the dsi is already switched of.
> >  As a simple workarund for both problems I have found it works to exchange
> > the
> > order of the bridge chain in drm_endcoder:
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > index 28b8012a21f2..990f7c68a27c 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> > @@ -550,6 +555,8 @@ int msm_dsi_manager_ext_bridge_init(u8 id)
> >                 if (ret < 0)
> >                         return ret;
> >         }
> > +       /* swap bridges in list */
> > +       list_swap(&encoder->bridge_chain, encoder->bridge_chain.next);
> >
> >         /* The pipeline is ready, ping encoders if necessary */
> >         msm_dsi_manager_set_split_display(id);
> >
> > But this does not look like a portable solution so I wonder if there is a
> > better
> > way to do this.
> >
> > The linux kernel used here is a linux-next-20220630, with several out-of-
> > tree
> > patches which are needed for the msm8953 gpu and the display used in the
> > fairphone-fp3 located here:
> > https://github.com/spasswolf/msm8953-linux.git in
> > branch msm8953_iommu_rebase_v2_wlan_modem_ipa_cpufreq_display_debug.
>
> Any chance that "pre_enable_prev_first" works for you? For the best
> summary I'm aware of this issue, see:
>
> https://lore.kernel.org/r/CAD=FV=X_xonf1Dz0BsNTKm4-zBm+ccKvPO+wEWFVMUVY_2=h3Q@mail.gmail.com
>
> -Doug

Yes, this is exactly what I needed. To enable pre_enable_prev_first for the
bridge one simply can set panel.prepare_prev_first=true in the probe function of
the panel driver.

Bert Karwatzki