| Message ID | 1390616265-4329-1-git-send-email-benjamin.widawsky@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/24/2014 06:17 PM, Ben Widawsky wrote: > From: Chris Wilson <chris@chris-wilson.co.uk> > > This is useful for debugging as we then know that the first entry is > always the global GTT, and all later entries the per-process GTT VM. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Reviewed-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 024e454..946a577 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4634,7 +4634,7 @@ void i915_init_vm(struct drm_i915_private *dev_priv, > INIT_LIST_HEAD(&vm->active_list); > INIT_LIST_HEAD(&vm->inactive_list); > INIT_LIST_HEAD(&vm->global_link); > - list_add(&vm->global_link, &dev_priv->vm_list); > + list_add_tail(&vm->global_link, &dev_priv->vm_list); > } > > void These five patches are: Tested-by: Kenneth Graunke <kenneth@whitecape.org> On Broadwell with drm-intel-nightly, I don't get the batchbuffer as part of my error state, which is a regression. With these patches, it's back again. Thanks for investigating this, Ben.
On Sat, Jan 25, 2014 at 12:32:33AM -0800, Kenneth Graunke wrote: > On 01/24/2014 06:17 PM, Ben Widawsky wrote: > > From: Chris Wilson <chris@chris-wilson.co.uk> > > > > This is useful for debugging as we then know that the first entry is > > always the global GTT, and all later entries the per-process GTT VM. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Reviewed-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/i915_gem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index 024e454..946a577 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -4634,7 +4634,7 @@ void i915_init_vm(struct drm_i915_private *dev_priv, > > INIT_LIST_HEAD(&vm->active_list); > > INIT_LIST_HEAD(&vm->inactive_list); > > INIT_LIST_HEAD(&vm->global_link); > > - list_add(&vm->global_link, &dev_priv->vm_list); > > + list_add_tail(&vm->global_link, &dev_priv->vm_list); > > } > > > > void > > These five patches are: > Tested-by: Kenneth Graunke <kenneth@whitecape.org> > > On Broadwell with drm-intel-nightly, I don't get the batchbuffer as part > of my error state, which is a regression. With these patches, it's back > again. > > Thanks for investigating this, Ben. I want to merge Mika's fixes for the reset stats ioctls, mostly since his patches are older and we also have neat testcases for them all. Then we can reconsider the issue here and how to best get at the right batch buffers. If it breaks development just boot with ppgtt=1 for now, for userspace nothing really changes compared to full ppgtt. -Daniel
On 01/25/2014 12:48 PM, Daniel Vetter wrote: > On Sat, Jan 25, 2014 at 12:32:33AM -0800, Kenneth Graunke wrote: >> On 01/24/2014 06:17 PM, Ben Widawsky wrote: >>> From: Chris Wilson <chris@chris-wilson.co.uk> >>> >>> This is useful for debugging as we then know that the first entry is >>> always the global GTT, and all later entries the per-process GTT VM. >>> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >>> Reviewed-by: Ben Widawsky <ben@bwidawsk.net> >>> --- >>> drivers/gpu/drm/i915/i915_gem.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >>> index 024e454..946a577 100644 >>> --- a/drivers/gpu/drm/i915/i915_gem.c >>> +++ b/drivers/gpu/drm/i915/i915_gem.c >>> @@ -4634,7 +4634,7 @@ void i915_init_vm(struct drm_i915_private *dev_priv, >>> INIT_LIST_HEAD(&vm->active_list); >>> INIT_LIST_HEAD(&vm->inactive_list); >>> INIT_LIST_HEAD(&vm->global_link); >>> - list_add(&vm->global_link, &dev_priv->vm_list); >>> + list_add_tail(&vm->global_link, &dev_priv->vm_list); >>> } >>> >>> void >> >> These five patches are: >> Tested-by: Kenneth Graunke <kenneth@whitecape.org> >> >> On Broadwell with drm-intel-nightly, I don't get the batchbuffer as part >> of my error state, which is a regression. With these patches, it's back >> again. >> >> Thanks for investigating this, Ben. > > I want to merge Mika's fixes for the reset stats ioctls, mostly since his > patches are older and we also have neat testcases for them all. Then we > can reconsider the issue here and how to best get at the right batch > buffers. If it breaks development just boot with ppgtt=1 for now, for > userspace nothing really changes compared to full ppgtt. > -Daniel > Do you mean i915.i915_enable_ppgtt=1? I still get no batchbuffer in the error state either with or without that. But for now, Ben gave me a branch that fixes the issue, so I'll just keep using that until you get this sorted in next/nightly. --Ken
On Sat, Jan 25, 2014 at 01:31:29PM -0800, Kenneth Graunke wrote: > On 01/25/2014 12:48 PM, Daniel Vetter wrote: > > On Sat, Jan 25, 2014 at 12:32:33AM -0800, Kenneth Graunke wrote: > >> On 01/24/2014 06:17 PM, Ben Widawsky wrote: > >>> From: Chris Wilson <chris@chris-wilson.co.uk> > >>> > >>> This is useful for debugging as we then know that the first entry is > >>> always the global GTT, and all later entries the per-process GTT VM. > >>> > >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > >>> Reviewed-by: Ben Widawsky <ben@bwidawsk.net> > >>> --- > >>> drivers/gpu/drm/i915/i915_gem.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > >>> index 024e454..946a577 100644 > >>> --- a/drivers/gpu/drm/i915/i915_gem.c > >>> +++ b/drivers/gpu/drm/i915/i915_gem.c > >>> @@ -4634,7 +4634,7 @@ void i915_init_vm(struct drm_i915_private *dev_priv, > >>> INIT_LIST_HEAD(&vm->active_list); > >>> INIT_LIST_HEAD(&vm->inactive_list); > >>> INIT_LIST_HEAD(&vm->global_link); > >>> - list_add(&vm->global_link, &dev_priv->vm_list); > >>> + list_add_tail(&vm->global_link, &dev_priv->vm_list); > >>> } > >>> > >>> void > >> > >> These five patches are: > >> Tested-by: Kenneth Graunke <kenneth@whitecape.org> > >> > >> On Broadwell with drm-intel-nightly, I don't get the batchbuffer as part > >> of my error state, which is a regression. With these patches, it's back > >> again. > >> > >> Thanks for investigating this, Ben. > > > > I want to merge Mika's fixes for the reset stats ioctls, mostly since his > > patches are older and we also have neat testcases for them all. Then we > > can reconsider the issue here and how to best get at the right batch > > buffers. If it breaks development just boot with ppgtt=1 for now, for > > userspace nothing really changes compared to full ppgtt. > > -Daniel > > > > Do you mean i915.i915_enable_ppgtt=1? I still get no batchbuffer in the > error state either with or without that. > > But for now, Ben gave me a branch that fixes the issue, so I'll just > keep using that until you get this sorted in next/nightly. > > --Ken > Daniel, the issue is exactly with aliasing (any platform >= gen7 with aliasing to be precise). I did ask Ken to try some of Mika's patches before pursuing this, but since then I've asked Mika to send me exactly what I need to review. I am not sure they are exactly what I asked Ken to test. In any case, I will try to review Mika's patches by Monday, however a quick glance lead me to believe they are unrelated to this.
On Sun, Jan 26, 2014 at 6:09 AM, Ben Widawsky <benjamin.widawsky@intel.com> wrote: > Daniel, the issue is exactly with aliasing (any platform >= gen7 with > aliasing to be precise). I did ask Ken to try some of Mika's patches > before pursuing this, but since then I've asked Mika to send me exactly > what I need to review. I am not sure they are exactly what I asked Ken > to test. Oh, iirc I've looked at gen6 and there it seemed to worked ok. Could this just be a ALIASING vs. FULL vs. HW_PPGTT mixup? If so I'd prefer just the minimal patch to remedy that and then fix the full ppgtt dumper in a 2nd step. -Daniel
On Sun, Jan 26, 2014 at 10:26 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Sun, Jan 26, 2014 at 6:09 AM, Ben Widawsky > <benjamin.widawsky@intel.com> wrote: >> Daniel, the issue is exactly with aliasing (any platform >= gen7 with >> aliasing to be precise). I did ask Ken to try some of Mika's patches >> before pursuing this, but since then I've asked Mika to send me exactly >> what I need to review. I am not sure they are exactly what I asked Ken >> to test. > > Oh, iirc I've looked at gen6 and there it seemed to worked ok. Could > this just be a ALIASING vs. FULL vs. HW_PPGTT mixup? If so I'd prefer > just the minimal patch to remedy that and then fix the full ppgtt > dumper in a 2nd step. And since we seem to have a bit a confusion going on with no-ppgtt, aliasing ppgtt and full ppgtt a testcase for this would be useful. Should be fairly simply by submitting a bit of special noise after the MI_BB_END command for the hanging batches in the hangman testcase. Then we can just grep for that in the resulting error state. Bonus points for doing that on each ring and parsing the error state to make sure the cookie dword is indeed in the batch (and not part of some other date we dump). -Daniel
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 024e454..946a577 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4634,7 +4634,7 @@ void i915_init_vm(struct drm_i915_private *dev_priv, INIT_LIST_HEAD(&vm->active_list); INIT_LIST_HEAD(&vm->inactive_list); INIT_LIST_HEAD(&vm->global_link); - list_add(&vm->global_link, &dev_priv->vm_list); + list_add_tail(&vm->global_link, &dev_priv->vm_list); } void