Re: [Mesa-dev] [PATCH 1/4] i965: Convert brw_bufmgr to use C11 mutexes instead of pthreads.

2017-09-26 Thread Timothy Arceri

On 25/09/17 03:37, Kenneth Graunke wrote:

On Sunday, September 24, 2017 7:36:17 AM PDT Jason Ekstrand wrote:

On September 24, 2017 01:02:21 Kenneth Graunke  wrote:

There's no real advantage or disadvantage here, it's just for stylistic
consistency with the rest of the codebase.


Didn't Kristian do a bunch of work done time ago to make mutexes faster by
not using pthread on Linux?  If so, this should be an unnoticeable
performance improvement.

--Jason


Kristian did a bunch of work that never landed.  krhmutex~1 of my tree
has his patch - IIRC it got review feedback that was never addressed.


I took a quick look. Seems everything got addressed besides the clashing 
with C11 [1].


[1] https://lists.freedesktop.org/archives/mesa-dev/2015-January/075415.html



Someone should probably revive that.  radeonsi might get more of a boost
at this point...


Should be fairly straight forward task if someone wanted to take a look, 
its the second and third last patches [2]. I suspect the draw overhead 
piglit test should show some improvements if we are going to see any [3].


[2] https://cgit.freedesktop.org/~kwg/mesa/log/?h=krhmutex
[3] https://cgit.freedesktop.org/piglit/tree/tests/perf/drawoverhead.c

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


[Mesa-dev] [PATCH] i965: Convert brw->*_program into a brw->programs[i] array.

2017-09-26 Thread Kenneth Graunke
This makes it easier to loop over programs.
---
 src/mesa/drivers/dri/i965/brw_binding_tables.c|  6 +--
 src/mesa/drivers/dri/i965/brw_context.h   |  7 +--
 src/mesa/drivers/dri/i965/brw_cs.c|  6 ++-
 src/mesa/drivers/dri/i965/brw_curbe.c | 12 +++--
 src/mesa/drivers/dri/i965/brw_gs.c|  6 ++-
 src/mesa/drivers/dri/i965/brw_gs_surface_state.c  |  7 +--
 src/mesa/drivers/dri/i965/brw_program.c   | 31 -
 src/mesa/drivers/dri/i965/brw_sf.c|  2 +-
 src/mesa/drivers/dri/i965/brw_state_upload.c  | 36 ---
 src/mesa/drivers/dri/i965/brw_tcs.c   | 11 +++--
 src/mesa/drivers/dri/i965/brw_tcs_surface_state.c |  7 +--
 src/mesa/drivers/dri/i965/brw_tes.c   |  9 ++--
 src/mesa/drivers/dri/i965/brw_tes_surface_state.c |  7 +--
 src/mesa/drivers/dri/i965/brw_vs.c|  7 +--
 src/mesa/drivers/dri/i965/brw_vs_surface_state.c  |  7 +--
 src/mesa/drivers/dri/i965/brw_wm.c|  7 +--
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c  | 24 +-
 src/mesa/drivers/dri/i965/gen6_sol.c  | 11 ++---
 src/mesa/drivers/dri/i965/gen6_urb.c  |  5 ++-
 src/mesa/drivers/dri/i965/gen7_cs_state.c |  6 ++-
 src/mesa/drivers/dri/i965/gen7_urb.c  |  4 +-
 src/mesa/drivers/dri/i965/genX_state_upload.c | 55 +--
 22 files changed, 147 insertions(+), 126 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_binding_tables.c 
b/src/mesa/drivers/dri/i965/brw_binding_tables.c
index 4bbaa5d0594..73f5e56010b 100644
--- a/src/mesa/drivers/dri/i965/brw_binding_tables.c
+++ b/src/mesa/drivers/dri/i965/brw_binding_tables.c
@@ -153,7 +153,7 @@ static void
 brw_tcs_upload_binding_table(struct brw_context *brw)
 {
/* Skip if the tessellation stages are disabled. */
-   if (brw->tess_eval_program == NULL)
+   if (brw->programs[MESA_SHADER_TESS_EVAL] == NULL)
   return;
 
/* BRW_NEW_TCS_PROG_DATA */
@@ -182,7 +182,7 @@ static void
 brw_tes_upload_binding_table(struct brw_context *brw)
 {
/* If there's no TES, skip changing anything. */
-   if (brw->tess_eval_program == NULL)
+   if (brw->programs[MESA_SHADER_TESS_EVAL] == NULL)
   return;
 
/* BRW_NEW_TES_PROG_DATA */
@@ -210,7 +210,7 @@ static void
 brw_gs_upload_binding_table(struct brw_context *brw)
 {
/* If there's no GS, skip changing anything. */
-   if (brw->geometry_program == NULL)
+   if (brw->programs[MESA_SHADER_GEOMETRY] == NULL)
   return;
 
/* BRW_NEW_GS_PROG_DATA */
diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 6de5c2d0974..bc3d3e398be 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -875,12 +875,7 @@ struct brw_context
 
/* Active vertex program:
 */
-   const struct gl_program *vertex_program;
-   const struct gl_program *geometry_program;
-   const struct gl_program *tess_ctrl_program;
-   const struct gl_program *tess_eval_program;
-   const struct gl_program *fragment_program;
-   const struct gl_program *compute_program;
+   struct gl_program *programs[MESA_SHADER_STAGES];
 
/**
 * Number of samples in ctx->DrawBuffer, updated by BRW_NEW_NUM_SAMPLES so
diff --git a/src/mesa/drivers/dri/i965/brw_cs.c 
b/src/mesa/drivers/dri/i965/brw_cs.c
index cf72889b411..bc09abd912c 100644
--- a/src/mesa/drivers/dri/i965/brw_cs.c
+++ b/src/mesa/drivers/dri/i965/brw_cs.c
@@ -177,7 +177,8 @@ brw_cs_populate_key(struct brw_context *brw, struct 
brw_cs_prog_key *key)
 {
struct gl_context *ctx = &brw->ctx;
/* BRW_NEW_COMPUTE_PROGRAM */
-   const struct brw_program *cp = (struct brw_program *) brw->compute_program;
+   const struct brw_program *cp =
+  (struct brw_program *) brw->programs[MESA_SHADER_COMPUTE];
const struct gl_program *prog = (struct gl_program *) cp;
 
memset(key, 0, sizeof(*key));
@@ -195,7 +196,8 @@ brw_upload_cs_prog(struct brw_context *brw)
 {
struct gl_context *ctx = &brw->ctx;
struct brw_cs_prog_key key;
-   struct brw_program *cp = (struct brw_program *) brw->compute_program;
+   struct brw_program *cp =
+  (struct brw_program *) brw->programs[MESA_SHADER_COMPUTE];
 
if (!cp)
   return;
diff --git a/src/mesa/drivers/dri/i965/brw_curbe.c 
b/src/mesa/drivers/dri/i965/brw_curbe.c
index 7adf911dea0..07e3c42b152 100644
--- a/src/mesa/drivers/dri/i965/brw_curbe.c
+++ b/src/mesa/drivers/dri/i965/brw_curbe.c
@@ -204,6 +204,12 @@ brw_upload_constant_buffer(struct brw_context *brw)
GLuint i;
gl_clip_plane *clip_planes;
 
+   /* BRW_NEW_FRAGMENT_PROGRAM */
+   struct gl_program *fp = brw->programs[MESA_SHADER_FRAGMENT];
+
+   /* BRW_NEW_VERTEX_PROGRAM */
+   struct gl_program *vp = brw->programs[MESA_SHADER_VERTEX];
+
if (sz == 0) {
   goto emit;
}
@@ -215,7 +221,7 @@ brw_upload_constant_buffer(struct brw_context *brw)
 
/* fragment shader constant

[Mesa-dev] [Bug 102852] Scons: Support the new Scons 3.0.0

2017-09-26 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=102852

Alex Granni  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|NEW |RESOLVED

--- Comment #5 from Alex Granni  ---
 Eric Engestrom py2/3 compatibility patch series landed in Mesa master
yesterday.
The first patch in the series address this issue so I am closing this.

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


[Mesa-dev] [PATCH] vc4: Fix infinite retry in vc4_bo_alloc()

2017-09-26 Thread Boris Brezillon
cleared_and_retried is always reset to false when jumping to the retry
label, thus leading to an infinite retry loop.

Fix that by moving the cleared_and_retried variable definitions at the
beginning of the function.
While we're at it, move the create variable with the other local
variables and explicitly reset its content in the retry path.

Signed-off-by: Boris Brezillon 
---
 src/gallium/drivers/vc4/vc4_bufmgr.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/gallium/drivers/vc4/vc4_bufmgr.c 
b/src/gallium/drivers/vc4/vc4_bufmgr.c
index 12af7f8a9ef2..0653f8823232 100644
--- a/src/gallium/drivers/vc4/vc4_bufmgr.c
+++ b/src/gallium/drivers/vc4/vc4_bufmgr.c
@@ -123,6 +123,8 @@ vc4_bo_from_cache(struct vc4_screen *screen, uint32_t size, 
const char *name)
 struct vc4_bo *
 vc4_bo_alloc(struct vc4_screen *screen, uint32_t size, const char *name)
 {
+bool cleared_and_retried = false;
+struct drm_vc4_create_bo create;
 struct vc4_bo *bo;
 int ret;
 
@@ -149,12 +151,8 @@ vc4_bo_alloc(struct vc4_screen *screen, uint32_t size, 
const char *name)
 bo->private = true;
 
  retry:
-;
-
-bool cleared_and_retried = false;
-struct drm_vc4_create_bo create = {
-.size = size
-};
+memset(&create, 0, sizeof(create));
+create.size = size;
 
 ret = vc4_ioctl(screen->fd, DRM_IOCTL_VC4_CREATE_BO, &create);
 bo->handle = create.handle;
-- 
2.11.0

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


[Mesa-dev] [PATCH] i965: skip reading clip distances from the URB for the FS if possible

2017-09-26 Thread Iago Toral Quiroga
we can skip these slots when they are not read in the fragment shader
and they are positioned right after a VUE header that we are already
skipping. We also need to ensure that we are passing at least one other
varying, since that is a hardware requirement.

This helps alleviate a problem introduced with 99df02ca26f61 for
separate shader objects: without separate shader objects we assign
locations sequentially, however, since that commit we have changed the
method for SSO so that the VUE slot assigned depends on the number of
builtin slots plus the location assigned to the varying. This fixed
layout is intended to help SSO programs by avoiding on-the-fly recompiles
when swapping out shaders, however, it also means that if a varying uses
a large location number close to the maximum allowed by the SF/FS units
(31), then the offset introduced by the number of builtin slots can push
the location outside the range and trigger an assertion.

This problem is affecting at least the following CTS tests for
enhanced layouts:

KHR-GL45.enhanced_layouts.varying_array_components
KHR-GL45.enhanced_layouts.varying_array_locations
KHR-GL45.enhanced_layouts.varying_components
KHR-GL45.enhanced_layouts.varying_locations

which use SSO and the the location layout qualifier to select such
location numbers explicitly.

This change helps these tests because for SSO we always have to include
VARYING_SLOT_CLIP_DIST{0,1} even if the fragment shader is very unlikely
to read them, so by doing this we free builtin slots from the fixed VUE
layout and we avoid the tests to crash in this scenario.

Of course, this is not a proper fix, we'd still run into problems if someone
tries to use an explicit max location and read gl_ViewportIndex, gl_LayerID or
gl_CullDistancein in the FS, but that would be a much less common bug and we
can probably wait to see if anyone actually runs into that situation in a real
world scenario before making the decision that more aggresive changes are
required to support this without reverting 99df02ca26f61.

Suggested-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/genX_state_upload.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c 
b/src/mesa/drivers/dri/i965/genX_state_upload.c
index 612761601a..ce3515067f 100644
--- a/src/mesa/drivers/dri/i965/genX_state_upload.c
+++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
@@ -1040,6 +1040,31 @@ genX(calculate_attr_overrides)(const struct brw_context 
*brw,
 
*urb_entry_read_offset = fs_needs_vue_header ? 0 : 1;
 
+   /* If we are already skipping the first 2 slots for the VUE header then we
+* can also skip clip distances if they are located right after the header
+* and the FS doesn't read them. Only do this is there are any user varyings
+* though, since the hardware requires that we pass at least one varying
+* between stages.
+*/
+   uint64_t var_bits = ~(VARYING_BIT_VAR(0) - 1);
+   uint64_t clip_dist_slots = VARYING_BIT_CLIP_DIST0 | VARYING_BIT_CLIP_DIST1;
+   if (!fs_needs_vue_header &&
+   var_bits &&
+   (brw->fragment_program->info.inputs_read & clip_dist_slots) == 0 &&
+   (brw->vue_map_geom_out.slots_valid & clip_dist_slots)) {
+
+  uint32_t clip_dist0_slot =
+ brw->vue_map_geom_out.varying_to_slot[VARYING_SLOT_CLIP_DIST0];
+
+  uint32_t clip_dist1_slot =
+ brw->vue_map_geom_out.varying_to_slot[VARYING_SLOT_CLIP_DIST1];
+
+  if (clip_dist0_slot >= 2 && clip_dist0_slot <= 3 &&
+  clip_dist1_slot >= 2 && clip_dist1_slot <= 3) {
+ *urb_entry_read_offset = 2;
+  }
+   }
+
/* From the Ivybridge PRM, Vol 2 Part 1, 3DSTATE_SBE,
 * description of dw10 Point Sprite Texture Coordinate Enable:
 *
-- 
2.11.0

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


Re: [Mesa-dev] [PATCH v2 7/7] i965: make use of nir linking

2017-09-26 Thread Kenneth Graunke
On Monday, September 25, 2017 6:23:13 PM PDT Timothy Arceri wrote:
> For now linking is just removing unused varyings between stages.
> 
> shader-db results BDW:
> 
> total instructions in shared programs: 13198288 -> 13191693 (-0.05%)
> instructions in affected programs: 48325 -> 41730 (-13.65%)
> helped: 473
> HURT: 0
> 
> total cycles in shared programs: 541184926 -> 541159260 (-0.00%)
> cycles in affected programs: 213238 -> 187572 (-12.04%)
> helped: 435
> HURT: 8
> 
> V2:
> - lower indirects on demoted inputs as well as outputs.
> ---
>  src/mesa/drivers/dri/i965/brw_link.cpp | 56 
> ++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp 
> b/src/mesa/drivers/dri/i965/brw_link.cpp
> index b7fab8d7a25..9ddf0230183 100644
> --- a/src/mesa/drivers/dri/i965/brw_link.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_link.cpp
> @@ -253,6 +253,62 @@ brw_link_shader(struct gl_context *ctx, struct 
> gl_shader_program *shProg)
>   compiler->scalar_stage[stage]);
> }
>  
> +   /* Determine first and last stage. */
> +   unsigned first = MESA_SHADER_STAGES;
> +   unsigned last = 0;
> +   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> +  if (!shProg->_LinkedShaders[i])
> + continue;
> +  if (first == MESA_SHADER_STAGES)
> + first = i;
> +  last = i;
> +   }
> +
> +   /* Linking the stages in the opposite order (from fragment to vertex)
> +* ensures that inter-shader outputs written to in an earlier stage
> +* are eliminated if they are (transitively) not used in a later
> +* stage.
> +*/
> +if (first != last) {
> +   int next = last;
> +   for (int i = next - 1; i >= 0; i--) {
> +  if (shProg->_LinkedShaders[i] == NULL)
> + continue;
> +
> +nir_shader *producer = shProg->_LinkedShaders[i]->Program->nir;
> +nir_shader *consumer = 
> shProg->_LinkedShaders[next]->Program->nir;
> +
> +nir_remove_dead_variables(producer, nir_var_shader_out);
> +nir_remove_dead_variables(consumer, nir_var_shader_in);
> +
> +if (nir_remove_unused_varyings(producer, consumer)) {
> +   nir_lower_global_vars_to_local(producer);
> +   nir_lower_global_vars_to_local(consumer);
> +
> +   nir_variable_mode indirect_mask = (nir_variable_mode) 0;
> +   if (compiler->glsl_compiler_options[i].EmitNoIndirectTemp)
> +  indirect_mask = (nir_variable_mode) nir_var_local;
> +
> +   /* The backend might not be able to handle indirects on
> +* temporaries so we need to lower indirects on any of the
> +* varyings we have demoted here.
> +*/
> +   nir_lower_indirect_derefs(producer, indirect_mask);
> +   nir_lower_indirect_derefs(consumer, indirect_mask);
> +
> +   const bool p_is_scalar = 
> compiler->scalar_stage[producer->stage];
> +   shProg->_LinkedShaders[i]->Program->nir =
> + brw_nir_optimize(producer, compiler, p_is_scalar);
> +
> +   const bool c_is_scalar = 
> compiler->scalar_stage[producer->stage];
> +   shProg->_LinkedShaders[next]->Program->nir =
> + brw_nir_optimize(consumer, compiler, c_is_scalar);
> +}
> +
> +next = i;
> +   }
> +}
> +
> for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders); stage++) {
>struct gl_linked_shader *shader = shProg->_LinkedShaders[stage];
>if (!shader)
> 

A couple more thoughts:

1. We're calling brw_nir_optimize even more now...it might make sense to
   drop it from brw_preprocess_nir, drop it here, and put it once in the
   final (third) loop, just before brw_shader_gather_info.  At least,
   it'd be interesting how that affects compile times / quality...

2. It would be great to use this in anv as well, especially given that
   it has no GLSL IR linking to do this sort of stuff.  Plus, the SPIR-V
   might be compiled independently...and when you build the pipeline,
   you have all the stages and can do cross-stage linking optimizations
   like this.

Series looks great though, and is:
Reviewed-by: Kenneth Graunke 

Thanks Tim!


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


Re: [Mesa-dev] [PATCH 05/22] i965: correctly assign SamplerCount of INTERFACE_DESCRIPTOR_DATA

2017-09-26 Thread Kenneth Graunke
On Monday, September 25, 2017 5:46:32 AM PDT Lionel Landwerlin wrote:
> I'm genuinely surprised we didn't noticed this problem before :|
> 
> Fixes: 71bfb44005bf ("i965: Port brw_cs_state tracked state to genxml.")
> Reviewed-by: Lionel Landwerlin 
> Cc: "17.2" 

I'm not - these fields are just used for prefetching the sampler states,
so programming too small a number will just prefetch fewer of them.  No
big deal, certainly not going to hurt correctness.

I probably wouldn't bother Cc'ing it to stable.

That said, nice find :)  We should definitely fix this.


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


Re: [Mesa-dev] MESA and KOTOR

2017-09-26 Thread Federico Dossena

Hello everyone,
you may remember that a few months ago I was trying to fix KOTOR to work 
with Mesa to use the Gallium llvmpipe software renderer.


Well, it's been a while and I'm happy to see that things are a bit 
better with Mesa 17.2. The game still crashes, but we're closer to 
fixing it.


Here's what I found using 17.2.1:
With frame buffer effects and soft shadows the game crashes at the end 
of loading; the crash is inside a function that amongst other things, 
generates mipmaps for a texture used in a pbuffer (function at offset 
2FB37D in my exe).
The crash happens when gluBuild2DMipmaps is called, however doesn't seem 
to be a null pointer like it was back in march: it's an access violation 
alright but no longer a null pointer. So I think it's a different, 
hopefully simpler, problem.


Back in march, Miklòs Màté suggested that changing the checks for the 
pixel format could fix the problem, and he was right; without those 
checks we definitely got a step closer to fixing it.


My first thought was to just NOP the entire section that generates 
mipmaps and a bit of code later that uses it. The game no longer 
crashes, however it displays nothing, but I can hear it running in 
background. So this is the last issue! We're almost there!


Now, I'm bothering you again because I think that at this point it's 
just a problem with the texture format used there. The call to 
gluBuild2DMipmaps uses LuminanceAlpha' as texture format as well as 
internal format (0x190a). I tried changing it to RGB and RGBA just to 
try something, but that didn't work because I guess the texture was 
already generated with another format.


What could I do to investigate this further? And where should I look 
inside Mesa if I wanted to say... force a specific texture format for 
pbuffers?


I feel that we're very close to fixing this. Your help would mean the 
world to me and the whole KOTOR community.


Thank you ;)

