[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 <michael.thayer at oracle.com>

LGTM: Reviewed-by: Hans de Goede <hdegoede at redhat.com>

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);
>