On Mon, Nov 24, 2014 at 06:49:33PM +0000, John.C.Harrison@xxxxxxxxx wrote:
> From: John Harrison <John.C.Harrison@xxxxxxxxx>
>
> The next patches in the series convert some display related seqno usage to
> request structure usage. However, the request dereference introduced must be
> done from interrupt context. As the dereference potentially involves freeing the
> request structure and thus calling lots of non-interrupt friendly code, this
> poses a problem.
>
> The solution is to add an IRQ friendly version of the dereference function. All
> this does is to add the request structure to a 'delayed free' list and return.
> The retire code, which is run periodically, then processes this list and does
> the actual dereferencing of the request structures.
>
> v2: Added a count to allow for multiple IRQ dereferences of the same request at
> a time.
>
> For: VIZ-4377
> Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
> Reviewed-by: Thomas Daniel <Thomas.Daniel@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
> drivers/gpu/drm/i915/i915_gem.c | 33 +++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_lrc.c | 2 ++
> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
> drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++
> 5 files changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 149532e6..e9b01e1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2025,6 +2025,10 @@ struct drm_i915_gem_request {
> struct drm_i915_file_private *file_priv;
> /** file_priv list entry for this request */
> struct list_head client_list;
> +
> + /** deferred free list for dereferencing from IRQ context */
> + struct list_head delay_free_list;
> + uint32_t delay_free_count;
> };
>
> void i915_gem_request_free(struct kref *req_ref);
> @@ -2050,9 +2054,12 @@ i915_gem_request_reference(struct drm_i915_gem_request *req)
> static inline void
> i915_gem_request_unreference(struct drm_i915_gem_request *req)
> {
> + WARN_ON(!mutex_is_locked(&req->ring->dev->struct_mutex));
> kref_put(&req->ref, i915_gem_request_free);
> }
>
> +void i915_gem_request_unreference_irq(struct drm_i915_gem_request *req);
> +
> static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst,
> struct drm_i915_gem_request *src)
> {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8e2c530..b62b9d9 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2732,6 +2732,19 @@ void i915_gem_reset(struct drm_device *dev)
> i915_gem_restore_fences(dev);
> }
>
> +void i915_gem_request_unreference_irq(struct drm_i915_gem_request *req)
> +{
> + struct intel_engine_cs *ring = req->ring;
> + unsigned long flags;
> +
> + /* Need to add the req to a deferred dereference list to be processed
> + * outside of interrupt time */
> + spin_lock_irqsave(&ring->reqlist_lock, flags);
> + if (req->delay_free_count++ == 0)
> + list_add_tail(&req->delay_free_list, &ring->delayed_free_list);
> + spin_unlock_irqrestore(&ring->reqlist_lock, flags);
> +}
> +
> /**
> * This function clears the request list as sequence numbers are passed.
> */
> @@ -2806,6 +2819,25 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
> ring->trace_irq_seqno = 0;
> }
>
> + while (!list_empty(&ring->delayed_free_list)) {
This needs locking. It might work as-is but it's definitely too tricky to
be worth it.
-Daniel
> + struct drm_i915_gem_request *request;
> + unsigned long flags;
> + uint32_t count;
> +
> + request = list_first_entry(&ring->delayed_free_list,
> + struct drm_i915_gem_request,
> + delay_free_list);
> +
> + spin_lock_irqsave(&request->ring->reqlist_lock, flags);
> + list_del(&request->delay_free_list);
> + count = request->delay_free_count;
> + request->delay_free_count = 0;
> + spin_unlock_irqrestore(&request->ring->reqlist_lock, flags);
> +
> + while (count-- > 0)
> + i915_gem_request_unreference(request);
> + }
> +
> WARN_ON(i915_verify_lists(ring->dev));
> }
>
> @@ -5007,6 +5039,7 @@ init_ring_lists(struct intel_engine_cs *ring)
> {
> INIT_LIST_HEAD(&ring->active_list);
> INIT_LIST_HEAD(&ring->request_list);
> + INIT_LIST_HEAD(&ring->delayed_free_list);
> }
>
> void i915_init_vm(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 6177179..09f4588 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1381,7 +1381,9 @@ static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs *rin
> ring->dev = dev;
> INIT_LIST_HEAD(&ring->active_list);
> INIT_LIST_HEAD(&ring->request_list);
> + spin_lock_init(&ring->reqlist_lock);
> init_waitqueue_head(&ring->irq_queue);
> + INIT_LIST_HEAD(&ring->delayed_free_list);
>
> INIT_LIST_HEAD(&ring->execlist_queue);
> INIT_LIST_HEAD(&ring->execlist_retired_req_list);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index b9d0d29..e3082af 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1807,6 +1807,8 @@ static int intel_init_ring_buffer(struct drm_device *dev,
> ring->dev = dev;
> INIT_LIST_HEAD(&ring->active_list);
> INIT_LIST_HEAD(&ring->request_list);
> + spin_lock_init(&ring->reqlist_lock);
> + INIT_LIST_HEAD(&ring->delayed_free_list);
> INIT_LIST_HEAD(&ring->execlist_queue);
> ringbuf->size = 32 * PAGE_SIZE;
> ringbuf->ring = ring;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 2a84bd9..cd1a9f9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -263,6 +263,8 @@ struct intel_engine_cs {
> * outstanding.
> */
> struct list_head request_list;
> + spinlock_t reqlist_lock;
> + struct list_head delayed_free_list;
>
> /**
> * Do we have some not yet emitted requests outstanding?
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
