[PATCH xserver 1/3] xfree86: Immediately handle failure to set HW cursor, v5
Hans de Goede
hdegoede at redhat.com
Sat Oct 1 10:01:42 UTC 2016
Hi,
On 30-09-16 17:55, Michael Thayer wrote:
> Based on v4 by Alexandre Courbot <acourbot at nvidia.com>
>
> 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 again. In the
> meantime, the cursor will be invisible to the user.
>
> This patch addresses that by adding _xf86CrtcFuncs::set_cursor_check and
> _xf86CursorInfoRec::ShowCursorCheck hook variants that return booleans.
> This allows to propagate errors up to xf86CursorSetCursor(), which can
> then fall back to using the SW cursor immediately.
>
> v5: Updated the patch to apply to current git HEAD, split up into two
> patches (server and modesetting driver) and adjusted the code slightly
> to match surrounding code. I also removed the new exported function
> ShowCursorCheck(), as instead just changing ShowCursor() to return Bool
> should not affect its current callers.
>
> Signed-off-by: Michael Thayer <michael.thayer at oracle.com>
Series looks good to me and is:
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
As discussed this will need to wait till we start the 1.20
cycle before it can be merged (it contains a video driver ABI change)
Regards,
Hans
> ---
> hw/xfree86/modes/xf86Crtc.h | 4 +++-
> hw/xfree86/modes/xf86Cursors.c | 40 ++++++++++++++++++++++++++++++----------
> hw/xfree86/ramdac/xf86Cursor.h | 16 ++++++++++++++++
> hw/xfree86/ramdac/xf86HWCurs.c | 8 ++++----
> 4 files changed, 53 insertions(+), 15 deletions(-)
>
> diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
> index 14ba9d7..215eb2a 100644
> --- a/hw/xfree86/modes/xf86Crtc.h
> +++ b/hw/xfree86/modes/xf86Crtc.h
> @@ -195,6 +195,8 @@ typedef struct _xf86CrtcFuncs {
> */
> void
> (*show_cursor) (xf86CrtcPtr crtc);
> + Bool
> + (*show_cursor_check) (xf86CrtcPtr crtc);
>
> /**
> * Hide cursor
> @@ -993,7 +995,7 @@ static _X_INLINE _X_DEPRECATED void xf86_reload_cursors(ScreenPtr screen) {}
> /**
> * 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 1bc2b27..9543eed 100644
> --- a/hw/xfree86/modes/xf86Cursors.c
> +++ b/hw/xfree86/modes/xf86Cursors.c
> @@ -210,9 +210,15 @@ set_bit(CARD8 *image, xf86CursorInfoPtr cursor_info, int x, int y, Bool mask)
>
> /*
> * Wrappers to deal with API compatibility with drivers that don't expose
> - * load_cursor_*_check
> + * *_cursor_*_check
> */
> static inline Bool
> +xf86_driver_has_show_cursor(xf86CrtcPtr crtc)
> +{
> + return crtc->funcs->show_cursor_check || crtc->funcs->show_cursor;
> +}
> +
> +static inline Bool
> xf86_driver_has_load_cursor_image(xf86CrtcPtr crtc)
> {
> return crtc->funcs->load_cursor_image_check || crtc->funcs->load_cursor_image;
> @@ -225,6 +231,15 @@ xf86_driver_has_load_cursor_argb(xf86CrtcPtr crtc)
> }
>
> static inline Bool
> +xf86_driver_show_cursor(xf86CrtcPtr crtc)
> +{
> + if (crtc->funcs->show_cursor_check)
> + return crtc->funcs->show_cursor_check(crtc);
> + crtc->funcs->show_cursor(crtc);
> + return TRUE;
> +}
> +
> +static inline Bool
> xf86_driver_load_cursor_image(xf86CrtcPtr crtc, CARD8 *cursor_image)
> {
> if (crtc->funcs->load_cursor_image_check)
> @@ -333,16 +348,19 @@ 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_in_range)
> + return TRUE;
> +
> + if (!crtc->cursor_shown)
> + crtc->cursor_shown = xf86_driver_show_cursor(crtc);
> +
> + return crtc->cursor_shown;
> }
>
> -void
> +Bool
> xf86_show_cursors(ScrnInfoPtr scrn)
> {
> xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
> @@ -352,9 +370,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;
> }
>
> static void
> @@ -653,7 +673,7 @@ xf86_cursors_init(ScreenPtr screen, int max_width, int max_height, int flags)
> cursor_info->SetCursorPosition = xf86_set_cursor_position;
> cursor_info->LoadCursorImageCheck = xf86_load_cursor_image;
> cursor_info->HideCursor = xf86_hide_cursors;
> - cursor_info->ShowCursor = xf86_show_cursors;
> + cursor_info->ShowCursorCheck = xf86_show_cursors;
> cursor_info->UseHWCursor = xf86_use_hw_cursor;
> if (flags & HARDWARE_CURSOR_ARGB) {
> cursor_info->UseHWCursorARGB = xf86_use_hw_cursor_argb;
> diff --git a/hw/xfree86/ramdac/xf86Cursor.h b/hw/xfree86/ramdac/xf86Cursor.h
> index 320ec0c..11a03b6 100644
> --- a/hw/xfree86/ramdac/xf86Cursor.h
> +++ b/hw/xfree86/ramdac/xf86Cursor.h
> @@ -16,6 +16,7 @@ typedef struct _xf86CursorInfoRec {
> Bool (*LoadCursorImageCheck) (ScrnInfoPtr pScrn, unsigned char *bits);
> void (*HideCursor) (ScrnInfoPtr pScrn);
> void (*ShowCursor) (ScrnInfoPtr pScrn);
> + Bool (*ShowCursorCheck) (ScrnInfoPtr pScrn);
> unsigned char *(*RealizeCursor) (struct _xf86CursorInfoRec *, CursorPtr);
> Bool (*UseHWCursor) (ScreenPtr, CursorPtr);
>
> @@ -41,6 +42,21 @@ xf86DriverLoadCursorImage(xf86CursorInfoPtr infoPtr, unsigned char *bits)
> }
>
> static inline Bool
> +xf86DriverHasShowCursor(xf86CursorInfoPtr infoPtr)
> +{
> + return infoPtr->ShowCursorCheck || infoPtr->ShowCursor;
> +}
> +
> +static inline Bool
> +xf86DriverShowCursor(xf86CursorInfoPtr infoPtr)
> +{
> + if(infoPtr->ShowCursorCheck)
> + return infoPtr->ShowCursorCheck(infoPtr->pScrn);
> + infoPtr->ShowCursor(infoPtr->pScrn);
> + return TRUE;
> +}
> +
> +static inline Bool
> xf86DriverHasLoadCursorARGB(xf86CursorInfoPtr infoPtr)
> {
> return infoPtr->LoadCursorARGBCheck || infoPtr->LoadCursorARGB;
> diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
> index e8966ed..bd287fc 100644
> --- a/hw/xfree86/ramdac/xf86HWCurs.c
> +++ b/hw/xfree86/ramdac/xf86HWCurs.c
> @@ -90,7 +90,8 @@ xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr)
> if (!infoPtr->SetCursorPosition ||
> !xf86DriverHasLoadCursorImage(infoPtr) ||
> !infoPtr->HideCursor ||
> - !infoPtr->ShowCursor || !infoPtr->SetCursorColors)
> + !xf86DriverHasShowCursor(infoPtr) ||
> + !infoPtr->SetCursorColors)
> return FALSE;
>
> if (infoPtr->RealizeCursor) {
> @@ -204,8 +205,7 @@ xf86ScreenSetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
>
> (*infoPtr->SetCursorPosition) (infoPtr->pScrn, x, y);
>
> - (*infoPtr->ShowCursor) (infoPtr->pScrn);
> - return TRUE;
> + return xf86DriverShowCursor(infoPtr);
> }
>
> Bool
> @@ -252,7 +252,7 @@ xf86SetTransparentCursor(ScreenPtr pScreen)
> xf86DriverLoadCursorImage (infoPtr,
> ScreenPriv->transparentData);
>
> - (*infoPtr->ShowCursor) (infoPtr->pScrn);
> + xf86DriverShowCursor(infoPtr);
> }
>
> static void
>
More information about the xorg-devel
mailing list