[Mesa-dev] [PATCH] radv: fix int-to-pointer-cast warnings on 32bit

[Grazvydas Ignotas]

Trivial.
---
 src/amd/vulkan/radv_debug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/amd/vulkan/radv_debug.c b/src/amd/vulkan/radv_debug.c
index 08fc80c12ab..e81b9cccb57 100644
--- a/src/amd/vulkan/radv_debug.c
+++ b/src/amd/vulkan/radv_debug.c
@@ -320,11 +320,11 @@ radv_dump_descriptors(struct radv_pipeline *pipeline, 
FILE *f)
int i;
 
fprintf(f, "List of descriptors:\n");
for (i = 0; i < MAX_SETS; i++) {
struct radv_descriptor_set *set =
-   (struct radv_descriptor_set *)ptr[i + 3];
+   (struct radv_descriptor_set *)(uintptr_t)ptr[i + 3];
 
radv_dump_descriptor_set(chip_class, set, i, f);
}
 }
 
@@ -565,19 +565,19 @@ radv_dump_compute_state(struct radv_pipeline 
*compute_pipeline, FILE *f)
 static struct radv_pipeline *
 radv_get_saved_graphics_pipeline(struct radv_device *device)
 {
uint64_t *ptr = (uint64_t *)device->trace_id_ptr;
 
-   return (struct radv_pipeline *)ptr[1];
+   return (struct radv_pipeline *)(uintptr_t)ptr[1];
 }
 
 static struct radv_pipeline *
 radv_get_saved_compute_pipeline(struct radv_device *device)
 {
uint64_t *ptr = (uint64_t *)device->trace_id_ptr;
 
-   return (struct radv_pipeline *)ptr[2];
+   return (struct radv_pipeline *)(uintptr_t)ptr[2];
 }
 
 static void
 radv_dump_dmesg(FILE *f)
 {
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH] radv: check return from mkdir

[Grazvydas Ignotas]

On Fri, Oct 5, 2018 at 3:38 AM Dave Airlie  wrote:
>
> From: Dave Airlie 
>
> There may be some security or sandbox reason this might fail, so
> check and fail appropriately.
> ---
>  src/amd/vulkan/radv_meta.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/amd/vulkan/radv_meta.c b/src/amd/vulkan/radv_meta.c
> index 1ec8896afa2..6616b1da65a 100644
> --- a/src/amd/vulkan/radv_meta.c
> +++ b/src/amd/vulkan/radv_meta.c
> @@ -248,7 +248,9 @@ radv_builtin_cache_path(char *path)
>
> strcpy(path, pwd.pw_dir);
> strcat(path, "/.cache");
> -   mkdir(path, 0755);
> +   ret = mkdir(path, 0755);
> +   if (ret == -1)

if (ret == -1 && errno != EEXIST) ?

> +  return false;
>
> ret = snprintf(path, PATH_MAX + 1, "%s%s%zd",
>pwd.pw_dir, suffix2, sizeof(void *) * 8);
> --
> 2.17.1

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

Re: [Mesa-dev] [PATCH 1/3] vulkan/alloc: Add a vk_strdup helper

[Grazvydas Ignotas]

