[Mesa-dev] SSBO + access qualifiers

[Ilia Mirkin]

Hello,

I was going to add support for the various volatile/etc annotations in
my gallium impl (which is fairly far along), but it seems like those
are currently being dropped on the floor by lower_ubo_references, and
NIR has no support for them either. Both in GLSL IR and NIR, the
variable is what holds the access annotation (volatile, etc). However
the ssbo intrinsics are all about a particular binding and offset,
without any reference to the variable.

What's the right way of handling this? (And do any tests exist that
would be sensitive to getting such things wrong?)

Thanks,

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

[Mesa-dev] [PATCH] draw: don't set start_instance and instance id for pt emit

[sroland]

From: Roland Scheidegger 

This just adds confusion, these parameters are used when fetching vertices
by translate, but certainly not when emitting hw vertices for drivers, they
make no sense there (setting them has no consequences otherwise since there
won't be any elements with instance_divisor set). So just set them to 0 (the
draw_pipe_vbuf code for emitting vertices when the draw pipeline is run
already does exactly that).
Also while here do some whitespace cleanup.
---
 src/gallium/auxiliary/draw/draw_pt_emit.c | 62 +++
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_pt_emit.c 
b/src/gallium/auxiliary/draw/draw_pt_emit.c
index d1eafd8..0165e5a 100644
--- a/src/gallium/auxiliary/draw/draw_pt_emit.c
+++ b/src/gallium/auxiliary/draw/draw_pt_emit.c
@@ -60,7 +60,7 @@ draw_pt_emit_prepare(struct pt_emit *emit,
 
/* XXX: need to flush to get prim_vbuf.c to release its allocation??
 */
-   draw_do_flush( draw, DRAW_FLUSH_BACKEND );
+   draw_do_flush(draw, DRAW_FLUSH_BACKEND);
 
/* XXX: may need to defensively reset this later on as clipping can
 * clobber this state in the render backend.
@@ -80,7 +80,7 @@ draw_pt_emit_prepare(struct pt_emit *emit,
   unsigned emit_sz = 0;
   unsigned src_buffer = 0;
   unsigned output_format;
-  unsigned src_offset = (vinfo->attrib[i].src_index * 4 * sizeof(float) );
+  unsigned src_offset = (vinfo->attrib[i].src_index * 4 * sizeof(float));
 
   output_format = draw_translate_vinfo_format(vinfo->attrib[i].emit);
   emit_sz = draw_translate_vinfo_size(vinfo->attrib[i].emit);
@@ -89,8 +89,8 @@ draw_pt_emit_prepare(struct pt_emit *emit,
   assert(emit_sz != 0);
 
   if (vinfo->attrib[i].emit == EMIT_1F_PSIZE) {
-src_buffer = 1;
-src_offset = 0;
+ src_buffer = 1;
+ src_offset = 0;
   }
 
   hw_key.element[i].type = TRANSLATE_ELEMENT_NORMAL;
@@ -138,7 +138,7 @@ draw_pt_emit(struct pt_emit *emit,
 
/* XXX: need to flush to get prim_vbuf.c to release its allocation??
 */
-   draw_do_flush( draw, DRAW_FLUSH_BACKEND );
+   draw_do_flush(draw, DRAW_FLUSH_BACKEND);
 
if (vertex_count == 0)
   return;
@@ -152,31 +152,31 @@ draw_pt_emit(struct pt_emit *emit,
  (ushort)translate->key.output_stride,
  (ushort)vertex_count);
 
-   hw_verts = render->map_vertices( render );
+   hw_verts = render->map_vertices(render);
if (!hw_verts) {
   debug_warn_once("map of vertex buffer failed (out of memory?)");
   return;
}
 
translate->set_buffer(translate,
-0,
-vertex_data,
-stride,
-~0);
+ 0,
+ vertex_data,
+ stride,
+ ~0);
 
translate->set_buffer(translate,
-1,
->rasterizer->point_size,
-0,
-~0);
+ 1,
+ >rasterizer->point_size,
+ 0,
+ ~0);
 
/* fetch/translate vertex attribs to fill hw_verts[] */
translate->run(translate,
- 0,
- vertex_count,
-  draw->start_instance,
-  draw->instance_id,
- hw_verts );
+  0,
+  vertex_count,
+  0,
+  0,
+  hw_verts);
 
render->unmap_vertices(render, 0, vertex_count - 1);
 
@@ -212,7 +212,7 @@ draw_pt_emit_linear(struct pt_emit *emit,
 #endif
/* XXX: need to flush to get prim_vbuf.c to release its allocation??
 */
-   draw_do_flush( draw, DRAW_FLUSH_BACKEND );
+   draw_do_flush(draw, DRAW_FLUSH_BACKEND);
 
/* XXX: and work out some way to coordinate the render primitive
 * between vbuf.c and here...
@@ -224,35 +224,35 @@ draw_pt_emit_linear(struct pt_emit *emit,
   (ushort)count))
   goto fail;
 
-   hw_verts = render->map_vertices( render );
+   hw_verts = render->map_vertices(render);
if (!hw_verts)
   goto fail;
 
translate->set_buffer(translate, 0,
-vertex_data, stride, count - 1);
+ vertex_data, stride, count - 1);
 
translate->set_buffer(translate, 1,
->rasterizer->point_size,
-0, ~0);
+ >rasterizer->point_size,
+ 0, ~0);
 
translate->run(translate,
   0,
   count,
-  draw->start_instance,
-  draw->instance_id,
+  0,
+  0,
   hw_verts);
 
if (0) {
   unsigned i;
   for (i = 0; i < count; i++) {
  

[Mesa-dev] [PATCH 1/8] nir: Silence missing field initializer warnings for nir_src

[Ian Romanick]

From: Ian Romanick 

nir/nir.h: In function 'nir_src_for_ssa':
nir/nir.h:552:4: warning: missing initializer for field 'use_link' of 'nir_src'
[-Wmissing-field-initializers]
nir_src src = NIR_SRC_INIT;
^
In file included from nir/nir.c:28:0:
nir/nir.h:508:21: note: 'use_link' declared here
struct list_head use_link;
 ^

Number of total warnings in my build reduced from 2329 to 1932
(reduction of 397).

Signed-off-by: Ian Romanick 
---
 src/glsl/nir/nir.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
index 2e72e66..5543c52 100644
--- a/src/glsl/nir/nir.h
+++ b/src/glsl/nir/nir.h
@@ -515,7 +515,7 @@ typedef struct nir_src {
bool is_ssa;
 } nir_src;
 
-#define NIR_SRC_INIT (nir_src) { { NULL } }
+#define NIR_SRC_INIT (nir_src) { { NULL }, { NULL, NULL }, { { NULL, NULL, 0 } 
}, false }
 
 #define nir_foreach_use(reg_or_ssa_def, src) \
list_for_each_entry(nir_src, src, &(reg_or_ssa_def)->uses, use_link)
-- 
2.5.0

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

[Mesa-dev] [PATCH 7/8] nir: Silence unused parameter warnings

[Ian Romanick]

From: Ian Romanick 

These cases had the parameter removed:

nir/nir_lower_vec_to_movs.c: In function ‘try_coalesce’:
nir/nir_lower_vec_to_movs.c:124:66: warning: unused parameter ‘shader’ 
[-Wunused-parameter]
 try_coalesce(nir_alu_instr *vec, unsigned start_idx, nir_shader *shader)
  ^
nir/nir_lower_io.c: In function ‘load_op’:
nir/nir_lower_io.c:147:32: warning: unused parameter ‘state’ 
[-Wunused-parameter]
 load_op(struct lower_io_state *state,
^
nir/nir_lower_vars_to_ssa.c: In function 'get_ssa_def_for_block':
nir/nir_lower_vars_to_ssa.c:527:59: warning: unused parameter 'block' 
[-Wunused-parameter]
 get_ssa_def_for_block(struct deref_node *node, nir_block *block,
   ^
nir/nir_print.c: In function 'print_deref_array':
nir/nir_print.c:278:60: warning: unused parameter 'state' [-Wunused-parameter]
 print_deref_array(nir_deref_array *deref, print_var_state *state, FILE *fp)
^
nir/nir_print.c: In function 'print_deref_struct':
nir/nir_print.c:299:37: warning: unused parameter 'state' [-Wunused-parameter]
print_var_state *state, FILE *fp)
 ^
nir/nir_print.c: In function 'print_load_const_instr':
nir/nir_print.c:519:62: warning: unused parameter 'tabs' [-Wunused-parameter]
 print_load_const_instr(nir_load_const_instr *instr, unsigned tabs, FILE *fp)
  ^

These cases had the parameter (void) silenced because the parameter was
necessary for an interface:

ir/nir_move_vec_src_uses_to_dest.c: In function 
‘move_vec_src_uses_to_dest_block’:
nir/nir_move_vec_src_uses_to_dest.c:65:57: warning: unused parameter ‘shader’ 
[-Wunused-parameter]
 move_vec_src_uses_to_dest_block(nir_block *block, void *shader)
 ^
nir/glsl_to_nir.cpp:1900:32: warning: unused parameter 'ir' [-Wunused-parameter]
 nir_visitor::visit(ir_barrier *ir)
^
nir/nir_lower_load_const_to_scalar.c: In function 
'lower_load_const_to_scalar_block':
nir/nir_lower_load_const_to_scalar.c:80:58: warning: unused parameter 'data' 
[-Wunused-parameter]
 lower_load_const_to_scalar_block(nir_block *block, void *data)
  ^
nir/nir_lower_to_source_mods.c: In function 'nir_lower_to_source_mods_block':
nir/nir_lower_to_source_mods.c:37:56: warning: unused parameter 'state' 
[-Wunused-parameter]
 nir_lower_to_source_mods_block(nir_block *block, void *state)
^
nir/nir.c: In function ‘remove_use_cb’:
nir/nir.c:802:35: warning: unused parameter ‘state’ [-Wunused-parameter]
 remove_use_cb(nir_src *src, void *state)
   ^
nir/nir.c: In function ‘remove_def_cb’:
nir/nir.c:811:37: warning: unused parameter ‘state’ [-Wunused-parameter]
 remove_def_cb(nir_dest *dest, void *state)
 ^

Number of total warnings in my build reduced from 1485 to 1476
(reduction of 9).

Signed-off-by: Ian Romanick 
---
 src/glsl/nir/glsl_to_nir.cpp  | 2 +-
 src/glsl/nir/nir.c| 4 
 src/glsl/nir/nir_lower_io.c   | 5 ++---
 src/glsl/nir/nir_lower_load_const_to_scalar.c | 2 ++
 src/glsl/nir/nir_lower_to_source_mods.c   | 2 ++
 src/glsl/nir/nir_lower_vars_to_ssa.c  | 6 +++---
 src/glsl/nir/nir_lower_vec_to_movs.c  | 4 ++--
 src/glsl/nir/nir_move_vec_src_uses_to_dest.c  | 2 ++
 8 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
index db8b0ca..97bfadb 100644
--- a/src/glsl/nir/glsl_to_nir.cpp
+++ b/src/glsl/nir/glsl_to_nir.cpp
@@ -2076,7 +2076,7 @@ nir_visitor::visit(ir_dereference_array *ir)
 }
 
 void
-nir_visitor::visit(ir_barrier *ir)
+nir_visitor::visit(ir_barrier *)
 {
nir_intrinsic_instr *instr =
   nir_intrinsic_instr_create(this->shader, nir_intrinsic_barrier);
diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index 35fc1de..6bae05b 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -801,6 +801,8 @@ src_is_valid(const nir_src *src)
 static bool
 remove_use_cb(nir_src *src, void *state)
 {
+   (void) state;
+
if (src_is_valid(src))
   list_del(>use_link);
 
@@ -810,6 +812,8 @@ remove_use_cb(nir_src *src, void *state)
 static bool
 remove_def_cb(nir_dest *dest, void *state)
 {
+   (void) state;
+
if (!dest->is_ssa)
   list_del(>reg.def_link);
 
diff --git a/src/glsl/nir/nir_lower_io.c b/src/glsl/nir/nir_lower_io.c
index 3d646eb..8b421a0 100644
--- a/src/glsl/nir/nir_lower_io.c
+++ b/src/glsl/nir/nir_lower_io.c
@@ -144,8 +144,7 @@ get_io_offset(nir_builder *b, nir_deref_var 

[Mesa-dev] [PATCH 3/8] nir: Silence missing field initializer warnings for nir_alu_src

[Ian Romanick]

From: Ian Romanick 

nir/nir_builder.h:234:4: warning: missing initializer for field 'use_link' of 'n
ir_src' [-Wmissing-field-initializers]
nir_alu_src alu_src = { NIR_SRC_INIT };
^

Number of total warnings in my build reduced from 1676 to 1643
(reduction of 33).

Patch generated by:

egrep -lr 'nir_alu_src .* = \{ NIR_SRC_INIT \}' src/ | while read f
do
sed --in-place -e 's/nir_alu_src \(.*\) = { NIR_SRC_INIT }/nir_alu_src 
\1 = { NIR_SRC_INIT, false, false, { 0, } }/' $f
done

Signed-off-by: Ian Romanick 
---
 src/glsl/nir/nir_builder.h | 4 ++--
 src/glsl/nir/nir_search.c  | 2 +-
 src/mesa/program/prog_to_nir.c | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/glsl/nir/nir_builder.h b/src/glsl/nir/nir_builder.h
index b909f483..2a68883 100644
--- a/src/glsl/nir/nir_builder.h
+++ b/src/glsl/nir/nir_builder.h
@@ -240,7 +240,7 @@ static inline nir_ssa_def *
 nir_swizzle(nir_builder *build, nir_ssa_def *src, unsigned swiz[4],
 unsigned num_components, bool use_fmov)
 {
-   nir_alu_src alu_src = { NIR_SRC_INIT };
+   nir_alu_src alu_src = { NIR_SRC_INIT, false, false, { 0, } };
alu_src.src = nir_src_for_ssa(src);
for (unsigned i = 0; i < num_components; i++)
   alu_src.swizzle[i] = swiz[i];
@@ -268,7 +268,7 @@ nir_ssa_for_src(nir_builder *build, nir_src src, int 
num_components)
if (src.is_ssa && src.ssa->num_components == num_components)
   return src.ssa;
 
-   nir_alu_src alu = { NIR_SRC_INIT };
+   nir_alu_src alu = { NIR_SRC_INIT, false, false, { 0, } };
alu.src = src;
for (int j = 0; j < 4; j++)
   alu.swizzle[j] = j;
diff --git a/src/glsl/nir/nir_search.c b/src/glsl/nir/nir_search.c
index 56d7e81..ab8a7d6 100644
--- a/src/glsl/nir/nir_search.c
+++ b/src/glsl/nir/nir_search.c
@@ -289,7 +289,7 @@ construct_value(const nir_search_value *value, nir_alu_type 
type,
   const nir_search_variable *var = nir_search_value_as_variable(value);
   assert(state->variables_seen & (1 << var->variable));
 
-  nir_alu_src val = { NIR_SRC_INIT };
+  nir_alu_src val = { NIR_SRC_INIT, false, false, { 0, } };
   nir_alu_src_copy(, >variables[var->variable], mem_ctx);
 
   assert(!var->is_constant);
diff --git a/src/mesa/program/prog_to_nir.c b/src/mesa/program/prog_to_nir.c
index 539e3c0..c5d5687 100644
--- a/src/mesa/program/prog_to_nir.c
+++ b/src/mesa/program/prog_to_nir.c
@@ -183,7 +183,7 @@ ptn_get_src(struct ptn_compile *c, const struct 
prog_src_register *prog_src)
  if (prog_src->RelAddr) {
 deref_arr->deref_array_type = nir_deref_array_type_indirect;
 
-nir_alu_src addr_src = { NIR_SRC_INIT };
+nir_alu_src addr_src = { NIR_SRC_INIT, false, false, { 0, } };
 addr_src.src = nir_src_for_reg(c->addr_reg);
 nir_ssa_def *reladdr = nir_imov_alu(b, addr_src, 1);
 
@@ -936,7 +936,7 @@ ptn_add_output_stores(struct ptn_compile *c)
   * a vec4 with undefined .xyw components.  We resolve it to a scalar, 
to
   * match GLSL's gl_FragDepth and the expectations of most backends.
   */
- nir_alu_src alu_src = { NIR_SRC_INIT };
+ nir_alu_src alu_src = { NIR_SRC_INIT, false, false, { 0, } };
  alu_src.src = nir_src_for_reg(c->output_regs[FRAG_RESULT_DEPTH]);
  alu_src.swizzle[0] = SWIZZLE_Z;
  store->src[0] = nir_src_for_ssa(nir_fmov_alu(b, alu_src, 1));
-- 
2.5.0

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

Re: [Mesa-dev] [PATCH 1/8] nir: Silence missing field initializer warnings for nir_src

[Ian Romanick]

On 12/14/2015 03:38 PM, Ilia Mirkin wrote:
> It's a pretty standard feature of compilers to init things to 0 and
> not have the full structure specified like that... what compiler are
> you seeing these with? Can we just fix the glitch with a
> -Wno-stupid-warnings?

I have observed this with several versions of GCC.

In C, you can avoid this with a trailing comma like:

#define NIR_SRC_INIT (nir_src) { { NULL }, }

However, nir.h is also used in some C++ code where that doesn't help.

To be honest, I'm not a big fan of these macros.  Without C99 designated
initalizers, maintaining initializers like these (or the ones in
src/glsl/builtin_variables.cpp) is a real pain.  We can't use those, and
we can't use C++ constructors.  We have no good options available. :(

I thought about replacing them with a static inline function that
returns a zero-initialized struct.  The compiler should generate the
same code.  However, that doesn't work with uses like those in patch 3.

I'm also a little curious why you didn't raise this issue when I sent
these patches out in August.  I removed the patch from the series that
you objected to back then.

> On Mon, Dec 14, 2015 at 6:34 PM, Ian Romanick  wrote:
>> From: Ian Romanick 
>>
>> nir/nir.h: In function 'nir_src_for_ssa':
>> nir/nir.h:552:4: warning: missing initializer for field 'use_link' of 
>> 'nir_src'
>> [-Wmissing-field-initializers]
>> nir_src src = NIR_SRC_INIT;
>> ^
>> In file included from nir/nir.c:28:0:
>> nir/nir.h:508:21: note: 'use_link' declared here
>> struct list_head use_link;
>>  ^
>>
>> Number of total warnings in my build reduced from 2329 to 1932
>> (reduction of 397).
>>
>> Signed-off-by: Ian Romanick 
>> ---
>>  src/glsl/nir/nir.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
>> index 2e72e66..5543c52 100644
>> --- a/src/glsl/nir/nir.h
>> +++ b/src/glsl/nir/nir.h
>> @@ -515,7 +515,7 @@ typedef struct nir_src {
>> bool is_ssa;
>>  } nir_src;
>>
>> -#define NIR_SRC_INIT (nir_src) { { NULL } }
>> +#define NIR_SRC_INIT (nir_src) { { NULL }, { NULL, NULL }, { { NULL, NULL, 
>> 0 } }, false }
>>
>>  #define nir_foreach_use(reg_or_ssa_def, src) \
>> list_for_each_entry(nir_src, src, &(reg_or_ssa_def)->uses, use_link)
>> --
>> 2.5.0
>>
>> ___
>> 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 v2] mesa: fix interface matching done in validate_io

[Timothy Arceri]

On Mon, 2015-12-14 at 10:29 +0200, Tapani Pälli wrote:
> Patch makes following changes for interface matching:
> 
>- do not try to match builtin variables
>- handle swizzle in input name, as example 'a.z' should
>  match with 'a'
>- check that amount of inputs and outputs matches
> 
> These changes make interface matching tests to work in:
>ES31-CTS.sepshaderobjs.StateInteraction
> 
> Test does not still pass completely due to errors in rendering
> output. IMO this is unrelated to interface matching.
> 
> v2: add spec reference, return true on desktop since we do not
> have failing cases for it, inputs and outputs amount do not
> need to match on desktop.
> 
> Signed-off-by: Tapani Pälli 

Hi Tapani,

Just a general comment first.

I think we should first move _mesa_validate_pipeline_io() and
 validate_io() to src/mesa/main/pipelineobj.c I don't think it belongs
here right?


> ---
>  src/mesa/main/shader_query.cpp | 54
> ++
>  1 file changed, 50 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mesa/main/shader_query.cpp
> b/src/mesa/main/shader_query.cpp
> index ced10a9..bc01b97 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -1377,19 +1377,38 @@ validate_io(const struct gl_shader
> *input_stage,
>  const struct gl_shader *output_stage, bool isES)
>  {
> assert(input_stage && output_stage);
> +   unsigned inputs = 0, outputs = 0;
> +
> +   /* Currently no matching done for desktop. */

I think the spec quote should be moved here as it applies to all the
rules in the function then you can also have the comment explaining why
validation for desktop it not done.

I've also filed a spec bug for desktop for the reasons I outlined in
irc previously. It would be great if you could quote the bug here also.
Something like:

/* FIXME: Update once Khronos spec bug #15331 is resolved. */

> +   if (!isES)
> +  return true;
>  
> /* For each output in a, find input in b and do any required
> checks. */
> foreach_in_list(ir_instruction, out, input_stage->ir) {
>ir_variable *out_var = out->as_variable();


It's existing code but it would also be nice to have a patch that
renames input_stage/output_stage to producer_stage/consumer_stage this
it what they are called in the linker code. Maybe its just me but
getting the outputs from input_stage just looks wrong.


> -  if (!out_var || out_var->data.mode != ir_var_shader_out)
> +  if (!out_var || out_var->data.mode != ir_var_shader_out ||
> +  is_gl_identifier(out_var->name))
>   continue;
>  
> +  outputs++;
> +
> +  inputs = 0;
>foreach_in_list(ir_instruction, in, output_stage->ir) {

Two comments here:

1. Take a look at cross_validate_outputs_to_inputs() in
link_varyings.cpp for a way to avoid the nested loop? Although it may
cause even more overhaed using the symbol table not sure.

2. Take a look at the same function for matching via explicit location.
Does the CTS not test for mismatched explicit locations? Maybe we
should create a piglit test for this as your existing code doesn't take
into account explicit locations.

I was going to suggest sharing the code between here and the linker
however I'm about to add a bunch of rules for matching the component
qualifier for enhanced layouts so not entirely sure if we should do
this what do you think?

>   ir_variable *in_var = in->as_variable();
> - if (!in_var || in_var->data.mode != ir_var_shader_in)
> + if (!in_var || in_var->data.mode != ir_var_shader_in ||
> + is_gl_identifier(in_var->name))
>  continue;
>  
> - if (strcmp(in_var->name, out_var->name) == 0) {
> + inputs++;
> +
> + unsigned len = strlen(in_var->name);
> +
> + /* Handle input swizzle in variable name. */
> + const char *dot = strchr(in_var->name, '.');
> + if (dot)
> +len = dot - in_var->name;

Hmm ... I wonder if this is also missing from the linker or maybe the
symbol table stuff handles this.

> +
> + if (strncmp(in_var->name, out_var->name, len) == 0) {
>  /* Since we now only validate precision, we can skip
> this step for
>   * desktop GLSL shaders, there precision qualifier is
> ignored.
>   *
> @@ -1412,7 +1431,34 @@ validate_io(const struct gl_shader
> *input_stage,
>   }
>}
> }
> -   return true;
> +
> +   /* Amount of inputs vs outputs should match when using OpenGL ES.
> +*
> +* From OpenGL ES 3.1 spec (Interface matching):
> +*
> +*"At an interface between program objects, the set of inputs
> and outputs
> +*are considered to match exactly if and only if:
> +*
> +*- Every declared input variable has a matching output, as
> described
> +*above.
> +*
> +*- There are no user-defined output variables 

[Mesa-dev] [Bug 80183] [llvmpipe] triangles with vertices that map to raster positions > viewport width/height are not displayed

[bugzilla-daemon]

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

Roland Scheidegger  changed:

   What|Removed |Added

 CC||cgerlac...@gmail.com

-- 
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

Re: [Mesa-dev] [PATCH 3/3] i965: Use MESA_FORMAT_B8G8R8X8_SRGB for RGB visuals

[Jason Ekstrand]

On Dec 13, 2015 2:06 PM, "Mike Lothian"  wrote:
>
> Hi
>
> These three commits have stopped Plasma5's kwin  working on my skylake
system

As another data point, it also breaks XWayland+DRI3 on my BDW. I can't even
run glsgears without it segfaulting.

> Reverting back to 7752bbc44e78e982de3cd4c34862adc38a338234 fixed it for me
>
> I can send you more details / raise a bug if you like
>
> Cheers
>
> Mike
>
> On Sat, 12 Dec 2015 at 20:56 Kenneth Graunke 
wrote:
>>
>> On Friday, December 11, 2015 12:32:18 PM Neil Roberts wrote:
>> > Previously if the visual didn't have an alpha channel then it would
>> > pick a format that is not sRGB-capable. I don't think there's any
>> > reason not to always have an sRGB-capable visual. Since 28090b30 there
>> > are now visuals advertised without an alpha channel which means that
>> > games that don't request alpha bits in the config would end up without
>> > an sRGB-capable visual. This was breaking supertuxkart which assumes
>> > the winsys buffer is always sRGB-capable.
>> >
>> > The previous code always used an RGBA format if the visual config
>> > itself was marked as sRGB-capable regardless of whether the visual has
>> > alpha bits. I think we don't actually advertise any sRGB-capable
>> > visuals (but we just use sRGB formats anyway) so it shouldn't make any
>> > difference. However this patch also changes it to use RGBX if an
>> > sRGB-capable visual is requested without alpha bits for consistency.
>> >
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92759
>> > Cc: "11.0 11.1" 
>> > Cc: Ilia Mirkin 
>> > Suggested-by: Ilia Mirkin 
>> > ---
>> >  src/mesa/drivers/dri/i965/intel_screen.c | 13 ++---
>> >  1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> The whole series is:
>> Reviewed-by: Kenneth Graunke 
>>
>> We definitely should have the same behavior regardless of whether the
>> config has an alpha channel.  So, this is good.
>> ___
>> 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 mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 8/8] nir: Remove empty visit_call_src and visit_load_const_src functions

[Ian Romanick]

From: Ian Romanick 

The guts were removed in dfb3abba.  It has been almost exactly a year,
so I dont think we're going to "decide we want [predication] back."

Silences several "unused parameter" warnings:

nir/nir.c: In function ‘visit_call_src’:
nir/nir.c:1052:32: warning: unused parameter ‘instr’ [-Wunused-parameter]
 visit_call_src(nir_call_instr *instr, nir_foreach_src_cb cb, void *state)
^
nir/nir.c:1052:58: warning: unused parameter ‘cb’ [-Wunused-parameter]
 visit_call_src(nir_call_instr *instr, nir_foreach_src_cb cb, void *state)
  ^
nir/nir.c:1052:68: warning: unused parameter ‘state’ [-Wunused-parameter]
 visit_call_src(nir_call_instr *instr, nir_foreach_src_cb cb, void *state)
^
nir/nir.c: In function ‘visit_load_const_src’:
nir/nir.c:1058:44: warning: unused parameter ‘instr’ [-Wunused-parameter]
 visit_load_const_src(nir_load_const_instr *instr, nir_foreach_src_cb cb,
^
nir/nir.c:1058:70: warning: unused parameter ‘cb’ [-Wunused-parameter]
 visit_load_const_src(nir_load_const_instr *instr, nir_foreach_src_cb cb,
  ^
nir/nir.c:1059:28: warning: unused parameter ‘state’ [-Wunused-parameter]
  void *state)
^

Signed-off-by: Ian Romanick 
Cc: Jason Ekstrand 
Cc: Connor Abbott 
---
 src/glsl/nir/nir.c | 17 -
 1 file changed, 17 deletions(-)

diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
index 6bae05b..8c86135 100644
--- a/src/glsl/nir/nir.c
+++ b/src/glsl/nir/nir.c
@@ -1049,19 +1049,6 @@ visit_intrinsic_src(nir_intrinsic_instr *instr, 
nir_foreach_src_cb cb,
 }
 
 static bool
-visit_call_src(nir_call_instr *instr, nir_foreach_src_cb cb, void *state)
-{
-   return true;
-}
-
-static bool
-visit_load_const_src(nir_load_const_instr *instr, nir_foreach_src_cb cb,
- void *state)
-{
-   return true;
-}
-
-static bool
 visit_phi_src(nir_phi_instr *instr, nir_foreach_src_cb cb, void *state)
 {
nir_foreach_phi_src(instr, src) {
@@ -1117,12 +1104,8 @@ nir_foreach_src(nir_instr *instr, nir_foreach_src_cb cb, 
void *state)
  return false;
   break;
case nir_instr_type_call:
-  if (!visit_call_src(nir_instr_as_call(instr), cb, state))
- return false;
   break;
case nir_instr_type_load_const:
-  if (!visit_load_const_src(nir_instr_as_load_const(instr), cb, state))
- return false;
   break;
case nir_instr_type_phi:
   if (!visit_phi_src(nir_instr_as_phi(instr), cb, state))
-- 
2.5.0

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

[Mesa-dev] [PATCH 6/8] nir: Trivial clean ups in the generated nir_constant_expressions.c

[Ian Romanick]

From: Ian Romanick 

Signed-off-by: Ian Romanick 
---
 src/glsl/nir/nir_constant_expressions.py | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/glsl/nir/nir_constant_expressions.py 
b/src/glsl/nir/nir_constant_expressions.py
index a83c12e..d82b865 100644
--- a/src/glsl/nir/nir_constant_expressions.py
+++ b/src/glsl/nir/nir_constant_expressions.py
@@ -219,7 +219,6 @@ static nir_const_value
 evaluate_${name}(unsigned num_components, nir_const_value *_src)
 {
nir_const_value _dst_val = { { {0, 0, 0, 0} } };
-
## For each non-per-component input, create a variable srcN that
## contains x, y, z, and w elements which are filled in with the
## appropriately-typed values.
@@ -231,7 +230,7 @@ evaluate_${name}(unsigned num_components, nir_const_value 
*_src)
  <% continue %>
   %endif
 
-  struct ${op.input_types[j]}_vec src${j} = {
+  const struct ${op.input_types[j]}_vec src${j} = {
   % for k in range(op.input_sizes[j]):
  % if op.input_types[j] == "bool":
 _src[${j}].u[${k}] != 0,
@@ -273,17 +272,17 @@ evaluate_${name}(unsigned num_components, nir_const_value 
*_src)
## Avoid unused variable warnings
<% continue %>
 % elif op.input_types[j] == "bool":
-   bool src${j} = _src[${j}].u[_i] != 0;
+   const bool src${j} = _src[${j}].u[_i] != 0;
 % else:
-   ${op.input_types[j]} src${j} = 
_src[${j}].${op.input_types[j][:1]}[_i];
+   const ${op.input_types[j]} src${j} = 
_src[${j}].${op.input_types[j][:1]}[_i];
 % endif
  % endfor
-
  ## Create an appropriately-typed variable dst and assign the
  ## result of the const_expr to it.  If const_expr already contains
  ## writes to dst, just include const_expr directly.
  % if "dst" in op.const_expr:
 ${op.output_type} dst;
+
 ${op.const_expr}
  % else:
 ${op.output_type} dst = ${op.const_expr};
@@ -337,10 +336,8 @@ nir_eval_const_opcode(nir_op op, unsigned num_components,
 {
switch (op) {
 % for name in sorted(opcodes.iterkeys()):
-   case nir_op_${name}: {
+   case nir_op_${name}:
   return evaluate_${name}(num_components, src);
-  break;
-   }
 % endfor
default:
   unreachable("shouldn't get here");
-- 
2.5.0

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

[Mesa-dev] [PATCH 4/8] nir: Silence missing field initializer warnings for vectors in nir_constant_expressions

[Ian Romanick]

From: Ian Romanick 

nir/nir_constant_expressions.c: In function 'evaluate_ball2':
nir/nir_constant_expressions.c:279:7: warning: missing initializer for field 
'z' of 'struct bool_vec' [-Wmissing-field-initializers]
   };
   ^
nir/nir_constant_expressions.c:234:10: note: 'z' declared here
bool z;
  ^

Number of total warnings in my build reduced from 1643 to 1574
(reduction of 69).

Signed-off-by: Ian Romanick 
---
 src/glsl/nir/nir_constant_expressions.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/glsl/nir/nir_constant_expressions.py 
b/src/glsl/nir/nir_constant_expressions.py
index 32784f6..81dd67f 100644
--- a/src/glsl/nir/nir_constant_expressions.py
+++ b/src/glsl/nir/nir_constant_expressions.py
@@ -239,6 +239,13 @@ evaluate_${name}(unsigned num_components, nir_const_value 
*_src)
 _src[${j}].${op.input_types[j][:1]}[${k}],
  % endif
   % endfor
+  % for k in range(op.input_sizes[j], 4):
+ % if op.input_types[j] == "bool":
+false,
+ % else:
+0,
+ % endif
+  % endfor
   };
% endfor
 
-- 
2.5.0

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

[Mesa-dev] [PATCH 5/8] nir: Silence unused parameter warnings in nir_constant_expression.c

[Ian Romanick]

From: Ian Romanick 

nir/nir_constant_expressions.c:290:25: warning: unused parameter 
'num_components' [-Wunused-parameter]
 evaluate_ball3(unsigned num_components, nir_const_value *_src)
 ^
nir/nir_constant_expressions.c: In function 'evaluate_fddx':
nir/nir_constant_expressions.c:1282:57: warning: unused parameter '_src' 
[-Wunused-parameter]
 evaluate_fddx(unsigned num_components, nir_const_value *_src)
 ^

Number of total warnings in my build reduced from 1575 to 1485
(reduction of 89).

Signed-off-by: Ian Romanick 
---
 src/glsl/nir/nir_constant_expressions.py | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/src/glsl/nir/nir_constant_expressions.py 
b/src/glsl/nir/nir_constant_expressions.py
index 81dd67f..a83c12e 100644
--- a/src/glsl/nir/nir_constant_expressions.py
+++ b/src/glsl/nir/nir_constant_expressions.py
@@ -248,6 +248,16 @@ evaluate_${name}(unsigned num_components, nir_const_value 
*_src)
   % endfor
   };
% endfor
+   % if op.output_size != 0 or "src" not in op.const_expr:
+
+   /* Silence compiler warnings. */
+  % if op.output_size != 0:
+   (void) num_components;
+  % endif
+  % if "src" not in op.const_expr:
+   (void) _src;
+  % endif
+   % endif
 
% if op.output_size == 0:
   ## For per-component instructions, we need to iterate over the
-- 
2.5.0

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

[Mesa-dev] [PATCH 2/8] nir: Silence missing field initializer warnings for nir_dest

[Ian Romanick]

From: Ian Romanick 

nir/nir.h:576:4: warning: missing initializer for field 'is_ssa' of 'nir_dest' [
-Wmissing-field-initializers]
nir_dest dest = NIR_DEST_INIT;
^
nir/nir.h:538:10: note: 'is_ssa' declared here
bool is_ssa;
  ^
nir/nir.h:576:55: warning: missing initializer for member 'nir_reg_dest::def_lin
k' [-Wmissing-field-initializers]
nir_dest dest = NIR_DEST_INIT;

Number of total warnings in my build reduced from 1932 to 1676
(reduction of 256).

Signed-off-by: Ian Romanick 
---
 src/glsl/nir/nir.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
index 5543c52..94f4c74 100644
--- a/src/glsl/nir/nir.h
+++ b/src/glsl/nir/nir.h
@@ -538,7 +538,7 @@ typedef struct {
bool is_ssa;
 } nir_dest;
 
-#define NIR_DEST_INIT (nir_dest) { { { NULL } } }
+#define NIR_DEST_INIT (nir_dest) { { { NULL, { NULL, NULL }, NULL, NULL, 0 } 
}, false }
 
 #define nir_foreach_def(reg, dest) \
list_for_each_entry(nir_dest, dest, &(reg)->defs, reg.def_link)
-- 
2.5.0

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

Re: [Mesa-dev] [PATCH 1/8] nir: Silence missing field initializer warnings for nir_src

[Ilia Mirkin]

On Mon, Dec 14, 2015 at 7:28 PM, Ian Romanick  wrote:
> On 12/14/2015 03:38 PM, Ilia Mirkin wrote:
>> It's a pretty standard feature of compilers to init things to 0 and
>> not have the full structure specified like that... what compiler are
>> you seeing these with? Can we just fix the glitch with a
>> -Wno-stupid-warnings?
>
> I have observed this with several versions of GCC.
>
> In C, you can avoid this with a trailing comma like:
>
> #define NIR_SRC_INIT (nir_src) { { NULL }, }
>
> However, nir.h is also used in some C++ code where that doesn't help.
>
> To be honest, I'm not a big fan of these macros.  Without C99 designated
> initalizers, maintaining initializers like these (or the ones in
> src/glsl/builtin_variables.cpp) is a real pain.  We can't use those, and
> we can't use C++ constructors.  We have no good options available. :(
>
> I thought about replacing them with a static inline function that
> returns a zero-initialized struct.  The compiler should generate the
> same code.  However, that doesn't work with uses like those in patch 3.
>
> I'm also a little curious why you didn't raise this issue when I sent
> these patches out in August.  I removed the patch from the series that
> you objected to back then.

I have absolutely no recollection of any of that. Perhaps I saw "nir"
and thought to myself, "don't care, let them do whatever, this won't
ever affect me". Which is a sentiment I'm happy to continue with, by
the way.

I know that doing

x = {}

is a gcc extension, but I thought that {0} should always work (with
enough {} nesting in case the first element is a struct). Perhaps it
doesn't in C++? I could believe that, although I'd be surprised.

Anyways, didn't mean to stir the pot too much, just thought there
might be a simpler way out of all this.

Cheers,

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

Re: [Mesa-dev] [PATCH 1/8] nir: Silence missing field initializer warnings for nir_src

[Ian Romanick]

On 12/14/2015 04:39 PM, Ilia Mirkin wrote:
> On Mon, Dec 14, 2015 at 7:28 PM, Ian Romanick  wrote:
>> On 12/14/2015 03:38 PM, Ilia Mirkin wrote:
>>> It's a pretty standard feature of compilers to init things to 0 and
>>> not have the full structure specified like that... what compiler are
>>> you seeing these with? Can we just fix the glitch with a
>>> -Wno-stupid-warnings?
>>
>> I have observed this with several versions of GCC.
>>
>> In C, you can avoid this with a trailing comma like:
>>
>> #define NIR_SRC_INIT (nir_src) { { NULL }, }
>>
>> However, nir.h is also used in some C++ code where that doesn't help.
>>
>> To be honest, I'm not a big fan of these macros.  Without C99 designated
>> initalizers, maintaining initializers like these (or the ones in
>> src/glsl/builtin_variables.cpp) is a real pain.  We can't use those, and
>> we can't use C++ constructors.  We have no good options available. :(
>>
>> I thought about replacing them with a static inline function that
>> returns a zero-initialized struct.  The compiler should generate the
>> same code.  However, that doesn't work with uses like those in patch 3.
>>
>> I'm also a little curious why you didn't raise this issue when I sent
>> these patches out in August.  I removed the patch from the series that
>> you objected to back then.
> 
> I have absolutely no recollection of any of that. Perhaps I saw "nir"
> and thought to myself, "don't care, let them do whatever, this won't
> ever affect me". Which is a sentiment I'm happy to continue with, by
> the way.

Fair enough. :)  The patch I removed was one that removed the gl_context
parameter from a function in dd_function_table.

http://patchwork.freedesktop.org/patch/58048/

> I know that doing
> 
> x = {}
> 
> is a gcc extension, but I thought that {0} should always work (with
> enough {} nesting in case the first element is a struct). Perhaps it

{0} is, basically what we're doing now, and GCC complains about it with
-Wmissing-field-initializers or -Wextra.  When we added C-style struct
and array initializers to GLSL, we discussed adding this sort of
implicit zero initialization.  I did some digging in the C89 and C99
specs, and I have some recollection that in this case the missing fields
get undefined values... but, starting with C99, {0, } implicitly
initializes the missing fields to zero.  I also seem to recall that bit
of weirdness in C is why quite a few people were opposed to adding it to
GLSL.  This was several years ago, so my memory may not be completely
reliable.

> doesn't in C++? I could believe that, although I'd be surprised.

The initializer support in C++ intentionally quite a bit more primitive
than in C99.  The language designers want you to use constructors
whether it's the best tool for the job or not... which is why there are
no designated initializers.

> Anyways, didn't mean to stir the pot too much, just thought there
> might be a simpler way out of all this.

Well, there are. :) We just can't use them due to some combination of
MSVC, C++, and C99.

> Cheers,
> 
>   -ilia

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

Re: [Mesa-dev] [PATCH] draw: don't set start_instance and instance id for pt emit

[Brian Paul]

Just one minor nit below.

Reviewed-by: Brian Paul 


On 12/14/2015 05:58 PM, srol...@vmware.com wrote:

From: Roland Scheidegger 

This just adds confusion, these parameters are used when fetching vertices
by translate, but certainly not when emitting hw vertices for drivers, they
make no sense there (setting them has no consequences otherwise since there
won't be any elements with instance_divisor set). So just set them to 0 (the
draw_pipe_vbuf code for emitting vertices when the draw pipeline is run
already does exactly that).
Also while here do some whitespace cleanup.
---
  src/gallium/auxiliary/draw/draw_pt_emit.c | 62 +++
  1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_pt_emit.c 
b/src/gallium/auxiliary/draw/draw_pt_emit.c
index d1eafd8..0165e5a 100644
--- a/src/gallium/auxiliary/draw/draw_pt_emit.c
+++ b/src/gallium/auxiliary/draw/draw_pt_emit.c
@@ -60,7 +60,7 @@ draw_pt_emit_prepare(struct pt_emit *emit,

 /* XXX: need to flush to get prim_vbuf.c to release its allocation??
  */
-   draw_do_flush( draw, DRAW_FLUSH_BACKEND );
+   draw_do_flush(draw, DRAW_FLUSH_BACKEND);

 /* XXX: may need to defensively reset this later on as clipping can
  * clobber this state in the render backend.
@@ -80,7 +80,7 @@ draw_pt_emit_prepare(struct pt_emit *emit,
unsigned emit_sz = 0;
unsigned src_buffer = 0;
unsigned output_format;
-  unsigned src_offset = (vinfo->attrib[i].src_index * 4 * sizeof(float) );
+  unsigned src_offset = (vinfo->attrib[i].src_index * 4 * sizeof(float));


You could drop the outer parens completely.



output_format = draw_translate_vinfo_format(vinfo->attrib[i].emit);
emit_sz = draw_translate_vinfo_size(vinfo->attrib[i].emit);
@@ -89,8 +89,8 @@ draw_pt_emit_prepare(struct pt_emit *emit,
assert(emit_sz != 0);

if (vinfo->attrib[i].emit == EMIT_1F_PSIZE) {
-src_buffer = 1;
-src_offset = 0;
+ src_buffer = 1;
+ src_offset = 0;
}

hw_key.element[i].type = TRANSLATE_ELEMENT_NORMAL;
@@ -138,7 +138,7 @@ draw_pt_emit(struct pt_emit *emit,

 /* XXX: need to flush to get prim_vbuf.c to release its allocation??
  */
-   draw_do_flush( draw, DRAW_FLUSH_BACKEND );
+   draw_do_flush(draw, DRAW_FLUSH_BACKEND);

 if (vertex_count == 0)
return;
@@ -152,31 +152,31 @@ draw_pt_emit(struct pt_emit *emit,
   (ushort)translate->key.output_stride,
   (ushort)vertex_count);

-   hw_verts = render->map_vertices( render );
+   hw_verts = render->map_vertices(render);
 if (!hw_verts) {
debug_warn_once("map of vertex buffer failed (out of memory?)");
return;
 }

 translate->set_buffer(translate,
-0,
-vertex_data,
-stride,
-~0);
+ 0,
+ vertex_data,
+ stride,
+ ~0);

 translate->set_buffer(translate,
-1,
->rasterizer->point_size,
-0,
-~0);
+ 1,
+ >rasterizer->point_size,
+ 0,
+ ~0);

 /* fetch/translate vertex attribs to fill hw_verts[] */
 translate->run(translate,
- 0,
- vertex_count,
-  draw->start_instance,
-  draw->instance_id,
- hw_verts );
+  0,
+  vertex_count,
+  0,
+  0,
+  hw_verts);

 render->unmap_vertices(render, 0, vertex_count - 1);

@@ -212,7 +212,7 @@ draw_pt_emit_linear(struct pt_emit *emit,
  #endif
 /* XXX: need to flush to get prim_vbuf.c to release its allocation??
  */
-   draw_do_flush( draw, DRAW_FLUSH_BACKEND );
+   draw_do_flush(draw, DRAW_FLUSH_BACKEND);

 /* XXX: and work out some way to coordinate the render primitive
  * between vbuf.c and here...
@@ -224,35 +224,35 @@ draw_pt_emit_linear(struct pt_emit *emit,
(ushort)count))
goto fail;

-   hw_verts = render->map_vertices( render );
+   hw_verts = render->map_vertices(render);
 if (!hw_verts)
goto fail;

 translate->set_buffer(translate, 0,
-vertex_data, stride, count - 1);
+ vertex_data, stride, count - 1);

 translate->set_buffer(translate, 1,
->rasterizer->point_size,
-0, ~0);
+ >rasterizer->point_size,
+ 0, ~0);

 translate->run(translate,
0,
count,
-  

Re: [Mesa-dev] [PATCH] clover: Fix build against LLVM 3.8 SVN >= r255078

[Ilia Mirkin]

On Wed, Dec 9, 2015 at 5:30 AM, Francisco Jerez  wrote:
> Michel Dänzer  writes:
>
>> From: Michel Dänzer 
>>
>> Signed-off-by: Michel Dänzer 
>
> Looks OK to me,
> Reviewed-by: Francisco Jerez 
>
>> ---
>>  src/gallium/state_trackers/clover/llvm/invocation.cpp | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp 
>> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> index 3b37f08..4d11c24 100644
>> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> @@ -661,7 +661,11 @@ namespace {
>>
>>if (dump_asm) {
>>   LLVMSetTargetMachineAsmVerbosity(tm, true);
>> +#if HAVE_LLVM >= 0x0308
>> + LLVMModuleRef debug_mod = wrap(llvm::CloneModule(mod).release());
>> +#else
>>   LLVMModuleRef debug_mod = wrap(llvm::CloneModule(mod));
>> +#endif
>>   emit_code(tm, debug_mod, LLVMAssemblyFile, _buffer, r_log);
>>   buffer_size = LLVMGetBufferSize(out_buffer);
>>   buffer_data = LLVMGetBufferStart(out_buffer);

Emil, consider cherry-picking this into 11.1 and perhaps even 11.0 to
save people from unnecessary compilation trouble. This is commit
b4a03e7f8f upstream.

Cheers,

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

Re: [Mesa-dev] [PATCH 8/8] nir: Remove empty visit_call_src and visit_load_const_src functions

[Jason Ekstrand]

On Mon, Dec 14, 2015 at 3:34 PM, Ian Romanick  wrote:
> From: Ian Romanick 
>
> The guts were removed in dfb3abba.  It has been almost exactly a year,
> so I dont think we're going to "decide we want [predication] back."

Yeah, I think that bridge is burned.

> Silences several "unused parameter" warnings:
>
> nir/nir.c: In function ‘visit_call_src’:
> nir/nir.c:1052:32: warning: unused parameter ‘instr’ [-Wunused-parameter]
>  visit_call_src(nir_call_instr *instr, nir_foreach_src_cb cb, void *state)
> ^
> nir/nir.c:1052:58: warning: unused parameter ‘cb’ [-Wunused-parameter]
>  visit_call_src(nir_call_instr *instr, nir_foreach_src_cb cb, void *state)
>   ^
> nir/nir.c:1052:68: warning: unused parameter ‘state’ [-Wunused-parameter]
>  visit_call_src(nir_call_instr *instr, nir_foreach_src_cb cb, void *state)
> ^
> nir/nir.c: In function ‘visit_load_const_src’:
> nir/nir.c:1058:44: warning: unused parameter ‘instr’ [-Wunused-parameter]
>  visit_load_const_src(nir_load_const_instr *instr, nir_foreach_src_cb cb,
> ^
> nir/nir.c:1058:70: warning: unused parameter ‘cb’ [-Wunused-parameter]
>  visit_load_const_src(nir_load_const_instr *instr, nir_foreach_src_cb cb,
>   ^
> nir/nir.c:1059:28: warning: unused parameter ‘state’ [-Wunused-parameter]
>   void *state)
> ^
>
> Signed-off-by: Ian Romanick 
> Cc: Jason Ekstrand 
> Cc: Connor Abbott 
> ---
>  src/glsl/nir/nir.c | 17 -
>  1 file changed, 17 deletions(-)
>
> diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
> index 6bae05b..8c86135 100644
> --- a/src/glsl/nir/nir.c
> +++ b/src/glsl/nir/nir.c
> @@ -1049,19 +1049,6 @@ visit_intrinsic_src(nir_intrinsic_instr *instr, 
> nir_foreach_src_cb cb,
>  }
>
>  static bool
> -visit_call_src(nir_call_instr *instr, nir_foreach_src_cb cb, void *state)
> -{
> -   return true;
> -}
> -
> -static bool
> -visit_load_const_src(nir_load_const_instr *instr, nir_foreach_src_cb cb,
> - void *state)
> -{
> -   return true;
> -}
> -
> -static bool
>  visit_phi_src(nir_phi_instr *instr, nir_foreach_src_cb cb, void *state)
>  {
> nir_foreach_phi_src(instr, src) {
> @@ -1117,12 +1104,8 @@ nir_foreach_src(nir_instr *instr, nir_foreach_src_cb 
> cb, void *state)
>   return false;
>break;
> case nir_instr_type_call:
> -  if (!visit_call_src(nir_instr_as_call(instr), cb, state))
> - return false;

Please at least add a "Call instructions have no regular sources" comment.

>break;
> case nir_instr_type_load_const:
> -  if (!visit_load_const_src(nir_instr_as_load_const(instr), cb, state))
> - return false;

Same for constant loads

With that added,

Reviewed-by: Jason Ekstrand 

>break;
> case nir_instr_type_phi:
>if (!visit_phi_src(nir_instr_as_phi(instr), cb, state))
> --
> 2.5.0
>
> ___
> 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/8] nir: Silence missing field initializer warnings for nir_src

[Ilia Mirkin]

It's a pretty standard feature of compilers to init things to 0 and
not have the full structure specified like that... what compiler are
you seeing these with? Can we just fix the glitch with a
-Wno-stupid-warnings?

On Mon, Dec 14, 2015 at 6:34 PM, Ian Romanick  wrote:
> From: Ian Romanick 
>
> nir/nir.h: In function 'nir_src_for_ssa':
> nir/nir.h:552:4: warning: missing initializer for field 'use_link' of 
> 'nir_src'
> [-Wmissing-field-initializers]
> nir_src src = NIR_SRC_INIT;
> ^
> In file included from nir/nir.c:28:0:
> nir/nir.h:508:21: note: 'use_link' declared here
> struct list_head use_link;
>  ^
>
> Number of total warnings in my build reduced from 2329 to 1932
> (reduction of 397).
>
> Signed-off-by: Ian Romanick 
> ---
>  src/glsl/nir/nir.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> index 2e72e66..5543c52 100644
> --- a/src/glsl/nir/nir.h
> +++ b/src/glsl/nir/nir.h
> @@ -515,7 +515,7 @@ typedef struct nir_src {
> bool is_ssa;
>  } nir_src;
>
> -#define NIR_SRC_INIT (nir_src) { { NULL } }
> +#define NIR_SRC_INIT (nir_src) { { NULL }, { NULL, NULL }, { { NULL, NULL, 0 
> } }, false }
>
>  #define nir_foreach_use(reg_or_ssa_def, src) \
> list_for_each_entry(nir_src, src, &(reg_or_ssa_def)->uses, use_link)
> --
> 2.5.0
>
> ___
> 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/8] st/va: remove nonesense HEVC picture id handling

[Christian König]

Yeah, at least that looks sane, but I don't think just continuing with 
the loop is the right things to do here.


E.g. if we just continue h265.ref[i] will never be initialized to 
something. It's either NULL, a value from the last decode or some random 
data.


vlVaGetReferenceFrame() can handle VA_INVALID_SURFACE, so that isn't 
much of a problem. I'm just not sure what to do with 
VA_PICTURE_HEVC_INVALID.


Regards,
Christian.

On 14.12.2015 09:46, Julien Isorce wrote:

Hi, shouldn't the check be the following condition instead:
hevc->ReferenceFrames[i].flags & VA_PICTURE_HEVC_INVALID ||
hevc->ReferenceFrames[i].picture_id == VA_INVALID_SURFACE
?

On 11 December 2015 at 12:33, Christian König > wrote:


From: Christian König >

The picture id in this case is a VA-API surface handle, checking
for a certain value can't be correct.

Signed-off-by: Christian König >
---
 src/gallium/state_trackers/va/picture_hevc.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/gallium/state_trackers/va/picture_hevc.c
b/src/gallium/state_trackers/va/picture_hevc.c
index dc66b0f..28743ee 100644
--- a/src/gallium/state_trackers/va/picture_hevc.c
+++ b/src/gallium/state_trackers/va/picture_hevc.c
@@ -159,11 +159,6 @@ void
vlVaHandlePictureParameterBufferHEVC(vlVaDriver *drv, vlVaContext
*context,
for (i = 0 ; i < 15 ; i++) {
   context->desc.h265.PicOrderCntVal[i] =
hevc->ReferenceFrames[i].pic_order_cnt;

-  unsigned int index = hevc->ReferenceFrames[i].picture_id &
0x7F;
-
-  if (index == 0x7F)
- continue;
-
   vlVaGetReferenceFrame(drv,
hevc->ReferenceFrames[i].picture_id, >desc.h265.ref[i]);

   if ((hevc->ReferenceFrames[i].flags &
VA_PICTURE_HEVC_RPS_ST_CURR_BEFORE) && (iBefore < 8)) {
--
2.5.0

___
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 v2 08/13] i965: Handle TCS outputs and TES inputs.

[Kenneth Graunke]

On Saturday, December 12, 2015 10:21:19 PM Jordan Justen wrote:
> On 2015-12-11 13:23:57, Kenneth Graunke wrote:
> > TCS outputs and TES inputs both refer to a common "patch URB entry"
> > shared across all invocations.  First, there are some number of
> > per-patch entries.  Then, there are per-vertex entries accessed via
> > an offset for the variable and a stride times the vertex index.
> > 
> > Because these calculations need to be done in both the vec4 and scalar
> > backends, it's simpler to just compute the offset calculations in NIR.
> > It doesn't necessarily make much sense to use per-vertex intrinsics
> > afterwards, but that at least means we don't lose the per-patch vs.
> > per-vertex information.
> > 
> > Signed-off-by: Kenneth Graunke 
> > ---
> >  src/mesa/drivers/dri/i965/brw_nir.c | 122 
> > +++-
> >  1 file changed, 120 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_nir.c 
> > b/src/mesa/drivers/dri/i965/brw_nir.c
> > index 9cf4944..e46e177 100644
> > --- a/src/mesa/drivers/dri/i965/brw_nir.c
> > +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> > @@ -133,6 +133,69 @@ remap_inputs_with_vue_map(nir_block *block, void 
> > *closure)
> > return true;
> >  }
> >  
> > +struct remap_patch_urb_offsets_state {
> > +   nir_builder b;
> > +   struct brw_vue_map vue_map;
> > +};
> > +
> > +static bool
> > +remap_patch_urb_offsets(nir_block *block, void *closure)
> > +{
> > +   struct remap_patch_urb_offsets_state *state = closure;
> > +
> > +   nir_foreach_instr_safe(block, instr) {
> > +  if (instr->type != nir_instr_type_intrinsic)
> > + continue;
> > +
> > +  nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr);
> > +
> > +  bool is_input =
> > + intrin->intrinsic == nir_intrinsic_load_input ||
> > + intrin->intrinsic == nir_intrinsic_load_per_vertex_input;
> > +
> > +  bool is_output =
> > + intrin->intrinsic == nir_intrinsic_load_output ||
> > + intrin->intrinsic == nir_intrinsic_load_per_vertex_output ||
> > + intrin->intrinsic == nir_intrinsic_store_output ||
> > + intrin->intrinsic == nir_intrinsic_store_per_vertex_output;
> 
> Can you call the functions you added previously? (i965: Separate base
> offset/constant offset combining from remapping.)
> 
> 7 & 8 Reviewed-by: Jordan Justen 

Oh, good call - I wrote this patch first, even though it came later in
the series.  That makes a ton of sense.  I've changed it to:

+  gl_shader_stage stage = state->b.shader->stage;
+
+  if ((stage == MESA_SHADER_TESS_CTRL && is_output(intrin)) ||
+  (stage == MESA_SHADER_TESS_EVAL && is_input(intrin))) {


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/5] nv50, nvc0: fix potential resource leak in nvXX_create_texture_view()

[Samuel Pitoiset]

Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nv50/nv50_tex.c | 3 ++-
 src/gallium/drivers/nouveau/nvc0/nvc0_tex.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_tex.c 
b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
index 6083ea9..9fb9dcf 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_tex.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
@@ -193,7 +193,8 @@ nv50_create_texture_view(struct pipe_context *pipe,
   break;
default:
   NOUVEAU_ERR("invalid texture target: %d\n", mt->base.base.target);
-  return false;
+  FREE(view);
+  return NULL;
}
 
tic[3] = (flags & NV50_TEXVIEW_FILTER_MSAA8) ? 0x2000 : 0x0030;
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
index 2dd100f..2503ee1 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
@@ -195,7 +195,8 @@ nvc0_create_texture_view(struct pipe_context *pipe,
default:
   NOUVEAU_ERR("unexpected/invalid texture target: %d\n",
   mt->base.base.target);
-  return false;
+  FREE(view);
+  return NULL;
}
 
tic[3] = (flags & NV50_TEXVIEW_FILTER_MSAA8) ? 0x2000 : 0x0030;
-- 
2.6.4

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

[Mesa-dev] [PATCH 0/5] nouveau: fix some Coverity issues

[Samuel Pitoiset]

Hi,

This series fixes some issues spotted by Coverity.

Feel free to review,
Thanks.

Samuel.

Samuel Pitoiset (5):
  nv50,nvc0: fix using uninitialized value 'templ.texture' in 
nvXX_blit_set_src()
  nv50,nvc0: fix potential resource leak in nvXX_create_texture_view()
  nvc0: make sure gmt programs are correctly validated
  nv50: check return value of nouveau_object_new()
  nouveau: remove logically dead code in nouveau_vpe_mb_mv_header()

 src/gallium/drivers/nouveau/nouveau_video.c  | 2 --
 src/gallium/drivers/nouveau/nv50/nv50_surface.c  | 1 +
 src/gallium/drivers/nouveau/nv50/nv50_tex.c  | 3 ++-
 src/gallium/drivers/nouveau/nv50/nv84_video.c| 8 
 src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c | 6 --
 src/gallium/drivers/nouveau/nvc0/nvc0_surface.c  | 1 +
 src/gallium/drivers/nouveau/nvc0/nvc0_tex.c  | 3 ++-
 7 files changed, 14 insertions(+), 10 deletions(-)

-- 
2.6.4

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

[Mesa-dev] [PATCH 5/5] nouveau: remove logically dead code in nouveau_vpe_mb_mv_header()

[Samuel Pitoiset]

frame cannot be NULL in that branch. Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nouveau_video.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nouveau_video.c 
b/src/gallium/drivers/nouveau/nouveau_video.c
index 8bb12b2..fe19bce 100644
--- a/src/gallium/drivers/nouveau/nouveau_video.c
+++ b/src/gallium/drivers/nouveau/nouveau_video.c
@@ -317,8 +317,6 @@ nouveau_vpe_mb_mv_header(struct nouveau_decoder *dec,
   case PIPE_MPEG12_MO_TYPE_16x8: goto mv2;
   case PIPE_MPEG12_MO_TYPE_DUAL_PRIME: {
   base = NV17_MPEG_CMD_CHROMA_MV_HEADER_MV_SPLIT_HALF_MB;
- if (frame)
-base |= NV17_MPEG_CMD_CHROMA_MV_HEADER_TYPE_FRAME;
  if (forward)
 nouveau_vpe_mb_mv(dec, base, luma, frame, true,
   dec->picture_structure != 
PIPE_MPEG12_PICTURE_STRUCTURE_FIELD_TOP,
-- 
2.6.4

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

[Mesa-dev] [PATCH 3/5] nvc0: make sure gmt programs are correctly validated

[Samuel Pitoiset]

Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
index 7e2e999..5e69e29 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
@@ -236,8 +236,10 @@ nvc0_gmtyprog_validate(struct nvc0_context *nvc0)
struct nouveau_pushbuf *push = nvc0->base.pushbuf;
struct nvc0_program *gp = nvc0->gmtyprog;
 
-   if (gp)
-  nvc0_program_validate(nvc0, gp);
+   if (gp) {
+  if (!nvc0_program_validate(nvc0, gp))
+ return;
+   }
 
/* we allow GPs with no code for specifying stream output state only */
if (gp && gp->code_size) {
-- 
2.6.4

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

[Mesa-dev] [PATCH 1/5] nv50, nvc0: fix using uninitialized value 'templ.texture' in nvXX_blit_set_src()

[Samuel Pitoiset]

Field 'templ.texture' is used uninitialized when calling
nvXX_create_texture_view(). Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nv50/nv50_surface.c | 1 +
 src/gallium/drivers/nouveau/nvc0/nvc0_surface.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c 
b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
index 86be1b4..b03e893 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
@@ -1031,6 +1031,7 @@ nv50_blit_set_src(struct nv50_blitctx *blit,
 
target = nv50_blit_reinterpret_pipe_texture_target(res->target);
 
+   templ.texture = NULL;
templ.format = format;
templ.u.tex.first_level = templ.u.tex.last_level = level;
templ.u.tex.first_layer = templ.u.tex.last_layer = layer;
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
index f8e1efb..41fb82a 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
@@ -841,6 +841,7 @@ nvc0_blit_set_src(struct nvc0_blitctx *ctx,
 
target = nv50_blit_reinterpret_pipe_texture_target(res->target);
 
+   templ.texture = NULL;
templ.format = format;
templ.u.tex.first_layer = templ.u.tex.last_layer = layer;
templ.u.tex.first_level = templ.u.tex.last_level = level;
-- 
2.6.4

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

[Mesa-dev] [PATCH 4/5] nv50: check return value of nouveau_object_new()

[Samuel Pitoiset]

Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nv50/nv84_video.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv84_video.c 
b/src/gallium/drivers/nouveau/nv50/nv84_video.c
index 7a4670f..5e2489a 100644
--- a/src/gallium/drivers/nouveau/nv50/nv84_video.c
+++ b/src/gallium/drivers/nouveau/nv50/nv84_video.c
@@ -756,8 +756,8 @@ firmware_present(struct pipe_screen *pscreen, enum 
pipe_video_format codec)
int present, ret;
 
if (!FIRMWARE_PRESENT(checked, VP_KERN)) {
-  nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0, );
-  if (obj)
+  ret = nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0, );
+  if (!ret && obj)
  screen->firmware_info.profiles_present |= FIRMWARE_VP_KERN;
   nouveau_object_del();
   screen->firmware_info.profiles_checked |= FIRMWARE_VP_KERN;
@@ -765,8 +765,8 @@ firmware_present(struct pipe_screen *pscreen, enum 
pipe_video_format codec)
 
if (codec == PIPE_VIDEO_FORMAT_MPEG4_AVC) {
   if (!FIRMWARE_PRESENT(checked, BSP_KERN)) {
- nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0, );
- if (obj)
+ ret = nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0, );
+ if (!ret && obj)
 screen->firmware_info.profiles_present |= FIRMWARE_BSP_KERN;
  nouveau_object_del();
  screen->firmware_info.profiles_checked |= FIRMWARE_BSP_KERN;
-- 
2.6.4

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

Re: [Mesa-dev] [PATCH 8/8] st/va: add BOB/WEAVE deinterlacing

[Julien Isorce]

This patch is:
Reviewed-by: Julien Isorce 
Tested-by: Julien Isorce 

(tested with gstvaapipostproc from gstreamer-vaapi)

On 11 December 2015 at 12:33, Christian König 
wrote:

> From: Christian König 
>
> Tested with MPV.
>
> Signed-off-by: Christian König 
> ---
>  src/gallium/state_trackers/va/postproc.c | 72
> 
>  src/gallium/state_trackers/va/surface.c  | 16 ++-
>  2 files changed, 78 insertions(+), 10 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/postproc.c
> b/src/gallium/state_trackers/va/postproc.c
> index c6cfd3a..bcac66a 100644
> --- a/src/gallium/state_trackers/va/postproc.c
> +++ b/src/gallium/state_trackers/va/postproc.c
> @@ -51,7 +51,8 @@ vlVaPostProcCompositor(vlVaDriver *drv, vlVaContext
> *context,
> const VARectangle *src_region,
> const VARectangle *dst_region,
> struct pipe_video_buffer *src,
> -   struct pipe_video_buffer *dst)
> +   struct pipe_video_buffer *dst,
> +   enum vl_compositor_deinterlace deinterlace)
>  {
> struct pipe_surface **surfaces;
> struct u_rect src_rect;
> @@ -100,7 +101,8 @@ static VAStatus vlVaPostProcBlit(vlVaDriver *drv,
> vlVaContext *context,
>   const VARectangle *src_region,
>   const VARectangle *dst_region,
>   struct pipe_video_buffer *src,
> - struct pipe_video_buffer *dst)
> + struct pipe_video_buffer *dst,
> + enum vl_compositor_deinterlace
> deinterlace)
>  {
> struct pipe_surface **src_surfaces;
> struct pipe_surface **dst_surfaces;
> @@ -118,18 +120,33 @@ static VAStatus vlVaPostProcBlit(vlVaDriver *drv,
> vlVaContext *context,
>return VA_STATUS_ERROR_INVALID_SURFACE;
>
> for (i = 0; i < VL_MAX_SURFACES; ++i) {
> +  struct pipe_surface *from = src_surfaces[i];
>struct pipe_blit_info blit;
>
> -  if (!src_surfaces[i] || !dst_surfaces[i])
> +  if (src->interlaced) {
> + /* Not 100% accurate, but close enough */
> + switch (deinterlace) {
> + case VL_COMPOSITOR_BOB_TOP:
> +from = src_surfaces[i & ~1];
> +break;
> + case VL_COMPOSITOR_BOB_BOTTOM:
> +from = src_surfaces[(i & ~1) + 1];
> +break;
> + default:
> +break;
> + }
> +  }
> +
> +  if (!from || !dst_surfaces[i])
>   continue;
>
>memset(, 0, sizeof(blit));
> -  blit.src.resource = src_surfaces[i]->texture;
> -  blit.src.format = src_surfaces[i]->format;
> +  blit.src.resource = from->texture;
> +  blit.src.format = from->format;
>blit.src.level = 0;
>blit.src.box.x = src_region->x;
>blit.src.box.y = src_region->y;
> -  blit.src.box.z = src_surfaces[i]->u.tex.first_layer;
> +  blit.src.box.z = from->u.tex.first_layer;
>blit.src.box.width = src_region->width;
>blit.src.box.height = src_region->height;
>blit.src.box.depth = 1;
> @@ -158,10 +175,12 @@ static VAStatus vlVaPostProcBlit(vlVaDriver *drv,
> vlVaContext *context,
>  VAStatus
>  vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContext
> *context, vlVaBuffer *buf)
>  {
> +   enum vl_compositor_deinterlace deinterlace = VL_COMPOSITOR_WEAVE;
> VARectangle def_src_region, def_dst_region;
> const VARectangle *src_region, *dst_region;
> VAProcPipelineParameterBuffer *param;
> vlVaSurface *src_surface;
> +   unsigned i;
>
> if (!drv || !context)
>return VA_STATUS_ERROR_INVALID_CONTEXT;
> @@ -178,13 +197,50 @@
> vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContext
> *contex
> if (!src_surface || !src_surface->buffer)
>return VA_STATUS_ERROR_INVALID_SURFACE;
>
> +   for (i = 0; i < param->num_filters; i++) {
> +  vlVaBuffer *buf = handle_table_get(drv->htab, param->filters[i]);
> +  VAProcFilterParameterBufferBase *filter;
> +
> +  if (!buf || buf->type != VAProcFilterParameterBufferType)
> + return VA_STATUS_ERROR_INVALID_BUFFER;
> +
> +  filter = buf->data;
> +  switch (filter->type) {
> +  case VAProcFilterDeinterlacing: {
> + VAProcFilterParameterBufferDeinterlacing *deint = buf->data;
> + switch (deint->algorithm) {
> + case VAProcDeinterlacingBob:
> +if (deint->flags & VA_DEINTERLACING_BOTTOM_FIELD)
> +   deinterlace = VL_COMPOSITOR_BOB_BOTTOM;
> +else
> +   deinterlace = VL_COMPOSITOR_BOB_TOP;
> +break;
> +
> + case VAProcDeinterlacingWeave:
> +deinterlace = VL_COMPOSITOR_WEAVE;
> +

Re: [Mesa-dev] [PATCH v2 11/13] i965: Add tessellation evaluation shaders

[Kenneth Graunke]

On Monday, December 14, 2015 01:24:37 AM Jordan Justen wrote:
> Whew... I probably would have split this one into 5 or so. Then krh
> would have grumbled at me. ;)

Sorry!  These are pretty huge.  I wasn't sure how best to split it up,
or how much time to spend on that.

> On 2015-12-11 13:24:00, Kenneth Graunke wrote:
> > The TES is essentially a post-tessellator VS, which has access to the
> > entire TCS output patch, and a special gl_TessCoord input.  Otherwise,
> > they're very straightforward.
> > 
> > This patch implements SIMD8 tessellation evaluation shaders for Gen8+.
> > The tessellator can generate a lot of geometry, so operating in SIMD8
> > mode (8 vertices per thread) is more efficient than SIMD4x2 mode (only
> > 2 vertices per thread).  I have another patch which implements SIMD4x2
> > mode for older hardware (or via an environment variable override).
> > 
> > We currently handle all inputs via the pull model.
> > 
> > Signed-off-by: Kenneth Graunke 
> > ---
> >  src/mesa/drivers/dri/i965/Makefile.sources   |   1 +
> >  src/mesa/drivers/dri/i965/brw_compiler.h |  24 +++
> >  src/mesa/drivers/dri/i965/brw_context.h  |   6 +
> >  src/mesa/drivers/dri/i965/brw_fs.cpp |  48 +
> >  src/mesa/drivers/dri/i965/brw_fs.h   |  10 +-
> >  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 121 +++
> >  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |  12 +-
> >  src/mesa/drivers/dri/i965/brw_link.cpp   |   4 +
> >  src/mesa/drivers/dri/i965/brw_program.h  |   2 +
> >  src/mesa/drivers/dri/i965/brw_shader.cpp |  94 +
> >  src/mesa/drivers/dri/i965/brw_shader.h   |   3 +
> >  src/mesa/drivers/dri/i965/brw_state_upload.c |   3 +
> >  src/mesa/drivers/dri/i965/brw_tes.c  | 300 
> > +++
> >  13 files changed, 625 insertions(+), 3 deletions(-)
> >  create mode 100644 src/mesa/drivers/dri/i965/brw_tes.c
> > 
> > diff --git a/src/mesa/drivers/dri/i965/Makefile.sources 
> > b/src/mesa/drivers/dri/i965/Makefile.sources
> > index d147a73..7354aaf 100644
> > --- a/src/mesa/drivers/dri/i965/Makefile.sources
> > +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> > @@ -151,6 +151,7 @@ i965_FILES = \
> > brw_state_upload.c \
> > brw_structs.h \
> > brw_tcs_surface_state.c \
> > +   brw_tes.c \
> > brw_tes_surface_state.c \
> > brw_tex.c \
> > brw_tex_layout.c \
> > diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h 
> > b/src/mesa/drivers/dri/i965/brw_compiler.h
> > index c9e0317..64d831d 100644
> > --- a/src/mesa/drivers/dri/i965/brw_compiler.h
> > +++ b/src/mesa/drivers/dri/i965/brw_compiler.h
> > @@ -191,6 +191,14 @@ struct brw_vs_prog_key {
> > struct brw_sampler_prog_key_data tex;
> >  };
> >  
> > +/** The program key for Tessellation Evaluation Shaders. */
> > +struct brw_tes_prog_key
> > +{
> > +   unsigned program_string_id;
> > +
> > +   struct brw_sampler_prog_key_data tex;
> > +};
> > +
> >  /** The program key for Geometry Shaders. */
> >  struct brw_gs_prog_key
> >  {
> > @@ -669,6 +677,22 @@ brw_compile_vs(const struct brw_compiler *compiler, 
> > void *log_data,
> > char **error_str);
> >  
> >  /**
> > + * Compile a tessellation evaluation shader.
> > + *
> > + * Returns the final assembly and the program's size.
> > + */
> > +const unsigned *
> > +brw_compile_tes(const struct brw_compiler *compiler, void *log_data,
> > +void *mem_ctx,
> > +const struct brw_tes_prog_key *key,
> > +struct brw_tes_prog_data *prog_data,
> > +const struct nir_shader *shader,
> > +struct gl_shader_program *shader_prog,
> > +int shader_time_index,
> > +unsigned *final_assembly_size,
> > +char **error_str);
> > +
> > +/**
> >   * Compile a vertex shader.
> >   *
> >   * Returns the final assembly and the program's size.
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> > b/src/mesa/drivers/dri/i965/brw_context.h
> > index 69bc04c..5e840d1 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.h
> > +++ b/src/mesa/drivers/dri/i965/brw_context.h
> > @@ -1704,6 +1704,12 @@ brw_vertex_program_const(const struct 
> > gl_vertex_program *p)
> > return (const struct brw_vertex_program *) p;
> >  }
> >  
> > +static inline struct brw_tess_eval_program *
> > +brw_tess_eval_program(struct gl_tess_eval_program *p)
> > +{
> > +   return (struct brw_tess_eval_program *) p;
> > +}
> > +
> >  static inline struct brw_geometry_program *
> >  brw_geometry_program(struct gl_geometry_program *p)
> >  {
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> > b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > index c833ef0..de584e4 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > @@ -1686,6 +1686,21 @@ fs_visitor::assign_vs_urb_setup()
> >  }
> >  
> >  void
> > 

Re: [Mesa-dev] [PATCH 6/8] st/va: remove fence handling

[Julien Isorce]

Hi Christian,

I have tested this patch but then the displayed video is garbage (mostly
white and sometimes just garbage). It also stall the nouveau driver which
requires to reboot but I guess this is another issue.
I tested with:
GST_GL_WINDOW=x11 GST_GL_PLATFORM=egl GST_GL_API=gles2 GST_DEBUG=2
LIBVA_DRIVER_NAME=gallium gst-launch-1.0 filesrc location=simpson.mp4 !
qtdemux ! vaapidecodebin ! glimagesink

(to test that you need my gstreamer-vaapi and gstgl branches on my github
but I would not waste time to try them since they should be merged upstream
at some point)

Also note that in this pipeline, HW decoding is done with nouveau driver
and rendering is done with intel. dmabuf in between.

Maybe the idea of the patch is good but something is still wrong.
I can test any update if it helps.

Cheers
Julien




On 11 December 2015 at 12:33, Christian König 
wrote:

> From: Christian König 
>
> It's nonsense to drain the pipeline like this.
>
> Signed-off-by: Christian König 
> ---
>  src/gallium/state_trackers/va/buffer.c |  5 -
>  src/gallium/state_trackers/va/image.c  |  1 -
>  src/gallium/state_trackers/va/postproc.c   |  6 --
>  src/gallium/state_trackers/va/surface.c| 10 +-
>  src/gallium/state_trackers/va/va_private.h |  2 --
>  5 files changed, 1 insertion(+), 23 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/buffer.c
> b/src/gallium/state_trackers/va/buffer.c
> index 769305e..2ec187c 100644
> --- a/src/gallium/state_trackers/va/buffer.c
> +++ b/src/gallium/state_trackers/va/buffer.c
> @@ -257,11 +257,6 @@ vlVaAcquireBufferHandle(VADriverContextP ctx,
> VABufferID buf_id,
>
> screen = VL_VA_PSCREEN(ctx);
>
> -   if (buf->derived_surface.fence) {
> -  screen->fence_finish(screen, buf->derived_surface.fence,
> PIPE_TIMEOUT_INFINITE);
> -  screen->fence_reference(screen, >derived_surface.fence, NULL);
> -   }
> -
> if (buf->export_refcount > 0) {
>if (buf->export_state.mem_type != mem_type)
>   return VA_STATUS_ERROR_INVALID_PARAMETER;
> diff --git a/src/gallium/state_trackers/va/image.c
> b/src/gallium/state_trackers/va/image.c
> index ae07da8..58c9ff7 100644
> --- a/src/gallium/state_trackers/va/image.c
> +++ b/src/gallium/state_trackers/va/image.c
> @@ -266,7 +266,6 @@ vlVaDeriveImage(VADriverContextP ctx, VASurfaceID
> surface, VAImage *image)
> img_buf->type = VAImageBufferType;
> img_buf->size = image->data_size;
> img_buf->num_elements = 1;
> -   img_buf->derived_surface.fence = surf->fence;
>
> pipe_resource_reference(_buf->derived_surface.resource,
> surfaces[0]->texture);
>
> diff --git a/src/gallium/state_trackers/va/postproc.c
> b/src/gallium/state_trackers/va/postproc.c
> index 105f251..1ee3587 100644
> --- a/src/gallium/state_trackers/va/postproc.c
> +++ b/src/gallium/state_trackers/va/postproc.c
> @@ -54,7 +54,6 @@ vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver
> *drv, vlVaContext *contex
> vlVaSurface *src_surface;
> VAProcPipelineParameterBuffer *pipeline_param;
> struct pipe_surface **surfaces;
> -   struct pipe_screen *screen;
> struct pipe_surface *psurf;
>
> if (!drv || !context)
> @@ -77,8 +76,6 @@ vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver
> *drv, vlVaContext *contex
> if (!surfaces || !surfaces[0])
>return VA_STATUS_ERROR_INVALID_SURFACE;
>
> -   screen = drv->pipe->screen;
> -
> psurf = surfaces[0];
>
> src_region = vlVaRegionDefault(pipeline_param->surface_region,
> src_surface->buffer, _src_region);
> @@ -99,9 +96,6 @@ vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver
> *drv, vlVaContext *contex
> vl_compositor_set_layer_dst_area(>cstate, 0, _rect);
> vl_compositor_render(>cstate, >compositor, psurf, NULL,
> false);
>
> -   screen->fence_reference(screen, _surface->fence, NULL);
> -   drv->pipe->flush(drv->pipe, _surface->fence, 0);
> -
> return VA_STATUS_SUCCESS;
>  }
>
> diff --git a/src/gallium/state_trackers/va/surface.c
> b/src/gallium/state_trackers/va/surface.c
> index 4a18a6f..5ddaf04 100644
> --- a/src/gallium/state_trackers/va/surface.c
> +++ b/src/gallium/state_trackers/va/surface.c
> @@ -72,8 +72,6 @@ vlVaDestroySurfaces(VADriverContextP ctx, VASurfaceID
> *surface_list, int num_sur
>vlVaSurface *surf = handle_table_get(drv->htab, surface_list[i]);
>if (surf->buffer)
>   surf->buffer->destroy(surf->buffer);
> -  if(surf->fence)
> - drv->pipe->screen->fence_reference(drv->pipe->screen,
> >fence, NULL);
>util_dynarray_fini(>subpics);
>FREE(surf);
>handle_table_remove(drv->htab, surface_list[i]);
> @@ -245,11 +243,6 @@ vlVaPutSurface(VADriverContextP ctx, VASurfaceID
> surface_id, void* draw, short s
> screen = drv->pipe->screen;
> vscreen = drv->vscreen;
>
> -   if(surf->fence) {
> -  screen->fence_finish(screen, surf->fence, PIPE_TIMEOUT_INFINITE);
> -

Re: [Mesa-dev] [PATCH 6/8] st/va: remove fence handling

[Christian König]

Also note that in this pipeline, HW decoding is done with nouveau 
driver and rendering is done with intel. dmabuf in between.
Yeah, I already thought that somebody is using it like this. I'm not 
sure if this is actually supposed to work because we don't have proper 
synchronization between kernel drivers with DMA-buf jet.



Maybe the idea of the patch is good but something is still wrong.
While it is not the proper solution I would say let's keep the pipeline 
draining during exporting the handle for now if that's really necessary 
for your use case. Please test the attached patch.


Coding the patch I've just noticed that there wasn't a pipe->flush() 
before exporting the handle. Does it work as well if you just flush the 
pipeline without waiting for the commands to be finished?


Regards,
Christian.

On 14.12.2015 10:14, Julien Isorce wrote:

Hi Christian,

I have tested this patch but then the displayed video is garbage 
(mostly white and sometimes just garbage). It also stall the nouveau 
driver which requires to reboot but I guess this is another issue.

I tested with:
GST_GL_WINDOW=x11 GST_GL_PLATFORM=egl GST_GL_API=gles2 GST_DEBUG=2 
LIBVA_DRIVER_NAME=gallium gst-launch-1.0 filesrc location=simpson.mp4 
! qtdemux ! vaapidecodebin ! glimagesink


(to test that you need my gstreamer-vaapi and gstgl branches on my 
github but I would not waste time to try them since they should be 
merged upstream at some point)


Also note that in this pipeline, HW decoding is done with nouveau 
driver and rendering is done with intel. dmabuf in between.


Maybe the idea of the patch is good but something is still wrong.
I can test any update if it helps.

Cheers
Julien




On 11 December 2015 at 12:33, Christian König > wrote:


From: Christian König >

It's nonsense to drain the pipeline like this.

Signed-off-by: Christian König >
---
 src/gallium/state_trackers/va/buffer.c |  5 -
 src/gallium/state_trackers/va/image.c  |  1 -
 src/gallium/state_trackers/va/postproc.c   |  6 --
 src/gallium/state_trackers/va/surface.c| 10 +-
 src/gallium/state_trackers/va/va_private.h |  2 --
 5 files changed, 1 insertion(+), 23 deletions(-)

diff --git a/src/gallium/state_trackers/va/buffer.c
b/src/gallium/state_trackers/va/buffer.c
index 769305e..2ec187c 100644
--- a/src/gallium/state_trackers/va/buffer.c
+++ b/src/gallium/state_trackers/va/buffer.c
@@ -257,11 +257,6 @@ vlVaAcquireBufferHandle(VADriverContextP ctx,
VABufferID buf_id,

screen = VL_VA_PSCREEN(ctx);

-   if (buf->derived_surface.fence) {
-  screen->fence_finish(screen, buf->derived_surface.fence,
PIPE_TIMEOUT_INFINITE);
-  screen->fence_reference(screen,
>derived_surface.fence, NULL);
-   }
-
if (buf->export_refcount > 0) {
   if (buf->export_state.mem_type != mem_type)
  return VA_STATUS_ERROR_INVALID_PARAMETER;
diff --git a/src/gallium/state_trackers/va/image.c
b/src/gallium/state_trackers/va/image.c
index ae07da8..58c9ff7 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -266,7 +266,6 @@ vlVaDeriveImage(VADriverContextP ctx,
VASurfaceID surface, VAImage *image)
img_buf->type = VAImageBufferType;
img_buf->size = image->data_size;
img_buf->num_elements = 1;
-   img_buf->derived_surface.fence = surf->fence;

pipe_resource_reference(_buf->derived_surface.resource,
surfaces[0]->texture);

diff --git a/src/gallium/state_trackers/va/postproc.c
b/src/gallium/state_trackers/va/postproc.c
index 105f251..1ee3587 100644
--- a/src/gallium/state_trackers/va/postproc.c
+++ b/src/gallium/state_trackers/va/postproc.c
@@ -54,7 +54,6 @@
vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv,
vlVaContext *contex
vlVaSurface *src_surface;
VAProcPipelineParameterBuffer *pipeline_param;
struct pipe_surface **surfaces;
-   struct pipe_screen *screen;
struct pipe_surface *psurf;

if (!drv || !context)
@@ -77,8 +76,6 @@
vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv,
vlVaContext *contex
if (!surfaces || !surfaces[0])
   return VA_STATUS_ERROR_INVALID_SURFACE;

-   screen = drv->pipe->screen;
-
psurf = surfaces[0];

src_region = vlVaRegionDefault(pipeline_param->surface_region,
src_surface->buffer, _src_region);
@@ -99,9 +96,6 @@
vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv,
vlVaContext *contex
vl_compositor_set_layer_dst_area(>cstate, 0, _rect);
vl_compositor_render(>cstate, >compositor, psurf,
NULL, false);

-   

Re: [Mesa-dev] [PATCH v2 05/13] i965: Allocate URB space for HS and DS stages when required.

[Kenneth Graunke]

On Friday, December 11, 2015 01:23:54 PM Kenneth Graunke wrote:
> From: Chris Forbes 
> 
> v2: Rewrite the push constant allocation code to be clearer.
> Only apply the minimum VS entries workaround on Gen 8.
> 
> Signed-off-by: Chris Forbes 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_context.h  |  21 +++-
>  src/mesa/drivers/dri/i965/gen7_blorp.cpp |   8 ++
>  src/mesa/drivers/dri/i965/gen7_urb.c | 174 
> ---
>  3 files changed, 166 insertions(+), 37 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 1cc4c7b..69bc04c 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1008,6 +1008,8 @@ struct brw_context
> struct {
>GLuint vsize;  /* vertex size plus header in urb registers */
>GLuint gsize;  /* GS output size in urb registers */
> +  GLuint hsize; /* Tessellation control output size in urb 
> registers */
> +  GLuint dsize; /* Tessellation evaluation output size in 
> urb registers */
>GLuint csize;  /* constant buffer size in urb registers */
>GLuint sfsize; /* setup data size in urb registers */
>  
> @@ -1020,12 +1022,16 @@ struct brw_context
>GLuint max_gs_entries; /* Maximum number of GS entries */
>  
>GLuint nr_vs_entries;
> +  GLuint nr_hs_entries;
> +  GLuint nr_ds_entries;
>GLuint nr_gs_entries;
>GLuint nr_clip_entries;
>GLuint nr_sf_entries;
>GLuint nr_cs_entries;
>  
>GLuint vs_start;
> +  GLuint hs_start;
> +  GLuint ds_start;
>GLuint gs_start;
>GLuint clip_start;
>GLuint sf_start;
> @@ -1042,6 +1048,11 @@ struct brw_context
> * URB space for the GS.
> */
>bool gs_present;
> +
> +  /* True if the most recently sent _3DSTATE_URB message allocated
> +   * URB space for the HS and DS.
> +   */
> +  bool tess_present;
> } urb;
>  
>  
> @@ -1648,12 +1659,18 @@ void gen8_emit_3dstate_sample_pattern(struct 
> brw_context *brw);
>  /* gen7_urb.c */
>  void
>  gen7_emit_push_constant_state(struct brw_context *brw, unsigned vs_size,
> +  unsigned hs_size, unsigned ds_size,
>unsigned gs_size, unsigned fs_size);
>  
>  void
>  gen7_emit_urb_state(struct brw_context *brw,
> -unsigned nr_vs_entries, unsigned vs_size,
> -unsigned vs_start, unsigned nr_gs_entries,
> +unsigned nr_vs_entries,
> +unsigned vs_size, unsigned vs_start,
> +unsigned nr_hs_entries,
> +unsigned hs_size, unsigned hs_start,
> +unsigned nr_ds_entries,
> +unsigned ds_size, unsigned ds_start,
> +unsigned nr_gs_entries,
>  unsigned gs_size, unsigned gs_start);
>  
>  
> diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp 
> b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> index e87b9d1..89b73ca 100644
> --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> @@ -50,6 +50,8 @@ gen7_blorp_emit_urb_config(struct brw_context *brw)
> unsigned urb_size = (brw->is_haswell && brw->gt == 3) ? 32 : 16;
> gen7_emit_push_constant_state(brw,
>   urb_size / 2 /* vs_size */,
> + 0 /* hs_size */,
> + 0 /* ds_size */,
>   0 /* gs_size */,
>   urb_size / 2 /* fs_size */);
>  
> @@ -60,6 +62,12 @@ gen7_blorp_emit_urb_config(struct brw_context *brw)
> 32 /* num_vs_entries */,
> 2 /* vs_size */,
> 2 /* vs_start */,
> +   0 /* num_hs_entries */,
> +   1 /* hs_size */,
> +   2 /* hs_start */,
> +   0 /* num_ds_entries */,
> +   1 /* ds_size */,
> +   2 /* ds_start */,
> 0 /* num_gs_entries */,
> 1 /* gs_size */,
> 2 /* gs_start */);
> diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c 
> b/src/mesa/drivers/dri/i965/gen7_urb.c
> index 99a9d3c..a598c89 100644
> --- a/src/mesa/drivers/dri/i965/gen7_urb.c
> +++ b/src/mesa/drivers/dri/i965/gen7_urb.c
> @@ -34,7 +34,7 @@
>   *   __-__   _-_
>   *  / \ /   \
>   * +-+
> - * | VS/FS/GS Push |  VS/GS URB  |
> + * |  

Re: [Mesa-dev] [Intel-gfx] [RFC libdrm] intel: Add support for softpin

[Daniel Vetter]

On Mon, Dec 14, 2015 at 08:41:05AM +, Song, Ruiling wrote:
> 
> 
> > -Original Message-
> > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Monday, December 14, 2015 4:28 PM
> > To: Song, Ruiling 
> > Cc: k...@bitplanet.net; Winiarski, Michal ;
> > mesa-dev@lists.freedesktop.org; intel-...@lists.freedesktop.org; Ben
> > Widawsky ; dri-de...@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [RFC libdrm] intel: Add support for softpin
> > 
> > On Mon, Dec 14, 2015 at 07:24:29AM +, Song, Ruiling wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: hoegsb...@gmail.com [mailto:hoegsb...@gmail.com] On Behalf
> > Of
> > > > Kristian H?gsberg
> > > > Sent: Monday, December 14, 2015 1:34 PM
> > > > To: Song, Ruiling 
> > > > Cc: Winiarski, Michal ; intel-
> > > > g...@lists.freedesktop.org; mesa-dev@lists.freedesktop.org; Ben
> > Widawsky
> > > > ; dri-de...@lists.freedesktop.org
> > > > Subject: Re: [Intel-gfx] [RFC libdrm] intel: Add support for softpin
> > > >
> > > > On Sun, Dec 13, 2015 at 7:17 PM, Song, Ruiling 
> > > > wrote:
> > > > >> -Original Message-
> > > > >> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On
> > > > Behalf
> > > > >> Of Micha? Winiarski
> > > > >> Sent: Wednesday, September 9, 2015 10:07 PM
> > > > >> To: intel-...@lists.freedesktop.org
> > > > >> Cc: Ben Widawsky ; dri-
> > > > de...@lists.freedesktop.org;
> > > > >> mesa-dev@lists.freedesktop.org
> > > > >> Subject: [Intel-gfx] [RFC libdrm] intel: Add support for softpin
> > > > >>
> > > > >> Softpin allows userspace to take greater control of GPU virtual 
> > > > >> address
> > > > >> space and eliminates the need of relocations. It can also be used to
> > > > >> mirror addresses between GPU and CPU (shared virtual memory).
> > > > >> Calls to drm_intel_bo_emit_reloc are still required to build the 
> > > > >> list of
> > > > >> drm_i915_gem_exec_objects at exec time, but no entries in relocs are
> > > > >> created. Self-relocs don't make any sense for softpinned objects and
> > can
> > > > >> indicate a programming errors, thus are forbidden. Softpinned objects
> > > > >> are marked by asterisk in debug dumps.
> > > > >>
> > > > >> Cc: Thomas Daniel 
> > > > >> Cc: Kristian Høgsberg 
> > > > >> Cc: Zou Nanhai 
> > > > >> Cc: Michel Thierry 
> > > > >> Cc: Ben Widawsky 
> > > > >> Cc: Chris Wilson 
> > > > >> Signed-off-by: Michał Winiarski 
> > > > >> ---
> > > > >>  include/drm/i915_drm.h|   4 +-
> > > > >>  intel/intel_bufmgr.c  |   9 +++
> > > > >>  intel/intel_bufmgr.h  |   1 +
> > > > >>  intel/intel_bufmgr_gem.c  | 176
> > > > >> --
> > > > >>  intel/intel_bufmgr_priv.h |   7 ++
> > > > >>  5 files changed, 173 insertions(+), 24 deletions(-)
> > > > >
> > > > > Will anybody help to push the patch to libdrm? Beignet highly depend
> > on
> > > > this to implement ocl2.0 svm.
> > > >
> > > > Is the kernel patch upstream?
> > >
> > > Yes, the kernel patch already merged, see:
> > > http://cgit.freedesktop.org/drm-
> > intel/commit/?id=506a8e87d8d2746b9e9d2433503fe237c54e4750
> > >
> > > I find below line of code in libdrm does not match the kernel version. The
> > kernel patch defined as:
> > > "#define EXEC_OBJECT_PINNED (1<<4)", but this patch defined it as (1<<3).
> > 
> > Please always regenerate the entire headers from the kernel sources using
> > 
> > $ make headers_install
> > 
> > Then copy the headers from the kernel's usr/include/drm to libdrm. Never
> > patch i915_drm.h manually.
> 
> Thanks for the info. But the problem is libdrm still tracks such kind of 
> header files.
> Should this kind of header file be removed from libdrm? Or any document in 
> libdrm to make this explicit?

All other kernel headers are extracted from the kernel directly, but
generally those packages only get update when a new kernel comes out.
Usually it's called linux-headers or similar. And only updating headers
when the release is out is _way_ too slow for drm/gfx. That's why we have
drm headers in libdrm.

But yeah we should document this, maybe even script it. Perhaps even just
upgrade headers automatically as soon as the upstream drm-next branch
changes.
-Daniel

> 
> Thanks!
> Ruiling
>  
> > Thanks, Daniel
> > 
> > >
> > > Hello Michal,
> > >
> > > Could you help to rebase the patch against:
> > > [Intel-gfx] [PATCH libdrm v4 0/2] 48-bit virtual address support in   
> > > i915
> > > I think we need both 48bit & softpin in libdrm.
> > >
> > > diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> > > index ded43b1..2b99fc6 

[Mesa-dev] [PATCH] nir: Fix number of indices on shared variable store intrinsics.

[Kenneth Graunke]

Shared variables and input reworks landed around the same time.
Presumably, this was some sort of mistake in rebase conflict resolution.

Found by inspection.

Signed-off-by: Kenneth Graunke 
---
 src/glsl/nir/nir_intrinsics.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

No effect in Jenkins, oddly.

diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
index 9811fb3..ec9e845 100644
--- a/src/glsl/nir/nir_intrinsics.h
+++ b/src/glsl/nir/nir_intrinsics.h
@@ -309,6 +309,6 @@ STORE(per_vertex_output, 3, 1, 0)
 /* src[] = { value, block_index, offset }. const_index[] = { write_mask } */
 STORE(ssbo, 3, 1, 0)
 /* src[] = { value, offset }. const_index[] = { base, write_mask } */
-STORE(shared, 2, 1, 0)
+STORE(shared, 2, 2, 0)
 
 LAST_INTRINSIC(store_shared)
-- 
2.6.3

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

Re: [Mesa-dev] [PATCH v2 11/13] i965: Add tessellation evaluation shaders

[Jordan Justen]

Whew... I probably would have split this one into 5 or so. Then krh
would have grumbled at me. ;)

On 2015-12-11 13:24:00, Kenneth Graunke wrote:
> The TES is essentially a post-tessellator VS, which has access to the
> entire TCS output patch, and a special gl_TessCoord input.  Otherwise,
> they're very straightforward.
> 
> This patch implements SIMD8 tessellation evaluation shaders for Gen8+.
> The tessellator can generate a lot of geometry, so operating in SIMD8
> mode (8 vertices per thread) is more efficient than SIMD4x2 mode (only
> 2 vertices per thread).  I have another patch which implements SIMD4x2
> mode for older hardware (or via an environment variable override).
> 
> We currently handle all inputs via the pull model.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/Makefile.sources   |   1 +
>  src/mesa/drivers/dri/i965/brw_compiler.h |  24 +++
>  src/mesa/drivers/dri/i965/brw_context.h  |   6 +
>  src/mesa/drivers/dri/i965/brw_fs.cpp |  48 +
>  src/mesa/drivers/dri/i965/brw_fs.h   |  10 +-
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 121 +++
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |  12 +-
>  src/mesa/drivers/dri/i965/brw_link.cpp   |   4 +
>  src/mesa/drivers/dri/i965/brw_program.h  |   2 +
>  src/mesa/drivers/dri/i965/brw_shader.cpp |  94 +
>  src/mesa/drivers/dri/i965/brw_shader.h   |   3 +
>  src/mesa/drivers/dri/i965/brw_state_upload.c |   3 +
>  src/mesa/drivers/dri/i965/brw_tes.c  | 300 
> +++
>  13 files changed, 625 insertions(+), 3 deletions(-)
>  create mode 100644 src/mesa/drivers/dri/i965/brw_tes.c
> 
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources 
> b/src/mesa/drivers/dri/i965/Makefile.sources
> index d147a73..7354aaf 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -151,6 +151,7 @@ i965_FILES = \
> brw_state_upload.c \
> brw_structs.h \
> brw_tcs_surface_state.c \
> +   brw_tes.c \
> brw_tes_surface_state.c \
> brw_tex.c \
> brw_tex_layout.c \
> diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h 
> b/src/mesa/drivers/dri/i965/brw_compiler.h
> index c9e0317..64d831d 100644
> --- a/src/mesa/drivers/dri/i965/brw_compiler.h
> +++ b/src/mesa/drivers/dri/i965/brw_compiler.h
> @@ -191,6 +191,14 @@ struct brw_vs_prog_key {
> struct brw_sampler_prog_key_data tex;
>  };
>  
> +/** The program key for Tessellation Evaluation Shaders. */
> +struct brw_tes_prog_key
> +{
> +   unsigned program_string_id;
> +
> +   struct brw_sampler_prog_key_data tex;
> +};
> +
>  /** The program key for Geometry Shaders. */
>  struct brw_gs_prog_key
>  {
> @@ -669,6 +677,22 @@ brw_compile_vs(const struct brw_compiler *compiler, void 
> *log_data,
> char **error_str);
>  
>  /**
> + * Compile a tessellation evaluation shader.
> + *
> + * Returns the final assembly and the program's size.
> + */
> +const unsigned *
> +brw_compile_tes(const struct brw_compiler *compiler, void *log_data,
> +void *mem_ctx,
> +const struct brw_tes_prog_key *key,
> +struct brw_tes_prog_data *prog_data,
> +const struct nir_shader *shader,
> +struct gl_shader_program *shader_prog,
> +int shader_time_index,
> +unsigned *final_assembly_size,
> +char **error_str);
> +
> +/**
>   * Compile a vertex shader.
>   *
>   * Returns the final assembly and the program's size.
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 69bc04c..5e840d1 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1704,6 +1704,12 @@ brw_vertex_program_const(const struct 
> gl_vertex_program *p)
> return (const struct brw_vertex_program *) p;
>  }
>  
> +static inline struct brw_tess_eval_program *
> +brw_tess_eval_program(struct gl_tess_eval_program *p)
> +{
> +   return (struct brw_tess_eval_program *) p;
> +}
> +
>  static inline struct brw_geometry_program *
>  brw_geometry_program(struct gl_geometry_program *p)
>  {
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index c833ef0..de584e4 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -1686,6 +1686,21 @@ fs_visitor::assign_vs_urb_setup()
>  }
>  
>  void
> +fs_visitor::assign_tes_urb_setup()
> +{
> +   assert(stage == MESA_SHADER_TESS_EVAL);
> +
> +   brw_vue_prog_data *vue_prog_data = (brw_vue_prog_data *) prog_data;
> +
> +   first_non_payload_grf += 8 * vue_prog_data->urb_read_length;
> +
> +   /* Rewrite all ATTR file references to HW_REGs. */
> +   foreach_block_and_inst(block, fs_inst, inst, cfg) {
> +  convert_attr_sources_to_hw_regs(inst);
> +   }
> +}
> +
> 

Re: [Mesa-dev] [PATCH v2 05/13] i965: Allocate URB space for HS and DS stages when required.

[Kenneth Graunke]

On Saturday, December 12, 2015 10:39:13 AM Jordan Justen wrote:
> On 2015-12-11 13:23:54, Kenneth Graunke wrote:
> > From: Chris Forbes 
> > 
> > v2: Rewrite the push constant allocation code to be clearer.
> > Only apply the minimum VS entries workaround on Gen 8.
> > 
> > Signed-off-by: Chris Forbes 
> > Signed-off-by: Kenneth Graunke 
> > ---
> >  src/mesa/drivers/dri/i965/brw_context.h  |  21 +++-
> >  src/mesa/drivers/dri/i965/gen7_blorp.cpp |   8 ++
> >  src/mesa/drivers/dri/i965/gen7_urb.c | 174 
> > ---
> >  3 files changed, 166 insertions(+), 37 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> > b/src/mesa/drivers/dri/i965/brw_context.h
> > index 1cc4c7b..69bc04c 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.h
> > +++ b/src/mesa/drivers/dri/i965/brw_context.h
> > @@ -1008,6 +1008,8 @@ struct brw_context
> > struct {
> >GLuint vsize;/* vertex size plus header in urb registers 
> > */
> >GLuint gsize;/* GS output size in urb registers */
> > +  GLuint hsize; /* Tessellation control output size in urb 
> > registers */
> > +  GLuint dsize; /* Tessellation evaluation output size in 
> > urb registers */
> >GLuint csize;/* constant buffer size in urb registers */
> >GLuint sfsize;   /* setup data size in urb registers */
> >  
> > @@ -1020,12 +1022,16 @@ struct brw_context
> >GLuint max_gs_entries;   /* Maximum number of GS entries */
> >  
> >GLuint nr_vs_entries;
> > +  GLuint nr_hs_entries;
> > +  GLuint nr_ds_entries;
> >GLuint nr_gs_entries;
> >GLuint nr_clip_entries;
> >GLuint nr_sf_entries;
> >GLuint nr_cs_entries;
> >  
> >GLuint vs_start;
> > +  GLuint hs_start;
> > +  GLuint ds_start;
> >GLuint gs_start;
> >GLuint clip_start;
> >GLuint sf_start;
> > @@ -1042,6 +1048,11 @@ struct brw_context
> > * URB space for the GS.
> > */
> >bool gs_present;
> > +
> > +  /* True if the most recently sent _3DSTATE_URB message allocated
> > +   * URB space for the HS and DS.
> > +   */
> > +  bool tess_present;
> > } urb;
> >  
> >  
> > @@ -1648,12 +1659,18 @@ void gen8_emit_3dstate_sample_pattern(struct 
> > brw_context *brw);
> >  /* gen7_urb.c */
> >  void
> >  gen7_emit_push_constant_state(struct brw_context *brw, unsigned vs_size,
> > +  unsigned hs_size, unsigned ds_size,
> >unsigned gs_size, unsigned fs_size);
> >  
> >  void
> >  gen7_emit_urb_state(struct brw_context *brw,
> > -unsigned nr_vs_entries, unsigned vs_size,
> > -unsigned vs_start, unsigned nr_gs_entries,
> > +unsigned nr_vs_entries,
> > +unsigned vs_size, unsigned vs_start,
> > +unsigned nr_hs_entries,
> > +unsigned hs_size, unsigned hs_start,
> > +unsigned nr_ds_entries,
> > +unsigned ds_size, unsigned ds_start,
> > +unsigned nr_gs_entries,
> >  unsigned gs_size, unsigned gs_start);
> >  
> >  
> > diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp 
> > b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > index e87b9d1..89b73ca 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > @@ -50,6 +50,8 @@ gen7_blorp_emit_urb_config(struct brw_context *brw)
> > unsigned urb_size = (brw->is_haswell && brw->gt == 3) ? 32 : 16;
> > gen7_emit_push_constant_state(brw,
> >   urb_size / 2 /* vs_size */,
> > + 0 /* hs_size */,
> > + 0 /* ds_size */,
> >   0 /* gs_size */,
> >   urb_size / 2 /* fs_size */);
> >  
> > @@ -60,6 +62,12 @@ gen7_blorp_emit_urb_config(struct brw_context *brw)
> > 32 /* num_vs_entries */,
> > 2 /* vs_size */,
> > 2 /* vs_start */,
> > +   0 /* num_hs_entries */,
> > +   1 /* hs_size */,
> > +   2 /* hs_start */,
> > +   0 /* num_ds_entries */,
> > +   1 /* ds_size */,
> > +   2 /* ds_start */,
> > 0 /* num_gs_entries */,
> > 1 /* gs_size */,
> > 2 /* gs_start */);
> > diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c 
> > b/src/mesa/drivers/dri/i965/gen7_urb.c
> > index 99a9d3c..a598c89 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_urb.c
> > +++ b/src/mesa/drivers/dri/i965/gen7_urb.c
> > @@ -34,7 +34,7 @@
> >   

[Mesa-dev] [PATCH 1/2] i965: Use DIV_ROUND_UP() in gen7_urb.c code.

[Kenneth Graunke]

This is a newer convention, which we prefer over ALIGN(x, n) / n.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/gen7_urb.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c 
b/src/mesa/drivers/dri/i965/gen7_urb.c
index 99a9d3c..421512b 100644
--- a/src/mesa/drivers/dri/i965/gen7_urb.c
+++ b/src/mesa/drivers/dri/i965/gen7_urb.c
@@ -193,11 +193,11 @@ gen7_upload_urb(struct brw_context *brw)
 
/* VS has a lower limit on the number of URB entries */
unsigned vs_chunks =
-  ALIGN(brw->urb.min_vs_entries * vs_entry_size_bytes, chunk_size_bytes) /
-  chunk_size_bytes;
+  DIV_ROUND_UP(brw->urb.min_vs_entries * vs_entry_size_bytes,
+   chunk_size_bytes);
unsigned vs_wants =
-  ALIGN(brw->urb.max_vs_entries * vs_entry_size_bytes,
-chunk_size_bytes) / chunk_size_bytes - vs_chunks;
+  DIV_ROUND_UP(brw->urb.max_vs_entries * vs_entry_size_bytes,
+   chunk_size_bytes) - vs_chunks;
 
unsigned gs_chunks = 0;
unsigned gs_wants = 0;
@@ -210,11 +210,10 @@ gen7_upload_urb(struct brw_context *brw)
*
* (2) We can't allocate less than nr_gs_entries_granularity.
*/
-  gs_chunks = ALIGN(MAX2(gs_granularity, 2) * gs_entry_size_bytes,
-chunk_size_bytes) / chunk_size_bytes;
-  gs_wants =
- ALIGN(brw->urb.max_gs_entries * gs_entry_size_bytes,
-   chunk_size_bytes) / chunk_size_bytes - gs_chunks;
+  gs_chunks = DIV_ROUND_UP(MAX2(gs_granularity, 2) * gs_entry_size_bytes,
+   chunk_size_bytes);
+  gs_wants = DIV_ROUND_UP(brw->urb.max_gs_entries * gs_entry_size_bytes,
+  chunk_size_bytes) - gs_chunks;
}
 
/* There should always be enough URB space to satisfy the minimum
-- 
2.6.3

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

[Mesa-dev] [PATCH 2/2] i965: Allocate URB space for HS and DS stages when required.

[Kenneth Graunke]

From: Chris Forbes 

v2: (by Ken, incorporating feedback from Matt Turner):
- Rewrite the push constant allocation code to be clearer.
- Only apply the minimum VS entries workaround on Gen 8.

v3: (by Ken)
- Fix a bug in v2 where we failed to allocate the full push constant
  space when the number of enabled stages didn't divide the available
  push constant space evenly.  (Any left over space is now allocated
  to the PS, as it was in v1.)
- Fix an off-by-one error in v2's number of enabled stages calculation.
- Use DIV_ROUND_UP for nicer formatting.
- Line wrapping fixes.

Signed-off-by: Chris Forbes 
Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_context.h  |  21 +++-
 src/mesa/drivers/dri/i965/gen7_blorp.cpp |   8 ++
 src/mesa/drivers/dri/i965/gen7_urb.c | 177 +--
 3 files changed, 170 insertions(+), 36 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 1cc4c7b..69bc04c 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -1008,6 +1008,8 @@ struct brw_context
struct {
   GLuint vsize;/* vertex size plus header in urb registers */
   GLuint gsize;/* GS output size in urb registers */
+  GLuint hsize; /* Tessellation control output size in urb 
registers */
+  GLuint dsize; /* Tessellation evaluation output size in urb 
registers */
   GLuint csize;/* constant buffer size in urb registers */
   GLuint sfsize;   /* setup data size in urb registers */
 
@@ -1020,12 +1022,16 @@ struct brw_context
   GLuint max_gs_entries;   /* Maximum number of GS entries */
 
   GLuint nr_vs_entries;
+  GLuint nr_hs_entries;
+  GLuint nr_ds_entries;
   GLuint nr_gs_entries;
   GLuint nr_clip_entries;
   GLuint nr_sf_entries;
   GLuint nr_cs_entries;
 
   GLuint vs_start;
+  GLuint hs_start;
+  GLuint ds_start;
   GLuint gs_start;
   GLuint clip_start;
   GLuint sf_start;
@@ -1042,6 +1048,11 @@ struct brw_context
* URB space for the GS.
*/
   bool gs_present;
+
+  /* True if the most recently sent _3DSTATE_URB message allocated
+   * URB space for the HS and DS.
+   */
+  bool tess_present;
} urb;
 
 
@@ -1648,12 +1659,18 @@ void gen8_emit_3dstate_sample_pattern(struct 
brw_context *brw);
 /* gen7_urb.c */
 void
 gen7_emit_push_constant_state(struct brw_context *brw, unsigned vs_size,
+  unsigned hs_size, unsigned ds_size,
   unsigned gs_size, unsigned fs_size);
 
 void
 gen7_emit_urb_state(struct brw_context *brw,
-unsigned nr_vs_entries, unsigned vs_size,
-unsigned vs_start, unsigned nr_gs_entries,
+unsigned nr_vs_entries,
+unsigned vs_size, unsigned vs_start,
+unsigned nr_hs_entries,
+unsigned hs_size, unsigned hs_start,
+unsigned nr_ds_entries,
+unsigned ds_size, unsigned ds_start,
+unsigned nr_gs_entries,
 unsigned gs_size, unsigned gs_start);
 
 
diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp 
b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
index e87b9d1..89b73ca 100644
--- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
+++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
@@ -50,6 +50,8 @@ gen7_blorp_emit_urb_config(struct brw_context *brw)
unsigned urb_size = (brw->is_haswell && brw->gt == 3) ? 32 : 16;
gen7_emit_push_constant_state(brw,
  urb_size / 2 /* vs_size */,
+ 0 /* hs_size */,
+ 0 /* ds_size */,
  0 /* gs_size */,
  urb_size / 2 /* fs_size */);
 
@@ -60,6 +62,12 @@ gen7_blorp_emit_urb_config(struct brw_context *brw)
32 /* num_vs_entries */,
2 /* vs_size */,
2 /* vs_start */,
+   0 /* num_hs_entries */,
+   1 /* hs_size */,
+   2 /* hs_start */,
+   0 /* num_ds_entries */,
+   1 /* ds_size */,
+   2 /* ds_start */,
0 /* num_gs_entries */,
1 /* gs_size */,
2 /* gs_start */);
diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c 
b/src/mesa/drivers/dri/i965/gen7_urb.c
index 421512b..f2b6ec3 100644
--- a/src/mesa/drivers/dri/i965/gen7_urb.c
+++ b/src/mesa/drivers/dri/i965/gen7_urb.c
@@ -34,7 +34,7 @@
  *   __-__   _-_
  *  / \ /   \
  * 

Re: [Mesa-dev] [Intel-gfx] [RFC libdrm] intel: Add support for softpin

[Emil Velikov]

On 14 December 2015 at 09:04, Daniel Vetter  wrote:
> On Mon, Dec 14, 2015 at 08:41:05AM +, Song, Ruiling wrote:
>>
>>
>> > -Original Message-
>> > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
>> > Vetter
>> > Sent: Monday, December 14, 2015 4:28 PM
>> > To: Song, Ruiling 
>> > Cc: k...@bitplanet.net; Winiarski, Michal ;
>> > mesa-dev@lists.freedesktop.org; intel-...@lists.freedesktop.org; Ben
>> > Widawsky ; dri-de...@lists.freedesktop.org
>> > Subject: Re: [Intel-gfx] [RFC libdrm] intel: Add support for softpin
>> >
>> > On Mon, Dec 14, 2015 at 07:24:29AM +, Song, Ruiling wrote:
>> > >
>> > >
>> > > > -Original Message-
>> > > > From: hoegsb...@gmail.com [mailto:hoegsb...@gmail.com] On Behalf
>> > Of
>> > > > Kristian H?gsberg
>> > > > Sent: Monday, December 14, 2015 1:34 PM
>> > > > To: Song, Ruiling 
>> > > > Cc: Winiarski, Michal ; intel-
>> > > > g...@lists.freedesktop.org; mesa-dev@lists.freedesktop.org; Ben
>> > Widawsky
>> > > > ; dri-de...@lists.freedesktop.org
>> > > > Subject: Re: [Intel-gfx] [RFC libdrm] intel: Add support for softpin
>> > > >
>> > > > On Sun, Dec 13, 2015 at 7:17 PM, Song, Ruiling 
>> > > > wrote:
>> > > > >> -Original Message-
>> > > > >> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On
>> > > > Behalf
>> > > > >> Of Micha? Winiarski
>> > > > >> Sent: Wednesday, September 9, 2015 10:07 PM
>> > > > >> To: intel-...@lists.freedesktop.org
>> > > > >> Cc: Ben Widawsky ; dri-
>> > > > de...@lists.freedesktop.org;
>> > > > >> mesa-dev@lists.freedesktop.org
>> > > > >> Subject: [Intel-gfx] [RFC libdrm] intel: Add support for softpin
>> > > > >>
>> > > > >> Softpin allows userspace to take greater control of GPU virtual 
>> > > > >> address
>> > > > >> space and eliminates the need of relocations. It can also be used to
>> > > > >> mirror addresses between GPU and CPU (shared virtual memory).
>> > > > >> Calls to drm_intel_bo_emit_reloc are still required to build the 
>> > > > >> list of
>> > > > >> drm_i915_gem_exec_objects at exec time, but no entries in relocs are
>> > > > >> created. Self-relocs don't make any sense for softpinned objects and
>> > can
>> > > > >> indicate a programming errors, thus are forbidden. Softpinned 
>> > > > >> objects
>> > > > >> are marked by asterisk in debug dumps.
>> > > > >>
>> > > > >> Cc: Thomas Daniel 
>> > > > >> Cc: Kristian Høgsberg 
>> > > > >> Cc: Zou Nanhai 
>> > > > >> Cc: Michel Thierry 
>> > > > >> Cc: Ben Widawsky 
>> > > > >> Cc: Chris Wilson 
>> > > > >> Signed-off-by: Michał Winiarski 
>> > > > >> ---
>> > > > >>  include/drm/i915_drm.h|   4 +-
>> > > > >>  intel/intel_bufmgr.c  |   9 +++
>> > > > >>  intel/intel_bufmgr.h  |   1 +
>> > > > >>  intel/intel_bufmgr_gem.c  | 176
>> > > > >> --
>> > > > >>  intel/intel_bufmgr_priv.h |   7 ++
>> > > > >>  5 files changed, 173 insertions(+), 24 deletions(-)
>> > > > >
>> > > > > Will anybody help to push the patch to libdrm? Beignet highly depend
>> > on
>> > > > this to implement ocl2.0 svm.
>> > > >
>> > > > Is the kernel patch upstream?
>> > >
>> > > Yes, the kernel patch already merged, see:
>> > > http://cgit.freedesktop.org/drm-
>> > intel/commit/?id=506a8e87d8d2746b9e9d2433503fe237c54e4750
>> > >
>> > > I find below line of code in libdrm does not match the kernel version. 
>> > > The
>> > kernel patch defined as:
>> > > "#define EXEC_OBJECT_PINNED (1<<4)", but this patch defined it as (1<<3).
>> >
>> > Please always regenerate the entire headers from the kernel sources using
>> >
>> > $ make headers_install
>> >
>> > Then copy the headers from the kernel's usr/include/drm to libdrm. Never
>> > patch i915_drm.h manually.
>>
>> Thanks for the info. But the problem is libdrm still tracks such kind of 
>> header files.
>> Should this kind of header file be removed from libdrm? Or any document in 
>> libdrm to make this explicit?
>
> All other kernel headers are extracted from the kernel directly, but
> generally those packages only get update when a new kernel comes out.
> Usually it's called linux-headers or similar. And only updating headers
> when the release is out is _way_ too slow for drm/gfx. That's why we have
> drm headers in libdrm.
>
That plus hysterical raisins, from when drm was part of userspace ;-)

> But yeah we should document this, maybe even script it. Perhaps even just
> upgrade headers automatically as soon as the upstream drm-next branch
> changes.
I'm all for tweaking the make target, although automating to the point
of zero developer interaction might not be ideal. 

Re: [Mesa-dev] [PATCH 7/8] st/va: add NV12 -> NV12 post processing

[Julien Isorce]

This patch is:
Reviewed-by: Julien Isorce 
Tested-by: Julien Isorce 

On 11 December 2015 at 12:33, Christian König 
wrote:

> From: Christian König 
>
> Usefull for mpv and GStreamer.
>
> Signed-off-by: Indrajit-kumar Das 
> Signed-off-by: Christian König 
> ---
>  src/gallium/state_trackers/va/picture.c  |  10 +-
>  src/gallium/state_trackers/va/postproc.c | 152
> ---
>  2 files changed, 125 insertions(+), 37 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/picture.c
> b/src/gallium/state_trackers/va/picture.c
> index 7b30bf8..b5e1ebc 100644
> --- a/src/gallium/state_trackers/va/picture.c
> +++ b/src/gallium/state_trackers/va/picture.c
> @@ -63,11 +63,11 @@ vlVaBeginPicture(VADriverContextP ctx, VAContextID
> context_id, VASurfaceID rende
> if (!context->decoder) {
>/* VPP */
>if (context->templat.profile == PIPE_VIDEO_PROFILE_UNKNOWN &&
> - ((context->target->buffer_format != PIPE_FORMAT_B8G8R8A8_UNORM
> &&
> -   context->target->buffer_format != PIPE_FORMAT_R8G8B8A8_UNORM
> &&
> -   context->target->buffer_format != PIPE_FORMAT_B8G8R8X8_UNORM
> &&
> -   context->target->buffer_format != PIPE_FORMAT_R8G8B8X8_UNORM)
> ||
> -   context->target->interlaced))
> +  context->target->buffer_format != PIPE_FORMAT_B8G8R8A8_UNORM &&
> +  context->target->buffer_format != PIPE_FORMAT_R8G8B8A8_UNORM &&
> +  context->target->buffer_format != PIPE_FORMAT_B8G8R8X8_UNORM &&
> +  context->target->buffer_format != PIPE_FORMAT_R8G8B8X8_UNORM &&
> +  context->target->buffer_format != PIPE_FORMAT_NV12)
>   return VA_STATUS_ERROR_UNIMPLEMENTED;
>
>return VA_STATUS_SUCCESS;
> diff --git a/src/gallium/state_trackers/va/postproc.c
> b/src/gallium/state_trackers/va/postproc.c
> index 1ee3587..c6cfd3a 100644
> --- a/src/gallium/state_trackers/va/postproc.c
> +++ b/src/gallium/state_trackers/va/postproc.c
> @@ -27,6 +27,8 @@
>
>  #include "util/u_handle_table.h"
>
> +#include "vl/vl_defines.h"
> +
>  #include "va_private.h"
>
>  static const VARectangle *
> @@ -44,43 +46,21 @@ vlVaRegionDefault(const VARectangle *region, struct
> pipe_video_buffer *buf,
> return def;
>  }
>
> -VAStatus
> -vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContext
> *context, vlVaBuffer *buf)
> +static VAStatus
> +vlVaPostProcCompositor(vlVaDriver *drv, vlVaContext *context,
> +   const VARectangle *src_region,
> +   const VARectangle *dst_region,
> +   struct pipe_video_buffer *src,
> +   struct pipe_video_buffer *dst)
>  {
> -   VARectangle def_src_region, def_dst_region;
> -   const VARectangle *src_region, *dst_region;
> +   struct pipe_surface **surfaces;
> struct u_rect src_rect;
> struct u_rect dst_rect;
> -   vlVaSurface *src_surface;
> -   VAProcPipelineParameterBuffer *pipeline_param;
> -   struct pipe_surface **surfaces;
> -   struct pipe_surface *psurf;
> -
> -   if (!drv || !context)
> -  return VA_STATUS_ERROR_INVALID_CONTEXT;
> -
> -   if (!buf || !buf->data)
> -  return VA_STATUS_ERROR_INVALID_BUFFER;
> -
> -   if (!context->target)
> -  return VA_STATUS_ERROR_INVALID_SURFACE;
> -
> -   pipeline_param = (VAProcPipelineParameterBuffer *)buf->data;
> -
> -   src_surface = handle_table_get(drv->htab, pipeline_param->surface);
> -   if (!src_surface || !src_surface->buffer)
> -  return VA_STATUS_ERROR_INVALID_SURFACE;
> -
> -   surfaces = context->target->get_surfaces(context->target);
>
> +   surfaces = dst->get_surfaces(dst);
> if (!surfaces || !surfaces[0])
>return VA_STATUS_ERROR_INVALID_SURFACE;
>
> -   psurf = surfaces[0];
> -
> -   src_region = vlVaRegionDefault(pipeline_param->surface_region,
> src_surface->buffer, _src_region);
> -   dst_region = vlVaRegionDefault(pipeline_param->output_region,
> context->target, _dst_region);
> -
> src_rect.x0 = src_region->x;
> src_rect.y0 = src_region->y;
> src_rect.x1 = src_region->x + src_region->width;
> @@ -92,11 +72,119 @@
> vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContext
> *contex
> dst_rect.y1 = dst_region->y + dst_region->height;
>
> vl_compositor_clear_layers(>cstate);
> -   vl_compositor_set_buffer_layer(>cstate, >compositor, 0,
> src_surface->buffer, _rect, NULL, VL_COMPOSITOR_WEAVE);
> +   vl_compositor_set_buffer_layer(>cstate, >compositor, 0, src,
> + _rect, NULL, VL_COMPOSITOR_WEAVE);
> vl_compositor_set_layer_dst_area(>cstate, 0, _rect);
> -   vl_compositor_render(>cstate, >compositor, psurf, NULL,
> false);
> +   vl_compositor_render(>cstate, >compositor, surfaces[0],
> NULL, false);
> +
> +   return VA_STATUS_SUCCESS;
> +}
> +
> +static void vlVaBoxAdjust(struct 

Re: [Mesa-dev] SSBO + access qualifiers

[Jason Ekstrand]

On Mon, Dec 14, 2015 at 8:03 PM, Ilia Mirkin  wrote:
> On Mon, Dec 14, 2015 at 10:50 PM, Jason Ekstrand  wrote:
>> On Mon, Dec 14, 2015 at 4:10 PM, Ilia Mirkin  wrote:
>>> Hello,
>>>
>>> I was going to add support for the various volatile/etc annotations in
>>> my gallium impl (which is fairly far along), but it seems like those
>>> are currently being dropped on the floor by lower_ubo_references, and
>>> NIR has no support for them either. Both in GLSL IR and NIR, the
>>> variable is what holds the access annotation (volatile, etc). However
>>> the ssbo intrinsics are all about a particular binding and offset,
>>> without any reference to the variable.
>>>
>>> What's the right way of handling this? (And do any tests exist that
>>> would be sensitive to getting such things wrong?)
>>
>> First off, why is it that way?  Well, because most of the IRs in mesa
>> don't have a memory model capable of handling these sorts of things.
>> Those that do (LLVM is the only one I'm aware of) can't handle the GL
>> packing rules.  The result is that we basically have to lower
>> everything away to byte-offset load/store intrinsics.
>>
>> What do we do about it?  My inclination would be to either add two new
>> intrinsics for load/store_ssbo_volatile or to add a new constant
>> boolean "volatile" parameter to the current intrinsics.  If a
>> load/store happens on a volatile things, you get the volatile version,
>> otherwise you get the plane version.  Then backends can know that they
>> are free to reorder, combine, etc. non-volatile load/store operations
>> as per their memory model and the provided barriers.  If they
>> encounter a volatile load/store, they can't do anything with it and
>> just have to do the memory op.
>>
>> Is that reasonable?
>
> Well, I don't really care much about the NIR-side of it, but I think
> at the very least coherent and volatile need to be exposed. I don't
> much care about restrict, that's a bit too fancy for me, but no harm
> in passing it through. TBH I don't get the point of
> readonly/writeonly. FWIW this is what the nvidia hw is capable of:
> http://docs.nvidia.com/cuda/parallel-thread-execution/#cache-operators
> (I know it's PTX, but the actual hw is basically the same.)
>
> I think that volatile = cv, coherent = cg, and everything else = ca/cs
> based on some clever heuristic (aka I'll always pick one of those
> until a situation where that doesn't work well arises).

Right.  I don't actually know what we can do with our HW.  I was
mostly thinking about what optimization passes can or cannot do with a
variable.  It sounds like, whatever we do, you want it passed through
somehow.  How about we just add a little well-defined bitfield as an
extra argument to the SSBO load/store intrinsics?  The fact that it
gets applied to a variable or a struct or a member or whatever is kind
of a moot point to the backend compiler.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] SSBO + access qualifiers

[Jason Ekstrand]

On Mon, Dec 14, 2015 at 8:57 PM, Ilia Mirkin  wrote:
> On Mon, Dec 14, 2015 at 11:44 PM, Jason Ekstrand  wrote:
>> On Mon, Dec 14, 2015 at 8:03 PM, Ilia Mirkin  wrote:
>>> On Mon, Dec 14, 2015 at 10:50 PM, Jason Ekstrand  
>>> wrote:
 On Mon, Dec 14, 2015 at 4:10 PM, Ilia Mirkin  wrote:
> Hello,
>
> I was going to add support for the various volatile/etc annotations in
> my gallium impl (which is fairly far along), but it seems like those
> are currently being dropped on the floor by lower_ubo_references, and
> NIR has no support for them either. Both in GLSL IR and NIR, the
> variable is what holds the access annotation (volatile, etc). However
> the ssbo intrinsics are all about a particular binding and offset,
> without any reference to the variable.
>
> What's the right way of handling this? (And do any tests exist that
> would be sensitive to getting such things wrong?)

 First off, why is it that way?  Well, because most of the IRs in mesa
 don't have a memory model capable of handling these sorts of things.
 Those that do (LLVM is the only one I'm aware of) can't handle the GL
 packing rules.  The result is that we basically have to lower
 everything away to byte-offset load/store intrinsics.

 What do we do about it?  My inclination would be to either add two new
 intrinsics for load/store_ssbo_volatile or to add a new constant
 boolean "volatile" parameter to the current intrinsics.  If a
 load/store happens on a volatile things, you get the volatile version,
 otherwise you get the plane version.  Then backends can know that they
 are free to reorder, combine, etc. non-volatile load/store operations
 as per their memory model and the provided barriers.  If they
 encounter a volatile load/store, they can't do anything with it and
 just have to do the memory op.

 Is that reasonable?
>>>
>>> Well, I don't really care much about the NIR-side of it, but I think
>>> at the very least coherent and volatile need to be exposed. I don't
>>> much care about restrict, that's a bit too fancy for me, but no harm
>>> in passing it through. TBH I don't get the point of
>>> readonly/writeonly. FWIW this is what the nvidia hw is capable of:
>>> http://docs.nvidia.com/cuda/parallel-thread-execution/#cache-operators
>>> (I know it's PTX, but the actual hw is basically the same.)
>>>
>>> I think that volatile = cv, coherent = cg, and everything else = ca/cs
>>> based on some clever heuristic (aka I'll always pick one of those
>>> until a situation where that doesn't work well arises).
>>
>> Right.  I don't actually know what we can do with our HW.  I was
>> mostly thinking about what optimization passes can or cannot do with a
>> variable.  It sounds like, whatever we do, you want it passed through
>> somehow.  How about we just add a little well-defined bitfield as an
>> extra argument to the SSBO load/store intrinsics?  The fact that it
>> gets applied to a variable or a struct or a member or whatever is kind
>> of a moot point to the backend compiler.
>
> An extra argument with a mask or enum is precisely what I was
> expecting to find when looking at the lower_ubo_references pass when I
> was rather surprised to see that the whole thing had been overlooked.

Seems reasonable to me.  However, Intel hardware doesn't really give
you that fine-grained control so we didn't care.  Sounds like someone
should add it.  Have fun!
--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 86326] clEnqueueNDRangeKernel global_work_offset ignored

[bugzilla-daemon]

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

--- Comment #14 from Vedran Miletić  ---
Ronie, was this fixed?

-- 
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

Re: [Mesa-dev] [PATCH 4/5] nv50: check return value of nouveau_object_new()

[Ilia Mirkin]

Bogus. obj will be null on failure.

On Mon, Dec 14, 2015 at 5:14 AM, Samuel Pitoiset
 wrote:
> Spotted by Coverity.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/nv50/nv84_video.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv84_video.c 
> b/src/gallium/drivers/nouveau/nv50/nv84_video.c
> index 7a4670f..5e2489a 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv84_video.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv84_video.c
> @@ -756,8 +756,8 @@ firmware_present(struct pipe_screen *pscreen, enum 
> pipe_video_format codec)
> int present, ret;
>
> if (!FIRMWARE_PRESENT(checked, VP_KERN)) {
> -  nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0, );
> -  if (obj)
> +  ret = nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0, );
> +  if (!ret && obj)
>   screen->firmware_info.profiles_present |= FIRMWARE_VP_KERN;
>nouveau_object_del();
>screen->firmware_info.profiles_checked |= FIRMWARE_VP_KERN;
> @@ -765,8 +765,8 @@ firmware_present(struct pipe_screen *pscreen, enum 
> pipe_video_format codec)
>
> if (codec == PIPE_VIDEO_FORMAT_MPEG4_AVC) {
>if (!FIRMWARE_PRESENT(checked, BSP_KERN)) {
> - nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0, );
> - if (obj)
> + ret = nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0, );
> + if (!ret && obj)
>  screen->firmware_info.profiles_present |= FIRMWARE_BSP_KERN;
>   nouveau_object_del();
>   screen->firmware_info.profiles_checked |= FIRMWARE_BSP_KERN;
> --
> 2.6.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 4/5] nv50: check return value of nouveau_object_new()

[Samuel Pitoiset]

On 12/14/2015 04:05 PM, Ilia Mirkin wrote:

Bogus. obj will be null on failure.


Well, checking the return value instead of obj seems better tbh, because 
we do that in the rest of the code.




On Mon, Dec 14, 2015 at 5:14 AM, Samuel Pitoiset
 wrote:

Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
  src/gallium/drivers/nouveau/nv50/nv84_video.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv84_video.c 
b/src/gallium/drivers/nouveau/nv50/nv84_video.c
index 7a4670f..5e2489a 100644
--- a/src/gallium/drivers/nouveau/nv50/nv84_video.c
+++ b/src/gallium/drivers/nouveau/nv50/nv84_video.c
@@ -756,8 +756,8 @@ firmware_present(struct pipe_screen *pscreen, enum 
pipe_video_format codec)
 int present, ret;

 if (!FIRMWARE_PRESENT(checked, VP_KERN)) {
-  nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0, );
-  if (obj)
+  ret = nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0, );
+  if (!ret && obj)
   screen->firmware_info.profiles_present |= FIRMWARE_VP_KERN;
nouveau_object_del();
screen->firmware_info.profiles_checked |= FIRMWARE_VP_KERN;
@@ -765,8 +765,8 @@ firmware_present(struct pipe_screen *pscreen, enum 
pipe_video_format codec)

 if (codec == PIPE_VIDEO_FORMAT_MPEG4_AVC) {
if (!FIRMWARE_PRESENT(checked, BSP_KERN)) {
- nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0, );
- if (obj)
+ ret = nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0, );
+ if (!ret && obj)
  screen->firmware_info.profiles_present |= FIRMWARE_BSP_KERN;
   nouveau_object_del();
   screen->firmware_info.profiles_checked |= FIRMWARE_BSP_KERN;
--
2.6.4

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


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

Re: [Mesa-dev] [PATCH 3/5] nvc0: make sure gmt programs are correctly validated

[Ilia Mirkin]

On Mon, Dec 14, 2015 at 11:02 AM, Samuel Pitoiset
 wrote:
>
>
> On 12/14/2015 04:04 PM, Ilia Mirkin wrote:
>>
>> Instead take the return value and stick it into the if below.
>> Returning early from these functions isn't something we generally want
>> to do.
>
>
> We already return at the same point for vertex programs, but I can
> definitely move this below.

If you return, whatever was set before stays. That's not great.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/8] st/va: clean up post process includes

[Julien Isorce]

This patch is:
Reviewed-by: Julien Isorce 
Tested-by: Julien Isorce 

On 11 December 2015 at 12:33, Christian König 
wrote:

> From: Christian König 
>
> Signed-off-by: Christian König 
> ---
>  src/gallium/state_trackers/va/postproc.c | 6 --
>  1 file changed, 6 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/postproc.c
> b/src/gallium/state_trackers/va/postproc.c
> index 1fdb4e7..2d17694 100644
> --- a/src/gallium/state_trackers/va/postproc.c
> +++ b/src/gallium/state_trackers/va/postproc.c
> @@ -25,13 +25,7 @@
>   *
>
> **/
>
> -//#include "pipe/p_video_codec.h"
> -
>  #include "util/u_handle_table.h"
> -//#include "util/u_video.h"
> -
> -//#include "vl/vl_vlc.h"
> -//#include "vl/vl_winsys.h"
>
>  #include "va_private.h"
>
> --
> 2.5.0
>
> ___
> 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/8] st/va: cleanup filter color standard handling

[Julien Isorce]

This patch is:
Reviewed-by: Julien Isorce 
Tested-by: ulien Isorce 

On 11 December 2015 at 12:33, Christian König 
wrote:

> From: Christian König 
>
> Signed-off-by: Christian König 
> ---
>  src/gallium/state_trackers/va/surface.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/surface.c
> b/src/gallium/state_trackers/va/surface.c
> index c052c8f..4a18a6f 100644
> --- a/src/gallium/state_trackers/va/surface.c
> +++ b/src/gallium/state_trackers/va/surface.c
> @@ -697,11 +697,11 @@ vlVaQueryVideoProcFilterCaps(VADriverContextP ctx,
> VAContextID context,
> return VA_STATUS_SUCCESS;
>  }
>
> -static VAProcColorStandardType
> vpp_input_color_standards[VAProcColorStandardCount] = {
> +static VAProcColorStandardType vpp_input_color_standards[] = {
> VAProcColorStandardBT601
>  };
>
> -static VAProcColorStandardType
> vpp_output_color_standards[VAProcColorStandardCount] = {
> +static VAProcColorStandardType vpp_output_color_standards[] = {
> VAProcColorStandardBT601
>  };
>
> @@ -725,9 +725,9 @@ vlVaQueryVideoProcPipelineCaps(VADriverContextP ctx,
> VAContextID context,
> pipeline_cap->filter_flags = 0;
> pipeline_cap->num_forward_references = 0;
> pipeline_cap->num_backward_references = 0;
> -   pipeline_cap->num_input_color_standards = 1;
> +   pipeline_cap->num_input_color_standards =
> Elements(vpp_input_color_standards);
> pipeline_cap->input_color_standards = vpp_input_color_standards;
> -   pipeline_cap->num_output_color_standards = 1;
> +   pipeline_cap->num_output_color_standards =
> Elements(vpp_output_color_standards);
> pipeline_cap->output_color_standards = vpp_output_color_standards;
>
> for (i = 0; i < num_filters; i++) {
> --
> 2.5.0
>
> ___
> 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 4/8] st/va: fix unused variable warning

[Julien Isorce]

This patch is:
Reviewed-by: Julien Isorce 

On 11 December 2015 at 12:33, Christian König 
wrote:

> From: Christian König 
>
> Signed-off-by: Christian König 
> ---
>  src/gallium/state_trackers/va/picture.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/src/gallium/state_trackers/va/picture.c
> b/src/gallium/state_trackers/va/picture.c
> index 8623139..7b30bf8 100644
> --- a/src/gallium/state_trackers/va/picture.c
> +++ b/src/gallium/state_trackers/va/picture.c
> @@ -92,7 +92,6 @@ vlVaGetReferenceFrame(vlVaDriver *drv, VASurfaceID
> surface_id,
>  static VAStatus
>  handlePictureParameterBuffer(vlVaDriver *drv, vlVaContext *context,
> vlVaBuffer *buf)
>  {
> -   unsigned int i;
> VAStatus vaStatus = VA_STATUS_SUCCESS;
>
> switch (u_reduce_video_profile(context->templat.profile)) {
> --
> 2.5.0
>
> ___
> 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 5/8] st/va: handle default post process regions

[Julien Isorce]

This patch is:
Reviewed-by: Julien Isorce 
Tested-by: Julien Isorce 

On 11 December 2015 at 12:33, Christian König 
wrote:

> From: Christian König 
>
> Avoid referencing NULL pointers.
>
> Signed-off-by: Christian König 
> ---
>  src/gallium/state_trackers/va/postproc.c | 36
> +---
>  1 file changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/postproc.c
> b/src/gallium/state_trackers/va/postproc.c
> index 2d17694..105f251 100644
> --- a/src/gallium/state_trackers/va/postproc.c
> +++ b/src/gallium/state_trackers/va/postproc.c
> @@ -29,9 +29,26 @@
>
>  #include "va_private.h"
>
> +static const VARectangle *
> +vlVaRegionDefault(const VARectangle *region, struct pipe_video_buffer
> *buf,
> + VARectangle *def)
> +{
> +   if (region)
> +  return region;
> +
> +   def->x = 0;
> +   def->y = 0;
> +   def->width = buf->width;
> +   def->height = buf->height;
> +
> +   return def;
> +}
> +
>  VAStatus
>  vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContext
> *context, vlVaBuffer *buf)
>  {
> +   VARectangle def_src_region, def_dst_region;
> +   const VARectangle *src_region, *dst_region;
> struct u_rect src_rect;
> struct u_rect dst_rect;
> vlVaSurface *src_surface;
> @@ -64,15 +81,18 @@ vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver
> *drv, vlVaContext *contex
>
> psurf = surfaces[0];
>
> -   src_rect.x0 = pipeline_param->surface_region->x;
> -   src_rect.y0 = pipeline_param->surface_region->y;
> -   src_rect.x1 = pipeline_param->surface_region->x +
> pipeline_param->surface_region->width;
> -   src_rect.y1 = pipeline_param->surface_region->y +
> pipeline_param->surface_region->height;
> +   src_region = vlVaRegionDefault(pipeline_param->surface_region,
> src_surface->buffer, _src_region);
> +   dst_region = vlVaRegionDefault(pipeline_param->output_region,
> context->target, _dst_region);
> +
> +   src_rect.x0 = src_region->x;
> +   src_rect.y0 = src_region->y;
> +   src_rect.x1 = src_region->x + src_region->width;
> +   src_rect.y1 = src_region->y + src_region->height;
>
> -   dst_rect.x0 = pipeline_param->output_region->x;
> -   dst_rect.y0 = pipeline_param->output_region->y;
> -   dst_rect.x1 = pipeline_param->output_region->x +
> pipeline_param->output_region->width;
> -   dst_rect.y1 = pipeline_param->output_region->y +
> pipeline_param->output_region->height;
> +   dst_rect.x0 = dst_region->x;
> +   dst_rect.y0 = dst_region->y;
> +   dst_rect.x1 = dst_region->x + dst_region->width;
> +   dst_rect.y1 = dst_region->y + dst_region->height;
>
> vl_compositor_clear_layers(>cstate);
> vl_compositor_set_buffer_layer(>cstate, >compositor, 0,
> src_surface->buffer, _rect, NULL, VL_COMPOSITOR_WEAVE);
> --
> 2.5.0
>
> ___
> 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 5/5] nouveau: remove logically dead code in nouveau_vpe_mb_mv_header()

[Ilia Mirkin]

This code is old, crusty and subtle. Clearly this can't happen,
however I'd rather not remove it... if you really hate it how about a

#if 0 /* XXX */
...
#endif

I have no idea what DUAL_PRIME is, or whether it appears for frame or
field stuff. Or what that PMPEG command does for that matter.

On Mon, Dec 14, 2015 at 5:14 AM, Samuel Pitoiset
 wrote:
> frame cannot be NULL in that branch. Spotted by Coverity.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/nouveau_video.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nouveau_video.c 
> b/src/gallium/drivers/nouveau/nouveau_video.c
> index 8bb12b2..fe19bce 100644
> --- a/src/gallium/drivers/nouveau/nouveau_video.c
> +++ b/src/gallium/drivers/nouveau/nouveau_video.c
> @@ -317,8 +317,6 @@ nouveau_vpe_mb_mv_header(struct nouveau_decoder *dec,
>case PIPE_MPEG12_MO_TYPE_16x8: goto mv2;
>case PIPE_MPEG12_MO_TYPE_DUAL_PRIME: {
>base = NV17_MPEG_CMD_CHROMA_MV_HEADER_MV_SPLIT_HALF_MB;
> - if (frame)
> -base |= NV17_MPEG_CMD_CHROMA_MV_HEADER_TYPE_FRAME;
>   if (forward)
>  nouveau_vpe_mb_mv(dec, base, luma, frame, true,
>dec->picture_structure != 
> PIPE_MPEG12_PICTURE_STRUCTURE_FIELD_TOP,
> --
> 2.6.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 3/5] nvc0: make sure gmt programs are correctly validated

[Samuel Pitoiset]

On 12/14/2015 04:04 PM, Ilia Mirkin wrote:

Instead take the return value and stick it into the if below.
Returning early from these functions isn't something we generally want
to do.


We already return at the same point for vertex programs, but I can 
definitely move this below.




On Mon, Dec 14, 2015 at 5:14 AM, Samuel Pitoiset
 wrote:

Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
  src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
index 7e2e999..5e69e29 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
@@ -236,8 +236,10 @@ nvc0_gmtyprog_validate(struct nvc0_context *nvc0)
 struct nouveau_pushbuf *push = nvc0->base.pushbuf;
 struct nvc0_program *gp = nvc0->gmtyprog;

-   if (gp)
-  nvc0_program_validate(nvc0, gp);
+   if (gp) {
+  if (!nvc0_program_validate(nvc0, gp))
+ return;
+   }

 /* we allow GPs with no code for specifying stream output state only */
 if (gp && gp->code_size) {
--
2.6.4

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


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

Re: [Mesa-dev] [PATCH 4/5] nv50: check return value of nouveau_object_new()

[Ilia Mirkin]

On Mon, Dec 14, 2015 at 11:04 AM, Samuel Pitoiset
 wrote:
>
>
> On 12/14/2015 04:05 PM, Ilia Mirkin wrote:
>>
>> Bogus. obj will be null on failure.
>
>
> Well, checking the return value instead of obj seems better tbh, because we
> do that in the rest of the code.

Can ret = 0 but obj still be null? If not, feel free to flip it.

>
>
>>
>> On Mon, Dec 14, 2015 at 5:14 AM, Samuel Pitoiset
>>  wrote:
>>>
>>> Spotted by Coverity.
>>>
>>> Signed-off-by: Samuel Pitoiset 
>>> ---
>>>   src/gallium/drivers/nouveau/nv50/nv84_video.c | 8 
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/nouveau/nv50/nv84_video.c
>>> b/src/gallium/drivers/nouveau/nv50/nv84_video.c
>>> index 7a4670f..5e2489a 100644
>>> --- a/src/gallium/drivers/nouveau/nv50/nv84_video.c
>>> +++ b/src/gallium/drivers/nouveau/nv50/nv84_video.c
>>> @@ -756,8 +756,8 @@ firmware_present(struct pipe_screen *pscreen, enum
>>> pipe_video_format codec)
>>>  int present, ret;
>>>
>>>  if (!FIRMWARE_PRESENT(checked, VP_KERN)) {
>>> -  nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0, );
>>> -  if (obj)
>>> +  ret = nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0,
>>> );
>>> +  if (!ret && obj)
>>>screen->firmware_info.profiles_present |= FIRMWARE_VP_KERN;
>>> nouveau_object_del();
>>> screen->firmware_info.profiles_checked |= FIRMWARE_VP_KERN;
>>> @@ -765,8 +765,8 @@ firmware_present(struct pipe_screen *pscreen, enum
>>> pipe_video_format codec)
>>>
>>>  if (codec == PIPE_VIDEO_FORMAT_MPEG4_AVC) {
>>> if (!FIRMWARE_PRESENT(checked, BSP_KERN)) {
>>> - nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0, );
>>> - if (obj)
>>> + ret = nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0,
>>> );
>>> + if (!ret && obj)
>>>   screen->firmware_info.profiles_present |=
>>> FIRMWARE_BSP_KERN;
>>>nouveau_object_del();
>>>screen->firmware_info.profiles_checked |= FIRMWARE_BSP_KERN;
>>> --
>>> 2.6.4
>>>
>>> ___
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
> --
> -Samuel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/5] nouveau: remove logically dead code in nouveau_vpe_mb_mv_header()

[Pierre Moreau]

On 11:14 AM - Dec 14 2015, Samuel Pitoiset wrote:
> frame cannot be NULL in that branch. Spotted by Coverity.
> 
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/nouveau_video.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/src/gallium/drivers/nouveau/nouveau_video.c 
> b/src/gallium/drivers/nouveau/nouveau_video.c
> index 8bb12b2..fe19bce 100644
> --- a/src/gallium/drivers/nouveau/nouveau_video.c
> +++ b/src/gallium/drivers/nouveau/nouveau_video.c
> @@ -317,8 +317,6 @@ nouveau_vpe_mb_mv_header(struct nouveau_decoder *dec,
>case PIPE_MPEG12_MO_TYPE_16x8: goto mv2;
>case PIPE_MPEG12_MO_TYPE_DUAL_PRIME: {
>base = NV17_MPEG_CMD_CHROMA_MV_HEADER_MV_SPLIT_HALF_MB;
> - if (frame)
> -base |= NV17_MPEG_CMD_CHROMA_MV_HEADER_TYPE_FRAME;

If frame can't be NULL, shouldn't you only remove the `if` statement as you're
otherwise removing used code as well?


Pierre


>   if (forward)
>  nouveau_vpe_mb_mv(dec, base, luma, frame, true,
>dec->picture_structure != 
> PIPE_MPEG12_PICTURE_STRUCTURE_FIELD_TOP,
> -- 
> 2.6.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

[Mesa-dev] [PATCH] docs: Replace sourceforge logo with a text link

[Andreas Boll]

Fixes the following Lintian (Debian package checker) error:

privacy-breach-logo

  usr/share/doc/mesa-common-dev/contents.html
(http://sourceforge.net/sflogo.php?group_id=3type=1)
  usr/share/doc/mesa-common-dev/thanks.html
(http://sourceforge.net/sflogo.php?group_id=3type=1)

The extended description of this tag is:

This package creates a potential privacy breach by fetching a logo
at runtime.

Before using a local copy you should check that the logo is suitable
for main. You can get help with determining this by posting a link to
the logo and a copy of, or a link to, the logo copyright and license
information to the debian-legal mailing list.

Please replace any scripts, images, or other remote resources with
non-remote resources. It is preferable to replace them with text and
links but local copies of the remote resources are also acceptable as
long as they don't also make calls to remote services. Please ensure
that the remote resources are suitable for Debian main before making
local copies of them.

Severity: serious, Certainty: possible

Check: files, Type: binary, udeb

Signed-off-by: Andreas Boll 
---
 docs/contents.html | 3 +--
 docs/thanks.html   | 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/docs/contents.html b/docs/contents.html
index 6612cbe..5e0f514 100644
--- a/docs/contents.html
+++ b/docs/contents.html
@@ -96,8 +96,7 @@
 
 
 http://sourceforge.net;
-target="_parent">http://sourceforge.net/sflogo.php?group_id=3type=1;
-width="88" height="31" align="bottom" alt="Sourceforge.net" border="0">
+target="_parent">sourceforge.net
 
 
 
diff --git a/docs/thanks.html b/docs/thanks.html
index 29a41e4..685cb31 100644
--- a/docs/thanks.html
+++ b/docs/thanks.html
@@ -42,9 +42,7 @@ Tungsten Graphics, Inc. have supported the ongoing 
development of Mesa.
 The
 http://www.mesa3d.org;>Mesa
 website is hosted by
-http://sourceforge.net;>
-http://sourceforge.net/sflogo.php?group_id=3type=1;
-width="88" height="31" align="bottom" alt="Sourceforge.net" border="0">
+http://sourceforge.net;>sourceforge.net.
 
 
 
-- 
2.1.4

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

Re: [Mesa-dev] [PATCH 2/5] nv50, nvc0: fix potential resource leak in nvXX_create_texture_view()

[Samuel Pitoiset]

On 12/14/2015 04:01 PM, Ilia Mirkin wrote:

Again, bogus. Can't get there. I'd take a patch that asserts though,
or marks it unreachable, or just drops the default case entirely.


Make it unreachable is fine by me.



On Mon, Dec 14, 2015 at 5:14 AM, Samuel Pitoiset
 wrote:

Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
  src/gallium/drivers/nouveau/nv50/nv50_tex.c | 3 ++-
  src/gallium/drivers/nouveau/nvc0/nvc0_tex.c | 3 ++-
  2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_tex.c 
b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
index 6083ea9..9fb9dcf 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_tex.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
@@ -193,7 +193,8 @@ nv50_create_texture_view(struct pipe_context *pipe,
break;
 default:
NOUVEAU_ERR("invalid texture target: %d\n", mt->base.base.target);
-  return false;
+  FREE(view);
+  return NULL;
 }

 tic[3] = (flags & NV50_TEXVIEW_FILTER_MSAA8) ? 0x2000 : 0x0030;
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
index 2dd100f..2503ee1 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
@@ -195,7 +195,8 @@ nvc0_create_texture_view(struct pipe_context *pipe,
 default:
NOUVEAU_ERR("unexpected/invalid texture target: %d\n",
mt->base.base.target);
-  return false;
+  FREE(view);
+  return NULL;
 }

 tic[3] = (flags & NV50_TEXVIEW_FILTER_MSAA8) ? 0x2000 : 0x0030;
--
2.6.4

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


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

Re: [Mesa-dev] [PATCH 3/5] nvc0: make sure gmt programs are correctly validated

[Ilia Mirkin]

Instead take the return value and stick it into the if below.
Returning early from these functions isn't something we generally want
to do.

On Mon, Dec 14, 2015 at 5:14 AM, Samuel Pitoiset
 wrote:
> Spotted by Coverity.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> index 7e2e999..5e69e29 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> @@ -236,8 +236,10 @@ nvc0_gmtyprog_validate(struct nvc0_context *nvc0)
> struct nouveau_pushbuf *push = nvc0->base.pushbuf;
> struct nvc0_program *gp = nvc0->gmtyprog;
>
> -   if (gp)
> -  nvc0_program_validate(nvc0, gp);
> +   if (gp) {
> +  if (!nvc0_program_validate(nvc0, gp))
> + return;
> +   }
>
> /* we allow GPs with no code for specifying stream output state only */
> if (gp && gp->code_size) {
> --
> 2.6.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 5/5] draw: remove clip_vertex from vertex header

[Jose Fonseca]

On 12/12/15 02:31, srol...@vmware.com wrote:

From: Roland Scheidegger 

vertex header had both clip_pos and clip_vertex.
We only really need one (clip_pos) because the draw llvm shader would
overwrite the position output from the vs with the viewport transformed.
However, we don't really need the second one, which was only really used
for gl_ClipVertex - if the shader didn't have that the values were just
duplicated to both clip_pos and clip_vertex. So, just use this from the vs
output instead when we actually need it.
Also change clip debug to output both the data from clip_pos and the
clipVertex output (if available).
Makes some things more complex, some things less complex, but seems more
easy to understand what clipping actually does (and what values it uses
to do its magic).
---
  src/gallium/auxiliary/draw/draw_cliptest_tmp.h |  1 -
  src/gallium/auxiliary/draw/draw_llvm.c | 21 ++--
  src/gallium/auxiliary/draw/draw_llvm.h |  4 --
  src/gallium/auxiliary/draw/draw_pipe_clip.c| 67 +++---
  src/gallium/auxiliary/draw/draw_private.h  |  1 -
  5 files changed, 54 insertions(+), 40 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h 
b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
index 06c82c3..c57e5f4 100644
--- a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
+++ b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
@@ -91,7 +91,6 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs,
   }

   for (i = 0; i < 4; i++) {
-out->clip_vertex[i] = clipvertex[i];
  out->clip_pos[i] = position[i];
   }

diff --git a/src/gallium/auxiliary/draw/draw_llvm.c 
b/src/gallium/auxiliary/draw/draw_llvm.c
index 934f452..a966e45 100644
--- a/src/gallium/auxiliary/draw/draw_llvm.c
+++ b/src/gallium/auxiliary/draw/draw_llvm.c
@@ -378,14 +378,13 @@ static LLVMTypeRef
  create_jit_vertex_header(struct gallivm_state *gallivm, int data_elems)
  {
 LLVMTargetDataRef target = gallivm->target;
-   LLVMTypeRef elem_types[4];
+   LLVMTypeRef elem_types[3];
 LLVMTypeRef vertex_header;
 char struct_name[24];

 util_snprintf(struct_name, 23, "vertex_header%d", data_elems);

 elem_types[DRAW_JIT_VERTEX_VERTEX_ID]  = 
LLVMIntTypeInContext(gallivm->context, 32);
-   elem_types[DRAW_JIT_VERTEX_CLIP_VERTEX]  = 
LLVMArrayType(LLVMFloatTypeInContext(gallivm->context), 4);
 elem_types[DRAW_JIT_VERTEX_CLIP_POS]  = 
LLVMArrayType(LLVMFloatTypeInContext(gallivm->context), 4);
 elem_types[DRAW_JIT_VERTEX_DATA]  = LLVMArrayType(elem_types[1], 
data_elems);

@@ -407,9 +406,6 @@ create_jit_vertex_header(struct gallivm_state *gallivm, int 
data_elems)
DRAW_JIT_VERTEX_VERTEX_ID);
 */
 (void) target; /* silence unused var warning for non-debug build */
-   LP_CHECK_MEMBER_OFFSET(struct vertex_header, clip_vertex,
-  target, vertex_header,
-  DRAW_JIT_VERTEX_CLIP_VERTEX);
 LP_CHECK_MEMBER_OFFSET(struct vertex_header, clip_pos,
target, vertex_header,
DRAW_JIT_VERTEX_CLIP_POS);
@@ -1019,7 +1015,7 @@ store_clip(struct gallivm_state *gallivm,
 const struct lp_type vs_type,
 LLVMValueRef io_ptr,
 LLVMValueRef (*outputs)[TGSI_NUM_CHANNELS],
-   boolean clip_vertex, int idx)
+   int idx)
  {
 LLVMBuilderRef builder = gallivm->builder;
 LLVMValueRef soa[4];
@@ -1046,14 +1042,8 @@ store_clip(struct gallivm_state *gallivm,
 soa[2] = LLVMBuildLoad(builder, outputs[idx][2], ""); /*z0 z1 .. zn*/
 soa[3] = LLVMBuildLoad(builder, outputs[idx][3], ""); /*w0 w1 .. wn*/

-   if (clip_vertex) {
-  for (i = 0; i < vs_type.length; i++) {
- clip_ptrs[i] = draw_jit_header_clip_vertex(gallivm, io_ptrs[i]);
-  }
-   } else {
-  for (i = 0; i < vs_type.length; i++) {
- clip_ptrs[i] = draw_jit_header_clip_pos(gallivm, io_ptrs[i]);
-  }
+   for (i = 0; i < vs_type.length; i++) {
+  clip_ptrs[i] = draw_jit_header_clip_pos(gallivm, io_ptrs[i]);
 }

 lp_build_transpose_aos(gallivm, vs_type, soa, soa);
@@ -1771,8 +1761,7 @@ draw_llvm_generate(struct draw_llvm *llvm, struct 
draw_llvm_variant *variant,

if (pos != -1 && cv != -1) {
   /* store original positions in clip before further manipulation */
- store_clip(gallivm, vs_type, io, outputs, TRUE, key->clip_user ? cv : 
pos);
- store_clip(gallivm, vs_type, io, outputs, FALSE, pos);
+ store_clip(gallivm, vs_type, io, outputs, pos);

   /* do cliptest */
   if (enable_cliptest) {
diff --git a/src/gallium/auxiliary/draw/draw_llvm.h 
b/src/gallium/auxiliary/draw/draw_llvm.h
index c40df1c..f617a29 100644
--- a/src/gallium/auxiliary/draw/draw_llvm.h
+++ b/src/gallium/auxiliary/draw/draw_llvm.h
@@ -104,7 +104,6 @@ enum {

  enum {
 DRAW_JIT_VERTEX_VERTEX_ID = 0,
-   

Re: [Mesa-dev] [PATCH 5/5] draw: remove clip_vertex from vertex header

[Brian Paul]

The series looks good to me.  Thanks for digging into this.

Reviewed-by: Brian Paul 



On 12/11/2015 07:31 PM, srol...@vmware.com wrote:

From: Roland Scheidegger 

vertex header had both clip_pos and clip_vertex.
We only really need one (clip_pos) because the draw llvm shader would
overwrite the position output from the vs with the viewport transformed.
However, we don't really need the second one, which was only really used
for gl_ClipVertex - if the shader didn't have that the values were just
duplicated to both clip_pos and clip_vertex. So, just use this from the vs
output instead when we actually need it.
Also change clip debug to output both the data from clip_pos and the
clipVertex output (if available).
Makes some things more complex, some things less complex, but seems more
easy to understand what clipping actually does (and what values it uses
to do its magic).
---
  src/gallium/auxiliary/draw/draw_cliptest_tmp.h |  1 -
  src/gallium/auxiliary/draw/draw_llvm.c | 21 ++--
  src/gallium/auxiliary/draw/draw_llvm.h |  4 --
  src/gallium/auxiliary/draw/draw_pipe_clip.c| 67 +++---
  src/gallium/auxiliary/draw/draw_private.h  |  1 -
  5 files changed, 54 insertions(+), 40 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h 
b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
index 06c82c3..c57e5f4 100644
--- a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
+++ b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
@@ -91,7 +91,6 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs,
   }

   for (i = 0; i < 4; i++) {
-out->clip_vertex[i] = clipvertex[i];
  out->clip_pos[i] = position[i];
   }

diff --git a/src/gallium/auxiliary/draw/draw_llvm.c 
b/src/gallium/auxiliary/draw/draw_llvm.c
index 934f452..a966e45 100644
--- a/src/gallium/auxiliary/draw/draw_llvm.c
+++ b/src/gallium/auxiliary/draw/draw_llvm.c
@@ -378,14 +378,13 @@ static LLVMTypeRef
  create_jit_vertex_header(struct gallivm_state *gallivm, int data_elems)
  {
 LLVMTargetDataRef target = gallivm->target;
-   LLVMTypeRef elem_types[4];
+   LLVMTypeRef elem_types[3];
 LLVMTypeRef vertex_header;
 char struct_name[24];

 util_snprintf(struct_name, 23, "vertex_header%d", data_elems);

 elem_types[DRAW_JIT_VERTEX_VERTEX_ID]  = 
LLVMIntTypeInContext(gallivm->context, 32);
-   elem_types[DRAW_JIT_VERTEX_CLIP_VERTEX]  = 
LLVMArrayType(LLVMFloatTypeInContext(gallivm->context), 4);
 elem_types[DRAW_JIT_VERTEX_CLIP_POS]  = 
LLVMArrayType(LLVMFloatTypeInContext(gallivm->context), 4);
 elem_types[DRAW_JIT_VERTEX_DATA]  = LLVMArrayType(elem_types[1], 
data_elems);

@@ -407,9 +406,6 @@ create_jit_vertex_header(struct gallivm_state *gallivm, int 
data_elems)
DRAW_JIT_VERTEX_VERTEX_ID);
 */
 (void) target; /* silence unused var warning for non-debug build */
-   LP_CHECK_MEMBER_OFFSET(struct vertex_header, clip_vertex,
-  target, vertex_header,
-  DRAW_JIT_VERTEX_CLIP_VERTEX);
 LP_CHECK_MEMBER_OFFSET(struct vertex_header, clip_pos,
target, vertex_header,
DRAW_JIT_VERTEX_CLIP_POS);
@@ -1019,7 +1015,7 @@ store_clip(struct gallivm_state *gallivm,
 const struct lp_type vs_type,
 LLVMValueRef io_ptr,
 LLVMValueRef (*outputs)[TGSI_NUM_CHANNELS],
-   boolean clip_vertex, int idx)
+   int idx)
  {
 LLVMBuilderRef builder = gallivm->builder;
 LLVMValueRef soa[4];
@@ -1046,14 +1042,8 @@ store_clip(struct gallivm_state *gallivm,
 soa[2] = LLVMBuildLoad(builder, outputs[idx][2], ""); /*z0 z1 .. zn*/
 soa[3] = LLVMBuildLoad(builder, outputs[idx][3], ""); /*w0 w1 .. wn*/

-   if (clip_vertex) {
-  for (i = 0; i < vs_type.length; i++) {
- clip_ptrs[i] = draw_jit_header_clip_vertex(gallivm, io_ptrs[i]);
-  }
-   } else {
-  for (i = 0; i < vs_type.length; i++) {
- clip_ptrs[i] = draw_jit_header_clip_pos(gallivm, io_ptrs[i]);
-  }
+   for (i = 0; i < vs_type.length; i++) {
+  clip_ptrs[i] = draw_jit_header_clip_pos(gallivm, io_ptrs[i]);
 }

 lp_build_transpose_aos(gallivm, vs_type, soa, soa);
@@ -1771,8 +1761,7 @@ draw_llvm_generate(struct draw_llvm *llvm, struct 
draw_llvm_variant *variant,

if (pos != -1 && cv != -1) {
   /* store original positions in clip before further manipulation */
- store_clip(gallivm, vs_type, io, outputs, TRUE, key->clip_user ? cv : 
pos);
- store_clip(gallivm, vs_type, io, outputs, FALSE, pos);
+ store_clip(gallivm, vs_type, io, outputs, pos);

   /* do cliptest */
   if (enable_cliptest) {
diff --git a/src/gallium/auxiliary/draw/draw_llvm.h 
b/src/gallium/auxiliary/draw/draw_llvm.h
index c40df1c..f617a29 100644
--- a/src/gallium/auxiliary/draw/draw_llvm.h
+++ 

Re: [Mesa-dev] [PATCH 5/5] nouveau: remove logically dead code in nouveau_vpe_mb_mv_header()

[Samuel Pitoiset]

On 12/14/2015 04:48 PM, Pierre Moreau wrote:

On 11:14 AM - Dec 14 2015, Samuel Pitoiset wrote:

frame cannot be NULL in that branch. Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
  src/gallium/drivers/nouveau/nouveau_video.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nouveau_video.c 
b/src/gallium/drivers/nouveau/nouveau_video.c
index 8bb12b2..fe19bce 100644
--- a/src/gallium/drivers/nouveau/nouveau_video.c
+++ b/src/gallium/drivers/nouveau/nouveau_video.c
@@ -317,8 +317,6 @@ nouveau_vpe_mb_mv_header(struct nouveau_decoder *dec,
case PIPE_MPEG12_MO_TYPE_16x8: goto mv2;
case PIPE_MPEG12_MO_TYPE_DUAL_PRIME: {
base = NV17_MPEG_CMD_CHROMA_MV_HEADER_MV_SPLIT_HALF_MB;
- if (frame)
-base |= NV17_MPEG_CMD_CHROMA_MV_HEADER_TYPE_FRAME;


If frame can't be NULL, shouldn't you only remove the `if` statement as you're
otherwise removing used code as well?


Wrong commit message... frame is *always* NULL in that branch...




Pierre



   if (forward)
  nouveau_vpe_mb_mv(dec, base, luma, frame, true,
dec->picture_structure != 
PIPE_MPEG12_PICTURE_STRUCTURE_FIELD_TOP,
--
2.6.4

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


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

Re: [Mesa-dev] [PATCH 1/8] nir: Silence missing field initializer warnings for nir_src

[Jason Ekstrand]

On Mon, Dec 14, 2015 at 5:12 PM, Ian Romanick  wrote:
> On 12/14/2015 04:39 PM, Ilia Mirkin wrote:
>> On Mon, Dec 14, 2015 at 7:28 PM, Ian Romanick  wrote:
>>> On 12/14/2015 03:38 PM, Ilia Mirkin wrote:
 It's a pretty standard feature of compilers to init things to 0 and
 not have the full structure specified like that... what compiler are
 you seeing these with? Can we just fix the glitch with a
 -Wno-stupid-warnings?
>>>
>>> I have observed this with several versions of GCC.
>>>
>>> In C, you can avoid this with a trailing comma like:
>>>
>>> #define NIR_SRC_INIT (nir_src) { { NULL }, }
>>>
>>> However, nir.h is also used in some C++ code where that doesn't help.
>>>
>>> To be honest, I'm not a big fan of these macros.  Without C99 designated
>>> initalizers, maintaining initializers like these (or the ones in
>>> src/glsl/builtin_variables.cpp) is a real pain.  We can't use those, and
>>> we can't use C++ constructors.  We have no good options available. :(
>>>
>>> I thought about replacing them with a static inline function that
>>> returns a zero-initialized struct.  The compiler should generate the
>>> same code.  However, that doesn't work with uses like those in patch 3.
>>>
>>> I'm also a little curious why you didn't raise this issue when I sent
>>> these patches out in August.  I removed the patch from the series that
>>> you objected to back then.
>>
>> I have absolutely no recollection of any of that. Perhaps I saw "nir"
>> and thought to myself, "don't care, let them do whatever, this won't
>> ever affect me". Which is a sentiment I'm happy to continue with, by
>> the way.
>
> Fair enough. :)  The patch I removed was one that removed the gl_context
> parameter from a function in dd_function_table.
>
> http://patchwork.freedesktop.org/patch/58048/
>
>> I know that doing
>>
>> x = {}
>>
>> is a gcc extension, but I thought that {0} should always work (with
>> enough {} nesting in case the first element is a struct). Perhaps it
>
> {0} is, basically what we're doing now, and GCC complains about it with
> -Wmissing-field-initializers or -Wextra.  When we added C-style struct

I'm not a big fan of spending time fixing warnings that you have to
add -Wextra to get.  However, if there are C++ issues, then those
definitely need to get fixed.

> and array initializers to GLSL, we discussed adding this sort of
> implicit zero initialization.  I did some digging in the C89 and C99
> specs, and I have some recollection that in this case the missing fields
> get undefined values... but, starting with C99, {0, } implicitly
> initializes the missing fields to zero.  I also seem to recall that bit
> of weirdness in C is why quite a few people were opposed to adding it to
> GLSL.  This was several years ago, so my memory may not be completely
> reliable.
>
>> doesn't in C++? I could believe that, although I'd be surprised.
>
> The initializer support in C++ intentionally quite a bit more primitive
> than in C99.  The language designers want you to use constructors
> whether it's the best tool for the job or not... which is why there are
> no designated initializers.

So, I've got a patch somewhere that switches based on __cplusplus and
defines NIR_SRC_INIT as either the C99 thing or nir_src() for C++.
Would that solve this problem?  There was also a bug recently about us
not building with oricle studio that it would probably fix.  If so,
let's do that rather than a gigantic mess of braces and zeros.
--Jason

>> Anyways, didn't mean to stir the pot too much, just thought there
>> might be a simpler way out of all this.
>
> Well, there are. :) We just can't use them due to some combination of
> MSVC, C++, and C99.
>
>> Cheers,
>>
>>   -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

Re: [Mesa-dev] [PATCH v5 12/70] glsl: implement unsized array length

[Samuel Iglesias Gonsálvez]

On Mon, 2015-12-14 at 08:00 +0100, Iago Toral wrote:
> On Sun, 2015-12-13 at 23:10 -0500, Ilia Mirkin wrote:
> > On Thu, Sep 10, 2015 at 9:35 AM, Iago Toral Quiroga  > com> wrote:
> > > +ir_expression *
> > > +lower_ubo_reference_visitor::process_ssbo_unsized_array_length(i
> > > r_rvalue **rvalue,
> > > +   i
> > > r_dereference *deref,
> > > +   i
> > > r_variable *var)
> > > +{
> > > +   mem_ctx = ralloc_parent(*rvalue);
> > > +
> > > +   ir_rvalue *base_offset = NULL;
> > > +   unsigned const_offset;
> > > +   bool row_major;
> > > +   int matrix_columns;
> > > +   int unsized_array_stride =
> > > calculate_unsized_array_stride(deref);
> > > +
> > > +   /* Compute the offset to the start if the dereference as well
> > > as other
> > > +* information we need to calculate the length.
> > > +*/
> > > +   setup_for_load_or_store(var, deref,
> > > +   _offset, _offset,
> > > +   _major, _columns);
> > > +   /* array.length() =
> > > +*  max((buffer_object_size - offset_of_array) /
> > > stride_of_array, 0)
> > > +*/
> > > +   ir_expression *buffer_size = emit_ssbo_get_buffer_size();
> > > +
> > > +   ir_expression *offset_of_array = new(mem_ctx)
> > > +  ir_expression(ir_binop_add, base_offset,
> > > +new(mem_ctx) ir_constant(const_offset));
> > > +   ir_expression *offset_of_array_int = new(mem_ctx)
> > > +  ir_expression(ir_unop_u2i, offset_of_array);
> > > +
> > > +   ir_expression *sub = new(mem_ctx)
> > > +  ir_expression(ir_binop_sub, buffer_size,
> > > offset_of_array_int);
> > > +   ir_expression *div =  new(mem_ctx)
> > > +  ir_expression(ir_binop_div, sub,
> > > +new(mem_ctx)
> > > ir_constant(unsized_array_stride));
> > > +   ir_expression *max = new(mem_ctx)
> > > +  ir_expression(ir_binop_max, div, new(mem_ctx)
> > > ir_constant(0));
> > > +
> > > +   return max;
> > > +}
> > 
> > Hi Iago,
> > 
> > I noticed that this comes out as a signed division. Is there any
> > way
> > to make it into an unsigned division? That way we can e.g. optimize
> > a
> > power-of-two division into a shift, and it's a few instructions
> > fewer
> > to emulate when there's no built-in integer division instruction
> > (which I think is most GPUs). It seems that you went to some
> > trouble
> > to do all this with signed integers, but I can't quite figure out
> > why.
> 
> Hi Ilia,
> 
> I agree, I don't see why we would do the extra work to make this
> signed... Samuel wrote this code though, so I'll let him confirm.
> 
> Iago
> 
> 

The formula is:

array.length() =
    max((buffer_object_size - offset_of_array) / stride_of_array, 0)

I did the signed division because the buffer size could be smaller than
the offset of the unsized array (i.e., the application setup it
wrongly), hence the result of the subtraction would be negative. Then,
after the signed division, max() would return 0 in that case.

If you want to use an unsigned division, it would be something like:

* Execute max((buffer_object_size - offset_of_array), 0).
* If the subtraction result is a positive value, max() would return a
non-zero value. We can detect that case (with a CMP for example),
convert the value to unsigned, do the unsigned division and return its
result.
* If the subtraction result is a negative or zero, max() would return
0, so we can detect that case (it is actually the other branch of the
previous CMP) and return it.

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

[Mesa-dev] [PATCH 3/3] nir/lower_system_values: Refactor and use the builder.

[Jason Ekstrand]

Now that we have a helper in the builder for system values and a helper in
core NIR to get the intrinsic opcode, there's really no point in having
things split out into a helper function.  This commit "modernizes" this
pass to use helpers better and look more like newer passes.
---
 src/glsl/nir/nir_lower_system_values.c | 60 +-
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/src/glsl/nir/nir_lower_system_values.c 
b/src/glsl/nir/nir_lower_system_values.c
index 03a9814..0f91478 100644
--- a/src/glsl/nir/nir_lower_system_values.c
+++ b/src/glsl/nir/nir_lower_system_values.c
@@ -26,44 +26,41 @@
  */
 
 #include "nir.h"
-#include "main/mtypes.h"
+#include "nir_builder.h"
+
+struct lower_system_values_state {
+   nir_builder b;
+   bool progress;
+};
 
 static bool
-convert_instr(nir_intrinsic_instr *instr)
+convert_block(nir_block *block, void *void_state)
 {
-   if (instr->intrinsic != nir_intrinsic_load_var)
-  return false;
+   struct lower_system_values_state *state = void_state;
 
-   nir_variable *var = instr->variables[0]->var;
-   if (var->data.mode != nir_var_system_value)
-  return false;
+   nir_foreach_instr_safe(block, instr) {
+  if (instr->type != nir_instr_type_intrinsic)
+ continue;
 
-   void *mem_ctx = ralloc_parent(instr);
+  nir_intrinsic_instr *load_var = nir_instr_as_intrinsic(instr);
 
-   assert(instr->dest.is_ssa);
+  if (load_var->intrinsic != nir_intrinsic_load_var)
+ continue;
 
-   nir_intrinsic_op op = nir_intrinsic_from_system_value(var->data.location);
-   nir_intrinsic_instr *new_instr = nir_intrinsic_instr_create(mem_ctx, op);
+  nir_variable *var = load_var->variables[0]->var;
+  if (var->data.mode != nir_var_system_value)
+ continue;
 
-   nir_ssa_dest_init(_instr->instr, _instr->dest,
- instr->dest.ssa.num_components, NULL);
-   nir_ssa_def_rewrite_uses(>dest.ssa,
-nir_src_for_ssa(_instr->dest.ssa));
+  state->b.cursor = nir_after_instr(_var->instr);
 
-   nir_instr_insert_before(>instr, _instr->instr);
-   nir_instr_remove(>instr);
+  nir_intrinsic_op sysval_op =
+ nir_intrinsic_from_system_value(var->data.location);
+  nir_ssa_def *sysval = nir_load_system_value(>b, sysval_op, 0);
 
-   return true;
-}
+  nir_ssa_def_rewrite_uses(_var->dest.ssa, nir_src_for_ssa(sysval));
+  nir_instr_remove(_var->instr);
 
-static bool
-convert_block(nir_block *block, void *state)
-{
-   bool *progress = state;
-
-   nir_foreach_instr_safe(block, instr) {
-  if (instr->type == nir_instr_type_intrinsic)
- *progress = convert_instr(nir_instr_as_intrinsic(instr)) || *progress;
+  state->progress = true;
}
 
return true;
@@ -72,12 +69,15 @@ convert_block(nir_block *block, void *state)
 static bool
 convert_impl(nir_function_impl *impl)
 {
-   bool progress = false;
+   struct lower_system_values_state state;
 
-   nir_foreach_block(impl, convert_block, );
+   state.progress = false;
+   nir_builder_init(, impl);
+
+   nir_foreach_block(impl, convert_block, );
nir_metadata_preserve(impl, nir_metadata_block_index |
nir_metadata_dominance);
-   return progress;
+   return state.progress;
 }
 
 bool
-- 
2.5.0.400.gff86faf

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

Re: [Mesa-dev] SSBO + access qualifiers

[Jason Ekstrand]

On Mon, Dec 14, 2015 at 4:10 PM, Ilia Mirkin  wrote:
> Hello,
>
> I was going to add support for the various volatile/etc annotations in
> my gallium impl (which is fairly far along), but it seems like those
> are currently being dropped on the floor by lower_ubo_references, and
> NIR has no support for them either. Both in GLSL IR and NIR, the
> variable is what holds the access annotation (volatile, etc). However
> the ssbo intrinsics are all about a particular binding and offset,
> without any reference to the variable.
>
> What's the right way of handling this? (And do any tests exist that
> would be sensitive to getting such things wrong?)

First off, why is it that way?  Well, because most of the IRs in mesa
don't have a memory model capable of handling these sorts of things.
Those that do (LLVM is the only one I'm aware of) can't handle the GL
packing rules.  The result is that we basically have to lower
everything away to byte-offset load/store intrinsics.

What do we do about it?  My inclination would be to either add two new
intrinsics for load/store_ssbo_volatile or to add a new constant
boolean "volatile" parameter to the current intrinsics.  If a
load/store happens on a volatile things, you get the volatile version,
otherwise you get the plane version.  Then backends can know that they
are free to reorder, combine, etc. non-volatile load/store operations
as per their memory model and the provided barriers.  If they
encounter a volatile load/store, they can't do anything with it and
just have to do the memory op.

Is that reasonable?
--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] glsl: fix overlapping of varying locations for arrays and structs

[Timothy Arceri]

Previously we were only reserving a single location for arrays and
structs.

We also didn't take into account implicit locations clashing with
explicit locations when assigning locations for their arrays or
structs.

This patch fixes both issues.

V2: also fix for arrays of arrays and structs.

Cc: Kenneth Graunke 
Cc: 11.1 
---
 src/glsl/link_varyings.cpp | 80 +-
 1 file changed, 72 insertions(+), 8 deletions(-)

diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index 71750d1..5a3f53d 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -766,7 +766,8 @@ public:
gl_shader_stage consumer_stage);
~varying_matches();
void record(ir_variable *producer_var, ir_variable *consumer_var);
-   unsigned assign_locations(uint64_t reserved_slots, bool separate_shader);
+   unsigned assign_locations(struct gl_shader_program *prog,
+ uint64_t reserved_slots, bool separate_shader);
void store_locations() const;
 
 private:
@@ -988,7 +989,9 @@ varying_matches::record(ir_variable *producer_var, 
ir_variable *consumer_var)
  * passed to varying_matches::record().
  */
 unsigned
-varying_matches::assign_locations(uint64_t reserved_slots, bool 
separate_shader)
+varying_matches::assign_locations(struct gl_shader_program *prog,
+  uint64_t reserved_slots,
+  bool separate_shader)
 {
/* We disable varying sorting for separate shader programs for the
 * following reasons:
@@ -1040,9 +1043,57 @@ varying_matches::assign_locations(uint64_t 
reserved_slots, bool separate_shader)
   != this->matches[i].packing_class) {
  *location = ALIGN(*location, 4);
   }
-  while ((*location < MAX_VARYING * 4u) &&
-(reserved_slots & (1u << *location / 4u))) {
- *location = ALIGN(*location + 1, 4);
+
+  const ir_variable *var =
+ matches[i].consumer_var ? matches[i].consumer_var :
+matches[i].producer_var;
+
+  unsigned num_elements;
+  if (var->type->is_array()) {
+ num_elements = var->type->arrays_of_arrays_size();
+  } else if (var->type->is_record()) {
+ num_elements = var->type->record_location_offset(var->type->length);
+  } else {
+ num_elements = 1;
+  }
+
+  unsigned slot_end = this->disable_varying_packing ? 4 :
+ var->type->without_array()->vector_elements;
+  slot_end += *location - 1;
+
+  /* FIXME: We could be smarter in the below code and loop back over
+   * trying to fill any locations that we skipped because we couldn't pack
+   * the varying between an explicit location. For now just let the user
+   * hit the linking error if we run out of room and suggest they use
+   * explicit locations.
+   */
+  for (unsigned j = 0; j < num_elements; j++) {
+ while ((slot_end < MAX_VARYING * 4u) &&
+((reserved_slots & (1u << *location / 4u) ||
+ (reserved_slots & (1u << slot_end / 4u) {
+
+*location = ALIGN(*location + 1, 4);
+slot_end = *location;
+
+/* reset the counter and try again */
+j = 0;
+ }
+
+ /* Increase the slot to make sure there is enough room for next
+  * array element.
+  */
+ if (this->disable_varying_packing)
+slot_end += 4;
+ else
+slot_end += var->type->without_array()->vector_elements;
+  }
+
+  if (*location >= MAX_VARYING * 4u) {
+ linker_error(prog, "insufficient contiguous locations available for "
+  "%s it is possible an array or struct could not be "
+  "packed between varyings with explicit locations. Try "
+  "using an explicit location for arrays and structs.",
+  var->name);
   }
 
   this->matches[i].generic_location = *location;
@@ -1430,8 +1481,21 @@ reserved_varying_slot(struct gl_shader *stage, 
ir_variable_mode io_mode)
  continue;
 
   var_slot = var->data.location - VARYING_SLOT_VAR0;
-  if (var_slot >= 0 && var_slot < MAX_VARYING)
- slots |= 1u << var_slot;
+
+  unsigned num_elements;
+  if (var->type->is_array()) {
+ num_elements = var->type->arrays_of_arrays_size();
+  } else if (var->type->is_record()) {
+ num_elements = var->type->record_location_offset(var->type->length);
+  } else {
+ num_elements = 1;
+  }
+
+  for (unsigned i = 0; i < num_elements; i++) {
+ if (var_slot >= 0 && var_slot < MAX_VARYING)
+slots |= 1u << var_slot;
+ var_slot += 1;
+  }
}
 
return slots;
@@ -1617,7 +1681,7 @@ assign_varying_locations(struct gl_context *ctx,
   reserved_varying_slot(producer, ir_var_shader_out) 

[Mesa-dev] [PATCH 2/2] glsl: don't try adding build-ins to explicit locations bitmask

[Timothy Arceri]

Cc: Kenneth Graunke 
Cc: 11.1 
---
 src/glsl/link_varyings.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index 5a3f53d..4944d19 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -1477,7 +1477,9 @@ reserved_varying_slot(struct gl_shader *stage, 
ir_variable_mode io_mode)
foreach_in_list(ir_instruction, node, stage->ir) {
   ir_variable *const var = node->as_variable();
 
-  if (var == NULL || var->data.mode != io_mode || 
!var->data.explicit_location)
+  if (var == NULL || var->data.mode != io_mode ||
+  !var->data.explicit_location ||
+  var->data.location < VARYING_SLOT_VAR0)
  continue;
 
   var_slot = var->data.location - VARYING_SLOT_VAR0;
-- 
2.4.3

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

Re: [Mesa-dev] SSBO + access qualifiers

[Ilia Mirkin]

On Mon, Dec 14, 2015 at 10:50 PM, Jason Ekstrand  wrote:
> On Mon, Dec 14, 2015 at 4:10 PM, Ilia Mirkin  wrote:
>> Hello,
>>
>> I was going to add support for the various volatile/etc annotations in
>> my gallium impl (which is fairly far along), but it seems like those
>> are currently being dropped on the floor by lower_ubo_references, and
>> NIR has no support for them either. Both in GLSL IR and NIR, the
>> variable is what holds the access annotation (volatile, etc). However
>> the ssbo intrinsics are all about a particular binding and offset,
>> without any reference to the variable.
>>
>> What's the right way of handling this? (And do any tests exist that
>> would be sensitive to getting such things wrong?)
>
> First off, why is it that way?  Well, because most of the IRs in mesa
> don't have a memory model capable of handling these sorts of things.
> Those that do (LLVM is the only one I'm aware of) can't handle the GL
> packing rules.  The result is that we basically have to lower
> everything away to byte-offset load/store intrinsics.
>
> What do we do about it?  My inclination would be to either add two new
> intrinsics for load/store_ssbo_volatile or to add a new constant
> boolean "volatile" parameter to the current intrinsics.  If a
> load/store happens on a volatile things, you get the volatile version,
> otherwise you get the plane version.  Then backends can know that they
> are free to reorder, combine, etc. non-volatile load/store operations
> as per their memory model and the provided barriers.  If they
> encounter a volatile load/store, they can't do anything with it and
> just have to do the memory op.
>
> Is that reasonable?

Well, I don't really care much about the NIR-side of it, but I think
at the very least coherent and volatile need to be exposed. I don't
much care about restrict, that's a bit too fancy for me, but no harm
in passing it through. TBH I don't get the point of
readonly/writeonly. FWIW this is what the nvidia hw is capable of:
http://docs.nvidia.com/cuda/parallel-thread-execution/#cache-operators
(I know it's PTX, but the actual hw is basically the same.)

I think that volatile = cv, coherent = cg, and everything else = ca/cs
based on some clever heuristic (aka I'll always pick one of those
until a situation where that doesn't work well arises).

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

Re: [Mesa-dev] [PATCH v2] mesa: fix interface matching done in validate_io

[Tapani Pälli]

On 12/15/2015 03:31 AM, Timothy Arceri wrote:

On Mon, 2015-12-14 at 10:29 +0200, Tapani Pälli wrote:

Patch makes following changes for interface matching:

- do not try to match builtin variables
- handle swizzle in input name, as example 'a.z' should
  match with 'a'
- check that amount of inputs and outputs matches

These changes make interface matching tests to work in:
ES31-CTS.sepshaderobjs.StateInteraction

Test does not still pass completely due to errors in rendering
output. IMO this is unrelated to interface matching.

v2: add spec reference, return true on desktop since we do not
 have failing cases for it, inputs and outputs amount do not
 need to match on desktop.

Signed-off-by: Tapani Pälli 

Hi Tapani,

Just a general comment first.

I think we should first move _mesa_validate_pipeline_io() and
  validate_io() to src/mesa/main/pipelineobj.c I don't think it belongs
here right?


Sure, it can be done now. Original intention was to use program 
resources and that is why it ended up being here.





---
  src/mesa/main/shader_query.cpp | 54
++
  1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/src/mesa/main/shader_query.cpp
b/src/mesa/main/shader_query.cpp
index ced10a9..bc01b97 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -1377,19 +1377,38 @@ validate_io(const struct gl_shader
*input_stage,
  const struct gl_shader *output_stage, bool isES)
  {
 assert(input_stage && output_stage);
+   unsigned inputs = 0, outputs = 0;
+
+   /* Currently no matching done for desktop. */

I think the spec quote should be moved here as it applies to all the
rules in the function then you can also have the comment explaining why
validation for desktop it not done.


OK


I've also filed a spec bug for desktop for the reasons I outlined in
irc previously. It would be great if you could quote the bug here also.
Something like:

/* FIXME: Update once Khronos spec bug #15331 is resolved. */


Sure, will add.


+   if (!isES)
+  return true;
  
 /* For each output in a, find input in b and do any required

checks. */
 foreach_in_list(ir_instruction, out, input_stage->ir) {
ir_variable *out_var = out->as_variable();


It's existing code but it would also be nice to have a patch that
renames input_stage/output_stage to producer_stage/consumer_stage this
it what they are called in the linker code. Maybe its just me but
getting the outputs from input_stage just looks wrong.


OK, can change this.




-  if (!out_var || out_var->data.mode != ir_var_shader_out)
+  if (!out_var || out_var->data.mode != ir_var_shader_out ||
+  is_gl_identifier(out_var->name))
   continue;
  
+  outputs++;

+
+  inputs = 0;
foreach_in_list(ir_instruction, in, output_stage->ir) {

Two comments here:

1. Take a look at cross_validate_outputs_to_inputs() in
link_varyings.cpp for a way to avoid the nested loop? Although it may
cause even more overhaed using the symbol table not sure.


I don't know if symbol table can be trusted as variables that get 
optimized away or changed in some way are still there. Only way to be 
sure is to iterate IR or use resource list. Also, symbol table gets 
destroyed after linking. My first implementation was using a hash but 
that was also bad idea because variables names do not necessarily match 
exactly.



2. Take a look at the same function for matching via explicit location.
Does the CTS not test for mismatched explicit locations? Maybe we
should create a piglit test for this as your existing code doesn't take
into account explicit locations.


No, I haven't seen this test using explicit locations. This patch also 
makes the interface matching pass.



I was going to suggest sharing the code between here and the linker
however I'm about to add a bunch of rules for matching the component
qualifier for enhanced layouts so not entirely sure if we should do
this what do you think?


Linker will need to do much more so maybe do separately, at least for now?


   ir_variable *in_var = in->as_variable();
- if (!in_var || in_var->data.mode != ir_var_shader_in)
+ if (!in_var || in_var->data.mode != ir_var_shader_in ||
+ is_gl_identifier(in_var->name))
  continue;
  
- if (strcmp(in_var->name, out_var->name) == 0) {

+ inputs++;
+
+ unsigned len = strlen(in_var->name);
+
+ /* Handle input swizzle in variable name. */
+ const char *dot = strchr(in_var->name, '.');
+ if (dot)
+len = dot - in_var->name;

Hmm ... I wonder if this is also missing from the linker or maybe the
symbol table stuff handles this.


Variable names get mangled during optimizations so symbol table should 
have the correct names left during linking.



+
+ if (strncmp(in_var->name, out_var->name, len) == 0) {

Re: [Mesa-dev] SSBO + access qualifiers

[Ilia Mirkin]

On Mon, Dec 14, 2015 at 11:44 PM, Jason Ekstrand  wrote:
> On Mon, Dec 14, 2015 at 8:03 PM, Ilia Mirkin  wrote:
>> On Mon, Dec 14, 2015 at 10:50 PM, Jason Ekstrand  
>> wrote:
>>> On Mon, Dec 14, 2015 at 4:10 PM, Ilia Mirkin  wrote:
 Hello,

 I was going to add support for the various volatile/etc annotations in
 my gallium impl (which is fairly far along), but it seems like those
 are currently being dropped on the floor by lower_ubo_references, and
 NIR has no support for them either. Both in GLSL IR and NIR, the
 variable is what holds the access annotation (volatile, etc). However
 the ssbo intrinsics are all about a particular binding and offset,
 without any reference to the variable.

 What's the right way of handling this? (And do any tests exist that
 would be sensitive to getting such things wrong?)
>>>
>>> First off, why is it that way?  Well, because most of the IRs in mesa
>>> don't have a memory model capable of handling these sorts of things.
>>> Those that do (LLVM is the only one I'm aware of) can't handle the GL
>>> packing rules.  The result is that we basically have to lower
>>> everything away to byte-offset load/store intrinsics.
>>>
>>> What do we do about it?  My inclination would be to either add two new
>>> intrinsics for load/store_ssbo_volatile or to add a new constant
>>> boolean "volatile" parameter to the current intrinsics.  If a
>>> load/store happens on a volatile things, you get the volatile version,
>>> otherwise you get the plane version.  Then backends can know that they
>>> are free to reorder, combine, etc. non-volatile load/store operations
>>> as per their memory model and the provided barriers.  If they
>>> encounter a volatile load/store, they can't do anything with it and
>>> just have to do the memory op.
>>>
>>> Is that reasonable?
>>
>> Well, I don't really care much about the NIR-side of it, but I think
>> at the very least coherent and volatile need to be exposed. I don't
>> much care about restrict, that's a bit too fancy for me, but no harm
>> in passing it through. TBH I don't get the point of
>> readonly/writeonly. FWIW this is what the nvidia hw is capable of:
>> http://docs.nvidia.com/cuda/parallel-thread-execution/#cache-operators
>> (I know it's PTX, but the actual hw is basically the same.)
>>
>> I think that volatile = cv, coherent = cg, and everything else = ca/cs
>> based on some clever heuristic (aka I'll always pick one of those
>> until a situation where that doesn't work well arises).
>
> Right.  I don't actually know what we can do with our HW.  I was
> mostly thinking about what optimization passes can or cannot do with a
> variable.  It sounds like, whatever we do, you want it passed through
> somehow.  How about we just add a little well-defined bitfield as an
> extra argument to the SSBO load/store intrinsics?  The fact that it
> gets applied to a variable or a struct or a member or whatever is kind
> of a moot point to the backend compiler.

An extra argument with a mask or enum is precisely what I was
expecting to find when looking at the lower_ubo_references pass when I
was rather surprised to see that the whole thing had been overlooked.

Cheers,

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

[Mesa-dev] [PATCH V3 1/2] glsl: fix overlapping of varying locations for arrays and structs

[Timothy Arceri]

Previously we were only reserving a single location for arrays and
structs.

We also didn't take into account implicit locations clashing with
explicit locations when assigning locations for their arrays or
structs.

This patch fixes both issues.

V3: handle arrays of structs
V2: also fix for arrays of arrays and structs.

Cc: Kenneth Graunke 
Cc: 11.1 
---
 src/glsl/link_varyings.cpp | 86 +-
 1 file changed, 78 insertions(+), 8 deletions(-)

diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index 71750d1..2f44a88 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -766,7 +766,8 @@ public:
gl_shader_stage consumer_stage);
~varying_matches();
void record(ir_variable *producer_var, ir_variable *consumer_var);
-   unsigned assign_locations(uint64_t reserved_slots, bool separate_shader);
+   unsigned assign_locations(struct gl_shader_program *prog,
+ uint64_t reserved_slots, bool separate_shader);
void store_locations() const;
 
 private:
@@ -988,7 +989,9 @@ varying_matches::record(ir_variable *producer_var, 
ir_variable *consumer_var)
  * passed to varying_matches::record().
  */
 unsigned
-varying_matches::assign_locations(uint64_t reserved_slots, bool 
separate_shader)
+varying_matches::assign_locations(struct gl_shader_program *prog,
+  uint64_t reserved_slots,
+  bool separate_shader)
 {
/* We disable varying sorting for separate shader programs for the
 * following reasons:
@@ -1040,9 +1043,60 @@ varying_matches::assign_locations(uint64_t 
reserved_slots, bool separate_shader)
   != this->matches[i].packing_class) {
  *location = ALIGN(*location, 4);
   }
-  while ((*location < MAX_VARYING * 4u) &&
-(reserved_slots & (1u << *location / 4u))) {
- *location = ALIGN(*location + 1, 4);
+
+  const ir_variable *var =
+ matches[i].consumer_var ? matches[i].consumer_var :
+matches[i].producer_var;
+
+  unsigned num_elements;
+  if (var->type->is_array()) {
+ num_elements = var->type->arrays_of_arrays_size();
+ if (var->type->without_array()->is_record())
+num_elements *= var->type->
+   record_location_offset(var->type->without_array()->length);
+  } else if (var->type->is_record()) {
+ num_elements = var->type->record_location_offset(var->type->length);
+  } else {
+ num_elements = 1;
+  }
+
+  unsigned slot_end = this->disable_varying_packing ? 4 :
+ var->type->without_array()->vector_elements;
+  slot_end += *location - 1;
+
+  /* FIXME: We could be smarter in the below code and loop back over
+   * trying to fill any locations that we skipped because we couldn't pack
+   * the varying between an explicit location. For now just let the user
+   * hit the linking error if we run out of room and suggest they use
+   * explicit locations.
+   */
+  for (unsigned j = 0; j < num_elements; j++) {
+ while ((slot_end < MAX_VARYING * 4u) &&
+((reserved_slots & (1u << *location / 4u) ||
+ (reserved_slots & (1u << slot_end / 4u) {
+
+*location = ALIGN(*location + 1, 4);
+slot_end = *location;
+
+/* reset the counter and try again */
+j = 0;
+ }
+
+ /* Increase the slot to make sure there is enough room for next
+  * array element.
+  */
+ if (this->disable_varying_packing)
+slot_end += 4;
+ else
+slot_end += var->type->without_array()->vector_elements;
+  }
+
+  if (*location >= MAX_VARYING * 4u) {
+ linker_error(prog, "insufficient contiguous locations available for "
+  "%s it is possible an array or struct could not be "
+  "packed between varyings with explicit locations. Try "
+  "using an explicit location for arrays and structs.",
+  var->name);
   }
 
   this->matches[i].generic_location = *location;
@@ -1430,8 +1484,24 @@ reserved_varying_slot(struct gl_shader *stage, 
ir_variable_mode io_mode)
  continue;
 
   var_slot = var->data.location - VARYING_SLOT_VAR0;
-  if (var_slot >= 0 && var_slot < MAX_VARYING)
- slots |= 1u << var_slot;
+
+  unsigned num_elements;
+  if (var->type->is_array()) {
+ num_elements = var->type->arrays_of_arrays_size();
+ if (var->type->without_array()->is_record())
+num_elements *= var->type->
+   record_location_offset(var->type->without_array()->length);
+  } else if (var->type->is_record()) {
+ num_elements = var->type->record_location_offset(var->type->length);
+  } else {
+ 

Re: [Mesa-dev] [PATCH 2/5] nv50, nvc0: fix potential resource leak in nvXX_create_texture_view()

[Ilia Mirkin]

Again, bogus. Can't get there. I'd take a patch that asserts though,
or marks it unreachable, or just drops the default case entirely.

On Mon, Dec 14, 2015 at 5:14 AM, Samuel Pitoiset
 wrote:
> Spotted by Coverity.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/nv50/nv50_tex.c | 3 ++-
>  src/gallium/drivers/nouveau/nvc0/nvc0_tex.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_tex.c 
> b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
> index 6083ea9..9fb9dcf 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_tex.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
> @@ -193,7 +193,8 @@ nv50_create_texture_view(struct pipe_context *pipe,
>break;
> default:
>NOUVEAU_ERR("invalid texture target: %d\n", mt->base.base.target);
> -  return false;
> +  FREE(view);
> +  return NULL;
> }
>
> tic[3] = (flags & NV50_TEXVIEW_FILTER_MSAA8) ? 0x2000 : 0x0030;
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
> index 2dd100f..2503ee1 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
> @@ -195,7 +195,8 @@ nvc0_create_texture_view(struct pipe_context *pipe,
> default:
>NOUVEAU_ERR("unexpected/invalid texture target: %d\n",
>mt->base.base.target);
> -  return false;
> +  FREE(view);
> +  return NULL;
> }
>
> tic[3] = (flags & NV50_TEXVIEW_FILTER_MSAA8) ? 0x2000 : 0x0030;
> --
> 2.6.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

[Mesa-dev] [Bug 86326] clEnqueueNDRangeKernel global_work_offset ignored

[bugzilla-daemon]

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

Vedran Miletić  changed:

   What|Removed |Added

 CC||riva...@gmail.com

-- 
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

Re: [Mesa-dev] [Bug 93352] GRID Autosport crashes on start of race

[Ilia Mirkin]

Hi Edwin,

Thanks for reaching out to the mesa community! See my comments inline.

On Mon, Dec 14, 2015 at 5:13 AM,   wrote:
> Comment # 20 on bug 93352 from Edwin Smith
> My name is Edwin Smith and I work for Feral Interactive who make the Linux
> version of the game. Thank you for fixing the issue in the latest drivers, I
> am
> sure people playing the game on AMD machines will be very happy!
>
> We tested the game using 11.0.2 and apart from missing features for the
> higher
> end effects meaning they had to be disabled we did not see any major
> rendering
> issues.

Odd... this bug should have been visible with radeonsi and nouveau
when enabling tessellation. Perhaps only on debug builds though (on
opt builds it would still have been an issue, just no crash).

> Please let us know if we can assist by providing people working on driver
> issues a copy of the title or other information to help with your driver
> work.

The most straight-forward way would be to do the same thing Valve did
-- basically they provide a (free) package on Steam that includes all
of their games to people who are active mesa contributors. You can see
the details of their program here:
http://lists.freedesktop.org/archives/dri-devel/2015-April/081045.html
.

Cheers,

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

Re: [Mesa-dev] [PATCH 1/5] nv50, nvc0: fix using uninitialized value 'templ.texture' in nvXX_blit_set_src()

[Ilia Mirkin]

While true, this isn't a real issue.

   view->pipe = *templ;
   view->pipe.reference.count = 1;
   view->pipe.texture = NULL;

texture can be uninitialized in the template. I know that your first
impulse might be "just make Coverity happy", but I don't pander to
broken tools. If the report is bogus, mark it bogus, don't add
bogosity to the code.

  -ilia


On Mon, Dec 14, 2015 at 5:14 AM, Samuel Pitoiset
 wrote:
> Field 'templ.texture' is used uninitialized when calling
> nvXX_create_texture_view(). Spotted by Coverity.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/nv50/nv50_surface.c | 1 +
>  src/gallium/drivers/nouveau/nvc0/nvc0_surface.c | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_surface.c 
> b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> index 86be1b4..b03e893 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_surface.c
> @@ -1031,6 +1031,7 @@ nv50_blit_set_src(struct nv50_blitctx *blit,
>
> target = nv50_blit_reinterpret_pipe_texture_target(res->target);
>
> +   templ.texture = NULL;
> templ.format = format;
> templ.u.tex.first_level = templ.u.tex.last_level = level;
> templ.u.tex.first_layer = templ.u.tex.last_layer = layer;
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
> index f8e1efb..41fb82a 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
> @@ -841,6 +841,7 @@ nvc0_blit_set_src(struct nvc0_blitctx *ctx,
>
> target = nv50_blit_reinterpret_pipe_texture_target(res->target);
>
> +   templ.texture = NULL;
> templ.format = format;
> templ.u.tex.first_layer = templ.u.tex.last_layer = layer;
> templ.u.tex.first_level = templ.u.tex.last_level = level;
> --
> 2.6.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 04/15] i965/fs: Get rid of reladdr

[Francisco Jerez]

Jason Ekstrand  writes:

> On Dec 11, 2015 5:44 AM, "Francisco Jerez"  wrote:
>>
>> Jason Ekstrand  writes:
>>
>> > On Dec 10, 2015 6:58 AM, "Francisco Jerez" 
> wrote:
>> >>
>> >> Jason Ekstrand  writes:
>> >>
>> >> > We aren't using it anymore.
>> >>
>> >> It seems useful to me to be able to represent indirect access as part
> of
>> >> any instruction source or destination register.
>> >>
>> >> The following:
>> >>
>> >> | mov_indirect g0, g1, a0
>> >> | foo g2, g0
>> >>
>> >> and the converse case with indirect destination offset (which you don't
>> >> seem to represent currently) can be implemented by the hardware more
>> >> efficiently using a single instruction in certain cases.  The current
> IR
>> >> is able to represent what the hardware can do, but supporting the
>> >> MOV_INDIRECT instruction only would force us to keep the indirection
>> >> separate from the instruction that uses it, so it seems like a less
>> >> expressive representation to me than the current approach, unless
> you're
>> >> willing to add _INDIRECT variants of most hardware opcodes.
>> >
>> > Yes and, mostly, no.  Yes, you can put an indirect on almost anything
> but
>> > it has substantial restrictions:
>> >
>> Yes, I'm aware of the restrictions of indirect addressing on Gen
>> hardware.  The fact that reladdr can represent addressing modes which
>> aren't directly implemented in the hardware doesn't invalidate the
>> abstraction, nor implies that optimization and translation passes must
>> be made aware of such restrictions.  It just means that our abstraction
>> is a superset of the hardware indirect addressing scheme, which is fine
>> given a lowering pass to convert indirect addressing into a legal form.
>>
>> Your proposal instead goes the opposite direction and replaces the
>> preexisting abstraction with a new abstraction which can only represent
>> a tiny subset of the functionality of hardware instructions, which I
>> don't think is acceptable for a low-level IR.
>>
>> > 1) Destination indirects must be uniform (I'm only 95% sure this is the
>> > case)
>> >
>> > 2) We only have 8 address subregisters on HSW and prior and 16 on BDW
> which
>> > means:
>> >
>>
>> That's the kind of thing that SIMD lowering would be able to take care
>> of easily.
>
> Yes, and we use it for MOV_INDIRECT.
>
Right, and there's no reason why it couldn't be used for reladdr in the
same way to handle the above restrictions.

>> > a) All indirects must be uniform OR
>> >
>> > b) We must be on Broadwell, have only two indirects, and split the
>> > instruction OR
>> >
>> > c) We can only have one indirect (and still split the instruction on
>> > HSW)
>> >
>> > So, yes, "everthing can be uniform", but not in real life.
>>
>> > The reladdr abstraction, while maybe ok for a high-level IR, doesn't
>> > accurately represent the hardware at all because it can't represent
>> > any of the restrictions without piles of helper functions and checks.
>>
>> The reladdr abstraction can represent the full flexibility of the
>> hardware, while MOV_INDIRECT cannot, that makes reladdr a more accurate
>> low-level representation of the program.  For that reason I'd argue the
>> exact opposite: MOV_INDIRECT would be a great abstraction and a welcome
>> simplification for a high-level IR (e.g. NIR), in which nothing is lost
>> by sacrificing the expressiveness of individual instructions in favour
>> of instruction composition, because the back-end can always combine
>> multiple high-level instructions into a single low-level instruction
>> where the hardware ISA allows, as long as the low-level IR is able to
>> represent the full functionality of hardware instructions (IOW the
>> mapping between low-level IR instructions and hardware instructions is
>> surjective), which is further from being the case after this series.
>
> Flexibility and expressiveness isn't always a good thing.

Expressiveness is a good thing as long as it's required to represent the
capabilities of the hardware.

> Yes, reladdr is more "expressive", but it's not getting used because
> that expressiveness makes it a pain.

AFAIK nobody has bothered to implement it completely down to the
back-end generator so it's not a wonder that it's not extensively used.
I'd like to see some evidence that it would actually be a pain
(e.g. some proof of concept implementation) before we give up and commit
to an inherently more limited model.  If you already have such a proof
of concept, please share it.  If you don't and are fully convinced that
it will be a pain I suggest you hold up this series for a while and let
me write the proof of concept -- Doing something with the expectation
that it's going to be a pain is the best way to make sure it actually
becomes a pain.  Worst-case scenario you're right and it does become a
pain, at which point I come back and review this series.  

Re: [Mesa-dev] [Intel-gfx] [RFC libdrm] intel: Add support for softpin

[Song, Ruiling]

> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Monday, December 14, 2015 4:28 PM
> To: Song, Ruiling 
> Cc: k...@bitplanet.net; Winiarski, Michal ;
> mesa-dev@lists.freedesktop.org; intel-...@lists.freedesktop.org; Ben
> Widawsky ; dri-de...@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [RFC libdrm] intel: Add support for softpin
> 
> On Mon, Dec 14, 2015 at 07:24:29AM +, Song, Ruiling wrote:
> >
> >
> > > -Original Message-
> > > From: hoegsb...@gmail.com [mailto:hoegsb...@gmail.com] On Behalf
> Of
> > > Kristian H?gsberg
> > > Sent: Monday, December 14, 2015 1:34 PM
> > > To: Song, Ruiling 
> > > Cc: Winiarski, Michal ; intel-
> > > g...@lists.freedesktop.org; mesa-dev@lists.freedesktop.org; Ben
> Widawsky
> > > ; dri-de...@lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] [RFC libdrm] intel: Add support for softpin
> > >
> > > On Sun, Dec 13, 2015 at 7:17 PM, Song, Ruiling 
> > > wrote:
> > > >> -Original Message-
> > > >> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On
> > > Behalf
> > > >> Of Micha? Winiarski
> > > >> Sent: Wednesday, September 9, 2015 10:07 PM
> > > >> To: intel-...@lists.freedesktop.org
> > > >> Cc: Ben Widawsky ; dri-
> > > de...@lists.freedesktop.org;
> > > >> mesa-dev@lists.freedesktop.org
> > > >> Subject: [Intel-gfx] [RFC libdrm] intel: Add support for softpin
> > > >>
> > > >> Softpin allows userspace to take greater control of GPU virtual address
> > > >> space and eliminates the need of relocations. It can also be used to
> > > >> mirror addresses between GPU and CPU (shared virtual memory).
> > > >> Calls to drm_intel_bo_emit_reloc are still required to build the list 
> > > >> of
> > > >> drm_i915_gem_exec_objects at exec time, but no entries in relocs are
> > > >> created. Self-relocs don't make any sense for softpinned objects and
> can
> > > >> indicate a programming errors, thus are forbidden. Softpinned objects
> > > >> are marked by asterisk in debug dumps.
> > > >>
> > > >> Cc: Thomas Daniel 
> > > >> Cc: Kristian Høgsberg 
> > > >> Cc: Zou Nanhai 
> > > >> Cc: Michel Thierry 
> > > >> Cc: Ben Widawsky 
> > > >> Cc: Chris Wilson 
> > > >> Signed-off-by: Michał Winiarski 
> > > >> ---
> > > >>  include/drm/i915_drm.h|   4 +-
> > > >>  intel/intel_bufmgr.c  |   9 +++
> > > >>  intel/intel_bufmgr.h  |   1 +
> > > >>  intel/intel_bufmgr_gem.c  | 176
> > > >> --
> > > >>  intel/intel_bufmgr_priv.h |   7 ++
> > > >>  5 files changed, 173 insertions(+), 24 deletions(-)
> > > >
> > > > Will anybody help to push the patch to libdrm? Beignet highly depend
> on
> > > this to implement ocl2.0 svm.
> > >
> > > Is the kernel patch upstream?
> >
> > Yes, the kernel patch already merged, see:
> > http://cgit.freedesktop.org/drm-
> intel/commit/?id=506a8e87d8d2746b9e9d2433503fe237c54e4750
> >
> > I find below line of code in libdrm does not match the kernel version. The
> kernel patch defined as:
> > "#define EXEC_OBJECT_PINNED (1<<4)", but this patch defined it as (1<<3).
> 
> Please always regenerate the entire headers from the kernel sources using
> 
> $ make headers_install
> 
> Then copy the headers from the kernel's usr/include/drm to libdrm. Never
> patch i915_drm.h manually.

Thanks for the info. But the problem is libdrm still tracks such kind of header 
files.
Should this kind of header file be removed from libdrm? Or any document in 
libdrm to make this explicit?

Thanks!
Ruiling
 
> Thanks, Daniel
> 
> >
> > Hello Michal,
> >
> > Could you help to rebase the patch against:
> > [Intel-gfx] [PATCH libdrm v4 0/2] 48-bit virtual address support in i915
> > I think we need both 48bit & softpin in libdrm.
> >
> > diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> > index ded43b1..2b99fc6 100644
> > --- a/include/drm/i915_drm.h
> > +++ b/include/drm/i915_drm.h
> > @@ -350,6 +350,7 @@ typedef struct drm_i915_irq_wait {
> >  #define I915_PARAM_REVISION  32
> >  #define I915_PARAM_SUBSLICE_TOTAL   33
> >  #define I915_PARAM_EU_TOTAL 34
> > +#define I915_PARAM_HAS_EXEC_SOFTPIN 37
> >
> >  typedef struct drm_i915_getparam {
> > int param;
> > @@ -680,7 +681,8 @@ struct drm_i915_gem_exec_object2 {
> >  #define EXEC_OBJECT_NEEDS_FENCE (1<<0)
> >  #define EXEC_OBJECT_NEEDS_GTT  (1<<1)
> >  #define EXEC_OBJECT_WRITE  (1<<2)
> > -#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_WRITE<<1)
> > +#define EXEC_OBJECT_PINNED (1<<3)
> > +#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_PINNED<<1)
> > __u64 flags;
> 

Re: [Mesa-dev] [Intel-gfx] [RFC libdrm] intel: Add support for softpin

[Song, Ruiling]

> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Micha? Winiarski
> Sent: Wednesday, September 9, 2015 10:07 PM
> To: intel-...@lists.freedesktop.org
> Cc: Ben Widawsky ; dri-de...@lists.freedesktop.org;
> mesa-dev@lists.freedesktop.org
> Subject: [Intel-gfx] [RFC libdrm] intel: Add support for softpin
> 
> Softpin allows userspace to take greater control of GPU virtual address
> space and eliminates the need of relocations. It can also be used to
> mirror addresses between GPU and CPU (shared virtual memory).
> Calls to drm_intel_bo_emit_reloc are still required to build the list of
> drm_i915_gem_exec_objects at exec time, but no entries in relocs are
> created. Self-relocs don't make any sense for softpinned objects and can
> indicate a programming errors, thus are forbidden. Softpinned objects
> are marked by asterisk in debug dumps.
> 
> Cc: Thomas Daniel 
> Cc: Kristian Høgsberg 
> Cc: Zou Nanhai 
> Cc: Michel Thierry 
> Cc: Ben Widawsky 
> Cc: Chris Wilson 
> Signed-off-by: Michał Winiarski 
> ---
>  include/drm/i915_drm.h|   4 +-
>  intel/intel_bufmgr.c  |   9 +++
>  intel/intel_bufmgr.h  |   1 +
>  intel/intel_bufmgr_gem.c  | 176
> --
>  intel/intel_bufmgr_priv.h |   7 ++
>  5 files changed, 173 insertions(+), 24 deletions(-)

Will anybody help to push the patch to libdrm? Beignet highly depend on this to 
implement ocl2.0 svm.

Thanks!
Ruiling

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

Re: [Mesa-dev] [Intel-gfx] [RFC libdrm] intel: Add support for softpin

[Song, Ruiling]

> -Original Message-
> From: hoegsb...@gmail.com [mailto:hoegsb...@gmail.com] On Behalf Of
> Kristian H?gsberg
> Sent: Monday, December 14, 2015 1:34 PM
> To: Song, Ruiling 
> Cc: Winiarski, Michal ; intel-
> g...@lists.freedesktop.org; mesa-dev@lists.freedesktop.org; Ben Widawsky
> ; dri-de...@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [RFC libdrm] intel: Add support for softpin
> 
> On Sun, Dec 13, 2015 at 7:17 PM, Song, Ruiling 
> wrote:
> >> -Original Message-
> >> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On
> Behalf
> >> Of Micha? Winiarski
> >> Sent: Wednesday, September 9, 2015 10:07 PM
> >> To: intel-...@lists.freedesktop.org
> >> Cc: Ben Widawsky ; dri-
> de...@lists.freedesktop.org;
> >> mesa-dev@lists.freedesktop.org
> >> Subject: [Intel-gfx] [RFC libdrm] intel: Add support for softpin
> >>
> >> Softpin allows userspace to take greater control of GPU virtual address
> >> space and eliminates the need of relocations. It can also be used to
> >> mirror addresses between GPU and CPU (shared virtual memory).
> >> Calls to drm_intel_bo_emit_reloc are still required to build the list of
> >> drm_i915_gem_exec_objects at exec time, but no entries in relocs are
> >> created. Self-relocs don't make any sense for softpinned objects and can
> >> indicate a programming errors, thus are forbidden. Softpinned objects
> >> are marked by asterisk in debug dumps.
> >>
> >> Cc: Thomas Daniel 
> >> Cc: Kristian Høgsberg 
> >> Cc: Zou Nanhai 
> >> Cc: Michel Thierry 
> >> Cc: Ben Widawsky 
> >> Cc: Chris Wilson 
> >> Signed-off-by: Michał Winiarski 
> >> ---
> >>  include/drm/i915_drm.h|   4 +-
> >>  intel/intel_bufmgr.c  |   9 +++
> >>  intel/intel_bufmgr.h  |   1 +
> >>  intel/intel_bufmgr_gem.c  | 176
> >> --
> >>  intel/intel_bufmgr_priv.h |   7 ++
> >>  5 files changed, 173 insertions(+), 24 deletions(-)
> >
> > Will anybody help to push the patch to libdrm? Beignet highly depend on
> this to implement ocl2.0 svm.
> 
> Is the kernel patch upstream?

Yes, the kernel patch already merged, see:
http://cgit.freedesktop.org/drm-intel/commit/?id=506a8e87d8d2746b9e9d2433503fe237c54e4750

I find below line of code in libdrm does not match the kernel version. The 
kernel patch defined as:
"#define EXEC_OBJECT_PINNED (1<<4)", but this patch defined it as (1<<3).

Hello Michal,

Could you help to rebase the patch against:
[Intel-gfx] [PATCH libdrm v4 0/2] 48-bit virtual address support in i915
I think we need both 48bit & softpin in libdrm.

diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index ded43b1..2b99fc6 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -350,6 +350,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_REVISION  32
 #define I915_PARAM_SUBSLICE_TOTAL   33
 #define I915_PARAM_EU_TOTAL 34
+#define I915_PARAM_HAS_EXEC_SOFTPIN 37
 
 typedef struct drm_i915_getparam {
int param;
@@ -680,7 +681,8 @@ struct drm_i915_gem_exec_object2 {
 #define EXEC_OBJECT_NEEDS_FENCE (1<<0)
 #define EXEC_OBJECT_NEEDS_GTT  (1<<1)
 #define EXEC_OBJECT_WRITE  (1<<2)
-#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_WRITE<<1)
+#define EXEC_OBJECT_PINNED (1<<3)
+#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_PINNED<<1)
__u64 flags;
 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] Further Coccinelle inspired cleanups

[Edward O'Callaghan]

The following patch series continues on what can be done with
Coccinelle to clean up Mesa3D house keeping duties. On a good
note we have a pretty clean tree for the fairly trivial runs
so far.

 [PATCH 1/6] gallium: Remove unnecessary semicolons
 [PATCH 2/6] gallium/drivers: Remove unnecessary semicolons
 [PATCH 3/6] gallium: Use unsigned for loop index
 [PATCH 4/6] gallium/drivers/ilo: Use unsigned for loop index
 [PATCH 5/6] gallium/drivers/r600: Use unsigned for loop index
 [PATCH 6/6] gallium/drivers/svga: Use unsigned for loop index
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/6] gallium: Remove unnecessary semicolons

[Edward O'Callaghan]

Found-by: Coccinelle
Signed-off-by: Edward O'Callaghan 
---
 src/gallium/auxiliary/draw/draw_pipe_aaline.c  | 2 +-
 src/gallium/auxiliary/gallivm/lp_bld_swizzle.c | 2 +-
 src/gallium/auxiliary/nir/tgsi_to_nir.c| 2 +-
 src/gallium/auxiliary/util/u_surface.c | 2 +-
 src/gallium/auxiliary/vl/vl_mpeg12_decoder.c   | 2 +-
 src/gallium/state_trackers/nine/swapchain9.c   | 2 +-
 src/gallium/state_trackers/omx/entrypoint.c| 2 +-
 src/gallium/state_trackers/vdpau/mixer.c   | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_pipe_aaline.c 
b/src/gallium/auxiliary/draw/draw_pipe_aaline.c
index 877db59..4a676b7 100644
--- a/src/gallium/auxiliary/draw/draw_pipe_aaline.c
+++ b/src/gallium/auxiliary/draw/draw_pipe_aaline.c
@@ -937,7 +937,7 @@ draw_aaline_prepare_outputs(struct draw_context *draw,
const struct pipe_rasterizer_state *rast = draw->rasterizer;
 
/* update vertex attrib info */
-   aaline->pos_slot = draw_current_shader_position_output(draw);;
+   aaline->pos_slot = draw_current_shader_position_output(draw);
 
if (!rast->line_smooth)
   return;
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_swizzle.c 
b/src/gallium/auxiliary/gallivm/lp_bld_swizzle.c
index b1aef71..f571838 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_swizzle.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_swizzle.c
@@ -720,7 +720,7 @@ lp_build_transpose_aos_n(struct gallivm_state *gallivm,
 
   default:
  assert(0);
-   };
+   }
 }
 
 
diff --git a/src/gallium/auxiliary/nir/tgsi_to_nir.c 
b/src/gallium/auxiliary/nir/tgsi_to_nir.c
index 5def6d3..5cbe8e9 100644
--- a/src/gallium/auxiliary/nir/tgsi_to_nir.c
+++ b/src/gallium/auxiliary/nir/tgsi_to_nir.c
@@ -1951,7 +1951,7 @@ tgsi_processor_to_shader_stage(unsigned processor)
case TGSI_PROCESSOR_COMPUTE:   return MESA_SHADER_COMPUTE;
default:
   unreachable("invalid TGSI processor");
-   };
+   }
 }
 
 struct nir_shader *
diff --git a/src/gallium/auxiliary/util/u_surface.c 
b/src/gallium/auxiliary/util/u_surface.c
index 6aa44f9..67da59f 100644
--- a/src/gallium/auxiliary/util/u_surface.c
+++ b/src/gallium/auxiliary/util/u_surface.c
@@ -600,7 +600,7 @@ is_box_inside_resource(const struct pipe_resource *res,
   depth = res->array_size;
   assert(res->array_size % 6 == 0);
   break;
-   case PIPE_MAX_TEXTURE_TYPES:;
+   case PIPE_MAX_TEXTURE_TYPES:
}
 
return box->x >= 0 &&
diff --git a/src/gallium/auxiliary/vl/vl_mpeg12_decoder.c 
b/src/gallium/auxiliary/vl/vl_mpeg12_decoder.c
index f5bb3a0..b5c7045 100644
--- a/src/gallium/auxiliary/vl/vl_mpeg12_decoder.c
+++ b/src/gallium/auxiliary/vl/vl_mpeg12_decoder.c
@@ -792,7 +792,7 @@ vl_mpeg12_end_frame(struct pipe_video_codec *decoder,
   for (j = 0; j < VL_MAX_REF_FRAMES; ++j) {
  if (!ref_frames[j] || !ref_frames[j][i]) continue;
 
- vb[2] = vl_vb_get_mv(>vertex_stream, j);;
+ vb[2] = vl_vb_get_mv(>vertex_stream, j);
  dec->context->set_vertex_buffers(dec->context, 0, 3, vb);
 
  vl_mc_render_ref(i ? >mc_c : >mc_y, >mc[i], 
ref_frames[j][i]);
diff --git a/src/gallium/state_trackers/nine/swapchain9.c 
b/src/gallium/state_trackers/nine/swapchain9.c
index 3f5be26..3b1a7a4 100644
--- a/src/gallium/state_trackers/nine/swapchain9.c
+++ b/src/gallium/state_trackers/nine/swapchain9.c
@@ -790,7 +790,7 @@ NineSwapChain9_Present( struct NineSwapChain9 *This,
 case D3DSWAPEFFECT_FLIP:
 UNTESTED(4);
 case D3DSWAPEFFECT_DISCARD:
-/* rotate the queue */;
+/* rotate the queue */
 pipe_resource_reference(, This->buffers[0]->base.resource);
 for (i = 1; i <= This->params.BackBufferCount; i++) {
 NineSurface9_SetResourceResize(This->buffers[i - 1],
diff --git a/src/gallium/state_trackers/omx/entrypoint.c 
b/src/gallium/state_trackers/omx/entrypoint.c
index da9ca10..afcbd97 100644
--- a/src/gallium/state_trackers/omx/entrypoint.c
+++ b/src/gallium/state_trackers/omx/entrypoint.c
@@ -137,7 +137,7 @@ OMX_ERRORTYPE omx_workaround_Destructor(OMX_COMPONENTTYPE 
*comp)
priv->state = OMX_StateInvalid;
tsem_up(priv->messageSem);
 
-   /* wait for thread to exit */;
+   /* wait for thread to exit */
pthread_join(priv->messageHandlerThread, NULL);
 
return omx_base_component_Destructor(comp);
diff --git a/src/gallium/state_trackers/vdpau/mixer.c 
b/src/gallium/state_trackers/vdpau/mixer.c
index c0b1ecc..dec79ff 100644
--- a/src/gallium/state_trackers/vdpau/mixer.c
+++ b/src/gallium/state_trackers/vdpau/mixer.c
@@ -294,7 +294,7 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer,
default:
   pipe_mutex_unlock(vmixer->device->mutex);
   return VDP_STATUS_INVALID_VIDEO_MIXER_PICTURE_STRUCTURE;
-   };
+   }
 
if (deinterlace != VL_COMPOSITOR_WEAVE && vmixer->deint.enabled &&
video_surface_past_count > 1 && video_surface_future_count > 0) {
-- 

[Mesa-dev] [PATCH 6/6] gallium/drivers/svga: Use unsigned for loop index

[Edward O'Callaghan]

Found-by: Coccinelle
Signed-off-by: Edward O'Callaghan 
---
 src/gallium/drivers/svga/svga_tgsi_insn.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/gallium/drivers/svga/svga_tgsi_insn.c 
b/src/gallium/drivers/svga/svga_tgsi_insn.c
index dbb90f7..1c7097f 100644
--- a/src/gallium/drivers/svga/svga_tgsi_insn.c
+++ b/src/gallium/drivers/svga/svga_tgsi_insn.c
@@ -166,7 +166,7 @@ scalar(struct src_register src, unsigned comp)
 static boolean
 svga_arl_needs_adjustment( const struct svga_shader_emitter *emit )
 {
-   int i;
+   unsigned int i;
 
for (i = 0; i < emit->num_arl_consts; ++i) {
   if (emit->arl_consts[i].arl_num == emit->current_arl)
@@ -179,7 +179,7 @@ svga_arl_needs_adjustment( const struct svga_shader_emitter 
*emit )
 static int
 svga_arl_adjustment( const struct svga_shader_emitter *emit )
 {
-   int i;
+   unsigned int i;
 
for (i = 0; i < emit->num_arl_consts; ++i) {
   if (emit->arl_consts[i].arl_num == emit->current_arl)
@@ -1175,7 +1175,7 @@ emit_div(struct svga_shader_emitter *emit,
const struct src_register src1 =
   translate_src_register(emit, >Src[1] );
SVGA3dShaderDestToken temp = get_temp( emit );
-   int i;
+   unsigned int i;
 
/* For each enabled element, perform a RCP instruction.  Note that
 * RCP is scalar in SVGA3D:
@@ -1822,7 +1822,7 @@ emit_tex_swizzle(struct svga_shader_emitter *emit,
const unsigned swizzleIn[4] = {swizzle_x, swizzle_y, swizzle_z, swizzle_w};
unsigned srcSwizzle[4];
unsigned srcWritemask = 0x0, zeroWritemask = 0x0, oneWritemask = 0x0;
-   int i;
+   unsigned int i;
 
/* build writemasks and srcSwizzle terms */
for (i = 0; i < 4; i++) {
@@ -3371,7 +3371,7 @@ emit_light_twoside(struct svga_shader_emitter *emit)
struct src_register back[2];
SVGA3dShaderDestToken color[2];
int count = emit->internal_color_count;
-   int i;
+   unsigned int i;
SVGA3dShaderInstToken if_token;
 
if (count == 0)
@@ -3698,7 +3698,7 @@ static boolean
 pre_parse_add_indirect( struct svga_shader_emitter *emit,
 int num, int current_arl)
 {
-   int i;
+   unsigned int i;
assert(num < 0);
 
for (i = 0; i < emit->num_arl_consts; ++i) {
-- 
2.5.0

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

[Mesa-dev] [PATCH v2 3/3] nv50: check return value of nouveau_object_new()

[Samuel Pitoiset]

When ret == 0, obj is not NULL. Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nv50/nv84_video.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv84_video.c 
b/src/gallium/drivers/nouveau/nv50/nv84_video.c
index 7a4670f..88655db 100644
--- a/src/gallium/drivers/nouveau/nv50/nv84_video.c
+++ b/src/gallium/drivers/nouveau/nv50/nv84_video.c
@@ -756,8 +756,8 @@ firmware_present(struct pipe_screen *pscreen, enum 
pipe_video_format codec)
int present, ret;
 
if (!FIRMWARE_PRESENT(checked, VP_KERN)) {
-  nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0, );
-  if (obj)
+  ret = nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0, );
+  if (!ret)
  screen->firmware_info.profiles_present |= FIRMWARE_VP_KERN;
   nouveau_object_del();
   screen->firmware_info.profiles_checked |= FIRMWARE_VP_KERN;
@@ -765,8 +765,8 @@ firmware_present(struct pipe_screen *pscreen, enum 
pipe_video_format codec)
 
if (codec == PIPE_VIDEO_FORMAT_MPEG4_AVC) {
   if (!FIRMWARE_PRESENT(checked, BSP_KERN)) {
- nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0, );
- if (obj)
+ ret = nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0, );
+ if (!ret)
 screen->firmware_info.profiles_present |= FIRMWARE_BSP_KERN;
  nouveau_object_del();
  screen->firmware_info.profiles_checked |= FIRMWARE_BSP_KERN;
-- 
2.6.4

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

Re: [Mesa-dev] [PATCH v2 1/3] nv50, nvc0: make use of unreachable() when invalid texture target happens

[Ilia Mirkin]

Reviewed-by: Ilia Mirkin 

On Mon, Dec 14, 2015 at 11:51 AM, Samuel Pitoiset
 wrote:
> Spotted by Coverity.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/nv50/nv50_tex.c | 3 +--
>  src/gallium/drivers/nouveau/nvc0/nvc0_tex.c | 4 +---
>  2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_tex.c 
> b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
> index 6083ea9..c3f4336 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_tex.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
> @@ -192,8 +192,7 @@ nv50_create_texture_view(struct pipe_context *pipe,
>tic[2] |= NV50_TIC_2_TARGET_BUFFER | NV50_TIC_2_LINEAR;
>break;
> default:
> -  NOUVEAU_ERR("invalid texture target: %d\n", mt->base.base.target);
> -  return false;
> +  unreachable("unexpected/invalid texture target");
> }
>
> tic[3] = (flags & NV50_TEXVIEW_FILTER_MSAA8) ? 0x2000 : 0x0030;
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
> index 2dd100f..74090ce 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
> @@ -193,9 +193,7 @@ nvc0_create_texture_view(struct pipe_context *pipe,
>tic[2] |= NV50_TIC_2_TARGET_CUBE_ARRAY;
>break;
> default:
> -  NOUVEAU_ERR("unexpected/invalid texture target: %d\n",
> -  mt->base.base.target);
> -  return false;
> +  unreachable("unexpected/invalid texture target");
> }
>
> tic[3] = (flags & NV50_TEXVIEW_FILTER_MSAA8) ? 0x2000 : 0x0030;
> --
> 2.6.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

[Mesa-dev] [PATCH v3] nvc0: check return value of nvc0_program_validate()

[Samuel Pitoiset]

Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
index 7e2e999..5e84ca9 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
@@ -236,11 +236,8 @@ nvc0_gmtyprog_validate(struct nvc0_context *nvc0)
struct nouveau_pushbuf *push = nvc0->base.pushbuf;
struct nvc0_program *gp = nvc0->gmtyprog;
 
-   if (gp)
-  nvc0_program_validate(nvc0, gp);
-
/* we allow GPs with no code for specifying stream output state only */
-   if (gp && gp->code_size) {
+   if (gp && nvc0_program_validate(nvc0, gp) && gp->code_size) {
   const bool gp_selects_layer = !!(gp->hdr[13] & (1 << 9));
 
   BEGIN_NVC0(push, NVC0_3D(MACRO_GP_SELECT), 1);
-- 
2.6.4

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

Re: [Mesa-dev] gallium r300 driver for PowerPC

[Nicolai Hähnle]

On 14.12.2015 04:10, Eero Tamminen wrote:

On 12/14/2015 10:44 AM, Herminio Hernandez, Jr. wrote:

I am new to this list. I have been trying to see if I can fix or at
least pin point an issue with Radeon r300 driver failing on PowerPC
systems. This has been a problem for a while and I would like to help
to get this fixed. I have done some debugging with valgrind and I
think I may see where the issue is but I would to have someone double
check what I am doing. So when I set my Default Depth to 16 I do get
3D acceleration but when I set to the default of 24 it breaks.
Valgrind reports memory leaks when I run glxgears with a Default Depth
of 24 but shows no definite memory leaks with a Depth of 16. I then
got the source code and created a dev environment andnran glxgears
through valgrind with my default depth of 24 and saw similar memory
leaks. Here is a sample of what I am seeing.

==25273== 108 (12 direct, 96 indirect) bytes in 1 blocks are
definitely lost in loss record 54 of 78
==25273==at 0xFFB2868: malloc (vg_replace_malloc.c:299)
==25273==by 0xED0457B: ???
==25273==by 0xEEC6F3B: ???
==25273==by 0xE95A78B: ???
==25273==by 0xED7DF7F: ???
==25273==by 0xED7D5DB: ???
==25273==by 0xEC5B377: ???
==25273==by 0xEC567EB: ???
==25273==by 0xFDEDFD3: dri2CreateScreen (dri2_glx.c:1235)
==25273==by 0xFDB866F: AllocAndFetchScreenConfigs (glxext.c:799)
==25273==by 0xFDB866F: __glXInitialize (glxext.c:910)
==25273==by 0xFDB36F3: GetGLXPrivScreenConfig.part.2 (glxcmds.c:172)
==25273==by 0xFDB396B: GetGLXPrivScreenConfig (glxcmds.c:168)
==25273==by 0xFDB396B: glXChooseVisual (glxcmds.c:1249)

It looks like the files in the src/glx branch is where the issue is. I
am attaching the summary portion of the output I got from valgrind. Am
I heading in the right direction?


Install debug symbols for the libraries that Valgrind is complaining
about, to see what actually leaks.  Because they all come from through
GetGLXPrivScreenConfig(), I think this is something (inconsequential) in
your X libraries, not Mesa.


This is below dri2CreateScreen in the call stack, so it's actually quite 
plausible that it's in the driver. Make sure you have those debug symbols.


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

[Mesa-dev] [PATCH 2/6] gallium/drivers: Remove unnecessary semicolons

[Edward O'Callaghan]

Found-by: Coccinelle
Signed-off-by: Edward O'Callaghan 
---
 src/gallium/drivers/i915/i915_state.c | 2 +-
 src/gallium/drivers/ilo/shader/ilo_shader_vs.c| 2 +-
 src/gallium/drivers/llvmpipe/lp_test_blend.c  | 2 +-
 src/gallium/drivers/llvmpipe/lp_test_conv.c   | 2 +-
 src/gallium/drivers/r300/compiler/r500_fragprog.c | 2 +-
 src/gallium/drivers/r600/r600_shader.c| 2 +-
 src/gallium/drivers/radeonsi/cik_sdma.c   | 2 +-
 src/gallium/drivers/softpipe/sp_query.c   | 2 +-
 src/gallium/drivers/svga/svga_cmd.c   | 4 ++--
 src/gallium/drivers/vc4/vc4_program.c | 2 +-
 10 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/gallium/drivers/i915/i915_state.c 
b/src/gallium/drivers/i915/i915_state.c
index 6ba9646..d1661fe 100644
--- a/src/gallium/drivers/i915/i915_state.c
+++ b/src/gallium/drivers/i915/i915_state.c
@@ -423,7 +423,7 @@ i915_prepare_vertex_sampling(struct i915_context *i915)
  for (j = view->u.tex.first_level; j <= tex->last_level; j++) {
 mip_offsets[j] = i915_texture_offset(i915_tex, j , 0 /* FIXME 
depth */);
 row_stride[j] = i915_tex->stride;
-img_stride[j] = 0; /* FIXME */;
+img_stride[j] = 0; /* FIXME */
  }
 
  draw_set_mapped_texture(i915->draw,
diff --git a/src/gallium/drivers/ilo/shader/ilo_shader_vs.c 
b/src/gallium/drivers/ilo/shader/ilo_shader_vs.c
index a29baab..46a7e6f 100644
--- a/src/gallium/drivers/ilo/shader/ilo_shader_vs.c
+++ b/src/gallium/drivers/ilo/shader/ilo_shader_vs.c
@@ -126,7 +126,7 @@ vs_lower_opcode_tgsi_const_gen6(struct vs_compile_context 
*vcc,
tc_MOV(tc, block_offsets, idx);
 
msg_type = GEN6_MSG_DP_OWORD_DUAL_BLOCK_READ;
-   msg_ctrl = GEN6_MSG_DP_OWORD_DUAL_BLOCK_SIZE_1;;
+   msg_ctrl = GEN6_MSG_DP_OWORD_DUAL_BLOCK_SIZE_1;
msg_len = 2;
 
desc = tsrc_imm_mdesc_data_port(tc, false, msg_len, 1, true, false,
diff --git a/src/gallium/drivers/llvmpipe/lp_test_blend.c 
b/src/gallium/drivers/llvmpipe/lp_test_blend.c
index 7b19174..9139b83 100644
--- a/src/gallium/drivers/llvmpipe/lp_test_blend.c
+++ b/src/gallium/drivers/llvmpipe/lp_test_blend.c
@@ -184,7 +184,7 @@ add_blend_test(struct gallivm_state *gallivm,
 
LLVMBuildStore(builder, res, res_ptr);
 
-   LLVMBuildRetVoid(builder);;
+   LLVMBuildRetVoid(builder);
 
gallivm_verify_function(gallivm, func);
 
diff --git a/src/gallium/drivers/llvmpipe/lp_test_conv.c 
b/src/gallium/drivers/llvmpipe/lp_test_conv.c
index a30f35c..02a6319 100644
--- a/src/gallium/drivers/llvmpipe/lp_test_conv.c
+++ b/src/gallium/drivers/llvmpipe/lp_test_conv.c
@@ -140,7 +140,7 @@ add_conv_test(struct gallivm_state *gallivm,
   LLVMBuildStore(builder, dst[i], ptr);
}
 
-   LLVMBuildRetVoid(builder);;
+   LLVMBuildRetVoid(builder);
 
gallivm_verify_function(gallivm, func);
 
diff --git a/src/gallium/drivers/r300/compiler/r500_fragprog.c 
b/src/gallium/drivers/r300/compiler/r500_fragprog.c
index 88aad8a..4c415af 100644
--- a/src/gallium/drivers/r300/compiler/r500_fragprog.c
+++ b/src/gallium/drivers/r300/compiler/r500_fragprog.c
@@ -384,7 +384,7 @@ void r500FragmentProgramDump(struct radeon_compiler *c, 
void *user)
 case R500_INST_TYPE_OUT: str = "OUT"; break;
 case R500_INST_TYPE_FC: str = "FC"; break;
 case R500_INST_TYPE_TEX: str = "TEX"; break;
-};
+}
 fprintf(stderr,"%s %s %s %s %s ", str,
inst & R500_INST_TEX_SEM_WAIT ? "TEX_WAIT" : "",
inst & R500_INST_LAST ? "LAST" : "",
diff --git a/src/gallium/drivers/r600/r600_shader.c 
b/src/gallium/drivers/r600/r600_shader.c
index d411b0b..db53eb4 100644
--- a/src/gallium/drivers/r600/r600_shader.c
+++ b/src/gallium/drivers/r600/r600_shader.c
@@ -4424,7 +4424,7 @@ static int cayman_mul_double_instr(struct r600_shader_ctx 
*ctx)
memset(, 0, sizeof(struct r600_bytecode_alu));
alu.op = ctx->inst_info->op;
for (j = 0; j < inst->Instruction.NumSrcRegs; j++) {
-   r600_bytecode_src([j], >src[j], k 
* 2 + ((i == 3) ? 0 : 1));;
+   r600_bytecode_src([j], >src[j], k 
* 2 + ((i == 3) ? 0 : 1));
}
alu.dst.sel = t1;
alu.dst.chan = i;
diff --git a/src/gallium/drivers/radeonsi/cik_sdma.c 
b/src/gallium/drivers/radeonsi/cik_sdma.c
index 2de237b..105a1b2 100644
--- a/src/gallium/drivers/radeonsi/cik_sdma.c
+++ b/src/gallium/drivers/radeonsi/cik_sdma.c
@@ -196,7 +196,7 @@ static void cik_sdma_copy_tile(struct si_context *ctx,
(tile_split << 11) | (mt << 8) | (array_mode << 3) |
lbpe;
cs->buf[cs->cdw++] = y << 16; /* | x */
-   cs->buf[cs->cdw++] = 0; /* z */;
+   cs->buf[cs->cdw++] = 0; /* z */
cs->buf[cs->cdw++] = addr & 0xfffc;

[Mesa-dev] [PATCH 3/6] gallium: Use unsigned for loop index

[Edward O'Callaghan]

Found-by: Coccinelle
Signed-off-by: Edward O'Callaghan 
---
 src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c 
b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
index 09c1b37..f070ce7 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
@@ -1197,7 +1197,7 @@ get_soa_array_offsets(struct lp_build_context *uint_bld,
 
if (need_perelement_offset) {
   LLVMValueRef pixel_offsets;
-  int i;
+  unsigned int i;
  /* build pixel offset vector: {0, 1, 2, 3, ...} */
   pixel_offsets = uint_bld->undef;
   for (i = 0; i < uint_bld->type.length; i++) {
@@ -1809,7 +1809,7 @@ emit_store_double_chan(struct lp_build_tgsi_context 
*bld_base,
struct gallivm_state *gallivm = bld_base->base.gallivm;
LLVMBuilderRef builder = gallivm->builder;
struct lp_build_context *float_bld = _base->base;
-   int i;
+   unsigned int i;
LLVMValueRef temp, temp2;
LLVMValueRef shuffles[8];
LLVMValueRef shuffles2[8];
@@ -2713,7 +2713,7 @@ static boolean
 near_end_of_shader(struct lp_build_tgsi_soa_context *bld,
int pc)
 {
-   int i;
+   unsigned int i;
 
for (i = 0; i < 5; i++) {
   unsigned opcode;
-- 
2.5.0

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

[Mesa-dev] [PATCH v2 2/3] nvc0: check return value of nvc0_program_validate()

[Samuel Pitoiset]

Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
index 7e2e999..7c3d03e 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
@@ -236,11 +236,8 @@ nvc0_gmtyprog_validate(struct nvc0_context *nvc0)
struct nouveau_pushbuf *push = nvc0->base.pushbuf;
struct nvc0_program *gp = nvc0->gmtyprog;
 
-   if (gp)
-  nvc0_program_validate(nvc0, gp);
-
/* we allow GPs with no code for specifying stream output state only */
-   if (gp && gp->code_size) {
+   if (gp && gp->code_size && nvc0_program_validate(nvc0, gp)) {
   const bool gp_selects_layer = !!(gp->hdr[13] & (1 << 9));
 
   BEGIN_NVC0(push, NVC0_3D(MACRO_GP_SELECT), 1);
-- 
2.6.4

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

Re: [Mesa-dev] [PATCH v2 3/3] nv50: check return value of nouveau_object_new()

[Ilia Mirkin]

Reviewed-by: Ilia Mirkin 

On Mon, Dec 14, 2015 at 11:51 AM, Samuel Pitoiset
 wrote:
> When ret == 0, obj is not NULL. Spotted by Coverity.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/nv50/nv84_video.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv84_video.c 
> b/src/gallium/drivers/nouveau/nv50/nv84_video.c
> index 7a4670f..88655db 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv84_video.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv84_video.c
> @@ -756,8 +756,8 @@ firmware_present(struct pipe_screen *pscreen, enum 
> pipe_video_format codec)
> int present, ret;
>
> if (!FIRMWARE_PRESENT(checked, VP_KERN)) {
> -  nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0, );
> -  if (obj)
> +  ret = nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0, );
> +  if (!ret)
>   screen->firmware_info.profiles_present |= FIRMWARE_VP_KERN;
>nouveau_object_del();
>screen->firmware_info.profiles_checked |= FIRMWARE_VP_KERN;
> @@ -765,8 +765,8 @@ firmware_present(struct pipe_screen *pscreen, enum 
> pipe_video_format codec)
>
> if (codec == PIPE_VIDEO_FORMAT_MPEG4_AVC) {
>if (!FIRMWARE_PRESENT(checked, BSP_KERN)) {
> - nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0, );
> - if (obj)
> + ret = nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0, );
> + if (!ret)
>  screen->firmware_info.profiles_present |= FIRMWARE_BSP_KERN;
>   nouveau_object_del();
>   screen->firmware_info.profiles_checked |= FIRMWARE_BSP_KERN;
> --
> 2.6.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 v2 2/3] nvc0: check return value of nvc0_program_validate()

[Pierre Moreau]

On 11:56 AM - Dec 14 2015, Ilia Mirkin wrote:
> No, gp->code_size is set by the validation. You need to put that last.

IIRC, you can't assume in which order the compiler will decide to evaluate the
different expressions being AND'ed.

> 
> On Mon, Dec 14, 2015 at 11:51 AM, Samuel Pitoiset
>  wrote:
> > Spotted by Coverity.
> >
> > Signed-off-by: Samuel Pitoiset 
> > ---
> >  src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c | 5 +
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c 
> > b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> > index 7e2e999..7c3d03e 100644
> > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> > @@ -236,11 +236,8 @@ nvc0_gmtyprog_validate(struct nvc0_context *nvc0)
> > struct nouveau_pushbuf *push = nvc0->base.pushbuf;
> > struct nvc0_program *gp = nvc0->gmtyprog;
> >
> > -   if (gp)
> > -  nvc0_program_validate(nvc0, gp);
> > -
> > /* we allow GPs with no code for specifying stream output state only */
> > -   if (gp && gp->code_size) {
> > +   if (gp && gp->code_size && nvc0_program_validate(nvc0, gp)) {
> >const bool gp_selects_layer = !!(gp->hdr[13] & (1 << 9));
> >
> >BEGIN_NVC0(push, NVC0_3D(MACRO_GP_SELECT), 1);
> > --
> > 2.6.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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/3] mesa/main: use BITSET_FOREACH_SET in perf_monitor_result_size

[Nicolai Hähnle]

From: Nicolai Hähnle 

This should make the code both faster and slightly clearer.
---
 src/mesa/main/performance_monitor.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/mesa/main/performance_monitor.c 
b/src/mesa/main/performance_monitor.c
index 98dfbea..43529b2 100644
--- a/src/mesa/main/performance_monitor.c
+++ b/src/mesa/main/performance_monitor.c
@@ -591,11 +591,10 @@ perf_monitor_result_size(const struct gl_context *ctx,
 
for (group = 0; group < ctx->PerfMonitor.NumGroups; group++) {
   const struct gl_perf_monitor_group *g = >PerfMonitor.Groups[group];
-  for (counter = 0; counter < g->NumCounters; counter++) {
- const struct gl_perf_monitor_counter *c = >Counters[counter];
+  BITSET_WORD tmp;
 
- if (!BITSET_TEST(m->ActiveCounters[group], counter))
-continue;
+  BITSET_FOREACH_SET(counter, tmp, m->ActiveCounters[group], 
g->NumCounters) {
+ const struct gl_perf_monitor_counter *c = >Counters[counter];
 
  size += sizeof(uint32_t); /* Group ID */
  size += sizeof(uint32_t); /* Counter ID */
-- 
2.5.0

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

[Mesa-dev] [PATCH 4/6] gallium/drivers/ilo: Use unsigned for loop index

[Edward O'Callaghan]

Found-by: Coccinelle
Signed-off-by: Edward O'Callaghan 
---
 src/gallium/drivers/ilo/core/ilo_builder.c   |  8 
 src/gallium/drivers/ilo/shader/ilo_shader_fs.c   | 16 
 src/gallium/drivers/ilo/shader/ilo_shader_vs.c   |  4 ++--
 src/gallium/drivers/ilo/shader/toy_legalize_ra.c |  4 ++--
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/gallium/drivers/ilo/core/ilo_builder.c 
b/src/gallium/drivers/ilo/core/ilo_builder.c
index 9d51951..629f239 100644
--- a/src/gallium/drivers/ilo/core/ilo_builder.c
+++ b/src/gallium/drivers/ilo/core/ilo_builder.c
@@ -333,7 +333,7 @@ ilo_builder_init(struct ilo_builder *builder,
  const struct ilo_dev *dev,
  struct intel_winsys *winsys)
 {
-   int i;
+   unsigned int i;
 
assert(ilo_is_zeroed(builder, sizeof(*builder)));
 
@@ -366,7 +366,7 @@ ilo_builder_init(struct ilo_builder *builder,
 void
 ilo_builder_reset(struct ilo_builder *builder)
 {
-   int i;
+   unsigned int i;
 
for (i = 0; i < ILO_BUILDER_WRITER_COUNT; i++)
   ilo_builder_writer_reset(builder, i);
@@ -382,7 +382,7 @@ ilo_builder_reset(struct ilo_builder *builder)
 bool
 ilo_builder_begin(struct ilo_builder *builder)
 {
-   int i;
+   unsigned int i;
 
for (i = 0; i < ILO_BUILDER_WRITER_COUNT; i++) {
   if (!ilo_builder_writer_alloc_and_map(builder, i)) {
@@ -407,7 +407,7 @@ struct intel_bo *
 ilo_builder_end(struct ilo_builder *builder, unsigned *used)
 {
struct ilo_builder_writer *bat;
-   int i;
+   unsigned int i;
 
ilo_builder_batch_patch_sba(builder);
 
diff --git a/src/gallium/drivers/ilo/shader/ilo_shader_fs.c 
b/src/gallium/drivers/ilo/shader/ilo_shader_fs.c
index 5250115..9a43514 100644
--- a/src/gallium/drivers/ilo/shader/ilo_shader_fs.c
+++ b/src/gallium/drivers/ilo/shader/ilo_shader_fs.c
@@ -266,7 +266,7 @@ fs_lower_opcode_tgsi_indirect_const(struct 
fs_compile_context *fcc,
struct toy_inst *inst;
struct toy_src desc, real_src[4];
struct toy_dst tmp, real_dst[4];
-   int i;
+   unsigned int i;
 
tsrc_transpose(idx, real_src);
 
@@ -319,7 +319,7 @@ fs_lower_opcode_tgsi_const_pcb(struct fs_compile_context 
*fcc,
const int grf_subreg = (idx.val32 & 1) * 16;
struct toy_src src;
struct toy_dst real_dst[4];
-   int i;
+   unsigned int i;
 
if (!fcc->variant->use_pcb || dim != 0 || idx.file != TOY_FILE_IMM ||
grf >= fcc->first_attr_grf)
@@ -350,7 +350,7 @@ fs_lower_opcode_tgsi_const_gen6(struct fs_compile_context 
*fcc,
struct toy_inst *inst;
struct toy_src desc;
struct toy_dst tmp, real_dst[4];
-   int i;
+   unsigned int i;
 
if (fs_lower_opcode_tgsi_const_pcb(fcc, dst, dim, idx))
   return;
@@ -396,7 +396,7 @@ fs_lower_opcode_tgsi_const_gen7(struct fs_compile_context 
*fcc,
struct toy_src desc;
struct toy_inst *inst;
struct toy_dst tmp, real_dst[4];
-   int i;
+   unsigned int i;
 
if (fs_lower_opcode_tgsi_const_pcb(fcc, dst, dim, idx))
   return;
@@ -1168,7 +1168,7 @@ fs_lower_opcode_derivative(struct toy_compiler *tc, 
struct toy_inst *inst)
 {
struct toy_dst dst[4];
struct toy_src src[4];
-   int i;
+   unsigned int i;
 
tdst_transpose(inst->dst, dst);
tsrc_transpose(inst->src[0], src);
@@ -1257,7 +1257,7 @@ fs_lower_opcode_kil(struct toy_compiler *tc, struct 
toy_inst *inst)
}
else {
   struct toy_src src[4];
-  int i;
+  unsigned int i;
 
   tsrc_transpose(inst->src[0], src);
   /* mask out killed pixels */
@@ -1583,7 +1583,7 @@ fs_write_fb(struct fs_compile_context *fcc)
 static void
 fs_setup_shader_out(struct ilo_shader *sh, const struct toy_tgsi *tgsi)
 {
-   int i;
+   unsigned int i;
 
sh->out.count = tgsi->num_outputs;
for (i = 0; i < tgsi->num_outputs; i++) {
@@ -1603,7 +1603,7 @@ static void
 fs_setup_shader_in(struct ilo_shader *sh, const struct toy_tgsi *tgsi,
bool flatshade)
 {
-   int i;
+   unsigned int i;
 
sh->in.count = tgsi->num_inputs;
for (i = 0; i < tgsi->num_inputs; i++) {
diff --git a/src/gallium/drivers/ilo/shader/ilo_shader_vs.c 
b/src/gallium/drivers/ilo/shader/ilo_shader_vs.c
index 46a7e6f..73cccb2 100644
--- a/src/gallium/drivers/ilo/shader/ilo_shader_vs.c
+++ b/src/gallium/drivers/ilo/shader/ilo_shader_vs.c
@@ -522,7 +522,7 @@ vs_prepare_tgsi_sampling(struct vs_compile_context *vcc,
   if (num_coords >= 3) {
  struct toy_dst tmp, max;
  struct toy_src abs_coords[3];
- int i;
+ unsigned int i;
 
  tmp = tc_alloc_tmp(tc);
  max = tdst_writemask(tmp, TOY_WRITEMASK_W);
@@ -804,7 +804,7 @@ static int
 vs_collect_outputs(struct vs_compile_context *vcc, struct toy_src *outs)
 {
const struct toy_tgsi *tgsi = >tgsi;
-   int i;
+   unsigned int i;
 
for (i = 0; i < vcc->shader->out.count; i++) {
   const int slot = vcc->output_map[i];
diff --git a/src/gallium/drivers/ilo/shader/toy_legalize_ra.c 

Re: [Mesa-dev] [PATCH v2 2/3] nvc0: check return value of nvc0_program_validate()

[Ilia Mirkin]

On Mon, Dec 14, 2015 at 11:59 AM, Pierre Moreau  wrote:
> On 11:56 AM - Dec 14 2015, Ilia Mirkin wrote:
>> No, gp->code_size is set by the validation. You need to put that last.
>
> IIRC, you can't assume in which order the compiler will decide to evaluate the
> different expressions being AND'ed.

Actually you can. It's known as "short-circuiting".

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

Re: [Mesa-dev] [PATCH v3] nvc0: check return value of nvc0_program_validate()

[Ilia Mirkin]

Reviewed-by: Ilia Mirkin 

On Mon, Dec 14, 2015 at 12:07 PM, Samuel Pitoiset
 wrote:
> Spotted by Coverity.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> index 7e2e999..5e84ca9 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
> @@ -236,11 +236,8 @@ nvc0_gmtyprog_validate(struct nvc0_context *nvc0)
> struct nouveau_pushbuf *push = nvc0->base.pushbuf;
> struct nvc0_program *gp = nvc0->gmtyprog;
>
> -   if (gp)
> -  nvc0_program_validate(nvc0, gp);
> -
> /* we allow GPs with no code for specifying stream output state only */
> -   if (gp && gp->code_size) {
> +   if (gp && nvc0_program_validate(nvc0, gp) && gp->code_size) {
>const bool gp_selects_layer = !!(gp->hdr[13] & (1 << 9));
>
>BEGIN_NVC0(push, NVC0_3D(MACRO_GP_SELECT), 1);
> --
> 2.6.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

[Mesa-dev] [PATCH 5/6] gallium/drivers/r600: Use unsigned for loop index

[Edward O'Callaghan]

Found-by: Coccinelle
Signed-off-by: Edward O'Callaghan 
---
 src/gallium/drivers/r600/r600_shader.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/gallium/drivers/r600/r600_shader.c 
b/src/gallium/drivers/r600/r600_shader.c
index db53eb4..e7932bf 100644
--- a/src/gallium/drivers/r600/r600_shader.c
+++ b/src/gallium/drivers/r600/r600_shader.c
@@ -394,7 +394,7 @@ static int tgsi_last_instruction(unsigned writemask)
 static int tgsi_is_supported(struct r600_shader_ctx *ctx)
 {
struct tgsi_full_instruction *i = >parse.FullToken.FullInstruction;
-   int j;
+   unsigned int j;
 
if (i->Instruction.NumDstRegs > 1 && i->Instruction.Opcode != 
TGSI_OPCODE_DFRACEXP) {
R600_ERR("too many dst (%d)\n", i->Instruction.NumDstRegs);
@@ -1166,7 +1166,7 @@ static int allocate_system_value_inputs(struct 
r600_shader_ctx *ctx, int gpr_off
 */
 static int evergreen_gpr_count(struct r600_shader_ctx *ctx)
 {
-   int i;
+   unsigned int i;
int num_baryc;
struct tgsi_parse_context parse;
 
@@ -1585,7 +1585,7 @@ static int fetch_gs_input(struct r600_shader_ctx *ctx, 
struct tgsi_full_src_regi
 static int tgsi_split_gs_inputs(struct r600_shader_ctx *ctx)
 {
struct tgsi_full_instruction *inst = 
>parse.FullToken.FullInstruction;
-   int i;
+   unsigned int i;
 
for (i = 0; i < inst->Instruction.NumSrcRegs; i++) {
struct tgsi_full_src_register *src = >Src[i];
@@ -1854,7 +1854,7 @@ static int fetch_tcs_output(struct r600_shader_ctx *ctx, 
struct tgsi_full_src_re
 static int tgsi_split_lds_inputs(struct r600_shader_ctx *ctx)
 {
struct tgsi_full_instruction *inst = 
>parse.FullToken.FullInstruction;
-   int i;
+   unsigned int i;
 
for (i = 0; i < inst->Instruction.NumSrcRegs; i++) {
struct tgsi_full_src_register *src = >Src[i];
@@ -2784,7 +2784,7 @@ static int r600_tess_factor_read(struct r600_shader_ctx 
*ctx,
 
 static int r600_emit_tess_factor(struct r600_shader_ctx *ctx)
 {
-   int i;
+   unsigned int i;
int stride, outer_comps, inner_comps;
int tessinner_idx = -1, tessouter_idx = -1;
int r;
@@ -4791,7 +4791,7 @@ static int tgsi_lit(struct r600_shader_ctx *ctx)
{
int chan;
int sel;
-   int i;
+   unsigned int i;
 
if (ctx->bc->chip_class == CAYMAN) {
for (i = 0; i < 3; i++) {
@@ -7925,7 +7925,7 @@ static int tgsi_exp(struct r600_shader_ctx *ctx)
struct tgsi_full_instruction *inst = 
>parse.FullToken.FullInstruction;
struct r600_bytecode_alu alu;
int r;
-   int i;
+   unsigned int i;
 
/* result.x = 2^floor(src); */
if (inst->Dst[0].Register.WriteMask & 1) {
@@ -8054,7 +8054,7 @@ static int tgsi_log(struct r600_shader_ctx *ctx)
struct tgsi_full_instruction *inst = 
>parse.FullToken.FullInstruction;
struct r600_bytecode_alu alu;
int r;
-   int i;
+   unsigned int i;
 
/* result.x = floor(log2(|src|)); */
if (inst->Dst[0].Register.WriteMask & 1) {
@@ -8781,7 +8781,7 @@ static int tgsi_bgnloop(struct r600_shader_ctx *ctx)
 
 static int tgsi_endloop(struct r600_shader_ctx *ctx)
 {
-   int i;
+   unsigned int i;
 
r600_bytecode_add_cfinst(ctx->bc, CF_OP_LOOP_END);
 
-- 
2.5.0

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

Re: [Mesa-dev] [PATCH 4/5] nv50: check return value of nouveau_object_new()

[Samuel Pitoiset]

On 12/14/2015 05:07 PM, Ilia Mirkin wrote:

On Mon, Dec 14, 2015 at 11:04 AM, Samuel Pitoiset
 wrote:



On 12/14/2015 04:05 PM, Ilia Mirkin wrote:


Bogus. obj will be null on failure.



Well, checking the return value instead of obj seems better tbh, because we
do that in the rest of the code.


Can ret = 0 but obj still be null? If not, feel free to flip it.


No, when ret == 0, object is not NULL. I'll flip that.








On Mon, Dec 14, 2015 at 5:14 AM, Samuel Pitoiset
 wrote:


Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
   src/gallium/drivers/nouveau/nv50/nv84_video.c | 8 
   1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv84_video.c
b/src/gallium/drivers/nouveau/nv50/nv84_video.c
index 7a4670f..5e2489a 100644
--- a/src/gallium/drivers/nouveau/nv50/nv84_video.c
+++ b/src/gallium/drivers/nouveau/nv50/nv84_video.c
@@ -756,8 +756,8 @@ firmware_present(struct pipe_screen *pscreen, enum
pipe_video_format codec)
  int present, ret;

  if (!FIRMWARE_PRESENT(checked, VP_KERN)) {
-  nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0, );
-  if (obj)
+  ret = nouveau_object_new(screen->channel, 0, 0x7476, NULL, 0,
);
+  if (!ret && obj)
screen->firmware_info.profiles_present |= FIRMWARE_VP_KERN;
 nouveau_object_del();
 screen->firmware_info.profiles_checked |= FIRMWARE_VP_KERN;
@@ -765,8 +765,8 @@ firmware_present(struct pipe_screen *pscreen, enum
pipe_video_format codec)

  if (codec == PIPE_VIDEO_FORMAT_MPEG4_AVC) {
 if (!FIRMWARE_PRESENT(checked, BSP_KERN)) {
- nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0, );
- if (obj)
+ ret = nouveau_object_new(screen->channel, 0, 0x74b0, NULL, 0,
);
+ if (!ret && obj)
   screen->firmware_info.profiles_present |=
FIRMWARE_BSP_KERN;
nouveau_object_del();
screen->firmware_info.profiles_checked |= FIRMWARE_BSP_KERN;
--
2.6.4

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



--
-Samuel


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

[Mesa-dev] [PATCH v2 1/3] nv50, nvc0: make use of unreachable() when invalid texture target happens

[Samuel Pitoiset]

Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
 src/gallium/drivers/nouveau/nv50/nv50_tex.c | 3 +--
 src/gallium/drivers/nouveau/nvc0/nvc0_tex.c | 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_tex.c 
b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
index 6083ea9..c3f4336 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_tex.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_tex.c
@@ -192,8 +192,7 @@ nv50_create_texture_view(struct pipe_context *pipe,
   tic[2] |= NV50_TIC_2_TARGET_BUFFER | NV50_TIC_2_LINEAR;
   break;
default:
-  NOUVEAU_ERR("invalid texture target: %d\n", mt->base.base.target);
-  return false;
+  unreachable("unexpected/invalid texture target");
}
 
tic[3] = (flags & NV50_TEXVIEW_FILTER_MSAA8) ? 0x2000 : 0x0030;
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
index 2dd100f..74090ce 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
@@ -193,9 +193,7 @@ nvc0_create_texture_view(struct pipe_context *pipe,
   tic[2] |= NV50_TIC_2_TARGET_CUBE_ARRAY;
   break;
default:
-  NOUVEAU_ERR("unexpected/invalid texture target: %d\n",
-  mt->base.base.target);
-  return false;
+  unreachable("unexpected/invalid texture target");
}
 
tic[3] = (flags & NV50_TEXVIEW_FILTER_MSAA8) ? 0x2000 : 0x0030;
-- 
2.6.4

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

[Mesa-dev] [PATCH 3/3] radeonsi: fix perfcounter selection for SI_PC_MULTI_BLOCK layouts

[Nicolai Hähnle]

From: Nicolai Hähnle 

The incorrectly computed register count caused lockups.
---
 src/gallium/drivers/radeonsi/si_perfcounter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeonsi/si_perfcounter.c 
b/src/gallium/drivers/radeonsi/si_perfcounter.c
index a0ddff6..7ee1dae 100644
--- a/src/gallium/drivers/radeonsi/si_perfcounter.c
+++ b/src/gallium/drivers/radeonsi/si_perfcounter.c
@@ -436,7 +436,7 @@ static void si_pc_emit_select(struct r600_common_context 
*ctx,
 
dw = count + regs->num_prelude;
if (count >= regs->num_multi)
-   count += regs->num_multi;
+   dw += regs->num_multi;
radeon_set_uconfig_reg_seq(cs, regs->select0, dw);
for (idx = 0; idx < regs->num_prelude; ++idx)
radeon_emit(cs, 0);
-- 
2.5.0

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

Re: [Mesa-dev] [PATCH v2 2/3] nvc0: check return value of nvc0_program_validate()

[Samuel Pitoiset]

On 12/14/2015 05:56 PM, Ilia Mirkin wrote:

No, gp->code_size is set by the validation. You need to put that last.


Right, it's set by nvc0_program_translate().



On Mon, Dec 14, 2015 at 11:51 AM, Samuel Pitoiset
 wrote:

Spotted by Coverity.

Signed-off-by: Samuel Pitoiset 
---
  src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c | 5 +
  1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
index 7e2e999..7c3d03e 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_shader_state.c
@@ -236,11 +236,8 @@ nvc0_gmtyprog_validate(struct nvc0_context *nvc0)
 struct nouveau_pushbuf *push = nvc0->base.pushbuf;
 struct nvc0_program *gp = nvc0->gmtyprog;

-   if (gp)
-  nvc0_program_validate(nvc0, gp);
-
 /* we allow GPs with no code for specifying stream output state only */
-   if (gp && gp->code_size) {
+   if (gp && gp->code_size && nvc0_program_validate(nvc0, gp)) {
const bool gp_selects_layer = !!(gp->hdr[13] & (1 << 9));

BEGIN_NVC0(push, NVC0_3D(MACRO_GP_SELECT), 1);
--
2.6.4

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


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