P.S.
This has nothing to do with mesa, but you should know that KOTOR is 
slowly dieing. It is currently unplayable on Intel and AMD graphics, and 
recent nVidia driver updates have introduced a glitch with 
transparencies (it can be fixed, but still, no one can play KOTOR on 
modern hardware properly and we have to keep old computers as dedicated 
"shrines" for KOTOR, that's why I insist so much on Mesa)


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


Re: [Mesa-dev] [PATCH] i965: Convert brw->*_program into a brw->programs[i] array.

2017-09-26 Thread Alejandro Piñeiro


On 26/09/17 09:39, Kenneth Graunke wrote:
> This makes it easier to loop over programs.
> ---
>  src/mesa/drivers/dri/i965/brw_binding_tables.c|  6 +--
>  src/mesa/drivers/dri/i965/brw_context.h   |  7 +--
>  src/mesa/drivers/dri/i965/brw_cs.c|  6 ++-
>  src/mesa/drivers/dri/i965/brw_curbe.c | 12 +++--
>  src/mesa/drivers/dri/i965/brw_gs.c|  6 ++-
>  src/mesa/drivers/dri/i965/brw_gs_surface_state.c  |  7 +--
>  src/mesa/drivers/dri/i965/brw_program.c   | 31 -
>  src/mesa/drivers/dri/i965/brw_sf.c|  2 +-
>  src/mesa/drivers/dri/i965/brw_state_upload.c  | 36 ---
>  src/mesa/drivers/dri/i965/brw_tcs.c   | 11 +++--
>  src/mesa/drivers/dri/i965/brw_tcs_surface_state.c |  7 +--
>  src/mesa/drivers/dri/i965/brw_tes.c   |  9 ++--
>  src/mesa/drivers/dri/i965/brw_tes_surface_state.c |  7 +--
>  src/mesa/drivers/dri/i965/brw_vs.c|  7 +--
>  src/mesa/drivers/dri/i965/brw_vs_surface_state.c  |  7 +--
>  src/mesa/drivers/dri/i965/brw_wm.c|  7 +--
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  | 24 +-
>  src/mesa/drivers/dri/i965/gen6_sol.c  | 11 ++---
>  src/mesa/drivers/dri/i965/gen6_urb.c  |  5 ++-
>  src/mesa/drivers/dri/i965/gen7_cs_state.c |  6 ++-
>  src/mesa/drivers/dri/i965/gen7_urb.c  |  4 +-
>  src/mesa/drivers/dri/i965/genX_state_upload.c | 55 
> +--
>  22 files changed, 147 insertions(+), 126 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_binding_tables.c 
> b/src/mesa/drivers/dri/i965/brw_binding_tables.c
> index 4bbaa5d0594..73f5e56010b 100644
> --- a/src/mesa/drivers/dri/i965/brw_binding_tables.c
> +++ b/src/mesa/drivers/dri/i965/brw_binding_tables.c
> @@ -153,7 +153,7 @@ static void
>  brw_tcs_upload_binding_table(struct brw_context *brw)
>  {
> /* Skip if the tessellation stages are disabled. */
> -   if (brw->tess_eval_program == NULL)
> +   if (brw->programs[MESA_SHADER_TESS_EVAL] == NULL)
>return;
>  
> /* BRW_NEW_TCS_PROG_DATA */
> @@ -182,7 +182,7 @@ static void
>  brw_tes_upload_binding_table(struct brw_context *brw)
>  {
> /* If there's no TES, skip changing anything. */
> -   if (brw->tess_eval_program == NULL)
> +   if (brw->programs[MESA_SHADER_TESS_EVAL] == NULL)
>return;
>  
> /* BRW_NEW_TES_PROG_DATA */
> @@ -210,7 +210,7 @@ static void
>  brw_gs_upload_binding_table(struct brw_context *brw)
>  {
> /* If there's no GS, skip changing anything. */
> -   if (brw->geometry_program == NULL)
> +   if (brw->programs[MESA_SHADER_GEOMETRY] == NULL)
>return;
>  
> /* BRW_NEW_GS_PROG_DATA */
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 6de5c2d0974..bc3d3e398be 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -875,12 +875,7 @@ struct brw_context
>  
> /* Active vertex program:
>  */
> -   const struct gl_program *vertex_program;
> -   const struct gl_program *geometry_program;
> -   const struct gl_program *tess_ctrl_program;
> -   const struct gl_program *tess_eval_program;
> -   const struct gl_program *fragment_program;
> -   const struct gl_program *compute_program;
> +   struct gl_program *programs[MESA_SHADER_STAGES];
>  
> /**
>  * Number of samples in ctx->DrawBuffer, updated by BRW_NEW_NUM_SAMPLES so
> diff --git a/src/mesa/drivers/dri/i965/brw_cs.c 
> b/src/mesa/drivers/dri/i965/brw_cs.c
> index cf72889b411..bc09abd912c 100644
> --- a/src/mesa/drivers/dri/i965/brw_cs.c
> +++ b/src/mesa/drivers/dri/i965/brw_cs.c
> @@ -177,7 +177,8 @@ brw_cs_populate_key(struct brw_context *brw, struct 
> brw_cs_prog_key *key)
>  {
> struct gl_context *ctx = &brw->ctx;
> /* BRW_NEW_COMPUTE_PROGRAM */
> -   const struct brw_program *cp = (struct brw_program *) 
> brw->compute_program;
> +   const struct brw_program *cp =
> +  (struct brw_program *) brw->programs[MESA_SHADER_COMPUTE];
> const struct gl_program *prog = (struct gl_program *) cp;
>  
> memset(key, 0, sizeof(*key));
> @@ -195,7 +196,8 @@ brw_upload_cs_prog(struct brw_context *brw)
>  {
> struct gl_context *ctx = &brw->ctx;
> struct brw_cs_prog_key key;
> -   struct brw_program *cp = (struct brw_program *) brw->compute_program;
> +   struct brw_program *cp =
> +  (struct brw_program *) brw->programs[MESA_SHADER_COMPUTE];
>  
> if (!cp)
>return;
> diff --git a/src/mesa/drivers/dri/i965/brw_curbe.c 
> b/src/mesa/drivers/dri/i965/brw_curbe.c
> index 7adf911dea0..07e3c42b152 100644
> --- a/src/mesa/drivers/dri/i965/brw_curbe.c
> +++ b/src/mesa/drivers/dri/i965/brw_curbe.c
> @@ -204,6 +204,12 @@ brw_upload_constant_buffer(struct brw_context *brw)
> GLuint i;
> gl_clip_plane *clip_planes;
>  
> +   /* BRW_NEW_FRAGMENT_PROGRAM */
> +   struct gl_program *fp = brw->pro

[Mesa-dev] [PATCH 2/2] anv: add an assertion in genX(BeginCommandBuffer)

2017-09-26 Thread Gwan-gyeong Mun
To check a valid usage requirement.

Signed-off-by: Mun Gwan-gyeong 
---
 src/intel/vulkan/genX_cmd_buffer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index fbc1995709..3559399019 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -1006,6 +1006,7 @@ genX(BeginCommandBuffer)(
VkResult result = VK_SUCCESS;
if (cmd_buffer->usage_flags &
VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT) {
+  assert(pBeginInfo->pInheritanceInfo);
   cmd_buffer->state.pass =
  anv_render_pass_from_handle(pBeginInfo->pInheritanceInfo->renderPass);
   cmd_buffer->state.subpass =
-- 
2.14.1

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


[Mesa-dev] [PATCH 1/2] radv: add an assertion in radv_BeginCommandBuffer()

2017-09-26 Thread Gwan-gyeong Mun
To check a valid usage requirement.

CID: 1401616

Signed-off-by: Mun Gwan-gyeong 
---
 src/amd/vulkan/radv_cmd_buffer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 1e0e366820..4db9d7628c 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -2124,6 +2124,7 @@ VkResult radv_BeginCommandBuffer(
}
 
if (pBeginInfo->flags & 
VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT) {
+   assert(pBeginInfo->pInheritanceInfo);
cmd_buffer->state.framebuffer = 
radv_framebuffer_from_handle(pBeginInfo->pInheritanceInfo->framebuffer);
cmd_buffer->state.pass = 
radv_render_pass_from_handle(pBeginInfo->pInheritanceInfo->renderPass);
 
-- 
2.14.1

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


Re: [Mesa-dev] [PATCH] glsl: Trigger precision mismatch error only if both symbols are used

2017-09-26 Thread Tomasz Figa
Hi Ian,

An update on my testing inline.

On Tue, Sep 26, 2017 at 7:13 AM, Tomasz Figa  wrote:
> On Tue, Sep 26, 2017 at 6:21 AM, Tomasz Figa  wrote:
>> Hi Ian,
>>
>> On Tue, Sep 26, 2017 at 5:59 AM, Ian Romanick  wrote:
>>> On 09/25/2017 02:53 AM, Tomasz Figa wrote:
 Commit 259fc505454ea6a67aeacf6cdebf1398d9947759 added linker error for
 mismatching uniform precision, as required by GLES specification and
 conformance test-suite.

 Several Android applications, including Forge of Empires, have shaders
 which violate this rule, on a dead varying that will be eliminated.
 The problem affects a big number of applications using Cocos2D engine
 and other GLES implementations accept this, so work around the problem
 by erroring out only if both symbols are actually referenced in the
 code, which is the only case when the mismatch can cause incorrect
 behavior.
>>>
>>> I think this change will cause failures in the CTS... but, there's a
>>> Khronos bug about this issue, and I think some of the rules are going to
>>> get relaxed.  I'd like to wait until that gets sorted before we start
>>> changing how our linker applies the rules.
>>
>> Are we 100% sure about the failures? There are multiple GLES
>> implementations that claim to be compliant, yet they allow such
>> behavior.
>
> I'm looking through Khronos CTS github repository and it seems like a
> related test 
> (dEQP-GLES3.functional.shaders.linkage.uniform.block.differing_precision)
> was removed from GLES 3 suite and instead few others added to GLES 3.1
> suite:
>
> https://github.com/KhronosGroup/VK-GL-CTS/commit/b8d0d0a842280c2594703ecce985563aebe1cde8#diff-72bd3f02df978e071f2a53ec2654ddd5
>
> Moreover I can see a test being included in GLES 3.2 suite
> (KHR-GLES32.shaders.negative.unused_uniform_precision_matching) that
> prohibits precision mismatch even for unused uniforms, but it is not
> included in any suites for earlier GLES versions.

Just ran the whole suite for GLES 3.0 conformance run with i965 driver
on Android and there is only 1 failure that seems to be unrelated to
the workaround (fails both with and without the change):

KHR-GLES3.shaders.uniform_block.common.precision_matching

However, I just realized that the applications affected are all using
GLSL ES 1.00. So, if we turn the condition introduced by my patch into
"(existing->data.used && var->data.used) || prog->data->Version >=
300", we should be safe both on compatibility and conformance side.

>>
>> Note that this is affecting a huge number of Android applications,
>> including a commonly used 2D game engine and we're having a really big
>> problem right now, because even if the engine gets fixed, it's very
>> unlikely that all the developers update their engine.
>>
>>>
 Based on Kenneth Graunke's patch from Bugzilla, reworked from a drirc
 option that completely bypasses the check into an incoditional check
>>>
>>> unconditional
>>>
 that triggers either an error or warning, respectively if both
 declarations are further referenced by the code or not.

 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97532
 Signed-off-by: Tomasz Figa 
 ---
  src/compiler/glsl/linker.cpp | 14 ++
  1 file changed, 10 insertions(+), 4 deletions(-)

 diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
 index f352c5385ca..1c534ea1a3b 100644
 --- a/src/compiler/glsl/linker.cpp
 +++ b/src/compiler/glsl/linker.cpp
 @@ -1121,10 +1121,16 @@ cross_validate_globals(struct gl_shader_program 
 *prog,
   if (prog->IsES && (prog->data->Version != 310 ||

>>>
>>> Does just removing the "prog->data->Version != 310" clause also fix the
>>> problem?
>>
>> Will check later today, but I suspect it would, since the applications
>> are not GLES 3.1.

I just tested it now and it doesn't fix the problem. However, I
misunderstood the code before. The shaders affected are not using
uniform blocks, so "!var->get_interface_type()" hits and leads to the
error.

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


[Mesa-dev] [PATCH] glsl: Allow precision mismatch on dead data with GLSL ES 1.00

2017-09-26 Thread Tomasz Figa
Commit 259fc505454ea6a67aeacf6cdebf1398d9947759 added linker error for
mismatching uniform precision, as required by GLES 3.0 specification and
conformance test-suite.

Several Android applications, including Forge of Empires, have shaders
which violate this rule, on a dead varying that will be eliminated.
The problem affects a big number of applications using Cocos2D engine
and other GLES implementations accept this, this poses a serious
application compatibility issue.

Starting from GLSL ES 3.0, declarations with conflicting precision
qualifiers are explicitly prohibited. However GLSL ES 1.00 does not
clearly specify the behavior, except that

  "Uniforms are defined to behave as if they are using the same storage in
  the vertex and fragment processors and may be implemented this way.
  If uniforms are used in both the vertex and fragment shaders, developers
  should be warned if the precisions are different. Conversion of
  precision should never be implicit."

The word "used" is not clear in this context and might refer to
 1) declared (same as GLES 3.x)
 2) referred after post-processing, or
 3) linked after all optimizations are done.

Looking at existing applications, 2) or 3) seems to be widely adopted.
To avoid compatibility issues, turn the error into a warning if GLSL ES
version is lower than 3.0 and the data is dead in at least one of the
shaders.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97532
Signed-off-by: Tomasz Figa 
---
 src/compiler/glsl/linker.cpp | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index f352c5385ca..ff2e6b7a0d3 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1121,10 +1121,16 @@ cross_validate_globals(struct gl_shader_program *prog,
  if (prog->IsES && (prog->data->Version != 310 ||
 !var->get_interface_type()) &&
  existing->data.precision != var->data.precision) {
-linker_error(prog, "declarations for %s `%s` have "
- "mismatching precision qualifiers\n",
- mode_string(var), var->name);
-return;
+if ((existing->data.used && var->data.used) || prog->data->Version 
>= 300) {
+   linker_error(prog, "declarations for %s `%s` have "
+"mismatching precision qualifiers\n",
+mode_string(var), var->name);
+   return;
+} else {
+   linker_warning(prog, "declarations for %s `%s` have "
+  "mismatching precision qualifiers\n",
+  mode_string(var), var->name);
+}
  }
   } else
  variables->add_variable(var);
-- 
2.14.1.821.g8fa685d3b7-goog

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


Re: [Mesa-dev] [PATCH 1/2] radv: add an assertion in radv_BeginCommandBuffer()

2017-09-26 Thread Samuel Pitoiset

Reviewed-by: Samuel Pitoiset 

On 09/26/2017 10:14 AM, Gwan-gyeong Mun wrote:

To check a valid usage requirement.

CID: 1401616

Signed-off-by: Mun Gwan-gyeong 
---
  src/amd/vulkan/radv_cmd_buffer.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 1e0e366820..4db9d7628c 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -2124,6 +2124,7 @@ VkResult radv_BeginCommandBuffer(
}
  
  	if (pBeginInfo->flags & VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT) {

+   assert(pBeginInfo->pInheritanceInfo);
cmd_buffer->state.framebuffer = 
radv_framebuffer_from_handle(pBeginInfo->pInheritanceInfo->framebuffer);
cmd_buffer->state.pass = 
radv_render_pass_from_handle(pBeginInfo->pInheritanceInfo->renderPass);
  


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


Re: [Mesa-dev] [PATCH v2 7/7] i965: make use of nir linking

2017-09-26 Thread Timothy Arceri



On 26/09/17 17:50, Kenneth Graunke wrote:

On Monday, September 25, 2017 6:23:13 PM PDT Timothy Arceri wrote:

For now linking is just removing unused varyings between stages.

shader-db results BDW:

total instructions in shared programs: 13198288 -> 13191693 (-0.05%)
instructions in affected programs: 48325 -> 41730 (-13.65%)
helped: 473
HURT: 0

total cycles in shared programs: 541184926 -> 541159260 (-0.00%)
cycles in affected programs: 213238 -> 187572 (-12.04%)
helped: 435
HURT: 8

V2:
- lower indirects on demoted inputs as well as outputs.
---
  src/mesa/drivers/dri/i965/brw_link.cpp | 56 ++
  1 file changed, 56 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp 
b/src/mesa/drivers/dri/i965/brw_link.cpp
index b7fab8d7a25..9ddf0230183 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -253,6 +253,62 @@ brw_link_shader(struct gl_context *ctx, struct 
gl_shader_program *shProg)
   compiler->scalar_stage[stage]);
 }
  
+   /* Determine first and last stage. */

+   unsigned first = MESA_SHADER_STAGES;
+   unsigned last = 0;
+   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
+  if (!shProg->_LinkedShaders[i])
+ continue;
+  if (first == MESA_SHADER_STAGES)
+ first = i;
+  last = i;
+   }
+
+   /* Linking the stages in the opposite order (from fragment to vertex)
+* ensures that inter-shader outputs written to in an earlier stage
+* are eliminated if they are (transitively) not used in a later
+* stage.
+*/
+if (first != last) {
+   int next = last;
+   for (int i = next - 1; i >= 0; i--) {
+  if (shProg->_LinkedShaders[i] == NULL)
+ continue;
+
+nir_shader *producer = shProg->_LinkedShaders[i]->Program->nir;
+nir_shader *consumer = shProg->_LinkedShaders[next]->Program->nir;
+
+nir_remove_dead_variables(producer, nir_var_shader_out);
+nir_remove_dead_variables(consumer, nir_var_shader_in);
+
+if (nir_remove_unused_varyings(producer, consumer)) {
+   nir_lower_global_vars_to_local(producer);
+   nir_lower_global_vars_to_local(consumer);
+
+   nir_variable_mode indirect_mask = (nir_variable_mode) 0;
+   if (compiler->glsl_compiler_options[i].EmitNoIndirectTemp)
+  indirect_mask = (nir_variable_mode) nir_var_local;
+
+   /* The backend might not be able to handle indirects on
+* temporaries so we need to lower indirects on any of the
+* varyings we have demoted here.
+*/
+   nir_lower_indirect_derefs(producer, indirect_mask);
+   nir_lower_indirect_derefs(consumer, indirect_mask);
+
+   const bool p_is_scalar = 
compiler->scalar_stage[producer->stage];
+   shProg->_LinkedShaders[i]->Program->nir =
+ brw_nir_optimize(producer, compiler, p_is_scalar);
+
+   const bool c_is_scalar = 
compiler->scalar_stage[producer->stage];
+   shProg->_LinkedShaders[next]->Program->nir =
+ brw_nir_optimize(consumer, compiler, c_is_scalar);
+}
+
+next = i;
+   }
+}
+
 for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders); stage++) {
struct gl_linked_shader *shader = shProg->_LinkedShaders[stage];
if (!shader)



A couple more thoughts:

1. We're calling brw_nir_optimize even more now...it might make sense to
drop it from brw_preprocess_nir, drop it here, and put it once in the
final (third) loop, just before brw_shader_gather_info.  At least,
it'd be interesting how that affects compile times / quality...


Yeah I can have a play around. As is it didn't really change shader-db 
compile times on my BDW so I wasn't too concerned.




2. It would be great to use this in anv as well, especially given that
it has no GLSL IR linking to do this sort of stuff.  Plus, the SPIR-V
might be compiled independently...and when you build the pipeline,
you have all the stages and can do cross-stage linking optimizations
like this.


Yeah the reason for doing this was to eventually add it to radv. i965 
was just a nicer place to test it, same goes for the passes that will 
follow. Right now the biggest blocked is justifying the added complexity 
as there isn't a standard shader-db like tool, and it's not making any 
noticeable differences in any benchmarks.


As far as anv I won't be doing any work on that directly, only radv.



Series looks great though, and is:
Reviewed-by: Kenneth Graunke 

Thanks Tim!


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


Re: [Mesa-dev] [PATCH] egl/dri: link directly to libglapi.so

2017-09-26 Thread Eric Engestrom
On 25 September 2017 12:01:33 BST, Tomasz Figa  wrote:
> On Mon, Sep 25, 2017 at 7:21 PM, Eric Engestrom
>  wrote:
> > On Saturday, 2017-09-23 13:37:29 +0900, Tomasz Figa wrote:
> >> On Wed, Sep 20, 2017 at 7:47 PM, Eric Engestrom
> >>  wrote:
> >> > On Tuesday, 2017-09-19 17:19:59 +, Emil Velikov wrote:
> >> >> From: Emil Velikov 
> >> >>
> >> >> In order to build EGL, one has to use shared glapi -
> libglapi.so.
> >> >>
> >> >> Thus the dlopen/dlsym dance is no longer needed and we can link
> to the
> >> >> library directly.
> >> >>
> >> >> This allows us to remove a handful of platform specific names of
> the
> >> >> library.
> >> >>
> >> >> Cc: Jonathan Gray 
> >> >> Cc: Jon Turney 
> >> >> Cc: Julien Isorce 
> >> >> Cc: Rob Herring 
> >> >> Cc: Tomasz Figa 
> >> >> Signed-off-by: Emil Velikov 
> >> >
> >> > Nice cleanup!
> >> > Assuming the build systems stuff works (with Rob's suggested
> change?):
> >> > Reviewed-by: Eric Engestrom 
> >> >
> >> > struct dri2_egl_driver now only contains the glFlush() pointer
> and
> >> > _EGLDriver.  Could we move the _glapi_get_proc_address() call to
> >> > the two places that use glFlush() (ie. dri2_make_current() and
> >> > dri2_client_wait_sync()), and get rid of this struct, as well as
> >> > the whole dri2_load() function?
> >> >
> >> > I'm happy to do this, just want to check that it would be ok :)
> >>
> >> Wouldn't that mean going through the series of strcmp()s over all
> the
> >> symbols every time EGL wants to call glFlush?
> >
> > Well, I was thinking of making that call static, maybe even global
> > (shared between the functions), so it would happen at most once for
> each
> > of the two functions, but yeah you're right, it would do it more
> often
> > than right now. Do you think this is an issue?
> > I guess I'm dropping this idea if that's the case :]
> 
> I guess a global function (let's say dri2_gl_flush()) that looks up
> the symbol only at the first call and then calls it, wouldn't be so
> bad.

Gave this a stab last night, it would look like this:
https://github.com/mesa3d/mesa/compare/master...1ace:wip/egl-drv-flatten

(This doesn't compile right now; I'll rebase it when Emil's patch lands,
but you can already see what I was thinking about.)

> 
> Best regards,
> Tomasz
> 
> >
> >>
> >> Best regards,
> >> Tomasz
> >>
> ___
> 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] egl/dri: link directly to libglapi.so

2017-09-26 Thread Tomasz Figa
On Tue, Sep 26, 2017 at 6:44 PM, Eric Engestrom  wrote:
> On 25 September 2017 12:01:33 BST, Tomasz Figa  wrote:
>> On Mon, Sep 25, 2017 at 7:21 PM, Eric Engestrom
>>  wrote:
>> > On Saturday, 2017-09-23 13:37:29 +0900, Tomasz Figa wrote:
>> >> On Wed, Sep 20, 2017 at 7:47 PM, Eric Engestrom
>> >>  wrote:
>> >> > On Tuesday, 2017-09-19 17:19:59 +, Emil Velikov wrote:
>> >> >> From: Emil Velikov 
>> >> >>
>> >> >> In order to build EGL, one has to use shared glapi -
>> libglapi.so.
>> >> >>
>> >> >> Thus the dlopen/dlsym dance is no longer needed and we can link
>> to the
>> >> >> library directly.
>> >> >>
>> >> >> This allows us to remove a handful of platform specific names of
>> the
>> >> >> library.
>> >> >>
>> >> >> Cc: Jonathan Gray 
>> >> >> Cc: Jon Turney 
>> >> >> Cc: Julien Isorce 
>> >> >> Cc: Rob Herring 
>> >> >> Cc: Tomasz Figa 
>> >> >> Signed-off-by: Emil Velikov 
>> >> >
>> >> > Nice cleanup!
>> >> > Assuming the build systems stuff works (with Rob's suggested
>> change?):
>> >> > Reviewed-by: Eric Engestrom 
>> >> >
>> >> > struct dri2_egl_driver now only contains the glFlush() pointer
>> and
>> >> > _EGLDriver.  Could we move the _glapi_get_proc_address() call to
>> >> > the two places that use glFlush() (ie. dri2_make_current() and
>> >> > dri2_client_wait_sync()), and get rid of this struct, as well as
>> >> > the whole dri2_load() function?
>> >> >
>> >> > I'm happy to do this, just want to check that it would be ok :)
>> >>
>> >> Wouldn't that mean going through the series of strcmp()s over all
>> the
>> >> symbols every time EGL wants to call glFlush?
>> >
>> > Well, I was thinking of making that call static, maybe even global
>> > (shared between the functions), so it would happen at most once for
>> each
>> > of the two functions, but yeah you're right, it would do it more
>> often
>> > than right now. Do you think this is an issue?
>> > I guess I'm dropping this idea if that's the case :]
>>
>> I guess a global function (let's say dri2_gl_flush()) that looks up
>> the symbol only at the first call and then calls it, wouldn't be so
>> bad.
>
> Gave this a stab last night, it would look like this:
> https://github.com/mesa3d/mesa/compare/master...1ace:wip/egl-drv-flatten
>
> (This doesn't compile right now; I'll rebase it when Emil's patch lands,
> but you can already see what I was thinking about.)

Looks quite good, thanks. I like the diffstat. ;)

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


Re: [Mesa-dev] [PATCH v2 7/7] i965: make use of nir linking

2017-09-26 Thread Timothy Arceri



On 26/09/17 17:50, Kenneth Graunke wrote:

On Monday, September 25, 2017 6:23:13 PM PDT Timothy Arceri wrote:

For now linking is just removing unused varyings between stages.

shader-db results BDW:

total instructions in shared programs: 13198288 -> 13191693 (-0.05%)
instructions in affected programs: 48325 -> 41730 (-13.65%)
helped: 473
HURT: 0

total cycles in shared programs: 541184926 -> 541159260 (-0.00%)
cycles in affected programs: 213238 -> 187572 (-12.04%)
helped: 435
HURT: 8

V2:
- lower indirects on demoted inputs as well as outputs.
---
  src/mesa/drivers/dri/i965/brw_link.cpp | 56 ++
  1 file changed, 56 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp 
b/src/mesa/drivers/dri/i965/brw_link.cpp
index b7fab8d7a25..9ddf0230183 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -253,6 +253,62 @@ brw_link_shader(struct gl_context *ctx, struct 
gl_shader_program *shProg)
   compiler->scalar_stage[stage]);
 }
  
+   /* Determine first and last stage. */

+   unsigned first = MESA_SHADER_STAGES;
+   unsigned last = 0;
+   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
+  if (!shProg->_LinkedShaders[i])
+ continue;
+  if (first == MESA_SHADER_STAGES)
+ first = i;
+  last = i;
+   }
+
+   /* Linking the stages in the opposite order (from fragment to vertex)
+* ensures that inter-shader outputs written to in an earlier stage
+* are eliminated if they are (transitively) not used in a later
+* stage.
+*/
+if (first != last) {
+   int next = last;
+   for (int i = next - 1; i >= 0; i--) {
+  if (shProg->_LinkedShaders[i] == NULL)
+ continue;
+
+nir_shader *producer = shProg->_LinkedShaders[i]->Program->nir;
+nir_shader *consumer = shProg->_LinkedShaders[next]->Program->nir;
+
+nir_remove_dead_variables(producer, nir_var_shader_out);
+nir_remove_dead_variables(consumer, nir_var_shader_in);
+
+if (nir_remove_unused_varyings(producer, consumer)) {
+   nir_lower_global_vars_to_local(producer);
+   nir_lower_global_vars_to_local(consumer);
+
+   nir_variable_mode indirect_mask = (nir_variable_mode) 0;
+   if (compiler->glsl_compiler_options[i].EmitNoIndirectTemp)
+  indirect_mask = (nir_variable_mode) nir_var_local;
+
+   /* The backend might not be able to handle indirects on
+* temporaries so we need to lower indirects on any of the
+* varyings we have demoted here.
+*/
+   nir_lower_indirect_derefs(producer, indirect_mask);
+   nir_lower_indirect_derefs(consumer, indirect_mask);
+
+   const bool p_is_scalar = 
compiler->scalar_stage[producer->stage];
+   shProg->_LinkedShaders[i]->Program->nir =
+ brw_nir_optimize(producer, compiler, p_is_scalar);
+
+   const bool c_is_scalar = 
compiler->scalar_stage[producer->stage];
+   shProg->_LinkedShaders[next]->Program->nir =
+ brw_nir_optimize(consumer, compiler, c_is_scalar);
+}
+
+next = i;
+   }
+}
+
 for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders); stage++) {
struct gl_linked_shader *shader = shProg->_LinkedShaders[stage];
if (!shader)



A couple more thoughts:

1. We're calling brw_nir_optimize even more now...it might make sense to
drop it from brw_preprocess_nir, drop it here, and put it once in the
final (third) loop, just before brw_shader_gather_info.  At least,
it'd be interesting how that affects compile times / quality...


Actually we can't drop the producer brw_nir_optimize call here anyway as 
we want to remove dead code before the next iteration of the loop.




2. It would be great to use this in anv as well, especially given that
it has no GLSL IR linking to do this sort of stuff.  Plus, the SPIR-V
might be compiled independently...and when you build the pipeline,
you have all the stages and can do cross-stage linking optimizations
like this.

Series looks great though, and is:
Reviewed-by: Kenneth Graunke 

Thanks Tim!


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


[Mesa-dev] [PATCH] etnaviv: Set up GC3000 states, fix point sprite rendering

2017-09-26 Thread Wladimir J. van der Laan
Set up new states that the blob started setting for GC3000 consistently.

This makes sure that when another test or driver leaves the GPU in
unpredictable state, these states are set up correctly for our
rendering.

Also, setting PA_VIEWPORT_UNK00A84 to fui(8192.0) is necessary
to make point sprite rendering on GC3000 work.

Signed-off-by: Wladimir J. van der Laan 
---
 src/gallium/drivers/etnaviv/etnaviv_context.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.c 
b/src/gallium/drivers/etnaviv/etnaviv_context.c
index 2ca09ce..67aab6a 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_context.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_context.c
@@ -317,6 +317,19 @@ etna_cmd_stream_reset_notify(struct etna_cmd_stream 
*stream, void *priv)
etna_set_state(stream, VIVS_GL_VERTEX_ELEMENT_CONFIG, 0x0001);
etna_set_state(stream, VIVS_RA_EARLY_DEPTH, 0x0031);
etna_set_state(stream, VIVS_PA_W_CLIP_LIMIT, 0x3401);
+   etna_set_state(stream, VIVS_PA_FLAGS, 0x); /* blob sets 
ZCONVERT_BYPASS on GC3000, this messes up z for us */
+   etna_set_state(stream, VIVS_RA_UNK00E0C, 0x);
+   etna_set_state(stream, VIVS_PA_VIEWPORT_UNK00A80, 0x38a01404);
+   etna_set_state(stream, VIVS_PA_VIEWPORT_UNK00A84, fui(8192.0));
+   etna_set_state(stream, VIVS_PA_ZFARCLIPPING, 0x);
+   etna_set_state(stream, VIVS_PE_ALPHA_COLOR_EXT0, 0x);
+   etna_set_state(stream, VIVS_PE_ALPHA_COLOR_EXT1, 0x);
+   etna_set_state(stream, VIVS_RA_HDEPTH_CONTROL, 0x7000);
+   etna_set_state(stream, VIVS_PE_STENCIL_CONFIG_EXT2, 0x);
+   etna_set_state(stream, VIVS_GL_UNK03834, 0x);
+   etna_set_state(stream, VIVS_GL_UNK03838, 0x);
+   etna_set_state(stream, VIVS_GL_UNK03854, 0x);
+   etna_set_state(stream, VIVS_PS_CONTROL_EXT, 0x);
 
/* Enable SINGLE_BUFFER for resolve, if supported */
etna_set_state(stream, VIVS_RS_SINGLE_BUFFER, 
COND(ctx->specs.single_buffer, VIVS_RS_SINGLE_BUFFER_ENABLE));
-- 
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] nv50/ir: skip optimizing ADD+SHL to SHLADD when src(1) is 0

2017-09-26 Thread Juan A. Suarez Romero
On Mon, 2017-05-01 at 12:11 -0400, Ilia Mirkin wrote:
> On Mon, May 1, 2017 at 12:09 PM, Samuel Pitoiset
>  wrote:
> > On 05/01/2017 05:59 PM, Ilia Mirkin wrote:
> > > 
> > > I think this is off. It shouldn't matter what the code sequence is,
> > > it's all representable. You need to teach replaceZero to not mess
> > > things up for SHLADD's src(1).
> > 
> > 
> > It's representable but stupid to do it. We should keep the ADD there and
> > this also avoids a workaround in the replaceZero logic just for that.
> > 
> > IMHO, this is the better solution.
> 
> Yes, that would be a better optimization. However at this stage, you
> have a y = SHL(x, 0) + ADD(y, z). I think it makes sense to combine
> them into one and let DCE take care of it.
> 
> Ideally we wouldn't get into such a situation, but the real issue is
> that a perfectly representable (if dumb) instruction like SHLADD(x, 0,
> y) gets messed up by replaceZero. We should fix replaceZero. We should
> also try to avoid generating such stupid instructions.
> 

So what was the final conclusion for this patch? Was replaceZero fixed?

J.A.

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nv50/ir: skip optimizing ADD+SHL to SHLADD when src(1) is 0

2017-09-26 Thread Samuel Pitoiset



On 09/26/2017 12:29 PM, Juan A. Suarez Romero wrote:

On Mon, 2017-05-01 at 12:11 -0400, Ilia Mirkin wrote:

On Mon, May 1, 2017 at 12:09 PM, Samuel Pitoiset
 wrote:

On 05/01/2017 05:59 PM, Ilia Mirkin wrote:


I think this is off. It shouldn't matter what the code sequence is,
it's all representable. You need to teach replaceZero to not mess
things up for SHLADD's src(1).



It's representable but stupid to do it. We should keep the ADD there and
this also avoids a workaround in the replaceZero logic just for that.

IMHO, this is the better solution.


Yes, that would be a better optimization. However at this stage, you
have a y = SHL(x, 0) + ADD(y, z). I think it makes sense to combine
them into one and let DCE take care of it.

Ideally we wouldn't get into such a situation, but the real issue is
that a perfectly representable (if dumb) instruction like SHLADD(x, 0,
y) gets messed up by replaceZero. We should fix replaceZero. We should
also try to avoid generating such stupid instructions.



So what was the final conclusion for this patch? Was replaceZero fixed?


I don't think it has been fixed, but I could be wrong.



J.A.


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


Re: [Mesa-dev] [Mesa-stable] [PATCH 1/3] anv: Stop racing relocation offsets

2017-09-26 Thread Juan A. Suarez Romero
On Mon, 2017-06-26 at 22:39 +0300, Andres Gomez wrote:
> Jason, it doesn't seem like this patch has landed in master. Are you in
> need of review or is it that this has been superseded?
> 


Gently ping to know what is the status for this patch.

Thaks in advance.


J.A.

