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

Jason Ekstrand jason at jlekstrand.net
Tue Jul 10 16:47:14 UTC 2018


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

--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/64cd7687/attachment-0001.html>


More information about the mesa-dev mailing list