From: Michel Dänzer michel.daenzer@amd.com
The hardware only takes 27 bits for the offset, so larger offsets are truncated, and the hardware cursor shows random bits other than the intended ones.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46796
Cc: stable@vger.kernel.org Signed-off-by: Michel Dänzer michel.daenzer@amd.com --- drivers/gpu/drm/radeon/radeon_cursor.c | 13 +++++++++++-- drivers/gpu/drm/radeon/radeon_object.c | 18 +++++++++++++++++- drivers/gpu/drm/radeon/radeon_object.h | 2 ++ 3 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c index fde25c0..986d608 100644 --- a/drivers/gpu/drm/radeon/radeon_cursor.c +++ b/drivers/gpu/drm/radeon/radeon_cursor.c @@ -151,7 +151,9 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc, uint32_t height) { struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc); + struct radeon_device *rdev = crtc->dev->dev_private; struct drm_gem_object *obj; + struct radeon_bo *robj; uint64_t gpu_addr; int ret;
@@ -173,7 +175,15 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc, return -ENOENT; }
- ret = radeon_gem_object_pin(obj, RADEON_GEM_DOMAIN_VRAM, &gpu_addr); + robj = gem_to_radeon_bo(obj); + ret = radeon_bo_reserve(robj, false); + if (unlikely(ret != 0)) + goto fail; + /* Only 27 bit offset for legacy cursor */ + ret = radeon_bo_pin_restricted(robj, RADEON_GEM_DOMAIN_VRAM, + ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27, + &gpu_addr); + radeon_bo_unreserve(robj); if (ret) goto fail;
@@ -181,7 +191,6 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc, radeon_crtc->cursor_height = height;
radeon_lock_cursor(crtc, true); - /* XXX only 27 bit offset for legacy cursor */ radeon_set_cursor(crtc, obj, gpu_addr); radeon_show_cursor(crtc); radeon_lock_cursor(crtc, false); diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index d45df17..388be34 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -224,7 +224,8 @@ void radeon_bo_unref(struct radeon_bo **bo) *bo = NULL; }
-int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) +int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, + u64 *gpu_addr) { int r, i;
@@ -232,6 +233,7 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) bo->pin_count++; if (gpu_addr) *gpu_addr = radeon_bo_gpu_offset(bo); + WARN_ON_ONCE(max_offset != 0); return 0; } radeon_ttm_placement_from_domain(bo, domain); @@ -239,6 +241,15 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) /* force to pin into visible video ram */ bo->placement.lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT; } + if (max_offset) { + u64 lpfn = max_offset >> PAGE_SHIFT; + + if (bo->placement.lpfn) { + if (lpfn < bo->placement.lpfn) + bo->placement.lpfn = lpfn; + } else + bo->placement.lpfn = lpfn; + } for (i = 0; i < bo->placement.num_placement; i++) bo->placements[i] |= TTM_PL_FLAG_NO_EVICT; r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false, false); @@ -252,6 +263,11 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) return r; }
+int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) +{ + return radeon_bo_pin_restricted(bo, domain, 0, gpu_addr); +} + int radeon_bo_unpin(struct radeon_bo *bo) { int r, i; diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h index cde4303..f9104be 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -118,6 +118,8 @@ extern int radeon_bo_kmap(struct radeon_bo *bo, void **ptr); extern void radeon_bo_kunmap(struct radeon_bo *bo); extern void radeon_bo_unref(struct radeon_bo **bo); extern int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr); +extern int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, + u64 max_offset, u64 *gpu_addr); extern int radeon_bo_unpin(struct radeon_bo *bo); extern int radeon_bo_evict_vram(struct radeon_device *rdev); extern void radeon_bo_force_delete(struct radeon_device *rdev);
From: Michel Dänzer michel.daenzer@amd.com
Only radeon_gem_object_unpin was used anymore, in only one place.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com --- drivers/gpu/drm/radeon/radeon.h | 3 --- drivers/gpu/drm/radeon/radeon_cursor.c | 7 ++++++- drivers/gpu/drm/radeon/radeon_gem.c | 26 -------------------------- 3 files changed, 6 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index ec995ca..1414900 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -375,9 +375,6 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size, int alignment, int initial_domain, bool discardable, bool kernel, struct drm_gem_object **obj); -int radeon_gem_object_pin(struct drm_gem_object *obj, uint32_t pin_domain, - uint64_t *gpu_addr); -void radeon_gem_object_unpin(struct drm_gem_object *obj);
int radeon_mode_dumb_create(struct drm_file *file_priv, struct drm_device *dev, diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c index 986d608..42acc64 100644 --- a/drivers/gpu/drm/radeon/radeon_cursor.c +++ b/drivers/gpu/drm/radeon/radeon_cursor.c @@ -197,7 +197,12 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc,
unpin: if (radeon_crtc->cursor_bo) { - radeon_gem_object_unpin(radeon_crtc->cursor_bo); + robj = gem_to_radeon_bo(radeon_crtc->cursor_bo); + ret = radeon_bo_reserve(robj, false); + if (likely(ret == 0)) { + radeon_bo_unpin(robj); + radeon_bo_unreserve(robj); + } drm_gem_object_unreference_unlocked(radeon_crtc->cursor_bo); }
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index 003eeec..090da31 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -75,32 +75,6 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size, return 0; }
-int radeon_gem_object_pin(struct drm_gem_object *obj, uint32_t pin_domain, - uint64_t *gpu_addr) -{ - struct radeon_bo *robj = gem_to_radeon_bo(obj); - int r; - - r = radeon_bo_reserve(robj, false); - if (unlikely(r != 0)) - return r; - r = radeon_bo_pin(robj, pin_domain, gpu_addr); - radeon_bo_unreserve(robj); - return r; -} - -void radeon_gem_object_unpin(struct drm_gem_object *obj) -{ - struct radeon_bo *robj = gem_to_radeon_bo(obj); - int r; - - r = radeon_bo_reserve(robj, false); - if (likely(r == 0)) { - radeon_bo_unpin(robj); - radeon_bo_unreserve(robj); - } -} - int radeon_gem_set_domain(struct drm_gem_object *gobj, uint32_t rdomain, uint32_t wdomain) {
2012/3/14 Michel Dänzer michel@daenzer.net:
From: Michel Dänzer michel.daenzer@amd.com
Only radeon_gem_object_unpin was used anymore, in only one place.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
Reviewed-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/radeon/radeon.h | 3 --- drivers/gpu/drm/radeon/radeon_cursor.c | 7 ++++++- drivers/gpu/drm/radeon/radeon_gem.c | 26 -------------------------- 3 files changed, 6 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index ec995ca..1414900 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -375,9 +375,6 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size, int alignment, int initial_domain, bool discardable, bool kernel, struct drm_gem_object **obj); -int radeon_gem_object_pin(struct drm_gem_object *obj, uint32_t pin_domain,
- uint64_t *gpu_addr);
-void radeon_gem_object_unpin(struct drm_gem_object *obj);
int radeon_mode_dumb_create(struct drm_file *file_priv, struct drm_device *dev, diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c index 986d608..42acc64 100644 --- a/drivers/gpu/drm/radeon/radeon_cursor.c +++ b/drivers/gpu/drm/radeon/radeon_cursor.c @@ -197,7 +197,12 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc,
unpin: if (radeon_crtc->cursor_bo) {
- radeon_gem_object_unpin(radeon_crtc->cursor_bo);
- robj = gem_to_radeon_bo(radeon_crtc->cursor_bo);
- ret = radeon_bo_reserve(robj, false);
- if (likely(ret == 0)) {
- radeon_bo_unpin(robj);
- radeon_bo_unreserve(robj);
- }
drm_gem_object_unreference_unlocked(radeon_crtc->cursor_bo); }
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c index 003eeec..090da31 100644 --- a/drivers/gpu/drm/radeon/radeon_gem.c +++ b/drivers/gpu/drm/radeon/radeon_gem.c @@ -75,32 +75,6 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size, return 0; }
-int radeon_gem_object_pin(struct drm_gem_object *obj, uint32_t pin_domain,
- uint64_t *gpu_addr)
-{
- struct radeon_bo *robj = gem_to_radeon_bo(obj);
- int r;
- r = radeon_bo_reserve(robj, false);
- if (unlikely(r != 0))
- return r;
- r = radeon_bo_pin(robj, pin_domain, gpu_addr);
- radeon_bo_unreserve(robj);
- return r;
-}
-void radeon_gem_object_unpin(struct drm_gem_object *obj) -{
- struct radeon_bo *robj = gem_to_radeon_bo(obj);
- int r;
- r = radeon_bo_reserve(robj, false);
- if (likely(r == 0)) {
- radeon_bo_unpin(robj);
- radeon_bo_unreserve(robj);
- }
-}
int radeon_gem_set_domain(struct drm_gem_object *gobj, uint32_t rdomain, uint32_t wdomain) { -- 1.7.9.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
2012/3/14 Michel Dänzer michel@daenzer.net:
From: Michel Dänzer michel.daenzer@amd.com
The hardware only takes 27 bits for the offset, so larger offsets are truncated, and the hardware cursor shows random bits other than the intended ones.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46796
Cc: stable@vger.kernel.org Signed-off-by: Michel Dänzer michel.daenzer@amd.com
Reviewed-by: Alex Deucher alexander.deucher@amd.com
FWIW, I think we may need a similar fix for the crtc base addresses. They are also limited to 27 bit offsets from the mc address specified in DISPLAY[2]_BASE_ADDR.
drivers/gpu/drm/radeon/radeon_cursor.c | 13 +++++++++++-- drivers/gpu/drm/radeon/radeon_object.c | 18 +++++++++++++++++- drivers/gpu/drm/radeon/radeon_object.h | 2 ++ 3 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c index fde25c0..986d608 100644 --- a/drivers/gpu/drm/radeon/radeon_cursor.c +++ b/drivers/gpu/drm/radeon/radeon_cursor.c @@ -151,7 +151,9 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc, uint32_t height) { struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
- struct radeon_device *rdev = crtc->dev->dev_private;
struct drm_gem_object *obj;
- struct radeon_bo *robj;
uint64_t gpu_addr; int ret;
@@ -173,7 +175,15 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc, return -ENOENT; }
- ret = radeon_gem_object_pin(obj, RADEON_GEM_DOMAIN_VRAM, &gpu_addr);
- robj = gem_to_radeon_bo(obj);
- ret = radeon_bo_reserve(robj, false);
- if (unlikely(ret != 0))
- goto fail;
- /* Only 27 bit offset for legacy cursor */
- ret = radeon_bo_pin_restricted(robj, RADEON_GEM_DOMAIN_VRAM,
- ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27,
- &gpu_addr);
- radeon_bo_unreserve(robj);
if (ret) goto fail;
@@ -181,7 +191,6 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc, radeon_crtc->cursor_height = height;
radeon_lock_cursor(crtc, true);
- /* XXX only 27 bit offset for legacy cursor */
radeon_set_cursor(crtc, obj, gpu_addr); radeon_show_cursor(crtc); radeon_lock_cursor(crtc, false); diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index d45df17..388be34 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -224,7 +224,8 @@ void radeon_bo_unref(struct radeon_bo **bo) *bo = NULL; }
-int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) +int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset,
- u64 *gpu_addr)
{ int r, i;
@@ -232,6 +233,7 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) bo->pin_count++; if (gpu_addr) *gpu_addr = radeon_bo_gpu_offset(bo);
- WARN_ON_ONCE(max_offset != 0);
return 0; } radeon_ttm_placement_from_domain(bo, domain); @@ -239,6 +241,15 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) /* force to pin into visible video ram */ bo->placement.lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT; }
- if (max_offset) {
- u64 lpfn = max_offset >> PAGE_SHIFT;
- if (bo->placement.lpfn) {
- if (lpfn < bo->placement.lpfn)
- bo->placement.lpfn = lpfn;
- } else
- bo->placement.lpfn = lpfn;
- }
for (i = 0; i < bo->placement.num_placement; i++) bo->placements[i] |= TTM_PL_FLAG_NO_EVICT; r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false, false); @@ -252,6 +263,11 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) return r; }
+int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) +{
- return radeon_bo_pin_restricted(bo, domain, 0, gpu_addr);
+}
int radeon_bo_unpin(struct radeon_bo *bo) { int r, i; diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h index cde4303..f9104be 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -118,6 +118,8 @@ extern int radeon_bo_kmap(struct radeon_bo *bo, void **ptr); extern void radeon_bo_kunmap(struct radeon_bo *bo); extern void radeon_bo_unref(struct radeon_bo **bo); extern int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr); +extern int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain,
- u64 max_offset, u64 *gpu_addr);
extern int radeon_bo_unpin(struct radeon_bo *bo); extern int radeon_bo_evict_vram(struct radeon_device *rdev); extern void radeon_bo_force_delete(struct radeon_device *rdev); -- 1.7.9.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mit, 2012-03-14 at 11:08 -0400, Alex Deucher wrote:
2012/3/14 Michel Dänzer michel@daenzer.net:
From: Michel Dänzer michel.daenzer@amd.com
The hardware only takes 27 bits for the offset, so larger offsets are truncated, and the hardware cursor shows random bits other than the intended ones.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46796
Cc: stable@vger.kernel.org Signed-off-by: Michel Dänzer michel.daenzer@amd.com
Reviewed-by: Alex Deucher alexander.deucher@amd.com
Thanks. In the meantime, it occurred to me there might be a problem when there's less than 128M of VRAM in the first place (so bo->placement.lpfn ends up larger than rdev->mman.bdev.man[TTM_PL_VRAM].size). To be safe, I'll send a v2 patch preventing that.
FWIW, I think we may need a similar fix for the crtc base addresses. They are also limited to 27 bit offsets from the mc address specified in DISPLAY[2]_BASE_ADDR.
Hmm, good point.
From: Michel Dänzer michel.daenzer@amd.com
The hardware only takes 27 bits for the offset, so larger offsets are truncated, and the hardware cursor shows random bits other than the intended ones.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46796
Cc: stable@vger.kernel.org Signed-off-by: Michel Dänzer michel.daenzer@amd.com ---
v2: Make sure bo->placement.lpfn isn't larger than the memory size, even if radeon_bo_pin_restricted were to be called for the GTT domain.
drivers/gpu/drm/radeon/radeon_cursor.c | 13 +++++++++++-- drivers/gpu/drm/radeon/radeon_object.c | 18 +++++++++++++++++- drivers/gpu/drm/radeon/radeon_object.h | 2 ++ 3 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c index fde25c0..986d608 100644 --- a/drivers/gpu/drm/radeon/radeon_cursor.c +++ b/drivers/gpu/drm/radeon/radeon_cursor.c @@ -151,7 +151,9 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc, uint32_t height) { struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc); + struct radeon_device *rdev = crtc->dev->dev_private; struct drm_gem_object *obj; + struct radeon_bo *robj; uint64_t gpu_addr; int ret;
@@ -173,7 +175,15 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc, return -ENOENT; }
- ret = radeon_gem_object_pin(obj, RADEON_GEM_DOMAIN_VRAM, &gpu_addr); + robj = gem_to_radeon_bo(obj); + ret = radeon_bo_reserve(robj, false); + if (unlikely(ret != 0)) + goto fail; + /* Only 27 bit offset for legacy cursor */ + ret = radeon_bo_pin_restricted(robj, RADEON_GEM_DOMAIN_VRAM, + ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27, + &gpu_addr); + radeon_bo_unreserve(robj); if (ret) goto fail;
@@ -181,7 +191,6 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc, radeon_crtc->cursor_height = height;
radeon_lock_cursor(crtc, true); - /* XXX only 27 bit offset for legacy cursor */ radeon_set_cursor(crtc, obj, gpu_addr); radeon_show_cursor(crtc); radeon_lock_cursor(crtc, false); diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index d45df17..fd2dc0e 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -224,7 +224,8 @@ void radeon_bo_unref(struct radeon_bo **bo) *bo = NULL; }
-int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) +int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset, + u64 *gpu_addr) { int r, i;
@@ -232,6 +233,7 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) bo->pin_count++; if (gpu_addr) *gpu_addr = radeon_bo_gpu_offset(bo); + WARN_ON_ONCE(max_offset != 0); return 0; } radeon_ttm_placement_from_domain(bo, domain); @@ -239,6 +241,15 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) /* force to pin into visible video ram */ bo->placement.lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT; } + if (max_offset) { + u64 lpfn = max_offset >> PAGE_SHIFT; + + if (!bo->placement.lpfn) + bo->placement.lpfn = bo->rdev->mc.gtt_size >> PAGE_SHIFT; + + if (lpfn < bo->placement.lpfn) + bo->placement.lpfn = lpfn; + } for (i = 0; i < bo->placement.num_placement; i++) bo->placements[i] |= TTM_PL_FLAG_NO_EVICT; r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false, false); @@ -252,6 +263,11 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) return r; }
+int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) +{ + return radeon_bo_pin_restricted(bo, domain, 0, gpu_addr); +} + int radeon_bo_unpin(struct radeon_bo *bo) { int r, i; diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h index cde4303..f9104be 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -118,6 +118,8 @@ extern int radeon_bo_kmap(struct radeon_bo *bo, void **ptr); extern void radeon_bo_kunmap(struct radeon_bo *bo); extern void radeon_bo_unref(struct radeon_bo **bo); extern int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr); +extern int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, + u64 max_offset, u64 *gpu_addr); extern int radeon_bo_unpin(struct radeon_bo *bo); extern int radeon_bo_evict_vram(struct radeon_device *rdev); extern void radeon_bo_force_delete(struct radeon_device *rdev);
From: Michel Dänzer michel.daenzer@amd.com
The hardware only takes 27 bits for the offset, so larger offsets are truncated, and the display shows random bits other than the intended ones.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com --- drivers/gpu/drm/radeon/radeon_display.c | 4 +++- drivers/gpu/drm/radeon/radeon_fb.c | 5 ++++- drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 4 +++- 3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index d3ffc18..63386a4 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -393,7 +393,9 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc, DRM_ERROR("failed to reserve new rbo buffer before flip\n"); goto pflip_cleanup; } - r = radeon_bo_pin(rbo, RADEON_GEM_DOMAIN_VRAM, &base); + /* Only 27 bit offset for legacy CRTC */ + r = radeon_bo_pin_restricted(rbo, RADEON_GEM_DOMAIN_VRAM, + ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27, &base); if (unlikely(r != 0)) { radeon_bo_unreserve(rbo); r = -EINVAL; diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index cf2bf35..93bf920 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -164,7 +164,10 @@ static int radeonfb_create_pinned_object(struct radeon_fbdev *rfbdev, ret = radeon_bo_reserve(rbo, false); if (unlikely(ret != 0)) goto out_unref; - ret = radeon_bo_pin(rbo, RADEON_GEM_DOMAIN_VRAM, NULL); + /* Only 27 bit offset for legacy CRTC */ + ret = radeon_bo_pin_restricted(rbo, RADEON_GEM_DOMAIN_VRAM, + ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27, + NULL); if (ret) { radeon_bo_unreserve(rbo); goto out_unref; diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c index 25a19c4..210317c 100644 --- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c +++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c @@ -419,7 +419,9 @@ int radeon_crtc_do_set_base(struct drm_crtc *crtc, r = radeon_bo_reserve(rbo, false); if (unlikely(r != 0)) return r; - r = radeon_bo_pin(rbo, RADEON_GEM_DOMAIN_VRAM, &base); + /* Only 27 bit offset for legacy CRTC */ + r = radeon_bo_pin_restricted(rbo, RADEON_GEM_DOMAIN_VRAM, 1 << 27, + &base); if (unlikely(r != 0)) { radeon_bo_unreserve(rbo); return -EINVAL;
2012/3/14 Michel Dänzer michel@daenzer.net:
From: Michel Dänzer michel.daenzer@amd.com
The hardware only takes 27 bits for the offset, so larger offsets are truncated, and the display shows random bits other than the intended ones.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
Reviewed-by: Alex Deucher alexander.deucher@amd.com
This should probably go to stable as well. I think it may also fix this bug: https://bugzilla.kernel.org/show_bug.cgi?id=16515
Alex
drivers/gpu/drm/radeon/radeon_display.c | 4 +++- drivers/gpu/drm/radeon/radeon_fb.c | 5 ++++- drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 4 +++- 3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index d3ffc18..63386a4 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -393,7 +393,9 @@ static int radeon_crtc_page_flip(struct drm_crtc *crtc, DRM_ERROR("failed to reserve new rbo buffer before flip\n"); goto pflip_cleanup; }
- r = radeon_bo_pin(rbo, RADEON_GEM_DOMAIN_VRAM, &base);
- /* Only 27 bit offset for legacy CRTC */
- r = radeon_bo_pin_restricted(rbo, RADEON_GEM_DOMAIN_VRAM,
- ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27, &base);
if (unlikely(r != 0)) { radeon_bo_unreserve(rbo); r = -EINVAL; diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index cf2bf35..93bf920 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -164,7 +164,10 @@ static int radeonfb_create_pinned_object(struct radeon_fbdev *rfbdev, ret = radeon_bo_reserve(rbo, false); if (unlikely(ret != 0)) goto out_unref;
- ret = radeon_bo_pin(rbo, RADEON_GEM_DOMAIN_VRAM, NULL);
- /* Only 27 bit offset for legacy CRTC */
- ret = radeon_bo_pin_restricted(rbo, RADEON_GEM_DOMAIN_VRAM,
- ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27,
- NULL);
if (ret) { radeon_bo_unreserve(rbo); goto out_unref; diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c index 25a19c4..210317c 100644 --- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c +++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c @@ -419,7 +419,9 @@ int radeon_crtc_do_set_base(struct drm_crtc *crtc, r = radeon_bo_reserve(rbo, false); if (unlikely(r != 0)) return r;
- r = radeon_bo_pin(rbo, RADEON_GEM_DOMAIN_VRAM, &base);
- /* Only 27 bit offset for legacy CRTC */
- r = radeon_bo_pin_restricted(rbo, RADEON_GEM_DOMAIN_VRAM, 1 << 27,
- &base);
if (unlikely(r != 0)) { radeon_bo_unreserve(rbo); return -EINVAL; -- 1.7.9.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mit, 2012-03-14 at 15:13 -0400, Alex Deucher wrote:
2012/3/14 Michel Dänzer michel@daenzer.net:
From: Michel Dänzer michel.daenzer@amd.com
The hardware only takes 27 bits for the offset, so larger offsets are truncated, and the display shows random bits other than the intended ones.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com
Reviewed-by: Alex Deucher alexander.deucher@amd.com
This should probably go to stable as well.
Probably, but I'm hesitating to tag it for that while it's only compile-tested. :)
I think it may also fix this bug: https://bugzilla.kernel.org/show_bug.cgi?id=16515
Hmm, I'm not very optimistic for that, but it's certainly worth a shot. Thanks for following up on it. I suspect this might be the root cause of other bug reports as well.
2012/3/14 Michel Dänzer michel@daenzer.net:
From: Michel Dänzer michel.daenzer@amd.com
The hardware only takes 27 bits for the offset, so larger offsets are truncated, and the hardware cursor shows random bits other than the intended ones.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46796
Cc: stable@vger.kernel.org Signed-off-by: Michel Dänzer michel.daenzer@amd.com
Reviewed-by: Alex Deucher alexander.deucher@amd.com
v2: Make sure bo->placement.lpfn isn't larger than the memory size, even if radeon_bo_pin_restricted were to be called for the GTT domain.
drivers/gpu/drm/radeon/radeon_cursor.c | 13 +++++++++++-- drivers/gpu/drm/radeon/radeon_object.c | 18 +++++++++++++++++- drivers/gpu/drm/radeon/radeon_object.h | 2 ++ 3 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c index fde25c0..986d608 100644 --- a/drivers/gpu/drm/radeon/radeon_cursor.c +++ b/drivers/gpu/drm/radeon/radeon_cursor.c @@ -151,7 +151,9 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc, uint32_t height) { struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
- struct radeon_device *rdev = crtc->dev->dev_private;
struct drm_gem_object *obj;
- struct radeon_bo *robj;
uint64_t gpu_addr; int ret;
@@ -173,7 +175,15 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc, return -ENOENT; }
- ret = radeon_gem_object_pin(obj, RADEON_GEM_DOMAIN_VRAM, &gpu_addr);
- robj = gem_to_radeon_bo(obj);
- ret = radeon_bo_reserve(robj, false);
- if (unlikely(ret != 0))
- goto fail;
- /* Only 27 bit offset for legacy cursor */
- ret = radeon_bo_pin_restricted(robj, RADEON_GEM_DOMAIN_VRAM,
- ASIC_IS_AVIVO(rdev) ? 0 : 1 << 27,
- &gpu_addr);
- radeon_bo_unreserve(robj);
if (ret) goto fail;
@@ -181,7 +191,6 @@ int radeon_crtc_cursor_set(struct drm_crtc *crtc, radeon_crtc->cursor_height = height;
radeon_lock_cursor(crtc, true);
- /* XXX only 27 bit offset for legacy cursor */
radeon_set_cursor(crtc, obj, gpu_addr); radeon_show_cursor(crtc); radeon_lock_cursor(crtc, false); diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index d45df17..fd2dc0e 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -224,7 +224,8 @@ void radeon_bo_unref(struct radeon_bo **bo) *bo = NULL; }
-int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) +int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain, u64 max_offset,
- u64 *gpu_addr)
{ int r, i;
@@ -232,6 +233,7 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) bo->pin_count++; if (gpu_addr) *gpu_addr = radeon_bo_gpu_offset(bo);
- WARN_ON_ONCE(max_offset != 0);
return 0; } radeon_ttm_placement_from_domain(bo, domain); @@ -239,6 +241,15 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) /* force to pin into visible video ram */ bo->placement.lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT; }
- if (max_offset) {
- u64 lpfn = max_offset >> PAGE_SHIFT;
- if (!bo->placement.lpfn)
- bo->placement.lpfn = bo->rdev->mc.gtt_size >> PAGE_SHIFT;
- if (lpfn < bo->placement.lpfn)
- bo->placement.lpfn = lpfn;
- }
for (i = 0; i < bo->placement.num_placement; i++) bo->placements[i] |= TTM_PL_FLAG_NO_EVICT; r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false, false); @@ -252,6 +263,11 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) return r; }
+int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr) +{
- return radeon_bo_pin_restricted(bo, domain, 0, gpu_addr);
+}
int radeon_bo_unpin(struct radeon_bo *bo) { int r, i; diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h index cde4303..f9104be 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -118,6 +118,8 @@ extern int radeon_bo_kmap(struct radeon_bo *bo, void **ptr); extern void radeon_bo_kunmap(struct radeon_bo *bo); extern void radeon_bo_unref(struct radeon_bo **bo); extern int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 *gpu_addr); +extern int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 domain,
- u64 max_offset, u64 *gpu_addr);
extern int radeon_bo_unpin(struct radeon_bo *bo); extern int radeon_bo_evict_vram(struct radeon_device *rdev); extern void radeon_bo_force_delete(struct radeon_device *rdev); -- 1.7.9.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Michel Dänzer