| Message ID | 1414769911-13761-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 31, 2014 at 05:38:31PM +0200, Mika Kuoppala wrote: > to move the interface to the ppgtt era. > > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/i915_gpu_error.c | 49 +++++++++++++++++++---------------- > 1 file changed, 27 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > index d17360b..a3e62f00 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -561,16 +561,20 @@ static void i915_error_state_free(struct kref *error_ref) > > static struct drm_i915_error_object * > i915_error_object_create(struct drm_i915_private *dev_priv, > - struct drm_i915_gem_object *src, > - struct i915_address_space *vm) > + struct i915_vma *vma) > { > struct drm_i915_error_object *dst; > - struct i915_vma *vma = NULL; > + struct drm_i915_gem_object *src; > int num_pages; > bool use_ggtt; > int i = 0; > u32 reloc_offset; > > + if (WARN_ON(vma == NULL)) > + return NULL; > + > + src = vma->obj; > + > if (src == NULL || src->pages == NULL) > return NULL; > > @@ -580,23 +584,21 @@ i915_error_object_create(struct drm_i915_private *dev_priv, > if (dst == NULL) > return NULL; > > - if (i915_gem_obj_bound(src, vm)) > - dst->gtt_offset = i915_gem_obj_offset(src, vm); > + if (i915_gem_obj_bound(src, vma->vm)) If we have a vma, it's bound. At least after execbuf and before retiring the cmds. So with the above early exit check (vma == NULL) you can remove this. > + dst->gtt_offset = i915_gem_obj_offset(src, vma->vm); Just look at the vma drm_mm node instead of walking the per-obj vma list again. > else > dst->gtt_offset = -1; > > reloc_offset = dst->gtt_offset; > - if (i915_is_ggtt(vm)) > - vma = i915_gem_obj_to_ggtt(src); > use_ggtt = (src->cache_level == I915_CACHE_NONE && > - vma && (vma->bound & GLOBAL_BIND) && > - reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end); > + (vma->bound & GLOBAL_BIND) && > + reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end); > > /* Cannot access stolen address directly, try to use the aperture */ > if (src->stolen) { > use_ggtt = true; > > - if (!(vma && vma->bound & GLOBAL_BIND)) > + if (!(vma->bound & GLOBAL_BIND)) > goto unwind; > > reloc_offset = i915_gem_obj_ggtt_offset(src); > @@ -658,13 +660,16 @@ unwind: > kfree(dst); > return NULL; > } > -#define i915_error_ggtt_object_create(dev_priv, src) \ > - i915_error_object_create((dev_priv), (src), &(dev_priv)->gtt.base) I would have left this little helper to cut down on diff size, but that's really just a bikeshed. -Daniel
On Mon, Nov 03, 2014 at 03:31:09PM +0100, Daniel Vetter wrote: > On Fri, Oct 31, 2014 at 05:38:31PM +0200, Mika Kuoppala wrote: > > to move the interface to the ppgtt era. > > > > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > > --- > > drivers/gpu/drm/i915/i915_gpu_error.c | 49 +++++++++++++++++++---------------- > > 1 file changed, 27 insertions(+), 22 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > > index d17360b..a3e62f00 100644 > > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > > @@ -561,16 +561,20 @@ static void i915_error_state_free(struct kref *error_ref) > > > > static struct drm_i915_error_object * > > i915_error_object_create(struct drm_i915_private *dev_priv, > > - struct drm_i915_gem_object *src, > > - struct i915_address_space *vm) > > + struct i915_vma *vma) > > { > > struct drm_i915_error_object *dst; > > - struct i915_vma *vma = NULL; > > + struct drm_i915_gem_object *src; > > int num_pages; > > bool use_ggtt; > > int i = 0; > > u32 reloc_offset; > > > > + if (WARN_ON(vma == NULL)) > > + return NULL; Silly, handling NULL here makes the callsites much cleaner. Why didn't you just reuse my ealier patch? -Chris
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index d17360b..a3e62f00 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -561,16 +561,20 @@ static void i915_error_state_free(struct kref *error_ref) static struct drm_i915_error_object * i915_error_object_create(struct drm_i915_private *dev_priv, - struct drm_i915_gem_object *src, - struct i915_address_space *vm) + struct i915_vma *vma) { struct drm_i915_error_object *dst; - struct i915_vma *vma = NULL; + struct drm_i915_gem_object *src; int num_pages; bool use_ggtt; int i = 0; u32 reloc_offset; + if (WARN_ON(vma == NULL)) + return NULL; + + src = vma->obj; + if (src == NULL || src->pages == NULL) return NULL; @@ -580,23 +584,21 @@ i915_error_object_create(struct drm_i915_private *dev_priv, if (dst == NULL) return NULL; - if (i915_gem_obj_bound(src, vm)) - dst->gtt_offset = i915_gem_obj_offset(src, vm); + if (i915_gem_obj_bound(src, vma->vm)) + dst->gtt_offset = i915_gem_obj_offset(src, vma->vm); else dst->gtt_offset = -1; reloc_offset = dst->gtt_offset; - if (i915_is_ggtt(vm)) - vma = i915_gem_obj_to_ggtt(src); use_ggtt = (src->cache_level == I915_CACHE_NONE && - vma && (vma->bound & GLOBAL_BIND) && - reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end); + (vma->bound & GLOBAL_BIND) && + reloc_offset + num_pages * PAGE_SIZE <= dev_priv->gtt.mappable_end); /* Cannot access stolen address directly, try to use the aperture */ if (src->stolen) { use_ggtt = true; - if (!(vma && vma->bound & GLOBAL_BIND)) + if (!(vma->bound & GLOBAL_BIND)) goto unwind; reloc_offset = i915_gem_obj_ggtt_offset(src); @@ -658,13 +660,16 @@ unwind: kfree(dst); return NULL; } -#define i915_error_ggtt_object_create(dev_priv, src) \ - i915_error_object_create((dev_priv), (src), &(dev_priv)->gtt.base) static void capture_bo(struct drm_i915_error_buffer *err, struct i915_vma *vma) { - struct drm_i915_gem_object *obj = vma->obj; + struct drm_i915_gem_object *obj; + + if (WARN_ON(vma == NULL)) + return; + + obj = vma->obj; err->size = obj->base.size; err->name = obj->base.name; @@ -808,8 +813,7 @@ static void gen8_record_semaphore_state(struct drm_i915_private *dev_priv, if (!error->semaphore_obj) error->semaphore_obj = i915_error_object_create(dev_priv, - dev_priv->semaphore_obj, - &dev_priv->gtt.base); + i915_gem_obj_to_ggtt(dev_priv->semaphore_obj)); for_each_ring(to, dev_priv, i) { int idx; @@ -965,7 +969,7 @@ static void i915_gem_record_active_context(struct intel_engine_cs *ring, continue; if ((error->ccid & PAGE_MASK) == i915_gem_obj_ggtt_offset(obj)) { - ering->ctx = i915_error_ggtt_object_create(dev_priv, obj); + ering->ctx = i915_error_object_create(dev_priv, i915_gem_obj_to_ggtt(obj)); break; } } @@ -1005,13 +1009,12 @@ static void i915_gem_record_rings(struct drm_device *dev, */ error->ring[i].batchbuffer = i915_error_object_create(dev_priv, - request->batch_obj, - vm); + i915_gem_obj_to_vma(request->batch_obj, vm)); if (HAS_BROKEN_CS_TLB(dev_priv->dev)) error->ring[i].wa_batchbuffer = - i915_error_ggtt_object_create(dev_priv, - ring->scratch.obj); + i915_error_object_create(dev_priv, + i915_gem_obj_to_ggtt(ring->scratch.obj)); if (request->file_priv) { struct task_struct *task; @@ -1044,10 +1047,12 @@ static void i915_gem_record_rings(struct drm_device *dev, error->ring[i].cpu_ring_tail = rbuf->tail; error->ring[i].ringbuffer = - i915_error_ggtt_object_create(dev_priv, rbuf->obj); + i915_error_object_create(dev_priv, + i915_gem_obj_to_ggtt(rbuf->obj)); error->ring[i].hws_page = - i915_error_ggtt_object_create(dev_priv, ring->status_page.obj); + i915_error_object_create(dev_priv, + i915_gem_obj_to_ggtt(ring->status_page.obj)); i915_gem_record_active_context(ring, error, &error->ring[i]);
to move the interface to the ppgtt era. Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_gpu_error.c | 49 +++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 22 deletions(-)