Message-ID: <20250618-silent-oriole-of-fertility-3cbbd6@houat>
Date: Wed, 18 Jun 2025 10:27:34 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Marek Szyprowski <m.szyprowski@...sung.com>
Cc: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, 
	Aradhya Bhatia <aradhya.bhatia@...ux.dev>, DRI Development List <dri-devel@...ts.freedesktop.org>, 
	Linux Kernel List <linux-kernel@...r.kernel.org>, Nishanth Menon <nm@...com>, 
	Vignesh Raghavendra <vigneshr@...com>, Devarsh Thakkar <devarsht@...com>, 
	Jayesh Choudhary <j-choudhary@...com>, Alexander Sverdlin <alexander.sverdlin@...mens.com>, 
	Dmitry Baryshkov <lumag@...nel.org>, Andrzej Hajda <andrzej.hajda@...el.com>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, 
	Laurent Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman <jonas@...boo.se>, 
	Jernej Skrabec <jernej.skrabec@...il.com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>
Subject: Re: [PATCH v13 3/4] drm/atomic-helper: Re-order bridge chain
 pre-enable and post-disable

On Wed, Jun 18, 2025 at 08:30:39AM +0200, Marek Szyprowski wrote:
> On 16.06.2025 17:40, Tomi Valkeinen wrote:
> > On 12/06/2025 09:31, Marek Szyprowski wrote:
> >> On 12.06.2025 07:49, Tomi Valkeinen wrote:
> >>> On 11/06/2025 13:45, Marek Szyprowski wrote:
> >>>> On 05.06.2025 19:15, Aradhya Bhatia wrote:
> >>>>> From: Aradhya Bhatia <a-bhatia1@...com>
> >>>>>
> >>>>> Move the bridge pre_enable call before crtc enable, and the bridge
> >>>>> post_disable call after the crtc disable.
> >>>>>
> >>>>> The sequence of enable after this patch will look like:
> >>>>>
> >>>>> 	bridge[n]_pre_enable
> >>>>> 	...
> >>>>> 	bridge[1]_pre_enable
> >>>>>
> >>>>> 	crtc_enable
> >>>>> 	encoder_enable
> >>>>>
> >>>>> 	bridge[1]_enable
> >>>>> 	...
> >>>>> 	bridge[n]_enable
> >>>>>
> >>>>> And, the disable sequence for the display pipeline will look like:
> >>>>>
> >>>>> 	bridge[n]_disable
> >>>>> 	...
> >>>>> 	bridge[1]_disable
> >>>>>
> >>>>> 	encoder_disable
> >>>>> 	crtc_disable
> >>>>>
> >>>>> 	bridge[1]_post_disable
> >>>>> 	...
> >>>>> 	bridge[n]_post_disable
> >>>>>
> >>>>> The definition of bridge pre_enable hook says that,
> >>>>> "The display pipe (i.e. clocks and timing signals) feeding this bridge
> >>>>> will not yet be running when this callback is called".
> >>>>>
> >>>>> Since CRTC is also a source feeding the bridge, it should not be enabled
> >>>>> before the bridges in the pipeline are pre_enabled. Fix that by
> >>>>> re-ordering the sequence of bridge pre_enable and bridge post_disable.
> >>>>>
> >>>>> While at it, update the drm bridge API documentation as well.
> >>>>>
> >>>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> >>>>> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> >>>>> Reviewed-by: Thomas Zimmermann <tzimmermann@...e.de>
> >>>>> Tested-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> >>>>> Tested-by: Alexander Sverdlin <alexander.sverdlin@...mens.com>
> >>>>> Signed-off-by: Aradhya Bhatia <a-bhatia1@...com>
> >>>>> Signed-off-by: Aradhya Bhatia <aradhya.bhatia@...ux.dev>
> >>>> This patch landed in today's linux-next as commit c9b1150a68d9
> >>>> ("drm/atomic-helper: Re-order bridge chain pre-enable and
> >>>> post-disable"). In my tests I found that it breaks booting of Samsung
> >>>> Exynos 5420/5800 based Chromebooks (Peach-Pit and Peach-Pi). Both of
> >>>> them use Exynos DRM with Exynos_DP sub-driver (Analogix DP) and EDP
> >>>> panel. Booting stops at '[drm] Initialized exynos 1.1.0 for exynos-drm
> >>>> on minor 0' message. On the other hand, the Samsung Exynos5250 based
> >>>> Snow Chromebook boots fine, but it uses dp-lvds nxp,ptn3460 bridge and
> >>>> lvds panel instead of edp panels. This looks like some sort of deadlock,
> >>>> because if I disable FBDEV emulation, those boards boots fine and I'm
> >>>> able to run modetest and enable the display. Also the DRM kernel logger
> >>>> seems to be working fine, although I didn't check the screen output yet,
> >>>> as I only have a remote access to those boards. I will investigate it
> >>>> further and let You know.
> >>> Thanks for the report. I was trying to understand the pipeline, but I'm
> >>> a bit confused. Above you say Peach-Pit uses DP and EDP panel, but if I
> >>> look at arch/arm/boot/dts/samsung/exynos5420-peach-pit.dts, it connects
> >>> a dp->lvds bridge (parade,ps8625). Peach-Pi seems to connect to an eDP
> >>> panel.
> >>>
> >>> Is the above correct? Do both Peach-Pi and Peach-Pit fail?
> >> Yes, sorry, my fault. I much have checked the same (peach-pi) dts 2
> >> times. Both Peach-Pi and Peach-Pit fails, while Snow works fine. All
> >> three use the same Exynos DP (based on analogix dp) driver. I will try
> >> to play a bit more with those boards in the afternoon, hopefully getting
> >> some more hints where the issue is.
> > Did you get a chance to test this more? Any hints what happens will help =)
> 
> I've spent some time debugging this issue, but so far I only got 
> something I don't really understand. This issue is somehow related with 
> the DP clock enabling and disabling, what is being done from 
> exynos_dp_poweron() and exynos_dp_poweroff() functions, which are called 
> from analogix_dp_resume() and analogix_dp_suspend(). The lockup happens 
> somewhere while registering the fbdev console, with console lock held, 
> what makes debugging much harder.

You can skip the locking part with the fb.lockless_register_fb=1 kernel
parameter. It's of course not meant for anything but debugging, but it's
useful :)

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)

Powered by blists - more mailing lists