[Intel-gfx] [PATCH 02/18] drm/i915: introduce drm_i915_gem_object page_size members

[Chris Wilson]

On Wed, Apr 05, 2017 at 11:07:55AM +0100, Matthew Auld wrote:
> On 04/05, Chris Wilson wrote:
> > On Wed, Apr 05, 2017 at 08:49:17AM +0200, Daniel Vetter wrote:
> > > On Tue, Apr 04, 2017 at 11:11:12PM +0100, Matthew Auld wrote:
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
> > > > index 174cf923c236..b1dacbfe5173 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem_object.h
> > > > +++ b/drivers/gpu/drm/i915/i915_gem_object.h
> > > > @@ -107,6 +107,9 @@ struct drm_i915_gem_object {
> > > >  	unsigned int cache_level:3;
> > > >  	unsigned int cache_dirty:1;
> > > >  
> > > > +	unsigned int page_size; /* CPU pov - 4K(default), 2M, 1G */
> > > > +	unsigned int gtt_page_size; /* GPU pov - 4K(default), 64K, 2M, 1G */
> > > 
> > > Just kinda archecture review, with a long-term view: Is the plan to
> > > eventually become more flexible here, i.e. allow mixed mode?
> > 
> > Simply put we can not support obj->page_size. Every object will be
> > composed of a mixture of page sizes, often outside of our control and
> > those page sizes may vary over the lifetime of the object.
> > 
> > Trying to design around an a priori static page_size is a bad idea, imo.
> 
> I think I've misrepresented the intention of obj->page_size, it merely
> serves as a hint to get pages, thereafter it represents the minimum page
> size in the mapping and is just bookkeeping, so mixed pages are totally
> fine and expected. I mostly wanted it to make it clear to the reader
> that we have a gtt and cpu page size. I also wanted to know if an object
> is entirely composed of huge-pages for debugfs purposes. I'll try to
> rework this to make it less terrible.

An approach that might be interesting. On pinning the pages
(i.e. ops->get_pages) if we fill in the bitmask of page sizes, something
like

for_each_sg() {
	obj->mm.page_sizes |= fls(sg->length);

obj->mm.pages_sizes &= ~i915->info.page_sizes;

Then in insert_pages or probably better as vma_insert:

	gtt_page_alignment = fls(obj->mm.page_sizes);

I think that will be useful info even prior to trying to put it to good
use, i.e. that will be enough to start dumping debugfs stats over how
frequently we get large allocations.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre