[PATCH xserver] xfree86: Immediately handle failure to set HW cursor
Michael Thayer
michael.thayer at oracle.com
Wed Mar 23 09:14:22 UTC 2016
Hello Alexander,
Looks good at first quick glance (I am away from the computer a bit over
the Easter weeks, I will take another look when I am back). One thing I
can say immediately though: when I submitted similar patches a while
back, Keith had the idea of keeping the old hooks untouched and adding
in the additional "checked" variants so that existing driver code does
not have to be changed to keep working. The other thing is that this
might need a bump of ABI_XINPUT_VERSION.
Regards,
Michael
On 23.03.2016 05:21, Alexandre Courbot wrote:
> There is currently no reliable way to report failure to set a HW
> cursor. Still such failures can happen if e.g. the MODE_CURSOR DRM
> ioctl fails (which currently happens at least with modesetting on Tegra
> for format incompatibility reasons).
>
> As failures are currently handled by setting the HW cursor size to
> (0,0), the fallback to SW cursor will not happen until the next time the
> cursor changes and xf86CursorSetCursor() is called. In the meantime, the
> cursor will be invisible to the user.
>
> This patch addresses this by making the _xf86CrtcFuncs::set_cursor and
> _xf86CursorInfoRec::ShowCursor hooks return a boolean. This allows to
> propagate errors up to xf86CursorSetCursor(), which makes it fall back
> to using the SW cursor immediately.
>
> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
> ---
> hw/xfree86/drivers/modesetting/drmmode_display.c | 11 +++++++----
> hw/xfree86/modes/xf86Crtc.h | 4 ++--
> hw/xfree86/modes/xf86Cursors.c | 18 ++++++++++--------
> hw/xfree86/ramdac/IBM.c | 8 ++++++--
> hw/xfree86/ramdac/TI.c | 4 +++-
> hw/xfree86/ramdac/xf86Cursor.h | 2 +-
> hw/xfree86/ramdac/xf86HWCurs.c | 3 +--
> 7 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index f573a27f4985..1ff9ed477624 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -532,7 +532,7 @@ drmmode_set_cursor_position(xf86CrtcPtr crtc, int x, int y)
> drmModeMoveCursor(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id, x, y);
> }
>
> -static void
> +static Bool
> drmmode_set_cursor(xf86CrtcPtr crtc)
> {
> drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> @@ -551,7 +551,7 @@ drmmode_set_cursor(xf86CrtcPtr crtc)
> handle, ms->cursor_width, ms->cursor_height,
> cursor->bits->xhot, cursor->bits->yhot);
> if (!ret)
> - return;
> + return TRUE;
> if (ret == -EINVAL)
> use_set_cursor2 = FALSE;
> }
> @@ -566,7 +566,10 @@ drmmode_set_cursor(xf86CrtcPtr crtc)
> cursor_info->MaxWidth = cursor_info->MaxHeight = 0;
> drmmode_crtc->drmmode->sw_cursor = TRUE;
> /* fallback to swcursor */
> + return FALSE;
> }
> +
> + return TRUE;
> }
>
> static void
> @@ -599,12 +602,12 @@ drmmode_hide_cursor(xf86CrtcPtr crtc)
> ms->cursor_width, ms->cursor_height);
> }
>
> -static void
> +static Bool
> drmmode_show_cursor(xf86CrtcPtr crtc)
> {
> drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> drmmode_crtc->cursor_up = TRUE;
> - drmmode_set_cursor(crtc);
> + return drmmode_set_cursor(crtc);
> }
>
> static void
> diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
> index 8b0160845a02..a81110c2b5c5 100644
> --- a/hw/xfree86/modes/xf86Crtc.h
> +++ b/hw/xfree86/modes/xf86Crtc.h
> @@ -185,7 +185,7 @@ typedef struct _xf86CrtcFuncs {
> /**
> * Show cursor
> */
> - void
> + Bool
> (*show_cursor) (xf86CrtcPtr crtc);
>
> /**
> @@ -980,7 +980,7 @@ extern _X_EXPORT void
> /**
> * Called from EnterVT to turn the cursors back on
> */
> -extern _X_EXPORT void
> +extern _X_EXPORT Bool
> xf86_show_cursors(ScrnInfoPtr scrn);
>
> /**
> diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c
> index 5df1ab73a37e..e9ae713e7bba 100644
> --- a/hw/xfree86/modes/xf86Cursors.c
> +++ b/hw/xfree86/modes/xf86Cursors.c
> @@ -333,16 +333,16 @@ xf86_hide_cursors(ScrnInfoPtr scrn)
> }
> }
>
> -static void
> +static Bool
> xf86_crtc_show_cursor(xf86CrtcPtr crtc)
> {
> - if (!crtc->cursor_shown && crtc->cursor_in_range) {
> - crtc->funcs->show_cursor(crtc);
> - crtc->cursor_shown = TRUE;
> - }
> + if (!crtc->cursor_shown && crtc->cursor_in_range)
> + crtc->cursor_shown = crtc->funcs->show_cursor(crtc);
> +
> + return crtc->cursor_shown;
> }
>
> -void
> +Bool
> xf86_show_cursors(ScrnInfoPtr scrn)
> {
> xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
> @@ -352,9 +352,11 @@ xf86_show_cursors(ScrnInfoPtr scrn)
> for (c = 0; c < xf86_config->num_crtc; c++) {
> xf86CrtcPtr crtc = xf86_config->crtc[c];
>
> - if (crtc->enabled)
> - xf86_crtc_show_cursor(crtc);
> + if (crtc->enabled && !xf86_crtc_show_cursor(crtc))
> + return FALSE;
> }
> +
> + return TRUE;
> }
>
> void
> diff --git a/hw/xfree86/ramdac/IBM.c b/hw/xfree86/ramdac/IBM.c
> index 6822be5f1dad..23fb55411b1e 100644
> --- a/hw/xfree86/ramdac/IBM.c
> +++ b/hw/xfree86/ramdac/IBM.c
> @@ -468,16 +468,18 @@ IBMramdac640SetBpp(ScrnInfoPtr pScrn, RamDacRegRecPtr ramdacReg)
> }
> }
>
> -static void
> +static Bool
> IBMramdac526ShowCursor(ScrnInfoPtr pScrn)
> {
> RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
>
> /* Enable cursor - X11 mode */
> (*ramdacPtr->WriteDAC) (pScrn, IBMRGB_curs, 0x00, 0x07);
> +
> + return TRUE;
> }
>
> -static void
> +static Bool
> IBMramdac640ShowCursor(ScrnInfoPtr pScrn)
> {
> RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
> @@ -485,6 +487,8 @@ IBMramdac640ShowCursor(ScrnInfoPtr pScrn)
> /* Enable cursor - mode2 (x11 mode) */
> (*ramdacPtr->WriteDAC) (pScrn, RGB640_CURSOR_CONTROL, 0x00, 0x0B);
> (*ramdacPtr->WriteDAC) (pScrn, RGB640_CROSSHAIR_CONTROL, 0x00, 0x00);
> +
> + return TRUE;
> }
>
> static void
> diff --git a/hw/xfree86/ramdac/TI.c b/hw/xfree86/ramdac/TI.c
> index 0ae1db57ecc6..d1a617de206e 100644
> --- a/hw/xfree86/ramdac/TI.c
> +++ b/hw/xfree86/ramdac/TI.c
> @@ -588,13 +588,15 @@ TIramdac3030SetBpp(ScrnInfoPtr pScrn, RamDacRegRecPtr ramdacReg)
> }
> }
>
> -static void
> +static Bool
> TIramdacShowCursor(ScrnInfoPtr pScrn)
> {
> RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
>
> /* Enable cursor - X11 mode */
> (*ramdacPtr->WriteDAC) (pScrn, TIDAC_ind_curs_ctrl, 0, 0x03);
> +
> + return TRUE;
> }
>
> static void
> diff --git a/hw/xfree86/ramdac/xf86Cursor.h b/hw/xfree86/ramdac/xf86Cursor.h
> index 6e88240d9ab7..6da75e2f6731 100644
> --- a/hw/xfree86/ramdac/xf86Cursor.h
> +++ b/hw/xfree86/ramdac/xf86Cursor.h
> @@ -15,7 +15,7 @@ typedef struct _xf86CursorInfoRec {
> void (*LoadCursorImage) (ScrnInfoPtr pScrn, unsigned char *bits);
> Bool (*LoadCursorImageCheck) (ScrnInfoPtr pScrn, unsigned char *bits);
> void (*HideCursor) (ScrnInfoPtr pScrn);
> - void (*ShowCursor) (ScrnInfoPtr pScrn);
> + Bool (*ShowCursor) (ScrnInfoPtr pScrn);
> unsigned char *(*RealizeCursor) (struct _xf86CursorInfoRec *, CursorPtr);
> Bool (*UseHWCursor) (ScreenPtr, CursorPtr);
>
> diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
> index 84febe0df5cd..a6efaf5dae80 100644
> --- a/hw/xfree86/ramdac/xf86HWCurs.c
> +++ b/hw/xfree86/ramdac/xf86HWCurs.c
> @@ -161,8 +161,7 @@ xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
>
> (*infoPtr->SetCursorPosition) (infoPtr->pScrn, x, y);
>
> - (*infoPtr->ShowCursor) (infoPtr->pScrn);
> - return TRUE;
> + return (*infoPtr->ShowCursor) (infoPtr->pScrn);
> }
>
> void
>
--
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
More information about the xorg-devel
mailing list