[Intel-gfx] [PATCH 07/42] drm/i915: Allow i915_sw_fence_await_sw_fence() to allocate

Chris Wilson chris at chris-wilson.co.uk
Fri Oct 7 16:22:57 UTC 2016


On Fri, Oct 07, 2016 at 05:10:04PM +0100, Tvrtko Ursulin wrote:
> 
> On 07/10/2016 10:46, Chris Wilson wrote:
> >In forthcoming patches, we want to be able to dynamically allocate the
> >wait_queue_t used whilst awaiting. This is more convenient if we extend
> >the i915_sw_fence_await_sw_fence() to perform the allocation for us if
> >we pass in a gfp mask as an alternative than a preallocated struct.
> >
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> >---
> >  drivers/gpu/drm/i915/i915_sw_fence.c | 40 ++++++++++++++++++++++++++++++++----
> >  drivers/gpu/drm/i915/i915_sw_fence.h |  8 ++++++++
> >  2 files changed, 44 insertions(+), 4 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
> >index 1e5cbc585ca2..dfb59dced0ce 100644
> >--- a/drivers/gpu/drm/i915/i915_sw_fence.c
> >+++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> >@@ -13,6 +13,8 @@
> >  #include "i915_sw_fence.h"
> >+#define I915_SW_FENCE_FLAG_ALLOC BIT(0)
> >+
> 
> You collide with WQ_FLAG_EXCLUSIVE here, so I assume i915 will have
> complete control of the wq in question?

Hmm, we should move bit as well, but yes the flag is atm private to the
wake up function for which we use a custom function.

> >+		wq = kmalloc(sizeof(*wq), gfp);
> >+		if (!wq) {
> >+			if (!gfpflags_allow_blocking(gfp))
> >+				return -ENOMEM;
> >+
> >+			i915_sw_fence_wait(signaler);
> 
> This looks like a strange API, unless I am missing something - it
> normally sets things up for waiting, unless the allocation fails
> when it actually does the wait?

Yes. If we blocked on the malloc and failed, we block a little more for
the completion. iirc, the idea came from the async task.
> 
> >+			return 0;
> >+		}
> >+
> >+		pending |= I915_SW_FENCE_FLAG_ALLOC;
> 
> Why is this stored in pending? Just because it is around as re-used?

Urm, yes. s/pending/tmp/
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


More information about the Intel-gfx mailing list