[Mesa-dev] [PATCH] vulkan/wsi/wayland: fix leaks

2018-03-30 Thread James Legg 
Fixes: bfa22266cd vulkan/wsi/wayland: Add support for zwp_dmabuf
CC: Daniel Stone 
CC: Jason Ekstrand 
---
 src/vulkan/wsi/wsi_common_wayland.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/vulkan/wsi/wsi_common_wayland.c 
b/src/vulkan/wsi/wsi_common_wayland.c
index d36947bc29..ec38a4e578 100644
--- a/src/vulkan/wsi/wsi_common_wayland.c
+++ b/src/vulkan/wsi/wsi_common_wayland.c
@@ -317,8 +317,12 @@ wsi_wl_display_finish(struct wsi_wl_display *display)
assert(display->refcount == 0);
 
u_vector_finish(>formats);
+   u_vector_finish(>modifiers.argb);
+   u_vector_finish(>modifiers.xrgb);
if (display->drm)
   wl_drm_destroy(display->drm);
+   if (display->dmabuf)
+  zwp_linux_dmabuf_v1_destroy(display->dmabuf);
if (display->wl_display_wrapper)
   wl_proxy_wrapper_destroy(display->wl_display_wrapper);
if (display->queue)
-- 
2.14.3

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

Re: [Mesa-dev] [PATCH] radv: Unset ZRANGE_PRECISION when depth was zeroed

2018-03-22 Thread James Legg 
On Thu, 2018-03-22 at 02:36 +0100, Bas Nieuwenhuizen wrote:
> On Thu, Mar 8, 2018 at 12:59 PM, James Legg <jl...@feralinteractive.com> 
> wrote:
> > This avoids bug 105396 somehow. I suspect it is a VI and GFX9 hardware
> > bug which PAL calls WaTcCompatZRange, but I don't know for sure.
> > 
> > In the VK_FORMAT_D32_SFLOAT case, TILE_STENCIL_DISABLE is not set for
> > tc compatible image formats regardless of not having a stencil aspect.
> > If TILE_STENCIL_DISABLE was set, ZRANGE_PRECISION would have no effect
> > and the bug would occur again.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105396
> > CC: <mesa-sta...@lists.freedesktop.org>
> > CC: Dave Airlie <airl...@redhat.com>
> > CC: Bas Nieuwenhuizen <b...@basnieuwenhuizen.nl>
> > CC: Samuel Pitoiset <samuel.pitoi...@gmail.com>
> > ---
> >  src/amd/vulkan/radv_cmd_buffer.c | 52 
> > +---
> >  1 file changed, 49 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
> > b/src/amd/vulkan/radv_cmd_buffer.c
> > index 3e0ed0e9a9..89e31a0347 100644
> > --- a/src/amd/vulkan/radv_cmd_buffer.c
> > +++ b/src/amd/vulkan/radv_cmd_buffer.c
> > @@ -915,6 +915,37 @@ radv_emit_fb_ds_state(struct radv_cmd_buffer 
> > *cmd_buffer,
> > 
> > }
> > 
> > +   if (image->surface.htile_size)
> > +   {
> > +   /* If the last depth clear value was 0.0f, set 
> > ZRANGE_PRECISION
> > +* to 0 in dp_z_info for more accuracy with reverse depth; 
> > and
> > +* to avoid 
> > https://bugs.freedesktop.org/show_bug.cgi?id=105396.
> > +* Otherwise, we leave it set to 1.
> > +*/
> > +   radeon_emit(cmd_buffer->cs, PKT3(PKT3_COND_WRITE, 7, 0));
> > +
> > +   const uint32_t write_space = 0 << 8;/* register */
> > +   const uint32_t poll_space = 1 << 4; /* memory */
> > +   const uint32_t function = 3 << 0;   /* equal to the 
> > reference */
> > +   const uint32_t options = write_space | poll_space | 
> > function;
> > +   radeon_emit(cmd_buffer->cs, options);
> > +
> > +   /* poll address - location of the depth clear value */
> > +   uint64_t va = radv_buffer_get_va(image->bo);
> > +   va += image->offset + image->clear_value_offset;
> > +   radeon_emit(cmd_buffer->cs, va);
> > +   radeon_emit(cmd_buffer->cs, va >> 32);
> > +
> > +   radeon_emit(cmd_buffer->cs, fui(0.0f)); /* 
> > reference value */
> > +   radeon_emit(cmd_buffer->cs, (uint32_t)-1);  /* 
> > comparison mask */
> > +   radeon_emit(cmd_buffer->cs, R_028040_DB_Z_INFO >> 2); /* 
> > write address low */
> > +   radeon_emit(cmd_buffer->cs, 0u);/* write 
> > address high */
> > +
> > +   /* The value to write data when the condition passes */
> > +   uint32_t db_z_info_clear_zero = db_z_info & 
> > C_028040_ZRANGE_PRECISION;
> > +   radeon_emit(cmd_buffer->cs, db_z_info_clear_zero);
> > +   }
> > +
> > radeon_set_context_reg(cmd_buffer->cs, 
> > R_028B78_PA_SU_POLY_OFFSET_DB_FMT_CNTL,
> >ds->pa_su_poly_offset_db_fmt_cntl);
> >  }
> > @@ -3479,7 +3510,8 @@ void radv_CmdEndRenderPass(
> > 
> >  /*
> >   * For HTILE we have the following interesting clear words:
> > - *   0xf30f: Uncompressed, full depth range, for depth+stencil HTILE
> > + *   0xf30f: Uncompressed, full depth range, for depth+stencil HTILE 
> > when ZRANGE_PRECISION is 1
> > + *   0x0003f30f: Uncompressed, full depth range, for depth+stencil HTILE 
> > when ZRANGE_PRECISION is 0
> >   *   0xfffc000f: Uncompressed, full depth range, for depth only HTILE.
> >   *   0xfff0: Clear depth to 1.0
> >   *   0x: Clear depth to 0.0
> > @@ -3528,8 +3560,22 @@ static void 
> > radv_handle_depth_image_transition(struct radv_cmd_buffer *cmd_buffe
> > radv_initialize_htile(cmd_buffer, image, range, 0);
> > } else if (!radv_layout_is_htile_compressed(image, src_layout, 
> > src_queue_mask) &&
> >radv_layout_is_htile_compressed(image, dst_layout, 
> > dst_queue_mask)) {
> > -

[Mesa-dev] [PATCH] radv: Unset ZRANGE_PRECISION when depth was zeroed

2018-03-08 Thread James Legg 
This avoids bug 105396 somehow. I suspect it is a VI and GFX9 hardware
bug which PAL calls WaTcCompatZRange, but I don't know for sure.

In the VK_FORMAT_D32_SFLOAT case, TILE_STENCIL_DISABLE is not set for
tc compatible image formats regardless of not having a stencil aspect.
If TILE_STENCIL_DISABLE was set, ZRANGE_PRECISION would have no effect
and the bug would occur again.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105396
CC: 
CC: Dave Airlie 
CC: Bas Nieuwenhuizen 
CC: Samuel Pitoiset 
---
 src/amd/vulkan/radv_cmd_buffer.c | 52 +---
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 3e0ed0e9a9..89e31a0347 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -915,6 +915,37 @@ radv_emit_fb_ds_state(struct radv_cmd_buffer *cmd_buffer,
 
}
 
+   if (image->surface.htile_size)
+   {
+   /* If the last depth clear value was 0.0f, set ZRANGE_PRECISION
+* to 0 in dp_z_info for more accuracy with reverse depth; and
+* to avoid https://bugs.freedesktop.org/show_bug.cgi?id=105396.
+* Otherwise, we leave it set to 1.
+*/
+   radeon_emit(cmd_buffer->cs, PKT3(PKT3_COND_WRITE, 7, 0));
+
+   const uint32_t write_space = 0 << 8;/* register */
+   const uint32_t poll_space = 1 << 4; /* memory */
+   const uint32_t function = 3 << 0;   /* equal to the 
reference */
+   const uint32_t options = write_space | poll_space | function;
+   radeon_emit(cmd_buffer->cs, options);
+
+   /* poll address - location of the depth clear value */
+   uint64_t va = radv_buffer_get_va(image->bo);
+   va += image->offset + image->clear_value_offset;
+   radeon_emit(cmd_buffer->cs, va);
+   radeon_emit(cmd_buffer->cs, va >> 32);
+
+   radeon_emit(cmd_buffer->cs, fui(0.0f)); /* reference 
value */
+   radeon_emit(cmd_buffer->cs, (uint32_t)-1);  /* comparison 
mask */
+   radeon_emit(cmd_buffer->cs, R_028040_DB_Z_INFO >> 2); /* write 
address low */
+   radeon_emit(cmd_buffer->cs, 0u);/* write 
address high */
+
+   /* The value to write data when the condition passes */
+   uint32_t db_z_info_clear_zero = db_z_info & 
C_028040_ZRANGE_PRECISION;
+   radeon_emit(cmd_buffer->cs, db_z_info_clear_zero);
+   }
+
radeon_set_context_reg(cmd_buffer->cs, 
R_028B78_PA_SU_POLY_OFFSET_DB_FMT_CNTL,
   ds->pa_su_poly_offset_db_fmt_cntl);
 }
