Re: [Mesa-dev] [PATCH 13/29] anv/cmd_buffer: Rework aux tracking

2018-01-13 Thread Jason Ekstrand 
On Fri, Jan 5, 2018 at 4:34 PM, Nanley Chery  wrote:

> On Mon, Nov 27, 2017 at 07:06:03PM -0800, Jason Ekstrand wrote:
> > This makes us start tracking two bits per level for aux to describe both
> > whether or not something is fast-cleared and whether or not it is
> > compressed as opposed to a single "needs resolve" bit.  The previous
> > scheme only worked because we assumed that CCS_E compressed images would
> > always be read with CCS_E enabled and so they didn't need any sort of
> > resolve if there was no fast clear color.
> >
> > The assumptions of the previous scheme held because the one case where
> > we do need a full resolve when CCS_E is enabled is for window-system
> > images.  Since we only ever allowed X-tiled window-system images, CCS
> > was entirely disabled on gen9+ and we never got CCS_E.  With the advent
> > of Y-tiled window-system buffers, we now need to properly support doing
> > a full resolve images marked CCS_E.  This requires us to track things
> > more granularly because, if the client does two back-to-back transitions
> > where we first do a partial resolve and then a full resolve, we need
> > both resolves to happen.
>
> This commit message says that this patch is necessary to do a full
> resolve with CCS_E, why is this so? Weren't the requirements fulfilled
> with the "anv/cmd_buffer: Generalize transition_color_buffer" patch
> (and my comment about disabling predication applied)?
>

Yes, we could just disable predication for full resolves on CCS_E but is
that going to lead to extra resolves?  Also, as per comments about clear
colors, this is going to have to be revised to force a resolve even if the
clear color is zero.  In particular, if we have CCS_E with a zero clear
color and we're transitioning it to VK_IMAGE_LAYOUT_PRESENT_SRC_KHR and the
image has I915_FORMAT_MOD_Y_TILED_CCS then we need to force a partial
resolve.  It's kind-of a mess and I don't see a good way to do it without
either tracking it all or having a bunch of redundant resolves.


> -Nanley
>
> > ---
> >  src/intel/vulkan/anv_private.h |  10 +--
> >  src/intel/vulkan/genX_cmd_buffer.c | 158 ++
> ---
> >  2 files changed, 135 insertions(+), 33 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
> private.h
> > index f805246..e875705 100644
> > --- a/src/intel/vulkan/anv_private.h
> > +++ b/src/intel/vulkan/anv_private.h
> > @@ -2476,16 +2476,16 @@ anv_fast_clear_state_entry_size(const struct
> anv_device *device)
> >  {
> > assert(device);
> > /* Entry contents:
> > -*   ++
> > -*   | clear value dword(s) | needs resolve dword |
> > -*   ++
> > +*   +-+
> > +*   | clear value dword(s) | needs resolve dwords |
> > +*   +-+
> >  */
> >
> > -   /* Ensure that the needs resolve dword is in fact dword-aligned to
> enable
> > +   /* Ensure that the needs resolve dwords are in fact dword-aligned to
> enable
> >  * GPU memcpy operations.
> >  */
> > assert(device->isl_dev.ss.clear_value_size % 4 == 0);
> > -   return device->isl_dev.ss.clear_value_size + 4;
> > +   return device->isl_dev.ss.clear_value_size + 8;
> >  }
> >
> >  static inline struct anv_address
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> > index 6aeffa3..2d47179 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -404,6 +404,22 @@ transition_depth_buffer(struct anv_cmd_buffer
> *cmd_buffer,
> > 0, 0, 1, hiz_op);
> >  }
> >
> > +/** Bitfield representing the needed resolves.
> > + *
> > + * This bitfield represents the kinds of compression we may or may not
> have in
> > + * a given image.  The ANV_IMAGE_NEEDS_* can be ANDed with the
> ANV_IMAGE_HAS
> > + * bits to determine when a given resolve actually needs to happen.
> > + *
> > + * For convenience of dealing with MI_PREDICATE, we blow these two bits
> out
> > + * into two dwords in the image meatadata page.
> > + */
> > +enum anv_image_resolve_bits {
> > +   ANV_IMAGE_HAS_FAST_CLEAR_BIT   = 0x1,
> > +   ANV_IMAGE_HAS_COMPRESSION_BIT  = 0x2,
> > +   ANV_IMAGE_NEEDS_PARTIAL_RESOLVE_BITS   = 0x1,
> > +   ANV_IMAGE_NEEDS_FULL_RESOLVE_BITS  = 0x3,
> > +};
> > +
> >  #define MI_PREDICATE_SRC0  0x2400
> >  #define MI_PREDICATE_SRC1  0x2408
> >
> > @@ -411,23 +427,62 @@ transition_depth_buffer(struct anv_cmd_buffer
> *cmd_buffer,
> >   * performed properly.
> >   */
> >  static void
> > -set_image_needs_resolve(struct anv_cmd_buffer *cmd_buffer,
> > -const struct anv_image *image,
> > -VkImageAspectFlagBits aspect,
> > -unsigned level, bool needs_resolve)
> >

Re: [Mesa-dev] [PATCH] ac: remove ac_shader_variant_info::fs::output_mask

2018-01-13 Thread Timothy Arceri 

Reviewed-by: Timothy Arceri 

On 13/01/18 00:21, Samuel Pitoiset wrote:

Unused.

Signed-off-by: Samuel Pitoiset 
---
  src/amd/common/ac_nir_to_llvm.c | 2 --
  src/amd/common/ac_nir_to_llvm.h | 1 -
  2 files changed, 3 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 90856933fd..7dff5fcaf8 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -6379,8 +6379,6 @@ handle_fs_outputs_post(struct nir_to_llvm_context *ctx)
si_export_mrt_color(ctx, NULL, V_008DFC_SQ_EXP_NULL, true, 
_args[0]);
ac_build_export(>ac, _args[0]);
}
-
-   ctx->shader_info->fs.output_mask = index ? ((1ull << index) - 1) : 0;
  }
  
  static void

