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@xxxxxxxxx>
> >>
> >> 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@xxxxxxxxx>
> >> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> >> ---
> >> 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@xxxxxxxxxxxxxxxxxx>
>
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
