Hi,
On Tue, Feb 4, 2025 at 7:01 AM Maxime Ripard <mripard@xxxxxxxxxx> wrote:
>
> The TI sn65dsi86 driver follows the drm_encoder->crtc pointer that is
> deprecated and shouldn't be used by atomic drivers.
>
> This was due to the fact that we did't have any other alternative to
> retrieve the CRTC pointer. Fortunately, the crtc pointer is now provided
> in the bridge state, so we can move to atomic callbacks and drop that
> deprecated pointer usage.
>
> Signed-off-by: Maxime Ripard <mripard@xxxxxxxxxx>
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 50 ++++++++++++++++++++++-------------
> 1 file changed, 32 insertions(+), 18 deletions(-)
I'm about out of time for now, but I finally managed to at least test
this and can confirm it _doesn't_ work. If I take the rest of the
series without this patch then things seem OK. When I add this patch
then the splash screen on my Chromebook comes up but the browser never
boots. :(
> @@ -374,12 +377,15 @@ static int __maybe_unused ti_sn65dsi86_resume(struct device *dev)
> * panel (including the aux channel) w/out any need for an input clock
> * so we can do it in resume which lets us read the EDID before
> * pre_enable(). Without a reference clock we need the MIPI reference
> * clock so reading early doesn't work.
> */
> - if (pdata->refclk)
> - ti_sn65dsi86_enable_comms(pdata);
> + if (pdata->refclk) {
> + drm_modeset_lock(&pdata->bridge.base.lock, NULL);
> + ti_sn65dsi86_enable_comms(pdata, drm_bridge_get_current_state(&pdata->bridge));
> + drm_modeset_unlock(&pdata->bridge.base.lock);
> + }
I believe grabbing the locks here is the problem. Sure enough,
commenting that out fixes things. Also, if I wait long enough:
[ 247.151951] INFO: task DrmThread:1838 blocked for more than 122 seconds.
[ 247.158862] Tainted: G W
6.14.0-rc1-00226-g4144859f9421 #1
[ 247.166474] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[ 247.174541] task:DrmThread state:D stack:0 pid:1838
tgid:1756 ppid:1 task_flags:0x400040 flags:0x00000a0d
[ 247.185904] Call trace:
[ 247.188450] __switch_to+0x12c/0x1e0 (T)
[ 247.192520] __schedule+0x2d0/0x4a0
[ 247.196132] schedule_preempt_disabled+0x50/0x88
[ 247.200904] __ww_mutex_lock+0x3d8/0xa68
[ 247.204970] __ww_mutex_lock_slowpath+0x24/0x38
[ 247.209653] ww_mutex_lock+0x7c/0x140
[ 247.213441] drm_modeset_lock+0xd4/0x110
[ 247.217493] ti_sn65dsi86_resume+0x78/0xe0
[ 247.221730] __rpm_callback+0x84/0x148
[ 247.225619] rpm_callback+0x34/0x98
[ 247.229232] rpm_resume+0x320/0x488
[ 247.232842] __pm_runtime_resume+0x54/0xa8
[ 247.237073] ti_sn_bridge_gpio_get+0x48/0xb8
[ 247.241486] gpiod_get_raw_value_commit+0x70/0x178
[ 247.246436] gpiod_get_value_cansleep+0x34/0x88
[ 247.251122] panel_edp_resume+0xf0/0x270
[ 247.255187] __rpm_callback+0x84/0x148
[ 247.259072] rpm_callback+0x34/0x98
[ 247.262685] rpm_resume+0x320/0x488
[ 247.266293] __pm_runtime_resume+0x54/0xa8
[ 247.270536] panel_edp_prepare+0x2c/0x68
[ 247.274591] drm_panel_prepare+0x54/0x118
[ 247.278743] panel_bridge_atomic_pre_enable+0x60/0x78
[ 247.283965] drm_atomic_bridge_chain_pre_enable+0x110/0x168
[ 247.289723] drm_atomic_helper_commit_modeset_enables+0x204/0x288
[ 247.296005] msm_atomic_commit_tail+0x1b4/0x510
[ 247.300690] commit_tail+0xa8/0x178
[ 247.304298] drm_atomic_helper_commit+0xec/0x180
[ 247.309066] drm_atomic_commit+0xa8/0xf8
[ 247.313125] drm_mode_atomic_ioctl+0x718/0xcd8
[ 247.317717] drm_ioctl+0x1ec/0x450
[ 247.321248] __arm64_sys_ioctl+0x3e4/0x4d8
[ 247.325494] invoke_syscall+0x4c/0xf0
[ 247.329284] do_el0_svc+0x70/0xf8
[ 247.332717] el0_svc+0x38/0x68
[ 247.335886] el0t_64_sync_handler+0x20/0x128
[ 247.340296] el0t_64_sync+0x1b0/0x1b8
I guess the problem is that the HPD gpio (which is given to the panel)
is implemented by ti-sn65dsi86. It's been a long time, but probably we
don't need to "enable comms" just to access a GPIO, but there's only
one level of runtime PM. Maybe the fix would be to separately enable
pm_runtime for the various sub-devices and the GPIO? ...and then the
"aux" channel enables comms and the bridge one also grabs a PM runtime
reference to the aux sub-device? Not sure I have time to dig into that
myself now.
