Hi,
On 14-09-16 15:53, Michael Thayer wrote:
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.
I advise you to (directly) mail the author of that commit
perhaps he can explain things.
> 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.
IIRC then during the review it was brought up that there
might be other errors to, e.g. I've seen checks for
-ENOSYS for other ioctls in some Xorg code. So I think
it is probably safest to just always try the fallback.
Regards,
Hans
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];
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
