[PATCH] dri2: add initial prime support. (v1.2)

Mario Kleiner mario.kleiner at tuebingen.mpg.de
Tue Jul 3 11:39:17 PDT 2012


On 03.07.2012, at 16:22, Dave Airlie wrote:

> From: Dave Airlie <airlied at redhat.com>
> 
> This adds the initial prime support for dri2 offload. The main thing is
> when we get a connection from a prime client, we stored the information
> and mark all drawables from that client as prime. We then create all
> buffers for that drawable on the prime device dri2screen.
> 
> Then DRI2UpdatePrime is provided which drivers can call to get a shared
> pixmap which they can use as the front buffer. The driver is then
> responsible for doing the back->front copy to the shared buffer.
> 
> prime requires a compositing manager be run, but it handles the case where
> a window get un-redirected by allocating a new pixmap and pointing the crtc
> at it while the client is in that state.
> 
> Currently prime can't handle pageflipping, so always does straight copy swap,
> 
> v1.1: renumber on top of master.
> v1.2: fix auth on top of master.
> 
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
> glx/glxdri2.c             |    2 +-
> hw/xfree86/dri2/dri2.c    |  265 +++++++++++++++++++++++++++++++++++++++++----
> hw/xfree86/dri2/dri2.h    |   25 ++++-
> hw/xfree86/dri2/dri2ext.c |    4 +-
> 4 files changed, 267 insertions(+), 29 deletions(-)
> 
> diff --git a/glx/glxdri2.c b/glx/glxdri2.c
> index 7b76c3a..984f115 100644
> --- a/glx/glxdri2.c
> +++ b/glx/glxdri2.c
> @@ -840,7 +840,7 @@ __glXDRIscreenProbe(ScreenPtr pScreen)
>         return NULL;
> 
>     if (!xf86LoaderCheckSymbol("DRI2Connect") ||
> -        !DRI2Connect(pScreen, DRI2DriverDRI,
> +        !DRI2Connect(serverClient, pScreen, DRI2DriverDRI,
>                      &screen->fd, &driverName, &deviceName)) {
>         LogMessage(X_INFO,
>                    "AIGLX: Screen %d is not DRI2 capable\n", pScreen->myNum);
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index d3b3c73..0f87820 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -45,7 +45,7 @@
> #include "dixstruct.h"
> #include "dri2.h"
> #include "xf86VGAarbiter.h"
> -
> +#include "damage.h"
> #include "xf86.h"
> 
> CARD8 dri2_major;               /* version of DRI2 supported by DDX */
> @@ -63,6 +63,17 @@ static DevPrivateKeyRec dri2PixmapPrivateKeyRec;
> 
> #define dri2PixmapPrivateKey (&dri2PixmapPrivateKeyRec)
> 
> +static DevPrivateKeyRec dri2ClientPrivateKeyRec;
> +
> +#define dri2ClientPrivateKey (&dri2ClientPrivateKeyRec)
> +
> +#define dri2ClientPrivate(_pClient) (dixLookupPrivate(&(_pClient)->devPrivates, \
> +                                                      dri2ClientPrivateKey))
> +
> +typedef struct _DRI2Client {
> +    int prime_id;
> +} DRI2ClientRec, *DRI2ClientPtr;
> +
> static RESTYPE dri2DrawableRes;
> 
> typedef struct _DRI2Screen *DRI2ScreenPtr;
> @@ -87,6 +98,9 @@ typedef struct _DRI2Drawable {
>     int swap_limit;             /* for N-buffering */
>     unsigned long serialNumber;
>     Bool needInvalidate;
> +    int prime_id;
> +    PixmapPtr prime_slave_pixmap;
> +    PixmapPtr redirectpixmap;
> } DRI2DrawableRec, *DRI2DrawablePtr;
> 
> typedef struct _DRI2Screen {
> @@ -113,14 +127,47 @@ typedef struct _DRI2Screen {
>     HandleExposuresProcPtr HandleExposures;
> 
>     ConfigNotifyProcPtr ConfigNotify;
> +    DRI2CreateBuffer2ProcPtr CreateBuffer2;
> +    DRI2DestroyBuffer2ProcPtr DestroyBuffer2;
> +    DRI2CopyRegion2ProcPtr CopyRegion2;
> } DRI2ScreenRec;
> 
> +static void
> +destroy_buffer(DrawablePtr pDraw, DRI2BufferPtr buffer, int prime_id);
> +
> static DRI2ScreenPtr
> DRI2GetScreen(ScreenPtr pScreen)
> {
>     return dixLookupPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey);
> }
> 
> +static ScreenPtr
> +GetScreenPrime(ScreenPtr master, int prime_id)
> +{
> +    ScreenPtr slave;
> +    int i;
> +
> +    if (prime_id == 0 || xorg_list_is_empty(&master->offload_slave_list)) {
> +        return master;
> +    }
> +    i = 0;
> +    xorg_list_for_each_entry(slave, &master->offload_slave_list, offload_head) {
> +        if (i == (prime_id - 1))
> +            break;
> +        i++;
> +    }
> +    if (!slave)
> +        return master;
> +    return slave;
> +}
> +
> +static DRI2ScreenPtr
> +DRI2GetScreenPrime(ScreenPtr master, int prime_id)
> +{
> +    ScreenPtr slave = GetScreenPrime(master, prime_id);
> +    return DRI2GetScreen(slave);
> +}
> +                      
> static DRI2DrawablePtr
> DRI2GetDrawable(DrawablePtr pDraw)
> {
> @@ -187,7 +234,8 @@ DRI2AllocateDrawable(DrawablePtr pDraw)
>     xorg_list_init(&pPriv->reference_list);
>     pPriv->serialNumber = DRI2DrawableSerial(pDraw);
>     pPriv->needInvalidate = FALSE;
> -
> +    pPriv->redirectpixmap = NULL;
> +    pPriv->prime_slave_pixmap = NULL;
>     if (pDraw->type == DRAWABLE_WINDOW) {
>         pWin = (WindowPtr) pDraw;
>         dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, pPriv);
> @@ -286,6 +334,7 @@ DRI2CreateDrawable(ClientPtr client, DrawablePtr pDraw, XID id,
>                    DRI2InvalidateProcPtr invalidate, void *priv)
> {
>     DRI2DrawablePtr pPriv;
> +    DRI2ClientPtr dri2_client = dri2ClientPrivate(client);
>     XID dri2_id;
>     int rc;
> 
> @@ -295,6 +344,8 @@ DRI2CreateDrawable(ClientPtr client, DrawablePtr pDraw, XID id,
>     if (pPriv == NULL)
>         return BadAlloc;
> 
> +    pPriv->prime_id = dri2_client->prime_id;
> +
>     dri2_id = FakeClientID(client->index);
>     rc = DRI2AddDrawableRef(pPriv, id, dri2_id, invalidate, priv);
>     if (rc != Success)
> @@ -307,7 +358,6 @@ static int
> DRI2DrawableGone(pointer p, XID id)
> {
>     DRI2DrawablePtr pPriv = p;
> -    DRI2ScreenPtr ds = pPriv->dri2_screen;
>     DRI2DrawableRefPtr ref, next;
>     WindowPtr pWin;
>     PixmapPtr pPixmap;
> @@ -347,16 +397,52 @@ DRI2DrawableGone(pointer p, XID id)
> 
>     if (pPriv->buffers != NULL) {
>         for (i = 0; i < pPriv->bufferCount; i++)
> -            (*ds->DestroyBuffer) (pDraw, pPriv->buffers[i]);
> +            destroy_buffer(pDraw, pPriv->buffers[i], pPriv->prime_id);
> 
>         free(pPriv->buffers);
>     }
> 
> +    if (pPriv->redirectpixmap) {
> +        (*pDraw->pScreen->ReplaceScanoutPixmap)(pDraw, pPriv->redirectpixmap, FALSE);
> +	(*pDraw->pScreen->DestroyPixmap)(pPriv->redirectpixmap);
> +    }
> +
>     free(pPriv);
> 
>     return Success;
> }
> 
> +static DRI2BufferPtr
> +create_buffer(DrawablePtr pDraw,
> +              unsigned int attachment, unsigned int format)
> +{
> +    ScreenPtr primeScreen;
> +    DRI2DrawablePtr pPriv;
> +    DRI2ScreenPtr ds;
> +    DRI2BufferPtr buffer;
> +    pPriv = DRI2GetDrawable(pDraw);
> +    primeScreen = GetScreenPrime(pDraw->pScreen, pPriv->prime_id);
> +    ds = DRI2GetScreenPrime(pDraw->pScreen, pPriv->prime_id);
> +    if (ds->CreateBuffer2)
> +        buffer = (*ds->CreateBuffer2)(primeScreen, pDraw, attachment, format);
> +    else
> +        buffer = (*ds->CreateBuffer)(pDraw, attachment, format);
> +    return buffer;
> +}
> +
> +static void
> +destroy_buffer(DrawablePtr pDraw, DRI2BufferPtr buffer, int prime_id)
> +{
> +    ScreenPtr primeScreen;
> +    DRI2ScreenPtr ds;
> +    primeScreen = GetScreenPrime(pDraw->pScreen, prime_id);
> +    ds = DRI2GetScreen(primeScreen);
> +    if (ds->DestroyBuffer2)
> +        (*ds->DestroyBuffer2)(primeScreen, pDraw, buffer);
> +    else
> +        (*ds->DestroyBuffer)(pDraw, buffer);
> +}
> +
> static int
> find_attachment(DRI2DrawablePtr pPriv, unsigned attachment)
> {
> @@ -387,7 +473,7 @@ allocate_or_reuse_buffer(DrawablePtr pDraw, DRI2ScreenPtr ds,
>     if ((old_buf < 0)
>         || attachment == DRI2BufferFrontLeft
>         || !dimensions_match || (pPriv->buffers[old_buf]->format != format)) {
> -        *buffer = (*ds->CreateBuffer) (pDraw, attachment, format);
> +        *buffer = create_buffer (pDraw, attachment, format);
>         pPriv->serialNumber = DRI2DrawableSerial(pDraw);
>         return TRUE;
> 
> @@ -408,13 +494,12 @@ update_dri2_drawable_buffers(DRI2DrawablePtr pPriv, DrawablePtr pDraw,
>                              DRI2BufferPtr * buffers, int out_count, int *width,
>                              int *height)
> {
> -    DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen);
>     int i;
> 
>     if (pPriv->buffers != NULL) {
>         for (i = 0; i < pPriv->bufferCount; i++) {
>             if (pPriv->buffers[i] != NULL) {
> -                (*ds->DestroyBuffer) (pDraw, pPriv->buffers[i]);
> +                destroy_buffer(pDraw, pPriv->buffers[i], pPriv->prime_id);
>             }
>         }
> 
> @@ -434,8 +519,8 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>                unsigned int *attachments, int count, int *out_count,
>                int has_format)
> {
> -    DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen);
>     DRI2DrawablePtr pPriv = DRI2GetDrawable(pDraw);
> +    DRI2ScreenPtr ds;
>     DRI2BufferPtr *buffers;
>     int need_real_front = 0;
>     int need_fake_front = 0;
> @@ -452,6 +537,8 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>         return NULL;
>     }
> 
> +    ds = DRI2GetScreen(pDraw->pScreen);
> +
>     dimensions_match = (pDraw->width == pPriv->width)
>         && (pDraw->height == pPriv->height)
>         && (pPriv->serialNumber == DRI2DrawableSerial(pDraw));
> @@ -556,7 +643,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>     if (buffers) {
>         for (i = 0; i < count; i++) {
>             if (buffers[i] != NULL)
> -                (*ds->DestroyBuffer) (pDraw, buffers[i]);
> +                destroy_buffer(pDraw, buffers[i], 0);
>         }
> 
>         free(buffers);
> @@ -650,11 +737,118 @@ DRI2BlockClient(ClientPtr client, DrawablePtr pDraw)
>     pPriv->blockedOnMsc = TRUE;
> }
> 
> +static inline PixmapPtr GetDrawablePixmap(DrawablePtr drawable)
> +{
> +    if (drawable->type == DRAWABLE_PIXMAP)
> +        return (PixmapPtr)drawable;
> +    else {
> +        struct _Window *pWin = (struct _Window *)drawable;
> +        return drawable->pScreen->GetWindowPixmap(pWin);
> +    }
> +}
> +

Hi Dave,

> +DrawablePtr DRI2UpdatePrime(DrawablePtr pDraw, DRI2BufferPtr pDest)
> +{
> +    DRI2DrawablePtr pPriv = DRI2GetDrawable(pDraw);
> +    PixmapPtr spix;
> +    PixmapPtr mpix = GetDrawablePixmap(pDraw);
> +    ScreenPtr master, slave;
> +    Bool ret;
> +
> +    master = mpix->drawable.pScreen;
> +
> +    if (pDraw->type == DRAWABLE_WINDOW) {
> +	WindowPtr pWin = (WindowPtr)pDraw;
> +	PixmapPtr pPixmap = pDraw->pScreen->GetWindowPixmap(pWin);
> +
> +	if (pDraw->pScreen->GetScreenPixmap(pDraw->pScreen) == pPixmap) {
> +	    if (pPriv->redirectpixmap &&
> +		pPriv->redirectpixmap->drawable.width == pDraw->width &&
> +		pPriv->redirectpixmap->drawable.height == pDraw->height &&
> +		pPriv->redirectpixmap->drawable.depth == pDraw->depth) {
> +		mpix = pPriv->redirectpixmap;
> +	    } else {
> +                if (master->ReplaceScanoutPixmap) {
> +                    mpix = (*master->CreatePixmap)(master, pDraw->width, pDraw->height,
> +                                                   pDraw->depth, CREATE_PIXMAP_USAGE_SHARED);
> +                    if (!mpix)
> +                        return NULL;
> +                
> +                    ret = (*master->ReplaceScanoutPixmap)(pDraw, mpix, TRUE);
> +                    if (ret == FALSE) {
> +                        (*master->DestroyPixmap)(mpix);
> +                        return NULL;
> +                    }

I probably just don't understand the code well enough, but is a ...

if (pPriv->redirectpixmap) (*master->DestroyPixmap)(pPriv->redirectpixmap);

… missing here before the assignment of the new mpix?

> +                    pPriv->redirectpixmap = mpix;
> +                } else
> +                    return NULL;
> +	    }
> +	} else if (pPriv->redirectpixmap) {
> +            (*master->ReplaceScanoutPixmap)(pDraw, pPriv->redirectpixmap, FALSE);
> +	    (*master->DestroyPixmap)(pPriv->redirectpixmap);
> +	    pPriv->redirectpixmap = NULL;
> +	}
> +    }
> +
> +    slave = GetScreenPrime(pDraw->pScreen, pPriv->prime_id);
> +
> +    /* check if the pixmap is still fine */
> +    if (pPriv->prime_slave_pixmap) {
> +        if (pPriv->prime_slave_pixmap->master_pixmap == mpix)
> +            return &pPriv->prime_slave_pixmap->drawable;
> +        else {
> +            (*master->DestroyPixmap)(pPriv->prime_slave_pixmap->master_pixmap);
> +            (*slave->DestroyPixmap)(pPriv->prime_slave_pixmap);
> +        }
> +    }
> +
> +    spix = PixmapShareToSlave(mpix, slave);
> +    if (!spix)
> +        return NULL;
> +
> +    pPriv->prime_slave_pixmap = spix;
> +#ifdef COMPOSITE
> +    spix->screen_x = mpix->screen_x;
> +    spix->screen_y = mpix->screen_y;
> +#endif
> +    return &spix->drawable;
> +}
> +
> +static void dri2_copy_region(DrawablePtr pDraw, RegionPtr pRegion,
> +                             DRI2BufferPtr pDest, DRI2BufferPtr pSrc)
> +{
> +    DRI2DrawablePtr pPriv = DRI2GetDrawable(pDraw);
> +    DRI2ScreenPtr ds;
> +    ScreenPtr primeScreen;
> +
> +    primeScreen = GetScreenPrime(pDraw->pScreen, pPriv->prime_id);
> +    ds = DRI2GetScreen(primeScreen);
> +
> +    if (ds->CopyRegion2)
> +        (*ds->CopyRegion2)(primeScreen, pDraw, pRegion, pDest, pSrc);
> +    else
> +        (*ds->CopyRegion) (pDraw, pRegion, pDest, pSrc);
> +

If you define a new CopyRegion2 hook, it would be good to now also pass along the pPriv-> 
swap_interval parameter, or a "bool vsync = pPriv->swap_interval ? true : false" flag. That would allow the ddx CopyRegion2() to skip sync to vblank during copy if swap_interval is zero. Other OS'es and the NVidia/AMD binary blobs on Linux treat a zero swap interval as "don't sync to retrace". DRI2SwapBuffers() currently skips all swap scheduling on zero swap interval and uses CopyRegion(), but CopyRegion doesn't know it is a zero swap interval/no-vsync swap, so it syncs anyway. All ddx'es have a "SwapBuffersWait" xorg.conf option to disable this behaviour, but i think that was added as a workaround, not really the amount of control that applications expect.

Pardon my ignorance, probably i just don't understand the bigger picture and should read up on this, but how does synchronization between the offload slave and the master work in CopyRegion2()? Do i understand correctly that CopyRegion2() is executed by the offload slave?

If so, how does a compositor in the redirected case know when the offload slave has finished its blit to the redirectPixmap? How does the offload slave sync to vblank of the masters crtc in the unredirected case when its pPriv->redirectpixmap is set as a crtc's scanout pixmap?
My main concern for timing-sensitive applications is basically how to get normal DRI2 swap scheduling and especially kms page-flipping and timestamping back.

The hack to use the fallback path for old drivers or zero swap_interval in DRI2SwapBuffers()  …

@@ -879,7 +1073,7 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
    }

    /* Old DDX or no swap interval, just blit */
-    if (!ds->ScheduleSwap || !pPriv->swap_interval) {
+    if (!ds->ScheduleSwap || !pPriv->swap_interval || pPriv->prime_id) {

… sounds ok to me as a start, but for future iterations i'd really like to not lose the real DRI2SwapBuffers functionality on dual-gpu setups. 

Currently muxless Optimus hardware is an absolute no-go for timing sensitive applications, making it difficult to find Laptops that are useable. It would be super-cool for such apps if the prime implementation could do well timing-wise on such setups.

thanks,
-mario

> +    /* cause damage to the box */
> +    if (pPriv->prime_id) {
> +       BoxRec box;
> +       RegionRec region;
> +       box.x1 = 0;
> +       box.x2 = box.x1 + pDraw->width;
> +       box.y1 = 0;
> +       box.y2 = box.y1 + pDraw->height;
> +       RegionInit(&region, &box, 1);
> +       RegionTranslate(&region, pDraw->x, pDraw->y);
> +       DamageRegionAppend(pDraw, &region);
> +       DamageRegionProcessPending(pDraw);
> +       RegionUninit(&region); 
> +    }
> +}
> +
> int
> DRI2CopyRegion(DrawablePtr pDraw, RegionPtr pRegion,
>                unsigned int dest, unsigned int src)
> {
> -    DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen);
>     DRI2DrawablePtr pPriv;
>     DRI2BufferPtr pDestBuffer, pSrcBuffer;
>     int i;
> @@ -674,7 +868,7 @@ DRI2CopyRegion(DrawablePtr pDraw, RegionPtr pRegion,
>     if (pSrcBuffer == NULL || pDestBuffer == NULL)
>         return BadValue;
> 
> -    (*ds->CopyRegion) (pDraw, pRegion, pDestBuffer, pSrcBuffer);
> +    dri2_copy_region(pDraw, pRegion, pDestBuffer, pSrcBuffer);
> 
>     return Success;
> }

> @@ -879,7 +1073,7 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
>     }
> 
>     /* Old DDX or no swap interval, just blit */
> -    if (!ds->ScheduleSwap || !pPriv->swap_interval) {
> +    if (!ds->ScheduleSwap || !pPriv->swap_interval || pPriv->prime_id) {
>         BoxRec box;
>         RegionRec region;
> 
> @@ -891,7 +1085,7 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
> 
>         pPriv->swapsPending++;
> 
> -        (*ds->CopyRegion) (pDraw, &region, pDestBuffer, pSrcBuffer);
> +        dri2_copy_region(pDraw, &region, pDestBuffer, pSrcBuffer);
>         DRI2SwapComplete(client, pDraw, target_msc, 0, 0, DRI2_BLIT_COMPLETE,
>                          func, data);
>         return Success;
> @@ -1091,22 +1285,34 @@ DRI2HasSwapControl(ScreenPtr pScreen)
> }
> 




More information about the xorg-devel mailing list