Hey,
Op 20-03-15 om 18:48 schreef John.C.Harrison@xxxxxxxxx:
> From: John Harrison <John.C.Harrison@xxxxxxxxx>
>
> There is a construct in the linux kernel called 'struct fence' that is intended
> to keep track of work that is executed on hardware. I.e. it solves the basic
> problem that the drivers 'struct drm_i915_gem_request' is trying to address. The
> request structure does quite a lot more than simply track the execution progress
> so is very definitely still required. However, the basic completion status side
> could be updated to use the ready made fence implementation and gain all the
> advantages that provides.
>
> This patch makes the first step of integrating a struct fence into the request.
> It replaces the explicit reference count with that of the fence. It also
> replaces the 'is completed' test with the fence's equivalent. Currently, that
> simply chains on to the original request implementation. A future patch will
> improve this.
>
> For: VIZ-5190
> Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 37 +++++++++------------
> drivers/gpu/drm/i915/i915_gem.c | 55 ++++++++++++++++++++++++++++---
> drivers/gpu/drm/i915/intel_lrc.c | 1 +
> drivers/gpu/drm/i915/intel_ringbuffer.c | 1 +
> drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++
> 5 files changed, 70 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ce3a536..7dcaf8c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -50,6 +50,7 @@
> #include <linux/intel-iommu.h>
> #include <linux/kref.h>
> #include <linux/pm_qos.h>
> +#include <linux/fence.h>
>
> /* General customization:
> */
> @@ -2048,7 +2049,11 @@ void i915_gem_track_fb(struct drm_i915_gem_object *old,
> * initial reference taken using kref_init
> */
> struct drm_i915_gem_request {
> - struct kref ref;
> + /** Underlying object for implementing the signal/wait stuff.
> + * NB: Never call fence_later()! Due to lazy allocation, scheduler
> + * re-ordering, pre-emption, etc., there is no guarantee at all
> + * about the validity or sequentialiaty of the fence's seqno! */
> + struct fence fence;
Set fence.context differently for each per context timeline. :-)
>+static bool i915_gem_request_enable_signaling(struct fence *req_fence)
>+{
>+ WARN(true, "Is this required?");
>+ return true;
>+}
Yes, try calling fence_wait() on the fence. :-) This function should call irq_get and add itself to ring->irq_queue.
See for an example radeon_fence_enable_signaling.
>@@ -2557,6 +2596,8 @@ int i915_gem_request_alloc(struct intel_engine_cs *ring,
> return ret;
> }
>
>+ fence_init(&request->fence, &i915_gem_request_fops, &ring->fence_lock, ring->fence_context, request->seqno);
>+
> /*
> * Reserve space in the ring buffer for all the commands required to
> * eventually emit this request. This is to guarantee that the
Use ring->irq_queue.lock instead of making a new lock? This will make implementing enable_signaling easier too.
~Maarten
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
