[Mesa-dev] [PATCH v5 10/18] drm/i915: Add engine reset count in get-reset-stats ioctl

2017-03-24 Thread Michel Thierry
Users/tests relying on the total reset count will start seeing a smaller
number since most of the hangs can be handled by engine reset.
Note that if reset engine x, context a running on engine y will be unaware
and unaffected.

To start the discussion, include just a total engine reset count. If it
is deemed useful, it can be extended to report each engine separately.

v2: s/engine_reset/reset_engine/, use union in uapi to not break compatibility.

Cc: Chris Wilson 
Cc: Mika Kuoppala 
Cc: mesa-dev@lists.freedesktop.org
Signed-off-by: Michel Thierry 
---
 drivers/gpu/drm/i915/i915_gem_context.c | 14 +++---
 include/uapi/drm/i915_drm.h |  6 +-
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 8bd0c4966913..edbed85a1c88 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -1133,9 +1133,11 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device 
*dev,
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_i915_reset_stats *args = data;
struct i915_gem_context *ctx;
+   struct intel_engine_cs *engine;
+   enum intel_engine_id id;
int ret;
 
-   if (args->flags || args->pad)
+   if (args->flags)
return -EINVAL;
 
if (args->ctx_id == DEFAULT_CONTEXT_HANDLE && !capable(CAP_SYS_ADMIN))
@@ -1151,10 +1153,16 @@ int i915_gem_context_reset_stats_ioctl(struct 
drm_device *dev,
return PTR_ERR(ctx);
}
 
-   if (capable(CAP_SYS_ADMIN))
+   if (capable(CAP_SYS_ADMIN)) {
args->reset_count = i915_reset_count(_priv->gpu_error);
-   else
+   for_each_engine(engine, dev_priv, id)
+   args->reset_engine_count +=
+   i915_reset_engine_count(_priv->gpu_error,
+   engine);
+   } else {
args->reset_count = 0;
+   args->reset_engine_count = 0;
+   }
 
args->batch_active = ctx->guilty_count;
args->batch_pending = ctx->active_count;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 3554495bef13..f083931a7809 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1263,7 +1263,11 @@ struct drm_i915_reset_stats {
/* Number of batches lost pending for execution, for this context */
__u32 batch_pending;
 
-   __u32 pad;
+   union {
+   __u32 pad;
+   /* Engine resets since boot/module reload, for all contexts */
+   __u32 reset_engine_count;
+   };
 };
 
 struct drm_i915_gem_userptr {
-- 
2.11.0

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


Re: [Mesa-dev] [PATCH] anv/cmd_buffer: Apply flush operations prior to executing secondaries

2017-03-24 Thread Greg Daniel
In Skia, we were running into a rendering bug where we would render to a
VkImage and then immediately do another draw to a second VkImage sampling
the first as a texture (with memory barrier in between). All our draws go
through secondary command buffers. On certain hardware, the effects of
draws to the first VkImage were not there when we sampled it in the next
draw (we would just get whatever values were in the VkImage previously). We
had double checked all our memory barriers and other synchronization, ran
all the code through the SDK debug layers, and other general debugging and
none of this showed any sort of error on our part.

After patching the above fix to flush before execute the secondary buffers,
all our unit and rendering tests started passing/drawing correctly.

Greg

On Fri, Mar 24, 2017 at 7:33 PM Jason Ekstrand  wrote:

This apparently fixes rendering corruptions on the Vulkan port of Skia on
some hardware.

On Fri, Mar 24, 2017 at 4:31 PM, Jason Ekstrand 
wrote:

Cc: "13.0 17.0" 
---
 src/intel/vulkan/genX_cmd_buffer.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/intel/vulkan/genX_cmd_buffer.c
b/src/intel/vulkan/genX_cmd_buffer.c
index e2364db..8ec882e 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -654,6 +654,11 @@ genX(CmdExecuteCommands)(
 */
genX(cmd_buffer_enable_pma_fix)(primary, false);

+   /* The secondary command buffer doesn't know which textures etc. have
been
+* flushed prior to their execution.  Apply those flushes now.
+*/
+   genX(cmd_buffer_apply_pipe_flushes)(primary);
+
for (uint32_t i = 0; i < commandBufferCount; i++) {
   ANV_FROM_HANDLE(anv_cmd_buffer, secondary, pCmdBuffers[i]);



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


[Mesa-dev] [PATCH] st/mesa: move duplicated st_ws_framebuffer() function into header file

2017-03-24 Thread Brian Paul
---
 src/mesa/state_tracker/st_cb_fbo.h  | 17 +
 src/mesa/state_tracker/st_cb_viewport.c | 15 +--
 src/mesa/state_tracker/st_manager.c | 14 --
 3 files changed, 18 insertions(+), 28 deletions(-)

diff --git a/src/mesa/state_tracker/st_cb_fbo.h 
b/src/mesa/state_tracker/st_cb_fbo.h
index 414a661..d3e0554 100644
--- a/src/mesa/state_tracker/st_cb_fbo.h
+++ b/src/mesa/state_tracker/st_cb_fbo.h
@@ -30,6 +30,7 @@
 #define ST_CB_FBO_H
 
 #include "main/compiler.h"
+#include "main/fbobject.h"
 #include "main/glheader.h"
 #include "main/mtypes.h"
 
@@ -74,6 +75,22 @@ st_renderbuffer(struct gl_renderbuffer *rb)
 }
 
 
+/**
+ * Cast wrapper to convert a struct gl_framebuffer to an st_framebuffer.
+ * Return NULL if the struct gl_framebuffer is a user-created framebuffer.
+ * We'll only return non-null for window system framebuffers.
+ * Note that this function may fail.
+ */
+static inline struct st_framebuffer *
+st_ws_framebuffer(struct gl_framebuffer *fb)
+{
+   /* FBO cannot be casted.  See st_new_framebuffer */
+   if (fb && _mesa_is_winsys_fbo(fb))
+  return (struct st_framebuffer *) fb;
+   return NULL;
+}
+
+
 extern struct gl_renderbuffer *
 st_new_renderbuffer_fb(enum pipe_format format, int samples, boolean sw);
 
diff --git a/src/mesa/state_tracker/st_cb_viewport.c 
b/src/mesa/state_tracker/st_cb_viewport.c
index d7a3412..ff18fd0 100644
--- a/src/mesa/state_tracker/st_cb_viewport.c
+++ b/src/mesa/state_tracker/st_cb_viewport.c
@@ -27,26 +27,13 @@
 
 #include "main/glheader.h"
 #include "st_context.h"
+#include "st_cb_fbo.h"
 #include "st_cb_viewport.h"
 
 #include "pipe/p_state.h"
 #include "pipe/p_defines.h"
 #include "util/u_atomic.h"
 
-/**
- * Cast wrapper to convert a struct gl_framebuffer to an st_framebuffer.
- * Return NULL if the struct gl_framebuffer is a user-created framebuffer.
- * We'll only return non-null for window system framebuffers.
- * Note that this function may fail.
- */
-static inline struct st_framebuffer *
-st_ws_framebuffer(struct gl_framebuffer *fb)
-{
-   /* FBO cannot be casted.  See st_new_framebuffer */
-   if (fb && _mesa_is_winsys_fbo(fb))
-  return (struct st_framebuffer *) fb;
-   return NULL;
-}
 
 static void st_viewport(struct gl_context *ctx)
 {
diff --git a/src/mesa/state_tracker/st_manager.c 
b/src/mesa/state_tracker/st_manager.c
index dad408a..5942bb7 100644
--- a/src/mesa/state_tracker/st_manager.c
+++ b/src/mesa/state_tracker/st_manager.c
@@ -58,20 +58,6 @@
 #include "util/u_atomic.h"
 #include "util/u_surface.h"
 
-/**
- * Cast wrapper to convert a struct gl_framebuffer to an st_framebuffer.
- * Return NULL if the struct gl_framebuffer is a user-created framebuffer.
- * We'll only return non-null for window system framebuffers.
- * Note that this function may fail.
- */
-static inline struct st_framebuffer *
-st_ws_framebuffer(struct gl_framebuffer *fb)
-{
-   /* FBO cannot be casted.  See st_new_framebuffer */
-   if (fb && _mesa_is_winsys_fbo(fb))
-  return (struct st_framebuffer *) fb;
-   return NULL;
-}
 
 /**
  * Map an attachment to a buffer index.
-- 
1.9.1

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


Re: [Mesa-dev] [PATCH] mesa: simplify code around 'variable_data' in marshal.c

2017-03-24 Thread Timothy Arceri

This looks better. Thanks Brian.

Reviewed-by: Timothy Arceri 

On 25/03/17 02:50, Brian Paul wrote:

Remove needless pointer increments, unneeded vars, etc.  Untested.
Plus, fix a couple comments.
---
 src/mesa/main/marshal.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
index cdc7fed..783ca0a 100644
--- a/src/mesa/main/marshal.c
+++ b/src/mesa/main/marshal.c
@@ -267,7 +267,7 @@ struct marshal_cmd_BufferData
GLsizeiptr size;
GLenum usage;
bool data_null; /* If set, no data follows for "data" */
-   /* Next size bytes are GLvoid data[size] */
+   /* Next size bytes are GLubyte data[size] */
 };

 void
@@ -277,13 +277,12 @@ _mesa_unmarshal_BufferData(struct gl_context *ctx,
const GLenum target = cmd->target;
const GLsizeiptr size = cmd->size;
const GLenum usage = cmd->usage;
-   const char *variable_data = (const char *) (cmd + 1);
-   const GLvoid *data = (const GLvoid *) variable_data;
+   const void *data;

if (cmd->data_null)
   data = NULL;
else
-  variable_data += size;
+  data = (const void *) (cmd + 1);

CALL_BufferData(ctx->CurrentServerDispatch, (target, size, data, usage));
 }
@@ -312,11 +311,10 @@ _mesa_marshal_BufferData(GLenum target, GLsizeiptr size, 
const GLvoid * data,
   cmd->target = target;
   cmd->size = size;
   cmd->usage = usage;
-  char *variable_data = (char *) (cmd + 1);
   cmd->data_null = !data;
-  if (!cmd->data_null) {
+  if (data) {
+ char *variable_data = (char *) (cmd + 1);
  memcpy(variable_data, data, size);
- variable_data += size;
   }
   _mesa_post_marshal_hook(ctx);
} else {
@@ -333,7 +331,7 @@ struct marshal_cmd_BufferSubData
GLenum target;
GLintptr offset;
GLsizeiptr size;
-   /* Next size bytes are GLvoid data[size] */
+   /* Next size bytes are GLubyte data[size] */
 };

 void
@@ -343,10 +341,8 @@ _mesa_unmarshal_BufferSubData(struct gl_context *ctx,
const GLenum target = cmd->target;
const GLintptr offset = cmd->offset;
const GLsizeiptr size = cmd->size;
-   const char *variable_data = (const char *) (cmd + 1);
-   const GLvoid *data = (const GLvoid *) variable_data;
+   const void *data = (const void *) (cmd + 1);

-   variable_data += size;
CALL_BufferSubData(ctx->CurrentServerDispatch,
   (target, offset, size, data));
 }
@@ -375,7 +371,6 @@ _mesa_marshal_BufferSubData(GLenum target, GLintptr offset, 
GLsizeiptr size,
   cmd->size = size;
   char *variable_data = (char *) (cmd + 1);
   memcpy(variable_data, data, size);
-  variable_data += size;
   _mesa_post_marshal_hook(ctx);
} else {
   _mesa_glthread_finish(ctx);


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


[Mesa-dev] [PATCH v5 4/5] intel: Fix requests for exact surface row pitch (v2)

2017-03-24 Thread Chad Versace
All callers of isl_surf_init() that set 'min_row_pitch' wanted to
request an *exact* row pitch, as evidenced by nearby asserts, but isl
lacked API for doing so. Now that isl has an API for that, update the
code to use it.

v2: Assert that isl_surf_init() succeeds because the callers assume
it.  [for jekstrand]

Reviewed-by: Nanley Chery  (v1)
Reviewed-by: Anuj Phogat  (v1)
Reviewed-by: Jason Ekstrand  (v2)
---
 src/intel/blorp/blorp_blit.c |  8 +---
 src/intel/vulkan/anv_blorp.c | 29 +++--
 src/intel/vulkan/anv_image.c |  2 +-
 3 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/src/intel/blorp/blorp_blit.c b/src/intel/blorp/blorp_blit.c
index 280b76ab70c..691564c8788 100644
--- a/src/intel/blorp/blorp_blit.c
+++ b/src/intel/blorp/blorp_blit.c
@@ -1375,6 +1375,8 @@ static void
 surf_convert_to_single_slice(const struct isl_device *isl_dev,
  struct brw_blorp_surface_info *info)
 {
+   bool ok UNUSED;
+
/* Just bail if we have nothing to do. */
if (info->surf.dim == ISL_SURF_DIM_2D &&
info->view.base_level == 0 && info->view.base_array_layer == 0 &&
@@ -1421,13 +1423,13 @@ surf_convert_to_single_slice(const struct isl_device 
*isl_dev,
   .levels = 1,
   .array_len = 1,
   .samples = info->surf.samples,
-  .min_pitch = info->surf.row_pitch,
+  .row_pitch = info->surf.row_pitch,
   .usage = info->surf.usage,
   .tiling_flags = 1 << info->surf.tiling,
};
 
-   isl_surf_init_s(isl_dev, >surf, _info);
-   assert(info->surf.row_pitch == init_info.min_pitch);
+   ok = isl_surf_init_s(isl_dev, >surf, _info);
+   assert(ok);
 
/* The view is also different now. */
info->view.base_level = 0;
diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
index 1f4fec5f35b..80ab4339ffe 100644
--- a/src/intel/vulkan/anv_blorp.c
+++ b/src/intel/vulkan/anv_blorp.c
@@ -133,6 +133,7 @@ get_blorp_surf_for_anv_buffer(struct anv_device *device,
 {
const struct isl_format_layout *fmtl =
   isl_format_get_layout(format);
+   bool ok UNUSED;
 
/* ASTC is the only format which doesn't support linear layouts.
 * Create an equivalently sized surface with ISL to get around this.
@@ -155,20 +156,20 @@ get_blorp_surf_for_anv_buffer(struct anv_device *device,
   },
};
 
-   isl_surf_init(>isl_dev, isl_surf,
- .dim = ISL_SURF_DIM_2D,
- .format = format,
- .width = width,
- .height = height,
- .depth = 1,
- .levels = 1,
- .array_len = 1,
- .samples = 1,
- .min_pitch = row_pitch,
- .usage = ISL_SURF_USAGE_TEXTURE_BIT |
-  ISL_SURF_USAGE_RENDER_TARGET_BIT,
- .tiling_flags = ISL_TILING_LINEAR_BIT);
-   assert(isl_surf->row_pitch == row_pitch);
+   ok = isl_surf_init(>isl_dev, isl_surf,
+ .dim = ISL_SURF_DIM_2D,
+ .format = format,
+ .width = width,
+ .height = height,
+ .depth = 1,
+ .levels = 1,
+ .array_len = 1,
+ .samples = 1,
+ .row_pitch = row_pitch,
+ .usage = ISL_SURF_USAGE_TEXTURE_BIT |
+  ISL_SURF_USAGE_RENDER_TARGET_BIT,
+ .tiling_flags = ISL_TILING_LINEAR_BIT);
+   assert(ok);
 }
 
 static void
diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
index 33499abca1a..cf34dbe3b0a 100644
--- a/src/intel/vulkan/anv_image.c
+++ b/src/intel/vulkan/anv_image.c
@@ -166,7 +166,7 @@ make_surface(const struct anv_device *dev,
   .array_len = vk_info->arrayLayers,
   .samples = vk_info->samples,
   .min_alignment = 0,
-  .min_pitch = anv_info->stride,
+  .row_pitch = anv_info->stride,
   .usage = choose_isl_surf_usage(image->usage, aspect),
   .tiling_flags = tiling_flags);
 
-- 
2.12.0

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


[Mesa-dev] [PATCH v5 1/5] genxml: New generated header genX_bits.h (v5)

2017-03-24 Thread Chad Versace
genX_bits.h contains the sizes of bitfields in genxml instructions,
structures, and registers. It also defines some functions to query those
sizes.

isl_surf_init() will use the new header to validate that requested
pitches fit in their destination bitfields.

What's currently in genX_bits.h:

  - Each CONTAINER::Field from gen*.xml that has a bitsize has a macro
in genX_bits.h:

#define GEN{N}_CONTAINER_Field_bits {bitsize}

  - For each set of macros whose name, after stripping the GEN prefix,
is the same, genX_bits.h contains a query function:

  static inline uint32_t __attribute__((pure))
  CONTAINER_Field_bits(const struct gen_device_info *devinfo);

v2 (Chad Versace):
  - Parse the XML instead of scraping the generated gen*_pack.h headers.

v3 (Dylan Baker):
  - Port to Mako.

v4 (Jason Ekstrand):
  - Make the _bits functions take a gen_device_info.

v5 (Chad Versace):
  - Fix autotools out-of-tree build.
  - Fix Android build. Tested with git://github.com/android-ia/manifest.
  - Fix macro names. They were all missing the "_bits" suffix.
  - Fix macros names more. Remove all double-underscores.
  - Unindent all generated code. (It was floating in a sea of whitespace).
  - Reformat header to appear human-written not machine-generated.
  - Sort gens from high to low. Newest gens should come first because,
when we read code, we likely want to read the gen8/9 code and ignore
the gen4 code. So put the gen4 code at the bottom.
  - Replace 'const' attributes with 'pure', because the functions now
have a pointer parameter.
  - Add --cpp-guard flag. Used by Android.
  - Kill class FieldCollection. After Jason's rewrite, it was just
a dict.

Co-authored-by: Dylan Baker 
Co-authored-by: Jason Ekstrand 
---
 src/intel/Android.genxml.mk |   9 +-
 src/intel/Makefile.genxml.am|   6 +-
 src/intel/Makefile.sources  |   6 +-
 src/intel/genxml/.gitignore |   1 +
 src/intel/genxml/gen_bits_header.py | 281 
 5 files changed, 300 insertions(+), 3 deletions(-)
 create mode 100644 src/intel/genxml/gen_bits_header.py

diff --git a/src/intel/Android.genxml.mk b/src/intel/Android.genxml.mk
index 79de7843801..842d0e13a33 100644
--- a/src/intel/Android.genxml.mk
+++ b/src/intel/Android.genxml.mk
@@ -46,9 +46,16 @@ LOCAL_GENERATED_SOURCES += $(addprefix $(intermediates)/, 
$(GENXML_GENERATED_FIL
 define header-gen
@mkdir -p $(dir $@)
@echo "Gen Header: $(PRIVATE_MODULE) <= $(notdir $(@))"
-   $(hide) $(PRIVATE_SCRIPT) $(PRIVATE_XML) > $@
+   $(hide) $(PRIVATE_SCRIPT) $(PRIVATE_SCRIPT_FLAGS) $(PRIVATE_XML) > $@
 endef
 
+$(intermediates)/genxml/genX_bits.h: PRIVATE_SCRIPT := $(MESA_PYTHON2) 
$(LOCAL_PATH)/genxml/gen_bits_header.py
+$(intermediates)/genxml/genX_bits.h: PRIVATE_SCRIPT_FLAGS := 
--cpp-guard=GENX_BITS_H
+$(intermediates)/genxml/genX_bits.h: PRIVATE_XML := $(addprefix 
$(LOCAL_PATH)/,$(GENXML_XML_FILES))
+$(intermediates)/genxml/genX_bits.h: $(LOCAL_PATH)/genxml/gen_bits_header.py
+$(intermediates)/genxml/genX_bits.h: $(addprefix 
$(LOCAL_PATH)/,$(GENXML_XML_FILES))
+   $(call header-gen)
+
 $(intermediates)/genxml/gen4_pack.h: PRIVATE_SCRIPT := $(MESA_PYTHON2) 
$(LOCAL_PATH)/genxml/gen_pack_header.py
 $(intermediates)/genxml/gen4_pack.h: PRIVATE_XML := 
$(LOCAL_PATH)/genxml/gen4.xml
 $(intermediates)/genxml/gen4_pack.h: $(LOCAL_PATH)/genxml/gen4.xml 
$(LOCAL_PATH)/genxml/gen_pack_header.py
diff --git a/src/intel/Makefile.genxml.am b/src/intel/Makefile.genxml.am
index 01a02b63b44..474b751f5fd 100644
--- a/src/intel/Makefile.genxml.am
+++ b/src/intel/Makefile.genxml.am
@@ -30,7 +30,7 @@ EXTRA_DIST += \
 
 SUFFIXES = _pack.h _xml.h .xml
 
-$(GENXML_GENERATED_FILES): genxml/gen_pack_header.py
+$(GENXML_GENERATED_PACK_FILES): genxml/gen_pack_header.py
 
 .xml_pack.h:
$(MKDIR_GEN)
@@ -42,6 +42,10 @@ $(AUBINATOR_GENERATED_FILES): genxml/gen_zipped_file.py
$(MKDIR_GEN)
$(AM_V_GEN) $(PYTHON2) $(srcdir)/genxml/gen_zipped_file.py $< > $@ || 
($(RM) $@; false)
 
+genxml/genX_bits.h: genxml/gen_bits_header.py $(GENXML_XML_FILES)
+   $(MKDIR_GEN)
+   $(PYTHON_GEN) $< -o $@ $(addprefix $(srcdir)/,$(GENXML_XML_FILES))
+
 EXTRA_DIST += \
genxml/genX_pack.h \
genxml/gen_macros.h \
diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
index 88bcf60f6e3..c56891643ce 100644
--- a/src/intel/Makefile.sources
+++ b/src/intel/Makefile.sources
@@ -119,7 +119,7 @@ GENXML_XML_FILES = \
genxml/gen8.xml \
genxml/gen9.xml
 
-GENXML_GENERATED_FILES = \
+GENXML_GENERATED_PACK_FILES = \
genxml/gen4_pack.h \
genxml/gen45_pack.h \
genxml/gen5_pack.h \
@@ -129,6 +129,10 @@ GENXML_GENERATED_FILES = \
genxml/gen8_pack.h \
genxml/gen9_pack.h
 
+GENXML_GENERATED_FILES = \
+   $(GENXML_GENERATED_PACK_FILES) \
+   genxml/genX_bits.h
+
 

[Mesa-dev] [PATCH v5 2/5] isl: Validate the calculated row pitch (v4)

2017-03-24 Thread Chad Versace
Validate that isl_surf::row_pitch fits in the below bitfields,
if applicable based on isl_surf::usage.

RENDER_SURFACE_STATE::SurfacePitch
RENDER_SURFACE_STATE::AuxiliarySurfacePitch
3DSTATE_DEPTH_BUFFER::SurfacePitch
3DSTATE_HIER_DEPTH_BUFFER::SurfacePitch

v2:
  -Add a Makefile dependency on generated header genX_bits.h.
v3:
  - Test ISL_SURF_USAGE_STORAGE_BIT too. [for jekstrand]
  - Drop explicity dependency on generated header. [for emil]
v4:
  - Rebase for new gen_bits_header.py script.
  - Replace gen_10x with gen_device_info*.
---
 src/intel/isl/isl.c | 71 -
 1 file changed, 65 insertions(+), 6 deletions(-)

diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
index 81f40b6a6fb..4777113de84 100644
--- a/src/intel/isl/isl.c
+++ b/src/intel/isl/isl.c
@@ -25,6 +25,8 @@
 #include 
 #include 
 
+#include "genxml/genX_bits.h"
+
 #include "isl.h"
 #include "isl_gen4.h"
 #include "isl_gen6.h"
@@ -1089,18 +1091,73 @@ isl_calc_min_row_pitch(const struct isl_device *dev,
}
 }
 
-static uint32_t
+/**
+ * Is `pitch` in the valid range for a hardware bitfield, if the bitfield's
+ * size is `bits` bits?
+ *
+ * Hardware pitch fields are offset by 1. For example, if the size of
+ * RENDER_SURFACE_STATE::SurfacePitch is B bits, then the range of valid
+ * pitches is [1, 2^b] inclusive.  If the surface pitch is N, then
+ * RENDER_SURFACE_STATE::SurfacePitch must be set to N-1.
+ */
+static bool
+pitch_in_range(uint32_t n, uint32_t bits)
+{
+   assert(n != 0);
+   return likely(bits != 0 && 1 <= n && n <= (1 << bits));
+}
+
+static bool
 isl_calc_row_pitch(const struct isl_device *dev,
const struct isl_surf_init_info *surf_info,
const struct isl_tile_info *tile_info,
enum isl_dim_layout dim_layout,
-   const struct isl_extent2d *phys_slice0_sa)
+   const struct isl_extent2d *phys_slice0_sa,
+   uint32_t *out_row_pitch)
 {
const uint32_t alignment =
   isl_calc_row_pitch_alignment(surf_info, tile_info);
 
-   return isl_calc_min_row_pitch(dev, surf_info, tile_info, phys_slice0_sa,
- alignment);
+   const uint32_t row_pitch =
+  isl_calc_min_row_pitch(dev, surf_info, tile_info, phys_slice0_sa,
+ alignment);
+
+   const uint32_t row_pitch_tiles = row_pitch / tile_info->phys_extent_B.width;
+
+   if (row_pitch == 0)
+  return false;
+
+   if (dim_layout == ISL_DIM_LAYOUT_GEN9_1D) {
+  /* SurfacePitch is ignored for this layout.How should we validate it? */
+  isl_finishme("validate row pitch for ISL_DIM_LAYOUT_GEN9_1D");
+  goto done;
+   }
+
+   if (((surf_info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) ||
+(surf_info->usage & ISL_SURF_USAGE_TEXTURE_BIT) ||
+(surf_info->usage & ISL_SURF_USAGE_STORAGE_BIT)) &&
+   !pitch_in_range(row_pitch, 
RENDER_SURFACE_STATE_SurfacePitch_bits(dev->info)))
+  return false;
+
+   if (((surf_info->usage & ISL_SURF_USAGE_CCS_BIT) ||
+(surf_info->usage & ISL_SURF_USAGE_MCS_BIT)) &&
+   !pitch_in_range(row_pitch_tiles, 
RENDER_SURFACE_STATE_AuxiliarySurfacePitch_bits(dev->info)))
+  return false;
+
+   if ((surf_info->usage & ISL_SURF_USAGE_DEPTH_BIT) &&
+   !pitch_in_range(row_pitch, 
_3DSTATE_DEPTH_BUFFER_SurfacePitch_bits(dev->info)))
+  return false;
+
+   if ((surf_info->usage & ISL_SURF_USAGE_HIZ_BIT) &&
+   !pitch_in_range(row_pitch, 
_3DSTATE_HIER_DEPTH_BUFFER_SurfacePitch_bits(dev->info)))
+  return false;
+
+   if (surf_info->usage & ISL_SURF_USAGE_STENCIL_BIT)
+  isl_finishme("validate row pitch of stencil surfaces");
+
+ done:
+   *out_row_pitch = row_pitch;
+   return true;
 }
 
 /**
@@ -1275,8 +1332,10 @@ isl_surf_init_s(const struct isl_device *dev,
uint32_t pad_bytes;
isl_apply_surface_padding(dev, info, _info, _h_el, _bytes);
 
-   const uint32_t row_pitch = isl_calc_row_pitch(dev, info, _info,
- dim_layout, _slice0_sa);
+   uint32_t row_pitch;
+   if (!isl_calc_row_pitch(dev, info, _info, dim_layout,
+   _slice0_sa, _pitch))
+  return false;
 
uint32_t size, base_alignment;
if (tiling == ISL_TILING_LINEAR) {
-- 
2.12.0

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


[Mesa-dev] [PATCH v5 5/5] isl: Drop unused isl_surf_init_info::min_pitch

2017-03-24 Thread Chad Versace
Reviewed-by: Nanley Chery 
Reviewed-by: Anuj Phogat 
Reviewed-by: Jason Ekstrand 
---
 src/intel/isl/isl.c | 13 +++--
 src/intel/isl/isl.h |  3 ---
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
index 1d5a9d85341..666fe98060b 100644
--- a/src/intel/isl/isl.c
+++ b/src/intel/isl/isl.c
@@ -1043,11 +1043,7 @@ isl_calc_linear_min_row_pitch(const struct isl_device 
*dev,
 
assert(phys_slice0_sa->w % fmtl->bw == 0);
 
-   uint32_t min_row_pitch = bs * (phys_slice0_sa->w / fmtl->bw);
-   min_row_pitch = MAX2(min_row_pitch, info->min_pitch);
-   min_row_pitch = isl_align_npot(min_row_pitch, alignment);
-
-   return min_row_pitch;
+   return isl_align_npot(bs * (phys_slice0_sa->w / fmtl->bw), alignment);
 }
 
 static uint32_t
@@ -1068,11 +1064,8 @@ isl_calc_tiled_min_row_pitch(const struct isl_device 
*dev,
   isl_align_div(total_w_el * tile_el_scale,
 tile_info->logical_extent_el.width);
 
-   uint32_t min_row_pitch = total_w_tl * tile_info->phys_extent_B.width;
-   min_row_pitch = MAX2(min_row_pitch, surf_info->min_pitch);
-   min_row_pitch = isl_align_npot(min_row_pitch, alignment);
-
-   return min_row_pitch;
+   assert(alignment == tile_info->phys_extent_B.width);
+   return total_w_tl * tile_info->phys_extent_B.width;
 }
 
 static uint32_t
diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
index 012be7b98e7..17b52cf2f4f 100644
--- a/src/intel/isl/isl.h
+++ b/src/intel/isl/isl.h
@@ -810,9 +810,6 @@ struct isl_surf_init_info {
/** Lower bound for isl_surf::alignment, in bytes. */
uint32_t min_alignment;
 
-   /** Lower bound for isl_surf::pitch, in bytes. */
-   uint32_t min_pitch;
-
/**
 * Exact value for isl_surf::row_pitch. Ignored if zero.  isl_surf_init()
 * will fail if this is misaligned or out of bounds.
-- 
2.12.0

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


[Mesa-dev] [PATCH v5 3/5] isl: Let isl_surf_init's caller set the exact row pitch (v2)

2017-03-24 Thread Chad Versace
The caller does so by setting the new field
isl_surf_init_info::row_pitch.

v2: Validate the requested row_pitch.

Reviewed-by: Jason Ekstrand  (v2)
---
 src/intel/isl/isl.c | 14 +-
 src/intel/isl/isl.h |  6 ++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
index 4777113de84..1d5a9d85341 100644
--- a/src/intel/isl/isl.c
+++ b/src/intel/isl/isl.c
@@ -1118,10 +1118,22 @@ isl_calc_row_pitch(const struct isl_device *dev,
const uint32_t alignment =
   isl_calc_row_pitch_alignment(surf_info, tile_info);
 
-   const uint32_t row_pitch =
+   const uint32_t min_row_pitch =
   isl_calc_min_row_pitch(dev, surf_info, tile_info, phys_slice0_sa,
  alignment);
 
+   uint32_t row_pitch = min_row_pitch;
+
+   if (surf_info->row_pitch != 0) {
+  row_pitch = surf_info->row_pitch;
+
+  if (row_pitch < min_row_pitch)
+ return false;
+
+  if (row_pitch % alignment != 0)
+ return false;
+   }
+
const uint32_t row_pitch_tiles = row_pitch / tile_info->phys_extent_B.width;
 
if (row_pitch == 0)
diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
index 9d92906ca71..012be7b98e7 100644
--- a/src/intel/isl/isl.h
+++ b/src/intel/isl/isl.h
@@ -813,6 +813,12 @@ struct isl_surf_init_info {
/** Lower bound for isl_surf::pitch, in bytes. */
uint32_t min_pitch;
 
+   /**
+* Exact value for isl_surf::row_pitch. Ignored if zero.  isl_surf_init()
+* will fail if this is misaligned or out of bounds.
+*/
+   uint32_t row_pitch;
+
isl_surf_usage_flags_t usage;
 
/** Flags that alter how ISL selects isl_surf::tiling.  */
-- 
2.12.0

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


[Mesa-dev] [PATCH v5 0/5] isl: Fix requests for exact row pitch

2017-03-24 Thread Chad Versace
All callers of isl_surf_init() that set 'min_pitch' wanted to
request an *exact* row pitch, as evidenced by nearby asserts, but isl
lacked API for doing so. This series fixes that by adding a field,
isl_surf_init_info::row_pitch.

This prepares for VK_MESAX_external_image_dma_buf, which requires
support for create VkImages with an exact, user-provided row pitch.

This patch series lives at:
git://git.kiwitree.net/~chadv/mesa
refs/tags/chadv/review/isl-request-exact-row-pitch-v05
gitweb: 
http://git.kiwitree.net/cgit/~chadv/mesa/tag/?h=chadv/review/isl-request-exact-row-pitch-v05

Testing:
  I ran dEQP-VK on Skylake.

dEQP-VK.memory.*
Test run totals:
Passed:1105/1115 (99.1%)
Failed:0/1115 (0.0%)
Not supported: 10/1115 (0.9%)
Warnings:  0/1115 (0.0%)

dEQP-VK.api.*
Test run totals:
Passed:18132/59782 (30.3%)
Failed:1/59782 (0.0%)
Not supported: 41645/59782 (69.7%)
Warnings:  4/59782 (0.0%)


  I also pushed this to my 'jenkins' branch. But I no longer know to
  view the Jenkins results, because they're behind the Intel firewall.


v2:
  - Validate the requested row pitch. This required more extensive
refactors in patch 2.

v3:
  - New generated genxml header genX_bits.h.
  - Fix the pitch validation. Inspect the surface's usage bits instead
of its tiling to determine the pitch constraints. Use the bitfield
sizes from genX_bits.h.

v4:
  - When generating genX_bits.h, parse the XML instead of scraping the
gen*_pack.h headers.  [for jekstrand]

v4.5:
  - Rewrite script with Mako by Dylan.
  - Big script rewrite by Jason.

v5:
  - Fix autotools out-of-tree build.
  - Fix Android build. Tested with git://github.com/android-ia/manifest.
  - Fixes and cleanups to generator script. See patch's version log.

Chad Versace (5):
  genxml: New generated header genX_bits.h (v5)
  isl: Validate the calculated row pitch (v4)
  isl: Let isl_surf_init's caller set the exact row pitch (v2)
  intel: Fix requests for exact surface row pitch (v2)
  isl: Drop unused isl_surf_init_info::min_pitch

 src/intel/Android.genxml.mk |   9 +-
 src/intel/Makefile.genxml.am|   6 +-
 src/intel/Makefile.sources  |   6 +-
 src/intel/blorp/blorp_blit.c|   8 +-
 src/intel/genxml/.gitignore |   1 +
 src/intel/genxml/gen_bits_header.py | 281 
 src/intel/isl/isl.c |  96 ++--
 src/intel/isl/isl.h |   7 +-
 src/intel/vulkan/anv_blorp.c|  29 ++--
 src/intel/vulkan/anv_image.c|   2 +-
 10 files changed, 406 insertions(+), 39 deletions(-)
 create mode 100644 src/intel/genxml/gen_bits_header.py

-- 
2.12.0

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


Re: [Mesa-dev] [PATCH] util/disk_cache: don't deadlock on premature EOF

2017-03-24 Thread Timothy Arceri

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


Re: [Mesa-dev] [PATCH] util/disk_cache: avoid making mesa subdir when cache dir is specified

2017-03-24 Thread Timothy Arceri
I'm not sure  this is a good idea. Adding an extra dir doesn't do much 
harm, on the other hand if MESA_GLSL_CACHE_DIR is not a newly create dir 
just for the cache we could run into problems.


I think I'd rather leave this as is.

On 25/03/17 10:06, Grazvydas Ignotas wrote:

When MESA_GLSL_CACHE_DIR is specified, we currently add a /mesa subdir
even though the documentation and nearby comment in the code make no
mention of it. Doesn't look useful too, remove.

Signed-off-by: Grazvydas Ignotas 
---
 src/compiler/glsl/tests/cache_test.c | 2 +-
 src/util/disk_cache.c| 4 
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/compiler/glsl/tests/cache_test.c 
b/src/compiler/glsl/tests/cache_test.c
index 537a81b..4665026 100644
--- a/src/compiler/glsl/tests/cache_test.c
+++ b/src/compiler/glsl/tests/cache_test.c
@@ -200,11 +200,11 @@ test_disk_cache_create(void)

mkdir(CACHE_TEST_TMP, 0755);
cache = disk_cache_create("test", "make_check");
expect_non_null(cache, "disk_cache_create with MESA_GLSL_CACHE_DIR set");

-   check_directories_created(CACHE_TEST_TMP "/mesa-glsl-cache-dir/mesa");
+   check_directories_created(CACHE_TEST_TMP "/mesa-glsl-cache-dir");

disk_cache_destroy(cache);
 }

 static bool
diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
index a9a3e59..4f66aa9 100644
--- a/src/util/disk_cache.c
+++ b/src/util/disk_cache.c
@@ -195,14 +195,10 @@ disk_cache_create(const char *gpu_name, const char 
*timestamp)
 */
path = getenv("MESA_GLSL_CACHE_DIR");
if (path) {
   if (mkdir_if_needed(path) == -1)
  goto fail;
-
-  path = concatenate_and_mkdir(local, path, "mesa");
-  if (path == NULL)
- goto fail;
}

if (path == NULL) {
   char *xdg_cache_home = getenv("XDG_CACHE_HOME");



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


Re: [Mesa-dev] [PATCH] tests/cache_test: mark arguments const

2017-03-24 Thread Timothy Arceri

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


Re: [Mesa-dev] [PATCH resend] mesa: Require mipmap completeness for glCopyImageSubData(), sometimes.

2017-03-24 Thread Kenneth Graunke
On Friday, March 24, 2017 6:05:36 PM PDT Roland Scheidegger wrote:
> Am 24.03.2017 um 22:51 schrieb Kenneth Graunke:
> > This patch makes glCopyImageSubData require mipmap completeness when the
> > texture object's built-in sampler object has a mipmapping MinFilter.
> > 
> > Fixes (on i965):
> > dEQP-GLES31.functional.debug.negative_coverage.*.buffer.copy_image_sub_data
> > 
> > Signed-off-by: Kenneth Graunke 
> > ---
> >  src/mesa/main/copyimage.c | 25 +++--
> >  1 file changed, 23 insertions(+), 2 deletions(-)
> > 
> > Resending the same patch as before.  The Khronos OpenGL ARB and ES working
> > group both decided that although this behavior is unfortunate and strange,
> > Android's CTS requires it and so everybody else's implementations already
> > work this way.
> 
> Sounds like a rather lame justification IMHO - especially since it
> apparently breaks other CTS tests.

I agree...someone made the comment of "if we start allowing this, and
apps use it, then those apps won't run on Android devices", which,
amusingly, is the flipside of the normal portability argument: "if we
start disallowing this we might break existing applications".

> But ah well if everybody prefers simple hacks in their driver to make it
> conformant to a pointless error behavior, so be it.

Yeah.  *shrug*.  There are a lot of failures on the road here:

- non-immutable textures being a thing
- copyimage allowing non-immutable textures
- texture objects including a sampler object 
- default filtering for integer textures making them incomplete...

I could go on, but...hindsight is 20/20, I suppose.

> Reviewed-by: Roland Scheidegger 

Thanks!


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] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Dylan Baker
Quoting Jose Fonseca (2017-03-24 14:16:13)
> 
> Evaluating is one thing.  Actually migrating is another.
> 
> Brian already said he'd take a look and evaluate.  And I'll help in what 
> I can.  I agree we should all evaluate early.
> 
> 
> But I don't think that the proposal of first migrate scons to meson, 
> then in a second separate step migrate autotools to meson, is viable. 
> Like I said: there's no knowledge overlap.  The two group of people -- 
> the Meson and Windows experts -- will be chasing each other tails.  And 
> all that while, the build will continue to be broken or diverge because 
> master dev will go on.
> 
> 
> Jose

https://github.com/dcbaker/mesa-demos wip/meson

I've blindly ported some of the windows bits but have no way to test them, so
you can either delete the lot and go from scratch or see what's left to fix (the
wgl folder, for example). I have not implemented much of the windows or apple
logic in the root CMakeLists.txt, so hopefully that's useful for your purposes.

That branch also builds on my Archlinux machine, but not on debian due to
difference in the way they package freeglut I just ran out of time today. For
the record, I started at about 12:00, and finished at about 17:00 with a 1 hour
lunch in there. So about 3 hours to get a mostly working build. I'm going to try
to iron out the debian and travis issues either over the weekend or next week.

There is one difference, because ninja is non-recursive some targets would have
the same name and collide, so I've renamed some of the not installed binaries. I
believe that a non-recursive make (such as one generated by cmake) would have
the same problem. meson doesn't seem to have a method to rename the target, but
it's also a bit of an odd corner to build multiple binaries with the same name
that are both not installed and are for people (not automated build steps) to
use.

I also have a not quite working .travis.yml on that branch.

I'm also planning to get a mesa RFC sent out early next week once I get i965 and
llvmpipe building.

If we merge mesa we (Intel) will move to using meson as our primary build system
in CI (the one we run tests against) as soon as it's ready. Building mesa is
quite slow for us considering the power of our build hardware, and meson should
help with that. We'll continue to build autotools much the way we do scons now,
as a secondary "buildtest" only target.

Dylan

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


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


Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Kenneth Graunke
On Thursday, March 23, 2017 4:39:50 AM PDT Emil Velikov wrote:
> On 22 March 2017 at 20:10, Dylan Baker  wrote:
[snip]
> The more frustrating part is that atm autotools build is "bug-free"
> and with meson will have to go through the same route again :-\

"Bug-free" - famous last words :)

It is definitely working a lot better than it used to.  I'm grateful
to those who helped make it so (yourself included).

> > For my part, it took me about 3 or 4 days of reading through the docs and
> > writing the libdrm port to get it right, and a lot of that is just the
> > boilerplate of having ~8 drivers that all need basically the same logic.
> >
> Slightly off-topic - 3 days to write the build script for ~10 [nearly]
> identical libraries which do not do anything fancy, seems a lot.
> Which was the most time consuming part ?

As I believe Dylan explained...a lot of his time was spent learning
autotools and its idioms, so he knew how best to translate things.
(Dylan is pretty familiar with CMake, but less so with automake.)
Eric was able to work much more quickly, being already familiar with
the existing build system.

> I'm concerned that we would have to enforce the same time penalty onto
> dozens of developers unfamiliar with meson.
> 
> Thanks
> Emil

There's a time penalty figuring out any build system.  Most people try
to remain blissfully unaware of it as much as possible.  And then, most
tasks people do are pretty simple (adding files, deleting files, etc).
When something complex comes up...it takes time, reading, and sometimes
asking for help...no matter what system you use.

--Ken


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 resend] mesa: Require mipmap completeness for glCopyImageSubData(), sometimes.

2017-03-24 Thread Roland Scheidegger
Am 24.03.2017 um 22:51 schrieb Kenneth Graunke:
> This patch makes glCopyImageSubData require mipmap completeness when the
> texture object's built-in sampler object has a mipmapping MinFilter.
> 
> Fixes (on i965):
> dEQP-GLES31.functional.debug.negative_coverage.*.buffer.copy_image_sub_data
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/main/copyimage.c | 25 +++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> Resending the same patch as before.  The Khronos OpenGL ARB and ES working
> group both decided that although this behavior is unfortunate and strange,
> Android's CTS requires it and so everybody else's implementations already
> work this way.

Sounds like a rather lame justification IMHO - especially since it
apparently breaks other CTS tests.
But ah well if everybody prefers simple hacks in their driver to make it
conformant to a pointless error behavior, so be it.

Reviewed-by: Roland Scheidegger 


> 
> This patch will break several Piglit tests.  You need these patches:
> 
> 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.freedesktop.org_patch_146384_=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0=ddCbljDQbqf2DE2Xfva-mHfiLudX_KuJxe_fLv1EgUE=tazFdPEIHji-rjtw6QZMUAwwD_4DXrcdpWl7Bpt14Ow=
>  
> 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.freedesktop.org_patch_146385_=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0=ddCbljDQbqf2DE2Xfva-mHfiLudX_KuJxe_fLv1EgUE=PXI9ONdY5AAYE1I-7Q-hWgGnFFr0C4scZjHwQsi3IrM=
>  
> 
> This patch will also break several GL 4.5 CTS tests.  You need this patch:
> 
> 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__gerrit.khronos.org_849=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0=ddCbljDQbqf2DE2Xfva-mHfiLudX_KuJxe_fLv1EgUE=g6bgMSK16HJnWNG5KqK_Watai5veMMjUPecpPLTOXJY=
>  
> 
> Or for those without Khronos access:
> 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__whitecape.org_paste_0001-2DSet-2Dnearest-2Dfiltering-2Din-2DGL-2DCopyImage-2Dtests.patch=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0=ddCbljDQbqf2DE2Xfva-mHfiLudX_KuJxe_fLv1EgUE=rfg2sx4KrgcBqZO944MVa8YmdTKJkeQWqe9iBfSo1oY=
>  
> 
> diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
> index cf25159e880..877c8ac246d 100644
> --- a/src/mesa/main/copyimage.c
> +++ b/src/mesa/main/copyimage.c
> @@ -149,9 +149,30 @@ prepare_target(struct gl_context *ctx, GLuint name, 
> GLenum target,
>   return false;
>}
>  
> +  /* The ARB_copy_image specification says:
> +   *
> +   *"INVALID_OPERATION is generated if either object is a texture and
> +   * the texture is not complete (as defined in section 3.9.14)"
> +   *
> +   * The cited section says:
> +   *
> +   *"Using the preceding definitions, a texture is complete unless 
> any
> +   * of the following conditions hold true: [...]
> +   *
> +   * * The minification filter requires a mipmap (is neither NEAREST
> +   *   nor LINEAR), and the texture is not mipmap complete."
> +   *
> +   * This imposes the bizarre restriction that glCopyImageSubData 
> requires
> +   * mipmap completion at times, which dEQP mandates, and other drivers
> +   * appear to implement.  We don't have any texture units here, so we
> +   * can't look at any bound separate sampler objects...it appears that
> +   * you're supposed to use the sampler object which is built-in to the
> +   * texture object.
> +   *
> +   * See 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__cvs.khronos.org_bugzilla_show-5Fbug.cgi-3Fid-3D16224=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0=ddCbljDQbqf2DE2Xfva-mHfiLudX_KuJxe_fLv1EgUE=vt4M9kQ3cwPeQwM-mZxfXsc1qDZfUR2jKf5RweuJLwY=
>  .
> +   */
>_mesa_test_texobj_completeness(ctx, texObj);
> -  if (!texObj->_BaseComplete ||
> -  (level != 0 && !texObj->_MipmapComplete)) {
> +  if (!_mesa_is_texture_complete(texObj, >Sampler)) {
>   _mesa_error(ctx, GL_INVALID_OPERATION,
>   "glCopyImageSubData(%sName incomplete)", dbg_prefix);
>   return false;
> 

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


Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Juan A. Suarez Romero
On Fri, 2017-03-24 at 10:13 -0700, Dylan Baker wrote:
> Quoting Jose Fonseca (2017-03-24 06:42:18)
> > 
> > I tend to disagree.  While we can't avoid a transitory period, when we 
> > embark on another build system (Meson or something else) I think we 
> > should aim at 1) ensure such tool can indeed _completely_ replace at 
> > least _one_ existing build system, 2) and aim at migration quickly.
> > 
> > Otherwise we'll just end up with yet another build system, yet another 
> > way builds can fail, with some developers stuck on old build systems 
> > because it works, or because the new build system quite doesn't work.
> > 
> > And this is from (painful) experience.
> 
> I tend to agree. Meson is *nice* because it's faster than autotools, but it's
> simplicity and the fact that it works for windows and *nix systems is one of 
> the
> best features. Having fewer build systems is better than more.
> 
> We had hoped that we could do one release with both autotools and meson, to 
> give
> some of the fast moving linux distributions (Arch, Fedora, etc) a chance to 
> help
> us iron out bugs, especially for pacakgers. I think it is important though to
> make a commitment for exactly when we're going to either migrate completely to
> meson, or abandon the attempt and revert it.
> 
> > So I think we should identify stake holders soon, collect requirements 
> > (OSes platforms, etc), make sure the prospective tool meets them, have 
> > all stakeholders collaborate on a prototype, them embark on mass migration.
> > 
> > That is, if this fails, let it fail early.  If it succeeds, may it 
> > succeed early.  Anything but a slow death / zombie life.
> 
> I have a branch that builds intel's Vulkan driver, I'm actively working to get
> intel's i965 dri driver and llvmpipe building and send that out as an RFC to
> mesa-dev. That should give us enough of mesa to evaluate the build system I
> hope (Since it touches all of the major mesa components [classic, gallium,
> neither]).
> 
> If other people are interested in collaborating I'd be happy to push the 
> branch
> sooner so that others can look at it.
> 
> I also think it's worth talking to Eric (who said he's porting X to meson),
> Daniel Stone (who has patches to port weston to meson), and Peter Hutterer 
> (who
> has patches to port libinput to meson). If they're serious about seeing those
> land meson is even more appealing, since it would be a single build system 
> that
> all of the *nix graphics stack would be moving towards, and would mean that we
> wouldn't have an "Autotools for xorg", "meson for weston and libinput", 
> "cmake for
> piglit", and " for mesa".
> 

Some months ago I've also ported a module used in GNOME (grilo) to
Meson.

Faster-than-light when building is awesome. But the feature I can't
stress enough is how much simple and intuitive is working with Meson.
Everything looks quite more natural. And authors are very supportive.


> > BTW, how about migrating mesademos to Meson?  It currently has autotools 
> > and cmake.  I was hoping that cmake would replace autotools, but I 
> > couldn't run fast enough, so I couldn't practice what preached above, 
> > hence cmake doing almost but not all what autotools does.
> > 
> > And is not a crucial project for Linux distros -- few distros package it 
> > -- and even if they do, no other package would depend on it.  And is one 
> > of those sort of projects that should be easy to port to any build too.
> 
> That's definitely doable, but most distros do package it, it's where glxgears,
> and more importantly glxinfo live.
> 
> I'll have a look at it and see. At the very least we should be able to drop
> cmake since I very much doubt anyone but you guys use it.
> 
> > Even if we ignore everything else, just replacing autotools + cmake with 
> > just one thing would be a net win.
> > 
> > 
> > Jose
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 100091] Failure to create folder for on-disk shader cache

2017-03-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=100091

John  changed:

   What|Removed |Added

 Status|RESOLVED|CLOSED

--- Comment #30 from John  ---
That seems to be working fine.

Thanks to Timothy, Grazvydas and all others involved on fixing this!

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


[Mesa-dev] [PATCH] tests/cache_test: allow crossing mount points

2017-03-24 Thread Juan A. Suarez Romero
When using an overlayfs system (like a Docker container), rmrf_local()
fails because part of the files to be removed are in different mount
points (layouts). And thus cache-test fails.

Letting crossing mount points is not a big problem, specially because
this is just for a test, not to be used in real code.
---
 src/compiler/glsl/tests/cache_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/tests/cache_test.c 
b/src/compiler/glsl/tests/cache_test.c
index 2302f44..ac60197 100644
--- a/src/compiler/glsl/tests/cache_test.c
+++ b/src/compiler/glsl/tests/cache_test.c
@@ -124,7 +124,7 @@ rmrf_local(const char *path)
if (path == NULL || *path == '\0' || *path != '.')
   return -1;
 
-   return nftw(path, remove_entry, 64, FTW_DEPTH | FTW_PHYS | FTW_MOUNT);
+   return nftw(path, remove_entry, 64, FTW_DEPTH | FTW_PHYS);
 }
 
 static void
