Re: [Mesa-dev] [PATCH 1/2] glsl: Don't rehash the values when copying to new table

[Tapani Pälli]

On 12/30/2016 05:53 AM, Vladislav Egorov wrote:

I've looked into it recently (I'm working on series of many various
trivial optimizations)

and it's faster to just memcpy() it. Just throwing out superfluous
hashing still keeps slow

hash-table insertion around -- with resizing, rehashing, memory
allocation/deallocation, internal

hash-function through integer division, collisions and so on. It
produces a nice speed improvement

actually. It's possible to explore approaches without any copying at
LOOP/IF entering at all,

but I am not sure it will improve performance.


When profiling copy_propagation(_elements) pass you can use Martina's 
testcase from this bug:


https://bugs.freedesktop.org/show_bug.cgi?id=94477

We still have a very long compile time for the WebGL case mentioned in 
the bug so would be cool to have some optimizations there.





30.12.2016 02:49, Thomas Helland пишет:

Really, we should have some kind of function for copying the whole table,
but this will work for now.
---
  src/compiler/glsl/opt_copy_propagation.cpp | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/opt_copy_propagation.cpp
b/src/compiler/glsl/opt_copy_propagation.cpp
index 247c498..e9f82e0 100644
--- a/src/compiler/glsl/opt_copy_propagation.cpp
+++ b/src/compiler/glsl/opt_copy_propagation.cpp
@@ -210,7 +210,8 @@
ir_copy_propagation_visitor::handle_if_block(exec_list *instructions)
 /* Populate the initial acp with a copy of the original */
 struct hash_entry *entry;
 hash_table_foreach(orig_acp, entry) {
-  _mesa_hash_table_insert(acp, entry->key, entry->data);
+  _mesa_hash_table_insert_pre_hashed(acp, entry->hash,
+ entry->key, entry->data);
 }
   visit_list_elements(this, instructions);
@@ -259,7 +260,8 @@ ir_copy_propagation_visitor::handle_loop(ir_loop
*ir, bool keep_acp)
 if (keep_acp) {
struct hash_entry *entry;
hash_table_foreach(orig_acp, entry) {
- _mesa_hash_table_insert(acp, entry->key, entry->data);
+ _mesa_hash_table_insert_pre_hashed(acp, entry->hash,
+entry->key, entry->data);
}
 }



___
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

Re: [Mesa-dev] [PATCH] anv, radv: disable StorageImageWriteWithoutFormat for now

[Dave Airlie]

On 30 December 2016 at 15:52, Ilia Mirkin  wrote:
> The SPIR-V capability isn't even marked as enabled, and there are no
> tests in Vulkan-CTS. Per Jason Ekstrand, this won't work in anv as such
> write-only surfaces require additional setup which is currently not
> performed.
>
> Signed-off-by: Ilia Mirkin 

Acked-by: Dave Airlie 

just remind me when tests show up :-)

