[Intel-gfx] [PATCH] drm/i915: Trim partial view sg lists

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-09-26 09:30:41)
> 
> On 26/09/2018 09:15, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2018-09-26 09:03:53)
> >> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>
> >> Partial views are small but there can be many of them, and since the sg
> >> list space for them is allocated pessimistically, we can save some slab by
> >> trimming the unused tail entries.
> >>
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> >> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> >> ---
> >> On the other hand with the diminishing importance of mmap ggtt do we
> >> expect to see partial views in use these days?
> > 
> > We have over a decade of hw that depends on them, and mesa is still not
> > entirely rid of them (and there's always i915_dri.so). So partials are
> > very much sticking around for a long time to come.
> > 
> >> ---
> >>   drivers/gpu/drm/i915/i915_drv.h     | 2 ++
> >>   drivers/gpu/drm/i915/i915_gem.c     | 2 +-
> >>   drivers/gpu/drm/i915/i915_gem_gtt.c | 2 ++
> >>   3 files changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 8624b4bdc242..0fe22263aa9b 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -2323,6 +2323,8 @@ static inline struct scatterlist *__sg_next(struct scatterlist *sg)
> >>               (((__iter).curr += PAGE_SIZE) >= (__iter).max) ?           \
> >>               (__iter) = __sgt_iter(__sg_next((__iter).sgp), false), 0 : 0)
> >>   
> >> +bool i915_sg_trim(struct sg_table *orig_st);
> >> +
> >>   static inline unsigned int i915_sg_page_sizes(struct scatterlist *sg)
> >>   {
> >>          unsigned int page_sizes;
> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >> index db9688d14912..7156ddef178b 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem.c
> >> @@ -2491,7 +2491,7 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
> >>          mutex_unlock(&obj->mm.lock);
> >>   }
> >>   
> >> -static bool i915_sg_trim(struct sg_table *orig_st)
> >> +bool i915_sg_trim(struct sg_table *orig_st)
> >>   {
> >>          struct sg_table new_st;
> >>          struct scatterlist *sg, *new_sg;
> > 
> > You can spare me having to move a few lines later if you move this to
> > i915_scatterlist_utils.c?
> 
> To be honest I am feeling optimistic today and said to myself I'd try to 
> add a proper trim to lib/scatterlist.c. It may take a better part of the 
> year to merge it :) but the dumb trim is starting to annoy me.
> >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >> index f6c7ab413081..1ec721c20581 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >> @@ -3835,6 +3835,8 @@ intel_partial_pages(const struct i915_ggtt_view *view,
> >>                  count -= len >> PAGE_SHIFT;
> >>                  if (count == 0) {
> >>                          sg_mark_end(sg);
> >> +                       i915_sg_trim(st); /* Drop any unused tail entries. */
> >> +
> > 
> > Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> 
> With the move to i915_scatterlist_utils.c and can keep it then?
> 
> What do you need the new file for - the error state stuff?

I'm just splitting i915_gem_object.c (again) for async get-pages. Trying
to get down to just the chipset level + ioctl interface in i915_gem.c.
-Chris