[Mesa-dev] [PATCH 4/9] anv: Added support for non-dynamic sample locations on Gen8+
Jason Ekstrand
jason at jlekstrand.net
Wed Mar 13 13:11:58 UTC 2019
On Mon, Mar 11, 2019 at 10:05 AM Eleni Maria Stea <estea at igalia.com> wrote:
> Allowing the user to set custom sample locations non-dynamically, by
> filling the extension structs and chaining them to the pipeline structs
> according to the Vulkan specification section [26.5. Custom Sample
> Locations]
> for the following structures:
>
> 'VkPipelineSampleLocationsStateCreateInfoEXT'
> 'VkSampleLocationsInfoEXT'
> 'VkSampleLocationEXT'
>
> Once custom locations are used, the default locations are lost and need to
> be
> re-emitted again in the next pipeline creation. For that, we emit the
> 3DSTATE_SAMPLE_PATTERN at every pipeline creation.
> ---
> src/intel/common/gen_sample_positions.h | 53 ++++++++++++++++
> src/intel/vulkan/anv_genX.h | 5 ++
> src/intel/vulkan/anv_private.h | 9 +++
> src/intel/vulkan/anv_sample_locations.c | 38 +++++++++++-
> src/intel/vulkan/anv_sample_locations.h | 29 +++++++++
> src/intel/vulkan/genX_pipeline.c | 80 +++++++++++++++++++++----
> src/intel/vulkan/genX_state.c | 59 ++++++++++++++++++
> 7 files changed, 259 insertions(+), 14 deletions(-)
> create mode 100644 src/intel/vulkan/anv_sample_locations.h
>
> diff --git a/src/intel/common/gen_sample_positions.h
> b/src/intel/common/gen_sample_positions.h
> index da48dcb5ed0..e8af2a552dc 100644
> --- a/src/intel/common/gen_sample_positions.h
> +++ b/src/intel/common/gen_sample_positions.h
> @@ -160,4 +160,57 @@ prefix##14YOffset = 0.9375; \
> prefix##15XOffset = 0.0625; \
> prefix##15YOffset = 0.0000;
>
> +/* Examples:
> + * in case of GEN_GEN < 8:
> + * SET_SAMPLE_POS(ms.Sample, 0); expands to:
> + * ms.Sample0XOffset = anv_samples[0].offs_x;
> + * ms.Sample0YOffset = anv_samples[0].offs_y;
> + *
> + * in case of GEN_GEN >= 8:
> + * SET_SAMPLE_POS(sp._16xSample, 0); expands to:
> + * sp._16xSample0XOffset = anv_samples[0].offs_x;
> + * sp._16xSample0YOffset = anv_samples[0].offs_y;
> + */
> +#define SET_SAMPLE_POS(prefix, sample_idx) \
> +prefix##sample_idx##XOffset = anv_samples[sample_idx].offs_x; \
> +prefix##sample_idx##YOffset = anv_samples[sample_idx].offs_y;
>
I'm not a huge fan of hanving anv-specific stuff in gen_sample_positions.h
and I also don't think I really like having the array be implicit in the
macro. Maybe the best thing to do here would be to have a
struct gen_sample_position {
float x_offset;
float y_offset;
}
And then
#define GEN_SAMPLE_POS_ELEM(prefix, arr, sample_idx) \
prefix##sample_idx##XOffset = arr[sample_idx].x_offset; \
prefix##sample_idx##YOffset = arr[sample_idx].y_offset;
And
#define GEN_SAMPLE_POS_1X_ARRAY(prefix, arr) \
SET_SAMPLE_POS_ELEM(prefix, arr, 0)
#define GEN_SAMPLE_POS_2X_ARRAY(prefix, arr) \
SET_SAMPLE_POS_ELEM(prefix, arr, 0) \
SET_SAMPLE_POS_ELEM(prefix, arr, 1)
etc.
+
> +#define SET_SAMPLE_POS_2X(prefix) \
> +SET_SAMPLE_POS(prefix, 0); \
> +SET_SAMPLE_POS(prefix, 1);
> +
> +#define SET_SAMPLE_POS_4X(prefix) \
> +SET_SAMPLE_POS(prefix, 0); \
> +SET_SAMPLE_POS(prefix, 1); \
> +SET_SAMPLE_POS(prefix, 2); \
> +SET_SAMPLE_POS(prefix, 3);
> +
> +#define SET_SAMPLE_POS_8X(prefix) \
> +SET_SAMPLE_POS(prefix, 0); \
> +SET_SAMPLE_POS(prefix, 1); \
> +SET_SAMPLE_POS(prefix, 2); \
> +SET_SAMPLE_POS(prefix, 3); \
> +SET_SAMPLE_POS(prefix, 4); \
> +SET_SAMPLE_POS(prefix, 5); \
> +SET_SAMPLE_POS(prefix, 6); \
> +SET_SAMPLE_POS(prefix, 7);
> +
> +#define SET_SAMPLE_POS_16X(prefix) \
> +SET_SAMPLE_POS(prefix, 0); \
> +SET_SAMPLE_POS(prefix, 1); \
> +SET_SAMPLE_POS(prefix, 2); \
> +SET_SAMPLE_POS(prefix, 3); \
> +SET_SAMPLE_POS(prefix, 4); \
> +SET_SAMPLE_POS(prefix, 5); \
> +SET_SAMPLE_POS(prefix, 6); \
> +SET_SAMPLE_POS(prefix, 7); \
> +SET_SAMPLE_POS(prefix, 8); \
> +SET_SAMPLE_POS(prefix, 9); \
> +SET_SAMPLE_POS(prefix, 10); \
> +SET_SAMPLE_POS(prefix, 11); \
> +SET_SAMPLE_POS(prefix, 12); \
> +SET_SAMPLE_POS(prefix, 13); \
> +SET_SAMPLE_POS(prefix, 14); \
> +SET_SAMPLE_POS(prefix, 15);
> +
> #endif /* GEN_SAMPLE_POSITIONS_H */
> diff --git a/src/intel/vulkan/anv_genX.h b/src/intel/vulkan/anv_genX.h
> index 8fd32cabf1e..52415c04a45 100644
> --- a/src/intel/vulkan/anv_genX.h
> +++ b/src/intel/vulkan/anv_genX.h
> @@ -88,3 +88,8 @@ void genX(cmd_buffer_mi_memset)(struct anv_cmd_buffer
> *cmd_buffer,
>
> void genX(blorp_exec)(struct blorp_batch *batch,
> const struct blorp_params *params);
> +
> +void genX(emit_sample_locations)(struct anv_batch *batch,
> + uint32_t num_samples,
> + const VkSampleLocationsInfoEXT *sl,
> + bool custom_locations);
> diff --git a/src/intel/vulkan/anv_private.h
> b/src/intel/vulkan/anv_private.h
> index 5905299e59d..981956e5706 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -71,6 +71,7 @@ struct anv_buffer;
> struct anv_buffer_view;
> struct anv_image_view;
> struct anv_instance;
> +struct anv_sample;
>
> struct gen_l3_config;
>
> @@ -165,6 +166,7 @@ struct gen_l3_config;
> #define MAX_PUSH_DESCRIPTORS 32 /* Minimum requirement */
> #define MAX_INLINE_UNIFORM_BLOCK_SIZE 4096
> #define MAX_INLINE_UNIFORM_BLOCK_DESCRIPTORS 32
> +#define MAX_SAMPLE_LOCATIONS 16
>
> /* The kernel relocation API has a limitation of a 32-bit delta value
> * applied to the address before it is written which, in spite of it being
> @@ -2086,6 +2088,13 @@ struct anv_push_constants {
> struct brw_image_param images[MAX_GEN8_IMAGES];
> };
>
> +struct
> +anv_sample {
> + float offs_x;
> + float offs_y;
> + float radius;
> +};
>
Here, you could either embed a gen_sample_pos struct or you could just use
it directly and calculate the radius on the fly in the comparison
function. That radius computation isn't so expensive that I'd worry about
it on a qsort of 16 sample locations.
> +
> struct anv_dynamic_state {
> struct {
> uint32_t count;
> diff --git a/src/intel/vulkan/anv_sample_locations.c
> b/src/intel/vulkan/anv_sample_locations.c
> index 1ebf280e05b..c660cb5ae84 100644
> --- a/src/intel/vulkan/anv_sample_locations.c
> +++ b/src/intel/vulkan/anv_sample_locations.c
> @@ -21,7 +21,7 @@
> * IN THE SOFTWARE.
> */
>
> -#include "anv_private.h"
> +#include "anv_sample_locations.h"
>
> void
> anv_GetPhysicalDeviceMultisamplePropertiesEXT(VkPhysicalDevice
> physicalDevice,
> @@ -58,3 +58,39 @@
> anv_GetPhysicalDeviceMultisamplePropertiesEXT(VkPhysicalDevice
> physicalDevice,
> .maxSampleLocationGridSize = grid_size
> };
> }
> +
> +static int
> +compare_samples(const void *a, const void *b)
> +{
> + struct anv_sample *s1 = (struct anv_sample *)a;
> + struct anv_sample *s2 = (struct anv_sample *)b;
> +
> + return s1->radius - s2->radius;
> +}
> +
> +void
> +anv_calc_sample_locations(struct anv_sample *samples,
> + uint32_t num_samples,
> + const VkSampleLocationsInfoEXT *info)
> +{
> + int i;
> +
> + for(i = 0; i < num_samples; i++) {
> + float dx, dy;
> +
> + /* this is because the grid is 1x1, in case that
> + * we support different grid sizes in the future
> + * this must be changed.
> + */
> + samples[i].offs_x = info->pSampleLocations[i].x;
> + samples[i].offs_y = info->pSampleLocations[i].y;
> +
> + /* distance from the center */
> + dx = samples[i].offs_x - 0.5;
> + dy = samples[i].offs_y - 0.5;
> +
> + samples[i].radius = dx * dx + dy * dy;
> + }
> +
> + qsort(samples, num_samples, sizeof *samples, compare_samples);
> +}
> diff --git a/src/intel/vulkan/anv_sample_locations.h
> b/src/intel/vulkan/anv_sample_locations.h
> new file mode 100644
> index 00000000000..000d04aca08
> --- /dev/null
> +++ b/src/intel/vulkan/anv_sample_locations.h
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> next
> + * paragraph) shall be included in all copies or substantial portions of
> the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include "anv_private.h"
> +
> +void
> +anv_calc_sample_locations(struct anv_sample *samples,
> + uint32_t num_samples,
> + const VkSampleLocationsInfoEXT *info);
> diff --git a/src/intel/vulkan/genX_pipeline.c
> b/src/intel/vulkan/genX_pipeline.c
> index 975052deb79..feb431e10cd 100644
> --- a/src/intel/vulkan/genX_pipeline.c
> +++ b/src/intel/vulkan/genX_pipeline.c
> @@ -22,6 +22,7 @@
> */
>
> #include "anv_private.h"
> +#include "anv_sample_locations.h"
>
> #include "genxml/gen_macros.h"
> #include "genxml/genX_pack.h"
> @@ -548,12 +549,9 @@ emit_rs_state(struct anv_pipeline *pipeline,
> }
>
> static void
> -emit_ms_state(struct anv_pipeline *pipeline,
> - const VkPipelineMultisampleStateCreateInfo *info)
> +emit_sample_mask(struct anv_pipeline *pipeline,
> + const VkPipelineMultisampleStateCreateInfo *info)
> {
> - uint32_t samples = 1;
> - uint32_t log2_samples = 0;
> -
> /* From the Vulkan 1.0 spec:
> * If pSampleMask is NULL, it is treated as if the mask has all bits
> * enabled, i.e. no coverage is removed from fragments.
> @@ -566,14 +564,19 @@ emit_ms_state(struct anv_pipeline *pipeline,
> uint32_t sample_mask = 0xff;
> #endif
>
> - if (info) {
> - samples = info->rasterizationSamples;
> - log2_samples = __builtin_ffs(samples) - 1;
> - }
> -
> if (info && info->pSampleMask)
> sample_mask &= info->pSampleMask[0];
>
> + anv_batch_emit(&pipeline->batch, GENX(3DSTATE_SAMPLE_MASK), sm) {
> + sm.SampleMask = sample_mask;
> + }
> +}
> +
> +static void
> +emit_multisample(struct anv_pipeline *pipeline,
> + uint32_t samples,
> + uint32_t log2_samples)
> +{
> anv_batch_emit(&pipeline->batch, GENX(3DSTATE_MULTISAMPLE), ms) {
> ms.NumberofMultisamples = log2_samples;
>
> @@ -605,10 +608,61 @@ emit_ms_state(struct anv_pipeline *pipeline,
> }
> #endif
> }
> +}
>
> - anv_batch_emit(&pipeline->batch, GENX(3DSTATE_SAMPLE_MASK), sm) {
> - sm.SampleMask = sample_mask;
> +static void
> +emit_ms_state(struct anv_pipeline *pipeline,
> + const VkPipelineMultisampleStateCreateInfo *info,
> + const VkPipelineDynamicStateCreateInfo *dinfo)
> +{
> +#if GEN_GEN >= 8
> + bool custom_locations = false;
> + VkSampleLocationsInfoEXT *sl;
> +#endif
> +
> + uint32_t samples = 1;
> + uint32_t log2_samples = 0;
> +
> + emit_sample_mask(pipeline, info);
> +
> + if (info) {
> + samples = info->rasterizationSamples;
> +
> +#if GEN_GEN >= 8
> + if (info->pNext) {
> + VkPipelineSampleLocationsStateCreateInfoEXT *slinfo =
> + (VkPipelineSampleLocationsStateCreateInfoEXT *)info->pNext;
> +
> + if (slinfo &&
> + (slinfo->sType ==
> +
> VK_STRUCTURE_TYPE_PIPELINE_SAMPLE_LOCATIONS_STATE_CREATE_INFO_EXT)) {
> + if (slinfo->sampleLocationsEnable == VK_TRUE) {
> + uint32_t i;
> +
> + for (i = 0; i < dinfo->dynamicStateCount; i++) {
> + if (dinfo->pDynamicStates[i] ==
> + VK_DYNAMIC_STATE_SAMPLE_LOCATIONS_EXT)
> + return;
> + }
> +
> + if ((sl = &slinfo->sampleLocationsInfo))
> + samples = sl->sampleLocationsPerPixel;
> +
> + custom_locations = true;
> + }
> + }
> + }
> +#endif
> +
> + log2_samples = __builtin_ffs(samples) - 1;
> }
> +
> + emit_multisample(pipeline, samples, log2_samples);
> +
> +#if GEN_GEN >= 8
> + genX(emit_sample_locations)(&pipeline->batch, samples, sl,
> + custom_locations);
> +#endif
> }
>
> static const uint32_t vk_to_gen_logic_op[] = {
> @@ -1938,7 +1992,7 @@ genX(graphics_pipeline_create)(
> assert(pCreateInfo->pRasterizationState);
> emit_rs_state(pipeline, pCreateInfo->pRasterizationState,
> pCreateInfo->pMultisampleState, pass, subpass);
> - emit_ms_state(pipeline, pCreateInfo->pMultisampleState);
> + emit_ms_state(pipeline, pCreateInfo->pMultisampleState,
> pCreateInfo->pDynamicState);
> emit_ds_state(pipeline, pCreateInfo->pDepthStencilState, pass,
> subpass);
> emit_cb_state(pipeline, pCreateInfo->pColorBlendState,
> pCreateInfo->pMultisampleState);
> diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c
> index cffd1e47247..c14e502a68d 100644
> --- a/src/intel/vulkan/genX_state.c
> +++ b/src/intel/vulkan/genX_state.c
> @@ -28,6 +28,7 @@
> #include <fcntl.h>
>
> #include "anv_private.h"
> +#include "anv_sample_locations.h"
>
> #include "common/gen_sample_positions.h"
> #include "genxml/gen_macros.h"
> @@ -435,3 +436,61 @@ VkResult genX(CreateSampler)(
>
> return VK_SUCCESS;
> }
> +
> +void
> +genX(emit_sample_locations)(struct anv_batch *batch,
> + uint32_t num_samples,
> + const VkSampleLocationsInfoEXT *sl,
> + bool custom_locations)
> +{
> +#if GEN_GEN >= 8
> + struct anv_sample anv_samples[MAX_SAMPLE_LOCATIONS];
> +
> +#if GEN_GEN == 10
> + gen10_emit_wa_cs_stall_flush(batch);
> +#endif
> +
> + if (custom_locations) {
> + anv_calc_sample_locations(anv_samples, num_samples, sl);
>
Except your doing it in an emit function... Is this ever going to be in the
hot path? If so, maybe the thing we should cache should be the sorted
thing? I'm also a bit unsure about the sorting.
> +
> + anv_batch_emit(batch, GENX(3DSTATE_SAMPLE_PATTERN), sp) {
> + switch (num_samples) {
> + case 1:
> + SET_SAMPLE_POS(sp._1xSample, 0);
> + break;
> + case 2:
> + SET_SAMPLE_POS_2X(sp._2xSample);
> + break;
> + case 4:
> + SET_SAMPLE_POS_4X(sp._4xSample);
> + break;
> + case 8:
> + SET_SAMPLE_POS_8X(sp._8xSample);
> + break;
> +#if GEN_GEN >= 9
> + case 16:
> + SET_SAMPLE_POS_16X(sp._16xSample);
> + break;
> +#endif
> + default:
> + break;
> + }
> + }
> + } else {
> + anv_batch_emit(batch, GENX(3DSTATE_SAMPLE_PATTERN), sp) {
> + GEN_SAMPLE_POS_1X(sp._1xSample);
> + GEN_SAMPLE_POS_2X(sp._2xSample);
> + GEN_SAMPLE_POS_4X(sp._4xSample);
> + GEN_SAMPLE_POS_8X(sp._8xSample);
> +#if GEN_GEN >= 9
> + GEN_SAMPLE_POS_16X(sp._16xSample);
> +#endif
> + }
> + }
> +
> +#if GEN_GEN == 10
> + gen10_emit_wa_lri_to_cache_mode_zero(batch);
> +#endif
> +
> +#endif
> +}
> --
> 2.20.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190313/cb58ed86/attachment-0001.html>
More information about the mesa-dev
mailing list