| Message ID | 1430232977-8789-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 28, 2015 at 05:56:17PM +0300, Mika Kuoppala wrote: > When we have bound vma into an address space, the layout > of page table structures is immutable. So we can be absolutely > certain that if vma is already bound, there is no need to > (re)allocate a virtual address range for it. > > v2: - add sanity checks and remove superfluous GLOBAL_BIND set > - we might do update for an unbound vma (Chris) > > Testcase: igt/gem_exec_big #bdw > Reported-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Michel Thierry <michel.thierry@intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 39 +++++++++++++++++++++---------------- > 1 file changed, 22 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 6fae6bd..9d3852c 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -1928,8 +1928,6 @@ static int ggtt_bind_vma(struct i915_vma *vma, > vma->vm->insert_entries(vma->vm, pages, > vma->node.start, > cache_level, pte_flags); > - > - vma->bound |= GLOBAL_BIND; > } > > if (dev_priv->mm.aliasing_ppgtt && flags & LOCAL_BIND) { > @@ -2804,21 +2802,13 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma) > int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > u32 flags) > { > - int ret = 0; > - u32 bind_flags = 0; > - > - if (vma->vm->allocate_va_range) { > - trace_i915_va_alloc(vma->vm, vma->node.start, > - vma->node.size, > - VM_TO_TRACE_NAME(vma->vm)); > + int ret; > + u32 bind_flags; Whilst you are here, can you fix this to be plain unsigned. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6279
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 302/302 302/302
SNB 316/316 316/316
IVB 264/264 264/264
BYT -3 227/227 224/227
BDW 318/318 318/318
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
BYT igt@drm_vma_limiter_cached FAIL(3)PASS(3) FAIL(2)
*BYT igt@gem_exec_params@rsvd2-dirt FAIL(1)PASS(2) 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
BYT igt@gem_pipe_control_store_loop@fresh-buffer FAIL(1)TIMEOUT(2)PASS(3) TIMEOUT(1)PASS(1)
Note: You need to pay more attention to line start with '*'
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 6fae6bd..9d3852c 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1928,8 +1928,6 @@ static int ggtt_bind_vma(struct i915_vma *vma, vma->vm->insert_entries(vma->vm, pages, vma->node.start, cache_level, pte_flags); - - vma->bound |= GLOBAL_BIND; } if (dev_priv->mm.aliasing_ppgtt && flags & LOCAL_BIND) { @@ -2804,21 +2802,13 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma) int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, u32 flags) { - int ret = 0; - u32 bind_flags = 0; - - if (vma->vm->allocate_va_range) { - trace_i915_va_alloc(vma->vm, vma->node.start, - vma->node.size, - VM_TO_TRACE_NAME(vma->vm)); + int ret; + u32 bind_flags; - ret = vma->vm->allocate_va_range(vma->vm, - vma->node.start, - vma->node.size); - if (ret) - return ret; - } + if (WARN_ON(flags == 0)) + return -EINVAL; + bind_flags = 0; if (flags & PIN_GLOBAL) bind_flags |= GLOBAL_BIND; if (flags & PIN_USER) @@ -2829,8 +2819,23 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, else bind_flags &= ~vma->bound; - if (bind_flags) - ret = vma->vm->bind_vma(vma, cache_level, bind_flags); + if (bind_flags == 0) + return 0; + + if (vma->bound == 0 && vma->vm->allocate_va_range) { + trace_i915_va_alloc(vma->vm, + vma->node.start, + vma->node.size, + VM_TO_TRACE_NAME(vma->vm)); + + ret = vma->vm->allocate_va_range(vma->vm, + vma->node.start, + vma->node.size); + if (ret) + return ret; + } + + ret = vma->vm->bind_vma(vma, cache_level, bind_flags); if (ret) return ret;
When we have bound vma into an address space, the layout of page table structures is immutable. So we can be absolutely certain that if vma is already bound, there is no need to (re)allocate a virtual address range for it. v2: - add sanity checks and remove superfluous GLOBAL_BIND set - we might do update for an unbound vma (Chris) Testcase: igt/gem_exec_big #bdw Reported-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 39 +++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 17 deletions(-)