[Mesa-dev] [PATCH 5/4] radv: create on-disk shader cache

[Timothy Arceri]

This is the drivers on-disk cache intended to be used as a
fallback as opposed to the pipeline cache provided by apps.
---
 src/amd/vulkan/radv_device.c  |  8 
 src/amd/vulkan/radv_private.h |  5 +
 src/util/disk_cache.h | 15 +++
 3 files changed, 28 insertions(+)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 65e6a2c..f31fde2 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -222,20 +222,27 @@ radv_physical_device_init(struct radv_physical_device 
*device,
}
 
if (radv_device_get_cache_uuid(device->rad_info.family, device->uuid)) {
radv_finish_wsi(device);
device->ws->destroy(device->ws);
result = vk_errorf(VK_ERROR_INITIALIZATION_FAILED,
   "cannot generate UUID");
goto fail;
}
 
+   /* The gpu id is already embeded in the uuid so we just pass "radv"
+* when creating the cache.
+*/
+   char buf[VK_UUID_SIZE];
+   disk_cache_format_hex_id(buf, device->uuid, VK_UUID_SIZE);
+   device->disk_cache = disk_cache_create("radv", buf);
+
result = radv_extensions_register(instance,
>extensions,
common_device_extensions,
ARRAY_SIZE(common_device_extensions));
if (result != VK_SUCCESS)
goto fail;
 
fprintf(stderr, "WARNING: radv is not a conformant vulkan 
implementation, testing use only.\n");
device->name = device->rad_info.name;
 
@@ -245,20 +252,21 @@ fail:
close(fd);
return result;
 }
 
 static void
 radv_physical_device_finish(struct radv_physical_device *device)
 {
radv_extensions_finish(device->instance, >extensions);
radv_finish_wsi(device);
device->ws->destroy(device->ws);
+   disk_cache_destroy(device->disk_cache);
close(device->local_fd);
 }
 
 
 static void *
 default_alloc_func(void *pUserData, size_t size, size_t align,
VkSystemAllocationScope allocationScope)
 {
return malloc(size);
 }
diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
index cbd2968..1fece19 100644
--- a/src/amd/vulkan/radv_private.h
+++ b/src/amd/vulkan/radv_private.h
@@ -262,20 +262,25 @@ struct radv_physical_device {
 
struct radeon_winsys *ws;
struct radeon_info rad_info;
charpath[20];
const char *name;
uint8_t uuid[VK_UUID_SIZE];
 
int local_fd;
struct wsi_device   wsi_device;
struct radv_extensions  extensions;
+
+   /* This is the drivers on-disk cache used as a fallback as opposed to
+* the pipeline cache defined by apps.
+*/
+   struct disk_cache * disk_cache;
 };
 
 struct radv_instance {
VK_LOADER_DATA  _loader_data;
 
VkAllocationCallbacks   alloc;
 
uint32_tapiVersion;
int physicalDeviceCount;
struct radv_physical_device 
physicalDevices[RADV_MAX_DRM_DEVICES];
diff --git a/src/util/disk_cache.h b/src/util/disk_cache.h
index 3659b6d..ca15f74 100644
--- a/src/util/disk_cache.h
+++ b/src/util/disk_cache.h
@@ -53,20 +53,35 @@ get_arch_bitness_str(void)
 return "32";
 #endif
 if (sizeof(void *) == 8)
 return "64";
 
 /* paranoia check which will be dropped by the optimiser */
 assert(!"unknown_arch");
 return "unknown_arch";
 }
 
