protonscr

Incorrect cpu handle of descriptor heap used in ClearUnorderedAccessView

vkd3dclosed
HansKristian-Work/vkd3d-proton#2122 · opened 2024-09-13 by lukelmy · updated 2024-09-20 · 4 comments · github
Llukelmy 2024-09-13 github

Recently we found that test_planar_video_formats failed with AMDVLK.
The failures are in the UAV test and ClearUnorderedAccessViewFloat returns early due to meta.view = NULL in d3d12_desc_decode_metadata.
However, this early return is not observed in other tests using ClearUnorderedAccessView*.

After investigation, I think the root cause is that this test uses cpu and gpu handle of same shader-visible descriptor heap for ClearUnorderedAccessView*.
For shader-visible descriptor heap, the low 5 bits of cpu va are 0, which leads to meta.view = NULL in d3d12_desc_decode_metadata.
https://github.com/HansKristian-Work/vkd3d-proton/blob/26d283ccbc3d6fbffd8e7b84215d2ecf2caa791f/libs/vkd3d/resource.c#L8177
And it’s Illegal according to https://learn.microsoft.com/en-us/windows/win32/api/d3d12/nf-d3d12-id3d12graphicscommandlist-clearunorderedaccessviewfloat.
The cpu handle should be created with a non-shader-visible heap. (Actually most tests in vkd3d follow this rule)
https://github.com/HansKristian-Work/vkd3d-proton/blob/26d283ccbc3d6fbffd8e7b84215d2ecf2caa791f/tests/d3d12_resource.c#L4286-L4289

I have checked other tests and found same issue in test_sm68_draw_parameters.
https://github.com/HansKristian-Work/vkd3d-proton/blob/26d283ccbc3d6fbffd8e7b84215d2ecf2caa791f/tests/d3d12_sm_advanced.c#L4180-L4183

BTW, since RADV supports VKD3D_BINDLESS_MUTABLE_EMBEDDED and VKD3D_BINDLESS_MUTABLE_EMBEDDED_PACKED_METADATA while AMDVLK only supports the first flag, 2 drivers would go into different code path in d3d12_desc_decode_metadata.
Maybe that's why no issue is observed with RADV. (meta.view != NULL)

My local fix which works:

diff --git a/tests/d3d12_resource.c b/tests/d3d12_resource.c
index 504c78cf..786915db 100644
--- a/tests/d3d12_resource.c
+++ b/tests/d3d12_resource.c
@@ -3606,7 +3606,7 @@ void test_planar_video_formats(void)
     D3D12_FEATURE_DATA_FORMAT_SUPPORT format_support;
     unsigned int subsample_x_log2, subsample_y_log2;
     ID3D12PipelineState *graphics_psos[MAX_PLANES];
-    ID3D12DescriptorHeap *rtv_heap, *srv_uav_heap;
+    ID3D12DescriptorHeap *rtv_heap, *srv_uav_heap, *srv_uav_cpu_heap;
     D3D12_DESCRIPTOR_RANGE descriptor_ranges[2];
     D3D12_FEATURE_DATA_FORMAT_INFO format_info;
     D3D12_UNORDERED_ACCESS_VIEW_DESC uav_desc;
@@ -3773,6 +3773,7 @@ void test_planar_video_formats(void)
     rtv = get_cpu_descriptor_handle(&context, rtv_heap, 0);
 
     srv_uav_heap = create_gpu_descriptor_heap(context.device, D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV, MAX_PLANES * 2);
+    srv_uav_cpu_heap = create_cpu_descriptor_heap(context.device, D3D12_DESCRIPTOR_HEAP_TYPE_CBV_SRV_UAV, MAX_PLANES * 2);
 
     memset(&scissor, 0, sizeof(scissor));
     memset(&viewport, 0, sizeof(viewport));
@@ -4263,6 +4264,8 @@ void test_planar_video_formats(void)
 
                     ID3D12Device_CreateUnorderedAccessView(context.device, resources[1], NULL, &uav_desc,
                             get_cpu_descriptor_handle(&context, srv_uav_heap, 2 * j + 1));
+                    ID3D12Device_CreateUnorderedAccessView(context.device, resources[1], NULL, &uav_desc,
+                            get_cpu_descriptor_handle(&context, srv_uav_cpu_heap, 2 * j + 1));
 
                     ID3D12GraphicsCommandList_SetDescriptorHeaps(context.list, 1, &srv_uav_heap);
                     ID3D12GraphicsCommandList_SetComputeRootSignature(context.list, context.root_signature);
@@ -4285,7 +4288,7 @@ void test_planar_video_formats(void)
 
                     ID3D12GraphicsCommandList_ClearUnorderedAccessViewFloat(context.list,
                             get_gpu_descriptor_handle(&context, srv_uav_heap, 2 * j + 1),
-                            get_cpu_descriptor_handle(&context, srv_uav_heap, 2 * j + 1),
+                            get_cpu_descriptor_handle(&context, srv_uav_cpu_heap, 2 * j + 1),
                             resources[1], clear_color, 0, NULL);
 
                     transition_sub_resource_state(context.list, resources[1], j,
@@ -4302,7 +4305,7 @@ void test_planar_video_formats(void)
 
                     ID3D12GraphicsCommandList_ClearUnorderedAccessViewUint(context.list,
                             get_gpu_descriptor_handle(&context, srv_uav_heap, 2 * j + 1),
-                            get_cpu_descriptor_handle(&context, srv_uav_heap, 2 * j + 1),
+                            get_cpu_descriptor_handle(&context, srv_uav_cpu_heap, 2 * j + 1),
                             resources[1], clear_color_uint, 0, NULL);
 
                     transition_sub_resource_state(context.list, resources[1], j,
@@ -4364,6 +4367,7 @@ void test_planar_video_formats(void)
 
     ID3D12DescriptorHeap_Release(rtv_heap);
     ID3D12DescriptorHeap_Release(srv_uav_heap);
+    ID3D12DescriptorHeap_Release(srv_uav_cpu_heap);
 
     ID3D12PipelineState_Release(compute_pso);

Software information

vkd3d test: test_planar_video_formats

System information

  • GPU: Radeon 7900XTX
  • Driver: AMDVLK
HHansKristian-Work maintainer 2024-09-13 github

Yes, this is a test bug. Will you submit a PR with the fix?

HHansKristian-Work maintainer 2024-09-13 github

Fixed.

Llukelmy 2024-09-20 github