On Don, 2003-02-13 at 22:54, Keith Packard wrote:
> Around 13 o'clock on Feb 13, Marc Aurele La France wrote:
> 
> > The problem was that the switching back-and-forth between the new "cutesy"
> > cursors and the standard ones tickles a hardware bug in Mach64, Rage 128
> > and Radeon variants.
> 
> The Radeon is slightly different in that it has hardware support for ARGB 
> cursors, hence switching on that chip is different.
> 
> I suggested that one fix for this is to simply always use the ARGB style 
> hardware cursors and map core cursors to that model, rather than 
> attempting to switch back and forth.
> 
> Daniel Stone built a patch, and I hacked it a bit.  

Fredrik H�glund (CC'd in case he's not subscribed here) has also written
such a patch, and I've fixed it for big endian machines. The result is
attached.

> Taking that along with Marc's idea of disabling cursors by turning them 
> transparent should leave us with Radeon cursor support that doesn't have 
> any issues with mode switching.

This patch still actually disables the cursor instead of only making it
fully transparent, is that really an issue?

> Would such a patch make sense for 4.3?

IMHO yes, the question is if Kevin agrees and if he'll make it.


-- 
Earthling Michel D�nzer (MrCooper)/ Debian GNU/Linux (powerpc) developer
XFree86 and DRI project member   /  CS student, Free Software enthusiast
Index: radeon_cursor.c
===================================================================
RCS file: /cvs/xc/programs/Xserver/hw/xfree86/drivers/ati/radeon_cursor.c,v
retrieving revision 1.20
diff -p -u -r1.20 radeon_cursor.c
--- radeon_cursor.c	2003/01/29 18:06:06	1.20
+++ radeon_cursor.c	2003/02/09 01:25:15
@@ -52,22 +52,56 @@
 				/* X and server generic header files */
 #include "xf86.h"
 
+#define WIDTH 64
+#define HEIGHT 64
 
+/* Mono ARGB cursor colors (premultiplied) */
+static const CARD32 color[] = {
+	0x00000000, /* White, fully transparent */
+	0x00000000, /* Black, fully transparent */
+	0xffffffff, /* White, fully opaque */
+	0xff000000, /* Black, fully opaque */
+};
+
 /* Set cursor foreground and background colors */
 static void RADEONSetCursorColors(ScrnInfoPtr pScrn, int bg, int fg)
 {
     RADEONInfoPtr  info       = RADEONPTR(pScrn);
+    CARD32        *pixels     = (CARD32 *)(pointer)(info->FB + info->cursor_start);
+    CARD32         pixel, i;
+#if X_BYTE_ORDER == X_BIG_ENDIAN
     unsigned char *RADEONMMIO = info->MMIO;
+    CARD32         surface_cntl = INREG(RADEON_SURFACE_CNTL);
+#endif
 
-    if (info->IsSecondary || info->Clone) {
-	OUTREG(RADEON_CUR2_CLR0, bg);
-	OUTREG(RADEON_CUR2_CLR1, fg);
-    }
+#ifdef ARGB_CURSOR
+    /* Don't recolor cursors set with SetCursorARGB. */
+    if (info->cursor_argb)
+	return;
+#endif
 
-    if (!info->IsSecondary) {
-	OUTREG(RADEON_CUR_CLR0, bg);
-	OUTREG(RADEON_CUR_CLR1, fg);
-    }
+    /* Don't recolor the image if we don't have to */
+    if (bg == 0x00ffffff && fg == 0)
+	return;
+
+#if X_BYTE_ORDER == X_BIG_ENDIAN
+    OUTREG(RADEON_SURFACE_CNTL, (surface_cntl | RADEON_NONSURF_AP0_SWP_32BPP)
+				& ~RADEON_NONSURF_AP0_SWP_16BPP	);
+#endif
+
+    /* Note: We assume that the pixels are either fully opaque or fully
+     *       transparent so we won't premultiply them. We also know that
+     *       the image is black & white since LoadCursorImage is called
+     *       before SetCursorColors.
+     */
+    for (i = 0; i < WIDTH * HEIGHT; i++, pixels++)
+    	if ((pixel = *pixels))
+	    *pixels = (pixel & 0x00ffffff ? bg : fg) | 0xff000000;
+
+#if X_BYTE_ORDER == X_BIG_ENDIAN
+    /* restore byte swapping */
+    OUTREG(RADEON_SURFACE_CNTL, surface_cntl);
+#endif
 }
 
 
