HansKristian-Work/vkd3d-proton

Incorrect cpu handle of descriptor heap used in ClearUnorderedAccessView

Closed this issue · 4 comments

lukelmy commented

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.

vkd3d-proton/libs/vkd3d/resource.c

Line 8177 in 26d283c

assert(!(object->cpu_va.ptr & VKD3D_RESOURCE_EMBEDDED_METADATA_OFFSET_LOG2_MASK));

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)

vkd3d-proton/tests/d3d12_resource.c

Lines 4286 to 4289 in 26d283c

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),
resources[1], clear_color, 0, NULL);

I have checked other tests and found same issue in test_sm68_draw_parameters.

vkd3d-proton/tests/d3d12_sm_advanced.c

Lines 4180 to 4183 in 26d283c

ID3D12GraphicsCommandList_ClearUnorderedAccessViewUint(context.list,
ID3D12DescriptorHeap_GetGPUDescriptorHandleForHeapStart(uav_heap),
ID3D12DescriptorHeap_GetCPUDescriptorHandleForHeapStart(uav_heap),
uav, zero_uint, 0, NULL);

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
HansKristian-Work commented

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

lukelmy commented

PR: #2126 for test_large_texel_buffer_view