Re: [Intel-gfx] [PATCH 02/17] drm/i915: Move vma vfuns to adddress_space
Chris Wilson ch...@chris-wilson.co.uk writes: On Tue, Apr 14, 2015 at 05:35:12PM +0200, Daniel Vetter wrote: They change with the address space and not with each vma, so move them into the right pile of vfuncs. Save 2 pointers per vma and clarifies the code. Using per-vma vfunc allows you make, for example, the pagetables themselves an ordinary vma in the GGTT and so operate identically wrt to Page tables don't need to be in ggtt. Is this comment about eviction of page directories (for gen 8)? Thanks, -Mika the shrinker and eviction logic - removing some very fragile code in the process. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/17] drm/i915: Move vma vfuns to adddress_space
On Thu, Apr 16, 2015 at 09:18:48AM +0300, Mika Kuoppala wrote: Chris Wilson ch...@chris-wilson.co.uk writes: On Tue, Apr 14, 2015 at 05:35:12PM +0200, Daniel Vetter wrote: They change with the address space and not with each vma, so move them into the right pile of vfuncs. Save 2 pointers per vma and clarifies the code. Using per-vma vfunc allows you make, for example, the pagetables themselves an ordinary vma in the GGTT and so operate identically wrt to Page tables don't need to be in ggtt. Is this comment about eviction of page directories (for gen 8)? Yes. The root table is always in ggtt. But we don't even have correct context eviction yet... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/17] drm/i915: Move vma vfuns to adddress_space
They change with the address space and not with each vma, so move them
into the right pile of vfuncs. Save 2 pointers per vma and clarifies
the code.
Signed-off-by: Daniel Vetter daniel.vet...@intel.com
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 28
drivers/gpu/drm/i915/i915_gem_gtt.h | 15 +++
3 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8ce363aa492c..4578772c5509 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3035,7 +3035,7 @@ int i915_vma_unbind(struct i915_vma *vma)
trace_i915_vma_unbind(vma);
- vma-unbind_vma(vma);
+ vma-vm-unbind_vma(vma);
list_del_init(vma-mm_list);
if (i915_is_ggtt(vma-vm)) {
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 1c8ef7c143aa..290db48faf27 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -995,6 +995,8 @@ static int gen8_ppgtt_init_common(struct i915_hw_ppgtt
*ppgtt, uint64_t size)
ppgtt-base.cleanup = gen8_ppgtt_cleanup;
ppgtt-base.insert_entries = gen8_ppgtt_insert_entries;
ppgtt-base.clear_range = gen8_ppgtt_clear_range;
+ ppgtt-base.unbind_vma = ppgtt_unbind_vma;
+ ppgtt-base.bind_vma = ppgtt_bind_vma;
ppgtt-switch_mm = gen8_mm_switch;
@@ -1579,6 +1581,8 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
bool aliasing)
ppgtt-base.allocate_va_range = aliasing ? NULL : gen6_alloc_va_range;
ppgtt-base.clear_range = gen6_ppgtt_clear_range;
ppgtt-base.insert_entries = gen6_ppgtt_insert_entries;
+ ppgtt-base.unbind_vma = ppgtt_unbind_vma;
+ ppgtt-base.bind_vma = ppgtt_bind_vma;
ppgtt-base.cleanup = gen6_ppgtt_cleanup;
ppgtt-base.start = 0;
ppgtt-base.total = I915_PDES * GEN6_PTES * PAGE_SIZE;
@@ -2573,6 +2577,8 @@ static int gen8_gmch_probe(struct drm_device *dev,
dev_priv-gtt.base.clear_range = gen8_ggtt_clear_range;
dev_priv-gtt.base.insert_entries = gen8_ggtt_insert_entries;
+ dev_priv-gtt.base.bind_vma = ggtt_bind_vma;
+ dev_priv-gtt.base.unbind_vma = ggtt_unbind_vma;
return ret;
}
@@ -2613,6 +2619,8 @@ static int gen6_gmch_probe(struct drm_device *dev,
dev_priv-gtt.base.clear_range = gen6_ggtt_clear_range;
dev_priv-gtt.base.insert_entries = gen6_ggtt_insert_entries;
+ dev_priv-gtt.base.bind_vma = ggtt_bind_vma;
+ dev_priv-gtt.base.unbind_vma = ggtt_unbind_vma;
return ret;
}
@@ -2645,6 +2653,8 @@ static int i915_gmch_probe(struct drm_device *dev,
dev_priv-gtt.do_idle_maps = needs_idle_maps(dev_priv-dev);
dev_priv-gtt.base.clear_range = i915_ggtt_clear_range;
+ dev_priv-gtt.base.bind_vma = i915_ggtt_bind_vma;
+ dev_priv-gtt.base.unbind_vma = i915_ggtt_unbind_vma;
if (unlikely(dev_priv-gtt.do_idle_maps))
DRM_INFO(applying Ironlake quirks for intel_iommu\n);
@@ -2732,22 +2742,8 @@ __i915_gem_vma_create(struct drm_i915_gem_object *obj,
vma-vm = vm;
vma-obj = obj;
- if (INTEL_INFO(vm-dev)-gen = 6) {
- if (i915_is_ggtt(vm)) {
- vma-ggtt_view = *ggtt_view;
-
- vma-unbind_vma = ggtt_unbind_vma;
- vma-bind_vma = ggtt_bind_vma;
- } else {
- vma-unbind_vma = ppgtt_unbind_vma;
- vma-bind_vma = ppgtt_bind_vma;
- }
- } else {
- BUG_ON(!i915_is_ggtt(vm));
+ if (i915_is_ggtt(vm))
vma-ggtt_view = *ggtt_view;
- vma-unbind_vma = i915_ggtt_unbind_vma;
- vma-bind_vma = i915_ggtt_bind_vma;
- }
list_add_tail(vma-vma_link, obj-vma_list);
if (!i915_is_ggtt(vm))
@@ -2957,7 +2953,7 @@ int i915_vma_bind(struct i915_vma *vma, enum
i915_cache_level cache_level,
return ret;
}
- vma-bind_vma(vma, cache_level, flags);
+ vma-vm-bind_vma(vma, cache_level, flags);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 29de64d1164e..12d0ded0d823 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -196,14 +196,6 @@ struct i915_vma {
* bits with absolutely no headroom. So use 4 bits. */
unsigned int pin_count:4;
#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT 0xf
-
- /** Unmap an object from an address space. This usually consists of
-* setting the valid PTE entries to a reserved scratch page. */
- void (*unbind_vma)(struct i915_vma *vma);
- /* Map an object into an address space with the given cache flags. */
- void (*bind_vma)(struct i915_vma *vma,
-
Re: [Intel-gfx] [PATCH 02/17] drm/i915: Move vma vfuns to adddress_space
On Tue, Apr 14, 2015 at 05:12:30PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 05:09:27PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 05:35:12PM +0200, Daniel Vetter wrote: They change with the address space and not with each vma, so move them into the right pile of vfuncs. Save 2 pointers per vma and clarifies the code. Using per-vma vfunc allows you make, for example, the pagetables themselves an ordinary vma in the GGTT and so operate identically wrt to the shrinker and eviction logic - removing some very fragile code in the process. A vma-ops would be an interesting compromise. Tbh still not really sold on this idea, since the complexit tends to be in the recursion. E.g. see all the fun we had with gpu_idle and the default context. So for now I still prefer things to be explicit. Also that would be a new op to first (try) to unuse the dma. If we don't do this and it fails then the shrinker gets annoyed since the nice hole-punching doesn't work correctly. There's also the fairness question, we'd need to make sure that e.g. the hw context gets cycled through the active list even when we don't switch contexts. Given all that I don't think this patch here is a blocker for doing other vma ops. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/17] drm/i915: Move vma vfuns to adddress_space
On Tue, Apr 14, 2015 at 07:08:35PM +0200, Daniel Vetter wrote: On Tue, Apr 14, 2015 at 05:12:30PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 05:09:27PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 05:35:12PM +0200, Daniel Vetter wrote: They change with the address space and not with each vma, so move them into the right pile of vfuncs. Save 2 pointers per vma and clarifies the code. Using per-vma vfunc allows you make, for example, the pagetables themselves an ordinary vma in the GGTT and so operate identically wrt to the shrinker and eviction logic - removing some very fragile code in the process. A vma-ops would be an interesting compromise. Tbh still not really sold on this idea, since the complexit tends to be in the recursion. E.g. see all the fun we had with gpu_idle and the default context. So for now I still prefer things to be explicit. Ah, you mean the fun that gpu_idle is broken at the moment. I think that lends weight to my argument tbh. Also that would be a new op to first (try) to unuse the dma. If we don't do this and it fails then the shrinker gets annoyed since the nice hole-punching doesn't work correctly. There's also the fairness question, we'd need to make sure that e.g. the hw context gets cycled through the active list even when we don't switch contexts. Yes, all that was taken into account by my patches to sanitize request handling. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/17] drm/i915: Move vma vfuns to adddress_space
On Tue, Apr 14, 2015 at 05:35:12PM +0200, Daniel Vetter wrote: They change with the address space and not with each vma, so move them into the right pile of vfuncs. Save 2 pointers per vma and clarifies the code. Using per-vma vfunc allows you make, for example, the pagetables themselves an ordinary vma in the GGTT and so operate identically wrt to the shrinker and eviction logic - removing some very fragile code in the process. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/17] drm/i915: Move vma vfuns to adddress_space
On Tue, Apr 14, 2015 at 05:09:27PM +0100, Chris Wilson wrote: On Tue, Apr 14, 2015 at 05:35:12PM +0200, Daniel Vetter wrote: They change with the address space and not with each vma, so move them into the right pile of vfuncs. Save 2 pointers per vma and clarifies the code. Using per-vma vfunc allows you make, for example, the pagetables themselves an ordinary vma in the GGTT and so operate identically wrt to the shrinker and eviction logic - removing some very fragile code in the process. A vma-ops would be an interesting compromise. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
