[PATCH] hw/xfree86: Restore API compatibility for cursor loading functions
Hans de Goede
hdegoede at redhat.com
Fri Apr 25 10:05:17 PDT 2014
Hi All,
On 04/25/2014 06:54 PM, Keith Packard wrote:
> Create load_cursor_image_check, load_cursor_argb_check,
> LoadCursorImageCheck and LoadCursorARGBCheck that can return failure
> and use them in preference to the old unchecked variants.
>
> Signed-off-by: Keith Packard <keithp at keithp.com>
> ---
>
> On IRC this morning, Hans and I thought this might provide the best
> compatibility story -- existing driver sources should work with only a
> recompile, which the existing ABI version checks will enforce. New
> drivers can provide the newer entry point where necessary and the
> server will choose the _check versions over the old ones when both are
> available.
>
> I've tested this with the upstream intel driver sources (without the
> return value change) and it works correctly
Thanks for working on this. Looks good:
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Regards,
Hans
>
> hw/xfree86/modes/xf86Crtc.h | 8 ++++--
> hw/xfree86/modes/xf86Cursors.c | 56 +++++++++++++++++++++++++++++++++---------
> hw/xfree86/ramdac/IBM.c | 4 +--
> hw/xfree86/ramdac/TI.c | 2 +-
> hw/xfree86/ramdac/xf86Cursor.h | 36 +++++++++++++++++++++++++--
> hw/xfree86/ramdac/xf86HWCurs.c | 14 +++++------
> 6 files changed, 95 insertions(+), 25 deletions(-)
>
> diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
> index 5407deb..eebe6f4 100644
> --- a/hw/xfree86/modes/xf86Crtc.h
> +++ b/hw/xfree86/modes/xf86Crtc.h
> @@ -186,14 +186,18 @@ typedef struct _xf86CrtcFuncs {
> /**
> * Load monochrome image
> */
> - Bool
> + void
> (*load_cursor_image) (xf86CrtcPtr crtc, CARD8 *image);
> + Bool
> + (*load_cursor_image_check) (xf86CrtcPtr crtc, CARD8 *image);
>
> /**
> * Load ARGB image
> */
> - Bool
> + void
> (*load_cursor_argb) (xf86CrtcPtr crtc, CARD32 *image);
> + Bool
> + (*load_cursor_argb_check) (xf86CrtcPtr crtc, CARD32 *image);
>
> /**
> * Clean up driver-specific bits of the crtc
> diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c
> index 10ef6f6..379a27a 100644
> --- a/hw/xfree86/modes/xf86Cursors.c
> +++ b/hw/xfree86/modes/xf86Cursors.c
> @@ -209,6 +209,40 @@ 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
> + */
> +static inline Bool
> +xf86_driver_has_load_cursor_image(xf86CrtcPtr crtc)
> +{
> + return crtc->funcs->load_cursor_image_check || crtc->funcs->load_cursor_image;
> +}
> +
> +static inline Bool
> +xf86_driver_has_load_cursor_argb(xf86CrtcPtr crtc)
> +{
> + return crtc->funcs->load_cursor_argb_check || crtc->funcs->load_cursor_argb;
> +}
> +
> +static inline Bool
> +xf86_driver_load_cursor_image(xf86CrtcPtr crtc, CARD8 *cursor_image)
> +{
> + if (crtc->funcs->load_cursor_image_check)
> + return crtc->funcs->load_cursor_image_check(crtc, cursor_image);
> + crtc->funcs->load_cursor_image(crtc, cursor_image);
> + return TRUE;
> +}
> +
> +static inline Bool
> +xf86_driver_load_cursor_argb(xf86CrtcPtr crtc, CARD32 *cursor_argb)
> +{
> + if (crtc->funcs->load_cursor_argb_check)
> + return crtc->funcs->load_cursor_argb_check(crtc, cursor_argb);
> + crtc->funcs->load_cursor_argb(crtc, cursor_argb);
> + return TRUE;
> +}
> +
> +/*
> * Load a two color cursor into a driver that supports only ARGB cursors
> */
> static Bool
> @@ -244,7 +278,7 @@ xf86_crtc_convert_cursor_to_argb(xf86CrtcPtr crtc, unsigned char *src)
> bits = 0;
> cursor_image[y * cursor_info->MaxWidth + x] = bits;
> }
> - return crtc->funcs->load_cursor_argb(crtc, cursor_image);
> + return xf86_driver_load_cursor_argb(crtc, cursor_image);
> }
>
> /*
> @@ -269,7 +303,7 @@ xf86_set_cursor_colors(ScrnInfoPtr scrn, int bg, int fg)
> xf86CrtcPtr crtc = xf86_config->crtc[c];
>
> if (crtc->enabled && !crtc->cursor_argb) {
> - if (crtc->funcs->load_cursor_image)
> + if (xf86_driver_has_load_cursor_image(crtc))
> crtc->funcs->set_cursor_colors(crtc, bg, fg);
> else if (bits)
> xf86_crtc_convert_cursor_to_argb(crtc, bits);
> @@ -450,7 +484,7 @@ xf86_crtc_load_cursor_image(xf86CrtcPtr crtc, CARD8 *src)
> set_bit(cursor_image, cursor_info, x, y, TRUE);
> }
> }
> - return crtc->funcs->load_cursor_image(crtc, cursor_image);
> + return xf86_driver_load_cursor_image(crtc, cursor_image);
> }
>
> /*
> @@ -466,10 +500,10 @@ xf86_load_cursor_image(ScrnInfoPtr scrn, unsigned char *src)
> xf86CrtcPtr crtc = xf86_config->crtc[c];
>
> if (crtc->enabled) {
> - if (crtc->funcs->load_cursor_image) {
> + if (xf86_driver_has_load_cursor_image(crtc)) {
> if (!xf86_crtc_load_cursor_image(crtc, src))
> return FALSE;
> - } else if (crtc->funcs->load_cursor_argb) {
> + } else if (xf86_driver_has_load_cursor_argb(crtc)) {
> if (!xf86_crtc_convert_cursor_to_argb(crtc, src))
> return FALSE;
> } else
> @@ -549,7 +583,7 @@ xf86_crtc_load_cursor_argb(xf86CrtcPtr crtc, CursorPtr cursor)
> cursor_image[y * image_width + x] = bits;
> }
>
> - return crtc->funcs->load_cursor_argb(crtc, cursor_image);
> + return xf86_driver_load_cursor_argb(crtc, cursor_image);
> }
>
> static Bool
> @@ -594,14 +628,14 @@ xf86_cursors_init(ScreenPtr screen, int max_width, int max_height, int flags)
>
> cursor_info->SetCursorColors = xf86_set_cursor_colors;
> cursor_info->SetCursorPosition = xf86_set_cursor_position;
> - cursor_info->LoadCursorImage = xf86_load_cursor_image;
> + cursor_info->LoadCursorImageCheck = xf86_load_cursor_image;
> cursor_info->HideCursor = xf86_hide_cursors;
> cursor_info->ShowCursor = xf86_show_cursors;
> cursor_info->UseHWCursor = xf86_use_hw_cursor;
> #ifdef ARGB_CURSOR
> if (flags & HARDWARE_CURSOR_ARGB) {
> cursor_info->UseHWCursorARGB = xf86_use_hw_cursor_argb;
> - cursor_info->LoadCursorARGB = xf86_load_cursor_argb;
> + cursor_info->LoadCursorARGBCheck = xf86_load_cursor_argb;
> }
> #endif
>
> @@ -658,11 +692,11 @@ xf86_reload_cursors(ScreenPtr screen)
> dixLookupScreenPrivate(&cursor->devPrivates, CursorScreenKey,
> screen);
> #ifdef ARGB_CURSOR
> - if (cursor->bits->argb && cursor_info->LoadCursorARGB)
> - (*cursor_info->LoadCursorARGB) (scrn, cursor);
> + if (cursor->bits->argb && xf86DriverHasLoadCursorARGB(cursor_info))
> + xf86DriverLoadCursorARGB(cursor_info, cursor);
> else if (src)
> #endif
> - (*cursor_info->LoadCursorImage) (scrn, src);
> + xf86DriverLoadCursorImage(cursor_info, src);
>
> x += scrn->frameX0 + cursor_screen_priv->HotX;
> y += scrn->frameY0 + cursor_screen_priv->HotY;
> diff --git a/hw/xfree86/ramdac/IBM.c b/hw/xfree86/ramdac/IBM.c
> index 872d3d4..45876cf 100644
> --- a/hw/xfree86/ramdac/IBM.c
> +++ b/hw/xfree86/ramdac/IBM.c
> @@ -622,7 +622,7 @@ IBMramdac526HWCursorInit(xf86CursorInfoPtr infoPtr)
> HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_1;
> infoPtr->SetCursorColors = IBMramdac526SetCursorColors;
> infoPtr->SetCursorPosition = IBMramdac526SetCursorPosition;
> - infoPtr->LoadCursorImage = IBMramdac526LoadCursorImage;
> + infoPtr->LoadCursorImageCheck = IBMramdac526LoadCursorImage;
> infoPtr->HideCursor = IBMramdac526HideCursor;
> infoPtr->ShowCursor = IBMramdac526ShowCursor;
> infoPtr->UseHWCursor = IBMramdac526UseHWCursor;
> @@ -638,7 +638,7 @@ IBMramdac640HWCursorInit(xf86CursorInfoPtr infoPtr)
> HARDWARE_CURSOR_SOURCE_MASK_INTERLEAVE_1;
> infoPtr->SetCursorColors = IBMramdac640SetCursorColors;
> infoPtr->SetCursorPosition = IBMramdac640SetCursorPosition;
> - infoPtr->LoadCursorImage = IBMramdac640LoadCursorImage;
> + infoPtr->LoadCursorImageCheck = IBMramdac640LoadCursorImage;
> infoPtr->HideCursor = IBMramdac640HideCursor;
> infoPtr->ShowCursor = IBMramdac640ShowCursor;
> infoPtr->UseHWCursor = IBMramdac640UseHWCursor;
> diff --git a/hw/xfree86/ramdac/TI.c b/hw/xfree86/ramdac/TI.c
> index 7d4e0d7..2492bb5 100644
> --- a/hw/xfree86/ramdac/TI.c
> +++ b/hw/xfree86/ramdac/TI.c
> @@ -676,7 +676,7 @@ TIramdacHWCursorInit(xf86CursorInfoPtr infoPtr)
> HARDWARE_CURSOR_SOURCE_MASK_NOT_INTERLEAVED;
> infoPtr->SetCursorColors = TIramdacSetCursorColors;
> infoPtr->SetCursorPosition = TIramdacSetCursorPosition;
> - infoPtr->LoadCursorImage = TIramdacLoadCursorImage;
> + infoPtr->LoadCursorImageCheck = TIramdacLoadCursorImage;
> infoPtr->HideCursor = TIramdacHideCursor;
> infoPtr->ShowCursor = TIramdacShowCursor;
> infoPtr->UseHWCursor = TIramdacUseHWCursor;
> diff --git a/hw/xfree86/ramdac/xf86Cursor.h b/hw/xfree86/ramdac/xf86Cursor.h
> index 1ecbdcd..a389a99 100644
> --- a/hw/xfree86/ramdac/xf86Cursor.h
> +++ b/hw/xfree86/ramdac/xf86Cursor.h
> @@ -12,7 +12,8 @@ typedef struct _xf86CursorInfoRec {
> int MaxHeight;
> void (*SetCursorColors) (ScrnInfoPtr pScrn, int bg, int fg);
> void (*SetCursorPosition) (ScrnInfoPtr pScrn, int x, int y);
> - Bool (*LoadCursorImage) (ScrnInfoPtr pScrn, unsigned char *bits);
> + void (*LoadCursorImage) (ScrnInfoPtr pScrn, unsigned char *bits);
> + Bool (*LoadCursorImageCheck) (ScrnInfoPtr pScrn, unsigned char *bits);
> void (*HideCursor) (ScrnInfoPtr pScrn);
> void (*ShowCursor) (ScrnInfoPtr pScrn);
> unsigned char *(*RealizeCursor) (struct _xf86CursorInfoRec *, CursorPtr);
> @@ -20,11 +21,42 @@ typedef struct _xf86CursorInfoRec {
>
> #ifdef ARGB_CURSOR
> Bool (*UseHWCursorARGB) (ScreenPtr, CursorPtr);
> - Bool (*LoadCursorARGB) (ScrnInfoPtr, CursorPtr);
> + void (*LoadCursorARGB) (ScrnInfoPtr, CursorPtr);
> + Bool (*LoadCursorARGBCheck) (ScrnInfoPtr, CursorPtr);
> #endif
>
> } xf86CursorInfoRec, *xf86CursorInfoPtr;
>
> +static inline Bool
> +xf86DriverHasLoadCursorImage(xf86CursorInfoPtr infoPtr)
> +{
> + return infoPtr->LoadCursorImageCheck || infoPtr->LoadCursorImage;
> +}
> +
> +static inline Bool
> +xf86DriverLoadCursorImage(xf86CursorInfoPtr infoPtr, unsigned char *bits)
> +{
> + if(infoPtr->LoadCursorImageCheck)
> + return infoPtr->LoadCursorImageCheck(infoPtr->pScrn, bits);
> + infoPtr->LoadCursorImage(infoPtr->pScrn, bits);
> + return TRUE;
> +}
> +
> +static inline Bool
> +xf86DriverHasLoadCursorARGB(xf86CursorInfoPtr infoPtr)
> +{
> + return infoPtr->LoadCursorARGBCheck || infoPtr->LoadCursorARGB;
> +}
> +
> +static inline Bool
> +xf86DriverLoadCursorARGB(xf86CursorInfoPtr infoPtr, CursorPtr pCursor)
> +{
> + if(infoPtr->LoadCursorARGBCheck)
> + return infoPtr->LoadCursorARGBCheck(infoPtr->pScrn, pCursor);
> + infoPtr->LoadCursorARGB(infoPtr->pScrn, pCursor);
> + return TRUE;
> +}
> +
> extern _X_EXPORT Bool xf86InitCursor(ScreenPtr pScreen,
> xf86CursorInfoPtr infoPtr);
> extern _X_EXPORT xf86CursorInfoPtr xf86CreateCursorInfoRec(void);
> diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
> index 0b5caa2..953c86a 100644
> --- a/hw/xfree86/ramdac/xf86HWCurs.c
> +++ b/hw/xfree86/ramdac/xf86HWCurs.c
> @@ -87,7 +87,7 @@ xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr)
>
> /* These are required for now */
> if (!infoPtr->SetCursorPosition ||
> - !infoPtr->LoadCursorImage ||
> + !xf86DriverHasLoadCursorImage(infoPtr) ||
> !infoPtr->HideCursor ||
> !infoPtr->ShowCursor || !infoPtr->SetCursorColors)
> return FALSE;
> @@ -140,7 +140,7 @@ xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
> y -= infoPtr->pScrn->frameY0 + ScreenPriv->HotY;
>
> #ifdef ARGB_CURSOR
> - if (!pCurs->bits->argb || !infoPtr->LoadCursorARGB)
> + if (!pCurs->bits->argb || !xf86DriverHasLoadCursorARGB(infoPtr))
> #endif
> if (!bits) {
> bits = (*infoPtr->RealizeCursor) (infoPtr, pCurs);
> @@ -152,13 +152,13 @@ xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
> (*infoPtr->HideCursor) (infoPtr->pScrn);
>
> #ifdef ARGB_CURSOR
> - if (pCurs->bits->argb && infoPtr->LoadCursorARGB) {
> - if (!(*infoPtr->LoadCursorARGB) (infoPtr->pScrn, pCurs))
> + if (pCurs->bits->argb && xf86DriverHasLoadCursorARGB(infoPtr)) {
> + if (!xf86DriverLoadCursorARGB (infoPtr, pCurs))
> return FALSE;
> } else
> #endif
> if (bits)
> - if (!(*infoPtr->LoadCursorImage) (infoPtr->pScrn, bits))
> + if (!xf86DriverLoadCursorImage (infoPtr, bits))
> return FALSE;
>
> xf86RecolorCursor(pScreen, pCurs, 1);
> @@ -185,8 +185,8 @@ xf86SetTransparentCursor(ScreenPtr pScreen)
> (*infoPtr->HideCursor) (infoPtr->pScrn);
>
> if (ScreenPriv->transparentData)
> - (*infoPtr->LoadCursorImage) (infoPtr->pScrn,
> - ScreenPriv->transparentData);
> + xf86DriverLoadCursorImage (infoPtr,
> + ScreenPriv->transparentData);
>
> (*infoPtr->ShowCursor) (infoPtr->pScrn);
> }
>
More information about the xorg-devel
mailing list