[Mesa-dev] [PATCH] anv/formats: handle correctly multisamples in gen7

[Jason Ekstrand]

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.

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!


>        .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.


> +   }
> +
>     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/20170215/50455e9a/attachment.html>