| Message ID | 20200708134742.727-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] drm/i915/gt: Replace opencoded i915_gem_object_pin_map() | expand |
Quoting Chris Wilson (2020-07-08 14:47:37) > Some objects we map once during their construction, and then never > access their mappings again, even if they are kept around for the > duration of the driver. Keeping those pages mapped, often vmapped, is > therefore wasteful and we should release the maps as soon as we no > longer need them. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/gem/i915_gem_object.h | 2 ++ > drivers/gpu/drm/i915/gem/i915_gem_pages.c | 20 +++++++++++++++++++ > drivers/gpu/drm/i915/gt/gen7_renderclear.c | 1 + > drivers/gpu/drm/i915/gt/intel_lrc.c | 1 + > .../gpu/drm/i915/gt/intel_ring_submission.c | 1 + > drivers/gpu/drm/i915/i915_perf.c | 2 ++ > 6 files changed, 27 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h > index 2faa481cc18f..a26a4faf014f 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h > @@ -394,6 +394,8 @@ static inline void i915_gem_object_unpin_map(struct drm_i915_gem_object *obj) > i915_gem_object_unpin_pages(obj); > } > > +int i915_gem_object_release_map(struct drm_i915_gem_object *obj); > + > void > i915_gem_object_flush_write_domain(struct drm_i915_gem_object *obj, > unsigned int flush_domains); > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c > index af9e48ee4a33..114256014a97 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c > @@ -408,6 +408,26 @@ void __i915_gem_object_flush_map(struct drm_i915_gem_object *obj, > } > } > > +int i915_gem_object_release_map(struct drm_i915_gem_object *obj) > +{ > + int err; > + > + err = mutex_lock_interruptible(&obj->mm.lock); > + if (err) > + return err; > + > + if (atomic_read(&obj->mm.pages_pin_count)) { > + err = -EBUSY; Hmm. I realise this slightly defeats the efficacy of the patch, since we often have a pinned vma at the time. Since we only call this at known points, we can just say the caller knows best and force the unmap. If we get it wrong, we'll have a very weird GPF. -Chris
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h index 2faa481cc18f..a26a4faf014f 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h @@ -394,6 +394,8 @@ static inline void i915_gem_object_unpin_map(struct drm_i915_gem_object *obj) i915_gem_object_unpin_pages(obj); } +int i915_gem_object_release_map(struct drm_i915_gem_object *obj); + void i915_gem_object_flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c index af9e48ee4a33..114256014a97 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c @@ -408,6 +408,26 @@ void __i915_gem_object_flush_map(struct drm_i915_gem_object *obj, } } +int i915_gem_object_release_map(struct drm_i915_gem_object *obj) +{ + int err; + + err = mutex_lock_interruptible(&obj->mm.lock); + if (err) + return err; + + if (atomic_read(&obj->mm.pages_pin_count)) { + err = -EBUSY; + } else if (obj->mm.mapping) { + unmap_object(obj, page_mask_bits(obj->mm.mapping)); + obj->mm.mapping = NULL; + } + + mutex_unlock(&obj->mm.lock); + + return err; +} + struct scatterlist * i915_gem_object_get_sg(struct drm_i915_gem_object *obj, unsigned int n, diff --git a/drivers/gpu/drm/i915/gt/gen7_renderclear.c b/drivers/gpu/drm/i915/gt/gen7_renderclear.c index de595b66a746..d2a94002ae6e 100644 --- a/drivers/gpu/drm/i915/gt/gen7_renderclear.c +++ b/drivers/gpu/drm/i915/gt/gen7_renderclear.c @@ -397,6 +397,7 @@ int gen7_setup_clear_gpr_bb(struct intel_engine_cs * const engine, i915_gem_object_flush_map(vma->obj); i915_gem_object_unpin_map(vma->obj); + i915_gem_object_release_map(vma->obj); return 0; } diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index 02a38810bcd3..476dfc6be15e 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -3938,6 +3938,7 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine) __i915_gem_object_flush_map(wa_ctx->vma->obj, 0, batch_ptr - batch); i915_gem_object_unpin_map(wa_ctx->vma->obj); + i915_gem_object_release_map(wa_ctx->vma->obj); if (ret) lrc_destroy_wa_ctx(engine); diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c index 68a08486fc87..a1e1d24da2fb 100644 --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c @@ -544,6 +544,7 @@ alloc_context_vma(struct intel_engine_cs *engine) i915_gem_object_flush_map(obj); i915_gem_object_unpin_map(obj); + i915_gem_object_release_map(obj); } vma = i915_vma_instance(obj, &engine->gt->ggtt->vm, NULL); diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 25329b7600c9..34f82e0e912a 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -1773,6 +1773,7 @@ static int alloc_noa_wait(struct i915_perf_stream *stream) i915_gem_object_flush_map(bo); i915_gem_object_unpin_map(bo); + i915_gem_object_release_map(bo); stream->noa_wait = vma; return 0; @@ -1868,6 +1869,7 @@ alloc_oa_config_buffer(struct i915_perf_stream *stream, i915_gem_object_flush_map(obj); i915_gem_object_unpin_map(obj); + i915_gem_object_release_map(obj); oa_bo->vma = i915_vma_instance(obj, &stream->engine->gt->ggtt->vm,
Some objects we map once during their construction, and then never access their mappings again, even if they are kept around for the duration of the driver. Keeping those pages mapped, often vmapped, is therefore wasteful and we should release the maps as soon as we no longer need them. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/gem/i915_gem_object.h | 2 ++ drivers/gpu/drm/i915/gem/i915_gem_pages.c | 20 +++++++++++++++++++ drivers/gpu/drm/i915/gt/gen7_renderclear.c | 1 + drivers/gpu/drm/i915/gt/intel_lrc.c | 1 + .../gpu/drm/i915/gt/intel_ring_submission.c | 1 + drivers/gpu/drm/i915/i915_perf.c | 2 ++ 6 files changed, 27 insertions(+)