[PATCH] Add a return value to set_cursor_position() to allow it to report failure

Michael Thayer michael.thayer at oracle.com
Thu Apr 24 02:36:34 PDT 2014


set_cursor_position() may need to be able to fail and have the server fall
back to a software cursor in at least the situation in which we are running
on virtual hardware and using the host cursor as a hardware cursor for the
guest but cannot change its position.  Rename relevant APIs to force driver
build breaks as the return type change will only trigger warnings otherwise.

Signed-off-by: Michael Thayer <michael.thayer at oracle.com>
---
 hw/xfree86/common/xf86Module.h     |  2 +-
 hw/xfree86/modes/xf86Crtc.h        |  4 ++--
 hw/xfree86/modes/xf86Cursors.c     | 18 ++++++++++++------
 hw/xfree86/ramdac/IBM.c            | 10 ++++++----
 hw/xfree86/ramdac/TI.c             |  5 +++--
 hw/xfree86/ramdac/xf86Cursor.c     | 28 ++++++++++++++++++++++++----
 hw/xfree86/ramdac/xf86Cursor.h     |  2 +-
 hw/xfree86/ramdac/xf86CursorPriv.h |  4 +++-
 hw/xfree86/ramdac/xf86HWCurs.c     | 10 +++++-----
 9 files changed, 57 insertions(+), 26 deletions(-)

