| Message ID | 1416481925-15138-1-git-send-email-thomas.daniel@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 20, 2014 at 11:12:05AM +0000, Thomas Daniel wrote: > LRC object does not need to be mapped into the GGTT when dumping. Just use > pin_pages. A side-effect of this patch is that a compiler warning goes away > (not checking return value of i915_gem_obj_ggtt_pin). Please explain why you need to pin the pages. -Chris
> -----Original Message----- > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > Sent: Thursday, November 20, 2014 11:16 AM > To: Daniel, Thomas > Cc: intel-gfx@lists.freedesktop.org; akash-goels@gmail.com > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when > dumping in debugfs > > On Thu, Nov 20, 2014 at 11:12:05AM +0000, Thomas Daniel wrote: > > LRC object does not need to be mapped into the GGTT when dumping. Just > > use pin_pages. A side-effect of this patch is that a compiler warning > > goes away (not checking return value of i915_gem_obj_ggtt_pin). > > Please explain why you need to pin the pages. I suppose I don't as this is protected by the struct mutex and unpin is called before returning. Thomas. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
On Thu, Nov 20, 2014 at 12:25 PM, Daniel, Thomas <thomas.daniel@intel.com> wrote: >> -----Original Message----- >> From: Chris Wilson [mailto:chris@chris-wilson.co.uk] >> Sent: Thursday, November 20, 2014 11:16 AM >> To: Daniel, Thomas >> Cc: intel-gfx@lists.freedesktop.org; akash-goels@gmail.com >> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when >> dumping in debugfs >> >> On Thu, Nov 20, 2014 at 11:12:05AM +0000, Thomas Daniel wrote: >> > LRC object does not need to be mapped into the GGTT when dumping. Just >> > use pin_pages. A side-effect of this patch is that a compiler warning >> > goes away (not checking return value of i915_gem_obj_ggtt_pin). >> >> Please explain why you need to pin the pages. > I suppose I don't as this is protected by the struct mutex and unpin is called before returning. Hm right a simple call to i915_gem_object_get_pages should be all that's needed to get at the pages list. Sorry for the misadvise on the internal mail. -Daniel
On Thu, Nov 20, 2014 at 11:25:57AM +0000, Daniel, Thomas wrote: > > -----Original Message----- > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > > Sent: Thursday, November 20, 2014 11:16 AM > > To: Daniel, Thomas > > Cc: intel-gfx@lists.freedesktop.org; akash-goels@gmail.com > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when > > dumping in debugfs > > > > On Thu, Nov 20, 2014 at 11:12:05AM +0000, Thomas Daniel wrote: > > > LRC object does not need to be mapped into the GGTT when dumping. Just > > > use pin_pages. A side-effect of this patch is that a compiler warning > > > goes away (not checking return value of i915_gem_obj_ggtt_pin). > > > > Please explain why you need to pin the pages. > I suppose I don't as this is protected by the struct mutex and unpin is called before returning. The question is: do we need protection against kmalloc and a potential call into the shrinker who may steal the pages from underneath us. Here, we only do a seq_printf() under the lock after get_pages() and that uses a preallocated buffer. -Chris
> -----Original Message----- > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > Sent: Thursday, November 20, 2014 11:41 AM > To: Daniel, Thomas > Cc: intel-gfx@lists.freedesktop.org; akash goel (akash.goels@gmail.com) > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when > dumping in debugfs > > On Thu, Nov 20, 2014 at 11:25:57AM +0000, Daniel, Thomas wrote: > > > -----Original Message----- > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > > > Sent: Thursday, November 20, 2014 11:16 AM > > > To: Daniel, Thomas > > > Cc: intel-gfx@lists.freedesktop.org; akash-goels@gmail.com > > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT > > > when dumping in debugfs > > > > > > On Thu, Nov 20, 2014 at 11:12:05AM +0000, Thomas Daniel wrote: > > > > LRC object does not need to be mapped into the GGTT when dumping. > > > > Just use pin_pages. A side-effect of this patch is that a compiler > > > > warning goes away (not checking return value of > i915_gem_obj_ggtt_pin). > > > > > > Please explain why you need to pin the pages. > > I suppose I don't as this is protected by the struct mutex and unpin is called > before returning. > > The question is: do we need protection against kmalloc and a potential call > into the shrinker who may steal the pages from underneath us. Here, we > only do a seq_printf() under the lock after get_pages() and that uses a > preallocated buffer. I don't think so... If a context is in the context_list then the ctx_obj will have a nonzero refcount. The struct mutex prevents the refcount from changing. Can you identify any situation where the pages may go away? Thomas. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
On Thu, Nov 20, 2014 at 12:09:18PM +0000, Daniel, Thomas wrote: > > -----Original Message----- > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > > Sent: Thursday, November 20, 2014 11:41 AM > > To: Daniel, Thomas > > Cc: intel-gfx@lists.freedesktop.org; akash goel (akash.goels@gmail.com) > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when > > dumping in debugfs > > > > On Thu, Nov 20, 2014 at 11:25:57AM +0000, Daniel, Thomas wrote: > > > > -----Original Message----- > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk] > > > > Sent: Thursday, November 20, 2014 11:16 AM > > > > To: Daniel, Thomas > > > > Cc: intel-gfx@lists.freedesktop.org; akash-goels@gmail.com > > > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT > > > > when dumping in debugfs > > > > > > > > On Thu, Nov 20, 2014 at 11:12:05AM +0000, Thomas Daniel wrote: > > > > > LRC object does not need to be mapped into the GGTT when dumping. > > > > > Just use pin_pages. A side-effect of this patch is that a compiler > > > > > warning goes away (not checking return value of > > i915_gem_obj_ggtt_pin). > > > > > > > > Please explain why you need to pin the pages. > > > I suppose I don't as this is protected by the struct mutex and unpin is called > > before returning. > > > > The question is: do we need protection against kmalloc and a potential call > > into the shrinker who may steal the pages from underneath us. Here, we > > only do a seq_printf() under the lock after get_pages() and that uses a > > preallocated buffer. > I don't think so... If a context is in the context_list then the ctx_obj will have a nonzero refcount. The struct mutex prevents the refcount from changing. The shrinker only steals pages. Well it may reap the active reference count as well, which must also be kept in mind, but the ctx_obj should have the reference from being inside the list and be safe. > Can you identify any situation where the pages may go away? Anytime you trigger an allocation, the system may reap any objects pages. It will even steal the dev->struct_mutex. To protect against the shrinker you have to call pin_pages(). Here, there are no allocations inside the loop and so you don't need to worry about the shrinker stealing your pages. -Chris
On Thu, Nov 20, 2014 at 1:28 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: >> Can you identify any situation where the pages may go away? > > Anytime you trigger an allocation, the system may reap any objects > pages. It will even steal the dev->struct_mutex. To protect against the > shrinker you have to call pin_pages(). Here, there are no allocations > inside the loop and so you don't need to worry about the shrinker > stealing your pages. Hm actually I think better safe than sorry here. At least I have (again) completely forgotten about our dear shrinker ... -Daniel
> -----Original Message----- > From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On Behalf Of > Daniel Vetter > Sent: Thursday, November 20, 2014 12:51 PM > To: Chris Wilson; Daniel, Thomas; intel-gfx@lists.freedesktop.org; akash goel > (akash.goels@gmail.com) > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when > dumping in debugfs > > On Thu, Nov 20, 2014 at 1:28 PM, Chris Wilson <chris@chris-wilson.co.uk> > wrote: > >> Can you identify any situation where the pages may go away? > > > > Anytime you trigger an allocation, the system may reap any objects > > pages. It will even steal the dev->struct_mutex. To protect against > > the shrinker you have to call pin_pages(). Here, there are no > > allocations inside the loop and so you don't need to worry about the > > shrinker stealing your pages. > > Hm actually I think better safe than sorry here. At least I have > (again) completely forgotten about our dear shrinker ... > -Daniel Does this discussion count as a review? What was the conclusion - do I need to make a version without pinning or is it better safe than sorry? Cheers, Thomas. > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Tue, Nov 25, 2014 at 02:30:55PM +0000, Daniel, Thomas wrote: > > -----Original Message----- > > From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On Behalf Of > > Daniel Vetter > > Sent: Thursday, November 20, 2014 12:51 PM > > To: Chris Wilson; Daniel, Thomas; intel-gfx@lists.freedesktop.org; akash goel > > (akash.goels@gmail.com) > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when > > dumping in debugfs > > > > On Thu, Nov 20, 2014 at 1:28 PM, Chris Wilson <chris@chris-wilson.co.uk> > > wrote: > > >> Can you identify any situation where the pages may go away? > > > > > > Anytime you trigger an allocation, the system may reap any objects > > > pages. It will even steal the dev->struct_mutex. To protect against > > > the shrinker you have to call pin_pages(). Here, there are no > > > allocations inside the loop and so you don't need to worry about the > > > shrinker stealing your pages. > > > > Hm actually I think better safe than sorry here. At least I have > > (again) completely forgotten about our dear shrinker ... > > -Daniel > > Does this discussion count as a review? What was the conclusion - do I need to make a version without pinning or is it better safe than sorry? To bring it full circle: >> LRC object does not need to be mapped into the GGTT when dumping. Just use >> pin_pages. A side-effect of this patch is that a compiler warning goes away >> (not checking return value of i915_gem_obj_ggtt_pin). > Please explain why you need to pin the pages. In particular, explain why just calling pin_pages() doesn't do what you want. And then afterwards you can leave a note in the commitlog why you use pin_pages() as overkill. -Chris
On Tue, Nov 25, 2014 at 02:44:33PM +0000, Chris Wilson wrote: > On Tue, Nov 25, 2014 at 02:30:55PM +0000, Daniel, Thomas wrote: > > > -----Original Message----- > > > From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On Behalf Of > > > Daniel Vetter > > > Sent: Thursday, November 20, 2014 12:51 PM > > > To: Chris Wilson; Daniel, Thomas; intel-gfx@lists.freedesktop.org; akash goel > > > (akash.goels@gmail.com) > > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when > > > dumping in debugfs > > > > > > On Thu, Nov 20, 2014 at 1:28 PM, Chris Wilson <chris@chris-wilson.co.uk> > > > wrote: > > > >> Can you identify any situation where the pages may go away? > > > > > > > > Anytime you trigger an allocation, the system may reap any objects > > > > pages. It will even steal the dev->struct_mutex. To protect against > > > > the shrinker you have to call pin_pages(). Here, there are no > > > > allocations inside the loop and so you don't need to worry about the > > > > shrinker stealing your pages. > > > > > > Hm actually I think better safe than sorry here. At least I have > > > (again) completely forgotten about our dear shrinker ... > > > -Daniel > > > > Does this discussion count as a review? What was the conclusion - do I need to make a version without pinning or is it better safe than sorry? > > To bring it full circle: > > >> LRC object does not need to be mapped into the GGTT when dumping. Just use > >> pin_pages. A side-effect of this patch is that a compiler warning goes away > >> (not checking return value of i915_gem_obj_ggtt_pin). > > > Please explain why you need to pin the pages. > > In particular, explain why just calling pin_pages() doesn't do what you > want. And then afterwards you can leave a note in the commitlog why you > use pin_pages() as overkill. Ok I think I see the confusion - we still don't have any error checking, because we don't call get_pages. pin_pages alone doesn't do a whole lot really. We might actually want to put the get_pages into the pin_pages to simplify the interface a bit. Well we need to keep the raw version as __ since some users really don't want a get_pages. -Daniel
> -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel > Vetter > Sent: Tuesday, November 25, 2014 4:00 PM > To: Chris Wilson; Daniel, Thomas; Daniel Vetter; intel- > gfx@lists.freedesktop.org; akash goel (akash.goels@gmail.com) > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when > dumping in debugfs > > On Tue, Nov 25, 2014 at 02:44:33PM +0000, Chris Wilson wrote: > > On Tue, Nov 25, 2014 at 02:30:55PM +0000, Daniel, Thomas wrote: > > > > -----Original Message----- > > > > From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On > > > > Behalf Of Daniel Vetter > > > > Sent: Thursday, November 20, 2014 12:51 PM > > > > To: Chris Wilson; Daniel, Thomas; intel-gfx@lists.freedesktop.org; > > > > akash goel > > > > (akash.goels@gmail.com) > > > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT > > > > when dumping in debugfs > > > > > > > > On Thu, Nov 20, 2014 at 1:28 PM, Chris Wilson > > > > <chris@chris-wilson.co.uk> > > > > wrote: > > > > >> Can you identify any situation where the pages may go away? > > > > > > > > > > Anytime you trigger an allocation, the system may reap any > > > > > objects pages. It will even steal the dev->struct_mutex. To > > > > > protect against the shrinker you have to call pin_pages(). Here, > > > > > there are no allocations inside the loop and so you don't need > > > > > to worry about the shrinker stealing your pages. > > > > > > > > Hm actually I think better safe than sorry here. At least I have > > > > (again) completely forgotten about our dear shrinker ... > > > > -Daniel > > > > > > Does this discussion count as a review? What was the conclusion - do I > need to make a version without pinning or is it better safe than sorry? > > > > To bring it full circle: > > > > >> LRC object does not need to be mapped into the GGTT when dumping. > > >> Just use pin_pages. A side-effect of this patch is that a compiler > > >> warning goes away (not checking return value of > i915_gem_obj_ggtt_pin). > > > > > Please explain why you need to pin the pages. > > > > In particular, explain why just calling pin_pages() doesn't do what > > you want. And then afterwards you can leave a note in the commitlog > > why you use pin_pages() as overkill. > > Ok I think I see the confusion - we still don't have any error checking, > because we don't call get_pages. pin_pages alone doesn't do a whole lot > really. We might actually want to put the get_pages into the pin_pages to > simplify the interface a bit. Well we need to keep the raw version as __ since > some users really don't want a get_pages. > -Daniel Well I'm more confused now. Pin_pages() just increments a refcount. There should already be backing store allocated as the context is in the dev_priv->context_list and we have the struct_mutex. The code calls i915_gem_object_get_page to get a pointer to page 1 of the context object (the logical ring context is here), then kmaps it. What is the benefit of calling get_pages? Do you mean i915_gem_object_ops.get_pages? I don't care if the object is mapped into GTT at this point - I just want to dump it from the CPU. That's why I removed the ggtt_pin in the first place. Thomas.
On Tue, Nov 25, 2014 at 04:42:22PM +0000, Daniel, Thomas wrote: > > -----Original Message----- > > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel > > Vetter > > Sent: Tuesday, November 25, 2014 4:00 PM > > To: Chris Wilson; Daniel, Thomas; Daniel Vetter; intel- > > gfx@lists.freedesktop.org; akash goel (akash.goels@gmail.com) > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when > > dumping in debugfs > > > > On Tue, Nov 25, 2014 at 02:44:33PM +0000, Chris Wilson wrote: > > > On Tue, Nov 25, 2014 at 02:30:55PM +0000, Daniel, Thomas wrote: > > > > > -----Original Message----- > > > > > From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On > > > > > Behalf Of Daniel Vetter > > > > > Sent: Thursday, November 20, 2014 12:51 PM > > > > > To: Chris Wilson; Daniel, Thomas; intel-gfx@lists.freedesktop.org; > > > > > akash goel > > > > > (akash.goels@gmail.com) > > > > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT > > > > > when dumping in debugfs > > > > > > > > > > On Thu, Nov 20, 2014 at 1:28 PM, Chris Wilson > > > > > <chris@chris-wilson.co.uk> > > > > > wrote: > > > > > >> Can you identify any situation where the pages may go away? > > > > > > > > > > > > Anytime you trigger an allocation, the system may reap any > > > > > > objects pages. It will even steal the dev->struct_mutex. To > > > > > > protect against the shrinker you have to call pin_pages(). Here, > > > > > > there are no allocations inside the loop and so you don't need > > > > > > to worry about the shrinker stealing your pages. > > > > > > > > > > Hm actually I think better safe than sorry here. At least I have > > > > > (again) completely forgotten about our dear shrinker ... > > > > > -Daniel > > > > > > > > Does this discussion count as a review? What was the conclusion - do I > > need to make a version without pinning or is it better safe than sorry? > > > > > > To bring it full circle: > > > > > > >> LRC object does not need to be mapped into the GGTT when dumping. > > > >> Just use pin_pages. A side-effect of this patch is that a compiler > > > >> warning goes away (not checking return value of > > i915_gem_obj_ggtt_pin). > > > > > > > Please explain why you need to pin the pages. > > > > > > In particular, explain why just calling pin_pages() doesn't do what > > > you want. And then afterwards you can leave a note in the commitlog > > > why you use pin_pages() as overkill. > > > > Ok I think I see the confusion - we still don't have any error checking, > > because we don't call get_pages. pin_pages alone doesn't do a whole lot > > really. We might actually want to put the get_pages into the pin_pages to > > simplify the interface a bit. Well we need to keep the raw version as __ since > > some users really don't want a get_pages. > > -Daniel > Well I'm more confused now. Pin_pages() just increments a refcount. There should already be backing store allocated as the context is in the dev_priv->context_list and we have the struct_mutex. The code calls i915_gem_object_get_page to get a pointer to page 1 of the context object (the logical ring context is here), then kmaps it. What is the benefit of calling get_pages? Do you mean i915_gem_object_ops.get_pages? I don't care if the object is mapped into GTT at this point - I just want to dump it from the CPU. That's why I removed the ggtt_pin in the first place. The object does not necessarily have any obj->pages at this point. i915_gem_object_get_pages() returns NULL and the code OOPSes. -Chris
On Tue, Nov 25, 2014 at 9:51 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Tue, Nov 25, 2014 at 04:42:22PM +0000, Daniel, Thomas wrote: >> > -----Original Message----- >> > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel >> > Vetter >> > Sent: Tuesday, November 25, 2014 4:00 PM >> > To: Chris Wilson; Daniel, Thomas; Daniel Vetter; intel- >> > gfx@lists.freedesktop.org; akash goel (akash.goels@gmail.com) >> > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT when >> > dumping in debugfs >> > >> > On Tue, Nov 25, 2014 at 02:44:33PM +0000, Chris Wilson wrote: >> > > On Tue, Nov 25, 2014 at 02:30:55PM +0000, Daniel, Thomas wrote: >> > > > > -----Original Message----- >> > > > > From: daniel.vetter@ffwll.ch [mailto:daniel.vetter@ffwll.ch] On >> > > > > Behalf Of Daniel Vetter >> > > > > Sent: Thursday, November 20, 2014 12:51 PM >> > > > > To: Chris Wilson; Daniel, Thomas; intel-gfx@lists.freedesktop.org; >> > > > > akash goel >> > > > > (akash.goels@gmail.com) >> > > > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Don't pin LRC in GGTT >> > > > > when dumping in debugfs >> > > > > >> > > > > On Thu, Nov 20, 2014 at 1:28 PM, Chris Wilson >> > > > > <chris@chris-wilson.co.uk> >> > > > > wrote: >> > > > > >> Can you identify any situation where the pages may go away? >> > > > > > >> > > > > > Anytime you trigger an allocation, the system may reap any >> > > > > > objects pages. It will even steal the dev->struct_mutex. To >> > > > > > protect against the shrinker you have to call pin_pages(). Here, >> > > > > > there are no allocations inside the loop and so you don't need >> > > > > > to worry about the shrinker stealing your pages. >> > > > > >> > > > > Hm actually I think better safe than sorry here. At least I have >> > > > > (again) completely forgotten about our dear shrinker ... >> > > > > -Daniel >> > > > >> > > > Does this discussion count as a review? What was the conclusion - do I >> > need to make a version without pinning or is it better safe than sorry? >> > > >> > > To bring it full circle: >> > > >> > > >> LRC object does not need to be mapped into the GGTT when dumping. >> > > >> Just use pin_pages. A side-effect of this patch is that a compiler >> > > >> warning goes away (not checking return value of >> > i915_gem_obj_ggtt_pin). >> > > >> > > > Please explain why you need to pin the pages. >> > > >> > > In particular, explain why just calling pin_pages() doesn't do what >> > > you want. And then afterwards you can leave a note in the commitlog >> > > why you use pin_pages() as overkill. >> > >> > Ok I think I see the confusion - we still don't have any error checking, >> > because we don't call get_pages. pin_pages alone doesn't do a whole lot >> > really. We might actually want to put the get_pages into the pin_pages to >> > simplify the interface a bit. Well we need to keep the raw version as __ since >> > some users really don't want a get_pages. >> > -Daniel >> Well I'm more confused now. Pin_pages() just increments a refcount. There should already be backing store allocated as the context is in the dev_priv->context_list and we have the struct_mutex. The code calls i915_gem_object_get_page to get a pointer to page 1 of the context object (the logical ring context is here), then kmaps it. What is the benefit of calling get_pages? Do you mean i915_gem_object_ops.get_pages? I don't care if the object is mapped into GTT at this point - I just want to dump it from the CPU. That's why I removed the ggtt_pin in the first place. > > The object does not necessarily have any obj->pages at this point. > i915_gem_object_get_pages() returns NULL and the code OOPSes. The following sequence should be enough to reproduce this: 1. Allocate a few contexts, use them. 2. Thrash memory by allocating&using enough gem buffer objects to hit swap (allocate 1MB objects until you have enough to fill main memory, touch each through gtt mmaps). 3. Read debugfs. Presuming Chris&I are not off the mark that should result in an oops with your patch. With the old code it'll only result in an oops if you also interrupt the process while doing 3 (which means ggtt pinning gets aborted with -EINTR). Can you please do a quick igt? Thanks, Daniel
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index f91e7f7..7e1e7f7 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1801,25 +1801,30 @@ static int i915_dump_lrc(struct seq_file *m, void *unused) struct page *page; uint32_t *reg_state; int j; + unsigned long ggtt_offset = 0; - i915_gem_obj_ggtt_pin(ctx_obj, - GEN8_LR_CONTEXT_ALIGN, 0); - - page = i915_gem_object_get_page(ctx_obj, 1); - reg_state = kmap_atomic(page); + i915_gem_object_pin_pages(ctx_obj); seq_printf(m, "CONTEXT: %s %u\n", ring->name, intel_execlists_ctx_id(ctx_obj)); + if (!i915_gem_obj_ggtt_bound(ctx_obj)) + seq_puts(m, "\tNot bound in GGTT\n"); + else + ggtt_offset = i915_gem_obj_ggtt_offset(ctx_obj); + + page = i915_gem_object_get_page(ctx_obj, 1); + reg_state = kmap_atomic(page); + for (j = 0; j < 0x600 / sizeof(u32) / 4; j += 4) { seq_printf(m, "\t[0x%08lx] 0x%08x 0x%08x 0x%08x 0x%08x\n", - i915_gem_obj_ggtt_offset(ctx_obj) + 4096 + (j * 4), + ggtt_offset + 4096 + (j * 4), reg_state[j], reg_state[j + 1], reg_state[j + 2], reg_state[j + 3]); } kunmap_atomic(reg_state); - i915_gem_object_ggtt_unpin(ctx_obj); + i915_gem_object_unpin_pages(ctx_obj); seq_putc(m, '\n'); }
LRC object does not need to be mapped into the GGTT when dumping. Just use pin_pages. A side-effect of this patch is that a compiler warning goes away (not checking return value of i915_gem_obj_ggtt_pin). Signed-off-by: Thomas Daniel <thomas.daniel@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)