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: > > > > Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx> > > > > --- > > > > drivers/gpu/drm/i915/i915_gem.c | 5 +++++ > > > > drivers/gpu/drm/i915/i915_gem_object.h | 3 +++ > > > > 2 files changed, 8 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > > > index 4ca88f2539c0..cbf97f4bbb72 100644 > > > > --- a/drivers/gpu/drm/i915/i915_gem.c > > > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > > > @@ -2441,6 +2441,8 @@ static int ____i915_gem_object_get_pages(struct drm_i915_gem_object *obj) > > > > struct sg_table *pages; > > > > > > > > GEM_BUG_ON(i915_gem_object_has_pinned_pages(obj)); > > > > + GEM_BUG_ON(!is_valid_gtt_page_size(obj->page_size)); > > > > + GEM_BUG_ON(!is_valid_gtt_page_size(obj->gtt_page_size)); > > > > > > > > if (unlikely(obj->mm.madv != I915_MADV_WILLNEED)) { > > > > DRM_DEBUG("Attempting to obtain a purgeable object\n"); > > > > @@ -4159,6 +4161,9 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj, > > > > > > > > obj->ops = ops; > > > > > > > > + obj->page_size = PAGE_SIZE; > > > > + obj->gtt_page_size = I915_GTT_PAGE_SIZE; > > > > + > > > > reservation_object_init(&obj->__builtin_resv); > > > > obj->resv = &obj->__builtin_resv; > > > > > > > > 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. But if you already handle mixed pages, why do we need this still? Viz. this thread, it does seem to cause quite a bit of confusion. And I have no idea why we need the cpu_page_size, we kmap at the page level anyway, and shmem handles the cpu mmap stuff for us. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx