Implement GLX_EXT_buffer_age for DRI2

Implement GLX_EXT_buffer_age for DRI2

[Chris Wilson]

In order to suport GLX_EXT_buffer_age in DRI2, we need to pass back the
last swap buffer count that the back buffer was defined for. For
simplicity, we can reuse an existing field in the DRI2GetBuffers reply
that is not used by current drivers, the flags. Since we change the
interpretation of this flag, we also declare the semantic change with a
DRI2 parameter and depend upon the DDX to enable the change
responsibility (which is just a matter of reviewing whether the flags
field has ever been used for a non-zero value).

Bugzilla: https://bugzilla.gnome.org/show_bug.cgi?id=701801

This series add the core support to X, mesa and -ati/-nouveau.

[dri2proto] Declare DRI2ParamXHasBufferAge
[xorg 1/3] dri2: Allow GetBuffers to match any format
[xorg 2/3] dri2: Pass swap-interval=0 ScheduleSwap requests to the
[xorg 3/3] dri2: Reuse unused flags in GetBuffers protocol to pass
[xf86-video-ati] dri2: Enable BufferAge support
[xf86-video-nouveau] dri2: Enable BufferAge support
[mesa 7/9] glx/dri2: Add DRI2GetParam()
[mesa 8/9] glx/dri2: Move the wait after SwapBuffers into the next
[mesa 9/9] glx/dri2: Implement getBufferAge
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[dri2proto] Declare DRI2ParamXHasBufferAge

[Chris Wilson]

In order for X/DDX to reuse a driver specific field of the DRI2GetBuffers
reply, we need to declare the change in semantics. To indicate that the
flags field now continues the last swap buffers count instead, we
introduce the has-buffer-age parameter.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 configure.ac  |  2 +-
 dri2proto.h   |  2 ++
 dri2proto.txt | 11 ++++++++---
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5fadf56..9f4c4a0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 AC_PREREQ([2.60])
-AC_INIT([DRI2Proto], [2.8], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
+AC_INIT([DRI2Proto], [2.9], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
 AM_INIT_AUTOMAKE([foreign dist-bzip2])
 
 # Require xorg-macros: XORG_DEFAULT_OPTIONS
diff --git a/dri2proto.h b/dri2proto.h
index 128b807..086dc96 100644
--- a/dri2proto.h
+++ b/dri2proto.h
@@ -340,6 +340,8 @@ typedef struct {
 } xDRI2GetParamReq;
 #define sz_xDRI2GetParamReq 12
 
+#define DRI2ParamXHasBufferAge 0
+
 typedef struct {
     BYTE    type; /*X_Reply*/
     BOOL    is_param_recognized;
diff --git a/dri2proto.txt b/dri2proto.txt
index 9921301..9daa58e 100644
--- a/dri2proto.txt
+++ b/dri2proto.txt
@@ -454,9 +454,14 @@ The name of this extension is "DRI2".
 	the screen associated with 'drawable'.
 
 	Parameter names in which the value of the most significant byte is
-	0 are reserved for the X server. Currently, no such parameter names
-	are defined. (When any such names are defined, they will be defined in
-	this extension specification and its associated headers).
+	0 are reserved for the X server. The complete list of known parameter
+        names for the X server are:
+
+                0 - DRI2ParamXHasBufferAge
+
+                    Query whether the X server and DDX support passing the
+                    buffers last swap buffer count in the flags field of
+                    the DRI2GetBuffers reply.
 
 	Parameter names in which the byte's value is 1 are reserved for the
 	DDX. Such names are private to each driver and shall be defined in the
-- 
2.1.4

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] [dri2proto] Declare DRI2ParamXHasBufferAge

[Ian Romanick]

On 01/19/2015 03:00 AM, Chris Wilson wrote:
> In order for X/DDX to reuse a driver specific field of the DRI2GetBuffers
> reply, we need to declare the change in semantics. To indicate that the
> flags field now continues the last swap buffers count instead, we
> introduce the has-buffer-age parameter.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

> ---
>  configure.ac  |  2 +-
>  dri2proto.h   |  2 ++
>  dri2proto.txt | 11 ++++++++---
>  3 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 5fadf56..9f4c4a0 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1,5 +1,5 @@
>  AC_PREREQ([2.60])
> -AC_INIT([DRI2Proto], [2.8], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
> +AC_INIT([DRI2Proto], [2.9], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg])
>  AM_INIT_AUTOMAKE([foreign dist-bzip2])
>  
>  # Require xorg-macros: XORG_DEFAULT_OPTIONS
> diff --git a/dri2proto.h b/dri2proto.h
> index 128b807..086dc96 100644
> --- a/dri2proto.h
> +++ b/dri2proto.h
> @@ -340,6 +340,8 @@ typedef struct {
>  } xDRI2GetParamReq;
>  #define sz_xDRI2GetParamReq 12
>  
> +#define DRI2ParamXHasBufferAge 0
> +
>  typedef struct {
>      BYTE    type; /*X_Reply*/
>      BOOL    is_param_recognized;
> diff --git a/dri2proto.txt b/dri2proto.txt
> index 9921301..9daa58e 100644
> --- a/dri2proto.txt
> +++ b/dri2proto.txt
> @@ -454,9 +454,14 @@ The name of this extension is "DRI2".
>  	the screen associated with 'drawable'.
>  
>  	Parameter names in which the value of the most significant byte is
> -	0 are reserved for the X server. Currently, no such parameter names
> -	are defined. (When any such names are defined, they will be defined in
> -	this extension specification and its associated headers).
> +	0 are reserved for the X server. The complete list of known parameter
> +        names for the X server are:
> +
> +                0 - DRI2ParamXHasBufferAge
> +
> +                    Query whether the X server and DDX support passing the
> +                    buffers last swap buffer count in the flags field of
> +                    the DRI2GetBuffers reply.
>  
>  	Parameter names in which the byte's value is 1 are reserved for the
>  	DDX. Such names are private to each driver and shall be defined in the
> 

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [dri2proto] Declare DRI2ParamXHasBufferAge

[Martin Peres]

On 20/01/15 22:53, Ian Romanick wrote:
> On 01/19/2015 03:00 AM, Chris Wilson wrote:
>> In order for X/DDX to reuse a driver specific field of the DRI2GetBuffers
>> reply, we need to declare the change in semantics. To indicate that the
>> flags field now continues the last swap buffers count instead, we
>> introduce the has-buffer-age parameter.
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
>

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[xorg 1/3] dri2: Allow GetBuffers to match any format

[Chris Wilson]

Since the introduction of DRI2GetBuffersWithFormat, the old
DRI2GetBuffers interface would always recreate all buffers all the time
as it was no longer agnostic to the format value being set by the DDXes.
This causes an issue with clients intermixing the two requests,
rendering any sharing or caching of buffers (e.g. for triple buffering)
void.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 hw/xfree86/dri2/dri2.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 43a1899..f9f594d 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -464,14 +464,16 @@ find_attachment(DRI2DrawablePtr pPriv, unsigned attachment)
 static Bool
 allocate_or_reuse_buffer(DrawablePtr pDraw, DRI2ScreenPtr ds,
                          DRI2DrawablePtr pPriv,
-                         unsigned int attachment, unsigned int format,
+                         unsigned int attachment,
+                         int has_format, unsigned int format,
                          int dimensions_match, DRI2BufferPtr * buffer)
 {
     int old_buf = find_attachment(pPriv, attachment);
 
     if ((old_buf < 0)
         || attachment == DRI2BufferFrontLeft
-        || !dimensions_match || (pPriv->buffers[old_buf]->format != format)) {
+        || !dimensions_match
+        || (has_format && pPriv->buffers[old_buf]->format != format)) {
         *buffer = create_buffer(ds, pDraw, attachment, format);
         return TRUE;
 
@@ -549,7 +551,8 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
         const unsigned format = (has_format) ? *(attachments++) : 0;
 
         if (allocate_or_reuse_buffer(pDraw, ds, pPriv, attachment,
-                                     format, dimensions_match, &buffers[i]))
+                                     has_format, format, dimensions_match,
+                                     &buffers[i]))
             buffers_changed = 1;
 
         if (buffers[i] == NULL)
@@ -584,7 +587,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
 
     if (need_real_front > 0) {
         if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFrontLeft,
-                                     front_format, dimensions_match,
+                                     has_format, front_format, dimensions_match,
                                      &buffers[i]))
             buffers_changed = 1;
 
@@ -595,7 +598,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
 
     if (need_fake_front > 0) {
         if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFakeFrontLeft,
-                                     front_format, dimensions_match,
+                                     has_format, front_format, dimensions_match,
                                      &buffers[i]))
             buffers_changed = 1;
 
-- 
2.1.4

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] [xorg 1/3] dri2: Allow GetBuffers to match any format

[Ian Romanick]

