| Message ID | 1470940785-25870-2-git-send-email-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 11-08-16 om 20:39 schreef Gustavo Padovan: > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > If userspace is running an synchronously atomic commit and interrupts the > atomic operation during fence_wait() it will hang until the timer expires, > so here we change the wait to be interruptible so it stop immediately when > userspace wants to quit. > > Also adds the necessary error checking for fence_wait(). > > v2: Comment by Daniel Vetter > - Add error checking for fence_wait() > > v3: Rebase on top of new atomic noblocking support Meh, I don't like the swapped parameter much, couldn't we infer it from intr? or rename intr to swapped? If we're not swapped yet, we should always wait interruptibly. When swapped, never.. ~Maarten
On Mon, Aug 15, 2016 at 12:15:32PM +0200, Maarten Lankhorst wrote: > Op 11-08-16 om 20:39 schreef Gustavo Padovan: > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > > > If userspace is running an synchronously atomic commit and interrupts the > > atomic operation during fence_wait() it will hang until the timer expires, > > so here we change the wait to be interruptible so it stop immediately when > > userspace wants to quit. > > > > Also adds the necessary error checking for fence_wait(). > > > > v2: Comment by Daniel Vetter > > - Add error checking for fence_wait() > > > > v3: Rebase on top of new atomic noblocking support > Meh, I don't like the swapped parameter much, couldn't we infer it from intr? or rename intr to swapped? > If we're not swapped yet, we should always wait interruptibly. When swapped, never.. Yeah this seems somewhat silly tbh, but then I guess making those waits interruptible is indeed somewhat nice. -Daniel
2016-08-15 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>: > Op 11-08-16 om 20:39 schreef Gustavo Padovan: > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > > > > If userspace is running an synchronously atomic commit and interrupts the > > atomic operation during fence_wait() it will hang until the timer expires, > > so here we change the wait to be interruptible so it stop immediately when > > userspace wants to quit. > > > > Also adds the necessary error checking for fence_wait(). > > > > v2: Comment by Daniel Vetter > > - Add error checking for fence_wait() > > > > v3: Rebase on top of new atomic noblocking support > Meh, I don't like the swapped parameter much, couldn't we infer it from intr? or rename intr to swapped? Definitely, I didn't realized it myself that both were saying the same thing. Thanks for the suggesttion. Gustavo
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index fa263b7..5e8ed15 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1018,20 +1018,31 @@ int drm_atomic_helper_wait_for_fences(struct drm_device *dev, { struct drm_plane *plane; struct drm_plane_state *plane_state; + struct fence *fence; int i, ret; for_each_plane_in_state(state, plane, plane_state, i) { - if (!plane->state->fence) + if (!plane->state->fence && !plane_state->fence) continue; - WARN_ON(!plane->state->fb); + if (state->swapped) { + fence = plane->state->fence; + WARN_ON(!plane->state->fb); + } else { + fence = plane_state->fence; + WARN_ON(!plane_state->fb); + } - ret = fence_wait(plane->state->fence, intr); + ret = fence_wait(fence, intr); if (ret) return ret; - fence_put(plane->state->fence); - plane->state->fence = NULL; + fence_put(fence); + + if (state->swapped) + plane->state->fence = NULL; + else + plane_state->fence = NULL; } return 0; @@ -1240,6 +1251,12 @@ int drm_atomic_helper_commit(struct drm_device *dev, if (ret) return ret; + if (!nonblock) { + ret = drm_atomic_helper_wait_for_fences(dev, state, true); + if (ret) + return ret; + } + /* * This is the point of no return - everything below never fails except * when the hw goes bonghits. Which means we can commit the new state on @@ -2009,6 +2026,8 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state *state, swap(state->planes[i].state, plane->state); plane->state->state = NULL; } + + state->swapped = true; } EXPORT_SYMBOL(drm_atomic_helper_swap_state); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b618b50..99455d8 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -2032,6 +2032,7 @@ struct __drm_connnectors_state { * @allow_modeset: allow full modeset * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics * @legacy_set_config: Disable conflicting encoders instead of failing with -EINVAL. + * @swapped: hint to inform if the state was swapped with the device state * @planes: pointer to array of structures with per-plane data * @crtcs: pointer to array of CRTC pointers * @num_connector: size of the @connectors and @connector_states arrays @@ -2043,6 +2044,7 @@ struct drm_atomic_state { bool allow_modeset : 1; bool legacy_cursor_update : 1; bool legacy_set_config : 1; + bool swapped : 1; struct __drm_planes_state *planes; struct __drm_crtcs_state *crtcs; int num_connector;