@@ -3479,7 +3510,8 @@ void radv_CmdEndRenderPass(
 
 /*
  * For HTILE we have the following interesting clear words:
- *   0xf30f: Uncompressed, full depth range, for depth+stencil HTILE
+ *   0xf30f: Uncompressed, full depth range, for depth+stencil HTILE when 
ZRANGE_PRECISION is 1
+ *   0x0003f30f: Uncompressed, full depth range, for depth+stencil HTILE when 
ZRANGE_PRECISION is 0
  *   0xfffc000f: Uncompressed, full depth range, for depth only HTILE.
  *   0xfff0: Clear depth to 1.0
  *   0x: Clear depth to 0.0
@@ -3528,8 +3560,22 @@ static void radv_handle_depth_image_transition(struct 
radv_cmd_buffer *cmd_buffe
radv_initialize_htile(cmd_buffer, image, range, 0);
} else if (!radv_layout_is_htile_compressed(image, src_layout, 
src_queue_mask) &&
   radv_layout_is_htile_compressed(image, dst_layout, 
dst_queue_mask)) {
-   uint32_t clear_value = vk_format_is_stencil(image->vk_format) ? 
0xf30f : 0xfffc000f;
-   radv_initialize_htile(cmd_buffer, image, range, clear_value);
+   if (vk_format_is_stencil(image->vk_format)) {
+   /* The appropriate clear value depends on DB_Z_INFO's
+* ZRANGE_PRECISION, which can vary depending on the
+* last used clear value, which could be from another
+* command buffer. Instead of picking the appropriate
+* clear value on the GPU, resummarize accurately.
+*/
+   VkImageSubresourceRange local_range = *range;
+   local_range.aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT;
+   local_range.baseMipLevel = 0;
+   local_range.levelCount = 1;
+
+   radv_resummarize_depth_image_inplace(cmd_buffer, image, 
_range);
+   } else {
+   radv_initialize_htile(cmd_buffer, image, range, 
0xfffc000f);
+   }
} else if (radv_layout_is_htile_compressed(image, src_layout,

Re: [Mesa-dev] [PATCH] radv: Really use correct HTILE expanded words.

2018-02-23 Thread James Legg 
On Thu, 2018-02-22 at 22:48 +0100, Bas Nieuwenhuizen wrote:
> since IIRC the last change was also done due to Feral noticing and we
> are clearly lacking testcases in this area, can you check that that
> case still works for you? Thanks a lot!

I looked at an issue that was fixed with 5158603182fe7435 (and still
occurs if I change the clear word back to 0x) and I can confirm
this patch does not reintroduce it.

> On Thu, Feb 22, 2018 at 5:57 PM, James Legg <jl...@feralinteractive.com> 
> wrote:
> > When transitioning to an htile compressed depth format, Set the full
> > depth range, so later rasterization can pass HiZ. Previously, for depth
> > only formats, the depth range was set to 0 to 0. This caused unwanted
> > HiZ rejections with a VK_FORMAT_D16_UNORM depth buffer
> > (VK_FORMAT_D32_SFLOAT was not affected somehow).
> > 
> > These values are derived from PAL [0], since I can't find the
> > specification describing the htile values.
> > 
> > Fixes 5158603182fe7435: radv: Use correct HTILE expanded words.
> > 
> > [0] 
> > https://github.com/GPUOpen-Drivers/pal/blob/5cba4ecbda9452773f59692f5915301e7db4a183/src/core/hw/gfxip/gfx9/gfx9MaskRam.cpp#L1500
> > 
> > CC: Dave Airlie <airl...@redhat.com>
> > CC: Bas Nieuwenhuizen <b...@basnieuwenhuizen.nl>
> > Cc: mesa-sta...@lists.freedesktop.org
> > ---
> >  src/amd/vulkan/radv_cmd_buffer.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
> > b/src/amd/vulkan/radv_cmd_buffer.c
> > index 8a384b114c..2b41baea3d 100644
> > --- a/src/amd/vulkan/radv_cmd_buffer.c
> > +++ b/src/amd/vulkan/radv_cmd_buffer.c
> > @@ -3440,8 +3440,8 @@ void radv_CmdEndRenderPass(
> > 
> >  /*
> >   * For HTILE we have the following interesting clear words:
> > - *   0x030f: Uncompressed for depth+stencil HTILE.
> > - *   0x000f: Uncompressed for depth only HTILE.
> > + *   0xf30f: Uncompressed, full depth range, for depth+stencil HTILE
> > + *   0xfffc000f: Uncompressed, full depth range, for depth only HTILE.
> >   *   0xfff0: Clear depth to 1.0
> >   *   0x: Clear depth to 0.0
> >   */
> > @@ -3489,7 +3489,7 @@ static void radv_handle_depth_image_transition(struct 
> > radv_cmd_buffer *cmd_buffe
> > radv_initialize_htile(cmd_buffer, image, range, 0);
> > } else if (!radv_layout_is_htile_compressed(image, src_layout, 
> > src_queue_mask) &&
> >radv_layout_is_htile_compressed(image, dst_layout, 
> > dst_queue_mask)) {
> > -   uint32_t clear_value = 
> > vk_format_is_stencil(image->vk_format) ? 0x30f : 0xf;
> > +   uint32_t clear_value = 
> > vk_format_is_stencil(image->vk_format) ? 0xf30f : 0xfffc000f;
> > radv_initialize_htile(cmd_buffer, image, range, 
> > clear_value);
> > } else if (radv_layout_is_htile_compressed(image, src_layout, 
> > src_queue_mask) &&
> >!radv_layout_is_htile_compressed(image, dst_layout, 
> > dst_queue_mask)) {
> > --
> > 2.14.3
> > 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] radv: Really use correct HTILE expanded words.

2018-02-22 Thread James Legg 
When transitioning to an htile compressed depth format, Set the full
depth range, so later rasterization can pass HiZ. Previously, for depth
only formats, the depth range was set to 0 to 0. This caused unwanted
HiZ rejections with a VK_FORMAT_D16_UNORM depth buffer
(VK_FORMAT_D32_SFLOAT was not affected somehow).

These values are derived from PAL [0], since I can't find the
specification describing the htile values.

Fixes 5158603182fe7435: radv: Use correct HTILE expanded words.

[0] 
https://github.com/GPUOpen-Drivers/pal/blob/5cba4ecbda9452773f59692f5915301e7db4a183/src/core/hw/gfxip/gfx9/gfx9MaskRam.cpp#L1500

CC: Dave Airlie 
CC: Bas Nieuwenhuizen 
Cc: mesa-sta...@lists.freedesktop.org
---
 src/amd/vulkan/radv_cmd_buffer.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 8a384b114c..2b41baea3d 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -3440,8 +3440,8 @@ void radv_CmdEndRenderPass(
 
 /*
  * For HTILE we have the following interesting clear words:
- *   0x030f: Uncompressed for depth+stencil HTILE.
- *   0x000f: Uncompressed for depth only HTILE.
+ *   0xf30f: Uncompressed, full depth range, for depth+stencil HTILE
+ *   0xfffc000f: Uncompressed, full depth range, for depth only HTILE.
  *   0xfff0: Clear depth to 1.0
  *   0x: Clear depth to 0.0
  */
@@ -3489,7 +3489,7 @@ static void radv_handle_depth_image_transition(struct 
radv_cmd_buffer *cmd_buffe
radv_initialize_htile(cmd_buffer, image, range, 0);
} else if (!radv_layout_is_htile_compressed(image, src_layout, 
src_queue_mask) &&
   radv_layout_is_htile_compressed(image, dst_layout, 
dst_queue_mask)) {
-   uint32_t clear_value = vk_format_is_stencil(image->vk_format) ? 
0x30f : 0xf;
+   uint32_t clear_value = vk_format_is_stencil(image->vk_format) ? 
0xf30f : 0xfffc000f;
radv_initialize_htile(cmd_buffer, image, range, clear_value);
} else if (radv_layout_is_htile_compressed(image, src_layout, 
src_queue_mask) &&
   !radv_layout_is_htile_compressed(image, dst_layout, 
dst_queue_mask)) {
-- 
2.14.3

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

Re: [Mesa-dev] Mesa 17.3.4 release candidate

2018-02-13 Thread James Legg 
The conflict resolution on this commit has a typo, it should use
(index + i) instead of (index + 1).

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

[Mesa-dev] [PATCH] ac/nir: Fix conflict resolution typo in handle_vs_input_decl

2018-02-13 Thread James Legg 
Fixes ad764e365beb8a119369b97f5cb95fc7ea8c:
"ac/nir: Use instance_rate_inputs per attribute, not per variable".

CC: 
CC: Emil Velikov 
CC: Bas Nieuwenhuizen 
---
 src/amd/common/ac_nir_to_llvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index dde6cbc8f5..4812debb2f 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -5117,7 +5117,7 @@ handle_vs_input_decl(struct nir_to_llvm_context *ctx,
variable->data.driver_location = idx * 4;
 
for (unsigned i = 0; i < attrib_count; ++i, ++idx) {
-   if (ctx->options->key.vs.instance_rate_inputs & (1u << (index + 
1))) {
+   if (ctx->options->key.vs.instance_rate_inputs & (1u << (index + 
i))) {
buffer_index = LLVMBuildAdd(ctx->builder, 
ctx->abi.instance_id,
ctx->abi.start_instance, 
"");
ctx->shader_info->vs.vgpr_comp_cnt =
-- 
2.14.3

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

Re: [Mesa-dev] [PATCH 1/2] radv: enable lowering of nir_op_bitfield_insert

2017-12-06 Thread James Legg 
On Tue, 2017-12-05 at 14:24 -0500, Connor Abbott wrote:
> lower_bitfield_insert lowers nir_op_bitfield_insert to DX10-style
> nir_op_bfi and nir_op_bfm, both of which aren't handled by
> ac_nir_to_llvm, so unless I'm missing something this will just break
> them even harder. We probably should use this lowering after adding
> support for bfi and bfm, since AMD does have native instructions for
> bfi and bfm, but first I'd like to see the actual bug fixed. Have you
> tried running it with NIR_PRINT=true to pin down which optimization
> pass is going wrong?

I've identified the constant folding pass as the culprit and fixed the
incorrect logic for bitfield_insert with this patch:
nir/opcodes: Fix constant-folding of bitfield_insert
https://patchwork.freedesktop.org/patch/191977/

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

[Mesa-dev] [PATCH] nir/opcodes: Fix constant-folding of bitfield_insert

2017-12-06 Thread James Legg 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104119
CC: 
CC: Samuel Pitoiset 
---
 src/compiler/nir/nir_opcodes.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py
index ac7333fe78..278562b2bd 100644
--- a/src/compiler/nir/nir_opcodes.py
+++ b/src/compiler/nir/nir_opcodes.py
@@ -724,12 +724,12 @@ opcode("bitfield_insert", 0, tuint32, [0, 0, 0, 0],
 unsigned base = src0, insert = src1;
 int offset = src2, bits = src3;
 if (bits == 0) {
-   dst = 0;
+   dst = base;
 } else if (offset < 0 || bits < 0 || bits + offset > 32) {
dst = 0;
 } else {
unsigned mask = ((1ull << bits) - 1) << offset;
-   dst = (base & ~mask) | ((insert << bits) & mask);
+   dst = (base & ~mask) | ((insert << offset) & mask);
 }
 """)
 
-- 
2.14.3

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

Re: [Mesa-dev] [PATCH] radv: do not dump meta shaders with RADV_DEBUG=shaders

2017-12-04 Thread James Legg 
Hi Samuel,

On Thu, 2017-11-30 at 22:16 +0100, Samuel Pitoiset wrote:
> It's really annoying and this pollutes the output especially
> when a bunch of non-meta shaders are compiled.

> diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
> index 32edf2abd2..5464d3a58e 100644
> --- a/src/amd/vulkan/radv_shader.c
> +++ b/src/amd/vulkan/radv_shader.c
> @@ -429,7 +429,7 @@ shader_variant_create(struct radv_device *device,
> unsigned *code_size_out)
>  {
>   enum radeon_family chip_family = 
> device->physical_device->rad_info.family;
> - bool dump_shaders = device->instance->debug_flags & 
> RADV_DEBUG_DUMP_SHADERS;
> + bool dump_shaders = radv_can_dump_shader(device, module);
>   enum ac_target_machine_options tm_options = 0;
>   struct radv_shader_variant *variant;
>   struct ac_shader_binary binary;

This shader_variant_create function is called with NULL for the module
parameter in radv_create_gs_copy_shader.

> diff --git a/src/amd/vulkan/radv_shader.h b/src/amd/vulkan/radv_shader.h
> index 9bdbe848c8..91f2e7f2a1 100644
> --- a/src/amd/vulkan/radv_shader.h
> +++ b/src/amd/vulkan/radv_shader.h
> @@ -112,4 +113,13 @@ radv_shader_dump_stats(struct radv_device *device,
>  gl_shader_stage stage,
>  FILE *file);
>  
> +static inline bool
> +radv_can_dump_shader(struct radv_device *device,
> +  struct radv_shader_module *module)
> +{
> + /* Only dump non-meta shaders, useful for debugging purposes. */
> + return !module->nir &&
> +device->instance->debug_flags & RADV_DEBUG_DUMP_SHADERS;
> +}
> +
>  #endif

Consequently, there is a NULL dereference here if
radv_create_gs_copy_shader is used.

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

Re: [Mesa-dev] [PATCH] st/mesa: create framebuffer iface hash table per st manager

2017-07-25 Thread James Legg 
On Sun, 2017-07-23 at 16:37 -0700, Charmaine Lee wrote:
> With this patch, framebuffer interface hash table is created
> per state tracker manager.
> 
> Fixes crash with steam.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101876
> Fixes: 5124bf98239 ("st/mesa: add destroy_drawable interface")
> Tested-by: Christoph Haag 

> --- a/src/mesa/state_tracker/st_manager.c
> +++ b/src/mesa/state_tracker/st_manager.c
> @@ -511,45 +515,63 @@ st_framebuffer_iface_equal(const void *a, const void *b)
>  
>  
>  static boolean
> -st_framebuffer_iface_lookup(const struct st_framebuffer_iface *stfbi)
> +st_framebuffer_iface_lookup(struct st_manager *smapi,
> +const struct st_framebuffer_iface *stfbi)
>  {
> +   struct st_manager_private *smPriv =
> +  (struct st_manager_private *)smapi->st_manager_private;
> struct hash_entry *entry;
>  
> -   mtx_lock(_mutex);
> -   entry = _mesa_hash_table_search(st_fbi_ht, stfbi);
> -   mtx_unlock(_mutex);
> +   assert(smPriv);
> +   assert(smPriv->stfbi_ht);
> +
> +   mtx_lock(>st_mutex);
> +   entry = _mesa_hash_table_search(smPriv->stfbi_ht, stfbi);
> +   mtx_unlock(>st_mutex);
>  
> return entry != NULL;
>  }
>  
>  
>  static boolean
> -st_framebuffer_iface_insert(struct st_framebuffer_iface *stfbi)
> +st_framebuffer_iface_insert(struct st_manager *smapi,
> +struct st_framebuffer_iface *stfbi)
>  {
> +   struct st_manager_private *smPriv =
> +  (struct st_manager_private *)smapi->st_manager_private;
> struct hash_entry *entry;
>  
> -   mtx_lock(_mutex);
> -   entry = _mesa_hash_table_insert(st_fbi_ht, stfbi, stfbi);
> -   mtx_unlock(_mutex);
> +   assert(smPriv);
> +   assert(smPriv->stfbi_ht);
> +
> +   mtx_lock(>st_mutex);
> +   entry = _mesa_hash_table_insert(smPriv->stfbi_ht, stfbi, stfbi);
> +   mtx_unlock(>st_mutex);
>  
> return entry != NULL;
>  }
>  
>  
>  static void
> -st_framebuffer_iface_remove(struct st_framebuffer_iface *stfbi)
> +st_framebuffer_iface_remove(struct st_manager *smapi,
> +struct st_framebuffer_iface *stfbi)
>  {
> +   struct st_manager_private *smPriv =
> +  (struct st_manager_private *)smapi->st_manager_private;
> struct hash_entry *entry;
>  
> -   mtx_lock(_mutex);
> -   entry = _mesa_hash_table_search(st_fbi_ht, stfbi);
> +   if (!smPriv || !smPriv->stfbi_ht);
> +  return;

The semicolon after the if causes the return to execute
unconditionally.

> +
> +   mtx_lock(>st_mutex);
> +   entry = _mesa_hash_table_search(smPriv->stfbi_ht, stfbi);
> if (!entry)
>goto unlock;
>  
> -   _mesa_hash_table_remove(st_fbi_ht, entry);
> +   _mesa_hash_table_remove(smPriv->stfbi_ht, entry);
>  
>  unlock:
> -   mtx_unlock(_mutex);
> +   mtx_unlock(>st_mutex);
>  }
>  
>  

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

Re: [Mesa-dev] [PATCH 03/11] anv/cmd_buffer: Properly handle render passes with 0 attachments

2017-07-17 Thread James Legg 
On Tue, 2017-07-11 at 17:04 -0700, Jason Ekstrand wrote:
> We were early returning and never created the NULL surface state.
> 
> Cc: mesa-sta...@lists.freedesktop.org

This patch fixes a bug that affected me.

Tested-by: James Legg <jl...@feralinteractive.com>

I submitted a similar patch before seeing this one
(https://patchwork.freedesktop.org/patch/167047/). I'll drop that if
this one is merged.

James

> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 23 +++
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 53c58ca..9b3bb10 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -429,19 +429,18 @@ genX(cmd_buffer_setup_attachments)(struct 
> anv_cmd_buffer *cmd_buffer,
>  
> vk_free(_buffer->pool->alloc, state->attachments);
>  
> -   if (pass->attachment_count == 0) {
> +   if (pass->attachment_count > 0) {
> +  state->attachments = vk_alloc(_buffer->pool->alloc,
> +pass->attachment_count *
> + sizeof(state->attachments[0]),
> +8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
> +  if (state->attachments == NULL) {
> + /* Propagate VK_ERROR_OUT_OF_HOST_MEMORY to vkEndCommandBuffer */
> + return anv_batch_set_error(_buffer->batch,
> +VK_ERROR_OUT_OF_HOST_MEMORY);
> +  }
> +   } else {
>    state->attachments = NULL;
> -  return VK_SUCCESS;
> -   }
> -
> -   state->attachments = vk_alloc(_buffer->pool->alloc,
> - pass->attachment_count *
> -  sizeof(state->attachments[0]),
> - 8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
> -   if (state->attachments == NULL) {
> -  /* Propagate VK_ERROR_OUT_OF_HOST_MEMORY to vkEndCommandBuffer */
> -  return anv_batch_set_error(_buffer->batch,
> - VK_ERROR_OUT_OF_HOST_MEMORY);
> }
>  
> /* Reserve one for the NULL state. */
> -- 
> 2.5.0.400.gff86faf
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] anv: remove early out from cmd_buffer_setup_attachments

2017-07-17 Thread James Legg 
On Fri, 2017-07-14 at 18:06 +0100, Lionel Landwerlin wrote:
> Hi James,
> 
> I think this patch from Jason might fix the same problem : 
> https://patchwork.freedesktop.org/patch/166280/

Yes it does. I'll drop my patch if and when Jason's is pushed to
master.

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

[Mesa-dev] [PATCH] anv: remove early out from cmd_buffer_setup_attachments

2017-07-14 Thread James Legg 
Even when there are no attachments, set up
cmd_buffer->state->render_pass_states, so that secondary command
buffers with the VK_COMMAND_BUFFER_USAGE_RENDER_PASS_CONTINUE_BIT
usage flag enabled can be used in VkCmdExecuteCommands with valid
allocation sizes.

CC: 
---
 src/intel/vulkan/genX_cmd_buffer.c | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index 53c58ca..7f733d8 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -431,17 +431,16 @@ genX(cmd_buffer_setup_attachments)(struct anv_cmd_buffer 
*cmd_buffer,
 
if (pass->attachment_count == 0) {
   state->attachments = NULL;
-  return VK_SUCCESS;
-   }
-
-   state->attachments = vk_alloc(_buffer->pool->alloc,
- pass->attachment_count *
-  sizeof(state->attachments[0]),
- 8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
-   if (state->attachments == NULL) {
-  /* Propagate VK_ERROR_OUT_OF_HOST_MEMORY to vkEndCommandBuffer */
-  return anv_batch_set_error(_buffer->batch,
- VK_ERROR_OUT_OF_HOST_MEMORY);
+   } else {
+  state->attachments = vk_alloc(_buffer->pool->alloc,
+pass->attachment_count *
+ sizeof(state->attachments[0]),
+8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
+  if (state->attachments == NULL) {
+ /* Propagate VK_ERROR_OUT_OF_HOST_MEMORY to vkEndCommandBuffer */
+ return anv_batch_set_error(_buffer->batch,
+VK_ERROR_OUT_OF_HOST_MEMORY);
+  }
}
 
/* Reserve one for the NULL state. */
-- 
2.9.4

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

[Mesa-dev] [PATCH v3 1/3] spirv: Fix reaching unreachable for compare exchange on images

2017-06-26 Thread James Legg 
We were hitting the
unreachable("Invalid image opcode")
near the end of vtn_handle_image when parsing the
SpvOpAtomicCompareExchange opcode.

v2: Add stable CC.
v3: Ignore SpvOpAtomicCompareExchangeWeak. It requires the Kernel
capability which is not exposed in Vulkan, and spirv_to_nir is not used
for OpenCL which does support it.

CC: 
---
 src/compiler/spirv/spirv_to_nir.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/compiler/spirv/spirv_to_nir.c 
b/src/compiler/spirv/spirv_to_nir.c
index 0a5eb0e..0e6229b 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -1977,6 +1977,7 @@ vtn_handle_image(struct vtn_builder *b, SpvOp opcode,
   intrin->src[2] = nir_src_for_ssa(vtn_ssa_value(b, w[3])->def);
   break;
 
+   case SpvOpAtomicCompareExchange:
case SpvOpAtomicIIncrement:
case SpvOpAtomicIDecrement:
case SpvOpAtomicExchange:
-- 
2.9.4

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

Re: [Mesa-dev] [PATCH 1/2] spirv: Fix reaching unreachable for compare exchange on images

2017-06-26 Thread James Legg 
On Fri, 2017-06-23 at 11:29 -0700, Jason Ekstrand wrote:
> OpAtomicCompareExchangeWeak is only available in OpenCL, not Vulkan. 
> As such, we probably don't want to handle it yet as no one uses
> spirv_to_nir for OpenCL.  The assert on AtomicCompareExchange is,
> however, a bug we should fix now.

I'll drop the OpAtomicCompareExchangeWeak changes. It is still
partially handled elsewhere in spirv_to_nir.c, however each case would
fail the assertion in fill_common_atomic_sources.

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

[Mesa-dev] [PATCH v2 2/3] ac/nir: Make intrinsic_name buffer long enough

2017-06-23 Thread James Legg 
When using cmpswap on an image, it was being trunctated to
lvm.amdgcn.image.atomic.cmpswa, with the coords type missing entirely.

v2: Add stable CC

CC: 
Reviewed-by: Grazvydas Ignotas 
---
 src/amd/common/ac_nir_to_llvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 49117d2..3a26668 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -3366,7 +3366,7 @@ static LLVMValueRef visit_image_atomic(struct 
nir_to_llvm_context *ctx,
const char *base_name = "llvm.amdgcn.image.atomic";
const char *atomic_name;
LLVMValueRef coords;
-   char intrinsic_name[32], coords_type[8];
+   char intrinsic_name[41], coords_type[8];
const struct glsl_type *type = glsl_without_array(var->type);
 
if (ctx->stage == MESA_SHADER_FRAGMENT)
-- 
2.9.4

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

[Mesa-dev] [PATCH v2 3/3] ac/nir: assert printfs will fit

2017-06-23 Thread James Legg 
---
 src/amd/common/ac_nir_to_llvm.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 3a26668..b32a9f5 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -1134,7 +1134,9 @@ static LLVMValueRef emit_intrin_1f_param(struct 
nir_to_llvm_context *ctx,
to_float(ctx, src0),
};
 
-   sprintf(name, "%s.f%d", intrin, get_elem_bits(ctx, result_type));
+   MAYBE_UNUSED const int length = snprintf(name, sizeof(name), "%s.f%d", 
intrin,
+get_elem_bits(ctx, 
result_type));
+   assert(length < sizeof(name));
return ac_build_intrinsic(>ac, name, result_type, params, 1, 
AC_FUNC_ATTR_READNONE);
 }
 
@@ -1149,7 +1151,9 @@ static LLVMValueRef emit_intrin_2f_param(struct 
nir_to_llvm_context *ctx,
to_float(ctx, src1),
};
 
-   sprintf(name, "%s.f%d", intrin, get_elem_bits(ctx, result_type));
+   MAYBE_UNUSED const int length = snprintf(name, sizeof(name), "%s.f%d", 
intrin,
+get_elem_bits(ctx, 
result_type));
+   assert(length < sizeof(name));
return ac_build_intrinsic(>ac, name, result_type, params, 2, 
AC_FUNC_ATTR_READNONE);
 }
 
@@ -1165,7 +1169,9 @@ static LLVMValueRef emit_intrin_3f_param(struct 
nir_to_llvm_context *ctx,
to_float(ctx, src2),
};
 
-   sprintf(name, "%s.f%d", intrin, get_elem_bits(ctx, result_type));
+   MAYBE_UNUSED const int length = snprintf(name, sizeof(name), "%s.f%d", 
intrin,
+get_elem_bits(ctx, 
result_type));
+   assert(length < sizeof(name));
return ac_build_intrinsic(>ac, name, result_type, params, 3, 
AC_FUNC_ATTR_READNONE);
 }
 
@@ -3425,8 +3431,9 @@ static LLVMValueRef visit_image_atomic(struct 
nir_to_llvm_context *ctx,
build_int_type_name(LLVMTypeOf(coords),
coords_type, sizeof(coords_type));
 
-   snprintf(intrinsic_name, sizeof(intrinsic_name),
-"%s.%s.%s", base_name, atomic_name, coords_type);
+   MAYBE_UNUSED const int length = snprintf(intrinsic_name, 
sizeof(intrinsic_name),
+"%s.%s.%s", base_name, 
atomic_name, coords_type);
+   assert(length < sizeof(intrinsic_name));
return ac_build_intrinsic(>ac, intrinsic_name, ctx->i32, params, 
param_count, 0);
 }
 
-- 
2.9.4

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

[Mesa-dev] [PATCH v2 1/3] spirv: Fix reaching unreachable for compare exchange on images

2017-06-23 Thread James Legg 
We were hitting the
unreachable("Invalid image opcode")
near the end of vtn_handle_image when parsing SpvOpAtomicCompareExchange
and SpvOpAtomicCompareExchangeWeak opcodes.

v2: Add stable CC

CC: 
---
 src/compiler/spirv/spirv_to_nir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/compiler/spirv/spirv_to_nir.c 
b/src/compiler/spirv/spirv_to_nir.c
index 0a5eb0e..8f21aec 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -1805,6 +1805,7 @@ fill_common_atomic_sources(struct vtn_builder *b, SpvOp 
opcode,
   break;
 
case SpvOpAtomicCompareExchange:
+   case SpvOpAtomicCompareExchangeWeak:
   src[0] = nir_src_for_ssa(vtn_ssa_value(b, w[8])->def);
   src[1] = nir_src_for_ssa(vtn_ssa_value(b, w[7])->def);
   break;
@@ -1977,6 +1978,8 @@ vtn_handle_image(struct vtn_builder *b, SpvOp opcode,
   intrin->src[2] = nir_src_for_ssa(vtn_ssa_value(b, w[3])->def);
   break;
 
+   case SpvOpAtomicCompareExchange:
+   case SpvOpAtomicCompareExchangeWeak:
case SpvOpAtomicIIncrement:
case SpvOpAtomicIDecrement:
case SpvOpAtomicExchange:
-- 
2.9.4

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

[Mesa-dev] [PATCH 3/2] ac/nir: assert printfs will fit

2017-06-22 Thread James Legg 
---
 src/amd/common/ac_nir_to_llvm.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 3a26668..b32a9f5 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -1134,7 +1134,9 @@ static LLVMValueRef emit_intrin_1f_param(struct 
nir_to_llvm_context *ctx,
to_float(ctx, src0),
};
 
-   sprintf(name, "%s.f%d", intrin, get_elem_bits(ctx, result_type));
+   MAYBE_UNUSED const int length = snprintf(name, sizeof(name), "%s.f%d", 
intrin,
+get_elem_bits(ctx, 
result_type));
+   assert(length < sizeof(name));
return ac_build_intrinsic(>ac, name, result_type, params, 1, 
AC_FUNC_ATTR_READNONE);
 }
 
@@ -1149,7 +1151,9 @@ static LLVMValueRef emit_intrin_2f_param(struct 
nir_to_llvm_context *ctx,
to_float(ctx, src1),
};
 
-   sprintf(name, "%s.f%d", intrin, get_elem_bits(ctx, result_type));
+   MAYBE_UNUSED const int length = snprintf(name, sizeof(name), "%s.f%d", 
intrin,
+get_elem_bits(ctx, 
result_type));
+   assert(length < sizeof(name));
return ac_build_intrinsic(>ac, name, result_type, params, 2, 
AC_FUNC_ATTR_READNONE);
 }
 
@@ -1165,7 +1169,9 @@ static LLVMValueRef emit_intrin_3f_param(struct 
nir_to_llvm_context *ctx,
to_float(ctx, src2),
};
 
-   sprintf(name, "%s.f%d", intrin, get_elem_bits(ctx, result_type));
+   MAYBE_UNUSED const int length = snprintf(name, sizeof(name), "%s.f%d", 
intrin,
+get_elem_bits(ctx, 
result_type));
+   assert(length < sizeof(name));
return ac_build_intrinsic(>ac, name, result_type, params, 3, 
AC_FUNC_ATTR_READNONE);
 }
 
@@ -3425,8 +3431,9 @@ static LLVMValueRef visit_image_atomic(struct 
nir_to_llvm_context *ctx,
build_int_type_name(LLVMTypeOf(coords),
coords_type, sizeof(coords_type));
 
-   snprintf(intrinsic_name, sizeof(intrinsic_name),
-"%s.%s.%s", base_name, atomic_name, coords_type);
+   MAYBE_UNUSED const int length = snprintf(intrinsic_name, 
sizeof(intrinsic_name),
+"%s.%s.%s", base_name, 
atomic_name, coords_type);
+   assert(length < sizeof(intrinsic_name));
return ac_build_intrinsic(>ac, intrinsic_name, ctx->i32, params, 
param_count, 0);
 }
 
-- 
2.9.4

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

[Mesa-dev] [PATCH 2/2] ac/nir: Make intrinsic_name buffer long enough

2017-06-21 Thread James Legg 
When using cmpswap on an image, it was being trunctated to
lvm.amdgcn.image.atomic.cmpswa, with the coords type missing entirely.
---
 src/amd/common/ac_nir_to_llvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 49117d2..3a26668 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -3366,7 +3366,7 @@ static LLVMValueRef visit_image_atomic(struct 
nir_to_llvm_context *ctx,
const char *base_name = "llvm.amdgcn.image.atomic";
const char *atomic_name;
LLVMValueRef coords;
-   char intrinsic_name[32], coords_type[8];
+   char intrinsic_name[41], coords_type[8];
const struct glsl_type *type = glsl_without_array(var->type);
 
if (ctx->stage == MESA_SHADER_FRAGMENT)
-- 
2.9.4

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

[Mesa-dev] [PATCH 1/2] spirv: Fix reaching unreachable for compare exchange on images

2017-06-21 Thread James Legg 
We were hitting the
unreachable("Invalid image opcode")
near the end of vtn_handle_image when parsing SpvOpAtomicCompareExchange
and SpvOpAtomicCompareExchangeWeak opcodes.
---
 src/compiler/spirv/spirv_to_nir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/compiler/spirv/spirv_to_nir.c 
b/src/compiler/spirv/spirv_to_nir.c
index 0a5eb0e..8f21aec 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -1805,6 +1805,7 @@ fill_common_atomic_sources(struct vtn_builder *b, SpvOp 
opcode,
   break;
 
case SpvOpAtomicCompareExchange:
+   case SpvOpAtomicCompareExchangeWeak:
   src[0] = nir_src_for_ssa(vtn_ssa_value(b, w[8])->def);
   src[1] = nir_src_for_ssa(vtn_ssa_value(b, w[7])->def);
   break;
@@ -1977,6 +1978,8 @@ vtn_handle_image(struct vtn_builder *b, SpvOp opcode,
   intrin->src[2] = nir_src_for_ssa(vtn_ssa_value(b, w[3])->def);
   break;
 
+   case SpvOpAtomicCompareExchange:
+   case SpvOpAtomicCompareExchangeWeak:
case SpvOpAtomicIIncrement:
case SpvOpAtomicIDecrement:
case SpvOpAtomicExchange:
-- 
2.9.4

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

Re: [Mesa-dev] [PATCH v3 44/63] st/mesa: add infrastructure for storing bound texture/image handles

2017-06-13 Thread James Legg 
On Fri, 2017-06-09 at 15:35 +0200, Samuel Pitoiset wrote:
> v2: - rename st_bound_handle to st_bound_handles
> 
> Signed-off-by: Samuel Pitoiset 
> Reviewed-by: Nicolai Hähnle  (v1)
> Reviewed-by: Marek Olšák  (v2)
> ---
>  src/mesa/state_tracker/st_context.c |  2 +
>  src/mesa/state_tracker/st_context.h | 11 ++
>  src/mesa/state_tracker/st_texture.c | 77
> +
>  src/mesa/state_tracker/st_texture.h |  5 +++
>  4 files changed, 95 insertions(+)

> diff --git a/src/mesa/state_tracker/st_texture.c
> b/src/mesa/state_tracker/st_texture.c
> index 7da111f39f..fdd727ec8e 100644
> --- a/src/mesa/state_tracker/st_texture.c
> +++ b/src/mesa/state_tracker/st_texture.c
> @@ -421,6 +421,83 @@ st_create_color_map_texture(struct gl_context
> *ctx)
> return pt;
>  }
>  
> +/**
> + * Destroy bound texture handles for the given stage.
> + */
> +static void
> +st_destroy_bound_texture_handles_per_stage(struct st_context *st,
> +   enum pipe_shader_type
> shader)
> +{
> +   struct st_bound_handles *bound_handles = 
> >bound_texture_handles[shader];
> +   struct pipe_context *pipe = st->pipe;
> +   unsigned i;
> +
> +   if (likely(!bound_handles->num_handles))
> +  return;
> +
> +   for (i = 0; i < bound_handles->num_handles; i++) {
> +  uint64_t handle = bound_handles->handles[i];
> +
> +  pipe->make_texture_handle_resident(pipe, handle, false);
> +  pipe->delete_texture_handle(pipe, handle);
> +   }
> +   free(bound_handles->handles);
> +   bound_handles->num_handles = 0;
> +}

Perhaps set bound_handles->handles to NULL here, otherwise the address
of freed memory can be used by the realloc added in the following
patch.

> +
> +
> +/**
> + * Destroy all bound texture handles in the context.
> + */
> +void
> +st_destroy_bound_texture_handles(struct st_context *st)
> +{
> +   unsigned i;
> +
> +   for (i = 0; i < PIPE_SHADER_TYPES; i++) {
> +  st_destroy_bound_texture_handles_per_stage(st, i);
> +   }
> +}
> +
> +
> +/**
> + * Destroy bound image handles for the given stage.
> + */
> +static void
> +st_destroy_bound_image_handles_per_stage(struct st_context *st,
> + enum pipe_shader_type
> shader)
> +{
> +   struct st_bound_handles *bound_handles = 
> >bound_image_handles[shader];
> +   struct pipe_context *pipe = st->pipe;
> +   unsigned i;
> +
> +   if (likely(!bound_handles->num_handles))
> +  return;
> +
> +   for (i = 0; i < bound_handles->num_handles; i++) {
> +  uint64_t handle = bound_handles->handles[i];
> +
> +  pipe->make_image_handle_resident(pipe, handle, GL_READ_WRITE,
> false);
> +  pipe->delete_image_handle(pipe, handle);
> +   }
> +   free(bound_handles->handles);
> +   bound_handles->num_handles = 0;

Same here.

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

Re: [Mesa-dev] [PATCH 21/25] radv/ac: add support for TCS/TES inputs/outputs.

2017-03-30 Thread James Legg 
On Thu, 2017-03-30 at 18:01 +1000, Dave Airlie wrote:
> --- a/src/amd/common/ac_nir_to_llvm.c
> +++ b/src/amd/common/ac_nir_to_llvm.c
> @@ -2244,6 +2370,322 @@ radv_get_deref_offset(struct
> nir_to_llvm_context *ctx, nir_deref *tail,
>  }
>  
>  static LLVMValueRef
> +lds_load(struct nir_to_llvm_context *ctx,
> +  LLVMValueRef dw_addr)
> +{
> + LLVMValueRef value;
> + value = ac_build_indexed_load(>ac, ctx->lds, dw_addr,
> false);
> + return value;
> +}
> +
> +static void
> +lds_store(struct nir_to_llvm_context *ctx,
> +   LLVMValueRef dw_addr, LLVMValueRef value)
> +{
> + value = LLVMBuildBitCast(ctx->builder, value, ctx->i32, "");
> + ac_build_indexed_store(>ac, ctx->lds,
> +    dw_addr, value);
> +}

Is ctx->lds supposed to set to something other than NULL before using
these functions? I can only see it being zeroed in
ac_create_gs_copy_shader and assigned in emit_ddxy. I'm seeing a crash
in LLVM 3.9.1's LLVMBuildGEP when this is dereferenced.

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

[Mesa-dev] [PATCH] radv: Fix using more than 4 bound descriptor sets

2017-03-16 Thread James Legg 
Avoid a buffer overflow in ac_nir_to_llvm.c's create_function when
using more than 4 descriptor sets. radv claims support for 8.

Cc: 17.0 
---
 src/amd/common/ac_nir_to_llvm.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/amd/common/ac_nir_to_llvm.h b/src/amd/common/ac_nir_to_llvm.h
index 6c2b78b..bb860e5 100644
--- a/src/amd/common/ac_nir_to_llvm.h
+++ b/src/amd/common/ac_nir_to_llvm.h
@@ -27,6 +27,7 @@
 #include "llvm-c/Core.h"
 #include "llvm-c/TargetMachine.h"
 #include "amd_family.h"
+#include "../vulkan/radv_descriptor_set.h"
 
 struct ac_shader_binary;
 struct ac_shader_config;
@@ -81,7 +82,8 @@ enum ac_ud_index {
AC_UD_MAX_UD = AC_UD_VS_MAX_UD,
 };
 
-#define AC_UD_MAX_SETS 4
+// Match MAX_SETS from radv_descriptor_set.h
+#define AC_UD_MAX_SETS MAX_SETS
 
 struct ac_userdata_locations {
struct ac_userdata_info descriptor_sets[AC_UD_MAX_SETS];
-- 
2.9.3

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

[Mesa-dev] [PATCH] mesa: lower severity for GLSL compiler warnings

2017-02-13 Thread James Legg 
This matches the examples in the GL_KHR_debug spec.
---
 src/mesa/main/errors.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index 3a40c74..ad495d6 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -385,7 +385,8 @@ _mesa_shader_debug(struct gl_context *ctx, GLenum type, 
GLuint *id,
const char *msg)
 {
enum mesa_debug_source source = MESA_DEBUG_SOURCE_SHADER_COMPILER;
-   enum mesa_debug_severity severity = MESA_DEBUG_SEVERITY_HIGH;
+   enum mesa_debug_severity severity = (type ==  MESA_DEBUG_TYPE_ERROR)
+   ? MESA_DEBUG_SEVERITY_HIGH : MESA_DEBUG_SEVERITY_MEDIUM;
int len;
 
_mesa_debug_get_id(id);
-- 
2.9.3

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

Re: [Mesa-dev] Time to merge threaded GL dispatch? (aka glthread)

2017-02-07 Thread James Legg 
Hi Gregory,

On Tue, 2017-02-07 at 16:04 +0100, Gregory Hainaut wrote:
> > Hi,
> > 
> > On Mon, 2017-02-06 at 13:43 +0100, Jan Ziak wrote:
> > > Shadow of Mordor benchmark: 30 FPS w/o glthread -> 20 FPS with
> > > glthread
> > > 
> > 
> > For what it is worth, all the Feral games have a dispatch thread that
> > primarily calls GL functions.
> > 
> > James
> > 
> 
> Hello James,
> 
> 
> Did you have the opportunity to compare Feral's dispatcher threading
> implementation versus Nvidia's threading option. If yes, is there any
> performance difference? In other word, did you implement it because
> some (most) drivers miss this optimization or because it can really
> bring some performance improvements (when done carefully).

We can't directly compare to Nvidia's option, as our threaded GL
implementation is a necessity for us. OpenGL contexts must be bound to
a specific thread, unlike D3D devices, and we have games written for
D3D using multiple threads to dispatch graphics work (and sometimes
using multiple D3D devices from the same thread). We make one dispatch
thread for each GL context to avoid having to unbind the context from a
thread. There is some locking so that only one game thread may submit
to a context's dispatch thread at once, but that locking is very
lightweight compared to what would happen if we repeatedly attached and
detached GL contexts on game threads. GL_KHR_context_flush_control was
supposed to help with that, by allowing opting out of flushing the GL
context when unbinding it from a thread, but last time we tried that,
there were some synchronous X requests in SDL2 making it remain slow.

Having said that, we did at one point have a path which used
GLX_MESA_multithreaded_make_current when it was available. We found the
GL dispatch threads was quicker on all the games we were working on at
the time.

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

Re: [Mesa-dev] Time to merge threaded GL dispatch? (aka glthread)

2017-02-06 Thread James Legg 
Hi,

On Mon, 2017-02-06 at 13:43 +0100, Jan Ziak wrote:
> Shadow of Mordor benchmark: 30 FPS w/o glthread -> 20 FPS with
> glthread
> 

For what it is worth, all the Feral games have a dispatch thread that
primarily calls GL functions.

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

[Mesa-dev] [PATCH] radeonsi: Fix primitive restart when index changes

2016-10-04 Thread James Legg 
If primitive restart is enabled for two consecutive draws which use
different primitive restart indices, then the first draw's primitive
restart index was incorrectly used for the second draw.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98025
---
 src/gallium/drivers/radeonsi/si_state_draw.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state_draw.c 
b/src/gallium/drivers/radeonsi/si_state_draw.c
index e44147f..10e8be4 100644
--- a/src/gallium/drivers/radeonsi/si_state_draw.c
+++ b/src/gallium/drivers/radeonsi/si_state_draw.c
@@ -509,13 +509,13 @@ static void si_emit_draw_registers(struct si_context 
*sctx,
radeon_set_context_reg(cs, R_028A94_VGT_MULTI_PRIM_IB_RESET_EN, 
info->primitive_restart);
sctx->last_primitive_restart_en = info->primitive_restart;
 
-   if (info->primitive_restart &&
-   (info->restart_index != sctx->last_restart_index ||
-sctx->last_restart_index == SI_RESTART_INDEX_UNKNOWN)) {
-   radeon_set_context_reg(cs, 
R_02840C_VGT_MULTI_PRIM_IB_RESET_INDX,
-  info->restart_index);
-   sctx->last_restart_index = info->restart_index;
-   }
+   }
+   if (info->primitive_restart &&
+   (info->restart_index != sctx->last_restart_index ||
+sctx->last_restart_index == SI_RESTART_INDEX_UNKNOWN)) {
+   radeon_set_context_reg(cs, 
R_02840C_VGT_MULTI_PRIM_IB_RESET_INDX,
+  info->restart_index);
+   sctx->last_restart_index = info->restart_index;
}
 }
 
-- 
2.8.3

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