| Message ID | 20190707210024.26192-5-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/11] drm/i915/gtt: Use shallow dma pages for scratch | expand |
Chris Wilson <chris@chris-wilson.co.uk> writes: > This will be useful to consolidate recursive code. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ > drivers/gpu/drm/i915/i915_gem_gtt.h | 1 + > 2 files changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index da4db76ce054..2fc60e8acd9a 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -1537,6 +1537,8 @@ static void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt) > ppgtt->vm.vma_ops.unbind_vma = ppgtt_unbind_vma; > ppgtt->vm.vma_ops.set_pages = ppgtt_set_pages; > ppgtt->vm.vma_ops.clear_pages = clear_pages; > + > + ppgtt->vm.top = i915_vm_is_4lvl(&ppgtt->vm) ? 3 : 2; Perhaps it becomes evident later in the series why top and not level, so these would be 4 and 3 here. -Mika > } > > static void init_pd_n(struct i915_address_space *vm, > @@ -2086,6 +2088,7 @@ static struct i915_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915) > return ERR_PTR(-ENOMEM); > > ppgtt_init(&ppgtt->base, &i915->gt); > + ppgtt->base.vm.top = 1; > > ppgtt->base.vm.allocate_va_range = gen6_alloc_va_range; > ppgtt->base.vm.clear_range = gen6_ppgtt_clear_range; > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h > index 48bb8c5125e3..119b6d33b266 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.h > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h > @@ -323,6 +323,7 @@ struct i915_address_space { > struct i915_page_dma scratch_pt; > struct i915_page_dma scratch_pd; > struct i915_page_dma scratch_pdp; /* GEN8+ & 48b PPGTT */ > + int top; > > /** > * List of vma currently bound. > -- > 2.20.1
Quoting Mika Kuoppala (2019-07-10 09:17:27) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > This will be useful to consolidate recursive code. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ > > drivers/gpu/drm/i915/i915_gem_gtt.h | 1 + > > 2 files changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index da4db76ce054..2fc60e8acd9a 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -1537,6 +1537,8 @@ static void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt) > > ppgtt->vm.vma_ops.unbind_vma = ppgtt_unbind_vma; > > ppgtt->vm.vma_ops.set_pages = ppgtt_set_pages; > > ppgtt->vm.vma_ops.clear_pages = clear_pages; > > + > > + ppgtt->vm.top = i915_vm_is_4lvl(&ppgtt->vm) ? 3 : 2; > > Perhaps it becomes evident later in the series why top and > not level, so these would be 4 and 3 here. Because we use top and not level :) -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Mika Kuoppala (2019-07-10 09:17:27) >> Chris Wilson <chris@chris-wilson.co.uk> writes: >> >> > This will be useful to consolidate recursive code. >> > >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> > --- >> > drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ >> > drivers/gpu/drm/i915/i915_gem_gtt.h | 1 + >> > 2 files changed, 4 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c >> > index da4db76ce054..2fc60e8acd9a 100644 >> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >> > @@ -1537,6 +1537,8 @@ static void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt) >> > ppgtt->vm.vma_ops.unbind_vma = ppgtt_unbind_vma; >> > ppgtt->vm.vma_ops.set_pages = ppgtt_set_pages; >> > ppgtt->vm.vma_ops.clear_pages = clear_pages; >> > + >> > + ppgtt->vm.top = i915_vm_is_4lvl(&ppgtt->vm) ? 3 : 2; >> >> Perhaps it becomes evident later in the series why top and >> not level, so these would be 4 and 3 here. > > Because we use top and not level :) You make me substract one with my biological processor. It is hard. Please do remake the i915_vm_is_4lvl() and include. -Mika
Quoting Mika Kuoppala (2019-07-10 15:25:38) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Quoting Mika Kuoppala (2019-07-10 09:17:27) > >> Chris Wilson <chris@chris-wilson.co.uk> writes: > >> > >> > This will be useful to consolidate recursive code. > >> > > >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > >> > --- > >> > drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ > >> > drivers/gpu/drm/i915/i915_gem_gtt.h | 1 + > >> > 2 files changed, 4 insertions(+) > >> > > >> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > >> > index da4db76ce054..2fc60e8acd9a 100644 > >> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > >> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > >> > @@ -1537,6 +1537,8 @@ static void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt) > >> > ppgtt->vm.vma_ops.unbind_vma = ppgtt_unbind_vma; > >> > ppgtt->vm.vma_ops.set_pages = ppgtt_set_pages; > >> > ppgtt->vm.vma_ops.clear_pages = clear_pages; > >> > + > >> > + ppgtt->vm.top = i915_vm_is_4lvl(&ppgtt->vm) ? 3 : 2; > >> > >> Perhaps it becomes evident later in the series why top and > >> not level, so these would be 4 and 3 here. > > > > Because we use top and not level :) > > You make me substract one with my biological processor. > It is hard. > > Please do remake the i915_vm_is_4lvl() and include. I'm tempted to put the gtt_depth in the device info. How do you want i915_vm_is_4lvl() remade? The special case going forward is really is_3lvl? -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Mika Kuoppala (2019-07-10 15:25:38) >> Chris Wilson <chris@chris-wilson.co.uk> writes: >> >> > Quoting Mika Kuoppala (2019-07-10 09:17:27) >> >> Chris Wilson <chris@chris-wilson.co.uk> writes: >> >> >> >> > This will be useful to consolidate recursive code. >> >> > >> >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> >> > --- >> >> > drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ >> >> > drivers/gpu/drm/i915/i915_gem_gtt.h | 1 + >> >> > 2 files changed, 4 insertions(+) >> >> > >> >> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c >> >> > index da4db76ce054..2fc60e8acd9a 100644 >> >> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >> >> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >> >> > @@ -1537,6 +1537,8 @@ static void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt) >> >> > ppgtt->vm.vma_ops.unbind_vma = ppgtt_unbind_vma; >> >> > ppgtt->vm.vma_ops.set_pages = ppgtt_set_pages; >> >> > ppgtt->vm.vma_ops.clear_pages = clear_pages; >> >> > + >> >> > + ppgtt->vm.top = i915_vm_is_4lvl(&ppgtt->vm) ? 3 : 2; >> >> >> >> Perhaps it becomes evident later in the series why top and >> >> not level, so these would be 4 and 3 here. >> > >> > Because we use top and not level :) >> >> You make me substract one with my biological processor. >> It is hard. >> >> Please do remake the i915_vm_is_4lvl() and include. > > I'm tempted to put the gtt_depth in the device info. > > How do you want i915_vm_is_4lvl() remade? The special case going > forward is really is_3lvl? No strong feelings here. How about i915_vm_get_lvl(vm) { return top + 1; } ? But anything which looks sleek on callsites is fine. -Mika
Quoting Mika Kuoppala (2019-07-10 15:50:37) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Quoting Mika Kuoppala (2019-07-10 15:25:38) > >> Chris Wilson <chris@chris-wilson.co.uk> writes: > >> > >> > Quoting Mika Kuoppala (2019-07-10 09:17:27) > >> >> Chris Wilson <chris@chris-wilson.co.uk> writes: > >> >> > >> >> > This will be useful to consolidate recursive code. > >> >> > > >> >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > >> >> > --- > >> >> > drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ > >> >> > drivers/gpu/drm/i915/i915_gem_gtt.h | 1 + > >> >> > 2 files changed, 4 insertions(+) > >> >> > > >> >> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > >> >> > index da4db76ce054..2fc60e8acd9a 100644 > >> >> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > >> >> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > >> >> > @@ -1537,6 +1537,8 @@ static void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt) > >> >> > ppgtt->vm.vma_ops.unbind_vma = ppgtt_unbind_vma; > >> >> > ppgtt->vm.vma_ops.set_pages = ppgtt_set_pages; > >> >> > ppgtt->vm.vma_ops.clear_pages = clear_pages; > >> >> > + > >> >> > + ppgtt->vm.top = i915_vm_is_4lvl(&ppgtt->vm) ? 3 : 2; > >> >> > >> >> Perhaps it becomes evident later in the series why top and > >> >> not level, so these would be 4 and 3 here. > >> > > >> > Because we use top and not level :) > >> > >> You make me substract one with my biological processor. > >> It is hard. > >> > >> Please do remake the i915_vm_is_4lvl() and include. > > > > I'm tempted to put the gtt_depth in the device info. > > > > How do you want i915_vm_is_4lvl() remade? The special case going > > forward is really is_3lvl? > > No strong feelings here. How about i915_vm_get_lvl(vm) > { return top + 1; } ? Who's going to be calling get_lvl() though? The one time where it might be useful, we just use "<= top" instead. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Mika Kuoppala (2019-07-10 15:50:37) >> Chris Wilson <chris@chris-wilson.co.uk> writes: >> >> > Quoting Mika Kuoppala (2019-07-10 15:25:38) >> >> Chris Wilson <chris@chris-wilson.co.uk> writes: >> >> >> >> > Quoting Mika Kuoppala (2019-07-10 09:17:27) >> >> >> Chris Wilson <chris@chris-wilson.co.uk> writes: >> >> >> >> >> >> > This will be useful to consolidate recursive code. >> >> >> > >> >> >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> >> >> > --- >> >> >> > drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ >> >> >> > drivers/gpu/drm/i915/i915_gem_gtt.h | 1 + >> >> >> > 2 files changed, 4 insertions(+) >> >> >> > >> >> >> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c >> >> >> > index da4db76ce054..2fc60e8acd9a 100644 >> >> >> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >> >> >> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >> >> >> > @@ -1537,6 +1537,8 @@ static void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt) >> >> >> > ppgtt->vm.vma_ops.unbind_vma = ppgtt_unbind_vma; >> >> >> > ppgtt->vm.vma_ops.set_pages = ppgtt_set_pages; >> >> >> > ppgtt->vm.vma_ops.clear_pages = clear_pages; >> >> >> > + >> >> >> > + ppgtt->vm.top = i915_vm_is_4lvl(&ppgtt->vm) ? 3 : 2; >> >> >> >> >> >> Perhaps it becomes evident later in the series why top and >> >> >> not level, so these would be 4 and 3 here. >> >> > >> >> > Because we use top and not level :) >> >> >> >> You make me substract one with my biological processor. >> >> It is hard. >> >> >> >> Please do remake the i915_vm_is_4lvl() and include. >> > >> > I'm tempted to put the gtt_depth in the device info. >> > >> > How do you want i915_vm_is_4lvl() remade? The special case going >> > forward is really is_3lvl? >> >> No strong feelings here. How about i915_vm_get_lvl(vm) >> { return top + 1; } ? > > Who's going to be calling get_lvl() though? The one time where it might > be useful, we just use "<= top" instead. Hmm right, prolly way too generic to query for lvl and compare. So then it is the one which reads best on the few callsites it will sit on. -Mika
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index da4db76ce054..2fc60e8acd9a 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1537,6 +1537,8 @@ static void ppgtt_init(struct i915_ppgtt *ppgtt, struct intel_gt *gt) ppgtt->vm.vma_ops.unbind_vma = ppgtt_unbind_vma; ppgtt->vm.vma_ops.set_pages = ppgtt_set_pages; ppgtt->vm.vma_ops.clear_pages = clear_pages; + + ppgtt->vm.top = i915_vm_is_4lvl(&ppgtt->vm) ? 3 : 2; } static void init_pd_n(struct i915_address_space *vm, @@ -2086,6 +2088,7 @@ static struct i915_ppgtt *gen6_ppgtt_create(struct drm_i915_private *i915) return ERR_PTR(-ENOMEM); ppgtt_init(&ppgtt->base, &i915->gt); + ppgtt->base.vm.top = 1; ppgtt->base.vm.allocate_va_range = gen6_alloc_va_range; ppgtt->base.vm.clear_range = gen6_ppgtt_clear_range; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 48bb8c5125e3..119b6d33b266 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -323,6 +323,7 @@ struct i915_address_space { struct i915_page_dma scratch_pt; struct i915_page_dma scratch_pd; struct i915_page_dma scratch_pdp; /* GEN8+ & 48b PPGTT */ + int top; /** * List of vma currently bound.
This will be useful to consolidate recursive code. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ drivers/gpu/drm/i915/i915_gem_gtt.h | 1 + 2 files changed, 4 insertions(+)