date:20181128

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

Bug 106287 Summary: 18.0.1 introduced glitches in Dying Light
https://bugs.freedesktop.org/show_bug.cgi?id=106287

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |DUPLICATE

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

[Mesa-dev] [PATCH 5/7] util/queue: hold a lock when reading num_threads in util_queue_finish

From: Marek Olšák 

---
 src/util/u_queue.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/util/u_queue.c b/src/util/u_queue.c
index 5aaf60ae78e..612ad5e83c6 100644
--- a/src/util/u_queue.c
+++ b/src/util/u_queue.c
@@ -582,29 +582,29 @@ util_queue_finish_execute(void *data, int num_thread)
util_barrier_wait(barrier);
 }
 
 /**
  * Wait until all previously added jobs have completed.
  */
 void
 util_queue_finish(struct util_queue *queue)
 {
util_barrier barrier;
-   struct util_queue_fence *fences = malloc(queue->num_threads * 
sizeof(*fences));
-
-   util_barrier_init(, queue->num_threads);
+   struct util_queue_fence *fences;
 
/* If 2 threads were adding jobs for 2 different barries at the same time,
 * a deadlock would happen, because 1 barrier requires that all threads
 * wait for it exclusively.
 */
mtx_lock(>finish_lock);
+   fences = malloc(queue->num_threads * sizeof(*fences));
+   util_barrier_init(, queue->num_threads);
 
for (unsigned i = 0; i < queue->num_threads; ++i) {
   util_queue_fence_init([i]);
   util_queue_add_job(queue, , [i], 
util_queue_finish_execute, NULL);
}
 
for (unsigned i = 0; i < queue->num_threads; ++i) {
   util_queue_fence_wait([i]);
   util_queue_fence_destroy([i]);
}
-- 
2.17.1

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

[Mesa-dev] [PATCH 4/7] util/queue: add ability to kill a subset of threads

From: Marek Olšák 

for ARB_parallel_shader_compile
---
 src/util/u_queue.c | 49 +-
 src/util/u_queue.h |  5 ++---
 2 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/src/util/u_queue.c b/src/util/u_queue.c
index 48c5c79552d..5aaf60ae78e 100644
--- a/src/util/u_queue.c
+++ b/src/util/u_queue.c
@@ -26,42 +26,43 @@
 
 #include "u_queue.h"
 
 #include 
 
 #include "util/os_time.h"
 #include "util/u_string.h"
 #include "util/u_thread.h"
 #include "u_process.h"
 
-static void util_queue_killall_and_wait(struct util_queue *queue);
+static void
+util_queue_kill_threads(struct util_queue *queue, unsigned keep_num_threads);
 
 /
  * Wait for all queues to assert idle when exit() is called.
  *
  * Otherwise, C++ static variable destructors can be called while threads
  * are using the static variables.
  */
 
 static once_flag atexit_once_flag = ONCE_FLAG_INIT;
 static struct list_head queue_list;
 static mtx_t exit_mutex = _MTX_INITIALIZER_NP;
 
 static void
 atexit_handler(void)
 {
struct util_queue *iter;
 
mtx_lock(_mutex);
/* Wait for all queues to assert idle. */
LIST_FOR_EACH_ENTRY(iter, _list, head) {
-  util_queue_killall_and_wait(iter);
+  util_queue_kill_threads(iter, 0);
}
mtx_unlock(_mutex);
 }
 
 static void
 global_init(void)
 {
LIST_INITHEAD(_list);
atexit(atexit_handler);
 }
@@ -259,55 +260,58 @@ util_queue_thread_func(void *input)
   u_thread_setname(name);
}
 
while (1) {
   struct util_queue_job job;
 
   mtx_lock(>lock);
   assert(queue->num_queued >= 0 && queue->num_queued <= queue->max_jobs);
 
   /* wait if the queue is empty */
-  while (!queue->kill_threads && queue->num_queued == 0)
+  while (thread_index < queue->num_threads && queue->num_queued == 0)
  cnd_wait(>has_queued_cond, >lock);
 
-  if (queue->kill_threads) {
+  /* only kill threads that are above "num_threads" */
+  if (thread_index >= queue->num_threads) {
  mtx_unlock(>lock);
  break;
   }
 
   job = queue->jobs[queue->read_idx];
   memset(>jobs[queue->read_idx], 0, sizeof(struct util_queue_job));
   queue->read_idx = (queue->read_idx + 1) % queue->max_jobs;
 
   queue->num_queued--;
   cnd_signal(>has_space_cond);
   mtx_unlock(>lock);
 
   if (job.job) {
  job.execute(job.job, thread_index);
  util_queue_fence_signal(job.fence);
  if (job.cleanup)
 job.cleanup(job.job, thread_index);
   }
}
 
-   /* signal remaining jobs before terminating */
+   /* signal remaining jobs if all threads are being terminated */
mtx_lock(>lock);
-   for (unsigned i = queue->read_idx; i != queue->write_idx;
-i = (i + 1) % queue->max_jobs) {
-  if (queue->jobs[i].job) {
- util_queue_fence_signal(queue->jobs[i].fence);
- queue->jobs[i].job = NULL;
+   if (queue->num_threads == 0) {
+  for (unsigned i = queue->read_idx; i != queue->write_idx;
+   i = (i + 1) % queue->max_jobs) {
+ if (queue->jobs[i].job) {
+util_queue_fence_signal(queue->jobs[i].fence);
+queue->jobs[i].job = NULL;
+ }
   }
+  queue->read_idx = queue->write_idx;
+  queue->num_queued = 0;
}
-   queue->read_idx = queue->write_idx;
-   queue->num_queued = 0;
mtx_unlock(>lock);
return 0;
 }
 
 static bool
 util_queue_create_thread(struct util_queue *queue, unsigned index)
 {
struct thread_input *input =
   (struct thread_input *) malloc(sizeof(struct thread_input));
input->queue = queue;
@@ -418,60 +422,69 @@ fail:
   cnd_destroy(>has_queued_cond);
   mtx_destroy(>lock);
   free(queue->jobs);
}
/* also util_queue_is_initialized can be used to check for success */
memset(queue, 0, sizeof(*queue));
return false;
 }
 
 static void
-util_queue_killall_and_wait(struct util_queue *queue)
+util_queue_kill_threads(struct util_queue *queue, unsigned keep_num_threads)
 {
unsigned i;
 
/* Signal all threads to terminate. */
+   mtx_lock(>finish_lock);
+
+   if (keep_num_threads >= queue->num_threads) {
+  mtx_unlock(>finish_lock);
+  return;
+   }
+
mtx_lock(>lock);
-   queue->kill_threads = 1;
+   unsigned old_num_threads = queue->num_threads;
+   queue->num_threads = keep_num_threads;
cnd_broadcast(>has_queued_cond);
mtx_unlock(>lock);
 
-   for (i = 0; i < queue->num_threads; i++)
+   for (i = keep_num_threads; i < old_num_threads; i++)
   thrd_join(queue->threads[i], NULL);
-   queue->num_threads = 0;
+
+   mtx_unlock(>finish_lock);
 }
 
 void
 util_queue_destroy(struct util_queue *queue)
 {
-   util_queue_killall_and_wait(queue);
+   util_queue_kill_threads(queue, 0);
remove_from_atexit_list(queue);
 
cnd_destroy(>has_space_cond);
cnd_destroy(>has_queued_cond);

[Mesa-dev] [PATCH 6/7] util/queue: add util_queue_adjust_num_threads

From: Marek Olšák 

for ARB_parallel_shader_compile
---
 src/util/u_queue.c | 50 --
 src/util/u_queue.h |  8 
 2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/src/util/u_queue.c b/src/util/u_queue.c
index 612ad5e83c6..383a9c09919 100644
--- a/src/util/u_queue.c
+++ b/src/util/u_queue.c
@@ -27,42 +27,43 @@
 #include "u_queue.h"
 
 #include 
 
 #include "util/os_time.h"
 #include "util/u_string.h"
 #include "util/u_thread.h"
 #include "u_process.h"
 
 static void
-util_queue_kill_threads(struct util_queue *queue, unsigned keep_num_threads);
+util_queue_kill_threads(struct util_queue *queue, unsigned keep_num_threads,
+bool finish_locked);
 
 /
  * Wait for all queues to assert idle when exit() is called.
  *
  * Otherwise, C++ static variable destructors can be called while threads
  * are using the static variables.
  */
 
 static once_flag atexit_once_flag = ONCE_FLAG_INIT;
 static struct list_head queue_list;
 static mtx_t exit_mutex = _MTX_INITIALIZER_NP;
 
 static void
 atexit_handler(void)
 {
struct util_queue *iter;
 
mtx_lock(_mutex);
/* Wait for all queues to assert idle. */
LIST_FOR_EACH_ENTRY(iter, _list, head) {
-  util_queue_kill_threads(iter, 0);
+  util_queue_kill_threads(iter, 0, false);
}
mtx_unlock(_mutex);
 }
 
 static void
 global_init(void)
 {
LIST_INITHEAD(_list);
atexit(atexit_handler);
 }
@@ -333,20 +334,53 @@ util_queue_create_thread(struct util_queue *queue, 
unsigned index)
*
* Note that Linux only allows decreasing the priority. The original
* priority can't be restored.
*/
   pthread_setschedparam(queue->threads[index], SCHED_IDLE, _param);
 #endif
}
return true;
 }
 
+void
+util_queue_adjust_num_threads(struct util_queue *queue, unsigned num_threads)
+{
+   num_threads = MIN2(num_threads, queue->max_threads);
+   num_threads = MAX2(num_threads, 1);
+
+   mtx_lock(>finish_lock);
+   unsigned old_num_threads = queue->num_threads;
+
+   if (num_threads == old_num_threads) {
+  mtx_unlock(>finish_lock);
+  return;
+   }
+
+   if (num_threads < old_num_threads) {
+  util_queue_kill_threads(queue, num_threads, true);
+  mtx_unlock(>finish_lock);
+  return;
+   }
+
+   /* Create threads.
+*
+* We need to update num_threads first, because threads terminate
+* when thread_index < num_threads.
+*/
+   queue->num_threads = num_threads;
+   for (unsigned i = old_num_threads; i < num_threads; i++) {
+  if (!util_queue_create_thread(queue, i))
+ break;
+   }
+   mtx_unlock(>finish_lock);
+}
+
 bool
 util_queue_init(struct util_queue *queue,
 const char *name,
 unsigned max_jobs,
 unsigned num_threads,
 unsigned flags)
 {
unsigned i;
 
/* Form the thread name from process_name and name, limited to 13
@@ -371,20 +405,21 @@ util_queue_init(struct util_queue *queue,
memset(queue, 0, sizeof(*queue));
 
if (process_len) {
   util_snprintf(queue->name, sizeof(queue->name), "%.*s:%s",
 process_len, process_name, name);
} else {
   util_snprintf(queue->name, sizeof(queue->name), "%s", name);
}
 
queue->flags = flags;
+   queue->max_threads = num_threads;
queue->num_threads = num_threads;
queue->max_jobs = max_jobs;
 
queue->jobs = (struct util_queue_job*)
  calloc(max_jobs, sizeof(struct util_queue_job));
if (!queue->jobs)
   goto fail;
 
(void) mtx_init(>lock, mtx_plain);
(void) mtx_init(>finish_lock, mtx_plain);
@@ -422,48 +457,51 @@ fail:
   cnd_destroy(>has_queued_cond);
   mtx_destroy(>lock);
   free(queue->jobs);
}
/* also util_queue_is_initialized can be used to check for success */
memset(queue, 0, sizeof(*queue));
return false;
 }
 
 static void
-util_queue_kill_threads(struct util_queue *queue, unsigned keep_num_threads)
+util_queue_kill_threads(struct util_queue *queue, unsigned keep_num_threads,
+bool finish_locked)
 {
unsigned i;
 
/* Signal all threads to terminate. */
-   mtx_lock(>finish_lock);
+   if (!finish_locked)
+  mtx_lock(>finish_lock);
 
if (keep_num_threads >= queue->num_threads) {
   mtx_unlock(>finish_lock);
   return;
}
 
mtx_lock(>lock);
unsigned old_num_threads = queue->num_threads;
queue->num_threads = keep_num_threads;
cnd_broadcast(>has_queued_cond);
mtx_unlock(>lock);
 
for (i = keep_num_threads; i < old_num_threads; i++)
   thrd_join(queue->threads[i], NULL);
 
-   mtx_unlock(>finish_lock);
+   if (!finish_locked)
+  mtx_unlock(>finish_lock);
 }
 
 void
 util_queue_destroy(struct util_queue *queue)
 {
-   util_queue_kill_threads(queue, 0);
+   util_queue_kill_threads(queue, 0, false);

[Mesa-dev] [PATCH 1/7] mesa: implement ARB/KHR_parallel_shader_compile

From: Marek Olšák 

Tested by piglit.
---
 docs/features.txt   |  2 +-
 docs/relnotes/19.0.0.html   |  2 ++
 src/mapi/glapi/gen/gl_API.xml   | 15 ++-
 src/mesa/main/dd.h  |  7 +++
 src/mesa/main/extensions_table.h|  2 ++
 src/mesa/main/get_hash_params.py|  3 +++
 src/mesa/main/hint.c| 12 
 src/mesa/main/hint.h|  4 
 src/mesa/main/mtypes.h  |  1 +
 src/mesa/main/shaderapi.c   | 10 ++
 src/mesa/main/tests/dispatch_sanity.cpp |  4 
 11 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/docs/features.txt b/docs/features.txt
index 8999e42519c..7b827de6a92 100644
--- a/docs/features.txt
+++ b/docs/features.txt
@@ -295,21 +295,21 @@ GLES3.2, GLSL ES 3.2 -- all DONE: i965/gen9+, radeonsi, 
virgl
   GL_OES_texture_storage_multisample_2d_array   DONE (all drivers that 
support GL_ARB_texture_multisample)
 
 Khronos, ARB, and OES extensions that are not part of any OpenGL or OpenGL ES 
version:
 
   GL_ARB_bindless_texture   DONE (nvc0, radeonsi)
   GL_ARB_cl_event   not started
   GL_ARB_compute_variable_group_sizeDONE (nvc0, radeonsi)
   GL_ARB_ES3_2_compatibilityDONE (i965/gen8+, 
radeonsi, virgl)
   GL_ARB_fragment_shader_interlock  DONE (i965)
   GL_ARB_gpu_shader_int64   DONE (i965/gen8+, 
nvc0, radeonsi, softpipe, llvmpipe)
-  GL_ARB_parallel_shader_compilenot started, but 
Chia-I Wu did some related work in 2014
+  GL_ARB_parallel_shader_compileDONE (all drivers)
   GL_ARB_post_depth_coverageDONE (i965, nvc0)
   GL_ARB_robustness_isolation   not started
   GL_ARB_sample_locations   DONE (nvc0)
   GL_ARB_seamless_cubemap_per_texture   DONE (freedreno, i965, 
nvc0, radeonsi, r600, softpipe, swr, virgl)
   GL_ARB_shader_ballot  DONE (i965/gen8+, 
nvc0, radeonsi)
   GL_ARB_shader_clock   DONE (i965/gen7+, 
nv50, nvc0, r600, radeonsi, virgl)
   GL_ARB_shader_stencil_export  DONE (i965/gen9+, 
r600, radeonsi, softpipe, llvmpipe, swr, virgl)
   GL_ARB_shader_viewport_layer_arrayDONE (i965/gen6+, 
nvc0, radeonsi)
   GL_ARB_sparse_buffer  DONE (radeonsi/CIK+)
   GL_ARB_sparse_texture not started
diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
index bc1776e8f4e..540482bca5f 100644
--- a/docs/relnotes/19.0.0.html
+++ b/docs/relnotes/19.0.0.html
@@ -33,24 +33,26 @@ Compatibility contexts may report a lower version depending 
on each driver.
 SHA256 checksums
 
 TBD.
 
 
 
 New features
 
 
 GL_AMD_texture_texture4 on all GL 4.0 drivers.
+GL_ARB_parallel_shader_compile on all drivers.
 GL_EXT_shader_implicit_conversions on all drivers (ES extension).
 GL_EXT_texture_compression_bptc on all GL 4.0 drivers (ES extension).
 GL_EXT_texture_compression_rgtc on all GL 3.0 drivers (ES extension).
 GL_EXT_texture_view on drivers supporting texture views (ES extension).
+GL_KHR_parallel_shader_compile on all drivers.
 GL_OES_texture_view on drivers supporting texture views (ES 
extension).
 
 
 Bug fixes
 
 
 TBD
 
 
 Changes
diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index f4d0808f13b..4ce691b361b 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -8402,21 +8402,34 @@
 
 
 
 
 
 
 
 
 http://www.w3.org/2001/XInclude"/>
 
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 
 http://www.w3.org/2001/XInclude"/>
 
 
 
 
 
 
 
 
diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
index f14c3e04e91..92b6ecac33c 100644
--- a/src/mesa/main/dd.h
+++ b/src/mesa/main/dd.h
@@ -1292,20 +1292,27 @@ struct dd_function_table {
/**
 * Called to initialize gl_program::driver_cache_blob (and size) with a
 * ralloc allocated buffer.
 *
 * This buffer will be saved and restored as part of the gl_program
 * serialization and deserialization.
 */
void (*ShaderCacheSerializeDriverBlob)(struct gl_context *ctx,
   struct gl_program *prog);
/*@}*/
+
+   /**
+* \name Set the number of compiler threads for ARB_parallel_shader_compile
+*/
+   void (*SetMaxShaderCompilerThreads)(struct gl_context *ctx, unsigned count);
+   bool (*GetShaderProgramCompletionStatus)(struct gl_context *ctx,
+struct gl_shader_program *shprog);
 };
 
 
 /**
  * Per-vertex functions.
  *
  * These are the functions which can appear

[Mesa-dev] [PATCH 7/7] radeonsi: implement ARB/KHR_parallel_shader_compile callbacks

From: Marek Olšák 

---
 src/gallium/drivers/radeonsi/si_pipe.c | 31 ++
 1 file changed, 31 insertions(+)

diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
b/src/gallium/drivers/radeonsi/si_pipe.c
index 503d8331906..cc56ce0c446 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -19,20 +19,21 @@
  * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
  * THE AUTHOR(S) AND/OR THEIR SUPPLIERS 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 "si_pipe.h"
 #include "si_public.h"
 #include "si_shader_internal.h"
+#include "si_compute.h"
 #include "sid.h"
 
 #include "ac_llvm_util.h"
 #include "radeon/radeon_uvd.h"
 #include "gallivm/lp_bld_misc.h"
 #include "util/disk_cache.h"
 #include "util/u_log.h"
 #include "util/u_memory.h"
 #include "util/u_suballoc.h"
 #include "util/u_tests.h"
@@ -859,20 +860,46 @@ static void si_disk_cache_create(struct si_screen 
*sscreen)
 */
STATIC_ASSERT(ALL_FLAGS <= UINT_MAX);
shader_debug_flags |= (uint64_t)sscreen->info.address32_hi << 32;
 
sscreen->disk_shader_cache =
disk_cache_create(sscreen->info.name,
  cache_id,
  shader_debug_flags);
 }
 
+static void si_set_max_shader_compiler_threads(struct pipe_screen *screen,
+  unsigned max_threads)
+{
+   struct si_screen *sscreen = (struct si_screen *)screen;
+
+   /* This function doesn't allow a greater number of threads than
+* the queue had at its creation. */
+   util_queue_adjust_num_threads(>shader_compiler_queue,
+ max_threads);
+   /* Don't change the number of threads on the low priority queue. */
+}
+
+static bool si_is_parallel_shader_compilation_finished(struct pipe_screen 
*screen,
+  void *shader,
+  unsigned shader_type)
+{
+   if (shader_type == PIPE_SHADER_COMPUTE) {
+   struct si_compute *cs = (struct si_compute*)shader;
+
+   return util_queue_fence_is_signalled(>ready);
+   }
+   struct si_shader_selector *sel = (struct si_shader_selector *)shader;
+
+   return util_queue_fence_is_signalled(>ready);
+}
+
 struct pipe_screen *radeonsi_screen_create(struct radeon_winsys *ws,
   const struct pipe_screen_config 
*config)
 {
struct si_screen *sscreen = CALLOC_STRUCT(si_screen);
unsigned hw_threads, num_comp_hi_threads, num_comp_lo_threads, i;
 
if (!sscreen) {
return NULL;
}
 
@@ -888,20 +915,24 @@ struct pipe_screen *radeonsi_screen_create(struct 
radeon_winsys *ws,
 >pa_sc_raster_config_1,
 >se_tile_repeat);
}
 
sscreen->debug_flags = debug_get_flags_option("R600_DEBUG",
debug_options, 0);
 
/* Set functions first. */
sscreen->b.context_create = si_pipe_create_context;
sscreen->b.destroy = si_destroy_screen;
+   sscreen->b.set_max_shader_compiler_threads =
+   si_set_max_shader_compiler_threads;
+   sscreen->b.is_parallel_shader_compilation_finished =
+   si_is_parallel_shader_compilation_finished;
 
si_init_screen_get_functions(sscreen);
si_init_screen_buffer_functions(sscreen);
si_init_screen_fence_functions(sscreen);
si_init_screen_state_functions(sscreen);
si_init_screen_texture_functions(sscreen);
si_init_screen_query_functions(sscreen);
 
