On Sat, Feb 09, 2013 at 05:27:36PM +0200, Imre Deak wrote:
> So far we created a sparse dma scatter list for gem objects, where each
> scatter list entry represented only a single page. In the future we'll
> have to handle compact scatter lists too where each entry can consist of
> multiple pages, for example for objects imported through PRIME.
>
> The previous patches have already fixed up all other places where the
> i915 driver _walked_ these lists. Here we have the corresponding fix to
> _create_ compact lists. It's not a performance or memory footprint
> improvement, but it helps to better exercise the new logic.
>
> Reference: http://www.spinics.net/lists/dri-devel/msg33917.html
> Signed-off-by: Imre Deak <imre.d...@intel.com>
Just a quick question: Have you checked with printks or so that we indeed
create such coalesced sg entries every once in a while?
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 31 ++++++++++++++++++++++---------
> 1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 4a199e0..1028dd5 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1623,9 +1623,8 @@ i915_gem_object_is_purgeable(struct drm_i915_gem_object
> *obj)
> static void
> i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj)
> {
> - int page_count = obj->base.size / PAGE_SIZE;
> - struct scatterlist *sg;
> - int ret, i;
> + struct drm_sg_iter sg_iter;
> + int ret;
>
> BUG_ON(obj->madv == __I915_MADV_PURGED);
>
> @@ -1645,8 +1644,8 @@ i915_gem_object_put_pages_gtt(struct
> drm_i915_gem_object *obj)
> if (obj->madv == I915_MADV_DONTNEED)
> obj->dirty = 0;
>
> - for_each_sg(obj->pages->sgl, sg, page_count, i) {
> - struct page *page = sg_page(sg);
> + drm_for_each_sg_page(&sg_iter, obj->pages->sgl, 0) {
> + struct page *page = sg_iter.page;
>
> if (obj->dirty)
> set_page_dirty(page);
> @@ -1740,7 +1739,9 @@ i915_gem_object_get_pages_gtt(struct
> drm_i915_gem_object *obj)
> struct address_space *mapping;
> struct sg_table *st;
> struct scatterlist *sg;
> + struct drm_sg_iter sg_iter;
> struct page *page;
> + unsigned long last_pfn = 0; /* suppress gcc warning */
> gfp_t gfp;
>
> /* Assert that the object is not currently in any GPU domain. As it
> @@ -1770,7 +1771,9 @@ i915_gem_object_get_pages_gtt(struct
> drm_i915_gem_object *obj)
> gfp = mapping_gfp_mask(mapping);
> gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
> gfp &= ~(__GFP_IO | __GFP_WAIT);
> - for_each_sg(st->sgl, sg, page_count, i) {
> + sg = st->sgl;
> + st->nents = 0;
> + for (i = 0; i < page_count; i++) {
> page = shmem_read_mapping_page_gfp(mapping, i, gfp);
> if (IS_ERR(page)) {
> i915_gem_purge(dev_priv, page_count);
> @@ -1793,9 +1796,18 @@ i915_gem_object_get_pages_gtt(struct
> drm_i915_gem_object *obj)
> gfp &= ~(__GFP_IO | __GFP_WAIT);
> }
>
> - sg_set_page(sg, page, PAGE_SIZE, 0);
> + if (!i || page_to_pfn(page) != last_pfn + 1) {
> + if (i)
> + sg = sg_next(sg);
> + st->nents++;
> + sg_set_page(sg, page, PAGE_SIZE, 0);
> + } else {
> + sg->length += PAGE_SIZE;
> + }
> + last_pfn = page_to_pfn(page);
> }
>
> + sg_mark_end(sg);
> obj->pages = st;
>
> if (i915_gem_object_needs_bit17_swizzle(obj))
> @@ -1804,8 +1816,9 @@ i915_gem_object_get_pages_gtt(struct
> drm_i915_gem_object *obj)
> return 0;
>
> err_pages:
> - for_each_sg(st->sgl, sg, i, page_count)
> - page_cache_release(sg_page(sg));
> + sg_mark_end(sg);
> + drm_for_each_sg_page(&sg_iter, st->sgl, 0)
> + page_cache_release(sg_iter.page);
> sg_free_table(st);
> kfree(st);
> return PTR_ERR(page);
> --
> 1.7.10.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
