Re: [Mesa-dev] [PATCH 4/4] nir: detect more induction variables

2018-11-27 Thread Thomas Helland
Den ons. 28. nov. 2018 kl. 04:26 skrev Timothy Arceri :
>
> This adds allows loop analysis to detect inductions varibales that
> are incremented in both branches of an if rather than in a main
> loop block. For example:
>
>loop {
>   block block_1:
>   /* preds: block_0 block_7 */
>   vec1 32 ssa_8 = phi block_0: ssa_4, block_7: ssa_20
>   vec1 32 ssa_9 = phi block_0: ssa_0, block_7: ssa_4
>   vec1 32 ssa_10 = phi block_0: ssa_1, block_7: ssa_4
>   vec1 32 ssa_11 = phi block_0: ssa_2, block_7: ssa_21
>   vec1 32 ssa_12 = phi block_0: ssa_3, block_7: ssa_22
>   vec4 32 ssa_13 = vec4 ssa_12, ssa_11, ssa_10, ssa_9
>   vec1 32 ssa_14 = ige ssa_8, ssa_5
>   /* succs: block_2 block_3 */
>   if ssa_14 {
>  block block_2:
>  /* preds: block_1 */
>  break
>  /* succs: block_8 */
>   } else {
>  block block_3:
>  /* preds: block_1 */
>  /* succs: block_4 */
>   }
>   block block_4:
>   /* preds: block_3 */
>   vec1 32 ssa_15 = ilt ssa_6, ssa_8
>   /* succs: block_5 block_6 */
>   if ssa_15 {
>  block block_5:
>  /* preds: block_4 */
>  vec1 32 ssa_16 = iadd ssa_8, ssa_7
>  vec1 32 ssa_17 = load_const (0x3f80 /* 1.00*/)
>  /* succs: block_7 */
>   } else {
>  block block_6:
>  /* preds: block_4 */
>  vec1 32 ssa_18 = iadd ssa_8, ssa_7
>  vec1 32 ssa_19 = load_const (0x3f80 /* 1.00*/)
>  /* succs: block_7 */
>   }
>   block block_7:
>   /* preds: block_5 block_6 */
>   vec1 32 ssa_20 = phi block_5: ssa_16, block_6: ssa_18
>   vec1 32 ssa_21 = phi block_5: ssa_17, block_6: ssa_4
>   vec1 32 ssa_22 = phi block_5: ssa_4, block_6: ssa_19
>   /* succs: block_1 */
>}
>
> Unfortunatly GCM could move the addition out of the if for us
> (making this patch unrequired) but we still cannot enable the GCM
> pass without regressions.
>

Just some questions / suggestions from my side for now.
I'll try to take a closer look at the patch later today.

While GCM would be nice, to me it seems that adding an
if-opt instead, that pulls common code from both branches
of an if out of the if on a more general basis, would get us
this, plus a bunch of other benefits? As far as I can see there
should never be negative impacts from pulling common code
out like that, but I might be wrong. Did you look into that?
I bet out did, I'm just interested in how that worked out.

Since GCM is not yet where we want it to be, maybe we'd
want to implement LICM? That obviously does not come
into play with what this patch adresses, but it might help
get a more accurate estimate of the cost/benefit of unrolling?
(Invariant computations that will be CSE'd will not be
counted multiple times). This might already be accounted
for by counting the invariant computations only once?

