| Message ID | 20210623112637.266855-1-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915/ttm: consider all placements for the page alignment | expand |
On Wed, 2021-06-23 at 12:26 +0100, Matthew Auld wrote: > Just checking the current region is not enough, if we later migrate > the > object somewhere else. For example if the placements are {SMEM, > LMEM}, > then we might get this wrong. Another idea might be to make the > page_alignment part of the ttm_place, instead of the BO. > > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > index c5deb8b7227c..5d894bba6430 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > @@ -753,6 +753,25 @@ void i915_ttm_bo_destroy(struct > ttm_buffer_object *bo) > call_rcu(&obj->rcu, __i915_gem_free_object_rcu); > } > > +static u64 i915_gem_object_page_size(struct drm_i915_gem_object > *obj) > +{ > + u64 page_size; > + int i; > + > + if (!obj->mm.n_placements) > + return obj->mm.region->min_page_size; > + > + page_size = 0; > + for (i = 0; i < obj->mm.n_placements; i++) { > + struct intel_memory_region *mr = obj- > >mm.placements[i]; > + > + page_size = max_t(u64, mr->min_page_size, page_size); > + } > + > + GEM_BUG_ON(!page_size); > + return page_size; > +} > + I think if at all possible, we really should try to avoid the above. Could we, just like in your next patch, perhaps set alignment to 0, indicating that we don't care at the per-object level and something else, indicating that we care. Then the manager could use its default if we don't care and the indicated alignment, even if it's less, if we care at the per object level? /Thomas > /** > * __i915_gem_ttm_object_init - Initialize a ttm-backed i915 gem > object > * @mem: The initial memory region for the object. > @@ -793,7 +812,7 @@ int __i915_gem_ttm_object_init(struct > intel_memory_region *mem, > obj->base.vma_node.driver_private = i915_gem_to_ttm(obj); > ret = ttm_bo_init(&i915->bdev, i915_gem_to_ttm(obj), size, > bo_type, &i915_sys_placement, > - mem->min_page_size >> PAGE_SHIFT, > + i915_gem_object_page_size(obj) >> > PAGE_SHIFT, > true, NULL, NULL, i915_ttm_bo_destroy); > if (!ret) > obj->ttm.created = true;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index c5deb8b7227c..5d894bba6430 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -753,6 +753,25 @@ void i915_ttm_bo_destroy(struct ttm_buffer_object *bo) call_rcu(&obj->rcu, __i915_gem_free_object_rcu); } +static u64 i915_gem_object_page_size(struct drm_i915_gem_object *obj) +{ + u64 page_size; + int i; + + if (!obj->mm.n_placements) + return obj->mm.region->min_page_size; + + page_size = 0; + for (i = 0; i < obj->mm.n_placements; i++) { + struct intel_memory_region *mr = obj->mm.placements[i]; + + page_size = max_t(u64, mr->min_page_size, page_size); + } + + GEM_BUG_ON(!page_size); + return page_size; +} + /** * __i915_gem_ttm_object_init - Initialize a ttm-backed i915 gem object * @mem: The initial memory region for the object. @@ -793,7 +812,7 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem, obj->base.vma_node.driver_private = i915_gem_to_ttm(obj); ret = ttm_bo_init(&i915->bdev, i915_gem_to_ttm(obj), size, bo_type, &i915_sys_placement, - mem->min_page_size >> PAGE_SHIFT, + i915_gem_object_page_size(obj) >> PAGE_SHIFT, true, NULL, NULL, i915_ttm_bo_destroy); if (!ret) obj->ttm.created = true;
Just checking the current region is not enough, if we later migrate the object somewhere else. For example if the placements are {SMEM, LMEM}, then we might get this wrong. Another idea might be to make the page_alignment part of the ttm_place, instead of the BO. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)