On Tue, Mar 01, 2016 at 04:33:58PM +0000, Dave Gordon wrote:
> This is essentially Chris Wilson's patch of a similar name, reworked on
> top of Alex Dai's recent patch:
> | drm/i915: Add i915_gem_object_vmap to map GEM object to virtual space
>
> Chris' original commentary said:
> | We now have two implementations for vmapping a whole object, one for
> | dma-buf and one for the ringbuffer. If we couple the vmapping into
> | the obj->pages lifetime, then we can reuse an obj->vmapping for both
> | and at the same time couple it into the shrinker.
> |
> | v2: Mark the failable kmalloc() as __GFP_NOWARN (vsyrjala)
> | v3: Call unpin_vmap from the right dmabuf unmapper
>
> v4: reimplements the same functionality, but now as wrappers round the
> recently-introduced i915_gem_object_vmap_range() from Alex's patch
> mentioned above.
>
> v5: separated from two minor but unrelated changes [Tvrtko Ursulin];
> this is the third and most substantial portion.
>
> Decided not to hold onto vmappings after the pin count goes to zero.
> This may reduce the benefit of Chris' scheme a bit, but does avoid
> any increased risk of exhausting kernel vmap space on 32-bit kernels
> [Tvrtko Ursulin]. Potentially, the vunmap() could be deferred until
> the put_pages() stage if a suitable notifier were written, but we're
> not doing that here. Nonetheless, the simplification of both dmabuf
> and ringbuffer code makes it worthwhile in its own right.
>
> v6: change BUG_ON() to WARN_ON(). [Tvrtko Ursulin]
>
> Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Alex Dai <yu.dai@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 22 ++++++++++++++-----
> drivers/gpu/drm/i915/i915_gem.c | 39 +++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_gem_dmabuf.c | 36 ++++--------------------------
> drivers/gpu/drm/i915/intel_ringbuffer.c | 9 ++++----
> 4 files changed, 65 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b3ae191..f1ad3b3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2172,10 +2172,7 @@ struct drm_i915_gem_object {
> struct scatterlist *sg;
> int last;
> } get_page;
> -
> - /* prime dma-buf support */
> - void *dma_buf_vmapping;
> - int vmapping_count;
> + void *vmapping;
>
> /** Breadcrumb of last rendering to the buffer.
> * There can only be one writer, but we allow for multiple readers.
> @@ -2980,7 +2977,22 @@ static inline void i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
> static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj)
> {
> BUG_ON(obj->pages_pin_count == 0);
> - obj->pages_pin_count--;
> + if (--obj->pages_pin_count == 0 && obj->vmapping) {
> + /*
> + * Releasing the vmapping here may yield less benefit than
> + * if we kept it until put_pages(), but on the other hand
Yields no benefit. Makes the patch pointless.
Plus there is also pressure to enable WC vmaps.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
