Re: [PATCH xserver] modesetting: only fall back to drmModeSetCursor() on -EINVAL

[Hans de Goede]

Hi All,

On 09/16/2016 06:51 PM, Michael Thayer wrote:

This change effectively reverts commit 074cf58.  We were falling back from
drmModeSetCursor2() to drmModeSetCursor() whenever the first failed.  This
fall-back only makes sense on pre-mid-2013 kernels which implemented the
cursor_set hook but not cursor_set2, and in this case the call to
drmModeSetCursor2() will always return -EINVAL.  Specifically, a return
value of -ENXIO usually means that neither are supported.

Signed-off-by: Michael Thayer 


LGTM: Reviewed-by: Hans de Goede 

Regards,

Hans



---
1) Tested that hardware cursors still work with the patch applied.
2) Tested in gdb that the expected path is taken when drmModeSetCursor2()
   returns -EINVAL.

 hw/xfree86/drivers/modesetting/drmmode_display.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 6b933e4..41810d9 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -768,11 +768,15 @@ drmmode_set_cursor(xf86CrtcPtr crtc)
 if (!ret)
 return TRUE;

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

 if (ret) {
 xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);


___
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

[PATCH xserver] modesetting: only fall back to drmModeSetCursor() on -EINVAL

[Michael Thayer]

This change effectively reverts commit 074cf58.  We were falling back from
drmModeSetCursor2() to drmModeSetCursor() whenever the first failed.  This
fall-back only makes sense on pre-mid-2013 kernels which implemented the
cursor_set hook but not cursor_set2, and in this case the call to
drmModeSetCursor2() will always return -EINVAL.  Specifically, a return
value of -ENXIO usually means that neither are supported.

Signed-off-by: Michael Thayer 
---
1) Tested that hardware cursors still work with the patch applied.
2) Tested in gdb that the expected path is taken when drmModeSetCursor2()
   returns -EINVAL.

 hw/xfree86/drivers/modesetting/drmmode_display.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c 
b/hw/xfree86/drivers/modesetting/drmmode_display.c
index 6b933e4..41810d9 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.c
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
@@ -768,11 +768,15 @@ drmmode_set_cursor(xf86CrtcPtr crtc)
 if (!ret)
 return TRUE;
 
-drmmode_crtc->set_cursor2_failed = 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 = drmModeSetCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id, 
handle,
-   ms->cursor_width, ms->cursor_height);
+if (ret == -EINVAL)
+ret = drmModeSetCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
+   handle, ms->cursor_width, ms->cursor_height);
 
 if (ret) {
 xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
-- 
2.9.3

___
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