On Wed, Apr 06, 2016 at 10:49:36AM +0100, Tvrtko Ursulin wrote:
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >index 985f067c1f0e..dc8e1b76c896 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -2233,7 +2233,10 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
> > list_del(&obj->global_list);
> >
> > if (obj->vmapping) {
> >- vunmap(obj->vmapping);
> >+ if (obj->base.size == PAGE_SIZE)
> >+ kunmap(kmap_to_page(obj->vmapping));
> >+ else
> >+ vunmap(obj->vmapping);
>
> Can't think of a reason why it would be better but there is also
> is_vmalloc_addr(addr) as used by kvfree.
For consistency with the shrinker (see below).
> > obj->vmapping = NULL;
> > }
> >
> >@@ -2416,15 +2419,22 @@ void *i915_gem_object_pin_vmap(struct drm_i915_gem_object *obj)
> > i915_gem_object_pin_pages(obj);
> >
> > if (obj->vmapping == NULL) {
> >- struct sg_page_iter sg_iter;
> > struct page **pages;
> >- int n;
> >
> >- n = obj->base.size >> PAGE_SHIFT;
> >- pages = drm_malloc_gfp(n, sizeof(*pages), GFP_TEMPORARY);
> >+ pages = NULL;
> >+ if (obj->base.size == PAGE_SIZE)
> >+ obj->vmapping = kmap(sg_page(obj->pages->sgl));
> >+ else
> >+ pages = drm_malloc_gfp(obj->base.size >> PAGE_SHIFT,
> >+ sizeof(*pages),
> >+ GFP_TEMPORARY);
> > if (pages != NULL) {
> >+ struct sg_page_iter sg_iter;
> >+ int n;
> >+
> > n = 0;
> >- for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0)
> >+ for_each_sg_page(obj->pages->sgl, &sg_iter,
> >+ obj->pages->nents, 0)
> > pages[n++] = sg_page_iter_page(&sg_iter);
> >
> > obj->vmapping = vmap(pages, n, 0, PAGE_KERNEL);
> >
>
> Two problems I can spot are:
>
> 1. Callers of pin_vmap now don't know which kind of address they are
> getting. Maybe call it pin_kvmap or something? Just mention in
> kerneldoc could be enough.
I think just mention, and we can rename this to i915_gem_object_pin_map().
Hmm. I liked the pin in the name since it ties to to pin_pages (later
I plan to change that to get_pages and get_vmap/get_map as the pin
becomes implicit).
> 2. Shrinker will try to kick out kmapped objects because they have
> obj->vmapping set.
Not caring that much since the vmap_purge is very heavy weight, but we
can apply is_vmalloc_addr() to the shrinker.
Ok, happy to call this obj->mapping and i915_gem_object_pin_map() ?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
