Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1460736 > unrolled thread

[PATCH 1/2] drm/fence: enable drm_atomic_helper_wait_for_fences() to interrupt

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2] drm/fence: enable drm_atomic_helper_wait_for_fences() to interrupt Gustavo Padovan <gustavo@padovan.org> - 2016-08-11 20:50 +0200
    [PATCH 2/2] drm/fence: allow fence waiting to be interrupted by userspace Gustavo Padovan <gustavo@padovan.org> - 2016-08-11 20:50 +0200
      Re: [PATCH 2/2] drm/fence: allow fence waiting to be interrupted by  userspace Maarten Lankhorst <maarten.lankhorst@linux.intel.com> - 2016-08-15 12:20 +0200
        Re: [PATCH 2/2] drm/fence: allow fence waiting to be interrupted by  userspace Daniel Vetter <daniel@ffwll.ch> - 2016-08-15 16:00 +0200
        Re: [PATCH 2/2] drm/fence: allow fence waiting to be interrupted by  userspace Gustavo Padovan <gustavo@padovan.org> - 2016-08-15 16:50 +0200

#1460736 — [PATCH 1/2] drm/fence: enable drm_atomic_helper_wait_for_fences() to interrupt

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Add an extra arg to  drm_atomic_helper_wait_for_fences() to inform
if fence_wait() should be called interruptible or uninterruptible.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/drm_atomic_helper.c | 19 ++++++++++++++-----
 drivers/gpu/drm/msm/msm_atomic.c    |  2 +-
 include/drm/drm_atomic_helper.h     |  5 +++--
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index f59e8c0..fa263b7 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1003,18 +1003,22 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_modeset_enables);
  * drm_atomic_helper_wait_for_fences - wait for fences stashed in plane state
  * @dev: DRM device
  * @state: atomic state object with old state structures
+ * @intr: if true, do an interruptible wait
  *
  * For implicit sync, driver should fish the exclusive fence out from the
  * incoming fb's and stash it in the drm_plane_state.  This is called after
  * drm_atomic_helper_swap_state() so it uses the current plane state (and
  * just uses the atomic state to find the changed planes)
+ *
+ * Returns zero if sucess or < 0 if fence_wait() fails.
  */
-void drm_atomic_helper_wait_for_fences(struct drm_device *dev,
-			    struct drm_atomic_state *state)
+int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
+				       struct drm_atomic_state *state,
+				       bool intr)
 {
 	struct drm_plane *plane;
 	struct drm_plane_state *plane_state;
-	int i;
+	int i, ret;
 
 	for_each_plane_in_state(state, plane, plane_state, i) {
 		if (!plane->state->fence)
@@ -1022,10 +1026,15 @@ void drm_atomic_helper_wait_for_fences(struct drm_device *dev,
 
 		WARN_ON(!plane->state->fb);
 
-		fence_wait(plane->state->fence, false);
+		ret = fence_wait(plane->state->fence, intr);
+		if (ret)
+			return ret;
+
 		fence_put(plane->state->fence);
 		plane->state->fence = NULL;
 	}
+
+	return 0;
 }
 EXPORT_SYMBOL(drm_atomic_helper_wait_for_fences);
 
@@ -1172,7 +1181,7 @@ static void commit_tail(struct drm_atomic_state *state)
 
 	funcs = dev->mode_config.helper_private;
 
-	drm_atomic_helper_wait_for_fences(dev, state);
+	drm_atomic_helper_wait_for_fences(dev, state, false);
 
 	drm_atomic_helper_wait_for_dependencies(state);
 
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 4a8a6f1..9518e43 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -112,7 +112,7 @@ static void complete_commit(struct msm_commit *c, bool async)
 	struct msm_drm_private *priv = dev->dev_private;
 	struct msm_kms *kms = priv->kms;
 
-	drm_atomic_helper_wait_for_fences(dev, state);
+	drm_atomic_helper_wait_for_fences(dev, state, false);
 
 	kms->funcs->prepare_commit(kms, state);
 
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index d86ae5d..a42c34b 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -43,8 +43,9 @@ int drm_atomic_helper_commit(struct drm_device *dev,
 			     struct drm_atomic_state *state,
 			     bool nonblock);
 
-void drm_atomic_helper_wait_for_fences(struct drm_device *dev,
-					struct drm_atomic_state *state);
+int drm_atomic_helper_wait_for_fences(struct drm_device *dev,
+					struct drm_atomic_state *state,
+					bool intr);
 bool drm_atomic_helper_framebuffer_changed(struct drm_device *dev,
 					   struct drm_atomic_state *old_state,
 					   struct drm_crtc *crtc);
-- 
2.5.5

[toc] | [next] | [standalone]

#1460739 — [PATCH 2/2] drm/fence: allow fence waiting to be interrupted by userspace

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

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/drm_atomic_helper.c | 29 ++++++++++++++++++++++++-----
 include/drm/drm_crtc.h              |  2 ++
 2 files changed, 26 insertions(+), 5 deletions(-)

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;
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1462695 — Re: [PATCH 2/2] drm/fence: allow fence waiting to be interrupted by userspace

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

[toc] | [prev] | [next] | [standalone]

#1462798 — Re: [PATCH 2/2] drm/fence: allow fence waiting to be interrupted by userspace

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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1462838 — Re: [PATCH 2/2] drm/fence: allow fence waiting to be interrupted by userspace

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web