[PATCH xserver 1/3] xfree86: Immediately handle failure to set HW cursor, v5
Michael Thayer
michael.thayer at oracle.com
Mon Dec 5 09:25:05 UTC 2016
Hello Hans,
Polite ping on this one.
Regards and thanks
Michael
01.10.2016 12:01, Hans de Goede wrote:
> 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
>>
--
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