Re: [PATCH 15/29] drm/i915: Add IRQ friendly request deference facility

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf
> Of John.C.Harrison@xxxxxxxxx
> Sent: Thursday, October 30, 2014 6:41 PM
> To: Intel-GFX@xxxxxxxxxxxxxxxxxxxxx
> Subject:  [PATCH 15/29] drm/i915: Add IRQ friendly request
> deference facility
> 
> From: John Harrison <John.C.Harrison@xxxxxxxxx>
> 
> The next patch in the series converts 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 call lots of non-interrupt
> friendly code, this poses a problem.
> 
> The solution is to add a 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.
> 
> For: VIZ-4377
> Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |    6 ++++++
>  drivers/gpu/drm/i915/i915_gem.c         |   28
> ++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_lrc.c        |    2 ++
>  drivers/gpu/drm/i915/intel_ringbuffer.c |    3 +++
>  drivers/gpu/drm/i915/intel_ringbuffer.h |    2 ++
>  5 files changed, 41 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index d184333..3e6082b 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1951,6 +1951,9 @@ 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 free_list;
>  };
> 
>  void i915_gem_request_free(struct kref *req_ref); @@ -1976,9 +1979,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 9130aa1..e4cb253 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2635,6 +2635,18 @@ 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);
> +	list_add_tail(&req->free_list, &ring->delayed_free_list);
> +	spin_unlock_irqrestore(&ring->reqlist_lock, flags); }
> +
>  /**
>   * This function clears the request list as sequence numbers are passed.
>   */
> @@ -2708,6 +2720,21 @@ i915_gem_retire_requests_ring(struct
> intel_engine_cs *ring)
>  		ring->trace_irq_seqno = 0;
>  	}
> 
> +	while (!list_empty(&ring->delayed_free_list)) {
> +		struct drm_i915_gem_request *request;
> +		unsigned long flags;
> +
> +		request = list_first_entry(&ring->delayed_free_list,
> +					   struct drm_i915_gem_request,
> +					   free_list);
> +
> +		spin_lock_irqsave(&request->ring->reqlist_lock, flags);
> +		list_del(&request->free_list);
> +		spin_unlock_irqrestore(&request->ring->reqlist_lock, flags);
> +
> +		i915_gem_request_unreference(request);
> +	}
> +
>  	WARN_ON(i915_verify_lists(ring->dev));
>  }
> 
> @@ -4948,6 +4975,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);
Do we need this?
This list is initialized in logical_ring_init and init_ring_buffer anyway.

Thomas.

>  }
> 
>  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 a6859ad..ba0d7b8 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1249,7 +1249,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);
>  	spin_lock_init(&ring->execlist_lock);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 164236c..f225ba3 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;
> @@ -2506,6 +2508,7 @@ int intel_render_ring_init_dri(struct drm_device
> *dev, u64 start, u32 size)
>  	ring->dev = dev;
>  	INIT_LIST_HEAD(&ring->active_list);
>  	INIT_LIST_HEAD(&ring->request_list);
> +	INIT_LIST_HEAD(&ring->delayed_free_list);
> 
>  	ringbuf->size = size;
>  	ringbuf->effective_size = ringbuf->size; diff --git
> a/drivers/gpu/drm/i915/intel_ringbuffer.h
> b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 98b219d..0451233 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -261,6 +261,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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux