| From 24835e442f289813aa568d142a755672a740503c Mon Sep 17 00:00:00 2001 |
| From: Daniel Vetter <daniel.vetter@ffwll.ch> |
| Date: Wed, 21 Dec 2016 11:23:30 +0100 |
| Subject: drm: reference count event->completion |
| |
| From: Daniel Vetter <daniel.vetter@ffwll.ch> |
| |
| commit 24835e442f289813aa568d142a755672a740503c upstream. |
| |
| When writing the generic nonblocking commit code I assumed that |
| through clever lifetime management I can assure that the completion |
| (stored in drm_crtc_commit) only gets freed after it is completed. And |
| that worked. |
| |
| I also wanted to make nonblocking helpers resilient against driver |
| bugs, by having timeouts everywhere. And that worked too. |
| |
| Unfortunately taking boths things together results in oopses :( Well, |
| at least sometimes: What seems to happen is that the drm event hangs |
| around forever stuck in limbo land. The nonblocking helpers eventually |
| time out, move on and release it. Now the bug I tested all this |
| against is drivers that just entirely fail to deliver the vblank |
| events like they should, and in those cases the event is simply |
| leaked. But what seems to happen, at least sometimes, on i915 is that |
| the event is set up correctly, but somohow the vblank fails to fire in |
| time. Which means the event isn't leaked, it's still there waiting for |
| eventually a vblank to fire. That tends to happen when re-enabling the |
| pipe, and then the trap springs and the kernel oopses. |
| |
| The correct fix here is simply to refcount the crtc commit to make |
| sure that the event sticks around even for drivers which only |
| sometimes fail to deliver vblanks for some arbitrary reasons. Since |
| crtc commits are already refcounted that's easy to do. |
| |
| References: https://bugs.freedesktop.org/show_bug.cgi?id=96781 |
| Cc: Jim Rees <rees@umich.edu> |
| Cc: Chris Wilson <chris@chris-wilson.co.uk> |
| Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> |
| Cc: Jani Nikula <jani.nikula@linux.intel.com> |
| Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> |
| Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> |
| Link: http://patchwork.freedesktop.org/patch/msgid/20161221102331.31033-1-daniel.vetter@ffwll.ch |
| Cc: Arnd Bergmann <arnd@arndb.de> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/gpu/drm/drm_atomic_helper.c | 11 +++++++++++ |
| drivers/gpu/drm/drm_fops.c | 2 +- |
| include/drm/drmP.h | 1 + |
| 3 files changed, 13 insertions(+), 1 deletion(-) |
| |
| --- a/drivers/gpu/drm/drm_atomic_helper.c |
| +++ b/drivers/gpu/drm/drm_atomic_helper.c |
| @@ -1382,6 +1382,15 @@ static int stall_checks(struct drm_crtc |
| return ret < 0 ? ret : 0; |
| } |
| |
| +void release_crtc_commit(struct completion *completion) |
| +{ |
| + struct drm_crtc_commit *commit = container_of(completion, |
| + typeof(*commit), |
| + flip_done); |
| + |
| + drm_crtc_commit_put(commit); |
| +} |
| + |
| /** |
| * drm_atomic_helper_setup_commit - setup possibly nonblocking commit |
| * @state: new modeset state to be committed |
| @@ -1474,6 +1483,8 @@ int drm_atomic_helper_setup_commit(struc |
| } |
| |
| crtc_state->event->base.completion = &commit->flip_done; |
| + crtc_state->event->base.completion_release = release_crtc_commit; |
| + drm_crtc_commit_get(commit); |
| } |
| |
| return 0; |
| --- a/drivers/gpu/drm/drm_fops.c |
| +++ b/drivers/gpu/drm/drm_fops.c |
| @@ -686,8 +686,8 @@ void drm_send_event_locked(struct drm_de |
| assert_spin_locked(&dev->event_lock); |
| |
| if (e->completion) { |
| - /* ->completion might disappear as soon as it signalled. */ |
| complete_all(e->completion); |
| + e->completion_release(e->completion); |
| e->completion = NULL; |
| } |
| |
| --- a/include/drm/drmP.h |
| +++ b/include/drm/drmP.h |
| @@ -361,6 +361,7 @@ struct drm_ioctl_desc { |
| /* Event queued up for userspace to read */ |
| struct drm_pending_event { |
| struct completion *completion; |
| + void (*completion_release)(struct completion *completion); |
| struct drm_event *event; |
| struct fence *fence; |
| struct list_head link; |