Re: [Mesa-dev] [PATCH 2/2] anv: add support for VK_EXT_inline_uniform_block

2018-10-04 Thread Tapani Pälli 



On 9/17/18 1:43 AM, Lionel Landwerlin wrote:

On 16/09/2018 21:57, Bas Nieuwenhuizen wrote:

On Tue, Sep 11, 2018 at 10:23 PM Lionel Landwerlin
 wrote:

This new extension adds an implicitly allocated block of uniforms into
the descriptors sets through a new descriptor type.

We implement this by having a single BO in the descriptor set pool
from which we source uniforms.

Signed-off-by: Lionel Landwerlin 
---
  src/intel/vulkan/anv_cmd_buffer.c |   3 +
  src/intel/vulkan/anv_descriptor_set.c | 238 +-
  src/intel/vulkan/anv_device.c |  22 ++
  src/intel/vulkan/anv_extensions.py    |   1 +
  .../vulkan/anv_nir_apply_pipeline_layout.c    |  52 
  src/intel/vulkan/anv_private.h    |  33 +++
  src/intel/vulkan/genX_cmd_buffer.c    |  32 ++-
  7 files changed, 367 insertions(+), 14 deletions(-)

diff --git a/src/intel/vulkan/anv_cmd_buffer.c 
b/src/intel/vulkan/anv_cmd_buffer.c

index 8ef71b0ed9c..b14be94f470 100644
--- a/src/intel/vulkan/anv_cmd_buffer.c
+++ b/src/intel/vulkan/anv_cmd_buffer.c
@@ -651,6 +651,7 @@ 
anv_isl_format_for_descriptor_type(VkDescriptorType type)

 switch (type) {
 case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
 case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
+   case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
    return ISL_FORMAT_R32G32B32A32_FLOAT;

 case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER:
@@ -1039,6 +1040,8 @@ void anv_CmdPushDescriptorSetKHR(
   }
   break;

+  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
+ unreachable("Invalid descriptor type for push descriptors");
    default:
   break;
    }
diff --git a/src/intel/vulkan/anv_descriptor_set.c 
b/src/intel/vulkan/anv_descriptor_set.c

index 3439f828900..2e5f2a1f288 100644
--- a/src/intel/vulkan/anv_descriptor_set.c
+++ b/src/intel/vulkan/anv_descriptor_set.c
@@ -26,8 +26,10 @@
  #include 
  #include 
  #include 
+#include 

  #include "util/mesa-sha1.h"
+#include "vk_util.h"

  #include "anv_private.h"

