[Mesa-dev] [PATCH v4] anv/device: fix maximum number of images supported
Jason Ekstrand
jason at jlekstrand.net
Wed Jan 16 15:28:15 UTC 2019
Yup.
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
On Wed, Jan 16, 2019 at 6:08 AM Iago Toral <itoral at igalia.com> wrote:
> Jason, does this version look good to you?
>
> On Mon, 2019-01-14 at 12:42 +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+ while making sure that only pre-SKL gens
> > use push constant space to handle images.
> >
> > v2:
> > - <= instead of < in the assert (Eric, Lionel)
> > - Change the way the assertion is written (Eric)
> >
> > v3:
> > - Revert the way the assertion is written to the form it had in v1,
> > the version in v2 was not equivalent and was incorrect. (Lionel)
> >
> > v4:
> > - gen9+ doesn't need push constants for images at all (Jason)
> > ---
> > src/intel/vulkan/anv_device.c | 7 ++++--
> > .../vulkan/anv_nir_apply_pipeline_layout.c | 4 +--
> > src/intel/vulkan/anv_private.h | 5 ++--
> > src/intel/vulkan/genX_cmd_buffer.c | 25 +++++++++++++--
> > ----
> > 4 files changed, 28 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_device.c
> > b/src/intel/vulkan/anv_device.c
> > index 523f1483e29..f85458b672e 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 b3daf702bc0..623984b0f8c 100644
> > --- a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c
> > +++ b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c
> > @@ -528,8 +528,8 @@ anv_nir_apply_pipeline_layout(const struct
> > anv_physical_device *pdevice,
> > }
> > }
> >
> > - if (map->image_count > 0) {
> > - assert(map->image_count <= MAX_IMAGES);
> > + if (map->image_count > 0 && pdevice->compiler->devinfo->gen < 9)
> > {
> > + assert(map->image_count <= MAX_GEN8_IMAGES);
> > 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 770254e93ea..47878adb066 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
> > @@ -1883,7 +1884,7 @@ struct anv_push_constants {
> > uint32_t base_work_group_id[3];
> >
> > /* Image data for image_load_store on pre-SKL */
> > - struct brw_image_param images[MAX_IMAGES];
> > + struct brw_image_param images[MAX_GEN8_IMAGES];
> > };
> >
> > struct anv_dynamic_state {
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > b/src/intel/vulkan/genX_cmd_buffer.c
> > index 35a70f7fe37..e23f8cfda45 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -2006,6 +2006,7 @@ emit_binding_table(struct anv_cmd_buffer
> > *cmd_buffer,
> > gl_shader_stage stage,
> > struct anv_state *bt_state)
> > {
> > + const struct gen_device_info *devinfo = &cmd_buffer->device-
> > >info;
> > struct anv_subpass *subpass = cmd_buffer->state.subpass;
> > struct anv_cmd_pipeline_state *pipe_state;
> > struct anv_pipeline *pipeline;
> > @@ -2063,7 +2064,8 @@ emit_binding_table(struct anv_cmd_buffer
> > *cmd_buffer,
> > if (map->surface_count == 0)
> > goto out;
> >
> > - if (map->image_count > 0) {
> > + /* We only use push constant space for images before gen9 */
> > + if (map->image_count > 0 && devinfo->gen < 9) {
> > VkResult result =
> > anv_cmd_buffer_ensure_push_constant_field(cmd_buffer,
> > stage, images);
> > if (result != VK_SUCCESS)
> > @@ -2177,10 +2179,15 @@ emit_binding_table(struct anv_cmd_buffer
> > *cmd_buffer,
> > assert(surface_state.alloc_size);
> > add_surface_state_relocs(cmd_buffer, sstate);
> >
> > - struct brw_image_param *image_param =
> > - &cmd_buffer->state.push_constants[stage]-
> > >images[image++];
> > + if (devinfo->gen < 9) {
> > + assert(image < MAX_GEN8_IMAGES);
> > + struct brw_image_param *image_param =
> > + &cmd_buffer->state.push_constants[stage]-
> > >images[image];
> >
> > - *image_param = desc->image_view->planes[binding-
> > >plane].storage_image_param;
> > + *image_param =
> > + desc->image_view->planes[binding-
> > >plane].storage_image_param;
> > + }
> > + image++;
> > break;
> > }
> >
> > @@ -2226,10 +2233,14 @@ emit_binding_table(struct anv_cmd_buffer
> > *cmd_buffer,
> > add_surface_reloc(cmd_buffer, surface_state,
> > desc->buffer_view->address);
> >
> > - struct brw_image_param *image_param =
> > - &cmd_buffer->state.push_constants[stage]-
> > >images[image++];
> > + if (devinfo->gen < 9) {
> > + assert(image < MAX_GEN8_IMAGES);
> > + struct brw_image_param *image_param =
> > + &cmd_buffer->state.push_constants[stage]-
> > >images[image];
> >
> > - *image_param = desc->buffer_view->storage_image_param;
> > + *image_param = desc->buffer_view->storage_image_param;
> > + }
> > + image++;
> > break;
> >
> > default:
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190116/cd238e9c/attachment-0001.html>
More information about the mesa-dev
mailing list