[Mesa-dev] [PATCH v4] anv/device: fix maximum number of images supported
Iago Toral
itoral at igalia.com
Fri Jan 18 06:54:05 UTC 2019
On Thu, 2019-01-17 at 12:57 -0600, Jason Ekstrand wrote:
> Yup, that'll do it. Gotta watch out for ++... Assuming it fixes the
> problem, that patch is
>
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
>
>
> On Thu, Jan 17, 2019 at 12:35 PM Lionel Landwerlin <
> lionel.g.landwerlin at intel.com> wrote:
> >
> >
> >
> >
> > Looking at the change the binding table
> > emission, I think the image++ has been moved such that it
> > doesn't
> > produce the same tables anymore.
> > Trying this change on CI :
> >
https://github.com/djdeath/mesa/commit/a6b8eaf1325389d94d1d8a5b3bb952a362125eb2
> >
> >
> >
Yes, this is what the patch that I sent for review did, I just pushed a
previous version, sorry for the mess :-(
Iago
> > On 17/01/2019 18:19, Clayton Craft
> > wrote:
> >
> >
> >
> > > Quoting Mark Janes (2019-01-17 10:13:37)
> > >
> > >
> > > > Hi Iago,
> > > > It looks like you tested this patch in CI and got the same
> > > > failures thatwe are seeing on master:
> > > >
http://mesa-ci-results.jf.intel.com/itoral/builds/263/group/63a9f0ea7bb98050796b649e85481845
> > > >
> > > >
> > >
> > > The correct link is:
> > > https://mesa-ci.01.org/itoral/builds/263/group/63a9f0ea7bb98050796b649e85481845
> > >
> > >
> > >
> > > > Why was this patch pushed?
> > > > -Mark
> > > > Mark Janes <mark.a.janes at intel.com> writes:
> > > >
> > > >
> > > >
> > > > > This patch regresses thousands of tests on BDW and
> > > > > HSW:
> > > > >
http://mesa-ci-results.jf.intel.com/vulkancts/builds/10035/group/63a9f0ea7bb98050796b649e85481845#fails
> > > > >
> > > > >
> > > >
> > > >
> > >
> > >
> > > https://mesa-ci.01.org/vulkancts/builds/10035/group/63a9f0ea7bb98050796b649e85481845#fails
> > >
> > >
> > >
> > > >
> > > > > I'll revert it as soon as my testing completes.
> > > > > Iago Toral Quiroga <itoral at igalia.com> writes:
> > > > >
> > > > >
> > > > >
> > > > > > We had defined MAX_IMAGES as 8, which we used
> > > > > > to size the array forimage push constant data. The comment
> > > > > > there stated that this was forgen8, but
> > > > > > anv_nir_apply_pipeline_layout runs for all gens and
> > > > > > writesthat 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 shaderstage on all gens, gen8 included.
> > > > > > This patch lowers the number of images we expose in gen8 to
> > > > > > 8 andkeeps 64 images for gen9+ while making sure that only
> > > > > > pre-SKL gensuse 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.cindex
> > > > > > 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( .maxPerStageDescript
> > > > > > orUniformBuffers =
> > > > > > 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( .maxDescriptorSetSto
> > > > > > rageBuffers = 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.cindex
> > > > > > 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.hindex
> > > > > > 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.cindex
> > > > > > 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_bu
> > > > > > ffer, 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/20190118/e4d26022/attachment-0001.html>
More information about the mesa-dev
mailing list