| Message ID | 20190619153942.8518-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915: Keep rings pinned while the context is active | expand |
On 19/06/2019 16:39, Chris Wilson wrote: > Remember to keep the rings pinned as well as the context image until the > GPU is no longer active. > > v2: Introduce a ring->pin_count primarily to hide the > mock_ring that doesn't fit into the normal GGTT vma picture. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110946 > Fixes: ce476c80b8bf ("drm/i915: Keep contexts pinned until after the next kernel context switch") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/gt/intel_context.c | 27 ++++++++++++------- > drivers/gpu/drm/i915/gt/intel_engine_types.h | 12 +++++++++ > drivers/gpu/drm/i915/gt/intel_lrc.c | 10 ++----- > drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 28 +++++++++++++------- > drivers/gpu/drm/i915/gt/mock_engine.c | 1 + > 5 files changed, 51 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c > index 2c454f227c2e..23120901c55f 100644 > --- a/drivers/gpu/drm/i915/gt/intel_context.c > +++ b/drivers/gpu/drm/i915/gt/intel_context.c > @@ -126,6 +126,7 @@ static void intel_context_retire(struct i915_active *active) > if (ce->state) > __context_unpin_state(ce->state); > > + intel_ring_unpin(ce->ring); > intel_context_put(ce); > } > > @@ -160,27 +161,35 @@ int intel_context_active_acquire(struct intel_context *ce, unsigned long flags) > > intel_context_get(ce); > > + err = intel_ring_pin(ce->ring); > + if (err) > + goto err_put; > + > if (!ce->state) > return 0; > > err = __context_pin_state(ce->state, flags); > - if (err) { > - i915_active_cancel(&ce->active); > - intel_context_put(ce); > - return err; > - } > + if (err) > + goto err_ring; > > /* Preallocate tracking nodes */ > if (!i915_gem_context_is_kernel(ce->gem_context)) { > err = i915_active_acquire_preallocate_barrier(&ce->active, > ce->engine); > - if (err) { > - i915_active_release(&ce->active); > - return err; > - } > + if (err) > + goto err_state; > } > > return 0; > + > +err_state: > + __context_unpin_state(ce->state); > +err_ring: > + intel_ring_unpin(ce->ring); > +err_put: > + intel_context_put(ce); > + i915_active_cancel(&ce->active); > + return err; > } > > void intel_context_active_release(struct intel_context *ce) > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h > index 868b220214f8..cc6b8eb0cda0 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h > @@ -70,6 +70,18 @@ struct intel_ring { > struct list_head request_list; > struct list_head active_link; > > + /* > + * As we have two types of rings, one global to the engine used > + * by ringbuffer submission and those that are exclusive to a > + * context used by execlists, we have to play safe and allow > + * atomic updates to the pin_count. However, the actual pinning > + * of the context is either done during initialisation for > + * ringbuffer submission or serialised as part of the context > + * pinning for execlists, and so we do not need a mutex ourselves > + * to * serialise intel_ring_pin/intel_ring_unpin. > + */ > + atomic_t pin_count; > + > u32 head; > u32 tail; > u32 emit; > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c > index b42b5f158295..82b7ace62d97 100644 > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > @@ -1414,6 +1414,7 @@ static void execlists_context_destroy(struct kref *kref) > { > struct intel_context *ce = container_of(kref, typeof(*ce), ref); > > + GEM_BUG_ON(!i915_active_is_idle(&ce->active)); > GEM_BUG_ON(intel_context_is_pinned(ce)); > > if (ce->state) > @@ -1426,7 +1427,6 @@ static void execlists_context_unpin(struct intel_context *ce) > { > i915_gem_context_unpin_hw_id(ce->gem_context); > i915_gem_object_unpin_map(ce->state->obj); > - intel_ring_unpin(ce->ring); > } > > static void > @@ -1478,13 +1478,9 @@ __execlists_context_pin(struct intel_context *ce, > goto unpin_active; > } > > - ret = intel_ring_pin(ce->ring); > - if (ret) > - goto unpin_map; > - > ret = i915_gem_context_pin_hw_id(ce->gem_context); > if (ret) > - goto unpin_ring; > + goto unpin_map; > > ce->lrc_desc = lrc_descriptor(ce, engine); > ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE; > @@ -1492,8 +1488,6 @@ __execlists_context_pin(struct intel_context *ce, > > return 0; > > -unpin_ring: > - intel_ring_unpin(ce->ring); > unpin_map: > i915_gem_object_unpin_map(ce->state->obj); > unpin_active: > diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c > index c6023bc9452d..0d8aaaa089cc 100644 > --- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c > @@ -1149,16 +1149,16 @@ i915_emit_bb_start(struct i915_request *rq, > int intel_ring_pin(struct intel_ring *ring) > { > struct i915_vma *vma = ring->vma; > - enum i915_map_type map = i915_coherent_map_type(vma->vm->i915); > unsigned int flags; > void *addr; > int ret; > > - GEM_BUG_ON(ring->vaddr); > + if (atomic_fetch_inc(&ring->pin_count)) > + return 0; > > ret = i915_timeline_pin(ring->timeline); > if (ret) > - return ret; > + goto err_unpin; > > flags = PIN_GLOBAL; > > @@ -1172,26 +1172,31 @@ int intel_ring_pin(struct intel_ring *ring) > > ret = i915_vma_pin(vma, 0, 0, flags); > if (unlikely(ret)) > - goto unpin_timeline; > + goto err_timeline; > > if (i915_vma_is_map_and_fenceable(vma)) > addr = (void __force *)i915_vma_pin_iomap(vma); > else > - addr = i915_gem_object_pin_map(vma->obj, map); > + addr = i915_gem_object_pin_map(vma->obj, > + i915_coherent_map_type(vma->vm->i915)); > if (IS_ERR(addr)) { > ret = PTR_ERR(addr); > - goto unpin_ring; > + goto err_ring; > } > > vma->obj->pin_global++; > > + GEM_BUG_ON(ring->vaddr); > ring->vaddr = addr; > + > return 0; > > -unpin_ring: > +err_ring: > i915_vma_unpin(vma); > -unpin_timeline: > +err_timeline: > i915_timeline_unpin(ring->timeline); > +err_unpin: > + atomic_dec(&ring->pin_count); > return ret; > } > > @@ -1207,16 +1212,19 @@ void intel_ring_reset(struct intel_ring *ring, u32 tail) > > void intel_ring_unpin(struct intel_ring *ring) > { > - GEM_BUG_ON(!ring->vma); > - GEM_BUG_ON(!ring->vaddr); > + if (!atomic_dec_and_test(&ring->pin_count)) > + return; > > /* Discard any unused bytes beyond that submitted to hw. */ > intel_ring_reset(ring, ring->tail); > > + GEM_BUG_ON(!ring->vma); > if (i915_vma_is_map_and_fenceable(ring->vma)) > i915_vma_unpin_iomap(ring->vma); > else > i915_gem_object_unpin_map(ring->vma->obj); > + > + GEM_BUG_ON(!ring->vaddr); > ring->vaddr = NULL; > > ring->vma->obj->pin_global--; > diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c > index 086801b51441..486c6953dcb1 100644 > --- a/drivers/gpu/drm/i915/gt/mock_engine.c > +++ b/drivers/gpu/drm/i915/gt/mock_engine.c > @@ -66,6 +66,7 @@ static struct intel_ring *mock_ring(struct intel_engine_cs *engine) > ring->base.effective_size = sz; > ring->base.vaddr = (void *)(ring + 1); > ring->base.timeline = &ring->timeline; > + atomic_set(&ring->base.pin_count, 1); > > INIT_LIST_HEAD(&ring->base.request_list); > intel_ring_update_space(&ring->base); > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
Quoting Tvrtko Ursulin (2019-06-19 17:26:05) > > On 19/06/2019 16:39, Chris Wilson wrote: > > Remember to keep the rings pinned as well as the context image until the > > GPU is no longer active. > > > > v2: Introduce a ring->pin_count primarily to hide the > > mock_ring that doesn't fit into the normal GGTT vma picture. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110946 > > Fixes: ce476c80b8bf ("drm/i915: Keep contexts pinned until after the next kernel context switch") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_context.c | 27 ++++++++++++------- > > drivers/gpu/drm/i915/gt/intel_engine_types.h | 12 +++++++++ > > drivers/gpu/drm/i915/gt/intel_lrc.c | 10 ++----- > > drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 28 +++++++++++++------- > > drivers/gpu/drm/i915/gt/mock_engine.c | 1 + > > 5 files changed, 51 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c > > index 2c454f227c2e..23120901c55f 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_context.c > > +++ b/drivers/gpu/drm/i915/gt/intel_context.c > > @@ -126,6 +126,7 @@ static void intel_context_retire(struct i915_active *active) > > if (ce->state) > > __context_unpin_state(ce->state); > > > > + intel_ring_unpin(ce->ring); > > intel_context_put(ce); > > } > > > > @@ -160,27 +161,35 @@ int intel_context_active_acquire(struct intel_context *ce, unsigned long flags) > > > > intel_context_get(ce); > > > > + err = intel_ring_pin(ce->ring); > > + if (err) > > + goto err_put; > > + > > if (!ce->state) > > return 0; > > > > err = __context_pin_state(ce->state, flags); > > - if (err) { > > - i915_active_cancel(&ce->active); > > - intel_context_put(ce); > > - return err; > > - } > > + if (err) > > + goto err_ring; > > > > /* Preallocate tracking nodes */ > > if (!i915_gem_context_is_kernel(ce->gem_context)) { > > err = i915_active_acquire_preallocate_barrier(&ce->active, > > ce->engine); > > - if (err) { > > - i915_active_release(&ce->active); > > - return err; > > - } > > + if (err) > > + goto err_state; > > } > > > > return 0; > > + > > +err_state: > > + __context_unpin_state(ce->state); > > +err_ring: > > + intel_ring_unpin(ce->ring); > > +err_put: > > + intel_context_put(ce); > > + i915_active_cancel(&ce->active); > > + return err; > > } > > > > void intel_context_active_release(struct intel_context *ce) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h > > index 868b220214f8..cc6b8eb0cda0 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h > > +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h > > @@ -70,6 +70,18 @@ struct intel_ring { > > struct list_head request_list; > > struct list_head active_link; > > > > + /* > > + * As we have two types of rings, one global to the engine used > > + * by ringbuffer submission and those that are exclusive to a > > + * context used by execlists, we have to play safe and allow > > + * atomic updates to the pin_count. However, the actual pinning > > + * of the context is either done during initialisation for > > + * ringbuffer submission or serialised as part of the context > > + * pinning for execlists, and so we do not need a mutex ourselves > > + * to * serialise intel_ring_pin/intel_ring_unpin. > > + */ > > + atomic_t pin_count; > > + > > u32 head; > > u32 tail; > > u32 emit; > > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c > > index b42b5f158295..82b7ace62d97 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > > @@ -1414,6 +1414,7 @@ static void execlists_context_destroy(struct kref *kref) > > { > > struct intel_context *ce = container_of(kref, typeof(*ce), ref); > > > > + GEM_BUG_ON(!i915_active_is_idle(&ce->active)); > > GEM_BUG_ON(intel_context_is_pinned(ce)); > > > > if (ce->state) > > @@ -1426,7 +1427,6 @@ static void execlists_context_unpin(struct intel_context *ce) > > { > > i915_gem_context_unpin_hw_id(ce->gem_context); > > i915_gem_object_unpin_map(ce->state->obj); > > - intel_ring_unpin(ce->ring); > > } > > > > static void > > @@ -1478,13 +1478,9 @@ __execlists_context_pin(struct intel_context *ce, > > goto unpin_active; > > } > > > > - ret = intel_ring_pin(ce->ring); > > - if (ret) > > - goto unpin_map; > > - > > ret = i915_gem_context_pin_hw_id(ce->gem_context); > > if (ret) > > - goto unpin_ring; > > + goto unpin_map; > > > > ce->lrc_desc = lrc_descriptor(ce, engine); > > ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE; > > @@ -1492,8 +1488,6 @@ __execlists_context_pin(struct intel_context *ce, > > > > return 0; > > > > -unpin_ring: > > - intel_ring_unpin(ce->ring); > > unpin_map: > > i915_gem_object_unpin_map(ce->state->obj); > > unpin_active: > > diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c > > index c6023bc9452d..0d8aaaa089cc 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c > > @@ -1149,16 +1149,16 @@ i915_emit_bb_start(struct i915_request *rq, > > int intel_ring_pin(struct intel_ring *ring) > > { > > struct i915_vma *vma = ring->vma; > > - enum i915_map_type map = i915_coherent_map_type(vma->vm->i915); > > unsigned int flags; > > void *addr; > > int ret; > > > > - GEM_BUG_ON(ring->vaddr); > > + if (atomic_fetch_inc(&ring->pin_count)) > > + return 0; > > > > ret = i915_timeline_pin(ring->timeline); > > if (ret) > > - return ret; > > + goto err_unpin; > > > > flags = PIN_GLOBAL; > > > > @@ -1172,26 +1172,31 @@ int intel_ring_pin(struct intel_ring *ring) > > > > ret = i915_vma_pin(vma, 0, 0, flags); > > if (unlikely(ret)) > > - goto unpin_timeline; > > + goto err_timeline; > > > > if (i915_vma_is_map_and_fenceable(vma)) > > addr = (void __force *)i915_vma_pin_iomap(vma); > > else > > - addr = i915_gem_object_pin_map(vma->obj, map); > > + addr = i915_gem_object_pin_map(vma->obj, > > + i915_coherent_map_type(vma->vm->i915)); > > if (IS_ERR(addr)) { > > ret = PTR_ERR(addr); > > - goto unpin_ring; > > + goto err_ring; > > } > > > > vma->obj->pin_global++; > > > > + GEM_BUG_ON(ring->vaddr); > > ring->vaddr = addr; > > + > > return 0; > > > > -unpin_ring: > > +err_ring: > > i915_vma_unpin(vma); > > -unpin_timeline: > > +err_timeline: > > i915_timeline_unpin(ring->timeline); > > +err_unpin: > > + atomic_dec(&ring->pin_count); > > return ret; > > } > > > > @@ -1207,16 +1212,19 @@ void intel_ring_reset(struct intel_ring *ring, u32 tail) > > > > void intel_ring_unpin(struct intel_ring *ring) > > { > > - GEM_BUG_ON(!ring->vma); > > - GEM_BUG_ON(!ring->vaddr); > > + if (!atomic_dec_and_test(&ring->pin_count)) > > + return; > > > > /* Discard any unused bytes beyond that submitted to hw. */ > > intel_ring_reset(ring, ring->tail); > > > > + GEM_BUG_ON(!ring->vma); > > if (i915_vma_is_map_and_fenceable(ring->vma)) > > i915_vma_unpin_iomap(ring->vma); > > else > > i915_gem_object_unpin_map(ring->vma->obj); > > + > > + GEM_BUG_ON(!ring->vaddr); > > ring->vaddr = NULL; > > > > ring->vma->obj->pin_global--; > > diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c > > index 086801b51441..486c6953dcb1 100644 > > --- a/drivers/gpu/drm/i915/gt/mock_engine.c > > +++ b/drivers/gpu/drm/i915/gt/mock_engine.c > > @@ -66,6 +66,7 @@ static struct intel_ring *mock_ring(struct intel_engine_cs *engine) > > ring->base.effective_size = sz; > > ring->base.vaddr = (void *)(ring + 1); > > ring->base.timeline = &ring->timeline; > > + atomic_set(&ring->base.pin_count, 1); > > > > INIT_LIST_HEAD(&ring->base.request_list); > > intel_ring_update_space(&ring->base); > > > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Sigh, CI complained that I left the pin_count unbalanced for ringbuffer. Around we go again. -Chris
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c index 2c454f227c2e..23120901c55f 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.c +++ b/drivers/gpu/drm/i915/gt/intel_context.c @@ -126,6 +126,7 @@ static void intel_context_retire(struct i915_active *active) if (ce->state) __context_unpin_state(ce->state); + intel_ring_unpin(ce->ring); intel_context_put(ce); } @@ -160,27 +161,35 @@ int intel_context_active_acquire(struct intel_context *ce, unsigned long flags) intel_context_get(ce); + err = intel_ring_pin(ce->ring); + if (err) + goto err_put; + if (!ce->state) return 0; err = __context_pin_state(ce->state, flags); - if (err) { - i915_active_cancel(&ce->active); - intel_context_put(ce); - return err; - } + if (err) + goto err_ring; /* Preallocate tracking nodes */ if (!i915_gem_context_is_kernel(ce->gem_context)) { err = i915_active_acquire_preallocate_barrier(&ce->active, ce->engine); - if (err) { - i915_active_release(&ce->active); - return err; - } + if (err) + goto err_state; } return 0; + +err_state: + __context_unpin_state(ce->state); +err_ring: + intel_ring_unpin(ce->ring); +err_put: + intel_context_put(ce); + i915_active_cancel(&ce->active); + return err; } void intel_context_active_release(struct intel_context *ce) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index 868b220214f8..cc6b8eb0cda0 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -70,6 +70,18 @@ struct intel_ring { struct list_head request_list; struct list_head active_link; + /* + * As we have two types of rings, one global to the engine used + * by ringbuffer submission and those that are exclusive to a + * context used by execlists, we have to play safe and allow + * atomic updates to the pin_count. However, the actual pinning + * of the context is either done during initialisation for + * ringbuffer submission or serialised as part of the context + * pinning for execlists, and so we do not need a mutex ourselves + * to * serialise intel_ring_pin/intel_ring_unpin. + */ + atomic_t pin_count; + u32 head; u32 tail; u32 emit; diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index b42b5f158295..82b7ace62d97 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -1414,6 +1414,7 @@ static void execlists_context_destroy(struct kref *kref) { struct intel_context *ce = container_of(kref, typeof(*ce), ref); + GEM_BUG_ON(!i915_active_is_idle(&ce->active)); GEM_BUG_ON(intel_context_is_pinned(ce)); if (ce->state) @@ -1426,7 +1427,6 @@ static void execlists_context_unpin(struct intel_context *ce) { i915_gem_context_unpin_hw_id(ce->gem_context); i915_gem_object_unpin_map(ce->state->obj); - intel_ring_unpin(ce->ring); } static void @@ -1478,13 +1478,9 @@ __execlists_context_pin(struct intel_context *ce, goto unpin_active; } - ret = intel_ring_pin(ce->ring); - if (ret) - goto unpin_map; - ret = i915_gem_context_pin_hw_id(ce->gem_context); if (ret) - goto unpin_ring; + goto unpin_map; ce->lrc_desc = lrc_descriptor(ce, engine); ce->lrc_reg_state = vaddr + LRC_STATE_PN * PAGE_SIZE; @@ -1492,8 +1488,6 @@ __execlists_context_pin(struct intel_context *ce, return 0; -unpin_ring: - intel_ring_unpin(ce->ring); unpin_map: i915_gem_object_unpin_map(ce->state->obj); unpin_active: diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c index c6023bc9452d..0d8aaaa089cc 100644 --- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c @@ -1149,16 +1149,16 @@ i915_emit_bb_start(struct i915_request *rq, int intel_ring_pin(struct intel_ring *ring) { struct i915_vma *vma = ring->vma; - enum i915_map_type map = i915_coherent_map_type(vma->vm->i915); unsigned int flags; void *addr; int ret; - GEM_BUG_ON(ring->vaddr); + if (atomic_fetch_inc(&ring->pin_count)) + return 0; ret = i915_timeline_pin(ring->timeline); if (ret) - return ret; + goto err_unpin; flags = PIN_GLOBAL; @@ -1172,26 +1172,31 @@ int intel_ring_pin(struct intel_ring *ring) ret = i915_vma_pin(vma, 0, 0, flags); if (unlikely(ret)) - goto unpin_timeline; + goto err_timeline; if (i915_vma_is_map_and_fenceable(vma)) addr = (void __force *)i915_vma_pin_iomap(vma); else - addr = i915_gem_object_pin_map(vma->obj, map); + addr = i915_gem_object_pin_map(vma->obj, + i915_coherent_map_type(vma->vm->i915)); if (IS_ERR(addr)) { ret = PTR_ERR(addr); - goto unpin_ring; + goto err_ring; } vma->obj->pin_global++; + GEM_BUG_ON(ring->vaddr); ring->vaddr = addr; + return 0; -unpin_ring: +err_ring: i915_vma_unpin(vma); -unpin_timeline: +err_timeline: i915_timeline_unpin(ring->timeline); +err_unpin: + atomic_dec(&ring->pin_count); return ret; } @@ -1207,16 +1212,19 @@ void intel_ring_reset(struct intel_ring *ring, u32 tail) void intel_ring_unpin(struct intel_ring *ring) { - GEM_BUG_ON(!ring->vma); - GEM_BUG_ON(!ring->vaddr); + if (!atomic_dec_and_test(&ring->pin_count)) + return; /* Discard any unused bytes beyond that submitted to hw. */ intel_ring_reset(ring, ring->tail); + GEM_BUG_ON(!ring->vma); if (i915_vma_is_map_and_fenceable(ring->vma)) i915_vma_unpin_iomap(ring->vma); else i915_gem_object_unpin_map(ring->vma->obj); + + GEM_BUG_ON(!ring->vaddr); ring->vaddr = NULL; ring->vma->obj->pin_global--; diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c index 086801b51441..486c6953dcb1 100644 --- a/drivers/gpu/drm/i915/gt/mock_engine.c +++ b/drivers/gpu/drm/i915/gt/mock_engine.c @@ -66,6 +66,7 @@ static struct intel_ring *mock_ring(struct intel_engine_cs *engine) ring->base.effective_size = sz; ring->base.vaddr = (void *)(ring + 1); ring->base.timeline = &ring->timeline; + atomic_set(&ring->base.pin_count, 1); INIT_LIST_HEAD(&ring->base.request_list); intel_ring_update_space(&ring->base);
Remember to keep the rings pinned as well as the context image until the GPU is no longer active. v2: Introduce a ring->pin_count primarily to hide the mock_ring that doesn't fit into the normal GGTT vma picture. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110946 Fixes: ce476c80b8bf ("drm/i915: Keep contexts pinned until after the next kernel context switch") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/gt/intel_context.c | 27 ++++++++++++------- drivers/gpu/drm/i915/gt/intel_engine_types.h | 12 +++++++++ drivers/gpu/drm/i915/gt/intel_lrc.c | 10 ++----- drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 28 +++++++++++++------- drivers/gpu/drm/i915/gt/mock_engine.c | 1 + 5 files changed, 51 insertions(+), 27 deletions(-)