| Message ID | 1453504211-7982-5-git-send-email-david.s.gordon@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 22, 2016 at 11:10:09PM +0000, Dave Gordon wrote: > The kunmap() call here didn't match the corresponding kmap(). > The kmapping was changed with the introduction of the GuC-compatible > layout of context objects and the introduction of "LRC_PPHWSP_PN", in > > d167519 drm/i915: Integrate GuC-based command submission > > but the corresponding kunmap() wasn't, probably because the old code > dug into the underlying sgl implementation instead of than calling > "i915_gem_object_get_page(ring->status_page.obj, 0)", which might more > easily have been noticed as containing an assumption about page 0. > > v2: > Split from "handle teardown of HWSP when context is freed". > > Signed-off-by: Dave Gordon <david.s.gordon@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_lrc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 282d66a..a3ab2b4 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1987,7 +1987,10 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *ring) > i915_gem_batch_pool_fini(&ring->batch_pool); > > if (ring->status_page.obj) { > - kunmap(sg_page(ring->status_page.obj->pages->sgl)); > + struct page *page; > + > + page = i915_gem_object_get_page(ring->status_page.obj, LRC_PPHWSP_PN); kunmap(kmap_to_page(ring->status_page.page_addr)); -Chris
On 25/01/16 10:54, Chris Wilson wrote: > On Fri, Jan 22, 2016 at 11:10:09PM +0000, Dave Gordon wrote: >> The kunmap() call here didn't match the corresponding kmap(). >> The kmapping was changed with the introduction of the GuC-compatible >> layout of context objects and the introduction of "LRC_PPHWSP_PN", in >> >> d167519 drm/i915: Integrate GuC-based command submission >> >> but the corresponding kunmap() wasn't, probably because the old code >> dug into the underlying sgl implementation instead of than calling >> "i915_gem_object_get_page(ring->status_page.obj, 0)", which might more >> easily have been noticed as containing an assumption about page 0. >> >> v2: >> Split from "handle teardown of HWSP when context is freed". >> >> Signed-off-by: Dave Gordon <david.s.gordon@intel.com> >> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_lrc.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c >> index 282d66a..a3ab2b4 100644 >> --- a/drivers/gpu/drm/i915/intel_lrc.c >> +++ b/drivers/gpu/drm/i915/intel_lrc.c >> @@ -1987,7 +1987,10 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *ring) >> i915_gem_batch_pool_fini(&ring->batch_pool); >> >> if (ring->status_page.obj) { >> - kunmap(sg_page(ring->status_page.obj->pages->sgl)); >> + struct page *page; >> + >> + page = i915_gem_object_get_page(ring->status_page.obj, LRC_PPHWSP_PN); > > kunmap(kmap_to_page(ring->status_page.page_addr)); > -Chris That's handy, that way it doesn't need to know *what* the offset within the GEM object is. I'll fix that one up in the next cycle :) .Dave.
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 282d66a..a3ab2b4 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1987,7 +1987,10 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *ring) i915_gem_batch_pool_fini(&ring->batch_pool); if (ring->status_page.obj) { - kunmap(sg_page(ring->status_page.obj->pages->sgl)); + struct page *page; + + page = i915_gem_object_get_page(ring->status_page.obj, LRC_PPHWSP_PN); + kunmap(page); ring->status_page.obj = NULL; }
The kunmap() call here didn't match the corresponding kmap(). The kmapping was changed with the introduction of the GuC-compatible layout of context objects and the introduction of "LRC_PPHWSP_PN", in d167519 drm/i915: Integrate GuC-based command submission but the corresponding kunmap() wasn't, probably because the old code dug into the underlying sgl implementation instead of than calling "i915_gem_object_get_page(ring->status_page.obj, 0)", which might more easily have been noticed as containing an assumption about page 0. v2: Split from "handle teardown of HWSP when context is freed". Signed-off-by: Dave Gordon <david.s.gordon@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)