[Mesa-dev] [PATCH] anv/formats: handle correctly multisamples in gen7
Juan A. Suarez Romero
jasuarez at igalia.com
Thu Feb 16 11:37:52 UTC 2017
On Wed, 2017-02-15 at 10:24 -0800, Jason Ekstrand wrote:
> On Wed, Feb 15, 2017 at 10:09 AM, Juan A. Suarez Romero <jasuarez at igalia.com> wrote:
> > According to Ivybridge PRM, Volume 4 Part 1 p73, signed integer formats
> >
> > cannot be multisampled.
> >
> >
> >
> > Also in the same PRM p63, any format with greater than 64 bits per
> >
> > element cannot be multisampled.
> >
> >
> >
> > This fixes the following Vulkan CTS tests in Haswell:
> >
> >
> >
> > dEQP-VK.memory.requirements.image.regular_tiling_optimal
> >
> > dEQP-VK.memory.requirements.image.transient_tiling_optimal
> >
> >
> >
> > It also fixes crashes in the following Vulkan CTS tests in Haswell (becoming now
> >
> > skip):
> >
> >
> >
> > dEQP-VK.glsl.texture_functions.query.texturesamples.isampler2dms_fragment
> >
> > dEQP-VK.glsl.texture_functions.query.texturesamples.isampler2dms_vertex
> >
> > dEQP-VK.glsl.texture_functions.query.texturesamples.isampler2dmsarray_fragment
> >
> > dEQP-VK.glsl.texture_functions.query.texturesamples.isampler2dmsarray_vertex
> >
> > dEQP-VK.pipeline.multisample.sampled_image.64x64_1.r16g16_sint.samples_4
> >
> > dEQP-VK.pipeline.multisample.sampled_image.64x64_1.r16g16_sint.samples_8
> >
> > dEQP-VK.pipeline.multisample.sampled_image.64x64_1.r32g32b32a32_sfloat.samples_4
> >
> > dEQP-VK.pipeline.multisample.sampled_image.64x64_1.r32g32b32a32_sfloat.samples_8
> >
> > dEQP-VK.pipeline.multisample.sampled_image.64x64_4.r16g16_sint.samples_4
> >
> > dEQP-VK.pipeline.multisample.sampled_image.64x64_4.r16g16_sint.samples_8
> >
> > dEQP-VK.pipeline.multisample.sampled_image.64x64_4.r32g32b32a32_sfloat.samples_4
> >
> > dEQP-VK.pipeline.multisample.sampled_image.64x64_4.r32g32b32a32_sfloat.samples_8
> >
> > dEQP-VK.pipeline.multisample.sampled_image.79x31_1.r16g16_sint.samples_4
> >
> > dEQP-VK.pipeline.multisample.sampled_image.79x31_1.r16g16_sint.samples_8
> >
> > dEQP-VK.pipeline.multisample.sampled_image.79x31_1.r32g32b32a32_sfloat.samples_4
> >
> > dEQP-VK.pipeline.multisample.sampled_image.79x31_1.r32g32b32a32_sfloat.samples_8
> >
> > dEQP-VK.pipeline.multisample.sampled_image.79x31_4.r16g16_sint.samples_4
> >
> > dEQP-VK.pipeline.multisample.sampled_image.79x31_4.r16g16_sint.samples_8
> >
> > dEQP-VK.pipeline.multisample.sampled_image.79x31_4.r32g32b32a32_sfloat.samples_4
> >
> > dEQP-VK.pipeline.multisample.sampled_image.79x31_4.r32g32b32a32_sfloat.samples_8
> >
> >
> >
> > Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> >
> > ---
> >
> > src/intel/vulkan/anv_device.c | 4 ++--
> >
> > src/intel/vulkan/anv_formats.c | 32 ++++++++++++++++++++++++++++++++
> >
> > 2 files changed, 34 insertions(+), 2 deletions(-)
> >
> >
> >
> > diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> >
> > index d1a6cc8..a8ab8c3 100644
> >
> > --- a/src/intel/vulkan/anv_device.c
> >
> > +++ b/src/intel/vulkan/anv_device.c
> >
> > @@ -622,12 +622,12 @@ void anv_GetPhysicalDeviceProperties(
> >
> > .maxFramebufferWidth = (1 << 14),
> >
> > .maxFramebufferHeight = (1 << 14),
> >
> > .maxFramebufferLayers = (1 << 11),
> >
> > - .framebufferColorSampleCounts = sample_counts,
> >
> > + .framebufferColorSampleCounts = devinfo->gen == 7 ? VK_SAMPLE_COUNT_1_BIT : sample_counts,
> >
> > .framebufferDepthSampleCounts = sample_counts,
> >
> > .framebufferStencilSampleCounts = sample_counts,
> >
> > .framebufferNoAttachmentsSampleCounts = sample_counts,
> >
> > .maxColorAttachments = MAX_RTS,
> >
> > - .sampledImageColorSampleCounts = sample_counts,
> >
> > + .sampledImageColorSampleCounts = devinfo->gen == 7 ? VK_SAMPLE_COUNT_1_BIT : sample_counts,
>
> The Vulkan spec says we're supposed to support at least 1x and 4x on both of these.
>
You're right. I didn't notice about this requirement.
> 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!
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.
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.
I'll send a V2 version.
>
> > .sampledImageIntegerSampleCounts = VK_SAMPLE_COUNT_1_BIT,
> >
> > .sampledImageDepthSampleCounts = sample_counts,
> >
> > .sampledImageStencilSampleCounts = sample_counts,
> >
> > diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c
> >
> > index 6005791..3eac931 100644
> >
> > --- a/src/intel/vulkan/anv_formats.c
> >
> > +++ b/src/intel/vulkan/anv_formats.c
> >
> > @@ -561,6 +561,38 @@ anv_get_image_format_properties(
> >
> > sampleCounts = isl_device_get_sample_counts(&physical_device->isl_dev);
> >
> > }
> >
> >
> >
> > + /*
> >
> > + * From the Ivybridge PRM, Volume 4 Part 1 p73, SURFACE_STATE, Number of
> >
> > + * Multisamples:
> >
> > + *
> >
> > + * - This field must be set to MULTISAMPLECOUNT_1 for SINT MSRTs when
> >
> > + * all RT channels are not written.
> >
> > + *
> >
> > + * And errata from the Ivybridge PRM, Volume 4 Part 1 p77,
> >
> > + * RENDER_SURFACE_STATE, MCS Enable:
> >
> > + *
> >
> > + * This field must be set to 0 [MULTISAMPLECOUNT_1] for all SINT MSRTs
> >
> > + * when all RT channels are not written.
> >
> > + *
> >
> > + * Note that the above SINT restrictions apply only to *MSRTs* (that is,
> >
> > + * *multisampled* render targets). The restrictions seem to permit an MCS
> >
> > + * if the render target is singlesampled.
> >
> > + *
> >
> > + * From the IvyBridge PRM, Volume 4 Part 1 p63, SURFACE_STATE, Surface
> >
> > + * Format:
> >
> > + *
> >
> > + * If Number of Multisamples is set to a value other than
> >
> > + * MULTISAMPLECOUNT_1, this field cannot be set to the following
> >
> > + * formats: any format with greater than 64 bits per element, if Number
> >
> > + * of Multisamples is MULTISAMPLECOUNT_8, any compressed texture format
> >
> > + * (BC*), and any YCRCB* format.
> >
> > + */
> >
> > + if (physical_device->info.gen < 8 &&
> >
> > + (isl_format_has_sint_channel(anv_formats[info->format].isl_format) ||
> >
> > + isl_format_get_layout(anv_formats[info->format].isl_format)->bpb > 64)) {
> >
> > + sampleCounts = VK_SAMPLE_COUNT_1_BIT;
>
> 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.
>
Nice. I'll add it the check and use this function instead.
> > + }
> >
> > +
> >
> > if (info->usage & (VK_IMAGE_USAGE_TRANSFER_SRC_BIT |
> >
> > VK_IMAGE_USAGE_TRANSFER_DST_BIT)) {
> >
> > /* Accept transfers on anything we can sample from or renderer to. */
> >
> > --
> >
> > 2.9.3
> >
> >
> >
> >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170216/33245448/attachment-0001.html>
More information about the mesa-dev
mailing list