Automate
A Showcase
Today I’m taking a break from writing about my work to write about the work of zink’s newest contributor, He Haocheng (aka @hch12907). Among other things, Haocheng has recently tackled the issue of extension refactoring, which is a huge help for future driver development. I’ve written time and time again about adding extensions, and with this patchset in place, the process is simplified and expedited almost into nonexistence.
Before
As an example, let’s look at the most recent extension that I’ve added support for, VK_EXT_extended_dynamic_state. The original patch looked like this:
diff --git a/src/gallium/drivers/zink/zink_screen.c b/src/gallium/drivers/zink/zink_screen.c
index 3effa2b0fe4..83b89106931 100644
--- a/src/gallium/drivers/zink/zink_screen.c
+++ b/src/gallium/drivers/zink/zink_screen.c
@@ -925,6 +925,10 @@ load_device_extensions(struct zink_screen *screen)
assert(have_device_time);
free(domains);
}
+ if (screen->have_EXT_extended_dynamic_state) {
+ GET_PROC_ADDR(CmdSetViewportWithCountEXT);
+ GET_PROC_ADDR(CmdSetScissorWithCountEXT);
+ }
#undef GET_PROC_ADDR
@@ -938,7 +942,8 @@ zink_internal_create_screen(struct sw_winsys *winsys, int fd, const struct pipe_
bool have_tf_ext = false, have_cond_render_ext = false, have_EXT_index_type_uint8 = false,
have_EXT_robustness2_features = false, have_EXT_vertex_attribute_divisor = false,
have_EXT_calibrated_timestamps = false, have_VK_KHR_vulkan_memory_model = false;
- bool have_EXT_custom_border_color = false, have_EXT_blend_operation_advanced = false;
+ bool have_EXT_custom_border_color = false, have_EXT_blend_operation_advanced = false,
+ have_EXT_extended_dynamic_state = false;
if (!screen)
return NULL;
@@ -1001,6 +1006,9 @@ zink_internal_create_screen(struct sw_winsys *winsys, int fd, const struct pipe_
if (!strcmp(extensions[i].extensionName,
VK_EXT_BLEND_OPERATION_ADVANCED_EXTENSION_NAME))
have_EXT_blend_operation_advanced = true;
+ if (!strcmp(extensions[i].extensionName,
+ VK_EXT_EXTENDED_DYNAMIC_STATE_EXTENSION_NAME))
+ have_EXT_extended_dynamic_state = true;
}
FREE(extensions);
@@ -1012,6 +1020,7 @@ zink_internal_create_screen(struct sw_winsys *winsys, int fd, const struct pipe_
VkPhysicalDeviceIndexTypeUint8FeaturesEXT index_uint8_feats = {};
VkPhysicalDeviceVulkanMemoryModelFeatures mem_feats = {};
VkPhysicalDeviceBlendOperationAdvancedFeaturesEXT blend_feats = {};
+ VkPhysicalDeviceExtendedDynamicStateFeaturesEXT dynamic_state_feats = {};
feats.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2;
screen->feats11.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_VULKAN_1_1_FEATURES;
@@ -1060,6 +1069,11 @@ zink_internal_create_screen(struct sw_winsys *winsys, int fd, const struct pipe_
blend_feats.pNext = feats.pNext;
feats.pNext = &blend_feats;
}
+ if (have_EXT_extended_dynamic_state) {
+ dynamic_state_feats.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_EXTENDED_DYNAMIC_STATE_FEATURES_EXT;
+ dynamic_state_feats.pNext = feats.pNext;
+ feats.pNext = &dynamic_state_feats;
+ }
vkGetPhysicalDeviceFeatures2(screen->pdev, &feats);
memcpy(&screen->feats, &feats.features, sizeof(screen->feats));
if (have_tf_ext && tf_feats.transformFeedback)
@@ -1074,6 +1088,8 @@ zink_internal_create_screen(struct sw_winsys *winsys, int fd, const struct pipe_
screen->have_EXT_calibrated_timestamps = have_EXT_calibrated_timestamps;
if (have_EXT_custom_border_color && screen->border_color_feats.customBorderColors)
screen->have_EXT_custom_border_color = true;
+ if (have_EXT_extended_dynamic_state && dynamic_state_feats.extendedDynamicState)
+ screen->have_EXT_extended_dynamic_state = true;
VkPhysicalDeviceProperties2 props = {};
VkPhysicalDeviceVertexAttributeDivisorPropertiesEXT vdiv_props = {};
@@ -1150,7 +1166,7 @@ zink_internal_create_screen(struct sw_winsys *winsys, int fd, const struct pipe_
* this requires us to pass the whole VkPhysicalDeviceFeatures2 struct
*/
dci.pNext = &feats;
- const char *extensions[12] = {
+ const char *extensions[13] = {
VK_KHR_MAINTENANCE1_EXTENSION_NAME,
};
num_extensions = 1;
@@ -1185,6 +1201,8 @@ zink_internal_create_screen(struct sw_winsys *winsys, int fd, const struct pipe_
extensions[num_extensions++] = VK_EXT_CUSTOM_BORDER_COLOR_EXTENSION_NAME;
if (have_EXT_blend_operation_advanced)
extensions[num_extensions++] = VK_EXT_BLEND_OPERATION_ADVANCED_EXTENSION_NAME;
+ if (have_EXT_extended_dynamic_state)
+ extensions[num_extensions++] = VK_EXT_EXTENDED_DYNAMIC_STATE_EXTENSION_NAME;
assert(num_extensions <= ARRAY_SIZE(extensions));
dci.ppEnabledExtensionNames = extensions;
diff --git a/src/gallium/drivers/zink/zink_screen.h b/src/gallium/drivers/zink/zink_screen.h
index 4ee409c0efd..1d35e775262 100644
--- a/src/gallium/drivers/zink/zink_screen.h
+++ b/src/gallium/drivers/zink/zink_screen.h
@@ -75,6 +75,7 @@ struct zink_screen {
bool have_EXT_calibrated_timestamps;
bool have_EXT_custom_border_color;
bool have_EXT_blend_operation_advanced;
+ bool have_EXT_extended_dynamic_state;
bool have_X8_D24_UNORM_PACK32;
bool have_D24_UNORM_S8_UINT;
It’s awful, right? There’s obviously lots of copy/pasted code here, and it’s a tremendous waste of time to have to do the copy/pasting, not to mention the time needed for reviewing such mind-numbing changes.
After
Here’s the same patch after He Haocheng’s work has been merged:
diff --git a/src/gallium/drivers/zink/zink_device_info.py b/src/gallium/drivers/zink/zink_device_info.py
index 0300e7f7574..69e475df2cf 100644
--- a/src/gallium/drivers/zink/zink_device_info.py
+++ b/src/gallium/drivers/zink/zink_device_info.py
@@ -62,6 +62,7 @@ def EXTENSIONS():
Extension("VK_EXT_calibrated_timestamps"),
Extension("VK_EXT_custom_border_color", alias="border_color", properties=True, feature="customBorderColors"),
Extension("VK_EXT_blend_operation_advanced", alias="blend", properties=True),
+ Extension("VK_EXT_extended_dynamic_state", alias="dynamic_state", feature="extendedDynamicState"),
]
# There exists some inconsistencies regarding the enum constants, fix them.
diff --git a/src/gallium/drivers/zink/zink_screen.c b/src/gallium/drivers/zink/zink_screen.c
index 864ec32fc22..3c1214d384b 100644
--- a/src/gallium/drivers/zink/zink_screen.c
+++ b/src/gallium/drivers/zink/zink_screen.c
@@ -926,6 +926,10 @@ load_device_extensions(struct zink_screen *screen)
assert(have_device_time);
free(domains);
}
+ if (screen->info.have_EXT_extended_dynamic_state) {
+ GET_PROC_ADDR(CmdSetViewportWithCountEXT);
+ GET_PROC_ADDR(CmdSetScissorWithCountEXT);
+ }
#undef GET_PROC_ADDR
I’m certain this is going to lead to an increase in my own productivity in the future given how quick the process has now become.
A Short Note On Hardware
I’ve been putting up posts lately about my benchmarking figures, and in them I’ve been referencing Intel hardware. The reason I use Intel is because I’m (currently) a hobbyist developer with exactly one computer capable of doing graphics development, and it has an Intel onboard GPU. I’m quite happy with this given the high quality state of Intel’s drivers, as things become much more challenging when I have to debug both my own bugs as well as an underlying Vulkan driver’s bugs at the same time.
With that said, I present to you this recent out-of-context statement from Dave Airlie regarding zink performance on a different driver:
<airlied> zmike: hey on a fiji amd card, you get 45/46 native vs 35 fps with zink on one heaven scene here, however zink is corrupted
I don’t have any further information about anything there, but it’s the best I can do given the limitations in my available hardware.