date:20150929

Re: [Mesa-dev] [PATCH V6 10/27] i965: fix atomic buffer index for bindings other than 0

2015-09-29 Thread Ilia Mirkin

On Tue, Sep 29, 2015 at 2:26 AM, Timothy Arceri  wrote:
> On Tue, 2015-09-29 at 02:08 -0400, Ilia Mirkin wrote:
>> On Tue, Sep 29, 2015 at 2:05 AM, Timothy Arceri <
>> t_arc...@yahoo.com.au> wrote:
>> > On Tue, 2015-09-29 at 01:05 -0400, Ilia Mirkin wrote:
>> > > On Mon, Sep 28, 2015 at 10:42 PM, Timothy Arceri <
>> > > t_arc...@yahoo.com.au> wrote:
>> > > > Since commit c0cd5b var->data.binding was being used as a
>> > > > replacement
>> > > > for atomic buffer index, but they don't have to be the same
>> > > > value
>> > > > they
>> > > > just happen to end up the same when binding is 0.
>> > > >
>> > > > Now that we store the atomic uniform location in var
>> > > > ->data.location
>> > > > we can use this to lookup the atomic buffer index in uniform
>> > > > storage.
>> > > >
>> > > > For arrays of arrays the outer arrays have separate uniform
>> > > > locations
>> > > > however they all share the same buffer so we can get the buffer
>> > > > index
>> > > > using the base uniform location.
>> > > >
>> > > > Cc: Alejandro Piñeiro 
>> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90175
>> > > > ---
>> > > >  src/glsl/nir/glsl_to_nir.cpp   |  2 --
>> > > >  src/glsl/nir/nir.h |  4 ++--
>> > > >  src/glsl/nir/nir_lower_atomics.c   | 18
>> > > > --
>> > > > 
>> > > >  src/mesa/drivers/dri/i965/brw_nir.c|  6 --
>> > > >  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |  2 +-
>> > > >  5 files changed, 19 insertions(+), 13 deletions(-)
>> > > >
>> > > > diff --git a/src/glsl/nir/glsl_to_nir.cpp
>> > > > b/src/glsl/nir/glsl_to_nir.cpp
>> > > > index f03a107..30726be 100644
>> > > > --- a/src/glsl/nir/glsl_to_nir.cpp
>> > > > +++ b/src/glsl/nir/glsl_to_nir.cpp
>> > > > @@ -330,8 +330,6 @@ nir_visitor::visit(ir_variable *ir)
>> > > >
>> > > > var->data.index = ir->data.index;
>> > > > var->data.binding = ir->data.binding;
>> > > > -   /* XXX Get rid of buffer_index */
>> > > > -   var->data.atomic.buffer_index = ir->data.binding;
>> > > > var->data.atomic.offset = ir->data.atomic.offset;
>> > > > var->data.image.read_only = ir->data.image_read_only;
>> > > > var->data.image.write_only = ir->data.image_write_only;
>> > > > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
>> > > > index 4f45770..670e39e 100644
>> > > > --- a/src/glsl/nir/nir.h
>> > > > +++ b/src/glsl/nir/nir.h
>> > > > @@ -308,7 +308,6 @@ typedef struct {
>> > > > * Location an atomic counter is stored at.
>> > > > */
>> > > >struct {
>> > > > - unsigned buffer_index;
>> > > >   unsigned offset;
>> > > >} atomic;
>> > > >
>> > > > @@ -1880,7 +1879,8 @@ void nir_lower_clip_fs(nir_shader
>> > > > *shader,
>> > > > unsigned ucp_enables);
>> > > >
>> > > >  void nir_lower_two_sided_color(nir_shader *shader);
>> > > >
>> > > > -void nir_lower_atomics(nir_shader *shader);
>> > > > +void nir_lower_atomics(nir_shader *shader,
>> > > > +   const struct gl_shader_program
>> > > > *shader_program);
>> > > >  void nir_lower_to_source_mods(nir_shader *shader);
>> > > >
>> > > >  bool nir_lower_gs_intrinsics(nir_shader *shader);
>> > > > diff --git a/src/glsl/nir/nir_lower_atomics.c
>> > > > b/src/glsl/nir/nir_lower_atomics.c
>> > > > index 46e1376..52e7675 100644
>> > > > --- a/src/glsl/nir/nir_lower_atomics.c
>> > > > +++ b/src/glsl/nir/nir_lower_atomics.c
>> > > > @@ -25,6 +25,7 @@
>> > > >   *
>> > > >   */
>> > > >
>> > > > +#include "ir_uniform.h"
>> > > >  #include "nir.h"
>> > > >  #include "main/config.h"
>> > > >  #include 
>> > > > @@ -35,7 +36,8 @@
>> > > >   */
>> > > >
>> > > >  static void
>> > > > -lower_instr(nir_intrinsic_instr *instr, nir_function_impl
>> > > > *impl)
>> > > > +lower_instr(nir_intrinsic_instr *instr,
>> > > > +const struct gl_shader_program *shader_program)
>> > > >  {
>> > > > nir_intrinsic_op op;
>> > > > switch (instr->intrinsic) {
>> > > > @@ -60,10 +62,11 @@ lower_instr(nir_intrinsic_instr *instr,
>> > > > nir_function_impl *impl)
>> > > >return; /* atomics passed as function arguments can't be
>> > > > lowered */
>> > > >
>> > > > void *mem_ctx = ralloc_parent(instr);
>> > > > +   unsigned uniform_loc = instr->variables[0]->var
>> > > > ->data.location;
>> > > >
>> > > > nir_intrinsic_instr *new_instr =
>> > > > nir_intrinsic_instr_create(mem_ctx, op);
>> > > > new_instr->const_index[0] =
>> > > > -  (int) instr->variables[0]->var
>> > > > ->data.atomic.buffer_index;
>> > > > +  shader_program
>> > > > ->UniformStorage[uniform_loc].atomic_buffer_index;
>> > > >
>> > > > nir_load_const_instr *offset_const =
>> > > > nir_load_const_instr_create(mem_ctx, 1);
>> > > > offset_const->value.u[0] = instr->variables[0]->var
>> > > > ->data.atomic.offset;
>> > > > @@ -128,22 +131,25 @@ lower_instr(nir_intrinsic_instr *instr,
>> > > >

Re: [Mesa-dev] [PATCH] util: implement strndup for WIN32

2015-09-29 Thread Samuel Iglesias Gonsálvez



On 28/09/15 18:14, Emil Velikov wrote:
> Hi Samuel,
> 
> On 28 September 2015 at 15:19, Samuel Iglesias Gonsalvez
>  wrote:
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92124
>> Cc: Jose Fonseca 
>> ---
>>
>> I tested it on MSVC but not MinGW. I hope I did not something wrong.
>>
>>  src/mesa/main/shader_query.cpp |  1 +
>>  src/util/Makefile.sources  |  1 +
>>  src/util/strndup.c | 47 
>> ++
>>  src/util/strndup.h | 28 +
>>  4 files changed, 77 insertions(+)
>>  create mode 100644 src/util/strndup.c
>>  create mode 100644 src/util/strndup.h
>>
>> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
>> index e020dce..0cada50 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -37,6 +37,7 @@
>>  #include "../glsl/program.h"
>>  #include "uniforms.h"
>>  #include "main/enums.h"
>> +#include "util/strndup.h"
>>
>>  extern "C" {
>>  #include "shaderapi.h"
>> diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
>> index afdd0cb..10f2c02 100644
>> --- a/src/util/Makefile.sources
>> +++ b/src/util/Makefile.sources
>> @@ -17,6 +17,7 @@ MESA_UTIL_FILES :=\
>> set.c \
>> set.h \
>> simple_list.h \
>> +   strndup.c \
> Please also add strndup.h.
> 

OK

>> strtod.c \
>> strtod.h \
>> texcompress_rgtc_tmp.h \
>> diff --git a/src/util/strndup.c b/src/util/strndup.c
>> new file mode 100644
>> index 000..6e5bb22
>> --- /dev/null
>> +++ b/src/util/strndup.c
>> @@ -0,0 +1,47 @@
>> +/*
>> + * Copyright (c) 2015 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
>> DEALINGS
>> + * IN THE SOFTWARE.
>> + */
>> +
>> +#if defined(_WIN32)
>> +#include 
>> +#include "strndup.h"
>> +
>> +char *
>> +strndup(const char *str, size_t max)
>> +{
>> +   size_t n;
>> +   char *ptr;
>> +
>> +   if (str == NULL)
>> +  return NULL;
>> +
>> +   n = strlen(str);
>> +   if (n > max)
>> +  n = max;
>> +
>> +   ptr = (char *) malloc((n + 1) * sizeof(char));
> Personally I would use calloc and (must) check if allocation fails.
> The former will handle the (extremely unlikely) overflow, plus you can
> drop the explicit null character. Come to think of it one can even
> inline this into the header.
> 

OK, I'll use calloc here, check if allocation fails and delete the null
character at the end of the string.

Thanks,

Sam

> It's up-to you really, just my 2c.
> -Emil
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 25/24] glsl: add std140 layout support for AoA

2015-09-29 Thread Timothy Arceri

On Thu, 2015-09-24 at 11:31 +0200, Samuel Iglesias Gonsálvez wrote:
> 
> On 24/09/15 02:16, Timothy Arceri wrote:
> > On Wed, 2015-09-23 at 13:21 +0200, Samuel Iglesias Gonsálvez wrote:
> > > On 22/09/15 15:30, Samuel Iglesias Gonsálvez wrote:
> > > > Reviewed-by: Samuel Iglesias Gonsálvez 
> > > > 
> > > 
> > > Forgot to say that we need to implement a similar patch for
> > > std430_size() function, right?
> > 
> > Yeah I think so.
> > 
> > > 
> > > I will write a follow-up patch adding std430 support for AoA.
> > 
> > That would be great thanks.
> > 
> > I assume there will also be other places in the SSBO series that
> > might
> > need updates for AoA?
> > 
> 
> As I am not very familiar with AoA, I will need to check how AoA
> affects
> to SSBO-related code.
> 
> I plan to look at it soon and send follow-up patches (if needed). I
> might ask you to review my patches as you have implemented AoA.

Hi Samuel,

Have you started looking at this yet?

If not I might start looking at it tomorrow. I'm hoping its a case of
things mostly just working but at the least we may need a few extra
piglit tests.

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

Re: [Mesa-dev] SSBO's in UniformBlocks list?

2015-09-29 Thread Iago Toral

Hi ilia,

On Tue, 2015-09-29 at 03:53 -0400, Ilia Mirkin wrote:
> Hi Samuel, and any other onlookers,
> 
> I was wondering why the decision was made to stick SSBO's onto the
> same list as constbufs. Seems like they're entirely separate entities,
> no? Perhaps I'm missing something?

The reason for this was that there is a lot of code in the compiler to
handle uniform blocks and all the rules for them and we needed the same
treatment for SSBOs, so that seemed like a reasonable way forward to
reuse a lot of the code in the compiler front end. I think the only
place where we needed to make explicit distinctions is when we check for
resource limits, since these are different for UBOs and SSBOs of course.
Although UBOs and SSBOs are separate entities they have a lot of
similarities too, so that did not look like a terrible idea, considering
the benefits.

If I remember correctly, Jordan suggested that we might want to change
the names of the structures/files involved so they do not refer to UBOs
explicitly and use something more generic instead, but that would be a
very large change affecting the compiler (and all the drivers) and we
thought it would be best to do that at some other moment, after the
series landed, to avoid being stuck in rebase hell for months.

Iago

> Thanks,
> 
>   -ilia
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Mesa-dev] [PATCH v2] nir: split SSBO min/max atomic instrinsics into signed/unsigned versions

2015-09-29 Thread Iago Toral Quiroga

NIR is typeless so this is the only way to keep track of the
type to select the proper atomic to use.

v2:
  - Use imin,imax,umin,umax for the intrinsic names (Connor Abbott)
  - Change message for unreachable paths (Michael Schellenberger)
---
 src/glsl/nir/glsl_to_nir.cpp   | 22 ++
 src/glsl/nir/nir_intrinsics.h  |  6 --
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp   | 20 ++--
 src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 22 +++---
 4 files changed, 43 insertions(+), 27 deletions(-)

diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
index f03a107..25f41a7 100644
--- a/src/glsl/nir/glsl_to_nir.cpp
+++ b/src/glsl/nir/glsl_to_nir.cpp
@@ -662,9 +662,21 @@ nir_visitor::visit(ir_call *ir)
   } else if (strcmp(ir->callee_name(), 
"__intrinsic_ssbo_atomic_xor_internal") == 0) {
  op = nir_intrinsic_ssbo_atomic_xor;
   } else if (strcmp(ir->callee_name(), 
"__intrinsic_ssbo_atomic_min_internal") == 0) {
- op = nir_intrinsic_ssbo_atomic_min;
+ assert(ir->return_deref);
+ if (ir->return_deref->type == glsl_type::int_type)
+op = nir_intrinsic_ssbo_atomic_imin;
+ else if (ir->return_deref->type == glsl_type::uint_type)
+op = nir_intrinsic_ssbo_atomic_umin;
+ else
+unreachable("Invalid type");
   } else if (strcmp(ir->callee_name(), 
"__intrinsic_ssbo_atomic_max_internal") == 0) {
- op = nir_intrinsic_ssbo_atomic_max;
+ assert(ir->return_deref);
+ if (ir->return_deref->type == glsl_type::int_type)
+op = nir_intrinsic_ssbo_atomic_imax;
+ else if (ir->return_deref->type == glsl_type::uint_type)
+op = nir_intrinsic_ssbo_atomic_umax;
+ else
+unreachable("Invalid type");
   } else if (strcmp(ir->callee_name(), 
"__intrinsic_ssbo_atomic_exchange_internal") == 0) {
  op = nir_intrinsic_ssbo_atomic_exchange;
   } else if (strcmp(ir->callee_name(), 
"__intrinsic_ssbo_atomic_comp_swap_internal") == 0) {
@@ -874,8 +886,10 @@ nir_visitor::visit(ir_call *ir)
  break;
   }
   case nir_intrinsic_ssbo_atomic_add:
-  case nir_intrinsic_ssbo_atomic_min:
-  case nir_intrinsic_ssbo_atomic_max:
+  case nir_intrinsic_ssbo_atomic_imin:
+  case nir_intrinsic_ssbo_atomic_umin:
+  case nir_intrinsic_ssbo_atomic_imax:
+  case nir_intrinsic_ssbo_atomic_umax:
   case nir_intrinsic_ssbo_atomic_and:
   case nir_intrinsic_ssbo_atomic_or:
   case nir_intrinsic_ssbo_atomic_xor:
diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
index 06f1b02..ff42bb2 100644
--- a/src/glsl/nir/nir_intrinsics.h
+++ b/src/glsl/nir/nir_intrinsics.h
@@ -174,8 +174,10 @@ INTRINSIC(image_samples, 0, ARR(), true, 1, 1, 0,
  * 3: For CompSwap only: the second data parameter.
  */
 INTRINSIC(ssbo_atomic_add, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
-INTRINSIC(ssbo_atomic_min, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
-INTRINSIC(ssbo_atomic_max, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
+INTRINSIC(ssbo_atomic_imin, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
+INTRINSIC(ssbo_atomic_umin, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
+INTRINSIC(ssbo_atomic_imax, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
+INTRINSIC(ssbo_atomic_umax, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
 INTRINSIC(ssbo_atomic_and, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
 INTRINSIC(ssbo_atomic_or, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
 INTRINSIC(ssbo_atomic_xor, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index a2bc5c6..0b9555c 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -1870,17 +1870,17 @@ fs_visitor::nir_emit_intrinsic(const fs_builder , 
nir_intrinsic_instr *instr
case nir_intrinsic_ssbo_atomic_add:
   nir_emit_ssbo_atomic(bld, BRW_AOP_ADD, instr);
   break;
-   case nir_intrinsic_ssbo_atomic_min:
-  if (dest.type == BRW_REGISTER_TYPE_D)
- nir_emit_ssbo_atomic(bld, BRW_AOP_IMIN, instr);
-  else
- nir_emit_ssbo_atomic(bld, BRW_AOP_UMIN, instr);
+   case nir_intrinsic_ssbo_atomic_imin:
+  nir_emit_ssbo_atomic(bld, BRW_AOP_IMIN, instr);
   break;
-   case nir_intrinsic_ssbo_atomic_max:
-  if (dest.type == BRW_REGISTER_TYPE_D)
- nir_emit_ssbo_atomic(bld, BRW_AOP_IMAX, instr);
-  else
- nir_emit_ssbo_atomic(bld, BRW_AOP_UMAX, instr);
+   case nir_intrinsic_ssbo_atomic_umin:
+  nir_emit_ssbo_atomic(bld, BRW_AOP_UMIN, instr);
+  break;
+   case nir_intrinsic_ssbo_atomic_imax:
+  nir_emit_ssbo_atomic(bld, BRW_AOP_IMAX, instr);
+  break;
+   case nir_intrinsic_ssbo_atomic_umax:
+  nir_emit_ssbo_atomic(bld, BRW_AOP_UMAX, instr);
   break;
case nir_intrinsic_ssbo_atomic_and:
   nir_emit_ssbo_atomic(bld, BRW_AOP_AND, instr);
diff --git

Re: [Mesa-dev] [PATCH] util: implement strndup for WIN32

2015-09-29 Thread Samuel Iglesias Gonsálvez



On 28/09/15 21:44, Jose Fonseca wrote:
> Looks great Samuel.
> 
> Just minor suggestions.
> 
> Either way:
> 
> Reviewed-by: Jose Fonseca 
> 

Thanks!

> 
> On 28/09/15 15:19, Samuel Iglesias Gonsalvez wrote:
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92124
>> Cc: Jose Fonseca 
>> ---
>>
>> I tested it on MSVC but not MinGW. I hope I did not something wrong.
>>
>>   src/mesa/main/shader_query.cpp |  1 +
>>   src/util/Makefile.sources  |  1 +
>>   src/util/strndup.c | 47
>> ++
>>   src/util/strndup.h | 28 +
>>   4 files changed, 77 insertions(+)
>>   create mode 100644 src/util/strndup.c
>>   create mode 100644 src/util/strndup.h
>>
>> diff --git a/src/mesa/main/shader_query.cpp
>> b/src/mesa/main/shader_query.cpp
>> index e020dce..0cada50 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -37,6 +37,7 @@
>>   #include "../glsl/program.h"
>>   #include "uniforms.h"
>>   #include "main/enums.h"
>> +#include "util/strndup.h"
>>
>>   extern "C" {
>>   #include "shaderapi.h"
>> diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
>> index afdd0cb..10f2c02 100644
>> --- a/src/util/Makefile.sources
>> +++ b/src/util/Makefile.sources
>> @@ -17,6 +17,7 @@ MESA_UTIL_FILES :=\
>>   set.c \
>>   set.h \
>>   simple_list.h \
>> +strndup.c \
>>   strtod.c \
>>   strtod.h \
>>   texcompress_rgtc_tmp.h \
>> diff --git a/src/util/strndup.c b/src/util/strndup.c
>> new file mode 100644
>> index 000..6e5bb22
>> --- /dev/null
>> +++ b/src/util/strndup.c
>> @@ -0,0 +1,47 @@
>> +/*
>> + * Copyright (c) 2015 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> obtaining a
>> + * copy of this software and associated documentation files (the
>> "Software"),
>> + * to deal in the Software without restriction, including without
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including
>> the next
>> + * paragraph) shall be included in all copies or substantial portions
>> of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + */
>> +
>> +#if defined(_WIN32)
>> +#include 
>> +#include "strndup.h"
>> +
>> +char *
>> +strndup(const char *str, size_t max)
>> +{
>> +   size_t n;
>> +   char *ptr;
>> +
>> +   if (str == NULL)
>> +  return NULL;
>> +
>> +   n = strlen(str);
>> +   if (n > max)
>> +  n = max;
>> +
>> +   ptr = (char *) malloc((n + 1) * sizeof(char));
> 
> Probably never happens, but still good idea to add
> 
>   if (!ptr) return NULL;
> 
> just in case
> 

OK, Emil said the same thing. I'll check if the allocation fails.

>> +   memcpy(ptr, str, n);
>> +   ptr[n] = '\0';
>> +   return ptr;
>> +}
>> +
>> +#endif
>> diff --git a/src/util/strndup.h b/src/util/strndup.h
>> new file mode 100644
>> index 000..dc8cdd2
>> --- /dev/null
>> +++ b/src/util/strndup.h
>> @@ -0,0 +1,28 @@
>> +/*
>> + * Copyright (c) 2015 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> obtaining a
>> + * copy of this software and associated documentation files (the
>> "Software"),
>> + * to deal in the Software without restriction, including without
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including
>> the next
>> + * paragraph) shall be included in all copies or substantial portions
>> of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE

Re: [Mesa-dev] [PATCH 25/24] glsl: add std140 layout support for AoA

2015-09-29 Thread Samuel Iglesias Gonsálvez



On 29/09/15 08:34, Timothy Arceri wrote:
> On Thu, 2015-09-24 at 11:31 +0200, Samuel Iglesias Gonsálvez wrote:
>>
>> On 24/09/15 02:16, Timothy Arceri wrote:
>>> On Wed, 2015-09-23 at 13:21 +0200, Samuel Iglesias Gonsálvez wrote:
 On 22/09/15 15:30, Samuel Iglesias Gonsálvez wrote:
> Reviewed-by: Samuel Iglesias Gonsálvez 
>

 Forgot to say that we need to implement a similar patch for
 std430_size() function, right?
>>>
>>> Yeah I think so.
>>>

 I will write a follow-up patch adding std430 support for AoA.
>>>
>>> That would be great thanks.
>>>
>>> I assume there will also be other places in the SSBO series that
>>> might
>>> need updates for AoA?
>>>
>>
>> As I am not very familiar with AoA, I will need to check how AoA
>> affects
>> to SSBO-related code.
>>
>> I plan to look at it soon and send follow-up patches (if needed). I
>> might ask you to review my patches as you have implemented AoA.
> 
> Hi Samuel,
> 
> Have you started looking at this yet?

Yes, I have a couple of patches that seem to work. Changes seem to be
trivial to SSBO load/stores/unsized-array-length-calculation but I would
like to test with complex variable types (matNxM, structures...).

I plan to do it along today.

> 
> If not I might start looking at it tomorrow. I'm hoping its a case of
> things mostly just working but at the least we may need a few extra
> piglit tests.
> 

OK, my idea is to send the patches today afternoon or tomorrow.

Thanks!

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

Re: [Mesa-dev] [PATCH V6 10/27] i965: fix atomic buffer index for bindings other than 0

2015-09-29 Thread Timothy Arceri

On Tue, 2015-09-29 at 02:33 -0400, Ilia Mirkin wrote:
> On Tue, Sep 29, 2015 at 2:26 AM, Timothy Arceri <
> t_arc...@yahoo.com.au> wrote:
> > On Tue, 2015-09-29 at 02:08 -0400, Ilia Mirkin wrote:
> > > On Tue, Sep 29, 2015 at 2:05 AM, Timothy Arceri <
> > > t_arc...@yahoo.com.au> wrote:
> > > > On Tue, 2015-09-29 at 01:05 -0400, Ilia Mirkin wrote:
> > > > > On Mon, Sep 28, 2015 at 10:42 PM, Timothy Arceri <
> > > > > t_arc...@yahoo.com.au> wrote:
> > > > > > Since commit c0cd5b var->data.binding was being used as a
> > > > > > replacement
> > > > > > for atomic buffer index, but they don't have to be the same
> > > > > > value
> > > > > > they
> > > > > > just happen to end up the same when binding is 0.
> > > > > > 
> > > > > > Now that we store the atomic uniform location in var
> > > > > > ->data.location
> > > > > > we can use this to lookup the atomic buffer index in
> > > > > > uniform
> > > > > > storage.
> > > > > > 
> > > > > > For arrays of arrays the outer arrays have separate uniform
> > > > > > locations
> > > > > > however they all share the same buffer so we can get the
> > > > > > buffer
> > > > > > index
> > > > > > using the base uniform location.
> > > > > > 
> > > > > > Cc: Alejandro Piñeiro 
> > > > > > Bugzilla: 
> > > > > > https://bugs.freedesktop.org/show_bug.cgi?id=90175
> > > > > > ---
> > > > > >  src/glsl/nir/glsl_to_nir.cpp   |  2 --
> > > > > >  src/glsl/nir/nir.h |  4 ++--
> > > > > >  src/glsl/nir/nir_lower_atomics.c   | 18
> > > > > > --
> > > > > > 
> > > > > >  src/mesa/drivers/dri/i965/brw_nir.c|  6 --
> > > > > >  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |  2 +-
> > > > > >  5 files changed, 19 insertions(+), 13 deletions(-)
> > > > > > 
> > > > > > diff --git a/src/glsl/nir/glsl_to_nir.cpp
> > > > > > b/src/glsl/nir/glsl_to_nir.cpp
> > > > > > index f03a107..30726be 100644
> > > > > > --- a/src/glsl/nir/glsl_to_nir.cpp
> > > > > > +++ b/src/glsl/nir/glsl_to_nir.cpp
> > > > > > @@ -330,8 +330,6 @@ nir_visitor::visit(ir_variable *ir)
> > > > > > 
> > > > > > var->data.index = ir->data.index;
> > > > > > var->data.binding = ir->data.binding;
> > > > > > -   /* XXX Get rid of buffer_index */
> > > > > > -   var->data.atomic.buffer_index = ir->data.binding;
> > > > > > var->data.atomic.offset = ir->data.atomic.offset;
> > > > > > var->data.image.read_only = ir->data.image_read_only;
> > > > > > var->data.image.write_only = ir->data.image_write_only;
> > > > > > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> > > > > > index 4f45770..670e39e 100644
> > > > > > --- a/src/glsl/nir/nir.h
> > > > > > +++ b/src/glsl/nir/nir.h
> > > > > > @@ -308,7 +308,6 @@ typedef struct {
> > > > > > * Location an atomic counter is stored at.
> > > > > > */
> > > > > >struct {
> > > > > > - unsigned buffer_index;
> > > > > >   unsigned offset;
> > > > > >} atomic;
> > > > > > 
> > > > > > @@ -1880,7 +1879,8 @@ void nir_lower_clip_fs(nir_shader
> > > > > > *shader,
> > > > > > unsigned ucp_enables);
> > > > > > 
> > > > > >  void nir_lower_two_sided_color(nir_shader *shader);
> > > > > > 
> > > > > > -void nir_lower_atomics(nir_shader *shader);
> > > > > > +void nir_lower_atomics(nir_shader *shader,
> > > > > > +   const struct gl_shader_program
> > > > > > *shader_program);
> > > > > >  void nir_lower_to_source_mods(nir_shader *shader);
> > > > > > 
> > > > > >  bool nir_lower_gs_intrinsics(nir_shader *shader);
> > > > > > diff --git a/src/glsl/nir/nir_lower_atomics.c
> > > > > > b/src/glsl/nir/nir_lower_atomics.c
> > > > > > index 46e1376..52e7675 100644
> > > > > > --- a/src/glsl/nir/nir_lower_atomics.c
> > > > > > +++ b/src/glsl/nir/nir_lower_atomics.c
> > > > > > @@ -25,6 +25,7 @@
> > > > > >   *
> > > > > >   */
> > > > > > 
> > > > > > +#include "ir_uniform.h"
> > > > > >  #include "nir.h"
> > > > > >  #include "main/config.h"
> > > > > >  #include 
> > > > > > @@ -35,7 +36,8 @@
> > > > > >   */
> > > > > > 
> > > > > >  static void
> > > > > > -lower_instr(nir_intrinsic_instr *instr, nir_function_impl
> > > > > > *impl)
> > > > > > +lower_instr(nir_intrinsic_instr *instr,
> > > > > > +const struct gl_shader_program
> > > > > > *shader_program)
> > > > > >  {
> > > > > > nir_intrinsic_op op;
> > > > > > switch (instr->intrinsic) {
> > > > > > @@ -60,10 +62,11 @@ lower_instr(nir_intrinsic_instr *instr,
> > > > > > nir_function_impl *impl)
> > > > > >return; /* atomics passed as function arguments
> > > > > > can't be
> > > > > > lowered */
> > > > > > 
> > > > > > void *mem_ctx = ralloc_parent(instr);
> > > > > > +   unsigned uniform_loc = instr->variables[0]->var
> > > > > > ->data.location;
> > > > > > 
> > > > > > nir_intrinsic_instr *new_instr =
> > > > > > nir_intrinsic_instr_create(mem_ctx, op);
> > > > > > new_instr->const_index[0] =

Re: [Mesa-dev] [PATCH] nir: Don't set dest in SSBO store glsl_to_nir conversion

2015-09-29 Thread Samuel Iglesias Gonsálvez



On 28/09/15 23:47, Jordan Justen wrote:
> This matches the function signature created in
> lower_ubo_reference_visitor::ssbo_store which has a void return.

True! Good catch.

Reviewed-by: Samuel Iglesias Gonsálvez 

Thanks,

Sam

> 
> Suggested-by: Jason Ekstrand 
> Signed-off-by: Jordan Justen 
> Cc: Samuel Iglesias Gonsalvez 
> Cc: Jason Ekstrand 
> ---
>  src/glsl/nir/glsl_to_nir.cpp | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
> index cb379ad..6132b82 100644
> --- a/src/glsl/nir/glsl_to_nir.cpp
> +++ b/src/glsl/nir/glsl_to_nir.cpp
> @@ -799,7 +799,6 @@ nir_visitor::visit(ir_call *ir)
>  instr = nir_intrinsic_instr_create(shader, op);
>  instr->src[2] = evaluate_rvalue(offset);
>  instr->const_index[0] = 0;
> -dest = >dest;
>   } else {
>  instr->const_index[0] = const_offset->value.u[0];
>   }
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] egl/dri2: don't require a context for ClientWaitSync

2015-09-29 Thread Albert Freeman

On 28 September 2015 at 08:10, Marek Olšák  wrote:
> On Mon, Sep 28, 2015 at 7:20 AM, Albert Freeman
>  wrote:
>> On 28 September 2015 at 14:38, Albert Freeman  
>> wrote:
>>> On 28 September 2015 at 04:12, Emil Velikov  
>>> wrote:
 On 26 September 2015 at 00:49, Marek Olšák  wrote:
> From: Marek Olšák 
>
> The spec doesn't require it. This fixes a crash on Android.
>
 Nicely spotted Marek.

 A couple of related (not supposed to be part of this patch) notes:
  - This will just move the crash (from libEGL to i965_dri.so) for i965
 hardware :)
>>> I rediscovered this before I read the email related to this patch
>>> (where Marek mentions it): "Re: [Mesa-dev] Pending issues of
>>> lollipop-x86".
>>> Who (if anyone) should we CC this to?
  - We really shouldn't be setting the flags if ctx is NULL, as per the 
 spec.

 "If no context is
 current for the bound API, the EGL_SYNC_FLUSH_COMMANDS_BIT_KHR bit
 is ignored."
>>> Maybe handle this in the driver (probably state_tracker in the case of
>>> gallium) level. Both here and drivers?
>> OpenGL ClientWaitSync is very clear about SYNC_FLUSH_COMMANDS_BIT only
>> flushing the GL context when the fence sync is created (it doesn’t
>> seem to require it at the moment wait sync is called (probably to
>> leave room for drivers to optimise)). EGL is a lot less clear, it
>> could be interpreted as behaving just like GL, but to someone writing
>> an EGL program it could be easily interpreted as "the flush() must
>> occur when eglClientWaitSync is called with SYNC_FLUSH_COMMANDS_BIT".
>>
>> The current behaviour of gallium (not sure about i965) is to always
>> flush() when creating the sync object (hence always behave as
>> SYNC_FLUSH_COMMANDS_BIT is set according to GL). However
>> implementations are allowed to behave as if flush was not required (in
>> GL and I am assuming EGL). So technically the current behaviour of the
>> gallium dri state tracker could reduce performance (since flushing
>> tends to do that). It however doesn’t actually violate the EGL spec.
>> Perhaps some heuristics on when to flush would be a good solution?
>
> Flushing is currently required for creating fences. This is a gallium
> design decision as well as radeon/amdgpu kernel driver decision. It's
> simply not possible to put a fence anywhere in the GPU command buffer.
> We can only submit a command buffer to the kernel and then get a fence
> tracking execution of the whole command buffer.
>
> Marek
Thanks for the clarification.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH V6 10/27] i965: fix atomic buffer index for bindings other than 0

2015-09-29 Thread Ilia Mirkin

On Tue, Sep 29, 2015 at 3:32 AM, Timothy Arceri  wrote:
> On Tue, 2015-09-29 at 02:55 -0400, Ilia Mirkin wrote:
>> On Tue, Sep 29, 2015 at 2:48 AM, Timothy Arceri <
>> t_arc...@yahoo.com.au> wrote:
>> > On Tue, 2015-09-29 at 02:33 -0400, Ilia Mirkin wrote:
>> > > On Tue, Sep 29, 2015 at 2:26 AM, Timothy Arceri <
>> > > t_arc...@yahoo.com.au> wrote:
>> > > > On Tue, 2015-09-29 at 02:08 -0400, Ilia Mirkin wrote:
>> > > > > On Tue, Sep 29, 2015 at 2:05 AM, Timothy Arceri <
>> > > > > t_arc...@yahoo.com.au> wrote:
>> > > > > > On Tue, 2015-09-29 at 01:05 -0400, Ilia Mirkin wrote:
>> > > > > No. I mean the line:
>> > > > >
>> > > > >   brw->vtbl.emit_buffer_surface_state(brw,
>> > > > > _offsets[i],
>> > > > > bo,
>> > > > >
>> > > > > Shouldn't that look up the atomic buffer index in prog
>> > > > > ->Uniforms
>> > > > > instead of using 'i'?
>> > > >
>> > > > No that looks fine. The problem in the patch was we didn't have
>> > > > the
>> > > > atomic buffex index and we used the binding as the offset which
>> > > > doesn't
>> > > > have to be the same as the buffer index.
>> > > >
>> > > > In brw_upload_abo_surfaces its the opposite we have the atomic
>> > > > buffer
>> > > > index which is i and we are looking up the binding to get the
>> > > > buffer
>> > > > object.
>> > > >
>> > > > Does that make sense?
>> > >
>> > > i is the binding index though...
>> >
>> > i is the buffer index
>> >
>> > See find_active_atomic_counters() for the code the counts the
>> > active
>> > buffers.
>> >
>> > http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_atomics.cp
>> > p#n9
>> > 9
>> >
>> > This count is then used to set NumAtomicBuffers and to create the
>> > prog
>> > ->AtomicBuffers array so the array is no bigger than it needs to
>> > be.
>> > This is the reason the binding and atomic buffer index can differ.
>> >
>> > http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_atomics.cp
>> > p#n1
>> > 73
>>
>> Do you agree that NumAtomicBuffers can go up to
>> MaxCombinedAtomicBuffers? If so, that's problematic, when it's >
>> MaxAtomicBuffers, right?
>
> Ok, so now we are talking about a new problem from what this patch is
> trying to solve.

Well, the problem is "the indexes are all messed up". I think this
patch fixes half the problem in a way that happens to work some of the
time, much like the existing logic. (Admittedly your solution works
*more* of the time...)

>
> I agree it can go up to MaxCombinedAtomicBuffers.
>
> I'm not sure what the answer is, maybe a question for curro. I don't
> know enough about this code to know if we want to upload everything
> here or just do a stage at a time.

We need to keep track of an intra-stage index, accessible both at
instruction emission time as well as at resource emission time. I
thought that was the atomic_buffer_index. Sounds like it's not, but in
that case it should be made to be that IMO. See
_mesa_get_sampler_uniform_value for how this is handled for
samplers... not sure if it'll be helpful or not.

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

Re: [Mesa-dev] [PATCH] i965/vec4/nir: add nir_intrinsic_memory_barrier support

2015-09-29 Thread Samuel Iglesias Gonsálvez

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256



On 28/09/15 17:48, Francisco Jerez wrote:
> Samuel Iglesias Gonsalvez  writes:
> 
>> Fix OpenGL ES 3.1 conformance tests:
>> advanced-readWrite-case1-vsfs and advanced-matrix-vsfs.
>> 
>> Signed-off-by: Samuel Iglesias Gonsalvez  
>> Tested-by: Tapani Pälli  Cc: Francisco
>> Jerez  --- 
>> src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 9 + 1 file
>> changed, 9 insertions(+)
>> 
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp index
>> 175d92b..5c418fb 100644 ---
>> a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp +++
>> b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp @@ -654,6 +654,15 @@
>> vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr) 
>> break; }
>> 
>> +   case nir_intrinsic_memory_barrier: { +  const
>> vec4_builder bld = +
>> vec4_builder(this).at_end().annotate(current_annotation,
>> base_ir); +  const src_reg tmp = src_reg(this,
>> glsl_type::uint_type);
> 
> The temporary register you pass to the MEMORY_FENCE instruction
> should be at least regs_written VGRFs, you could use 
> 'bld.vgrf(BRW_REGISTER_TYPE_UD, 2)' instead.
> 

OK

>> +  bld.emit(SHADER_OPCODE_MEMORY_FENCE, dest, tmp)
> 
> MEMORY_FENCE only takes a destination (which should be tmp rather
> than dest) and no source registers.
> 

OK.

Thanks,

Sam

>> + ->regs_written = 2; +  break; +   } + default: 
>> unreachable("Unknown intrinsic"); } -- 2.1.4
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQIcBAEBCAAGBQJWCjT1AAoJEH/0ujLxfcNDrqMQALov9lvW//2nCmlpqLWy5RkI
mJPgoOlBKdYiPOYDflgpZm2GlfX2TtOV/QAileQcKjQnOt+rhL5fGgCipR3L1zDE
Ka1F2NxrUVi3tY/I/D4oK4NCMp3DdI7J1epV1fxKdl9F+9lW79kuW8Kc9Y+fFqcf
WyghqGn4UscLgPLCZUm0D7gVPW5yZ22vG0DmNQauQNdrx4fWKNolc/ZL1SR+Nbxp
3DcfP8+vOtL8a7/6lpCprZPIxMc1nsc4HajfIYVvPFWI1K26q03vtahIdQWq2knc
3Z6oE+92aMBWC9yMePS2ssyPvGt1/BcWWDpY0+03zci39uS4UgMyAec3WgdFoMJU
TrdWeae+qsNnuiz02U1UVuROqd7ucDusdJSfsWjwS1/tfdZbsXBoiuXY/uUkgSCB
E48ImjC+sSgDEszWxIfbWYRxUk6YlMxeywl8F+RTL/gE/+yhTz2uxGzbpMX6HxzH
efqAUe+vLk6HvfBgYfQOGZujZbh1xMUJB+wIRAf2x6yuKVC2fWXDFIzBIZkkeayw
Wz95sUYZF+5CS6F31lwitA4C5koCEx9hwToTTiyPiiLgl0Xq/OHgVTQHLjtjJ1Jq
dLaXsGyYUjhFoDOBNGoBXIdGRt1NfB3iPEWpqyn1175Okdq6wUtXmVSkY5bu6kdr
cxkqYDzWLMwl2L9JFIUJ
=x/5U
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH V6 10/27] i965: fix atomic buffer index for bindings other than 0

2015-09-29 Thread Ilia Mirkin

On Tue, Sep 29, 2015 at 2:05 AM, Timothy Arceri  wrote:
> On Tue, 2015-09-29 at 01:05 -0400, Ilia Mirkin wrote:
>> On Mon, Sep 28, 2015 at 10:42 PM, Timothy Arceri <
>> t_arc...@yahoo.com.au> wrote:
>> > Since commit c0cd5b var->data.binding was being used as a
>> > replacement
>> > for atomic buffer index, but they don't have to be the same value
>> > they
>> > just happen to end up the same when binding is 0.
>> >
>> > Now that we store the atomic uniform location in var->data.location
>> > we can use this to lookup the atomic buffer index in uniform
>> > storage.
>> >
>> > For arrays of arrays the outer arrays have separate uniform
>> > locations
>> > however they all share the same buffer so we can get the buffer
>> > index
>> > using the base uniform location.
>> >
>> > Cc: Alejandro Piñeiro 
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90175
>> > ---
>> >  src/glsl/nir/glsl_to_nir.cpp   |  2 --
>> >  src/glsl/nir/nir.h |  4 ++--
>> >  src/glsl/nir/nir_lower_atomics.c   | 18 --
>> > 
>> >  src/mesa/drivers/dri/i965/brw_nir.c|  6 --
>> >  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |  2 +-
>> >  5 files changed, 19 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/src/glsl/nir/glsl_to_nir.cpp
>> > b/src/glsl/nir/glsl_to_nir.cpp
>> > index f03a107..30726be 100644
>> > --- a/src/glsl/nir/glsl_to_nir.cpp
>> > +++ b/src/glsl/nir/glsl_to_nir.cpp
>> > @@ -330,8 +330,6 @@ nir_visitor::visit(ir_variable *ir)
>> >
>> > var->data.index = ir->data.index;
>> > var->data.binding = ir->data.binding;
>> > -   /* XXX Get rid of buffer_index */
>> > -   var->data.atomic.buffer_index = ir->data.binding;
>> > var->data.atomic.offset = ir->data.atomic.offset;
>> > var->data.image.read_only = ir->data.image_read_only;
>> > var->data.image.write_only = ir->data.image_write_only;
>> > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
>> > index 4f45770..670e39e 100644
>> > --- a/src/glsl/nir/nir.h
>> > +++ b/src/glsl/nir/nir.h
>> > @@ -308,7 +308,6 @@ typedef struct {
>> > * Location an atomic counter is stored at.
>> > */
>> >struct {
>> > - unsigned buffer_index;
>> >   unsigned offset;
>> >} atomic;
>> >
>> > @@ -1880,7 +1879,8 @@ void nir_lower_clip_fs(nir_shader *shader,
>> > unsigned ucp_enables);
>> >
>> >  void nir_lower_two_sided_color(nir_shader *shader);
>> >
>> > -void nir_lower_atomics(nir_shader *shader);
>> > +void nir_lower_atomics(nir_shader *shader,
>> > +   const struct gl_shader_program
>> > *shader_program);
>> >  void nir_lower_to_source_mods(nir_shader *shader);
>> >
>> >  bool nir_lower_gs_intrinsics(nir_shader *shader);
>> > diff --git a/src/glsl/nir/nir_lower_atomics.c
>> > b/src/glsl/nir/nir_lower_atomics.c
>> > index 46e1376..52e7675 100644
>> > --- a/src/glsl/nir/nir_lower_atomics.c
>> > +++ b/src/glsl/nir/nir_lower_atomics.c
>> > @@ -25,6 +25,7 @@
>> >   *
>> >   */
>> >
>> > +#include "ir_uniform.h"
>> >  #include "nir.h"
>> >  #include "main/config.h"
>> >  #include 
>> > @@ -35,7 +36,8 @@
>> >   */
>> >
>> >  static void
>> > -lower_instr(nir_intrinsic_instr *instr, nir_function_impl *impl)
>> > +lower_instr(nir_intrinsic_instr *instr,
>> > +const struct gl_shader_program *shader_program)
>> >  {
>> > nir_intrinsic_op op;
>> > switch (instr->intrinsic) {
>> > @@ -60,10 +62,11 @@ lower_instr(nir_intrinsic_instr *instr,
>> > nir_function_impl *impl)
>> >return; /* atomics passed as function arguments can't be
>> > lowered */
>> >
>> > void *mem_ctx = ralloc_parent(instr);
>> > +   unsigned uniform_loc = instr->variables[0]->var->data.location;
>> >
>> > nir_intrinsic_instr *new_instr =
>> > nir_intrinsic_instr_create(mem_ctx, op);
>> > new_instr->const_index[0] =
>> > -  (int) instr->variables[0]->var->data.atomic.buffer_index;
>> > +  shader_program
>> > ->UniformStorage[uniform_loc].atomic_buffer_index;
>> >
>> > nir_load_const_instr *offset_const =
>> > nir_load_const_instr_create(mem_ctx, 1);
>> > offset_const->value.u[0] = instr->variables[0]->var
>> > ->data.atomic.offset;
>> > @@ -128,22 +131,25 @@ lower_instr(nir_intrinsic_instr *instr,
>> > nir_function_impl *impl)
>> >  }
>> >
>> >  static bool
>> > -lower_block(nir_block *block, void *state)
>> > +lower_block(nir_block *block, void *prog)
>> >  {
>> > nir_foreach_instr_safe(block, instr) {
>> >if (instr->type == nir_instr_type_intrinsic)
>> > - lower_instr(nir_instr_as_intrinsic(instr), state);
>> > + lower_instr(nir_instr_as_intrinsic(instr),
>> > + (const struct gl_shader_program *) prog);
>> > }
>> >
>> > return true;
>> >  }
>> >
>> >  void
>> > -nir_lower_atomics(nir_shader *shader)
>> > +nir_lower_atomics(nir_shader *shader,
>> > +  const struct

Re: [Mesa-dev] [PATCH V6 10/27] i965: fix atomic buffer index for bindings other than 0

2015-09-29 Thread Timothy Arceri

On Tue, 2015-09-29 at 02:08 -0400, Ilia Mirkin wrote:
> On Tue, Sep 29, 2015 at 2:05 AM, Timothy Arceri <
> t_arc...@yahoo.com.au> wrote:
> > On Tue, 2015-09-29 at 01:05 -0400, Ilia Mirkin wrote:
> > > On Mon, Sep 28, 2015 at 10:42 PM, Timothy Arceri <
> > > t_arc...@yahoo.com.au> wrote:
> > > > Since commit c0cd5b var->data.binding was being used as a
> > > > replacement
> > > > for atomic buffer index, but they don't have to be the same
> > > > value
> > > > they
> > > > just happen to end up the same when binding is 0.
> > > > 
> > > > Now that we store the atomic uniform location in var
> > > > ->data.location
> > > > we can use this to lookup the atomic buffer index in uniform
> > > > storage.
> > > > 
> > > > For arrays of arrays the outer arrays have separate uniform
> > > > locations
> > > > however they all share the same buffer so we can get the buffer
> > > > index
> > > > using the base uniform location.
> > > > 
> > > > Cc: Alejandro Piñeiro 
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90175
> > > > ---
> > > >  src/glsl/nir/glsl_to_nir.cpp   |  2 --
> > > >  src/glsl/nir/nir.h |  4 ++--
> > > >  src/glsl/nir/nir_lower_atomics.c   | 18
> > > > --
> > > > 
> > > >  src/mesa/drivers/dri/i965/brw_nir.c|  6 --
> > > >  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |  2 +-
> > > >  5 files changed, 19 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/src/glsl/nir/glsl_to_nir.cpp
> > > > b/src/glsl/nir/glsl_to_nir.cpp
> > > > index f03a107..30726be 100644
> > > > --- a/src/glsl/nir/glsl_to_nir.cpp
> > > > +++ b/src/glsl/nir/glsl_to_nir.cpp
> > > > @@ -330,8 +330,6 @@ nir_visitor::visit(ir_variable *ir)
> > > > 
> > > > var->data.index = ir->data.index;
> > > > var->data.binding = ir->data.binding;
> > > > -   /* XXX Get rid of buffer_index */
> > > > -   var->data.atomic.buffer_index = ir->data.binding;
> > > > var->data.atomic.offset = ir->data.atomic.offset;
> > > > var->data.image.read_only = ir->data.image_read_only;
> > > > var->data.image.write_only = ir->data.image_write_only;
> > > > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> > > > index 4f45770..670e39e 100644
> > > > --- a/src/glsl/nir/nir.h
> > > > +++ b/src/glsl/nir/nir.h
> > > > @@ -308,7 +308,6 @@ typedef struct {
> > > > * Location an atomic counter is stored at.
> > > > */
> > > >struct {
> > > > - unsigned buffer_index;
> > > >   unsigned offset;
> > > >} atomic;
> > > > 
> > > > @@ -1880,7 +1879,8 @@ void nir_lower_clip_fs(nir_shader
> > > > *shader,
> > > > unsigned ucp_enables);
> > > > 
> > > >  void nir_lower_two_sided_color(nir_shader *shader);
> > > > 
> > > > -void nir_lower_atomics(nir_shader *shader);
> > > > +void nir_lower_atomics(nir_shader *shader,
> > > > +   const struct gl_shader_program
> > > > *shader_program);
> > > >  void nir_lower_to_source_mods(nir_shader *shader);
> > > > 
> > > >  bool nir_lower_gs_intrinsics(nir_shader *shader);
> > > > diff --git a/src/glsl/nir/nir_lower_atomics.c
> > > > b/src/glsl/nir/nir_lower_atomics.c
> > > > index 46e1376..52e7675 100644
> > > > --- a/src/glsl/nir/nir_lower_atomics.c
> > > > +++ b/src/glsl/nir/nir_lower_atomics.c
> > > > @@ -25,6 +25,7 @@
> > > >   *
> > > >   */
> > > > 
> > > > +#include "ir_uniform.h"
> > > >  #include "nir.h"
> > > >  #include "main/config.h"
> > > >  #include 
> > > > @@ -35,7 +36,8 @@
> > > >   */
> > > > 
> > > >  static void
> > > > -lower_instr(nir_intrinsic_instr *instr, nir_function_impl
> > > > *impl)
> > > > +lower_instr(nir_intrinsic_instr *instr,
> > > > +const struct gl_shader_program *shader_program)
> > > >  {
> > > > nir_intrinsic_op op;
> > > > switch (instr->intrinsic) {
> > > > @@ -60,10 +62,11 @@ lower_instr(nir_intrinsic_instr *instr,
> > > > nir_function_impl *impl)
> > > >return; /* atomics passed as function arguments can't be
> > > > lowered */
> > > > 
> > > > void *mem_ctx = ralloc_parent(instr);
> > > > +   unsigned uniform_loc = instr->variables[0]->var
> > > > ->data.location;
> > > > 
> > > > nir_intrinsic_instr *new_instr =
> > > > nir_intrinsic_instr_create(mem_ctx, op);
> > > > new_instr->const_index[0] =
> > > > -  (int) instr->variables[0]->var
> > > > ->data.atomic.buffer_index;
> > > > +  shader_program
> > > > ->UniformStorage[uniform_loc].atomic_buffer_index;
> > > > 
> > > > nir_load_const_instr *offset_const =
> > > > nir_load_const_instr_create(mem_ctx, 1);
> > > > offset_const->value.u[0] = instr->variables[0]->var
> > > > ->data.atomic.offset;
> > > > @@ -128,22 +131,25 @@ lower_instr(nir_intrinsic_instr *instr,
> > > > nir_function_impl *impl)
> > > >  }
> > > > 
> > > >  static bool
> > > > -lower_block(nir_block *block, void *state)
> > > > +lower_block(nir_block *block, void *prog)
> > > >  {
> > > >

Re: [Mesa-dev] [PATCH 5/6] glsl: apply shader storage block member rules when adding program resources

2015-09-29 Thread Samuel Iglesias Gonsálvez



On 28/09/15 00:05, Timothy Arceri wrote:
> On Fri, 2015-09-25 at 10:24 +0200, Samuel Iglesias Gonsalvez wrote:
>> From ARB_program_interface_query:
>>
>> "For an active shader storage block member declared as an array, an
>>  entry will be generated only for the first array element, regardless
>>  of its type. For arrays of aggregate types, the enumeration rules
>> are
>>  applied recursively for the single enumerated array element."
>>
>> Signed-off-by: Samuel Iglesias Gonsalvez 
>> ---
>>  src/glsl/linker.cpp | 56
>> +
>>  1 file changed, 56 insertions(+)
>>
>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>> index be04f5b..8cc9350 100644
>> --- a/src/glsl/linker.cpp
>> +++ b/src/glsl/linker.cpp
>> @@ -3134,6 +3134,57 @@ check_explicit_uniform_locations(struct
>> gl_context *ctx,
>>  }
>>  
>>  static bool
>> +should_add_buffer_variable(struct gl_shader_program *shProg,
>> +   GLenum type, const char *name)
>> +{
>> +   bool found_interface = false;
>> +   const char *block_name = NULL;
>> +
>> +   assert(type == GL_BUFFER_VARIABLE);
>> +
>> +   for (unsigned i = 0; i < shProg->NumUniformShaderStorageBlocks;
>> i++) {
>> +  block_name = shProg->UniformBlocks[i].Name;
>> +  if (strncmp(block_name, name, strlen(block_name)) == 0) {
>> + found_interface = true;
>> + break;
>> +  }
>> +   }
>> +
>> +   /* We remove the interface name from the buffer variable name,
>> +* including the dot that follows it.
>> +*/
>> +   if (found_interface)
>> +  name = name + strlen(block_name) + 1;
>> +
>> +   /* From: ARB_program_interface_query extension:
>> +*
>> +*  "For an active shader storage block member declared as an
>> array, an
>> +*   entry will be generated only for the first array element,
>> regardless
>> +*   of its type.  For arrays of aggregate types, the enumeration
>> rules are
>> +*   applied recursively for the single enumerated array element.
>> +*/
>> +   const char *first_dot = strchr(name, '.');
>> +   const char *first_square_bracket = strchr(name, '[');
>> +
>> +   /* The buffer variable is on top level and it is not an array or
>> struct */
>> +   if (!first_square_bracket && !first_dot) {
>> +  return true;
>> +   /* The shader storage block member is a struct, then generate the
>> entry */
>> +   } else if ((!first_square_bracket ||
>> +   (first_dot && first_dot < first_square_bracket))) {
> 
> I think the above can be simplified to:
> 
>if (!first_square_bracket) {
>   return true;
>/* The shader storage block member is a struct, then generate the
> entry */
>} else if (first_dot && first_dot < first_square_bracket)) {
> 

Yes, thanks.

>> +  return true;
>> +   } else {
>> +  /* Shader storage block member is an array, only generate an
>> entry for the
>> +   * first array element.
>> +   */
>> +  if (strncmp(first_square_bracket, "[0]", 3) == 0)
>> + return true;
>> +   }
>> +
>> +   return false;
>> +}
>> +
>> +static bool
>>  add_program_resource(struct gl_shader_program *prog, GLenum type,
>>   const void *data, uint8_t stages)
>>  {
>> @@ -3408,6 +3459,11 @@ build_program_resource_list(struct
>> gl_shader_program *shProg)
>>  
>>bool is_shader_storage =  shProg
>> ->UniformStorage[i].is_shader_storage;
>>GLenum type = is_shader_storage ? GL_BUFFER_VARIABLE :
>> GL_UNIFORM;
>> +  if (is_shader_storage &&
>> +  !should_add_buffer_variable(shProg, type,
>> +  shProg
>> ->UniformStorage[i].name))
> 
> Rather than check is_shader_storage here it would be better to replace
> the assert at the top of should_add_buffer_variable() with:
> 
>if (type != GL_BUFFER_VARIABLE)
>   return false;
> 

Actually, it should return true, as we don't want to skip any uniform. I
will add this code to should_add_buffer_variable() together with a
comment to explain why it returns true.

Sam

>> + continue;
>> +
>>if (!add_program_resource(shProg, type,
>>  >UniformStorage[i],
>> stageref))
>>   return;
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] nir: Use a system value for gl_PrimitiveIDIn.

2015-09-29 Thread Kenneth Graunke

At least on Intel hardware, gl_PrimitiveIDIn comes in as a special part
of the payload rather than a normal input.  This is typically what we
use system values for.  Dave and Ilia also agree that a system value
would be nicer.

At some point, we should change it at the GLSL IR level as well.  But
that requires changing most of the drivers.  For now, let's at least
make NIR do the right thing, which is easy.

Signed-off-by: Kenneth Graunke 
---
 src/glsl/nir/glsl_to_nir.cpp  | 5 +
 src/glsl/nir/nir.c| 5 -
 src/glsl/nir/nir_intrinsics.h | 1 +
 src/glsl/shader_enums.h   | 2 +-
 src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp | 9 +
 5 files changed, 20 insertions(+), 2 deletions(-)

I bypassed most of the system value boilerplate in the backend.  Notably,
this means we just access g1 directly rather than moving it to a VGRF at
the start of the program and using that later.  This means more HW_REG
usage, but it also means less seemingly pointless copies.

I'm hoping to simplify the handling of other system values too, but I'm
waiting until we delete the GLSL IR code paths first.

diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
index f03a107..c0b2634 100644
--- a/src/glsl/nir/glsl_to_nir.cpp
+++ b/src/glsl/nir/glsl_to_nir.cpp
@@ -271,6 +271,11 @@ nir_visitor::visit(ir_variable *ir)
  /* For whatever reason, GLSL IR makes gl_FrontFacing an input */
  var->data.location = SYSTEM_VALUE_FRONT_FACE;
  var->data.mode = nir_var_system_value;
+  } else if (shader->stage == MESA_SHADER_GEOMETRY &&
+ ir->data.location == VARYING_SLOT_PRIMITIVE_ID) {
+ /* For whatever reason, GLSL IR makes gl_PrimitiveIDIn an input */
+ var->data.location = SYSTEM_VALUE_PRIMITIVE_ID;
+ var->data.mode = nir_var_system_value;
   } else {
  var->data.mode = nir_var_shader_in;
   }
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index 1206bb4..7f30b8a 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -1487,10 +1487,11 @@ nir_intrinsic_from_system_value(gl_system_value val)
   return nir_intrinsic_load_local_invocation_id;
case SYSTEM_VALUE_WORK_GROUP_ID:
   return nir_intrinsic_load_work_group_id;
+   case SYSTEM_VALUE_PRIMITIVE_ID:
+  return nir_intrinsic_load_primitive_id;
/* FINISHME: Add tessellation intrinsics.
case SYSTEM_VALUE_TESS_COORD:
case SYSTEM_VALUE_VERTICES_IN:
-   case SYSTEM_VALUE_PRIMITIVE_ID:
case SYSTEM_VALUE_TESS_LEVEL_OUTER:
case SYSTEM_VALUE_TESS_LEVEL_INNER:
 */
@@ -1525,6 +1526,8 @@ nir_system_value_from_intrinsic(nir_intrinsic_op intrin)
   return SYSTEM_VALUE_LOCAL_INVOCATION_ID;
case nir_intrinsic_load_work_group_id:
   return SYSTEM_VALUE_WORK_GROUP_ID;
+   case nir_intrinsic_load_primitive_id:
+  return SYSTEM_VALUE_PRIMITIVE_ID;
/* FINISHME: Add tessellation intrinsics.
   return SYSTEM_VALUE_TESS_COORD;
   return SYSTEM_VALUE_VERTICES_IN;
diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
index 06f1b02..0d93b12 100644
--- a/src/glsl/nir/nir_intrinsics.h
+++ b/src/glsl/nir/nir_intrinsics.h
@@ -194,6 +194,7 @@ SYSTEM_VALUE(instance_id, 1, 0)
 SYSTEM_VALUE(sample_id, 1, 0)
 SYSTEM_VALUE(sample_pos, 2, 0)
 SYSTEM_VALUE(sample_mask_in, 1, 0)
+SYSTEM_VALUE(primitive_id, 1, 0)
 SYSTEM_VALUE(invocation_id, 1, 0)
 SYSTEM_VALUE(local_invocation_id, 3, 0)
 SYSTEM_VALUE(work_group_id, 3, 0)
diff --git a/src/glsl/shader_enums.h b/src/glsl/shader_enums.h
index 3978007..f6b71a3 100644
--- a/src/glsl/shader_enums.h
+++ b/src/glsl/shader_enums.h
@@ -399,7 +399,7 @@ typedef enum
/*@{*/
SYSTEM_VALUE_TESS_COORD,
SYSTEM_VALUE_VERTICES_IN,/**< Tessellation vertices in input patch */
-   SYSTEM_VALUE_PRIMITIVE_ID,   /**< (currently not used by GS) */
+   SYSTEM_VALUE_PRIMITIVE_ID,
SYSTEM_VALUE_TESS_LEVEL_OUTER, /**< TES input */
SYSTEM_VALUE_TESS_LEVEL_INNER, /**< TES input */
/*@}*/
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp
index 4f4e1e1..26766a0 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp
@@ -72,6 +72,9 @@ 
vec4_gs_visitor::nir_setup_system_value_intrinsic(nir_intrinsic_instr *instr)
dst_reg *reg;
 
switch (instr->intrinsic) {
+   case nir_intrinsic_load_primitive_id:
+  break;
+
case nir_intrinsic_load_invocation_id:
   reg = >nir_system_values[SYSTEM_VALUE_INVOCATION_ID];
   if (reg->file == BAD_FILE)
@@ -111,6 +114,12 @@ vec4_gs_visitor::nir_emit_intrinsic(nir_intrinsic_instr 
*instr)
  retype(get_nir_src(instr->src[0], 1), BRW_REGISTER_TYPE_UD);
   break;
 
+   case nir_intrinsic_load_primitive_id:
+  assert(c->prog_data.include_primitive_id);
+  dest = get_nir_dest(instr->dest, BRW_REGISTER_TYPE_D);

Re: [Mesa-dev] List of unsupported extensions per driver

2015-09-29 Thread Marek Olšák

FMA isn't required really. R600 is mainly missing GS streams, which
are complete on the mailing somewhere I think.

Marek

On Tue, Sep 29, 2015 at 7:32 AM, Romain Failliot
 wrote:
> Hi!
>
> I'm diving into the unsupported extensions list and I'm wondering how is it
> possible that GL_ARB_gpu_shader5 is unsupported for R600, but some of the
> "sub-extensions" like "Dynamically uniform sampler array indices" are
> supported nonetheless.
>
> That makes me wonder if "not done" sub-extensions, like "Fused
> multiply-add", are really not done for R600 yet, or if they are indeed
> unsupported as the parent extension status let us suppose.
>
> Thx
> Romain
>
> 2015-07-31 23:42 GMT+02:00 Ilia Mirkin :
>>
>> OK, I believe I've fixed my list up. Note that you may have to
>> shift-reload to get the updates, I think fd.o isn't setting the proper
>> cache headers or something else is messed up.
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH V6 10/27] i965: fix atomic buffer index for bindings other than 0

2015-09-29 Thread Timothy Arceri

On Tue, 2015-09-29 at 01:05 -0400, Ilia Mirkin wrote:
> On Mon, Sep 28, 2015 at 10:42 PM, Timothy Arceri <
> t_arc...@yahoo.com.au> wrote:
> > Since commit c0cd5b var->data.binding was being used as a
> > replacement
> > for atomic buffer index, but they don't have to be the same value
> > they
> > just happen to end up the same when binding is 0.
> > 
> > Now that we store the atomic uniform location in var->data.location
> > we can use this to lookup the atomic buffer index in uniform
> > storage.
> > 
> > For arrays of arrays the outer arrays have separate uniform
> > locations
> > however they all share the same buffer so we can get the buffer
> > index
> > using the base uniform location.
> > 
> > Cc: Alejandro Piñeiro 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90175
> > ---
> >  src/glsl/nir/glsl_to_nir.cpp   |  2 --
> >  src/glsl/nir/nir.h |  4 ++--
> >  src/glsl/nir/nir_lower_atomics.c   | 18 --
> > 
> >  src/mesa/drivers/dri/i965/brw_nir.c|  6 --
> >  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |  2 +-
> >  5 files changed, 19 insertions(+), 13 deletions(-)
> > 
> > diff --git a/src/glsl/nir/glsl_to_nir.cpp
> > b/src/glsl/nir/glsl_to_nir.cpp
> > index f03a107..30726be 100644
> > --- a/src/glsl/nir/glsl_to_nir.cpp
> > +++ b/src/glsl/nir/glsl_to_nir.cpp
> > @@ -330,8 +330,6 @@ nir_visitor::visit(ir_variable *ir)
> > 
> > var->data.index = ir->data.index;
> > var->data.binding = ir->data.binding;
> > -   /* XXX Get rid of buffer_index */
> > -   var->data.atomic.buffer_index = ir->data.binding;
> > var->data.atomic.offset = ir->data.atomic.offset;
> > var->data.image.read_only = ir->data.image_read_only;
> > var->data.image.write_only = ir->data.image_write_only;
> > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> > index 4f45770..670e39e 100644
> > --- a/src/glsl/nir/nir.h
> > +++ b/src/glsl/nir/nir.h
> > @@ -308,7 +308,6 @@ typedef struct {
> > * Location an atomic counter is stored at.
> > */
> >struct {
> > - unsigned buffer_index;
> >   unsigned offset;
> >} atomic;
> > 
> > @@ -1880,7 +1879,8 @@ void nir_lower_clip_fs(nir_shader *shader,
> > unsigned ucp_enables);
> > 
> >  void nir_lower_two_sided_color(nir_shader *shader);
> > 
> > -void nir_lower_atomics(nir_shader *shader);
> > +void nir_lower_atomics(nir_shader *shader,
> > +   const struct gl_shader_program
> > *shader_program);
> >  void nir_lower_to_source_mods(nir_shader *shader);
> > 
> >  bool nir_lower_gs_intrinsics(nir_shader *shader);
> > diff --git a/src/glsl/nir/nir_lower_atomics.c
> > b/src/glsl/nir/nir_lower_atomics.c
> > index 46e1376..52e7675 100644
> > --- a/src/glsl/nir/nir_lower_atomics.c
> > +++ b/src/glsl/nir/nir_lower_atomics.c
> > @@ -25,6 +25,7 @@
> >   *
> >   */
> > 
> > +#include "ir_uniform.h"
> >  #include "nir.h"
> >  #include "main/config.h"
> >  #include 
> > @@ -35,7 +36,8 @@
> >   */
> > 
> >  static void
> > -lower_instr(nir_intrinsic_instr *instr, nir_function_impl *impl)
> > +lower_instr(nir_intrinsic_instr *instr,
> > +const struct gl_shader_program *shader_program)
> >  {
> > nir_intrinsic_op op;
> > switch (instr->intrinsic) {
> > @@ -60,10 +62,11 @@ lower_instr(nir_intrinsic_instr *instr,
> > nir_function_impl *impl)
> >return; /* atomics passed as function arguments can't be
> > lowered */
> > 
> > void *mem_ctx = ralloc_parent(instr);
> > +   unsigned uniform_loc = instr->variables[0]->var->data.location;
> > 
> > nir_intrinsic_instr *new_instr =
> > nir_intrinsic_instr_create(mem_ctx, op);
> > new_instr->const_index[0] =
> > -  (int) instr->variables[0]->var->data.atomic.buffer_index;
> > +  shader_program
> > ->UniformStorage[uniform_loc].atomic_buffer_index;
> > 
> > nir_load_const_instr *offset_const =
> > nir_load_const_instr_create(mem_ctx, 1);
> > offset_const->value.u[0] = instr->variables[0]->var
> > ->data.atomic.offset;
> > @@ -128,22 +131,25 @@ lower_instr(nir_intrinsic_instr *instr,
> > nir_function_impl *impl)
> >  }
> > 
> >  static bool
> > -lower_block(nir_block *block, void *state)
> > +lower_block(nir_block *block, void *prog)
> >  {
> > nir_foreach_instr_safe(block, instr) {
> >if (instr->type == nir_instr_type_intrinsic)
> > - lower_instr(nir_instr_as_intrinsic(instr), state);
> > + lower_instr(nir_instr_as_intrinsic(instr),
> > + (const struct gl_shader_program *) prog);
> > }
> > 
> > return true;
> >  }
> > 
> >  void
> > -nir_lower_atomics(nir_shader *shader)
> > +nir_lower_atomics(nir_shader *shader,
> > +  const struct gl_shader_program *shader_program)
> >  {
> > nir_foreach_overload(shader, overload) {
> >if (overload->impl) {
> > - nir_foreach_block(overload->impl, lower_block, overload
> > ->impl);
>

Re: [Mesa-dev] [PATCH V6 13/27] glsl: fix whitespace

2015-09-29 Thread Iago Toral

Reviewed-by: Iago Toral Quiroga 

On Tue, 2015-09-29 at 12:42 +1000, Timothy Arceri wrote:
> ---
>  src/glsl/ast_to_hir.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index a9696dc..2fd4443 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -6400,7 +6400,7 @@ ast_interface_block::hir(exec_list *instructions,
>  }
>   }
>  
> -  /* From section 4.3.9 (Interface Blocks) of the GLSL ES 3.10 spec:
> + /* From section 4.3.9 (Interface Blocks) of the GLSL ES 3.10 spec:
>*
>* * Arrays of arrays of blocks are not allowed
>*/


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

Re: [Mesa-dev] [PATCH 2/2] egl/dri2: don't require a context for ClientWaitSync

2015-09-29 Thread Albert Freeman

On 29 September 2015 at 07:25, Albert Freeman  wrote:
> On 28 September 2015 at 08:10, Marek Olšák  wrote:
>> On Mon, Sep 28, 2015 at 7:20 AM, Albert Freeman
>>  wrote:
>>> On 28 September 2015 at 14:38, Albert Freeman  
>>> wrote:
 On 28 September 2015 at 04:12, Emil Velikov  
 wrote:
> On 26 September 2015 at 00:49, Marek Olšák  wrote:
>> From: Marek Olšák 
>>
>> The spec doesn't require it. This fixes a crash on Android.
>>
> Nicely spotted Marek.
>
> A couple of related (not supposed to be part of this patch) notes:
>  - This will just move the crash (from libEGL to i965_dri.so) for i965
> hardware :)
 I rediscovered this before I read the email related to this patch
 (where Marek mentions it): "Re: [Mesa-dev] Pending issues of
 lollipop-x86".
 Who (if anyone) should we CC this to?
>  - We really shouldn't be setting the flags if ctx is NULL, as per the 
> spec.
>
> "If no context is
> current for the bound API, the EGL_SYNC_FLUSH_COMMANDS_BIT_KHR bit
> is ignored."
 Maybe handle this in the driver (probably state_tracker in the case of
 gallium) level. Both here and drivers?
>>> OpenGL ClientWaitSync is very clear about SYNC_FLUSH_COMMANDS_BIT only
>>> flushing the GL context when the fence sync is created (it doesn’t
>>> seem to require it at the moment wait sync is called (probably to
>>> leave room for drivers to optimise)). EGL is a lot less clear, it
>>> could be interpreted as behaving just like GL, but to someone writing
>>> an EGL program it could be easily interpreted as "the flush() must
>>> occur when eglClientWaitSync is called with SYNC_FLUSH_COMMANDS_BIT".
>>>
>>> The current behaviour of gallium (not sure about i965) is to always
>>> flush() when creating the sync object (hence always behave as
>>> SYNC_FLUSH_COMMANDS_BIT is set according to GL). However
>>> implementations are allowed to behave as if flush was not required (in
>>> GL and I am assuming EGL). So technically the current behaviour of the
>>> gallium dri state tracker could reduce performance (since flushing
>>> tends to do that). It however doesn’t actually violate the EGL spec.
>>> Perhaps some heuristics on when to flush would be a good solution?
>>
>> Flushing is currently required for creating fences. This is a gallium
>> design decision as well as radeon/amdgpu kernel driver decision. It's
>> simply not possible to put a fence anywhere in the GPU command buffer.
>> We can only submit a command buffer to the kernel and then get a fence
>> tracking execution of the whole command buffer.
>>
>> Marek
> Thanks for the clarification.
Also when I said "Can easily be reverted later if a better solution is
found. :)". It seemed to me that this was an issue better resolved in
the code outside of mesa that had the issue.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] SSBO's in UniformBlocks list?

2015-09-29 Thread Ilia Mirkin

Hi Samuel, and any other onlookers,

I was wondering why the decision was made to stick SSBO's onto the
same list as constbufs. Seems like they're entirely separate entities,
no? Perhaps I'm missing something?

Thanks,

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

Re: [Mesa-dev] Pending issues of lollipop-x86

2015-09-29 Thread Albert Freeman

On 28 September 2015 at 20:25, Marek Olšák  wrote:
> On Mon, Sep 28, 2015 at 10:19 PM, Chad Versace  wrote:
>> On Fri 25 Sep 2015, Chih-Wei Huang wrote:
>>> CC to mesa-dev for help.
>>>
>>> 2015-09-25 22:12 GMT+08:00 Chih-Wei Huang :
>>> > 2015-09-25 16:21 GMT+08:00 Chih-Wei Huang :
>>> >> Actually I'm testing your mesa 11.0 branch
>>> >> to see if it is acceptable.
>>> >> The major issue I found is the
>>> >> Camera and Youtube crashing in mesa.
>>> >
>>> > OK, I can almost confirm this is a known issue
>>> > I reported to mesa devs before.
>>> > It is caused by this commit:
>>> >
>>> > commit c636284ee8ee95bb3f3ad31aaf26a9512ec5006c
>>> > Author: Chad Versace 
>>> > Date:   Tue May 5 19:05:32 2015 -0700
>>> >
>>> > i965/sync: Implement DRI2_Fence extension
>>> >
>>> > By reverting it the crashing is gone.
>>> >
>>> > However, I still hope we can find
>>> > a correct fix.
>>>
>>> After some debugging, it crashed in
>>> dri2_client_wait_sync() of
>>> ...src/egl/drivers/dri2/egl_dri2.c
>>> The ctx returned by _eglGetCurrentContext()
>>> is NULL.
>>>
>>> static EGLint
>>> dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync,
>>>   EGLint flags, EGLTime timeout)
>>> {
>>>_EGLContext *ctx = _eglGetCurrentContext();
>>>
>>> ==> ctx is NULL
>>>
>>>if (dri2_dpy->fence->client_wait_sync(dri2_ctx->dri_context,  <== OOPS!
>>>  dri2_sync->fence, wait_flags,
>>>  timeout))
>>>
>>>
>>> Why does _eglGetCurrentContext() return NULL?
>>> Any recommended fix?
>>
>> I don't know. Can provide a crashing test case for non-Android Linux? If
>> so, then I'll look into debugging it.
>
> Chad, you would be wasting your time. :) I'd already sent patches that
> fix this for libEGL and gallium and Emil sent an initial fix for i965
> which might work too.
>
> Marek
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Sorry, I misunderstood where the issue was.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir: split SSBO min/max atomic instrinsics into signed/unsigned versions

2015-09-29 Thread Francisco Jerez

Kristian Høgsberg  writes:

> On Mon, Sep 28, 2015 at 1:59 PM, Ilia Mirkin  wrote:
>> On Mon, Sep 28, 2015 at 4:55 PM, Kristian Høgsberg  
>> wrote:
>>> On Mon, Sep 28, 2015 at 1:47 AM, Iago Toral Quiroga  
>>> wrote:
 NIR is typeless so this is the only way to keep track of the
 type to select the proper atomic to use.
 ---
 I decided to squash the i965 changes in because otherwise we would break
 the build between the nir and i965 patches. Let me know if we rather
 split them anyway.

  src/glsl/nir/glsl_to_nir.cpp   | 22 ++
  src/glsl/nir/nir_intrinsics.h  |  6 --
  src/mesa/drivers/dri/i965/brw_fs_nir.cpp   | 20 ++--
  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 22 +++---
  4 files changed, 43 insertions(+), 27 deletions(-)

 diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
 index f03a107..27e9276 100644
 --- a/src/glsl/nir/glsl_to_nir.cpp
 +++ b/src/glsl/nir/glsl_to_nir.cpp
 @@ -662,9 +662,21 @@ nir_visitor::visit(ir_call *ir)
} else if (strcmp(ir->callee_name(), 
 "__intrinsic_ssbo_atomic_xor_internal") == 0) {
   op = nir_intrinsic_ssbo_atomic_xor;
} else if (strcmp(ir->callee_name(), 
 "__intrinsic_ssbo_atomic_min_internal") == 0) {
 - op = nir_intrinsic_ssbo_atomic_min;
 + assert(ir->return_deref);
 + if (ir->return_deref->type == glsl_type::int_type)
 +op = nir_intrinsic_ssbo_atomic_min_d;
 + else if (ir->return_deref->type == glsl_type::uint_type)
 +op = nir_intrinsic_ssbo_atomic_min_ud;
 + else
 +unreachable("Not reached");
} else if (strcmp(ir->callee_name(), 
 "__intrinsic_ssbo_atomic_max_internal") == 0) {
 - op = nir_intrinsic_ssbo_atomic_max;
 + assert(ir->return_deref);
 + if (ir->return_deref->type == glsl_type::int_type)
 +op = nir_intrinsic_ssbo_atomic_max_d;
 + else if (ir->return_deref->type == glsl_type::uint_type)
 +op = nir_intrinsic_ssbo_atomic_max_ud;
 + else
 +unreachable("Not reached");
} else if (strcmp(ir->callee_name(), 
 "__intrinsic_ssbo_atomic_exchange_internal") == 0) {
   op = nir_intrinsic_ssbo_atomic_exchange;
} else if (strcmp(ir->callee_name(), 
 "__intrinsic_ssbo_atomic_comp_swap_internal") == 0) {
 @@ -874,8 +886,10 @@ nir_visitor::visit(ir_call *ir)
   break;
}
case nir_intrinsic_ssbo_atomic_add:
 -  case nir_intrinsic_ssbo_atomic_min:
 -  case nir_intrinsic_ssbo_atomic_max:
 +  case nir_intrinsic_ssbo_atomic_min_d:
 +  case nir_intrinsic_ssbo_atomic_min_ud:
 +  case nir_intrinsic_ssbo_atomic_max_d:
 +  case nir_intrinsic_ssbo_atomic_max_ud:
case nir_intrinsic_ssbo_atomic_and:
case nir_intrinsic_ssbo_atomic_or:
case nir_intrinsic_ssbo_atomic_xor:
 diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
 index 06f1b02..956e68d 100644
 --- a/src/glsl/nir/nir_intrinsics.h
 +++ b/src/glsl/nir/nir_intrinsics.h
 @@ -174,8 +174,10 @@ INTRINSIC(image_samples, 0, ARR(), true, 1, 1, 0,
   * 3: For CompSwap only: the second data parameter.
   */
  INTRINSIC(ssbo_atomic_add, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
 -INTRINSIC(ssbo_atomic_min, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
 -INTRINSIC(ssbo_atomic_max, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
 +INTRINSIC(ssbo_atomic_min_d, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
 +INTRINSIC(ssbo_atomic_min_ud, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
 +INTRINSIC(ssbo_atomic_max_d, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
 +INTRINSIC(ssbo_atomic_max_ud, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
  INTRINSIC(ssbo_atomic_and, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
  INTRINSIC(ssbo_atomic_or, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
  INTRINSIC(ssbo_atomic_xor, 3, ARR(1, 1, 1), true, 1, 0, 0, 0)
 diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 index a2bc5c6..9fcddab 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 @@ -1870,17 +1870,17 @@ fs_visitor::nir_emit_intrinsic(const fs_builder 
 , nir_intrinsic_instr *instr
 case nir_intrinsic_ssbo_atomic_add:
nir_emit_ssbo_atomic(bld, BRW_AOP_ADD, instr);
break;
 -   case nir_intrinsic_ssbo_atomic_min:
 -  if (dest.type == BRW_REGISTER_TYPE_D)
 - nir_emit_ssbo_atomic(bld, BRW_AOP_IMIN, instr);
 -  else
 - nir_emit_ssbo_atomic(bld, BRW_AOP_UMIN, instr);
 +

Re: [Mesa-dev] [PATCH V6 10/27] i965: fix atomic buffer index for bindings other than 0

2015-09-29 Thread Ilia Mirkin

On Tue, Sep 29, 2015 at 2:48 AM, Timothy Arceri  wrote:
> On Tue, 2015-09-29 at 02:33 -0400, Ilia Mirkin wrote:
>> On Tue, Sep 29, 2015 at 2:26 AM, Timothy Arceri <
>> t_arc...@yahoo.com.au> wrote:
>> > On Tue, 2015-09-29 at 02:08 -0400, Ilia Mirkin wrote:
>> > > On Tue, Sep 29, 2015 at 2:05 AM, Timothy Arceri <
>> > > t_arc...@yahoo.com.au> wrote:
>> > > > On Tue, 2015-09-29 at 01:05 -0400, Ilia Mirkin wrote:
>> > > No. I mean the line:
>> > >
>> > >   brw->vtbl.emit_buffer_surface_state(brw, _offsets[i],
>> > > bo,
>> > >
>> > > Shouldn't that look up the atomic buffer index in prog->Uniforms
>> > > instead of using 'i'?
>> >
>> > No that looks fine. The problem in the patch was we didn't have the
>> > atomic buffex index and we used the binding as the offset which
>> > doesn't
>> > have to be the same as the buffer index.
>> >
>> > In brw_upload_abo_surfaces its the opposite we have the atomic
>> > buffer
>> > index which is i and we are looking up the binding to get the
>> > buffer
>> > object.
>> >
>> > Does that make sense?
>>
>> i is the binding index though...
>
> i is the buffer index
>
> See find_active_atomic_counters() for the code the counts the active
> buffers.
>
> http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_atomics.cpp#n9
> 9
>
> This count is then used to set NumAtomicBuffers and to create the prog
> ->AtomicBuffers array so the array is no bigger than it needs to be.
> This is the reason the binding and atomic buffer index can differ.
>
> http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_atomics.cpp#n1
> 73

Do you agree that NumAtomicBuffers can go up to
MaxCombinedAtomicBuffers? If so, that's problematic, when it's >
MaxAtomicBuffers, right?

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

Re: [Mesa-dev] [PATCH V6 10/27] i965: fix atomic buffer index for bindings other than 0

2015-09-29 Thread Timothy Arceri

On Tue, 2015-09-29 at 02:55 -0400, Ilia Mirkin wrote:
> On Tue, Sep 29, 2015 at 2:48 AM, Timothy Arceri <
> t_arc...@yahoo.com.au> wrote:
> > On Tue, 2015-09-29 at 02:33 -0400, Ilia Mirkin wrote:
> > > On Tue, Sep 29, 2015 at 2:26 AM, Timothy Arceri <
> > > t_arc...@yahoo.com.au> wrote:
> > > > On Tue, 2015-09-29 at 02:08 -0400, Ilia Mirkin wrote:
> > > > > On Tue, Sep 29, 2015 at 2:05 AM, Timothy Arceri <
> > > > > t_arc...@yahoo.com.au> wrote:
> > > > > > On Tue, 2015-09-29 at 01:05 -0400, Ilia Mirkin wrote:
> > > > > No. I mean the line:
> > > > > 
> > > > >   brw->vtbl.emit_buffer_surface_state(brw,
> > > > > _offsets[i],
> > > > > bo,
> > > > > 
> > > > > Shouldn't that look up the atomic buffer index in prog
> > > > > ->Uniforms
> > > > > instead of using 'i'?
> > > > 
> > > > No that looks fine. The problem in the patch was we didn't have
> > > > the
> > > > atomic buffex index and we used the binding as the offset which
> > > > doesn't
> > > > have to be the same as the buffer index.
> > > > 
> > > > In brw_upload_abo_surfaces its the opposite we have the atomic
> > > > buffer
> > > > index which is i and we are looking up the binding to get the
> > > > buffer
> > > > object.
> > > > 
> > > > Does that make sense?
> > > 
> > > i is the binding index though...
> > 
> > i is the buffer index
> > 
> > See find_active_atomic_counters() for the code the counts the
> > active
> > buffers.
> > 
> > http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_atomics.cp
> > p#n9
> > 9
> > 
> > This count is then used to set NumAtomicBuffers and to create the
> > prog
> > ->AtomicBuffers array so the array is no bigger than it needs to
> > be.
> > This is the reason the binding and atomic buffer index can differ.
> > 
> > http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_atomics.cp
> > p#n1
> > 73
> 
> Do you agree that NumAtomicBuffers can go up to
> MaxCombinedAtomicBuffers? If so, that's problematic, when it's >
> MaxAtomicBuffers, right?

Ok, so now we are talking about a new problem from what this patch is
trying to solve.

I agree it can go up to MaxCombinedAtomicBuffers.

I'm not sure what the answer is, maybe a question for curro. I don't
know enough about this code to know if we want to upload everything
here or just do a stage at a time.


> 
>   -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir: Use a system value for gl_PrimitiveIDIn.

2015-09-29 Thread Iago Toral

On Mon, 2015-09-28 at 23:05 -0700, Kenneth Graunke wrote:
> At least on Intel hardware, gl_PrimitiveIDIn comes in as a special part
> of the payload rather than a normal input.  This is typically what we
> use system values for.  Dave and Ilia also agree that a system value
> would be nicer.
> 
> At some point, we should change it at the GLSL IR level as well.  But
> that requires changing most of the drivers.  For now, let's at least
> make NIR do the right thing, which is easy.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/glsl/nir/glsl_to_nir.cpp  | 5 +
>  src/glsl/nir/nir.c| 5 -
>  src/glsl/nir/nir_intrinsics.h | 1 +
>  src/glsl/shader_enums.h   | 2 +-
>  src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp | 9 +
>  5 files changed, 20 insertions(+), 2 deletions(-)
> 
> I bypassed most of the system value boilerplate in the backend.  Notably,
> this means we just access g1 directly rather than moving it to a VGRF at
> the start of the program and using that later.  This means more HW_REG
> usage, but it also means less seemingly pointless copies.
> 
> I'm hoping to simplify the handling of other system values too, but I'm
> waiting until we delete the GLSL IR code paths first.
> 
> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
> index f03a107..c0b2634 100644
> --- a/src/glsl/nir/glsl_to_nir.cpp
> +++ b/src/glsl/nir/glsl_to_nir.cpp
> @@ -271,6 +271,11 @@ nir_visitor::visit(ir_variable *ir)
>   /* For whatever reason, GLSL IR makes gl_FrontFacing an input */
>   var->data.location = SYSTEM_VALUE_FRONT_FACE;
>   var->data.mode = nir_var_system_value;
> +  } else if (shader->stage == MESA_SHADER_GEOMETRY &&
> + ir->data.location == VARYING_SLOT_PRIMITIVE_ID) {
> + /* For whatever reason, GLSL IR makes gl_PrimitiveIDIn an input */
> + var->data.location = SYSTEM_VALUE_PRIMITIVE_ID;
> + var->data.mode = nir_var_system_value;
>} else {
>   var->data.mode = nir_var_shader_in;
>}
> diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
> index 1206bb4..7f30b8a 100644
> --- a/src/glsl/nir/nir.c
> +++ b/src/glsl/nir/nir.c
> @@ -1487,10 +1487,11 @@ nir_intrinsic_from_system_value(gl_system_value val)
>return nir_intrinsic_load_local_invocation_id;
> case SYSTEM_VALUE_WORK_GROUP_ID:
>return nir_intrinsic_load_work_group_id;
> +   case SYSTEM_VALUE_PRIMITIVE_ID:
> +  return nir_intrinsic_load_primitive_id;
> /* FINISHME: Add tessellation intrinsics.
> case SYSTEM_VALUE_TESS_COORD:
> case SYSTEM_VALUE_VERTICES_IN:
> -   case SYSTEM_VALUE_PRIMITIVE_ID:
> case SYSTEM_VALUE_TESS_LEVEL_OUTER:
> case SYSTEM_VALUE_TESS_LEVEL_INNER:
>  */
> @@ -1525,6 +1526,8 @@ nir_system_value_from_intrinsic(nir_intrinsic_op intrin)
>return SYSTEM_VALUE_LOCAL_INVOCATION_ID;
> case nir_intrinsic_load_work_group_id:
>return SYSTEM_VALUE_WORK_GROUP_ID;
> +   case nir_intrinsic_load_primitive_id:
> +  return SYSTEM_VALUE_PRIMITIVE_ID;
> /* FINISHME: Add tessellation intrinsics.
>return SYSTEM_VALUE_TESS_COORD;
>return SYSTEM_VALUE_VERTICES_IN;
> diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
> index 06f1b02..0d93b12 100644
> --- a/src/glsl/nir/nir_intrinsics.h
> +++ b/src/glsl/nir/nir_intrinsics.h
> @@ -194,6 +194,7 @@ SYSTEM_VALUE(instance_id, 1, 0)
>  SYSTEM_VALUE(sample_id, 1, 0)
>  SYSTEM_VALUE(sample_pos, 2, 0)
>  SYSTEM_VALUE(sample_mask_in, 1, 0)
> +SYSTEM_VALUE(primitive_id, 1, 0)
>  SYSTEM_VALUE(invocation_id, 1, 0)
>  SYSTEM_VALUE(local_invocation_id, 3, 0)
>  SYSTEM_VALUE(work_group_id, 3, 0)
> diff --git a/src/glsl/shader_enums.h b/src/glsl/shader_enums.h
> index 3978007..f6b71a3 100644
> --- a/src/glsl/shader_enums.h
> +++ b/src/glsl/shader_enums.h
> @@ -399,7 +399,7 @@ typedef enum
> /*@{*/
> SYSTEM_VALUE_TESS_COORD,
> SYSTEM_VALUE_VERTICES_IN,/**< Tessellation vertices in input patch */
> -   SYSTEM_VALUE_PRIMITIVE_ID,   /**< (currently not used by GS) */
> +   SYSTEM_VALUE_PRIMITIVE_ID,
> SYSTEM_VALUE_TESS_LEVEL_OUTER, /**< TES input */
> SYSTEM_VALUE_TESS_LEVEL_INNER, /**< TES input */
> /*@}*/
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp
> index 4f4e1e1..26766a0 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp
> @@ -72,6 +72,9 @@ 
> vec4_gs_visitor::nir_setup_system_value_intrinsic(nir_intrinsic_instr *instr)
> dst_reg *reg;
>  
> switch (instr->intrinsic) {
> +   case nir_intrinsic_load_primitive_id:

Maybe add a one-line comment saying that we will access grf1 directly so
there is no need to setup a vgrf here? Either way:

Reviewed-by: Iago Toral Quiroga 

> +  break;
> +
> case

Re: [Mesa-dev] [PATCH] util: implement strndup for WIN32

2015-09-29 Thread Neil Roberts

I think this implementation will have problems if the string being
copied is not null terminated. It's not clear from the man pages whether
that is an allowed way to use the function but a quick Google shows up a
few similar patches where they have later been fixed by using strnlen.
It looks like strnlen is available in MinGW so I think it would be worth
doing.

I notice that ralloc_strndup has a similar problem so maybe it would be
worth fixing both of them in a separate patch.

I don't understand the reasoning for using calloc instead of malloc.
Won't that just add redundant overhead?

Sorry this review was too late to get in before the patch landed.

Regards,
- Neil

Samuel Iglesias Gonsalvez  writes:

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92124
> Cc: Jose Fonseca 
> ---
>
> I tested it on MSVC but not MinGW. I hope I did not something wrong.
>
>  src/mesa/main/shader_query.cpp |  1 +
>  src/util/Makefile.sources  |  1 +
>  src/util/strndup.c | 47 
> ++
>  src/util/strndup.h | 28 +
>  4 files changed, 77 insertions(+)
>  create mode 100644 src/util/strndup.c
>  create mode 100644 src/util/strndup.h
>
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index e020dce..0cada50 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -37,6 +37,7 @@
>  #include "../glsl/program.h"
>  #include "uniforms.h"
>  #include "main/enums.h"
> +#include "util/strndup.h"
>  
>  extern "C" {
>  #include "shaderapi.h"
> diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
> index afdd0cb..10f2c02 100644
> --- a/src/util/Makefile.sources
> +++ b/src/util/Makefile.sources
> @@ -17,6 +17,7 @@ MESA_UTIL_FILES :=  \
>   set.c \
>   set.h \
>   simple_list.h \
> + strndup.c \
>   strtod.c \
>   strtod.h \
>   texcompress_rgtc_tmp.h \
> diff --git a/src/util/strndup.c b/src/util/strndup.c
> new file mode 100644
> index 000..6e5bb22
> --- /dev/null
> +++ b/src/util/strndup.c
> @@ -0,0 +1,47 @@
> +/*
> + * Copyright (c) 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#if defined(_WIN32)
> +#include 
> +#include "strndup.h"
> +
> +char *
> +strndup(const char *str, size_t max)
> +{
> +   size_t n;
> +   char *ptr;
> +
> +   if (str == NULL)
> +  return NULL;
> +
> +   n = strlen(str);
> +   if (n > max)
> +  n = max;
> +
> +   ptr = (char *) malloc((n + 1) * sizeof(char));
> +   memcpy(ptr, str, n);
> +   ptr[n] = '\0';
> +   return ptr;
> +}
> +
> +#endif
> diff --git a/src/util/strndup.h b/src/util/strndup.h
> new file mode 100644
> index 000..dc8cdd2
> --- /dev/null
> +++ b/src/util/strndup.h
> @@ -0,0 +1,28 @@
> +/*
> + * Copyright (c) 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR

[Mesa-dev] [PATCH v2] mesa/meta: Use interpolateAtOffset for 16x MSAA copy blit

2015-09-29 Thread Neil Roberts

Previously there was a problem in i965 where if 16x MSAA is used then
some of the sample positions are exactly on the 0 x or y axis. When
the MSAA copy blit shader interpolates the texture coordinates at
these sample positions it was possible that it would jump to a
neighboring texel due to rounding errors. It is likely that these
positions would be used on 16x MSAA because that is where they are
defined to be in D3D.

To fix that this patch makes it use interpolateAtOffset in the blit
shader whenever 16x MSAA is used and the GL_ARB_gpu_shader5 extension
is available. This forces it to interpolate the texture coordinates at
the pixel center to avoid these problematic positions.

This fixes ext_framebuffer_multisample-unaligned-blit and
ext_framebuffer_multisample-clip-and-scissor-blit with 16x MSAA on
SKL+.
---
 src/mesa/drivers/common/meta_blit.c | 64 ++---
 1 file changed, 52 insertions(+), 12 deletions(-)

diff --git a/src/mesa/drivers/common/meta_blit.c 
b/src/mesa/drivers/common/meta_blit.c
index 1f9515a..e812ecb 100644
--- a/src/mesa/drivers/common/meta_blit.c
+++ b/src/mesa/drivers/common/meta_blit.c
@@ -352,17 +352,27 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,
shader_index == BLIT_MSAA_SHADER_2D_MULTISAMPLE_ARRAY_DEPTH_COPY ||
shader_index == BLIT_MSAA_SHADER_2D_MULTISAMPLE_DEPTH_COPY) {
   char *sample_index;
-  const char *arb_sample_shading_extension_string;
+  const char *extra_extensions;
+  const char *tex_coords = "texCoords";
 
   if (dst_is_msaa) {
- arb_sample_shading_extension_string = "#extension 
GL_ARB_sample_shading : enable";
  sample_index = "gl_SampleID";
  name = "depth MSAA copy";
+
+ if (ctx->Extensions.ARB_gpu_shader5 && samples >= 16) {
+extra_extensions =
+   "#extension GL_ARB_sample_shading : enable\n"
+   "#extension GL_ARB_gpu_shader5 : enable\n";
+/* See comment below for the color copy */
+tex_coords = "interpolateAtOffset(texCoords, vec2(0.0))";
+ } else {
+extra_extensions = "#extension GL_ARB_sample_shading : enable\n";
+ }
   } else {
- /* Don't need that extension, since we're drawing to a single-sampled
-  * destination.
+ /* Don't need any extra extensions, since we're drawing to a
+  * single-sampled destination.
   */
- arb_sample_shading_extension_string = "";
+ extra_extensions = "";
  /* From the GL 4.3 spec:
   *
   * "If there is a multisample buffer (the value of SAMPLE_BUFFERS
@@ -399,27 +409,57 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,
   "\n"
   "void main()\n"
   "{\n"
-  "   gl_FragDepth = texelFetch(texSampler, 
i%s(texCoords), %s).r;\n"
+  "   gl_FragDepth = texelFetch(texSampler, 
i%s(%s), %s).r;\n"
   "}\n",
-  arb_sample_shading_extension_string,
+  extra_extensions,
   sampler_array_suffix,
   texcoord_type,
   texcoord_type,
+  tex_coords,
   sample_index);
} else {
   /* You can create 2D_MULTISAMPLE textures with 0 sample count (meaning 1
* sample).  Yes, this is ridiculous.
*/
   char *sample_resolve;
-  const char *arb_sample_shading_extension_string;
+  const char *extra_extensions;
   const char *merge_function;
   name = ralloc_asprintf(mem_ctx, "%svec4 MSAA %s",
  vec4_prefix,
  dst_is_msaa ? "copy" : "resolve");
 
   if (dst_is_msaa) {
- arb_sample_shading_extension_string = "#extension 
GL_ARB_sample_shading : enable";
- sample_resolve = ralloc_asprintf(mem_ctx, "   out_color = 
texelFetch(texSampler, i%s(texCoords), gl_SampleID);", texcoord_type);
+ const char *tex_coords;
+
+ if (ctx->Extensions.ARB_gpu_shader5 && samples >= 16) {
+/* If interpolateAtOffset is available then it will be used to
+ * force the interpolation to the center. This is required at
+ * least on Intel hardware because it is possible to have a sample
+ * position on the 0 x or y axis which means it will lie exactly
+ * on the pixel boundary. If we let the hardware interpolate the
+ * coordinates at one of these positions then it is possible for
+ * it to jump to a neighboring texel when converting to ints due
+ * to rounding errors. This is only done for >= 16x MSAA because
+ * it probably has some overhead. It is

Re: [Mesa-dev] [PATCH V6 10/27] i965: fix atomic buffer index for bindings other than 0

2015-09-29 Thread Francisco Jerez

Ilia Mirkin  writes:

> On Tue, Sep 29, 2015 at 3:32 AM, Timothy Arceri  wrote:
>> On Tue, 2015-09-29 at 02:55 -0400, Ilia Mirkin wrote:
>>> On Tue, Sep 29, 2015 at 2:48 AM, Timothy Arceri <
>>> t_arc...@yahoo.com.au> wrote:
>>> > On Tue, 2015-09-29 at 02:33 -0400, Ilia Mirkin wrote:
>>> > > On Tue, Sep 29, 2015 at 2:26 AM, Timothy Arceri <
>>> > > t_arc...@yahoo.com.au> wrote:
>>> > > > On Tue, 2015-09-29 at 02:08 -0400, Ilia Mirkin wrote:
>>> > > > > On Tue, Sep 29, 2015 at 2:05 AM, Timothy Arceri <
>>> > > > > t_arc...@yahoo.com.au> wrote:
>>> > > > > > On Tue, 2015-09-29 at 01:05 -0400, Ilia Mirkin wrote:
>>> > > > > No. I mean the line:
>>> > > > >
>>> > > > >   brw->vtbl.emit_buffer_surface_state(brw,
>>> > > > > _offsets[i],
>>> > > > > bo,
>>> > > > >
>>> > > > > Shouldn't that look up the atomic buffer index in prog
>>> > > > > ->Uniforms
>>> > > > > instead of using 'i'?
>>> > > >
>>> > > > No that looks fine. The problem in the patch was we didn't have
>>> > > > the
>>> > > > atomic buffex index and we used the binding as the offset which
>>> > > > doesn't
>>> > > > have to be the same as the buffer index.
>>> > > >
>>> > > > In brw_upload_abo_surfaces its the opposite we have the atomic
>>> > > > buffer
>>> > > > index which is i and we are looking up the binding to get the
>>> > > > buffer
>>> > > > object.
>>> > > >
>>> > > > Does that make sense?
>>> > >
>>> > > i is the binding index though...
>>> >
>>> > i is the buffer index
>>> >
>>> > See find_active_atomic_counters() for the code the counts the
>>> > active
>>> > buffers.
>>> >
>>> > http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_atomics.cp
>>> > p#n9
>>> > 9
>>> >
>>> > This count is then used to set NumAtomicBuffers and to create the
>>> > prog
>>> > ->AtomicBuffers array so the array is no bigger than it needs to
>>> > be.
>>> > This is the reason the binding and atomic buffer index can differ.
>>> >
>>> > http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_atomics.cp
>>> > p#n1
>>> > 73
>>>
>>> Do you agree that NumAtomicBuffers can go up to
>>> MaxCombinedAtomicBuffers? If so, that's problematic, when it's >
>>> MaxAtomicBuffers, right?
>>
>> Ok, so now we are talking about a new problem from what this patch is
>> trying to solve.
>
> Well, the problem is "the indexes are all messed up". I think this
> patch fixes half the problem in a way that happens to work some of the
> time, much like the existing logic. (Admittedly your solution works
> *more* of the time...)
>
I think this patch is correct in the sense that it makes the atomic
buffer index used by the compiler consistent with the index of the same
atomic counter buffer used by the state upload code, which was no longer
the case since c0cd5bedf66887e958e140c047afc5bc2616.
>>
>> I agree it can go up to MaxCombinedAtomicBuffers.
>>
>> I'm not sure what the answer is, maybe a question for curro. I don't
>> know enough about this code to know if we want to upload everything
>> here or just do a stage at a time.
>
> We need to keep track of an intra-stage index, accessible both at
> instruction emission time as well as at resource emission time. I
> thought that was the atomic_buffer_index. Sounds like it's not, but in
> that case it should be made to be that IMO. See
> _mesa_get_sampler_uniform_value for how this is handled for
> samplers... not sure if it'll be helpful or not.
>
You're right that gl_uniform_storage::atomic_buffer_index is per-program
rather than per-stage, but the choice is not arbitrary, it's mandated by
the spec: ARB_shader_atomic_counters defines a space of active atomic
counter buffer indices (e.g. the bufferIndex parameter passed to
glGetActiveAtomicCounterBufferiv()) with one entry for each individual
atomic counter buffer actually referenced by the program.  The state
required for each active atomic counter buffer in the program maps to an
entry of the gl_shader_program::AtomicBuffers array.  The purpose of
gl_uniform_storage::atomic_buffer_index is to map a given atomic counter
uniform to the index of its BO in the same index space (i.e. it's the
same thing you query with the GL_UNIFORM_ATOMIC_COUNTER_BUFFER_INDEX
uniform param).

I agree with you that some sort of intra-stage index (kind of like
gl_uniform_storage::sampler[], ::image[] and such) would be useful, at
least for the i965 driver: The current i965 implementation uses the
active atomic counter buffer index as surface index for simplicity, what
is definitely suboptimal since it imposes a limit on the sum of distinct
atomic counter buffers that may be used in the whole program, because it
means we have to upload the same set of ABO surfaces to all stages in
the program.

>   -ilia
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature

Re: [Mesa-dev] [PATCH] util: implement strndup for WIN32

2015-09-29 Thread Samuel Iglesias Gonsálvez



On 29/09/15 15:21, Neil Roberts wrote:
> I think this implementation will have problems if the string being
> copied is not null terminated. It's not clear from the man pages whether
> that is an allowed way to use the function but a quick Google shows up a
> few similar patches where they have later been fixed by using strnlen.
> It looks like strnlen is available in MinGW so I think it would be worth
> doing.
> 

You are right. If string is not null-terminated, we would have an
undefined result from strlen().

> I notice that ralloc_strndup has a similar problem so maybe it would be
> worth fixing both of them in a separate patch.
> 

OK, I can write a patch this patch.

> I don't understand the reasoning for using calloc instead of malloc.
> Won't that just add redundant overhead?
>

Yes, it would add some overhead but we get rid of the null character we
would need add at the end, and calloc checks for overflow on
multiplication: if the total size of the requested block is too large
(like overflows size_t), calloc returns null pointer to indicate failure.

However, it's true this overflow is not very common.

> Sorry this review was too late to get in before the patch landed.
> 

No problem. Next time I will wait some time before pushing any patch.

Thanks,

Sam

> Regards,
> - Neil
> 
> Samuel Iglesias Gonsalvez  writes:
> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92124
>> Cc: Jose Fonseca 
>> ---
>>
>> I tested it on MSVC but not MinGW. I hope I did not something wrong.
>>
>>  src/mesa/main/shader_query.cpp |  1 +
>>  src/util/Makefile.sources  |  1 +
>>  src/util/strndup.c | 47 
>> ++
>>  src/util/strndup.h | 28 +
>>  4 files changed, 77 insertions(+)
>>  create mode 100644 src/util/strndup.c
>>  create mode 100644 src/util/strndup.h
>>
>> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
>> index e020dce..0cada50 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -37,6 +37,7 @@
>>  #include "../glsl/program.h"
>>  #include "uniforms.h"
>>  #include "main/enums.h"
>> +#include "util/strndup.h"
>>  
>>  extern "C" {
>>  #include "shaderapi.h"
>> diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
>> index afdd0cb..10f2c02 100644
>> --- a/src/util/Makefile.sources
>> +++ b/src/util/Makefile.sources
>> @@ -17,6 +17,7 @@ MESA_UTIL_FILES := \
>>  set.c \
>>  set.h \
>>  simple_list.h \
>> +strndup.c \
>>  strtod.c \
>>  strtod.h \
>>  texcompress_rgtc_tmp.h \
>> diff --git a/src/util/strndup.c b/src/util/strndup.c
>> new file mode 100644
>> index 000..6e5bb22
>> --- /dev/null
>> +++ b/src/util/strndup.c
>> @@ -0,0 +1,47 @@
>> +/*
>> + * Copyright (c) 2015 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
>> DEALINGS
>> + * IN THE SOFTWARE.
>> + */
>> +
>> +#if defined(_WIN32)
>> +#include 
>> +#include "strndup.h"
>> +
>> +char *
>> +strndup(const char *str, size_t max)
>> +{
>> +   size_t n;
>> +   char *ptr;
>> +
>> +   if (str == NULL)
>> +  return NULL;
>> +
>> +   n = strlen(str);
>> +   if (n > max)
>> +  n = max;
>> +
>> +   ptr = (char *) malloc((n + 1) * sizeof(char));
>> +   memcpy(ptr, str, n);
>> +   ptr[n] = '\0';
>> +   return ptr;
>> +}
>> +
>> +#endif
>> diff --git a/src/util/strndup.h b/src/util/strndup.h
>> new file mode 100644
>> index 000..dc8cdd2
>> --- /dev/null
>> +++ b/src/util/strndup.h
>> @@ -0,0 +1,28 @@
>> +/*
>> + * Copyright (c) 2015 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including

Re: [Mesa-dev] Pending issues of lollipop-x86

2015-09-29 Thread Rob Clark

btw, on the topic of lollipop issues, I don't suppose anyone reported
yet (or started to debug) this issue:

http://git.android-x86.org/?p=platform/frameworks/base.git;a=commitdiff;h=a0571d1e73a34251bf86de5ee85ac2c306c40ea3

I don't have the setup with me today, but I was still seeing a crash
in mesa compiler without that patch (git master as of a couple weeks
ago).  But I couldn't reproduce it on linux where I have gdb, with a
quick attempt to hack up a simple test program using the same
shaders..

BR,
-R


On Fri, Sep 25, 2015 at 11:43 AM, Chih-Wei Huang
 wrote:
> CC to mesa-dev for help.
>
> 2015-09-25 22:12 GMT+08:00 Chih-Wei Huang :
>> 2015-09-25 16:21 GMT+08:00 Chih-Wei Huang :
>>> Actually I'm testing your mesa 11.0 branch
>>> to see if it is acceptable.
>>> The major issue I found is the
>>> Camera and Youtube crashing in mesa.
>>
>> OK, I can almost confirm this is a known issue
>> I reported to mesa devs before.
>> It is caused by this commit:
>>
>> commit c636284ee8ee95bb3f3ad31aaf26a9512ec5006c
>> Author: Chad Versace 
>> Date:   Tue May 5 19:05:32 2015 -0700
>>
>> i965/sync: Implement DRI2_Fence extension
>>
>> By reverting it the crashing is gone.
>>
>> However, I still hope we can find
>> a correct fix.
>
> After some debugging, it crashed in
> dri2_client_wait_sync() of
> ...src/egl/drivers/dri2/egl_dri2.c
> The ctx returned by _eglGetCurrentContext()
> is NULL.
>
> static EGLint
> dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync,
>   EGLint flags, EGLTime timeout)
> {
>_EGLContext *ctx = _eglGetCurrentContext();
>
> ==> ctx is NULL
>
>if (dri2_dpy->fence->client_wait_sync(dri2_ctx->dri_context,  <== OOPS!
>  dri2_sync->fence, wait_flags,
>  timeout))
>
>
> Why does _eglGetCurrentContext() return NULL?
> Any recommended fix?
>
>
> --
> Chih-Wei
> Android-x86 project
> http://www.android-x86.org
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH V6 10/27] i965: fix atomic buffer index for bindings other than 0

2015-09-29 Thread Timothy Arceri

On Tue, 2015-09-29 at 16:08 +0300, Francisco Jerez wrote:
> Ilia Mirkin  writes:
> 
> > On Tue, Sep 29, 2015 at 3:32 AM, Timothy Arceri <
> > t_arc...@yahoo.com.au> wrote:
> > > On Tue, 2015-09-29 at 02:55 -0400, Ilia Mirkin wrote:
> > > > On Tue, Sep 29, 2015 at 2:48 AM, Timothy Arceri <
> > > > t_arc...@yahoo.com.au> wrote:
> > > > > On Tue, 2015-09-29 at 02:33 -0400, Ilia Mirkin wrote:
> > > > > > On Tue, Sep 29, 2015 at 2:26 AM, Timothy Arceri <
> > > > > > t_arc...@yahoo.com.au> wrote:
> > > > > > > On Tue, 2015-09-29 at 02:08 -0400, Ilia Mirkin wrote:
> > > > > > > > On Tue, Sep 29, 2015 at 2:05 AM, Timothy Arceri <
> > > > > > > > t_arc...@yahoo.com.au> wrote:
> > > > > > > > > On Tue, 2015-09-29 at 01:05 -0400, Ilia Mirkin wrote:
> > > > > > > > No. I mean the line:
> > > > > > > > 
> > > > > > > >   brw->vtbl.emit_buffer_surface_state(brw,
> > > > > > > > _offsets[i],
> > > > > > > > bo,
> > > > > > > > 
> > > > > > > > Shouldn't that look up the atomic buffer index in prog
> > > > > > > > ->Uniforms
> > > > > > > > instead of using 'i'?
> > > > > > > 
> > > > > > > No that looks fine. The problem in the patch was we
> > > > > > > didn't have
> > > > > > > the
> > > > > > > atomic buffex index and we used the binding as the offset
> > > > > > > which
> > > > > > > doesn't
> > > > > > > have to be the same as the buffer index.
> > > > > > > 
> > > > > > > In brw_upload_abo_surfaces its the opposite we have the
> > > > > > > atomic
> > > > > > > buffer
> > > > > > > index which is i and we are looking up the binding to get
> > > > > > > the
> > > > > > > buffer
> > > > > > > object.
> > > > > > > 
> > > > > > > Does that make sense?
> > > > > > 
> > > > > > i is the binding index though...
> > > > > 
> > > > > i is the buffer index
> > > > > 
> > > > > See find_active_atomic_counters() for the code the counts the
> > > > > active
> > > > > buffers.
> > > > > 
> > > > > http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_atom
> > > > > ics.cp
> > > > > p#n9
> > > > > 9
> > > > > 
> > > > > This count is then used to set NumAtomicBuffers and to create
> > > > > the
> > > > > prog
> > > > > ->AtomicBuffers array so the array is no bigger than it needs
> > > > > to
> > > > > be.
> > > > > This is the reason the binding and atomic buffer index can
> > > > > differ.
> > > > > 
> > > > > http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/link_atom
> > > > > ics.cp
> > > > > p#n1
> > > > > 73
> > > > 
> > > > Do you agree that NumAtomicBuffers can go up to
> > > > MaxCombinedAtomicBuffers? If so, that's problematic, when it's
> > > > >
> > > > MaxAtomicBuffers, right?
> > > 
> > > Ok, so now we are talking about a new problem from what this
> > > patch is
> > > trying to solve.
> > 
> > Well, the problem is "the indexes are all messed up". I think this
> > patch fixes half the problem in a way that happens to work some of
> > the
> > time, much like the existing logic. (Admittedly your solution works
> > *more* of the time...)
> > 
> I think this patch is correct in the sense that it makes the atomic
> buffer index used by the compiler consistent with the index of the
> same
> atomic counter buffer used by the state upload code, which was no
> longer
> the case since c0cd5bedf66887e958e140c047afc5bc2616.
> > > 
> > > I agree it can go up to MaxCombinedAtomicBuffers.
> > > 
> > > I'm not sure what the answer is, maybe a question for curro. I
> > > don't
> > > know enough about this code to know if we want to upload
> > > everything
> > > here or just do a stage at a time.
> > 
> > We need to keep track of an intra-stage index, accessible both at
> > instruction emission time as well as at resource emission time. I
> > thought that was the atomic_buffer_index. Sounds like it's not, but
> > in
> > that case it should be made to be that IMO. See
> > _mesa_get_sampler_uniform_value for how this is handled for
> > samplers... not sure if it'll be helpful or not.
> > 
> You're right that gl_uniform_storage::atomic_buffer_index is per
> -program
> rather than per-stage, but the choice is not arbitrary, it's mandated
> by
> the spec: ARB_shader_atomic_counters defines a space of active atomic
> counter buffer indices (e.g. the bufferIndex parameter passed to
> glGetActiveAtomicCounterBufferiv()) with one entry for each
> individual
> atomic counter buffer actually referenced by the program.  The state
> required for each active atomic counter buffer in the program maps to
> an
> entry of the gl_shader_program::AtomicBuffers array.  The purpose of
> gl_uniform_storage::atomic_buffer_index is to map a given atomic
> counter
> uniform to the index of its BO in the same index space (i.e. it's the
> same thing you query with the GL_UNIFORM_ATOMIC_COUNTER_BUFFER_INDEX
> uniform param).
> 
> I agree with you that some sort of intra-stage index (kind of like
> gl_uniform_storage::sampler[], ::image[] and such) would be useful,
> at
> least for the i965 driver: The current i965

[Mesa-dev] [Bug 92124] shader_query.cpp:841:34: error: ‘strndup’ was not declared in this scope

2015-09-29 Thread bugzilla-daemon

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

--- Comment #5 from Samuel Iglesias  ---
Should be fixed by:

commit bea66d22f2c0e97242c15652376ae4e82779f343
Author: Samuel Iglesias Gonsalvez 
Date:   Mon Sep 28 14:23:34 2015 +0200

util: implement strndup for WIN32

v2:
- Add strndup.h to Makefile.sources (Emil)
- Use calloc instead of malloc (Emil).
- Check if allocation fails (Emil, Jose)
- Add '#pragma once' and include stdlib.h to strndup.h (Jose)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92124
Reviewed-by: Jose Fonseca 

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa: fix ARRAY_SIZE query for GetProgramResourceiv

2015-09-29 Thread Emil Velikov

Hi guys,

On 29 September 2015 at 10:30, Martin Peres
 wrote:
> On 28/09/15 13:51, Tapani Pälli wrote:
>>
>> Patch also refactors name length queries which were using array size
>> in computation, this has to be done in same time to avoid regression in
>> arb_program_interface_query-resource-query Piglit test.
>>
>> Fixes rest of the failures with
>> ES31-CTS.program_interface_query.no-locations
>>
>> v2: make additional check only for GS inputs
>> v3: create helper function for resource name length
>>  so that it gets calculated only in one place
>>
>> Signed-off-by: Tapani Pälli 
>> Reviewed-by: Martin Peres  [v2]
>> ---
>>   src/mesa/main/program_resource.c |  8 ++--
>>   src/mesa/main/shader_query.cpp   | 94
>> 
>>   src/mesa/main/shaderapi.h|  3 ++
>>   3 files changed, 62 insertions(+), 43 deletions(-)
>>
>> diff --git a/src/mesa/main/program_resource.c
>> b/src/mesa/main/program_resource.c
>> index c609abe..eb71fdd 100644
>> --- a/src/mesa/main/program_resource.c
>> +++ b/src/mesa/main/program_resource.c
>> @@ -111,11 +111,9 @@ _mesa_GetProgramInterfaceiv(GLuint program, GLenum
>> programInterface,
>> for (i = 0, *params = 0; i < shProg->NumProgramResourceList; i++)
>> {
>>if (shProg->ProgramResourceList[i].Type != programInterface)
>>   continue;
>> - const char *name =
>> -_mesa_program_resource_name(>ProgramResourceList[i]);
>> - unsigned array_size =
>> -
>> _mesa_program_resource_array_size(>ProgramResourceList[i]);
>> - *params = MAX2(*params, strlen(name) + (array_size ? 3 : 0) +
>> 1);
>> + unsigned len =
>> +
>> _mesa_program_resource_name_len(>ProgramResourceList[i]);
>> + *params = MAX2(*params, len + 1);
>> }
>> break;
>>  case GL_MAX_NUM_ACTIVE_VARIABLES:
>> diff --git a/src/mesa/main/shader_query.cpp
>> b/src/mesa/main/shader_query.cpp
>> index 99d9e10..a5572ff 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -478,7 +478,7 @@ _mesa_program_resource_array_size(struct
>> gl_program_resource *res)
>>RESOURCE_XFB(res)->Size : 0;
>>  case GL_PROGRAM_INPUT:
>>  case GL_PROGRAM_OUTPUT:
>> -  return RESOURCE_VAR(res)->data.max_array_access;
>> +  return RESOURCE_VAR(res)->type->length;
>>  case GL_UNIFORM:
>>  case GL_VERTEX_SUBROUTINE_UNIFORM:
>>  case GL_GEOMETRY_SUBROUTINE_UNIFORM:
>> @@ -670,6 +670,57 @@ _mesa_program_resource_find_index(struct
>> gl_shader_program *shProg,
>>  return NULL;
>>   }
>>   +/* Function returns if resource name is expected to have index
>> + * appended into it.
>> + *
>> + *
>> + * Page 61 (page 73 of the PDF) in section 2.11 of the OpenGL ES 3.0
>> + * spec says:
>> + *
>> + * "If the active uniform is an array, the uniform name returned in
>> + * name will always be the name of the uniform array appended with
>> + * "[0]"."
>> + *
>> + * The same text also appears in the OpenGL 4.2 spec.  It does not,
>> + * however, appear in any previous spec.  Previous specifications are
>> + * ambiguous in this regard.  However, either name can later be passed
>> + * to glGetUniformLocation (and related APIs), so there shouldn't be any
>> + * harm in always appending "[0]" to uniform array names.
>> + *
>> + * Geometry shader stage has different naming convention where the
>> 'normal'
>> + * condition is an array, therefore for variables referenced in geometry
>> + * stage we do not add '[0]'.
>> + *
>> + * Note, that TCS outputs and TES inputs should not have index appended
>> + * either.
>> + */
>> +bool
>> +add_index_to_name(struct gl_program_resource *res)
>
> static please!
>
> With this, you get:
> Reviewed-by: Martin Peres 
>
> Thanks for the cleanup!
>
Would this be mesa-stable worthy material ?

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] configure.ac: handle llvm built with cmake in one shared library.

2015-09-29 Thread Emil Velikov

On Tuesday, 29 September 2015, Dave Airlie  wrote:

> On 29 September 2015 at 11:31, Tom Stellard  > wrote:
> > On Mon, Sep 28, 2015 at 03:15:27PM +0100, Emil Velikov wrote:
> >> Hi Laurent,
> >>
> >> On 27 September 2015 at 13:21, Laurent Carlier  > wrote:
> >> > llvm can be built with cmake in a libray with the name
> libLLVM.so.$version
> >> >
> >> Do you know if this is intentional and consistent (i.e. all the build
> >> produce such library/ies) behaviour ?
> >>
> >
> > The name of the library depends on the build system used and not the
> > version of LLVM.  There was a flag added to llvm-config in 3.8 to
> > report which build system was used.
>
> So you're saying that, regardless of the version, the autoconf llvm build
produces libllvm-$version.so as opposed to libllvm.so.$version with cmake ?
How is that even supposed to work - it seems horribly broken imho.


> This is totally broken upstream, and I suggest we don't facilitate the
> cmake
> behaviour in mesa until it just does what the autoconf build does.
>
> libLLVM.so.3.7 is not going to be compatible ABI wise with libLLVM.so.3.8
> but ldconfig versioning will consider it to be.
>
> the llvm cmake behaviour is just broken and should be fixed upstream.
>
> I second that. This must be fixed upsteam. The autoconf builds even
allows you to install multiple versions in parallel, as needed.

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

Re: [Mesa-dev] [PATCH] mesa: fix ARRAY_SIZE query for GetProgramResourceiv

2015-09-29 Thread Tapani Pälli




On 09/29/2015 01:42 PM, Emil Velikov wrote:

Hi guys,

On 29 September 2015 at 10:30, Martin Peres
 wrote:

On 28/09/15 13:51, Tapani Pälli wrote:


Patch also refactors name length queries which were using array size
in computation, this has to be done in same time to avoid regression in
arb_program_interface_query-resource-query Piglit test.

Fixes rest of the failures with
 ES31-CTS.program_interface_query.no-locations

v2: make additional check only for GS inputs
v3: create helper function for resource name length
  so that it gets calculated only in one place

Signed-off-by: Tapani Pälli 
Reviewed-by: Martin Peres  [v2]
---
   src/mesa/main/program_resource.c |  8 ++--
   src/mesa/main/shader_query.cpp   | 94

   src/mesa/main/shaderapi.h|  3 ++
   3 files changed, 62 insertions(+), 43 deletions(-)

diff --git a/src/mesa/main/program_resource.c
b/src/mesa/main/program_resource.c
index c609abe..eb71fdd 100644
--- a/src/mesa/main/program_resource.c
+++ b/src/mesa/main/program_resource.c
@@ -111,11 +111,9 @@ _mesa_GetProgramInterfaceiv(GLuint program, GLenum
programInterface,
 for (i = 0, *params = 0; i < shProg->NumProgramResourceList; i++)
{
if (shProg->ProgramResourceList[i].Type != programInterface)
   continue;
- const char *name =
-_mesa_program_resource_name(>ProgramResourceList[i]);
- unsigned array_size =
-
_mesa_program_resource_array_size(>ProgramResourceList[i]);
- *params = MAX2(*params, strlen(name) + (array_size ? 3 : 0) +
1);
+ unsigned len =
+
_mesa_program_resource_name_len(>ProgramResourceList[i]);
+ *params = MAX2(*params, len + 1);
 }
 break;
  case GL_MAX_NUM_ACTIVE_VARIABLES:
diff --git a/src/mesa/main/shader_query.cpp
b/src/mesa/main/shader_query.cpp
index 99d9e10..a5572ff 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -478,7 +478,7 @@ _mesa_program_resource_array_size(struct
gl_program_resource *res)
RESOURCE_XFB(res)->Size : 0;
  case GL_PROGRAM_INPUT:
  case GL_PROGRAM_OUTPUT:
-  return RESOURCE_VAR(res)->data.max_array_access;
+  return RESOURCE_VAR(res)->type->length;
  case GL_UNIFORM:
  case GL_VERTEX_SUBROUTINE_UNIFORM:
  case GL_GEOMETRY_SUBROUTINE_UNIFORM:
@@ -670,6 +670,57 @@ _mesa_program_resource_find_index(struct
gl_shader_program *shProg,
  return NULL;
   }
   +/* Function returns if resource name is expected to have index
+ * appended into it.
+ *
+ *
+ * Page 61 (page 73 of the PDF) in section 2.11 of the OpenGL ES 3.0
+ * spec says:
+ *
+ * "If the active uniform is an array, the uniform name returned in
+ * name will always be the name of the uniform array appended with
+ * "[0]"."
+ *
+ * The same text also appears in the OpenGL 4.2 spec.  It does not,
+ * however, appear in any previous spec.  Previous specifications are
+ * ambiguous in this regard.  However, either name can later be passed
+ * to glGetUniformLocation (and related APIs), so there shouldn't be any
+ * harm in always appending "[0]" to uniform array names.
+ *
+ * Geometry shader stage has different naming convention where the
'normal'
+ * condition is an array, therefore for variables referenced in geometry
+ * stage we do not add '[0]'.
+ *
+ * Note, that TCS outputs and TES inputs should not have index appended
+ * either.
+ */
+bool
+add_index_to_name(struct gl_program_resource *res)


static please!

With this, you get:
Reviewed-by: Martin Peres 

Thanks for the cleanup!


Would this be mesa-stable worthy material ?


We haven't had bugs filed for this but I guess it's just matter of time. 
Yes please!


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

Re: [Mesa-dev] [PATCH] mesa/meta: Use interpolateAtSample for 16x MSAA copy blit

2015-09-29 Thread Neil Roberts

Ilia Mirkin  writes:

> A couple of fairly generic comments:
>
> - It is not at all clear to me why it's OK to interpolate at sample 0

Yes, this was cheating a little bit. At least on Intel hardware the
samples are supposed to be sorted by order of distance from the centre
so sample 0 will be the furthest from the edges.

> -- this was previously interpolated at the sample, but apparently it
> doesn't matter where it's interpolated? Why not? [A future reader of
> the code might have a similar question.]

It doesn't matter where it's interpolated within the pixel because the
coordinates get clamped to an integer anyway so anywhere within the
pixel will result in the same texture coordinates. I think this is not
made any less clear by my patch so I'm inclined to leave the long
comment as it is.

> - I think that it should be possible to force interpolating at pixel
> center -- by having any varying with the 'sample' keyword, all the
> other ones don't end up getting per-sample interpolated. Not 100% sure
> though.

Hrm, that could work. I tried to find some clarification of where the
input is interpolated if neither centroid nor sample is specified but I
couldn't find any explicit mention so perhaps it is
implementation-dependant.

It just occured to me that we can use interpolateAtOffset to force it to
interpolate the inputs at the pixel centre instead of
interpolateAtSample. This avoids the problem of having to pick a sample
location that we can assume isn't on the edge. I think this is probably
the most explicit solution so I will post a v2 with that.

Thanks for looking at the patch.

Regards,
- Neil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2] i965/vec4/nir: add nir_intrinsic_memory_barrier support

2015-09-29 Thread Samuel Iglesias Gonsalvez

Fix OpenGL ES 3.1 conformance tests: advanced-readWrite-case1-vsfs
and advanced-matrix-vsfs.

v2:
- Fix SHADER_OPCODE_MEMORY_FENCE emission and the allocation of 'tmp'
  (Francisco).

Signed-off-by: Samuel Iglesias Gonsalvez 
Tested-by: Tapani Pälli 
Cc: Francisco Jerez 
---
 src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
index 94906d2..71a4dd2 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
@@ -921,6 +921,15 @@ vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr 
*instr)
   break;
}
 
+   case nir_intrinsic_memory_barrier: {
+  const vec4_builder bld =
+ vec4_builder(this).at_end().annotate(current_annotation, base_ir);
+  dst_reg tmp = bld.vgrf(BRW_REGISTER_TYPE_UD, 2);
+  bld.emit(SHADER_OPCODE_MEMORY_FENCE, tmp)
+ ->regs_written = 2;
+  break;
+   }
+
default:
   unreachable("Unknown intrinsic");
}
-- 
2.1.4

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

[Mesa-dev] [PATCH v2] glsl: apply shader storage block member rules when adding program resources

2015-09-29 Thread Samuel Iglesias Gonsalvez

From ARB_program_interface_query:

"For an active shader storage block member declared as an array, an
 entry will be generated only for the first array element, regardless
 of its type. For arrays of aggregate types, the enumeration rules are
 applied recursively for the single enumerated array element."

v2:
- Simplify 'if' conditions and return true if it is not a buffer
  variable, because these rules only apply to buffer variables (Timothy).

Signed-off-by: Samuel Iglesias Gonsalvez 
---
 src/glsl/linker.cpp | 58 +
 1 file changed, 58 insertions(+)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 87c7d4b..dbf300a 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -3134,6 +3134,60 @@ check_explicit_uniform_locations(struct gl_context *ctx,
 }
 
 static bool
+should_add_buffer_variable(struct gl_shader_program *shProg,
+   GLenum type, const char *name)
+{
+   bool found_interface = false;
+   const char *block_name = NULL;
+
+   /* These rules only apply to buffer variables. So we return
+* true for the rest of types.
+*/
+   if (type != GL_BUFFER_VARIABLE)
+  return true;
+
+   for (unsigned i = 0; i < shProg->NumBufferInterfaceBlocks; i++) {
+  block_name = shProg->UniformBlocks[i].Name;
+  if (strncmp(block_name, name, strlen(block_name)) == 0) {
+ found_interface = true;
+ break;
+  }
+   }
+
+   /* We remove the interface name from the buffer variable name,
+* including the dot that follows it.
+*/
+   if (found_interface)
+  name = name + strlen(block_name) + 1;
+
+   /* From: ARB_program_interface_query extension:
+*
+*  "For an active shader storage block member declared as an array, an
+*   entry will be generated only for the first array element, regardless
+*   of its type.  For arrays of aggregate types, the enumeration rules are
+*   applied recursively for the single enumerated array element.
+*/
+   const char *first_dot = strchr(name, '.');
+   const char *first_square_bracket = strchr(name, '[');
+
+   /* The buffer variable is on top level and it is not an array */
+   if (!first_square_bracket) {
+  return true;
+   /* The shader storage block member is a struct, then generate the entry */
+   } else if (first_dot && first_dot < first_square_bracket) {
+  return true;
+   } else {
+  /* Shader storage block member is an array, only generate an entry for 
the
+   * first array element.
+   */
+  if (strncmp(first_square_bracket, "[0]", 3) == 0)
+ return true;
+   }
+
+   return false;
+}
+
+static bool
 add_program_resource(struct gl_shader_program *prog, GLenum type,
  const void *data, uint8_t stages)
 {
@@ -3412,6 +3466,10 @@ build_program_resource_list(struct gl_shader_program 
*shProg)
 
   bool is_shader_storage =  shProg->UniformStorage[i].is_shader_storage;
   GLenum type = is_shader_storage ? GL_BUFFER_VARIABLE : GL_UNIFORM;
+  if (!should_add_buffer_variable(shProg, type,
+  shProg->UniformStorage[i].name))
+ continue;
+
   if (!add_program_resource(shProg, type,
 >UniformStorage[i], stageref))
  return;
-- 
2.1.4

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

[Mesa-dev] [PATCH] glsl: assert base_alignment > 0 for records

2015-09-29 Thread Samuel Iglesias Gonsalvez

From GLSL 1.50 spec, section 4.1.8 "Structures":

"Structures must have at least one member declaration."

So the base_alignment should be higher than zero.

Signed-off-by: Samuel Iglesias Gonsalvez 
Cc: Ilia Mirkin 
---
 src/glsl/glsl_types.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index 0ead0f2..8586b2e 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -1511,6 +1511,7 @@ glsl_type::std430_base_alignment(bool row_major) const
  base_alignment = MAX2(base_alignment,

field_type->std430_base_alignment(field_row_major));
   }
+  assert(base_alignment > 0);
   return base_alignment;
}
assert(!"not reached");
-- 
2.1.4

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

Re: [Mesa-dev] glx-tls: Visibility hidden attribute and fix x86/x86_64 tls/tdsentry points

2015-09-29 Thread Marc Dietrich

Am Samstag, 26. September 2015, 04:47:19 schrieb Sedat Dilek:
> On Sat, Sep 26, 2015 at 1:59 AM, Sedat Dilek  wrote:
> > Hi,
> > 
> > unfortunately, we still have no cool upstream fix for
> > -fvisibility=hidden compiler flag handling.
> > There is still no visibility-attribute "hidden" available or defined.
> > 
> > I see people again fell over this issue [2].
> > 
> > I have one concern...
> > 
> > GCC uses "default" visibility-attribute (defined as PUBLIC) when
> > compiler-flag "-fvisibility=hidden" is used.
> > But CLANG needs in some cases "hidden" visibility-attribute.
> > 
> > [ src/util/macros.h ]
> > 
> > /**
> > 
> >  * PUBLIC/USED macros
> >  *
> >  * If we build the library with gcc's -fvisibility=hidden flag, we'll
> >  * use the PUBLIC macro to mark functions that are to be exported.
> >  *
> >  * We also need to define a USED attribute, so the optimizer doesn't
> >  * inline a static function that we later use in an alias. - ajax
> >  */
> > 
> > #ifndef PUBLIC
> > #  if defined(__GNUC__) || (defined(__SUNPRO_C) && (__SUNPRO_C >= 0x590))
> > #define PUBLIC __attribute__((visibility("default")))
> > #define USED __attribute__((used))
> > #  elif defined(_MSC_VER)
> > #define PUBLIC __declspec(dllexport)
> > #define USED
> > #  else
> > #define PUBLIC
> > #define USED
> > #  endif
> > #endif
> > 
> > #ifdef HAVE_FUNC_ATTRIBUTE_VISIBILITY
> > #define HIDDEN __attribute__((visibility("hidden")))
> > #else
> > #define HIDDEN
> > #endif
> > 
> > [ src/util/macros.h ]
> > 
> > Alan pointed to a solution like in [4] ("Use clang's __has_attribute
> > to check for attribute support")
> > 
> > So, what can people do to help to nail this down?
> > 
> > I have attached a refreshed version of Marc's and added the TDS
> > snippet from [2] to 0002 patch.
> > ( Patches have no changelog and are against mesa v10.6.8. )
> > 
> > Hope this helps!
> > 
> > Regards,
> > - Sedat -
> > 
> > [1]
> > https://www.mail-archive.com/mesa-dev@lists.freedesktop.org/msg76122.html
> > [2] http://patchwork.freedesktop.org/patch/49494/
> > [3]
> > https://developer.apple.com/library/mac/documentation/DeveloperTools/Conc
> > eptual/CppRuntimeEnv/Articles/SymbolVisibility.html [4]
> > http://cgit.freedesktop.org/xorg/proto/x11proto/commit/?id=ffd4a13042d24c
> > b5c
> So, I played a bit more and the result is a v3 of both patches with
> some simplification etc.
> 
> Emil asked in another thread to check with --enable-asm and
> --{en|dis}able-shared-glapi, but disabling for the latter was not
> suitable for my setup (disabling OpenGL resulted in a conflict with
> dri-drivers etc.).
> 
> Brian asked if this works with GCC v4.9 - yes.

As expressed before, using hidden attribute only hides some hack on how to 
find the head of the dispatch entry table afaict.

I just replaced it with shared_dispatch_stub_0() and this works for shared-
glapi case, but not without shared-api (the first function is named 
differently). IMHO, a better fix would be to mark the head of the dispatch 
table for all cases and use this mark instead of the asm hack.

Marc






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

Re: [Mesa-dev] [PATCH v2] i965/vec4/nir: add nir_intrinsic_memory_barrier support

2015-09-29 Thread Francisco Jerez

Samuel Iglesias Gonsalvez  writes:

> Fix OpenGL ES 3.1 conformance tests: advanced-readWrite-case1-vsfs
> and advanced-matrix-vsfs.
>
> v2:
> - Fix SHADER_OPCODE_MEMORY_FENCE emission and the allocation of 'tmp'
>   (Francisco).
>
> Signed-off-by: Samuel Iglesias Gonsalvez 
> Tested-by: Tapani Pälli 
> Cc: Francisco Jerez 
> ---
>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> index 94906d2..71a4dd2 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> @@ -921,6 +921,15 @@ vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr 
> *instr)
>break;
> }
>  
> +   case nir_intrinsic_memory_barrier: {
> +  const vec4_builder bld =
> + vec4_builder(this).at_end().annotate(current_annotation, base_ir);
> +  dst_reg tmp = bld.vgrf(BRW_REGISTER_TYPE_UD, 2);

This could be declared const, with that fixed:

Reviewed-by: Francisco Jerez 

> +  bld.emit(SHADER_OPCODE_MEMORY_FENCE, tmp)
> + ->regs_written = 2;
> +  break;
> +   }
> +
> default:
>unreachable("Unknown intrinsic");
> }
> -- 
> 2.1.4


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

Re: [Mesa-dev] [PATCH] mesa: fix ARRAY_SIZE query for GetProgramResourceiv

2015-09-29 Thread Martin Peres




On 28/09/15 13:51, Tapani Pälli wrote:

Patch also refactors name length queries which were using array size
in computation, this has to be done in same time to avoid regression in
arb_program_interface_query-resource-query Piglit test.

Fixes rest of the failures with
ES31-CTS.program_interface_query.no-locations

v2: make additional check only for GS inputs
v3: create helper function for resource name length
 so that it gets calculated only in one place

Signed-off-by: Tapani Pälli 
Reviewed-by: Martin Peres  [v2]
---
  src/mesa/main/program_resource.c |  8 ++--
  src/mesa/main/shader_query.cpp   | 94 
  src/mesa/main/shaderapi.h|  3 ++
  3 files changed, 62 insertions(+), 43 deletions(-)

diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
index c609abe..eb71fdd 100644
--- a/src/mesa/main/program_resource.c
+++ b/src/mesa/main/program_resource.c
@@ -111,11 +111,9 @@ _mesa_GetProgramInterfaceiv(GLuint program, GLenum 
programInterface,
for (i = 0, *params = 0; i < shProg->NumProgramResourceList; i++) {
   if (shProg->ProgramResourceList[i].Type != programInterface)
  continue;
- const char *name =
-_mesa_program_resource_name(>ProgramResourceList[i]);
- unsigned array_size =
-_mesa_program_resource_array_size(>ProgramResourceList[i]);
- *params = MAX2(*params, strlen(name) + (array_size ? 3 : 0) + 1);
+ unsigned len =
+_mesa_program_resource_name_len(>ProgramResourceList[i]);
+ *params = MAX2(*params, len + 1);
}
break;
 case GL_MAX_NUM_ACTIVE_VARIABLES:
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 99d9e10..a5572ff 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -478,7 +478,7 @@ _mesa_program_resource_array_size(struct 
gl_program_resource *res)
   RESOURCE_XFB(res)->Size : 0;
 case GL_PROGRAM_INPUT:
 case GL_PROGRAM_OUTPUT:
-  return RESOURCE_VAR(res)->data.max_array_access;
+  return RESOURCE_VAR(res)->type->length;
 case GL_UNIFORM:
 case GL_VERTEX_SUBROUTINE_UNIFORM:
 case GL_GEOMETRY_SUBROUTINE_UNIFORM:
@@ -670,6 +670,57 @@ _mesa_program_resource_find_index(struct gl_shader_program 
*shProg,
 return NULL;
  }
  
+/* Function returns if resource name is expected to have index

+ * appended into it.
+ *
+ *
+ * Page 61 (page 73 of the PDF) in section 2.11 of the OpenGL ES 3.0
+ * spec says:
+ *
+ * "If the active uniform is an array, the uniform name returned in
+ * name will always be the name of the uniform array appended with
+ * "[0]"."
+ *
+ * The same text also appears in the OpenGL 4.2 spec.  It does not,
+ * however, appear in any previous spec.  Previous specifications are
+ * ambiguous in this regard.  However, either name can later be passed
+ * to glGetUniformLocation (and related APIs), so there shouldn't be any
+ * harm in always appending "[0]" to uniform array names.
+ *
+ * Geometry shader stage has different naming convention where the 'normal'
+ * condition is an array, therefore for variables referenced in geometry
+ * stage we do not add '[0]'.
+ *
+ * Note, that TCS outputs and TES inputs should not have index appended
+ * either.
+ */
+bool
+add_index_to_name(struct gl_program_resource *res)

static please!

With this, you get:
Reviewed-by: Martin Peres 

Thanks for the cleanup!

+{
+   bool add_index = !(((res->Type == GL_PROGRAM_INPUT) &&
+   res->StageReferences & (1 << MESA_SHADER_GEOMETRY)));
+
+   /* Transform feedback varyings have array index already appended
+* in their names.
+*/
+   if (res->Type == GL_TRANSFORM_FEEDBACK_VARYING)
+  add_index = false;
+
+   return add_index;
+}
+
+/* Get name length of a program resource. This consists of
+ * base name + 3 for '[0]' if resource is an array.
+ */
+extern unsigned
+_mesa_program_resource_name_len(struct gl_program_resource *res)
+{
+   unsigned length = strlen(_mesa_program_resource_name(res));
+   if (_mesa_program_resource_array_size(res) && add_index_to_name(res))
+  length += 3;
+   return length;
+}
+
  /* Get full name of a program resource.
   */
  bool
@@ -705,36 +756,7 @@ _mesa_get_program_resource_name(struct gl_shader_program 
*shProg,
  
 _mesa_copy_string(name, bufSize, length, _mesa_program_resource_name(res));
  
-   /* Page 61 (page 73 of the PDF) in section 2.11 of the OpenGL ES 3.0

-* spec says:
-*
-* "If the active uniform is an array, the uniform name returned in
-* name will always be the name of the uniform array appended with
-* "[0]"."
-*
-* The same text also appears in the OpenGL 4.2 spec.  It does not,
-* however, appear in any previous spec.  Previous specifications are
-* ambiguous in

[Mesa-dev] [PATCH] util: use strnlen() in strndup() implementations

2015-09-29 Thread Samuel Iglesias Gonsalvez

If the string being copied is not NULL-terminated the result of
strlen() is undefined.

Signed-off-by: Samuel Iglesias Gonsalvez 
---
 src/util/ralloc.c  | 5 +
 src/util/strndup.c | 6 ++
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/src/util/ralloc.c b/src/util/ralloc.c
index 01719c8..e07fce7 100644
--- a/src/util/ralloc.c
+++ b/src/util/ralloc.c
@@ -359,10 +359,7 @@ ralloc_strndup(const void *ctx, const char *str, size_t 
max)
if (unlikely(str == NULL))
   return NULL;
 
-   n = strlen(str);
-   if (n > max)
-  n = max;
-
+   n = strnlen(str, max);
ptr = ralloc_array(ctx, char, n + 1);
memcpy(ptr, str, n);
ptr[n] = '\0';
diff --git a/src/util/strndup.c b/src/util/strndup.c
index 2c24d37..5ceb32f 100644
--- a/src/util/strndup.c
+++ b/src/util/strndup.c
@@ -23,6 +23,7 @@
 
 #if defined(_WIN32)
 #include 
+#include 
 #include "strndup.h"
 
 char *
@@ -34,10 +35,7 @@ strndup(const char *str, size_t max)
if (!str)
   return NULL;
 
-   n = strlen(str);
-   if (n > max)
-  n = max;
-
+   n = strnlen(str, max);
ptr = (char *) calloc(n + 1, sizeof(char));
if (!ptr)
   return NULL;
-- 
2.1.4

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

Re: [Mesa-dev] List of unsupported extensions per driver

2015-09-29 Thread Marek Olšák

On Tue, Sep 29, 2015 at 4:48 PM, Romain Failliot
 wrote:
> What I don't understand is that all the lines starting with a "-" seems to
> be part of the GL_ARB_gpu_shader5 extension. See the line here:
> http://cgit.freedesktop.org/mesa/mesa/tree/docs/GL3.txt#n99
>
> If I'm right, it means that, considering Ilia's web site, GL_ARB_gpu_shader5
> is unsupported by R600, but everything in its sublist is supported. You see
> why it is confusing?

No, not everything is supported. GS streams aren't.

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

[Mesa-dev] [PATCH 3/3] mesa: clean up #includes in shaderapi.c

2015-09-29 Thread Brian Paul

---
 src/mesa/main/shaderapi.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 418121d..82a1ec3 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -37,6 +37,7 @@
  */
 
 
+#include 
 #include "main/glheader.h"
 #include "main/context.h"
 #include "main/dispatch.h"
@@ -48,17 +49,16 @@
 #include "main/shaderobj.h"
 #include "main/transformfeedback.h"
 #include "main/uniforms.h"
+#include "glsl/glsl_parser_extras.h"
+#include "glsl/ir.h"
+#include "glsl/ir_uniform.h"
+#include "glsl/program.h"
 #include "program/program.h"
 #include "program/prog_print.h"
 #include "program/prog_parameter.h"
 #include "util/ralloc.h"
 #include "util/hash_table.h"
 #include "util/mesa-sha1.h"
-#include 
-#include "../glsl/glsl_parser_extras.h"
-#include "../glsl/ir.h"
-#include "../glsl/ir_uniform.h"
-#include "../glsl/program.h"
 
 
 /**
-- 
1.9.1

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

[Mesa-dev] [PATCH 2/3] mesa: clean up the #includes in shader_query.cpp

2015-09-29 Thread Brian Paul

---
 src/mesa/main/shader_query.cpp | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 16b43e8..73dee85 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -30,14 +30,14 @@
 
 #include "main/context.h"
 #include "main/core.h"
-#include "glsl_symbol_table.h"
-#include "ir.h"
-#include "shaderapi.h"
-#include "shaderobj.h"
-#include "program/hash_table.h"
-#include "../glsl/program.h"
-#include "uniforms.h"
 #include "main/enums.h"
+#include "main/shaderapi.h"
+#include "main/shaderobj.h"
+#include "main/uniforms.h"
+#include "glsl/glsl_symbol_table.h"
+#include "glsl/ir.h"
+#include "glsl/program.h"
+#include "program/hash_table.h"
 #include "util/strndup.h"
 
 
-- 
1.9.1

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

Re: [Mesa-dev] SSBO's in UniformBlocks list?

2015-09-29 Thread Ilia Mirkin

On Tue, Sep 29, 2015 at 4:33 AM, Iago Toral  wrote:
> Hi ilia,
>
> On Tue, 2015-09-29 at 03:53 -0400, Ilia Mirkin wrote:
>> Hi Samuel, and any other onlookers,
>>
>> I was wondering why the decision was made to stick SSBO's onto the
>> same list as constbufs. Seems like they're entirely separate entities,
>> no? Perhaps I'm missing something?
>
> The reason for this was that there is a lot of code in the compiler to
> handle uniform blocks and all the rules for them and we needed the same
> treatment for SSBOs, so that seemed like a reasonable way forward to
> reuse a lot of the code in the compiler front end. I think the only
> place where we needed to make explicit distinctions is when we check for
> resource limits, since these are different for UBOs and SSBOs of course.
> Although UBOs and SSBOs are separate entities they have a lot of
> similarities too, so that did not look like a terrible idea, considering
> the benefits.

My concern is around indexing... now the per-stage indices are in the
combined UBO/SSBO space -- how do I tease out the individual ones?
Easy enough when you can loop over NumUniformBlocks and just count the
right type, but what about in the shader, where I get the buffer index
in a ir_rvalue?

Thanks,

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

[Mesa-dev] [PATCH] configure.ac: handle llvm built with cmake in one shared library

2015-09-29 Thread Laurent Carlier

llvm can be built with cmake in a libray with the name libLLVM.so.$version
Tested with both llvm-3.7.0 and llvm-3.8.0svn

v2: check and use llvm build-system feature, update comments

Signed-off-by: Laurent Carlier 
---
 configure.ac | 60 +---
 1 file changed, 37 insertions(+), 23 deletions(-)

diff --git a/configure.ac b/configure.ac
index 217281f..6101836 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2188,39 +2188,53 @@ dnl by calling llvm-config --libs 
${DRIVER_LLVM_COMPONENTS}, but
 dnl this was causing the same libraries to be appear multiple times
 dnl in LLVM_LIBS.
 
+llvm_cmake_check_libs() {
+dnl If LLVM was built with CMake, there will be one shared object per
+dnl component or one shared object.
+LLVM_SO_NAME=LLVM.$IMP_LIB_EXT.`$LLVM_CONFIG --version`
+AS_IF([test -f "$LLVM_LIBDIR/lib$LLVM_SO_NAME"], [llvm_have_one_so=yes])
+
+if test "x$llvm_have_one_so" = xyes; then
+dnl LLVM was built with cmake with only one shared versioned object.
+LLVM_LIBS="-l:lib$LLVM_SO_NAME"
+else
+AS_IF([test ! -f "$LLVM_LIBDIR/libLLVMTarget.$IMP_LIB_EXT"],
+ [AC_MSG_ERROR([Could not find llvm shared libraries:
+   Please make sure you have built llvm with the --enable-shared option
+   and that your llvm libraries are installed in $LLVM_LIBDIR
+   If you have installed your llvm libraries to a different directory you
+   can use the --with-llvm-prefix= configure flag to specify this 
directory.
+   NOTE: Mesa is attempting to use llvm shared libraries by default.
+   If you do not want to build with llvm shared libraries and instead want 
to
+   use llvm static libraries then add --disable-llvm-shared-libs to your 
configure
+   invocation and rebuild.])])
+fi
+}
+
 if test "x$MESA_LLVM" != x0; then
 
 LLVM_LIBS="`$LLVM_CONFIG --libs ${LLVM_COMPONENTS}`"
+dnl with llvm 3.8 we can try to determine libray name from the buildsystem
+AS_IF([test "$LLVM_VERSION_INT" -ge "308"], 
[llvm_build_system="`$LLVM_CONFIG --build-system`"])
 
 dnl llvm-config may not give the right answer when llvm is a built as a
 dnl single shared library, so we must work the library name out for
 dnl ourselves.
 dnl (See https://llvm.org/bugs/show_bug.cgi?id=6823)
 if test "x$enable_llvm_shared_libs" = xyes; then
-dnl We can't use $LLVM_VERSION because it has 'svn' stripped out,
-LLVM_SO_NAME=LLVM-`$LLVM_CONFIG --version`
-AS_IF([test -f "$LLVM_LIBDIR/lib$LLVM_SO_NAME.$IMP_LIB_EXT"], 
[llvm_have_one_so=yes])
-
-if test "x$llvm_have_one_so" = xyes; then
-dnl LLVM was built using auto*, so there is only one shared object.
-LLVM_LIBS="-l$LLVM_SO_NAME"
+if test "x$llvm_build_system" = xcmake; then
+llvm_cmake_check_libs
 else
-dnl If LLVM was built with CMake, there will be one shared object 
per
-dnl component.
-AS_IF([test ! -f "$LLVM_LIBDIR/libLLVMTarget.$IMP_LIB_EXT"],
-[AC_MSG_ERROR([Could not find llvm shared libraries:
-   Please make sure you have built llvm with the --enable-shared option
-   and that your llvm libraries are installed in $LLVM_LIBDIR
-   If you have installed your llvm libraries to a different directory you
-   can use the --with-llvm-prefix= configure flag to specify this 
directory.
-   NOTE: Mesa is attempting to use llvm shared libraries by default.
-   If you do not want to build with llvm shared libraries and instead want 
to
-   use llvm static libraries then add --disable-llvm-shared-libs to your 
configure
-   invocation and rebuild.])])
-
-   dnl We don't need to update LLVM_LIBS in this case because the LLVM
-   dnl install uses a shared object for each component and we have
-   dnl already added all of these objects to LLVM_LIBS.
+dnl We can't use $LLVM_VERSION because it has 'svn' stripped out,
+LLVM_SO_NAME=LLVM-`$LLVM_CONFIG --version`
+AS_IF([test -f "$LLVM_LIBDIR/lib$LLVM_SO_NAME.$IMP_LIB_EXT"], 
[llvm_have_one_so=yes])
+
+if test "x$llvm_have_one_so" = xyes; then
+dnl LLVM was built using auto*, so there is only one shared 
object.
+LLVM_LIBS="-l$LLVM_SO_NAME"
+else
+llvm_cmake_check_libs
+fi
 fi
 else
 AC_MSG_WARN([Building mesa with statically linked LLVM may cause 
compilation issues])
-- 
2.6.0

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

Re: [Mesa-dev] List of unsupported extensions per driver

2015-09-29 Thread Romain Failliot

What I don't understand is that all the lines starting with a "-" seems to
be part of the GL_ARB_gpu_shader5 extension. See the line here:
http://cgit.freedesktop.org/mesa/mesa/tree/docs/GL3.txt#n99

If I'm right, it means that, considering Ilia's web site,
GL_ARB_gpu_shader5 is unsupported by R600, but everything in its sublist is
supported. You see why it is confusing?
Le 29 sept. 2015 8:06 AM, "Marek Olšák"  a écrit :

> FMA isn't required really. R600 is mainly missing GS streams, which
> are complete on the mailing somewhere I think.
>
> Marek
>
> On Tue, Sep 29, 2015 at 7:32 AM, Romain Failliot
>  wrote:
> > Hi!
> >
> > I'm diving into the unsupported extensions list and I'm wondering how is
> it
> > possible that GL_ARB_gpu_shader5 is unsupported for R600, but some of the
> > "sub-extensions" like "Dynamically uniform sampler array indices" are
> > supported nonetheless.
> >
> > That makes me wonder if "not done" sub-extensions, like "Fused
> > multiply-add", are really not done for R600 yet, or if they are indeed
> > unsupported as the parent extension status let us suppose.
> >
> > Thx
> > Romain
> >
> > 2015-07-31 23:42 GMT+02:00 Ilia Mirkin :
> >>
> >> OK, I believe I've fixed my list up. Note that you may have to
> >> shift-reload to get the updates, I think fd.o isn't setting the proper
> >> cache headers or something else is messed up.
> >
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/3] mesa: remove an extern "C" wrapper in shader_query.cpp

2015-09-29 Thread Brian Paul

The shaderapi.h header already has the extern "C" wrapper.
---
 src/mesa/main/shader_query.cpp | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index b6d3677..16b43e8 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -32,6 +32,7 @@
 #include "main/core.h"
 #include "glsl_symbol_table.h"
 #include "ir.h"
+#include "shaderapi.h"
 #include "shaderobj.h"
 #include "program/hash_table.h"
 #include "../glsl/program.h"
@@ -39,9 +40,6 @@
 #include "main/enums.h"
 #include "util/strndup.h"
 
-extern "C" {
-#include "shaderapi.h"
-}
 
 static GLint
 program_resource_location(struct gl_shader_program *shProg,
-- 
1.9.1

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

Re: [Mesa-dev] List of unsupported extensions per driver

2015-09-29 Thread Glenn Kennard


On Tue, 29 Sep 2015 17:00:31 +0200, Marek Olšák  wrote:


On Tue, Sep 29, 2015 at 4:48 PM, Romain Failliot
 wrote:
What I don't understand is that all the lines starting with a "-" seems  
to

be part of the GL_ARB_gpu_shader5 extension. See the line here:
http://cgit.freedesktop.org/mesa/mesa/tree/docs/GL3.txt#n99

If I'm right, it means that, considering Ilia's web site,  
GL_ARB_gpu_shader5
is unsupported by R600, but everything in its sublist is supported. You  
see

why it is confusing?


No, not everything is supported. GS streams aren't.



Actually GS streams were merged a few weeks ago. gpu_shader5 isn't enabled  
yet because SB doesn't support all the features yet, and games etc getting  
unoptimized shaders when trying to use more modern GL4 features is not an  
acceptable regression.


Specifically, sampler and UBO indexing need SB support, the first i posted  
a patch for but it's not merged yet (needs someone who can grok SB code to  
review that), and the second, well, no ETA on that but work happens on it.


GL3.txt doesn't tell the whole story, its just a rough idea of whats going  
on featurewise, for the details inquire, like Romain just did :-)



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

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

Re: [Mesa-dev] [PATCH 2/2] egl/dri2: don't require a context for ClientWaitSync

2015-09-29 Thread Albert Freeman

On 29 September 2015 at 07:28, Albert Freeman  wrote:
> On 29 September 2015 at 07:25, Albert Freeman  
> wrote:
>> On 28 September 2015 at 08:10, Marek Olšák  wrote:
>>> On Mon, Sep 28, 2015 at 7:20 AM, Albert Freeman
>>>  wrote:
 On 28 September 2015 at 14:38, Albert Freeman  
 wrote:
> On 28 September 2015 at 04:12, Emil Velikov  
> wrote:
>> On 26 September 2015 at 00:49, Marek Olšák  wrote:
>>> From: Marek Olšák 
>>>
>>> The spec doesn't require it. This fixes a crash on Android.
>>>
>> Nicely spotted Marek.
>>
>> A couple of related (not supposed to be part of this patch) notes:
>>  - This will just move the crash (from libEGL to i965_dri.so) for i965
>> hardware :)
> I rediscovered this before I read the email related to this patch
> (where Marek mentions it): "Re: [Mesa-dev] Pending issues of
> lollipop-x86".
> Who (if anyone) should we CC this to?
>>  - We really shouldn't be setting the flags if ctx is NULL, as per the 
>> spec.
>>
>> "If no context is
>> current for the bound API, the EGL_SYNC_FLUSH_COMMANDS_BIT_KHR bit
>> is ignored."
> Maybe handle this in the driver (probably state_tracker in the case of
> gallium) level. Both here and drivers?
 OpenGL ClientWaitSync is very clear about SYNC_FLUSH_COMMANDS_BIT only
 flushing the GL context when the fence sync is created (it doesn’t
 seem to require it at the moment wait sync is called (probably to
 leave room for drivers to optimise)). EGL is a lot less clear, it
 could be interpreted as behaving just like GL, but to someone writing
 an EGL program it could be easily interpreted as "the flush() must
 occur when eglClientWaitSync is called with SYNC_FLUSH_COMMANDS_BIT".

 The current behaviour of gallium (not sure about i965) is to always
 flush() when creating the sync object (hence always behave as
 SYNC_FLUSH_COMMANDS_BIT is set according to GL). However
 implementations are allowed to behave as if flush was not required (in
 GL and I am assuming EGL). So technically the current behaviour of the
 gallium dri state tracker could reduce performance (since flushing
 tends to do that). It however doesn’t actually violate the EGL spec.
 Perhaps some heuristics on when to flush would be a good solution?
>>>
>>> Flushing is currently required for creating fences. This is a gallium
>>> design decision as well as radeon/amdgpu kernel driver decision. It's
>>> simply not possible to put a fence anywhere in the GPU command buffer.
>>> We can only submit a command buffer to the kernel and then get a fence
>>> tracking execution of the whole command buffer.
>>>
>>> Marek
>> Thanks for the clarification.
> Also when I said "Can easily be reverted later if a better solution is
> found. :)". It seemed to me that this was an issue better resolved in
> the code outside of mesa that had the issue.
So basically:
Sorry if I upset anyone. The only thing that has changed is that I
approve of the patch a lot more than I originally did (since my
misunderstanding was cleared up). "The spec doesn't require it." in
the original patch implies an issue in the application and not the
driver. This lead me to believe that for some reason Marek had decided
that an applications incorrect behavior should be fixed in the driver,
hence my reply (I thought Marek was thinking approximately the same
thing). I understood what the patch did and its implications within
mesa, just not what it fixed (now I understand both).
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] glsl: apply shader storage block member rules when adding program resources

2015-09-29 Thread Timothy Arceri

On Tue, 2015-09-29 at 11:02 +0200, Samuel Iglesias Gonsalvez wrote:
> From ARB_program_interface_query:
> 
> "For an active shader storage block member declared as an array, an
>  entry will be generated only for the first array element, regardless
>  of its type. For arrays of aggregate types, the enumeration rules
> are
>  applied recursively for the single enumerated array element."
> 
> v2:
> - Simplify 'if' conditions and return true if it is not a buffer
>   variable, because these rules only apply to buffer variables
> (Timothy).
> 
> Signed-off-by: Samuel Iglesias Gonsalvez 

Reviewed-by: Timothy Arceri 

> ---
>  src/glsl/linker.cpp | 58
> +
>  1 file changed, 58 insertions(+)
> 
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 87c7d4b..dbf300a 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -3134,6 +3134,60 @@ check_explicit_uniform_locations(struct
> gl_context *ctx,
>  }
>  
>  static bool
> +should_add_buffer_variable(struct gl_shader_program *shProg,
> +   GLenum type, const char *name)
> +{
> +   bool found_interface = false;
> +   const char *block_name = NULL;
> +
> +   /* These rules only apply to buffer variables. So we return
> +* true for the rest of types.
> +*/
> +   if (type != GL_BUFFER_VARIABLE)
> +  return true;
> +
> +   for (unsigned i = 0; i < shProg->NumBufferInterfaceBlocks; i++) {
> +  block_name = shProg->UniformBlocks[i].Name;
> +  if (strncmp(block_name, name, strlen(block_name)) == 0) {
> + found_interface = true;
> + break;
> +  }
> +   }
> +
> +   /* We remove the interface name from the buffer variable name,
> +* including the dot that follows it.
> +*/
> +   if (found_interface)
> +  name = name + strlen(block_name) + 1;
> +
> +   /* From: ARB_program_interface_query extension:
> +*
> +*  "For an active shader storage block member declared as an
> array, an
> +*   entry will be generated only for the first array element,
> regardless
> +*   of its type.  For arrays of aggregate types, the enumeration
> rules are
> +*   applied recursively for the single enumerated array element.
> +*/
> +   const char *first_dot = strchr(name, '.');
> +   const char *first_square_bracket = strchr(name, '[');
> +
> +   /* The buffer variable is on top level and it is not an array */
> +   if (!first_square_bracket) {
> +  return true;
> +   /* The shader storage block member is a struct, then generate the
> entry */
> +   } else if (first_dot && first_dot < first_square_bracket) {
> +  return true;
> +   } else {
> +  /* Shader storage block member is an array, only generate an
> entry for the
> +   * first array element.
> +   */
> +  if (strncmp(first_square_bracket, "[0]", 3) == 0)
> + return true;
> +   }
> +
> +   return false;
> +}
> +
> +static bool
>  add_program_resource(struct gl_shader_program *prog, GLenum type,
>   const void *data, uint8_t stages)
>  {
> @@ -3412,6 +3466,10 @@ build_program_resource_list(struct
> gl_shader_program *shProg)
>  
>bool is_shader_storage =  shProg
> ->UniformStorage[i].is_shader_storage;
>GLenum type = is_shader_storage ? GL_BUFFER_VARIABLE :
> GL_UNIFORM;
> +  if (!should_add_buffer_variable(shProg, type,
> +  shProg
> ->UniformStorage[i].name))
> + continue;
> +
>if (!add_program_resource(shProg, type,
>  >UniformStorage[i],
> stageref))
>   return;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] SSBO's in UniformBlocks list?

2015-09-29 Thread Francisco Jerez

Ilia Mirkin  writes:

> On Tue, Sep 29, 2015 at 4:33 AM, Iago Toral  wrote:
>> Hi ilia,
>>
>> On Tue, 2015-09-29 at 03:53 -0400, Ilia Mirkin wrote:
>>> Hi Samuel, and any other onlookers,
>>>
>>> I was wondering why the decision was made to stick SSBO's onto the
>>> same list as constbufs. Seems like they're entirely separate entities,
>>> no? Perhaps I'm missing something?
>>
>> The reason for this was that there is a lot of code in the compiler to
>> handle uniform blocks and all the rules for them and we needed the same
>> treatment for SSBOs, so that seemed like a reasonable way forward to
>> reuse a lot of the code in the compiler front end. I think the only
>> place where we needed to make explicit distinctions is when we check for
>> resource limits, since these are different for UBOs and SSBOs of course.
>> Although UBOs and SSBOs are separate entities they have a lot of
>> similarities too, so that did not look like a terrible idea, considering
>> the benefits.
>
> My concern is around indexing... now the per-stage indices are in the
> combined UBO/SSBO space -- how do I tease out the individual ones?
> Easy enough when you can loop over NumUniformBlocks and just count the
> right type, but what about in the shader, where I get the buffer index
> in a ir_rvalue?
>
Yeah, this seems rather dubious to me too.  Even if you had re-used the
current gl_uniform_block type for SSBOs for the sake of code-sharing I
think it would have made more sense to split them into a different index
space, because SSBOs are a different index space at the API level and
drivers will want them to be a different index space too.

I believe that this leads to a bug the i965 implementation -- We expose
12 SSBOs per stage and 12 UBOs per stage, but we only have 12 binding
table entries reserved for the block of the binding table currently
shared among UBOs and SSBOs, so you might overflow the number of
available surface entries if the combined number of UBOs and SSBOs is
greater than 12 for some stage.

> Thanks,
>
>   -ilia
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

[Mesa-dev] [PATCH] main: Fix block index when mixing UBO and SSBO blocks

2015-09-29 Thread Iago Toral Quiroga

Since we store both in UniformBlocks, we can't just compute the index by
subtracting the array address start, we need to count the number of
buffers of the approriate type.
---

Or we can just fall back to calc_resource_index... that would also work.
This should be a bit faster though since it only traverses the list of
uniform blocks and the code is simple enough, but it probably won't make
a significant difference anyway.

 src/mesa/main/shader_query.cpp | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 0cada50..33c95b4 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -602,6 +602,22 @@ calc_resource_index(struct gl_shader_program *shProg,
return GL_INVALID_INDEX;
 }
 
+static GLuint
+calc_ubo_ssbo_index(struct gl_shader_program *shProg,
+struct gl_program_resource *res)
+{
+   unsigned i;
+   GLuint index = 0;
+   bool is_shader_storage = res->Type == GL_SHADER_STORAGE_BLOCK;
+   for (i = 0; i < shProg->NumBufferInterfaceBlocks; i++) {
+  if (>UniformBlocks[i] == RESOURCE_UBO(res))
+ return index;
+  if (shProg->UniformBlocks[i].IsShaderStorage == is_shader_storage)
+ index++;
+   }
+   return GL_INVALID_INDEX;
+}
+
 /**
  * Calculate index for the given resource.
  */
@@ -615,7 +631,7 @@ _mesa_program_resource_index(struct gl_shader_program 
*shProg,
switch (res->Type) {
case GL_UNIFORM_BLOCK:
case GL_SHADER_STORAGE_BLOCK:
-  return RESOURCE_UBO(res)- shProg->UniformBlocks;
+  return calc_ubo_ssbo_index(shProg, res);
case GL_ATOMIC_COUNTER_BUFFER:
   return RESOURCE_ATC(res) - shProg->AtomicBuffers;
case GL_TRANSFORM_FEEDBACK_VARYING:
-- 
1.9.1

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

Re: [Mesa-dev] [PATCH 2/2] egl/dri2: don't require a context for ClientWaitSync

2015-09-29 Thread Marek Olšák

On Tue, Sep 29, 2015 at 4:54 PM, Albert Freeman
 wrote:
> On 29 September 2015 at 07:28, Albert Freeman  
> wrote:
>> On 29 September 2015 at 07:25, Albert Freeman  
>> wrote:
>>> On 28 September 2015 at 08:10, Marek Olšák  wrote:
 On Mon, Sep 28, 2015 at 7:20 AM, Albert Freeman
  wrote:
> On 28 September 2015 at 14:38, Albert Freeman  
> wrote:
>> On 28 September 2015 at 04:12, Emil Velikov  
>> wrote:
>>> On 26 September 2015 at 00:49, Marek Olšák  wrote:
 From: Marek Olšák 

 The spec doesn't require it. This fixes a crash on Android.

>>> Nicely spotted Marek.
>>>
>>> A couple of related (not supposed to be part of this patch) notes:
>>>  - This will just move the crash (from libEGL to i965_dri.so) for i965
>>> hardware :)
>> I rediscovered this before I read the email related to this patch
>> (where Marek mentions it): "Re: [Mesa-dev] Pending issues of
>> lollipop-x86".
>> Who (if anyone) should we CC this to?
>>>  - We really shouldn't be setting the flags if ctx is NULL, as per the 
>>> spec.
>>>
>>> "If no context is
>>> current for the bound API, the EGL_SYNC_FLUSH_COMMANDS_BIT_KHR bit
>>> is ignored."
>> Maybe handle this in the driver (probably state_tracker in the case of
>> gallium) level. Both here and drivers?
> OpenGL ClientWaitSync is very clear about SYNC_FLUSH_COMMANDS_BIT only
> flushing the GL context when the fence sync is created (it doesn’t
> seem to require it at the moment wait sync is called (probably to
> leave room for drivers to optimise)). EGL is a lot less clear, it
> could be interpreted as behaving just like GL, but to someone writing
> an EGL program it could be easily interpreted as "the flush() must
> occur when eglClientWaitSync is called with SYNC_FLUSH_COMMANDS_BIT".
>
> The current behaviour of gallium (not sure about i965) is to always
> flush() when creating the sync object (hence always behave as
> SYNC_FLUSH_COMMANDS_BIT is set according to GL). However
> implementations are allowed to behave as if flush was not required (in
> GL and I am assuming EGL). So technically the current behaviour of the
> gallium dri state tracker could reduce performance (since flushing
> tends to do that). It however doesn’t actually violate the EGL spec.
> Perhaps some heuristics on when to flush would be a good solution?

 Flushing is currently required for creating fences. This is a gallium
 design decision as well as radeon/amdgpu kernel driver decision. It's
 simply not possible to put a fence anywhere in the GPU command buffer.
 We can only submit a command buffer to the kernel and then get a fence
 tracking execution of the whole command buffer.

 Marek
>>> Thanks for the clarification.
>> Also when I said "Can easily be reverted later if a better solution is
>> found. :)". It seemed to me that this was an issue better resolved in
>> the code outside of mesa that had the issue.
> So basically:
> Sorry if I upset anyone. The only thing that has changed is that I
> approve of the patch a lot more than I originally did (since my
> misunderstanding was cleared up). "The spec doesn't require it." in
> the original patch implies an issue in the application and not the
> driver. This lead me to believe that for some reason Marek had decided
> that an applications incorrect behavior should be fixed in the driver,
> hence my reply (I thought Marek was thinking approximately the same
> thing). I understood what the patch did and its implications within
> mesa, just not what it fixed (now I understand both).

No, this is not an application bug. It's really driver bug that was
incorrectly evaluated as an application bug.

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

[Mesa-dev] [PATCH 2/2] st/mesa: try PIPE_BIND_RENDER_TARGET when choosing float texture formats

2015-09-29 Thread Brian Paul

For 8-bit RGB(A) texture formats we set the PIPE_BIND_RENDER_TARGET flag
to try to get a hardware format which also supports rendering (for FBO
textures).  Do the same thing for floating point formats.

This allows the Redway Flat demo to run.
---
 src/mesa/state_tracker/st_format.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_format.c 
b/src/mesa/state_tracker/st_format.c
index 0c94428..144b7d6 100644
--- a/src/mesa/state_tracker/st_format.c
+++ b/src/mesa/state_tracker/st_format.c
@@ -1963,7 +1963,11 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum 
target,
else if (internalFormat == 3 || internalFormat == 4 ||
 internalFormat == GL_RGB || internalFormat == GL_RGBA ||
 internalFormat == GL_RGB8 || internalFormat == GL_RGBA8 ||
-internalFormat == GL_BGRA)
+internalFormat == GL_BGRA ||
+internalFormat == GL_RGB16F ||
+internalFormat == GL_RGBA16F ||
+internalFormat == GL_RGB32F ||
+internalFormat == GL_RGBA32F)
 bindings |= PIPE_BIND_RENDER_TARGET;
 
/* GLES allows the driver to choose any format which matches
-- 
1.9.1

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

Re: [Mesa-dev] [PATCH] util: use strnlen() in strndup() implementations

2015-09-29 Thread Neil Roberts

Looks good to me. Thanks for doing that.

Reviewed-by: Neil Roberts 

- Neil

Samuel Iglesias Gonsalvez  writes:

> If the string being copied is not NULL-terminated the result of
> strlen() is undefined.
>
> Signed-off-by: Samuel Iglesias Gonsalvez 
> ---
>  src/util/ralloc.c  | 5 +
>  src/util/strndup.c | 6 ++
>  2 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/src/util/ralloc.c b/src/util/ralloc.c
> index 01719c8..e07fce7 100644
> --- a/src/util/ralloc.c
> +++ b/src/util/ralloc.c
> @@ -359,10 +359,7 @@ ralloc_strndup(const void *ctx, const char *str, size_t 
> max)
> if (unlikely(str == NULL))
>return NULL;
>  
> -   n = strlen(str);
> -   if (n > max)
> -  n = max;
> -
> +   n = strnlen(str, max);
> ptr = ralloc_array(ctx, char, n + 1);
> memcpy(ptr, str, n);
> ptr[n] = '\0';
> diff --git a/src/util/strndup.c b/src/util/strndup.c
> index 2c24d37..5ceb32f 100644
> --- a/src/util/strndup.c
> +++ b/src/util/strndup.c
> @@ -23,6 +23,7 @@
>  
>  #if defined(_WIN32)
>  #include 
> +#include 
>  #include "strndup.h"
>  
>  char *
> @@ -34,10 +35,7 @@ strndup(const char *str, size_t max)
> if (!str)
>return NULL;
>  
> -   n = strlen(str);
> -   if (n > max)
> -  n = max;
> -
> +   n = strnlen(str, max);
> ptr = (char *) calloc(n + 1, sizeof(char));
> if (!ptr)
>return NULL;
> -- 
> 2.1.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] util: use strnlen() in strndup() implementations

2015-09-29 Thread Jose Fonseca


On 29/09/15 15:35, Samuel Iglesias Gonsalvez wrote:

If the string being copied is not NULL-terminated the result of
strlen() is undefined.

Signed-off-by: Samuel Iglesias Gonsalvez 
---
  src/util/ralloc.c  | 5 +
  src/util/strndup.c | 6 ++
  2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/src/util/ralloc.c b/src/util/ralloc.c
index 01719c8..e07fce7 100644
--- a/src/util/ralloc.c
+++ b/src/util/ralloc.c
@@ -359,10 +359,7 @@ ralloc_strndup(const void *ctx, const char *str, size_t 
max)
 if (unlikely(str == NULL))
return NULL;

-   n = strlen(str);
-   if (n > max)
-  n = max;
-
+   n = strnlen(str, max);
 ptr = ralloc_array(ctx, char, n + 1);
 memcpy(ptr, str, n);
 ptr[n] = '\0';
diff --git a/src/util/strndup.c b/src/util/strndup.c
index 2c24d37..5ceb32f 100644
--- a/src/util/strndup.c
+++ b/src/util/strndup.c
@@ -23,6 +23,7 @@

  #if defined(_WIN32)
  #include 
+#include 
  #include "strndup.h"

  char *
@@ -34,10 +35,7 @@ strndup(const char *str, size_t max)
 if (!str)
return NULL;

-   n = strlen(str);
-   if (n > max)
-  n = max;
-
+   n = strnlen(str, max);
 ptr = (char *) calloc(n + 1, sizeof(char));
 if (!ptr)
return NULL;



Reviewed-by: Jose Fonseca 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] egl/dri2: don't require a context for ClientWaitSync

2015-09-29 Thread Albert Freeman

On 29 September 2015 at 15:01, Marek Olšák  wrote:
> On Tue, Sep 29, 2015 at 4:54 PM, Albert Freeman
>  wrote:
>> On 29 September 2015 at 07:28, Albert Freeman  
>> wrote:
>>> On 29 September 2015 at 07:25, Albert Freeman  
>>> wrote:
 On 28 September 2015 at 08:10, Marek Olšák  wrote:
> On Mon, Sep 28, 2015 at 7:20 AM, Albert Freeman
>  wrote:
>> On 28 September 2015 at 14:38, Albert Freeman 
>>  wrote:
>>> On 28 September 2015 at 04:12, Emil Velikov  
>>> wrote:
 On 26 September 2015 at 00:49, Marek Olšák  wrote:
> From: Marek Olšák 
>
> The spec doesn't require it. This fixes a crash on Android.
>
 Nicely spotted Marek.

 A couple of related (not supposed to be part of this patch) notes:
  - This will just move the crash (from libEGL to i965_dri.so) for i965
 hardware :)
>>> I rediscovered this before I read the email related to this patch
>>> (where Marek mentions it): "Re: [Mesa-dev] Pending issues of
>>> lollipop-x86".
>>> Who (if anyone) should we CC this to?
  - We really shouldn't be setting the flags if ctx is NULL, as per the 
 spec.

 "If no context is
 current for the bound API, the EGL_SYNC_FLUSH_COMMANDS_BIT_KHR bit
 is ignored."
>>> Maybe handle this in the driver (probably state_tracker in the case of
>>> gallium) level. Both here and drivers?
>> OpenGL ClientWaitSync is very clear about SYNC_FLUSH_COMMANDS_BIT only
>> flushing the GL context when the fence sync is created (it doesn’t
>> seem to require it at the moment wait sync is called (probably to
>> leave room for drivers to optimise)). EGL is a lot less clear, it
>> could be interpreted as behaving just like GL, but to someone writing
>> an EGL program it could be easily interpreted as "the flush() must
>> occur when eglClientWaitSync is called with SYNC_FLUSH_COMMANDS_BIT".
>>
>> The current behaviour of gallium (not sure about i965) is to always
>> flush() when creating the sync object (hence always behave as
>> SYNC_FLUSH_COMMANDS_BIT is set according to GL). However
>> implementations are allowed to behave as if flush was not required (in
>> GL and I am assuming EGL). So technically the current behaviour of the
>> gallium dri state tracker could reduce performance (since flushing
>> tends to do that). It however doesn’t actually violate the EGL spec.
>> Perhaps some heuristics on when to flush would be a good solution?
>
> Flushing is currently required for creating fences. This is a gallium
> design decision as well as radeon/amdgpu kernel driver decision. It's
> simply not possible to put a fence anywhere in the GPU command buffer.
> We can only submit a command buffer to the kernel and then get a fence
> tracking execution of the whole command buffer.
>
> Marek
 Thanks for the clarification.
>>> Also when I said "Can easily be reverted later if a better solution is
>>> found. :)". It seemed to me that this was an issue better resolved in
>>> the code outside of mesa that had the issue.
>> So basically:
>> Sorry if I upset anyone. The only thing that has changed is that I
>> approve of the patch a lot more than I originally did (since my
>> misunderstanding was cleared up). "The spec doesn't require it." in
>> the original patch implies an issue in the application and not the
>> driver. This lead me to believe that for some reason Marek had decided
>> that an applications incorrect behavior should be fixed in the driver,
>> hence my reply (I thought Marek was thinking approximately the same
>> thing). I understood what the patch did and its implications within
>> mesa, just not what it fixed (now I understand both).
>
> No, this is not an application bug. It's really driver bug that was
> incorrectly evaluated as an application bug.
>
> Marek
What I am saying is, I know that now, I didn't then. :)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: assert base_alignment > 0 for records

2015-09-29 Thread Ilia Mirkin

On Tue, Sep 29, 2015 at 5:02 AM, Samuel Iglesias Gonsalvez
 wrote:
> From GLSL 1.50 spec, section 4.1.8 "Structures":
>
> "Structures must have at least one member declaration."
>
> So the base_alignment should be higher than zero.
>
> Signed-off-by: Samuel Iglesias Gonsalvez 
> Cc: Ilia Mirkin 
> ---
>  src/glsl/glsl_types.cpp | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index 0ead0f2..8586b2e 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -1511,6 +1511,7 @@ glsl_type::std430_base_alignment(bool row_major) const
>   base_alignment = MAX2(base_alignment,
> 
> field_type->std430_base_alignment(field_row_major));
>}
> +  assert(base_alignment > 0);

Reviewed-by: Ilia Mirkin 

>return base_alignment;
> }
> assert(!"not reached");
> --
> 2.1.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] radeonsi: don't set DATA_FORMAT if ADD_TID_ENABLE is set on VI

2015-09-29 Thread Marek Olšák

From: Marek Olšák 

This can cause incorrect address calculations and hangs.

Cc: mesa-sta...@lists.freedesktop.org
---
 src/gallium/drivers/radeonsi/si_descriptors.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
b/src/gallium/drivers/radeonsi/si_descriptors.c
index b07ab3b..0ced6c3 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -619,11 +619,14 @@ void si_set_ring_buffer(struct pipe_context *ctx, uint 
shader, uint slot,
  S_008F0C_DST_SEL_Z(V_008F0C_SQ_SEL_Z) |
  S_008F0C_DST_SEL_W(V_008F0C_SQ_SEL_W) |
  S_008F0C_NUM_FORMAT(V_008F0C_BUF_NUM_FORMAT_FLOAT) |
- S_008F0C_DATA_FORMAT(V_008F0C_BUF_DATA_FORMAT_32) |
  S_008F0C_ELEMENT_SIZE(element_size) |
  S_008F0C_INDEX_STRIDE(index_stride) |
  S_008F0C_ADD_TID_ENABLE(add_tid);
 
+   /* If ADD_TID_ENABLE is set, DATA_FORMAT specifies STRIDE bits 
[14:17] */
+   if (sctx->b.chip_class >= VI && !add_tid)
+   desc[3] |= 
S_008F0C_DATA_FORMAT(V_008F0C_BUF_DATA_FORMAT_32);
+
pipe_resource_reference(>buffers[slot], buffer);
radeon_add_to_buffer_list(>b, >b.rings.gfx,
  (struct r600_resource*)buffer,
-- 
2.1.4

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

[Mesa-dev] [Patchwork] The infrequent patchwork update #1

2015-09-29 Thread Damien Lespiau

Hi all,

You may have noticed already, patchwork.freedesktop.org looks different.

That new version includes:
  - Some re-design. Design is very much an iterative process, thoughts
and comments are welcome,
  - Showing the number of Reviewed-by, Acked-by, Tested-by tags,
  - Some cleanup of the data base, removing stale registration email
addresses (mostly bots trying to register) that were showing in the
completion list.

That's it for the first update. I wanted to keep it small-ish to do some
heavy lifting behind the scenes (latest patchwork, latest django, new db
migration system, virtualenvs, ...)

One can open issues against the freedesktop version of patchwork (not
all is upstream just yet, but hopefully on its way there) on github:

  https://github.com/dlespiau/patchwork/issues

For administrative tasks (eg. add projects/maintainers), please use
fdo's bugzilla, product freedeskop.org, component patchwork:

  https://bugs.freedesktop.org/enter_bug.cgi?product=freedesktop.org

Future plans include a "series-aware" patchwork with the goal of
exposing series and revision of series (v2, 3, ..) to the world so one
can hook automatic testing to patchwork, read more at:

  https://lists.ozlabs.org/pipermail/patchwork/2015-September/001601.html

HTH,

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

[Mesa-dev] [PATCH 1/2] st/mesa: add some debugging code in st_ChooseTextureFormat()

2015-09-29 Thread Brian Paul

I've temporarily added code like this many times.  Wrap it in a
conditional that can be enabled when needed.
---
 src/mesa/state_tracker/st_format.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_format.c 
b/src/mesa/state_tracker/st_format.c
index e3fb761..0c94428 100644
--- a/src/mesa/state_tracker/st_format.c
+++ b/src/mesa/state_tracker/st_format.c
@@ -34,6 +34,8 @@
 
 #include "main/imports.h"
 #include "main/context.h"
+#include "main/enums.h"
+#include "main/formats.h"
 #include "main/glformats.h"
 #include "main/texgetimage.h"
 #include "main/teximage.h"
@@ -1938,6 +1940,7 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum 
target,
 {
struct st_context *st = st_context(ctx);
enum pipe_format pFormat;
+   mesa_format mFormat;
unsigned bindings;
enum pipe_texture_target pTarget = gl_target_to_pipe(target);
 
@@ -2010,7 +2013,20 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum 
target,
   return MESA_FORMAT_NONE;
}
 
-   return st_pipe_format_to_mesa_format(pFormat);
+   mFormat = st_pipe_format_to_mesa_format(pFormat);
+
+   /* Debugging aid */
+   if (0) {
+  debug_printf("%s(intFormat=%s, format=%s, type=%s) -> %s, %s\n",
+   __func__,
+   _mesa_enum_to_string(internalFormat),
+   _mesa_enum_to_string(format),
+   _mesa_enum_to_string(type),
+   util_format_name(pFormat),
+   _mesa_get_format_name(mFormat));
+   }
+
+   return mFormat;
 }
 
 
-- 
1.9.1

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

Re: [Mesa-dev] [PATCH 2/2] st/mesa: try PIPE_BIND_RENDER_TARGET when choosing float texture formats

2015-09-29 Thread Marek Olšák

Cc: 10.6 11.0 
Reviewed-by: Marek Olšák 

Marek

On Tue, Sep 29, 2015 at 5:39 PM, Brian Paul  wrote:
> For 8-bit RGB(A) texture formats we set the PIPE_BIND_RENDER_TARGET flag
> to try to get a hardware format which also supports rendering (for FBO
> textures).  Do the same thing for floating point formats.
>
> This allows the Redway Flat demo to run.
> ---
>  src/mesa/state_tracker/st_format.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/state_tracker/st_format.c 
> b/src/mesa/state_tracker/st_format.c
> index 0c94428..144b7d6 100644
> --- a/src/mesa/state_tracker/st_format.c
> +++ b/src/mesa/state_tracker/st_format.c
> @@ -1963,7 +1963,11 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum 
> target,
> else if (internalFormat == 3 || internalFormat == 4 ||
>  internalFormat == GL_RGB || internalFormat == GL_RGBA ||
>  internalFormat == GL_RGB8 || internalFormat == GL_RGBA8 ||
> -internalFormat == GL_BGRA)
> +internalFormat == GL_BGRA ||
> +internalFormat == GL_RGB16F ||
> +internalFormat == GL_RGBA16F ||
> +internalFormat == GL_RGB32F ||
> +internalFormat == GL_RGBA32F)
>  bindings |= PIPE_BIND_RENDER_TARGET;
>
> /* GLES allows the driver to choose any format which matches
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] mesa: remove an extern "C" wrapper in shader_query.cpp

2015-09-29 Thread Matt Turner

All three are

Reviewed-by: Matt Turner 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] List of unsupported extensions per driver

2015-09-29 Thread Albert Freeman

On 29 September 2015 at 14:48, Romain Failliot
 wrote:
> What I don't understand is that all the lines starting with a "-" seems to
> be part of the GL_ARB_gpu_shader5 extension. See the line here:
> http://cgit.freedesktop.org/mesa/mesa/tree/docs/GL3.txt#n99
>
> If I'm right, it means that, considering Ilia's web site, GL_ARB_gpu_shader5
> is unsupported by R600, but everything in its sublist is supported. You see
> why it is confusing?
>
> Le 29 sept. 2015 8:06 AM, "Marek Olšák"  a écrit :
>>
>> FMA isn't required really. R600 is mainly missing GS streams, which
>> are complete on the mailing somewhere I think.
>>
>> Marek
>>
>> On Tue, Sep 29, 2015 at 7:32 AM, Romain Failliot
>>  wrote:
>> > Hi!
>> >
>> > I'm diving into the unsupported extensions list and I'm wondering how is
>> > it
>> > possible that GL_ARB_gpu_shader5 is unsupported for R600, but some of
>> > the
>> > "sub-extensions" like "Dynamically uniform sampler array indices" are
>> > supported nonetheless.
>> >
>> > That makes me wonder if "not done" sub-extensions, like "Fused
>> > multiply-add", are really not done for R600 yet, or if they are indeed
>> > unsupported as the parent extension status let us suppose.
>> >
>> > Thx
>> > Romain
>> >
>> > 2015-07-31 23:42 GMT+02:00 Ilia Mirkin :
>> >>
>> >> OK, I believe I've fixed my list up. Note that you may have to
>> >> shift-reload to get the updates, I think fd.o isn't setting the proper
>> >> cache headers or something else is messed up.
>> >
>> >
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
Guide to GL3.txt:
{
If a line starting with '-' has no braces next to it, it means it is
supported for all drivers (if it is marked DONE).
If a line starting with '-' has braces next to it, whatever driver/s
is in the braces has support for that feature (if any) (if it is
marked DONE).
When a driver has support for all features within an extension, it is
removed from all braces on lines starting with '-' and placed in the
braces for the extension name (the line above the first line marked
'-').

DONE just means that non driver specific support is complete (which is
required before any driver can support that feature/extension).

Usually extensions are not advertised to GL applications until they
are deemed complete (for the driver that is in use), this can be
overridden with e.g. export
MESA_EXTENSION_OVERRIDE=GL_ARB_gpu_shader5. You can also disable
advertising of a GL feature with e.g. export
MESA_EXTENSION_OVERRIDE=-GL_ARB_gpu_shader5.
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/5] i965/miptree: Rename align_w, align_h -> halign, valign

2015-09-29 Thread Chad Versace

On Sun 27 Sep 2015, Ben Widawsky wrote:
> On Fri, Sep 25, 2015 at 12:05:49PM -0700, Chad Versace wrote:
> > The values of intel_mipmap_tree::align_w and ::align_h correspond to the
> > hardware enums HALIGN_* and VALIGN_*.
> > 
> > See the confusion?
> > align_h != HALIGN
> > align_h == VALIGN
> > 
> > Reduce the confusion by renaming the variables to match the hardware
> > enum names:
> > git ls-files |
> > xargs sed -i -e 's/align_w/halign/g' \
> >  -e 's/align_h/valign/g'
> > 
> > Suggested-by: Kenneth Graunke 
> > Cc: Ben Widawsky 
> > Cc: Kenneth Graunke 
> > ---
> >  src/mesa/drivers/dri/i965/brw_tex_layout.c| 62 
> > +++
> >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  |  4 +-
> >  src/mesa/drivers/dri/i965/gen6_blorp.cpp  |  2 +-
> >  src/mesa/drivers/dri/i965/gen6_surface_state.c|  2 +-
> >  src/mesa/drivers/dri/i965/gen7_blorp.cpp  |  4 +-
> >  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c |  8 +--
> >  src/mesa/drivers/dri/i965/gen8_surface_state.c| 12 ++---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c |  2 +-
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 18 +--
> >  9 files changed, 62 insertions(+), 52 deletions(-)


> Reviewed-by: Ben Widawsky 

Is this a rb for only this patch? Or for the series?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 92183] linker.cpp:3187:46: error: ‘strtok_r’ was not declared in this scope

2015-09-29 Thread bugzilla-daemon

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

Bug ID: 92183
   Summary: linker.cpp:3187:46: error: ‘strtok_r’ was not declared
in this scope
   Product: Mesa
   Version: git
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Keywords: bisected, regression
  Severity: normal
  Priority: medium
 Component: Mesa core
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: v...@freedesktop.org
QA Contact: mesa-dev@lists.freedesktop.org
CC: lem...@gmail.com, marta.lofst...@intel.com

mesa: cb758b892a7e62ff1f6187f2ca9ac543ff70a096 (master 11.1.0-devel)

MinGW and MSVC build error

  Compiling src/glsl/linker.cpp ...
src/glsl/linker.cpp: In function ‘bool included_in_packed_varying(ir_variable*,
const char*)’:
src/glsl/linker.cpp:3187:46: error: ‘strtok_r’ was not declared in this scope
char *token = strtok_r(list, ",", );
  ^

commit 4639cea2921669527eb43dcb49724c05afb27e8e
Author: Tapani Pälli 
Date:   Fri Sep 4 11:30:34 2015 +0300

glsl: add packed varyings to program resource list

This makes sure that user is still able to query properties about
variables that have gotten packed by lower_packed_varyings pass.

Fixes following OpenGL ES 3.1 test:
   ES31-CTS.program_interface_query.separate-programs-vertex

v2: fix 'name included in packed list' check (Ilia Mirkin)
v3: iterate over instances of name using strtok_r (Ilia Mirkin)

Signed-off-by: Tapani Pälli 
Reviewed-by: Marta Lofstedt 

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/5] i965/miptree: Rename align_w, align_h -> halign, valign

2015-09-29 Thread Ben Widawsky

On Tue, Sep 29, 2015 at 02:16:04PM -0700, Chad Versace wrote:
> On Sun 27 Sep 2015, Ben Widawsky wrote:
> > On Fri, Sep 25, 2015 at 12:05:49PM -0700, Chad Versace wrote:
> > > The values of intel_mipmap_tree::align_w and ::align_h correspond to the
> > > hardware enums HALIGN_* and VALIGN_*.
> > > 
> > > See the confusion?
> > > align_h != HALIGN
> > > align_h == VALIGN
> > > 
> > > Reduce the confusion by renaming the variables to match the hardware
> > > enum names:
> > > git ls-files |
> > > xargs sed -i -e 's/align_w/halign/g' \
> > >  -e 's/align_h/valign/g'
> > > 
> > > Suggested-by: Kenneth Graunke 
> > > Cc: Ben Widawsky 
> > > Cc: Kenneth Graunke 
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_tex_layout.c| 62 
> > > +++
> > >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  |  4 +-
> > >  src/mesa/drivers/dri/i965/gen6_blorp.cpp  |  2 +-
> > >  src/mesa/drivers/dri/i965/gen6_surface_state.c|  2 +-
> > >  src/mesa/drivers/dri/i965/gen7_blorp.cpp  |  4 +-
> > >  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c |  8 +--
> > >  src/mesa/drivers/dri/i965/gen8_surface_state.c| 12 ++---
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c |  2 +-
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 18 +--
> > >  9 files changed, 62 insertions(+), 52 deletions(-)
> 
> 
> > Reviewed-by: Ben Widawsky 
> 
> Is this a rb for only this patch? Or for the series?

It was just this. I haven't had time to look at the others in any detail, but
you can add my Acked-by on those if you'd like. The idea is sound.

-- 
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 08/70] i965: Remove early release of DRI2 miptree

2015-09-29 Thread Chad Versace

On Mon 28 Sep 2015, Martin Peres wrote:
> On 28/09/15 17:27, Emil Velikov wrote:
> >Hi all,
> >
> >On 17 August 2015 at 19:06, Chad Versace  wrote:
> >>On Fri 14 Aug 2015, Chris Wilson wrote:
> >>>On Thu, Aug 13, 2015 at 09:58:52PM -0700, Kenneth Graunke wrote:
> On Thursday, August 13, 2015 02:57:20 PM Martin Peres wrote:
> >On 07/08/15 23:13, Chris Wilson wrote:
> >>intel_update_winsys_renderbuffer_miptree() will release the existing
> >>miptree when wrapping a new DRI2 buffer, so we can remove the early
> >>release and so prevent a NULL mt dereference should importing the new
> >>DRI2 name fail for any reason. (Reusing the old DRI2 name will result
> >>in the rendering going astray, to a stale buffer, and not shown on the
> >>screen, but it allows us to issue a warning and not crash much later in
> >>innocent code.)
> >>
> >>Signed-off-by: Chris Wilson 
> >>---
> >>   src/mesa/drivers/dri/i965/brw_context.c | 1 -
> >>   1 file changed, 1 deletion(-)


> >As I haven't been following the discussion, can anyone point me to the
> >solution (if any) on this topic ?
> >Archlinux seems to be using the original "remove early release..."
> >alongside 10.6 and 11.0 as it is still an issue [1]. On the other hand
> >I've not seen any other distro picking it/any fixes on top of the
> >upstream tarball.
> >
> >Thanks
> >Emil
> >
> >[1] https://bugs.archlinux.org/task/45750

> I would be in favour of merging this patch right now. It is partial and will
> require more work to handle more cases but this is already a huge
> improvement for kde users.

I agree. This patch does not eliminate all crashes, but's still a large
improvement. And the logic in the commit message is sound.

Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/5] i965/miptree: Rename align_w, align_h -> halign, valign

2015-09-29 Thread Chad Versace

On Sun 27 Sep 2015, Ben Widawsky wrote:
> On Fri, Sep 25, 2015 at 12:05:49PM -0700, Chad Versace wrote:
> > The values of intel_mipmap_tree::align_w and ::align_h correspond to the
> > hardware enums HALIGN_* and VALIGN_*.
> > 
> > See the confusion?
> > align_h != HALIGN
> > align_h == VALIGN
> > 
> > Reduce the confusion by renaming the variables to match the hardware
> > enum names:
> > git ls-files |
> > xargs sed -i -e 's/align_w/halign/g' \
> >  -e 's/align_h/valign/g'
> > 
> > Suggested-by: Kenneth Graunke 
> > Cc: Ben Widawsky 
> > Cc: Kenneth Graunke 
> > ---
> >  src/mesa/drivers/dri/i965/brw_tex_layout.c| 62 
> > +++
> >  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  |  4 +-
> >  src/mesa/drivers/dri/i965/gen6_blorp.cpp  |  2 +-
> >  src/mesa/drivers/dri/i965/gen6_surface_state.c|  2 +-
> >  src/mesa/drivers/dri/i965/gen7_blorp.cpp  |  4 +-
> >  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c |  8 +--
> >  src/mesa/drivers/dri/i965/gen8_surface_state.c| 12 ++---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c |  2 +-
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 18 +--
> >  9 files changed, 62 insertions(+), 52 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c 
> > b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> > index 268b995..2955c8d 100644
> > --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> > +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> > @@ -282,7 +282,7 @@ gen9_miptree_layout_1d(struct intel_mipmap_tree *mt)
> > /* When this layout is used the horizontal alignment is fixed at 64 and 
> > the
> >  * hardware ignores the value given in the surface state
> >  */
> > -   const unsigned int align_w = 64;
> > +   const unsigned int halign = 64;
> >  
> > mt->total_height = mt->physical_height0;
> > mt->total_width = 0;
> > @@ -292,7 +292,7 @@ gen9_miptree_layout_1d(struct intel_mipmap_tree *mt)
> >  
> >intel_miptree_set_level_info(mt, level, x, 0, depth);
> >  
> > -  img_width = ALIGN(width, align_w);
> > +  img_width = ALIGN(width, halign);
> >  
> >mt->total_width = MAX2(mt->total_width, x + img_width);
> >  
> > @@ -328,10 +328,10 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
> > unsigned mip1_width;
> >  
> > if (mt->compressed) {
> > -  mip1_width = ALIGN_NPOT(minify(mt->physical_width0, 1), 
> > mt->align_w) +
> > +  mip1_width = ALIGN_NPOT(minify(mt->physical_width0, 1), 
> > mt->halign) +
> >   ALIGN_NPOT(minify(mt->physical_width0, 2), bw);
> > } else {
> > -  mip1_width = ALIGN_NPOT(minify(mt->physical_width0, 1), 
> > mt->align_w) +
> > +  mip1_width = ALIGN_NPOT(minify(mt->physical_width0, 1), 
> > mt->halign) +
> >   minify(mt->physical_width0, 2);
> > }
> >  
> > @@ -348,7 +348,7 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
> >  
> >intel_miptree_set_level_info(mt, level, x, y, depth);
> >  
> > -  img_height = ALIGN_NPOT(height, mt->align_h);
> > +  img_height = ALIGN_NPOT(height, mt->valign);
> >if (mt->compressed)
> >  img_height /= bh;
> >  
> > @@ -365,7 +365,7 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
> >/* Layout_below: step right after second mipmap.
> > */
> >if (level == mt->first_level + 1) {
> > -x += ALIGN_NPOT(width, mt->align_w) / bw;
> > +x += ALIGN_NPOT(width, mt->halign) / bw;
> >} else {
> >  y += img_height;
> >}
> > @@ -385,7 +385,7 @@ brw_miptree_get_horizontal_slice_pitch(const struct 
> > brw_context *brw,
> >  {
> > if ((brw->gen < 9 && mt->target == GL_TEXTURE_3D) ||
> > (brw->gen == 4 && mt->target == GL_TEXTURE_CUBE_MAP)) {
> > -  return ALIGN_NPOT(minify(mt->physical_width0, level), mt->align_w);
> > +  return ALIGN_NPOT(minify(mt->physical_width0, level), mt->halign);
> > } else {
> >return 0;
> > }
> > @@ -426,13 +426,13 @@ brw_miptree_get_vertical_slice_pitch(const struct 
> > brw_context *brw,
> > } else if (mt->target == GL_TEXTURE_3D ||
> >(brw->gen == 4 && mt->target == GL_TEXTURE_CUBE_MAP) ||
> >mt->array_layout == ALL_SLICES_AT_EACH_LOD) {
> > -  return ALIGN_NPOT(minify(mt->physical_height0, level), mt->align_h);
> > +  return ALIGN_NPOT(minify(mt->physical_height0, level), mt->valign);
> >  
> > } else {
> > -  const unsigned h0 = ALIGN_NPOT(mt->physical_height0, mt->align_h);
> > -  const unsigned h1 = ALIGN_NPOT(minify(mt->physical_height0, 1), 
> > mt->align_h);
> > +  const unsigned h0 = ALIGN_NPOT(mt->physical_height0, mt->valign);
> > +  const unsigned h1 = ALIGN_NPOT(minify(mt->physical_height0, 1), 
> > mt->valign);
> >  
> > -  return h0 + h1 + (brw->gen >= 7 ? 12 : 11) *

[Mesa-dev] [PATCH] i965/cs: Upload UBO/SSBO surfaces

2015-09-29 Thread Jordan Justen

Signed-off-by: Jordan Justen 
---
 src/mesa/drivers/dri/i965/brw_context.h  |  2 +-
 src/mesa/drivers/dri/i965/brw_state.h|  1 +
 src/mesa/drivers/dri/i965/brw_state_upload.c |  2 ++
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 26 
 4 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 80e5b74..ea7ea42 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1549,7 +1549,7 @@ struct brw_context
 
int num_atoms[BRW_NUM_PIPELINES];
const struct brw_tracked_state render_atoms[60];
-   const struct brw_tracked_state compute_atoms[7];
+   const struct brw_tracked_state compute_atoms[8];
 
/* If (INTEL_DEBUG & DEBUG_BATCH) */
struct {
diff --git a/src/mesa/drivers/dri/i965/brw_state.h 
b/src/mesa/drivers/dri/i965/brw_state.h
index 3b7a433..dc2b941 100644
--- a/src/mesa/drivers/dri/i965/brw_state.h
+++ b/src/mesa/drivers/dri/i965/brw_state.h
@@ -87,6 +87,7 @@ extern const struct brw_tracked_state brw_vs_binding_table;
 extern const struct brw_tracked_state brw_wm_ubo_surfaces;
 extern const struct brw_tracked_state brw_wm_abo_surfaces;
 extern const struct brw_tracked_state brw_wm_image_surfaces;
+extern const struct brw_tracked_state brw_cs_ubo_surfaces;
 extern const struct brw_tracked_state brw_cs_abo_surfaces;
 extern const struct brw_tracked_state brw_cs_image_surfaces;
 extern const struct brw_tracked_state brw_wm_unit;
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c 
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index 46687e3..79b8301 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -259,6 +259,7 @@ static const struct brw_tracked_state *gen7_compute_atoms[] 
=
_state_base_address,
_cs_image_surfaces,
_cs_push_constants,
+   _cs_ubo_surfaces,
_cs_abo_surfaces,
_texture_surfaces,
_cs_work_groups_surface,
@@ -352,6 +353,7 @@ static const struct brw_tracked_state *gen8_compute_atoms[] 
=
_state_base_address,
_cs_image_surfaces,
_cs_push_constants,
+   _cs_ubo_surfaces,
_cs_abo_surfaces,
_texture_surfaces,
_cs_work_groups_surface,
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index c931696..4e70cae 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1001,6 +1001,32 @@ const struct brw_tracked_state brw_wm_ubo_surfaces = {
.emit = brw_upload_wm_ubo_surfaces,
 };
 
+static void
+brw_upload_cs_ubo_surfaces(struct brw_context *brw)
+{
+   struct gl_context *ctx = >ctx;
+   /* _NEW_PROGRAM */
+   struct gl_shader_program *prog =
+  ctx->_Shader->CurrentProgram[MESA_SHADER_COMPUTE];
+
+   if (!prog)
+  return;
+
+   /* BRW_NEW_CS_PROG_DATA */
+   brw_upload_ubo_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_COMPUTE],
+   >cs.base, >cs.prog_data->base, true);
+}
+
+const struct brw_tracked_state brw_cs_ubo_surfaces = {
+   .dirty = {
+  .mesa = _NEW_PROGRAM,
+  .brw = BRW_NEW_BATCH |
+ BRW_NEW_CS_PROG_DATA |
+ BRW_NEW_UNIFORM_BUFFER,
+   },
+   .emit = brw_upload_cs_ubo_surfaces,
+};
+
 void
 brw_upload_abo_surfaces(struct brw_context *brw,
struct gl_shader_program *prog,
-- 
2.5.1

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

[Mesa-dev] [Bug 91889] Planetary Anihilation: Titans display content of other processes buffers

2015-09-29 Thread bugzilla-daemon

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

--- Comment #28 from Krzysztof A. Sobiecki  ---
So there was additional trace file hidden in a subfolder:
Main:
https://drive.google.com/file/d/0B3J0Mg89izcbYnpzVjhSeVRmNEk/view?usp=sharing
Hidden one:
https://drive.google.com/file/d/0B3J0Mg89izcbdk01UHgyS0s0OFk/view?usp=sharing

Didn't have time to look at it yet.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: store reference to the context within struct brw_fence

2015-09-29 Thread Chad Versace

On Mon 28 Sep 2015, Marek Olšák wrote:
> On Mon, Sep 28, 2015 at 1:29 PM, Emil Velikov  
> wrote:
> > As the spec allows for {server,client}_wait_sync to be called without
> > currently bound context, while our implementation requires context
> > pointer.
> >
> > UNTESTED.
> >
> > Cc: Chad Versace 
> > Cc: Marek Olšák 
> > Cc: Chih-Wei Huang 
> > Cc: Mauro Rossi 
> > Cc: 10.6 11.0 
> > Signed-off-by: Emil Velikov 
> > ---
> >
> > Upon second thought I'm leaning that we should move this to the API
> > specific library (i.e. libEGL) rather than keeping it here. It will give
> > us remove API specific from the DRI extension, plus it'll give us better
> > mix'n'match (loader+dri module) compatibility.
> >
> > How do you guys feel on the topic ?
> 
> I don't think there is anything to move into libEGL. st/dri doesn't
> add a context pointer to the fence, instead it adds a screen pointer,
> which is a different thing.

I agree with Mark here.  Even if there was something you could move into
libEGL, I don't it could be done safely. It needs to remain in the
driver.

> This i965 fix is probably not thread-safe, because eglClientWaitSync
> can be called from another thread and contexts typically contain no
> locks.

Emil, the fix is not thread-safe because drm_intel_gem_bo_wait() mutates
the bo. Thread-safety matters because some Chromium Ozone code that's
cooking does call eglCreateSync and eglClientWaitSync from different
threads.

If you add a mutex to struct brw_fence, and lock it for the duration of
brw_fence_client_wait() and brw_fence_is_completed(), then I think
that's sufficient for ensuring thread-safety.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] configure.ac: handle llvm built with cmake in one shared library

2015-09-29 Thread Dave Airlie

On 30 September 2015 at 01:34, Laurent Carlier  wrote:
> llvm can be built with cmake in a libray with the name libLLVM.so.$version
> Tested with both llvm-3.7.0 and llvm-3.8.0svn
>
> v2: check and use llvm build-system feature, update comments
>

just to reiterate, this should be fixed in llvm.

cmake and autoconf should produce libLLVM$version.so since their ABI
isn't compatible across releases.

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

[Mesa-dev] [Bug 92183] linker.cpp:3187:46: error: ‘strtok_r’ was not declared in this scope

2015-09-29 Thread bugzilla-daemon

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

--- Comment #2 from Ilia Mirkin  ---
strtok isn't threadsafe

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] glsl: reduce memory footprint of uniform_storage struct

2015-09-29 Thread Timothy Arceri

The uniform will only be of a single type so store the data for
opaque types in a single array.
---
 src/glsl/ir_uniform.h|  6 +---
 src/glsl/link_uniform_initializers.cpp   | 12 +++
 src/glsl/link_uniforms.cpp   | 41 +++-
 src/glsl/linker.cpp  |  2 +-
 src/glsl/nir/nir_lower_samplers.c|  4 +--
 src/glsl/tests/set_uniform_initializer_tests.cpp |  8 ++---
 src/mesa/drivers/dri/i965/brw_shader.cpp |  2 +-
 src/mesa/main/shaderapi.c|  2 +-
 src/mesa/main/uniform_query.cpp  |  8 ++---
 src/mesa/program/ir_to_mesa.cpp  |  5 +--
 src/mesa/program/sampler.cpp |  4 +--
 11 files changed, 43 insertions(+), 51 deletions(-)

diff --git a/src/glsl/ir_uniform.h b/src/glsl/ir_uniform.h
index 858a7da..50fe76b 100644
--- a/src/glsl/ir_uniform.h
+++ b/src/glsl/ir_uniform.h
@@ -110,11 +110,7 @@ struct gl_uniform_storage {
 */
bool initialized;
 
-   struct gl_opaque_uniform_index sampler[MESA_SHADER_STAGES];
-
-   struct gl_opaque_uniform_index image[MESA_SHADER_STAGES];
-
-   struct gl_opaque_uniform_index subroutine[MESA_SHADER_STAGES];
+   struct gl_opaque_uniform_index opaque[MESA_SHADER_STAGES];
 
/**
 * Storage used by the driver for the uniform
diff --git a/src/glsl/link_uniform_initializers.cpp 
b/src/glsl/link_uniform_initializers.cpp
index 3483082..0918d2a 100644
--- a/src/glsl/link_uniform_initializers.cpp
+++ b/src/glsl/link_uniform_initializers.cpp
@@ -134,16 +134,16 @@ set_opaque_binding(gl_shader_program *prog, const char 
*name, int binding)
 
   if (shader) {
  if (storage->type->base_type == GLSL_TYPE_SAMPLER &&
- storage->sampler[sh].active) {
+ storage->opaque[sh].active) {
 for (unsigned i = 0; i < elements; i++) {
-   const unsigned index = storage->sampler[sh].index + i;
+   const unsigned index = storage->opaque[sh].index + i;
shader->SamplerUnits[index] = storage->storage[i].i;
 }
 
  } else if (storage->type->base_type == GLSL_TYPE_IMAGE &&
-storage->image[sh].active) {
+storage->opaque[sh].active) {
 for (unsigned i = 0; i < elements; i++) {
-   const unsigned index = storage->image[sh].index + i;
+   const unsigned index = storage->opaque[sh].index + i;
shader->ImageUnits[index] = storage->storage[i].i;
 }
  }
@@ -243,8 +243,8 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program 
*prog,
  for (int sh = 0; sh < MESA_SHADER_STAGES; sh++) {
 gl_shader *shader = prog->_LinkedShaders[sh];
 
-if (shader && storage->sampler[sh].active) {
-   unsigned index = storage->sampler[sh].index;
+if (shader && storage->opaque[sh].active) {
+   unsigned index = storage->opaque[sh].index;
 
shader->SamplerUnits[index] = storage->storage[0].i;
 }
diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
index 47d49c8..b92e73c 100644
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -566,7 +566,7 @@ private:
 struct gl_uniform_storage *uniform, const char *name)
{
   if (base_type->is_sampler()) {
- uniform->sampler[shader_type].active = true;
+ uniform->opaque[shader_type].active = true;
 
  /* Handle multiple samplers inside struct arrays */
  if (this->record_array_count > 1) {
@@ -586,8 +586,8 @@ private:
/* In this case, we've already seen this uniform so we just use
 * the next sampler index recorded the last time we visited.
 */
-   uniform->sampler[shader_type].index = index;
-   index = inner_array_size + uniform->sampler[shader_type].index;
+   uniform->opaque[shader_type].index = index;
+   index = inner_array_size + uniform->opaque[shader_type].index;
this->record_next_sampler->put(index, name_copy);
 
ralloc_free(name_copy);
@@ -605,13 +605,13 @@ private:
 * structs. This allows the offset to be easily calculated for
 * indirect indexing.
 */
-   uniform->sampler[shader_type].index = this->next_sampler;
+   uniform->opaque[shader_type].index = this->next_sampler;
this->next_sampler +=
   inner_array_size * this->record_array_count;
 
/* Store the next index for future passes over the struct array
 */
-   index = uniform->sampler[shader_type].index + inner_array_size;
+   index = uniform->opaque[shader_type].index + inner_array_size;
this->record_next_sampler->put(index,

Re: [Mesa-dev] [PATCH] radeonsi: don't set DATA_FORMAT if ADD_TID_ENABLE is set on VI

2015-09-29 Thread Michel Dänzer

On 29.09.2015 23:54, Marek Olšák wrote:
> From: Marek Olšák 
> 
> This can cause incorrect address calculations and hangs.
> 
> Cc: mesa-sta...@lists.freedesktop.org

IIRC without "10.6 11.0" this only makes it a candidate for the 11.0
branch at this point[0]. Is that your intention?

[0] If so, maybe the rules should be changed such that no specific
version means it's a candidate for all active stable branches?


> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
> b/src/gallium/drivers/radeonsi/si_descriptors.c
> index b07ab3b..0ced6c3 100644
> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> @@ -619,11 +619,14 @@ void si_set_ring_buffer(struct pipe_context *ctx, uint 
> shader, uint slot,
> S_008F0C_DST_SEL_Z(V_008F0C_SQ_SEL_Z) |
> S_008F0C_DST_SEL_W(V_008F0C_SQ_SEL_W) |
> S_008F0C_NUM_FORMAT(V_008F0C_BUF_NUM_FORMAT_FLOAT) |
> -   S_008F0C_DATA_FORMAT(V_008F0C_BUF_DATA_FORMAT_32) |
> S_008F0C_ELEMENT_SIZE(element_size) |
> S_008F0C_INDEX_STRIDE(index_stride) |
> S_008F0C_ADD_TID_ENABLE(add_tid);
>  
> + /* If ADD_TID_ENABLE is set, DATA_FORMAT specifies STRIDE bits 
> [14:17] */
> + if (sctx->b.chip_class >= VI && !add_tid)
> + desc[3] |= 
> S_008F0C_DATA_FORMAT(V_008F0C_BUF_DATA_FORMAT_32);
> +
>   pipe_resource_reference(>buffers[slot], buffer);
>   radeon_add_to_buffer_list(>b, >b.rings.gfx,
> (struct r600_resource*)buffer,
> 

Since the DATA_FORMAT behaviour only changed as of VI, I think this
should be:

/* If ADD_TID_ENABLE is set, DATA_FORMAT specifies STRIDE bits 
[14:17] on VI */
if (sctx->b.chip_class < VI || !add_tid)
desc[3] |= 
S_008F0C_DATA_FORMAT(V_008F0C_BUF_DATA_FORMAT_32);


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965: Get rid of prog_data compare functions

2015-09-29 Thread Kenneth Graunke

On Tuesday, September 29, 2015 09:05:41 PM Jason Ekstrand wrote:
> They are no longer used.
> ---
>  src/mesa/drivers/dri/i965/brw_context.h | 26 --
>  src/mesa/drivers/dri/i965/brw_cs.c  | 21 -
>  src/mesa/drivers/dri/i965/brw_cs.h  |  2 --
>  src/mesa/drivers/dri/i965/brw_gs.c  | 21 -
>  src/mesa/drivers/dri/i965/brw_gs.h  |  2 --
>  src/mesa/drivers/dri/i965/brw_program.c | 17 -
>  src/mesa/drivers/dri/i965/brw_program.h |  4 
>  src/mesa/drivers/dri/i965/brw_vs.c  | 21 -
>  src/mesa/drivers/dri/i965/brw_vs.h  |  1 -
>  src/mesa/drivers/dri/i965/brw_wm.c  | 19 ---
>  src/mesa/drivers/dri/i965/brw_wm.h  |  1 -
>  11 files changed, 135 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 3bae90d..bf4deaa 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -340,9 +340,6 @@ struct brw_shader {
> bool compiled_once;
>  };
>  
> -/* Note: If adding fields that need anything besides a normal memcmp() for
> - * comparing them, be sure to go fix brw_stage_prog_data_compare().
> - */
>  struct brw_stage_prog_data {
> struct {
>/** size of our binding table. */
> @@ -378,18 +375,10 @@ struct brw_stage_prog_data {
>  
> /* Pointers to tracked values (only valid once
>  * _mesa_load_state_parameters has been called at runtime).
> -*
> -* These must be the last fields of the struct (see
> -* brw_stage_prog_data_compare()).
>  */
> const gl_constant_value **param;
> const gl_constant_value **pull_param;
>  
> -   /**
> -* Image metadata passed to the shader as uniforms.  This is deliberately

Please keep this first sentence.  Otherwise, both are
Reviewed-by: Kenneth Graunke 

> -* ignored by brw_stage_prog_data_compare() because its contents don't 
> have
> -* any influence on program compilation.
> -*/
> struct brw_image_param *image_param;
>  };


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

Re: [Mesa-dev] [PATCH] i965/copy_image: Fix a copy+past error

2015-09-29 Thread Kenneth Graunke

On Monday, September 28, 2015 05:39:54 PM Jason Ekstrand wrote:
> Reported-by: Ilia Mirkin 
> ---
>  src/mesa/drivers/dri/i965/intel_copy_image.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_copy_image.c 
> b/src/mesa/drivers/dri/i965/intel_copy_image.c
> index d57651c..0a3337e 100644
> --- a/src/mesa/drivers/dri/i965/intel_copy_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_copy_image.c
> @@ -235,7 +235,7 @@ intel_copy_image_sub_data(struct gl_context *ctx,
> } else {
>assert(dst_renderbuffer);
>dst_mt = intel_renderbuffer(dst_renderbuffer)->mt;
> -  src_image = src_renderbuffer->TexImage;
> +  dst_image = dst_renderbuffer->TexImage;
> }
>  
> if (src_mt->num_samples > 0 || dst_mt->num_samples > 0) {
> 

Reviewed-by: Kenneth Graunke 


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

[Mesa-dev] [PATCH 2/2] i965: Get rid of prog_data compare functions

2015-09-29 Thread Jason Ekstrand

They are no longer used.
---
 src/mesa/drivers/dri/i965/brw_context.h | 26 --
 src/mesa/drivers/dri/i965/brw_cs.c  | 21 -
 src/mesa/drivers/dri/i965/brw_cs.h  |  2 --
 src/mesa/drivers/dri/i965/brw_gs.c  | 21 -
 src/mesa/drivers/dri/i965/brw_gs.h  |  2 --
 src/mesa/drivers/dri/i965/brw_program.c | 17 -
 src/mesa/drivers/dri/i965/brw_program.h |  4 
 src/mesa/drivers/dri/i965/brw_vs.c  | 21 -
 src/mesa/drivers/dri/i965/brw_vs.h  |  1 -
 src/mesa/drivers/dri/i965/brw_wm.c  | 19 ---
 src/mesa/drivers/dri/i965/brw_wm.h  |  1 -
 11 files changed, 135 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 3bae90d..bf4deaa 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -340,9 +340,6 @@ struct brw_shader {
bool compiled_once;
 };
 
-/* Note: If adding fields that need anything besides a normal memcmp() for
- * comparing them, be sure to go fix brw_stage_prog_data_compare().
- */
 struct brw_stage_prog_data {
struct {
   /** size of our binding table. */
@@ -378,18 +375,10 @@ struct brw_stage_prog_data {
 
/* Pointers to tracked values (only valid once
 * _mesa_load_state_parameters has been called at runtime).
-*
-* These must be the last fields of the struct (see
-* brw_stage_prog_data_compare()).
 */
const gl_constant_value **param;
const gl_constant_value **pull_param;
 
-   /**
-* Image metadata passed to the shader as uniforms.  This is deliberately
-* ignored by brw_stage_prog_data_compare() because its contents don't have
-* any influence on program compilation.
-*/
struct brw_image_param *image_param;
 };
 
@@ -443,9 +432,6 @@ struct brw_image_param {
  * there can be many of these, each in a different GL state
  * corresponding to a different brw_wm_prog_key struct, with different
  * compiled programs.
- *
- * Note: brw_wm_prog_data_compare() must be updated when adding fields to this
- * struct!
  */
 struct brw_wm_prog_data {
struct brw_stage_prog_data base;
@@ -489,9 +475,6 @@ struct brw_wm_prog_data {
int urb_setup[VARYING_SLOT_MAX];
 };
 
-/* Note: brw_cs_prog_data_compare() must be updated when adding fields to this
- * struct!
- */
 struct brw_cs_prog_data {
struct brw_stage_prog_data base;
 
@@ -692,9 +675,6 @@ enum shader_dispatch_mode {
DISPATCH_MODE_SIMD8 = 3,
 };
 
-/* Note: brw_vue_prog_data_compare() must be updated when adding fields to
- * this struct!
- */
 struct brw_vue_prog_data {
struct brw_stage_prog_data base;
struct brw_vue_map vue_map;
@@ -712,9 +692,6 @@ struct brw_vue_prog_data {
 };
 
 
-/* Note: brw_vs_prog_data_compare() must be updated when adding fields to this
- * struct!
- */
 struct brw_vs_prog_data {
struct brw_vue_prog_data base;
 
@@ -774,9 +751,6 @@ struct brw_vs_prog_data {
 
 #define SURF_INDEX_GEN6_SOL_BINDING(t) (t)
 
-/* Note: brw_gs_prog_data_compare() must be updated when adding fields to
- * this struct!
- */
 struct brw_gs_prog_data
 {
struct brw_vue_prog_data base;
diff --git a/src/mesa/drivers/dri/i965/brw_cs.c 
b/src/mesa/drivers/dri/i965/brw_cs.c
index cb3fae6..02eeeda 100644
--- a/src/mesa/drivers/dri/i965/brw_cs.c
+++ b/src/mesa/drivers/dri/i965/brw_cs.c
@@ -31,27 +31,6 @@
 #include "brw_state.h"
 #include "intel_batchbuffer.h"
 
-bool
-brw_cs_prog_data_compare(const void *in_a, const void *in_b)
-{
-   const struct brw_cs_prog_data *a =
-  (const struct brw_cs_prog_data *)in_a;
-   const struct brw_cs_prog_data *b =
-  (const struct brw_cs_prog_data *)in_b;
-
-   /* Compare the base structure. */
-   if (!brw_stage_prog_data_compare(>base, >base))
-  return false;
-
-   /* Compare the rest of the structure. */
-   const unsigned offset = sizeof(struct brw_stage_prog_data);
-   if (memcmp(((char *) a) + offset, ((char *) b) + offset,
-  sizeof(struct brw_cs_prog_data) - offset))
-  return false;
-
-   return true;
-}
-
 static bool
 brw_codegen_cs_prog(struct brw_context *brw,
 struct gl_shader_program *prog,
diff --git a/src/mesa/drivers/dri/i965/brw_cs.h 
b/src/mesa/drivers/dri/i965/brw_cs.h
index 746fb05..0018c04 100644
--- a/src/mesa/drivers/dri/i965/brw_cs.h
+++ b/src/mesa/drivers/dri/i965/brw_cs.h
@@ -36,8 +36,6 @@ struct brw_cs_prog_key {
 extern "C" {
 #endif
 
-bool brw_cs_prog_data_compare(const void *a, const void *b);
-
 void
 brw_upload_cs_prog(struct brw_context *brw);
 
diff --git a/src/mesa/drivers/dri/i965/brw_gs.c 
b/src/mesa/drivers/dri/i965/brw_gs.c
index 0de36cc..61e7b2a 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_gs.c
@@ -394,24 +394,3 @@ brw_gs_precompile(struct gl_context *ctx,
 
return success;
 }
-
-
-bool
-brw_gs_prog_data_compare(const void *in_a, const void *in_b)
-{
-

[Mesa-dev] [PATCH 1/2] i965/state_cache: Remove the aux_compare fields

2015-09-29 Thread Jason Ekstrand

They haven't been used since 1bba29ed403e735ba0bf04ed8aa2e571884fcaaf so
there's no good reason to keep them around.
---
 src/mesa/drivers/dri/i965/brw_context.h | 7 ---
 src/mesa/drivers/dri/i965/brw_state_cache.c | 4 
 2 files changed, 11 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 8b790fe..3bae90d 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -869,7 +869,6 @@ struct brw_cache_item {
 };
 
 
-typedef bool (*cache_aux_compare_func)(const void *a, const void *b);
 typedef void (*cache_aux_free_func)(const void *aux);
 
 struct brw_cache {
@@ -882,12 +881,6 @@ struct brw_cache {
uint32_t next_offset;
bool bo_used_by_gpu;
 
-   /**
-* Optional functions used in determining whether the prog_data for a new
-* cache item matches an existing cache item (in case there's relevant data
-* outside of the prog_data).  If NULL, a plain memcmp is done.
-*/
-   cache_aux_compare_func aux_compare[BRW_MAX_CACHE];
/** Optional functions for freeing other pointers attached to a prog_data. 
*/
cache_aux_free_func aux_free[BRW_MAX_CACHE];
 };
diff --git a/src/mesa/drivers/dri/i965/brw_state_cache.c 
b/src/mesa/drivers/dri/i965/brw_state_cache.c
index fbc0419..2fbcd14 100644
--- a/src/mesa/drivers/dri/i965/brw_state_cache.c
+++ b/src/mesa/drivers/dri/i965/brw_state_cache.c
@@ -350,10 +350,6 @@ brw_init_caches(struct brw_context *brw)
if (brw->has_llc)
   drm_intel_gem_bo_map_unsynchronized(cache->bo);
 
-   cache->aux_compare[BRW_CACHE_VS_PROG] = brw_vs_prog_data_compare;
-   cache->aux_compare[BRW_CACHE_GS_PROG] = brw_gs_prog_data_compare;
-   cache->aux_compare[BRW_CACHE_FS_PROG] = brw_wm_prog_data_compare;
-   cache->aux_compare[BRW_CACHE_CS_PROG] = brw_cs_prog_data_compare;
cache->aux_free[BRW_CACHE_VS_PROG] = brw_stage_prog_data_free;
cache->aux_free[BRW_CACHE_GS_PROG] = brw_stage_prog_data_free;
cache->aux_free[BRW_CACHE_FS_PROG] = brw_stage_prog_data_free;
-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [Bug 92183] linker.cpp:3187:46: error: ‘strtok_r’ was not declared in this scope

2015-09-29 Thread bugzilla-daemon

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

--- Comment #1 from Tapani Pälli  ---
it seems strtok_s would be the windows equivalent:
https://msdn.microsoft.com/en-us/library/ftsafwz3.aspx

I can make a patch that uses strtok_s when on windows or maybe just go for
regular strtok() ? I can see there's some usage of regular strtok() in Mesa.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 92183] linker.cpp:3187:46: error: ‘strtok_r’ was not declared in this scope

2015-09-29 Thread bugzilla-daemon

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

--- Comment #3 from Tapani Pälli  ---
(In reply to Ilia Mirkin from comment #2)
> strtok isn't threadsafe

right I'll go for strtok_s, I was just wondering if the occurences of strtok
should be converted to mesa_strtok which would then be strtok_r/s or is it
worth the trouble?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] List of unsupported extensions per driver

2015-09-29 Thread Alex Deucher

On Tue, Sep 29, 2015 at 1:29 PM, Romain Failliot
 wrote:
> Le 29 sept. 2015 7:22 PM, "Ilia Mirkin"  a écrit :
>> R600/R700 will never support ARB_gpu_shader5. Evergreen and NI
>> families (also driven by the r600g driver) will gain support for it in
>> due time.
>
> Aaaah I think I understand my misunderstanding: the R600 drivers (in
> GL3.txt) gather several drivers, some of them will support ARB_gpu_shader5,
> some won't. That's it?

There is one driver, r600, which supports several generations of
hardware (r6xx, r7xx, evergreen, NI).  Each generation of hardware has
different features.

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

Re: [Mesa-dev] List of unsupported extensions per driver

2015-09-29 Thread Ilia Mirkin

On Tue, Sep 29, 2015 at 1:19 PM, Romain Failliot
 wrote:
> @glenn:so, if I sum up, even though ARB_gpu_shader5 is marked unsupported
> for R600 here
> (https://secure.freedesktop.org/~imirkin/glxinfo/glxinfo.html), it doesn't
> mean that it is really unsupported for this driver.

R600/R700 will never support ARB_gpu_shader5. Evergreen and NI
families (also driven by the r600g driver) will gain support for it in
due time.

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

Re: [Mesa-dev] [PATCH 1/2] st/mesa: add some debugging code in st_ChooseTextureFormat()

2015-09-29 Thread Marek Olšák

Reviewed-by: Marek Olšák 

On Tue, Sep 29, 2015 at 5:39 PM, Brian Paul  wrote:
> I've temporarily added code like this many times.  Wrap it in a
> conditional that can be enabled when needed.
> ---
>  src/mesa/state_tracker/st_format.c | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/state_tracker/st_format.c 
> b/src/mesa/state_tracker/st_format.c
> index e3fb761..0c94428 100644
> --- a/src/mesa/state_tracker/st_format.c
> +++ b/src/mesa/state_tracker/st_format.c
> @@ -34,6 +34,8 @@
>
>  #include "main/imports.h"
>  #include "main/context.h"
> +#include "main/enums.h"
> +#include "main/formats.h"
>  #include "main/glformats.h"
>  #include "main/texgetimage.h"
>  #include "main/teximage.h"
> @@ -1938,6 +1940,7 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum 
> target,
>  {
> struct st_context *st = st_context(ctx);
> enum pipe_format pFormat;
> +   mesa_format mFormat;
> unsigned bindings;
> enum pipe_texture_target pTarget = gl_target_to_pipe(target);
>
> @@ -2010,7 +2013,20 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum 
> target,
>return MESA_FORMAT_NONE;
> }
>
> -   return st_pipe_format_to_mesa_format(pFormat);
> +   mFormat = st_pipe_format_to_mesa_format(pFormat);
> +
> +   /* Debugging aid */
> +   if (0) {
> +  debug_printf("%s(intFormat=%s, format=%s, type=%s) -> %s, %s\n",
> +   __func__,
> +   _mesa_enum_to_string(internalFormat),
> +   _mesa_enum_to_string(format),
> +   _mesa_enum_to_string(type),
> +   util_format_name(pFormat),
> +   _mesa_get_format_name(mFormat));
> +   }
> +
> +   return mFormat;
>  }
>
>
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] List of unsupported extensions per driver

2015-09-29 Thread Alex Deucher

On Tue, Sep 29, 2015 at 1:19 PM, Romain Failliot
 wrote:
> @glenn:so, if I sum up, even though ARB_gpu_shader5 is marked unsupported
> for R600 here
> (https://secure.freedesktop.org/~imirkin/glxinfo/glxinfo.html), it doesn't
> mean that it is really unsupported for this driver.
>
> @albert: thanks, I think I got that right in Mesamatrix
> (http://mesamatrix.net) ;)
>
> So that I'm clear, it's not about the fact that it is not done (i.e. red in
> Mesamatrix), but I'm merely trying to find out which extensions will never
> be supported by some drivers because the hardwares for them don't support
> these extensions.

If you look at imirkin's page above, the grey boxes indicate when the
hw does not support the feature.

Alex

>
> Cheers!
> Romain
>
> 2015-09-29 17:51 GMT+02:00 Albert Freeman :
>>
>> On 29 September 2015 at 14:48, Romain Failliot
>>  wrote:
>> > What I don't understand is that all the lines starting with a "-" seems
>> > to
>> > be part of the GL_ARB_gpu_shader5 extension. See the line here:
>> > http://cgit.freedesktop.org/mesa/mesa/tree/docs/GL3.txt#n99
>> >
>> > If I'm right, it means that, considering Ilia's web site,
>> > GL_ARB_gpu_shader5
>> > is unsupported by R600, but everything in its sublist is supported. You
>> > see
>> > why it is confusing?
>> >
>> > Le 29 sept. 2015 8:06 AM, "Marek Olšák"  a écrit :
>> >>
>> >> FMA isn't required really. R600 is mainly missing GS streams, which
>> >> are complete on the mailing somewhere I think.
>> >>
>> >> Marek
>> >>
>> >> On Tue, Sep 29, 2015 at 7:32 AM, Romain Failliot
>> >>  wrote:
>> >> > Hi!
>> >> >
>> >> > I'm diving into the unsupported extensions list and I'm wondering how
>> >> > is
>> >> > it
>> >> > possible that GL_ARB_gpu_shader5 is unsupported for R600, but some of
>> >> > the
>> >> > "sub-extensions" like "Dynamically uniform sampler array indices" are
>> >> > supported nonetheless.
>> >> >
>> >> > That makes me wonder if "not done" sub-extensions, like "Fused
>> >> > multiply-add", are really not done for R600 yet, or if they are
>> >> > indeed
>> >> > unsupported as the parent extension status let us suppose.
>> >> >
>> >> > Thx
>> >> > Romain
>> >> >
>> >> > 2015-07-31 23:42 GMT+02:00 Ilia Mirkin :
>> >> >>
>> >> >> OK, I believe I've fixed my list up. Note that you may have to
>> >> >> shift-reload to get the updates, I think fd.o isn't setting the
>> >> >> proper
>> >> >> cache headers or something else is messed up.
>> >> >
>> >> >
>> >
>> >
>> > ___
>> > mesa-dev mailing list
>> > mesa-dev@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> >
>> Guide to GL3.txt:
>> {
>> If a line starting with '-' has no braces next to it, it means it is
>> supported for all drivers (if it is marked DONE).
>> If a line starting with '-' has braces next to it, whatever driver/s
>> is in the braces has support for that feature (if any) (if it is
>> marked DONE).
>> When a driver has support for all features within an extension, it is
>> removed from all braces on lines starting with '-' and placed in the
>> braces for the extension name (the line above the first line marked
>> '-').
>>
>> DONE just means that non driver specific support is complete (which is
>> required before any driver can support that feature/extension).
>>
>> Usually extensions are not advertised to GL applications until they
>> are deemed complete (for the driver that is in use), this can be
>> overridden with e.g. export
>> MESA_EXTENSION_OVERRIDE=GL_ARB_gpu_shader5. You can also disable
>> advertising of a GL feature with e.g. export
>> MESA_EXTENSION_OVERRIDE=-GL_ARB_gpu_shader5.
>> }
>
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] List of unsupported extensions per driver

2015-09-29 Thread Ilia Mirkin

On Tue, Sep 29, 2015 at 1:29 PM, Romain Failliot
 wrote:
> Le 29 sept. 2015 7:22 PM, "Ilia Mirkin"  a écrit :
>> R600/R700 will never support ARB_gpu_shader5. Evergreen and NI
>> families (also driven by the r600g driver) will gain support for it in
>> due time.
>
> Aaaah I think I understand my misunderstanding: the R600 drivers (in
> GL3.txt) gather several drivers, some of them will support ARB_gpu_shader5,
> some won't. That's it?

A single driver, r600g, supports multiple pieces of (highly similar)
hardware. Some of that hardware has support for the features required
by ARB_gpu_shader5, some doesn't.

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

Re: [Mesa-dev] [PATCH 2/2] st/mesa: try PIPE_BIND_RENDER_TARGET when choosing float texture formats

2015-09-29 Thread Roland Scheidegger

Probably makes sense. Albeit I'm not sure about the L/A/LA/I stuff,
probably apps are aware that trying to render to that may fail (and
hence potentially choosing some rgb format instead of a single-channel
format is really not desirable for those).

Roland


Am 29.09.2015 um 19:50 schrieb Brian Paul:
> I was actually thinking of expanding this change to cover _all_ color
> formats but wanted to take a small step first.  What do you think?
> 
> -Brian
> 
> On 09/29/2015 11:46 AM, Roland Scheidegger wrote:
>> If that was due to some rgb vs. rgbx thing (that is, could texture from
>> rgb but only render to rgbx) I wonder if the same logic shouldn't also
>> apply to the integer formats.
>> But either way (I guess the app should check fbo completeness in any
>> case so strictly speaking for correctness this isn't really required)
>>
>> Reviewed-by: Roland Scheidegger 
>>
>> Am 29.09.2015 um 17:39 schrieb Brian Paul:
>>> For 8-bit RGB(A) texture formats we set the PIPE_BIND_RENDER_TARGET flag
>>> to try to get a hardware format which also supports rendering (for FBO
>>> textures).  Do the same thing for floating point formats.
>>>
>>> This allows the Redway Flat demo to run.
>>> ---
>>>   src/mesa/state_tracker/st_format.c | 6 +-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/mesa/state_tracker/st_format.c
>>> b/src/mesa/state_tracker/st_format.c
>>> index 0c94428..144b7d6 100644
>>> --- a/src/mesa/state_tracker/st_format.c
>>> +++ b/src/mesa/state_tracker/st_format.c
>>> @@ -1963,7 +1963,11 @@ st_ChooseTextureFormat(struct gl_context *ctx,
>>> GLenum target,
>>>  else if (internalFormat == 3 || internalFormat == 4 ||
>>>   internalFormat == GL_RGB || internalFormat == GL_RGBA ||
>>>   internalFormat == GL_RGB8 || internalFormat == GL_RGBA8 ||
>>> -internalFormat == GL_BGRA)
>>> +internalFormat == GL_BGRA ||
>>> +internalFormat == GL_RGB16F ||
>>> +internalFormat == GL_RGBA16F ||
>>> +internalFormat == GL_RGB32F ||
>>> +internalFormat == GL_RGBA32F)
>>>bindings |= PIPE_BIND_RENDER_TARGET;
>>>
>>>  /* GLES allows the driver to choose any format which matches
>>>
>>
> 

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

Re: [Mesa-dev] List of unsupported extensions per driver

2015-09-29 Thread Romain Failliot

@glenn:so, if I sum up, even though ARB_gpu_shader5 is marked unsupported
for R600 here (https://secure.freedesktop.org/~imirkin/glxinfo/glxinfo.html),
it doesn't mean that it is really unsupported for this driver.

@albert: thanks, I think I got that right in Mesamatrix (
http://mesamatrix.net) ;)

So that I'm clear, it's not about the fact that it is not done (i.e. red in
Mesamatrix), but I'm merely trying to find out which extensions will never
be supported by some drivers because the hardwares for them don't support
these extensions.

Cheers!
Romain

2015-09-29 17:51 GMT+02:00 Albert Freeman :

> On 29 September 2015 at 14:48, Romain Failliot
>  wrote:
> > What I don't understand is that all the lines starting with a "-" seems
> to
> > be part of the GL_ARB_gpu_shader5 extension. See the line here:
> > http://cgit.freedesktop.org/mesa/mesa/tree/docs/GL3.txt#n99
> >
> > If I'm right, it means that, considering Ilia's web site,
> GL_ARB_gpu_shader5
> > is unsupported by R600, but everything in its sublist is supported. You
> see
> > why it is confusing?
> >
> > Le 29 sept. 2015 8:06 AM, "Marek Olšák"  a écrit :
> >>
> >> FMA isn't required really. R600 is mainly missing GS streams, which
> >> are complete on the mailing somewhere I think.
> >>
> >> Marek
> >>
> >> On Tue, Sep 29, 2015 at 7:32 AM, Romain Failliot
> >>  wrote:
> >> > Hi!
> >> >
> >> > I'm diving into the unsupported extensions list and I'm wondering how
> is
> >> > it
> >> > possible that GL_ARB_gpu_shader5 is unsupported for R600, but some of
> >> > the
> >> > "sub-extensions" like "Dynamically uniform sampler array indices" are
> >> > supported nonetheless.
> >> >
> >> > That makes me wonder if "not done" sub-extensions, like "Fused
> >> > multiply-add", are really not done for R600 yet, or if they are indeed
> >> > unsupported as the parent extension status let us suppose.
> >> >
> >> > Thx
> >> > Romain
> >> >
> >> > 2015-07-31 23:42 GMT+02:00 Ilia Mirkin :
> >> >>
> >> >> OK, I believe I've fixed my list up. Note that you may have to
> >> >> shift-reload to get the updates, I think fd.o isn't setting the
> proper
> >> >> cache headers or something else is messed up.
> >> >
> >> >
> >
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> Guide to GL3.txt:
> {
> If a line starting with '-' has no braces next to it, it means it is
> supported for all drivers (if it is marked DONE).
> If a line starting with '-' has braces next to it, whatever driver/s
> is in the braces has support for that feature (if any) (if it is
> marked DONE).
> When a driver has support for all features within an extension, it is
> removed from all braces on lines starting with '-' and placed in the
> braces for the extension name (the line above the first line marked
> '-').
>
> DONE just means that non driver specific support is complete (which is
> required before any driver can support that feature/extension).
>
> Usually extensions are not advertised to GL applications until they
> are deemed complete (for the driver that is in use), this can be
> overridden with e.g. export
> MESA_EXTENSION_OVERRIDE=GL_ARB_gpu_shader5. You can also disable
> advertising of a GL feature with e.g. export
> MESA_EXTENSION_OVERRIDE=-GL_ARB_gpu_shader5.
> }
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] st/mesa: try PIPE_BIND_RENDER_TARGET when choosing float texture formats

2015-09-29 Thread Roland Scheidegger

If that was due to some rgb vs. rgbx thing (that is, could texture from
rgb but only render to rgbx) I wonder if the same logic shouldn't also
apply to the integer formats.
But either way (I guess the app should check fbo completeness in any
case so strictly speaking for correctness this isn't really required)

Reviewed-by: Roland Scheidegger 

Am 29.09.2015 um 17:39 schrieb Brian Paul:
> For 8-bit RGB(A) texture formats we set the PIPE_BIND_RENDER_TARGET flag
> to try to get a hardware format which also supports rendering (for FBO
> textures).  Do the same thing for floating point formats.
> 
> This allows the Redway Flat demo to run.
> ---
>  src/mesa/state_tracker/st_format.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/state_tracker/st_format.c 
> b/src/mesa/state_tracker/st_format.c
> index 0c94428..144b7d6 100644
> --- a/src/mesa/state_tracker/st_format.c
> +++ b/src/mesa/state_tracker/st_format.c
> @@ -1963,7 +1963,11 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum 
> target,
> else if (internalFormat == 3 || internalFormat == 4 ||
>  internalFormat == GL_RGB || internalFormat == GL_RGBA ||
>  internalFormat == GL_RGB8 || internalFormat == GL_RGBA8 ||
> -internalFormat == GL_BGRA)
> +internalFormat == GL_BGRA ||
> +internalFormat == GL_RGB16F ||
> +internalFormat == GL_RGBA16F ||
> +internalFormat == GL_RGB32F ||
> +internalFormat == GL_RGBA32F)
>bindings |= PIPE_BIND_RENDER_TARGET;
>  
> /* GLES allows the driver to choose any format which matches
> 

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

Re: [Mesa-dev] List of unsupported extensions per driver

2015-09-29 Thread Romain Failliot

Le 29 sept. 2015 7:22 PM, "Ilia Mirkin"  a écrit :
> R600/R700 will never support ARB_gpu_shader5. Evergreen and NI
> families (also driven by the r600g driver) will gain support for it in
> due time.

Aaaah I think I understand my misunderstanding: the R600 drivers (in
GL3.txt) gather several drivers, some of them will support ARB_gpu_shader5,
some won't. That's it?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] st/mesa: try PIPE_BIND_RENDER_TARGET when choosing float texture formats

2015-09-29 Thread Brian Paul

I was actually thinking of expanding this change to cover _all_ color 
formats but wanted to take a small step first.  What do you think?


-Brian

On 09/29/2015 11:46 AM, Roland Scheidegger wrote:

If that was due to some rgb vs. rgbx thing (that is, could texture from
rgb but only render to rgbx) I wonder if the same logic shouldn't also
apply to the integer formats.
But either way (I guess the app should check fbo completeness in any
case so strictly speaking for correctness this isn't really required)

Reviewed-by: Roland Scheidegger 

Am 29.09.2015 um 17:39 schrieb Brian Paul:

For 8-bit RGB(A) texture formats we set the PIPE_BIND_RENDER_TARGET flag
to try to get a hardware format which also supports rendering (for FBO
textures).  Do the same thing for floating point formats.

This allows the Redway Flat demo to run.
---
  src/mesa/state_tracker/st_format.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_format.c 
b/src/mesa/state_tracker/st_format.c
index 0c94428..144b7d6 100644
--- a/src/mesa/state_tracker/st_format.c
+++ b/src/mesa/state_tracker/st_format.c
@@ -1963,7 +1963,11 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum 
target,
 else if (internalFormat == 3 || internalFormat == 4 ||
  internalFormat == GL_RGB || internalFormat == GL_RGBA ||
  internalFormat == GL_RGB8 || internalFormat == GL_RGBA8 ||
-internalFormat == GL_BGRA)
+internalFormat == GL_BGRA ||
+internalFormat == GL_RGB16F ||
+internalFormat == GL_RGBA16F ||
+internalFormat == GL_RGB32F ||
+internalFormat == GL_RGBA32F)
 bindings |= PIPE_BIND_RENDER_TARGET;

 /* GLES allows the driver to choose any format which matches





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

[Mesa-dev] [Bug 91797] [r600g] Company of Heroes 2 crash when zooming on the map

2015-09-29 Thread bugzilla-daemon

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

Benjamin Bellec  changed:

   What|Removed |Added

 Status|NEW |NEEDINFO

--- Comment #3 from Benjamin Bellec  ---
Unfortunately, I just tried the game during the free-to-play weekend after the
game release. So I can't test anymore.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

97 matches

Mail list logo