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

Jason Ekstrand jason at jlekstrand.net
Tue Jul 10 17:03:08 UTC 2018


On Tue, Jul 10, 2018 at 9:47 AM Jason Ekstrand <jason at jlekstrand.net> wrote:

> On Mon, Jul 9, 2018 at 5:35 PM Keith Packard <keithp at keithp.com> wrote:
>
>> This extension adds a single function to query the current GPU
>> timestamp, just like glGetInteger64v(GL_TIMESTAMP, &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 <jason.ekstrand at intel.com>
>>
>> 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 <keithp at keithp.com>
>> ---
>>  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 00000000000..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_t    deviceTimestamp;
>> +    uint64_t    surfaceTimestamp;
>> +} 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 currently lacks any sort of CPU
> timestamp query and that's something which is likely useful in its own
> right.  It'd be annoying if we added such a useful thing to the API and
> then made it require a VkSurface to work. :-)
>

Another option, instead of having two extensions, would be to make the
second timestamp that gets returned be some sort of OS timestamp.  On
Linux, it would be defined to be CLOCK_MONOTONIC; on windows, I'm not sure
what the right thing would be.  Then we could just magically know in the
window system code which calls this extension that we get CLOCK_MONTONIC
from that function and we can do whatever conversions we want there.


> --Jason
>
>
>
>> +
>> +#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.
>>              <member><type>VkBool32</type>
>>  <name>conditionalRendering</name></member>
>>              <member><type>VkBool32</type>
>>  <name>inheritedConditionalRendering</name></member>
>>          </type>
>> +       <type catagory="struct" name="VkCurrentTimestampMESA">
>> +         <member><type>uint64_t</type>
>>  <name>deviceTimestamp</name></member>
>> +         <member><type>uint64_t</type>
>> <name>surfaceTimestamp</name></member>
>> +       </type>
>>      </types>
>>
>>      <comment>Vulkan enumerant (token) definitions</comment>
>> @@ -6239,6 +6243,12 @@ 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>VkSurfaceKHR</type> <name>surface</name></param>
>> +            <param><type>VkCurrentTimestampMESA</type>*
>> <name>pTimestamp</name></param>
>> +        </command>
>>      </commands>
>>
>>      <feature api="vulkan" name="VK_VERSION_1_0" number="1.0"
>> comment="Vulkan core API interface definitions">
>> @@ -9008,5 +9018,15 @@ server.
>>                  <enum value=""VK_KHR_extension_214""
>>     name="VK_KHR_EXTENSION_214_EXTENSION_NAME"/>
>>              </require>
>>          </extension>
>> +        <extension name="VK_MESA_query_timestamp" number="215"
>> +                  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>
>> 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,
>> +                             VkDevice device_h,
>> +                             VkSurfaceKHR surface_h,
>> +                             uint64_t monotonic_timestamp,
>> +                             uint64_t *surface_timestamp)
>> +{
>> +   ICD_FROM_HANDLE(VkIcdSurfaceBase, surface, surface_h);
>> +   struct wsi_interface *iface = wsi->wsi[surface->platform];
>> +
>> +   if (iface->convert_timestamp)
>> +      return iface->convert_timestamp(surface, wsi, monotonic_timestamp,
>> +                                      surface_timestamp);
>> +   *surface_timestamp = monotonic_timestamp;
>> +   return VK_SUCCESS;
>> +}
>> diff --git a/src/vulkan/wsi/wsi_common.h b/src/vulkan/wsi/wsi_common.h
>> index 33e4f849ac9..0a57dc4eb23 100644
>> --- a/src/vulkan/wsi/wsi_common.h
>> +++ b/src/vulkan/wsi/wsi_common.h
>> @@ -235,4 +235,11 @@ wsi_common_queue_present(const struct wsi_device
>> *wsi,
>>                           int queue_family_index,
>>                           const VkPresentInfoKHR *pPresentInfo);
>>
>> +VkResult
>> +wsi_common_convert_timestamp(const struct wsi_device *wsi,
>> +                             VkDevice device_h,
>> +                             VkSurfaceKHR surface_h,
>> +                             uint64_t monotonic_timestamp,
>> +                             uint64_t *surface_timestamp);
>> +
>>  #endif
>> diff --git a/src/vulkan/wsi/wsi_common_private.h
>> b/src/vulkan/wsi/wsi_common_private.h
>> index 9f2aacd6560..843b4c9b286 100644
>> --- a/src/vulkan/wsi/wsi_common_private.h
>> +++ b/src/vulkan/wsi/wsi_common_private.h
>> @@ -126,6 +126,10 @@ struct wsi_interface {
>>                                  const VkSwapchainCreateInfoKHR*
>> pCreateInfo,
>>                                  const VkAllocationCallbacks* pAllocator,
>>                                  struct wsi_swapchain **swapchain);
>> +   VkResult (*convert_timestamp)(VkIcdSurfaceBase *surface,
>> +                                 const struct wsi_device *wsi_device,
>> +                                 uint64_t monotonic_timestamp,
>> +                                 uint64_t *surface_timestamp);
>>  };
>>
>>  VkResult wsi_x11_init_wsi(struct wsi_device *wsi_device,
>> --
>> 2.18.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180710/ff962ab4/attachment-0001.html>


More information about the mesa-dev mailing list