Mailinglist Archive: radeonhd (427 mails)

[Yang Zhao <yang@xxxxxxxxxx>]

2009/5/5 Matthias Hopf <mhopf@xxxxxxx>:
> On May 04, 09 15:27:00 -0700, Yang Zhao wrote:
> > 0002 is the refactoring. It's almost trivial now that we don't need
> > the logic to keep the cursor enabled on both CRTCs.
>
> > diff --git a/src/rhd_cursor.c b/src/rhd_cursor.c
> > index 3a0c7a9..a2b52b1 100644
> > --- a/src/rhd_cursor.c
> > +++ b/src/rhd_cursor.c
> > @@ -625,10 +625,9 @@ void
> >  rhdCrtcShowCursor(struct rhdCrtc *Crtc)
> >  {
> >      struct rhdCursor *Cursor = Crtc->Cursor;
> > -
> > -    lockCursor   (Cursor, TRUE);
> > -    displayCursor(Crtc,   TRUE);
> > -    lockCursor   (Cursor, FALSE);
> > +    lockCursor(Cursor, TRUE);
> > +    enableCursor(Cursor, TRUE);
> > +    lockCursor(Cursor, FALSE);
>
> I don't like the whitespace changes - IMHO it's much harder to read.

Fixed.


> > @@ -651,11 +649,27 @@ void
> >  rhdCrtcSetCursorPosition(struct rhdCrtc *Crtc, int x, int y)
> >  {
> >      struct rhdCursor *Cursor = Crtc->Cursor;
> > +    int hotx, hoty;
>
> What I don't really like here is the duplication of hotspot calculation.
> That has been the reason behind displayCursor() in the first place.
> Not really sure what to do about that, it's probably best to first apply
> this and then refactor the logic.

displayCursor() currently contains pretty much everything: position
setting, size adjustment, and visibility toggle, along with logic to
determine if the cursor is within each CRTC's visible area.  For RandR
cursor, this duplicates a lot of the work the server is already doing.

Once the new RandR cursor code is in place, the idea is to refactor
the non-RandR cursor code such that we check which CRTC is affected,
then simply call the appropriate rhdCrtc*() function.


> Also please separate logic and register access, so setCursorPos() should
> be used instead of direct RHDRegWrite(). It's a small static function,
> so it will be inlined anyway.

Done. I seem to have forgotten setCursorPos() existed.


Revised 0002 and 0003 attached.
-- 
Yang Zhao
http://yangman.ca
From af29e97ac5c6d72f4cbdda99883844697837f7b2 Mon Sep 17 00:00:00 2001
From: Yang Zhao <yang@xxxxxxxxxx>
Date: Sun, 26 Apr 2009 15:31:29 -0700
Subject: [PATCH] Cursor: refactor RandR cursor code

Eliminate calls to displayCursor() and generally reduce the number of
register writes.

Signed-off-by: Yang Zhao <yang@xxxxxxxxxx>
---
 src/rhd_cursor.c |   27 +++++++++++++++++++++------
 1 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/rhd_cursor.c b/src/rhd_cursor.c
index 3a0c7a9..751c614 100644
--- a/src/rhd_cursor.c
+++ b/src/rhd_cursor.c
@@ -625,10 +625,9 @@ void
 rhdCrtcShowCursor(struct rhdCrtc *Crtc)
 {
     struct rhdCursor *Cursor = Crtc->Cursor;
-
-    lockCursor   (Cursor, TRUE);
-    displayCursor(Crtc,   TRUE);
-    lockCursor   (Cursor, FALSE);
+    lockCursor  (Cursor, TRUE);
+    enableCursor(Cursor, TRUE);
+    lockCursor  (Cursor, FALSE);
 }
 
 /*
@@ -651,11 +650,26 @@ void
 rhdCrtcSetCursorPosition(struct rhdCrtc *Crtc, int x, int y)
 {
     struct rhdCursor *Cursor = Crtc->Cursor;
+    int hotx, hoty;
+
     Cursor->X = x;
     Cursor->Y = y;
 
+    hotx = 0;
+    hoty = 0;
+
+    /* Hardware doesn't allow negative cursor pos; compensate using hotspot */
+    if (x < 0) {
+        hotx = -x;
+        x = 0;
+    }
+    if (y < 0) {
+        hoty = -y;
+        y = 0;
+    }
+
     lockCursor   (Cursor, TRUE);
-    displayCursor(Crtc,   TRUE);
+    setCursorPos (Cursor, x, y, hotx, hoty);
     lockCursor   (Cursor, FALSE);
 }
 
@@ -679,13 +693,14 @@ rhdCrtcLoadCursorARGB(struct rhdCrtc *Crtc, CARD32 *Image)
 {
     struct rhdCursor *Cursor = Crtc->Cursor;
 
+    /* X server always loads cursors that are the maximum allowed size */
     Cursor->Width = MAX_CURSOR_WIDTH;
     Cursor->Height = MAX_CURSOR_HEIGHT;
 
     lockCursor       (Cursor, TRUE);
     uploadCursorImage(Cursor, Image);
     setCursorImage   (Cursor);
-    displayCursor    (Crtc,   Crtc->Active);
+    setCursorSize    (Cursor, Cursor->Width, Cursor->Height);
     lockCursor       (Cursor, FALSE);
 }
 
-- 
1.6.0.6

From 773b0c27424e98bca23febcea7364b506282af6c Mon Sep 17 00:00:00 2001
From: Yang Zhao <yang@xxxxxxxxxx>
Date: Mon, 4 May 2009 13:38:27 -0700
Subject: [PATCH] Cursor: Fix remaning corruption cases

Port of f668cc06cd7f338888a7dce1507026af0e9e36ad to new code

Found by Alex:
Apparently the cursor image cannot end on a multiple of 128 pixels.
Also, for panning, the end of the cursor image cannot extend past
the end of the viewport.

Signed-off-by: Yang Zhao <yang@xxxxxxxxxx>
---
 src/rhd_cursor.c |   33 ++++++++++++++++++++++++++++++++-
 1 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/src/rhd_cursor.c b/src/rhd_cursor.c
index 751c614..91c320b 100644
--- a/src/rhd_cursor.c
+++ b/src/rhd_cursor.c
@@ -649,8 +649,9 @@ rhdCrtcHideCursor(struct rhdCrtc *Crtc)
 void
 rhdCrtcSetCursorPosition(struct rhdCrtc *Crtc, int x, int y)
 {
+    RHDPtr rhdPtr = RHDPTRI(Crtc);
     struct rhdCursor *Cursor = Crtc->Cursor;
-    int hotx, hoty;
+    int hotx, hoty, width, cursor_end, frame_end;
 
     Cursor->X = x;
     Cursor->Y = y;
@@ -669,6 +670,36 @@ rhdCrtcSetCursorPosition(struct rhdCrtc *Crtc, int x, int 
y)
     }
 
     lockCursor   (Cursor, TRUE);
+
+    /* Work around rare corruption cases by adjusting cursor size;
+     * related to bug #13405
+     * For dual-screen:
+     * - Cursor's right-edge must not end on multiples of 128px.
+     * - For panning, cursor image cannot horizontally extend past end of 
viewport.
+     */
+    if (rhdPtr->Crtc[0]->Active && rhdPtr->Crtc[1]->Active) {
+        width      = Cursor->Width;
+        cursor_end = x + width;
+        frame_end  = Crtc->X   + Crtc->Width;
+
+        if (cursor_end > frame_end) {
+            width     -= cursor_end - frame_end;
+            cursor_end = x + width;
+        }
+        if (! (cursor_end & 0x7f)) {
+            width--;
+        }
+        /* If the cursor is effectively invisible, move it out of visible area 
*/
+        if (width <= 0) {
+            width = 1;
+            x = 0;
+            y = Crtc->Y + Crtc->Height;
+            hotx = 0;
+            hoty = 0;
+        }
+        setCursorSize(Cursor, width, Cursor->Height);
+    }
+
     setCursorPos (Cursor, x, y, hotx, hoty);
     lockCursor   (Cursor, FALSE);
 }
-- 
1.6.0.6