[Mesa-dev] radeonfb: BAR 0: can't reserve [mem 0x98000000-0x9fffffff pref]

2016-06-05 Thread Mathieu Malaterre
Hi,

Before reporting a possible invalid bug report. Does anyone knows why
radeaonfb is not configured the same way radeon is ? For instance on a
PowerPC machine, when Open Firmware Frame Buffer is used (OFfb), I
cannot `modprobe radeonfb` (but I can load `radeon`). It fails with:

[   96.551486] radeonfb :00:10.0: enabling device (0006 -> 0007)
[   96.551526] radeonfb :00:10.0: BAR 0: can't reserve [mem
0x9800-0x9fff pref]
[   96.551531] radeonfb (:00:10.0): cannot request region 0.
[   96.551545] radeonfb: probe of :00:10.0 failed with error -16

It seems (to me) that it should be possible to add something like this
to `radeonfb`:

https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/radeon/radeon_drv.c#L353

Is the above correct ?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] glsl: geom shader max_vertices layout must match.

2016-06-05 Thread Alejandro Piñeiro
On 03/06/16 23:04, Dave Airlie wrote:
> On 4 June 2016 at 03:39, Alejandro Piñeiro  wrote:
>> On 03/06/16 02:46, Dave Airlie wrote:
>>> From: Dave Airlie 
>>>
>>> "all geometry shader output vertex count declarations in a
>>> program must declare the same count."
>> This spec quote lacks context. As far as I see it comes from GLSL 4.5
>> spec, "4.4.2.3 Geometry Outputs".
>>
>>> Fixes:
>>> GL45-CTS.geometry_shader.output.conflicted_output_vertices_max
>> The patch looks good. But testing on Skylake and Broadwell, current
>> master is passing this test without this patch. Did you mean a different
>> test, or you made the test with a different hw?
> It might be a regression since,
> aaa69c79cd584db4d9c6ea7794e93d29f3d54572
>
> Can you confirm you are running on master,

Yes. And just in case, I updated today, and also tried at that commit
point, and just before. In all cases the test is passing on Skylake.

>  some of the CTS results
> I've seen suggest you might not have all the changes.

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


Re: [Mesa-dev] [PATCH] glsl: use enum glsl_interface_packing in more places. (v2)

2016-06-05 Thread Ilia Mirkin
With the const's gone on the return types of the new helpers, this is

Reviewed-by: Ilia Mirkin 

On Mon, Jun 6, 2016 at 12:21 AM, Dave Airlie  wrote:
> From: Dave Airlie 
>
> Although the glsl_types.h stores this in a bitfield,
> we should hide that from everyone else. Hide the cast
> in an accessor method and use the enum everywhere.
>
> This makes things a bit nicer in gdb, and improves type
> safety.
>
> v2: fix a few pieces of interface I missed that caused some
> piglit regressions.
>
> Signed-off-by: Dave Airlie 
> ---
>  src/compiler/glsl/ir.h |  4 
>  .../glsl/link_uniform_block_active_visitor.cpp |  6 ++
>  src/compiler/glsl/link_uniform_blocks.cpp  |  6 +++---
>  src/compiler/glsl/link_uniforms.cpp| 22 
> +++---
>  src/compiler/glsl/linker.h |  8 
>  src/compiler/glsl/lower_buffer_access.cpp  |  2 +-
>  src/compiler/glsl/lower_buffer_access.h|  2 +-
>  src/compiler/glsl/lower_shared_reference.cpp   |  6 +++---
>  src/compiler/glsl/lower_ubo_reference.cpp  | 16 
>  src/compiler/glsl/opt_dead_code.cpp|  2 +-
>  src/compiler/glsl_types.h  |  8 
>  11 files changed, 46 insertions(+), 36 deletions(-)
>
> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
> index 93716c4..b4c7ba2 100644
> --- a/src/compiler/glsl/ir.h
> +++ b/src/compiler/glsl/ir.h
> @@ -537,6 +537,10 @@ public:
>return this->interface_type;
> }
>
> +   const enum glsl_interface_packing get_interface_type_packing() const
> +   {
> + return this->interface_type->get_interface_packing();
> +   }
> /**
>  * Get the max_ifc_array_access pointer
>  *
> diff --git a/src/compiler/glsl/link_uniform_block_active_visitor.cpp 
> b/src/compiler/glsl/link_uniform_block_active_visitor.cpp
> index 54fea70..df8b221 100644
> --- a/src/compiler/glsl/link_uniform_block_active_visitor.cpp
> +++ b/src/compiler/glsl/link_uniform_block_active_visitor.cpp
> @@ -167,8 +167,7 @@ link_uniform_block_active_visitor::visit(ir_variable *var)
>  * also considered active, even if no member of the block is
>  * referenced."
>  */
> -   if (var->get_interface_type()->interface_packing ==
> -   GLSL_INTERFACE_PACKING_PACKED)
> +   if (var->get_interface_type_packing() == GLSL_INTERFACE_PACKING_PACKED)
>return visit_continue;
>
> /* Process the block.  Bail if there was an error.
> @@ -258,8 +257,7 @@ 
> link_uniform_block_active_visitor::visit_enter(ir_dereference_array *ir)
>  * std140 layout qualifier, all its instances have been already marked
>  * as used in link_uniform_block_active_visitor::visit(ir_variable *).
>  */
> -   if (var->get_interface_type()->interface_packing ==
> -   GLSL_INTERFACE_PACKING_PACKED) {
> +   if (var->get_interface_type_packing() == GLSL_INTERFACE_PACKING_PACKED) {
>b->var = var;
>process_arrays(this->mem_ctx, ir, b);
> }
> diff --git a/src/compiler/glsl/link_uniform_blocks.cpp 
> b/src/compiler/glsl/link_uniform_blocks.cpp
> index 3c2d13c..b816dab 100644
> --- a/src/compiler/glsl/link_uniform_blocks.cpp
> +++ b/src/compiler/glsl/link_uniform_blocks.cpp
> @@ -70,7 +70,7 @@ private:
> }
>
> virtual void enter_record(const glsl_type *type, const char *,
> - bool row_major, const unsigned packing) {
> + bool row_major, const enum 
> glsl_interface_packing packing) {
>assert(type->is_record());
>if (packing == GLSL_INTERFACE_PACKING_STD430)
>   this->offset = glsl_align(
> @@ -81,7 +81,7 @@ private:
> }
>
> virtual void leave_record(const glsl_type *type, const char *,
> - bool row_major, const unsigned packing) {
> + bool row_major, const enum 
> glsl_interface_packing packing) {
>assert(type->is_record());
>
>/* If this is the last field of a structure, apply rule #9.  The
> @@ -106,7 +106,7 @@ private:
>
> virtual void visit_field(const glsl_type *type, const char *name,
>  bool row_major, const glsl_type *,
> -const unsigned packing,
> +const enum glsl_interface_packing packing,
>  bool last_field)
> {
>assert(this->index < this->num_variables);
> diff --git a/src/compiler/glsl/link_uniforms.cpp 
> b/src/compiler/glsl/link_uniforms.cpp
> index ff2989f..3a2ac4d 100644
> --- a/src/compiler/glsl/link_uniforms.cpp
> +++ b/src/compiler/glsl/link_uniforms.cpp
> @@ -65,7 +65,7 @@ program_resource_visitor::process(const glsl_type *type, 
> const char *name)
>
> unsigned record_array_count = 1;
> char *name_copy = ralloc_strdup(NULL, name);
> -   unsigned packing = type->interface_packing;
> +   enum glsl_interface_packing packing = type

[Mesa-dev] [PATCH] glsl: use enum glsl_interface_packing in more places. (v2)

2016-06-05 Thread Dave Airlie
From: Dave Airlie 

Although the glsl_types.h stores this in a bitfield,
we should hide that from everyone else. Hide the cast
in an accessor method and use the enum everywhere.

This makes things a bit nicer in gdb, and improves type
safety.

v2: fix a few pieces of interface I missed that caused some
piglit regressions.

Signed-off-by: Dave Airlie 
---
 src/compiler/glsl/ir.h |  4 
 .../glsl/link_uniform_block_active_visitor.cpp |  6 ++
 src/compiler/glsl/link_uniform_blocks.cpp  |  6 +++---
 src/compiler/glsl/link_uniforms.cpp| 22 +++---
 src/compiler/glsl/linker.h |  8 
 src/compiler/glsl/lower_buffer_access.cpp  |  2 +-
 src/compiler/glsl/lower_buffer_access.h|  2 +-
 src/compiler/glsl/lower_shared_reference.cpp   |  6 +++---
 src/compiler/glsl/lower_ubo_reference.cpp  | 16 
 src/compiler/glsl/opt_dead_code.cpp|  2 +-
 src/compiler/glsl_types.h  |  8 
 11 files changed, 46 insertions(+), 36 deletions(-)

diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
index 93716c4..b4c7ba2 100644
--- a/src/compiler/glsl/ir.h
+++ b/src/compiler/glsl/ir.h
@@ -537,6 +537,10 @@ public:
   return this->interface_type;
}
 
+   const enum glsl_interface_packing get_interface_type_packing() const
+   {
+ return this->interface_type->get_interface_packing();
+   }
/**
 * Get the max_ifc_array_access pointer
 *
diff --git a/src/compiler/glsl/link_uniform_block_active_visitor.cpp 
b/src/compiler/glsl/link_uniform_block_active_visitor.cpp
index 54fea70..df8b221 100644
--- a/src/compiler/glsl/link_uniform_block_active_visitor.cpp
+++ b/src/compiler/glsl/link_uniform_block_active_visitor.cpp
@@ -167,8 +167,7 @@ link_uniform_block_active_visitor::visit(ir_variable *var)
 * also considered active, even if no member of the block is
 * referenced."
 */
-   if (var->get_interface_type()->interface_packing ==
-   GLSL_INTERFACE_PACKING_PACKED)
+   if (var->get_interface_type_packing() == GLSL_INTERFACE_PACKING_PACKED)
   return visit_continue;
 
/* Process the block.  Bail if there was an error.
@@ -258,8 +257,7 @@ 
link_uniform_block_active_visitor::visit_enter(ir_dereference_array *ir)
 * std140 layout qualifier, all its instances have been already marked
 * as used in link_uniform_block_active_visitor::visit(ir_variable *).
 */
-   if (var->get_interface_type()->interface_packing ==
-   GLSL_INTERFACE_PACKING_PACKED) {
+   if (var->get_interface_type_packing() == GLSL_INTERFACE_PACKING_PACKED) {
   b->var = var;
   process_arrays(this->mem_ctx, ir, b);
}
diff --git a/src/compiler/glsl/link_uniform_blocks.cpp 
b/src/compiler/glsl/link_uniform_blocks.cpp
index 3c2d13c..b816dab 100644
--- a/src/compiler/glsl/link_uniform_blocks.cpp
+++ b/src/compiler/glsl/link_uniform_blocks.cpp
@@ -70,7 +70,7 @@ private:
}
 
virtual void enter_record(const glsl_type *type, const char *,
- bool row_major, const unsigned packing) {
+ bool row_major, const enum glsl_interface_packing 
packing) {
   assert(type->is_record());
   if (packing == GLSL_INTERFACE_PACKING_STD430)
  this->offset = glsl_align(
@@ -81,7 +81,7 @@ private:
}
 
virtual void leave_record(const glsl_type *type, const char *,
- bool row_major, const unsigned packing) {
+ bool row_major, const enum glsl_interface_packing 
packing) {
   assert(type->is_record());
 
   /* If this is the last field of a structure, apply rule #9.  The
@@ -106,7 +106,7 @@ private:
 
virtual void visit_field(const glsl_type *type, const char *name,
 bool row_major, const glsl_type *,
-const unsigned packing,
+const enum glsl_interface_packing packing,
 bool last_field)
{
   assert(this->index < this->num_variables);
diff --git a/src/compiler/glsl/link_uniforms.cpp 
b/src/compiler/glsl/link_uniforms.cpp
index ff2989f..3a2ac4d 100644
--- a/src/compiler/glsl/link_uniforms.cpp
+++ b/src/compiler/glsl/link_uniforms.cpp
@@ -65,7 +65,7 @@ program_resource_visitor::process(const glsl_type *type, 
const char *name)
 
unsigned record_array_count = 1;
char *name_copy = ralloc_strdup(NULL, name);
-   unsigned packing = type->interface_packing;
+   enum glsl_interface_packing packing = type->get_interface_packing();
 
recursion(type, &name_copy, strlen(name), false, NULL, packing, false,
  record_array_count, NULL);
@@ -79,9 +79,9 @@ program_resource_visitor::process(ir_variable *var)
const bool row_major =
   var->data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR;
 
-   const unsigned packing = var->get_interface_type() 

Re: [Mesa-dev] [PATCH v2] i965: Fix point size with tessellation/geometry shaders in GLES.

2016-06-05 Thread Kenneth Graunke
On Friday, June 3, 2016 4:40:46 PM PDT Ilia Mirkin wrote:
> On Fri, Jun 3, 2016 at 4:08 AM, Kenneth Graunke  wrote:
> > Our previous code worked for desktop GL, and ES without geometry or
> > tessellation shaders.  But those features require fancier point size
> > handling.  Fortunately, we can use one rule for all APIs.
> >
> > Fixes a number of dEQP tests with EXT_tessellation_shader enabled:
> > dEQP-GLES31.functional.tessellation_geometry_interaction.point_size.*
> >
> > Cc: Ilia Mirkin 
> > Signed-off-by: Kenneth Graunke 
> > ---
> >  src/mesa/drivers/dri/i965/brw_state.h | 49 
> > +++
> >  src/mesa/drivers/dri/i965/gen6_sf_state.c |  5 ++--
> >  src/mesa/drivers/dri/i965/gen7_sf_state.c |  7 +++--
> >  src/mesa/drivers/dri/i965/gen8_sf_state.c |  7 +++--
> >  4 files changed, 59 insertions(+), 9 deletions(-)
> >
> > Hey Ilia,
> >
> > Thanks for helping me figure out these rules on IRC yesterday.
> > I think this version should work...
> >
> >  --Ken
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_state.h 
> > b/src/mesa/drivers/dri/i965/brw_state.h
> > index 0a4c21f..be7f6ce 100644
> > --- a/src/mesa/drivers/dri/i965/brw_state.h
> > +++ b/src/mesa/drivers/dri/i965/brw_state.h
> > @@ -473,6 +473,55 @@ is_drawing_lines(const struct brw_context *brw)
> > return false;
> >  }
> >
> > +static inline bool
> > +use_state_point_size(const struct brw_context *brw)
> > +{
> > +   const struct gl_context *ctx = &brw->ctx;
> > +
> > +   /* Section 14.4 (Points) of the OpenGL 4.5 specification says:
> > +*
> > +*"If program point size mode is enabled, the derived point size is
> > +* taken from the (potentially clipped) shader built-in gl_PointSize
> > +* written by:
> > +*
> > +** the geometry shader, if active;
> > +** the tessellation evaluation shader, if active and no
> > +*  geometry shader is active;
> > +** the vertex shader, otherwise
> > +*
> > +*and clamped to the implementation-dependent point size range.  If
> > +*the value written to gl_PointSize is less than or equal to zero,
> > +*or if no value was written to gl_PointSize, results are undefined.
> > +*If program point size mode is disabled, the derived point size is
> > +*specified with the command
> > +*
> > +*   void PointSize(float size);
> > +*
> > +*size specifies the requested size of a point.  The default value
> > +*is 1.0."
> > +*
> > +* The rules for GLES come from the ES 3.2, OES_geometry_point_size, and
> > +* OES_tessellation_point_size specifications.  To summarize: if the 
> > last
> > +* stage before rasterization is a GS or TES, then use gl_PointSize from
> > +* the shader if written.  Otherwise, use 1.0.  If the last stage is a
> > +* vertex shader, use gl_PointSize, or it is undefined.
> > +*
> > +* We can combine these rules into a single condition for both APIs.
> > +* Using the state point size when the last shader stage doesn't write
> > +* gl_PointSize satisfies GL's requirements, as it's undefined.  Because
> > +* ES doesn't have a PointSize() command, the state point size will
> > +* remain 1.0, satisfying the ES default value in the GS/TES case, and
> > +* the VS case (1.0 works for "undefined").  Mesa sets the program point
> > +* mode flag to always-enabled in ES, so we can safely check that, and
> > +* it'll be ignored for ES.
> > +*
> > +* _NEW_PROGRAM | _NEW_POINT
> > +* BRW_NEW_VUE_MAP_GEOM_OUT
> > +*/
> > +   return (!ctx->VertexProgram.PointSizeEnabled && 
> > !ctx->Point._Attenuated) ||
> > +  (brw->vue_map_geom_out.slots_valid & VARYING_BIT_PSIZ) == 0;
> 
> I believe that this is "overly" nice to buggy programs, but not
> incorrect. The cases where this has to return true:
> 
> (a) point size is explicitly disabled
> (b) ES2 context, and point size isn't emitted in TES/GS
> 
> And it has to return false when point size is enabled && "last stage"
> emits a point size. In all other cases, you can do whatever. I believe
> you're choosing to be nice and use the fixed function point size if a
> point size isn't being emitted, which is fine but unnecessary.

Right.  I decided to use the API-specified point size in a few undefined
cases because I figured it would actually take more code to be meaner.

> Dunno what the _Attenuated thing is, but it was like that before too,
> so I assume you know what you're doing.

Apparently there's a GL_DISTANCE_ATTENUATION_EXT point parameter, where
you can set three values, and it varies the point size based on the
distance.  We emulate this in ffvertex_prog.c.

> In principle this makes sense to me, but I don't know what
> vue_map_geom_out actually is, so I'm going to leave the actual review
> to someone who knows the driver better.
> 
>   -ilia

vue_map_geom_out is just a data structure representing the o

Re: [Mesa-dev] [PATCH 0/5] GL_ARB_shader_group_vote support

2016-06-05 Thread Dave Airlie
On 6 June 2016 at 11:47, Ilia Mirkin  wrote:
> ping
>
> On Sun, May 29, 2016 at 2:01 PM, Ilia Mirkin  wrote:
>> Turns out Matt already did a bunch of this work last year, but since I
>> only found out about that after I wrote my patches, I decided to keep
>> mine in place (I like the "vote" naming better TBH). This passes the
>> very simple piglit tests I just sent out to the list.

For the first 4,

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


Re: [Mesa-dev] [PATCH v2] i965: Fix point size with tessellation/geometry shaders in GLES.

2016-06-05 Thread Michael Schellenberger Costa
Hi Kenneth,

Am 03.06.2016 um 10:08 schrieb Kenneth Graunke:
> Our previous code worked for desktop GL, and ES without geometry or
> tessellation shaders.  But those features require fancier point size
> handling.  Fortunately, we can use one rule for all APIs.
> 
> Fixes a number of dEQP tests with EXT_tessellation_shader enabled:
> dEQP-GLES31.functional.tessellation_geometry_interaction.point_size.*
> 
> Cc: Ilia Mirkin 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_state.h | 49 
> +++
>  src/mesa/drivers/dri/i965/gen6_sf_state.c |  5 ++--
>  src/mesa/drivers/dri/i965/gen7_sf_state.c |  7 +++--
>  src/mesa/drivers/dri/i965/gen8_sf_state.c |  7 +++--
>  4 files changed, 59 insertions(+), 9 deletions(-)
> 
> Hey Ilia,
> 
> Thanks for helping me figure out these rules on IRC yesterday.
> I think this version should work...
> 
>  --Ken
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h 
> b/src/mesa/drivers/dri/i965/brw_state.h
> index 0a4c21f..be7f6ce 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -473,6 +473,55 @@ is_drawing_lines(const struct brw_context *brw)
> return false;
>  }
>  
> +static inline bool
> +use_state_point_size(const struct brw_context *brw)
> +{
> +   const struct gl_context *ctx = &brw->ctx;
> +
> +   /* Section 14.4 (Points) of the OpenGL 4.5 specification says:
> +*
> +*"If program point size mode is enabled, the derived point size is
> +* taken from the (potentially clipped) shader built-in gl_PointSize
> +* written by:
> +*
> +** the geometry shader, if active;
> +** the tessellation evaluation shader, if active and no
> +*  geometry shader is active;
> +** the vertex shader, otherwise
> +*
> +*and clamped to the implementation-dependent point size range.  If
> +*the value written to gl_PointSize is less than or equal to zero,
> +*or if no value was written to gl_PointSize, results are undefined.
> +*If program point size mode is disabled, the derived point size is
> +*specified with the command
> +*
> +*   void PointSize(float size);
> +*
> +*size specifies the requested size of a point.  The default value
> +*is 1.0."
> +*
> +* The rules for GLES come from the ES 3.2, OES_geometry_point_size, and
> +* OES_tessellation_point_size specifications.  To summarize: if the last
> +* stage before rasterization is a GS or TES, then use gl_PointSize from
> +* the shader if written.  Otherwise, use 1.0.  If the last stage is a
> +* vertex shader, use gl_PointSize, or it is undefined.
> +*
> +* We can combine these rules into a single condition for both APIs.
> +* Using the state point size when the last shader stage doesn't write
> +* gl_PointSize satisfies GL's requirements, as it's undefined.  Because
> +* ES doesn't have a PointSize() command, the state point size will
> +* remain 1.0, satisfying the ES default value in the GS/TES case, and
> +* the VS case (1.0 works for "undefined").  Mesa sets the program point
> +* mode flag to always-enabled in ES, so we can safely check that, and
> +* it'll be ignored for ES.
> +*
> +* _NEW_PROGRAM | _NEW_POINT
> +* BRW_NEW_VUE_MAP_GEOM_OUT
> +*/
> +   return (!ctx->VertexProgram.PointSizeEnabled && !ctx->Point._Attenuated) 
> ||
> +  (brw->vue_map_geom_out.slots_valid & VARYING_BIT_PSIZ) == 0;
That last condition looks decidedly weird. Are those brackets correct?
--Michael
> +}
> +
>  
>  #ifdef __cplusplus
>  }
> diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c 
> b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> index 0538ab7..94731e0 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
> @@ -373,9 +373,8 @@ upload_sf_state(struct brw_context *brw)
> if (multisampled_fbo && ctx->Multisample.Enabled)
>dw3 |= GEN6_SF_MSRAST_ON_PATTERN;
>  
> -   /* _NEW_PROGRAM | _NEW_POINT */
> -   if (!(ctx->VertexProgram.PointSizeEnabled ||
> -  ctx->Point._Attenuated))
> +   /* _NEW_PROGRAM | _NEW_POINT, BRW_NEW_VUE_MAP_GEOM_OUT */
> +   if (use_state_point_size(brw))
>dw4 |= GEN6_SF_USE_STATE_POINT_WIDTH;
>  
> /* _NEW_POINT - Clamp to ARB_point_parameters user limits */
> diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c 
> b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> index d3a658c..8d49e24 100644
> --- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> @@ -213,8 +213,8 @@ upload_sf_state(struct brw_context *brw)
>  
> dw3 = GEN6_SF_LINE_AA_MODE_TRUE;
>  
> -   /* _NEW_PROGRAM | _NEW_POINT */
> -   if (!(ctx->VertexProgram.PointSizeEnabled || ctx->Point._Attenuated))
> +   /* _NEW_PROGRAM | _NEW_POINT, BRW_NEW_VUE_MAP_GEOM_OUT */
> +   if (use_state_point_size(b

Re: [Mesa-dev] [PATCH 0/5] GL_ARB_shader_group_vote support

2016-06-05 Thread Ilia Mirkin
ping

On Sun, May 29, 2016 at 2:01 PM, Ilia Mirkin  wrote:
> Turns out Matt already did a bunch of this work last year, but since I
> only found out about that after I wrote my patches, I decided to keep
> mine in place (I like the "vote" naming better TBH). This passes the
> very simple piglit tests I just sent out to the list.
>
> Ilia Mirkin (5):
>   mesa: hook up core bits of GL_ARB_shader_group_vote
>   gallium: add VOTE_* opcodes to implement GL_ARB_shader_group_vote
>   gallium: add PIPE_CAP_TGSI_VOTE for when the VOTE ops are allowed
>   st/mesa: expose GL_ARB_shader_group_vote when supported by backend
>   nvc0: add support for VOTE tgsi opcodes
>
>  src/compiler/glsl/builtin_functions.cpp| 22 ++
>  src/compiler/glsl/glcpp/glcpp-parse.y  |  3 +++
>  src/compiler/glsl/glsl_parser_extras.cpp   |  1 +
>  src/compiler/glsl/glsl_parser_extras.h |  2 ++
>  src/compiler/glsl/ir.cpp   |  6 +
>  src/compiler/glsl/ir.h |  9 +++-
>  src/compiler/glsl/ir_validate.cpp  |  8 +++
>  src/gallium/auxiliary/tgsi/tgsi_info.c |  3 +++
>  src/gallium/docs/source/screen.rst |  1 +
>  src/gallium/docs/source/tgsi.rst   | 17 ++
>  src/gallium/drivers/freedreno/freedreno_screen.c   |  1 +
>  src/gallium/drivers/i915/i915_screen.c |  1 +
>  src/gallium/drivers/ilo/ilo_screen.c   |  1 +
>  src/gallium/drivers/llvmpipe/lp_screen.c   |  1 +
>  .../drivers/nouveau/codegen/nv50_ir_emit_gk110.cpp | 23 ++
>  .../drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp | 27 
> +-
>  .../drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp  | 23 ++
>  .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp  | 21 +
>  src/gallium/drivers/nouveau/nv30/nv30_screen.c |  1 +
>  src/gallium/drivers/nouveau/nv50/nv50_screen.c |  1 +
>  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c |  4 ++--
>  src/gallium/drivers/r300/r300_screen.c |  1 +
>  src/gallium/drivers/r600/r600_pipe.c   |  1 +
>  src/gallium/drivers/radeonsi/si_pipe.c |  1 +
>  src/gallium/drivers/softpipe/sp_screen.c   |  1 +
>  src/gallium/drivers/svga/svga_screen.c |  1 +
>  src/gallium/drivers/swr/swr_screen.cpp |  1 +
>  src/gallium/drivers/vc4/vc4_screen.c   |  1 +
>  src/gallium/drivers/virgl/virgl_screen.c   |  1 +
>  src/gallium/include/pipe/p_defines.h   |  1 +
>  src/gallium/include/pipe/p_shader_tokens.h |  7 +-
>  .../dri/i965/brw_fs_channel_expressions.cpp|  5 
>  src/mesa/main/extensions_table.h   |  1 +
>  src/mesa/main/mtypes.h |  1 +
>  src/mesa/program/ir_to_mesa.cpp|  3 +++
>  src/mesa/state_tracker/st_extensions.c |  1 +
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 10 
>  37 files changed, 190 insertions(+), 23 deletions(-)
>
> --
> 2.7.3
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] glsl: for anonymous struct matching use without_array() (v3)

2016-06-05 Thread Ilia Mirkin
On Sun, Jun 5, 2016 at 9:30 PM, Dave Airlie  wrote:
> From: Dave Airlie 
>
> With tessellation shaders we can have cases where we have
> arrays of anon structs, so make sure we match using without_array().
>
> Fixes:
> GL45-CTS.tessellation_shader.tessellation_control_to_tessellation_evaluation.gl_in
>
> v2:
> test lengths match as well (Ilia)
> v3:
> descend array lengths to check for matches as well (Ilia)
>
> Signed-off-by: Dave Airlie 

This looks good, but I have some petty suggestions which you're free
to incorporate, or not.

> ---
>  src/compiler/glsl/link_varyings.cpp | 29 ++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/src/compiler/glsl/link_varyings.cpp 
> b/src/compiler/glsl/link_varyings.cpp
> index a286e77..1392095 100644
> --- a/src/compiler/glsl/link_varyings.cpp
> +++ b/src/compiler/glsl/link_varyings.cpp
> @@ -182,6 +182,31 @@ process_xfb_layout_qualifiers(void *mem_ctx, const 
> gl_shader *sh,
> return has_xfb_qualifiers;
>  }
>
> +static bool
> +anonymous_struct_type_matches(const glsl_type *output_type,
> +  const glsl_type *to_match)
> +{
> +const glsl_type *output_tmp = output_type;
> +const glsl_type *to_match_tmp = to_match;

I'd personally not even have the _tmp vars, just use the args.

> +
> +while (output_tmp) {
> +if (output_tmp->is_array() && to_match_tmp->is_array()) {

You could just do

while (output_tmp->is_array() && .. ) {

I don't think output_tmp == null can ever happen.

> +/* if the lengths at each level don't match fail. */
> +if (output_tmp->length != to_match_tmp->length)
> +return false;
> +output_tmp = output_tmp->fields.array;
> +to_match_tmp = to_match_tmp->fields.array;
> +} else {

And then this would be outside of the while loop. IMHO it closer
matches the flow of the code. Your call. Either way,

Reviewed-by: Ilia Mirkin 

> +if (output_tmp->is_array() || to_match_tmp->is_array())
> +return false;
> +return output_tmp->is_anonymous() &&
> +   to_match_tmp->is_anonymous() &&
> +   to_match_tmp->record_compare(output_tmp);
> +}
> +}
> +return false;
> +}
> +
>  /**
>   * Validate the types and qualifiers of an output from one stage against the
>   * matching input to another stage.
> @@ -226,9 +251,7 @@ cross_validate_types_and_qualifiers(struct 
> gl_shader_program *prog,
> * fragment language."
> */
>if (!output->type->is_array() || !is_gl_identifier(output->name)) {
> - bool anon_matches = output->type->is_anonymous() &&
> -type_to_match->is_anonymous() &&
> -type_to_match->record_compare(output->type);
> + bool anon_matches = anonymous_struct_type_matches(output->type, 
> type_to_match);
>
>   if (!anon_matches) {
>  linker_error(prog,
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] glsl: for anonymous struct matching use without_array() (v3)

2016-06-05 Thread Dave Airlie
From: Dave Airlie 

With tessellation shaders we can have cases where we have
arrays of anon structs, so make sure we match using without_array().

Fixes:
GL45-CTS.tessellation_shader.tessellation_control_to_tessellation_evaluation.gl_in

v2:
test lengths match as well (Ilia)
v3:
descend array lengths to check for matches as well (Ilia)

Signed-off-by: Dave Airlie 
---
 src/compiler/glsl/link_varyings.cpp | 29 ++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index a286e77..1392095 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -182,6 +182,31 @@ process_xfb_layout_qualifiers(void *mem_ctx, const 
gl_shader *sh,
return has_xfb_qualifiers;
 }
 
+static bool
+anonymous_struct_type_matches(const glsl_type *output_type,
+  const glsl_type *to_match)
+{
+const glsl_type *output_tmp = output_type;
+const glsl_type *to_match_tmp = to_match;
+
+while (output_tmp) {
+if (output_tmp->is_array() && to_match_tmp->is_array()) {
+/* if the lengths at each level don't match fail. */
+if (output_tmp->length != to_match_tmp->length)
+return false;
+output_tmp = output_tmp->fields.array;
+to_match_tmp = to_match_tmp->fields.array;
+} else {
+if (output_tmp->is_array() || to_match_tmp->is_array())
+return false;
+return output_tmp->is_anonymous() &&
+   to_match_tmp->is_anonymous() &&
+   to_match_tmp->record_compare(output_tmp);
+}
+}
+return false;
+}
+
 /**
  * Validate the types and qualifiers of an output from one stage against the
  * matching input to another stage.
@@ -226,9 +251,7 @@ cross_validate_types_and_qualifiers(struct 
gl_shader_program *prog,
* fragment language."
*/
   if (!output->type->is_array() || !is_gl_identifier(output->name)) {
- bool anon_matches = output->type->is_anonymous() &&
-type_to_match->is_anonymous() &&
-type_to_match->record_compare(output->type);
+ bool anon_matches = anonymous_struct_type_matches(output->type, 
type_to_match);
 
  if (!anon_matches) {
 linker_error(prog,
-- 
2.7.4

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


[Mesa-dev] [PATCH] glsl: for anonymous struct matching use without_array() (v2)

2016-06-05 Thread Dave Airlie
From: Dave Airlie 

With tessellation shaders we can have cases where we have
arrays of anon structs, so make sure we match using without_array().

Fixes:
GL45-CTS.tessellation_shader.tessellation_control_to_tessellation_evaluation.gl_in

v2:
test lengths match as well (Ilia)

Signed-off-by: Dave Airlie 
---
 src/compiler/glsl/link_varyings.cpp | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index a286e77..2252cad 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -226,9 +226,12 @@ cross_validate_types_and_qualifiers(struct 
gl_shader_program *prog,
* fragment language."
*/
   if (!output->type->is_array() || !is_gl_identifier(output->name)) {
- bool anon_matches = output->type->is_anonymous() &&
-type_to_match->is_anonymous() &&
-type_to_match->record_compare(output->type);
+ const struct glsl_type *to_match_wa = type_to_match->without_array();
+ const struct glsl_type *out_type_wa = output->type->without_array();
+ bool anon_matches = out_type_wa->is_anonymous() &&
+to_match_wa->is_anonymous() &&
+to_match_wa->record_compare(out_type_wa) &&
+(type_to_match->array_size() == output->type->array_size());
 
  if (!anon_matches) {
 linker_error(prog,
-- 
2.7.4

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


[Mesa-dev] [PATCH] glsl: for anonymous struct matching use without_array()

2016-06-05 Thread Dave Airlie
From: Dave Airlie 

With tessellation shaders we can have cases where we have
arrays of anon structs, so make sure we match using without_array().

Fixes:
GL45-CTS.tessellation_shader.tessellation_control_to_tessellation_evaluation.gl_in

Signed-off-by: Dave Airlie 
---
 src/compiler/glsl/link_varyings.cpp | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index a286e77..395fef4 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -226,9 +226,11 @@ cross_validate_types_and_qualifiers(struct 
gl_shader_program *prog,
* fragment language."
*/
   if (!output->type->is_array() || !is_gl_identifier(output->name)) {
- bool anon_matches = output->type->is_anonymous() &&
-type_to_match->is_anonymous() &&
-type_to_match->record_compare(output->type);
+ const struct glsl_type *to_match_wa = type_to_match->without_array();
+ const struct glsl_type *out_type_wa = output->type->without_array();
+ bool anon_matches = out_type_wa->is_anonymous() &&
+to_match_wa->is_anonymous() &&
+to_match_wa->record_compare(out_type_wa);
 
  if (!anon_matches) {
 linker_error(prog,
-- 
2.7.4

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


Re: [Mesa-dev] [PATCH] anv/pipeline: Add support for caching the push constant map

2016-06-05 Thread Kristian Høgsberg
On Sat, Jun 4, 2016 at 3:39 PM, Jason Ekstrand  wrote:
> Signed-off-by: Jason Ekstrand 
> Cc: Kristian Høgsberg Kristensen 

Thanks for adding this, looks good to me. The copying of prog_data in
anv_pipeline_cache_load() is a little fiddly, but I got nothing
better...

Reviewed-by: Kristian Høgsberg 

> ---
>  src/intel/vulkan/anv_pipeline_cache.c | 33 +
>  1 file changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_pipeline_cache.c 
> b/src/intel/vulkan/anv_pipeline_cache.c
> index 62dbe3e..f75c423 100644
> --- a/src/intel/vulkan/anv_pipeline_cache.c
> +++ b/src/intel/vulkan/anv_pipeline_cache.c
> @@ -89,11 +89,15 @@ entry_size(struct cache_entry *entry)
>  * doesn't include the alignment padding bytes.
>  */
>
> +   struct brw_stage_prog_data *prog_data = (void *)entry->prog_data;
> +   const uint32_t param_size =
> +  prog_data->nr_params * sizeof(*prog_data->param);
> +
> const uint32_t map_size =
>entry->surface_count * sizeof(struct anv_pipeline_binding) +
>entry->sampler_count * sizeof(struct anv_pipeline_binding);
>
> -   return sizeof(*entry) + entry->prog_data_size + map_size;
> +   return sizeof(*entry) + entry->prog_data_size + param_size + map_size;
>  }
>
>  void
> @@ -141,6 +145,7 @@ anv_pipeline_cache_search_unlocked(struct 
> anv_pipeline_cache *cache,
>  void *p = entry->prog_data;
>  *prog_data = p;
>  p += entry->prog_data_size;
> +p += (*prog_data)->nr_params * sizeof(*(*prog_data)->param);
>  map->surface_count = entry->surface_count;
>  map->sampler_count = entry->sampler_count;
>  map->image_count = entry->image_count;
> @@ -267,12 +272,18 @@ anv_pipeline_cache_upload_kernel(struct 
> anv_pipeline_cache *cache,
>
> struct cache_entry *entry;
>
> +   assert((*prog_data)->nr_pull_params == 0);
> +   assert((*prog_data)->nr_image_params == 0);
> +
> +   const uint32_t param_size =
> +  (*prog_data)->nr_params * sizeof(*(*prog_data)->param);
> +
> const uint32_t map_size =
>map->surface_count * sizeof(struct anv_pipeline_binding) +
>map->sampler_count * sizeof(struct anv_pipeline_binding);
>
> const uint32_t preamble_size =
> -  align_u32(sizeof(*entry) + prog_data_size + map_size, 64);
> +  align_u32(sizeof(*entry) + prog_data_size + param_size + map_size, 64);
>
> const uint32_t size = preamble_size + kernel_size;
>
> @@ -291,6 +302,10 @@ anv_pipeline_cache_upload_kernel(struct 
> anv_pipeline_cache *cache,
> memcpy(p, *prog_data, prog_data_size);
> p += prog_data_size;
>
> +   memcpy(p, (*prog_data)->param, param_size);
> +   ((struct brw_stage_prog_data *)entry->prog_data)->param = p;
> +   p += param_size;
> +
> memcpy(p, map->surface_to_descriptor,
>map->surface_count * sizeof(struct anv_pipeline_binding));
> map->surface_to_descriptor = p;
> @@ -358,9 +373,17 @@ anv_pipeline_cache_load(struct anv_pipeline_cache *cache,
>struct cache_entry *entry = p;
>
>void *data = entry->prog_data;
> -  const struct brw_stage_prog_data *prog_data = data;
> +
> +  /* Make a copy of prog_data so that it's mutable */
> +  uint8_t prog_data_tmp[512];
> +  assert(entry->prog_data_size <= sizeof(prog_data_tmp));
> +  memcpy(prog_data_tmp, data, entry->prog_data_size);
> +  struct brw_stage_prog_data *prog_data = (void *)prog_data_tmp;
>data += entry->prog_data_size;
>
> +  prog_data->param = data;
> +  data += prog_data->nr_params * sizeof(*prog_data->param);
> +
>struct anv_pipeline_binding *surface_to_descriptor = data;
>data += entry->surface_count * sizeof(struct anv_pipeline_binding);
>struct anv_pipeline_binding *sampler_to_descriptor = data;
> @@ -375,9 +398,11 @@ anv_pipeline_cache_load(struct anv_pipeline_cache *cache,
>   .sampler_to_descriptor = sampler_to_descriptor
>};
>
> +  const struct brw_stage_prog_data *const_prog_data = prog_data;
> +
>anv_pipeline_cache_upload_kernel(cache, entry->sha1,
> kernel, entry->kernel_size,
> -   &prog_data,
> +   &const_prog_data,
> entry->prog_data_size, &map);
>p = kernel + entry->kernel_size;
> }
> --
> 2.5.0.400.gff86faf
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v4] clover: Update OpenCL version string to match OpenGL

2016-06-05 Thread Francisco Jerez
Vedran Miletić  writes:

> Change MESA into Mesa in CL_PLATFORM_VERSION and CL_DEVICE_VERSION. For
> both, always append git version suffix from git_sha1.h.
>
> v4: dropped #ifdef guards.
>
> It would be very nice if this could be considered for 12.0 release.
> ---
>  src/gallium/state_trackers/clover/api/device.cpp   | 5 -
>  src/gallium/state_trackers/clover/api/platform.cpp | 5 -
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/api/device.cpp 
> b/src/gallium/state_trackers/clover/api/device.cpp
> index bc93f91..0d3de45 100644
> --- a/src/gallium/state_trackers/clover/api/device.cpp
> +++ b/src/gallium/state_trackers/clover/api/device.cpp
> @@ -23,6 +23,7 @@
>  #include "api/util.hpp"
>  #include "core/platform.hpp"
>  #include "core/device.hpp"
> +#include "git_sha1.h"
>  
>  using namespace clover;
>  
> @@ -300,7 +301,9 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param,
>break;
>  
> case CL_DEVICE_VERSION:
> -  buf.as_string() = "OpenCL 1.1 MESA " PACKAGE_VERSION;
> +  buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
> +" (" MESA_GIT_SHA1 ")"
> +;

No need to put the semicolon in a separate line.

>break;
>  
> case CL_DEVICE_EXTENSIONS:
> diff --git a/src/gallium/state_trackers/clover/api/platform.cpp 
> b/src/gallium/state_trackers/clover/api/platform.cpp
> index cf71593..1784fbc 100644
> --- a/src/gallium/state_trackers/clover/api/platform.cpp
> +++ b/src/gallium/state_trackers/clover/api/platform.cpp
> @@ -22,6 +22,7 @@
>  
>  #include "api/util.hpp"
>  #include "core/platform.hpp"
> +#include "git_sha1.h"
>  
>  using namespace clover;
>  
> @@ -57,7 +58,9 @@ clover::GetPlatformInfo(cl_platform_id d_platform, 
> cl_platform_info param,
>break;
>  
> case CL_PLATFORM_VERSION:
> -  buf.as_string() = "OpenCL 1.1 MESA " PACKAGE_VERSION;
> +  buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
> +" (" MESA_GIT_SHA1 ")"
> +;

Same here.  With my nitpicks addressed:

Reviewed-by: Francisco Jerez 

>break;
>  
> case CL_PLATFORM_NAME:
> -- 
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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


Re: [Mesa-dev] [PATCH v1.1 2/2] clover: fix getting scalar args api size

2016-06-05 Thread Francisco Jerez
Vedran Miletić  writes:

> On 06/04/2016 04:27 AM, Francisco Jerez wrote:
>> Serge Martin  writes:
>>
>>> This fix getting the size of a struct arg. vec3 types still work ok.
>>> Only buit-in args need to have power of two alignment, getTypeAllocSize
>>> reports the correct size.
>>>
>> Is there any guarantee that the alloc size of the type will match what
>> the CL API expects? (which is the only thing arg_api_size is used for
>> IIRC) Isn't it fully dependent on the data layout which is pretty much
>> up to the target?
>>
>
> It is dependent on the target. The issue is that right now we get the 
> struct size wrongly, as we round it here to the next power of two, store 
> that in scalar arg object, and later compare that rounded value to 
> sizeof(struct_arg_type) which user has passed via the API.
>
I know, but using the target data layout-dependent alloc size of the
type as API-visible size seems pretty bogus too -- Unless we set as
requirement on all targets that their data alignment rules should match
the CL API's exactly, which I don't think is the case right now but
doesn't sound like a completely crazy idea to me.

> Regards,
> Vedran
>
> -- 
> Vedran Miletić
> vedran.miletic.net


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


Re: [Mesa-dev] [PATCH] clover: Pass unquoted compiler arguments to Clang

2016-06-05 Thread Francisco Jerez
Vedran Miletić  writes:

> On 06/04/2016 10:13 AM, Francisco Jerez wrote:
>> Vedran Miletić  writes:
>>
>>> OpenCL apps can quote arguments they pass to the OpenCL compiler, most
>>> commonly include paths containing spaces. If the OpenCL compiler was
>>> called via a shell, the shell would remove (single or double) quotes
>>> before passing the argument to the compiler. Since we call Clang as a
>>> library, we have to remove quotes before passing the argument.
>>
>> Sigh, it's a shame that the OpenCL spec doesn't specify what format the
>> option string should have, whether quotes have any special
>> interpretation at all, whether escape sequences are supported, etc.
>> Might be worth reporting the problem at Khronos -- Or have you found any
>> evidence in the spec that the parsing logic you implement below is the
>> correct one?
>>
>
> Aside from working just like NVIDIA and AMD proprietary stacks, no.
>
> However, what we do right now is most certainly broken. Consider the 
> following argument
>
> -I"/foo bar/baz"
>
> which is going to be sent to Clang as two arguments broken over space, 
> while it should be one.

But the OpenCL spec doesn't impose any explicit requirements on the
formatting of the option string (unless I'm missing something), so
strictly speaking both interpretations are equally broken.  It wasn't my
intention to argue against implementing the same behaviour as the
proprietary CL stacks as temporary solution, but the fact that this is
unspecified is certainly an OpenCL spec bug.

> We should split arguments correctly, and whether or not we remove
> quotes in the process or we expect Clang to handle them is the easier
> part of the job.
>
> Regards,
> Vedran
>
> -- 
> Vedran Miletić
> vedran.miletic.net


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


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

2016-06-05 Thread AppVeyor


Build mesa 1517 completed



Commit f657a59d98 by Kenneth Graunke on 6/5/2016 11:31 PM:

mesa: Try to unbreak the MSVC build.\n\nPATH_MAX is apparently not a thing on Windows.  Borrow the hack from\npipe_loader.c to try and make this work.\n\nSigned-off-by: Kenneth Graunke 


Configure your notification preferences

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


Re: [Mesa-dev] [PATCH v4] clover: Update OpenCL version string to match OpenGL

2016-06-05 Thread Vedran Miletić

On 06/06/2016 12:44 AM, Vedran Miletić wrote:

Change MESA into Mesa in CL_PLATFORM_VERSION and CL_DEVICE_VERSION. For
both, always append git version suffix from git_sha1.h.

v4: dropped #ifdef guards.

It would be very nice if this could be considered for 12.0 release.
---
 src/gallium/state_trackers/clover/api/device.cpp   | 5 -
 src/gallium/state_trackers/clover/api/platform.cpp | 5 -
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/gallium/state_trackers/clover/api/device.cpp 
b/src/gallium/state_trackers/clover/api/device.cpp
index bc93f91..0d3de45 100644
--- a/src/gallium/state_trackers/clover/api/device.cpp
+++ b/src/gallium/state_trackers/clover/api/device.cpp
@@ -23,6 +23,7 @@
 #include "api/util.hpp"
 #include "core/platform.hpp"
 #include "core/device.hpp"
+#include "git_sha1.h"

 using namespace clover;

@@ -300,7 +301,9 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param,
   break;

case CL_DEVICE_VERSION:
-  buf.as_string() = "OpenCL 1.1 MESA " PACKAGE_VERSION;
+  buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
+" (" MESA_GIT_SHA1 ")"
+;
   break;

case CL_DEVICE_EXTENSIONS:
diff --git a/src/gallium/state_trackers/clover/api/platform.cpp 
b/src/gallium/state_trackers/clover/api/platform.cpp
index cf71593..1784fbc 100644
--- a/src/gallium/state_trackers/clover/api/platform.cpp
+++ b/src/gallium/state_trackers/clover/api/platform.cpp
@@ -22,6 +22,7 @@

 #include "api/util.hpp"
 #include "core/platform.hpp"
+#include "git_sha1.h"

 using namespace clover;

@@ -57,7 +58,9 @@ clover::GetPlatformInfo(cl_platform_id d_platform, 
cl_platform_info param,
   break;

case CL_PLATFORM_VERSION:
-  buf.as_string() = "OpenCL 1.1 MESA " PACKAGE_VERSION;
+  buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
+" (" MESA_GIT_SHA1 ")"
+;
   break;

case CL_PLATFORM_NAME:



CC'ing mesa-stable as well.

--
Vedran Miletić
vedran.miletic.net
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Ilia Mirkin
On Sun, Jun 5, 2016 at 6:30 PM, Emil Velikov  wrote:
> On 5 June 2016 at 23:22, Ilia Mirkin  wrote:
>> You're arguing about clarity with the ~2 active developers/reviewers
>> who understand the code.
> What I'm saying is that with contradicting commit messages like this
> one there'll be little others who will understand it :-\

If it were that easy to get people to be interested in development,
I'd write a novel for every commit.

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


Re: [Mesa-dev] [PATCH v1.1 2/2] clover: fix getting scalar args api size

2016-06-05 Thread Vedran Miletić

On 06/04/2016 04:27 AM, Francisco Jerez wrote:

Serge Martin  writes:


This fix getting the size of a struct arg. vec3 types still work ok.
Only buit-in args need to have power of two alignment, getTypeAllocSize
reports the correct size.


Is there any guarantee that the alloc size of the type will match what
the CL API expects? (which is the only thing arg_api_size is used for
IIRC) Isn't it fully dependent on the data layout which is pretty much
up to the target?



It is dependent on the target. The issue is that right now we get the 
struct size wrongly, as we round it here to the next power of two, store 
that in scalar arg object, and later compare that rounded value to 
sizeof(struct_arg_type) which user has passed via the API.


Regards,
Vedran

--
Vedran Miletić
vedran.miletic.net
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] nv50: reinstate dedicated constbuf push path

2016-06-05 Thread Ilia Mirkin
This was disabled due to occasionally incorrect behavior when trying to
upload data. It later became apparent that nvc0 also had a similar but
slightly different issue, which was resolved in commit e50c01d5. This
takes the same logic as nvc0 and applies it to nv50 (which has somewhat
different interfaces).

Unfortunately I did not note down precisely what was broken with UBOs
when removing the support from nv50, but I've tested a bunch of local
traces, and none of them appear to regress. This should hopefully
improve performance when UBOs are used, but this was not directly
verified.

Signed-off-by: Ilia Mirkin 
---
 src/gallium/drivers/nouveau/nv50/nv50_context.c|  7 ---
 src/gallium/drivers/nouveau/nv50/nv50_context.h|  3 +-
 .../drivers/nouveau/nv50/nv50_shader_state.c   |  1 +
 src/gallium/drivers/nouveau/nv50/nv50_state.c  |  5 +-
 src/gallium/drivers/nouveau/nv50/nv50_transfer.c   | 63 +++---
 5 files changed, 50 insertions(+), 29 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c 
b/src/gallium/drivers/nouveau/nv50/nv50_context.c
index 8a148fc..84b5e3f 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_context.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c
@@ -318,14 +318,7 @@ nv50_create(struct pipe_screen *pscreen, void *priv, 
unsigned ctxflags)
nv50->base.screen= &screen->base;
nv50->base.copy_data = nv50_m2mf_copy_linear;
nv50->base.push_data = nv50_sifc_linear_u8;
-   /* FIXME: Make it possible to use this again. The problem is that there is
-* some clever logic in the card that allows for multiple renders to happen
-* when there are only constbuf changes. However that relies on the
-* constbuf updates happening to the right constbuf slots. Currently
-* implementation just makes it go through a separate slot which doesn't
-* properly update the right constbuf data.
nv50->base.push_cb   = nv50_cb_push;
-*/
 
nv50->screen = screen;
pipe->screen = pscreen;
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.h 
b/src/gallium/drivers/nouveau/nv50/nv50_context.h
index b7963a4..0582e24 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_context.h
+++ b/src/gallium/drivers/nouveau/nv50/nv50_context.h
@@ -286,8 +286,7 @@ nv50_m2mf_copy_linear(struct nouveau_context *pipe,
   unsigned size);
 void
 nv50_cb_push(struct nouveau_context *nv,
- struct nouveau_bo *bo, unsigned domain,
- unsigned base, unsigned size,
+ struct nv04_resource *res,
  unsigned offset, unsigned words, const uint32_t *data);
 
 /* nv50_vbo.c */
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c 
b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
index f838d15..2326394 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
@@ -99,6 +99,7 @@ nv50_constbufs_validate(struct nv50_context *nv50)
BCTX_REFN(nv50->bufctx_3d, 3D_CB(s, i), res, RD);
 
nv50->cb_dirty = 1; /* Force cache flush for UBO. */
+   res->cb_bindings[s] |= 1 << i;
 } else {
BEGIN_NV04(push, NV50_3D(SET_PROGRAM_CB), 1);
PUSH_DATA (push, (i << 8) | p | 0);
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state.c 
b/src/gallium/drivers/nouveau/nv50/nv50_state.c
index 86e74d6..3231aba 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_state.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_state.c
@@ -856,9 +856,10 @@ nv50_set_constant_buffer(struct pipe_context *pipe, uint 
shader, uint index,
if (nv50->constbuf[s][i].user)
   nv50->constbuf[s][i].u.buf = NULL;
else
-   if (nv50->constbuf[s][i].u.buf)
+   if (nv50->constbuf[s][i].u.buf) {
   nouveau_bufctx_reset(nv50->bufctx_3d, NV50_BIND_3D_CB(s, i));
-
+  nv04_resource(nv50->constbuf[s][i].u.buf)->cb_bindings[s] &= ~(1 << i);
+   }
pipe_resource_reference(&nv50->constbuf[s][i].u.buf, res);
 
nv50->constbuf[s][i].user = (cb && cb->user_buffer) ? true : false;
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_transfer.c 
b/src/gallium/drivers/nouveau/nv50/nv50_transfer.c
index f5c7c57..c751ade 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_transfer.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_transfer.c
@@ -377,32 +377,24 @@ nv50_miptree_transfer_unmap(struct pipe_context *pctx,
FREE(tx);
 }
 
-void
-nv50_cb_push(struct nouveau_context *nv,
- struct nouveau_bo *bo, unsigned domain,
- unsigned base, unsigned size,
- unsigned offset, unsigned words, const uint32_t *data)
+static void
+nv50_cb_bo_push(struct nouveau_context *nv,
+struct nouveau_bo *bo, unsigned domain,
+unsigned bufid,
+unsigned offset, unsigned words,
+const uint32_t *data)
 {
struct nouveau_pushbuf *push = nv->pushbuf;
-   struct nouveau_bufctx *bctx = nv50_

Re: [Mesa-dev] [PATCH 1/2] clover: assert struct argument is compiled usably

2016-06-05 Thread Vedran Miletić

On 06/04/2016 04:18 AM, Francisco Jerez wrote:

Serge Martin  writes:


From: Vedran Miletić 

Make sure that a struct argument did not get compiled into a pointer
type with the byval attribute. If we try to handle the pointer with
byval, we end up with the pointer size instead of the struct size.


Ugh, is that a bug in the code below?  How are byval pointers supposed
to be handled here?  Exactly as if the argument wasn't a pointer at all
by providing a copy of the pointed-to object as-is in the kernel input
buffer?  In that case wouldn't the code below need to pass the correct
size of the pointed-to object as target/api size rather than the size of
the pointer?



Yes, byval+pointer should be handled as there is no pointer at all.

I have tried passing the correct size, but IIRC LLVM AMDGPU backend does 
not generate correct asm for byval+pointer variant. The simple solution 
is to fail with an assert here unless Clang generates code both Clover 
and the backend can handle.


Regards,
Vedran

--
Vedran Miletić
vedran.miletic.net
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] st/mesa: check shader image format support before using PBO download

2016-06-05 Thread Ilia Mirkin
ARB_shader_image_load_store only requires a very fixed list of formats
to be supported, while textures may be in all kinds of formats, like
BGRA which are presently not supported on at least Kepler.

Signed-off-by: Ilia Mirkin 
---
 src/mesa/state_tracker/st_cb_readpixels.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/mesa/state_tracker/st_cb_readpixels.c 
b/src/mesa/state_tracker/st_cb_readpixels.c
index 6df3a39..09450c9 100644
--- a/src/mesa/state_tracker/st_cb_readpixels.c
+++ b/src/mesa/state_tracker/st_cb_readpixels.c
@@ -79,6 +79,7 @@ try_pbo_readpixels(struct st_context *st, struct 
st_renderbuffer *strb,
const struct gl_pixelstore_attrib *pack, void *pixels)
 {
struct pipe_context *pipe = st->pipe;
+   struct pipe_screen *screen = pipe->screen;
struct cso_context *cso = st->cso_context;
struct pipe_surface *surface = strb->surface;
struct pipe_resource *texture = strb->texture;
@@ -91,6 +92,11 @@ try_pbo_readpixels(struct st_context *st, struct 
st_renderbuffer *strb,
if (texture->nr_samples > 1)
   return false;
 
+   if (!screen->is_format_supported(screen, dst_format, PIPE_TEXTURE_2D,
+texture->nr_samples,
+PIPE_BIND_SHADER_IMAGE))
+  return false;
+
desc = util_format_description(dst_format);
 
/* Compute PBO addresses */
-- 
2.7.3

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


Re: [Mesa-dev] [PATCH] clover: Pass unquoted compiler arguments to Clang

2016-06-05 Thread Vedran Miletić

On 06/04/2016 10:13 AM, Francisco Jerez wrote:

Vedran Miletić  writes:


OpenCL apps can quote arguments they pass to the OpenCL compiler, most
commonly include paths containing spaces. If the OpenCL compiler was
called via a shell, the shell would remove (single or double) quotes
before passing the argument to the compiler. Since we call Clang as a
library, we have to remove quotes before passing the argument.


Sigh, it's a shame that the OpenCL spec doesn't specify what format the
option string should have, whether quotes have any special
interpretation at all, whether escape sequences are supported, etc.
Might be worth reporting the problem at Khronos -- Or have you found any
evidence in the spec that the parsing logic you implement below is the
correct one?



Aside from working just like NVIDIA and AMD proprietary stacks, no.

However, what we do right now is most certainly broken. Consider the 
following argument


-I"/foo bar/baz"

which is going to be sent to Clang as two arguments broken over space, 
while it should be one. We should split arguments correctly, and whether 
or not we remove quotes in the process or we expect Clang to handle them 
is the easier part of the job.


Regards,
Vedran

--
Vedran Miletić
vedran.miletic.net
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH v4] clover: Update OpenCL version string to match OpenGL

2016-06-05 Thread Vedran Miletić
Change MESA into Mesa in CL_PLATFORM_VERSION and CL_DEVICE_VERSION. For
both, always append git version suffix from git_sha1.h.

v4: dropped #ifdef guards.

It would be very nice if this could be considered for 12.0 release.
---
 src/gallium/state_trackers/clover/api/device.cpp   | 5 -
 src/gallium/state_trackers/clover/api/platform.cpp | 5 -
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/gallium/state_trackers/clover/api/device.cpp 
b/src/gallium/state_trackers/clover/api/device.cpp
index bc93f91..0d3de45 100644
--- a/src/gallium/state_trackers/clover/api/device.cpp
+++ b/src/gallium/state_trackers/clover/api/device.cpp
@@ -23,6 +23,7 @@
 #include "api/util.hpp"
 #include "core/platform.hpp"
 #include "core/device.hpp"
+#include "git_sha1.h"
 
 using namespace clover;
 
@@ -300,7 +301,9 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param,
   break;
 
case CL_DEVICE_VERSION:
-  buf.as_string() = "OpenCL 1.1 MESA " PACKAGE_VERSION;
+  buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
+" (" MESA_GIT_SHA1 ")"
+;
   break;
 
case CL_DEVICE_EXTENSIONS:
diff --git a/src/gallium/state_trackers/clover/api/platform.cpp 
b/src/gallium/state_trackers/clover/api/platform.cpp
index cf71593..1784fbc 100644
--- a/src/gallium/state_trackers/clover/api/platform.cpp
+++ b/src/gallium/state_trackers/clover/api/platform.cpp
@@ -22,6 +22,7 @@
 
 #include "api/util.hpp"
 #include "core/platform.hpp"
+#include "git_sha1.h"
 
 using namespace clover;
 
@@ -57,7 +58,9 @@ clover::GetPlatformInfo(cl_platform_id d_platform, 
cl_platform_info param,
   break;
 
case CL_PLATFORM_VERSION:
-  buf.as_string() = "OpenCL 1.1 MESA " PACKAGE_VERSION;
+  buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
+" (" MESA_GIT_SHA1 ")"
+;
   break;
 
case CL_PLATFORM_NAME:
-- 
2.7.4

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Samuel Pitoiset



On 06/06/2016 12:30 AM, Emil Velikov wrote:

On 5 June 2016 at 23:22, Ilia Mirkin  wrote:

On Sun, Jun 5, 2016 at 6:08 PM, Emil Velikov  wrote:

On 5 June 2016 at 23:00, Samuel Pitoiset  wrote:



On 06/05/2016 11:50 PM, Emil Velikov wrote:


On 5 June 2016 at 22:36, Ilia Mirkin  wrote:


On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov 
wrote:


On 5 June 2016 at 22:17, Ilia Mirkin  wrote:


On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov 
wrote:


On 5 June 2016 at 22:13, Emil Velikov 
wrote:


On 5 June 2016 at 17:56, Samuel Pitoiset 
wrote:


We should not call nouveau_bufctx_reset() inside a validate
function.


This seems to contradict the changes introduced in nvc0_compute.c.
Worth explaining a bit better the dos and don'ts ?


As this is already in master, can you please provide a more
elaborate/correct summary for -stable ?



I think it's fine as is.

Do: reset bufctx when setting dirty bit
Don't: reset bufctx in validate logic, since it's "too late" by then.
(Not strictly wrong, but just should do it earlier.)



So nvc0_compute_*validate*_surfaces is not validate logic ? Err...
what a confusing name it has ;-)



It validates compute. And it invalidates (and clears) the 3d bin.


So one can reset_bufctx(3d) from the compute validate and vice-versa.
While doing reset_bufctx(foo) from foo validate is a bad idea ?
Shouldn't one just say so in the commit message ?



Because the common practice is to clear foo bins at the same place where the
dirty_3d |= foo is updated, this makes sense. :)


Yet the commit message does not say that, right ? It says "We should
not call nouveau_bufctx_reset() inside a validate function.", while
the patch does the complete opposite - it adds a call to
nouveau_bufctx_reset() inside a validate function.

All I'm asking is for the commit message to reflect the code change or
vice-versa. I hope I'm not being unreasonable ?


The commit message also doesn't explain what bufctx's are, what a
validation function is, and the overall structure of the nouveau code
and how it uses those bufctx's.

You're arguing about clarity with the ~2 active developers/reviewers
who understand the code.

What I'm saying is that with contradicting commit messages like this
one there'll be little others who will understand it :-\


I understand that this might be unclear to
you, but I know I'm not about to explain everything in commit messages
all the time -- too much effort for zero benefit. Case in point - had
this commit said "nvc0: fix validation logic" and left it at that, we
wouldn't be having this discussion. But there was a bit of an
explanation, that was perhaps not infinitely precise, and now there's
a long discussion about how it's unclear.


Looking from another angle - is false information better than no
information ? All I was asking is to correct the commit message. I
wasn't asking that one should explain reasoning behind it or anything
deeper into the stack, which I believe was understood.

Seems like that is too much to ask so I won't bother you guys any more.


I'll make an effort the next time. At least, try to not introduce 
contradictions between the code and the commit message.




Thanks
Emil


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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Emil Velikov
On 5 June 2016 at 23:22, Ilia Mirkin  wrote:
> On Sun, Jun 5, 2016 at 6:08 PM, Emil Velikov  wrote:
>> On 5 June 2016 at 23:00, Samuel Pitoiset  wrote:
>>>
>>>
>>> On 06/05/2016 11:50 PM, Emil Velikov wrote:

 On 5 June 2016 at 22:36, Ilia Mirkin  wrote:
>
> On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov 
> wrote:
>>
>> On 5 June 2016 at 22:17, Ilia Mirkin  wrote:
>>>
>>> On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov 
>>> wrote:

 On 5 June 2016 at 22:13, Emil Velikov 
 wrote:
>
> On 5 June 2016 at 17:56, Samuel Pitoiset 
> wrote:
>>
>> We should not call nouveau_bufctx_reset() inside a validate
>> function.
>
> This seems to contradict the changes introduced in nvc0_compute.c.
> Worth explaining a bit better the dos and don'ts ?
>
 As this is already in master, can you please provide a more
 elaborate/correct summary for -stable ?
>>>
>>>
>>> I think it's fine as is.
>>>
>>> Do: reset bufctx when setting dirty bit
>>> Don't: reset bufctx in validate logic, since it's "too late" by then.
>>> (Not strictly wrong, but just should do it earlier.)
>>
>>
>> So nvc0_compute_*validate*_surfaces is not validate logic ? Err...
>> what a confusing name it has ;-)
>
>
> It validates compute. And it invalidates (and clears) the 3d bin.
>
 So one can reset_bufctx(3d) from the compute validate and vice-versa.
 While doing reset_bufctx(foo) from foo validate is a bad idea ?
 Shouldn't one just say so in the commit message ?
>>>
>>>
>>> Because the common practice is to clear foo bins at the same place where the
>>> dirty_3d |= foo is updated, this makes sense. :)
>>>
>> Yet the commit message does not say that, right ? It says "We should
>> not call nouveau_bufctx_reset() inside a validate function.", while
>> the patch does the complete opposite - it adds a call to
>> nouveau_bufctx_reset() inside a validate function.
>>
>> All I'm asking is for the commit message to reflect the code change or
>> vice-versa. I hope I'm not being unreasonable ?
>
> The commit message also doesn't explain what bufctx's are, what a
> validation function is, and the overall structure of the nouveau code
> and how it uses those bufctx's.
>
> You're arguing about clarity with the ~2 active developers/reviewers
> who understand the code.
What I'm saying is that with contradicting commit messages like this
one there'll be little others who will understand it :-\

> I understand that this might be unclear to
> you, but I know I'm not about to explain everything in commit messages
> all the time -- too much effort for zero benefit. Case in point - had
> this commit said "nvc0: fix validation logic" and left it at that, we
> wouldn't be having this discussion. But there was a bit of an
> explanation, that was perhaps not infinitely precise, and now there's
> a long discussion about how it's unclear.
>
Looking from another angle - is false information better than no
information ? All I was asking is to correct the commit message. I
wasn't asking that one should explain reasoning behind it or anything
deeper into the stack, which I believe was understood.

Seems like that is too much to ask so I won't bother you guys any more.

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


[Mesa-dev] [PATCH] radeonsi: Save and restore entire CE RAM.

2016-06-05 Thread Bas Nieuwenhuizen
This fixes a problem with the CE preamble and restoring only stuff in the
preamble when needed.

To illustrate suppose we have two graphics IB's 1 and 2, which  are submitted in
that order. Furthermore suppose IB 1 does not use CE ram, but IB 2 does, and we
have a context switch at the start of IB 1, but not between IB 1 and IB 2.

The old code put the CE RAM loads in the preamble of IB 2. As the preamble of
IB 1 does not have the loads and the preamble of IB 2 does not get executed, the
old values are not load into CE RAM.

Fix this by always restoring the entire CE RAM.

Signed-off-by: Bas Nieuwenhuizen 
Cc: "12.0" 
---
 src/gallium/drivers/radeonsi/si_descriptors.c | 56 +--
 src/gallium/drivers/radeonsi/si_hw_context.c  |  4 ++
 src/gallium/drivers/radeonsi/si_pipe.c|  7 
 src/gallium/drivers/radeonsi/si_pipe.h|  1 +
 src/gallium/drivers/radeonsi/si_state.h   |  7 ++--
 5 files changed, 43 insertions(+), 32 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
b/src/gallium/drivers/radeonsi/si_descriptors.c
index baddc5f..5a1a344 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -160,30 +160,39 @@ static bool si_ce_upload(struct si_context *sctx, 
unsigned ce_offset, unsigned s
return true;
 }
 
-static void si_reinitialize_ce_ram(struct si_context *sctx,
-struct si_descriptors *desc)
+void si_ce_save_ram(struct si_context *sctx)
 {
-   if (desc->buffer) {
-   struct r600_resource *buffer = (struct 
r600_resource*)desc->buffer;
-   unsigned list_size = desc->num_elements * desc->element_dw_size 
* 4;
-   uint64_t va = buffer->gpu_address + desc->buffer_offset;
-   struct radeon_winsys_cs *ib = sctx->ce_preamble_ib;
+   if (!sctx->ce_ib)
+   return;
 
-   if (!ib)
-   ib = sctx->ce_ib;
+   radeon_emit(sctx->ce_ib, PKT3(PKT3_DUMP_CONST_RAM, 3, 0));
+   radeon_emit(sctx->ce_ib, 0);
+   radeon_emit(sctx->ce_ib, SI_CE_RAM_SIZE / 4);
+   radeon_emit(sctx->ce_ib, sctx->ce_ram_data->gpu_address);
+   radeon_emit(sctx->ce_ib, sctx->ce_ram_data->gpu_address >> 32);
 
-   list_size = align(list_size, 32);
+   radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, sctx->ce_ram_data,
+  RADEON_USAGE_WRITE, RADEON_PRIO_DESCRIPTORS);
+}
 
-   radeon_emit(ib, PKT3(PKT3_LOAD_CONST_RAM, 3, 0));
-   radeon_emit(ib, va);
-   radeon_emit(ib, va >> 32);
-   radeon_emit(ib, list_size / 4);
-   radeon_emit(ib, desc->ce_offset);
+void si_ce_restore_ram(struct si_context *sctx)
+{
+   struct radeon_winsys_cs *ib = sctx->ce_preamble_ib;
 
-   radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, desc->buffer,
-   RADEON_USAGE_READ, RADEON_PRIO_DESCRIPTORS);
-   }
-   desc->ce_ram_dirty = false;
+   if (!ib)
+   ib = sctx->ce_ib;
+
+   if (!ib)
+   return;
+
+   radeon_emit(ib, PKT3(PKT3_LOAD_CONST_RAM, 3, 0));
+   radeon_emit(ib, sctx->ce_ram_data->gpu_address);
+   radeon_emit(ib, sctx->ce_ram_data->gpu_address >> 32);
+   radeon_emit(ib, SI_CE_RAM_SIZE / 4);
+   radeon_emit(ib, 0);
+
+   radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, sctx->ce_ram_data,
+   RADEON_USAGE_READ, RADEON_PRIO_DESCRIPTORS);
 }
 
 void si_ce_enable_loads(struct radeon_winsys_cs *ib)
@@ -206,9 +215,6 @@ static bool si_upload_descriptors(struct si_context *sctx,
if (sctx->ce_ib) {
uint32_t const* list = (uint32_t const*)desc->list;
 
-   if (desc->ce_ram_dirty)
-   si_reinitialize_ce_ram(sctx, desc);
-
while(desc->dirty_mask) {
int begin, count;
u_bit_scan_consecutive_range(&desc->dirty_mask, &begin,
@@ -287,8 +293,6 @@ static void si_sampler_views_begin_new_cs(struct si_context 
*sctx,
   RADEON_USAGE_READ);
}
 
-   views->desc.ce_ram_dirty = true;
-
if (!views->desc.buffer)
return;
radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, views->desc.buffer,
@@ -489,8 +493,6 @@ si_image_views_begin_new_cs(struct si_context *sctx, struct 
si_images_info *imag
   RADEON_USAGE_READWRITE);
}
 
-   images->desc.ce_ram_dirty = true;
-
if (images->desc.buffer) {
radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx,
  images->desc.buffer,
@@ -737,8 +739,6 @@ static void si_buffer_resources_begin_new_cs(struct 
si_context *sctx,
  buffers->shader_usage, buffers->priority);
}
 
-   buffers->desc.ce_ra

Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Ilia Mirkin
On Sun, Jun 5, 2016 at 6:08 PM, Emil Velikov  wrote:
> On 5 June 2016 at 23:00, Samuel Pitoiset  wrote:
>>
>>
>> On 06/05/2016 11:50 PM, Emil Velikov wrote:
>>>
>>> On 5 June 2016 at 22:36, Ilia Mirkin  wrote:

 On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov 
 wrote:
>
> On 5 June 2016 at 22:17, Ilia Mirkin  wrote:
>>
>> On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov 
>> wrote:
>>>
>>> On 5 June 2016 at 22:13, Emil Velikov 
>>> wrote:

 On 5 June 2016 at 17:56, Samuel Pitoiset 
 wrote:
>
> We should not call nouveau_bufctx_reset() inside a validate
> function.

 This seems to contradict the changes introduced in nvc0_compute.c.
 Worth explaining a bit better the dos and don'ts ?

>>> As this is already in master, can you please provide a more
>>> elaborate/correct summary for -stable ?
>>
>>
>> I think it's fine as is.
>>
>> Do: reset bufctx when setting dirty bit
>> Don't: reset bufctx in validate logic, since it's "too late" by then.
>> (Not strictly wrong, but just should do it earlier.)
>
>
> So nvc0_compute_*validate*_surfaces is not validate logic ? Err...
> what a confusing name it has ;-)


 It validates compute. And it invalidates (and clears) the 3d bin.

>>> So one can reset_bufctx(3d) from the compute validate and vice-versa.
>>> While doing reset_bufctx(foo) from foo validate is a bad idea ?
>>> Shouldn't one just say so in the commit message ?
>>
>>
>> Because the common practice is to clear foo bins at the same place where the
>> dirty_3d |= foo is updated, this makes sense. :)
>>
> Yet the commit message does not say that, right ? It says "We should
> not call nouveau_bufctx_reset() inside a validate function.", while
> the patch does the complete opposite - it adds a call to
> nouveau_bufctx_reset() inside a validate function.
>
> All I'm asking is for the commit message to reflect the code change or
> vice-versa. I hope I'm not being unreasonable ?

The commit message also doesn't explain what bufctx's are, what a
validation function is, and the overall structure of the nouveau code
and how it uses those bufctx's.

You're arguing about clarity with the ~2 active developers/reviewers
who understand the code. I understand that this might be unclear to
you, but I know I'm not about to explain everything in commit messages
all the time -- too much effort for zero benefit. Case in point - had
this commit said "nvc0: fix validation logic" and left it at that, we
wouldn't be having this discussion. But there was a bit of an
explanation, that was perhaps not infinitely precise, and now there's
a long discussion about how it's unclear.

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Samuel Pitoiset



On 06/06/2016 12:08 AM, Emil Velikov wrote:

On 5 June 2016 at 23:00, Samuel Pitoiset  wrote:



On 06/05/2016 11:50 PM, Emil Velikov wrote:


On 5 June 2016 at 22:36, Ilia Mirkin  wrote:


On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov 
wrote:


On 5 June 2016 at 22:17, Ilia Mirkin  wrote:


On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov 
wrote:


On 5 June 2016 at 22:13, Emil Velikov 
wrote:


On 5 June 2016 at 17:56, Samuel Pitoiset 
wrote:


We should not call nouveau_bufctx_reset() inside a validate
function.


This seems to contradict the changes introduced in nvc0_compute.c.
Worth explaining a bit better the dos and don'ts ?


As this is already in master, can you please provide a more
elaborate/correct summary for -stable ?



I think it's fine as is.

Do: reset bufctx when setting dirty bit
Don't: reset bufctx in validate logic, since it's "too late" by then.
(Not strictly wrong, but just should do it earlier.)



So nvc0_compute_*validate*_surfaces is not validate logic ? Err...
what a confusing name it has ;-)



It validates compute. And it invalidates (and clears) the 3d bin.


So one can reset_bufctx(3d) from the compute validate and vice-versa.
While doing reset_bufctx(foo) from foo validate is a bad idea ?
Shouldn't one just say so in the commit message ?



Because the common practice is to clear foo bins at the same place where the
dirty_3d |= foo is updated, this makes sense. :)


Yet the commit message does not say that, right ? It says "We should
not call nouveau_bufctx_reset() inside a validate function.", while
the patch does the complete opposite - it adds a call to
nouveau_bufctx_reset() inside a validate function.

All I'm asking is for the commit message to reflect the code change or
vice-versa. I hope I'm not being unreasonable ?


You are reasonable. I assume something like "nvc0: clear out surfaces 
bins at the right place" (and explain why in the description) is more 
close to what the code does.




Thanks
Emil


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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Emil Velikov
On 5 June 2016 at 23:00, Samuel Pitoiset  wrote:
>
>
> On 06/05/2016 11:50 PM, Emil Velikov wrote:
>>
>> On 5 June 2016 at 22:36, Ilia Mirkin  wrote:
>>>
>>> On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov 
>>> wrote:

 On 5 June 2016 at 22:17, Ilia Mirkin  wrote:
>
> On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov 
> wrote:
>>
>> On 5 June 2016 at 22:13, Emil Velikov 
>> wrote:
>>>
>>> On 5 June 2016 at 17:56, Samuel Pitoiset 
>>> wrote:

 We should not call nouveau_bufctx_reset() inside a validate
 function.
>>>
>>> This seems to contradict the changes introduced in nvc0_compute.c.
>>> Worth explaining a bit better the dos and don'ts ?
>>>
>> As this is already in master, can you please provide a more
>> elaborate/correct summary for -stable ?
>
>
> I think it's fine as is.
>
> Do: reset bufctx when setting dirty bit
> Don't: reset bufctx in validate logic, since it's "too late" by then.
> (Not strictly wrong, but just should do it earlier.)


 So nvc0_compute_*validate*_surfaces is not validate logic ? Err...
 what a confusing name it has ;-)
>>>
>>>
>>> It validates compute. And it invalidates (and clears) the 3d bin.
>>>
>> So one can reset_bufctx(3d) from the compute validate and vice-versa.
>> While doing reset_bufctx(foo) from foo validate is a bad idea ?
>> Shouldn't one just say so in the commit message ?
>
>
> Because the common practice is to clear foo bins at the same place where the
> dirty_3d |= foo is updated, this makes sense. :)
>
Yet the commit message does not say that, right ? It says "We should
not call nouveau_bufctx_reset() inside a validate function.", while
the patch does the complete opposite - it adds a call to
nouveau_bufctx_reset() inside a validate function.

All I'm asking is for the commit message to reflect the code change or
vice-versa. I hope I'm not being unreasonable ?

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Samuel Pitoiset



On 06/05/2016 11:50 PM, Emil Velikov wrote:

On 5 June 2016 at 22:36, Ilia Mirkin  wrote:

On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov  wrote:

On 5 June 2016 at 22:17, Ilia Mirkin  wrote:

On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov  wrote:

On 5 June 2016 at 22:13, Emil Velikov  wrote:

On 5 June 2016 at 17:56, Samuel Pitoiset  wrote:

We should not call nouveau_bufctx_reset() inside a validate function.

This seems to contradict the changes introduced in nvc0_compute.c.
Worth explaining a bit better the dos and don'ts ?


As this is already in master, can you please provide a more
elaborate/correct summary for -stable ?


I think it's fine as is.

Do: reset bufctx when setting dirty bit
Don't: reset bufctx in validate logic, since it's "too late" by then.
(Not strictly wrong, but just should do it earlier.)


So nvc0_compute_*validate*_surfaces is not validate logic ? Err...
what a confusing name it has ;-)


It validates compute. And it invalidates (and clears) the 3d bin.


So one can reset_bufctx(3d) from the compute validate and vice-versa.
While doing reset_bufctx(foo) from foo validate is a bad idea ?
Shouldn't one just say so in the commit message ?


Because the common practice is to clear foo bins at the same place where 
the dirty_3d |= foo is updated, this makes sense. :)




-Emil


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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Emil Velikov
On 5 June 2016 at 22:36, Ilia Mirkin  wrote:
> On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov  wrote:
>> On 5 June 2016 at 22:17, Ilia Mirkin  wrote:
>>> On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov  
>>> wrote:
 On 5 June 2016 at 22:13, Emil Velikov  wrote:
> On 5 June 2016 at 17:56, Samuel Pitoiset  
> wrote:
>> We should not call nouveau_bufctx_reset() inside a validate function.
> This seems to contradict the changes introduced in nvc0_compute.c.
> Worth explaining a bit better the dos and don'ts ?
>
 As this is already in master, can you please provide a more
 elaborate/correct summary for -stable ?
>>>
>>> I think it's fine as is.
>>>
>>> Do: reset bufctx when setting dirty bit
>>> Don't: reset bufctx in validate logic, since it's "too late" by then.
>>> (Not strictly wrong, but just should do it earlier.)
>>
>> So nvc0_compute_*validate*_surfaces is not validate logic ? Err...
>> what a confusing name it has ;-)
>
> It validates compute. And it invalidates (and clears) the 3d bin.
>
So one can reset_bufctx(3d) from the compute validate and vice-versa.
While doing reset_bufctx(foo) from foo validate is a bad idea ?
Shouldn't one just say so in the commit message ?

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Ilia Mirkin
On Sun, Jun 5, 2016 at 5:34 PM, Emil Velikov  wrote:
> On 5 June 2016 at 22:17, Ilia Mirkin  wrote:
>> On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov  
>> wrote:
>>> On 5 June 2016 at 22:13, Emil Velikov  wrote:
 On 5 June 2016 at 17:56, Samuel Pitoiset  wrote:
> We should not call nouveau_bufctx_reset() inside a validate function.
 This seems to contradict the changes introduced in nvc0_compute.c.
 Worth explaining a bit better the dos and don'ts ?

>>> As this is already in master, can you please provide a more
>>> elaborate/correct summary for -stable ?
>>
>> I think it's fine as is.
>>
>> Do: reset bufctx when setting dirty bit
>> Don't: reset bufctx in validate logic, since it's "too late" by then.
>> (Not strictly wrong, but just should do it earlier.)
>
> So nvc0_compute_*validate*_surfaces is not validate logic ? Err...
> what a confusing name it has ;-)

It validates compute. And it invalidates (and clears) the 3d bin.

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Emil Velikov
On 5 June 2016 at 22:17, Ilia Mirkin  wrote:
> On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov  wrote:
>> On 5 June 2016 at 22:13, Emil Velikov  wrote:
>>> On 5 June 2016 at 17:56, Samuel Pitoiset  wrote:
 We should not call nouveau_bufctx_reset() inside a validate function.
>>> This seems to contradict the changes introduced in nvc0_compute.c.
>>> Worth explaining a bit better the dos and don'ts ?
>>>
>> As this is already in master, can you please provide a more
>> elaborate/correct summary for -stable ?
>
> I think it's fine as is.
>
> Do: reset bufctx when setting dirty bit
> Don't: reset bufctx in validate logic, since it's "too late" by then.
> (Not strictly wrong, but just should do it earlier.)

So nvc0_compute_*validate*_surfaces is not validate logic ? Err...
what a confusing name it has ;-)

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Samuel Pitoiset



On 06/05/2016 11:17 PM, Ilia Mirkin wrote:

On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov  wrote:

On 5 June 2016 at 22:13, Emil Velikov  wrote:

On 5 June 2016 at 17:56, Samuel Pitoiset  wrote:

We should not call nouveau_bufctx_reset() inside a validate function.

This seems to contradict the changes introduced in nvc0_compute.c.
Worth explaining a bit better the dos and don'ts ?


As this is already in master, can you please provide a more
elaborate/correct summary for -stable ?


I think it's fine as is.

Do: reset bufctx when setting dirty bit
Don't: reset bufctx in validate logic, since it's "too late" by then.
(Not strictly wrong, but just should do it earlier.)



And usually, we clear the bins (ie. nouveau_bufctx_reset()) when the 
dirty flag is updated. This doesn't change anything, it's just wrong to 
do it inside the validate function.

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Ilia Mirkin
On Sun, Jun 5, 2016 at 5:16 PM, Emil Velikov  wrote:
> On 5 June 2016 at 22:13, Emil Velikov  wrote:
>> On 5 June 2016 at 17:56, Samuel Pitoiset  wrote:
>>> We should not call nouveau_bufctx_reset() inside a validate function.
>> This seems to contradict the changes introduced in nvc0_compute.c.
>> Worth explaining a bit better the dos and don'ts ?
>>
> As this is already in master, can you please provide a more
> elaborate/correct summary for -stable ?

I think it's fine as is.

Do: reset bufctx when setting dirty bit
Don't: reset bufctx in validate logic, since it's "too late" by then.
(Not strictly wrong, but just should do it earlier.)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Emil Velikov
On 5 June 2016 at 22:13, Emil Velikov  wrote:
> On 5 June 2016 at 17:56, Samuel Pitoiset  wrote:
>> We should not call nouveau_bufctx_reset() inside a validate function.
> This seems to contradict the changes introduced in nvc0_compute.c.
> Worth explaining a bit better the dos and don'ts ?
>
As this is already in master, can you please provide a more
elaborate/correct summary for -stable ?

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Emil Velikov
On 5 June 2016 at 17:56, Samuel Pitoiset  wrote:
> We should not call nouveau_bufctx_reset() inside a validate function.
This seems to contradict the changes introduced in nvc0_compute.c.
Worth explaining a bit better the dos and don'ts ?

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


[Mesa-dev] [PATCH] glsl/ast: don't crash when func_name is NULL

2016-06-05 Thread Dave Airlie
From: Dave Airlie 

This fixes a crash in
GL43-CTS.shader_subroutine.subroutines_not_allowed_as_variables_constructors_and_argument_or_return_types

If we can't find the func_name in one of these paths,
we have emitted an earlier error so just return here.

Signed-off-by: Dave Airlie 
---
 src/compiler/glsl/ast_function.cpp | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/compiler/glsl/ast_function.cpp 
b/src/compiler/glsl/ast_function.cpp
index a97e6c9..f74394f 100644
--- a/src/compiler/glsl/ast_function.cpp
+++ b/src/compiler/glsl/ast_function.cpp
@@ -2078,6 +2078,10 @@ ast_function_expression::hir(exec_list *instructions,
  func_name = id->primary_expression.identifier;
   }
 
+  /* an error was emitted earlier */
+  if (!func_name)
+ return ir_rvalue::error_value(ctx);
+
   ir_function_signature *sig =
 match_function_by_name(func_name, &actual_parameters, state);
 
-- 
2.7.4

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


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

2016-06-05 Thread AppVeyor



Build mesa 1516 failed


Commit c417c0c9c3 by Kenneth Graunke on 9/7/2014 3:26 AM:

mesa: Add MESA_SHADER_CAPTURE_PATH for writing .shader_test files.\n\nThis writes linked shader programs to .shader_test files to\n$MESA_SHADER_CAPTURE_PATH in the format used by shader-db\n(http://cgit.freedesktop.org/mesa/shader-db).\n\nIt supports both GLSL shaders and ARB programs.  All stages that\nare linked together are written in a single .shader_test file.\n\nThis eliminates the need for shader-db's split-to-files.py, as Mesa\nproduces the desired format directly.  It's much more reliable than\nparsing stdout/stderr, as those may contain extraneous messages, or\nsimply be closed by the application and unavailable.\n\nWe have many similar features already, but this is a bit different:\n- MESA_GLSL=dump writes to stdout, not files.\n- MESA_GLSL=log writes each stage to separate files (rather than\n  all linked shaders in one file), at draw time (not link time),\n  with uniform data and state flag info.\n- Tapani's shader replacement mechanism (MESA_SHADER_DUMP_PATH and\n  MESA_SHADER_READ_PATH) also uses separate files per shader stage,\n  but allows reading in files to replace an app's shader code.\n\nv2:  Dump ARB programs too, not just GLSL.\nv3:  Don't dump bogus 0.shader_test file.\nv4:  Add "GL_ARB_separate_shader_objects" to the [require] block.\nv5:  Print "GLSL 4.00" instead of "GLSL 4.0" in the [require] block.\nv6:  Don't hardcode /tmp/mesa.\nv7:  Fix memoization of getenv().\nv8:  Also print "SSO ENABLED" (suggested by Timothy).\nv9:  Also handle ES shaders (suggested by Ilia).\nv10: Guard against MESA_SHADER_CAPTURE_PATH being too long; add\n _mesa_warning calls on error handling (suggested by Ben).\nv11: Fix crash when variable is unset introduced in v10.\n\nSigned-off-by: Kenneth Graunke \nReviewed-by: Matt Turner 


Configure your notification preferences

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


[Mesa-dev] [PATCH] glsl: handle ast_aggregate in has_sequence_subexpression. (v2)

2016-06-05 Thread Dave Airlie
From: Dave Airlie 

GL43-CTS.compute_shader.work-group-size does
uniform uint g_uniform[gl_WorkGroupSize.z + 20] = { 
1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24 };

The initializer triggers the GLSL 4.30/GLES3 tests
for constant sequence subexpressions, so it doesn't
happen unless you are using those, so just return
false as this path is now reachable.

v2: update commit msg with diagnosis
Signed-off-by: Dave Airlie 
---
 src/compiler/glsl/ast_to_hir.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 400d3c4..b7192b2 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -2100,7 +2100,7 @@ ast_expression::has_sequence_subexpression() const
   return false;
 
case ast_aggregate:
-  unreachable("ast_aggregate: Should never get here.");
+  return false;
 
case ast_function_call:
   unreachable("should be handled by ast_function_expression::hir");
-- 
2.7.4

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


Re: [Mesa-dev] [PATCH] mesa/readpix: clamp SNORM properly (v2)

2016-06-05 Thread Dave Airlie
On 6 June 2016 at 05:40, Dave Airlie  wrote:
> From: Dave Airlie 
>
> The clamping code always clamps to 0..1, which for SNORM is
> incorrect. This adds a bit to say that clamping is for
> an snorm and to use the correct range.
>
> This fixes a number of SNORM cases in:
> GL33-CTS.texture_size_promotion.functional

actually ignore this, it's a test bug I think now that I've reread the spec.

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


Re: [Mesa-dev] [PATCH 2/3] radeonsi: use hw MSAA resolve for layered resolves

2016-06-05 Thread Marek Olšák
Please ignore this patch. It doesn't work.

Marek

On Sun, Jun 5, 2016 at 5:07 PM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> ---
>  src/gallium/drivers/radeonsi/si_blit.c | 34 
> +-
>  1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_blit.c 
> b/src/gallium/drivers/radeonsi/si_blit.c
> index 1c2c538..80844ec 100644
> --- a/src/gallium/drivers/radeonsi/si_blit.c
> +++ b/src/gallium/drivers/radeonsi/si_blit.c
> @@ -859,13 +859,13 @@ static bool do_hardware_msaa_resolve(struct 
> pipe_context *ctx,
> unsigned sample_mask = ~0;
> struct pipe_resource *tmp, templ;
> struct pipe_blit_info blit;
> +   unsigned i;
>
> -   /* Check basic requirements for hw resolve. */
> +   /* Check basic requirements for MSAA resolve. */
> if (!(info->src.resource->nr_samples > 1 &&
>   info->dst.resource->nr_samples <= 1 &&
>   !util_format_is_pure_integer(format) &&
> - !util_format_is_depth_or_stencil(format) &&
> - util_max_layer(info->src.resource, 0) == 0))
> + !util_format_is_depth_or_stencil(format)))
> return false;
>
> /* Hardware MSAA resolve doesn't work if SPI format = NORM16_ABGR and
> @@ -878,6 +878,7 @@ static bool do_hardware_msaa_resolve(struct pipe_context 
> *ctx,
>
> /* Check the remaining requirements for hw resolve. */
> if (util_max_layer(info->dst.resource, info->dst.level) == 0 &&
> +   util_max_layer(info->src.resource, 0) == 0 &&
> !info->scissor_enable &&
> (info->mask & PIPE_MASK_RGBA) == PIPE_MASK_RGBA &&
> 
> util_is_format_compatible(util_format_description(info->src.format),
> @@ -914,12 +915,12 @@ static bool do_hardware_msaa_resolve(struct 
> pipe_context *ctx,
>  * a temporary texture and blit.
>  */
> memset(&templ, 0, sizeof(templ));
> -   templ.target = PIPE_TEXTURE_2D;
> +   templ.target = info->src.resource->target;
> templ.format = info->src.resource->format;
> templ.width0 = info->src.resource->width0;
> templ.height0 = info->src.resource->height0;
> templ.depth0 = 1;
> -   templ.array_size = 1;
> +   templ.array_size = info->src.resource->array_size;
> templ.usage = PIPE_USAGE_DEFAULT;
> templ.flags = R600_RESOURCE_FLAG_FORCE_TILING;
>
> @@ -927,14 +928,21 @@ static bool do_hardware_msaa_resolve(struct 
> pipe_context *ctx,
> if (!tmp)
> return false;
>
> -   /* resolve */
> -   si_blitter_begin(ctx, SI_COLOR_RESOLVE |
> -(info->render_condition_enable ? 0 : 
> SI_DISABLE_RENDER_COND));
> -   util_blitter_custom_resolve_color(sctx->blitter, tmp, 0, 0,
> - info->src.resource, info->src.box.z,
> - sample_mask, 
> sctx->custom_blend_resolve,
> - format);
> -   si_blitter_end(ctx);
> +   /* resolve all layers that need to be resolved */
> +   assert(info->src.box.depth > 0);
> +   for (i = 0; i < info->src.box.depth; i++) {
> +   si_blitter_begin(ctx, SI_COLOR_RESOLVE |
> +(info->render_condition_enable ?
> + 0 : SI_DISABLE_RENDER_COND));
> +   util_blitter_custom_resolve_color(sctx->blitter, tmp, 0,
> + info->src.box.z + i,
> + info->src.resource,
> + info->src.box.z + i,
> + sample_mask,
> + sctx->custom_blend_resolve,
> + format);
> +   si_blitter_end(ctx);
> +   }
>
> /* blit */
> blit = *info;
> --
> 2.7.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] mesa/readpix: clamp SNORM properly (v2)

2016-06-05 Thread Dave Airlie
From: Dave Airlie 

The clamping code always clamps to 0..1, which for SNORM is
incorrect. This adds a bit to say that clamping is for
an snorm and to use the correct range.

This fixes a number of SNORM cases in:
GL33-CTS.texture_size_promotion.functional

v2: handle the case where the format is an array format.
don't test for snorm in readpixels, readpixels always does
[0..1] according to spec.

Signed-off-by: Dave Airlie 
---
 src/mesa/main/pack.c  |  4 +++-
 src/mesa/main/pixeltransfer.c | 10 ++
 src/mesa/main/pixeltransfer.h |  1 +
 src/mesa/main/readpix.c   |  4 +++-
 src/mesa/main/texgetimage.c   |  4 +++-
 src/mesa/main/texstore.c  | 10 +-
 src/mesa/swrast/s_copypix.c   |  4 +++-
 src/mesa/swrast/s_drawpix.c   |  2 +-
 8 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/mesa/main/pack.c b/src/mesa/main/pack.c
index 89faf51..f810929 100644
--- a/src/mesa/main/pack.c
+++ b/src/mesa/main/pack.c
@@ -1607,7 +1607,9 @@ _mesa_unpack_color_index_to_rgba_float(struct gl_context 
*ctx, GLuint dims,
* with color indexes.
*/
   transferOps &= ~(IMAGE_SCALE_BIAS_BIT | IMAGE_MAP_COLOR_BIT);
-  _mesa_apply_rgba_transfer_ops(ctx, transferOps, count, (float 
(*)[4])dstPtr);
+  _mesa_apply_rgba_transfer_ops(ctx, transferOps,
+false,
+count, (float (*)[4])dstPtr);
 
   dstPtr += srcHeight * srcWidth * 4;
}
diff --git a/src/mesa/main/pixeltransfer.c b/src/mesa/main/pixeltransfer.c
index 22eac00..93ab8ad 100644
--- a/src/mesa/main/pixeltransfer.c
+++ b/src/mesa/main/pixeltransfer.c
@@ -162,6 +162,7 @@ _mesa_scale_and_bias_depth_uint(const struct gl_context 
*ctx, GLuint n,
  */
 void
 _mesa_apply_rgba_transfer_ops(struct gl_context *ctx, GLbitfield transferOps,
+  bool clamp_snorm,
   GLuint n, GLfloat rgba[][4])
 {
/* scale & bias */
@@ -180,11 +181,12 @@ _mesa_apply_rgba_transfer_ops(struct gl_context *ctx, 
GLbitfield transferOps,
/* clamping to [0,1] */
if (transferOps & IMAGE_CLAMP_BIT) {
   GLuint i;
+  GLfloat lower_val = clamp_snorm ? -1.0F : 0.0F;
   for (i = 0; i < n; i++) {
- rgba[i][RCOMP] = CLAMP(rgba[i][RCOMP], 0.0F, 1.0F);
- rgba[i][GCOMP] = CLAMP(rgba[i][GCOMP], 0.0F, 1.0F);
- rgba[i][BCOMP] = CLAMP(rgba[i][BCOMP], 0.0F, 1.0F);
- rgba[i][ACOMP] = CLAMP(rgba[i][ACOMP], 0.0F, 1.0F);
+ rgba[i][RCOMP] = CLAMP(rgba[i][RCOMP], lower_val, 1.0F);
+ rgba[i][GCOMP] = CLAMP(rgba[i][GCOMP], lower_val, 1.0F);
+ rgba[i][BCOMP] = CLAMP(rgba[i][BCOMP], lower_val, 1.0F);
+ rgba[i][ACOMP] = CLAMP(rgba[i][ACOMP], lower_val, 1.0F);
   }
}
 }
diff --git a/src/mesa/main/pixeltransfer.h b/src/mesa/main/pixeltransfer.h
index b0a301f..e67ab53 100644
--- a/src/mesa/main/pixeltransfer.h
+++ b/src/mesa/main/pixeltransfer.h
@@ -56,6 +56,7 @@ _mesa_scale_and_bias_depth_uint(const struct gl_context *ctx, 
GLuint n,
 
 extern void
 _mesa_apply_rgba_transfer_ops(struct gl_context *ctx, GLbitfield transferOps,
+  bool clamp_snorm,
   GLuint n, GLfloat rgba[][4]);
 
 extern void
diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index 1cb06c7..9c17a10 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -544,7 +544,9 @@ read_rgba_pixels( struct gl_context *ctx,
 
   /* Handle transfer ops if necessary */
   if (transferOps)
- _mesa_apply_rgba_transfer_ops(ctx, transferOps, width * height, rgba);
+ _mesa_apply_rgba_transfer_ops(ctx, transferOps,
+   false,
+   width * height, rgba);
 
   /* If we had to rebase, we have already taken care of that */
   needs_rebase = false;
diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
index fc3cc6b..c501f9e 100644
--- a/src/mesa/main/texgetimage.c
+++ b/src/mesa/main/texgetimage.c
@@ -518,7 +518,9 @@ get_tex_rgba_uncompressed(struct gl_context *ctx, GLuint 
dimensions,
   needsRebase ? rebaseSwizzle : NULL);
 
  /* Handle transfer ops now */
- _mesa_apply_rgba_transfer_ops(ctx, transferOps, width * height, rgba);
+ _mesa_apply_rgba_transfer_ops(ctx, transferOps,
+   _mesa_get_format_datatype(texFormat) == 
GL_SIGNED_NORMALIZED,
+   width * height, rgba);
 
  /* If we had to rebase, we have already handled that */
  needsRebase = false;
diff --git a/src/mesa/main/texstore.c b/src/mesa/main/texstore.c
index b54e033..bed416a 100644
--- a/src/mesa/main/texstore.c
+++ b/src/mesa/main/texstore.c
@@ -760,6 +760,7 @@ texstore_rgba(TEXSTORE_PARAMS)
_mesa_texstore_needs_transfer_ops(ctx, baseInternalFormat, dstFormat)) {
   /* Allocate RGBA float image 

Re: [Mesa-dev] [PATCH] st/mesa: change SQRT lowering to fix the game Risen

2016-06-05 Thread Marek Olšák
On Sun, Jun 5, 2016 at 6:34 PM, Axel Davy  wrote:
> if abs is not inserted in the source glsl, that's a wine bug, not a mesa
> bug,
> Thus I don't think doing this is a good idea.

The old code used CMP, this code uses ABS instead. ABS is faster. I
don't see a problem with that considering the behavior is "undefined" in GLSL.

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


[Mesa-dev] [Bug 94512] X segfaults with glx-tls enabled in a x32 environment

2016-06-05 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=94512

--- Comment #6 from Emil Velikov  ---
Based of the backtrace there is no information if mesa/libglapi was build with
or w/o glx-tls. Following an IRC conversation it seems that the library is
built w/o glx-tls as i is missing the _glapi_tls_Context and
_glapi_tls_Dispatch symbols.

Perhaps they are different builds ? Either way it's good to attach the output
of $nm -CD --defined-only /usr/libx32/libglapi.so in this particular case.

Also please that the correct libraries are being picked up. This can be done
via something like:
 $LD_DEBUG=libs foo (startx) 2>ld_debug.log

Obviously none of these excludes that there is an actual issue with x32 and
glx-tls and/or that mesa silently ignores the flag on said platform ;-)

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


Re: [Mesa-dev] Patchwork review process (efficiency) questions

2016-06-05 Thread Gediminas Jakutis
On 2016.06.03 18:49, ⚛ wrote:
> On Fri, Jun 3, 2016 at 2:08 PM, Nicolai Hähnle  wrote:
>> On 03.06.2016 13:12, wrote:
>>>
>>> Situation: Looking at the content displayed by the web browser for URL
>>> http://patchwork.freedesktop.org/project/mesa/series and sub-pages
>>> accessible via the links.
>>
>>
>> Patchwork isn't really central to most people's workflow as far as I
>> understand it.
> 
> Ok. But how does a developer know (better: gets notified) when a patch
> has been accepted and added to mainline mesa-git? Viewing all git log
> messages every day because - just in case - the patch _might_ have
> been added to mesa-git seems quite inefficient to me.

I only ever sent one (1) patch to mesa "out-of-the-blue", so I believe I
can speak from a "newcomer" POV here.
I did not know about this patchwork thing until You mentioned it here.
I just sent my patch to mesa-dev and waited. It did get some comments
right away, but did not get reviewed for quite a while. That prompted me
to simply ask on IRC about it. I got promptly informed that the person
who can review it was tad busy at the time, so he might take a some time
to do it.

When it did get reviewed and committed, I knew instantly. By automation,
as You suggest. By adding a simple filter rule in my mailing client to
alert me when something gets pushed to mesa-commit mailing list
containing my name.
As I like to say it in such cases "wow, that was hard" \_(:_/

So, as a by definition and "outsider"/"newcomer" contributor I can say
I encountered zero problems with the process. 
 
>> Most of your questions' answers naturally fall out from that.
> 
> Not in my world.
> 
>> This mailing list is what's important.
> 
> In my opinion, now isn't the 1990-ties. (This is just my personal
> opinion, you do not need to agree with it.)
> 
>>> - What is the influence of the default ordering (URL suffix
>>> "?ordering=-last_updated") on the behavior of reviewers?
>>
>> Probably zero, because I doubt people go via Patchwork.
> 
> In that case, the process might be even less efficient than I though.
> 
>>> - What about those patches on the 10th page from previous year? Why
>>> are they in the list?
>>
>> Nothing.
>>
>>> - Do patch submitters regularly clean up outdated patches?
>>
>> No.
>>
>>> - Does a patch submitter receive a notification email when he/she
>>> forgets about a patch over time?
>>
>> No.
>>
>>> It seems to me that the current review process isn't as efficient as it
>>> can be.
>>
>> To be blunt: that may be the case, but even if so, it's extremely unlikely
>> bordering on the impossible that comments from the sideline from somebody
>> who hasn't got an experience of contributing could ever be helpful.
> 
> Outsiders and enemies. That is so ... cool.

Again, I was and still am very much an outsider, I should say.
But a bit of sincere modesty goes a long way. And it did for me.

Please do note that I am just sharing my experiences and providing and
opinion of a "newcomer"/"outsider" here.
And my opinion is that it is not broken. And what is not broken does not need
any fixing.

I do not know how You are, Mr./Ms./Mrs. Atom-icon. Since that means You could
be anything from a lowly troll to a wizard in disguise dispensing wisdom,
I shall not dare to question Your credibility on the matter, but I must
point out that this makes it tad complicated to take You as someone serious
instead of someone who is just bored and has nothing better to do. I believe
You should realize that.

Lastly, I agree that some points You made do make a lot sense "on paper".
Sadly, that does not mean they always end up being good in practice.
Since I do plan on contributing to mesa again in the future, I am quite eager
to see how this turns out in the end, so I shall be watching closely. Oh boy! :]

Those are my 2¢
I Hope the currency of the said 2¢ is strong, making it a valuable 2¢, if You
catch my drift... ;]

- 
Gediminas Jakutis
LDK Varčiai www.varciai.lt



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


Re: [Mesa-dev] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Ilia Mirkin
Reviewed-by: Ilia Mirkin 

On Sun, Jun 5, 2016 at 12:56 PM, Samuel Pitoiset
 wrote:
> We should not call nouveau_bufctx_reset() inside a validate function.
> This only affects Fermi where images are aliased between 3D and CP.
>
> Signed-off-by: Samuel Pitoiset 
> Cc: "12.0" 
> ---
>  src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 1 +
>  src/gallium/drivers/nouveau/nvc0/nvc0_tex.c | 6 +-
>  2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> index 21c8b2e..59bbe1e 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> @@ -324,6 +324,7 @@ nvc0_compute_validate_surfaces(struct nvc0_context *nvc0)
> nvc0_validate_suf(nvc0, 5);
>
> /* Invalidate all FRAGMENT images because they are aliased with COMPUTE. 
> */
> +   nouveau_bufctx_reset(nvc0->bufctx_3d, NVC0_BIND_3D_SUF);
> nvc0->dirty_3d |= NVC0_NEW_3D_SURFACES;
> nvc0->images_dirty[4] |= nvc0->images_valid[4];
>  }
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
> index b54de0f..1a5d8ec 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
> @@ -994,11 +994,6 @@ nvc0_validate_suf(struct nvc0_context *nvc0, int s)
> struct nouveau_pushbuf *push = nvc0->base.pushbuf;
> struct nvc0_screen *screen = nvc0->screen;
>
> -   if (s == 5)
> -  nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_SUF);
> -   else
> -  nouveau_bufctx_reset(nvc0->bufctx_3d, NVC0_BIND_3D_SUF);
> -
> for (int i = 0; i < NVC0_MAX_IMAGES; ++i) {
>struct pipe_image_view *view = &nvc0->images[s][i];
>int width, height, depth;
> @@ -1099,6 +1094,7 @@ nvc0_update_surface_bindings(struct nvc0_context *nvc0)
> nvc0_validate_suf(nvc0, 4);
>
> /* Invalidate all COMPUTE images because they are aliased with FRAGMENT. 
> */
> +   nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_SUF);
> nvc0->dirty_cp |= NVC0_NEW_CP_SURFACES;
> nvc0->images_dirty[5] |= nvc0->images_valid[5];
>  }
> --
> 2.8.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] nvc0: do not clear surfaces bins in the validate function

2016-06-05 Thread Samuel Pitoiset
We should not call nouveau_bufctx_reset() inside a validate function.
This only affects Fermi where images are aliased between 3D and CP.

Signed-off-by: Samuel Pitoiset 
Cc: "12.0" 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 1 +
 src/gallium/drivers/nouveau/nvc0/nvc0_tex.c | 6 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
index 21c8b2e..59bbe1e 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
@@ -324,6 +324,7 @@ nvc0_compute_validate_surfaces(struct nvc0_context *nvc0)
nvc0_validate_suf(nvc0, 5);
 
/* Invalidate all FRAGMENT images because they are aliased with COMPUTE. */
+   nouveau_bufctx_reset(nvc0->bufctx_3d, NVC0_BIND_3D_SUF);
nvc0->dirty_3d |= NVC0_NEW_3D_SURFACES;
nvc0->images_dirty[4] |= nvc0->images_valid[4];
 }
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
index b54de0f..1a5d8ec 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
@@ -994,11 +994,6 @@ nvc0_validate_suf(struct nvc0_context *nvc0, int s)
struct nouveau_pushbuf *push = nvc0->base.pushbuf;
struct nvc0_screen *screen = nvc0->screen;
 
-   if (s == 5)
-  nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_SUF);
-   else
-  nouveau_bufctx_reset(nvc0->bufctx_3d, NVC0_BIND_3D_SUF);
-
for (int i = 0; i < NVC0_MAX_IMAGES; ++i) {
   struct pipe_image_view *view = &nvc0->images[s][i];
   int width, height, depth;
@@ -1099,6 +1094,7 @@ nvc0_update_surface_bindings(struct nvc0_context *nvc0)
nvc0_validate_suf(nvc0, 4);
 
/* Invalidate all COMPUTE images because they are aliased with FRAGMENT. */
+   nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_SUF);
nvc0->dirty_cp |= NVC0_NEW_CP_SURFACES;
nvc0->images_dirty[5] |= nvc0->images_valid[5];
 }
-- 
2.8.3

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


Re: [Mesa-dev] [PATCH v2] nvc0: re-validate images after launching a grid on Fermi

2016-06-05 Thread Ilia Mirkin
On Sun, Jun 5, 2016 at 12:28 PM, Samuel Pitoiset
 wrote:
> Images invalidation is a bit weird on Fermi and there is already a hack
> which forces invalidating all images when launching a computer shader
> to help in fixing 3D<->CP interaction.
>
> However, we need to re-validate images for compute because
> nvc0_compute_invalidate_surfaces() will destroy the previous binding.
> This is not really good for performance purposes but this might be
> improved later.
>
> This fixes the following piglits:
> - spec/arb_compute_shader/execution/basic-uniform-access
> - spec/arb_compute_shader/execution/mutiple-texture-reading
> - spec/arb_compute_shader/execution/multiple-workgroups
> - spec/glsl-4.30/execution/built-in-functions/cs-* (207 tests)
>
> v2: reset CP_SUF bins
>
> Signed-off-by: Samuel Pitoiset 
> Cc: "12.0" 

Reviewed-by: Ilia Mirkin 

> ---
>  src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> index 8c88225..21c8b2e 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> @@ -458,4 +458,7 @@ nvc0_launch_grid(struct pipe_context *pipe, const struct 
> pipe_grid_info *info)
>
> /* TODO: Not sure if this is really necessary. */
> nvc0_compute_invalidate_surfaces(nvc0, 5);
> +   nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_SUF);
> +   nvc0->dirty_cp |= NVC0_NEW_CP_SURFACES;
> +   nvc0->images_dirty[5] |= nvc0->images_valid[5];
>  }
> --
> 2.8.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] st/mesa: change SQRT lowering to fix the game Risen

2016-06-05 Thread Axel Davy
if abs is not inserted in the source glsl, that's a wine bug, not a mesa 
bug,

Thus I don't think doing this is a good idea.

On 05/06/2016 17:17, Ilia Mirkin wrote:

On Mon, May 30, 2016 at 7:19 PM, Marek Olšák  wrote:

From: Marek Olšák 

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94627
(against nouveau)
---
  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 16 +---
  1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index aa443a5..0f5ee02 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -1901,13 +1901,15 @@ glsl_to_tgsi_visitor::visit_expression(ir_expression* 
ir, st_src_reg *op)
if (have_sqrt) {
   emit_scalar(ir, TGSI_OPCODE_SQRT, result_dst, op[0]);
} else {
- /* sqrt(x) = x * rsq(x). */
- emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, op[0]);
- emit_asm(ir, TGSI_OPCODE_MUL, result_dst, result_src, op[0]);
- /* For incoming channels <= 0, set the result to 0. */
- op[0].negate = ~op[0].negate;
- emit_asm(ir, TGSI_OPCODE_CMP, result_dst,
-  op[0], result_src, st_src_reg_for_float(0.0));
+ /* This is the only instruction sequence that makes the game "Risen"
+  * render correctly. ABS is not required for the game, but since GLSL
+  * declares negative values as "undefined", allowing us to do whatever
+  * we want, I choose to use ABS to match DX9 and pre-GLSL RSQ
+  * behavior.
+  */
+ emit_scalar(ir, TGSI_OPCODE_ABS, result_dst, op[0]);
+ emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, result_src);
+ emit_scalar(ir, TGSI_OPCODE_RCP, result_dst, result_src);

I spent a bunch of time trying to come up with alternatives that still
had the desired behavior for 0 and Infinity (since RCP sucks in terms
of precision). At the end of the day, it depends on what the hw does
with RSQ(0) and RCP(0). For nv50/nvc0, this sequence did have the
desired behavior - perhaps that holds for all hardware?

Either way, I'm fairly ambivalent about this - nv50/nvc0 now claim to
have SQRT support (and perform the lowering in the backend), and I
doubt much of anything would care on nv30. I'd just as soon leave the
abs out of it, but I don't strongly care.

Acked-by: Ilia Mirkin 


}
break;
 case ir_unop_rsq:
--
2.7.4

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

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



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


[Mesa-dev] [PATCH v2] nvc0: re-validate images after launching a grid on Fermi

2016-06-05 Thread Samuel Pitoiset
Images invalidation is a bit weird on Fermi and there is already a hack
which forces invalidating all images when launching a computer shader
to help in fixing 3D<->CP interaction.

However, we need to re-validate images for compute because
nvc0_compute_invalidate_surfaces() will destroy the previous binding.
This is not really good for performance purposes but this might be
improved later.

This fixes the following piglits:
- spec/arb_compute_shader/execution/basic-uniform-access
- spec/arb_compute_shader/execution/mutiple-texture-reading
- spec/arb_compute_shader/execution/multiple-workgroups
- spec/glsl-4.30/execution/built-in-functions/cs-* (207 tests)

v2: reset CP_SUF bins

Signed-off-by: Samuel Pitoiset 
Cc: "12.0" 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
index 8c88225..21c8b2e 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
@@ -458,4 +458,7 @@ nvc0_launch_grid(struct pipe_context *pipe, const struct 
pipe_grid_info *info)
 
/* TODO: Not sure if this is really necessary. */
nvc0_compute_invalidate_surfaces(nvc0, 5);
+   nouveau_bufctx_reset(nvc0->bufctx_cp, NVC0_BIND_CP_SUF);
+   nvc0->dirty_cp |= NVC0_NEW_CP_SURFACES;
+   nvc0->images_dirty[5] |= nvc0->images_valid[5];
 }
-- 
2.8.3

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


Re: [Mesa-dev] [PATCH] nvc0: re-validate images after launching a grid on Fermi

2016-06-05 Thread Ilia Mirkin
On Sun, Jun 5, 2016 at 12:13 PM, Samuel Pitoiset
 wrote:
> Images invalidation is a bit weird on Fermi and there is already a hack
> which forces invalidating all images when launching a computer shader
> to help in fixing 3D<->CP interaction.
>
> However, we need to re-validate images for compute because
> nvc0_compute_invalidate_surfaces() will destroy the previous binding.
> This is not really good for performance purposes but this might be
> improved later.
>
> This fixes the following piglits:
> - spec/arb_compute_shader/execution/basic-uniform-access
> - spec/arb_compute_shader/execution/mutiple-texture-reading
> - spec/arb_compute_shader/execution/multiple-workgroups
> - spec/glsl-4.30/execution/built-in-functions/cs-* (207 tests)
>
> Signed-off-by: Samuel Pitoiset 
> Cc: "12.0" 
> ---
>  src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> index 8c88225..baeeca5 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
> @@ -458,4 +458,6 @@ nvc0_launch_grid(struct pipe_context *pipe, const struct 
> pipe_grid_info *info)
>
> /* TODO: Not sure if this is really necessary. */
> nvc0_compute_invalidate_surfaces(nvc0, 5);
> +   nvc0->dirty_cp |= NVC0_NEW_CP_SURFACES;
> +   nvc0->images_dirty[5] |= nvc0->images_valid[5];

Don't forget to clear out the bufctx. Also (in another patch), do a
pass over all your invalidation hacks - any time you do nvc0->dirty |=
foo, you should have an associated nouveau_bufctx_reset(). Otherwise
things can start piling up in there. (Since validation only adds to
the buckets.)

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


[Mesa-dev] [PATCH] nvc0: re-validate images after launching a grid on Fermi

2016-06-05 Thread Samuel Pitoiset
Images invalidation is a bit weird on Fermi and there is already a hack
which forces invalidating all images when launching a computer shader
to help in fixing 3D<->CP interaction.

However, we need to re-validate images for compute because
nvc0_compute_invalidate_surfaces() will destroy the previous binding.
This is not really good for performance purposes but this might be
improved later.

This fixes the following piglits:
- spec/arb_compute_shader/execution/basic-uniform-access
- spec/arb_compute_shader/execution/mutiple-texture-reading
- spec/arb_compute_shader/execution/multiple-workgroups
- spec/glsl-4.30/execution/built-in-functions/cs-* (207 tests)

Signed-off-by: Samuel Pitoiset 
Cc: "12.0" 
---
 src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
index 8c88225..baeeca5 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
@@ -458,4 +458,6 @@ nvc0_launch_grid(struct pipe_context *pipe, const struct 
pipe_grid_info *info)
 
/* TODO: Not sure if this is really necessary. */
nvc0_compute_invalidate_surfaces(nvc0, 5);
+   nvc0->dirty_cp |= NVC0_NEW_CP_SURFACES;
+   nvc0->images_dirty[5] |= nvc0->images_valid[5];
 }
