This extension fetches the current GPU timestamp from the hardware, just like the OpenGL API glGetInteger64v(GL_TIMESTAMP, ×tamp) function.
I need this to correlate GPU and CPU timestamps for the GOOGLE_display_timing extension, but I think it will be useful for applications as well.
I'm not sure this is exactly the API we need for this; it might be better for it to return *two* timestamps, a GPU and a CPU one which were as closely correlated as possible down in the kernel.
The kernel APIs that this calls on anv and radv don't do that, so I didn't want to pretend to offer functionality I couldn't actually supply.
Suggestions on what to do here are welcome!
-keith
This extension adds a single function to query the current GPU timestamp, just like glGetInteger64v(GL_TIMESTAMP, ×tamp). 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 jason.ekstrand@intel.com
Signed-off-by: Keith Packard keithp@keithp.com --- include/vulkan/vk_mesa_query_timestamp.h | 41 ++++++++++++++++++++++++ src/vulkan/registry/vk.xml | 15 +++++++++ 2 files changed, 56 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 00000000000..4e0b50cb9cc --- /dev/null +++ b/include/vulkan/vk_mesa_query_timestamp.h @@ -0,0 +1,41 @@ +/* + * 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 VkResult (VKAPI_PTR *PFN_vkQueryCurrentTimestampMESA)( + VkDevice device, uint64_t *timestamp); + +VKAPI_ATTR VkResult VKAPI_CALL vkQueryCurrentTimestampMESA( + VkDevice device, uint64_t *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..3a5cecdc8e3 100644 --- a/src/vulkan/registry/vk.xml +++ b/src/vulkan/registry/vk.xml @@ -6102,6 +6102,11 @@ server. <param><type>uint32_t</type> <name>maxDrawCount</name></param> <param><type>uint32_t</type> <name>stride</name></param> </command> + <command> + <proto><type>VkResult</type> <name>vkQueryCurrentTimestampMESA</name></proto> + <param><type>VkDevice</type> <name>device</name></param> + <param><type>uint64_t</type>* <name>pTimestamp</name></param> + </command> </commands>
<feature api="vulkan" name="VK_VERSION_1_0" number="1.0" comment="Vulkan core API interface definitions"> @@ -8826,5 +8831,15 @@ server. <enum value=""VK_KHR_extension_211"" name="VK_KHR_EXTENSION_211_EXTENSION_NAME"/> </require> </extension> + <extension name="VK_MESA_query_timestamp" number="212" + type="device" author="MESA" + contact="Keith Packard @keithp" + supported="vulkan"> + <require> + <enum value="1" name="VK_MESA_QUERY_TIMESTAMP_SPEC_VERSION"/> + <enum value=""VK_MESA_query_timestamp"" name="VK_MESA_QUERY_TIMESTAMP_NAME"/> + <command name="vkQueryCurrentTimestampMESA"/> + </require> + </extension> </extensions> </registry>
I haven't thought through this comment all that hard but would it make sense to have three timestamps, CPU, GPU, CPU so that you have error bars on the GPU timestamp? At the very least, two timestamps would be better than one so that, when we pull it into the kernel, it can provide something more accurate than userspace trying to grab a snapshot.
--Jason
On June 23, 2018 10:15:34 Keith Packard keithp@keithp.com wrote:
This extension adds a single function to query the current GPU timestamp, just like glGetInteger64v(GL_TIMESTAMP, ×tamp). 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 jason.ekstrand@intel.com
Signed-off-by: Keith Packard keithp@keithp.com
include/vulkan/vk_mesa_query_timestamp.h | 41 ++++++++++++++++++++++++ src/vulkan/registry/vk.xml | 15 +++++++++ 2 files changed, 56 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 00000000000..4e0b50cb9cc --- /dev/null +++ b/include/vulkan/vk_mesa_query_timestamp.h @@ -0,0 +1,41 @@ +/*
- 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 VkResult (VKAPI_PTR *PFN_vkQueryCurrentTimestampMESA)(
- VkDevice device, uint64_t *timestamp);
+VKAPI_ATTR VkResult VKAPI_CALL vkQueryCurrentTimestampMESA(
- VkDevice device, uint64_t *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..3a5cecdc8e3 100644 --- a/src/vulkan/registry/vk.xml +++ b/src/vulkan/registry/vk.xml @@ -6102,6 +6102,11 @@ server. <param><type>uint32_t</type> <name>maxDrawCount</name></param> <param><type>uint32_t</type> <name>stride</name></param> </command>
<command><proto><type>VkResult</type><name>vkQueryCurrentTimestampMESA</name></proto>
<param><type>VkDevice</type> <name>device</name></param><param><type>uint64_t</type>* <name>pTimestamp</name></param></command></commands>
<feature api="vulkan" name="VK_VERSION_1_0" number="1.0" comment="Vulkan core API interface definitions">
@@ -8826,5 +8831,15 @@ server. <enum value=""VK_KHR_extension_211"" name="VK_KHR_EXTENSION_211_EXTENSION_NAME"/> </require> </extension>
<extension name="VK_MESA_query_timestamp" number="212"type="device" author="MESA"contact="Keith Packard @keithp"supported="vulkan"><require><enum value="1"name="VK_MESA_QUERY_TIMESTAMP_SPEC_VERSION"/>
<enum value=""VK_MESA_query_timestamp""name="VK_MESA_QUERY_TIMESTAMP_NAME"/>
<command name="vkQueryCurrentTimestampMESA"/></require></extension></extensions>
</registry> -- 2.17.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Sat, 23 Jun 2018 12:13:53 -0500 Jason Ekstrand jason@jlekstrand.net wrote:
I haven't thought through this comment all that hard but would it make sense to have three timestamps, CPU, GPU, CPU so that you have error bars on the GPU timestamp? At the very least, two timestamps would be better than one so that, when we pull it into the kernel, it can provide something more accurate than userspace trying to grab a snapshot.
Hi,
three timestamps sounds like a good idea to me, but you might want to reach out to media developers (e.g. gstreamer) who have experience in synchronizing different clocks and what that will actually take.
When reading the CPU timestamp, I believe it would be necessary for userspace to tell which clock it should be reading. I'm not sure all userspace is always using CLOCK_MONOTONIC. But if you have a better rationale, that would be interesting to record and document.
Thanks, pq
On June 23, 2018 10:15:34 Keith Packard keithp@keithp.com wrote:
This extension adds a single function to query the current GPU timestamp, just like glGetInteger64v(GL_TIMESTAMP, ×tamp). 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 jason.ekstrand@intel.com
Signed-off-by: Keith Packard keithp@keithp.com
include/vulkan/vk_mesa_query_timestamp.h | 41 ++++++++++++++++++++++++ src/vulkan/registry/vk.xml | 15 +++++++++ 2 files changed, 56 insertions(+) create mode 100644 include/vulkan/vk_mesa_query_timestamp.h
Pekka Paalanen ppaalanen@gmail.com writes:
On Sat, 23 Jun 2018 12:13:53 -0500 Jason Ekstrand jason@jlekstrand.net wrote:
I haven't thought through this comment all that hard but would it make sense to have three timestamps, CPU, GPU, CPU so that you have error bars on the GPU timestamp? At the very least, two timestamps would be better than one so that, when we pull it into the kernel, it can provide something more accurate than userspace trying to grab a snapshot.
Hi,
three timestamps sounds like a good idea to me, but you might want to reach out to media developers (e.g. gstreamer) who have experience in synchronizing different clocks and what that will actually take.
Oh, I know that's really hard, and I don't want to solve that problem here. I explicitly *don't* solve it though -- I simply expose the ability to get correlated values in the two application-visible time domains that the Vulkan API already exposes (surface time and GPU time). How to synchronize the application as those two clocks drift around is outside the domain of this extension.
When reading the CPU timestamp, I believe it would be necessary for userspace to tell which clock it should be reading. I'm not sure all userspace is always using CLOCK_MONOTONIC. But if you have a better rationale, that would be interesting to record and document.
The extension doesn't state which clock is returned, you provide a device and a surface and get timestamps for both of them. This allows applications to translate between the two Vulkan time domains.
As far as the implementation goes, it asks the device driver to get correlated GPU and CLOCK_MONOTONIC values and then asks the WSI layer to translate between CLOCK_MONOTONIC and surface time. Right now, all three surface types already operate in CLOCK_MONOTONIC natively, so there isn't any translation needed to surface time.
The anv and radv driver implementations are simplistic and could easily be improved; they both simply fetch the GPU clock and then CLOCK_MONOTONIC sequentially without attempting to bound the error between them.
The entire reason for this extension is to allow me to implement the GOOGLE_display_timing extension using only public interfaces into the drivers. That patch is blocked on getting this functionality landed.
On Tue, 10 Jul 2018 11:02:23 -0700 "Keith Packard" keithp@keithp.com wrote:
Pekka Paalanen ppaalanen@gmail.com writes:
On Sat, 23 Jun 2018 12:13:53 -0500 Jason Ekstrand jason@jlekstrand.net wrote:
I haven't thought through this comment all that hard but would it make sense to have three timestamps, CPU, GPU, CPU so that you have error bars on the GPU timestamp? At the very least, two timestamps would be better than one so that, when we pull it into the kernel, it can provide something more accurate than userspace trying to grab a snapshot.
Hi,
three timestamps sounds like a good idea to me, but you might want to reach out to media developers (e.g. gstreamer) who have experience in synchronizing different clocks and what that will actually take.
Oh, I know that's really hard, and I don't want to solve that problem here. I explicitly *don't* solve it though -- I simply expose the ability to get correlated values in the two application-visible time domains that the Vulkan API already exposes (surface time and GPU time). How to synchronize the application as those two clocks drift around is outside the domain of this extension.
Hi Keith,
I did not mean you would be solving that problem. I meant that it would be good to figure out what people actually want from the API to be able to solve the problem themselves.
Thanks, pq
Pekka Paalanen ppaalanen@gmail.com writes:
I did not mean you would be solving that problem. I meant that it would be good to figure out what people actually want from the API to be able to solve the problem themselves.
Thanks for the clarification. I'd suggest that we not try and solve that problem until we have someone who needs it?
What I'm using this extension for is to correlate GPU times with WSI times as required by the GOOGLE_display_timing extension. That extension reports the time gap between the end of rendering and when a frame is displayed as reported by the WSI backend, so I needed some way to translate between those two time domains. To do this, I call this new function once per presentation, which means I'm getting recent values for both clocks which should track any minor clock skew.
This extension adds a single function to query the current GPU timestamp, just like glGetInteger64v(GL_TIMESTAMP, ×tamp). 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 jason.ekstrand@intel.com
Signed-off-by: Keith Packard keithp@keithp.com --- src/intel/vulkan/anv_extensions.py | 1 + src/intel/vulkan/anv_gem.c | 13 +++++++++++++ src/intel/vulkan/anv_private.h | 3 +++ src/intel/vulkan/genX_query.c | 15 +++++++++++++++ 4 files changed, 32 insertions(+)
diff --git a/src/intel/vulkan/anv_extensions.py b/src/intel/vulkan/anv_extensions.py index 0f99f58ecb1..37bace23857 100644 --- a/src/intel/vulkan/anv_extensions.py +++ b/src/intel/vulkan/anv_extensions.py @@ -120,6 +120,7 @@ EXTENSIONS = [ 'device->has_context_priority'), Extension('VK_EXT_shader_viewport_index_layer', 1, True), Extension('VK_EXT_shader_stencil_export', 1, 'device->info.gen >= 9'), + Extension('VK_MESA_query_timestamp', 1, True), ]
class VkVersion: diff --git a/src/intel/vulkan/anv_gem.c b/src/intel/vulkan/anv_gem.c index 3ba6d198a8a..8a31940e7aa 100644 --- a/src/intel/vulkan/anv_gem.c +++ b/src/intel/vulkan/anv_gem.c @@ -423,6 +423,19 @@ anv_gem_fd_to_handle(struct anv_device *device, int fd) return args.handle; }
+int +anv_gem_reg_read(struct anv_device *device, uint32_t offset, uint64_t *result) +{ + struct drm_i915_reg_read args = { + .offset = offset + }; + + int ret = anv_ioctl(device->fd, DRM_IOCTL_I915_REG_READ, &args); + + *result = args.val; + return ret; +} + #ifndef SYNC_IOC_MAGIC /* duplicated from linux/sync_file.h to avoid build-time dependency * on new (v4.7) kernel headers. Once distro's are mostly using diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 510471da602..0fa2a46e333 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -76,6 +76,7 @@ struct gen_l3_config; #include <vulkan/vulkan_intel.h> #include <vulkan/vk_icd.h> #include <vulkan/vk_android_native_buffer.h> +#include <vulkan/vk_mesa_query_timestamp.h>
#include "anv_entrypoints.h" #include "anv_extensions.h" @@ -1055,6 +1056,8 @@ int anv_gem_get_aperture(int fd, uint64_t *size); int anv_gem_gpu_get_reset_stats(struct anv_device *device, uint32_t *active, uint32_t *pending); int anv_gem_handle_to_fd(struct anv_device *device, uint32_t gem_handle); +int anv_gem_reg_read(struct anv_device *device, + uint32_t offset, uint64_t *result); uint32_t anv_gem_fd_to_handle(struct anv_device *device, int fd); int anv_gem_set_caching(struct anv_device *device, uint32_t gem_handle, uint32_t caching); int anv_gem_set_domain(struct anv_device *device, uint32_t gem_handle, diff --git a/src/intel/vulkan/genX_query.c b/src/intel/vulkan/genX_query.c index e35e9b85844..980c543460e 100644 --- a/src/intel/vulkan/genX_query.c +++ b/src/intel/vulkan/genX_query.c @@ -552,6 +552,21 @@ void genX(CmdWriteTimestamp)( } }
+VkResult genX(QueryCurrentTimestampMESA)( + VkDevice _device, + uint64_t *timestamp) +{ + ANV_FROM_HANDLE(anv_device, device, _device); + int ret; + + /* XXX older kernels don't support this interface. */ + ret = anv_gem_reg_read(device, TIMESTAMP | 1, timestamp); + + if (ret != 0) + return VK_ERROR_DEVICE_LOST; + return VK_SUCCESS; +} + #if GEN_GEN > 7 || GEN_IS_HASWELL
static uint32_t
This extension adds a single function to query the current GPU timestamp, just like glGetInteger64v(GL_TIMESTAMP, ×tamp). This function is needed to complete the implementation of GOOGLE_display_timing, which needs to be able to correlate GPU and CPU timestamps.
Signed-off-by: Keith Packard keithp@keithp.com --- src/amd/vulkan/radv_device.c | 8 ++++++++ src/amd/vulkan/radv_extensions.py | 1 + src/amd/vulkan/radv_private.h | 1 + 3 files changed, 10 insertions(+)
diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c index 62e1b9dba66..c552030491f 100644 --- a/src/amd/vulkan/radv_device.c +++ b/src/amd/vulkan/radv_device.c @@ -4770,3 +4770,11 @@ radv_GetDeviceGroupPeerMemoryFeatures( VK_PEER_MEMORY_FEATURE_GENERIC_SRC_BIT | VK_PEER_MEMORY_FEATURE_GENERIC_DST_BIT; } + +VkResult radv_QueryCurrentTimestampMESA(VkDevice _device, uint64_t *timestamp) +{ + RADV_FROM_HANDLE(radv_device, device, _device); + + *timestamp = device->ws->query_value(device->ws, RADEON_TIMESTAMP); + return VK_SUCCESS; +} diff --git a/src/amd/vulkan/radv_extensions.py b/src/amd/vulkan/radv_extensions.py index ebc3f6bc2b5..008e5cfe960 100644 --- a/src/amd/vulkan/radv_extensions.py +++ b/src/amd/vulkan/radv_extensions.py @@ -107,6 +107,7 @@ EXTENSIONS = [ Extension('VK_AMD_shader_core_properties', 1, True), Extension('VK_AMD_shader_info', 1, True), Extension('VK_AMD_shader_trinary_minmax', 1, True), + Extension('VK_MESA_query_timestamp', 1, True), ]
class VkVersion: diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h index f001b836c8f..a3e0a6f013c 100644 --- a/src/amd/vulkan/radv_private.h +++ b/src/amd/vulkan/radv_private.h @@ -75,6 +75,7 @@ typedef uint32_t xcb_window_t; #include <vulkan/vulkan_intel.h> #include <vulkan/vk_icd.h> #include <vulkan/vk_android_native_buffer.h> +#include <vulkan/vk_mesa_query_timestamp.h>
#include "radv_entrypoints.h"
dri-devel@lists.freedesktop.org
-
Jason Ekstrand -
Keith Packard -
Pekka Paalanen