xserver: Branch 'master' - 3 commits

Adam Jackson ajax at kemper.freedesktop.org
Wed Feb 8 17:05:05 UTC 2017


 hw/xfree86/drivers/modesetting/drmmode_display.c |   45 +++++++++--------------
 hw/xfree86/drivers/modesetting/drmmode_display.h |    2 -
 hw/xfree86/modes/xf86Crtc.h                      |    4 +-
 hw/xfree86/modes/xf86Cursors.c                   |   40 +++++++++++++++-----
 hw/xfree86/ramdac/xf86Cursor.h                   |   16 ++++++++
 hw/xfree86/ramdac/xf86HWCurs.c                   |    8 ++--
 6 files changed, 71 insertions(+), 44 deletions(-)

New commits:
commit eb04b20160d706d4e0e67122d0adb1e723c0da92
Author: Michael Thayer <michael.thayer at oracle.com>
Date:   Fri Sep 30 15:18:45 2016 +0200

    modesetting: allow switching from software to hardware cursors (v5).
    
    Currently if modesetting ever fails to set a hardware cursor it will switch
    to using a software cursor and never go back.  Change this to only
    permanently switch to a software cursor if -ENXIO is returned (which means
    hardware cursors not supported), and to otherwise still try a hardware
    cursor first every time a new one is set.  This is needed because hardware
    may be able to handle some cursors in hardware and others not, or virtual
    hardware may be able to handle hardware cursors at some times and not
    others.
    
    Changes since v1, v2 and v3:
     * take into account the switch to load_cursor_argb_check
     * keep the permanent software cursor fall-back if -ENXIO is returned
     * move parts of v3 into separate patches
    
    Reviewed-by: Adam Jackson <ajax at redhat.com>
    Signed-off-by: Michael Thayer <michael.thayer at oracle.com>

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 9d28068..c1e489e 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -756,37 +756,33 @@ drmmode_set_cursor(xf86CrtcPtr crtc)
     drmmode_ptr drmmode = drmmode_crtc->drmmode;
     uint32_t handle = drmmode_crtc->cursor_bo->handle;
     modesettingPtr ms = modesettingPTR(crtc->scrn);
+    CursorPtr cursor = xf86CurrentCursor(crtc->scrn->pScreen);
     int ret = -EINVAL;
 
-    if (!drmmode_crtc->set_cursor2_failed) {
-        CursorPtr cursor = xf86CurrentCursor(crtc->scrn->pScreen);
-
-        ret =
-            drmModeSetCursor2(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
-                              handle, ms->cursor_width, ms->cursor_height,
-                              cursor->bits->xhot, cursor->bits->yhot);
-        if (!ret)
-            return TRUE;
-
-        /* -EINVAL can mean that an old kernel supports drmModeSetCursor but
-         * not drmModeSetCursor2, though it can mean other things too. */
-        if (ret == -EINVAL)
-            drmmode_crtc->set_cursor2_failed = TRUE;
-    }
+    ret = drmModeSetCursor2(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
+                            handle, ms->cursor_width, ms->cursor_height,
+                            cursor->bits->xhot, cursor->bits->yhot);
 
+    /* -EINVAL can mean that an old kernel supports drmModeSetCursor but
+     * not drmModeSetCursor2, though it can mean other things too. */
     if (ret == -EINVAL)
         ret = drmModeSetCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
                                handle, ms->cursor_width, ms->cursor_height);
 
-    if (ret) {
+    /* -ENXIO normally means that the current drm driver supports neither
+     * cursor_set nor cursor_set2.  Disable hardware cursor support for
+     * the rest of the session in that case. */
+    if (ret == -ENXIO) {
         xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
         xf86CursorInfoPtr cursor_info = xf86_config->cursor_info;
 
         cursor_info->MaxWidth = cursor_info->MaxHeight = 0;
         drmmode_crtc->drmmode->sw_cursor = TRUE;
+    }
+
+    if (ret)
         /* fallback to swcursor */
         return FALSE;
-    }
     return TRUE;
 }
 
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h b/hw/xfree86/drivers/modesetting/drmmode_display.h
index 3b0eb2b..f774250 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.h
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.h
@@ -92,7 +92,6 @@ typedef struct {
     int dpms_mode;
     struct dumb_bo *cursor_bo;
     Bool cursor_up;
-    Bool set_cursor2_failed;
     uint16_t lut_r[256], lut_g[256], lut_b[256];
 
     drmmode_bo rotate_bo;
commit ecd0a62323f26b333c49bddd7237dd5118482a35
Author: Michael Thayer <michael.thayer at oracle.com>
Date:   Fri Sep 30 08:02:09 2016 +0200

    modesetting: Immediately handle failure to set HW cursor, v5
    
    Based on v4 by Alexandre Courbot <acourbot at nvidia.com>
    
    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.
    
    v5:
     - Removed parts of patch already committed as part of 14c21ea1.
     - Adjusted code slightly to match surrounding code.
     - Effectively reverted af916477 which is made unnecessary by this patch.
    
    Reviewed-by: Adam Jackson <ajax at redhat.com>
    Signed-off-by: Michael Thayer <michael.thayer at oracle.com>

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 6e755e9..9d28068 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -813,13 +813,8 @@ drmmode_load_cursor_argb_check(xf86CrtcPtr crtc, CARD32 *image)
     for (i = 0; i < ms->cursor_width * ms->cursor_height; i++)
         ptr[i] = image[i];      // cpu_to_le32(image[i]);
 
-    if (drmmode_crtc->cursor_up || !drmmode_crtc->first_cursor_load_done) {
-        Bool ret = drmmode_set_cursor(crtc);
-        if (!drmmode_crtc->cursor_up)
-            drmmode_hide_cursor(crtc);
-        drmmode_crtc->first_cursor_load_done = TRUE;
-        return ret;
-    }
+    if (drmmode_crtc->cursor_up)
+        return drmmode_set_cursor(crtc);
     return TRUE;
 }
 
@@ -835,12 +830,12 @@ drmmode_hide_cursor(xf86CrtcPtr crtc)
                      ms->cursor_width, ms->cursor_height);
 }
 
-static void
+static Bool
 drmmode_show_cursor(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
@@ -1108,7 +1103,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,
     .hide_cursor = drmmode_hide_cursor,
     .load_cursor_argb_check = drmmode_load_cursor_argb_check,
 
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h b/hw/xfree86/drivers/modesetting/drmmode_display.h
index 50976b8..3b0eb2b 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.h
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.h
@@ -93,7 +93,6 @@ typedef struct {
     struct dumb_bo *cursor_bo;
     Bool cursor_up;
     Bool set_cursor2_failed;
-    Bool first_cursor_load_done;
     uint16_t lut_r[256], lut_g[256], lut_b[256];
 
     drmmode_bo rotate_bo;
commit c02f6a687c3d6bd0727322b055ee788f8fefa005
Author: Michael Thayer <michael.thayer at oracle.com>
Date:   Thu Sep 29 20:23:12 2016 +0200

    xfree86: Immediately handle failure to set HW cursor, v5
    
    Based on v4 by Alexandre Courbot <acourbot at nvidia.com>
    
    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.
    
    v5: Updated the patch to apply to current git HEAD, split up into two
    patches (server and modesetting driver) and adjusted the code slightly
    to match surrounding code.  I also removed the new exported function
    ShowCursorCheck(), as instead just changing ShowCursor() to return Bool
    should not affect its current callers.
    
    Reviewed-by: Adam Jackson <ajax at redhat.com>
    Signed-off-by: Michael Thayer <michael.thayer at oracle.com>

diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
index 14ba9d7..215eb2a 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
@@ -993,7 +995,7 @@ static _X_INLINE _X_DEPRECATED void xf86_reload_cursors(ScreenPtr screen) {}
 /**
  * Called from EnterVT to turn the cursors back on
  */
-extern _X_EXPORT void
+extern _X_EXPORT Bool
  xf86_show_cursors(ScrnInfoPtr scrn);
 
 /**
diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c
index 1bc2b27..9543eed 100644
--- a/hw/xfree86/modes/xf86Cursors.c
+++ b/hw/xfree86/modes/xf86Cursors.c
@@ -210,9 +210,15 @@ set_bit(CARD8 *image, xf86CursorInfoPtr cursor_info, int x, int y, Bool mask)
 
 /*
  * Wrappers to deal with API compatibility with drivers that don't expose
- * load_cursor_*_check
+ * *_cursor_*_check
  */
 static inline Bool
+xf86_driver_has_show_cursor(xf86CrtcPtr crtc)
+{
+    return crtc->funcs->show_cursor_check || crtc->funcs->show_cursor;
+}
+
+static inline Bool
 xf86_driver_has_load_cursor_image(xf86CrtcPtr crtc)
 {
     return crtc->funcs->load_cursor_image_check || crtc->funcs->load_cursor_image;
@@ -225,6 +231,15 @@ xf86_driver_has_load_cursor_argb(xf86CrtcPtr crtc)
 }
 
 static inline Bool
+xf86_driver_show_cursor(xf86CrtcPtr crtc)
+{
+    if (crtc->funcs->show_cursor_check)
+        return crtc->funcs->show_cursor_check(crtc);
+    crtc->funcs->show_cursor(crtc);
+    return TRUE;
+}
+
+static inline Bool
 xf86_driver_load_cursor_image(xf86CrtcPtr crtc, CARD8 *cursor_image)
 {
     if (crtc->funcs->load_cursor_image_check)
@@ -333,16 +348,19 @@ 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)
+        crtc->cursor_shown = xf86_driver_show_cursor(crtc);
+
+    return crtc->cursor_shown;
 }
 
-void
+Bool
 xf86_show_cursors(ScrnInfoPtr scrn)
 {
     xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
@@ -352,9 +370,11 @@ 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;
 }
 
 static void
@@ -653,7 +673,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;
     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 320ec0c..11a03b6 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);
 
