Query pool reads for QUEUE_OPERATIONS_END_BIT_EXT skipped when not ready

In this change, the return status from query pool reads for
QUEUE_OPERATIONS_END_BIT_EXT are checked and skipped when not ready.
To cater for correctness of any later reads, the value returned is
cached and compared. Additionally, a fix is added to avoid data
corruption with presentation feedback object in present timing when
vector was used.

Signed-off-by: Ginu Jacob <ginu.jacob@arm.com>
Change-Id: I5f26b6a3c81eb73eff8a4073be7d7ffc81f9eef8

wsi/extensions/present_timing.cpp

@@ -56,6 +56,7 @@ wsi_ext_present_timing::wsi_ext_present_timing(const util::allocator &allocator,
    , m_query_pool(VK_NULL_HANDLE)
    , m_command_pool(VK_NULL_HANDLE)
    , m_command_buffer(allocator)
+   , m_device_timestamp_cached(allocator)
    , m_queue_mutex()
    , m_queue(allocator)
    , m_scheduled_present_targets(allocator)
@@ -130,6 +131,11 @@ VkResult wsi_ext_present_timing::init_timing_resources()
    {
       return VK_ERROR_OUT_OF_HOST_MEMORY;
    }
+   /* Resize cached device timestamp records to the number of images. */
+   if (!m_device_timestamp_cached.try_resize(m_num_images, 0ULL))
+   {
+      return VK_ERROR_OUT_OF_HOST_MEMORY;
+   }
    for (auto &command_buffer : m_command_buffer)
    {
       command_buffer = VK_NULL_HANDLE;
@@ -196,10 +202,19 @@ VkResult wsi_ext_present_timing::write_pending_results()
    {
       if (slot.is_pending(VK_PRESENT_STAGE_QUEUE_OPERATIONS_END_BIT_EXT))
       {
-         uint64_t time;
-         TRY(m_device.disp.GetQueryPoolResults(m_device.device, m_query_pool, slot.m_image_index, 1, sizeof(time),
-                                               &time, 0, VK_QUERY_RESULT_64_BIT));
-         slot.set_stage_timing(VK_PRESENT_STAGE_QUEUE_OPERATIONS_END_BIT_EXT, ticks_to_ns(time, m_timestamp_period));
+         uint64_t timestamp;
+         VkResult res = m_device.disp.GetQueryPoolResults(m_device.device, m_query_pool, slot.m_image_index, 1,
+                                                          sizeof(timestamp), &timestamp, 0, VK_QUERY_RESULT_64_BIT);
+         if (res != VK_SUCCESS && res != VK_NOT_READY)
+         {
+            return res;
+         }
+         if (res == VK_SUCCESS && m_device_timestamp_cached[slot.m_image_index] != timestamp)
+         {
+            m_device_timestamp_cached[slot.m_image_index] = timestamp;
+            slot.set_stage_timing(VK_PRESENT_STAGE_QUEUE_OPERATIONS_END_BIT_EXT,
+                                  ticks_to_ns(timestamp, m_timestamp_period));
+         }
       }
       if (slot.is_pending(VK_PRESENT_STAGE_IMAGE_FIRST_PIXEL_OUT_BIT_EXT))
       {

wsi/extensions/present_timing.hpp

@@ -536,6 +536,13 @@ private:
     */
    util::vector<VkCommandBuffer> m_command_buffer;
 
+   /**
+    * @brief Stores the device timestamp recorded from the previous
+    * VK_PRESENT_STAGE_QUEUE_OPERATIONS_END_BIT_EXT stage for each image
+    * index of the swapchain.
+    */
+   util::vector<uint64_t> m_device_timestamp_cached;
+
    /**
     * @brief Mutex guarding the internal presentation-timing queue.
     *

wsi/wayland/present_timing_handler.cpp

@@ -40,7 +40,6 @@ wsi_ext_present_timing_wayland::wsi_ext_present_timing_wayland(
    const util::allocator &allocator, VkDevice device, uint32_t num_images,
    util::vector<std::optional<uint64_t>> &&timestamp_first_pixel_out_storage)
    : wsi_ext_present_timing(allocator, device, num_images)
-   , m_pending_presents(allocator)
    , m_timestamp_first_pixel_out(allocator)
 {
    m_timestamp_first_pixel_out.swap(timestamp_first_pixel_out_storage);
@@ -101,19 +100,15 @@ VkResult wsi_ext_present_timing_wayland::get_swapchain_timing_properties(
 presentation_feedback *wsi_ext_present_timing_wayland::insert_into_pending_present_feedback_list(
    uint32_t image_index, struct wp_presentation_feedback *feedback_obj)
 {
+
    util::unique_lock<util::mutex> lock(m_pending_presents_lock);
    if (!lock)
    {
       WSI_LOG_ERROR("Failed to acquire pending presents lock in insert_into_pending_present_feedback_list.\n");
       abort();
    }
-   presentation_feedback fb(feedback_obj, this, image_index);
-   size_t position = m_pending_presents.size();
-   if (!m_pending_presents.try_push_back(std::move(fb)))
-   {
-      return nullptr;
-   }
-   return &m_pending_presents[position];
+   m_pending_presents[image_index] = presentation_feedback(feedback_obj, this, image_index);
+   return &m_pending_presents[image_index].value();
 }
 
 void wsi_ext_present_timing_wayland::remove_from_pending_present_feedback_list(uint32_t image_index)
@@ -124,13 +119,7 @@ void wsi_ext_present_timing_wayland::remove_from_pending_present_feedback_list(u
       WSI_LOG_ERROR("Failed to acquire pending presents lock in remove_from_pending_present_feedback_list.\n");
       abort();
    }
-   auto it = std::find_if(m_pending_presents.begin(), m_pending_presents.end(),
-                          [image_index](const presentation_feedback &p) { return p.get_image_index() == image_index; });
-
-   if (it != m_pending_presents.end())
-   {
-      m_pending_presents.erase(it);
-   }
+   m_pending_presents[image_index].reset();
 }
 
 void wsi_ext_present_timing_wayland::pixelout_callback(uint32_t image_index, uint64_t time)

wsi/wayland/present_timing_handler.hpp

@@ -119,7 +119,6 @@ private:
     * @brief Stores the presentation feedbacks that have been queued.
     */
 
-   util::vector<presentation_feedback> m_pending_presents;
    wsi_ext_present_timing_wayland(const util::allocator &allocator, VkDevice device, uint32_t num_images,
                                   util::vector<std::optional<uint64_t>> &&timestamp_first_pixel_out_storage);
 
@@ -133,6 +132,12 @@ private:
 
    /* Allow util::allocator to access the private constructor */
    friend util::allocator;
+
+   /**
+    * @brief Stores the presentation feedbacks that have been queued.
+    */
+   std::array<std::optional<presentation_feedback>, wsi::surface_properties::MAX_SWAPCHAIN_IMAGE_COUNT>
+      m_pending_presents{};
 };
 
 } // namespace wayland

wsi/wayland/swapchain.cpp

@@ -602,7 +602,7 @@ void swapchain::present_image(const pending_present_request &pending_present)
    if (m_device_data.is_present_id_enabled())
    {
       auto *ext = get_swapchain_extension<wsi_ext_present_id_wayland>(true);
-      if (m_wsi_surface->get_presentation_time_interface() != nullptr)
+      if (m_wsi_surface->get_presentation_time_interface() != nullptr && pending_present.present_id)
       {
          wp_presentation *pres = m_wsi_surface->get_presentation_time_interface();
          struct wp_presentation_feedback *feedback = wp_presentation_feedback(pres, m_wsi_surface->get_wl_surface());

wsi/wayland/wp_presentation_feedback.cpp

@@ -55,7 +55,7 @@ wp_presentation_feedback_presented(void *data, struct wp_presentation_feedback *
       feedback_obj->ext_present_timing()->pixelout_callback(feedback_obj->get_image_index(), timestamp_ns);
       feedback_obj->ext_present_timing()->remove_from_pending_present_feedback_list(feedback_obj->get_image_index());
    }
-   if (feedback_obj->ext_present_id() != nullptr)
+   else if (feedback_obj->ext_present_id() != nullptr)
    {
       feedback_obj->ext_present_id()->mark_delivered(feedback_obj->get_present_id());
       feedback_obj->ext_present_id()->remove_from_pending_present_feedback_list(feedback_obj->get_present_id());