[PATCH 5/6] cursor: add hw cursor support for prime
Eric Anholt
eric at anholt.net
Fri Jun 19 11:05:13 PDT 2015
Dave Airlie <airlied at gmail.com> writes:
> Currently with PRIME if we detect a secondary GPU,
> we switch to using SW cursors, this isn't optimal,
> esp for the intel/nvidia combinations, we have
> no choice for the USB offload devices.
>
> This patch checks on each slave screen if hw
> cursors are enabled, and also calls set cursor
> and move cursor on all screens.
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
> dix/dispatch.c | 2 +
> hw/xfree86/ramdac/xf86Cursor.c | 12 +----
> hw/xfree86/ramdac/xf86CursorPriv.h | 1 +
> hw/xfree86/ramdac/xf86HWCurs.c | 92 ++++++++++++++++++++++++++++++++++++--
> 4 files changed, 94 insertions(+), 13 deletions(-)
>
> diff --git a/dix/dispatch.c b/dix/dispatch.c
> index 9208582..f3d0f19 100644
> --- a/dix/dispatch.c
> +++ b/dix/dispatch.c
> @@ -3920,6 +3920,8 @@ AddGPUScreen(Bool (*pfnInit) (ScreenPtr /*pScreen */ ,
>
> update_desktop_dimensions();
>
> + dixRegisterScreenPrivateKey(&cursorScreenDevPriv, pScreen, PRIVATE_CURSOR,
> + 0);
> return i;
> }
>
> diff --git a/hw/xfree86/ramdac/xf86Cursor.c b/hw/xfree86/ramdac/xf86Cursor.c
> index 2a54571..175bed7 100644
> --- a/hw/xfree86/ramdac/xf86Cursor.c
> +++ b/hw/xfree86/ramdac/xf86Cursor.c
> @@ -337,17 +337,9 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr pScreen, CursorPtr pCurs,
> return;
> }
>
> - if (infoPtr->pScrn->vtSema && xorg_list_is_empty(&pScreen->pixmap_dirty_list) &&
> + if (infoPtr->pScrn->vtSema &&
> (ScreenPriv->ForceHWCursorCount ||
> - ((
> - cursor->bits->argb &&
> - infoPtr->UseHWCursorARGB &&
> - (*infoPtr->UseHWCursorARGB)(pScreen, cursor)) ||
> - (cursor->bits->argb == 0 &&
> - (cursor->bits->height <= infoPtr->MaxHeight) &&
> - (cursor->bits->width <= infoPtr->MaxWidth) &&
> - (!infoPtr->UseHWCursor || (*infoPtr->UseHWCursor) (pScreen, cursor)))))) {
> -
> + xf86CheckHWCursor(pScreen, cursor, infoPtr))) {
> if (ScreenPriv->SWCursor) /* remove the SW cursor */
> (*ScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen,
> NullCursor, x, y);
> diff --git a/hw/xfree86/ramdac/xf86CursorPriv.h b/hw/xfree86/ramdac/xf86CursorPriv.h
> index f34c1c7..397d2a1 100644
> --- a/hw/xfree86/ramdac/xf86CursorPriv.h
> +++ b/hw/xfree86/ramdac/xf86CursorPriv.h
> @@ -43,6 +43,7 @@ void xf86MoveCursor(ScreenPtr pScreen, int x, int y);
> void xf86RecolorCursor(ScreenPtr pScreen, CursorPtr pCurs, Bool displayed);
> Bool xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr);
>
> +Bool xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr);
> extern _X_EXPORT DevPrivateKeyRec xf86CursorScreenKeyRec;
>
> #define xf86CursorScreenKey (&xf86CursorScreenKeyRec)
> diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
> index 84febe0..84d1ff7 100644
> --- a/hw/xfree86/ramdac/xf86HWCurs.c
> +++ b/hw/xfree86/ramdac/xf86HWCurs.c
> @@ -119,8 +119,52 @@ xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr)
> return TRUE;
> }
>
> +static Bool
> +xf86CursorScreenCheckHW(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr)
This might be better renamed xf86ScreenCheckHWCursor, to match how you
handled MoveCursor/SetCursor outside-loop versus screen functions.
> +{
> + return ((
> + cursor->bits->argb &&
> + infoPtr->UseHWCursorARGB &&
> + (*infoPtr->UseHWCursorARGB)(pScreen, cursor)) ||
> + (cursor->bits->argb == 0 &&
> + (cursor->bits->height <= infoPtr->MaxHeight) &&
> + (cursor->bits->width <= infoPtr->MaxWidth) &&
> + (!infoPtr->UseHWCursor || (*infoPtr->UseHWCursor) (pScreen, cursor))));
It might be nice to expand this out a little:
if (cursor->bits->argb) {
if (infoPtr->UseHWCursorARGB &&
(*infoPtr->UseHWCursorARGB)(pScreen, cursor)) {
return TRUE;
}
} else {
if (cursor->bits->height <= infoPtr->MaxHeight &&
cursor->bits->width <= infoPtr->MaxWidth &&
(!infoPtr->UseHWCursor ||
(*infoPtr->UseHWCursor) (pScreen, cursor)) {
return TRUE;
}
}
return FALSE;
but you're just moving the code around, not writing it in the first
place, so either way.
> +}
> +
> Bool
> -xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
> +xf86CheckHWCursor(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr)
> +{
> + ScreenPtr pSlave;
> + PixmapDirtyUpdatePtr ent;
> + Bool ret;
> + ret = xf86CursorScreenCheckHW(pScreen, cursor, infoPtr);
> + if (ret == FALSE)
> + return FALSE;
> +
> + if (xorg_list_is_empty(&pScreen->pixmap_dirty_list))
> + return TRUE;
The empty check here seems gratuitous, since the list_for_each just
won't do anything if it's empty, right? (same below)
> +
> + /* ask each driver consuming a pixmap if it can support HW cursor */
> + xorg_list_for_each_entry(ent, &pScreen->pixmap_dirty_list, ent) {
> + xf86CursorScreenPtr ScreenPriv;
> + xf86CursorInfoPtr slaveInfoPtr;
> +
> + pSlave = ent->slave_dst->drawable.pScreen;
> +
> + ScreenPriv = (xf86CursorScreenPtr) dixLookupPrivate(&pSlave->devPrivates,
> + xf86CursorScreenKey);
> + slaveInfoPtr = ScreenPriv->CursorInfoPtr;
> + ret = xf86CursorScreenCheckHW(pSlave, cursor, slaveInfoPtr);
> + if (ret == FALSE)
> + return FALSE;
> + }
> + return TRUE;
> +}
> +
> +
> +static Bool
> +xf86ScreenSetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
> {
> xf86CursorScreenPtr ScreenPriv =
> (xf86CursorScreenPtr) dixLookupPrivate(&pScreen->devPrivates,
> @@ -165,6 +209,30 @@ xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
> return TRUE;
> }
>
> +Bool
> +xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
> +{
> + ScreenPtr pSlave;
> + PixmapDirtyUpdatePtr ent;
> + Bool ret;
> +
> + ret = xf86ScreenSetCursor(pScreen, pCurs, x, y);
> + if (ret == FALSE)
> + return FALSE;
> +
> + if (xorg_list_is_empty(&pScreen->pixmap_dirty_list))
> + return TRUE;
> +
> + /* ask each driver consuming a pixmap if it can support HW cursor */
> + xorg_list_for_each_entry(ent, &pScreen->pixmap_dirty_list, ent) {
> + pSlave = ent->slave_dst->drawable.pScreen;
> + ret = xf86ScreenSetCursor(pSlave, pCurs, x, y);
> + if (ret == FALSE)
> + return ret;
> + }
> + return TRUE;
Same as a previous comment, saving the bool ret to return it when FALSE
seems just extra verbose.
> +}
> +
> void
> xf86SetTransparentCursor(ScreenPtr pScreen)
> {
> @@ -187,8 +255,8 @@ xf86SetTransparentCursor(ScreenPtr pScreen)
> (*infoPtr->ShowCursor) (infoPtr->pScrn);
> }
>
> -void
> -xf86MoveCursor(ScreenPtr pScreen, int x, int y)
> +static void
> +xf86ScreenMoveCursor(ScreenPtr pScreen, int x, int y)
> {
> xf86CursorScreenPtr ScreenPriv =
> (xf86CursorScreenPtr) dixLookupPrivate(&pScreen->devPrivates,
> @@ -202,6 +270,24 @@ xf86MoveCursor(ScreenPtr pScreen, int x, int y)
> }
>
> void
> +xf86MoveCursor(ScreenPtr pScreen, int x, int y)
> +{
> + ScreenPtr pSlave;
> + PixmapDirtyUpdatePtr ent;
> +
> + xf86ScreenMoveCursor(pScreen, x, y);
> +
> + if (xorg_list_is_empty(&pScreen->pixmap_dirty_list))
> + return;
> +
> + /* ask each driver consuming a pixmap if it can support HW cursor */
> + xorg_list_for_each_entry(ent, &pScreen->pixmap_dirty_list, ent) {
> + pSlave = ent->slave_dst->drawable.pScreen;
> + xf86ScreenMoveCursor(pSlave, x, y);
> + }
> +}
> +
> +void
> xf86RecolorCursor(ScreenPtr pScreen, CursorPtr pCurs, Bool displayed)
> {
> xf86CursorScreenPtr ScreenPriv =
> --
Dropping the list_is_empty checks is the only thing I feel strongly
about here, everything else is take it or leave it, then:
Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20150619/afe45101/attachment.sig>
More information about the xorg-devel
mailing list