-- 
2.8.3

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


Re: [Mesa-dev] [PATCH] st/mesa: change SQRT lowering to fix the game Risen

2016-06-05 Thread Ilia Mirkin
On Mon, May 30, 2016 at 7:19 PM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94627
> (against nouveau)
> ---
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index aa443a5..0f5ee02 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -1901,13 +1901,15 @@ glsl_to_tgsi_visitor::visit_expression(ir_expression* 
> ir, st_src_reg *op)
>if (have_sqrt) {
>   emit_scalar(ir, TGSI_OPCODE_SQRT, result_dst, op[0]);
>} else {
> - /* sqrt(x) = x * rsq(x). */
> - emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, op[0]);
> - emit_asm(ir, TGSI_OPCODE_MUL, result_dst, result_src, op[0]);
> - /* For incoming channels <= 0, set the result to 0. */
> - op[0].negate = ~op[0].negate;
> - emit_asm(ir, TGSI_OPCODE_CMP, result_dst,
> -  op[0], result_src, st_src_reg_for_float(0.0));
> + /* This is the only instruction sequence that makes the game "Risen"
> +  * render correctly. ABS is not required for the game, but since 
> GLSL
> +  * declares negative values as "undefined", allowing us to do 
> whatever
> +  * we want, I choose to use ABS to match DX9 and pre-GLSL RSQ
> +  * behavior.
> +  */
> + emit_scalar(ir, TGSI_OPCODE_ABS, result_dst, op[0]);
> + emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, result_src);
> + emit_scalar(ir, TGSI_OPCODE_RCP, result_dst, result_src);