diff --git a/hw/xfree86/common/xf86Module.h b/hw/xfree86/common/xf86Module.h
index 62ac95d..b848f53 100644
--- a/hw/xfree86/common/xf86Module.h
+++ b/hw/xfree86/common/xf86Module.h
@@ -80,7 +80,7 @@ typedef enum {
  * mask is 0xFFFF0000.
  */
 #define ABI_ANSIC_VERSION	SET_ABI_VERSION(0, 4)
-#define ABI_VIDEODRV_VERSION	SET_ABI_VERSION(17, 0)
+#define ABI_VIDEODRV_VERSION	SET_ABI_VERSION(18, 0)
 #define ABI_XINPUT_VERSION	SET_ABI_VERSION(21, 0)
 #define ABI_EXTENSION_VERSION	SET_ABI_VERSION(8, 0)
 #define ABI_FONT_VERSION	SET_ABI_VERSION(0, 6)
diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
index 5407deb..a5c05f6 100644
--- a/hw/xfree86/modes/xf86Crtc.h
+++ b/hw/xfree86/modes/xf86Crtc.h
@@ -168,8 +168,8 @@ typedef struct _xf86CrtcFuncs {
     /**
      * Set cursor position
      */
-    void
-     (*set_cursor_position) (xf86CrtcPtr crtc, int x, int y);
+    Bool
+     (*set_cursor_position2) (xf86CrtcPtr crtc, int x, int y);
 
     /**
      * Show cursor
diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c
index 10ef6f6..828b3ba 100644
--- a/hw/xfree86/modes/xf86Cursors.c
+++ b/hw/xfree86/modes/xf86Cursors.c
@@ -355,7 +355,7 @@ xf86CrtcTransformCursorPos(xf86CrtcPtr crtc, int *x, int *y)
     *y -= dy;
 }
 
-static void
+static Bool
 xf86_crtc_set_cursor_position(xf86CrtcPtr crtc, int x, int y)
 {
     ScrnInfoPtr scrn = crtc->scrn;
@@ -363,6 +363,7 @@ xf86_crtc_set_cursor_position(xf86CrtcPtr crtc, int x, int y)
     xf86CursorInfoPtr cursor_info = xf86_config->cursor_info;
     DisplayModePtr mode = &crtc->mode;
     Bool in_range;
+    Bool ret = FALSE;
 
     /*
      * Transform position of cursor on screen
@@ -388,18 +389,21 @@ xf86_crtc_set_cursor_position(xf86CrtcPtr crtc, int x, int y)
     crtc->cursor_in_range = in_range;
 
     if (in_range) {
-        crtc->funcs->set_cursor_position(crtc, x, y);
+        if (crtc->funcs->set_cursor_position2(crtc, x, y))
+            ret = TRUE;
         xf86_crtc_show_cursor(crtc);
     }
     else
         xf86_crtc_hide_cursor(crtc);
+    return ret;
 }
 
-static void
+static Bool
 xf86_set_cursor_position(ScrnInfoPtr scrn, int x, int y)
 {
     xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
     int c;
+    Bool ret = FALSE;
 
     /* undo what xf86HWCurs did to the coordinates */
     x += scrn->frameX0;
@@ -408,8 +412,10 @@ xf86_set_cursor_position(ScrnInfoPtr scrn, int x, int y)
         xf86CrtcPtr crtc = xf86_config->crtc[c];
 
         if (crtc->enabled)
-            xf86_crtc_set_cursor_position(crtc, x, y);
+            if (xf86_crtc_set_cursor_position(crtc, x, y))
+                ret = TRUE;
     }
+    return ret;
 }
 
 /*
@@ -593,7 +599,7 @@ xf86_cursors_init(ScreenPtr screen, int max_width, int max_height, int flags)
     cursor_info->Flags = flags;
 
     cursor_info->SetCursorColors = xf86_set_cursor_colors;
-    cursor_info->SetCursorPosition = xf86_set_cursor_position;
+    cursor_info->SetCursorPosition2 = xf86_set_cursor_position;
     cursor_info->LoadCursorImage = xf86_load_cursor_image;
     cursor_info->HideCursor = xf86_hide_cursors;
     cursor_info->ShowCursor = xf86_show_cursors;
@@ -666,7 +672,7 @@ xf86_reload_cursors(ScreenPtr screen)
 
         x += scrn->frameX0 + cursor_screen_priv->HotX;
         y += scrn->frameY0 + cursor_screen_priv->HotY;
-        (*cursor_info->SetCursorPosition) (scrn, x, y);
+        (*cursor_info->SetCursorPosition2) (scrn, x, y);
     }
 }
 
diff --git a/hw/xfree86/ramdac/IBM.c b/hw/xfree86/ramdac/IBM.c
index 872d3d4..21dc172 100644
--- a/hw/xfree86/ramdac/IBM.c
+++ b/hw/xfree86/ramdac/IBM.c
@@ -505,7 +505,7 @@ IBMramdac640HideCursor(ScrnInfoPtr pScrn)
     (*ramdacPtr->WriteDAC) (pScrn, RGB640_CURSOR_CONTROL, 0x00, 0x08);
 }
 
-static void
+static Bool
 IBMramdac526SetCursorPosition(ScrnInfoPtr pScrn, int x, int y)
 {
     RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
@@ -519,9 +519,10 @@ IBMramdac526SetCursorPosition(ScrnInfoPtr pScrn, int x, int y)
     (*ramdacPtr->WriteDAC) (pScrn, IBMRGB_curs_xh, 0x00, (x >> 8) & 0xf);
     (*ramdacPtr->WriteDAC) (pScrn, IBMRGB_curs_yl, 0x00, y & 0xff);
     (*ramdacPtr->WriteDAC) (pScrn, IBMRGB_curs_yh, 0x00, (y >> 8) & 0xf);
+    return TRUE;
 }
 
-static void
+static Bool
 IBMramdac640SetCursorPosition(ScrnInfoPtr pScrn, int x, int y)
 {
     RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
@@ -535,6 +536,7 @@ IBMramdac640SetCursorPosition(ScrnInfoPtr pScrn, int x, int y)
     (*ramdacPtr->WriteDAC) (pScrn, RGB640_CURS_X_HIGH, 0x00, (x >> 8) & 0xf);
     (*ramdacPtr->WriteDAC) (pScrn, RGB640_CURS_Y_LOW, 0x00, y & 0xff);
     (*ramdacPtr->WriteDAC) (pScrn, RGB640_CURS_Y_HIGH, 0x00, (y >> 8) & 0xf);
+    return TRUE;
 }
 
 static void
@@ -621,7 +623,7 @@ IBMramdac526HWCursorInit(xf86CursorInfoPtr infoPtr)
         HARDWARE_CURSOR_AND_SOURCE_WITH_MASK |
         HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_1;
     infoPtr->SetCursorColors = IBMramdac526SetCursorColors;
-    infoPtr->SetCursorPosition = IBMramdac526SetCursorPosition;
+    infoPtr->SetCursorPosition2 = IBMramdac526SetCursorPosition;
     infoPtr->LoadCursorImage = IBMramdac526LoadCursorImage;
     infoPtr->HideCursor = IBMramdac526HideCursor;
     infoPtr->ShowCursor = IBMramdac526ShowCursor;
@@ -637,7 +639,7 @@ IBMramdac640HWCursorInit(xf86CursorInfoPtr infoPtr)
         HARDWARE_CURSOR_AND_SOURCE_WITH_MASK |
         HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_1;
     infoPtr->SetCursorColors = IBMramdac640SetCursorColors;
-    infoPtr->SetCursorPosition = IBMramdac640SetCursorPosition;
+    infoPtr->SetCursorPosition2 = IBMramdac640SetCursorPosition;
     infoPtr->LoadCursorImage = IBMramdac640LoadCursorImage;
     infoPtr->HideCursor = IBMramdac640HideCursor;
     infoPtr->ShowCursor = IBMramdac640ShowCursor;
diff --git a/hw/xfree86/ramdac/TI.c b/hw/xfree86/ramdac/TI.c
index 7d4e0d7..1027743 100644
--- a/hw/xfree86/ramdac/TI.c
+++ b/hw/xfree86/ramdac/TI.c
@@ -606,7 +606,7 @@ TIramdacHideCursor(ScrnInfoPtr pScrn)
     (*ramdacPtr->WriteDAC) (pScrn, TIDAC_ind_curs_ctrl, 0, 0x00);
 }
 
-static void
+static Bool
 TIramdacSetCursorPosition(ScrnInfoPtr pScrn, int x, int y)
 {
     RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
@@ -618,6 +618,7 @@ TIramdacSetCursorPosition(ScrnInfoPtr pScrn, int x, int y)
     (*ramdacPtr->WriteDAC) (pScrn, TIDAC_CURS_XHIGH, 0, (x >> 8) & 0x0f);
     (*ramdacPtr->WriteDAC) (pScrn, TIDAC_CURS_YLOW, 0, y & 0xff);
     (*ramdacPtr->WriteDAC) (pScrn, TIDAC_CURS_YHIGH, 0, (y >> 8) & 0x0f);
+    return TRUE;
 }
 
 static void
@@ -675,7 +676,7 @@ TIramdacHWCursorInit(xf86CursorInfoPtr infoPtr)
         HARDWARE_CURSOR_TRUECOLOR_AT_8BPP |
         HARDWARE_CURSOR_SOURCE_MASK_NOT_INTERLEAVED;
     infoPtr->SetCursorColors = TIramdacSetCursorColors;
-    infoPtr->SetCursorPosition = TIramdacSetCursorPosition;
+    infoPtr->SetCursorPosition2 = TIramdacSetCursorPosition;
     infoPtr->LoadCursorImage = TIramdacLoadCursorImage;
     infoPtr->HideCursor = TIramdacHideCursor;
     infoPtr->ShowCursor = TIramdacShowCursor;
diff --git a/hw/xfree86/ramdac/xf86Cursor.c b/hw/xfree86/ramdac/xf86Cursor.c
index fac6822..cc0d011 100644
--- a/hw/xfree86/ramdac/xf86Cursor.c
+++ b/hw/xfree86/ramdac/xf86Cursor.c
@@ -72,6 +72,8 @@ xf86InitCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr)
 
     ScreenPriv->SWCursor = TRUE;
     ScreenPriv->isUp = FALSE;
+    ScreenPriv->lastCursorInHardware = FALSE;
+    ScreenPriv->lastMoveCursorSucceeded = TRUE;
     ScreenPriv->CurrentCursor = NULL;
     ScreenPriv->CursorInfoPtr = infoPtr;
     ScreenPriv->PalettedCursor = FALSE;
@@ -303,6 +305,7 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs,
         (xf86CursorScreenPtr) dixLookupPrivate(&pScreen->devPrivates,
                                                xf86CursorScreenKey);
     xf86CursorInfoPtr infoPtr = ScreenPriv->CursorInfoPtr;
+    Bool showHWCursor = ScreenPriv->lastMoveCursorSucceeded;
 
     if (pCurs == NullCursor) {  /* means we're supposed to remove the cursor */
         if (ScreenPriv->SWCursor ||
@@ -312,6 +315,7 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs,
         else if (ScreenPriv->isUp) {
             xf86SetCursor(pScreen, NullCursor, x, y);
             ScreenPriv->isUp = FALSE;
+            ScreenPriv->lastCursorInHardware = FALSE;
         }
         if (ScreenPriv->CurrentCursor)
             FreeCursor(ScreenPriv->CurrentCursor, None);
@@ -348,11 +352,13 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs,
                (cursor->bits->width <= infoPtr->MaxWidth) &&
                (!infoPtr->UseHWCursor || (*infoPtr->UseHWCursor) (pScreen, cursor)))))) {
             
-            if (ScreenPriv->SWCursor)   /* remove the SW cursor */
+            if (ScreenPriv->SWCursor && showHWCursor)   /* remove the SW cursor */
                 (*ScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen,
                                                        NullCursor, x, y);
 
-            if (xf86SetCursor(pScreen, cursor, x, y)) {
+            ScreenPriv->lastCursorInHardware =
+                xf86SetCursor(pScreen, cursor, x, y);
+            if (ScreenPriv->lastCursorInHardware && showHWCursor) {
                 ScreenPriv->SWCursor = FALSE;
                 ScreenPriv->isUp = TRUE;
 
@@ -360,6 +366,8 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs,
                 return;
             }
         }
+        else
+            ScreenPriv->lastCursorInHardware = FALSE;
 
         miPointerSetWaitForUpdate(pScreen, TRUE);
 
@@ -391,6 +399,7 @@ xf86CursorMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y)
     xf86CursorScreenPtr ScreenPriv =
         (xf86CursorScreenPtr) dixLookupPrivate(&pScreen->devPrivates,
                                                xf86CursorScreenKey);