On 01/19/2015 03:00 AM, Chris Wilson wrote:
> Since the introduction of DRI2GetBuffersWithFormat, the old
> DRI2GetBuffers interface would always recreate all buffers all the time
> as it was no longer agnostic to the format value being set by the DDXes.
> This causes an issue with clients intermixing the two requests,
> rendering any sharing or caching of buffers (e.g. for triple buffering)
> void.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  hw/xfree86/dri2/dri2.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index 43a1899..f9f594d 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -464,14 +464,16 @@ find_attachment(DRI2DrawablePtr pPriv, unsigned attachment)
>  static Bool
>  allocate_or_reuse_buffer(DrawablePtr pDraw, DRI2ScreenPtr ds,
>                           DRI2DrawablePtr pPriv,
> -                         unsigned int attachment, unsigned int format,
> +                         unsigned int attachment,
> +                         int has_format, unsigned int format,
>                           int dimensions_match, DRI2BufferPtr * buffer)
>  {
>      int old_buf = find_attachment(pPriv, attachment);
>  
>      if ((old_buf < 0)
>          || attachment == DRI2BufferFrontLeft
> -        || !dimensions_match || (pPriv->buffers[old_buf]->format != format)) {
> +        || !dimensions_match
> +        || (has_format && pPriv->buffers[old_buf]->format != format)) {
>          *buffer = create_buffer(ds, pDraw, attachment, format);

Shouldn't the create_buffer change if !has_format?  If !has_format and,
say, !dimensions_match, create_buffer will get format = 0 when it should
get format = pPriv->buffers[old_buf]->format.  Right?

Another alternative would be to have the caller always pass a format:
either the format supplied in the protocol or the format of the old
buffer.  That might be more messy.  Dunno.

>          return TRUE;
>  
> @@ -549,7 +551,8 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>          const unsigned format = (has_format) ? *(attachments++) : 0;
>  
>          if (allocate_or_reuse_buffer(pDraw, ds, pPriv, attachment,
> -                                     format, dimensions_match, &buffers[i]))
> +                                     has_format, format, dimensions_match,
> +                                     &buffers[i]))
>              buffers_changed = 1;
>  
>          if (buffers[i] == NULL)
> @@ -584,7 +587,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>  
>      if (need_real_front > 0) {
>          if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFrontLeft,
> -                                     front_format, dimensions_match,
> +                                     has_format, front_format, dimensions_match,
>                                       &buffers[i]))
>              buffers_changed = 1;
>  
> @@ -595,7 +598,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>  
>      if (need_fake_front > 0) {
>          if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFakeFrontLeft,
> -                                     front_format, dimensions_match,
> +                                     has_format, front_format, dimensions_match,
>                                       &buffers[i]))
>              buffers_changed = 1;
>  
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [xorg 1/3] dri2: Allow GetBuffers to match any format

[Martin Peres]

On 20/01/15 22:49, Ian Romanick wrote:
> On 01/19/2015 03:00 AM, Chris Wilson wrote:
>> Since the introduction of DRI2GetBuffersWithFormat, the old
>> DRI2GetBuffers interface would always recreate all buffers all the time
>> as it was no longer agnostic to the format value being set by the DDXes.
>> This causes an issue with clients intermixing the two requests,
>> rendering any sharing or caching of buffers (e.g. for triple buffering)
>> void.
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   hw/xfree86/dri2/dri2.c | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
>> index 43a1899..f9f594d 100644
>> --- a/hw/xfree86/dri2/dri2.c
>> +++ b/hw/xfree86/dri2/dri2.c
>> @@ -464,14 +464,16 @@ find_attachment(DRI2DrawablePtr pPriv, unsigned attachment)
>>   static Bool
>>   allocate_or_reuse_buffer(DrawablePtr pDraw, DRI2ScreenPtr ds,
>>                            DRI2DrawablePtr pPriv,
>> -                         unsigned int attachment, unsigned int format,
>> +                         unsigned int attachment,
>> +                         int has_format, unsigned int format,
>>                            int dimensions_match, DRI2BufferPtr * buffer)
>>   {
>>       int old_buf = find_attachment(pPriv, attachment);
>>   
>>       if ((old_buf < 0)
>>           || attachment == DRI2BufferFrontLeft
>> -        || !dimensions_match || (pPriv->buffers[old_buf]->format != format)) {
>> +        || !dimensions_match
>> +        || (has_format && pPriv->buffers[old_buf]->format != format)) {
>>           *buffer = create_buffer(ds, pDraw, attachment, format);
> Shouldn't the create_buffer change if !has_format?  If !has_format and,
> say, !dimensions_match, create_buffer will get format = 0 when it should
> get format = pPriv->buffers[old_buf]->format.  Right?

This is still a problem in the current patchset that I have. Since the 
client did not specifically ask for a certain format, why not increase 
the likeliness of us being able to reuse the buffer later on by using a 
format that the application already asked before?

>
> Another alternative would be to have the caller always pass a format:
> either the format supplied in the protocol or the format of the old
> buffer.  That might be more messy.  Dunno.
>
>>           return TRUE;
>>   
>> @@ -549,7 +551,8 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>>           const unsigned format = (has_format) ? *(attachments++) : 0;
>>   
>>           if (allocate_or_reuse_buffer(pDraw, ds, pPriv, attachment,
>> -                                     format, dimensions_match, &buffers[i]))
>> +                                     has_format, format, dimensions_match,
>> +                                     &buffers[i]))
>>               buffers_changed = 1;
>>   
>>           if (buffers[i] == NULL)
>> @@ -584,7 +587,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>>   
>>       if (need_real_front > 0) {
>>           if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFrontLeft,
>> -                                     front_format, dimensions_match,
>> +                                     has_format, front_format, dimensions_match,
>>                                        &buffers[i]))
>>               buffers_changed = 1;
>>   
>> @@ -595,7 +598,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height,
>>   
>>       if (need_fake_front > 0) {
>>           if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFakeFrontLeft,
>> -                                     front_format, dimensions_match,
>> +                                     has_format, front_format, dimensions_match,
>>                                        &buffers[i]))
>>               buffers_changed = 1;
>>   
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[xorg 2/3] dri2: Pass swap-interval=0 ScheduleSwap requests to the ddx

[Chris Wilson]

Allow the DDXes to opt-in and handle swap-interval=0 requests for
themselves, for example by using asynchronous pageflips, rather than
forcing a blit. This has the important side-effect of also
disambiguating CopyRegion calls to always be client requests.

References: http://lists.x.org/archives/xorg-devel/2011-June/023102.html
References: http://lists.x.org/archives/xorg-devel/2012-February/029336.html
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 hw/xfree86/dri2/dri2.c | 14 ++++++++++----
 hw/xfree86/dri2/dri2.h |  5 ++++-
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index f9f594d..2c0367e 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -114,6 +114,7 @@ typedef struct _DRI2Screen {
     int fd;
     unsigned int lastSequence;
     int prime_id;
+    int scheduleSwap0;
 
     DRI2CreateBufferProcPtr CreateBuffer;
     DRI2DestroyBufferProcPtr DestroyBuffer;
@@ -1116,8 +1117,8 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
         return BadDrawable;
     }
 
