[PATCH v2 2/3] drm/radeon: Restore GART table contents after pinning it in VRAM
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 >> >> 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 at vger.kernel.org >> Signed-off-by: Michel Dänzer >> --- >> >> 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, >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
[PATCH v2 2/3] drm/radeon: Restore GART table contents after pinning it in VRAM
From: Michel DänzerThe 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 at vger.kernel.org Signed-off-by: Michel Dänzer --- 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, >gart.ptr); if (r) radeon_bo_unpin(rdev->gart.robj); + else { + 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); + } radeon_bo_unreserve(rdev->gart.robj); rdev->gart.table_addr = gpu_addr; return r; -- 2.1.4
[PATCH v2 2/3] drm/radeon: Restore GART table contents after pinning it in VRAM
Am 22.01.2015 um 08:30 schrieb Michel Dänzer: > From: Michel Dänzer > > 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 at vger.kernel.org > Signed-off-by: Michel Dänzer > --- > > 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, >gart.ptr); > if (r) > radeon_bo_unpin(rdev->gart.robj); > + else { I would add a comment why we do this here. > + 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. > + } > 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. Christian. > return r;