+    Bool needSetCursor = FALSE;
 
     /* only update coordinate state for first sprite, otherwise we get jumps
        when removing a sprite. The second sprite is never HW rendered anyway */
@@ -398,13 +407,24 @@ xf86CursorMoveCursor(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y)
         ScreenPriv->x = x;
         ScreenPriv->y = y;
 
+        if (ScreenPriv->lastCursorInHardware) {
+            Bool moveCursorSucceeded = xf86MoveCursor2(pScreen, x, y);
+            if (!moveCursorSucceeded !=
+                !ScreenPriv->lastMoveCursorSucceeded) {
+                ScreenPriv->lastMoveCursorSucceeded = moveCursorSucceeded;
+                needSetCursor = TRUE;
+            }
+        }
         if (ScreenPriv->CursorToRestore)
             xf86CursorSetCursor(pDev, pScreen, ScreenPriv->CursorToRestore, x,
                                 y);
+        else if (needSetCursor)
+            /* This contains the logic to correctly select whether to use a
+             * software or a hardware cursor. */
+            xf86CursorSetCursor(pDev, pScreen, ScreenPriv->CurrentCursor, x,
+                                y);
         else if (ScreenPriv->SWCursor)
             (*ScreenPriv->spriteFuncs->MoveCursor) (pDev, pScreen, x, y);
