[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230117025122.jt3wrjkqfnogu4ci@builder.lan>
Date: Mon, 16 Jan 2023 20:51:22 -0600
From: Bjorn Andersson <andersson@...nel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Bjorn Andersson <quic_bjorande@...cinc.com>,
Rob Clark <robdclark@...il.com>,
Abhinav Kumar <quic_abhinavk@...cinc.com>,
Johan Hovold <johan@...nel.org>, Sean Paul <sean@...rly.run>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>, linux-arm-msm@...r.kernel.org,
dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/msm: Initialize mode_config earlier
On Fri, Jan 13, 2023 at 10:57:18AM +0200, Dmitry Baryshkov wrote:
> On 13/01/2023 06:23, Dmitry Baryshkov wrote:
> > On 13/01/2023 06:10, Bjorn Andersson wrote:
> > > Invoking drm_bridge_hpd_notify() on a drm_bridge with a HPD-enabled
> > > bridge_connector ends up in drm_bridge_connector_hpd_cb() calling
> > > drm_kms_helper_hotplug_event(), which assumes that the associated
> > > drm_device's mode_config.funcs is a valid pointer.
> > >
> > > But in the MSM DisplayPort driver the HPD enablement happens at bind
> > > time and mode_config.funcs is initialized late in msm_drm_init(). This
> > > means that there's a window for hot plug events to dereference a NULL
> > > mode_config.funcs.
> > >
> > > Move the assignment of mode_config.funcs before the bind, to avoid this
> > > scenario.
> >
> > Cam we make DP driver not to report HPD events until the enable_hpd()
> > was called? I think this is what was fixed by your internal_hpd
> > patchset.
>
> Or to express this in another words: I thought that internal_hpd already
> deferred enabling hpd event reporting till the time when we need it, didn't
> it?
>
I added a WARN_ON(1) in drm_bridge_hpd_enable() to get a sense of when
this window of "opportunity" opens up, and here's the callstack:
------------[ cut here ]------------
WARNING: CPU: 6 PID: 99 at drivers/gpu/drm/drm_bridge.c:1260 drm_bridge_hpd_enable+0x48/0x94 [drm]
...
Call trace:
drm_bridge_hpd_enable+0x48/0x94 [drm]
drm_bridge_connector_enable_hpd+0x30/0x3c [drm_kms_helper]
drm_kms_helper_poll_enable+0xa4/0x114 [drm_kms_helper]
drm_kms_helper_poll_init+0x6c/0x7c [drm_kms_helper]
msm_drm_bind+0x370/0x628 [msm]
try_to_bring_up_aggregate_device+0x170/0x1bc
__component_add+0xb0/0x168
component_add+0x20/0x2c
dp_display_probe+0x40c/0x468 [msm]
platform_probe+0xb4/0xdc
really_probe+0x13c/0x300
__driver_probe_device+0xc0/0xec
driver_probe_device+0x48/0x204
__device_attach_driver+0x124/0x14c
bus_for_each_drv+0x90/0xdc
__device_attach+0xdc/0x1a8
device_initial_probe+0x20/0x2c
bus_probe_device+0x40/0xa4
deferred_probe_work_func+0x94/0xd0
process_one_work+0x1a8/0x3c0
worker_thread+0x254/0x47c
kthread+0xf8/0x1b8
ret_from_fork+0x10/0x20
---[ end trace 0000000000000000 ]---
As drm_kms_helper_poll_init() is the last thing being called in
msm_drm_init() shifting around the mode_config.func assignment would not
have any impact.
Perhaps we have shuffled other things around to avoid this bug? Either
way, let's this on hold until further proof that it's still
reproducible.
Sorry for the noise,
Bjorn
Powered by blists - more mailing lists