On Friday, 2019-01-11 10:58:38 +0100, Iago Toral Quiroga wrote:
> We had defined MAX_IMAGES as 8, which we used to size the array for
> image push constant data. The comment there stated that this was for
> gen8, but anv_nir_apply_pipeline_layout runs for all gens and writes
> that array, asserting that we don't exceed that number of images,
> which imposes a limit of MAX_IMAGES on all gens.
>
> Furthermore, despite this, we are exposing up to 64 images per shader
> stage on all gens, gen8 included.
>
> This patch lowers the number of images we expose in gen8 to 8 and
> keeps 64 images for gen9+. It also sets MAX_IMAGES to 64 so we can
> actually use up to 64 images in shaders, and then adds an assert
> specific to gen8 to check that we never exceed 8.
> ---
> src/intel/vulkan/anv_device.c | 7 +++++--
> src/intel/vulkan/anv_nir_apply_pipeline_layout.c | 4 +++-
> src/intel/vulkan/anv_private.h | 4 ++--
> 3 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index cd179e6801..122a58cfd6 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -987,9 +987,12 @@ void anv_GetPhysicalDeviceProperties(
> const uint32_t max_samplers = (devinfo->gen >= 8 || devinfo->is_haswell) ?
> 128 : 16;
>
> + const uint32_t max_images = devinfo->gen < 9 ? MAX_GEN8_IMAGES :
> MAX_IMAGES;
> +
> VkSampleCountFlags sample_counts =
> isl_device_get_sample_counts(&pdevice->isl_dev);
>
> +
> VkPhysicalDeviceLimits limits = {
> .maxImageDimension1D = (1 << 14),
> .maxImageDimension2D = (1 << 14),
> @@ -1009,7 +1012,7 @@ void anv_GetPhysicalDeviceProperties(
> .maxPerStageDescriptorUniformBuffers = 64,
> .maxPerStageDescriptorStorageBuffers = 64,
> .maxPerStageDescriptorSampledImages = max_samplers,
> - .maxPerStageDescriptorStorageImages = 64,
> + .maxPerStageDescriptorStorageImages = max_images,
> .maxPerStageDescriptorInputAttachments = 64,
> .maxPerStageResources = 250,
> .maxDescriptorSetSamplers = 6 * max_samplers, /*
> number of stages * maxPerStageDescriptorSamplers */
> @@ -1018,7 +1021,7 @@ void anv_GetPhysicalDeviceProperties(
> .maxDescriptorSetStorageBuffers = 6 * 64, /*
> number of stages * maxPerStageDescriptorStorageBuffers */
> .maxDescriptorSetStorageBuffersDynamic = MAX_DYNAMIC_BUFFERS / 2,
> .maxDescriptorSetSampledImages = 6 * max_samplers, /*
> number of stages * maxPerStageDescriptorSampledImages */
> - .maxDescriptorSetStorageImages = 6 * 64, /*
> number of stages * maxPerStageDescriptorStorageImages */
> + .maxDescriptorSetStorageImages = 6 * max_images, /*
> number of stages * maxPerStageDescriptorStorageImages */
> .maxDescriptorSetInputAttachments = 256,
> .maxVertexInputAttributes = MAX_VBS,
> .maxVertexInputBindings = MAX_VBS,
> diff --git a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c
> b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c
> index a0fd226b0a..e16ad4f032 100644
> --- a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c
> +++ b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c
> @@ -537,7 +537,9 @@ anv_nir_apply_pipeline_layout(const struct
> anv_physical_device *pdevice,
> }
>
> if (map->image_count > 0) {
> - assert(map->image_count <= MAX_IMAGES);
> + assert(map->image_count <= MAX_IMAGES &&
> + (pdevice->compiler->devinfo->gen > 8 ||
> + map->image_count < MAX_GEN8_IMAGES));
s/ < / <= /, but I would also write it differently:
assert((pdevice->compiler->devinfo->gen <= 8 && map->image_count <=
MAX_GEN8_IMAGES) ||
map->image_count <= MAX_IMAGES);
I don't know about the numbers, but code-wise it looks all reasonable.
> assert(shader->num_uniforms == prog_data->nr_params * 4);
> state.first_image_uniform = shader->num_uniforms;
> uint32_t *param = brw_stage_prog_data_add_params(prog_data,
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index 903931472d..3af7982bae 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -157,7 +157,8 @@ struct gen_l3_config;
> #define MAX_SCISSORS 16
> #define MAX_PUSH_CONSTANTS_SIZE 128
> #define MAX_DYNAMIC_BUFFERS 16
> -#define MAX_IMAGES 8
> +#define MAX_IMAGES 64
> +#define MAX_GEN8_IMAGES 8
> #define MAX_PUSH_DESCRIPTORS 32 /* Minimum requirement */
>
> /* The kernel relocation API has a limitation of a 32-bit delta value
> @@ -1864,7 +1865,6 @@ struct anv_push_constants {
> /* Used for vkCmdDispatchBase */
> uint32_t base_work_group_id[3];
>
> - /* Image data for image_load_store on pre-SKL */
> struct brw_image_param images[MAX_IMAGES];
> };
>
> --
> 2.17.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