-    /* Old DDX or no swap interval, just blit */
-    if (!ds->ScheduleSwap || !pPriv->swap_interval || pPriv->prime_id) {
+    /* Old DDX or PRIME, just blit */
+    if (!ds->scheduleSwap0 || pPriv->prime_id) {
         BoxRec box;
         RegionRec region;
 
@@ -1139,7 +1140,9 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
      * In the simple glXSwapBuffers case, all params will be 0, and we just
      * need to schedule a swap for the last swap target + the swap interval.
      */
-    if (target_msc == 0 && divisor == 0 && remainder == 0) {
+    if (pPriv->swap_interval == 0) {
+	target_msc = 0;
+    } else if (target_msc == 0 && divisor == 0 && remainder == 0) {
         /* If the current vblank count of the drawable's crtc is lower
          * than the count stored in last_swap_target from a previous swap
          * then reinitialize last_swap_target to the current crtc's msc,
@@ -1162,7 +1165,6 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
          * number of pending swaps.
          */
         target_msc = pPriv->last_swap_target + pPriv->swap_interval;
-
     }
 
     pPriv->swapsPending++;
@@ -1558,6 +1560,10 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
         ds->CopyRegion2 = info->CopyRegion2;
     }
 
+    if (info->version >= 10) {
+        ds->scheduleSwap0 = info->scheduleSwap0;
+    }
+
     /*
      * if the driver doesn't provide an AuthMagic function or the info struct
      * version is too low, call through LegacyAuthMagic
diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
index 1e7afdd..1cf4288 100644
--- a/hw/xfree86/dri2/dri2.h
+++ b/hw/xfree86/dri2/dri2.h
@@ -205,7 +205,7 @@ typedef int (*DRI2GetParamProcPtr) (ClientPtr client,
 /**
  * Version of the DRI2InfoRec structure defined in this header
  */
-#define DRI2INFOREC_VERSION 9
+#define DRI2INFOREC_VERSION 10
 
 typedef struct {
     unsigned int version;       /**< Version of this struct */
@@ -252,6 +252,9 @@ typedef struct {
     DRI2CreateBuffer2ProcPtr CreateBuffer2;
     DRI2DestroyBuffer2ProcPtr DestroyBuffer2;
     DRI2CopyRegion2ProcPtr CopyRegion2;
+
+    /* added in version 10 */
+    int scheduleSwap0;
 } DRI2InfoRec, *DRI2InfoPtr;
 
 extern _X_EXPORT Bool DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info);
-- 
2.1.4

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [xorg 2/3] dri2: Pass swap-interval=0 ScheduleSwap requests to the ddx

[Fredrik Höglund]

On Monday 19 January 2015, Chris Wilson wrote:
> Allow the DDXes to opt-in and handle swap-interval=0 requests for
> themselves, for example by using asynchronous pageflips, rather than
> forcing a blit. This has the important side-effect of also
> disambiguating CopyRegion calls to always be client requests.
> 
> References: http://lists.x.org/archives/xorg-devel/2011-June/023102.html
> References: http://lists.x.org/archives/xorg-devel/2012-February/029336.html
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  hw/xfree86/dri2/dri2.c | 14 ++++++++++----
>  hw/xfree86/dri2/dri2.h |  5 ++++-
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index f9f594d..2c0367e 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -114,6 +114,7 @@ typedef struct _DRI2Screen {
>      int fd;
>      unsigned int lastSequence;
>      int prime_id;
> +    int scheduleSwap0;
>  
>      DRI2CreateBufferProcPtr CreateBuffer;
>      DRI2DestroyBufferProcPtr DestroyBuffer;
> @@ -1116,8 +1117,8 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
>          return BadDrawable;
>      }
>  
> -    /* Old DDX or no swap interval, just blit */
> -    if (!ds->ScheduleSwap || !pPriv->swap_interval || pPriv->prime_id) {
> +    /* Old DDX or PRIME, just blit */
> +    if (!ds->scheduleSwap0 || pPriv->prime_id) {

I've been testing these patches with the radeon driver, and happened to
notice that it never pageflips. I think this line needs to be changed to
something along the lines of:

if (!ds->ScheduleSwap || (pPriv->swap_interval == 0 && !ds->scheduleSwap0) || pPriv->prime_id)

>          BoxRec box;
>          RegionRec region;
>  
> @@ -1139,7 +1140,9 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
>       * In the simple glXSwapBuffers case, all params will be 0, and we just
>       * need to schedule a swap for the last swap target + the swap interval.
>       */
> -    if (target_msc == 0 && divisor == 0 && remainder == 0) {
> +    if (pPriv->swap_interval == 0) {
> +	target_msc = 0;
> +    } else if (target_msc == 0 && divisor == 0 && remainder == 0) {
>          /* If the current vblank count of the drawable's crtc is lower
>           * than the count stored in last_swap_target from a previous swap
>           * then reinitialize last_swap_target to the current crtc's msc,
> @@ -1162,7 +1165,6 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
>           * number of pending swaps.
>           */
>          target_msc = pPriv->last_swap_target + pPriv->swap_interval;
> -
>      }
>  
>      pPriv->swapsPending++;
> @@ -1558,6 +1560,10 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
>          ds->CopyRegion2 = info->CopyRegion2;
>      }
>  
> +    if (info->version >= 10) {
> +        ds->scheduleSwap0 = info->scheduleSwap0;
> +    }
> +
>      /*
>       * if the driver doesn't provide an AuthMagic function or the info struct
>       * version is too low, call through LegacyAuthMagic
> diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
> index 1e7afdd..1cf4288 100644
> --- a/hw/xfree86/dri2/dri2.h
> +++ b/hw/xfree86/dri2/dri2.h
> @@ -205,7 +205,7 @@ typedef int (*DRI2GetParamProcPtr) (ClientPtr client,
>  /**
>   * Version of the DRI2InfoRec structure defined in this header
>   */
> -#define DRI2INFOREC_VERSION 9
> +#define DRI2INFOREC_VERSION 10
>  
>  typedef struct {
>      unsigned int version;       /**< Version of this struct */
> @@ -252,6 +252,9 @@ typedef struct {
>      DRI2CreateBuffer2ProcPtr CreateBuffer2;
>      DRI2DestroyBuffer2ProcPtr DestroyBuffer2;
>      DRI2CopyRegion2ProcPtr CopyRegion2;
> +
> +    /* added in version 10 */
> +    int scheduleSwap0;
>  } DRI2InfoRec, *DRI2InfoPtr;
>  
>  extern _X_EXPORT Bool DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info);
> 

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[xorg 3/3] dri2: Reuse unused flags in GetBuffers protocol to pass last SBC

[Chris Wilson]

Allow mesa/dri2 to implement GLX_EXT_buffer_age by reporting the sbc of
when the current back buffer was defined. As this may require ddx
support, only set the value if enabled by the ddx and report the new
semantics via a DRI2GetParam request.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 configure.ac           |  2 +-
 hw/xfree86/dri2/dri2.c | 24 ++++++++++++++++++++----
 hw/xfree86/dri2/dri2.h |  1 +
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac
index b593fc7..e49c35e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -768,7 +768,7 @@ RECORDPROTO="recordproto >= 1.13.99.1"
 SCRNSAVERPROTO="scrnsaverproto >= 1.1"
 RESOURCEPROTO="resourceproto >= 1.2.0"
 DRIPROTO="xf86driproto >= 2.1.0"
-DRI2PROTO="dri2proto >= 2.8"
+DRI2PROTO="dri2proto >= 2.9"
 DRI3PROTO="dri3proto >= 1.0"
 XINERAMAPROTO="xineramaproto"
 BIGFONTPROTO="xf86bigfontproto >= 1.2.0"
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 2c0367e..a998034 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -49,6 +49,8 @@
 #include "damage.h"
 #include "xf86.h"
 
+#include <X11/extensions/dri2proto.h> /* for parameter names */
+
 CARD8 dri2_major;               /* version of DRI2 supported by DDX */
 CARD8 dri2_minor;
 
@@ -115,6 +117,7 @@ typedef struct _DRI2Screen {
     unsigned int lastSequence;
     int prime_id;
     int scheduleSwap0;
+    int bufferAge;
 
     DRI2CreateBufferProcPtr CreateBuffer;
     DRI2DestroyBufferProcPtr DestroyBuffer;
@@ -1104,6 +1107,8 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
      * it as early as possible, just to be sure.
      */
     *swap_target = pPriv->swap_count + pPriv->swapsPending + 1;
+    if (ds->bufferAge)
+        pSrcBuffer->flags = *swap_target;
 
     for (i = 0; i < pPriv->bufferCount; i++) {
         if (pPriv->buffers[i]->attachment == DRI2BufferFrontLeft)
@@ -1562,6 +1567,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
 
     if (info->version >= 10) {
         ds->scheduleSwap0 = info->scheduleSwap0;
+        ds->bufferAge = info->bufferAge;
     }
 
     /*
@@ -1684,10 +1690,20 @@ DRI2GetParam(ClientPtr client,
 
     switch (high_byte) {
     case 0:
-        /* Parameter names whose high_byte is 0 are reserved for the X
-         * server. The server currently recognizes no parameters.
-         */
-        goto not_recognized;
+	/* Parameter names whose high_byte is 0 are reserved for the X
+	 * server.
+	 */
+	switch (param) {
+	case DRI2ParamXHasBufferAge:
+	    *value = ds->bufferAge;
+	    break;
+	default:
+	    goto not_recognized;
+	}
+
+	*is_param_recognized = TRUE;
+	return Success;
+
     case 1:
         /* Parameter names whose high byte is 1 are reserved for the DDX. */
         if (ds->GetParam)
diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
index 1cf4288..e76f7a8 100644
--- a/hw/xfree86/dri2/dri2.h
+++ b/hw/xfree86/dri2/dri2.h
@@ -255,6 +255,7 @@ typedef struct {
 
     /* added in version 10 */
     int scheduleSwap0;
+    int bufferAge;
 } DRI2InfoRec, *DRI2InfoPtr;
 
 extern _X_EXPORT Bool DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info);
-- 
2.1.4

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [xorg 3/3] dri2: Reuse unused flags in GetBuffers protocol to pass last SBC

[Chris Wilson]

On Mon, Jan 19, 2015 at 11:00:40AM +0000, Chris Wilson wrote:
> @@ -1104,6 +1107,8 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
>       * it as early as possible, just to be sure.
>       */
>      *swap_target = pPriv->swap_count + pPriv->swapsPending + 1;
> +    if (ds->bufferAge)
> +        pSrcBuffer->flags = *swap_target;

I made the cardinal sin of trying to tidy the patch up at the last
moment and moved this hunk before the definition of pSrcBuffer.

Sighs.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] dri2: Reuse unused flags in GetBuffers protocol to pass last SBC

[Chris Wilson]

Allow mesa/dri2 to implement GLX_EXT_buffer_age by reporting the sbc of
when the current back buffer was defined. As this may require ddx
support, only set the value if enabled by the ddx and report the new
semantics via a DRI2GetParam request.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 configure.ac           |  2 +-
 hw/xfree86/dri2/dri2.c | 25 +++++++++++++++++++++----
 hw/xfree86/dri2/dri2.h |  1 +
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac
index b593fc7..e49c35e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -768,7 +768,7 @@ RECORDPROTO="recordproto >= 1.13.99.1"
 SCRNSAVERPROTO="scrnsaverproto >= 1.1"
 RESOURCEPROTO="resourceproto >= 1.2.0"
 DRIPROTO="xf86driproto >= 2.1.0"
-DRI2PROTO="dri2proto >= 2.8"
+DRI2PROTO="dri2proto >= 2.9"
 DRI3PROTO="dri3proto >= 1.0"
 XINERAMAPROTO="xineramaproto"
 BIGFONTPROTO="xf86bigfontproto >= 1.2.0"
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index 2c0367e..d70a632 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
@@ -49,6 +49,8 @@
 #include "damage.h"
 #include "xf86.h"
 
+#include <X11/extensions/dri2proto.h> /* for parameter names */
+
 CARD8 dri2_major;               /* version of DRI2 supported by DDX */
 CARD8 dri2_minor;
 
@@ -115,6 +117,7 @@ typedef struct _DRI2Screen {
     unsigned int lastSequence;
     int prime_id;
     int scheduleSwap0;
+    int bufferAge;
 
     DRI2CreateBufferProcPtr CreateBuffer;
     DRI2DestroyBufferProcPtr DestroyBuffer;
@@ -1117,6 +1120,9 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
         return BadDrawable;
     }
 
+    if (ds->bufferAge)
+        pSrcBuffer->flags = *swap_target;
+
     /* Old DDX or PRIME, just blit */
     if (!ds->scheduleSwap0 || pPriv->prime_id) {
         BoxRec box;
@@ -1562,6 +1568,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
 
     if (info->version >= 10) {
         ds->scheduleSwap0 = info->scheduleSwap0;
+        ds->bufferAge = info->bufferAge;
     }
 
     /*
@@ -1684,10 +1691,20 @@ DRI2GetParam(ClientPtr client,
 
     switch (high_byte) {
     case 0:
-        /* Parameter names whose high_byte is 0 are reserved for the X
-         * server. The server currently recognizes no parameters.
-         */
-        goto not_recognized;
+	/* Parameter names whose high_byte is 0 are reserved for the X
+	 * server.
+	 */
+	switch (param) {
+	case DRI2ParamXHasBufferAge:
+	    *value = ds->bufferAge;
+	    break;
+	default:
+	    goto not_recognized;
+	}
+
+	*is_param_recognized = TRUE;
+	return Success;
+
     case 1:
         /* Parameter names whose high byte is 1 are reserved for the DDX. */
         if (ds->GetParam)
diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
index 1cf4288..e76f7a8 100644
--- a/hw/xfree86/dri2/dri2.h
+++ b/hw/xfree86/dri2/dri2.h
@@ -255,6 +255,7 @@ typedef struct {
 
     /* added in version 10 */
     int scheduleSwap0;
+    int bufferAge;
 } DRI2InfoRec, *DRI2InfoPtr;
 
 extern _X_EXPORT Bool DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info);
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] dri2: Reuse unused flags in GetBuffers protocol to pass last SBC

[Ian Romanick]

On 01/19/2015 06:29 AM, Chris Wilson wrote:
> Allow mesa/dri2 to implement GLX_EXT_buffer_age by reporting the sbc of
> when the current back buffer was defined. As this may require ddx
> support, only set the value if enabled by the ddx and report the new
> semantics via a DRI2GetParam request.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  configure.ac           |  2 +-
>  hw/xfree86/dri2/dri2.c | 25 +++++++++++++++++++++----
>  hw/xfree86/dri2/dri2.h |  1 +
>  3 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index b593fc7..e49c35e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -768,7 +768,7 @@ RECORDPROTO="recordproto >= 1.13.99.1"
>  SCRNSAVERPROTO="scrnsaverproto >= 1.1"
>  RESOURCEPROTO="resourceproto >= 1.2.0"
>  DRIPROTO="xf86driproto >= 2.1.0"
> -DRI2PROTO="dri2proto >= 2.8"
> +DRI2PROTO="dri2proto >= 2.9"
>  DRI3PROTO="dri3proto >= 1.0"
>  XINERAMAPROTO="xineramaproto"
>  BIGFONTPROTO="xf86bigfontproto >= 1.2.0"
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index 2c0367e..d70a632 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -49,6 +49,8 @@
>  #include "damage.h"
>  #include "xf86.h"
>  
> +#include <X11/extensions/dri2proto.h> /* for parameter names */
> +
>  CARD8 dri2_major;               /* version of DRI2 supported by DDX */
>  CARD8 dri2_minor;
>  
> @@ -115,6 +117,7 @@ typedef struct _DRI2Screen {
>      unsigned int lastSequence;
>      int prime_id;
>      int scheduleSwap0;
> +    int bufferAge;
>  
>      DRI2CreateBufferProcPtr CreateBuffer;
>      DRI2DestroyBufferProcPtr DestroyBuffer;
> @@ -1117,6 +1120,9 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
>          return BadDrawable;
>      }
>  
> +    if (ds->bufferAge)
> +        pSrcBuffer->flags = *swap_target;
> +
>      /* Old DDX or PRIME, just blit */
>      if (!ds->scheduleSwap0 || pPriv->prime_id) {
>          BoxRec box;
> @@ -1562,6 +1568,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info)
>  
>      if (info->version >= 10) {
>          ds->scheduleSwap0 = info->scheduleSwap0;
> +        ds->bufferAge = info->bufferAge;
>      }
>  
>      /*
> @@ -1684,10 +1691,20 @@ DRI2GetParam(ClientPtr client,
>  
>      switch (high_byte) {
>      case 0:
> -        /* Parameter names whose high_byte is 0 are reserved for the X
> -         * server. The server currently recognizes no parameters.
> -         */
> -        goto not_recognized;
> +	/* Parameter names whose high_byte is 0 are reserved for the X
> +	 * server.
> +	 */

Why did the whitespace change?  I don't remember what the whitespace
rules are in the server...

> +	switch (param) {
> +	case DRI2ParamXHasBufferAge:
> +	    *value = ds->bufferAge;
> +	    break;
> +	default:
> +	    goto not_recognized;
> +	}
> +
> +	*is_param_recognized = TRUE;
> +	return Success;
> +
>      case 1:
>          /* Parameter names whose high byte is 1 are reserved for the DDX. */
>          if (ds->GetParam)
> diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h
> index 1cf4288..e76f7a8 100644
> --- a/hw/xfree86/dri2/dri2.h
> +++ b/hw/xfree86/dri2/dri2.h
> @@ -255,6 +255,7 @@ typedef struct {
>  
>      /* added in version 10 */
>      int scheduleSwap0;
> +    int bufferAge;

Both fields should get added in the patch where the version is bumped,
right?

>  } DRI2InfoRec, *DRI2InfoPtr;
>  
>  extern _X_EXPORT Bool DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info);
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[xf86-video-ati] dri2: Enable BufferAge support

[Chris Wilson]

For BufferAge support, we just have to guarrantee that we were not using
the DRI2Buffer->flags field for anything else (i.e. it is always 0) and
then to make sure that we exchange the flags field after buffer
exchanges. radeon does not support TripleBuffering so we do not have to
worry about perserving the flags when injecting the third buffer.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jerome Glisse <jglisse@redhat.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Michel Dänzer <michel.daenzer@amd.com>
---
 src/radeon_dri2.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c
index 0fbe96c..091cd06 100644
--- a/src/radeon_dri2.c
+++ b/src/radeon_dri2.c
@@ -764,6 +764,11 @@ radeon_dri2_exchange_buffers(DrawablePtr draw, DRI2BufferPtr front, DRI2BufferPt
     front->name = back->name;
     back->name = tmp;
 
+    /* Swap flags so BufferAge works */
+    tmp = front->flags;
+    front->flags = back->flags;
+    back->flags = tmp;
+
     /* Swap pixmap bos */
     front_bo = radeon_get_pixmap_bo(front_priv->pixmap);
     back_bo = radeon_get_pixmap_bo(back_priv->pixmap);
@@ -1647,6 +1652,11 @@ radeon_dri2_screen_init(ScreenPtr pScreen)
     dri2_info.CopyRegion2 = radeon_dri2_copy_region2;
 #endif
 
+#if DRI2INFOREC_VERSION >= 10
+    dri2_info.version = 10;
+    dri2_info.bufferAge = 1;
+#endif
+
     info->dri2.enabled = DRI2ScreenInit(pScreen, &dri2_info);
     return info->dri2.enabled;
 }
-- 
2.1.4

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [xf86-video-ati] dri2: Enable BufferAge support

[Alex Deucher]

On Mon, Jan 19, 2015 at 6:00 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> For BufferAge support, we just have to guarrantee that we were not using
> the DRI2Buffer->flags field for anything else (i.e. it is always 0) and
> then to make sure that we exchange the flags field after buffer
> exchanges. radeon does not support TripleBuffering so we do not have to
> worry about perserving the flags when injecting the third buffer.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Jerome Glisse <jglisse@redhat.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Michel Dänzer <michel.daenzer@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  src/radeon_dri2.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c
> index 0fbe96c..091cd06 100644
> --- a/src/radeon_dri2.c
> +++ b/src/radeon_dri2.c
> @@ -764,6 +764,11 @@ radeon_dri2_exchange_buffers(DrawablePtr draw, DRI2BufferPtr front, DRI2BufferPt
>      front->name = back->name;
>      back->name = tmp;
>
> +    /* Swap flags so BufferAge works */
> +    tmp = front->flags;
> +    front->flags = back->flags;
> +    back->flags = tmp;
> +
>      /* Swap pixmap bos */
>      front_bo = radeon_get_pixmap_bo(front_priv->pixmap);
>      back_bo = radeon_get_pixmap_bo(back_priv->pixmap);
> @@ -1647,6 +1652,11 @@ radeon_dri2_screen_init(ScreenPtr pScreen)
>      dri2_info.CopyRegion2 = radeon_dri2_copy_region2;
>  #endif
>
> +#if DRI2INFOREC_VERSION >= 10
> +    dri2_info.version = 10;
> +    dri2_info.bufferAge = 1;
> +#endif
> +
>      info->dri2.enabled = DRI2ScreenInit(pScreen, &dri2_info);
>      return info->dri2.enabled;
>  }
> --
> 2.1.4
>
> _______________________________________________
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[xf86-video-nouveau] dri2: Enable BufferAge support

[Chris Wilson]

For enable BufferAge support, we just have to be not using the
DRI2Buffer->flags field for any purpose (i.e. it is always expected to
be 0, as it is now) and to be sure to swap the flags field whenever we
exchange buffers. As nouveau does not exactly support TripleBuffer, we
don't have to worry about setting the copying the flags field when
injecting the third buffer.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@ubuntu.com>
Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 src/nouveau_dri2.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
index e3445b2..428ef92 100644
--- a/src/nouveau_dri2.c
+++ b/src/nouveau_dri2.c
@@ -711,6 +711,7 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int frame,
 		}
 
 		SWAP(s->dst->name, s->src->name);
+		SWAP(s->dst->flags, s->src->flags);
 		SWAP(nouveau_pixmap(dst_pix)->bo, nouveau_pixmap(src_pix)->bo);
 
 		DamageRegionProcessPending(draw);
@@ -1003,6 +1004,12 @@ nouveau_dri2_init(ScreenPtr pScreen)
 	dri2.DestroyBuffer2 = nouveau_dri2_destroy_buffer2;
 	dri2.CopyRegion2 = nouveau_dri2_copy_region2;
 #endif
+
+#if DRI2INFOREC_VERSION >= 10
+	dri2.version = 10;
+	dri2.bufferAge = 1;
+#endif
+
 	return DRI2ScreenInit(pScreen, &dri2);
 }
 
