| Message ID | 1414694481-15724-16-git-send-email-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf > Of John.C.Harrison@Intel.com > Sent: Thursday, October 30, 2014 6:41 PM > To: Intel-GFX@Lists.FreeDesktop.Org > Subject: [Intel-gfx] [PATCH 15/29] drm/i915: Add IRQ friendly request > deference facility > > From: John Harrison <John.C.Harrison@Intel.com> > > 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@Intel.com> > --- > 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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); } 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?