Re: [PATCH v2 4/6] drm/i915: unmap the correct page in intel_logical_ring_cleanup()

[Dave Gordon <david.s.gordon@intel.com>]

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.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx