[PATCH] cursor: add hw cursor support for prime (v2)

Aaron Plattner aplattner at nvidia.com
Thu Jul 16 13:58:05 PDT 2015


On 07/14/2015 05:15 PM, Dave Airlie wrote:
> 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.
> 
> v2:
> hotplugging causes the driver to try and register
> the cursor private, however we can't register
> these at runtime yet, we need to add support for
> reallocation of cursor screen privates. However
> all hotplugged devices currently don't support
> HW cursors, so punt for now. This means GPUs
> already in the system will get hw cursors
> and USB ones won't which is all we care about
> presently.
> drop list empty checks (Eric)
> fixup comments (Michel)
> 
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  dix/dispatch.c                     |  3 ++
>  dix/privates.c                     |  9 +++++
>  hw/xfree86/ramdac/xf86Cursor.c     | 12 +-----
>  hw/xfree86/ramdac/xf86CursorPriv.h |  1 +
>  hw/xfree86/ramdac/xf86HWCurs.c     | 83 ++++++++++++++++++++++++++++++++++++--
>  include/privates.h                 |  3 ++
>  6 files changed, 98 insertions(+), 13 deletions(-)
> 
> diff --git a/dix/dispatch.c b/dix/dispatch.c
> index 9208582..0f4354d 100644
> --- a/dix/dispatch.c
> +++ b/dix/dispatch.c
> @@ -3920,6 +3920,9 @@ AddGPUScreen(Bool (*pfnInit) (ScreenPtr /*pScreen */ ,
>  
>      update_desktop_dimensions();
>  
> +    if (!dixPrivatesCreated(PRIVATE_CURSOR))
> +        dixRegisterScreenPrivateKey(&cursorScreenDevPriv, pScreen, PRIVATE_CURSOR,
> +                                    0);
>      return i;
>  }
>  
> diff --git a/dix/privates.c b/dix/privates.c
> index e03b225..b638d23 100644
> --- a/dix/privates.c
> +++ b/dix/privates.c
> @@ -774,3 +774,12 @@ dixResetPrivates(void)
>          global_keys[t].allocated = 0;
>      }
>  }
> +
> +Bool
> +dixPrivatesCreated(DevPrivateType type)
> +{
> +    if (global_keys[type].created)
> +        return TRUE;
> +    else
> +        return FALSE;
> +}
> 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..ceddb6f 100644
> --- a/hw/xfree86/ramdac/xf86HWCurs.c
> +++ b/hw/xfree86/ramdac/xf86HWCurs.c
> @@ -119,8 +119,49 @@ xf86InitHardwareCursor(ScreenPtr pScreen, xf86CursorInfoPtr infoPtr)
>      return TRUE;
>  }
>  
> +static Bool
> +xf86CursorScreenCheckHW(ScreenPtr pScreen, CursorPtr cursor, xf86CursorInfoPtr infoPtr)
> +{
> +    return ((
> +             cursor->bits->argb &&
> +             infoPtr->UseHWCursorARGB &&
> +             (*infoPtr->UseHWCursorARGB)(pScreen, cursor)) ||
> +            (cursor->bits->argb == 0 &&
> +            (cursor->bits->height <= infoPtr->MaxHeight) &&

I think this needs to be indented one more space.

> +            (cursor->bits->width <= infoPtr->MaxWidth) &&
> +             (!infoPtr->UseHWCursor || (*infoPtr->UseHWCursor) (pScreen, cursor))));
> +}
> +
>  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;
> +
> +    /* ask each driver consuming a pixmap if it can support HW cursor */
> +    xorg_list_for_each_entry(ent, &pScreen->pixmap_dirty_list, ent) {

Hmm, we don't use PixmapStartDirtyTracking(), so pixmap_dirty_list is
empty for us.  Is there some other way you could structure this loop?
Otherwise, I could probably mock up entries on this list to trigger this
code despite the fact that we do our own dirty region tracking and
compositing.

Currently we just always composite the cursor ourselves into the
destination image using the GPU, so we don't really want software cursor
if the sink side doesn't support hardware cursors.  I'm not sure what
that should look like to this code, though.

> +        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 +206,27 @@ 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;
> +
> +    /* ask each slave driver to set the 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 FALSE;
> +    }
> +    return TRUE;
> +}
> +
>  void
>  xf86SetTransparentCursor(ScreenPtr pScreen)
>  {
> @@ -187,8 +249,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 +264,21 @@ xf86MoveCursor(ScreenPtr pScreen, int x, int y)
>  }
>  
>  void
> +xf86MoveCursor(ScreenPtr pScreen, int x, int y)
> +{
> +    ScreenPtr pSlave;
> +    PixmapDirtyUpdatePtr ent;
> +
> +    xf86ScreenMoveCursor(pScreen, x, y);
> +
> +    /* ask each slave driver to move the 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 =
> diff --git a/include/privates.h b/include/privates.h
> index 7d1461c..a2bb1a4 100644
> --- a/include/privates.h
> +++ b/include/privates.h
> @@ -252,6 +252,9 @@ dixFreeScreenSpecificPrivates(ScreenPtr pScreen);
>  extern void
>  dixInitScreenSpecificPrivates(ScreenPtr pScreen);
>  
> +/* is this private created - so hotplug can avoid crashing */
> +Bool dixPrivatesCreated(DevPrivateType type);
> +
>  extern _X_EXPORT void *
>  _dixAllocateScreenObjectWithPrivates(ScreenPtr pScreen,
>                                       unsigned size,
> 


-- 
Aaron


More information about the xorg-devel mailing list