+static inline char *
+disk_cache_format_hex_id(char *buf, const uint8_t *hex_id, unsigned size)
+{
+   static const char hex_digits[] = "0123456789abcdef";
+   unsigned i;
+
+   for (i = 0; i < size; i += 2) {
+  buf[i] = hex_digits[hex_id[i >> 1] >> 4];
+  buf[i + 1] = hex_digits[hex_id[i >> 1] & 0x0f];
+   }
+   buf[i] = '\0';
+
+   return buf;
+}
+
 static inline bool
 disk_cache_get_function_timestamp(void *ptr, uint32_t* timestamp)
 {
 #ifdef ENABLE_SHADER_CACHE
Dl_info info;
struct stat st;
if (!dladdr(ptr, ) || !info.dli_fname) {
   return false;
}
if (stat(info.dli_fname, )) {
-- 
2.9.3

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

[Mesa-dev] [PATCH 6/4] radv: make use of on-disk cache

[Timothy Arceri]

If the app provided and in-memory pipeline caches don't contain
what we are looking for then we fallback to the on-disk cache.
---
 src/amd/vulkan/radv_pipeline_cache.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/amd/vulkan/radv_pipeline_cache.c 
b/src/amd/vulkan/radv_pipeline_cache.c
index 3a58f6a..ffcb8e5 100644
--- a/src/amd/vulkan/radv_pipeline_cache.c
+++ b/src/amd/vulkan/radv_pipeline_cache.c
@@ -16,20 +16,21 @@
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
  * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
  * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
  * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
  * IN THE SOFTWARE.
  */
 
 #include "util/mesa-sha1.h"
 #include "util/debug.h"
+#include "util/disk_cache.h"
 #include "radv_private.h"
 
 #include "ac_nir_to_llvm.h"
 
 struct cache_entry {
union {
unsigned char sha1[20];
uint32_t sha1_dw[5];
};
uint32_t code_size;
@@ -152,22 +153,28 @@ radv_create_shader_variant_from_pipeline_cache(struct 
radv_device *device,
   struct radv_pipeline_cache 
*cache,
   const unsigned char *sha1)
 {
struct cache_entry *entry = NULL;
 
if (cache)
entry = radv_pipeline_cache_search(cache, sha1);
else
entry = radv_pipeline_cache_search(device->mem_cache, sha1);
 
-   if (!entry)
-   return NULL;
+   if (!entry) {
+   size_t entry_size;
+   entry = (struct cache_entry *)
+   disk_cache_get(device->physical_device->disk_cache,
+  sha1, _size);
+   if (!entry)
+   return NULL;
+   }
 
if (!entry->variant) {
struct radv_shader_variant *variant;
 
variant = calloc(1, sizeof(struct radv_shader_variant));
if (!variant)
return NULL;
 
variant->config = entry->config;
variant->info = entry->variant_info;
@@ -294,20 +301,22 @@ radv_pipeline_cache_insert_shader(struct radv_device 
*device,
memcpy(entry->code, code, code_size);
entry->config = variant->config;
entry->variant_info = variant->info;
entry->rsrc1 = variant->rsrc1;
entry->rsrc2 = variant->rsrc2;
entry->code_size = code_size;
entry->variant = variant;
__sync_fetch_and_add(>ref_count, 1);
 
radv_pipeline_cache_add_entry(cache, entry);
+   disk_cache_put(device->physical_device->disk_cache,
+  sha1, entry, sizeof(*entry) + code_size);
 
cache->modified = true;
pthread_mutex_unlock(>mutex);
return variant;
 }
 
 struct cache_header {
uint32_t header_size;
uint32_t header_version;
uint32_t vendor_id;
-- 
2.9.3

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

[Mesa-dev] [PATCH] radv/ac: round cube array coordinate before fixup.

[Dave Airlie]

From: Dave Airlie 

This fixes:
dEQP-VK.glsl.texture_functions.texture.samplercubearray*

Signed-off-by: Dave Airlie 
---
 src/amd/common/ac_nir_to_llvm.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 01346c3..8563d18 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -3654,6 +3654,8 @@ static void visit_tex(struct nir_to_llvm_context *ctx, 
nir_tex_instr *instr)
}
 
if (instr->sampler_dim == GLSL_SAMPLER_DIM_CUBE && coord) {
+   if (instr->is_array)
+   coords[3] = apply_round_slice(ctx, coords[3]);
for (chan = 0; chan < instr->coord_components; chan++)
coords[chan] = to_float(ctx, coords[chan]);
if (instr->coord_components == 3)
@@ -3681,7 +3683,9 @@ static void visit_tex(struct nir_to_llvm_context *ctx, 
nir_tex_instr *instr)
}
if (instr->coord_components > 2) {
/* This seems like a bit of a hack - but it passes 
Vulkan CTS with it */
-   if (instr->sampler_dim != GLSL_SAMPLER_DIM_3D && 
instr->op != nir_texop_txf) {
+   if (instr->sampler_dim != GLSL_SAMPLER_DIM_3D &&
+   instr->sampler_dim != GLSL_SAMPLER_DIM_CUBE &&
+   instr->op != nir_texop_txf) {
coords[2] = apply_round_slice(ctx, coords[2]);
}
address[count++] = coords[2];
-- 
2.9.3

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

[Mesa-dev] [PATCH v2 07/18] anv/allocator: Add a BO cache

[Jason Ekstrand]

This cache allows us to easily ensure that we have a unique anv_bo for
each gem handle.  We'll need this in order to support multiple-import of
memory objects and semaphores.

v2 (Jason Ekstrand):
 - Reject BO imports if the size doesn't match the prime fd size as
   reported by lseek().

---
 src/intel/vulkan/anv_allocator.c | 227 +++
 src/intel/vulkan/anv_private.h   |  26 +
 2 files changed, 253 insertions(+)

diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_allocator.c
index 45c663b..4cfb554 100644
--- a/src/intel/vulkan/anv_allocator.c
+++ b/src/intel/vulkan/anv_allocator.c
@@ -34,6 +34,8 @@
 
 #include "anv_private.h"
 
+#include "util/hash_table.h"
+
 #ifdef HAVE_VALGRIND
 #define VG_NOACCESS_READ(__ptr) ({   \
VALGRIND_MAKE_MEM_DEFINED((__ptr), sizeof(*(__ptr))); \
@@ -976,3 +978,228 @@ anv_scratch_pool_alloc(struct anv_device *device, struct 
anv_scratch_pool *pool,
 
return >bo;
 }
+
+struct anv_cached_bo {
+   struct anv_bo bo;
+
+   uint32_t refcount;
+};
+
+static uint32_t
+hash_uint32_t(const void *key)
+{
+   return (uint32_t)(uintptr_t)key;
+}
+
+static bool
+uint32_t_equal(const void *a, const void *b)
+{
+   return a == b;
+}
+
+VkResult
+anv_bo_cache_init(struct anv_bo_cache *cache)
+{
+   cache->bo_map = _mesa_hash_table_create(NULL, hash_uint32_t, 
uint32_t_equal);
+   if (!cache->bo_map)
+  return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
+
+   if (pthread_mutex_init(>mutex, NULL)) {
+  _mesa_hash_table_destroy(cache->bo_map, NULL);
+  return vk_errorf(VK_ERROR_OUT_OF_HOST_MEMORY,
+   "pthread_mutex_inti failed: %m");
+   }
+
+   return VK_SUCCESS;
+}
+
+void
+anv_bo_cache_finish(struct anv_bo_cache *cache)
+{
+   _mesa_hash_table_destroy(cache->bo_map, NULL);
+   pthread_mutex_destroy(>mutex);
+}
+
+static struct anv_cached_bo *
+anv_bo_cache_lookup_locked(struct anv_bo_cache *cache, uint32_t gem_handle)
+{
+   struct hash_entry *entry =
+  _mesa_hash_table_search(cache->bo_map,
+  (const void *)(uintptr_t)gem_handle);
+   if (!entry)
+  return NULL;
+
+   struct anv_cached_bo *bo = (struct anv_cached_bo *)entry->data;
+   assert(bo->bo.gem_handle == gem_handle);
+
+   return bo;
+}
+
+VkResult
+anv_bo_cache_alloc(struct anv_device *device,
+   struct anv_bo_cache *cache,
+   uint64_t size, struct anv_bo **bo_out,
+   VkAllocationCallbacks *alloc)
+{
+   struct anv_cached_bo *bo =
+  vk_alloc(alloc, size, 8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
+   if (!bo)
+  return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
+
+   bo->refcount = 1;
+
+   /* The kernel is going to give us whole pages anyway */
+   size = align_u64(size, 4096);
+
+   VkResult result = anv_bo_init_new(>bo, device, size);
+   if (result != VK_SUCCESS) {
+  vk_free(alloc, bo);
+  return result;
+   }
+
+   assert(bo->bo.gem_handle);
+
+   pthread_mutex_lock(>mutex);
+
+   _mesa_hash_table_insert(cache->bo_map,
+   (void *)(uintptr_t)bo->bo.gem_handle, bo);
+
+   pthread_mutex_unlock(>mutex);
+
+   *bo_out = >bo;
+
+   return VK_SUCCESS;
+}
+
+VkResult
+anv_bo_cache_import(struct anv_device *device,
+struct anv_bo_cache *cache,
+int fd, uint64_t size, struct anv_bo **bo_out,
+VkAllocationCallbacks *alloc)
+{
+   pthread_mutex_lock(>mutex);
+
+   /* The kernel is going to give us whole pages anyway */
+   size = align_u64(size, 4096);
+
+   uint32_t gem_handle = anv_gem_fd_to_handle(device, fd);
+   if (!gem_handle) {
+  pthread_mutex_unlock(>mutex);
+  return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHX);
+   }
+
+   struct anv_cached_bo *bo = anv_bo_cache_lookup_locked(cache, gem_handle);
+   if (bo) {
+  assert(bo->bo.size == size);
+  __sync_fetch_and_add(>refcount, 1);
+   } else {
+  /* For security purposes, we reject BO imports where the size does not
+   * match exactly.  This prevents a malicious client from passing a
+   * buffer to a trusted client, lying about the size, and telling the
+   * trusted client to try and texture from an image that goes
+   * out-of-bounds.  This sort of thing could lead to GPU hangs or worse
+   * in the trusted client.  The trusted client can protect itself against
+   * this sort of attack but only if it can trust the buffer size.
+   */
+  off_t import_size = lseek(fd, 0, SEEK_END);
+  if (import_size == (off_t)-1 || import_size != size) {
+ anv_gem_close(device, gem_handle);
+ pthread_mutex_unlock(>mutex);
+ return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHX);
+  }
+
+  struct anv_cached_bo *bo =
+ vk_alloc(alloc, size, 8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
+  if (!bo) {
+ anv_gem_close(device, gem_handle);
+ pthread_mutex_unlock(>mutex);
+ return 

[Mesa-dev] [PATCH] anv: Implement support for exporting semaphores as FENCE_FD

[Jason Ekstrand]

---
 src/intel/vulkan/anv_batch_chain.c | 86 --
 src/intel/vulkan/anv_device.c  | 26 
 src/intel/vulkan/anv_gem.c | 36 
 src/intel/vulkan/anv_private.h | 24 ---
 src/intel/vulkan/anv_queue.c   | 61 +--
 5 files changed, 221 insertions(+), 12 deletions(-)

diff --git a/src/intel/vulkan/anv_batch_chain.c 
b/src/intel/vulkan/anv_batch_chain.c
index 3f6b767..ac43888 100644
--- a/src/intel/vulkan/anv_batch_chain.c
+++ b/src/intel/vulkan/anv_batch_chain.c
@@ -1352,6 +1352,23 @@ setup_execbuf_for_cmd_buffer(struct anv_execbuf *execbuf,
}
 }
 
+static void
+setup_empty_execbuf(struct anv_execbuf *execbuf, struct anv_device *device)
+{
+   anv_execbuf_add_bo(execbuf, >trivial_batch_bo, NULL, 0,
+  >alloc);
+
+   execbuf->execbuf = (struct drm_i915_gem_execbuffer2) {
+  .buffers_ptr = (uintptr_t) execbuf->objects,
+  .buffer_count = execbuf->bo_count,
+  .batch_start_offset = 0,
+  .batch_len = 8, /* GEN8_MI_BATCH_BUFFER_END and NOOP */
+  .flags = I915_EXEC_HANDLE_LUT | I915_EXEC_RENDER,
+  .rsvd1 = device->context_id,
+  .rsvd2 = 0,
+   };
+}
+
 VkResult
 anv_cmd_buffer_execbuf(struct anv_device *device,
struct anv_cmd_buffer *cmd_buffer,
@@ -1363,22 +1380,50 @@ anv_cmd_buffer_execbuf(struct anv_device *device,
struct anv_execbuf execbuf;
anv_execbuf_init();
 
+   int in_fence = -1;
for (uint32_t i = 0; i < num_in_semaphores; i++) {
   ANV_FROM_HANDLE(anv_semaphore, semaphore, in_semaphores[i]);
-  assert(semaphore->temporary.type == ANV_SEMAPHORE_TYPE_NONE);
-  struct anv_semaphore_impl *impl = >permanent;
+  struct anv_semaphore_impl *impl =
+ semaphore->temporary.type != ANV_SEMAPHORE_TYPE_NONE ?
+ >temporary : >permanent;
 
   switch (impl->type) {
   case ANV_SEMAPHORE_TYPE_BO:
  anv_execbuf_add_bo(, impl->bo, NULL, 0, >alloc);
  break;
+  case ANV_SEMAPHORE_TYPE_SYNC_FILE:
+ if (in_fence == -1) {
+in_fence = impl->fd;
+ } else {
+int merge = anv_gem_sync_file_merge(device, in_fence, impl->fd);
+if (merge == -1)
+   return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHX);
+
+close(impl->fd);
+close(in_fence);
+in_fence = merge;
+ }
+
+ impl->fd = -1;
   default:
  break;
   }
+
+  /* Waiting on a semaphore with temporary state implicitly resets it back
+   * to the permanent state.
+   */
+  if (semaphore->temporary.type != ANV_SEMAPHORE_TYPE_NONE) {
+ assert(semaphore->temporary.type == ANV_SEMAPHORE_TYPE_SYNC_FILE);
+ semaphore->temporary.type = ANV_SEMAPHORE_TYPE_NONE;
+  }
}
 
+   bool need_out_fence = false;
for (uint32_t i = 0; i < num_out_semaphores; i++) {
   ANV_FROM_HANDLE(anv_semaphore, semaphore, out_semaphores[i]);
+  /* Out fences can't have temporary state because that would imply
+   * that we imported a sync file and are trying to signal it.
+   */
   assert(semaphore->temporary.type == ANV_SEMAPHORE_TYPE_NONE);
   struct anv_semaphore_impl *impl = >permanent;
 
@@ -1387,15 +1432,50 @@ anv_cmd_buffer_execbuf(struct anv_device *device,
  anv_execbuf_add_bo(, impl->bo, NULL, EXEC_OBJECT_WRITE,
 >alloc);
  break;
+
+  case ANV_SEMAPHORE_TYPE_SYNC_FILE:
+ need_out_fence = true;
+ break;
+
   default:
  break;
   }
}
 
-   setup_execbuf_for_cmd_buffer(, cmd_buffer);
+   if (cmd_buffer) {
+  setup_execbuf_for_cmd_buffer(, cmd_buffer);
+   } else {
+  setup_empty_execbuf(, device);
+   }
+
+   if (in_fence != -1) {
+  execbuf.execbuf.flags |= I915_EXEC_FENCE_IN;
+  execbuf.execbuf.rsvd2 |= (uint32_t)in_fence;
+   }
+
+   if (need_out_fence)
+  execbuf.execbuf.flags |= I915_EXEC_FENCE_OUT;
 
VkResult result = anv_device_execbuf(device, , execbuf.bos);
 
+   if (result == VK_SUCCESS && need_out_fence) {
+  int out_fence = execbuf.execbuf.rsvd2 >> 32;
+  for (uint32_t i = 0; i < num_out_semaphores; i++) {
+ ANV_FROM_HANDLE(anv_semaphore, semaphore, out_semaphores[i]);
+ /* Out fences can't have temporary state because that would imply
+  * that we imported a sync file and are trying to signal it.
+  */
+ assert(semaphore->temporary.type == ANV_SEMAPHORE_TYPE_NONE);
+ struct anv_semaphore_impl *impl = >permanent;
+
+ if (impl->type == ANV_SEMAPHORE_TYPE_SYNC_FILE) {
+assert(impl->fd == -1);
+impl->fd = dup(out_fence);
+ }
+  }
+  close(out_fence);
+   }
+
anv_execbuf_finish(, >alloc);
 
return result;
diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index e393596..2cce07d 100644
--- 

Re: [Mesa-dev] [PATCH 3/4] radv: fallback to an in-memory cache when no pipline cache is provided

[Timothy Arceri]

On 15/03/17 15:17, Timothy Arceri wrote:

---
 src/amd/vulkan/radv_device.c | 4 +++-
 src/amd/vulkan/radv_pipeline.c   | 9 ++---
 src/amd/vulkan/radv_pipeline_cache.c | 7 +--
 src/amd/vulkan/radv_private.h| 3 ++-
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 875057c..65e6a2c 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -984,26 +984,28 @@ VkResult radv_CreateDevice(

if (device->physical_device->rad_info.chip_class >= CIK)
cik_create_gfx_config(device);

VkPipelineCacheCreateInfo ci;
ci.sType = VK_STRUCTURE_TYPE_PIPELINE_CACHE_CREATE_INFO;
ci.pNext = NULL;
ci.flags = 0;
ci.pInitialData = NULL;
ci.initialDataSize = 0;
-   VkPipelineCache pc = radv_pipeline_cache_to_handle(device->mem_cache);
+   VkPipelineCache pc;
result = radv_CreatePipelineCache(radv_device_to_handle(device),
  , NULL, );
if (result != VK_SUCCESS)
goto fail;

+   device->mem_cache = radv_pipeline_cache_from_handle(pc);
+
*pDevice = radv_device_to_handle(device);
return VK_SUCCESS;



This hunk was meant to be squashed into the previous commit. Fixed locally.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 3/4] radv: fallback to an in-memory cache when no pipline cache is provided

[Timothy Arceri]

---
 src/amd/vulkan/radv_device.c | 4 +++-
 src/amd/vulkan/radv_pipeline.c   | 9 ++---
 src/amd/vulkan/radv_pipeline_cache.c | 7 +--
 src/amd/vulkan/radv_private.h| 3 ++-
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 875057c..65e6a2c 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -984,26 +984,28 @@ VkResult radv_CreateDevice(
 
if (device->physical_device->rad_info.chip_class >= CIK)
cik_create_gfx_config(device);
 
VkPipelineCacheCreateInfo ci;
ci.sType = VK_STRUCTURE_TYPE_PIPELINE_CACHE_CREATE_INFO;
ci.pNext = NULL;
ci.flags = 0;
ci.pInitialData = NULL;
ci.initialDataSize = 0;
-   VkPipelineCache pc = radv_pipeline_cache_to_handle(device->mem_cache);
+   VkPipelineCache pc;
result = radv_CreatePipelineCache(radv_device_to_handle(device),
  , NULL, );
if (result != VK_SUCCESS)
goto fail;
 
+   device->mem_cache = radv_pipeline_cache_from_handle(pc);
+
*pDevice = radv_device_to_handle(device);
return VK_SUCCESS;
 
 fail:
if (device->trace_bo)
device->ws->buffer_destroy(device->trace_bo);
 
if (device->gfx_init)
device->ws->buffer_destroy(device->gfx_init);
 
diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index 13ae87c..73a3776 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -552,33 +552,36 @@ radv_pipeline_compile(struct radv_pipeline *pipeline,
 , _size, dump);
 
if (stage == MESA_SHADER_GEOMETRY) {
void *gs_copy_code = NULL;
unsigned gs_copy_code_size = 0;
pipeline->gs_copy_shader = radv_pipeline_create_gs_copy_shader(
pipeline, nir, _copy_code, _copy_code_size, dump);
 
if (pipeline->gs_copy_shader) {
pipeline->gs_copy_shader =
-   radv_pipeline_cache_insert_shader(cache,
+   
radv_pipeline_cache_insert_shader(pipeline->device,
+ cache,
  gs_copy_sha1,
  
pipeline->gs_copy_shader,
  gs_copy_code,
  
gs_copy_code_size);
}
}
if (!module->nir)
ralloc_free(nir);
 
if (variant)
-   variant = radv_pipeline_cache_insert_shader(cache, sha1, 
variant,
-   code, code_size);
+   variant = radv_pipeline_cache_insert_shader(pipeline->device,
+   cache, sha1,
+   variant, code,
+   code_size);
 
if (code)
free(code);
return variant;
 }
 
 static VkResult
 radv_pipeline_scratch_init(struct radv_device *device,
struct radv_pipeline *pipeline)
 {
diff --git a/src/amd/vulkan/radv_pipeline_cache.c 
b/src/amd/vulkan/radv_pipeline_cache.c
index 5b7e1c4..3a58f6a 100644
--- a/src/amd/vulkan/radv_pipeline_cache.c
+++ b/src/amd/vulkan/radv_pipeline_cache.c
@@ -149,20 +149,22 @@ radv_pipeline_cache_search(struct radv_pipeline_cache 
*cache,
 
 struct radv_shader_variant *
 radv_create_shader_variant_from_pipeline_cache(struct radv_device *device,
   struct radv_pipeline_cache 
*cache,
   const unsigned char *sha1)
 {
struct cache_entry *entry = NULL;
 
if (cache)
entry = radv_pipeline_cache_search(cache, sha1);
+   else
+   entry = radv_pipeline_cache_search(device->mem_cache, sha1);
 
if (!entry)
return NULL;
 
if (!entry->variant) {
struct radv_shader_variant *variant;
 
variant = calloc(1, sizeof(struct radv_shader_variant));
if (!variant)
return NULL;
@@ -251,27 +253,28 @@ radv_pipeline_cache_add_entry(struct radv_pipeline_cache 
*cache,
radv_pipeline_cache_grow(cache);
 
/* Failing to grow that hash table isn't fatal, but may mean we don't
 * have enough space to add this new kernel. Only add it if there's 
room.
 */
if (cache->kernel_count < cache->table_size / 2)
radv_pipeline_cache_set_entry(cache, entry);
 }
 
 

[Mesa-dev] [PATCH 1/4] radv: move cache check inside insert and search functions

[Timothy Arceri]

This will allow us to use fallback in-memory and on-disk caches
should the app not provide a pipeline cache.
---
 src/amd/vulkan/radv_pipeline.c   | 28 +---
 src/amd/vulkan/radv_pipeline_cache.c |  8 +++-
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index df76396..13ae87c 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -521,64 +521,62 @@ radv_pipeline_compile(struct radv_pipeline *pipeline,
if (module->nir)
_mesa_sha1_compute(module->nir->info->name,
   strlen(module->nir->info->name),
   module->sha1);
 
radv_hash_shader(sha1, module, entrypoint, spec_info, layout, key, 0);
if (stage == MESA_SHADER_GEOMETRY)
radv_hash_shader(gs_copy_sha1, module, entrypoint, spec_info,
 layout, key, 1);
 
-   if (cache) {
-   variant = 
radv_create_shader_variant_from_pipeline_cache(pipeline->device,
-cache,
-sha1);
+   variant = 
radv_create_shader_variant_from_pipeline_cache(pipeline->device,
+cache,
+sha1);
 
-   if (stage == MESA_SHADER_GEOMETRY) {
-   pipeline->gs_copy_shader =
-   radv_create_shader_variant_from_pipeline_cache(
-   pipeline->device,
-   cache,
-   gs_copy_sha1);
-   }
-   if (variant)
-   return variant;
+   if (stage == MESA_SHADER_GEOMETRY) {
+   pipeline->gs_copy_shader =
+   radv_create_shader_variant_from_pipeline_cache(
+   pipeline->device,
+   cache,
+   gs_copy_sha1);
}
+   if (variant)
+   return variant;
 
nir = radv_shader_compile_to_nir(pipeline->device,
 module, entrypoint, stage,
 spec_info, dump);
if (nir == NULL)
return NULL;
 
variant = radv_shader_variant_create(pipeline->device, nir, layout, key,
 , _size, dump);
 
if (stage == MESA_SHADER_GEOMETRY) {
void *gs_copy_code = NULL;
unsigned gs_copy_code_size = 0;
pipeline->gs_copy_shader = radv_pipeline_create_gs_copy_shader(
pipeline, nir, _copy_code, _copy_code_size, dump);
 
-   if (pipeline->gs_copy_shader && cache) {
+   if (pipeline->gs_copy_shader) {
pipeline->gs_copy_shader =
radv_pipeline_cache_insert_shader(cache,
  gs_copy_sha1,
  
pipeline->gs_copy_shader,
  gs_copy_code,
  
gs_copy_code_size);
}
}
if (!module->nir)
ralloc_free(nir);
 
-   if (variant && cache)
+   if (variant)
variant = radv_pipeline_cache_insert_shader(cache, sha1, 
variant,
code, code_size);
 
if (code)
free(code);
return variant;
 }
 
 static VkResult
 radv_pipeline_scratch_init(struct radv_device *device,
diff --git a/src/amd/vulkan/radv_pipeline_cache.c 
b/src/amd/vulkan/radv_pipeline_cache.c
index 30f2dc1..5b7e1c4 100644
--- a/src/amd/vulkan/radv_pipeline_cache.c
+++ b/src/amd/vulkan/radv_pipeline_cache.c
@@ -145,21 +145,24 @@ radv_pipeline_cache_search(struct radv_pipeline_cache 
*cache,
pthread_mutex_unlock(>mutex);
 
return entry;
 }
 
 struct radv_shader_variant *
 radv_create_shader_variant_from_pipeline_cache(struct radv_device *device,
   struct radv_pipeline_cache 
*cache,
   const unsigned char *sha1)
 {
-   struct cache_entry *entry = radv_pipeline_cache_search(cache, sha1);
+   struct cache_entry *entry = NULL;
+
+   if (cache)
+   entry = radv_pipeline_cache_search(cache, sha1);
 
if (!entry)
return NULL;
 
if (!entry->variant) {
struct radv_shader_variant *variant;
 
variant = 

[Mesa-dev] [PATCH 2/4] radv: always create an fallback pipeline cache

[Timothy Arceri]

This will be used as an in-memory cache when a pipeline cache is
not provided by the app.
---
 src/amd/vulkan/radv_device.c  | 15 +++
 src/amd/vulkan/radv_private.h |  3 +++
 2 files changed, 18 insertions(+)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index d1fd58d..875057c 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -978,20 +978,32 @@ VkResult radv_CreateDevice(
goto fail;
 
device->trace_id_ptr = device->ws->buffer_map(device->trace_bo);
if (!device->trace_id_ptr)
goto fail;
}
 
if (device->physical_device->rad_info.chip_class >= CIK)
cik_create_gfx_config(device);
 
+   VkPipelineCacheCreateInfo ci;
+   ci.sType = VK_STRUCTURE_TYPE_PIPELINE_CACHE_CREATE_INFO;
+   ci.pNext = NULL;
+   ci.flags = 0;
+   ci.pInitialData = NULL;
+   ci.initialDataSize = 0;
+   VkPipelineCache pc = radv_pipeline_cache_to_handle(device->mem_cache);
+   result = radv_CreatePipelineCache(radv_device_to_handle(device),
+ , NULL, );
+   if (result != VK_SUCCESS)
+   goto fail;
+
*pDevice = radv_device_to_handle(device);
return VK_SUCCESS;
 
 fail:
if (device->trace_bo)
device->ws->buffer_destroy(device->trace_bo);
 
if (device->gfx_init)
device->ws->buffer_destroy(device->gfx_init);
 
@@ -1026,20 +1038,23 @@ void radv_DestroyDevice(
radv_queue_finish(>queues[i][q]);
if (device->queue_count[i])
vk_free(>alloc, device->queues[i]);
if (device->empty_cs[i])
device->ws->cs_destroy(device->empty_cs[i]);
if (device->flush_cs[i])
device->ws->cs_destroy(device->flush_cs[i]);
}
radv_device_finish_meta(device);
 
+   VkPipelineCache pc = radv_pipeline_cache_to_handle(device->mem_cache);
+   radv_DestroyPipelineCache(radv_device_to_handle(device), pc, NULL);
+
vk_free(>alloc, device);
 }
 
 VkResult radv_EnumerateInstanceExtensionProperties(
const char* pLayerName,
uint32_t*   pPropertyCount,
VkExtensionProperties*  pProperties)
 {
if (pProperties == NULL) {
*pPropertyCount = ARRAY_SIZE(instance_extensions);
diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
index 890e6b8..c11fb74 100644
--- a/src/amd/vulkan/radv_private.h
+++ b/src/amd/vulkan/radv_private.h
@@ -501,20 +501,23 @@ struct radv_device {
float sample_locations_16x[16][2];
 
/* CIK and later */
uint32_t gfx_init_size_dw;
struct radeon_winsys_bo  *gfx_init;
 
struct radeon_winsys_bo  *trace_bo;
uint32_t *trace_id_ptr;
 
struct radv_physical_device  *physical_device;
+
+   /* Backup in-memory cache to be used if the app doesn't provide one */
+   struct radv_pipeline_cache *mem_cache;
 };
 
 struct radv_device_memory {
struct radeon_winsys_bo  *bo;
/* for dedicated allocations */
struct radv_image*image;
struct radv_buffer   *buffer;
uint32_t type_index;
VkDeviceSize map_size;
void *   map;
-- 
2.9.3

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

[Mesa-dev] [PATCH 4/4] radv: make sure gs copy shader is retrieved from the cache with the variant

[Timothy Arceri]

Apps can limit the size of the cache via VkAllocationCallbacks so we
can't be sure that both are always in the cache.
---
 src/amd/vulkan/radv_pipeline.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index 73a3776..0eeb535 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -532,33 +532,38 @@ radv_pipeline_compile(struct radv_pipeline *pipeline,
 cache,
 sha1);
 
if (stage == MESA_SHADER_GEOMETRY) {
pipeline->gs_copy_shader =
radv_create_shader_variant_from_pipeline_cache(
pipeline->device,
cache,
gs_copy_sha1);
}
-   if (variant)
+
+   if (variant &&
+   (stage != MESA_SHADER_GEOMETRY || pipeline->gs_copy_shader))
return variant;
 
nir = radv_shader_compile_to_nir(pipeline->device,
 module, entrypoint, stage,
 spec_info, dump);
if (nir == NULL)
return NULL;
 
-   variant = radv_shader_variant_create(pipeline->device, nir, layout, key,
-, _size, dump);
+   if (!variant) {
+   variant = radv_shader_variant_create(pipeline->device, nir,
+layout, key, ,
+_size, dump);
+   }
 
-   if (stage == MESA_SHADER_GEOMETRY) {
+   if (stage == MESA_SHADER_GEOMETRY && !pipeline->gs_copy_shader) {
void *gs_copy_code = NULL;
unsigned gs_copy_code_size = 0;
pipeline->gs_copy_shader = radv_pipeline_create_gs_copy_shader(
pipeline, nir, _copy_code, _copy_code_size, dump);
 
if (pipeline->gs_copy_shader) {
pipeline->gs_copy_shader =

radv_pipeline_cache_insert_shader(pipeline->device,
  cache,
  gs_copy_sha1,
-- 
2.9.3

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

Re: [Mesa-dev] MESA and KOTOR

[Federico Dossena]

The game still crashes at the same null pointer, but I've seen it use 
that function you changed so I guess I'll keep the patch just to be safe.


Are there any patches that touch wglMakeContextCurrentARB and didn't 
make it to master?


(Resent the email because thunderbird removed the mesa-dev cc)


Il 2017-03-14 03:44, Brian Paul ha scritto:
Looks like my KOTOR patch never made it to master.  I'm attaching it 
below so you can try it.  I should commit it master.  In any case, let 
me know if it helps.


-Brian

On 03/13/2017 10:55 AM, Federico Dossena wrote:

Hi Jose, thanks for replying, I've seen your name inside many files in
mesa

I have tried mesa master (previously I was using 17.0.1) but it still
crashes for the same null pointer.
Do you have a link to that patch you've mentioned for kotor?

I have used apitrace and took traces of both the nvidia driver (which
runs kotor) and mesa (up until the crash).
Here's a link to them:
https://drive.google.com/file/d/0B6qj91UlSYlYa3dIM0FtaHNULW8/view?usp=sharing 

 



I tried reading them with the dump function but it's way above my
comprehension.

I know that some applications use wglGetProcAddress to check if an
extension if available, but I've seen KOTOR check for the
WGL_ARB_render_texture, and when it's present it enables frame buffer
effects and soft shadows, which use wglMakeContextCurrentARB (not
wglBindTexImageARB, I was wrong in my previous mail), which for some
reason is a null pointer.




Il 2017-03-13 14:39, Jose Fonseca ha scritto:

On 13/03/17 11:09, Emil Velikov wrote:

On 11 March 2017 at 11:51, Federico Dossena 
wrote:

In the last week I've been trying to bring an "old" game back to
life, Star
Wars Knights of the old republic (KOTOR, for short). It's from 2003
and uses
OpenGL 1.4.

I have used Mesa, libtxc_dxtn and some trickery to decompress the
textures
to boost performance, and right now I have it up and running
smoothly with
Gallium on LLVMPipe, compiled on Windows. (I can upload a copy if
someone is
interested). This took me about 2 days of compiling and figuring out
stuff.

Here's where the weirdness begins:
Turning on framebuffer effects or soft shadows make the game crash
right
after the menu. Using a disassembler and debugger and what little
knowledge
I have of reverse engineering, I managed to track down the issue to a
function which uses wglGetProcAddress to get the addresses of
several OpenGL
functions. Some of these calls return a null pointer (even if there
is a
valid context and it is current), and when the game tries to call
them, it
crashes. The first one that makes it crash is a pointer to
wglBindTexImageARB, but there are a few others. NOPing the offending
instructions did not work, and returning a nop function just makes
the game
display artifacts.


Strange - afaict mesa (st/wgl) exposes both wglBindTexImageARB and the
WGL_ARB_render_texture extension.
You can break on DrvGetProcAddress and trace where/how we end up with
NULL function pointer.

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



Federico,

You should be using latest master for this.   There have been recent
changes/fixes to our WGL implementation.


Last fall Brian Paul fixed an issue with WGL extension seen on KOTOR.
I'm not sure the the issue has been crossported to Mesa master yet,
and it might be unrelated.


Generally speaking, wglGetProcAddress returning NULL by itself is not
a problem.  Many games wrongly rely on wglGetProcAddress NULL results
to detect whether an GL/WGL extension is present (which goes against
the spec).  Other libraries try to bindly get every possible
entrypoint through wglGetProcAddress, then check which ones to use
based on supported extensions (which is actually fine by the spec.)


For the record, getting an apitrace is usually useful to debug this
sort of issues.  One can use apitrace straigh from windows or with
WINE -- https://github.com/apitrace/apitrace/wiki/WINE


Jose




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=Ie7_encNUsqxbSRbqbNgofw0ITcfE8JKfaUjIQhncGA=u9OxTt6a0b4XxAVoFjjG7RmQNYLAIe3smaD2NtY0mhE=jnsrLdbWwBr7d8cUeUr_dxHK8sN25_6TfLQjoVbMCj8= 





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

Re: [Mesa-dev] MESA and KOTOR

[Federico Dossena]

The game runs smoothly if you decompress the textures, I'm getting 30-40 
fps with llvmpipe and no "grass in the sky" artifacts.
The problem is frame buffer effects and soft shadows, were they enabled 
when you tested it? If so, how did you make it use llvmpipe?



Il 2017-03-13 22:40, Jan Vesely ha scritto:

On Mon, 2017-03-13 at 11:40 +0200, Grazvydas Ignotas wrote:

On Sat, Mar 11, 2017 at 1:51 PM, Federico Dossena  wrote:

This issue is not new: a guy named Miklós Máté, here in the Mesa mailing
list somehow managed to fix it in Mesa 11, but his patches do not seem to
work anymore
(https://lists.freedesktop.org/archives/mesa-dev/2016-February/106695.html).

He was using wine on Linux, I guess wine provides an implementation of
that function. You could try running the game on Linux too.

runs mostly OK using wine here. Intel(skylake) driver suffers terrible
performance and displays artifacts in certain locations. nouveau(950m)
works nicely. llvmpipe+llvm3.7 also works, albeit the performance is
terrible.

Jan


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


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

Re: [Mesa-dev] MESA and KOTOR

[Federico Dossena]

Hello again,

After further investigation it seems to me that wglMakeContextCurrentARB 
is not implemented by gallium llvmpipe, that's why it gets a null pointer.


Can we add a stub or something?


Il 2017-03-14 03:44, Brian Paul ha scritto:
Looks like my KOTOR patch never made it to master.  I'm attaching it 
below so you can try it.  I should commit it master.  In any case, let 
me know if it helps.


-Brian

On 03/13/2017 10:55 AM, Federico Dossena wrote:

Hi Jose, thanks for replying, I've seen your name inside many files in
mesa ;)

I have tried mesa master (previously I was using 17.0.1) but it still
crashes for the same null pointer.
Do you have a link to that patch you've mentioned for kotor?

I have used apitrace and took traces of both the nvidia driver (which
runs kotor) and mesa (up until the crash).
Here's a link to them:
https://drive.google.com/file/d/0B6qj91UlSYlYa3dIM0FtaHNULW8/view?usp=sharing 

 



I tried reading them with the dump function but it's way above my
comprehension.

I know that some applications use wglGetProcAddress to check if an
extension if available, but I've seen KOTOR check for the
WGL_ARB_render_texture, and when it's present it enables frame buffer
effects and soft shadows, which use wglMakeContextCurrentARB (not
wglBindTexImageARB, I was wrong in my previous mail), which for some
reason is a null pointer.




Il 2017-03-13 14:39, Jose Fonseca ha scritto:

On 13/03/17 11:09, Emil Velikov wrote:

On 11 March 2017 at 11:51, Federico Dossena 
wrote:

In the last week I've been trying to bring an "old" game back to
life, Star
Wars Knights of the old republic (KOTOR, for short). It's from 2003
and uses
OpenGL 1.4.

I have used Mesa, libtxc_dxtn and some trickery to decompress the
textures
to boost performance, and right now I have it up and running
smoothly with
Gallium on LLVMPipe, compiled on Windows. (I can upload a copy if
someone is
interested). This took me about 2 days of compiling and figuring out
stuff.

Here's where the weirdness begins:
Turning on framebuffer effects or soft shadows make the game crash
right
after the menu. Using a disassembler and debugger and what little
knowledge
I have of reverse engineering, I managed to track down the issue to a
function which uses wglGetProcAddress to get the addresses of
several OpenGL
functions. Some of these calls return a null pointer (even if there
is a
valid context and it is current), and when the game tries to call
them, it
crashes. The first one that makes it crash is a pointer to
wglBindTexImageARB, but there are a few others. NOPing the offending
instructions did not work, and returning a nop function just makes
the game
display artifacts.


Strange - afaict mesa (st/wgl) exposes both wglBindTexImageARB and the
WGL_ARB_render_texture extension.
You can break on DrvGetProcAddress and trace where/how we end up with
NULL function pointer.

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



Federico,

You should be using latest master for this.   There have been recent
changes/fixes to our WGL implementation.


Last fall Brian Paul fixed an issue with WGL extension seen on KOTOR.
I'm not sure the the issue has been crossported to Mesa master yet,
and it might be unrelated.


Generally speaking, wglGetProcAddress returning NULL by itself is not
a problem.  Many games wrongly rely on wglGetProcAddress NULL results
to detect whether an GL/WGL extension is present (which goes against
the spec).  Other libraries try to bindly get every possible
entrypoint through wglGetProcAddress, then check which ones to use
based on supported extensions (which is actually fine by the spec.)


For the record, getting an apitrace is usually useful to debug this
sort of issues.  One can use apitrace straigh from windows or with
WINE -- https://github.com/apitrace/apitrace/wiki/WINE


Jose




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=Ie7_encNUsqxbSRbqbNgofw0ITcfE8JKfaUjIQhncGA=u9OxTt6a0b4XxAVoFjjG7RmQNYLAIe3smaD2NtY0mhE=jnsrLdbWwBr7d8cUeUr_dxHK8sN25_6TfLQjoVbMCj8= 







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

Re: [Mesa-dev] mx6q: Cannot run Cinematic demo correctly

[Otavio Salvador]

On Tue, Mar 14, 2017 at 2:53 PM, Fabio Estevam  wrote:
...
> Care to submit the revert for the two patches?

I think Christian is going to proper fix it in next days; as we have a
workaround for now I see no reason to speed this up.

-- 
Otavio Salvador O.S. Systems
http://www.ossystems.com.brhttp://code.ossystems.com.br
Mobile: +55 (53) 9981-7854Mobile: +1 (347) 903-9750
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] mx6q: Cannot run Cinematic demo correctly

[Otavio Salvador]

On Tue, Mar 14, 2017 at 12:31 PM, Fabio Estevam  wrote:
...
> The cover of the movies are just black rectangles instead of showing
> the actual movie pictures.
...

I am attaching the two patches I am applying on mesa locally and which
has fixed our current issues.

-- 
Otavio Salvador O.S. Systems
http://www.ossystems.com.brhttp://code.ossystems.com.br
Mobile: +55 (53) 9981-7854Mobile: +1 (347) 903-9750
From ee18f89c49927f6696df2d557837acc9bfd71f71 Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" 
Date: Fri, 3 Feb 2017 09:30:53 +
Subject: [PATCH 2/2] Revert "Subject: [PATCH] etnaviv: add RGBX/RGBA pixel
 formats"
Organization: O.S. Systems Software LTDA.

This reverts commit 89bb5c79e29613ad9a4e43d670654e98a220fc60.

Signed-off-by: Otavio Salvador 
---
 src/gallium/drivers/etnaviv/etnaviv_format.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/gallium/drivers/etnaviv/etnaviv_format.c b/src/gallium/drivers/etnaviv/etnaviv_format.c
index 0794603b2f..693558476b 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_format.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_format.c
@@ -141,8 +141,6 @@ static struct etna_format formats[PIPE_FORMAT_COUNT] = {
V_(R16G16_SSCALED, SHORT,  NONE),
V_(R16G16_FLOAT,   HALF_FLOAT, NONE),
 
-   V_(A8B8G8R8_UNORM,   UNSIGNED_BYTE, NONE),
-
V_(R8G8B8A8_UNORM,   UNSIGNED_BYTE, A8B8G8R8),
V_(R8G8B8A8_SNORM,   BYTE,  A8B8G8R8),
_T(R8G8B8X8_UNORM,   X8B8G8R8,  X8B8G8R8),
@@ -151,9 +149,6 @@ static struct etna_format formats[PIPE_FORMAT_COUNT] = {
V_(R8G8B8A8_USCALED, UNSIGNED_BYTE, A8B8G8R8),
V_(R8G8B8A8_SSCALED, BYTE,  A8B8G8R8),
 
-   _T(R8G8B8A8_UNORM, A8B8G8R8, A8B8G8R8),
-   _T(R8G8B8X8_UNORM, X8B8G8R8, X8B8G8R8),
-
_T(B8G8R8A8_UNORM, A8R8G8B8, A8R8G8B8),
_T(B8G8R8X8_UNORM, X8R8G8B8, X8R8G8B8),
 
-- 
2.12.0

From 309d4f0f11bac24a0b487404c51564735ac2c9ed Mon Sep 17 00:00:00 2001
From: Otavio Salvador 
Date: Tue, 14 Mar 2017 09:51:23 -0300
Subject: [PATCH 1/2] Revert "etnaviv: Cannot render to rb-swapped formats"
Organization: O.S. Systems Software LTDA.

This reverts commit 6c89a728d9e5d072cb504453e73077564c6523d3.

Signed-off-by: Otavio Salvador 
---
 src/gallium/drivers/etnaviv/etnaviv_screen.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c b/src/gallium/drivers/etnaviv/etnaviv_screen.c
index 28db9b95bf..2298936f51 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
@@ -469,11 +469,8 @@ etna_screen_is_format_supported(struct pipe_screen *pscreen,
   return FALSE;
 
if (usage & PIPE_BIND_RENDER_TARGET) {
-  /* If render target, must be RS-supported format that is not rb swapped.
-   * Exposing rb swapped (or other swizzled) formats for rendering would
-   * involve swizzing in the pixel shader.
-   */
-  if (translate_rs_format(format) != ETNA_NO_MATCH && !translate_rs_format_rb_swap(format)) {
+  /* if render target, must be RS-supported format */
+  if (translate_rs_format(format) != ETNA_NO_MATCH) {
  /* Validate MSAA; number of samples must be allowed, and render target
   * must have MSAA'able format. */
  if (sample_count > 1) {
-- 
2.12.0

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

Re: [Mesa-dev] mx6q: Cannot run Cinematic demo correctly

[Otavio Salvador]

On Tue, Mar 14, 2017 at 2:39 PM, Christian Gmeiner
 wrote:
> 2017-03-14 18:16 GMT+01:00 Fabio Estevam :
...
> shader variants - due to lot of people are have this issue I will
> spend some time the next
> days to cleanup my wip patch series. I have too may concurrent etnaviv
> work items running
> in parallel :(

I can easily test a proposed patchset or Git tree against our Yocto
Project build.

-- 
Otavio Salvador O.S. Systems
http://www.ossystems.com.brhttp://code.ossystems.com.br
Mobile: +55 (53) 9981-7854Mobile: +1 (347) 903-9750
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] radeonsi: disable sinking common instructions down to the end block

[Samuel Pitoiset]

On 03/15/2017 01:59 AM, Matt Arsenault wrote:



On Mar 14, 2017, at 17:21, Samuel Pitoiset  wrote:

Initially this was a workaround for a bug introduced in LLVM 4.0
in the SimplifyCFG pass that caused image instrinsics to disappear
(because they were badly sunk). Finally, this is a win because it
decreases SGPR spilling and increases the number of waves a bit.

Although, shader-db results are good I think we might want to
remove it in the future once the issue is fixed. For now, enable
it for LLVM >= 4.0.

This also fixes a rendering issue with the speedometer in Dirt Rally.

More information can be found here https://reviews.llvm.org/D26348.

Thanks to Dave Airlie for the patch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99484
Signed-off-by: Samuel Pitoiset 
---

For those interested, here's the full shader-db report:

https://hastebin.com/osepehehat.pas

src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 6 ++
1 file changed, 6 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c 
b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
index 5c63b732b3..4fb7a126e4 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
@@ -40,6 +40,7 @@
#include 
#include 
#include 
+#include 

/* Data for if/else/endif and bgnloop/endloop control flow structures.
 */
@@ -124,6 +125,11 @@ static void init_amdgpu_target()
LLVMInitializeAMDGPUTarget();
LLVMInitializeAMDGPUTargetMC();
LLVMInitializeAMDGPUAsmPrinter();
+
+#if HAVE_LLVM >= 0x0400
+   const char *argv[2] = {"mesa", "-simplifycfg-sink-common=false"};
+   LLVMParseCommandLineOptions(2, argv, NULL);
+#endif
}


Probably should have a FIXME that this is a workaround and a link to an open 
bug to really fix these issues


Okay, I will change it locally.



-Matt


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

[Mesa-dev] [PATCH 1/2] radv/formats: reverse how the image format properties KHR2 is handled

[Dave Airlie]

From: Dave Airlie 

This just aligns with how anv does it.

Signed-off-by: Dave Airlie 
---
 src/amd/vulkan/radv_formats.c | 76 ++-
 1 file changed, 46 insertions(+), 30 deletions(-)

diff --git a/src/amd/vulkan/radv_formats.c b/src/amd/vulkan/radv_formats.c
index 9b21e04..e4cc4ba 100644
--- a/src/amd/vulkan/radv_formats.c
+++ b/src/amd/vulkan/radv_formats.c
@@ -1006,16 +1006,11 @@ void radv_GetPhysicalDeviceFormatProperties2KHR(
   
>formatProperties);
 }
 
-VkResult radv_GetPhysicalDeviceImageFormatProperties(
-   VkPhysicalDevicephysicalDevice,
-   VkFormatformat,
-   VkImageType type,
-   VkImageTiling   tiling,
-   VkImageUsageFlags   usage,
-   VkImageCreateFlags  createFlags,
-   VkImageFormatProperties*pImageFormatProperties)
+static VkResult radv_get_image_format_properties(struct radv_physical_device 
*physical_device,
+const 
VkPhysicalDeviceImageFormatInfo2KHR *info,
+VkImageFormatProperties 
*pImageFormatProperties)
+
 {
-   RADV_FROM_HANDLE(radv_physical_device, physical_device, physicalDevice);
VkFormatProperties format_props;
VkFormatFeatureFlags format_feature_flags;
VkExtent3D maxExtent;
@@ -1023,11 +1018,11 @@ VkResult radv_GetPhysicalDeviceImageFormatProperties(
uint32_t maxArraySize;
VkSampleCountFlags sampleCounts = VK_SAMPLE_COUNT_1_BIT;
 
-   radv_physical_device_get_format_properties(physical_device, format,
+   radv_physical_device_get_format_properties(physical_device, 
info->format,
   _props);
-   if (tiling == VK_IMAGE_TILING_LINEAR) {
+   if (info->tiling == VK_IMAGE_TILING_LINEAR) {
format_feature_flags = format_props.linearTilingFeatures;
-   } else if (tiling == VK_IMAGE_TILING_OPTIMAL) {
+   } else if (info->tiling == VK_IMAGE_TILING_OPTIMAL) {
format_feature_flags = format_props.optimalTilingFeatures;
} else {
unreachable("bad VkImageTiling");
@@ -1036,7 +1031,7 @@ VkResult radv_GetPhysicalDeviceImageFormatProperties(
if (format_feature_flags == 0)
goto unsupported;
 
-   switch (type) {
+   switch (info->type) {
default:
unreachable("bad vkimage type\n");
case VK_IMAGE_TYPE_1D:
@@ -1062,34 +1057,34 @@ VkResult radv_GetPhysicalDeviceImageFormatProperties(
break;
}
 
-   if (tiling == VK_IMAGE_TILING_OPTIMAL &&
-   type == VK_IMAGE_TYPE_2D &&
+   if (info->tiling == VK_IMAGE_TILING_OPTIMAL &&
+   info->type == VK_IMAGE_TYPE_2D &&
(format_feature_flags & (VK_FORMAT_FEATURE_COLOR_ATTACHMENT_BIT |
 
VK_FORMAT_FEATURE_DEPTH_STENCIL_ATTACHMENT_BIT)) &&
-   !(createFlags & VK_IMAGE_CREATE_CUBE_COMPATIBLE_BIT) &&
-   !(usage & VK_IMAGE_USAGE_STORAGE_BIT)) {
+   !(info->flags & VK_IMAGE_CREATE_CUBE_COMPATIBLE_BIT) &&
+   !(info->usage & VK_IMAGE_USAGE_STORAGE_BIT)) {
sampleCounts |= VK_SAMPLE_COUNT_2_BIT | VK_SAMPLE_COUNT_4_BIT | 
VK_SAMPLE_COUNT_8_BIT;
}
 
-   if (usage & VK_IMAGE_USAGE_SAMPLED_BIT) {
+   if (info->usage & VK_IMAGE_USAGE_SAMPLED_BIT) {
if (!(format_feature_flags & 
VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT)) {
goto unsupported;
}
}
 
-   if (usage & VK_IMAGE_USAGE_STORAGE_BIT) {
+   if (info->usage & VK_IMAGE_USAGE_STORAGE_BIT) {
if (!(format_feature_flags & 
VK_FORMAT_FEATURE_STORAGE_IMAGE_BIT)) {
goto unsupported;
}
}
 
-   if (usage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) {
+   if (info->usage & VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT) {
if (!(format_feature_flags & 
VK_FORMAT_FEATURE_COLOR_ATTACHMENT_BIT)) {
goto unsupported;
}
}
 
-   if (usage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) {
+   if (info->usage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) {
if (!(format_feature_flags & 
VK_FORMAT_FEATURE_DEPTH_STENCIL_ATTACHMENT_BIT)) {
goto unsupported;
}
@@ -1120,18 +1115,39 @@ unsupported:
return VK_ERROR_FORMAT_NOT_SUPPORTED;
 }
 
+VkResult radv_GetPhysicalDeviceImageFormatProperties(
+   VkPhysicalDevicephysicalDevice,
+   VkFormatformat,
+   VkImageType 

[Mesa-dev] [PATCH 2/2] radv: add external memory support.

[Dave Airlie]

From: Dave Airlie 

This adds support for exporting 2D images, to an
opaque fd.

This implements the:
VK_KHX_external_memory_capabilities
VK_KHX_external_memory
VK_KHX_external_memory_fd

extensions.

These are used by SteamVR, we should work with anv
to decide if we should ship these under an env
var or something.

Signed-off-by: Dave Airlie 
---
 src/amd/vulkan/radv_device.c   |  85 ++-
 src/amd/vulkan/radv_entrypoints_gen.py |   3 +
 src/amd/vulkan/radv_formats.c  | 104 -
 3 files changed, 178 insertions(+), 14 deletions(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index d1fd58d..7266d0a 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -84,6 +84,18 @@ static const VkExtensionProperties instance_extensions[] = {
.specVersion = 5,
},
 #endif
+   {
+   .extensionName = 
VK_KHR_GET_PHYSICAL_DEVICE_PROPERTIES_2_EXTENSION_NAME,
+   .specVersion = 1,
+   },
+   {
+   .extensionName = 
VK_KHX_EXTERNAL_MEMORY_CAPABILITIES_EXTENSION_NAME,
+   .specVersion = 1,
+   },
+   {
+   .extensionName = 
VK_KHX_EXTERNAL_SEMAPHORE_CAPABILITIES_EXTENSION_NAME,
+   .specVersion = 1,
+   },
 };
 
 static const VkExtensionProperties common_device_extensions[] = {
@@ -115,6 +127,18 @@ static const VkExtensionProperties 
common_device_extensions[] = {
.extensionName = VK_NV_DEDICATED_ALLOCATION_EXTENSION_NAME,
.specVersion = 1,
},
+   {
+   .extensionName = 
VK_KHX_EXTERNAL_MEMORY_CAPABILITIES_EXTENSION_NAME,
+   .specVersion = 1,
+   },
+   {
+   .extensionName = VK_KHX_EXTERNAL_MEMORY_EXTENSION_NAME,
+   .specVersion = 1,
+   },
+   {
+   .extensionName = VK_KHX_EXTERNAL_MEMORY_FD_EXTENSION_NAME,
+   .specVersion = 1,
+   },
 };
 
 static VkResult
@@ -255,7 +279,6 @@ radv_physical_device_finish(struct radv_physical_device 
*device)
close(device->local_fd);
 }
 
-
 static void *
 default_alloc_func(void *pUserData, size_t size, size_t align,
VkSystemAllocationScope allocationScope)
@@ -1694,7 +1717,7 @@ VkResult radv_AllocateMemory(
VkResult result;
enum radeon_bo_domain domain;
uint32_t flags = 0;
-   const VkDedicatedAllocationMemoryAllocateInfoNV *dedicate_info = NULL;
+
assert(pAllocateInfo->sType == VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO);
 
if (pAllocateInfo->allocationSize == 0) {
@@ -1703,15 +1726,10 @@ VkResult radv_AllocateMemory(
return VK_SUCCESS;
}
 
-   vk_foreach_struct(ext, pAllocateInfo->pNext) {
-   switch (ext->sType) {
-   case 
VK_STRUCTURE_TYPE_DEDICATED_ALLOCATION_MEMORY_ALLOCATE_INFO_NV:
-   dedicate_info = (const 
VkDedicatedAllocationMemoryAllocateInfoNV *)ext;
-   break;
-   default:
-   break;
-   }
-   }
+   const VkImportMemoryFdInfoKHX *import_info =
+   vk_find_struct_const(pAllocateInfo->pNext, 
IMPORT_MEMORY_FD_INFO_KHX);
+   const VkDedicatedAllocationMemoryAllocateInfoNV *dedicate_info =
+   vk_find_struct_const(pAllocateInfo->pNext, 
DEDICATED_ALLOCATION_MEMORY_ALLOCATE_INFO_NV);
 
mem = vk_alloc2(>alloc, pAllocator, sizeof(*mem), 8,
  VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
@@ -1726,6 +1744,17 @@ VkResult radv_AllocateMemory(
mem->buffer = NULL;
}
 
+   if (import_info) {
+   assert(import_info->handleType ==
+  VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHX);
+   mem->bo = device->ws->buffer_from_fd(device->ws, 
import_info->fd,
+NULL, NULL);
+   if (!mem->bo)
+   goto fail;
+   else
+   goto out_success;
+   }
+
uint64_t alloc_size = align_u64(pAllocateInfo->allocationSize, 4096);
if (pAllocateInfo->memoryTypeIndex == RADV_MEM_TYPE_GTT_WRITE_COMBINE ||
pAllocateInfo->memoryTypeIndex == RADV_MEM_TYPE_GTT_CACHED)
@@ -1749,7 +1778,7 @@ VkResult radv_AllocateMemory(
goto fail;
}
mem->type_index = pAllocateInfo->memoryTypeIndex;
-
+out_success:
*pMem = radv_device_memory_to_handle(mem);
 
return VK_SUCCESS;
@@ -2695,7 +2724,6 @@ void radv_DestroySampler(
vk_free2(>alloc, pAllocator, sampler);
 }
 
-
 /* vk_icd.h does not declare this function, so we declare it here to
  * suppress Wmissing-prototypes.
  */
@@ -2739,3 +2767,34 @@ vk_icdNegotiateLoaderICDInterfaceVersion(uint32_t 
*pSupportedVersion)
*pSupportedVersion = 

[Mesa-dev] [PATCH] radeonsi: disable sinking common instructions down to the end block

[Samuel Pitoiset]

Initially this was a workaround for a bug introduced in LLVM 4.0
in the SimplifyCFG pass that caused image instrinsics to disappear
(because they were badly sunk). Finally, this is a win because it
decreases SGPR spilling and increases the number of waves a bit.

Although, shader-db results are good I think we might want to
remove it in the future once the issue is fixed. For now, enable
it for LLVM >= 4.0.

This also fixes a rendering issue with the speedometer in Dirt Rally.

More information can be found here https://reviews.llvm.org/D26348.

Thanks to Dave Airlie for the patch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99484
Signed-off-by: Samuel Pitoiset 
---

For those interested, here's the full shader-db report:

https://hastebin.com/osepehehat.pas

 src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c 
b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
index 5c63b732b3..4fb7a126e4 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Data for if/else/endif and bgnloop/endloop control flow structures.
  */
@@ -124,6 +125,11 @@ static void init_amdgpu_target()
LLVMInitializeAMDGPUTarget();
LLVMInitializeAMDGPUTargetMC();
LLVMInitializeAMDGPUAsmPrinter();
+
+#if HAVE_LLVM >= 0x0400
+   const char *argv[2] = {"mesa", "-simplifycfg-sink-common=false"};
+   LLVMParseCommandLineOptions(2, argv, NULL);
+#endif
 }
 
 static once_flag init_amdgpu_target_once_flag = ONCE_FLAG_INIT;
-- 
2.12.0

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

Re: [Mesa-dev] [PATCH 2/7] isl: Refactor row pitch calculation (v2)

[Nanley Chery]

On Mon, Mar 13, 2017 at 03:27:59PM -0700, Chad Versace wrote:
> The calculations of row_pitch, the row pitch's alignment, surface size,
> and base_alignment were mixed together. This patch moves the calculation
> of row_pitch and its alignment to occur before the calculation of
> surface_size and base_alignment.
> 
> This simplifies a follow-on patch that adds a new member, 'row_pitch',
> to struct isl_surf_init_info.
> 
> v2:
>   - Also extract the row pitch alignment.
>   - More helper functions that will later help validate the row pitch.
> ---
>  src/intel/isl/isl.c | 122 
> ++--
>  1 file changed, 89 insertions(+), 33 deletions(-)
> 
> diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> index ebc8d84be0f..784566749b4 100644
> --- a/src/intel/isl/isl.c
> +++ b/src/intel/isl/isl.c
> @@ -995,19 +995,12 @@ isl_calc_array_pitch_el_rows(const struct isl_device 
> *dev,
> return pitch_el_rows;
>  }
>  
> -/**
> - * Calculate the pitch of each surface row, in bytes.
> - */
>  static uint32_t
> -isl_calc_linear_row_pitch(const struct isl_device *dev,
> -  const struct isl_surf_init_info *restrict info,
> -  const struct isl_extent2d *phys_slice0_sa)
> +isl_calc_row_pitch_alignment(const struct isl_surf_init_info *surf_info,
> + const struct isl_tile_info *tile_info)
>  {
> -   const struct isl_format_layout *fmtl = 
> isl_format_get_layout(info->format);
> -   uint32_t row_pitch = info->min_pitch;
> -   assert(phys_slice0_sa->w % fmtl->bw == 0);
> -   const uint32_t bs = fmtl->bpb / 8;
> -   row_pitch = MAX(row_pitch, bs * (phys_slice0_sa->w / fmtl->bw));
> +   if (tile_info->tiling != ISL_TILING_LINEAR)
> +  return tile_info->phys_extent_B.width;
>  
> /* From the Broadwel PRM >> Volume 2d: Command Reference: Structures >>
>  * RENDER_SURFACE_STATE Surface Pitch (p349):
> @@ -1023,15 +1016,91 @@ isl_calc_linear_row_pitch(const struct isl_device 
> *dev,
>  *- For other linear surfaces, the pitch can be any multiple of
>  *  bytes.
>  */
> -   if (info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) {
> -  if (isl_format_is_yuv(info->format)) {
> - row_pitch = isl_align_npot(row_pitch, 2 * bs);
> +   const struct isl_format_layout *fmtl = 
> isl_format_get_layout(surf_info->format);
> +   const uint32_t bs = fmtl->bpb / 8;
> +
> +   if (surf_info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) {
> +  if (isl_format_is_yuv(surf_info->format)) {
> + return 2 * bs;
>} else  {
> - row_pitch = isl_align_npot(row_pitch, bs);
> + return bs;
>}
> }
>  
> -   return row_pitch;
> +   return 1;
> +}
> +
> +static uint32_t
> +isl_calc_linear_min_row_pitch(const struct isl_device *dev,
> +  const struct isl_surf_init_info *info,
> +  const struct isl_extent2d *phys_slice0_sa,
> +  uint32_t alignment)
> +{
> +   const struct isl_format_layout *fmtl = 
> isl_format_get_layout(info->format);
> +   const uint32_t bs = fmtl->bpb / 8;
> +
> +   assert(phys_slice0_sa->w % fmtl->bw == 0);
> +
> +   uint32_t min_row_pitch = bs * (phys_slice0_sa->w / fmtl->bw);
> +   min_row_pitch = MAX2(min_row_pitch, info->min_pitch);
> +   min_row_pitch =  isl_align_npot(min_row_pitch, alignment);
  ^
  extra space

With that fixed, this patch is
Reviewed-by: Nanley Chery 

> +
> +   return min_row_pitch;
> +}
> +
> +static uint32_t
> +isl_calc_tiled_min_row_pitch(const struct isl_device *dev,
> + const struct isl_surf_init_info *surf_info,
> + const struct isl_tile_info *tile_info,
> + const struct isl_extent2d *phys_slice0_sa,
> + uint32_t alignment)
> +{
> +   const struct isl_format_layout *fmtl = 
> isl_format_get_layout(surf_info->format);
> +
> +   assert(fmtl->bpb % tile_info->format_bpb == 0);
> +   assert(phys_slice0_sa->w % fmtl->bw == 0);
> +
> +   const uint32_t tile_el_scale = fmtl->bpb / tile_info->format_bpb;
> +   const uint32_t total_w_el = phys_slice0_sa->width / fmtl->bw;
> +   const uint32_t total_w_tl =
> +  isl_align_div(total_w_el * tile_el_scale,
> +tile_info->logical_extent_el.width);
> +
> +   uint32_t min_row_pitch = total_w_tl * tile_info->phys_extent_B.width;
> +   min_row_pitch = MAX2(min_row_pitch, surf_info->min_pitch);
> +   min_row_pitch = isl_align_npot(min_row_pitch, alignment);
> +
> +   return min_row_pitch;
> +}
> +
> +static uint32_t
> +isl_calc_min_row_pitch(const struct isl_device *dev,
> +   const struct isl_surf_init_info *surf_info,
> +   const struct isl_tile_info *tile_info,
> +   const struct isl_extent2d *phys_slice0_sa,
> +   

Re: [Mesa-dev] [PATCH shader-db] run: disable GLSL shader cache

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Wed, Mar 15, 2017 at 12:17 AM, Samuel Pitoiset
 wrote:
> I think we just don't want to store those shaders.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  run.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/run.c b/run.c
> index 74f87fd..12a78c6 100644
> --- a/run.c
> +++ b/run.c
> @@ -392,6 +392,7 @@ main(int argc, char **argv)
>
>  setenv("allow_glsl_extension_directive_midshader", "true", 1);
>  setenv("shader_precompile", "true", 1);
> +setenv("MESA_GLSL_CACHE_DISABLE", "true", 1);
>  addenv("ST_DEBUG", "precompile");
>  addenv("R600_DEBUG", "precompile");
>  addenv("FD_MESA_DEBUG", "shaderdb");
> --
> 2.12.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] radv/ac: workaround regression in llvm 4.0 release

[Samuel Pitoiset]

On 03/14/2017 10:21 PM, Dave Airlie wrote:

On 15 March 2017 at 07:17, Dave Airlie  wrote:

From: Dave Airlie 

LLVM 4.0 released with a pretty messy regression, that hopefully
get fixed in the future.

This work around was proposed by Tom, and it fixes the CTS regressions
here at least, I'm not sure if this will cause any major side effects,
but correctness over speed and all that.

radeonsi should possibly consider the same workaround until an llvm
fix can be found.



I also deliberately only put this for LLVM 4.0 in the hope llvm will
get fixed before
5 releases.


I think you want to make it >= 0x0400 and change it later because it's 
still broken with LLVM master.




Dave.


Signed-off-by: Dave Airlie 
---
 src/amd/common/ac_llvm_util.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/amd/common/ac_llvm_util.c b/src/amd/common/ac_llvm_util.c
index a2fe631..da12aaa 100644
--- a/src/amd/common/ac_llvm_util.c
+++ b/src/amd/common/ac_llvm_util.c
@@ -26,7 +26,7 @@
 #include "ac_llvm_util.h"
 #include "util/bitscan.h"
 #include 
-
+#include 
 #include "c11/threads.h"

 #include 
@@ -39,6 +39,17 @@ static void ac_init_llvm_target()
LLVMInitializeAMDGPUTarget();
LLVMInitializeAMDGPUTargetMC();
LLVMInitializeAMDGPUAsmPrinter();
+
+   /*
+* Workaround for bug in llvm 4.0 that causes image intrinsics
+* to disappear.
+* https://reviews.llvm.org/D26348
+*/
+#if HAVE_LLVM == 0x0400
+   const char *argv[2] = {"mesa", "-simplifycfg-sink-common=false"};
+   LLVMParseCommandLineOptions(2, argv, NULL);
+#endif
+
 }

 static once_flag ac_init_llvm_target_once_flag = ONCE_FLAG_INIT;
--
2.9.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 mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/7] isl: Drop misplaced comment about padding

[Nanley Chery]

On Mon, Mar 13, 2017 at 03:27:58PM -0700, Chad Versace wrote:
> isl has a giant comment that explains the hardware's padding
> requirements. (Hint: Cache lines and page faults). But the comment is in
> the wrong place, in isl_calc_linear_row_pitch(), which is unrelated to
> padding.
> 
> The important parts of that comment were copied to
> isl_apply_surface_padding() long ago. So drop the misplaced comment.
> ---
>  src/intel/isl/isl.c | 46 --
>  1 file changed, 46 deletions(-)

This patch is
Reviewed-by: Nanley Chery 

> 
> diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> index 6eb1e93efd9..ebc8d84be0f 100644
> --- a/src/intel/isl/isl.c
> +++ b/src/intel/isl/isl.c
> @@ -1004,53 +1004,7 @@ isl_calc_linear_row_pitch(const struct isl_device *dev,
>const struct isl_extent2d *phys_slice0_sa)
>  {
> const struct isl_format_layout *fmtl = 
> isl_format_get_layout(info->format);
> -
> uint32_t row_pitch = info->min_pitch;
> -
> -   /* First, align the surface to a cache line boundary, as the PRM explains
> -* below.
> -*
> -* From the Broadwell PRM >> Volume 5: Memory Views >> Common Surface
> -* Formats >> Surface Padding Requirements >> Render Target and Media
> -* Surfaces:
> -*
> -*The data port accesses data (pixels) outside of the surface if they
> -*are contained in the same cache request as pixels that are within 
> the
> -*surface. These pixels will not be returned by the requesting 
> message,
> -*however if these pixels lie outside of defined pages in the GTT,
> -*a GTT error will result when the cache request is processed. In 
> order
> -*to avoid these GTT errors, “padding” at the bottom of the surface is
> -*sometimes necessary.
> -*
> -* From the Broadwell PRM >> Volume 5: Memory Views >> Common Surface
> -* Formats >> Surface Padding Requirements >> Sampling Engine Surfaces:
> -*
> -*The sampling engine accesses texels outside of the surface if they
> -*are contained in the same cache line as texels that are within the
> -*surface.  These texels will not participate in any calculation
> -*performed by the sampling engine and will not affect the result of
> -*any sampling engine operation, however if these texels lie outside 
> of
> -*defined pages in the GTT, a GTT error will result when the cache 
> line
> -*is accessed. In order to avoid these GTT errors, “padding” at the
> -*bottom and right side of a sampling engine surface is sometimes
> -*necessary.
> -*
> -*It is possible that a cache line will straddle a page boundary if 
> the
> -*base address or pitch is not aligned. All pages included in the 
> cache
> -*lines that are part of the surface must map to valid GTT entries to
> -*avoid errors. To determine the necessary padding on the bottom and
> -*right side of the surface, refer to the table in  Alignment Unit 
> Size
> -*section for the i and j parameters for the surface format in use. 
> The
> -*surface must then be extended to the next multiple of the alignment
> -*unit size in each dimension, and all texels contained in this
> -*extended surface must have valid GTT entries.
> -*
> -*For example, suppose the surface size is 15 texels by 10 texels and
> -*the alignment parameters are i=4 and j=2. In this case, the extended
> -*surface would be 16 by 10. Note that these calculations are done in
> -*texels, and must be converted to bytes based on the surface format
> -*being used to determine whether additional pages need to be defined.
> -*/
> assert(phys_slice0_sa->w % fmtl->bw == 0);
> const uint32_t bs = fmtl->bpb / 8;
> row_pitch = MAX(row_pitch, bs * (phys_slice0_sa->w / fmtl->bw));
> -- 
> 2.12.0
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] radv: Set driver version to mesa version;

[Vedran Miletić]

14.03.2017 u 23:08, Bas Nieuwenhuizen je napisao/la:

I couldn't really find an encoding in the spec. I'm not sure it
prescribes VK_MAKE_VERSION format, but vulkan.gpuinfo.org interprets
it that way by default. vulkaninfo gives the raw number, so we could
alternatively do something like 17001000, but that doesn't show
up right on vulkan.gpuinfo.org again. Looking at that site, the -pro
driver also uses VK_MAKE_VERSION, so keeping consistency is probably
best.

Signed-off-by: Bas Nieuwenhuizen 
---
 src/amd/vulkan/radv_device.c | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 09ae4c9faab..d1fd58d77b9 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -477,6 +477,28 @@ void radv_GetPhysicalDeviceFeatures2KHR(
return radv_GetPhysicalDeviceFeatures(physicalDevice, 
>features);
 }

+static uint32_t radv_get_driver_version()
+{
+   const char *minor_string = strchr(VERSION, '.');
+   const char *patch_string = minor_string ? strchr(minor_string + 1, 
','): NULL;
+   int major = atoi(VERSION);
+   int minor = minor_string ? atoi(minor_string + 1) : 0;
+   int patch = patch_string ? atoi(patch_string + 1) : 0;
+   if (strstr(VERSION, "devel")) {
+   if (patch == 0) {
+   patch = 99;
+   if (minor == 0) {
+   minor = 99;
+   --major;
+   } else
+   --minor;
+   } else
+   --patch;
+   }
+   uint32_t version = VK_MAKE_VERSION(major, minor, patch);
+   return version;
+}
+
 void radv_GetPhysicalDeviceProperties(
VkPhysicalDevicephysicalDevice,
VkPhysicalDeviceProperties* pProperties)
@@ -598,7 +620,7 @@ void radv_GetPhysicalDeviceProperties(

*pProperties = (VkPhysicalDeviceProperties) {
.apiVersion = VK_MAKE_VERSION(1, 0, 42),
-   .driverVersion = 1,
+   .driverVersion = radv_get_driver_version(),
.vendorID = 0x1002,
.deviceID = pdevice->rad_info.pci_id,
.deviceType = VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU,



Works with Dota 2, Talos and Doom over Wine. Mesa version appears nicely 
in Doom. This patch and the previous one is


Tested-by: Vedran Miletić 

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

[Mesa-dev] [PATCH shader-db] run: disable GLSL shader cache

[Samuel Pitoiset]

I think we just don't want to store those shaders.

Signed-off-by: Samuel Pitoiset 
---
 run.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/run.c b/run.c
index 74f87fd..12a78c6 100644
--- a/run.c
+++ b/run.c
@@ -392,6 +392,7 @@ main(int argc, char **argv)
 
 setenv("allow_glsl_extension_directive_midshader", "true", 1);
 setenv("shader_precompile", "true", 1);
+setenv("MESA_GLSL_CACHE_DISABLE", "true", 1);
 addenv("ST_DEBUG", "precompile");
 addenv("R600_DEBUG", "precompile");
 addenv("FD_MESA_DEBUG", "shaderdb");
-- 
2.12.0

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

[Mesa-dev] [PATCH 2/2] radv: Set driver version to mesa version;

[Bas Nieuwenhuizen]

I couldn't really find an encoding in the spec. I'm not sure it
prescribes VK_MAKE_VERSION format, but vulkan.gpuinfo.org interprets
it that way by default. vulkaninfo gives the raw number, so we could
alternatively do something like 17001000, but that doesn't show
up right on vulkan.gpuinfo.org again. Looking at that site, the -pro
driver also uses VK_MAKE_VERSION, so keeping consistency is probably
best.

Signed-off-by: Bas Nieuwenhuizen 
---
 src/amd/vulkan/radv_device.c | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 09ae4c9faab..d1fd58d77b9 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -477,6 +477,28 @@ void radv_GetPhysicalDeviceFeatures2KHR(
return radv_GetPhysicalDeviceFeatures(physicalDevice, 
>features);
 }
 
+static uint32_t radv_get_driver_version()
+{
+   const char *minor_string = strchr(VERSION, '.');
+   const char *patch_string = minor_string ? strchr(minor_string + 1, 
','): NULL;
+   int major = atoi(VERSION);
+   int minor = minor_string ? atoi(minor_string + 1) : 0;
+   int patch = patch_string ? atoi(patch_string + 1) : 0;
+   if (strstr(VERSION, "devel")) {
+   if (patch == 0) {
+   patch = 99;
+   if (minor == 0) {
+   minor = 99;
+   --major;
+   } else
+   --minor;
+   } else
+   --patch;
+   }
+   uint32_t version = VK_MAKE_VERSION(major, minor, patch);
+   return version;
+}
+
 void radv_GetPhysicalDeviceProperties(
VkPhysicalDevicephysicalDevice,
VkPhysicalDeviceProperties* pProperties)
@@ -598,7 +620,7 @@ void radv_GetPhysicalDeviceProperties(
 
*pProperties = (VkPhysicalDeviceProperties) {
.apiVersion = VK_MAKE_VERSION(1, 0, 42),
-   .driverVersion = 1,
+   .driverVersion = radv_get_driver_version(),
.vendorID = 0x1002,
.deviceID = pdevice->rad_info.pci_id,
.deviceType = VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU,
-- 
2.12.0

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

Re: [Mesa-dev] [PATCH] radv: Flush before copying with PKT3_WRITE_DATA in CmdUpdateBuffer

[Bas Nieuwenhuizen]

Thanks. Pushed.

On Tue, Mar 14, 2017 at 4:26 PM, Alex Smith  wrote:
> Need to flush before updating the buffer to ensure that the copy is
> ordered after previous accesses (assuming the app has performed the
> appropriate barriers).
>
> This fixes potential issues due to draws prior to an update reading
> the new buffer content, despite having the necessary barriers between
> them.
>
> Signed-off-by: Alex Smith 
> ---
>  src/amd/vulkan/radv_meta_buffer.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/amd/vulkan/radv_meta_buffer.c 
> b/src/amd/vulkan/radv_meta_buffer.c
> index cd2973f..1e94f3b 100644
> --- a/src/amd/vulkan/radv_meta_buffer.c
> +++ b/src/amd/vulkan/radv_meta_buffer.c
> @@ -524,6 +524,8 @@ void radv_CmdUpdateBuffer(
> assert(!(va & 3));
>
> if (dataSize < 4096) {
> +   si_emit_cache_flush(cmd_buffer);
> +
> cmd_buffer->device->ws->cs_add_buffer(cmd_buffer->cs, 
> dst_buffer->bo, 8);
>
> radeon_check_space(cmd_buffer->device->ws, cmd_buffer->cs, 
> words + 4);
> --
> 2.9.3
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] radv: Increase api version to 1.0.42.

[Bas Nieuwenhuizen]

I've skimmed to changes from 1.0.5 to 1.0.42 and I think we have all
changes. We're still not conformant ofcourse, but this should not
regress stuff,

Signed-off-by: Bas Nieuwenhuizen 
---
 src/amd/vulkan/radv_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 17529c1f88e..09ae4c9faab 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -597,7 +597,7 @@ void radv_GetPhysicalDeviceProperties(
};
 
*pProperties = (VkPhysicalDeviceProperties) {
-   .apiVersion = VK_MAKE_VERSION(1, 0, 5),
+   .apiVersion = VK_MAKE_VERSION(1, 0, 42),
.driverVersion = 1,
.vendorID = 0x1002,
.deviceID = pdevice->rad_info.pci_id,
-- 
2.12.0

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

[Mesa-dev] [PATCH 3/6] [v6] gbm: Introduce modifiers into surface/bo creation

[Ben Widawsky]

The idea behind modifiers like this is that the user of GBM will have
some mechanism to query what properties the hardware supports for its BO
or surface. This information is directly passed in (and stored) so that
the DRI implementation can create an image with the appropriate
attributes.

A getter() will be added later so that the user GBM will be able to
query what modifier should be used.

Only in surface creation, the modifiers are stored until the BO is
actually allocated. In regular buffer allocation, the correct modifier
can (will be, in future patches be chosen at creation time.

v2: Make sure to check if count is non-zero in addition to testing if
calloc fails. (Daniel)

v3: Remove "usage" and "flags" from modifier creation. Requested by
Kristian.

v4: Take advantage of the "INVALID" modifier added by the GET_PLANE2
series.

v5: Don't bother with storing modifiers for gbm_bo_create because that's
a synchronous operation and we can actually select the correct modifier
at create time (done in a later patch) (Jason)

v6: Make modifier condition outside the check so that dri_use will work
properly (Jason)

Cc: Kristian Høgsberg 
References (v4): 
https://lists.freedesktop.org/archives/intel-gfx/2017-January/116636.html
Signed-off-by: Ben Widawsky 
Reviewed-by: Eric Engestrom  (v1)
Reviewed-by: Jason Ekstrand 
Acked-by: Daniel Stone 
---
 src/gbm/backends/dri/gbm_dri.c | 63 --
 src/gbm/gbm-symbols-check  |  2 ++
 src/gbm/main/gbm.c | 39 --
 src/gbm/main/gbm.h | 12 
 src/gbm/main/gbmint.h  | 12 ++--
 5 files changed, 116 insertions(+), 12 deletions(-)

diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
index 7106dc1229..cb3bfe1c2f 100644
--- a/src/gbm/backends/dri/gbm_dri.c
+++ b/src/gbm/backends/dri/gbm_dri.c
@@ -1023,13 +1023,20 @@ free_bo:
 static struct gbm_bo *
 gbm_dri_bo_create(struct gbm_device *gbm,
   uint32_t width, uint32_t height,
-  uint32_t format, uint32_t usage)
+  uint32_t format, uint32_t usage,
+  const uint64_t *modifiers,
+  const unsigned int count)
 {
struct gbm_dri_device *dri = gbm_dri_device(gbm);
struct gbm_dri_bo *bo;
int dri_format;
unsigned dri_use = 0;
 
+   /* Callers of this may specify a modifier, or a dri usage, but not both. The
+* newer modifier interface deprecates the older usage flags.
+*/
+   assert(!(usage && count));
+
if (usage & GBM_BO_USE_WRITE || dri->image == NULL)
   return create_dumb(gbm, width, height, format, usage);
 
@@ -1087,11 +1094,25 @@ gbm_dri_bo_create(struct gbm_device *gbm,
/* Gallium drivers requires shared in order to get the handle/stride */
dri_use |= __DRI_IMAGE_USE_SHARE;
 
-   bo->image =
-  dri->image->createImage(dri->screen,
-  width, height,
-  dri_format, dri_use,
-  bo);
+   if (modifiers) {
+  if (!dri->image || dri->image->base.version < 14 ||
+  !dri->image->createImageWithModifiers) {
+ fprintf(stderr, "Modifiers specified, but DRI is too old\n");
+ errno = ENOSYS;
+ goto failed;
+  }
+
+  bo->image =
+ dri->image->createImageWithModifiers(dri->screen,
+  width, height,
+  dri_format,
+  modifiers, count,
+  bo);
+   } else {
+  bo->image = dri->image->createImage(dri->screen, width, height,
+  dri_format, dri_use, bo);
+   }
+
if (bo->image == NULL)
   goto failed;
 
@@ -1165,19 +1186,44 @@ gbm_dri_bo_unmap(struct gbm_bo *_bo, void *map_data)
 static struct gbm_surface *
 gbm_dri_surface_create(struct gbm_device *gbm,
uint32_t width, uint32_t height,
-  uint32_t format, uint32_t flags)
+  uint32_t format, uint32_t flags,
+   const uint64_t *modifiers, const unsigned count)
 {
+   struct gbm_dri_device *dri = gbm_dri_device(gbm);
struct gbm_dri_surface *surf;
 
+   if (modifiers &&
+   (!dri->image || dri->image->base.version < 14 ||
+!dri->image->createImageWithModifiers)) {
+  errno = ENOSYS;
+  return NULL;
+   }
+
surf = calloc(1, sizeof *surf);
-   if (surf == NULL)
+   if (surf == NULL) {
+  errno = ENOMEM;
   return NULL;
+   }
 
surf->base.gbm = gbm;
surf->base.width = width;
surf->base.height = height;
surf->base.format = format;
surf->base.flags = flags;
+   if (!modifiers) {
+  assert(!count);
+  return >base;
+   }
+
+   surf->base.modifiers 

Re: [Mesa-dev] [PATCH] radv/ac: workaround regression in llvm 4.0 release

[Dave Airlie]

On 15 March 2017 at 07:17, Dave Airlie  wrote:
> From: Dave Airlie 
>
> LLVM 4.0 released with a pretty messy regression, that hopefully
> get fixed in the future.
>
> This work around was proposed by Tom, and it fixes the CTS regressions
> here at least, I'm not sure if this will cause any major side effects,
> but correctness over speed and all that.
>
> radeonsi should possibly consider the same workaround until an llvm
> fix can be found.
>

I also deliberately only put this for LLVM 4.0 in the hope llvm will
get fixed before
5 releases.

Dave.

> Signed-off-by: Dave Airlie 
> ---
>  src/amd/common/ac_llvm_util.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/src/amd/common/ac_llvm_util.c b/src/amd/common/ac_llvm_util.c
> index a2fe631..da12aaa 100644
> --- a/src/amd/common/ac_llvm_util.c
> +++ b/src/amd/common/ac_llvm_util.c
> @@ -26,7 +26,7 @@
>  #include "ac_llvm_util.h"
>  #include "util/bitscan.h"
>  #include 
> -
> +#include 
>  #include "c11/threads.h"
>
>  #include 
> @@ -39,6 +39,17 @@ static void ac_init_llvm_target()
> LLVMInitializeAMDGPUTarget();
> LLVMInitializeAMDGPUTargetMC();
> LLVMInitializeAMDGPUAsmPrinter();
> +
> +   /*
> +* Workaround for bug in llvm 4.0 that causes image intrinsics
> +* to disappear.
> +* https://reviews.llvm.org/D26348
> +*/
> +#if HAVE_LLVM == 0x0400
> +   const char *argv[2] = {"mesa", "-simplifycfg-sink-common=false"};
> +   LLVMParseCommandLineOptions(2, argv, NULL);
> +#endif
> +
>  }
>
>  static once_flag ac_init_llvm_target_once_flag = ONCE_FLAG_INIT;
> --
> 2.9.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] radv/ac: workaround regression in llvm 4.0 release

[Dave Airlie]

From: Dave Airlie 

LLVM 4.0 released with a pretty messy regression, that hopefully
get fixed in the future.

This work around was proposed by Tom, and it fixes the CTS regressions
here at least, I'm not sure if this will cause any major side effects,
but correctness over speed and all that.

radeonsi should possibly consider the same workaround until an llvm
fix can be found.

Signed-off-by: Dave Airlie 
---
 src/amd/common/ac_llvm_util.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/amd/common/ac_llvm_util.c b/src/amd/common/ac_llvm_util.c
index a2fe631..da12aaa 100644
--- a/src/amd/common/ac_llvm_util.c
+++ b/src/amd/common/ac_llvm_util.c
@@ -26,7 +26,7 @@
 #include "ac_llvm_util.h"
 #include "util/bitscan.h"
 #include 
-
+#include 
 #include "c11/threads.h"
 
 #include 
@@ -39,6 +39,17 @@ static void ac_init_llvm_target()
LLVMInitializeAMDGPUTarget();
LLVMInitializeAMDGPUTargetMC();
LLVMInitializeAMDGPUAsmPrinter();
+
+   /*
+* Workaround for bug in llvm 4.0 that causes image intrinsics
+* to disappear.
+* https://reviews.llvm.org/D26348
+*/
+#if HAVE_LLVM == 0x0400
+   const char *argv[2] = {"mesa", "-simplifycfg-sink-common=false"};
+   LLVMParseCommandLineOptions(2, argv, NULL);
+#endif
+
 }
 
 static once_flag ac_init_llvm_target_once_flag = ONCE_FLAG_INIT;
-- 
2.9.3

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

[Mesa-dev] [PATCH 1/2] nir/constant_expressions: Pull the guts out into a helper block

[Jason Ekstrand]

---
 src/compiler/nir/nir_constant_expressions.py | 199 ++-
 1 file changed, 101 insertions(+), 98 deletions(-)

diff --git a/src/compiler/nir/nir_constant_expressions.py 
b/src/compiler/nir/nir_constant_expressions.py
index c6745f1..ad841e3 100644
--- a/src/compiler/nir/nir_constant_expressions.py
+++ b/src/compiler/nir/nir_constant_expressions.py
@@ -266,116 +266,119 @@ struct bool32_vec {
 bool w;
 };
 
-% for name, op in sorted(opcodes.iteritems()):
-static nir_const_value
-evaluate_${name}(MAYBE_UNUSED unsigned num_components, unsigned bit_size,
- MAYBE_UNUSED nir_const_value *_src)
-{
-   nir_const_value _dst_val = { {0, } };
-
-   switch (bit_size) {
-   % for bit_size in op_bit_sizes(op):
-   case ${bit_size}: {
-  <%
-  output_type = type_add_size(op.output_type, bit_size)
-  input_types = [type_add_size(type_, bit_size) for type_ in 
op.input_types]
-  %>
-
-  ## For each non-per-component input, create a variable srcN that
-  ## contains x, y, z, and w elements which are filled in with the
-  ## appropriately-typed values.
-  % for j in range(op.num_inputs):
- % if op.input_sizes[j] == 0:
-<% continue %>
- % elif "src" + str(j) not in op.const_expr:
-## Avoid unused variable warnings
-<% continue %>
- %endif
-
- const struct ${input_types[j]}_vec src${j} = {
- % for k in range(op.input_sizes[j]):
-% if input_types[j] == "bool32":
-   _src[${j}].u32[${k}] != 0,
-% else:
-   _src[${j}].${get_const_field(input_types[j])}[${k}],
-% endif
- % endfor
- % for k in range(op.input_sizes[j], 4):
-0,
- % endfor
- };
+<%def name="evaluate_op(op, bit_size)">
+   <%
+   output_type = type_add_size(op.output_type, bit_size)
+   input_types = [type_add_size(type_, bit_size) for type_ in op.input_types]
+   %>
+
+   ## For each non-per-component input, create a variable srcN that
+   ## contains x, y, z, and w elements which are filled in with the
+   ## appropriately-typed values.
+   % for j in range(op.num_inputs):
+  % if op.input_sizes[j] == 0:
+ <% continue %>
+  % elif "src" + str(j) not in op.const_expr:
+ ## Avoid unused variable warnings
+ <% continue %>
+  %endif
+
+  const struct ${input_types[j]}_vec src${j} = {
+  % for k in range(op.input_sizes[j]):
+ % if input_types[j] == "bool32":
+_src[${j}].u32[${k}] != 0,
+ % else:
+_src[${j}].${get_const_field(input_types[j])}[${k}],
+ % endif
   % endfor
+  % for k in range(op.input_sizes[j], 4):
+ 0,
+  % endfor
+  };
+   % endfor
 
-  % if op.output_size == 0:
- ## For per-component instructions, we need to iterate over the
- ## components and apply the constant expression one component
- ## at a time.
- for (unsigned _i = 0; _i < num_components; _i++) {
-## For each per-component input, create a variable srcN that
-## contains the value of the current (_i'th) component.
-% for j in range(op.num_inputs):
-   % if op.input_sizes[j] != 0:
-  <% continue %>
-   % elif "src" + str(j) not in op.const_expr:
-  ## Avoid unused variable warnings
-  <% continue %>
-   % elif input_types[j] == "bool32":
-  const bool src${j} = _src[${j}].u32[_i] != 0;
-   % else:
-  const ${input_types[j]}_t src${j} =
- _src[${j}].${get_const_field(input_types[j])}[_i];
-   % endif
-% endfor
-
-## Create an appropriately-typed variable dst and assign the
-## result of the const_expr to it.  If const_expr already contains
-## writes to dst, just include const_expr directly.
-% if "dst" in op.const_expr:
-   ${output_type}_t dst;
-
-   ${op.const_expr}
-% else:
-   ${output_type}_t dst = ${op.const_expr};
-% endif
-
-## Store the current component of the actual destination to the
-## value of dst.
-% if output_type == "bool32":
-   ## Sanitize the C value to a proper NIR bool
-   _dst_val.u32[_i] = dst ? NIR_TRUE : NIR_FALSE;
+   % if op.output_size == 0:
+  ## For per-component instructions, we need to iterate over the
+  ## components and apply the constant expression one component
+  ## at a time.
+  for (unsigned _i = 0; _i < num_components; _i++) {
+ ## For each per-component input, create a variable srcN that
+ ## contains the value of the current (_i'th) component.
+ % for j in range(op.num_inputs):
+% if op.input_sizes[j] != 0:
+   <% 

[Mesa-dev] [PATCH 2/2] nir/constant_expressions: Don't switch on bit size when not needed

[Jason Ekstrand]

For opcodes such as the nir_op_pack_64_2x32 for which all sources and
destinations have explicit sizes, the bit_size parameter to the evaluate
function is pointless and *should* do nothing.  Previously, we were
always switching on the bit_size and asserting if it isn't one of the
sizes in the list.  This generates way more code than needed and is a
bit cruel because it doesn't let us have a bit_size of zero on an ALU op
which shouldn't need a bit_size.
---
 src/compiler/nir/nir_constant_expressions.py | 37 +---
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/src/compiler/nir/nir_constant_expressions.py 
b/src/compiler/nir/nir_constant_expressions.py
index ad841e3..0ff8dfb 100644
--- a/src/compiler/nir/nir_constant_expressions.py
+++ b/src/compiler/nir/nir_constant_expressions.py
@@ -22,13 +22,18 @@ def type_add_size(type_, size):
 return type_ + str(size)
 
 def op_bit_sizes(op):
-sizes = set([8, 16, 32, 64])
+sizes = None
 if not type_has_size(op.output_type):
-sizes = sizes.intersection(set(type_sizes(op.output_type)))
+sizes = set(type_sizes(op.output_type))
+
 for input_type in op.input_types:
 if not type_has_size(input_type):
-sizes = sizes.intersection(set(type_sizes(input_type)))
-return sorted(list(sizes))
+if sizes is None:
+sizes = set(type_sizes(input_type))
+else:
+sizes = sizes.intersection(set(type_sizes(input_type)))
+
+return sorted(list(sizes)) if sizes is not None else None
 
 def get_const_field(type_):
 if type_ == "bool32":
@@ -375,17 +380,21 @@ evaluate_${name}(MAYBE_UNUSED unsigned num_components, 
unsigned bit_size,
 {
nir_const_value _dst_val = { {0, } };
 
-   switch (bit_size) {
-   % for bit_size in op_bit_sizes(op):
-   case ${bit_size}: {
-  ${evaluate_op(op, bit_size)}
-  break;
-   }
-   % endfor
+   % if op_bit_sizes(op) is not None:
+  switch (bit_size) {
+  % for bit_size in op_bit_sizes(op):
+  case ${bit_size}: {
+ ${evaluate_op(op, bit_size)}
+ break;
+  }
+  % endfor
 
-   default:
-  unreachable("unknown bit width");
-   }
+  default:
+ unreachable("unknown bit width");
+  }
+   % else:
+  ${evaluate_op(op, 0)}
+   % endif
 
return _dst_val;
 }
-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [PATCH] radv: Emit cache flushes before CP DMA.

[Bas Nieuwenhuizen]

The flushes could be due to TRANSFER barriers.

Signed-off-by: Bas Nieuwenhuizen 
Cc: 17.0 
---
 src/amd/vulkan/si_cmd_buffer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/amd/vulkan/si_cmd_buffer.c b/src/amd/vulkan/si_cmd_buffer.c
index 5d35287f8e3..b808052ddb2 100644
--- a/src/amd/vulkan/si_cmd_buffer.c
+++ b/src/amd/vulkan/si_cmd_buffer.c
@@ -998,6 +998,7 @@ void si_cp_dma_buffer_copy(struct radv_cmd_buffer 
*cmd_buffer,
uint64_t main_src_va, main_dest_va;
uint64_t skipped_size = 0, realign_size = 0;
 
+   si_emit_cache_flush(cmd_buffer);
 
if (cmd_buffer->device->physical_device->rad_info.family <= 
CHIP_CARRIZO ||
cmd_buffer->device->physical_device->rad_info.family == 
CHIP_STONEY) {
@@ -1061,6 +1062,8 @@ void si_cp_dma_clear_buffer(struct radv_cmd_buffer 
*cmd_buffer, uint64_t va,
 
assert(va % 4 == 0 && size % 4 == 0);
 
+   si_emit_cache_flush(cmd_buffer);
+
while (size) {
unsigned byte_count = MIN2(size, CP_DMA_MAX_BYTE_COUNT);
unsigned dma_flags = 0;
-- 
2.12.0

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

Re: [Mesa-dev] [PATCH 3/6] gbm: Introduce modifiers into surface/bo creation

[Ben Widawsky]

On 17-03-14 08:53:45, Jason Ekstrand wrote:

On Mon, Mar 13, 2017 at 10:24 PM, Ben Widawsky  wrote:


The idea behind modifiers like this is that the user of GBM will have
some mechanism to query what properties the hardware supports for its BO
or surface. This information is directly passed in (and stored) so that
the DRI implementation can create an image with the appropriate
attributes.

A getter() will be added later so that the user GBM will be able to
query what modifier should be used.

Only in surface creation, the modifiers are stored until the BO is
actually allocated. In regular buffer allocation, the correct modifier
can (will be, in future patches be chosen at creation time.

v2: Make sure to check if count is non-zero in addition to testing if
calloc fails. (Daniel)

v3: Remove "usage" and "flags" from modifier creation. Requested by
Kristian.

v4: Take advantage of the "INVALID" modifier added by the GET_PLANE2
series.

v5: Don't bother with storing modifiers for gbm_bo_create because that's
a synchronous operation and we can actually select the correct modifier
at create time (done in a later patch) (Jason)

Cc: Kristian Høgsberg 
Cc: Jason Ekstrand 
References (v4): https://lists.freedesktop.org/archives/intel-gfx/2017-
January/116636.html
Signed-off-by: Ben Widawsky 
Reviewed-by: Eric Engestrom  (v1)
Acked-by: Daniel Stone 
---
 src/gbm/backends/dri/gbm_dri.c | 62 ++
++--
 src/gbm/gbm-symbols-check  |  2 ++
 src/gbm/main/gbm.c | 39 --
 src/gbm/main/gbm.h | 12 
 src/gbm/main/gbmint.h  | 12 ++--
 5 files changed, 115 insertions(+), 12 deletions(-)

diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_
dri.c
index 7106dc1229..1224ce4476 100644
--- a/src/gbm/backends/dri/gbm_dri.c
+++ b/src/gbm/backends/dri/gbm_dri.c
@@ -1023,13 +1023,20 @@ free_bo:
 static struct gbm_bo *
 gbm_dri_bo_create(struct gbm_device *gbm,
   uint32_t width, uint32_t height,
-  uint32_t format, uint32_t usage)
+  uint32_t format, uint32_t usage,
+  const uint64_t *modifiers,
+  const unsigned int count)
 {
struct gbm_dri_device *dri = gbm_dri_device(gbm);
struct gbm_dri_bo *bo;
int dri_format;
unsigned dri_use = 0;

+   /* Callers of this may specify a modifier, or a dri usage, but not
both. The
+* newer modifier interface deprecates the older usage flags.
+*/
+   assert(!(usage && count));
+
if (usage & GBM_BO_USE_WRITE || dri->image == NULL)
   return create_dumb(gbm, width, height, format, usage);

@@ -1087,11 +1094,24 @@ gbm_dri_bo_create(struct gbm_device *gbm,
/* Gallium drivers requires shared in order to get the handle/stride */
dri_use |= __DRI_IMAGE_USE_SHARE;

-   bo->image =
-  dri->image->createImage(dri->screen,
-  width, height,
-  dri_format, dri_use,
-  bo);
+   if (!dri->image || dri->image->base.version < 14 ||
+   !dri->image->createImageWithModifiers) {
+  if (modifiers) {



This logic still isn't correct.  As written, it will always call
createImageWithModifiers if it's available even if no modifiers are
provided.  I think you want the "if (modifiers)" to be the outside bit of
control-flow




I don't see a problem here. Calling createImageWithModifiers without modifiers
(provided count is correct) is a perfectly acceptable thing to do.


+ fprintf(stderr, "Modifiers specified, but DRI is too old\n");
+ errno = ENOSYS;
+ goto failed;
+  }
+
+  bo->image = dri->image->createImage(dri->screen, width, height,
+  dri_format, dri_use, bo);
+   } else {
+  bo->image =
+ dri->image->createImageWithModifiers(dri->screen,
+  width, height,
+  dri_format,
+  modifiers, count,
+  bo);
+   }
if (bo->image == NULL)
   goto failed;

@@ -1165,19 +1185,44 @@ gbm_dri_bo_unmap(struct gbm_bo *_bo, void
*map_data)
 static struct gbm_surface *
 gbm_dri_surface_create(struct gbm_device *gbm,
uint32_t width, uint32_t height,
-  uint32_t format, uint32_t flags)
+  uint32_t format, uint32_t flags,
+   const uint64_t *modifiers, const unsigned count)
 {
+   struct gbm_dri_device *dri = gbm_dri_device(gbm);
struct gbm_dri_surface *surf;

+   if (modifiers &&
+   (!dri->image || dri->image->base.version < 14 ||
+!dri->image->createImageWithModifiers)) {
+  errno = ENOSYS;
+  return NULL;

[Mesa-dev] [Bug 94739] Mesa 11.1.2 implementation error: bad format MESA_FORMAT_Z_FLOAT32 in _mesa_unpack_uint_24_8_depth_stencil_row

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=94739

Luca Weiss  changed:

   What|Removed |Added

 CC||bugzi...@z3ntu.xyz

--- Comment #6 from Luca Weiss  ---
GL_RENDERER   = Mesa DRI Intel(R) Haswell Mobile 
GL_VERSION= 3.0 Mesa 17.0.1
GL_VENDOR = Intel Open Source Technology Center

I get that error when running Cemu 1.7.2c (Wii U emulator) with Wine. At least
Mario Kart 8 and Splatoon can produce that issue.

-- 
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] MESA and KOTOR

[Jan Vesely]

On Tue, 2017-03-14 at 05:22 +0100, Federico Dossena wrote:
> The game runs smoothly if you decompress the textures, I'm getting 30-40 
> fps with llvmpipe and no "grass in the sky" artifacts.
> The problem is frame buffer effects and soft shadows, were they enabled 
> when you tested it? If so, how did you make it use llvmpipe?

it was just a quick test, I'm not too worried about performance. both
frame buffer effects and soft shadows were off (it won't allow me to
turn soft shadows ON), but it used 4x AA so poor performance was
expected.
I used LIBGL_ALWAYS_SOFTWARE=1 in the environment to run llvmpipe.

Jan

> 
> 
> Il 2017-03-13 22:40, Jan Vesely ha scritto:
> > On Mon, 2017-03-13 at 11:40 +0200, Grazvydas Ignotas wrote:
> > > On Sat, Mar 11, 2017 at 1:51 PM, Federico Dossena  
> > > wrote:
> > > > This issue is not new: a guy named Miklós Máté, here in the Mesa mailing
> > > > list somehow managed to fix it in Mesa 11, but his patches do not seem 
> > > > to
> > > > work anymore
> > > > (https://lists.freedesktop.org/archives/mesa-dev/2016-February/106695.html).
> > > 
> > > He was using wine on Linux, I guess wine provides an implementation of
> > > that function. You could try running the game on Linux too.
> > 
> > runs mostly OK using wine here. Intel(skylake) driver suffers terrible
> > performance and displays artifacts in certain locations. nouveau(950m)
> > works nicely. llvmpipe+llvm3.7 also works, albeit the performance is
> > terrible.
> > 
> > Jan
> > 
> > > Gražvydas
> > > ___
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> 

-- 
Jan Vesely 

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 100199] clover: implement non-blocking clEnqueueWriteBuffer and clEnqueueReadBuffer

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100199

--- Comment #2 from Vedran Miletić  ---
Benchmarking ftp://ftp.gromacs.org/pub/benchmarks/water_GMX50_bare.tar.gz with
GROMACS 2016.3 I get the following results:

Without the patch:
 Launch GPU ops.18  66001  16.905460.911  67.1
 Wait GPU local 18  66001   2.569 70.041  10.2
 Launch GPU ops.18  66001  20.110548.283  65.2
 Wait GPU local 18  66001   2.586 70.511   8.4
 Launch GPU ops.18  33001  10.902297.224  61.0
 Wait GPU local 18  33001   1.309 35.695   7.3
 Launch GPU ops.18  16001   6.460176.126  54.4
 Wait GPU local 18  16001   0.642 17.499   5.4
 Launch GPU ops.18   8001   4.624126.062  47.7
 Wait GPU local 18   8001   0.361  9.833   3.7
 Launch GPU ops.18   4001   3.471 94.623  39.5
 Wait GPU local 18   4001   0.246  6.699   2.8
 Launch GPU ops.18   2001   3.648 99.469  38.5
 Wait GPU local 18   2001   0.185  5.054   2.0
 Launch GPU ops.18   1001   3.396 92.583  35.5
 Wait GPU local 18   1001   0.106  2.902   1.1
 Launch GPU ops.18501   6.106166.478  46.2
 Wait GPU local 18501   0.064  1.745   0.5
 Launch GPU ops.18251   4.909133.845  35.1
 Wait GPU local 18251   0.040  1.089   0.3
 Launch GPU ops.18126   3.936107.314  26.5
 Wait GPU local 18126   0.021  0.576   0.1
 Launch GPU ops.18 66   3.314 90.362  18.5
 Wait GPU local 18 66   0.011  0.296   0.1
 Launch GPU ops.18 34   3.139 85.581  14.4
 Wait GPU local 18 34   0.005  0.144   0.0

With the patch:
 Launch GPU ops.18  66001  18.298498.894  66.8
 Wait GPU local 18  66001   2.519 68.691   9.2
 Launch GPU ops.18  66001  22.010600.094  66.4
 Wait GPU local 18  66001   2.543 69.343   7.7
 Launch GPU ops.18  33001  11.510313.821  60.9
 Wait GPU local 18  33001   1.261 34.368   6.7
 Launch GPU ops.18  16001   6.928188.899  56.0
 Wait GPU local 18  16001   0.626 17.077   5.1
 Launch GPU ops.18   8001   4.850132.245  48.3
 Wait GPU local 18   8001   0.350  9.531   3.5
 Launch GPU ops.18   4001   3.657 99.696  40.2
 Wait GPU local 18   4001   0.272  7.415   3.0
 Launch GPU ops.18   2001   3.791103.350  38.9
 Wait GPU local 18   2001   0.191  5.202   2.0
 Launch GPU ops.18   1001   3.514 95.813  36.0
 Wait GPU local 18   1001   0.106  2.886   1.1
 Launch GPU ops.18501   6.102166.361  46.0
 Wait GPU local 18501   0.063  1.716   0.5
 Launch GPU ops.18251   4.936134.578  35.1
 Wait GPU local 18251   0.040  1.086   0.3
 Launch GPU ops.18126   3.941107.435  26.5
 Wait GPU local 18126   0.021  0.559   0.1
 Launch GPU ops.18 66   3.335 90.920  18.3
 Wait GPU local 18 66   0.011  0.297   0.1
 Launch GPU ops.18 34   3.173 86.500  14.6
 Wait GPU local 18 34   0.005  0.147   0.0

So it's not an improvement.

-- 
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] mx6q: Cannot run Cinematic demo correctly

[Fabio Estevam]

Hi Wladimir,

On Tue, Mar 14, 2017 at 2:47 PM, Wladimir J. van der Laan
 wrote:

> Yea, variants would be the way to render to RGBA succesfully
> on vivantes.
>
> Until then it's best to revert those two patches.
>
> Reverting only the one (latest) patch can give red/blue swapped issues when
> doing render-to-texture, it was applied in the first place to fix those issues
> caused by the first patch, but it didn't really make things better.

Thanks for your feedback.

Otavio,

Care to submit the revert for the two patches?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] mx6q: Cannot run Cinematic demo correctly

[Wladimir J. van der Laan]

On Tue, Mar 14, 2017 at 06:39:32PM +0100, Christian Gmeiner wrote:
> > By only reverting:
> >
> > commit 6c89a728d9e5d072cb504453e73077564c6523d3
> > Author: Wladimir J. van der Laan 
> > Date:   Wed Dec 7 12:59:54 2016 +
> >
> > etnaviv: Cannot render to rb-swapped formats
> >
> > Exposing rb swapped (or other swizzled) formats for rendering would
> > involve swizzing in the pixel shader. This is not the case at the
> > moment, so reject requests for creating such surfaces.
> >
> > (GPUs that need an extra resolve step anyway due to multiple pixel
> > pipes, such as gc2000, might also do this swap in the resolve operation.
> > But this would be tricky to keep track of)
> >
> > CC: 
> > Signed-off-by: Wladimir J. van der Laan 
> > Acked-by: Christian Gmeiner 
> >
> > I can confirm that the Cinematic demo can run successfully. I do not
> > see the black boxes, nor font issues anymore.
> >
> 
> I am a little bit overloaded with my day job (again) and the initial plan was 
> to
> land something in stable for 17.0.1 release failed - but he 17.0.2 will come.
> 
> >
> > Also the previous error messages are gone:
> > QOpenGLFramebufferObject: Unsupported framebuffer format.
> > QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
> > QOpenGLFramebufferObject: Unsupported framebuffer format.
> > QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
> > QOpenGLFramebufferObject: Unsupported framebuffer format.
> > QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
> > QOpenGLFramebufferObject: Unsupported framebuffer format.
> > QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
> >
> > as to your other patch: I do not see commit
> > 89bb5c79e29613ad9a4e43d670654e98a220fc60 in mesa tree.
> >
> > Wladimir/Christian,
> >
> > What would be the proper fix for this problem?
> >
> 
> shader variants - due to lot of people are have this issue I will
> spend some time the next
> days to cleanup my wip patch series. I have too may concurrent etnaviv
> work items running
> in parallel :(

Yea, variants would be the way to render to RGBA succesfully
on vivantes.

Until then it's best to revert those two patches.

Reverting only the one (latest) patch can give red/blue swapped issues when
doing render-to-texture, it was applied in the first place to fix those issues
caused by the first patch, but it didn't really make things better.

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

Re: [Mesa-dev] mx6q: Cannot run Cinematic demo correctly

[Christian Gmeiner]

Hi Fabio

2017-03-14 18:16 GMT+01:00 Fabio Estevam :
> Hi Otavio,
>
> On Tue, Mar 14, 2017 at 1:58 PM, Otavio Salvador
>  wrote:
>> On Tue, Mar 14, 2017 at 12:31 PM, Fabio Estevam  wrote:
>> ...
>>> The cover of the movies are just black rectangles instead of showing
>>> the actual movie pictures.
>> ...
>>
>> I am attaching the two patches I am applying on mesa locally and which
>> has fixed our current issues.
>
> Thanks for the patches!
>
> By only reverting:
>
> commit 6c89a728d9e5d072cb504453e73077564c6523d3
> Author: Wladimir J. van der Laan 
> Date:   Wed Dec 7 12:59:54 2016 +
>
> etnaviv: Cannot render to rb-swapped formats
>
> Exposing rb swapped (or other swizzled) formats for rendering would
> involve swizzing in the pixel shader. This is not the case at the
> moment, so reject requests for creating such surfaces.
>
> (GPUs that need an extra resolve step anyway due to multiple pixel
> pipes, such as gc2000, might also do this swap in the resolve operation.
> But this would be tricky to keep track of)
>
> CC: 
> Signed-off-by: Wladimir J. van der Laan 
> Acked-by: Christian Gmeiner 
>
> I can confirm that the Cinematic demo can run successfully. I do not
> see the black boxes, nor font issues anymore.
>

I am a little bit overloaded with my day job (again) and the initial plan was to
land something in stable for 17.0.1 release failed - but he 17.0.2 will come.

>
> Also the previous error messages are gone:
> QOpenGLFramebufferObject: Unsupported framebuffer format.
> QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
> QOpenGLFramebufferObject: Unsupported framebuffer format.
> QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
> QOpenGLFramebufferObject: Unsupported framebuffer format.
> QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
> QOpenGLFramebufferObject: Unsupported framebuffer format.
> QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
>
> as to your other patch: I do not see commit
> 89bb5c79e29613ad9a4e43d670654e98a220fc60 in mesa tree.
>
> Wladimir/Christian,
>
> What would be the proper fix for this problem?
>

shader variants - due to lot of people are have this issue I will
spend some time the next
days to cleanup my wip patch series. I have too may concurrent etnaviv
work items running
in parallel :(

greets
--
Christian Gmeiner, MSc

https://www.youtube.com/user/AloryOFFICIAL
https://soundcloud.com/christian-gmeiner
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 99987] Mesa 13+ breaks Xvnc (and similar X servers)

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=99987

--- Comment #15 from Dennis Schridde  ---
(In reply to Pierre Ossman from comment #14)
> b) If it is indeed a fallback, why is that needed? I.e. why isn't Mesa being
> chosen? It does not seem robust and future proof to rely on a fallback
> mechanism. Which leads to...

Because libglvnd tries to be vendor neutral (i.e. not tied to either Mesa nor
Nvidia) and there might be other vendors providing a software / indirect
rendering implementation. Maybe not now and on current Linux distributions, but
e.g. on another OS or in the future.

> c) What happens when both Mesa and NVIDIA wants to install the fallback
> symlink? It sounds like things will just stop working, getting us right back
> to the situation that GLVND was supposed to solve.

I was wondering about the same thing. The env-var solution actually seems
superior to having a file collision between different packages. Even a config
file might be better than a symlink.

But Hans is already working on this:
https://bugzilla.redhat.com/show_bug.cgi?id=1413579#57
> I will prepare an update to add this to the Fedora mesa pkgs, but I
> need to coordinate this with the rpmfusion packages, since those currently add
> such a symlink to the nvidia libGLX.

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

Re: [Mesa-dev] mx6q: Cannot run Cinematic demo correctly

[Fabio Estevam]

Hi Otavio,

On Tue, Mar 14, 2017 at 1:58 PM, Otavio Salvador
 wrote:
> On Tue, Mar 14, 2017 at 12:31 PM, Fabio Estevam  wrote:
> ...
>> The cover of the movies are just black rectangles instead of showing
>> the actual movie pictures.
> ...
>
> I am attaching the two patches I am applying on mesa locally and which
> has fixed our current issues.

Thanks for the patches!

By only reverting:

commit 6c89a728d9e5d072cb504453e73077564c6523d3
Author: Wladimir J. van der Laan 
Date:   Wed Dec 7 12:59:54 2016 +

etnaviv: Cannot render to rb-swapped formats

Exposing rb swapped (or other swizzled) formats for rendering would
involve swizzing in the pixel shader. This is not the case at the
moment, so reject requests for creating such surfaces.

(GPUs that need an extra resolve step anyway due to multiple pixel
pipes, such as gc2000, might also do this swap in the resolve operation.
But this would be tricky to keep track of)

CC: 
Signed-off-by: Wladimir J. van der Laan 
Acked-by: Christian Gmeiner 

I can confirm that the Cinematic demo can run successfully. I do not
see the black boxes, nor font issues anymore.

Also the previous error messages are gone:
QOpenGLFramebufferObject: Unsupported framebuffer format.
QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
QOpenGLFramebufferObject: Unsupported framebuffer format.
QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
QOpenGLFramebufferObject: Unsupported framebuffer format.
QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
QOpenGLFramebufferObject: Unsupported framebuffer format.
QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.

as to your other patch: I do not see commit
89bb5c79e29613ad9a4e43d670654e98a220fc60 in mesa tree.

Wladimir/Christian,

What would be the proper fix for this problem?

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

[Mesa-dev] [Bug 100202] llvmpipe Windows scons build can't detect Visual Studio 2017 toolchain

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100202

Bug ID: 100202
   Summary: llvmpipe Windows scons build can't detect Visual
Studio 2017 toolchain
   Product: Mesa
   Version: 17.0
  Hardware: All
OS: Windows (All)
Status: NEW
  Severity: major
  Priority: medium
 Component: Mesa core
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: anonym...@dodgeit.com
QA Contact: mesa-dev@lists.freedesktop.org

Created attachment 130220
  --> https://bugs.freedesktop.org/attachment.cgi?id=130220=edit
Build log

-- 
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 1/2] anv/image: Move handling of INTEL_VK_HIZ

[Pohjolainen, Topi]

On Tue, Mar 14, 2017 at 07:55:01AM -0700, Jason Ekstrand wrote:
> This makes it so that you don't get an "Implement gen7 HiZ" perf warning
> when you manually disable HiZ on gen8.

Both:

Reviewed-by: Topi Pohjolainen 

> ---
>  src/intel/vulkan/anv_image.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index 5f17351..931ee22 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -190,7 +190,7 @@ make_surface(const struct anv_device *dev,
> */
>if (!(image->usage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT)) {
>   /* It will never be used as an attachment, HiZ is pointless. */
> -  } else if (!env_var_as_boolean("INTEL_VK_HIZ", dev->info.gen >= 8)) {
> +  } else if (dev->info.gen == 7) {
>   anv_perf_warn("Implement gen7 HiZ");
>} else if (vk_info->mipLevels > 1) {
>   anv_perf_warn("Enable multi-LOD HiZ");
> @@ -198,7 +198,7 @@ make_surface(const struct anv_device *dev,
>   anv_perf_warn("Implement multi-arrayLayer HiZ clears and resolves");
>} else if (dev->info.gen == 8 && vk_info->samples > 1) {
>   anv_perf_warn("Enable gen8 multisampled HiZ");
> -  } else {
> +  } else if (env_var_as_boolean("INTEL_VK_HIZ", true)) {
>   assert(image->aux_surface.isl.size == 0);
>   ok = isl_surf_get_hiz_surf(>isl_dev, >depth_surface.isl,
>  >aux_surface.isl);
> -- 
> 2.5.0.400.gff86faf
> 
> ___
> 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 100201] llvmpipe Windows scons build with Visual Studio 2015 toolchain and LLVM 4.0 fails

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100201

Bug ID: 100201
   Summary: llvmpipe Windows scons build with Visual Studio 2015
toolchain and LLVM 4.0 fails
   Product: Mesa
   Version: 17.0
  Hardware: All
OS: Windows (All)
Status: NEW
  Severity: major
  Priority: medium
 Component: Mesa core
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: anonym...@dodgeit.com
QA Contact: mesa-dev@lists.freedesktop.org

Created attachment 130219
  --> https://bugs.freedesktop.org/attachment.cgi?id=130219=edit
Build output.

llvmpipe build is broken again with newest LLVM release - 4.0.

-- 
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 3/6] gbm: Introduce modifiers into surface/bo creation

[Jason Ekstrand]

The comments below are my only remaining questions on this 6-patch series.
All of the *other* patches are

Reviewed-by: Jason Ekstrand 

On Tue, Mar 14, 2017 at 8:53 AM, Jason Ekstrand 
wrote:

> On Mon, Mar 13, 2017 at 10:24 PM, Ben Widawsky  wrote:
>
>> The idea behind modifiers like this is that the user of GBM will have
>> some mechanism to query what properties the hardware supports for its BO
>> or surface. This information is directly passed in (and stored) so that
>> the DRI implementation can create an image with the appropriate
>> attributes.
>>
>> A getter() will be added later so that the user GBM will be able to
>> query what modifier should be used.
>>
>> Only in surface creation, the modifiers are stored until the BO is
>> actually allocated. In regular buffer allocation, the correct modifier
>> can (will be, in future patches be chosen at creation time.
>>
>> v2: Make sure to check if count is non-zero in addition to testing if
>> calloc fails. (Daniel)
>>
>> v3: Remove "usage" and "flags" from modifier creation. Requested by
>> Kristian.
>>
>> v4: Take advantage of the "INVALID" modifier added by the GET_PLANE2
>> series.
>>
>> v5: Don't bother with storing modifiers for gbm_bo_create because that's
>> a synchronous operation and we can actually select the correct modifier
>> at create time (done in a later patch) (Jason)
>>
>> Cc: Kristian Høgsberg 
>> Cc: Jason Ekstrand 
>> References (v4): https://lists.freedesktop.org/
>> archives/intel-gfx/2017-January/116636.html
>> Signed-off-by: Ben Widawsky 
>> Reviewed-by: Eric Engestrom  (v1)
>> Acked-by: Daniel Stone 
>> ---
>>  src/gbm/backends/dri/gbm_dri.c | 62 ++
>> ++--
>>  src/gbm/gbm-symbols-check  |  2 ++
>>  src/gbm/main/gbm.c | 39 --
>>  src/gbm/main/gbm.h | 12 
>>  src/gbm/main/gbmint.h  | 12 ++--
>>  5 files changed, 115 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/gbm/backends/dri/gbm_dri.c
>> b/src/gbm/backends/dri/gbm_dri.c
>> index 7106dc1229..1224ce4476 100644
>> --- a/src/gbm/backends/dri/gbm_dri.c
>> +++ b/src/gbm/backends/dri/gbm_dri.c
>> @@ -1023,13 +1023,20 @@ free_bo:
>>  static struct gbm_bo *
>>  gbm_dri_bo_create(struct gbm_device *gbm,
>>uint32_t width, uint32_t height,
>> -  uint32_t format, uint32_t usage)
>> +  uint32_t format, uint32_t usage,
>> +  const uint64_t *modifiers,
>> +  const unsigned int count)
>>  {
>> struct gbm_dri_device *dri = gbm_dri_device(gbm);
>> struct gbm_dri_bo *bo;
>> int dri_format;
>> unsigned dri_use = 0;
>>
>> +   /* Callers of this may specify a modifier, or a dri usage, but not
>> both. The
>> +* newer modifier interface deprecates the older usage flags.
>> +*/
>> +   assert(!(usage && count));
>> +
>> if (usage & GBM_BO_USE_WRITE || dri->image == NULL)
>>return create_dumb(gbm, width, height, format, usage);
>>
>> @@ -1087,11 +1094,24 @@ gbm_dri_bo_create(struct gbm_device *gbm,
>> /* Gallium drivers requires shared in order to get the handle/stride
>> */
>> dri_use |= __DRI_IMAGE_USE_SHARE;
>>
>> -   bo->image =
>> -  dri->image->createImage(dri->screen,
>> -  width, height,
>> -  dri_format, dri_use,
>> -  bo);
>> +   if (!dri->image || dri->image->base.version < 14 ||
>> +   !dri->image->createImageWithModifiers) {
>> +  if (modifiers) {
>>
>
> This logic still isn't correct.  As written, it will always call
> createImageWithModifiers if it's available even if no modifiers are
> provided.  I think you want the "if (modifiers)" to be the outside bit of
> control-flow
>
>
>> + fprintf(stderr, "Modifiers specified, but DRI is too old\n");
>> + errno = ENOSYS;
>> + goto failed;
>> +  }
>> +
>> +  bo->image = dri->image->createImage(dri->screen, width, height,
>> +  dri_format, dri_use, bo);
>> +   } else {
>> +  bo->image =
>> + dri->image->createImageWithModifiers(dri->screen,
>> +  width, height,
>> +  dri_format,
>> +  modifiers, count,
>> +  bo);
>> +   }
>> if (bo->image == NULL)
>>goto failed;
>>
>> @@ -1165,19 +1185,44 @@ gbm_dri_bo_unmap(struct gbm_bo *_bo, void
>> *map_data)
>>  static struct gbm_surface *
>>  gbm_dri_surface_create(struct gbm_device *gbm,
>> uint32_t width, uint32_t height,
>> -  uint32_t format, uint32_t flags)
>> +  

Re: [Mesa-dev] [PATCH 3/6] gbm: Introduce modifiers into surface/bo creation

[Jason Ekstrand]

On Mon, Mar 13, 2017 at 10:24 PM, Ben Widawsky  wrote:

> The idea behind modifiers like this is that the user of GBM will have
> some mechanism to query what properties the hardware supports for its BO
> or surface. This information is directly passed in (and stored) so that
> the DRI implementation can create an image with the appropriate
> attributes.
>
> A getter() will be added later so that the user GBM will be able to
> query what modifier should be used.
>
> Only in surface creation, the modifiers are stored until the BO is
> actually allocated. In regular buffer allocation, the correct modifier
> can (will be, in future patches be chosen at creation time.
>
> v2: Make sure to check if count is non-zero in addition to testing if
> calloc fails. (Daniel)
>
> v3: Remove "usage" and "flags" from modifier creation. Requested by
> Kristian.
>
> v4: Take advantage of the "INVALID" modifier added by the GET_PLANE2
> series.
>
> v5: Don't bother with storing modifiers for gbm_bo_create because that's
> a synchronous operation and we can actually select the correct modifier
> at create time (done in a later patch) (Jason)
>
> Cc: Kristian Høgsberg 
> Cc: Jason Ekstrand 
> References (v4): https://lists.freedesktop.org/archives/intel-gfx/2017-
> January/116636.html
> Signed-off-by: Ben Widawsky 
> Reviewed-by: Eric Engestrom  (v1)
> Acked-by: Daniel Stone 
> ---
>  src/gbm/backends/dri/gbm_dri.c | 62 ++
> ++--
>  src/gbm/gbm-symbols-check  |  2 ++
>  src/gbm/main/gbm.c | 39 --
>  src/gbm/main/gbm.h | 12 
>  src/gbm/main/gbmint.h  | 12 ++--
>  5 files changed, 115 insertions(+), 12 deletions(-)
>
> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_
> dri.c
> index 7106dc1229..1224ce4476 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -1023,13 +1023,20 @@ free_bo:
>  static struct gbm_bo *
>  gbm_dri_bo_create(struct gbm_device *gbm,
>uint32_t width, uint32_t height,
> -  uint32_t format, uint32_t usage)
> +  uint32_t format, uint32_t usage,
> +  const uint64_t *modifiers,
> +  const unsigned int count)
>  {
> struct gbm_dri_device *dri = gbm_dri_device(gbm);
> struct gbm_dri_bo *bo;
> int dri_format;
> unsigned dri_use = 0;
>
> +   /* Callers of this may specify a modifier, or a dri usage, but not
> both. The
> +* newer modifier interface deprecates the older usage flags.
> +*/
> +   assert(!(usage && count));
> +
> if (usage & GBM_BO_USE_WRITE || dri->image == NULL)
>return create_dumb(gbm, width, height, format, usage);
>
> @@ -1087,11 +1094,24 @@ gbm_dri_bo_create(struct gbm_device *gbm,
> /* Gallium drivers requires shared in order to get the handle/stride */
> dri_use |= __DRI_IMAGE_USE_SHARE;
>
> -   bo->image =
> -  dri->image->createImage(dri->screen,
> -  width, height,
> -  dri_format, dri_use,
> -  bo);
> +   if (!dri->image || dri->image->base.version < 14 ||
> +   !dri->image->createImageWithModifiers) {
> +  if (modifiers) {
>

This logic still isn't correct.  As written, it will always call
createImageWithModifiers if it's available even if no modifiers are
provided.  I think you want the "if (modifiers)" to be the outside bit of
control-flow


> + fprintf(stderr, "Modifiers specified, but DRI is too old\n");
> + errno = ENOSYS;
> + goto failed;
> +  }
> +
> +  bo->image = dri->image->createImage(dri->screen, width, height,
> +  dri_format, dri_use, bo);
> +   } else {
> +  bo->image =
> + dri->image->createImageWithModifiers(dri->screen,
> +  width, height,
> +  dri_format,
> +  modifiers, count,
> +  bo);
> +   }
> if (bo->image == NULL)
>goto failed;
>
> @@ -1165,19 +1185,44 @@ gbm_dri_bo_unmap(struct gbm_bo *_bo, void
> *map_data)
>  static struct gbm_surface *
>  gbm_dri_surface_create(struct gbm_device *gbm,
> uint32_t width, uint32_t height,
> -  uint32_t format, uint32_t flags)
> +  uint32_t format, uint32_t flags,
> +   const uint64_t *modifiers, const unsigned count)
>  {
> +   struct gbm_dri_device *dri = gbm_dri_device(gbm);
> struct gbm_dri_surface *surf;
>
> +   if (modifiers &&
> +   (!dri->image || dri->image->base.version < 14 ||
> +!dri->image->createImageWithModifiers)) {
> +  errno = 

[Mesa-dev] [Bug 99987] Mesa 13+ breaks Xvnc (and similar X servers)

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=99987

--- Comment #14 from Pierre Ossman  ---
That does make things work. But I'm hesitant to consider the issue closed
though. There are some outstanding questions:

a) I'm still getting direct rendering, despite the file being called
libGLX_indirect.so.0. Is the naming more an historical glitch and it should be
read as libGLX_fallback.so.0?

b) If it is indeed a fallback, why is that needed? I.e. why isn't Mesa being
chosen? It does not seem robust and future proof to rely on a fallback
mechanism. Which leads to...

c) What happens when both Mesa and NVIDIA wants to install the fallback
symlink? It sounds like things will just stop working, getting us right back to
the situation that GLVND was supposed to solve.

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

[Mesa-dev] [Bug 100199] clover: implement non-blocking clEnqueueWriteBuffer and clEnqueueReadBuffer

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100199

--- Comment #1 from Vedran Miletić  ---
Created attachment 130217
  --> https://bugs.freedesktop.org/attachment.cgi?id=130217=edit
Attempt at a fix

-- 
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] vulkan: enums: fix generation with enum aliases

[Jason Ekstrand]

On Tue, Mar 14, 2017 at 4:04 AM, Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> It seems new extensions will start to use aliases in enums, meaning
> multiple enums with the same value. This forces us to track the values
> of the enums to generate correct C code.
>

Seriously?  That seems a bit sketchy.  Or is this for promoted extensions?

The patch looks correct assuming this is something we actually want.


> Signed-off-by: Lionel Landwerlin 
> Cc: Dylan Baker 
> ---
>  src/vulkan/util/gen_enum_to_str.py | 49 ++
> 
>  1 file changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/src/vulkan/util/gen_enum_to_str.py
> b/src/vulkan/util/gen_enum_to_str.py
> index fb31addf94..1ffe4e362b 100644
> --- a/src/vulkan/util/gen_enum_to_str.py
> +++ b/src/vulkan/util/gen_enum_to_str.py
> @@ -24,6 +24,7 @@
>  from __future__ import print_function
>  import argparse
>  import os
> +import re
>  import textwrap
>  import xml.etree.cElementTree as et
>
> @@ -67,9 +68,9 @@ C_TEMPLATE = Template(textwrap.dedent(u"""\
>  vk_${enum.name[2:]}_to_str(${enum.name} input)
>  {
>  switch(input) {
> -% for v in enum.values:
> -case ${v}:
> -return "${v}";
> +% for k, v in enum.values.iteritems():
> +case ${v[0]}:
> +return "${' / '.join(v)}";
>  % endfor
>  default:
>  unreachable("Undefined enum value.");
> @@ -116,10 +117,17 @@ class EnumFactory(object):
>  class VkEnum(object):
>  """Simple struct-like class representing a single Vulkan Enum."""
>
> -def __init__(self, name, values=None):
> +def __init__(self, name):
>  self.name = name
> -self.values = values or []
> +self.values = {}
> +self.names = {}
>
> +def add_item(self, name, value):
> +if value in self.values:
> +self.values[value].append(name)
> +else:
> +self.values[value] = [name]
> +self.names[name] = value
>
>  def xml_parser(filename):
>  """Parse the XML file and return parsed data.
> @@ -129,6 +137,24 @@ def xml_parser(filename):
>  """
>  efactory = EnumFactory(VkEnum)
>
> +number_regexp = re.compile('^-?[0-9]+$')
> +
> +def compute_enum_value(enum, elem, parent):
> +if 'offset' in elem.attrib:
> +# Formula taken from the src/spec/generator.py
> +# https://github.com/khronosGroup/Vulkan-Docs
> +value = 10 + (int(parent.attrib['number']) - 1) * 1000
> +value += int(elem.attrib['offset'])
> +if 'dir' in e.attrib:
> +value = -value
> +else:
> +number = elem.attrib['value']
> +if re.match(number_regexp, number):
> +value = int(number)
> +else:
> +value = enum.names[number]
> +return value
> +
>  with open(filename, 'rb') as f:
>  context = iter(et.iterparse(f, events=('start', 'end')))
>
> @@ -140,16 +166,19 @@ def xml_parser(filename):
>  for event, elem in context:
>  if event == 'end' and elem.tag == 'enums':
>  type_ = elem.attrib.get('type')
> -if type_ == 'enum':
> -enum = efactory(elem.attrib['name'])
> -enum.values.extend([e.attrib['name'] for e in elem
> -if e.tag == 'enum'])
> +if type_ != 'enum':
> +continue
> +enum = efactory(elem.attrib['name'])
> +for e in elem:
> +if e.tag != 'enum':
> +continue
> +enum.add_item(e.attrib['name'],
> compute_enum_value(enum, e, None))
>  elif event == 'end' and elem.tag == 'extension':
>  if elem.attrib['supported'] != 'vulkan':
>  continue
>  for e in elem.findall('.//enum[@extends][@offset]'):
>  enum = efactory(e.attrib['extends'])
> -enum.values.append(e.attrib['name'])
> +enum.add_item(e.attrib['name'],
> compute_enum_value(enum, e, elem))
>
>  root.clear()
>
> --
> 2.11.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 100199] clover: implement non-blocking clEnqueueWriteBuffer and clEnqueueReadBuffer

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100199

Vedran Miletić  changed:

   What|Removed |Added

   Assignee|mesa-dev@lists.freedesktop. |ved...@miletic.net
   |org |

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

[Mesa-dev] [Bug 100199] clover: implement non-blocking clEnqueueWriteBuffer and clEnqueueReadBuffer

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100199

Bug ID: 100199
   Summary: clover: implement non-blocking clEnqueueWriteBuffer
and clEnqueueReadBuffer
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Other
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: ved...@miletic.net
QA Contact: mesa-dev@lists.freedesktop.org

Right now most of the transfer functions do something roughly in between
blocking and non-blocking transfers, and blocking boolean parameter is ignored.
We should implement support for non-blocking transfer and clearly separate the
two.

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

[Mesa-dev] mx6q: Cannot run Cinematic demo correctly

[Fabio Estevam]

Hi,

On a imx6q-sabresd board running kernel 4.9.14, mesa 17.0.1 and QT5.8:

# export QT_QPA_EGLFS_KMS_CONFIG=/media/sabresd.json

where sabresd.json contains:

{
  "device": "/dev/dri/card1",
  "hwcursor": false,
  "pbuffers": true,
  "outputs": [
{
  "name": "HDMI-1",
  "mode": "off"
},
{
  "name": "LVDS-1",
  "mode": "1024x768"
}
  ]
}

# /usr/share/Qt5/CinematicExperience/Qt5_CinematicExperience
libpng warning: iCCP: known incorrect sRGB profile
libpng warning: iCCP: known incorrect sRGB profile
QOpenGLFramebufferObject: Unsupported framebuffer format.
QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
QOpenGLFramebufferObject: Unsupported framebuffer format.
QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
QOpenGLFramebufferObject: Unsupported framebuffer format.
QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.
QOpenGLFramebufferObject: Unsupported framebuffer format.
QOpenGLFramebufferObject: Framebuffer incomplete, missing attachment.

The cover of the movies are just black rectangles instead of showing
the actual movie pictures.

As Gary Bisson noticed, if we go to bottom left setting icon and
select 'Show movies cover lighting', then all boxes show the correct
pictures, but fonts in the descriptions are still wrong.

Here is a video clip showing the the board in action:
https://www.dropbox.com/s/uuqcr3opjwlfjj5/QT5_8_on_mx6q.mp4?dl=0

Any ideas as to how to fix these problems?

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

[Mesa-dev] [PATCH] radv: Flush before copying with PKT3_WRITE_DATA in CmdUpdateBuffer

[Alex Smith]

Need to flush before updating the buffer to ensure that the copy is
ordered after previous accesses (assuming the app has performed the
appropriate barriers).

This fixes potential issues due to draws prior to an update reading
the new buffer content, despite having the necessary barriers between
them.

Signed-off-by: Alex Smith 
---
 src/amd/vulkan/radv_meta_buffer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/amd/vulkan/radv_meta_buffer.c 
b/src/amd/vulkan/radv_meta_buffer.c
index cd2973f..1e94f3b 100644
--- a/src/amd/vulkan/radv_meta_buffer.c
+++ b/src/amd/vulkan/radv_meta_buffer.c
@@ -524,6 +524,8 @@ void radv_CmdUpdateBuffer(
assert(!(va & 3));
 
if (dataSize < 4096) {
+   si_emit_cache_flush(cmd_buffer);
+
cmd_buffer->device->ws->cs_add_buffer(cmd_buffer->cs, 
dst_buffer->bo, 8);
 
radeon_check_space(cmd_buffer->device->ws, cmd_buffer->cs, 
words + 4);
-- 
2.9.3

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

[Mesa-dev] [PATCH 4/4] anv: Use subpass dependencies for flushes

[Jason Ekstrand]

Instead of figuring it all out ourselves, just use the information given
to us by the client.
---
 src/intel/vulkan/anv_blorp.c   | 88 --
 src/intel/vulkan/genX_cmd_buffer.c | 10 +
 2 files changed, 18 insertions(+), 80 deletions(-)

diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
index 8de339c..41966d6 100644
--- a/src/intel/vulkan/anv_blorp.c
+++ b/src/intel/vulkan/anv_blorp.c
@@ -1070,80 +1070,6 @@ enum subpass_stage {
 };
 
 static bool
-attachment_needs_flush(struct anv_cmd_buffer *cmd_buffer,
-   struct anv_render_pass_attachment *att,
-   enum subpass_stage stage)
-{
-   struct anv_render_pass *pass = cmd_buffer->state.pass;
-   const uint32_t subpass_idx = anv_get_subpass_id(_buffer->state);
-
-   /* We handle this subpass specially based on the current stage */
-   enum anv_subpass_usage usage = att->subpass_usage[subpass_idx];
-   switch (stage) {
-   case SUBPASS_STAGE_LOAD:
-  if (usage & (ANV_SUBPASS_USAGE_INPUT | ANV_SUBPASS_USAGE_RESOLVE_SRC))
- return true;
-  break;
-
-   case SUBPASS_STAGE_DRAW:
-  if (usage & ANV_SUBPASS_USAGE_RESOLVE_SRC)
- return true;
-  break;
-
-   default:
-  break;
-   }
-
-   for (uint32_t s = subpass_idx + 1; s < pass->subpass_count; s++) {
-  usage = att->subpass_usage[s];
-
-  /* If this attachment is going to be used as an input in this or any
-   * future subpass, then we need to flush its cache and invalidate the
-   * texture cache.
-   */
-  if (att->subpass_usage[s] & ANV_SUBPASS_USAGE_INPUT)
- return true;
-
-  if (usage & (ANV_SUBPASS_USAGE_DRAW | ANV_SUBPASS_USAGE_RESOLVE_DST)) {
- /* We found another subpass that draws to this attachment.  We'll
-  * wait to resolve until then.
-  */
- return false;
-  }
-   }
-
-   return false;
-}
-
-static void
-anv_cmd_buffer_flush_attachments(struct anv_cmd_buffer *cmd_buffer,
- enum subpass_stage stage)
-{
-   struct anv_subpass *subpass = cmd_buffer->state.subpass;
-   struct anv_render_pass *pass = cmd_buffer->state.pass;
-
-   for (uint32_t i = 0; i < subpass->color_count; ++i) {
-  uint32_t att = subpass->color_attachments[i].attachment;
-  assert(att < pass->attachment_count);
-  if (attachment_needs_flush(cmd_buffer, >attachments[att], stage)) {
- cmd_buffer->state.pending_pipe_bits |=
-ANV_PIPE_TEXTURE_CACHE_INVALIDATE_BIT |
-ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT;
-  }
-   }
-
-   if (subpass->depth_stencil_attachment.attachment != VK_ATTACHMENT_UNUSED) {
-  uint32_t att = subpass->depth_stencil_attachment.attachment;
-  assert(att < pass->attachment_count);
-  if (attachment_needs_flush(cmd_buffer, >attachments[att], stage)) {
- cmd_buffer->state.pending_pipe_bits |=
-ANV_PIPE_TEXTURE_CACHE_INVALIDATE_BIT |
-ANV_PIPE_DEPTH_CACHE_FLUSH_BIT;
-  }
-   }
-}
-
-static bool
 subpass_needs_clear(const struct anv_cmd_buffer *cmd_buffer)
 {
const struct anv_cmd_state *cmd_state = _buffer->state;
@@ -1327,8 +1253,6 @@ anv_cmd_buffer_clear_subpass(struct anv_cmd_buffer 
*cmd_buffer)
}
 
blorp_batch_finish();
-
-   anv_cmd_buffer_flush_attachments(cmd_buffer, SUBPASS_STAGE_LOAD);
 }
 
 static void
@@ -1554,9 +1478,15 @@ anv_cmd_buffer_resolve_subpass(struct anv_cmd_buffer 
*cmd_buffer)
  subpass->color_attachments[i].attachment);
}
 
-   anv_cmd_buffer_flush_attachments(cmd_buffer, SUBPASS_STAGE_DRAW);
-
if (subpass->has_resolve) {
+  /* We are about to do some MSAA resolves.  We need to flush so that the
+   * result of writes to the MSAA color attachments show up in the sampler
+   * when we blit to the single-sampled resolve target.
+   */
+  cmd_buffer->state.pending_pipe_bits |=
+ ANV_PIPE_TEXTURE_CACHE_INVALIDATE_BIT |
+ ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT;
+
   for (uint32_t i = 0; i < subpass->color_count; ++i) {
  uint32_t src_att = subpass->color_attachments[i].attachment;
  uint32_t dst_att = subpass->resolve_attachments[i].attachment;
@@ -1594,8 +1524,6 @@ anv_cmd_buffer_resolve_subpass(struct anv_cmd_buffer 
*cmd_buffer)
 
  ccs_resolve_attachment(cmd_buffer, , dst_att);
   }
-
-  anv_cmd_buffer_flush_attachments(cmd_buffer, SUBPASS_STAGE_RESOLVE);
}
 
blorp_batch_finish();
diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index acb59d5..02dff44 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -2456,6 +2456,9 @@ void genX(CmdBeginRenderPass)(
genX(flush_pipeline_select_3d)(cmd_buffer);
 
genX(cmd_buffer_set_subpass)(cmd_buffer, pass->subpasses);
+
+   cmd_buffer->state.pending_pipe_bits |=
+  cmd_buffer->state.pass->subpass_flushes[0];
 }
 
 

[Mesa-dev] [PATCH 3/4] anv/pass: Record required pipe flushes

[Jason Ekstrand]

---
 src/intel/vulkan/anv_pass.c| 87 ++
 src/intel/vulkan/anv_private.h |  2 +
 2 files changed, 89 insertions(+)

diff --git a/src/intel/vulkan/anv_pass.c b/src/intel/vulkan/anv_pass.c
index 8d1768d..d6d3794 100644
--- a/src/intel/vulkan/anv_pass.c
+++ b/src/intel/vulkan/anv_pass.c
@@ -98,6 +98,7 @@ VkResult anv_CreateRenderPass(
if (pass->subpass_attachments == NULL)
   goto fail_subpass_usages;
 
+   bool has_color = false, has_depth = false, has_input = false;
p = pass->subpass_attachments;
for (uint32_t i = 0; i < pCreateInfo->subpassCount; i++) {
   const VkSubpassDescription *desc = >pSubpasses[i];
@@ -115,6 +116,7 @@ VkResult anv_CreateRenderPass(
 uint32_t a = desc->pInputAttachments[j].attachment;
 subpass->input_attachments[j] = desc->pInputAttachments[j];
 if (a != VK_ATTACHMENT_UNUSED) {
+   has_input = true;
pass->attachments[a].usage |= 
VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT;
pass->attachments[a].subpass_usage[i] |= 
ANV_SUBPASS_USAGE_INPUT;
pass->attachments[a].last_subpass_idx = i;
@@ -134,6 +136,7 @@ VkResult anv_CreateRenderPass(
 uint32_t a = desc->pColorAttachments[j].attachment;
 subpass->color_attachments[j] = desc->pColorAttachments[j];
 if (a != VK_ATTACHMENT_UNUSED) {
+   has_color = true;
pass->attachments[a].usage |= 
VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT;
pass->attachments[a].subpass_usage[i] |= ANV_SUBPASS_USAGE_DRAW;
pass->attachments[a].last_subpass_idx = i;
@@ -170,6 +173,7 @@ VkResult anv_CreateRenderPass(
  *p++ = subpass->depth_stencil_attachment =
 *desc->pDepthStencilAttachment;
  if (a != VK_ATTACHMENT_UNUSED) {
+has_depth = true;
 pass->attachments[a].usage |=
VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT;
 pass->attachments[a].subpass_usage[i] |= ANV_SUBPASS_USAGE_DRAW;
@@ -181,10 +185,92 @@ VkResult anv_CreateRenderPass(
   }
}
 
+   pass->subpass_flushes =
+  vk_zalloc2(>alloc, pAllocator,
+ (pass->subpass_count + 1) * sizeof(*pass->subpass_flushes),
+ 8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
+   if (pass->subpass_flushes == NULL)
+  goto fail_subpass_attachments;
+
+   for (uint32_t i = 0; i < pCreateInfo->dependencyCount; i++) {
+  const VkSubpassDependency *dep = >pDependencies[i];
+  if (dep->srcSubpass != VK_SUBPASS_EXTERNAL) {
+ pass->subpass_flushes[dep->dstSubpass] |=
+anv_pipe_invalidate_bits_for_access_flags(dep->dstAccessMask);
+  }
+
+  if (dep->dstSubpass != VK_SUBPASS_EXTERNAL) {
+ pass->subpass_flushes[dep->srcSubpass + 1] |=
+anv_pipe_flush_bits_for_access_flags(dep->srcAccessMask);
+  }
+   }
+
+   /* From the Vulkan 1.0.39 spec:
+*
+*If there is no subpass dependency from VK_SUBPASS_EXTERNAL to the
+*first subpass that uses an attachment, then an implicit subpass
+*dependency exists from VK_SUBPASS_EXTERNAL to the first subpass it is
+*used in. The subpass dependency operates as if defined with the
+*following parameters:
+*
+*VkSubpassDependency implicitDependency = {
+*.srcSubpass = VK_SUBPASS_EXTERNAL;
+*.dstSubpass = firstSubpass; // First subpass attachment is used in
+*.srcStageMask = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT;
+*.dstStageMask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT;
+*.srcAccessMask = 0;
+*.dstAccessMask = VK_ACCESS_INPUT_ATTACHMENT_READ_BIT |
+* VK_ACCESS_COLOR_ATTACHMENT_READ_BIT |
+* VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT |
+* VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT |
+* VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT;
+*.dependencyFlags = 0;
+*};
+*
+*Similarly, if there is no subpass dependency from the last subpass
+*that uses an attachment to VK_SUBPASS_EXTERNAL, then an implicit
+*subpass dependency exists from the last subpass it is used in to
+*VK_SUBPASS_EXTERNAL. The subpass dependency operates as if defined
+*with the following parameters:
+*
+*VkSubpassDependency implicitDependency = {
+*.srcSubpass = lastSubpass; // Last subpass attachment is used in
+*.dstSubpass = VK_SUBPASS_EXTERNAL;
+*.srcStageMask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT;
+*.dstStageMask = VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT;
+*.srcAccessMask = VK_ACCESS_INPUT_ATTACHMENT_READ_BIT |
+* VK_ACCESS_COLOR_ATTACHMENT_READ_BIT |
+* VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT |
+* 

[Mesa-dev] [PATCH 2/4] anv/pass: Use goto-based error handling in CreateRenderPass

[Jason Ekstrand]

---
 src/intel/vulkan/anv_pass.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/intel/vulkan/anv_pass.c b/src/intel/vulkan/anv_pass.c
index 4a1a340..8d1768d 100644
--- a/src/intel/vulkan/anv_pass.c
+++ b/src/intel/vulkan/anv_pass.c
@@ -59,10 +59,8 @@ VkResult anv_CreateRenderPass(
  pass->subpass_count * pass->attachment_count *
sizeof(*pass->subpass_usages),
  8, VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
-   if (pass->subpass_usages == NULL) {
-  vk_free2(>alloc, pAllocator, pass);
-  return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
-   }
+   if (pass->subpass_usages == NULL)
+  goto fail_pass;
 
enum anv_subpass_usage *usages = pass->subpass_usages;
for (uint32_t i = 0; i < pCreateInfo->attachmentCount; i++) {
@@ -97,11 +95,8 @@ VkResult anv_CreateRenderPass(
   vk_alloc2(>alloc, pAllocator,
  subpass_attachment_count * sizeof(VkAttachmentReference), 8,
  VK_SYSTEM_ALLOCATION_SCOPE_OBJECT);
-   if (pass->subpass_attachments == NULL) {
-  vk_free2(>alloc, pAllocator, pass->subpass_usages);
-  vk_free2(>alloc, pAllocator, pass);
-  return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
-   }
+   if (pass->subpass_attachments == NULL)
+  goto fail_subpass_usages;
 
p = pass->subpass_attachments;
for (uint32_t i = 0; i < pCreateInfo->subpassCount; i++) {
@@ -189,6 +184,13 @@ VkResult anv_CreateRenderPass(
*pRenderPass = anv_render_pass_to_handle(pass);
 
return VK_SUCCESS;
+
+fail_subpass_usages:
+   vk_free2(>alloc, pAllocator, pass->subpass_usages);
+fail_pass:
+   vk_free2(>alloc, pAllocator, pass);
+
+   return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
 }
 
 void anv_DestroyRenderPass(
-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [PATCH 1/4] anv: Add helpers for converting access flags to pipe bits

[Jason Ekstrand]

---
 src/intel/vulkan/anv_private.h | 59 ++
 src/intel/vulkan/genX_cmd_buffer.c | 48 ++-
 2 files changed, 62 insertions(+), 45 deletions(-)

diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index b11adfd..a0eefe3 100644
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -1156,6 +1156,65 @@ enum anv_pipe_bits {
ANV_PIPE_TEXTURE_CACHE_INVALIDATE_BIT | \
ANV_PIPE_INSTRUCTION_CACHE_INVALIDATE_BIT)
 
+static inline enum anv_pipe_bits
+anv_pipe_flush_bits_for_access_flags(VkAccessFlags flags)
+{
+   enum anv_pipe_bits pipe_bits = 0;
+
+   unsigned b;
+   for_each_bit(b, flags) {
+  switch ((VkAccessFlagBits)(1 << b)) {
+  case VK_ACCESS_SHADER_WRITE_BIT:
+ pipe_bits |= ANV_PIPE_DATA_CACHE_FLUSH_BIT;
+ break;
+  case VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT:
+ pipe_bits |= ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT;
+ break;
+  case VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT:
+ pipe_bits |= ANV_PIPE_DEPTH_CACHE_FLUSH_BIT;
+ break;
+  case VK_ACCESS_TRANSFER_WRITE_BIT:
+ pipe_bits |= ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT;
+ pipe_bits |= ANV_PIPE_DEPTH_CACHE_FLUSH_BIT;
+ break;
+  default:
+ break; /* Nothing to do */
+  }
+   }
+
+   return pipe_bits;
+}
+
+static inline enum anv_pipe_bits
+anv_pipe_invalidate_bits_for_access_flags(VkAccessFlags flags)
+{
+   enum anv_pipe_bits pipe_bits = 0;
+
+   unsigned b;
+   for_each_bit(b, flags) {
+  switch ((VkAccessFlagBits)(1 << b)) {
+  case VK_ACCESS_INDIRECT_COMMAND_READ_BIT:
+  case VK_ACCESS_INDEX_READ_BIT:
+  case VK_ACCESS_VERTEX_ATTRIBUTE_READ_BIT:
+ pipe_bits |= ANV_PIPE_VF_CACHE_INVALIDATE_BIT;
+ break;
+  case VK_ACCESS_UNIFORM_READ_BIT:
+ pipe_bits |= ANV_PIPE_CONSTANT_CACHE_INVALIDATE_BIT;
+ pipe_bits |= ANV_PIPE_TEXTURE_CACHE_INVALIDATE_BIT;
+ break;
+  case VK_ACCESS_SHADER_READ_BIT:
+  case VK_ACCESS_INPUT_ATTACHMENT_READ_BIT:
+  case VK_ACCESS_TRANSFER_READ_BIT:
+ pipe_bits |= ANV_PIPE_TEXTURE_CACHE_INVALIDATE_BIT;
+ break;
+  default:
+ break; /* Nothing to do */
+  }
+   }
+
+   return pipe_bits;
+}
+
 struct anv_vertex_binding {
struct anv_buffer *  buffer;
VkDeviceSize offset;
diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index a12bd67..acb59d5 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -920,7 +920,6 @@ void genX(CmdPipelineBarrier)(
 const VkImageMemoryBarrier* pImageMemoryBarriers)
 {
ANV_FROM_HANDLE(anv_cmd_buffer, cmd_buffer, commandBuffer);
-   uint32_t b;
 
/* XXX: Right now, we're really dumb and just flush whatever categories
 * the app asks for.  One of these days we may make this a bit better
@@ -951,50 +950,9 @@ void genX(CmdPipelineBarrier)(
   }
}
 
-   enum anv_pipe_bits pipe_bits = 0;
-
-   for_each_bit(b, src_flags) {
-  switch ((VkAccessFlagBits)(1 << b)) {
-  case VK_ACCESS_SHADER_WRITE_BIT:
- pipe_bits |= ANV_PIPE_DATA_CACHE_FLUSH_BIT;
- break;
-  case VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT:
- pipe_bits |= ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT;
- break;
-  case VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT:
- pipe_bits |= ANV_PIPE_DEPTH_CACHE_FLUSH_BIT;
- break;
-  case VK_ACCESS_TRANSFER_WRITE_BIT:
- pipe_bits |= ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT;
- pipe_bits |= ANV_PIPE_DEPTH_CACHE_FLUSH_BIT;
- break;
-  default:
- break; /* Nothing to do */
-  }
-   }
-
-   for_each_bit(b, dst_flags) {
-  switch ((VkAccessFlagBits)(1 << b)) {
-  case VK_ACCESS_INDIRECT_COMMAND_READ_BIT:
-  case VK_ACCESS_INDEX_READ_BIT:
-  case VK_ACCESS_VERTEX_ATTRIBUTE_READ_BIT:
- pipe_bits |= ANV_PIPE_VF_CACHE_INVALIDATE_BIT;
- break;
-  case VK_ACCESS_UNIFORM_READ_BIT:
- pipe_bits |= ANV_PIPE_CONSTANT_CACHE_INVALIDATE_BIT;
- pipe_bits |= ANV_PIPE_TEXTURE_CACHE_INVALIDATE_BIT;
- break;
-  case VK_ACCESS_SHADER_READ_BIT:
-  case VK_ACCESS_INPUT_ATTACHMENT_READ_BIT:
-  case VK_ACCESS_TRANSFER_READ_BIT:
- pipe_bits |= ANV_PIPE_TEXTURE_CACHE_INVALIDATE_BIT;
- break;
-  default:
- break; /* Nothing to do */
-  }
-   }
-
-   cmd_buffer->state.pending_pipe_bits |= pipe_bits;
+   cmd_buffer->state.pending_pipe_bits |=
+  anv_pipe_flush_bits_for_access_flags(src_flags) |
+  anv_pipe_invalidate_bits_for_access_flags(dst_flags);
 }
 
 static void
-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [PATCH 1/2] anv/image: Move handling of INTEL_VK_HIZ

[Jason Ekstrand]

This makes it so that you don't get an "Implement gen7 HiZ" perf warning
when you manually disable HiZ on gen8.
---
 src/intel/vulkan/anv_image.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
index 5f17351..931ee22 100644
--- a/src/intel/vulkan/anv_image.c
+++ b/src/intel/vulkan/anv_image.c
@@ -190,7 +190,7 @@ make_surface(const struct anv_device *dev,
*/
   if (!(image->usage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT)) {
  /* It will never be used as an attachment, HiZ is pointless. */
-  } else if (!env_var_as_boolean("INTEL_VK_HIZ", dev->info.gen >= 8)) {
+  } else if (dev->info.gen == 7) {
  anv_perf_warn("Implement gen7 HiZ");
   } else if (vk_info->mipLevels > 1) {
  anv_perf_warn("Enable multi-LOD HiZ");
@@ -198,7 +198,7 @@ make_surface(const struct anv_device *dev,
  anv_perf_warn("Implement multi-arrayLayer HiZ clears and resolves");
   } else if (dev->info.gen == 8 && vk_info->samples > 1) {
  anv_perf_warn("Enable gen8 multisampled HiZ");
-  } else {
+  } else if (env_var_as_boolean("INTEL_VK_HIZ", true)) {
  assert(image->aux_surface.isl.size == 0);
  ok = isl_surf_get_hiz_surf(>isl_dev, >depth_surface.isl,
 >aux_surface.isl);
-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [PATCH 2/2] intel/debug: Add a common INTEL_DEBUG=nohiz option

[Jason Ekstrand]

The GL driver had a driconf option (which doesn't make much sense) and
the Vulkan driver had a hand-rolled environment variable.  Instead,
let's tie both into the INTEL_DEBUG mechanism and unify things.
---
 src/intel/common/gen_debug.c | 1 +
 src/intel/common/gen_debug.h | 1 +
 src/intel/vulkan/anv_image.c | 2 +-
 src/mesa/drivers/dri/i965/brw_context.c  | 2 +-
 src/mesa/drivers/dri/i965/intel_screen.c | 4 
 5 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/intel/common/gen_debug.c b/src/intel/common/gen_debug.c
index 1e0ba57..f90f00d 100644
--- a/src/intel/common/gen_debug.c
+++ b/src/intel/common/gen_debug.c
@@ -82,6 +82,7 @@ static const struct debug_control debug_control[] = {
{ "l3",  DEBUG_L3 },
{ "do32",DEBUG_DO32 },
{ "norbc",   DEBUG_NO_RBC },
+   { "nohiz",   DEBUG_NO_HIZ },
{ NULL,0 }
 };
 
diff --git a/src/intel/common/gen_debug.h b/src/intel/common/gen_debug.h
index db13a06..d007aeb 100644
--- a/src/intel/common/gen_debug.h
+++ b/src/intel/common/gen_debug.h
@@ -78,6 +78,7 @@ extern uint64_t INTEL_DEBUG;
 #define DEBUG_L3  (1ull << 36)
 #define DEBUG_DO32(1ull << 37)
 #define DEBUG_NO_RBC  (1ull << 38)
+#define DEBUG_NO_HIZ  (1ull << 39)
 
 #ifdef HAVE_ANDROID_PLATFORM
 #define LOG_TAG "INTEL-MESA"
diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
index 931ee22..7be988a 100644
--- a/src/intel/vulkan/anv_image.c
+++ b/src/intel/vulkan/anv_image.c
@@ -198,7 +198,7 @@ make_surface(const struct anv_device *dev,
  anv_perf_warn("Implement multi-arrayLayer HiZ clears and resolves");
   } else if (dev->info.gen == 8 && vk_info->samples > 1) {
  anv_perf_warn("Enable gen8 multisampled HiZ");
-  } else if (env_var_as_boolean("INTEL_VK_HIZ", true)) {
+  } else if (!unlikely(INTEL_DEBUG & DEBUG_NO_HIZ)) {
  assert(image->aux_surface.isl.size == 0);
  ok = isl_surf_get_hiz_surf(>isl_dev, >depth_surface.isl,
 >aux_surface.isl);
diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
b/src/mesa/drivers/dri/i965/brw_context.c
index 0e882ca..32cfb2e 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -880,7 +880,7 @@ brw_process_driconf_options(struct brw_context *brw)
   break;
}
 
-   if (!driQueryOptionb(options, "hiz")) {
+   if (INTEL_DEBUG & DEBUG_NO_HIZ) {
brw->has_hiz = false;
/* On gen6, you can only do separate stencil with HIZ. */
if (brw->gen == 6)
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
b/src/mesa/drivers/dri/i965/intel_screen.c
index 21786eb..10dab23 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -56,10 +56,6 @@ DRI_CONF_BEGIN
DRI_CONF_ENUM(1, "Enable reuse of all sizes of buffer objects")
 DRI_CONF_DESC_END
   DRI_CONF_OPT_END
-
-  DRI_CONF_OPT_BEGIN_B(hiz, "true")
-DRI_CONF_DESC(en, "Enable Hierarchical Z on gen6+")
-  DRI_CONF_OPT_END
DRI_CONF_SECTION_END
 
DRI_CONF_SECTION_QUALITY
-- 
2.5.0.400.gff86faf

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

[Mesa-dev] [Bug 99467] [radv] DOOM 2016 + wine. Green screen everywhere (but can be started)

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=99467

--- Comment #22 from Vedran Miletić  ---
(In reply to Drago from comment #21)
> Since workaround is not that big, can't it be committed and activated with
> ENV var switch, until Id fixes the game. This will allow stressing the RADV
> driver with the most demanding Vulkan game we have, and polishing other
> parts of the driver.
> 

That would be the equivalent of drirc with OpenGL. Again, I don't think that's
the path we should take, but it's ultimately maintainer's decision.

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

Re: [Mesa-dev] [PATCH] anv/blorp: Only set a clear color for resolves if fast-cleared

[Jason Ekstrand]

On Tue, Mar 14, 2017 at 5:23 AM, Emil Velikov 
wrote:

> On 14 March 2017 at 01:45, Jason Ekstrand  wrote:
> > On Mon, Mar 13, 2017 at 5:34 PM, Emil Velikov 
> > wrote:
> >>
> >> On 13 March 2017 at 19:08, Jason Ekstrand  wrote:
> >> > On Mon, Mar 13, 2017 at 11:19 AM, Emil Velikov
> >> > 
> >> > wrote:
> >> >>
> >> >> On 5 March 2017 at 16:36, Eduardo Lima Mitev 
> wrote:
> >> >> > Reviewed-by: Eduardo Lima Mitev 
> >> >> >
> >> >> > On 03/04/2017 01:56 AM, Jason Ekstrand wrote:
> >> >> >> Cc: "17.0" 
> >> >> >> ---
> >> >> >>  src/intel/vulkan/anv_blorp.c | 3 ++-
> >> >> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >> >> >>
> >> >> Is there anything wrong with the patch or it simply fell through the
> >> >> cracks ?
> >> >
> >> >
> >> > There was a small issue but both this patch and the fix landed this
> >> > morning.
> >> >
> >> Right so the patch is superseded [by 6b644e571e2 ("anv: Stall before
> >> fast-clear operations")] - I wasn't too sure.
> >
> >
> > No, it isn't.  They're completely separate patches.
> >
> Barring that one, I cannot spot anything alike the the above patch
> that landed yesterday morning.
> That said - I trust you tagged the patch and if so you'll see it in
> -stable.
>

I thought I'd pushed it yesterday but I hadn't. :(  I'll push today.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] anv: Properly enumerate physical devices when none are present

[Jason Ekstrand]

---
 src/intel/vulkan/anv_device.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
index d2d9117..c73b9af 100644
--- a/src/intel/vulkan/anv_device.c
+++ b/src/intel/vulkan/anv_device.c
@@ -412,8 +412,11 @@ VkResult anv_EnumeratePhysicalDevices(
   }
}
 
-   vk_outarray_append(, i) {
-  *i = anv_physical_device_to_handle(>physicalDevice);
+   if (instance->physicalDeviceCount > 0) {
+  assert(instance->physicalDeviceCount == 1);
+  vk_outarray_append(, i) {
+ *i = anv_physical_device_to_handle(>physicalDevice);
+  }
}
 
return vk_outarray_status();
-- 
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] [libdrm PATCH] intel: Make unsynchronized GTT mappings work on systems with snooping.

[Eero Tamminen]

Hi,

On 14.03.2017 12:48, Eero Tamminen wrote:

On 11.03.2017 03:14, Kenneth Graunke wrote:

On systems without LLC, drm_intel_gem_bo_map_unsynchronized() has
had the surprising behavior of doing a synchronized GTT mapping.
This is obviously not what the user of the API wanted.

Eric left a comment indicating a valid concern: if the CPU and GPU
caches are incoherent, we don't keep track of where the user last
mapped the buffer, and what caches might contain relevant data.

Modern Atom systems still don't have LLC, but they do offer snooping,
which effectively makes the caches coherent.  The kernel appears to
set up the PTE/PPAT to enable snooping for everything where the cache
level is not I915_CACHE_NONE.  As far as I know, only scanout buffers
are marked as uncached.

Any buffers used by scanout should be flagged as non-reusable with
drm_intel_bo_disable_reuse(), prime export, or flink.  So, we can
assume that any reusable buffer should be snooped.

This patch enables unsynchronized mappings for reusable buffers
on all Gen6+ hardware (which have either LLC or snooping).

On Broxton, this improves the performance of Unigine Valley 1.0
on Low settings at 1280x720 by about 45%, and Unigine Heaven 4.0
(same settings) by about 53%.


I tested it with our normal set of benchmarks.

Using FullHD resolution and "high" quality settings, on Broxton, Valley
improved by ~11% and Heaven (with tessellation enabled) by 2-3%.


BSW: Valley +10%, Heaven +4%.  Blend showed regression, but it could 
still be within variance.


BYT: Valley +5%, Heaven +11%.  Rest of changes were within normal variance.


- Eero


CarChase seemed to improve also by several percents, but everything else
was within normal variation.

I'll check BYT & BSW too.


- Eero



Signed-off-by: Kenneth Graunke 
Cc: Chris Wilson 
Cc: mesa-dev@lists.freedesktop.org
---
 intel/intel_bufmgr_gem.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

It looks like Mesa and Beignet are the only callers of this function
(SNA and Anvil don't use libdrm, UXA and vaapi don't use this function.)

This passed our full barrage of Piglit/dEQP/GLCTS/GLESCTS testing.
gnome-shell still works, as does Unigine, and GLBenchmark.

I haven't tested any OpenCL workloads.

diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index e260f2dc..f53f1fcc 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -1630,9 +1630,7 @@ int
 drm_intel_gem_bo_map_unsynchronized(drm_intel_bo *bo)
 {
 drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)
bo->bufmgr;
-#ifdef HAVE_VALGRIND
 drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
-#endif
 int ret;

 /* If the CPU cache isn't coherent with the GTT, then use a
@@ -1641,8 +1639,12 @@
drm_intel_gem_bo_map_unsynchronized(drm_intel_bo *bo)
  * terms of drm_intel_bo_map vs drm_intel_gem_bo_map_gtt, so
  * we would potentially corrupt the buffer even when the user
  * does reasonable things.
+ *
+ * The caches are coherent on LLC platforms or snooping is enabled
+ * for the BO.  The kernel enables snooping for non-scanout
(reusable)
+ * buffers on modern non-LLC systems.
  */
-if (!bufmgr_gem->has_llc)
+if (bufmgr_gem->gen < 6 || !bo_gem->reusable)
 return drm_intel_gem_bo_map_gtt(bo);

 pthread_mutex_lock(_gem->lock);



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


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

Re: [Mesa-dev] [PATCH v2 10/24] anv: handle failures when growing reloc lists

[Pohjolainen, Topi]

On Tue, Mar 14, 2017 at 04:02:17PM +0200, Pohjolainen, Topi wrote:
> On Tue, Mar 14, 2017 at 02:08:10PM +0100, Iago Toral wrote:
> > On Tue, 2017-03-14 at 14:04 +0200, Pohjolainen, Topi wrote:
> > > On Tue, Mar 14, 2017 at 12:06:16PM +0100, Iago Toral wrote:
> > > > 
> > > > On Tue, 2017-03-14 at 12:01 +0100, Iago Toral wrote:
> > > > > 
> > > > > On Tue, 2017-03-14 at 11:25 +0200, Pohjolainen, Topi wrote:
> > > > > > 
> > > > > > 
> > > > > > On Fri, Mar 10, 2017 at 01:38:23PM +0100, Iago Toral Quiroga
> > > > > > wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > Growing the reloc list happens through calling
> > > > > > > anv_reloc_list_add()
> > > > > > > or
> > > > > > > anv_reloc_list_append(). Make sure that we call these through
> > > > > > > helpers
> > > > > > > that check the result and set the batch error status if
> > > > > > > needed.
> > > > > > > 
> > > > > > > v2:
> > > > > > >   - Handling the crashes is not good enough, we need to keep
> > > > > > > track
> > > > > > > of
> > > > > > > the error, for that, keep track of the errors in the
> > > > > > > batch
> > > > > > > instead (Jason).
> > > > > > >   - Make reloc list growth go through helpers so we can have
> > > > > > > a
> > > > > > > central
> > > > > > > place where we can do error tracking (Jason).
> > > > > > > ---
> > > > > > >  src/intel/vulkan/anv_batch_chain.c | 29
> > > > > > > 
> > > > > > > -
> > > > > > >  src/intel/vulkan/genX_blorp_exec.c |  7 +--
> > > > > > >  src/intel/vulkan/genX_cmd_buffer.c | 25 ++
> > > > > > > 
> > > > > > > ---
> > > > > > >  3 files changed, 39 insertions(+), 22 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/src/intel/vulkan/anv_batch_chain.c
> > > > > > > b/src/intel/vulkan/anv_batch_chain.c
> > > > > > > index 3774172..2add5bd 100644
> > > > > > > --- a/src/intel/vulkan/anv_batch_chain.c
> > > > > > > +++ b/src/intel/vulkan/anv_batch_chain.c
> > > > > > > @@ -151,8 +151,9 @@ anv_reloc_list_add(struct anv_reloc_list
> > > > > > > *list,
> > > > > > > const uint32_t domain =
> > > > > > >    target_bo->is_winsys_bo ? I915_GEM_DOMAIN_RENDER : 0;
> > > > > > >  
> > > > > > > -   anv_reloc_list_grow(list, alloc, 1);
> > > > > > > -   /* TODO: Handle failure */
> > > > > > > +   VkResult result = anv_reloc_list_grow(list, alloc, 1);
> > > > > > > +   if (result != VK_SUCCESS)
> > > > > > > +  return 0;
> > > > > > None of the callers of anv_reloc_list_add() or
> > > > > > anv_reloc_list_append() are
> > > > > > currently interested of the return value.
> > > > > and_reloc_list_append() already returns a VkResult.
> > > > > 
> > > > > As for the callers of anv_reloc_list_add(),
> > > > > anv_batch_emit_reloc()
> > > > > needs the offset computed by anv_reloc_list_add(), but that seems
> > > > > to
> > > > > be
> > > > > the only one, so we could do what you suggest below.
> > > > > 
> > > > > > 
> > > > > > 
> > > > > >  And if they were, they could
> > > > > > easily calculate it themselves: both target_bo and delta are
> > > > > > inputs.
> > > > > > So instead of relying on zero offset indicating error we could
> > > > > > simply
> > > > > > change the return type to VkResult. Or did I miss something?
> > > > > I think this should be fine.
> > > > Well, except that anv_batch_emit_reloc needs to return that offset
> > > > to
> > > > its callers, so even if receives a VkResult and computes the offset
> > > > itself, in the case of an error we still need to return an offset,
> > > > so I
> > > > don't think there is much gain in doing this, right?
> > > Oh, I missed that. Yeah, then it doesn't work. I'm just not super
> > > happy
> > > relying on offset == 0 indicating failure. Does the diff get out hand
> > > if 
> > > provided offset as outgoing argument (and returning VkResult)?
> > 
> > Right I don't particularly like this either, however, I am not sure we
> > can do much about it. This is called from two places:
> > 
> > 1) _blorp_combine_address() -> blorp_emit_reloc()
> > -> anv_batch_emit_reloc()
> > 
> > 2) _anv_combine_address() -> anv_batch_emit_reloc()
> > 
> > _blorp_combine_address and _anv_combine_address are wrapped by the
> > _gen_combine_address macros that are used all over the place in auto-
> > generated files to fill dwords in various state packets.

Couldn't we still make anv_batch_emit_reloc() return VkResult and to provide
the offset as argument. Those two callers, _anv_combine_address() and
blorp_emit_reloc(), could set the error state and like you said they could
return zeros as it doesn't matter what gets filled into the batch.

Would that work?

> > 
> > So the thing is, these state packets need to be filled one way or
> > another. If we don't they will just have garbage, so in case of
> > failure, writing 0 does not sound particularly bad to me, the focus
> > needs to be on avoiding to execute things that are known to be broken,
> > which is what we are trying to do with 

Re: [Mesa-dev] [PATCH kmscube 1/7] kmscube.c: remove uninitialized var returned

[Eric Engestrom]

> Subject: kmscube.c: remove uninitialized var returned

Huh? Not sure what my brain was thinking; changed that locally to
"don't return uninitialized variable" :)

On Tuesday, 2017-03-14 13:33:51 +, Eric Engestrom wrote:
> `ret` isn't used by anything, so remove it as well.
> 
> Signed-off-by: Eric Engestrom 
> ---
>  kmscube.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kmscube.c b/kmscube.c
> index 91e0104..8057e66 100644
> --- a/kmscube.c
> +++ b/kmscube.c
> @@ -67,7 +67,7 @@ int main(int argc, char *argv[])
>   const char *device = "/dev/dri/card0";
>   enum mode mode = SMOOTH;
>   int atomic = 0;
> - int opt, ret;
> + int opt;
>  
>   while ((opt = getopt_long_only(argc, argv, shortopts, longopts, NULL)) 
> != -1) {
>   switch (opt) {
> @@ -104,7 +104,7 @@ int main(int argc, char *argv[])
>   drm = init_drm_legacy(device);
>   if (!drm) {
>   printf("failed to initialize %s DRM\n", atomic ? "atomic" : 
> "legacy");
> - return ret;
> + return -1;
>   }
>  
>   gbm = init_gbm(drm->fd, drm->mode->hdisplay, drm->mode->vdisplay);
> -- 
> Cheers,
>   Eric
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 10/24] anv: handle failures when growing reloc lists

[Pohjolainen, Topi]

On Tue, Mar 14, 2017 at 02:08:10PM +0100, Iago Toral wrote:
> On Tue, 2017-03-14 at 14:04 +0200, Pohjolainen, Topi wrote:
> > On Tue, Mar 14, 2017 at 12:06:16PM +0100, Iago Toral wrote:
> > > 
> > > On Tue, 2017-03-14 at 12:01 +0100, Iago Toral wrote:
> > > > 
> > > > On Tue, 2017-03-14 at 11:25 +0200, Pohjolainen, Topi wrote:
> > > > > 
> > > > > 
> > > > > On Fri, Mar 10, 2017 at 01:38:23PM +0100, Iago Toral Quiroga
> > > > > wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > Growing the reloc list happens through calling
> > > > > > anv_reloc_list_add()
> > > > > > or
> > > > > > anv_reloc_list_append(). Make sure that we call these through
> > > > > > helpers
> > > > > > that check the result and set the batch error status if
> > > > > > needed.
> > > > > > 
> > > > > > v2:
> > > > > >   - Handling the crashes is not good enough, we need to keep
> > > > > > track
> > > > > > of
> > > > > > the error, for that, keep track of the errors in the
> > > > > > batch
> > > > > > instead (Jason).
> > > > > >   - Make reloc list growth go through helpers so we can have
> > > > > > a
> > > > > > central
> > > > > > place where we can do error tracking (Jason).
> > > > > > ---
> > > > > >  src/intel/vulkan/anv_batch_chain.c | 29
> > > > > > 
> > > > > > -
> > > > > >  src/intel/vulkan/genX_blorp_exec.c |  7 +--
> > > > > >  src/intel/vulkan/genX_cmd_buffer.c | 25 ++
> > > > > > 
> > > > > > ---
> > > > > >  3 files changed, 39 insertions(+), 22 deletions(-)
> > > > > > 
> > > > > > diff --git a/src/intel/vulkan/anv_batch_chain.c
> > > > > > b/src/intel/vulkan/anv_batch_chain.c
> > > > > > index 3774172..2add5bd 100644
> > > > > > --- a/src/intel/vulkan/anv_batch_chain.c
> > > > > > +++ b/src/intel/vulkan/anv_batch_chain.c
> > > > > > @@ -151,8 +151,9 @@ anv_reloc_list_add(struct anv_reloc_list
> > > > > > *list,
> > > > > > const uint32_t domain =
> > > > > >    target_bo->is_winsys_bo ? I915_GEM_DOMAIN_RENDER : 0;
> > > > > >  
> > > > > > -   anv_reloc_list_grow(list, alloc, 1);
> > > > > > -   /* TODO: Handle failure */
> > > > > > +   VkResult result = anv_reloc_list_grow(list, alloc, 1);
> > > > > > +   if (result != VK_SUCCESS)
> > > > > > +  return 0;
> > > > > None of the callers of anv_reloc_list_add() or
> > > > > anv_reloc_list_append() are
> > > > > currently interested of the return value.
> > > > and_reloc_list_append() already returns a VkResult.
> > > > 
> > > > As for the callers of anv_reloc_list_add(),
> > > > anv_batch_emit_reloc()
> > > > needs the offset computed by anv_reloc_list_add(), but that seems
> > > > to
> > > > be
> > > > the only one, so we could do what you suggest below.
> > > > 
> > > > > 
> > > > > 
> > > > >  And if they were, they could
> > > > > easily calculate it themselves: both target_bo and delta are
> > > > > inputs.
> > > > > So instead of relying on zero offset indicating error we could
> > > > > simply
> > > > > change the return type to VkResult. Or did I miss something?
> > > > I think this should be fine.
> > > Well, except that anv_batch_emit_reloc needs to return that offset
> > > to
> > > its callers, so even if receives a VkResult and computes the offset
> > > itself, in the case of an error we still need to return an offset,
> > > so I
> > > don't think there is much gain in doing this, right?
> > Oh, I missed that. Yeah, then it doesn't work. I'm just not super
> > happy
> > relying on offset == 0 indicating failure. Does the diff get out hand
> > if 
> > provided offset as outgoing argument (and returning VkResult)?
> 
> Right I don't particularly like this either, however, I am not sure we
> can do much about it. This is called from two places:
> 
> 1) _blorp_combine_address() -> blorp_emit_reloc()
> -> anv_batch_emit_reloc()
> 
> 2) _anv_combine_address() -> anv_batch_emit_reloc()
> 
> _blorp_combine_address and _anv_combine_address are wrapped by the
> _gen_combine_address macros that are used all over the place in auto-
> generated files to fill dwords in various state packets.
> 
> So the thing is, these state packets need to be filled one way or
> another. If we don't they will just have garbage, so in case of
> failure, writing 0 does not sound particularly bad to me, the focus
> needs to be on avoiding to execute things that are known to be broken,
> which is what we are trying to do with this series.
> 
> In other words, we could modify the functions in those chains above to
> return a VkResult, that should not be a big change, however,
> eventually, the generator script would have to do something depending
> on the result: if it is a failure there is not much we can do, we could
> skip writing the state packet, but that would only leave garbage, or we
> could decide to write it to 0, but then all this change would be for
> nothing anyway.
> 
> Also, I count  ~380 call sites of the _gen_combine_address macro in the
> autogenerated files, so for each call site, the 

Re: [Mesa-dev] [Mesa-stable] [PATCH] st/mesa: set result writemask based on ir type

[Emil Velikov]

On 14 March 2017 at 13:16, Ilia Mirkin  wrote:
> I am waiting to have time to do proper testing on the patch like I promised.
> Thus far, the time to do so has failed to materialize.
>
Right - I seems to have misread your reply s/will do some testing
tonight/have done some testing/ :-\

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

[Mesa-dev] [PATCH kmscube 5/7] drm-atomic.c: remove unused variables `gpu_fence_fd`, `kms_fence_fd`

[Eric Engestrom]

Signed-off-by: Eric Engestrom 
---
I feel like I might be missing something here; is copying the fields
into these vars of any use when debugging?
---
 drm-atomic.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drm-atomic.c b/drm-atomic.c
index 5d64e50..27c6b1e 100644
--- a/drm-atomic.c
+++ b/drm-atomic.c
@@ -201,9 +201,6 @@ static int atomic_run(const struct gbm *gbm, const struct 
egl *egl)
struct gbm_bo *next_bo;
EGLSyncKHR gpu_fence = NULL;   /* out-fence from gpu, in-fence 
to kms */
EGLSyncKHR kms_fence = NULL;   /* in-fence to gpu, out-fence 
from kms */
-   int gpu_fence_fd, kms_fence_fd;  /* just for debugging */
-
-   kms_fence_fd = drm.kms_out_fence_fd;
 
if (drm.kms_out_fence_fd != -1) {
kms_fence = create_fence(egl, drm.kms_out_fence_fd);
@@ -233,7 +230,6 @@ static int atomic_run(const struct gbm *gbm, const struct 
egl *egl)
 * to get fd:
 */
drm.kms_in_fence_fd = 
egl->eglDupNativeFenceFDANDROID(egl->display, gpu_fence);
-   gpu_fence_fd = drm.kms_in_fence_fd;
egl->eglDestroySyncKHR(egl->display, gpu_fence);
assert(drm.kms_in_fence_fd != -1);
 
-- 
Cheers,
  Eric

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

[Mesa-dev] [PATCH kmscube 7/7] drm-legacy.c: suppress 'unused parameter warnings

[Eric Engestrom]

Signed-off-by: Eric Engestrom 
---
 drm-legacy.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drm-legacy.c b/drm-legacy.c
index 2392a3d..8e49075 100644
--- a/drm-legacy.c
+++ b/drm-legacy.c
@@ -33,6 +33,9 @@ static struct drm drm;
 static void page_flip_handler(int fd, unsigned int frame,
  unsigned int sec, unsigned int usec, void *data)
 {
+   /* suppress 'unused parameter' warnings */
+   (void)fd, (void)frame, (void)sec, (void)usec;
+
int *waiting_for_flip = data;
*waiting_for_flip = 0;
 }
-- 
Cheers,
  Eric

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

[Mesa-dev] [PATCH kmscube 3/7] drm-atomic.c: fix signed comparison

[Eric Engestrom]

Signed-off-by: Eric Engestrom 
---
 drm-atomic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drm-atomic.c b/drm-atomic.c
index 3362eac..01fa35a 100644
--- a/drm-atomic.c
+++ b/drm-atomic.c
@@ -362,7 +362,7 @@ const struct drm * init_drm_atomic(const char *device)
get_resource(connector, Connector, drm.connector_id);
 
 #define get_properties(type, TYPE, id) do {
\
-   int i;  
\
+   uint32_t i; 
\
drm.type->props = drmModeObjectGetProperties(drm.fd,\
id, DRM_MODE_OBJECT_##TYPE);
\
if (!drm.type->props) { 
\
-- 
Cheers,
  Eric

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

[Mesa-dev] [PATCH kmscube 4/7] drm-atomic.c: remove unused variables `i`

[Eric Engestrom]

Signed-off-by: Eric Engestrom 
---
 drm-atomic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drm-atomic.c b/drm-atomic.c
index 01fa35a..5d64e50 100644
--- a/drm-atomic.c
+++ b/drm-atomic.c
@@ -108,7 +108,6 @@ static int drm_atomic_commit(uint32_t fb_id, uint32_t flags)
drmModeAtomicReq *req;
uint32_t plane_id = drm.plane->plane->plane_id;
uint32_t blob_id;
-   unsigned int i;
int ret;
 
req = drmModeAtomicAlloc();
@@ -320,7 +319,7 @@ static int get_plane_id(void)
 const struct drm * init_drm_atomic(const char *device)
 {
uint32_t plane_id;
-   int i, ret;
+   int ret;
 
ret = init_drm(, device);
if (ret)
-- 
Cheers,
  Eric

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

[Mesa-dev] [PATCH kmscube 6/7] drm-common.c: remove unused variable `gbm`

[Eric Engestrom]

Signed-off-by: Eric Engestrom 
---
 drm-common.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drm-common.c b/drm-common.c
index 764ffa2..0262816 100644
--- a/drm-common.c
+++ b/drm-common.c
@@ -35,7 +35,6 @@ drm_fb_destroy_callback(struct gbm_bo *bo, void *data)
 {
int drm_fd = gbm_device_get_fd(gbm_bo_get_device(bo));
struct drm_fb *fb = data;
-   struct gbm_device *gbm = gbm_bo_get_device(bo);
 
if (fb->fb_id)
drmModeRmFB(drm_fd, fb->fb_id);
-- 
Cheers,
  Eric

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

[Mesa-dev] [PATCH kmscube 1/7] kmscube.c: remove uninitialized var returned

[Eric Engestrom]

`ret` isn't used by anything, so remove it as well.

Signed-off-by: Eric Engestrom 
---
 kmscube.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kmscube.c b/kmscube.c
index 91e0104..8057e66 100644
--- a/kmscube.c
+++ b/kmscube.c
@@ -67,7 +67,7 @@ int main(int argc, char *argv[])
const char *device = "/dev/dri/card0";
enum mode mode = SMOOTH;
int atomic = 0;
-   int opt, ret;
+   int opt;
 
while ((opt = getopt_long_only(argc, argv, shortopts, longopts, NULL)) 
!= -1) {
switch (opt) {
@@ -104,7 +104,7 @@ int main(int argc, char *argv[])
drm = init_drm_legacy(device);
if (!drm) {
printf("failed to initialize %s DRM\n", atomic ? "atomic" : 
"legacy");
-   return ret;
+   return -1;
}
 
gbm = init_gbm(drm->fd, drm->mode->hdisplay, drm->mode->vdisplay);
-- 
Cheers,
  Eric

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

[Mesa-dev] [PATCH kmscube 2/7] cube-tex.c: fix signed comparison

[Eric Engestrom]

Signed-off-by: Eric Engestrom 
---
 cube-tex.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cube-tex.c b/cube-tex.c
index 1e7741d..a543e83 100644
--- a/cube-tex.c
+++ b/cube-tex.c
@@ -227,7 +227,7 @@ static int get_fd_rgba(uint32_t *pstride)
 
map = gbm_bo_map(bo, 0, 0, texw, texh, GBM_BO_TRANSFER_WRITE, , 
_data);
 
-   for (int i = 0; i < texh; i++) {
+   for (uint32_t i = 0; i < texh; i++) {
memcpy([stride * i], [texw * 4 * i], texw * 4);
}
 
@@ -258,7 +258,7 @@ static int get_fd_y(uint32_t *pstride)
 
map = gbm_bo_map(bo, 0, 0, texw/4, texh, GBM_BO_TRANSFER_WRITE, 
, _data);
 
-   for (int i = 0; i < texh; i++) {
+   for (uint32_t i = 0; i < texh; i++) {
memcpy([stride * i], [texw * i], texw);
}
 
@@ -289,7 +289,7 @@ static int get_fd_uv(uint32_t *pstride)
 
map = gbm_bo_map(bo, 0, 0, texw/2/2, texh/2, GBM_BO_TRANSFER_WRITE, 
, _data);
 
-   for (int i = 0; i < texh/2; i++) {
+   for (uint32_t i = 0; i < texh/2; i++) {
memcpy([stride * i], [texw * i], texw);
}
 
-- 
Cheers,
  Eric

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

Re: [Mesa-dev] [Mesa-stable] [PATCH] st/mesa: set result writemask based on ir type

[Ilia Mirkin]

I am waiting to have time to do proper testing on the patch like I
promised. Thus far, the time to do so has failed to materialize.

On Mar 14, 2017 8:27 AM, "Emil Velikov"  wrote:

> Ilia, are you waiting for another person to review the patch or it's
> no longer applicable ?
>
> Thanks
> Emil
>
> On 10 March 2017 at 11:07, Marek Olšák  wrote:
> > Reviewed-by: Marek Olšák 
> >
> > Marek
> >
> > On Sat, Mar 4, 2017 at 7:52 PM, Ilia Mirkin 
> wrote:
> >> This prevents textureQueryLevels, which maps as LODQ, from ending up
> >> with a xyzw writemask, which is illegal.
> >>
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100061
> >> Signed-off-by: Ilia Mirkin 
> >> Cc: mesa-sta...@lists.freedesktop.org
> >> ---
> >>
> >> Untested beyond making sure that the shader in the above bug ends up
> with
> >> a .xy writemask. I don't have access to a reasonable testing rig right
> now,
> >> would be ideal if someone could give it a run through theirs...
> >>
> >>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> >> index af41bdb..63b681f 100644
> >> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> >> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> >> @@ -4165,6 +4165,7 @@ glsl_to_tgsi_visitor::visit(ir_texture *ir)
> >>  */
> >> result_src = get_temp(ir->type);
> >> result_dst = st_dst_reg(result_src);
> >> +   result_dst.writemask = (1 << ir->type->vector_elements) - 1;
> >>
> >> switch (ir->op) {
> >> case ir_tex:
> >> --
> >> 2.10.2
> >>
> >> ___
> >> mesa-stable mailing list
> >> mesa-sta...@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
> > ___
> > mesa-stable mailing list
> > mesa-sta...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-stable
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 00/24] anv: Handle out of memory situations better

[Iago Toral]

BTW, in case it is useful, I have the series available here, including
fixes to review feedback obtained so far:

https://github.com/Igalia/mesa/tree/vk-oom

Iago

On Fri, 2017-03-10 at 13:38 +0100, Iago Toral Quiroga wrote:
> I think this series addresses all the issues raised for v1, except
> making similar changes to anv_cmd_buffer_alloc_dynamic_state and
> anv_cmd_buffer_alloc_blorp_binding_table which Jason suggested we
> should
> probably do as well, and that I'd like to cover separately from this.
> The main change from v1 is that we now track errors in anv_batch,
> which
> allows us to track and report errors that occur during batch
> emissions as
> well.
> 
> As discussed during v1, I have dropped the patch
> "anv: ensure that we don't ever try to adjust relocations more than
> once", since
> Jason pointed out that the issue this was trying to address was under
> discussion
> in Khronos. I have also pointed that discussion to the test
> developers so that
> the tests are updated accordingly when a final decision is made.
> 
> Besides addressing review feedback, this series incorporates a bunch
> of
> additional improvements including better reporting of errors for
> pipelines
> (we can do this now because we track out of memory errors in batches)
> and a few other fixes, mostly crash guards that I had missed before
> and
> a few other minor things.
> 
> I have also changed a bit the order of some patches in the series so
> related fixes are closer to each other.
> 
> Iago Toral Quiroga (24):
>   anv: remove unnecessary function prototype.
>   anv: do not try to ref/unref NULL shaders
>   anv/blorp: return early if we failed to create the shader binary
>   anv/cmd_buffer: report errors in vkBeginCommandBuffer()
>   anv/cmd_buffer: add a status field to anv_batch
>   anv: add anv_batch_set_error() and anv_batch_has_error() helpers
>   anv: handle allocation failure in anv_batch_emit_batch()
>   anv: handle allocation failure in anv_batch_emit_dwords()
>   anv: avoid crashes when failing to allocate batches
>   anv: handle failures when growing reloc lists
>   anv/cmd_buffer: report tracked errors in vkEndCommandBuffer()
>   anv/cmd_buffer: skip vkCmdNextSubpass() for broken command buffers
>   anv/cmd_buffer: skip vkCmdEndRenderPass() for broken command
> buffers
>   anv/cmd_buffer: handle allocation errors during
> vkCmdBeginRenderPass()
>   anv/cmd_buffer: handle out of memory during vkCmdPushConstants
>   anv: handle memory allocation errors during queue submissions
>   anv/cmd_buffer: skip vkCmdDraw*() on broken command buffers
>   anv/cmd_buffer: skip vkCmdDispatch() on broken command buffers
>   anv/cmd_buffer: skip vkCmdExecuteCommands() on broken command
> buffers
>   anv/device: assert that commands submitted to a queue are not bogus
>   anv/blorp: make anv_cmd_buffer_alloc_blorp_binding_table() return a
> VkResult
>   anv: handle errors while allocating new binding table blocks
>   anv: handle errors in emit_binding_table() and emit_samplers()
>   anv: improve error reporting when creating pipelines
> 
>  src/intel/blorp/blorp_genX_exec.h |  25 +---
>  src/intel/vulkan/anv_batch_chain.c|  62 +--
>  src/intel/vulkan/anv_blorp.c  |  38 +++-
>  src/intel/vulkan/anv_cmd_buffer.c |  19 +-
>  src/intel/vulkan/anv_device.c |   1 +
>  src/intel/vulkan/anv_pipeline.c   |   1 +
>  src/intel/vulkan/anv_pipeline_cache.c |   5 +-
>  src/intel/vulkan/anv_private.h|  56 +
>  src/intel/vulkan/genX_blorp_exec.c|  18 +++---
>  src/intel/vulkan/genX_cmd_buffer.c| 113
> +-
>  src/intel/vulkan/genX_pipeline.c  |   9 ++-
>  11 files changed, 253 insertions(+), 94 deletions(-)
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 10/24] anv: handle failures when growing reloc lists

[Iago Toral]

On Tue, 2017-03-14 at 14:04 +0200, Pohjolainen, Topi wrote:
> On Tue, Mar 14, 2017 at 12:06:16PM +0100, Iago Toral wrote:
> > 
> > On Tue, 2017-03-14 at 12:01 +0100, Iago Toral wrote:
> > > 
> > > On Tue, 2017-03-14 at 11:25 +0200, Pohjolainen, Topi wrote:
> > > > 
> > > > 
> > > > On Fri, Mar 10, 2017 at 01:38:23PM +0100, Iago Toral Quiroga
> > > > wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > Growing the reloc list happens through calling
> > > > > anv_reloc_list_add()
> > > > > or
> > > > > anv_reloc_list_append(). Make sure that we call these through
> > > > > helpers
> > > > > that check the result and set the batch error status if
> > > > > needed.
> > > > > 
> > > > > v2:
> > > > >   - Handling the crashes is not good enough, we need to keep
> > > > > track
> > > > > of
> > > > > the error, for that, keep track of the errors in the
> > > > > batch
> > > > > instead (Jason).
> > > > >   - Make reloc list growth go through helpers so we can have
> > > > > a
> > > > > central
> > > > > place where we can do error tracking (Jason).
> > > > > ---
> > > > >  src/intel/vulkan/anv_batch_chain.c | 29
> > > > > 
> > > > > -
> > > > >  src/intel/vulkan/genX_blorp_exec.c |  7 +--
> > > > >  src/intel/vulkan/genX_cmd_buffer.c | 25 ++
> > > > > 
> > > > > ---
> > > > >  3 files changed, 39 insertions(+), 22 deletions(-)
> > > > > 
> > > > > diff --git a/src/intel/vulkan/anv_batch_chain.c
> > > > > b/src/intel/vulkan/anv_batch_chain.c
> > > > > index 3774172..2add5bd 100644
> > > > > --- a/src/intel/vulkan/anv_batch_chain.c
> > > > > +++ b/src/intel/vulkan/anv_batch_chain.c
> > > > > @@ -151,8 +151,9 @@ anv_reloc_list_add(struct anv_reloc_list
> > > > > *list,
> > > > > const uint32_t domain =
> > > > >    target_bo->is_winsys_bo ? I915_GEM_DOMAIN_RENDER : 0;
> > > > >  
> > > > > -   anv_reloc_list_grow(list, alloc, 1);
> > > > > -   /* TODO: Handle failure */
> > > > > +   VkResult result = anv_reloc_list_grow(list, alloc, 1);
> > > > > +   if (result != VK_SUCCESS)
> > > > > +  return 0;
> > > > None of the callers of anv_reloc_list_add() or
> > > > anv_reloc_list_append() are
> > > > currently interested of the return value.
> > > and_reloc_list_append() already returns a VkResult.
> > > 
> > > As for the callers of anv_reloc_list_add(),
> > > anv_batch_emit_reloc()
> > > needs the offset computed by anv_reloc_list_add(), but that seems
> > > to
> > > be
> > > the only one, so we could do what you suggest below.
> > > 
> > > > 
> > > > 
> > > >  And if they were, they could
> > > > easily calculate it themselves: both target_bo and delta are
> > > > inputs.
> > > > So instead of relying on zero offset indicating error we could
> > > > simply
> > > > change the return type to VkResult. Or did I miss something?
> > > I think this should be fine.
> > Well, except that anv_batch_emit_reloc needs to return that offset
> > to
> > its callers, so even if receives a VkResult and computes the offset
> > itself, in the case of an error we still need to return an offset,
> > so I
> > don't think there is much gain in doing this, right?
> Oh, I missed that. Yeah, then it doesn't work. I'm just not super
> happy
> relying on offset == 0 indicating failure. Does the diff get out hand
> if 
> provided offset as outgoing argument (and returning VkResult)?

Right I don't particularly like this either, however, I am not sure we
can do much about it. This is called from two places:

1) _blorp_combine_address() -> blorp_emit_reloc()
-> anv_batch_emit_reloc()

2) _anv_combine_address() -> anv_batch_emit_reloc()

_blorp_combine_address and _anv_combine_address are wrapped by the
_gen_combine_address macros that are used all over the place in auto-
generated files to fill dwords in various state packets.

So the thing is, these state packets need to be filled one way or
another. If we don't they will just have garbage, so in case of
failure, writing 0 does not sound particularly bad to me, the focus
needs to be on avoiding to execute things that are known to be broken,
which is what we are trying to do with this series.

In other words, we could modify the functions in those chains above to
return a VkResult, that should not be a big change, however,
eventually, the generator script would have to do something depending
on the result: if it is a failure there is not much we can do, we could
skip writing the state packet, but that would only leave garbage, or we
could decide to write it to 0, but then all this change would be for
nothing anyway.

Also, I count  ~380 call sites of the _gen_combine_address macro in the
autogenerated files, so for each call site, the generator would
generate code that:

1. declares a variable so it can pass a pointer through that chain of
functions.
2. Checks the result
3. Depending on the result, decides what to write to the state packet.

to me it doesn't look worth the trouble when there is not a clear
gain...

Iago

Re: [Mesa-dev] [PATCH kmscube 1/5] configure.ac: wire -Wall -Wextra directly into the build

[Eric Engestrom]

On Tuesday, 2017-03-14 12:10:48 +, Emil Velikov wrote:
> Both flags are widely available and the rest of MAYBE_WARN are of little
> interest. Since atm no flags were passed, we might as well not bother
> with anything but the former two.
> 
> Signed-off-by: Emil Velikov 
> ---
> This will prompt some warnings, but we can sort this at a later stage.

I already started tackling these, patches coming later today/tonight.

Series is:
Reviewed-by: Eric Engestrom 

> ---
>  Makefile.am  |  1 +
>  configure.ac | 38 --
>  2 files changed, 1 insertion(+), 38 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 80de719..a6abf78 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -37,6 +37,7 @@ kmscube_LDADD = \
>  
>  kmscube_CFLAGS = \
>   -O0 -g \
> + -Wall -Wextra \
>   $(DRM_CFLAGS) \
>   $(GBM_CFLAGS) \
>   $(EGL_CFLAGS) \
> diff --git a/configure.ac b/configure.ac
> index 615488a..785cd76 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -44,43 +44,5 @@ PKG_CHECK_MODULES(GBM, gbm)
>  PKG_CHECK_MODULES(EGL, egl)
>  PKG_CHECK_MODULES(GLES2, glesv2)
>  
> -dnl 
> ===
> -dnl check compiler flags
> -AC_DEFUN([LIBDRM_CC_TRY_FLAG], [
> - AC_MSG_CHECKING([whether $CC supports $1])
> -
> - libdrm_save_CFLAGS="$CFLAGS"
> - CFLAGS="$CFLAGS $1"
> -
> - AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]],[[]])],
> - [libdrm_cc_flag=yes],
> - [libdrm_cc_flag=no])
> - CFLAGS="$libdrm_save_CFLAGS"
> - 
> - if test "x$libdrm_cc_flag" = "xyes"; then
> - ifelse([$2], , :, [$2])
> - else
> - ifelse([$3], , :, [$3])
> - fi
> - AC_MSG_RESULT([$libdrm_cc_flag])
> -])
> -
> -MAYBE_WARN="-Wall -Wextra -Wno-packed-bitfield-compat \
> --Wsign-compare -Werror-implicit-function-declaration \
> --Wpointer-arith -Wwrite-strings -Wstrict-prototypes \
> --Wnested-externs \
> --Wpacked -Wswitch-enum -Wmissing-format-attribute \
> --Wstrict-aliasing=2 -Winit-self -Wunsafe-loop-optimizations \
> --Wdeclaration-after-statement -Wold-style-definition \
> --Wno-missing-field-initializers -Wno-unused-parameter \
> --Wno-attributes -Wno-long-long -Winline"
> -
> -AC_MSG_CHECKING([which warning flags were supported])
> -for W in $MAYBE_WARN; do
> - LIBDRM_CC_TRY_FLAG([$W], [WARN_CFLAGS="$WARN_CFLAGS $W"])
> -done
> -
> -AC_SUBST(WARN_CFLAGS)
> -
>  AC_CONFIG_FILES([Makefile])
>  AC_OUTPUT
> -- 
> 2.11.1
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 03/24] anv/blorp: return early if we failed to create the shader binary

[Iago Toral]

On Tue, 2017-03-14 at 11:53 +0200, Pohjolainen, Topi wrote:
> On Tue, Mar 14, 2017 at 10:35:43AM +0100, Iago Toral wrote:
> > 
> > On Tue, 2017-03-14 at 10:12 +0200, Pohjolainen, Topi wrote:
> > > 
> > > On Fri, Mar 10, 2017 at 01:38:16PM +0100, Iago Toral Quiroga
> > > wrote:
> > > > 
> > > > 
> > > > ---
> > > >  src/intel/vulkan/anv_blorp.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/src/intel/vulkan/anv_blorp.c
> > > > b/src/intel/vulkan/anv_blorp.c
> > > > index d79c5e0..c871f03 100644
> > > > --- a/src/intel/vulkan/anv_blorp.c
> > > > +++ b/src/intel/vulkan/anv_blorp.c
> > > > @@ -72,6 +72,9 @@ upload_blorp_shader(struct blorp_context
> > > > *blorp,
> > > > key, key_size, kernel,
> > > > kernel_size,
> > > > prog_data,
> > > > prog_data_size,
> > > > _map);
> > > >  
> > > > +   if (!bin)
> > > > +  return;
> > > > +
> > > This is called by brw_blorp_get_blit_kernel() which in turn is
> > > called
> > > from
> > > try_blorp_blit(). The latter will happily submit batch to the
> > > kernel
> > > where
> > > there is now no kernel to execute - that will surely lead to gpu
> > > hang.
> > > 
> > > It looks to me we would need to mark down the failure here for
> > > anv so
> > > that
> > > it knows the blit failed. In addition,
> > > brw_blorp_get_blit_kernel()
> > > should
> > > tell to try_blorp_blit() that shader upload failed so that
> > > try_blorp_blit()
> > > won't send the invalid batch to the gpu.
> > The problem here is that as far as I understand, all this happens
> > in
> > code that doesn't know if it is being run inside anv or i965 and
> > that
> > does not have access to an anv batch object.
> > 
> > One solution would be to make all the functions in between these
> > and
> > the entry points (blorp_copy and blorp_blit as far as I can tell)
> > return a bool to indicate failure so that when we call this from
> > anv we
> > can register the error in the corresponding batch.
> At least blorp_context::upload_shader() needs to indicate failure so
> that
> common code doesn't try to submit invalid batches.

Agreed, at least that seems like a reasonable first step.

Iago

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

Re: [Mesa-dev] [PATCH] anv/blorp: Only set a clear color for resolves if fast-cleared

[Emil Velikov]

On 14 March 2017 at 01:45, Jason Ekstrand  wrote:
> On Mon, Mar 13, 2017 at 5:34 PM, Emil Velikov 
> wrote:
>>
>> On 13 March 2017 at 19:08, Jason Ekstrand  wrote:
>> > On Mon, Mar 13, 2017 at 11:19 AM, Emil Velikov
>> > 
>> > wrote:
>> >>
>> >> On 5 March 2017 at 16:36, Eduardo Lima Mitev  wrote:
>> >> > Reviewed-by: Eduardo Lima Mitev 
>> >> >
>> >> > On 03/04/2017 01:56 AM, Jason Ekstrand wrote:
>> >> >> Cc: "17.0" 
>> >> >> ---
>> >> >>  src/intel/vulkan/anv_blorp.c | 3 ++-
>> >> >>  1 file changed, 2 insertions(+), 1 deletion(-)
>> >> >>
>> >> Is there anything wrong with the patch or it simply fell through the
>> >> cracks ?
>> >
>> >
>> > There was a small issue but both this patch and the fix landed this
>> > morning.
>> >
>> Right so the patch is superseded [by 6b644e571e2 ("anv: Stall before
>> fast-clear operations")] - I wasn't too sure.
>
>
> No, it isn't.  They're completely separate patches.
>
Barring that one, I cannot spot anything alike the the above patch
that landed yesterday morning.
That said - I trust you tagged the patch and if so you'll see it in -stable.

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

Re: [Mesa-dev] [Mesa-stable] [PATCH] st/mesa: set result writemask based on ir type

[Emil Velikov]

Ilia, are you waiting for another person to review the patch or it's
no longer applicable ?

Thanks
Emil

On 10 March 2017 at 11:07, Marek Olšák  wrote:
> Reviewed-by: Marek Olšák 
>
> Marek
>
> On Sat, Mar 4, 2017 at 7:52 PM, Ilia Mirkin  wrote:
>> This prevents textureQueryLevels, which maps as LODQ, from ending up
>> with a xyzw writemask, which is illegal.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100061
>> Signed-off-by: Ilia Mirkin 
>> Cc: mesa-sta...@lists.freedesktop.org
>> ---
>>
>> Untested beyond making sure that the shader in the above bug ends up with
>> a .xy writemask. I don't have access to a reasonable testing rig right now,
>> would be ideal if someone could give it a run through theirs...
>>
>>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
>> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index af41bdb..63b681f 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -4165,6 +4165,7 @@ glsl_to_tgsi_visitor::visit(ir_texture *ir)
>>  */
>> result_src = get_temp(ir->type);
>> result_dst = st_dst_reg(result_src);
>> +   result_dst.writemask = (1 << ir->type->vector_elements) - 1;
>>
>> switch (ir->op) {
>> case ir_tex:
>> --
>> 2.10.2
>>
>> ___
>> mesa-stable mailing list
>> mesa-sta...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 77449] Tracker bug for all bugs related to Steam titles

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=77449
Bug 77449 depends on bug 99194, which changed state.

Bug 99194 Summary: Saints Row IV and Unturned cause GPU resets
https://bugs.freedesktop.org/show_bug.cgi?id=99194

   What|Removed |Added

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

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

Re: [Mesa-dev] [Mesa-stable] [PATCH 1/8] nir/intrinsics: Make load_barycentric_input take a 2-component coor

[Emil Velikov]

On 4 March 2017 at 01:12, Jason Ekstrand  wrote:
> Cc: "17.0 13.0" 
> ---
>  src/compiler/nir/nir_intrinsics.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/compiler/nir/nir_intrinsics.h 
> b/src/compiler/nir/nir_intrinsics.h
> index f45bfe2..5c8f283 100644
> --- a/src/compiler/nir/nir_intrinsics.h
> +++ b/src/compiler/nir/nir_intrinsics.h
> @@ -401,7 +401,9 @@ LOAD(input, 1, 2, BASE, COMPONENT, xx, 
> NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINS
>  /* src[] = { vertex, offset }. const_index[] = { base, component } */
>  LOAD(per_vertex_input, 2, 2, BASE, COMPONENT, xx, 
> NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
>  /* src[] = { barycoord, offset }. const_index[] = { base, component } */
> -LOAD(interpolated_input, 2, 2, BASE, COMPONENT, xx, 
> NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
> +INTRINSIC(load_interpolated_input, 2, ARR(2, 1), true, 0, 0,
> +  2, BASE, COMPONENT, xx,
> +  NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
>
Looking through master - this did not seem to have lander, in it's current form.
is it superseded, fallen through the cracks or simply short on review ?

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

[Mesa-dev] [PATCH kmscube 3/5] configure.ac: remove unused AC_PROG_LIBTOOL

[Emil Velikov]

We're creating a single binary with no static/shared libraries.

We implicitly relied on the macro to pull AC_PROG_CC, so we should add
it now, otherwise configure will fail.

As a nice bonus, with the last three commits combined the execution time
of autogen.sh drops by more than half.

before  after
real0m7.965s0m3.661s
user0m4.530s0m1.280s
sys 0m0.280s0m0.070s

Signed-off-by: Emil Velikov 
---
 Makefile.am  | 3 ---
 configure.ac | 5 ++---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index a6abf78..7caf0fe 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -25,9 +25,6 @@ ACLOCAL_AMFLAGS = -I m4 ${ACLOCAL_FLAGS}
 
 bin_PROGRAMS = kmscube
 
-kmscube_LDFLAGS = \
-   -no-undefined
-
 kmscube_LDADD = \
$(DRM_LIBS) \
$(GBM_LIBS) \
diff --git a/configure.ac b/configure.ac
index 9b92d0e..2fe99d1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -30,12 +30,11 @@ AC_CONFIG_MACRO_DIR([m4])
 # Initialize Automake
 AM_INIT_AUTOMAKE([foreign dist-bzip2])
 
+AC_PROG_CC
+
 # Enable quiet compiles on automake 1.11.
 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
 
-# Initialize libtool
-AC_PROG_LIBTOOL
-
 # Obtain compiler/linker options for depedencies
 PKG_CHECK_MODULES(DRM, libdrm)
 PKG_CHECK_MODULES(GBM, gbm)
-- 
2.11.1

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

[Mesa-dev] [PATCH kmscube 4/5] configure.ac: remove no-longer applicable m4 folder

[Emil Velikov]

With the removal of libtool as of last commit we no longer need the m4
folder or any of the related infra.

Signed-off-by: Emil Velikov 
---
 Makefile.am   | 2 --
 configure.ac  | 1 -
 m4/.gitignore | 5 -
 3 files changed, 8 deletions(-)
 delete mode 100644 m4/.gitignore

diff --git a/Makefile.am b/Makefile.am
index 7caf0fe..b0467f8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -21,8 +21,6 @@
 #  SOFTWARE.
 #
 
-ACLOCAL_AMFLAGS = -I m4 ${ACLOCAL_FLAGS}
-
 bin_PROGRAMS = kmscube
 
 kmscube_LDADD = \
diff --git a/configure.ac b/configure.ac
index 2fe99d1..4df788c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -25,7 +25,6 @@
 AC_PREREQ([2.60])
 AC_INIT([kmscube], [0.0.1], [https://cgit.freedesktop.org/mesa/kmscube/], 
[kmscube])
 AC_CONFIG_AUX_DIR([build-aux])
-AC_CONFIG_MACRO_DIR([m4])
 
 # Initialize Automake
 AM_INIT_AUTOMAKE([foreign dist-bzip2])
diff --git a/m4/.gitignore b/m4/.gitignore
deleted file mode 100644
index 464ba5c..000
--- a/m4/.gitignore
+++ /dev/null
@@ -1,5 +0,0 @@
-libtool.m4
-lt~obsolete.m4
-ltoptions.m4
-ltsugar.m4
-ltversion.m4
-- 
2.11.1

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

[Mesa-dev] [PATCH kmscube 5/5] .gitignore: cleanups no-longer applicable files

[Emil Velikov]

Signed-off-by: Emil Velikov 
---
 .gitignore | 17 -
 1 file changed, 17 deletions(-)

diff --git a/.gitignore b/.gitignore
index 3f95cde..1fc8950 100644
--- a/.gitignore
+++ b/.gitignore
@@ -5,26 +5,9 @@ Makefile
 .deps
 .libs
 *.o
-*.lo
-*.la
-libtool
-*.pc
 config.log
 config.status
-config.guess
-config.h
-config.h.in
-config.sub
-config
 configure
-install-sh
-ltmain.sh
-missing
-stamp-h1
-depcomp
-.cproject
-.project
-.settings
 kmscube
 tags
 TAGS
-- 
2.11.1

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

[Mesa-dev] [PATCH kmscube 1/5] configure.ac: wire -Wall -Wextra directly into the build

[Emil Velikov]

Both flags are widely available and the rest of MAYBE_WARN are of little
interest. Since atm no flags were passed, we might as well not bother
with anything but the former two.

Signed-off-by: Emil Velikov 
---
This will prompt some warnings, but we can sort this at a later stage.
---
 Makefile.am  |  1 +
 configure.ac | 38 --
 2 files changed, 1 insertion(+), 38 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 80de719..a6abf78 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -37,6 +37,7 @@ kmscube_LDADD = \
 
 kmscube_CFLAGS = \
-O0 -g \
+   -Wall -Wextra \
$(DRM_CFLAGS) \
$(GBM_CFLAGS) \
$(EGL_CFLAGS) \
diff --git a/configure.ac b/configure.ac
index 615488a..785cd76 100644
--- a/configure.ac
+++ b/configure.ac
@@ -44,43 +44,5 @@ PKG_CHECK_MODULES(GBM, gbm)
 PKG_CHECK_MODULES(EGL, egl)
 PKG_CHECK_MODULES(GLES2, glesv2)
 
-dnl ===
-dnl check compiler flags
-AC_DEFUN([LIBDRM_CC_TRY_FLAG], [
-   AC_MSG_CHECKING([whether $CC supports $1])
-
-   libdrm_save_CFLAGS="$CFLAGS"
-   CFLAGS="$CFLAGS $1"
-
-   AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]],[[]])],
-   [libdrm_cc_flag=yes],
-   [libdrm_cc_flag=no])
-   CFLAGS="$libdrm_save_CFLAGS"
-   
-   if test "x$libdrm_cc_flag" = "xyes"; then
-   ifelse([$2], , :, [$2])
-   else
-   ifelse([$3], , :, [$3])
-   fi
-   AC_MSG_RESULT([$libdrm_cc_flag])
-])
-
-MAYBE_WARN="-Wall -Wextra -Wno-packed-bitfield-compat \
--Wsign-compare -Werror-implicit-function-declaration \
--Wpointer-arith -Wwrite-strings -Wstrict-prototypes \
--Wnested-externs \
--Wpacked -Wswitch-enum -Wmissing-format-attribute \
--Wstrict-aliasing=2 -Winit-self -Wunsafe-loop-optimizations \
--Wdeclaration-after-statement -Wold-style-definition \
--Wno-missing-field-initializers -Wno-unused-parameter \
--Wno-attributes -Wno-long-long -Winline"
-
-AC_MSG_CHECKING([which warning flags were supported])
-for W in $MAYBE_WARN; do
-   LIBDRM_CC_TRY_FLAG([$W], [WARN_CFLAGS="$WARN_CFLAGS $W"])
-done
-
-AC_SUBST(WARN_CFLAGS)
-
 AC_CONFIG_FILES([Makefile])
 AC_OUTPUT
-- 
2.11.1

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

[Mesa-dev] [PATCH kmscube 2/5] configure.ac: remove useless AC_CONFIG_* macros

[Emil Velikov]

Namely:
AC_CONFIG_SRCDIR - it's used to "help" people who deliberatelly override
--srcdir. Nobody should be doing this to begin with, so just let them
get what they're asking for.

AC_CONFIG_HEADERS - we don't have any conditionals, function or macro
checks. Hence we don't need the header.

Signed-off-by: Emil Velikov 
---
 configure.ac | 2 --
 1 file changed, 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 785cd76..9b92d0e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -24,8 +24,6 @@
 # Initialize Autoconf
 AC_PREREQ([2.60])
 AC_INIT([kmscube], [0.0.1], [https://cgit.freedesktop.org/mesa/kmscube/], 
[kmscube])
-AC_CONFIG_SRCDIR([Makefile.am])
-AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_MACRO_DIR([m4])
 
-- 
2.11.1

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

Re: [Mesa-dev] [PATCH kmscube] drm-common.h: forward-declare needed structs

[Emil Velikov]

On 13 March 2017 at 19:24, Emil Velikov  wrote:
> On 13 March 2017 at 17:14, Eric Engestrom  wrote:
>> drm-common.h:63:49: warning: ‘struct egl’ declared inside parameter list 
>> will not be
>> visible outside of this definition or declaration
>>   int (*run)(const struct gbm *gbm, const struct egl *egl);
>>  ^~~
>> drm-common.h:63:26: warning: ‘struct gbm’ declared inside parameter list 
>> will not be
>> visible outside of this definition or declaration
>>   int (*run)(const struct gbm *gbm, const struct egl *egl);
>>   ^~~
>>
> Was going to say "how ?" only to notice that WARN_CFLAGS is unused.
> Might as well cut it down to -Wall -Wextra and make sure it's used ?
>
Actually I went ahead and did these last night. Patches coming in a minute.

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

Re: [Mesa-dev] [PATCH v2 10/24] anv: handle failures when growing reloc lists

[Pohjolainen, Topi]

On Tue, Mar 14, 2017 at 12:06:16PM +0100, Iago Toral wrote:
> On Tue, 2017-03-14 at 12:01 +0100, Iago Toral wrote:
> > On Tue, 2017-03-14 at 11:25 +0200, Pohjolainen, Topi wrote:
> > > 
> > > On Fri, Mar 10, 2017 at 01:38:23PM +0100, Iago Toral Quiroga wrote:
> > > > 
> > > > 
> > > > Growing the reloc list happens through calling
> > > > anv_reloc_list_add()
> > > > or
> > > > anv_reloc_list_append(). Make sure that we call these through
> > > > helpers
> > > > that check the result and set the batch error status if needed.
> > > > 
> > > > v2:
> > > >   - Handling the crashes is not good enough, we need to keep
> > > > track
> > > > of
> > > > the error, for that, keep track of the errors in the batch
> > > > instead (Jason).
> > > >   - Make reloc list growth go through helpers so we can have a
> > > > central
> > > > place where we can do error tracking (Jason).
> > > > ---
> > > >  src/intel/vulkan/anv_batch_chain.c | 29 
> > > > -
> > > >  src/intel/vulkan/genX_blorp_exec.c |  7 +--
> > > >  src/intel/vulkan/genX_cmd_buffer.c | 25 ++
> > > > ---
> > > >  3 files changed, 39 insertions(+), 22 deletions(-)
> > > > 
> > > > diff --git a/src/intel/vulkan/anv_batch_chain.c
> > > > b/src/intel/vulkan/anv_batch_chain.c
> > > > index 3774172..2add5bd 100644
> > > > --- a/src/intel/vulkan/anv_batch_chain.c
> > > > +++ b/src/intel/vulkan/anv_batch_chain.c
> > > > @@ -151,8 +151,9 @@ anv_reloc_list_add(struct anv_reloc_list
> > > > *list,
> > > > const uint32_t domain =
> > > >    target_bo->is_winsys_bo ? I915_GEM_DOMAIN_RENDER : 0;
> > > >  
> > > > -   anv_reloc_list_grow(list, alloc, 1);
> > > > -   /* TODO: Handle failure */
> > > > +   VkResult result = anv_reloc_list_grow(list, alloc, 1);
> > > > +   if (result != VK_SUCCESS)
> > > > +  return 0;
> > > None of the callers of anv_reloc_list_add() or
> > > anv_reloc_list_append() are
> > > currently interested of the return value.
> > and_reloc_list_append() already returns a VkResult.
> > 
> > As for the callers of anv_reloc_list_add(), anv_batch_emit_reloc()
> > needs the offset computed by anv_reloc_list_add(), but that seems to
> > be
> > the only one, so we could do what you suggest below.
> > 
> > > 
> > >  And if they were, they could
> > > easily calculate it themselves: both target_bo and delta are
> > > inputs.
> > > So instead of relying on zero offset indicating error we could
> > > simply
> > > change the return type to VkResult. Or did I miss something?
> > I think this should be fine.
> 
> Well, except that anv_batch_emit_reloc needs to return that offset to
> its callers, so even if receives a VkResult and computes the offset
> itself, in the case of an error we still need to return an offset, so I
> don't think there is much gain in doing this, right?

Oh, I missed that. Yeah, then it doesn't work. I'm just not super happy
relying on offset == 0 indicating failure. Does the diff get out hand if 
provided offset as outgoing argument (and returning VkResult)?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/5] util/disk_cache: actually enforce cache size

[Grazvydas Ignotas]

On Tue, Mar 14, 2017 at 4:08 AM, Timothy Arceri  wrote:
> From: Alan Swanson 
>
> Currently only a one in one out eviction so if at max_size and
> cache files were to constantly increase in size then so would the
> cache. Restrict to limit of 8 evictions per new cache entry.
>
> V2: (Timothy Arceri) fix make check tests
> ---
>  src/compiler/glsl/tests/cache_test.c | 22 +++---
>  src/util/disk_cache.c|  6 +-
>  2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/src/compiler/glsl/tests/cache_test.c 
> b/src/compiler/glsl/tests/cache_test.c
> index 10505d4..5ad438c 100644
> --- a/src/compiler/glsl/tests/cache_test.c
> +++ b/src/compiler/glsl/tests/cache_test.c
> @@ -345,30 +345,38 @@ test_put_and_get(void)
>  * finish.
>  */
> wait_until_file_written(cache, one_KB_key);
>
> result = disk_cache_get(cache, one_KB_key, );
> expect_non_null(result, "3rd disk_cache_get of existing item (pointer)");
> expect_equal(size, 1024, "3rd disk_cache_get of existing item (size)");
>
> free(result);
>
> -   /* Ensure eviction happened by checking that only one of the two
> -* previously-added items can still be fetched.
> +   /* Ensure eviction happened by checking that both of the previous
> +* cache itesm were evicted.

items

for patches 2-5:
Reviewed-by: Grazvydas Ignotas 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 21/24] anv/blorp: make anv_cmd_buffer_alloc_blorp_binding_table() return a VkResult

[Pohjolainen, Topi]

On Tue, Mar 14, 2017 at 12:18:59PM +0100, Iago Toral wrote:
> On Tue, 2017-03-14 at 11:39 +0200, Pohjolainen, Topi wrote:
> > On Fri, Mar 10, 2017 at 01:38:34PM +0100, Iago Toral Quiroga wrote:
> > > 
> > > Instead of asserting inside the function, and then use use that
> > > information
> > > to return early from its callers upon failure.
> > > ---
> > >  src/intel/vulkan/anv_blorp.c   | 35 
> > > ---
> > >  src/intel/vulkan/anv_private.h |  5 +++--
> > >  src/intel/vulkan/genX_blorp_exec.c |  8 ++--
> > >  3 files changed, 29 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/src/intel/vulkan/anv_blorp.c
> > > b/src/intel/vulkan/anv_blorp.c
> > > index c871f03..de5b0ec 100644
> > > --- a/src/intel/vulkan/anv_blorp.c
> > > +++ b/src/intel/vulkan/anv_blorp.c
> > > @@ -904,31 +904,31 @@ void anv_CmdClearDepthStencilImage(
> > > blorp_batch_finish();
> > >  }
> > >  
> > > -struct anv_state
> > > +VkResult
> > >  anv_cmd_buffer_alloc_blorp_binding_table(struct anv_cmd_buffer
> > > *cmd_buffer,
> > >   uint32_t num_entries,
> > > - uint32_t *state_offset)
> > > + uint32_t *state_offset,
> > > + struct anv_state
> > > *bt_state)
> > >  {
> > > -   struct anv_state bt_state =
> > > -  anv_cmd_buffer_alloc_binding_table(cmd_buffer, num_entries,
> > > - state_offset);
> > > -   if (bt_state.map == NULL) {
> > > +   *bt_state = anv_cmd_buffer_alloc_binding_table(cmd_buffer,
> > > num_entries,
> > > +  state_offset);
> > > +   if (bt_state->map == NULL) {
> > >    /* We ran out of space.  Grab a new binding table block. */
> > > -  MAYBE_UNUSED VkResult result =
> > > - anv_cmd_buffer_new_binding_table_block(cmd_buffer);
> > > -  assert(result == VK_SUCCESS);
> > > +  VkResult result =
> > > anv_cmd_buffer_new_binding_table_block(cmd_buffer);
> > > +  if (result != VK_SUCCESS)
> > > + return result;
> > >  
> > >    /* Re-emit state base addresses so we get the new surface
> > > state base
> > > * address before we start emitting binding tables etc.
> > > */
> > >    anv_cmd_buffer_emit_state_base_address(cmd_buffer);
> > >  
> > > -  bt_state = anv_cmd_buffer_alloc_binding_table(cmd_buffer,
> > > num_entries,
> > > -state_offset);
> > > -  assert(bt_state.map != NULL);
> > > +  *bt_state = anv_cmd_buffer_alloc_binding_table(cmd_buffer,
> > > num_entries,
> > > + state_offset)
> > > ;
> > > +  assert(bt_state->map != NULL);
> > > }
> > >  
> > > -   return bt_state;
> > > +   return VK_SUCCESS;
> > >  }
> > >  
> > >  static uint32_t
> > > @@ -936,8 +936,13 @@ binding_table_for_surface_state(struct
> > > anv_cmd_buffer *cmd_buffer,
> > >  struct anv_state surface_state)
> > >  {
> > > uint32_t state_offset;
> > > -   struct anv_state bt_state =
> > > -  anv_cmd_buffer_alloc_blorp_binding_table(cmd_buffer, 1,
> > > _offset);
> > > +   struct anv_state bt_state;
> > > +
> > > +   VkResult result =
> > > +  anv_cmd_buffer_alloc_blorp_binding_table(cmd_buffer, 1,
> > > _offset,
> > > +   _state);
> > > +   if (result != VK_SUCCESS)
> > > +  return 0;
> > This leads to gpu hang as well I think: clear_color_attachment()
> > calls
> > blorp_clear_attachments() which sends a batch to gpu regardless of
> > binding
> > table offset being zero.
> 
> Also, thinking a bit more about this, I am not sure that 0 can't be a
> valid binding table offset...
> 
> How about we make this function take a pointer to the biding table
> offset instead and we make it return a VkResult? Then we make callers
> bail if the result is not VK_SUCCESS.

Sounds good, thanks :)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 21/24] anv/blorp: make anv_cmd_buffer_alloc_blorp_binding_table() return a VkResult

[Iago Toral]

On Tue, 2017-03-14 at 11:39 +0200, Pohjolainen, Topi wrote:
> On Fri, Mar 10, 2017 at 01:38:34PM +0100, Iago Toral Quiroga wrote:
> > 
> > Instead of asserting inside the function, and then use use that
> > information
> > to return early from its callers upon failure.
> > ---
> >  src/intel/vulkan/anv_blorp.c   | 35 
> > ---
> >  src/intel/vulkan/anv_private.h |  5 +++--
> >  src/intel/vulkan/genX_blorp_exec.c |  8 ++--
> >  3 files changed, 29 insertions(+), 19 deletions(-)
> > 
> > diff --git a/src/intel/vulkan/anv_blorp.c
> > b/src/intel/vulkan/anv_blorp.c
> > index c871f03..de5b0ec 100644
> > --- a/src/intel/vulkan/anv_blorp.c
> > +++ b/src/intel/vulkan/anv_blorp.c
> > @@ -904,31 +904,31 @@ void anv_CmdClearDepthStencilImage(
> > blorp_batch_finish();
> >  }
> >  
> > -struct anv_state
> > +VkResult
> >  anv_cmd_buffer_alloc_blorp_binding_table(struct anv_cmd_buffer
> > *cmd_buffer,
> >   uint32_t num_entries,
> > - uint32_t *state_offset)
> > + uint32_t *state_offset,
> > + struct anv_state
> > *bt_state)
> >  {
> > -   struct anv_state bt_state =
> > -  anv_cmd_buffer_alloc_binding_table(cmd_buffer, num_entries,
> > - state_offset);
> > -   if (bt_state.map == NULL) {
> > +   *bt_state = anv_cmd_buffer_alloc_binding_table(cmd_buffer,
> > num_entries,
> > +  state_offset);
> > +   if (bt_state->map == NULL) {
> >    /* We ran out of space.  Grab a new binding table block. */
> > -  MAYBE_UNUSED VkResult result =
> > - anv_cmd_buffer_new_binding_table_block(cmd_buffer);
> > -  assert(result == VK_SUCCESS);
> > +  VkResult result =
> > anv_cmd_buffer_new_binding_table_block(cmd_buffer);
> > +  if (result != VK_SUCCESS)
> > + return result;
> >  
> >    /* Re-emit state base addresses so we get the new surface
> > state base
> > * address before we start emitting binding tables etc.
> > */
> >    anv_cmd_buffer_emit_state_base_address(cmd_buffer);
> >  
> > -  bt_state = anv_cmd_buffer_alloc_binding_table(cmd_buffer,
> > num_entries,
> > -state_offset);
> > -  assert(bt_state.map != NULL);
> > +  *bt_state = anv_cmd_buffer_alloc_binding_table(cmd_buffer,
> > num_entries,
> > + state_offset)
> > ;
> > +  assert(bt_state->map != NULL);
> > }
> >  
> > -   return bt_state;
> > +   return VK_SUCCESS;
> >  }
> >  
> >  static uint32_t
> > @@ -936,8 +936,13 @@ binding_table_for_surface_state(struct
> > anv_cmd_buffer *cmd_buffer,
> >  struct anv_state surface_state)
> >  {
> > uint32_t state_offset;
> > -   struct anv_state bt_state =
> > -  anv_cmd_buffer_alloc_blorp_binding_table(cmd_buffer, 1,
> > _offset);
> > +   struct anv_state bt_state;
> > +
> > +   VkResult result =
> > +  anv_cmd_buffer_alloc_blorp_binding_table(cmd_buffer, 1,
> > _offset,
> > +   _state);
> > +   if (result != VK_SUCCESS)
> > +  return 0;
> This leads to gpu hang as well I think: clear_color_attachment()
> calls
> blorp_clear_attachments() which sends a batch to gpu regardless of
> binding
> table offset being zero.

Also, thinking a bit more about this, I am not sure that 0 can't be a
valid binding table offset...

How about we make this function take a pointer to the biding table
offset instead and we make it return a VkResult? Then we make callers
bail if the result is not VK_SUCCESS.

> > 
> >  
> > uint32_t *bt_map = bt_state.map;
> > bt_map[0] = surface_state.offset + state_offset;
> > diff --git a/src/intel/vulkan/anv_private.h
> > b/src/intel/vulkan/anv_private.h
> > index 7490d0c..adb8ce2 100644
> > --- a/src/intel/vulkan/anv_private.h
> > +++ b/src/intel/vulkan/anv_private.h
> > @@ -1463,10 +1463,11 @@ void anv_cmd_buffer_resolve_subpass(struct
> > anv_cmd_buffer *cmd_buffer);
> >  const struct anv_image_view *
> >  anv_cmd_buffer_get_depth_stencil_view(const struct anv_cmd_buffer
> > *cmd_buffer);
> >  
> > -struct anv_state
> > +VkResult
> >  anv_cmd_buffer_alloc_blorp_binding_table(struct anv_cmd_buffer
> > *cmd_buffer,
> >   uint32_t num_entries,
> > - uint32_t *state_offset);
> > + uint32_t *state_offset,
> > + struct anv_state
> > *bt_state);
> >  
> >  void anv_cmd_buffer_dump(struct anv_cmd_buffer *cmd_buffer);
> >  
> > diff --git a/src/intel/vulkan/genX_blorp_exec.c
> > b/src/intel/vulkan/genX_blorp_exec.c
> > index 90adefe..0649d3b 100644
> > --- 

Re: [Mesa-dev] [PATCH V3] util/disk_cache: use a thread queue to write to shader cache

[Timothy Arceri]

On 14/03/17 21:27, Grazvydas Ignotas wrote:

On Tue, Mar 14, 2017 at 4:06 AM, Timothy Arceri  wrote:

This should help reduce any overhead added by the shader cache
when programs are not found in the cache.

To avoid creating any special function just for the sake of the
tests we add a one second delay whenever we call dick_cache_put()
to give it time to finish.

V2: poll for file when waiting for thread in test
V3: fix poll delay to really be 100ms, and simplify the wait function
---
  src/compiler/glsl/tests/cache_test.c | 63 
  src/util/disk_cache.c| 41 +++
  2 files changed, 85 insertions(+), 19 deletions(-)

diff --git a/src/compiler/glsl/tests/cache_test.c 
b/src/compiler/glsl/tests/cache_test.c
index 6451e65..10505d4 100644
--- a/src/compiler/glsl/tests/cache_test.c
+++ b/src/compiler/glsl/tests/cache_test.c
@@ -25,20 +25,21 @@

  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
+#include 
  #include 

  #include "util/mesa-sha1.h"
  #include "util/disk_cache.h"

  bool error = false;

  #ifdef ENABLE_SHADER_CACHE

  static void
@@ -224,20 +225,40 @@ does_cache_contain(struct disk_cache *cache, cache_key 
key)

 if (result) {
free(result);
return true;
 }

 return false;
  }

  static void
+wait_until_file_written(struct disk_cache *cache, cache_key key)
+{
+   struct timespec req;
+   struct timespec rem;
+
+   /* Set 100ms delay */
+   req.tv_sec = 0;
+   req.tv_nsec = 1;
+
+   unsigned retries = 0;
+   while (retries++ < 20) {
+  if (does_cache_contain(cache, key)) {
+ break;
+  }
+
+  nanosleep(, );
+   }
+}
+
+static void
  test_put_and_get(void)
  {
 struct disk_cache *cache;
 /* If the text of this blob is changed, then blob_key_byte_zero
  * also needs to be updated.
  */
 char blob[] = "This is a blob of thirty-seven bytes";
 uint8_t blob_key[20];
 uint8_t blob_key_byte_zero = 0xca;
 char string[] = "While this string has thirty-four";
@@ -253,35 +274,49 @@ test_put_and_get(void)
 _mesa_sha1_compute(blob, sizeof(blob), blob_key);

 /* Ensure that disk_cache_get returns nothing before anything is added. */
 result = disk_cache_get(cache, blob_key, );
 expect_null(result, "disk_cache_get with non-existent item (pointer)");
 expect_equal(size, 0, "disk_cache_get with non-existent item (size)");

 /* Simple test of put and get. */
 disk_cache_put(cache, blob_key, blob, sizeof(blob), NULL);

+   /* disk_cache_put() hands things off to a thread give it some time to
+* finsih.

finish


+*/
+   wait_until_file_written(cache, blob_key);
+
 result = disk_cache_get(cache, blob_key, );
-   expect_equal_str(blob, result, "disk_cache_get of existing item (pointer)");
-   expect_equal(size, sizeof(blob), "disk_cache_get of existing item (size)");
+   if (result) {
+  expect_equal_str(blob, result, "disk_cache_get of existing item 
(pointer)");
+  expect_equal(size, sizeof(blob), "disk_cache_get of existing item 
(size)");

-   free(result);
+  free(result);
+   }

 /* Test put and get of a second item. */
 _mesa_sha1_compute(string, sizeof(string), string_key);
 disk_cache_put(cache, string_key, string, sizeof(string), NULL);

+   /* disk_cache_put() hands things off to a thread give it some time to
+* finsih.
+*/

finish (do we need to repeat the comment every time?)


+   wait_until_file_written(cache, string_key);
+
 result = disk_cache_get(cache, string_key, );
-   expect_equal_str(result, string, "2nd disk_cache_get of existing item 
(pointer)");
-   expect_equal(size, sizeof(string), "2nd disk_cache_get of existing item 
(size)");
+   if (result) {
+  expect_equal_str(result, string, "2nd disk_cache_get of existing item 
(pointer)");
+  expect_equal(size, sizeof(string), "2nd disk_cache_get of existing item 
(size)");

-   free(result);
+  free(result);
+   }

Here (and before) you check disk_cache_get() result for NULL ,,,


 /* Set the cache size to 1KB and add a 1KB item to force an eviction. */
 disk_cache_destroy(cache);

 setenv("MESA_GLSL_CACHE_MAX_SIZE", "1K", 1);
 cache = disk_cache_create("test", "make_check");

 one_KB = calloc(1, 1024);

 /* Obviously the SHA-1 hash of 1024 zero bytes isn't particularly
@@ -299,20 +334,25 @@ test_put_and_get(void)
  * interestingly full than that).
  *
  * For this test, we force this signature to land in the same
  * directory as the original blob first written to the cache.
  */
 _mesa_sha1_compute(one_KB, 1024, one_KB_key);
 one_KB_key[0] = blob_key_byte_zero;

 disk_cache_put(cache, one_KB_key, one_KB, 1024, one_KB);

+   /* disk_cache_put() hands things off to a thread give it some time to
+* finish.
+*/
+   wait_until_file_written(cache, one_KB_key);
+
 

Re: [Mesa-dev] [PATCH v2 10/24] anv: handle failures when growing reloc lists

[Iago Toral]

On Tue, 2017-03-14 at 12:01 +0100, Iago Toral wrote:
> On Tue, 2017-03-14 at 11:25 +0200, Pohjolainen, Topi wrote:
> > 
> > On Fri, Mar 10, 2017 at 01:38:23PM +0100, Iago Toral Quiroga wrote:
> > > 
> > > 
> > > Growing the reloc list happens through calling
> > > anv_reloc_list_add()
> > > or
> > > anv_reloc_list_append(). Make sure that we call these through
> > > helpers
> > > that check the result and set the batch error status if needed.
> > > 
> > > v2:
> > >   - Handling the crashes is not good enough, we need to keep
> > > track
> > > of
> > > the error, for that, keep track of the errors in the batch
> > > instead (Jason).
> > >   - Make reloc list growth go through helpers so we can have a
> > > central
> > > place where we can do error tracking (Jason).
> > > ---
> > >  src/intel/vulkan/anv_batch_chain.c | 29 
> > > -
> > >  src/intel/vulkan/genX_blorp_exec.c |  7 +--
> > >  src/intel/vulkan/genX_cmd_buffer.c | 25 ++
> > > ---
> > >  3 files changed, 39 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/src/intel/vulkan/anv_batch_chain.c
> > > b/src/intel/vulkan/anv_batch_chain.c
> > > index 3774172..2add5bd 100644
> > > --- a/src/intel/vulkan/anv_batch_chain.c
> > > +++ b/src/intel/vulkan/anv_batch_chain.c
> > > @@ -151,8 +151,9 @@ anv_reloc_list_add(struct anv_reloc_list
> > > *list,
> > > const uint32_t domain =
> > >    target_bo->is_winsys_bo ? I915_GEM_DOMAIN_RENDER : 0;
> > >  
> > > -   anv_reloc_list_grow(list, alloc, 1);
> > > -   /* TODO: Handle failure */
> > > +   VkResult result = anv_reloc_list_grow(list, alloc, 1);
> > > +   if (result != VK_SUCCESS)
> > > +  return 0;
> > None of the callers of anv_reloc_list_add() or
> > anv_reloc_list_append() are
> > currently interested of the return value.
> and_reloc_list_append() already returns a VkResult.
> 
> As for the callers of anv_reloc_list_add(), anv_batch_emit_reloc()
> needs the offset computed by anv_reloc_list_add(), but that seems to
> be
> the only one, so we could do what you suggest below.
> 
> > 
> >  And if they were, they could
> > easily calculate it themselves: both target_bo and delta are
> > inputs.
> > So instead of relying on zero offset indicating error we could
> > simply
> > change the return type to VkResult. Or did I miss something?
> I think this should be fine.

Well, except that anv_batch_emit_reloc needs to return that offset to
its callers, so even if receives a VkResult and computes the offset
itself, in the case of an error we still need to return an offset, so I
don't think there is much gain in doing this, right?

> Iago
> 
> > 
> > > 
> > > 
> > >  
> > > /* XXX: Can we use I915_EXEC_HANDLE_LUT? */
> > > index = list->num_relocs++;
> > > @@ -169,13 +170,14 @@ anv_reloc_list_add(struct anv_reloc_list
> > > *list,
> > > return target_bo->offset + delta;
> > >  }
> > >  
> > > -static void
> > > +static VkResult
> > >  anv_reloc_list_append(struct anv_reloc_list *list,
> > >    const VkAllocationCallbacks *alloc,
> > >    struct anv_reloc_list *other, uint32_t
> > > offset)
> > >  {
> > > -   anv_reloc_list_grow(list, alloc, other->num_relocs);
> > > -   /* TODO: Handle failure */
> > > +   VkResult result = anv_reloc_list_grow(list, alloc, other-
> > > > 
> > > > num_relocs);
> > > +   if (result != VK_SUCCESS)
> > > +  return result;
> > >  
> > > memcpy(>relocs[list->num_relocs], >relocs[0],
> > >    other->num_relocs * sizeof(other->relocs[0]));
> > > @@ -186,6 +188,7 @@ anv_reloc_list_append(struct anv_reloc_list
> > > *list,
> > >    list->relocs[i + list->num_relocs].offset += offset;
> > >  
> > > list->num_relocs += other->num_relocs;
> > > +   return VK_SUCCESS;
> > >  }
> > >  
> > >  /*--
> > > -*
> > > @@ -215,8 +218,12 @@ uint64_t
> > >  anv_batch_emit_reloc(struct anv_batch *batch,
> > >   void *location, struct anv_bo *bo, uint32_t
> > > delta)
> > >  {
> > > -   return anv_reloc_list_add(batch->relocs, batch->alloc,
> > > - location - batch->start, bo,
> > > delta);
> > > +   uint64_t offset = anv_reloc_list_add(batch->relocs, batch-
> > > > 
> > > > alloc,
> > > +location - batch->start,
> > > bo, delta);
> > > +   if (offset == 0)
> > > +  anv_batch_set_error(batch, VK_ERROR_OUT_OF_HOST_MEMORY);
> > > +
> > > +   return offset;
> > >  }
> > >  
> > >  void
> > > @@ -239,8 +246,12 @@ anv_batch_emit_batch(struct anv_batch
> > > *batch,
> > > struct anv_batch *other)
> > > memcpy(batch->next, other->start, size);
> > >  
> > > offset = batch->next - batch->start;
> > > -   anv_reloc_list_append(batch->relocs, batch->alloc,
> > > - other->relocs, offset);
> > > +   VkResult result = anv_reloc_list_append(batch->relocs, batch-
> > > > 
> > > > 

[Mesa-dev] [PATCH] vulkan: enums: fix generation with enum aliases

[Lionel Landwerlin]

It seems new extensions will start to use aliases in enums, meaning
multiple enums with the same value. This forces us to track the values
of the enums to generate correct C code.

Signed-off-by: Lionel Landwerlin 
Cc: Dylan Baker 
---
 src/vulkan/util/gen_enum_to_str.py | 49 ++
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/src/vulkan/util/gen_enum_to_str.py 
b/src/vulkan/util/gen_enum_to_str.py
index fb31addf94..1ffe4e362b 100644
--- a/src/vulkan/util/gen_enum_to_str.py
+++ b/src/vulkan/util/gen_enum_to_str.py
@@ -24,6 +24,7 @@
 from __future__ import print_function
 import argparse
 import os
+import re
 import textwrap
 import xml.etree.cElementTree as et
 
@@ -67,9 +68,9 @@ C_TEMPLATE = Template(textwrap.dedent(u"""\
 vk_${enum.name[2:]}_to_str(${enum.name} input)
 {
 switch(input) {
-% for v in enum.values:
-case ${v}:
-return "${v}";
+% for k, v in enum.values.iteritems():
+case ${v[0]}:
+return "${' / '.join(v)}";
 % endfor
 default:
 unreachable("Undefined enum value.");
@@ -116,10 +117,17 @@ class EnumFactory(object):
 class VkEnum(object):
 """Simple struct-like class representing a single Vulkan Enum."""
 
-def __init__(self, name, values=None):
+def __init__(self, name):
 self.name = name
-self.values = values or []
+self.values = {}
+self.names = {}
 
+def add_item(self, name, value):
+if value in self.values:
+self.values[value].append(name)
+else:
+self.values[value] = [name]
+self.names[name] = value
 
 def xml_parser(filename):
 """Parse the XML file and return parsed data.
@@ -129,6 +137,24 @@ def xml_parser(filename):
 """
 efactory = EnumFactory(VkEnum)
 
+number_regexp = re.compile('^-?[0-9]+$')
+
+def compute_enum_value(enum, elem, parent):
+if 'offset' in elem.attrib:
+# Formula taken from the src/spec/generator.py
+# https://github.com/khronosGroup/Vulkan-Docs
+value = 10 + (int(parent.attrib['number']) - 1) * 1000
+value += int(elem.attrib['offset'])
+if 'dir' in e.attrib:
+value = -value
+else:
+number = elem.attrib['value']
+if re.match(number_regexp, number):
+value = int(number)
+else:
+value = enum.names[number]
+return value
+
 with open(filename, 'rb') as f:
 context = iter(et.iterparse(f, events=('start', 'end')))
 
@@ -140,16 +166,19 @@ def xml_parser(filename):
 for event, elem in context:
 if event == 'end' and elem.tag == 'enums':
 type_ = elem.attrib.get('type')
-if type_ == 'enum':
-enum = efactory(elem.attrib['name'])
-enum.values.extend([e.attrib['name'] for e in elem
-if e.tag == 'enum'])
+if type_ != 'enum':
+continue
+enum = efactory(elem.attrib['name'])
+for e in elem:
+if e.tag != 'enum':
+continue
+enum.add_item(e.attrib['name'], compute_enum_value(enum, 
e, None))
 elif event == 'end' and elem.tag == 'extension':
 if elem.attrib['supported'] != 'vulkan':
 continue
 for e in elem.findall('.//enum[@extends][@offset]'):
 enum = efactory(e.attrib['extends'])
-enum.values.append(e.attrib['name'])
+enum.add_item(e.attrib['name'], compute_enum_value(enum, 
e, elem))
 
 root.clear()
 
-- 
2.11.0

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

Re: [Mesa-dev] [PATCH v2 10/24] anv: handle failures when growing reloc lists

[Iago Toral]

On Tue, 2017-03-14 at 11:25 +0200, Pohjolainen, Topi wrote:
> On Fri, Mar 10, 2017 at 01:38:23PM +0100, Iago Toral Quiroga wrote:
> > 
> > Growing the reloc list happens through calling anv_reloc_list_add()
> > or
> > anv_reloc_list_append(). Make sure that we call these through
> > helpers
> > that check the result and set the batch error status if needed.
> > 
> > v2:
> >   - Handling the crashes is not good enough, we need to keep track
> > of
> > the error, for that, keep track of the errors in the batch
> > instead (Jason).
> >   - Make reloc list growth go through helpers so we can have a
> > central
> > place where we can do error tracking (Jason).
> > ---
> >  src/intel/vulkan/anv_batch_chain.c | 29 
> > -
> >  src/intel/vulkan/genX_blorp_exec.c |  7 +--
> >  src/intel/vulkan/genX_cmd_buffer.c | 25 ++---
> >  3 files changed, 39 insertions(+), 22 deletions(-)
> > 
> > diff --git a/src/intel/vulkan/anv_batch_chain.c
> > b/src/intel/vulkan/anv_batch_chain.c
> > index 3774172..2add5bd 100644
> > --- a/src/intel/vulkan/anv_batch_chain.c
> > +++ b/src/intel/vulkan/anv_batch_chain.c
> > @@ -151,8 +151,9 @@ anv_reloc_list_add(struct anv_reloc_list *list,
> > const uint32_t domain =
> >    target_bo->is_winsys_bo ? I915_GEM_DOMAIN_RENDER : 0;
> >  
> > -   anv_reloc_list_grow(list, alloc, 1);
> > -   /* TODO: Handle failure */
> > +   VkResult result = anv_reloc_list_grow(list, alloc, 1);
> > +   if (result != VK_SUCCESS)
> > +  return 0;
> None of the callers of anv_reloc_list_add() or
> anv_reloc_list_append() are
> currently interested of the return value.

and_reloc_list_append() already returns a VkResult.

As for the callers of anv_reloc_list_add(), anv_batch_emit_reloc()
needs the offset computed by anv_reloc_list_add(), but that seems to be
the only one, so we could do what you suggest below.

>  And if they were, they could
> easily calculate it themselves: both target_bo and delta are inputs.
> So instead of relying on zero offset indicating error we could simply
> change the return type to VkResult. Or did I miss something?

I think this should be fine.

Iago

> > 
> >  
> > /* XXX: Can we use I915_EXEC_HANDLE_LUT? */
> > index = list->num_relocs++;
> > @@ -169,13 +170,14 @@ anv_reloc_list_add(struct anv_reloc_list
> > *list,
> > return target_bo->offset + delta;
> >  }
> >  
> > -static void
> > +static VkResult
> >  anv_reloc_list_append(struct anv_reloc_list *list,
> >    const VkAllocationCallbacks *alloc,
> >    struct anv_reloc_list *other, uint32_t
> > offset)
> >  {
> > -   anv_reloc_list_grow(list, alloc, other->num_relocs);
> > -   /* TODO: Handle failure */
> > +   VkResult result = anv_reloc_list_grow(list, alloc, other-
> > >num_relocs);
> > +   if (result != VK_SUCCESS)
> > +  return result;
> >  
> > memcpy(>relocs[list->num_relocs], >relocs[0],
> >    other->num_relocs * sizeof(other->relocs[0]));
> > @@ -186,6 +188,7 @@ anv_reloc_list_append(struct anv_reloc_list
> > *list,
> >    list->relocs[i + list->num_relocs].offset += offset;
> >  
> > list->num_relocs += other->num_relocs;
> > +   return VK_SUCCESS;
> >  }
> >  
> >  /*--
> > -*
> > @@ -215,8 +218,12 @@ uint64_t
> >  anv_batch_emit_reloc(struct anv_batch *batch,
> >   void *location, struct anv_bo *bo, uint32_t
> > delta)
> >  {
> > -   return anv_reloc_list_add(batch->relocs, batch->alloc,
> > - location - batch->start, bo, delta);
> > +   uint64_t offset = anv_reloc_list_add(batch->relocs, batch-
> > >alloc,
> > +location - batch->start,
> > bo, delta);
> > +   if (offset == 0)
> > +  anv_batch_set_error(batch, VK_ERROR_OUT_OF_HOST_MEMORY);
> > +
> > +   return offset;
> >  }
> >  
> >  void
> > @@ -239,8 +246,12 @@ anv_batch_emit_batch(struct anv_batch *batch,
> > struct anv_batch *other)
> > memcpy(batch->next, other->start, size);
> >  
> > offset = batch->next - batch->start;
> > -   anv_reloc_list_append(batch->relocs, batch->alloc,
> > - other->relocs, offset);
> > +   VkResult result = anv_reloc_list_append(batch->relocs, batch-
> > >alloc,
> > +   other->relocs, offset);
> > +   if (result != VK_SUCCESS) {
> > +  anv_batch_set_error(batch, result);
> > +  return;
> > +   }
> >  
> > batch->next += size;
> >  }
> > diff --git a/src/intel/vulkan/genX_blorp_exec.c
> > b/src/intel/vulkan/genX_blorp_exec.c
> > index 7084735..90adefe 100644
> > --- a/src/intel/vulkan/genX_blorp_exec.c
> > +++ b/src/intel/vulkan/genX_blorp_exec.c
> > @@ -57,8 +57,11 @@ blorp_surface_reloc(struct blorp_batch *batch,
> > uint32_t ss_offset,
> >  struct blorp_address address, uint32_t delta)
> >  {
> > struct anv_cmd_buffer