@@ -78,23 +112,19 @@ static void RADEONSetCursorPosition(Scrn
 {
     RADEONInfoPtr      info       = RADEONPTR(pScrn);
     unsigned char     *RADEONMMIO = info->MMIO;
-    xf86CursorInfoPtr  cursor     = info->cursor;
     int                xorigin    = 0;
     int                yorigin    = 0;
     int                total_y    = pScrn->frameY1 - pScrn->frameY0;
     int                X2         = pScrn->frameX0 + x;
     int                Y2         = pScrn->frameY0 + y;
-    int		       stride     = 16;
+    int		       stride     = 256;
 
-#ifdef ARGB_CURSOR
-    if (info->cursor_argb) stride = 256;
-#endif
     if (x < 0)                        xorigin = -x+1;
     if (y < 0)                        yorigin = -y+1;
     if (y > total_y)                  y       = total_y;
     if (info->Flags & V_DBLSCAN)      y       *= 2;
-    if (xorigin >= cursor->MaxWidth)  xorigin = cursor->MaxWidth - 1;
-    if (yorigin >= cursor->MaxHeight) yorigin = cursor->MaxHeight - 1;
+    if (xorigin >= WIDTH)             xorigin = WIDTH - 1;
+    if (yorigin >= HEIGHT)            yorigin = HEIGHT - 1;
 
     if (info->Clone) {
 	int X0 = 0;
@@ -171,8 +201,8 @@ static void RADEONSetCursorPosition(Scrn
 	yorigin = 0;
 	if (X2 < 0) xorigin = -X2 + 1;
 	if (Y2 < 0) yorigin = -Y2 + 1;
-	if (xorigin >= cursor->MaxWidth)  xorigin = cursor->MaxWidth - 1;
-	if (yorigin >= cursor->MaxHeight) yorigin = cursor->MaxHeight - 1;
+	if (xorigin >= WIDTH)  xorigin = WIDTH - 1;
+	if (yorigin >= HEIGHT) yorigin = HEIGHT - 1;
 
 	OUTREG(RADEON_CUR2_HORZ_VERT_OFF,  (RADEON_CUR2_LOCK
 					    | (xorigin << 16)
@@ -192,37 +222,28 @@ static void RADEONLoadCursorImage(ScrnIn
 {
     RADEONInfoPtr  info       = RADEONPTR(pScrn);
     unsigned char *RADEONMMIO = info->MMIO;
-    CARD32        *s          = (CARD32 *)(pointer)image;
+    CARD8         *s          = (CARD8 *)(pointer)image;
     CARD32        *d          = (CARD32 *)(pointer)(info->FB + info->cursor_start);
-    int            y;
     CARD32         save1      = 0;
     CARD32         save2      = 0;
+    CARD8          chunk;
+    CARD32         i, j;
 #if X_BYTE_ORDER == X_BIG_ENDIAN
     CARD32         surface_cntl = INREG(RADEON_SURFACE_CNTL);
 
-    OUTREG(RADEON_SURFACE_CNTL, surface_cntl & ~(RADEON_NONSURF_AP0_SWP_16BPP
-						 | RADEON_NONSURF_AP0_SWP_32BPP));
+    OUTREG(RADEON_SURFACE_CNTL, (surface_cntl | RADEON_NONSURF_AP0_SWP_32BPP)
+				& ~RADEON_NONSURF_AP0_SWP_16BPP	);
 #endif
 
     if (!info->IsSecondary) {
-	save1 = INREG(RADEON_CRTC_GEN_CNTL);
-	if (save1 & (3 << 20)) {
-	    /* Workaround the flickering problem when switching from
-	     * rgba cursor.  This happens even when cursor is turned
-	     * off.  There may be a better way to do this.
-	     */
-	    RADEONWaitForVerticalSync(pScrn);
-	    save1 &= ~(CARD32) (3 << 20);
-	}
+	save1 = INREG(RADEON_CRTC_GEN_CNTL) & ~(CARD32) (3 << 20);
+	save1 |=  (CARD32) (2 << 20);
 	OUTREG(RADEON_CRTC_GEN_CNTL, save1 & (CARD32)~RADEON_CRTC_CUR_EN);
     }
 
     if (info->IsSecondary || info->Clone) {
-	save2 = INREG(RADEON_CRTC2_GEN_CNTL);
-	if (save2 & (3 << 20)) {
-	    RADEONWaitForVerticalSync2(pScrn);
-	    save2 &= ~(CARD32) (3 << 20);
-	}
+	save2 = INREG(RADEON_CRTC2_GEN_CNTL) & ~(CARD32) (3 << 20);
+	save2 |= (CARD32) (2 << 20);
 	OUTREG(RADEON_CRTC2_GEN_CNTL, save2 & (CARD32)~RADEON_CRTC2_CUR_EN);
     }
 
@@ -230,20 +251,13 @@ static void RADEONLoadCursorImage(ScrnIn
     info->cursor_argb = FALSE;
 #endif
 
-    for (y = 0; y < 64; y++) {
-	*d++ = *s++;
-	*d++ = *s++;
-	*d++ = *s++;
-	*d++ = *s++;
-    }
+    /* Convert the bitmap to ARGB32 */
+    for (i = 0; i < WIDTH * HEIGHT / (8 * sizeof(chunk) / 2); i++)
+    {
+	chunk = *s++;
 
-    /* Set the area after the cursor to be all transparent so that we
-       won't display corrupted cursors on the screen */
-    for (y = 0; y < 64; y++) {
-	*d++ = 0xffffffff; /* The AND bits */
-	*d++ = 0xffffffff;
-	*d++ = 0x00000000; /* The XOR bits */
-	*d++ = 0x00000000;
+	for (j = 0; j < 4 * sizeof(chunk); j++, chunk >>= 2)
+	    *d++ = color[chunk & 3];
     }
 
     if (!info->IsSecondary)
@@ -304,7 +318,7 @@ static Bool RADEONUseHWCursorARGB (Scree
     RADEONInfoPtr  info  = RADEONPTR(pScrn);
 
     if (info->cursor_start &&
-	pCurs->bits->height <= 64 && pCurs->bits->width <= 64)
+	pCurs->bits->height <= HEIGHT && pCurs->bits->width <= WIDTH)
 	return TRUE;
     return FALSE;
 }
@@ -321,36 +335,25 @@ static void RADEONLoadCursorARGB (ScrnIn
     CARD32	  *i;
 #if X_BYTE_ORDER == X_BIG_ENDIAN
     CARD32         surface_cntl = INREG(RADEON_SURFACE_CNTL);
-
-    OUTREG(RADEON_SURFACE_CNTL, (surface_cntl | RADEON_NONSURF_AP0_SWP_32BPP)
-				& ~RADEON_NONSURF_AP0_SWP_16BPP	);
 #endif
 
     if (!image)
 	return;	/* XXX can't happen */
     
+#if X_BYTE_ORDER == X_BIG_ENDIAN
+    OUTREG(RADEON_SURFACE_CNTL, (surface_cntl | RADEON_NONSURF_AP0_SWP_32BPP)
+				& ~RADEON_NONSURF_AP0_SWP_16BPP	);
+#endif
+
     if (!info->IsSecondary) {
-	save1 = INREG(RADEON_CRTC_GEN_CNTL);
+	save1 = INREG(RADEON_CRTC_GEN_CNTL) & ~(CARD32) (3 << 20);
+	save1 |= (CARD32) (2 << 20);
 	OUTREG(RADEON_CRTC_GEN_CNTL, save1 & (CARD32)~RADEON_CRTC_CUR_EN);
-	if ((save1 & (3 << 20)) != (2 << 20)) {
-	    /* Workaround the flickering problem when switching from
-	     * mono cursor.  This happens even when cursor is turned
-	     * off.  There may be a better way to do this.
-	     */
-	    RADEONWaitForVerticalSync(pScrn);
-	    save1 &= ~(CARD32) (3 << 20);
-	    save1 |=  (CARD32) (2 << 20);
-	}
-	OUTREG(RADEON_CRTC_GEN_CNTL, save1 & (CARD32)~RADEON_CRTC_CUR_EN);
     }
 
     if (info->IsSecondary || info->Clone) {
-	save2 = INREG(RADEON_CRTC2_GEN_CNTL);
-	if ((save2 & (3 << 20)) != (2 << 20)) {
-	    RADEONWaitForVerticalSync2(pScrn);
-	    save2 &= ~(CARD32) (3 << 20);
-	    save2 |= (CARD32) (2 << 20);
-	}
+	save2 = INREG(RADEON_CRTC2_GEN_CNTL) & ~(CARD32) (3 << 20);
+	save2 |= (CARD32) (2 << 20);
 	OUTREG(RADEON_CRTC2_GEN_CNTL, save2 & (CARD32)~RADEON_CRTC2_CUR_EN);
     }
 
@@ -358,12 +361,8 @@ static void RADEONLoadCursorARGB (ScrnIn
     info->cursor_argb = TRUE;
 #endif
     
-    w = pCurs->bits->width;
-    if (w > 64)
-	w = 64;
-    h = pCurs->bits->height;
-    if (h > 64)
-	h = 64;
+    w = min(pCurs->bits->width, WIDTH);
+    h = min(pCurs->bits->height, HEIGHT);
     for (y = 0; y < h; y++)
     {
 	i = image;
@@ -371,12 +370,12 @@ static void RADEONLoadCursorARGB (ScrnIn
 	for (x = 0; x < w; x++)
 	    *d++ = *i++;
 	/* pad to the right with transparent */
-	for (; x < 64; x++)
+	for (; x < WIDTH; x++)
 	    *d++ = 0;
     }
     /* pad below with transparent */
-    for (; y < 64; y++)
-	for (x = 0; x < 64; x++)
+    for (; y < HEIGHT; y++)
+	for (x = 0; x < WIDTH; x++)
 	    *d++ = 0;
 
     if (!info->IsSecondary)
@@ -408,17 +407,15 @@ Bool RADEONCursorInit(ScreenPtr pScreen)
 
     if (!(cursor = info->cursor = xf86CreateCursorInfoRec())) return FALSE;
 
-    cursor->MaxWidth          = 64;
-    cursor->MaxHeight         = 64;
+    cursor->MaxWidth          = WIDTH;
+    cursor->MaxHeight         = HEIGHT;
     cursor->Flags             = (HARDWARE_CURSOR_TRUECOLOR_AT_8BPP
 
-#if X_BYTE_ORDER == X_LITTLE_ENDIAN
+#if X_BYTE_ORDER == X_BIG_ENDIAN
 				 | HARDWARE_CURSOR_BIT_ORDER_MSBFIRST
 #endif
-				 | HARDWARE_CURSOR_INVERT_MASK
 				 | HARDWARE_CURSOR_AND_SOURCE_WITH_MASK
-				 | HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_64
-				 | HARDWARE_CURSOR_SWAP_SOURCE_AND_MASK);
+				 | HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_1);
 
     cursor->SetCursorColors   = RADEONSetCursorColors;
     cursor->SetCursorPosition = RADEONSetCursorPosition;
@@ -427,14 +424,16 @@ Bool RADEONCursorInit(ScreenPtr pScreen)
     cursor->ShowCursor        = RADEONShowCursor;
     cursor->UseHWCursor       = RADEONUseHWCursor;
 
-    size                      = (cursor->MaxWidth/4) * cursor->MaxHeight;
 #ifdef ARGB_CURSOR
     cursor->UseHWCursorARGB   = RADEONUseHWCursorARGB;
     cursor->LoadCursorARGB    = RADEONLoadCursorARGB;
-    size                      = (cursor->MaxWidth * 4) * cursor->MaxHeight;
 #endif
+
+    size                      = (WIDTH * 4) * HEIGHT;
     width                     = pScrn->displayWidth;
-    height                    = (size*2 + 1023) / pScrn->displayWidth;
+    height                    = (size*2 / (pScrn->bitsPerPixel / 8)
+				 + pScrn->displayWidth - 1)
+				/ pScrn->displayWidth;
     fbarea                    = xf86AllocateOffscreenArea(pScreen,
 							  width,
 							  height,

Reply via email to