From d3875fc9b645c5276d95d1662859ad75bbd51ff4 Mon Sep 17 00:00:00 2001 From: Li Hao Date: Fri, 19 May 2023 08:07:40 +0000 Subject: [PATCH] Add workaround for resolving overlapping queries on Intel Gen12 TimestampQueryTests.ResolveTwiceToSameBuffer fails on Mesa driver >= 21.2.0 and D3D driver >= 31.0.101.3413 on Intel Gen12 GPUs due to driver bugs with different root causes, but the workaround of clearing destination buffer before resolving queries works for both. Bug: dawn:1546, dawn:1823 Change-Id: I3f20a9100f4b6d3386e9685b351ad4fed69195bd Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/133284 Reviewed-by: Austin Eng Commit-Queue: Hao Li Kokoro: Kokoro --- src/dawn/native/Toggles.cpp | 5 +++++ src/dawn/native/Toggles.h | 1 + src/dawn/native/d3d12/CommandBufferD3D12.cpp | 6 +++++- src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp | 12 ++++++++++++ src/dawn/native/vulkan/CommandBufferVk.cpp | 6 +++++- src/dawn/native/vulkan/PhysicalDeviceVk.cpp | 13 +++++++++++++ src/dawn/tests/end2end/QueryTests.cpp | 4 ---- 7 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp index 40765de7c2..f4213c5701 100644 --- a/src/dawn/native/Toggles.cpp +++ b/src/dawn/native/Toggles.cpp @@ -251,6 +251,11 @@ static constexpr ToggleEnumAndInfoList kToggleNameAndInfoList = {{ {"disable_timestamp_query_conversion", "Resolve timestamp queries into ticks instead of nanoseconds.", "https://crbug.com/dawn/1305", ToggleStage::Device}}, + {Toggle::ClearBufferBeforeResolveQueries, + {"clear_buffer_before_resolve_queries", + "clear destination buffer to zero before resolving queries. This toggle is enabled on Intel " + "Gen12 GPUs due to driver issue.", + "https://crbug.com/dawn/1823", ToggleStage::Device}}, {Toggle::VulkanUseZeroInitializeWorkgroupMemoryExtension, {"use_vulkan_zero_initialize_workgroup_memory_extension", "Initialize workgroup memory with OpConstantNull on Vulkan when the Vulkan extension " diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h index d0b02127ee..5fa5eff829 100644 --- a/src/dawn/native/Toggles.h +++ b/src/dawn/native/Toggles.h @@ -71,6 +71,7 @@ enum class Toggle { FxcOptimizations, RecordDetailedTimingInTraceEvents, DisableTimestampQueryConversion, + ClearBufferBeforeResolveQueries, VulkanUseZeroInitializeWorkgroupMemoryExtension, D3D12SplitBufferTextureCopyForRowsPerImagePaddings, MetalRenderR8RG8UnormSmallMipToTempTexture, diff --git a/src/dawn/native/d3d12/CommandBufferD3D12.cpp b/src/dawn/native/d3d12/CommandBufferD3D12.cpp index a57fccc98a..77e2fa65d8 100644 --- a/src/dawn/native/d3d12/CommandBufferD3D12.cpp +++ b/src/dawn/native/d3d12/CommandBufferD3D12.cpp @@ -1056,7 +1056,11 @@ MaybeError CommandBuffer::RecordCommands(CommandRecordingContext* commandContext auto startIt = querySet->GetQueryAvailability().begin() + firstQuery; auto endIt = querySet->GetQueryAvailability().begin() + firstQuery + queryCount; bool hasUnavailableQueries = std::find(startIt, endIt, false) != endIt; - if (hasUnavailableQueries) { + // Workaround for resolving overlapping queries to a same buffer on Intel Gen12 GPUs + // due to D3D12 driver issue. + // See http://crbug.com/dawn/1546 for more information. + bool clearNeeded = device->IsToggleEnabled(Toggle::ClearBufferBeforeResolveQueries); + if (hasUnavailableQueries || clearNeeded) { DAWN_TRY(device->ClearBufferToZero(commandContext, destination, destinationOffset, queryCount * sizeof(uint64_t))); diff --git a/src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp b/src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp index 607d21d952..acecd96f2b 100644 --- a/src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp +++ b/src/dawn/native/d3d12/PhysicalDeviceD3D12.cpp @@ -535,6 +535,18 @@ void PhysicalDevice::SetupBackendDeviceToggles(TogglesState* deviceToggles) cons } } + // D3D driver has a bug resolving overlapping queries to a same buffer on Intel Gen12 GPUs. This + // workaround is needed on the driver version >= 30.0.101.3413. + // TODO(crbug.com/dawn/1546): Remove the workaround when the bug is fixed in D3D driver. + if (gpu_info::IsIntelGen12LP(vendorId, deviceId) || + gpu_info::IsIntelGen12HP(vendorId, deviceId)) { + const gpu_info::DriverVersion kDriverVersion = {30, 0, 101, 3413}; + if (gpu_info::CompareWindowsDriverVersion(vendorId, GetDriverVersion(), kDriverVersion) != + -1) { + deviceToggles->Default(Toggle::ClearBufferBeforeResolveQueries, true); + } + } + // Currently these workarounds are only needed on Intel Gen9.5 and Gen11 GPUs. // See http://crbug.com/1237175 and http://crbug.com/dawn/1628 for more information. if ((gpu_info::IsIntelGen9(vendorId, deviceId) && !gpu_info::IsSkylake(deviceId)) || diff --git a/src/dawn/native/vulkan/CommandBufferVk.cpp b/src/dawn/native/vulkan/CommandBufferVk.cpp index 138ad74514..ba2c5d7dc1 100644 --- a/src/dawn/native/vulkan/CommandBufferVk.cpp +++ b/src/dawn/native/vulkan/CommandBufferVk.cpp @@ -767,7 +767,11 @@ MaybeError CommandBuffer::RecordCommands(CommandRecordingContext* recordingConte auto endIt = querySet->GetQueryAvailability().begin() + cmd->firstQuery + cmd->queryCount; bool hasUnavailableQueries = std::find(startIt, endIt, false) != endIt; - if (hasUnavailableQueries) { + // Workaround for resolving overlapping queries to a same buffer on Intel Gen12 GPUs + // due to Mesa driver issue. + // See http://crbug.com/dawn/1823 for more information. + bool clearNeeded = device->IsToggleEnabled(Toggle::ClearBufferBeforeResolveQueries); + if (hasUnavailableQueries || clearNeeded) { destination->TransitionUsageNow(recordingContext, wgpu::BufferUsage::CopyDst); device->fn.CmdFillBuffer(commands, destination->GetHandle(), cmd->destinationOffset, diff --git a/src/dawn/native/vulkan/PhysicalDeviceVk.cpp b/src/dawn/native/vulkan/PhysicalDeviceVk.cpp index dcf3f039bf..79032c68f7 100644 --- a/src/dawn/native/vulkan/PhysicalDeviceVk.cpp +++ b/src/dawn/native/vulkan/PhysicalDeviceVk.cpp @@ -485,6 +485,19 @@ void PhysicalDevice::SetupBackendDeviceToggles(TogglesState* deviceToggles) cons } } + if (IsIntelMesa() && (gpu_info::IsIntelGen12LP(GetVendorId(), GetDeviceId()) || + gpu_info::IsIntelGen12HP(GetVendorId(), GetDeviceId()))) { + // Intel Mesa driver has a bug where vkCmdCopyQueryPoolResults fails to write overlapping + // queries to a same buffer after the buffer is accessed by a compute shader with correct + // resource barriers, which may caused by flush and memory coherency issue on Intel Gen12 + // GPUs. Workaround for it to clear the buffer before vkCmdCopyQueryPoolResults. + // TODO(crbug.com/dawn/1823): Remove the workaround when the bug is fixed in Mesa driver. + const gpu_info::DriverVersion kBuggyDriverVersion = {21, 2, 0, 0}; + if (gpu_info::CompareIntelMesaDriverVersion(GetDriverVersion(), kBuggyDriverVersion) >= 0) { + deviceToggles->Default(Toggle::ClearBufferBeforeResolveQueries, true); + } + } + // The environment can request to various options for depth-stencil formats that could be // unavailable. Override the decision if it is not applicable. bool supportsD32s8 = IsDepthStencilFormatSupported(VK_FORMAT_D32_SFLOAT_S8_UINT); diff --git a/src/dawn/tests/end2end/QueryTests.cpp b/src/dawn/tests/end2end/QueryTests.cpp index a246aeb6a1..0295ce9f2d 100644 --- a/src/dawn/tests/end2end/QueryTests.cpp +++ b/src/dawn/tests/end2end/QueryTests.cpp @@ -1091,10 +1091,6 @@ TEST_P(TimestampQueryTests, ResolveToBufferWithOffset) { // Test resolving a query set twice into the same destination buffer with potentially overlapping // ranges TEST_P(TimestampQueryTests, ResolveTwiceToSameBuffer) { - // TODO(dawn:1546): Intel D3D driver regression on Gen12 GPUs. The compute shader in two - // ResolveQuerySet execute wrong. - DAWN_SUPPRESS_TEST_IF(IsD3D12() && IsIntelGen12()); - constexpr uint32_t kQueryCount = kMinCount + 2; wgpu::QuerySet querySet = CreateQuerySetForTimestamp(kQueryCount);