-- 
2.1.4

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [xf86-video-nouveau] dri2: Enable BufferAge support

[Mario Kleiner]

On 01/19/2015 12:00 PM, Chris Wilson wrote:
> For enable BufferAge support, we just have to be not using the
> DRI2Buffer->flags field for any purpose (i.e. it is always expected to
> be 0, as it is now) and to be sure to swap the flags field whenever we
> exchange buffers. As nouveau does not exactly support TripleBuffer, we
> don't have to worry about setting the copying the flags field when
> injecting the third buffer.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@ubuntu.com>
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> ---
>   src/nouveau_dri2.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c
> index e3445b2..428ef92 100644
> --- a/src/nouveau_dri2.c
> +++ b/src/nouveau_dri2.c
> @@ -711,6 +711,7 @@ nouveau_dri2_finish_swap(DrawablePtr draw, unsigned int frame,
>   		}
>
>   		SWAP(s->dst->name, s->src->name);
> +		SWAP(s->dst->flags, s->src->flags);
>   		SWAP(nouveau_pixmap(dst_pix)->bo, nouveau_pixmap(src_pix)->bo);
>
>   		DamageRegionProcessPending(draw);
> @@ -1003,6 +1004,12 @@ nouveau_dri2_init(ScreenPtr pScreen)
>   	dri2.DestroyBuffer2 = nouveau_dri2_destroy_buffer2;
>   	dri2.CopyRegion2 = nouveau_dri2_copy_region2;
>   #endif
> +
> +#if DRI2INFOREC_VERSION >= 10
> +	dri2.version = 10;
> +	dri2.bufferAge = 1;
> +#endif
> +
>   	return DRI2ScreenInit(pScreen, &dri2);
>   }
>
>

Seems ok to me.

Reviewed-by: Mario Kleiner <mario.kleiner.de@gmail.com>

-mario
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[mesa 7/9] glx/dri2: Add DRI2GetParam()

[Chris Wilson]

Available since the inclusion of dri2proto 1.4 is a DRI2 request to
query and set certain parameters about the X/DDX configuration. This
implements the getter request.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 src/glx/dri2.c | 29 +++++++++++++++++++++++++++++
 src/glx/dri2.h |  4 ++++
 2 files changed, 33 insertions(+)

diff --git a/src/glx/dri2.c b/src/glx/dri2.c
index cc6c164..6d9403e 100644
--- a/src/glx/dri2.c
+++ b/src/glx/dri2.c
@@ -546,4 +546,33 @@ DRI2CopyRegion(Display * dpy, XID drawable, XserverRegion region,
    SyncHandle();
 }
 
+Bool
+DRI2GetParam(Display * dpy, XID drawable, CARD32 param, CARD64 *value)
+{
+   XExtDisplayInfo *info = DRI2FindDisplay(dpy);
+   xDRI2GetParamReply rep;
+   xDRI2GetParamReq *req;
+
+   XextCheckExtension(dpy, info, dri2ExtensionName, False);
+
+   LockDisplay(dpy);
+   GetReq(DRI2GetParam, req);
+   req->reqType = info->codes->major_opcode;
+   req->dri2ReqType = X_DRI2GetParam;
+   req->drawable = drawable;
+   req->param = param;
+
+   if (!_XReply(dpy, (xReply *) & rep, 0, xFalse)) {
+      UnlockDisplay(dpy);
+      SyncHandle();
+      return False;
+   }
+
+   *value = (CARD64)rep.value_hi << 32 | rep.value_lo;
+   UnlockDisplay(dpy);
+   SyncHandle();
+
+   return rep.is_param_recognized;
+}
+
 #endif /* GLX_DIRECT_RENDERING */
diff --git a/src/glx/dri2.h b/src/glx/dri2.h
index 4be5bf8..a5b23f0 100644
--- a/src/glx/dri2.h
+++ b/src/glx/dri2.h
@@ -88,4 +88,8 @@ DRI2CopyRegion(Display * dpy, XID drawable,
                XserverRegion region,
                CARD32 dest, CARD32 src);
 
+extern Bool
+DRI2GetParam(Display * dpy, XID drawable,
+	     CARD32 param, CARD64 *value);
+
 #endif
-- 
2.1.4

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [Mesa-dev] [mesa 7/9] glx/dri2: Add DRI2GetParam()

[Ian Romanick]

On 01/19/2015 03:00 AM, Chris Wilson wrote:
> Available since the inclusion of dri2proto 1.4 is a DRI2 request to
> query and set certain parameters about the X/DDX configuration. This
> implements the getter request.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

This patch is

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

> ---
>  src/glx/dri2.c | 29 +++++++++++++++++++++++++++++
>  src/glx/dri2.h |  4 ++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/src/glx/dri2.c b/src/glx/dri2.c
> index cc6c164..6d9403e 100644
> --- a/src/glx/dri2.c
> +++ b/src/glx/dri2.c
> @@ -546,4 +546,33 @@ DRI2CopyRegion(Display * dpy, XID drawable, XserverRegion region,
>     SyncHandle();
>  }
>  
> +Bool
> +DRI2GetParam(Display * dpy, XID drawable, CARD32 param, CARD64 *value)
> +{
> +   XExtDisplayInfo *info = DRI2FindDisplay(dpy);
> +   xDRI2GetParamReply rep;
> +   xDRI2GetParamReq *req;
> +
> +   XextCheckExtension(dpy, info, dri2ExtensionName, False);
> +
> +   LockDisplay(dpy);
> +   GetReq(DRI2GetParam, req);
> +   req->reqType = info->codes->major_opcode;
> +   req->dri2ReqType = X_DRI2GetParam;
> +   req->drawable = drawable;
> +   req->param = param;
> +
> +   if (!_XReply(dpy, (xReply *) & rep, 0, xFalse)) {
> +      UnlockDisplay(dpy);
> +      SyncHandle();
> +      return False;
> +   }
> +
> +   *value = (CARD64)rep.value_hi << 32 | rep.value_lo;
> +   UnlockDisplay(dpy);
> +   SyncHandle();
> +
> +   return rep.is_param_recognized;
> +}
> +
>  #endif /* GLX_DIRECT_RENDERING */
> diff --git a/src/glx/dri2.h b/src/glx/dri2.h
> index 4be5bf8..a5b23f0 100644
> --- a/src/glx/dri2.h
> +++ b/src/glx/dri2.h
> @@ -88,4 +88,8 @@ DRI2CopyRegion(Display * dpy, XID drawable,
>                 XserverRegion region,
>                 CARD32 dest, CARD32 src);
>  
> +extern Bool
> +DRI2GetParam(Display * dpy, XID drawable,
> +	     CARD32 param, CARD64 *value);
> +
>  #endif
> 

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[mesa 8/9] glx/dri2: Move the wait after SwapBuffers into the next GetBuffers

[Chris Wilson]

As the SBC from the reply from SwapBuffers is not used immediately and
can be easily determined by counting the new of SwapBuffers requests
made by the client, we can defer the synchronisation point to the
pending GetBuffers round trip. (We force the invalidation event in order
to require the GetBuffers round trip - we know that all X servers will
send the invalidation after SwapBuffers and that invalidation will
arrive before the SwapBuffers reply, so no extra roundtrips are forced.)

