<div dir="ltr"><div>Yup, that'll do it. Gotta watch out for ++... Assuming it fixes the problem, that patch is</div><div><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Jan 17, 2019 at 12:35 PM Lionel Landwerlin <<a href="mailto:lionel.g.landwerlin@intel.com">lionel.g.landwerlin@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<div class="gmail-m_6603867202764786441moz-cite-prefix">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.</div>
<div class="gmail-m_6603867202764786441moz-cite-prefix">Trying this change on CI :
<a class="gmail-m_6603867202764786441moz-txt-link-freetext" href="https://github.com/djdeath/mesa/commit/a6b8eaf1325389d94d1d8a5b3bb952a362125eb2" target="_blank">https://github.com/djdeath/mesa/commit/a6b8eaf1325389d94d1d8a5b3bb952a362125eb2</a><br>
</div>
<div class="gmail-m_6603867202764786441moz-cite-prefix"><br>
</div>
<div class="gmail-m_6603867202764786441moz-cite-prefix">On 17/01/2019 18:19, Clayton Craft
wrote:<br>
</div>
<blockquote type="cite">
<pre class="gmail-m_6603867202764786441moz-quote-pre">Quoting Mark Janes (2019-01-17 10:13:37)
</pre>
<blockquote type="cite">
<pre class="gmail-m_6603867202764786441moz-quote-pre">Hi Iago,
It looks like you tested this patch in CI and got the same failures that
we are seeing on master:
<a class="gmail-m_6603867202764786441moz-txt-link-freetext" href="http://mesa-ci-results.jf.intel.com/itoral/builds/263/group/63a9f0ea7bb98050796b649e85481845" target="_blank">http://mesa-ci-results.jf.intel.com/itoral/builds/263/group/63a9f0ea7bb98050796b649e85481845</a>
</pre>
</blockquote>
<pre class="gmail-m_6603867202764786441moz-quote-pre">The correct link is:
<a class="gmail-m_6603867202764786441moz-txt-link-freetext" href="https://mesa-ci.01.org/itoral/builds/263/group/63a9f0ea7bb98050796b649e85481845" target="_blank">https://mesa-ci.01.org/itoral/builds/263/group/63a9f0ea7bb98050796b649e85481845</a>
</pre>
<blockquote type="cite">
<pre class="gmail-m_6603867202764786441moz-quote-pre">Why was this patch pushed?
-Mark
Mark Janes <a class="gmail-m_6603867202764786441moz-txt-link-rfc2396E" href="mailto:mark.a.janes@intel.com" target="_blank"><mark.a.janes@intel.com></a> writes:
</pre>
<blockquote type="cite">
<pre class="gmail-m_6603867202764786441moz-quote-pre">This patch regresses thousands of tests on BDW and HSW:
<a class="gmail-m_6603867202764786441moz-txt-link-freetext" href="http://mesa-ci-results.jf.intel.com/vulkancts/builds/10035/group/63a9f0ea7bb98050796b649e85481845#fails" target="_blank">http://mesa-ci-results.jf.intel.com/vulkancts/builds/10035/group/63a9f0ea7bb98050796b649e85481845#fails</a>
</pre>
</blockquote>
</blockquote>
<pre class="gmail-m_6603867202764786441moz-quote-pre"><a class="gmail-m_6603867202764786441moz-txt-link-freetext" href="https://mesa-ci.01.org/vulkancts/builds/10035/group/63a9f0ea7bb98050796b649e85481845#fails" target="_blank">https://mesa-ci.01.org/vulkancts/builds/10035/group/63a9f0ea7bb98050796b649e85481845#fails</a>
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre class="gmail-m_6603867202764786441moz-quote-pre">I'll revert it as soon as my testing completes.
Iago Toral Quiroga <a class="gmail-m_6603867202764786441moz-txt-link-rfc2396E" href="mailto:itoral@igalia.com" target="_blank"><itoral@igalia.com></a> writes:
</pre>
<blockquote type="cite">
<pre class="gmail-m_6603867202764786441moz-quote-pre">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:
--
2.17.1
_______________________________________________
mesa-dev mailing list
<a class="gmail-m_6603867202764786441moz-txt-link-abbreviated" href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a>
<a class="gmail-m_6603867202764786441moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>
</pre>
</blockquote>
</blockquote>
<pre class="gmail-m_6603867202764786441moz-quote-pre">_______________________________________________
mesa-dev mailing list
<a class="gmail-m_6603867202764786441moz-txt-link-abbreviated" href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a>
<a class="gmail-m_6603867202764786441moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>
</pre>
<br>
<fieldset class="gmail-m_6603867202764786441mimeAttachmentHeader"></fieldset>
<pre class="gmail-m_6603867202764786441moz-quote-pre">_______________________________________________
mesa-dev mailing list
<a class="gmail-m_6603867202764786441moz-txt-link-abbreviated" href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a>
<a class="gmail-m_6603867202764786441moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>
</pre>
</blockquote>
</blockquote>
<p><br>
</p>
</div>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div>