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

2012-07-05 Thread Mario Kleiner 
On 03.07.2012, at 20:47, Dave Airlie wrote:

 On Tue, Jul 3, 2012 at 7:39 PM, Mario Kleiner
 mario.klei...@tuebingen.mpg.de wrote:
 
 On 03.07.2012, at 16:22, Dave Airlie wrote:
 
 From: Dave Airlie airl...@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.
 

...

 
 +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;
 +}
 

The following bit ok?

 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;
 + }
 +}
 +

…

 
 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.

Ok. Also Chris Wilson just posted an updated patch for an AsyncSwap hook, which 
would solve the same problem, depending on what gets in, in what order.

 
 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.

Thanks for the explanation. Posting damage at the current place could also 
cause composition of partially copied frames, right? Just a question to see if 
i understand the damage tracking correctly.

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

Not so sure how well i know this stuff, but i totally agreed to the definitely 
part 2 thing :). I'm happy if your current patches get in soonish. I think i 
will just shut up and wait

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

2012-07-03 Thread Dave Airlie 
From: Dave Airlie airl...@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 airl...@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 =

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

2012-07-03 Thread Mario Kleiner 

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

 From: Dave Airlie airl...@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 airl...@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,