diff --git a/src/amd/common/ac_nir_to_llvm.h b/src/amd/common/ac_nir_to_llvm.h
index ca48cd7c07..111e41e549 100644
--- a/src/amd/common/ac_nir_to_llvm.h
+++ b/src/amd/common/ac_nir_to_llvm.h
@@ -169,7 +169,6 @@ struct ac_shader_variant_info {
struct {
unsigned num_interp;
uint32_t input_mask;
-   unsigned output_mask;
uint32_t flat_shaded_mask;
bool can_discard;
bool writes_z;


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

[Mesa-dev] [PATCH 2/2] radv: Use the CS bo for other uploads too.

2018-01-13 Thread Bas Nieuwenhuizen 
If an app keeps building and resetting the command buffers, most
command buffers will  have 1 CS bo and 1 upload bo. This merges them
by allocating uploads from the end of the CS bo.

On dota2, this goes from ~700 to ~400 BOs after the merging in the
create_bo_list. Sadly no discernible performance difference though.
The create_bo_list CPU usage went down by about half, but looks
like this thread is not a bottleneck.

We still need to keep the old path for SI where we don't write the
CS to a BO immegiately.
---
 src/amd/vulkan/radv_cmd_buffer.c  | 11 +
 src/amd/vulkan/radv_cs.h  |  2 +-
 src/amd/vulkan/radv_radeon_winsys.h   | 15 --
 src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 67 ++-
 4 files changed, 58 insertions(+), 37 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 6dfae4d5e3..7df7928180 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -367,6 +367,17 @@ radv_cmd_buffer_upload_alloc(struct radv_cmd_buffer 
*cmd_buffer,
 unsigned *out_offset,
 void **ptr)
 {
+   if (cmd_buffer->cs->bo) {
+   radeon_check_space(cmd_buffer->device->ws, cmd_buffer->cs, size 
+ alignment);
+   cmd_buffer->cs->max_dw -= size;
+   cmd_buffer->cs->max_dw &= ~(MAX2(alignment >> 2, 1u) - 1u);
+
+   *out_offset = cmd_buffer->cs->max_dw * 4;
+   *ptr = cmd_buffer->cs->buf + cmd_buffer->cs->max_dw;
+   *bo = cmd_buffer->cs->bo;
+   return true;
+   }
+
uint64_t offset = align(cmd_buffer->upload.offset, alignment);
if (offset + size > cmd_buffer->upload.size) {
if (!radv_cmd_buffer_resize_upload_buf(cmd_buffer, size))
diff --git a/src/amd/vulkan/radv_cs.h b/src/amd/vulkan/radv_cs.h
index 840597686a..9ca5d41b1d 100644
--- a/src/amd/vulkan/radv_cs.h
+++ b/src/amd/vulkan/radv_cs.h
@@ -34,7 +34,7 @@ static inline unsigned radeon_check_space(struct 
radeon_winsys *ws,
   struct radeon_winsys_cs *cs,
   unsigned needed)
 {
-if (cs->max_dw - cs->cdw < needed)
+if (cs->cdw + cs->reserved_dw + needed >= cs->max_dw)
 ws->cs_grow(cs, needed);
 return cs->cdw + needed;
 }
diff --git a/src/amd/vulkan/radv_radeon_winsys.h 
b/src/amd/vulkan/radv_radeon_winsys.h
index 341e40505c..a1b2b011f0 100644
--- a/src/amd/vulkan/radv_radeon_winsys.h
+++ b/src/amd/vulkan/radv_radeon_winsys.h
@@ -94,10 +94,19 @@ enum radeon_value_id {
RADEON_CURRENT_MCLK,
 };
 
+struct radeon_winsys_bo {
+   uint64_t va;
+   bool is_local;
+};
+
 struct radeon_winsys_cs {
unsigned cdw;  /* Number of used dwords. */
-   unsigned max_dw; /* Maximum number of dwords. */
+   unsigned max_dw; /* Maximum number of dwords. With a combined upload 
buf, this is the start of the upload buf. */
uint32_t *buf; /* The base pointer of the chunk. */
+
+   /* For having a combined CS / upload buffer */
+   struct radeon_winsys_bo *bo; /* the bo backing the CS if there is one */
+   uint32_t reserved_dw; /* Required buffer space between the end of the 
CS and the start of the upload data. */
 };
 
 #define RADEON_SURF_TYPE_MASK   0xFF
@@ -159,10 +168,6 @@ struct radeon_bo_metadata {
 uint32_t syncobj_handle;
 struct radeon_winsys_fence;
 
-struct radeon_winsys_bo {
-   uint64_t va;
-   bool is_local;
-};
 struct radv_winsys_sem_counts {
uint32_t syncobj_count;
uint32_t sem_count;
diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c 
b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
index 0ee56f9144..2d103e2b09 100644
--- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
+++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
@@ -44,7 +44,8 @@ struct radv_amdgpu_cs {
 
struct amdgpu_cs_ib_infoib;
 
-   struct radeon_winsys_bo *ib_buffer;
+   uint32_tib_size;
+
uint8_t *ib_mapped;
unsignedmax_num_buffers;
unsignednum_buffers;
@@ -158,8 +159,8 @@ static void radv_amdgpu_cs_destroy(struct radeon_winsys_cs 
*rcs)
 {
struct radv_amdgpu_cs *cs = radv_amdgpu_cs(rcs);
 
-   if (cs->ib_buffer)
-   cs->ws->base.buffer_destroy(cs->ib_buffer);
+   if (cs->base.bo)
+   cs->ws->base.buffer_destroy(cs->base.bo);
else
free(cs->base.buf);
 
@@ -199,32 +200,34 @@ radv_amdgpu_cs_create(struct radeon_winsys *ws,
radv_amdgpu_init_cs(cs, ring_type);
 
if (cs->ws->use_ib_bos) {
-   cs->ib_buffer = ws->buffer_create(ws, ib_size, 0,
+   cs->base.bo = ws->buffer_create(ws, ib_size, 0,
  RADEON_DOMAIN_GTT,

[Mesa-dev] [PATCH 1/2] radv: Add bo argument to upload functions.

2018-01-13 Thread Bas Nieuwenhuizen 
---
 src/amd/vulkan/radv_cmd_buffer.c  | 37 +++--
 src/amd/vulkan/radv_meta_buffer.c |  5 +++--
 src/amd/vulkan/radv_private.h |  6 --
 src/amd/vulkan/si_cmd_buffer.c|  5 +++--
 4 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 67799a13cc..6dfae4d5e3 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -297,10 +297,11 @@ radv_reset_cmd_buffer(struct radv_cmd_buffer *cmd_buffer)
 
if (cmd_buffer->device->physical_device->rad_info.chip_class >= GFX9) {
void *fence_ptr;
+   struct radeon_winsys_bo *upload_bo;
radv_cmd_buffer_upload_alloc(cmd_buffer, 8, 0,
-_buffer->gfx9_fence_offset,
+_bo, 
_buffer->gfx9_fence_offset,
 _ptr);
-   cmd_buffer->gfx9_fence_bo = cmd_buffer->upload.upload_bo;
+   cmd_buffer->gfx9_fence_bo = upload_bo;
}
 
cmd_buffer->status = RADV_CMD_BUFFER_STATUS_INITIAL;
@@ -362,6 +363,7 @@ bool
 radv_cmd_buffer_upload_alloc(struct radv_cmd_buffer *cmd_buffer,
 unsigned size,
 unsigned alignment,
+struct radeon_winsys_bo **bo,
 unsigned *out_offset,
 void **ptr)
 {
@@ -374,6 +376,7 @@ radv_cmd_buffer_upload_alloc(struct radv_cmd_buffer 
*cmd_buffer,
 
*out_offset = offset;
*ptr = cmd_buffer->upload.map + offset;
+   *bo = cmd_buffer->upload.upload_bo;
 
cmd_buffer->upload.offset = offset + size;
return true;
@@ -382,12 +385,13 @@ radv_cmd_buffer_upload_alloc(struct radv_cmd_buffer 
*cmd_buffer,
 bool
 radv_cmd_buffer_upload_data(struct radv_cmd_buffer *cmd_buffer,
unsigned size, unsigned alignment,
-   const void *data, unsigned *out_offset)
+   const void *data, struct radeon_winsys_bo **bo,
+   unsigned *out_offset)
 {
uint8_t *ptr;
 
if (!radv_cmd_buffer_upload_alloc(cmd_buffer, size, alignment,
- out_offset, (void **)))
+ bo, out_offset, (void **)))
return false;
 
if (ptr)
@@ -1709,13 +1713,14 @@ radv_flush_push_descriptors(struct radv_cmd_buffer 
*cmd_buffer)
 {
struct radv_descriptor_set *set = _buffer->push_descriptors.set;
unsigned bo_offset;
+   struct radeon_winsys_bo *bo;
 
if (!radv_cmd_buffer_upload_data(cmd_buffer, set->size, 32,
 set->mapped_ptr,
-_offset))
+, _offset))
return;
 
-   set->va = radv_buffer_get_va(cmd_buffer->upload.upload_bo);
+   set->va = radv_buffer_get_va(bo);
set->va += bo_offset;
 }
 
@@ -1725,9 +1730,10 @@ radv_flush_indirect_descriptor_sets(struct 
radv_cmd_buffer *cmd_buffer)
uint32_t size = MAX_SETS * 2 * 4;
uint32_t offset;
void *ptr;
+   struct radeon_winsys_bo *bo;

if (!radv_cmd_buffer_upload_alloc(cmd_buffer, size,
- 256, , ))
+ 256, , , ))
return;
 
for (unsigned i = 0; i < MAX_SETS; i++) {
@@ -1740,7 +1746,7 @@ radv_flush_indirect_descriptor_sets(struct 
radv_cmd_buffer *cmd_buffer)
uptr[1] = set_va >> 32;
}
 
-   uint64_t va = radv_buffer_get_va(cmd_buffer->upload.upload_bo);
+   uint64_t va = radv_buffer_get_va(bo);
va += offset;
 
if (cmd_buffer->state.pipeline) {
@@ -1822,16 +1828,17 @@ radv_flush_constants(struct radv_cmd_buffer *cmd_buffer,
(!layout->push_constant_size && !layout->dynamic_offset_count))
return;
 
+   struct radeon_winsys_bo *upload_bo;
if (!radv_cmd_buffer_upload_alloc(cmd_buffer, 
layout->push_constant_size +
  16 * layout->dynamic_offset_count,
- 256, , ))
+ 256, _bo, , ))
return;
 
memcpy(ptr, cmd_buffer->push_constants, layout->push_constant_size);
memcpy((char*)ptr + layout->push_constant_size, 
cmd_buffer->dynamic_buffers,
   16 * layout->dynamic_offset_count);
 
-   va = radv_buffer_get_va(cmd_buffer->upload.upload_bo);
+   va = radv_buffer_get_va(upload_bo);
va += offset;
 
MAYBE_UNUSED unsigned cdw_max = 
radeon_check_space(cmd_buffer->device->ws,
@@ -1861,10 +1868,11 @@ radv_cmd_buffer_update_vertex_descriptors(struct 
radv_cmd_buffer *cmd_buffer,

Re: [Mesa-dev] [PATCH 2/2] ac: add doubles support to isign

2018-01-13 Thread Bas Nieuwenhuizen 
Reviewed-by: Bas Nieuwenhuizen 

for the series.

On Sun, Jan 14, 2018 at 12:09 AM, Timothy Arceri  wrote:
> Fixes a number of int64 piglit tests, for example:
>
> generated_tests/spec/arb_gpu_shader_int64/execution/built-in-functions/fs-sign-i64vec2.shader_test
> ---
>  src/amd/common/ac_nir_to_llvm.c | 25 ++---
>  1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
> index 04ffbedfb3..e79fdd2ec2 100644
> --- a/src/amd/common/ac_nir_to_llvm.c
> +++ b/src/amd/common/ac_nir_to_llvm.c
> @@ -1362,14 +1362,25 @@ static LLVMValueRef emit_fsign(struct ac_llvm_context 
> *ctx,
>  }
>
>  static LLVMValueRef emit_isign(struct ac_llvm_context *ctx,
> -  LLVMValueRef src0)
> +  LLVMValueRef src0, unsigned bitsize)
>  {
> -   LLVMValueRef cmp, val;
> +   LLVMValueRef cmp, val, zero, one;
> +   LLVMTypeRef type;
> +
> +   if (bitsize == 32) {
> +   type = ctx->i32;
> +   zero = ctx->i32_0;
> +   one = ctx->i32_1;
> +   } else {
> +   type = ctx->i64;
> +   zero = ctx->i64_0;
> +   one = ctx->i64_1;
> +   }
>
> -   cmp = LLVMBuildICmp(ctx->builder, LLVMIntSGT, src0, ctx->i32_0, "");
> -   val = LLVMBuildSelect(ctx->builder, cmp, ctx->i32_1, src0, "");
> -   cmp = LLVMBuildICmp(ctx->builder, LLVMIntSGE, val, ctx->i32_0, "");
> -   val = LLVMBuildSelect(ctx->builder, cmp, val, LLVMConstInt(ctx->i32, 
> -1, true), "");
> +   cmp = LLVMBuildICmp(ctx->builder, LLVMIntSGT, src0, zero, "");
> +   val = LLVMBuildSelect(ctx->builder, cmp, one, src0, "");
> +   cmp = LLVMBuildICmp(ctx->builder, LLVMIntSGE, val, zero, "");
> +   val = LLVMBuildSelect(ctx->builder, cmp, val, LLVMConstInt(type, -1, 
> true), "");
> return val;
>  }
>
> @@ -1813,7 +1824,7 @@ static void visit_alu(struct ac_nir_context *ctx, const 
> nir_alu_instr *instr)
> result = emit_minmax_int(>ac, LLVMIntULT, src[0], 
> src[1]);
> break;
> case nir_op_isign:
> -   result = emit_isign(>ac, src[0]);
> +   result = emit_isign(>ac, src[0], 
> instr->dest.dest.ssa.bit_size);
> break;
> case nir_op_fsign:
> src[0] = ac_to_float(>ac, src[0]);
> --
> 2.14.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] ac: add doubles support to isign

2018-01-13 Thread Timothy Arceri 
Fixes a number of int64 piglit tests, for example:

generated_tests/spec/arb_gpu_shader_int64/execution/built-in-functions/fs-sign-i64vec2.shader_test
---
 src/amd/common/ac_nir_to_llvm.c | 25 ++---
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 04ffbedfb3..e79fdd2ec2 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -1362,14 +1362,25 @@ static LLVMValueRef emit_fsign(struct ac_llvm_context 
*ctx,
 }
 
 static LLVMValueRef emit_isign(struct ac_llvm_context *ctx,
-  LLVMValueRef src0)
+  LLVMValueRef src0, unsigned bitsize)
 {
-   LLVMValueRef cmp, val;
+   LLVMValueRef cmp, val, zero, one;
+   LLVMTypeRef type;
+
+   if (bitsize == 32) {
+   type = ctx->i32;
+   zero = ctx->i32_0;
+   one = ctx->i32_1;
+   } else {
+   type = ctx->i64;
+   zero = ctx->i64_0;
+   one = ctx->i64_1;
+   }
 
-   cmp = LLVMBuildICmp(ctx->builder, LLVMIntSGT, src0, ctx->i32_0, "");
-   val = LLVMBuildSelect(ctx->builder, cmp, ctx->i32_1, src0, "");
-   cmp = LLVMBuildICmp(ctx->builder, LLVMIntSGE, val, ctx->i32_0, "");
-   val = LLVMBuildSelect(ctx->builder, cmp, val, LLVMConstInt(ctx->i32, 
-1, true), "");
+   cmp = LLVMBuildICmp(ctx->builder, LLVMIntSGT, src0, zero, "");
+   val = LLVMBuildSelect(ctx->builder, cmp, one, src0, "");
+   cmp = LLVMBuildICmp(ctx->builder, LLVMIntSGE, val, zero, "");
+   val = LLVMBuildSelect(ctx->builder, cmp, val, LLVMConstInt(type, -1, 
true), "");
return val;
 }
 
@@ -1813,7 +1824,7 @@ static void visit_alu(struct ac_nir_context *ctx, const 
nir_alu_instr *instr)
result = emit_minmax_int(>ac, LLVMIntULT, src[0], src[1]);
break;
case nir_op_isign:
-   result = emit_isign(>ac, src[0]);
+   result = emit_isign(>ac, src[0], 
instr->dest.dest.ssa.bit_size);
break;
case nir_op_fsign:
src[0] = ac_to_float(>ac, src[0]);
-- 
2.14.3

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

[Mesa-dev] [PATCH 1/2] ac: add i64_0 and i64_1 to llvm build context

2018-01-13 Thread Timothy Arceri 
These will be used in the following patch.
---
 src/amd/common/ac_llvm_build.c | 2 ++
 src/amd/common/ac_llvm_build.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c
index f0a1788eaf..3467bba693 100644
--- a/src/amd/common/ac_llvm_build.c
+++ b/src/amd/common/ac_llvm_build.c
@@ -76,6 +76,8 @@ ac_llvm_context_init(struct ac_llvm_context *ctx, 
LLVMContextRef context,
 
ctx->i32_0 = LLVMConstInt(ctx->i32, 0, false);
ctx->i32_1 = LLVMConstInt(ctx->i32, 1, false);
+   ctx->i64_0 = LLVMConstInt(ctx->i64, 0, false);
+   ctx->i64_1 = LLVMConstInt(ctx->i64, 1, false);
ctx->f32_0 = LLVMConstReal(ctx->f32, 0.0);
ctx->f32_1 = LLVMConstReal(ctx->f32, 1.0);
ctx->f64_0 = LLVMConstReal(ctx->f64, 0.0);
diff --git a/src/amd/common/ac_llvm_build.h b/src/amd/common/ac_llvm_build.h
index 78322bbf01..f87889daf6 100644
--- a/src/amd/common/ac_llvm_build.h
+++ b/src/amd/common/ac_llvm_build.h
@@ -61,6 +61,8 @@ struct ac_llvm_context {
 
LLVMValueRef i32_0;
LLVMValueRef i32_1;
+   LLVMValueRef i64_0;
+   LLVMValueRef i64_1;
LLVMValueRef f32_0;
LLVMValueRef f32_1;
LLVMValueRef f64_0;
-- 
2.14.3

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

Re: [Mesa-dev] [PATCH v2] ac/nir: fix translation of nir_op_b2i for doubles

2018-01-13 Thread Bas Nieuwenhuizen 
Reviewed-by: Bas Nieuwenhuizen 

On Sat, Jan 13, 2018 at 11:52 PM, Timothy Arceri  wrote:
> V2: just zero-extend the 32-bit value.
>
> Fixes a number of int64 piglet tests, for example:
>
> generated_tests/spec/arb_gpu_shader_int64/execution/conversion/frag-conversion-explicit-bool-int64_t.shader_test
> ---
>  src/amd/common/ac_nir_to_llvm.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
> index 1cc44c5cae..04ffbedfb3 100644
> --- a/src/amd/common/ac_nir_to_llvm.c
> +++ b/src/amd/common/ac_nir_to_llvm.c
> @@ -1422,9 +1422,15 @@ static LLVMValueRef emit_f2b(struct ac_llvm_context 
> *ctx,
>  }
>
>  static LLVMValueRef emit_b2i(struct ac_llvm_context *ctx,
> -LLVMValueRef src0)
> +LLVMValueRef src0,
> +unsigned bitsize)
>  {
> -   return LLVMBuildAnd(ctx->builder, src0, ctx->i32_1, "");
> +   LLVMValueRef result = LLVMBuildAnd(ctx->builder, src0, ctx->i32_1, 
> "");
> +
> +   if (bitsize == 32)
> +   return result;
> +
> +   return LLVMBuildZExt(ctx->builder, result, ctx->i64, "");
>  }
>
>  static LLVMValueRef emit_i2b(struct ac_llvm_context *ctx,
> @@ -1979,7 +1985,7 @@ static void visit_alu(struct ac_nir_context *ctx, const 
> nir_alu_instr *instr)
> result = emit_f2b(>ac, src[0]);
> break;
> case nir_op_b2i:
> -   result = emit_b2i(>ac, src[0]);
> +   result = emit_b2i(>ac, src[0], 
> instr->dest.dest.ssa.bit_size);
> break;
> case nir_op_i2b:
> src[0] = ac_to_integer(>ac, src[0]);
> --
> 2.14.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2] ac/nir: fix translation of nir_op_b2i for doubles

2018-01-13 Thread Timothy Arceri 
V2: just zero-extend the 32-bit value.

Fixes a number of int64 piglet tests, for example:

generated_tests/spec/arb_gpu_shader_int64/execution/conversion/frag-conversion-explicit-bool-int64_t.shader_test
---
 src/amd/common/ac_nir_to_llvm.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 1cc44c5cae..04ffbedfb3 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -1422,9 +1422,15 @@ static LLVMValueRef emit_f2b(struct ac_llvm_context *ctx,
 }
 
 static LLVMValueRef emit_b2i(struct ac_llvm_context *ctx,
-LLVMValueRef src0)
+LLVMValueRef src0,
+unsigned bitsize)
 {
-   return LLVMBuildAnd(ctx->builder, src0, ctx->i32_1, "");
+   LLVMValueRef result = LLVMBuildAnd(ctx->builder, src0, ctx->i32_1, "");
+
+   if (bitsize == 32)
+   return result;
+
+   return LLVMBuildZExt(ctx->builder, result, ctx->i64, "");
 }
 
 static LLVMValueRef emit_i2b(struct ac_llvm_context *ctx,
@@ -1979,7 +1985,7 @@ static void visit_alu(struct ac_nir_context *ctx, const 
nir_alu_instr *instr)
result = emit_f2b(>ac, src[0]);
break;
case nir_op_b2i:
-   result = emit_b2i(>ac, src[0]);
+   result = emit_b2i(>ac, src[0], 
instr->dest.dest.ssa.bit_size);
break;
case nir_op_i2b:
src[0] = ac_to_integer(>ac, src[0]);
-- 
2.14.3

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

Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op

2018-01-13 Thread Jason Ekstrand 

On January 12, 2018 14:56:26 "Ian Romanick"  wrote:


From: Ian Romanick 

With the exception of NVIDIA hardware, these are is the values that all
hardware and Gallium want.  The remapping is currently implemented in at
least 6 places.  This starts the process of consolidating to a single
place.

Signed-off-by: Ian Romanick 
---
 src/mesa/main/blend.c  | 22 ++
 src/mesa/main/mtypes.h | 29 +
 2 files changed, 51 insertions(+)

diff --git a/src/mesa/main/blend.c b/src/mesa/main/blend.c
index 01721ab..f47b102 100644
--- a/src/mesa/main/blend.c
+++ b/src/mesa/main/blend.c
@@ -849,6 +849,26 @@ _mesa_AlphaFunc( GLenum func, GLclampf ref )
}
 }

+static const enum color_logic_ops color_logicop_mapping[16] = {
+   COLOR_LOGICOP_CLEAR,
+   COLOR_LOGICOP_AND,
+   COLOR_LOGICOP_AND_REVERSE,
+   COLOR_LOGICOP_COPY,
+   COLOR_LOGICOP_AND_INVERTED,
+   COLOR_LOGICOP_NOOP,
+   COLOR_LOGICOP_XOR,
+   COLOR_LOGICOP_OR,
+   COLOR_LOGICOP_NOR,
+   COLOR_LOGICOP_EQUIV,
+   COLOR_LOGICOP_INVERT,
+   COLOR_LOGICOP_OR_REVERSE,
+   COLOR_LOGICOP_COPY_INVERTED,
+   COLOR_LOGICOP_OR_INVERTED,
+   COLOR_LOGICOP_NAND,
+   COLOR_LOGICOP_SET
+};
+
+#define GLenum_to_color_logicop(x) color_logicop_mapping[x & 0x0f]


Would it be better to make this a static inline so we can add a little 
range check assert?




 static ALWAYS_INLINE void
 logic_op(struct gl_context *ctx, GLenum opcode, bool no_error)
@@ -884,6 +904,7 @@ logic_op(struct gl_context *ctx, GLenum opcode, bool 
no_error)

FLUSH_VERTICES(ctx, ctx->DriverFlags.NewLogicOp ? 0 : _NEW_COLOR);
ctx->NewDriverState |= ctx->DriverFlags.NewLogicOp;
ctx->Color.LogicOp = opcode;
+   ctx->Color._LogicOp = GLenum_to_color_logicop(opcode);

if (ctx->Driver.LogicOpcode)
   ctx->Driver.LogicOpcode(ctx, opcode);
@@ -1189,6 +1210,7 @@ void _mesa_init_color( struct gl_context * ctx )
ctx->Color.IndexLogicOpEnabled = GL_FALSE;
ctx->Color.ColorLogicOpEnabled = GL_FALSE;
ctx->Color.LogicOp = GL_COPY;
+   ctx->Color._LogicOp = COLOR_LOGICOP_COPY;
ctx->Color.DitherFlag = GL_TRUE;

/* GL_FRONT is not possible on GLES. Instead GL_BACK will render to either
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 226eb94..2fbfd27 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -418,6 +418,34 @@ union gl_color_union
GLuint ui[4];
 };

+/**
+ * Remapped color logical operations
+ *
+ * With the exception of NVIDIA hardware, which consumes the OpenGL enumerants
+ * directly, everything wants this mapping of color logical operations.
+ *
+ * Fun fact: These values are just the bit-reverse of the low-nibble of the GL
+ * enumerant values (i.e., `GL_NOOP & 0x0f` is `b0101' while
+ * \c COLOR_LOGICOP_NOOP is `b1010`).
+ */
+enum PACKED color_logic_ops {
+   COLOR_LOGICOP_CLEAR = 0,
+   COLOR_LOGICOP_NOR = 1,
+   COLOR_LOGICOP_AND_INVERTED = 2,
+   COLOR_LOGICOP_COPY_INVERTED = 3,
+   COLOR_LOGICOP_AND_REVERSE = 4,
+   COLOR_LOGICOP_INVERT = 5,
+   COLOR_LOGICOP_XOR = 6,
+   COLOR_LOGICOP_NAND = 7,
+   COLOR_LOGICOP_AND = 8,
+   COLOR_LOGICOP_EQUIV = 9,
+   COLOR_LOGICOP_NOOP = 10,
+   COLOR_LOGICOP_OR_INVERTED = 11,
+   COLOR_LOGICOP_COPY = 12,
+   COLOR_LOGICOP_OR_REVERSE = 13,
+   COLOR_LOGICOP_OR = 14,
+   COLOR_LOGICOP_SET = 15
+};

 /**
  * Color buffer attribute group (GL_COLOR_BUFFER_BIT).
@@ -493,6 +521,7 @@ struct gl_colorbuffer_attrib
GLboolean IndexLogicOpEnabled;  /**< Color index logic op enabled flag 
*/
GLboolean ColorLogicOpEnabled;  /**< RGBA logic op enabled flag */
GLenum LogicOp; /**< Logic operator */
+   enum color_logic_ops _LogicOp;

/*@}*/

--
2.9.5

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



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

[Mesa-dev] [Bug 104214] Dota crashes when switching from game to desktop

2018-01-13 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=104214

--- Comment #54 from Jason Ekstrand  ---
(In reply to Thomas Hellström from comment #53)
> So IMHO we should try to write piglit tests for the areas where we know
> there are remaining issues.

That's a reasonable thing to say.  However, our DRI testing in piglit is rather
pitiful across the board IMHO.  Part of the reason why you had to fix 6
different bugs in order to turn on DRI3 is that we've been lazy about testing
when working on the DRI code.  Window system stuff is annoyingly full of
edge-cases and the piglit tests we have tend to only touch-test things.  This
is only one of the 3 or 4 DRI bugs we've shipped in the last few months that
piglit has been perfectly happy with and then it messes up users badly.

While I would love to give someone the general task of improving piglit testing
of DRI, a good place to start is to write tests for known bugs.  If we have a
really nasty threading bug, let's make a test that hammers on threading.

Ok, I've said my piece.  Maybe I'm being unreasonable but it sounds good in my
head. :-)

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

Re: [Mesa-dev] [PATCH 06/29] anv/image: Add a helper for determining when fast clears are supported

2018-01-13 Thread Jason Ekstrand 
Sorry for all the list spam, but I'm sort of thinking out-loud and writing
it on the list for all to read.

I'm thinking that what we want this list to return is not a bool but an enum

/* The ordering of this enum is important */
enum anv_fast_clear_support {
   ANV_FAST_CLEAR_NONE = 0,
   ANV_FAST_CLEAR_ZERO_ONLY = 1,
   ANV_FAST_CLEAR_ANY = 2,
};

And then the predicate for whether or not to do the resolve becomes
(has_compression && !supports_compression) || (image_fast_clear >
supported_fast_clear).  I still haven't quite figured out what to do with
MI_PREDICATE to make this work out.  I'm sure it's possible with MI math
but maybe I can come up with something clever that doesn't require that.
In the worst case, we only have to deal with this complexity on gen9+ where
we have MI_MATH so it'll be ok.

Does this seem like a reasonable plan?

On Fri, Jan 12, 2018 at 5:23 PM, Jason Ekstrand 
wrote:

> I made a table to help visualize all the different cases:
>
> Layout  | compression | zero clear | non-zero clear
> +=++
> GENERAL |  N  |  N |   N
> +-++
> COLOR_ATTACHMENT|  Y  |  Y |   Y
> +-++
> SHADER_READ_ONLY|  Y  |  Y |   N
> +-++
> GENERAL (RT/tex/blit only)  |  Y  |  Y |   N
> +-++
> PRESENT_SRC (Y_TILED)   |  N  |  N |   N
> +-++
> PRESENT_SRC (Y_TILED_CCS)   |  Y  |  N |   N
> +-++
>
> We will need to think about what to do with this.  Having non-zero clears
> be different from zero clears is tricky.  We may need to have even more
> bits in our aux tracking. :-/
>
> On January 12, 2018 16:05:12 Jason Ekstrand  wrote:
>
>> On Wed, Dec 13, 2017 at 11:26 AM, Nanley Chery 
>> wrote:
>>
>>> On Mon, Nov 27, 2017 at 07:05:56PM -0800, Jason Ekstrand wrote:
>>> > ---
>>> >  src/intel/vulkan/anv_image.c   | 58 ++
>>> 
>>> >  src/intel/vulkan/anv_private.h |  5 
>>> >  2 files changed, 63 insertions(+)
>>> >
>>> > diff --git a/src/intel/vulkan/anv_image.c
>>> b/src/intel/vulkan/anv_image..c
>>>
>>> > index a872149..561da28 100644
>>> > --- a/src/intel/vulkan/anv_image.c
>>> > +++ b/src/intel/vulkan/anv_image.c
>>> > @@ -837,6 +837,64 @@ anv_layout_to_aux_usage(const struct
>>> gen_device_info * const devinfo,
>>> > unreachable("layout is not a VkImageLayout enumeration member.");
>>> >  }
>>> >
>>> > +/**
>>> > + * This function returns true if the given image in the given
>>> VkImageLayout
>>> > + * supports unresolved fast-clears.
>>> > + *
>>> > + * @param devinfo The device information of the Intel GPU.
>>> > + * @param image The image that may contain a collection of buffers.
>>> > + * @param aspect The aspect of the image to be accessed.
>>> > + * @param layout The current layout of the image aspect(s).
>>> > + */
>>> > +bool
>>> > +anv_layout_supports_fast_clear(const struct gen_device_info * const
>>> devinfo,
>>> > +   const struct anv_image * const image,
>>> > +   const VkImageAspectFlagBits aspect,
>>> > +   const VkImageLayout layout)
>>> > +{
>>> > +   /* The aspect must be exactly one of the image aspects. */
>>> > +   assert(_mesa_bitcount(aspect) == 1 && (aspect & image->aspects));
>>> > +
>>> > +   uint32_t plane = anv_image_aspect_to_plane(image->aspects,
>>> aspect);
>>> > +
>>> > +   /* If there is no auxiliary surface allocated, there are no
>>> fast-clears */
>>> > +   if (image->planes[plane].aux_surface.isl.size == 0)
>>> > +  return false;
>>> > +
>>> > +   /* All images that use an auxiliary surface are required to be
>>> tiled. */
>>> > +   assert(image->tiling == VK_IMAGE_TILING_OPTIMAL);
>>> > +
>>> > +   /* Stencil has no aux */
>>> > +   assert(aspect != VK_IMAGE_ASPECT_STENCIL_BIT);
>>> > +
>>> > +   if (aspect == VK_IMAGE_ASPECT_DEPTH_BIT) {
>>> > +  /* For depth images (with HiZ), the layout supports fast-clears
>>> if and
>>> > +   * only if it supports HiZ.
>>> > +   */
>>> > +  enum isl_aux_usage aux_usage =
>>> > + anv_layout_to_aux_usage(devinfo, image, aspect, layout);
>>> > +  return aux_usage == ISL_AUX_USAGE_HIZ;
>>> > +   }
>>> > +
>>> > +   assert(image->aspects & VK_IMAGE_ASPECT_ANY_COLOR_BIT_ANV);
>>> > +
>>> > +   /* Multisample fast-clear is not yet supported. */
>>> > +   if

Re: [Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2018-01-13 Thread Jason Ekstrand 
On Sat, Jan 13, 2018 at 9:55 AM, Jason Ekstrand 
wrote:

> On Wed, Dec 13, 2017 at 4:46 PM, Nanley Chery 
> wrote:
>
>> On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
>> > Currently, this helper does nothing but we call it every place where an
>> > image is written through the render pipeline.  This will allow us to
>> > properly mark the aux state so that we can handle resolves correctly.
>> > ---
>> >  src/intel/vulkan/anv_blorp.c   | 36 ++
>> ++
>> >  src/intel/vulkan/anv_cmd_buffer.c  | 12 
>> >  src/intel/vulkan/anv_genX.h|  6 ++
>> >  src/intel/vulkan/anv_private.h |  7 +++
>> >  src/intel/vulkan/genX_cmd_buffer.c | 17 +
>> >  5 files changed, 78 insertions(+)
>> >
>> > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
>> > index da273d6..46e2eb0 100644
>> > --- a/src/intel/vulkan/anv_blorp.c
>> > +++ b/src/intel/vulkan/anv_blorp.c
>> > @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
>> >
>> >assert(anv_image_aspects_compatible(src_mask, dst_mask));
>> >
>> > +  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
>> >if (_mesa_bitcount(src_mask) > 1) {
>> >   uint32_t aspect_bit;
>> >   anv_foreach_image_aspect_bit(aspect_bit, src_image,
>> src_mask) {
>> > @@ -281,6 +282,10 @@ void anv_CmdCopyImage(
>> >  get_blorp_surf_for_anv_image(cmd_buffer->device,
>> >   dst_image, 1UL << aspect_bit,
>> >   ANV_AUX_USAGE_DEFAULT,
>> _surf);
>> > +anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
>> > +  1UL << aspect_bit,
>> > +  dst_surf.aux_usage,
>> > +  dst_level);
>> >
>> >  for (unsigned i = 0; i < layer_count; i++) {
>> > blorp_copy(, _surf,
>> pRegions[r].srcSubresource.mipLevel,
>> > @@ -298,6 +303,8 @@ void anv_CmdCopyImage(
>> >ANV_AUX_USAGE_DEFAULT,
>> _surf);
>> >   get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image,
>> dst_mask,
>> >ANV_AUX_USAGE_DEFAULT,
>> _surf);
>> > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
>> dst_mask,
>> > +   dst_surf.aux_usage,
>> dst_level);
>> >
>> >   for (unsigned i = 0; i < layer_count; i++) {
>> >  blorp_copy(, _surf,
>> pRegions[r].srcSubresource.mipLevel,
>> > @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer
>> *cmd_buffer,
>> >  extent.width, extent.height,
>> >  buffer_row_pitch, buffer_format,
>> >  , _isl_surf);
>> > +  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {
>> > + assert(dst == );
>> > + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
>> > +   aspect, dst->surf.aux_usage,
>> > +   dst->level);
>> > +  }
>> >
>> >for (unsigned z = 0; z < extent.depth; z++) {
>> >   blorp_copy(, >surf, src->level, src->offset.z,
>> > @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
>> >get_blorp_surf_for_anv_image(cmd_buffer->device,
>> > dst_image, dst_res->aspectMask,
>> > ANV_AUX_USAGE_DEFAULT, );
>> > +  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
>> > +dst_res->aspectMask,
>> > +dst.aux_usage,
>> > +dst_res->mipLevel);
>> >
>> >struct anv_format_plane src_format =
>> >   anv_get_format_plane(_buffer->device->info,
>> src_image->vk_format,
>> > @@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
>> >  layer_count = anv_minify(image->extent.depth, level);
>> >   }
>> >
>> > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
>> > +   pRanges[r].aspectMask,
>> > +   surf.aux_usage, level);
>> > +
>> >   blorp_clear(, ,
>> >   src_format.isl_format, src_format.swizzle,
>> >   level, base_layer, layer_count,
>> > @@ -1215,6 +1236,11 @@ anv_cmd_buffer_clear_subpass(struct
>> anv_cmd_buffer *cmd_buffer)
>> >  ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT |
>> ANV_PIPE_CS_STALL_BIT;
>> >} else {
>> >   assert(image->n_planes == 1);
>> > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
>> > +   VK_IMAGE_ASPECT_COLOR_BIT,

Re: [Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2018-01-13 Thread Jason Ekstrand 
On Thu, Jan 11, 2018 at 5:14 PM, Nanley Chery  wrote:

> On Thu, Nov 30, 2017 at 06:20:51PM +0200, Pohjolainen, Topi wrote:
> > On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
> > > Currently, this helper does nothing but we call it every place where an
> > > image is written through the render pipeline.  This will allow us to
> > > properly mark the aux state so that we can handle resolves correctly.
> > > ---
> > >  src/intel/vulkan/anv_blorp.c   | 36 ++
> ++
> > >  src/intel/vulkan/anv_cmd_buffer.c  | 12 
> > >  src/intel/vulkan/anv_genX.h|  6 ++
> > >  src/intel/vulkan/anv_private.h |  7 +++
> > >  src/intel/vulkan/genX_cmd_buffer.c | 17 +
> > >  5 files changed, 78 insertions(+)
> > >
> > > diff --git a/src/intel/vulkan/anv_blorp.c
> b/src/intel/vulkan/anv_blorp.c
> > > index da273d6..46e2eb0 100644
> > > --- a/src/intel/vulkan/anv_blorp.c
> > > +++ b/src/intel/vulkan/anv_blorp.c
> > > @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
> > >
> > >assert(anv_image_aspects_compatible(src_mask, dst_mask));
> > >
> > > +  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
> > >if (_mesa_bitcount(src_mask) > 1) {
> > >   uint32_t aspect_bit;
> > >   anv_foreach_image_aspect_bit(aspect_bit, src_image,
> src_mask) {
> > > @@ -281,6 +282,10 @@ void anv_CmdCopyImage(
> > >  get_blorp_surf_for_anv_image(cmd_buffer->device,
> > >   dst_image, 1UL << aspect_bit,
> > >   ANV_AUX_USAGE_DEFAULT,
> _surf);
> > > +anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > > +  1UL << aspect_bit,
> > > +  dst_surf.aux_usage,
> > > +  dst_level);
> > >
> > >  for (unsigned i = 0; i < layer_count; i++) {
> > > blorp_copy(, _surf,
> pRegions[r].srcSubresource.mipLevel,
> > > @@ -298,6 +303,8 @@ void anv_CmdCopyImage(
> > >ANV_AUX_USAGE_DEFAULT,
> _surf);
> > >   get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image,
> dst_mask,
> > >ANV_AUX_USAGE_DEFAULT,
> _surf);
> > > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> dst_mask,
> > > +   dst_surf.aux_usage,
> dst_level);
> > >
> > >   for (unsigned i = 0; i < layer_count; i++) {
> > >  blorp_copy(, _surf, pRegions[r].srcSubresource.
> mipLevel,
> > > @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer
> *cmd_buffer,
> > >  extent.width, extent.height,
> > >  buffer_row_pitch, buffer_format,
> > >  , _isl_surf);
> > > +  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {
> >
> > In all the other call sites you call anv_cmd_buffer_mark_image_written()
> > regardless if aux usage is none. Is there something special here?
> >
>
> Here we need to call anv_cmd_buffer_mark_image_written() when
> buffer_to_image == true, so there must be an if condition. I think this
> if condition is more direct compared to the alternatives of:
>* if (buffer_to_image)
>* if (dst == image)
>

I know I had some reason but I don't remember exactly what it was.  I think
it was probably an artifact of history combined with me being overly
clever.  I've switched it to the second of Nanley's suggestions.


> -Nanley
>
> > > + assert(dst == );
> > > + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
> > > +   aspect,
> dst->surf.aux_usage,
> > > +   dst->level);
> > > +  }
> > >
> > >for (unsigned z = 0; z < extent.depth; z++) {
> > >   blorp_copy(, >surf, src->level, src->offset.z,
> > > @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
> > >get_blorp_surf_for_anv_image(cmd_buffer->device,
> > > dst_image, dst_res->aspectMask,
> > > ANV_AUX_USAGE_DEFAULT, );
> > > +  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > > +dst_res->aspectMask,
> > > +dst.aux_usage,
> > > +dst_res->mipLevel);
> > >
> > >struct anv_format_plane src_format =
> > >   anv_get_format_plane(_buffer->device->info,
> src_image->vk_format,
> > > @@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
> > >  layer_count = anv_minify(image->extent.depth, level);
> > >   }
> > >
> > > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> > > +

Re: [Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2018-01-13 Thread Jason Ekstrand 
On Wed, Dec 13, 2017 at 4:46 PM, Nanley Chery  wrote:

> On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
> > Currently, this helper does nothing but we call it every place where an
> > image is written through the render pipeline.  This will allow us to
> > properly mark the aux state so that we can handle resolves correctly.
> > ---
> >  src/intel/vulkan/anv_blorp.c   | 36 ++
> ++
> >  src/intel/vulkan/anv_cmd_buffer.c  | 12 
> >  src/intel/vulkan/anv_genX.h|  6 ++
> >  src/intel/vulkan/anv_private.h |  7 +++
> >  src/intel/vulkan/genX_cmd_buffer.c | 17 +
> >  5 files changed, 78 insertions(+)
> >
> > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> > index da273d6..46e2eb0 100644
> > --- a/src/intel/vulkan/anv_blorp.c
> > +++ b/src/intel/vulkan/anv_blorp.c
> > @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
> >
> >assert(anv_image_aspects_compatible(src_mask, dst_mask));
> >
> > +  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
> >if (_mesa_bitcount(src_mask) > 1) {
> >   uint32_t aspect_bit;
> >   anv_foreach_image_aspect_bit(aspect_bit, src_image, src_mask)
> {
> > @@ -281,6 +282,10 @@ void anv_CmdCopyImage(
> >  get_blorp_surf_for_anv_image(cmd_buffer->device,
> >   dst_image, 1UL << aspect_bit,
> >   ANV_AUX_USAGE_DEFAULT,
> _surf);
> > +anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > +  1UL << aspect_bit,
> > +  dst_surf.aux_usage,
> > +  dst_level);
> >
> >  for (unsigned i = 0; i < layer_count; i++) {
> > blorp_copy(, _surf, pRegions[r].srcSubresource.
> mipLevel,
> > @@ -298,6 +303,8 @@ void anv_CmdCopyImage(
> >ANV_AUX_USAGE_DEFAULT, _surf);
> >   get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image,
> dst_mask,
> >ANV_AUX_USAGE_DEFAULT, _surf);
> > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> dst_mask,
> > +   dst_surf.aux_usage,
> dst_level);
> >
> >   for (unsigned i = 0; i < layer_count; i++) {
> >  blorp_copy(, _surf, pRegions[r].srcSubresource.
> mipLevel,
> > @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer
> *cmd_buffer,
> >  extent.width, extent.height,
> >  buffer_row_pitch, buffer_format,
> >  , _isl_surf);
> > +  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {
> > + assert(dst == );
> > + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
> > +   aspect, dst->surf.aux_usage,
> > +   dst->level);
> > +  }
> >
> >for (unsigned z = 0; z < extent.depth; z++) {
> >   blorp_copy(, >surf, src->level, src->offset.z,
> > @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
> >get_blorp_surf_for_anv_image(cmd_buffer->device,
> > dst_image, dst_res->aspectMask,
> > ANV_AUX_USAGE_DEFAULT, );
> > +  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > +dst_res->aspectMask,
> > +dst.aux_usage,
> > +dst_res->mipLevel);
> >
> >struct anv_format_plane src_format =
> >   anv_get_format_plane(_buffer->device->info,
> src_image->vk_format,
> > @@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
> >  layer_count = anv_minify(image->extent.depth, level);
> >   }
> >
> > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> > +   pRanges[r].aspectMask,
> > +   surf.aux_usage, level);
> > +
> >   blorp_clear(, ,
> >   src_format.isl_format, src_format.swizzle,
> >   level, base_layer, layer_count,
> > @@ -1215,6 +1236,11 @@ anv_cmd_buffer_clear_subpass(struct
> anv_cmd_buffer *cmd_buffer)
> >  ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT |
> ANV_PIPE_CS_STALL_BIT;
> >} else {
> >   assert(image->n_planes == 1);
> > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> > +   VK_IMAGE_ASPECT_COLOR_BIT,
> > +   att_state->aux_usage,
> > +   iview->planes[0].isl.base_
> level);
> > +
> >

Re: [Mesa-dev] [PATCH 4/7] report.py: Add option to only display the final summary

2018-01-13 Thread Dylan Baker 
Quoting Ian Romanick (2018-01-12 12:06:58)
> From: Ian Romanick 
> 
> This is useful for preparing data to go in a Mesa commit message.
> 
> Signed-off-by: Ian Romanick 
> ---
>  report.py | 60 
>  1 file changed, 32 insertions(+), 28 deletions(-)
> 
> diff --git a/report.py b/report.py
> index 003a1ee..e0068bc 100755
> --- a/report.py
> +++ b/report.py
> @@ -60,6 +60,8 @@ def main():
>  parser.add_argument("--measurements", "-m", type=split_list,
>  default=["instructions", "cycles", "loops", 
> "spills", "fills"],
>  help="comma-separated list of measurements to 
> report")
> +parser.add_argument("--summary-only", "-s", action="store_true", 
> default=False,

It doesn't really matter, but you don't need the default here, store_true
defaults to false.

> +help="do not show the per-shader helped / hurt data")
>  parser.add_argument("before", type=get_results, help="the output of the 
> original code")
>  parser.add_argument("after", type=get_results, help="the output of the 
> new code")
>  args = parser.parse_args()
> @@ -104,23 +106,24 @@ def main():
>  else:
>  helped.append(p)
>  
> -helped.sort(
> -key=lambda k: args.after[k][m] if args.before[k][m] == 0 else 
> float(args.before[k][m] - args.after[k][m]) / args.before[k][m])
> -for p in helped:
> -namestr = p[0] + " " + p[1]
> -print(m + " helped:   " + get_result_string(
> -namestr, args.before[p][m], args.after[p][m]))
> -if len(helped) > 0:
> -print("")
> -
> -hurt.sort(
> -key=lambda k: args.after[k][m] if args.before[k][m] == 0 else 
> float(args.after[k][m] - args.before[k][m]) / args.before[k][m])
> -for p in hurt:
> -namestr = p[0] + " " + p[1]
> -print(m + " HURT:   " + get_result_string(
> -namestr, args.before[p][m], args.after[p][m]))
> -if len(hurt) > 0:
> -print("")
> +if not args.summary_only:
> +helped.sort(
> +key=lambda k: args.after[k][m] if args.before[k][m] == 0 
> else float(args.before[k][m] - args.after[k][m]) / args.before[k][m])
> +for p in helped:
> +namestr = p[0] + " " + p[1]
> +print(m + " helped:   " + get_result_string(
> +namestr, args.before[p][m], args.after[p][m]))
> +if len(helped) > 0:
> +print("")
> +
> +hurt.sort(
> +key=lambda k: args.after[k][m] if args.before[k][m] == 0 
> else float(args.after[k][m] - args.before[k][m]) / args.before[k][m])
> +for p in hurt:
> +namestr = p[0] + " " + p[1]
> +print(m + " HURT:   " + get_result_string(
> +namestr, args.before[p][m], args.after[p][m]))
> +if len(hurt) > 0:
> +print("")
>  
>  num_helped[m] = len(helped)
>  num_hurt[m] = len(hurt)
> @@ -137,17 +140,18 @@ def main():
>  if args.before.get(p) is None:
>  gained.append(p[0] + " " + p[1])
>  
> -lost.sort()
> -for p in lost:
> -print("LOST:   " + p)
> -if len(lost) > 0:
> -print("")
> -
> -gained.sort()
> -for p in gained:
> -print("GAINED: " + p)
> -if len(gained) > 0:
> -print("")
> +if not args.summary_only:
> +lost.sort()
> +for p in lost:
> +print("LOST:   " + p)
> +if len(lost) > 0:
> +print("")
> +
> +gained.sort()
> +for p in gained:
> +print("GAINED: " + p)
> +if len(gained) > 0:
> +print("")
>  
>  for m in args.measurements:
>  print("total {0} in shared programs: {1}\n"
> -- 
> 2.9.5
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

[Mesa-dev] [PATCH 2/2] meson: set the minimum version correctly

2018-01-13 Thread Dylan Baker 
Currently we ask for 0.42, but we actually require 0.43 because we pass
file objects as arguments to tests. If someone needs version 0.42 it
wouldn't be hard, just a lot of replacing files() with strings.

Signed-off-by: Dylan Baker 
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 0556608..d7a50cf 100644
--- a/meson.build
+++ b/meson.build
@@ -23,7 +23,7 @@ project(
   ['c'],
   version : '2.4.89',
   license : 'MIT',
-  meson_version : '>= 0.42',
+  meson_version : '>= 0.43',
   default_options : ['buildtype=debugoptimized', 'c_std=gnu99'],
 )
 
-- 
git-series 0.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] meson: set proper pkg-config version for libdrm_freedreno

2018-01-13 Thread Dylan Baker 
Copy and paste error from exynos.

Signed-off-by: Dylan Baker 
---
 freedreno/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/freedreno/meson.build b/freedreno/meson.build
index b4035e1..de6a413 100644
--- a/freedreno/meson.build
+++ b/freedreno/meson.build
@@ -64,7 +64,7 @@ pkg.generate(
   name : 'libdrm_freedreno',
   libraries : libdrm_freedreno,
   subdirs : ['.', 'libdrm', 'freedreno'],
-  version : '0.7',
+  version : meson.project_version(),
   requires_private : 'libdrm',
   description : 'Userspace interface to freedreno kernel DRM services',
 )
-- 
git-series 0.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/7] report.py: Add option to only display measurements that have changes

2018-01-13 Thread Dylan Baker 
Quoting Ian Romanick (2018-01-12 12:06:59)
> From: Ian Romanick 
> 
> This is useful for preparing data to go in a Mesa commit message.
> 
> Signed-off-by: Ian Romanick 
> ---
>  report.py | 53 +++--
>  1 file changed, 31 insertions(+), 22 deletions(-)
> 
> diff --git a/report.py b/report.py
> index e0068bc..72752c1 100755
> --- a/report.py
> +++ b/report.py
> @@ -62,6 +62,8 @@ def main():
>  help="comma-separated list of measurements to 
> report")
>  parser.add_argument("--summary-only", "-s", action="store_true", 
> default=False,
>  help="do not show the per-shader helped / hurt data")
> +parser.add_argument("--changes-only", "-c", action="store_true", 
> default=False,
> +help="only show measurements that have changes")
>  parser.add_argument("before", type=get_results, help="the output of the 
> original code")
>  parser.add_argument("after", type=get_results, help="the output of the 
> new code")
>  args = parser.parse_args()
> @@ -116,14 +118,14 @@ def main():
>  if len(helped) > 0:
>  print("")
>  
> -hurt.sort(
> -key=lambda k: args.after[k][m] if args.before[k][m] == 0 
> else float(args.after[k][m] - args.before[k][m]) / args.before[k][m])
> -for p in hurt:
> -namestr = p[0] + " " + p[1]
> -print(m + " HURT:   " + get_result_string(
> -namestr, args.before[p][m], args.after[p][m]))
> -if len(hurt) > 0:
> -print("")
> +hurt.sort(
> +key=lambda k: args.after[k][m] if args.before[k][m] == 0 
> else float(args.after[k][m] - args.before[k][m]) / args.before[k][m])
> +for p in hurt:
> +namestr = p[0] + " " + p[1]
> +print(m + " HURT:   " + get_result_string(
> +namestr, args.before[p][m], args.after[p][m]))
> +if len(hurt) > 0:
> +print("")
>  
>  num_helped[m] = len(helped)
>  num_hurt[m] = len(hurt)
> @@ -153,21 +155,28 @@ def main():
>  if len(gained) > 0:
>  print("")
>  
> +any_helped_or_hurt = False
>  for m in args.measurements:
> -print("total {0} in shared programs: {1}\n"
> -  "{0} in affected programs: {2}\n"
> -  "helped: {3}\n"
> -  "HURT: {4}\n".format(
> -   m,
> -   change(total_before[m], total_after[m]),
> -   change(affected_before[m], affected_after[m]),
> -   num_helped[m],
> -   num_hurt[m]))
> -
> -
> -print("LOST:   " + str(len(lost)))
> -print("GAINED: " + str(len(gained)))
> -
> +if num_helped[m] > 0 or num_hurt[m] > 0:
> +any_helped_or_hurt = True
> +
> +if num_helped[m] > 0 or num_hurt[m] > 0 or not args.changes_only:

Couldn't this be: `if any_helped_or_hurt or not args.changes_only:`

> +print("total {0} in shared programs: {1}\n"
> +  "{0} in affected programs: {2}\n"
> +  "helped: {3}\n"
> +  "HURT: {4}\n".format(
> + m,
> + change(total_before[m], total_after[m]),
> + change(affected_before[m], affected_after[m]),
> + num_helped[m],
> + num_hurt[m]))
> +
> +if len(lost) > 0 or len(gained) > 0 or not args.changes_only:

Don't use len() for checking if a list has members:
`if lost or gained or not args.changes_only:`

> +print("LOST:   " + str(len(lost)))
> +print("GAINED: " + str(len(gained)))
> +
> +if args.changes_only and len(lost) == 0 and len(gained) == 0 and not 
> any_helped_or_hurt:

same here:
`if args.changes_only and not lost and not gained and not any_helped_or_hurt`:

> +print("No changes.")
>  
>  if __name__ == "__main__":
>  main()
> -- 
> 2.9.5
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

[Mesa-dev] [PATCH 0/2] Small fixes for the meson build

2018-01-13 Thread Dylan Baker 
Here's a few things I've caught as I've started trying to add the meson
build to our CI system.

Dylan Baker (2):
  meson: set proper pkg-config version for libdrm_freedreno
  meson: set the minimum version correctly

 freedreno/meson.build | 2 +-
 meson.build   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

base-commit: fd9bcb73e9c5a01085069b37c2f5e04300a9b4d4
-- 
git-series 0.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] meson: Fix configuring dri glx with only gallium drivers

2018-01-13 Thread Dylan Baker 
meson considers classic swrast to be a dri driver, I know it's not exactly
accurate, but, at least for me, it made the build system easier to reason about.

Quoting Adam Jackson (2018-01-12 09:06:37)
> On Fri, 2018-01-12 at 13:18 +, Jon Turney wrote:
> > 'meson -Ddri-drivers= -Dgallium-drivers=swrast -Dglx=dri' fails with 'dri
> > based GLX requires at least one DRI driver'
> > 
> > Signed-off-by: Jon Turney 
> > ---
> >  meson.build | 2 +-
> >  src/glx/meson.build | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/meson.build b/meson.build
> > index 77e4e894b23..dd8e6145edb 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -323,7 +323,7 @@ if with_glx != 'disabled'
> >  if with_dri
> >error('xlib conflicts with any dri driver')
> >  endif
> > -  elif with_glx == 'dri' and not with_dri
> > +  elif with_glx == 'dri' and not (with_dri or with_gallium)
> >  error('dri based GLX requires at least one DRI driver')
> 
> We should just remove this check. libGL doesn't actually require a DRI
> driver, and at least on OSX there's no DRI driver you could possibly
> build.
> 
> - ajax
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

Re: [Mesa-dev] [PATCH 1/2] meson: Fix configuring dri glx with only gallium drivers

2018-01-13 Thread Dylan Baker 
Maybe this is correct, but it makes me nervous treating with_gallium as
equivalent to with_dri, since gallium drivers can be built dri-less
(gallium-xlib, and some other configurations on windows). I think something
like:

  with_glx = get_option('glx')
  if with_glx == 'auto'
if with_dri
  with_glx = 'dri'
elif with_gallium
  # Even when building just gallium drivers the user probably wants dri
  with_glx = 'dri'
  with_dri = true
elif with_platform_x11 and with_any_opengl and not with_any_vk
  # The automatic behavior should not be to turn on xlib based glx when
  # building only vulkan drivers
  with_glx = 'xlib'
else
  with_glx = 'disabled'
endif
+ elif with_glx == 'dri'
+   if with_gallium
+ with_dri = true
+   endif
  endif


Would achieve the correct result, be simpler, and avoid accidentally adding dri
sources when we shouldn't.

Dylan

Quoting Jon Turney (2018-01-12 05:18:35)
> 'meson -Ddri-drivers= -Dgallium-drivers=swrast -Dglx=dri' fails with 'dri
> based GLX requires at least one DRI driver'
> 
> Signed-off-by: Jon Turney 
> ---
>  meson.build | 2 +-
>  src/glx/meson.build | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 77e4e894b23..dd8e6145edb 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -323,7 +323,7 @@ if with_glx != 'disabled'
>  if with_dri
>error('xlib conflicts with any dri driver')
>  endif
> -  elif with_glx == 'dri' and not with_dri
> +  elif with_glx == 'dri' and not (with_dri or with_gallium)
>  error('dri based GLX requires at least one DRI driver')
>endif
>  endif
> diff --git a/src/glx/meson.build b/src/glx/meson.build
> index cdb388e9837..ead6e6138f7 100644
> --- a/src/glx/meson.build
> +++ b/src/glx/meson.build
> @@ -65,7 +65,7 @@ extra_libs_libglx = []
>  extra_deps_libgl = []
>  extra_ld_args_libgl = []
>  
> -if with_dri
> +if with_dri or with_gallium
>files_libglx += files(
>  'dri_common.c',
>  'dri_common.h',
> -- 
> 2.15.1
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

Re: [Mesa-dev] [Test Patch] Meson: ensure vdpau has proper symbols exposed

2018-01-13 Thread haagch+mesadev 
This patch makes vdpau work here too on RX 480.

vaapi has the same problem. I tried doing an analog patch and also had to set
libva_version = ['1', '0', '0'] in src/gallium/state_trackers/va/meson.build
because libva on arch only tries to call __vaDriverInit_1_0 on arch.
The autotools makefile uses pkg-config which doesn't output 2.3 either:
$ pkg-config --modversion libva 
 :(
1.0.0

But then actually using vaapi for encoding or decoding gets into a
weird assertion:
mpv: ../src/gallium/drivers/radeon/radeon_winsys.h:773: radeon_get_heap_index: 
Assertion `!"READ_ONLY without WC is disallowed"' failed.

omx is also broken:
OMX-the library /usr/lib/bellagio/libomx_mesa.so is not compatible with ST 
static component loader - /usr/lib/bellagio/libomx_mesa.so: undefined symbol: 
omx_component_library_Setup

Someone with radeon/radeonsi should really look over these build files.

On 11.01.2018 18:24, Dylan Baker wrote:
> Signed-off-by: Dylan Baker 
> ---
>  src/gallium/targets/vdpau/meson.build | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/targets/vdpau/meson.build 
> b/src/gallium/targets/vdpau/meson.build
> index 67f1469fb0f..432a32a7340 100644
> --- a/src/gallium/targets/vdpau/meson.build
> +++ b/src/gallium/targets/vdpau/meson.build
> @@ -23,6 +23,7 @@
>  # configure.ac)
>  
>  vdpau_link_args = []
> +vdpau_link_with = []
>  vdpau_link_depends = []
>  vdpau_drivers = []
>  
> @@ -35,6 +36,13 @@ if with_ld_dynamic_list
>vdpau_link_depends += files('../dri-vdpau.dyn')
>  endif
>  
> +if with_dri
> +  vdpau_link_with += libswdri
> +endif
> +if with_gallium_drisw_kms
> +  vdpau_link_with += libswkmsdri
> +endif
> +
>  libvdpau_gallium = shared_library(
>'vdpau_gallium',
>'target.c',
> @@ -44,12 +52,14 @@ libvdpau_gallium = shared_library(
>include_directories : [
>  inc_common, inc_util, inc_gallium_winsys, inc_gallium_drivers,
>],
> +  link_whole : [libvdpau_st],
>link_with : [
> -libvdpau_st, libgalliumvlwinsys, libgalliumvl, libgallium, libmesa_util,
> -libpipe_loader_static, libws_null, libwsw,
> +libgalliumvlwinsys, libgalliumvl, libgallium, libmesa_util,
> +libpipe_loader_static, libws_null, libwsw, vdpau_link_with,
>],
>dependencies : [
> -dep_thread, dep_xcb, dep_x11_xcb, dep_xcb_dri2, dep_libdrm,
> +dep_thread, dep_xcb, dep_x11_xcb, dep_xcb_dri2, dep_xcb_dri3,
> +dep_xcb_present, dep_xshmfence, dep_xcb_xfixes, dep_xcb_sync, dep_libdrm,
>  driver_r300, driver_r600, driver_radeonsi, driver_nouveau,
>],
>link_depends : vdpau_link_depends,


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

Re: [Mesa-dev] [Test Patch] Meson: ensure vdpau has proper symbolsexposed

2018-01-13 Thread Marc Dietrich 
Hi Dylan,


Am Donnerstag, 11. Januar 2018, 18:24:42 CET schrieb Dylan Baker:
> Signed-off-by: Dylan Baker 
> ---
>  src/gallium/targets/vdpau/meson.build | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)


this makes the symbol (and a few more) available here. The whole linking stuff 
looks very dubious to me. This might also explain some of the size differences 
between autotools and meson builds, e.g. meson always uses "-Wl,O1", whatever 
this does or means. I didn't found a place where to change this default? 
setting. 

Anyway, thanks for tracking this down!

Marc

> 
> diff --git a/src/gallium/targets/vdpau/meson.build
> b/src/gallium/targets/vdpau/meson.build index 67f1469fb0f..432a32a7340
> 100644
> --- a/src/gallium/targets/vdpau/meson.build
> +++ b/src/gallium/targets/vdpau/meson.build
> @@ -23,6 +23,7 @@
>  # configure.ac)
> 
>  vdpau_link_args = []
> +vdpau_link_with = []
>  vdpau_link_depends = []
>  vdpau_drivers = []
> 
> @@ -35,6 +36,13 @@ if with_ld_dynamic_list
>vdpau_link_depends += files('../dri-vdpau.dyn')
>  endif
> 
> +if with_dri
> +  vdpau_link_with += libswdri
> +endif
> +if with_gallium_drisw_kms
> +  vdpau_link_with += libswkmsdri
> +endif
> +
>  libvdpau_gallium = shared_library(
>'vdpau_gallium',
>'target.c',
> @@ -44,12 +52,14 @@ libvdpau_gallium = shared_library(
>include_directories : [
>  inc_common, inc_util, inc_gallium_winsys, inc_gallium_drivers,
>],
> +  link_whole : [libvdpau_st],
>link_with : [
> -libvdpau_st, libgalliumvlwinsys, libgalliumvl, libgallium,
> libmesa_util, -libpipe_loader_static, libws_null, libwsw,
> +libgalliumvlwinsys, libgalliumvl, libgallium, libmesa_util,
> +libpipe_loader_static, libws_null, libwsw, vdpau_link_with,
>],
>dependencies : [
> -dep_thread, dep_xcb, dep_x11_xcb, dep_xcb_dri2, dep_libdrm,
> +dep_thread, dep_xcb, dep_x11_xcb, dep_xcb_dri2, dep_xcb_dri3,
> +dep_xcb_present, dep_xshmfence, dep_xcb_xfixes, dep_xcb_sync,
> dep_libdrm, driver_r300, driver_r600, driver_radeonsi, driver_nouveau,
>],
>link_depends : vdpau_link_depends,



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

[Mesa-dev] [Bug 104214] Dota crashes when switching from game to desktop

2018-01-13 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=104214

--- Comment #53 from Thomas Hellström  ---
(In reply to Jason Ekstrand from comment #51)
> Can we write a piglit test or two that reproduces this bug?  It would be
> very good if had a nice self-contained test that we can run in CI and avoid
> these types of issues in the future.

As a comment to this, the origin of this bug is in itself an interesting
sequence of events that might in one way answer your question:

1) Implemented and enabled dri3 support in the vmware Xorg driver.
2) A lot of glretrace automated tests started failing.
3) Fixed / worked around a number of viewport issues in the mesa dri3 core.
3a) Fixed a number of synchronization issues in the mesa dri3 core.
4) Some glretrace automated tests still failing.
5) Implemented dri3 SWAP_COPY_OML to have glretrace mimic the capturing
platform behaviour (which was in some cases WGL)
6) SWAP_COPY_OML didn't work due to a long standing bug in mesa core dri.
7) Fixed that. glretrace started to render transparently with SWAP_COPY_OML
fbconfigs.
8) This was caused by a long standing issue in the Xorg GLX layer. Worked
around that.
9) Kwin and other applications started having transparent rendering issues.
10) Implemented a proper fix for 8)
11) Item 3) was reported to cause a multi-threaded game to fail to start on
some platforms with page-flip only. Turned out mesa core dri3 is not
thread-safe. Implemented a game-specific workaround.
12) This bug surfaced (was caused by 5)) and was fixed in a way that will
decrease the chances of it ever happening again. (Perhaps an extra comment in
the source is needed).

So why am I listing this? The reason is that I agree we need more piglit tests
to verify GLX and dri3 functionality, but while this bug probably has had the
strongest user impact, that doesn't necessarily mean it's likely to happen
again. I think the biggest problem with the mesa dri3 implementation currently
is that it was written without multi-threading in mind, and adding
thread-safety as a hindsight might prove difficult and error-prone. It's also
currently lacking a well-documented and well defined strategy to handle
drawable size-changes and viewport changes.

So IMHO we should try to write piglit tests for the areas where we know there
are remaining issues.

/Thomas

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

Re: [Mesa-dev] [PATCH 11/29] anv/cmd_buffer: Add a mark_image_written helper

2018-01-13 Thread Pohjolainen, Topi 
On Thu, Jan 11, 2018 at 05:14:57PM -0800, Nanley Chery wrote:
> On Thu, Nov 30, 2017 at 06:20:51PM +0200, Pohjolainen, Topi wrote:
> > On Mon, Nov 27, 2017 at 07:06:01PM -0800, Jason Ekstrand wrote:
> > > Currently, this helper does nothing but we call it every place where an
> > > image is written through the render pipeline.  This will allow us to
> > > properly mark the aux state so that we can handle resolves correctly.
> > > ---
> > >  src/intel/vulkan/anv_blorp.c   | 36 
> > > 
> > >  src/intel/vulkan/anv_cmd_buffer.c  | 12 
> > >  src/intel/vulkan/anv_genX.h|  6 ++
> > >  src/intel/vulkan/anv_private.h |  7 +++
> > >  src/intel/vulkan/genX_cmd_buffer.c | 17 +
> > >  5 files changed, 78 insertions(+)
> > > 
> > > diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> > > index da273d6..46e2eb0 100644
> > > --- a/src/intel/vulkan/anv_blorp.c
> > > +++ b/src/intel/vulkan/anv_blorp.c
> > > @@ -271,6 +271,7 @@ void anv_CmdCopyImage(
> > >  
> > >assert(anv_image_aspects_compatible(src_mask, dst_mask));
> > >  
> > > +  const uint32_t dst_level = pRegions[r].dstSubresource.mipLevel;
> > >if (_mesa_bitcount(src_mask) > 1) {
> > >   uint32_t aspect_bit;
> > >   anv_foreach_image_aspect_bit(aspect_bit, src_image, src_mask) {
> > > @@ -281,6 +282,10 @@ void anv_CmdCopyImage(
> > >  get_blorp_surf_for_anv_image(cmd_buffer->device,
> > >   dst_image, 1UL << aspect_bit,
> > >   ANV_AUX_USAGE_DEFAULT, 
> > > _surf);
> > > +anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > > +  1UL << aspect_bit,
> > > +  dst_surf.aux_usage,
> > > +  dst_level);
> > >  
> > >  for (unsigned i = 0; i < layer_count; i++) {
> > > blorp_copy(, _surf, 
> > > pRegions[r].srcSubresource.mipLevel,
> > > @@ -298,6 +303,8 @@ void anv_CmdCopyImage(
> > >ANV_AUX_USAGE_DEFAULT, _surf);
> > >   get_blorp_surf_for_anv_image(cmd_buffer->device, dst_image, 
> > > dst_mask,
> > >ANV_AUX_USAGE_DEFAULT, _surf);
> > > + anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image, 
> > > dst_mask,
> > > +   dst_surf.aux_usage, 
> > > dst_level);
> > >  
> > >   for (unsigned i = 0; i < layer_count; i++) {
> > >  blorp_copy(, _surf, 
> > > pRegions[r].srcSubresource.mipLevel,
> > > @@ -387,6 +394,12 @@ copy_buffer_to_image(struct anv_cmd_buffer 
> > > *cmd_buffer,
> > >  extent.width, extent.height,
> > >  buffer_row_pitch, buffer_format,
> > >  , _isl_surf);
> > > +  if (dst->surf.aux_usage != ISL_AUX_USAGE_NONE) {
> > 
> > In all the other call sites you call anv_cmd_buffer_mark_image_written()
> > regardless if aux usage is none. Is there something special here?
> > 
> 
> Here we need to call anv_cmd_buffer_mark_image_written() when
> buffer_to_image == true, so there must be an if condition. I think this
> if condition is more direct compared to the alternatives of:
>* if (buffer_to_image)
>* if (dst == image)

Right, thanks, I got it now. This also became clearer to me when looking in
a tree.

> 
> -Nanley
> 
> > > + assert(dst == );
> > > + anv_cmd_buffer_mark_image_written(cmd_buffer, anv_image,
> > > +   aspect, dst->surf.aux_usage,
> > > +   dst->level);
> > > +  }
> > >  
> > >for (unsigned z = 0; z < extent.depth; z++) {
> > >   blorp_copy(, >surf, src->level, src->offset.z,
> > > @@ -497,6 +510,10 @@ void anv_CmdBlitImage(
> > >get_blorp_surf_for_anv_image(cmd_buffer->device,
> > > dst_image, dst_res->aspectMask,
> > > ANV_AUX_USAGE_DEFAULT, );
> > > +  anv_cmd_buffer_mark_image_written(cmd_buffer, dst_image,
> > > +dst_res->aspectMask,
> > > +dst.aux_usage,
> > > +dst_res->mipLevel);
> > >  
> > >struct anv_format_plane src_format =
> > >   anv_get_format_plane(_buffer->device->info, 
> > > src_image->vk_format,
> > > @@ -820,6 +837,10 @@ void anv_CmdClearColorImage(
> > >  layer_count = anv_minify(image->extent.depth, level);
> > >   }
> > >  
> > > + anv_cmd_buffer_mark_image_written(cmd_buffer, image,
> > > +   pRanges[r].aspectMask,
> > > +

[Mesa-dev] [Bug 100430] [radv] graphical glitches on dolphin emulator

2018-01-13 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=100430

--- Comment #14 from jdr...@gmail.com ---
Still present in 17.3.2

-- 
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 29/29] anv: Use blorp_ccs_ambiguate instead of fast-clears

2018-01-13 Thread Pohjolainen, Topi 
On Mon, Nov 27, 2017 at 07:06:19PM -0800, Jason Ekstrand wrote:
> Even though the blorp pass looks a bit on the sketchy side, the end
> result in the Vulkan driver is very nice.  Instead of having this weird
> case where you do a fast clear and then maybe have to resolve, we just
> do the ambiguate and are done with it.  The ambiguate does exactly what
> we want of setting all the CCS values to 0 which puts it inot the
> pass-through state.

For me there wasn't enough context here to understand fully what is going on.
Looking in a tree made it clearer why we don't need to bother about the
current color values themselves.

Reviewed-by: Topi Pohjolainen 

> ---
>  src/intel/vulkan/anv_blorp.c   |  5 +
>  src/intel/vulkan/genX_cmd_buffer.c | 40 
> ++
>  2 files changed, 7 insertions(+), 38 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index 45d7b12..84ac720 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -1705,6 +1705,11 @@ anv_image_ccs_op(struct anv_cmd_buffer *cmd_buffer,
>  surf.surf->format, 
> isl_to_blorp_fast_clear_op(ccs_op));
>break;
> case ISL_AUX_OP_AMBIGUATE:
> +  for (uint32_t a = 0; a < layer_count; a++) {
> + const uint32_t layer = base_layer + a;
> + blorp_ccs_ambiguate(, , level, layer);
> +  }
> +  break;
> default:
>unreachable("Unsupported CCS operation");
> }
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index dcd5a8f..e8a4d90 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -614,17 +614,7 @@ init_fast_clear_state_entry(struct anv_cmd_buffer 
> *cmd_buffer,
> uint32_t plane = anv_image_aspect_to_plane(image->aspects, aspect);
> enum isl_aux_usage aux_usage = image->planes[plane].aux_usage;
>  
> -   /* The resolve flag should updated to signify that fast-clear/compression
> -* data needs to be removed when leaving the undefined layout. Such data
> -* may need to be removed if it would cause accesses to the color buffer
> -* to return incorrect data. The fast clear data in CCS_D buffers should
> -* be removed because CCS_D isn't enabled all the time.
> -*/
> -   if (aux_usage == ISL_AUX_USAGE_NONE) {
> -  set_image_needs_resolve_bits(cmd_buffer, image, aspect, level, ~0);
> -   } else {
> -  clear_image_needs_resolve_bits(cmd_buffer, image, aspect, level, ~0);
> -   }
> +   clear_image_needs_resolve_bits(cmd_buffer, image, aspect, level, ~0);
>  
> /* The fast clear value dword(s) will be copied into a surface state 
> object.
>  * Ensure that the restrictions of the fields in the dword(s) are 
> followed.
> @@ -830,7 +820,7 @@ transition_color_buffer(struct anv_cmd_buffer *cmd_buffer,
> MIN2(layer_count, anv_image_aux_layers(image, aspect, level));
>  anv_image_ccs_op(cmd_buffer, image, aspect, level,
>   base_layer, level_layer_count,
> - ISL_AUX_OP_FAST_CLEAR, false);
> + ISL_AUX_OP_AMBIGUATE, false);
>   }
>} else if (image->samples > 1) {
>   if (image->samples == 4 || image->samples == 16) {
> @@ -844,32 +834,6 @@ transition_color_buffer(struct anv_cmd_buffer 
> *cmd_buffer,
>base_layer, layer_count,
>ISL_AUX_OP_FAST_CLEAR, false);
>}
> -  /* At this point, some elements of the CCS buffer may have the 
> fast-clear
> -   * bit-arrangement. As the user writes to a subresource, we need to 
> have
> -   * the associated CCS elements enter the ambiguated state. This enables
> -   * reads (implicit or explicit) to reflect the user-written data 
> instead
> -   * of the clear color. The only time such elements will not change 
> their
> -   * state as described above, is in a final layout that doesn't have CCS
> -   * enabled. In this case, we must force the associated CCS buffers of 
> the
> -   * specified range to enter the ambiguated state in advance.
> -   */
> -  if (image->samples == 1 &&
> -  image->planes[plane].aux_usage != ISL_AUX_USAGE_CCS_E &&
> -  final_layout != VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL) {
> - /* The CCS_D buffer may not be enabled in the final layout. Call 
> this
> -  * function again with a initial layout of COLOR_ATTACHMENT_OPTIMAL
> -  * to perform a resolve.
> -  */
> -  anv_perf_warn(cmd_buffer->device->instance, image,
> -"Performing an additional resolve for CCS_D layout "
> -"transition. Consider always leaving it on or "
> -"performing an ambiguation pass.");
> - transition_color_buffer(cmd_buffer, image,

[Mesa-dev] [Bug 102891] [radv] glitches on rpcs3 emulator (green zones)

2018-01-13 Thread bugzilla-daemon 
https://bugs.freedesktop.org/show_bug.cgi?id=102891

jdr...@gmail.com changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|NEEDINFO|RESOLVED

--- Comment #14 from jdr...@gmail.com ---
solved in 17.3.2

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