@@ -41,6 +42,21 @@ xf86DriverLoadCursorImage(xf86CursorInfoPtr infoPtr, unsigned char *bits)
 }
 
 static inline Bool
+xf86DriverHasShowCursor(xf86CursorInfoPtr infoPtr)
+{
+    return infoPtr->ShowCursorCheck || infoPtr->ShowCursor;
+}
+
+static inline Bool
+xf86DriverShowCursor(xf86CursorInfoPtr infoPtr)
+{
+    if(infoPtr->ShowCursorCheck)
+        return infoPtr->ShowCursorCheck(infoPtr->pScrn);
+    infoPtr->ShowCursor(infoPtr->pScrn);
+    return TRUE;
+}
+
+static inline Bool
 xf86DriverHasLoadCursorARGB(xf86CursorInfoPtr infoPtr)
 {
     return infoPtr->LoadCursorARGBCheck || infoPtr->LoadCursorARGB;
diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
index 7043a9c..2e4c9e5 100644
--- a/hw/xfree86/ramdac/xf86HWCurs.c
+++ b/hw/xfree86/ramdac/xf86HWCurs.c
@@ -93,7 +93,8 @@ xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr)
     if (!infoPtr->SetCursorPosition ||
         !xf86DriverHasLoadCursorImage(infoPtr) ||
         !infoPtr->HideCursor ||
-        !infoPtr->ShowCursor || !infoPtr->SetCursorColors)
+        !xf86DriverHasShowCursor(infoPtr) ||
+        !infoPtr->SetCursorColors)
         return FALSE;
 
     if (infoPtr->RealizeCursor) {
@@ -224,8 +225,7 @@ xf86ScreenSetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
 
     (*infoPtr->SetCursorPosition) (infoPtr->pScrn, x, y);
 
-    (*infoPtr->ShowCursor) (infoPtr->pScrn);
-    return TRUE;
+    return xf86DriverShowCursor(infoPtr);
 }
 
 Bool
@@ -287,7 +287,7 @@ xf86SetTransparentCursor(ScreenPtr pScreen)
         xf86DriverLoadCursorImage (infoPtr,
                                    ScreenPriv->transparentData);
 
-    (*infoPtr->ShowCursor) (infoPtr->pScrn);
+    xf86DriverShowCursor(infoPtr);
 
     input_unlock();
 }


More information about the xorg-commit mailing list