On 23/09/16 10:06 PM, Takashi Iwai wrote:
> radeon_cursor_move_unlock() contains a workaround for AVIVO chips that
> are older than DCE6 when the cursor ends on 128 pixel boundary. It
> decreases the position when the calculated end position is on 128
> pixel boundary. However, it hits also the condition where x=-1 and
> width=1 are passed, since cursor_end is 0 (which is aligned with 128,
> too). Then the value gets decreased and it hits WARN_ON_ONCE()
> below, eventually screws up the GPU.
>
> The fix is simply to skip the workaround when x is already zero.
> Also, remove the superfluous WARN_ON_ON() for the negative value check
> that wouldn't happen any longer after this change.
>
> Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1000433
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> ---
> drivers/gpu/drm/radeon/radeon_cursor.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c
> index 2a10e24b34b1..4e436eb49a56 100644
> --- a/drivers/gpu/drm/radeon/radeon_cursor.c
> +++ b/drivers/gpu/drm/radeon/radeon_cursor.c
> @@ -193,10 +193,8 @@ static int radeon_cursor_move_locked(struct drm_crtc *crtc, int x, int y)
> if (w <= 0) {
> w = 1;
> cursor_end = x - xorigin + w;
> - if (!(cursor_end & 0x7f)) {
> + if (x > 0 && !(cursor_end & 0x7f))
> x--;
> - WARN_ON_ONCE(x < 0);
> - }
> }
The problem with this change is that the horizontal cursor end position
can again and up as a multiple of 128, which this code is trying to
avoid, because it can break the cursor with some versions of AVIVO / DCE
display engines.
Attached is a proof-of-concept (only compile tested) alternative fix
which hides the cursor while it's out of bounds. I'll test it, clean it
up and split it up into multiple patches when I get a chance, but maybe
you or the reporter of the referenced bug can test it in the meantime.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
diff --git a/drivers/gpu/drm/radeon/radeon_cursor.c b/drivers/gpu/drm/radeon/radeon_cursor.c
index 2a10e24..a656410 100644
--- a/drivers/gpu/drm/radeon/radeon_cursor.c
+++ b/drivers/gpu/drm/radeon/radeon_cursor.c
@@ -90,6 +90,9 @@ static void radeon_show_cursor(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct radeon_device *rdev = crtc->dev->dev_private;
+ if (radeon_crtc->cursor_out_of_bounds)
+ return;
+
if (ASIC_IS_DCE4(rdev)) {
WREG32(EVERGREEN_CUR_SURFACE_ADDRESS_HIGH + radeon_crtc->crtc_offset,
upper_32_bits(radeon_crtc->cursor_addr));
@@ -150,15 +153,6 @@ static int radeon_cursor_move_locked(struct drm_crtc *crtc, int x, int 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, radeon_crtc->max_cursor_width - 1);
- x = 0;
- }
- if (y < 0) {
- yorigin = min(-y, radeon_crtc->max_cursor_height - 1);
- y = 0;
- }
-
/* fixed on DCE6 and newer */
if (ASIC_IS_AVIVO(rdev) && !ASIC_IS_DCE6(rdev)) {
int i = 0;
@@ -180,27 +174,36 @@ static int radeon_cursor_move_locked(struct drm_crtc *crtc, int x, int y)
if (i > 1) {
int cursor_end, frame_end;
- cursor_end = x - xorigin + w;
+ cursor_end = x + w;
frame_end = crtc->x + crtc->mode.crtc_hdisplay;
if (cursor_end >= frame_end) {
w = w - (cursor_end - frame_end);
if (!(frame_end & 0x7f))
w--;
- } else {
- if (!(cursor_end & 0x7f))
- w--;
+ } else if (cursor_end <= 0) {
+ goto out_of_bounds;
+ } else if (!(cursor_end & 0x7f)) {
+ w--;
}
if (w <= 0) {
- w = 1;
- cursor_end = x - xorigin + w;
- if (!(cursor_end & 0x7f)) {
- x--;
- WARN_ON_ONCE(x < 0);
- }
+ goto out_of_bounds;
}
}
}
+ if (x <= -w || y <= -radeon_crtc->cursor_height ||
+ x >= crtc->mode.crtc_hdisplay || y >= crtc->mode.crtc_vdisplay)
+ goto out_of_bounds;
+
+ if (x < 0) {
+ xorigin = min(-x, radeon_crtc->max_cursor_width - 1);
+ x = 0;
+ }
+ if (y < 0) {
+ yorigin = min(-y, radeon_crtc->max_cursor_height - 1);
+ y = 0;
+ }
+
if (ASIC_IS_DCE4(rdev)) {
WREG32(EVERGREEN_CUR_POSITION + radeon_crtc->crtc_offset, (x << 16) | y);
WREG32(EVERGREEN_CUR_HOT_SPOT + radeon_crtc->crtc_offset, (xorigin << 16) | yorigin);
@@ -232,6 +235,19 @@ static int radeon_cursor_move_locked(struct drm_crtc *crtc, int x, int y)
radeon_crtc->cursor_x = x;
radeon_crtc->cursor_y = y;
+ if (radeon_crtc->cursor_out_of_bounds) {
+ radeon_crtc->cursor_out_of_bounds = false;
+ if (radeon_crtc->cursor_bo)
+ radeon_show_cursor(crtc);
+ }
+
+ return 0;
+
+ out_of_bounds:
+ if (!radeon_crtc->cursor_out_of_bounds) {
+ radeon_hide_cursor(crtc);
+ radeon_crtc->cursor_out_of_bounds = true;
+ }
return 0;
}
@@ -259,6 +275,7 @@ int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
struct radeon_device *rdev = crtc->dev->dev_private;
struct drm_gem_object *obj;
struct radeon_bo *robj;
+ int x, y;
int ret;
if (!handle) {
@@ -302,19 +319,18 @@ int radeon_crtc_cursor_set2(struct drm_crtc *crtc,
radeon_lock_cursor(crtc, true);
+ x = radeon_crtc->cursor_x;
+ y = radeon_crtc->cursor_y;
if (hot_x != radeon_crtc->cursor_hot_x ||
hot_y != radeon_crtc->cursor_hot_y) {
- int x, y;
-
- x = radeon_crtc->cursor_x + radeon_crtc->cursor_hot_x - hot_x;
- y = radeon_crtc->cursor_y + radeon_crtc->cursor_hot_y - hot_y;
-
- radeon_cursor_move_locked(crtc, x, y);
+ x += radeon_crtc->cursor_hot_x - hot_x;
+ y += radeon_crtc->cursor_hot_y - hot_y;
radeon_crtc->cursor_hot_x = hot_x;
radeon_crtc->cursor_hot_y = hot_y;
}
+ radeon_cursor_move_locked(crtc, x, y);
radeon_show_cursor(crtc);
radeon_lock_cursor(crtc, false);
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index bb75201a..f1da484 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -330,6 +330,7 @@ struct radeon_crtc {
u16 lut_r[256], lut_g[256], lut_b[256];
bool enabled;
bool can_tile;
+ bool cursor_out_of_bounds;
uint32_t crtc_offset;
struct drm_gem_object *cursor_bo;
uint64_t cursor_addr;
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