-        else if (ScreenPriv->isUp)
-            xf86MoveCursor(pScreen, x, y);
     }
     else
         (*ScreenPriv->spriteFuncs->MoveCursor) (pDev, pScreen, x, y);
diff --git a/hw/xfree86/ramdac/xf86Cursor.h b/hw/xfree86/ramdac/xf86Cursor.h
index 1ecbdcd..2d00333 100644
--- a/hw/xfree86/ramdac/xf86Cursor.h
+++ b/hw/xfree86/ramdac/xf86Cursor.h
@@ -11,7 +11,7 @@ typedef struct _xf86CursorInfoRec {
     int MaxWidth;
     int MaxHeight;
     void (*SetCursorColors) (ScrnInfoPtr pScrn, int bg, int fg);
-    void (*SetCursorPosition) (ScrnInfoPtr pScrn, int x, int y);
+    Bool (*SetCursorPosition2) (ScrnInfoPtr pScrn, int x, int y);
     Bool (*LoadCursorImage) (ScrnInfoPtr pScrn, unsigned char *bits);
     void (*HideCursor) (ScrnInfoPtr pScrn);
     void (*ShowCursor) (ScrnInfoPtr pScrn);
diff --git a/hw/xfree86/ramdac/xf86CursorPriv.h b/hw/xfree86/ramdac/xf86CursorPriv.h
index f34c1c7..c8dc7ae 100644
--- a/hw/xfree86/ramdac/xf86CursorPriv.h
+++ b/hw/xfree86/ramdac/xf86CursorPriv.h
@@ -12,6 +12,8 @@
 typedef struct {
     Bool SWCursor;
     Bool isUp;
+    Bool lastCursorInHardware;
+    Bool lastMoveCursorSucceeded;
     Bool showTransparent;
     short HotX;
     short HotY;
@@ -39,7 +41,7 @@ typedef struct {
 
 Bool xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y);
 void xf86SetTransparentCursor(ScreenPtr pScreen);
-void xf86MoveCursor(ScreenPtr pScreen, int x, int y);
+Bool xf86MoveCursor2(ScreenPtr pScreen, int x, int y);
 void xf86RecolorCursor(ScreenPtr pScreen, CursorPtr pCurs, Bool displayed);
 Bool xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr);
 
diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
index 0b5caa2..4fa3f7d 100644
--- a/hw/xfree86/ramdac/xf86HWCurs.c
+++ b/hw/xfree86/ramdac/xf86HWCurs.c
@@ -86,7 +86,7 @@ xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr)
         return FALSE;
 
     /* These are required for now */
