<html><head></head><body><div>On Thu, 2017-02-16 at 12:37 +0100, Juan A. Suarez Romero wrote:</div><blockquote type="cite"><div>On Wed, 2017-02-15 at 10:24 -0800, Jason Ekstrand wrote:</div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Feb 15, 2017 at 10:09 AM, Juan A. Suarez Romero <span dir="ltr"><<a href="mailto:jasuarez@igalia.com" target="_blank">jasuarez@igalia.com</a>></span> wrote:<br><blockquote type="cite">According to Ivybridge PRM, Volume 4 Part 1 p73, signed integer formats<br>
cannot be multisampled.<br>
<br>
Also in the same PRM p63, any format with greater than 64 bits per<br>
element cannot be multisampled.<br>
<br>
This fixes the following Vulkan CTS tests in Haswell:<br>
<br>
dEQP-VK.memory.requirements.image.regular_tiling_optimal<br>
dEQP-VK.memory.requirements.image.transient_tiling_optimal<br>
<br>
It also fixes crashes in the following Vulkan CTS tests in Haswell (becoming now<br>
skip):<br>
<br>
dEQP-VK.glsl.texture_functions.query.texturesamples.isampler2dms_fragment<br>
dEQP-VK.glsl.texture_functions.query.texturesamples.isampler2dms_vertex<br>
dEQP-VK.glsl.texture_functions.query.texturesamples.isampler2dmsarray_fragment<br>
dEQP-VK.glsl.texture_functions.query.texturesamples.isampler2dmsarray_vertex<br>
dEQP-VK.pipeline.multisample.sampled_image.64x64_1.r16g16_sint.samples_4<br>
dEQP-VK.pipeline.multisample.sampled_image.64x64_1.r16g16_sint.samples_8<br>
dEQP-VK.pipeline.multisample.sampled_image.64x64_1.r32g32b32a32_sfloat.samples_4<br>
dEQP-VK.pipeline.multisample.sampled_image.64x64_1.r32g32b32a32_sfloat.samples_8<br>
dEQP-VK.pipeline.multisample.sampled_image.64x64_4.r16g16_sint.samples_4<br>
dEQP-VK.pipeline.multisample.sampled_image.64x64_4.r16g16_sint.samples_8<br>
dEQP-VK.pipeline.multisample.sampled_image.64x64_4.r32g32b32a32_sfloat.samples_4<br>
dEQP-VK.pipeline.multisample.sampled_image.64x64_4.r32g32b32a32_sfloat.samples_8<br>
dEQP-VK.pipeline.multisample.sampled_image.79x31_1.r16g16_sint.samples_4<br>
dEQP-VK.pipeline.multisample.sampled_image.79x31_1.r16g16_sint.samples_8<br>
dEQP-VK.pipeline.multisample.sampled_image.79x31_1.r32g32b32a32_sfloat.samples_4<br>
dEQP-VK.pipeline.multisample.sampled_image.79x31_1.r32g32b32a32_sfloat.samples_8<br>
dEQP-VK.pipeline.multisample.sampled_image.79x31_4.r16g16_sint.samples_4<br>
dEQP-VK.pipeline.multisample.sampled_image.79x31_4.r16g16_sint.samples_8<br>
dEQP-VK.pipeline.multisample.sampled_image.79x31_4.r32g32b32a32_sfloat.samples_4<br>
dEQP-VK.pipeline.multisample.sampled_image.79x31_4.r32g32b32a32_sfloat.samples_8<br>
<br>
Signed-off-by: Juan A. Suarez Romero <<a href="mailto:jasuarez@igalia.com">jasuarez@igalia.com</a>><br>
---<br>
src/intel/vulkan/anv_device.c | 4 ++--<br>
src/intel/vulkan/anv_formats.c | 32 ++++++++++++++++++++++++++++++++<br>
2 files changed, 34 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c<br>
index d1a6cc8..a8ab8c3 100644<br>
--- a/src/intel/vulkan/anv_device.c<br>
+++ b/src/intel/vulkan/anv_device.c<br>
@@ -622,12 +622,12 @@ void anv_GetPhysicalDeviceProperties(<br>
.maxFramebufferWidth = (1 << 14),<br>
.maxFramebufferHeight = (1 << 14),<br>
.maxFramebufferLayers = (1 << 11),<br>
- .framebufferColorSampleCounts = sample_counts,<br>
+ .framebufferColorSampleCounts = devinfo->gen == 7 ? VK_SAMPLE_COUNT_1_BIT : sample_counts,<br>
.framebufferDepthSampleCounts = sample_counts,<br>
.framebufferStencilSampleCounts = sample_counts,<br>
.framebufferNoAttachmentsSampleCounts = sample_counts,<br>
.maxColorAttachments = MAX_RTS,<br>
- .sampledImageColorSampleCounts = sample_counts,<br>
+ .sampledImageColorSampleCounts = devinfo->gen == 7 ? VK_SAMPLE_COUNT_1_BIT : sample_counts,<br></blockquote><div><br></div><div>The Vulkan spec says we're supposed to support at least 1x and 4x on both of these.<br><br></div></div></div></div></blockquote><div><br></div><div>You're right. I didn't notice about this requirement.</div><div><br></div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>So I guess the real question is what's the best choice that will let apps run. My feeling is that setting these fields to 1x and 4x and then just dying horribly if they use an integer format is probably actually the better choice. That said, we should definitely make sure we're dying horribly if they use an integer format!<br></div></div></div></div></blockquote><div><br></div><div>I agree. Right now we are already crashing if we try it to use that 4x multisampling with a SINT format. With this patch, at least if user asks if this combination is supported with vkGetPhysicalDeviceImageFormatProperties(), we will return FALSE.</div><div><br></div><div>The bad part is that we are somewhat breaking the spec: sampleCounts returned by that function must be a superset of framebufferColorSampleCounts; but we are returning 1x when framebufferColorSampleCounts is 1x and 4x. But I think we can live with it.</div><div><br></div></blockquote><div><br></div><div><span style="font-family: monospace;">In fact, this is causing a regression in dEQP-VK.api.info.image_format_properties.2d.optimal.* tests, for those formats that either have a SINT or bpb > 0, because it detects 4x multisampling is not allowed. But I think we can't do much more about it, unless we remove those formats as supported.</span></div><div><span style="font-family: monospace;"><br></span><br></div><div><span style="font-family: monospace;"><br></span></div><blockquote type="cite"><div>I'll send a V2 version.</div><div><br></div><div><br></div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote type="cite">
.sampledImageIntegerSampleCounts = VK_SAMPLE_COUNT_1_BIT,<br>
.sampledImageDepthSampleCounts = sample_counts,<br>
.sampledImageStencilSampleCounts = sample_counts,<br>
diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c<br>
index 6005791..3eac931 100644<br>
--- a/src/intel/vulkan/anv_formats.c<br>
+++ b/src/intel/vulkan/anv_formats.c<br>
@@ -561,6 +561,38 @@ anv_get_image_format_properties(<br>
sampleCounts = isl_device_get_sample_counts(&physical_device->isl_dev);<br>
}<br>
<br>
+ /*<br>
+ * From the Ivybridge PRM, Volume 4 Part 1 p73, SURFACE_STATE, Number of<br>
+ * Multisamples:<br>
+ *<br>
+ * - This field must be set to MULTISAMPLECOUNT_1 for SINT MSRTs when<br>
+ * all RT channels are not written.<br>
+ *<br>
+ * And errata from the Ivybridge PRM, Volume 4 Part 1 p77,<br>
+ * RENDER_SURFACE_STATE, MCS Enable:<br>
+ *<br>
+ * This field must be set to 0 [MULTISAMPLECOUNT_1] for all SINT MSRTs<br>
+ * when all RT channels are not written.<br>
+ *<br>
+ * Note that the above SINT restrictions apply only to *MSRTs* (that is,<br>
+ * *multisampled* render targets). The restrictions seem to permit an MCS<br>
+ * if the render target is singlesampled.<br>
+ *<br>
+ * From the IvyBridge PRM, Volume 4 Part 1 p63, SURFACE_STATE, Surface<br>
+ * Format:<br>
+ *<br>
+ * If Number of Multisamples is set to a value other than<br>
+ * MULTISAMPLECOUNT_1, this field cannot be set to the following<br>
+ * formats: any format with greater than 64 bits per element, if Number<br>
+ * of Multisamples is MULTISAMPLECOUNT_8, any compressed texture format<br>
+ * (BC*), and any YCRCB* format.<br>
+ */<br>
+ if (physical_device->info.gen < 8 &&<br>
+ (isl_format_has_sint_channel(anv_formats[info->format].isl_format) ||<br>
+ isl_format_get_layout(anv_formats[info->format].isl_format)->bpb > 64)) {<br>
+ sampleCounts = VK_SAMPLE_COUNT_1_BIT;<br></blockquote><div><br></div><div>We have this very helpful function in isl called isl_format_supports_multisampling which I don't believe has the SINT restriction yet. I think the better thing to do would be to add the SINT restriction to that helper and use it rather than hand-coding it here.<br></div><div> </div></div></div></div></blockquote><div><br></div><div>Nice. I'll add it the check and use this function instead.</div><div><br></div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote type="cite">
+ }<br>
+<br>
if (info->usage & (VK_IMAGE_USAGE_TRANSFER_SRC_BIT |<br>
VK_IMAGE_USAGE_TRANSFER_DST_BIT)) {<br>
/* Accept transfers on anything we can sample from or renderer to. */<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.9.3<br>
<br>
</font></span><br></blockquote></div><br></div></div>
</blockquote><pre>_______________________________________________
mesa-dev mailing list
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>
</pre></blockquote></body></html>