On May 04, 09 15:27:00 -0700, Yang Zhao wrote:
Alex managed to find the root of the easy cursor corruption issue: the cursor mode should be the same for both CRTCs even when disabled. This is 0001.
Superb! Applying now.
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.
@@ -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. 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.
0003 contains the multiple-of-128px and outside-viewport code with some tweaks, and is thus different from radeon. My system that's set up with dual-screen doesn't seem to be panning capable, so the viewport part is untested; please scrutinize 0003 with extra attention.
Why doesn't it seem to be panning capable?
Anyway, this patch looks good.
Thanks
Matthias
--
Matthias Hopf