-    if (!infoPtr->SetCursorPosition ||
+    if (!infoPtr->SetCursorPosition2 ||
         !infoPtr->LoadCursorImage ||
         !infoPtr->HideCursor ||
         !infoPtr->ShowCursor || !infoPtr->SetCursorColors)
@@ -163,7 +163,7 @@ xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
 
     xf86RecolorCursor(pScreen, pCurs, 1);
 
-    (*infoPtr->SetCursorPosition) (infoPtr->pScrn, x, y);
+    (*infoPtr->SetCursorPosition2) (infoPtr->pScrn, x, y);
 
     (*infoPtr->ShowCursor) (infoPtr->pScrn);
     return TRUE;
@@ -191,8 +191,8 @@ xf86SetTransparentCursor(ScreenPtr pScreen)
     (*infoPtr->ShowCursor) (infoPtr->pScrn);
 }
 
-void
-xf86MoveCursor(ScreenPtr pScreen, int x, int y)
+Bool
+xf86MoveCursor2(ScreenPtr pScreen, int x, int y)
 {
     xf86CursorScreenPtr ScreenPriv =
         (xf86CursorScreenPtr) dixLookupPrivate(&pScreen->devPrivates,
@@ -202,7 +202,7 @@ xf86MoveCursor(ScreenPtr pScreen, int x, int y)
     x -= infoPtr->pScrn->frameX0 + ScreenPriv->HotX;
     y -= infoPtr->pScrn->frameY0 + ScreenPriv->HotY;
 
-    (*infoPtr->SetCursorPosition) (infoPtr->pScrn, x, y);
+    return (*infoPtr->SetCursorPosition2) (infoPtr->pScrn, x, y);
 }
 
 void
-- 
ORACLE Deutschland B.V. & Co. KG   Michael Thayer
Werkstrasse 24                     VirtualBox engineering
71384 Weinstadt, Germany           mailto:michael.thayer at oracle.com

Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Geschäftsführer: Jürgen Kunz 
Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Astrid Kepper, Val Maher


More information about the xorg-devel mailing list