| Message ID | 1457135033-11791-1-git-send-email-dianders@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 04, 2016 at 03:43:53PM -0800, Douglas Anderson wrote: > On a system I'm doing development on I found a crash. The crawl looked > like: > > PC is at drm_atomic_add_affected_connectors+0x98/0xe8 > ... > drm_atomic_add_affected_connectors from __drm_atomic_helper_set_config+0x218/0x344 > __drm_atomic_helper_set_config from restore_fbdev_mode+0x108/0x250 > restore_fbdev_mode from drm_fb_helper_restore_fbdev_mode_unlocked+0x3c/0x80 > drm_fb_helper_restore_fbdev_mode_unlocked from rockchip_drm_lastclose+0x1c/0x20 > rockchip_drm_lastclose from drm_lastclose+0x4c/0x104 > drm_lastclose from drm_release+0x424/0x47c > drm_release from __fput+0xf8/0x1d4 > __fput from ____fput+0x18/0x1c > ____fput from task_work_run+0xa8/0xbc > task_work_run from do_exit+0x448/0x91c > do_exit from do_group_exit+0x5c/0xcc > do_group_exit from get_signal+0x4dc/0x57c > get_signal from do_signal+0x9c/0x3b4 > do_signal from do_work_pending+0x60/0xb8 > do_work_pending from slow_work_pending+0xc/0x20 > > I found that I could fix the crash by checking connector->state against > NULL. This isn't code I'm familiar with and I didn't dig too deep, so > I'd appreciate any opinions about whether this is a sane thing to do. > Note that I don't actually have graphics up on my system at the moment, > so perhaps this is all just a symptom of the strange state I'm in. > > Signed-off-by: Douglas Anderson <dianders@chromium.org> This is a driver bug - under atomic the assumption is that there is _always_ a current software state. Most driver set up the initial "everything off" state in the ->reset functions. -Daniel > --- > drivers/gpu/drm/drm_atomic.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 8fb469c4e4b8..3377d7ddc6d1 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -1147,7 +1147,7 @@ drm_atomic_add_affected_connectors(struct drm_atomic_state *state, > * current configuration. > */ > drm_for_each_connector(connector, state->dev) { > - if (connector->state->crtc != crtc) > + if (!connector->state || connector->state->crtc != crtc) > continue; > > conn_state = drm_atomic_get_connector_state(state, connector); > -- > 2.7.0.rc3.207.g0ac5344 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Sun, Mar 06, 2016 at 05:29:51PM +0100, Daniel Vetter wrote: > On Fri, Mar 04, 2016 at 03:43:53PM -0800, Douglas Anderson wrote: > > On a system I'm doing development on I found a crash. The crawl looked > > like: > > > > PC is at drm_atomic_add_affected_connectors+0x98/0xe8 > > ... > > drm_atomic_add_affected_connectors from __drm_atomic_helper_set_config+0x218/0x344 > > __drm_atomic_helper_set_config from restore_fbdev_mode+0x108/0x250 > > restore_fbdev_mode from drm_fb_helper_restore_fbdev_mode_unlocked+0x3c/0x80 > > drm_fb_helper_restore_fbdev_mode_unlocked from rockchip_drm_lastclose+0x1c/0x20 > > rockchip_drm_lastclose from drm_lastclose+0x4c/0x104 > > drm_lastclose from drm_release+0x424/0x47c > > drm_release from __fput+0xf8/0x1d4 > > __fput from ____fput+0x18/0x1c > > ____fput from task_work_run+0xa8/0xbc > > task_work_run from do_exit+0x448/0x91c > > do_exit from do_group_exit+0x5c/0xcc > > do_group_exit from get_signal+0x4dc/0x57c > > get_signal from do_signal+0x9c/0x3b4 > > do_signal from do_work_pending+0x60/0xb8 > > do_work_pending from slow_work_pending+0xc/0x20 > > > > I found that I could fix the crash by checking connector->state against > > NULL. This isn't code I'm familiar with and I didn't dig too deep, so > > I'd appreciate any opinions about whether this is a sane thing to do. > > Note that I don't actually have graphics up on my system at the moment, > > so perhaps this is all just a symptom of the strange state I'm in. > > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > This is a driver bug - under atomic the assumption is that there is > _always_ a current software state. Most driver set up the initial > "everything off" state in the ->reset functions. Except if it's due to kzalloc() failing in the atomic helper reset implementation. There's no way to propagate the error upwards, so the current implementation just silently sets state=NULL and goes on like nothing happened.
Daniel, On Sun, Mar 6, 2016 at 8:29 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, Mar 04, 2016 at 03:43:53PM -0800, Douglas Anderson wrote: >> On a system I'm doing development on I found a crash. The crawl looked >> like: >> >> PC is at drm_atomic_add_affected_connectors+0x98/0xe8 >> ... >> drm_atomic_add_affected_connectors from __drm_atomic_helper_set_config+0x218/0x344 >> __drm_atomic_helper_set_config from restore_fbdev_mode+0x108/0x250 >> restore_fbdev_mode from drm_fb_helper_restore_fbdev_mode_unlocked+0x3c/0x80 >> drm_fb_helper_restore_fbdev_mode_unlocked from rockchip_drm_lastclose+0x1c/0x20 >> rockchip_drm_lastclose from drm_lastclose+0x4c/0x104 >> drm_lastclose from drm_release+0x424/0x47c >> drm_release from __fput+0xf8/0x1d4 >> __fput from ____fput+0x18/0x1c >> ____fput from task_work_run+0xa8/0xbc >> task_work_run from do_exit+0x448/0x91c >> do_exit from do_group_exit+0x5c/0xcc >> do_group_exit from get_signal+0x4dc/0x57c >> get_signal from do_signal+0x9c/0x3b4 >> do_signal from do_work_pending+0x60/0xb8 >> do_work_pending from slow_work_pending+0xc/0x20 >> >> I found that I could fix the crash by checking connector->state against >> NULL. This isn't code I'm familiar with and I didn't dig too deep, so >> I'd appreciate any opinions about whether this is a sane thing to do. >> Note that I don't actually have graphics up on my system at the moment, >> so perhaps this is all just a symptom of the strange state I'm in. >> >> Signed-off-by: Douglas Anderson <dianders@chromium.org> > > This is a driver bug - under atomic the assumption is that there is > _always_ a current software state. Most driver set up the initial > "everything off" state in the ->reset functions. Ah, I see what the problem is. It looks like the main Rockchip subsystem moved to atomic but the patch to support that in dw_hdmi never landed. If I pick <https://patchwork.kernel.org/patch/7868951/> then my crash goes away. :) In case it's not obvious, please consider $SUBJECT patch abandoned. Thanks! -Doug
Hi, On Mon, Mar 7, 2016 at 4:05 PM, Doug Anderson <dianders@chromium.org> wrote: > Daniel, > > On Sun, Mar 6, 2016 at 8:29 AM, Daniel Vetter <daniel@ffwll.ch> wrote: >> On Fri, Mar 04, 2016 at 03:43:53PM -0800, Douglas Anderson wrote: >>> On a system I'm doing development on I found a crash. The crawl looked >>> like: >>> >>> PC is at drm_atomic_add_affected_connectors+0x98/0xe8 >>> ... >>> drm_atomic_add_affected_connectors from __drm_atomic_helper_set_config+0x218/0x344 >>> __drm_atomic_helper_set_config from restore_fbdev_mode+0x108/0x250 >>> restore_fbdev_mode from drm_fb_helper_restore_fbdev_mode_unlocked+0x3c/0x80 >>> drm_fb_helper_restore_fbdev_mode_unlocked from rockchip_drm_lastclose+0x1c/0x20 >>> rockchip_drm_lastclose from drm_lastclose+0x4c/0x104 >>> drm_lastclose from drm_release+0x424/0x47c >>> drm_release from __fput+0xf8/0x1d4 >>> __fput from ____fput+0x18/0x1c >>> ____fput from task_work_run+0xa8/0xbc >>> task_work_run from do_exit+0x448/0x91c >>> do_exit from do_group_exit+0x5c/0xcc >>> do_group_exit from get_signal+0x4dc/0x57c >>> get_signal from do_signal+0x9c/0x3b4 >>> do_signal from do_work_pending+0x60/0xb8 >>> do_work_pending from slow_work_pending+0xc/0x20 >>> >>> I found that I could fix the crash by checking connector->state against >>> NULL. This isn't code I'm familiar with and I didn't dig too deep, so >>> I'd appreciate any opinions about whether this is a sane thing to do. >>> Note that I don't actually have graphics up on my system at the moment, >>> so perhaps this is all just a symptom of the strange state I'm in. >>> >>> Signed-off-by: Douglas Anderson <dianders@chromium.org> >> >> This is a driver bug - under atomic the assumption is that there is >> _always_ a current software state. Most driver set up the initial >> "everything off" state in the ->reset functions. > > Ah, I see what the problem is. It looks like the main Rockchip > subsystem moved to atomic but the patch to support that in dw_hdmi > never landed. If I pick <https://patchwork.kernel.org/patch/7868951/> > then my crash goes away. :) > > In case it's not obvious, please consider $SUBJECT patch abandoned. Thanks! Argh. ...or the needed patch landed but I didn't pick it back in my backport. Even dumber. :( -Doug
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 8fb469c4e4b8..3377d7ddc6d1 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1147,7 +1147,7 @@ drm_atomic_add_affected_connectors(struct drm_atomic_state *state, * current configuration. */ drm_for_each_connector(connector, state->dev) { - if (connector->state->crtc != crtc) + if (!connector->state || connector->state->crtc != crtc) continue; conn_state = drm_atomic_get_connector_state(state, connector);
On a system I'm doing development on I found a crash. The crawl looked like: PC is at drm_atomic_add_affected_connectors+0x98/0xe8 ... drm_atomic_add_affected_connectors from __drm_atomic_helper_set_config+0x218/0x344 __drm_atomic_helper_set_config from restore_fbdev_mode+0x108/0x250 restore_fbdev_mode from drm_fb_helper_restore_fbdev_mode_unlocked+0x3c/0x80 drm_fb_helper_restore_fbdev_mode_unlocked from rockchip_drm_lastclose+0x1c/0x20 rockchip_drm_lastclose from drm_lastclose+0x4c/0x104 drm_lastclose from drm_release+0x424/0x47c drm_release from __fput+0xf8/0x1d4 __fput from ____fput+0x18/0x1c ____fput from task_work_run+0xa8/0xbc task_work_run from do_exit+0x448/0x91c do_exit from do_group_exit+0x5c/0xcc do_group_exit from get_signal+0x4dc/0x57c get_signal from do_signal+0x9c/0x3b4 do_signal from do_work_pending+0x60/0xb8 do_work_pending from slow_work_pending+0xc/0x20 I found that I could fix the crash by checking connector->state against NULL. This isn't code I'm familiar with and I didn't dig too deep, so I'd appreciate any opinions about whether this is a sane thing to do. Note that I don't actually have graphics up on my system at the moment, so perhaps this is all just a symptom of the strange state I'm in. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- drivers/gpu/drm/drm_atomic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)