/* Set these flags in debug_flags early, so that the shader cache takes
 * them into account.
-- 
2.17.1

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

[Mesa-dev] [PATCH 3/7] util/queue: move thread creation into a separate function

From: Marek Olšák 

---
 src/util/u_queue.c | 56 ++
 1 file changed, 32 insertions(+), 24 deletions(-)

diff --git a/src/util/u_queue.c b/src/util/u_queue.c
index 3812c824b6d..48c5c79552d 100644
--- a/src/util/u_queue.c
+++ b/src/util/u_queue.c
@@ -298,20 +298,51 @@ util_queue_thread_func(void *input)
  util_queue_fence_signal(queue->jobs[i].fence);
  queue->jobs[i].job = NULL;
   }
}
queue->read_idx = queue->write_idx;
queue->num_queued = 0;
mtx_unlock(>lock);
return 0;
 }
 
+static bool
+util_queue_create_thread(struct util_queue *queue, unsigned index)
+{
+   struct thread_input *input =
+  (struct thread_input *) malloc(sizeof(struct thread_input));
+   input->queue = queue;
+   input->thread_index = index;
+
+   queue->threads[index] = u_thread_create(util_queue_thread_func, input);
+
+   if (!queue->threads[index]) {
+  free(input);
+  return false;
+   }
+
+   if (queue->flags & UTIL_QUEUE_INIT_USE_MINIMUM_PRIORITY) {
+#if defined(__linux__) && defined(SCHED_IDLE)
+  struct sched_param sched_param = {0};
+
+  /* The nice() function can only set a maximum of 19.
+   * SCHED_IDLE is the same as nice = 20.
+   *
+   * Note that Linux only allows decreasing the priority. The original
+   * priority can't be restored.
+   */
+  pthread_setschedparam(queue->threads[index], SCHED_IDLE, _param);
+#endif
+   }
+   return true;
+}
+
 bool
 util_queue_init(struct util_queue *queue,
 const char *name,
 unsigned max_jobs,
 unsigned num_threads,
 unsigned flags)
 {
unsigned i;
 
/* Form the thread name from process_name and name, limited to 13
@@ -357,53 +388,30 @@ util_queue_init(struct util_queue *queue,
queue->num_queued = 0;
cnd_init(>has_queued_cond);
cnd_init(>has_space_cond);
 
queue->threads = (thrd_t*) calloc(num_threads, sizeof(thrd_t));
if (!queue->threads)
   goto fail;
 
/* start threads */
for (i = 0; i < num_threads; i++) {
-  struct thread_input *input =
- (struct thread_input *) malloc(sizeof(struct thread_input));
-  input->queue = queue;
-  input->thread_index = i;
-
-  queue->threads[i] = u_thread_create(util_queue_thread_func, input);
-
-  if (!queue->threads[i]) {
- free(input);
-
+  if (!util_queue_create_thread(queue, i)) {
  if (i == 0) {
 /* no threads created, fail */
 goto fail;
  } else {
 /* at least one thread created, so use it */
 queue->num_threads = i;
 break;
  }
   }
-
-  if (flags & UTIL_QUEUE_INIT_USE_MINIMUM_PRIORITY) {
-   #if defined(__linux__) && defined(SCHED_IDLE)
- struct sched_param sched_param = {0};
-
- /* The nice() function can only set a maximum of 19.
-  * SCHED_IDLE is the same as nice = 20.
-  *
-  * Note that Linux only allows decreasing the priority. The original
-  * priority can't be restored.
-  */
- pthread_setschedparam(queue->threads[i], SCHED_IDLE, _param);
-   #endif
-  }
}
 
add_to_atexit_list(queue);
return true;
 
 fail:
free(queue->threads);
 
if (queue->jobs) {
   cnd_destroy(>has_space_cond);
-- 
2.17.1

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

[Mesa-dev] [PATCH 2/7] gallium: implement ARB/KHR_parallel_shader_compile

From: Marek Olšák 

---
 src/gallium/include/pipe/p_screen.h| 13 ++
 src/mesa/state_tracker/st_cb_program.c | 59 +-
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/src/gallium/include/pipe/p_screen.h 
b/src/gallium/include/pipe/p_screen.h
index c4d6e1cc94f..d4e2d9f63ac 100644
--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -435,20 +435,33 @@ struct pipe_screen {
 * \param uuidpointer to a memory region of PIPE_UUID_SIZE bytes
 */
void (*get_driver_uuid)(struct pipe_screen *screen, char *uuid);
 
/**
 * Fill @uuid with a unique device identifier
 *
 * \param uuidpointer to a memory region of PIPE_UUID_SIZE bytes
 */
void (*get_device_uuid)(struct pipe_screen *screen, char *uuid);
+
+   /**
+* Set the maximum number of parallel shader compiler threads.
+*/
+   void (*set_max_shader_compiler_threads)(struct pipe_screen *screen,
+   unsigned max_threads);
+
+   /**
+* Return whether parallel shader compilation has finished.
+*/
+   bool (*is_parallel_shader_compilation_finished)(struct pipe_screen *screen,
+   void *shader,
+   unsigned shader_type);
 };
 
 
 /**
  * Global configuration options for screen creation.
  */
 struct pipe_screen_config {
const struct driOptionCache *options;
 };
 
diff --git a/src/mesa/state_tracker/st_cb_program.c 
b/src/mesa/state_tracker/st_cb_program.c
index 555fc5d5ad9..cc96ec552bb 100644
--- a/src/mesa/state_tracker/st_cb_program.c
+++ b/src/mesa/state_tracker/st_cb_program.c
@@ -259,23 +259,80 @@ st_program_string_notify( struct gl_context *ctx,
 static struct gl_program *
 st_new_ati_fs(struct gl_context *ctx, struct ati_fragment_shader *curProg)
 {
struct gl_program *prog = ctx->Driver.NewProgram(ctx, 
GL_FRAGMENT_PROGRAM_ARB,
  curProg->Id, true);
struct st_fragment_program *stfp = (struct st_fragment_program *)prog;
stfp->ati_fs = curProg;
return prog;
 }
 
+static void
+st_max_shader_compiler_threads(struct gl_context *ctx, unsigned count)
+{
+   struct pipe_screen *screen = st_context(ctx)->pipe->screen;
+
+   if (screen->set_max_shader_compiler_threads)
+  screen->set_max_shader_compiler_threads(screen, count);
+}
+
+static bool
+st_get_shader_program_completion_status(struct gl_context *ctx,
+struct gl_shader_program *shprog)
+{
+   struct pipe_screen *screen = st_context(ctx)->pipe->screen;
+
+   if (!screen->is_parallel_shader_compilation_finished)
+  return true;
+
+   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
+  struct gl_linked_shader *linked = shprog->_LinkedShaders[i];
+  void *sh = NULL;
+
+  if (!linked || !linked->Program)
+ continue;
+
+  switch (i) {
+  case MESA_SHADER_VERTEX:
+ if (st_vertex_program(linked->Program)->variants)
+sh = st_vertex_program(linked->Program)->variants->driver_shader;
+ break;
+  case MESA_SHADER_FRAGMENT:
+ if (st_fragment_program(linked->Program)->variants)
+sh = st_fragment_program(linked->Program)->variants->driver_shader;
+ break;
+  case MESA_SHADER_TESS_CTRL:
+  case MESA_SHADER_TESS_EVAL:
+  case MESA_SHADER_GEOMETRY:
+ if (st_common_program(linked->Program)->variants)
+sh = st_common_program(linked->Program)->variants->driver_shader;
+ break;
+  case MESA_SHADER_COMPUTE:
+ if (st_compute_program(linked->Program)->variants)
+sh = st_compute_program(linked->Program)->variants->driver_shader;
+ break;
+  }
+
+  unsigned type = pipe_shader_type_from_mesa(i);
+
+  if (sh &&
+  !screen->is_parallel_shader_compilation_finished(screen, sh, type))
+ return false;
+   }
+   return true;
+}
+
 /**
  * Plug in the program and shader-related device driver functions.
  */
 void
 st_init_program_functions(struct dd_function_table *functions)
 {
functions->NewProgram = st_new_program;
functions->DeleteProgram = st_delete_program;
functions->ProgramStringNotify = st_program_string_notify;
functions->NewATIfs = st_new_ati_fs;
-   
functions->LinkShader = st_link_shader;
+   functions->SetMaxShaderCompilerThreads = st_max_shader_compiler_threads;
+   functions->GetShaderProgramCompletionStatus =
+  st_get_shader_program_completion_status;
 }
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH 3/3] mesa: expose AMD_texture_texture4

The closed driver treats it the same as textureGather.

Marek

On Wed, Nov 28, 2018 at 8:58 PM Ilia Mirkin  wrote:

> Series is
>
> Reviewed-by: Ilia Mirkin 
>
> Note that AMD_texture_texture4 is only defined for single-component
> textures (not even RED allowed due to the age of the ext -- only ALPHA
> / LUMINANCE / DEPTH_COMPONENT / INTENSITY) -- but it doesn't say what
> will happen when a "bad" texture is supplied. I guess it's reasonable
> to take the ARB_texture_gather interpretation as if the driver
> reported MAX_PROGRAM_TEXTURE_GATHER_COMPONENTS_ARB == 1, i.e. it's the
> user's fault if something doesn't work.
>
>   -ilia
> On Wed, Nov 28, 2018 at 8:22 PM Marek Olšák  wrote:
> >
> > From: Marek Olšák 
> >
> > because the closed driver exposes it. Tested by piglit.
> > ---
> >  docs/relnotes/19.0.0.html|  1 +
> >  src/compiler/glsl/builtin_functions.cpp  | 10 ++
> >  src/compiler/glsl/glsl_parser_extras.cpp |  1 +
> >  src/compiler/glsl/glsl_parser_extras.h   |  2 ++
> >  src/mesa/main/extensions_table.h |  1 +
> >  5 files changed, 15 insertions(+)
> >
> > diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
> > index d10bd2cf720..bc1776e8f4e 100644
> > --- a/docs/relnotes/19.0.0.html
> > +++ b/docs/relnotes/19.0.0.html
> > @@ -32,20 +32,21 @@ Compatibility contexts may report a lower version
> depending on each driver.
> >
> >  SHA256 checksums
> >  
> >  TBD.
> >  
> >
> >
> >  New features
> >
> >  
> > +GL_AMD_texture_texture4 on all GL 4.0 drivers.
> >  GL_EXT_shader_implicit_conversions on all drivers (ES
> extension).
> >  GL_EXT_texture_compression_bptc on all GL 4.0 drivers (ES
> extension).
> >  GL_EXT_texture_compression_rgtc on all GL 3.0 drivers (ES
> extension).
> >  GL_EXT_texture_view on drivers supporting texture views (ES
> extension).
> >  GL_OES_texture_view on drivers supporting texture views (ES
> extension).
> >  
> >
> >  Bug fixes
> >
> >  
> > diff --git a/src/compiler/glsl/builtin_functions.cpp
> b/src/compiler/glsl/builtin_functions.cpp
> > index 5650365d1d5..62fbe10c623 100644
> > --- a/src/compiler/glsl/builtin_functions.cpp
> > +++ b/src/compiler/glsl/builtin_functions.cpp
> > @@ -411,20 +411,26 @@ texture_query_lod(const _mesa_glsl_parse_state
> *state)
> >  static bool
> >  texture_gather_cube_map_array(const _mesa_glsl_parse_state *state)
> >  {
> > return state->is_version(400, 320) ||
> >state->ARB_texture_gather_enable ||
> >state->ARB_gpu_shader5_enable ||
> >state->EXT_texture_cube_map_array_enable ||
> >state->OES_texture_cube_map_array_enable;
> >  }
> >
> > +static bool
> > +texture_texture4(const _mesa_glsl_parse_state *state)
> > +{
> > +   return state->AMD_texture_texture4_enable;
> > +}
> > +
> >  static bool
> >  texture_gather_or_es31(const _mesa_glsl_parse_state *state)
> >  {
> > return state->is_version(400, 310) ||
> >state->ARB_texture_gather_enable ||
> >state->ARB_gpu_shader5_enable;
> >  }
> >
> >  /* Only ARB_texture_gather but not GLSL 4.0 or ARB_gpu_shader5.
> >   * used for relaxation of const offset requirements.
> > @@ -2895,20 +2901,24 @@ builtin_builder::create_builtins()
> >  NULL);
> >
> > add_function("shadow2DRectGradARB",
> >  _texture(ir_txd, shader_texture_lod_and_rect,
> glsl_type::vec4_type,  glsl_type::sampler2DRectShadow_type,
> glsl_type::vec3_type),
> >  NULL);
> >
> > add_function("shadow2DRectProjGradARB",
> >  _texture(ir_txd, shader_texture_lod_and_rect,
> glsl_type::vec4_type,  glsl_type::sampler2DRectShadow_type,
> glsl_type::vec4_type, TEX_PROJECT),
> >  NULL);
> >
> > +   add_function("texture4",
> > +_texture(ir_tg4, texture_texture4,
> glsl_type::vec4_type, glsl_type::sampler2D_type, glsl_type::vec2_type),
> > +NULL);
> > +
> > add_function("textureGather",
> >  _texture(ir_tg4, texture_gather_or_es31,
> glsl_type::vec4_type, glsl_type::sampler2D_type, glsl_type::vec2_type),
> >  _texture(ir_tg4, texture_gather_or_es31,
> glsl_type::ivec4_type, glsl_type::isampler2D_type, glsl_type::vec2_type),
> >  _texture(ir_tg4, texture_gather_or_es31,
> glsl_type::uvec4_type, glsl_type::usampler2D_type, glsl_type::vec2_type),
> >
> >  _texture(ir_tg4, gpu_shader5, glsl_type::vec4_type,
> glsl_type::sampler2DRect_type, glsl_type::vec2_type),
> >  _texture(ir_tg4, gpu_shader5, glsl_type::ivec4_type,
> glsl_type::isampler2DRect_type, glsl_type::vec2_type),
> >  _texture(ir_tg4, gpu_shader5, glsl_type::uvec4_type,
> glsl_type::usampler2DRect_type, glsl_type::vec2_type),
> >
> >  _texture(ir_tg4, texture_gather_or_es31,
> glsl_type::vec4_type, glsl_type::sampler2DArray_type, glsl_type::vec3_type),
> > diff --git a/src/compiler/glsl/glsl_parser_extras.cpp
>

Re: [Mesa-dev] [PATCH 3/3] mesa: expose AMD_texture_texture4

2018-11-28 Thread Ilia Mirkin

Series is

Reviewed-by: Ilia Mirkin 

Note that AMD_texture_texture4 is only defined for single-component
textures (not even RED allowed due to the age of the ext -- only ALPHA
/ LUMINANCE / DEPTH_COMPONENT / INTENSITY) -- but it doesn't say what
will happen when a "bad" texture is supplied. I guess it's reasonable
to take the ARB_texture_gather interpretation as if the driver
reported MAX_PROGRAM_TEXTURE_GATHER_COMPONENTS_ARB == 1, i.e. it's the
user's fault if something doesn't work.

  -ilia
On Wed, Nov 28, 2018 at 8:22 PM Marek Olšák  wrote:
>
> From: Marek Olšák 
>
> because the closed driver exposes it. Tested by piglit.
> ---
>  docs/relnotes/19.0.0.html|  1 +
>  src/compiler/glsl/builtin_functions.cpp  | 10 ++
>  src/compiler/glsl/glsl_parser_extras.cpp |  1 +
>  src/compiler/glsl/glsl_parser_extras.h   |  2 ++
>  src/mesa/main/extensions_table.h |  1 +
>  5 files changed, 15 insertions(+)
>
> diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
> index d10bd2cf720..bc1776e8f4e 100644
> --- a/docs/relnotes/19.0.0.html
> +++ b/docs/relnotes/19.0.0.html
> @@ -32,20 +32,21 @@ Compatibility contexts may report a lower version 
> depending on each driver.
>
>  SHA256 checksums
>  
>  TBD.
>  
>
>
>  New features
>
>  
> +GL_AMD_texture_texture4 on all GL 4.0 drivers.
>  GL_EXT_shader_implicit_conversions on all drivers (ES extension).
>  GL_EXT_texture_compression_bptc on all GL 4.0 drivers (ES extension).
>  GL_EXT_texture_compression_rgtc on all GL 3.0 drivers (ES extension).
>  GL_EXT_texture_view on drivers supporting texture views (ES 
> extension).
>  GL_OES_texture_view on drivers supporting texture views (ES 
> extension).
>  
>
>  Bug fixes
>
>  
> diff --git a/src/compiler/glsl/builtin_functions.cpp 
> b/src/compiler/glsl/builtin_functions.cpp
> index 5650365d1d5..62fbe10c623 100644
> --- a/src/compiler/glsl/builtin_functions.cpp
> +++ b/src/compiler/glsl/builtin_functions.cpp
> @@ -411,20 +411,26 @@ texture_query_lod(const _mesa_glsl_parse_state *state)
>  static bool
>  texture_gather_cube_map_array(const _mesa_glsl_parse_state *state)
>  {
> return state->is_version(400, 320) ||
>state->ARB_texture_gather_enable ||
>state->ARB_gpu_shader5_enable ||
>state->EXT_texture_cube_map_array_enable ||
>state->OES_texture_cube_map_array_enable;
>  }
>
> +static bool
> +texture_texture4(const _mesa_glsl_parse_state *state)
> +{
> +   return state->AMD_texture_texture4_enable;
> +}
> +
>  static bool
>  texture_gather_or_es31(const _mesa_glsl_parse_state *state)
>  {
> return state->is_version(400, 310) ||
>state->ARB_texture_gather_enable ||
>state->ARB_gpu_shader5_enable;
>  }
>
>  /* Only ARB_texture_gather but not GLSL 4.0 or ARB_gpu_shader5.
>   * used for relaxation of const offset requirements.
> @@ -2895,20 +2901,24 @@ builtin_builder::create_builtins()
>  NULL);
>
> add_function("shadow2DRectGradARB",
>  _texture(ir_txd, shader_texture_lod_and_rect, 
> glsl_type::vec4_type,  glsl_type::sampler2DRectShadow_type, 
> glsl_type::vec3_type),
>  NULL);
>
> add_function("shadow2DRectProjGradARB",
>  _texture(ir_txd, shader_texture_lod_and_rect, 
> glsl_type::vec4_type,  glsl_type::sampler2DRectShadow_type, 
> glsl_type::vec4_type, TEX_PROJECT),
>  NULL);
>
> +   add_function("texture4",
> +_texture(ir_tg4, texture_texture4, glsl_type::vec4_type, 
> glsl_type::sampler2D_type, glsl_type::vec2_type),
> +NULL);
> +
> add_function("textureGather",
>  _texture(ir_tg4, texture_gather_or_es31, 
> glsl_type::vec4_type, glsl_type::sampler2D_type, glsl_type::vec2_type),
>  _texture(ir_tg4, texture_gather_or_es31, 
> glsl_type::ivec4_type, glsl_type::isampler2D_type, glsl_type::vec2_type),
>  _texture(ir_tg4, texture_gather_or_es31, 
> glsl_type::uvec4_type, glsl_type::usampler2D_type, glsl_type::vec2_type),
>
>  _texture(ir_tg4, gpu_shader5, glsl_type::vec4_type, 
> glsl_type::sampler2DRect_type, glsl_type::vec2_type),
>  _texture(ir_tg4, gpu_shader5, glsl_type::ivec4_type, 
> glsl_type::isampler2DRect_type, glsl_type::vec2_type),
>  _texture(ir_tg4, gpu_shader5, glsl_type::uvec4_type, 
> glsl_type::usampler2DRect_type, glsl_type::vec2_type),
>
>  _texture(ir_tg4, texture_gather_or_es31, 
> glsl_type::vec4_type, glsl_type::sampler2DArray_type, glsl_type::vec3_type),
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
> b/src/compiler/glsl/glsl_parser_extras.cpp
> index 21ed34d79d0..1def5c7da5c 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -699,20 +699,21 @@ static const _mesa_glsl_extension 
> _mesa_glsl_supported_extensions[] = {
> EXT(OES_texture_cube_map_array),
>

Re: [Mesa-dev] [PATCH 1/3] radeonsi: allow si_cp_dma_clear_buffer to clear GDS from any IB

Hi Christian,

I just pushed the commits.

The best way to reproduce the out-of-memory errors is to run 2 instances of
the test simultaneously:

R600_DEBUG=testgdsmm glxgears &
R600_DEBUG=testgdsmm glxgears &

It takes about 10 seconds to finish and you'll get a lot of errors.

If you run it again, all GDS allocations will fail:

R600_DEBUG=testgdsmm glxgears
amdgpu: Failed to allocate a buffer:
amdgpu:size  : 32768 bytes
amdgpu:alignment : 4 bytes
amdgpu:domains   : 8

Marek

On Wed, Nov 28, 2018 at 2:16 PM Christian König <
ckoenig.leichtzumer...@gmail.com> wrote:

> Are those committed yet? They don't seem to apply cleanly on master.
>
> Christian.
>
> Am 27.11.18 um 02:56 schrieb Marek Olšák:
> > From: Marek Olšák 
> >
> > ---
> >   .../drivers/radeonsi/si_compute_blit.c|  4 +-
> >   src/gallium/drivers/radeonsi/si_cp_dma.c  | 49 ++-
> >   src/gallium/drivers/radeonsi/si_pipe.h|  8 +--
> >   .../drivers/radeonsi/si_test_dma_perf.c   |  3 +-
> >   4 files changed, 33 insertions(+), 31 deletions(-)
> >
> > diff --git a/src/gallium/drivers/radeonsi/si_compute_blit.c
> b/src/gallium/drivers/radeonsi/si_compute_blit.c
> > index 20e4f591fbb..086793637f0 100644
> > --- a/src/gallium/drivers/radeonsi/si_compute_blit.c
> > +++ b/src/gallium/drivers/radeonsi/si_compute_blit.c
> > @@ -212,22 +212,22 @@ void si_clear_buffer(struct si_context *sctx,
> struct pipe_resource *dst,
> >*/
> >   if (clear_value_size > 4 ||
> >   (clear_value_size == 4 &&
> >offset % 4 == 0 &&
> >(size > 32*1024 || sctx->chip_class <= VI))) {
> >   si_compute_do_clear_or_copy(sctx, dst, offset,
> NULL, 0,
> >   aligned_size,
> clear_value,
> >   clear_value_size,
> coher);
> >   } else {
> >   assert(clear_value_size == 4);
> > - si_cp_dma_clear_buffer(sctx, dst, offset,
> > -aligned_size, *clear_value,
> coher,
> > + si_cp_dma_clear_buffer(sctx, sctx->gfx_cs, dst,
> offset,
> > +aligned_size, *clear_value,
> 0, coher,
> >  get_cache_policy(sctx,
> coher, size));
> >   }
> >
> >   offset += aligned_size;
> >   size -= aligned_size;
> >   }
> >
> >   /* Handle non-dword alignment. */
> >   if (size) {
> >   assert(dst);
> > diff --git a/src/gallium/drivers/radeonsi/si_cp_dma.c
> b/src/gallium/drivers/radeonsi/si_cp_dma.c
> > index 839b31b7fdf..33220d9f0fa 100644
> > --- a/src/gallium/drivers/radeonsi/si_cp_dma.c
> > +++ b/src/gallium/drivers/radeonsi/si_cp_dma.c
> > @@ -47,25 +47,24 @@ static inline unsigned cp_dma_max_byte_count(struct
> si_context *sctx)
> >
> >   /* make it aligned for optimal performance */
> >   return max & ~(SI_CPDMA_ALIGNMENT - 1);
> >   }
> >
> >
> >   /* Emit a CP DMA packet to do a copy from one buffer to another, or to
> clear
> >* a buffer. The size must fit in bits [20:0]. If CP_DMA_CLEAR is set,
> src_va is a 32-bit
> >* clear value.
> >*/
> > -static void si_emit_cp_dma(struct si_context *sctx, uint64_t dst_va,
> > -uint64_t src_va, unsigned size, unsigned flags,
> > -enum si_cache_policy cache_policy)
> > +static void si_emit_cp_dma(struct si_context *sctx, struct
> radeon_cmdbuf *cs,
> > +uint64_t dst_va, uint64_t src_va, unsigned size,
> > +unsigned flags, enum si_cache_policy
> cache_policy)
> >   {
> > - struct radeon_cmdbuf *cs = sctx->gfx_cs;
> >   uint32_t header = 0, command = 0;
> >
> >   assert(size <= cp_dma_max_byte_count(sctx));
> >   assert(sctx->chip_class != SI || cache_policy == L2_BYPASS);
> >
> >   if (sctx->chip_class >= GFX9)
> >   command |= S_414_BYTE_COUNT_GFX9(size);
> >   else
> >   command |= S_414_BYTE_COUNT_GFX6(size);
> >
> > @@ -139,21 +138,21 @@ static void si_emit_cp_dma(struct si_context
> *sctx, uint64_t dst_va,
> >   }
> >
> >   void si_cp_dma_wait_for_idle(struct si_context *sctx)
> >   {
> >   /* Issue a dummy DMA that copies zero bytes.
> >*
> >* The DMA engine will see that there's no work to do and skip this
> >* DMA request, however, the CP will see the sync flag and still
> wait
> >* for all DMAs to complete.
> >*/
> > - si_emit_cp_dma(sctx, 0, 0, 0, CP_DMA_SYNC, L2_BYPASS);
> > + si_emit_cp_dma(sctx, sctx->gfx_cs, 0, 0, 0, CP_DMA_SYNC,
> L2_BYPASS);
> >   }
> >
> >   static void si_cp_dma_prepare(struct si_context *sctx, struct
> pipe_resource *dst,
> > struct pipe_resource *src, unsigned
> byte_count,
> >

[Mesa-dev] [PATCH 3/3] mesa: expose AMD_texture_texture4

From: Marek Olšák 

because the closed driver exposes it. Tested by piglit.
---
 docs/relnotes/19.0.0.html|  1 +
 src/compiler/glsl/builtin_functions.cpp  | 10 ++
 src/compiler/glsl/glsl_parser_extras.cpp |  1 +
 src/compiler/glsl/glsl_parser_extras.h   |  2 ++
 src/mesa/main/extensions_table.h |  1 +
 5 files changed, 15 insertions(+)

diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
index d10bd2cf720..bc1776e8f4e 100644
--- a/docs/relnotes/19.0.0.html
+++ b/docs/relnotes/19.0.0.html
@@ -32,20 +32,21 @@ Compatibility contexts may report a lower version depending 
on each driver.
 
 SHA256 checksums
 
 TBD.
 
 
 
 New features
 
 
+GL_AMD_texture_texture4 on all GL 4.0 drivers.
 GL_EXT_shader_implicit_conversions on all drivers (ES extension).
 GL_EXT_texture_compression_bptc on all GL 4.0 drivers (ES extension).
 GL_EXT_texture_compression_rgtc on all GL 3.0 drivers (ES extension).
 GL_EXT_texture_view on drivers supporting texture views (ES extension).
 GL_OES_texture_view on drivers supporting texture views (ES 
extension).
 
 
 Bug fixes
 
 
diff --git a/src/compiler/glsl/builtin_functions.cpp 
b/src/compiler/glsl/builtin_functions.cpp
index 5650365d1d5..62fbe10c623 100644
--- a/src/compiler/glsl/builtin_functions.cpp
+++ b/src/compiler/glsl/builtin_functions.cpp
@@ -411,20 +411,26 @@ texture_query_lod(const _mesa_glsl_parse_state *state)
 static bool
 texture_gather_cube_map_array(const _mesa_glsl_parse_state *state)
 {
return state->is_version(400, 320) ||
   state->ARB_texture_gather_enable ||
   state->ARB_gpu_shader5_enable ||
   state->EXT_texture_cube_map_array_enable ||
   state->OES_texture_cube_map_array_enable;
 }
 
+static bool
+texture_texture4(const _mesa_glsl_parse_state *state)
+{
+   return state->AMD_texture_texture4_enable;
+}
+
 static bool
 texture_gather_or_es31(const _mesa_glsl_parse_state *state)
 {
return state->is_version(400, 310) ||
   state->ARB_texture_gather_enable ||
   state->ARB_gpu_shader5_enable;
 }
 
 /* Only ARB_texture_gather but not GLSL 4.0 or ARB_gpu_shader5.
  * used for relaxation of const offset requirements.
@@ -2895,20 +2901,24 @@ builtin_builder::create_builtins()
 NULL);
 
add_function("shadow2DRectGradARB",
 _texture(ir_txd, shader_texture_lod_and_rect, 
glsl_type::vec4_type,  glsl_type::sampler2DRectShadow_type, 
glsl_type::vec3_type),
 NULL);
 
add_function("shadow2DRectProjGradARB",
 _texture(ir_txd, shader_texture_lod_and_rect, 
glsl_type::vec4_type,  glsl_type::sampler2DRectShadow_type, 
glsl_type::vec4_type, TEX_PROJECT),
 NULL);
 
+   add_function("texture4",
+_texture(ir_tg4, texture_texture4, glsl_type::vec4_type, 
glsl_type::sampler2D_type, glsl_type::vec2_type),
+NULL);
+
add_function("textureGather",
 _texture(ir_tg4, texture_gather_or_es31, glsl_type::vec4_type, 
glsl_type::sampler2D_type, glsl_type::vec2_type),
 _texture(ir_tg4, texture_gather_or_es31, 
glsl_type::ivec4_type, glsl_type::isampler2D_type, glsl_type::vec2_type),
 _texture(ir_tg4, texture_gather_or_es31, 
glsl_type::uvec4_type, glsl_type::usampler2D_type, glsl_type::vec2_type),
 
 _texture(ir_tg4, gpu_shader5, glsl_type::vec4_type, 
glsl_type::sampler2DRect_type, glsl_type::vec2_type),
 _texture(ir_tg4, gpu_shader5, glsl_type::ivec4_type, 
glsl_type::isampler2DRect_type, glsl_type::vec2_type),
 _texture(ir_tg4, gpu_shader5, glsl_type::uvec4_type, 
glsl_type::usampler2DRect_type, glsl_type::vec2_type),
 
 _texture(ir_tg4, texture_gather_or_es31, glsl_type::vec4_type, 
glsl_type::sampler2DArray_type, glsl_type::vec3_type),
diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 21ed34d79d0..1def5c7da5c 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -699,20 +699,21 @@ static const _mesa_glsl_extension 
_mesa_glsl_supported_extensions[] = {
EXT(OES_texture_cube_map_array),
EXT_AEP(OES_texture_storage_multisample_2d_array),
EXT(OES_viewport_array),
 
/* All other extensions go here, sorted alphabetically.
 */
EXT(AMD_conservative_depth),
EXT(AMD_gpu_shader_int64),
EXT(AMD_shader_stencil_export),
EXT(AMD_shader_trinary_minmax),
+   EXT(AMD_texture_texture4),
EXT(AMD_vertex_shader_layer),
EXT(AMD_vertex_shader_viewport_index),
EXT(ANDROID_extension_pack_es31a),
EXT(EXT_blend_func_extended),
EXT(EXT_frag_depth),
EXT(EXT_draw_buffers),
EXT(EXT_clip_cull_distance),
EXT(EXT_geometry_point_size),
EXT_AEP(EXT_geometry_shader),
EXT_AEP(EXT_gpu_shader5),
diff --git a/src/compiler/glsl/glsl_parser_extras.h 
b/src/compiler/glsl/glsl_parser_extras.h
index da8b2fa3ab5..ce18f3f89cd

[Mesa-dev] [PATCH 2/3] mesa: expose EXT_texture_compression_bptc in GLES

From: Marek Olšák 

tested by piglit.
---
 docs/relnotes/19.0.0.html|  1 +
 src/mesa/main/extensions_table.h |  1 +
 src/mesa/main/glformats.c| 13 +++--
 src/mesa/main/texcompress.c  |  9 +
 4 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
index 50cdda6cbc7..d10bd2cf720 100644
--- a/docs/relnotes/19.0.0.html
+++ b/docs/relnotes/19.0.0.html
@@ -33,20 +33,21 @@ Compatibility contexts may report a lower version depending 
on each driver.
 SHA256 checksums
 
 TBD.
 
 
 
 New features
 
 
 GL_EXT_shader_implicit_conversions on all drivers (ES extension).
+GL_EXT_texture_compression_bptc on all GL 4.0 drivers (ES extension).
 GL_EXT_texture_compression_rgtc on all GL 3.0 drivers (ES extension).
 GL_EXT_texture_view on drivers supporting texture views (ES extension).
 GL_OES_texture_view on drivers supporting texture views (ES 
extension).
 
 
 Bug fixes
 
 
 TBD
 
diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
index dd7a4d45079..45ee7675ab2 100644
--- a/src/mesa/main/extensions_table.h
+++ b/src/mesa/main/extensions_table.h
@@ -269,20 +269,21 @@ EXT(EXT_shadow_funcs, ARB_shadow
 EXT(EXT_stencil_two_side, EXT_stencil_two_side 
  , GLL,  x ,  x ,  x , 2001)
 EXT(EXT_stencil_wrap, dummy_true   
  , GLL,  x ,  x ,  x , 2002)
 EXT(EXT_subtexture  , dummy_true   
  , GLL,  x ,  x ,  x , 1995)
 EXT(EXT_tessellation_point_size , ARB_tessellation_shader  
  ,  x ,  x ,  x ,  31, 2013)
 EXT(EXT_tessellation_shader , ARB_tessellation_shader  
  ,  x ,  x ,  x ,  31, 2013)
 EXT(EXT_texture , dummy_true   
  , GLL,  x ,  x ,  x , 1996)
 EXT(EXT_texture3D   , dummy_true   
  , GLL,  x ,  x ,  x , 1996)
 EXT(EXT_texture_array   , EXT_texture_array
  , GLL, GLC,  x ,  x , 2006)
 EXT(EXT_texture_border_clamp, ARB_texture_border_clamp 
  ,  x ,  x ,  x , ES2, 2014)
 EXT(EXT_texture_buffer  , OES_texture_buffer   
  ,  x ,  x ,  x ,  31, 2014)
+EXT(EXT_texture_compression_bptc, ARB_texture_compression_bptc 
  ,  x ,  x ,  x ,  30, 2017)
 EXT(EXT_texture_compression_dxt1, ANGLE_texture_compression_dxt
  , GLL, GLC, ES1, ES2, 2004)
 EXT(EXT_texture_compression_latc, EXT_texture_compression_latc 
  , GLL,  x ,  x ,  x , 2006)
 EXT(EXT_texture_compression_rgtc, ARB_texture_compression_rgtc 
  , GLL, GLC,  x ,  30, 2004)
 EXT(EXT_texture_compression_s3tc, EXT_texture_compression_s3tc 
  , GLL, GLC,  x , ES2, 2000)
 EXT(EXT_texture_cube_map, ARB_texture_cube_map 
  , GLL,  x ,  x ,  x , 2001)
 EXT(EXT_texture_cube_map_array  , OES_texture_cube_map_array   
  ,  x ,  x ,  x ,  31, 2014)
 EXT(EXT_texture_edge_clamp  , dummy_true   
  , GLL,  x ,  x ,  x , 1997)
 EXT(EXT_texture_env_add , dummy_true   
  , GLL,  x ,  x ,  x , 1999)
 EXT(EXT_texture_env_combine , dummy_true   
  , GLL,  x ,  x ,  x , 2000)
 EXT(EXT_texture_env_dot3, EXT_texture_env_dot3 
  , GLL,  x ,  x ,  x , 2000)
diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
index bacaa9519ca..98ae5d93234 100644
--- a/src/mesa/main/glformats.c
+++ b/src/mesa/main/glformats.c
@@ -1389,22 +1389,21 @@ _mesa_is_compressed_format(const struct gl_context 
*ctx, GLenum format)
   return ctx->Extensions.ARB_texture_compression_rgtc;
case MESA_FORMAT_LAYOUT_LATC:
   return ctx->API == API_OPENGL_COMPAT
  && ctx->Extensions.EXT_texture_compression_latc;
case MESA_FORMAT_LAYOUT_ETC1:
   return _mesa_is_gles(ctx)
  && ctx->Extensions.OES_compressed_ETC1_RGB8_texture;
case MESA_FORMAT_LAYOUT_ETC2:
   return _mesa_is_gles3(ctx) || ctx->Extensions.ARB_ES3_compatibility;
case MESA_FORMAT_LAYOUT_BPTC:
-  return _mesa_is_desktop_gl(ctx) &&
- ctx->Extensions.ARB_texture_compression_bptc;
+  return ctx->Extensions.ARB_texture_compression_bptc;
case MESA_FORMAT_LAYOUT_ASTC:
   return ctx->Extensions.KHR_texture_compression_astc_ldr;
default:
   return GL_FALSE;
}
 }
 
 /**
  * Test if the given format represents an sRGB format.
  * \param format the GL format (can be an internal format)
@@ -2834,20 +2833,25 @@ _mesa_es3_error_check_format_and_type(const struct 
gl_context *ctx,
  switch (internalFormat) {
  case GL_RGBA:
  case

[Mesa-dev] [PATCH 1/3] mesa: expose EXT_texture_compression_rgtc on GLES

From: Marek Olšák 

The spec was modified to support GLES. Tested by piglit.
---
 docs/relnotes/19.0.0.html|  1 +
 src/mesa/main/extensions_table.h |  2 +-
 src/mesa/main/glformats.c| 19 +--
 src/mesa/main/texcompress.c  |  9 +
 4 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html
index 920cf803f5d..50cdda6cbc7 100644
--- a/docs/relnotes/19.0.0.html
+++ b/docs/relnotes/19.0.0.html
@@ -33,20 +33,21 @@ Compatibility contexts may report a lower version depending 
on each driver.
 SHA256 checksums
 
 TBD.
 
 
 
 New features
 
 
 GL_EXT_shader_implicit_conversions on all drivers (ES extension).
+GL_EXT_texture_compression_rgtc on all GL 3.0 drivers (ES extension).
 GL_EXT_texture_view on drivers supporting texture views (ES extension).
 GL_OES_texture_view on drivers supporting texture views (ES 
extension).
 
 
 Bug fixes
 
 
 TBD
 
 
diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
index 4f2707b65a4..dd7a4d45079 100644
--- a/src/mesa/main/extensions_table.h
+++ b/src/mesa/main/extensions_table.h
@@ -271,21 +271,21 @@ EXT(EXT_stencil_wrap, dummy_true
 EXT(EXT_subtexture  , dummy_true   
  , GLL,  x ,  x ,  x , 1995)
 EXT(EXT_tessellation_point_size , ARB_tessellation_shader  
  ,  x ,  x ,  x ,  31, 2013)
 EXT(EXT_tessellation_shader , ARB_tessellation_shader  
  ,  x ,  x ,  x ,  31, 2013)
 EXT(EXT_texture , dummy_true   
  , GLL,  x ,  x ,  x , 1996)
 EXT(EXT_texture3D   , dummy_true   
  , GLL,  x ,  x ,  x , 1996)
 EXT(EXT_texture_array   , EXT_texture_array
  , GLL, GLC,  x ,  x , 2006)
 EXT(EXT_texture_border_clamp, ARB_texture_border_clamp 
  ,  x ,  x ,  x , ES2, 2014)
 EXT(EXT_texture_buffer  , OES_texture_buffer   
  ,  x ,  x ,  x ,  31, 2014)
 EXT(EXT_texture_compression_dxt1, ANGLE_texture_compression_dxt
  , GLL, GLC, ES1, ES2, 2004)
 EXT(EXT_texture_compression_latc, EXT_texture_compression_latc 
  , GLL,  x ,  x ,  x , 2006)
-EXT(EXT_texture_compression_rgtc, ARB_texture_compression_rgtc 
  , GLL, GLC,  x ,  x , 2004)
+EXT(EXT_texture_compression_rgtc, ARB_texture_compression_rgtc 
  , GLL, GLC,  x ,  30, 2004)
 EXT(EXT_texture_compression_s3tc, EXT_texture_compression_s3tc 
  , GLL, GLC,  x , ES2, 2000)
 EXT(EXT_texture_cube_map, ARB_texture_cube_map 
  , GLL,  x ,  x ,  x , 2001)
 EXT(EXT_texture_cube_map_array  , OES_texture_cube_map_array   
  ,  x ,  x ,  x ,  31, 2014)
 EXT(EXT_texture_edge_clamp  , dummy_true   
  , GLL,  x ,  x ,  x , 1997)
 EXT(EXT_texture_env_add , dummy_true   
  , GLL,  x ,  x ,  x , 1999)
 EXT(EXT_texture_env_combine , dummy_true   
  , GLL,  x ,  x ,  x , 2000)
 EXT(EXT_texture_env_dot3, EXT_texture_env_dot3 
  , GLL,  x ,  x ,  x , 2000)
 EXT(EXT_texture_filter_anisotropic  , EXT_texture_filter_anisotropic   
  , GLL, GLC, ES1, ES2, 1999)
 EXT(EXT_texture_format_BGRA , dummy_true   
  ,  x ,  x , ES1, ES2, 2005)
 EXT(EXT_texture_integer , EXT_texture_integer  
  , GLL, GLC,  x ,  x , 2006)
diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
index 6cdc7818756..bacaa9519ca 100644
--- a/src/mesa/main/glformats.c
+++ b/src/mesa/main/glformats.c
@@ -1379,22 +1379,21 @@ _mesa_is_compressed_format(const struct gl_context 
*ctx, GLenum format)
  return ctx->Extensions.ANGLE_texture_compression_dxt;
   } else {
  return _mesa_is_desktop_gl(ctx)
 && ctx->Extensions.EXT_texture_sRGB
 && ctx->Extensions.EXT_texture_compression_s3tc;
   }
case MESA_FORMAT_LAYOUT_FXT1:
   return _mesa_is_desktop_gl(ctx)
  && ctx->Extensions.TDFX_texture_compression_FXT1;
case MESA_FORMAT_LAYOUT_RGTC:
-  return _mesa_is_desktop_gl(ctx)
- && ctx->Extensions.ARB_texture_compression_rgtc;
+  return ctx->Extensions.ARB_texture_compression_rgtc;
case MESA_FORMAT_LAYOUT_LATC:
   return ctx->API == API_OPENGL_COMPAT
  && ctx->Extensions.EXT_texture_compression_latc;
case MESA_FORMAT_LAYOUT_ETC1:
   return _mesa_is_gles(ctx)
  && ctx->Extensions.OES_compressed_ETC1_RGB8_texture;
case MESA_FORMAT_LAYOUT_ETC2:
   return _mesa_is_gles3(ctx) || ctx->Extensions.ARB_ES3_compatibility;
case

Re: [Mesa-dev] [PATCH 3/3] radeonsi: add memory management stress tests for GDS

On Wed, Nov 28, 2018 at 4:31 PM Bas Nieuwenhuizen 
wrote:

> Reviewed-by: Bas Nieuwenhuizen 
>

Thanks.

> for the series. Any plans with the newly gotten GDS/OA support?
>

Maybe. Right now we just need to make it stable.

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

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

On 2018-11-28 13:43:29, Eric Anholt wrote:
> Jordan Justen  writes:
> 
> > This adds the "Developer's Certificate of Origin 1.1" from the Linux
> > kernel. It indicates that by using Signed-off-by you are certifying
> > that your patch meets the DCO 1.1 guidelines.
> >
> > It also changes Signed-off-by from being optional to being required.
> >
> > Signed-off-by: Jordan Justen 
> > Cc: Matt Turner 
> 
> What problem for our project is solved by signed-off-by?  Do you think
> that it has actually reduced the incidence of people submitting code
> they don't have permission to submit in the projects where it's been
> used?  I know the behavior in the kernel is that people submit a patch,
> it's missing s-o-b, so it gets bounced, then they maybe add s-o-b or
> just give up.  I don't think anyone stops and says "Wow, that's a good
> question.  Maybe I don't have permission to distribute this after all?"

Is it possible that some of the cases of "just give up" were "Oh I
didn't think about that. I guess I actually don't have permission to
contribute this under that license."?

It seems like contributors have to spend a little brain power once to
read and ponder the ~25 line document with regards to their
contributions, and then just add a -s to their commit command line.

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

Re: [Mesa-dev] Q: to which software renderers should we contribute to help virgl conformance testing

2018-11-28 Thread Cherniak, Bruce

Hello and apologies for such a delayed response — we were quite busy with the
SC’18
supercomputing conference.

On Oct 17, 2018, at 3:37 PM, Roland Scheidegger
mailto:srol...@vmware.com>> wrote:

Am 17.10.18 um 19:15 schrieb Gert Wollny:
Dear all,

we are looking into doing a CI for virglrenderer that also runs a
subset of the GLES dEQP, and in order to be able to run this also in
gitlab.fd.o we were looking into the available gallium software
renderers. Inital tests by just running the dEQP-GLES2 were quite
successful in the sense that the exection time is not too long (a full
run on the GL and GLES host with llvmpipe takes about 10 min [1]).

Now to extend on that work the focus is turning to which software
renderer has the most features, the least failing tests, and is
actively developed.

Simply looking at the commit stats it seems that the developement of
softpipe and llvmpipe is mostly stalled, swr, on the other had has seen
quite some development, but mostly regarding performance, and given the
FAQ [2] the focus is on a very specific application space and not so
much on getting more features in.
I wouldn't quite say llvmpipe is stalled, although it's true that there
weren't all that many changes (in particular as new features are concerned).

You are correct in the assessment that OpenSWR development has been focused
primarily on
performance improvements over additional features at this time. If key
applications within
our "data visualization" focused space require additional features, we will
consider those features
on a case-by-case basis.

When checking for conformance of virglrenderer we need a host driver
that is conformant itself, and we are willing to contribute here, but
it seems to make most sense to focus this work on just one driver. To
make sensible choice there are some open questions:

Are there plans to get swr and/or llvmpipe to support gles 3.1, or
carry any of the drivers even further, maybe GLES 3.2 and desktop 4.x?
At a quick glance for for gles 3.1 llvmpipe would be missing mostly
compute shaders and shader images / ssbo, so definitely some work. GL 4
would add tessellation as well (at least I think these are the big parts
missing).
Unfortunately I don't have time to work on this, but it would be nice to
have indeed. Well volunteers welcome, no special hw nor docs needed :-).
(Although softpipe is easier to work with, but it's just not all that
interesting.)

Indeed, it would be very nice if someone in the community wishes to contribute.
:-)

Is there any specific interest to fix all failures that occur when
running gles dEQP? In this bug report [3] Roland pointed out that
"there is no goal as such to pass dEQP, although patches are welcome",
any opinion for the other drivers? (for swr beyond what is written in
the FAQ).
I think it wouldn't really be all that much work to get dEQP passing -
since llvmpipe is built to honor dx10 rules, which are typically more
strict than GL. But some things specific to GL fail. So IMHO if you want
a non-hw driver to pass dEQP, llvmpipe is probably still your best bet
(but of course, softpipe is generally easier to fix).
Can't really comment on swr there.

The OpenSWR rasterizer core also supports DX10 rasterization rules,
tessellation, and far more
than we have exposed through the OpenSWR gallium driver. However, the source
has become
considerably more difficult to fully understand than llvmpipe.

Roland

As pointed out in the FAQ, swr is very Intel specific, are there plans
not layed out in the FAQ to support other, non-x86 hardware?

The only x86 architecture specific code in OpenSWR should be limited to the
SIMD abstraction wrappers
(https://gitlab.freedesktop.org/mesa/mesa/tree/master/src/gallium/drivers/swr/rasterizer/common/simdlib*)
and a few things in the jitter. Theoretically, non-x86 SIMD abstraction
wrappers could be written, but we do
not have that expertise, nor the resources.

We always appreciate and welcome community contributions, however.

Thank you,
Bruce

many thanks
Gert

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

[Mesa-dev] [Bug 108311] Query buffer object support is broken on r600.

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

Dave Airlie  changed:

   What|Removed |Added

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

--- Comment #4 from Dave Airlie  ---
I pushed my patch for simplicity sakes, I don't think we'd notice the
difference in perf or mem usage.

Thanks for pointing out the problem and the first patch!

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

Re: [Mesa-dev] [PATCH] freedreno: Fix autotools build.

2018-11-28 Thread Rob Clark

On Wed, Nov 28, 2018 at 4:28 PM Vinson Lee  wrote:
>
> Fix build error.
>
>   CXXLDpipe_msm.la
> ../../../../src/gallium/drivers/freedreno/.libs/libfreedreno.a(freedreno_batch.o):
>  In function `batch_init':
> src/gallium/drivers/freedreno/freedreno_batch.c:54: undefined reference to 
> `fd_device_version'
> src/gallium/drivers/freedreno/freedreno_batch.c:59: undefined reference to 
> `fd_submit_new'
> src/gallium/drivers/freedreno/freedreno_batch.c:61: undefined reference to 
> `fd_submit_new_ringbuffer'
> src/gallium/drivers/freedreno/freedreno_batch.c:64: undefined reference to 
> `fd_submit_new_ringbuffer'
> src/gallium/drivers/freedreno/freedreno_batch.c:66: undefined reference to 
> `fd_submit_new_ringbuffer'
> src/gallium/drivers/freedreno/freedreno_batch.c:70: undefined reference to 
> `fd_submit_new_ringbuffer'
>

hmm, I guess my autoconf build did not enable xa or anything non-megadriver?

Thanks for this

Reviewed-by: Rob Clark 

(go ahead and push or ping me if you want me to push)

> Fixes: b4476138d5ad ("freedreno: move drm to common location")
> Fixes: aa0fed10d357 ("freedreno: move ir3 to common location")
> Signed-off-by: Vinson Lee 
> ---
>  src/gallium/targets/pipe-loader/Makefile.am | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/gallium/targets/pipe-loader/Makefile.am 
> b/src/gallium/targets/pipe-loader/Makefile.am
> index fa16e8535ff1..db515e3097b7 100644
> --- a/src/gallium/targets/pipe-loader/Makefile.am
> +++ b/src/gallium/targets/pipe-loader/Makefile.am
> @@ -157,6 +157,8 @@ pipe_msm_la_LIBADD = \
> $(PIPE_LIBS) \
> $(top_builddir)/src/gallium/winsys/freedreno/drm/libfreedrenodrm.la \
> $(top_builddir)/src/gallium/drivers/freedreno/libfreedreno.la \
> +   $(top_builddir)/src/freedreno/libfreedreno_drm.la \
> +   $(top_builddir)/src/freedreno/libfreedreno_ir3.la \
> $(LIBDRM_LIBS) \
> $(FREEDRENO_LIBS)
>
> --
> 2.17.1
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 108572] Could not start gimp (probably due to opencl)

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

Marco  changed:

   What|Removed |Added

  Component|Gallium/StateTracker/Clover |Drivers/Gallium/radeonsi
   Assignee|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop
   |org |.org
 QA Contact|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop
   |org |.org

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

[Mesa-dev] [Bug 108572] Could not start gimp (probably due to opencl)

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

--- Comment #2 from Marco  ---
Created attachment 142654
  --> https://bugs.freedesktop.org/attachment.cgi?id=142654=edit
clinfo output

Thi is the output of clinfo after applying the patch (commenting
si_clear_buffer)

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

[Mesa-dev] [Bug 108572] Could not start gimp (probably due to opencl)

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

--- Comment #1 from Marco  ---
Taking inspiration from:
https://bugs.freedesktop.org/show_bug.cgi?id=108879
I tried to comment the call to si_clear_buffer and magically clinfo start to
work again.

diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
b/src/gallium/drivers/radeonsi/si_pipe.c
index c487ef4..04678fb 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -620,10 +620,10 @@ static struct pipe_context *si_create_context(struct
pipe_screen *screen,

if (sctx->chip_class == CIK) {
/* Clear the NULL constant buffer, because loads should return
zeros. */
-   uint32_t clear_value = 0;
-   si_clear_buffer(sctx, sctx->null_const_buf.buffer, 0,
-   sctx->null_const_buf.buffer->width0,
-   _value, 4, SI_COHERENCY_SHADER);
+   uint32_t clear_value = 0x;
+   //si_clear_buffer(sctx, sctx->null_const_buf.buffer, 0,
+   //  sctx->null_const_buf.buffer->width0,
+   //  _value, 4, SI_COHERENCY_SHADER);
}
return >b;
 fail:


This is my hardware:
00:00.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 16h Processor
Root Complex
00:01.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
Kabini [Radeon HD 8330]
00:01.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Kabini HDMI/DP
Audio
00:02.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 16h Processor
Function 0
00:02.1 PCI bridge: Advanced Micro Devices, Inc. [AMD] Family 16h Processor
Functions 5:1
00:02.3 PCI bridge: Advanced Micro Devices, Inc. [AMD] Family 16h Processor
Functions 5:1
00:02.4 PCI bridge: Advanced Micro Devices, Inc. [AMD] Family 16h Processor
Functions 5:1
00:10.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB XHCI
Controller (rev 01)
00:11.0 SATA controller: Advanced Micro Devices, Inc. [AMD] FCH SATA Controller
[AHCI mode]
00:12.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB OHCI
Controller (rev 39)
00:12.2 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB EHCI
Controller (rev 39)
00:13.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB OHCI
Controller (rev 39)
00:13.2 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB EHCI
Controller (rev 39)
00:14.0 SMBus: Advanced Micro Devices, Inc. [AMD] FCH SMBus Controller (rev 3a)
00:14.2 Audio device: Advanced Micro Devices, Inc. [AMD] FCH Azalia Controller
(rev 02)
00:14.3 ISA bridge: Advanced Micro Devices, Inc. [AMD] FCH LPC Bridge (rev 11)
00:18.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 16h Processor
Function 0
00:18.1 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 16h Processor
Function 1
00:18.2 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 16h Processor
Function 2
00:18.3 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 16h Processor
Function 3
00:18.4 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 16h Processor
Function 4
00:18.5 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 16h Processor
Function 5
01:00.0 Display controller: Advanced Micro Devices, Inc. [AMD/ATI] Sun PRO
[Radeon HD 8570A/8570M]
02:00.0 Network controller: Qualcomm Atheros QCA9565 / AR9565 Wireless Network
Adapter (rev 01)
03:00.0 Ethernet controller: Qualcomm Atheros QCA8172 Fast Ethernet (rev 10)

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

Re: [Mesa-dev] [PATCH B 14/14] FIXUP: Fix NIR producers and consumers to use unsized conversions

2018-11-28 Thread Eric Anholt

Jason Ekstrand  writes:

> ---
>  src/amd/common/ac_nir_to_llvm.c   | 12 
>  src/compiler/glsl/glsl_to_nir.cpp |  2 +-
>  src/compiler/nir/nir_builder.h| 12 
>  src/compiler/spirv/vtn_glsl450.c  |  4 ++--
>  .../drivers/freedreno/ir3/ir3_compiler_nir.c  | 11 +++
>  src/gallium/drivers/vc4/vc4_program.c |  8 
>  src/intel/compiler/brw_fs_nir.cpp | 19 ---
>  src/intel/compiler/brw_vec4_nir.cpp   |  9 +
>  src/mesa/program/prog_to_nir.c|  4 ++--
>  9 files changed, 53 insertions(+), 28 deletions(-)

Don't forget src/broadcom/compiler :)

B looks pretty reasonable.  I've tried to muster the concentration to
review this series a few times, and between the python and algebraic
changes my eyes keep glazing over.  Acked-in-principle, I guess?


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

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

2018-11-28 Thread Dylan Baker

Quoting Jason Ekstrand (2018-11-28 11:30:32)
> Yes, but the point is that we (the reviewers) know that we're conflicting. 
> That's very different from what I could easily see happening *a lot* were ML
> reviewer A is perfectly happy with some bit of code but MR reviewer B asks for
> it to be completely reworked.  In v2 of the series, the submitter reworks it
> but now reviewer A is unhappy.  "Why did you change it?" he says, "It was just
> fine before!".  "Reviewer B requested the rework," says the submitter.  "When
> did he say that?  I didn't see that comment." says B.  "On the GitLab MR," 
> says
> the submitter.  "Well, I don't read MRs; this kind of feedback should happen 
> on
> the list where we can all read it," says A.
> 
> If you can't immagine that exchange happening, then you haven't been on this
> list long enough. :-)  (Says a guy who's been on the list for about half as
> long as Jordan.)
> 
> We have enough stubborn people on the list that MRs are going to constantly 
> get
> pulled back to the list just because someone doesn't want to use the web
> interface.  That's really mean to submitters who actually want to use the MR
> process and is strictly worse than what we have today.  If we're going to
> actually try out MRs, we need those people trying it too at least from the
> reviewer side.

This is exactly my concern, and the reason I think we need to be "all in" on one
or the other. I have a preferences for MRs, but I'm fine with continuing to use
the mailing list. This is (IMHO) a case where both is strictly worse than
either choice.

Dylan


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

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

2018-11-28 Thread Dylan Baker

Quoting Jordan Justen (2018-11-28 10:21:13)
> On 2018-11-28 09:22:35, Dylan Baker wrote:
> > Quoting Matt Turner (2018-11-27 19:20:09)
> > > On Tue, Nov 27, 2018 at 5:13 PM Jordan Justen  
> > > wrote:
> > > >
> > > > This documents a mechanism for using GitLab Merge Requests as an
> > > > optional, secondary way to get code reviews for patchsets.
> > > >
> > > > We still require all patches to be emailed.
> > > >
> > > > Aside from the potential usage for code review comments, it might also
> > > > help reviewers to find unmerged patchsets which need review. (Assuming
> > > > it doesn't fall into the same fate of patchwork with hundreds of open
> > > > MRs.)
> > > >
> > > > Signed-off-by: Jordan Justen 
> > > > Cc: Jason Ekstrand 
> > > > ---
> > > >  docs/submittingpatches.html | 25 +
> > > >  1 file changed, 25 insertions(+)
> > > >
> > > > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
> > > > index 5d8ba443191..852f28c198a 100644
> > > > --- a/docs/submittingpatches.html
> > > > +++ b/docs/submittingpatches.html
> > > > @@ -24,6 +24,7 @@
> > > >  Testing Patches
> > > >  Mailing Patches
> > > >  Reviewing Patches
> > > > +GitLab Merge Requests
> > > >  Nominating a commit for a stable branch
> > > >  Criteria for accepting patches to the stable 
> > > > branch
> > > >  Sending backports for the stable branch
> > > > @@ -282,6 +283,30 @@ which tells the patch author that the patch can be 
> > > > committed, as long
> > > >  as the issues are resolved first.
> > > >  
> > > >
> > > > +GitLab Merge Requests
> > > > +
> > > > +
> > > > +  https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge
> > > > +  Requests (MR) can be used as an optional, secondary method of
> > > > +  obtaining code review for patches.
> > > > +
> > > > +
> > > > +
> > > > +  All patches should be submitted using email as well
> > > > +  Consider including a link to the MR in your email based
> > > > +cover-letter
> > > > +  Address code review from both email and the MR
> > > 
> > > Discussion point: I think attempting to have simultaneous review in
> > > two places is a recipe for wasted time. Strawman: maybe we can only
> > > email the cover letter to the mailing list and include in it a link to
> > > the MR?
> > 
> > I think it's a *really* bad idea to allow both. Some people will immediately
> > begin using MR/PR's only and never read the list, others will never check
> > MR/PRs and only use the list. It'll leave the rest of us forced to use both.
> 
> Requiring emails seems to avoid these issues for now. We'd just be
> trying out using merge requests, but if you want to see all patches,
> use email.
> 
> > We should either go all in and archive the mailing list and use only
> > gitlab, or not use PR/MR's IMHO.
> 
> If that is the only choice, then I choose that we not use MR. Or, wait
> until everyone is forced to try MR via other freedesktop projects. :)
> 
> One motivation I had for this patch was to see if MR could be a
> 'patchwork with labels'. Although, unlike patchwork, the contributor
> would have to take an extra step to open the MR and apply the label.
> One positive vs patchwork would be having the same login system as
> Mesa's repo.
> 
> -Jordan

