Re: [PATCH] drm/radeon: Fix negative cursor position

[Takashi Iwai <tiwai@suse.de>]

On Thu, 27 Oct 2016 11:02:30 +0200,
Michel D4nzer wrote:
> 
> On 26/09/16 09:09 PM, Takashi Iwai wrote:
> > On Mon, 26 Sep 2016 11:42:16 +0200, Takashi Iwai wrote:
> >> On Mon, 26 Sep 2016 10:57:50 +0200, Michel D4nzer wrote:
> >>> 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@vger.kernel.org>
> >>>> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> >>>> ---
> >>>>  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.
> >>
> >> Sure, I'll ask the reporter to test with your tentative fix.
> > 
> > Through a quick test by Tomas (added to Cc), the new patch seems
> > working.
> 
> Thanks for testing, Tomáš! I sent an updated patch series to the amd-gfx
> mailing list and put you guys in Cc.

Thanks!

I updated the test package with your latest patchset for openSUSE
Tumbleweed:
  https://bugzilla.suse.com/show_bug.cgi?id=1000433

Feel free to join to the bugzilla.


Takashi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel