Re: [Mesa-dev] [PATCH mesa 1/3] vulkan: Define new VK_MESA_query_timestamp extension [v3]

2018-07-10 Thread Jason Ekstrand
On Tue, Jul 10, 2018 at 9:47 AM Jason Ekstrand  wrote:

> On Mon, Jul 9, 2018 at 5:35 PM Keith Packard  wrote:
>
>> This extension adds a single function to query the current GPU
>> timestamp, just like glGetInteger64v(GL_TIMESTAMP, ). This
>> function is needed to complete the implementation of
>> GOOGLE_display_timing, which needs to be able to correlate GPU and CPU
>> timestamps.
>>
>> v2: Adopt Jason Ekstrand's coding conventions
>>
>> Declare variables at first use, eliminate extra whitespace between
>> types and names. Wrap lines to 80 columns.
>>
>> Add extension to list in alphabetical order
>>
>> Suggested-by: Jason Ekstrand 
>>
>> v3: Report both device and surface timestamps
>>
>> This will allow us to get timestamps more closely aligned by
>> getting both in a single call from the kernel.
>>
>> To make this independent of the timebase used by the WSI
>> layer, provide a new wsi hook which converts CLOCK_MONOTONIC
>> into the matching WSI timebase. Right now, all of our backends
>> use CLOCK_MONOTONIC, so there's nothing actually doing
>> conversions, but it seemed best to put the infrastructure in
>> place so that I could validate the extension interface would
>> work if that became necessary.
>>
>> Signed-off-by: Keith Packard 
>> ---
>>  include/vulkan/vk_mesa_query_timestamp.h | 46 
>>  src/vulkan/registry/vk.xml   | 20 +++
>>  src/vulkan/wsi/wsi_common.c  | 17 +
>>  src/vulkan/wsi/wsi_common.h  |  7 
>>  src/vulkan/wsi/wsi_common_private.h  |  4 +++
>>  5 files changed, 94 insertions(+)
>>  create mode 100644 include/vulkan/vk_mesa_query_timestamp.h
>>
>> diff --git a/include/vulkan/vk_mesa_query_timestamp.h
>> b/include/vulkan/vk_mesa_query_timestamp.h
>> new file mode 100644
>> index 000..262f094db27
>> --- /dev/null
>> +++ b/include/vulkan/vk_mesa_query_timestamp.h
>> @@ -0,0 +1,46 @@
>> +/*
>> + * Copyright © 2018 Keith Packard
>> + *
>> + * Permission to use, copy, modify, distribute, and sell this software
>> and its
>> + * documentation for any purpose is hereby granted without fee, provided
>> that
>> + * the above copyright notice appear in all copies and that both that
>> copyright
>> + * notice and this permission notice appear in supporting documentation,
>> and
>> + * that the name of the copyright holders not be used in advertising or
>> + * publicity pertaining to distribution of the software without specific,
>> + * written prior permission.  The copyright holders make no
>> representations
>> + * about the suitability of this software for any purpose.  It is
>> provided "as
>> + * is" without express or implied warranty.
>> + *
>> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
>> SOFTWARE,
>> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
>> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT
>> OR
>> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
>> OF USE,
>> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
>> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
>> PERFORMANCE
>> + * OF THIS SOFTWARE.
>> + */
>> +
>> +#ifndef __VK_MESA_QUERY_TIMESTAMP_H__
>> +#define __VK_MESA_QUERY_TIMESTAMP_H__
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +typedef struct VkCurrentTimestampMESA {
>> +uint64_tdeviceTimestamp;
>> +uint64_tsurfaceTimestamp;
>> +} VkCurrentTimestampMESA;
>> +
>> +typedef VkResult (VKAPI_PTR *PFN_vkQueryCurrentTimestampMESA)(
>> +VkDevice device, VkSurfaceKHR surface, VkCurrentTimestampMESA
>> *timestamp);
>> +
>> +VKAPI_ATTR VkResult VKAPI_CALL vkQueryCurrentTimestampMESA(
>> +VkDevice device, VkSurfaceKHR surface, VkCurrentTimestampMESA
>> *timestamp);
>>
>
> I hate to salami extensions too much but it might be better if this were
> two extensions.  One would be
>
> typedef struct VkCurrentTimestampMESA {
> VkStructType sType;
> void *pNext;
> uint64_t timestamp;
> } VkCurrentTimestampMESA;
>
> VkResult vkGetCurrentTimestampMESA(VkDevice device, VkBaseInStructureKHR
> *pInfo, VkCurrentTimestampMESA *pTimestamp);
>
> And the second would just define two chain in structs
>
> typedef struct VkCurrentPresentTimestampGetInfoMESA {
> VkStructureType sType;
> void *pNext;
> VkSurfaceKHR surface;
> } VkCurrentPresentTimestampGetInfoMESA;
>
> typedef struct VkCurrentPresentTimestampMESA {
> VkStructureType sType;
> void *pNext;
> uint64_t presentTimestamp;
> } VkCurrentPresentTimestampMESA;
>
> You could probably also just make the surface part of
> VkCurrentPresentTimestampMESA and not have the extendible input struct.
> That might be a bit cleaner in some ways.  If the vkGetCurrentTimestampMESA
> call is extended to get the surface timestamp, the 