An important side-effect is demonstrated in the next patch where we can
detect when the buffers are stale before querying properties.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 src/glx/dri2_glx.c | 73 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 38 insertions(+), 35 deletions(-)

diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
index 462d560..0577804 100644
--- a/src/glx/dri2_glx.c
+++ b/src/glx/dri2_glx.c
@@ -93,6 +93,10 @@ struct dri2_drawable
    int have_back;
    int have_fake_front;
    int swap_interval;
+   int swap_pending;
+
+   xcb_dri2_swap_buffers_cookie_t swap_buffers_cookie;
+   int64_t last_swap_sbc;
 
    uint64_t previous_time;
    unsigned frames;
@@ -778,50 +782,51 @@ static void show_fps(struct dri2_drawable *draw)
    }
 }
 
-static int64_t
-dri2XcbSwapBuffers(Display *dpy,
-                  __GLXDRIdrawable *pdraw,
+static void
+dri2XcbSwapBuffers(struct dri2_drawable *priv,
                   int64_t target_msc,
                   int64_t divisor,
                   int64_t remainder)
 {
-   xcb_dri2_swap_buffers_cookie_t swap_buffers_cookie;
-   xcb_dri2_swap_buffers_reply_t *swap_buffers_reply;
+   xcb_connection_t *c = XGetXCBConnection(priv->base.psc->dpy);
    uint32_t target_msc_hi, target_msc_lo;
    uint32_t divisor_hi, divisor_lo;
    uint32_t remainder_hi, remainder_lo;
-   int64_t ret = 0;
-   xcb_connection_t *c = XGetXCBConnection(dpy);
 
    split_counter(target_msc, &target_msc_hi, &target_msc_lo);
    split_counter(divisor, &divisor_hi, &divisor_lo);
    split_counter(remainder, &remainder_hi, &remainder_lo);
 
-   swap_buffers_cookie =
-      xcb_dri2_swap_buffers_unchecked(c, pdraw->xDrawable,
+   priv->swap_buffers_cookie =
+      xcb_dri2_swap_buffers_unchecked(c, priv->base.xDrawable,
                                       target_msc_hi, target_msc_lo,
                                       divisor_hi, divisor_lo,
                                       remainder_hi, remainder_lo);
+   xcb_flush(c);
+   priv->swap_pending++;
 
-   /* Immediately wait on the swapbuffers reply.  If we didn't, we'd have
-    * to do so some time before reusing a (non-pageflipped) backbuffer.
-    * Otherwise, the new rendering could get ahead of the X Server's
-    * dispatch of the swapbuffer and you'd display garbage.
-    *
-    * We use XSync() first to reap the invalidate events through the event
-    * filter, to ensure that the next drawing doesn't use an invalidated
-    * buffer.
-    */
-   XSync(dpy, False);
+   /* Force a synchronous completion prior to the next rendering */
+   dri2InvalidateBuffers(priv->base.psc->dpy, priv->base.xDrawable);
+}
+
+static void dri2XcbSwapBuffersComplete(struct dri2_drawable *priv)
+{
+   xcb_dri2_swap_buffers_reply_t *swap_buffers_reply;
+
+   if (!priv->swap_pending)
+      return;
+
+   priv->swap_pending = 0;
 
    swap_buffers_reply =
-      xcb_dri2_swap_buffers_reply(c, swap_buffers_cookie, NULL);
-   if (swap_buffers_reply) {
-      ret = merge_counter(swap_buffers_reply->swap_hi,
-                          swap_buffers_reply->swap_lo);
-      free(swap_buffers_reply);
-   }
-   return ret;
+        xcb_dri2_swap_buffers_reply(XGetXCBConnection(priv->base.psc->dpy),
+				    priv->swap_buffers_cookie, NULL);
+   if (swap_buffers_reply == NULL)
+      return;
+
+   priv->last_swap_sbc = merge_counter(swap_buffers_reply->swap_hi,
+                                       swap_buffers_reply->swap_lo);
+   free(swap_buffers_reply);
 }
 
 static int64_t
@@ -833,11 +838,10 @@ dri2SwapBuffers(__GLXDRIdrawable *pdraw, int64_t target_msc, int64_t divisor,
     struct dri2_screen *psc = (struct dri2_screen *) priv->base.psc;
     struct dri2_display *pdp =
 	(struct dri2_display *)dpyPriv->dri2Display;
-    int64_t ret = 0;
 
     /* Check we have the right attachments */
     if (!priv->have_back)
-	return ret;
+	return 0;
 
     /* Old servers can't handle swapbuffers */
     if (!pdp->swapAvailable) {
@@ -850,19 +854,14 @@ dri2SwapBuffers(__GLXDRIdrawable *pdraw, int64_t target_msc, int64_t divisor,
           flags |= __DRI2_FLUSH_CONTEXT;
        dri2Flush(psc, ctx, priv, flags, __DRI2_THROTTLE_SWAPBUFFER);
 
-       ret = dri2XcbSwapBuffers(pdraw->psc->dpy, pdraw,
-                                target_msc, divisor, remainder);
+       dri2XcbSwapBuffers(priv, target_msc, divisor, remainder);
     }
 
     if (psc->show_fps_interval) {
        show_fps(priv);
     }
 
-    /* Old servers don't send invalidate events */
-    if (!pdp->invalidateAvailable)
-       dri2InvalidateBuffers(dpyPriv->dpy, pdraw->xDrawable);
-
-    return ret;
+    return priv->last_swap_sbc + priv->swap_pending;
 }
 
 static __DRIbuffer *
@@ -885,6 +884,8 @@ dri2GetBuffers(__DRIdrawable * driDrawable,
 
    free(buffers);
 
+   dri2XcbSwapBuffersComplete(pdraw);
+
    return pdraw->buffers;
 }
 
@@ -910,6 +911,8 @@ dri2GetBuffersWithFormat(__DRIdrawable * driDrawable,
 
    free(buffers);
 
+   dri2XcbSwapBuffersComplete(pdraw);
+
    return pdraw->buffers;
 }
 
-- 
2.1.4

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [mesa 8/9] glx/dri2: Move the wait after SwapBuffers into the next GetBuffers

[Ian Romanick]

On 01/19/2015 03:00 AM, Chris Wilson wrote:
> As the SBC from the reply from SwapBuffers is not used immediately and
> can be easily determined by counting the new of SwapBuffers requests
> made by the client, we can defer the synchronisation point to the
> pending GetBuffers round trip. (We force the invalidation event in order
> to require the GetBuffers round trip - we know that all X servers will
> send the invalidation after SwapBuffers and that invalidation will
> arrive before the SwapBuffers reply, so no extra roundtrips are forced.)

This is a pretty big change in behavior.  How much testing has it
received?  I'm nervous that applications that "work fine" today could
misbehave in subtle ways.  How much work would it be to add a way to
disable the new behavior, perhaps via an environment variable, at
run-time?  That would make it much easier for users to determine whether
this change was responsible for a problem in some game we don't have.

Additional comments in-line below.

> An important side-effect is demonstrated in the next patch where we can
> detect when the buffers are stale before querying properties.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  src/glx/dri2_glx.c | 73 ++++++++++++++++++++++++++++--------------------------
>  1 file changed, 38 insertions(+), 35 deletions(-)
> 
> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
> index 462d560..0577804 100644
> --- a/src/glx/dri2_glx.c
> +++ b/src/glx/dri2_glx.c
> @@ -93,6 +93,10 @@ struct dri2_drawable
>     int have_back;
>     int have_fake_front;
>     int swap_interval;
> +   int swap_pending;
> +
> +   xcb_dri2_swap_buffers_cookie_t swap_buffers_cookie;
> +   int64_t last_swap_sbc;
>  
>     uint64_t previous_time;
>     unsigned frames;
> @@ -778,50 +782,51 @@ static void show_fps(struct dri2_drawable *draw)
>     }
>  }
>  
> -static int64_t
> -dri2XcbSwapBuffers(Display *dpy,
> -                  __GLXDRIdrawable *pdraw,
> +static void
> +dri2XcbSwapBuffers(struct dri2_drawable *priv,
>                    int64_t target_msc,
>                    int64_t divisor,
>                    int64_t remainder)
>  {
> -   xcb_dri2_swap_buffers_cookie_t swap_buffers_cookie;
> -   xcb_dri2_swap_buffers_reply_t *swap_buffers_reply;
> +   xcb_connection_t *c = XGetXCBConnection(priv->base.psc->dpy);
>     uint32_t target_msc_hi, target_msc_lo;
>     uint32_t divisor_hi, divisor_lo;
>     uint32_t remainder_hi, remainder_lo;
> -   int64_t ret = 0;
> -   xcb_connection_t *c = XGetXCBConnection(dpy);
>  
>     split_counter(target_msc, &target_msc_hi, &target_msc_lo);
>     split_counter(divisor, &divisor_hi, &divisor_lo);
>     split_counter(remainder, &remainder_hi, &remainder_lo);
>  
> -   swap_buffers_cookie =
> -      xcb_dri2_swap_buffers_unchecked(c, pdraw->xDrawable,
> +   priv->swap_buffers_cookie =
> +      xcb_dri2_swap_buffers_unchecked(c, priv->base.xDrawable,
>                                        target_msc_hi, target_msc_lo,
>                                        divisor_hi, divisor_lo,
>                                        remainder_hi, remainder_lo);
> +   xcb_flush(c);
> +   priv->swap_pending++;
>  
> -   /* Immediately wait on the swapbuffers reply.  If we didn't, we'd have
> -    * to do so some time before reusing a (non-pageflipped) backbuffer.
> -    * Otherwise, the new rendering could get ahead of the X Server's
> -    * dispatch of the swapbuffer and you'd display garbage.
> -    *
> -    * We use XSync() first to reap the invalidate events through the event
> -    * filter, to ensure that the next drawing doesn't use an invalidated
> -    * buffer.
> -    */
> -   XSync(dpy, False);
> +   /* Force a synchronous completion prior to the next rendering */
> +   dri2InvalidateBuffers(priv->base.psc->dpy, priv->base.xDrawable);
> +}
> +
> +static void dri2XcbSwapBuffersComplete(struct dri2_drawable *priv)

static void
dri2XcbSwapBuffersComplete(struct dri2_drawable *priv)

> +{
> +   xcb_dri2_swap_buffers_reply_t *swap_buffers_reply;
> +
> +   if (!priv->swap_pending)
> +      return;
> +
> +   priv->swap_pending = 0;

Is this actually right?  dri2XcbSwapBuffers increments the count, do we
know the single reply will be for all pending swaps?

>  
>     swap_buffers_reply =
> -      xcb_dri2_swap_buffers_reply(c, swap_buffers_cookie, NULL);
> -   if (swap_buffers_reply) {
> -      ret = merge_counter(swap_buffers_reply->swap_hi,
> -                          swap_buffers_reply->swap_lo);
> -      free(swap_buffers_reply);
> -   }
> -   return ret;
> +        xcb_dri2_swap_buffers_reply(XGetXCBConnection(priv->base.psc->dpy),
> +				    priv->swap_buffers_cookie, NULL);
> +   if (swap_buffers_reply == NULL)
> +      return;
> +
> +   priv->last_swap_sbc = merge_counter(swap_buffers_reply->swap_hi,
> +                                       swap_buffers_reply->swap_lo);
> +   free(swap_buffers_reply);
>  }
>  
>  static int64_t
> @@ -833,11 +838,10 @@ dri2SwapBuffers(__GLXDRIdrawable *pdraw, int64_t target_msc, int64_t divisor,
>      struct dri2_screen *psc = (struct dri2_screen *) priv->base.psc;
>      struct dri2_display *pdp =
>  	(struct dri2_display *)dpyPriv->dri2Display;
> -    int64_t ret = 0;
>  
>      /* Check we have the right attachments */
>      if (!priv->have_back)
> -	return ret;
> +	return 0;
>  
>      /* Old servers can't handle swapbuffers */
>      if (!pdp->swapAvailable) {
> @@ -850,19 +854,14 @@ dri2SwapBuffers(__GLXDRIdrawable *pdraw, int64_t target_msc, int64_t divisor,
>            flags |= __DRI2_FLUSH_CONTEXT;
>         dri2Flush(psc, ctx, priv, flags, __DRI2_THROTTLE_SWAPBUFFER);
>  
> -       ret = dri2XcbSwapBuffers(pdraw->psc->dpy, pdraw,
> -                                target_msc, divisor, remainder);
> +       dri2XcbSwapBuffers(priv, target_msc, divisor, remainder);
>      }
>  
>      if (psc->show_fps_interval) {
>         show_fps(priv);
>      }
>  
> -    /* Old servers don't send invalidate events */
> -    if (!pdp->invalidateAvailable)
> -       dri2InvalidateBuffers(dpyPriv->dpy, pdraw->xDrawable);

It seems like this should still happen somewhere, right?  Or are we
assuming there are no more old servers anywhere (that will be used with
this)?

> -
> -    return ret;
> +    return priv->last_swap_sbc + priv->swap_pending;
>  }
>  
>  static __DRIbuffer *
> @@ -885,6 +884,8 @@ dri2GetBuffers(__DRIdrawable * driDrawable,
>  
>     free(buffers);
>  
> +   dri2XcbSwapBuffersComplete(pdraw);
> +

Are there other places that need this?  It seems like all the important
paths will eventually hit this or dri2GetBuffersWithFormat.  Will
glFinish always hit this?  Like:

    glXSwapBuffers();
    glFinish();

Hmm... that case may not even matter.

>     return pdraw->buffers;
>  }
>  
> @@ -910,6 +911,8 @@ dri2GetBuffersWithFormat(__DRIdrawable * driDrawable,
>  
>     free(buffers);
>  
> +   dri2XcbSwapBuffersComplete(pdraw);
> +
>     return pdraw->buffers;
>  }
>  

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[mesa 9/9] glx/dri2: Implement getBufferAge

[Chris Wilson]

Within the DRI2GetBuffers return packet there is a 4-byte field that is
currently unused by any driver, i.e. flags. With the co-operation of a
suitably modified X server, we can pass the last SBC on which the buffer
was defined (i.e. the last SwapBuffers for which it was used) and 0 if
it is fresh (with a slight loss of precision). We can then compare the
flags field of the DRIBuffer against the current swap buffers count and
so compute the age of the back buffer (thus satisfying
GLX_EXT_buffer_age).

As we reuse a driver specific field within the DRI2GetBuffers packet, we
first query whether the X/DDX are ready to supply the new value using a
DRI2GetParam request.

Another caveat is that we need to complete the SwapBuffers/GetBuffers
roundtrip before reporting the back buffer age so that it tallies
against the buffer used for rendering. As with all things X, there is a
race between the query and the rendering where the buffer may be
invalidated by the server. However, for the primary usecase (that of a
compositing manager), the DRI2Drawable is only accessible to a single
client mitigating the impact of the issue.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 configure.ac       |  2 +-
 src/glx/dri2_glx.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 870435c..ca1da86 100644
--- a/configure.ac
+++ b/configure.ac
@@ -65,7 +65,7 @@ LIBDRM_INTEL_REQUIRED=2.4.52
 LIBDRM_NVVIEUX_REQUIRED=2.4.33
 LIBDRM_NOUVEAU_REQUIRED="2.4.33 libdrm >= 2.4.41"
 LIBDRM_FREEDRENO_REQUIRED=2.4.57
-DRI2PROTO_REQUIRED=2.6
+DRI2PROTO_REQUIRED=2.9
 DRI3PROTO_REQUIRED=1.0
 PRESENTPROTO_REQUIRED=1.0
 LIBUDEV_REQUIRED=151
diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
index 0577804..b43f115 100644
--- a/src/glx/dri2_glx.c
+++ b/src/glx/dri2_glx.c
@@ -917,6 +917,67 @@ dri2GetBuffersWithFormat(__DRIdrawable * driDrawable,
 }
 
 static int
+dri2HasBufferAge(int screen, struct glx_display * priv)
+{
+   const struct dri2_display *const pdp =
+	   (struct dri2_display *)priv->dri2Display;
+   CARD64 value;
+
+   if (pdp->driMajor <= 1 && pdp->driMinor < 4)
+	   return 0;
+
+   value = 0;
+   if (!DRI2GetParam(priv->dpy, RootWindow(priv->dpy, screen),
+                     DRI2ParamXHasBufferAge, &value))
+      return 0;
+
+   return value;
+}
+
+static int
+dri2GetBufferAge(__GLXDRIdrawable *pdraw)
+{
+   struct dri2_drawable *priv = (struct dri2_drawable *) pdraw;
+   int i, age = 0;
+
+   if (priv->swap_pending) {
+	   unsigned int attachments[5];
+	   DRI2Buffer *buffers;
+
+	   for (i = 0; i < priv->bufferCount; i++)
+		   attachments[i] = priv->buffers[i].attachment;
+
+	   buffers = DRI2GetBuffers(priv->base.psc->dpy, priv->base.xDrawable,
+				    &priv->width, &priv->height,
+				    attachments, i, &i);
+	   if (buffers == NULL)
+		   return 0;
+
+	   process_buffers(priv, buffers, i);
+	   free(buffers);
+
+	   dri2XcbSwapBuffersComplete(priv);
+   }
+
+   if (!priv->have_back)
+      return 0;
+
+   for (i = 0; i < priv->bufferCount; i++) {
+	   if (priv->buffers[i].attachment != __DRI_BUFFER_BACK_LEFT)
+		   continue;
+
+	   if (priv->buffers[i].flags == 0)
+		   continue;
+
+	   age = priv->last_swap_sbc - priv->buffers[i].flags + 1;
+	   if (age < 0)
+		   age = 0;
+   }
+
+   return age;
+}
+
+static int
 dri2SetSwapInterval(__GLXDRIdrawable *pdraw, int interval)
 {
    xcb_connection_t *c = XGetXCBConnection(pdraw->psc->dpy);
@@ -1290,6 +1351,10 @@ dri2CreateScreen(int screen, struct glx_display * priv)
    psp->setSwapInterval = NULL;
    psp->getSwapInterval = NULL;
    psp->getBufferAge = NULL;
+   if (dri2HasBufferAge(screen, priv)) {
+	   psp->getBufferAge = dri2GetBufferAge;
+	   __glXEnableDirectExtension(&psc->base, "GLX_EXT_buffer_age");
+   }
 
    if (pdp->driMinor >= 2) {
       psp->getDrawableMSC = dri2DrawableGetMSC;
-- 
2.1.4

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [mesa 9/9] glx/dri2: Implement getBufferAge

[Ian Romanick]

On 01/19/2015 03:00 AM, Chris Wilson wrote:
> Within the DRI2GetBuffers return packet there is a 4-byte field that is
> currently unused by any driver, i.e. flags. With the co-operation of a
> suitably modified X server, we can pass the last SBC on which the buffer
> was defined (i.e. the last SwapBuffers for which it was used) and 0 if
> it is fresh (with a slight loss of precision). We can then compare the
> flags field of the DRIBuffer against the current swap buffers count and
> so compute the age of the back buffer (thus satisfying
> GLX_EXT_buffer_age).
> 
> As we reuse a driver specific field within the DRI2GetBuffers packet, we
> first query whether the X/DDX are ready to supply the new value using a
> DRI2GetParam request.
> 
> Another caveat is that we need to complete the SwapBuffers/GetBuffers
> roundtrip before reporting the back buffer age so that it tallies
> against the buffer used for rendering. As with all things X, there is a
> race between the query and the rendering where the buffer may be
> invalidated by the server. However, for the primary usecase (that of a
> compositing manager), the DRI2Drawable is only accessible to a single
> client mitigating the impact of the issue.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  configure.ac       |  2 +-
>  src/glx/dri2_glx.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 870435c..ca1da86 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -65,7 +65,7 @@ LIBDRM_INTEL_REQUIRED=2.4.52
>  LIBDRM_NVVIEUX_REQUIRED=2.4.33
>  LIBDRM_NOUVEAU_REQUIRED="2.4.33 libdrm >= 2.4.41"
>  LIBDRM_FREEDRENO_REQUIRED=2.4.57
> -DRI2PROTO_REQUIRED=2.6
> +DRI2PROTO_REQUIRED=2.9
>  DRI3PROTO_REQUIRED=1.0
>  PRESENTPROTO_REQUIRED=1.0
>  LIBUDEV_REQUIRED=151
> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
> index 0577804..b43f115 100644
> --- a/src/glx/dri2_glx.c
> +++ b/src/glx/dri2_glx.c
> @@ -917,6 +917,67 @@ dri2GetBuffersWithFormat(__DRIdrawable * driDrawable,
>  }
>  
>  static int
> +dri2HasBufferAge(int screen, struct glx_display * priv)
> +{
> +   const struct dri2_display *const pdp =
> +	   (struct dri2_display *)priv->dri2Display;
> +   CARD64 value;
> +
> +   if (pdp->driMajor <= 1 && pdp->driMinor < 4)
> +	   return 0;
> +
> +   value = 0;
> +   if (!DRI2GetParam(priv->dpy, RootWindow(priv->dpy, screen),
> +                     DRI2ParamXHasBufferAge, &value))
> +      return 0;
> +
> +   return value;
> +}
> +
> +static int
> +dri2GetBufferAge(__GLXDRIdrawable *pdraw)
> +{
> +   struct dri2_drawable *priv = (struct dri2_drawable *) pdraw;
> +   int i, age = 0;
> +
> +   if (priv->swap_pending) {
> +	   unsigned int attachments[5];

I see other callers that have attachments of at least 8 (although it
appears that intel_query_dri2_buffers only needs 2).  Could we at least
get an assertion or something that priv->bufferCount <=
ARRAY_SIZE(attachments)?  A (hypothetical) driver doing stereo rendering
with separate, DDX managed, depth and stencil buffers would need 6.  A
(again, hypothetical) driver with AUX buffers could need... more.

> +	   DRI2Buffer *buffers;
> +
> +	   for (i = 0; i < priv->bufferCount; i++)
> +		   attachments[i] = priv->buffers[i].attachment;
> +
> +	   buffers = DRI2GetBuffers(priv->base.psc->dpy, priv->base.xDrawable,
> +				    &priv->width, &priv->height,
> +				    attachments, i, &i);

Most drivers prefer DRI2GetBuffersWithFormat, and some drivers only use
DRI2GetBuffersWithFormat.  Is mixing DRI2GetBuffersWithFormat and
DRI2GetBuffers going to cause problems or unexpected behavior changes?

> +	   if (buffers == NULL)
> +		   return 0;
> +
> +	   process_buffers(priv, buffers, i);
> +	   free(buffers);
> +
> +	   dri2XcbSwapBuffersComplete(priv);
> +   }
> +
> +   if (!priv->have_back)
> +      return 0;
> +
> +   for (i = 0; i < priv->bufferCount; i++) {
> +	   if (priv->buffers[i].attachment != __DRI_BUFFER_BACK_LEFT)
> +		   continue;
> +
> +	   if (priv->buffers[i].flags == 0)
> +		   continue;
> +
> +	   age = priv->last_swap_sbc - priv->buffers[i].flags + 1;
> +	   if (age < 0)
> +		   age = 0;

I was going to comment that this looked like it calculated age wrong
when the buffers had different ages.  Then I realized that age should
only be calculated once.  I think this would be more obvious if the body
of the loop were:

      if (priv->buffers[i].attachment == __DRI_BUFFER_BACK_LEFT &&
          priv->buffers[i].flags != 0) {
         age = priv->last_swap_sbc - priv->buffers[i].flags + 1;
         if (age < 0)
            age = 0;

         break;
      }

I also just noticed that your patches are mixing tabs and spaces (use
spaces only) and are using a mix of 3-space and 8-space (maybe?) indents
(use 3 spaces only).

> +   }
> +
> +   return age;
> +}
> +
> +static int
>  dri2SetSwapInterval(__GLXDRIdrawable *pdraw, int interval)
>  {
>     xcb_connection_t *c = XGetXCBConnection(pdraw->psc->dpy);
> @@ -1290,6 +1351,10 @@ dri2CreateScreen(int screen, struct glx_display * priv)
>     psp->setSwapInterval = NULL;
>     psp->getSwapInterval = NULL;
>     psp->getBufferAge = NULL;

Blank line here.

> +   if (dri2HasBufferAge(screen, priv)) {
> +	   psp->getBufferAge = dri2GetBufferAge;
> +	   __glXEnableDirectExtension(&psc->base, "GLX_EXT_buffer_age");
> +   }
>  
>     if (pdp->driMinor >= 2) {
>        psp->getDrawableMSC = dri2DrawableGetMSC;
> 

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [mesa 9/9] glx/dri2: Implement getBufferAge

[Ian Romanick]

On 01/20/2015 12:35 PM, Ian Romanick wrote:
> On 01/19/2015 03:00 AM, Chris Wilson wrote:
>> +	   DRI2Buffer *buffers;
>> +
>> +	   for (i = 0; i < priv->bufferCount; i++)
>> +		   attachments[i] = priv->buffers[i].attachment;
>> +
>> +	   buffers = DRI2GetBuffers(priv->base.psc->dpy, priv->base.xDrawable,
>> +				    &priv->width, &priv->height,
>> +				    attachments, i, &i);
> 
> Most drivers prefer DRI2GetBuffersWithFormat, and some drivers only use
> DRI2GetBuffersWithFormat.  Is mixing DRI2GetBuffersWithFormat and
> DRI2GetBuffers going to cause problems or unexpected behavior changes?

Okay... I hadn't seen the server change until after I sent this review.
 I sent some comments there.

This should be fine, but can we get a comment that it relies on
DRI2GetBuffers re-using the format from the previous
DRI2GetBuffersWithFormat?  That way the next person to look at the code
won't make the same mistake I made.

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: Implement GLX_EXT_buffer_age for DRI2

[Dave Airlie]

On 19 January 2015 at 21:00, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> In order to suport GLX_EXT_buffer_age in DRI2, we need to pass back the
> last swap buffer count that the back buffer was defined for. For
> simplicity, we can reuse an existing field in the DRI2GetBuffers reply
> that is not used by current drivers, the flags. Since we change the
> interpretation of this flag, we also declare the semantic change with a
> DRI2 parameter and depend upon the DDX to enable the change
> responsibility (which is just a matter of reviewing whether the flags
> field has ever been used for a non-zero value).
>

This is just missing the why do we need to add this to DRI2 when we
have DRI3/Present that should be solving it.

Doesn't dri3 already do this?

Dave.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Implement GLX_EXT_buffer_age for DRI2

[Daniel Stone]

Hi,

On 20 January 2015 at 21:49, Dave Airlie <airlied@gmail.com> wrote:
> On 19 January 2015 at 21:00, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> In order to suport GLX_EXT_buffer_age in DRI2, we need to pass back the
>> last swap buffer count that the back buffer was defined for. For
>> simplicity, we can reuse an existing field in the DRI2GetBuffers reply
>> that is not used by current drivers, the flags. Since we change the
>> interpretation of this flag, we also declare the semantic change with a
>> DRI2 parameter and depend upon the DDX to enable the change
>> responsibility (which is just a matter of reviewing whether the flags
>> field has ever been used for a non-zero value).
>
> This is just missing the why do we need to add this to DRI2 when we
> have DRI3/Present that should be solving it.
>
> Doesn't dri3 already do this?

DRI3/Present still seem to be pretty unstable and break a bunch of
stuff, and buffer_age is definitely a useful optimisation for non-TBDR
platforms, so I don't see why not.

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel