Re: [PATCH v2 2/3] drm/radeon: Restore GART table contents after pinning it in VRAM

["Michel Dänzer" <michel@daenzer.net>]

On 22.01.2015 18:06, Christian König wrote:
> Am 22.01.2015 um 08:30 schrieb Michel Dänzer:
>> From: Michel Dänzer <michel.daenzer@amd.com>
>>
>> The GART table BO has to be moved out of VRAM for suspend/resume. Any
>> updates to the GART table during that time were silently dropped without
>> this change. This caused GPU lockups on resume in some cases, see the bug
>> reports referenced below.
>>
>> This might also make GPU reset more robust in some cases, as we no longer
>> rely on the GART table in VRAM being preserved across the GPU
>> lockup/reset.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85204
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86267
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>> ---
>>
>> v2: Add logic to radeon_gart_table_vram_pin directly instead of
>> reinstating
>>      radeon_gart_restore function
>>
>>   drivers/gpu/drm/radeon/radeon_gart.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_gart.c
>> b/drivers/gpu/drm/radeon/radeon_gart.c
>> index a530932..0c8c739 100644
>> --- a/drivers/gpu/drm/radeon/radeon_gart.c
>> +++ b/drivers/gpu/drm/radeon/radeon_gart.c
>> @@ -163,6 +163,14 @@ int radeon_gart_table_vram_pin(struct
>> radeon_device *rdev)
>>       r = radeon_bo_kmap(rdev->gart.robj, &rdev->gart.ptr);
>>       if (r)
>>           radeon_bo_unpin(rdev->gart.robj);
>> +    else {
> 
> I would add a comment why we do this here.

Added in v3.


>> +        int i;
>> +
>> +        for (i = 0; i < rdev->gart.num_gpu_pages; i++)
>> +            radeon_gart_set_page(rdev, i, rdev->gart.pages_entry[i]);
>> +        mb();
>> +        radeon_gart_tlb_flush(rdev);
> 
> That TLB flush won't work correctly because the table_addr isn't up to
> date yet.

Ugh, thanks for the catch.

>> +    }
>>       radeon_bo_unreserve(rdev->gart.robj);
>>       rdev->gart.table_addr = gpu_addr;
> It's updated here instead. Maybe completely drop the local gpu_addr
> variable and update the table_addr directly instead.

I chose the less invasive fix in v3.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel