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 <[email protected]>
---
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.
I see that the commit message to 074cf587 states that sometimes ENXIO
can be returned. Sorry if I am being blind here (it would not be the
first time), but I cannot see which path that could happen in. It also
seems a bit silly to unconditionally call drmModeSetCursor() every time
drmModeSetCursor2() fails if it can be reasonably avoided. So if I am
being blind, would it be alright if I made the test (ret == -EINVAL ||
ret == ENXIO)? Not that it would bring the world to an end, but still.
Regards,
Michael
Also please actually test the patch before posting a new version.
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
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