I spent a bunch of time trying to come up with alternatives that still
had the desired behavior for 0 and Infinity (since RCP sucks in terms
of precision). At the end of the day, it depends on what the hw does
with RSQ(0) and RCP(0). For nv50/nvc0, this sequence did have the
desired behavior - perhaps that holds for all hardware?

Either way, I'm fairly ambivalent about this - nv50/nvc0 now claim to
have SQRT support (and perform the lowering in the backend), and I
doubt much of anything would care on nv30. I'd just as soon leave the
abs out of it, but I don't strongly care.

Acked-by: Ilia Mirkin 

>}
>break;
> case ir_unop_rsq:
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/3] radeonsi: use hw MSAA resolve for non-trivial resolves

2016-06-05 Thread Marek Olšák
From: Marek Olšák 

This improves MSAA resolve performance.
---
 src/gallium/drivers/radeonsi/si_blit.c | 62 +-
 1 file changed, 53 insertions(+), 9 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_blit.c 
b/src/gallium/drivers/radeonsi/si_blit.c
index 3748a59..1c2c538 100644
--- a/src/gallium/drivers/radeonsi/si_blit.c
+++ b/src/gallium/drivers/radeonsi/si_blit.c
@@ -857,6 +857,16 @@ static bool do_hardware_msaa_resolve(struct pipe_context 
*ctx,
unsigned dst_height = u_minify(info->dst.resource->height0, 
info->dst.level);
enum pipe_format format = info->dst.format;
unsigned sample_mask = ~0;
+   struct pipe_resource *tmp, templ;
+   struct pipe_blit_info blit;
+
+   /* Check basic requirements for hw resolve. */
+   if (!(info->src.resource->nr_samples > 1 &&
+ info->dst.resource->nr_samples <= 1 &&
+ !util_format_is_pure_integer(format) &&
+ !util_format_is_depth_or_stencil(format) &&
+ util_max_layer(info->src.resource, 0) == 0))
+   return false;
 
/* Hardware MSAA resolve doesn't work if SPI format = NORM16_ABGR and
 * the format is R16G16. Use R16A16, which does work.
@@ -866,16 +876,12 @@ static bool do_hardware_msaa_resolve(struct pipe_context 
*ctx,
if (format == PIPE_FORMAT_R16G16_SNORM)
format = PIPE_FORMAT_R16A16_SNORM;
 
-   if (info->src.resource->nr_samples > 1 &&
-   info->dst.resource->nr_samples <= 1 &&
-   util_max_layer(info->src.resource, 0) == 0 &&
-   util_max_layer(info->dst.resource, info->dst.level) == 0 &&
-   util_is_format_compatible(util_format_description(info->src.format),
- 
util_format_description(info->dst.format)) &&
-   !util_format_is_pure_integer(format) &&
-   !util_format_is_depth_or_stencil(format) &&
+   /* Check the remaining requirements for hw resolve. */
+   if (util_max_layer(info->dst.resource, info->dst.level) == 0 &&
!info->scissor_enable &&
(info->mask & PIPE_MASK_RGBA) == PIPE_MASK_RGBA &&
+   util_is_format_compatible(util_format_description(info->src.format),
+ 
util_format_description(info->dst.format)) &&
dst_width == info->src.resource->width0 &&
dst_height == info->src.resource->height0 &&
info->dst.box.x == 0 &&
@@ -903,7 +909,45 @@ static bool do_hardware_msaa_resolve(struct pipe_context 
*ctx,
si_blitter_end(ctx);
return true;
}
-   return false;
+
+   /* Shader-based resolve is VERY SLOW. Instead, resolve into
+* a temporary texture and blit.
+*/
+   memset(&templ, 0, sizeof(templ));
+   templ.target = PIPE_TEXTURE_2D;
+   templ.format = info->src.resource->format;
+   templ.width0 = info->src.resource->width0;
+   templ.height0 = info->src.resource->height0;
+   templ.depth0 = 1;
+   templ.array_size = 1;
+   templ.usage = PIPE_USAGE_DEFAULT;
+   templ.flags = R600_RESOURCE_FLAG_FORCE_TILING;
+
+   tmp = ctx->screen->resource_create(ctx->screen, &templ);
+   if (!tmp)
+   return false;
+
+   /* resolve */
+   si_blitter_begin(ctx, SI_COLOR_RESOLVE |
+(info->render_condition_enable ? 0 : 
SI_DISABLE_RENDER_COND));
+   util_blitter_custom_resolve_color(sctx->blitter, tmp, 0, 0,
+ info->src.resource, info->src.box.z,
+ sample_mask, 
sctx->custom_blend_resolve,
+ format);
+   si_blitter_end(ctx);
+
+   /* blit */
+   blit = *info;
+   blit.src.resource = tmp;
+   blit.src.box.z = 0;
+
+   si_blitter_begin(ctx, SI_BLIT |
+(info->render_condition_enable ? 0 : 
SI_DISABLE_RENDER_COND));
+   util_blitter_blit(sctx->blitter, &blit);
+   si_blitter_end(ctx);
+
+   pipe_resource_reference(&tmp, NULL);
+   return true;
 }
 
 static void si_blit(struct pipe_context *ctx,
-- 
2.7.4

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


[Mesa-dev] [PATCH 3/3] r600g: use hw MSAA resolve for non-trivial resolves

2016-06-05 Thread Marek Olšák
From: Marek Olšák 

This improves MSAA resolve performance.
---
 src/gallium/drivers/r600/r600_blit.c | 60 +++-
 1 file changed, 52 insertions(+), 8 deletions(-)

diff --git a/src/gallium/drivers/r600/r600_blit.c 
b/src/gallium/drivers/r600/r600_blit.c
index c9d7823..6277fa3 100644
--- a/src/gallium/drivers/r600/r600_blit.c
+++ b/src/gallium/drivers/r600/r600_blit.c
@@ -761,15 +761,21 @@ static bool do_hardware_msaa_resolve(struct pipe_context 
*ctx,
unsigned sample_mask =
rctx->b.chip_class == CAYMAN ? ~0 :
((1ull << MAX2(1, info->src.resource->nr_samples)) - 1);
-
-   if (info->src.resource->nr_samples > 1 &&
-   info->dst.resource->nr_samples <= 1 &&
-   util_max_layer(info->src.resource, 0) == 0 &&
-   util_max_layer(info->dst.resource, info->dst.level) == 0 &&
+   struct pipe_resource *tmp, templ;
+   struct pipe_blit_info blit;
+
+   /* Check basic requirements for hw resolve. */
+   if (!(info->src.resource->nr_samples > 1 &&
+ info->dst.resource->nr_samples <= 1 &&
+ !util_format_is_pure_integer(format) &&
+ !util_format_is_depth_or_stencil(format) &&
+ util_max_layer(info->src.resource, 0) == 0))
+   return false;
+
+   /* Check the remaining requirements for hw resolve. */
+   if (util_max_layer(info->dst.resource, info->dst.level) == 0 &&
util_is_format_compatible(util_format_description(info->src.format),
  
util_format_description(info->dst.format)) &&
-   !util_format_is_pure_integer(format) &&
-   !util_format_is_depth_or_stencil(format) &&
!info->scissor_enable &&
(info->mask & PIPE_MASK_RGBA) == PIPE_MASK_RGBA &&
dst_width == info->src.resource->width0 &&
@@ -797,7 +803,45 @@ static bool do_hardware_msaa_resolve(struct pipe_context 
*ctx,
r600_blitter_end(ctx);
return true;
}
-   return false;
+
+   /* Shader-based resolve is VERY SLOW. Instead, resolve into
+* a temporary texture and blit.
+*/
+   memset(&templ, 0, sizeof(templ));
+   templ.target = PIPE_TEXTURE_2D;
+   templ.format = info->src.resource->format;
+   templ.width0 = info->src.resource->width0;
+   templ.height0 = info->src.resource->height0;
+   templ.depth0 = 1;
+   templ.array_size = 1;
+   templ.usage = PIPE_USAGE_DEFAULT;
+   templ.flags = R600_RESOURCE_FLAG_FORCE_TILING;
+
+   tmp = ctx->screen->resource_create(ctx->screen, &templ);
+   if (!tmp)
+   return false;
+
+   /* resolve */
+   r600_blitter_begin(ctx, R600_COLOR_RESOLVE |
+  (info->render_condition_enable ? 0 : 
R600_DISABLE_RENDER_COND));
+   util_blitter_custom_resolve_color(rctx->blitter, tmp, 0, 0,
+ info->src.resource, info->src.box.z,
+ sample_mask, 
rctx->custom_blend_resolve,
+ format);
+   r600_blitter_end(ctx);
+
+   /* blit */
+   blit = *info;
+   blit.src.resource = tmp;
+   blit.src.box.z = 0;
+
+   r600_blitter_begin(ctx, R600_BLIT |
+  (info->render_condition_enable ? 0 : 
R600_DISABLE_RENDER_COND));
+   util_blitter_blit(rctx->blitter, &blit);
+   r600_blitter_end(ctx);
+
+   pipe_resource_reference(&tmp, NULL);
+   return true;
 }
 
 static void r600_blit(struct pipe_context *ctx,
-- 
2.7.4

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


[Mesa-dev] [PATCH 0/3] R600-GCN: Use hw MSAA resolve for non-trivial resolves

2016-06-05 Thread Marek Olšák
Hi,

The shader-based resolve is slow.

With this series, one scene with 8xMSAA in HL2: Lost Coast goes from 8-9 FPS to 
32 FPS on Evergreen & Wine/Nine.

Please review.

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


[Mesa-dev] [PATCH 2/3] radeonsi: use hw MSAA resolve for layered resolves

2016-06-05 Thread Marek Olšák
From: Marek Olšák 

---
 src/gallium/drivers/radeonsi/si_blit.c | 34 +-
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_blit.c 
b/src/gallium/drivers/radeonsi/si_blit.c
index 1c2c538..80844ec 100644
--- a/src/gallium/drivers/radeonsi/si_blit.c
+++ b/src/gallium/drivers/radeonsi/si_blit.c
@@ -859,13 +859,13 @@ static bool do_hardware_msaa_resolve(struct pipe_context 
*ctx,
unsigned sample_mask = ~0;
struct pipe_resource *tmp, templ;
struct pipe_blit_info blit;
+   unsigned i;
 
-   /* Check basic requirements for hw resolve. */
+   /* Check basic requirements for MSAA resolve. */
if (!(info->src.resource->nr_samples > 1 &&
  info->dst.resource->nr_samples <= 1 &&
  !util_format_is_pure_integer(format) &&
- !util_format_is_depth_or_stencil(format) &&
- util_max_layer(info->src.resource, 0) == 0))
+ !util_format_is_depth_or_stencil(format)))
return false;
 
/* Hardware MSAA resolve doesn't work if SPI format = NORM16_ABGR and
@@ -878,6 +878,7 @@ static bool do_hardware_msaa_resolve(struct pipe_context 
*ctx,
 
/* Check the remaining requirements for hw resolve. */
if (util_max_layer(info->dst.resource, info->dst.level) == 0 &&
+   util_max_layer(info->src.resource, 0) == 0 &&
!info->scissor_enable &&
(info->mask & PIPE_MASK_RGBA) == PIPE_MASK_RGBA &&
util_is_format_compatible(util_format_description(info->src.format),
@@ -914,12 +915,12 @@ static bool do_hardware_msaa_resolve(struct pipe_context 
*ctx,
 * a temporary texture and blit.
 */
memset(&templ, 0, sizeof(templ));
-   templ.target = PIPE_TEXTURE_2D;
+   templ.target = info->src.resource->target;
templ.format = info->src.resource->format;
templ.width0 = info->src.resource->width0;
templ.height0 = info->src.resource->height0;
templ.depth0 = 1;
-   templ.array_size = 1;
+   templ.array_size = info->src.resource->array_size;
templ.usage = PIPE_USAGE_DEFAULT;
templ.flags = R600_RESOURCE_FLAG_FORCE_TILING;
 
@@ -927,14 +928,21 @@ static bool do_hardware_msaa_resolve(struct pipe_context 
*ctx,
if (!tmp)
return false;
 
-   /* resolve */
-   si_blitter_begin(ctx, SI_COLOR_RESOLVE |
-(info->render_condition_enable ? 0 : 
SI_DISABLE_RENDER_COND));
-   util_blitter_custom_resolve_color(sctx->blitter, tmp, 0, 0,
- info->src.resource, info->src.box.z,
- sample_mask, 
sctx->custom_blend_resolve,
- format);
-   si_blitter_end(ctx);
+   /* resolve all layers that need to be resolved */
+   assert(info->src.box.depth > 0);
+   for (i = 0; i < info->src.box.depth; i++) {
+   si_blitter_begin(ctx, SI_COLOR_RESOLVE |
+(info->render_condition_enable ?
+ 0 : SI_DISABLE_RENDER_COND));
+   util_blitter_custom_resolve_color(sctx->blitter, tmp, 0,
+ info->src.box.z + i,
+ info->src.resource,
+ info->src.box.z + i,
+ sample_mask,
+ sctx->custom_blend_resolve,
+ format);
+   si_blitter_end(ctx);
+   }
 
/* blit */
blit = *info;
-- 
2.7.4

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


Re: [Mesa-dev] [PATCH] st/mesa: change SQRT lowering to fix the game Risen

2016-06-05 Thread Marek Olšák
Ping. This is a more accurate version than RSQ+MUL+CMP.

Marek

On Tue, May 31, 2016 at 1:19 AM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94627
> (against nouveau)
> ---
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index aa443a5..0f5ee02 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -1901,13 +1901,15 @@ glsl_to_tgsi_visitor::visit_expression(ir_expression* 
> ir, st_src_reg *op)
>if (have_sqrt) {
>   emit_scalar(ir, TGSI_OPCODE_SQRT, result_dst, op[0]);
>} else {
> - /* sqrt(x) = x * rsq(x). */
> - emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, op[0]);
> - emit_asm(ir, TGSI_OPCODE_MUL, result_dst, result_src, op[0]);
> - /* For incoming channels <= 0, set the result to 0. */
> - op[0].negate = ~op[0].negate;
> - emit_asm(ir, TGSI_OPCODE_CMP, result_dst,
> -  op[0], result_src, st_src_reg_for_float(0.0));
> + /* This is the only instruction sequence that makes the game "Risen"
> +  * render correctly. ABS is not required for the game, but since 
> GLSL
> +  * declares negative values as "undefined", allowing us to do 
> whatever
> +  * we want, I choose to use ABS to match DX9 and pre-GLSL RSQ
> +  * behavior.
> +  */
> + emit_scalar(ir, TGSI_OPCODE_ABS, result_dst, op[0]);
> + emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, result_src);
> + emit_scalar(ir, TGSI_OPCODE_RCP, result_dst, result_src);
>}
>break;
> case ir_unop_rsq:
> --
> 2.7.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev