On Fri, Feb 07, 2025 at 05:44:38PM -0800, Doug Anderson wrote:
> 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. :(
Thanks for testing still :)
Could you add your tested-by on the previous patches if you found that
they were working?
> > @@ -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.
I don't know the hardware, so I can't really comment, unfortunately.
I'll drop it if it's broken.
Maxime
Attachment:
signature.asc
Description: PGP signature