-- 
2.9.3

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


[Mesa-dev] [PATCH 3/3] radeonsi: use DMA for clears with unaligned size

2017-03-24 Thread Nicolai Hähnle
From: Nicolai Hähnle 

Only a small tail needs to be uploaded manually.

This is only partly a performance measure (apps are expected to use
aligned access). Mostly it is preparation for sparse buffers, which the
old code would incorrectly have attempted to map directly.
---
 src/gallium/drivers/radeonsi/si_cp_dma.c | 46 +++-
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_cp_dma.c 
b/src/gallium/drivers/radeonsi/si_cp_dma.c
index 0cf7b3b..812fcbc 100644
--- a/src/gallium/drivers/radeonsi/si_cp_dma.c
+++ b/src/gallium/drivers/radeonsi/si_cp_dma.c
@@ -178,87 +178,95 @@ static void si_cp_dma_prepare(struct si_context *sctx, 
struct pipe_resource *dst
 
 static void si_clear_buffer(struct pipe_context *ctx, struct pipe_resource 
*dst,
uint64_t offset, uint64_t size, unsigned value,
enum r600_coherency coher)
 {
struct si_context *sctx = (struct si_context*)ctx;
struct radeon_winsys *ws = sctx->b.ws;
struct r600_resource *rdst = r600_resource(dst);
unsigned tc_l2_flag = get_tc_l2_flag(sctx, coher);
unsigned flush_flags = get_flush_flags(sctx, coher);
+   uint64_t dma_clear_size;
bool is_first = true;
 
if (!size)
return;
 
+   dma_clear_size = size & ~3llu;
+
/* Mark the buffer range of destination as valid (initialized),
 * so that transfer_map knows it should wait for the GPU when mapping
 * that range. */
util_range_add(>valid_buffer_range, offset,
-  offset + size);
-
-   /* Fallback for unaligned clears. */
-   if (size % 4 != 0) {
-   uint8_t *map = r600_buffer_map_sync_with_rings(>b, rdst,
-  
PIPE_TRANSFER_WRITE);
-   map += offset;
-   for (uint64_t i = 0; i < size; i++) {
-   unsigned byte_within_dword = (offset + i) % 4;
-   *map++ = (value >> (byte_within_dword * 8)) & 0xff;
-   }
-   return;
-   }
+  offset + dma_clear_size);
 
/* dma_clear_buffer can use clear_buffer on failure. Make sure that
 * doesn't happen. We don't want an infinite recursion: */
if (sctx->b.dma.cs &&
(offset % 4 == 0) &&
/* CP DMA is very slow. Always use SDMA for big clears. This
 * alone improves DeusEx:MD performance by 70%. */
(size > 128 * 1024 ||
 /* Buffers not used by the GFX IB yet will be cleared by SDMA.
  * This happens to move most buffer clears to SDMA, including
  * DCC and CMASK clears, because pipe->clear clears them before
  * si_emit_framebuffer_state (in a draw call) adds them.
  * For example, DeusEx:MD has 21 buffer clears per frame and all
  * of them are moved to SDMA thanks to this. */
 !ws->cs_is_buffer_referenced(sctx->b.gfx.cs, rdst->buf,
  RADEON_USAGE_READWRITE))) {
-   sctx->b.dma_clear_buffer(ctx, dst, offset, size, value);
-   } else {
+   sctx->b.dma_clear_buffer(ctx, dst, offset, dma_clear_size, 
value);
+
+   offset += dma_clear_size;
+   size -= dma_clear_size;
+   } else if (dma_clear_size >= 4) {
uint64_t va = rdst->gpu_address + offset;
 
+   offset += dma_clear_size;
+   size -= dma_clear_size;
+
/* Flush the caches. */
sctx->b.flags |= SI_CONTEXT_PS_PARTIAL_FLUSH |
 SI_CONTEXT_CS_PARTIAL_FLUSH | flush_flags;
 
-   while (size) {
-   unsigned byte_count = MIN2(size, CP_DMA_MAX_BYTE_COUNT);
+   while (dma_clear_size) {
+   unsigned byte_count = MIN2(dma_clear_size, 
CP_DMA_MAX_BYTE_COUNT);
unsigned dma_flags = tc_l2_flag  | CP_DMA_CLEAR;
 
-   si_cp_dma_prepare(sctx, dst, NULL, byte_count, size, 0,
+   si_cp_dma_prepare(sctx, dst, NULL, byte_count, 
dma_clear_size, 0,
  _first, _flags);
 
/* Emit the clear packet. */
si_emit_cp_dma(sctx, va, value, byte_count, dma_flags, 
coher);
 
-   size -= byte_count;
+   dma_clear_size -= byte_count;
va += byte_count;
}
 
if (tc_l2_flag)
rdst->TC_L2_dirty = true;
 
/* If it's not a framebuffer fast clear... */
if (coher == R600_COHERENCY_SHADER)
sctx->b.num_cp_dma_calls++;
}
+
+   if (size) {
+   /* Handle non-dword 

[Mesa-dev] [PATCH 1/3] radeonsi: remove the early-out for SDMA in si_clear_buffer

2017-03-24 Thread Nicolai Hähnle
From: Nicolai Hähnle 

This allows the next patches to be simple while still being able
to make use of SDMA even in some unusual cases.
---
 src/gallium/drivers/radeonsi/si_cp_dma.c | 43 
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_cp_dma.c 
b/src/gallium/drivers/radeonsi/si_cp_dma.c
index 1be7586..b40f5cc 100644
--- a/src/gallium/drivers/radeonsi/si_cp_dma.c
+++ b/src/gallium/drivers/radeonsi/si_cp_dma.c
@@ -216,49 +216,48 @@ static void si_clear_buffer(struct pipe_context *ctx, 
struct pipe_resource *dst,
(size > 128 * 1024 ||
 /* Buffers not used by the GFX IB yet will be cleared by SDMA.
  * This happens to move most buffer clears to SDMA, including
  * DCC and CMASK clears, because pipe->clear clears them before
  * si_emit_framebuffer_state (in a draw call) adds them.
  * For example, DeusEx:MD has 21 buffer clears per frame and all
  * of them are moved to SDMA thanks to this. */
 !ws->cs_is_buffer_referenced(sctx->b.gfx.cs, rdst->buf,
  RADEON_USAGE_READWRITE))) {
sctx->b.dma_clear_buffer(ctx, dst, offset, size, value);
-   return;
-   }
-
-   uint64_t va = rdst->gpu_address + offset;
+   } else {
+   uint64_t va = rdst->gpu_address + offset;
 
-   /* Flush the caches. */
-   sctx->b.flags |= SI_CONTEXT_PS_PARTIAL_FLUSH |
-SI_CONTEXT_CS_PARTIAL_FLUSH | flush_flags;
+   /* Flush the caches. */
+   sctx->b.flags |= SI_CONTEXT_PS_PARTIAL_FLUSH |
+SI_CONTEXT_CS_PARTIAL_FLUSH | flush_flags;
 
-   while (size) {
-   unsigned byte_count = MIN2(size, CP_DMA_MAX_BYTE_COUNT);
-   unsigned dma_flags = tc_l2_flag  | CP_DMA_CLEAR;
+   while (size) {
+   unsigned byte_count = MIN2(size, CP_DMA_MAX_BYTE_COUNT);
+   unsigned dma_flags = tc_l2_flag  | CP_DMA_CLEAR;
 
-   si_cp_dma_prepare(sctx, dst, NULL, byte_count, size, 0,
- _first, _flags);
+   si_cp_dma_prepare(sctx, dst, NULL, byte_count, size, 0,
+ _first, _flags);
 
-   /* Emit the clear packet. */
-   si_emit_cp_dma(sctx, va, value, byte_count, dma_flags, coher);
+   /* Emit the clear packet. */
+   si_emit_cp_dma(sctx, va, value, byte_count, dma_flags, 
coher);
 
-   size -= byte_count;
-   va += byte_count;
-   }
+   size -= byte_count;
+   va += byte_count;
+   }
 
-   if (tc_l2_flag)
-   rdst->TC_L2_dirty = true;
+   if (tc_l2_flag)
+   rdst->TC_L2_dirty = true;
 
-   /* If it's not a framebuffer fast clear... */
-   if (coher == R600_COHERENCY_SHADER)
-   sctx->b.num_cp_dma_calls++;
+   /* If it's not a framebuffer fast clear... */
+   if (coher == R600_COHERENCY_SHADER)
+   sctx->b.num_cp_dma_calls++;
+   }
 }
 
 /**
  * Realign the CP DMA engine. This must be done after a copy with an unaligned
  * size.
  *
  * \param size  Remaining size to the CP DMA alignment.
  */
 static void si_cp_dma_realign_engine(struct si_context *sctx, unsigned size,
 unsigned user_flags, bool *is_first)
-- 
2.9.3

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


[Mesa-dev] [PATCH 2/3] radeonsi: CP DMA clear supports unaligned destination addresses

2017-03-24 Thread Nicolai Hähnle
From: Nicolai Hähnle 

---
 src/gallium/drivers/radeonsi/si_cp_dma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeonsi/si_cp_dma.c 