Re: [Mesa-dev] [PATCH mesa 1/3] vulkan: Define new VK_MESA_query_timestamp extension [v3]

2018-07-10 Thread Jason Ekstrand
On Mon, Jul 9, 2018 at 5:35 PM Keith Packard  wrote:

> This extension adds a single function to query the current GPU
> timestamp, just like glGetInteger64v(GL_TIMESTAMP, ). This
> function is needed to complete the implementation of
> GOOGLE_display_timing, which needs to be able to correlate GPU and CPU
> timestamps.
>
> v2: Adopt Jason Ekstrand's coding conventions
>
> Declare variables at first use, eliminate extra whitespace between
> types and names. Wrap lines to 80 columns.
>
> Add extension to list in alphabetical order
>
> Suggested-by: Jason Ekstrand 
>
> v3: Report both device and surface timestamps
>
> This will allow us to get timestamps more closely aligned by
> getting both in a single call from the kernel.
>
> To make this independent of the timebase used by the WSI
> layer, provide a new wsi hook which converts CLOCK_MONOTONIC
> into the matching WSI timebase. Right now, all of our backends
> use CLOCK_MONOTONIC, so there's nothing actually doing
> conversions, but it seemed best to put the infrastructure in
> place so that I could validate the extension interface would
> work if that became necessary.
>
> Signed-off-by: Keith Packard 
> ---
>  include/vulkan/vk_mesa_query_timestamp.h | 46 
>  src/vulkan/registry/vk.xml   | 20 +++
>  src/vulkan/wsi/wsi_common.c  | 17 +
>  src/vulkan/wsi/wsi_common.h  |  7 
>  src/vulkan/wsi/wsi_common_private.h  |  4 +++
>  5 files changed, 94 insertions(+)
>  create mode 100644 include/vulkan/vk_mesa_query_timestamp.h
>
> diff --git a/include/vulkan/vk_mesa_query_timestamp.h
> b/include/vulkan/vk_mesa_query_timestamp.h
> new file mode 100644
> index 000..262f094db27
> --- /dev/null
> +++ b/include/vulkan/vk_mesa_query_timestamp.h
> @@ -0,0 +1,46 @@
> +/*
> + * Copyright © 2018 Keith Packard
> + *
> + * Permission to use, copy, modify, distribute, and sell this software
> and its
> + * documentation for any purpose is hereby granted without fee, provided
> that
> + * the above copyright notice appear in all copies and that both that
> copyright
> + * notice and this permission notice appear in supporting documentation,
> and
> + * that the name of the copyright holders not be used in advertising or
> + * publicity pertaining to distribution of the software without specific,
> + * written prior permission.  The copyright holders make no
> representations
> + * about the suitability of this software for any purpose.  It is
> provided "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT
> OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF
> USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
> PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +
> +#ifndef __VK_MESA_QUERY_TIMESTAMP_H__
> +#define __VK_MESA_QUERY_TIMESTAMP_H__
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +typedef struct VkCurrentTimestampMESA {
> +uint64_tdeviceTimestamp;
> +uint64_tsurfaceTimestamp;
> +} VkCurrentTimestampMESA;
> +
> +typedef VkResult (VKAPI_PTR *PFN_vkQueryCurrentTimestampMESA)(
> +VkDevice device, VkSurfaceKHR surface, VkCurrentTimestampMESA
> *timestamp);
> +
> +VKAPI_ATTR VkResult VKAPI_CALL vkQueryCurrentTimestampMESA(
> +VkDevice device, VkSurfaceKHR surface, VkCurrentTimestampMESA
> *timestamp);
>

I hate to salami extensions too much but it might be better if this were
two extensions.  One would be

typedef struct VkCurrentTimestampMESA {
VkStructType sType;
void *pNext;
uint64_t timestamp;
} VkCurrentTimestampMESA;

VkResult vkGetCurrentTimestampMESA(VkDevice device, VkBaseInStructureKHR
*pInfo, VkCurrentTimestampMESA *pTimestamp);

And the second would just define two chain in structs

typedef struct VkCurrentPresentTimestampGetInfoMESA {
VkStructureType sType;
void *pNext;
VkSurfaceKHR surface;
} VkCurrentPresentTimestampGetInfoMESA;

typedef struct VkCurrentPresentTimestampMESA {
VkStructureType sType;
void *pNext;
uint64_t presentTimestamp;
} VkCurrentPresentTimestampMESA;

You could probably also just make the surface part of
VkCurrentPresentTimestampMESA and not have the extendible input struct.
That might be a bit cleaner in some ways.  If the vkGetCurrentTimestampMESA
call is extended to get the surface timestamp, the driver would have the
option to call the kernel for the more accurate thing or it could just
return the two separate things that your implementation already does.

The reason I suggest this is that Vulkan 

[Mesa-dev] [PATCH mesa 1/3] vulkan: Define new VK_MESA_query_timestamp extension [v3]

2018-07-09 Thread Keith Packard
This extension adds a single function to query the current GPU
timestamp, just like glGetInteger64v(GL_TIMESTAMP, ). This
function is needed to complete the implementation of
GOOGLE_display_timing, which needs to be able to correlate GPU and CPU
timestamps.

v2: Adopt Jason Ekstrand's coding conventions

Declare variables at first use, eliminate extra whitespace between
types and names. Wrap lines to 80 columns.

Add extension to list in alphabetical order

Suggested-by: Jason Ekstrand 

v3: Report both device and surface timestamps

This will allow us to get timestamps more closely aligned by
getting both in a single call from the kernel.

To make this independent of the timebase used by the WSI
layer, provide a new wsi hook which converts CLOCK_MONOTONIC
into the matching WSI timebase. Right now, all of our backends
use CLOCK_MONOTONIC, so there's nothing actually doing
conversions, but it seemed best to put the infrastructure in
place so that I could validate the extension interface would
work if that became necessary.

Signed-off-by: Keith Packard 
---
 include/vulkan/vk_mesa_query_timestamp.h | 46 
 src/vulkan/registry/vk.xml   | 20 +++
 src/vulkan/wsi/wsi_common.c  | 17 +
 src/vulkan/wsi/wsi_common.h  |  7 
 src/vulkan/wsi/wsi_common_private.h  |  4 +++
 5 files changed, 94 insertions(+)
 create mode 100644 include/vulkan/vk_mesa_query_timestamp.h

diff --git a/include/vulkan/vk_mesa_query_timestamp.h 
b/include/vulkan/vk_mesa_query_timestamp.h
new file mode 100644
index 000..262f094db27
--- /dev/null
+++ b/include/vulkan/vk_mesa_query_timestamp.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright © 2018 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#ifndef __VK_MESA_QUERY_TIMESTAMP_H__
+#define __VK_MESA_QUERY_TIMESTAMP_H__
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+typedef struct VkCurrentTimestampMESA {
+uint64_tdeviceTimestamp;
+uint64_tsurfaceTimestamp;
+} VkCurrentTimestampMESA;
+
+typedef VkResult (VKAPI_PTR *PFN_vkQueryCurrentTimestampMESA)(
+VkDevice device, VkSurfaceKHR surface, VkCurrentTimestampMESA *timestamp);
+
+VKAPI_ATTR VkResult VKAPI_CALL vkQueryCurrentTimestampMESA(
+VkDevice device, VkSurfaceKHR surface, VkCurrentTimestampMESA *timestamp);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __VK_MESA_QUERY_TIMESTAMP_H__ */
+
diff --git a/src/vulkan/registry/vk.xml b/src/vulkan/registry/vk.xml
index 4419c6fbf96..9c5a2f79398 100644
--- a/src/vulkan/registry/vk.xml
+++ b/src/vulkan/registry/vk.xml
@@ -3198,6 +3198,10 @@ server.
 VkBool32   
conditionalRendering
 VkBool32   
inheritedConditionalRendering
 
+   
+ uint64_t 
deviceTimestamp
+ uint64_t
surfaceTimestamp
+   
 
 
 Vulkan enumerant (token) definitions
@@ -6239,6 +6243,12 @@ server.
 uint32_t maxDrawCount
 uint32_t stride
 
+
+VkResult 
vkQueryCurrentTimestampMESA
+VkDevice device
+   VkSurfaceKHR surface
+VkCurrentTimestampMESA* 
pTimestamp
+
 
 
 
@@ -9008,5 +9018,15 @@ server.
 
 
 
+
+
+
+
+
+
+
 
 
diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
index f2d90a6bba2..9316470ad20 100644
--- a/src/vulkan/wsi/wsi_common.c
+++ b/src/vulkan/wsi/wsi_common.c
@@ -975,3 +975,20 @@ wsi_common_queue_present(const struct wsi_device *wsi,
 
return final_result;
 }
+
+VkResult
+wsi_common_convert_timestamp(const struct wsi_device 

[Mesa-dev] [PATCH mesa 1/3] vulkan: Define new VK_MESA_query_timestamp extension [v3]

2018-06-27 Thread Keith Packard
This extension adds a single function to query the current GPU
timestamp, just like glGetInteger64v(GL_TIMESTAMP, ). This
function is needed to complete the implementation of
GOOGLE_display_timing, which needs to be able to correlate GPU and CPU
timestamps.

v2: Adopt Jason Ekstrand's coding conventions

Declare variables at first use, eliminate extra whitespace between
types and names. Wrap lines to 80 columns.

Add extension to list in alphabetical order

Suggested-by: Jason Ekstrand 

v3: Report both device and surface timestamps

This will allow us to get timestamps more closely aligned by
getting both in a single call from the kernel.

To make this independent of the timebase used by the WSI
layer, provide a new wsi hook which converts CLOCK_MONOTONIC
into the matching WSI timebase. Right now, all of our backends
use CLOCK_MONOTONIC, so there's nothing actually doing
conversions, but it seemed best to put the infrastructure in
place so that I could validate the extension interface would
work if that became necessary.

Signed-off-by: Keith Packard 
---
 include/vulkan/vk_mesa_query_timestamp.h | 46 
 src/vulkan/registry/vk.xml   | 20 +++
 src/vulkan/wsi/wsi_common.c  | 17 +
 src/vulkan/wsi/wsi_common.h  |  7 
 src/vulkan/wsi/wsi_common_private.h  |  4 +++
 5 files changed, 94 insertions(+)
 create mode 100644 include/vulkan/vk_mesa_query_timestamp.h

diff --git a/include/vulkan/vk_mesa_query_timestamp.h 
b/include/vulkan/vk_mesa_query_timestamp.h
new file mode 100644
index 000..262f094db27
--- /dev/null
+++ b/include/vulkan/vk_mesa_query_timestamp.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright © 2018 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#ifndef __VK_MESA_QUERY_TIMESTAMP_H__
+#define __VK_MESA_QUERY_TIMESTAMP_H__
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+typedef struct VkCurrentTimestampMESA {
+uint64_tdeviceTimestamp;
+uint64_tsurfaceTimestamp;
+} VkCurrentTimestampMESA;
+
+typedef VkResult (VKAPI_PTR *PFN_vkQueryCurrentTimestampMESA)(
+VkDevice device, VkSurfaceKHR surface, VkCurrentTimestampMESA *timestamp);
+
+VKAPI_ATTR VkResult VKAPI_CALL vkQueryCurrentTimestampMESA(
+VkDevice device, VkSurfaceKHR surface, VkCurrentTimestampMESA *timestamp);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __VK_MESA_QUERY_TIMESTAMP_H__ */
+
diff --git a/src/vulkan/registry/vk.xml b/src/vulkan/registry/vk.xml
index 7018bbe8421..e7a0c657724 100644
--- a/src/vulkan/registry/vk.xml
+++ b/src/vulkan/registry/vk.xml
@@ -3106,6 +3106,10 @@ server.
 void*  
pNext
 uint64_t   
externalFormat
 
+   
+ uint64_t 
deviceTimestamp
+ uint64_t
surfaceTimestamp
+   
 
 
 Vulkan enumerant (token) definitions
@@ -6102,6 +6106,12 @@ server.
 uint32_t maxDrawCount
 uint32_t stride
 
+
+VkResult 
vkQueryCurrentTimestampMESA
+VkDevice device
+   VkSurfaceKHR surface
+VkCurrentTimestampMESA* 
pTimestamp
+
 
 
 
@@ -8826,5 +8836,15 @@ server.
 
 
 
+
+
+
+
+
+
+
 
 
diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
index f2d90a6bba2..9316470ad20 100644
--- a/src/vulkan/wsi/wsi_common.c
+++ b/src/vulkan/wsi/wsi_common.c
@@ -975,3 +975,20 @@ wsi_common_queue_present(const struct wsi_device *wsi,
 
return final_result;
 }
+
+VkResult
+wsi_common_convert_timestamp(const struct wsi_device *wsi,
+