Hey,

Op 20-03-15 om 18:48 schreef john.c.harri...@intel.com:
> From: John Harrison <john.c.harri...@intel.com>
>
> 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.harri...@intel.com>
>
> ---
> 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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to