b/src/gallium/drivers/radeonsi/si_cp_dma.c
index b40f5cc..0cf7b3b 100644
--- a/src/gallium/drivers/radeonsi/si_cp_dma.c
+++ b/src/gallium/drivers/radeonsi/si_cp_dma.c
@@ -190,34 +190,35 @@ static void si_clear_buffer(struct pipe_context *ctx, 
struct pipe_resource *dst,
if (!size)
return;
 
/* Mark the buffer range of destination as valid (initialized),
 * so that transfer_map knows it should wait for the GPU when mapping
 * that range. */
util_range_add(>valid_buffer_range, offset,
   offset + size);
 
/* Fallback for unaligned clears. */
-   if (offset % 4 != 0 || size % 4 != 0) {
+   if (size % 4 != 0) {
uint8_t *map = r600_buffer_map_sync_with_rings(>b, rdst,
   
PIPE_TRANSFER_WRITE);
map += offset;
for (uint64_t i = 0; i < size; i++) {
unsigned byte_within_dword = (offset + i) % 4;
*map++ = (value >> (byte_within_dword * 8)) & 0xff;
}
return;
}
 
/* dma_clear_buffer can use clear_buffer on failure. Make sure that
 * doesn't happen. We don't want an infinite recursion: */
if (sctx->b.dma.cs &&
+   (offset % 4 == 0) &&
/* CP DMA is very slow. Always use SDMA for big clears. This
 * alone improves DeusEx:MD performance by 70%. */
(size > 128 * 1024 ||
 /* Buffers not used by the GFX IB yet will be cleared by SDMA.
  * This happens to move most buffer clears to SDMA, including
  * DCC and CMASK clears, because pipe->clear clears them before
  * si_emit_framebuffer_state (in a draw call) adds them.
  * For example, DeusEx:MD has 21 buffer clears per frame and all
  * of them are moved to SDMA thanks to this. */
 !ws->cs_is_buffer_referenced(sctx->b.gfx.cs, rdst->buf,
-- 
2.9.3

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


[Mesa-dev] [PATCH] util/disk_cache: don't deadlock on premature EOF

2017-03-24 Thread Grazvydas Ignotas
If we get EOF earlier than expected, the current read loops will
deadlock. This may easily happen if the disk cache gets corrupted.
Fix it by using a helper function that handles EOF.

Steps to reproduce (on a build with asserts disabled):
$ glxgears
$ find ~/.cache/mesa/ -type f -exec truncate -s 0 '{}' \;
$ glxgears # deadlock

Signed-off-by: Grazvydas Ignotas 
---
 src/util/disk_cache.c | 43 ++-
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
index 4f66aa9..9677f93 100644
--- a/src/util/disk_cache.c
+++ b/src/util/disk_cache.c
@@ -636,10 +636,25 @@ disk_cache_remove(struct disk_cache *cache, const 
cache_key key)
if (sb.st_size)
   p_atomic_add(cache->size, - (uint64_t)sb.st_size);
 }
 
 static ssize_t
+read_all(int fd, void *buf, size_t count)
+{
+   char *in = buf;
+   ssize_t read_ret;
+   size_t done;
+
+   for (done = 0; done < count; done += read_ret) {
+  read_ret = read(fd, in + done, count - done);
+  if (read_ret == -1 || read_ret == 0)
+ return -1;
+   }
+   return done;
+}
+
+static ssize_t
 write_all(int fd, const void *buf, size_t count)
 {
const char *out = buf;
ssize_t written;
size_t done;
@@ -932,11 +947,11 @@ inflate_cache_data(uint8_t *in_data, size_t in_data_size,
 }
 
 void *
 disk_cache_get(struct disk_cache *cache, const cache_key key, size_t *size)
 {
-   int fd = -1, ret, len;
+   int fd = -1, ret;
struct stat sb;
char *filename = NULL;
uint8_t *data = NULL;
uint8_t *uncompressed_data = NULL;
 
@@ -963,16 +978,14 @@ disk_cache_get(struct disk_cache *cache, const cache_key 
key, size_t *size)
uint8_t *file_header = malloc(ck_size);
if (!file_header)
   goto fail;
 
assert(sb.st_size > ck_size);
-   for (len = 0; len < ck_size; len += ret) {
-  ret = read(fd, ((uint8_t *) file_header) + len, ck_size - len);
-  if (ret == -1) {
- free(file_header);
- goto fail;
-  }
+   ret = read_all(fd, file_header, ck_size);
+   if (ret == -1) {
+  free(file_header);
+  goto fail;
}
 
assert(memcmp(cache->driver_keys_blob, file_header, ck_size) == 0);
 
free(file_header);
@@ -986,23 +999,19 @@ disk_cache_get(struct disk_cache *cache, const cache_key 
key, size_t *size)
 #endif
 
/* Load the CRC that was created when the file was written. */
struct cache_entry_file_data cf_data;
size_t cf_data_size = sizeof(cf_data);
-   for (len = 0; len < cf_data_size; len += ret) {
-  ret = read(fd, ((uint8_t *) _data) + len, cf_data_size - len);
-  if (ret == -1)
- goto fail;
-   }
+   ret = read_all(fd, _data, cf_data_size);
+   if (ret == -1)
+  goto fail;
 
/* Load the actual cache data. */
size_t cache_data_size = sb.st_size - cf_data_size - ck_size;
-   for (len = 0; len < cache_data_size; len += ret) {
-  ret = read(fd, data + len, cache_data_size - len);
-  if (ret == -1)
- goto fail;
-   }
+   ret = read_all(fd, data, cache_data_size);
+   if (ret == -1)
+  goto fail;
 
/* Uncompress the cache data */
uncompressed_data = malloc(cf_data.uncompressed_size);
if (!inflate_cache_data(data, cache_data_size, uncompressed_data,
cf_data.uncompressed_size))
-- 
2.7.4

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


Re: [Mesa-dev] [RFC 11/12] anv/cmd_buffer: Emit instanced draws for multiple views

2017-03-24 Thread Jason Ekstrand
On Fri, Mar 24, 2017 at 5:53 AM, Iago Toral  wrote:

> On Wed, 2017-03-22 at 21:01 -0700, Jason Ekstrand wrote:
> ---
>  src/intel/vulkan/anv_private.h |   6 ++
>  src/intel/vulkan/genX_cmd_buffer.c | 141
> -
>  src/intel/vulkan/genX_pipeline.c   |  10 ++-
>  3 files changed, 152 insertions(+), 5 deletions(-)
>
>
> diff --git a/src/intel/vulkan/anv_private.h
> b/src/intel/vulkan/anv_private.h
> index b402bc3..253dce2 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -1980,6 +1980,12 @@ struct anv_subpass {
> bool has_resolve;
>  };
>
> +static inline unsigned
> +anv_subpass_view_count(const struct anv_subpass *subpass)
> +{
> +   return MAX2(1, _mesa_bitcount(subpass->view_mask));
> +}
> +
>  enum anv_subpass_usage {
> ANV_SUBPASS_USAGE_DRAW = (1 << 0),
> ANV_SUBPASS_USAGE_INPUT =(1 << 1),
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index aafe7fd..8c21c33 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -26,6 +26,7 @@
>
>  #include "anv_private.h"
>  #include "vk_format_info.h"
> +#include "util/vk_util.h"
>
>  #include "common/gen_l3_config.h"
>  #include "genxml/gen_macros.h"
> @@ -50,6 +51,17 @@ emit_lri(struct anv_batch *batch, uint32_t reg,
> uint32_t imm)
> }
>  }
>
> +#if GEN_IS_HASWELL || GEN_GEN >= 8
> +static void
> +emit_lrr(struct anv_batch *batch, uint32_t dst, uint32_t src)
> +{
> +   anv_batch_emit(batch, GENX(MI_LOAD_REGISTER_REG), lrr) {
> +  lrr.SourceRegisterAddress= src;
> +  lrr.DestinationRegisterAddress   = dst;
> +   }
> +}
> +#endif
> +
>  void
>  genX(cmd_buffer_emit_state_base_address)(struct anv_cmd_buffer
> *cmd_buffer)
>  {
> @@ -1525,7 +1537,13 @@ genX(cmd_buffer_flush_state)(struct
> anv_cmd_buffer *cmd_buffer)
>  .MemoryObjectControlState = GENX(MOCS),
>  #else
>  .BufferAccessType = pipeline->instancing_enable[vb] ?
> INSTANCEDATA : VERTEXDATA,
> -.InstanceDataStepRate = 1,
> +/* Our implementation of VK_KHR_multiview uses
> instancing to draw
> + * the different views.  If the client asks for
> instancing, we
> + * need to use the Instance Data Step Rate to ensure
> that we
> + * repeat the client's per-instance data once for each
> view.
> + */
> +.InstanceDataStepRate =
> +   _mesa_bitcount(pipeline->subpass->view_mask),
>
> mmm... shouldn't this be:
>
> .InstanceDataStepRate = anv_subpass_view_count(pipeline->subpass);
>
> so that we set this to 1 when multiview is not in use? (view_mask == 0)
>

Good call!  You're absolutely right.  This line is older than
anv_subpass_view_count, I think.


>
>  .VertexBufferMemoryObjectControlState = GENX(MOCS),
>  #endif
>
> @@ -1715,6 +1733,11 @@ void genX(CmdDraw)(
> if (vs_prog_data->uses_drawid)
>emit_draw_index(cmd_buffer, 0);
>
> +   /* Our implementation of VK_KHR_multiview uses instancing to draw
> the
> +* different views.  We need to multiply instanceCount by the
> view count.
> +*/
> +   instanceCount *= anv_subpass_view_count(cmd_buffer-
> >state.subpass);
> +
> anv_batch_emit(_buffer->batch, GENX(3DPRIMITIVE), prim) {
>prim.VertexAccessType = SEQUENTIAL;
>prim.PrimitiveTopologyType= pipeline->topology;
> @@ -1748,6 +1771,11 @@ void genX(CmdDrawIndexed)(
> if (vs_prog_data->uses_drawid)
>emit_draw_index(cmd_buffer, 0);
>
> +   /* Our implementation of VK_KHR_multiview uses instancing to draw
> the
> +* different views.  We need to multiply instanceCount by the
> view count.
> +*/
> +   instanceCount *= anv_subpass_view_count(cmd_buffer-
> >state.subpass);
> +
> anv_batch_emit(_buffer->batch, GENX(3DPRIMITIVE), prim) {
>prim.VertexAccessType = RANDOM;
>prim.PrimitiveTopologyType= pipeline->topology;
> @@ -1767,6 +1795,90 @@ void genX(CmdDrawIndexed)(
>  #define GEN7_3DPRIM_START_INSTANCE  0x243C
>  #define GEN7_3DPRIM_BASE_VERTEX 0x2440
>
> +/* MI_MATH only exists on Haswell+ */
> +#if GEN_IS_HASWELL || GEN_GEN >= 8
> +
> +#define alu_opcode(v)   __gen_uint((v),  20, 31)
> +#define alu_operand1(v) __gen_uint((v),  10, 19)
> +#define alu_operand2(v) __gen_uint((v),   0,  9)
> +#define alu(opcode, operand1, operand2) \
> +   alu_opcode(opcode) | alu_operand1(operand1) |
> alu_operand2(operand2)
> +
> +#define OPCODE_NOOP  0x000
> +#define OPCODE_LOAD  0x080
> +#define OPCODE_LOADINV   0x480
> +#define OPCODE_LOAD0 0x081
> +#define OPCODE_LOAD1 0x481
> +#define OPCODE_ADD   0x100
> +#define OPCODE_SUB   0x101
> +#define OPCODE_AND   0x102
> +#define OPCODE_OR0x103
> +#define OPCODE_XOR   0x104
> +#define OPCODE_STORE 0x180
> +#define OPCODE_STOREINV  0x580
> +

Re: [Mesa-dev] [PATCH] anv/cmd_buffer: Apply flush operations prior to executing secondaries

2017-03-24 Thread Jason Ekstrand
This apparently fixes rendering corruptions on the Vulkan port of Skia on
some hardware.

On Fri, Mar 24, 2017 at 4:31 PM, Jason Ekstrand 
wrote:

> Cc: "13.0 17.0" 
> ---
>  src/intel/vulkan/genX_cmd_buffer.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index e2364db..8ec882e 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -654,6 +654,11 @@ genX(CmdExecuteCommands)(
>  */
> genX(cmd_buffer_enable_pma_fix)(primary, false);
>
> +   /* The secondary command buffer doesn't know which textures etc. have
> been
> +* flushed prior to their execution.  Apply those flushes now.
> +*/
> +   genX(cmd_buffer_apply_pipe_flushes)(primary);
> +
> for (uint32_t i = 0; i < commandBufferCount; i++) {
>ANV_FROM_HANDLE(anv_cmd_buffer, secondary, pCmdBuffers[i]);
>
> --
> 2.5.0.400.gff86faf
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] anv/cmd_buffer: Apply flush operations prior to executing secondaries

2017-03-24 Thread Jason Ekstrand
Cc: "13.0 17.0" 
---
 src/intel/vulkan/genX_cmd_buffer.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/intel/vulkan/genX_cmd_buffer.c 
b/src/intel/vulkan/genX_cmd_buffer.c
index e2364db..8ec882e 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -654,6 +654,11 @@ genX(CmdExecuteCommands)(
 */
genX(cmd_buffer_enable_pma_fix)(primary, false);
 
+   /* The secondary command buffer doesn't know which textures etc. have been
+* flushed prior to their execution.  Apply those flushes now.
+*/
+   genX(cmd_buffer_apply_pipe_flushes)(primary);
+
for (uint32_t i = 0; i < commandBufferCount; i++) {
   ANV_FROM_HANDLE(anv_cmd_buffer, secondary, pCmdBuffers[i]);
 
-- 
2.5.0.400.gff86faf

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


[Mesa-dev] [PATCH] util/disk_cache: avoid making mesa subdir when cache dir is specified

2017-03-24 Thread Grazvydas Ignotas
When MESA_GLSL_CACHE_DIR is specified, we currently add a /mesa subdir
even though the documentation and nearby comment in the code make no
mention of it. Doesn't look useful too, remove.

Signed-off-by: Grazvydas Ignotas 
---
 src/compiler/glsl/tests/cache_test.c | 2 +-
 src/util/disk_cache.c| 4 
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/compiler/glsl/tests/cache_test.c 
b/src/compiler/glsl/tests/cache_test.c
index 537a81b..4665026 100644
--- a/src/compiler/glsl/tests/cache_test.c
+++ b/src/compiler/glsl/tests/cache_test.c
@@ -200,11 +200,11 @@ test_disk_cache_create(void)
 
mkdir(CACHE_TEST_TMP, 0755);
cache = disk_cache_create("test", "make_check");
expect_non_null(cache, "disk_cache_create with MESA_GLSL_CACHE_DIR set");
 
-   check_directories_created(CACHE_TEST_TMP "/mesa-glsl-cache-dir/mesa");
+   check_directories_created(CACHE_TEST_TMP "/mesa-glsl-cache-dir");
 
disk_cache_destroy(cache);
 }
 
 static bool
diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
index a9a3e59..4f66aa9 100644
--- a/src/util/disk_cache.c
+++ b/src/util/disk_cache.c
@@ -195,14 +195,10 @@ disk_cache_create(const char *gpu_name, const char 
*timestamp)
 */
path = getenv("MESA_GLSL_CACHE_DIR");
if (path) {
   if (mkdir_if_needed(path) == -1)
  goto fail;
-
-  path = concatenate_and_mkdir(local, path, "mesa");
-  if (path == NULL)
- goto fail;
}
 
if (path == NULL) {
   char *xdg_cache_home = getenv("XDG_CACHE_HOME");
 
-- 
2.7.4

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


[Mesa-dev] [PATCH] tests/cache_test: mark arguments const

2017-03-24 Thread Grazvydas Ignotas
While at it, also fix up a failure message to not reference timestamp
and gpu dirs as those are no longer being made.

Signed-off-by: Grazvydas Ignotas 
---
 src/compiler/glsl/tests/cache_test.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/compiler/glsl/tests/cache_test.c 
b/src/compiler/glsl/tests/cache_test.c
index 2302f44..537a81b 100644
--- a/src/compiler/glsl/tests/cache_test.c
+++ b/src/compiler/glsl/tests/cache_test.c
@@ -126,11 +126,11 @@ rmrf_local(const char *path)
 
return nftw(path, remove_entry, 64, FTW_DEPTH | FTW_PHYS | FTW_MOUNT);
 }
 
 static void
-check_directories_created(char *cache_dir)
+check_directories_created(const char *cache_dir)
 {
bool sub_dirs_created = false;
 
char buf[PATH_MAX];
if (getcwd(buf, PATH_MAX)) {
@@ -142,11 +142,11 @@ check_directories_created(char *cache_dir)
 
  free(full_path);
   }
}
 
-   expect_true(sub_dirs_created, "create timestamp and gpu ip sub dirs");
+   expect_true(sub_dirs_created, "create sub dirs");
 }
 
 #define CACHE_TEST_TMP "./cache-test-tmp"
 
 static void
@@ -206,11 +206,11 @@ test_disk_cache_create(void)
 
disk_cache_destroy(cache);
 }
 
 static bool
-does_cache_contain(struct disk_cache *cache, cache_key key)
+does_cache_contain(struct disk_cache *cache, const cache_key key)
 {
void *result;
 
result = disk_cache_get(cache, key, NULL);
 
@@ -221,11 +221,11 @@ does_cache_contain(struct disk_cache *cache, cache_key 
key)
 
return false;
 }
 
 static void
-wait_until_file_written(struct disk_cache *cache, cache_key key)
+wait_until_file_written(struct disk_cache *cache, const cache_key key)
 {
struct timespec req;
struct timespec rem;
 
/* Set 100ms delay */
-- 
2.7.4

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


Re: [Mesa-dev] Mesa (master): mesa/marshal: add custom BufferData/ BufferSubData marshalling

2017-03-24 Thread Timothy Arceri



On 25/03/17 02:49, Brian Paul wrote:

Hi Timothy,

Some late comments below.

On 03/23/2017 06:28 PM, Timothy Arceri wrote:

Module: Mesa
Branch: master
Commit: adced4a2f9d017ae126a438f97eb305fa0ca3bd0
URL:
https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_mesa_mesa_commit_-3Fid-3Dadced4a2f9d017ae126a438f97eb305fa0ca3bd0=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=Ie7_encNUsqxbSRbqbNgofw0ITcfE8JKfaUjIQhncGA=euA0jTcoAmQ2Zvy_d-Fv7gEc7REbDj19MbIRrYVfXoc=RM6-RjO08ktuyTV6AhhjU7t-LGPw-SgL78pFH2QXiQs=


Author: Timothy Arceri 
Date:   Thu Mar 23 17:19:36 2017 +1100

mesa/marshal: add custom BufferData/BufferSubData marshalling

GL_AMD_pinned_memory requires memory to be aligned correctly, so
we skip marshalling in this case. Also copying the data defeats
the purpose of EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD.

Fixes GL_AMD_pinned_memory piglit tests when glthread is enabled.

Acked-by: Edward O'Callaghan 

---

  src/mapi/glapi/gen/gl_API.xml |   4 +-
  src/mesa/main/marshal.c   | 125
++
  src/mesa/main/marshal.h   |  18 ++
  3 files changed, 145 insertions(+), 2 deletions(-)

diff --git a/src/mapi/glapi/gen/gl_API.xml
b/src/mapi/glapi/gen/gl_API.xml
index c1f0f8fe92..dfaeaafa03 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -5033,7 +5033,7 @@
  
  

-
+
  
  
  
@@ -5041,7 +5041,7 @@
  
  

-
+
  
  
  
diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
index f8cad30e20..cdc7fed8a5 100644
--- a/src/mesa/main/marshal.c
+++ b/src/mesa/main/marshal.c
@@ -259,4 +259,129 @@ _mesa_marshal_BindBuffer(GLenum target, GLuint
buffer)
 }
  }

+/* BufferData: marshalled asynchronously */
+struct marshal_cmd_BufferData
+{
+   struct marshal_cmd_base cmd_base;
+   GLenum target;
+   GLsizeiptr size;
+   GLenum usage;
+   bool data_null; /* If set, no data follows for "data" */
+   /* Next size bytes are GLvoid data[size] */
+};
+
+void
+_mesa_unmarshal_BufferData(struct gl_context *ctx,
+   const struct marshal_cmd_BufferData *cmd)
+{
+   const GLenum target = cmd->target;
+   const GLsizeiptr size = cmd->size;
+   const GLenum usage = cmd->usage;
+   const char *variable_data = (const char *) (cmd + 1);
+   const GLvoid *data = (const GLvoid *) variable_data;
+
+   if (cmd->data_null)
+  data = NULL;
+   else
+  variable_data += size;


I don't understand why this var is incremented if it's never used again.


+
+   CALL_BufferData(ctx->CurrentServerDispatch, (target, size, data,
usage));
+}
+
+void GLAPIENTRY
+_mesa_marshal_BufferData(GLenum target, GLsizeiptr size, const GLvoid
* data,
+ GLenum usage)
+{
+   GET_CURRENT_CONTEXT(ctx);
+   size_t cmd_size =
+  sizeof(struct marshal_cmd_BufferData) + (data ? size : 0);
+   debug_print_marshal("BufferData");
+
+   if (unlikely(size < 0)) {
+  _mesa_glthread_finish(ctx);
+  _mesa_error(ctx, GL_INVALID_VALUE, "BufferData(size < 0)");
+  return;
+   }
+
+   if (target != GL_EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD &&
+   cmd_size <= MARSHAL_MAX_CMD_SIZE) {
+  struct marshal_cmd_BufferData *cmd =
+ _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_BufferData,
+ cmd_size);
+
+  cmd->target = target;
+  cmd->size = size;
+  cmd->usage = usage;
+  char *variable_data = (char *) (cmd + 1);
+  cmd->data_null = !data;
+  if (!cmd->data_null) {
+ memcpy(variable_data, data, size);
+ variable_data += size;


Same here.


+  }
+  _mesa_post_marshal_hook(ctx);
+   } else {
+  _mesa_glthread_finish(ctx);
+  CALL_BufferData(ctx->CurrentServerDispatch,
+  (target, size, data, usage));
+   }
+}
+
+/* BufferSubData: marshalled asynchronously */
+struct marshal_cmd_BufferSubData
+{
+   struct marshal_cmd_base cmd_base;
+   GLenum target;
+   GLintptr offset;
+   GLsizeiptr size;
+   /* Next size bytes are GLvoid data[size] */
+};
+
+void
+_mesa_unmarshal_BufferSubData(struct gl_context *ctx,
+  const struct marshal_cmd_BufferSubData
*cmd)
+{
+   const GLenum target = cmd->target;
+   const GLintptr offset = cmd->offset;
+   const GLsizeiptr size = cmd->size;
+   const char *variable_data = (const char *) (cmd + 1);
+   const GLvoid *data = (const GLvoid *) variable_data;
+
+   variable_data += size;


And here, and below.


This code is mostly a copy from the generated code, which adds these for 
some reason (possibly a bug in generation, but it may be used by other 
functions and just excess here).




I'm going to post a patch which simplifies some of this code...


Thanks, will review. Sorry for the trouble, I noticed these while 
writing the patch but forgot to remove them.




-Brian




+   

Re: [Mesa-dev] MESA and KOTOR

2017-03-24 Thread Brian Paul
I'm going to re-post my recent wgl patches (verified to work now) for 
review before committing to master.


Looking at some other notes in our code, there's another issue with 
KOTOR.  Try setting the following env var:


MESA_EXTENSION_OVERRIDE=-GL_ATI_fragment_shader

The '-' means disable the GL_ATI_fragment_shader extension.

-Brian

On 03/16/2017 12:39 PM, Federico Dossena wrote:

I managed to fix the patch and apply it to mesa master, but I'm getting
the same result as with my stub. The crash is still the same, in
glu32.dll, I wonder if the GLU that you guys have in your repo will work
any better. I tried to crosscompile it but without luck, any instructions?

Still, I want to thank all of you for helping me out on this one. I've
been fixing old games for years but this one has always been my nemesis.


Il 2017-03-16 19:04, Brian Paul ha scritto:

Patch for implementing WGL_ARB_make_current_read attached.  I can’t test it at 
the moment since I’m not near my Windows development environment.  Let me know 
what you find.

-Brian





On Mar 15, 2017, at 12:26 PM, Federico Dossena  wrote:

That's good, can't wait to see your implementation.

I have tried to simply return wglMakeCurrent(hReadDC,hglrc); but then I get a 
crash in gluBuild2DMipmaps (not mesa, glu32.dll). According to the 
specification, it should work, or at least draw some glitches.
Looking at the parameters passed by the game to wglMakeContextCurrentARB, I see 
that hReadDC and hDrawDC are the same so I guess they intended to use it as a 
replacement for wglMakeCurrent, but still, it's not working. So, does 
wglMakeContextCurrentARB do something else in addition to that?


Il 2017-03-15 15:50, Jose Fonseca ha scritto:

VMware maintains a Windows OpenGL driver based off Mesa source.

We typically open source most of our modifications, but these haven't been yet 
open sourced.  No particular reason I believe. We've been just busy with other 
stuff.

The simplest shim would be to invoke wglMakeCurrent from 
wglMakeContextCurrentARB, ignoring exttra arg.


Jose

On 15/03/17 14:31, Federico Dossena wrote:

Where can I find that implementation?

Also, is there an alternative to that function? As in a snippet of code
that does the same thing and can be used to create a "shim"?
It's so old, I can barely find documentation about it...

On March 15, 2017 2:42:35 PM GMT+01:00, Jose Fonseca
  wrote:

It looks like wglMakeContextCurrentARB too has been implemented
internally but not yet crossported.

It's far from trivial (especially because Microsoft ICD interface never
was designed to allow implementations to provide alternative
imlpementations of functions like wglMakeCurrent or wglCreateContext)
though in the way you're using it, it's less important, as the original
opengl32.dll is never used.

I don't know how much effort / time it takes to crossport this and other
outstanding patches to master, but my guess is that it would be more
effective to wait a bit.

Jose

On 15/03/17 06:35, Federico Dossena wrote:

I have created a simple stub for wglMakeContextCurrentARB in
stw_wgl.c
and stw_getprocaddress.c. It simply returns TRUE, but the good
thing is
that now the game no longer crashes because the function is missing!
However I get a divide by zero in glu32.dll, presumably because
the stub
doesn't do jack.
I tried returning FALSE but the game has no fallback, it just
ignores
the return values and assumes that everything is fine.

From what I've seen, there is no need to override the system's
opengl32.dll like you did for wglCreateContext/wglDeleteContext,
so it
shouldn't be too tricky to implement the function. However, I
can't seem
to find any real documentation about what it's supposed to do. I
found
this at
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.khronos.org_registry_OpenGL_extensions_ARB_WGL-5FARB-5Fmake-5Fcurrent-5Fread.txt=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=Ie7_encNUsqxbSRbqbNgofw0ITcfE8JKfaUjIQhncGA=SWh6lT89FsLAyTgJ-rsJ9RAojPix3V1ZDKyBjIR31pI=fUdpS5GuTWuLy5BFTEQD_f8_MfXcrh7ZeLWr6WDKvk0=
  but it's pretty vague:

The function wglMakeContextCurrentARB associates the context 
with the device  for draws and the device  for
reads. All subsequent OpenGL calls made by the calling thread are
drawn on the device identified by  and read on the device
identified by .

How do I do that? Do I have to copy the frame buffer? Or just the
pointer? Or am I completely off road?


Thanks for helping me out ;)



Il 2017-03-14 03:44, Brian Paul ha scritto:

Looks like my KOTOR patch never made it to master. I'm
attaching it
below so you can try it. I should commit it master. In any

[Mesa-dev] [PATCH resend] mesa: Require mipmap completeness for glCopyImageSubData(), sometimes.

2017-03-24 Thread Kenneth Graunke
This patch makes glCopyImageSubData require mipmap completeness when the
texture object's built-in sampler object has a mipmapping MinFilter.

Fixes (on i965):
dEQP-GLES31.functional.debug.negative_coverage.*.buffer.copy_image_sub_data

Signed-off-by: Kenneth Graunke 
---
 src/mesa/main/copyimage.c | 25 +++--
 1 file changed, 23 insertions(+), 2 deletions(-)

Resending the same patch as before.  The Khronos OpenGL ARB and ES working
group both decided that although this behavior is unfortunate and strange,
Android's CTS requires it and so everybody else's implementations already
work this way.

This patch will break several Piglit tests.  You need these patches:

https://patchwork.freedesktop.org/patch/146384/
https://patchwork.freedesktop.org/patch/146385/

This patch will also break several GL 4.5 CTS tests.  You need this patch:

https://gerrit.khronos.org/849

Or for those without Khronos access:

http://whitecape.org/paste/0001-Set-nearest-filtering-in-GL-CopyImage-tests.patch

diff --git a/src/mesa/main/copyimage.c b/src/mesa/main/copyimage.c
index cf25159e880..877c8ac246d 100644
--- a/src/mesa/main/copyimage.c
+++ b/src/mesa/main/copyimage.c
@@ -149,9 +149,30 @@ prepare_target(struct gl_context *ctx, GLuint name, GLenum 
target,
  return false;
   }
 
+  /* The ARB_copy_image specification says:
+   *
+   *"INVALID_OPERATION is generated if either object is a texture and
+   * the texture is not complete (as defined in section 3.9.14)"
+   *
+   * The cited section says:
+   *
+   *"Using the preceding definitions, a texture is complete unless any
+   * of the following conditions hold true: [...]
+   *
+   * * The minification filter requires a mipmap (is neither NEAREST
+   *   nor LINEAR), and the texture is not mipmap complete."
+   *
+   * This imposes the bizarre restriction that glCopyImageSubData requires
+   * mipmap completion at times, which dEQP mandates, and other drivers
+   * appear to implement.  We don't have any texture units here, so we
+   * can't look at any bound separate sampler objects...it appears that
+   * you're supposed to use the sampler object which is built-in to the
+   * texture object.
+   *
+   * See https://cvs.khronos.org/bugzilla/show_bug.cgi?id=16224.
+   */
   _mesa_test_texobj_completeness(ctx, texObj);
-  if (!texObj->_BaseComplete ||
-  (level != 0 && !texObj->_MipmapComplete)) {
+  if (!_mesa_is_texture_complete(texObj, >Sampler)) {
  _mesa_error(ctx, GL_INVALID_OPERATION,
  "glCopyImageSubData(%sName incomplete)", dbg_prefix);
  return false;
-- 
2.12.1

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


Re: [Mesa-dev] [PATCH 7/8] genxml: Whitespace fixes

2017-03-24 Thread Chad Versace
Patches 6 and 7 are
Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 5/8] genxml/gen6: Remove a couple of bogus values

2017-03-24 Thread Chad Versace
On Fri 24 Mar 2017, Jason Ekstrand wrote:
> ---
>  src/intel/genxml/gen6.xml | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)

Thanks. I discovered these in writing my patches, but forgot to fix
them.

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


Re: [Mesa-dev] [PATCH 4/8] genxml/gen8: Remove BLACK_LEVEL_CORRECTION_STATE

2017-03-24 Thread Chad Versace
On Fri 24 Mar 2017, Jason Ekstrand wrote:
> We've never used it, it only exists on gen8, and the name of the struct
> contains piles of bad characters.
> ---
>  src/intel/genxml/gen8.xml | 6 --
>  1 file changed, 6 deletions(-)


> -  
wtf.
Reviewed-by: Chad Versace 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/3] st/wgl: add support for WGL_ARB_make_current_read

2017-03-24 Thread Brian Paul
This adds the wglMakeContextCurrentARB() and wglMakeContextCurrentARB()
functions.
---
 src/gallium/state_trackers/wgl/stw_context.c   | 77 ++
 src/gallium/state_trackers/wgl/stw_context.h   |  5 +-
 src/gallium/state_trackers/wgl/stw_ext_context.c   | 15 +
 .../state_trackers/wgl/stw_ext_extensionsstring.c  |  1 +
 .../state_trackers/wgl/stw_ext_rendertexture.c |  5 +-
 .../state_trackers/wgl/stw_getprocaddress.c|  3 +
 src/gallium/state_trackers/wgl/stw_wgl.c   |  7 ++
 src/gallium/state_trackers/wgl/stw_wgl.h   | 10 +++
 8 files changed, 107 insertions(+), 16 deletions(-)

diff --git a/src/gallium/state_trackers/wgl/stw_context.c 
b/src/gallium/state_trackers/wgl/stw_context.c
index 85cffa6..18273ac 100644
--- a/src/gallium/state_trackers/wgl/stw_context.c
+++ b/src/gallium/state_trackers/wgl/stw_context.c
@@ -187,6 +187,7 @@ stw_create_context_attribs(HDC hdc, INT iLayerPlane, DHGLRC 
hShareContext,
   goto no_ctx;
 
ctx->hdc = hdc;
+   ctx->hReadDC = hdc;
ctx->iPixelFormat = iPixelFormat;
ctx->shared = shareCtx != NULL;
 
@@ -357,7 +358,7 @@ DrvReleaseContext(DHGLRC dhglrc)
if (ctx != stw_current_context())
   return FALSE;
 
-   if (stw_make_current( NULL, 0 ) == FALSE)
+   if (stw_make_current( NULL, NULL, 0 ) == FALSE)
   return FALSE;
 
return TRUE;
@@ -389,9 +390,20 @@ stw_get_current_dc( void )
return ctx->hdc;
 }
 
+HDC
+stw_get_current_read_dc( void )
+{
+   struct stw_context *ctx;
+
+   ctx = stw_current_context();
+   if (!ctx)
+  return NULL;
+
+   return ctx->hReadDC;
+}
 
 BOOL
-stw_make_current(HDC hdc, DHGLRC dhglrc)
+stw_make_current(HDC hdc, HDC hReadDC, DHGLRC dhglrc)
 {
struct stw_context *old_ctx = NULL;
struct stw_context *ctx = NULL;
@@ -403,7 +415,7 @@ stw_make_current(HDC hdc, DHGLRC dhglrc)
old_ctx = stw_current_context();
if (old_ctx != NULL) {
   if (old_ctx->dhglrc == dhglrc) {
- if (old_ctx->hdc == hdc) {
+ if (old_ctx->hdc == hdc && old_ctx->hReadDC == hReadDC) {
 /* Return if already current. */
 return TRUE;
  }
@@ -421,6 +433,7 @@ stw_make_current(HDC hdc, DHGLRC dhglrc)
 
if (dhglrc) {
   struct stw_framebuffer *fb = NULL;
+  struct stw_framebuffer *fbRead = NULL;
   stw_lock_contexts(stw_dev);
   ctx = stw_lookup_context_locked( dhglrc );
   stw_unlock_contexts(stw_dev);
@@ -454,6 +467,7 @@ stw_make_current(HDC hdc, DHGLRC dhglrc)
 
   /* Bind the new framebuffer */
   ctx->hdc = hdc;
+  ctx->hReadDC = hReadDC;
 
   struct stw_framebuffer *old_fb = ctx->current_framebuffer;
   if (old_fb != fb) {
@@ -462,12 +476,47 @@ stw_make_current(HDC hdc, DHGLRC dhglrc)
   }
   stw_framebuffer_unlock(fb);
 
-  /* Note: when we call this function we will wind up in the
-   * stw_st_framebuffer_validate_locked() function which will incur
-   * a recursive fb->mutex lock.
-   */
-  ret = stw_dev->stapi->make_current(stw_dev->stapi, ctx->st,
- fb->stfb, fb->stfb);
+  if (hReadDC) {
+ if (hReadDC == hDrawDC) {
+fbRead = fb;
+ }
+ else {
+fbRead = stw_framebuffer_from_hdc( hReadDC );
+
+if (fbRead) {
+   stw_framebuffer_update(fbRead);
+}
+else {
+   /* Applications should call SetPixelFormat before creating a
+* context, but not all do, and the opengl32 runtime seems to
+* use a default pixel format in some cases, so we must create
+* a framebuffer for those here.
+*/
+   int iPixelFormat = GetPixelFormat(hReadDC);
+   if (iPixelFormat)
+  fbRead = stw_framebuffer_create( hReadDC, iPixelFormat );
+   if (!fbRead)
+  goto fail;
+}
+
+if (fbRead->iPixelFormat != ctx->iPixelFormat) {
+   stw_framebuffer_unlock(fbRead);
+   SetLastError(ERROR_INVALID_PIXEL_FORMAT);
+   goto fail;
+}
+stw_framebuffer_unlock(fbRead);
+ }
+ ret = stw_dev->stapi->make_current(stw_dev->stapi, ctx->st,
+fb->stfb, fbRead->stfb);
+  }
+  else {
+ /* Note: when we call this function we will wind up in the
+  * stw_st_framebuffer_validate_locked() function which will incur
+  * a recursive fb->mutex lock.
+  */
+ ret = stw_dev->stapi->make_current(stw_dev->stapi, ctx->st,
+fb->stfb, fb->stfb);
+  }
 
   if (old_fb && old_fb != fb) {
  stw_lock_framebuffers(stw_dev);
@@ -477,14 +526,16 @@ stw_make_current(HDC hdc, DHGLRC dhglrc)
   }
 
 fail:
-  /* fb must be unlocked at this point. */
-  assert(!stw_own_mutex(>mutex));
+  if (fb) {
+   

[Mesa-dev] [PATCH 3/3] st/wgl: Replace variable name hdc with hDrawDC

2017-03-24 Thread Brian Paul
From: Neha Bhende 

Reviewed-by: Brian Paul 
---
 src/gallium/state_trackers/wgl/stw_context.c | 16 
 src/gallium/state_trackers/wgl/stw_context.h |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/gallium/state_trackers/wgl/stw_context.c 
b/src/gallium/state_trackers/wgl/stw_context.c
index 18273ac..f3145a9 100644
--- a/src/gallium/state_trackers/wgl/stw_context.c
+++ b/src/gallium/state_trackers/wgl/stw_context.c
@@ -186,7 +186,7 @@ stw_create_context_attribs(HDC hdc, INT iLayerPlane, DHGLRC 
hShareContext,
if (ctx == NULL)
   goto no_ctx;
 
-   ctx->hdc = hdc;
+   ctx->hDrawDC = hdc;
ctx->hReadDC = hdc;
ctx->iPixelFormat = iPixelFormat;
ctx->shared = shareCtx != NULL;
@@ -387,7 +387,7 @@ stw_get_current_dc( void )
if (!ctx)
   return NULL;
 
-   return ctx->hdc;
+   return ctx->hDrawDC;
 }
 
 HDC
@@ -403,7 +403,7 @@ stw_get_current_read_dc( void )
 }
 
 BOOL
-stw_make_current(HDC hdc, HDC hReadDC, DHGLRC dhglrc)
+stw_make_current(HDC hDrawDC, HDC hReadDC, DHGLRC dhglrc)
 {
struct stw_context *old_ctx = NULL;
struct stw_context *ctx = NULL;
@@ -415,7 +415,7 @@ stw_make_current(HDC hdc, HDC hReadDC, DHGLRC dhglrc)
old_ctx = stw_current_context();
if (old_ctx != NULL) {
   if (old_ctx->dhglrc == dhglrc) {
- if (old_ctx->hdc == hdc && old_ctx->hReadDC == hReadDC) {
+ if (old_ctx->hDrawDC == hDrawDC && old_ctx->hReadDC == hReadDC) {
 /* Return if already current. */
 return TRUE;
  }
@@ -442,7 +442,7 @@ stw_make_current(HDC hdc, HDC hReadDC, DHGLRC dhglrc)
   }
 
   /* This call locks fb's mutex */
-  fb = stw_framebuffer_from_hdc( hdc );
+  fb = stw_framebuffer_from_hdc( hDrawDC );
   if (fb) {
  stw_framebuffer_update(fb);
   }
@@ -452,9 +452,9 @@ stw_make_current(HDC hdc, HDC hReadDC, DHGLRC dhglrc)
   * pixel format in some cases, so we must create a framebuffer for
   * those here.
   */
- int iPixelFormat = GetPixelFormat(hdc);
+ int iPixelFormat = GetPixelFormat(hDrawDC);
  if (iPixelFormat)
-fb = stw_framebuffer_create( hdc, iPixelFormat );
+fb = stw_framebuffer_create( hDrawDC, iPixelFormat );
  if (!fb)
 goto fail;
   }
@@ -466,7 +466,7 @@ stw_make_current(HDC hdc, HDC hReadDC, DHGLRC dhglrc)
   }
 
   /* Bind the new framebuffer */
-  ctx->hdc = hdc;
+  ctx->hDrawDC = hDrawDC;
   ctx->hReadDC = hReadDC;
 
   struct stw_framebuffer *old_fb = ctx->current_framebuffer;
diff --git a/src/gallium/state_trackers/wgl/stw_context.h 
b/src/gallium/state_trackers/wgl/stw_context.h
index d0e7f2c..b630fc3 100644
--- a/src/gallium/state_trackers/wgl/stw_context.h
+++ b/src/gallium/state_trackers/wgl/stw_context.h
@@ -39,7 +39,7 @@ struct stw_context
struct st_context_iface *st;
DHGLRC dhglrc;
int iPixelFormat;
-   HDC hdc;
+   HDC hDrawDC;
HDC hReadDC;
BOOL shared;
 
@@ -62,7 +62,7 @@ HDC stw_get_current_dc( void );
 
 HDC stw_get_current_read_dc( void );
 
-BOOL stw_make_current( HDC hdc, HDC hReadDC, DHGLRC dhglrc );
+BOOL stw_make_current( HDC hDrawDC, HDC hReadDC, DHGLRC dhglrc );
 
 void stw_notify_current_locked( struct stw_framebuffer *fb );
 
-- 
1.9.1

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


[Mesa-dev] [PATCH 1/3] stw/wgl: add null context check in wglBindTexImageARB()

2017-03-24 Thread Brian Paul
To avoid dereferencing a null pointer in case wglMakeCurrent() wasn't
called.  Found while debugging SWKOTOR game.

Reviewed-by: Neha Bhende 
Reviewed-by: Charmaine Lee 
---
 src/gallium/state_trackers/wgl/stw_ext_rendertexture.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/gallium/state_trackers/wgl/stw_ext_rendertexture.c 
b/src/gallium/state_trackers/wgl/stw_ext_rendertexture.c
index 5eeb0df..9d76696 100644
--- a/src/gallium/state_trackers/wgl/stw_ext_rendertexture.c
+++ b/src/gallium/state_trackers/wgl/stw_ext_rendertexture.c
@@ -129,6 +129,12 @@ wglBindTexImageARB(HPBUFFERARB hPbuffer, int iBuffer)
 * we do here.
 */
 
+   if (!curctx) {
+  debug_printf("No rendering context in wglBindTexImageARB()\n");
+  SetLastError(ERROR_INVALID_OPERATION);
+  return FALSE;
+   }
+
fb = stw_framebuffer_from_HPBUFFERARB(hPbuffer);
if (!fb) {
   debug_printf("Invalid pbuffer handle in wglBindTexImageARB()\n");
-- 
1.9.1

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


Re: [Mesa-dev] [PATCH 2/8] genxml: Fix gen_zipped_file.py dependency

2017-03-24 Thread Chad Versace
Patches 1 and 2 are pushed, with Emil's r-b.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/8] genxml: Rename two MCS fields to Auxiliary Surface on gen7

2017-03-24 Thread Chad Versace
On Fri 24 Mar 2017, Jason Ekstrand wrote:
> This makes gen7 more consistent with gen8+
> ---
>  src/intel/genxml/gen7.xml | 4 ++--
>  src/intel/genxml/gen75.xml| 4 ++--
>  src/intel/isl/isl_surface_state.c | 7 +++
>  3 files changed, 7 insertions(+), 8 deletions(-)

Reviewed-by: Chad Versace 

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


Re: [Mesa-dev] [PATCH 6/9] isl: Validate the calculated row pitch (v2)

2017-03-24 Thread Jason Ekstrand
On Fri, Mar 24, 2017 at 11:10 AM, Chad Versace 
wrote:

> On Thu 23 Mar 2017, Jason Ekstrand wrote:
> > On Wed, Mar 22, 2017 at 6:04 PM, Chad Versace 
> > wrote:
> >
> > > Validate that isl_surf::row_pitch fits in the below bitfields,
> > > if applicable based on isl_surf::usage.
> > >
> > > RENDER_SURFACE_STATE::SurfacePitch
> > > RENDER_SURFACE_STATE::AuxiliarySurfacePitch
> > > 3DSTATE_DEPTH_BUFFER::SurfacePitch
> > > 3DSTATE_HIER_DEPTH_BUFFER::SurfacePitch
> > >
> > > v2: Add a Makefile dependency on generated header genX_bits.h.
> > > ---
> > >  src/intel/Makefile.isl.am |  3 ++
> > >  src/intel/isl/isl.c   | 72 ++
> > > +
> > >  2 files changed, 69 insertions(+), 6 deletions(-)
>
>
>
>
> > > +
> > > +   if (row_pitch == 0)
> > > +  return false;
> > > +
> > > +   if (dim_layout == ISL_DIM_LAYOUT_GEN9_1D) {
> > > +  /* SurfacePitch is ignored for this layout.How should we
> validate
> > > it? */
> > > +  isl_finishme("validate row pitch for ISL_DIM_LAYOUT_GEN9_1D");
> > > +  goto done;
> > > +   }
> > > +
> > > +   if (((surf_info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) ||
> > > +(surf_info->usage & ISL_SURF_USAGE_TEXTURE_BIT)) &&
> > >
> >
> > We also want to handle STORAGE_BIT
>
> Done. I added it locally.
>
> > > +   !pitch_in_range(row_pitch, RENDER_SURFACE_STATE_
> > > SurfacePitch_bits(gen_10x)))
> > > +  return false;
> > > +
> > > +   if (((surf_info->usage & ISL_SURF_USAGE_CCS_BIT) ||
> > > +(surf_info->usage & ISL_SURF_USAGE_MCS_BIT)) &&
> > > +   !pitch_in_range(row_pitch_tiles, RENDER_SURFACE_STATE_
> > > AuxiliarySurfacePitch_bits(gen_10x)))
> > > +  return false;
> > > +
> > > +   if ((surf_info->usage & ISL_SURF_USAGE_DEPTH_BIT) &&
> > > +   !pitch_in_range(row_pitch, _3DSTATE_DEPTH_BUFFER_
> > > SurfacePitch_bits(gen_10x)))
> > > +  return false;
> > > +
> > > +   if ((surf_info->usage & ISL_SURF_USAGE_HIZ_BIT) &&
> > > +   !pitch_in_range(row_pitch, _3DSTATE_HIER_DEPTH_BUFFER_
> > > SurfacePitch_bits(gen_10x)))
> > > +  return false;
> > > +
> > > +   if (surf_info->usage & ISL_SURF_USAGE_STENCIL_BIT)
> > > +  isl_finishme("validate row pitch of stencil surfaces");
> > >
> >
> > How hard is this?  I assume it's not trivial or you would have done it.
>
> It's not trivial. There's corner cases, especially for older gens.  But
> it's not *too* hard. It's just that I have no confidence that I could
> write it correctly on the first try.
>
> I wanted to land all the obvious pitch validations asap. Where obvious
> means:
>
>if ((usage & bits) && !pitch_in_range())
>   return false;
>
> Since stencil pitch validation is the only non-obvious validation,
> I wanted to do it as a follow-up. Because there WILL be errors in v1 of
> the patch. And I didn't want this patch to be blocked while I debugged
> the stencil validation failures and argued over the corner cases.
>

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


[Mesa-dev] [PATCH 8/8] genxml: New generated header genX_bits.h (v2)

2017-03-24 Thread Jason Ekstrand
From: Chad Versace 

genX_bits.h contains the sizes of bitfields in genxml instructions,
structures, and registers. It also defines some functions to query those
sizes.

isl_surf_init() will use the new header to validate that requested
pitches fit in their destination bitfields.

What's currently in genX_bits.h:

  - For each CONTAINER::Field in gen{n}.xml whose name matches
/.*Surface(Q?)Pitch_bits$/, genX_bits.h contains the line:

  #define GEN{n}_CONTAINER_Field_bits {number of bits in field}

STREAMOUT fields are omitted because isl doesn't care about them.

  - For each set of macros whose name, after stripping the GEN prefix,
is the same, genX_bits.h contains the query function:

  static inline uint32_t __attribute__((const))
  CONTAINER_Field_bits(const struct gen_device_info *devinfo);

which returns the number of bits in that field or 0 if the field
does not exist on the specified hardware.

v2 (Chad Versace):
  - Parse the XML instead of scraping the generated gen*_pack.h headers.
[for jekstrand]

Jason and I tentatively agreed that I should just hand-write the
header. But my conscience refused. The XML way is the right way.
Anyway, the generator script are about the same number of lines (259
vs 222), so the generator is the clear winner in my opinion.

v3 (Dylan Baker):
  - Port to Mako

v4 (Jason Ekstrand):
  - Make the _bits functions take a gen_device_info

Co-authored-by: Dylan Baker 
Co-authored-by: Jason Ekstrand 
---
 src/intel/Makefile.genxml.am|   6 +-
 src/intel/Makefile.sources  |   6 +-
 src/intel/genxml/.gitignore |   1 +
 src/intel/genxml/gen_bits_header.py | 293 
 4 files changed, 304 insertions(+), 2 deletions(-)
 create mode 100644 src/intel/genxml/gen_bits_header.py

diff --git a/src/intel/Makefile.genxml.am b/src/intel/Makefile.genxml.am
index 01a02b6..4e59a91 100644
--- a/src/intel/Makefile.genxml.am
+++ b/src/intel/Makefile.genxml.am
@@ -30,7 +30,7 @@ EXTRA_DIST += \
 
 SUFFIXES = _pack.h _xml.h .xml
 
-$(GENXML_GENERATED_FILES): genxml/gen_pack_header.py
+$(GENXML_GENERATED_PACK_FILES): genxml/gen_pack_header.py
 
 .xml_pack.h:
$(MKDIR_GEN)
@@ -42,6 +42,10 @@ $(AUBINATOR_GENERATED_FILES): genxml/gen_zipped_file.py
$(MKDIR_GEN)
$(AM_V_GEN) $(PYTHON2) $(srcdir)/genxml/gen_zipped_file.py $< > $@ || 
($(RM) $@; false)
 
+genxml/genX_bits.h: genxml/gen_bits_header.py $(GENXML_XML_FILES)
+   $(MKDIR_GEN)
+   $(PYTHON_GEN) $< -o $@ $(GENXML_XML_FILES)
+
 EXTRA_DIST += \
genxml/genX_pack.h \
genxml/gen_macros.h \
diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
index 88bcf60..c568916 100644
--- a/src/intel/Makefile.sources
+++ b/src/intel/Makefile.sources
@@ -119,7 +119,7 @@ GENXML_XML_FILES = \
genxml/gen8.xml \
genxml/gen9.xml
 
-GENXML_GENERATED_FILES = \
+GENXML_GENERATED_PACK_FILES = \
genxml/gen4_pack.h \
genxml/gen45_pack.h \
genxml/gen5_pack.h \
@@ -129,6 +129,10 @@ GENXML_GENERATED_FILES = \
genxml/gen8_pack.h \
genxml/gen9_pack.h
 
+GENXML_GENERATED_FILES = \
+   $(GENXML_GENERATED_PACK_FILES) \
+   genxml/genX_bits.h
+
 AUBINATOR_GENERATED_FILES = \
genxml/gen6_xml.h \
genxml/gen7_xml.h \
diff --git a/src/intel/genxml/.gitignore b/src/intel/genxml/.gitignore
index c5672b5..3e2f1cf 100644
--- a/src/intel/genxml/.gitignore
+++ b/src/intel/genxml/.gitignore
@@ -1,2 +1,3 @@
+gen*_bits.h
 gen*_pack.h
 gen*_xml.h
diff --git a/src/intel/genxml/gen_bits_header.py 
b/src/intel/genxml/gen_bits_header.py
new file mode 100644
index 000..53a980e
--- /dev/null
+++ b/src/intel/genxml/gen_bits_header.py
@@ -0,0 +1,293 @@
+#encoding=utf-8
+# Copyright © 2017 Intel Corporation
+
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+from __future__ import (
+  

[Mesa-dev] [PATCH 6/8] genxml: Replace "[N]" with "N"

2017-03-24 Thread Jason Ekstrand
---
 src/intel/genxml/gen6.xml  |  6 +++---
 src/intel/genxml/gen7.xml  | 12 ++--
 src/intel/genxml/gen75.xml | 12 ++--
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml
index d77d158..8eef7bf 100644
--- a/src/intel/genxml/gen6.xml
+++ b/src/intel/genxml/gen6.xml
@@ -1413,9 +1413,9 @@
 
 
 
-
-
-
+
+
+
 
 
 
diff --git a/src/intel/genxml/gen7.xml b/src/intel/genxml/gen7.xml
index 40927ef..ee7877a 100644
--- a/src/intel/genxml/gen7.xml
+++ b/src/intel/genxml/gen7.xml
@@ -1312,7 +1312,7 @@
 
 
 
-
+
 
 
 
@@ -1351,11 +1351,11 @@
 
 
 
-
-
-
-
-
+
+
+
+
+
   
 
   
diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
index 7b5c2af..094095e 100644
--- a/src/intel/genxml/gen75.xml
+++ b/src/intel/genxml/gen75.xml
@@ -1538,7 +1538,7 @@
 
 
 
-
+
 
 
 
@@ -1582,11 +1582,11 @@
 
 
 
-
-
-
-
-
+
+
+
+
+
   
 
   
-- 
2.5.0.400.gff86faf

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


[Mesa-dev] [PATCH 5/8] genxml/gen6: Remove a couple of bogus values

2017-03-24 Thread Jason Ekstrand
---
 src/intel/genxml/gen6.xml | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml
index da3f64d..d77d158 100644
--- a/src/intel/genxml/gen6.xml
+++ b/src/intel/genxml/gen6.xml
@@ -939,18 +939,14 @@
 
 
 
-
-  
-
+
 
 
 
   
   
 
-
-  
-
+
 
 
 
-- 
2.5.0.400.gff86faf

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


[Mesa-dev] [PATCH 1/8] genxml: Define GENXML_XML_FILES in Makefile.sources

2017-03-24 Thread Jason Ekstrand
From: Chad Versace 

The future header genX_bits.h will depend on GENXML_XML_FILES.
---
 src/intel/Makefile.genxml.am |  9 +
 src/intel/Makefile.sources   | 10 ++
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/intel/Makefile.genxml.am b/src/intel/Makefile.genxml.am
index bea0aab..eab6ccd 100644
--- a/src/intel/Makefile.genxml.am
+++ b/src/intel/Makefile.genxml.am
@@ -24,6 +24,7 @@ BUILT_SOURCES += \
$(AUBINATOR_GENERATED_FILES)
 
 EXTRA_DIST += \
+   $(GENXML_XML_FILES) \
$(GENXML_GENERATED_FILES) \
$(AUBINATOR_GENERATED_FILES)
 
@@ -42,14 +43,6 @@ $(GENXML_GENERATED_FILES): genxml/gen_zipped_file.py
$(AM_V_GEN) $(PYTHON2) $(srcdir)/genxml/gen_zipped_file.py $< > $@ || 
($(RM) $@; false)
 
 EXTRA_DIST += \
-   genxml/gen4.xml \
-   genxml/gen45.xml \
-   genxml/gen5.xml \
-   genxml/gen6.xml \
-   genxml/gen7.xml \
-   genxml/gen75.xml \
-   genxml/gen8.xml \
-   genxml/gen9.xml \
genxml/genX_pack.h \
genxml/gen_macros.h \
genxml/gen_pack_header.py \
diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
index bdd8fe6..88bcf60 100644
--- a/src/intel/Makefile.sources
+++ b/src/intel/Makefile.sources
@@ -109,6 +109,16 @@ COMPILER_FILES = \
 COMPILER_GENERATED_FILES = \
compiler/brw_nir_trig_workarounds.c
 
+GENXML_XML_FILES = \
+   genxml/gen4.xml \
+   genxml/gen45.xml \
+   genxml/gen5.xml \
+   genxml/gen6.xml \
+   genxml/gen7.xml \
+   genxml/gen75.xml \
+   genxml/gen8.xml \
+   genxml/gen9.xml
+
 GENXML_GENERATED_FILES = \
genxml/gen4_pack.h \
genxml/gen45_pack.h \
-- 
2.5.0.400.gff86faf

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


[Mesa-dev] [PATCH 7/8] genxml: Whitespace fixes

2017-03-24 Thread Jason Ekstrand
Some field names had extra spaces and some had places where we should
have had a space but didn't.
---
 src/intel/genxml/gen6.xml  | 16 
 src/intel/genxml/gen7.xml  | 24 
 src/intel/genxml/gen75.xml | 22 +++---
 src/intel/genxml/gen8.xml  |  8 
 src/intel/genxml/gen9.xml  |  8 
 5 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/src/intel/genxml/gen6.xml b/src/intel/genxml/gen6.xml
index 8eef7bf..33969d9 100644
--- a/src/intel/genxml/gen6.xml
+++ b/src/intel/genxml/gen6.xml
@@ -811,9 +811,9 @@
   
   
 
-
-
-
+
+
+
 
 
   
@@ -839,8 +839,8 @@
 
 
 
-
-
+
+
   
 
   
@@ -996,7 +996,7 @@
 
 
 
-
+
 
 
 
@@ -1230,7 +1230,7 @@
   
 
 
-
+
 
 
   
@@ -1374,7 +1374,7 @@
   
 
 
-
+
 
 
 
diff --git a/src/intel/genxml/gen7.xml b/src/intel/genxml/gen7.xml
index ee7877a..f46dae7 100644
--- a/src/intel/genxml/gen7.xml
+++ b/src/intel/genxml/gen7.xml
@@ -918,7 +918,7 @@
 
 
 
-
+
 
   
   
@@ -932,9 +932,9 @@
   
   
 
-
-
-
+
+
+
 
 
   
@@ -959,8 +959,8 @@
 
 
 
-
-
+
+
   
 
   
@@ -1131,7 +1131,7 @@
 
 
 
-
+
 
 
 
@@ -1194,7 +1194,7 @@
   
 
 
-
+
 
 
 
@@ -1333,7 +1333,7 @@
 
 
 
-
+
 
 
 
@@ -1602,7 +1602,7 @@
   
 
 
-
+
 
 
   
@@ -1846,7 +1846,7 @@
   
 
 
-
+
 
 
 
@@ -2430,7 +2430,7 @@
 
 
 
-
+
 
 
 
diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
index 094095e..7fe9b02 100644
--- a/src/intel/genxml/gen75.xml
+++ b/src/intel/genxml/gen75.xml
@@ -1037,7 +1037,7 @@
 
 
 
-
+
 
   
   
@@ -1051,9 +1051,9 @@
   
   
 
-
-
-
+
+
+
 
 
   
@@ -1078,8 +1078,8 @@
 
 
 
-
-
+
+
   
 
   
@@ -1350,7 +1350,7 @@
 
 
 
-
+
 
 
 
@@ -1562,7 +1562,7 @@
 
 
 
-
+
 
 
 
@@ -1859,7 +1859,7 @@
 
 
 
-
+
 
 
   
@@ -2119,7 +2119,7 @@
 
 
 
-
+
 
 
 
@@ -2839,7 +2839,7 @@
 
 
 
-
+
 
 
 
diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
index 193a183..0ebf2aa 100644
--- a/src/intel/genxml/gen8.xml
+++ b/src/intel/genxml/gen8.xml
@@ -1415,7 +1415,7 @@
 
 
 
-
+
 
 
 
@@ -1619,7 +1619,7 @@
 
 
 
-
+
 
 
 
@@ -2280,7 +2280,7 @@
 
 
 
-
+
 
 
 
@@ -3099,7 +3099,7 @@
 
 
 
-
+
 
 
 
diff --git a/src/intel/genxml/gen9.xml b/src/intel/genxml/gen9.xml
index bfe4932..79fad00 100644
--- a/src/intel/genxml/gen9.xml
+++ b/src/intel/genxml/gen9.xml
@@ -1520,7 +1520,7 @@
 
 
 
-
+
 
 
 
@@ -1732,7 +1732,7 @@
 
 
 
-
+
 
 
 
@@ -2508,7 +2508,7 @@
 
 
 
-
+
 
 
 
@@ -3382,7 +3382,7 @@
 
 
 
-
+
 
 
 
-- 
2.5.0.400.gff86faf

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


[Mesa-dev] [PATCH 3/8] genxml: Rename two MCS fields to Auxiliary Surface on gen7

2017-03-24 Thread Jason Ekstrand
This makes gen7 more consistent with gen8+
---
 src/intel/genxml/gen7.xml | 4 ++--
 src/intel/genxml/gen75.xml| 4 ++--
 src/intel/isl/isl_surface_state.c | 7 +++
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/intel/genxml/gen7.xml b/src/intel/genxml/gen7.xml
index 3f3b188..40927ef 100644
--- a/src/intel/genxml/gen7.xml
+++ b/src/intel/genxml/gen7.xml
@@ -672,8 +672,8 @@
 
 
 
-
-
+
+
 
 
 
diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
index 91fe02f..7b5c2af 100644
--- a/src/intel/genxml/gen75.xml
+++ b/src/intel/genxml/gen75.xml
@@ -683,8 +683,8 @@
 
 
 
-
-
+
+
 
 
 
diff --git a/src/intel/isl/isl_surface_state.c 
b/src/intel/isl/isl_surface_state.c
index 853bb11..fa46469 100644
--- a/src/intel/isl/isl_surface_state.c
+++ b/src/intel/isl/isl_surface_state.c
@@ -548,16 +548,17 @@ isl_genX(surf_fill_state_s)(const struct isl_device *dev, 
void *state,
   uint32_t pitch_in_tiles =
  info->aux_surf->row_pitch / tile_info.phys_extent_B.width;
 
+  s.AuxiliarySurfaceBaseAddress = info->aux_address;
+  s.AuxiliarySurfacePitch = pitch_in_tiles - 1;
+
 #if GEN_GEN >= 8
   assert(GEN_GEN >= 9 || info->aux_usage != ISL_AUX_USAGE_CCS_E);
-  s.AuxiliarySurfacePitch = pitch_in_tiles - 1;
   /* Auxiliary surfaces in ISL have compressed formats but the hardware
* doesn't expect our definition of the compression, it expects qpitch
* in units of samples on the main surface.
*/
   s.AuxiliarySurfaceQPitch =
  isl_surf_get_array_pitch_sa_rows(info->aux_surf) >> 2;
-  s.AuxiliarySurfaceBaseAddress = info->aux_address;
 
   if (info->aux_usage == ISL_AUX_USAGE_HIZ) {
  /* The number of samples must be 1 */
@@ -582,8 +583,6 @@ isl_genX(surf_fill_state_s)(const struct isl_device *dev, 
void *state,
 #else
   assert(info->aux_usage == ISL_AUX_USAGE_MCS ||
  info->aux_usage == ISL_AUX_USAGE_CCS_D);
-  s.MCSBaseAddress = info->aux_address,
-  s.MCSSurfacePitch = pitch_in_tiles - 1;
   s.MCSEnable = true;
 #endif
}
-- 
2.5.0.400.gff86faf

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


[Mesa-dev] [PATCH 4/8] genxml/gen8: Remove BLACK_LEVEL_CORRECTION_STATE

2017-03-24 Thread Jason Ekstrand
We've never used it, it only exists on gen8, and the name of the struct
contains piles of bad characters.
---
 src/intel/genxml/gen8.xml | 6 --
 1 file changed, 6 deletions(-)

diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
index f9899c6..193a183 100644
--- a/src/intel/genxml/gen8.xml
+++ b/src/intel/genxml/gen8.xml
@@ -582,12 +582,6 @@
 
   
 
-  
-
-
-
-  
-
   
 
 
-- 
2.5.0.400.gff86faf

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


[Mesa-dev] [PATCH 2/8] genxml: Fix gen_zipped_file.py dependency

2017-03-24 Thread Jason Ekstrand
From: Chad Versace 

The gen*_xml.h files depend on gen_zipped_file.py, not the gen*_pack.h
files.
---
 src/intel/Makefile.genxml.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/intel/Makefile.genxml.am b/src/intel/Makefile.genxml.am
index eab6ccd..01a02b6 100644
--- a/src/intel/Makefile.genxml.am
+++ b/src/intel/Makefile.genxml.am
@@ -36,7 +36,7 @@ $(GENXML_GENERATED_FILES): genxml/gen_pack_header.py
$(MKDIR_GEN)
$(PYTHON_GEN) $(srcdir)/genxml/gen_pack_header.py $< > $@ || ($(RM) $@; 
false)
 
-$(GENXML_GENERATED_FILES): genxml/gen_zipped_file.py
+$(AUBINATOR_GENERATED_FILES): genxml/gen_zipped_file.py
 
 .xml_xml.h:
$(MKDIR_GEN)
-- 
2.5.0.400.gff86faf

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


[Mesa-dev] [PATCH 0/8] genxml: Add a _bits header

2017-03-24 Thread Jason Ekstrand
This series is a re-spin of some patches that Chad wrote to add a
genX_bits.h header which contains functions for getting the number of bits
in any given hardware state field.

The first few patches are fixes for genxml.  There were a bunch of places
where the sanitized field names matched but not the actual field names
because of extra spaces or some such.  This cleans those up.

Then Dylna respun Chad's generator to use mako and I respun it again to
make the *_bits functions it generates take a gen_device_info struct
instead of int(gen x 10).

Cc: Chad Versace 

Chad Versace (3):
  genxml: Define GENXML_XML_FILES in Makefile.sources
  genxml: Fix gen_zipped_file.py dependency
  genxml: New generated header genX_bits.h (v2)

Jason Ekstrand (5):
  genxml: Rename two MCS fields to Auxiliary Surface on gen7
  genxml/gen8: Remove BLACK_LEVEL_CORRECTION_STATE
  genxml/gen6: Remove a couple of bogus values
  genxml: Replace "[N]" with "N"
  genxml: Whitespace fixes

 src/intel/Makefile.genxml.am|  17 +--
 src/intel/Makefile.sources  |  16 +-
 src/intel/genxml/.gitignore |   1 +
 src/intel/genxml/gen6.xml   |  30 ++--
 src/intel/genxml/gen7.xml   |  40 ++---
 src/intel/genxml/gen75.xml  |  38 ++---
 src/intel/genxml/gen8.xml   |  14 +-
 src/intel/genxml/gen9.xml   |   8 +-
 src/intel/genxml/gen_bits_header.py | 293 
 src/intel/isl/isl_surface_state.c   |   7 +-
 10 files changed, 379 insertions(+), 85 deletions(-)
 create mode 100644 src/intel/genxml/gen_bits_header.py

-- 
2.5.0.400.gff86faf

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


Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Rob Clark
On Fri, Mar 24, 2017 at 5:16 PM, Jose Fonseca  wrote:
> On 24/03/17 20:08, Kristian Høgsberg wrote:
>>
>> On Fri, Mar 24, 2017 at 12:44 PM, Jose Fonseca 
>> wrote:
>>>
>>> On 24/03/17 19:10, Kristian Høgsberg wrote:


 On Fri, Mar 24, 2017 at 6:42 AM, Jose Fonseca 
 wrote:
>
>
> On 23/03/17 01:38, Rob Clark wrote:
>>
>>
>>
>> On Wed, Mar 22, 2017 at 9:18 PM, Jonathan Gray  wrote:
>>>
>>>
>>>
>>> On Wed, Mar 22, 2017 at 01:10:14PM -0700, Dylan Baker wrote:



 On Wed, Mar 22, 2017 at 12:40 PM, Alex Deucher
 
 wrote:
>
>
>
> I guess I'm a little late to the party here, but I haven't had time
> to
> really let all of this sink in and actually look at meson.  It
> doesn't
> seem so bad with a quick look and I think I could probably sort it
> out
> when the time came, but there would still be a bit of a learning
> curve.  While that may not be a big deal at the micro level, I have
> concerns at the macro level.
>
> First, I'm concerned it may discourage casual developers and
> packagers.  autotools isn't great, but most people are familiar
> enough
> with it that they can get by.  Most people have enough knowledge of
> autotools that they can pretty easily diagnose a configuration
> based
> failure. There are a lot of resources for autotools.  I'm not sure
> that would be the case for meson.  Do we as a community feel we
> have
> enough meson experience to get people over the hump?  Anything that
> makes it harder for someone to try a new build or do a bisect is a
> big
> problem in my opinion.




 One of the things that's prompted this on our side (I've talked this
 over with
 other people at Intel before starting), was that clearly we *don't*
 know
 autotools well enough to get it right. Emil almost always finds
 cases
 were we've
 done things *almost*, but not quite right.

 For my part, it took me about 3 or 4 days of reading through the
 docs
 and
 writing the libdrm port to get it right, and a lot of that is just
 the
 boilerplate of having ~8 drivers that all need basically the same
 logic.

> Next, my bigger concern is for distro and casual packagers and
> people
> that maintain large build systems with lots of existing custom
> configurations.  Changing from autotools would basically require
> many
> of these existing tools and systems to be rewritten and then deal
> with
> the debugging and fall out from that.  The potential decreased
> build
> time is a nice bonus, but frankly a lot of people/companies have
> years
> of investment in existing tools.




 Sure, but we're also not the only ones investigating meson. Gnome is
 using it
 already, libepoxy is using it, gstreamer is using it. There are
 patches
 for
 weston (written by Daniel Stone) and libinput (written by Peter
 Hutterer), there
 are some other projects in the graphics sphere that people are
 working
 on. So
 even if we as a community decide that meson isn't for us, it's not
 going
 away.
>>>
>>>
>>>
>>>
>>> It is worth pointing out that it is currently required by no
>>> component
>>> of an x.org stack.  In the case of libepoxy it was added by a new
>>> maintainer
>>> on a new release and even then autoconf remains.
>>>
>>> And as far as I can tell nothing in the entire OpenBSD ports tree
>>> currently requires meson to build including gnome and gstreamer.
>>>
>>
>> but I guess that is conflating two completely different topics..
>> addition of meson and removal of autotools.  It is probably better
>> that we treat the topics separately.  I don't see any way that the two
>> can happen at the same time.
>>
>> The autotools build probably needs to remain for at least a couple
>> releases, and I certainly wouldn't mind if some of the other desktop
>> projects took the leap of dropping autotools first (at least then
>> various different "distro" consumers will have already dealt with how
>> to build meson packages)
>>
>> None of that blocks addition of a meson build system (or what various
>> developers use day to day)
>>
>> BR,
>> -R
>
>
>
>
> I tend to disagree.  While we can't avoid a transitory 

[Mesa-dev] [PATCH 1/2] gallium/util: clean up stack frame printing

2017-03-24 Thread Rob Clark
Prep work for next patch.

Ideally 'struct debug_stack_frame' would be opaque, but it is embedded
in a bunch of places.  But at least we can treat it opaquely.

Signed-off-by: Rob Clark 
---
 src/gallium/auxiliary/util/u_debug_refcnt.c | 27 ---
 src/gallium/auxiliary/util/u_debug_stack.c  | 17 +
 src/gallium/auxiliary/util/u_debug_stack.h  |  5 +
 3 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/src/gallium/auxiliary/util/u_debug_refcnt.c 
b/src/gallium/auxiliary/util/u_debug_refcnt.c
index cb01582..728dbee 100644
--- a/src/gallium/auxiliary/util/u_debug_refcnt.c
+++ b/src/gallium/auxiliary/util/u_debug_refcnt.c
@@ -134,18 +134,6 @@ debug_serial_delete(void *p)
 
 #define STACK_LEN 64
 
-static void
-dump_stack(const char *symbols[STACK_LEN])
-{
-   unsigned i;
-   for (i = 0; i < STACK_LEN; ++i) {
-  if (symbols[i])
- fprintf(stream, "%s\n", symbols[i]);
-   }
-   fprintf(stream, "\n");
-}
-
-
 /**
  * Log a reference count change to the log file (if enabled).
  * This is called via the pipe_reference() and debug_reference() functions,
@@ -180,7 +168,6 @@ debug_reference_slowpath(const struct pipe_reference *p,
 
if (debug_refcnt_state > 0) {
   struct debug_stack_frame frames[STACK_LEN];
-  const char *symbols[STACK_LEN];
   char buf[1024];
   unsigned i;
   unsigned refcnt = p->count;
@@ -188,18 +175,12 @@ debug_reference_slowpath(const struct pipe_reference *p,
   boolean existing = debug_serial((void *) p, );
 
   debug_backtrace_capture(frames, 1, STACK_LEN);
-  for (i = 0; i < STACK_LEN; ++i) {
- if (frames[i].function)
-symbols[i] = debug_symbol_name_cached(frames[i].function);
- else
-symbols[i] = 0;
-  }
 
   get_desc(buf, p);
 
   if (!existing) {
  fprintf(stream, "<%s> %p %u Create\n", buf, (void *) p, serial);
- dump_stack(symbols);
+ debug_backtrace_print(stream, frames, STACK_LEN);
 
  /* this is here to provide a gradual change even if we don't see
   * the initialization
@@ -207,20 +188,20 @@ debug_reference_slowpath(const struct pipe_reference *p,
  for (i = 1; i <= refcnt - change; ++i) {
 fprintf(stream, "<%s> %p %u AddRef %u\n", buf, (void *) p,
 serial, i);
-dump_stack(symbols);
+debug_backtrace_print(stream, frames, STACK_LEN);
  }
   }
 
   if (change) {
  fprintf(stream, "<%s> %p %u %s %u\n", buf, (void *) p, serial,
  change > 0 ? "AddRef" : "Release", refcnt);
- dump_stack(symbols);
+ debug_backtrace_print(stream, frames, STACK_LEN);
   }
 
   if (!refcnt) {
  debug_serial_delete((void *) p);
  fprintf(stream, "<%s> %p %u Destroy\n", buf, (void *) p, serial);
- dump_stack(symbols);
+ debug_backtrace_print(stream, frames, STACK_LEN);
   }
 
   fflush(stream);
diff --git a/src/gallium/auxiliary/util/u_debug_stack.c 
b/src/gallium/auxiliary/util/u_debug_stack.c
index 1faa190..f941234 100644
--- a/src/gallium/auxiliary/util/u_debug_stack.c
+++ b/src/gallium/auxiliary/util/u_debug_stack.c
@@ -162,3 +162,20 @@ debug_backtrace_dump(const struct debug_stack_frame 
*backtrace,
}
 }
 
+
+void
+debug_backtrace_print(FILE *f,
+  const struct debug_stack_frame *backtrace,
+  unsigned nr_frames)
+{
+   unsigned i;
+
+   for (i = 0; i < nr_frames; ++i) {
+  const char *symbol;
+  if (!backtrace[i].function)
+ break;
+  symbol = debug_symbol_name_cached(backtrace[i].function);
+  if (symbol)
+ fprintf(f, "%s\n", symbol);
+   }
+}
diff --git a/src/gallium/auxiliary/util/u_debug_stack.h 
b/src/gallium/auxiliary/util/u_debug_stack.h
index b1848dd..04eba08 100644
--- a/src/gallium/auxiliary/util/u_debug_stack.h
+++ b/src/gallium/auxiliary/util/u_debug_stack.h
@@ -28,6 +28,7 @@
 #ifndef U_DEBUG_STACK_H_
 #define U_DEBUG_STACK_H_
 
+#include 
 
 /**
  * @file
@@ -64,6 +65,10 @@ void
 debug_backtrace_dump(const struct debug_stack_frame *backtrace, 
  unsigned nr_frames);
 
+void
+debug_backtrace_print(FILE *f,
+  const struct debug_stack_frame *backtrace,
+  unsigned nr_frames);
 
 #ifdef __cplusplus
 }
-- 
2.9.3

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


[Mesa-dev] [PATCH 2/2] gallium/util: libunwind support

2017-03-24 Thread Rob Clark
It's kinda sad that (a) we don't have debug_backtrace support on !X86
and that (b) we re-invent our own crude backtrace support in the first
place.  If available, use libunwind instead.  The backtrace format is
based on what xserver and weston use, since it is nice not to have to
figure out a different format.

Signed-off-by: Rob Clark 
---
 configure.ac   | 24 
 src/gallium/Automake.inc   |  1 +
 src/gallium/auxiliary/util/u_debug_stack.c | 91 ++
 src/gallium/auxiliary/util/u_debug_stack.h | 15 -
 4 files changed, 129 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index a99684b..5046acb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1025,6 +1025,30 @@ AC_SUBST([LLVM_LIBS])
 AC_SUBST([LLVM_LDFLAGS])
 AC_SUBST([LLVM_INCLUDEDIR])
 
+dnl
+dnl libunwind
+dnl
+AC_ARG_ENABLE([libunwind],
+[AS_HELP_STRING([--enable-libunwind],
+[Use libunwind for backtracing (default: auto)])],
+[LIBUNWIND="$enableval"],
+[LIBUNWIND="auto"])
+
+PKG_CHECK_MODULES(LIBUNWIND, libunwind, [HAVE_LIBUNWIND=yes], 
[HAVE_LIBUNWIND=no])
+if test "x$LIBUNWIND" = "xauto"; then
+LIBUNWIND="$HAVE_LIBUNWIND"
+fi
+
+if test "x$LIBUNWIND" = "xyes"; then
+if test "x$HAVE_LIBUNWIND" != "xyes"; then
+AC_MSG_ERROR([libunwind requested but not installed.])
+fi
+AC_DEFINE(HAVE_LIBUNWIND, 1, [Have libunwind support])
+fi
+
+AM_CONDITIONAL(HAVE_LIBUNWIND, [test "x$LIBUNWIND" = xyes])
+
+
 dnl Options for APIs
 AC_ARG_ENABLE([opengl],
 [AS_HELP_STRING([--disable-opengl],
diff --git a/src/gallium/Automake.inc b/src/gallium/Automake.inc
index a01fa54..48b5a44 100644
--- a/src/gallium/Automake.inc
+++ b/src/gallium/Automake.inc
@@ -46,6 +46,7 @@ GALLIUM_TARGET_CFLAGS = \
 
 GALLIUM_COMMON_LIB_DEPS = \
-lm \
+   $(LIBUNWIND_LIBS) \
$(LIBSENSORS_LIBS) \
$(CLOCK_LIB) \
$(PTHREAD_LIBS) \
diff --git a/src/gallium/auxiliary/util/u_debug_stack.c 
b/src/gallium/auxiliary/util/u_debug_stack.c
index f941234..cf05f13 100644
--- a/src/gallium/auxiliary/util/u_debug_stack.c
+++ b/src/gallium/auxiliary/util/u_debug_stack.c
@@ -36,6 +36,95 @@
 #include "u_debug_symbol.h"
 #include "u_debug_stack.h"
 
+#if defined(HAVE_LIBUNWIND)
+
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+#include 
+
+void
+debug_backtrace_capture(struct debug_stack_frame *backtrace,
+unsigned start_frame,
+unsigned nr_frames)
+{
+   unw_cursor_t cursor;
+   unw_context_t context;
+   unw_proc_info_t pip;
+   unsigned i = 0;
+   int ret;
+
+   pip.unwind_info = NULL;
+
+   unw_getcontext();
+   unw_init_local(, );
+
+   while ((start_frame > 0) && (unw_step() > 0))
+  start_frame--;
+
+   while (unw_step() > 0) {
+  char procname[256];
+  const char *filename;
+  unw_word_t off;
+  Dl_info dlinfo;
+
+  unw_get_proc_info(, );
+
+  ret = unw_get_proc_name(, procname, 256, );
+  if (ret && ret != -UNW_ENOMEM) {
+ procname[0] = '?';
+ procname[1] = 0;
+  }
+
+   if (dladdr((void *)(uintptr_t)(pip.start_ip + off), ) && 
dlinfo.dli_fname &&
+   *dlinfo.dli_fname)
+   filename = dlinfo.dli_fname;
+   else
+   filename = "?";
+
+  snprintf(backtrace[i].buf, sizeof(backtrace[i].buf),
+"%u: %s (%s%s+0x%x) [%p]", i, filename, procname,
+ret == -UNW_ENOMEM ? "..." : "", (int)off,
+(void *)(uintptr_t)(pip.start_ip + off));
+
+  i++;
+   }
+
+   while (i < nr_frames) {
+  backtrace[i].buf[0] = '\0';
+  i++;
+   }
+}
+
+void
+debug_backtrace_dump(const struct debug_stack_frame *backtrace,
+ unsigned nr_frames)
+{
+   unsigned i;
+
+   for (i = 0; i < nr_frames; ++i) {
+  if (backtrace[i].buf[0] == '\0')
+ break;
+  debug_printf("\t%s\n", backtrace[i].buf);
+   }
+}
+
+void
+debug_backtrace_print(FILE *f,
+  const struct debug_stack_frame *backtrace,
+  unsigned nr_frames)
+{
+   unsigned i;
+
+   for (i = 0; i < nr_frames; ++i) {
+  if (backtrace[i].buf[0] == '\0')
+ break;
+  fprintf(f, "\t%s\n", backtrace[i].buf);
+   }
+}
+
+#else /* ! HAVE_LIBUNWIND */
+
 #if defined(PIPE_OS_WINDOWS)
 #include 
 #endif
@@ -179,3 +268,5 @@ debug_backtrace_print(FILE *f,
  fprintf(f, "%s\n", symbol);
}
 }
+
+#endif /* HAVE_LIBUNWIND */
diff --git a/src/gallium/auxiliary/util/u_debug_stack.h 
b/src/gallium/auxiliary/util/u_debug_stack.h
index 04eba08..0effcbe 100644
--- a/src/gallium/auxiliary/util/u_debug_stack.h
+++ b/src/gallium/auxiliary/util/u_debug_stack.h
@@ -30,6 +30,11 @@
 
 #include 
 
+#ifdef HAVE_LIBUNWIND
+#define UNW_LOCAL_ONLY
+#include 
+#endif
+
 /**
  * @file
  * Stack backtracing.
@@ -46,15 +51,21 @@ extern "C" {
 /**
  * Represent a frame from a stack backtrace.
  *
- * XXX: Do not change this.

Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Jason Ekstrand
On Fri, Mar 24, 2017 at 2:16 PM, Jose Fonseca  wrote:

> On 24/03/17 20:08, Kristian Høgsberg wrote:
>
>> On Fri, Mar 24, 2017 at 12:44 PM, Jose Fonseca 
>> wrote:
>>
>>> On 24/03/17 19:10, Kristian Høgsberg wrote:
>>>

 On Fri, Mar 24, 2017 at 6:42 AM, Jose Fonseca 
 wrote:

>
> On 23/03/17 01:38, Rob Clark wrote:
>
>>
>>
>> On Wed, Mar 22, 2017 at 9:18 PM, Jonathan Gray  wrote:
>>
>>>
>>>
>>> On Wed, Mar 22, 2017 at 01:10:14PM -0700, Dylan Baker wrote:
>>>


 On Wed, Mar 22, 2017 at 12:40 PM, Alex Deucher <
 alexdeuc...@gmail.com>
 wrote:

>
>
> I guess I'm a little late to the party here, but I haven't had time
> to
> really let all of this sink in and actually look at meson.  It
> doesn't
> seem so bad with a quick look and I think I could probably sort it
> out
> when the time came, but there would still be a bit of a learning
> curve.  While that may not be a big deal at the micro level, I have
> concerns at the macro level.
>
> First, I'm concerned it may discourage casual developers and
> packagers.  autotools isn't great, but most people are familiar
> enough
> with it that they can get by.  Most people have enough knowledge of
> autotools that they can pretty easily diagnose a configuration
> based
> failure. There are a lot of resources for autotools.  I'm not sure
> that would be the case for meson.  Do we as a community feel we
> have
> enough meson experience to get people over the hump?  Anything that
> makes it harder for someone to try a new build or do a bisect is a
> big
> problem in my opinion.
>



 One of the things that's prompted this on our side (I've talked this
 over with
 other people at Intel before starting), was that clearly we *don't*
 know
 autotools well enough to get it right. Emil almost always finds
 cases
 were we've
 done things *almost*, but not quite right.

 For my part, it took me about 3 or 4 days of reading through the
 docs
 and
 writing the libdrm port to get it right, and a lot of that is just
 the
 boilerplate of having ~8 drivers that all need basically the same
 logic.

 Next, my bigger concern is for distro and casual packagers and
> people
> that maintain large build systems with lots of existing custom
> configurations.  Changing from autotools would basically require
> many
> of these existing tools and systems to be rewritten and then deal
> with
> the debugging and fall out from that.  The potential decreased
> build
> time is a nice bonus, but frankly a lot of people/companies have
> years
> of investment in existing tools.
>



 Sure, but we're also not the only ones investigating meson. Gnome is
 using it
 already, libepoxy is using it, gstreamer is using it. There are
 patches
 for
 weston (written by Daniel Stone) and libinput (written by Peter
 Hutterer), there
 are some other projects in the graphics sphere that people are
 working
 on. So
 even if we as a community decide that meson isn't for us, it's not
 going
 away.

>>>
>>>
>>>
>>> It is worth pointing out that it is currently required by no
>>> component
>>> of an x.org stack.  In the case of libepoxy it was added by a new
>>> maintainer
>>> on a new release and even then autoconf remains.
>>>
>>> And as far as I can tell nothing in the entire OpenBSD ports tree
>>> currently requires meson to build including gnome and gstreamer.
>>>
>>>
>> but I guess that is conflating two completely different topics..
>> addition of meson and removal of autotools.  It is probably better
>> that we treat the topics separately.  I don't see any way that the two
>> can happen at the same time.
>>
>> The autotools build probably needs to remain for at least a couple
>> releases, and I certainly wouldn't mind if some of the other desktop
>> projects took the leap of dropping autotools first (at least then
>> various different "distro" consumers will have already dealt with how
>> to build meson packages)
>>
>> None of that blocks addition of a meson build system (or what various
>> developers use day to day)
>>
>> BR,
>> -R
>>
>
>
>
> I tend to disagree.  While we can't avoid a transitory 

Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Jose Fonseca

On 24/03/17 19:10, Kristian Høgsberg wrote:

On Fri, Mar 24, 2017 at 6:42 AM, Jose Fonseca  wrote:

On 23/03/17 01:38, Rob Clark wrote:


On Wed, Mar 22, 2017 at 9:18 PM, Jonathan Gray  wrote:


On Wed, Mar 22, 2017 at 01:10:14PM -0700, Dylan Baker wrote:


On Wed, Mar 22, 2017 at 12:40 PM, Alex Deucher 
wrote:


I guess I'm a little late to the party here, but I haven't had time to
really let all of this sink in and actually look at meson.  It doesn't
seem so bad with a quick look and I think I could probably sort it out
when the time came, but there would still be a bit of a learning
curve.  While that may not be a big deal at the micro level, I have
concerns at the macro level.

First, I'm concerned it may discourage casual developers and
packagers.  autotools isn't great, but most people are familiar enough
with it that they can get by.  Most people have enough knowledge of
autotools that they can pretty easily diagnose a configuration based
failure. There are a lot of resources for autotools.  I'm not sure
that would be the case for meson.  Do we as a community feel we have
enough meson experience to get people over the hump?  Anything that
makes it harder for someone to try a new build or do a bisect is a big
problem in my opinion.



One of the things that's prompted this on our side (I've talked this
over with
other people at Intel before starting), was that clearly we *don't* know
autotools well enough to get it right. Emil almost always finds cases
were we've
done things *almost*, but not quite right.

For my part, it took me about 3 or 4 days of reading through the docs
and
writing the libdrm port to get it right, and a lot of that is just the
boilerplate of having ~8 drivers that all need basically the same logic.


Next, my bigger concern is for distro and casual packagers and people
that maintain large build systems with lots of existing custom
configurations.  Changing from autotools would basically require many
of these existing tools and systems to be rewritten and then deal with
the debugging and fall out from that.  The potential decreased build
time is a nice bonus, but frankly a lot of people/companies have years
of investment in existing tools.



Sure, but we're also not the only ones investigating meson. Gnome is
using it
already, libepoxy is using it, gstreamer is using it. There are patches
for
weston (written by Daniel Stone) and libinput (written by Peter
Hutterer), there
are some other projects in the graphics sphere that people are working
on. So
even if we as a community decide that meson isn't for us, it's not going
away.



It is worth pointing out that it is currently required by no component
of an x.org stack.  In the case of libepoxy it was added by a new
maintainer
on a new release and even then autoconf remains.

And as far as I can tell nothing in the entire OpenBSD ports tree
currently requires meson to build including gnome and gstreamer.



but I guess that is conflating two completely different topics..
addition of meson and removal of autotools.  It is probably better
that we treat the topics separately.  I don't see any way that the two
can happen at the same time.

The autotools build probably needs to remain for at least a couple
releases, and I certainly wouldn't mind if some of the other desktop
projects took the leap of dropping autotools first (at least then
various different "distro" consumers will have already dealt with how
to build meson packages)

None of that blocks addition of a meson build system (or what various
developers use day to day)

BR,
-R



I tend to disagree.  While we can't avoid a transitory period, when we
embark on another build system (Meson or something else) I think we should
aim at 1) ensure such tool can indeed _completely_ replace at least _one_
existing build system, 2) and aim at migration quickly.

Otherwise we'll just end up with yet another build system, yet another way
builds can fail, with some developers stuck on old build systems because it
works, or because the new build system quite doesn't work.

And this is from (painful) experience.


I agree, adding a meson build system should aim to phase out one of
the other build systems within one or two release cycles. But (and
maybe that was Robs point) that doesn't have be autotools. What if we
phase out scons? It doesn't seem like anybody is really attached to it
and if meson is as good as scons on windows, then if nothing else
happens we end up with the same number of build systems. What's more
likely to happen is that a lot of Linux developers (and CI systems)
will also start using meson, which means that life gets easier for
vmware wrt maintaining the build system, and easier for all developers
who have spent to much of their life waiting for autogen.sh.  Then
decommissioning autotools can be a separate topic as Rob suggests,
something we'll do when the world is ready.


There's zero overlap between SCons 

Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Jose Fonseca

On 24/03/17 20:08, Kristian Høgsberg wrote:

On Fri, Mar 24, 2017 at 12:44 PM, Jose Fonseca  wrote:

On 24/03/17 19:10, Kristian Høgsberg wrote:


On Fri, Mar 24, 2017 at 6:42 AM, Jose Fonseca  wrote:


On 23/03/17 01:38, Rob Clark wrote:



On Wed, Mar 22, 2017 at 9:18 PM, Jonathan Gray  wrote:



On Wed, Mar 22, 2017 at 01:10:14PM -0700, Dylan Baker wrote:



On Wed, Mar 22, 2017 at 12:40 PM, Alex Deucher 
wrote:



I guess I'm a little late to the party here, but I haven't had time
to
really let all of this sink in and actually look at meson.  It
doesn't
seem so bad with a quick look and I think I could probably sort it
out
when the time came, but there would still be a bit of a learning
curve.  While that may not be a big deal at the micro level, I have
concerns at the macro level.

First, I'm concerned it may discourage casual developers and
packagers.  autotools isn't great, but most people are familiar
enough
with it that they can get by.  Most people have enough knowledge of
autotools that they can pretty easily diagnose a configuration based
failure. There are a lot of resources for autotools.  I'm not sure
that would be the case for meson.  Do we as a community feel we have
enough meson experience to get people over the hump?  Anything that
makes it harder for someone to try a new build or do a bisect is a
big
problem in my opinion.




One of the things that's prompted this on our side (I've talked this
over with
other people at Intel before starting), was that clearly we *don't*
know
autotools well enough to get it right. Emil almost always finds cases
were we've
done things *almost*, but not quite right.

For my part, it took me about 3 or 4 days of reading through the docs
and
writing the libdrm port to get it right, and a lot of that is just the
boilerplate of having ~8 drivers that all need basically the same
logic.


Next, my bigger concern is for distro and casual packagers and people
that maintain large build systems with lots of existing custom
configurations.  Changing from autotools would basically require many
of these existing tools and systems to be rewritten and then deal
with
the debugging and fall out from that.  The potential decreased build
time is a nice bonus, but frankly a lot of people/companies have
years
of investment in existing tools.




Sure, but we're also not the only ones investigating meson. Gnome is
using it
already, libepoxy is using it, gstreamer is using it. There are
patches
for
weston (written by Daniel Stone) and libinput (written by Peter
Hutterer), there
are some other projects in the graphics sphere that people are working
on. So
even if we as a community decide that meson isn't for us, it's not
going
away.




It is worth pointing out that it is currently required by no component
of an x.org stack.  In the case of libepoxy it was added by a new
maintainer
on a new release and even then autoconf remains.

And as far as I can tell nothing in the entire OpenBSD ports tree
currently requires meson to build including gnome and gstreamer.



but I guess that is conflating two completely different topics..
addition of meson and removal of autotools.  It is probably better
that we treat the topics separately.  I don't see any way that the two
can happen at the same time.

The autotools build probably needs to remain for at least a couple
releases, and I certainly wouldn't mind if some of the other desktop
projects took the leap of dropping autotools first (at least then
various different "distro" consumers will have already dealt with how
to build meson packages)

None of that blocks addition of a meson build system (or what various
developers use day to day)

BR,
-R




I tend to disagree.  While we can't avoid a transitory period, when we
embark on another build system (Meson or something else) I think we
should
aim at 1) ensure such tool can indeed _completely_ replace at least _one_
existing build system, 2) and aim at migration quickly.

Otherwise we'll just end up with yet another build system, yet another
way
builds can fail, with some developers stuck on old build systems because
it
works, or because the new build system quite doesn't work.

And this is from (painful) experience.



I agree, adding a meson build system should aim to phase out one of
the other build systems within one or two release cycles. But (and
maybe that was Robs point) that doesn't have be autotools. What if we
phase out scons? It doesn't seem like anybody is really attached to it
and if meson is as good as scons on windows, then if nothing else
happens we end up with the same number of build systems. What's more
likely to happen is that a lot of Linux developers (and CI systems)
will also start using meson, which means that life gets easier for
vmware wrt maintaining the build system, and easier for all developers
who have spent to much of their life waiting for autogen.sh.  Then
decommissioning 

Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Rob Clark
On Fri, Mar 24, 2017 at 3:10 PM, Kristian Høgsberg  wrote:
> On Fri, Mar 24, 2017 at 6:42 AM, Jose Fonseca  wrote:
>> On 23/03/17 01:38, Rob Clark wrote:
>>>
>>> On Wed, Mar 22, 2017 at 9:18 PM, Jonathan Gray  wrote:

 On Wed, Mar 22, 2017 at 01:10:14PM -0700, Dylan Baker wrote:
>
> On Wed, Mar 22, 2017 at 12:40 PM, Alex Deucher 
> wrote:
>>
>> I guess I'm a little late to the party here, but I haven't had time to
>> really let all of this sink in and actually look at meson.  It doesn't
>> seem so bad with a quick look and I think I could probably sort it out
>> when the time came, but there would still be a bit of a learning
>> curve.  While that may not be a big deal at the micro level, I have
>> concerns at the macro level.
>>
>> First, I'm concerned it may discourage casual developers and
>> packagers.  autotools isn't great, but most people are familiar enough
>> with it that they can get by.  Most people have enough knowledge of
>> autotools that they can pretty easily diagnose a configuration based
>> failure. There are a lot of resources for autotools.  I'm not sure
>> that would be the case for meson.  Do we as a community feel we have
>> enough meson experience to get people over the hump?  Anything that
>> makes it harder for someone to try a new build or do a bisect is a big
>> problem in my opinion.
>
>
> One of the things that's prompted this on our side (I've talked this
> over with
> other people at Intel before starting), was that clearly we *don't* know
> autotools well enough to get it right. Emil almost always finds cases
> were we've
> done things *almost*, but not quite right.
>
> For my part, it took me about 3 or 4 days of reading through the docs
> and
> writing the libdrm port to get it right, and a lot of that is just the
> boilerplate of having ~8 drivers that all need basically the same logic.
>
>> Next, my bigger concern is for distro and casual packagers and people
>> that maintain large build systems with lots of existing custom
>> configurations.  Changing from autotools would basically require many
>> of these existing tools and systems to be rewritten and then deal with
>> the debugging and fall out from that.  The potential decreased build
>> time is a nice bonus, but frankly a lot of people/companies have years
>> of investment in existing tools.
>
>
> Sure, but we're also not the only ones investigating meson. Gnome is
> using it
> already, libepoxy is using it, gstreamer is using it. There are patches
> for
> weston (written by Daniel Stone) and libinput (written by Peter
> Hutterer), there
> are some other projects in the graphics sphere that people are working
> on. So
> even if we as a community decide that meson isn't for us, it's not going
> away.


 It is worth pointing out that it is currently required by no component
 of an x.org stack.  In the case of libepoxy it was added by a new
 maintainer
 on a new release and even then autoconf remains.

 And as far as I can tell nothing in the entire OpenBSD ports tree
 currently requires meson to build including gnome and gstreamer.

>>>
>>> but I guess that is conflating two completely different topics..
>>> addition of meson and removal of autotools.  It is probably better
>>> that we treat the topics separately.  I don't see any way that the two
>>> can happen at the same time.
>>>
>>> The autotools build probably needs to remain for at least a couple
>>> releases, and I certainly wouldn't mind if some of the other desktop
>>> projects took the leap of dropping autotools first (at least then
>>> various different "distro" consumers will have already dealt with how
>>> to build meson packages)
>>>
>>> None of that blocks addition of a meson build system (or what various
>>> developers use day to day)
>>>
>>> BR,
>>> -R
>>
>>
>> I tend to disagree.  While we can't avoid a transitory period, when we
>> embark on another build system (Meson or something else) I think we should
>> aim at 1) ensure such tool can indeed _completely_ replace at least _one_
>> existing build system, 2) and aim at migration quickly.
>>
>> Otherwise we'll just end up with yet another build system, yet another way
>> builds can fail, with some developers stuck on old build systems because it
>> works, or because the new build system quite doesn't work.
>>
>> And this is from (painful) experience.
>
> I agree, adding a meson build system should aim to phase out one of
> the other build systems within one or two release cycles. But (and
> maybe that was Robs point) that doesn't have be autotools. What if we
> phase out scons? It doesn't seem like anybody is really attached to it
> and if meson is as good as 

Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Chad Versace
On Tue 21 Mar 2017, Matt Turner wrote:
> On Tue, Mar 21, 2017 at 10:16 AM, Emil Velikov  
> wrote:
> > On 21 March 2017 at 15:57, Matt Turner  wrote:
> >> On Mon, Mar 20, 2017 at 12:39 PM, Emil Velikov  
> >> wrote:
> >>> On 20 March 2017 at 18:30, Matt Turner  wrote:
>  On Mon, Mar 20, 2017 at 6:55 AM, Emil Velikov  
>  wrote:


> Let me try one last time:
> 
> (1) Non-recursive automake is necessary for parallel build performance
> (2) Non-recursive automake is intractably unmaintainable for
> sufficiently large projects
> (3) Mesa is a sufficiently large project
> 
> Therefore using automake will be either bad for parallel build
> performance, unmaintainable, or both.
> 
> Meson aims to be a build system actually capable of replacing
> autotools (IMO unlike cmake, scons, waf, et al.). It offers a much
> cleaner domain specific language for writing the build rules, while
> generating non-recursive ninja build files. It does not use libtool.
> It supports Windows. It supports cross compilation.
> 
> And it has momentum: libepoxy already has a Meson build system. Others
> in the X.Org community are experimenting with it for libinput, Wayland
> and Weston, and the xserver.
> 
> All of that makes Meson very compelling.

Matt, I just wanted to say thanks for capturing in a nutshell the
argument for a Meson trial.

I want Meson because my builds are too slow. I regularly build an entire
fucking operating system from scratch (Chrome OS), and I cry when top
shows only 1 core at 100% and the other 47 cores sitting idle.

Autotools and libtool are often the culprit. The libtool penalty is paid
on every build, not just at configure time.

The only way to get all 48 cores closer to 100% is to (1) use
a non-recursive build system that (2) doesnt' use shell nor (3) libtool.

I don't care about new shiny. I'm often wary of new shiny things that
receive too much excitement.  Meson is a such a new shiny. But I'm
willing to overlook my aversion to new shiny Meson's immaturity if
it makes Chrome OS build faster... at least for Mesa.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v4.1 07/28] i965/fs: generalize the legalization d2x pass

2017-03-24 Thread Francisco Jerez
Samuel Iglesias Gonsálvez  writes:

> Generalize it to lower any unsupported narrower conversion.
>
> v2 (Curro):
> - Add supports_type_conversion()
> - Reuse existing intruction instead of cloning it.
> - Generalize d2x to narrower and equal size conversions.
>
> v3 (Curro):
> - Make supports_type_conversion() const and improve it.
> - Use foreach_block_and_inst to process added instructions.
> - Simplify code.
> - Add assert and improve comments.
> - Remove redundant mov.
> - Remove useless comment.
> - Remove saturate == false assert and add support for saturation
>   when fixing the conversion.
> - Add get_exec_type() function.
>
> Signed-off-by: Samuel Iglesias Gonsálvez 
> ---
>
> This patch replaces the respective v3 one.
>
>  src/intel/compiler/brw_fs.cpp   |  11 +--
>  src/intel/compiler/brw_fs_lower_d2x.cpp | 117 
> +++-
>  2 files changed, 91 insertions(+), 37 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
> index 086b1a04855..8eb8789905c 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -5694,11 +5694,6 @@ fs_visitor::optimize()
>OPT(dead_code_eliminate);
> }
>  
> -   if (OPT(lower_d2x)) {
> -  OPT(opt_copy_propagation);
> -  OPT(dead_code_eliminate);
> -   }
> -
> OPT(lower_simd_width);
>  
> /* After SIMD lowering just in case we had to unroll the EOT send. */
> @@ -5745,6 +5740,12 @@ fs_visitor::optimize()
>OPT(dead_code_eliminate);
> }
>  
> +   if (OPT(lower_d2x)) {
> +  OPT(opt_copy_propagation);
> +  OPT(dead_code_eliminate);
> +  OPT(lower_simd_width);
> +   }
> +
> lower_uniform_pull_constant_loads();
>  
> validate();
> diff --git a/src/intel/compiler/brw_fs_lower_d2x.cpp 
> b/src/intel/compiler/brw_fs_lower_d2x.cpp
> index a2db1154615..9bc78fa969b 100644
> --- a/src/intel/compiler/brw_fs_lower_d2x.cpp
> +++ b/src/intel/compiler/brw_fs_lower_d2x.cpp
> @@ -27,48 +27,101 @@
>  
>  using namespace brw;
>  
> +static unsigned

The return type is not really an integer.

> +get_exec_type(const fs_inst *inst)
> +{
> +   unsigned exec_type = 0;

This isn't either.  You could initialize this to BRW_REGISTER_TYPE_B
which isn't a valid execution type so you can drop the whole
has_valid_source tracking.

> +   bool has_valid_source = false;
> +
> +   for (int i = 0; i < inst->sources; i++) {
> +  if (inst->src[i].type != BAD_FILE) {
> + if (!has_valid_source) {
> +exec_type = inst->src[i].type;
> +has_valid_source = true;
> + } else {
> +if (type_sz(inst->src[i].type) > type_sz(exec_type))
> +exec_type = inst->src[i].type;

Note that this doesn't handle vector immediates correctly (which give
you either an F or W execution type), byte nor unsigned source types.  I
suggest you add a get_exec_type(brw_reg_type) overload you'd call here
to translate a source type into the matching exec type.

To handle cases where you have mixed floating point and integer types it
would probably be sensible to do something along the lines of:

| const brw_reg_type t = get_exec_type(inst->src[i].type);
| // ...
| if (type_sz(t) == type_sz(exec_type) && is_floating_point(t))
| exec_type = t;

> + }
> +  }
> +   }
> +
> +   /* FIXME: if this assert fails, then the instruction has no valid sources 
> */
> +   assert(has_valid_source);
> +

You could assert 'exec_type != BRW_REGISTER_TYPE_HF || inst->dst.type ==
BRW_REGISTER_TYPE_HF' so we remember to handle half-float conversions
here when they are enabled in the back-end.

> +   return exec_type;

With this in place the get_exec_type_size(inst) helper seems redundant
since it should be equivalent to type_sz(get_exec_type(inst)).  I think
this should replace PATCH 3.

> +}
> +
> +static bool
> +supports_type_conversion(const fs_inst *inst) {
> +   switch(inst->opcode) {
> +   case BRW_OPCODE_MOV:
> +   case SHADER_OPCODE_MOV_INDIRECT:
> +  return true;
> +   case BRW_OPCODE_SEL:
> +  return inst->dst.type == get_exec_type(inst);
> +   default:
> +  /* FIXME: We assume the opcodes don't explicitly mentioned
> +   * before just work fine with arbitrary conversions.
> +   */
> +  return true;
> +   }
> +}
> +
>  bool
>  fs_visitor::lower_d2x()
>  {
> bool progress = false;
>  
> -   foreach_block_and_inst_safe(block, fs_inst, inst, cfg) {
> -  if (inst->opcode != BRW_OPCODE_MOV)
> - continue;
> -
> -  if (inst->dst.type != BRW_REGISTER_TYPE_F &&
> -  inst->dst.type != BRW_REGISTER_TYPE_D &&
> -  inst->dst.type != BRW_REGISTER_TYPE_UD)
> - continue;
> +   foreach_block_and_inst(block, fs_inst, inst, cfg) {
> +  const fs_builder ibld(this, block, inst);
> +  fs_reg dst = inst->dst;
> +  bool saturate = inst->saturate;
>  
> -  if (inst->src[0].type != BRW_REGISTER_TYPE_DF &&
> -

Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Chad Versace
On Tue 21 Mar 2017, Matt Turner wrote:
> On Mon, Mar 20, 2017 at 12:39 PM, Emil Velikov  
> wrote:
> > On 20 March 2017 at 18:30, Matt Turner  wrote:
> >> On Mon, Mar 20, 2017 at 6:55 AM, Emil Velikov  
> >> wrote:

> >>> These projects have been getting closer to upstream and "forcing" the
> >>> extra obstacle is effectively giving them the middle finger.
> >>
> >> No. Requiring us to compile with a 10 year old GCC is giving a middle 
> >> finger.
> >>
> > Can we stop with the GCC thing ? Can we point to a place where we want
> > to use feature A introduced with GCC B and we don't ?
> 
> Are you freaking serious?!
> 
> This happens *all* the time. It happened like two days ago with commit
> 28b134c75c1fa3b2aaa00dc168f0eca35ccd346d. I'm sure it probably
> happened at least once in the previous month, and every month before
> that.

More examples:

- Jason and I wanted to use C11 generic expressions (that's what the
  C11 spec calls them) in anvil, but old GCC => !C11.

- I *still* want to use _Generic.

- If we could use C11, then we could stop using the
  include/c11/thread.h wrapper for the C11 thread features. We could use,
  you know, *real* C11 threads instead of faking it.

- I want to do this:

  #define let __auto_type

  But __auto_type requires GCC 4.9
   or a comparably dated clang.

- I want to use GCC's builtin overflow arithmetic functions
   (such 
as
  __builtin_mul_overflow(), __builtin_add_overflow) where we currently do 
overflow checking by hand.
  The builtin functions are more secure (no chance of stupid mistakes) and
  faster (they simply do the arithmetic op then test the overflow flag in 
the
  CPU's status register).

- I tend to be guilty occasionally breaking the build in
  anvil code, due to old GCC. I think it happened again again this week:

freenode.#dri-devel.log-2017-03-16 11:07:12 | imirkin_ | 
vulkan/anv_device.c:697:4: error: initializer element is not constant
freenode.#dri-devel.log-2017-03-16 11:07:12 | imirkin_ | 
.minImageTransferGranularity = (VkExtent3D) { 1, 1, 1 },
freenode.#dri-devel.log-2017-03-16 11:07:23 | imirkin_ | anyone seen 
this? i'm on HEAD
freenode.#dri-devel.log-2017-03-16 11:13:50 | vsyrjala | yep. gcc-4.9 
strikes again?
freenode.#dri-devel.log-2017-03-16 11:14:54 | imirkin_ | i'm definitely 
using gcc-4.9
freenode.#dri-devel.log-2017-03-16 11:15:16 | vsyrjala | that 
'(VkExtent3D)' looks very much pointless there
freenode.#dri-devel.log-2017-03-16 11:15:56 | imirkin_ | 4.9.4 as it 
happens, which is the "stable" gcc on gentoo
freenode.#dri-devel.log:2017-03-16 11:18:04 | vsyrjala | looks like 
chadv broke it
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] anv: enable sampling from fast-cleared images on SKL

2017-03-24 Thread Nanley Chery
On Mar 23, 2017 11:00 PM, "Samuel Iglesias Gonsálvez" 
wrote:

On Thu, 2017-03-23 at 11:09 -0700, Nanley Chery wrote:
> On Thu, Mar 23, 2017 at 12:25:28PM +0100, Samuel Iglesias Gonsálvez
> wrote:
> > A resolve is not needed on Skylake in this case. We were forcing
> > a resolve because we set the input_aux_usage to ISL_AUX_USAGE_NONE.
> >
> > Signed-off-by: Samuel Iglesias Gonsálvez 
> > ---
> >
> > This doesn't fix the problem with BDW but I found it while
> > reviewing
> > the code.
> >
> >  src/intel/vulkan/genX_cmd_buffer.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > b/src/intel/vulkan/genX_cmd_buffer.c
> > index e2364dbfd52..39856b9af7c 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -305,8 +305,8 @@ color_attachment_compute_aux_usage(struct
> > anv_device *device,
> >* doesn't also support color compression.
> >*/
> >   att_state->input_aux_usage = ISL_AUX_USAGE_NONE;
> > -  } else if (GEN_GEN == 8) {
> > - /* Broadwell can sample from fast-cleared images */
> > +  } else if (GEN_GEN >= 8) {
> > + /* Broadwell/Skylake can sample from fast-cleared images
> > */
> >   att_state->input_aux_usage = ISL_AUX_USAGE_CCS_D;
>
> This doesn't work in all cases. SKL can only sample from CCS_D if the
> format is supported for CCS_E.
>

It is already covered. The 'if' condition before this is triggered only
if gen9+ and format doesn't support CCS_E, so if you end up here in
gen9+ is because the format supports CCS_E.


You're right. Sorry about that. Let's go with your​ patch.
Reviewed-by: Nanley Chery 


> By the way, I actually have a work-in-progress branch that has a
> patch to do this:
> https://cgit.freedesktop.org/~nchery/mesa/commit/?h=ccs-layouts/1-ccs
> d-layout=e428f44e1835cb00f52848d09350957ef6f73495
>

This is a similar solution to this bug. If you prefer to stick with
yours, please add my R-b to it:

Reviewed-by: Samuel Iglesias Gonsálvez 

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


[Mesa-dev] [Bug 99591] Segmentation fault when running vulkaninfo with RADV Radeon Vulkan driver

2017-03-24 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=99591

Luiz Angelo Daros de Luca  changed:

   What|Removed |Added

 CC||luizl...@gmail.com

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


Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Kristian Høgsberg
On Fri, Mar 24, 2017 at 12:44 PM, Jose Fonseca  wrote:
> On 24/03/17 19:10, Kristian Høgsberg wrote:
>>
>> On Fri, Mar 24, 2017 at 6:42 AM, Jose Fonseca  wrote:
>>>
>>> On 23/03/17 01:38, Rob Clark wrote:


 On Wed, Mar 22, 2017 at 9:18 PM, Jonathan Gray  wrote:
>
>
> On Wed, Mar 22, 2017 at 01:10:14PM -0700, Dylan Baker wrote:
>>
>>
>> On Wed, Mar 22, 2017 at 12:40 PM, Alex Deucher 
>> wrote:
>>>
>>>
>>> I guess I'm a little late to the party here, but I haven't had time
>>> to
>>> really let all of this sink in and actually look at meson.  It
>>> doesn't
>>> seem so bad with a quick look and I think I could probably sort it
>>> out
>>> when the time came, but there would still be a bit of a learning
>>> curve.  While that may not be a big deal at the micro level, I have
>>> concerns at the macro level.
>>>
>>> First, I'm concerned it may discourage casual developers and
>>> packagers.  autotools isn't great, but most people are familiar
>>> enough
>>> with it that they can get by.  Most people have enough knowledge of
>>> autotools that they can pretty easily diagnose a configuration based
>>> failure. There are a lot of resources for autotools.  I'm not sure
>>> that would be the case for meson.  Do we as a community feel we have
>>> enough meson experience to get people over the hump?  Anything that
>>> makes it harder for someone to try a new build or do a bisect is a
>>> big
>>> problem in my opinion.
>>
>>
>>
>> One of the things that's prompted this on our side (I've talked this
>> over with
>> other people at Intel before starting), was that clearly we *don't*
>> know
>> autotools well enough to get it right. Emil almost always finds cases
>> were we've
>> done things *almost*, but not quite right.
>>
>> For my part, it took me about 3 or 4 days of reading through the docs
>> and
>> writing the libdrm port to get it right, and a lot of that is just the
>> boilerplate of having ~8 drivers that all need basically the same
>> logic.
>>
>>> Next, my bigger concern is for distro and casual packagers and people
>>> that maintain large build systems with lots of existing custom
>>> configurations.  Changing from autotools would basically require many
>>> of these existing tools and systems to be rewritten and then deal
>>> with
>>> the debugging and fall out from that.  The potential decreased build
>>> time is a nice bonus, but frankly a lot of people/companies have
>>> years
>>> of investment in existing tools.
>>
>>
>>
>> Sure, but we're also not the only ones investigating meson. Gnome is
>> using it
>> already, libepoxy is using it, gstreamer is using it. There are
>> patches
>> for
>> weston (written by Daniel Stone) and libinput (written by Peter
>> Hutterer), there
>> are some other projects in the graphics sphere that people are working
>> on. So
>> even if we as a community decide that meson isn't for us, it's not
>> going
>> away.
>
>
>
> It is worth pointing out that it is currently required by no component
> of an x.org stack.  In the case of libepoxy it was added by a new
> maintainer
> on a new release and even then autoconf remains.
>
> And as far as I can tell nothing in the entire OpenBSD ports tree
> currently requires meson to build including gnome and gstreamer.
>

 but I guess that is conflating two completely different topics..
 addition of meson and removal of autotools.  It is probably better
 that we treat the topics separately.  I don't see any way that the two
 can happen at the same time.

 The autotools build probably needs to remain for at least a couple
 releases, and I certainly wouldn't mind if some of the other desktop
 projects took the leap of dropping autotools first (at least then
 various different "distro" consumers will have already dealt with how
 to build meson packages)

 None of that blocks addition of a meson build system (or what various
 developers use day to day)

 BR,
 -R
>>>
>>>
>>>
>>> I tend to disagree.  While we can't avoid a transitory period, when we
>>> embark on another build system (Meson or something else) I think we
>>> should
>>> aim at 1) ensure such tool can indeed _completely_ replace at least _one_
>>> existing build system, 2) and aim at migration quickly.
>>>
>>> Otherwise we'll just end up with yet another build system, yet another
>>> way
>>> builds can fail, with some developers stuck on old build systems because
>>> it
>>> works, or because the new build system quite doesn't work.
>>>
>>> And this is from (painful) experience.
>>
>>
>> I agree, adding 

Re: [Mesa-dev] [Mesa-stable] [PATCH] i965/fs: Don't emit SEL instructions for type-converting MOVs.

2017-03-24 Thread Francisco Jerez
Matt Turner  writes:

> On Fri, Mar 24, 2017 at 12:06 AM, Francisco Jerez  
> wrote:
>> Samuel Iglesias Gonsálvez  writes:
>>
>>> On Thu, 2017-03-23 at 13:50 -0700, Matt Turner wrote:
 SEL can only convert between a few integer types, which we basically
 never do.

 Fixes fs/vs-double-uniform-array-direct-indirect-non-uniform-control-
 flow
 Cc: mesa-sta...@lists.freedesktop.org
>>>
>>> I sent a similar but wrong patch (taking only into account the type
>>> size) some time ago, but after discussing it with Curro, the solution
>>> was to fix it inside d2x pass. This is what this patch "i965/fs:
>>> generalize the legalization d2x pass" does.
>>>
>>> I am still working on improving that patch but I expect to have
>>> something soon. If you prefer to land this now, please add my R-b but
>>> you probably want to discuss it with Curro before:
>>>
>>> Reviewed-by: Samuel Iglesias Gonsálvez 
>>>
>>
>> Samuel's d2x patch has the advantage that it will allow the SEL peephole
>> to replace control flow with SEL instructions even where there is a type
>> conversion.  That said this patch shouldn't hurt in mesa-stable in the
>> meantime if we remember to revert it in master when Samuel's patch
>> lands.  Patch is:
>>
>> Acked-by: Francisco Jerez 
>
> Oh, I didn't realize that pass was going to handle instructions not
> operating on DF types. That's surprising given its name.

Yeah, he's also sent a patch to rename it.

>
> To confirm: with that pass in place it should be save to do a
> type-converting SEL (on, say, integer sources and a float destination)
> in the IR?
>

Yes, in principle it should be safe for the optimizer to introduce type
conversions of any kind, although at this point the lower_conversions
pass only handles MOV, MOV_INDIRECT and SEL opcodes it should be
straightforward to extend it to handle the type conversion restrictions
of any instruction.

> If that's the case, I'll delay committing this patch until lower_d2x
> is committed, so that we don't have to remember to revert my patch,
> and there's no chance of bugs being fixed on the stable branch but not
> in master.

I guess if you commit it to master already there's no chance of it not
getting fixed in master, the only concern is that we'll end up with both
fixes checked in forever.  Samuel, would you mind including a revert of
this change as PATCH 7.5 of your FP64 series?


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


[Mesa-dev] [PATCH v4 1/1] clover: use pipe_resource references

2017-03-24 Thread Jan Vesely
v2: buffers are created with one reference.
v3: add pipe_resource reference to mapping object
v4: rename to pres and drop inline initializers

CC: "17.0 13.0" 

Signed-off-by: Jan Vesely 
---
resend to include mesa-dev ML
sorry for spamming

jan

 src/gallium/state_trackers/clover/core/resource.cpp | 11 ---
 src/gallium/state_trackers/clover/core/resource.hpp |  1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/gallium/state_trackers/clover/core/resource.cpp 
b/src/gallium/state_trackers/clover/core/resource.cpp
index 06fd3f6..28fba1a 100644
--- a/src/gallium/state_trackers/clover/core/resource.cpp
+++ b/src/gallium/state_trackers/clover/core/resource.cpp
@@ -25,6 +25,7 @@
 #include "pipe/p_screen.h"
 #include "util/u_sampler.h"
 #include "util/u_format.h"
+#include "util/u_inlines.h"
 
 using namespace clover;
 
@@ -176,7 +177,7 @@ root_resource::root_resource(clover::device , 
memory_obj ,
 }
 
 root_resource::~root_resource() {
-   device().pipe->resource_destroy(device().pipe, pipe);
+   pipe_resource_reference(>pipe, NULL);
 }
 
 sub_resource::sub_resource(resource , const vector ) :
@@ -189,7 +190,7 @@ mapping::mapping(command_queue , resource ,
  cl_map_flags flags, bool blocking,
  const resource::vector ,
  const resource::vector ) :
-   pctx(q.pipe) {
+   pctx(q.pipe), pres(NULL) {
unsigned usage = ((flags & CL_MAP_WRITE ? PIPE_TRANSFER_WRITE : 0 ) |
  (flags & CL_MAP_READ ? PIPE_TRANSFER_READ : 0 ) |
  (flags & CL_MAP_WRITE_INVALIDATE_REGION ?
@@ -202,12 +203,14 @@ mapping::mapping(command_queue , resource ,
   pxfer = NULL;
   throw error(CL_OUT_OF_RESOURCES);
}
+   pipe_resource_reference(, r.pipe);
 }
 
 mapping::mapping(mapping &) :
-   pctx(m.pctx), pxfer(m.pxfer), p(m.p) {
+   pctx(m.pctx), pxfer(m.pxfer), pres(m.pres), p(m.p) {
m.pctx = NULL;
m.pxfer = NULL;
+   m.pres = NULL;
m.p = NULL;
 }
 
@@ -215,12 +218,14 @@ mapping::~mapping() {
if (pxfer) {
   pctx->transfer_unmap(pctx, pxfer);
}
+   pipe_resource_reference(, NULL);
 }
 
 mapping &
 mapping::operator=(mapping m) {
std::swap(pctx, m.pctx);
std::swap(pxfer, m.pxfer);
+   std::swap(pres, m.pres);
std::swap(p, m.p);
return *this;
 }
diff --git a/src/gallium/state_trackers/clover/core/resource.hpp 
b/src/gallium/state_trackers/clover/core/resource.hpp
index 9993dcb..3b994b4 100644
--- a/src/gallium/state_trackers/clover/core/resource.hpp
+++ b/src/gallium/state_trackers/clover/core/resource.hpp
@@ -125,6 +125,7 @@ namespace clover {
private:
   pipe_context *pctx;
   pipe_transfer *pxfer;
+  pipe_resource *pres;
   void *p;
};
 }
-- 
2.9.3

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] i965/fs: Don't emit SEL instructions for type-converting MOVs.

2017-03-24 Thread Matt Turner
On Fri, Mar 24, 2017 at 12:06 AM, Francisco Jerez  wrote:
> Samuel Iglesias Gonsálvez  writes:
>
>> On Thu, 2017-03-23 at 13:50 -0700, Matt Turner wrote:
>>> SEL can only convert between a few integer types, which we basically
>>> never do.
>>>
>>> Fixes fs/vs-double-uniform-array-direct-indirect-non-uniform-control-
>>> flow
>>> Cc: mesa-sta...@lists.freedesktop.org
>>
>> I sent a similar but wrong patch (taking only into account the type
>> size) some time ago, but after discussing it with Curro, the solution
>> was to fix it inside d2x pass. This is what this patch "i965/fs:
>> generalize the legalization d2x pass" does.
>>
>> I am still working on improving that patch but I expect to have
>> something soon. If you prefer to land this now, please add my R-b but
>> you probably want to discuss it with Curro before:
>>
>> Reviewed-by: Samuel Iglesias Gonsálvez 
>>
>
> Samuel's d2x patch has the advantage that it will allow the SEL peephole
> to replace control flow with SEL instructions even where there is a type
> conversion.  That said this patch shouldn't hurt in mesa-stable in the
> meantime if we remember to revert it in master when Samuel's patch
> lands.  Patch is:
>
> Acked-by: Francisco Jerez 

Oh, I didn't realize that pass was going to handle instructions not
operating on DF types. That's surprising given its name.

To confirm: with that pass in place it should be save to do a
type-converting SEL (on, say, integer sources and a float destination)
in the IR?

If that's the case, I'll delay committing this patch until lower_d2x
is committed, so that we don't have to remember to revert my patch,
and there's no chance of bugs being fixed on the stable branch but not
in master.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965: Require Kernel 3.6 for Gen4-5 platforms.

2017-03-24 Thread Matt Turner
On Wed, Mar 22, 2017 at 3:20 PM, Kenneth Graunke  wrote:
> We've already required Kernel 3.6 on Gen6+ since Mesa 9.2 (May 2013,
> commit 92d2f5acfadea672417b6785710c9e8b7f605e41).  It seems reasonable
> to require it for Gen4-5 as well, bumping the requirement from 2.6.39.
>
> This is necessary for glClientWaitSync with a timeout to work, which
> is a feature we expose on Gen4-5.  Without it, we would fall back to an
> infinite wait, which is pretty bad.
>
> See kernel commit 172cf15d18889313bf2c3bfb81fcea08369274ef in 3.6+.

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


Re: [Mesa-dev] [PATCH] i965: Remove pointless NULL check from Gen6 primitive counting code.

2017-03-24 Thread Matt Turner
Reviewed-by: Matt Turner 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Kristian Høgsberg
On Fri, Mar 24, 2017 at 6:42 AM, Jose Fonseca  wrote:
> On 23/03/17 01:38, Rob Clark wrote:
>>
>> On Wed, Mar 22, 2017 at 9:18 PM, Jonathan Gray  wrote:
>>>
>>> On Wed, Mar 22, 2017 at 01:10:14PM -0700, Dylan Baker wrote:

 On Wed, Mar 22, 2017 at 12:40 PM, Alex Deucher 
 wrote:
>
> I guess I'm a little late to the party here, but I haven't had time to
> really let all of this sink in and actually look at meson.  It doesn't
> seem so bad with a quick look and I think I could probably sort it out
> when the time came, but there would still be a bit of a learning
> curve.  While that may not be a big deal at the micro level, I have
> concerns at the macro level.
>
> First, I'm concerned it may discourage casual developers and
> packagers.  autotools isn't great, but most people are familiar enough
> with it that they can get by.  Most people have enough knowledge of
> autotools that they can pretty easily diagnose a configuration based
> failure. There are a lot of resources for autotools.  I'm not sure
> that would be the case for meson.  Do we as a community feel we have
> enough meson experience to get people over the hump?  Anything that
> makes it harder for someone to try a new build or do a bisect is a big
> problem in my opinion.


 One of the things that's prompted this on our side (I've talked this
 over with
 other people at Intel before starting), was that clearly we *don't* know
 autotools well enough to get it right. Emil almost always finds cases
 were we've
 done things *almost*, but not quite right.

 For my part, it took me about 3 or 4 days of reading through the docs
 and
 writing the libdrm port to get it right, and a lot of that is just the
 boilerplate of having ~8 drivers that all need basically the same logic.

> Next, my bigger concern is for distro and casual packagers and people
> that maintain large build systems with lots of existing custom
> configurations.  Changing from autotools would basically require many
> of these existing tools and systems to be rewritten and then deal with
> the debugging and fall out from that.  The potential decreased build
> time is a nice bonus, but frankly a lot of people/companies have years
> of investment in existing tools.


 Sure, but we're also not the only ones investigating meson. Gnome is
 using it
 already, libepoxy is using it, gstreamer is using it. There are patches
 for
 weston (written by Daniel Stone) and libinput (written by Peter
 Hutterer), there
 are some other projects in the graphics sphere that people are working
 on. So
 even if we as a community decide that meson isn't for us, it's not going
 away.
>>>
>>>
>>> It is worth pointing out that it is currently required by no component
>>> of an x.org stack.  In the case of libepoxy it was added by a new
>>> maintainer
>>> on a new release and even then autoconf remains.
>>>
>>> And as far as I can tell nothing in the entire OpenBSD ports tree
>>> currently requires meson to build including gnome and gstreamer.
>>>
>>
>> but I guess that is conflating two completely different topics..
>> addition of meson and removal of autotools.  It is probably better
>> that we treat the topics separately.  I don't see any way that the two
>> can happen at the same time.
>>
>> The autotools build probably needs to remain for at least a couple
>> releases, and I certainly wouldn't mind if some of the other desktop
>> projects took the leap of dropping autotools first (at least then
>> various different "distro" consumers will have already dealt with how
>> to build meson packages)
>>
>> None of that blocks addition of a meson build system (or what various
>> developers use day to day)
>>
>> BR,
>> -R
>
>
> I tend to disagree.  While we can't avoid a transitory period, when we
> embark on another build system (Meson or something else) I think we should
> aim at 1) ensure such tool can indeed _completely_ replace at least _one_
> existing build system, 2) and aim at migration quickly.
>
> Otherwise we'll just end up with yet another build system, yet another way
> builds can fail, with some developers stuck on old build systems because it
> works, or because the new build system quite doesn't work.
>
> And this is from (painful) experience.

I agree, adding a meson build system should aim to phase out one of
the other build systems within one or two release cycles. But (and
maybe that was Robs point) that doesn't have be autotools. What if we
phase out scons? It doesn't seem like anybody is really attached to it
and if meson is as good as scons on windows, then if nothing else
happens we end up with the same number of build systems. What's more
likely to happen is that a lot of Linux developers (and CI systems)
will also start 

Re: [Mesa-dev] [PATCH] i965: Fix symbolic size of next_offset[] array.

2017-03-24 Thread Matt Turner
Reviewed-by: Matt Turner 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 6/9] isl: Validate the calculated row pitch (v2)

2017-03-24 Thread Emil Velikov
On 24 March 2017 at 18:02, Chad Versace  wrote:
> On Fri 24 Mar 2017, Emil Velikov wrote:
>> Hi Chad,
>>
>> On 23 March 2017 at 01:04, Chad Versace  wrote:
>> > Validate that isl_surf::row_pitch fits in the below bitfields,
>> > if applicable based on isl_surf::usage.
>> >
>> > RENDER_SURFACE_STATE::SurfacePitch
>> > RENDER_SURFACE_STATE::AuxiliarySurfacePitch
>> > 3DSTATE_DEPTH_BUFFER::SurfacePitch
>> > 3DSTATE_HIER_DEPTH_BUFFER::SurfacePitch
>> >
>> > v2: Add a Makefile dependency on generated header genX_bits.h.
>> > ---
>> >  src/intel/Makefile.isl.am |  3 ++
>> >  src/intel/isl/isl.c   | 72 
>> > +++
>> >  2 files changed, 69 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/src/intel/Makefile.isl.am b/src/intel/Makefile.isl.am
>> > index ee2215df1d1..09a10281b45 100644
>> > --- a/src/intel/Makefile.isl.am
>> > +++ b/src/intel/Makefile.isl.am
>> > @@ -63,6 +63,9 @@ isl/isl_format_layout.c: isl/gen_format_layout.py \
>> > $(PYTHON_GEN) $(srcdir)/isl/gen_format_layout.py \
>> > --csv $(srcdir)/isl/isl_format_layout.csv --out $@
>> >
>> > +# Dependencies on generated files
>> > +$(builddir)/isl/isl.o: $(srcdir)/genxml/genX_bits.h
>> > +
>> Can we have this as below. We could also squash it with 3/9.
>>
>> BUILT_SOURCES += genxml/genX_bits.h
>> EXTRA_DIST += genxml/genX_bits.h
>
> Patch 3/9, in Makefile.gexml.am, does
>   GENXML_GENERATED_FILES += genxml/genX_bits.h
> which indirectly accomplishes
>   BUILT_SOURCES += genxml/genX_bits.h
>   EXTRA_DIST += genxml/genX_bits.h
>
Right i got confused by the simultaneous rename + adding the new file.

> Do you me want to drop this?
>   $(builddir)/isl/isl.o: $(srcdir)/genxml/genX_bits.h
> I'll drop it if you want.
>
That'll be great, thank you.

> I don't understand autoconf...  I added the dependency because I was
> unsure how BUILT_SOURCES worked.

All the files listed in BUILT_SOURCES, through the whole tree, are
generated before any compilation has started.

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


Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Daniel Stone
Hi,

On 24 March 2017 at 17:51, Eric Anholt  wrote:
> Dylan Baker  writes:
>> I also think it's worth talking to Eric (who said he's porting X to meson),
>> Daniel Stone (who has patches to port weston to meson), and Peter Hutterer 
>> (who
>> has patches to port libinput to meson). If they're serious about seeing those
>> land meson is even more appealing, since it would be a single build system 
>> that
>> all of the *nix graphics stack would be moving towards, and would mean that 
>> we
>> wouldn't have an "Autotools for xorg", "meson for weston and libinput", 
>> "cmake for
>> piglit", and " for mesa".
>
> My desire is to push enough of X.org to Meson that I can actually do CI
> of the X Server.  Right now CI is not really tractable on Travis because
> autogen.sh of all the dependencies takes too long.

... and I'm equally serious about Wayland/Weston. I find how slow it
is infuriating for personal development, but for CI, it means that
there's no point trying to test different build options, since the
overhead is so high. Meson would let us actually do that.

I understand there's a huge amount of acquired folk knowledge we're
throwing away with autotools, and in the ~13 years since I started
working on moving X.Org to autotools, I've evaluated and discarded
most of the others because I didn't think they were sufficiently
compelling to do so. In terms of its featureset,
accessibility/tractability to mere mortals, etc, Meson is the only one
I've found where the advantages of moving outweighed the
disadvantages.

I've been pretty buried in atomic work lately, but am going to send
out a revised patchset for Wayland & Weston after I've got the next
iteration out. Similarly I'd expect to see one or two releases
shipping with both build systems, whilst we work with downstreams to
beat any bugs out of the new system before cutting over.

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


Re: [Mesa-dev] [PATCH 6/9] isl: Validate the calculated row pitch (v2)

2017-03-24 Thread Chad Versace
On Thu 23 Mar 2017, Jason Ekstrand wrote:
> On Wed, Mar 22, 2017 at 6:04 PM, Chad Versace 
> wrote:
> 
> > Validate that isl_surf::row_pitch fits in the below bitfields,
> > if applicable based on isl_surf::usage.
> >
> > RENDER_SURFACE_STATE::SurfacePitch
> > RENDER_SURFACE_STATE::AuxiliarySurfacePitch
> > 3DSTATE_DEPTH_BUFFER::SurfacePitch
> > 3DSTATE_HIER_DEPTH_BUFFER::SurfacePitch
> >
> > v2: Add a Makefile dependency on generated header genX_bits.h.
> > ---
> >  src/intel/Makefile.isl.am |  3 ++
> >  src/intel/isl/isl.c   | 72 ++
> > +
> >  2 files changed, 69 insertions(+), 6 deletions(-)




> > +
> > +   if (row_pitch == 0)
> > +  return false;
> > +
> > +   if (dim_layout == ISL_DIM_LAYOUT_GEN9_1D) {
> > +  /* SurfacePitch is ignored for this layout.How should we validate
> > it? */
> > +  isl_finishme("validate row pitch for ISL_DIM_LAYOUT_GEN9_1D");
> > +  goto done;
> > +   }
> > +
> > +   if (((surf_info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) ||
> > +(surf_info->usage & ISL_SURF_USAGE_TEXTURE_BIT)) &&
> >
> 
> We also want to handle STORAGE_BIT

Done. I added it locally.

> > +   !pitch_in_range(row_pitch, RENDER_SURFACE_STATE_
> > SurfacePitch_bits(gen_10x)))
> > +  return false;
> > +
> > +   if (((surf_info->usage & ISL_SURF_USAGE_CCS_BIT) ||
> > +(surf_info->usage & ISL_SURF_USAGE_MCS_BIT)) &&
> > +   !pitch_in_range(row_pitch_tiles, RENDER_SURFACE_STATE_
> > AuxiliarySurfacePitch_bits(gen_10x)))
> > +  return false;
> > +
> > +   if ((surf_info->usage & ISL_SURF_USAGE_DEPTH_BIT) &&
> > +   !pitch_in_range(row_pitch, _3DSTATE_DEPTH_BUFFER_
> > SurfacePitch_bits(gen_10x)))
> > +  return false;
> > +
> > +   if ((surf_info->usage & ISL_SURF_USAGE_HIZ_BIT) &&
> > +   !pitch_in_range(row_pitch, _3DSTATE_HIER_DEPTH_BUFFER_
> > SurfacePitch_bits(gen_10x)))
> > +  return false;
> > +
> > +   if (surf_info->usage & ISL_SURF_USAGE_STENCIL_BIT)
> > +  isl_finishme("validate row pitch of stencil surfaces");
> >
> 
> How hard is this?  I assume it's not trivial or you would have done it.

It's not trivial. There's corner cases, especially for older gens.  But
it's not *too* hard. It's just that I have no confidence that I could
write it correctly on the first try.

I wanted to land all the obvious pitch validations asap. Where obvious
means:

   if ((usage & bits) && !pitch_in_range())
  return false;

Since stencil pitch validation is the only non-obvious validation,
I wanted to do it as a follow-up. Because there WILL be errors in v1 of
the patch. And I didn't want this patch to be blocked while I debugged
the stencil validation failures and argued over the corner cases.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/4] vc4: Fix indenting in vc4_screen_get_param()

2017-03-24 Thread Eric Anholt
Lyude  writes:

> Signed-off-by: Lyude 

Reviewed-by: Eric Anholt 


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 6/9] isl: Validate the calculated row pitch (v2)

2017-03-24 Thread Chad Versace
On Fri 24 Mar 2017, Emil Velikov wrote:
> Hi Chad,
> 
> On 23 March 2017 at 01:04, Chad Versace  wrote:
> > Validate that isl_surf::row_pitch fits in the below bitfields,
> > if applicable based on isl_surf::usage.
> >
> > RENDER_SURFACE_STATE::SurfacePitch
> > RENDER_SURFACE_STATE::AuxiliarySurfacePitch
> > 3DSTATE_DEPTH_BUFFER::SurfacePitch
> > 3DSTATE_HIER_DEPTH_BUFFER::SurfacePitch
> >
> > v2: Add a Makefile dependency on generated header genX_bits.h.
> > ---
> >  src/intel/Makefile.isl.am |  3 ++
> >  src/intel/isl/isl.c   | 72 
> > +++
> >  2 files changed, 69 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/intel/Makefile.isl.am b/src/intel/Makefile.isl.am
> > index ee2215df1d1..09a10281b45 100644
> > --- a/src/intel/Makefile.isl.am
> > +++ b/src/intel/Makefile.isl.am
> > @@ -63,6 +63,9 @@ isl/isl_format_layout.c: isl/gen_format_layout.py \
> > $(PYTHON_GEN) $(srcdir)/isl/gen_format_layout.py \
> > --csv $(srcdir)/isl/isl_format_layout.csv --out $@
> >
> > +# Dependencies on generated files
> > +$(builddir)/isl/isl.o: $(srcdir)/genxml/genX_bits.h
> > +
> Can we have this as below. We could also squash it with 3/9.
> 
> BUILT_SOURCES += genxml/genX_bits.h
> EXTRA_DIST += genxml/genX_bits.h

Patch 3/9, in Makefile.gexml.am, does
  GENXML_GENERATED_FILES += genxml/genX_bits.h
which indirectly accomplishes
  BUILT_SOURCES += genxml/genX_bits.h
  EXTRA_DIST += genxml/genX_bits.h

Do you me want to drop this?
  $(builddir)/isl/isl.o: $(srcdir)/genxml/genX_bits.h
I'll drop it if you want.

I don't understand autoconf...  I added the dependency because I was
unsure how BUILT_SOURCES worked.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v2 1/2] mesa: make glFramebuffer* check immutable texture level bounds

2017-03-24 Thread Ilia Mirkin
Wait, so are we saying that my original patch is good? I've
beyond-lost context on all this, but as I recall my original patch had
some issues.

As I don't have Khronos access to the bug in question, I'm not sure
what the right thing is. The original patch applies this logic to
gl[Named]FramebufferTexture[Layer|1D|2D|3D]

Is that correct? Is there some sort of late-binding thing that should
happen (e.g. if you do glFramebufferTexture2D(), does it use the
currently-bound TEXTURE_2D, or is that resolved later? (I haven't
checked.)

  -ilia

On Fri, Mar 24, 2017 at 8:17 AM, Antía Puentes  wrote:
> Reviewed-by: Antia Puentes 
>
>
> On jue, 2017-01-26 at 00:47 -0500, Ilia Mirkin wrote:
>> When a texture is immutable, we can't tack on extra levels
>> after-the-fact like we could with glTexImage. So check against that
>> level limit and return an error if it's surpassed.
>>
>> The spec is a little unclear in that it says to check if "level is
>> not a
>> supported texture level", however that is never fully defined.
>>
>> This fixes:
>> GL45-CTS.geometry_shader.layered_fbo.fb_texture_invalid_level_number
>>
>> Signed-off-by: Ilia Mirkin 
>> ---
>>
>> v1 -> v2: use NumLevels instead of _MaxLevel.
>>
>> Not sure why this isn't showing up as failing in the Intel CI, but it
>> was
>> definitely failing here.
>>
>>  src/mesa/main/fbobject.c | 22 ++
>>  1 file changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index 6934805..6e86248 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -3128,16 +3128,22 @@ check_layer(struct gl_context *ctx, GLenum
>> target, GLint layer,
>>   * \return true if no errors, false if errors
>>   */
>>  static bool
>> -check_level(struct gl_context *ctx, GLenum target, GLint level,
>> -const char *caller)
>> +check_level(struct gl_context *ctx, const struct gl_texture_object
>> *texObj,
>> +GLint level, const char *caller)
>>  {
>> if ((level < 0) ||
>> -   (level >= _mesa_max_texture_levels(ctx, target))) {
>> +   (level >= _mesa_max_texture_levels(ctx, texObj->Target))) {
>>_mesa_error(ctx, GL_INVALID_VALUE,
>>"%s(invalid level %d)", caller, level);
>>return false;
>> }
>>
>> +   if (texObj->Immutable && level >= texObj->NumLevels) {
>> +  _mesa_error(ctx, GL_INVALID_VALUE,
>> +  "%s(level %u >= %u)", caller, level, texObj-
>> >NumLevels);
>> +  return false;
>> +   }
>> +
>> return true;
>>  }
>>
>> @@ -3260,7 +3266,7 @@ framebuffer_texture_with_dims(int dims, GLenum
>> target,
>>if ((dims == 3) && !check_layer(ctx, texObj->Target, layer,
>> caller))
>>   return;
>>
>> -  if (!check_level(ctx, textarget, level, caller))
>> +  if (!check_level(ctx, texObj, level, caller))
>>   return;
>> }
>>
>> @@ -3328,7 +3334,7 @@ _mesa_FramebufferTextureLayer(GLenum target,
>> GLenum attachment,
>>if (!check_layer(ctx, texObj->Target, layer, func))
>>   return;
>>
>> -  if (!check_level(ctx, texObj->Target, level, func))
>> +  if (!check_level(ctx, texObj, level, func))
>>   return;
>>
>>if (texObj->Target == GL_TEXTURE_CUBE_MAP) {
>> @@ -3370,7 +3376,7 @@ _mesa_NamedFramebufferTextureLayer(GLuint
>> framebuffer, GLenum attachment,
>>if (!check_layer(ctx, texObj->Target, layer, func))
>>   return;
>>
>> -  if (!check_level(ctx, texObj->Target, level, func))
>> +  if (!check_level(ctx, texObj, level, func))
>>   return;
>>
>>if (texObj->Target == GL_TEXTURE_CUBE_MAP) {
>> @@ -3419,7 +3425,7 @@ _mesa_FramebufferTexture(GLenum target, GLenum
>> attachment,
>>if (!check_layered_texture_target(ctx, texObj->Target, func,
>> ))
>>   return;
>>
>> -  if (!check_level(ctx, texObj->Target, level, func))
>> +  if (!check_level(ctx, texObj, level, func))
>>   return;
>> }
>>
>> @@ -3459,7 +3465,7 @@ _mesa_NamedFramebufferTexture(GLuint
>> framebuffer, GLenum attachment,
>>  ))
>>   return;
>>
>> -  if (!check_level(ctx, texObj->Target, level, func))
>> +  if (!check_level(ctx, texObj, level, func))
>>   return;
>> }
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Eric Anholt
Dylan Baker  writes:

> [ Unknown signature status ]
> Quoting Jose Fonseca (2017-03-24 06:42:18)
>> 
>> I tend to disagree.  While we can't avoid a transitory period, when we 
>> embark on another build system (Meson or something else) I think we 
>> should aim at 1) ensure such tool can indeed _completely_ replace at 
>> least _one_ existing build system, 2) and aim at migration quickly.
>> 
>> Otherwise we'll just end up with yet another build system, yet another 
>> way builds can fail, with some developers stuck on old build systems 
>> because it works, or because the new build system quite doesn't work.
>> 
>> And this is from (painful) experience.
>
> I tend to agree. Meson is *nice* because it's faster than autotools, but it's
> simplicity and the fact that it works for windows and *nix systems is one of 
> the
> best features. Having fewer build systems is better than more.
>
> We had hoped that we could do one release with both autotools and meson, to 
> give
> some of the fast moving linux distributions (Arch, Fedora, etc) a chance to 
> help
> us iron out bugs, especially for pacakgers. I think it is important though to
> make a commitment for exactly when we're going to either migrate completely to
> meson, or abandon the attempt and revert it.
>
>> So I think we should identify stake holders soon, collect requirements 
>> (OSes platforms, etc), make sure the prospective tool meets them, have 
>> all stakeholders collaborate on a prototype, them embark on mass migration.
>> 
>> That is, if this fails, let it fail early.  If it succeeds, may it 
>> succeed early.  Anything but a slow death / zombie life.
>
> I have a branch that builds intel's Vulkan driver, I'm actively working to get
> intel's i965 dri driver and llvmpipe building and send that out as an RFC to
> mesa-dev. That should give us enough of mesa to evaluate the build system I
> hope (Since it touches all of the major mesa components [classic, gallium,
> neither]).
>
> If other people are interested in collaborating I'd be happy to push the 
> branch
> sooner so that others can look at it.
>
> I also think it's worth talking to Eric (who said he's porting X to meson),
> Daniel Stone (who has patches to port weston to meson), and Peter Hutterer 
> (who
> has patches to port libinput to meson). If they're serious about seeing those
> land meson is even more appealing, since it would be a single build system 
> that
> all of the *nix graphics stack would be moving towards, and would mean that we
> wouldn't have an "Autotools for xorg", "meson for weston and libinput", 
> "cmake for
> piglit", and " for mesa".

My desire is to push enough of X.org to Meson that I can actually do CI
of the X Server.  Right now CI is not really tractable on Travis because
autogen.sh of all the dependencies takes too long.


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] targets/va: export radeon winsys_create functions

2017-03-24 Thread Emil Velikov
On 24 March 2017 at 15:26, Marek Olšák  wrote:
> On Fri, Mar 24, 2017 at 4:09 PM, Emil Velikov  
> wrote:
>> On 24 March 2017 at 14:42, Marek Olšák  wrote:
>>> On Fri, Mar 24, 2017 at 12:24 PM, Emil Velikov  
>>> wrote:
 On 24 March 2017 at 11:02, Nicolai Hähnle  wrote:
> On 24.03.2017 01:00, Marek Olšák wrote:
>>
>> From: Marek Olšák 
>>
>> This should fix this radeonsi error:
>>   "mesa: for the -simplifycfg-sink-common option: may only occur zero or
>> one
>>times!"
 Can we have some commit message. Feel free to reuse the following:

 Earlier commit added a LLVM 4.0 workaround by passing
 -simplifycfg-sink-common=false to LLVM.
 When using multiple drivers, for example GL/dri and VAAPI, we may end
 up with the option being parsed multiple times.
 Hence we'll see errors like

   "mesa: for the -simplifycfg-sink-common option: may only occur zero or 
 one
times!"

 Workaround this by exporting the driver entry point. This will lead to
 the function being called once.

 Fixes: 7751ed39e40 ("radeonsi: disable sinking common instructions
 down to the end block")

>> ---
>>  src/gallium/targets/va/va.sym | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/src/gallium/targets/va/va.sym 
>> b/src/gallium/targets/va/va.sym
>> index c925b2e..b19bc36 100644
>> --- a/src/gallium/targets/va/va.sym
>> +++ b/src/gallium/targets/va/va.sym
>> @@ -1,6 +1,8 @@
>>  {
>> global:
>> __vaDriverInit_*_*;
>> +   radeon_drm_winsys_create;
>> +   amdgpu_winsys_create;

 Please add a reference to the si_shader_tgsi_setup.c and vice-versa.
 Otherwise we will end up removing one but not the other.
>>>
>>> It's actually not a workaround. It's a fix of va.sym that happens to
>>> fix that bug and it would be the correct thing to do even if there was
>>> no issue. Thus, I think a reference to si_shader_tgsi_setup is
>>> unnecessary. I'm not fixing the bug here. I'm just enforcing one
>>> screen+winsys per device per process like we already do between GL and
>>> VDPAU.
>>>
>> One of us is getting confused by how it works, hence the confusion how
>> it should be called.
>>
>> On DRI2 we need to share the same $driver_winsys_create as need the
>> same GEM handles.
>> For DRI3 we don't need either of these, due to the handy code written
>> by Christian.
>>
>> Since we have a GL extension for GL VDPAU interop, the DRI and VDPAU
>> drivers must expose the entry point, for as long as DRI2 is supported.
>> Other APIs, such as VAAPI, implicitly fall-back to the DRI3 codepath.
>>
>> Or if we ignore everything I said, for a minute:
>>
>> Does the workflow work correctly, before the si_shader_tgsi_setup
>> patch, even without this fix ?
>> I believe the answer is, yes it does. So even if workaround is the
>> wrong word, the two are interconnected and should be cross-referenced.
>> Pretty please ?
>
> Yes, I understand your point of view. BTW, the message is only an LLVM
> warning. AFAIK, everything works both with and without the fix.
>
Your earlier message said "error", so I get confused there.

If everything works without the patch then it sounds like a
workaround. But let's ignore the name and call it "X".

> My question is whether we should establish this one-screen-per-process
> policy for all libs. It's not only VAAPI that's affected. Any lib that
> contains gallium but doesn't export the symbols is affected.
>
> These files export the symbols (including this patch):
> ./src/gallium/targets/vdpau/vdpau.sym
> ./src/gallium/targets/dri/dri.sym
> ./src/gallium/targets/dri-vdpau.dyn
These are _absolutely_ necessary for GL VDPAU to work on DRI2.

> ./src/gallium/targets/va/va.sym
>
> These files should export the symbols too, because the APIs are
> commonly used alongside other Mesa libs:
> ./src/gallium/targets/omx/omx.sym
> ./src/gallium/targets/opencl/opencl.sym
>
Above you want to swap opencl.sym with pipe.sym. The former is merely
st/clover + gallium/aux while the latter has the driver specifics.

If you want to update those, please do. All I'm kindly asking for is a
3-4 word comment in either place.

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


Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Dylan Baker
Quoting Jose Fonseca (2017-03-24 06:42:18)
> 
> I tend to disagree.  While we can't avoid a transitory period, when we 
> embark on another build system (Meson or something else) I think we 
> should aim at 1) ensure such tool can indeed _completely_ replace at 
> least _one_ existing build system, 2) and aim at migration quickly.
> 
> Otherwise we'll just end up with yet another build system, yet another 
> way builds can fail, with some developers stuck on old build systems 
> because it works, or because the new build system quite doesn't work.
> 
> And this is from (painful) experience.

I tend to agree. Meson is *nice* because it's faster than autotools, but it's
simplicity and the fact that it works for windows and *nix systems is one of the
best features. Having fewer build systems is better than more.

We had hoped that we could do one release with both autotools and meson, to give
some of the fast moving linux distributions (Arch, Fedora, etc) a chance to help
us iron out bugs, especially for pacakgers. I think it is important though to
make a commitment for exactly when we're going to either migrate completely to
meson, or abandon the attempt and revert it.

> So I think we should identify stake holders soon, collect requirements 
> (OSes platforms, etc), make sure the prospective tool meets them, have 
> all stakeholders collaborate on a prototype, them embark on mass migration.
> 
> That is, if this fails, let it fail early.  If it succeeds, may it 
> succeed early.  Anything but a slow death / zombie life.

I have a branch that builds intel's Vulkan driver, I'm actively working to get
intel's i965 dri driver and llvmpipe building and send that out as an RFC to
mesa-dev. That should give us enough of mesa to evaluate the build system I
hope (Since it touches all of the major mesa components [classic, gallium,
neither]).

If other people are interested in collaborating I'd be happy to push the branch
sooner so that others can look at it.

I also think it's worth talking to Eric (who said he's porting X to meson),
Daniel Stone (who has patches to port weston to meson), and Peter Hutterer (who
has patches to port libinput to meson). If they're serious about seeing those
land meson is even more appealing, since it would be a single build system that
all of the *nix graphics stack would be moving towards, and would mean that we
wouldn't have an "Autotools for xorg", "meson for weston and libinput", "cmake 
for
piglit", and " for mesa".

> BTW, how about migrating mesademos to Meson?  It currently has autotools 
> and cmake.  I was hoping that cmake would replace autotools, but I 
> couldn't run fast enough, so I couldn't practice what preached above, 
> hence cmake doing almost but not all what autotools does.
> 
> And is not a crucial project for Linux distros -- few distros package it 
> -- and even if they do, no other package would depend on it.  And is one 
> of those sort of projects that should be easy to port to any build too.

That's definitely doable, but most distros do package it, it's where glxgears,
and more importantly glxinfo live.

I'll have a look at it and see. At the very least we should be able to drop
cmake since I very much doubt anyone but you guys use it.

> Even if we ignore everything else, just replacing autotools + cmake with 
> just one thing would be a net win.
> 
> 
> Jose


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


Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread randyf



On Mon, 20 Mar 2017, Matt Turner wrote:




 - dropping the Autotools will lead to OpenBSD and NetBSD having to
write one from scratch, IIRC Solaris/FreeBSD and others are in similar
boat.


Solaris is a closed source operating system whose developers do not
contribute to the project. We do not need to base our decisions on
them.



  For the parts that are relevant to this discussion, this isn't true. 
Yes, the core OS is closed (Oracle Solaris, not the OpenSolaris 
derivative), but the entire graphics stack, including kernel graphics 
drivers are available on java.net and/or github.



  That developers haven't been contributing is primarily due to a 
significant amount of work required on a small set of developers in an 
attempt to just catch up, but they are listening.  What a migration to 
new build system would mean is another item in the list requiring more 
catch-up.



  That said (and I hope I don't get into too hot water), I don't think it 
would be an extremely difficult task for Solaris to migrate; just would 
require resources.



  Cheers!

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


Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Dylan Baker
Quoting Colin Cross (2017-03-23 17:03:58)
> On Thu, Mar 23, 2017 at 4:56 PM, Dylan Baker  wrote:
> >
> > I'm hoping you can clarify a couple of questions I have about blueprint:
> > 1) android is moving to blueprint from android.mk files?
> 
> Yes, in a phased transition.  We support both for now.
> 
> > 2) is there a publicly available project that uses blueprint I could look 
> > at?
> >The documentation I've been able to find is pretty sparse.
> 
> Blueprint is a framework for writing build systems, and Soong is
> AOSP's Blueprint build system.  See
> https://android.googlesource.com/platform/build/soong/+/master/README.md
> for documentation on Soong.

Thanks, that's very useful information.


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 3/3] targets/va: export radeon winsys_create functions

2017-03-24 Thread Nicolai Hähnle

On 24.03.2017 15:31, Marek Olšák wrote:

On Fri, Mar 24, 2017 at 12:02 PM, Nicolai Hähnle  wrote:

On 24.03.2017 01:00, Marek Olšák wrote:


From: Marek Olšák 

This should fix this radeonsi error:
  "mesa: for the -simplifycfg-sink-common option: may only occur zero or
one
   times!"
---
 src/gallium/targets/va/va.sym | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/gallium/targets/va/va.sym b/src/gallium/targets/va/va.sym
index c925b2e..b19bc36 100644
--- a/src/gallium/targets/va/va.sym
+++ b/src/gallium/targets/va/va.sym
@@ -1,6 +1,8 @@
 {
global:
__vaDriverInit_*_*;
+   radeon_drm_winsys_create;
+   amdgpu_winsys_create;



Oof, that's ugly, but at least it's only a change to va and the chance of
collision is low. Have you verified that it fixes the bug? If so, this (and
the other patches anyway) is


It's not ugly. It's actually the correct fix to ensure that there is
only one winsys & screen per device per process. Our GL and VDPAU
drivers do it too and initially it was the only way to share
pipe_resource between libs. It's also done by nouveau and freedreno.
The idea is that first loaded lib exports the whole gallium driver via
these symbols and any libs loaded later are forced to use it and can't
use their own.


Ok, that's interesting, thanks. It feels a bit weird, but there's a 
definite upside to doing this that would be difficult to get otherwise.


Nicolai



Marek




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/4] r600_state_common: check NULL return from u_upload_alloc

2017-03-24 Thread Nicolai Hähnle

On 24.03.2017 13:11, Julien Isorce wrote:

1: Ack, thx.
2: u_upload_alloc can fail, i.e. set output param *ptr to NULL, for 2
reasons: alloc fails or map fails. For both there are already a
fprintf/stderr, i.e., in radeon_create_bo and radeon_bo_do_map .
It looks to me that in src/gallium/drivers/ it is a common usage to just
avoid to crash by doing a silent check. But it defers the fprintf to
where the error comes from, i.e. libdrm calls.


Okay, that makes sense.



Also I think that if drmCommandWriteRead returns -ENOMEM it should be
translated to _mesa_error_no_memory(__func__); at some point. But it
seems it is not possible to use _mesa_error_no_memory in
mesa/src/gallium/winsys (though it is possible from
gallium/state_trackers/glx). And it would require to be in the gl
context's thread to call it.


Unfortunately, that doesn't work because the driver could be called from 
another state tracker (video APIs, nine, opencl).


Although, we could add a way for the state tracker to register an 
out-of-memory callback function, similar to the debug callback.


Also, I remember thinking at one point that in si_state_draw.c, it might 
be helpful to just flag out-of-memory conditions in a context-wide bool 
which is checked once before the actual draw commands are written. This 
would reduce the need to bubble up error conditions.


Cheers,
Nicolai



Regards
Julien


On 24 March 2017 at 11:19, Nicolai Hähnle > wrote:

I generally like the patches in the series, thanks for that. Two
points though:

1. The order of patches in series is usually general code before
driver code, i.e. the st/cb_bitmap should come first.

2. I don't like having silent errors, as that could be confusing. In
places where the error isn't propagated to the application (as it
really should be...), I think we should have an fprintf to stderr.

Cheers,
Nicolai

On 24.03.2017 12:08, Julien Isorce wrote:

Like done in si_state_draw.c::si_draw_vbo

Signed-off-by: Julien Isorce >
---
 src/gallium/drivers/r600/r600_state_common.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/gallium/drivers/r600/r600_state_common.c
b/src/gallium/drivers/r600/r600_state_common.c
index 6f8279f..cedeb74 100644
--- a/src/gallium/drivers/r600/r600_state_common.c
+++ b/src/gallium/drivers/r600/r600_state_common.c
@@ -1746,6 +1746,10 @@ static void r600_draw_vbo(struct
pipe_context *ctx, const struct pipe_draw_info

u_upload_alloc(ctx->stream_uploader,
start, count * 2,
256, _offset,
_buffer, );
+   if (unlikely(!ptr)) {
+
 pipe_resource_reference(, NULL);
+   return;
+   }

util_shorten_ubyte_elts_to_userptr(
>b.b, ,
0, 0, ib.offset + start, count, ptr);



--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.





--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Bas Nieuwenhuizen
> Another tool I heard good about but have not direct experience is
> https://bazel.build .  Any thoughts about it?

Having looked a bit into it, it also is just a build system (albeit
higher level than ninja or make). It doesn't do configure or install
and working with system dependencies is annoying. While it is awesome
at some stuff, I think for these reasons mesa just is not a good
usecase for it.

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


Re: [Mesa-dev] Mesa (master): mesa/marshal: add custom BufferData/ BufferSubData marshalling

2017-03-24 Thread Brian Paul

Hi Timothy,

Some late comments below.

On 03/23/2017 06:28 PM, Timothy Arceri wrote:

Module: Mesa
Branch: master
Commit: adced4a2f9d017ae126a438f97eb305fa0ca3bd0
URL:
https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_mesa_mesa_commit_-3Fid-3Dadced4a2f9d017ae126a438f97eb305fa0ca3bd0=DwIGaQ=uilaK90D4TOVoH58JNXRgQ=Ie7_encNUsqxbSRbqbNgofw0ITcfE8JKfaUjIQhncGA=euA0jTcoAmQ2Zvy_d-Fv7gEc7REbDj19MbIRrYVfXoc=RM6-RjO08ktuyTV6AhhjU7t-LGPw-SgL78pFH2QXiQs=

Author: Timothy Arceri 
Date:   Thu Mar 23 17:19:36 2017 +1100

mesa/marshal: add custom BufferData/BufferSubData marshalling

GL_AMD_pinned_memory requires memory to be aligned correctly, so
we skip marshalling in this case. Also copying the data defeats
the purpose of EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD.

Fixes GL_AMD_pinned_memory piglit tests when glthread is enabled.

Acked-by: Edward O'Callaghan 

---

  src/mapi/glapi/gen/gl_API.xml |   4 +-
  src/mesa/main/marshal.c   | 125 ++
  src/mesa/main/marshal.h   |  18 ++
  3 files changed, 145 insertions(+), 2 deletions(-)

diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index c1f0f8fe92..dfaeaafa03 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -5033,7 +5033,7 @@
  
  

-
+
  
  
  
@@ -5041,7 +5041,7 @@
  
  

-
+
  
  
  
diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
index f8cad30e20..cdc7fed8a5 100644
--- a/src/mesa/main/marshal.c
+++ b/src/mesa/main/marshal.c
@@ -259,4 +259,129 @@ _mesa_marshal_BindBuffer(GLenum target, GLuint buffer)
 }
  }

+/* BufferData: marshalled asynchronously */
+struct marshal_cmd_BufferData
+{
+   struct marshal_cmd_base cmd_base;
+   GLenum target;
+   GLsizeiptr size;
+   GLenum usage;
+   bool data_null; /* If set, no data follows for "data" */
+   /* Next size bytes are GLvoid data[size] */
+};
+
+void
+_mesa_unmarshal_BufferData(struct gl_context *ctx,
+   const struct marshal_cmd_BufferData *cmd)
+{
+   const GLenum target = cmd->target;
+   const GLsizeiptr size = cmd->size;
+   const GLenum usage = cmd->usage;
+   const char *variable_data = (const char *) (cmd + 1);
+   const GLvoid *data = (const GLvoid *) variable_data;
+
+   if (cmd->data_null)
+  data = NULL;
+   else
+  variable_data += size;


I don't understand why this var is incremented if it's never used again.


+
+   CALL_BufferData(ctx->CurrentServerDispatch, (target, size, data, usage));
+}
+
+void GLAPIENTRY
+_mesa_marshal_BufferData(GLenum target, GLsizeiptr size, const GLvoid * data,
+ GLenum usage)
+{
+   GET_CURRENT_CONTEXT(ctx);
+   size_t cmd_size =
+  sizeof(struct marshal_cmd_BufferData) + (data ? size : 0);
+   debug_print_marshal("BufferData");
+
+   if (unlikely(size < 0)) {
+  _mesa_glthread_finish(ctx);
+  _mesa_error(ctx, GL_INVALID_VALUE, "BufferData(size < 0)");
+  return;
+   }
+
+   if (target != GL_EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD &&
+   cmd_size <= MARSHAL_MAX_CMD_SIZE) {
+  struct marshal_cmd_BufferData *cmd =
+ _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_BufferData,
+ cmd_size);
+
+  cmd->target = target;
+  cmd->size = size;
+  cmd->usage = usage;
+  char *variable_data = (char *) (cmd + 1);
+  cmd->data_null = !data;
+  if (!cmd->data_null) {
+ memcpy(variable_data, data, size);
+ variable_data += size;


Same here.


+  }
+  _mesa_post_marshal_hook(ctx);
+   } else {
+  _mesa_glthread_finish(ctx);
+  CALL_BufferData(ctx->CurrentServerDispatch,
+  (target, size, data, usage));
+   }
+}
+
+/* BufferSubData: marshalled asynchronously */
+struct marshal_cmd_BufferSubData
+{
+   struct marshal_cmd_base cmd_base;
+   GLenum target;
+   GLintptr offset;
+   GLsizeiptr size;
+   /* Next size bytes are GLvoid data[size] */
+};
+
+void
+_mesa_unmarshal_BufferSubData(struct gl_context *ctx,
+  const struct marshal_cmd_BufferSubData *cmd)
+{
+   const GLenum target = cmd->target;
+   const GLintptr offset = cmd->offset;
+   const GLsizeiptr size = cmd->size;
+   const char *variable_data = (const char *) (cmd + 1);
+   const GLvoid *data = (const GLvoid *) variable_data;
+
+   variable_data += size;


And here, and below.

I'm going to post a patch which simplifies some of this code...

-Brian




+   CALL_BufferSubData(ctx->CurrentServerDispatch,
+  (target, offset, size, data));
+}
+
+void GLAPIENTRY
+_mesa_marshal_BufferSubData(GLenum target, GLintptr offset, GLsizeiptr size,
+const GLvoid * data)
+{
+   GET_CURRENT_CONTEXT(ctx);
+   size_t cmd_size = sizeof(struct marshal_cmd_BufferSubData) + size;
+
+   

[Mesa-dev] [PATCH] mesa: simplify code around 'variable_data' in marshal.c

2017-03-24 Thread Brian Paul
Remove needless pointer increments, unneeded vars, etc.  Untested.
Plus, fix a couple comments.
---
 src/mesa/main/marshal.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
index cdc7fed..783ca0a 100644
--- a/src/mesa/main/marshal.c
+++ b/src/mesa/main/marshal.c
@@ -267,7 +267,7 @@ struct marshal_cmd_BufferData
GLsizeiptr size;
GLenum usage;
bool data_null; /* If set, no data follows for "data" */
-   /* Next size bytes are GLvoid data[size] */
+   /* Next size bytes are GLubyte data[size] */
 };
 
 void
@@ -277,13 +277,12 @@ _mesa_unmarshal_BufferData(struct gl_context *ctx,
const GLenum target = cmd->target;
const GLsizeiptr size = cmd->size;
const GLenum usage = cmd->usage;
-   const char *variable_data = (const char *) (cmd + 1);
-   const GLvoid *data = (const GLvoid *) variable_data;
+   const void *data;
 
if (cmd->data_null)
   data = NULL;
else
-  variable_data += size;
+  data = (const void *) (cmd + 1);
 
CALL_BufferData(ctx->CurrentServerDispatch, (target, size, data, usage));
 }
@@ -312,11 +311,10 @@ _mesa_marshal_BufferData(GLenum target, GLsizeiptr size, 
const GLvoid * data,
   cmd->target = target;
   cmd->size = size;
   cmd->usage = usage;
-  char *variable_data = (char *) (cmd + 1);
   cmd->data_null = !data;
-  if (!cmd->data_null) {
+  if (data) {
+ char *variable_data = (char *) (cmd + 1);
  memcpy(variable_data, data, size);
- variable_data += size;
   }
   _mesa_post_marshal_hook(ctx);
} else {
@@ -333,7 +331,7 @@ struct marshal_cmd_BufferSubData
GLenum target;
GLintptr offset;
GLsizeiptr size;
-   /* Next size bytes are GLvoid data[size] */
+   /* Next size bytes are GLubyte data[size] */
 };
 
 void
@@ -343,10 +341,8 @@ _mesa_unmarshal_BufferSubData(struct gl_context *ctx,
const GLenum target = cmd->target;
const GLintptr offset = cmd->offset;
const GLsizeiptr size = cmd->size;
-   const char *variable_data = (const char *) (cmd + 1);
-   const GLvoid *data = (const GLvoid *) variable_data;
+   const void *data = (const void *) (cmd + 1);
 
-   variable_data += size;
CALL_BufferSubData(ctx->CurrentServerDispatch,
   (target, offset, size, data));
 }
@@ -375,7 +371,6 @@ _mesa_marshal_BufferSubData(GLenum target, GLintptr offset, 
GLsizeiptr size,
   cmd->size = size;
   char *variable_data = (char *) (cmd + 1);
   memcpy(variable_data, data, size);
-  variable_data += size;
   _mesa_post_marshal_hook(ctx);
} else {
   _mesa_glthread_finish(ctx);
-- 
1.9.1

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


Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

2017-03-24 Thread Jose Fonseca

On 24/03/17 14:22, Daniel Stone wrote:

Hi Jose,

On 24 March 2017 at 14:03, Jose Fonseca  wrote:

On 22/03/17 20:57, Dylan Baker wrote:

Cross compiling for mingw is supported, and it provides a way to
differentiate
the build, host, and target machines [1], I've cross compiled for
aarch64-linux-gnu, and it was trivial (I've been told autotools has a flag
for
this, but the meson approach is to write an ini file once, and use it
again and
again), and the first example of cross compiling is using mingw from linux
[2].


[1]https://github.com/mesonbuild/meson/wiki/Reference-manual#build_machine-object
[2]https://github.com/mesonbuild/meson/wiki/Cross-compilation



Thanks for the info.

It still scares me a bit that most Meson users are mostly Linux focused.


That's not actually true ...

A lot of the reason GStreamer went towards Meson is because it has
native OS X (XCode project) and Windows (MinGW or Visual Studio
project) support. Meson's own source tree goes out of its way to use
spaces in directory and file names for its test suite, to make sure
that that support never breaks. Every build of Meson is CI'd on
AppVeyor as well as Travis for OS X. And of course, the GStreamer CI
uses Meson for its non-Linux builds.

It's true that a lot of the projects taking an interest of late have
been Linux, yes. But if you're native to Windows / OS X, you just use
VS/Xcode. There are (IMO) very few properly cross-platform projects
who are 'Windows projects with a port' or 'mainly OS X but also runs
on Linux', rather than cross-platform 'Linux projects'.


OK.


Another tool I heard good about but have not direct experience is
https://bazel.build .  Any thoughts about it?


If you look at the FAQ, one of the first things you'll see is that
they don't properly support Windows. It also requires Java to run,
which I'd say would be a no-go if people are already complaining about
the impossibility of installing several megabytes of Python stdlib.


I see.  Indeed.


Thanks.  From the label on the tin, it does sound like Meson ticks a lot 
of boxes then.



Like I said in another email, maybe mesademos is a good way to get our 
feet wet.


I can't guarantee I'll be able to dedicate much time, but at least get 
AppVeyor builds of it, since it has been a long objective of mine to get 
mesademos/piglit/etc builds on AppVeyor instead of a private Jenkins server.



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


Re: [Mesa-dev] [PATCH 6/9] isl: Validate the calculated row pitch (v2)

2017-03-24 Thread Emil Velikov
Hi Chad,

On 23 March 2017 at 01:04, Chad Versace  wrote:
> Validate that isl_surf::row_pitch fits in the below bitfields,
> if applicable based on isl_surf::usage.
>
> RENDER_SURFACE_STATE::SurfacePitch
> RENDER_SURFACE_STATE::AuxiliarySurfacePitch
> 3DSTATE_DEPTH_BUFFER::SurfacePitch
> 3DSTATE_HIER_DEPTH_BUFFER::SurfacePitch
>
> v2: Add a Makefile dependency on generated header genX_bits.h.
> ---
>  src/intel/Makefile.isl.am |  3 ++
>  src/intel/isl/isl.c   | 72 
> +++
>  2 files changed, 69 insertions(+), 6 deletions(-)
>
> diff --git a/src/intel/Makefile.isl.am b/src/intel/Makefile.isl.am
> index ee2215df1d1..09a10281b45 100644
> --- a/src/intel/Makefile.isl.am
> +++ b/src/intel/Makefile.isl.am
> @@ -63,6 +63,9 @@ isl/isl_format_layout.c: isl/gen_format_layout.py \
> $(PYTHON_GEN) $(srcdir)/isl/gen_format_layout.py \
> --csv $(srcdir)/isl/isl_format_layout.csv --out $@
>
> +# Dependencies on generated files
> +$(builddir)/isl/isl.o: $(srcdir)/genxml/genX_bits.h
> +
Can we have this as below. We could also squash it with 3/9.

BUILT_SOURCES += genxml/genX_bits.h
EXTRA_DIST += genxml/genX_bits.h

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


Re: [Mesa-dev] [PATCH 3/9] genxml: New generated header genX_bits.h (v2)

2017-03-24 Thread Emil Velikov
On 23 March 2017 at 01:03, Chad Versace  wrote:

> ---
>  src/intel/Makefile.genxml.am|   6 +-
>  src/intel/Makefile.sources  |   6 +-
>  src/intel/genxml/.gitignore |   1 +
>  src/intel/genxml/gen_bits_header.py | 259 
> 
>  4 files changed, 270 insertions(+), 2 deletions(-)
>  create mode 100644 src/intel/genxml/gen_bits_header.py
>
> diff --git a/src/intel/Makefile.genxml.am b/src/intel/Makefile.genxml.am
> index 01a02b63b44..4e59a918618 100644
> --- a/src/intel/Makefile.genxml.am
> +++ b/src/intel/Makefile.genxml.am
> @@ -30,7 +30,7 @@ EXTRA_DIST += \
>
>  SUFFIXES = _pack.h _xml.h .xml
>
> -$(GENXML_GENERATED_FILES): genxml/gen_pack_header.py
> +$(GENXML_GENERATED_PACK_FILES): genxml/gen_pack_header.py
>
>  .xml_pack.h:
> $(MKDIR_GEN)
> @@ -42,6 +42,10 @@ $(AUBINATOR_GENERATED_FILES): genxml/gen_zipped_file.py
> $(MKDIR_GEN)
> $(AM_V_GEN) $(PYTHON2) $(srcdir)/genxml/gen_zipped_file.py $< > $@ || 
> ($(RM) $@; false)
>
> +genxml/genX_bits.h: genxml/gen_bits_header.py $(GENXML_XML_FILES)
> +   $(MKDIR_GEN)
> +   $(PYTHON_GEN) $< -o $@ $(GENXML_XML_FILES)
> +
>  EXTRA_DIST += \
> genxml/genX_pack.h \
> genxml/gen_macros.h \
> diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> index 801797d2768..ec43f06a495 100644
> --- a/src/intel/Makefile.sources
> +++ b/src/intel/Makefile.sources
> @@ -117,7 +117,7 @@ GENXML_XML_FILES = \
> genxml/gen8.xml \
> genxml/gen9.xml
>
> -GENXML_GENERATED_FILES = \
> +GENXML_GENERATED_PACK_FILES = \
> genxml/gen4_pack.h \
> genxml/gen45_pack.h \
> genxml/gen5_pack.h \
> @@ -127,6 +127,10 @@ GENXML_GENERATED_FILES = \
> genxml/gen8_pack.h \
> genxml/gen9_pack.h
>
> +GENXML_GENERATED_FILES = \
> +   $(GENXML_GENERATED_PACK_FILES) \
> +   genxml/genX_bits.h
> +
Chad, this patch will need a fix on Android side.

Tapani, RobH, please let use know how you would like that sorted -
provide a fix to squash or address as follow-up.

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


Re: [Mesa-dev] [PATCH 1/9] genxml: Define GENXML_XML_FILES in Makefile.sources

2017-03-24 Thread Emil Velikov
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 2/9] genxml: Fix gen_zipped_file.py dependency

2017-03-24 Thread Emil Velikov
On 23 March 2017 at 01:03, Chad Versace  wrote:
> The gen*_xml.h files depend on gen_zipped_file.py, not the gen*_pack.h
> files.

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 2/4] si_compute: check NULL return from u_upload_alloc

2017-03-24 Thread Marek Olšák
I think we should also return from si_launch_grid if this fails.

Marek

On Fri, Mar 24, 2017 at 12:08 PM, Julien Isorce  wrote:
> Signed-off-by: Julien Isorce 
> ---
>  src/gallium/drivers/radeonsi/si_compute.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/gallium/drivers/radeonsi/si_compute.c 
> b/src/gallium/drivers/radeonsi/si_compute.c
> index 19a9189..e3ccc55 100644
> --- a/src/gallium/drivers/radeonsi/si_compute.c
> +++ b/src/gallium/drivers/radeonsi/si_compute.c
> @@ -602,6 +602,9 @@ static void si_upload_compute_input(struct si_context 
> *sctx,
>_args_offset,
>(struct pipe_resource**)_buffer, 
> _args_ptr);
>
> +   if (unlikely(!kernel_args_ptr))
> +   return;
> +
> kernel_args = (uint32_t*)kernel_args_ptr;
> kernel_args_va = input_buffer->gpu_address + kernel_args_offset;
>
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/3] targets/va: export radeon winsys_create functions

2017-03-24 Thread Marek Olšák
On Fri, Mar 24, 2017 at 4:09 PM, Emil Velikov  wrote:
> On 24 March 2017 at 14:42, Marek Olšák  wrote:
>> On Fri, Mar 24, 2017 at 12:24 PM, Emil Velikov  
>> wrote:
>>> On 24 March 2017 at 11:02, Nicolai Hähnle  wrote:
 On 24.03.2017 01:00, Marek Olšák wrote:
>
> From: Marek Olšák 
>
> This should fix this radeonsi error:
>   "mesa: for the -simplifycfg-sink-common option: may only occur zero or
> one
>times!"
>>> Can we have some commit message. Feel free to reuse the following:
>>>
>>> Earlier commit added a LLVM 4.0 workaround by passing
>>> -simplifycfg-sink-common=false to LLVM.
>>> When using multiple drivers, for example GL/dri and VAAPI, we may end
>>> up with the option being parsed multiple times.
>>> Hence we'll see errors like
>>>
>>>   "mesa: for the -simplifycfg-sink-common option: may only occur zero or one
>>>times!"
>>>
>>> Workaround this by exporting the driver entry point. This will lead to
>>> the function being called once.
>>>
>>> Fixes: 7751ed39e40 ("radeonsi: disable sinking common instructions
>>> down to the end block")
>>>
> ---
>  src/gallium/targets/va/va.sym | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/gallium/targets/va/va.sym b/src/gallium/targets/va/va.sym
> index c925b2e..b19bc36 100644
> --- a/src/gallium/targets/va/va.sym
> +++ b/src/gallium/targets/va/va.sym
> @@ -1,6 +1,8 @@
>  {
> global:
> __vaDriverInit_*_*;
> +   radeon_drm_winsys_create;
> +   amdgpu_winsys_create;
>>>
>>> Please add a reference to the si_shader_tgsi_setup.c and vice-versa.
>>> Otherwise we will end up removing one but not the other.
>>
>> It's actually not a workaround. It's a fix of va.sym that happens to
>> fix that bug and it would be the correct thing to do even if there was
>> no issue. Thus, I think a reference to si_shader_tgsi_setup is
>> unnecessary. I'm not fixing the bug here. I'm just enforcing one
>> screen+winsys per device per process like we already do between GL and
>> VDPAU.
>>
> One of us is getting confused by how it works, hence the confusion how
> it should be called.
>
> On DRI2 we need to share the same $driver_winsys_create as need the
> same GEM handles.
> For DRI3 we don't need either of these, due to the handy code written
> by Christian.
>
> Since we have a GL extension for GL VDPAU interop, the DRI and VDPAU
> drivers must expose the entry point, for as long as DRI2 is supported.
> Other APIs, such as VAAPI, implicitly fall-back to the DRI3 codepath.
>
> Or if we ignore everything I said, for a minute:
>
> Does the workflow work correctly, before the si_shader_tgsi_setup
> patch, even without this fix ?
> I believe the answer is, yes it does. So even if workaround is the
> wrong word, the two are interconnected and should be cross-referenced.
> Pretty please ?

Yes, I understand your point of view. BTW, the message is only an LLVM
warning. AFAIK, everything works both with and without the fix.

My question is whether we should establish this one-screen-per-process
policy for all libs. It's not only VAAPI that's affected. Any lib that
contains gallium but doesn't export the symbols is affected.

These files export the symbols (including this patch):
./src/gallium/targets/vdpau/vdpau.sym
./src/gallium/targets/dri/dri.sym
./src/gallium/targets/dri-vdpau.dyn
./src/gallium/targets/va/va.sym

These files should export the symbols too, because the APIs are
commonly used alongside other Mesa libs:
./src/gallium/targets/omx/omx.sym
./src/gallium/targets/opencl/opencl.sym

Not sure if we have to worry about these:
./src/gallium/targets/xvmc/xvmc.sym
./src/gallium/targets/xa/xa.sym
./src/gallium/targets/libgl-xlib/libgl-xlib.sym
./src/gallium/targets/pipe-loader/pipe.sym
./src/gallium/targets/osmesa/osmesa.sym
./src/gallium/targets/d3dadapter9/d3dadapter9.sym

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


Re: [Mesa-dev] [PATCH 08/11] st/va: clear the video surface on allocation

2017-03-24 Thread Andy Furniss

Andy Furniss wrote:

ping.


Christian König wrote:

From: Christian König 

This makes debugging of decoding problems quite a bit easier.


This breaks gstreamer encode for me.

ffmpeg is OK, but then IIRC ffmpeg only uses one of something that
gstreamer uses two of, not wishing to get too technical here :-)

Whatever the cause, gst is twice as fast as ffmpeg and with this
I get -

[drm:amdgpu_vce_cs_reloc [amdgpu]] *ERROR* BO to small for addr
0x01000b 48 47

andy [vce-tests]$ time gst-launch-1.0 -f filesrc
location=/mnt/ramdisk/trees-1440p50.nv12 blocksize=5529600 !
video/x-raw,format=NV12,width=2560,height=1440,framerate=50/1,pixel-aspect-ratio=1/1
! queue ! vaapih264enc  rate-control=cbr bitrate=1
keyframe-period=25 max-bframes=0 ! video/x-h264, profile=main ! filesink
location=/mnt/ramdisk/gst.264
libva info: VA-API version 0.40.0
libva info: va_getDriverName() returns 0
libva info: User requested driver 'radeonsi'
libva info: Trying to open /usr/lib/dri/radeonsi_drv_video.so
libva info: Found init function __vaDriverInit_0_40
libva info: va_openDriver() returns 0
libva info: VA-API version 0.40.0
libva info: va_getDriverName() returns 0
libva info: User requested driver 'radeonsi'
libva info: Trying to open /usr/lib/dri/radeonsi_drv_video.so
libva info: Found init function __vaDriverInit_0_40
libva info: va_openDriver() returns 0
Setting pipeline to PAUSED ...
libva info: VA-API version 0.40.0
libva info: va_getDriverName() returns 0
libva info: User requested driver 'radeonsi'
libva info: Trying to open /usr/lib/dri/radeonsi_drv_video.so
libva info: Found init function __vaDriverInit_0_40
libva info: va_openDriver() returns 0
Pipeline is PREROLLING ...
Got context from element 'vaapiencodeh264-0': gst.vaapi.Display=context,
gst.vaapi.Display=(GstVaapiDisplay)"\(GstVaapiDisplayGLX\)\
vaapidisplayglx1";
0:00:01.134997848   790  0x19310f0 ERROR   vaapivideomemory
gstvaapivideomemory.c:736:gst_video_info_update_from_surface: Cannot
create a VA derived image from surface 0x7fefcc002b70
amdgpu: The CS has been rejected, see dmesg for more information (-22).
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
0:00:01.157992874   790 0x7fefcc0028a0 ERRORvaapiencode
gstvaapiencode.c:210:gst_vaapiencode_default_alloc_buffer: invalid
GstVaapiCodedBuffer size (0 bytes)
amdgpu: The CS has been cancelled because the context is lost.
0:00:01.161509344   790 0x7fefcc0028a0 ERRORvaapiencode
gstvaapiencode.c:316:gst_vaapiencode_push_frame: failed to allocate
encoded buffer in system memory
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.
amdgpu: The CS has been cancelled because the context is lost.




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


Re: [Mesa-dev] [PATCH 3/3] targets/va: export radeon winsys_create functions

2017-03-24 Thread Ilia Mirkin
Does the nouveau winsys create function also need to be added? I'm not sure
I understand what problem this is solving... But afaik it's possible to use
va + nouveau.

On Mar 23, 2017 8:00 PM, "Marek Olšák"  wrote:

From: Marek Olšák 

This should fix this radeonsi error:
  "mesa: for the -simplifycfg-sink-common option: may only occur zero or one
   times!"
---
 src/gallium/targets/va/va.sym | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/gallium/targets/va/va.sym b/src/gallium/targets/va/va.sym
index c925b2e..b19bc36 100644
--- a/src/gallium/targets/va/va.sym
+++ b/src/gallium/targets/va/va.sym
@@ -1,6 +1,8 @@
 {
global:
__vaDriverInit_*_*;
+   radeon_drm_winsys_create;
+   amdgpu_winsys_create;
local:
*;
 };
--
2.7.4

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


Re: [Mesa-dev] [PATCH 3/3] targets/va: export radeon winsys_create functions

2017-03-24 Thread Emil Velikov
On 24 March 2017 at 14:42, Marek Olšák  wrote:
> On Fri, Mar 24, 2017 at 12:24 PM, Emil Velikov  
> wrote:
>> On 24 March 2017 at 11:02, Nicolai Hähnle  wrote:
>>> On 24.03.2017 01:00, Marek Olšák wrote:

 From: Marek Olšák 

 This should fix this radeonsi error:
   "mesa: for the -simplifycfg-sink-common option: may only occur zero or
 one
times!"
>> Can we have some commit message. Feel free to reuse the following:
>>
>> Earlier commit added a LLVM 4.0 workaround by passing
>> -simplifycfg-sink-common=false to LLVM.
>> When using multiple drivers, for example GL/dri and VAAPI, we may end
>> up with the option being parsed multiple times.
>> Hence we'll see errors like
>>
>>   "mesa: for the -simplifycfg-sink-common option: may only occur zero or one
>>times!"
>>
>> Workaround this by exporting the driver entry point. This will lead to
>> the function being called once.
>>
>> Fixes: 7751ed39e40 ("radeonsi: disable sinking common instructions
>> down to the end block")
>>
 ---
  src/gallium/targets/va/va.sym | 2 ++
  1 file changed, 2 insertions(+)

 diff --git a/src/gallium/targets/va/va.sym b/src/gallium/targets/va/va.sym
 index c925b2e..b19bc36 100644
 --- a/src/gallium/targets/va/va.sym
 +++ b/src/gallium/targets/va/va.sym
 @@ -1,6 +1,8 @@
  {
 global:
 __vaDriverInit_*_*;
 +   radeon_drm_winsys_create;
 +   amdgpu_winsys_create;
>>
>> Please add a reference to the si_shader_tgsi_setup.c and vice-versa.
>> Otherwise we will end up removing one but not the other.
>
> It's actually not a workaround. It's a fix of va.sym that happens to
> fix that bug and it would be the correct thing to do even if there was
> no issue. Thus, I think a reference to si_shader_tgsi_setup is
> unnecessary. I'm not fixing the bug here. I'm just enforcing one
> screen+winsys per device per process like we already do between GL and
> VDPAU.
>
One of us is getting confused by how it works, hence the confusion how
it should be called.

On DRI2 we need to share the same $driver_winsys_create as need the
same GEM handles.
For DRI3 we don't need either of these, due to the handy code written
by Christian.

Since we have a GL extension for GL VDPAU interop, the DRI and VDPAU
drivers must expose the entry point, for as long as DRI2 is supported.
Other APIs, such as VAAPI, implicitly fall-back to the DRI3 codepath.

Or if we ignore everything I said, for a minute:

Does the workflow work correctly, before the si_shader_tgsi_setup
patch, even without this fix ?
I believe the answer is, yes it does. So even if workaround is the
wrong word, the two are interconnected and should be cross-referenced.
Pretty please ?

>>
>>>
>>>
>>> Oof, that's ugly, but at least it's only a change to va and the chance of
>>> collision is low. Have you verified that it fixes the bug?
>
> I've not verified that, no. If it doesn't fix the bug, we still need
> this patch because of aforementioned reasons, and then we have to fix
> the bug.
>
>>
>> Same question - does it work without the -Wl,--dynamic-list?
>
> What is that?
>
This is part of how everything works together.

The modules must expose the winsys symbol, and the very same must be
in the dynamic list.
The latter ensures that the first module which provide
$driver_winsys_create which will be used the any other modules,
instead of their own copy.

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


Re: [Mesa-dev] [PATCH 3/3] targets/va: export radeon winsys_create functions

2017-03-24 Thread Andy Furniss

Andy Furniss wrote:


I've not verified that, no. If it doesn't fix the bug, we still need
this patch because of aforementioned reasons, and then we have to fix
the bug.


It fixes mpv for me testing vaapi hw decode + gl display.

It doesn't fix ffmpeg cli though.

ffmpeg  -hwaccel vaapi -i testfile -f null -

still gets the message.


So this seems to be because I compile ffmpeg with --enable-opencl

Built without that there is no message.


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


Re: [Mesa-dev] [PATCH 3/3] targets/va: export radeon winsys_create functions

2017-03-24 Thread Marek Olšák
On Fri, Mar 24, 2017 at 3:50 PM, Andy Furniss  wrote:
> Marek Olšák wrote:
>>
>> On Fri, Mar 24, 2017 at 12:24 PM, Emil Velikov 
>> wrote:
>>>
>>> On 24 March 2017 at 11:02, Nicolai Hähnle  wrote:

 On 24.03.2017 01:00, Marek Olšák wrote:
>
>
> From: Marek Olšák 
>
> This should fix this radeonsi error:
>   "mesa: for the -simplifycfg-sink-common option: may only occur zero
> or
> one
>times!"
>>>
>>> Can we have some commit message. Feel free to reuse the following:
>>>
>>> Earlier commit added a LLVM 4.0 workaround by passing
>>> -simplifycfg-sink-common=false to LLVM.
>>> When using multiple drivers, for example GL/dri and VAAPI, we may end
>>> up with the option being parsed multiple times.
>>> Hence we'll see errors like
>>>
>>>   "mesa: for the -simplifycfg-sink-common option: may only occur zero or
>>> one
>>>times!"
>>>
>>> Workaround this by exporting the driver entry point. This will lead to
>>> the function being called once.
>>>
>>> Fixes: 7751ed39e40 ("radeonsi: disable sinking common instructions
>>> down to the end block")
>>>
> ---
>  src/gallium/targets/va/va.sym | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/gallium/targets/va/va.sym
> b/src/gallium/targets/va/va.sym
> index c925b2e..b19bc36 100644
> --- a/src/gallium/targets/va/va.sym
> +++ b/src/gallium/targets/va/va.sym
> @@ -1,6 +1,8 @@
>  {
> global:
> __vaDriverInit_*_*;
> +   radeon_drm_winsys_create;
> +   amdgpu_winsys_create;
>>>
>>>
>>> Please add a reference to the si_shader_tgsi_setup.c and vice-versa.
>>> Otherwise we will end up removing one but not the other.
>>
>>
>> It's actually not a workaround. It's a fix of va.sym that happens to
>> fix that bug and it would be the correct thing to do even if there was
>> no issue. Thus, I think a reference to si_shader_tgsi_setup is
>> unnecessary. I'm not fixing the bug here. I'm just enforcing one
>> screen+winsys per device per process like we already do between GL and
>> VDPAU.
>>
>>>


 Oof, that's ugly, but at least it's only a change to va and the chance
 of
 collision is low. Have you verified that it fixes the bug?
>>
>>
>> I've not verified that, no. If it doesn't fix the bug, we still need
>> this patch because of aforementioned reasons, and then we have to fix
>> the bug.
>
>
> It fixes mpv for me testing vaapi hw decode + gl display.
>
> It doesn't fix ffmpeg cli though.
>
> ffmpeg  -hwaccel vaapi -i testfile -f null -
>
> still gets the message.

Thanks for testing. I wonder what other Mesa libs ffmpeg loads that
are missing the exported symbols.

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


Re: [Mesa-dev] [PATCH 3/3] targets/va: export radeon winsys_create functions

2017-03-24 Thread Andy Furniss

Marek Olšák wrote:

On Fri, Mar 24, 2017 at 12:24 PM, Emil Velikov  wrote:

On 24 March 2017 at 11:02, Nicolai Hähnle  wrote:

On 24.03.2017 01:00, Marek Olšák wrote:


From: Marek Olšák 

This should fix this radeonsi error:
  "mesa: for the -simplifycfg-sink-common option: may only occur zero or
one
   times!"

Can we have some commit message. Feel free to reuse the following:

Earlier commit added a LLVM 4.0 workaround by passing
-simplifycfg-sink-common=false to LLVM.
When using multiple drivers, for example GL/dri and VAAPI, we may end
up with the option being parsed multiple times.
Hence we'll see errors like

  "mesa: for the -simplifycfg-sink-common option: may only occur zero or one
   times!"

Workaround this by exporting the driver entry point. This will lead to
the function being called once.

Fixes: 7751ed39e40 ("radeonsi: disable sinking common instructions
down to the end block")


---
 src/gallium/targets/va/va.sym | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/gallium/targets/va/va.sym b/src/gallium/targets/va/va.sym
index c925b2e..b19bc36 100644
--- a/src/gallium/targets/va/va.sym
+++ b/src/gallium/targets/va/va.sym
@@ -1,6 +1,8 @@
 {
global:
__vaDriverInit_*_*;
+   radeon_drm_winsys_create;
+   amdgpu_winsys_create;


Please add a reference to the si_shader_tgsi_setup.c and vice-versa.
Otherwise we will end up removing one but not the other.


It's actually not a workaround. It's a fix of va.sym that happens to
fix that bug and it would be the correct thing to do even if there was
no issue. Thus, I think a reference to si_shader_tgsi_setup is
unnecessary. I'm not fixing the bug here. I'm just enforcing one
screen+winsys per device per process like we already do between GL and
VDPAU.






Oof, that's ugly, but at least it's only a change to va and the chance of
collision is low. Have you verified that it fixes the bug?


I've not verified that, no. If it doesn't fix the bug, we still need
this patch because of aforementioned reasons, and then we have to fix
the bug.


It fixes mpv for me testing vaapi hw decode + gl display.

It doesn't fix ffmpeg cli though.

ffmpeg  -hwaccel vaapi -i testfile -f null -

still gets the message.


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


Re: [Mesa-dev] [PATCH 3/3] targets/va: export radeon winsys_create functions

2017-03-24 Thread Marek Olšák
On Fri, Mar 24, 2017 at 12:24 PM, Emil Velikov  wrote:
> On 24 March 2017 at 11:02, Nicolai Hähnle  wrote:
>> On 24.03.2017 01:00, Marek Olšák wrote:
>>>
>>> From: Marek Olšák 
>>>
>>> This should fix this radeonsi error:
>>>   "mesa: for the -simplifycfg-sink-common option: may only occur zero or
>>> one
>>>times!"
> Can we have some commit message. Feel free to reuse the following:
>
> Earlier commit added a LLVM 4.0 workaround by passing
> -simplifycfg-sink-common=false to LLVM.
> When using multiple drivers, for example GL/dri and VAAPI, we may end
> up with the option being parsed multiple times.
> Hence we'll see errors like
>
>   "mesa: for the -simplifycfg-sink-common option: may only occur zero or one
>times!"
>
> Workaround this by exporting the driver entry point. This will lead to
> the function being called once.
>
> Fixes: 7751ed39e40 ("radeonsi: disable sinking common instructions
> down to the end block")
>
>>> ---
>>>  src/gallium/targets/va/va.sym | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/src/gallium/targets/va/va.sym b/src/gallium/targets/va/va.sym
>>> index c925b2e..b19bc36 100644
>>> --- a/src/gallium/targets/va/va.sym
>>> +++ b/src/gallium/targets/va/va.sym
>>> @@ -1,6 +1,8 @@
>>>  {
>>> global:
>>> __vaDriverInit_*_*;
>>> +   radeon_drm_winsys_create;
>>> +   amdgpu_winsys_create;
>
> Please add a reference to the si_shader_tgsi_setup.c and vice-versa.
> Otherwise we will end up removing one but not the other.

It's actually not a workaround. It's a fix of va.sym that happens to
fix that bug and it would be the correct thing to do even if there was
no issue. Thus, I think a reference to si_shader_tgsi_setup is
unnecessary. I'm not fixing the bug here. I'm just enforcing one
screen+winsys per device per process like we already do between GL and
VDPAU.

>
>>
>>
>> Oof, that's ugly, but at least it's only a change to va and the chance of
>> collision is low. Have you verified that it fixes the bug?

I've not verified that, no. If it doesn't fix the bug, we still need
this patch because of aforementioned reasons, and then we have to fix
the bug.

>
> Same question - does it work without the -Wl,--dynamic-list?

What is that?

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


  1   2   >