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

Alexandre Courbot acourbot at nvidia.com
Wed Jun 22 08:48:59 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>
Reviewed-by: Keith Packard <keithp at keithp.com>
Cc: Michael Thayer <michael.thayer at oracle.com>
Cc: Michel Dänzer <michel at daenzer.net>
---
 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 7e5901a71bcc..15285cb9f8f5 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -543,7 +543,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;
@@ -561,7 +561,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;
     }
@@ -576,7 +576,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
@@ -609,12 +612,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
@@ -861,7 +864,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 fb1dd4f31890..7a2a47360cd3 100644
--- a/hw/xfree86/modes/xf86Crtc.h
+++ b/hw/xfree86/modes/xf86Crtc.h
@@ -195,6 +195,8 @@ typedef struct _xf86CrtcFuncs {
      */
     void
      (*show_cursor) (xf86CrtcPtr crtc);
+    Bool
+     (*show_cursor_check) (xf86CrtcPtr crtc);
 
     /**
      * Hide cursor
@@ -991,6 +993,8 @@ static _X_INLINE _X_DEPRECATED void xf86_reload_cursors(ScreenPtr screen) {}
  */
 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 d1e3302b0ef9..1a8e9d5d8708 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);
 }
 
 static void
@@ -624,7 +641,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 320ec0ce621e..c69d9022184d 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.3



More information about the xorg-devel mailing list