On 14.09.2016 12:07, Hans de Goede wrote:
Hi,

On 13-09-16 17:42, Michael Thayer wrote:
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 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 harware and others not, or virtual
hardware may be able to handle hardware cursors at some times and not
others.

Signed-off-by: Michael Thayer <michael.tha...@oracle.com>
---
Checked the current git source and this change is still needed.  This
is an
updated patch which takes into account changes in the driver source since
the original patch was created.  Note that other than building I have not
had a chance to test that the updated patch still works, but the
difference
against the original is pretty minimal.

 hw/xfree86/drivers/modesetting/drmmode_display.c | 36
+++++++++---------------
 hw/xfree86/drivers/modesetting/drmmode_display.h |  1 -
 2 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 6b933e4..ad39f76 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -756,33 +756,23 @@ 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;

-    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;
-
-        drmmode_crtc->set_cursor2_failed = TRUE;
-    }
-
-    ret = drmModeSetCursor(drmmode->fd,
drmmode_crtc->mode_crtc->crtc_id, handle,
-                           ms->cursor_width, ms->cursor_height);
+    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;

-    if (ret) {
-        xf86CrtcConfigPtr xf86_config =
XF86_CRTC_CONFIG_PTR(crtc->scrn);
-        xf86CursorInfoPtr cursor_info = xf86_config->cursor_info;
+    /* -EINVAL can mean bad cursor parameters or Cursor2 API not
supported. */
+    if (ret == -EINVAL)

You're reintroducing the -EINVAL check which was deliberately dropped
recently because sometimes another error is given while the non 2 version
might still work, please drop this bit.

Also please actually test the patch before posting a new version.

I did some testing which did indeed show a problem with the patch (not sure if the original version had this problem, but I certainly did not hit it when I tested it): if the cursor is currently hidden then the cursor set is silently skipped. There is some logic added in af91647 to work around this once. This logic seems unnecessary to me, since all successful calls to set the cursor in the server code are immediately followed up by a call to show the cursor (see xf86HWCurs.c), and failed calls should result in the cursor being hidden anyway before the software cursor is enabled. Updated (tested) patch to follow.

Thanks and regards,

Michael

Other then that this looks like an ok change to me.

Regards,

Hans



+        ret = drmModeSetCursor(drmmode->fd,
drmmode_crtc->mode_crtc->crtc_id, handle,
+                               ms->cursor_width, ms->cursor_height);

-        cursor_info->MaxWidth = cursor_info->MaxHeight = 0;
-        drmmode_crtc->drmmode->sw_cursor = TRUE;
-        /* fallback to swcursor */
-        return FALSE;
-    }
+    if (ret)
+        return FALSE; /* fallback to swcursor */
     return TRUE;
 }

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h
b/hw/xfree86/drivers/modesetting/drmmode_display.h
index 50976b8..5170bf5 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;
     Bool first_cursor_load_done;
     uint16_t lut_r[256], lut_g[256], lut_b[256];



--
Michael Thayer | VirtualBox engineer
ORACLE Deutschland B.V. & Co. KG | Werkstr. 24 | D-71384 Weinstadt

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstraße 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to