On Thu, 2017-02-16 at 12:37 +0100, Juan A. Suarez Romero wrote:
> 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
> > <jasua...@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 <jasua...@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.
>
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.
> 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
> > >
> > >
> > >
> > >
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