This really feels like we're asking people to do more work. They now need to
send patches to the list, open an MR, add lables, then take into account review
feedback from both sources (and deal with conflicting review, which resolves
itself naturally with only *one* place to do review), send out a v2 and force
push so gitlab notices. Then use git to push, and close their MR.

I know I sound really critical here and I'm sorry if I'm coming off gruff.
Patchwork has really burned me on "optional" features like this. I tried to use
patchwork, but just ended up spending a ton of time doing janitorial work, which
very few people really seemed interested in. I'm afraid this is going to be a
community split with neither option being definitively "better" than the other.
Some members of the community really like email, plain text, and list based
review; others hate it, and really like the web driven approach; but those are
just "likes" there isn't something we can point at and say "yeah, X is annoying,
but Y makes it worth while".

I'm being honest, I don't like mailing lists as much as I like gitlab, but this
hybrid approach feels like it's just doubling the amount of work I need to do,
and under this proposal I don't plan to use gitlab, I'm just going to keep
sending regular series the mailinglists. Again, if the community is interested
in trying an MR workflow I think piglit is a great place to try that out.

Dylan


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

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

2018-11-28 Thread Eric Anholt

Jordan Justen  writes:

> This adds the "Developer's Certificate of Origin 1.1" from the Linux
> kernel. It indicates that by using Signed-off-by you are certifying
> that your patch meets the DCO 1.1 guidelines.
>
> It also changes Signed-off-by from being optional to being required.
>
> Signed-off-by: Jordan Justen 
> Cc: Matt Turner 

What problem for our project is solved by signed-off-by?  Do you think
that it has actually reduced the incidence of people submitting code
they don't have permission to submit in the projects where it's been
used?  I know the behavior in the kernel is that people submit a patch,
it's missing s-o-b, so it gets bounced, then they maybe add s-o-b or
just give up.  I don't think anyone stops and says "Wow, that's a good
question.  Maybe I don't have permission to distribute this after all?"

/me sees s-o-b as basically just a linux kernel hazing ritual

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

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

2018-11-28 Thread Eric Anholt

Jordan Justen  writes:

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

Like others, I disagree and think this will lead to confusion.  Let
people send to one or the other.

> +  Consider including a link to the MR in your email based
> +cover-letter
> +  Address code review from both email and the MR
> +  Add appropriate labels to your MR. For example:
> +
> +  Mesa changes affect all drivers: mesa
> +  Hardware vendor specific code: amd, intel, nvidia, etc
> +  Driver specific code: anvil, freedreno, i965, iris, radeonsi, 
> radv, vc4, etc
> +  Other tag examples: gallium, util
> +
> +  Never use the merge button on the GitLab page to push patches

Why "never use the merge button"?  We've been using rebase+merge in
xserver and it's *awesome* and has greatly increased the review rate.

> +  Add Reviewed-by tags to your commits and push using git
> +  Close your MR when your patches get pushed!
> +
>  
>  Nominating a commit for a stable branch

Overall:

I can't wait to have a full MR process.  What if we just *never* merged
code that broke the build or introduced warnings, because the MR process
ensured it?  How much time would we all save?  How much less training
would we need to do on new contributors?


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

Re: [Mesa-dev] [PATCH 3/3] radeonsi: add memory management stress tests for GDS

2018-11-28 Thread Bas Nieuwenhuizen

Reviewed-by: Bas Nieuwenhuizen 