@@ -40,7 +42,8 @@ void anv_GetDescriptorSetLayoutSupport(
  const VkDescriptorSetLayoutCreateInfo*  pCreateInfo,
  VkDescriptorSetLayoutSupport*   pSupport)
  {
-   uint32_t surface_count[MESA_SHADER_STAGES] = { 0, };
+   int16_t surface_count[MESA_SHADER_STAGES] = { 0, };
+   int16_t inline_surface_indexes[MESA_SHADER_STAGES] = { -1, };

 for (uint32_t b = 0; b < pCreateInfo->bindingCount; b++) {
    const VkDescriptorSetLayoutBinding *binding = 
>pBindings[b];

@@ -50,6 +53,15 @@ void anv_GetDescriptorSetLayoutSupport(
   /* There is no real limit on samplers */
   break;

+  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
+ anv_foreach_stage(s, binding->stageFlags) {
+    if (inline_surface_indexes[s] < 0) {
+   inline_surface_indexes[s] = surface_count[s];
+   surface_count[s] += 1;
+    }
+ }
+ break;
+
    case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
   if (binding->pImmutableSamplers) {
  for (uint32_t i = 0; i < binding->descriptorCount; i++) {
@@ -118,6 +130,9 @@ VkResult anv_CreateDescriptorSetLayout(
 memset(set_layout, 0, sizeof(*set_layout));
 set_layout->ref_cnt = 1;
 set_layout->binding_count = max_binding + 1;
+   set_layout->inline_blocks_descriptor_index = -1;
+   memset(set_layout->inline_blocks_surface_indexes,
+  -1, sizeof(set_layout->inline_blocks_surface_indexes));

 for (uint32_t b = 0; b <= max_binding; b++) {
    /* Initialize all binding_layout entries to -1 */
@@ -159,9 +174,24 @@ VkResult anv_CreateDescriptorSetLayout(
  #ifndef NDEBUG
    set_layout->binding[b].type = binding->descriptorType;
  #endif
-  set_layout->binding[b].array_size = binding->descriptorCount;
-  set_layout->binding[b].descriptor_index = set_layout->size;
-  set_layout->size += binding->descriptorCount;
+
+  if (binding->descriptorType == 
VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT) {
+ /* We only a single descriptor entry for all the inline 
uniforms. */

+ set_layout->binding[b].array_size = 1;
+ if (set_layout->inline_blocks_descriptor_index < 0) {
+    set_layout->binding[b].descriptor_index =
+   set_layout->inline_blocks_descriptor_index =
+   set_layout->size;
+    set_layout->size += 1;
+ } else {
+    set_layout->binding[b].descriptor_index =
+   set_layout->inline_blocks_descriptor_index;
+ }
+  } else {
+ set_layout->binding[b].array_size = binding->descriptorCount;
+ set_layout->binding[b].descriptor_index = set_layout->size;
+ set_layout->size += binding->descriptorCount;
+  }

    switch (binding->descriptorType) {
    case VK_DESCRIPTOR_TYPE_SAMPLER:
@@ -176,6 +206,24 @@ VkResult anv_CreateDescriptorSetLayout(
    }

    switch

Re: [Mesa-dev] [PATCH 2/2] anv: add support for VK_EXT_inline_uniform_block

2018-09-16 Thread Lionel Landwerlin 

On 16/09/2018 21:57, Bas Nieuwenhuizen wrote:

On Tue, Sep 11, 2018 at 10:23 PM Lionel Landwerlin
 wrote:

This new extension adds an implicitly allocated block of uniforms into
the descriptors sets through a new descriptor type.

We implement this by having a single BO in the descriptor set pool
from which we source uniforms.

Signed-off-by: Lionel Landwerlin 
---
  src/intel/vulkan/anv_cmd_buffer.c |   3 +
  src/intel/vulkan/anv_descriptor_set.c | 238 +-
  src/intel/vulkan/anv_device.c |  22 ++
  src/intel/vulkan/anv_extensions.py|   1 +
  .../vulkan/anv_nir_apply_pipeline_layout.c|  52 
  src/intel/vulkan/anv_private.h|  33 +++
  src/intel/vulkan/genX_cmd_buffer.c|  32 ++-
  7 files changed, 367 insertions(+), 14 deletions(-)

diff --git a/src/intel/vulkan/anv_cmd_buffer.c 
b/src/intel/vulkan/anv_cmd_buffer.c
index 8ef71b0ed9c..b14be94f470 100644
--- a/src/intel/vulkan/anv_cmd_buffer.c
+++ b/src/intel/vulkan/anv_cmd_buffer.c
@@ -651,6 +651,7 @@ anv_isl_format_for_descriptor_type(VkDescriptorType type)
 switch (type) {
 case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
 case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
+   case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
return ISL_FORMAT_R32G32B32A32_FLOAT;

 case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER:
@@ -1039,6 +1040,8 @@ void anv_CmdPushDescriptorSetKHR(
   }
   break;

+  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
+ unreachable("Invalid descriptor type for push descriptors");
default:
   break;
}
diff --git a/src/intel/vulkan/anv_descriptor_set.c 
b/src/intel/vulkan/anv_descriptor_set.c
index 3439f828900..2e5f2a1f288 100644
--- a/src/intel/vulkan/anv_descriptor_set.c
+++ b/src/intel/vulkan/anv_descriptor_set.c
@@ -26,8 +26,10 @@
  #include 
  #include 
  #include 
+#include 

  #include "util/mesa-sha1.h"
+#include "vk_util.h"

  #include "anv_private.h"

@@ -40,7 +42,8 @@ void anv_GetDescriptorSetLayoutSupport(
  const VkDescriptorSetLayoutCreateInfo*  pCreateInfo,
  VkDescriptorSetLayoutSupport*   pSupport)
  {
-   uint32_t surface_count[MESA_SHADER_STAGES] = { 0, };
+   int16_t surface_count[MESA_SHADER_STAGES] = { 0, };
+   int16_t inline_surface_indexes[MESA_SHADER_STAGES] = { -1, };

 for (uint32_t b = 0; b < pCreateInfo->bindingCount; b++) {
const VkDescriptorSetLayoutBinding *binding = 
>pBindings[b];
@@ -50,6 +53,15 @@ void anv_GetDescriptorSetLayoutSupport(
   /* There is no real limit on samplers */
   break;

+  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
+ anv_foreach_stage(s, binding->stageFlags) {
+if (inline_surface_indexes[s] < 0) {
+   inline_surface_indexes[s] = surface_count[s];
+   surface_count[s] += 1;
+}
+ }
+ break;
+
case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
   if (binding->pImmutableSamplers) {
  for (uint32_t i = 0; i < binding->descriptorCount; i++) {
@@ -118,6 +130,9 @@ VkResult anv_CreateDescriptorSetLayout(
 memset(set_layout, 0, sizeof(*set_layout));
 set_layout->ref_cnt = 1;
 set_layout->binding_count = max_binding + 1;
+   set_layout->inline_blocks_descriptor_index = -1;
+   memset(set_layout->inline_blocks_surface_indexes,
+  -1, sizeof(set_layout->inline_blocks_surface_indexes));

 for (uint32_t b = 0; b <= max_binding; b++) {
/* Initialize all binding_layout entries to -1 */
@@ -159,9 +174,24 @@ VkResult anv_CreateDescriptorSetLayout(
  #ifndef NDEBUG
set_layout->binding[b].type = binding->descriptorType;
  #endif
-  set_layout->binding[b].array_size = binding->descriptorCount;
-  set_layout->binding[b].descriptor_index = set_layout->size;
-  set_layout->size += binding->descriptorCount;
+
+  if (binding->descriptorType == 
VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT) {
+ /* We only a single descriptor entry for all the inline uniforms. */
+ set_layout->binding[b].array_size = 1;
+ if (set_layout->inline_blocks_descriptor_index < 0) {
+set_layout->binding[b].descriptor_index =
+   set_layout->inline_blocks_descriptor_index =
+   set_layout->size;
+set_layout->size += 1;
+ } else {
+set_layout->binding[b].descriptor_index =
+   set_layout->inline_blocks_descriptor_index;
+ }
+  } else {
+ set_layout->binding[b].array_size = binding->descriptorCount;
+ set_layout->binding[b].descriptor_index = set_layout->size;
+ set_layout->size += binding->descriptorCount;
+  }

switch (binding->descriptorType) {
case VK_DESCRIPTOR_TYPE_SAMPLER:
@@ -176,6 +206,24 @@ VkResult anv_CreateDescriptorSetLayout(
}

switch (binding->descriptorType) {
+  case

Re: [Mesa-dev] [PATCH 2/2] anv: add support for VK_EXT_inline_uniform_block

2018-09-16 Thread Bas Nieuwenhuizen 
On Tue, Sep 11, 2018 at 10:23 PM Lionel Landwerlin
 wrote:
>
> This new extension adds an implicitly allocated block of uniforms into
> the descriptors sets through a new descriptor type.
>
> We implement this by having a single BO in the descriptor set pool
> from which we source uniforms.
>
> Signed-off-by: Lionel Landwerlin 
> ---
>  src/intel/vulkan/anv_cmd_buffer.c |   3 +
>  src/intel/vulkan/anv_descriptor_set.c | 238 +-
>  src/intel/vulkan/anv_device.c |  22 ++
>  src/intel/vulkan/anv_extensions.py|   1 +
>  .../vulkan/anv_nir_apply_pipeline_layout.c|  52 
>  src/intel/vulkan/anv_private.h|  33 +++
>  src/intel/vulkan/genX_cmd_buffer.c|  32 ++-
>  7 files changed, 367 insertions(+), 14 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_cmd_buffer.c 
> b/src/intel/vulkan/anv_cmd_buffer.c
> index 8ef71b0ed9c..b14be94f470 100644
> --- a/src/intel/vulkan/anv_cmd_buffer.c
> +++ b/src/intel/vulkan/anv_cmd_buffer.c
> @@ -651,6 +651,7 @@ anv_isl_format_for_descriptor_type(VkDescriptorType type)
> switch (type) {
> case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
> case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
> +   case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
>return ISL_FORMAT_R32G32B32A32_FLOAT;
>
> case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER:
> @@ -1039,6 +1040,8 @@ void anv_CmdPushDescriptorSetKHR(
>   }
>   break;
>
> +  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
> + unreachable("Invalid descriptor type for push descriptors");
>default:
>   break;
>}
> diff --git a/src/intel/vulkan/anv_descriptor_set.c 
> b/src/intel/vulkan/anv_descriptor_set.c
> index 3439f828900..2e5f2a1f288 100644
> --- a/src/intel/vulkan/anv_descriptor_set.c
> +++ b/src/intel/vulkan/anv_descriptor_set.c
> @@ -26,8 +26,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include "util/mesa-sha1.h"
> +#include "vk_util.h"
>
>  #include "anv_private.h"
>
> @@ -40,7 +42,8 @@ void anv_GetDescriptorSetLayoutSupport(
>  const VkDescriptorSetLayoutCreateInfo*  pCreateInfo,
>  VkDescriptorSetLayoutSupport*   pSupport)
>  {
> -   uint32_t surface_count[MESA_SHADER_STAGES] = { 0, };
> +   int16_t surface_count[MESA_SHADER_STAGES] = { 0, };
> +   int16_t inline_surface_indexes[MESA_SHADER_STAGES] = { -1, };
>
> for (uint32_t b = 0; b < pCreateInfo->bindingCount; b++) {
>const VkDescriptorSetLayoutBinding *binding = 
> >pBindings[b];
> @@ -50,6 +53,15 @@ void anv_GetDescriptorSetLayoutSupport(
>   /* There is no real limit on samplers */
>   break;
>
> +  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
> + anv_foreach_stage(s, binding->stageFlags) {
> +if (inline_surface_indexes[s] < 0) {
> +   inline_surface_indexes[s] = surface_count[s];
> +   surface_count[s] += 1;
> +}
> + }
> + break;
> +
>case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
>   if (binding->pImmutableSamplers) {
>  for (uint32_t i = 0; i < binding->descriptorCount; i++) {
> @@ -118,6 +130,9 @@ VkResult anv_CreateDescriptorSetLayout(
> memset(set_layout, 0, sizeof(*set_layout));
> set_layout->ref_cnt = 1;
> set_layout->binding_count = max_binding + 1;
> +   set_layout->inline_blocks_descriptor_index = -1;
> +   memset(set_layout->inline_blocks_surface_indexes,
> +  -1, sizeof(set_layout->inline_blocks_surface_indexes));
>
> for (uint32_t b = 0; b <= max_binding; b++) {
>/* Initialize all binding_layout entries to -1 */
> @@ -159,9 +174,24 @@ VkResult anv_CreateDescriptorSetLayout(
>  #ifndef NDEBUG
>set_layout->binding[b].type = binding->descriptorType;
>  #endif
> -  set_layout->binding[b].array_size = binding->descriptorCount;
> -  set_layout->binding[b].descriptor_index = set_layout->size;
> -  set_layout->size += binding->descriptorCount;
> +
> +  if (binding->descriptorType == 
> VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT) {
> + /* We only a single descriptor entry for all the inline uniforms. */
> + set_layout->binding[b].array_size = 1;
> + if (set_layout->inline_blocks_descriptor_index < 0) {
> +set_layout->binding[b].descriptor_index =
> +   set_layout->inline_blocks_descriptor_index =
> +   set_layout->size;
> +set_layout->size += 1;
> + } else {
> +set_layout->binding[b].descriptor_index =
> +   set_layout->inline_blocks_descriptor_index;
> + }
> +  } else {
> + set_layout->binding[b].array_size = binding->descriptorCount;
> + set_layout->binding[b].descriptor_index = set_layout->size;
> + set_layout->size += binding->descriptorCount;
> +  }
>
>switch (binding->descriptorType) {
>case VK_DESCRIPTOR_TYPE_SAMPLER:
>

Re: [Mesa-dev] [PATCH 2/2] anv: add support for VK_EXT_inline_uniform_block

2018-09-16 Thread Lionel Landwerlin 

Hey Tapani,

Descriptors were kind of tricky to get my head around so thanks a lot 
for looking into this.


Regarding this max values, there isn't really a limit with our hardware. 
I just picked the minimum required by the spec.
I think the assert are somewhat unnecessary but I don't really object to 
them.


Thanks again for your time on this!

-
Lionel

On 14/09/2018 11:32, Tapani Pälli wrote:
I can't say I know enough of all these parts but I went through the 
API functions and tried to check that you have proper checks in place. 
Will try to still review :)


I did not see any check against MAX_INLINE_UNIFORM_BLOCK_DESCRIPTORS 
when creating pipeline layout. I'm not sure if such is necessary 
(since it's implicit rule), do you think there should there be 
check/assert?


one minor possible addition below ..

On 11.09.2018 23:22, Lionel Landwerlin wrote:

This new extension adds an implicitly allocated block of uniforms into
the descriptors sets through a new descriptor type. > We implement 
this by having a single BO in the descriptor set pool

from which we source uniforms.

Signed-off-by: Lionel Landwerlin 
---
  src/intel/vulkan/anv_cmd_buffer.c |   3 +
  src/intel/vulkan/anv_descriptor_set.c | 238 +-
  src/intel/vulkan/anv_device.c |  22 ++
  src/intel/vulkan/anv_extensions.py    |   1 +
  .../vulkan/anv_nir_apply_pipeline_layout.c    |  52 
  src/intel/vulkan/anv_private.h    |  33 +++
  src/intel/vulkan/genX_cmd_buffer.c    |  32 ++-
  7 files changed, 367 insertions(+), 14 deletions(-)

diff --git a/src/intel/vulkan/anv_cmd_buffer.c 
b/src/intel/vulkan/anv_cmd_buffer.c

index 8ef71b0ed9c..b14be94f470 100644
--- a/src/intel/vulkan/anv_cmd_buffer.c
+++ b/src/intel/vulkan/anv_cmd_buffer.c
@@ -651,6 +651,7 @@ 
anv_isl_format_for_descriptor_type(VkDescriptorType type)

 switch (type) {
 case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
 case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
+   case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
    return ISL_FORMAT_R32G32B32A32_FLOAT;
   case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER:
@@ -1039,6 +1040,8 @@ void anv_CmdPushDescriptorSetKHR(
   }
   break;
  +  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
+ unreachable("Invalid descriptor type for push descriptors");
    default:
   break;
    }
diff --git a/src/intel/vulkan/anv_descriptor_set.c 
b/src/intel/vulkan/anv_descriptor_set.c

index 3439f828900..2e5f2a1f288 100644
--- a/src/intel/vulkan/anv_descriptor_set.c
+++ b/src/intel/vulkan/anv_descriptor_set.c
@@ -26,8 +26,10 @@
  #include 
  #include 
  #include 
+#include 
    #include "util/mesa-sha1.h"
+#include "vk_util.h"
    #include "anv_private.h"
  @@ -40,7 +42,8 @@ void anv_GetDescriptorSetLayoutSupport(
  const VkDescriptorSetLayoutCreateInfo*  pCreateInfo,
  VkDescriptorSetLayoutSupport*   pSupport)
  {
-   uint32_t surface_count[MESA_SHADER_STAGES] = { 0, };
+   int16_t surface_count[MESA_SHADER_STAGES] = { 0, };
+   int16_t inline_surface_indexes[MESA_SHADER_STAGES] = { -1, };
   for (uint32_t b = 0; b < pCreateInfo->bindingCount; b++) {
    const VkDescriptorSetLayoutBinding *binding = 
>pBindings[b];

@@ -50,6 +53,15 @@ void anv_GetDescriptorSetLayoutSupport(
   /* There is no real limit on samplers */
   break;
  +  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
+ anv_foreach_stage(s, binding->stageFlags) {
+    if (inline_surface_indexes[s] < 0) {
+   inline_surface_indexes[s] = surface_count[s];
+   surface_count[s] += 1;
+    }
+ }
+ break;
+
    case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
   if (binding->pImmutableSamplers) {
  for (uint32_t i = 0; i < binding->descriptorCount; i++) {
@@ -118,6 +130,9 @@ VkResult anv_CreateDescriptorSetLayout(
 memset(set_layout, 0, sizeof(*set_layout));
 set_layout->ref_cnt = 1;
 set_layout->binding_count = max_binding + 1;
+   set_layout->inline_blocks_descriptor_index = -1;
+   memset(set_layout->inline_blocks_surface_indexes,
+  -1, sizeof(set_layout->inline_blocks_surface_indexes));
   for (uint32_t b = 0; b <= max_binding; b++) {
    /* Initialize all binding_layout entries to -1 */
@@ -159,9 +174,24 @@ VkResult anv_CreateDescriptorSetLayout(
  #ifndef NDEBUG
    set_layout->binding[b].type = binding->descriptorType;
  #endif
-  set_layout->binding[b].array_size = binding->descriptorCount;
-  set_layout->binding[b].descriptor_index = set_layout->size;
-  set_layout->size += binding->descriptorCount;
+
+  if (binding->descriptorType == 
VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT) {


Maybe add here

assert(binding->descriptorCount % 4 == 0 &&
   binding->descriptorCount <= MAX_INLINE_UNIFORM_BLOCK_SIZE);



Sure, added locally.




?

+ /* We only a single

Re: [Mesa-dev] [PATCH 2/2] anv: add support for VK_EXT_inline_uniform_block

2018-09-14 Thread Tapani Pälli 
I can't say I know enough of all these parts but I went through the API 
functions and tried to check that you have proper checks in place. Will 
try to still review :)


I did not see any check against MAX_INLINE_UNIFORM_BLOCK_DESCRIPTORS 
when creating pipeline layout. I'm not sure if such is necessary (since 
it's implicit rule), do you think there should there be check/assert?


one minor possible addition below ..

On 11.09.2018 23:22, Lionel Landwerlin wrote:

This new extension adds an implicitly allocated block of uniforms into
the descriptors sets through a new descriptor type. > We implement this by 
having a single BO in the descriptor set pool
from which we source uniforms.

Signed-off-by: Lionel Landwerlin 
---
  src/intel/vulkan/anv_cmd_buffer.c |   3 +
  src/intel/vulkan/anv_descriptor_set.c | 238 +-
  src/intel/vulkan/anv_device.c |  22 ++
  src/intel/vulkan/anv_extensions.py|   1 +
  .../vulkan/anv_nir_apply_pipeline_layout.c|  52 
  src/intel/vulkan/anv_private.h|  33 +++
  src/intel/vulkan/genX_cmd_buffer.c|  32 ++-
  7 files changed, 367 insertions(+), 14 deletions(-)

diff --git a/src/intel/vulkan/anv_cmd_buffer.c 
b/src/intel/vulkan/anv_cmd_buffer.c
index 8ef71b0ed9c..b14be94f470 100644
--- a/src/intel/vulkan/anv_cmd_buffer.c
+++ b/src/intel/vulkan/anv_cmd_buffer.c
@@ -651,6 +651,7 @@ anv_isl_format_for_descriptor_type(VkDescriptorType type)
 switch (type) {
 case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
 case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
+   case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
return ISL_FORMAT_R32G32B32A32_FLOAT;
  
 case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER:

@@ -1039,6 +1040,8 @@ void anv_CmdPushDescriptorSetKHR(
   }
   break;
  
+  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:

+ unreachable("Invalid descriptor type for push descriptors");
default:
   break;
}
diff --git a/src/intel/vulkan/anv_descriptor_set.c 
b/src/intel/vulkan/anv_descriptor_set.c
index 3439f828900..2e5f2a1f288 100644
--- a/src/intel/vulkan/anv_descriptor_set.c
+++ b/src/intel/vulkan/anv_descriptor_set.c
@@ -26,8 +26,10 @@
  #include 
  #include 
  #include 
+#include 
  
  #include "util/mesa-sha1.h"

+#include "vk_util.h"
  
  #include "anv_private.h"
  
@@ -40,7 +42,8 @@ void anv_GetDescriptorSetLayoutSupport(

  const VkDescriptorSetLayoutCreateInfo*  pCreateInfo,
  VkDescriptorSetLayoutSupport*   pSupport)
  {
-   uint32_t surface_count[MESA_SHADER_STAGES] = { 0, };
+   int16_t surface_count[MESA_SHADER_STAGES] = { 0, };
+   int16_t inline_surface_indexes[MESA_SHADER_STAGES] = { -1, };
  
 for (uint32_t b = 0; b < pCreateInfo->bindingCount; b++) {

const VkDescriptorSetLayoutBinding *binding = 
>pBindings[b];
@@ -50,6 +53,15 @@ void anv_GetDescriptorSetLayoutSupport(
   /* There is no real limit on samplers */
   break;
  
+  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:

+ anv_foreach_stage(s, binding->stageFlags) {
+if (inline_surface_indexes[s] < 0) {
+   inline_surface_indexes[s] = surface_count[s];
+   surface_count[s] += 1;
+}
+ }
+ break;
+
case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
   if (binding->pImmutableSamplers) {
  for (uint32_t i = 0; i < binding->descriptorCount; i++) {
@@ -118,6 +130,9 @@ VkResult anv_CreateDescriptorSetLayout(
 memset(set_layout, 0, sizeof(*set_layout));
 set_layout->ref_cnt = 1;
 set_layout->binding_count = max_binding + 1;
+   set_layout->inline_blocks_descriptor_index = -1;
+   memset(set_layout->inline_blocks_surface_indexes,
+  -1, sizeof(set_layout->inline_blocks_surface_indexes));
  
 for (uint32_t b = 0; b <= max_binding; b++) {

/* Initialize all binding_layout entries to -1 */
@@ -159,9 +174,24 @@ VkResult anv_CreateDescriptorSetLayout(
  #ifndef NDEBUG
set_layout->binding[b].type = binding->descriptorType;
  #endif
-  set_layout->binding[b].array_size = binding->descriptorCount;
-  set_layout->binding[b].descriptor_index = set_layout->size;
-  set_layout->size += binding->descriptorCount;
+
+  if (binding->descriptorType == 
VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT) {


Maybe add here

assert(binding->descriptorCount % 4 == 0 &&
   binding->descriptorCount <= MAX_INLINE_UNIFORM_BLOCK_SIZE);

?


+ /* We only a single descriptor entry for all the inline uniforms. */
+ set_layout->binding[b].array_size = 1;
+ if (set_layout->inline_blocks_descriptor_index < 0) {
+set_layout->binding[b].descriptor_index =
+   set_layout->inline_blocks_descriptor_index =
+   set_layout->size;
+set_layout->size += 1;
+ } else {
+set_layout->binding[b].descriptor_index =

[Mesa-dev] [PATCH 2/2] anv: add support for VK_EXT_inline_uniform_block

2018-09-11 Thread Lionel Landwerlin 
This new extension adds an implicitly allocated block of uniforms into
the descriptors sets through a new descriptor type.

We implement this by having a single BO in the descriptor set pool
from which we source uniforms.

Signed-off-by: Lionel Landwerlin 
---
 src/intel/vulkan/anv_cmd_buffer.c |   3 +
 src/intel/vulkan/anv_descriptor_set.c | 238 +-
 src/intel/vulkan/anv_device.c |  22 ++
 src/intel/vulkan/anv_extensions.py|   1 +
 .../vulkan/anv_nir_apply_pipeline_layout.c|  52 
 src/intel/vulkan/anv_private.h|  33 +++
 src/intel/vulkan/genX_cmd_buffer.c|  32 ++-
 7 files changed, 367 insertions(+), 14 deletions(-)

diff --git a/src/intel/vulkan/anv_cmd_buffer.c 
b/src/intel/vulkan/anv_cmd_buffer.c
index 8ef71b0ed9c..b14be94f470 100644
--- a/src/intel/vulkan/anv_cmd_buffer.c
+++ b/src/intel/vulkan/anv_cmd_buffer.c
@@ -651,6 +651,7 @@ anv_isl_format_for_descriptor_type(VkDescriptorType type)
switch (type) {
case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER:
case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
+   case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
   return ISL_FORMAT_R32G32B32A32_FLOAT;
 
case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER:
@@ -1039,6 +1040,8 @@ void anv_CmdPushDescriptorSetKHR(
  }
  break;
 
+  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
+ unreachable("Invalid descriptor type for push descriptors");
   default:
  break;
   }
diff --git a/src/intel/vulkan/anv_descriptor_set.c 
b/src/intel/vulkan/anv_descriptor_set.c
index 3439f828900..2e5f2a1f288 100644
--- a/src/intel/vulkan/anv_descriptor_set.c
+++ b/src/intel/vulkan/anv_descriptor_set.c
@@ -26,8 +26,10 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "util/mesa-sha1.h"
+#include "vk_util.h"
 
 #include "anv_private.h"
 
@@ -40,7 +42,8 @@ void anv_GetDescriptorSetLayoutSupport(
 const VkDescriptorSetLayoutCreateInfo*  pCreateInfo,
 VkDescriptorSetLayoutSupport*   pSupport)
 {
-   uint32_t surface_count[MESA_SHADER_STAGES] = { 0, };
+   int16_t surface_count[MESA_SHADER_STAGES] = { 0, };
+   int16_t inline_surface_indexes[MESA_SHADER_STAGES] = { -1, };
 
for (uint32_t b = 0; b < pCreateInfo->bindingCount; b++) {
   const VkDescriptorSetLayoutBinding *binding = >pBindings[b];
@@ -50,6 +53,15 @@ void anv_GetDescriptorSetLayoutSupport(
  /* There is no real limit on samplers */
  break;
 
+  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
+ anv_foreach_stage(s, binding->stageFlags) {
+if (inline_surface_indexes[s] < 0) {
+   inline_surface_indexes[s] = surface_count[s];
+   surface_count[s] += 1;
+}
+ }
+ break;
+
   case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
  if (binding->pImmutableSamplers) {
 for (uint32_t i = 0; i < binding->descriptorCount; i++) {
@@ -118,6 +130,9 @@ VkResult anv_CreateDescriptorSetLayout(
memset(set_layout, 0, sizeof(*set_layout));
set_layout->ref_cnt = 1;
set_layout->binding_count = max_binding + 1;
+   set_layout->inline_blocks_descriptor_index = -1;
+   memset(set_layout->inline_blocks_surface_indexes,
+  -1, sizeof(set_layout->inline_blocks_surface_indexes));
 
for (uint32_t b = 0; b <= max_binding; b++) {
   /* Initialize all binding_layout entries to -1 */
@@ -159,9 +174,24 @@ VkResult anv_CreateDescriptorSetLayout(
 #ifndef NDEBUG
   set_layout->binding[b].type = binding->descriptorType;
 #endif
-  set_layout->binding[b].array_size = binding->descriptorCount;
-  set_layout->binding[b].descriptor_index = set_layout->size;
-  set_layout->size += binding->descriptorCount;
+
+  if (binding->descriptorType == 
VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT) {
+ /* We only a single descriptor entry for all the inline uniforms. */
+ set_layout->binding[b].array_size = 1;
+ if (set_layout->inline_blocks_descriptor_index < 0) {
+set_layout->binding[b].descriptor_index =
+   set_layout->inline_blocks_descriptor_index =
+   set_layout->size;
+set_layout->size += 1;
+ } else {
+set_layout->binding[b].descriptor_index =
+   set_layout->inline_blocks_descriptor_index;
+ }
+  } else {
+ set_layout->binding[b].array_size = binding->descriptorCount;
+ set_layout->binding[b].descriptor_index = set_layout->size;
+ set_layout->size += binding->descriptorCount;
+  }
 
   switch (binding->descriptorType) {
   case VK_DESCRIPTOR_TYPE_SAMPLER:
@@ -176,6 +206,24 @@ VkResult anv_CreateDescriptorSetLayout(
   }
 
   switch (binding->descriptorType) {
+  case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK_EXT:
+ set_layout->binding[b].inline_block_offset = 
set_layout->inline_blocks_size;
+