While reviewing Nicholas Miell's patch 'drm/radeon/kms: fix cursor image off-by-one error', I noticed at least one other bug (fixed by patch 2, and one potential bug fixed by patch 3) and opportunities for cleanup.
Patch 1 is based on Nicholas' patch and can be dropped if he amends his patch along the same lines.
[PATCH 1/3] drm/radeon: Simplify cursor x/yorigin calculation. [PATCH 2/3] drm/radeon: Update AVIVO cursor coordinate origin before [PATCH 3/3] drm/radeon: Set cursor x/y to 0 when x/yorigin > 0.
From: Michel Dänzer michel.daenzer@amd.com
Signed-off-by: Michel Dänzer michel.daenzer@amd.com --- drivers/gpu/drm/radeon/radeon_cursor.c | 8 ++------ 1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c index c495575..bac8ee7 100644 --- a/drivers/gpu/drm/radeon/radeon_cursor.c +++ b/drivers/gpu/drm/radeon/radeon_cursor.c @@ -209,13 +209,9 @@ int radeon_crtc_cursor_move(struct drm_crtc *crtc, int w = radeon_crtc->cursor_width;
if (x < 0) - xorigin = -x; + xorigin = min(-x, CURSOR_WIDTH - 1); if (y < 0) - yorigin = -y; - if (xorigin >= CURSOR_WIDTH) - xorigin = CURSOR_WIDTH - 1; - if (yorigin >= CURSOR_HEIGHT) - yorigin = CURSOR_HEIGHT - 1; + yorigin = min(-y, CURSOR_HEIGHT - 1);
if (ASIC_IS_AVIVO(rdev)) { int i = 0;
From: Michel Dänzer michel.daenzer@amd.com
Fixes cursor disappearing prematurely when moving off a top/left edge which is not located at the desktop top/left edge.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com Cc: stable@kernel.org --- drivers/gpu/drm/radeon/radeon_cursor.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c index bac8ee7..f1d871d 100644 --- a/drivers/gpu/drm/radeon/radeon_cursor.c +++ b/drivers/gpu/drm/radeon/radeon_cursor.c @@ -208,6 +208,13 @@ int radeon_crtc_cursor_move(struct drm_crtc *crtc, int xorigin = 0, yorigin = 0; int w = radeon_crtc->cursor_width;
+ if (ASIC_IS_AVIVO(rdev)) { + /* avivo cursor are offset into the total surface */ + x += crtc->x; + y += crtc->y; + } + DRM_DEBUG("x %d y %d c->x %d c->y %d\n", x, y, crtc->x, crtc->y); + if (x < 0) xorigin = min(-x, CURSOR_WIDTH - 1); if (y < 0) @@ -217,11 +224,6 @@ int radeon_crtc_cursor_move(struct drm_crtc *crtc, int i = 0; struct drm_crtc *crtc_p;
- /* avivo cursor are offset into the total surface */ - x += crtc->x; - y += crtc->y; - DRM_DEBUG("x %d y %d c->x %d c->y %d\n", x, y, crtc->x, crtc->y); - /* avivo cursor image can't end on 128 pixel boundary or * go past the end of the frame if both crtcs are enabled */
From: Michel Dänzer michel.daenzer@amd.com
Apart from the obvious cleanup, this should make the line
cursor_end = x - xorigin + w;
correct now.
Signed-off-by: Michel Dänzer michel.daenzer@amd.com --- drivers/gpu/drm/radeon/radeon_cursor.c | 20 ++++++++++---------- 1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c index f1d871d..fde25c0 100644 --- a/drivers/gpu/drm/radeon/radeon_cursor.c +++ b/drivers/gpu/drm/radeon/radeon_cursor.c @@ -215,10 +215,14 @@ int radeon_crtc_cursor_move(struct drm_crtc *crtc, } DRM_DEBUG("x %d y %d c->x %d c->y %d\n", x, y, crtc->x, crtc->y);
- if (x < 0) + if (x < 0) { xorigin = min(-x, CURSOR_WIDTH - 1); - if (y < 0) + x = 0; + } + if (y < 0) { yorigin = min(-y, CURSOR_HEIGHT - 1); + y = 0; + }
if (ASIC_IS_AVIVO(rdev)) { int i = 0; @@ -251,16 +255,12 @@ int radeon_crtc_cursor_move(struct drm_crtc *crtc,
radeon_lock_cursor(crtc, true); if (ASIC_IS_DCE4(rdev)) { - WREG32(EVERGREEN_CUR_POSITION + radeon_crtc->crtc_offset, - ((xorigin ? 0 : x) << 16) | - (yorigin ? 0 : y)); + WREG32(EVERGREEN_CUR_POSITION + radeon_crtc->crtc_offset, (x << 16) | y); WREG32(EVERGREEN_CUR_HOT_SPOT + radeon_crtc->crtc_offset, (xorigin << 16) | yorigin); WREG32(EVERGREEN_CUR_SIZE + radeon_crtc->crtc_offset, ((w - 1) << 16) | (radeon_crtc->cursor_height - 1)); } else if (ASIC_IS_AVIVO(rdev)) { - WREG32(AVIVO_D1CUR_POSITION + radeon_crtc->crtc_offset, - ((xorigin ? 0 : x) << 16) | - (yorigin ? 0 : y)); + WREG32(AVIVO_D1CUR_POSITION + radeon_crtc->crtc_offset, (x << 16) | y); WREG32(AVIVO_D1CUR_HOT_SPOT + radeon_crtc->crtc_offset, (xorigin << 16) | yorigin); WREG32(AVIVO_D1CUR_SIZE + radeon_crtc->crtc_offset, ((w - 1) << 16) | (radeon_crtc->cursor_height - 1)); @@ -274,8 +274,8 @@ int radeon_crtc_cursor_move(struct drm_crtc *crtc, | yorigin)); WREG32(RADEON_CUR_HORZ_VERT_POSN + radeon_crtc->crtc_offset, (RADEON_CUR_LOCK - | ((xorigin ? 0 : x) << 16) - | (yorigin ? 0 : y))); + | (x << 16) + | y)); /* offset is from DISP(2)_BASE_ADDRESS */ WREG32(RADEON_CUR_OFFSET + radeon_crtc->crtc_offset, (radeon_crtc->legacy_cursor_offset + (yorigin * 256)));
On 09/30/2011 08:16 AM, Michel Dänzer wrote:
While reviewing Nicholas Miell's patch 'drm/radeon/kms: fix cursor image off-by-one error', I noticed at least one other bug (fixed by patch 2, and one potential bug fixed by patch 3) and opportunities for cleanup.
Patch 1 is based on Nicholas' patch and can be dropped if he amends his patch along the same lines.
[PATCH 1/3] drm/radeon: Simplify cursor x/yorigin calculation. [PATCH 2/3] drm/radeon: Update AVIVO cursor coordinate origin before [PATCH 3/3] drm/radeon: Set cursor x/y to 0 when x/yorigin > 0.
Feel free to squash my patch into yours.
2011/9/30 Michel Dänzer michel@daenzer.net:
While reviewing Nicholas Miell's patch 'drm/radeon/kms: fix cursor image off-by-one error', I noticed at least one other bug (fixed by patch 2, and one potential bug fixed by patch 3) and opportunities for cleanup.
Patch 1 is based on Nicholas' patch and can be dropped if he amends his patch along the same lines.
[PATCH 1/3] drm/radeon: Simplify cursor x/yorigin calculation. [PATCH 2/3] drm/radeon: Update AVIVO cursor coordinate origin before [PATCH 3/3] drm/radeon: Set cursor x/y to 0 when x/yorigin > 0.
Looks good to me, both your patches and Nicholas' patch are: Reviewed-by: Alex Deucher alexander.deucher@amd.com
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 -
Nicholas Miell