On Fri, Jul 15, 2016 at 12:31:23PM +0100, Tvrtko Ursulin wrote:
[snip good ideas, leaving the questions]
> > /**
> > * i915_gem_object_unpin_map - releases an earlier mapping
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >index 8f50919..c431b40 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -2471,10 +2471,11 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
> > list_del(&obj->global_list);
> >
> > if (obj->mapping) {
> >- if (is_vmalloc_addr(obj->mapping))
> >- vunmap(obj->mapping);
> >+ void *ptr = (void *)((uintptr_t)obj->mapping & ~1);
>
> How many bits we have to play with here? Is there a suitable define
> somewhere we could use for a mask instead of hardcoded "1" or we
> could add one if you think that would be better?
The mapping is always page-aligned, so bits 0-11 are available.
PAGE_MASK should do the trick
> >- addr = vmap(pages, n_pages, 0, PAGE_KERNEL);
> >+ addr = vmap(pages, n_pages, VM_NO_GUARD,
> >+ use_wc ? pgprot_writecombine(PAGE_KERNEL_IO) : PAGE_KERNEL);
>
> For educational benefit, what is the importance and difference
> between PAGE_KERNEL and PAGE_KERNEL_IO here?
One day I'll find out. We've always tagged our WC mmapings as
PAGE_KERNEL_IO, so cargo-culted it in.
> >
> > if (pages != stack_pages)
> > drm_free_large(pages);
> >@@ -2684,27 +2687,55 @@ static void *i915_gem_object_map(const struct drm_i915_gem_object *obj)
> > }
> >
> > /* get, pin, and map the pages of the object into kernel space */
> >-void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj)
> >+void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, bool use_wc)
> > {
> >+ void *ptr;
> >+ bool has_wc;
> >+ bool pinned;
> > int ret;
> >
> > lockdep_assert_held(&obj->base.dev->struct_mutex);
> >+ GEM_BUG_ON((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0);
> >
> > ret = i915_gem_object_get_pages(obj);
> > if (ret)
> > return ERR_PTR(ret);
> >
> >+ GEM_BUG_ON(obj->pages == NULL);
>
> Looks like belongs to i915_gem_object_get_pages and not to callers.
This is from later where it did:
pinned = true;
if (!atomic_inc_not_zero(&obj->mm.pages_pin_count)) {
ret = ____i915_gem_object_get_pages(obj);
if (ret)
goto err_unlock;
smp_mb__before_atomic();
GEM_BUG_ON(atomic_read(&obj->mm.pages_pin_count));
atomic_set(&obj->mm.pages_pin_count, 1);
pinned = false;
}
GEM_BUG_ON(obj->mm.pages == NULL);
Right now the BUG_ON is superfluous.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
