On Thu, Dec 04, 2014 at 09:11:10AM +0000, Chris Wilson wrote:
> On Wed, Dec 03, 2014 at 03:16:09PM +0100, Daniel Vetter wrote:
> > On Tue, Dec 02, 2014 at 04:46:38PM +0000, Chris Wilson wrote:
> > > On Tue, Dec 02, 2014 at 04:19:43PM +0100, Daniel Vetter wrote:
> > > > /* Generate a semi-unique error code. The code is not meant to have meaning, The
> > > > @@ -1085,7 +1083,6 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv,
> > > > const int ndx)
> > > > {
> > > > struct drm_i915_error_buffer *active_bo = NULL, *pinned_bo = NULL;
> > > > - struct drm_i915_gem_object *obj;
> > > > struct i915_vma *vma;
> > > > int i;
> > > >
> > > > @@ -1094,12 +1091,12 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv,
> > > > i++;
> > > > error->active_bo_count[ndx] = i;
> > > >
> > > > - list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> > > > - list_for_each_entry(vma, &obj->vma_list, vma_link)
> > > > - if (vma->vm == vm && vma->pin_count > 0) {
> > > > - i++;
> > > > + if (i915_is_ggtt(vm)) {
> > > > + list_for_each_entry(vma, &vm->inactive_list, mm_list) {
> > > > + if (vma->pin_count == 0)
> > > > break;
> > > > - }
> > > > + i++;
> > > > + }
> > > > }
> > > > error->pinned_bo_count[ndx] = i - error->active_bo_count[ndx];
> > > >
> > > > @@ -1119,7 +1116,7 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv,
> > > > error->pinned_bo_count[ndx] =
> > > > capture_pinned_bo(pinned_bo,
> > > > error->pinned_bo_count[ndx],
> > > > - &dev_priv->mm.bound_list, vm);
> > > > + &vm->inactive_list);
> > >
> > > I much prefer this to be an iteraction over the bound_list and check if
> > > the object has a ggtt bound vma. Combined with the request to only print
> > > out the pinned bo in the GGTT, it becomes much clearer and doesn't need
> > > to result in the confusing "active + pinned for GGTT".
> >
> > We do still want to dump active objects for the GGTT, e.g. shadow batch
> > (well we probably should add the GGTT vm to the dumper ...) and definitely
> > when full ppgtt isn't enabled (i.e. everything).
> >
> > And if we dump the active list then I think we should scan only the
> > inactive list for pinned bo to avoid duplicate listing.
>
> No. The pinned list includes the pinned active bo because it makes it
> easier for me when checking if the right objects are being pinned (I
> only have to crosscheck a single list rather than multiple).
But I've thought the point of patch 2 was to long longer have that
disdinction? Or why did you want to remove the separate pinned_bo list?
Throughroughly confused over here now ..
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