On Thu, Aug 23, 2018 at 6:13 PM, Jason Ekstrand  wrote:
> ---
>  src/vulkan/util/vk_alloc.h | 17 +
>  1 file changed, 17 insertions(+)
>
> diff --git a/src/vulkan/util/vk_alloc.h b/src/vulkan/util/vk_alloc.h
> index f58a80625a8..bc2bb7509e4 100644
> --- a/src/vulkan/util/vk_alloc.h
> +++ b/src/vulkan/util/vk_alloc.h
> @@ -67,6 +67,23 @@ vk_free(const VkAllocationCallbacks *alloc, void *data)
> alloc->pfnFree(alloc->pUserData, data);
>  }
>
> +static inline char *
> +vk_strdup(const VkAllocationCallbacks *alloc, const char *s,
> +  VkSystemAllocationScope scope)
> +{
> +   if (s == NULL)
> +  return NULL;
> +
> +   size_t len = strlen(s);
> +   char *copy = vk_alloc(alloc, len, 1, scope);
> +   if (copy == NULL)
> +  return NULL;
> +
> +   memcpy(copy, s, len);

Don't you need the nul char?

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

Re: [Mesa-dev] [PATCH] vulkan/wsi: fix pointer-integer conversion warnings

[Grazvydas Ignotas]

On Tue, Aug 21, 2018 at 1:05 AM, Bas Nieuwenhuizen
 wrote:
> Reviewed-by: Bas Nieuwenhuizen 
>
> Did you have access to push?

Yeah, just pushed everything.

Gražvydas

>
> On Mon, Aug 20, 2018 at 11:40 PM, Grazvydas Ignotas  wrote:
>> For 32bit build. Trivial.
>> ---
>>  src/vulkan/wsi/wsi_common_display.c | 4 ++--
>>  src/vulkan/wsi/wsi_common_x11.c | 2 +-
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/vulkan/wsi/wsi_common_display.c 
>> b/src/vulkan/wsi/wsi_common_display.c
>> index e6cba188dfa..1e90bba460c 100644
>> --- a/src/vulkan/wsi/wsi_common_display.c
>> +++ b/src/vulkan/wsi/wsi_common_display.c
>> @@ -1515,11 +1515,11 @@ wsi_register_vblank_event(struct wsi_display_fence 
>> *fence,
>> for (;;) {
>>int ret = drmCrtcQueueSequence(wsi->fd, connector->crtc_id,
>>   flags,
>>   frame_requested,
>>   frame_queued,
>> - (uint64_t) fence);
>> + (uintptr_t) fence);
>>
>>if (!ret)
>>   return VK_SUCCESS;
>>
>>if (errno != ENOMEM) {
>> @@ -2340,11 +2340,11 @@ wsi_get_randr_output_display(VkPhysicalDevice 
>> physical_device,
>>wsi_display_get_output(wsi_device, connection, (xcb_randr_output_t) 
>> output);
>>
>> if (connector)
>>*display = wsi_display_connector_to_handle(connector);
>> else
>> -  *display = NULL;
>> +  *display = VK_NULL_HANDLE;
>> return VK_SUCCESS;
>>  }
>>
>>  #endif
>>
>> diff --git a/src/vulkan/wsi/wsi_common_x11.c 
>> b/src/vulkan/wsi/wsi_common_x11.c
>> index 7b930884b47..aaa4d1e658e 100644
>> --- a/src/vulkan/wsi/wsi_common_x11.c
>> +++ b/src/vulkan/wsi/wsi_common_x11.c
>> @@ -1323,11 +1323,11 @@ x11_surface_create_swapchain(VkIcdSurfaceBase 
>> *icd_surface,
>>  * last completion mode, to ensure we don't get into reallocation
>>  * cycles. If we are starting anew, we set 'COPY', as that is the only
>>  * mode which provokes reallocation when anything changes, to make
>>  * sure we have the most optimal allocation.
>>  */
>> -   struct x11_swapchain *old_chain = (void *) pCreateInfo->oldSwapchain;
>> +   struct x11_swapchain *old_chain = (void *)(intptr_t) 
>> pCreateInfo->oldSwapchain;
>> if (old_chain)
>>chain->last_present_mode = old_chain->last_present_mode;
>> else
>>chain->last_present_mode = XCB_PRESENT_COMPLETE_MODE_COPY;
>>
>> --
>> 2.17.1
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] vulkan/wsi: fix pointer-integer conversion warnings

[Grazvydas Ignotas]

For 32bit build. Trivial.
---
 src/vulkan/wsi/wsi_common_display.c | 4 ++--
 src/vulkan/wsi/wsi_common_x11.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/vulkan/wsi/wsi_common_display.c 
b/src/vulkan/wsi/wsi_common_display.c
index e6cba188dfa..1e90bba460c 100644
--- a/src/vulkan/wsi/wsi_common_display.c
+++ b/src/vulkan/wsi/wsi_common_display.c
@@ -1515,11 +1515,11 @@ wsi_register_vblank_event(struct wsi_display_fence 
*fence,
for (;;) {
   int ret = drmCrtcQueueSequence(wsi->fd, connector->crtc_id,
  flags,
  frame_requested,
  frame_queued,
- (uint64_t) fence);
+ (uintptr_t) fence);
 
   if (!ret)
  return VK_SUCCESS;
 
   if (errno != ENOMEM) {
@@ -2340,11 +2340,11 @@ wsi_get_randr_output_display(VkPhysicalDevice 
physical_device,
   wsi_display_get_output(wsi_device, connection, (xcb_randr_output_t) 
output);
 
if (connector)
   *display = wsi_display_connector_to_handle(connector);
else
-  *display = NULL;
+  *display = VK_NULL_HANDLE;
return VK_SUCCESS;
 }
 
 #endif
 
diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c
index 7b930884b47..aaa4d1e658e 100644
--- a/src/vulkan/wsi/wsi_common_x11.c
+++ b/src/vulkan/wsi/wsi_common_x11.c
@@ -1323,11 +1323,11 @@ x11_surface_create_swapchain(VkIcdSurfaceBase 
*icd_surface,
 * last completion mode, to ensure we don't get into reallocation
 * cycles. If we are starting anew, we set 'COPY', as that is the only
 * mode which provokes reallocation when anything changes, to make
 * sure we have the most optimal allocation.
 */
-   struct x11_swapchain *old_chain = (void *) pCreateInfo->oldSwapchain;
+   struct x11_swapchain *old_chain = (void *)(intptr_t) 
pCreateInfo->oldSwapchain;
if (old_chain)
   chain->last_present_mode = old_chain->last_present_mode;
else
   chain->last_present_mode = XCB_PRESENT_COMPLETE_MODE_COPY;
 
-- 
2.17.1

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

[Mesa-dev] [PATCH 2/2] radv: use different builtin shader cache for 32bit

[Grazvydas Ignotas]

Currently if 64bit and 32bit programs are used interchangeably, radv
will keep overwriting the cache. Use separate cache files to avoid
that.
---
 src/amd/vulkan/radv_meta.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/src/amd/vulkan/radv_meta.c b/src/amd/vulkan/radv_meta.c
index b8d0a7cfc70..1ec8896afa2 100644
--- a/src/amd/vulkan/radv_meta.c
+++ b/src/amd/vulkan/radv_meta.c
@@ -232,31 +232,29 @@ radv_builtin_cache_path(char *path)
char *xdg_cache_home = getenv("XDG_CACHE_HOME");
const char *suffix = "/radv_builtin_shaders";
const char *suffix2 = "/.cache/radv_builtin_shaders";
struct passwd pwd, *result;
char path2[PATH_MAX + 1]; /* PATH_MAX is not a real max,but suffices 
here. */
+   int ret;
 
if (xdg_cache_home) {
-
-   if (strlen(xdg_cache_home) + strlen(suffix) > PATH_MAX)
-   return false;
-
-   strcpy(path, xdg_cache_home);
-   strcat(path, suffix);
-   return true;
+   ret = snprintf(path, PATH_MAX + 1, "%s%s%zd",
+  xdg_cache_home, suffix, sizeof(void *) * 8);
+   return ret > 0 && ret < PATH_MAX + 1;
}
 
getpwuid_r(getuid(), , path2, PATH_MAX - strlen(suffix2), );
if (!result)
return false;
 
strcpy(path, pwd.pw_dir);
strcat(path, "/.cache");
mkdir(path, 0755);
 
-   strcat(path, suffix);
-   return true;
+   ret = snprintf(path, PATH_MAX + 1, "%s%s%zd",
+  pwd.pw_dir, suffix2, sizeof(void *) * 8);
+   return ret > 0 && ret < PATH_MAX + 1;
 }
 
 static bool
 radv_load_meta_pipeline(struct radv_device *device)
 {
-- 
2.17.1

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

[Mesa-dev] [PATCH 1/2] radv: place pointer length into cache uuid

[Grazvydas Ignotas]

Thanks to reproducible builds, binary file timestamps may be identical
for both 32bit and 64bit packages when built from the same source.
This means radv will use the same cache for both 32 and 64 bit
processes, which leads to crashes.

Conveniently there is a spare byte in cache_uuid, let's place the
pointer size there.

Fixes: f4e499ec79 "radv: add initial non-conformant radv vulkan driver"
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107601
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105904
---
 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 cc88abb57a8..79dbbd886d5 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -58,11 +58,11 @@ radv_device_get_cache_uuid(enum radeon_family family, void 
*uuid)
return -1;
 
memcpy(uuid, _timestamp, 4);
memcpy((char*)uuid + 4, _timestamp, 4);
memcpy((char*)uuid + 8, , 2);
-   snprintf((char*)uuid + 10, VK_UUID_SIZE - 10, "radv");
+   snprintf((char*)uuid + 10, VK_UUID_SIZE - 10, "radv%zd", sizeof(void 
*));
return 0;
 }
 
 static void
 radv_get_driver_uuid(void *uuid)
-- 
2.17.1

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

[Mesa-dev] [PATCH] llvmpipe: add cc clobber to inline asm

[Grazvydas Ignotas]

The bsr instruction modifies flags, so that needs to be indicated to the
compiler. No effect on generated code, but still needed for correctness.
---
 src/gallium/drivers/llvmpipe/lp_setup_tri.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/llvmpipe/lp_setup_tri.c 
b/src/gallium/drivers/llvmpipe/lp_setup_tri.c
index cec6198ec63..1852ec05d56 100644
--- a/src/gallium/drivers/llvmpipe/lp_setup_tri.c
+++ b/src/gallium/drivers/llvmpipe/lp_setup_tri.c
@@ -732,11 +732,12 @@ floor_pot(uint32_t n)
if (n == 0)
   return 0;
 
__asm__("bsr %1,%0"
   : "=r" (n)
-  : "rm" (n));
+  : "rm" (n)
+  : "cc");
return 1 << n;
 #else
n |= (n >>  1);
n |= (n >>  2);
n |= (n >>  4);
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH 2/3] nir: Add a discard optimization pass

[Grazvydas Ignotas]

On Wed, Jul 4, 2018 at 9:13 AM, Jason Ekstrand  wrote:
> Many fragment shaders do a discard using relatively little information
> but still put the discard fairly far down in the shader for no good
> reason.  If the discard is moved higher up, we can possibly avoid doing
> some or almost all of the work in the shader.  When this lets us skip
> texturing operations, it's an especially high win.
>
> One of the biggest offenders here is DXVK.  The D3D APIs have different
> rules for discards than OpenGL and Vulkan.  One effective way (which is
> what DXVK uses) to implement DX behavior on top of GL or Vulkan is to
> wait until the very end of the shader to discard.  This ends up in the
> pessimal case where we always do all of the work before discarding.
> This pass helps some DXVK shaders significantly.
> ---
>  src/compiler/Makefile.sources  |   1 +
>  src/compiler/nir/meson.build   |   1 +
>  src/compiler/nir/nir.h |  10 +
>  src/compiler/nir/nir_opt_discard.c | 404 +
>  4 files changed, 416 insertions(+)
>  create mode 100644 src/compiler/nir/nir_opt_discard.c
>
> diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
> index 9e3fbdc2612..8600ce81281 100644
> --- a/src/compiler/Makefile.sources
> +++ b/src/compiler/Makefile.sources
> @@ -271,6 +271,7 @@ NIR_FILES = \
> nir/nir_opt_cse.c \
> nir/nir_opt_dce.c \
> nir/nir_opt_dead_cf.c \
> +   nir/nir_opt_discard.c \
> nir/nir_opt_gcm.c \
> nir/nir_opt_global_to_local.c \
> nir/nir_opt_if.c \
> diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
> index 28aa8de7014..e339258bb94 100644
> --- a/src/compiler/nir/meson.build
> +++ b/src/compiler/nir/meson.build
> @@ -156,6 +156,7 @@ files_libnir = files(
>'nir_opt_cse.c',
>'nir_opt_dce.c',
>'nir_opt_dead_cf.c',
> +  'nir_opt_discard.c',
>'nir_opt_gcm.c',
>'nir_opt_global_to_local.c',
>'nir_opt_if.c',
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 92ab3a699cc..44006dac160 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -2022,6 +2022,13 @@ typedef struct nir_shader_compiler_options {
>  */
> bool vs_inputs_dual_locations;
>
> +   /**
> +* Whether or not derivatives are still a safe operation after a discard
> +* has occurred.  Optimization passes may be able to be a bit more
> +* agressive if this is true.
> +*/
> +   bool derivatives_safe_after_discard;
> +
> unsigned max_unroll_iterations;
>  } nir_shader_compiler_options;
>
> @@ -2899,6 +2906,9 @@ bool nir_opt_dce(nir_shader *shader);
>
>  bool nir_opt_dead_cf(nir_shader *shader);
>
> +bool nir_opt_discard_if(nir_shader *shader);
> +bool nir_opt_move_discards_to_top(nir_shader *shader);
> +
>  bool nir_opt_gcm(nir_shader *shader, bool value_number);
>
>  bool nir_opt_if(nir_shader *shader);
> diff --git a/src/compiler/nir/nir_opt_discard.c 
> b/src/compiler/nir/nir_opt_discard.c
> new file mode 100644
> index 000..eb1a8296282
> --- /dev/null
> +++ b/src/compiler/nir/nir_opt_discard.c
> @@ -0,0 +1,404 @@
> +/*
> + * Copyright © 2014 Intel Corporation

2018?

> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Jason Ekstrand (ja...@jlekstrand.net)
> + *
> + */
> +
> +#include "nir.h"
> +#include "nir_builder.h"
> +#include "nir_control_flow.h"
> +#include "nir_worklist.h"
> +
> +static bool
> +block_has_only_discard(nir_block *block)
> +{
> +   nir_instr *instr = nir_block_first_instr(block);
> +   if (instr == NULL || instr != nir_block_last_instr(block))
> +  return false;
> +
> +   if (instr->type != nir_instr_type_intrinsic)
> +  return false;
> +
> +   nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr);
> +   return intrin->intrinsic == nir_intrinsic_discard;
> +}
> +

[Mesa-dev] [PATCH] radeonsi: add a debug flag to zero vram allocations

[Grazvydas Ignotas]

This allows to avoid having to see garbage in Dying Light loading screen
at least, which probably expects Windows/NV behavior of all allocations
being zeroed by default.

Analogous to radv flag with the same name.
---
 src/gallium/drivers/radeonsi/si_pipe.c| 1 +
 src/gallium/drivers/radeonsi/si_pipe.h| 1 +
 src/gallium/winsys/amdgpu/drm/amdgpu_bo.c | 3 +++
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c | 1 +
 src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h | 1 +
 5 files changed, 7 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
b/src/gallium/drivers/radeonsi/si_pipe.c
index cc871b1..1c3405f 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -80,10 +80,11 @@ static const struct debug_named_value debug_options[] = {
{ "forcedma", DBG(FORCE_DMA), "Use asynchronous DMA for all operations 
when possible." },
{ "nodma", DBG(NO_ASYNC_DMA), "Disable asynchronous DMA" },
{ "nowc", DBG(NO_WC), "Disable GTT write combining" },
{ "check_vm", DBG(CHECK_VM), "Check VM faults and dump debug info." },
{ "reserve_vmid", DBG(RESERVE_VMID), "Force VMID reservation per 
context." },
+   { "zerovram", DBG(ZERO_VRAM), "Clear VRAM allocations." },
 
/* 3D engine options: */
{ "switch_on_eop", DBG(SWITCH_ON_EOP), "Program WD/IA to switch on 
end-of-packet." },
{ "nooutoforder", DBG(NO_OUT_OF_ORDER), "Disable out-of-order 
rasterization" },
{ "nodpbb", DBG(NO_DPBB), "Disable DPBB." },
diff --git a/src/gallium/drivers/radeonsi/si_pipe.h 
b/src/gallium/drivers/radeonsi/si_pipe.h
index eb0c226..60dc8b0 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.h
+++ b/src/gallium/drivers/radeonsi/si_pipe.h
@@ -139,10 +139,11 @@ enum {
DBG_FORCE_DMA,
DBG_NO_ASYNC_DMA,
DBG_NO_WC,
DBG_CHECK_VM,
DBG_RESERVE_VMID,
+   DBG_ZERO_VRAM,
 
/* 3D engine options: */
DBG_SWITCH_ON_EOP,
DBG_NO_OUT_OF_ORDER,
DBG_NO_DPBB,
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c 
b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
index df8b829..15916ad 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
@@ -426,10 +426,13 @@ static struct amdgpu_winsys_bo *amdgpu_create_bo(struct 
amdgpu_winsys *ws,
if (flags & RADEON_FLAG_GTT_WC)
   request.flags |= AMDGPU_GEM_CREATE_CPU_GTT_USWC;
if (flags & RADEON_FLAG_NO_INTERPROCESS_SHARING &&
ws->info.has_local_buffers)
   request.flags |= AMDGPU_GEM_CREATE_VM_ALWAYS_VALID;
+   if (ws->zero_all_vram_allocs &&
+   (request.preferred_heap & AMDGPU_GEM_DOMAIN_VRAM))
+  request.flags |= AMDGPU_GEM_CREATE_VRAM_CLEARED;
 
r = amdgpu_bo_alloc(ws->dev, , _handle);
if (r) {
   fprintf(stderr, "amdgpu: Failed to allocate a buffer:\n");
   fprintf(stderr, "amdgpu:size  : %"PRIu64" bytes\n", size);
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c 
b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
index caa7991..44429e2 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c
@@ -60,10 +60,11 @@ static bool do_winsys_init(struct amdgpu_winsys *ws, int fd)
}
 
ws->check_vm = strstr(debug_get_option("R600_DEBUG", ""), "check_vm") != 
NULL;
ws->debug_all_bos = debug_get_option_all_bos();
ws->reserve_vmid = strstr(debug_get_option("R600_DEBUG", ""), 
"reserve_vmid") != NULL;
+   ws->zero_all_vram_allocs = strstr(debug_get_option("R600_DEBUG", ""), 
"zerovram") != NULL;
 
return true;
 
 fail:
amdgpu_device_deinitialize(ws->dev);
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h 
b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
index a6784e8..8079255 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.h
@@ -77,10 +77,11 @@ struct amdgpu_winsys {
ADDR_HANDLE addrlib;
 
bool check_vm;
bool debug_all_bos;
bool reserve_vmid;
+   bool zero_all_vram_allocs;
 
/* List of all allocated buffers */
simple_mtx_t global_bo_list_lock;
struct list_head global_bo_list;
unsigned num_buffers;
-- 
2.7.4

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

[Mesa-dev] [PATCH] radeonsi: use shifts for sign extension

[Grazvydas Ignotas]

Avoids a branch and reduces code size a tiny bit:
text   data bss  dechex filename
10804563 398653 2070368 13273584 ca89f0 /tmp/radeonsi_dri.so.old
10804499 398653 2070368 13273520 ca89b0 /tmp/radeonsi_dri.so
---
 src/gallium/drivers/radeonsi/si_descriptors.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
b/src/gallium/drivers/radeonsi/si_descriptors.c
index 57a3124..f368ea6 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -98,12 +98,12 @@ static uint64_t si_desc_extract_buffer_address(const 
uint32_t *desc)
 {
uint64_t va = desc[0] |
  ((uint64_t)G_008F04_BASE_ADDRESS_HI(desc[1]) << 32);
 
/* Sign-extend the 48-bit address. */
-   if (va & (1ull << 47))
-   va |= 0xull << 48;
+   va <<= 16;
+   va = (int64_t)va >> 16;
return va;
 }
 
 static void si_init_descriptor_list(uint32_t *desc_list,
unsigned element_dw_size,
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 1/2] vkpipeline-db: add sType to pipeline info

[Grazvydas Ignotas]

On Tue, Jun 12, 2018 at 8:07 AM, Timothy Arceri  wrote:
> From: Timothy Arceri 

localhost.localdomain?

>
> This avoids hitting asserts in anv.
> ---
>  run.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/run.c b/run.c
> index eafbd7d..a962887 100644
> --- a/run.c
> +++ b/run.c
> @@ -264,6 +264,7 @@ create_graphics_pipeline(VkDevice device, struct 
> pipeline_info *info,
>  return result;
>
>  /* Graphics pipeline. */
> +pipelineInfo.sType = VK_STRUCTURE_TYPE_GRAPHICS_PIPELINE_CREATE_INFO;
>  pipelineInfo.stageCount = info->stageCount;
>  pipelineInfo.pStages = info->pShaderStagesInfo;
>  pipelineInfo.pVertexInputState = >vertexInputState;
> @@ -292,6 +293,7 @@ create_compute_pipeline(VkDevice device, struct 
> pipeline_info *info,
>  VkComputePipelineCreateInfo pipelineInfo = {};
>
>  /* Compute pipeline. */
> +pipelineInfo.sType = VK_STRUCTURE_TYPE_COMPUTE_PIPELINE_CREATE_INFO;
>  pipelineInfo.stage = *info->pShaderStagesInfo;
>  pipelineInfo.layout = layout;
>
> --
> 2.17.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] radv: add a workaround for DXVK hangs by setting amdgpu-skip-threshold

[Grazvydas Ignotas]

I haven't tried it, but maybe calling LLVMParseCommandLineOptions()
second time would work (after creating the target machine, through
call_once) to avoid all this code duplication?

Gražvydas

On Fri, Jun 8, 2018 at 12:38 PM, Samuel Pitoiset
 wrote:
> Workaround for bug in llvm that causes the GPU to hang in presence
> of nested loops because there is an exec mask issue. The proper
> solution is to fix LLVM but this might require a bunch of work.
>
> This fixes a bunch of GPU hangs that happen with DXVK.
>
> Vega10:
> Totals from affected shaders:
> SGPRS: 110456 -> 110456 (0.00 %)
> VGPRS: 122800 -> 122800 (0.00 %)
> Spilled SGPRs: 7478 -> 7478 (0.00 %)
> Spilled VGPRs: 36 -> 36 (0.00 %)
> Code Size: 9901104 -> 9922928 (0.22 %) bytes
> Max Waves: 7143 -> 7143 (0.00 %)
>
> Code size slightly increases because it inserts more branch
> instructions but that's expected. I don't see any real performance
> changes.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105613
> Cc: mesa-sta...@lists.freedesktop.org
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/amd/vulkan/radv_shader.c | 79 +++-
>  1 file changed, 78 insertions(+), 1 deletion(-)
>
> diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
> index 68f2a55e81..76790a1904 100644
> --- a/src/amd/vulkan/radv_shader.c
> +++ b/src/amd/vulkan/radv_shader.c
> @@ -36,6 +36,7 @@
>
>  #include 
>  #include 
> +#include 
>
>  #include "sid.h"
>  #include "gfx9d.h"
> @@ -467,6 +468,82 @@ radv_fill_shader_variant(struct radv_device *device,
> memcpy(ptr, binary->code, binary->code_size);
>  }
>
> +static void radv_init_llvm_target()
> +{
> +   LLVMInitializeAMDGPUTargetInfo();
> +   LLVMInitializeAMDGPUTarget();
> +   LLVMInitializeAMDGPUTargetMC();
> +   LLVMInitializeAMDGPUAsmPrinter();
> +
> +   /* For inline assembly. */
> +   LLVMInitializeAMDGPUAsmParser();
> +
> +   /* Workaround for bug in llvm 4.0 that causes image intrinsics
> +* to disappear.
> +* https://reviews.llvm.org/D26348
> +*
> +* Workaround for bug in llvm that causes the GPU to hang in presence
> +* of nested loops because there is an exec mask issue. The proper
> +* solution is to fix LLVM but this might require a bunch of work.
> +* https://bugs.llvm.org/show_bug.cgi?id=37744
> +*
> +* "mesa" is the prefix for error messages.
> +*/
> +   const char *argv[3] = { "mesa", "-simplifycfg-sink-common=false",
> +   "-amdgpu-skip-threshold=1" };
> +   LLVMParseCommandLineOptions(3, argv, NULL);
> +}
> +
> +static once_flag radv_init_llvm_target_once_flag = ONCE_FLAG_INIT;
> +
> +static LLVMTargetRef radv_get_llvm_target(const char *triple)
> +{
> +   LLVMTargetRef target = NULL;
> +   char *err_message = NULL;
> +
> +   call_once(_init_llvm_target_once_flag, radv_init_llvm_target);
> +
> +   if (LLVMGetTargetFromTriple(triple, , _message)) {
> +   fprintf(stderr, "Cannot find target for triple %s ", triple);
> +   if (err_message) {
> +   fprintf(stderr, "%s\n", err_message);
> +   }
> +   LLVMDisposeMessage(err_message);
> +   return NULL;
> +   }
> +   return target;
> +}
> +
> +static LLVMTargetMachineRef radv_create_target_machine(enum radeon_family 
> family,
> +  enum 
> ac_target_machine_options tm_options,
> +  const char 
> **out_triple)
> +{
> +   assert(family >= CHIP_TAHITI);
> +   char features[256];
> +   const char *triple = (tm_options & AC_TM_SUPPORTS_SPILL) ? 
> "amdgcn-mesa-mesa3d" : "amdgcn--";
> +   LLVMTargetRef target = radv_get_llvm_target(triple);
> +
> +   snprintf(features, sizeof(features),
> +
> "+DumpCode,+vgpr-spilling,-fp32-denormals,+fp64-denormals%s%s%s%s",
> +tm_options & AC_TM_SISCHED ? ",+si-scheduler" : "",
> +tm_options & AC_TM_FORCE_ENABLE_XNACK ? ",+xnack" : "",
> +tm_options & AC_TM_FORCE_DISABLE_XNACK ? ",-xnack" : "",
> +tm_options & AC_TM_PROMOTE_ALLOCA_TO_SCRATCH ? 
> ",-promote-alloca" : "");
> +
> +   LLVMTargetMachineRef tm = LLVMCreateTargetMachine(
> +target,
> +triple,
> +ac_get_llvm_processor_name(family),
> +features,
> +LLVMCodeGenLevelDefault,
> +LLVMRelocDefault,
> +LLVMCodeModelDefault);
> +
> +   if (out_triple)
> +   *out_triple = triple;
> +   return tm;
> +}
> +
>  static struct radv_shader_variant *
>  shader_variant_create(struct radv_device *device,
>  

Re: [Mesa-dev] [PATCH 2/6] radeonsi: disable primitive binning for all blitter ops

[Grazvydas Ignotas]

On Thu, May 17, 2018 at 4:47 AM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> same as Vulkan.

Ambiguous. Did you mean amdvlk?

Gražvydas

> ---
>  src/gallium/drivers/radeonsi/si_blit.c  | 10 ++
>  src/gallium/drivers/radeonsi/si_pipe.h  |  2 +-
>  src/gallium/drivers/radeonsi/si_state_binning.c |  2 +-
>  3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_blit.c 
> b/src/gallium/drivers/radeonsi/si_blit.c
> index 6f4cd1f9044..fe059b36577 100644
> --- a/src/gallium/drivers/radeonsi/si_blit.c
> +++ b/src/gallium/drivers/radeonsi/si_blit.c
> @@ -68,24 +68,34 @@ void si_blitter_begin(struct si_context *sctx, enum 
> si_blitter_op op)
> util_blitter_save_fragment_sampler_states(
> sctx->blitter, 2,
> 
> (void**)sctx->samplers[PIPE_SHADER_FRAGMENT].sampler_states);
>
> util_blitter_save_fragment_sampler_views(sctx->blitter, 2,
> sctx->samplers[PIPE_SHADER_FRAGMENT].views);
> }
>
> if (op & SI_DISABLE_RENDER_COND)
> sctx->render_cond_force_off = true;
> +
> +   if (sctx->screen->dpbb_allowed) {
> +   sctx->dpbb_force_off = true;
> +   si_mark_atom_dirty(sctx, >atoms.s.dpbb_state);
> +   }
>  }
>
>  void si_blitter_end(struct si_context *sctx)
>  {
> +   if (sctx->screen->dpbb_allowed) {
> +   sctx->dpbb_force_off = false;
> +   si_mark_atom_dirty(sctx, >atoms.s.dpbb_state);
> +   }
> +
> sctx->render_cond_force_off = false;
>
> /* Restore shader pointers because the VS blit shader changed all
>  * non-global VS user SGPRs. */
> sctx->shader_pointers_dirty |= SI_DESCS_SHADER_MASK(VERTEX);
> sctx->vertex_buffer_pointer_dirty = sctx->vb_descriptors_buffer != 
> NULL;
> si_mark_atom_dirty(sctx, >atoms.s.shader_pointers);
>  }
>
>  static unsigned u_max_sample(struct pipe_resource *r)
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.h 
> b/src/gallium/drivers/radeonsi/si_pipe.h
> index 6917d5e6068..5d1671fb87f 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.h
> +++ b/src/gallium/drivers/radeonsi/si_pipe.h
> @@ -920,21 +920,21 @@ struct si_context {
>
> /* Debug state. */
> boolis_debug;
> struct si_saved_cs  *current_saved_cs;
> uint64_tdmesg_timestamp;
> unsignedapitrace_call_number;
>
> /* Other state */
> bool need_check_render_feedback;
> booldecompression_enabled;
> -
> +   booldpbb_force_off;
> boolvs_writes_viewport_index;
> boolvs_disables_clipping_viewport;
>
> /* Precomputed IA_MULTI_VGT_PARAM */
> union si_vgt_param_key  ia_multi_vgt_param_key;
> unsignedia_multi_vgt_param[SI_NUM_VGT_PARAM_STATES];
>
> /* Bindless descriptors. */
> struct si_descriptors   bindless_descriptors;
> struct util_idalloc bindless_used_slots;
> diff --git a/src/gallium/drivers/radeonsi/si_state_binning.c 
> b/src/gallium/drivers/radeonsi/si_state_binning.c
> index 80b1137fd0c..2605b59bbef 100644
> --- a/src/gallium/drivers/radeonsi/si_state_binning.c
> +++ b/src/gallium/drivers/radeonsi/si_state_binning.c
> @@ -336,21 +336,21 @@ static void si_emit_dpbb_disable(struct si_context 
> *sctx)
>
>  void si_emit_dpbb_state(struct si_context *sctx)
>  {
> struct si_screen *sscreen = sctx->screen;
> struct si_state_blend *blend = sctx->queued.named.blend;
> struct si_state_dsa *dsa = sctx->queued.named.dsa;
> unsigned db_shader_control = sctx->ps_db_shader_control;
>
> assert(sctx->chip_class >= GFX9);
>
> -   if (!sscreen->dpbb_allowed || !blend || !dsa) {
> +   if (!sscreen->dpbb_allowed || !blend || !dsa || sctx->dpbb_force_off) 
> {
> si_emit_dpbb_disable(sctx);
> return;
> }
>
> bool ps_can_kill = G_02880C_KILL_ENABLE(db_shader_control) ||
>G_02880C_MASK_EXPORT_ENABLE(db_shader_control) ||
>
> G_02880C_COVERAGE_TO_MASK_ENABLE(db_shader_control) ||
>blend->alpha_to_coverage;
>
> /* This is ported from Vulkan, but it doesn't make much sense to me.
> --
> 2.17.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 v2] st/mesa: only define GLSL 1.4 for compat if driver supports it

[Grazvydas Ignotas]

On Wed, May 16, 2018 at 5:02 PM, Christian Gmeiner
 wrote:
> Currently GLSL 1.4 is defined for all gallium drivers even only
> GLSL 1.2 is supported as seen on etnaviv.
>
> v1 -> v2:
>  - use _min(..) as suggested by Lucas Stach and Michel Dänzer
>
> Fixes 4560aad780b ("mesa: add GLSLVersionCompat constant")

Should be "Fixes: ...".
See git log for examples.

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

[Mesa-dev] [PATCH] radv: assorted typo fixes

[Grazvydas Ignotas]

Trivial.
---
 src/amd/vulkan/radv_cmd_buffer.c   | 4 ++--
 src/amd/vulkan/radv_descriptor_set.c   | 2 +-
 src/amd/vulkan/radv_device.c   | 6 +++---
 src/amd/vulkan/radv_entrypoints_gen.py | 2 +-
 src/amd/vulkan/radv_image.c| 4 ++--
 src/amd/vulkan/radv_nir_to_llvm.c  | 4 ++--
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index baa28d4..1ca6874 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -236,11 +236,11 @@ static VkResult radv_create_cmd_buffer(
if (pool) {
list_addtail(_buffer->pool_link, >cmd_buffers);
cmd_buffer->queue_family_index = pool->queue_family_index;
 
} else {
-   /* Init the pool_link so we can safefly call list_del when we 
destroy
+   /* Init the pool_link so we can safely call list_del when we 
destroy
 * the command buffer
 */
list_inithead(_buffer->pool_link);
cmd_buffer->queue_family_index = RADV_QUEUE_GENERAL;
}
@@ -1154,11 +1154,11 @@ radv_load_depth_clear_regs(struct radv_cmd_buffer 
*cmd_buffer,
radeon_emit(cmd_buffer->cs, PKT3(PKT3_PFP_SYNC_ME, 0, 0));
radeon_emit(cmd_buffer->cs, 0);
 }
 
 /*
- *with DCC some colors don't require CMASK elimiation before being
+ * With DCC some colors don't require CMASK elimination before being
  * used as a texture. This sets a predicate value to determine if the
  * cmask eliminate is required.
  */
 void
 radv_set_dcc_need_cmask_elim_pred(struct radv_cmd_buffer *cmd_buffer,
diff --git a/src/amd/vulkan/radv_descriptor_set.c 
b/src/amd/vulkan/radv_descriptor_set.c
index 4b08a1f..9d783b8 100644
--- a/src/amd/vulkan/radv_descriptor_set.c
+++ b/src/amd/vulkan/radv_descriptor_set.c
@@ -320,11 +320,11 @@ void radv_GetDescriptorSetLayoutSupport(VkDevice device,
pSupport->supported = supported;
 }
 
 /*
  * Pipeline layouts.  These have nothing to do with the pipeline.  They are
- * just muttiple descriptor set layouts pasted together
+ * just multiple descriptor set layouts pasted together.
  */
 
 VkResult radv_CreatePipelineLayout(
VkDevice_device,
const VkPipelineLayoutCreateInfo*   pCreateInfo,
diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 9fe415c..08795dd 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -276,11 +276,11 @@ radv_physical_device_init(struct radv_physical_device 
*device,
/* These flags affect shader compilation. */
uint64_t shader_env_flags =
(device->instance->perftest_flags & RADV_PERFTEST_SISCHED ? 0x1 
: 0) |
(device->instance->debug_flags & RADV_DEBUG_UNSAFE_MATH ? 0x2 : 
0);
 
-   /* The gpu id is already embeded in the uuid so we just pass "radv"
+   /* The gpu id is already embedded in the uuid so we just pass "radv"
 * when creating the cache.
 */
char buf[VK_UUID_SIZE * 2 + 1];
disk_cache_format_hex_id(buf, device->cache_uuid, VK_UUID_SIZE * 2);
device->disk_cache = disk_cache_create(device->name, buf, 
shader_env_flags);
@@ -298,11 +298,11 @@ radv_physical_device_init(struct radv_physical_device 
*device,
device->rbplus_allowed = device->rad_info.family == CHIP_STONEY 
||
 device->rad_info.family == CHIP_VEGA12 
||
 device->rad_info.family == CHIP_RAVEN;
}
 
-   /* The mere presense of CLEAR_STATE in the IB causes random GPU hangs
+   /* The mere presence of CLEAR_STATE in the IB causes random GPU hangs
 * on SI.
 */
device->has_clear_state = device->rad_info.chip_class >= CIK;
 
device->cpdma_prefetch_writes_memory = device->rad_info.chip_class <= 
VI;
@@ -1464,11 +1464,11 @@ VkResult radv_CreateDevice(
 
/* The maximum number of scratch waves. Scratch space isn't divided
 * evenly between CUs. The number is only a function of the number of 
CUs.
 * We can decrease the constant to decrease the scratch buffer size.
 *
-* sctx->scratch_waves must be >= the maximum posible size of
+* sctx->scratch_waves must be >= the maximum possible size of
 * 1 threadgroup, so that the hw doesn't hang from being unable
 * to start any.
 *
 * The recommended value is 4 per CU at most. Higher numbers don't
 * bring much benefit, but they still occupy chip resources (think
diff --git a/src/amd/vulkan/radv_entrypoints_gen.py 
b/src/amd/vulkan/radv_entrypoints_gen.py
index 892491e..a201142 100644
--- a/src/amd/vulkan/radv_entrypoints_gen.py
+++ b/src/amd/vulkan/radv_entrypoints_gen.py
@@ -114,11 +114,11 @@ struct string_map_entry {
uint32_t name;
uint32_t hash;

Re: [Mesa-dev] [PATCH v2 1/3] mesa: actually support compat profile creation with MESA_GL_VERSION_OVERRIDE

[Grazvydas Ignotas]

On Wed, May 2, 2018 at 3:38 PM, Timothy Arceri <tarc...@itsqueeze.com> wrote:
> On 02/05/18 22:36, Timothy Arceri wrote:
>>
>> On 02/05/18 21:48, Grazvydas Ignotas wrote:
>>>
>>> On Wed, May 2, 2018 at 1:27 PM, Timothy Arceri <tarc...@itsqueeze.com>
>>> wrote:
>>>>
>>>> ---
>>>>   src/mesa/drivers/dri/common/dri_util.c | 9 +
>>>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/common/dri_util.c
>>>> b/src/mesa/drivers/dri/common/dri_util.c
>>>> index 7cb6248b130..d72f72d0756 100644
>>>> --- a/src/mesa/drivers/dri/common/dri_util.c
>>>> +++ b/src/mesa/drivers/dri/common/dri_util.c
>>>> @@ -389,10 +389,11 @@ driCreateContextAttribs(__DRIscreen *screen, int
>>>> api,
>>>>   screen->max_gl_compat_version < 31)
>>>>  mesa_api = API_OPENGL_CORE;
>>>>
>>>> -if (mesa_api == API_OPENGL_COMPAT
>>>> -&& ((ctx_config.major_version > 3)
>>>> -|| (ctx_config.major_version == 3 &&
>>>> -ctx_config.minor_version >= 2))) {
>>>> +if (mesa_api == API_OPENGL_COMPAT &&
>>>> +((ctx_config.major_version > 3) || (ctx_config.major_version ==
>>>> 3 &&
>>>> +ctx_config.minor_version >=
>>>> 2)) &&
>>>> +!((ctx_config.major_version * 10 + ctx_config.minor_version) <=
>>>> +  screen->max_gl_compat_version)) {
>>>
>>>
>>> nit: what about using (a * 10 + b  > c) to make it easier to read than
>>> !(a * 10 + b <= c) ?
>>
>>
>> That will break 3.1 compat on radeonsi because max_gl_compat_version is -1
>> unless overridden.
>
>
> Ignore that I should have just gone to bed rather than replying. That should
> work ok.

Hmm, if max_gl_compat_version can be -1 then it is still a problem,
.major_version and .minor_version are unsigned, so the compiler does
an unsigned comparison in the end (I've checked the asm), so in both
cases (ctx_config.major_version * 10 + ctx_config.minor_version) needs
casting to int.

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

Re: [Mesa-dev] [PATCH v2 1/3] mesa: actually support compat profile creation with MESA_GL_VERSION_OVERRIDE

[Grazvydas Ignotas]

On Wed, May 2, 2018 at 1:27 PM, Timothy Arceri  wrote:
> ---
>  src/mesa/drivers/dri/common/dri_util.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/common/dri_util.c 
> b/src/mesa/drivers/dri/common/dri_util.c
> index 7cb6248b130..d72f72d0756 100644
> --- a/src/mesa/drivers/dri/common/dri_util.c
> +++ b/src/mesa/drivers/dri/common/dri_util.c
> @@ -389,10 +389,11 @@ driCreateContextAttribs(__DRIscreen *screen, int api,
>  screen->max_gl_compat_version < 31)
> mesa_api = API_OPENGL_CORE;
>
> -if (mesa_api == API_OPENGL_COMPAT
> -&& ((ctx_config.major_version > 3)
> -|| (ctx_config.major_version == 3 &&
> -ctx_config.minor_version >= 2))) {
> +if (mesa_api == API_OPENGL_COMPAT &&
> +((ctx_config.major_version > 3) || (ctx_config.major_version == 3 &&
> +ctx_config.minor_version >= 2)) 
> &&
> +!((ctx_config.major_version * 10 + ctx_config.minor_version) <=
> +  screen->max_gl_compat_version)) {

nit: what about using (a * 10 + b  > c) to make it easier to read than
!(a * 10 + b <= c) ?

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

Re: [Mesa-dev] [PATCH] radv/winsys: allow to submit up to 4 IBs for chips without chaining

[Grazvydas Ignotas]

On Fri, Apr 20, 2018 at 3:21 PM, Samuel Pitoiset
 wrote:
> The SI family doesn't support chaining which means the maximum
> size in dwords per CS is limited. When that limit was reached
> we failed to submit the CS and the application crashed.
>
> This patch allows to submit up to 4 IBs which is currently the
> limit, but recent amdgpu supports more than that.
>
> Please note that we can reach the limit of 4 IBs per submit
> but currently we can't improve that. The only solution is to
> upgrade libdrm. That will be improved later but for now this
> should fix crashes on SI or when using RADV_DEBUG=noibs.
>
> Fixes: 36cb5508e89 ("radv/winsys: Fail early on overgrown cs.")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105775
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 218 ++
>  1 file changed, 168 insertions(+), 50 deletions(-)
>
> diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c 
> b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
> index c4b2232ce9e..17e6d8ba2b6 100644
> --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
> +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
> @@ -68,6 +68,10 @@ struct radv_amdgpu_cs {
> struct radeon_winsys_bo **virtual_buffers;
> uint8_t *virtual_buffer_priorities;
> int *virtual_buffer_hash_table;
> +
> +   /* For chips that don't support chaining. */
> +   struct radeon_winsys_cs *old_cs_buffers;
> +   unsignednum_old_cs_buffers;
>  };
>
>  static inline struct radv_amdgpu_cs *
> @@ -201,6 +205,12 @@ static void radv_amdgpu_cs_destroy(struct 
> radeon_winsys_cs *rcs)
> for (unsigned i = 0; i < cs->num_old_ib_buffers; ++i)
> cs->ws->base.buffer_destroy(cs->old_ib_buffers[i]);
>
> +   for (unsigned i = 0; i < cs->num_old_cs_buffers; ++i) {
> +   struct radeon_winsys_cs *rcs = >old_cs_buffers[i];
> +   free(rcs->buf);
> +   }
> +
> +   free(cs->old_cs_buffers);
> free(cs->old_ib_buffers);
> free(cs->virtual_buffers);
> free(cs->virtual_buffer_priorities);
> @@ -286,9 +296,46 @@ static void radv_amdgpu_cs_grow(struct radeon_winsys_cs 
> *_cs, size_t min_size)
> /* The total ib size cannot exceed limit_dws dwords. */
> if (ib_dws > limit_dws)
> {
> -   cs->failed = true;
> +   /* The maximum size in dwords has been reached,
> +* try to allocate a new one.
> +*/
> +   if (cs->num_old_cs_buffers + 1 >= 
> AMDGPU_CS_MAX_IBS_PER_SUBMIT) {
> +   /* TODO: Allow to submit more than 4 IBs. */
> +   fprintf(stderr, "amdgpu: Maximum number of 
> IBs "
> +   "per submit reached.\n");
> +   cs->failed = true;
> +   cs->base.cdw = 0;
> +   return;
> +   }
> +
> +   cs->old_cs_buffers =
> +   realloc(cs->old_cs_buffers,
> +   (cs->num_old_cs_buffers + 1) * 
> sizeof(*cs->old_cs_buffers));
> +   if (!cs->old_cs_buffers) {

leaking previous cs->old_cs_buffers memory here.

> +   cs->failed = true;
> +   cs->base.cdw = 0;
> +   return;
> +   }
> +
> +   /* Store the current one for submitting it later. */
> +   cs->old_cs_buffers[cs->num_old_cs_buffers].cdw = 
> cs->base.cdw;
> +   cs->old_cs_buffers[cs->num_old_cs_buffers].max_dw = 
> cs->base.max_dw;
> +   cs->old_cs_buffers[cs->num_old_cs_buffers].buf = 
> cs->base.buf;
> +   cs->num_old_cs_buffers++;
> +
> +   /* Reset the cs, it will be re-allocated below. */
> cs->base.cdw = 0;
> -   return;
> +   cs->base.buf = NULL;
> +
> +   /* Re-compute the number of dwords to allocate. */
> +   ib_dws = MAX2(cs->base.cdw + min_size,
> + MIN2(cs->base.max_dw * 2, limit_dws));
> +   if (ib_dws > limit_dws) {
> +   fprintf(stderr, "amdgpu: Too high number of "
> +   "dwords to allocate\n");
> +   cs->failed = true;
> +   return;
> +   }
> }
>
> uint32_t *new_buf = realloc(cs->base.buf, ib_dws * 4);
> @@ -400,6 +447,15 

Re: [Mesa-dev] [PATCH 4/4] radv: implement VK_AMD_shader_core_properties

[Grazvydas Ignotas]

On Fri, Apr 6, 2018 at 3:28 PM, Samuel Pitoiset
 wrote:
> Simple extension that only returns information for AMD hw.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/amd/vulkan/radv_device.c  | 71 
> +++
>  src/amd/vulkan/radv_extensions.py |  1 +
>  2 files changed, 72 insertions(+)
>
> diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
> index 41f8242754..fba0b5c586 100644
> --- a/src/amd/vulkan/radv_device.c
> +++ b/src/amd/vulkan/radv_device.c
> @@ -888,6 +888,39 @@ void radv_GetPhysicalDeviceProperties(
> memcpy(pProperties->pipelineCacheUUID, pdevice->cache_uuid, 
> VK_UUID_SIZE);
>  }
>
> +static uint32_t
> +radv_get_max_cu_per_sh(struct radv_physical_device *device)
> +{
> +   /* This should be queried from the KMD, like the number of SEs. */
> +   switch (device->rad_info.family) {
> +   case CHIP_TAHITI:
> +   return 8;
> +   case CHIP_HAINAN:
> +   return 5;
> +   case CHIP_BONAIRE:
> +   return 7;
> +   case CHIP_HAWAII:
> +   return 11;
> +   case CHIP_ICELAND:
> +   return 6;
> +   case CHIP_CARRIZO:
> +   return 8;
> +   case CHIP_TONGA:
> +   return 8;
> +   case CHIP_FIJI:
> +   return 16;
> +   case CHIP_STONEY:
> +   return 3;
> +   case CHIP_VEGA10:
> +   return 16;
> +   case CHIP_RAVEN:
> +   return 11;
> +   default:
> +   fprintf(stderr, "Number of CUs per SH unknown!\n");
> +   return 0;
> +   }
> +}
> +
>  void radv_GetPhysicalDeviceProperties2(
> VkPhysicalDevicephysicalDevice,
> VkPhysicalDeviceProperties2KHR *pProperties)
> @@ -961,6 +994,44 @@ void radv_GetPhysicalDeviceProperties2(
> properties->filterMinmaxSingleComponentFormats = true;
> break;
> }
> +   case 
> VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_CORE_PROPERTIES_AMD: {
> +   VkPhysicalDeviceShaderCorePropertiesAMD *properties =
> +   (VkPhysicalDeviceShaderCorePropertiesAMD 
> *)ext;
> +
> +   /* Shader engines. */
> +   properties->shaderEngineCount =
> +   pdevice->rad_info.max_se;
> +   properties->shaderArraysPerEngineCount =
> +   pdevice->rad_info.max_sh_per_se;
> +   properties->computeUnitsPerShaderArray =
> +   radv_get_max_cu_per_sh(pdevice);

Maybe
pdevice->rad_info.num_good_compute_units / (pdevice->rad_info.max_se *
pdevice->rad_info.max_sh_per_se);
would do the trick?

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

[Mesa-dev] [PATCH 1/2] radv: fix stale comment in generated vk_format_table.c

[Grazvydas Ignotas]

It seems to be a leftover from u_format_table.py.
---
 src/amd/vulkan/vk_format_table.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/amd/vulkan/vk_format_table.py 
b/src/amd/vulkan/vk_format_table.py
index 36352b1..c33a4ce 100644
--- a/src/amd/vulkan/vk_format_table.py
+++ b/src/amd/vulkan/vk_format_table.py
@@ -84,11 +84,11 @@ def print_channels(format, func):
 print '#else'
 func(format.le_channels, format.le_swizzles)
 print '#endif'
 
 def write_format_table(formats):
-print '/* This file is autogenerated by u_format_table.py from 
u_format.csv. Do not edit directly. */'
+print '/* This file is autogenerated by vk_format_table.py from 
vk_format_layout.csv. Do not edit directly. */'
 print
 # This will print the copyright message on the top of this file
 print CopyRight.strip()
 print
 print '#include "stdbool.h"'
-- 
2.7.4

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

[Mesa-dev] [PATCH 2/2] radv: make vk_format_description structures static

[Grazvydas Ignotas]

No need to bother the linker about them.
---
 src/amd/vulkan/vk_format_table.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/amd/vulkan/vk_format_table.py 
b/src/amd/vulkan/vk_format_table.py
index c33a4ce..cd1af62 100644
--- a/src/amd/vulkan/vk_format_table.py
+++ b/src/amd/vulkan/vk_format_table.py
@@ -123,11 +123,11 @@ def write_format_table(formats):
 comment = 'ignored'
 print "  %s%s\t/* %s */" % (swizzle_map[swizzle], sep, comment)
 print "   },"
 
 for format in formats:
-print 'const struct vk_format_description'
+print 'static const struct vk_format_description'
 print 'vk_format_%s_description = {' % (format.short_name(),)
 print "   %s," % (format.name,)
 print "   \"%s\"," % (format.name,)
 print "   \"%s\"," % (format.short_name(),)
 print "   {%u, %u, %u},\t/* block */" % (format.block_width, 
format.block_height, format.block_size())
-- 
2.7.4

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

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

[Grazvydas Ignotas]

Seems to fix dxvk, nice! Also tested DOOM which still works.
Tested-by: Grazvydas Ignotas <nota...@gmail.com>

On Thu, Feb 22, 2018 at 6:57 PM, James Legg <jl...@feralinteractive.com> wrote:
> When transitioning to an htile compressed depth format, Set the full
> depth range, so later rasterization can pass HiZ. Previously, for depth
> only formats, the depth range was set to 0 to 0. This caused unwanted
> HiZ rejections with a VK_FORMAT_D16_UNORM depth buffer
> (VK_FORMAT_D32_SFLOAT was not affected somehow).
>
> These values are derived from PAL [0], since I can't find the
> specification describing the htile values.
>
> Fixes 5158603182fe7435: radv: Use correct HTILE expanded words.

Please put this as "Fixes: ..." below along with other tags.

>
> [0] 
> https://github.com/GPUOpen-Drivers/pal/blob/5cba4ecbda9452773f59692f5915301e7db4a183/src/core/hw/gfxip/gfx9/gfx9MaskRam.cpp#L1500
>
> CC: Dave Airlie <airl...@redhat.com>
> CC: Bas Nieuwenhuizen <b...@basnieuwenhuizen.nl>
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/amd/vulkan/radv_cmd_buffer.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
> b/src/amd/vulkan/radv_cmd_buffer.c
> index 8a384b114c..2b41baea3d 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -3440,8 +3440,8 @@ void radv_CmdEndRenderPass(
>
>  /*
>   * For HTILE we have the following interesting clear words:
> - *   0x030f: Uncompressed for depth+stencil HTILE.
> - *   0x000f: Uncompressed for depth only HTILE.
> + *   0xf30f: Uncompressed, full depth range, for depth+stencil HTILE
> + *   0xfffc000f: Uncompressed, full depth range, for depth only HTILE.
>   *   0xfff0: Clear depth to 1.0
>   *   0x: Clear depth to 0.0
>   */
> @@ -3489,7 +3489,7 @@ static void radv_handle_depth_image_transition(struct 
> radv_cmd_buffer *cmd_buffe
> radv_initialize_htile(cmd_buffer, image, range, 0);
> } else if (!radv_layout_is_htile_compressed(image, src_layout, 
> src_queue_mask) &&
>radv_layout_is_htile_compressed(image, dst_layout, 
> dst_queue_mask)) {
> -   uint32_t clear_value = vk_format_is_stencil(image->vk_format) 
> ? 0x30f : 0xf;
> +   uint32_t clear_value = vk_format_is_stencil(image->vk_format) 
> ? 0xf30f : 0xfffc000f;
> radv_initialize_htile(cmd_buffer, image, range, clear_value);
> } else if (radv_layout_is_htile_compressed(image, src_layout, 
> src_queue_mask) &&
>!radv_layout_is_htile_compressed(image, dst_layout, 
> dst_queue_mask)) {
> --
> 2.14.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] intel/compiler: fix 64bit value prints on 32bit

[Grazvydas Ignotas]

ping

On Sun, Feb 4, 2018 at 12:19 AM, Grazvydas Ignotas <nota...@gmail.com> wrote:
> Fix the following:
> warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but
> argument 3 has type ‘uint64_t {aka long long unsigned int}.
> ---
>  src/intel/compiler/brw_disasm.c | 4 ++--
>  src/intel/compiler/brw_nir_analyze_ubo_ranges.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c
> index 1a94ed3..429ed78 100644
> --- a/src/intel/compiler/brw_disasm.c
> +++ b/src/intel/compiler/brw_disasm.c
> @@ -1247,14 +1247,14 @@ static int
>  imm(FILE *file, const struct gen_device_info *devinfo, enum brw_reg_type 
> type,
>  const brw_inst *inst)
>  {
> switch (type) {
> case BRW_REGISTER_TYPE_UQ:
> -  format(file, "0x%016lxUQ", brw_inst_imm_uq(devinfo, inst));
> +  format(file, "0x%016"PRIx64"UQ", brw_inst_imm_uq(devinfo, inst));
>break;
> case BRW_REGISTER_TYPE_Q:
> -  format(file, "%ldQ", brw_inst_imm_uq(devinfo, inst));
> +  format(file, "%"PRId64"Q", brw_inst_imm_uq(devinfo, inst));
>break;
> case BRW_REGISTER_TYPE_UD:
>format(file, "0x%08xUD", brw_inst_imm_ud(devinfo, inst));
>break;
> case BRW_REGISTER_TYPE_D:
> diff --git a/src/intel/compiler/brw_nir_analyze_ubo_ranges.c 
> b/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
> index 32fbf8f..d58fe3d 100644
> --- a/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
> +++ b/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
> @@ -156,11 +156,11 @@ print_ubo_entry(FILE *file,
>  struct ubo_analysis_state *state)
>  {
> struct ubo_block_info *info = get_block_info(state, entry->range.block);
>
> fprintf(file,
> -   "block %2d, start %2d, length %2d, bits = %zx, "
> +   "block %2d, start %2d, length %2d, bits = %"PRIx64", "
> "benefit %2d, cost %2d, score = %2d\n",
> entry->range.block, entry->range.start, entry->range.length,
> info->offsets, entry->benefit, entry->range.length, score(entry));
>  }
>
> --
> 2.7.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: perf: ensure reading config IDs from sysfs isn't interrupted

[Grazvydas Ignotas]

Is this really sufficient? From what I read in the manpage, you can
get an incomplete read instead when a signal arrives.

Gražvydas

On Fri, Feb 9, 2018 at 12:03 PM, Lionel Landwerlin
 wrote:
> Fixes: 458468c136e "i965: Expose OA counters via INTEL_performance_query"
> Signed-off-by: Lionel Landwerlin 
> Cc: "18.0" 
> ---
>  src/mesa/drivers/dri/i965/brw_performance_query.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_performance_query.c 
> b/src/mesa/drivers/dri/i965/brw_performance_query.c
> index c0bb4442bec..613e61653fe 100644
> --- a/src/mesa/drivers/dri/i965/brw_performance_query.c
> +++ b/src/mesa/drivers/dri/i965/brw_performance_query.c
> @@ -1720,7 +1720,8 @@ read_file_uint64(const char *file, uint64_t *val)
>  fd = open(file, 0);
>  if (fd < 0)
> return false;
> -n = read(fd, buf, sizeof (buf) - 1);
> +while ((n = read(fd, buf, sizeof (buf) - 1)) < 0 &&
> +   errno == EINTR);
>  close(fd);
>  if (n < 0)
> return false;
> --
> 2.16.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] radeonsi: set indent_size in .editorconfig

[Grazvydas Ignotas]

On Tue, Feb 6, 2018 at 1:37 PM, Eric Engestrom
<eric.engest...@imgtec.com> wrote:
> On Sunday, 2018-02-04 00:19:33 +0200, Grazvydas Ignotas wrote:
>> At least with vim, this is needed to actually get tab instead of
>> 3 spaces after hitting the tab key.
>
> Are you sure?

For my copy of vim (7.4.1689) yes. Not sure what version of
editorconfig plugin I have, can't find a version anywhere.

I don't know about other editors, src/amd/ has the same settings and
nobody complained or changed them.

>
> The doc [1] says:
>
>> indent_size:
>> [...]
>> When set to tab, the value of tab_width (if specified) will be used.
>
>> tab_width:
>> a whole number defining the number of columns used to represent a tab
>> character. This defaults to the value of indent_size and doesn't
>> usually need to be specified.
>
> This sounds to me like `indent_size=tab` means that you want to indent
> with `tab_width` spaces?

That's not how I read it, to me it's the user's preferred tab width
that they configured outside of mesa.

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

[Mesa-dev] [PATCH] gallium/hud: update some query functions

[Grazvydas Ignotas]

It seems these were missed when struct pipe_context * argument was
added to hud_graph::query_new_value.

Fixes: 3132afdf4c "gallium/hud: pass pipe_context explicitly to most functions"
---
 src/gallium/auxiliary/hud/hud_cpufreq.c  | 2 +-
 src/gallium/auxiliary/hud/hud_diskstat.c | 2 +-
 src/gallium/auxiliary/hud/hud_nic.c  | 2 +-
 src/gallium/auxiliary/hud/hud_sensors_temp.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/gallium/auxiliary/hud/hud_cpufreq.c 
b/src/gallium/auxiliary/hud/hud_cpufreq.c
index f5b7c0f2..78a6607 100644
--- a/src/gallium/auxiliary/hud/hud_cpufreq.c
+++ b/src/gallium/auxiliary/hud/hud_cpufreq.c
@@ -89,11 +89,11 @@ get_file_value(const char *fn, uint64_t *KHz)
 
return ret;
 }
 
 static void
-query_cfi_load(struct hud_graph *gr)
+query_cfi_load(struct hud_graph *gr, struct pipe_context *pipe)
 {
struct cpufreq_info *cfi = gr->query_data;
 
uint64_t now = os_time_get();
if (cfi->last_time) {
diff --git a/src/gallium/auxiliary/hud/hud_diskstat.c 
b/src/gallium/auxiliary/hud/hud_diskstat.c
index 44174d6..7eaaf35 100644
--- a/src/gallium/auxiliary/hud/hud_diskstat.c
+++ b/src/gallium/auxiliary/hud/hud_diskstat.c
@@ -115,11 +115,11 @@ get_file_values(const char *fn, struct stat_s *s)
 
return ret;
 }
 
 static void
-query_dsi_load(struct hud_graph *gr)
+query_dsi_load(struct hud_graph *gr, struct pipe_context *pipe)
 {
/* The framework calls us periodically, compensate for the
 * calling interval accordingly when reporting per second.
 */
struct diskstat_info *dsi = gr->query_data;
diff --git a/src/gallium/auxiliary/hud/hud_nic.c 
b/src/gallium/auxiliary/hud/hud_nic.c
index 1de5705..b6c0d9e 100644
--- a/src/gallium/auxiliary/hud/hud_nic.c
+++ b/src/gallium/auxiliary/hud/hud_nic.c
@@ -169,11 +169,11 @@ query_nic_rssi(const struct nic_info *nic, uint64_t 
*leveldBm)
 
close(sockfd);
 }
 
 static void
-query_nic_load(struct hud_graph *gr)
+query_nic_load(struct hud_graph *gr, struct pipe_context *pipe)
 {
/* The framework calls us at a regular but indefined period,
 * not once per second, compensate the statistics accordingly.
 */
 
diff --git a/src/gallium/auxiliary/hud/hud_sensors_temp.c 
b/src/gallium/auxiliary/hud/hud_sensors_temp.c
index 4d3a11b..c26e7b9 100644
--- a/src/gallium/auxiliary/hud/hud_sensors_temp.c
+++ b/src/gallium/auxiliary/hud/hud_sensors_temp.c
@@ -152,11 +152,11 @@ find_sti_by_name(const char *n, unsigned int mode)
}
return 0;
 }
 
 static void
-query_sti_load(struct hud_graph *gr)
+query_sti_load(struct hud_graph *gr, struct pipe_context *pipe)
 {
struct sensors_temp_info *sti = gr->query_data;
uint64_t now = os_time_get();
 
if (sti->last_time) {
-- 
2.7.4

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

[Mesa-dev] [PATCH] radeonsi: set indent_size in .editorconfig

[Grazvydas Ignotas]

At least with vim, this is needed to actually get tab instead of
3 spaces after hitting the tab key.
---
 src/gallium/drivers/radeonsi/.editorconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/gallium/drivers/radeonsi/.editorconfig 
b/src/gallium/drivers/radeonsi/.editorconfig
index cc8e11f..21a3c7d 100644
--- a/src/gallium/drivers/radeonsi/.editorconfig
+++ b/src/gallium/drivers/radeonsi/.editorconfig
@@ -1,2 +1,3 @@
 [*.{c,h}]
 indent_style = tab
+indent_size = tab
-- 
2.7.4

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

[Mesa-dev] [PATCH] intel/compiler: fix 64bit value prints on 32bit

[Grazvydas Ignotas]

Fix the following:
warning: format ‘%lx’ expects argument of type ‘long unsigned int’, but
argument 3 has type ‘uint64_t {aka long long unsigned int}.
---
 src/intel/compiler/brw_disasm.c | 4 ++--
 src/intel/compiler/brw_nir_analyze_ubo_ranges.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c
index 1a94ed3..429ed78 100644
--- a/src/intel/compiler/brw_disasm.c
+++ b/src/intel/compiler/brw_disasm.c
@@ -1247,14 +1247,14 @@ static int
 imm(FILE *file, const struct gen_device_info *devinfo, enum brw_reg_type type,
 const brw_inst *inst)
 {
switch (type) {
case BRW_REGISTER_TYPE_UQ:
-  format(file, "0x%016lxUQ", brw_inst_imm_uq(devinfo, inst));
+  format(file, "0x%016"PRIx64"UQ", brw_inst_imm_uq(devinfo, inst));
   break;
case BRW_REGISTER_TYPE_Q:
-  format(file, "%ldQ", brw_inst_imm_uq(devinfo, inst));
+  format(file, "%"PRId64"Q", brw_inst_imm_uq(devinfo, inst));
   break;
case BRW_REGISTER_TYPE_UD:
   format(file, "0x%08xUD", brw_inst_imm_ud(devinfo, inst));
   break;
case BRW_REGISTER_TYPE_D:
diff --git a/src/intel/compiler/brw_nir_analyze_ubo_ranges.c 
b/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
index 32fbf8f..d58fe3d 100644
--- a/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
+++ b/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
@@ -156,11 +156,11 @@ print_ubo_entry(FILE *file,
 struct ubo_analysis_state *state)
 {
struct ubo_block_info *info = get_block_info(state, entry->range.block);
 
fprintf(file,
-   "block %2d, start %2d, length %2d, bits = %zx, "
+   "block %2d, start %2d, length %2d, bits = %"PRIx64", "
"benefit %2d, cost %2d, score = %2d\n",
entry->range.block, entry->range.start, entry->range.length,
info->offsets, entry->benefit, entry->range.length, score(entry));
 }
 
-- 
2.7.4

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

[Mesa-dev] [PATCH] radeonsi: avoid int-to-pointer-cast warnings on 32bit

[Grazvydas Ignotas]

I hope the actual dropping of MSB is ok, but that's what's already
happened before this change.
---
 src/gallium/drivers/radeonsi/si_descriptors.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
b/src/gallium/drivers/radeonsi/si_descriptors.c
index 17115e1..009e803 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -2263,11 +2263,12 @@ static uint64_t si_create_texture_handle(struct 
pipe_context *ctx,
return 0;
}
 
handle = tex_handle->desc_slot;
 
-   if (!_mesa_hash_table_insert(sctx->tex_handles, (void *)handle,
+   if (!_mesa_hash_table_insert(sctx->tex_handles,
+(void *)(uintptr_t)handle,
 tex_handle)) {
FREE(tex_handle);
return 0;
}
 
@@ -2282,11 +2283,12 @@ static void si_delete_texture_handle(struct 
pipe_context *ctx, uint64_t handle)
 {
struct si_context *sctx = (struct si_context *)ctx;
struct si_texture_handle *tex_handle;
struct hash_entry *entry;
 
-   entry = _mesa_hash_table_search(sctx->tex_handles, (void *)handle);
+   entry = _mesa_hash_table_search(sctx->tex_handles,
+   (void *)(uintptr_t)handle);
if (!entry)
return;
 
tex_handle = (struct si_texture_handle *)entry->data;
 
@@ -2304,11 +2306,12 @@ static void si_make_texture_handle_resident(struct 
pipe_context *ctx,
struct si_context *sctx = (struct si_context *)ctx;
struct si_texture_handle *tex_handle;
struct si_sampler_view *sview;
struct hash_entry *entry;
 
-   entry = _mesa_hash_table_search(sctx->tex_handles, (void *)handle);
+   entry = _mesa_hash_table_search(sctx->tex_handles,
+   (void *)(uintptr_t)handle);
if (!entry)
return;
 
tex_handle = (struct si_texture_handle *)entry->data;
sview = (struct si_sampler_view *)tex_handle->view;
@@ -2406,11 +2409,12 @@ static uint64_t si_create_image_handle(struct 
pipe_context *ctx,
return 0;
}
 
handle = img_handle->desc_slot;
 
-   if (!_mesa_hash_table_insert(sctx->img_handles, (void *)handle,
+   if (!_mesa_hash_table_insert(sctx->img_handles,
+(void *)(uintptr_t)handle,
 img_handle)) {
FREE(img_handle);
return 0;
}
 
@@ -2425,11 +2429,12 @@ static void si_delete_image_handle(struct pipe_context 
*ctx, uint64_t handle)
 {
struct si_context *sctx = (struct si_context *)ctx;
struct si_image_handle *img_handle;
struct hash_entry *entry;
 
-   entry = _mesa_hash_table_search(sctx->img_handles, (void *)handle);
+   entry = _mesa_hash_table_search(sctx->img_handles,
+   (void *)(uintptr_t)handle);
if (!entry)
return;
 
img_handle = (struct si_image_handle *)entry->data;
 
@@ -2446,11 +2451,12 @@ static void si_make_image_handle_resident(struct 
pipe_context *ctx,
struct si_image_handle *img_handle;
struct pipe_image_view *view;
struct r600_resource *res;
struct hash_entry *entry;
 
-   entry = _mesa_hash_table_search(sctx->img_handles, (void *)handle);
+   entry = _mesa_hash_table_search(sctx->img_handles,
+   (void *)(uintptr_t)handle);
if (!entry)
return;
 
img_handle = (struct si_image_handle *)entry->data;
view = _handle->view;
-- 
2.7.4

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

[Mesa-dev] [PATCH] anv: correct a duplicate check in an assert

[Grazvydas Ignotas]

Looks like checking both sources was intended, instead of the first one
twice. Found with Coccinelle, coccinellery/xand/xand.cocci semantic patch.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/intel/vulkan/anv_nir_apply_pipeline_layout.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c 
b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c
index 978a8a5..6775f9b 100644
--- a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c
+++ b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c
@@ -145,11 +145,11 @@ lower_res_reindex_intrinsic(nir_intrinsic_instr *intrin,
 
/* For us, the resource indices are just indices into the binding table and
 * array elements are sequential.  A resource_reindex just turns into an
 * add of the two indices.
 */
-   assert(intrin->src[0].is_ssa && intrin->src[0].is_ssa);
+   assert(intrin->src[0].is_ssa && intrin->src[1].is_ssa);
nir_ssa_def *new_index = nir_iadd(b, intrin->src[0].ssa,
 intrin->src[1].ssa);
 
assert(intrin->dest.is_ssa);
nir_ssa_def_rewrite_uses(>dest.ssa, nir_src_for_ssa(new_index));
-- 
2.7.4

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

[Mesa-dev] [PATCH 1/2] svga: fix context alloc error handling

[Grazvydas Ignotas]

'cleanup' path is dereferencing 'svga' a lot, 'done' is a better choice.
Found by Coccinelle.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/gallium/drivers/svga/svga_context.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/svga/svga_context.c 
b/src/gallium/drivers/svga/svga_context.c
index af16a99..7b3e9e8 100644
--- a/src/gallium/drivers/svga/svga_context.c
+++ b/src/gallium/drivers/svga/svga_context.c
@@ -132,11 +132,11 @@ svga_context_create(struct pipe_screen *screen, void 
*priv, unsigned flags)
 
SVGA_STATS_TIME_PUSH(svgascreen->sws, SVGA_STATS_TIME_CREATECONTEXT);
 
svga = CALLOC_STRUCT(svga_context);
if (!svga)
-  goto cleanup;
+  goto done;
 
LIST_INITHEAD(>dirty_buffers);
 
svga->pipe.screen = screen;
svga->pipe.priv = priv;
-- 
2.7.4

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

[Mesa-dev] [PATCH 2/2] winsys/svga: check correct member after create

[Grazvydas Ignotas]

.mob_fenced was already checked, probably a copy-paste bug.
Found by Coccinelle.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/gallium/winsys/svga/drm/vmw_screen_pools.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/winsys/svga/drm/vmw_screen_pools.c 
b/src/gallium/winsys/svga/drm/vmw_screen_pools.c
index 48c95e5..29cdc69 100644
--- a/src/gallium/winsys/svga/drm/vmw_screen_pools.c
+++ b/src/gallium/winsys/svga/drm/vmw_screen_pools.c
@@ -148,11 +148,11 @@ vmw_mob_pools_init(struct vmw_winsys_screen *vws)
   goto out_no_mob_shader_slab;
 
vws->pools.mob_shader_slab_fenced =
   simple_fenced_bufmgr_create(vws->pools.mob_shader_slab,
  vws->fence_ops);
-   if(!vws->pools.mob_fenced)
+   if(!vws->pools.mob_shader_slab_fenced)
   goto out_no_mob_shader_slab_fenced;
 
return TRUE;
 
  out_no_mob_shader_slab_fenced:
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 2/2] st/vdpau: release held lock in error path

[Grazvydas Ignotas]

Anyone cares about vdpau?

Gražvydas

On Tue, Jan 16, 2018 at 12:03 AM, Grazvydas Ignotas <nota...@gmail.com> wrote:
> Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
> ---
>  src/gallium/state_trackers/vdpau/surface.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/state_trackers/vdpau/surface.c 
> b/src/gallium/state_trackers/vdpau/surface.c
> index c678eb7..012d303 100644
> --- a/src/gallium/state_trackers/vdpau/surface.c
> +++ b/src/gallium/state_trackers/vdpau/surface.c
> @@ -367,12 +367,14 @@ vlVdpVideoSurfacePutBitsYCbCr(VdpVideoSurface surface,
>
> if (pformat != p_surf->video_buffer->buffer_format) {
>if (pformat == PIPE_FORMAT_YV12 &&
>p_surf->video_buffer->buffer_format == PIPE_FORMAT_NV12)
>   conversion = CONVERSION_YV12_TO_NV12;
> -  else
> +  else {
> + mtx_unlock(_surf->device->mutex);
>   return VDP_STATUS_NO_IMPLEMENTATION;
> +  }
> }
>
> sampler_views = 
> p_surf->video_buffer->get_sampler_view_planes(p_surf->video_buffer);
> if (!sampler_views) {
>mtx_unlock(_surf->device->mutex);
> --
> 2.7.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] st/va: release held locks in error paths

[Grazvydas Ignotas]

On Tue, Jan 16, 2018 at 10:15 AM, Christian König
<ckoenig.leichtzumer...@gmail.com> wrote:
> Reviewed-by: Christian König <christian.koe...@amd.com>
>
> Do you have commit right by now or should Leo or I commit that for you?

Yes I do.

>
> Thanks for the help,
> Christian.
>
>
> Am 15.01.2018 um 23:03 schrieb Grazvydas Ignotas:
>>
>> Found with the help of following Coccinelle semantic patch:
>> // 
>> @@
>> expression E;
>> @@
>>
>>\(pthread_mutex_lock\|mtx_lock\|simple_mtx_lock\)(E)
>>...
>> (
>>\(pthread_mutex_unlock\|mtx_unlock\|simple_mtx_unlock\)(E);
>>...
>>return ...;
>> |
>> + maybe need_unlock(E);
>>return ...;
>> )
>> // 
>>
>> Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
>> ---
>>   src/gallium/state_trackers/va/config.c  | 4 +++-
>>   src/gallium/state_trackers/va/image.c   | 4 +++-
>>   src/gallium/state_trackers/va/picture.c | 4 +++-
>>   3 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/va/config.c
>> b/src/gallium/state_trackers/va/config.c
>> index 25043d6..7bc031a 100644
>> --- a/src/gallium/state_trackers/va/config.c
>> +++ b/src/gallium/state_trackers/va/config.c
>> @@ -306,12 +306,14 @@ vlVaDestroyConfig(VADriverContextP ctx, VAConfigID
>> config_id)
>> return VA_STATUS_ERROR_INVALID_CONTEXT;
>>mtx_lock(>mutex);
>>  config = handle_table_get(drv->htab, config_id);
>>   -   if (!config)
>> +   if (!config) {
>> +  mtx_unlock(>mutex);
>> return VA_STATUS_ERROR_INVALID_CONFIG;
>> +   }
>>FREE(config);
>>  handle_table_remove(drv->htab, config_id);
>>  mtx_unlock(>mutex);
>>   diff --git a/src/gallium/state_trackers/va/image.c
>> b/src/gallium/state_trackers/va/image.c
>> index 86ae868..3f892c9 100644
>> --- a/src/gallium/state_trackers/va/image.c
>> +++ b/src/gallium/state_trackers/va/image.c
>> @@ -546,12 +546,14 @@ vlVaPutImage(VADriverContextP ctx, VASurfaceID
>> surface, VAImageID image,
>> tex,
>> 0,
>> PIPE_TRANSFER_WRITE |
>> PIPE_TRANSFER_DISCARD_RANGE,
>> _box, );
>> -if (map == NULL)
>> +if (map == NULL) {
>> +   mtx_unlock(>mutex);
>>  return VA_STATUS_ERROR_OPERATION_FAILED;
>> +}
>> u_copy_nv12_from_yv12((const void * const*) data, pitches,
>> i, j,
>> transfer->stride, tex->array_size,
>> map, dst_box.width, dst_box.height);
>>   pipe_transfer_unmap(drv->pipe, transfer);
>> diff --git a/src/gallium/state_trackers/va/picture.c
>> b/src/gallium/state_trackers/va/picture.c
>> index 8951573..cfcf986 100644
>> --- a/src/gallium/state_trackers/va/picture.c
>> +++ b/src/gallium/state_trackers/va/picture.c
>> @@ -675,13 +675,15 @@ vlVaEndPicture(VADriverContextP ctx, VAContextID
>> context_id)
>>   dst_rect.x1 = src_rect.x1 = surf->templat.width;
>>   dst_rect.y1 = src_rect.y1 = surf->templat.height;
>>   vl_compositor_yuv_deint_full(>cstate, >compositor,
>>old_buf, surf->buffer,
>>_rect, _rect,
>> VL_COMPOSITOR_WEAVE);
>> - } else
>> + } else {
>>   /* Can't convert from progressive to interlaced yet */
>> +mtx_unlock(>mutex);
>>   return VA_STATUS_ERROR_INVALID_SURFACE;
>> + }
>> }
>>   old_buf->destroy(old_buf);
>> context->target = surf->buffer;
>>  }
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] st/vdpau: release held lock in error path

[Grazvydas Ignotas]

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/gallium/state_trackers/vdpau/surface.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/gallium/state_trackers/vdpau/surface.c 
b/src/gallium/state_trackers/vdpau/surface.c
index c678eb7..012d303 100644
--- a/src/gallium/state_trackers/vdpau/surface.c
+++ b/src/gallium/state_trackers/vdpau/surface.c
@@ -367,12 +367,14 @@ vlVdpVideoSurfacePutBitsYCbCr(VdpVideoSurface surface,
 
if (pformat != p_surf->video_buffer->buffer_format) {
   if (pformat == PIPE_FORMAT_YV12 &&
   p_surf->video_buffer->buffer_format == PIPE_FORMAT_NV12)
  conversion = CONVERSION_YV12_TO_NV12;
-  else
+  else {
+ mtx_unlock(_surf->device->mutex);
  return VDP_STATUS_NO_IMPLEMENTATION;
+  }
}
 
sampler_views = 
p_surf->video_buffer->get_sampler_view_planes(p_surf->video_buffer);
if (!sampler_views) {
   mtx_unlock(_surf->device->mutex);
-- 
2.7.4

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

[Mesa-dev] [PATCH 1/2] st/va: release held locks in error paths

[Grazvydas Ignotas]

Found with the help of following Coccinelle semantic patch:
// 
@@
expression E;
@@

  \(pthread_mutex_lock\|mtx_lock\|simple_mtx_lock\)(E)
  ...
(
  \(pthread_mutex_unlock\|mtx_unlock\|simple_mtx_unlock\)(E);
  ...
  return ...;
|
+ maybe need_unlock(E);
  return ...;
)
// 

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/gallium/state_trackers/va/config.c  | 4 +++-
 src/gallium/state_trackers/va/image.c   | 4 +++-
 src/gallium/state_trackers/va/picture.c | 4 +++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/gallium/state_trackers/va/config.c 
b/src/gallium/state_trackers/va/config.c
index 25043d6..7bc031a 100644
--- a/src/gallium/state_trackers/va/config.c
+++ b/src/gallium/state_trackers/va/config.c
@@ -306,12 +306,14 @@ vlVaDestroyConfig(VADriverContextP ctx, VAConfigID 
config_id)
   return VA_STATUS_ERROR_INVALID_CONTEXT;
 
mtx_lock(>mutex);
config = handle_table_get(drv->htab, config_id);
 
-   if (!config)
+   if (!config) {
+  mtx_unlock(>mutex);
   return VA_STATUS_ERROR_INVALID_CONFIG;
+   }
 
FREE(config);
handle_table_remove(drv->htab, config_id);
mtx_unlock(>mutex);
 
diff --git a/src/gallium/state_trackers/va/image.c 
b/src/gallium/state_trackers/va/image.c
index 86ae868..3f892c9 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -546,12 +546,14 @@ vlVaPutImage(VADriverContextP ctx, VASurfaceID surface, 
VAImageID image,
   tex,
   0,
   PIPE_TRANSFER_WRITE |
   PIPE_TRANSFER_DISCARD_RANGE,
   _box, );
-if (map == NULL)
+if (map == NULL) {
+   mtx_unlock(>mutex);
return VA_STATUS_ERROR_OPERATION_FAILED;
+}
 
 u_copy_nv12_from_yv12((const void * const*) data, pitches, i, j,
   transfer->stride, tex->array_size,
   map, dst_box.width, dst_box.height);
 pipe_transfer_unmap(drv->pipe, transfer);
diff --git a/src/gallium/state_trackers/va/picture.c 
b/src/gallium/state_trackers/va/picture.c
index 8951573..cfcf986 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -675,13 +675,15 @@ vlVaEndPicture(VADriverContextP ctx, VAContextID 
context_id)
 dst_rect.x1 = src_rect.x1 = surf->templat.width;
 dst_rect.y1 = src_rect.y1 = surf->templat.height;
 vl_compositor_yuv_deint_full(>cstate, >compositor,
  old_buf, surf->buffer,
  _rect, _rect, 
VL_COMPOSITOR_WEAVE);
- } else
+ } else {
 /* Can't convert from progressive to interlaced yet */
+mtx_unlock(>mutex);
 return VA_STATUS_ERROR_INVALID_SURFACE;
+ }
   }
 
   old_buf->destroy(old_buf);
   context->target = surf->buffer;
}
-- 
2.7.4

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

[Mesa-dev] [PATCH] radeon: remove unneeded semicolons

[Grazvydas Ignotas]

Trivial. Found by Coccinelle.
---
 src/gallium/drivers/radeon/radeon_vcn_dec.c | 6 +++---
 src/mesa/drivers/dri/radeon/radeon_debug.c  | 2 +-
 src/mesa/drivers/dri/radeon/radeon_state_init.c | 2 +-
 src/mesa/drivers/dri/radeon/radeon_swtcl.c  | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/gallium/drivers/radeon/radeon_vcn_dec.c 
b/src/gallium/drivers/radeon/radeon_vcn_dec.c
index 8be9538..5999b92 100644
--- a/src/gallium/drivers/radeon/radeon_vcn_dec.c
+++ b/src/gallium/drivers/radeon/radeon_vcn_dec.c
@@ -633,14 +633,14 @@ static struct pb_buffer *rvcn_dec_message_decode(struct 
radeon_decoder *dec,
 
index->offset = offset_codec;
index->size = sizeof(rvcn_dec_message_avc_t);
index->filled = 0;
 
-   decode->stream_type = dec->stream_type;;
+   decode->stream_type = dec->stream_type;
decode->decode_flags = 0x1;
-   decode->width_in_samples = dec->base.width;;
-   decode->height_in_samples = dec->base.height;;
+   decode->width_in_samples = dec->base.width;
+   decode->height_in_samples = dec->base.height;
 
decode->bsd_size = align(dec->bs_size, 128);
decode->dpb_size = dec->dpb.res->buf->size;
decode->dt_size =
((struct r600_resource *)((struct vl_video_buffer 
*)target)->resources[0])->buf->size +
diff --git a/src/mesa/drivers/dri/radeon/radeon_debug.c 
b/src/mesa/drivers/dri/radeon/radeon_debug.c
index 383a5df..91f86a9 100644
--- a/src/mesa/drivers/dri/radeon/radeon_debug.c
+++ b/src/mesa/drivers/dri/radeon/radeon_debug.c
@@ -73,11 +73,11 @@ void _radeon_debug_add_indent(void)
const size_t length = sizeof(radeon->debug.indent)
/ sizeof(radeon->debug.indent[0]);
if (radeon->debug.indent_depth < length - 1) {
radeon->debug.indent[radeon->debug.indent_depth] = '\t';
++radeon->debug.indent_depth;
-   };
+   }
 }
 
 void _radeon_debug_remove_indent(void)
 {
GET_CURRENT_CONTEXT(ctx);
diff --git a/src/mesa/drivers/dri/radeon/radeon_state_init.c 
b/src/mesa/drivers/dri/radeon/radeon_state_init.c
index 99c535a..b847be5 100644
--- a/src/mesa/drivers/dri/radeon/radeon_state_init.c
+++ b/src/mesa/drivers/dri/radeon/radeon_state_init.c
@@ -438,11 +438,11 @@ static void cube_emit_cs(struct gl_context *ctx, struct 
radeon_state_atom *atom)
switch(i) {
case 1: base_reg = RADEON_PP_CUBIC_OFFSET_T1_0; break;
case 2: base_reg = RADEON_PP_CUBIC_OFFSET_T2_0; break;
default:
case 0: base_reg = RADEON_PP_CUBIC_OFFSET_T0_0; break;
-   };
+   }
BEGIN_BATCH(dwords);
OUT_BATCH_TABLE(atom->cmd, 2);
lvl = >mt->levels[0];
for (j = 0; j < 5; j++) {
OUT_BATCH(CP_PACKET0(base_reg + (4 * j), 0));
diff --git a/src/mesa/drivers/dri/radeon/radeon_swtcl.c 
b/src/mesa/drivers/dri/radeon/radeon_swtcl.c
index d5365cd..860bba6 100644
--- a/src/mesa/drivers/dri/radeon/radeon_swtcl.c
+++ b/src/mesa/drivers/dri/radeon/radeon_swtcl.c
@@ -207,11 +207,11 @@ static void radeonSetVertexFormat( struct gl_context *ctx 
)
 radeon_cp_vc_frmts[i][1] );
   }
   break;
default:
   continue;
-   };
+   }
 }
   }
}
 
if (rmesa->radeon.tnl_index_bitset != index_bitset ||
-- 
2.7.4

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

[Mesa-dev] [PATCH] mesa: remove unneeded semicolons

[Grazvydas Ignotas]

Trivial. Found by Coccinelle.
---
 src/gbm/backends/dri/gbm_dri.c   | 2 +-
 src/mesa/main/dlist.c| 2 +-
 src/mesa/main/program_resource.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
index b2121cb..fd5fb4b 100644
--- a/src/gbm/backends/dri/gbm_dri.c
+++ b/src/gbm/backends/dri/gbm_dri.c
@@ -438,11 +438,11 @@ dri_screen_create_dri2(struct gbm_dri_device *dri, char 
*driver_name)
 
ret = dri_load_driver(dri);
if (ret) {
   fprintf(stderr, "failed to load driver: %s\n", dri->driver_name);
   return ret;
-   };
+   }
 
dri->loader_extensions = gbm_dri_screen_extensions;
 
if (dri->dri2 == NULL)
   return -1;
diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c
index b7d1406..a6b212e 100644
--- a/src/mesa/main/dlist.c
+++ b/src/mesa/main/dlist.c
@@ -1910,11 +1910,11 @@ save_CallLists(GLsizei num, GLenum type, const GLvoid * 
lists)
n = alloc_instruction(ctx, OPCODE_CALL_LISTS, 2 + POINTER_DWORDS);
if (n) {
   n[1].i = num;
   n[2].e = type;
   save_pointer([3], lists_copy);
-   };
+   }
 
/* After this, we don't know what state we're in.  Invalidate all
 * cached information previously gathered:
 */
invalidate_saved_current_state( ctx );
diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
index 4eacdfb..5fa5d75 100644
--- a/src/mesa/main/program_resource.c
+++ b/src/mesa/main/program_resource.c
@@ -198,11 +198,11 @@ _mesa_GetProgramInterfaceiv(GLuint program, GLenum 
programInterface,
   default:
 _mesa_error(ctx, GL_INVALID_OPERATION,
 "glGetProgramInterfaceiv(%s pname %s)",
 _mesa_enum_to_string(programInterface),
 _mesa_enum_to_string(pname));
-  };
+  }
   break;
case GL_MAX_NUM_COMPATIBLE_SUBROUTINES:
   switch (programInterface) {
   case GL_VERTEX_SUBROUTINE_UNIFORM:
   case GL_FRAGMENT_SUBROUTINE_UNIFORM:
-- 
2.7.4

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

[Mesa-dev] [PATCH] osmesa: don't check SmoothFlag twice

[Grazvydas Ignotas]

Trivial. Found by Coccinelle.
---
 src/mesa/drivers/osmesa/osmesa.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/mesa/drivers/osmesa/osmesa.c b/src/mesa/drivers/osmesa/osmesa.c
index 1df3da4..e0f87b8 100644
--- a/src/mesa/drivers/osmesa/osmesa.c
+++ b/src/mesa/drivers/osmesa/osmesa.c
@@ -212,11 +212,10 @@ osmesa_choose_line_function( struct gl_context *ctx )
*/
   return NULL;
}
 
if (ctx->RenderMode != GL_RENDER ||
-   ctx->Line.SmoothFlag ||
ctx->Texture._MaxEnabledTexImageUnit == -1 ||
ctx->Light.ShadeModel != GL_FLAT ||
ctx->Line.Width != 1.0F ||
ctx->Line.StippleFlag ||
ctx->Line.SmoothFlag) {
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 1/2] amd/common: import get_{load, store}_intr_attribs() from RadeonSI

[Grazvydas Ignotas]

On Wed, Jan 10, 2018 at 1:57 PM, Samuel Pitoiset
 wrote:
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/amd/common/ac_llvm_build.c| 12 ++-
>  src/amd/common/ac_llvm_util.c | 18 
>  src/amd/common/ac_llvm_util.h |  6 ++
>  src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c | 26 
> +--
>  4 files changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c
> index efc6fa12e5..07044142b0 100644
> --- a/src/amd/common/ac_llvm_build.c
> +++ b/src/amd/common/ac_llvm_build.c
> @@ -983,11 +983,7 @@ ac_build_buffer_load(struct ac_llvm_context *ctx,
>
> return ac_build_intrinsic(ctx, name, types[func], args,
>   ARRAY_SIZE(args),
> - /* READNONE means writes can't affect it, 
> while
> -  * READONLY means that writes can affect 
> it. */
> - can_speculate && HAVE_LLVM >= 0x0400 ?
> - AC_FUNC_ATTR_READNONE :
> - AC_FUNC_ATTR_READONLY);
> + ac_get_load_intr_attribs(can_speculate));
>  }
>
>  LLVMValueRef ac_build_buffer_load_format(struct ac_llvm_context *ctx,
> @@ -1007,11 +1003,7 @@ LLVMValueRef ac_build_buffer_load_format(struct 
> ac_llvm_context *ctx,
> return ac_build_intrinsic(ctx,
>   "llvm.amdgcn.buffer.load.format.v4f32",
>   ctx->v4f32, args, ARRAY_SIZE(args),
> - /* READNONE means writes can't affect it, 
> while
> -  * READONLY means that writes can affect 
> it. */
> - can_speculate && HAVE_LLVM >= 0x0400 ?
> - AC_FUNC_ATTR_READNONE :
> - AC_FUNC_ATTR_READONLY);
> + ac_get_load_intr_attribs(can_speculate));
>  }
>
>  /**
> diff --git a/src/amd/common/ac_llvm_util.c b/src/amd/common/ac_llvm_util.c
> index 429904c040..976d6540a1 100644
> --- a/src/amd/common/ac_llvm_util.c
> +++ b/src/amd/common/ac_llvm_util.c
> @@ -240,3 +240,21 @@ ac_llvm_add_target_dep_function_attr(LLVMValueRef F,
> snprintf(str, sizeof(str), "%i", value);
> LLVMAddTargetDependentFunctionAttr(F, name, str);
>  }
> +
> +unsigned
> +ac_get_load_intr_attribs(bool can_speculate)
> +{
> +   /* READNONE means writes can't affect it, while READONLY means that
> +* writes can affect it. */
> +   return can_speculate && HAVE_LLVM >= 0x0400 ?
> +AC_FUNC_ATTR_READNONE :
> +AC_FUNC_ATTR_READONLY;
> +}
> +
> +unsigned
> +ac_get_store_intr_attribs(bool writeonly_memory)
> +{
> +   return writeonly_memory && HAVE_LLVM >= 0x0400 ?
> + AC_FUNC_ATTR_INACCESSIBLE_MEM_ONLY :
> + AC_FUNC_ATTR_WRITEONLY;
> +}

What about putting such tiny functions in the header as static inline
to avoid function call overheads?

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

[Mesa-dev] [PATCH v2 1/3] util/crc32: don't drop the const qualifier

[Grazvydas Ignotas]

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/util/crc32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/crc32.c b/src/util/crc32.c
index 44d637c..f2e01c6 100644
--- a/src/util/crc32.c
+++ b/src/util/crc32.c
@@ -109,11 +109,11 @@ util_crc32_table[256] = {
  * @sa http://www.w3.org/TR/PNG/#D-CRCAppendix
  */
 uint32_t
 util_hash_crc32(const void *data, size_t size)
 {
-   uint8_t *p = (uint8_t *)data;
+   const uint8_t *p = data;
uint32_t crc = 0x;
  
while (size--)
   crc = util_crc32_table[(crc ^ *p++) & 0xff] ^ (crc >> 8);

-- 
2.7.4

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

[Mesa-dev] [PATCH v2 2/3] android, configure, meson: define HAVE_ZLIB

[Grazvydas Ignotas]

The next change wants to use some optional zlib functionality, however
not all platforms currently use it. Based on earlier Jordan Justen's
patches and their review feedback.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 Android.common.mk | 1 +
 configure.ac  | 1 +
 meson.build   | 1 +
 3 files changed, 3 insertions(+)

diff --git a/Android.common.mk b/Android.common.mk
index d9f871c..52dc7bf 100644
--- a/Android.common.mk
+++ b/Android.common.mk
@@ -68,10 +68,11 @@ LOCAL_CFLAGS += \
-DHAVE___BUILTIN_UNREACHABLE \
-DHAVE_PTHREAD=1 \
-DHAVE_DLADDR \
-DHAVE_DL_ITERATE_PHDR \
-DHAVE_LINUX_FUTEX_H \
+   -DHAVE_ZLIB \
-DMAJOR_IN_SYSMACROS \
-fvisibility=hidden \
-Wno-sign-compare
 
 LOCAL_CPPFLAGS += \
diff --git a/configure.ac b/configure.ac
index 79f275d..e236a3c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -904,10 +904,11 @@ esac
 dnl See if posix_memalign is available
 AC_CHECK_FUNC([posix_memalign], [DEFINES="$DEFINES -DHAVE_POSIX_MEMALIGN"])
 
 dnl Check for zlib
 PKG_CHECK_MODULES([ZLIB], [zlib >= $ZLIB_REQUIRED])
+DEFINES="$DEFINES -DHAVE_ZLIB"
 
 dnl Check for pthreads
 AX_PTHREAD
 if test "x$ax_pthread_ok" = xno; then
 AC_MSG_ERROR([Building mesa on this platform requires pthreads])
diff --git a/meson.build b/meson.build
index 77e4e89..ae31cdd 100644
--- a/meson.build
+++ b/meson.build
@@ -941,10 +941,11 @@ if dep_libdrm.found()
   endif
 endif
 
 # TODO: some of these may be conditional
 dep_zlib = dependency('zlib', version : '>= 1.2.3')
+pre_args += '-DHAVE_ZLIB'
 dep_thread = dependency('threads')
 if dep_thread.found() and host_machine.system() != 'windows'
   pre_args += '-DHAVE_PTHREAD'
 endif
 if with_amd_vk or with_gallium_radeonsi or with_gallium_r600 or 
with_gallium_opencl
-- 
2.7.4

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

[Mesa-dev] [PATCH v2 3/3] util: use faster zlib's CRC32 implementaion

[Grazvydas Ignotas]

zlib provides a faster slice-by-4 CRC32 implementation than the
traditional single byte lookup one used by mesa. As most supported
platforms now link zlib unconditionally, we can easily use it.

Improvement for a 1MB buffer (avg MB/s, n=100, zlib 1.2.8):

  i5-6600KC2D E4500
mesa zlibmesa zlib
 443 1443 225% +/- 2.1%   403 1175 191% +/- 0.9%

It has been verified the calculation results stay the same after this
change.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
v2: drop the size threshold check because size is unlikely to be that
low of things mesa is typically hashing

 src/util/crc32.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/src/util/crc32.c b/src/util/crc32.c
index f2e01c6..9edd3e1 100644
--- a/src/util/crc32.c
+++ b/src/util/crc32.c
@@ -31,10 +31,13 @@
  * 
  * @author Jose Fonseca
  */
 
 
+#ifdef HAVE_ZLIB
+#include 
+#endif
 #include "crc32.h"
 
 
 static const uint32_t 
 util_crc32_table[256] = {
@@ -112,10 +115,19 @@ uint32_t
 util_hash_crc32(const void *data, size_t size)
 {
const uint8_t *p = data;
uint32_t crc = 0x;
  
+#ifdef HAVE_ZLIB
+   /* zlib's uInt is always "unsigned int" while size_t can be 64bit.
+* Since 1.2.9 there's crc32_z that takes size_t, but use the more
+* available function to avoid build system complications.
+*/
+   if ((uInt)size == size)
+  return ~crc32(0, data, size);
+#endif
+
while (size--)
   crc = util_crc32_table[(crc ^ *p++) & 0xff] ^ (crc >> 8);

return crc;
 }
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 3/3] util: use zlib's CRC32 implementaion for larger buffers

[Grazvydas Ignotas]

On Wed, Jan 3, 2018 at 3:09 AM, Ian Romanick <i...@freedesktop.org> wrote:
> On 01/02/2018 04:52 PM, Grazvydas Ignotas wrote:
>> On Tue, Jan 2, 2018 at 11:38 PM, Ian Romanick <i...@freedesktop.org> wrote:
>>> On 12/28/2017 05:56 PM, Grazvydas Ignotas wrote:
>>>> zlib provides a faster slice-by-4 CRC32 implementation than the
>>>> traditional single byte lookup one used by mesa. As most supported
>>>> platforms now link zlib unconditionally, we can easily use it.
>>>> For small buffers the old implementation is still used as it's faster
>>>> with cold cache (first call), as indicated by some throughput
>>>> benchmarking (avg MB/s, n=100, zlib 1.2.8):
>>>>
>>>> i5-6600KC2D E4500
>>>> size  mesa zlibmesa zlib
>>>> 4   66   43 -35% +/- 4.8%43   22 -49% +/- 9.6%
>>>> 32 193  171 -11% +/- 5.8%   129   49 -61% +/- 7.2%
>>>> 64 256  267   4% +/- 4.1%   171   63 -63% +/- 5.4%
>>>> 128317  389  22% +/- 5.8%   253   89 -64% +/- 4.2%
>>>> 256364  596  63% +/- 5.6%   304  166 -45% +/- 2.8%
>>>> 512401  838 108% +/- 5.3%   338  296 -12% +/- 3.1%
>>>> 1024   420 1036 146% +/- 7.6%   375  461  23% +/- 3.7%
>>>> 1M 443 1443 225% +/- 2.1%   403 1175 191% +/- 0.9%
>>>> 100M   448 1452 224% +/- 0.3%   406 1214 198% +/- 0.3%
>>>>
>>>> With hot cache (repeated calls) zlib almost always wins on both CPUS.
>>>> It has been verified the calculation results stay the same after this
>>>> change.
>>>>
>>>> Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
>>>> ---
>>>>  src/util/crc32.c | 13 +
>>>>  1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/src/util/crc32.c b/src/util/crc32.c
>>>> index f2e01c6..0cffa49 100644
>>>> --- a/src/util/crc32.c
>>>> +++ b/src/util/crc32.c
>>>> @@ -31,12 +31,20 @@
>>>>   *
>>>>   * @author Jose Fonseca
>>>>   */
>>>>
>>>>
>>>> +#ifdef HAVE_ZLIB
>>>> +#include 
>>>> +#endif
>>>>  #include "crc32.h"
>>>>
>>>> +/* For small buffers it's faster to avoid the library call.
>>>> + * The optimal threshold depends on CPU characteristics, it is hoped
>>>> + * the choice below is reasonable for typical modern CPU.
>>>> + */
>>>> +#define ZLIB_SIZE_THRESHOLD 64
>>>
>>> For the actual users of this function in Mesa, is it even possible to
>>> pass less than 64 bytes (I'm assuming that's the units)?
>>
>> Hmm why wouldn't it be? The unit is a byte, and you can compute CRC32
>> of a single byte.
>
> It can be done, but that's not my question.  My question is: would any
> of the existing users actually do this?  I thought the main user was the
> various disk cache / GetProgramBinary kind of things.  You won't have a
> shader binary that's a single byte... less than 64 also seems unlikely.
> Unless there are other users?

You're most likely right, I'll just drop this.

>
>>>>
>>>>  static const uint32_t
>>>>  util_crc32_table[256] = {
>>>> 0x, 0x77073096, 0xee0e612c, 0x990951ba,
>>>> 0x076dc419, 0x706af48f, 0xe963a535, 0x9e6495a3,
>>>> @@ -112,10 +120,15 @@ uint32_t
>>>>  util_hash_crc32(const void *data, size_t size)
>>>>  {
>>>> const uint8_t *p = data;
>>>> uint32_t crc = 0x;
>>>>
>>>> +#ifdef HAVE_ZLIB
>>>> +   if (size >= ZLIB_SIZE_THRESHOLD && (uInt)size == size)
>>>  ^^
>>> This comparison is always true (unless sizeof(uInt) != sizeof(size_t)?).
>>>  I'm not 100% sure what you're trying to accomplish here, but I think
>>> you want 'size > 0'.  I'm not familiar with this zlib function, so I
>>> don't know what it's expectations for size are.
>>
>> zlib's uInt is always 32bit while size_t is 64bit on most (all?) 64bit
>> architectures, so if someone decides to CRC32 >= 4GiB buffer, this
>> function will do the wrong thing without such check. Newer zlib has
>> crc32_z that takes size_t, but I was trying to avoid build system
>> complications of detecting that function...
>
> Ok.  That makes sense.  I'll bet this adds a warning about tautological
> compares only on 32-bit, then.

I don't seem to be getting a warning on gcc 7.2.0 or 5.4.0 as well as
clang 3.8.0 at least.

>  I was going to suggest comparing with
> 0x instead, but GCC emits worse code for that... and it probably
> still results in the warning on 32-bit.  Maybe just a comment (based on
> your reply) that describes what's happening for the next person that
> reads the code.

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

Re: [Mesa-dev] [PATCH 3/3] util: use zlib's CRC32 implementaion for larger buffers

[Grazvydas Ignotas]

On Tue, Jan 2, 2018 at 11:38 PM, Ian Romanick <i...@freedesktop.org> wrote:
> On 12/28/2017 05:56 PM, Grazvydas Ignotas wrote:
>> zlib provides a faster slice-by-4 CRC32 implementation than the
>> traditional single byte lookup one used by mesa. As most supported
>> platforms now link zlib unconditionally, we can easily use it.
>> For small buffers the old implementation is still used as it's faster
>> with cold cache (first call), as indicated by some throughput
>> benchmarking (avg MB/s, n=100, zlib 1.2.8):
>>
>> i5-6600KC2D E4500
>> size  mesa zlibmesa zlib
>> 4   66   43 -35% +/- 4.8%43   22 -49% +/- 9.6%
>> 32 193  171 -11% +/- 5.8%   129   49 -61% +/- 7.2%
>> 64 256  267   4% +/- 4.1%   171   63 -63% +/- 5.4%
>> 128317  389  22% +/- 5.8%   253   89 -64% +/- 4.2%
>> 256364  596  63% +/- 5.6%   304  166 -45% +/- 2.8%
>> 512401  838 108% +/- 5.3%   338  296 -12% +/- 3.1%
>> 1024   420 1036 146% +/- 7.6%   375  461  23% +/- 3.7%
>> 1M 443 1443 225% +/- 2.1%   403 1175 191% +/- 0.9%
>> 100M   448 1452 224% +/- 0.3%   406 1214 198% +/- 0.3%
>>
>> With hot cache (repeated calls) zlib almost always wins on both CPUS.
>> It has been verified the calculation results stay the same after this
>> change.
>>
>> Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
>> ---
>>  src/util/crc32.c | 13 +
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/src/util/crc32.c b/src/util/crc32.c
>> index f2e01c6..0cffa49 100644
>> --- a/src/util/crc32.c
>> +++ b/src/util/crc32.c
>> @@ -31,12 +31,20 @@
>>   *
>>   * @author Jose Fonseca
>>   */
>>
>>
>> +#ifdef HAVE_ZLIB
>> +#include 
>> +#endif
>>  #include "crc32.h"
>>
>> +/* For small buffers it's faster to avoid the library call.
>> + * The optimal threshold depends on CPU characteristics, it is hoped
>> + * the choice below is reasonable for typical modern CPU.
>> + */
>> +#define ZLIB_SIZE_THRESHOLD 64
>
> For the actual users of this function in Mesa, is it even possible to
> pass less than 64 bytes (I'm assuming that's the units)?

Hmm why wouldn't it be? The unit is a byte, and you can compute CRC32
of a single byte.

>
>>
>>  static const uint32_t
>>  util_crc32_table[256] = {
>> 0x, 0x77073096, 0xee0e612c, 0x990951ba,
>> 0x076dc419, 0x706af48f, 0xe963a535, 0x9e6495a3,
>> @@ -112,10 +120,15 @@ uint32_t
>>  util_hash_crc32(const void *data, size_t size)
>>  {
>> const uint8_t *p = data;
>> uint32_t crc = 0x;
>>
>> +#ifdef HAVE_ZLIB
>> +   if (size >= ZLIB_SIZE_THRESHOLD && (uInt)size == size)
>  ^^
> This comparison is always true (unless sizeof(uInt) != sizeof(size_t)?).
>  I'm not 100% sure what you're trying to accomplish here, but I think
> you want 'size > 0'.  I'm not familiar with this zlib function, so I
> don't know what it's expectations for size are.

zlib's uInt is always 32bit while size_t is 64bit on most (all?) 64bit
architectures, so if someone decides to CRC32 >= 4GiB buffer, this
function will do the wrong thing without such check. Newer zlib has
crc32_z that takes size_t, but I was trying to avoid build system
complications of detecting that function...

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

Re: [Mesa-dev] [PATCH 1/3] spirv: Add a mechanism for dumping failing shaders

[Grazvydas Ignotas]

On Tue, Jan 2, 2018 at 6:30 PM, Jason Ekstrand  wrote:
> ---
>  src/compiler/spirv/spirv_to_nir.c | 29 +
>  src/compiler/spirv/vtn_private.h  |  1 +
>  2 files changed, 30 insertions(+)
>
> diff --git a/src/compiler/spirv/spirv_to_nir.c 
> b/src/compiler/spirv/spirv_to_nir.c
> index dcff56f..751fb03 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -31,6 +31,9 @@
>  #include "nir/nir_constant_expressions.h"
>  #include "spirv_info.h"
>
> +#include 
> +#include 
> +
>  void
>  vtn_log(struct vtn_builder *b, enum nir_spirv_debug_level level,
>  size_t spirv_offset, const char *message)
> @@ -94,6 +97,27 @@ vtn_log_err(struct vtn_builder *b,
> ralloc_free(msg);
>  }
>
> +static void
> +vtn_dump_shader(struct vtn_builder *b, const char *path, const char *prefix)
> +{
> +   static int idx = 0;
> +
> +   char filename[1024];
> +   int len = snprintf(filename, sizeof(filename), "%s/%s-%d.spirv",
> +  path, prefix, idx++);
> +   if (len < 0 || len >= sizeof(filename))
> +  return;
> +
> +   int fd = open(filename, O_CREAT | O_CLOEXEC | O_WRONLY, 0777);
> +   if (fd < 0)
> +  return;
> +
> +   write(fd, b->spirv, b->spirv_word_count * 4);

Feel free to ignore, but what about * sizeof(b->spirv[0]) ?

also, this emits a not-so-useful warning for me:
warning: ignoring return value of ‘write’, declared with attribute
warn_unused_result [-Wunused-result]
(and no, sticking (void) before write() doesn't help)

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

Re: [Mesa-dev] [PATCH 3/8] spirv: Add basic type validation for OpLoad, OpStore, and OpCopyMemory

[Grazvydas Ignotas]

Hi,

I don't know if it's the game's fault, but it appears this change broke DOOM.
here is the offending spirv binary:
https://people.freedesktop.org/~notaz/doom_compute_spirv

Gražvydas


On Thu, Dec 7, 2017 at 6:12 PM, Jason Ekstrand  wrote:
> ---
>  src/compiler/spirv/vtn_variables.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/src/compiler/spirv/vtn_variables.c 
> b/src/compiler/spirv/vtn_variables.c
> index cf44ed3..8ce19ff 100644
> --- a/src/compiler/spirv/vtn_variables.c
> +++ b/src/compiler/spirv/vtn_variables.c
> @@ -1969,6 +1969,9 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp 
> opcode,
>struct vtn_value *dest = vtn_value(b, w[1], vtn_value_type_pointer);
>struct vtn_value *src = vtn_value(b, w[2], vtn_value_type_pointer);
>
> +  vtn_fail_if(dest->type->deref != src->type->deref,
> +  "Result and pointer types of OpLoad do not match");
> +
>vtn_variable_copy(b, dest->pointer, src->pointer);
>break;
> }
> @@ -1976,8 +1979,11 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp 
> opcode,
> case SpvOpLoad: {
>struct vtn_type *res_type =
>   vtn_value(b, w[1], vtn_value_type_type)->type;
> -  struct vtn_pointer *src =
> - vtn_value(b, w[3], vtn_value_type_pointer)->pointer;
> +  struct vtn_value *src_val = vtn_value(b, w[3], vtn_value_type_pointer);
> +  struct vtn_pointer *src = src_val->pointer;
> +
> +  vtn_fail_if(res_type != src_val->type->deref,
> +  "Result and pointer types of OpLoad do not match");
>
>if (src->mode == vtn_variable_mode_image ||
>src->mode == vtn_variable_mode_sampler) {
> @@ -1990,8 +1996,12 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp 
> opcode,
> }
>
> case SpvOpStore: {
> -  struct vtn_pointer *dest =
> - vtn_value(b, w[1], vtn_value_type_pointer)->pointer;
> +  struct vtn_value *dest_val = vtn_value(b, w[1], 
> vtn_value_type_pointer);
> +  struct vtn_pointer *dest = dest_val->pointer;
> +  struct vtn_value *src_val = vtn_untyped_value(b, w[2]);
> +
> +  vtn_fail_if(dest_val->type->deref != src_val->type,
> +  "Value and pointer types of OpStore do not match");
>
>if (glsl_type_is_sampler(dest->type->type)) {
>   vtn_warn("OpStore of a sampler detected.  Doing on-the-fly copy "
> --
> 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] [PATCH 3/3] util: use zlib's CRC32 implementaion for larger buffers

[Grazvydas Ignotas]

zlib provides a faster slice-by-4 CRC32 implementation than the
traditional single byte lookup one used by mesa. As most supported
platforms now link zlib unconditionally, we can easily use it.
For small buffers the old implementation is still used as it's faster
with cold cache (first call), as indicated by some throughput
benchmarking (avg MB/s, n=100, zlib 1.2.8):

i5-6600KC2D E4500
size  mesa zlibmesa zlib
4   66   43 -35% +/- 4.8%43   22 -49% +/- 9.6%
32 193  171 -11% +/- 5.8%   129   49 -61% +/- 7.2%
64 256  267   4% +/- 4.1%   171   63 -63% +/- 5.4%
128317  389  22% +/- 5.8%   253   89 -64% +/- 4.2%
256364  596  63% +/- 5.6%   304  166 -45% +/- 2.8%
512401  838 108% +/- 5.3%   338  296 -12% +/- 3.1%
1024   420 1036 146% +/- 7.6%   375  461  23% +/- 3.7%
1M 443 1443 225% +/- 2.1%   403 1175 191% +/- 0.9%
100M   448 1452 224% +/- 0.3%   406 1214 198% +/- 0.3%

With hot cache (repeated calls) zlib almost always wins on both CPUS.
It has been verified the calculation results stay the same after this
change.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/util/crc32.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/util/crc32.c b/src/util/crc32.c
index f2e01c6..0cffa49 100644
--- a/src/util/crc32.c
+++ b/src/util/crc32.c
@@ -31,12 +31,20 @@
  * 
  * @author Jose Fonseca
  */
 
 
+#ifdef HAVE_ZLIB
+#include 
+#endif
 #include "crc32.h"
 
+/* For small buffers it's faster to avoid the library call.
+ * The optimal threshold depends on CPU characteristics, it is hoped
+ * the choice below is reasonable for typical modern CPU.
+ */
+#define ZLIB_SIZE_THRESHOLD 64
 
 static const uint32_t 
 util_crc32_table[256] = {
0x, 0x77073096, 0xee0e612c, 0x990951ba, 
0x076dc419, 0x706af48f, 0xe963a535, 0x9e6495a3, 
@@ -112,10 +120,15 @@ uint32_t
 util_hash_crc32(const void *data, size_t size)
 {
const uint8_t *p = data;
uint32_t crc = 0x;
  
+#ifdef HAVE_ZLIB
+   if (size >= ZLIB_SIZE_THRESHOLD && (uInt)size == size)
+  return ~crc32(0, data, size);
+#endif
+
while (size--)
   crc = util_crc32_table[(crc ^ *p++) & 0xff] ^ (crc >> 8);

return crc;
 }
-- 
2.7.4

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

[Mesa-dev] [PATCH 1/3] android,configure,meson: define HAVE_ZLIB

[Grazvydas Ignotas]

The next change wants to use some optional zlib functionality, however
not all platforms currently use zlib. Based on earlier Jordan Justen's
patches and their review feedback.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 Android.common.mk | 1 +
 configure.ac  | 1 +
 meson.build   | 1 +
 3 files changed, 3 insertions(+)

diff --git a/Android.common.mk b/Android.common.mk
index d9f871c..52dc7bf 100644
--- a/Android.common.mk
+++ b/Android.common.mk
@@ -68,10 +68,11 @@ LOCAL_CFLAGS += \
-DHAVE___BUILTIN_UNREACHABLE \
-DHAVE_PTHREAD=1 \
-DHAVE_DLADDR \
-DHAVE_DL_ITERATE_PHDR \
-DHAVE_LINUX_FUTEX_H \
+   -DHAVE_ZLIB \
-DMAJOR_IN_SYSMACROS \
-fvisibility=hidden \
-Wno-sign-compare
 
 LOCAL_CPPFLAGS += \
diff --git a/configure.ac b/configure.ac
index 79f275d..e236a3c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -904,10 +904,11 @@ esac
 dnl See if posix_memalign is available
 AC_CHECK_FUNC([posix_memalign], [DEFINES="$DEFINES -DHAVE_POSIX_MEMALIGN"])
 
 dnl Check for zlib
 PKG_CHECK_MODULES([ZLIB], [zlib >= $ZLIB_REQUIRED])
+DEFINES="$DEFINES -DHAVE_ZLIB"
 
 dnl Check for pthreads
 AX_PTHREAD
 if test "x$ax_pthread_ok" = xno; then
 AC_MSG_ERROR([Building mesa on this platform requires pthreads])
diff --git a/meson.build b/meson.build
index d9f7ea9..9d9d074 100644
--- a/meson.build
+++ b/meson.build
@@ -922,10 +922,11 @@ if dep_libdrm.found()
   endif
 endif
 
 # TODO: some of these may be conditional
 dep_zlib = dependency('zlib', version : '>= 1.2.3')
+pre_args += '-DHAVE_ZLIB'
 dep_thread = dependency('threads')
 if dep_thread.found() and host_machine.system() != 'windows'
   pre_args += '-DHAVE_PTHREAD'
 endif
 if with_amd_vk or with_gallium_radeonsi or with_gallium_r600 # TODO: clover
-- 
2.7.4

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

[Mesa-dev] [PATCH 2/3] util/crc32: don't drop the const qualifier

[Grazvydas Ignotas]

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/util/crc32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/crc32.c b/src/util/crc32.c
index 44d637c..f2e01c6 100644
--- a/src/util/crc32.c
+++ b/src/util/crc32.c
@@ -109,11 +109,11 @@ util_crc32_table[256] = {
  * @sa http://www.w3.org/TR/PNG/#D-CRCAppendix
  */
 uint32_t
 util_hash_crc32(const void *data, size_t size)
 {
-   uint8_t *p = (uint8_t *)data;
+   const uint8_t *p = data;
uint32_t crc = 0x;
  
while (size--)
   crc = util_crc32_table[(crc ^ *p++) & 0xff] ^ (crc >> 8);

-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 00/28] vulkan/wsi: Rework WSI to look a lot more like a layer

[Grazvydas Ignotas]

On Wed, Nov 22, 2017 at 7:54 AM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
> On Tue, Nov 21, 2017 at 1:21 PM, Grazvydas Ignotas <nota...@gmail.com>
> wrote:
>>
>> On Mon, Nov 20, 2017 at 6:08 PM, Jason Ekstrand <ja...@jlekstrand.net>
>> wrote:
>> > On Sun, Nov 19, 2017 at 5:07 AM, Grazvydas Ignotas <nota...@gmail.com>
>> > wrote:
>> >>
>> >> On Sun, Nov 19, 2017 at 1:51 AM, Jason Ekstrand <ja...@jlekstrand.net>
>> >> wrote:
>> >> >
>> >> > I force-pushed the branch again with an added commit: "radv: Move wsi
>> >> > initialization later in physical_device_init" that fixes the memory
>> >> > type
>> >> > issue with radv.  I've tested both radv + radeon and anv + radeon on
>> >> > my
>> >> > HSW
>> >> > + Rx550 and they both work now.  I'm having a bit of trouble actually
>> >> > getting my system to start up on the Intel card so I'll have to leave
>> >> > testing radv on Intel for another day.
>> >>
>> >> Radv is working now on both displays, however "display on amd + anv"
>> >> case still acts the same (black window on most, but not all
>> >> SaschaWillems demos). I'm using xf86-video-amdgpu 1.4.0, 4.14 kernel
>> >> and xorg-server 1.18.4, if that makes a difference.
>> >
>> >
>> > I'm completely unable to reproduce.  Here's my setup:
>> >
>> >  - Fedora 27
>> >  - X.org 1.19.5
>> >  - xf86-video-amdgpu 1.3.0
>> >  - Linux 4.13.12
>> >  - Intel Haswell
>> >  - AMD RX550
>> >
>> > I've tried with amdgpu, modesetting, and XWayland all running on the AMD
>> > card and anv works on all three.  I'm a little weirded out by the fact
>> > that
>> > my X server is newer but my xf86-video-amdgpu is older.
>>
>> Well I compiled my own xf86-video-amdgpu. Not sure why.
>>
>> > Two things I'd like you to try if you can:
>> >
>> >  1) Use modesetting.  It may be a bug in your version of amdgpu.
>>
>> Same results (black window), plus all the tearing all over I usually
>> get with it. Also tried the distro kernel (4.10).
>>
>> >  2) Try the attached patch with radv + display on AMD.  It will make
>> > radv
>> > use the prime path regardless of the fact that it's displaying on the
>> > same
>> > GPU.
>>
>> Still works fine, albeit a bit slower (as expected I guess).
>> Maybe something specific to SKL?
>
>
> I think it may be weirdly enough.  More specifically, I think you're
> probably hitting a bug I found today in the Sascha demos that probably only
> actually shows up on SKL.  Give this PR a try:
>
> https://github.com/SaschaWillems/Vulkan/pull/400

That helps, thanks. But textoverlay and subpasses still show black
screen on amd display and multisampling is dark even on intel. All are
fine on radv on both displays.

Here are some weird ones:
- texture3d works on anv on amd display, but triggers
"vulkan/anv_batch_chain.c:1139: adjust_relocations_from_state_pool:
Assertion `last_pool_center_bo_offset <=
pool->block_pool.center_bo_offset' failed." on intel display.
- multisampling on radv isn't clearing the background on intel
display, but is fine on amd display.

In other tests with this series, Talos Principle and Serious Sam
Fusion work on anv+amd display, while DOOM is failing with
VK_ERROR_FEATURE_NOT_PRESENT on any display (and does so for many
months now, probably since 5dd96b1156, but I haven't tried looking
what the feature is).

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

Re: [Mesa-dev] [PATCH 00/28] vulkan/wsi: Rework WSI to look a lot more like a layer

[Grazvydas Ignotas]

On Mon, Nov 20, 2017 at 6:08 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
> On Sun, Nov 19, 2017 at 5:07 AM, Grazvydas Ignotas <nota...@gmail.com>
> wrote:
>>
>> On Sun, Nov 19, 2017 at 1:51 AM, Jason Ekstrand <ja...@jlekstrand.net>
>> wrote:
>> >
>> > I force-pushed the branch again with an added commit: "radv: Move wsi
>> > initialization later in physical_device_init" that fixes the memory type
>> > issue with radv.  I've tested both radv + radeon and anv + radeon on my
>> > HSW
>> > + Rx550 and they both work now.  I'm having a bit of trouble actually
>> > getting my system to start up on the Intel card so I'll have to leave
>> > testing radv on Intel for another day.
>>
>> Radv is working now on both displays, however "display on amd + anv"
>> case still acts the same (black window on most, but not all
>> SaschaWillems demos). I'm using xf86-video-amdgpu 1.4.0, 4.14 kernel
>> and xorg-server 1.18.4, if that makes a difference.
>
>
> I'm completely unable to reproduce.  Here's my setup:
>
>  - Fedora 27
>  - X.org 1.19.5
>  - xf86-video-amdgpu 1.3.0
>  - Linux 4.13.12
>  - Intel Haswell
>  - AMD RX550
>
> I've tried with amdgpu, modesetting, and XWayland all running on the AMD
> card and anv works on all three.  I'm a little weirded out by the fact that
> my X server is newer but my xf86-video-amdgpu is older.

Well I compiled my own xf86-video-amdgpu. Not sure why.

> Two things I'd like you to try if you can:
>
>  1) Use modesetting.  It may be a bug in your version of amdgpu.

Same results (black window), plus all the tearing all over I usually
get with it. Also tried the distro kernel (4.10).

>  2) Try the attached patch with radv + display on AMD.  It will make radv
> use the prime path regardless of the fact that it's displaying on the same
> GPU.

Still works fine, albeit a bit slower (as expected I guess).
Maybe something specific to SKL?

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

Re: [Mesa-dev] [PATCH 00/28] vulkan/wsi: Rework WSI to look a lot more like a layer

[Grazvydas Ignotas]

On Sun, Nov 19, 2017 at 1:51 AM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
> On Sat, Nov 18, 2017 at 9:02 AM, Grazvydas Ignotas <nota...@gmail.com>
> wrote:
>>
>> On Sat, Nov 18, 2017 at 3:06 AM, Jason Ekstrand <ja...@jlekstrand.net>
>> wrote:
>> > On Fri, Nov 17, 2017 at 2:18 PM, Grazvydas Ignotas <nota...@gmail.com>
>> > wrote:
>> >>
>> >> I've tested this branch (rx470 + hd530) and it's only partially
>> >> working:
>> >> - display on amd, radv: always fails with "offscreen:
>> >> wsi/wsi_common.c:172: select_memory_type: Assertion `!"No memory type
>> >> found"' failed."
>> >> - display on amd, anv: misrenders. For SaschaWillems, only
>> >> multisampling, subpasses and triangle renders, other demos show black
>> >> window with only text (fps counter and help) visible
>> >> - display on intel, radv: same "No memory type found"'
>> >> - display on intel, anv: seems to be ok
>> >
>> >
>> > Thanks for testing!  I've got a HSW+radeon box at home.  I'll see if I
>> > can
>> > get it fixed early next week.  I did find a couple bugs by inspection
>> > when I
>> > was working on getting modifiers working today.  I've re-pushed if you
>> > feel
>> > like trying again.  Otherwise, I'll see if I can repro on my box.
>>
>> Seems to be the same as before. Assertion `!"No memory type found"'
>> starts hitting at "vulkan/wsi: Do image creation in common code".
>
>
> I force-pushed the branch again with an added commit: "radv: Move wsi
> initialization later in physical_device_init" that fixes the memory type
> issue with radv.  I've tested both radv + radeon and anv + radeon on my HSW
> + Rx550 and they both work now.  I'm having a bit of trouble actually
> getting my system to start up on the Intel card so I'll have to leave
> testing radv on Intel for another day.

Radv is working now on both displays, however "display on amd + anv"
case still acts the same (black window on most, but not all
SaschaWillems demos). I'm using xf86-video-amdgpu 1.4.0, 4.14 kernel
and xorg-server 1.18.4, if that makes a difference.

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

Re: [Mesa-dev] [PATCH 00/28] vulkan/wsi: Rework WSI to look a lot more like a layer

[Grazvydas Ignotas]

On Sat, Nov 18, 2017 at 3:06 AM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
> On Fri, Nov 17, 2017 at 2:18 PM, Grazvydas Ignotas <nota...@gmail.com>
> wrote:
>>
>> I've tested this branch (rx470 + hd530) and it's only partially working:
>> - display on amd, radv: always fails with "offscreen:
>> wsi/wsi_common.c:172: select_memory_type: Assertion `!"No memory type
>> found"' failed."
>> - display on amd, anv: misrenders. For SaschaWillems, only
>> multisampling, subpasses and triangle renders, other demos show black
>> window with only text (fps counter and help) visible
>> - display on intel, radv: same "No memory type found"'
>> - display on intel, anv: seems to be ok
>
>
> Thanks for testing!  I've got a HSW+radeon box at home.  I'll see if I can
> get it fixed early next week.  I did find a couple bugs by inspection when I
> was working on getting modifiers working today.  I've re-pushed if you feel
> like trying again.  Otherwise, I'll see if I can repro on my box.

Seems to be the same as before. Assertion `!"No memory type found"'
starts hitting at "vulkan/wsi: Do image creation in common code".

Gražvydas

>
> --Jason
>
>>
>> Gražvydas
>>
>> On Fri, Nov 17, 2017 at 4:44 AM, Jason Ekstrand <ja...@jlekstrand.net>
>> wrote:
>> > I just force-pushed my branch with some changes as per Dave to more
>> > explicitly enable implicit sync when allocating memory objects.
>> >
>> > On Thu, Nov 16, 2017 at 1:28 PM, Jason Ekstrand <ja...@jlekstrand.net>
>> > wrote:
>> >>
>> >> This patch series is the combined brain-child of Dave and myself.  The
>> >> objective is to rewrite Vulkan WSI to look as much like a layer as
>> >> possible
>> >> and to reduce the driver <-> WSI interface.  We try very hard to have
>> >> as
>> >> many of the WSI details as possible in common code and to use standard
>> >> Vulkan interfaces for everything.  Among other things, this means that
>> >> prime support is now implemented in an entirely driver-agnostic way and
>> >> the
>> >> driver doesn't even know it's happening.  As a side-effect anv now has
>> >> prime support.
>> >>
>> >> Eventually, someone could pull what's left out into a proper layer and
>> >> we
>> >> could drop WSI support from our drivers entirely.  There are a fiew
>> >> pieces
>> >> of work that would be required to do this:
>> >>
>> >>  1) Write all of the annoying layer bits.  There are some short-cuts
>> >> that
>> >> we can take because we're not in a layer and those will have to go.
>> >>
>> >>  2) Define a VK_MESA_legacy_swapchain_image extension to replace the
>> >> hack
>> >> introduced in patch 8.
>> >>
>> >>  3) It looks like modifiers support will land before the official
>> >> Vulkan
>> >> extensions get finished.  It will have to be ported to the official
>> >> extensions.
>> >>
>> >>  4) Figure out what to do about the fence in AcquireNextImage. In a
>> >> future
>> >> world of explicit synchronization, we can just import the sync_file
>> >> from X or the Wayland compositor, but with implicit sync like we
>> >> have
>> >> today, it's a bit harder.  Right now, the helper in wsi_common does
>> >> nothing with it and trusts the caller to handle it.
>> >>
>> >> The two drivers handle this differently today.  In anv, we do a
>> >> dummy
>> >> QueueSubmit to trigger the fence while radv triggers it a bit more
>> >> manually.  In both cases, we trigger the fence immediately and
>> >> trust
>> >> in
>> >> the GEM's implicit synchronization to sort things out for us.  We
>> >> can't
>> >> use the anv method as directly with radv because no queue is passed
>> >> in
>> >> so we don't know what queue to use in the dummy QueueSubmit.  (In
>> >> ANV,
>> >> we only have the one queue so that isn't a problem.)
>> >>
>> >>
>> >> Dave, I tried to pull patches from your series where practical but,
>> >> because
>> >> we did things in a different order, it frequently wasn't.  If you want
>> >> to
>> >> claim credit for any of these patches, just 

Re: [Mesa-dev] [PATCH 00/28] vulkan/wsi: Rework WSI to look a lot more like a layer

[Grazvydas Ignotas]

I've tested this branch (rx470 + hd530) and it's only partially working:
- display on amd, radv: always fails with "offscreen:
wsi/wsi_common.c:172: select_memory_type: Assertion `!"No memory type
found"' failed."
- display on amd, anv: misrenders. For SaschaWillems, only
multisampling, subpasses and triangle renders, other demos show black
window with only text (fps counter and help) visible
- display on intel, radv: same "No memory type found"'
- display on intel, anv: seems to be ok

Gražvydas

On Fri, Nov 17, 2017 at 4:44 AM, Jason Ekstrand  wrote:
> I just force-pushed my branch with some changes as per Dave to more
> explicitly enable implicit sync when allocating memory objects.
>
> On Thu, Nov 16, 2017 at 1:28 PM, Jason Ekstrand 
> wrote:
>>
>> This patch series is the combined brain-child of Dave and myself.  The
>> objective is to rewrite Vulkan WSI to look as much like a layer as
>> possible
>> and to reduce the driver <-> WSI interface.  We try very hard to have as
>> many of the WSI details as possible in common code and to use standard
>> Vulkan interfaces for everything.  Among other things, this means that
>> prime support is now implemented in an entirely driver-agnostic way and
>> the
>> driver doesn't even know it's happening.  As a side-effect anv now has
>> prime support.
>>
>> Eventually, someone could pull what's left out into a proper layer and we
>> could drop WSI support from our drivers entirely.  There are a fiew pieces
>> of work that would be required to do this:
>>
>>  1) Write all of the annoying layer bits.  There are some short-cuts that
>> we can take because we're not in a layer and those will have to go.
>>
>>  2) Define a VK_MESA_legacy_swapchain_image extension to replace the hack
>> introduced in patch 8.
>>
>>  3) It looks like modifiers support will land before the official Vulkan
>> extensions get finished.  It will have to be ported to the official
>> extensions.
>>
>>  4) Figure out what to do about the fence in AcquireNextImage. In a future
>> world of explicit synchronization, we can just import the sync_file
>> from X or the Wayland compositor, but with implicit sync like we have
>> today, it's a bit harder.  Right now, the helper in wsi_common does
>> nothing with it and trusts the caller to handle it.
>>
>> The two drivers handle this differently today.  In anv, we do a dummy
>> QueueSubmit to trigger the fence while radv triggers it a bit more
>> manually.  In both cases, we trigger the fence immediately and trust
>> in
>> the GEM's implicit synchronization to sort things out for us.  We
>> can't
>> use the anv method as directly with radv because no queue is passed in
>> so we don't know what queue to use in the dummy QueueSubmit.  (In ANV,
>> we only have the one queue so that isn't a problem.)
>>
>>
>> Dave, I tried to pull patches from your series where practical but,
>> because
>> we did things in a different order, it frequently wasn't.  If you want to
>> claim credit for any of these patches, just say so and I'll --reset-author
>> on them.
>>
>> The series can be found on freedesktop.org here:
>>
>> https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=wip/vulkan-wsi-prime
>>
>>
>> Cc: Dave Airlie 
>> Cc: Daniel Stone 
>> Cc: Chad Versace 
>> Cc: James Jones 
>>
>> Daniel Stone (1):
>>   vulkan/wsi: Add a wsi_image structure
>>
>> Dave Airlie (4):
>>   vulkan/wsi: use function ptr definitions from the spec.
>>   radv/wsi: drop allocate memory special case
>>   radv/wsi: Move the guts of QueuePresent to wsi common
>>   vulkan/wsi: move swapchain create/destroy to common code
>>
>> Jason Ekstrand (23):
>>   vulkan/wsi/x11: Handle the geometry check earlier in create_swapchain
>>   vulkan/wsi: Add a wsi_device_init function
>>   vulkan/wsi: Add wsi_swapchain_init/finish functions
>>   vulkan/wsi: Implement prime in a completely generic way
>>   anv/image: Add a return value to bind_memory_plane
>>   vulkan/wsi: Add a mock image creation extension
>>   anv/image: Implement the wsi "extension"
>>   radv/image: Implement the wsi "extension"
>>   vulkan/wsi: Do image creation in common code
>>   vulkan/wsi: Add a WSI_FROM_HANDLE macro
>>   vulkan/wsi: Refactor result handling in queue_present
>>   vulkan/wsi: Only wait on semaphores on the first swapchain
>>   vulkan/wsi: Set a proper pWaitDstStageMask on the dummy submit
>>   anv/wsi: Use the common QueuePresent code
>>   anv/wsi: Enable prime support
>>   vulkan/wsi: Move get_images into common code
>>   vulkan/wsi: Move prime blitting into queue_present
>>   vulkan/wsi: Add a helper for AcquireNextImage
>>   vulkan/wsi: Move wsi_swapchain to wsi_common_private.h
>>   vulkan/wsi: Drop the can_handle_different_gpu parameter from
>> get_support
>>   vulkan/wsi: Add wrappers for all of the surface queries
>>   

Re: [Mesa-dev] [PATCH 3/3] RFC: vulkan/wsi: Rework the way prime support works

[Grazvydas Ignotas]

On Thu, Nov 16, 2017 at 12:33 AM, Dave Airlie  wrote:
> On 15 November 2017 at 04:40, Jason Ekstrand  wrote:
>> This commit significantly reworks the way prime support works and lets
>> us pull it even further into radv.  The old mechanism required the
>> specific WSI layer to be aware of the linear shadow copy that has to be
>> done in order for prime to work.  In the new paradigm, we better define
>> what bits of wsi_image go to the client and what bits go off to the
>> window system.  It's then the job of the driver to allocate two separate
>> images and stash whatever intermediates it needs in driver_private.
>> There are a few advantages to this method:
>>
>>  1) It separates supporting prime from the driver decision as to whether
>> it's better to render directly into the window-system-compatible
>> image or if it's better to blit.
>>
>>  2) Because of this separation, it's now possible for a driver to use a
>> different scheme for WSI image presentation where it hooks the
>> vkCmdPipelineBarrier that transitions the image to
>> VK_IMAGE_LAYOUT_PRESENT_SRC_KHR and does the blit there.
>>
>>  3) It lets us pull more of the details into radv and, in my opinion,
>> actually makes the radv code more straightforward.
>
> For the record, PRIME is not radv specific, stop trying to make it so,
> anv should support display to other GPUs.

Yes it would be great if that worked, would make radv vs anv testing
so much easier as I much prefer my display to be on dGPU.

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

Re: [Mesa-dev] [PATCH v3 32/34] disk_cache: Fix issue reading GLSL metadata

[Grazvydas Ignotas]

On Tue, Oct 31, 2017 at 12:54 AM, Kenneth Graunke  wrote:
> On Sunday, October 22, 2017 1:01:40 PM PDT Jordan Justen wrote:
>> This would cause the read of the metadata content to fail, which would
>> prevent the linking from being skipped.
>>
>> Seen on Rocket League with i965 shader cache.
>>
>> Cc: Timothy Arceri 
>> Signed-off-by: Jordan Justen 
>> Reviewed-by: Timothy Arceri 
>> ---
>>  src/util/disk_cache.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
>> index e38cacb259b..fde6e2e0974 100644
>> --- a/src/util/disk_cache.c
>> +++ b/src/util/disk_cache.c
>> @@ -1110,7 +1110,7 @@ disk_cache_get(struct disk_cache *cache, const 
>> cache_key key, size_t *size)
>> * TODO: pass the metadata back to the caller and do some basic
>> * validation.
>> */
>> -  cache_item_md_size += sizeof(cache_key);
>> +  cache_item_md_size += num_keys * sizeof(cache_key);
>>ret = lseek(fd, num_keys * sizeof(cache_key), SEEK_CUR);
>>if (ret == -1)
>>   goto fail;
>>
>
> Reviewed-by: Kenneth Graunke 

Fixes: b86ecea3446e "util/disk_cache: write cache item metadata to disk"
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/7] u_queue: add a futex-based implementation of fences

[Grazvydas Ignotas]

On Mon, Oct 23, 2017 at 4:04 PM, Nicolai Hähnle <nicolai.haeh...@amd.com> wrote:
> On 23.10.2017 13:50, Grazvydas Ignotas wrote:
>>
>> On Sun, Oct 22, 2017 at 9:33 PM, Nicolai Hähnle <nhaeh...@gmail.com>
>> wrote:
>>>
>>> From: Nicolai Hähnle <nicolai.haeh...@amd.com>
>>>
>>> Fences are now 4 bytes instead of 96 bytes (on my 64-bit system).
>>>
>>> Signaling a fence is a single atomic operation in the fast case plus a
>>> syscall in the slow case.
>>>
>>> Testing if a fence is signaled is the same as before (a simple
>>> comparison),
>>> but waiting on a fence is now no more expensive than just testing it in
>>> the fast (already signaled) case.
>>> ---
>>>   src/util/futex.h   |  4 +++
>>>   src/util/u_queue.c |  2 ++
>>>   src/util/u_queue.h | 88
>>> ++
>>>   3 files changed, 94 insertions(+)
>>>
>>> ...
>>>
>>> diff --git a/src/util/u_queue.h b/src/util/u_queue.h
>>> index a3e12260e30..3d9f19f4e6c 100644
>>> --- a/src/util/u_queue.h
>>> +++ b/src/util/u_queue.h
>>> @@ -28,30 +28,117 @@
>>>*
>>>* Jobs can be added from any thread. After that, the wait call can be
>>> used
>>>* to wait for completion of the job.
>>>*/
>>>
>>>   #ifndef U_QUEUE_H
>>>   #define U_QUEUE_H
>>>
>>>   #include 
>>>
>>> +#include "util/futex.h"
>>>   #include "util/list.h"
>>> +#include "util/macros.h"
>>>   #include "util/u_thread.h"
>>>
>>>   #ifdef __cplusplus
>>>   extern "C" {
>>>   #endif
>>>
>>>   #define UTIL_QUEUE_INIT_USE_MINIMUM_PRIORITY  (1 << 0)
>>>   #define UTIL_QUEUE_INIT_RESIZE_IF_FULL(1 << 1)
>>>
>>> +#if defined(__GNUC__) && defined(HAVE_FUTEX)
>>> +#define UTIL_QUEUE_FENCE_FUTEX
>>> +#else
>>> +#define UTIL_QUEUE_FENCE_STANDARD
>>> +#endif
>>> +
>>> +#ifdef UTIL_QUEUE_FENCE_FUTEX
>>> +/* Job completion fence.
>>> + * Put this into your job structure.
>>> + */
>>> +struct util_queue_fence {
>>> +   /* The fence can be in one of three states:
>>> +*  0 - signaled
>>> +*  1 - unsignaled
>>> +*  2 - unsignaled, may have waiters
>>> +*/
>>> +   uint32_t val;
>>> +};
>>> +
>>> +static inline void
>>> +util_queue_fence_init(struct util_queue_fence *fence)
>>> +{
>>> +   fence->val = 0;
>>> +}
>>> +
>>> +static inline void
>>> +util_queue_fence_destroy(struct util_queue_fence *fence)
>>> +{
>>> +   assert(fence->val == 0);
>>> +   /* no-op */
>>> +}
>>> +
>>> +static inline void
>>> +util_queue_fence_wait(struct util_queue_fence *fence)
>>> +{
>>> +   uint32_t v = fence->val;
>>> +
>>> +   if (likely(v == 0))
>>> +  return;
>>> +
>>> +   do {
>>> +  if (v != 2)
>>> + v = __sync_val_compare_and_swap(>val, 1, 2);
>>> +
>>> +  futex_wait(>val, 2);
>>> +  v = fence->val;
>>> +   } while(v != 0);
>>> +}
>>> +
>>> +static inline void
>>> +util_queue_fence_signal(struct util_queue_fence *fence)
>>> +{
>>> +   uint32_t val = __sync_lock_test_and_set(>val, 0);
>>
>>
>> As this is _signal(), don't you need a full barrier here?
>
>
> You're right. It's a bit surprising that __sync_lock_test_and_set isn't one.
>
>
>>> +
>>> +   assert(val != 0);
>>> +
>>> +   if (val == 2)
>>
>>
>> The documentation says some architectures may only support a constant
>> of 1 here...
>
>
> Again, surprising, but you're right.
>
> I believe both of these could be addressed by using
> __sync_val_compare_and_swap instead, right?

I believe so too.

> Or we could move to new-style gcc atomic built-ins, but I'm not sure
> off-hand how widely available those are.

There's u_atomic.h that already uses some __atomic_*() built-ins,
p_atomic_cmpxchg is just __sync_val_compare_and_swap but could use it
for consistency with other mesa code.

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

Re: [Mesa-dev] [PATCH 7/7] radeonsi: reduce the scope of sel->mutex in si_shader_select_with_key

[Grazvydas Ignotas]

On Sun, Oct 22, 2017 at 9:33 PM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> We only need the lock to guard changes in the variant linked list. The
> actual compilation can happen outside the lock, since we use the ready
> fence as a guard.
> ---
>  src/gallium/drivers/radeonsi/si_state_shaders.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c 
> b/src/gallium/drivers/radeonsi/si_state_shaders.c
> index 8c589928b8c..d0bef09748f 100644
> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
> @@ -1618,41 +1618,42 @@ current_not_ready:
> if (thread_index < 0)
> util_queue_fence_wait(>ready);
>
> mtx_lock(>mutex);
>
> /* Find the shader variant. */
> for (iter = sel->first_variant; iter; iter = iter->next_variant) {
> /* Don't check the "current" shader. We checked it above. */
> if (current != iter &&
> memcmp(>key, key, sizeof(*key)) == 0) {
> +   mtx_unlock(>mutex);
> +
> if 
> (unlikely(!util_queue_fence_is_signalled(>ready))) {
> /* If it's an optimized shader and its 
> compilation has
>  * been started but isn't done, use the 
> unoptimized
>  * shader so as not to cause a stall due to 
> compilation.
>  */
> if (iter->is_optimized) {
> memset(>opt, 0, 
> sizeof(key->opt));
> mtx_unlock(>mutex);

Double unlock

> goto again;
> }
>
> util_queue_fence_wait(>ready);
> }
>
> if (iter->compilation_failed) {
> mtx_unlock(>mutex);

here too.

> return -1; /* skip the draw call */
> }
>
> state->current = iter;
> -   mtx_unlock(>mutex);
> return 0;
> }
> }
>
> /* Build a new shader. */
> shader = CALLOC_STRUCT(si_shader);
> if (!shader) {
> mtx_unlock(>mutex);
> return -ENOMEM;
> }

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

Re: [Mesa-dev] [PATCH 5/7] u_queue: add a futex-based implementation of fences

[Grazvydas Ignotas]

On Sun, Oct 22, 2017 at 9:33 PM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> Fences are now 4 bytes instead of 96 bytes (on my 64-bit system).
>
> Signaling a fence is a single atomic operation in the fast case plus a
> syscall in the slow case.
>
> Testing if a fence is signaled is the same as before (a simple comparison),
> but waiting on a fence is now no more expensive than just testing it in
> the fast (already signaled) case.
> ---
>  src/util/futex.h   |  4 +++
>  src/util/u_queue.c |  2 ++
>  src/util/u_queue.h | 88 
> ++
>  3 files changed, 94 insertions(+)
>
> ...
>
> diff --git a/src/util/u_queue.h b/src/util/u_queue.h
> index a3e12260e30..3d9f19f4e6c 100644
> --- a/src/util/u_queue.h
> +++ b/src/util/u_queue.h
> @@ -28,30 +28,117 @@
>   *
>   * Jobs can be added from any thread. After that, the wait call can be used
>   * to wait for completion of the job.
>   */
>
>  #ifndef U_QUEUE_H
>  #define U_QUEUE_H
>
>  #include 
>
> +#include "util/futex.h"
>  #include "util/list.h"
> +#include "util/macros.h"
>  #include "util/u_thread.h"
>
>  #ifdef __cplusplus
>  extern "C" {
>  #endif
>
>  #define UTIL_QUEUE_INIT_USE_MINIMUM_PRIORITY  (1 << 0)
>  #define UTIL_QUEUE_INIT_RESIZE_IF_FULL(1 << 1)
>
> +#if defined(__GNUC__) && defined(HAVE_FUTEX)
> +#define UTIL_QUEUE_FENCE_FUTEX
> +#else
> +#define UTIL_QUEUE_FENCE_STANDARD
> +#endif
> +
> +#ifdef UTIL_QUEUE_FENCE_FUTEX
> +/* Job completion fence.
> + * Put this into your job structure.
> + */
> +struct util_queue_fence {
> +   /* The fence can be in one of three states:
> +*  0 - signaled
> +*  1 - unsignaled
> +*  2 - unsignaled, may have waiters
> +*/
> +   uint32_t val;
> +};
> +
> +static inline void
> +util_queue_fence_init(struct util_queue_fence *fence)
> +{
> +   fence->val = 0;
> +}
> +
> +static inline void
> +util_queue_fence_destroy(struct util_queue_fence *fence)
> +{
> +   assert(fence->val == 0);
> +   /* no-op */
> +}
> +
> +static inline void
> +util_queue_fence_wait(struct util_queue_fence *fence)
> +{
> +   uint32_t v = fence->val;
> +
> +   if (likely(v == 0))
> +  return;
> +
> +   do {
> +  if (v != 2)
> + v = __sync_val_compare_and_swap(>val, 1, 2);
> +
> +  futex_wait(>val, 2);
> +  v = fence->val;
> +   } while(v != 0);
> +}
> +
> +static inline void
> +util_queue_fence_signal(struct util_queue_fence *fence)
> +{
> +   uint32_t val = __sync_lock_test_and_set(>val, 0);

As this is _signal(), don't you need a full barrier here?

> +
> +   assert(val != 0);
> +
> +   if (val == 2)

The documentation says some architectures may only support a constant
of 1 here...

> +  futex_wake_all(>val);
> +}

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

Re: [Mesa-dev] [PATCH 1/2] configure: check if -latomic is needed for __atomic_*

[Grazvydas Ignotas]

On Tue, Sep 19, 2017 at 2:04 PM, Emil Velikov <emil.l.veli...@gmail.com> wrote:
> On 19 September 2017 at 10:12, Grazvydas Ignotas <nota...@gmail.com> wrote:
>> On Mon, Sep 18, 2017 at 11:30 PM, Matt Turner <matts...@gmail.com> wrote:
>>> On Mon, Sep 18, 2017 at 12:28 PM, Grazvydas Ignotas <nota...@gmail.com> 
>>> wrote:
>>>> On some platforms, gcc generates library calls when __atomic_* functions
>>>> are used, but does not link the required library automatically. Detect
>>>> this and add the library when needed.
>>>>
>>>> This change was tested on armel (library was added) and on x86_64 (was
>>>> not, as expected).
>>>>
> On one hand I see the reason why things are as-is. On the other its
> rather odd that I couldn't find an official GCC doc that describes the
> lot.
> Most likely I failed at searching.

I can't find it either. there is only this wiki that looks more like a
design document:
https://gcc.gnu.org/wiki/Atomic/GCCMM

If I understand it right, when gcc can't do the operations with
instructions it emits calls, but doesn't care how they are satisfied
and leaves dealing with that to the application. libatomic is provided
since 4.8, but there is no requirement to use it (to allow using
something else?).

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

Re: [Mesa-dev] [PATCH 1/2] configure: check if -latomic is needed for __atomic_*

[Grazvydas Ignotas]

On Mon, Sep 18, 2017 at 11:30 PM, Matt Turner <matts...@gmail.com> wrote:
> On Mon, Sep 18, 2017 at 12:28 PM, Grazvydas Ignotas <nota...@gmail.com> wrote:
>> On some platforms, gcc generates library calls when __atomic_* functions
>> are used, but does not link the required library automatically. Detect
>> this and add the library when needed.
>>
>> This change was tested on armel (library was added) and on x86_64 (was
>> not, as expected).
>>
>> Fixes: 8915f0c0 "util: use GCC atomic intrinsics with explicit memory model"
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102573
>> Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
>> ---
>>  configure.ac | 13 +
>>  src/util/Makefile.am |  3 ++-
>>  2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 605c9b4..b46f29d 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -377,12 +377,25 @@ int main() {
>>  int n;
>>  return __atomic_load_n(, __ATOMIC_ACQUIRE);
>>  }]])], GCC_ATOMIC_BUILTINS_SUPPORTED=1)
>>  if test "x$GCC_ATOMIC_BUILTINS_SUPPORTED" = x1; then
>>  DEFINES="$DEFINES -DUSE_GCC_ATOMIC_BUILTINS"
>> +dnl On some platforms, new-style atomics need a helper library
>> +AC_MSG_CHECKING(whether -latomic is needed)
>> +AC_LINK_IFELSE([AC_LANG_SOURCE([[
>> +#include 
>> +uint64_t v;
>> +int main() {
>> +return (int)__atomic_load_n(, __ATOMIC_ACQUIRE);
>> +}]])], GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC=no, 
>> GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC=yes)
>> +AC_MSG_RESULT($GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC)
>> +if test "x$GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC" = xyes; then
>> +LIBATOMIC_LIBS="-latomic"
>> +fi
>>  fi
>>  AM_CONDITIONAL([GCC_ATOMIC_BUILTINS_SUPPORTED], [test 
>> x$GCC_ATOMIC_BUILTINS_SUPPORTED = x1])
>> +AC_SUBST([LIBATOMIC_LIBS])
>>
>>  dnl Check if host supports 64-bit atomics
>>  dnl note that lack of support usually results in link (not compile) error
>>  AC_MSG_CHECKING(whether __sync_add_and_fetch_8 is supported)
>>  AC_LINK_IFELSE([AC_LANG_SOURCE([[
>> diff --git a/src/util/Makefile.am b/src/util/Makefile.am
>> index 9885bbe..c37afff 100644
>> --- a/src/util/Makefile.am
>> +++ b/src/util/Makefile.am
>> @@ -46,11 +46,12 @@ libmesautil_la_SOURCES = \
>> $(MESA_UTIL_FILES) \
>> $(MESA_UTIL_GENERATED_FILES)
>>
>>  libmesautil_la_LIBADD = \
>> $(CLOCK_LIB) \
>> -   $(ZLIB_LIBS)
>> +   $(ZLIB_LIBS) \
>> +   $(LIBATOMIC_LIBS)
>
> I can't remember how this works -- will $(LIBATOMIC_LIBS) be added
> transitively to anything that LDADDs libmesautil? I hope so...

I'm not sure either, I've simply tried it and it seemed to work. Maybe
Emil can comment if it's the right way...

> Oh, also, these should get a Cc: mesa-stable tag.

It has been said (and documented in docs/submittingpatches.html) that
Fixes: tag is enough for stable, and 2/2 it's strictly necessary I
think.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] r600: fork and import gallium/radeon

[Grazvydas Ignotas]

Has anyone ran full piglit on these with r600? Copy-paste bugs are
very easy to introduce.

Gražvydas

On Mon, Sep 18, 2017 at 7:32 PM, Marek Olšák  wrote:
> Latest version with an updated commit message:
> https://cgit.freedesktop.org/~mareko/mesa/commit/?h=master=8671613ed19586ac1eceda48eb067a416f9c93be
>
> Marek
>
> On Mon, Sep 18, 2017 at 6:18 PM, Emil Velikov  
> wrote:
>> On 18 September 2017 at 17:08, Marek Olšák  wrote:
>>> On Thu, Sep 14, 2017 at 4:52 PM, Nicolai Hähnle  wrote:
 On 14.09.2017 16:19, Emil Velikov wrote:
>
> Hi Marek,
>
> On 14 September 2017 at 14:06, Marek Olšák  wrote:
>>
>> From: Marek Olšák 
>>
>> This marks the end of code sharing between r600 and radeonsi.
>>
> It has the "what" but it's missing the "why". Can you please add some
> information.


 Basically, it's getting difficult to work on radeonsi without breaking 
 r600.

 It's not ideal, but (1) without a solid testing infrastructure that tests
 all the way back to r600, this is a reasonable measure to reduce the
 development risk, and (2) with some rare exceptions, our work on radeonsi
 tends not to help r600, neither in performance nor in features (and least 
 of
 all bug fixes).

 I haven't looked at the patch itself yet, but I'm okay with the general
 idea.
>>>
>>> Does it mean I can push this?
>>>
>> I've got a local implementation that I'm fuzzing as we speak.
>> Can you please give me a bit of time to check for explosions.
>>
>> Meanwhile if someone can provide something we could add in the commit
>> message ("why") that will be appreciated.
>>
>> -Emil
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] util/u_atomic: remove unnecessaty __atomic functions

[Grazvydas Ignotas]

They are now provided by -latomic, which should be linked as needed
since previous commit.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/util/u_atomic.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/src/util/u_atomic.c b/src/util/u_atomic.c
index b32527f..e4bffa8 100644
--- a/src/util/u_atomic.c
+++ b/src/util/u_atomic.c
@@ -72,18 +72,6 @@ __sync_val_compare_and_swap_8(uint64_t *ptr, uint64_t 
oldval, uint64_t newval)
pthread_mutex_unlock(_mutex);
 
return r;
 }
 
-WEAK uint64_t
-__atomic_fetch_add_8(uint64_t *ptr, uint64_t val, int memorder)
-{
-   return __sync_add_and_fetch(ptr, val);
-}
-
-WEAK uint64_t
-__atomic_fetch_sub_8(uint64_t *ptr, uint64_t val, int memorder)
-{
-   return __sync_sub_and_fetch(ptr, val);
-}
-
 #endif
-- 
2.7.4

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

[Mesa-dev] [PATCH 1/2] configure: check if -latomic is needed for __atomic_*

[Grazvydas Ignotas]

On some platforms, gcc generates library calls when __atomic_* functions
are used, but does not link the required library automatically. Detect
this and add the library when needed.

This change was tested on armel (library was added) and on x86_64 (was
not, as expected).

Fixes: 8915f0c0 "util: use GCC atomic intrinsics with explicit memory model"
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102573
Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 configure.ac | 13 +
 src/util/Makefile.am |  3 ++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 605c9b4..b46f29d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -377,12 +377,25 @@ int main() {
 int n;
 return __atomic_load_n(, __ATOMIC_ACQUIRE);
 }]])], GCC_ATOMIC_BUILTINS_SUPPORTED=1)
 if test "x$GCC_ATOMIC_BUILTINS_SUPPORTED" = x1; then
 DEFINES="$DEFINES -DUSE_GCC_ATOMIC_BUILTINS"
+dnl On some platforms, new-style atomics need a helper library
+AC_MSG_CHECKING(whether -latomic is needed)
+AC_LINK_IFELSE([AC_LANG_SOURCE([[
+#include 
+uint64_t v;
+int main() {
+return (int)__atomic_load_n(, __ATOMIC_ACQUIRE);
+}]])], GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC=no, 
GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC=yes)
+AC_MSG_RESULT($GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC)
+if test "x$GCC_ATOMIC_BUILTINS_NEED_LIBATOMIC" = xyes; then
+LIBATOMIC_LIBS="-latomic"
+fi
 fi
 AM_CONDITIONAL([GCC_ATOMIC_BUILTINS_SUPPORTED], [test 
x$GCC_ATOMIC_BUILTINS_SUPPORTED = x1])
+AC_SUBST([LIBATOMIC_LIBS])
 
 dnl Check if host supports 64-bit atomics
 dnl note that lack of support usually results in link (not compile) error
 AC_MSG_CHECKING(whether __sync_add_and_fetch_8 is supported)
 AC_LINK_IFELSE([AC_LANG_SOURCE([[
diff --git a/src/util/Makefile.am b/src/util/Makefile.am
index 9885bbe..c37afff 100644
--- a/src/util/Makefile.am
+++ b/src/util/Makefile.am
@@ -46,11 +46,12 @@ libmesautil_la_SOURCES = \
$(MESA_UTIL_FILES) \
$(MESA_UTIL_GENERATED_FILES)
 
 libmesautil_la_LIBADD = \
$(CLOCK_LIB) \
-   $(ZLIB_LIBS)
+   $(ZLIB_LIBS) \
+   $(LIBATOMIC_LIBS)
 
 libxmlconfig_la_SOURCES = $(XMLCONFIG_FILES)
 libxmlconfig_la_CFLAGS = \
$(DEFINES) \
-I$(top_srcdir)/include \
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 2/2] util/u_atomic: Add implementation of __sync_val_compare_and_swap_8

[Grazvydas Ignotas]

On Fri, Sep 15, 2017 at 12:04 PM, Grazvydas Ignotas <nota...@gmail.com> wrote:
> It was because on some platforms ./configure test looking for
> __atomic_* passes because it's a compile (not link) test, even when
> the compiler inserts library calls instead of actual implementation.
> It looks like the correct solution is to link against libatomic (which
> I was not aware of at that time) and drop these from u_atomic.c (amend
> it to 1/2?).

Whoops somehow I misread 1/2 as adding -latomic , so ignore the
"amend" part of this comment. It probably came as a result of thinking
about bug 102573 and some lack of sleep.

>
> Gražvydas
>
>
> On Thu, Sep 14, 2017 at 10:08 PM, Matt Turner <matts...@gmail.com> wrote:
>> Grazvydas,
>>
>> I noticed that there are some __atomic functions in this file, but I'm
>> not sure what they do or why they're necessary. Remind me?
>>
>> Thanks,
>> Matt
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] util/u_atomic: Add implementation of __sync_val_compare_and_swap_8

[Grazvydas Ignotas]

It was because on some platforms ./configure test looking for
__atomic_* passes because it's a compile (not link) test, even when
the compiler inserts library calls instead of actual implementation.
It looks like the correct solution is to link against libatomic (which
I was not aware of at that time) and drop these from u_atomic.c (amend
it to 1/2?).

Gražvydas


On Thu, Sep 14, 2017 at 10:08 PM, Matt Turner  wrote:
> Grazvydas,
>
> I noticed that there are some __atomic functions in this file, but I'm
> not sure what they do or why they're necessary. Remind me?
>
> Thanks,
> Matt
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] radv: initialize the trace BO to 0

[Grazvydas Ignotas]

Wouldn't it be better to set AMDGPU_GEM_CREATE_VRAM_CLEARED when
creating the bo? That would need new RADEON_FLAG_* for winsys.

If not, at least make 4096 a define to use for both
ws->buffer_create() and memset.

Gražvydas

On Wed, Sep 13, 2017 at 12:26 PM, Samuel Pitoiset
 wrote:
> To avoid random initial values.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/amd/vulkan/radv_debug.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/amd/vulkan/radv_debug.c b/src/amd/vulkan/radv_debug.c
> index c004bc436b..1e5c578aae 100644
> --- a/src/amd/vulkan/radv_debug.c
> +++ b/src/amd/vulkan/radv_debug.c
> @@ -65,6 +65,8 @@ radv_init_trace(struct radv_device *device)
> if (!device->trace_id_ptr)
> return false;
>
> +   memset(device->trace_id_ptr, 0, 4096);
> +
> ac_vm_fault_occured(device->physical_device->rad_info.chip_class,
> >dmesg_timestamp, NULL);
>
> --
> 2.14.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] radv: clear dynamic_shader_stages on create

[Grazvydas Ignotas]

Valgrind reports it's being used uninitialized.

Fixes: f4e499ec791 "radv: add initial non-conformant radv vulkan driver"
Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/amd/vulkan/radv_descriptor_set.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/amd/vulkan/radv_descriptor_set.c 
b/src/amd/vulkan/radv_descriptor_set.c
index 7cee9d4..314ab5a 100644
--- a/src/amd/vulkan/radv_descriptor_set.c
+++ b/src/amd/vulkan/radv_descriptor_set.c
@@ -64,10 +64,11 @@ VkResult radv_CreateDescriptorSetLayout(
/* We just allocate all the samplers at the end of the struct */
uint32_t *samplers = (uint32_t*)_layout->binding[max_binding + 1];
 
set_layout->binding_count = max_binding + 1;
set_layout->shader_stages = 0;
+   set_layout->dynamic_shader_stages = 0;
set_layout->size = 0;
 
memset(set_layout->binding, 0, size - sizeof(struct 
radv_descriptor_set_layout));
 
uint32_t buffer_count = 0;
-- 
2.7.4

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

[Mesa-dev] [PATCH] radv: don't assert on empty hash table

[Grazvydas Ignotas]

Currently if table_size is 0, it's falling through to:

unreachable("hash table should never be full");

But table_size can be 0 when RADV_DEBUG=nocache is set, or when the
table allocation fails (which is not considered an error).

Fixes: f4e499ec791 "radv: add initial non-conformant radv vulkan driver"
Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/amd/vulkan/radv_pipeline_cache.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/amd/vulkan/radv_pipeline_cache.c 
b/src/amd/vulkan/radv_pipeline_cache.c
index 99a614d..beed35b 100644
--- a/src/amd/vulkan/radv_pipeline_cache.c
+++ b/src/amd/vulkan/radv_pipeline_cache.c
@@ -116,10 +116,13 @@ radv_pipeline_cache_search_unlocked(struct 
radv_pipeline_cache *cache,
const unsigned char *sha1)
 {
const uint32_t mask = cache->table_size - 1;
const uint32_t start = (*(uint32_t *) sha1);
 
+   if (cache->table_size == 0)
+   return NULL;
+
for (uint32_t i = 0; i < cache->table_size; i++) {
const uint32_t index = (start + i) & mask;
struct cache_entry *entry = cache->hash_table[index];
 
if (!entry)
-- 
2.7.4

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

[Mesa-dev] [PATCH] amd: add .editorconfig

[Grazvydas Ignotas]

amd/common/ and amd/vulkan/ are using tabs for indent, which doesn't
match the settings in root .editorconfig, so let's override.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/amd/common/.editorconfig | 3 +++
 src/amd/vulkan/.editorconfig | 3 +++
 2 files changed, 6 insertions(+)
 create mode 100644 src/amd/common/.editorconfig
 create mode 100644 src/amd/vulkan/.editorconfig

diff --git a/src/amd/common/.editorconfig b/src/amd/common/.editorconfig
new file mode 100644
index 000..21a3c7d
--- /dev/null
+++ b/src/amd/common/.editorconfig
@@ -0,0 +1,3 @@
+[*.{c,h}]
+indent_style = tab
+indent_size = tab
diff --git a/src/amd/vulkan/.editorconfig b/src/amd/vulkan/.editorconfig
new file mode 100644
index 000..21a3c7d
--- /dev/null
+++ b/src/amd/vulkan/.editorconfig
@@ -0,0 +1,3 @@
+[*.{c,h}]
+indent_style = tab
+indent_size = tab
-- 
2.7.4

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

[Mesa-dev] [PATCH] ac/nir: silence maybe-uninitialized warnings

[Grazvydas Ignotas]

These are likely false positives, but are also annoying because they
show up on every "make install", which causes ac_nir_to_llvm to be
rebuilt here. Initializing those variables to NULL should be harmless
even when unnecessary.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/amd/common/ac_nir_to_llvm.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 664d83b..823ea42 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -1332,11 +1332,11 @@ static LLVMValueRef emit_i2b(struct ac_llvm_context 
*ctx,
 
 static LLVMValueRef emit_f2f16(struct nir_to_llvm_context *ctx,
   LLVMValueRef src0)
 {
LLVMValueRef result;
-   LLVMValueRef cond;
+   LLVMValueRef cond = NULL;
 
src0 = to_float(>ac, src0);
result = LLVMBuildFPTrunc(ctx->builder, src0, ctx->f16, "");
 
if (ctx->options->chip_class >= VI) {
@@ -2052,11 +2052,11 @@ static LLVMValueRef radv_lower_gather4_integer(struct 
ac_llvm_context *ctx,
   const nir_tex_instr *instr)
 {
enum glsl_base_type stype = 
glsl_get_sampler_result_type(instr->texture->var->type);
LLVMValueRef coord = args->addr;
LLVMValueRef half_texel[2];
-   LLVMValueRef compare_cube_wa;
+   LLVMValueRef compare_cube_wa = NULL;
LLVMValueRef result;
int c;
unsigned coord_vgpr_index = (unsigned)args->offset + 
(unsigned)args->compare;
 
//TODO Rect
@@ -2778,11 +2778,12 @@ load_tcs_input(struct nir_to_llvm_context *ctx,
 
 static LLVMValueRef
 load_tcs_output(struct nir_to_llvm_context *ctx,
   nir_intrinsic_instr *instr)
 {
-   LLVMValueRef dw_addr, stride;
+   LLVMValueRef dw_addr;
+   LLVMValueRef stride = NULL;
LLVMValueRef value[4], result;
LLVMValueRef vertex_index = NULL;
LLVMValueRef indir_index = NULL;
unsigned const_index = 0;
unsigned param;
@@ -2817,11 +2818,12 @@ static void
 store_tcs_output(struct nir_to_llvm_context *ctx,
 nir_intrinsic_instr *instr,
 LLVMValueRef src,
 unsigned writemask)
 {
-   LLVMValueRef stride, dw_addr;
+   LLVMValueRef dw_addr;
+   LLVMValueRef stride = NULL;
LLVMValueRef buf_addr = NULL;
LLVMValueRef vertex_index = NULL;
LLVMValueRef indir_index = NULL;
unsigned const_index = 0;
unsigned param;
@@ -3817,12 +3819,13 @@ static LLVMValueRef visit_interp(struct 
nir_to_llvm_context *ctx,
 {
LLVMValueRef result[2];
LLVMValueRef interp_param, attr_number;
unsigned location;
unsigned chan;
-   LLVMValueRef src_c0, src_c1;
-   LLVMValueRef src0;
+   LLVMValueRef src_c0 = NULL;
+   LLVMValueRef src_c1 = NULL;
+   LLVMValueRef src0 = NULL;
int input_index = instr->variables[0]->var->data.location - 
VARYING_SLOT_VAR0;
switch (instr->intrinsic) {
case nir_intrinsic_interp_var_at_centroid:
location = INTERP_CENTROID;
break;
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH] configure: Trust LLVM >= 4.0 llvm-config --libs for shared libraries

[Grazvydas Ignotas]

On Tue, Aug 15, 2017 at 1:55 AM, Grazvydas Ignotas <nota...@gmail.com> wrote:
>
> Also, llvm-config --version says "6.0.0git-f49c401", which means I had
> to manually hack configure.ac every time to "6.0.0svn" to get mesa
> building too :(

Nevermind about this one, -DLLVM_APPEND_VC_REV=OFF solves it. Would be
great to have a "how to configure llvm for mesa" section somewhere.

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

Re: [Mesa-dev] [PATCH] configure: Trust LLVM >= 4.0 llvm-config --libs for shared libraries

[Grazvydas Ignotas]

On Mon, Aug 14, 2017 at 11:13 PM, Andy Furniss  wrote:
> Michel Dänzer wrote:
>>
>> On 11/08/17 01:45 AM, Emil Velikov wrote:
>>
>> Thanks, pushed.
>
>
> Don't know if this is expected or not, but llvm built like -
>
> cmake -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_BUILD_TYPE=Release
> -DLLVM_TARGETS_TO_BUILD="host;AMDGPU" -DLLVM_ENABLE_ASSERTIONS=ON
> -DLLVM_APPEND_VC_REV=OFF -DLLVM_BUILD_LLVM_DYLIB=ON
>
> now fails to build mesa (I also tried without vulkan and it still fails
> raseonsi IIRC).
>
>  CXXLDlibvulkan_radeon.la
> /usr/lib/libLLVMSupport.a(Process.cpp.o): In function
> `llvm::sys::Process::FileDescriptorHasColors(int)':
> Process.cpp:(.text._ZN4llvm3sys7Process23FileDescriptorHasColorsEi+0x4d):
> undefined reference to `setupterm'
> Process.cpp:(.text._ZN4llvm3sys7Process23FileDescriptorHasColorsEi+0x78):
> undefined reference to `tigetnum'
> Process.cpp:(.text._ZN4llvm3sys7Process23FileDescriptorHasColorsEi+0x81):
> undefined reference to `set_curterm'
> Process.cpp:(.text._ZN4llvm3sys7Process23FileDescriptorHasColorsEi+0x89):
> undefined reference to `del_curterm'
> collect2: error: ld returned 1 exit status
>
> Haven't tried llvm with -DLLVM_APPEND_VC_REV=ON yet, mesa still builds OK
> with this commit reverted.

Same problem here, using LLVM-6.0 from a week ago, my cmake is just
-DCMAKE_INSTALL_PREFIX=/opt/xorg -DLLVM_TARGETS_TO_BUILD='AMDGPU;X86'
-DCMAKE_BUILD_TYPE=Release -DLLVM_BUILD_LLVM_DYLIB=TRUE

llvm-config --libs
seems to return tons of output like -lLLVMBitWriter -lLLVMAnalysis
-lLLVMProfileData -lLLVMObject ... which seems to result in static
linking that's supposedly not supported by mesa? The only .so I see is
libLLVM.so , all others are .a files.

Meanwhile
llvm-config --shared-mode
returns "static", which implies it wants be statically linked. There
is BUILD_SHARED_LIBS cmake option, but llvm documentation advices
against using it and suggests LLVM_BUILD_LLVM_DYLIB instead.
Confusing...

Also, llvm-config --version says "6.0.0git-f49c401", which means I had
to manually hack configure.ac every time to "6.0.0svn" to get mesa
building too :(

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

Re: [Mesa-dev] [PATCH 2/2] i965: Guard GetBufferSubData's streaming memcpy load with USE_SSE41

[Grazvydas Ignotas]

On Fri, Aug 11, 2017 at 8:52 AM, Kenneth Graunke  wrote:
> This should hopefully fix build issues on 32-bit Android-x86.
>
> Cc: Mauro Rossi 
> Cc: Tapani Pälli 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102050
> ---
>  src/mesa/drivers/dri/i965/intel_buffer_objects.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> Mauro, hopefully this helps?
>
> diff --git a/src/mesa/drivers/dri/i965/intel_buffer_objects.c 
> b/src/mesa/drivers/dri/i965/intel_buffer_objects.c
> index ee591168283..9afd98edb87 100644
> --- a/src/mesa/drivers/dri/i965/intel_buffer_objects.c
> +++ b/src/mesa/drivers/dri/i965/intel_buffer_objects.c
> @@ -342,6 +342,7 @@ brw_get_buffer_subdata(struct gl_context *ctx,
>
> unsigned int map_flags = MAP_READ;
> mem_copy_fn memcpy_fn = memcpy;
> +#ifdef USE_SSE4_1

I don't think anything is defining USE_SSE4_1 in mesa, so you are
disabling this code for everyone.

> if (!intel_obj->buffer->cache_coherent && cpu_has_sse4_1) {
>/* Rather than acquire a new WB mmaping of the buffer object and pull
> * it into the CPU cache, keep using the WC mmap that we have for 
> writes,
> @@ -350,6 +351,7 @@ brw_get_buffer_subdata(struct gl_context *ctx,
>map_flags |= MAP_COHERENT;
>memcpy_fn = (mem_copy_fn) _mesa_streaming_load_memcpy;
> }
> +#endif
>
> void *map = brw_bo_map(brw, intel_obj->buffer, map_flags);
> if (unlikely(!map)) {
> --
> 2.14.0

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

Re: [Mesa-dev] [PATCH 1/3] gallium/util: fix unused variable warning

[Grazvydas Ignotas]

FWIW:
Reviewed-by: Grazvydas Ignotas <nota...@gmail.com>
for all 3.

On Thu, Jul 20, 2017 at 3:07 AM, Timothy Arceri <tarc...@itsqueeze.com> wrote:
> ---
>  src/gallium/auxiliary/util/u_threaded_context.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_threaded_context.c 
> b/src/gallium/auxiliary/util/u_threaded_context.c
> index cb9ea3a..a9fc670 100644
> --- a/src/gallium/auxiliary/util/u_threaded_context.c
> +++ b/src/gallium/auxiliary/util/u_threaded_context.c
> @@ -1978,9 +1978,11 @@ static void
>  tc_call_generate_mipmap(struct pipe_context *pipe, union tc_payload *payload)
>  {
> struct tc_generate_mipmap *p = (struct tc_generate_mipmap *)payload;
> -   bool result = pipe->generate_mipmap(pipe, p->res, p->format, 
> p->base_level,
> -   p->last_level, p->first_layer,
> -   p->last_layer);
> +   bool MAYBE_UNUSED result = pipe->generate_mipmap(pipe, p->res, p->format,
> +p->base_level,
> +p->last_level,
> +p->first_layer,
> +p->last_layer);
> assert(result);
> pipe_resource_reference(>res, NULL);
>  }
> --
> 2.9.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] radv: Fix descriptors for cube images with VK_IMAGE_USAGE_STORAGE_BIT

[Grazvydas Ignotas]

On Fri, Jul 21, 2017 at 11:21 AM, Alex Smith
 wrote:
>
> I don't have Doom available to test. How's it broken?

After the menu loads, it gets stuck flipping between some 2 (?)
arbitrary frames than are no longer updating.

> Could you see if removing the usage flags condition on the second call to
> radv_image_view_make_descriptor makes any difference? Otherwise I'm not sure
> what's wrong - as far as I could tell this should be making RADV do
> basically the same as what radeonsi does.

That seems to fix it, as well as reverting the whole patch (still
reverts cleanly).

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

Re: [Mesa-dev] [PATCH v2] radv: Fix descriptors for cube images with VK_IMAGE_USAGE_STORAGE_BIT

[Grazvydas Ignotas]

For whatever reason this patch is breaking DOOM.

Gražvydas

On Wed, Jul 12, 2017 at 12:29 PM, Alex Smith
 wrote:
> If a cube image has VK_IMAGE_USAGE_STORAGE_BIT set, the type in an image
> view's descriptor was set to a 2D array (and a few other fields adjusted
> accordingly). This is correct when the image view is actually bound as a
> storage image, but not when bound as a sampled image. In that case the
> type should be set as a cube.
>
> Fix by generating 2 sets of descriptors at view creation time for both
> storage and non-storage usage, and then choose between them based on
> descriptor type when writing descriptor sets.
>
> v2: Generate storage descriptors for images with TRANSFER_DST, since
> those may be used as storage images internally.
>
> Signed-off-by: Alex Smith 
> Reviewed-by: Bas Nieuwenhuizen 
> ---
>  src/amd/vulkan/radv_descriptor_set.c | 18 ++--
>  src/amd/vulkan/radv_image.c  | 79 
> 
>  src/amd/vulkan/radv_private.h|  6 +++
>  3 files changed, 74 insertions(+), 29 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_descriptor_set.c 
> b/src/amd/vulkan/radv_descriptor_set.c
> index ec7fd3d..b4a78aa 100644
> --- a/src/amd/vulkan/radv_descriptor_set.c
> +++ b/src/amd/vulkan/radv_descriptor_set.c
> @@ -603,11 +603,18 @@ write_image_descriptor(struct radv_device *device,
>struct radv_cmd_buffer *cmd_buffer,
>unsigned *dst,
>struct radeon_winsys_bo **buffer_list,
> +  VkDescriptorType descriptor_type,
>const VkDescriptorImageInfo *image_info)
>  {
> RADV_FROM_HANDLE(radv_image_view, iview, image_info->imageView);
> -   memcpy(dst, iview->descriptor, 8 * 4);
> -   memcpy(dst + 8, iview->fmask_descriptor, 8 * 4);
> +
> +   if (descriptor_type == VK_DESCRIPTOR_TYPE_STORAGE_IMAGE) {
> +   memcpy(dst, iview->storage_descriptor, 8 * 4);
> +   memcpy(dst + 8, iview->storage_fmask_descriptor, 8 * 4);
> +   } else {
> +   memcpy(dst, iview->descriptor, 8 * 4);
> +   memcpy(dst + 8, iview->fmask_descriptor, 8 * 4);
> +   }
>
> if (cmd_buffer)
> device->ws->cs_add_buffer(cmd_buffer->cs, iview->bo, 7);
> @@ -620,12 +627,13 @@ write_combined_image_sampler_descriptor(struct 
> radv_device *device,
> struct radv_cmd_buffer *cmd_buffer,
> unsigned *dst,
> struct radeon_winsys_bo **buffer_list,
> +   VkDescriptorType descriptor_type,
> const VkDescriptorImageInfo 
> *image_info,
> bool has_sampler)
>  {
> RADV_FROM_HANDLE(radv_sampler, sampler, image_info->sampler);
>
> -   write_image_descriptor(device, cmd_buffer, dst, buffer_list, 
> image_info);
> +   write_image_descriptor(device, cmd_buffer, dst, buffer_list, 
> descriptor_type, image_info);
> /* copy over sampler state */
> if (has_sampler)
> memcpy(dst + 16, sampler->state, 16);
> @@ -696,10 +704,12 @@ void radv_update_descriptor_sets(
> case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE:
> case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT:
> write_image_descriptor(device, cmd_buffer, 
> ptr, buffer_list,
> +  
> writeset->descriptorType,
>writeset->pImageInfo + 
> j);
> break;
> case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER:
> 
> write_combined_image_sampler_descriptor(device, cmd_buffer, ptr, buffer_list,
> +   
> writeset->descriptorType,
> 
> writeset->pImageInfo + j,
> 
> !binding_layout->immutable_samplers_offset);
> if (copy_immutable_samplers) {
> @@ -866,10 +876,12 @@ void radv_update_descriptor_set_with_template(struct 
> radv_device *device,
> case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE:
> case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT:
> write_image_descriptor(device, cmd_buffer, 
> pDst, buffer_list,
> +  
> templ->entry[i].descriptor_type,
>(struct 
> VkDescriptorImageInfo *) pSrc);
> break;
>

Re: [Mesa-dev] [PATCH 8/9] anv: don't use strcpy for copying strings

[Grazvydas Ignotas]

On Thu, Jul 13, 2017 at 8:09 PM, Lionel Landwerlin
 wrote:
> CID: 1358935
> Signed-off-by: Lionel Landwerlin 
> ---
>  src/intel/vulkan/anv_device.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index 39c907e9f2d..d3607deccfc 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -895,7 +895,8 @@ void anv_GetPhysicalDeviceProperties(
>.sparseProperties = {0}, /* Broadwell doesn't do sparse. */
> };
>
> -   strcpy(pProperties->deviceName, pdevice->name);
> +   strncpy(pProperties->deviceName, pdevice->name,
> +   VK_MAX_PHYSICAL_DEVICE_NAME_SIZE);

It will still misbehave if pdevice->name is too long, maybe clear the
last byte manually or use snprintf?

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

Re: [Mesa-dev] [PATCH 07/10] util/u_queue: add an option to resize the queue when it's full

[Grazvydas Ignotas]

On Tue, Jul 11, 2017 at 12:21 AM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> Consider the following situation:
>   mtx_lock(mutex);
>   do_something();
>   util_queue_add_job(...);
>   mtx_unlock(mutex);
>
> If the queue is full, util_queue_add_job will wait for a free slot.
> If the job which is currently being executed tries to lock the mutex,
> it will be stuck forever, because util_queue_add_job is stuck.
>
> The deadlock can be trivially resolved by increasing the queue size
> (reallocating the queue) in util_queue_add_job if the queue is full.
> Then util_queue_add_job becomes wait-free.
>
> radeonsi will use it.

Can't this cause the queue to grow uncontrollably, like on GPU hangs,
making already difficult to debug situations worse? Perhaps
util_queue_add_job() could have a non-blocking-fail option and the
caller could then retry after releasing the mutex for a bit.

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

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

[Grazvydas Ignotas]

On Wed, Jun 21, 2017 at 6:59 PM, James Legg <jl...@feralinteractive.com> wrote:
> When using cmpswap on an image, it was being trunctated to
> lvm.amdgcn.image.atomic.cmpswa, with the coords type missing entirely.

You could add an assert to catch this easier in future, like

MAYBE_UNUSED int ret;
...
ret = snprintf(intrinsic_name, ...
assert(ret < sizeof(intrinsic_name));

In either case
Reviewed-by: Grazvydas Ignotas <nota...@gmail.com>

> ---
>  src/amd/common/ac_nir_to_llvm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
> index 49117d2..3a26668 100644
> --- a/src/amd/common/ac_nir_to_llvm.c
> +++ b/src/amd/common/ac_nir_to_llvm.c
> @@ -3366,7 +3366,7 @@ static LLVMValueRef visit_image_atomic(struct 
> nir_to_llvm_context *ctx,
> const char *base_name = "llvm.amdgcn.image.atomic";
> const char *atomic_name;
> LLVMValueRef coords;
> -   char intrinsic_name[32], coords_type[8];
> +   char intrinsic_name[41], coords_type[8];
> const struct glsl_type *type = glsl_without_array(var->type);
>
> if (ctx->stage == MESA_SHADER_FRAGMENT)
> --
> 2.9.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] radeonsi: ppc64 glamor backtrace

[Grazvydas Ignotas]

Looks like nobody tested radeonsi on BE for 5 months at least. You can
try the attached patch, but I suspect there will be other places like
this...

Gražvydas

On Wed, Jun 21, 2017 at 3:26 PM, Jack Mitchell  wrote:
> Hi,
>
> I'm trying to run a VERDE based AMD card in a big endian powerpc64. I
> can get everything working with swrast but when I try to enable
> acceleration mesa bombs out when starting Xorg. I've got a backtrace
> from gdb but I'm not sure where to go next in debugging, any pointers
> would be appreciated.
>
> Attached (1)
>
> Regards,
> Jack.
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
diff --git a/src/gallium/drivers/radeonsi/si_pipe.h b/src/gallium/drivers/radeonsi/si_pipe.h
index 0c77f93..148fa69 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.h
+++ b/src/gallium/drivers/radeonsi/si_pipe.h
@@ -214,20 +214,25 @@ struct si_shader_ctx_state {
 /* The IA_MULTI_VGT_PARAM key used to index the table of precomputed values.
  * Some fields are set by state-change calls, most are set by draw_vbo.
  */
 union si_vgt_param_key {
 	struct {
+#ifdef PIPE_ARCH_BIG_ENDIAN
+		unsigned _pad:32 - SI_NUM_VGT_PARAM_KEY_BITS;
+#endif
 		unsigned prim:4;
 		unsigned uses_instancing:1;
 		unsigned multi_instances_smaller_than_primgroup:1;
 		unsigned primitive_restart:1;
 		unsigned count_from_stream_output:1;
 		unsigned line_stipple_enabled:1;
 		unsigned uses_tess:1;
 		unsigned tess_uses_prim_id:1;
 		unsigned uses_gs:1;
+#ifdef PIPE_ARCH_LITTLE_ENDIAN
 		unsigned _pad:32 - SI_NUM_VGT_PARAM_KEY_BITS;
+#endif
 	} u;
 	uint32_t index;
 };
 
 struct si_bindless_descriptor
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] spirv: Work around the Doom shader bug

[Grazvydas Ignotas]

On Tue, Jun 20, 2017 at 7:11 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
> Doom shipped with a broken version of GLSLang which handles samplers as
> function arguments in a way that isn't spec-compliant.  In particular,
> it creates a temporary local sampler variable and copies the sampler
> into it.  While Dave has had a hack patch out for a while that gets it
> working, we've never landed it because we've been hoping that a game
> update would come out with fixed shaders.  Unfortunately, no game update
> appears on to be on the horizon and I've found this issue in yet another
> application so I think we're stuck working around it.  Hopefully, we can
> delete this code one day.
>
> Cc: Dave Airlie <airl...@redhat.com>

Yes please, would be one less patch to carry.
I'd even vote for it to go to stable.

Tested-by: Grazvydas Ignotas <nota...@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99467
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH mesa v2 1/2] egl/display: only detect the platform once

[Grazvydas Ignotas]

On Fri, Jun 16, 2017 at 1:53 AM, Eric Engestrom <e...@engestrom.ch> wrote:
> My refactor missed the fact that `native_platform` is static.
> Add the proper guard around the detection code, as it might not be
> necessary, and only print the debug message when a detection was
> actually performed.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101252
> Fixes: 7adb9b094894a512c019 ("egl/display: remove unnecessary code and
>   make it easier to read")
> Signed-off-by: Eric Engestrom <e...@engestrom.ch>
> Reviewed-by: Grazvydas Ignotas <nota...@gmail.com>
> Ack-by: Emil Velikov <emil.l.veli...@gmail.com>

nit1: usually it's "Acked-by", but there are a few "Ack-by" in git
history too (2255 vs 3).

> ---
>  src/egl/main/egldisplay.c | 31 +--
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
> index 2a1f02738d..ffcebb74c2 100644
> --- a/src/egl/main/egldisplay.c
> +++ b/src/egl/main/egldisplay.c
> @@ -180,24 +180,27 @@ _eglNativePlatformDetectNativeDisplay(void 
> *nativeDisplay)
>  _EGLPlatformType
>  _eglGetNativePlatform(void *nativeDisplay)
>  {
> -   static _EGLPlatformType native_platform;
> -   char *detection_method;
> -
> -   native_platform = _eglGetNativePlatformFromEnv();
> -   detection_method = "environment overwrite";
> +   static _EGLPlatformType native_platform = _EGL_INVALID_PLATFORM;
>
> if (native_platform == _EGL_INVALID_PLATFORM) {
> -  native_platform = _eglNativePlatformDetectNativeDisplay(nativeDisplay);
> -  detection_method = "autodetected";
> -   }
> +  char *detection_method;

nit2: you can make it const. Either way, my r-b still stands.

>
> -   if (native_platform == _EGL_INVALID_PLATFORM) {
> -  native_platform = _EGL_NATIVE_PLATFORM;
> -  detection_method = "build-time configuration";
> -   }
> +  native_platform = _eglGetNativePlatformFromEnv();
> +  detection_method = "environment overwrite";
>
> -   _eglLog(_EGL_DEBUG, "Native platform type: %s (%s)",
> -   egl_platforms[native_platform].name, detection_method);
> +  if (native_platform == _EGL_INVALID_PLATFORM) {
> + native_platform = 
> _eglNativePlatformDetectNativeDisplay(nativeDisplay);
> + detection_method = "autodetected";
> +  }
> +
> +  if (native_platform == _EGL_INVALID_PLATFORM) {
> + native_platform = _EGL_NATIVE_PLATFORM;
> + detection_method = "build-time configuration";
> +  }
> +
> +  _eglLog(_EGL_DEBUG, "Native platform type: %s (%s)",
> +  egl_platforms[native_platform].name, detection_method);
> +   }
>
> return native_platform;
>  }
> --
> Cheers,
>   Eric
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH mesa 2/2] egl/display: make platform detection thread-safe

[Grazvydas Ignotas]

On Thu, Jun 1, 2017 at 2:15 PM, Eric Engestrom
<eric.engest...@imgtec.com> wrote:
> If the detections methods ever become able to return different results
> for different threads, the if chain might make threads go back and forth
> between invalid and valid platforms.
> Solve this by doing the detection in a local var and only overwriting
> the global one at the end, if no other thread has updated it since.
>
> This means the platform detected in the thread might not be the platform
> returned by the function, but this is a different issue that will need
> to be discussed when this becomes possible.
>
> Reported-by: Grazvydas Ignotas <nota...@gmail.com>

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

> Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>

It seems this has stalled. With the commit message updated (as
detailed in my previous mail, bad things can happen even with matching
thread results), and optionally putting everything in a if block to
avoid expensive cmpxchg on non-first run, this is

Reviewed-by: Grazvydas Ignotas <nota...@gmail.com>

I think you can add Emil's Acked-by too.

> ---
>
> This is unnecessary in my opinion, but doesn't hurt :)
> Putting it out there as a possible fix, but my vote is to not merge it,
> not until this per-thread-platform-detection becomes possible anyway.
> ---
>  src/egl/main/egldisplay.c | 17 +++--
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
> index 3f7752f53d..92586def0c 100644
> --- a/src/egl/main/egldisplay.c
> +++ b/src/egl/main/egldisplay.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include 
>  #include "c11/threads.h"
> +#include "util/u_atomic.h"
>
>  #include "eglcontext.h"
>  #include "eglcurrent.h"
> @@ -181,23 +182,27 @@ _EGLPlatformType
>  _eglGetNativePlatform(void *nativeDisplay)
>  {
> static _EGLPlatformType native_platform = _EGL_INVALID_PLATFORM;
> +   _EGLPlatformType detected_platform = native_platform;
> char *detection_method = NULL;
>
> -   if (native_platform == _EGL_INVALID_PLATFORM) {
> -  native_platform = _eglGetNativePlatformFromEnv();
> +   if (detected_platform == _EGL_INVALID_PLATFORM) {
> +  detected_platform = _eglGetNativePlatformFromEnv();
>detection_method = "environment overwrite";
> }
>
> -   if (native_platform == _EGL_INVALID_PLATFORM) {
> -  native_platform = _eglNativePlatformDetectNativeDisplay(nativeDisplay);
> +   if (detected_platform == _EGL_INVALID_PLATFORM) {
> +  detected_platform = 
> _eglNativePlatformDetectNativeDisplay(nativeDisplay);
>detection_method = "autodetected";
> }
>
> -   if (native_platform == _EGL_INVALID_PLATFORM) {
> -  native_platform = _EGL_NATIVE_PLATFORM;
> +   if (detected_platform == _EGL_INVALID_PLATFORM) {
> +  detected_platform = _EGL_NATIVE_PLATFORM;
>detection_method = "build-time configuration";
> }
>
> +   p_atomic_cmpxchg(_platform, _EGL_INVALID_PLATFORM,
> +detected_platform);
> +
> if (detection_method != NULL) {
>_eglLog(_EGL_DEBUG, "Native platform type: %s (%s)",
>egl_platforms[native_platform].name, detection_method);
> --
> Cheers,
>   Eric
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] mesa/util: add a hash table wrapper which support 64-bit keys

[Grazvydas Ignotas]

On Tue, Jun 13, 2017 at 10:58 AM, Samuel Pitoiset
 wrote:
> Needed for bindless handles which are represented using
> 64-bit unsigned integers. All hash table implementations should
> be uniformized later on.
>
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/util/hash_table.c | 149 
> ++
>  src/util/hash_table.h |  25 +
>  2 files changed, 174 insertions(+)
>
> diff --git a/src/util/hash_table.c b/src/util/hash_table.c
> index 9e643af8b23..c2b9210218a 100644
> --- a/src/util/hash_table.c
> +++ b/src/util/hash_table.c
> @@ -47,6 +47,7 @@
>  #include "hash_table.h"
>  #include "ralloc.h"
>  #include "macros.h"
> +#include "main/hash.h"
>
>  static const uint32_t deleted_key_value;
>
> @@ -502,3 +503,151 @@ _mesa_key_pointer_equal(const void *a, const void *b)
>  {
> return a == b;
>  }
> +
> +/**
> + * Hash table wrapper which supports 64-bit keys.
> + *
> + * TODO: unify all hash table implementations.
> + */
> +#if !defined(MESA_ARCH_X86_64)
> +
> +struct hash_key_u64 {
> +   uint64_t value;
> +};
> +
> +static uint32_t
> +key_u64_hash(const void *key)
> +{
> +   return _mesa_hash_data(key, sizeof(struct hash_key_u64));
> +}
> +
> +static bool
> +key_u64_equals(const void *a, const void *b)
> +{
> +   struct hash_key_u64 *aa = (struct hash_key_u64 *)a;

const struct hash_key_u64, and then you can drop the cast.

> +   struct hash_key_u64 *bb = (struct hash_key_u64 *)b;
> +
> +   return aa->value == bb->value;
> +}
> +
> +#endif
> +
> +struct hash_table_u64 *
> +_mesa_hash_table_u64_create(void *mem_ctx)
> +{
> +   struct hash_table_u64 *ht;
> +
> +   ht = CALLOC_STRUCT(hash_table_u64);
> +   if (!ht)
> +  return NULL;
> +
> +#if !defined(MESA_ARCH_X86_64)

I can't seem to find anything defining MESA_ARCH_X86_64 anywhere.
There is also nothing x86-64 specific here, what about doing

  if (sizeof(void *) == 8) {

and letting the compiler optimizer to drop the other case?

> +   ht->table = _mesa_hash_table_create(mem_ctx, key_u64_hash, 
> key_u64_equals);
> +#else
> +   ht->table = _mesa_hash_table_create(mem_ctx, _mesa_hash_pointer,
> +   _mesa_key_pointer_equal);
> +#endif

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

[Mesa-dev] [PATCH] radv: fix trace dumping for !use_ib_bos

[Grazvydas Ignotas]

Fixes trace dumping crash for SI or when RADV_DEBUG=noibs is set.

Fixes: 97dfff5410 "radv: Dump command buffer on hang."
Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
Not sure if chained buffer dumping can be done for !use_ib_bos,
returning NULL in _get_cpu_addr() just skips that.

 src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c 
b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
index 7b74970..ffc7566 100644
--- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
+++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
@@ -950,10 +950,13 @@ static int radv_amdgpu_winsys_cs_submit(struct 
radeon_winsys_ctx *_ctx,
 
 static void *radv_amdgpu_winsys_get_cpu_addr(void *_cs, uint64_t addr)
 {
struct radv_amdgpu_cs *cs = (struct radv_amdgpu_cs *)_cs;
void *ret = NULL;
+
+   if (!cs->ib_buffer)
+   return NULL;
for (unsigned i = 0; i <= cs->num_old_ib_buffers; ++i) {
struct radv_amdgpu_winsys_bo *bo;
 
bo = (struct radv_amdgpu_winsys_bo*)
   (i == cs->num_old_ib_buffers ? cs->ib_buffer : 
cs->old_ib_buffers[i]);
@@ -968,14 +971,19 @@ static void *radv_amdgpu_winsys_get_cpu_addr(void *_cs, 
uint64_t addr)
 static void radv_amdgpu_winsys_cs_dump(struct radeon_winsys_cs *_cs,
FILE* file,
uint32_t trace_id)
 {
struct radv_amdgpu_cs *cs = (struct radv_amdgpu_cs *)_cs;
+   void *ib = cs->base.buf;
+   int num_dw = cs->base.cdw;
 
-   ac_parse_ib(file,
-   radv_amdgpu_winsys_get_cpu_addr(cs, cs->ib.ib_mc_address),
-   cs->ib.size, trace_id,  "main IB", cs->ws->info.chip_class,
+   if (cs->ws->use_ib_bos) {
+   ib = radv_amdgpu_winsys_get_cpu_addr(cs, cs->ib.ib_mc_address);
+   num_dw = cs->ib.size;
+   }
+   assert(ib);
+   ac_parse_ib(file, ib, num_dw, trace_id, "main IB", 
cs->ws->info.chip_class,
radv_amdgpu_winsys_get_cpu_addr, cs);
 }
 
 static struct radeon_winsys_ctx *radv_amdgpu_ctx_create(struct radeon_winsys 
*_ws)
 {
-- 
2.7.4

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

[Mesa-dev] [PATCH 2/2] radv: don't even attempt to prefetch on SI

[Grazvydas Ignotas]

Before bcae327469 this was emitting CP DMA packet even on SI, but
apparently hasn't caused too many problems. After that commit the
CP DMA code now always sets the CIK+ only bit for prefetch. Just
follow radeonsi there and don't try to prefetch at all.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101334
Fixes: bcae327469 "radv: realign cp dma code with radeonsi"
Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/amd/vulkan/radv_cmd_buffer.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
index 1ac9de1..b08f218 100644
--- a/src/amd/vulkan/radv_cmd_buffer.c
+++ b/src/amd/vulkan/radv_cmd_buffer.c
@@ -529,10 +529,18 @@ radv_emit_graphics_raster_state(struct radv_cmd_buffer 
*cmd_buffer,
 
radeon_set_context_reg(cmd_buffer->cs, R_028814_PA_SU_SC_MODE_CNTL,
   raster->pa_su_sc_mode_cntl);
 }
 
+static inline void
+radv_emit_prefetch(struct radv_cmd_buffer *cmd_buffer, uint64_t va,
+  unsigned size)
+{
+   if (cmd_buffer->device->physical_device->rad_info.chip_class >= CIK)
+   si_cp_dma_prefetch(cmd_buffer, va, size);
+}
+
 static void
 radv_emit_hw_vs(struct radv_cmd_buffer *cmd_buffer,
struct radv_pipeline *pipeline,
struct radv_shader_variant *shader,
struct ac_vs_output_info *outinfo)
@@ -540,11 +548,11 @@ radv_emit_hw_vs(struct radv_cmd_buffer *cmd_buffer,
struct radeon_winsys *ws = cmd_buffer->device->ws;
uint64_t va = ws->buffer_get_va(shader->bo);
unsigned export_count;
 
ws->cs_add_buffer(cmd_buffer->cs, shader->bo, 8);
-   si_cp_dma_prefetch(cmd_buffer, va, shader->code_size);
+   radv_emit_prefetch(cmd_buffer, va, shader->code_size);
 
export_count = MAX2(1, outinfo->param_exports);
radeon_set_context_reg(cmd_buffer->cs, R_0286C4_SPI_VS_OUT_CONFIG,
   S_0286C4_VS_EXPORT_COUNT(export_count - 1));
 
@@ -589,11 +597,11 @@ radv_emit_hw_es(struct radv_cmd_buffer *cmd_buffer,
 {
struct radeon_winsys *ws = cmd_buffer->device->ws;
uint64_t va = ws->buffer_get_va(shader->bo);
 
ws->cs_add_buffer(cmd_buffer->cs, shader->bo, 8);
-   si_cp_dma_prefetch(cmd_buffer, va, shader->code_size);
+   radv_emit_prefetch(cmd_buffer, va, shader->code_size);
 
radeon_set_context_reg(cmd_buffer->cs, R_028AAC_VGT_ESGS_RING_ITEMSIZE,
   outinfo->esgs_itemsize / 4);
radeon_set_sh_reg_seq(cmd_buffer->cs, R_00B320_SPI_SHADER_PGM_LO_ES, 4);
radeon_emit(cmd_buffer->cs, va >> 8);
@@ -609,11 +617,11 @@ radv_emit_hw_ls(struct radv_cmd_buffer *cmd_buffer,
struct radeon_winsys *ws = cmd_buffer->device->ws;
uint64_t va = ws->buffer_get_va(shader->bo);
uint32_t rsrc2 = shader->rsrc2;
 
ws->cs_add_buffer(cmd_buffer->cs, shader->bo, 8);
-   si_cp_dma_prefetch(cmd_buffer, va, shader->code_size);
+   radv_emit_prefetch(cmd_buffer, va, shader->code_size);
 
radeon_set_sh_reg_seq(cmd_buffer->cs, R_00B520_SPI_SHADER_PGM_LO_LS, 2);
radeon_emit(cmd_buffer->cs, va >> 8);
radeon_emit(cmd_buffer->cs, va >> 40);
 
@@ -633,11 +641,11 @@ radv_emit_hw_hs(struct radv_cmd_buffer *cmd_buffer,
 {
struct radeon_winsys *ws = cmd_buffer->device->ws;
uint64_t va = ws->buffer_get_va(shader->bo);
 
ws->cs_add_buffer(cmd_buffer->cs, shader->bo, 8);
-   si_cp_dma_prefetch(cmd_buffer, va, shader->code_size);
+   radv_emit_prefetch(cmd_buffer, va, shader->code_size);
 
radeon_set_sh_reg_seq(cmd_buffer->cs, R_00B420_SPI_SHADER_PGM_LO_HS, 4);
radeon_emit(cmd_buffer->cs, va >> 8);
radeon_emit(cmd_buffer->cs, va >> 40);
radeon_emit(cmd_buffer->cs, shader->rsrc1);
@@ -767,11 +775,12 @@ radv_emit_geometry_shader(struct radv_cmd_buffer 
*cmd_buffer,
   S_028B90_CNT(MIN2(gs_num_invocations, 127)) |
   S_028B90_ENABLE(gs_num_invocations > 0));
 
va = ws->buffer_get_va(gs->bo);
ws->cs_add_buffer(cmd_buffer->cs, gs->bo, 8);
-   si_cp_dma_prefetch(cmd_buffer, va, gs->code_size);
+   radv_emit_prefetch(cmd_buffer, va, gs->code_size);
+
radeon_set_sh_reg_seq(cmd_buffer->cs, R_00B220_SPI_SHADER_PGM_LO_GS, 4);
radeon_emit(cmd_buffer->cs, va >> 8);
radeon_emit(cmd_buffer->cs, va >> 40);
radeon_emit(cmd_buffer->cs, gs->rsrc1);
radeon_emit(cmd_buffer->cs, gs->rsrc2);
@@ -808,11 +817,11 @@ radv_emit_fragment_shader(struct radv_cmd_buffer 
*cmd_b

[Mesa-dev] [PATCH 1/2] radv: assert on CP_DMA_USE_L2 for SI

[Grazvydas Ignotas]

The register header (and radeonsi comment) states V_411_SRC_ADDR_TC_L2
is for CIK+ only, so let's assert on earlier ASICs.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/amd/vulkan/si_cmd_buffer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/amd/vulkan/si_cmd_buffer.c b/src/amd/vulkan/si_cmd_buffer.c
index 33414c1..962b76f 100644
--- a/src/amd/vulkan/si_cmd_buffer.c
+++ b/src/amd/vulkan/si_cmd_buffer.c
@@ -1191,10 +1191,11 @@ static void si_emit_cp_dma(struct radv_cmd_buffer 
*cmd_buffer,
radeon_emit(cs, src_va >> 32);  /* SRC_ADDR_HI [31:0] */
radeon_emit(cs, dst_va);/* DST_ADDR_LO [31:0] */
radeon_emit(cs, dst_va >> 32);  /* DST_ADDR_HI [31:0] */
radeon_emit(cs, command);
} else {
+   assert(!(flags & CP_DMA_USE_L2));
header |= S_411_SRC_ADDR_HI(src_va >> 32);
radeon_emit(cs, PKT3(PKT3_CP_DMA, 4, 0));
radeon_emit(cs, src_va);/* SRC_ADDR_LO 
[31:0] */
radeon_emit(cs, header);/* SRC_ADDR_HI 
[15:0] + flags. */
radeon_emit(cs, dst_va);/* DST_ADDR_LO 
[31:0] */
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH] nir: make various getters take const pointers

[Grazvydas Ignotas]

Ping. Boring patch, should be easy to review or NAK.

On Wed, Jun 7, 2017 at 2:25 AM, Grazvydas Ignotas <nota...@gmail.com> wrote:
> This will allow to constify other things.
>
> Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
> ---
>  src/compiler/nir/nir.h  | 25 +
>  src/compiler/nir/nir_lower_io.c |  2 +-
>  2 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 3b827bf..ab7ba14 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -436,19 +436,19 @@ nir_instr_prev(nir_instr *instr)
> else
>return exec_node_data(nir_instr, prev, node);
>  }
>
>  static inline bool
> -nir_instr_is_first(nir_instr *instr)
> +nir_instr_is_first(const nir_instr *instr)
>  {
> -   return exec_node_is_head_sentinel(exec_node_get_prev(>node));
> +   return exec_node_is_head_sentinel(exec_node_get_prev_const(>node));
>  }
>
>  static inline bool
> -nir_instr_is_last(nir_instr *instr)
> +nir_instr_is_last(const nir_instr *instr)
>  {
> -   return exec_node_is_tail_sentinel(exec_node_get_next(>node));
> +   return exec_node_is_tail_sentinel(exec_node_get_next_const(>node));
>  }
>
>  typedef struct nir_ssa_def {
> /** for debugging only, can be NULL */
> const char* name;
> @@ -802,11 +802,12 @@ void nir_alu_src_copy(nir_alu_src *dest, const 
> nir_alu_src *src,
>  void nir_alu_dest_copy(nir_alu_dest *dest, const nir_alu_dest *src,
> nir_alu_instr *instr);
>
>  /* is this source channel used? */
>  static inline bool
> -nir_alu_instr_channel_used(nir_alu_instr *instr, unsigned src, unsigned 
> channel)
> +nir_alu_instr_channel_used(const nir_alu_instr *instr, unsigned src,
> +   unsigned channel)
>  {
> if (nir_op_infos[instr->op].input_sizes[src] > 0)
>return channel < nir_op_infos[instr->op].input_sizes[src];
>
> return (instr->dest.write_mask >> channel) & 1;
> @@ -1085,11 +1086,11 @@ typedef struct {
>  extern const nir_intrinsic_info nir_intrinsic_infos[nir_num_intrinsics];
>
>
>  #define INTRINSIC_IDX_ACCESSORS(name, flag, type)
>  \
>  static inline type   
>  \
> -nir_intrinsic_##name(nir_intrinsic_instr *instr) 
>  \
> +nir_intrinsic_##name(const nir_intrinsic_instr *instr)   
>  \
>  {
>  \
> const nir_intrinsic_info *info = _intrinsic_infos[instr->intrinsic];  
>  \
> assert(info->index_map[NIR_INTRINSIC_##flag] > 0);
>  \
> return instr->const_index[info->index_map[NIR_INTRINSIC_##flag] - 1]; 
>  \
>  }
>  \
> @@ -1219,11 +1220,11 @@ typedef struct {
>  */
> nir_deref_var *sampler;
>  } nir_tex_instr;
>
>  static inline unsigned
> -nir_tex_instr_dest_size(nir_tex_instr *instr)
> +nir_tex_instr_dest_size(const nir_tex_instr *instr)
>  {
> switch (instr->op) {
> case nir_texop_txs: {
>unsigned ret;
>switch (instr->sampler_dim) {
> @@ -1268,11 +1269,11 @@ nir_tex_instr_dest_size(nir_tex_instr *instr)
>
>  /* Returns true if this texture operation queries something about the texture
>   * rather than actually sampling it.
>   */
>  static inline bool
> -nir_tex_instr_is_query(nir_tex_instr *instr)
> +nir_tex_instr_is_query(const nir_tex_instr *instr)
>  {
> switch (instr->op) {
> case nir_texop_txs:
> case nir_texop_lod:
> case nir_texop_texture_samples:
> @@ -1291,11 +1292,11 @@ nir_tex_instr_is_query(nir_tex_instr *instr)
>unreachable("Invalid texture opcode");
> }
>  }
>
>  static inline nir_alu_type
> -nir_tex_instr_src_type(nir_tex_instr *instr, unsigned src)
> +nir_tex_instr_src_type(const nir_tex_instr *instr, unsigned src)
>  {
> switch (instr->src[src].src_type) {
> case nir_tex_src_coord:
>switch (instr->op) {
>case nir_texop_txf:
> @@ -1335,11 +1336,11 @@ nir_tex_instr_src_type(nir_tex_instr *instr, unsigned 
> src)
>unreachable("Invalid texture source type");
> }
>  }
>
>  static inline unsigned
> -nir_tex_instr_src_size(nir_tex_instr *instr, unsigned src)
> +nir_tex_instr_src_size(const nir_tex_instr *instr, unsigned src)
>  {
> if (instr->src[src].src_type == nir_tex_src_coord)
>return instr->c

[Mesa-dev] [PATCH 1/3] ac/nir: mark some arguments const

[Grazvydas Ignotas]

Most functions are only inspecting nir, so nir related arguments can be
marked const. Some more can be done if/when some nir changes are
accepted.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
does *not* depend on the nir patch

 src/amd/common/ac_nir_to_llvm.c | 61 +
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 4e5d19a..5f62769 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -174,11 +174,11 @@ struct nir_to_llvm_context {
uint64_t tess_outputs_written;
uint64_t tess_patch_outputs_written;
 };
 
 static LLVMValueRef get_sampler_desc(struct nir_to_llvm_context *ctx,
-nir_deref_var *deref,
+const nir_deref_var *deref,
 enum desc_type desc_type);
 static unsigned radeon_llvm_reg_index_soa(unsigned index, unsigned chan)
 {
return (index * 4) + chan;
 }
@@ -1077,11 +1077,11 @@ build_store_values_extended(struct nir_to_llvm_context 
*ctx,
LLVMBuildStore(builder, value, ptr);
}
 }
 
 static LLVMTypeRef get_def_type(struct nir_to_llvm_context *ctx,
-nir_ssa_def *def)
+const nir_ssa_def *def)
 {
LLVMTypeRef type = LLVMIntTypeInContext(ctx->context, def->bit_size);
if (def->num_components > 1) {
type = LLVMVectorType(type, def->num_components);
}
@@ -1095,11 +1095,11 @@ static LLVMValueRef get_src(struct nir_to_llvm_context 
*ctx, nir_src src)
return (LLVMValueRef)entry->data;
 }
 
 
 static LLVMBasicBlockRef get_block(struct nir_to_llvm_context *ctx,
-   struct nir_block *b)
+   const struct nir_block *b)
 {
struct hash_entry *entry = _mesa_hash_table_search(ctx->defs, b);
return (LLVMBasicBlockRef)entry->data;
 }
 
@@ -1385,11 +1385,11 @@ static LLVMValueRef emit_imul_high(struct 
nir_to_llvm_context *ctx,
return result;
 }
 
 static LLVMValueRef emit_bitfield_extract(struct nir_to_llvm_context *ctx,
  bool is_signed,
- LLVMValueRef srcs[3])
+ const LLVMValueRef srcs[3])
 {
LLVMValueRef result;
LLVMValueRef icond = LLVMBuildICmp(ctx->builder, LLVMIntEQ, srcs[2], 
LLVMConstInt(ctx->i32, 32, false), "");
 
result = ac_build_bfe(>ac, srcs[0], srcs[1], srcs[2], is_signed);
@@ -1524,11 +1524,11 @@ static LLVMValueRef emit_ddxy_interp(
result[2+i] = emit_ddxy(ctx, nir_op_fddy, a);
}
return ac_build_gather_values(>ac, result, 4);
 }
 
-static void visit_alu(struct nir_to_llvm_context *ctx, nir_alu_instr *instr)
+static void visit_alu(struct nir_to_llvm_context *ctx, const nir_alu_instr 
*instr)
 {
LLVMValueRef src[4], result = NULL;
unsigned num_components = instr->dest.dest.ssa.num_components;
unsigned src_components;
LLVMTypeRef def_type = get_def_type(ctx, >dest.dest.ssa);
@@ -1890,11 +1890,11 @@ static void visit_alu(struct nir_to_llvm_context *ctx, 
nir_alu_instr *instr)
result);
}
 }
 
 static void visit_load_const(struct nir_to_llvm_context *ctx,
- nir_load_const_instr *instr)
+ const nir_load_const_instr *instr)
 {
LLVMValueRef values[4], value = NULL;
LLVMTypeRef element_type =
LLVMIntTypeInContext(ctx->context, instr->def.bit_size);
 
@@ -1974,11 +1974,11 @@ static void build_int_type_name(
strcpy(buf, "i32");
 }
 
 static LLVMValueRef radv_lower_gather4_integer(struct nir_to_llvm_context *ctx,
   struct ac_image_args *args,
-  nir_tex_instr *instr)
+  const nir_tex_instr *instr)
 {
enum glsl_base_type stype = 
glsl_get_sampler_result_type(instr->texture->var->type);
LLVMValueRef coord = args->addr;
LLVMValueRef half_texel[2];
LLVMValueRef compare_cube_wa;
@@ -2087,11 +2087,11 @@ static LLVMValueRef radv_lower_gather4_integer(struct 
nir_to_llvm_context *ctx,
}
return result;
 }
 
 static LLVMValueRef build_tex_intrinsic(struct nir_to_llvm_context *ctx,
-   nir_tex_instr *instr,
+   const nir_tex_instr *instr,
bool lod_is_zero,
struct ac_image_args *args)
 {
if (instr->sampler_dim == GLSL_SAMPLER_DIM_BUF)

[Mesa-dev] [PATCH 2/3] ac/nir: convert several ifs to a switch

[Grazvydas Ignotas]

Also solve "outinfo may be used uninitialized" warning by putting in an
unreachable().

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/amd/common/ac_nir_to_llvm.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 5f62769..005e2be 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -5802,27 +5802,29 @@ static void ac_llvm_finalize_module(struct 
nir_to_llvm_context * ctx)
 static void
 ac_nir_eliminate_const_vs_outputs(struct nir_to_llvm_context *ctx)
 {
struct ac_vs_output_info *outinfo;
 
-   if (ctx->stage == MESA_SHADER_FRAGMENT ||
-   ctx->stage == MESA_SHADER_COMPUTE ||
-   ctx->stage == MESA_SHADER_TESS_CTRL ||
-   ctx->stage == MESA_SHADER_GEOMETRY)
+   switch (ctx->stage) {
+   case MESA_SHADER_FRAGMENT:
+   case MESA_SHADER_COMPUTE:
+   case MESA_SHADER_TESS_CTRL:
+   case MESA_SHADER_GEOMETRY:
return;
-
-   if (ctx->stage == MESA_SHADER_VERTEX) {
+   case MESA_SHADER_VERTEX:
if (ctx->options->key.vs.as_ls ||
ctx->options->key.vs.as_es)
return;
outinfo = >shader_info->vs.outinfo;
-   }
-
-   if (ctx->stage == MESA_SHADER_TESS_EVAL) {
+   break;
+   case MESA_SHADER_TESS_EVAL:
if (ctx->options->key.vs.as_es)
return;
outinfo = >shader_info->tes.outinfo;
+   break;
+   default:
+   unreachable("Unhandled shader type");
}
 
ac_optimize_vs_outputs(>ac,
   ctx->main_function,
   outinfo->vs_output_param_offset,
-- 
2.7.4

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

[Mesa-dev] [PATCH 3/3] radv/meta: remove an unused variable

[Grazvydas Ignotas]

Trivial.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/amd/vulkan/radv_meta_clear.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/amd/vulkan/radv_meta_clear.c b/src/amd/vulkan/radv_meta_clear.c
index f4cb787..d7e7c5b 100644
--- a/src/amd/vulkan/radv_meta_clear.c
+++ b/src/amd/vulkan/radv_meta_clear.c
@@ -670,11 +670,10 @@ emit_fast_htile_clear(struct radv_cmd_buffer *cmd_buffer,
const struct radv_framebuffer *fb = cmd_buffer->state.framebuffer;
const struct radv_image_view *iview = 
fb->attachments[pass_att].attachment;
VkClearDepthStencilValue clear_value = 
clear_att->clearValue.depthStencil;
VkImageAspectFlags aspects = clear_att->aspectMask;
uint32_t clear_word;
-   bool ret;
 
if (!iview->image->surface.htile_size)
return false;
 
if (cmd_buffer->device->debug_flags & RADV_DEBUG_NO_FAST_CLEARS)
-- 
2.7.4

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

[Mesa-dev] [PATCH] nir: make various getters take const pointers

[Grazvydas Ignotas]

This will allow to constify other things.

Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/compiler/nir/nir.h  | 25 +
 src/compiler/nir/nir_lower_io.c |  2 +-
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 3b827bf..ab7ba14 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -436,19 +436,19 @@ nir_instr_prev(nir_instr *instr)
else
   return exec_node_data(nir_instr, prev, node);
 }
 
 static inline bool
-nir_instr_is_first(nir_instr *instr)
+nir_instr_is_first(const nir_instr *instr)
 {
-   return exec_node_is_head_sentinel(exec_node_get_prev(>node));
+   return exec_node_is_head_sentinel(exec_node_get_prev_const(>node));
 }
 
 static inline bool
-nir_instr_is_last(nir_instr *instr)
+nir_instr_is_last(const nir_instr *instr)
 {
-   return exec_node_is_tail_sentinel(exec_node_get_next(>node));
+   return exec_node_is_tail_sentinel(exec_node_get_next_const(>node));
 }
 
 typedef struct nir_ssa_def {
/** for debugging only, can be NULL */
const char* name;
@@ -802,11 +802,12 @@ void nir_alu_src_copy(nir_alu_src *dest, const 
nir_alu_src *src,
 void nir_alu_dest_copy(nir_alu_dest *dest, const nir_alu_dest *src,
nir_alu_instr *instr);
 
 /* is this source channel used? */
 static inline bool
-nir_alu_instr_channel_used(nir_alu_instr *instr, unsigned src, unsigned 
channel)
+nir_alu_instr_channel_used(const nir_alu_instr *instr, unsigned src,
+   unsigned channel)
 {
if (nir_op_infos[instr->op].input_sizes[src] > 0)
   return channel < nir_op_infos[instr->op].input_sizes[src];
 
return (instr->dest.write_mask >> channel) & 1;
@@ -1085,11 +1086,11 @@ typedef struct {
 extern const nir_intrinsic_info nir_intrinsic_infos[nir_num_intrinsics];
 
 
 #define INTRINSIC_IDX_ACCESSORS(name, flag, type) \
 static inline type\
-nir_intrinsic_##name(nir_intrinsic_instr *instr)  \
+nir_intrinsic_##name(const nir_intrinsic_instr *instr)\
 { \
const nir_intrinsic_info *info = _intrinsic_infos[instr->intrinsic];   \
assert(info->index_map[NIR_INTRINSIC_##flag] > 0); \
return instr->const_index[info->index_map[NIR_INTRINSIC_##flag] - 1];  \
 } \
@@ -1219,11 +1220,11 @@ typedef struct {
 */
nir_deref_var *sampler;
 } nir_tex_instr;
 
 static inline unsigned
-nir_tex_instr_dest_size(nir_tex_instr *instr)
+nir_tex_instr_dest_size(const nir_tex_instr *instr)
 {
switch (instr->op) {
case nir_texop_txs: {
   unsigned ret;
   switch (instr->sampler_dim) {
@@ -1268,11 +1269,11 @@ nir_tex_instr_dest_size(nir_tex_instr *instr)
 
 /* Returns true if this texture operation queries something about the texture
  * rather than actually sampling it.
  */
 static inline bool
-nir_tex_instr_is_query(nir_tex_instr *instr)
+nir_tex_instr_is_query(const nir_tex_instr *instr)
 {
switch (instr->op) {
case nir_texop_txs:
case nir_texop_lod:
case nir_texop_texture_samples:
@@ -1291,11 +1292,11 @@ nir_tex_instr_is_query(nir_tex_instr *instr)
   unreachable("Invalid texture opcode");
}
 }
 
 static inline nir_alu_type
-nir_tex_instr_src_type(nir_tex_instr *instr, unsigned src)
+nir_tex_instr_src_type(const nir_tex_instr *instr, unsigned src)
 {
switch (instr->src[src].src_type) {
case nir_tex_src_coord:
   switch (instr->op) {
   case nir_texop_txf:
@@ -1335,11 +1336,11 @@ nir_tex_instr_src_type(nir_tex_instr *instr, unsigned 
src)
   unreachable("Invalid texture source type");
}
 }
 
 static inline unsigned
-nir_tex_instr_src_size(nir_tex_instr *instr, unsigned src)
+nir_tex_instr_src_size(const nir_tex_instr *instr, unsigned src)
 {
if (instr->src[src].src_type == nir_tex_src_coord)
   return instr->coord_components;
 
/* The MCS value is expected to be a vec4 returned by a txf_ms_mcs */
@@ -1357,11 +1358,11 @@ nir_tex_instr_src_size(nir_tex_instr *instr, unsigned 
src)
 
return 1;
 }
 
 static inline int
-nir_tex_instr_src_index(nir_tex_instr *instr, nir_tex_src_type type)
+nir_tex_instr_src_index(const nir_tex_instr *instr, nir_tex_src_type type)
 {
for (unsigned i = 0; i < instr->num_srcs; i++)
   if (instr->src[i].src_type == type)
  return (int) i;
 
@@ -2392,11 +2393,11 @@ bool nir_lower_io(nir_shader *shader,
   int (*type_size)(const struct glsl_type *),
   nir_lower_io_options);
 nir_src *nir_get_io_offset_src(nir_intrinsic_instr *instr);
 nir_src *nir_get_io_vertex_

Re: [Mesa-dev] [PATCH 03/11] radeonsi: decrease the number of compiler threads to num CPUs - 1

[Grazvydas Ignotas]

With the "core wars" starting (16core/32thread "consumer" CPUs
announced), maybe it's time to add an upper limit? Waking up a core
from low power state has it's latency, it's sometimes faster to just
do several jobs on an active core than to wake another one (which will
also cause more lock contention, the core will just see the job was
already taken by someone else and go back to sleep).

Gražvydas

On Thu, Jun 1, 2017 at 9:18 PM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> Reserve one core for other things (like draw calls).
> ---
>  src/gallium/drivers/radeonsi/si_pipe.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
> b/src/gallium/drivers/radeonsi/si_pipe.c
> index de4e5da..4704304 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -874,22 +874,25 @@ struct pipe_screen *radeonsi_screen_create(struct 
> radeon_winsys *ws)
>
> si_init_screen_state_functions(sscreen);
>
> if (!r600_common_screen_init(>b, ws) ||
> !si_init_gs_info(sscreen) ||
> !si_init_shader_cache(sscreen)) {
> FREE(sscreen);
> return NULL;
> }
>
> -   /* Only enable as many threads as we have target machines and CPUs. */
> +   /* Only enable as many threads as we have target machines, but at most
> +* the number of CPUs - 1 if there is more than one.
> +*/
> num_cpus = sysconf(_SC_NPROCESSORS_ONLN);
> +   num_cpus = MAX2(1, num_cpus - 1);
> num_compiler_threads = MIN2(num_cpus, ARRAY_SIZE(sscreen->tm));
>
> if (!util_queue_init(>shader_compiler_queue, "si_shader",
>  32, num_compiler_threads)) {
> si_destroy_shader_cache(sscreen);
> FREE(sscreen);
> return NULL;
> }
>
> si_handle_env_var_force_family(sscreen);
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH mesa 2/2] egl/display: make platform detection thread-safe

[Grazvydas Ignotas]

On Thu, Jun 1, 2017 at 4:23 PM, Emil Velikov <emil.l.veli...@gmail.com> wrote:
> Hi guys,
>
> On 1 June 2017 at 12:56, Grazvydas Ignotas <nota...@gmail.com> wrote:
>> On Thu, Jun 1, 2017 at 2:15 PM, Eric Engestrom
>> <eric.engest...@imgtec.com> wrote:
>>> If the detections methods ever become able to return different results
>>> for different threads, the if chain might make threads go back and forth
>>> between invalid and valid platforms.
>>> Solve this by doing the detection in a local var and only overwriting
>>> the global one at the end, if no other thread has updated it since.
>>>
>>> This means the platform detected in the thread might not be the platform
>>> returned by the function, but this is a different issue that will need
>>> to be discussed when this becomes possible.
>>>
>>> Reported-by: Grazvydas Ignotas <nota...@gmail.com>
>>
>> Not really, see https://bugs.freedesktop.org/show_bug.cgi?id=101252
>>
>>> Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
>>> ---
>>>
>>> This is unnecessary in my opinion, but doesn't hurt :)
>>
>> It is necessary, without it things will work most of the time but
>> there will be rare failures.
>> Imagine there are 2 threads that both call _eglGetNativePlatform()
>> simultaneously:
>> - thread 1 completes the first "if (native_platform ==
>> _EGL_INVALID_PLATFORM)" check and is preemted to do something else
>> - thread 2 executes the whole function, does "native_platform =
>> _EGL_NATIVE_PLATFORM" and just before returning it's preemted
>> - thread 1 wakes up and calls _eglGetNativePlatformFromEnv() which
>> returns _EGL_INVALID_PLATFORM because no env vars are set, updates
>> native_platform and then gets preemted again
>> - thread 2 wakes up and returns wrong _EGL_INVALID_PLATFORM
>>
> Afaict this/similar fix is necessary, yet let me put an alternative
> idea - add locking via _eglGlobal.Mutex.
> May be an bit of overkill, but it's what we consistently use
> throughout the code base.

It looks like that mutex is meant to protect _eglGlobal and not some
random variables, so I'd prefer the atomic version.

> Reverting the patch (as suggested by Grazvydas) does not fully address
> the problem, but only makes it less likely to hit.

Yeah I meant reverting instead of taking 1/2 and then applying this
fix on top. OTOH I'm also ok with this series as-is.

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

Re: [Mesa-dev] [PATCH mesa 2/2] egl/display: make platform detection thread-safe

[Grazvydas Ignotas]

On Thu, Jun 1, 2017 at 2:15 PM, Eric Engestrom
<eric.engest...@imgtec.com> wrote:
> If the detections methods ever become able to return different results
> for different threads, the if chain might make threads go back and forth
> between invalid and valid platforms.
> Solve this by doing the detection in a local var and only overwriting
> the global one at the end, if no other thread has updated it since.
>
> This means the platform detected in the thread might not be the platform
> returned by the function, but this is a different issue that will need
> to be discussed when this becomes possible.
>
> Reported-by: Grazvydas Ignotas <nota...@gmail.com>

Not really, see https://bugs.freedesktop.org/show_bug.cgi?id=101252

> Signed-off-by: Eric Engestrom <eric.engest...@imgtec.com>
> ---
>
> This is unnecessary in my opinion, but doesn't hurt :)

It is necessary, without it things will work most of the time but
there will be rare failures.
Imagine there are 2 threads that both call _eglGetNativePlatform()
simultaneously:
- thread 1 completes the first "if (native_platform ==
_EGL_INVALID_PLATFORM)" check and is preemted to do something else
- thread 2 executes the whole function, does "native_platform =
_EGL_NATIVE_PLATFORM" and just before returning it's preemted
- thread 1 wakes up and calls _eglGetNativePlatformFromEnv() which
returns _EGL_INVALID_PLATFORM because no env vars are set, updates
native_platform and then gets preemted again
- thread 2 wakes up and returns wrong _EGL_INVALID_PLATFORM

> Putting it out there as a possible fix, but my vote is to not merge it,
> not until this per-thread-platform-detection becomes possible anyway.
> ---
>  src/egl/main/egldisplay.c | 17 +++--
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
> index 3f7752f53d..92586def0c 100644
> --- a/src/egl/main/egldisplay.c
> +++ b/src/egl/main/egldisplay.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include 
>  #include "c11/threads.h"
> +#include "util/u_atomic.h"
>
>  #include "eglcontext.h"
>  #include "eglcurrent.h"
> @@ -181,23 +182,27 @@ _EGLPlatformType
>  _eglGetNativePlatform(void *nativeDisplay)
>  {
> static _EGLPlatformType native_platform = _EGL_INVALID_PLATFORM;
> +   _EGLPlatformType detected_platform = native_platform;
> char *detection_method = NULL;
>
> -   if (native_platform == _EGL_INVALID_PLATFORM) {
> -  native_platform = _eglGetNativePlatformFromEnv();
> +   if (detected_platform == _EGL_INVALID_PLATFORM) {
> +  detected_platform = _eglGetNativePlatformFromEnv();
>detection_method = "environment overwrite";
> }
>
> -   if (native_platform == _EGL_INVALID_PLATFORM) {
> -  native_platform = _eglNativePlatformDetectNativeDisplay(nativeDisplay);
> +   if (detected_platform == _EGL_INVALID_PLATFORM) {
> +  detected_platform = 
> _eglNativePlatformDetectNativeDisplay(nativeDisplay);
>detection_method = "autodetected";
> }
>
> -   if (native_platform == _EGL_INVALID_PLATFORM) {
> -  native_platform = _EGL_NATIVE_PLATFORM;
> +   if (detected_platform == _EGL_INVALID_PLATFORM) {
> +  detected_platform = _EGL_NATIVE_PLATFORM;
>detection_method = "build-time configuration";
> }
>
> +   p_atomic_cmpxchg(_platform, _EGL_INVALID_PLATFORM,
> +detected_platform);

I'd make the first "if (detected_platform == _EGL_INVALID_PLATFORM) {"
block end here to avoid doing the slow cmpxchg every time, which makes
it similar to how it looked before your original  patch that I wanted
to revert.

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

[Mesa-dev] [PATCH] Revert "egl/display: remove unnecessary code and make it easier to read"

[Grazvydas Ignotas]

This reverts commit 7adb9b094894a512c019b3378eb9e3c69d830edc.

The "No functional change" claim is wrong, after that commit, the
_eglGetNativePlatformFromEnv() function is now called every time instead
of just once it was called before. What's worse, when
_eglGetNativePlatform() is called from multiple threads, the static
native_platform variable may now alternate between _EGL_INVALID_PLATFORM
and some other value, which causes some of the threads to see and return
unintended result.

The old code is not exactly thread safe either, but because the code
stops changing 'native_platform' as soon as it's not
_EGL_INVALID_PLATFORM, it should not cause problems in practice.

Cc: Eric Engestrom <eric.engest...@imgtec.com>
Fixes: 7adb9b0948 "egl/display: remove unnecessary code and make it easier to 
read"
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101252
Signed-off-by: Grazvydas Ignotas <nota...@gmail.com>
---
 src/egl/main/egldisplay.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index 2a1f027..6e007a1 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -178,28 +178,29 @@ _eglNativePlatformDetectNativeDisplay(void *nativeDisplay)
  * Return the native platform.  It is the platform of the EGL native types.
  */
 _EGLPlatformType
 _eglGetNativePlatform(void *nativeDisplay)
 {
-   static _EGLPlatformType native_platform;
-   char *detection_method;
-
-   native_platform = _eglGetNativePlatformFromEnv();
-   detection_method = "environment overwrite";
-
-   if (native_platform == _EGL_INVALID_PLATFORM) {
-  native_platform = _eglNativePlatformDetectNativeDisplay(nativeDisplay);
-  detection_method = "autodetected";
-   }
+   static _EGLPlatformType native_platform = _EGL_INVALID_PLATFORM;
+   char *detection_method = NULL;
 
if (native_platform == _EGL_INVALID_PLATFORM) {
-  native_platform = _EGL_NATIVE_PLATFORM;
-  detection_method = "build-time configuration";
+  native_platform = _eglGetNativePlatformFromEnv();
+  detection_method = "environment overwrite";
+  if (native_platform == _EGL_INVALID_PLATFORM) {
+ native_platform = 
_eglNativePlatformDetectNativeDisplay(nativeDisplay);
+ detection_method = "autodetected";
+ if (native_platform == _EGL_INVALID_PLATFORM) {
+native_platform = _EGL_NATIVE_PLATFORM;
+detection_method = "build-time configuration";
+ }
+  }
}
 
-   _eglLog(_EGL_DEBUG, "Native platform type: %s (%s)",
-   egl_platforms[native_platform].name, detection_method);
+   if (detection_method != NULL)
+  _eglLog(_EGL_DEBUG, "Native platform type: %s (%s)",
+  egl_platforms[native_platform].name, detection_method);
 
return native_platform;
 }
 
 
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 1/9] util: Add a string buffer implementation

[Grazvydas Ignotas]

On Sun, May 21, 2017 at 11:49 PM, Thomas Helland
 wrote:
> Based on Vladislav Egorov's work on the preprocessor, but split
> out to a util functionality that should be universal. Setup, teardown,
> memory handling and general layout is modeled around the hash_table
> and the set, to make it familiar for everyone.
>
> A notable change is that this implementation is always null terminated.
> The rationale is that it will be less error-prone, as one might
> access the buffer directly, thereby reading a non-terminated string.
> Also, vsnprintf and friends prints the null-terminator.
> ---
>  src/util/Makefile.sources |   2 +
>  src/util/string_buffer.c  | 180 
> ++
>  src/util/string_buffer.h  |  75 +++
>  3 files changed, 257 insertions(+)
>  create mode 100644 src/util/string_buffer.c
>  create mode 100644 src/util/string_buffer.h
>
> diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
> index e9057343dc..cb46639c87 100644
> --- a/src/util/Makefile.sources
> +++ b/src/util/Makefile.sources
> @@ -37,6 +37,8 @@ MESA_UTIL_FILES := \
> simple_list.h \
> slab.c \
> slab.h \
> +   string_buffer.c \
> +   string_buffer.h \
> string_to_uint_map.cpp \
> string_to_uint_map.h \
> strndup.h \
> diff --git a/src/util/string_buffer.c b/src/util/string_buffer.c
> new file mode 100644
> index 00..a500ffd389
> --- /dev/null
> +++ b/src/util/string_buffer.c
> @@ -0,0 +1,180 @@
> +/*
> + * Copyright © 2017 Thomas Helland
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +#include "ralloc.h"
> +#include "string_buffer.h"
> +
> +static bool
> +ensure_capacity(struct _mesa_string_buffer *str, uint32_t needed_capacity)
> +{
> +   if (needed_capacity <= str->capacity)
> +  return true;
> +
> +   /* Too small, we have to resize our buffer.
> +* Double until we can fit the new string.
> +*/
> +   uint32_t new_capacity = str->capacity * 2;
> +   while (needed_capacity > new_capacity)
> +  new_capacity *= 2;
> +
> +   str->buf = reralloc_array_size(str, str->buf, sizeof(char), new_capacity);
> +   if (str->buf == NULL)
> +  return false;
> +
> +   str->capacity = new_capacity;
> +   return true;
> +}
> +
> +struct _mesa_string_buffer *
> +_mesa_string_buffer_create(void *mem_ctx, uint32_t initial_capacity)
> +{
> +   struct _mesa_string_buffer *str;
> +   str = ralloc(mem_ctx, struct _mesa_string_buffer);
> +
> +   if (str == NULL)
> +  return NULL;
> +
> +   /* If no initial capacity is set then set it to something */
> +   str->capacity = initial_capacity ? initial_capacity : 8;
> +   str->buf = ralloc_array(str, char, str->capacity);
> +   str->length = 0;
> +   str->buf[str->length] = '\0';
> +   return str;
> +}
> +
> +void
> +_mesa_string_buffer_destroy(struct _mesa_string_buffer *str)
> +{
> +   ralloc_free(str);
> +}
> +
> +bool
> +_mesa_string_buffer_append(struct _mesa_string_buffer *str, char *c)

const char *c

> +{
> +   return _mesa_string_buffer_append_len(str, c, strlen(c));
> +}
> +
> +bool
> +_mesa_string_buffer_append_all(struct _mesa_string_buffer *str,
> +   uint32_t num_args, ...)
> +{
> +   int i;
> +   char* s;
> +   va_list args;
> +   va_start(args, num_args);
> +   for (i = 0; i < num_args; i++) {
> +  s = va_arg(args, char*);
> +  if (!_mesa_string_buffer_append_len(str, s, strlen(s)))
> + return false;
> +   }
> +   va_end(args);
> +   return true;
> +}
> +
> +bool
> +_mesa_string_buffer_append_len(struct _mesa_string_buffer *str,
> +   char *c, uint32_t len)
> +{
> +   uint32_t needed_length = str->length + len + 1;
> +   if (!ensure_capacity(str, needed_length))
> +  return false;
> +
> +   memcpy(str->buf + str->length, c, len);