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

Dave Airlie airlied at gmail.com
Tue Jul 3 11:47:24 PDT 2012


On Tue, Jul 3, 2012 at 7:39 PM, Mario Kleiner
<mario.kleiner at tuebingen.mpg.de> wrote:
>
> 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.

I could do that but I really would prefer we do things in iterations
and not confuse the interfaces, so add a CopyRegion3 with this sort of
thing.

The thing is I've not really considered timing because its really
messy with two GPUs, current offload design is offload GPU calls
CopyRegion2 which uses the hw copy engine to copy from the VRAM to
GTT, the GTT object is shared with the other GPU and is the backing
for the redirected window pixmap. So when the first driver copies to
the second, it causes damage on the backing pixmap which gets sent to
compositor which copies from it. Doing sync and flipping is a lot
messier, since you probably want to sync everything on the displaying
GPU.


> 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?

Again sync to vblank is definitely part 2, we've been looking at
inter-gpu semaphores (by we I mean mlankhorst), but I'd appreciate any
input you can give since you know this stuff fairly well ;-).

> 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.

Yes, step 1 is just enabling basic apps on them so users can play
games etc, I think we'll have to revisit things for timing sensitive
apps etc.

Dave.


More information about the xorg-devel mailing list