| Message ID | 1429719218-9700-1-git-send-email-michel.thierry@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 22, 2015 at 05:13:38PM +0100, Michel Thierry wrote: > Aliasing ppgtt is fully allocated right after creation, thus shouldn't > need to call allocate_va_range in i915_vma_bind. And? > This duplication started after commit 5c5f645773b6d147bf68c350674dc3ef4f8de83d > ("drm/i915: drm/i915: Unify aliasing ppgtt handling"), as aliasing ppgtt > now also uses allocate_va_range. The choice would be either to allow calling the no-op function, or to not install a vfunc. I would move the tracepoint to the actual allocation side, presumably a real allocate_va_range() will do some filtering and we only really want to know when the available space is updated. -Chris
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6248
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 301/301 301/301
SNB 318/318 318/318
IVB 345/345 345/345
BYT -1 285/285 284/285
BDW 321/321 321/321
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*BYT igt@kms_setmode@invalid-clone-exclusive-crtc PASS(5) DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+0x
Note: You need to pay more attention to line start with '*'
Michel Thierry <michel.thierry@intel.com> writes: > Aliasing ppgtt is fully allocated right after creation, thus shouldn't > need to call allocate_va_range in i915_vma_bind. > > This duplication started after commit 5c5f645773b6d147bf68c350674dc3ef4f8de83d > ("drm/i915: drm/i915: Unify aliasing ppgtt handling"), as aliasing ppgtt > now also uses allocate_va_range. > I understood that Daniel's intention was to unify the initialization and the handling of ppgtt vm areas. The only special case for aliasing ppgtt would be that the whole vm area would be preallocated after generic ppgtt_init (what the Unify patch does). Even with full ppgtt, we get calls to allocate_va_range where there is already structure in place (as we dont teardown vm space). I would prefer to keep the aliasing like that. There might be small performance cost if we omit checking for aliasing early. But we gain common code path and aliasing looks like less special and only contained in init (in this case). Thanks, -Mika > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Mika Kuoppala <mika.kuoppala@intel.com> > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 7b13273..e8c0ab0 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -3242,7 +3242,7 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > { > int ret; > > - if (vma->vm->allocate_va_range) { > + if (vma->vm->allocate_va_range && USES_FULL_PPGTT(dev)) { > trace_i915_va_alloc(vma->vm, vma->node.start, > vma->node.size, > VM_TO_TRACE_NAME(vma->vm)); > -- > 2.1.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 7b13273..e8c0ab0 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3242,7 +3242,7 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, { int ret; - if (vma->vm->allocate_va_range) { + if (vma->vm->allocate_va_range && USES_FULL_PPGTT(dev)) { trace_i915_va_alloc(vma->vm, vma->node.start, vma->node.size, VM_TO_TRACE_NAME(vma->vm));
Aliasing ppgtt is fully allocated right after creation, thus shouldn't need to call allocate_va_range in i915_vma_bind. This duplication started after commit 5c5f645773b6d147bf68c350674dc3ef4f8de83d ("drm/i915: drm/i915: Unify aliasing ppgtt handling"), as aliasing ppgtt now also uses allocate_va_range. Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Signed-off-by: Michel Thierry <michel.thierry@intel.com> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)