On Fri, Feb 13, 2015 at 01:35:26PM +0000, John Harrison wrote: > Accidentally hit send too early, ignore the other reply! > > On 14/01/2015 11:20, Chris Wilson wrote: > >The biggest user of i915_gem_object_get_page() is the relocation > >processing during execbuffer. Typically userspace passes in a set of > >relocations in sorted order. Sadly, we alternate between relocations > >increasing from the start of the buffers, and relocations decreasing > >from the end. However the majority of consecutive lookups will still be > >in the same page. We could cache the start of the last sg chain, however > >for most callers, the entire sgl is inside a single chain and so we see > >no improve from the extra layer of caching. > > > >References: https://bugs.freedesktop.org/show_bug.cgi?id=88308 > >Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > >--- > > drivers/gpu/drm/i915/i915_drv.h | 31 ++++++++++++++++++++++++++----- > > drivers/gpu/drm/i915/i915_gem.c | 4 ++++ > > 2 files changed, 30 insertions(+), 5 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >index 66f0c607dbef..04a7d594d933 100644 > >--- a/drivers/gpu/drm/i915/i915_drv.h > >+++ b/drivers/gpu/drm/i915/i915_drv.h > >@@ -2005,6 +2005,10 @@ struct drm_i915_gem_object { > > struct sg_table *pages; > > int pages_pin_count; > >+ struct get_page { > >+ struct scatterlist *sg; > >+ int last; > >+ } get_page; > > /* prime dma-buf support */ > > void *dma_buf_vmapping; > >@@ -2612,15 +2616,32 @@ int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj, > > int *needs_clflush); > > int __must_check i915_gem_object_get_pages(struct drm_i915_gem_object *obj); > >-static inline struct page *i915_gem_object_get_page(struct drm_i915_gem_object *obj, int n) > >+ > >+static inline int sg_page_count(struct scatterlist *sg) > >+{ > >+ return PAGE_ALIGN(sg->offset + sg->length) >> PAGE_SHIFT; > Does this need to be rounded up or are sg->offset and sg->length > guaranteed to always be a multiple of the page size? For our sg, sg->offset is always 0, but sg->length may be a multiple of pages. I kept the generic version, but we could just do sg->length >> PAGE_SHIFT. > >+ while (obj->get_page.last + sg_page_count(obj->get_page.sg) <= n) { > >+ obj->get_page.last += sg_page_count(obj->get_page.sg); > >+ if (unlikely(sg_is_chain(++obj->get_page.sg))) > Is it safe to do the ++ inside a nested pair of macros? There is at > least one definition of 'unlikely' in the linux source that would > cause multiple evaluations. That's easy enough to fix. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