> Thanks!
> 
> On Wed, 2017-05-10 at 16:08 -0700, Jason Ekstrand wrote:
> > One of the key invariants of the relocation system is the
> > presumed_offset field.  The assumption is made that the value currently
> > in the address to be relocated agrees with the presumed_offset field.
> > If presumed_offset is equal to the offset of the BO, the kernel will
> > skip the relocation assuming that the value is already correct.
> > 
> > Our initial implementation of relocation handling had a race where we
> > would read bo->offset once when we wrote the relocation entry and again
> > when we filled out actual address.
> > 
> > Found with helgrind
> > 
> > Cc: "17.0 17.1" 
> > ---
> >  src/intel/vulkan/anv_batch_chain.c | 21 +
> >  src/intel/vulkan/anv_private.h |  2 +-
> >  src/intel/vulkan/genX_blorp_exec.c |  5 -
> >  src/intel/vulkan/genX_cmd_buffer.c |  7 +--
> >  4 files changed, 27 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/intel/vulkan/anv_batch_chain.c 
> > b/src/intel/vulkan/anv_batch_chain.c
> > index 9def174..13303b1 100644
> > --- a/src/intel/vulkan/anv_batch_chain.c
> > +++ b/src/intel/vulkan/anv_batch_chain.c
> > @@ -143,7 +143,8 @@ anv_reloc_list_grow(struct anv_reloc_list *list,
> >  VkResult
> >  anv_reloc_list_add(struct anv_reloc_list *list,
> > const VkAllocationCallbacks *alloc,
> > -   uint32_t offset, struct anv_bo *target_bo, uint32_t 
> > delta)
> > +   uint32_t offset, struct anv_bo *target_bo, uint32_t 
> > delta,
> > +   uint64_t *bo_offset_out)
> >  {
> > struct drm_i915_gem_relocation_entry *entry;
> > int index;
> > @@ -155,6 +156,14 @@ anv_reloc_list_add(struct anv_reloc_list *list,
> > if (result != VK_SUCCESS)
> >return result;
> >  
> > +   /* Read the BO offset once.  This same value will be used in the 
> > relocation
> > +* entry and passed back to the caller for it to use when it writes the
> > +* actual value.  This guarantees that the two values match even if 
> > there
> > +* is a data race between now and when the caller gets around to writing
> > +* the address into the BO.
> > +*/
> > +   uint64_t presumed_offset = target_bo->offset;
> > +
> > /* XXX: Can we use I915_EXEC_HANDLE_LUT? */
> > index = list->num_relocs++;
> > list->reloc_bos[index] = target_bo;
> > @@ -162,11 +171,13 @@ anv_reloc_list_add(struct anv_reloc_list *list,
> > entry->target_handle = target_bo->gem_handle;
> > entry->delta = delta;
> > entry->offset = offset;
> > -   entry->presumed_offset = target_bo->offset;
> > +   entry->presumed_offset = presumed_offset;
> > entry->read_domains = domain;
> > entry->write_domain = domain;
> > VG(VALGRIND_CHECK_MEM_IS_DEFINED(entry, sizeof(*entry)));
> >  
> > +   *bo_offset_out = presumed_offset;
> > +
> > return VK_SUCCESS;
> >  }
> >  
> > @@ -218,14 +229,16 @@ uint64_t
> >  anv_batch_emit_reloc(struct anv_batch *batch,
> >   void *location, struct anv_bo *bo, uint32_t delta)
> >  {
> > +   uint64_t bo_offset;
> > VkResult result = anv_reloc_list_add(batch->relocs, batch->alloc,
> > -location - batch->start, bo, 
> > delta);
> > +location - batch->start, bo, delta,
> > +&bo_offset);
> > if (result != VK_SUCCESS) {
> >anv_batch_set_error(batch, result);
> >return 0;
> > }
> >  
> > -   return bo->offset + delta;
> > +   return bo_offset + delta;
> >  }
> >  
> >  void
> > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> > index 9b0dd67..1686da8 100644
> > --- a/src/intel/vulkan/anv_private.h
> > +++ b/src/intel/vulkan/anv_private.h
> > @@ -825,7 +825,7 @@ void anv_reloc_list_finish(struct anv_reloc_list *list,
> >  VkResult anv_reloc_list_add(struct anv_reloc_list *list,
> >  const VkAllocationCallbacks *alloc,
> >  uint32_t offset, struct anv_bo *target_bo,
> > -uint32_t delta);
> > +uint32_t delta, uint64_t *bo_offset_out);
> >  
> >  struct anv_batch_bo {
> > /* Link in the anv_cmd_buffer.owned_batch_bos list */
> > diff --git a/src/intel/vulkan/genX_blorp_exec.c 
> > b/src/intel/vulkan/genX_blorp_exec.c
> > index 71ed707..513c269 100644
> > --- a/src/intel/vulkan/genX_blorp_exec.c
> > +++ b/src/intel/vulkan/genX_blorp_exec.c
> > @@ -57,9 +57,12 @@ blorp_surface_reloc(struct blorp_batch *batch, uint32_t 
> > ss_offset,
> >  struct blorp_address address, uint32_t 

Re: [Mesa-dev] [PATCH] i965: skip reading clip distances from the URB for the FS if possible

2017-09-26 Thread Ilia Mirkin
Perhaps a debug message would be warranted in such a situation? I suspect
it would be difficult to debug, esp if it came up in a regular application.

On Sep 26, 2017 3:50 AM, "Iago Toral Quiroga"  wrote:

we can skip these slots when they are not read in the fragment shader
and they are positioned right after a VUE header that we are already
skipping. We also need to ensure that we are passing at least one other
varying, since that is a hardware requirement.

This helps alleviate a problem introduced with 99df02ca26f61 for
separate shader objects: without separate shader objects we assign
locations sequentially, however, since that commit we have changed the
method for SSO so that the VUE slot assigned depends on the number of
builtin slots plus the location assigned to the varying. This fixed
layout is intended to help SSO programs by avoiding on-the-fly recompiles
when swapping out shaders, however, it also means that if a varying uses
a large location number close to the maximum allowed by the SF/FS units
(31), then the offset introduced by the number of builtin slots can push
the location outside the range and trigger an assertion.

This problem is affecting at least the following CTS tests for
enhanced layouts:

KHR-GL45.enhanced_layouts.varying_array_components
KHR-GL45.enhanced_layouts.varying_array_locations
KHR-GL45.enhanced_layouts.varying_components
KHR-GL45.enhanced_layouts.varying_locations

which use SSO and the the location layout qualifier to select such
location numbers explicitly.

This change helps these tests because for SSO we always have to include
VARYING_SLOT_CLIP_DIST{0,1} even if the fragment shader is very unlikely
to read them, so by doing this we free builtin slots from the fixed VUE
layout and we avoid the tests to crash in this scenario.

Of course, this is not a proper fix, we'd still run into problems if someone
tries to use an explicit max location and read gl_ViewportIndex, gl_LayerID
or
gl_CullDistancein in the FS, but that would be a much less common bug and we
can probably wait to see if anyone actually runs into that situation in a
real
world scenario before making the decision that more aggresive changes are
required to support this without reverting 99df02ca26f61.

Suggested-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/genX_state_upload.c | 25
+
 1 file changed, 25 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
b/src/mesa/drivers/dri/i965/genX_state_upload.c
index 612761601a..ce3515067f 100644
--- a/src/mesa/drivers/dri/i965/genX_state_upload.c
+++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
@@ -1040,6 +1040,31 @@ genX(calculate_attr_overrides)(const struct
brw_context *brw,

*urb_entry_read_offset = fs_needs_vue_header ? 0 : 1;

+   /* If we are already skipping the first 2 slots for the VUE header then
we
+* can also skip clip distances if they are located right after the
header
+* and the FS doesn't read them. Only do this is there are any user
varyings
+* though, since the hardware requires that we pass at least one varying
+* between stages.
+*/
+   uint64_t var_bits = ~(VARYING_BIT_VAR(0) - 1);
+   uint64_t clip_dist_slots = VARYING_BIT_CLIP_DIST0 |
VARYING_BIT_CLIP_DIST1;
+   if (!fs_needs_vue_header &&
+   var_bits &&
+   (brw->fragment_program->info.inputs_read & clip_dist_slots) == 0 &&
+   (brw->vue_map_geom_out.slots_valid & clip_dist_slots)) {
+
+  uint32_t clip_dist0_slot =
+ brw->vue_map_geom_out.varying_to_slot[VARYING_SLOT_CLIP_DIST0];
+
+  uint32_t clip_dist1_slot =
+ brw->vue_map_geom_out.varying_to_slot[VARYING_SLOT_CLIP_DIST1];
+
+  if (clip_dist0_slot >= 2 && clip_dist0_slot <= 3 &&
+  clip_dist1_slot >= 2 && clip_dist1_slot <= 3) {
+ *urb_entry_read_offset = 2;
+  }
+   }
+
/* From the Ivybridge PRM, Vol 2 Part 1, 3DSTATE_SBE,
 * description of dw10 Point Sprite Texture Coordinate Enable:
 *
--
2.11.0

___
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] [Mesa-stable] [PATCH] nv50/ir: skip optimizing ADD+SHL to SHLADD when src(1) is 0

2017-09-26 Thread Ilia Mirkin
On Tue, Sep 26, 2017 at 6:30 AM, Samuel Pitoiset
 wrote:
>
>
> On 09/26/2017 12:29 PM, Juan A. Suarez Romero wrote:
>>
>> On Mon, 2017-05-01 at 12:11 -0400, Ilia Mirkin wrote:
>>>
>>> On Mon, May 1, 2017 at 12:09 PM, Samuel Pitoiset
>>>  wrote:

 On 05/01/2017 05:59 PM, Ilia Mirkin wrote:
>
>
> I think this is off. It shouldn't matter what the code sequence is,
> it's all representable. You need to teach replaceZero to not mess
> things up for SHLADD's src(1).



 It's representable but stupid to do it. We should keep the ADD there and
 this also avoids a workaround in the replaceZero logic just for that.

 IMHO, this is the better solution.
>>>
>>>
>>> Yes, that would be a better optimization. However at this stage, you
>>> have a y = SHL(x, 0) + ADD(y, z). I think it makes sense to combine
>>> them into one and let DCE take care of it.
>>>
>>> Ideally we wouldn't get into such a situation, but the real issue is
>>> that a perfectly representable (if dumb) instruction like SHLADD(x, 0,
>>> y) gets messed up by replaceZero. We should fix replaceZero. We should
>>> also try to avoid generating such stupid instructions.
>>>
>>
>> So what was the final conclusion for this patch? Was replaceZero fixed?
>
>
> I don't think it has been fixed, but I could be wrong.

https://cgit.freedesktop.org/mesa/mesa/commit/src/gallium/drivers/nouveau/codegen?id=82e77d4e4484b5d4f6a7b4751a17c882e6d2ad69
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/5] util: Add tests for the string buffer

2017-09-26 Thread Nicolai Hähnle

On 21.09.2017 21:13, Thomas Helland wrote:

Fixed the missing newline at the end of this cpp file locally.
This is the only patch left in the series without an RB.
If there's no objections I plan on pushing this once I get an RB on this.
Someone mind having a look at it?


Sorry, I forgot to send out the R-b. So:

Reviewed-by: Nicolai Hähnle 




2017-09-11 22:21 GMT+02:00 Thomas Helland :

More tests could probably be added, but this should cover
concatenation, resizing, clearing, formatted printing,
and checking the length, so it should be quite complete.

V2: Address review feedback from Timothy, plus fixes
- Use a large enough char array
- Actually test the formatted appending
- Test that clear function resets string length

V3: Port to gtest

V4: Fix test makefile
 Fix copyright header
 Fix missing extern C
 Use more appropriate name for C-file
 Add tests for append_char
---
  configure.ac   |   1 +
  src/util/Makefile.am   |   5 +-
  src/util/tests/string_buffer/Makefile.am   |  40 +++
  .../tests/string_buffer/string_buffer_test.cpp | 119 +
  4 files changed, 164 insertions(+), 1 deletion(-)
  create mode 100644 src/util/tests/string_buffer/Makefile.am
  create mode 100644 src/util/tests/string_buffer/string_buffer_test.cpp

diff --git a/configure.ac b/configure.ac
index d0d4c0dfd1..20727c7bb4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2924,6 +2924,7 @@ AC_CONFIG_FILES([Makefile
   src/mesa/state_tracker/tests/Makefile
   src/util/Makefile
   src/util/tests/hash_table/Makefile
+ src/util/tests/string_buffer/Makefile
   src/util/xmlpool/Makefile
   src/vulkan/Makefile])

diff --git a/src/util/Makefile.am b/src/util/Makefile.am
index 4512dc99d5..2b47143ad7 100644
--- a/src/util/Makefile.am
+++ b/src/util/Makefile.am
@@ -19,7 +19,10 @@
  # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
  # IN THE SOFTWARE.

-SUBDIRS = xmlpool . tests/hash_table
+SUBDIRS = . \
+   xmlpool \
+   tests/hash_table \
+   tests/string_buffer

  include Makefile.sources

diff --git a/src/util/tests/string_buffer/Makefile.am 
b/src/util/tests/string_buffer/Makefile.am
new file mode 100644
index 00..bd04d86349
--- /dev/null
+++ b/src/util/tests/string_buffer/Makefile.am
@@ -0,0 +1,40 @@
+# Copyright © 2017 Thomas Helland
+#
+#  Permission is hereby granted, free of charge, to any person obtaining a
+#  copy of this software and associated documentation files (the "Software"),
+#  to deal in the Software without restriction, including without limitation
+#  the rights to use, copy, modify, merge, publish, distribute, sublicense,
+#  and/or sell copies of the Software, and to permit persons to whom the
+#  Software is furnished to do so, subject to the following conditions:
+#
+#  The above copyright notice and this permission notice (including the next
+#  paragraph) shall be included in all copies or substantial portions of the
+#  Software.
+#
+#  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+#  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+#  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+#  THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+#  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+#  FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+#  IN THE SOFTWARE.
+
+AM_CPPFLAGS = \
+   -I$(top_srcdir)/src \
+   -I$(top_srcdir)/include \
+   -I$(top_srcdir)/src/gtest/include \
+   $(PTHREAD_CFLAGS) \
+   $(DEFINES)
+
+TESTS = string_buffer_test
+
+check_PROGRAMS = $(TESTS)
+
+string_buffer_test_SOURCES = \
+   string_buffer_test.cpp
+
+string_buffer_test_LDADD = \
+   $(top_builddir)/src/gtest/libgtest.la \
+   $(top_builddir)/src/util/libmesautil.la \
+   $(PTHREAD_LIBS) \
+   $(DLOPEN_LIBS)
diff --git a/src/util/tests/string_buffer/string_buffer_test.cpp 
b/src/util/tests/string_buffer/string_buffer_test.cpp
new file mode 100644
index 00..e80ee8b135
--- /dev/null
+++ b/src/util/tests/string_buffer/string_buffer_test.cpp
@@ -0,0 +1,119 @@
+/*
+ * Copyright © 2017 Thomas Helland
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included 

Re: [Mesa-dev] [PATCH 2/2] radeonsi: remove useless check in si_blit_decompress_color()

2017-09-26 Thread Nicolai Hähnle

Gert's suggestion makes sense. Apart from that, the series is

Reviewed-by: Nicolai Hähnle 


On 22.09.2017 09:22, Samuel Pitoiset wrote:

That's unnecessary to double-check that dcc_offset is not 0
because all callers already check that.

Signed-off-by: Samuel Pitoiset 
---
  src/gallium/drivers/radeonsi/si_blit.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeonsi/si_blit.c 
b/src/gallium/drivers/radeonsi/si_blit.c
index 0ecfc83fe2..2c846d2914 100644
--- a/src/gallium/drivers/radeonsi/si_blit.c
+++ b/src/gallium/drivers/radeonsi/si_blit.c
@@ -470,7 +470,7 @@ static void si_blit_decompress_color(struct pipe_context 
*ctx,
 "Decompress Color (levels %u - %u, mask 0x%x)\n\n",
 first_level, last_level, level_mask);
  
-	if (rtex->dcc_offset && need_dcc_decompress) {

+   if (need_dcc_decompress) {
custom_blend = sctx->custom_blend_dcc_decompress;
  
  		/* disable levels without DCC */





--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] MESA and KOTOR

2017-09-26 Thread Nicolai Hähnle

On 25.09.2017 18:50, Federico Dossena wrote:

Hello everyone,
you may remember that a few months ago I was trying to fix KOTOR to work 
with Mesa to use the Gallium llvmpipe software renderer.


Well, it's been a while and I'm happy to see that things are a bit 
better with Mesa 17.2. The game still crashes, but we're closer to 
fixing it.


Here's what I found using 17.2.1:
With frame buffer effects and soft shadows the game crashes at the end 
of loading; the crash is inside a function that amongst other things, 
generates mipmaps for a texture used in a pbuffer (function at offset 
2FB37D in my exe).
The crash happens when gluBuild2DMipmaps is called, however doesn't seem 
to be a null pointer like it was back in march: it's an access violation 
alright but no longer a null pointer. So I think it's a different, 
hopefully simpler, problem.


So is it a crash in KOTOR, in glu, or in Mesa? If it's in one of the 
last two, then you should be able to compile both glu and Mesa from 
source with debugging info to help you narrow things down. A backtrace 
would be a good start.


If it's in KOTOR itself, then I'm afraid we're probably not going to be 
a lot of help here...


Cheers,
Nicolai



Back in march, Miklòs Màté suggested that changing the checks for the 
pixel format could fix the problem, and he was right; without those 
checks we definitely got a step closer to fixing it.


My first thought was to just NOP the entire section that generates 
mipmaps and a bit of code later that uses it. The game no longer 
crashes, however it displays nothing, but I can hear it running in 
background. So this is the last issue! We're almost there!


Now, I'm bothering you again because I think that at this point it's 
just a problem with the texture format used there. The call to 
gluBuild2DMipmaps uses LuminanceAlpha' as texture format as well as 
internal format (0x190a). I tried changing it to RGB and RGBA just to 
try something, but that didn't work because I guess the texture was 
already generated with another format.


What could I do to investigate this further? And where should I look 
inside Mesa if I wanted to say... force a specific texture format for 
pbuffers?


I feel that we're very close to fixing this. Your help would mean the 
world to me and the whole KOTOR community.


Thank you ;)

P.S.
This has nothing to do with mesa, but you should know that KOTOR is 
slowly dieing. It is currently unplayable on Intel and AMD graphics, and 
recent nVidia driver updates have introduced a glitch with 
transparencies (it can be fixed, but still, no one can play KOTOR on 
modern hardware properly and we have to keep old computers as dedicated 
"shrines" for KOTOR, that's why I insist so much on Mesa)




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




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] gallium/util: use new util_vasprintf() function

2017-09-26 Thread Nicolai Hähnle

Thanks :)

Both patches:

Reviewed-by: Nicolai Hähnle 


On 25.09.2017 23:08, Brian Paul wrote:

---
  src/gallium/auxiliary/util/u_log.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/util/u_log.c 
b/src/gallium/auxiliary/util/u_log.c
index 359b3e1..dacbe05 100644
--- a/src/gallium/auxiliary/util/u_log.c
+++ b/src/gallium/auxiliary/util/u_log.c
@@ -24,6 +24,7 @@
  #include "u_log.h"
  
  #include "u_memory.h"

+#include "util/u_string.h"
  
  struct page_entry {

 const struct u_log_chunk_type *type;
@@ -129,7 +130,7 @@ u_log_printf(struct u_log_context *ctx, const char *fmt, 
...)
 char *str = NULL;
  
 va_start(va, fmt);

-   int ret = vasprintf(&str, fmt, va);
+   int ret = util_vasprintf(&str, fmt, va);
 va_end(va);
  
 if (ret >= 0) {





--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 5/5] radeonsi: allow out-of-order rasterization in commutative blending cases

2017-09-26 Thread Nicolai Hähnle

On 21.09.2017 23:44, Matias N. Goldberg wrote:

I'm reviewing the patch and there's something that confuses me about 
radeonsi_assume_no_z_fights (which got implemented in a later patch). It 
appears to be that part of the logic is flawed.

Please correct me whenever you feel I misunderstood what is going on.

Assuming colour writes are enabled, si_out_of_order_rasterization will return 
true only if the following conditions are met (simplified):
* There is a zsbuf set (If I interpret it correctly, this means if there is a 
zs buffer attached)
* dsa_order_invariant.pass_set is true
* dsa_order_invariant.pass_last is true


That's not what the code does, look again:

* if blending is enabled and commutative, pass_set must be true
* if blending is enabled and non-commutative, we simply cannot enable ooo
o if blending is disabled, pass_last must be true

What may be confusing when you read the code is that, with multiple 
render targets, it's theoretically possible for render targets to have 
different blend states.




_or_

* There is no zsbuf set
* dsa_order_invariant.pass_last is true


However, the logic is apparently contradictory, because:
* pass_set  will only be true when depth writes are disabled or depth func is 
set to always or depth func is set to never.
* pass_last will only be true when depth writes are enabled or depth func is 
not set to always nor not_equal.


!!This is impossible to satisfy unless depth function is set to never!!
Not only this is extremely rare, it appears this is not the intention behind the option 
"radeonsi_assume_no_z_fights" which I believe is an optimization for gamers to 
get a performance boost in most games where forcing this option doesn't matter (either 
because the artifacts are extremely rare or not present).

Additionally, there seems to be a bug because si_out_of_order_rasterization can 
return true if there is no zs buffer and user enabled 
radeonsi_assume_no_z_fights, which AFAIK is blatantly wrong (correct me if I'm 
wrong, but if there is no zs buffer, out of order rasterization can cause 
really wrong results).


When there is no Z buffer, radeonsi_assume_no_z_fights has no effect. 
radeonsi_assume_no_z_fights only affects anything via the 
si_dsa_order_invariance settings, and without a Z buffer we simply use 
the default (which is set in si_state.c:3232, the initializer of the 
dsa_order_invariance variable).


Cheers,
Nicolai




Maybe I misunderstood what's going on, or I missed something key. But if I'm 
right then the logic needs to be revised.


It would appear to me that idea of radeonsi_assume_no_z_fights is that it 
should always enable OoO rasterization as long as depth writes are on and a 
valid zs is present (and other conditions are met such as shaders not 
requesting early depth stencil, blending operations, etc). But written as is 
right now, it will almost never be enabled even if the options is forced on.

Cheers
Matias

De: Marek Olšák 
Para: Nicolai Hähnle 
CC: "mesa-dev@lists.freedesktop.org" ; Nicolai 
Hähnle 
Enviado: Miércoles, 13 de septiembre, 2017 21:19:26
Asunto: Re: [Mesa-dev] [PATCH 5/5] radeonsi: allow out-of-order rasterization 
in commutative blending cases



For the series:

Reviewed-by: Marek Olšák 

Marek

On Sat, Sep 9, 2017 at 12:43 PM, Nicolai Hähnle  wrote:

From: Nicolai Hähnle 

We do not enable this by default for additive blending, since it slightly
breaks OpenGL invariance guarantees due to non-determinism.

Still, there may be some applications can benefit from white-listing
via the radeonsi_commutative_blend_add drirc setting without any real
visible artifacts.
---
  src/gallium/drivers/radeonsi/driinfo_radeonsi.h |  1 +
  src/gallium/drivers/radeonsi/si_pipe.c  |  2 +
  src/gallium/drivers/radeonsi/si_pipe.h  |  1 +
  src/gallium/drivers/radeonsi/si_state.c | 67 +++--
  src/gallium/drivers/radeonsi/si_state.h |  1 +
  src/util/xmlpool/t_options.h|  5 ++
  6 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/driinfo_radeonsi.h 
b/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
index 8be85289a0c..989e5175cc0 100644
--- a/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
+++ b/src/gallium/drivers/radeonsi/driinfo_radeonsi.h
@@ -1,5 +1,6 @@
  // DriConf options specific to radeonsi
  DRI_CONF_SECTION_PERFORMANCE
  DRI_CONF_RADEONSI_ENABLE_SISCHED("false")
  DRI_CONF_RADEONSI_ASSUME_NO_Z_FIGHTS("false")
+DRI_CONF_RADEONSI_COMMUTATIVE_BLEND_ADD("false")
  DRI_CONF_SECTION_END
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
b/src/gallium/drivers/radeonsi/si_pipe.c
index b4972be739c..c44ea3be740 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -1043,20 +1043,22 @@ struct pipe_screen *radeonsi_screen_create(struct 
radeon_winsys *ws,
 (sscreen->b.chip_class == SI &&
  sscre

Re: [Mesa-dev] [PATCH mesa 6/6] scons: use python3-compatible string-check

2017-09-26 Thread Jose Fonseca

On 25/09/17 14:30, Eric Engestrom wrote:

I pushed the rest of the series.
See below for discussion on this patch.


On Wednesday, 2017-09-20 17:05:21 +, Jose Fonseca wrote:

On 19/09/17 15:14, Eric Engestrom wrote:

Signed-off-by: Eric Engestrom 
---
   scons/custom.py | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scons/custom.py b/scons/custom.py
index 0767ba936d410167116d..978ee5f9ec7c23a74cb9 100644
--- a/scons/custom.py
+++ b/scons/custom.py
@@ -257,7 +257,7 @@ def parse_source_list(env, filename, names=None):
   sym_table = parser.parse(src.abspath)
   if names:
-if isinstance(names, basestring):
+if isinstance(names, str):
   names = [names]
   symbols = names



I'm not sure if this won't give the wrong results for unicode strings, but
at any rate, I don't think that should ever happen in practice.


Are you replying to Ilia [1] here?

He left a comment on this patch, saying:

This might be python3-compatible, but it's not the same thing. str !=
unicode. Not sure where "names" can come from, but if it can come in
as a unicode string, this won't work.


My knowledge of python is quite basic, so I'd rather you discuss between
you two rather than me trying to forward a conversation with each of you :P

[1] https://lists.freedesktop.org/archives/mesa-dev/2017-September/170130.html



I just checked all uses of ParseSourceList and it's never used with 
unicode strings.  It's always used with plain strings or lists of plain 
strings.


So on 2nd thought, I think this patch should be safe.  Ilia, would you 
agree?



basestring doesn't exist on Python 3, so this will be necessary eventually.


BTW, thank you for looking into this Eric.


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


Re: [Mesa-dev] [PATCH mesa 6/6] scons: use python3-compatible string-check

2017-09-26 Thread Ilia Mirkin
On Tue, Sep 26, 2017 at 7:07 AM, Jose Fonseca  wrote:
> On 25/09/17 14:30, Eric Engestrom wrote:
>>
>> I pushed the rest of the series.
>> See below for discussion on this patch.
>>
>>
>> On Wednesday, 2017-09-20 17:05:21 +, Jose Fonseca wrote:
>>>
>>> On 19/09/17 15:14, Eric Engestrom wrote:

 Signed-off-by: Eric Engestrom 
 ---
scons/custom.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/scons/custom.py b/scons/custom.py
 index 0767ba936d410167116d..978ee5f9ec7c23a74cb9 100644
 --- a/scons/custom.py
 +++ b/scons/custom.py
 @@ -257,7 +257,7 @@ def parse_source_list(env, filename, names=None):
sym_table = parser.parse(src.abspath)
if names:
 -if isinstance(names, basestring):
 +if isinstance(names, str):
names = [names]
symbols = names

>>>
>>> I'm not sure if this won't give the wrong results for unicode strings,
>>> but
>>> at any rate, I don't think that should ever happen in practice.
>>
>>
>> Are you replying to Ilia [1] here?
>>
>> He left a comment on this patch, saying:
>>>
>>> This might be python3-compatible, but it's not the same thing. str !=
>>> unicode. Not sure where "names" can come from, but if it can come in
>>> as a unicode string, this won't work.
>>
>>
>> My knowledge of python is quite basic, so I'd rather you discuss between
>> you two rather than me trying to forward a conversation with each of you
>> :P
>>
>> [1]
>> https://lists.freedesktop.org/archives/mesa-dev/2017-September/170130.html
>>
>
> I just checked all uses of ParseSourceList and it's never used with unicode
> strings.  It's always used with plain strings or lists of plain strings.
>
> So on 2nd thought, I think this patch should be safe.  Ilia, would you
> agree?

So really this whole thing is about whether names is a list or not.
You can't check if it's iterable since strings are also iterable. What
lists are passed in here? If it's all list-type lists (and not
tuples/etc), we can change this to be

if not isinstance(names, list)

Or we want to be cheeky,

if type(names) == type(names[0])

For a list of strings, that won't be the case, but for a string, it
should be, since a single char has the same type as a string. At least
in python2.

I don't use scons, or know how all this stuff is used, so you guys can
do whatever. In my experience, unicode strings spring up from the most
unexpected places.

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] nv50/ir: skip optimizing ADD+SHL to SHLADD when src(1) is 0

2017-09-26 Thread Juan A. Suarez Romero
On Tue, 2017-09-26 at 06:45 -0400, Ilia Mirkin wrote:
> On Tue, Sep 26, 2017 at 6:30 AM, Samuel Pitoiset
>  wrote:
> > 
> > 
> > On 09/26/2017 12:29 PM, Juan A. Suarez Romero wrote:
> > > 
> > > On Mon, 2017-05-01 at 12:11 -0400, Ilia Mirkin wrote:
> > > > 
> > > > On Mon, May 1, 2017 at 12:09 PM, Samuel Pitoiset
> > > >  wrote:
> > > > > 
> > > > > On 05/01/2017 05:59 PM, Ilia Mirkin wrote:
> > > > > > 
> > > > > > 
> > > > > > I think this is off. It shouldn't matter what the code sequence is,
> > > > > > it's all representable. You need to teach replaceZero to not mess
> > > > > > things up for SHLADD's src(1).
> > > > > 
> > > > > 
> > > > > 
> > > > > It's representable but stupid to do it. We should keep the ADD there 
> > > > > and
> > > > > this also avoids a workaround in the replaceZero logic just for that.
> > > > > 
> > > > > IMHO, this is the better solution.
> > > > 
> > > > 
> > > > Yes, that would be a better optimization. However at this stage, you
> > > > have a y = SHL(x, 0) + ADD(y, z). I think it makes sense to combine
> > > > them into one and let DCE take care of it.
> > > > 
> > > > Ideally we wouldn't get into such a situation, but the real issue is
> > > > that a perfectly representable (if dumb) instruction like SHLADD(x, 0,
> > > > y) gets messed up by replaceZero. We should fix replaceZero. We should
> > > > also try to avoid generating such stupid instructions.
> > > > 
> > > 
> > > So what was the final conclusion for this patch? Was replaceZero fixed?
> > 
> > 
> > I don't think it has been fixed, but I could be wrong.
> 
> https://cgit.freedesktop.org/mesa/mesa/commit/src/gallium/drivers/nouveau/codegen?id=82e77d4e4484b5d4f6a7b4751a17c882e6d2ad69

Thanks for the reply!


J.A.

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


Re: [Mesa-dev] [PATCH] glsl: Trigger precision mismatch error only if both symbols are used

2017-09-26 Thread Eero Tamminen

Hi,

On 26.09.2017 11:33, Tomasz Figa wrote:

An update on my testing inline.

On Tue, Sep 26, 2017 at 7:13 AM, Tomasz Figa  wrote:

On Tue, Sep 26, 2017 at 6:21 AM, Tomasz Figa  wrote:

Hi Ian,

On Tue, Sep 26, 2017 at 5:59 AM, Ian Romanick  wrote:

On 09/25/2017 02:53 AM, Tomasz Figa wrote:

Commit 259fc505454ea6a67aeacf6cdebf1398d9947759 added linker error for
mismatching uniform precision, as required by GLES specification and
conformance test-suite.

Several Android applications, including Forge of Empires, have shaders
which violate this rule, on a dead varying that will be eliminated.
The problem affects a big number of applications using Cocos2D engine
and other GLES implementations accept this, so work around the problem
by erroring out only if both symbols are actually referenced in the
code, which is the only case when the mismatch can cause incorrect
behavior.


I think this change will cause failures in the CTS... but, there's a
Khronos bug about this issue, and I think some of the rules are going to
get relaxed.  I'd like to wait until that gets sorted before we start
changing how our linker applies the rules.


Are we 100% sure about the failures? There are multiple GLES
implementations that claim to be compliant, yet they allow such
behavior.


I'm looking through Khronos CTS github repository and it seems like a
related test 
(dEQP-GLES3.functional.shaders.linkage.uniform.block.differing_precision)
was removed from GLES 3 suite and instead few others added to GLES 3.1
suite:

https://github.com/KhronosGroup/VK-GL-CTS/commit/b8d0d0a842280c2594703ecce985563aebe1cde8#diff-72bd3f02df978e071f2a53ec2654ddd5

Moreover I can see a test being included in GLES 3.2 suite
(KHR-GLES32.shaders.negative.unused_uniform_precision_matching) that
prohibits precision mismatch even for unused uniforms, but it is not
included in any suites for earlier GLES versions.


Just ran the whole suite for GLES 3.0 conformance run with i965 driver
on Android and there is only 1 failure that seems to be unrelated to
the workaround (fails both with and without the change):

KHR-GLES3.shaders.uniform_block.common.precision_matching

However, I just realized that the applications affected are all using
GLSL ES 1.00. So, if we turn the condition introduced by my patch into
"(existing->data.used && var->data.used) || prog->data->Version >=
300", we should be safe both on compatibility and conformance side.


Btw. If you're also interested about performance, there's a difference 
between GLES 3.x and earlier GLES version semantics that affects 
performance.


According to the GLES 3.0 spec, cubemap sampling is required to always 
be seamless (sampling from multiple surfaces near the edges), but in 
earlier GLES versions, this isn't required (on GL side, it's application 
decision, so there it doesn't matter).


For anything using 1x1 / 2x2 cubemaps, this can have clear performance. 
Do many Android applications use them?



This is relevant as Mesa uses GLES 3 semantics also for GLES 2:
- src/mesa/main/texstate.c -
   /* Appendix F.2 of the OpenGL ES 3.0 spec says:
*
* "OpenGL ES 3.0 requires that all cube map filtering be
* seamless. OpenGL ES 2.0 specified that a single cube map face be
* selected and used for filtering."
*
* Unfortunatley, a call to _mesa_is_gles3 below will only work if
* the driver has already computed and set ctx->Version, however drivers
* seem to call _mesa_initialize_context (which calls this) early
* in the CreateContext hook and _mesa_compute_version much later (since
* it needs information about available extensions). So, we will
* enable seamless cubemaps by default since GLES2. This should work
* for most implementations and drivers that don't support seamless
* cubemaps for GLES2 can still disable it.
*/
   ctx->Texture.CubeMapSeamless = ctx->API == API_OPENGLES2;



- Eero


Note that this is affecting a huge number of Android applications,
including a commonly used 2D game engine and we're having a really big
problem right now, because even if the engine gets fixed, it's very
unlikely that all the developers update their engine.




Based on Kenneth Graunke's patch from Bugzilla, reworked from a drirc
option that completely bypasses the check into an incoditional check


 unconditional


that triggers either an error or warning, respectively if both
declarations are further referenced by the code or not.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97532
Signed-off-by: Tomasz Figa 
---
  src/compiler/glsl/linker.cpp | 14 ++
  1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index f352c5385ca..1c534ea1a3b 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1121,10 +1121,

Re: [Mesa-dev] [PATCH v2 7/7] i965: make use of nir linking

2017-09-26 Thread Eero Tamminen

Hi,

On 26.09.2017 12:04, Timothy Arceri wrote:

On 26/09/17 17:50, Kenneth Graunke wrote:

[...]

2. It would be great to use this in anv as well, especially given that
    it has no GLSL IR linking to do this sort of stuff.  Plus, the SPIR-V
    might be compiled independently...and when you build the pipeline,
    you have all the stages and can do cross-stage linking optimizations
    like this.


Yeah the reason for doing this was to eventually add it to radv. i965 
was just a nicer place to test it, same goes for the passes that will 
follow. Right now the biggest blocked is justifying the added complexity 
as there isn't a standard shader-db like tool, and it's not making any 
noticeable differences in any benchmarks.


I think the old GLBenchmark v2.5 / v2.7 tests (e.g. Egypt) might be 
something where cross-stage linking optimizations impact could show up.



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


Re: [Mesa-dev] MESA and KOTOR

2017-09-26 Thread Nicolai Hähnle

On 26.09.2017 12:59, Federico Dossena wrote:
The crash is in GLU, and I'm 99% sure that it has to do with the format 
of that texture.


So how about a backtrace?

It would be really valuable to have a simple stand-alone reproduction.

Cheers,
Nicolai


I saw a patch from Miklòs Màté a while ago that changed something in 
src/mesa/state_tracker/st_atom_sampler.c to fix this (link: 
https://patchwork.freedesktop.org/patch/68298/). It never made it to 
master, and that function has changed quite a bit since his patch. I 
don't really understand what it does since I don't know mesa very well, 
do you know how I could do the same thing in the new st_convert_sampler 
function?


Thanks


Il 2017-09-26 12:56, Nicolai Hähnle ha scritto:

On 25.09.2017 18:50, Federico Dossena wrote:

Hello everyone,
you may remember that a few months ago I was trying to fix KOTOR to 
work with Mesa to use the Gallium llvmpipe software renderer.


Well, it's been a while and I'm happy to see that things are a bit 
better with Mesa 17.2. The game still crashes, but we're closer to 
fixing it.


Here's what I found using 17.2.1:
With frame buffer effects and soft shadows the game crashes at the 
end of loading; the crash is inside a function that amongst other 
things, generates mipmaps for a texture used in a pbuffer (function 
at offset 2FB37D in my exe).
The crash happens when gluBuild2DMipmaps is called, however doesn't 
seem to be a null pointer like it was back in march: it's an access 
violation alright but no longer a null pointer. So I think it's a 
different, hopefully simpler, problem.


So is it a crash in KOTOR, in glu, or in Mesa? If it's in one of the 
last two, then you should be able to compile both glu and Mesa from 
source with debugging info to help you narrow things down. A backtrace 
would be a good start.


If it's in KOTOR itself, then I'm afraid we're probably not going to 
be a lot of help here...


Cheers,
Nicolai



Back in march, Miklòs Màté suggested that changing the checks for the 
pixel format could fix the problem, and he was right; without those 
checks we definitely got a step closer to fixing it.


My first thought was to just NOP the entire section that generates 
mipmaps and a bit of code later that uses it. The game no longer 
crashes, however it displays nothing, but I can hear it running in 
background. So this is the last issue! We're almost there!


Now, I'm bothering you again because I think that at this point it's 
just a problem with the texture format used there. The call to 
gluBuild2DMipmaps uses LuminanceAlpha' as texture format as well as 
internal format (0x190a). I tried changing it to RGB and RGBA just to 
try something, but that didn't work because I guess the texture was 
already generated with another format.


What could I do to investigate this further? And where should I look 
inside Mesa if I wanted to say... force a specific texture format for 
pbuffers?


I feel that we're very close to fixing this. Your help would mean the 
world to me and the whole KOTOR community.


Thank you ;)

P.S.
This has nothing to do with mesa, but you should know that KOTOR is 
slowly dieing. It is currently unplayable on Intel and AMD graphics, 
and recent nVidia driver updates have introduced a glitch with 
transparencies (it can be fixed, but still, no one can play KOTOR on 
modern hardware properly and we have to keep old computers as 
dedicated "shrines" for KOTOR, that's why I insist so much on Mesa)




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









--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] egl/dri: link directly to libglapi.so

2017-09-26 Thread Eric Engestrom
On Tuesday, 2017-09-19 17:19:59 +, Emil Velikov wrote:
> From: Emil Velikov 
> 
> In order to build EGL, one has to use shared glapi - libglapi.so.
> 
> Thus the dlopen/dlsym dance is no longer needed and we can link to the
> library directly.
> 
> This allows us to remove a handful of platform specific names of the
> library.
> 
> Cc: Jonathan Gray 
> Cc: Jon Turney 
> Cc: Julien Isorce 
> Cc: Rob Herring 
> Cc: Tomasz Figa 
> Signed-off-by: Emil Velikov 
> ---
>  src/egl/Android.mk  |  2 ++
>  src/egl/Makefile.am |  2 ++
>  src/egl/drivers/dri2/egl_dri2.c | 40 
>  src/egl/drivers/dri2/egl_dri2.h |  2 --
>  4 files changed, 8 insertions(+), 38 deletions(-)
> 
[snip]
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 42bca61cfda..c67117212c8 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -64,6 +64,7 @@
>  #include "loader/loader.h"
>  #include "util/u_atomic.h"
>  #include "util/u_vector.h"
> +#include "mapi/glapi/glapi.h"
>  
>  /* The kernel header drm_fourcc.h defines the DRM formats below.  We 
> duplicate
>   * some of the definitions here so that building Mesa won't bleeding-edge
> @@ -1556,7 +1557,7 @@ dri2_get_proc_address(_EGLDriver *drv, const char 
> *procname)
>  {
> struct dri2_egl_driver *dri2_drv = dri2_egl_driver(drv);

^ dead code (== compiler warning)

>  
> -   return dri2_drv->get_proc_address(procname);
> +   return _glapi_get_proc_address(procname);
>  }
>  
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] radeonsi: remove useless check in si_blit_decompress_color()

2017-09-26 Thread Samuel Pitoiset



On 09/26/2017 12:45 PM, Nicolai Hähnle wrote:

Gert's suggestion makes sense. Apart from that, the series is


Yeah, I will add an assert over there, thanks!



Reviewed-by: Nicolai Hähnle 


On 22.09.2017 09:22, Samuel Pitoiset wrote:

That's unnecessary to double-check that dcc_offset is not 0
because all callers already check that.

Signed-off-by: Samuel Pitoiset 
---
  src/gallium/drivers/radeonsi/si_blit.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeonsi/si_blit.c 
b/src/gallium/drivers/radeonsi/si_blit.c

index 0ecfc83fe2..2c846d2914 100644
--- a/src/gallium/drivers/radeonsi/si_blit.c
+++ b/src/gallium/drivers/radeonsi/si_blit.c
@@ -470,7 +470,7 @@ static void si_blit_decompress_color(struct 
pipe_context *ctx,

   "Decompress Color (levels %u - %u, mask 0x%x)\n\n",
   first_level, last_level, level_mask);
-    if (rtex->dcc_offset && need_dcc_decompress) {
+    if (need_dcc_decompress) {
  custom_blend = sctx->custom_blend_dcc_decompress;
  /* disable levels without DCC */





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


Re: [Mesa-dev] [PATCH 2/2] anv: add an assertion in genX(BeginCommandBuffer)

2017-09-26 Thread Jason Ekstrand
Reviewed-by: Jason Ekstrand 

On Tue, Sep 26, 2017 at 1:14 AM, Gwan-gyeong Mun  wrote:

> To check a valid usage requirement.
>
> Signed-off-by: Mun Gwan-gyeong 
> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index fbc1995709..3559399019 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -1006,6 +1006,7 @@ genX(BeginCommandBuffer)(
> VkResult result = VK_SUCCESS;
> if (cmd_buffer->usage_flags &
> VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT) {
> +  assert(pBeginInfo->pInheritanceInfo);
>cmd_buffer->state.pass =
>   anv_render_pass_from_handle(pBeginInfo->pInheritanceInfo->
> renderPass);
>cmd_buffer->state.subpass =
> --
> 2.14.1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] [rfc] spirv/nir: handle casting in OpSampledImage

2017-09-26 Thread Jason Ekstrand
On Mon, Sep 25, 2017 at 7:36 PM, Dave Airlie  wrote:

> From: Dave Airlie 
>
> %9903 = OpImageSampleDrefExplicitLod %float %14616 %14315 %16081 Lod
> %float_0
> %14616 = OpSampledImage %510 %8499 %13137
>
> %278 = OpTypeImage %float 2D 1 0 0 1 Unknown
> %510 = OpTypeSampledImage %278
>
> %8499 = OpLoad %150 %4159
> %150 = OpTypeImage %float 2D 0 0 0 1 Unknown
>
> Is being generated by a hlsl->spirv convertor.
>
> So it appears that the 510 return type from sampledimage is a shadow
> sampler,
> however the descriptor it's loading is for a 2D non-shadow image, which
> makes it seems like OpSampledImage should cast appropriately.
>
> Now I'm not sure enough to know if this is valid spir-v in the first place,
> and I don't think this patch is good enough to fix it even if it is, but
> let's use it to open discussions.
>

Yeah, that's not a thing From the SPIR-V spec for OpLoad:

*Pointer* is the pointer to load through. Its type must be an
*OpTypePointer* whose *Type* operand is the same as *Result Type*.

You're not allowed to do an implicit cast in an OpLoad.  My guess is that
HLSL only makes a distinction between shadow and non-shadow sampling at the
sample instruction and not at the interface point.  GL, however, has lots
of history encoded in these things...

Please file a bug against GLSLang.  If it's too hard to fix there, I'm
happy for John to kick it back to the SPIR-V working group and try to come
up with a way to do this properly.  I'm reluctant to just let it through
because I have no idea what will happen in our hardware if you try to do a
depth comparison on an RGBA8 image.

--Jason

---
>  src/compiler/spirv/spirv_to_nir.c | 6 +-
>  src/compiler/spirv/vtn_private.h  | 1 +
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/compiler/spirv/spirv_to_nir.c
> b/src/compiler/spirv/spirv_to_nir.c
> index 6ce9d1a..d962e93 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -1490,6 +1490,8 @@ vtn_handle_texture(struct vtn_builder *b, SpvOp
> opcode,
>struct vtn_value *val =
>   vtn_push_value(b, w[2], vtn_value_type_sampled_image);
>val->sampled_image = ralloc(b, struct vtn_sampled_image);
> +  val->sampled_image->ret_type =
> + vtn_value(b, w[1], vtn_value_type_type)->type;
>val->sampled_image->image =
>   vtn_value(b, w[3], vtn_value_type_pointer)->pointer;
>val->sampled_image->sampler =
> @@ -1521,7 +1523,9 @@ vtn_handle_texture(struct vtn_builder *b, SpvOp
> opcode,
> }
>
> const struct glsl_type *image_type;
> -   if (sampled.image) {
> +   if (sampled.ret_type) {
> +  image_type = sampled.ret_type->type;
> +   } else if (sampled.image) {
>image_type = sampled.image->var->var->interface_type;
> } else {
>image_type = sampled.sampler->var->var->interface_type;
> diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_
> private.h
> index 8458462..ce26326 100644
> --- a/src/compiler/spirv/vtn_private.h
> +++ b/src/compiler/spirv/vtn_private.h
> @@ -413,6 +413,7 @@ struct vtn_image_pointer {
>  struct vtn_sampled_image {
> struct vtn_pointer *image; /* Image or array of images */
> struct vtn_pointer *sampler; /* Sampler */
> +   struct vtn_type *ret_type;
>  };
>
>  struct vtn_value {
> --
> 2.9.5
>
> ___
> 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 v2 7/7] i965: make use of nir linking

2017-09-26 Thread Jason Ekstrand

On September 26, 2017 4:05:04 AM Timothy Arceri  wrote:


On 26/09/17 17:50, Kenneth Graunke wrote:

On Monday, September 25, 2017 6:23:13 PM PDT Timothy Arceri wrote:

For now linking is just removing unused varyings between stages.

shader-db results BDW:

total instructions in shared programs: 13198288 -> 13191693 (-0.05%)
instructions in affected programs: 48325 -> 41730 (-13.65%)
helped: 473
HURT: 0

total cycles in shared programs: 541184926 -> 541159260 (-0.00%)
cycles in affected programs: 213238 -> 187572 (-12.04%)
helped: 435
HURT: 8

V2:
- lower indirects on demoted inputs as well as outputs.
---
  src/mesa/drivers/dri/i965/brw_link.cpp | 56 ++
  1 file changed, 56 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp 
b/src/mesa/drivers/dri/i965/brw_link.cpp

index b7fab8d7a25..9ddf0230183 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -253,6 +253,62 @@ brw_link_shader(struct gl_context *ctx, struct 
gl_shader_program *shProg)

   compiler->scalar_stage[stage]);
 }

+   /* Determine first and last stage. */
+   unsigned first = MESA_SHADER_STAGES;
+   unsigned last = 0;
+   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
+  if (!shProg->_LinkedShaders[i])
+ continue;
+  if (first == MESA_SHADER_STAGES)
+ first = i;
+  last = i;
+   }
+
+   /* Linking the stages in the opposite order (from fragment to vertex)
+* ensures that inter-shader outputs written to in an earlier stage
+* are eliminated if they are (transitively) not used in a later
+* stage.
+*/
+if (first != last) {
+   int next = last;
+   for (int i = next - 1; i >= 0; i--) {
+  if (shProg->_LinkedShaders[i] == NULL)
+ continue;
+
+nir_shader *producer = shProg->_LinkedShaders[i]->Program->nir;
+nir_shader *consumer = shProg->_LinkedShaders[next]->Program->nir;
+
+nir_remove_dead_variables(producer, nir_var_shader_out);
+nir_remove_dead_variables(consumer, nir_var_shader_in);
+
+if (nir_remove_unused_varyings(producer, consumer)) {
+   nir_lower_global_vars_to_local(producer);
+   nir_lower_global_vars_to_local(consumer);
+
+   nir_variable_mode indirect_mask = (nir_variable_mode) 0;
+   if (compiler->glsl_compiler_options[i].EmitNoIndirectTemp)
+  indirect_mask = (nir_variable_mode) nir_var_local;
+
+   /* The backend might not be able to handle indirects on
+* temporaries so we need to lower indirects on any of the
+* varyings we have demoted here.
+*/
+   nir_lower_indirect_derefs(producer, indirect_mask);
+   nir_lower_indirect_derefs(consumer, indirect_mask);
+
+   const bool p_is_scalar = 
compiler->scalar_stage[producer->stage];

+   shProg->_LinkedShaders[i]->Program->nir =
+ brw_nir_optimize(producer, compiler, p_is_scalar);
+
+   const bool c_is_scalar = 
compiler->scalar_stage[producer->stage];

+   shProg->_LinkedShaders[next]->Program->nir =
+ brw_nir_optimize(consumer, compiler, c_is_scalar);
+}
+
+next = i;
+   }
+}
+
 for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders); stage++) {
struct gl_linked_shader *shader = shProg->_LinkedShaders[stage];
if (!shader)



A couple more thoughts:

1. We're calling brw_nir_optimize even more now...it might make sense to
drop it from brw_preprocess_nir, drop it here, and put it once in the
final (third) loop, just before brw_shader_gather_info.  At least,
it'd be interesting how that affects compile times / quality...


Yeah I can have a play around. As is it didn't really change shader-db
compile times on my BDW so I wasn't too concerned.



2. It would be great to use this in anv as well, especially given that
it has no GLSL IR linking to do this sort of stuff.  Plus, the SPIR-V
might be compiled independently...and when you build the pipeline,
you have all the stages and can do cross-stage linking optimizations
like this.


Yeah the reason for doing this was to eventually add it to radv. i965
was just a nicer place to test it, same goes for the passes that will
follow. Right now the biggest blocked is justifying the added complexity
as there isn't a standard shader-db like tool, and it's not making any
noticeable differences in any benchmarks.

As far as anv I won't be doing any work on that directly, only radv.


That's fine. I'll add it in one of these days.

--Jason


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


Re: [Mesa-dev] [PATCH v3 1/2] etnaviv: fix varying interpolation

2017-09-26 Thread Wladimir J. van der Laan
Hello Lucas,

On Fri, Sep 22, 2017 at 11:27:36AM +0200, Lucas Stach wrote:
> It seems that newer cores don't use the PA_ATTRIBUTES to decide if the
> varying should bypass the flat shading, but derive this from the component
> use. This fixes flat shading on GC880+.
> 
> VARYING_COMPONENT_USE_POINTCOORD is a bit of a misnomer now, as it isn't
> only used for pointcoords, but missing a better name I left it as-is.

I was just looking at recent command streams, and it appears that the blob
uses VARYING_COMPONENT_USE_UNUSED even for active components.
I've not seen it use VARYING_COMPONENT_USE_USED at all anymore.

Maybe that works instead of using the POINTCOORD for this? I'm glad
this solved the issue, and I'm ok with merging this as-is, but I have never
seen the blob use POINTCOORD for non-pointcoords and feel it would potentially
interfere with point sprites in some cases.

Regards,
Wladimir

> 
> Signed-off-by: Lucas Stach 
> Reviewed-by: Philipp Zabel 
> Reviewed-by: Wladimir J. van der Laan 
> ---
> v2: fix invalid vreg assignment
> v3: fix missed negation, improve variable naming
> ---
>  src/gallium/drivers/etnaviv/etnaviv_compiler.c | 22 ++
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler.c 
> b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> index 165ab74298a4..0f6a5d23425d 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> @@ -2607,6 +2607,7 @@ etna_link_shader(struct etna_shader_link_info *info,
>const struct etna_shader_inout *fsio = &fs->infile.reg[idx];
>const struct etna_shader_inout *vsio = etna_shader_vs_lookup(vs, fsio);
>struct etna_varying *varying;
> +  bool interpolate_always = fsio->semantic.Name != TGSI_SEMANTIC_COLOR;
>  
>assert(fsio->reg > 0 && fsio->reg <= ARRAY_SIZE(info->varyings));
>  
> @@ -2616,27 +2617,24 @@ etna_link_shader(struct etna_shader_link_info *info,
>varying = &info->varyings[fsio->reg - 1];
>varying->num_components = fsio->num_components;
>  
> -  if (fsio->semantic.Name == TGSI_SEMANTIC_COLOR) /* colors affected by 
> flat shading */
> +  if (!interpolate_always) /* colors affected by flat shading */
>   varying->pa_attributes = 0x200;
>else /* texture coord or other bypasses flat shading */
>   varying->pa_attributes = 0x2f1;
>  
> -  if (fsio->semantic.Name == TGSI_SEMANTIC_PCOORD) {
> - varying->use[0] = VARYING_COMPONENT_USE_POINTCOORD_X;
> - varying->use[1] = VARYING_COMPONENT_USE_POINTCOORD_Y;
> - varying->use[2] = VARYING_COMPONENT_USE_USED;
> - varying->use[3] = VARYING_COMPONENT_USE_USED;
> - varying->reg = 0; /* replaced by point coord -- doesn't matter */
> +  varying->use[0] = interpolate_always ? 
> VARYING_COMPONENT_USE_POINTCOORD_X : VARYING_COMPONENT_USE_USED;
> +  varying->use[1] = interpolate_always ? 
> VARYING_COMPONENT_USE_POINTCOORD_Y : VARYING_COMPONENT_USE_USED;
> +  varying->use[2] = VARYING_COMPONENT_USE_USED;
> +  varying->use[3] = VARYING_COMPONENT_USE_USED;
> +
> +
> +  /* point coord is position output from VS, so has no dedicated reg */
> +  if (fsio->semantic.Name == TGSI_SEMANTIC_PCOORD)
>   continue;
> -  }
>  
>if (vsio == NULL)
>   return true; /* not found -- link error */
>  
> -  varying->use[0] = VARYING_COMPONENT_USE_USED;
> -  varying->use[1] = VARYING_COMPONENT_USE_USED;
> -  varying->use[2] = VARYING_COMPONENT_USE_USED;
> -  varying->use[3] = VARYING_COMPONENT_USE_USED;
>varying->reg = vsio->reg;
> }
>  
> -- 
> 2.11.0
> 
> ___
> etnaviv mailing list
> etna...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/etnaviv
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 00/11] glsl, tgsi, radeonsi: ldexp and frexp bug fixes and features

2017-09-26 Thread Nicolai Hähnle

Ping.

On 16.09.2017 13:23, Nicolai Hähnle wrote:

Hi all,

This series was motivated by radeonsi failing some ldexp tests due to
not handling denorms correctly and not handling overflows (which GLSL
doesn't require, but GLSL ES does).

The first patch fixes the GLSL IR lowering of ldexp() to handle all cases
fully except:

1. Denorms; they're annoying and therefore all flushed to zero.

2. Exponent 32-bit overflow. This would be easy to fix at the cost of
an additional min() instruction, but neither GLSL nor GLSL ES
requires it.

The goal of the remainder of the series is to move radeonsi to use
the hardware implementation of ldexp and frexp. This entails cleaning
up a lot of stuff around the existing DLDEXP and DFRACEXP (implemented
by softpipe and r600), as well as adding a new LDEXP opcode.

Please review!

Cheers,
Nicolai
--
  src/compiler/glsl/lower_instructions.cpp | 171 +++--
  .../auxiliary/gallivm/lp_bld_limits.h|   1 +
  src/gallium/auxiliary/gallivm/lp_bld_tgsi.c  |  26 ++-
  src/gallium/auxiliary/gallivm/lp_bld_tgsi.h  |   1 +
  .../auxiliary/gallivm/lp_bld_tgsi_action.h   |   5 +
  .../auxiliary/gallivm/lp_bld_tgsi_soa.c  |  19 +-
  src/gallium/auxiliary/nir/tgsi_to_nir.c  |   7 +-
  src/gallium/auxiliary/tgsi/tgsi_exec.c   |  31 ++-
  src/gallium/auxiliary/tgsi/tgsi_exec.h   |  10 +
  src/gallium/auxiliary/tgsi/tgsi_info.c   |  11 +-
  src/gallium/auxiliary/tgsi/tgsi_info.h   |   4 +-
  .../auxiliary/tgsi/tgsi_info_opcodes.h   |   4 +-
  src/gallium/docs/source/screen.rst   |   1 +
  src/gallium/docs/source/tgsi.rst |  24 ++-
  src/gallium/drivers/etnaviv/etnaviv_screen.c |   1 +
  .../drivers/freedreno/freedreno_screen.c |   1 +
  src/gallium/drivers/i915/i915_screen.c   |   1 +
  .../drivers/nouveau/nv30/nv30_screen.c   |   2 +
  .../drivers/nouveau/nv50/nv50_screen.c   |   1 +
  .../drivers/nouveau/nvc0/nvc0_screen.c   |   1 +
  src/gallium/drivers/r300/r300_screen.c   |   2 +
  src/gallium/drivers/r600/r600_pipe.c |   1 +
  src/gallium/drivers/r600/r600_shader.c   |  14 +-
  src/gallium/drivers/radeonsi/si_pipe.c   |   3 +-
  src/gallium/drivers/radeonsi/si_shader.c |  14 +-
  .../drivers/radeonsi/si_shader_internal.h|   1 +
  .../drivers/radeonsi/si_shader_tgsi_alu.c|  27 +++
  .../drivers/radeonsi/si_shader_tgsi_setup.c  |  16 +-
  src/gallium/drivers/svga/svga_screen.c   |   3 +
  src/gallium/drivers/vc4/vc4_screen.c |   1 +
  src/gallium/include/pipe/p_defines.h |   1 +
  src/gallium/include/pipe/p_shader_tokens.h   |   2 +-
  src/mesa/state_tracker/st_glsl_to_tgsi.cpp   |  10 +-
  33 files changed, 290 insertions(+), 127 deletions(-)




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/3] amd/common: remove ac_shader_abi::chip_class

2017-09-26 Thread Nicolai Hähnle
From: Nicolai Hähnle 

Redundant with the recently added ac_llvm_context::chip_class.
---
 src/amd/common/ac_nir_to_llvm.c  | 21 ++---
 src/amd/common/ac_shader_abi.h   |  2 --
 src/gallium/drivers/radeonsi/si_shader.c |  2 --
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 217d1e67ae2..d7b6259fe8f 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -1926,21 +1926,21 @@ static LLVMValueRef cast_ptr(struct nir_to_llvm_context 
*ctx, LLVMValueRef ptr,
 }
 
 static LLVMValueRef
 get_buffer_size(struct ac_nir_context *ctx, LLVMValueRef descriptor, bool 
in_elements)
 {
LLVMValueRef size =
LLVMBuildExtractElement(ctx->ac.builder, descriptor,
LLVMConstInt(ctx->ac.i32, 2, false), 
"");
 
/* VI only */
-   if (ctx->abi->chip_class == VI && in_elements) {
+   if (ctx->ac.chip_class == VI && in_elements) {
/* On VI, the descriptor contains the size in bytes,
 * but TXQ must return the size in elements.
 * The stride is always non-zero for resources using TXQ.
 */
LLVMValueRef stride =
LLVMBuildExtractElement(ctx->ac.builder, descriptor,
LLVMConstInt(ctx->ac.i32, 1, 
false), "");
stride = LLVMBuildLShr(ctx->ac.builder, stride,
   LLVMConstInt(ctx->ac.i32, 16, false), 
"");
stride = LLVMBuildAnd(ctx->ac.builder, stride,
@@ -2132,21 +2132,21 @@ static LLVMValueRef build_tex_intrinsic(struct 
ac_nir_context *ctx,
break;
case nir_texop_lod:
args->opcode = ac_image_get_lod;
args->compare = false;
args->offset = false;
break;
default:
break;
}
 
-   if (instr->op == nir_texop_tg4 && ctx->abi->chip_class <= VI) {
+   if (instr->op == nir_texop_tg4 && ctx->ac.chip_class <= VI) {
enum glsl_base_type stype = 
glsl_get_sampler_result_type(instr->texture->var->type);
if (stype == GLSL_TYPE_UINT || stype == GLSL_TYPE_INT) {
return radv_lower_gather4_integer(&ctx->ac, args, 
instr);
}
}
return ac_build_image_opcode(&ctx->ac, args);
 }
 
 static LLVMValueRef visit_vulkan_resource_index(struct nir_to_llvm_context 
*ctx,
 nir_intrinsic_instr *instr)
@@ -3259,21 +3259,21 @@ static LLVMValueRef get_image_coords(struct 
ac_nir_context *ctx,
LLVMValueRef res;
LLVMValueRef sample_index = llvm_extract_elem(&ctx->ac, get_src(ctx, 
instr->src[1]), 0);
 
int count;
enum glsl_sampler_dim dim = glsl_get_sampler_dim(type);
bool is_array = glsl_sampler_type_is_array(type);
bool add_frag_pos = (dim == GLSL_SAMPLER_DIM_SUBPASS ||
 dim == GLSL_SAMPLER_DIM_SUBPASS_MS);
bool is_ms = (dim == GLSL_SAMPLER_DIM_MS ||
  dim == GLSL_SAMPLER_DIM_SUBPASS_MS);
-   bool gfx9_1d = ctx->abi->chip_class >= GFX9 && dim == 
GLSL_SAMPLER_DIM_1D;
+   bool gfx9_1d = ctx->ac.chip_class >= GFX9 && dim == GLSL_SAMPLER_DIM_1D;
count = image_type_to_components_count(dim, is_array);
 
if (is_ms) {
LLVMValueRef fmask_load_address[3];
int chan;
 
fmask_load_address[0] = 
LLVMBuildExtractElement(ctx->ac.builder, src0, masks[0], "");
fmask_load_address[1] = 
LLVMBuildExtractElement(ctx->ac.builder, src0, masks[1], "");
if (is_array)
fmask_load_address[2] = 
LLVMBuildExtractElement(ctx->ac.builder, src0, masks[2], "");
@@ -3404,21 +3404,21 @@ static LLVMValueRef visit_image_load(struct 
ac_nir_context *ctx,
 static void visit_image_store(struct ac_nir_context *ctx,
  nir_intrinsic_instr *instr)
 {
LLVMValueRef params[8];
char intrinsic_name[64];
const nir_variable *var = instr->variables[0]->var;
const struct glsl_type *type = glsl_without_array(var->type);
LLVMValueRef i1false = LLVMConstInt(ctx->ac.i1, 0, false);
LLVMValueRef i1true = LLVMConstInt(ctx->ac.i1, 1, false);
LLVMValueRef glc = i1false;
-   bool force_glc = ctx->abi->chip_class == SI;
+   bool force_glc = ctx->ac.chip_class == SI;
if (force_glc)
glc = i1true;
 
if (glsl_get_sampler_dim(type) == GLSL_SAMPLER_DIM_BUF) {
params[0] = ac_to_float(&ctx->ac, get_src(ctx, instr->src[2])); 
/* data */
params[1] = get_sampler_desc(ctx, instr->variables[0], 
AC_DESC_BUFFER, true, true);
params[2] = LLVMBuildExtractElement(ctx

[Mesa-dev] [PATCH 3/3] radeonsi: move descriptor logs to after corresponding draw/compute packet

2017-09-26 Thread Nicolai Hähnle
From: Nicolai Hähnle 

It has to happen after descriptor uploads since otherwise we'll print out
the wrong GPU list / incorrectly claim descriptor corruption.
---
 src/gallium/drivers/radeonsi/si_compute.c| 7 +++
 src/gallium/drivers/radeonsi/si_state_draw.c | 7 +++
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_compute.c 
b/src/gallium/drivers/radeonsi/si_compute.c
index ca334949d77..2346e2e95bc 100644
--- a/src/gallium/drivers/radeonsi/si_compute.c
+++ b/src/gallium/drivers/radeonsi/si_compute.c
@@ -799,23 +799,20 @@ static void si_launch_grid(
/* Indirect buffers use TC L2 on GFX9, but not older hw. */
if (sctx->b.chip_class <= VI &&
r600_resource(info->indirect)->TC_L2_dirty) {
sctx->b.flags |= SI_CONTEXT_WRITEBACK_GLOBAL_L2;
r600_resource(info->indirect)->TC_L2_dirty = false;
}
}
 
si_need_cs_space(sctx);
 
-   if (sctx->b.log)
-   si_log_compute_state(sctx, sctx->b.log);
-
if (!sctx->cs_shader_state.initialized)
si_initialize_compute(sctx);
 
if (sctx->b.flags)
si_emit_cache_flush(sctx);
 
if (!si_switch_compute_shader(sctx, program, &program->shader,
code_object, info->pc))
return;
 
@@ -844,22 +841,24 @@ static void si_launch_grid(
radeon_add_to_buffer_list(&sctx->b, &sctx->b.gfx, buffer,
  RADEON_USAGE_READWRITE,
  RADEON_PRIO_COMPUTE_GLOBAL);
}
 
if (program->ir_type == PIPE_SHADER_IR_TGSI)
si_setup_tgsi_grid(sctx, info);
 
si_emit_dispatch_packets(sctx, info);
 
-   if (unlikely(sctx->current_saved_cs))
+   if (unlikely(sctx->current_saved_cs)) {
si_trace_emit(sctx);
+   si_log_compute_state(sctx, sctx->b.log);
+   }
 
sctx->compute_is_busy = true;
sctx->b.num_compute_calls++;
if (sctx->cs_shader_state.uses_scratch)
sctx->b.num_spill_compute_calls++;
 
if (cs_regalloc_hang)
sctx->b.flags |= SI_CONTEXT_CS_PARTIAL_FLUSH;
 }
 
diff --git a/src/gallium/drivers/radeonsi/si_state_draw.c 
b/src/gallium/drivers/radeonsi/si_state_draw.c
index 05ed85475bf..e29d716580a 100644
--- a/src/gallium/drivers/radeonsi/si_state_draw.c
+++ b/src/gallium/drivers/radeonsi/si_state_draw.c
@@ -1369,23 +1369,20 @@ void si_draw_vbo(struct pipe_context *ctx, const struct 
pipe_draw_info *info)
if (indirect->indirect_draw_count &&

r600_resource(indirect->indirect_draw_count)->TC_L2_dirty) {
sctx->b.flags |= SI_CONTEXT_WRITEBACK_GLOBAL_L2;

r600_resource(indirect->indirect_draw_count)->TC_L2_dirty = false;
}
}
}
 
si_need_cs_space(sctx);
 
-   if (unlikely(sctx->b.log))
-   si_log_draw_state(sctx, sctx->b.log);
-
/* Since we've called r600_context_add_resource_size for vertex buffers,
 * this must be called after si_need_cs_space, because we must let
 * need_cs_space flush before we add buffers to the buffer list.
 */
if (!si_upload_vertex_buffer_descriptors(sctx))
return;
 
/* GFX9 scissor bug workaround. This must be done before VPORT scissor
 * registers are changed. There is also a more efficient but more
 * involved alternative workaround.
@@ -1447,22 +1444,24 @@ void si_draw_vbo(struct pipe_context *ctx, const struct 
pipe_draw_info *info)
if (sctx->b.chip_class >= CIK && sctx->prefetch_L2_mask)
cik_emit_prefetch_L2(sctx);
 
if (!si_upload_graphics_shader_descriptors(sctx))
return;
 
si_emit_all_states(sctx, info, 0);
si_emit_draw_packets(sctx, info, indexbuf, index_size, 
index_offset);
}
 
-   if (unlikely(sctx->current_saved_cs))
+   if (unlikely(sctx->current_saved_cs)) {
si_trace_emit(sctx);
+   si_log_draw_state(sctx, sctx->b.log);
+   }
 
/* Workaround for a VGT hang when streamout is enabled.
 * It must be done after drawing. */
if ((sctx->b.family == CHIP_HAWAII ||
 sctx->b.family == CHIP_TONGA ||
 sctx->b.family == CHIP_FIJI) &&
r600_get_strmout_en(&sctx->b)) {
sctx->b.flags |= SI_CONTEXT_VGT_STREAMOUT_SYNC;
}
 
-- 
2.11.0

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


[Mesa-dev] [PATCH 1/3] gallium/radeon: fix a comment

2017-09-26 Thread Nicolai Hähnle
From: Nicolai Hähnle 

---
 src/gallium/drivers/radeon/cayman_msaa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeon/cayman_msaa.c 
b/src/gallium/drivers/radeon/cayman_msaa.c
index 33f1040185a..ec6c49bafa0 100644
--- a/src/gallium/drivers/radeon/cayman_msaa.c
+++ b/src/gallium/drivers/radeon/cayman_msaa.c
@@ -29,21 +29,21 @@
 /* 2xMSAA
  * There are two locations (4, 4), (-4, -4). */
 const uint32_t eg_sample_locs_2x[4] = {
FILL_SREG(4, 4, -4, -4, 4, 4, -4, -4),
FILL_SREG(4, 4, -4, -4, 4, 4, -4, -4),
FILL_SREG(4, 4, -4, -4, 4, 4, -4, -4),
FILL_SREG(4, 4, -4, -4, 4, 4, -4, -4),
 };
 const unsigned eg_max_dist_2x = 4;
 /* 4xMSAA
- * There are 4 locations: (-2, 6), (6, -2), (-6, 2), (2, 6). */
+ * There are 4 locations: (-2, -6), (6, -2), (-6, 2), (2, 6). */
 const uint32_t eg_sample_locs_4x[4] = {
FILL_SREG(-2, -6, 6, -2, -6, 2, 2, 6),
FILL_SREG(-2, -6, 6, -2, -6, 2, 2, 6),
FILL_SREG(-2, -6, 6, -2, -6, 2, 2, 6),
FILL_SREG(-2, -6, 6, -2, -6, 2, 2, 6),
 };
 const unsigned eg_max_dist_4x = 6;
 
 /* Cayman 8xMSAA */
 static const uint32_t cm_sample_locs_8x[] = {
-- 
2.11.0

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


[Mesa-dev] [PATCH 0/5] st/mesa,radeonsi: various compiler fixes

2017-09-26 Thread Nicolai Hähnle
Hi all,

There isn't much of a theme to this series, except that all those
patches are vaguely compiler-related.

Please review!

Thanks,
Nicolai
--
 src/amd/common/ac_llvm_build.c| 17 +
 .../drivers/radeonsi/si_state_shaders.c   |  8 +---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp| 13 +++--
 3 files changed, 25 insertions(+), 13 deletions(-)

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


[Mesa-dev] [PATCH 4/5] amd/common: save an instruction in the build_cube_select sequence

2017-09-26 Thread Nicolai Hähnle
From: Nicolai Hähnle 

Avoid a v_cndmask: the absolute value is free due to input modifiers.
---
 src/amd/common/ac_llvm_build.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c
index 8c050f31a76..71468df2dbc 100644
--- a/src/amd/common/ac_llvm_build.c
+++ b/src/amd/common/ac_llvm_build.c
@@ -431,26 +431,27 @@ build_cube_intrinsic(struct ac_llvm_context *ctx,
 
 /**
  * Build a manual selection sequence for cube face sc/tc coordinates and
  * major axis vector (multiplied by 2 for consistency) for the given
  * vec3 \p coords, for the face implied by \p selcoords.
  *
  * For the major axis, we always adjust the sign to be in the direction of
  * selcoords.ma; i.e., a positive out_ma means that coords is pointed towards
  * the selcoords major axis.
  */
-static void build_cube_select(LLVMBuilderRef builder,
+static void build_cube_select(struct ac_llvm_context *ctx,
  const struct cube_selection_coords *selcoords,
  const LLVMValueRef *coords,
  LLVMValueRef *out_st,
  LLVMValueRef *out_ma)
 {
+   LLVMBuilderRef builder = ctx->builder;
LLVMTypeRef f32 = LLVMTypeOf(coords[0]);
LLVMValueRef is_ma_positive;
LLVMValueRef sgn_ma;
LLVMValueRef is_ma_z, is_not_ma_z;
LLVMValueRef is_ma_y;
LLVMValueRef is_ma_x;
LLVMValueRef sgn;
LLVMValueRef tmp;
 
is_ma_positive = LLVMBuildFCmp(builder, LLVMRealUGE,
@@ -473,23 +474,23 @@ static void build_cube_select(LLVMBuilderRef builder,
 
/* Select tc */
tmp = LLVMBuildSelect(builder, is_ma_y, coords[2], coords[1], "");
sgn = LLVMBuildSelect(builder, is_ma_y, sgn_ma,
LLVMConstReal(f32, -1.0), "");
out_st[1] = LLVMBuildFMul(builder, tmp, sgn, "");
 
/* Select ma */
tmp = LLVMBuildSelect(builder, is_ma_z, coords[2],
LLVMBuildSelect(builder, is_ma_y, coords[1], coords[0], ""), 
"");
-   sgn = LLVMBuildSelect(builder, is_ma_positive,
-   LLVMConstReal(f32, 2.0), LLVMConstReal(f32, -2.0), "");
-   *out_ma = LLVMBuildFMul(builder, tmp, sgn, "");
+   tmp = ac_build_intrinsic(ctx, "llvm.fabs.f32",
+ctx->f32, &tmp, 1, AC_FUNC_ATTR_READNONE);
+   *out_ma = LLVMBuildFMul(builder, tmp, LLVMConstReal(f32, 2.0), "");
 }
 
 void
 ac_prepare_cube_coords(struct ac_llvm_context *ctx,
   bool is_deriv, bool is_array, bool is_lod,
   LLVMValueRef *coords_arg,
   LLVMValueRef *derivs_arg)
 {
 
LLVMBuilderRef builder = ctx->builder;
@@ -563,21 +564,21 @@ ac_prepare_cube_coords(struct ac_llvm_context *ctx,
 *= 1/z * dx/dh - x/z * 1/z * dz/dh.
 *
 * This motivatives the implementation below.
 *
 * Whether this actually gives the expected results for
 * apps that might feed in derivatives obtained via
 * finite differences is anyone's guess. The OpenGL spec
 * seems awfully quiet about how textureGrad for cube
 * maps should be handled.
 */
-   build_cube_select(builder, &selcoords, &derivs_arg[axis 
* 3],
+   build_cube_select(ctx, &selcoords, &derivs_arg[axis * 
3],
  deriv_st, &deriv_ma);
 
deriv_ma = LLVMBuildFMul(builder, deriv_ma, invma, "");
 
for (int i = 0; i < 2; ++i)
derivs[axis * 2 + i] =
LLVMBuildFSub(builder,
LLVMBuildFMul(builder, 
deriv_st[i], invma, ""),
LLVMBuildFMul(builder, 
deriv_ma, coords[i], ""), "");
}
-- 
2.11.0

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


[Mesa-dev] [PATCH 2/5] st/glsl_to_tgsi: fix conditional assignments to packed shader outputs

2017-09-26 Thread Nicolai Hähnle
From: Nicolai Hähnle 

Overriding the default (no-op) swizzle is clearly counter-productive,
since the whole point is putting the destination register as one of
the source operands so that it remains unmodified when the assignment
condition is false.

Fragment depth and stencil outputs are a special case due to how their
source swizzles are manipulated in translate_src when compiling to
TGSI.

Fixes dEQP-GLES2.functional.shaders.conditionals.if.*_vertex
Cc: mesa-sta...@lists.freedesktop.org
---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index f4870a1c606..0daf5a14285 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -2890,21 +2890,29 @@ glsl_to_tgsi_visitor::emit_block_mov(ir_assignment *ir, 
const struct glsl_type *
   }
   return;
}
 
assert(type->is_scalar() || type->is_vector());
 
l->type = type->base_type;
r->type = type->base_type;
if (cond) {
   st_src_reg l_src = st_src_reg(*l);
-  l_src.swizzle = swizzle_for_size(type->vector_elements);
+
+  if (l_src.file == PROGRAM_OUTPUT &&
+  this->prog->Target == GL_FRAGMENT_PROGRAM_ARB &&
+  (l_src.index == FRAG_RESULT_DEPTH || l_src.index == 
FRAG_RESULT_STENCIL)) {
+ /* This is a special case because the source swizzles will be shifted
+  * later to account for the difference between GLSL (where they're
+  * plain floats) and TGSI (where they're Z and Y components). */
+ l_src.swizzle = SWIZZLE_;
+  }
 
   if (native_integers) {
  emit_asm(ir, TGSI_OPCODE_UCMP, *l, *cond,
   cond_swap ? l_src : *r,
   cond_swap ? *r : l_src);
   } else {
  emit_asm(ir, TGSI_OPCODE_CMP, *l, *cond,
   cond_swap ? l_src : *r,
   cond_swap ? *r : l_src);
   }
-- 
2.11.0

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


[Mesa-dev] [PATCH 1/5] st/glsl_to_tgsi: fix a use-after-free in merge_two_dsts

2017-09-26 Thread Nicolai Hähnle
From: Nicolai Hähnle 

Found by address sanitizer.

The loop here tries to be safe, but in doing so, it ends up doing
exactly the wrong thing: the safe foreach is for when the loop
variable (inst) could be deleted and nothing else. However, this
particular can delete inst's successor, but not inst itself.

Fixes: 8c6a0ebaad72 ("st/mesa: add st fp64 support (v7.1)")
---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 609920a7a87..f4870a1c606 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -5141,21 +5141,22 @@ glsl_to_tgsi_visitor::eliminate_dead_code(void)
ralloc_free(write_level);
ralloc_free(writes);
 
return removed;
 }
 
 /* merge DFRACEXP instructions into one. */
 void
 glsl_to_tgsi_visitor::merge_two_dsts(void)
 {
-   foreach_in_list_safe(glsl_to_tgsi_instruction, inst, &this->instructions) {
+   /* We never delete inst, but we may delete its successor. */
+   foreach_in_list(glsl_to_tgsi_instruction, inst, &this->instructions) {
   glsl_to_tgsi_instruction *inst2;
   bool merged;
   if (num_inst_dst_regs(inst) != 2)
  continue;
 
   if (inst->dst[0].file != PROGRAM_UNDEFINED &&
   inst->dst[1].file != PROGRAM_UNDEFINED)
  continue;
 
   inst2 = (glsl_to_tgsi_instruction *) inst->next;
-- 
2.11.0

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


[Mesa-dev] [PATCH 5/5] radeonsi/gfx9: fix geometry shaders without output vertices

2017-09-26 Thread Nicolai Hähnle
From: Nicolai Hähnle 

Not that those are super common or useful, but hey! Fun corner cases
of the API...

Fixes dEQP-GLES31.functional.geometry_shading.emit.*

Cc: mesa-sta...@lists.freedesktop.org
---
 src/gallium/drivers/radeonsi/si_state_shaders.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c 
b/src/gallium/drivers/radeonsi/si_state_shaders.c
index 53a60ba11ed..985ee130e6d 100644
--- a/src/gallium/drivers/radeonsi/si_state_shaders.c
+++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
@@ -637,23 +637,25 @@ static void gfx9_get_gs_info(struct si_shader_selector 
*es,
assert(gs_num_invocations <= 32); /* GL maximum */
 
if (uses_adjacency || gs_num_invocations > 1)
max_gs_prims = 127 / gs_num_invocations;
else
max_gs_prims = 255;
 
/* MAX_PRIMS_PER_SUBGROUP = gs_prims * max_vert_out * gs_invocations.
 * Make sure we don't go over the maximum value.
 */
-   max_gs_prims = MIN2(max_gs_prims,
-   max_out_prims /
-   (gs->gs_max_out_vertices * gs_num_invocations));
+   if (gs->gs_max_out_vertices > 0) {
+   max_gs_prims = MIN2(max_gs_prims,
+   max_out_prims /
+   (gs->gs_max_out_vertices * 
gs_num_invocations));
+   }
assert(max_gs_prims > 0);
 
/* If the primitive has adjacency, halve the number of vertices
 * that will be reused in multiple primitives.
 */
min_es_verts = gs->gs_input_verts_per_prim / (uses_adjacency ? 2 : 1);
 
gs_prims = MIN2(ideal_gs_prims, max_gs_prims);
worst_case_es_verts = MIN2(min_es_verts * gs_prims, max_es_verts);
 
-- 
2.11.0

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


[Mesa-dev] [PATCH 3/5] amd/common: fix build_cube_select

2017-09-26 Thread Nicolai Hähnle
From: Nicolai Hähnle 

Fix the custom cube coord selection sequence to be identical to
the hardware v_cubesc/tc and OpenGL spec. Affects texture sampling
with user-provided derivatives.

Fixes dEQP-GLES3.functional.shaders.texture_functions.texturegrad.*

Cc: mesa-sta...@lists.freedesktop.org
---
 src/amd/common/ac_llvm_build.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c
index 8a329515b57..8c050f31a76 100644
--- a/src/amd/common/ac_llvm_build.c
+++ b/src/amd/common/ac_llvm_build.c
@@ -458,29 +458,29 @@ static void build_cube_select(LLVMBuilderRef builder,
sgn_ma = LLVMBuildSelect(builder, is_ma_positive,
LLVMConstReal(f32, 1.0), LLVMConstReal(f32, -1.0), "");
 
is_ma_z = LLVMBuildFCmp(builder, LLVMRealUGE, selcoords->id, 
LLVMConstReal(f32, 4.0), "");
is_not_ma_z = LLVMBuildNot(builder, is_ma_z, "");
is_ma_y = LLVMBuildAnd(builder, is_not_ma_z,
LLVMBuildFCmp(builder, LLVMRealUGE, selcoords->id, 
LLVMConstReal(f32, 2.0), ""), "");
is_ma_x = LLVMBuildAnd(builder, is_not_ma_z, LLVMBuildNot(builder, 
is_ma_y, ""), "");
 
/* Select sc */
-   tmp = LLVMBuildSelect(builder, is_ma_z, coords[2], coords[0], "");
+   tmp = LLVMBuildSelect(builder, is_ma_x, coords[2], coords[0], "");
sgn = LLVMBuildSelect(builder, is_ma_y, LLVMConstReal(f32, 1.0),
-   LLVMBuildSelect(builder, is_ma_x, sgn_ma,
+   LLVMBuildSelect(builder, is_ma_z, sgn_ma,
LLVMBuildFNeg(builder, sgn_ma, ""), ""), "");
out_st[0] = LLVMBuildFMul(builder, tmp, sgn, "");
 
/* Select tc */
tmp = LLVMBuildSelect(builder, is_ma_y, coords[2], coords[1], "");
-   sgn = LLVMBuildSelect(builder, is_ma_y, LLVMBuildFNeg(builder, sgn_ma, 
""),
+   sgn = LLVMBuildSelect(builder, is_ma_y, sgn_ma,
LLVMConstReal(f32, -1.0), "");
out_st[1] = LLVMBuildFMul(builder, tmp, sgn, "");
 
/* Select ma */
tmp = LLVMBuildSelect(builder, is_ma_z, coords[2],
LLVMBuildSelect(builder, is_ma_y, coords[1], coords[0], ""), 
"");
sgn = LLVMBuildSelect(builder, is_ma_positive,
LLVMConstReal(f32, 2.0), LLVMConstReal(f32, -2.0), "");
*out_ma = LLVMBuildFMul(builder, tmp, sgn, "");
 }
-- 
2.11.0

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


[Mesa-dev] [PATCH 3/3] radeonsi: fix border color translation for integer textures

2017-09-26 Thread Nicolai Hähnle
From: Nicolai Hähnle 

This fixes the extremely unlikely case that an application uses
0x8000 or 0x3f80 as border color for an integer texture and
helps in the also, but perhaps slightly less, unlikely case that 1 is
used as a border color.
---
 src/gallium/drivers/radeonsi/si_descriptors.c | 37 
 src/gallium/drivers/radeonsi/si_pipe.h|  2 ++
 src/gallium/drivers/radeonsi/si_state.c   | 50 +++
 3 files changed, 60 insertions(+), 29 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
b/src/gallium/drivers/radeonsi/si_descriptors.c
index 27239c25389..db77b1cb982 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -372,20 +372,34 @@ void si_set_mutable_tex_desc_fields(struct si_screen 
*sscreen,
unsigned pitch = base_level_info->nblk_x * block_width;
unsigned index = si_tile_mode_index(tex, base_level, 
is_stencil);
 
state[3] &= C_008F1C_TILING_INDEX;
state[3] |= S_008F1C_TILING_INDEX(index);
state[4] &= C_008F20_PITCH_GFX6;
state[4] |= S_008F20_PITCH_GFX6(pitch - 1);
}
 }
 
+static void si_set_sampler_state_desc(struct si_sampler_state *sstate,
+ struct si_sampler_view *sview,
+ struct r600_texture *tex,
+ uint32_t *desc)
+{
+   if (sview && sview->is_integer)
+   memcpy(desc, sstate->integer_val, 4*4);
+   else if (tex && tex->upgraded_depth &&
+(!sview || !sview->is_stencil_sampler))
+   memcpy(desc, sstate->upgraded_depth_val, 4*4);
+   else
+   memcpy(desc, sstate->val, 4*4);
+}
+
 static void si_set_sampler_view_desc(struct si_context *sctx,
 struct si_sampler_view *sview,
 struct si_sampler_state *sstate,
 uint32_t *desc)
 {
struct pipe_sampler_view *view = &sview->base;
struct r600_texture *rtex = (struct r600_texture *)view->texture;
bool is_buffer = rtex->resource.b.b.target == PIPE_BUFFER;
 
if (unlikely(!is_buffer && sview->dcc_incompatible)) {
@@ -415,27 +429,24 @@ static void si_set_sampler_view_desc(struct si_context 
*sctx,
   is_separate_stencil,
   desc);
}
 
if (!is_buffer && rtex->fmask.size) {
memcpy(desc + 8, sview->fmask_state, 8*4);
} else {
/* Disable FMASK and bind sampler state in [12:15]. */
memcpy(desc + 8, null_texture_descriptor, 4*4);
 
-   if (sstate) {
-   if (!is_buffer && rtex->upgraded_depth &&
-   !sview->is_stencil_sampler)
-   memcpy(desc + 12, sstate->upgraded_depth_val, 
4*4);
-   else
-   memcpy(desc + 12, sstate->val, 4*4);
-   }
+   if (sstate)
+   si_set_sampler_state_desc(sstate, sview,
+ is_buffer ? NULL : rtex,
+ desc + 12);
}
 }
 
 static void si_set_sampler_view(struct si_context *sctx,
unsigned shader,
unsigned slot, struct pipe_sampler_view *view,
bool disallow_early_out)
 {
struct si_sampler_views *views = &sctx->samplers[shader].views;
struct si_sampler_view *rview = (struct si_sampler_view*)view;
@@ -463,22 +474,22 @@ static void si_set_sampler_view(struct si_context *sctx,
si_sampler_view_add_buffer(sctx, view->texture,
   RADEON_USAGE_READ,
   rview->is_stencil_sampler, true);
} else {
pipe_sampler_view_reference(&views->views[slot], NULL);
memcpy(desc, null_texture_descriptor, 8*4);
/* Only clear the lower dwords of FMASK. */
memcpy(desc + 8, null_texture_descriptor, 4*4);
/* Re-set the sampler state if we are transitioning from FMASK. 
*/
if (views->sampler_states[slot])
-   memcpy(desc + 12,
-  views->sampler_states[slot]->val, 4*4);
+   si_set_sampler_state_desc(views->sampler_states[slot], 
NULL, NULL,
+ desc + 12);
 
views->enabled_mask &= ~(1u << slot);
}
 
sctx->descriptors_dirty |= 1u << 
si_sampler_and_image_descriptors_idx(shader);
 }
 
 static bool color_needs_decompression(struct r600_texture *rtex)
 {

[Mesa-dev] [PATCH 2/3] radeonsi: clamp border colors for upgraded depth textures

2017-09-26 Thread Nicolai Hähnle
From: Nicolai Hähnle 

The hardware does this automatically for unorm formats, but we need to
do it manually for unorm depth formats that have been upgraded to
Z32_FLOAT.

Fixes 
dEQP-GLES31.functional.texture.border_clamp.range_clamp.nearest_unorm_depth
and others.

Fixes: d4d9ec55c589 ("radeonsi: implement TC-compatible HTILE")
---
 src/gallium/drivers/radeonsi/si_state.c | 119 
 1 file changed, 60 insertions(+), 59 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c 
b/src/gallium/drivers/radeonsi/si_state.c
index 551bb17503c..2a27ea0c503 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -3888,96 +3888,85 @@ static void si_sampler_view_destroy(struct pipe_context 
*ctx,
 
 static bool wrap_mode_uses_border_color(unsigned wrap, bool linear_filter)
 {
return wrap == PIPE_TEX_WRAP_CLAMP_TO_BORDER ||
   wrap == PIPE_TEX_WRAP_MIRROR_CLAMP_TO_BORDER ||
   (linear_filter &&
(wrap == PIPE_TEX_WRAP_CLAMP ||
 wrap == PIPE_TEX_WRAP_MIRROR_CLAMP));
 }
 
-static bool sampler_state_needs_border_color(const struct pipe_sampler_state 
*state)
+static uint32_t si_translate_border_color(struct si_context *sctx,
+ const struct pipe_sampler_state 
*state,
+ const union pipe_color_union *color)
 {
bool linear_filter = state->min_img_filter != PIPE_TEX_FILTER_NEAREST ||
 state->mag_img_filter != PIPE_TEX_FILTER_NEAREST;
 
-   return (state->border_color.ui[0] || state->border_color.ui[1] ||
-   state->border_color.ui[2] || state->border_color.ui[3]) &&
-  (wrap_mode_uses_border_color(state->wrap_s, linear_filter) ||
-   wrap_mode_uses_border_color(state->wrap_t, linear_filter) ||
-   wrap_mode_uses_border_color(state->wrap_r, linear_filter));
+   if ((color->f[0] == 0 && color->f[1] == 0 &&
+color->f[2] == 0 && color->f[3] == 0) ||
+   (!wrap_mode_uses_border_color(state->wrap_s, linear_filter) &&
+!wrap_mode_uses_border_color(state->wrap_t, linear_filter) &&
+!wrap_mode_uses_border_color(state->wrap_r, linear_filter)))
+   return 
S_008F3C_BORDER_COLOR_TYPE(V_008F3C_SQ_TEX_BORDER_COLOR_TRANS_BLACK);
+
+   if (color->f[0] == 0 && color->f[1] == 0 &&
+   color->f[2] == 0 && color->f[3] == 1)
+   return 
S_008F3C_BORDER_COLOR_TYPE(V_008F3C_SQ_TEX_BORDER_COLOR_OPAQUE_BLACK);
+   if (color->f[0] == 1 && color->f[1] == 1 &&
+   color->f[2] == 1 && color->f[3] == 1)
+   return 
S_008F3C_BORDER_COLOR_TYPE(V_008F3C_SQ_TEX_BORDER_COLOR_OPAQUE_WHITE);
+
+   int i;
+
+   /* Check if the border has been uploaded already. */
+   for (i = 0; i < sctx->border_color_count; i++)
+   if (memcmp(&sctx->border_color_table[i], color,
+  sizeof(*color)) == 0)
+   break;
+
+   if (i >= SI_MAX_BORDER_COLORS) {
+   /* Getting 4096 unique border colors is very unlikely. */
+   fprintf(stderr, "radeonsi: The border color table is full. "
+   "Any new border colors will be just black. "
+   "Please file a bug.\n");
+   return 
S_008F3C_BORDER_COLOR_TYPE(V_008F3C_SQ_TEX_BORDER_COLOR_TRANS_BLACK);
+   }
+
+   if (i == sctx->border_color_count) {
+   /* Upload a new border color. */
+   memcpy(&sctx->border_color_table[i], color,
+  sizeof(*color));
+   util_memcpy_cpu_to_le32(&sctx->border_color_map[i],
+   color, sizeof(*color));
+   sctx->border_color_count++;
+   }
+
+   return S_008F3C_BORDER_COLOR_PTR(i) |
+  
S_008F3C_BORDER_COLOR_TYPE(V_008F3C_SQ_TEX_BORDER_COLOR_REGISTER);
 }
 
 static void *si_create_sampler_state(struct pipe_context *ctx,
 const struct pipe_sampler_state *state)
 {
struct si_context *sctx = (struct si_context *)ctx;
struct r600_common_screen *rscreen = sctx->b.screen;
struct si_sampler_state *rstate = CALLOC_STRUCT(si_sampler_state);
-   unsigned border_color_type, border_color_index = 0;
unsigned max_aniso = rscreen->force_aniso >= 0 ? rscreen->force_aniso
   : state->max_anisotropy;
unsigned max_aniso_ratio = r600_tex_aniso_filter(max_aniso);
+   union pipe_color_union clamped_border_color;
 
if (!rstate) {
return NULL;
}
 
-   if (!sampler_state_needs_border_color(state))
-   border_color_type = V_008F3C_SQ_TEX_BORDER_COLOR_TRANS_BLACK;
-   else if (state->border_color.f[0] == 0 &&
-state->border_color.f[1] == 0 &&
-   

[Mesa-dev] [PATCH 0/3] radeonsi: depth texture clamping and border color fixes

2017-09-26 Thread Nicolai Hähnle
Hi all,

radeonsi can transparently upgrade unorm depth textures to float,
which means that some of the depth value clamping that is usually
performed in hardware must be done manually.

Also, setting the border color for integer textures properly
requires doing it slightly differently. This is something that
I only noticed upon inspection.

Please review!

Thanks,
Nicolai
--
 src/amd/common/sid.h |   2 +
 .../drivers/radeon/r600_pipe_common.h|   1 +
 src/gallium/drivers/radeon/r600_texture.c|   5 +-
 .../drivers/radeonsi/si_descriptors.c|  40 -
 src/gallium/drivers/radeonsi/si_pipe.h   |   3 +
 .../drivers/radeonsi/si_shader_tgsi_mem.c|  25 ++-
 src/gallium/drivers/radeonsi/si_state.c  | 143 ++---
 7 files changed, 146 insertions(+), 73 deletions(-)

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


[Mesa-dev] [PATCH 1/3] radeonsi: clamp depth comparison value only for fixed point formats

2017-09-26 Thread Nicolai Hähnle
From: Nicolai Hähnle 

The hardware usually does this automatically. However, we upgrade
depth to Z32_FLOAT to enable TC-compatible HTILE, which means the
hardware no longer clamps the comparison value for us.

The only way to tell in the shader whether a clamp is required
seems to be to communicate an additional bit in the descriptor
table. While VI has some unused bits in the resource descriptor,
those bits have unfortunately all been used in gfx9. So we use
an unused bit in the sampler state instead.

Fixes dEQP-GLES3.functional.texture.shadow.2d.linear.equal_depth_component32f
and many other tests in dEQP-GLES3.functional.texture.shadow.*

Fixes: d4d9ec55c589 ("radeonsi: implement TC-compatible HTILE")
---
 src/amd/common/sid.h  |  2 ++
 src/gallium/drivers/radeon/r600_pipe_common.h |  1 +
 src/gallium/drivers/radeon/r600_texture.c |  5 +++-
 src/gallium/drivers/radeonsi/si_descriptors.c | 31 ++-
 src/gallium/drivers/radeonsi/si_pipe.h|  1 +
 src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c | 25 +-
 src/gallium/drivers/radeonsi/si_state.c   |  4 +++
 7 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/src/amd/common/sid.h b/src/amd/common/sid.h
index a8c78c1b2bf..1016f674707 100644
--- a/src/amd/common/sid.h
+++ b/src/amd/common/sid.h
@@ -2446,20 +2446,22 @@
 #define   S_008F38_FILTER_PREC_FIX(x) 
(((unsigned)(x) & 0x1) << 30)
 #define   G_008F38_FILTER_PREC_FIX(x) (((x) >> 
30) & 0x1)
 #define   C_008F38_FILTER_PREC_FIX
0xBFFF
 #define   S_008F38_ANISO_OVERRIDE(x)  
(((unsigned)(x) & 0x1) << 31)
 #define   G_008F38_ANISO_OVERRIDE(x)  (((x) >> 
31) & 0x1)
 #define   C_008F38_ANISO_OVERRIDE 
0x7FFF
 #define R_008F3C_SQ_IMG_SAMP_WORD3  
0x008F3C
 #define   S_008F3C_BORDER_COLOR_PTR(x)
(((unsigned)(x) & 0xFFF) << 0)
 #define   G_008F3C_BORDER_COLOR_PTR(x)(((x) >> 
0) & 0xFFF)
 #define   C_008F3C_BORDER_COLOR_PTR   
0xF000
+/* The UPGRADED_DEPTH field is driver-specific and does not exist in hardware. 
*/
+#define   S_008F3C_UPGRADED_DEPTH(x)  
(((unsigned)(x) & 0x1) << 29)
 #define   S_008F3C_BORDER_COLOR_TYPE(x)   
(((unsigned)(x) & 0x03) << 30)
 #define   G_008F3C_BORDER_COLOR_TYPE(x)   (((x) >> 
30) & 0x03)
 #define   C_008F3C_BORDER_COLOR_TYPE  
0x3FFF
 #define V_008F3C_SQ_TEX_BORDER_COLOR_TRANS_BLACK0x00
 #define V_008F3C_SQ_TEX_BORDER_COLOR_OPAQUE_BLACK   0x01
 #define V_008F3C_SQ_TEX_BORDER_COLOR_OPAQUE_WHITE   0x02
 #define V_008F3C_SQ_TEX_BORDER_COLOR_REGISTER   0x03
 #define R_0090DC_SPI_DYN_GPR_LOCK_EN
0x0090DC /* not on CIK */
 #define   S_0090DC_VS_LOW_THRESHOLD(x)
(((unsigned)(x) & 0x0F) << 0)
 #define   G_0090DC_VS_LOW_THRESHOLD(x)(((x) >> 
0) & 0x0F)
diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h 
b/src/gallium/drivers/radeon/r600_pipe_common.h
index bd0dc76ec2b..41d8cc6f0a7 100644
--- a/src/gallium/drivers/radeon/r600_pipe_common.h
+++ b/src/gallium/drivers/radeon/r600_pipe_common.h
@@ -243,20 +243,21 @@ struct r600_texture {
unsignedcolor_clear_value[2];
unsignedlast_msaa_resolve_target_micro_mode;
 
/* Depth buffer compression and fast clear. */
uint64_thtile_offset;
booltc_compatible_htile;
booldepth_cleared; /* if it was cleared at 
least once */
float   depth_clear_value;
boolstencil_cleared; /* if it was cleared 
at least once */
uint8_t stencil_clear_value;
+   boolupgraded_depth; /* upgraded from unorm 
to Z32_FLOAT */
 
boolnon_disp_tiling; /* R600-Cayman only */
 
/* Whether the texture is a displayable back buffer and needs DCC
 * decompression, which is expensive. Therefore, it's enabled only
 * if statistics suggest that it will pay off and it's allocated
 * separately. It can't be bound as a sampler by apps. Limited to
 * target == 2D and last_level == 0. If enabled, dcc_offset contains
 * the absolute GPUVM address, not the relative one.
 */
diff --git a/src/gallium/drivers/radeon/r600_texture.c 
b/src/gallium/drivers/radeon/r600_

Re: [Mesa-dev] [PATCH 2/2] i965: Implement ARB_indirect_parameters

2017-09-26 Thread Manolova, Plamena
Hi Ken,
Thank you so much for reviewing and helping me out with this! I'll make the
changes you suggested
to both patches and resubmit.

Thanks,
Pam

On Tue, Sep 26, 2017 at 3:32 AM, Kenneth Graunke 
wrote:

> On Tuesday, August 29, 2017 9:24:01 AM PDT Plamena Manolova wrote:
> > We can implement ARB_indirect_parameters for i965 by
> > taking advantage of the conditional rendering mechanism.
> > This works by issuing maxdrawcount draw calls and using
> > conditional rendering to predicate each of them with
> > "drawcount > gl_DrawID"
> >
> > Signed-off-by: Plamena Manolova 
> > ---
> >  src/mesa/drivers/dri/i965/brw_context.h  |  3 +
> >  src/mesa/drivers/dri/i965/brw_draw.c | 97
> 
> >  src/mesa/drivers/dri/i965/brw_draw.h | 11 
> >  src/mesa/drivers/dri/i965/intel_extensions.c |  2 +
> >  4 files changed, 113 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> > index 2274fe5..4639d5b 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.h
> > +++ b/src/mesa/drivers/dri/i965/brw_context.h
> > @@ -831,6 +831,9 @@ struct brw_context
> >int gl_drawid;
> >struct brw_bo *draw_id_bo;
> >uint32_t draw_id_offset;
> > +
> > +  struct brw_bo *draw_params_count_bo;
> > +  uint32_t draw_params_count_offset;
> > } draw;
> >
> > struct {
> > diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> b/src/mesa/drivers/dri/i965/brw_draw.c
> > index 7597bae..473958c 100644
> > --- a/src/mesa/drivers/dri/i965/brw_draw.c
> > +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> > @@ -820,6 +820,11 @@ brw_end_draw_prims(struct gl_context *ctx,
> > brw_program_cache_check_size(brw);
> > brw_postdraw_reconcile_align_wa_slices(brw);
> > brw_postdraw_set_buffers_need_resolve(brw);
> > +
> > +   if (brw->draw.draw_params_count_bo) {
> > +  brw_bo_unreference(brw->draw.draw_params_count_bo);
> > +  brw->draw.draw_params_count_bo = NULL;
> > +   }
> >  }
> >
> >  void
> > @@ -837,6 +842,8 @@ brw_draw_prims(struct gl_context *ctx,
> > struct brw_context *brw = brw_context(ctx);
> > const struct gl_vertex_array **arrays = ctx->Array._DrawArrays;
> > int i;
> > +   int predicate_state = brw->predicate.state;
> > +   int combine_op = MI_PREDICATE_COMBINEOP_SET;
> > struct brw_transform_feedback_object *xfb_obj =
> >(struct brw_transform_feedback_object *) gl_xfb_obj;
> >
> > @@ -890,12 +897,101 @@ brw_draw_prims(struct gl_context *ctx,
> >  * to it.
> >  */
> >
> > +   if (brw->draw.draw_params_count_bo &&
> > +   predicate_state == BRW_PREDICATE_STATE_USE_BIT) {
> > +  /* We do this to empty the MI_PREDICATE_DATA register */
> > +  BEGIN_BATCH(4);
> > +  OUT_BATCH(MI_PREDICATE_DATA);
> > +  OUT_BATCH(0u);
> > +  OUT_BATCH(MI_PREDICATE_DATA + 4);
> > +  OUT_BATCH(0u);
> > +  ADVANCE_BATCH();
> > +
> > +  combine_op = MI_PREDICATE_COMBINEOP_AND;
> > +   }
> > +
> > for (i = 0; i < nr_prims; i++) {
> > +  if (brw->draw.draw_params_count_bo) {
> > + struct brw_bo *draw_id_bo = brw_bo_alloc(brw->bufmgr,
> "draw_id", 4, 4);
> > +
> > + brw_bo_reference(draw_id_bo);
>
> brw_bo_alloc starts you with a reference count of 1, so you don't want
> to call brw_bo_reference here - that would increase the refcount to 2,
> and your brw_bo_unreference() call below would only drop it to 1, which
> would leak the BO.
>
> > + brw_bo_subdata(draw_id_bo, 0, 4, &prims[i].draw_id);
>
> Buffer objects are always page-aligned sizes, so we're actually
> allocating a full 4 kilobytes for this 32-bit integer.  That's kind
> of a bummer.  To avoid this, we have a streaming upload buffer which we
> use to append random data we want to use in the batch.  I'd use that
> instead:
>
>struct brw_bo *draw_id_bo;
>uint32_t draw_id_offset;
>
>intel_upload_data(brw, &prims[i].draw_id, 4, 4,
>  &draw_id_bo, &draw_id_offset);
>
> (I'm assuming prims[i].draw_id is 4-byte aligned...it should be unless
> people have explicitly marked the _mesa_prim struct PACKED...)
>
> > +
> > + brw_emit_pipe_control_flush(brw, PIPE_CONTROL_FLUSH_ENABLE);
> > +
> > + brw_load_register_mem(brw, MI_PREDICATE_SRC0,
> > +   brw->draw.draw_params_count_bo,
> > +   brw->draw.draw_params_count_offset);
> > + brw_load_register_mem(brw, MI_PREDICATE_SRC1, draw_id_bo, 0);
>
> then make sure to use the offset:
>
>  brw_load_register_mem(brw, MI_PREDICATE_SRC1,
>draw_id_bo, draw_id_offset);
>
> Note that intel_upload_* is only meant to be used for a single batch...
> if you need it to persist across multiple batches, you probably want to
> allocate your own BO like you originally did here.
>
> > +
> > + BEGIN_BATCH(1);
> > + OUT_BATCH(GEN7_MI_PREDICATE |
> > +   M

[Mesa-dev] [PATCH] radv: fix saved compute state when doing statistics/occlusion queries

2017-09-26 Thread Samuel Pitoiset
We are pushing 16-bytes of constants, so we have to save/restore
the same amount of data to avoid data corruption.

Signed-off-by: Samuel Pitoiset 
Cc: 17.2 
---
 src/amd/vulkan/radv_query.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c
index 4f79db4a93..1dbc493e1b 100644
--- a/src/amd/vulkan/radv_query.c
+++ b/src/amd/vulkan/radv_query.c
@@ -653,7 +653,7 @@ static void radv_query_shader(struct radv_cmd_buffer 
*cmd_buffer,
struct radv_device *device = cmd_buffer->device;
struct radv_meta_saved_compute_state saved_state;
 
-   radv_meta_save_compute(&saved_state, cmd_buffer, 4);
+   radv_meta_save_compute(&saved_state, cmd_buffer, 16);
 
struct radv_buffer dst_buffer = {
.bo = dst_bo,
@@ -737,7 +737,7 @@ static void radv_query_shader(struct radv_cmd_buffer 
*cmd_buffer,
RADV_CMD_FLAG_INV_VMEM_L1 |
RADV_CMD_FLAG_CS_PARTIAL_FLUSH;
 
-   radv_meta_restore_compute(&saved_state, cmd_buffer, 4);
+   radv_meta_restore_compute(&saved_state, cmd_buffer, 16);
 }
 
 VkResult radv_CreateQueryPool(
-- 
2.14.1

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


[Mesa-dev] [PATCH] st/va: use pipe transfer_map to upload mapped buffer

2017-09-26 Thread Leo Liu
The function pipe_buffer_map() is only for linear pipe buffer,
with height as 0, and it's not for any 2D textures.

Signed-off-by: Leo Liu 
Cc: mesa-sta...@lists.freedesktop.org
Cc: Mark Thompson 
---
 src/gallium/state_trackers/va/buffer.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/gallium/state_trackers/va/buffer.c 
b/src/gallium/state_trackers/va/buffer.c
index fb5b20e44b..deaeb1939f 100644
--- a/src/gallium/state_trackers/va/buffer.c
+++ b/src/gallium/state_trackers/va/buffer.c
@@ -125,9 +125,15 @@ vlVaMapBuffer(VADriverContextP ctx, VABufferID buf_id, 
void **pbuff)
}
 
if (buf->derived_surface.resource) {
-  *pbuff = pipe_buffer_map(drv->pipe, buf->derived_surface.resource,
-   PIPE_TRANSFER_WRITE,
-   &buf->derived_surface.transfer);
+  struct pipe_resource *resource;
+  struct pipe_box box = {};
+
+  resource = buf->derived_surface.resource;
+  box.width = resource->width0;
+  box.height = resource->height0;
+  box.depth = resource->depth0;
+  *pbuff = drv->pipe->transfer_map(drv->pipe, resource, 0, 
PIPE_TRANSFER_WRITE,
+   &box, &buf->derived_surface.transfer);
   mtx_unlock(&drv->mutex);
 
   if (!buf->derived_surface.transfer || !*pbuff)
-- 
2.11.0

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


[Mesa-dev] [PATCH] glsl: do not set the 'smooth' qualifier by default on ES shaders

2017-09-26 Thread Nicolai Hähnle
From: Nicolai Hähnle 

It leads to surprising states with integer inputs and outputs on
vertex processing stages (e.g. geometry stages). Instead, rely on the
driver to choose smooth interpolation by default.

We still allow varyings to match when one stage declares it as smooth
and the other declares it without interpolation qualifiers.
--
This should cover all the relevant I/O checks, tested with dEQP.
---
 src/compiler/glsl/ast_to_hir.cpp| 11 ---
 src/compiler/glsl/link_varyings.cpp | 17 -
 src/mesa/main/shader_query.cpp  |  8 +++-
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index c46454956d7..1999e68158c 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3119,31 +3119,20 @@ interpret_interpolation_qualifier(const struct 
ast_type_qualifier *qual,
   struct _mesa_glsl_parse_state *state,
   YYLTYPE *loc)
 {
glsl_interp_mode interpolation;
if (qual->flags.q.flat)
   interpolation = INTERP_MODE_FLAT;
else if (qual->flags.q.noperspective)
   interpolation = INTERP_MODE_NOPERSPECTIVE;
else if (qual->flags.q.smooth)
   interpolation = INTERP_MODE_SMOOTH;
-   else if (state->es_shader &&
-((mode == ir_var_shader_in &&
-  state->stage != MESA_SHADER_VERTEX) ||
- (mode == ir_var_shader_out &&
-  state->stage != MESA_SHADER_FRAGMENT)))
-  /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
-   *
-   *"When no interpolation qualifier is present, smooth interpolation
-   *is used."
-   */
-  interpolation = INTERP_MODE_SMOOTH;
else
   interpolation = INTERP_MODE_NONE;
 
validate_interpolation_qualifier(state, loc,
 interpolation,
 qual, var_type, mode);
 
return interpolation;
 }
 
diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index 528506fd0eb..7c90de393ad 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -318,22 +318,37 @@ cross_validate_types_and_qualifiers(struct 
gl_shader_program *prog,
}
 
/* GLSL >= 4.40 removes text requiring interpolation qualifiers
 * to match cross stage, they must only match within the same stage.
 *
 * From page 84 (page 90 of the PDF) of the GLSL 4.40 spec:
 *
 * "It is a link-time error if, within the same stage, the interpolation
 * qualifiers of variables of the same name do not match.
 *
+* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
+*
+*"When no interpolation qualifier is present, smooth interpolation
+*is used."
+*
+* So we match variables where one is smooth and the other has no explicit
+* qualifier.
 */
-   if (input->data.interpolation != output->data.interpolation &&
+   unsigned input_interpolation = input->data.interpolation;
+   unsigned output_interpolation = output->data.interpolation;
+   if (prog->IsES) {
+  if (input_interpolation == INTERP_MODE_NONE)
+ input_interpolation = INTERP_MODE_SMOOTH;
+  if (output_interpolation == INTERP_MODE_NONE)
+ output_interpolation = INTERP_MODE_SMOOTH;
+   }
+   if (input_interpolation != output_interpolation &&
prog->data->Version < 440) {
   linker_error(prog,
"%s shader output `%s' specifies %s "
"interpolation qualifier, "
"but %s shader input specifies %s "
"interpolation qualifier\n",
_mesa_shader_stage_to_string(producer_stage),
output->name,
interpolation_string(output->data.interpolation),
_mesa_shader_stage_to_string(consumer_stage),
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 64e68b4a26d..6712bb45fb2 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -1627,21 +1627,27 @@ validate_io(struct gl_program *producer, struct 
gl_program *consumer)
*Precision  |   mediump   |  Yes
*   |highp|
*---+-+--
*Variance   |  invariant  |   No
*---+-+--
*Memory | all |  N/A
*
* Note that location mismatches are detected by the loops above that
* find the producer variable that goes with the consumer variable.
*/
-  if (producer_var->interpolation != consumer_var->interpolation) {
+  unsigned producer_interpolation = producer_var->interpolation;
+  unsigned consumer_interpolation = consumer_var->interpolation;
+  if (producer_interpolation == INTE

Re: [Mesa-dev] [PATCH mesa 6/6] scons: use python3-compatible string-check

2017-09-26 Thread Jose Fonseca

On 26/09/17 12:20, Ilia Mirkin wrote:

On Tue, Sep 26, 2017 at 7:07 AM, Jose Fonseca  wrote:

On 25/09/17 14:30, Eric Engestrom wrote:


I pushed the rest of the series.
See below for discussion on this patch.


On Wednesday, 2017-09-20 17:05:21 +, Jose Fonseca wrote:


On 19/09/17 15:14, Eric Engestrom wrote:


Signed-off-by: Eric Engestrom 
---
scons/custom.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scons/custom.py b/scons/custom.py
index 0767ba936d410167116d..978ee5f9ec7c23a74cb9 100644
--- a/scons/custom.py
+++ b/scons/custom.py
@@ -257,7 +257,7 @@ def parse_source_list(env, filename, names=None):
sym_table = parser.parse(src.abspath)
if names:
-if isinstance(names, basestring):
+if isinstance(names, str):
names = [names]
symbols = names



I'm not sure if this won't give the wrong results for unicode strings,
but
at any rate, I don't think that should ever happen in practice.



Are you replying to Ilia [1] here?

He left a comment on this patch, saying:


This might be python3-compatible, but it's not the same thing. str !=
unicode. Not sure where "names" can come from, but if it can come in
as a unicode string, this won't work.



My knowledge of python is quite basic, so I'd rather you discuss between
you two rather than me trying to forward a conversation with each of you
:P

[1]
https://lists.freedesktop.org/archives/mesa-dev/2017-September/170130.html



I just checked all uses of ParseSourceList and it's never used with unicode
strings.  It's always used with plain strings or lists of plain strings.

So on 2nd thought, I think this patch should be safe.  Ilia, would you
agree?


So really this whole thing is about whether names is a list or not.
You can't check if it's iterable since strings are also iterable. What
lists are passed in here? If it's all list-type lists (and not
tuples/etc), we can change this to be

if not isinstance(names, list)


In this case, the odds of a tuple to creep in are not smaller than an 
unicode to creep in.




Or we want to be cheeky,

if type(names) == type(names[0])


That's an interesting suggestion.  There's the issue on empty lists/strings.



For a list of strings, that won't be the case, but for a string, it
should be, since a single char has the same type as a string. At least
in python2.

I don't use scons, or know how all this stuff is used, so you guys can
do whatever. In my experience, unicode strings spring up from the most
unexpected places.

   -ilia


I think in the end I `isinstance(names, str)` really should be sufficient.

Jose

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


Re: [Mesa-dev] MESA and KOTOR

2017-09-26 Thread Federico Dossena
The crash is in GLU, and I'm 99% sure that it has to do with the format 
of that texture.
I saw a patch from Miklòs Màté a while ago that changed something in 
src/mesa/state_tracker/st_atom_sampler.c to fix this (link: 
https://patchwork.freedesktop.org/patch/68298/). It never made it to 
master, and that function has changed quite a bit since his patch. I 
don't really understand what it does since I don't know mesa very well, 
do you know how I could do the same thing in the new st_convert_sampler 
function?


Thanks


Il 2017-09-26 12:56, Nicolai Hähnle ha scritto:

On 25.09.2017 18:50, Federico Dossena wrote:

Hello everyone,
you may remember that a few months ago I was trying to fix KOTOR to 
work with Mesa to use the Gallium llvmpipe software renderer.


Well, it's been a while and I'm happy to see that things are a bit 
better with Mesa 17.2. The game still crashes, but we're closer to 
fixing it.


Here's what I found using 17.2.1:
With frame buffer effects and soft shadows the game crashes at the 
end of loading; the crash is inside a function that amongst other 
things, generates mipmaps for a texture used in a pbuffer (function 
at offset 2FB37D in my exe).
The crash happens when gluBuild2DMipmaps is called, however doesn't 
seem to be a null pointer like it was back in march: it's an access 
violation alright but no longer a null pointer. So I think it's a 
different, hopefully simpler, problem.


So is it a crash in KOTOR, in glu, or in Mesa? If it's in one of the 
last two, then you should be able to compile both glu and Mesa from 
source with debugging info to help you narrow things down. A backtrace 
would be a good start.


If it's in KOTOR itself, then I'm afraid we're probably not going to 
be a lot of help here...


Cheers,
Nicolai



Back in march, Miklòs Màté suggested that changing the checks for the 
pixel format could fix the problem, and he was right; without those 
checks we definitely got a step closer to fixing it.


My first thought was to just NOP the entire section that generates 
mipmaps and a bit of code later that uses it. The game no longer 
crashes, however it displays nothing, but I can hear it running in 
background. So this is the last issue! We're almost there!


Now, I'm bothering you again because I think that at this point it's 
just a problem with the texture format used there. The call to 
gluBuild2DMipmaps uses LuminanceAlpha' as texture format as well as 
internal format (0x190a). I tried changing it to RGB and RGBA just to 
try something, but that didn't work because I guess the texture was 
already generated with another format.


What could I do to investigate this further? And where should I look 
inside Mesa if I wanted to say... force a specific texture format for 
pbuffers?


I feel that we're very close to fixing this. Your help would mean the 
world to me and the whole KOTOR community.


Thank you ;)

P.S.
This has nothing to do with mesa, but you should know that KOTOR is 
slowly dieing. It is currently unplayable on Intel and AMD graphics, 
and recent nVidia driver updates have introduced a glitch with 
transparencies (it can be fixed, but still, no one can play KOTOR on 
modern hardware properly and we have to keep old computers as 
dedicated "shrines" for KOTOR, that's why I insist so much on Mesa)




___
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 v2 7/7] i965: make use of nir linking

2017-09-26 Thread Kenneth Graunke
On Tuesday, September 26, 2017 2:54:28 AM PDT Timothy Arceri wrote:
> On 26/09/17 17:50, Kenneth Graunke wrote:
> > A couple more thoughts:
> > 
> > 1. We're calling brw_nir_optimize even more now...it might make sense to
> > drop it from brw_preprocess_nir, drop it here, and put it once in the
> > final (third) loop, just before brw_shader_gather_info.  At least,
> > it'd be interesting how that affects compile times / quality...
> 
> Actually we can't drop the producer brw_nir_optimize call here anyway as 
> we want to remove dead code before the next iteration of the loop.

Ah, good point...we need to optimize to remove dead code, to make
varyings dead, so we can remove them too.  Nevermind then :)

--Ken


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


Re: [Mesa-dev] [PATCH] i965: Convert brw->*_program into a brw->programs[i] array.

2017-09-26 Thread Kenneth Graunke
On Tuesday, September 26, 2017 1:13:18 AM PDT Alejandro Piñeiro wrote:
> On 26/09/17 09:39, Kenneth Graunke wrote:
> > diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c 
> > b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > index 612761601a2..2a99376e3c2 100644
> > --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> > +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > @@ -1019,6 +1019,9 @@ genX(calculate_attr_overrides)(const struct 
> > brw_context *brw,
> > /* _NEW_POINT */
> > const struct gl_point_attrib *point = &ctx->Point;
> >  
> > +   /* BRW_NEW_FRAGMENT_PROGRAM */
> > +   const struct gl_program *fp = brw->programs[MESA_SHADER_FRAGMENT];
> > +
> > /* BRW_NEW_FS_PROG_DATA */
> > const struct brw_wm_prog_data *wm_prog_data =
> >brw_wm_prog_data(brw->wm.base.prog_data);
> > @@ -1026,16 +1029,14 @@ genX(calculate_attr_overrides)(const struct 
> > brw_context *brw,
> >  
> > *point_sprite_enables = 0;
> >  
> > -   /* BRW_NEW_FRAGMENT_PROGRAM
> 
> Was this header removed on purpose? Don't care too much about it,
> pointing just in case.

Yeah, I kept it with the use of brw->fragment_program (initializing fp).

> > -*
> > -* If the fragment shader reads VARYING_SLOT_LAYER, then we need to 
> > pass in
> > +   /* If the fragment shader reads VARYING_SLOT_LAYER, then we need to 
> > pass in
> >  * the full vertex header.  Otherwise, we can program the SF to start
> >  * reading at an offset of 1 (2 varying slots) to skip unnecessary data:
> >  * - VARYING_SLOT_PSIZ and BRW_VARYING_SLOT_NDC on gen4-5
> >  * - VARYING_SLOT_{PSIZ,LAYER} and VARYING_SLOT_POS on gen6+
> >  */
> >  
> > -   bool fs_needs_vue_header = brw->fragment_program->info.inputs_read &
> > +   bool fs_needs_vue_header = fp->info.inputs_read &
> >(VARYING_BIT_LAYER | VARYING_BIT_VIEWPORT);
> >  
> > *urb_entry_read_offset = fs_needs_vue_header ? 0 : 1;
[snip]
> As mentioned, my previous comment was a just-in-case. You can ignore it. So:
> Reviewed-by: Alejandro Piñeiro 

Thanks!


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


[Mesa-dev] [PATCH v3 4/4] meson: build "radv" vulkan driver for radeon hardware

2017-09-26 Thread Dylan Baker
This builds and installs, but I haven't had a chance to test it yet.

v2: - enable radv by default
- add shader cache support and enforce that it's built for radv
v3: - Fix typo in meson_options (Nicholas)
- strip trailing 'svn' from llvm version before setting the version
  preprocessor flag (Bas)
- Check for LLVM module requirements

Signed-off-by: Dylan Baker 
---
 meson.build   |  40 +++-
 meson_options.txt |   4 +-
 src/{ => amd/addrlib}/meson.build |  69 -
 src/{ => amd/common}/meson.build  |  65 
 src/{ => amd}/meson.build |  33 ++
 src/amd/vulkan/meson.build| 124 ++
 src/meson.build   |   2 +-
 7 files changed, 253 insertions(+), 84 deletions(-)
 copy src/{ => amd/addrlib}/meson.build (51%)
 copy src/{ => amd/common}/meson.build (51%)
 copy src/{ => amd}/meson.build (65%)
 create mode 100644 src/amd/vulkan/meson.build

diff --git a/meson.build b/meson.build
index 09e53957fe9..b57d800d5d0 100644
--- a/meson.build
+++ b/meson.build
@@ -71,6 +71,12 @@ if get_option('buildtype').startswith('debug')
   pre_args += '-DDEBUG'
 endif
 
+if get_option('shader-cache')
+  pre_args += '-DENABLE_SHADER_CACHE'
+elif with_amd_vk
+  error('Radv requires shader cache support')
+endif
+
 # Check for GCC style builtins
 foreach b : ['bswap32', 'bswap64', 'clz', 'clzll', 'ctz', 'expect', 'ffs',
  'ffsll', 'popcount', 'popcountll', 'unreachable']
@@ -79,7 +85,7 @@ foreach b : ['bswap32', 'bswap64', 'clz', 'clzll', 'ctz', 
'expect', 'ffs',
   endif
 endforeach
 
-# check for GCC __attribute__ s
+# check for GCC __attribute__
 foreach a : ['const', 'flatten', 'malloc', 'pure', 'unused',
  'warn_unused_result', 'weak',]
   if cc.compiles('int foo(void) __attribute__((@0@));'.format(a),
@@ -291,6 +297,36 @@ dep_m = cc.find_library('m', required : false)
 # TODO: conditionalize libdrm requirement
 dep_libdrm = dependency('libdrm', version : '>= 2.4.75')
 pre_args += '-DHAVE_LIBDRM'
+dep_libdrm_amdgpu = []
+if with_amd_vk
+  dep_libdrm_amdgpu = dependency('libdrm_amdgpu', version : '>= 2.4.82')
+endif
+
+llvm_modules = []
+if with_amd_vk
+  llvm_modules += ['amdgpu', 'bitreader', 'ipo']
+endif
+dep_llvm = dependency(
+  'llvm', version : '>= 3.9.0', required : false,
+  modules : ['bitwriter', 'engine', 'mcdisassembler', 'mcjit', llvm_modules],
+)
+if not dep_llvm.found()
+  if with_amd_vk
+error('Radv requires llvm.')
+  endif
+else
+  _llvm_version = dep_llvm.version().split('.')
+  # Development versions of LLVM have an 'svn' suffix, we don't want that for
+  # our version checks.
+  _llvm_patch = _llvm_version[2]
+  if _llvm_patch.endswith('svn')
+_llvm_patch = _llvm_patch.split('s')[0]
+  endif
+  pre_args += [
+'-DHAVE_LLVM=0x0@0@@1@@2@'.format(_llvm_version[0], _llvm_version[1], 
_llvm_patch),
+'-DMESA_LLVM_VERSION_PATCH=@0@'.format(_llvm_patch),
+  ]
+endif
 
 # TODO: make this conditional
 dep_valgrind = dependency('valgrind', required : false)
@@ -304,8 +340,6 @@ endif
 
 # TODO: llvm-prefix and llvm-shared-libs
 
-# TODO: llvm dependency (that's all native now, yay!)
-
 # TODO: unwind (llvm [radeon, gallivm] and gallium)
 
 # TODO: flags for opengl, gles, dri
diff --git a/meson_options.txt b/meson_options.txt
index 0e0d04a0f8f..082ade7f480 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -20,8 +20,10 @@
 
 option('platforms',  type : 'string',  value : 'x11,wayland',
description : 'comma separated list of window systems to support. 
wayland, x11, surfaceless, drm, etc.')
-option('vulkan-drivers', type : 'string',  value : 'intel',
+option('vulkan-drivers', type : 'string',  value : 'intel,amd',
description : 'comma separated list of vulkan drivers to build.')
+option('shader-cache',type : 'boolean', value : true,
+   description : 'Build with on-disk shader cache support')
 option('vulkan_icd_dir', type : 'string',  value : '',
description : 'Location relative to prefix to put vulkan icds on 
install. Default: $datadir/vulkan/icd.d')
 option('valgrind',   type : 'boolean', value : true,
diff --git a/src/meson.build b/src/amd/addrlib/meson.build
similarity index 51%
copy from src/meson.build
copy to src/amd/addrlib/meson.build
index 4c82eec70f1..a6cad1207b0 100644
--- a/src/meson.build
+++ b/src/amd/addrlib/meson.build
@@ -18,31 +18,46 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
-# TODO: libglsl_util
+files_addrlib = files(
+  'addrinterface.cpp',
+  'addrinterface.h',
+  'addrtypes.h',
+  'core/addrcommon.h',
+  'core/addrelemlib.cpp',
+  'core/addrelemlib.h',
+  'core/addrlib.cpp',
+  'core/addrlib.h',
+  'core/addrlib1.cpp',
+  'core/addrlib1.h',
+  'core/addrlib2.cpp',
+  'core/addrlib2.h',
+  'core/addrobject.cpp',
+  'core/addrobject.h',
+  'gfx9/chip/gfx9_enum.h',
+  'gfx9/coord.cpp',
+  'gfx9/

[Mesa-dev] [PATCH v3 1/4] intel: use a flag instead of setting PYTHONPATH

2017-09-26 Thread Dylan Baker
Meson doesn't allow setting environment variables for custom targets, so
we either need to not pass this as an environment variable or use a
shell script to wrap the invocation. The chosen solution has the
advantage of working for both autotools and meson.

v2: - put rules back in top scope (Ken)

Reviewed-by: Kenneth Graunke 
Signed-off-by: Dylan Baker 
---
 src/intel/Makefile.compiler.am |  2 +-
 src/intel/compiler/brw_nir_trig_workarounds.py | 33 +++---
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/src/intel/Makefile.compiler.am b/src/intel/Makefile.compiler.am
index 3ab550c96b1..45e7a6ccce8 100644
--- a/src/intel/Makefile.compiler.am
+++ b/src/intel/Makefile.compiler.am
@@ -35,7 +35,7 @@ BUILT_SOURCES += $(COMPILER_GENERATED_FILES)
 compiler/brw_nir_trig_workarounds.c: compiler/brw_nir_trig_workarounds.py \
  
$(top_srcdir)/src/compiler/nir/nir_algebraic.py
$(MKDIR_GEN)
-   $(AM_V_GEN) PYTHONPATH=$(top_srcdir)/src/compiler/nir $(PYTHON2) 
$(PYTHON_FLAGS) $(srcdir)/compiler/brw_nir_trig_workarounds.py > $@ || ($(RM) 
$@; false)
+   $(AM_V_GEN) $(PYTHON2) $(PYTHON_FLAGS) 
$(srcdir)/compiler/brw_nir_trig_workarounds.py -p 
$(top_srcdir)/src/compiler/nir > $@ || ($(RM) $@; false)
 
 EXTRA_DIST += \
compiler/brw_nir_trig_workarounds.py
diff --git a/src/intel/compiler/brw_nir_trig_workarounds.py 
b/src/intel/compiler/brw_nir_trig_workarounds.py
index 6a77d64dbd4..3d08b9a41ea 100644
--- a/src/intel/compiler/brw_nir_trig_workarounds.py
+++ b/src/intel/compiler/brw_nir_trig_workarounds.py
@@ -20,8 +20,6 @@
 # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
 # IN THE SOFTWARE.
 
-import nir_algebraic
-
 # Prior to Kaby Lake, The SIN and COS instructions on Intel hardware can
 # produce values slightly outside of the [-1.0, 1.0] range for a small set of
 # values.  Obviously, this can break everyone's expectations about trig
@@ -33,11 +31,30 @@ import nir_algebraic
 # amplitude slightly.  Apparently this also minimizes the error function,
 # reducing the maximum error from 0.6 to about 0.3.
 
-trig_workarounds = [
-   (('fsin', 'x'), ('fmul', ('fsin', 'x'), 0.7)),
-   (('fcos', 'x'), ('fmul', ('fcos', 'x'), 0.7)),
+import argparse
+import sys
+
+TRIG_WORKAROUNDS = [
+(('fsin', 'x'), ('fmul', ('fsin', 'x'), 0.7)),
+(('fcos', 'x'), ('fmul', ('fcos', 'x'), 0.7)),
 ]
 
-print '#include "brw_nir.h"'
-print nir_algebraic.AlgebraicPass("brw_nir_apply_trig_workarounds",
-  trig_workarounds).render()
+
+def main():
+parser = argparse.ArgumentParser()
+parser.add_argument('-p', '--import-path', required=True)
+args = parser.parse_args()
+sys.path.insert(0, args.import_path)
+run()
+
+
+def run():
+import nir_algebraic  # pylint: disable=import-error
+
+print '#include "brw_nir.h"'
+print nir_algebraic.AlgebraicPass("brw_nir_apply_trig_workarounds",
+  TRIG_WORKAROUNDS).render()
+
+
+if __name__ == '__main__':
+main()
-- 
2.14.1

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


[Mesa-dev] [PATCH v3 2/4] util/ralloc: Don't define assert with magic member without DEBUG

2017-09-26 Thread Dylan Baker
It is possible to have DEBUG disabled but asserts on (NDEBUG(, which
cannot build because these asserts work on members that are only present
when DEBUG is on.

Reviewed-by: Kenneth Graunke 
Signed-off-by: Dylan Baker 
---
 src/util/ralloc.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/util/ralloc.c b/src/util/ralloc.c
index 566f08ad94e..9cce9e02f68 100644
--- a/src/util/ralloc.c
+++ b/src/util/ralloc.c
@@ -630,7 +630,9 @@ linear_alloc_child(void *parent, unsigned size)
linear_size_chunk *ptr;
unsigned full_size;
 
+#ifdef DEBUG
assert(first->magic == LMAGIC);
+#endif
assert(!latest->next);
 
size = ALIGN_POT(size, SUBALLOC_ALIGNMENT);
@@ -702,7 +704,9 @@ linear_free_parent(void *ptr)
   return;
 
node = LINEAR_PARENT_TO_HEADER(ptr);
+#ifdef DEBUG
assert(node->magic == LMAGIC);
+#endif
 
while (node) {
   void *ptr = node;
@@ -721,7 +725,9 @@ ralloc_steal_linear_parent(void *new_ralloc_ctx, void *ptr)
   return;
 
node = LINEAR_PARENT_TO_HEADER(ptr);
+#ifdef DEBUG
assert(node->magic == LMAGIC);
+#endif
 
while (node) {
   ralloc_steal(new_ralloc_ctx, node);
@@ -734,7 +740,9 @@ void *
 ralloc_parent_of_linear_parent(void *ptr)
 {
linear_header *node = LINEAR_PARENT_TO_HEADER(ptr);
+#ifdef DEBUG
assert(node->magic == LMAGIC);
+#endif
return node->ralloc_parent;
 }
 
-- 
2.14.1

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


[Mesa-dev] [PATCH v3 3/4] meson: Add build Intel "anv" vulkan driver

2017-09-26 Thread Dylan Baker
This allows building and installing the Intel "anv" Vulkan driver using
meson and ninja, the driver has been tested against the CTS and has
seems to pass the same series of tests (they both segfault when the CTS
tries to run wayland wsi tests).

There are still a mess of TODO, XXX, and FIXME comments in here. Those
are mostly for meson bugs I'm trying to fix, or for additional things to
implement for other drivers/features.

I have configured all intermediate libraries and optional tools to not
build by default, meaning they will only be built if they're pulled in
as a dependency of a target that will actually be installed) this allows
us to avoid massive if chains, while ensuring that only the bits that
need to be built are.

v2: - enable anv, x11, and wayland by default
- add configure option to disable valgrind
v3: - fix typo in meson_options (Nicholas)

Signed-off-by: Dylan Baker 
---
 include/meson.build |  22 ++
 meson.build | 423 
 meson_options.txt   |  30 +++
 src/compiler/glsl/meson.build   |  29 +++
 src/compiler/meson.build|  57 +
 src/compiler/nir/meson.build| 205 
 src/egl/wayland/wayland-drm/meson.build |  21 ++
 src/gtest/meson.build   |  26 ++
 src/intel/blorp/meson.build |  37 +++
 src/intel/common/meson.build|  44 
 src/intel/compiler/meson.build  | 155 
 src/intel/genxml/meson.build|  59 +
 src/intel/isl/meson.build   | 105 
 src/intel/meson.build   |  31 +++
 src/intel/tools/meson.build |  39 +++
 src/intel/vulkan/meson.build| 182 ++
 src/mapi/glapi/gen/meson.build  |  19 ++
 src/meson.build |  48 
 src/util/meson.build| 134 ++
 src/util/tests/hash_table/meson.build   |  32 +++
 src/vulkan/meson.build  |  28 +++
 src/vulkan/util/gen_enum_to_str.py  |   4 +-
 src/vulkan/util/meson.build |  47 
 src/vulkan/wsi/meson.build  |  71 ++
 24 files changed, 1846 insertions(+), 2 deletions(-)
 create mode 100644 include/meson.build
 create mode 100644 meson.build
 create mode 100644 meson_options.txt
 create mode 100644 src/compiler/glsl/meson.build
 create mode 100644 src/compiler/meson.build
 create mode 100644 src/compiler/nir/meson.build
 create mode 100644 src/egl/wayland/wayland-drm/meson.build
 create mode 100644 src/gtest/meson.build
 create mode 100644 src/intel/blorp/meson.build
 create mode 100644 src/intel/common/meson.build
 create mode 100644 src/intel/compiler/meson.build
 create mode 100644 src/intel/genxml/meson.build
 create mode 100644 src/intel/isl/meson.build
 create mode 100644 src/intel/meson.build
 create mode 100644 src/intel/tools/meson.build
 create mode 100644 src/intel/vulkan/meson.build
 create mode 100644 src/mapi/glapi/gen/meson.build
 create mode 100644 src/meson.build
 create mode 100644 src/util/meson.build
 create mode 100644 src/util/tests/hash_table/meson.build
 create mode 100644 src/vulkan/meson.build
 create mode 100644 src/vulkan/util/meson.build
 create mode 100644 src/vulkan/wsi/meson.build

diff --git a/include/meson.build b/include/meson.build
new file mode 100644
index 000..93def7e0ec2
--- /dev/null
+++ b/include/meson.build
@@ -0,0 +1,22 @@
+# Copyright ?? 2017 Intel Corporation
+
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+inc_drm_uapi = include_directories('drm-uapi')
+inc_vulkan = include_directories('vulkan')
diff --git a/meson.build b/meson.build
new file mode 100644
index 000..09e53957fe9
--- /dev/null
+++ b/meson.build
@@ -0,0 +1,423 @@
+# Copyright ?? 2017 Intel Corporation
+
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documen

Re: [Mesa-dev] [Mesa-stable] [PATCH] meta: Fix BlitFramebuffer temp texture setup

2017-09-26 Thread Juan A. Suarez Romero
On Tue, 2017-07-11 at 15:42 +0300, Ville Syrjälä wrote:
> On Mon, Jul 10, 2017 at 11:42:18PM +0300, Andres Gomez wrote:
> > Ville, has this patch fallen through the cracks ?
> 
> Nope. I've still been looking into the issue whenever I've had a
> few minutes to spare. I think I have it more or les figured out
> at this stage, but I'll need to respin this patch, and clean up
> some further patches to fix this stuff properly for i915.
> 

Any news on this patch?

J.A.

> > 
> > On Fri, 2017-06-23 at 14:58 +0300, ville.syrj...@linux.intel.com wrote:
> > > From: Ville Syrjälä 
> > > 
> > > Pass the correct src coordinates to CopyTexSubImage()
> > > when creating the temporary texture, and also take care to adjust
> > > flipX/Y if the original src coordinates were flipped compared to
> > > the new temporary texture src coordinates.
> > > 
> > > This fixes all the flip_src_x/y tests in
> > > piglit.spec.arb_framebuffer_object.fbo-blit-stretch on i915, but
> > > we're still left with the some failures in the stretch tests.
> > > 
> > > It looks to me like commit b702233f53d6 ("meta: Refactor the
> > > BlitFramebuffer color CopyTexImage fallback.") most likely
> > > broke this codepath.
> > > 
> > > Cc: mesa-sta...@lists.freedesktop.org
> > > Cc: Eric Anholt 
> > > Cc: Kenneth Graunke 
> > > Cc: Ian Romanick 
> > > Cc: Anuj Phogat 
> > > Fixes: b702233f53d6 ("meta: Refactor the BlitFramebuffer color 
> > > CopyTexImage fallback.")
> > > References: https://bugs.freedesktop.org/show_bug.cgi?id=101414
> > > Signed-off-by: Ville Syrjälä 
> > > ---
> > >  src/mesa/drivers/common/meta_blit.c | 8 ++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/common/meta_blit.c 
> > > b/src/mesa/drivers/common/meta_blit.c
> > > index 7adad469aceb..7262ecdfaf13 100644
> > > --- a/src/mesa/drivers/common/meta_blit.c
> > > +++ b/src/mesa/drivers/common/meta_blit.c
> > > @@ -680,12 +680,16 @@ blitframebuffer_texture(struct gl_context *ctx,
> > >}
> > >  
> > >_mesa_meta_setup_copypix_texture(ctx, meta_temp_texture,
> > > -   srcX0, srcY0,
> > > +   MIN2(srcX0, srcX1),
> > > +   MIN2(srcY0, srcY1),
> > > srcW, srcH,
> > > tex_base_format,
> > > filter);
> > >  
> > > -
> > > +  if (srcX0 > srcX1)
> > > + flipX = -flipX;
> > > +  if (srcY0 > srcY1)
> > > + flipY = -flipY;
> > >srcX0 = 0;
> > >srcY0 = 0;
> > >srcX1 = srcW;
> > 
> > -- 
> > Br,
> > 
> > Andres
> 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] glcpp: Avoid unnecessary call to strlen

2017-09-26 Thread Thomas Helland
I've now pushed the series, so feel free to rebase =)

2017-09-22 16:10 GMT+02:00 Ian Romanick :
> This patch is
>
> Reviewed-by: Ian Romanick 
>
> I have a couple patches that go on top of this particular patch, and I'd
> rather rebase before I send them out for review. :)
>
> On 09/14/2017 03:39 PM, Thomas Helland wrote:
>> Length of the token was already calculated by flex and stored in yyleng,
>> no need to implicitly call strlen() via linear_strdup().
>>
>> Reviewed-by: Nicolai Hähnle 
>> Reviewed-by: Timothy Arceri 
>>
>> V2: Also convert this pattern in glsl_lexer.ll
>>
>> V3: Remove a misplaced comment
>>
>> V4: Use a temporary char to avoid type change
>> Remove bogus +1 on length check of identifier
>> ---
>>  src/compiler/glsl/glcpp/glcpp-lex.l |  9 -
>>  src/compiler/glsl/glsl_lexer.ll | 39 
>> +
>>  2 files changed, 39 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/compiler/glsl/glcpp/glcpp-lex.l 
>> b/src/compiler/glsl/glcpp/glcpp-lex.l
>> index 381b97364a..9cfcc12022 100644
>> --- a/src/compiler/glsl/glcpp/glcpp-lex.l
>> +++ b/src/compiler/glsl/glcpp/glcpp-lex.l
>> @@ -101,7 +101,14 @@ void glcpp_set_column (int  column_no , yyscan_t 
>> yyscanner);
>>  #define RETURN_STRING_TOKEN(token)   \
>>   do {\
>>   if (! parser->skipping) {   \
>> - yylval->str = linear_strdup(yyextra->linalloc, 
>> yytext); \
>> + /* We're not doing linear_strdup here, to avoid \
>> +  * an implicit call on strlen() for the length  \
>> +  * of the string, as this is already found by   \
>> +  * flex and stored in yyleng */ \
>> + void *mem_ctx = yyextra->linalloc;  \
>> + yylval->str = linear_alloc_child(mem_ctx,   \
>> +  yyleng + 1);   \
>> + memcpy(yylval->str, yytext, yyleng + 1);\
>>   RETURN_TOKEN_NEVER_SKIP (token);\
>>   }   \
>>   } while(0)
>> diff --git a/src/compiler/glsl/glsl_lexer.ll 
>> b/src/compiler/glsl/glsl_lexer.ll
>> index 7c41455d98..56519bf92d 100644
>> --- a/src/compiler/glsl/glsl_lexer.ll
>> +++ b/src/compiler/glsl/glsl_lexer.ll
>> @@ -81,8 +81,13 @@ static int classify_identifier(struct 
>> _mesa_glsl_parse_state *, const char *);
>> "illegal use of reserved word `%s'", yytext); \
>>return ERROR_TOK;  \
>>} else {  
>>  \
>> -  void *mem_ctx = yyextra->linalloc;
>>  \
>> -  yylval->identifier = linear_strdup(mem_ctx, yytext);   \
>> +  /* We're not doing linear_strdup here, to avoid an implicit\
>> +   * call on strlen() for the length of the string, as this is   \
>> +   * already found by flex and stored in yyleng */   \
>> +  void *mem_ctx = yyextra->linalloc; \
>> + char *id = (char *) linear_alloc_child(mem_ctx, yyleng + 1);   \
>> + memcpy(id, yytext, yyleng + 1);\
>> + yylval->identifier = id;   \
>>return classify_identifier(yyextra, yytext);   \
>>} 
>>  \
>> } while (0)
>> @@ -261,8 +266,14 @@ HASH ^{SPC}#{SPC}
>>  [ \t\r]* { }
>>  :return COLON;
>>  [_a-zA-Z][_a-zA-Z0-9]*   {
>> -void *mem_ctx = yyextra->linalloc;
>> -yylval->identifier = linear_strdup(mem_ctx, 
>> yytext);
>> +/* We're not doing linear_strdup here, to 
>> avoid an implicit call
>> + * on strlen() for the length of the 
>> string, as this is already
>> + * found by flex and stored in yyleng
>> + */
>> +void *mem_ctx = yyextra->linalloc;
>> +char *id = (char *) 
>> linear_alloc_child(mem_ctx, yyleng + 1);
>> +memcpy(id, yytext, yyleng + 1);
>> +yylval->identifier = id;
>>  return IDENTIFIER;
>>   }
>>  [1-9][0-9]*  {
>> @@ -449,8 +460,14 @@ layout   {
>>|| yyextra->ARB_tessellation_shader_enable) {
>> return LAY

Re: [Mesa-dev] [PATCH] glx: use correct table offset for glAreTexturesResidentEXT

2017-09-26 Thread Juan A. Suarez Romero
On Tue, 2017-07-25 at 11:43 +0100, Emil Velikov wrote:
> On 25 July 2017 at 09:12, Nicolai Hähnle  wrote:
> > On 21.07.2017 16:57, Emil Velikov wrote:
> > > 
> > > From: Emil Velikov 
> > > 
> > > The correct offset is 322 as opposed to 332.
> > > Broken since the rework of GET_DISPATCH back in ~2012.
> > 
> > 
> > This is kind of amazing. How did you run into this?
> > 
> 
> Looking at [truly] vendor neutral indirect libGLX, as opposed to the
> current solution.
> 
> > Maybe add a simple touch test to piglit?
> > 
> 
> Ack, will look into.
> 
> > Maybe use _gloffset_AreTexturesResident (from dispatch.h) instead of using a
> > magic number?
> > 
> 
> As the offending commit 99fee476a102 says "... can't use
> src/mesa/main/dispatch.h ..."
> 
> At the same time, the function was moved outside of the python
> generator(s) to address a out of bounds access - see commit
> 13f96c5401f.
> Sadly the same issue exists elsewhere - so I'm looking into addressing
> that once and for all, and purging the handwritten function.
> 
> That's my evil plan ;-)

Emil, how's going your evil plan? :)

J.A.

> 
> -Emil
> ___
> 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 3/3] radeonsi: fix border color translation for integer textures

2017-09-26 Thread Gustaw Smolarczyk
2017-09-26 16:46 GMT+02:00 Nicolai Hähnle :
> From: Nicolai Hähnle 
>
> This fixes the extremely unlikely case that an application uses
> 0x8000 or 0x3f80 as border color for an integer texture and
> helps in the also, but perhaps slightly less, unlikely case that 1 is
> used as a border color.

Hi,

I see that it fixes the wrong optimization in si_translate_border_color.

However, I also see that for floating point textures this will change
-0.0 into 0.0 (if I understand how
V_008F3C_SQ_TEX_BORDER_COLOR_*_BLACK work). I don't know GL rules
enough to judge that (whether 0.0 and -0.0 should be
indistinguishable), but is that ok?

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


[Mesa-dev] XDC 2017 feedback

2017-09-26 Thread Daniel Vetter
Hi all,

First again big thanks to Stéphane and Jennifer for organizing a great XDC.

Like last year we'd like to hear feedback on how this year's XDC went,
both the good (and what you'd like to see more of) and the not so
good. Talk selection, organization, location, scheduling of talks,
anything really.

Here's a few things we took into account from Helsinki and tried to apply:
- More breaks for more hallway track.
- Try to schedule the hot topics on the first day (did we pick the
right ones) for better hallway track.
- More lightning talk time to give all the late/rejected submissions
some place to give a quick showcase.

Things that didn't work out perfectly this year and that we'll try to
get better at next year:
- Lots of people missed the submission deadline and their talks were
rejected only because of that. We'll do better PR by sending out a
pile of reminders.
- Comparitively few people asked for travel assistance. No idea
whether this was a year with more budget around, or whether this isn't
all that well know and we need to make more PR in maybe the call for
papers about it.

But that's just the stuff we've gathered already, we'd like to hear
more feedback. Just reply to this mail or send a mail to
bo...@foundation.x.org if you don't want the entire world to read it.
And if you want to send at pseudonymous feedback, drop a pastebin onto
the #xf-bod channel on the OFTC irc server.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC PATCH] radeonsi: flush DB caches when a Z/stencil buffer is attached

2017-09-26 Thread Juan A. Suarez Romero
On Tue, 2017-07-25 at 17:56 +0200, Samuel Pitoiset wrote:
> 
> On 07/25/2017 05:51 PM, Nicolai Hähnle wrote:
> > On 25.07.2017 15:29, Samuel Pitoiset wrote:
> > > This is a workaround which fixes a rendering issue with Dawn
> > > Of War III in full bindless mode because a depth texture most
> > > likely doesn't invoke the decompression pass when it should.
> > 
> > So do I understand correctly that the previous depth target is used 
> > bindless, and for whatever reason the si_decompress_depth doesn't 
> > properly trigger for bindless? Why doesn't it trigger?
> 
> Exactly, but I still don't know why it is not triggered correctly.

Hey, Samuel!

Are you investigating the issue Nicolai raised?

Asking because this patch didn't get any R-b, so I'm wondering if this
work in progress.

Thanks

J.A.

> 
> > 
> > Cheers,
> > Nicolai
> > 
> > > 
> > > Performance drops by 1% which doesn't really matter.
> > > 
> > > Fixes: 2263610827 ("radeonsi: flush DB caches only when transitioning 
> > > from DB to texturing")
> > > Signed-off-by: Samuel Pitoiset 
> > > Cc: "17.2" 
> > > ---
> > >   src/gallium/drivers/radeonsi/si_state.c | 10 ++
> > >   1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/src/gallium/drivers/radeonsi/si_state.c 
> > > b/src/gallium/drivers/radeonsi/si_state.c
> > > index 7e3d1a02e0..675b61ad7f 100644
> > > --- a/src/gallium/drivers/radeonsi/si_state.c
> > > +++ b/src/gallium/drivers/radeonsi/si_state.c
> > > @@ -2546,6 +2546,16 @@ static void si_set_framebuffer_state(struct 
> > > pipe_context *ctx,
> > >   }
> > >   sctx->b.flags |= SI_CONTEXT_CS_PARTIAL_FLUSH;
> > > +/* Flush DB caches when a Z/stencil buffer is attached to the
> > > + * framebuffer. This is a workaround which fixes a rendering 
> > > issue with
> > > + * Dawn Of War III in full bindless mode because a depth texture 
> > > most
> > > + * likely doesn't invoke the decompression pass when it should.
> > > + *
> > > + * TODO: Figure out a better fix.
> > > + */
> > > +if (sctx->framebuffer.state.zsbuf)
> > > +sctx->b.flags |= SI_CONTEXT_FLUSH_AND_INV_DB;
> > > +
> > >   /* u_blitter doesn't invoke depth decompression when it does 
> > > multiple
> > >* blits in a row, but the only case when it matters for DB is when
> > >* doing generate_mipmap. So here we flush DB manually between
> > > 
> > 
> > 
> 
> ___
> 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] [Mesa-stable] [PATCH 2/2] egl/drm: set the VISUAL_TYPE alongside the VISUAL_ID

2017-09-26 Thread Juan A. Suarez Romero
On Tue, 2017-08-22 at 09:20 +0100, Daniel Stone wrote:
> Hi,
> 
> On 21 August 2017 at 18:30, Emil Velikov  wrote:
> > On 21 August 2017 at 15:44, Daniel Stone  wrote:
> > > My take on it is that the visual types are defined by the platform,
> > > and 0 is a perfectly sensible visual type for a platform which does
> > > not actually have any.
> > > 
> > 
> > Having the exact same visual type for all visual IDs does strikes me
> > as a bit odd.
> > That said, the spec does not explicitly forbids it so I guess it should be 
> > fine?
> 
> It should be, yeah. It would be quite odd to use anything other than
> TrueColor for X11, so in effect that only has one value for the type
> either. Maybe just imagine '#define GBM_VISUAL_TYPE_FOURCC 0'
> existing, with no others to ever be defined, and then it might make
> more sense?
> 

Hello, people.

What's the status of this patch? It was tagged as candidate for stable,
and hence I'd like to know if requires changes or a R-b.

Thanks in advance!

J.A.



> Cheers,
> Daniel
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 4/8] egl: rework input validation order in _eglCreateWindowSurfaceCommon

2017-09-26 Thread Juan A. Suarez Romero
On Wed, 2017-09-06 at 15:07 +0100, Emil Velikov wrote:
> On 5 August 2017 at 00:25, Emil Velikov  wrote:
> > From: Emil Velikov 
> > 
> > As mentioned in previous commit the negative tests in dEQP expect the
> > arguments to be evaluated in particular order.
> > 
> > Namely - first the dpy, then the config, followed by the surface/window.
> > 
> > Move the check further down or executing the test below will produce
> > the following error.
> > 
> >dEQP-EGL.functional.negative_api.create_pbuffer_surface
> > 
> > 
> >
> >   eglCreateWindowSurface(0x9bfff0f150, 0x, 
> > 0x, { EGL_NONE });
> >   // 0x returned
> >   // ERROR expected: EGL_BAD_CONFIG, Got: 
> > EGL_BAD_NATIVE_WINDOW
> >
> > 
> 
> FTR dEQP has been "fixed" (by removing the test all together) to not
> generate the above error. At the same the Pixman equivalent is still
> buggy, hence the v2 of commit
> df8efd5b74d45e2bfb977a92dcd3db86abd6b143.
> 

Emil, does it mean this patch is superseded? I'm interesting in knowing
the situation as this was tagged to be included in stable release.

Thanks in advance.


J.A.

> I've sent patches [1] to restore the test and fix the buggy ones.
> 
> As those land, I would love to have symmetrical Window/Pixman paths. Be that:
>  - merging this patch (with polished commit message), or
>  - updating the Window path to be like this one.
> 
> I have no preference on which one ;-)
> 
> Thanks
> Emil
> 
> [1] 
> https://android-review.googlesource.com/#/q/owner:emil.l.velikov%2540gmail.com
> ___
> 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] [RFC PATCH] radeonsi: flush DB caches when a Z/stencil buffer is attached

2017-09-26 Thread Samuel Pitoiset



On 09/26/2017 06:57 PM, Juan A. Suarez Romero wrote:

On Tue, 2017-07-25 at 17:56 +0200, Samuel Pitoiset wrote:


On 07/25/2017 05:51 PM, Nicolai Hähnle wrote:

On 25.07.2017 15:29, Samuel Pitoiset wrote:

This is a workaround which fixes a rendering issue with Dawn
Of War III in full bindless mode because a depth texture most
likely doesn't invoke the decompression pass when it should.


So do I understand correctly that the previous depth target is used
bindless, and for whatever reason the si_decompress_depth doesn't
properly trigger for bindless? Why doesn't it trigger?


Exactly, but I still don't know why it is not triggered correctly.


Hey, Samuel!

Are you investigating the issue Nicolai raised?


Hey,

This has been fixed by Marek.

f4d095cc651af005d5760aa9dd06e6ae7007fab6

Thanks.



Asking because this patch didn't get any R-b, so I'm wondering if this
work in progress.

Thanks

J.A.





Cheers,
Nicolai



Performance drops by 1% which doesn't really matter.

Fixes: 2263610827 ("radeonsi: flush DB caches only when transitioning
from DB to texturing")
Signed-off-by: Samuel Pitoiset 
Cc: "17.2" 
---
   src/gallium/drivers/radeonsi/si_state.c | 10 ++
   1 file changed, 10 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_state.c
b/src/gallium/drivers/radeonsi/si_state.c
index 7e3d1a02e0..675b61ad7f 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -2546,6 +2546,16 @@ static void si_set_framebuffer_state(struct
pipe_context *ctx,
   }
   sctx->b.flags |= SI_CONTEXT_CS_PARTIAL_FLUSH;
+/* Flush DB caches when a Z/stencil buffer is attached to the
+ * framebuffer. This is a workaround which fixes a rendering
issue with
+ * Dawn Of War III in full bindless mode because a depth texture
most
+ * likely doesn't invoke the decompression pass when it should.
+ *
+ * TODO: Figure out a better fix.
+ */
+if (sctx->framebuffer.state.zsbuf)
+sctx->b.flags |= SI_CONTEXT_FLUSH_AND_INV_DB;
+
   /* u_blitter doesn't invoke depth decompression when it does
multiple
* blits in a row, but the only case when it matters for DB is when
* doing generate_mipmap. So here we flush DB manually between






___
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] [Mesa-stable] [RFC PATCH] radeonsi: flush DB caches when a Z/stencil buffer is attached

2017-09-26 Thread Juan A. Suarez Romero
On Tue, 2017-09-26 at 19:11 +0200, Samuel Pitoiset wrote:
> 
> On 09/26/2017 06:57 PM, Juan A. Suarez Romero wrote:
> > On Tue, 2017-07-25 at 17:56 +0200, Samuel Pitoiset wrote:
> > > 
> > > On 07/25/2017 05:51 PM, Nicolai Hähnle wrote:
> > > > On 25.07.2017 15:29, Samuel Pitoiset wrote:
> > > > > This is a workaround which fixes a rendering issue with Dawn
> > > > > Of War III in full bindless mode because a depth texture most
> > > > > likely doesn't invoke the decompression pass when it should.
> > > > 
> > > > So do I understand correctly that the previous depth target is used
> > > > bindless, and for whatever reason the si_decompress_depth doesn't
> > > > properly trigger for bindless? Why doesn't it trigger?
> > > 
> > > Exactly, but I still don't know why it is not triggered correctly.
> > 
> > Hey, Samuel!
> > 
> > Are you investigating the issue Nicolai raised?
> 
> Hey,
> 
> This has been fixed by Marek.
> 
> f4d095cc651af005d5760aa9dd06e6ae7007fab6
> 
> Thanks.
> 

Sweet! Thank you very much!


J.A.

> > 
> > Asking because this patch didn't get any R-b, so I'm wondering if this
> > work in progress.
> > 
> > Thanks
> > 
> > J.A.
> > 
> > > 
> > > > 
> > > > Cheers,
> > > > Nicolai
> > > > 
> > > > > 
> > > > > Performance drops by 1% which doesn't really matter.
> > > > > 
> > > > > Fixes: 2263610827 ("radeonsi: flush DB caches only when transitioning
> > > > > from DB to texturing")
> > > > > Signed-off-by: Samuel Pitoiset 
> > > > > Cc: "17.2" 
> > > > > ---
> > > > >src/gallium/drivers/radeonsi/si_state.c | 10 ++
> > > > >1 file changed, 10 insertions(+)
> > > > > 
> > > > > diff --git a/src/gallium/drivers/radeonsi/si_state.c
> > > > > b/src/gallium/drivers/radeonsi/si_state.c
> > > > > index 7e3d1a02e0..675b61ad7f 100644
> > > > > --- a/src/gallium/drivers/radeonsi/si_state.c
> > > > > +++ b/src/gallium/drivers/radeonsi/si_state.c
> > > > > @@ -2546,6 +2546,16 @@ static void si_set_framebuffer_state(struct
> > > > > pipe_context *ctx,
> > > > >}
> > > > >sctx->b.flags |= SI_CONTEXT_CS_PARTIAL_FLUSH;
> > > > > +/* Flush DB caches when a Z/stencil buffer is attached to the
> > > > > + * framebuffer. This is a workaround which fixes a rendering
> > > > > issue with
> > > > > + * Dawn Of War III in full bindless mode because a depth texture
> > > > > most
> > > > > + * likely doesn't invoke the decompression pass when it should.
> > > > > + *
> > > > > + * TODO: Figure out a better fix.
> > > > > + */
> > > > > +if (sctx->framebuffer.state.zsbuf)
> > > > > +sctx->b.flags |= SI_CONTEXT_FLUSH_AND_INV_DB;
> > > > > +
> > > > >/* u_blitter doesn't invoke depth decompression when it does
> > > > > multiple
> > > > > * blits in a row, but the only case when it matters for DB is 
> > > > > when
> > > > > * doing generate_mipmap. So here we flush DB manually between
> > > > > 
> > > > 
> > > > 
> > > 
> > > ___
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] XDC 2017 feedback

2017-09-26 Thread Andres Rodriguez

Hi,

A small piece of feedback from those of us watching remotely. It would 
be nice to have a simple to access index for the long livestream videos.


I think the XDC 2017 wiki page would be a good place for this 
information. A brief example:


Presentation Title | Presenter Name | Link with timestamp
---||-
...| ...| youtu.be/video-id?t=XhYmZs

Or alternatively, a list of hyperlinks with the presentation title as 
text that point to the correct timestamp in the video would also be 
sufficient.


Really enjoyed the talks, and would like them to be slightly easier to 
access and share with others.


Regards,
Andres


On 2017-09-26 12:57 PM, Daniel Vetter wrote:

Hi all,

First again big thanks to Stéphane and Jennifer for organizing a great XDC.

Like last year we'd like to hear feedback on how this year's XDC went,
both the good (and what you'd like to see more of) and the not so
good. Talk selection, organization, location, scheduling of talks,
anything really.

Here's a few things we took into account from Helsinki and tried to apply:
- More breaks for more hallway track.
- Try to schedule the hot topics on the first day (did we pick the
right ones) for better hallway track.
- More lightning talk time to give all the late/rejected submissions
some place to give a quick showcase.

Things that didn't work out perfectly this year and that we'll try to
get better at next year:
- Lots of people missed the submission deadline and their talks were
rejected only because of that. We'll do better PR by sending out a
pile of reminders.
- Comparitively few people asked for travel assistance. No idea
whether this was a year with more budget around, or whether this isn't
all that well know and we need to make more PR in maybe the call for
papers about it.

But that's just the stuff we've gathered already, we'd like to hear
more feedback. Just reply to this mail or send a mail to
bo...@foundation.x.org if you don't want the entire world to read it.
And if you want to send at pseudonymous feedback, drop a pastebin onto
the #xf-bod channel on the OFTC irc server.

Thanks, Daniel


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


Re: [Mesa-dev] [Mesa-stable] [PATCH 1/3] mesa: GL_TEXTURE_BORDER_COLOR exists in OpenGL 1.0, so don't depend on GL_ARB_texture_border_clamp

2017-09-26 Thread Juan A. Suarez Romero
On Thu, 2017-09-21 at 10:39 -0500, Ian Romanick wrote:
> On 09/20/2017 03:12 AM, Juan A. Suarez Romero wrote:
> > On Sat, 2017-07-08 at 02:03 +0300, Andres Gomez wrote:
> > > Ian, it looks like we could want this patch (and the others from the
> > > series when they land) in -stable (?)
> > > 
> > 
> > As we are preparing a new stable 17.1 release, gently pinging.
> > 
> 
> I completely forgot about this series... thanks for the reminder. :)
> 


Ian, patch 1/3 is already included in 17.2 stable.

Probably including 2/3 and 3/3 could be a good idea. Wdyt? Do include
them in next stable release?

Thanks


> > J.A.
> > 
> > > On Tue, 2017-06-27 at 10:09 -0700, Ian Romanick wrote:
> > > > From: Ian Romanick 
> > > > 
> > > > On NV20 (and probably also on earlier NV GPUs that lack
> > > > GL_ARB_texture_border_clamp) fixes the following piglit tests:
> > > > 
> > > > gl-1.0-beginend-coverage gltexparameter[if]{v,}
> > > > push-pop-texture-state
> > > > texwrap 1d
> > > > texwrap 1d proj
> > > > texwrap 2d proj
> > > > texwrap formats
> > > > 
> > > > All told, 49 more tests pass on NV20 (10de:0201).
> > > > 
> > > > No changes on Intel CI run or RV250 (1002:4c66).
> > > > 
> > > > Signed-off-by: Ian Romanick 
> > > > ---
> > > >  src/mesa/main/texparam.c | 10 +-
> > > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
> > > > index 3c110de..857faf6 100644
> > > > --- a/src/mesa/main/texparam.c
> > > > +++ b/src/mesa/main/texparam.c
> > > > @@ -736,8 +736,16 @@ set_tex_parameterf(struct gl_context *ctx,
> > > >break;
> > > >  
> > > > case GL_TEXTURE_BORDER_COLOR:
> > > > +  /* Border color exists in desktop OpenGL since 1.0 for GL_CLAMP. 
> > > >  In
> > > > +   * OpenGL ES 2.0+, it only exists in when 
> > > > GL_OES_texture_border_clamp is
> > > > +   * enabled.  It is never available in OpenGL ES 1.x.
> > > > +   *
> > > > +   * FIXME: Every driver that supports GLES2 has this extension.  
> > > > Elide
> > > > +   * the check?
> > > > +   */
> > > >if (ctx->API == API_OPENGLES ||
> > > > -  !ctx->Extensions.ARB_texture_border_clamp)
> > > > +  (ctx->API == API_OPENGLES2 &&
> > > > +   !ctx->Extensions.ARB_texture_border_clamp))
> > > >   goto invalid_pname;
> > > >  
> > > >if 
> > > > (!_mesa_target_allows_setting_sampler_parameters(texObj->Target))
> 
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [Mesa-stable] [PATCH] egl/dri2: Implement swapInterval fallback in a conformant way (v2)

2017-09-26 Thread Juan A. Suarez Romero
On Mon, 2017-09-25 at 16:25 +0900, Tomasz Figa wrote:
> On Fri, Aug 11, 2017 at 1:31 PM, Tomasz Figa  wrote:
> > On Fri, Aug 11, 2017 at 2:29 AM, Emil Velikov  
> > wrote:
> > > On 10 August 2017 at 14:59, Tomasz Figa  wrote:
> > > > dri2_fallback_swap_interval() currently used to stub out swap interval
> > > > support in Android backend does nothing besides returning EGL_FALSE.
> > > > This causes at least one known application (Android Snapchat) to fail
> > > > due to an unexpected error and my loose interpretation of the EGL 1.5
> > > > specification justifies it. Relevant quote below:
> > > > 
> > > > The function
> > > > 
> > > > EGLBoolean eglSwapInterval(EGLDisplay dpy, EGLint interval);
> > > > 
> > > > specifies the minimum number of video frame periods per buffer swap
> > > > for the draw surface of the current context, for the current 
> > > > rendering
> > > > API. [...]
> > > > 
> > > > The parameter interval specifies the minimum number of video frames
> > > > that are displayed before a buffer swap will occur. The interval
> > > > specified by the function applies to the draw surface bound to the
> > > > context that is current on the calling thread. [...] interval is
> > > > silently clamped to minimum and maximum implementation dependent
> > > > values before being stored; these values are defined by EGLConfig
> > > > attributes EGL_MIN_SWAP_INTERVAL and EGL_MAX_SWAP_INTERVAL
> > > > respectively.
> > > > 
> > > > The default swap interval is 1.
> > > > 
> > > > Even though it does not specify the exact behavior if the platform does
> > > > not support changing the swap interval, the default assumed state is the
> > > > swap interval of 1, which I interpret as a value that eglSwapInterval()
> > > > should succeed if called with, even if there is no ability to change the
> > > > interval (but there is no change requested). Moreover, since the
> > > > behavior is defined to clamp the requested value to minimum and maximum
> > > > and at least the default value of 1 must be present in the range, the
> > > > implementation might be expected to have a valid range, which in case of
> > > > the feature being unsupported, would correspond to {1} and any request
> > > > might be expected to be clamped to this value.
> > > > 
> > > > Fix this by defaulting dri2_dpy's min_swap_interval, max_swap_interval
> > > > and default_swap_interval to 1 in dri2_setup_screen() and let platforms,
> > > > which support this functionality set their own values after this
> > > > function returns. Thanks to patches merged earlier, we can also remove
> > > > the dri2_fallback_swap_interval() completely, as with a singular range
> > > > it would not be called anyway.
> > > > 
> > > > v2: Remove dri2_fallback_swap_interval() completely thanks to higher
> > > > layer already clamping the requested interval and not calling the
> > > > driver layer if the clamped value is the same as current.
> > > > 
> > > 
> > > Not every driver is EGL 1.5, although the 1.4 spec has exact same text.
> > > 
> > > Reviewed-by: Emil Velikov 
> > > 
> > > We could have this in stable, but will need a clamp like v1 since
> > > Eric's rework did not land there.
> > > Gents, how do you feel on the topic?
> > 
> > FWIW, Eric's patches applied cleanly onto our chromium branch, which
> > was based on something not very far away from what 17.2 branched at.
> > If for some reasons we prefer to avoid further cherry picks, we can go
> > with my v1 for stable, which doesn't depend on them.
> 
> Gentle ping. :)
> 

Tomasz, didn't Chad and Tapani granted a R-b?

J.A.

> Thanks,
> Tomasz
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/3] radeonsi: fix border color translation for integer textures

2017-09-26 Thread Nicolai Hähnle

On 26.09.2017 18:55, Gustaw Smolarczyk wrote:

2017-09-26 16:46 GMT+02:00 Nicolai Hähnle :

From: Nicolai Hähnle 

This fixes the extremely unlikely case that an application uses
0x8000 or 0x3f80 as border color for an integer texture and
helps in the also, but perhaps slightly less, unlikely case that 1 is
used as a border color.


Hi,

I see that it fixes the wrong optimization in si_translate_border_color.

However, I also see that for floating point textures this will change
-0.0 into 0.0 (if I understand how
V_008F3C_SQ_TEX_BORDER_COLOR_*_BLACK work). I don't know GL rules
enough to judge that (whether 0.0 and -0.0 should be
indistinguishable), but is that ok?


Good question :)

There is actually a tiny bit of evidence in the spec that GLSL care 
slightly about the sign of zero, though IMO it's a bit silly to assume 
you're getting the correct sign for zero when you're not even guaranteed 
to get denormals. Personally, I wouldn't worry about it.


Cheers,
Nicolai




Regards,
Gustaw Smolarczyk



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


[Mesa-dev] [PATCH] glx: Be slightly more tolerant in glXImportContext

2017-09-26 Thread Adam Jackson
Ugh the GLX code. __GLX_MAX_CONTEXT_PROPS is 3 because glxproto.h is
just a pile of ancient runes, so when the server begins sending more
than 3 context properties this code refuses to work _at all_.

This extension is pretty obscure, so this probably isn't a big deal (if
it was we'd need to hack the server side to send fewer properties unless
the client seemed to be "new enough"). And we're probably not going to
ever have tremendous numbers of context properties. So bump the number
we'll accept in a reply out to 32 (since we still want to avoid overflow
etc.) and copy in just the ones we know about, as since these will be
indirect contexts the client library probably needs to be explicitly
aware of any new context state.

Signed-off-by: Adam Jackson 
---
 src/glx/glxcmds.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
index 29b94b8810..fd917671eb 100644
--- a/src/glx/glxcmds.c
+++ b/src/glx/glxcmds.c
@@ -1395,6 +1395,16 @@ GLX_ALIAS(Display *, glXGetCurrentDisplayEXT, (void), (),
   glXGetCurrentDisplay)
 
 #ifndef GLX_USE_APPLEGL
+
+/*
+ * This is just a random large number, so that the server can start sending
+ * more properties and we won't arbitrarily reject it, but we still won't save
+ * any attributes we don't know about, since the correct usage of an imported
+ * (and thus indirect) context almost certainly requires explicit awareness in
+ * libGL of any extensions in use. Right now we know about 5, 32 should be
+ * plenty of headroom.
+ */
+#define MAX_CONTEXT_PROPS 32
 _GLX_PUBLIC GLXContext
 glXImportContextEXT(Display *dpy, GLXContextID contextID)
 {
@@ -1403,11 +1413,7 @@ glXImportContextEXT(Display *dpy, GLXContextID contextID)
xGLXQueryContextReply reply;
CARD8 opcode;
struct glx_context *ctx;
-
-   /* This GLX implementation knows about 5 different properties, so
-* allow the server to send us one of each.
-*/
-   int propList[5 * 2], *pProp, nPropListBytes;
+   int propList[MAX_CONTEXT_PROPS * 2], *pProp, nPropListBytes;
int numProps;
int i, renderType;
XID share;
@@ -1471,7 +1477,7 @@ glXImportContextEXT(Display *dpy, GLXContextID contextID)
 
_XReply(dpy, (xReply *) & reply, 0, False);
 
-   if (reply.n <= __GLX_MAX_CONTEXT_PROPS)
+   if (reply.n <= MAX_CONTEXT_PROPS)
   nPropListBytes = reply.n * 2 * sizeof propList[0];
else
   nPropListBytes = 0;
-- 
2.14.1

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


Re: [Mesa-dev] [PATCH 1/2] swr: Handle resource across context changes

2017-09-26 Thread Cherniak, Bruce
Reviewed-by: Bruce Cherniak  

> On Sep 25, 2017, at 5:28 PM, George Kyriazis  
> wrote:
> 
> Swr caches fb contents in tiles.  Those tiles are stored on a per-context
> basis.
> 
> When switching contexts that share resources we need to make sure that
> the tiles of the old context are being stored and the tiles of the new
> context are being invalidated (marked as invalid, hence contents need
> to be reloaded).
> 
> The context does not get any dirty bits to identify this case.  This has
> to be, then, coordinated by the resources that are being shared between
> the contexts.
> 
> Add a "curr_pipe" hook in swr_resource that will allow us to identify a
> MakeCurrent of the above form during swr_update_derived().  At that time,
> we invalidate the tiles of the new context.  The old context, will need to
> have already store its tiles by that time, which happens during glFlush().
> glFlush() is being called at the beginning of MakeCurrent.
> 
> So, the sequence of operations is:
> - At the beginning of glXMakeCurrent(), glFlush() will store the tiles
>  of all bound surfaces of the old context.
> - After the store, a fence will guarantee that the all tile store make
>  it to the surface
> - During swr_update_derived(), when we validate the new context, we check
>  all resources to see what changed, and if so, we invalidate the
>  current tiles.
> 
> Fixes rendering problems with CEI/Ensight.
> ---
> src/gallium/drivers/swr/swr_context.cpp | 14 +--
> src/gallium/drivers/swr/swr_draw.cpp| 19 --
> src/gallium/drivers/swr/swr_resource.h  |  3 +++
> src/gallium/drivers/swr/swr_state.cpp   | 44 +
> 4 files changed, 70 insertions(+), 10 deletions(-)
> 
> diff --git a/src/gallium/drivers/swr/swr_context.cpp 
> b/src/gallium/drivers/swr/swr_context.cpp
> index e95bd3b..34d9a25 100644
> --- a/src/gallium/drivers/swr/swr_context.cpp
> +++ b/src/gallium/drivers/swr/swr_context.cpp
> @@ -365,10 +365,20 @@ swr_destroy(struct pipe_context *pipe)
>   util_blitter_destroy(ctx->blitter);
> 
>for (unsigned i = 0; i < PIPE_MAX_COLOR_BUFS; i++) {
> -  pipe_surface_reference(&ctx->framebuffer.cbufs[i], NULL);
> +  if (ctx->framebuffer.cbufs[i]) {
> + struct swr_resource *res = 
> swr_resource(ctx->framebuffer.cbufs[i]->texture);
> + /* NULL curr_pipe, so we don't have a reference to a deleted pipe */
> + res->curr_pipe = NULL;
> + pipe_surface_reference(&ctx->framebuffer.cbufs[i], NULL);
> +  }
>}
> 
> -   pipe_surface_reference(&ctx->framebuffer.zsbuf, NULL);
> +   if (ctx->framebuffer.zsbuf) {
> +  struct swr_resource *res = 
> swr_resource(ctx->framebuffer.zsbuf->texture);
> +  /* NULL curr_pipe, so we don't have a reference to a deleted pipe */
> +  res->curr_pipe = NULL;
> +  pipe_surface_reference(&ctx->framebuffer.zsbuf, NULL);
> +   }
> 
>for (unsigned i = 0; i < ARRAY_SIZE(ctx->sampler_views[0]); i++) {
>   
> pipe_sampler_view_reference(&ctx->sampler_views[PIPE_SHADER_FRAGMENT][i], 
> NULL);
> diff --git a/src/gallium/drivers/swr/swr_draw.cpp 
> b/src/gallium/drivers/swr/swr_draw.cpp
> index d7f24d6..57660c7 100644
> --- a/src/gallium/drivers/swr/swr_draw.cpp
> +++ b/src/gallium/drivers/swr/swr_draw.cpp
> @@ -239,14 +239,17 @@ swr_flush(struct pipe_context *pipe,
> {
>struct swr_context *ctx = swr_context(pipe);
>struct swr_screen *screen = swr_screen(pipe->screen);
> -   struct pipe_surface *cb = ctx->framebuffer.cbufs[0];
> -
> -   /* If the current renderTarget is the display surface, store tiles back to
> -* the surface, in preparation for present (swr_flush_frontbuffer).
> -* Other renderTargets get stored back when attachment changes or
> -* swr_surface_destroy */
> -   if (cb && swr_resource(cb->texture)->display_target)
> -  swr_store_dirty_resource(pipe, cb->texture, SWR_TILE_RESOLVED);
> +
> +   for (int i=0; i < ctx->framebuffer.nr_cbufs; i++) {
> +  struct pipe_surface *cb = ctx->framebuffer.cbufs[i];
> +  if (cb) {
> + swr_store_dirty_resource(pipe, cb->texture, SWR_TILE_RESOLVED);
> +  }
> +   }
> +   if (ctx->framebuffer.zsbuf) {
> +  swr_store_dirty_resource(pipe, ctx->framebuffer.zsbuf->texture,
> +   SWR_TILE_RESOLVED);
> +   }
> 
>if (fence)
>   swr_fence_reference(pipe->screen, fence, screen->flush_fence);
> diff --git a/src/gallium/drivers/swr/swr_resource.h 
> b/src/gallium/drivers/swr/swr_resource.h
> index 4a2d669..1269433 100644
> --- a/src/gallium/drivers/swr/swr_resource.h
> +++ b/src/gallium/drivers/swr/swr_resource.h
> @@ -54,6 +54,9 @@ struct swr_resource {
>size_t secondary_mip_offsets[PIPE_MAX_TEXTURE_LEVELS];
> 
>enum swr_resource_status status;
> +
> +   /* last pipe that used (validated) this resource */
> +   struct pipe_context *curr_pipe;
> };
> 
> 
> diff --git a/src/gallium/drivers/swr/swr_state.cpp 
> b/src/gallium/drivers/swr/swr_state.cpp
> index 93108d

Re: [Mesa-dev] [PATCH 2/2] swr: Remove unneeeded comparison

2017-09-26 Thread Cherniak, Bruce
Reviewed-by: Bruce Cherniak  

> On Sep 25, 2017, at 5:28 PM, George Kyriazis  
> wrote:
> 
> No need to check if screen->pipe != pipe, so we can just assign it.  Just do 
> it.
> ---
> src/gallium/drivers/swr/swr_state.cpp | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/gallium/drivers/swr/swr_state.cpp 
> b/src/gallium/drivers/swr/swr_state.cpp
> index 893bd6e..c6da4fc 100644
> --- a/src/gallium/drivers/swr/swr_state.cpp
> +++ b/src/gallium/drivers/swr/swr_state.cpp
> @@ -1074,8 +1074,7 @@ swr_update_derived(struct pipe_context *pipe,
>}
> 
>/* Update screen->pipe to current pipe context. */
> -   if (screen->pipe != pipe)
> -  screen->pipe = pipe;
> +   screen->pipe = pipe;
> 
>/* Any state that requires dirty flags to be re-triggered sets this mask */
>/* For example, user_buffer vertex and index buffers. */
> -- 
> 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


Re: [Mesa-dev] [PATCH] vc4: Fix infinite retry in vc4_bo_alloc()

2017-09-26 Thread Eric Engestrom
On Tuesday, 2017-09-26 07:48:37 +, Boris Brezillon wrote:
> cleared_and_retried is always reset to false when jumping to the retry
> label, thus leading to an infinite retry loop.
> 
> Fix that by moving the cleared_and_retried variable definitions at the
> beginning of the function.

Reviewed-by: Eric Engestrom 
Fixes: 78087676c98aa8884ba92 "vc4: Restructure the simulator mode."
Cc: Eric Anholt 

> While we're at it, move the create variable with the other local
> variables and explicitly reset its content in the retry path.

That was actually changed the other way around by the commit which
introduced the bug (78087676c98aa8884ba92); you should probably wait for
Eric Anholt's reply (cc'ed) on this before reverting that.

(you might want to split the two changes into two patches)

> 
> Signed-off-by: Boris Brezillon 
> ---
>  src/gallium/drivers/vc4/vc4_bufmgr.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/src/gallium/drivers/vc4/vc4_bufmgr.c 
> b/src/gallium/drivers/vc4/vc4_bufmgr.c
> index 12af7f8a9ef2..0653f8823232 100644
> --- a/src/gallium/drivers/vc4/vc4_bufmgr.c
> +++ b/src/gallium/drivers/vc4/vc4_bufmgr.c
> @@ -123,6 +123,8 @@ vc4_bo_from_cache(struct vc4_screen *screen, uint32_t 
> size, const char *name)
>  struct vc4_bo *
>  vc4_bo_alloc(struct vc4_screen *screen, uint32_t size, const char *name)
>  {
> +bool cleared_and_retried = false;
> +struct drm_vc4_create_bo create;
>  struct vc4_bo *bo;
>  int ret;
>  
> @@ -149,12 +151,8 @@ vc4_bo_alloc(struct vc4_screen *screen, uint32_t size, 
> const char *name)
>  bo->private = true;
>  
>   retry:
> -;
> -
> -bool cleared_and_retried = false;
> -struct drm_vc4_create_bo create = {
> -.size = size
> -};
> +memset(&create, 0, sizeof(create));
> +create.size = size;
>  
>  ret = vc4_ioctl(screen->fd, DRM_IOCTL_VC4_CREATE_BO, &create);
>  bo->handle = create.handle;
> -- 
> 2.11.0
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/5] st/glsl_to_tgsi: fix conditional assignments to packed shader outputs

2017-09-26 Thread Ian Romanick
I'm hoping to land my patch series that removes ir_assignment::condition
either today or tomorrow.  I believe that series deletes all of this
code... but this patch would still be useful for stable.

On 09/26/2017 07:42 AM, Nicolai Hähnle wrote:
> From: Nicolai Hähnle 
> 
> Overriding the default (no-op) swizzle is clearly counter-productive,
> since the whole point is putting the destination register as one of
> the source operands so that it remains unmodified when the assignment
> condition is false.
> 
> Fragment depth and stencil outputs are a special case due to how their
> source swizzles are manipulated in translate_src when compiling to
> TGSI.
> 
> Fixes dEQP-GLES2.functional.shaders.conditionals.if.*_vertex
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index f4870a1c606..0daf5a14285 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -2890,21 +2890,29 @@ glsl_to_tgsi_visitor::emit_block_mov(ir_assignment 
> *ir, const struct glsl_type *
>}
>return;
> }
>  
> assert(type->is_scalar() || type->is_vector());
>  
> l->type = type->base_type;
> r->type = type->base_type;
> if (cond) {
>st_src_reg l_src = st_src_reg(*l);
> -  l_src.swizzle = swizzle_for_size(type->vector_elements);
> +
> +  if (l_src.file == PROGRAM_OUTPUT &&
> +  this->prog->Target == GL_FRAGMENT_PROGRAM_ARB &&
> +  (l_src.index == FRAG_RESULT_DEPTH || l_src.index == 
> FRAG_RESULT_STENCIL)) {
> + /* This is a special case because the source swizzles will be 
> shifted
> +  * later to account for the difference between GLSL (where they're
> +  * plain floats) and TGSI (where they're Z and Y components). */
> + l_src.swizzle = SWIZZLE_;
> +  }
>  
>if (native_integers) {
>   emit_asm(ir, TGSI_OPCODE_UCMP, *l, *cond,
>cond_swap ? l_src : *r,
>cond_swap ? *r : l_src);
>} else {
>   emit_asm(ir, TGSI_OPCODE_CMP, *l, *cond,
>cond_swap ? l_src : *r,
>cond_swap ? *r : l_src);
>}
> 

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


Re: [Mesa-dev] [Mesa-stable] [PATCH 1/3] mesa: GL_TEXTURE_BORDER_COLOR exists in OpenGL 1.0, so don't depend on GL_ARB_texture_border_clamp

2017-09-26 Thread Ian Romanick
On 09/26/2017 10:27 AM, Juan A. Suarez Romero wrote:
> On Thu, 2017-09-21 at 10:39 -0500, Ian Romanick wrote:
>> On 09/20/2017 03:12 AM, Juan A. Suarez Romero wrote:
>>> On Sat, 2017-07-08 at 02:03 +0300, Andres Gomez wrote:
 Ian, it looks like we could want this patch (and the others from the
 series when they land) in -stable (?)

>>>
>>> As we are preparing a new stable 17.1 release, gently pinging.
>>>
>>
>> I completely forgot about this series... thanks for the reminder. :)
> 
> Ian, patch 1/3 is already included in 17.2 stable.
> 
> Probably including 2/3 and 3/3 could be a good idea. Wdyt? Do include
> them in next stable release?

Patch 2 fixes a problem, but I wasn't able to observe any changes in
piglit.  I'm not that worried about it.  I think we can leave it out.

Patch 3 enables a new feature, so it's not a candidate.

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


Re: [Mesa-dev] XDC 2017 feedback

2017-09-26 Thread Manasi Navare
Hi,

XDC was a really great conference and loved the fact that it was
in just one room which kept all the hallway conversations in that room resulting
into more networking.
But I agree with Andres that for the videos, it would be great to split the
huge youtube video stream per presentation and have seperate links for each
talk somewhere on the XDC page.

Regards
Manasi



On Tue, Sep 26, 2017 at 01:25:15PM -0400, Andres Rodriguez wrote:
> Hi,
> 
> A small piece of feedback from those of us watching remotely. It would be
> nice to have a simple to access index for the long livestream videos.
> 
> I think the XDC 2017 wiki page would be a good place for this information. A
> brief example:
> 
> Presentation Title | Presenter Name | Link with timestamp
> ---||-
> ...| ...| youtu.be/video-id?t=XhYmZs
> 
> Or alternatively, a list of hyperlinks with the presentation title as text
> that point to the correct timestamp in the video would also be sufficient.
> 
> Really enjoyed the talks, and would like them to be slightly easier to
> access and share with others.
> 
> Regards,
> Andres
> 
> 
> On 2017-09-26 12:57 PM, Daniel Vetter wrote:
> >Hi all,
> >
> >First again big thanks to Stéphane and Jennifer for organizing a great XDC.
> >
> >Like last year we'd like to hear feedback on how this year's XDC went,
> >both the good (and what you'd like to see more of) and the not so
> >good. Talk selection, organization, location, scheduling of talks,
> >anything really.
> >
> >Here's a few things we took into account from Helsinki and tried to apply:
> >- More breaks for more hallway track.
> >- Try to schedule the hot topics on the first day (did we pick the
> >right ones) for better hallway track.
> >- More lightning talk time to give all the late/rejected submissions
> >some place to give a quick showcase.
> >
> >Things that didn't work out perfectly this year and that we'll try to
> >get better at next year:
> >- Lots of people missed the submission deadline and their talks were
> >rejected only because of that. We'll do better PR by sending out a
> >pile of reminders.
> >- Comparitively few people asked for travel assistance. No idea
> >whether this was a year with more budget around, or whether this isn't
> >all that well know and we need to make more PR in maybe the call for
> >papers about it.
> >
> >But that's just the stuff we've gathered already, we'd like to hear
> >more feedback. Just reply to this mail or send a mail to
> >bo...@foundation.x.org if you don't want the entire world to read it.
> >And if you want to send at pseudonymous feedback, drop a pastebin onto
> >the #xf-bod channel on the OFTC irc server.
> >
> >Thanks, Daniel
> >
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v3 3/4] meson: Add build Intel "anv" vulkan driver

2017-09-26 Thread Eric Anholt
Dylan Baker  writes:

> This allows building and installing the Intel "anv" Vulkan driver using
> meson and ninja, the driver has been tested against the CTS and has
> seems to pass the same series of tests (they both segfault when the CTS
> tries to run wayland wsi tests).
>
> There are still a mess of TODO, XXX, and FIXME comments in here. Those
> are mostly for meson bugs I'm trying to fix, or for additional things to
> implement for other drivers/features.
>
> I have configured all intermediate libraries and optional tools to not
> build by default, meaning they will only be built if they're pulled in
> as a dependency of a target that will actually be installed) this allows
> us to avoid massive if chains, while ensuring that only the bits that
> need to be built are.

I am really looking forward to converting my vc5-vulkan work over to the
meson build.  Given the rate I have to rebase that branch, it would be
worth my time to convert over it in the next day of actual code
development I get to do.

So, I'll offer a few bits of feedback on meson usage, after which you
can have my

Reviewed-by: Eric Anholt 

so that we can start working in the tree together.

> diff --git a/meson.build b/meson.build
> new file mode 100644
> index 000..09e53957fe9
> --- /dev/null
> +++ b/meson.build
> @@ -0,0 +1,423 @@
> +# Copyright © 2017 Intel Corporation
> +
> +# Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> +# of this software and associated documentation files (the "Software"), to 
> deal
> +# in the Software without restriction, including without limitation the 
> rights
> +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> +# copies of the Software, and to permit persons to whom the Software is
> +# furnished to do so, subject to the following conditions:
> +
> +# The above copyright notice and this permission notice shall be included in
> +# all copies or substantial portions of the Software.
> +
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
> THE
> +# SOFTWARE.
> +
> +project('mesa', ['c', 'cpp'], version : '17.3.0-devel', license : 'MIT',
> +default_options : ['c_std=c99'])
> +
> +with_dri3 = true  # XXX: need a switch for this
> +with_vulkan_icd_dir = get_option('vulkan_icd_dir')
> +with_tests = get_option('build-tests')
> +with_valgrind = get_option('valgrind')
> +
> +# TODO: there are more platforms required for non-vulkan drivers
> +with_platform_wayland = false
> +with_platform_x11 = false
> +_platforms = get_option('platforms')
> +if _platforms != ''
> +  _split = _platforms.split(',')
> +  with_platform_x11 = _split.contains('x11')
> +  with_platform_wayland = _split.contains('wayland')
> +endif
> +
> +if with_vulkan_icd_dir == ''
> +  with_vulkan_icd_dir = join_paths(get_option('datadir'), 'vulkan/icd.d')
> +endif
> +
> +with_intel_vk = false
> +with_amd_vk = false
> +_vulkan_drivers = get_option('vulkan-drivers')
> +if _vulkan_drivers != ''
> +  _split = _vulkan_drivers.split(',')
> +  with_intel_vk = _split.contains('intel')
> +  with_amd_vk = _split.contains('amd')
> +  if not (with_platform_x11 or with_platform_wayland)
> +error('Vulkan requires at least one platform (x11, wayland)')
> +  endif
> +endif
> +
> +prog_python2 = find_program('python2')
> +
> +cc = meson.get_compiler('c')
> +if cc.get_id() == 'gcc' and cc.version().version_compare('< 4.4.6')
> +  error('When using GCC version 4.2.0 or later required.')
> +endif
> +
> +# Arguments for the preprocessor. These need to be added to both the C and 
> the
> +# C++ (cpp in meson terminology) arguments, so the they're calculated

"so that they're"

> +# separately
> +pre_args = ['-D__STDC_CONSTANT_MACROS', '-D__STDC_FORMAT_MACROS',
> +'-D__STDC_LIMIT_MACROS',
> +'-DVERSION="@0@"'.format(meson.project_version())]
> +
> +# Define debug for debug and debugoptimized builds

DEBUG

> +if get_option('buildtype').startswith('debug')
> +  pre_args += '-DDEBUG'
> +endif
> +


> +# check for dl support
> +if cc.has_function('dlopen')
> +  pre_args += '-DHAVE_DLOPEN'
> +  dep_dl = []
> +else
> +  dep_dl = cc.find_library('dl')

Note: The configure.ac path sets HAVE_DLOPEN if libdl is found.  Needed
for core mesa.

> +endif
> +
> +if not cc.has_function('dladdr', dependencies : dep_dl)
> +  error('dl library doesn\'t have dladdr')
> +endif
> +
> +if cc.has_function('dl_iterate_phdr')
> +  pre_args += '-DHAVE_DL_ITERATE_PHDR'
> +else
> +  # TODO: this is required for vulkan
> +endif
> +
> +# Determine whether or no

Re: [Mesa-dev] [PATCH] glx: Be slightly more tolerant in glXImportContext

2017-09-26 Thread Eric Anholt
Adam Jackson  writes:

> Ugh the GLX code. __GLX_MAX_CONTEXT_PROPS is 3 because glxproto.h is
> just a pile of ancient runes, so when the server begins sending more
> than 3 context properties this code refuses to work _at all_.
>
> This extension is pretty obscure, so this probably isn't a big deal (if
> it was we'd need to hack the server side to send fewer properties unless
> the client seemed to be "new enough"). And we're probably not going to
> ever have tremendous numbers of context properties. So bump the number
> we'll accept in a reply out to 32 (since we still want to avoid overflow
> etc.) and copy in just the ones we know about, as since these will be
> indirect contexts the client library probably needs to be explicitly
> aware of any new context state.
>
> Signed-off-by: Adam Jackson 
> ---
>  src/glx/glxcmds.c | 18 --
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index 29b94b8810..fd917671eb 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -1395,6 +1395,16 @@ GLX_ALIAS(Display *, glXGetCurrentDisplayEXT, (void), 
> (),
>glXGetCurrentDisplay)
>  
>  #ifndef GLX_USE_APPLEGL
> +
> +/*
> + * This is just a random large number, so that the server can start sending
> + * more properties and we won't arbitrarily reject it, but we still won't 
> save
> + * any attributes we don't know about, since the correct usage of an imported
> + * (and thus indirect) context almost certainly requires explicit awareness 
> in
> + * libGL of any extensions in use. Right now we know about 5, 32 should be
> + * plenty of headroom.
> + */
> +#define MAX_CONTEXT_PROPS 32
>  _GLX_PUBLIC GLXContext
>  glXImportContextEXT(Display *dpy, GLXContextID contextID)
>  {
> @@ -1403,11 +1413,7 @@ glXImportContextEXT(Display *dpy, GLXContextID 
> contextID)
> xGLXQueryContextReply reply;
> CARD8 opcode;
> struct glx_context *ctx;
> -
> -   /* This GLX implementation knows about 5 different properties, so
> -* allow the server to send us one of each.
> -*/
> -   int propList[5 * 2], *pProp, nPropListBytes;
> +   int propList[MAX_CONTEXT_PROPS * 2], *pProp, nPropListBytes;
> int numProps;
> int i, renderType;
> XID share;
> @@ -1471,7 +1477,7 @@ glXImportContextEXT(Display *dpy, GLXContextID 
> contextID)
>  
> _XReply(dpy, (xReply *) & reply, 0, False);
>  
> -   if (reply.n <= __GLX_MAX_CONTEXT_PROPS)
> +   if (reply.n <= MAX_CONTEXT_PROPS)
>nPropListBytes = reply.n * 2 * sizeof propList[0];
> else
>nPropListBytes = 0;

Instead of magic numbers, couldn't we just malloc the proplist here and
free it after the loop?


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] glsl: Allow precision mismatch on dead data with GLSL ES 1.00

2017-09-26 Thread Kenneth Graunke
This seems reasonable to me, and gets myReviewed-by: Kenneth Graunke but I would wait to hear back from Ian since he's been looking at this too.Thanks for researching this carefully!___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v3 4/4] meson: build "radv" vulkan driver for radeon hardware

2017-09-26 Thread Nirbheek Chauhan
On Tue, Sep 26, 2017 at 9:46 PM, Dylan Baker  wrote:
> This builds and installs, but I haven't had a chance to test it yet.
>
> v2: - enable radv by default
> - add shader cache support and enforce that it's built for radv
> v3: - Fix typo in meson_options (Nicholas)
> - strip trailing 'svn' from llvm version before setting the version
>   preprocessor flag (Bas)

Should this also be added to the LLVMDependency meson class?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/5] st/glsl_to_tgsi: fix conditional assignments to packed shader outputs

2017-09-26 Thread Nicolai Hähnle

On 26.09.2017 20:34, Ian Romanick wrote:

I'm hoping to land my patch series that removes ir_assignment::condition
either today or tomorrow.  I believe that series deletes all of this
code... but this patch would still be useful for stable.


Yep, that series is much appreciated. If it weren't for stable, I would 
have just waited :)


Cheers,
Nicolai



On 09/26/2017 07:42 AM, Nicolai Hähnle wrote:

From: Nicolai Hähnle 

Overriding the default (no-op) swizzle is clearly counter-productive,
since the whole point is putting the destination register as one of
the source operands so that it remains unmodified when the assignment
condition is false.

Fragment depth and stencil outputs are a special case due to how their
source swizzles are manipulated in translate_src when compiling to
TGSI.

Fixes dEQP-GLES2.functional.shaders.conditionals.if.*_vertex
Cc: mesa-sta...@lists.freedesktop.org
---
  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index f4870a1c606..0daf5a14285 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -2890,21 +2890,29 @@ glsl_to_tgsi_visitor::emit_block_mov(ir_assignment *ir, 
const struct glsl_type *
}
return;
 }
  
 assert(type->is_scalar() || type->is_vector());
  
 l->type = type->base_type;

 r->type = type->base_type;
 if (cond) {
st_src_reg l_src = st_src_reg(*l);
-  l_src.swizzle = swizzle_for_size(type->vector_elements);
+
+  if (l_src.file == PROGRAM_OUTPUT &&
+  this->prog->Target == GL_FRAGMENT_PROGRAM_ARB &&
+  (l_src.index == FRAG_RESULT_DEPTH || l_src.index == 
FRAG_RESULT_STENCIL)) {
+ /* This is a special case because the source swizzles will be shifted
+  * later to account for the difference between GLSL (where they're
+  * plain floats) and TGSI (where they're Z and Y components). */
+ l_src.swizzle = SWIZZLE_;
+  }
  
if (native_integers) {

   emit_asm(ir, TGSI_OPCODE_UCMP, *l, *cond,
cond_swap ? l_src : *r,
cond_swap ? *r : l_src);
} else {
   emit_asm(ir, TGSI_OPCODE_CMP, *l, *cond,
cond_swap ? l_src : *r,
cond_swap ? *r : l_src);
}





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


Re: [Mesa-dev] [PATCH v3 4/4] meson: build "radv" vulkan driver for radeon hardware

2017-09-26 Thread Dylan Baker
Quoting Nirbheek Chauhan (2017-09-26 12:29:30)
> On Tue, Sep 26, 2017 at 9:46 PM, Dylan Baker  wrote:
> > This builds and installs, but I haven't had a chance to test it yet.
> >
> > v2: - enable radv by default
> > - add shader cache support and enforce that it's built for radv
> > v3: - Fix typo in meson_options (Nicholas)
> > - strip trailing 'svn' from llvm version before setting the version
> >   preprocessor flag (Bas)
> 
> Should this also be added to the LLVMDependency meson class?

I was looking at that. I'm not sure since I think that 6.0.0svn *should* <
6.0.0, but meson just throws the svn away when it does the comparison.


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


Re: [Mesa-dev] [PATCH v3 4/4] meson: build "radv" vulkan driver for radeon hardware

2017-09-26 Thread Nirbheek Chauhan
On Wed, Sep 27, 2017 at 1:11 AM, Dylan Baker  wrote:
> Quoting Nirbheek Chauhan (2017-09-26 12:29:30)
>> On Tue, Sep 26, 2017 at 9:46 PM, Dylan Baker  wrote:
>> > This builds and installs, but I haven't had a chance to test it yet.
>> >
>> > v2: - enable radv by default
>> > - add shader cache support and enforce that it's built for radv
>> > v3: - Fix typo in meson_options (Nicholas)
>> > - strip trailing 'svn' from llvm version before setting the version
>> >   preprocessor flag (Bas)
>>
>> Should this also be added to the LLVMDependency meson class?
>
> I was looking at that. I'm not sure since I think that 6.0.0svn *should* <
> 6.0.0, but meson just throws the svn away when it does the comparison.

That's version_compare(), which should perhaps follow rpm's version
comparison algorithm[1], since that is also what pkg-config and many
other package managers use, but in practice almost all libraries use
semantic versioning (which is what it implements right now).

For the LLVMDependency class, as a start I think we could strip the
'svn' from the version since most things aren't really ready for alpha
to be present in version numbers. It's up to you though, since you
wrote the class and are more familiar with llvm itself.

1. http://blog.jasonantman.com/2014/07/how-yum-and-rpm-compare-versions/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v3 3/4] meson: Add build Intel "anv" vulkan driver

2017-09-26 Thread Dylan Baker
Quoting Eric Anholt (2017-09-26 11:46:50)
> Dylan Baker  writes:
> 
> > This allows building and installing the Intel "anv" Vulkan driver using
> > meson and ninja, the driver has been tested against the CTS and has
> > seems to pass the same series of tests (they both segfault when the CTS
> > tries to run wayland wsi tests).
> >
> > There are still a mess of TODO, XXX, and FIXME comments in here. Those
> > are mostly for meson bugs I'm trying to fix, or for additional things to
> > implement for other drivers/features.
> >
> > I have configured all intermediate libraries and optional tools to not
> > build by default, meaning they will only be built if they're pulled in
> > as a dependency of a target that will actually be installed) this allows
> > us to avoid massive if chains, while ensuring that only the bits that
> > need to be built are.
> 
> I am really looking forward to converting my vc5-vulkan work over to the
> meson build.  Given the rate I have to rebase that branch, it would be
> worth my time to convert over it in the next day of actual code
> development I get to do.
> 
> So, I'll offer a few bits of feedback on meson usage, after which you
> can have my
> 
> Reviewed-by: Eric Anholt 
> 
> so that we can start working in the tree together.
> 
> > diff --git a/meson.build b/meson.build
> > new file mode 100644
> > index 000..09e53957fe9
> > --- /dev/null
> > +++ b/meson.build
> > @@ -0,0 +1,423 @@
> > +# Copyright © 2017 Intel Corporation
> > +
> > +# Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > +# of this software and associated documentation files (the "Software"), to 
> > deal
> > +# in the Software without restriction, including without limitation the 
> > rights
> > +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > +# copies of the Software, and to permit persons to whom the Software is
> > +# furnished to do so, subject to the following conditions:
> > +
> > +# The above copyright notice and this permission notice shall be included 
> > in
> > +# all copies or substantial portions of the Software.
> > +
> > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> > THE
> > +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> > FROM,
> > +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
> > IN THE
> > +# SOFTWARE.
> > +
> > +project('mesa', ['c', 'cpp'], version : '17.3.0-devel', license : 'MIT',
> > +default_options : ['c_std=c99'])
> > +
> > +with_dri3 = true  # XXX: need a switch for this
> > +with_vulkan_icd_dir = get_option('vulkan_icd_dir')
> > +with_tests = get_option('build-tests')
> > +with_valgrind = get_option('valgrind')
> > +
> > +# TODO: there are more platforms required for non-vulkan drivers
> > +with_platform_wayland = false
> > +with_platform_x11 = false
> > +_platforms = get_option('platforms')
> > +if _platforms != ''
> > +  _split = _platforms.split(',')
> > +  with_platform_x11 = _split.contains('x11')
> > +  with_platform_wayland = _split.contains('wayland')
> > +endif
> > +
> > +if with_vulkan_icd_dir == ''
> > +  with_vulkan_icd_dir = join_paths(get_option('datadir'), 'vulkan/icd.d')
> > +endif
> > +
> > +with_intel_vk = false
> > +with_amd_vk = false
> > +_vulkan_drivers = get_option('vulkan-drivers')
> > +if _vulkan_drivers != ''
> > +  _split = _vulkan_drivers.split(',')
> > +  with_intel_vk = _split.contains('intel')
> > +  with_amd_vk = _split.contains('amd')
> > +  if not (with_platform_x11 or with_platform_wayland)
> > +error('Vulkan requires at least one platform (x11, wayland)')
> > +  endif
> > +endif
> > +
> > +prog_python2 = find_program('python2')
> > +
> > +cc = meson.get_compiler('c')
> > +if cc.get_id() == 'gcc' and cc.version().version_compare('< 4.4.6')
> > +  error('When using GCC version 4.2.0 or later required.')
> > +endif
> > +
> > +# Arguments for the preprocessor. These need to be added to both the C and 
> > the
> > +# C++ (cpp in meson terminology) arguments, so the they're calculated
> 
> "so that they're"
> 

I think that "the" is just useless here, I'm trying to say "put preprocessor
args in a separate array since they need to be added to the default arguments
for C and C++"

> > +# separately
> > +pre_args = ['-D__STDC_CONSTANT_MACROS', '-D__STDC_FORMAT_MACROS',
> > +'-D__STDC_LIMIT_MACROS',
> > +'-DVERSION="@0@"'.format(meson.project_version())]
> > +
> > +# Define debug for debug and debugoptimized builds
> 
> DEBUG
> 
> > +if get_option('buildtype').startswith('debug')
> > +  pre_args += '-DDEBUG'
> > +endif
> > +
> 
> 
> > +# check for dl support
> > +if cc.has_function('dlopen')
> > +  pre

Re: [Mesa-dev] [PATCH v3 4/4] meson: build "radv" vulkan driver for radeon hardware

2017-09-26 Thread Dylan Baker
Quoting Nirbheek Chauhan (2017-09-26 12:58:12)
> On Wed, Sep 27, 2017 at 1:11 AM, Dylan Baker  wrote:
> > Quoting Nirbheek Chauhan (2017-09-26 12:29:30)
> >> On Tue, Sep 26, 2017 at 9:46 PM, Dylan Baker  wrote:
> >> > This builds and installs, but I haven't had a chance to test it yet.
> >> >
> >> > v2: - enable radv by default
> >> > - add shader cache support and enforce that it's built for radv
> >> > v3: - Fix typo in meson_options (Nicholas)
> >> > - strip trailing 'svn' from llvm version before setting the version
> >> >   preprocessor flag (Bas)
> >>
> >> Should this also be added to the LLVMDependency meson class?
> >
> > I was looking at that. I'm not sure since I think that 6.0.0svn *should* <
> > 6.0.0, but meson just throws the svn away when it does the comparison.
> 
> That's version_compare(), which should perhaps follow rpm's version
> comparison algorithm[1], since that is also what pkg-config and many
> other package managers use, but in practice almost all libraries use
> semantic versioning (which is what it implements right now).
> 
> For the LLVMDependency class, as a start I think we could strip the
> 'svn' from the version since most things aren't really ready for alpha
> to be present in version numbers. It's up to you though, since you
> wrote the class and are more familiar with llvm itself.
> 
> 1. http://blog.jasonantman.com/2014/07/how-yum-and-rpm-compare-versions/

I actually knew almost nothing about LLVM before I wrote the dependency handler,
but I think you're right, if nothing else in meson handles pre-releases llvm
shouldn't either. I have some other LLVMDependency cleanups I was working on, so
I'll add this to it.


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


Re: [Mesa-dev] [PATCH] glx: Be slightly more tolerant in glXImportContext

2017-09-26 Thread Adam Jackson
On Tue, 2017-09-26 at 11:59 -0700, Eric Anholt wrote:

> Instead of magic numbers, couldn't we just malloc the proplist here and
> free it after the loop?

Fine. You tyrant.

Actually I have a better idea. We've buffered the entire reply by this
point, we can just read two dwords at a time until we're done and avoid
malloc games entirely.

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


Re: [Mesa-dev] [PATCH 00/22] RFC: Batchbuffer Logger for Intel GPU

2017-09-26 Thread Chris Wilson
Quoting Rogovin, Kevin (2017-09-26 10:35:44)
> Hi,
> 
>   Attached to this message are the following:
>  1. a file giving example usage of the tool with a modified apitrace to 
> produce json output
> 
>  2. the patches to apitrace to make it BatchbufferLogger aware
> 
>  3. the JSON files (gzipped) made from the example.
> 
> 
> I encourage (and hope) people will take a look at the JSON to see the 
> potential of the tool.

The automatic apitrace-esque logging seems very useful. How easy would
it be to write that trace into a bo and associate with the execbuffer
(from my pov, it should be that hard)? That way you could get the most
recent actions before a GPU hang, attach them to a bug and decode them
at leisure. (An extension may be to keep a ring of the last N traces so
that you can see some setup a few batches ago that triggered a hang in
this one.)

I presume you already have such a plan, and I'm just preaching to the
choir.
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 103002] string_buffer_test.cpp:43: error: ISO C++ forbids initialization of member ‘str1’

2017-09-26 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=103002

Bug ID: 103002
   Summary: string_buffer_test.cpp:43: error: ISO C++ forbids
initialization of member ‘str1’
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Keywords: bisected, regression
  Severity: normal
  Priority: medium
 Component: Mesa core
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: v...@freedesktop.org
QA Contact: mesa-dev@lists.freedesktop.org
CC: die...@nuetzel-hh.de, nhaeh...@gmail.com,
thomashellan...@gmail.com

mesa: 8822ea100cfd7482290c3c6b2a7200c8b888a7f4 (master 17.3.0-devel)

Build error with g++ 4.4.

  CXXstring_buffer_test.o
string_buffer_test.cpp:43: error: ISO C++ forbids initialization of member
‘str1’
string_buffer_test.cpp:43: error: making ‘str1’ static
string_buffer_test.cpp:43: error: invalid in-class initialization of static
data member of non-integral type ‘const char*’


Introduced with this commit.

commit 584a2a22ea40cdc030db5b1d70b23dddcc06a556
Author: Thomas Helland 
Date:   Fri May 19 22:07:17 2017 +0200

util: Add tests for the string buffer

More tests could probably be added, but this should cover
concatenation, resizing, clearing, formatted printing,
and checking the length, so it should be quite complete.

Signed-off-by: Thomas Helland 
Reviewed-by: Nicolai Hähnle 
Tested-by: Dieter Nützel 

V2: Address review feedback from Timothy, plus fixes
   - Use a large enough char array
   - Actually test the formatted appending
   - Test that clear function resets string length

V3: Port to gtest

V4: Fix test makefile
Fix copyright header
Fix missing extern C
Use more appropriate name for C-file
Add tests for append_char

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


  1   2   >