> This unrolls a loop in Rise of The Tomb Raider.
>
> vkpipeline-db results (VEGA):
>
> Totals from affected shaders:
> SGPRS: 88 -> 96 (9.09 %)
> VGPRS: 56 -> 52 (-7.14 %)
> Spilled SGPRs: 0 -> 0 (0.00 %)
> Spilled VGPRs: 0 -> 0 (0.00 %)
> Private memory VGPRs: 0 -> 0 (0.00 %)
> Scratch size: 0 -> 0 (0.00 %) dwords per thread
> Code Size: 2168 -> 4560 (110.33 %) bytes
> LDS: 0 -> 0 (0.00 %) blocks
> Max Waves: 4 -> 4 (0.00 %)
> Wait states: 0 -> 0 (0.00 %)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=32211
> ---
>  src/compiler/nir/nir_loop_analyze.c | 36 +
>  1 file changed, 36 insertions(+)
>
> diff --git a/src/compiler/nir/nir_loop_analyze.c 
> b/src/compiler/nir/nir_loop_analyze.c
> index 8903e15105..cf97d6bf06 100644
> --- a/src/compiler/nir/nir_loop_analyze.c
> +++ b/src/compiler/nir/nir_loop_analyze.c
> @@ -245,6 +245,42 @@ compute_induction_information(loop_info_state *state)
>   if (src_var->in_if_branch || src_var->in_nested_loop)
>  break;
>
> + /* Detect inductions varibales that are incremented in both branches
> +  * of an unnested if rather than in a loop block.
> +  */
> + if (is_var_phi(src_var)) {
> +nir_phi_instr *src_phi =
> +   nir_instr_as_phi(src_var->def->parent_instr);
> +
> +nir_op alu_op;
> +nir_ssa_def *alu_srcs[2] = {0};
> +nir_foreach_phi_src(src2, src_phi) {
> +   nir_loop_variable *src_var2 =
> +  get_loop_var(src2->src.ssa, state);
> +
> +   if (!src_var2->in_if_branch || !is_var_alu(src_var2))
> +  break;
> +
> +   nir_alu_instr *alu =
> +  nir_instr_as_alu(src_var2->def->parent_instr);
> +   if (nir_op_infos[alu->op].num_inputs != 2)
> +  break;
> +
> +   if (alu->src[0].src.ssa == alu_srcs[0] &&
> +   alu->src[1].src.ssa == alu_srcs[1] &&
> +   alu->op == alu_op) {
> +  /* 

Re: [Mesa-dev] [PATCH] intel/compiler: Use nir's info when checking uses_streams.

2018-11-27 Thread Iago Toral
Reviewed-by: Iago Toral Quiroga 

On Tue, 2018-11-27 at 15:34 -0800, Kenneth Graunke wrote:
> Vulkan and Gallium don't use Mesa's gl_program data structure, so
> they
> can't poke at 'prog'.  But we can simply use the copy of the shader
> info
> stored with the NIR shader, which is guaranteed to exist.
> 
> Reviewed-by: Jason Ekstrand 
> ---
>  src/intel/compiler/brw_vec4_gs_visitor.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/intel/compiler/brw_vec4_gs_visitor.cpp
> b/src/intel/compiler/brw_vec4_gs_visitor.cpp
> index 63ff27e5e08..a6e38b0f379 100644
> --- a/src/intel/compiler/brw_vec4_gs_visitor.cpp
> +++ b/src/intel/compiler/brw_vec4_gs_visitor.cpp
> @@ -667,7 +667,7 @@ brw_compile_gs(const struct brw_compiler
> *compiler, void *log_data,
>   prog_data->control_data_format =
> GEN7_GS_CONTROL_DATA_FORMAT_GSCTL_SID;
>  
>   /* We only have to emit control bits if we are using
> streams */
> - if (prog && prog->info.gs.uses_streams)
> + if (shader->info.gs.uses_streams)
>  c.control_data_bits_per_vertex = 2;
>   else
>  c.control_data_bits_per_vertex = 0;

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


[Mesa-dev] [PATCH] docs: Document and *require* usage of Signed-off-by

2018-11-27 Thread Jordan Justen
This adds the "Developer's Certificate of Origin 1.1" from the Linux
kernel. It indicates that by using Signed-off-by you are certifying
that your patch meets the DCO 1.1 guidelines.

It also changes Signed-off-by from being optional to being required.

Signed-off-by: Jordan Justen 
Cc: Matt Turner 
---
 docs/submittingpatches.html | 52 -
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
index 9ae750d5a15..6d506b3691b 100644
--- a/docs/submittingpatches.html
+++ b/docs/submittingpatches.html
@@ -20,6 +20,8 @@
 
 Basic guidelines
 Patch formatting
+Patch Signing (Signed-off-by, Developer's
+  Certificate of Origin)
 Testing Patches
 Mailing Patches
 Reviewing Patches
@@ -73,7 +75,9 @@ if needed.  For example:
 is necessary, and removing it causes no regressions in piglit on any
 platform.
 
-A "Signed-off-by:" line is not required, but not discouraged either.
+A "Signed-off-by:" line is required. The format
+and meaning of Signed-off-by is documented below in
+the patch signing section.
 If a patch addresses a bugzilla issue, that should be noted in the
 patch comment.  For example:
 
@@ -129,7 +133,53 @@ Please use common sense and do not 
blindly add everyone.
 
 
 
+
+  Patch Signing (Signed-off-by, Developer's Certificate of Origin)
+
 
+
+  As described in the patch formatting
+  section, you must sign your patch by including Signed-off-by in the
+  patch commit message. The Signed-off-by must include your real name
+  and email address in this format:
+
+
+  Signed-off-by: Joe Hacker jhac...@foo.com
+
+
+  By adding Signed-of-by to your contributed patch, you certify that
+  your contribution meets the guidelines of the Developer's
+  Certificate of Origin:
+
+
+
+Developer's Certificate of Origin 1.1
+-
+
+By making a contribution to this project, I certify that:
+
+ (a) The contribution was created in whole or in part by me and I
+ have the right to submit it under the open source license
+ indicated in the file; or
+
+ (b) The contribution is based upon previous work that, to the best
+ of my knowledge, is covered under an appropriate open source
+ license and I have the right under that license to submit that
+ work with modifications, whether created in whole or in part
+ by me, under the same open source license (unless I am
+ permitted to submit under a different license), as indicated
+ in the file; or
+
+ (c) The contribution was provided directly to me by some other
+ person who certified (a), (b) or (c) and I have not modified
+ it.
+
+ (d) I understand and agree that this project and the contribution
+ are public and that a record of the contribution (including all
+ personal information I submit with it, including my sign-off) is
+ maintained indefinitely and may be redistributed consistent with
+ this project or the open source license(s) involved.
+
 
 Testing Patches
 
-- 
2.20.0.rc1

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


Re: [Mesa-dev] [PATCH] egl: define NULL in egldevice.h

2018-11-27 Thread Matt Turner
Reviewed-by: Matt Turner 

I'll commit it tomorrow.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] i965: Set the FBO error state INCOMPLETE_ATTACHMENT only for SRGB_R8

2018-11-27 Thread Tapani Pälli



On 11/22/18 8:00 PM, Gert Wollny wrote:

Originally the driver reported GL_FRAMEBUFFER_UNSUPPORTED in all cases,
adding more specific error messages was not correct and broke many tests.
Mostly revert this and only report GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT
for MESA_FORMAT_R_SRGB8.

Fixes: ebcde3454552adc6d3fea8af2207aafaba857796
   i965: be more specific about FBO completeness errors


Like with patch 1, fix 'Fixes', with that
Reviewed-by: Tapani Pälli 



Signed-off-by: Gert Wollny 
---
  src/mesa/drivers/dri/i965/intel_fbo.c | 13 ++---
  1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c 
b/src/mesa/drivers/dri/i965/intel_fbo.c
index 7e40d61a47..5bcd846a1b 100644
--- a/src/mesa/drivers/dri/i965/intel_fbo.c
+++ b/src/mesa/drivers/dri/i965/intel_fbo.c
@@ -719,7 +719,7 @@ intel_validate_framebuffer(struct gl_context *ctx, struct 
gl_framebuffer *fb)
"FBO incomplete: separate stencil unsupported\n");
 }
 if (stencil_mt->format != MESA_FORMAT_S_UINT8) {
-   fbo_incomplete(fb, GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT,
+   fbo_incomplete(fb, GL_FRAMEBUFFER_UNSUPPORTED,
"FBO incomplete: separate stencil is %s "
"instead of S8\n",
_mesa_get_format_name(stencil_mt->format));
@@ -750,7 +750,7 @@ intel_validate_framebuffer(struct gl_context *ctx, struct 
gl_framebuffer *fb)
 */
rb = fb->Attachment[i].Renderbuffer;
if (rb == NULL) {
-fbo_incomplete(fb, GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT,
+fbo_incomplete(fb, GL_FRAMEBUFFER_UNSUPPORTED,
 "FBO incomplete: attachment without "
 "renderbuffer\n");
 continue;
@@ -771,8 +771,15 @@ intel_validate_framebuffer(struct gl_context *ctx, struct 
gl_framebuffer *fb)
 continue;
}
  
+ if (rb->Format == MESA_FORMAT_R_SRGB8) {

+fbo_incomplete(fb, GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT,
+   "FBO incomplete: Format not color renderable: %s\n",
+   _mesa_get_format_name(rb->Format));
+continue;
+ }
+
if (!brw_render_target_supported(brw, rb)) {
-fbo_incomplete(fb, GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT,
+fbo_incomplete(fb, GL_FRAMEBUFFER_UNSUPPORTED,
 "FBO incomplete: Unsupported HW "
 "texture/renderbuffer format attached: %s\n",
 _mesa_get_format_name(intel_rb_format(irb)));


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


Re: [Mesa-dev] [PATCH 1/2] i965: Explicitely handle swizzles for MESA_FORMAT_R_SRGB8

2018-11-27 Thread Tapani Pälli


On 11/22/18 8:00 PM, Gert Wollny wrote:

The format is emulated by using ISL_FORMAT_L8_SRGB, therefore we need to
force swizzles for the GBA channels. However, doing this only based on the
data type GL_RED breaks other formats, therefore, test specifically for the
format.

Fixes: 5363869d4971780401b21bb75083ef2518c12be
   965: Force zero swizzles for unused components in GL_RED and GL_RG


As Emil said, 'Fixes' line needs to be fixed before committing. 
Otherwise this LGTM;


Reviewed-by: Tapani Pälli 


Signed-off-by: Gert Wollny 
---
  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 018bae98e8..4daa0e2add 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -420,11 +420,15 @@ brw_get_texture_swizzle(const struct gl_context *ctx,
}
break;
 case GL_RED:
-  swizzles[1] = SWIZZLE_ZERO;
+  if (img->TexFormat == MESA_FORMAT_R_SRGB8) {
+ swizzles[0] = SWIZZLE_X;
+ swizzles[1] = SWIZZLE_ZERO;
+ swizzles[2] = SWIZZLE_ZERO;
+ swizzles[3] = SWIZZLE_ONE;
+ break;
+  }
/* fallthrough */
 case GL_RG:
-  swizzles[2] = SWIZZLE_ZERO;
-  /* fallthrough */
 case GL_RGB:
if (_mesa_get_format_bits(img->TexFormat, GL_ALPHA_BITS) > 0 ||
img->TexFormat == MESA_FORMAT_RGB_DXT1 ||


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


[Mesa-dev] [PATCH] gallivm: Use nextafterf(0.5, 0.0) as rounding constant

2018-11-27 Thread Matt Turner
The common truncf(x + 0.5) fails for the floating-point value just less
than 0.5 (nextafterf(0.5, 0.0)). nextafterf(0.5, 0.0) + 0.5, after
rounding is 1.0, thus truncf does not produce the desired value.

The solution is to add nextafterf(0.5, 0.0) instead of 0.5 before
truncating. This works for all values.
---
I noticed this while investigating https://bugs.gentoo.org/665570 but it
does not fix it.

Roland, do you have a suggestion for how to make lp_build_iround() work
on non-SSE/non-Altivec platforms? I notice that if I unconditionally
return TRUE from arch_rounding_available() and make
lp_build_round_arch() take the SSE4.1 path (that emits llvm.nearbyint)
it passes on ARM64.

I noticed there's some hack in lp_test_arit.c:test_unary:

   if (test->ref ==  && length == 2 &&
   ref != roundf(testval)) {
  /* FIXME: The generic (non SSE) path in lp_build_iround, which is
   * always taken for length==2 regardless of native round support,
   * does not round to even. */
  expected_pass = FALSE;
   }

It'd be nice to get rid of that.. but maybe we can somehow use it to
just mark all the round tests as expected fail on other platforms if no
real fix is forthcoming?

 src/gallium/auxiliary/gallivm/lp_bld_arit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c 
b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
index f348833206b..c050bfdb936 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
@@ -2477,7 +2477,7 @@ lp_build_iround(struct lp_build_context *bld,
else {
   LLVMValueRef half;
 
-  half = lp_build_const_vec(bld->gallivm, type, 0.5);
+  half = lp_build_const_vec(bld->gallivm, type, nextafterf(0.5, 0.0));
 
   if (type.sign) {
  LLVMTypeRef vec_type = bld->vec_type;
-- 
2.18.1

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


Re: [Mesa-dev] [PATCH] docs: Document the optional usage of Signed-off-by

2018-11-27 Thread Jordan Justen
On 2018-11-27 19:53:38, Matt Turner wrote:
> On Tue, Nov 27, 2018 at 7:26 PM Jordan Justen  
> wrote:
> > On 2018-11-27 18:04:17, Matt Turner wrote:
> > > By all means, require it (with a git hook) if you like.
> >
> > I personally don't want to push for that right now.
> >
> > I guess I would like it to be required someday, primarily because it
> > creates a standard process for open source projects to use. (So,
> > people are more likely to be used to it in general when contributing
> > to open source projects.)
> >
> > But, I'm not confident that the consensus for Mesa would be in favor
> > of making that change right now. So, as an alternative I'd like to
> > remove any barriers (such as ambiguity) to its usage in Mesa.
> 
> I don't think requiring S-o-b is onerous. Using S-o-b in a defined way
> is just making explicit the current implicit promise to the community
> that "I have the right to contribute this code" etc. I'm not sure why
> you're hesitant.

Ok. I'll type that version up.

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


[Mesa-dev] [PATCH] egl: define NULL in egldevice.h

2018-11-27 Thread Gurchetan Singh
Otherwise, I get this error:

main/egldevice.h:54:13: error: ‘NULL’ undeclared (first use in this function)
   dev = NULL;
 ^~~~
with this config:

./autogen.sh --enable-gles1 --enable-gles2 --with-platforms='surfaceless' 
--disable-glx
 --with-dri-drivers="i965" --with-gallium-drivers="" --enable-gbm

v3: Use stddef.h (Matt)
---
 src/egl/main/egldevice.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/egl/main/egldevice.h b/src/egl/main/egldevice.h
index ddcdcd17f5..83a47d5eac 100644
--- a/src/egl/main/egldevice.h
+++ b/src/egl/main/egldevice.h
@@ -31,9 +31,9 @@
 
 
 #include 
+#include 
 #include "egltypedefs.h"
 
-
 #ifdef __cplusplus
 extern "C" {
 #endif
-- 
2.18.1

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


Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process

2018-11-27 Thread Matt Turner
On Tue, Nov 27, 2018 at 7:45 PM Jordan Justen  wrote:
>
> On 2018-11-27 19:20:09, Matt Turner wrote:
> > On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen  
> > wrote:
> > >
> > > This documents a mechanism for using GitLab Merge Requests as an
> > > optional, secondary way to get code reviews for patchsets.
> > >
> > > We still require all patches to be emailed.
> > >
> > > Aside from the potential usage for code review comments, it might also
> > > help reviewers to find unmerged patchsets which need review. (Assuming
> > > it doesn't fall into the same fate of patchwork with hundreds of open
> > > MRs.)
> > >
> > > Signed-off-by: Jordan Justen 
> > > Cc: Jason Ekstrand 
> > > ---
> > >  docs/submittingpatches.html | 25 +
> > >  1 file changed, 25 insertions(+)
> > >
> > > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
> > > index 5d8ba443191..852f28c198a 100644
> > > --- a/docs/submittingpatches.html
> > > +++ b/docs/submittingpatches.html
> > > @@ -24,6 +24,7 @@
> > >  Testing Patches
> > >  Mailing Patches
> > >  Reviewing Patches
> > > +GitLab Merge Requests
> > >  Nominating a commit for a stable branch
> > >  Criteria for accepting patches to the stable 
> > > branch
> > >  Sending backports for the stable branch
> > > @@ -282,6 +283,30 @@ which tells the patch author that the patch can be 
> > > committed, as long
> > >  as the issues are resolved first.
> > >  
> > >
> > > +GitLab Merge Requests
> > > +
> > > +
> > > +  https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge
> > > +  Requests (MR) can be used as an optional, secondary method of
> > > +  obtaining code review for patches.
> > > +
> > > +
> > > +
> > > +  All patches should be submitted using email as well
> > > +  Consider including a link to the MR in your email based
> > > +cover-letter
> > > +  Address code review from both email and the MR
> >
> > Discussion point: I think attempting to have simultaneous review in
> > two places is a recipe for wasted time.
>
> That's possible. It also happens on email sometimes. But, I want to
> say that maybe the usual problem is too little code review, and not
> too much? :)
>
> > Strawman: maybe we can only email the cover letter to the mailing
> > list and include in it a link to the MR?
>
> I was hoping to make a smaller step and see what happens. Maybe this
> will give people the chance to try out MR based reviews, but not take
> away email based reviews just yet.
>
> I don't think we should move away from email based reviews until we
> are sure MR's actually work better. (I'm far from convinced on this
> point. :)

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


Re: [Mesa-dev] [PATCH] docs: Document the optional usage of Signed-off-by

2018-11-27 Thread Matt Turner
On Tue, Nov 27, 2018 at 7:26 PM Jordan Justen  wrote:
>
> On 2018-11-27 18:04:17, Matt Turner wrote:
> > On Tue, Nov 27, 2018 at 6:00 PM Jordan Justen  
> > wrote:
> > >
> > > On 2018-11-27 17:17:15, Matt Turner wrote:
> > > > On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen 
> > > >  wrote:
> > > > >
> > > > > This adds the "Developer's Certificate of Origin 1.1" from the Linux
> > > > > kernel. It indicates that by using Signed-off-by you are certifying
> > > > > that your patch meets the DCO 1.1 guidelines.
> > > >
> > > > Do we gain anything if it's optional?
> > >
> > > As I recall, one thing that bothered you about Signed-off-by in Mesa
> > > is that it wasn't documented what it meant when it was used.
> > >
> > > Perhaps there are developers that don't want to use Signed-off-by with
> > > an undocumented meaning for Mesa. If that is the case, then this might
> > > help. I wasn't sure if you fell into that category.
> > >
> > > I use -s whenever I commit, so requiring it would not bother me. But,
> > > I notice that many people (such as yourself) do not, so I didn't see
> > > the need to push for that.
> > >
> > > If it's well documented, and becomes commonly used, then perhaps
> > > requiring it might be a reasonable thing to consider. I won't be
> > > holding my breath while waiting on that. :)
> >
> > I don't have a problem requiring it. I sign-off on commits I make to
> > Gentoo, to Linux, etc.
>
> If it has the same meaning as with the Linux kernel, but is not
> required, then you won't use it?
>
> I guess your concern might be that you are then giving something to
> the project that others can choose not to. ?

I'm not aware of any projects that define S-o-b but make it optional.
I suspect there's a reason that those that define it require it.

> > I'm just against cargo-culting it like we're doing now without a
> > defined meaning.
>
> The purpose of this patch is to give it a defined meaning. And the
> meaning I chose is the one that people are more likely to have in mind
> when they use Signed-off-by. Maybe that's too big of an assumption on
> my part, but I think several other open source projects have followed
> the kernel on this.

I understand, and agree. I'm just suggesting that it may not be good
to do something different than all the other projects that
meaningfully use S-o-b (i.e., by making it optional).

> > By all means, require it (with a git hook) if you like.
>
> I personally don't want to push for that right now.
>
> I guess I would like it to be required someday, primarily because it
> creates a standard process for open source projects to use. (So,
> people are more likely to be used to it in general when contributing
> to open source projects.)
>
> But, I'm not confident that the consensus for Mesa would be in favor
> of making that change right now. So, as an alternative I'd like to
> remove any barriers (such as ambiguity) to its usage in Mesa.

I don't think requiring S-o-b is onerous. Using S-o-b in a defined way
is just making explicit the current implicit promise to the community
that "I have the right to contribute this code" etc. I'm not sure why
you're hesitant.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] st/mesa: expose GL_OES_texture_view

2018-11-27 Thread Marek Olšák
On Tue, Nov 27, 2018 at 9:17 PM Ilia Mirkin  wrote:

> On Tue, Nov 27, 2018 at 9:00 PM Marek Olšák  wrote:
> >
> > From: Marek Olšák 
> >
> > For format fallbacks like ETC and ASTC, switching between sRGB and linear
> > decoding is undefined, or at least is not bit-exact. Same as
> > EXT_texture_sRGB_decode on GLES.
> >
> > There are no piglit or dEQP regresssions.
>
> Just checking -- did you run dEQP on hardware that uses the ASTC
> fallbacks? If not, could you?
>

radeonsi doesn't support ASTC, so I always use the fallback. Like I said,
no regressions in newly enabled tests.


>
> Also, what are the conditions for enabling ASTC fallbacks? Basically
> the issue is that OES_texture_view requires ASTC, so I want to make
> sure it'll be enabled for any drivers that
> PIPE_CAP_SAMPLER_VIEW_TARGET is enabled for.
>

ASTC is always enabled.

The existing behavior being talked about is already exposed by
EXT_texture_sRGB_decode on GLES. The extension only adds another way to
access that behavior. I'm open to discussing how to make
EXT_texture_sRGB_decode bit-exact for compressed format fallbacks. I may
also let it slide if test suites don't care.

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


Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process

2018-11-27 Thread Jordan Justen
On 2018-11-27 19:20:09, Matt Turner wrote:
> On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen  
> wrote:
> >
> > This documents a mechanism for using GitLab Merge Requests as an
> > optional, secondary way to get code reviews for patchsets.
> >
> > We still require all patches to be emailed.
> >
> > Aside from the potential usage for code review comments, it might also
> > help reviewers to find unmerged patchsets which need review. (Assuming
> > it doesn't fall into the same fate of patchwork with hundreds of open
> > MRs.)
> >
> > Signed-off-by: Jordan Justen 
> > Cc: Jason Ekstrand 
> > ---
> >  docs/submittingpatches.html | 25 +
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
> > index 5d8ba443191..852f28c198a 100644
> > --- a/docs/submittingpatches.html
> > +++ b/docs/submittingpatches.html
> > @@ -24,6 +24,7 @@
> >  Testing Patches
> >  Mailing Patches
> >  Reviewing Patches
> > +GitLab Merge Requests
> >  Nominating a commit for a stable branch
> >  Criteria for accepting patches to the stable 
> > branch
> >  Sending backports for the stable branch
> > @@ -282,6 +283,30 @@ which tells the patch author that the patch can be 
> > committed, as long
> >  as the issues are resolved first.
> >  
> >
> > +GitLab Merge Requests
> > +
> > +
> > +  https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge
> > +  Requests (MR) can be used as an optional, secondary method of
> > +  obtaining code review for patches.
> > +
> > +
> > +
> > +  All patches should be submitted using email as well
> > +  Consider including a link to the MR in your email based
> > +cover-letter
> > +  Address code review from both email and the MR
> 
> Discussion point: I think attempting to have simultaneous review in
> two places is a recipe for wasted time.

That's possible. It also happens on email sometimes. But, I want to
say that maybe the usual problem is too little code review, and not
too much? :)

> Strawman: maybe we can only email the cover letter to the mailing
> list and include in it a link to the MR?

I was hoping to make a smaller step and see what happens. Maybe this
will give people the chance to try out MR based reviews, but not take
away email based reviews just yet.

I don't think we should move away from email based reviews until we
are sure MR's actually work better. (I'm far from convinced on this
point. :)

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


Re: [Mesa-dev] [PATCH] egl: define NULL in egldevice.h

2018-11-27 Thread Matt Turner
Wouldn't it be better to just include the stdlib.h/stddef.h header
that defines NULL?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] egl: define NULL in egldevice.h

2018-11-27 Thread Gurchetan Singh
Otherwise, I get this error:

main/egldevice.h:54:13: error: ‘NULL’ undeclared (first use in this function)
   dev = NULL;
 ^~~~
with this config:

./autogen.sh --enable-gles1 --enable-gles2 --with-platforms='surfaceless' 
--disable-glx
 --with-dri-drivers="i965" --with-gallium-drivers="" --enable-gbm

v2: remove change id
---
 src/egl/main/egldevice.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/egl/main/egldevice.h b/src/egl/main/egldevice.h
index ddcdcd17f5..f161942d26 100644
--- a/src/egl/main/egldevice.h
+++ b/src/egl/main/egldevice.h
@@ -33,6 +33,9 @@
 #include 
 #include "egltypedefs.h"
 
+#ifndef NULL
+#define NULL 0
+#endif
 
 #ifdef __cplusplus
 extern "C" {
-- 
2.18.1

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


[Mesa-dev] [PATCH] egl: define NULL in egldevice.h

2018-11-27 Thread Gurchetan Singh
Otherwise, I get this error:

main/egldevice.h:54:13: error: ‘NULL’ undeclared (first use in this function)
   dev = NULL;
 ^~~~
with this config:

./autogen.sh --enable-gles1 --enable-gles2 --with-platforms='surfaceless' 
--disable-glx
 --with-dri-drivers="i965" --with-gallium-drivers="" --enable-gbm

Change-Id: I4332bfcfd19aecf239497591507aad921fffedf1
---
 src/egl/main/egldevice.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/egl/main/egldevice.h b/src/egl/main/egldevice.h
index ddcdcd17f5a..f161942d26f 100644
--- a/src/egl/main/egldevice.h
+++ b/src/egl/main/egldevice.h
@@ -33,6 +33,9 @@
 #include 
 #include "egltypedefs.h"
 
+#ifndef NULL
+#define NULL 0
+#endif
 
 #ifdef __cplusplus
 extern "C" {
-- 
2.18.1

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


[Mesa-dev] [Bug 32211] [GLSL] lower_jumps with continue-statements in for-loops prevents loop unrolling

2018-11-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=32211

--- Comment #16 from Timothy Arceri  ---
(In reply to Danylo from comment #15)
> Created attachment 142567 [details] [review]
> Removing unnecessary continue
> 
> Optimization in question.

Thanks! I've tried a few variations of this patch and eventually settled on a
v2 which I sent out with an update to loop analysis so that we can detect the
duplicate additions.

https://patchwork.freedesktop.org/series/53128/

-- 
You are receiving this mail because:
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 2/4] nir: in loop analysis track actual control flow type

2018-11-27 Thread Timothy Arceri
This will allow us to improve analysis to find more induction
variables.
---
 src/compiler/nir/nir_loop_analyze.c | 34 ++---
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/compiler/nir/nir_loop_analyze.c 
b/src/compiler/nir/nir_loop_analyze.c
index 587e9d7865..c804a66ac4 100644
--- a/src/compiler/nir/nir_loop_analyze.c
+++ b/src/compiler/nir/nir_loop_analyze.c
@@ -49,8 +49,11 @@ typedef struct {
/* If this is of type basic_induction */
struct nir_basic_induction_var *ind;
 
-   /* True if variable is in an if branch or a nested loop */
-   bool in_control_flow;
+   /* True if variable is in an if branch */
+   bool in_if_branch;
+
+   /* True if variable is in a nested loop */
+   bool in_nested_loop;
 
 } nir_loop_variable;
 
@@ -83,7 +86,8 @@ get_loop_var(nir_ssa_def *value, loop_info_state *state)
 
 typedef struct {
loop_info_state *state;
-   bool in_control_flow;
+   bool in_if_branch;
+   bool in_nested_loop;
 } init_loop_state;
 
 static bool
@@ -92,8 +96,10 @@ init_loop_def(nir_ssa_def *def, void *void_init_loop_state)
init_loop_state *loop_init_state = void_init_loop_state;
nir_loop_variable *var = get_loop_var(def, loop_init_state->state);
 
-   if (loop_init_state->in_control_flow) {
-  var->in_control_flow = true;
+   if (loop_init_state->in_nested_loop) {
+  var->in_nested_loop = true;
+   } else if (loop_init_state->in_if_branch) {
+  var->in_if_branch = true;
} else {
   /* Add to the tail of the list. That way we start at the beginning of
* the defs in the loop instead of the end when walking the list. This
@@ -110,9 +116,10 @@ init_loop_def(nir_ssa_def *def, void *void_init_loop_state)
 
 static bool
 init_loop_block(nir_block *block, loop_info_state *state,
-bool in_control_flow)
+bool in_if_branch, bool in_nested_loop)
 {
-   init_loop_state init_state = {.in_control_flow = in_control_flow,
+   init_loop_state init_state = {.in_if_branch = in_if_branch,
+ .in_nested_loop = in_nested_loop,
  .state = state };
 
nir_foreach_instr(instr, block) {
@@ -198,7 +205,7 @@ compute_invariance_information(loop_info_state *state)
 */
list_for_each_entry_safe(nir_loop_variable, var, >process_list,
 process_link) {
-  assert(!var->in_control_flow);
+  assert(!var->in_if_branch && !var->in_nested_loop);
 
   if (mark_invariant(var->def, state))
  list_del(>process_link);
@@ -216,7 +223,8 @@ compute_induction_information(loop_info_state *state)
* things in nested loops or conditionals should have been removed from
* the list by compute_invariance_information().
*/
-  assert(!var->in_control_flow && var->type != invariant);
+  assert(!var->in_if_branch && !var->in_nested_loop &&
+ var->type != invariant);
 
   /* We are only interested in checking phis for the basic induction
* variable case as its simple to detect. All basic induction variables
@@ -234,7 +242,7 @@ compute_induction_information(loop_info_state *state)
  /* If one of the sources is in a conditional or nested block then
   * panic.
   */
- if (src_var->in_control_flow)
+ if (src_var->in_if_branch || src_var->in_nested_loop)
 break;
 
  if (!src_var->in_loop) {
@@ -833,17 +841,17 @@ get_loop_info(loop_info_state *state, nir_function_impl 
*impl)
   switch (node->type) {
 
   case nir_cf_node_block:
- init_loop_block(nir_cf_node_as_block(node), state, false);
+ init_loop_block(nir_cf_node_as_block(node), state, false, false);
  break;
 
   case nir_cf_node_if:
  nir_foreach_block_in_cf_node(block, node)
-init_loop_block(block, state, true);
+init_loop_block(block, state, true, false);
  break;
 
   case nir_cf_node_loop:
  nir_foreach_block_in_cf_node(block, node) {
-init_loop_block(block, state, true);
+init_loop_block(block, state, false, true);
  }
  break;
 
-- 
2.19.1

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


[Mesa-dev] [PATCH 3/4] nir: reword code comment

2018-11-27 Thread Timothy Arceri
---
 src/compiler/nir/nir_loop_analyze.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/compiler/nir/nir_loop_analyze.c 
b/src/compiler/nir/nir_loop_analyze.c
index c804a66ac4..8903e15105 100644
--- a/src/compiler/nir/nir_loop_analyze.c
+++ b/src/compiler/nir/nir_loop_analyze.c
@@ -239,8 +239,8 @@ compute_induction_information(loop_info_state *state)
   nir_foreach_phi_src(src, phi) {
  nir_loop_variable *src_var = get_loop_var(src->src.ssa, state);
 
- /* If one of the sources is in a conditional or nested block then
-  * panic.
+ /* If one of the sources is in an if branch or nested loop then don't
+  * attempt to go any further.
   */
  if (src_var->in_if_branch || src_var->in_nested_loop)
 break;
-- 
2.19.1

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


[Mesa-dev] [PATCH 4/4] nir: detect more induction variables

2018-11-27 Thread Timothy Arceri
This adds allows loop analysis to detect inductions varibales that
are incremented in both branches of an if rather than in a main
loop block. For example:

   loop {
  block block_1:
  /* preds: block_0 block_7 */
  vec1 32 ssa_8 = phi block_0: ssa_4, block_7: ssa_20
  vec1 32 ssa_9 = phi block_0: ssa_0, block_7: ssa_4
  vec1 32 ssa_10 = phi block_0: ssa_1, block_7: ssa_4
  vec1 32 ssa_11 = phi block_0: ssa_2, block_7: ssa_21
  vec1 32 ssa_12 = phi block_0: ssa_3, block_7: ssa_22
  vec4 32 ssa_13 = vec4 ssa_12, ssa_11, ssa_10, ssa_9
  vec1 32 ssa_14 = ige ssa_8, ssa_5
  /* succs: block_2 block_3 */
  if ssa_14 {
 block block_2:
 /* preds: block_1 */
 break
 /* succs: block_8 */
  } else {
 block block_3:
 /* preds: block_1 */
 /* succs: block_4 */
  }
  block block_4:
  /* preds: block_3 */
  vec1 32 ssa_15 = ilt ssa_6, ssa_8
  /* succs: block_5 block_6 */
  if ssa_15 {
 block block_5:
 /* preds: block_4 */
 vec1 32 ssa_16 = iadd ssa_8, ssa_7
 vec1 32 ssa_17 = load_const (0x3f80 /* 1.00*/)
 /* succs: block_7 */
  } else {
 block block_6:
 /* preds: block_4 */
 vec1 32 ssa_18 = iadd ssa_8, ssa_7
 vec1 32 ssa_19 = load_const (0x3f80 /* 1.00*/)
 /* succs: block_7 */
  }
  block block_7:
  /* preds: block_5 block_6 */
  vec1 32 ssa_20 = phi block_5: ssa_16, block_6: ssa_18
  vec1 32 ssa_21 = phi block_5: ssa_17, block_6: ssa_4
  vec1 32 ssa_22 = phi block_5: ssa_4, block_6: ssa_19
  /* succs: block_1 */
   }

Unfortunatly GCM could move the addition out of the if for us
(making this patch unrequired) but we still cannot enable the GCM
pass without regressions.

This unrolls a loop in Rise of The Tomb Raider.

vkpipeline-db results (VEGA):

Totals from affected shaders:
SGPRS: 88 -> 96 (9.09 %)
VGPRS: 56 -> 52 (-7.14 %)
Spilled SGPRs: 0 -> 0 (0.00 %)
Spilled VGPRs: 0 -> 0 (0.00 %)
Private memory VGPRs: 0 -> 0 (0.00 %)
Scratch size: 0 -> 0 (0.00 %) dwords per thread
Code Size: 2168 -> 4560 (110.33 %) bytes
LDS: 0 -> 0 (0.00 %) blocks
Max Waves: 4 -> 4 (0.00 %)
Wait states: 0 -> 0 (0.00 %)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=32211
---
 src/compiler/nir/nir_loop_analyze.c | 36 +
 1 file changed, 36 insertions(+)

diff --git a/src/compiler/nir/nir_loop_analyze.c 
b/src/compiler/nir/nir_loop_analyze.c
index 8903e15105..cf97d6bf06 100644
--- a/src/compiler/nir/nir_loop_analyze.c
+++ b/src/compiler/nir/nir_loop_analyze.c
@@ -245,6 +245,42 @@ compute_induction_information(loop_info_state *state)
  if (src_var->in_if_branch || src_var->in_nested_loop)
 break;
 
+ /* Detect inductions varibales that are incremented in both branches
+  * of an unnested if rather than in a loop block.
+  */
+ if (is_var_phi(src_var)) {
+nir_phi_instr *src_phi =
+   nir_instr_as_phi(src_var->def->parent_instr);
+
+nir_op alu_op;
+nir_ssa_def *alu_srcs[2] = {0};
+nir_foreach_phi_src(src2, src_phi) {
+   nir_loop_variable *src_var2 =
+  get_loop_var(src2->src.ssa, state);
+
+   if (!src_var2->in_if_branch || !is_var_alu(src_var2))
+  break;
+
+   nir_alu_instr *alu =
+  nir_instr_as_alu(src_var2->def->parent_instr);
+   if (nir_op_infos[alu->op].num_inputs != 2)
+  break;
+
+   if (alu->src[0].src.ssa == alu_srcs[0] &&
+   alu->src[1].src.ssa == alu_srcs[1] &&
+   alu->op == alu_op) {
+  /* Both branches perform the same calculation so we can use
+   * one of them to find the induction variable.
+   */
+  src_var = src_var2;
+   } else {
+  alu_srcs[0] = alu->src[0].src.ssa;
+  alu_srcs[1] = alu->src[1].src.ssa;
+  alu_op = alu->op;
+   }
+}
+ }
+
  if (!src_var->in_loop) {
 biv->def_outside_loop = src_var;
  } else if (is_var_alu(src_var)) {
-- 
2.19.1

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


Re: [Mesa-dev] [PATCH] docs: Document the optional usage of Signed-off-by

2018-11-27 Thread Jordan Justen
On 2018-11-27 18:04:17, Matt Turner wrote:
> On Tue, Nov 27, 2018 at 6:00 PM Jordan Justen  
> wrote:
> >
> > On 2018-11-27 17:17:15, Matt Turner wrote:
> > > On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen  
> > > wrote:
> > > >
> > > > This adds the "Developer's Certificate of Origin 1.1" from the Linux
> > > > kernel. It indicates that by using Signed-off-by you are certifying
> > > > that your patch meets the DCO 1.1 guidelines.
> > >
> > > Do we gain anything if it's optional?
> >
> > As I recall, one thing that bothered you about Signed-off-by in Mesa
> > is that it wasn't documented what it meant when it was used.
> >
> > Perhaps there are developers that don't want to use Signed-off-by with
> > an undocumented meaning for Mesa. If that is the case, then this might
> > help. I wasn't sure if you fell into that category.
> >
> > I use -s whenever I commit, so requiring it would not bother me. But,
> > I notice that many people (such as yourself) do not, so I didn't see
> > the need to push for that.
> >
> > If it's well documented, and becomes commonly used, then perhaps
> > requiring it might be a reasonable thing to consider. I won't be
> > holding my breath while waiting on that. :)
> 
> I don't have a problem requiring it. I sign-off on commits I make to
> Gentoo, to Linux, etc.

If it has the same meaning as with the Linux kernel, but is not
required, then you won't use it?

I guess your concern might be that you are then giving something to
the project that others can choose not to. ?

> I'm just against cargo-culting it like we're doing now without a
> defined meaning.

The purpose of this patch is to give it a defined meaning. And the
meaning I chose is the one that people are more likely to have in mind
when they use Signed-off-by. Maybe that's too big of an assumption on
my part, but I think several other open source projects have followed
the kernel on this.

> By all means, require it (with a git hook) if you like.

I personally don't want to push for that right now.

I guess I would like it to be required someday, primarily because it
creates a standard process for open source projects to use. (So,
people are more likely to be used to it in general when contributing
to open source projects.)

But, I'm not confident that the consensus for Mesa would be in favor
of making that change right now. So, as an alternative I'd like to
remove any barriers (such as ambiguity) to its usage in Mesa.

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


[Mesa-dev] [PATCH 1/4] nir: add if opt opt_if_loop_last_continue()

2018-11-27 Thread Timothy Arceri
From: Danylo Piliaiev 

Removing the last continue can allow more loops to unroll. Also
inserting code into the if branch can allow the various if opts
to progress further.

The insertion of some loops into the if branch also reduces VGPR
use in some shaders.

vkpipeline-db results (VEGA):

Totals from affected shaders:
SGPRS: 6552 -> 6576 (0.37 %)
VGPRS: 6544 -> 6532 (-0.18 %)
Spilled SGPRs: 0 -> 0 (0.00 %)
Spilled VGPRs: 0 -> 0 (0.00 %)
Private memory VGPRs: 0 -> 0 (0.00 %)
Scratch size: 0 -> 0 (0.00 %) dwords per thread
Code Size: 481952 -> 478032 (-0.81 %) bytes
LDS: 13 -> 13 (0.00 %) blocks
Max Waves: 241 -> 242 (0.41 %)
Wait states: 0 -> 0 (0.00 %)

Shader-db results radeonsi (VEGA):

Totals from affected shaders:
SGPRS: 168 -> 168 (0.00 %)
VGPRS: 144 -> 140 (-2.78 %)
Spilled SGPRs: 157 -> 157 (0.00 %)
Spilled VGPRs: 0 -> 0 (0.00 %)
Private memory VGPRs: 0 -> 0 (0.00 %)
Scratch size: 0 -> 0 (0.00 %) dwords per thread
Code Size: 8524 -> 8488 (-0.42 %) bytes
LDS: 0 -> 0 (0.00 %) blocks
Max Waves: 7 -> 7 (0.00 %)
Wait states: 0 -> 0 (0.00 %)

v2: (Timothy Arceri):
- allow for continues in either branch
- move any trailing loops inside the if as well as blocks.
- leave nir_opt_trivial_continues() to actually remove the
  continue.

Signed-off-by: Timothy Arceri 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=32211
---
 src/compiler/nir/nir_opt_if.c | 95 +++
 1 file changed, 95 insertions(+)

diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index dd488b1787..4a9dffb782 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -263,6 +263,100 @@ rewrite_phi_predecessor_blocks(nir_if *nif,
}
 }
 
+static bool
+nir_block_ends_in_continue(nir_block *block)
+{
+   if (exec_list_is_empty(>instr_list))
+  return false;
+
+   nir_instr *instr = nir_block_last_instr(block);
+   return instr->type == nir_instr_type_jump &&
+  nir_instr_as_jump(instr)->type == nir_jump_continue;
+}
+
+/**
+ * This optimization turns:
+ *
+ * loop {
+ *...
+ *if (cond) {
+ *   do_work_1();
+ *   continue;
+ *} else {
+ *}
+ *do_work_2();
+ * }
+ *
+ * into:
+ *
+ * loop {
+ *...
+ *if (cond) {
+ *   do_work_1();
+ *   continue;
+ *} else {
+ *   do_work_2();
+ *}
+ * }
+ *
+ * The continue should then be removed by nir_opt_trivial_continues() and the
+ * loop can potentially be unrolled.
+ *
+ * Note: do_work_2() is only ever blocks and nested loops. We could also nest
+ * other if-statments in the branch which would allow further continues to
+ * be removed. However in practice this can result in increased register
+ * pressure.
+ */
+static bool
+opt_if_loop_last_continue(nir_loop *loop)
+{
+   /* Get the last if-stament in the loop */
+   nir_block *last_block = nir_loop_last_block(loop);
+   nir_cf_node *if_node = nir_cf_node_prev(_block->cf_node);
+   while (if_node) {
+  if (if_node->type == nir_cf_node_if)
+ break;
+
+  if_node = nir_cf_node_prev(if_node);
+   }
+
+   if (!if_node || if_node->type != nir_cf_node_if)
+  return false;
+
+   nir_if *nif = nir_cf_node_as_if(if_node);
+   nir_block *then_block = nir_if_last_then_block(nif);
+   nir_block *else_block = nir_if_last_else_block(nif);
+
+   bool then_ends_in_continue = nir_block_ends_in_continue(then_block);
+   bool else_ends_in_continue = nir_block_ends_in_continue(else_block);
+
+   /* If both branches end in a continue do nothing, this should be handled
+* by nir_opt_dead_cf().
+*/
+   if (then_ends_in_continue && else_ends_in_continue)
+  return false;
+
+   if (!then_ends_in_continue && !else_ends_in_continue)
+  return false;
+
+   /* Move the last block of the loop inside the last if-statement */
+   nir_cf_list tmp;
+   nir_cf_extract(, nir_after_cf_node(if_node),
+nir_after_block(last_block));
+   if (then_ends_in_continue) {
+  nir_cf_reinsert(, nir_after_cf_list(>else_list));
+   } else {
+  nir_cf_reinsert(, nir_after_cf_list(>then_list));
+   }
+
+   /* In order to avoid running nir_lower_regs_to_ssa_impl() every time an if
+* opt makes progress we leave nir_opt_trivial_continues() to remove the
+* continue now that the end of the loop has been simplified.
+*/
+
+   return true;
+}
+
 /**
  * This optimization turns:
  *
@@ -700,6 +794,7 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
  nir_loop *loop = nir_cf_node_as_loop(cf_node);
  progress |= opt_if_cf_list(b, >body);
  progress |= opt_peel_loop_initial_if(loop);
+ progress |= opt_if_loop_last_continue(loop);
  break;
   }
 
-- 
2.19.1

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


Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process

2018-11-27 Thread Matt Turner
On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen  wrote:
>
> This documents a mechanism for using GitLab Merge Requests as an
> optional, secondary way to get code reviews for patchsets.
>
> We still require all patches to be emailed.
>
> Aside from the potential usage for code review comments, it might also
> help reviewers to find unmerged patchsets which need review. (Assuming
> it doesn't fall into the same fate of patchwork with hundreds of open
> MRs.)
>
> Signed-off-by: Jordan Justen 
> Cc: Jason Ekstrand 
> ---
>  docs/submittingpatches.html | 25 +
>  1 file changed, 25 insertions(+)
>
> diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
> index 5d8ba443191..852f28c198a 100644
> --- a/docs/submittingpatches.html
> +++ b/docs/submittingpatches.html
> @@ -24,6 +24,7 @@
>  Testing Patches
>  Mailing Patches
>  Reviewing Patches
> +GitLab Merge Requests
>  Nominating a commit for a stable branch
>  Criteria for accepting patches to the stable 
> branch
>  Sending backports for the stable branch
> @@ -282,6 +283,30 @@ which tells the patch author that the patch can be 
> committed, as long
>  as the issues are resolved first.
>  
>
> +GitLab Merge Requests
> +
> +
> +  https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge
> +  Requests (MR) can be used as an optional, secondary method of
> +  obtaining code review for patches.
> +
> +
> +
> +  All patches should be submitted using email as well
> +  Consider including a link to the MR in your email based
> +cover-letter
> +  Address code review from both email and the MR

Discussion point: I think attempting to have simultaneous review in
two places is a recipe for wasted time. Strawman: maybe we can only
email the cover letter to the mailing list and include in it a link to
the MR?

> +  Add appropriate labels to your MR. For example:
> +
> +  Mesa changes affect all drivers: mesa
> +  Hardware vendor specific code: amd, intel, nvidia, etc
> +  Driver specific code: anvil, freedreno, i965, iris, radeonsi, 
> radv, vc4, etc
> +  Other tag examples: gallium, util
> +
> +  Never use the merge button on the GitLab page to push patches

Can we disable this in Gitlab? If the button is there, people *will*
accidentally press it.

> +  Close your MR when your patches get pushed!

Gentoo has some automation that scans the git log for "Closes: ..."
and automatically closes the merge requests or bugzilla bugs.

Closes: https://github.com/gentoo/gentoo/pull/7423
Closes: https://bugs.gentoo.org/603294

I'm not sure if it's a custom thing or what (I can find out) but I'd
much prefer to automate things if we can. Just like patchwork, people
*will* forget to close merge requests if it's possible. (And people
currently *do* forget to close bugzilla bugs)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] st/mesa: expose GL_OES_texture_view

2018-11-27 Thread Ilia Mirkin
On Tue, Nov 27, 2018 at 9:00 PM Marek Olšák  wrote:
>
> From: Marek Olšák 
>
> For format fallbacks like ETC and ASTC, switching between sRGB and linear
> decoding is undefined, or at least is not bit-exact. Same as
> EXT_texture_sRGB_decode on GLES.
>
> There are no piglit or dEQP regresssions.

Just checking -- did you run dEQP on hardware that uses the ASTC
fallbacks? If not, could you?

Also, what are the conditions for enabling ASTC fallbacks? Basically
the issue is that OES_texture_view requires ASTC, so I want to make
sure it'll be enabled for any drivers that
PIPE_CAP_SAMPLER_VIEW_TARGET is enabled for.

Assuming neither of those have surprising responses, this series is

Reviewed-by: Ilia Mirkin 

> ---
>  docs/features.txt  | 2 +-
>  docs/relnotes/19.0.0.html  | 1 +
>  src/mesa/state_tracker/st_extensions.c | 1 +
>  3 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/docs/features.txt b/docs/features.txt
> index a97f998a5cc..8999e42519c 100644
> --- a/docs/features.txt
> +++ b/docs/features.txt
> @@ -331,21 +331,21 @@ Khronos, ARB, and OES extensions that are not part of 
> any OpenGL or OpenGL ES ve
>GL_OES_EGL_image  DONE (all drivers)
>GL_OES_EGL_image_external DONE (all drivers)
>GL_OES_EGL_image_external_essl3   DONE (all drivers)
>GL_OES_required_internalformatDONE (all drivers)
>GL_OES_surfaceless_contextDONE (all drivers)
>GL_OES_texture_compression_astc   DONE (core only)
>GL_OES_texture_float  DONE (freedreno, 
> i965, r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe)
>GL_OES_texture_float_linear   DONE (freedreno, 
> i965, r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe)
>GL_OES_texture_half_float DONE (freedreno, 
> i965, r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe)
>GL_OES_texture_half_float_linear  DONE (freedreno, 
> i965, r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe)
> -  GL_OES_texture_view   DONE (i965/gen8+)
> +  GL_OES_texture_view   DONE (i965/gen8+, 
> freedreno, nv50, r600, radeonsi, nv50, nvc0, llvmpipe, softpipe, swr)
>GL_OES_viewport_array DONE (i965, nvc0, 
> radeonsi)
>GLX_ARB_context_flush_control not started
>GLX_ARB_robustness_application_isolation  not started
>GLX_ARB_robustness_share_group_isolation  not started
>
>  GL_EXT_direct_state_access subfeatures (in the spec order):
>GL 1.1: Client commands   not started
>GL 1.0-1.3: Matrix and transpose matrix commands  not started
>GL 1.1-1.2: Texture commands  not started
>GL 1.2: 3D texture commands   not started
> diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
> index 1b839b0a485..f66f22132e1 100644
> --- a/docs/relnotes/19.0.0.html
> +++ b/docs/relnotes/19.0.0.html
> @@ -33,20 +33,21 @@ Compatibility contexts may report a lower version 
> depending on each driver.
>  SHA256 checksums
>  
>  TBD.
>  
>
>
>  New features
>
>  
>  GL_EXT_shader_implicit_conversions on all drivers (ES extension).
> +GL_OES_texture_view on drivers supporting texture views (ES 
> extension).
>  
>
>  Bug fixes
>
>  
>  TBD
>  
>
>  Changes
>
> diff --git a/src/mesa/state_tracker/st_extensions.c 
> b/src/mesa/state_tracker/st_extensions.c
> index 16889074f66..5c068d14e93 100644
> --- a/src/mesa/state_tracker/st_extensions.c
> +++ b/src/mesa/state_tracker/st_extensions.c
> @@ -760,20 +760,21 @@ void st_init_extensions(struct pipe_screen *screen,
>{ o(NV_fill_rectangle),
> PIPE_CAP_POLYGON_MODE_FILL_RECTANGLE  },
>{ o(NV_primitive_restart), PIPE_CAP_PRIMITIVE_RESTART  
>   },
>{ o(NV_texture_barrier),   PIPE_CAP_TEXTURE_BARRIER
>   },
>{ o(NVX_gpu_memory_info),  PIPE_CAP_QUERY_MEMORY_INFO  
>   },
>/* GL_NV_point_sprite is not supported by gallium because we don't
> * support the GL_POINT_SPRITE_R_MODE_NV option. */
>
>{ o(OES_standard_derivatives), PIPE_CAP_SM3
>   },
>{ o(OES_texture_float_linear), PIPE_CAP_TEXTURE_FLOAT_LINEAR   
>   },
>{ o(OES_texture_half_float_linear),
> PIPE_CAP_TEXTURE_HALF_FLOAT_LINEAR},
> +  { o(OES_texture_view), PIPE_CAP_SAMPLER_VIEW_TARGET
>   },
> };
>
> /* Required: render target and sampler support */
> static const struct st_extension_format_mapping rendertarget_mapping[] = {

Re: [Mesa-dev] [PATCH 6/6] nir: Release per-block metadata in nir_sweep

2018-11-27 Thread Jason Ekstrand

Rb for this patch

On November 27, 2018 18:39:25 "Ian Romanick"  wrote:


From: Ian Romanick 

nir_sweep already marks all metadata invalid, so it is safe to release
the memory here too.

mean soft fp64 using uint64:   1,342,759,331 => 1,010,670,475
gfxbench5 aztec ruins high 11:63,555,571 =>61,889,811
deus ex mankind divided 148:  62,845,304 =>62,829,640
deus ex mankind divided 2890: 71,922,686 =>71,922,686
dirt showdown 676:69,238,607 =>69,238,607
dolphin ubershaders 210:  77,822,072 =>77,822,072

Signed-off-by: Ian Romanick 
---
src/compiler/nir/nir_sweep.c | 9 +
1 file changed, 9 insertions(+)

diff --git a/src/compiler/nir/nir_sweep.c b/src/compiler/nir/nir_sweep.c
index aab641388db..b6b56aa078c 100644
--- a/src/compiler/nir/nir_sweep.c
+++ b/src/compiler/nir/nir_sweep.c
@@ -63,6 +63,15 @@ sweep_block(nir_shader *nir, nir_block *block)
{
   ralloc_steal(nir, block);

+   /* sweep_impl will mark all metadata invalid.  We can safely release all of
+* this here.
+*/
+   ralloc_free(block->live_in);
+   block->live_in = NULL;
+
+   ralloc_free(block->live_out);
+   block->live_out = NULL;
+
   nir_foreach_instr(instr, block) {
  ralloc_steal(nir, instr);

--
2.14.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] docs: Document the optional usage of Signed-off-by

2018-11-27 Thread Matt Turner
On Tue, Nov 27, 2018 at 6:00 PM Jordan Justen  wrote:
>
> On 2018-11-27 17:17:15, Matt Turner wrote:
> > On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen  
> > wrote:
> > >
> > > This adds the "Developer's Certificate of Origin 1.1" from the Linux
> > > kernel. It indicates that by using Signed-off-by you are certifying
> > > that your patch meets the DCO 1.1 guidelines.
> >
> > Do we gain anything if it's optional?
>
> As I recall, one thing that bothered you about Signed-off-by in Mesa
> is that it wasn't documented what it meant when it was used.
>
> Perhaps there are developers that don't want to use Signed-off-by with
> an undocumented meaning for Mesa. If that is the case, then this might
> help. I wasn't sure if you fell into that category.
>
> I use -s whenever I commit, so requiring it would not bother me. But,
> I notice that many people (such as yourself) do not, so I didn't see
> the need to push for that.
>
> If it's well documented, and becomes commonly used, then perhaps
> requiring it might be a reasonable thing to consider. I won't be
> holding my breath while waiting on that. :)

I don't have a problem requiring it. I sign-off on commits I make to
Gentoo, to Linux, etc. I'm just against cargo-culting it like we're
doing now without a defined meaning.

By all means, require it (with a git hook) if you like.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] mesa: expose GL_EXT_texture_view as an alias of GL_OES_texture_view

2018-11-27 Thread Marek Olšák
From: Marek Olšák 

There no spec changes.
---
 docs/relnotes/19.0.0.html|  1 +
 src/mapi/glapi/gen/es_EXT.xml| 13 +
 src/mesa/main/extensions_table.h |  1 +
 3 files changed, 15 insertions(+)

diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
index f66f22132e1..920cf803f5d 100644
--- a/docs/relnotes/19.0.0.html
+++ b/docs/relnotes/19.0.0.html
@@ -33,20 +33,21 @@ Compatibility contexts may report a lower version depending 
on each driver.
 SHA256 checksums
 
 TBD.
 
 
 
 New features
 
 
 GL_EXT_shader_implicit_conversions on all drivers (ES extension).
+GL_EXT_texture_view on drivers supporting texture views (ES extension).
 GL_OES_texture_view on drivers supporting texture views (ES 
extension).
 
 
 Bug fixes
 
 
 TBD
 
 
 Changes
diff --git a/src/mapi/glapi/gen/es_EXT.xml b/src/mapi/glapi/gen/es_EXT.xml
index 459f642e477..bbc4a1a1118 100644
--- a/src/mapi/glapi/gen/es_EXT.xml
+++ b/src/mapi/glapi/gen/es_EXT.xml
@@ -1453,11 +1453,24 @@
 
 
 
 
 
 
 

 
 
+
+
+
+
+
+
+
+
+
+
+   
+
+
 
diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
index dd846d7bc5c..4f2707b65a4 100644
--- a/src/mesa/main/extensions_table.h
+++ b/src/mesa/main/extensions_table.h
@@ -295,20 +295,21 @@ EXT(EXT_texture_norm16  , dummy_true
 EXT(EXT_texture_object  , dummy_true   
  , GLL,  x ,  x ,  x , 1995)
 EXT(EXT_texture_rectangle   , NV_texture_rectangle 
  , GLL,  x ,  x ,  x , 2004)
 EXT(EXT_texture_rg  , ARB_texture_rg   
  ,  x ,  x ,  x , ES2, 2011)
 EXT(EXT_texture_sRGB, EXT_texture_sRGB 
  , GLL, GLC,  x ,  x , 2004)
 EXT(EXT_texture_sRGB_R8 , EXT_texture_sRGB_R8  
  ,  x ,  x ,  x ,  30, 2015)
 EXT(EXT_texture_sRGB_decode , EXT_texture_sRGB_decode  
  , GLL, GLC,  x ,  30, 2006)
 EXT(EXT_texture_shared_exponent , EXT_texture_shared_exponent  
  , GLL, GLC,  x ,  x , 2004)
 EXT(EXT_texture_snorm   , EXT_texture_snorm
  , GLL, GLC,  x ,  x , 2009)
 EXT(EXT_texture_swizzle , EXT_texture_swizzle  
  , GLL, GLC,  x ,  x , 2008)
 EXT(EXT_texture_type_2_10_10_10_REV , EXT_texture_type_2_10_10_10_REV  
  ,  x ,  x ,  x , ES2, 2008)
+EXT(EXT_texture_view, OES_texture_view 
  ,  x ,  x ,  x ,  31, 2014)
 EXT(EXT_timer_query , EXT_timer_query  
  , GLL, GLC,  x ,  x , 2006)
 EXT(EXT_transform_feedback  , EXT_transform_feedback   
  , GLL, GLC,  x ,  x , 2011)
 EXT(EXT_unpack_subimage , dummy_true   
  ,  x ,  x ,  x , ES2, 2011)
 EXT(EXT_vertex_array, dummy_true   
  , GLL,  x ,  x ,  x , 1995)
 EXT(EXT_vertex_array_bgra   , EXT_vertex_array_bgra
  , GLL, GLC,  x ,  x , 2008)
 EXT(EXT_vertex_attrib_64bit , ARB_vertex_attrib_64bit  
  ,  32, GLC,  x ,  x , 2010)
 EXT(EXT_window_rectangles   , EXT_window_rectangles
  , GLL, GLC,  x ,  30, 2016)
 
 EXT(GREMEDY_string_marker   , GREMEDY_string_marker
  , GLL, GLC,  x ,  x , 2007)
 
-- 
2.17.1

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


[Mesa-dev] [PATCH 1/2] st/mesa: expose GL_OES_texture_view

2018-11-27 Thread Marek Olšák
From: Marek Olšák 

For format fallbacks like ETC and ASTC, switching between sRGB and linear
decoding is undefined, or at least is not bit-exact. Same as
EXT_texture_sRGB_decode on GLES.

There are no piglit or dEQP regresssions.
---
 docs/features.txt  | 2 +-
 docs/relnotes/19.0.0.html  | 1 +
 src/mesa/state_tracker/st_extensions.c | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/features.txt b/docs/features.txt
index a97f998a5cc..8999e42519c 100644
--- a/docs/features.txt
+++ b/docs/features.txt
@@ -331,21 +331,21 @@ Khronos, ARB, and OES extensions that are not part of any 
OpenGL or OpenGL ES ve
   GL_OES_EGL_image  DONE (all drivers)
   GL_OES_EGL_image_external DONE (all drivers)
   GL_OES_EGL_image_external_essl3   DONE (all drivers)
   GL_OES_required_internalformatDONE (all drivers)
   GL_OES_surfaceless_contextDONE (all drivers)
   GL_OES_texture_compression_astc   DONE (core only)
   GL_OES_texture_float  DONE (freedreno, i965, 
r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe)
   GL_OES_texture_float_linear   DONE (freedreno, i965, 
r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe)
   GL_OES_texture_half_float DONE (freedreno, i965, 
r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe)
   GL_OES_texture_half_float_linear  DONE (freedreno, i965, 
r300, r600, radeonsi, nv30, nv50, nvc0, softpipe, llvmpipe)
-  GL_OES_texture_view   DONE (i965/gen8+)
+  GL_OES_texture_view   DONE (i965/gen8+, 
freedreno, nv50, r600, radeonsi, nv50, nvc0, llvmpipe, softpipe, swr)
   GL_OES_viewport_array DONE (i965, nvc0, 
radeonsi)
   GLX_ARB_context_flush_control not started
   GLX_ARB_robustness_application_isolation  not started
   GLX_ARB_robustness_share_group_isolation  not started
 
 GL_EXT_direct_state_access subfeatures (in the spec order):
   GL 1.1: Client commands   not started
   GL 1.0-1.3: Matrix and transpose matrix commands  not started
   GL 1.1-1.2: Texture commands  not started
   GL 1.2: 3D texture commands   not started
diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
index 1b839b0a485..f66f22132e1 100644
--- a/docs/relnotes/19.0.0.html
+++ b/docs/relnotes/19.0.0.html
@@ -33,20 +33,21 @@ Compatibility contexts may report a lower version depending 
on each driver.
 SHA256 checksums
 
 TBD.
 
 
 
 New features
 
 
 GL_EXT_shader_implicit_conversions on all drivers (ES extension).
+GL_OES_texture_view on drivers supporting texture views (ES 
extension).
 
 
 Bug fixes
 
 
 TBD
 
 
 Changes
 
diff --git a/src/mesa/state_tracker/st_extensions.c 
b/src/mesa/state_tracker/st_extensions.c
index 16889074f66..5c068d14e93 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -760,20 +760,21 @@ void st_init_extensions(struct pipe_screen *screen,
   { o(NV_fill_rectangle),
PIPE_CAP_POLYGON_MODE_FILL_RECTANGLE  },
   { o(NV_primitive_restart), PIPE_CAP_PRIMITIVE_RESTART
},
   { o(NV_texture_barrier),   PIPE_CAP_TEXTURE_BARRIER  
},
   { o(NVX_gpu_memory_info),  PIPE_CAP_QUERY_MEMORY_INFO
},
   /* GL_NV_point_sprite is not supported by gallium because we don't
* support the GL_POINT_SPRITE_R_MODE_NV option. */
 
   { o(OES_standard_derivatives), PIPE_CAP_SM3  
},
   { o(OES_texture_float_linear), PIPE_CAP_TEXTURE_FLOAT_LINEAR 
},
   { o(OES_texture_half_float_linear),
PIPE_CAP_TEXTURE_HALF_FLOAT_LINEAR},
+  { o(OES_texture_view), PIPE_CAP_SAMPLER_VIEW_TARGET  
},
};
 
/* Required: render target and sampler support */
static const struct st_extension_format_mapping rendertarget_mapping[] = {
   { { o(ARB_texture_float) },
 { PIPE_FORMAT_R32G32B32A32_FLOAT,
   PIPE_FORMAT_R16G16B16A16_FLOAT } },
 
   { { o(OES_texture_float) },
 { PIPE_FORMAT_R32G32B32A32_FLOAT } },
-- 
2.17.1

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


Re: [Mesa-dev] [PATCH] docs: Document the optional usage of Signed-off-by

2018-11-27 Thread Jordan Justen
On 2018-11-27 17:17:15, Matt Turner wrote:
> On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen  
> wrote:
> >
> > This adds the "Developer's Certificate of Origin 1.1" from the Linux
> > kernel. It indicates that by using Signed-off-by you are certifying
> > that your patch meets the DCO 1.1 guidelines.
> 
> Do we gain anything if it's optional?

As I recall, one thing that bothered you about Signed-off-by in Mesa
is that it wasn't documented what it meant when it was used.

Perhaps there are developers that don't want to use Signed-off-by with
an undocumented meaning for Mesa. If that is the case, then this might
help. I wasn't sure if you fell into that category.

I use -s whenever I commit, so requiring it would not bother me. But,
I notice that many people (such as yourself) do not, so I didn't see
the need to push for that.

If it's well documented, and becomes commonly used, then perhaps
requiring it might be a reasonable thing to consider. I won't be
holding my breath while waiting on that. :)

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


Re: [Mesa-dev] [PATCH] docs: Document the optional usage of Signed-off-by

2018-11-27 Thread Matt Turner
On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen  wrote:
>
> This adds the "Developer's Certificate of Origin 1.1" from the Linux
> kernel. It indicates that by using Signed-off-by you are certifying
> that your patch meets the DCO 1.1 guidelines.

Do we gain anything if it's optional?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] docs: Document optional GitLab code review process

2018-11-27 Thread Jordan Justen
This documents a mechanism for using GitLab Merge Requests as an
optional, secondary way to get code reviews for patchsets.

We still require all patches to be emailed.

Aside from the potential usage for code review comments, it might also
help reviewers to find unmerged patchsets which need review. (Assuming
it doesn't fall into the same fate of patchwork with hundreds of open
MRs.)

Signed-off-by: Jordan Justen 
Cc: Jason Ekstrand 
---
 docs/submittingpatches.html | 25 +
 1 file changed, 25 insertions(+)

diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
index 5d8ba443191..852f28c198a 100644
--- a/docs/submittingpatches.html
+++ b/docs/submittingpatches.html
@@ -24,6 +24,7 @@
 Testing Patches
 Mailing Patches
 Reviewing Patches
+GitLab Merge Requests
 Nominating a commit for a stable branch
 Criteria for accepting patches to the stable branch
 Sending backports for the stable branch
@@ -282,6 +283,30 @@ which tells the patch author that the patch can be 
committed, as long
 as the issues are resolved first.
 
 
+GitLab Merge Requests
+
+
+  https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge
+  Requests (MR) can be used as an optional, secondary method of
+  obtaining code review for patches.
+
+
+
+  All patches should be submitted using email as well
+  Consider including a link to the MR in your email based
+cover-letter
+  Address code review from both email and the MR
+  Add appropriate labels to your MR. For example:
+
+  Mesa changes affect all drivers: mesa
+  Hardware vendor specific code: amd, intel, nvidia, etc
+  Driver specific code: anvil, freedreno, i965, iris, radeonsi, radv, 
vc4, etc
+  Other tag examples: gallium, util
+
+  Never use the merge button on the GitLab page to push patches
+  Add Reviewed-by tags to your commits and push using git
+  Close your MR when your patches get pushed!
+
 
 Nominating a commit for a stable branch
 
-- 
2.20.0.rc1

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


[Mesa-dev] [PATCH] docs: Document the optional usage of Signed-off-by

2018-11-27 Thread Jordan Justen
This adds the "Developer's Certificate of Origin 1.1" from the Linux
kernel. It indicates that by using Signed-off-by you are certifying
that your patch meets the DCO 1.1 guidelines.

Signed-off-by: Jordan Justen 
Cc: Matt Turner 
---
 docs/submittingpatches.html | 53 -
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
index 3f97c941aa5..5d8ba443191 100644
--- a/docs/submittingpatches.html
+++ b/docs/submittingpatches.html
@@ -20,6 +20,7 @@
 
 Basic guidelines
 Patch formatting
+Patch signing
 Testing Patches
 Mailing Patches
 Reviewing Patches
@@ -72,7 +73,9 @@ if needed.  For example:
 is necessary, and removing it causes no regressions in piglit on any
 platform.
 
-A "Signed-off-by:" line is not required, but not discouraged either.
+A "Signed-off-by:" line is not required, but not discouraged
+either. The format and meaning of Signed-off-by is documented below in
+the patch signing section.
 If a patch addresses a bugzilla issue, that should be noted in the
 patch comment.  For example:
 
@@ -128,7 +131,55 @@ Please use common sense and do not 
blindly add everyone.
 
 
 
+Patch signing
 
+
+  Note: Patch signing is optional for the Mesa project.
+
+
+
+  As described in the patch formatting
+  section, you can optionally sign your patch by including
+  Signed-off-by in the patch commit message. The Signed-off-by must
+  include your real name and email address in this format:
+
+
+  Signed-off-by: Joe Hacker jhac...@foo.com
+
+
+  By adding Signed-of-by to your contributed patch, you certify that
+  your contribution meets the guidelines of the Developer's
+  Certificate of Origin:
+
+
+
+Developer's Certificate of Origin 1.1
+-
+
+By making a contribution to this project, I certify that:
+
+ (a) The contribution was created in whole or in part by me and I
+ have the right to submit it under the open source license
+ indicated in the file; or
+
+ (b) The contribution is based upon previous work that, to the best
+ of my knowledge, is covered under an appropriate open source
+ license and I have the right under that license to submit that
+ work with modifications, whether created in whole or in part
+ by me, under the same open source license (unless I am
+ permitted to submit under a different license), as indicated
+ in the file; or
+
+ (c) The contribution was provided directly to me by some other
+ person who certified (a), (b) or (c) and I have not modified
+ it.
+
+ (d) I understand and agree that this project and the contribution
+ are public and that a record of the contribution (including all
+ personal information I submit with it, including my sign-off) is
+ maintained indefinitely and may be redistributed consistent with
+ this project or the open source license(s) involved.
+
 
 Testing Patches
 
-- 
2.20.0.rc1

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


[Mesa-dev] [PATCH 4/6] nir/phi_builder: Use per-value hash table to store [block] -> def mapping

2018-11-27 Thread Ian Romanick
From: Ian Romanick 

Replace the old array in each value with a hash table in each value.

Changes in peak memory usage according to Valgrind massif:

mean soft fp64 using uint64:   5,499,875,082 => 1,343,991,403
gfxbench5 aztec ruins high 11:63,619,971 =>63,619,971
deus ex mankind divided 148:  62,887,728 =>62,887,728
deus ex mankind divided 2890: 72,402,222 =>72,399,750
dirt showdown 676:74,466,431 =>69,464,023
dolphin ubershaders 210: 109,630,376 =>78,359,728

Run-time change for a full run on shader-db on my Skylake laptop (with
-march=native) is 0.590037% +/- 0.0873431% (n=50).  This is about +0.6
seconds on a 111 second run.

The previous version of this patch used a single hash table for the
whole phi builder.  The mapping was from [value, block] -> def, so a
separate allocation was needed for each [value, block] tuple.  There was
quite a bit of per-allocation overhead (due to ralloc), so the patch was
followed by a patch that added the use of the slab allocator.  The
results of those two patches was not quite as good:

mean soft fp64 using uint64:   5,499,875,082 => 1,343,991,403
gfxbench5 aztec ruins high 11:63,619,971 =>63,619,971
deus ex mankind divided 148:  62,887,728 =>62,887,728
deus ex mankind divided 2890: 72,402,222 =>72,402,222 *
dirt showdown 676:74,466,431 =>72,443,591 *
dolphin ubershaders 210: 109,630,376 =>81,034,320 *

The * denote tests that are better now.  In the tests that are the same
in both patches, the "after" peak memory usage was at a different
location.  I did not check the local peaks.

Signed-off-by: Ian Romanick 
Suggested-by: Jason Ekstrand 
---
 src/compiler/nir/nir_phi_builder.c | 45 ++
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/src/compiler/nir/nir_phi_builder.c 
b/src/compiler/nir/nir_phi_builder.c
index add3efd2dfc..621777d6ecc 100644
--- a/src/compiler/nir/nir_phi_builder.c
+++ b/src/compiler/nir/nir_phi_builder.c
@@ -75,9 +75,18 @@ struct nir_phi_builder_value {
 *  - A regular SSA def.  This will be either the result of a phi node or
 *one of the defs provided by nir_phi_builder_value_set_blocK_def().
 */
-   nir_ssa_def *defs[0];
+   struct hash_table ht;
 };
 
+/**
+ * Convert a block index into a value that can be used as a key for a hash 
table
+ *
+ * The hash table functions want a pointer that is not \c NULL.
+ * _mesa_hash_pointer drops the two least significant bits, but that's where
+ * most of our data likely is.  Shift by 2 and add 1 to make everything happy.
+ */
+#define INDEX_TO_KEY(x) ((void *)(uintptr_t) ((x << 2) + 1))
+
 struct nir_phi_builder *
 nir_phi_builder_create(nir_function_impl *impl)
 {
@@ -111,13 +120,16 @@ nir_phi_builder_add_value(struct nir_phi_builder *pb, 
unsigned num_components,
struct nir_phi_builder_value *val;
unsigned i, w_start = 0, w_end = 0;
 
-   val = rzalloc_size(pb, sizeof(*val) + sizeof(val->defs[0]) * 
pb->num_blocks);
+   val = rzalloc_size(pb, sizeof(*val));
val->builder = pb;
val->num_components = num_components;
val->bit_size = bit_size;
exec_list_make_empty(>phis);
exec_list_push_tail(>values, >node);
 
+   _mesa_hash_table_init(>ht, pb, _mesa_hash_pointer,
+ _mesa_key_pointer_equal);
+
pb->iter_count++;
 
BITSET_WORD tmp;
@@ -142,7 +154,7 @@ nir_phi_builder_add_value(struct nir_phi_builder *pb, 
unsigned num_components,
  if (next == pb->impl->end_block)
 continue;
 
- if (val->defs[next->index] == NULL) {
+ if (_mesa_hash_table_search(>ht, INDEX_TO_KEY(next->index)) == 
NULL) {
 /* Instead of creating a phi node immediately, we simply set the
  * value to the magic value NEEDS_PHI.  Later, we create phi nodes
  * on demand in nir_phi_builder_value_get_block_def().
@@ -164,7 +176,7 @@ void
 nir_phi_builder_value_set_block_def(struct nir_phi_builder_value *val,
 nir_block *block, nir_ssa_def *def)
 {
-   val->defs[block->index] = def;
+   _mesa_hash_table_insert(>ht, INDEX_TO_KEY(block->index), def);
 }
 
 nir_ssa_def *
@@ -175,8 +187,18 @@ nir_phi_builder_value_get_block_def(struct 
nir_phi_builder_value *val,
 * have a valid ssa_def, if any.
 */
nir_block *dom = block;
-   while (dom && val->defs[dom->index] == NULL)
+   struct hash_entry *he = NULL;
+
+   while (dom != NULL) {
+  he = _mesa_hash_table_search(>ht, INDEX_TO_KEY(dom->index));
+  if (he != NULL)
+ break;
+
   dom = dom->imm_dom;
+   }
+
+   /* Exactly one of (he != NULL) and (dom == NULL) must be true. */
+   assert((he != NULL) != (dom == NULL));
 
nir_ssa_def *def;
if (dom == NULL) {
@@ -191,7 +213,7 @@ nir_phi_builder_value_get_block_def(struct 
nir_phi_builder_value *val,
   nir_instr_insert(nir_before_cf_list(>builder->impl->body),
 

[Mesa-dev] [PATCH 2/6] util/slab: Rename slab_mempool typed parameters to mempool

2018-11-27 Thread Ian Romanick
From: Ian Romanick 

Now everything with type 'struct slab_child_pool *' is name pool, and
everything with type 'struct slab_mempool *' is named mempool.

Signed-off-by: Ian Romanick 
---
 src/util/slab.c | 20 ++--
 src/util/slab.h |  8 
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/util/slab.c b/src/util/slab.c
index 5f048666b56..5477c75d443 100644
--- a/src/util/slab.c
+++ b/src/util/slab.c
@@ -280,25 +280,25 @@ void slab_free(struct slab_child_pool *pool, void *ptr)
  * Allocate an object from the slab. Single-threaded (no mutex).
  */
 void *
-slab_alloc_st(struct slab_mempool *pool)
+slab_alloc_st(struct slab_mempool *mempool)
 {
-   return slab_alloc(>child);
+   return slab_alloc(>child);
 }
 
 /**
  * Free an object allocated from the slab. Single-threaded (no mutex).
  */
 void
-slab_free_st(struct slab_mempool *pool, void *ptr)
+slab_free_st(struct slab_mempool *mempool, void *ptr)
 {
-   slab_free(>child, ptr);
+   slab_free(>child, ptr);
 }
 
 void
-slab_destroy(struct slab_mempool *pool)
+slab_destroy(struct slab_mempool *mempool)
 {
-   slab_destroy_child(>child);
-   slab_destroy_parent(>parent);
+   slab_destroy_child(>child);
+   slab_destroy_parent(>parent);
 }
 
 /**
@@ -308,10 +308,10 @@ slab_destroy(struct slab_mempool *pool)
  * \param num_items Number of objects to allocate at once.
  */
 void
-slab_create(struct slab_mempool *pool,
+slab_create(struct slab_mempool *mempool,
 unsigned item_size,
 unsigned num_items)
 {
-   slab_create_parent(>parent, item_size, num_items);
-   slab_create_child(>child, >parent);
+   slab_create_parent(>parent, item_size, num_items);
+   slab_create_child(>child, >parent);
 }
diff --git a/src/util/slab.h b/src/util/slab.h
index e83f8ec1a0e..5a25adaf7f4 100644
--- a/src/util/slab.h
+++ b/src/util/slab.h
@@ -84,11 +84,11 @@ struct slab_mempool {
struct slab_child_pool child;
 };
 
-void slab_create(struct slab_mempool *pool,
+void slab_create(struct slab_mempool *mempool,
  unsigned item_size,
  unsigned num_items);
-void slab_destroy(struct slab_mempool *pool);
-void *slab_alloc_st(struct slab_mempool *pool);
-void slab_free_st(struct slab_mempool *pool, void *ptr);
+void slab_destroy(struct slab_mempool *mempool);
+void *slab_alloc_st(struct slab_mempool *mempool);
+void slab_free_st(struct slab_mempool *mempool, void *ptr);
 
 #endif
-- 
2.14.5

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


[Mesa-dev] [PATCH 3/6] util/hash_table: Add _mesa_hash_table_init function

2018-11-27 Thread Ian Romanick
From: Ian Romanick 

Signed-off-by: Ian Romanick 
---
 src/util/hash_table.c | 39 ++-
 src/util/hash_table.h |  8 
 2 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/src/util/hash_table.c b/src/util/hash_table.c
index fc152f84a4d..4f510612a8f 100644
--- a/src/util/hash_table.c
+++ b/src/util/hash_table.c
@@ -110,6 +110,27 @@ entry_is_present(const struct hash_table *ht, struct 
hash_entry *entry)
return entry->key != NULL && entry->key != ht->deleted_key;
 }
 
+bool
+_mesa_hash_table_init(struct hash_table *ht,
+  void *mem_ctx,
+  uint32_t (*key_hash_function)(const void *key),
+  bool (*key_equals_function)(const void *a,
+  const void *b))
+{
+   ht->size_index = 0;
+   ht->size = hash_sizes[ht->size_index].size;
+   ht->rehash = hash_sizes[ht->size_index].rehash;
+   ht->max_entries = hash_sizes[ht->size_index].max_entries;
+   ht->key_hash_function = key_hash_function;
+   ht->key_equals_function = key_equals_function;
+   ht->table = rzalloc_array(mem_ctx, struct hash_entry, ht->size);
+   ht->entries = 0;
+   ht->deleted_entries = 0;
+   ht->deleted_key = _key_value;
+
+   return ht->table != NULL;
+}
+
 struct hash_table *
 _mesa_hash_table_create(void *mem_ctx,
 uint32_t (*key_hash_function)(const void *key),
@@ -118,22 +139,14 @@ _mesa_hash_table_create(void *mem_ctx,
 {
struct hash_table *ht;
 
+   /* mem_ctx is used to allocate the hash table, but the hash table is used
+* to allocate all of the suballocations.
+*/
ht = ralloc(mem_ctx, struct hash_table);
if (ht == NULL)
   return NULL;
 
-   ht->size_index = 0;
-   ht->size = hash_sizes[ht->size_index].size;
-   ht->rehash = hash_sizes[ht->size_index].rehash;
-   ht->max_entries = hash_sizes[ht->size_index].max_entries;
-   ht->key_hash_function = key_hash_function;
-   ht->key_equals_function = key_equals_function;
-   ht->table = rzalloc_array(ht, struct hash_entry, ht->size);
-   ht->entries = 0;
-   ht->deleted_entries = 0;
-   ht->deleted_key = _key_value;
-
-   if (ht->table == NULL) {
+   if (!_mesa_hash_table_init(ht, ht, key_hash_function, key_equals_function)) 
{
   ralloc_free(ht);
   return NULL;
}
@@ -287,7 +300,7 @@ _mesa_hash_table_rehash(struct hash_table *ht, unsigned 
new_size_index)
if (new_size_index >= ARRAY_SIZE(hash_sizes))
   return;
 
-   table = rzalloc_array(ht, struct hash_entry,
+   table = rzalloc_array(ralloc_parent(ht->table), struct hash_entry,
  hash_sizes[new_size_index].size);
if (table == NULL)
   return;
diff --git a/src/util/hash_table.h b/src/util/hash_table.h
index d89fc1dc1c8..40acda1fd1e 100644
--- a/src/util/hash_table.h
+++ b/src/util/hash_table.h
@@ -62,6 +62,14 @@ _mesa_hash_table_create(void *mem_ctx,
 uint32_t (*key_hash_function)(const void *key),
 bool (*key_equals_function)(const void *a,
 const void *b));
+
+bool
+_mesa_hash_table_init(struct hash_table *ht,
+  void *mem_ctx,
+  uint32_t (*key_hash_function)(const void *key),
+  bool (*key_equals_function)(const void *a,
+  const void *b));
+
 struct hash_table *
 _mesa_hash_table_clone(struct hash_table *src, void *dst_mem_ctx);
 void _mesa_hash_table_destroy(struct hash_table *ht,
-- 
2.14.5

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


[Mesa-dev] [PATCH 5/6] nir: Fix holes in nir_instr

2018-11-27 Thread Ian Romanick
From: Ian Romanick 

Found using pahole.

Changes in peak memory usage according to Valgrind massif:

mean soft fp64 using uint64:   1,343,991,403 => 1,342,759,331
gfxbench5 aztec ruins high 11:63,619,971 =>63,555,571
deus ex mankind divided 148:  62,887,728 =>62,845,304
deus ex mankind divided 2890: 72,399,750 =>71,922,686
dirt showdown 676:69,464,023 =>69,238,607
dolphin ubershaders 210:  78,359,728 =>77,822,072

Signed-off-by: Ian Romanick 
---
 src/compiler/nir/nir.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index a292ec73e1e..74c700026ad 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -486,7 +486,7 @@ typedef struct nir_register {
 #define nir_foreach_register_safe(reg, reg_list) \
foreach_list_typed_safe(nir_register, reg, node, reg_list)
 
-typedef enum {
+typedef enum PACKED {
nir_instr_type_alu,
nir_instr_type_deref,
nir_instr_type_call,
@@ -501,16 +501,16 @@ typedef enum {
 
 typedef struct nir_instr {
struct exec_node node;
-   nir_instr_type type;
struct nir_block *block;
-
-   /** generic instruction index. */
-   unsigned index;
+   nir_instr_type type;
 
/* A temporary for optimization and analysis passes to use for storing
 * flags.  For instance, DCE uses this to store the "dead/live" info.
 */
uint8_t pass_flags;
+
+   /** generic instruction index. */
+   unsigned index;
 } nir_instr;
 
 static inline nir_instr *
-- 
2.14.5

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


[Mesa-dev] [PATCH 1/6] nir/phi_builder: Internal users should use nir_phi_builder_value_set_block_def too

2018-11-27 Thread Ian Romanick
From: Ian Romanick 

Signed-off-by: Ian Romanick 
Reviewed-by: Jason Ekstrand 
---
 src/compiler/nir/nir_phi_builder.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/compiler/nir/nir_phi_builder.c 
b/src/compiler/nir/nir_phi_builder.c
index cc5ce81d120..add3efd2dfc 100644
--- a/src/compiler/nir/nir_phi_builder.c
+++ b/src/compiler/nir/nir_phi_builder.c
@@ -147,7 +147,7 @@ nir_phi_builder_add_value(struct nir_phi_builder *pb, 
unsigned num_components,
  * value to the magic value NEEDS_PHI.  Later, we create phi nodes
  * on demand in nir_phi_builder_value_get_block_def().
  */
-val->defs[next->index] = NEEDS_PHI;
+nir_phi_builder_value_set_block_def(val, next, NEEDS_PHI);
 
 if (pb->work[next->index] < pb->iter_count) {
pb->work[next->index] = pb->iter_count;
@@ -232,7 +232,7 @@ nir_phi_builder_value_get_block_def(struct 
nir_phi_builder_value *val,
 *  2) To avoid unneeded recreation of phi nodes and undefs.
 */
for (dom = block; dom && val->defs[dom->index] == NULL; dom = dom->imm_dom)
-  val->defs[dom->index] = def;
+  nir_phi_builder_value_set_block_def(val, dom, def);
 
return def;
 }
-- 
2.14.5

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


[Mesa-dev] [PATCH v2 0/6]

2018-11-27 Thread Ian Romanick
After incorporating several of Jason's suggestions, I have decided to
just resend this series.  The end result is the same for the worst case.
The other cases are 1% - 4% less peak memory usage.  The new series is
also about 80 lines less code (git diff phi-builder-v1..phi-builder-v2
is 72 insertions(+), 151 deletions(-)).

The new series is:

https://cgit.freedesktop.org/~idr/mesa/log/?h=phi-builder-v2

The old series (with trivial changes) is:

https://cgit.freedesktop.org/~idr/mesa/log/?h=phi-builder-v1

I just re-ordered the old series so that the first two patches of both
are the same.

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


[Mesa-dev] [PATCH 6/6] nir: Release per-block metadata in nir_sweep

2018-11-27 Thread Ian Romanick
From: Ian Romanick 

nir_sweep already marks all metadata invalid, so it is safe to release
the memory here too.

mean soft fp64 using uint64:   1,342,759,331 => 1,010,670,475
gfxbench5 aztec ruins high 11:63,555,571 =>61,889,811
deus ex mankind divided 148:  62,845,304 =>62,829,640
deus ex mankind divided 2890: 71,922,686 =>71,922,686
dirt showdown 676:69,238,607 =>69,238,607
dolphin ubershaders 210:  77,822,072 =>77,822,072

Signed-off-by: Ian Romanick 
---
 src/compiler/nir/nir_sweep.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/compiler/nir/nir_sweep.c b/src/compiler/nir/nir_sweep.c
index aab641388db..b6b56aa078c 100644
--- a/src/compiler/nir/nir_sweep.c
+++ b/src/compiler/nir/nir_sweep.c
@@ -63,6 +63,15 @@ sweep_block(nir_shader *nir, nir_block *block)
 {
ralloc_steal(nir, block);
 
+   /* sweep_impl will mark all metadata invalid.  We can safely release all of
+* this here.
+*/
+   ralloc_free(block->live_in);
+   block->live_in = NULL;
+
+   ralloc_free(block->live_out);
+   block->live_out = NULL;
+
nir_foreach_instr(instr, block) {
   ralloc_steal(nir, instr);
 
-- 
2.14.5

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


[Mesa-dev] [PATCH] intel/compiler: Use nir's info when checking uses_streams.

2018-11-27 Thread Kenneth Graunke
Vulkan and Gallium don't use Mesa's gl_program data structure, so they
can't poke at 'prog'.  But we can simply use the copy of the shader info
stored with the NIR shader, which is guaranteed to exist.

Reviewed-by: Jason Ekstrand 
---
 src/intel/compiler/brw_vec4_gs_visitor.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/intel/compiler/brw_vec4_gs_visitor.cpp 
b/src/intel/compiler/brw_vec4_gs_visitor.cpp
index 63ff27e5e08..a6e38b0f379 100644
--- a/src/intel/compiler/brw_vec4_gs_visitor.cpp
+++ b/src/intel/compiler/brw_vec4_gs_visitor.cpp
@@ -667,7 +667,7 @@ brw_compile_gs(const struct brw_compiler *compiler, void 
*log_data,
  prog_data->control_data_format = 
GEN7_GS_CONTROL_DATA_FORMAT_GSCTL_SID;
 
  /* We only have to emit control bits if we are using streams */
- if (prog && prog->info.gs.uses_streams)
+ if (shader->info.gs.uses_streams)
 c.control_data_bits_per_vertex = 2;
  else
 c.control_data_bits_per_vertex = 0;
-- 
2.19.1

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


Re: [Mesa-dev] Please bring back __GL_FSAA_MODE

2018-11-27 Thread Marek Olšák
Yes, it could be added back.

Marek

On Tue, Nov 27, 2018 at 3:15 AM Tom Butler  wrote:

> Hello,
>
> I realise this was removed for a reason (
> https://lists.freedesktop.org/archives/mesa-dev/2014-September/067864.html
> ) but there are cases where it is useful. In older games that do not
> support native FSAA being able to force it in the driver is the only way
> to enable it.
>
>
> A quick google search for AMD linux force msaa shows that I'm not the
> only one who would like to see this feature return:
>
>
> https://www.phoronix.com/forums/forum/linux-graphics-x-org-drivers/open-source-amd-linux/1024166-radeon-eqaa-anti-aliasing-support-merged-to-mesa-18-2?p=1024185#post1024185
>
> https://bbs.archlinux.org/viewtopic.php?id=225425
>
>
> https://www.reddit.com/r/linux_gaming/comments/671yzm/forcing_antialiasing_with_mesa_radeon_driver/
>
> https://github.com/dscharrer/void/blob/master/hacks/forcemsaa.c
>
> I understand it does cause issues in some cases but there are times when
> it is useful. Could it be reintroduced with a more relevant name that
> implies it shouldn't be used? E.g. GALLIUM_LEGACY_MSAA or
> GALLIUM_FORCE_MSAA_EXPERIMENTAL. That way it would lower users
> expectations while still making the option available to try.
>
>
> Kind Regards,
>
> Tom Butler
>
>
>
> ___
> 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] gallium: Remove unused variable in u_tests.

2018-11-27 Thread Marek Olšák
Reviewed-by: Marek Olšák 

Marek

On Tue, Nov 27, 2018 at 2:26 PM Eric Anholt  wrote:

> Fixes: 0d17b685b1ff ("gallium/u_tests: add a compute shader test that
> clears an image")
> ---
>  src/gallium/auxiliary/util/u_tests.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/src/gallium/auxiliary/util/u_tests.c
> b/src/gallium/auxiliary/util/u_tests.c
> index 7c4e98402bef..365d4fa8f171 100644
> --- a/src/gallium/auxiliary/util/u_tests.c
> +++ b/src/gallium/auxiliary/util/u_tests.c
> @@ -792,7 +792,6 @@ test_compute_clear_image(struct pipe_context *ctx)
>  {
> struct cso_context *cso;
> struct pipe_resource *cb;
> -   struct pipe_sampler_view *view = NULL;
> const char *text;
>
> cso = cso_create_context(ctx, 0);
> --
> 2.20.0.rc1
>
> ___
> 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 2/2] virgl: Don't try handling server fences when they are not supported

2018-11-27 Thread Robert Foss

This patch has been:

Reviewed-by: Robert Foss 

On 2018-11-27 20:50, Gert Wollny wrote:

From: Gert Wollny 

vtest doesn't implement the according API and would segfault:

Program received signal SIGSEGV, Segmentation fault.
   #0  0x in ?? ()
   #1  in virgl_fence_server_sync  at
src/gallium/drivers/virgl/virgl_context.c:1049
   #2  in st_server_wait_sync  at
src/mesa/state_tracker/st_cb_syncobj.c:155

so just don't do the call when the function pointers are not set.

Fixes dEQP:
   dEQP-GLES3.functional.fence_sync.wait_sync_smalldraw
   dEQP-GLES3.functional.fence_sync.wait_sync_largedraw

Fixes: d1a1c21e7621b5177febf191fcd3d3b8ef69dc96
   virgl: native fence fd support

Signed-off-by: Gert Wollny 
---
  src/gallium/drivers/virgl/virgl_context.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_context.c 
b/src/gallium/drivers/virgl/virgl_context.c
index 892fef76c7..f0ee64c145 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -1037,7 +1037,8 @@ static void virgl_create_fence_fd(struct pipe_context 
*ctx,
 assert(type == PIPE_FD_TYPE_NATIVE_SYNC);
 struct virgl_screen *rs = virgl_screen(ctx->screen);
  
-   *fence = rs->vws->cs_create_fence(rs->vws, fd);

+   if (rs->vws->cs_create_fence)
+  *fence = rs->vws->cs_create_fence(rs->vws, fd);
  }
  
  static void virgl_fence_server_sync(struct pipe_context *ctx,

@@ -1046,7 +1047,8 @@ static void virgl_fence_server_sync(struct pipe_context 
*ctx,
 struct virgl_context *vctx = virgl_context(ctx);
 struct virgl_screen *rs = virgl_screen(ctx->screen);
  
-   rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence);

+   if (rs->vws->fence_server_sync)
+  rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence);
  }
  
  static void virgl_set_shader_images(struct pipe_context *ctx,



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


Re: [Mesa-dev] [PATCH 1/2] virgl,vtest: Initialize return value

2018-11-27 Thread Robert Foss

This patch has been:

Reviewed-by: Robert Foss 

On 2018-11-27 20:50, Gert Wollny wrote:

From: Gert Wollny 

Avoids:
Conditional jump or move depends on uninitialised value(s)
   at 0x9E2B39F: virgl_vtest_winsys_resource_cache_create 
(virgl_vtest_winsys.c:379)
   by 0x9E2725F: virgl_buffer_create (virgl_buffer.c:169)
   by 0x9E246D5: virgl_resource_create (virgl_resource.c:60)
   by 0xA0C1B9F: bufferobj_data (st_cb_bufferobjects.c:344)
   by 0xA0C1B9F: st_bufferobj_data (st_cb_bufferobjects.c:390)
   by 0x9F4ACE3: vbo_use_buffer_objects (vbo_exec_api.c:1136)
   by 0xA0C68C3: st_create_context_priv (st_context.c:416)
   by 0xA0C707A: st_create_context (st_context.c:598)
   by 0x9F81C6B: st_api_create_context (st_manager.c:918)
   by 0x9BBE591: dri_create_context (dri_context.c:161)
   by 0x9BB6931: driCreateContextAttribs (dri_util.c:473)
   by 0x4E97A44: drisw_create_context_attribs (drisw_glx.c:630)
   by 0x4E7C591: glXCreateContextAttribsARB (create_context.c:78)
Uninitialised value was created by a stack allocation
   at 0x9E2B249: virgl_vtest_winsys_resource_cache_create 
(virgl_vtest_winsys.c:342)


Signed-off-by: Gert Wollny 
---
  src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c 
b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c
index a23f853924..65963ad50e 100644
--- a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c
+++ b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c
@@ -344,7 +344,7 @@ virgl_vtest_winsys_resource_cache_create(struct 
virgl_winsys *vws,
 struct virgl_hw_res *res, *curr_res;
 struct list_head *curr, *next;
 int64_t now;
-   int ret;
+   int ret = -1;
  
 /* only store binds for vertex/index/const buffers */

 if (bind != VIRGL_BIND_CONSTANT_BUFFER && bind != VIRGL_BIND_INDEX_BUFFER 
&&


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


Re: [Mesa-dev] [PATCH mesa] wsi/display: fix mem leak when freeing swapchains

2018-11-27 Thread Keith Packard
Eric Engestrom  writes:

> Fixes: da997ebec92942193955 "vulkan: Add KHR_display extension using DRM 
> [v10]"
> Cc: Keith Packard 
> Signed-off-by: Eric Engestrom 

Reviewed-by: Keith Packard 

I checked to ensure that this is sufficient to clean all allocated
objects:

 * Creating a display swapchain allocates the chain, image_count images
   and call wsi_swapchain_init. Adding the wsi_swapchain_finish cleans
   the last of these allocations.

 * The queue of to-be-presented images exists only as state values
   within the images themselves, so there's no additional allocation
   there.

-- 
-keith


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 2/2] virgl: Don't try handling server fences when they are not supported

2018-11-27 Thread Dave Airlie
On Wed, 28 Nov 2018 at 05:51, Gert Wollny  wrote:
>
> From: Gert Wollny 
>
> vtest doesn't implement the according API and would segfault:
>
> Program received signal SIGSEGV, Segmentation fault.
>   #0  0x in ?? ()
>   #1  in virgl_fence_server_sync  at
>src/gallium/drivers/virgl/virgl_context.c:1049
>   #2  in st_server_wait_sync  at
>src/mesa/state_tracker/st_cb_syncobj.c:155
>
> so just don't do the call when the function pointers are not set.
>
> Fixes dEQP:
>   dEQP-GLES3.functional.fence_sync.wait_sync_smalldraw
>   dEQP-GLES3.functional.fence_sync.wait_sync_largedraw
>
> Fixes: d1a1c21e7621b5177febf191fcd3d3b8ef69dc96
>   virgl: native fence fd support
>
> Signed-off-by: Gert Wollny 

Reviewed-by: Dave Airlie 

For both.

Dave.

> ---
>  src/gallium/drivers/virgl/virgl_context.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/virgl/virgl_context.c 
> b/src/gallium/drivers/virgl/virgl_context.c
> index 892fef76c7..f0ee64c145 100644
> --- a/src/gallium/drivers/virgl/virgl_context.c
> +++ b/src/gallium/drivers/virgl/virgl_context.c
> @@ -1037,7 +1037,8 @@ static void virgl_create_fence_fd(struct pipe_context 
> *ctx,
> assert(type == PIPE_FD_TYPE_NATIVE_SYNC);
> struct virgl_screen *rs = virgl_screen(ctx->screen);
>
> -   *fence = rs->vws->cs_create_fence(rs->vws, fd);
> +   if (rs->vws->cs_create_fence)
> +  *fence = rs->vws->cs_create_fence(rs->vws, fd);
>  }
>
>  static void virgl_fence_server_sync(struct pipe_context *ctx,
> @@ -1046,7 +1047,8 @@ static void virgl_fence_server_sync(struct pipe_context 
> *ctx,
> struct virgl_context *vctx = virgl_context(ctx);
> struct virgl_screen *rs = virgl_screen(ctx->screen);
>
> -   rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence);
> +   if (rs->vws->fence_server_sync)
> +  rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence);
>  }
>
>  static void virgl_set_shader_images(struct pipe_context *ctx,
> --
> 2.18.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] virgl: Don't try handling server fences when they are not supported

2018-11-27 Thread Gert Wollny
From: Gert Wollny 

vtest doesn't implement the according API and would segfault:

Program received signal SIGSEGV, Segmentation fault.
  #0  0x in ?? ()
  #1  in virgl_fence_server_sync  at
   src/gallium/drivers/virgl/virgl_context.c:1049
  #2  in st_server_wait_sync  at
   src/mesa/state_tracker/st_cb_syncobj.c:155

so just don't do the call when the function pointers are not set.

Fixes dEQP:
  dEQP-GLES3.functional.fence_sync.wait_sync_smalldraw
  dEQP-GLES3.functional.fence_sync.wait_sync_largedraw

Fixes: d1a1c21e7621b5177febf191fcd3d3b8ef69dc96
  virgl: native fence fd support

Signed-off-by: Gert Wollny 
---
 src/gallium/drivers/virgl/virgl_context.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_context.c 
b/src/gallium/drivers/virgl/virgl_context.c
index 892fef76c7..f0ee64c145 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -1037,7 +1037,8 @@ static void virgl_create_fence_fd(struct pipe_context 
*ctx,
assert(type == PIPE_FD_TYPE_NATIVE_SYNC);
struct virgl_screen *rs = virgl_screen(ctx->screen);
 
-   *fence = rs->vws->cs_create_fence(rs->vws, fd);
+   if (rs->vws->cs_create_fence)
+  *fence = rs->vws->cs_create_fence(rs->vws, fd);
 }
 
 static void virgl_fence_server_sync(struct pipe_context *ctx,
@@ -1046,7 +1047,8 @@ static void virgl_fence_server_sync(struct pipe_context 
*ctx,
struct virgl_context *vctx = virgl_context(ctx);
struct virgl_screen *rs = virgl_screen(ctx->screen);
 
-   rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence);
+   if (rs->vws->fence_server_sync)
+  rs->vws->fence_server_sync(rs->vws, vctx->cbuf, fence);
 }
 
 static void virgl_set_shader_images(struct pipe_context *ctx,
-- 
2.18.1

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


[Mesa-dev] [PATCH 1/2] virgl,vtest: Initialize return value

2018-11-27 Thread Gert Wollny
From: Gert Wollny 

Avoids:
Conditional jump or move depends on uninitialised value(s)
  at 0x9E2B39F: virgl_vtest_winsys_resource_cache_create 
(virgl_vtest_winsys.c:379)
  by 0x9E2725F: virgl_buffer_create (virgl_buffer.c:169)
  by 0x9E246D5: virgl_resource_create (virgl_resource.c:60)
  by 0xA0C1B9F: bufferobj_data (st_cb_bufferobjects.c:344)
  by 0xA0C1B9F: st_bufferobj_data (st_cb_bufferobjects.c:390)
  by 0x9F4ACE3: vbo_use_buffer_objects (vbo_exec_api.c:1136)
  by 0xA0C68C3: st_create_context_priv (st_context.c:416)
  by 0xA0C707A: st_create_context (st_context.c:598)
  by 0x9F81C6B: st_api_create_context (st_manager.c:918)
  by 0x9BBE591: dri_create_context (dri_context.c:161)
  by 0x9BB6931: driCreateContextAttribs (dri_util.c:473)
  by 0x4E97A44: drisw_create_context_attribs (drisw_glx.c:630)
  by 0x4E7C591: glXCreateContextAttribsARB (create_context.c:78)
Uninitialised value was created by a stack allocation
  at 0x9E2B249: virgl_vtest_winsys_resource_cache_create 
(virgl_vtest_winsys.c:342)


Signed-off-by: Gert Wollny 
---
 src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c 
b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c
index a23f853924..65963ad50e 100644
--- a/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c
+++ b/src/gallium/winsys/virgl/vtest/virgl_vtest_winsys.c
@@ -344,7 +344,7 @@ virgl_vtest_winsys_resource_cache_create(struct 
virgl_winsys *vws,
struct virgl_hw_res *res, *curr_res;
struct list_head *curr, *next;
int64_t now;
-   int ret;
+   int ret = -1;
 
/* only store binds for vertex/index/const buffers */
if (bind != VIRGL_BIND_CONSTANT_BUFFER && bind != VIRGL_BIND_INDEX_BUFFER &&
-- 
2.18.1

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


Re: [Mesa-dev] [RFC 4/5] intel/compiler: Implement unordered comparisons

2018-11-27 Thread Jason Ekstrand
On Tue, Nov 27, 2018 at 12:55 PM Ian Romanick  wrote:

> On 11/22/2018 10:46 AM, Jason Ekstrand wrote:
> > The vec4 path has only been compile-tested as there's no easy way to
> > generate a vec4 shader with an unordered equality.
> > ---
> >  src/intel/compiler/brw_compiler.c   |  3 +++
> >  src/intel/compiler/brw_fs_nir.cpp   | 20 +---
> >  src/intel/compiler/brw_vec4_nir.cpp | 21 +
> >  3 files changed, 37 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/intel/compiler/brw_compiler.c
> b/src/intel/compiler/brw_compiler.c
> > index fe632c5badc..f9e8fa09a34 100644
> > --- a/src/intel/compiler/brw_compiler.c
> > +++ b/src/intel/compiler/brw_compiler.c
> > @@ -42,6 +42,9 @@
> > .lower_fdiv = true,
>   \
> > .lower_flrp64 = true,
>   \
> > .lower_ldexp = true,
>\
> > +   .lower_fltu = true,
>   \
> > +   .lower_fgeu = true,
>   \
>
> Can't we use cmpn.l and cmpn.ge for these?  On some platforms it has to
> be split into two SIMD8 instructions, but that seems better than the
> lowering... or maybe just use those on BDW+?
>

I didn't know about CMPN!  I guess that would do the trick though I suppose
we have a pile of back-end work to be able to optimize it.  I think the
back-end usually gets rid of the inot by flipping the cmod but maybe not.


> > +   .lower_fne_to_fequ = true,
>\
> > .lower_cs_local_id_from_index = true,
>   \
> > .lower_device_index_to_zero = true,
>   \
> > .native_integers = true,
>\
> > diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> > index a62d521bb5d..eba3611e447 100644
> > --- a/src/intel/compiler/brw_fs_nir.cpp
> > +++ b/src/intel/compiler/brw_fs_nir.cpp
> > @@ -1049,6 +1049,8 @@ fs_visitor::nir_emit_alu(const fs_builder ,
> nir_alu_instr *instr)
> > case nir_op_flt:
> > case nir_op_fge:
> > case nir_op_feq:
> > +   case nir_op_fne:
> > +   case nir_op_fequ:
> > case nir_op_fneu: {
> >fs_reg dest = result;
> >
> > @@ -1056,26 +1058,30 @@ fs_visitor::nir_emit_alu(const fs_builder ,
> nir_alu_instr *instr)
> >if (bit_size != 32)
> >   dest = bld.vgrf(op[0].type, 1);
> >
> > -  brw_conditional_mod cond;
> >switch (instr->op) {
> >case nir_op_flt:
> > - cond = BRW_CONDITIONAL_L;
> > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_L);
> >   break;
> >case nir_op_fge:
> > - cond = BRW_CONDITIONAL_GE;
> > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_GE);
> >   break;
> >case nir_op_feq:
> > - cond = BRW_CONDITIONAL_Z;
> > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_Z);
> > + break;
> > +  case nir_op_fequ:
> > + bld.CMP(dest, op[0], op[0], BRW_CONDITIONAL_NZ);
> > + set_predicate_inv(BRW_PREDICATE_NORMAL, true, /* inverse */
> > +   bld.CMP(dest, op[1], op[1],
> BRW_CONDITIONAL_NZ));
> > + set_predicate_inv(BRW_PREDICATE_NORMAL, true, /* inverse */
> > +   bld.CMP(dest, op[0], op[1],
> BRW_CONDITIONAL_Z));
>
> OpFUnordEqual is (isnan(a) || isnan(b) || a == b), and that's literally
> what this code does.  It seems like this could be implemented (perhaps
> as a lowering?) as (OpFUnordGreaterThanEqual(a,b) &&
> OpFUnordGreaterThanEqual(b, a)) via cmpn.ge.  Using this same technique,
> that would be 2 instructions (on BDW+) instead of 3.  HSW has to do cmpn
> as two SIMD8 instructions, so this may be better there.  Dunno.
>

That's a good idea.  Unfortunately, CMPN dosn't modify Z or NZ.  We could
also lower fequ(x, y) to inot(fne(x, y)) and implement fne(x, y) as (x < y)
|| (x > y) which wouldn't require CMPN.

--Jason


> >   break;
> >case nir_op_fneu:
> > - cond = BRW_CONDITIONAL_NZ;
> > + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_NZ);
> >   break;
> >default:
> >   unreachable("bad opcode");
> >}
> >
> > -  bld.CMP(dest, op[0], op[1], cond);
> > -
> >if (bit_size > 32) {
> >   bld.MOV(result, subscript(dest, BRW_REGISTER_TYPE_UD, 0));
> >} else if(bit_size < 32) {
> > diff --git a/src/intel/compiler/brw_vec4_nir.cpp
> b/src/intel/compiler/brw_vec4_nir.cpp
> > index f7f46f5034c..32559e1aade 100644
> > --- a/src/intel/compiler/brw_vec4_nir.cpp
> > +++ b/src/intel/compiler/brw_vec4_nir.cpp
> > @@ -1366,6 +1366,27 @@ vec4_visitor::nir_emit_alu(nir_alu_instr *instr)
> >break;
> > }
> >
> > +   case nir_op_fequ: {
> > +  dst_reg cmp_res = dst;
> > +  if (nir_src_bit_size(instr->src[0].src) == 64)
> > + cmp_res = dst_reg(this, glsl_type::dvec4_type);
> > +
> > +  vec4_instruction *inst;
> > +  inst = emit(CMP(cmp_res, op[0], op[0], BRW_CONDITIONAL_NZ));
> > +  inst = emit(CMP(cmp_res, op[1], op[1], BRW_CONDITIONAL_NZ));
> > +  inst->predicate = BRW_PREDICATE_NORMAL;
> > +  

[Mesa-dev] [PATCH] gallium: Remove unused variable in u_tests.

2018-11-27 Thread Eric Anholt
Fixes: 0d17b685b1ff ("gallium/u_tests: add a compute shader test that clears an 
image")
---
 src/gallium/auxiliary/util/u_tests.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/gallium/auxiliary/util/u_tests.c 
b/src/gallium/auxiliary/util/u_tests.c
index 7c4e98402bef..365d4fa8f171 100644
--- a/src/gallium/auxiliary/util/u_tests.c
+++ b/src/gallium/auxiliary/util/u_tests.c
@@ -792,7 +792,6 @@ test_compute_clear_image(struct pipe_context *ctx)
 {
struct cso_context *cso;
struct pipe_resource *cb;
-   struct pipe_sampler_view *view = NULL;
const char *text;
 
cso = cso_create_context(ctx, 0);
-- 
2.20.0.rc1

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


Re: [Mesa-dev] Lets talk about autotools

2018-11-27 Thread Timo Aaltonen
On 27.11.2018 19.05, Matt Turner wrote:
> On Tue, Nov 27, 2018 at 1:13 AM Timo Aaltonen  wrote:
>>
>> On 17.11.2018 6.04, Dylan Baker wrote:
>>> Quoting Dylan Baker (2018-09-17 09:44:07)
 I feel like for !windows meson is in good enough shape at this point that 
 we
 can start having the discussion about deleting the autotools build. So, is 
 there
 anything left that autotools can do that meson cannot (that we actually 
 want to
 implement)? And, what is a reasonable time-table to remove the autotools 
 build?
 I think we could reasonably remove it as soon as 18.3 if others felt 
 confident
 that it would work for them.

 Dylan
>>>
>>> Okay, time for an update on things and a chance to talk about what else we 
>>> need.
>>>
>>> Support for llvm-config (and any binary, actually) overriding has landed in
>>> meson, and will be present in the 0.49.0 release, which is due out December 
>>> 9th.
>>
>> Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and will
>> get Mesa backports up until and including 20.0.x, so I wonder how
>> complex these required new features in meson are to be backported, or
>> perhaps easily worked around? Backporting a whole new version of meson
>> might not happen..
> 
> I understand the LTS concept, but what's the value in never upgrading
> something like a build tool like Meson? Yeah, new versions give a
> possibility of regressions, but with something evolving as quickly as
> Meson the version available in April 2018 becomes less useful for its
> intended purpose with each passing month...

Fair enough, I'll just package a newer, renamed meson for mesa if necessary.


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


Re: [Mesa-dev] [RFC 4/5] intel/compiler: Implement unordered comparisons

2018-11-27 Thread Ian Romanick
On 11/22/2018 10:46 AM, Jason Ekstrand wrote:
> The vec4 path has only been compile-tested as there's no easy way to
> generate a vec4 shader with an unordered equality.
> ---
>  src/intel/compiler/brw_compiler.c   |  3 +++
>  src/intel/compiler/brw_fs_nir.cpp   | 20 +---
>  src/intel/compiler/brw_vec4_nir.cpp | 21 +
>  3 files changed, 37 insertions(+), 7 deletions(-)
> 
> diff --git a/src/intel/compiler/brw_compiler.c 
> b/src/intel/compiler/brw_compiler.c
> index fe632c5badc..f9e8fa09a34 100644
> --- a/src/intel/compiler/brw_compiler.c
> +++ b/src/intel/compiler/brw_compiler.c
> @@ -42,6 +42,9 @@
> .lower_fdiv = true,   
>  \
> .lower_flrp64 = true, 
>  \
> .lower_ldexp = true,  
>  \
> +   .lower_fltu = true,   
>  \
> +   .lower_fgeu = true,   
>  \

Can't we use cmpn.l and cmpn.ge for these?  On some platforms it has to
be split into two SIMD8 instructions, but that seems better than the
lowering... or maybe just use those on BDW+?

> +   .lower_fne_to_fequ = true,
>  \
> .lower_cs_local_id_from_index = true, 
>  \
> .lower_device_index_to_zero = true,   
>  \
> .native_integers = true,  
>  \
> diff --git a/src/intel/compiler/brw_fs_nir.cpp 
> b/src/intel/compiler/brw_fs_nir.cpp
> index a62d521bb5d..eba3611e447 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -1049,6 +1049,8 @@ fs_visitor::nir_emit_alu(const fs_builder , 
> nir_alu_instr *instr)
> case nir_op_flt:
> case nir_op_fge:
> case nir_op_feq:
> +   case nir_op_fne:
> +   case nir_op_fequ:
> case nir_op_fneu: {
>fs_reg dest = result;
>  
> @@ -1056,26 +1058,30 @@ fs_visitor::nir_emit_alu(const fs_builder , 
> nir_alu_instr *instr)
>if (bit_size != 32)
>   dest = bld.vgrf(op[0].type, 1);
>  
> -  brw_conditional_mod cond;
>switch (instr->op) {
>case nir_op_flt:
> - cond = BRW_CONDITIONAL_L;
> + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_L);
>   break;
>case nir_op_fge:
> - cond = BRW_CONDITIONAL_GE;
> + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_GE);
>   break;
>case nir_op_feq:
> - cond = BRW_CONDITIONAL_Z;
> + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_Z);
> + break;
> +  case nir_op_fequ:
> + bld.CMP(dest, op[0], op[0], BRW_CONDITIONAL_NZ);
> + set_predicate_inv(BRW_PREDICATE_NORMAL, true, /* inverse */
> +   bld.CMP(dest, op[1], op[1], BRW_CONDITIONAL_NZ));
> + set_predicate_inv(BRW_PREDICATE_NORMAL, true, /* inverse */
> +   bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_Z));

OpFUnordEqual is (isnan(a) || isnan(b) || a == b), and that's literally
what this code does.  It seems like this could be implemented (perhaps
as a lowering?) as (OpFUnordGreaterThanEqual(a,b) &&
OpFUnordGreaterThanEqual(b, a)) via cmpn.ge.  Using this same technique,
that would be 2 instructions (on BDW+) instead of 3.  HSW has to do cmpn
as two SIMD8 instructions, so this may be better there.  Dunno.

>   break;
>case nir_op_fneu:
> - cond = BRW_CONDITIONAL_NZ;
> + bld.CMP(dest, op[0], op[1], BRW_CONDITIONAL_NZ);
>   break;
>default:
>   unreachable("bad opcode");
>}
>  
> -  bld.CMP(dest, op[0], op[1], cond);
> -
>if (bit_size > 32) {
>   bld.MOV(result, subscript(dest, BRW_REGISTER_TYPE_UD, 0));
>} else if(bit_size < 32) {
> diff --git a/src/intel/compiler/brw_vec4_nir.cpp 
> b/src/intel/compiler/brw_vec4_nir.cpp
> index f7f46f5034c..32559e1aade 100644
> --- a/src/intel/compiler/brw_vec4_nir.cpp
> +++ b/src/intel/compiler/brw_vec4_nir.cpp
> @@ -1366,6 +1366,27 @@ vec4_visitor::nir_emit_alu(nir_alu_instr *instr)
>break;
> }
>  
> +   case nir_op_fequ: {
> +  dst_reg cmp_res = dst;
> +  if (nir_src_bit_size(instr->src[0].src) == 64)
> + cmp_res = dst_reg(this, glsl_type::dvec4_type);
> +
> +  vec4_instruction *inst;
> +  inst = emit(CMP(cmp_res, op[0], op[0], BRW_CONDITIONAL_NZ));
> +  inst = emit(CMP(cmp_res, op[1], op[1], BRW_CONDITIONAL_NZ));
> +  inst->predicate = BRW_PREDICATE_NORMAL;
> +  inst->predicate_inverse = true;
> +  inst = emit(CMP(cmp_res, op[0], op[1], BRW_CONDITIONAL_Z));
> +  inst->predicate = BRW_PREDICATE_NORMAL;
> +  inst->predicate_inverse = true;
> +
> +  if (nir_src_bit_size(instr->src[0].src) == 64) {
> + dst_reg cmp_res32 = 

Re: [Mesa-dev] [RFC 1/5] nir: Rename nir_op_fne to nir_op_fneu

2018-11-27 Thread Ian Romanick
On 11/26/2018 01:05 PM, Eric Anholt wrote:
> Jason Ekstrand  writes:
> 
>> This way, it's explicit in the opcode name that it's an unordered
>> comparison.
>> ---
>>  src/amd/common/ac_nir_to_llvm.c   |  2 +-
>>  src/compiler/glsl/glsl_to_nir.cpp |  4 +-
>>  src/compiler/nir/nir.h|  2 +-
>>  src/compiler/nir/nir_builder.h|  2 +-
>>  src/compiler/nir/nir_loop_analyze.c   |  4 +-
>>  src/compiler/nir/nir_lower_alu_to_scalar.c|  2 +-
>>  src/compiler/nir/nir_lower_double_ops.c   |  6 +--
>>  src/compiler/nir/nir_opcodes.py   |  2 +-
>>  src/compiler/nir/nir_opt_algebraic.py | 46 +--
>>  src/compiler/spirv/vtn_alu.c  | 10 ++--
>>  src/compiler/spirv/vtn_glsl450.c  |  4 +-
>>  src/gallium/auxiliary/nir/tgsi_to_nir.c   |  4 +-
>>  .../drivers/freedreno/ir3/ir3_compiler_nir.c  |  2 +-
>>  src/gallium/drivers/vc4/vc4_program.c |  4 +-
>>  src/intel/compiler/brw_fs_nir.cpp |  4 +-
>>  src/intel/compiler/brw_vec4_nir.cpp   |  4 +-
>>  16 files changed, 51 insertions(+), 51 deletions(-)
> 
> Looks like you missed src/broadcom/compiler/

I was going to suggest that this patch should just be a sed job, but...
It's a bummer that 'grep -rl nir_op_fne[^g] src/' doesn't produce the
full list.  It does get src/broadcom/compiler/nir_to_vir.c.

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



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


Re: [Mesa-dev] [PATCH] intel/compiler: fix register allocation in opt_peephole_sel

2018-11-27 Thread Ian Romanick
Seems obviously correct.

Reviewed-by: Ian Romanick 

On 11/27/2018 01:24 AM, Iago Toral Quiroga wrote:
> This wasn't handling 64-bit cases properly. Found by inspection.
> ---
>  src/intel/compiler/brw_fs_sel_peephole.cpp | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/intel/compiler/brw_fs_sel_peephole.cpp 
> b/src/intel/compiler/brw_fs_sel_peephole.cpp
> index 4d11d10cc6..98d640a3bf 100644
> --- a/src/intel/compiler/brw_fs_sel_peephole.cpp
> +++ b/src/intel/compiler/brw_fs_sel_peephole.cpp
> @@ -198,8 +198,7 @@ fs_visitor::opt_peephole_sel()
>   */
>  fs_reg src0(then_mov[i]->src[0]);
>  if (src0.file == IMM) {
> -   src0 = vgrf(glsl_type::float_type);
> -   src0.type = then_mov[i]->src[0].type;
> +   src0 = ibld.vgrf(then_mov[i]->src[0].type);
> ibld.MOV(src0, then_mov[i]->src[0]);
>  }
>  
> 

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


Re: [Mesa-dev] [PATCH 2/2] nir: merge some basic consecutive ifs

2018-11-27 Thread Ian Romanick
On 11/26/2018 09:31 PM, Timothy Arceri wrote:
> After trying multiple times to merge if-statements with phis
> between them I've come to the conclusion that it cannot be done
> without regressions. The problem is for some shaders we end up
> with a whole bunch of phis for the merged ifs resulting in
> increased register pressure.
> 
> So this patch just merges ifs that have no phis between them.
> This seems to be consistent with what LLVM does so for radeonsi
> we only see a change (although its a large change) in a single
> shader.
> 
> Shader-db results i965 (SKL):
> 
> total instructions in shared programs: 13098176 -> 13098152 (<.01%)
> instructions in affected programs: 1326 -> 1302 (-1.81%)
> helped: 4
> HURT: 0
> 
> total cycles in shared programs: 332032989 -> 332037583 (<.01%)
> cycles in affected programs: 60665 -> 65259 (7.57%)
> helped: 0
> HURT: 4
> 
> The cycles estimates reported by shader-db for i965 seem inaccurate
> as the only difference in the final code is the removal of the
> redundent condition evaluations and jumps.

The scheduling doesn't even change?  Whenever I've encountered things
like this, the scheduler has been to blame.  It seems surprising that
only 4 shaders are affected.  Are these all different instances of the
same shader? :)

> Also the biggest code reduction (~7%) for radeonsi was in a tomb
> raider tressfx shader but for some reason this does not get merged
> for i965.
> 
> Shader-db results radeonsi (VEGA):
> 
> Totals from affected shaders:
> SGPRS: 232 -> 232 (0.00 %)
> VGPRS: 164 -> 164 (0.00 %)
> Spilled SGPRs: 59 -> 59 (0.00 %)
> Spilled VGPRs: 0 -> 0 (0.00 %)
> Private memory VGPRs: 0 -> 0 (0.00 %)
> Scratch size: 0 -> 0 (0.00 %) dwords per thread
> Code Size: 14584 -> 13520 (-7.30 %) bytes
> LDS: 0 -> 0 (0.00 %) blocks
> Max Waves: 13 -> 13 (0.00 %)
> Wait states: 0 -> 0 (0.00 %)
> ---
>  src/compiler/nir/nir_opt_if.c | 93 +++
>  1 file changed, 93 insertions(+)
> 
> diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
> index 62566eb403..dd488b1787 100644
> --- a/src/compiler/nir/nir_opt_if.c
> +++ b/src/compiler/nir/nir_opt_if.c
> @@ -585,6 +585,98 @@ opt_if_evaluate_condition_use(nir_builder *b, nir_if 
> *nif)
> return progress;
>  }
>  
> +static void
> +simple_merge_if(nir_if *dest_if, nir_if *src_if, bool dest_if_then,
> +bool src_if_then)
> +{
> +   /* Now merge the if branch */
> +   nir_block *dest_blk = dest_if_then ? nir_if_last_then_block(dest_if)
> +  : nir_if_last_else_block(dest_if);
> +
> +   struct exec_list *list = src_if_then ? _if->then_list
> +: _if->else_list;
> +
> +   nir_cf_list if_cf_list;
> +   nir_cf_extract(_cf_list, nir_before_cf_list(list),
> +  nir_after_cf_list(list));
> +   nir_cf_reinsert(_cf_list, nir_after_block(dest_blk));
> +}
> +
> +static bool
> +opt_if_merge(nir_if *nif)
> +{
> +   bool progress = false;
> +
> +   nir_block *next_blk = nir_cf_node_cf_tree_next(>cf_node);
> +   if (next_blk && nif->condition.is_ssa) {
> +  nir_if *next_if = nir_block_get_following_if(next_blk);
> +  if (next_if && next_if->condition.is_ssa) {
> +
> + /* Here we merge two consecutive ifs that have the same
> +  * condition e.g:
> +  *
> +  *   if ssa_12 {
> +  *  ...
> +  *   } else {
> +  *  ...
> +  *   }
> +  *   if ssa_12 {
> +  *  ...
> +  *   } else {
> +  *  ...
> +  *   }
> +  *
> +  * Note: This only merges if-statements when the block between them
> +  * is empty. The reason we don't try to merge ifs that just have 
> phis
> +  * between them is because this can results in increased register
> +  * pressure. For example when merging if ladders created by indirect
> +  * indexing.
> +  */
> + if (nif->condition.ssa == next_if->condition.ssa &&
> + exec_list_is_empty(_blk->instr_list)) {
> +
> +simple_merge_if(nif, next_if, true, true);
> +simple_merge_if(nif, next_if, false, false);
> +
> +nir_block *new_then_block = nir_if_last_then_block(nif);
> +nir_block *new_else_block = nir_if_last_else_block(nif);
> +
> +nir_block *old_then_block = nir_if_last_then_block(next_if);
> +nir_block *old_else_block = nir_if_last_else_block(next_if);
> +
> +/* Rewrite the predecessor block for any phis following the 
> second
> + * if-statement.
> + */
> +rewrite_phi_predecessor_blocks(next_if, old_then_block,
> +   old_else_block,
> +   new_then_block,
> +   new_else_block);
> +
> +/* Move phis after merged if to avoid them being 

Re: [Mesa-dev] Lets talk about autotools

2018-11-27 Thread Eero Tamminen

Hi,

On 27.11.2018 19.05, Matt Turner wrote:

On Tue, Nov 27, 2018 at 1:13 AM Timo Aaltonen  wrote:

On 17.11.2018 6.04, Dylan Baker wrote:

Quoting Dylan Baker (2018-09-17 09:44:07)

I feel like for !windows meson is in good enough shape at this point that we
can start having the discussion about deleting the autotools build. So, is there
anything left that autotools can do that meson cannot (that we actually want to
implement)? And, what is a reasonable time-table to remove the autotools build?
I think we could reasonably remove it as soon as 18.3 if others felt confident
that it would work for them.


Okay, time for an update on things and a chance to talk about what else we need.

Support for llvm-config (and any binary, actually) overriding has landed in
meson, and will be present in the 0.49.0 release, which is due out December 9th.


Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and will
get Mesa backports up until and including 20.0.x, so I wonder how
complex these required new features in meson are to be backported, or
perhaps easily worked around? Backporting a whole new version of meson
might not happen..


I understand the LTS concept, but what's the value in never upgrading
something like a build tool like Meson? Yeah, new versions give a
possibility of regressions, but with something evolving as quickly as
Meson the version available in April 2018 becomes less useful for its
intended purpose with each passing month...


Ubuntu has so called hardware enabling (HWE) packages, which provide
newer versions of kernel, mesa and few other components for LTS, and
a meta package(s) that pull those in. They're based on package versions
tested in development versions of Ubuntu.

For example, relevant Ubuntu 18.10 packages would be available as
HWE packages for 18.04 somewhere around February, according to
preliminary Ubuntu kernel schedule.

Of the packages relevant to Mesa, 18.10 has for example:
- kernel v4.18   (18.04 has v4.15)
- LLVM v7.0  (18.04 has LLVM v6)
- Meson v0.47.2  (18.04 has v0.45)

I don't know how much that 4 month gap (before new development
distro release package versions become available in preceding LTS
release as HW packages) fluctuates.

Timo?


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


Re: [Mesa-dev] Lets talk about autotools

2018-11-27 Thread Matt Turner
On Tue, Nov 27, 2018 at 1:13 AM Timo Aaltonen  wrote:
>
> On 17.11.2018 6.04, Dylan Baker wrote:
> > Quoting Dylan Baker (2018-09-17 09:44:07)
> >> I feel like for !windows meson is in good enough shape at this point that 
> >> we
> >> can start having the discussion about deleting the autotools build. So, is 
> >> there
> >> anything left that autotools can do that meson cannot (that we actually 
> >> want to
> >> implement)? And, what is a reasonable time-table to remove the autotools 
> >> build?
> >> I think we could reasonably remove it as soon as 18.3 if others felt 
> >> confident
> >> that it would work for them.
> >>
> >> Dylan
> >
> > Okay, time for an update on things and a chance to talk about what else we 
> > need.
> >
> > Support for llvm-config (and any binary, actually) overriding has landed in
> > meson, and will be present in the 0.49.0 release, which is due out December 
> > 9th.
>
> Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and will
> get Mesa backports up until and including 20.0.x, so I wonder how
> complex these required new features in meson are to be backported, or
> perhaps easily worked around? Backporting a whole new version of meson
> might not happen..

I understand the LTS concept, but what's the value in never upgrading
something like a build tool like Meson? Yeah, new versions give a
possibility of regressions, but with something evolving as quickly as
Meson the version available in April 2018 becomes less useful for its
intended purpose with each passing month...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH mesa] wsi/display: fix mem leak when freeing swapchains

2018-11-27 Thread Eric Engestrom
Fixes: da997ebec92942193955 "vulkan: Add KHR_display extension using DRM [v10]"
Cc: Keith Packard 
Signed-off-by: Eric Engestrom 
---
 src/vulkan/wsi/wsi_common_display.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/vulkan/wsi/wsi_common_display.c 
b/src/vulkan/wsi/wsi_common_display.c
index cdd84dd720d99d95f291..072f47421db8666cef2f 100644
--- a/src/vulkan/wsi/wsi_common_display.c
+++ b/src/vulkan/wsi/wsi_common_display.c
@@ -1048,6 +1048,8 @@ wsi_display_swapchain_destroy(struct wsi_swapchain 
*drv_chain,
 
for (uint32_t i = 0; i < chain->base.image_count; i++)
   wsi_display_image_finish(drv_chain, >images[i]);
+
+   wsi_swapchain_finish(>base);
vk_free(allocator, chain);
return VK_SUCCESS;
 }
-- 
Cheers,
  Eric

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


Re: [Mesa-dev] [PATCH 2/2] egl/wayland: plug memory leak in drm_handle_device()

2018-11-27 Thread Eric Engestrom
On Tuesday, 2018-11-27 11:39:42 +, Emil Velikov wrote:
> From: Emil Velikov 
> 
> As we fail to open the node, we leak the node/device name.
> 
> Cc: mesa-sta...@lists.freedesktop.org
> Cc: Eric Engestrom 
> Signed-off-by: Emil Velikov 
> ---
>  src/egl/drivers/dri2/platform_wayland.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/egl/drivers/dri2/platform_wayland.c 
> b/src/egl/drivers/dri2/platform_wayland.c
> index b05f5363163..df25a482908 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -1125,6 +1125,8 @@ drm_handle_device(void *data, struct wl_drm *drm, const 
> char *device)
>  
> dri2_dpy->fd = loader_open_device(dri2_dpy->device_name);
> if (dri2_dpy->fd == -1) {
> +  free(dri2_dpy->device_name);
> +  dri2_dpy->device_name = NULL:
>_eglLog(_EGL_WARNING, "wayland-egl: could not open %s (%s)",
>dri2_dpy->device_name, strerror(errno));

You'll want to free *after* printing device_name ;)
With that:
Reviewed-by: Eric Engestrom 

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


[Mesa-dev] [PATCH 2/2] egl/wayland: plug memory leak in drm_handle_device()

2018-11-27 Thread Emil Velikov
From: Emil Velikov 

As we fail to open the node, we leak the node/device name.

Cc: mesa-sta...@lists.freedesktop.org
Cc: Eric Engestrom 
Signed-off-by: Emil Velikov 
---
 src/egl/drivers/dri2/platform_wayland.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/egl/drivers/dri2/platform_wayland.c 
b/src/egl/drivers/dri2/platform_wayland.c
index b05f5363163..df25a482908 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -1125,6 +1125,8 @@ drm_handle_device(void *data, struct wl_drm *drm, const 
char *device)
 
dri2_dpy->fd = loader_open_device(dri2_dpy->device_name);
if (dri2_dpy->fd == -1) {
+  free(dri2_dpy->device_name);
+  dri2_dpy->device_name = NULL:
   _eglLog(_EGL_WARNING, "wayland-egl: could not open %s (%s)",
   dri2_dpy->device_name, strerror(errno));
   return;
-- 
2.19.1

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


[Mesa-dev] [PATCH 1/2] egl/wayland: bail out when drmGetMagic fails

2018-11-27 Thread Emil Velikov
From: Emil Velikov 

Currently as the function fails, we pass uninitialized data to the
authentication function. Stop doing that and print an warning when
the function fails.

v2: Plug memory leak in error path (Eric)

Cc: mesa-sta...@lists.freedesktop.org
Cc: Tapani Pälli  (v1)
Cc: Eric Engestrom 
Signed-off-by: Emil Velikov 
Reviewed-by: Tapani Pälli  (v1)
Reviewed-by: Eric Engestrom 
---
Sending out for posterity.
---
 src/egl/drivers/dri2/platform_wayland.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/egl/drivers/dri2/platform_wayland.c 
b/src/egl/drivers/dri2/platform_wayland.c
index 8122c811288..b05f5363163 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -1133,7 +1133,14 @@ drm_handle_device(void *data, struct wl_drm *drm, const 
char *device)
if (drmGetNodeTypeFromFd(dri2_dpy->fd) == DRM_NODE_RENDER) {
   dri2_dpy->authenticated = true;
} else {
-  drmGetMagic(dri2_dpy->fd, );
+  if (drmGetMagic(dri2_dpy->fd, )) {
+ close(dri2_dpy->fd);
+ dri2_dpy->fd = -1;
+ free(dri2_dpy->device_name);
+ dri2_dpy->device_name = NULL:
+ _eglLog(_EGL_WARNING, "wayland-egl: drmGetMagic failed");
+ return;
+  }
   wl_drm_authenticate(dri2_dpy->wl_drm, magic);
}
 }
-- 
2.19.1

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


Re: [Mesa-dev] Lets talk about autotools

2018-11-27 Thread Timo Aaltonen
On 27.11.2018 12.20, Erik Faye-Lund wrote:
> On Tue, 2018-11-27 at 11:13 +0200, Timo Aaltonen wrote:
>> On 17.11.2018 6.04, Dylan Baker wrote:
>>> Quoting Dylan Baker (2018-09-17 09:44:07)
 I feel like for !windows meson is in good enough shape at this
 point that we
 can start having the discussion about deleting the autotools
 build. So, is there
 anything left that autotools can do that meson cannot (that we
 actually want to
 implement)? And, what is a reasonable time-table to remove the
 autotools build?
 I think we could reasonably remove it as soon as 18.3 if others
 felt confident
 that it would work for them.

 Dylan
>>>
>>> Okay, time for an update on things and a chance to talk about what
>>> else we need.
>>>
>>> Support for llvm-config (and any binary, actually) overriding has
>>> landed in
>>> meson, and will be present in the 0.49.0 release, which is due out
>>> December 9th.
>>
>> Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and
>> will
>> get Mesa backports up until and including 20.0.x, so I wonder how
>> complex these required new features in meson are to be backported, or
>> perhaps easily worked around? Backporting a whole new version of
>> meson
>> might not happen..
>>
> 
> I don't know if this is acceptable or not for packaging, but one could
> always install meson from pip, I would assume...

Nope, network resources aren't available (nor allowed) when building for
the distro.


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


Re: [Mesa-dev] [PATCH] gallium: Fix uninitialized variable warning in compute test.

2018-11-27 Thread Elie Tournier
On Mon, Nov 26, 2018 at 01:13:16PM -0800, Eric Anholt wrote:
> The compiler doesn't know that ny != 0, so x might be uninitialized for
> the printf at the end.

Reviewed-by: Elie Tournier 
> ---
>  src/gallium/tests/trivial/compute.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/gallium/tests/trivial/compute.c 
> b/src/gallium/tests/trivial/compute.c
> index afe5d3e9f2be..20e5a4f140c9 100644
> --- a/src/gallium/tests/trivial/compute.c
> +++ b/src/gallium/tests/trivial/compute.c
> @@ -240,7 +240,7 @@ static void check_tex(struct context *ctx, int slot,
>util_format_get_nblocksy(tex->format, tex->height0));
>  struct pipe_transfer *xfer;
>  char *map;
> -int x, y, i;
> +int x = 0, y, i;
>  int err = 0;
>  
>  if (!check)
> -- 
> 2.20.0.rc1
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 10/15] anv/android: support creating images from external format

2018-11-27 Thread Tapani Pälli
Since we don't know the exact format at creation time, some initialization
is done only when bound with memory in vkBindImageMemory.

v2: demand dedicated allocation in vkGetImageMemoryRequirements2 if
image has external format

v3: refactor prepare_ahw_image, support vkBindImageMemory2,
calculate stride correctly for rgb(x) surfaces, rename as
'resolve_ahw_image'

v4: rebase to b43f955037c changes

Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_android.c   |  36 ++
 src/intel/vulkan/anv_android.h   |   8 +++
 src/intel/vulkan/anv_android_stubs.c |  10 +++
 src/intel/vulkan/anv_device.c|   2 +-
 src/intel/vulkan/anv_image.c | 103 ++-
 src/intel/vulkan/anv_private.h   |   4 ++
 6 files changed, 160 insertions(+), 3 deletions(-)

diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c
index bdc720214c4..3bc2b63b1c9 100644
--- a/src/intel/vulkan/anv_android.c
+++ b/src/intel/vulkan/anv_android.c
@@ -356,6 +356,42 @@ anv_create_ahw_memory(VkDevice device_h,
return VK_SUCCESS;
 }
 
+VkResult
+anv_image_from_external(
+   VkDevice device_h,
+   const VkImageCreateInfo *base_info,
+   const struct VkExternalMemoryImageCreateInfo *create_info,
+   const VkAllocationCallbacks *alloc,
+   VkImage *out_image_h)
+{
+   ANV_FROM_HANDLE(anv_device, device, device_h);
+
+   const struct VkExternalFormatANDROID *ext_info =
+  vk_find_struct_const(base_info->pNext, EXTERNAL_FORMAT_ANDROID);
+
+   if (ext_info && ext_info->externalFormat != 0) {
+  assert(base_info->format == VK_FORMAT_UNDEFINED);
+  assert(base_info->imageType == VK_IMAGE_TYPE_2D);
+  assert(base_info->usage == VK_IMAGE_USAGE_SAMPLED_BIT);
+  assert(base_info->tiling == VK_IMAGE_TILING_OPTIMAL);
+   }
+
+   struct anv_image_create_info anv_info = {
+  .vk_info = base_info,
+  .isl_extra_usage_flags = ISL_SURF_USAGE_DISABLE_AUX_BIT,
+  .external_format = true,
+   };
+
+   VkImage image_h;
+   VkResult result = anv_image_create(device_h, _info, alloc, _h);
+   if (result != VK_SUCCESS)
+  return result;
+
+   *out_image_h = image_h;
+
+   return VK_SUCCESS;
+}
+
 VkResult
 anv_image_from_gralloc(VkDevice device_h,
const VkImageCreateInfo *base_info,
diff --git a/src/intel/vulkan/anv_android.h b/src/intel/vulkan/anv_android.h
index 01f0e856291..d5c073126e3 100644
--- a/src/intel/vulkan/anv_android.h
+++ b/src/intel/vulkan/anv_android.h
@@ -29,6 +29,8 @@
 #include 
 
 struct anv_device_memory;
+struct anv_device;
+struct anv_image;
 
 VkResult anv_image_from_gralloc(VkDevice device_h,
 const VkImageCreateInfo *base_info,
@@ -36,6 +38,12 @@ VkResult anv_image_from_gralloc(VkDevice device_h,
 const VkAllocationCallbacks *alloc,
 VkImage *pImage);
 
+VkResult anv_image_from_external(VkDevice device_h,
+ const VkImageCreateInfo *base_info,
+ const struct VkExternalMemoryImageCreateInfo 
*create_info,
+ const VkAllocationCallbacks *alloc,
+ VkImage *out_image_h);
+
 uint64_t anv_ahw_usage_from_vk_usage(const VkImageCreateFlags vk_create,
  const VkImageUsageFlags vk_usage);
 
diff --git a/src/intel/vulkan/anv_android_stubs.c 
b/src/intel/vulkan/anv_android_stubs.c
index ccc16500494..a34afc198a1 100644
--- a/src/intel/vulkan/anv_android_stubs.c
+++ b/src/intel/vulkan/anv_android_stubs.c
@@ -55,3 +55,13 @@ anv_create_ahw_memory(VkDevice device_h,
 {
return VK_ERROR_EXTENSION_NOT_PRESENT;
 }
+
+VkResult
+anv_image_from_external(VkDevice device_h,
+const VkImageCreateInfo *base_info,
+const struct VkExternalMemoryImageCreateInfo 
*create_info,
+const VkAllocationCallbacks *alloc,
+VkImage *out_image_h)
+{
+   return VK_ERROR_EXTENSION_NOT_PRESENT;
+}
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index a1ee9315956..5777de96d80 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -2768,7 +2768,7 @@ void anv_GetImageMemoryRequirements2(
   switch (ext->sType) {
   case VK_STRUCTURE_TYPE_MEMORY_DEDICATED_REQUIREMENTS: {
  VkMemoryDedicatedRequirements *requirements = (void *)ext;
- if (image->needs_set_tiling) {
+ if (image->needs_set_tiling || image->external_format) {
 /* If we need to set the tiling for external consumers, we need a
  * dedicated allocation.
  *
diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
index 59e9d007831..79777efe456 100644
--- a/src/intel/vulkan/anv_image.c
+++ b/src/intel/vulkan/anv_image.c
@@ -594,6 +594,15 @@ anv_image_create(VkDevice _device,
image->drm_format_mod = 

[Mesa-dev] [PATCH 07/15] anv: refactor, remove else block in AllocateMemory

2018-11-27 Thread Tapani Pälli
This makes it cleaner to introduce more cases where we import memory
from different types of external memory buffers.

Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_device.c | 64 +++
 1 file changed, 34 insertions(+), 30 deletions(-)

diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index 6b5ba25c6bc..0cbbaeca3b3 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -2340,42 +2340,46 @@ VkResult anv_AllocateMemory(
* If the import fails, we leave the file descriptor open.
*/
   close(fd_info->fd);
-   } else {
-  const VkExportMemoryAllocateInfoKHR *fd_info =
- vk_find_struct_const(pAllocateInfo->pNext, 
EXPORT_MEMORY_ALLOCATE_INFO_KHR);
-  if (fd_info && fd_info->handleTypes)
- bo_flags |= ANV_BO_EXTERNAL;
-
-  result = anv_bo_cache_alloc(device, >bo_cache,
-  pAllocateInfo->allocationSize, bo_flags,
-  >bo);
-  if (result != VK_SUCCESS)
- goto fail;
+  goto success;
+   }
 
-  const VkMemoryDedicatedAllocateInfoKHR *dedicated_info =
- vk_find_struct_const(pAllocateInfo->pNext, 
MEMORY_DEDICATED_ALLOCATE_INFO_KHR);
-  if (dedicated_info && dedicated_info->image != VK_NULL_HANDLE) {
- ANV_FROM_HANDLE(anv_image, image, dedicated_info->image);
+   /* Regular allocate (not importing memory). */
 
- /* Some legacy (non-modifiers) consumers need the tiling to be set on
-  * the BO.  In this case, we have a dedicated allocation.
-  */
- if (image->needs_set_tiling) {
-const uint32_t i915_tiling =
-   isl_tiling_to_i915_tiling(image->planes[0].surface.isl.tiling);
-int ret = anv_gem_set_tiling(device, mem->bo->gem_handle,
- 
image->planes[0].surface.isl.row_pitch_B,
- i915_tiling);
-if (ret) {
-   anv_bo_cache_release(device, >bo_cache, mem->bo);
-   return vk_errorf(device->instance, NULL,
-VK_ERROR_OUT_OF_DEVICE_MEMORY,
-"failed to set BO tiling: %m");
-}
+   const VkExportMemoryAllocateInfoKHR *export_info =
+  vk_find_struct_const(pAllocateInfo->pNext, 
EXPORT_MEMORY_ALLOCATE_INFO_KHR);
+   if (export_info && export_info->handleTypes)
+  bo_flags |= ANV_BO_EXTERNAL;
+
+   result = anv_bo_cache_alloc(device, >bo_cache,
+   pAllocateInfo->allocationSize, bo_flags,
+   >bo);
+   if (result != VK_SUCCESS)
+  goto fail;
+
+   const VkMemoryDedicatedAllocateInfoKHR *dedicated_info =
+  vk_find_struct_const(pAllocateInfo->pNext, 
MEMORY_DEDICATED_ALLOCATE_INFO_KHR);
+   if (dedicated_info && dedicated_info->image != VK_NULL_HANDLE) {
+  ANV_FROM_HANDLE(anv_image, image, dedicated_info->image);
+
+  /* Some legacy (non-modifiers) consumers need the tiling to be set on
+   * the BO.  In this case, we have a dedicated allocation.
+   */
+  if (image->needs_set_tiling) {
+ const uint32_t i915_tiling =
+isl_tiling_to_i915_tiling(image->planes[0].surface.isl.tiling);
+ int ret = anv_gem_set_tiling(device, mem->bo->gem_handle,
+  image->planes[0].surface.isl.row_pitch_B,
+  i915_tiling);
+ if (ret) {
+anv_bo_cache_release(device, >bo_cache, mem->bo);
+return vk_errorf(device->instance, NULL,
+ VK_ERROR_OUT_OF_DEVICE_MEMORY,
+ "failed to set BO tiling: %m");
  }
   }
}
 
+ success:
*pMem = anv_device_memory_to_handle(mem);
 
return VK_SUCCESS;
-- 
2.17.2

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


[Mesa-dev] [PATCH 11/15] anv: support VkExternalFormatANDROID in vkCreateSamplerYcbcrConversion

2018-11-27 Thread Tapani Pälli
If external format is used, we store the external format identifier in
conversion to be used later when creating VkImageView.

v2: rebase to b43f955037c changes

Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_formats.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c
index 972a6f98620..6e7807579df 100644
--- a/src/intel/vulkan/anv_formats.c
+++ b/src/intel/vulkan/anv_formats.c
@@ -1170,6 +1170,15 @@ VkResult anv_CreateSamplerYcbcrConversion(
ANV_FROM_HANDLE(anv_device, device, _device);
struct anv_ycbcr_conversion *conversion;
 
+   /* Search for VkExternalFormatANDROID and resolve the format. */
+   struct anv_format *ext_format = NULL;
+   const struct VkExternalFormatANDROID *ext_info =
+  vk_find_struct_const(pCreateInfo->pNext, EXTERNAL_FORMAT_ANDROID);
+
+   uint64_t format = ext_info ? ext_info->externalFormat : 0;
+   if (format)
+  ext_format = (struct anv_format *) (uintptr_t) format;
+
assert(pCreateInfo->sType == 
VK_STRUCTURE_TYPE_SAMPLER_YCBCR_CONVERSION_CREATE_INFO);
 
conversion = vk_alloc2(>alloc, pAllocator, sizeof(*conversion), 8,
@@ -1190,6 +1199,10 @@ VkResult anv_CreateSamplerYcbcrConversion(
conversion->chroma_offsets[1] = pCreateInfo->yChromaOffset;
conversion->chroma_filter = pCreateInfo->chromaFilter;
 
+   /* Setup external format. */
+   if (ext_format)
+  conversion->format = ext_format;
+
bool has_chroma_subsampled = false;
for (uint32_t p = 0; p < conversion->format->n_planes; p++) {
   if (conversion->format->planes[p].has_chroma &&
-- 
2.17.2

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


[Mesa-dev] [PATCH 06/15] anv: add anv_ahw_usage_from_vk_usage helper function

2018-11-27 Thread Tapani Pälli
v2: rebase to b43f955037c changes

Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_android.c   | 31 
 src/intel/vulkan/anv_android.h   |  2 ++
 src/intel/vulkan/anv_android_stubs.c |  7 +++
 3 files changed, 40 insertions(+)

diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c
index 54b62b9d02f..f2dd16212c1 100644
--- a/src/intel/vulkan/anv_android.c
+++ b/src/intel/vulkan/anv_android.c
@@ -202,6 +202,37 @@ anv_GetAndroidHardwareBufferPropertiesANDROID(
return VK_SUCCESS;
 }
 
+/* Construct ahw usage mask from image usage bits, see
+ * 'AHardwareBuffer Usage Equivalence' in Vulkan spec.
+ */
+uint64_t
+anv_ahw_usage_from_vk_usage(const VkImageCreateFlags vk_create,
+const VkImageUsageFlags vk_usage)
+{
+   uint64_t ahw_usage = 0;
+
+   if (vk_usage & VK_IMAGE_USAGE_SAMPLED_BIT)
+  ahw_usage |= AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE;
+
+   if (vk_usage & VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT)
+  ahw_usage |= AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE;
+
+   if (vk_usage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT)
+  ahw_usage |= AHARDWAREBUFFER_USAGE_GPU_COLOR_OUTPUT;
+
+   if (vk_create & VK_IMAGE_CREATE_CUBE_COMPATIBLE_BIT)
+  ahw_usage |= AHARDWAREBUFFER_USAGE_GPU_CUBE_MAP;
+
+   if (vk_create & VK_IMAGE_CREATE_PROTECTED_BIT)
+  ahw_usage |= AHARDWAREBUFFER_USAGE_PROTECTED_CONTENT;
+
+   /* No usage bits set - set at least one GPU usage. */
+   if (ahw_usage == 0)
+  ahw_usage = AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE;
+
+   return ahw_usage;
+}
+
 VkResult
 anv_image_from_gralloc(VkDevice device_h,
const VkImageCreateInfo *base_info,
diff --git a/src/intel/vulkan/anv_android.h b/src/intel/vulkan/anv_android.h
index a27c364471b..8d748cecc39 100644
--- a/src/intel/vulkan/anv_android.h
+++ b/src/intel/vulkan/anv_android.h
@@ -34,4 +34,6 @@ VkResult anv_image_from_gralloc(VkDevice device_h,
 const VkAllocationCallbacks *alloc,
 VkImage *pImage);
 
+uint64_t anv_ahw_usage_from_vk_usage(const VkImageCreateFlags vk_create,
+ const VkImageUsageFlags vk_usage);
 #endif /* ANV_ANDROID_H */
diff --git a/src/intel/vulkan/anv_android_stubs.c 
b/src/intel/vulkan/anv_android_stubs.c
index a6fe5a5e6b3..0671d5635ee 100644
--- a/src/intel/vulkan/anv_android_stubs.c
+++ b/src/intel/vulkan/anv_android_stubs.c
@@ -32,3 +32,10 @@ anv_image_from_gralloc(VkDevice device_h,
 {
return VK_ERROR_EXTENSION_NOT_PRESENT;
 }
+
+uint64_t
+anv_ahw_usage_from_vk_usage(const VkImageCreateFlags vk_create,
+const VkImageUsageFlags vk_usage)
+{
+   return 0;
+}
-- 
2.17.2

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


[Mesa-dev] [PATCH 05/15] anv/android: add GetAndroidHardwareBufferPropertiesANDROID

2018-11-27 Thread Tapani Pälli
Use the anv_format address in formats table as implementation-defined
external format identifier for now. When adding YUV format support this
might need to change.

v2: code cleanup (Jason)
v3: set anv_format address as identifier
v4: setup suggestedYcbcrModel and suggested[X|Y]ChromaOffset
as expected for HAL_PIXEL_FORMAT_NV12_Y_TILED_INTEL

Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_android.c | 106 +
 1 file changed, 106 insertions(+)

diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c
index 916e76c93ff..54b62b9d02f 100644
--- a/src/intel/vulkan/anv_android.c
+++ b/src/intel/vulkan/anv_android.c
@@ -29,6 +29,8 @@
 #include 
 
 #include "anv_private.h"
+#include "vk_format_info.h"
+#include "vk_util.h"
 
 static int anv_hal_open(const struct hw_module_t* mod, const char* id, struct 
hw_device_t** dev);
 static int anv_hal_close(struct hw_device_t *dev);
@@ -96,6 +98,110 @@ anv_hal_close(struct hw_device_t *dev)
return -1;
 }
 
+static VkResult
+get_ahw_buffer_format_properties(
+   VkDevice device_h,
+   const struct AHardwareBuffer *buffer,
+   VkAndroidHardwareBufferFormatPropertiesANDROID *pProperties)
+{
+   ANV_FROM_HANDLE(anv_device, device, device_h);
+
+   /* Get a description of buffer contents . */
+   AHardwareBuffer_Desc desc;
+   AHardwareBuffer_describe(buffer, );
+
+   /* Verify description. */
+   uint64_t gpu_usage =
+  AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE |
+  AHARDWAREBUFFER_USAGE_GPU_COLOR_OUTPUT |
+  AHARDWAREBUFFER_USAGE_GPU_DATA_BUFFER;
+
+   /* "Buffer must be a valid Android hardware buffer object with at least
+* one of the AHARDWAREBUFFER_USAGE_GPU_* usage flags."
+*/
+   if (!(desc.usage & (gpu_usage)))
+  return VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR;
+
+   /* Fill properties fields based on description. */
+   VkAndroidHardwareBufferFormatPropertiesANDROID *p = pProperties;
+
+   p->format = vk_format_from_android(desc.format);
+
+   const struct anv_format *anv_format = anv_get_format(p->format);
+   p->externalFormat = (uint64_t) (uintptr_t) anv_format;
+
+   p->formatFeatures =
+  anv_get_image_format_features(>info, p->format,
+anv_format, VK_IMAGE_TILING_OPTIMAL);
+
+   /* "Images can be created with an external format even if the Android 
hardware
+*  buffer has a format which has an equivalent Vulkan format to enable
+*  consistent handling of images from sources that might use either 
category
+*  of format. However, all images created with an external format are 
subject
+*  to the valid usage requirements associated with external formats, even 
if
+*  the Android hardware buffer’s format has a Vulkan equivalent."
+*
+* "The formatFeatures member *must* include
+*  VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT and at least one of
+*  VK_FORMAT_FEATURE_MIDPOINT_CHROMA_SAMPLES_BIT or
+*  VK_FORMAT_FEATURE_COSITED_CHROMA_SAMPLES_BIT"
+*/
+   p->formatFeatures |=
+  VK_FORMAT_FEATURE_MIDPOINT_CHROMA_SAMPLES_BIT;
+
+   /* "Implementations may not always be able to determine the color model,
+*  numerical range, or chroma offsets of the image contents, so the values
+*  in VkAndroidHardwareBufferFormatPropertiesANDROID are only suggestions.
+*  Applications should treat these values as sensible defaults to use in
+*  the absence of more reliable information obtained through some other
+*  means."
+*/
+   p->samplerYcbcrConversionComponents.r = VK_COMPONENT_SWIZZLE_IDENTITY;
+   p->samplerYcbcrConversionComponents.g = VK_COMPONENT_SWIZZLE_IDENTITY;
+   p->samplerYcbcrConversionComponents.b = VK_COMPONENT_SWIZZLE_IDENTITY;
+   p->samplerYcbcrConversionComponents.a = VK_COMPONENT_SWIZZLE_IDENTITY;
+
+   p->suggestedYcbcrModel = VK_SAMPLER_YCBCR_MODEL_CONVERSION_YCBCR_601;
+   p->suggestedYcbcrRange = VK_SAMPLER_YCBCR_RANGE_ITU_FULL;
+
+   p->suggestedXChromaOffset = VK_CHROMA_LOCATION_MIDPOINT;
+   p->suggestedYChromaOffset = VK_CHROMA_LOCATION_MIDPOINT;
+
+   return VK_SUCCESS;
+}
+
+VkResult
+anv_GetAndroidHardwareBufferPropertiesANDROID(
+   VkDevice device_h,
+   const struct AHardwareBuffer *buffer,
+   VkAndroidHardwareBufferPropertiesANDROID *pProperties)
+{
+   ANV_FROM_HANDLE(anv_device, dev, device_h);
+   struct anv_physical_device *pdevice = >instance->physicalDevice;
+
+   VkAndroidHardwareBufferFormatPropertiesANDROID *format_prop =
+  vk_find_struct(pProperties->pNext,
+ ANDROID_HARDWARE_BUFFER_FORMAT_PROPERTIES_ANDROID);
+
+   /* Fill format properties of an Android hardware buffer. */
+   if (format_prop)
+  get_ahw_buffer_format_properties(device_h, buffer, format_prop);
+
+   const native_handle_t *handle =
+  AHardwareBuffer_getNativeHandle(buffer);
+   int dma_buf = (handle && handle->numFds) ? handle->data[0] : -1;
+   if (dma_buf < 0)
+  return VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR;
+
+   /* All memory types. */
+   

[Mesa-dev] [PATCH 04/15] anv: add from/to helpers with android and vulkan formats

2018-11-27 Thread Tapani Pälli
v2: handle R8G8B8X8 as R8G8B8_UNORM (Jason)
v3: add HAL_PIXEL_FORMAT_NV12_Y_TILED_INTEL, we make it define
for now to avoid direct dependency to minigbm headers

Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/vk_format_info.h | 50 +++
 1 file changed, 50 insertions(+)

diff --git a/src/intel/vulkan/vk_format_info.h 
b/src/intel/vulkan/vk_format_info.h
index a1cc6952c8f..555c67704bc 100644
--- a/src/intel/vulkan/vk_format_info.h
+++ b/src/intel/vulkan/vk_format_info.h
@@ -27,6 +27,56 @@
 #include 
 #include 
 
+#ifdef ANDROID
+#include 
+/* See i915_private_android_types.h in minigbm. */
+#define HAL_PIXEL_FORMAT_NV12_Y_TILED_INTEL 0x100
+
+static inline VkFormat
+vk_format_from_android(unsigned android_format)
+{
+   switch (android_format) {
+   case AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM:
+  return VK_FORMAT_R8G8B8A8_UNORM;
+   case AHARDWAREBUFFER_FORMAT_R8G8B8X8_UNORM:
+   case AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM:
+  return VK_FORMAT_R8G8B8_UNORM;
+   case AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM:
+  return VK_FORMAT_R5G6B5_UNORM_PACK16;
+   case AHARDWAREBUFFER_FORMAT_R16G16B16A16_FLOAT:
+  return VK_FORMAT_R16G16B16A16_SFLOAT;
+   case AHARDWAREBUFFER_FORMAT_R10G10B10A2_UNORM:
+  return VK_FORMAT_A2B10G10R10_UNORM_PACK32;
+   case HAL_PIXEL_FORMAT_NV12_Y_TILED_INTEL:
+  return VK_FORMAT_G8_B8R8_2PLANE_420_UNORM;
+   case AHARDWAREBUFFER_FORMAT_BLOB:
+   default:
+  return VK_FORMAT_UNDEFINED;
+   }
+}
+
+static inline unsigned
+android_format_from_vk(unsigned vk_format)
+{
+   switch (vk_format) {
+   case VK_FORMAT_R8G8B8A8_UNORM:
+  return AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM;
+   case VK_FORMAT_R8G8B8_UNORM:
+  return AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM;
+   case VK_FORMAT_R5G6B5_UNORM_PACK16:
+  return AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM;
+   case VK_FORMAT_R16G16B16A16_SFLOAT:
+  return AHARDWAREBUFFER_FORMAT_R16G16B16A16_FLOAT;
+   case VK_FORMAT_A2B10G10R10_UNORM_PACK32:
+  return AHARDWAREBUFFER_FORMAT_R10G10B10A2_UNORM;
+   case VK_FORMAT_G8_B8R8_2PLANE_420_UNORM:
+  return HAL_PIXEL_FORMAT_NV12_Y_TILED_INTEL;
+   default:
+  return AHARDWAREBUFFER_FORMAT_BLOB;
+   }
+}
+#endif
+
 static inline VkImageAspectFlags
 vk_format_aspects(VkFormat format)
 {
-- 
2.17.2

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


[Mesa-dev] [PATCH 13/15] anv: support VkSamplerYcbcrConversionInfo in vkCreateImageView

2018-11-27 Thread Tapani Pälli
If a conversion struct was passed, then initialize view using
format from the conversion structure.

v2: use vk_format directly from the anv_format struct

Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_image.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
index 79777efe456..2ac3eccbbe0 100644
--- a/src/intel/vulkan/anv_image.c
+++ b/src/intel/vulkan/anv_image.c
@@ -1391,6 +1391,16 @@ anv_CreateImageView(VkDevice _device,
assert(range->layerCount > 0);
assert(range->baseMipLevel < image->levels);
 
+   /* Check if a conversion info was passed. */
+   const struct anv_format *conv_format = NULL;
+   const struct VkSamplerYcbcrConversionInfo *conv_info =
+  vk_find_struct_const(pCreateInfo->pNext, SAMPLER_YCBCR_CONVERSION_INFO);
+
+   if (conv_info) {
+  ANV_FROM_HANDLE(anv_ycbcr_conversion, conversion, conv_info->conversion);
+  conv_format = conversion->format;
+   }
+
const VkImageViewUsageCreateInfo *usage_info =
   vk_find_struct_const(pCreateInfo, IMAGE_VIEW_USAGE_CREATE_INFO);
VkImageUsageFlags view_usage = usage_info ? usage_info->usage : 
image->usage;
@@ -1435,6 +1445,12 @@ anv_CreateImageView(VkDevice _device,
iview->n_planes = anv_image_aspect_get_planes(iview->aspect_mask);
iview->vk_format = pCreateInfo->format;
 
+   /* Format is undefined, this can happen when using external formats. Set
+* view format from the passed conversion info.
+*/
+   if (iview->vk_format == VK_FORMAT_UNDEFINED && conv_format)
+  iview->vk_format = conv_format->vk_format;
+
iview->extent = (VkExtent3D) {
   .width  = anv_minify(image->extent.width , range->baseMipLevel),
   .height = anv_minify(image->extent.height, range->baseMipLevel),
@@ -1451,7 +1467,7 @@ anv_CreateImageView(VkDevice _device,
   VkImageAspectFlags vplane_aspect =
  anv_plane_to_aspect(iview->aspect_mask, vplane);
   struct anv_format_plane format =
- anv_get_format_plane(>info, pCreateInfo->format,
+ anv_get_format_plane(>info, iview->vk_format,
   vplane_aspect, image->tiling);
 
   iview->planes[vplane].image_plane = iplane;
-- 
2.17.2

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


[Mesa-dev] [PATCH 12/15] anv: add VkFormat field as part of anv_format

2018-11-27 Thread Tapani Pälli
Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_formats.c | 4 
 src/intel/vulkan/anv_private.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c
index 6e7807579df..f27456fa41d 100644
--- a/src/intel/vulkan/anv_formats.c
+++ b/src/intel/vulkan/anv_formats.c
@@ -54,6 +54,7 @@
.aspect = VK_IMAGE_ASPECT_COLOR_BIT, \
  }, \
   }, \
+  .vk_format = __vk_fmt, \
   .n_planes = 1, \
}
 
@@ -94,6 +95,7 @@
.aspect = VK_IMAGE_ASPECT_STENCIL_BIT, \
  }, \
   }, \
+  .vk_format = __vk_fmt, \
   .n_planes = 2, \
}
 
@@ -102,6 +104,7 @@
   .planes = { \
  { .isl_format = ISL_FORMAT_UNSUPPORTED, }, \
   }, \
+  .vk_format = VK_FORMAT_UNDEFINED, \
}
 
 #define y_plane(__plane, __hw_fmt, __swizzle, __ycbcr_swizzle, dhs, dvs) \
@@ -127,6 +130,7 @@
   .planes = { \
  __VA_ARGS__, \
   }, \
+  .vk_format = __vk_fmt, \
   .n_planes = __n_planes, \
   .can_ycbcr = true, \
}
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index 893c5da7abc..84678de37f8 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -2562,6 +2562,7 @@ struct anv_format_plane {
 
 struct anv_format {
struct anv_format_plane planes[3];
+   VkFormat vk_format;
uint8_t n_planes;
bool can_ycbcr;
 };
-- 
2.17.2

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


[Mesa-dev] [PATCH 03/15] anv: make anv_get_image_format_features public

2018-11-27 Thread Tapani Pälli
This will be utilized later by GetAndroidHardwareBufferPropertiesANDROID.

Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_formats.c | 22 +++---
 src/intel/vulkan/anv_private.h |  5 +
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c
index d3a7c2be85c..aa2093f7f15 100644
--- a/src/intel/vulkan/anv_formats.c
+++ b/src/intel/vulkan/anv_formats.c
@@ -492,11 +492,11 @@ anv_get_format_plane(const struct gen_device_info 
*devinfo, VkFormat vk_format,
 
 // Format capabilities
 
-static VkFormatFeatureFlags
-get_image_format_features(const struct gen_device_info *devinfo,
-  VkFormat vk_format,
-  const struct anv_format *anv_format,
-  VkImageTiling vk_tiling)
+VkFormatFeatureFlags
+anv_get_image_format_features(const struct gen_device_info *devinfo,
+  VkFormat vk_format,
+  const struct anv_format *anv_format,
+  VkImageTiling vk_tiling)
 {
VkFormatFeatureFlags flags = 0;
 
@@ -743,11 +743,11 @@ void anv_GetPhysicalDeviceFormatProperties(
 
*pFormatProperties = (VkFormatProperties) {
   .linearTilingFeatures =
- get_image_format_features(devinfo, vk_format, anv_format,
-   VK_IMAGE_TILING_LINEAR),
+ anv_get_image_format_features(devinfo, vk_format, anv_format,
+   VK_IMAGE_TILING_LINEAR),
   .optimalTilingFeatures =
- get_image_format_features(devinfo, vk_format, anv_format,
-   VK_IMAGE_TILING_OPTIMAL),
+ anv_get_image_format_features(devinfo, vk_format, anv_format,
+   VK_IMAGE_TILING_OPTIMAL),
   .bufferFeatures =
  get_buffer_format_features(devinfo, vk_format, anv_format),
};
@@ -794,8 +794,8 @@ anv_get_image_format_properties(
if (format == NULL)
   goto unsupported;
 
-   format_feature_flags = get_image_format_features(devinfo, info->format,
-format, info->tiling);
+   format_feature_flags = anv_get_image_format_features(devinfo, info->format,
+format, info->tiling);
 
switch (info->type) {
default:
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index cfe16d2f0d2..c855f9e4b0d 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -3121,6 +3121,11 @@ anv_sanitize_image_offset(const VkImageType imageType,
}
 }
 
+VkFormatFeatureFlags
+anv_get_image_format_features(const struct gen_device_info *devinfo,
+  VkFormat vk_format,
+  const struct anv_format *anv_format,
+  VkImageTiling vk_tiling);
 
 void anv_fill_buffer_surface_state(struct anv_device *device,
struct anv_state state,
-- 
2.17.2

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


[Mesa-dev] [PATCH 08/15] anv/android: support import/export of AHardwareBuffer objects

2018-11-27 Thread Tapani Pälli
v2: add support for non-image buffers (AHARDWAREBUFFER_FORMAT_BLOB)
v3: properly handle usage bits when creating from image
v4: refactor, code cleanup (Jason)
v5: rebase to b43f955037c changes,
initialize bo flags as ANV_BO_EXTERNAL (Lionel)

Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_android.c   | 123 +++
 src/intel/vulkan/anv_android.h   |  10 +++
 src/intel/vulkan/anv_android_stubs.c |  16 
 src/intel/vulkan/anv_device.c|  45 +-
 src/intel/vulkan/anv_private.h   |   5 ++
 5 files changed, 197 insertions(+), 2 deletions(-)

diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c
index f2dd16212c1..bdc720214c4 100644
--- a/src/intel/vulkan/anv_android.c
+++ b/src/intel/vulkan/anv_android.c
@@ -233,6 +233,129 @@ anv_ahw_usage_from_vk_usage(const VkImageCreateFlags 
vk_create,
return ahw_usage;
 }
 
+VkResult
+anv_GetMemoryAndroidHardwareBufferANDROID(
+   VkDevice device_h,
+   const VkMemoryGetAndroidHardwareBufferInfoANDROID *pInfo,
+   struct AHardwareBuffer **pBuffer)
+{
+   ANV_FROM_HANDLE(anv_device_memory, mem, pInfo->memory);
+
+   /* Some quotes from Vulkan spec:
+*
+* "If the device memory was created by importing an Android hardware
+* buffer, vkGetMemoryAndroidHardwareBufferANDROID must return that same
+* Android hardware buffer object."
+*
+* "VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID must
+* have been included in VkExportMemoryAllocateInfoKHR::handleTypes when
+* memory was created."
+*/
+   if (mem->ahw) {
+  *pBuffer = mem->ahw;
+  /* Increase refcount. */
+  AHardwareBuffer_acquire(mem->ahw);
+  return VK_SUCCESS;
+   }
+
+   return VK_ERROR_OUT_OF_HOST_MEMORY;
+}
+
+/*
+ * Called from anv_AllocateMemory when import AHardwareBuffer.
+ */
+VkResult
+anv_import_ahw_memory(VkDevice device_h,
+  struct anv_device_memory *mem,
+  const VkImportAndroidHardwareBufferInfoANDROID *info)
+{
+   ANV_FROM_HANDLE(anv_device, device, device_h);
+
+   /* Import from AHardwareBuffer to anv_device_memory. */
+   const native_handle_t *handle =
+  AHardwareBuffer_getNativeHandle(info->buffer);
+
+   int dma_buf = (handle && handle->numFds) ? handle->data[0] : -1;
+   if (dma_buf < 0)
+  return VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR;
+
+   uint64_t bo_flags = ANV_BO_EXTERNAL;
+   if (device->instance->physicalDevice.supports_48bit_addresses)
+  bo_flags |= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
+   if (device->instance->physicalDevice.use_softpin)
+  bo_flags |= EXEC_OBJECT_PINNED;
+
+   VkResult result = anv_bo_cache_import(device, >bo_cache,
+dma_buf, bo_flags, >bo);
+   if (result != VK_SUCCESS)
+  return result;
+
+   /* "If the vkAllocateMemory command succeeds, the implementation must
+* acquire a reference to the imported hardware buffer, which it must
+* release when the device memory object is freed. If the command fails,
+* the implementation must not retain a reference."
+*/
+   AHardwareBuffer_acquire(info->buffer);
+   mem->ahw = info->buffer;
+
+   return VK_SUCCESS;
+}
+
+VkResult
+anv_create_ahw_memory(VkDevice device_h,
+  struct anv_device_memory *mem,
+  const VkMemoryAllocateInfo *pAllocateInfo)
+{
+   ANV_FROM_HANDLE(anv_device, dev, device_h);
+
+   const VkMemoryDedicatedAllocateInfo *dedicated_info =
+  vk_find_struct_const(pAllocateInfo->pNext,
+   MEMORY_DEDICATED_ALLOCATE_INFO);
+
+   uint32_t w = 0;
+   uint32_t h = 1;
+   uint32_t layers = 1;
+   uint32_t format = 0;
+   uint64_t usage = 0;
+
+   /* If caller passed dedicated information. */
+   if (dedicated_info && dedicated_info->image) {
+  ANV_FROM_HANDLE(anv_image, image, dedicated_info->image);
+  w = image->extent.width;
+  h = image->extent.height;
+  layers = image->array_size;
+  format = android_format_from_vk(image->vk_format);
+  usage = anv_ahw_usage_from_vk_usage(image->create_flags, image->usage);
+   } else if (dedicated_info && dedicated_info->buffer) {
+  ANV_FROM_HANDLE(anv_buffer, buffer, dedicated_info->buffer);
+  w = buffer->size;
+  format = AHARDWAREBUFFER_FORMAT_BLOB;
+  usage = AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN |
+  AHARDWAREBUFFER_USAGE_CPU_WRITE_OFTEN;
+   } else {
+  w = pAllocateInfo->allocationSize;
+  format = AHARDWAREBUFFER_FORMAT_BLOB;
+  usage = AHARDWAREBUFFER_USAGE_CPU_READ_OFTEN |
+  AHARDWAREBUFFER_USAGE_CPU_WRITE_OFTEN;
+   }
+
+   struct AHardwareBuffer *ahw = NULL;
+   struct AHardwareBuffer_Desc desc = {
+  .width = w,
+  .height = h,
+  .layers = layers,
+  .format = format,
+  .usage = usage,
+};
+
+   if (AHardwareBuffer_allocate(, ) != 0)
+  return VK_ERROR_OUT_OF_HOST_MEMORY;
+
+   mem->ahw = ahw;
+
+   return VK_SUCCESS;
+}
+
 

[Mesa-dev] [PATCH 09/15] anv/android: add ahardwarebuffer external memory properties

2018-11-27 Thread Tapani Pälli
v2: have separate memory properties for android, set usage
flags for buffers correctly

v3: code cleanup (Jason)
+ limit maxArrayLayers to 1 for AHardwareBuffer based images

v4: rebase to b43f955037c changes

Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_formats.c | 41 ++
 1 file changed, 41 insertions(+)

diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c
index aa2093f7f15..972a6f98620 100644
--- a/src/intel/vulkan/anv_formats.c
+++ b/src/intel/vulkan/anv_formats.c
@@ -973,6 +973,26 @@ static const VkExternalMemoryProperties prime_fd_props = {
   VK_EXTERNAL_MEMORY_HANDLE_TYPE_DMA_BUF_BIT_EXT,
 };
 
+static const VkExternalMemoryProperties android_buffer_props = {
+   .externalMemoryFeatures = VK_EXTERNAL_MEMORY_FEATURE_EXPORTABLE_BIT |
+ VK_EXTERNAL_MEMORY_FEATURE_IMPORTABLE_BIT,
+   .exportFromImportedHandleTypes =
+  VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID,
+   .compatibleHandleTypes =
+  VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID,
+};
+
+
+static const VkExternalMemoryProperties android_image_props = {
+   .externalMemoryFeatures = VK_EXTERNAL_MEMORY_FEATURE_EXPORTABLE_BIT |
+ VK_EXTERNAL_MEMORY_FEATURE_IMPORTABLE_BIT |
+ VK_EXTERNAL_MEMORY_FEATURE_DEDICATED_ONLY_BIT,
+   .exportFromImportedHandleTypes =
+  VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID,
+   .compatibleHandleTypes =
+  VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID,
+};
+
 VkResult anv_GetPhysicalDeviceImageFormatProperties2(
 VkPhysicalDevicephysicalDevice,
 const VkPhysicalDeviceImageFormatInfo2* base_info,
@@ -982,6 +1002,7 @@ VkResult anv_GetPhysicalDeviceImageFormatProperties2(
const VkPhysicalDeviceExternalImageFormatInfo *external_info = NULL;
VkExternalImageFormatPropertiesKHR *external_props = NULL;
VkSamplerYcbcrConversionImageFormatProperties *ycbcr_props = NULL;
+   struct VkAndroidHardwareBufferUsageANDROID *android_usage = NULL;
VkResult result;
 
/* Extract input structs */
@@ -1005,6 +1026,9 @@ VkResult anv_GetPhysicalDeviceImageFormatProperties2(
   case VK_STRUCTURE_TYPE_SAMPLER_YCBCR_CONVERSION_IMAGE_FORMAT_PROPERTIES:
  ycbcr_props = (void *) s;
  break;
+  case VK_STRUCTURE_TYPE_ANDROID_HARDWARE_BUFFER_USAGE_ANDROID:
+ android_usage = (void *) s;
+ break;
   default:
  anv_debug_ignored_stype(s->sType);
  break;
@@ -1016,6 +1040,15 @@ VkResult anv_GetPhysicalDeviceImageFormatProperties2(
if (result != VK_SUCCESS)
   goto fail;
 
+   if (android_usage) {
+  android_usage->androidHardwareBufferUsage =
+ anv_ahw_usage_from_vk_usage(base_info->flags,
+ base_info->usage);
+
+  /* Limit maxArrayLayers to 1 for AHardwareBuffer based images for now. */
+  base_props->imageFormatProperties.maxArrayLayers = 1;
+   }
+
/* From the Vulkan 1.0.42 spec:
 *
 *If handleType is 0, vkGetPhysicalDeviceImageFormatProperties2 will
@@ -1029,6 +1062,10 @@ VkResult anv_GetPhysicalDeviceImageFormatProperties2(
  if (external_props)
 external_props->externalMemoryProperties = prime_fd_props;
  break;
+  case VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID:
+ if (external_props)
+external_props->externalMemoryProperties = android_image_props;
+ break;
   default:
  /* From the Vulkan 1.0.42 spec:
   *
@@ -,6 +1148,10 @@ void anv_GetPhysicalDeviceExternalBufferProperties(
case VK_EXTERNAL_MEMORY_HANDLE_TYPE_DMA_BUF_BIT_EXT:
   pExternalBufferProperties->externalMemoryProperties = prime_fd_props;
   return;
+   case VK_EXTERNAL_MEMORY_HANDLE_TYPE_ANDROID_HARDWARE_BUFFER_BIT_ANDROID:
+  pExternalBufferProperties->externalMemoryProperties =
+ android_buffer_props;
+  return;
default:
   goto unsupported;
}
-- 
2.17.2

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


[Mesa-dev] [PATCH 15/15] anv/android: turn on VK_ANDROID_external_memory_android_hardware_buffer

2018-11-27 Thread Tapani Pälli
Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_extensions.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/intel/vulkan/anv_extensions.py 
b/src/intel/vulkan/anv_extensions.py
index 7c81228f705..ed9b2edef32 100644
--- a/src/intel/vulkan/anv_extensions.py
+++ b/src/intel/vulkan/anv_extensions.py
@@ -69,6 +69,7 @@ MAX_API_VERSION = None # Computed later
 # the those extension strings, then tests dEQP-VK.api.info.instance.extensions
 # and dEQP-VK.api.info.device fail due to the duplicated strings.
 EXTENSIONS = [
+Extension('VK_ANDROID_external_memory_android_hardware_buffer', 3, 
'ANDROID'),
 Extension('VK_ANDROID_native_buffer', 5, 'ANDROID'),
 Extension('VK_KHR_16bit_storage', 1, 'device->info.gen 
>= 8'),
 Extension('VK_KHR_8bit_storage',  1, 'device->info.gen 
>= 8'),
-- 
2.17.2

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


[Mesa-dev] [PATCH 00/15] VK_ANDROID_external_memory_android_hardware_buffer

2018-11-27 Thread Tapani Pälli
Hi;

Series was rebased and fixes applied from review, also some changes
applied to support HAL_PIXEL_FORMAT_NV12_Y_TILED_INTEL. With these 
changes android.graphics.cts.MediaVulkanGpuTest starts to pass, now 
all tests utilizing AHardwareBuffer pass (CTS + SkQP) \o/

tree:
https://cgit.freedesktop.org/~tpalli/mesa/log/?h=ahw

android tree used in testing:
https://github.com/tpalli/external-mesa/tree/ahw-android

CI was happy:
https://mesa-ci.01.org/tpalli/builds/642/group/63a9f0ea7bb98050796b649e85481845

Tapani Pälli (15):
  anv: add create_flags as part of anv_image
  anv: refactor make_surface to use data from anv_image
  anv: make anv_get_image_format_features public
  anv: add from/to helpers with android and vulkan formats
  anv/android: add GetAndroidHardwareBufferPropertiesANDROID
  anv: add anv_ahw_usage_from_vk_usage helper function
  anv: refactor, remove else block in AllocateMemory
  anv/android: support import/export of AHardwareBuffer objects
  anv/android: add ahardwarebuffer external memory properties
  anv/android: support creating images from external format
  anv: support VkExternalFormatANDROID in vkCreateSamplerYcbcrConversion
  anv: add VkFormat field as part of anv_format
  anv: support VkSamplerYcbcrConversionInfo in vkCreateImageView
  anv: ignore VkSamplerYcbcrConversion on non-yuv formats
  anv/android: turn on
VK_ANDROID_external_memory_android_hardware_buffer

 src/intel/vulkan/anv_android.c   | 296 +++
 src/intel/vulkan/anv_android.h   |  20 ++
 src/intel/vulkan/anv_android_stubs.c |  33 +++
 src/intel/vulkan/anv_device.c| 107 +++---
 src/intel/vulkan/anv_extensions.py   |   1 +
 src/intel/vulkan/anv_formats.c   |  80 +++-
 src/intel/vulkan/anv_image.c | 200 ++
 src/intel/vulkan/anv_private.h   |  21 ++
 src/intel/vulkan/genX_state.c|   7 +-
 src/intel/vulkan/vk_format_info.h|  50 +
 10 files changed, 731 insertions(+), 84 deletions(-)

-- 
2.17.2

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


[Mesa-dev] [PATCH 14/15] anv: ignore VkSamplerYcbcrConversion on non-yuv formats

2018-11-27 Thread Tapani Pälli
This fulfills a requirement for clients that want to utilize same
code path for images with external formats (VK_FORMAT_UNDEFINED) and
"regular" RGBA images where format is known. This is similar to how
OES_EGL_image_external works.

To support this, we allow color conversion samplers for non-YUV
formats but skip setting up conversion when format does not have
can_ycbcr flag set.

Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/genX_state.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c
index 42800a2581e..9b579d3118c 100644
--- a/src/intel/vulkan/genX_state.c
+++ b/src/intel/vulkan/genX_state.c
@@ -337,8 +337,11 @@ VkResult genX(CreateSampler)(
  if (conversion == NULL)
 break;
 
- sampler->n_planes = conversion->format->n_planes;
- sampler->conversion = conversion;
+ /* Setup conversion only if format is YUV format. */
+ if (conversion && conversion->format->can_ycbcr) {
+sampler->n_planes = conversion->format->n_planes;
+sampler->conversion = conversion;
+ }
  break;
   }
 #if GEN_GEN >= 9
-- 
2.17.2

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


[Mesa-dev] [PATCH 02/15] anv: refactor make_surface to use data from anv_image

2018-11-27 Thread Tapani Pälli
Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_image.c   | 78 ++
 src/intel/vulkan/anv_private.h |  5 +++
 2 files changed, 46 insertions(+), 37 deletions(-)

diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
index 50fa688c052..59e9d007831 100644
--- a/src/intel/vulkan/anv_image.c
+++ b/src/intel/vulkan/anv_image.c
@@ -159,28 +159,26 @@ add_surface(struct anv_image *image, struct anv_surface 
*surf, uint32_t plane)
 
 static bool
 all_formats_ccs_e_compatible(const struct gen_device_info *devinfo,
- const struct VkImageCreateInfo *vk_info)
+ const VkImageFormatListCreateInfoKHR *fmt_list,
+ struct anv_image *image)
 {
enum isl_format format =
-  anv_get_isl_format(devinfo, vk_info->format,
- VK_IMAGE_ASPECT_COLOR_BIT, vk_info->tiling);
+  anv_get_isl_format(devinfo, image->vk_format,
+ VK_IMAGE_ASPECT_COLOR_BIT, image->tiling);
 
if (!isl_format_supports_ccs_e(devinfo, format))
   return false;
 
-   if (!(vk_info->flags & VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT))
+   if (!(image->create_flags & VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT))
   return true;
 
-   const VkImageFormatListCreateInfoKHR *fmt_list =
-  vk_find_struct_const(vk_info->pNext, IMAGE_FORMAT_LIST_CREATE_INFO_KHR);
-
if (!fmt_list || fmt_list->viewFormatCount == 0)
   return false;
 
for (uint32_t i = 0; i < fmt_list->viewFormatCount; i++) {
   enum isl_format view_format =
  anv_get_isl_format(devinfo, fmt_list->pViewFormats[i],
-VK_IMAGE_ASPECT_COLOR_BIT, vk_info->tiling);
+VK_IMAGE_ASPECT_COLOR_BIT, image->tiling);
 
   if (!isl_formats_are_ccs_e_compatible(devinfo, format, view_format))
  return false;
@@ -299,11 +297,11 @@ add_aux_state_tracking_buffer(struct anv_image *image,
 static VkResult
 make_surface(const struct anv_device *dev,
  struct anv_image *image,
- const struct anv_image_create_info *anv_info,
+ uint32_t stride,
  isl_tiling_flags_t tiling_flags,
+ isl_surf_usage_flags_t isl_extra_usage_flags,
  VkImageAspectFlagBits aspect)
 {
-   const VkImageCreateInfo *vk_info = anv_info->vk_info;
bool ok;
 
static const enum isl_surf_dim vk_to_isl_surf_dim[] = {
@@ -312,8 +310,7 @@ make_surface(const struct anv_device *dev,
   [VK_IMAGE_TYPE_3D] = ISL_SURF_DIM_3D,
};
 
-   image->extent = anv_sanitize_image_extent(vk_info->imageType,
- vk_info->extent);
+   image->extent = anv_sanitize_image_extent(image->type, image->extent);
 
const unsigned plane = anv_image_aspect_to_plane(image->aspects, aspect);
const  struct anv_format_plane plane_format =
@@ -321,8 +318,8 @@ make_surface(const struct anv_device *dev,
struct anv_surface *anv_surf = >planes[plane].surface;
 
const isl_surf_usage_flags_t usage =
-  choose_isl_surf_usage(vk_info->flags, image->usage,
-anv_info->isl_extra_usage_flags, aspect);
+  choose_isl_surf_usage(image->create_flags, image->usage,
+isl_extra_usage_flags, aspect);
 
/* If an image is created as BLOCK_TEXEL_VIEW_COMPATIBLE, then we need to
 * fall back to linear on Broadwell and earlier because we aren't
@@ -332,24 +329,24 @@ make_surface(const struct anv_device *dev,
 */
bool needs_shadow = false;
if (dev->info.gen <= 8 &&
-   (vk_info->flags & VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT) &&
-   vk_info->tiling == VK_IMAGE_TILING_OPTIMAL) {
+   (image->create_flags & VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT) 
&&
+   image->tiling == VK_IMAGE_TILING_OPTIMAL) {
   assert(isl_format_is_compressed(plane_format.isl_format));
   tiling_flags = ISL_TILING_LINEAR_BIT;
   needs_shadow = true;
}
 
ok = isl_surf_init(>isl_dev, _surf->isl,
-  .dim = vk_to_isl_surf_dim[vk_info->imageType],
+  .dim = vk_to_isl_surf_dim[image->type],
   .format = plane_format.isl_format,
   .width = image->extent.width / plane_format.denominator_scales[0],
   .height = image->extent.height / plane_format.denominator_scales[1],
   .depth = image->extent.depth,
-  .levels = vk_info->mipLevels,
-  .array_len = vk_info->arrayLayers,
-  .samples = vk_info->samples,
+  .levels = image->levels,
+  .array_len = image->array_size,
+  .samples = image->samples,
   .min_alignment_B = 0,
-  .row_pitch_B = anv_info->stride,
+  .row_pitch_B = stride,
   .usage = usage,
   .tiling_flags = tiling_flags);
 
@@ -369,16 +366,16 @@ make_surface(const struct anv_device *dev,
   assert(tiling_flags == ISL_TILING_LINEAR_BIT);
 
   ok = isl_surf_init(>isl_dev, 
>planes[plane].shadow_surface.isl,
- .dim = 

[Mesa-dev] [PATCH 01/15] anv: add create_flags as part of anv_image

2018-11-27 Thread Tapani Pälli
This will make it possible for next patch to rip
anv_image_create_info out from make_surface function.

Signed-off-by: Tapani Pälli 
---
 src/intel/vulkan/anv_image.c   | 1 +
 src/intel/vulkan/anv_private.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
index 4003ac28444..50fa688c052 100644
--- a/src/intel/vulkan/anv_image.c
+++ b/src/intel/vulkan/anv_image.c
@@ -590,6 +590,7 @@ anv_image_create(VkDevice _device,
image->array_size = pCreateInfo->arrayLayers;
image->samples = pCreateInfo->samples;
image->usage = pCreateInfo->usage;
+   image->create_flags = pCreateInfo->flags;
image->tiling = pCreateInfo->tiling;
image->disjoint = pCreateInfo->flags & VK_IMAGE_CREATE_DISJOINT_BIT;
image->needs_set_tiling = wsi_info && wsi_info->scanout;
diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index 62c563294f5..9281479e2e8 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -2670,6 +2670,7 @@ struct anv_image {
uint32_t samples; /**< VkImageCreateInfo::samples */
uint32_t n_planes;
VkImageUsageFlags usage; /**< Superset of VkImageCreateInfo::usage. */
+   VkImageCreateFlags create_flags; /* Flags used when creating image. */
VkImageTiling tiling; /** VkImageCreateInfo::tiling */
 
/** True if this is needs to be bound to an appropriately tiled BO.
-- 
2.17.2

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


Re: [Mesa-dev] Lets talk about autotools

2018-11-27 Thread Erik Faye-Lund
On Tue, 2018-11-27 at 11:13 +0200, Timo Aaltonen wrote:
> On 17.11.2018 6.04, Dylan Baker wrote:
> > Quoting Dylan Baker (2018-09-17 09:44:07)
> > > I feel like for !windows meson is in good enough shape at this
> > > point that we
> > > can start having the discussion about deleting the autotools
> > > build. So, is there
> > > anything left that autotools can do that meson cannot (that we
> > > actually want to
> > > implement)? And, what is a reasonable time-table to remove the
> > > autotools build?
> > > I think we could reasonably remove it as soon as 18.3 if others
> > > felt confident
> > > that it would work for them.
> > > 
> > > Dylan
> > 
> > Okay, time for an update on things and a chance to talk about what
> > else we need.
> > 
> > Support for llvm-config (and any binary, actually) overriding has
> > landed in
> > meson, and will be present in the 0.49.0 release, which is due out
> > December 9th.
> 
> Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and
> will
> get Mesa backports up until and including 20.0.x, so I wonder how
> complex these required new features in meson are to be backported, or
> perhaps easily worked around? Backporting a whole new version of
> meson
> might not happen..
> 

I don't know if this is acceptable or not for packaging, but one could
always install meson from pip, I would assume...

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


Re: [Mesa-dev] [PATCH 3/3] winsys/amdgpu: use optimal VM alignment for CPU allocations

2018-11-27 Thread Christian König

Am 27.11.18 um 00:02 schrieb Marek Olšák:

From: Marek Olšák 

---
  src/gallium/winsys/amdgpu/drm/amdgpu_bo.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c 
b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
index 7b239695872..a9170a2bc69 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
@@ -1541,22 +1541,24 @@ static struct pb_buffer *amdgpu_bo_from_ptr(struct 
radeon_winsys *rws,
  
  bo = CALLOC_STRUCT(amdgpu_winsys_bo);

  if (!bo)
  return NULL;
  
  if (amdgpu_create_bo_from_user_mem(ws->dev, pointer,

 aligned_size, _handle))
  goto error;
  
  if (amdgpu_va_range_alloc(ws->dev, amdgpu_gpu_va_range_general,

-  aligned_size, 1 << 12, 0, , _handle,
- AMDGPU_VA_RANGE_HIGH))
+  aligned_size,
+  amdgpu_get_optimal_vm_alignment(ws, aligned_size,
+  
ws->info.gart_page_size),
+  0, , _handle, AMDGPU_VA_RANGE_HIGH))


For userptrs the VA alignment is most likely irrelevant because they are 
composed out of 4k pages anyway. On the other hand it shouldn't hurt to 
handle them the same way.


Feel free to add an Acked-by: Christian König  
to the series.


Christian.


  goto error_va_alloc;
  
  if (amdgpu_bo_va_op(buf_handle, 0, aligned_size, va, 0, AMDGPU_VA_OP_MAP))

  goto error_va_map;
  
  /* Initialize it. */

  pipe_reference_init(>base.reference, 1);
  bo->bo = buf_handle;
  bo->base.alignment = 0;
  bo->base.size = size;


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


Re: [Mesa-dev] [PATCH] glsl: Remove unused member variable

2018-11-27 Thread Tapani Pälli

Seems already unused at the commit that introduced it ..

Reviewed-by: Tapani Pälli 

On 11/27/18 10:10 AM, Matt Turner wrote:

---
  src/compiler/glsl/glsl_to_nir.cpp | 4 
  1 file changed, 4 deletions(-)

diff --git a/src/compiler/glsl/glsl_to_nir.cpp 
b/src/compiler/glsl/glsl_to_nir.cpp
index dc6ffa3599d..61b544fd6d3 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -96,8 +96,6 @@ private:
 /* most recent deref instruction created */
 nir_deref_instr *deref;
  
-   nir_variable *var; /* variable created by ir_variable visitor */

-
 /* whether the IR we're operating on is per-function or global */
 bool is_global;
  
@@ -179,7 +177,6 @@ nir_visitor::nir_visitor(nir_shader *shader)

_mesa_key_pointer_equal);
 this->result = NULL;
 this->impl = NULL;
-   this->var = NULL;
 memset(>b, 0, sizeof(this->b));
  }
  
@@ -453,7 +450,6 @@ nir_visitor::visit(ir_variable *ir)

nir_shader_add_variable(shader, var);
  
 _mesa_hash_table_insert(var_table, ir, var);

-   this->var = var;
  }
  
  ir_visitor_status



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


Re: [Mesa-dev] [PATCH] nir: Call fflush() at the end of nir_print_shader()

2018-11-27 Thread Tapani Pälli

Reviewed-by: Tapani Pälli 

On 11/27/18 10:08 AM, Matt Turner wrote:

We normally call with stderr which is unbuffered, so this won't affect
that, but it does let me call nir_print_shader(nir, fopen("log", "w+"))
from gdb and actually get the whole shader in my file.
---
  src/compiler/nir/nir_print.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/src/compiler/nir/nir_print.c b/src/compiler/nir/nir_print.c
index 19f26f46405..1dcf043a799 100644
--- a/src/compiler/nir/nir_print.c
+++ b/src/compiler/nir/nir_print.c
@@ -1311,6 +1311,7 @@ void
  nir_print_shader(nir_shader *shader, FILE *fp)
  {
 nir_print_shader_annotated(shader, fp, NULL);
+   fflush(fp);
  }
  
  void



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


[Mesa-dev] [PATCH] intel/compiler: fix register allocation in opt_peephole_sel

2018-11-27 Thread Iago Toral Quiroga
This wasn't handling 64-bit cases properly. Found by inspection.
---
 src/intel/compiler/brw_fs_sel_peephole.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/intel/compiler/brw_fs_sel_peephole.cpp 
b/src/intel/compiler/brw_fs_sel_peephole.cpp
index 4d11d10cc6..98d640a3bf 100644
--- a/src/intel/compiler/brw_fs_sel_peephole.cpp
+++ b/src/intel/compiler/brw_fs_sel_peephole.cpp
@@ -198,8 +198,7 @@ fs_visitor::opt_peephole_sel()
  */
 fs_reg src0(then_mov[i]->src[0]);
 if (src0.file == IMM) {
-   src0 = vgrf(glsl_type::float_type);
-   src0.type = then_mov[i]->src[0].type;
+   src0 = ibld.vgrf(then_mov[i]->src[0].type);
ibld.MOV(src0, then_mov[i]->src[0]);
 }
 
-- 
2.17.1

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


Re: [Mesa-dev] [PATCH] radv: Clamp gfx9 image view extents to the allocated image extents.

2018-11-27 Thread Alex Smith
Tested-by: Alex Smith 

Confirmed it fixes both the testcase and the in-game bug it was causing.
Thanks!

On Tue, 27 Nov 2018 at 08:34, Samuel Pitoiset 
wrote:

> cc stable?
>
> Reviewed-by: Samuel Pitoiset 
>
> On 11/24/18 11:31 PM, Bas Nieuwenhuizen wrote:
> > Mirrors AMDVLK. Looks like if we go over the alignment of height
> > we actually start to change the addressing. Seems like the extra
> > miplevels actually work with this.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108245
> > Fixes: f6cc15dccd5 "radv/gfx9: fix block compression texture views. (v2)"
> > ---
> >   src/amd/vulkan/radv_image.c | 6 ++
> >   1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c
> > index 7492bf48b51..ba8e28f0e23 100644
> > --- a/src/amd/vulkan/radv_image.c
> > +++ b/src/amd/vulkan/radv_image.c
> > @@ -1175,8 +1175,6 @@ radv_image_view_init(struct radv_image_view *iview,
> >if (device->physical_device->rad_info.chip_class >= GFX9
> &&
> >vk_format_is_compressed(image->vk_format) &&
> >!vk_format_is_compressed(iview->vk_format)) {
> > -  unsigned rounded_img_w =
> util_next_power_of_two(iview->extent.width);
> > -  unsigned rounded_img_h =
> util_next_power_of_two(iview->extent.height);
> >unsigned lvl_width  =
> radv_minify(image->info.width , range->baseMipLevel);
> >unsigned lvl_height =
> radv_minify(image->info.height, range->baseMipLevel);
> >
> > @@ -1186,8 +1184,8 @@ radv_image_view_init(struct radv_image_view *iview,
> >lvl_width <<= range->baseMipLevel;
> >lvl_height <<= range->baseMipLevel;
> >
> > -  iview->extent.width = CLAMP(lvl_width,
> iview->extent.width, rounded_img_w);
> > -  iview->extent.height = CLAMP(lvl_height,
> iview->extent.height, rounded_img_h);
> > +  iview->extent.width = CLAMP(lvl_width,
> iview->extent.width, iview->image->surface.u.gfx9.surf_pitch);
> > +  iview->extent.height = CLAMP(lvl_height,
> iview->extent.height, iview->image->surface.u.gfx9.surf_height);
> >}
> >   }
> >
> >
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 108245] RADV/Vega: Low mip levels of large BCn textures get corrupted by vkCmdCopyBufferToImage

2018-11-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=108245

Bas Nieuwenhuizen  changed:

   What|Removed |Added

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

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


Re: [Mesa-dev] [PATCH 04/11] st/mesa: Use Array._DrawVAO in st_atom_array.c.

2018-11-27 Thread Erik Faye-Lund
On Tue, 2018-11-27 at 07:11 +0100, Mathias Fröhlich wrote:
> Hi Erik,
> 
> > > On Monday, 26 November 2018 19:39:50 CET Erik Faye-Lund wrote:
> > > > I know this is *very* late notice, but this commit broke Super
> > > > Tux
> > > > Kart
> > > > on VirGL. Both the player-models as as well as the level data
> > > > renders
> > > > with gibberish vertex positions since this commit.
> > > > 
> > > > The fix that Rob Clark did on top does not fix the problem (and
> > > > shouldn't have; VirGL doesn't use NIR).
> > > 
> > > Do you have any idea how I can reproduce that issue with the
> > > least
> > > effort?
> > > 
> > 
> > Sadly, no. I run a qemu VM where I run super tux cart. It's a
> > rather
> > convoluted setup, I'm afraid. If you're interested in that Robert
> > Foss
> > has written an article about how to set something like this up
> > here:
> > https://memcpy.io/virtualizing-gpu-access.html
> > 
> > ...But I totally understand if this is asking a bit too much. I can
> > help out with any information you need...
> 
> Thanks!
> That, just means that looking into has to wait at least until the
> weekend.
> Probably even later.
> 
> And thanks for looking up the constants.
> The effective binding computation depends on these and may change
> the set up combined buffer objects. So these are interesting to know.
> 
> I have been putting a lot of internal verification into the code
> paths
> especially _mesa_update_vao_derived_arrays contains a larger
> #ifndef NDEBUG part that may tell us if there is something
> unexpected.
> 
> I assume you did run also with asserts enabled in the build?

Yes, I ran with asserts on, and none triggered.

> I can observe some flicker in supertuxcart on i965. The nvidia blob
> seems
> not to flicker here. Also when running through valgrind I don't get
> that flicker
> on i965. Is that flashing - initially looked like a lighting effect
> of the game to
> me - what you observe too?

No, the models are completely garbled. You can find some example
screenshots here:

https://gitlab.freedesktop.org/virgl/virglrenderer/issues/59

> Also what are the game options? Are shaders enabled in some way?

I'm playing with the default settings. I'm not sure what you mean with
"are shaders enabled"; VirGL is a gallium-driver, everything uses
shaders.

> What does change if you change the game settings?
> May be that gives us some hints?
> 

I've tried setting both the lowest and highest graphics settings in the
game, and I get the same problem. This seems to happen regardless of
graphics settings.

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


Re: [Mesa-dev] Lets talk about autotools

2018-11-27 Thread Timo Aaltonen
On 17.11.2018 6.04, Dylan Baker wrote:
> Quoting Dylan Baker (2018-09-17 09:44:07)
>> I feel like for !windows meson is in good enough shape at this point that we
>> can start having the discussion about deleting the autotools build. So, is 
>> there
>> anything left that autotools can do that meson cannot (that we actually want 
>> to
>> implement)? And, what is a reasonable time-table to remove the autotools 
>> build?
>> I think we could reasonably remove it as soon as 18.3 if others felt 
>> confident
>> that it would work for them.
>>
>> Dylan
> 
> Okay, time for an update on things and a chance to talk about what else we 
> need.
> 
> Support for llvm-config (and any binary, actually) overriding has landed in
> meson, and will be present in the 0.49.0 release, which is due out December 
> 9th.

Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and will
get Mesa backports up until and including 20.0.x, so I wonder how
complex these required new features in meson are to be backported, or
perhaps easily worked around? Backporting a whole new version of meson
might not happen..



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


[Mesa-dev] [Bug 106958] Mass Effect Andromeda renders correctly on RX480 POLARIS but BAD ON RX VEGA 64 on wine 3.10 stagingf with DXVK

2018-11-27 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=106958

--- Comment #14 from Samuel Pitoiset  ---
The trace crashes for me as well. Can you record a renderdoc capture instead? I
don't have that game, so can't do anything.

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


Re: [Mesa-dev] [PATCH] radv: Clamp gfx9 image view extents to the allocated image extents.

2018-11-27 Thread Samuel Pitoiset

cc stable?

Reviewed-by: Samuel Pitoiset 

On 11/24/18 11:31 PM, Bas Nieuwenhuizen wrote:

Mirrors AMDVLK. Looks like if we go over the alignment of height
we actually start to change the addressing. Seems like the extra
miplevels actually work with this.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108245
Fixes: f6cc15dccd5 "radv/gfx9: fix block compression texture views. (v2)"
---
  src/amd/vulkan/radv_image.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c
index 7492bf48b51..ba8e28f0e23 100644
--- a/src/amd/vulkan/radv_image.c
+++ b/src/amd/vulkan/radv_image.c
@@ -1175,8 +1175,6 @@ radv_image_view_init(struct radv_image_view *iview,
 if (device->physical_device->rad_info.chip_class >= GFX9 &&
 vk_format_is_compressed(image->vk_format) &&
 !vk_format_is_compressed(iview->vk_format)) {
-unsigned rounded_img_w = 
util_next_power_of_two(iview->extent.width);
-unsigned rounded_img_h = 
util_next_power_of_two(iview->extent.height);
 unsigned lvl_width  = radv_minify(image->info.width , 
range->baseMipLevel);
 unsigned lvl_height = radv_minify(image->info.height, 
range->baseMipLevel);
  
@@ -1186,8 +1184,8 @@ radv_image_view_init(struct radv_image_view *iview,

 lvl_width <<= range->baseMipLevel;
 lvl_height <<= range->baseMipLevel;
  
-			 iview->extent.width = CLAMP(lvl_width, iview->extent.width, rounded_img_w);

-iview->extent.height = CLAMP(lvl_height, 
iview->extent.height, rounded_img_h);
+iview->extent.width = CLAMP(lvl_width, iview->extent.width, 
iview->image->surface.u.gfx9.surf_pitch);
+iview->extent.height = CLAMP(lvl_height, 
iview->extent.height, iview->image->surface.u.gfx9.surf_height);
 }
}
  


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


[Mesa-dev] Please bring back __GL_FSAA_MODE

2018-11-27 Thread Tom Butler

Hello,

I realise this was removed for a reason ( 
https://lists.freedesktop.org/archives/mesa-dev/2014-September/067864.html 
) but there are cases where it is useful. In older games that do not 
support native FSAA being able to force it in the driver is the only way 
to enable it.



A quick google search for AMD linux force msaa shows that I'm not the 
only one who would like to see this feature return:


https://www.phoronix.com/forums/forum/linux-graphics-x-org-drivers/open-source-amd-linux/1024166-radeon-eqaa-anti-aliasing-support-merged-to-mesa-18-2?p=1024185#post1024185

https://bbs.archlinux.org/viewtopic.php?id=225425

https://www.reddit.com/r/linux_gaming/comments/671yzm/forcing_antialiasing_with_mesa_radeon_driver/

https://github.com/dscharrer/void/blob/master/hacks/forcemsaa.c

I understand it does cause issues in some cases but there are times when 
it is useful. Could it be reintroduced with a more relevant name that 
implies it shouldn't be used? E.g. GALLIUM_LEGACY_MSAA or 
GALLIUM_FORCE_MSAA_EXPERIMENTAL. That way it would lower users 
expectations while still making the option available to try.



Kind Regards,

Tom Butler



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


[Mesa-dev] [PATCH] glsl: Remove unused member variable

2018-11-27 Thread Matt Turner
---
 src/compiler/glsl/glsl_to_nir.cpp | 4 
 1 file changed, 4 deletions(-)

diff --git a/src/compiler/glsl/glsl_to_nir.cpp 
b/src/compiler/glsl/glsl_to_nir.cpp
index dc6ffa3599d..61b544fd6d3 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -96,8 +96,6 @@ private:
/* most recent deref instruction created */
nir_deref_instr *deref;
 
-   nir_variable *var; /* variable created by ir_variable visitor */
-
/* whether the IR we're operating on is per-function or global */
bool is_global;
 
@@ -179,7 +177,6 @@ nir_visitor::nir_visitor(nir_shader *shader)
   _mesa_key_pointer_equal);
this->result = NULL;
this->impl = NULL;
-   this->var = NULL;
memset(>b, 0, sizeof(this->b));
 }
 
@@ -453,7 +450,6 @@ nir_visitor::visit(ir_variable *ir)
   nir_shader_add_variable(shader, var);
 
_mesa_hash_table_insert(var_table, ir, var);
-   this->var = var;
 }
 
 ir_visitor_status
-- 
2.18.1

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


[Mesa-dev] [PATCH] i965/fs: Handle V/UV immediates in dump_instructions()

2018-11-27 Thread Matt Turner
---
 src/intel/compiler/brw_fs.cpp | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
index e030f7215ce..c3a09d31938 100644
--- a/src/intel/compiler/brw_fs.cpp
+++ b/src/intel/compiler/brw_fs.cpp
@@ -6036,6 +6036,11 @@ fs_visitor::dump_instruction(backend_instruction 
*be_inst, FILE *file)
 brw_vf_to_float((inst->src[i].ud >> 16) & 0xff),
 brw_vf_to_float((inst->src[i].ud >> 24) & 0xff));
 break;
+ case BRW_REGISTER_TYPE_V:
+ case BRW_REGISTER_TYPE_UV:
+fprintf(file, "%08x%s", inst->src[i].ud,
+inst->src[i].type == BRW_REGISTER_TYPE_V ? "V" : "UV");
+break;
  default:
 fprintf(file, "???");
 break;
-- 
2.18.1

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


[Mesa-dev] [PATCH] nir: Call fflush() at the end of nir_print_shader()

2018-11-27 Thread Matt Turner
We normally call with stderr which is unbuffered, so this won't affect
that, but it does let me call nir_print_shader(nir, fopen("log", "w+"))
from gdb and actually get the whole shader in my file.
---
 src/compiler/nir/nir_print.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/compiler/nir/nir_print.c b/src/compiler/nir/nir_print.c
index 19f26f46405..1dcf043a799 100644
--- a/src/compiler/nir/nir_print.c
+++ b/src/compiler/nir/nir_print.c
@@ -1311,6 +1311,7 @@ void
 nir_print_shader(nir_shader *shader, FILE *fp)
 {
nir_print_shader_annotated(shader, fp, NULL);
+   fflush(fp);
 }
 
 void
-- 
2.18.1

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