for the series. Any plans with the newly gotten GDS/OA support?
On Tue, Nov 27, 2018 at 2:57 AM Marek Olšák  wrote:
>
> From: Marek Olšák 
>
> ---
>  src/gallium/drivers/radeonsi/si_pipe.c | 46 ++
>  src/gallium/drivers/radeonsi/si_pipe.h |  2 ++
>  2 files changed, 48 insertions(+)
>
> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c 
> b/src/gallium/drivers/radeonsi/si_pipe.c
> index 9080de1ceca..503d8331906 100644
> --- a/src/gallium/drivers/radeonsi/si_pipe.c
> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
> @@ -96,20 +96,22 @@ static const struct debug_named_value debug_options[] = {
> { "nodccmsaa", DBG(NO_DCC_MSAA), "Disable DCC for MSAA" },
> { "nofmask", DBG(NO_FMASK), "Disable MSAA compression" },
>
> /* Tests: */
> { "testdma", DBG(TEST_DMA), "Invoke SDMA tests and exit." },
> { "testvmfaultcp", DBG(TEST_VMFAULT_CP), "Invoke a CP VM fault test 
> and exit." },
> { "testvmfaultsdma", DBG(TEST_VMFAULT_SDMA), "Invoke a SDMA VM fault 
> test and exit." },
> { "testvmfaultshader", DBG(TEST_VMFAULT_SHADER), "Invoke a shader VM 
> fault test and exit." },
> { "testdmaperf", DBG(TEST_DMA_PERF), "Test DMA performance" },
> { "testgds", DBG(TEST_GDS), "Test GDS." },
> +   { "testgdsmm", DBG(TEST_GDS_MM), "Test GDS memory management." },
> +   { "testgdsoamm", DBG(TEST_GDS_OA_MM), "Test GDS OA memory 
> management." },
>
> DEBUG_NAMED_VALUE_END /* must be last */
>  };
>
>  static void si_init_compiler(struct si_screen *sscreen,
>  struct ac_llvm_compiler *compiler)
>  {
> /* Only create the less-optimizing version of the compiler on APUs
>  * predating Ryzen (Raven). */
> bool create_low_opt_compiler = !sscreen->info.has_dedicated_vram &&
> @@ -781,20 +783,55 @@ static void si_test_vmfault(struct si_screen *sscreen)
> ctx->flush(ctx, NULL, 0);
> puts("VM fault test: SDMA - done.");
> }
> if (sscreen->debug_flags & DBG(TEST_VMFAULT_SHADER)) {
> util_test_constant_buffer(ctx, buf);
> puts("VM fault test: Shader - done.");
> }
> exit(0);
>  }
>
> +static void si_test_gds_memory_management(struct si_context *sctx,
> + unsigned alloc_size, unsigned 
> alignment,
> + enum radeon_bo_domain domain)
> +{
> +   struct radeon_winsys *ws = sctx->ws;
> +   struct radeon_cmdbuf *cs[8];
> +   struct pb_buffer *gds_bo[ARRAY_SIZE(cs)];
> +
> +   for (unsigned i = 0; i < ARRAY_SIZE(cs); i++) {
> +   cs[i] = ws->cs_create(sctx->ctx, RING_COMPUTE,
> + NULL, NULL, false);
> +   gds_bo[i] = ws->buffer_create(ws, alloc_size, alignment, 
> domain, 0);
> +   assert(gds_bo[i]);
> +   }
> +
> +   for (unsigned iterations = 0; iterations < 2; iterations++) {
> +   for (unsigned i = 0; i < ARRAY_SIZE(cs); i++) {
> +   /* This clears GDS with CP DMA.
> +*
> +* We don't care if GDS is present. Just add some 
> packet
> +* to make the GPU busy for a moment.
> +*/
> +   si_cp_dma_clear_buffer(sctx, cs[i], NULL, 0, 
> alloc_size, 0,
> +  SI_CPDMA_SKIP_BO_LIST_UPDATE |
> +  SI_CPDMA_SKIP_CHECK_CS_SPACE |
> +  SI_CPDMA_SKIP_GFX_SYNC, 0, 0);
> +
> +   ws->cs_add_buffer(cs[i], gds_bo[i], domain,
> + RADEON_USAGE_READWRITE, 0);
> +   ws->cs_flush(cs[i], PIPE_FLUSH_ASYNC, NULL);
> +   }
> +   }
> +   exit(0);
> +}
> +
>  static void si_disk_cache_create(struct si_screen *sscreen)
>  {
> /* Don't use the cache if shader dumping is enabled. */
> if (sscreen->debug_flags & DBG_ALL_SHADERS)
> return;
>
> struct mesa_sha1 ctx;
> unsigned char sha1[20];
> char cache_id[20 * 2 + 1];
>
> @@ -1135,12 +1172,21 @@ struct pipe_screen *radeonsi_screen_create(struct 
> radeon_winsys *ws,
> }
>
> if (sscreen->debug_flags & (DBG(TEST_VMFAULT_CP) |
>   DBG(TEST_VMFAULT_SDMA) |
>   DBG(TEST_VMFAULT_SHADER)))
> si_test_vmfault(sscreen);
>
> if (sscreen->debug_flags & DBG(TEST_GDS))
> si_test_gds((struct si_context*)sscreen->aux_context);
>
> +   if (sscreen->debug_flags & DBG(TEST_GDS_MM)) {
> +   si_test_gds_memory_management((struct 
> si_context*)sscreen->aux_context,
> + 32 *

[Mesa-dev] [PATCH] freedreno: Fix autotools build.

2018-11-28 Thread Vinson Lee

Fix build error.

  CXXLDpipe_msm.la
../../../../src/gallium/drivers/freedreno/.libs/libfreedreno.a(freedreno_batch.o):
 In function `batch_init':
src/gallium/drivers/freedreno/freedreno_batch.c:54: undefined reference to 
`fd_device_version'
src/gallium/drivers/freedreno/freedreno_batch.c:59: undefined reference to 
`fd_submit_new'
src/gallium/drivers/freedreno/freedreno_batch.c:61: undefined reference to 
`fd_submit_new_ringbuffer'
src/gallium/drivers/freedreno/freedreno_batch.c:64: undefined reference to 
`fd_submit_new_ringbuffer'
src/gallium/drivers/freedreno/freedreno_batch.c:66: undefined reference to 
`fd_submit_new_ringbuffer'
src/gallium/drivers/freedreno/freedreno_batch.c:70: undefined reference to 
`fd_submit_new_ringbuffer'

Fixes: b4476138d5ad ("freedreno: move drm to common location")
Fixes: aa0fed10d357 ("freedreno: move ir3 to common location")
Signed-off-by: Vinson Lee 
---
 src/gallium/targets/pipe-loader/Makefile.am | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/gallium/targets/pipe-loader/Makefile.am 
b/src/gallium/targets/pipe-loader/Makefile.am
index fa16e8535ff1..db515e3097b7 100644
--- a/src/gallium/targets/pipe-loader/Makefile.am
+++ b/src/gallium/targets/pipe-loader/Makefile.am
@@ -157,6 +157,8 @@ pipe_msm_la_LIBADD = \
$(PIPE_LIBS) \
$(top_builddir)/src/gallium/winsys/freedreno/drm/libfreedrenodrm.la \
$(top_builddir)/src/gallium/drivers/freedreno/libfreedreno.la \
+   $(top_builddir)/src/freedreno/libfreedreno_drm.la \
+   $(top_builddir)/src/freedreno/libfreedreno_ir3.la \
$(LIBDRM_LIBS) \
$(FREEDRENO_LIBS)
 
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH 4/7] winsys/amdgpu: clean up code around BO VM alignment

2018-11-28 Thread Bas Nieuwenhuizen

On Mon, Nov 26, 2018 at 8:54 PM Marek Olšák  wrote:
>
> On Mon, Nov 26, 2018 at 8:46 AM Bas Nieuwenhuizen  wrote:
>>
>> patches 4-7 are
>>
>> Reviewed-by: Bas Nieuwenhuizen 
>>
>> though I agree with Christian that it would be nice to get a case
>> where 7 improves things before we submit it.
>>
>> For patches 1-3 I need to dive into the slab allocator before I have
>> enough knowledge to review.
>
>
> It's actually pretty simple. Each slab buffer contains suballocations of the 
> same size. E.g. one 2M slab can only have 16 128K suballocations. If you want 
> a 64K buffer, you have to allocate a separate 2M slab with 32 64K 
> suballocations. If you need to allocate just 512B, you need another 2MB slab. 
> This wastes memory in 2 ways:
> 1) a 129K buffer will have to be allocated as a 256K buffer, because the 
> allocation size has to be a power of two, wasting space
> 2) there will be too many 2MB slabs, one slab for each size (from 2^9 to 
> 2^18), each having some unused space, wasting space
> 3) having too many 2K and smaller allocations actually decreases memory 
> usage, because the minimum kernel allocation size is 4K
>
> Problem 2 is mitigated by layering slab allocators on top of each other. The 
> top slab allocator can only allocate sizes 2^17 to 2^18, so you only need two 
> 2M slabs to cover all your needs. The middle allocator uses the top allocator 
> and allocates slabs of size 2^17 and uses it to allocate 2^13 to 2^16 sizes. 
> The bottom allocator allocates slabs of size 2^13 and uses it allocate 2^9 to 
> 2^12 sizes.

Sure, just needed to familiarize myself with the code since I don't
want to review just the idea.

1-3 are

Reviewed-by: Bas Nieuwenhuizen 
>
> Marek
>
>
>> On Sat, Nov 24, 2018 at 12:41 AM Marek Olšák  wrote:
>> >
>> > From: Marek Olšák 
>> >
>> > ---
>> >  src/gallium/winsys/amdgpu/drm/amdgpu_bo.c | 9 +++--
>> >  1 file changed, 7 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c 
>> > b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
>> > index a9271c33ee9..36e2c4ec0dc 100644
>> > --- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
>> > +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
>> > @@ -449,24 +449,29 @@ static struct amdgpu_winsys_bo 
>> > *amdgpu_create_bo(struct amdgpu_winsys *ws,
>> > 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);
>> >fprintf(stderr, "amdgpu:alignment : %u bytes\n", alignment);
>> >fprintf(stderr, "amdgpu:domains   : %u\n", initial_domain);
>> >goto error_bo_alloc;
>> > }
>> >
>> > va_gap_size = ws->check_vm ? MAX2(4 * alignment, 64 * 1024) : 0;
>> > +
>> > +   unsigned vm_alignment = alignment;
>> > +
>> > +   /* Increase the VM alignment for faster address translation. */
>> > if (size > ws->info.pte_fragment_size)
>> > -  alignment = MAX2(alignment, ws->info.pte_fragment_size);
>> > +  vm_alignment = MAX2(vm_alignment, ws->info.pte_fragment_size);
>> > +
>> > r = amdgpu_va_range_alloc(ws->dev, amdgpu_gpu_va_range_general,
>> > - size + va_gap_size, alignment, 0, , 
>> > _handle,
>> > + size + va_gap_size, vm_alignment, 0, , 
>> > _handle,
>> >   (flags & RADEON_FLAG_32BIT ? 
>> > AMDGPU_VA_RANGE_32_BIT : 0) |
>> >  AMDGPU_VA_RANGE_HIGH);
>> > if (r)
>> >goto error_va_alloc;
>> >
>> > unsigned vm_flags = AMDGPU_VM_PAGE_READABLE |
>> > AMDGPU_VM_PAGE_EXECUTABLE;
>> >
>> > if (!(flags & RADEON_FLAG_READ_ONLY))
>> > vm_flags |= AMDGPU_VM_PAGE_WRITEABLE;
>> > --
>> > 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 mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] radeonsi: allow si_cp_dma_clear_buffer to clear GDS from any IB

No. I can push them after they are reviewed or acked.

Marek

On Wed, Nov 28, 2018 at 2:16 PM Christian König <
ckoenig.leichtzumer...@gmail.com> wrote:

> Are those committed yet? They don't seem to apply cleanly on master.
>
> Christian.
>
> Am 27.11.18 um 02:56 schrieb Marek Olšák:
> > From: Marek Olšák 
> >
> > ---
> >   .../drivers/radeonsi/si_compute_blit.c|  4 +-
> >   src/gallium/drivers/radeonsi/si_cp_dma.c  | 49 ++-
> >   src/gallium/drivers/radeonsi/si_pipe.h|  8 +--
> >   .../drivers/radeonsi/si_test_dma_perf.c   |  3 +-
> >   4 files changed, 33 insertions(+), 31 deletions(-)
> >
> > diff --git a/src/gallium/drivers/radeonsi/si_compute_blit.c
> b/src/gallium/drivers/radeonsi/si_compute_blit.c
> > index 20e4f591fbb..086793637f0 100644
> > --- a/src/gallium/drivers/radeonsi/si_compute_blit.c
> > +++ b/src/gallium/drivers/radeonsi/si_compute_blit.c
> > @@ -212,22 +212,22 @@ void si_clear_buffer(struct si_context *sctx,
> struct pipe_resource *dst,
> >*/
> >   if (clear_value_size > 4 ||
> >   (clear_value_size == 4 &&
> >offset % 4 == 0 &&
> >(size > 32*1024 || sctx->chip_class <= VI))) {
> >   si_compute_do_clear_or_copy(sctx, dst, offset,
> NULL, 0,
> >   aligned_size,
> clear_value,
> >   clear_value_size,
> coher);
> >   } else {
> >   assert(clear_value_size == 4);
> > - si_cp_dma_clear_buffer(sctx, dst, offset,
> > -aligned_size, *clear_value,
> coher,
> > + si_cp_dma_clear_buffer(sctx, sctx->gfx_cs, dst,
> offset,
> > +aligned_size, *clear_value,
> 0, coher,
> >  get_cache_policy(sctx,
> coher, size));
> >   }
> >
> >   offset += aligned_size;
> >   size -= aligned_size;
> >   }
> >
> >   /* Handle non-dword alignment. */
> >   if (size) {
> >   assert(dst);
> > diff --git a/src/gallium/drivers/radeonsi/si_cp_dma.c
> b/src/gallium/drivers/radeonsi/si_cp_dma.c
> > index 839b31b7fdf..33220d9f0fa 100644
> > --- a/src/gallium/drivers/radeonsi/si_cp_dma.c
> > +++ b/src/gallium/drivers/radeonsi/si_cp_dma.c
> > @@ -47,25 +47,24 @@ static inline unsigned cp_dma_max_byte_count(struct
> si_context *sctx)
> >
> >   /* make it aligned for optimal performance */
> >   return max & ~(SI_CPDMA_ALIGNMENT - 1);
> >   }
> >
> >
> >   /* Emit a CP DMA packet to do a copy from one buffer to another, or to
> clear
> >* a buffer. The size must fit in bits [20:0]. If CP_DMA_CLEAR is set,
> src_va is a 32-bit
> >* clear value.
> >*/
> > -static void si_emit_cp_dma(struct si_context *sctx, uint64_t dst_va,
> > -uint64_t src_va, unsigned size, unsigned flags,
> > -enum si_cache_policy cache_policy)
> > +static void si_emit_cp_dma(struct si_context *sctx, struct
> radeon_cmdbuf *cs,
> > +uint64_t dst_va, uint64_t src_va, unsigned size,
> > +unsigned flags, enum si_cache_policy
> cache_policy)
> >   {
> > - struct radeon_cmdbuf *cs = sctx->gfx_cs;
> >   uint32_t header = 0, command = 0;
> >
> >   assert(size <= cp_dma_max_byte_count(sctx));
> >   assert(sctx->chip_class != SI || cache_policy == L2_BYPASS);
> >
> >   if (sctx->chip_class >= GFX9)
> >   command |= S_414_BYTE_COUNT_GFX9(size);
> >   else
> >   command |= S_414_BYTE_COUNT_GFX6(size);
> >
> > @@ -139,21 +138,21 @@ static void si_emit_cp_dma(struct si_context
> *sctx, uint64_t dst_va,
> >   }
> >
> >   void si_cp_dma_wait_for_idle(struct si_context *sctx)
> >   {
> >   /* Issue a dummy DMA that copies zero bytes.
> >*
> >* The DMA engine will see that there's no work to do and skip this
> >* DMA request, however, the CP will see the sync flag and still
> wait
> >* for all DMAs to complete.
> >*/
> > - si_emit_cp_dma(sctx, 0, 0, 0, CP_DMA_SYNC, L2_BYPASS);
> > + si_emit_cp_dma(sctx, sctx->gfx_cs, 0, 0, 0, CP_DMA_SYNC,
> L2_BYPASS);
> >   }
> >
> >   static void si_cp_dma_prepare(struct si_context *sctx, struct
> pipe_resource *dst,
> > struct pipe_resource *src, unsigned
> byte_count,
> > uint64_t remaining_size, unsigned user_flags,
> > enum si_coherency coher, bool *is_first,
> > unsigned *packet_flags)
> >   {
> >   /* Fast exit for a CPDMA prefetch. */
> >   if ((user_flags & SI_CPDMA_SKIP_ALL) == SI_CPDMA_SKIP_ALL) {
> > @@ -200,51 +199,53 @@ static void si_cp_dma_prepare(struct si_context
> *sctx, struct pipe_resource *dst
> >

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

2018-11-28 Thread Rob Clark

On Wed, Nov 28, 2018 at 2:16 PM Jordan Justen  wrote:
>
> On 2018-11-28 10:14:49, Jason Ekstrand wrote:
> > On Wed, Nov 28, 2018 at 11:35 AM Jordan Justen 
> > wrote:
> > > On 2018-11-28 06:59:42, Eric Engestrom wrote:
> > > > On Tuesday, 2018-11-27 19:45:50 -0800, Jordan Justen wrote:
> > > > > On 2018-11-27 19:20:09, Matt Turner wrote:
> > > > > >
> > > > > > Discussion point: I think attempting to have simultaneous review in
> > > > > > two places is a recipe for wasted time.
> > > > >
> > > > > That's possible. It also happens on email sometimes. But, I want to
> > > > > say that maybe the usual problem is too little code review, and not
> > > > > too much? :)
> > > >
> > > > I think duplicating the review possibilities might very well lead to
> > > > less review as people might (unconsciously) think "it has been/will be
> > > > reviewed on the other one".
> > >
> > > Maybe they were looking for an excuse to not do code review? :)
> > >
> > > I agree with your point to some extent, but I think it's better to try
> > > to work out a transition. To not jump immediately to forcing people to
> > > go to the web page.
> > >
> > > >
> > > > > > Strawman: maybe we can only email the cover letter to the mailing
> > > > > > list and include in it a link to the MR?
> > > > >
> > > > > I was hoping to make a smaller step and see what happens. Maybe this
> > > > > will give people the chance to try out MR based reviews, but not take
> > > > > away email based reviews just yet.
> > > > >
> > > > > I don't think we should move away from email based reviews until we
> > > > > are sure MR's actually work better. (I'm far from convinced on this
> > > > > point. :)
> > > >
> > > > I think one way to solve this is to allow both, but only one at a time.
> > > > Devs can choose to send their patches/series as either an MR _or_ as
> > > > emails, but not both. One can still send a cover-letter style email to
> > > > the ML to present the MR with a link to it.
> > >
> > > I would prefer to keep emails as required for now. Let people
> > > optionally try it out for some time.
> > >
> > > Perhaps as a next step we can let the poster consider using a MR and
> > > only a cover letter. (Assuming the MR method proves useful. If it
> > > doesn't we should revert back to email only.)
> > >
> >
> > I agree with Eric on this one.  If a single submission has both types of
> > review then you will effectively end up with two different groups of
> > reviewers providing potentially conflicting feedback without seeing each
> > other's feedback and the submitter has to mediate.
>
> They always have to. Case in point, this patch. I want to keep all
> patches on the list but let people optionally post an MR so people can
> try it out as a first step. Dylan says it has to be all or nothing.
> You and Eric say the submitter should be forced to choose one or the
> other. (I'm not sure where to go from here. :)

I kinda liked the idea of send-cover-letter-to-list-with-link-to-MR
approach, just to avoid encouraging review happening in two places..
but that is just my $0.02

If there is not consensus, perhaps just picking one option for a
time-limited trial period (one or two months) and then revisit
afterwards would be a way to move forward?

BR,
-R


>
> I think this is a worst case scenario situation. Normally people
> comment about separate parts of code, and not often in conflicting
> ways.
>
> If a major conflict comes up, then the submitter could ask the MR
> based reviewer to add their point on the email list discussion. (Since
> we'd be keeping email as the primary.)
>
> > It's better just to
> > have any given series have a single point for review.  Yes, this means that
> > everyone will be forced to start doing GitHub review as soon as someone
> > does an MR that's relevant to them.  I don't see a good way around that
> > which doesn't make a mess.
> >
> > Certain projects could, I suppose, have a requirement that all significant
> > work on that project be done on the list or on GitLab.  However, people who
> > feel like custodians of Mesa as a whole will have to do both as long as
> > both are supported.
>
> I guess by 'projects', you mean sub-projects within Mesa? I'm not
> against Eric's idea of allowing the submitter to choose email list vs
> MR, but I do think we should hold off and consider that in 3~6 months.
>
> > > > Other things that need to be present in this 'GitLab MR' document are:
> > > >
> > > > - Review process: when you change something in your MR, force-push the
> > > >   new version, don't add fixup commits. The history of the MR at any one
> > > >   time must be "clean" in the sense that it would be acceptable to push
> > > >   it as is.
> > >
> > > We don't document this for email based reviews, but people seem to
> > > figure it out. :)
> > >
> > > Is this more of a concern because they might not know that pushing the
> > > same branch updates the MR?
> > >
> >
> > I think it's worth explicitly documenting.  Many

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

2018-11-28 Thread Jason Ekstrand

On Wed, Nov 28, 2018 at 1:16 PM Jordan Justen 
wrote:

> On 2018-11-28 10:14:49, Jason Ekstrand wrote:
> > On Wed, Nov 28, 2018 at 11:35 AM Jordan Justen <
> jordan.l.jus...@intel.com>
> > wrote:
> > > On 2018-11-28 06:59:42, Eric Engestrom wrote:
> > > > On Tuesday, 2018-11-27 19:45:50 -0800, Jordan Justen wrote:
> > > > > On 2018-11-27 19:20:09, Matt Turner wrote:
> > > > > > Strawman: maybe we can only email the cover letter to the mailing
> > > > > > list and include in it a link to the MR?
> > > > >
> > > > > I was hoping to make a smaller step and see what happens. Maybe
> this
> > > > > will give people the chance to try out MR based reviews, but not
> take
> > > > > away email based reviews just yet.
> > > > >
> > > > > I don't think we should move away from email based reviews until we
> > > > > are sure MR's actually work better. (I'm far from convinced on this
> > > > > point. :)
> > > >
> > > > I think one way to solve this is to allow both, but only one at a
> time.
> > > > Devs can choose to send their patches/series as either an MR _or_ as
> > > > emails, but not both. One can still send a cover-letter style email
> to
> > > > the ML to present the MR with a link to it.
> > >
> > > I would prefer to keep emails as required for now. Let people
> > > optionally try it out for some time.
> > >
> > > Perhaps as a next step we can let the poster consider using a MR and
> > > only a cover letter. (Assuming the MR method proves useful. If it
> > > doesn't we should revert back to email only.)
> > >
> >
> > I agree with Eric on this one.  If a single submission has both types of
> > review then you will effectively end up with two different groups of
> > reviewers providing potentially conflicting feedback without seeing each
> > other's feedback and the submitter has to mediate.
>
> They always have to. Case in point, this patch. I want to keep all
> patches on the list but let people optionally post an MR so people can
> try it out as a first step. Dylan says it has to be all or nothing.
> You and Eric say the submitter should be forced to choose one or the
> other. (I'm not sure where to go from here. :)
>

Yes, but the point is that we (the reviewers) know that we're conflicting.
That's very different from what I could easily see happening *a lot* were
ML reviewer A is perfectly happy with some bit of code but MR reviewer B
asks for it to be completely reworked.  In v2 of the series, the submitter
reworks it but now reviewer A is unhappy.  "Why did you change it?" he
says, "It was just fine before!".  "Reviewer B requested the rework," says
the submitter.  "When did he say that?  I didn't see that comment." says
B.  "On the GitLab MR," says the submitter.  "Well, I don't read MRs; this
kind of feedback should happen on the list where we can all read it," says
A.

If you can't immagine that exchange happening, then you haven't been on
this list long enough. :-)  (Says a guy who's been on the list for about
half as long as Jordan.)

We have enough stubborn people on the list that MRs are going to constantly
get pulled back to the list just because someone doesn't want to use the
web interface.  That's really mean to submitters who actually want to use
the MR process and is strictly worse than what we have today.  If we're
going to actually try out MRs, we need those people trying it too at least
from the reviewer side.

> I think this is a worst case scenario situation. Normally people
> comment about separate parts of code, and not often in conflicting
> ways.
>
> If a major conflict comes up, then the submitter could ask the MR
> based reviewer to add their point on the email list discussion. (Since
> we'd be keeping email as the primary.)
>
> > It's better just to
> > have any given series have a single point for review.  Yes, this means
> that
> > everyone will be forced to start doing GitHub review as soon as someone
> > does an MR that's relevant to them.  I don't see a good way around that
> > which doesn't make a mess.
> >
> > Certain projects could, I suppose, have a requirement that all
> significant
> > work on that project be done on the list or on GitLab.  However, people
> who
> > feel like custodians of Mesa as a whole will have to do both as long as
> > both are supported.
>
> I guess by 'projects', you mean sub-projects within Mesa? I'm not
> against Eric's idea of allowing the submitter to choose email list vs
> MR, but I do think we should hold off and consider that in 3~6 months.
>

Right.  Eric Anholt has already moved most of vc4 development to GitHub
just so he can use PRs for review.  If he wants vc4 submissions to go
through GitLab MRs, that should be his choice.  I don't see us making that
switch for Intel code any time particularly soon but I think it's worth
allowing.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/3] radeonsi: allow si_cp_dma_clear_buffer to clear GDS from any IB

2018-11-28 Thread Christian König


Are those committed yet? They don't seem to apply cleanly on master.

Christian.

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

From: Marek Olšák 

---
  .../drivers/radeonsi/si_compute_blit.c|  4 +-
  src/gallium/drivers/radeonsi/si_cp_dma.c  | 49 ++-
  src/gallium/drivers/radeonsi/si_pipe.h|  8 +--
  .../drivers/radeonsi/si_test_dma_perf.c   |  3 +-
  4 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_compute_blit.c 
b/src/gallium/drivers/radeonsi/si_compute_blit.c
index 20e4f591fbb..086793637f0 100644
--- a/src/gallium/drivers/radeonsi/si_compute_blit.c
+++ b/src/gallium/drivers/radeonsi/si_compute_blit.c
@@ -212,22 +212,22 @@ void si_clear_buffer(struct si_context *sctx, struct 
pipe_resource *dst,
 */
if (clear_value_size > 4 ||
(clear_value_size == 4 &&
 offset % 4 == 0 &&
 (size > 32*1024 || sctx->chip_class <= VI))) {
si_compute_do_clear_or_copy(sctx, dst, offset, NULL, 0,
aligned_size, clear_value,
clear_value_size, coher);
} else {
assert(clear_value_size == 4);
-   si_cp_dma_clear_buffer(sctx, dst, offset,
-  aligned_size, *clear_value, 
coher,
+   si_cp_dma_clear_buffer(sctx, sctx->gfx_cs, dst, offset,
+  aligned_size, *clear_value, 0, 
coher,
   get_cache_policy(sctx, coher, 
size));
}
  
  		offset += aligned_size;

size -= aligned_size;
}
  
  	/* Handle non-dword alignment. */

if (size) {
assert(dst);
diff --git a/src/gallium/drivers/radeonsi/si_cp_dma.c 
b/src/gallium/drivers/radeonsi/si_cp_dma.c
index 839b31b7fdf..33220d9f0fa 100644
--- a/src/gallium/drivers/radeonsi/si_cp_dma.c
+++ b/src/gallium/drivers/radeonsi/si_cp_dma.c
@@ -47,25 +47,24 @@ static inline unsigned cp_dma_max_byte_count(struct 
si_context *sctx)
  
  	/* make it aligned for optimal performance */

return max & ~(SI_CPDMA_ALIGNMENT - 1);
  }
  
  
  /* Emit a CP DMA packet to do a copy from one buffer to another, or to clear

   * a buffer. The size must fit in bits [20:0]. If CP_DMA_CLEAR is set, src_va 
is a 32-bit
   * clear value.
   */
-static void si_emit_cp_dma(struct si_context *sctx, uint64_t dst_va,
-  uint64_t src_va, unsigned size, unsigned flags,
-  enum si_cache_policy cache_policy)
+static void si_emit_cp_dma(struct si_context *sctx, struct radeon_cmdbuf *cs,
+  uint64_t dst_va, uint64_t src_va, unsigned size,
+  unsigned flags, enum si_cache_policy cache_policy)
  {
-   struct radeon_cmdbuf *cs = sctx->gfx_cs;
uint32_t header = 0, command = 0;
  
  	assert(size <= cp_dma_max_byte_count(sctx));

assert(sctx->chip_class != SI || cache_policy == L2_BYPASS);
  
  	if (sctx->chip_class >= GFX9)

command |= S_414_BYTE_COUNT_GFX9(size);
else
command |= S_414_BYTE_COUNT_GFX6(size);
  
@@ -139,21 +138,21 @@ static void si_emit_cp_dma(struct si_context *sctx, uint64_t dst_va,

  }
  
  void si_cp_dma_wait_for_idle(struct si_context *sctx)

  {
/* Issue a dummy DMA that copies zero bytes.
 *
 * The DMA engine will see that there's no work to do and skip this
 * DMA request, however, the CP will see the sync flag and still wait
 * for all DMAs to complete.
 */
-   si_emit_cp_dma(sctx, 0, 0, 0, CP_DMA_SYNC, L2_BYPASS);
+   si_emit_cp_dma(sctx, sctx->gfx_cs, 0, 0, 0, CP_DMA_SYNC, L2_BYPASS);
  }
  
  static void si_cp_dma_prepare(struct si_context *sctx, struct pipe_resource *dst,

  struct pipe_resource *src, unsigned byte_count,
  uint64_t remaining_size, unsigned user_flags,
  enum si_coherency coher, bool *is_first,
  unsigned *packet_flags)
  {
/* Fast exit for a CPDMA prefetch. */
if ((user_flags & SI_CPDMA_SKIP_ALL) == SI_CPDMA_SKIP_ALL) {
@@ -200,51 +199,53 @@ static void si_cp_dma_prepare(struct si_context *sctx, 
struct pipe_resource *dst
 */
if (!(user_flags & SI_CPDMA_SKIP_SYNC_AFTER) &&
byte_count == remaining_size) {
*packet_flags |= CP_DMA_SYNC;
  
  		if (coher == SI_COHERENCY_SHADER)

*packet_flags |= CP_DMA_PFP_SYNC_ME;
}
  }
  
-void si_cp_dma_clear_buffer(struct si_context *sctx, struct pipe_resource *dst,

-   uint64_t offset, uint64_t size, unsigned value,
-

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

On 2018-11-28 10:14:49, Jason Ekstrand wrote:
> On Wed, Nov 28, 2018 at 11:35 AM Jordan Justen 
> wrote:
> > On 2018-11-28 06:59:42, Eric Engestrom wrote:
> > > On Tuesday, 2018-11-27 19:45:50 -0800, Jordan Justen wrote:
> > > > On 2018-11-27 19:20:09, Matt Turner wrote:
> > > > >
> > > > > Discussion point: I think attempting to have simultaneous review in
> > > > > two places is a recipe for wasted time.
> > > >
> > > > That's possible. It also happens on email sometimes. But, I want to
> > > > say that maybe the usual problem is too little code review, and not
> > > > too much? :)
> > >
> > > I think duplicating the review possibilities might very well lead to
> > > less review as people might (unconsciously) think "it has been/will be
> > > reviewed on the other one".
> >
> > Maybe they were looking for an excuse to not do code review? :)
> >
> > I agree with your point to some extent, but I think it's better to try
> > to work out a transition. To not jump immediately to forcing people to
> > go to the web page.
> >
> > >
> > > > > Strawman: maybe we can only email the cover letter to the mailing
> > > > > list and include in it a link to the MR?
> > > >
> > > > I was hoping to make a smaller step and see what happens. Maybe this
> > > > will give people the chance to try out MR based reviews, but not take
> > > > away email based reviews just yet.
> > > >
> > > > I don't think we should move away from email based reviews until we
> > > > are sure MR's actually work better. (I'm far from convinced on this
> > > > point. :)
> > >
> > > I think one way to solve this is to allow both, but only one at a time.
> > > Devs can choose to send their patches/series as either an MR _or_ as
> > > emails, but not both. One can still send a cover-letter style email to
> > > the ML to present the MR with a link to it.
> >
> > I would prefer to keep emails as required for now. Let people
> > optionally try it out for some time.
> >
> > Perhaps as a next step we can let the poster consider using a MR and
> > only a cover letter. (Assuming the MR method proves useful. If it
> > doesn't we should revert back to email only.)
> >
> 
> I agree with Eric on this one.  If a single submission has both types of
> review then you will effectively end up with two different groups of
> reviewers providing potentially conflicting feedback without seeing each
> other's feedback and the submitter has to mediate.

They always have to. Case in point, this patch. I want to keep all
patches on the list but let people optionally post an MR so people can
try it out as a first step. Dylan says it has to be all or nothing.
You and Eric say the submitter should be forced to choose one or the
other. (I'm not sure where to go from here. :)

I think this is a worst case scenario situation. Normally people
comment about separate parts of code, and not often in conflicting
ways.

If a major conflict comes up, then the submitter could ask the MR
based reviewer to add their point on the email list discussion. (Since
we'd be keeping email as the primary.)

> It's better just to
> have any given series have a single point for review.  Yes, this means that
> everyone will be forced to start doing GitHub review as soon as someone
> does an MR that's relevant to them.  I don't see a good way around that
> which doesn't make a mess.
> 
> Certain projects could, I suppose, have a requirement that all significant
> work on that project be done on the list or on GitLab.  However, people who
> feel like custodians of Mesa as a whole will have to do both as long as
> both are supported.

I guess by 'projects', you mean sub-projects within Mesa? I'm not
against Eric's idea of allowing the submitter to choose email list vs
MR, but I do think we should hold off and consider that in 3~6 months.

> > > Other things that need to be present in this 'GitLab MR' document are:
> > >
> > > - Review process: when you change something in your MR, force-push the
> > >   new version, don't add fixup commits. The history of the MR at any one
> > >   time must be "clean" in the sense that it would be acceptable to push
> > >   it as is.
> >
> > We don't document this for email based reviews, but people seem to
> > figure it out. :)
> >
> > Is this more of a concern because they might not know that pushing the
> > same branch updates the MR?
> >
> 
> I think it's worth explicitly documenting.  Many people who are used to the
> MR workflow are also used to having very sloppy branches because they
> assume that only the final merge matters.  This is why GitLab and GitHub
> both have an option for "squash before merge". :-(

Ok. It seems reasonable to add.

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

[Mesa-dev] [ANNOUNCE] mesa 18.2.6

2018-11-28 Thread Juan A. Suarez Romero

Mesa 18.2.6 is now available.

In this release we have:

Several patches fixing leaks in glsl, winsys and r600.

Improvements in the scripts that helps in preparing releases.

Added PCI IDs for Amber Lake and Whiskey Lake.

Fixes for radv, anv, i965 and vc4 drivers.

A couple of fixes in NIR backend.

Finally, several fixes in meson build system.


Andrii Simiklit (1):
  i965/batch: avoid reverting batch buffer if saved state is an empty

Bas Nieuwenhuizen (1):
  radv: Fix opaque metadata descriptor last layer.

Brian Paul (1):
  scons/svga: remove opt from the list of valid build types

Danylo Piliaiev (1):
  i965: Fix calculation of layers array length for isl_view

Dylan Baker (2):
  meson: Don't set -Wall
  meson: Don't force libva to required from auto

Emil Velikov (13):
  bin/get-pick-list.sh: simplify git oneline printing
  bin/get-pick-list.sh: prefix output with "[stable] "
  bin/get-pick-list.sh: handle "typod" usecase.
  bin/get-pick-list.sh: handle the fixes tag
  bin/get-pick-list.sh: tweak the commit sha matching pattern
  bin/get-pick-list.sh: flesh out is_sha_nomination
  bin/get-pick-list.sh: handle fixes tag with missing colon
  bin/get-pick-list.sh: handle unofficial "broken by" tag
  bin/get-pick-list.sh: use test instead of [ ]
  bin/get-pick-list.sh: handle reverts prior to the branchpoint
  travis: drop unneeded x11proto-xf86vidmode-dev
  glx: make xf86vidmode mandatory for direct rendering
  travis: adding missing x11-xcb for meson+vulkan

Eric Anholt (1):
  vc4: Make sure we make ro scanout resources for create_with_modifiers.

Eric Engestrom (5):
  meson: only run vulkan's meson.build when building vulkan
  gbm: remove unnecessary meson include
  meson: fix wayland-less builds
  egl: add missing glvnd entrypoint for EGL_ANDROID_blob_cache
  glapi: add missing visibility args

Erik Faye-Lund (1):
  mesa/main: remove bogus error for zero-sized images

Gert Wollny (3):
  mesa: Reference count shaders that are used by transform feedback objects
  r600: clean up the GS ring buffers when the context is destroyed
  glsl: free or reuse memory allocated for TF varying

Jason Ekstrand (2):
  nir/lower_alu_to_scalar: Don't try to lower unpack_32_2x16
  anv: Put robust buffer access in the pipeline hash

Juan A. Suarez Romero (7):
  cherry-ignore: add explicit 18.3 only nominations
  cherry-ignore: intel/aub_viewer: fix dynamic state printing
  cherry-ignore: intel/aub_viewer: Print blend states properly
  cherry-ignore: mesa/main: fix incorrect depth-error
  docs: add sha256 checksums for 18.2.5
  Update version to 18.2.6
  docs: add release notes for 18.2.6

Karol Herbst (1):
  nir/spirv: cast shift operand to u32

Kenneth Graunke (1):
  i965: Add PCI IDs for new Amberlake parts that are Coffeelake based

Lionel Landwerlin (1):
  egl/dri: fix error value with unknown drm format

Marek Olšák (2):
  winsys/amdgpu: fix a buffer leak in amdgpu_bo_from_handle
  winsys/amdgpu: fix a device handle leak in amdgpu_winsys_create

Rodrigo Vivi (4):
  i965: Add a new CFL PCI ID.
  intel: aubinator: Adding missed platforms to the error message.
  intel: Introducing Amber Lake platform
  intel: Introducing Whiskey Lake platform

git tag: mesa-18.2.6

https://mesa.freedesktop.org/archive/mesa-18.2.6.tar.gz
MD5:  b15d4e6ed4e9bea91efef787e69f3fd8  mesa-18.2.6.tar.gz
SHA1: 8de0ea20dd458e0a12f1f190eae75c4fcab19279  mesa-18.2.6.tar.gz
SHA256: e0ea1236dbc6c412b02e1b5d7f838072525971a6630246fa82ae4466a6d8a587  
mesa-18.2.6.tar.gz
SHA512: 
f9496b6279a196130329640f9b3d5158d21ffba22e603bf7ee8fe3370f6bc6937099f4b919c15f2efc9e08a51dfc5265e8bf4696cc6da68ea63efc66aea687a2
  mesa-18.2.6.tar.gz
PGP:  https://mesa.freedesktop.org/archive/mesa-18.2.6.tar.gz.sig

https://mesa.freedesktop.org/archive/mesa-18.2.6.tar.xz
MD5:  7c61a801311fb8d2f7b3cceb7b5cf308  mesa-18.2.6.tar.xz
SHA1: 435affd7a2c35782b21533ed2c65c487e40abcf8  mesa-18.2.6.tar.xz
SHA256: 9ebafa4f8249df0c718e93b9ca155e3593a1239af303aa2a8b0f2056a7efdc12  
mesa-18.2.6.tar.xz
SHA512: 
a7dd02f67384bb800dff70a0672a968ced96bb438605cdb39bde3e468d4dcf6162414a44e5da1abe7a1831fceb6f23e6c850eb5f80cfc5ee3861c14924c10ed4
  mesa-18.2.6.tar.xz
PGP:  https://mesa.freedesktop.org/archive/mesa-18.2.6.tar.xz.sig



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

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

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

Requiring emails seems to avoid these issues for now. We'd just be
trying out using merge requests, but if you want to see all patches,
use email.

> We should either go all in and archive the mailing list and use only
> gitlab, or not use PR/MR's IMHO.

If that is the only choice, then I choose that we not use MR. Or, wait
until everyone is forced to try MR via other freedesktop projects. :)

One motivation I had for this patch was to see if MR could be a
'patchwork with labels'. Although, unlike patchwork, the contributor
would have to take an extra step to open the MR and apply the label.
One positive vs patchwork would be having the same login system as
Mesa's repo.

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

[Mesa-dev] [Bug 108713] Gallium: use after free with transform feedback

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

Juan A. Suarez  changed:

   What|Removed |Added

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

--- Comment #2 from Juan A. Suarez  ---
This was fixed in upstream and 18.2.6.

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

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

2018-11-28 Thread Jason Ekstrand

First off, +1 to experimenting with MRs.  I've been working with GitLab MRs
in another context for some time and I think the process actually works out
really pretty well.  There are issues, of course, but I don't think there's
any real show-stoppers as long as we have a bit of process around it such
as requiring the submitter to apply Reviewed-by tags manually before
merging.

On Wed, Nov 28, 2018 at 11:35 AM Jordan Justen 
wrote:

> On 2018-11-28 06:59:42, Eric Engestrom wrote:
> > On Tuesday, 2018-11-27 19:45:50 -0800, Jordan Justen wrote:
> > > On 2018-11-27 19:20:09, Matt Turner wrote:
> > > >
> > > > Discussion point: I think attempting to have simultaneous review in
> > > > two places is a recipe for wasted time.
> > >
> > > That's possible. It also happens on email sometimes. But, I want to
> > > say that maybe the usual problem is too little code review, and not
> > > too much? :)
> >
> > I think duplicating the review possibilities might very well lead to
> > less review as people might (unconsciously) think "it has been/will be
> > reviewed on the other one".
>
> Maybe they were looking for an excuse to not do code review? :)
>
> I agree with your point to some extent, but I think it's better to try
> to work out a transition. To not jump immediately to forcing people to
> go to the web page.
>
> >
> > > > Strawman: maybe we can only email the cover letter to the mailing
> > > > list and include in it a link to the MR?
> > >
> > > I was hoping to make a smaller step and see what happens. Maybe this
> > > will give people the chance to try out MR based reviews, but not take
> > > away email based reviews just yet.
> > >
> > > I don't think we should move away from email based reviews until we
> > > are sure MR's actually work better. (I'm far from convinced on this
> > > point. :)
> >
> > I think one way to solve this is to allow both, but only one at a time.
> > Devs can choose to send their patches/series as either an MR _or_ as
> > emails, but not both. One can still send a cover-letter style email to
> > the ML to present the MR with a link to it.
>
> I would prefer to keep emails as required for now. Let people
> optionally try it out for some time.
>
> Perhaps as a next step we can let the poster consider using a MR and
> only a cover letter. (Assuming the MR method proves useful. If it
> doesn't we should revert back to email only.)
>

I agree with Eric on this one.  If a single submission has both types of
review then you will effectively end up with two different groups of
reviewers providing potentially conflicting feedback without seeing each
other's feedback and the submitter has to mediate.  It's better just to
have any given series have a single point for review.  Yes, this means that
everyone will be forced to start doing GitHub review as soon as someone
does an MR that's relevant to them.  I don't see a good way around that
which doesn't make a mess.

Certain projects could, I suppose, have a requirement that all significant
work on that project be done on the list or on GitLab.  However, people who
feel like custodians of Mesa as a whole will have to do both as long as
both are supported.

> > I also agree with Matt that we need to disable everything that shouldn't
> > be used, but unfortunately we can't disable the merge button without
> > disabling merge requests.
>
> I guess we can't do this. Can we just tell people with push access to
> not do that again if they make a mistake?
>
> Can merge request approvals be used to only prevent the web page from
> merging things? (But, still allow pushes.)
>
> https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html
>
> Maybe require approval from "dont-merge-use-git-push"? :)
>

I don't think we can disable pushing the "merge" button.  However, we can
require fast-forward merges and we can require either through process or
through a Git hook of some form that they have the right tags.  At that
point whether the person rebases, applies tags, force pushes the MR branch,
and then clicks merge or pushes directly to master is immaterial.

> > As for auto-closing issues and MRs by adding a special string to the
> > commit message, GitLab doesn't support that for commits that are pushed
> > behind its back, but it does support it it GitLab is used.
>
> Are you sure? Pushes seem to work this way on github, and my reading of
> https://docs.gitlab.com/ee/user/project/issues/automatic_issue_closing.html
> is that on gitlab pushes do the same.
>

I think pushes do close issues.

It's also worth throwing out there that GitLab has some sort of Bugzilla
interaction interface.  I'm not sure what all it lets you do but it's there
if we want to experiment with it.  That said, migrating to GitLab issues
may be a better plan.

> > Adding a post-receive hook that does that should be possible, I'm not
> > sure how much work that would be and if it would be worth it.
> > DanielS?
>

GitLab does support hooks but it does so

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

On 2018-11-28 06:59:42, Eric Engestrom wrote:
> On Tuesday, 2018-11-27 19:45:50 -0800, Jordan Justen wrote:
> > On 2018-11-27 19:20:09, Matt Turner wrote:
> > > 
> > > Discussion point: I think attempting to have simultaneous review in
> > > two places is a recipe for wasted time.
> > 
> > That's possible. It also happens on email sometimes. But, I want to
> > say that maybe the usual problem is too little code review, and not
> > too much? :)
> 
> I think duplicating the review possibilities might very well lead to
> less review as people might (unconsciously) think "it has been/will be
> reviewed on the other one".

Maybe they were looking for an excuse to not do code review? :)

I agree with your point to some extent, but I think it's better to try
to work out a transition. To not jump immediately to forcing people to
go to the web page.

> > 
> > > Strawman: maybe we can only email the cover letter to the mailing
> > > list and include in it a link to the MR?
> > 
> > I was hoping to make a smaller step and see what happens. Maybe this
> > will give people the chance to try out MR based reviews, but not take
> > away email based reviews just yet.
> > 
> > I don't think we should move away from email based reviews until we
> > are sure MR's actually work better. (I'm far from convinced on this
> > point. :)
> 
> I think one way to solve this is to allow both, but only one at a time.
> Devs can choose to send their patches/series as either an MR _or_ as
> emails, but not both. One can still send a cover-letter style email to
> the ML to present the MR with a link to it.

I would prefer to keep emails as required for now. Let people
optionally try it out for some time.

Perhaps as a next step we can let the poster consider using a MR and
only a cover letter. (Assuming the MR method proves useful. If it
doesn't we should revert back to email only.)

> I also agree with Matt that we need to disable everything that shouldn't
> be used, but unfortunately we can't disable the merge button without
> disabling merge requests.

I guess we can't do this. Can we just tell people with push access to
not do that again if they make a mistake?

Can merge request approvals be used to only prevent the web page from
merging things? (But, still allow pushes.)
https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html

Maybe require approval from "dont-merge-use-git-push"? :)

> As for auto-closing issues and MRs by adding a special string to the
> commit message, GitLab doesn't support that for commits that are pushed
> behind its back, but it does support it it GitLab is used.

Are you sure? Pushes seem to work this way on github, and my reading of
https://docs.gitlab.com/ee/user/project/issues/automatic_issue_closing.html
is that on gitlab pushes do the same.

> Adding a post-receive hook that does that should be possible, I'm not
> sure how much work that would be and if it would be worth it.
> DanielS?
> 
> Other things that need to be present in this 'GitLab MR' document are:
> 
> - Review process: when you change something in your MR, force-push the
>   new version, don't add fixup commits. The history of the MR at any one
>   time must be "clean" in the sense that it would be acceptable to push
>   it as is.

We don't document this for email based reviews, but people seem to
figure it out. :)

Is this more of a concern because they might not know that pushing the
same branch updates the MR?

If we only use git push, isn't this also unlikely to cause a problem
in the main branch?

> - WIP MRs: prefix your MR title with "WIP" to indicate that it's a work
>   in progress and is not ready to be merged as is (Side note: GitLab
>   will not offer to merge an MR that is prefixed with "WIP")

If we are saying that merge should not be used from the web page, then
I don't think this is in scope yet.

> - I think we should split the list in two between the dev side and the
>   reviewer side.

I'm not sure I understand what you mean, but it sounds bad. :)

> - When an issue is raised, who closes it? The dev to indicate that the
>   new revision should fix it, or the reviewer to indicate that they have
>   verified the fix?

The dev has to fix issues with getting their code merged.

If you are talking about a case of someone posting an MR that doesn't
have push access, then still the dev. In trivial cases the person with
push access can choose to go out of their way to fix up a minor issue.

We don't document the similar case with email patches, so I don't
think this is something separate to consider documenting here.

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

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

2018-11-28 Thread Dylan Baker

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

I think it's a *really* bad idea to allow both. Some people will immediately
begin using MR/PR's only and never read the list, others will never check
MR/PRs and only use the list. It'll leave the rest of us forced to use both. We
should either go all in and archive the mailing list and use only gitlab, or not
use PR/MR's IMHO.

> > +  Add appropriate labels to your MR. For example:
> > +
> > +  Mesa changes affect all drivers: mesa
> > +  Hardware vendor specific code: amd, intel, nvidia, etc
> > +  Driver specific code: anvil, freedreno, i965, iris, radeonsi, 
> > radv, vc4, etc
> > +  Other tag examples: gallium, util
> > +
> > +  Never use the merge button on the GitLab page to push patches
> 
> Can we disable this in Gitlab? If the button is there, people *will*
> accidentally press it.

I think Daniel was working on getting a "rebase and push" button like github has
so that we could use the button, CC'd.

> 
> > +  Close your MR when your patches get pushed!
> 
> Gentoo has some automation that scans the git log for "Closes: ..."
> and automatically closes the merge requests or bugzilla bugs.
> 
> Closes: https://github.com/gentoo/gentoo/pull/7423
> Closes: https://bugs.gentoo.org/603294
> 
> I'm not sure if it's a custom thing or what (I can find out) but I'd
> much prefer to automate things if we can. Just like patchwork, people
> *will* forget to close merge requests if it's possible. (And people
> currently *do* forget to close bugzilla bugs)

Or we could just use gitlab's issue tracker, which will automatically close
issues when a commit pushed to master has a tag like `Fixes: #1234` or
`Closes: #1234`.

Personally speaking, I think that better next steps for gitlab integration are:
- migrate from bugzilla to gitlab issues
- convert piglit to a PR/MR workflow and see how it goes. The list for piglit
  is barely looked at anyway.

Dylan


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

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

2018-11-28 Thread Roland Scheidegger

Am 28.11.18 um 07:37 schrieb Matt Turner:
> The common truncf(x + 0.5) fails for the floating-point value just less
> than 0.5 (nextafterf(0.5, 0.0)). nextafterf(0.5, 0.0) + 0.5, after
> rounding is 1.0, thus truncf does not produce the desired value.
> 
> The solution is to add nextafterf(0.5, 0.0) instead of 0.5 before
> truncating. This works for all values.

Reviewed-by: Roland Scheidegger 

Although this will still do round-to-nearest-away-from-zero, instead of
nearest-even which we probably want.
That said, I don't think it matters anywhere - d3d10 round will require
round-to-nearest-even, and the shader round might fall back to this, but
I don't think we really care in this case. GL in general doesn't care
anyway of course. (I don't think you can easily implement
round-to-nearest-even with a fallback.)


> ---
> I noticed this while investigating 
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.gentoo.org%2F665570data=02%7C01%7Csroland%40vmware.com%7C63d1da48159f4cbe95e308d654fbf3d8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636789838438856537sdata=yzXELCbFCR0IsjzrObWN%2B%2BdzVAOV0HQFkSn%2FxxWesWI%3Dreserved=0
>  but it
> does not fix it.
> 
> Roland, do you have a suggestion for how to make lp_build_iround() work
> on non-SSE/non-Altivec platforms? I notice that if I unconditionally
> return TRUE from arch_rounding_available() and make
> lp_build_round_arch() take the SSE4.1 path (that emits llvm.nearbyint)
> it passes on ARM64.
> 
> I noticed there's some hack in lp_test_arit.c:test_unary:
> 
>if (test->ref ==  && length == 2 &&
>ref != roundf(testval)) {
>   /* FIXME: The generic (non SSE) path in lp_build_iround, which is
>* always taken for length==2 regardless of native round support,
>* does not round to even. */
>   expected_pass = FALSE;
>}
> 
> It'd be nice to get rid of that.. but maybe we can somehow use it to
> just mark all the round tests as expected fail on other platforms if no
> real fix is forthcoming?

Actually I think arch_rounding_available() would not need to check for
vector size (but we should check for type width instead) for the sse41
case? I think llvm should be able to handle any vector size reasonably,
type legalization should take care of it, which would eliminate the
length-2 problem for sse41 (at some point we actually used sse
intrinsics, not the llvm ones.)

As a side note, perhaps we could use the llvm intrinsics on altivec too
instead nowadays, I don't know if they do the right thing there, but I
can't see why they wouldn't, but someone would need to test it.

The only problem with using the llvm intrinsics is you really really
really don't want to do it if the cpu doesn't natively support it. In
this case llvm (at least used to) emit (scalar of course) calls to math
library, which is completely horrendous (in particular since we don't
really care that much about the exact rounding), if it even works. Hence
despite using llvm intrinsics we need to know if the cpu actually
supports it.
So ideally for arm we'd actually detect cpu features like for other
archs (I don't think all arm chips can do rounding ops, even if they
have neon, although perhaps all arm64 ones can) and follow similar
logic. But noone ever submitted a arm-specific patch for better llvmpipe
support...

Roland

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

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

Re: [Mesa-dev] [PATCH mesa] build: make passing an incorrect pointer type a hard error

On 2018/11/23, Eric Engestrom wrote:
> On Thursday, 2018-11-22 15:04:54 +, Emil Velikov wrote:

> > Please add the hunk back?
> 
> Right, I thought the -Wno-error after the -Werror=foo would disable all
> -Werror=*, but after testing it, you're right, I had misunderstood.
> Adding the android hunk back.
> 
> That said, android doesn't have any of the other -Werror=foo that meson
> and autotools have; should it? Did it just get neglected when those were
> added, or is there a reason not to have any -Werror=foo on android?
> 
Perhaps it should. Personally I'd stick this one in and perhaps add others
at a later stage. That said, if you feel strongly about it - just drop it.

HTH
Emil

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

[Mesa-dev] [PATCH] egl: add missing #include in egldevice.h

2018-11-28 Thread Gurchetan Singh

Otherwise, I get this error:

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

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

v3: Use stddef.h (Matt)
v4: Modify commit message (Eric)

Reviewed-by: Eric Engestrom 
---
 src/egl/main/egldevice.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

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

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

On Wednesday, 2018-11-28 16:28:53 +0100, Erik Faye-Lund wrote:
> On Wed, 2018-11-28 at 15:20 +, Emil Velikov wrote:
> > On Wed, 28 Nov 2018 at 14:29, Eric Engestrom <
> > eric.engest...@intel.com> wrote:
> > > On Wednesday, 2018-11-28 01:18:16 -0800, Jordan Justen wrote:
> > > > On 2018-11-28 00:47:25, Erik Faye-Lund wrote:
> > > > > On Tue, 2018-11-27 at 23:20 -0800, Jordan Justen wrote:
> > > > > > This adds the "Developer's Certificate of Origin 1.1" from
> > > > > > the Linux
> > > > > > kernel. It indicates that by using Signed-off-by you are
> > > > > > certifying
> > > > > > that your patch meets the DCO 1.1 guidelines.
> > > > > > 
> > > > > > It also changes Signed-off-by from being optional to being
> > > > > > required.
> > > > > > 
> > > > > > Signed-off-by: Jordan Justen 
> > > > > > Cc: Matt Turner 
> > > > > > ---
> > > > > >  docs/submittingpatches.html | 52
> > > > > > -
> > > > > >  1 file changed, 51 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/docs/submittingpatches.html
> > > > > > b/docs/submittingpatches.html
> > > > > > index 9ae750d5a15..6d506b3691b 100644
> > > > > > --- a/docs/submittingpatches.html
> > > > > > +++ b/docs/submittingpatches.html
> > > > > > @@ -20,6 +20,8 @@
> > > > > >  
> > > > > >  Basic guidelines
> > > > > >  Patch formatting
> > > > > > +Patch Signing (Signed-off-by,
> > > > > > Developer's
> > > > > > +  Certificate of
> > > > > > Origin)
> > > > > >  Testing Patches
> > > > > >  Mailing Patches
> > > > > >  Reviewing Patches
> > > > > > @@ -73,7 +75,9 @@ if needed.  For example:
> > > > > >  is necessary, and removing it causes no regressions in
> > > > > > piglit on
> > > > > > any
> > > > > >  platform.
> > > > > >  
> > > > > > -A "Signed-off-by:" line is not required, but not
> > > > > > discouraged
> > > > > > either.
> > > > > > +A "Signed-off-by:" line is required.
> > > > > > The format
> > > > > > +and meaning of Signed-off-by is documented below in
> > > > > > +the patch signing section.
> > > > > >  If a patch addresses a bugzilla issue, that should be
> > > > > > noted in
> > > > > > the
> > > > > >  patch comment.  For example:
> > > > > >  
> > > > > > @@ -129,7 +133,53 @@ Please use common sense and do
> > > > > > not blindly add everyone.
> > > > > >  
> > > > > >  
> > > > > > 
> > > > > > +
> > > > > > +  Patch Signing (Signed-off-by, Developer's Certificate of
> > > > > > Origin)
> > > > > > +
> > > > > > 
> > > > > > +
> > > > > > +  As described in the patch
> > > > > > formatting
> > > > > > +  section, you must sign your patch by including Signed-off-
> > > > > > by in
> > > > > > the
> > > > > > +  patch commit message. The Signed-off-by must include your
> > > > > > real
> > > > > > name
> > > > > > +  and email address in this format:
> > > > > > +
> > > > > > +
> > > > > > +  Signed-off-by: Joe Hacker jhac...@foo.com
> > > > > > +
> > > > > > +
> > > > > > +  By adding Signed-of-by to your contributed patch, you
> > > > > > certify that
> > > > > > +  your contribution meets the guidelines of the Developer's
> > > > > > +  Certificate of Origin:
> > > > > > +
> > > > > > +
> > > > > > +
> > > > > > +Developer's Certificate of Origin 1.1
> > > > > > +-
> > > > > > +
> > > > > > +By making a contribution to this project, I certify that:
> > > > > > +
> > > > > > + (a) The contribution was created in whole or in part by me
> > > > > > and I
> > > > > > + have the right to submit it under the open source
> > > > > > license
> > > > > > + indicated in the file; or
> > > > > > +
> > > > > > + (b) The contribution is based upon previous work that, to
> > > > > > the best
> > > > > > + of my knowledge, is covered under an appropriate open
> > > > > > source
> > > > > > + license and I have the right under that license to
> > > > > > submit that
> > > > > > + work with modifications, whether created in whole or in
> > > > > > part
> > > > > > + by me, under the same open source license (unless I am
> > > > > > + permitted to submit under a different license), as
> > > > > > indicated
> > > > > > + in the file; or
> > > > > > +
> > > > > > + (c) The contribution was provided directly to me by some
> > > > > > other
> > > > > > + person who certified (a), (b) or (c) and I have not
> > > > > > modified
> > > > > > + it.
> > > > > > +
> > > > > > + (d) I understand and agree that this project and the
> > > > > > contribution
> > > > > > + are public and that a record of the contribution
> > > > > > (including all
> > > > > > + personal information I submit with it, including my
> > > > > > sign-off)
> > > > > > is
> > > > > > + maintained indefinitely and may be redistributed
> > > > > > consistent
> > > > > > with
> > > > > > + this project or the open source license(s) involved.
> > > > > > +
> > > > > 
> > > > > I don't think you can

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

2018-11-28 Thread Ian Romanick

On 11/28/2018 02:54 AM, Eero Tamminen wrote:
> Hi,
> 
> On 28.11.2018 2.38, Ian Romanick wrote:
>> From: Ian Romanick 
>>
>> Found using pahole.
> 
> While Pahole does static analysis for binaries, Valgrind has tool that
> can do also _run-time_ analysis of structures and other heap allocations
> utilization:
> http://valgrind.org/docs/manual/dh-manual.html

That is interesting.  I did not know about that!  pahole has some
problems with Mesa.  At least the version in my (aging) distro crashes
on some of our C++ classes.

> With the structure members usage frequency information, one can move
> often used parts to same cache-lines and infrequently used parts
> together for more efficient cache utilization.  One may also find
> structure members that get never used in practice although there's
> code that refers them.
> 
> 
> - Eero
> 
>> Changes in peak memory usage according to Valgrind massif:
>>
>> mean soft fp64 using uint64:   1,343,991,403 => 1,342,759,331
>> gfxbench5 aztec ruins high 11:    63,619,971 =>    63,555,571
>> deus ex mankind divided 148:  62,887,728 =>    62,845,304
>> deus ex mankind divided 2890: 72,399,750 =>    71,922,686
>> dirt showdown 676:    69,464,023 =>    69,238,607
>> dolphin ubershaders 210:  78,359,728 =>    77,822,072
>>
>> Signed-off-by: Ian Romanick 
>> ---
>>   src/compiler/nir/nir.h | 10 +-
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> index a292ec73e1e..74c700026ad 100644
>> --- a/src/compiler/nir/nir.h
>> +++ b/src/compiler/nir/nir.h
>> @@ -486,7 +486,7 @@ typedef struct nir_register {
>>   #define nir_foreach_register_safe(reg, reg_list) \
>>  foreach_list_typed_safe(nir_register, reg, node, reg_list)
>>   -typedef enum {
>> +typedef enum PACKED {
>>  nir_instr_type_alu,
>>  nir_instr_type_deref,
>>  nir_instr_type_call,
>> @@ -501,16 +501,16 @@ typedef enum {
>>     typedef struct nir_instr {
>>  struct exec_node node;
>> -   nir_instr_type type;
>>  struct nir_block *block;
>> -
>> -   /** generic instruction index. */
>> -   unsigned index;
>> +   nir_instr_type type;
>>    /* A temporary for optimization and analysis passes to use for
>> storing
>>   * flags.  For instance, DCE uses this to store the "dead/live"
>> info.
>>   */
>>  uint8_t pass_flags;
>> +
>> +   /** generic instruction index. */
>> +   unsigned index;
>>   } nir_instr;
>>     static inline nir_instr *
> 
> ___
> 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] egl: define NULL in egldevice.h

On Wed, 28 Nov 2018 at 12:33, Eric Engestrom  wrote:
>
> On Tuesday, 2018-11-27 22:52:47 -0800, Matt Turner wrote:
> > Reviewed-by: Matt Turner 
> >
> > I'll commit it tomorrow.
>
> When you do that, the commit title should be fixed to something like
>   egl: add missing #include  in egldevice.h
> since the original commit title doesn't apply anymore.
>
> And while at it:
> Reviewed-by: Eric Engestrom 
> Cc: mesa-sta...@lists.freedesktop.org

With the above, patch is
Reviewed-by: Emil Velikov 

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

Re: [Mesa-dev] [PATCH] winsys/svga: Fix a memory leak

On Wed, 28 Nov 2018 at 08:14, Thomas Hellstrom  wrote:
>
> The ioctl.cap_3d member was never freed.
>
> Signed-off-by: Thomas Hellstrom 
> Reviewed-by: Sinclair Yeh 

Cc: mesa-sta...@lists.freedesktop.org
Reviewed-by: Emil Velikov 

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

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

2018-11-28 Thread Erik Faye-Lund

On Wed, 2018-11-28 at 15:20 +, Emil Velikov wrote:
> On Wed, 28 Nov 2018 at 14:29, Eric Engestrom <
> eric.engest...@intel.com> wrote:
> > On Wednesday, 2018-11-28 01:18:16 -0800, Jordan Justen wrote:
> > > On 2018-11-28 00:47:25, Erik Faye-Lund wrote:
> > > > On Tue, 2018-11-27 at 23:20 -0800, Jordan Justen wrote:
> > > > > This adds the "Developer's Certificate of Origin 1.1" from
> > > > > the Linux
> > > > > kernel. It indicates that by using Signed-off-by you are
> > > > > certifying
> > > > > that your patch meets the DCO 1.1 guidelines.
> > > > > 
> > > > > It also changes Signed-off-by from being optional to being
> > > > > required.
> > > > > 
> > > > > Signed-off-by: Jordan Justen 
> > > > > Cc: Matt Turner 
> > > > > ---
> > > > >  docs/submittingpatches.html | 52
> > > > > -
> > > > >  1 file changed, 51 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/docs/submittingpatches.html
> > > > > b/docs/submittingpatches.html
> > > > > index 9ae750d5a15..6d506b3691b 100644
> > > > > --- a/docs/submittingpatches.html
> > > > > +++ b/docs/submittingpatches.html
> > > > > @@ -20,6 +20,8 @@
> > > > >  
> > > > >  Basic guidelines
> > > > >  Patch formatting
> > > > > +Patch Signing (Signed-off-by,
> > > > > Developer's
> > > > > +  Certificate of
> > > > > Origin)
> > > > >  Testing Patches
> > > > >  Mailing Patches
> > > > >  Reviewing Patches
> > > > > @@ -73,7 +75,9 @@ if needed.  For example:
> > > > >  is necessary, and removing it causes no regressions in
> > > > > piglit on
> > > > > any
> > > > >  platform.
> > > > >  
> > > > > -A "Signed-off-by:" line is not required, but not
> > > > > discouraged
> > > > > either.
> > > > > +A "Signed-off-by:" line is required.
> > > > > The format
> > > > > +and meaning of Signed-off-by is documented below in
> > > > > +the patch signing section.
> > > > >  If a patch addresses a bugzilla issue, that should be
> > > > > noted in
> > > > > the
> > > > >  patch comment.  For example:
> > > > >  
> > > > > @@ -129,7 +133,53 @@ Please use common sense and do
> > > > > not blindly add everyone.
> > > > >  
> > > > >  
> > > > > 
> > > > > +
> > > > > +  Patch Signing (Signed-off-by, Developer's Certificate of
> > > > > Origin)
> > > > > +
> > > > > 
> > > > > +
> > > > > +  As described in the patch
> > > > > formatting
> > > > > +  section, you must sign your patch by including Signed-off-
> > > > > by in
> > > > > the
> > > > > +  patch commit message. The Signed-off-by must include your
> > > > > real
> > > > > name
> > > > > +  and email address in this format:
> > > > > +
> > > > > +
> > > > > +  Signed-off-by: Joe Hacker jhac...@foo.com
> > > > > +
> > > > > +
> > > > > +  By adding Signed-of-by to your contributed patch, you
> > > > > certify that
> > > > > +  your contribution meets the guidelines of the Developer's
> > > > > +  Certificate of Origin:
> > > > > +
> > > > > +
> > > > > +
> > > > > +Developer's Certificate of Origin 1.1
> > > > > +-
> > > > > +
> > > > > +By making a contribution to this project, I certify that:
> > > > > +
> > > > > + (a) The contribution was created in whole or in part by me
> > > > > and I
> > > > > + have the right to submit it under the open source
> > > > > license
> > > > > + indicated in the file; or
> > > > > +
> > > > > + (b) The contribution is based upon previous work that, to
> > > > > the best
> > > > > + of my knowledge, is covered under an appropriate open
> > > > > source
> > > > > + license and I have the right under that license to
> > > > > submit that
> > > > > + work with modifications, whether created in whole or in
> > > > > part
> > > > > + by me, under the same open source license (unless I am
> > > > > + permitted to submit under a different license), as
> > > > > indicated
> > > > > + in the file; or
> > > > > +
> > > > > + (c) The contribution was provided directly to me by some
> > > > > other
> > > > > + person who certified (a), (b) or (c) and I have not
> > > > > modified
> > > > > + it.
> > > > > +
> > > > > + (d) I understand and agree that this project and the
> > > > > contribution
> > > > > + are public and that a record of the contribution
> > > > > (including all
> > > > > + personal information I submit with it, including my
> > > > > sign-off)
> > > > > is
> > > > > + maintained indefinitely and may be redistributed
> > > > > consistent
> > > > > with
> > > > > + this project or the open source license(s) involved.
> > > > > +
> > > > 
> > > > I don't think you can legally copy parts for this file, but not
> > > > all of
> > > > it, due to this text (from here: 
> > > > https://developercertificate.org/)
> > > > 
> > > > "Everyone is permitted to copy and distribute verbatim copies
> > > > of this
> > > > license document, but changing it is not allowed."
> > > > 
> > > > Removing that text

Re: [Mesa-dev] [PATCH] st/xa: Fix a memory leak

On Wed, 28 Nov 2018 at 08:13, Thomas Hellstrom  wrote:
>
> Free the context after destruction.
>
> Signed-off-by: Thomas Hellstrom 
> Reviewed-by: Sinclair Yeh 

Cc: mesa-sta...@lists.freedesktop.org
Reviewed-by: Emil Velikov 

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

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

On Wed, 28 Nov 2018 at 14:29, Eric Engestrom  wrote:
>
> On Wednesday, 2018-11-28 01:18:16 -0800, Jordan Justen wrote:
> > On 2018-11-28 00:47:25, Erik Faye-Lund wrote:
> > > On Tue, 2018-11-27 at 23:20 -0800, Jordan Justen wrote:
> > > > This adds the "Developer's Certificate of Origin 1.1" from the Linux
> > > > kernel. It indicates that by using Signed-off-by you are certifying
> > > > that your patch meets the DCO 1.1 guidelines.
> > > >
> > > > It also changes Signed-off-by from being optional to being required.
> > > >
> > > > Signed-off-by: Jordan Justen 
> > > > Cc: Matt Turner 
> > > > ---
> > > >  docs/submittingpatches.html | 52
> > > > -
> > > >  1 file changed, 51 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/docs/submittingpatches.html
> > > > b/docs/submittingpatches.html
> > > > index 9ae750d5a15..6d506b3691b 100644
> > > > --- a/docs/submittingpatches.html
> > > > +++ b/docs/submittingpatches.html
> > > > @@ -20,6 +20,8 @@
> > > >  
> > > >  Basic guidelines
> > > >  Patch formatting
> > > > +Patch Signing (Signed-off-by, Developer's
> > > > +  Certificate of Origin)
> > > >  Testing Patches
> > > >  Mailing Patches
> > > >  Reviewing Patches
> > > > @@ -73,7 +75,9 @@ if needed.  For example:
> > > >  is necessary, and removing it causes no regressions in piglit on
> > > > any
> > > >  platform.
> > > >  
> > > > -A "Signed-off-by:" line is not required, but not discouraged
> > > > either.
> > > > +A "Signed-off-by:" line is required. The format
> > > > +and meaning of Signed-off-by is documented below in
> > > > +the patch signing section.
> > > >  If a patch addresses a bugzilla issue, that should be noted in
> > > > the
> > > >  patch comment.  For example:
> > > >  
> > > > @@ -129,7 +133,53 @@ Please use common sense and do
> > > > not blindly add everyone.
> > > >  
> > > >  
> > > >
> > > > +
> > > > +  Patch Signing (Signed-off-by, Developer's Certificate of Origin)
> > > > +
> > > >
> > > > +
> > > > +  As described in the patch formatting
> > > > +  section, you must sign your patch by including Signed-off-by in
> > > > the
> > > > +  patch commit message. The Signed-off-by must include your real
> > > > name
> > > > +  and email address in this format:
> > > > +
> > > > +
> > > > +  Signed-off-by: Joe Hacker jhac...@foo.com
> > > > +
> > > > +
> > > > +  By adding Signed-of-by to your contributed patch, you certify that
> > > > +  your contribution meets the guidelines of the Developer's
> > > > +  Certificate of Origin:
> > > > +
> > > > +
> > > > +
> > > > +Developer's Certificate of Origin 1.1
> > > > +-
> > > > +
> > > > +By making a contribution to this project, I certify that:
> > > > +
> > > > + (a) The contribution was created in whole or in part by me and I
> > > > + have the right to submit it under the open source license
> > > > + indicated in the file; or
> > > > +
> > > > + (b) The contribution is based upon previous work that, to the best
> > > > + of my knowledge, is covered under an appropriate open source
> > > > + license and I have the right under that license to submit that
> > > > + work with modifications, whether created in whole or in part
> > > > + by me, under the same open source license (unless I am
> > > > + permitted to submit under a different license), as indicated
> > > > + in the file; or
> > > > +
> > > > + (c) The contribution was provided directly to me by some other
> > > > + person who certified (a), (b) or (c) and I have not modified
> > > > + it.
> > > > +
> > > > + (d) I understand and agree that this project and the contribution
> > > > + are public and that a record of the contribution (including all
> > > > + personal information I submit with it, including my sign-off)
> > > > is
> > > > + maintained indefinitely and may be redistributed consistent
> > > > with
> > > > + this project or the open source license(s) involved.
> > > > +
> > >
> > > I don't think you can legally copy parts for this file, but not all of
> > > it, due to this text (from here: https://developercertificate.org/)
> > >
> > > "Everyone is permitted to copy and distribute verbatim copies of this
> > > license document, but changing it is not allowed."
> > >
> > > Removing that text (and the copyright statement above it), is changing
> > > it.
> >
> > It came from the kernel Documentation/process/submitting-patches.rst,
> > which doesn't have that specific text about "verbatim copies". I guess
> > you prefer we copy it from https://developercertificate.org/?
> >
> > > I would propose you add it as a separate file and link that, to avoid
> > > confusion about what "this license document" refers to.
> >
> > I do see that Eclipse had it on a page with other content. Although,
> > the main focus of the page is the DCO.
> > https://www.eclipse.org/legal/DCO.php
> >
> > It doesn't

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

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

I think duplicating the review possibilities might very well lead to
less review as people might (unconsciously) think "it has been/will be
reviewed on the other one".

> 
> > Strawman: maybe we can only email the cover letter to the mailing
> > list and include in it a link to the MR?
> 
> I was hoping to make a smaller step and see what happens. Maybe this
> will give people the chance to try out MR based reviews, but not take
> away email based reviews just yet.
> 
> I don't think we should move away from email based reviews until we
> are sure MR's actually work better. (I'm far from convinced on this
> point. :)

I think one way to solve this is to allow both, but only one at a time.
Devs can choose to send their patches/series as either an MR _or_ as
emails, but not both. One can still send a cover-letter style email to
the ML to present the MR with a link to it.

I also agree with Matt that we need to disable everything that shouldn't
be used, but unfortunately we can't disable the merge button without
disabling merge requests.

As for auto-closing issues and MRs by adding a special string to the
commit message, GitLab doesn't support that for commits that are pushed
behind its back, but it does support it it GitLab is used.
Adding a post-receive hook that does that should be possible, I'm not
sure how much work that would be and if it would be worth it.
DanielS?

Other things that need to be present in this 'GitLab MR' document are:

- Review process: when you change something in your MR, force-push the
  new version, don't add fixup commits. The history of the MR at any one
  time must be "clean" in the sense that it would be acceptable to push
  it as is.

- WIP MRs: prefix your MR title with "WIP" to indicate that it's a work
  in progress and is not ready to be merged as is (Side note: GitLab
  will not offer to merge an MR that is prefixed with "WIP")

- I think we should split the list in two between the dev side and the
  reviewer side.

- When an issue is raised, who closes it? The dev to indicate that the
  new revision should fix it, or the reviewer to indicate that they have
  verified the fix?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-11-28 Thread Erik Faye-Lund

On Wed, 2018-11-28 at 01:18 -0800, Jordan Justen wrote:
> On 2018-11-28 00:47:25, Erik Faye-Lund wrote:
> > On Tue, 2018-11-27 at 23:20 -0800, Jordan Justen wrote:
> > > This adds the "Developer's Certificate of Origin 1.1" from the
> > > Linux
> > > kernel. It indicates that by using Signed-off-by you are
> > > certifying
> > > that your patch meets the DCO 1.1 guidelines.
> > > 
> > > It also changes Signed-off-by from being optional to being
> > > required.
> > > 
> > > Signed-off-by: Jordan Justen 
> > > Cc: Matt Turner 
> > > ---
> > >  docs/submittingpatches.html | 52
> > > -
> > >  1 file changed, 51 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/docs/submittingpatches.html
> > > b/docs/submittingpatches.html
> > > index 9ae750d5a15..6d506b3691b 100644
> > > --- a/docs/submittingpatches.html
> > > +++ b/docs/submittingpatches.html
> > > @@ -20,6 +20,8 @@
> > >  
> > >  Basic guidelines
> > >  Patch formatting
> > > +Patch Signing (Signed-off-by,
> > > Developer's
> > > +  Certificate of Origin)
> > >  Testing Patches
> > >  Mailing Patches
> > >  Reviewing Patches
> > > @@ -73,7 +75,9 @@ if needed.  For example:
> > >  is necessary, and removing it causes no regressions in
> > > piglit on
> > > any
> > >  platform.
> > >  
> > > -A "Signed-off-by:" line is not required, but not discouraged
> > > either.
> > > +A "Signed-off-by:" line is required. The
> > > format
> > > +and meaning of Signed-off-by is documented below in
> > > +the patch signing section.
> > >  If a patch addresses a bugzilla issue, that should be noted
> > > in
> > > the
> > >  patch comment.  For example:
> > >  
> > > @@ -129,7 +133,53 @@ Please use common sense and do
> > > not blindly add everyone.
> > >  
> > >  
> > >  
> > > +
> > > +  Patch Signing (Signed-off-by, Developer's Certificate of
> > > Origin)
> > > +
> > >  
> > > +
> > > +  As described in the patch formatting
> > > +  section, you must sign your patch by including Signed-off-by
> > > in
> > > the
> > > +  patch commit message. The Signed-off-by must include your real
> > > name
> > > +  and email address in this format:
> > > +
> > > +
> > > +  Signed-off-by: Joe Hacker jhac...@foo.com
> > > +
> > > +
> > > +  By adding Signed-of-by to your contributed patch, you certify
> > > that
> > > +  your contribution meets the guidelines of the Developer's
> > > +  Certificate of Origin:
> > > +
> > > +
> > > +
> > > +Developer's Certificate of Origin 1.1
> > > +-
> > > +
> > > +By making a contribution to this project, I certify that:
> > > +
> > > + (a) The contribution was created in whole or in part by me and
> > > I
> > > + have the right to submit it under the open source license
> > > + indicated in the file; or
> > > +
> > > + (b) The contribution is based upon previous work that, to the
> > > best
> > > + of my knowledge, is covered under an appropriate open
> > > source
> > > + license and I have the right under that license to submit
> > > that
> > > + work with modifications, whether created in whole or in
> > > part
> > > + by me, under the same open source license (unless I am
> > > + permitted to submit under a different license), as
> > > indicated
> > > + in the file; or
> > > +
> > > + (c) The contribution was provided directly to me by some other
> > > + person who certified (a), (b) or (c) and I have not
> > > modified
> > > + it.
> > > +
> > > + (d) I understand and agree that this project and the
> > > contribution
> > > + are public and that a record of the contribution (including
> > > all
> > > + personal information I submit with it, including my sign-
> > > off)
> > > is
> > > + maintained indefinitely and may be redistributed consistent
> > > with
> > > + this project or the open source license(s) involved.
> > > +
> > 
> > I don't think you can legally copy parts for this file, but not all
> > of
> > it, due to this text (from here: https://developercertificate.org/)
> > 
> > "Everyone is permitted to copy and distribute verbatim copies of
> > this
> > license document, but changing it is not allowed."
> > 
> > Removing that text (and the copyright statement above it), is
> > changing
> > it.
> 
> It came from the kernel Documentation/process/submitting-patches.rst,
> which doesn't have that specific text about "verbatim copies". I
> guess
> you prefer we copy it from https://developercertificate.org/?
> 

I just assumed it came from developercertificate.org. In the Kernel
case, I suppose thatt since The Linux Foundation "controls" that
project, this is not a worry for them. But I don't really know. Seems
best to stick with the verbatim version from 
https://developercertificate.org/ IMO.

> > I would propose you add it as a separate file and link that, to
> > avoid
> > confusion about what "this license document" refers to.
> 
> I do see that Eclipse had it

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

On Wednesday, 2018-11-28 01:18:16 -0800, Jordan Justen wrote:
> On 2018-11-28 00:47:25, Erik Faye-Lund wrote:
> > On Tue, 2018-11-27 at 23:20 -0800, Jordan Justen wrote:
> > > This adds the "Developer's Certificate of Origin 1.1" from the Linux
> > > kernel. It indicates that by using Signed-off-by you are certifying
> > > that your patch meets the DCO 1.1 guidelines.
> > > 
> > > It also changes Signed-off-by from being optional to being required.
> > > 
> > > Signed-off-by: Jordan Justen 
> > > Cc: Matt Turner 
> > > ---
> > >  docs/submittingpatches.html | 52
> > > -
> > >  1 file changed, 51 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/docs/submittingpatches.html
> > > b/docs/submittingpatches.html
> > > index 9ae750d5a15..6d506b3691b 100644
> > > --- a/docs/submittingpatches.html
> > > +++ b/docs/submittingpatches.html
> > > @@ -20,6 +20,8 @@
> > >  
> > >  Basic guidelines
> > >  Patch formatting
> > > +Patch Signing (Signed-off-by, Developer's
> > > +  Certificate of Origin)
> > >  Testing Patches
> > >  Mailing Patches
> > >  Reviewing Patches
> > > @@ -73,7 +75,9 @@ if needed.  For example:
> > >  is necessary, and removing it causes no regressions in piglit on
> > > any
> > >  platform.
> > >  
> > > -A "Signed-off-by:" line is not required, but not discouraged
> > > either.
> > > +A "Signed-off-by:" line is required. The format
> > > +and meaning of Signed-off-by is documented below in
> > > +the patch signing section.
> > >  If a patch addresses a bugzilla issue, that should be noted in
> > > the
> > >  patch comment.  For example:
> > >  
> > > @@ -129,7 +133,53 @@ Please use common sense and do
> > > not blindly add everyone.
> > >  
> > >  
> > >  
> > > +
> > > +  Patch Signing (Signed-off-by, Developer's Certificate of Origin)
> > > +
> > >  
> > > +
> > > +  As described in the patch formatting
> > > +  section, you must sign your patch by including Signed-off-by in
> > > the
> > > +  patch commit message. The Signed-off-by must include your real
> > > name
> > > +  and email address in this format:
> > > +
> > > +
> > > +  Signed-off-by: Joe Hacker jhac...@foo.com
> > > +
> > > +
> > > +  By adding Signed-of-by to your contributed patch, you certify that
> > > +  your contribution meets the guidelines of the Developer's
> > > +  Certificate of Origin:
> > > +
> > > +
> > > +
> > > +Developer's Certificate of Origin 1.1
> > > +-
> > > +
> > > +By making a contribution to this project, I certify that:
> > > +
> > > + (a) The contribution was created in whole or in part by me and I
> > > + have the right to submit it under the open source license
> > > + indicated in the file; or
> > > +
> > > + (b) The contribution is based upon previous work that, to the best
> > > + of my knowledge, is covered under an appropriate open source
> > > + license and I have the right under that license to submit that
> > > + work with modifications, whether created in whole or in part
> > > + by me, under the same open source license (unless I am
> > > + permitted to submit under a different license), as indicated
> > > + in the file; or
> > > +
> > > + (c) The contribution was provided directly to me by some other
> > > + person who certified (a), (b) or (c) and I have not modified
> > > + it.
> > > +
> > > + (d) I understand and agree that this project and the contribution
> > > + are public and that a record of the contribution (including all
> > > + personal information I submit with it, including my sign-off)
> > > is
> > > + maintained indefinitely and may be redistributed consistent
> > > with
> > > + this project or the open source license(s) involved.
> > > +
> > 
> > I don't think you can legally copy parts for this file, but not all of
> > it, due to this text (from here: https://developercertificate.org/)
> > 
> > "Everyone is permitted to copy and distribute verbatim copies of this
> > license document, but changing it is not allowed."
> > 
> > Removing that text (and the copyright statement above it), is changing
> > it.
> 
> It came from the kernel Documentation/process/submitting-patches.rst,
> which doesn't have that specific text about "verbatim copies". I guess
> you prefer we copy it from https://developercertificate.org/?
> 
> > I would propose you add it as a separate file and link that, to avoid
> > confusion about what "this license document" refers to.
> 
> I do see that Eclipse had it on a page with other content. Although,
> the main focus of the page is the DCO.
> https://www.eclipse.org/legal/DCO.php
> 
> It doesn't look like https://developercertificate.org/ has a filename
> associated with the content. So, something like docs/dco.txt or
> docs/developer-certificate-of-origin.txt?

If we need to have a local copy, then I'd prefer a verbose name (so 2nd
option), but can't we simply link to it?

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

2018-11-28 Thread Danylo Piliaiev


Thanks! Looks much better than my attempt.

Series overall work as expected and eliminate the loop from the
bug: https://bugs.freedesktop.org/show_bug.cgi?id=32211

Also found two typos:

On 11/28/18 5:25 AM, Timothy Arceri wrote:

From: Danylo Piliaiev 

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

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

vkpipeline-db results (VEGA):

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

Shader-db results radeonsi (VEGA):

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

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

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

diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index dd488b1787..4a9dffb782 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -263,6 +263,100 @@ rewrite_phi_predecessor_blocks(nir_if *nif,
 }
  }
  
+static bool

+nir_block_ends_in_continue(nir_block *block)
+{
+   if (exec_list_is_empty(>instr_list))
+  return false;
+
+   nir_instr *instr = nir_block_last_instr(block);
+   return instr->type == nir_instr_type_jump &&
+  nir_instr_as_jump(instr)->type == nir_jump_continue;
+}
+
+/**
+ * This optimization turns:
+ *
+ * loop {
+ *...
+ *if (cond) {
+ *   do_work_1();
+ *   continue;
+ *} else {
+ *}
+ *do_work_2();
+ * }
+ *
+ * into:
+ *
+ * loop {
+ *...
+ *if (cond) {
+ *   do_work_1();
+ *   continue;
+ *} else {
+ *   do_work_2();
+ *}
+ * }
+ *
+ * The continue should then be removed by nir_opt_trivial_continues() and the
+ * loop can potentially be unrolled.
+ *
+ * Note: do_work_2() is only ever blocks and nested loops. We could also nest
+ * other if-statments in the branch which would allow further continues to

s/statments/statements

+ * be removed. However in practice this can result in increased register
+ * pressure.
+ */
+static bool
+opt_if_loop_last_continue(nir_loop *loop)
+{
+   /* Get the last if-stament in the loop */

s/stament/statement

+   nir_block *last_block = nir_loop_last_block(loop);
+   nir_cf_node *if_node = nir_cf_node_prev(_block->cf_node);
+   while (if_node) {
+  if (if_node->type == nir_cf_node_if)
+ break;
+
+  if_node = nir_cf_node_prev(if_node);
+   }
+
+   if (!if_node || if_node->type != nir_cf_node_if)
+  return false;
+
+   nir_if *nif = nir_cf_node_as_if(if_node);
+   nir_block *then_block = nir_if_last_then_block(nif);
+   nir_block *else_block = nir_if_last_else_block(nif);
+
+   bool then_ends_in_continue = nir_block_ends_in_continue(then_block);
+   bool else_ends_in_continue = nir_block_ends_in_continue(else_block);
+
+   /* If both branches end in a continue do nothing, this should be handled
+* by nir_opt_dead_cf().
+*/
+   if (then_ends_in_continue && else_ends_in_continue)
+  return false;
+
+   if (!then_ends_in_continue && !else_ends_in_continue)
+  return false;
+
+   /* Move the last block of the loop inside the last if-statement */
+   nir_cf_list tmp;
+   nir_cf_extract(, nir_after_cf_node(if_node),
+nir_after_block(last_block));
+   if (then_ends_in_continue) {
+  nir_cf_reinsert(, nir_after_cf_list(>else_list));
+   } else {
+  nir_cf_reinsert(, nir_after_cf_list(>then_list));
+   }
+
+   /* In order to avoid running nir_lower_regs_to_ssa_impl() every time an if
+* opt makes progress we leave nir_opt_trivial_continues() to remove the
+* continue now that the end of the loop has been simplified.
+*/
+
+   return true;
+}
+
  /**
   * This optimization turns:
   *
@@ -700,6 +794,7 @@ opt_if_cf_list(nir_builder *b, struct exec_list *cf_list)
   nir_loop *loop = nir_cf_node_as_loop(cf_node);
   progress |= opt_if_cf_list(b, >body);
   progress |= opt_peel_loop_initial_if(loop);
+

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

On Tuesday, 2018-11-27 22:52:47 -0800, Matt Turner wrote:
> Reviewed-by: Matt Turner 
> 
> I'll commit it tomorrow.

When you do that, the commit title should be fixed to something like
  egl: add missing #include  in egldevice.h
since the original commit title doesn't apply anymore.

And while at it:
Reviewed-by: Eric Engestrom 
Cc: mesa-sta...@lists.freedesktop.org
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-11-28 Thread Eero Tamminen


Hi,

On 28.11.2018 5.25, Timothy Arceri wrote:

This adds allows loop analysis to detect inductions varibales that
are incremented in both branches of an if rather than in a main
loop block. For example:

[...]> Unfortunatly GCM could move the addition out of the if for us

(making this patch unrequired) but we still cannot enable the GCM
pass without regressions.


Maybe there should be a TODO comment about removing this code when GCM 
is added?



- Eero


This unrolls a loop in Rise of The Tomb Raider.

vkpipeline-db results (VEGA):

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

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

diff --git a/src/compiler/nir/nir_loop_analyze.c 
b/src/compiler/nir/nir_loop_analyze.c
index 8903e15105..cf97d6bf06 100644
--- a/src/compiler/nir/nir_loop_analyze.c
+++ b/src/compiler/nir/nir_loop_analyze.c
@@ -245,6 +245,42 @@ compute_induction_information(loop_info_state *state)
   if (src_var->in_if_branch || src_var->in_nested_loop)
  break;
  
+ /* Detect inductions varibales that are incremented in both branches

+  * of an unnested if rather than in a loop block.
+  */
+ if (is_var_phi(src_var)) {
+nir_phi_instr *src_phi =
+   nir_instr_as_phi(src_var->def->parent_instr);
+
+nir_op alu_op;
+nir_ssa_def *alu_srcs[2] = {0};
+nir_foreach_phi_src(src2, src_phi) {
+   nir_loop_variable *src_var2 =
+  get_loop_var(src2->src.ssa, state);
+
+   if (!src_var2->in_if_branch || !is_var_alu(src_var2))
+  break;
+
+   nir_alu_instr *alu =
+  nir_instr_as_alu(src_var2->def->parent_instr);
+   if (nir_op_infos[alu->op].num_inputs != 2)
+  break;
+
+   if (alu->src[0].src.ssa == alu_srcs[0] &&
+   alu->src[1].src.ssa == alu_srcs[1] &&
+   alu->op == alu_op) {
+  /* Both branches perform the same calculation so we can use
+   * one of them to find the induction variable.
+   */
+  src_var = src_var2;
+   } else {
+  alu_srcs[0] = alu->src[0].src.ssa;
+  alu_srcs[1] = alu->src[1].src.ssa;
+  alu_op = alu->op;
+   }
+}
+ }
+
   if (!src_var->in_loop) {
  biv->def_outside_loop = src_var;
   } else if (is_var_alu(src_var)) {



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

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

2018-11-28 Thread Eero Tamminen


Hi,

On 28.11.2018 2.38, Ian Romanick wrote:

From: Ian Romanick 

Found using pahole.


While Pahole does static analysis for binaries, Valgrind has tool that
can do also _run-time_ analysis of structures and other heap allocations
utilization:
http://valgrind.org/docs/manual/dh-manual.html

With the structure members usage frequency information, one can move
often used parts to same cache-lines and infrequently used parts
together for more efficient cache utilization.  One may also find
structure members that get never used in practice although there's
code that refers them.


- Eero


Changes in peak memory usage according to Valgrind massif:

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

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

diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index a292ec73e1e..74c700026ad 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -486,7 +486,7 @@ typedef struct nir_register {
  #define nir_foreach_register_safe(reg, reg_list) \
 foreach_list_typed_safe(nir_register, reg, node, reg_list)
  
-typedef enum {

+typedef enum PACKED {
 nir_instr_type_alu,
 nir_instr_type_deref,
 nir_instr_type_call,
@@ -501,16 +501,16 @@ typedef enum {
  
  typedef struct nir_instr {

 struct exec_node node;
-   nir_instr_type type;
 struct nir_block *block;
-
-   /** generic instruction index. */
-   unsigned index;
+   nir_instr_type type;
  
 /* A temporary for optimization and analysis passes to use for storing

  * flags.  For instance, DCE uses this to store the "dead/live" info.
  */
 uint8_t pass_flags;
+
+   /** generic instruction index. */
+   unsigned index;
  } nir_instr;
  
  static inline nir_instr *




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

Re: [Mesa-dev] [PATCH v2 3.2/7] util/slab: Add function to flush allocations from a child pool

2018-11-28 Thread Haehnle, Nicolai

Both patches:

Reviewed-by: Nicolai Hähnle 

On 26.11.18 20:18, Ian Romanick wrote:
> From: Ian Romanick 
> 
> Ralloc has a feature that all allocations from a temporary memory
> context can be whisked away in a single call without fear of leaks.  As
> the slab allocator is designed for use in multhreaded scenarios with a
> child pool per CPU, it lacks this feature.  However, many users will be
> single threaded with a single child pool.  For these users, we can have
> our cake and eat it too.
> 
> v2: Fix incorrect pointer types in an assertion.  Notice by Nicolai.
>  Rename slab_mempool parameter.
> 
> Signed-off-by: Ian Romanick 
> Cc: Nicolai Hähnle 
> ---
>   src/util/slab.c | 21 +
>   src/util/slab.h |  1 +
>   2 files changed, 22 insertions(+)
> 
> diff --git a/src/util/slab.c b/src/util/slab.c
> index 5477c75d443..c11b485e587 100644
> --- a/src/util/slab.c
> +++ b/src/util/slab.c
> @@ -172,6 +172,27 @@ void slab_destroy_child(struct slab_child_pool *pool)
>  pool->parent = NULL;
>   }
>   
> +/**
> + * Flush all allocations from a pool.  Single-threaded (no mutex).
> + */
> +void
> +slab_flush_st(struct slab_mempool *mempool)
> +{
> +   struct slab_child_pool *const pool = >child;
> +
> +   assert(pool->migrated == NULL);
> +   assert(pool->parent == >parent);
> +
> +   while (pool->pages) {
> +  struct slab_page_header *page = pool->pages;
> +  pool->pages = page->u.next;
> +
> +  free(page);
> +   }
> +
> +   pool->free = NULL;
> +}
> +
>   static bool
>   slab_add_new_page(struct slab_child_pool *pool)
>   {
> diff --git a/src/util/slab.h b/src/util/slab.h
> index 5a25adaf7f4..6f309504a5f 100644
> --- a/src/util/slab.h
> +++ b/src/util/slab.h
> @@ -90,5 +90,6 @@ void slab_create(struct slab_mempool *mempool,
>   void slab_destroy(struct slab_mempool *mempool);
>   void *slab_alloc_st(struct slab_mempool *mempool);
>   void slab_free_st(struct slab_mempool *mempool, void *ptr);
> +void slab_flush_st(struct slab_mempool *mempool);
>   
>   #endif
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-11-28 Thread Timothy Arceri


On 28/11/18 6:52 pm, Thomas Helland wrote:

Den ons. 28. nov. 2018 kl. 04:26 skrev Timothy Arceri :


This adds allows loop analysis to detect inductions varibales that
are incremented in both branches of an if rather than in a main
loop block. For example:

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

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



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

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


I didn't attempt this because pulling code out of the ifs can increase 
register pressure. This is one of the problems we have with the GCM pass 
currently, so for now I chose a more conservative approach.




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


No we don't do anything like this currently. The GCM pass can pull 
things out of loops also, but again we hit register pressure issues with 
that pass.


As far as I'm aware reducing invariants is not where we get most of our 
wins from with unrolling. Removing indirect array access, improving 
opportunities for constant folding (and a bunch of other passes), being 
able to evaluate the unfolded loop with the surrounding code etc all 
result in greater benefits.


With the limits we place on making sure we don't unroll large loops that 
are going to cause register use issues, nobody has yet been able to show 
that always unrolling loops is causing any harm, and it's certainly been 
shown to help :)

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

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