[Mesa-dev] [PATCH 1/2] radv: don't overwrite results in VkGetQueryPoolResults() when queries are not available
If the query is not available and VK_QUERY_RESULT_WAIT_BIT and VK_QUERY_RESULT_PARTIAL_BIT are both not set, the spec doesn't allow to modify its result. From Vulkan spec: "If VK_QUERY_RESULT_WAIT_BIT and VK_QUERY_RESULT_PARTIAL_BIT are both not set then no result values are written to pData for queries that are in the unavailable state at the time of the call, and vkGetQueryPoolResults returns VK_NOT_READY. However, availability state is still written to pData for those queries if VK_QUERY_RESULT_WITH_AVAILABILITY_BIT is set." Signed-off-by: Samuel Iglesias Gonsálvez --- src/amd/vulkan/radv_query.c | 52 +++-- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c index e808aa65170..8578680f09d 100644 --- a/src/amd/vulkan/radv_query.c +++ b/src/amd/vulkan/radv_query.c @@ -1148,10 +1148,12 @@ VkResult radv_GetQueryPoolResults( } if (flags & VK_QUERY_RESULT_64_BIT) { - *(uint64_t*)dest = *(uint64_t*)src; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint64_t*)dest = *(uint64_t*)src; dest += 8; } else { - *(uint32_t*)dest = *(uint32_t*)src; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint32_t*)dest = *(uint32_t*)src; dest += 4; } break; @@ -1183,10 +1185,12 @@ VkResult radv_GetQueryPoolResults( } if (flags & VK_QUERY_RESULT_64_BIT) { - *(uint64_t*)dest = sample_count; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint64_t*)dest = sample_count; dest += 8; } else { - *(uint32_t*)dest = sample_count; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint32_t*)dest = sample_count; dest += 4; } break; @@ -1203,18 +1207,26 @@ VkResult radv_GetQueryPoolResults( if (flags & VK_QUERY_RESULT_64_BIT) { uint64_t *dst = (uint64_t*)dest; dest += util_bitcount(pool->pipeline_stats_mask) * 8; - for(int i = 0; i < 11; ++i) - if(pool->pipeline_stats_mask & (1u << i)) - *dst++ = stop[pipeline_statistics_indices[i]] - - start[pipeline_statistics_indices[i]]; + for(int i = 0; i < 11; ++i) { + if(pool->pipeline_stats_mask & (1u << i)) { + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *dst = stop[pipeline_statistics_indices[i]] - + start[pipeline_statistics_indices[i]]; + dst++; + } + } } else { uint32_t *dst = (uint32_t*)dest; dest += util_bitcount(pool->pipeline_stats_mask) * 4; - for(int i = 0; i < 11; ++i) - if(pool->pipeline_stats_mask & (1u << i)) - *dst++ = stop[pipeline_statistics_indices[i]] - - start[pipeline_statistics_indices[i]]; + for(int i = 0; i < 11; ++i) { + if(pool->pipeline_stats_mask & (1u << i)) { + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *dst = stop[pipeline_statistics_indices[i]] - + start[pipeline_statistics_indices[i]]; + dst++; + } + } } break; } @@ -1235,23 +1247,25 @@ VkResult radv_GetQueryPoolResults( available = 0; } -
Re: [Mesa-dev] [PATCH 1/2] radv: don't overwrite results in VkGetQueryPoolResults() when queries are not available
On 3/22/19 1:03 PM, Samuel Iglesias Gonsálvez wrote: If the query is not available and VK_QUERY_RESULT_WAIT_BIT and VK_QUERY_RESULT_PARTIAL_BIT are both not set, the spec doesn't allow to modify its result. From Vulkan spec: "If VK_QUERY_RESULT_WAIT_BIT and VK_QUERY_RESULT_PARTIAL_BIT are both not set then no result values are written to pData for queries that are in the unavailable state at the time of the call, and vkGetQueryPoolResults returns VK_NOT_READY. However, availability state is still written to pData for those queries if VK_QUERY_RESULT_WITH_AVAILABILITY_BIT is set." Signed-off-by: Samuel Iglesias Gonsálvez --- src/amd/vulkan/radv_query.c | 52 +++-- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c index e808aa65170..8578680f09d 100644 --- a/src/amd/vulkan/radv_query.c +++ b/src/amd/vulkan/radv_query.c @@ -1148,10 +1148,12 @@ VkResult radv_GetQueryPoolResults( } if (flags & VK_QUERY_RESULT_64_BIT) { - *(uint64_t*)dest = *(uint64_t*)src; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint64_t*)dest = *(uint64_t*)src; dest += 8; } else { - *(uint32_t*)dest = *(uint32_t*)src; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint32_t*)dest = *(uint32_t*)src; dest += 4; } break; @@ -1183,10 +1185,12 @@ VkResult radv_GetQueryPoolResults( } if (flags & VK_QUERY_RESULT_64_BIT) { - *(uint64_t*)dest = sample_count; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint64_t*)dest = sample_count; dest += 8; } else { - *(uint32_t*)dest = sample_count; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint32_t*)dest = sample_count; dest += 4; } break; @@ -1203,18 +1207,26 @@ VkResult radv_GetQueryPoolResults( if (flags & VK_QUERY_RESULT_64_BIT) { uint64_t *dst = (uint64_t*)dest; dest += util_bitcount(pool->pipeline_stats_mask) * 8; - for(int i = 0; i < 11; ++i) - if(pool->pipeline_stats_mask & (1u << i)) - *dst++ = stop[pipeline_statistics_indices[i]] - - start[pipeline_statistics_indices[i]]; + for(int i = 0; i < 11; ++i) { + if(pool->pipeline_stats_mask & (1u << i)) { + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *dst = stop[pipeline_statistics_indices[i]] - + start[pipeline_statistics_indices[i]]; + dst++; + } + } } else { uint32_t *dst = (uint32_t*)dest; dest += util_bitcount(pool->pipeline_stats_mask) * 4; - for(int i = 0; i < 11; ++i) - if(pool->pipeline_stats_mask & (1u << i)) - *dst++ = stop[pipeline_statistics_indices[i]] - - start[pipeline_statistics_indices[i]]; + for(int i = 0; i < 11; ++i) { + if(pool->pipeline_stats_mask & (1u << i)) { + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *dst = stop[pipeline_statistics_indices[i]] - + start[pipeline_statistics_indices[i]]; + dst++; + } + } } break; } @@ -1235,23 +1247,25 @@ VkResult radv_GetQueryPoolResults( available = 0; } - if
Re: [Mesa-dev] [PATCH 1/2] radv: don't overwrite results in VkGetQueryPoolResults() when queries are not available
On 3/22/19 5:18 PM, Samuel Pitoiset wrote: On 3/22/19 1:03 PM, Samuel Iglesias Gonsálvez wrote: If the query is not available and VK_QUERY_RESULT_WAIT_BIT and VK_QUERY_RESULT_PARTIAL_BIT are both not set, the spec doesn't allow to modify its result. From Vulkan spec: "If VK_QUERY_RESULT_WAIT_BIT and VK_QUERY_RESULT_PARTIAL_BIT are both not set then no result values are written to pData for queries that are in the unavailable state at the time of the call, and vkGetQueryPoolResults returns VK_NOT_READY. However, availability state is still written to pData for those queries if VK_QUERY_RESULT_WITH_AVAILABILITY_BIT is set." Signed-off-by: Samuel Iglesias Gonsálvez --- src/amd/vulkan/radv_query.c | 52 +++-- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c index e808aa65170..8578680f09d 100644 --- a/src/amd/vulkan/radv_query.c +++ b/src/amd/vulkan/radv_query.c @@ -1148,10 +1148,12 @@ VkResult radv_GetQueryPoolResults( } if (flags & VK_QUERY_RESULT_64_BIT) { - *(uint64_t*)dest = *(uint64_t*)src; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint64_t*)dest = *(uint64_t*)src; dest += 8; } else { - *(uint32_t*)dest = *(uint32_t*)src; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint32_t*)dest = *(uint32_t*)src; dest += 4; } break; @@ -1183,10 +1185,12 @@ VkResult radv_GetQueryPoolResults( } if (flags & VK_QUERY_RESULT_64_BIT) { - *(uint64_t*)dest = sample_count; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint64_t*)dest = sample_count; dest += 8; } else { - *(uint32_t*)dest = sample_count; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint32_t*)dest = sample_count; dest += 4; } break; @@ -1203,18 +1207,26 @@ VkResult radv_GetQueryPoolResults( if (flags & VK_QUERY_RESULT_64_BIT) { uint64_t *dst = (uint64_t*)dest; dest += util_bitcount(pool->pipeline_stats_mask) * 8; - for(int i = 0; i < 11; ++i) - if(pool->pipeline_stats_mask & (1u << i)) - *dst++ = stop[pipeline_statistics_indices[i]] - - start[pipeline_statistics_indices[i]]; + for(int i = 0; i < 11; ++i) { + if(pool->pipeline_stats_mask & (1u << i)) { + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *dst = stop[pipeline_statistics_indices[i]] - + start[pipeline_statistics_indices[i]]; + dst++; + } + } } else { uint32_t *dst = (uint32_t*)dest; dest += util_bitcount(pool->pipeline_stats_mask) * 4; - for(int i = 0; i < 11; ++i) - if(pool->pipeline_stats_mask & (1u << i)) - *dst++ = stop[pipeline_statistics_indices[i]] - - start[pipeline_statistics_indices[i]]; + for(int i = 0; i < 11; ++i) { + if(pool->pipeline_stats_mask & (1u << i)) { + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *dst = stop[pipeline_statistics_indices[i]] - + start[pipeline_statistics_indices[i]]; + dst++; + } + } } break; } @@ -1235,23 +1247,25 @@ VkResult radv_GetQueryPoolResults( available = 0; } - if (!available && !(flags & VK_QUERY_RESULT_PARTIAL_BIT)) { + if (!available && !(flags & VK_QUERY_RESULT_PARTIAL_BIT)) result = VK_NOT_READY; - break; - } Removing that break is useless? After reading patch 2, I understand. This change should be moved to next patch. num_primitives_written = src64[3] - src64[1]; primitive_storage_needed = src64[2] - src64[0]; if (flags & VK_QUERY_RESULT_64_BIT) { - *(uint64_t *)dest = num_primitives_written; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint64_t *)dest = num_primitives_written; dest += 8; - *(uint64_t *)dest = primitive_storage_needed; + if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT)) + *(uint64_t *)dest = primitive_storage_needed; dest += 8; } else { - *(uint32_t *)d