[PATCH RESEND xserver v3] xfree86: Immediately handle failure to set HW cursor

Alexandre Courbot acourbot at nvidia.com
Thu May 12 07:04:56 UTC 2016


There is currently no reliable way to report failure to set a HW
cursor. Still such failures can happen if e.g. the MODE_CURSOR DRM
ioctl fails (which currently happens at least with modesetting on Tegra
for format incompatibility reasons).

As failures are currently handled by setting the HW cursor size to
(0,0), the fallback to SW cursor will not happen until the next time the
cursor changes and xf86CursorSetCursor() is called again. In the
meantime, the cursor will be invisible to the user.

This patch addresses that by adding _xf86CrtcFuncs::set_cursor_check and
_xf86CursorInfoRec::ShowCursorCheck hook variants that return booleans.
This allows to propagate errors up to xf86CursorSetCursor(), which can
then fall back to using the SW cursor immediately.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
Cc: Michael Thayer <michael.thayer at oracle.com>
---
Hoping to see this patch go through - all issues in the previous versions
have been addressed, and the logic it introduces seems saner than setting
the cursor size to (0, 0).

 hw/xfree86/drivers/modesetting/drmmode_display.c | 15 ++++++----
 hw/xfree86/modes/xf86Crtc.h                      |  4 +++
 hw/xfree86/modes/xf86Cursors.c                   | 35 ++++++++++++++++++------
 hw/xfree86/ramdac/xf86Cursor.h                   |  1 +
 hw/xfree86/ramdac/xf86HWCurs.c                   | 18 +++++++++---
 5 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
index a34bae65b533..aba9080e40d0 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -590,7 +590,7 @@ drmmode_set_cursor_position(xf86CrtcPtr crtc, int x, int y)
     drmModeMoveCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id, x, y);
 }
 
-static void
+static Bool
 drmmode_set_cursor(xf86CrtcPtr crtc)
 {
     drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
@@ -609,7 +609,7 @@ drmmode_set_cursor(xf86CrtcPtr crtc)
                               handle, ms->cursor_width, ms->cursor_height,
                               cursor->bits->xhot, cursor->bits->yhot);
         if (!ret)
-            return;
+            return TRUE;
         if (ret == -EINVAL)
             use_set_cursor2 = FALSE;
     }
@@ -624,7 +624,10 @@ drmmode_set_cursor(xf86CrtcPtr crtc)
         cursor_info->MaxWidth = cursor_info->MaxHeight = 0;
         drmmode_crtc->drmmode->sw_cursor = TRUE;
         /* fallback to swcursor */
+	return FALSE;
     }
+
+    return TRUE;
 }
 
 static void
@@ -657,12 +660,12 @@ drmmode_hide_cursor(xf86CrtcPtr crtc)
                      ms->cursor_width, ms->cursor_height);
 }
 
-static void
-drmmode_show_cursor(xf86CrtcPtr crtc)
+static Bool
+drmmode_show_cursor_check(xf86CrtcPtr crtc)
 {
     drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
     drmmode_crtc->cursor_up = TRUE;
-    drmmode_set_cursor(crtc);
+    return drmmode_set_cursor(crtc);
 }
 
 static void
@@ -902,7 +905,7 @@ static const xf86CrtcFuncsRec drmmode_crtc_funcs = {
     .set_mode_major = drmmode_set_mode_major,
     .set_cursor_colors = drmmode_set_cursor_colors,
     .set_cursor_position = drmmode_set_cursor_position,
-    .show_cursor = drmmode_show_cursor,
+    .show_cursor_check = drmmode_show_cursor_check,
     .hide_cursor = drmmode_hide_cursor,
     .load_cursor_argb = drmmode_load_cursor_argb,
 
diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
index 8b0160845a02..4bc4d6fc78fd 100644
--- a/hw/xfree86/modes/xf86Crtc.h
+++ b/hw/xfree86/modes/xf86Crtc.h
@@ -187,6 +187,8 @@ typedef struct _xf86CrtcFuncs {
      */
     void
      (*show_cursor) (xf86CrtcPtr crtc);
+    Bool
+     (*show_cursor_check) (xf86CrtcPtr crtc);
 
     /**
      * Hide cursor
@@ -982,6 +984,8 @@ extern _X_EXPORT void
  */
 extern _X_EXPORT void
  xf86_show_cursors(ScrnInfoPtr scrn);
+extern _X_EXPORT Bool
+ xf86_show_cursors_check(ScrnInfoPtr scrn);
 
 /**
  * Called by the driver to turn cursors off
diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c
index 5df1ab73a37e..2ba150ce6441 100644
--- a/hw/xfree86/modes/xf86Cursors.c
+++ b/hw/xfree86/modes/xf86Cursors.c
@@ -333,17 +333,26 @@ xf86_hide_cursors(ScrnInfoPtr scrn)
     }
 }
 
-static void
+static Bool
 xf86_crtc_show_cursor(xf86CrtcPtr crtc)
 {
-    if (!crtc->cursor_shown && crtc->cursor_in_range) {
-        crtc->funcs->show_cursor(crtc);
-        crtc->cursor_shown = TRUE;
+    if (!crtc->cursor_in_range)
+        return TRUE;
+
+    if (!crtc->cursor_shown) {
+        if (crtc->funcs->show_cursor_check) {
+            crtc->cursor_shown = crtc->funcs->show_cursor_check(crtc);
+        } else {
+            crtc->funcs->show_cursor(crtc);
+            crtc->cursor_shown = TRUE;
+        }
     }
+
+    return crtc->cursor_shown;
 }
 
-void
-xf86_show_cursors(ScrnInfoPtr scrn)
+Bool
+xf86_show_cursors_check(ScrnInfoPtr scrn)
 {
     xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
     int c;
@@ -352,9 +361,17 @@ xf86_show_cursors(ScrnInfoPtr scrn)
     for (c = 0; c < xf86_config->num_crtc; c++) {
         xf86CrtcPtr crtc = xf86_config->crtc[c];
 
-        if (crtc->enabled)
-            xf86_crtc_show_cursor(crtc);
+        if (crtc->enabled && !xf86_crtc_show_cursor(crtc))
+            return FALSE;
     }
+
+    return TRUE;
+}
+
+void
+xf86_show_cursors(ScrnInfoPtr scrn)
+{
+    xf86_show_cursors_check(scrn);
 }
 
 void
@@ -631,7 +648,7 @@ xf86_cursors_init(ScreenPtr screen, int max_width, int max_height, int flags)
     cursor_info->SetCursorPosition = xf86_set_cursor_position;
     cursor_info->LoadCursorImageCheck = xf86_load_cursor_image;
     cursor_info->HideCursor = xf86_hide_cursors;
-    cursor_info->ShowCursor = xf86_show_cursors;
+    cursor_info->ShowCursorCheck = xf86_show_cursors_check;
     cursor_info->UseHWCursor = xf86_use_hw_cursor;
     if (flags & HARDWARE_CURSOR_ARGB) {
         cursor_info->UseHWCursorARGB = xf86_use_hw_cursor_argb;
diff --git a/hw/xfree86/ramdac/xf86Cursor.h b/hw/xfree86/ramdac/xf86Cursor.h
index 6e88240d9ab7..a1afb1c86ed9 100644
--- a/hw/xfree86/ramdac/xf86Cursor.h
+++ b/hw/xfree86/ramdac/xf86Cursor.h
@@ -16,6 +16,7 @@ typedef struct _xf86CursorInfoRec {
     Bool (*LoadCursorImageCheck) (ScrnInfoPtr pScrn, unsigned char *bits);
     void (*HideCursor) (ScrnInfoPtr pScrn);
     void (*ShowCursor) (ScrnInfoPtr pScrn);
+    Bool (*ShowCursorCheck) (ScrnInfoPtr pScrn);
     unsigned char *(*RealizeCursor) (struct _xf86CursorInfoRec *, CursorPtr);
     Bool (*UseHWCursor) (ScreenPtr, CursorPtr);
 
diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
index 84febe0df5cd..458781cae7e9 100644
--- a/hw/xfree86/ramdac/xf86HWCurs.c
+++ b/hw/xfree86/ramdac/xf86HWCurs.c
@@ -89,7 +89,8 @@ xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr)
     if (!infoPtr->SetCursorPosition ||
         !xf86DriverHasLoadCursorImage(infoPtr) ||
         !infoPtr->HideCursor ||
-        !infoPtr->ShowCursor || !infoPtr->SetCursorColors)
+        (!infoPtr->ShowCursor && !infoPtr->ShowCursorCheck) ||
+        !infoPtr->SetCursorColors)
         return FALSE;
 
     if (infoPtr->RealizeCursor) {
@@ -119,6 +120,15 @@ xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr)
     return TRUE;
 }
 
+static Bool
+xf86ShowCursor(xf86CursorInfoPtr infoPtr)
+{
+    if (*infoPtr->ShowCursorCheck)
+        return (*infoPtr->ShowCursorCheck) (infoPtr->pScrn);
+    (*infoPtr->ShowCursor) (infoPtr->pScrn);
+    return TRUE;
+}
+
 Bool
 xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
 {
@@ -161,8 +171,8 @@ xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
 
     (*infoPtr->SetCursorPosition) (infoPtr->pScrn, x, y);
 
-    (*infoPtr->ShowCursor) (infoPtr->pScrn);
-    return TRUE;
+    return xf86ShowCursor(infoPtr);
+
 }
 
 void
@@ -184,7 +194,7 @@ xf86SetTransparentCursor(ScreenPtr pScreen)
         xf86DriverLoadCursorImage (infoPtr,
                                    ScreenPriv->transparentData);
 
-    (*infoPtr->ShowCursor) (infoPtr->pScrn);
+    xf86ShowCursor(infoPtr);
 }
 
 void
-- 
2.8.2



More information about the xorg-devel mailing list