> ---
>
> I don't care whether this patch or the one to enable the SPIR-V cap goes in.
> However the current state is inconsistent. We can even make use of the new
> SPIR-V extensions capability if radv wants it on while anv wants it off.
>
>  src/amd/vulkan/radv_device.c  | 2 +-
>  src/intel/vulkan/anv_device.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index dcbb015..e57a419 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -403,7 +403,7 @@ void radv_GetPhysicalDeviceFeatures(
> .shaderStorageBufferArrayDynamicIndexing  = true,
> .shaderStorageImageArrayDynamicIndexing   = true,
> .shaderStorageImageReadWithoutFormat  = false,
> -   .shaderStorageImageWriteWithoutFormat = true,
> +   .shaderStorageImageWriteWithoutFormat = false,
> .shaderClipDistance   = true,
> .shaderCullDistance   = true,
> .shaderFloat64= false,
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index ec1e0e8..69e1dc2 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -472,7 +472,7 @@ void anv_GetPhysicalDeviceFeatures(
>.shaderStorageImageExtendedFormats= true,
>.shaderStorageImageMultisample= false,
>.shaderStorageImageReadWithoutFormat  = false,
> -  .shaderStorageImageWriteWithoutFormat = true,
> +  .shaderStorageImageWriteWithoutFormat = false,
>.shaderUniformBufferArrayDynamicIndexing  = true,
>.shaderSampledImageArrayDynamicIndexing   = true,
>.shaderStorageBufferArrayDynamicIndexing  = true,
> --
> 2.10.2
>
> ___
> 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

[Mesa-dev] [PATCH] anv, radv: disable StorageImageWriteWithoutFormat for now

[Ilia Mirkin]

The SPIR-V capability isn't even marked as enabled, and there are no
tests in Vulkan-CTS. Per Jason Ekstrand, this won't work in anv as such
write-only surfaces require additional setup which is currently not
performed.

Signed-off-by: Ilia Mirkin 
---

I don't care whether this patch or the one to enable the SPIR-V cap goes in.
However the current state is inconsistent. We can even make use of the new
SPIR-V extensions capability if radv wants it on while anv wants it off.

 src/amd/vulkan/radv_device.c  | 2 +-
 src/intel/vulkan/anv_device.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index dcbb015..e57a419 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -403,7 +403,7 @@ void radv_GetPhysicalDeviceFeatures(
.shaderStorageBufferArrayDynamicIndexing  = true,
.shaderStorageImageArrayDynamicIndexing   = true,
.shaderStorageImageReadWithoutFormat  = false,
-   .shaderStorageImageWriteWithoutFormat = true,
+   .shaderStorageImageWriteWithoutFormat = false,
.shaderClipDistance   = true,
.shaderCullDistance   = true,
.shaderFloat64= false,
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index ec1e0e8..69e1dc2 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -472,7 +472,7 @@ void anv_GetPhysicalDeviceFeatures(
   .shaderStorageImageExtendedFormats= true,
   .shaderStorageImageMultisample= false,
   .shaderStorageImageReadWithoutFormat  = false,
-  .shaderStorageImageWriteWithoutFormat = true,
+  .shaderStorageImageWriteWithoutFormat = false,
   .shaderUniformBufferArrayDynamicIndexing  = true,
   .shaderSampledImageArrayDynamicIndexing   = true,
   .shaderStorageBufferArrayDynamicIndexing  = true,
-- 
2.10.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] spirv: mark SpvCapabilityStorageImageWriteWithoutFormat supported

[Jason Ekstrand]

Ugh...  The problem is that we have to set the surface up differently for
write-only surfaces.  We should shut it off for now.

On Dec 29, 2016 11:01 PM, "Ilia Mirkin"  wrote:

> Well, maybe not, but
>
> src/amd/vulkan/radv_device.c:
> .shaderStorageImageWriteWithoutFormat = true,
> src/intel/vulkan/anv_device.c:
> .shaderStorageImageWriteWithoutFormat = true,
>
> and from a brief look at the code, it seems like it should work -
> image_format gets set to 0 (GL_NONE), which is exactly what happens in
> the GLSL path. I noticed this when adding ImageReadWithoutFormat for
> gen9+, but sadly there are no tests for either of those.
>
>   -ilia
>
> On Thu, Dec 29, 2016 at 11:57 PM, Jason Ekstrand 
> wrote:
> > I don't think we actually do...
> >
> > On Dec 29, 2016 9:34 PM, "Ilia Mirkin"  wrote:
> >>
> >> Both anv and radv support this.
> >>
> >> Signed-off-by: Ilia Mirkin 
> >> ---
> >>  src/compiler/spirv/spirv_to_nir.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/src/compiler/spirv/spirv_to_nir.c
> >> b/src/compiler/spirv/spirv_to_nir.c
> >> index 07980aa..22e14f6 100644
> >> --- a/src/compiler/spirv/spirv_to_nir.c
> >> +++ b/src/compiler/spirv/spirv_to_nir.c
> >> @@ -2507,6 +2507,7 @@ vtn_handle_preamble_instruction(struct
> vtn_builder
> >> *b, SpvOp opcode,
> >>case SpvCapabilityInputAttachment:
> >>case SpvCapabilityImageGatherExtended:
> >>case SpvCapabilityStorageImageExtendedFormats:
> >> +  case SpvCapabilityStorageImageWriteWithoutFormat:
> >>   break;
> >>
> >>case SpvCapabilityGeometryStreams:
> >> @@ -2528,7 +2529,6 @@ vtn_handle_preamble_instruction(struct
> vtn_builder
> >> *b, SpvOp opcode,
> >>case SpvCapabilityMinLod:
> >>case SpvCapabilityTransformFeedback:
> >>case SpvCapabilityStorageImageReadWithoutFormat:
> >> -  case SpvCapabilityStorageImageWriteWithoutFormat:
> >>   vtn_warn("Unsupported SPIR-V capability: %s",
> >>spirv_capability_to_string(cap));
> >>   break;
> >> --
> >> 2.10.2
> >>
> >> ___
> >> 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

[Mesa-dev] [PATCH] radv/ac: add support for multi sample image coords

[Dave Airlie]

From: Dave Airlie 

This just adds the nir->llvm support, enabling
the extension causes some failures on llvm 3.9 at least,
but this code seems fine.

NIR passes the sampler in src[1].x, and we LLVM/SI requires
it as the last parameters in the coords (coord[2] for 2D,
coord[3] for 2DArray).

Signed-off-by: Dave Airlie 
---
 src/amd/common/ac_nir_to_llvm.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index f214fcd..bc3a345 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -2386,12 +2386,16 @@ static int image_type_to_components_count(enum 
glsl_sampler_dim dim, bool array)
return array ? 2 : 1;
case GLSL_SAMPLER_DIM_2D:
return array ? 3 : 2;
+   case GLSL_SAMPLER_DIM_MS:
+   return array ? 4 : 3;
case GLSL_SAMPLER_DIM_3D:
case GLSL_SAMPLER_DIM_CUBE:
return 3;
case GLSL_SAMPLER_DIM_RECT:
case GLSL_SAMPLER_DIM_SUBPASS:
return 2;
+   case GLSL_SAMPLER_DIM_SUBPASS_MS:
+   return 3;
default:
break;
}
@@ -2413,7 +2417,11 @@ static LLVMValueRef get_image_coords(struct 
nir_to_llvm_context *ctx,
};
LLVMValueRef res;
int count;
-   count = image_type_to_components_count(glsl_get_sampler_dim(type),
+   enum glsl_sampler_dim dim = glsl_get_sampler_dim(type);
+   bool is_ms = (dim == GLSL_SAMPLER_DIM_MS ||
+ dim == GLSL_SAMPLER_DIM_SUBPASS_MS);
+
+   count = image_type_to_components_count(dim,
   
glsl_sampler_type_is_array(type));
 
if (count == 1) {
@@ -2423,6 +2431,8 @@ static LLVMValueRef get_image_coords(struct 
nir_to_llvm_context *ctx,
res = src0;
} else {
int chan;
+   if (is_ms)
+   count--;
for (chan = 0; chan < count; ++chan) {
coords[chan] = LLVMBuildExtractElement(ctx->builder, 
src0, masks[chan], "");
}
@@ -2431,6 +2441,11 @@ static LLVMValueRef get_image_coords(struct 
nir_to_llvm_context *ctx,
for (chan = 0; chan < count; ++chan)
coords[chan] = LLVMBuildAdd(ctx->builder, 
coords[chan], LLVMBuildFPToUI(ctx->builder, ctx->frag_pos[chan], ctx->i32, ""), 
"");
}
+   if (is_ms) {
+   coords[count] = llvm_extract_elem(ctx, get_src(ctx, 
instr->src[1]), 0);
+   count++;
+   }
+
if (count == 3) {
coords[3] = LLVMGetUndef(ctx->i32);
count = 4;
-- 
2.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] spirv: mark SpvCapabilityStorageImageWriteWithoutFormat supported

[Ilia Mirkin]

Well, maybe not, but

src/amd/vulkan/radv_device.c:
.shaderStorageImageWriteWithoutFormat = true,
src/intel/vulkan/anv_device.c:
.shaderStorageImageWriteWithoutFormat = true,

and from a brief look at the code, it seems like it should work -
image_format gets set to 0 (GL_NONE), which is exactly what happens in
the GLSL path. I noticed this when adding ImageReadWithoutFormat for
gen9+, but sadly there are no tests for either of those.

  -ilia

On Thu, Dec 29, 2016 at 11:57 PM, Jason Ekstrand  wrote:
> I don't think we actually do...
>
> On Dec 29, 2016 9:34 PM, "Ilia Mirkin"  wrote:
>>
>> Both anv and radv support this.
>>
>> Signed-off-by: Ilia Mirkin 
>> ---
>>  src/compiler/spirv/spirv_to_nir.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/compiler/spirv/spirv_to_nir.c
>> b/src/compiler/spirv/spirv_to_nir.c
>> index 07980aa..22e14f6 100644
>> --- a/src/compiler/spirv/spirv_to_nir.c
>> +++ b/src/compiler/spirv/spirv_to_nir.c
>> @@ -2507,6 +2507,7 @@ vtn_handle_preamble_instruction(struct vtn_builder
>> *b, SpvOp opcode,
>>case SpvCapabilityInputAttachment:
>>case SpvCapabilityImageGatherExtended:
>>case SpvCapabilityStorageImageExtendedFormats:
>> +  case SpvCapabilityStorageImageWriteWithoutFormat:
>>   break;
>>
>>case SpvCapabilityGeometryStreams:
>> @@ -2528,7 +2529,6 @@ vtn_handle_preamble_instruction(struct vtn_builder
>> *b, SpvOp opcode,
>>case SpvCapabilityMinLod:
>>case SpvCapabilityTransformFeedback:
>>case SpvCapabilityStorageImageReadWithoutFormat:
>> -  case SpvCapabilityStorageImageWriteWithoutFormat:
>>   vtn_warn("Unsupported SPIR-V capability: %s",
>>spirv_capability_to_string(cap));
>>   break;
>> --
>> 2.10.2
>>
>> ___
>> 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

Re: [Mesa-dev] [PATCH] spirv: mark SpvCapabilityStorageImageWriteWithoutFormat supported

[Jason Ekstrand]

I don't think we actually do...

On Dec 29, 2016 9:34 PM, "Ilia Mirkin"  wrote:

> Both anv and radv support this.
>
> Signed-off-by: Ilia Mirkin 
> ---
>  src/compiler/spirv/spirv_to_nir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/compiler/spirv/spirv_to_nir.c
> b/src/compiler/spirv/spirv_to_nir.c
> index 07980aa..22e14f6 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -2507,6 +2507,7 @@ vtn_handle_preamble_instruction(struct vtn_builder
> *b, SpvOp opcode,
>case SpvCapabilityInputAttachment:
>case SpvCapabilityImageGatherExtended:
>case SpvCapabilityStorageImageExtendedFormats:
> +  case SpvCapabilityStorageImageWriteWithoutFormat:
>   break;
>
>case SpvCapabilityGeometryStreams:
> @@ -2528,7 +2529,6 @@ vtn_handle_preamble_instruction(struct vtn_builder
> *b, SpvOp opcode,
>case SpvCapabilityMinLod:
>case SpvCapabilityTransformFeedback:
>case SpvCapabilityStorageImageReadWithoutFormat:
> -  case SpvCapabilityStorageImageWriteWithoutFormat:
>   vtn_warn("Unsupported SPIR-V capability: %s",
>spirv_capability_to_string(cap));
>   break;
> --
> 2.10.2
>
> ___
> 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

Re: [Mesa-dev] [PATCH 1/2] glsl: Don't rehash the values when copying to new table

[Vladislav Egorov]

I've looked into it recently (I'm working on series of many various 
trivial optimizations)


and it's faster to just memcpy() it. Just throwing out superfluous 
hashing still keeps slow


hash-table insertion around -- with resizing, rehashing, memory 
allocation/deallocation, internal


hash-function through integer division, collisions and so on. It 
produces a nice speed improvement


actually. It's possible to explore approaches without any copying at 
LOOP/IF entering at all,


but I am not sure it will improve performance.


30.12.2016 02:49, Thomas Helland пишет:

Really, we should have some kind of function for copying the whole table,
but this will work for now.
---
  src/compiler/glsl/opt_copy_propagation.cpp | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/opt_copy_propagation.cpp 
b/src/compiler/glsl/opt_copy_propagation.cpp
index 247c498..e9f82e0 100644
--- a/src/compiler/glsl/opt_copy_propagation.cpp
+++ b/src/compiler/glsl/opt_copy_propagation.cpp
@@ -210,7 +210,8 @@ ir_copy_propagation_visitor::handle_if_block(exec_list 
*instructions)
 /* Populate the initial acp with a copy of the original */
 struct hash_entry *entry;
 hash_table_foreach(orig_acp, entry) {
-  _mesa_hash_table_insert(acp, entry->key, entry->data);
+  _mesa_hash_table_insert_pre_hashed(acp, entry->hash,
+ entry->key, entry->data);
 }
  
 visit_list_elements(this, instructions);

@@ -259,7 +260,8 @@ ir_copy_propagation_visitor::handle_loop(ir_loop *ir, bool 
keep_acp)
 if (keep_acp) {
struct hash_entry *entry;
hash_table_foreach(orig_acp, entry) {
- _mesa_hash_table_insert(acp, entry->key, entry->data);
+ _mesa_hash_table_insert_pre_hashed(acp, entry->hash,
+entry->key, entry->data);
}
 }
  


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] spirv: mark SpvCapabilityStorageImageWriteWithoutFormat supported

[Ilia Mirkin]

Both anv and radv support this.

Signed-off-by: Ilia Mirkin 
---
 src/compiler/spirv/spirv_to_nir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/spirv/spirv_to_nir.c 
b/src/compiler/spirv/spirv_to_nir.c
index 07980aa..22e14f6 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -2507,6 +2507,7 @@ vtn_handle_preamble_instruction(struct vtn_builder *b, 
SpvOp opcode,
   case SpvCapabilityInputAttachment:
   case SpvCapabilityImageGatherExtended:
   case SpvCapabilityStorageImageExtendedFormats:
+  case SpvCapabilityStorageImageWriteWithoutFormat:
  break;
 
   case SpvCapabilityGeometryStreams:
@@ -2528,7 +2529,6 @@ vtn_handle_preamble_instruction(struct vtn_builder *b, 
SpvOp opcode,
   case SpvCapabilityMinLod:
   case SpvCapabilityTransformFeedback:
   case SpvCapabilityStorageImageReadWithoutFormat:
-  case SpvCapabilityStorageImageWriteWithoutFormat:
  vtn_warn("Unsupported SPIR-V capability: %s",
   spirv_capability_to_string(cap));
  break;
-- 
2.10.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [AppVeyor] mesa master #3005 completed

[AppVeyor]

Build mesa 3005 completed



Commit 36c648b894 by Ilia Mirkin on 12/30/2016 1:59 AM:

spirv: always expose SpvCapabilityStorageImageExtendedFormats\n\nI forgot to do this in commit 76b97d544e ("anv: enable storage image\nextended formats"). Since both drivers support this now, no need for the\nconditional enable.\n\nSigned-off-by: Ilia Mirkin \nReviewed-by: Jason Ekstrand \nReviewed-by: Dave Airlie 


Configure your notification preferences

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] spirv: always expose SpvCapabilityStorageImageExtendedFormats

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand 

On Thu, Dec 29, 2016 at 6:00 PM, Ilia Mirkin  wrote:

> I forgot to do this in commit 76b97d544e ("anv: enable storage image
> extended formats"). Since both drivers support this now, no need for the
> conditional enable.
>
> Signed-off-by: Ilia Mirkin 
> ---
>  src/amd/vulkan/radv_pipeline.c| 1 -
>  src/compiler/spirv/nir_spirv.h| 1 -
>  src/compiler/spirv/spirv_to_nir.c | 4 +---
>  3 files changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_
> pipeline.c
> index 25d7805..75785ec 100644
> --- a/src/amd/vulkan/radv_pipeline.c
> +++ b/src/amd/vulkan/radv_pipeline.c
> @@ -192,7 +192,6 @@ radv_shader_compile_to_nir(struct radv_device *device,
> }
> }
> const struct nir_spirv_supported_extensions supported_ext
> = {
> -   .storage_image_extended_formats = true,
> };
> entry_point = spirv_to_nir(spirv, module->size / 4,
>spec_entries, num_spec_entries,
> diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_
> spirv.h
> index d959f3f..e0112ef 100644
> --- a/src/compiler/spirv/nir_spirv.h
> +++ b/src/compiler/spirv/nir_spirv.h
> @@ -42,7 +42,6 @@ struct nir_spirv_specialization {
>  };
>
>  struct nir_spirv_supported_extensions {
> -   bool storage_image_extended_formats;
> bool image_ms_array;
>  };
>
> diff --git a/src/compiler/spirv/spirv_to_nir.c
> b/src/compiler/spirv/spirv_to_nir.c
> index b8acc1e..07980aa 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -2506,6 +2506,7 @@ vtn_handle_preamble_instruction(struct vtn_builder
> *b, SpvOp opcode,
>case SpvCapabilityCullDistance:
>case SpvCapabilityInputAttachment:
>case SpvCapabilityImageGatherExtended:
> +  case SpvCapabilityStorageImageExtendedFormats:
>   break;
>
>case SpvCapabilityGeometryStreams:
> @@ -2546,9 +2547,6 @@ vtn_handle_preamble_instruction(struct vtn_builder
> *b, SpvOp opcode,
>spirv_capability_to_string(cap));
>   break;
>
> -  case SpvCapabilityStorageImageExtendedFormats:
> - spv_check_supported(storage_image_extended_formats, cap);
> - break;
>case SpvCapabilityImageMSArray:
>   spv_check_supported(image_ms_array, cap);
>   break;
> --
> 2.10.2
>
> ___
> 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

[Mesa-dev] [PATCH] spirv: always expose SpvCapabilityStorageImageExtendedFormats

[Ilia Mirkin]

I forgot to do this in commit 76b97d544e ("anv: enable storage image
extended formats"). Since both drivers support this now, no need for the
conditional enable.

Signed-off-by: Ilia Mirkin 
---
 src/amd/vulkan/radv_pipeline.c| 1 -
 src/compiler/spirv/nir_spirv.h| 1 -
 src/compiler/spirv/spirv_to_nir.c | 4 +---
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index 25d7805..75785ec 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -192,7 +192,6 @@ radv_shader_compile_to_nir(struct radv_device *device,
}
}
const struct nir_spirv_supported_extensions supported_ext = {
-   .storage_image_extended_formats = true,
};
entry_point = spirv_to_nir(spirv, module->size / 4,
   spec_entries, num_spec_entries,
diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_spirv.h
index d959f3f..e0112ef 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -42,7 +42,6 @@ struct nir_spirv_specialization {
 };
 
 struct nir_spirv_supported_extensions {
-   bool storage_image_extended_formats;
bool image_ms_array;
 };
 
diff --git a/src/compiler/spirv/spirv_to_nir.c 
b/src/compiler/spirv/spirv_to_nir.c
index b8acc1e..07980aa 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -2506,6 +2506,7 @@ vtn_handle_preamble_instruction(struct vtn_builder *b, 
SpvOp opcode,
   case SpvCapabilityCullDistance:
   case SpvCapabilityInputAttachment:
   case SpvCapabilityImageGatherExtended:
+  case SpvCapabilityStorageImageExtendedFormats:
  break;
 
   case SpvCapabilityGeometryStreams:
@@ -2546,9 +2547,6 @@ vtn_handle_preamble_instruction(struct vtn_builder *b, 
SpvOp opcode,
   spirv_capability_to_string(cap));
  break;
 
-  case SpvCapabilityStorageImageExtendedFormats:
- spv_check_supported(storage_image_extended_formats, cap);
- break;
   case SpvCapabilityImageMSArray:
  spv_check_supported(image_ms_array, cap);
  break;
-- 
2.10.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [AppVeyor] mesa master #3004 failed

[AppVeyor]

Build mesa 3004 failed


Commit c633f228b4 by Ilia Mirkin on 11/27/2016 8:45 PM:

anv: add support for extended texture gather\n\nNow that the SPIR-V -> NIR translation is in place, no additional logic\nis required.\n\nSigned-off-by: Ilia Mirkin \nReviewed-by: Dave Airlie \nAcked-by: Jason Ekstrand 


Configure your notification preferences

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 16/27] i965/miptree: Allocate mcs_buf for an image's CCS_E

[Ben Widawsky]

On 16-12-10 15:26:02, Pohjolainen, Topi wrote:

On Thu, Dec 01, 2016 at 02:09:57PM -0800, Ben Widawsky wrote:

From: Ben Widawsky 

This code will disable actually creating these buffers for the scanout,
but it puts the allocation in place.

Primarily this patch is split out for review, it can be squashed in
later if preferred.

Signed-off-by: Ben Widawsky 
---
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 89 +++
 1 file changed, 77 insertions(+), 12 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index cfa2dc0..d002546 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -58,6 +58,11 @@ intel_miptree_alloc_mcs(struct brw_context *brw,
 struct intel_mipmap_tree *mt,
 GLuint num_samples);

+static void
+intel_miptree_init_mcs(struct brw_context *brw,
+   struct intel_mipmap_tree *mt,
+   int init_value);
+
 /**
  * Determine which MSAA layout should be used by the MSAA surface being
  * created, based on the chip generation and the surface type.
@@ -152,6 +157,9 @@ intel_miptree_supports_non_msrt_fast_clear(struct 
brw_context *brw,
if (mt->disable_aux_buffers)
   return false;

+   if (mt->is_scanout)
+  return false;
+
/* This function applies only to non-multisampled render targets. */
if (mt->num_samples > 1)
   return false;
@@ -744,6 +752,7 @@ intel_miptree_create(struct brw_context *brw,
* resolves.
*/
   const bool lossless_compression_disabled = INTEL_DEBUG & DEBUG_NO_RBC;
+  assert(!mt->is_scanout);
   const bool is_lossless_compressed =
  unlikely(!lossless_compression_disabled) &&
  brw->gen >= 9 && !mt->is_scanout &&
@@ -810,6 +819,36 @@ intel_miptree_create_for_bo(struct brw_context *brw,
return mt;
 }

+static bool
+create_ccs_buf_for_image(struct brw_context *intel,
+ __DRIimage *image,
+ struct intel_mipmap_tree *mt)
+{
+
+   struct isl_surf temp_main_surf;
+   struct isl_surf temp_ccs_surf;
+   uint32_t offset = mt->offset + image->aux_offset;


I need to ask about this as I'm not sure myself. We usually use offset to
move to a slice other than the first. Here the offset to the start of the
auxiliary is taken as relative to that. This seems awkward to me. Moreover,
if we move to another slice, don't we need to move equally within the auxiliary
also.



There are restrictions in the hardware that the x,y offset of the main buffer
need to match that of the aux buffer - that is what this is trying to achieve.
The scanout shouldn't ever have more than 1 slice (AFAIK), and even if it does,
we'd do the allocations for all slices here. What do you mean by, "if we move to
another slice"?


Further down you assert for mipmap levels "assert(mt->last_level < 2)" just
before calling this. So shoudln't we actually assert here that:

 assert(mt->offset == 0);



I think today that is fine, but I don't believe there is any reason that the
offset couldn't be non-zero. I am not the expert though.


+
+   intel_miptree_get_isl_surf(intel, mt, _main_surf);
+   if (!isl_surf_get_ccs_surf(>isl_dev, _main_surf, 
_ccs_surf))
+  return false;
+
+   mt->mcs_buf = calloc(1, sizeof(*mt->mcs_buf));
+   mt->mcs_buf->bo = image->bo;
+   drm_intel_bo_reference(image->bo);
+
+   mt->mcs_buf->offset = offset;
+   mt->mcs_buf->size = temp_ccs_surf.size;
+   mt->mcs_buf->pitch = temp_ccs_surf.row_pitch;
+   mt->mcs_buf->qpitch = isl_surf_get_array_pitch_sa_rows(_ccs_surf);
+
+   intel_miptree_init_mcs(intel, mt, 0);
+   mt->no_ccs = false;
+   mt->msaa_layout = INTEL_MSAA_LAYOUT_CMS;
+
+   return true;
+}
+
 struct intel_mipmap_tree *
 intel_miptree_create_for_image(struct brw_context *intel,
__DRIimage *image,
@@ -820,17 +859,43 @@ intel_miptree_create_for_image(struct brw_context *intel,
uint32_t pitch,
uint32_t layout_flags)
 {
-   layout_flags = (layout_flags & MIPTREE_LAYOUT_FOR_SCANOUT) ?
-  MIPTREE_LAYOUT_FOR_SCANOUT : MIPTREE_LAYOUT_DISABLE_AUX;
-   return intel_miptree_create_for_bo(intel,
-  image->bo,
-  format,
-  offset,
-  width,
-  height,
-  1,
-  pitch,
-  layout_flags);
+   struct intel_mipmap_tree *mt;
+
+   /* Other flags will be ignored, so make sure the caller didn't pass any. */
+   assert((layout_flags & ~MIPTREE_LAYOUT_FOR_SCANOUT) == 0);
+
+   if (!image->aux_offset)
+  layout_flags |= 

Re: [Mesa-dev] [PATCH 00/27] Renderbuffer Decompression (and GBM modifiers)

[Ben Widawsky]

On 16-12-06 13:34:02, Paulo Zanoni wrote:

2016-12-01 20:09 GMT-02:00 Ben Widawsky :

From: Ben Widawsky 

This patch series ultimately adds support within the i965 driver for
Renderbuffer Decompression with GBM. In short, this feature reduces memory
bandwidth by allowing the GPU to work with losslessly compressed data and having
that compression scheme understood by the display engine for decompression. The
display engine will decompress on the fly and scanout the image.

Quoting from the final patch, the bandwidth savings on a SKL GT4 with a 19x10
display running kmscube:

Without compression:
Read bandwidth: 603.91 MiB/s
Write bandwidth: 615.28 MiB/s

With compression:
Read bandwidth: 259.34 MiB/s
Write bandwidth: 337.83 MiB/s


The hardware achieves this savings by maintaining an auxiliary buffer
containing "opaque" compression information. It's opaque in the sense that the
low level compression scheme is not needed, but, knowledge of the overall
layout of the compressed data is required. The auxiliary buffer is created by
the driver on behalf of the client when requested. That buffer needs to be
passed along wherever the main image's buffer goes.

The overall strategy is that the buffer/surface is created with a list of
modifiers. The list of modifiers the hardware is capable of using will come from
a new kernel API that is aware of the hardware and general constraints. A client
will request the list of modifiers and pass it directly back in during buffer
creation (potentially the client can prune the list, but as of now there is no
reason to.) This new API is being developed by Kristian. I did not get far
enough to play with that.

For EGL, a similar mechanism would exist whereby when importing a buffer into
EGL, one would provide a modifier and probably a pointer to the auxiliary data
upon import. (Import therefore might require multiple dma-buf fds), but for i965
and Intel, this wouldn't be necessary.

Here is a brief description of the series:
1-6 Adds support in GBM for per plane functions where necessary. This is
required because the kernel expects the auxiliary buffer to be passed along as a
plane. It has its own offset, and stride, and the client shouldn't need to
calculate those.

7-9 Adds support in GBM to understand modifiers. When creating a buffer or
surface, the client is expected to pass in a list of modifiers that the driver
will optimally choose from. As a result of this, the GBM APIs need to support
modifiers.

10-12 Support Y-tiled modifier. Y-tiling was already a modifier exposed by the
kernel. With the previous patches in place, it's easy to support this too.

13-26 Plumbing to support sending CCS buffers to display. Leveraging much of the
existing code for MCS buffers, these patches creating an MCS for the scanout
buffer. The trickery here is that a single BO contains both the main surface and
the auxiliary data. Previously, auxiliary data always lived in its own BO.

27 Support CCS-modifier. Finally, the code can parse the CCS fb modifier(s) and
realize the bandwidth savings that come with it.

This was tested using kmscube
(https://github.com/bwidawsk/kmscube/tree/modifiers). The kmscube implementation
is missing support for GET_PLANE2 - which is currently being worked on by
Kristian.

Upstream plan:


First of all, I'd like to point that I haven't really been following
this feature closely, so maybe my questions are irrelevant to this
series. But still, I feel I have to poitn these things since maybe
they are relevant. Please tell me if I'm not talking about the same
thing as you are.

The main question is: where's the matching i915.ko series? Shouldn't
that be step 0 in your upstream plan?



Ville is working on it. All patches except the last can be merged without kernel
support. That is assuming that we agree upon the general solution, using the
modifiers and having both buffers be part of the same BO. There is also a
requisite series from Kristian which will allow the client to query per plane
modifiers.


I do recall seeing BSpec text containing "do this thing if render
decompression is enabled" and, at that time, our code wasn't
implementing those instructions. AFAIU, the Kernel didn't really had
support for render decompression, so its specific bits were just
ignored. I was assuming that whoever implemented the feature would add
all the necessary bits, especially since we didn't seem to have any
sort of "if (has_render_decompression(dev_priv))" to call. I am 100%
sure there's such an example in the Gen 9 Watermarks instructions, but
I'm sure I saw more somewhere else (Display WA page?). And reember:
missing watermarks workarounds equals flickering screens.

Is this relevant to your series? How will Mesa be able to detect that
the Kernel it's running on contains the necessary Render Decompression
checks/WAs/code it needs? How can the Kernel detect that Render
Decompression is in use and start doing the 

Re: [Mesa-dev] Seeking advice on implementing KHR_no_error

[Timothy Arceri]

On Fri, 2016-12-30 at 10:02 +1000, Dave Airlie wrote:
> On 30 December 2016 at 09:48, Ilia Mirkin 
> wrote:
> > On Thu, Dec 29, 2016 at 5:54 PM, Thomas Helland
> >  wrote:
> > > Apart from that it's basically a case of looking at a patch
> > > series implementing
> > > some other fairly trivial extension to get an idea of what needs
> > > changing apart
> > > from the functionality of the extension itself.
> > 
> > There are a few ways to implement the ext. You could have the
> > trivial
> > implementation which just makes it not report errors. However
> > there's
> > a bunch of validation that gets done in various cases, which can be
> > costly. I believe the point of the extension is to get rid of that
> > extra costly validation.
> > 
> > You're going to have to track down the specifics, but one thing
> > that
> > comes off the top of my head is the validation that's done for ES
> > interface matching. There's probably 30 other things too though,
> > that
> > one is just one that I happen to remember at this moment. I think
> > sampler/texture validation is another, probably stuff with
> > uniforms,
> > etc.
> > 
> > So the idea would be for those validation functions to just do if
> > (ctxflags & NO_ERROR) { return; } somewhere near the top. (For
> > example.)
> 
> In theory for a lot of the API you could split the API into
> 
> _mesa_API_nocheck
> and
> _mesa_API {
> do error checks
> call _mesa_API_nocheck
> }
> variants, then have a separate dispatch table that goes straight to
> nocheck for a bunch of the API.
> 

Yeah that's what I was thinking when I looked at this extension a while
ago. For my quick look at various api calls at the time there was a lot
of:

_mesa_API {
 do error checks
 do useful things
 do error checks
 }

Which meant there is likely a lot of refactoring work in future for
anyone implementing this extension.

> Then you'd fixed up the other callsites on a case by case basis.
> 
> Dave.
> ___
> 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

Re: [Mesa-dev] [PATCH] dri: allow 16bit R/GR images to be exported via drm buffers

[Ben Widawsky]

On 16-12-16 21:27:51, Rainer Hochecker wrote:

From: Rainer Hochecker 

This allows eglCreateImageKHR to access P010 surfaces created by vaapi

patch for drm, fourcc:
http://paste.ubuntu.com/23638632/

Signed-off-by: Rainer Hochecker 
---
include/GL/internal/dri_interface.h  |  4 
src/egl/drivers/dri2/egl_dri2.c  | 10 ++
src/mesa/drivers/dri/common/dri_util.c   |  4 
src/mesa/drivers/dri/i965/intel_screen.c |  6 ++
4 files changed, 24 insertions(+)

diff --git a/include/GL/internal/dri_interface.h 
b/include/GL/internal/dri_interface.h
index d0b1bc6..933277e 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -1121,6 +1121,8 @@ struct __DRIdri2ExtensionRec {
#define __DRI_IMAGE_FORMAT_XRGB2101010  0x1009
#define __DRI_IMAGE_FORMAT_ARGB2101010  0x100a
#define __DRI_IMAGE_FORMAT_SARGB8   0x100b
+#define __DRI_IMAGE_FORMAT_R16  0x100c
+#define __DRI_IMAGE_FORMAT_GR16 0x100d

#define __DRI_IMAGE_USE_SHARE   0x0001
#define __DRI_IMAGE_USE_SCANOUT 0x0002
@@ -1148,6 +1150,8 @@ struct __DRIdri2ExtensionRec {

#define __DRI_IMAGE_FOURCC_R8   0x20203852
#define __DRI_IMAGE_FOURCC_GR88 0x38385247
+#define __DRI_IMAGE_FOURCC_R16 0x20363152
+#define __DRI_IMAGE_FOURCC_GR160x36315247
#define __DRI_IMAGE_FOURCC_RGB565   0x36314752
#define __DRI_IMAGE_FOURCC_ARGB 0x34325241
#define __DRI_IMAGE_FOURCC_XRGB 0x34325258
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index f18e9fb..f4ed022 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -75,6 +75,14 @@
#define DRM_FORMAT_GR88  fourcc_code('G', 'R', '8', '8') /* [15:0] G:R 
8:8 little endian */
#endif

+#ifndef DRM_FORMAT_R16
+#define DRM_FORMAT_R16   fourcc_code('R', '1', '6', ' ') /* [15:0] R 
16 little endian */
+#endif
+
+#ifndef DRM_FORMAT_GR16
+#define DRM_FORMAT_GR16  fourcc_code('G', 'R', '1', '6') /* [31:0] R:G 
16:16 little endian */
+#endif
+
const __DRIuseInvalidateExtension use_invalidate = {
   .base = { __DRI_USE_INVALIDATE, 1 }
};
@@ -1951,6 +1959,8 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
   case DRM_FORMAT_R8:
   case DRM_FORMAT_RG88:
   case DRM_FORMAT_GR88:
+   case DRM_FORMAT_R16:
+   case DRM_FORMAT_GR16:
   case DRM_FORMAT_RGB332:
   case DRM_FORMAT_BGR233:
   case DRM_FORMAT_XRGB:
diff --git a/src/mesa/drivers/dri/common/dri_util.c 
b/src/mesa/drivers/dri/common/dri_util.c
index 3b81799..c275c07 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -877,8 +877,12 @@ driImageFormatToGLFormat(uint32_t image_format)
  return MESA_FORMAT_R8G8B8X8_UNORM;
   case __DRI_IMAGE_FORMAT_R8:
  return MESA_FORMAT_R_UNORM8;
+   case __DRI_IMAGE_FORMAT_R16:
+  return MESA_FORMAT_R_UNORM16;
   case __DRI_IMAGE_FORMAT_GR88:
  return MESA_FORMAT_R8G8_UNORM;
+   case __DRI_IMAGE_FORMAT_GR16:
+  return MESA_FORMAT_R16G16_UNORM;
   case __DRI_IMAGE_FORMAT_SARGB8:
  return MESA_FORMAT_B8G8R8A8_SRGB;
   case __DRI_IMAGE_FORMAT_NONE:
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
b/src/mesa/drivers/dri/i965/intel_screen.c
index e1c3c19..b3700c6 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -236,9 +236,15 @@ static struct intel_image_format intel_image_formats[] = {
   { __DRI_IMAGE_FOURCC_R8, __DRI_IMAGE_COMPONENTS_R, 1,
 { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 }, } },

+   { __DRI_IMAGE_FOURCC_R16, __DRI_IMAGE_COMPONENTS_R, 1,
+ { { 0, 0, 0, __DRI_IMAGE_FORMAT_R16, 1 }, } },
+
   { __DRI_IMAGE_FOURCC_GR88, __DRI_IMAGE_COMPONENTS_RG, 1,
 { { 0, 0, 0, __DRI_IMAGE_FORMAT_GR88, 2 }, } },

+   { __DRI_IMAGE_FOURCC_GR16, __DRI_IMAGE_COMPONENTS_RG, 1,
+ { { 0, 0, 0, __DRI_IMAGE_FORMAT_GR16, 2 }, } },
+
   { __DRI_IMAGE_FOURCC_YUV410, __DRI_IMAGE_COMPONENTS_Y_U_V, 3,
 { { 0, 0, 0, __DRI_IMAGE_FORMAT_R8, 1 },
   { 1, 2, 2, __DRI_IMAGE_FORMAT_R8, 1 },


This patch looks fine to me. It'd be nice to get a piglit test for it. I think
Chad added one of these last time around.

Anyone have objections?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Seeking advice on implementing KHR_no_error

[Thomas Helland]

2016-12-30 0:39 GMT+01:00 Timothy Arceri :
> On Thu, 2016-12-29 at 23:54 +0100, Thomas Helland wrote:
>> Hi all,
>>
>> I'm sitting here looking for something usefull to do on mesa in my
>> spare time.
>> I've considered implementing some optimization passes, or finding
>> some overhead
>> that I can have a shot at reducing, like the huge amount of
>> malloc/alloc/free calls.
>> However, I've found that this is hard when I have only 2-hour bursts
>> to work on it.
>> It quickly becomes a case where more time is spent catching up than
>> implementing.
>>
>> I've looked at the KHR_no_error extension. It looks like a usefull
>> one.
>> However, I don't know if it is used by any applications as of yet.
>
> Last time I looked at this extension I think there was at least one of
> the mainstream emulators using it. Can't recall which one ... pcsx2
> maybe.
>
>> It also looks like it can be implemented piecewise, as it allows
>> errors to be
>> submitted by the driver even if the extension is enabled.
>> So one can track down each error-checking codepath one by one.
>
> Yep. We would likely enable it and continue to turn things off over
> time as I see it.
>
>> As far as I've understood, _mesa_glsl_error is (one of) the
>> function(s) used to
>> report errors, and glsl_parser_extras.cpp is where a lot of the error
>> checking happens?
>
> Removing error checking from the compiler isn't going to be overly
> useful. The big wins will be from dropping draw time validation, for
> example _mesa_validate_program_pipeline() gets called at draw time in
> ES. In fact we may be able to totally drop calls to
> _mesa_valid_to_render() entirely.
>

Yeah, I was not planning on just disabling the error reporting,
as that would not be as helpful. I was planning something along
the following:

- Find all functions used for error reporting back to the user.
- Grep for the uses of these throughout the codebase
- Start at the ones that are called most often, or have a lot of
  costly work with validation before eventually reporting an error.
- Cut them off as early as possible, before to much work is done.

>> Apart from that it's basically a case of looking at a patch series
>> implementing
>> some other fairly trivial extension to get an idea of what needs
>> changing apart
>> from the functionality of the extension itself.
>>
>> I haven't investigated to much into this yet, as I wanted to air the
>> suggestion.
>> Do you guys think this is a usefull extension, and one that is
>> beneficial?
>
> I think it could be useful. My only concern with this extension was
> that we would plaster if (KHR_no_error_enabled) everywhere. With a bit
> of thought and careful refactoring we might be able to do it in a way
> that avoids this to much.
>

I was kinda worried about this myself. The spec says that GetError
should return NO_ERROR or OUT_OF_MEMORY only. So I don't
think we can simply enable the extension and pretend nothing
ever happened.

>> If anyone has any suggestions for other small tasks, feel free to
>> share them.
>
> Not sure if you noticed but NIR loop unrolling landed recently. Yay! I
> believe there is likely more room for improvements in unrolling further
>   loops. More analysis of the remaining loops in shader-db is needed.
> Looking at those would probably fit well into 2-hour bursts.
>

Thanks for completing the work! It's nice to see this finally land.
You've done one heck of a job, if I dare say so.

>>
>> Holiday greetings,
>> Thomas
>> ___
>> 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

Re: [Mesa-dev] Seeking advice on implementing KHR_no_error

[Dave Airlie]

On 30 December 2016 at 09:48, Ilia Mirkin  wrote:
> On Thu, Dec 29, 2016 at 5:54 PM, Thomas Helland
>  wrote:
>> Apart from that it's basically a case of looking at a patch series 
>> implementing
>> some other fairly trivial extension to get an idea of what needs changing 
>> apart
>> from the functionality of the extension itself.
>
> There are a few ways to implement the ext. You could have the trivial
> implementation which just makes it not report errors. However there's
> a bunch of validation that gets done in various cases, which can be
> costly. I believe the point of the extension is to get rid of that
> extra costly validation.
>
> You're going to have to track down the specifics, but one thing that
> comes off the top of my head is the validation that's done for ES
> interface matching. There's probably 30 other things too though, that
> one is just one that I happen to remember at this moment. I think
> sampler/texture validation is another, probably stuff with uniforms,
> etc.
>
> So the idea would be for those validation functions to just do if
> (ctxflags & NO_ERROR) { return; } somewhere near the top. (For
> example.)

In theory for a lot of the API you could split the API into

_mesa_API_nocheck
and
_mesa_API {
do error checks
call _mesa_API_nocheck
}
variants, then have a separate dispatch table that goes straight to
nocheck for a bunch of the API.

Then you'd fixed up the other callsites on a case by case basis.

Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] radv: only allow cmask/dcc in color optimal.

[Dave Airlie]

From: Dave Airlie 

I had this on transfers due to the clear color cmd, but
it seems like that path shouldn't get fast clears.

Signed-off-by: Dave Airlie 
---
 src/amd/vulkan/radv_image.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c
index a3f5676..9c0bba2 100644
--- a/src/amd/vulkan/radv_image.c
+++ b/src/amd/vulkan/radv_image.c
@@ -712,7 +712,7 @@ radv_image_create(VkDevice _device,
image->size = image->surface.bo_size;
image->alignment = image->surface.bo_alignment;
 
-   if (image->exclusive || (image->queue_family_mask & 1) == 1)
+   if (image->exclusive || image->queue_family_mask == 1)
can_cmask_dcc = true;
 
if ((pCreateInfo->usage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) &&
@@ -900,8 +900,7 @@ bool radv_layout_can_fast_clear(const struct radv_image 
*image,
VkImageLayout layout,
unsigned queue_mask)
 {
-   return (layout == VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL ||
-   layout == VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL) &&
+   return layout == VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL &&
queue_mask == (1u << RADV_QUEUE_GENERAL);
 }
 
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] radv: only allow cmask/dcc on exclusive or concurrent with graphics queue.

[Dave Airlie]

From: Dave Airlie 

Otherwise we don't get the barriers to flush dcc etc.

Signed-off-by: Dave Airlie 
---
 src/amd/vulkan/radv_image.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c
index 8af064b..a3f5676 100644
--- a/src/amd/vulkan/radv_image.c
+++ b/src/amd/vulkan/radv_image.c
@@ -674,7 +674,7 @@ radv_image_create(VkDevice _device,
RADV_FROM_HANDLE(radv_device, device, _device);
const VkImageCreateInfo *pCreateInfo = create_info->vk_info;
struct radv_image *image = NULL;
-
+   bool can_cmask_dcc = false;
assert(pCreateInfo->sType == VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO);
 
radv_assert(pCreateInfo->mipLevels > 0);
@@ -712,15 +712,18 @@ radv_image_create(VkDevice _device,
image->size = image->surface.bo_size;
image->alignment = image->surface.bo_alignment;
 
+   if (image->exclusive || (image->queue_family_mask & 1) == 1)
+   can_cmask_dcc = true;
+
if ((pCreateInfo->usage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) &&
-   image->surface.dcc_size)
+   image->surface.dcc_size && can_cmask_dcc)
radv_image_alloc_dcc(device, image);
else
image->surface.dcc_size = 0;
 
if ((pCreateInfo->usage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) &&
pCreateInfo->mipLevels == 1 &&
-   !image->surface.dcc_size && image->extent.depth == 1)
+   !image->surface.dcc_size && image->extent.depth == 1 && 
can_cmask_dcc)
radv_image_alloc_cmask(device, image);
if (image->samples > 1 && vk_format_is_color(pCreateInfo->format)) {
radv_image_alloc_fmask(device, image);
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] glsl: Don't compute the hash when we already have it

[Thomas Helland]

We should really have a function to copy the table contents,
but this will at least get us somewhere in the meantime.
---
 src/compiler/glsl/opt_copy_propagation_elements.cpp | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/opt_copy_propagation_elements.cpp 
b/src/compiler/glsl/opt_copy_propagation_elements.cpp
index 9f79fa9..3d11d02 100644
--- a/src/compiler/glsl/opt_copy_propagation_elements.cpp
+++ b/src/compiler/glsl/opt_copy_propagation_elements.cpp
@@ -143,11 +143,13 @@ public:
   struct hash_entry *entry;
 
   hash_table_foreach(lhs, entry) {
- _mesa_hash_table_insert(lhs_ht, entry->key, entry->data);
+ _mesa_hash_table_insert_pre_hashed(lhs_ht, entry->hash,
+entry->key, entry->data);
   }
 
   hash_table_foreach(rhs, entry) {
- _mesa_hash_table_insert(rhs_ht, entry->key, entry->data);
+ _mesa_hash_table_insert_pre_hashed(rhs_ht, entry->hash,
+entry->key, entry->data);
   }
}
 
-- 
2.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] glsl: Don't rehash the values when copying to new table

[Thomas Helland]

Really, we should have some kind of function for copying the whole table,
but this will work for now.
---
 src/compiler/glsl/opt_copy_propagation.cpp | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/opt_copy_propagation.cpp 
b/src/compiler/glsl/opt_copy_propagation.cpp
index 247c498..e9f82e0 100644
--- a/src/compiler/glsl/opt_copy_propagation.cpp
+++ b/src/compiler/glsl/opt_copy_propagation.cpp
@@ -210,7 +210,8 @@ ir_copy_propagation_visitor::handle_if_block(exec_list 
*instructions)
/* Populate the initial acp with a copy of the original */
struct hash_entry *entry;
hash_table_foreach(orig_acp, entry) {
-  _mesa_hash_table_insert(acp, entry->key, entry->data);
+  _mesa_hash_table_insert_pre_hashed(acp, entry->hash,
+ entry->key, entry->data);
}
 
visit_list_elements(this, instructions);
@@ -259,7 +260,8 @@ ir_copy_propagation_visitor::handle_loop(ir_loop *ir, bool 
keep_acp)
if (keep_acp) {
   struct hash_entry *entry;
   hash_table_foreach(orig_acp, entry) {
- _mesa_hash_table_insert(acp, entry->key, entry->data);
+ _mesa_hash_table_insert_pre_hashed(acp, entry->hash,
+entry->key, entry->data);
   }
}
 
-- 
2.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Seeking advice on implementing KHR_no_error

[Ilia Mirkin]

On Thu, Dec 29, 2016 at 5:54 PM, Thomas Helland
 wrote:
> Apart from that it's basically a case of looking at a patch series 
> implementing
> some other fairly trivial extension to get an idea of what needs changing 
> apart
> from the functionality of the extension itself.

There are a few ways to implement the ext. You could have the trivial
implementation which just makes it not report errors. However there's
a bunch of validation that gets done in various cases, which can be
costly. I believe the point of the extension is to get rid of that
extra costly validation.

You're going to have to track down the specifics, but one thing that
comes off the top of my head is the validation that's done for ES
interface matching. There's probably 30 other things too though, that
one is just one that I happen to remember at this moment. I think
sampler/texture validation is another, probably stuff with uniforms,
etc.

So the idea would be for those validation functions to just do if
(ctxflags & NO_ERROR) { return; } somewhere near the top. (For
example.)

Cheers,

  -ilia

[P.S. Just read Tim's reply, sounds like we're talking about similar things.]
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] clover: Check for executables before enqueueing a kernel

[Francisco Jerez]

Pierre Moreau  writes:

> Without this check, the kernel::bind() method would fail with a
> std::out_of_range exception, letting an exception escape from the
> library into the client, rather than returning the corresponding error
> code CL_INVALID_PROGRAM_EXECUTABLE.
>
> Signed-off-by: Pierre Moreau 

Reviewed-by: Francisco Jerez 

> ---
>  src/gallium/state_trackers/clover/api/kernel.cpp | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/state_trackers/clover/api/kernel.cpp 
> b/src/gallium/state_trackers/clover/api/kernel.cpp
> index 73ba34abe8..e3d75af972 100644
> --- a/src/gallium/state_trackers/clover/api/kernel.cpp
> +++ b/src/gallium/state_trackers/clover/api/kernel.cpp
> @@ -215,7 +215,10 @@ namespace {
>  }, kern.args()))
>   throw error(CL_INVALID_KERNEL_ARGS);
>  
> -  if (!count(q.device(), kern.program().devices()))
> +  // If the command queue's device is not associated to the program, we 
> get
> +  // a module, with no sections, which will also fail the following test.
> +  auto  = kern.program().build(q.device()).binary;
> +  if (!any_of(type_equals(module::section::text_executable), m.secs))
>   throw error(CL_INVALID_PROGRAM_EXECUTABLE);
> }
>  
> -- 
> 2.11.0


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Seeking advice on implementing KHR_no_error

[Timothy Arceri]

On Thu, 2016-12-29 at 23:54 +0100, Thomas Helland wrote:
> Hi all,
> 
> I'm sitting here looking for something usefull to do on mesa in my
> spare time.
> I've considered implementing some optimization passes, or finding
> some overhead
> that I can have a shot at reducing, like the huge amount of
> malloc/alloc/free calls.
> However, I've found that this is hard when I have only 2-hour bursts
> to work on it.
> It quickly becomes a case where more time is spent catching up than
> implementing.
> 
> I've looked at the KHR_no_error extension. It looks like a usefull
> one.
> However, I don't know if it is used by any applications as of yet.

Last time I looked at this extension I think there was at least one of
the mainstream emulators using it. Can't recall which one ... pcsx2
maybe.

> It also looks like it can be implemented piecewise, as it allows
> errors to be
> submitted by the driver even if the extension is enabled.
> So one can track down each error-checking codepath one by one.

Yep. We would likely enable it and continue to turn things off over
time as I see it.

> As far as I've understood, _mesa_glsl_error is (one of) the
> function(s) used to
> report errors, and glsl_parser_extras.cpp is where a lot of the error
> checking happens?

Removing error checking from the compiler isn't going to be overly
useful. The big wins will be from dropping draw time validation, for
example _mesa_validate_program_pipeline() gets called at draw time in
ES. In fact we may be able to totally drop calls to
_mesa_valid_to_render() entirely.

> Apart from that it's basically a case of looking at a patch series
> implementing
> some other fairly trivial extension to get an idea of what needs
> changing apart
> from the functionality of the extension itself.
> 
> I haven't investigated to much into this yet, as I wanted to air the
> suggestion.
> Do you guys think this is a usefull extension, and one that is
> beneficial?

I think it could be useful. My only concern with this extension was
that we would plaster if (KHR_no_error_enabled) everywhere. With a bit
of thought and careful refactoring we might be able to do it in a way
that avoids this to much.

> If anyone has any suggestions for other small tasks, feel free to
> share them.

Not sure if you noticed but NIR loop unrolling landed recently. Yay! I
believe there is likely more room for improvements in unrolling further
  loops. More analysis of the remaining loops in shader-db is needed.
Looking at those would probably fit well into 2-hour bursts.

> 
> Holiday greetings,
> Thomas
> ___
> 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

[Mesa-dev] [PATCH] clover: Check for executables before enqueueing a kernel

[Pierre Moreau]

Without this check, the kernel::bind() method would fail with a
std::out_of_range exception, letting an exception escape from the
library into the client, rather than returning the corresponding error
code CL_INVALID_PROGRAM_EXECUTABLE.

Signed-off-by: Pierre Moreau 
---
 src/gallium/state_trackers/clover/api/kernel.cpp | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/gallium/state_trackers/clover/api/kernel.cpp 
b/src/gallium/state_trackers/clover/api/kernel.cpp
index 73ba34abe8..e3d75af972 100644
--- a/src/gallium/state_trackers/clover/api/kernel.cpp
+++ b/src/gallium/state_trackers/clover/api/kernel.cpp
@@ -215,7 +215,10 @@ namespace {
 }, kern.args()))
  throw error(CL_INVALID_KERNEL_ARGS);
 
-  if (!count(q.device(), kern.program().devices()))
+  // If the command queue's device is not associated to the program, we get
+  // a module, with no sections, which will also fail the following test.
+  auto  = kern.program().build(q.device()).binary;
+  if (!any_of(type_equals(module::section::text_executable), m.secs))
  throw error(CL_INVALID_PROGRAM_EXECUTABLE);
}
 
-- 
2.11.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] meta: Disable dithering during glGenerateMipmap

[Jason Ekstrand]

On Dec 29, 2016 4:58 PM, "Kenneth Graunke"  wrote:

On Thursday, December 29, 2016 1:25:57 PM PST Chad Versace wrote:
> Fixes tests 'dEQP-GLES3.functional.texture.mipmap.*.generate.rgba5551*' on
> Intel Broadwell 0x1616.
>
> The GL 4.5 spec describes the algorithm of glGenerateMipmap as:
>
> The contents of the derived images are computed by repeated, filtered
> reduction of the level base image.  [...] No particular filter
algorithm is
> required, though a box filter is recommended as the default filter.
>
> Consider a texture for which all pixels are identical at level 0.
> From the spec's description above, one may reasonably assume that the
"filtered
> reduction" of level 0 produces a new miplevel for which again all pixels
are
> identical. For any 2x2 subspan of identical pixels, it is difficult to
see how
> the "filtered reduction" of that subspan can produce a pixel that differs
from
> the source pixels.
>
> Dithering during _mesa_meta_GenerateMipmap() violated that reasonable
> assumption.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99210
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/mesa/drivers/common/meta_generate_mipmap.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/mesa/drivers/common/meta_generate_mipmap.c
b/src/mesa/drivers/common/meta_generate_mipmap.c
> index bbe9d6d886..55093e9553 100644
> --- a/src/mesa/drivers/common/meta_generate_mipmap.c
> +++ b/src/mesa/drivers/common/meta_generate_mipmap.c
> @@ -182,6 +182,7 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx,
GLenum target,
>
> _mesa_meta_begin(ctx, MESA_META_ALL & ~MESA_META_DRAW_BUFFERS);
> _mesa_ColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
> +   _mesa_Disable(GL_DITHER);
>
> /* Choose between glsl version and fixed function version of
>  * GenerateMipmap function.
>

Good catch!  That makes sense.  I verified that the meta begin flags
will restore the dither mode at the end.


Die meta! Die!

Reviewed-by: Kenneth Graunke 

___
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

Re: [Mesa-dev] [PATCH] meta: Disable dithering during glGenerateMipmap

[Kenneth Graunke]

On Thursday, December 29, 2016 1:25:57 PM PST Chad Versace wrote:
> Fixes tests 'dEQP-GLES3.functional.texture.mipmap.*.generate.rgba5551*' on
> Intel Broadwell 0x1616.
> 
> The GL 4.5 spec describes the algorithm of glGenerateMipmap as:
> 
> The contents of the derived images are computed by repeated, filtered
> reduction of the level base image.  [...] No particular filter algorithm 
> is
> required, though a box filter is recommended as the default filter.
> 
> Consider a texture for which all pixels are identical at level 0.
> From the spec's description above, one may reasonably assume that the 
> "filtered
> reduction" of level 0 produces a new miplevel for which again all pixels are
> identical. For any 2x2 subspan of identical pixels, it is difficult to see how
> the "filtered reduction" of that subspan can produce a pixel that differs from
> the source pixels.
> 
> Dithering during _mesa_meta_GenerateMipmap() violated that reasonable
> assumption.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99210
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/mesa/drivers/common/meta_generate_mipmap.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/mesa/drivers/common/meta_generate_mipmap.c 
> b/src/mesa/drivers/common/meta_generate_mipmap.c
> index bbe9d6d886..55093e9553 100644
> --- a/src/mesa/drivers/common/meta_generate_mipmap.c
> +++ b/src/mesa/drivers/common/meta_generate_mipmap.c
> @@ -182,6 +182,7 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, GLenum 
> target,
>  
> _mesa_meta_begin(ctx, MESA_META_ALL & ~MESA_META_DRAW_BUFFERS);
> _mesa_ColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
> +   _mesa_Disable(GL_DITHER);
>  
> /* Choose between glsl version and fixed function version of
>  * GenerateMipmap function.
> 

Good catch!  That makes sense.  I verified that the meta begin flags
will restore the dither mode at the end.

Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] Seeking advice on implementing KHR_no_error

[Thomas Helland]

Hi all,

I'm sitting here looking for something usefull to do on mesa in my spare time.
I've considered implementing some optimization passes, or finding some overhead
that I can have a shot at reducing, like the huge amount of
malloc/alloc/free calls.
However, I've found that this is hard when I have only 2-hour bursts
to work on it.
It quickly becomes a case where more time is spent catching up than
implementing.

I've looked at the KHR_no_error extension. It looks like a usefull one.
However, I don't know if it is used by any applications as of yet.
It also looks like it can be implemented piecewise, as it allows errors to be
submitted by the driver even if the extension is enabled.
So one can track down each error-checking codepath one by one.
As far as I've understood, _mesa_glsl_error is (one of) the function(s) used to
report errors, and glsl_parser_extras.cpp is where a lot of the error
checking happens?
Apart from that it's basically a case of looking at a patch series implementing
some other fairly trivial extension to get an idea of what needs changing apart
from the functionality of the extension itself.

I haven't investigated to much into this yet, as I wanted to air the suggestion.
Do you guys think this is a usefull extension, and one that is beneficial?
If anyone has any suggestions for other small tasks, feel free to share them.

Holiday greetings,
Thomas
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] meta: Disable dithering during glGenerateMipmap

[Chad Versace]

Fixes tests 'dEQP-GLES3.functional.texture.mipmap.*.generate.rgba5551*' on
Intel Broadwell 0x1616.

The GL 4.5 spec describes the algorithm of glGenerateMipmap as:

The contents of the derived images are computed by repeated, filtered
reduction of the level base image.  [...] No particular filter algorithm is
required, though a box filter is recommended as the default filter.

Consider a texture for which all pixels are identical at level 0.
From the spec's description above, one may reasonably assume that the "filtered
reduction" of level 0 produces a new miplevel for which again all pixels are
identical. For any 2x2 subspan of identical pixels, it is difficult to see how
the "filtered reduction" of that subspan can produce a pixel that differs from
the source pixels.

Dithering during _mesa_meta_GenerateMipmap() violated that reasonable
assumption.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99210
Cc: mesa-sta...@lists.freedesktop.org
---
 src/mesa/drivers/common/meta_generate_mipmap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/mesa/drivers/common/meta_generate_mipmap.c 
b/src/mesa/drivers/common/meta_generate_mipmap.c
index bbe9d6d886..55093e9553 100644
--- a/src/mesa/drivers/common/meta_generate_mipmap.c
+++ b/src/mesa/drivers/common/meta_generate_mipmap.c
@@ -182,6 +182,7 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, GLenum 
target,
 
_mesa_meta_begin(ctx, MESA_META_ALL & ~MESA_META_DRAW_BUFFERS);
_mesa_ColorMask(GL_TRUE, GL_TRUE, GL_TRUE, GL_TRUE);
+   _mesa_Disable(GL_DITHER);
 
/* Choose between glsl version and fixed function version of
 * GenerateMipmap function.
-- 
2.11.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 6/9] i965: Add support for tex upload using gpu

[Anuj Phogat]

On Tue, Dec 20, 2016 at 6:45 AM, Topi Pohjolainen
 wrote:
> Signed-off-by: Topi Pohjolainen 
> ---
>  src/mesa/drivers/dri/i965/intel_tex.h  |   8 +
>  src/mesa/drivers/dri/i965/intel_tex_subimage.c | 194 
> +
>  2 files changed, 202 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_tex.h 
> b/src/mesa/drivers/dri/i965/intel_tex.h
> index 376f075..c7d0937 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex.h
> +++ b/src/mesa/drivers/dri/i965/intel_tex.h
> @@ -65,6 +65,14 @@ intel_texsubimage_tiled_memcpy(struct gl_context *ctx,
> bool for_glTexImage);
>
>  bool
> +intel_texsubimage_gpu_copy(struct brw_context *brw, GLuint dims,
> +   struct gl_texture_image *tex_image,
> +   unsigned x, unsigned y, unsigned z,
> +   unsigned w, unsigned h, unsigned d,
> +   GLenum format, GLenum type, const void *pixels,
> +   const struct gl_pixelstore_attrib *packing);
> +
> +bool
>  intel_gettexsubimage_tiled_memcpy(struct gl_context *ctx,
>struct gl_texture_image *texImage,
>GLint xoffset, GLint yofset,
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_subimage.c 
> b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> index b7e52bc..f999a93 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> @@ -24,6 +24,7 @@
>   */
>
>  #include "main/bufferobj.h"
> +#include "main/glformats.h"
>  #include "main/image.h"
>  #include "main/macros.h"
>  #include "main/mtypes.h"
> @@ -34,8 +35,10 @@
>  #include "main/enums.h"
>  #include "drivers/common/meta.h"
>
> +#include "brw_blorp.h"
>  #include "brw_context.h"
>  #include "intel_batchbuffer.h"
> +#include "intel_buffer_objects.h"
>  #include "intel_tex.h"
>  #include "intel_mipmap_tree.h"
>  #include "intel_blit.h"
> @@ -43,6 +46,197 @@
>
>  #define FILE_DEBUG_FLAG DEBUG_TEXTURE
>
> +static drm_intel_bo *
> +intel_texsubimage_get_src_as_bo(struct brw_context *brw, unsigned dims,
> +struct gl_texture_image *tex_image,
> +unsigned w, unsigned h, unsigned d,
> +GLenum format, GLenum type, const void 
> *pixels,
> +const struct gl_pixelstore_attrib *packing)
> +{
> +   /* Account for SKIP_PIXELS, SKIP_ROWS, ALIGNMENT, and SKIP_IMAGES */
> +   const uint32_t first_pixel = _mesa_image_offset(dims, packing, w, h,
> +   format, type, 0, 0, 0);
> +   const uint32_t last_pixel =  _mesa_image_offset(dims, packing, w, h,
> +   format, type,
> +   d - 1, h - 1, w);
> +   const uint32_t size = last_pixel - first_pixel;
> +
> +   drm_intel_bo * const bo =
> +  drm_intel_bo_alloc(brw->bufmgr, "tmp_tex_subimage_src", size, 64);
> +
> +   if (bo == NULL) {
> +  perf_debug("intel_texsubimage: temp bo creation failed: size = %u\n",
> + size);
> +  return false;
> +   }
> +
> +   if (drm_intel_bo_subdata(bo, 0, size, pixels + first_pixel)) {
> +  perf_debug("intel_texsubimage: temp bo upload failed\n");
> +  drm_intel_bo_unreference(bo);
> +  return NULL;
> +   }
> +
> +   return bo;
> +}
> +
> +static uint32_t
> +intel_texsubimage_get_src_offset(unsigned dims, unsigned w, unsigned h,
> + GLenum format, GLenum type,
> + const void *pixels,
> + const struct gl_pixelstore_attrib *packing)
> +{
> +   /* Account for SKIP_PIXELS, SKIP_ROWS, ALIGNMENT, and SKIP_IMAGES */
> +   const uint32_t first_pixel = _mesa_image_offset(dims, packing, w, h,
> +   format, type, 0, 0, 0);
> +
> +   /* In case of buffer object source 'pixels' represents offset in bytes. */
> +   return first_pixel + (intptr_t)pixels;
> +}
> +
> +/* Consider all the restrictions and determine the format of the source. */
> +static mesa_format
> +intel_texsubimage_check_upload(struct brw_context *brw,
> +   const struct gl_texture_image *tex_image,
> +   unsigned h, GLenum format, GLenum type,
> +   const struct gl_pixelstore_attrib *packing)
> +{
> +   /* TODO: Add support for buffer object upload 1D alignment or perhaps use
> +* flat 2D source.
> +*/
> +   if (tex_image->TexObject->Target == GL_TEXTURE_1D_ARRAY) {
> +  perf_debug("intel_texsubimage: 1D_ARRAY not supported\n");
> +  return MESA_FORMAT_NONE;
> +   }
> +
> +   if (brw->ctx._ImageTransferState)
> +  return MESA_FORMAT_NONE;
> +
> +   if 

Re: [Mesa-dev] [PATCH V2 2/70] mesa/glsl: move subroutine metadata to gl_program

[Eric Anholt]

Timothy Arceri  writes:

> This will allow us to store gl_program rather than gl_shader_program
> as the current program perstage which allows us to simplify code
> that makes use of the CurrentProgram list.
>
> V2: don't change subroutine init helper interface

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH V2] mesa/compiler: add stage to shader_info

[Eric Anholt]

Timothy Arceri  writes:

> This will allow us to simplify the current program logic for SSO.
>
> Also since we aim to detach shader_info from nir_shader this will come
> in handy avoiding passing nir_shader around just to keep track of
> the stage we are dealing with.
>
> V2: set stage for arb asm programs also.

Reviewed-by: Eric Anholt 


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] glsl: disable dead code removal of lowered ubos

[Kenneth Graunke]

On Thursday, December 29, 2016 1:40:00 PM PST Timothy Arceri wrote:
> This lets us assign uniform storage for packed UBOs after
> they have been lowered otherwise the var is removed too early.
> ---
>  src/compiler/glsl/glsl_parser_extras.cpp   | 5 +++--
>  src/compiler/glsl/ir_optimization.h| 4 +++-
>  src/compiler/glsl/link_varyings.cpp| 2 +-
>  src/compiler/glsl/linker.cpp   | 1 +
>  src/compiler/glsl/opt_dead_code.cpp| 8 +---
>  src/compiler/glsl/standalone.cpp   | 1 +
>  src/compiler/glsl/test_optpass.cpp | 5 +++--
>  src/mesa/drivers/dri/i965/brw_link.cpp | 2 +-
>  src/mesa/main/ff_fragment_shader.cpp   | 2 +-
>  src/mesa/program/ir_to_mesa.cpp| 2 +-
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 +-
>  11 files changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
> b/src/compiler/glsl/glsl_parser_extras.cpp
> index f1820b9..673f6d2 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -1969,7 +1969,7 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, 
> struct gl_shader *shader,
>/* Do some optimization at compile time to reduce shader IR size
> * and reduce later work if the same shader is linked multiple times
> */
> -  while (do_common_optimization(shader->ir, false, false, options,
> +  while (do_common_optimization(shader->ir, false, false, false, options,
>  ctx->Const.NativeIntegers))
>   ;
>  
> @@ -2067,6 +2067,7 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, 
> struct gl_shader *shader,
>  bool
>  do_common_optimization(exec_list *ir, bool linked,
>  bool uniform_locations_assigned,
> +   bool ubos_lowered,
> const struct gl_shader_compiler_options *options,
> bool native_integers)
>  {
> @@ -2109,7 +2110,7 @@ do_common_optimization(exec_list *ir, bool linked,
> }
>  
> if (linked)
> -  OPT(do_dead_code, ir, uniform_locations_assigned);
> +  OPT(do_dead_code, ir, uniform_locations_assigned, ubos_lowered);
> else
>OPT(do_dead_code_unlinked, ir);
> OPT(do_dead_code_local, ir);
> diff --git a/src/compiler/glsl/ir_optimization.h 
> b/src/compiler/glsl/ir_optimization.h
> index 0d6c4e6..88a4ebd 100644
> --- a/src/compiler/glsl/ir_optimization.h
> +++ b/src/compiler/glsl/ir_optimization.h
> @@ -77,6 +77,7 @@ enum lower_packing_builtins_op {
>  
>  bool do_common_optimization(exec_list *ir, bool linked,
>   bool uniform_locations_assigned,
> +bool ubos_lowered,
>  const struct gl_shader_compiler_options *options,
>  bool native_integers);
>  
> @@ -97,7 +98,8 @@ void do_dead_builtin_varyings(struct gl_context *ctx,
>gl_linked_shader *consumer,
>unsigned num_tfeedback_decls,
>class tfeedback_decl *tfeedback_decls);
> -bool do_dead_code(exec_list *instructions, bool uniform_locations_assigned);
> +bool do_dead_code(exec_list *instructions, bool uniform_locations_assigned,
> +  bool ubos_lowered);
>  bool do_dead_code_local(exec_list *instructions);
>  bool do_dead_code_unlinked(exec_list *instructions);
>  bool do_dead_functions(exec_list *instructions);
> diff --git a/src/compiler/glsl/link_varyings.cpp 
> b/src/compiler/glsl/link_varyings.cpp
> index fe499a5..dd1c36a 100644
> --- a/src/compiler/glsl/link_varyings.cpp
> +++ b/src/compiler/glsl/link_varyings.cpp
> @@ -607,7 +607,7 @@ remove_unused_shader_inputs_and_outputs(bool 
> is_separate_shader_object,
> /* Eliminate code that is now dead due to unused inputs/outputs being
>  * demoted.
>  */
> -   while (do_dead_code(sh->ir, false))
> +   while (do_dead_code(sh->ir, false, true))
>;
>  
>  }
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 126d11d..d7ed436 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -5053,6 +5053,7 @@ link_shaders(struct gl_context *ctx, struct 
> gl_shader_program *prog)
>}
>  
>while (do_common_optimization(prog->_LinkedShaders[i]->ir, true, false,
> +false,
>  >Const.ShaderCompilerOptions[i],
>  ctx->Const.NativeIntegers))
>   ;
> diff --git a/src/compiler/glsl/opt_dead_code.cpp 
> b/src/compiler/glsl/opt_dead_code.cpp
> index 75e668a..980660e 100644
> --- a/src/compiler/glsl/opt_dead_code.cpp
> +++ b/src/compiler/glsl/opt_dead_code.cpp
> @@ -43,7 +43,8 @@ static bool debug = false;
>   * for usage on an unlinked instruction stream.
>   */
>  bool
> -do_dead_code(exec_list *instructions, bool 

Re: [Mesa-dev] [PATCH 1/4] glsl: move more varying linking code to link_varyings.cpp

[Kenneth Graunke]

On Thursday, December 29, 2016 2:15:10 PM PST Timothy Arceri wrote:
> ---
>  src/compiler/glsl/link_varyings.cpp | 157 
> 
>  src/compiler/glsl/link_varyings.h   |   3 +
>  src/compiler/glsl/linker.cpp| 149 +-
>  3 files changed, 161 insertions(+), 148 deletions(-)

Good stuff!  Series is:
Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev