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 <[email protected]>
> >---
> > 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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
