[Mesa-dev] [PATCH 4/5] iris: disable repacking for compression for applicable gen

[Dongwon Kim]

set bit15 (Disable Rebacking for Compression) of CACHE_MODE_0 register
if the gen attribute, 'disable_ccs_repack' is set.

Signed-off-by: Dongwon Kim 
---
 src/gallium/drivers/iris/iris_state.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/src/gallium/drivers/iris/iris_state.c 
b/src/gallium/drivers/iris/iris_state.c
index bf31f31f3e4..ce25f1ffcb3 100644
--- a/src/gallium/drivers/iris/iris_state.c
+++ b/src/gallium/drivers/iris/iris_state.c
@@ -755,6 +755,16 @@ iris_init_render_context(struct iris_screen *screen,
   }
   iris_emit_lri(batch, SLICE_COMMON_ECO_CHICKEN1, reg_val);
 
+  /* hardware specification recommends disabling repacking for
+   * the compatibility with decompression mechanism in display controller.
+   */
+  if (devinfo->disable_ccs_repack) {
+ iris_pack_state(GENX(CACHE_MODE_0), _val, reg) {
+reg.DisableRepackingforCompression = true;
+reg.DisableRepackingforCompressionMask = true;
+ }
+ iris_emit_lri(batch, CACHE_MODE_0, reg_val);
+  }
 
   // XXX: 3D_MODE?
 #endif
-- 
2.17.1

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

[Mesa-dev] [PATCH 5/5] anv: disable repacking for compression for applicable gen

[Dongwon Kim]

set bit15 (Disable Rebacking for Compression) of CACHE_MODE_0 register
if the gen attribute, 'disable_ccs_repack' is set.

Signed-off-by: Dongwon Kim 
---
 src/intel/vulkan/genX_state.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c
index 21b8cd648d4..c1163628cc0 100644
--- a/src/intel/vulkan/genX_state.c
+++ b/src/intel/vulkan/genX_state.c
@@ -225,6 +225,24 @@ genX(init_device_state)(struct anv_device *device)
}
 #endif
 
+#if GEN_GEN >= 11
+   /* hardware specification recommends disabling repacking for
+* the compatibility with decompression mechanism in display controller.
+*/
+   if (device->info.disable_ccs_repack) {
+  uint32_t cache_mode_0;
+  anv_pack_struct(_mode_0,
+  GENX(CACHE_MODE_0),
+  .DisableRepackingforCompression = true,
+  .DisableRepackingforCompressionMask = true);
+
+  anv_batch_emit(, GENX(MI_LOAD_REGISTER_IMM), lri) {
+ lri.RegisterOffset = GENX(CACHE_MODE_0_num);
+ lri.DataDWord  = cache_mode_0;
+  }
+   }
+#endif
+
/* Set the "CONSTANT_BUFFER Address Offset Disable" bit, so
 * 3DSTATE_CONSTANT_XS buffer 0 is an absolute address.
 *
-- 
2.17.1

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

[Mesa-dev] [PATCH 3/5] i965: disable repacking for compression for applicable gen

[Dongwon Kim]

set bit15 (Disable Rebacking for Compression) of CACHE_MODE_0 register
if the gen attribute, 'disable_ccs_repack' is set.

Signed-off-by: Dongwon Kim 
---
 src/mesa/drivers/dri/i965/brw_defines.h  | 1 +
 src/mesa/drivers/dri/i965/brw_state_upload.c | 9 +
 2 files changed, 10 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index 17bca1991f1..e8507b7e5ff 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1576,6 +1576,7 @@ enum brw_pixel_shader_coverage_mask_mode {
 # define GEN9_PARTIAL_RESOLVE_DISABLE_IN_VC (1 << 1)
 # define GEN8_HIZ_PMA_MASK_BITS \
REG_MASK(GEN8_HIZ_NP_PMA_FIX_ENABLE | GEN8_HIZ_NP_EARLY_Z_FAILS_DISABLE)
+# define GEN11_DISABLE_REPACKING_FOR_COMPRESSION (1 << 15)
 
 #define GEN7_GT_MODE0x7008
 # define GEN9_SUBSLICE_HASHING_8x8  (0 << 8)
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c 
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index 938b9defeda..09303600308 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -121,6 +121,15 @@ brw_upload_initial_gpu_state(struct brw_context *brw)

REG_MASK(GEN11_STATE_CACHE_REDIRECT_TO_CS_SECTION_ENABLE));
}
 
+   /* hardware specification recommends disabling repacking for
+* the compatibility with decompression mechanism in display controller.
+*/
+   if (devinfo->disable_ccs_repack) {
+  brw_load_register_imm32(brw, GEN7_CACHE_MODE_0,
+  GEN11_DISABLE_REPACKING_FOR_COMPRESSION |
+  
REG_MASK(GEN11_DISABLE_REPACKING_FOR_COMPRESSION));
+   }
+
if (devinfo->gen == 10 || devinfo->gen == 11) {
   /* From gen10 workaround table in h/w specs:
*
-- 
2.17.1

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

[Mesa-dev] [PATCH 1/5] intel/genxml: correct bit fields in CACHE_MODE_0 reg for gen11

[Dongwon Kim]

correct bit fields information of CACHE_MODE_0 reg in current gen11.xml

Signed-off-by: Dongwon Kim 
---
 src/intel/genxml/gen11.xml | 30 ++
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/src/intel/genxml/gen11.xml b/src/intel/genxml/gen11.xml
index 1579345f69f..c1774501f4c 100644
--- a/src/intel/genxml/gen11.xml
+++ b/src/intel/genxml/gen11.xml
@@ -6818,30 +6818,28 @@
   
 
   
-
+
 
-
+
 
-
-
-  
-  
-  
-
+
+
+
 
-
+
 
-
-
+
+
 
-
+
 
-
-
+
+
+
 
-
+
 
-
+
   
 
   
-- 
2.17.1

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

[Mesa-dev] [PATCH 2/5] intel: add disable_ccs_repack to gen_device_info

[Dongwon Kim]

add a new attribute, 'disable_ccs_repack' to gen_device info, which
indicates whether repacking of components in certain pixel formats
before compression needs to be disabled to keep the compatibility
with decompression capability of display controller (gen11+)

Signed-off-by: Dongwon Kim 
---
 src/intel/dev/gen_device_info.c | 3 +++
 src/intel/dev/gen_device_info.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/src/intel/dev/gen_device_info.c b/src/intel/dev/gen_device_info.c
index fec6159fd37..2b5be842a4e 100644
--- a/src/intel/dev/gen_device_info.c
+++ b/src/intel/dev/gen_device_info.c
@@ -958,6 +958,7 @@ static const struct gen_device_info gen_device_info_ehl_4x8 
= {
  [MESA_SHADER_GEOMETRY]  = 1032,
   },
},
+   .disable_ccs_repack = 1,
.simulator_id = 28,
 };
 
@@ -978,6 +979,7 @@ static const struct gen_device_info gen_device_info_ehl_4x4 
= {
  [MESA_SHADER_GEOMETRY]  = 1032,
   },
},
+   .disable_ccs_repack = 1,
.num_eu_per_subslice = 4,
.simulator_id = 28,
 };
@@ -999,6 +1001,7 @@ static const struct gen_device_info 
gen_device_info_ehl_2x4 = {
  [MESA_SHADER_GEOMETRY]  = 1032,
   },
},
+   .disable_ccs_repack = 1,
.num_eu_per_subslice =4,
.simulator_id = 28,
 };
diff --git a/src/intel/dev/gen_device_info.h b/src/intel/dev/gen_device_info.h
index af13615be2b..4fe937355a7 100644
--- a/src/intel/dev/gen_device_info.h
+++ b/src/intel/dev/gen_device_info.h
@@ -74,6 +74,7 @@ struct gen_device_info
bool has_surface_tile_offset;
bool supports_simd16_3src;
bool has_resource_streamer;
+   bool disable_ccs_repack;
 
/**
 * \name Intel hardware quirks
-- 
2.17.1

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

[Mesa-dev] [PATCH] iris/ehl: disable repacking for compression for compatibilty

[Dongwon Kim]

Repacking components in certain pixel formats before compression
shouldn't be done for EHL to keep the compatibility with decompression
capability in its display controller.

Signed-off-by: Dongwon Kim 
---
 src/gallium/drivers/iris/iris_state.c | 10 +
 src/intel/dev/gen_device_info.c   |  4 
 src/intel/dev/gen_device_info.h   |  1 +
 src/intel/genxml/gen11.xml| 30 +--
 4 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/src/gallium/drivers/iris/iris_state.c 
b/src/gallium/drivers/iris/iris_state.c
index fb72c4b5b7d..cc9e1148418 100644
--- a/src/gallium/drivers/iris/iris_state.c
+++ b/src/gallium/drivers/iris/iris_state.c
@@ -749,6 +749,16 @@ iris_init_render_context(struct iris_screen *screen,
   }
   iris_emit_lri(batch, SLICE_COMMON_ECO_CHICKEN1, reg_val);
 
+  /* needed to disable replacking to keep the compatibility with
+   * decompressing mechanism in display controller.
+   */
+  if (devinfo->need_disable_ccs_repack) {
+ iris_pack_state(GENX(CACHE_MODE_0), _val, reg) {
+reg.DisableRepackingforCompression = true;
+reg.DisableRepackingforCompressionMask = true;
+ }
+ iris_emit_lri(batch, CACHE_MODE_0, reg_val);
+  }
 
   // XXX: 3D_MODE?
 #endif
diff --git a/src/intel/dev/gen_device_info.c b/src/intel/dev/gen_device_info.c
index fec6159fd37..ee3ad7ba085 100644
--- a/src/intel/dev/gen_device_info.c
+++ b/src/intel/dev/gen_device_info.c
@@ -64,6 +64,7 @@ gen_device_name_to_pci_device_id(const char *name)
   { "cml", 0x9b41 },
   { "cnl", 0x5a52 },
   { "icl", 0x8a52 },
+  { "ehl", 0x4500 },
};
 
for (unsigned i = 0; i < ARRAY_SIZE(name_map); i++) {
@@ -958,6 +959,7 @@ static const struct gen_device_info gen_device_info_ehl_4x8 
= {
  [MESA_SHADER_GEOMETRY]  = 1032,
   },
},
+   .need_disable_ccs_repack = 1,
.simulator_id = 28,
 };
 
@@ -978,6 +980,7 @@ static const struct gen_device_info gen_device_info_ehl_4x4 
= {
  [MESA_SHADER_GEOMETRY]  = 1032,
   },
},
+   .need_disable_ccs_repack = 1,
.num_eu_per_subslice = 4,
.simulator_id = 28,
 };
@@ -999,6 +1002,7 @@ static const struct gen_device_info 
gen_device_info_ehl_2x4 = {
  [MESA_SHADER_GEOMETRY]  = 1032,
   },
},
+   .need_disable_ccs_repack = 1,
.num_eu_per_subslice =4,
.simulator_id = 28,
 };
diff --git a/src/intel/dev/gen_device_info.h b/src/intel/dev/gen_device_info.h
index af13615be2b..e90d31935f1 100644
--- a/src/intel/dev/gen_device_info.h
+++ b/src/intel/dev/gen_device_info.h
@@ -74,6 +74,7 @@ struct gen_device_info
bool has_surface_tile_offset;
bool supports_simd16_3src;
bool has_resource_streamer;
+   bool need_disable_ccs_repack;
 
/**
 * \name Intel hardware quirks
diff --git a/src/intel/genxml/gen11.xml b/src/intel/genxml/gen11.xml
index 1579345f69f..c1774501f4c 100644
--- a/src/intel/genxml/gen11.xml
+++ b/src/intel/genxml/gen11.xml
@@ -6818,30 +6818,28 @@
   
 
   
-
+
 
-
+
 
-
-
-  
-  
-  
-
+
+
+
 
-
+
 
-
-
+
+
 
-
+
 
-
-
+
+
+
 
-
+
 
-
+
   
 
   
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH] i965: don't check ccs_e support if isl_format is ISL_FORMAT_UNSUPPORTED

[Dongwon Kim]

Hi Lionel,

The original problem we saw happened when dri format is
__DRI_IMAGE_FORMAT_XBGR2101010, which doesn't have any corresponding
ISL format available.

And yes, I verified the problem we saw doesn't happen anymore
with the latest code base possibly with your fix. So I could turn down
this patch. However, in general, doesn't it make sense to stop when
there's no matched ISL format instead of passing it down, which leads to
assertion error?

Thanks,
DW

On Fri, Jul 06, 2018 at 02:14:42PM +0100, Lionel Landwerlin wrote:
> Hi Dongwon,
> 
> Jason & I merged some patches to fix similar issues a few weeks ago.
> I think we didn't change this function because a crash or hitting an assert
> is a good indication that something's gone wrong before we run into this
> function.
> 
> If you patch fixes an issue, could you give some detail about it?
> Maybe a gdb backtrace?
> 
> Thanks,
> 
> -
> Lionel
> 
> On 05/07/18 19:27, Dongwon Kim wrote:
> >'ISL_FORMAT_UNSUPPORTED' shouldn't be passed down for evaluation as it is
> >strictly prohibited in isl code (e.g. format_info_exists).
> >
> >Signed-off-by: Dongwon Kim 
> >---
> >  src/mesa/drivers/dri/i965/intel_screen.c | 12 ++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> >diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
> >b/src/mesa/drivers/dri/i965/intel_screen.c
> >index cb357419a7..a65042da72 100644
> >--- a/src/mesa/drivers/dri/i965/intel_screen.c
> >+++ b/src/mesa/drivers/dri/i965/intel_screen.c
> >@@ -346,8 +346,16 @@ modifier_is_supported(const struct gen_device_info 
> >*devinfo,
> > */
> >format = _mesa_format_fallback_rgbx_to_rgba(format);
> >format = _mesa_get_srgb_format_linear(format);
> >-  if (!isl_format_supports_ccs_e(devinfo,
> >- 
> >brw_isl_format_for_mesa_format(format)))
> >+
> >+  enum isl_format isl_format;
> >+  isl_format = brw_isl_format_for_mesa_format(format);
> >+
> >+  /* whether there is supported ISL format for given mesa format */
> >+  if (isl_format == ISL_FORMAT_UNSUPPORTED)
> >+ return false;
> >+
> >+  /* check if isl_fomat supports ccs_e */
> >+  if (!isl_format_supports_ccs_e(devinfo, isl_format))
> >   return false;
> > }
> 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] i965: don't check ccs_e support if isl_format is ISL_FORMAT_UNSUPPORTED

[Dongwon Kim]

'ISL_FORMAT_UNSUPPORTED' shouldn't be passed down for evaluation as it is
strictly prohibited in isl code (e.g. format_info_exists).

Signed-off-by: Dongwon Kim 
---
 src/mesa/drivers/dri/i965/intel_screen.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
b/src/mesa/drivers/dri/i965/intel_screen.c
index cb357419a7..a65042da72 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -346,8 +346,16 @@ modifier_is_supported(const struct gen_device_info 
*devinfo,
*/
   format = _mesa_format_fallback_rgbx_to_rgba(format);
   format = _mesa_get_srgb_format_linear(format);
-  if (!isl_format_supports_ccs_e(devinfo,
- brw_isl_format_for_mesa_format(format)))
+
+  enum isl_format isl_format;
+  isl_format = brw_isl_format_for_mesa_format(format);
+
+  /* whether there is supported ISL format for given mesa format */
+  if (isl_format == ISL_FORMAT_UNSUPPORTED)
+ return false;
+
+  /* check if isl_fomat supports ccs_e */
+  if (!isl_format_supports_ccs_e(devinfo, isl_format))
  return false;
}
 
-- 
2.18.0

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

Re: [Mesa-dev] [PATCH shaderdb] run: shader program file created via GetProgramBinary (v5)

[Dongwon Kim]

Thanks, Matt!!
I am glad to see it is finally merged. By the way, not sure if you
saw this patch.

[PATCH shaderdb] run: handling binding of attribute variable name (v2)
https://lists.freedesktop.org/archives/mesa-dev/2018-March/190179.html

This is required for pre-binding of uniforms. It would be great
if you can review and consider merging this as well.

Thanks again!

On Wed, May 23, 2018 at 12:57:09PM -0700, Matt Turner wrote:
> Hi Dongwon,
> 
> I just pushed this. No point in delaying any longer! :)
> 
> Thanks,
> Matt
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH shaderdb] run: handling binding of attribute variable name (v2)

[Dongwon Kim]

Optional binding of variables can be processed before linking shader
objects for creating shader program. It is activated by adding lines
with a keyword "BindAttribLoc" followed by name and index as,

"BindAttribLoc name_str1 "

For example,

[require]
..
BindAttrbLoc vertex 1
BindAttrbLoc coord 2
BindAttrbLoc col 3

This makes the shader-db run

glBindAttribLocation(p, 1, "vertex");
glBindAttribLocation(p, 2, "coord");
glBindAttribLocation(p, 3, "col");

before glLinkProgram() to include these binding info in binary shader
program.

v2: get_shaders returns its own head of list for binding variables
instead of using a global head to support parallel processing of
    multiple shader_tests

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 run.c | 81 +--
 1 file changed, 79 insertions(+), 2 deletions(-)

diff --git a/run.c b/run.c
index 4712e27..ce8ef41 100644
--- a/run.c
+++ b/run.c
@@ -71,6 +71,12 @@ struct shader {
 int type;
 };
 
+struct binding_list {
+char *name;
+GLint index;
+struct binding_list *prev;
+};
+
 static bool
 extension_in_string(const char *haystack, const char *needle)
 {
@@ -100,13 +106,15 @@ extension_in_string(const char *haystack, const char 
*needle)
 return false;
 }
 
+#define SKIP_SPACES(str) while (*(str) == ' ') str++
+
 static struct shader *
 get_shaders(const struct context_info *core, const struct context_info *compat,
 const struct context_info *es,
 const char *text, size_t text_size,
 enum shader_type *type, unsigned *num_shaders,
 bool *use_separate_shader_objects,
-const char *shader_name)
+const char *shader_name, struct binding_list *binding)
 {
 static const char *req = "[require]";
 static const char *gl_req = "\nGL >= ";
@@ -115,6 +123,7 @@ get_shaders(const struct context_info *core, const struct 
context_info *compat,
 static const char *fp_req = "\nGL_ARB_fragment_program";
 static const char *vp_req = "\nGL_ARB_vertex_program";
 static const char *sso_req = "\nSSO ENABLED";
+static const char *binding_req = "\nBindAttribLoc";
 static const char *gs = "geometry shader]\n";
 static const char *fs = "fragment ";
 static const char *vs = "vertex ";
@@ -181,11 +190,13 @@ get_shaders(const struct context_info *core, const struct 
context_info *compat,
 const struct context_info *info = *type == TYPE_CORE ? core : compat;
 
 const char *extension_text = text;
+
 while ((extension_text = memmem(extension_text, end_text - extension_text,
 "\nGL_", strlen("\nGL_"))) != NULL) {
 extension_text += 1;
 const char *newline = memchr(extension_text, '\n',
  end_text - extension_text);
+
 if (memmem(info->extension_string, info->extension_string_len,
extension_text, newline - extension_text) == NULL) {
 fprintf(stderr, "SKIP: %s requires unavailable extension %.*s\n",
@@ -197,6 +208,62 @@ get_shaders(const struct context_info *core, const struct 
context_info *compat,
 }
 }
 
+/* process binding */
+struct binding_list *binding_prev = binding = NULL;
+const char *pre_binding_text = text;
+
+/* binding = NULL if there's no binding required */
+binding = NULL;
+while ((pre_binding_text = memmem(pre_binding_text, end_text - 
pre_binding_text,
+  binding_req, strlen(binding_req))) != 
NULL) {
+pre_binding_text += strlen(binding_req);
+
+const char *newline = memchr(pre_binding_text, '\n', end_text - 
pre_binding_text);
+
+SKIP_SPACES(pre_binding_text);
+
+char *endword = memchr(pre_binding_text, ' ', newline - 
pre_binding_text);
+
+/* if there's no more space in the same line */
+if (!endword) {
+fprintf(stderr, "SKIP: can't find attr index for this binding\n");
+continue;
+}
+
+char *binding_name = (char *)calloc(1, endword - pre_binding_text + 1);
+
+strncpy(binding_name, pre_binding_text, endword - pre_binding_text);
+
+pre_binding_text = endword;
+
+SKIP_SPACES(pre_binding_text);
+if (*pre_binding_text == '\n') {
+fprintf(stderr, "SKIP: can't find attr variable name for this 
binding\n");
+continue;
+}
+
+endword = memchr(pre_binding_text, ' ', newline - pre_binding_text);
+
+if (!endword)
+endword = (char *)newline;
+
+char *index_string = calloc(1, endword - pre_binding_text + 1);
+strncpy(index_string, pre_binding_text, endword - pre_binding_text);
+
+

Re: [Mesa-dev] [PATCH shader-db 4/4] run: handling binding of attribute variable name

[Dongwon Kim]

I realized this model won't work with parellel execution.
I will fix it and post another version shortly.

On Wed, Mar 14, 2018 at 03:15:20PM -0700, Kenneth Graunke wrote:
> On Friday, March 9, 2018 2:28:36 PM PDT Dongwon Kim wrote:
> > Optional binding of variables can be processed before linking shader
> > objects for creating shader program. It is activated by adding lines
> > with a keyword "BindAttribLoc" followed by name and index as,
> > 
> > "BindAttribLoc name_str1 "
> > 
> > For example,
> > 
> > [require]
> > ..
> > BindAttrbLoc vertex 1
> > BindAttrbLoc coord 2
> > BindAttrbLoc col 3
> > 
> > This makes the shader-db run
> > 
> > glBindAttribLocation(p, 1, "vertex");
> > glBindAttribLocation(p, 2, "coord");
> > glBindAttribLocation(p, 3, "col");
> > 
> > before glLinkProgram() to include these binding info in binary shader
> > program.
> > 
> > Signed-off-by: Dongwon Kim <dongwon@intel.com>
> 
> Matt, do you have an opinion on this?  This seems like the sort of
> commands that would normally go in the [test] block, rather than the
> [require] block.  But it looks like shader_runner doesn't have any
> syntax for glBindAttribLocation today.
> 
> It's definitely a useful thing to have if we're going to use run.c
> to produce shader binaries for ARB_get_program_binary...
> 
> > ---
> >  run.c | 79 
> > +++
> >  1 file changed, 79 insertions(+)
> > 
> > diff --git a/run.c b/run.c
> > index bbab5d9..fe2a97a 100644
> > --- a/run.c
> > +++ b/run.c
> > @@ -76,6 +76,12 @@ struct shader {
> >  int type;
> >  };
> >  
> > +struct binding_var {
> > +char *name;
> > +GLint index;
> > +struct binding_var *next;
> > +};
> > +
> >  static bool
> >  extension_in_string(const char *haystack, const char *needle)
> >  {
> > @@ -105,6 +111,10 @@ extension_in_string(const char *haystack, const char 
> > *needle)
> >  return false;
> >  }
> >  
> > +#define SKIP_SPACES(str) while (*(str) == ' ') str++
> > +
> > +struct binding_var binding_head = {"NULL", -1, NULL};
> > +
> >  static struct shader *
> >  get_shaders(const struct context_info *core, const struct context_info 
> > *compat,
> >  const struct context_info *es,
> > @@ -120,6 +130,7 @@ get_shaders(const struct context_info *core, const 
> > struct context_info *compat,
> >  static const char *fp_req = "\nGL_ARB_fragment_program";
> >  static const char *vp_req = "\nGL_ARB_vertex_program";
> >  static const char *sso_req = "\nSSO ENABLED";
> > +static const char *binding = "\nBindAttribLoc";
> >  static const char *gs = "geometry shader]\n";
> >  static const char *fs = "fragment ";
> >  static const char *vs = "vertex ";
> > @@ -186,11 +197,13 @@ get_shaders(const struct context_info *core, const 
> > struct context_info *compat,
> >  const struct context_info *info = *type == TYPE_CORE ? core : compat;
> >  
> >  const char *extension_text = text;
> > +
> >  while ((extension_text = memmem(extension_text, end_text - 
> > extension_text,
> >  "\nGL_", strlen("\nGL_"))) != NULL) {
> >  extension_text += 1;
> >  const char *newline = memchr(extension_text, '\n',
> >   end_text - extension_text);
> > +
> >  if (memmem(info->extension_string, info->extension_string_len,
> > extension_text, newline - extension_text) == NULL) {
> >  fprintf(stderr, "SKIP: %s requires unavailable extension 
> > %.*s\n",
> > @@ -202,6 +215,62 @@ get_shaders(const struct context_info *core, const 
> > struct context_info *compat,
> >  }
> >  }
> >  
> > +/* process binding */
> > +struct binding_var *binding_prev = _head;
> > +const char *pre_binding_text = text;
> > +
> > +while ((pre_binding_text = memmem(pre_binding_text, end_text - 
> > pre_binding_text,
> > +  binding, strlen(binding))) != NULL) {
> > +pre_binding_text += strlen(binding);
> > +
> > +const char *newline = memchr(pre_binding_text, '\n', end_text - 
> > pre_binding_

[Mesa-dev] [PATCH shaderdb] run: shader program file created via GetProgramBinary (v5)

[Dongwon Kim]

With optin '-b', shader-db now generates a shader program binary file
using GetProgramBinary(). This shader program binary can be loaded via
ProgramBinary() to be executed by an application later.

v2: 1. define MAX_LOG_LEN and use it as the size of gl log
2. define MAX_PROG_SIZE and use it as the max size of extracted
   shader_program
3. out_file is now pointer allocated by strdup for the file name

v3: 1. automatically using original shader test file's name +  ".bin"
   as a filename for program binary - better way to cover the case
   with batch compilation of many shader test files in the same
   directory
2. remove --out= since it is now unnecessary (due to v3-1.)
   to provide custom file name. Instead, option, "--bin", which is
   basically a flag that enables getting program binary as a file.
3. Now it tries to get the length of binary by reading program's
   GL_PROGRAM_BINARY_LENGTH_OES parameter

v4: 1. '--bin' -> '-b'
2. stop generating binary program when failing to retrieve the binary
   size
3. error checking after malloc for binary program
4. changed some of variable names
5. several consecutive fprintfs are consolidated
6. removed MAX_LOG_LEN and MAX_PROG_SIZE

v5: bug fix: +1 to the length of the output file to cover '/0'

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 run.c | 81 ---
 1 file changed, 78 insertions(+), 3 deletions(-)

diff --git a/run.c b/run.c
index 69e64c7..4712e27 100644
--- a/run.c
+++ b/run.c
@@ -356,7 +356,8 @@ const struct platform platforms[] = {
 void print_usage(const char *prog_name)
 {
 fprintf(stderr,
-"Usage: %s [-d ] [-j ] [-o ] [-p 
] \n",
+"Usage: %s [-d ] [-j ] [-o ] "
+"[-p ] [-b] \n",
 prog_name);
 }
 
@@ -435,10 +436,11 @@ main(int argc, char **argv)
 char device_path[64];
 int device_id = 0;
 int opt;
+bool generate_prog_bin = 0;
 
 max_threads = omp_get_max_threads();
 
-while ((opt = getopt(argc, argv, "d:j:o:p:")) != -1) {
+while ((opt = getopt(argc, argv, "d:j:o:p:b")) != -1) {
 switch(opt) {
 case 'd': {
 char *endptr;
@@ -478,6 +480,9 @@ main(int argc, char **argv)
 case 'j':
 max_threads = atoi(optarg);
 break;
+case 'b':
+generate_prog_bin = 1;
+break;
 default:
 fprintf(stderr, "Unknown option: %x\n", opt);
 print_usage(argv[0]);
@@ -813,18 +818,24 @@ main(int argc, char **argv)
 const_text = text;
 GLuint prog = glCreateShaderProgramv(shader[i].type, 1,
  _text);
+
+if (generate_prog_bin)
+fprintf(stderr,
+"Currently, program binary generation "
+"doesn't support SSO.\n");
+
 glDeleteProgram(prog);
 free(text);
 }
 } else if (type == TYPE_CORE || type == TYPE_COMPAT || type == 
TYPE_ES) {
 GLuint prog = glCreateProgram();
+GLint param;
 
 for (unsigned i = 0; i < num_shaders; i++) {
 GLuint s = glCreateShader(shader[i].type);
 glShaderSource(s, 1, [i].text, [i].length);
 glCompileShader(s);
 
-GLint param;
 glGetShaderiv(s, GL_COMPILE_STATUS, );
 if (unlikely(!param)) {
 GLchar log[4096];
@@ -839,6 +850,70 @@ main(int argc, char **argv)
 }
 
 glLinkProgram(prog);
+
+glGetProgramiv(prog, GL_LINK_STATUS, );
+if (unlikely(!param)) {
+GLchar log[4096];
+GLsizei length;
+glGetProgramInfoLog(prog, sizeof(log), , log);
+
+fprintf(stderr, "ERROR: failed to link progam:\n%s\n",
+   log);
+} else if (generate_prog_bin) {
+/* generating shader program binary */
+char *prog_buf;
+GLenum format;
+GLsizei length = 0;
+FILE *fp;
+
+glGetProgramiv(prog, GL_PROGRAM_BINARY_LENGTH, );
+
+if (glGetError() != GL_NO_ERROR) {
+fprintf(stderr,
+"ERROR: failed to generate a program binary "
+"(invalid program size).\n");
+continue;
+}
+
+prog_buf = (char *)ma

[Mesa-dev] [PATCH shaderdb] run: shader program file created via GetProgramBinary (v4)

[Dongwon Kim]

With optin '-b', shader-db now generates a shader program binary file
using GetProgramBinary(). This shader program binary can be loaded via
ProgramBinary() to be executed by an application later.

v2: 1. define MAX_LOG_LEN and use it as the size of gl log
2. define MAX_PROG_SIZE and use it as the max size of extracted
   shader_program
3. out_file is now pointer allocated by strdup for the file name

v3: 1. automatically using original shader test file's name +  ".bin"
   as a filename for program binary - better way to cover the case
   with batch compilation of many shader test files in the same
   directory
2. remove --out= since it is now unnecessary (due to v3-1.)
   to provide custom file name. Instead, option, "--bin", which is
   basically a flag that enables getting program binary as a file.
3. Now it tries to get the length of binary by reading program's
   GL_PROGRAM_BINARY_LENGTH_OES parameter

v4: 1. '--bin' -> '-b'
2. stop generating binary program when failing to retrieve the binary
   size
3. error checking after malloc for binary program
4. changed some of variable names
5. several consecutive fprintfs are consolidated
6. removed MAX_LOG_LEN and MAX_PROG_SIZE

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 run.c | 81 ---
 1 file changed, 78 insertions(+), 3 deletions(-)

diff --git a/run.c b/run.c
index 69e64c7..bcec5ca 100644
--- a/run.c
+++ b/run.c
@@ -356,7 +356,8 @@ const struct platform platforms[] = {
 void print_usage(const char *prog_name)
 {
 fprintf(stderr,
-"Usage: %s [-d ] [-j ] [-o ] [-p 
] \n",
+"Usage: %s [-d ] [-j ] [-o ] "
+"[-p ] [-b] \n",
 prog_name);
 }
 
@@ -435,10 +436,11 @@ main(int argc, char **argv)
 char device_path[64];
 int device_id = 0;
 int opt;
+bool generate_prog_bin = 0;
 
 max_threads = omp_get_max_threads();
 
-while ((opt = getopt(argc, argv, "d:j:o:p:")) != -1) {
+while ((opt = getopt(argc, argv, "d:j:o:p:b")) != -1) {
 switch(opt) {
 case 'd': {
 char *endptr;
@@ -478,6 +480,9 @@ main(int argc, char **argv)
 case 'j':
 max_threads = atoi(optarg);
 break;
+case 'b':
+generate_prog_bin = 1;
+break;
 default:
 fprintf(stderr, "Unknown option: %x\n", opt);
 print_usage(argv[0]);
@@ -813,18 +818,24 @@ main(int argc, char **argv)
 const_text = text;
 GLuint prog = glCreateShaderProgramv(shader[i].type, 1,
  _text);
+
+if (generate_prog_bin)
+fprintf(stderr,
+"Currently, program binary generation "
+"doesn't support SSO.\n");
+
 glDeleteProgram(prog);
 free(text);
 }
 } else if (type == TYPE_CORE || type == TYPE_COMPAT || type == 
TYPE_ES) {
 GLuint prog = glCreateProgram();
+GLint param;
 
 for (unsigned i = 0; i < num_shaders; i++) {
 GLuint s = glCreateShader(shader[i].type);
 glShaderSource(s, 1, [i].text, [i].length);
 glCompileShader(s);
 
-GLint param;
 glGetShaderiv(s, GL_COMPILE_STATUS, );
 if (unlikely(!param)) {
 GLchar log[4096];
@@ -839,6 +850,70 @@ main(int argc, char **argv)
 }
 
 glLinkProgram(prog);
+
+glGetProgramiv(prog, GL_LINK_STATUS, );
+if (unlikely(!param)) {
+GLchar log[4096];
+GLsizei length;
+glGetProgramInfoLog(prog, sizeof(log), , log);
+
+fprintf(stderr, "ERROR: failed to link progam:\n%s\n",
+   log);
+} else if (generate_prog_bin) {
+/* generating shader program binary */
+char *prog_buf;
+GLenum format;
+GLsizei length = 0;
+FILE *fp;
+
+glGetProgramiv(prog, GL_PROGRAM_BINARY_LENGTH, );
+
+if (glGetError() != GL_NO_ERROR) {
+fprintf(stderr,
+"ERROR: failed to generate a program binary "
+"(invalid program size).\n");
+continue;
+}
+
+prog_buf = (char *)malloc(length);
+
+

[Mesa-dev] [PATCH shaderdb] run: -p option accepts hex format pci-id

[Dongwon Kim]

-p option now takes hex format pci-id of target architecture.

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 run.c | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/run.c b/run.c
index 69e64c7..3db97ec 100644
--- a/run.c
+++ b/run.c
@@ -356,7 +356,8 @@ const struct platform platforms[] = {
 void print_usage(const char *prog_name)
 {
 fprintf(stderr,
-"Usage: %s [-d ] [-j ] [-o ] [-p 
] \n",
+"Usage: %s [-d ] [-j ] [-o ] [-p 
] \n",
 prog_name);
 }
 
@@ -456,6 +457,7 @@ main(int argc, char **argv)
 break;
 case 'p': {
 const struct platform *platform = NULL;
+
 for (unsigned i = 0; i < ARRAY_SIZE(platforms); i++) {
 if (strcasecmp(optarg, platforms[i].name) == 0) {
 platform = platforms + i;
@@ -463,17 +465,30 @@ main(int argc, char **argv)
 }
 }
 
-if (platform == NULL) {
-fprintf(stderr, "Invalid platform.\nValid platforms are:");
-for (unsigned i = 0; i < ARRAY_SIZE(platforms); i++)
-fprintf(stderr, " %s", platforms[i].name);
-fprintf(stderr, "\n");
-return -1;
+if (platform) {
+printf("### Compiling for %s(PCI_ID=%s) ###\n", platform->name,
+   platform->pci_id);
+setenv("INTEL_DEVID_OVERRIDE", platform->pci_id, 1);
+break;
 }
 
-printf("### Compiling for %s ###\n", platform->name);
-setenv("INTEL_DEVID_OVERRIDE", platform->pci_id, 1);
-break;
+if (optarg[0] == '0' && optarg[1] == 'x') {
+/* check if rest of given string indicates hex number */
+if (strtol(optarg, NULL, 16) > 0) {
+setenv("INTEL_DEVID_OVERRIDE", optarg, 1);
+printf("### Compiling for GEN arch with PCI_ID=%s ###\n",
+   optarg);
+break;
+}
+}
+
+fprintf(stderr, "Invalid platform.\nValid platforms are:");
+for (unsigned i = 0; i < ARRAY_SIZE(platforms); i++)
+fprintf(stderr, " %s", platforms[i].name);
+
+fprintf(stderr, "\n");
+fprintf(stderr, "Or\nPCI-ID of other supported platform.\n");
+return -1;
 }
 case 'j':
 max_threads = atoi(optarg);
-- 
2.16.2

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

Re: [Mesa-dev] [PATCH shaderdb 2/3] run: new '--pci-id' option for overriding pci-id

[Dongwon Kim]

I see. Because user will be putting PCI_ID instead if a specific device
variant is required. I shouldn't have been confused in the first
place :-).

Thanks,

On Wed, Mar 14, 2018 at 04:03:05PM -0700, Kenneth Graunke wrote:
> On Wednesday, March 14, 2018 3:43:18 PM PDT Dongwon Kim wrote:
> > Yeah, thought about that (checking name then -> try to parse it as PCI-ID)
> > but didn't implement it because it won't work when there are multiple
> > different PCI-ID bound to same 'name' (e.g. want to use a specific PCI-ID
> > hsw). But wait a minite I think the opposite way (check if it's PCI-ID
> > first) should cover that case
> > 
> > I will upload v2 with this change shortly.
> 
> It should work either way... 'hsw' would pick some arbitrary Haswell
> PCI ID (if you don't care which one), and 0xD26 would pick a specific
> Haswell PCI ID.
> 
> --Ken


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

Re: [Mesa-dev] [PATCH shaderdb 3/3] run: shader program file created via GetProgramBinary (v3)

[Dongwon Kim]

Thanks for the review, Ken
I agree on most of your proposals.
I will upload another version shortly.

On Wed, Mar 14, 2018 at 03:10:25PM -0700, Kenneth Graunke wrote:
> On Monday, February 26, 2018 2:17:05 PM PDT Dongwon Kim wrote:
> > extraction of linked binary program to a file using glGetProgramBinary.
> > This file is intended to be loaded by glProgramBinary in the graphic
> > application running on the target system.
> > 
> > To enable this feature, a new option '--bin' has to be passed to the
> > program execution.
> > 
> > v2: 1. define MAX_LOG_LEN and use it as the size of gl log
> > 2. define MAX_PROG_SIZE and use it as the max size of extracted
> >shader_program
> > 3. out_file is now pointer allocated by strdup for the file name
> > 
> > v3: 1. automatically using original shader test file's name +  ".bin"
> >as a filename for program binary - better way to cover the case
> >with batch compilation of many shader test files in the same
> >directory
> > 2. remove --out= since it is now unnecessary (due to v3-1.)
> >to provide custom file name. Instead, option, "--bin", which is
> >basically a flag that enables getting program binary as a file.
> > 3. Now it tries to get the length of binary by reading program's
> >GL_PROGRAM_BINARY_LENGTH_OES parameter
> > 
> > Signed-off-by: Dongwon Kim <dongwon@intel.com>
> > ---
> >  run.c | 68 
> > +++
> >  1 file changed, 64 insertions(+), 4 deletions(-)
> > 
> > diff --git a/run.c b/run.c
> > index d066567..bbab5d9 100644
> > --- a/run.c
> > +++ b/run.c
> > @@ -52,6 +52,9 @@
> >  
> >  #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> >  
> > +#define MAX_LOG_LEN 4096
> > +#define MAX_PROG_SIZE (10*1024*1024) /* maximum 10MB for shader program */
> > +
> >  struct context_info {
> >  char *extension_string;
> >  int extension_string_len;
> > @@ -358,18 +361,20 @@ const struct platform platforms[] = {
> >  enum
> >  {
> >  PCI_ID_OVERRIDE_OPTION = CHAR_MAX + 1,
> > +LOADABLE_PROGRAM_BINARY_OPTION,
> >  };
> >  
> >  const struct option const long_options[] =
> >  {
> >  {"pciid", required_argument, NULL, PCI_ID_OVERRIDE_OPTION},
> > +{"bin", no_argument, NULL, LOADABLE_PROGRAM_BINARY_OPTION},
> 
> This sounds like we're loading binaries.  Can we call it
> GENERATE_PROGRAM_BINARY_OPTION instead?

Yeah, I will change this.

> 
> >  {NULL, 0, NULL, 0}
> >  };
> >  
> >  void print_usage(const char *prog_name)
> >  {
> >  fprintf(stderr,
> > -"Usage: %s [-d ] [-j ] [-o ] [-p 
> > ] [--pciid=]  > *.shader_test files>\n",
> > +"Usage: %s [-d ] [-j ] [-o ] [-p 
> > ] [--pciid=]  > *.shader_test files>\n",
> >  prog_name);
> >  }
> >  
> > @@ -450,6 +455,7 @@ main(int argc, char **argv)
> >  int opt;
> >  bool platf_overridden = 0;
> >  bool pci_id_overridden = 0;
> > +bool enable_prog_bin = 0;
> 
> Maybe generate_prog_bin here as well.

sure.

> 
> >  
> >  max_threads = omp_get_max_threads();
> >  
> > @@ -518,6 +524,9 @@ main(int argc, char **argv)
> >  setenv("INTEL_DEVID_OVERRIDE", optarg, 1);
> >  pci_id_overridden = 1;
> >  break;
> > +case LOADABLE_PROGRAM_BINARY_OPTION:
> > +enable_prog_bin = 1;
> > +break;
> >  default:
> >  fprintf(stderr, "Unknown option: %x\n", opt);
> >  print_usage(argv[0]);
> > @@ -858,18 +867,18 @@ main(int argc, char **argv)
> >  }
> >  } else if (type == TYPE_CORE || type == TYPE_COMPAT || type == 
> > TYPE_ES) {
> >  GLuint prog = glCreateProgram();
> > +GLint param;
> 
> So...putting this here means that you're not going to support generating
> program binaries for SSO-based programs.  That seems a bit unfortunate...
> 

I can consider this later.

> >  
> >  for (unsigned i = 0; i < num_shaders; i++) {
> >  GLuint s = glCreateShader(shader[i].type);
> >  glShaderSource(s, 1, [i].text, 
> > [i].length);
> >  glCompileShader(s);
> >  
> > -  

Re: [Mesa-dev] [PATCH shaderdb 2/3] run: new '--pci-id' option for overriding pci-id

[Dongwon Kim]

Yeah, I am using "intel_run" script that lets "run" use intel_stub
layer instead. Pretty useful..

On Wed, Mar 14, 2018 at 02:54:08PM -0700, Kenneth Graunke wrote:
> On Monday, February 12, 2018 5:26:15 PM PDT Dongwon Kim wrote:
> > Add a new option, '--pciid' to override a pci id of the target arch
> > to support cross-architecture shader compilation. Not like "-p" option,
> > it is for accepting any GFX devices supported by the driver.
> > 
> > Setting both "-p" and "--pciid" is blocked to avoid conflict.
> > 
> > Signed-off-by: Dongwon Kim <dongwon@intel.com>
> > ---
> >  run.c | 44 ++--
> >  1 file changed, 42 insertions(+), 2 deletions(-)
> 
> Oh, another thing I forgot to mention - you might find intel_run and
> intel_stub.c to be useful.  They allow you to emulate any Intel GPU
> for purposes of running shader-db, without needing to have any graphics
> hardware present in your system.  This can be useful when building on
> Xeon build servers or the like...


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

Re: [Mesa-dev] [PATCH shaderdb 2/3] run: new '--pci-id' option for overriding pci-id

[Dongwon Kim]

Yeah, thought about that (checking name then -> try to parse it as PCI-ID)
but didn't implement it because it won't work when there are multiple
different PCI-ID bound to same 'name' (e.g. want to use a specific PCI-ID
hsw). But wait a minite I think the opposite way (check if it's PCI-ID
first) should cover that case

I will upload v2 with this change shortly.

On Wed, Mar 14, 2018 at 02:52:36PM -0700, Kenneth Graunke wrote:
> On Monday, February 12, 2018 5:26:15 PM PDT Dongwon Kim wrote:
> > Add a new option, '--pciid' to override a pci id of the target arch
> > to support cross-architecture shader compilation. Not like "-p" option,
> > it is for accepting any GFX devices supported by the driver.
> > 
> > Setting both "-p" and "--pciid" is blocked to avoid conflict.
> > 
> > Signed-off-by: Dongwon Kim <dongwon@intel.com>
> > ---
> >  run.c | 44 ++--
> >  1 file changed, 42 insertions(+), 2 deletions(-)
> 
> Hi Dongwon,
> 
> It looks like this does the exact same thing as -p, but it accepts
> arbitrary numerical PCI IDs instead of a platform name.  IMHO, I think
> we should just make -p try parsing it as a number of it doesn't match
> any of the names.  Then either -p bdw or -p 0x1616 would work.
> 
> It seems much less complicated that way.
> 
> --Ken


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

[Mesa-dev] [PATCH shader-db 4/4] run: handling binding of attribute variable name

[Dongwon Kim]

Optional binding of variables can be processed before linking shader
objects for creating shader program. It is activated by adding lines
with a keyword "BindAttribLoc" followed by name and index as,

"BindAttribLoc name_str1 "

For example,

[require]
..
BindAttrbLoc vertex 1
BindAttrbLoc coord 2
BindAttrbLoc col 3

This makes the shader-db run

glBindAttribLocation(p, 1, "vertex");
glBindAttribLocation(p, 2, "coord");
glBindAttribLocation(p, 3, "col");

before glLinkProgram() to include these binding info in binary shader
program.

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 run.c | 79 +++
 1 file changed, 79 insertions(+)

diff --git a/run.c b/run.c
index bbab5d9..fe2a97a 100644
--- a/run.c
+++ b/run.c
@@ -76,6 +76,12 @@ struct shader {
 int type;
 };
 
+struct binding_var {
+char *name;
+GLint index;
+struct binding_var *next;
+};
+
 static bool
 extension_in_string(const char *haystack, const char *needle)
 {
@@ -105,6 +111,10 @@ extension_in_string(const char *haystack, const char 
*needle)
 return false;
 }
 
+#define SKIP_SPACES(str) while (*(str) == ' ') str++
+
+struct binding_var binding_head = {"NULL", -1, NULL};
+
 static struct shader *
 get_shaders(const struct context_info *core, const struct context_info *compat,
 const struct context_info *es,
@@ -120,6 +130,7 @@ get_shaders(const struct context_info *core, const struct 
context_info *compat,
 static const char *fp_req = "\nGL_ARB_fragment_program";
 static const char *vp_req = "\nGL_ARB_vertex_program";
 static const char *sso_req = "\nSSO ENABLED";
+static const char *binding = "\nBindAttribLoc";
 static const char *gs = "geometry shader]\n";
 static const char *fs = "fragment ";
 static const char *vs = "vertex ";
@@ -186,11 +197,13 @@ get_shaders(const struct context_info *core, const struct 
context_info *compat,
 const struct context_info *info = *type == TYPE_CORE ? core : compat;
 
 const char *extension_text = text;
+
 while ((extension_text = memmem(extension_text, end_text - extension_text,
 "\nGL_", strlen("\nGL_"))) != NULL) {
 extension_text += 1;
 const char *newline = memchr(extension_text, '\n',
  end_text - extension_text);
+
 if (memmem(info->extension_string, info->extension_string_len,
extension_text, newline - extension_text) == NULL) {
 fprintf(stderr, "SKIP: %s requires unavailable extension %.*s\n",
@@ -202,6 +215,62 @@ get_shaders(const struct context_info *core, const struct 
context_info *compat,
 }
 }
 
+/* process binding */
+struct binding_var *binding_prev = _head;
+const char *pre_binding_text = text;
+
+while ((pre_binding_text = memmem(pre_binding_text, end_text - 
pre_binding_text,
+  binding, strlen(binding))) != NULL) {
+pre_binding_text += strlen(binding);
+
+const char *newline = memchr(pre_binding_text, '\n', end_text - 
pre_binding_text);
+
+SKIP_SPACES(pre_binding_text);
+
+char *endword = memchr(pre_binding_text, ' ', newline - 
pre_binding_text);
+
+/* if there's no more space in the same line */
+if (!endword) {
+fprintf(stderr, "SKIP: can't find attr index for this binding\n");
+continue;
+}
+
+char *binding_name = (char *)calloc(1, endword - pre_binding_text + 1);
+
+strncpy(binding_name, pre_binding_text, endword - pre_binding_text);
+
+pre_binding_text = endword;
+
+SKIP_SPACES(pre_binding_text);
+if (*pre_binding_text == '\n') {
+fprintf(stderr, "SKIP: can't find attr variable name for this 
binding\n");
+continue;
+}
+
+endword = memchr(pre_binding_text, ' ', newline - pre_binding_text);
+
+if (!endword)
+endword = (char *)newline;
+
+char *index_string = calloc(1, endword - pre_binding_text + 1);
+strncpy(index_string, pre_binding_text, endword - pre_binding_text);
+
+struct binding_var *binding_new = malloc(sizeof(struct binding_var));
+
+binding_new->index = strtol(index_string, NULL, 10);
+binding_new->name = binding_name;
+binding_new->next = NULL;
+
+free(index_string);
+
+fprintf(stdout,
+"LOG: glBindAttribLocation(prog, %d, \"%s\") will be executed 
before linking\n",
+binding_new->index, binding_new->name);
+
+binding_prev->next = binding_new;
+binding_prev = binding_new;
+}
+
 /* Find the shaders. */
 unsigned s

[Mesa-dev] [PATCH shaderdb 3/3] run: shader program file created via GetProgramBinary (v3)

[Dongwon Kim]

extraction of linked binary program to a file using glGetProgramBinary.
This file is intended to be loaded by glProgramBinary in the graphic
application running on the target system.

To enable this feature, a new option '--bin' has to be passed to the
program execution.

v2: 1. define MAX_LOG_LEN and use it as the size of gl log
2. define MAX_PROG_SIZE and use it as the max size of extracted
   shader_program
3. out_file is now pointer allocated by strdup for the file name

v3: 1. automatically using original shader test file's name +  ".bin"
   as a filename for program binary - better way to cover the case
   with batch compilation of many shader test files in the same
   directory
2. remove --out= since it is now unnecessary (due to v3-1.)
   to provide custom file name. Instead, option, "--bin", which is
   basically a flag that enables getting program binary as a file.
3. Now it tries to get the length of binary by reading program's
   GL_PROGRAM_BINARY_LENGTH_OES parameter

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 run.c | 68 +++
 1 file changed, 64 insertions(+), 4 deletions(-)

diff --git a/run.c b/run.c
index d066567..bbab5d9 100644
--- a/run.c
+++ b/run.c
@@ -52,6 +52,9 @@
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
+#define MAX_LOG_LEN 4096
+#define MAX_PROG_SIZE (10*1024*1024) /* maximum 10MB for shader program */
+
 struct context_info {
 char *extension_string;
 int extension_string_len;
@@ -358,18 +361,20 @@ const struct platform platforms[] = {
 enum
 {
 PCI_ID_OVERRIDE_OPTION = CHAR_MAX + 1,
+LOADABLE_PROGRAM_BINARY_OPTION,
 };
 
 const struct option const long_options[] =
 {
 {"pciid", required_argument, NULL, PCI_ID_OVERRIDE_OPTION},
+{"bin", no_argument, NULL, LOADABLE_PROGRAM_BINARY_OPTION},
 {NULL, 0, NULL, 0}
 };
 
 void print_usage(const char *prog_name)
 {
 fprintf(stderr,
-"Usage: %s [-d ] [-j ] [-o ] [-p 
] [--pciid=] \n",
+"Usage: %s [-d ] [-j ] [-o ] [-p 
] [--pciid=] \n",
 prog_name);
 }
 
@@ -450,6 +455,7 @@ main(int argc, char **argv)
 int opt;
 bool platf_overridden = 0;
 bool pci_id_overridden = 0;
+bool enable_prog_bin = 0;
 
 max_threads = omp_get_max_threads();
 
@@ -518,6 +524,9 @@ main(int argc, char **argv)
 setenv("INTEL_DEVID_OVERRIDE", optarg, 1);
 pci_id_overridden = 1;
 break;
+case LOADABLE_PROGRAM_BINARY_OPTION:
+enable_prog_bin = 1;
+break;
 default:
 fprintf(stderr, "Unknown option: %x\n", opt);
 print_usage(argv[0]);
@@ -858,18 +867,18 @@ main(int argc, char **argv)
 }
 } else if (type == TYPE_CORE || type == TYPE_COMPAT || type == 
TYPE_ES) {
 GLuint prog = glCreateProgram();
+GLint param;
 
 for (unsigned i = 0; i < num_shaders; i++) {
 GLuint s = glCreateShader(shader[i].type);
 glShaderSource(s, 1, [i].text, [i].length);
 glCompileShader(s);
 
-GLint param;
 glGetShaderiv(s, GL_COMPILE_STATUS, );
 if (unlikely(!param)) {
-GLchar log[4096];
+GLchar log[MAX_LOG_LEN];
 GLsizei length;
-glGetShaderInfoLog(s, 4096, , log);
+glGetShaderInfoLog(s, sizeof(log), , log);
 
 fprintf(stderr, "ERROR: %s failed to compile:\n%s\n",
 current_shader_name, log);
@@ -879,6 +888,57 @@ main(int argc, char **argv)
 }
 
 glLinkProgram(prog);
+
+glGetProgramiv(prog, GL_LINK_STATUS, );
+if (unlikely(!param)) {
+   GLchar log[MAX_LOG_LEN];
+   GLsizei length;
+   glGetProgramInfoLog(prog, sizeof(log), , log);
+
+   fprintf(stderr, "ERROR: failed to link progam:\n%s\n",
+   log);
+} else if (enable_prog_bin) { /* generate program binary if 
enable_prog_bin == true */
+   char *prog_buf;
+   GLenum format;
+   GLsizei length = 0;
+   FILE *fp;
+
+   glGetProgramiv(prog,
+  (type == TYPE_ES) ? 
GL_PROGRAM_BINARY_LENGTH_OES :
+  GL_PROGRAM_BINARY_LENGTH, );
+
+   if (glGetError() != GL_NO_ERROR)
+   length = MAX_PROG_SIZE;
+
+   prog_buf = (char *)malloc(length);
+   glGetProgramBinary(prog, length, , , 
prog_buf);
+

[Mesa-dev] [PATCH shaderdb 3/3] run: shader program file created via GetProgramBinary (v2)

[Dongwon Kim]

extraction of linked binary program to a file using glGetProgramBinary.
This file is intended to be loaded by glProgramBinary in the graphic
application running on the target system.

A new option, '--out=' is available to be used for specifying
the output file name.

v2: 1. define MAX_LOG_LEN and use it as the size of gl log
2. define MAX_PROG_SIZE and use it as the max size of extracted
   shader_program
3. out_file is now pointer allocated by strdup for the file name

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 run.c | 57 +
 1 file changed, 53 insertions(+), 4 deletions(-)

diff --git a/run.c b/run.c
index d066567..df466eb 100644
--- a/run.c
+++ b/run.c
@@ -52,6 +52,9 @@
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
+#define MAX_LOG_LEN 4096
+#define MAX_PROG_SIZE (10*1024*1024) /* maximum 10MB for shader program */
+
 struct context_info {
 char *extension_string;
 int extension_string_len;
@@ -358,18 +361,20 @@ const struct platform platforms[] = {
 enum
 {
 PCI_ID_OVERRIDE_OPTION = CHAR_MAX + 1,
+OUT_PROGRAM_OPTION,
 };
 
 const struct option const long_options[] =
 {
 {"pciid", required_argument, NULL, PCI_ID_OVERRIDE_OPTION},
+{"out", required_argument, NULL, OUT_PROGRAM_OPTION},
 {NULL, 0, NULL, 0}
 };
 
 void print_usage(const char *prog_name)
 {
 fprintf(stderr,
-"Usage: %s [-d ] [-j ] [-o ] [-p 
] [--pciid=] \n",
+"Usage: %s [-d ] [-j ] [-o ] [-p 
] [--pciid=] [--out=] \n",
 prog_name);
 }
 
@@ -450,6 +455,7 @@ main(int argc, char **argv)
 int opt;
 bool platf_overridden = 0;
 bool pci_id_overridden = 0;
+char *out_file = NULL;
 
 max_threads = omp_get_max_threads();
 
@@ -518,6 +524,14 @@ main(int argc, char **argv)
 setenv("INTEL_DEVID_OVERRIDE", optarg, 1);
 pci_id_overridden = 1;
 break;
+case OUT_PROGRAM_OPTION:
+if (optarg[0] == 0) {
+  fprintf(stderr, "Output file name is empty.\n");
+  return -1;
+}
+out_file = strdup(optarg);
+assert(out_file != NULL);
+break;
 default:
 fprintf(stderr, "Unknown option: %x\n", opt);
 print_usage(argv[0]);
@@ -751,6 +765,8 @@ main(int argc, char **argv)
 EGLContext compat_ctx = create_context(egl_dpy, cfg, TYPE_COMPAT);
 if (compat_ctx == EGL_NO_CONTEXT) {
 fprintf(stderr, "ERROR: eglCreateContext() failed\n");
+if (out_file)
+free(out_file);
 exit(-1);
 }
 
@@ -858,18 +874,18 @@ main(int argc, char **argv)
 }
 } else if (type == TYPE_CORE || type == TYPE_COMPAT || type == 
TYPE_ES) {
 GLuint prog = glCreateProgram();
+GLint param;
 
 for (unsigned i = 0; i < num_shaders; i++) {
 GLuint s = glCreateShader(shader[i].type);
 glShaderSource(s, 1, [i].text, [i].length);
 glCompileShader(s);
 
-GLint param;
 glGetShaderiv(s, GL_COMPILE_STATUS, );
 if (unlikely(!param)) {
-GLchar log[4096];
+GLchar log[MAX_LOG_LEN];
 GLsizei length;
-glGetShaderInfoLog(s, 4096, , log);
+glGetShaderInfoLog(s, sizeof(log), , log);
 
 fprintf(stderr, "ERROR: %s failed to compile:\n%s\n",
 current_shader_name, log);
@@ -879,6 +895,36 @@ main(int argc, char **argv)
 }
 
 glLinkProgram(prog);
+
+glGetProgramiv(prog, GL_LINK_STATUS, );
+if (unlikely(!param)) {
+   GLchar log[MAX_LOG_LEN];
+   GLsizei length;
+   glGetProgramInfoLog(prog, sizeof(log), , log);
+
+   fprintf(stderr, "ERROR: failed to link progam:\n%s\n",
+   log);
+} else {
+   char *prog_buf = (char *)malloc(MAX_PROG_SIZE);
+   GLenum format;
+   GLsizei length;
+   FILE *fp;
+
+   glGetProgramBinary(prog, MAX_PROG_SIZE, , , 
prog_buf);
+
+   param = glGetError();
+   if (param != GL_NO_ERROR) {
+  fprintf(stderr, "ERROR: failed to get Program Binary\n");
+   } else {
+  fp = fopen(out_file, "wb");
+  fprintf(stdout, "Binary program is generated (%d 
Byte).\n", length);
+  fprintf(stdout, "Binary Format is %d\n&

Re: [Mesa-dev] [PATCH shaderdb 3/3] run: shader program file created via GetProgramBinary

[Dongwon Kim]

Thanks for the review. I put my comments below yours.

On Wed, Feb 14, 2018 at 10:56:14AM +0200, Eero Tamminen wrote:
> Hi,
> 
> On 13.02.2018 03:26, Dongwon Kim wrote:
> >extraction of linked binary program to a file using glGetProgramBinary.
> >This file is intended to be loaded by glProgramBinary in the graphic
> >application running on the target system.
> >
> >A new option, '--out=' is available to be used for specifying
> >the output file name.
> >
> >Signed-off-by: Dongwon Kim <dongwon@intel.com>
> >---
> >  run.c | 46 --
> >  1 file changed, 44 insertions(+), 2 deletions(-)
> >
> >diff --git a/run.c b/run.c
> >index d066567..54575e1 100644
> >--- a/run.c
> >+++ b/run.c
> >@@ -358,18 +358,20 @@ const struct platform platforms[] = {
> >  enum
> >  {
> >  PCI_ID_OVERRIDE_OPTION = CHAR_MAX + 1,
> >+OUT_PROGRAM_OPTION,
> >  };
> >  const struct option const long_options[] =
> >  {
> >  {"pciid", required_argument, NULL, PCI_ID_OVERRIDE_OPTION},
> >+{"out", required_argument, NULL, OUT_PROGRAM_OPTION},
> >  {NULL, 0, NULL, 0}
> >  };
> >  void print_usage(const char *prog_name)
> >  {
> >  fprintf(stderr,
> >-"Usage: %s [-d ] [-j ] [-o ] [-p 
> >] [--pciid=]  >*.shader_test files>\n",
> >+"Usage: %s [-d ] [-j ] [-o ] [-p 
> >] [--pciid=] [--out= >output shader program>] \n",
> >  prog_name);
> >  }
> >@@ -450,6 +452,7 @@ main(int argc, char **argv)
> >  int opt;
> >  bool platf_overridden = 0;
> >  bool pci_id_overridden = 0;
> >+char out_file[64] = {0};
> 
> File names can be potentially thousands of chars long.
> 
> Why not just use:
>   const char *out_file = NULL;
> ?
> 
> >  max_threads = omp_get_max_threads();
> >@@ -518,6 +521,13 @@ main(int argc, char **argv)
> >  setenv("INTEL_DEVID_OVERRIDE", optarg, 1);
> >  pci_id_overridden = 1;
> >  break;
> >+case OUT_PROGRAM_OPTION:
> >+if (optarg[0] == 0) {
> >+  fprintf(stderr, "Output file name is empty.\n");
> >+  return -1;
> >+}
> >+strncpy(out_file, optarg, 64);
> 
> ...and if pointer cannot assigned directly, strdup & assert if that fails.

yeah, your proposal sounds better. I will do so.

> 
> 
> >+break;
> >  default:
> >  fprintf(stderr, "Unknown option: %x\n", opt);
> >  print_usage(argv[0]);
> >@@ -858,13 +868,13 @@ main(int argc, char **argv)
> >  }
> >  } else if (type == TYPE_CORE || type == TYPE_COMPAT || type == 
> > TYPE_ES) {
> >  GLuint prog = glCreateProgram();
> >+GLint param;
> >  for (unsigned i = 0; i < num_shaders; i++) {
> >  GLuint s = glCreateShader(shader[i].type);
> >  glShaderSource(s, 1, [i].text, 
> > [i].length);
> >  glCompileShader(s);
> >-GLint param;
> >  glGetShaderiv(s, GL_COMPILE_STATUS, );
> >  if (unlikely(!param)) {
> >  GLchar log[4096];
> >@@ -879,6 +889,38 @@ main(int argc, char **argv)
> >  }
> >  glLinkProgram(prog);
> >+
> >+glGetProgramiv(prog, GL_LINK_STATUS, );
> >+if (unlikely(!param)) {
> >+   GLchar log[4096];
> 
> Maybe add define for log buffer size as it's used in multiple places?

I just followed the existing notation. However, I agree with you on this.
I am going to define a constant and use it instead.

> 
> 
> >+   GLsizei length;
> >+   glGetProgramInfoLog(prog, 4096, , log);
> 
> 4096 -> sizeof(log)
> 
> 
> >+
> >+   fprintf(stderr, "ERROR: failed to link progam:\n%s\n",
> >+   log);
> >+} else {
> >+   if (out_file[0] != 0) {
> 
> If changed to pointer, check for NULL.

With assert(outfile != NULL) above, I don't think I actually need this condition
check..

> 
> 
> >+  char *prog_buf = (char *)malloc(10*1024*1024);
> >+  GLenum format;
> >+ 

[Mesa-dev] [PATCH shaderdb 2/3] run: new '--pci-id' option for overriding pci-id

[Dongwon Kim]

Add a new option, '--pciid' to override a pci id of the target arch
to support cross-architecture shader compilation. Not like "-p" option,
it is for accepting any GFX devices supported by the driver.

Setting both "-p" and "--pciid" is blocked to avoid conflict.

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 run.c | 44 ++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/run.c b/run.c
index 23d2b07..d066567 100644
--- a/run.c
+++ b/run.c
@@ -36,6 +36,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -353,10 +355,21 @@ const struct platform platforms[] = {
 "skl",  "0x191D",
 };
 
+enum
+{
+PCI_ID_OVERRIDE_OPTION = CHAR_MAX + 1,
+};
+
+const struct option const long_options[] =
+{
+{"pciid", required_argument, NULL, PCI_ID_OVERRIDE_OPTION},
+{NULL, 0, NULL, 0}
+};
+
 void print_usage(const char *prog_name)
 {
 fprintf(stderr,
-"Usage: %s [-d ] [-j ] [-o ] [-p 
] \n",
+"Usage: %s [-d ] [-j ] [-o ] [-p 
] [--pciid=] \n",
 prog_name);
 }
 
@@ -435,10 +448,13 @@ main(int argc, char **argv)
 char device_path[64];
 int device_id = 0;
 int opt;
+bool platf_overridden = 0;
+bool pci_id_overridden = 0;
 
 max_threads = omp_get_max_threads();
 
-while ((opt = getopt(argc, argv, "d:j:o:p:")) != -1) {
+while ((opt = getopt_long(argc, argv, "d:j:o:p:",
+  long_options, NULL)) != -1) {
 switch(opt) {
 case 'd': {
 char *endptr;
@@ -456,6 +472,13 @@ main(int argc, char **argv)
 break;
 case 'p': {
 const struct platform *platform = NULL;
+
+if (pci_id_overridden) {
+unsetenv("INTEL_DEVID_OVERRIDE");
+fprintf(stderr, "'-p' and '--pciid' can't be used 
together.\n");
+return -1;
+}
+
 for (unsigned i = 0; i < ARRAY_SIZE(platforms); i++) {
 if (strcmp(optarg, platforms[i].name) == 0) {
 platform = platforms + i;
@@ -473,11 +496,28 @@ main(int argc, char **argv)
 
 printf("### Compiling for %s ###\n", platform->name);
 setenv("INTEL_DEVID_OVERRIDE", platform->pci_id, 1);
+platf_overridden = 1;
 break;
 }
 case 'j':
 max_threads = atoi(optarg);
 break;
+case PCI_ID_OVERRIDE_OPTION:
+if (platf_overridden) {
+unsetenv("INTEL_DEVID_OVERRIDE");
+fprintf(stderr, "'-p' and '--pciid' can't be used 
together.\n");
+return -1;
+}
+
+if (optarg[0] != '0' || optarg[1] != 'x') {
+  fprintf(stderr, "pci-id should be a hex number starting with 
'0x'\n");
+  return -1;
+}
+
+printf("### Compiling for GEN arch with PCI_ID=%s ###\n", optarg);
+setenv("INTEL_DEVID_OVERRIDE", optarg, 1);
+pci_id_overridden = 1;
+break;
 default:
 fprintf(stderr, "Unknown option: %x\n", opt);
 print_usage(argv[0]);
-- 
2.16.1

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

[Mesa-dev] [PATCH shaderdb 3/3] run: shader program file created via GetProgramBinary

[Dongwon Kim]

extraction of linked binary program to a file using glGetProgramBinary.
This file is intended to be loaded by glProgramBinary in the graphic
application running on the target system.

A new option, '--out=' is available to be used for specifying
the output file name.

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 run.c | 46 --
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/run.c b/run.c
index d066567..54575e1 100644
--- a/run.c
+++ b/run.c
@@ -358,18 +358,20 @@ const struct platform platforms[] = {
 enum
 {
 PCI_ID_OVERRIDE_OPTION = CHAR_MAX + 1,
+OUT_PROGRAM_OPTION,
 };
 
 const struct option const long_options[] =
 {
 {"pciid", required_argument, NULL, PCI_ID_OVERRIDE_OPTION},
+{"out", required_argument, NULL, OUT_PROGRAM_OPTION},
 {NULL, 0, NULL, 0}
 };
 
 void print_usage(const char *prog_name)
 {
 fprintf(stderr,
-"Usage: %s [-d ] [-j ] [-o ] [-p 
] [--pciid=] \n",
+"Usage: %s [-d ] [-j ] [-o ] [-p 
] [--pciid=] [--out=] \n",
 prog_name);
 }
 
@@ -450,6 +452,7 @@ main(int argc, char **argv)
 int opt;
 bool platf_overridden = 0;
 bool pci_id_overridden = 0;
+char out_file[64] = {0};
 
 max_threads = omp_get_max_threads();
 
@@ -518,6 +521,13 @@ main(int argc, char **argv)
 setenv("INTEL_DEVID_OVERRIDE", optarg, 1);
 pci_id_overridden = 1;
 break;
+case OUT_PROGRAM_OPTION:
+if (optarg[0] == 0) {
+  fprintf(stderr, "Output file name is empty.\n");
+  return -1;
+}
+strncpy(out_file, optarg, 64);
+break;
 default:
 fprintf(stderr, "Unknown option: %x\n", opt);
 print_usage(argv[0]);
@@ -858,13 +868,13 @@ main(int argc, char **argv)
 }
 } else if (type == TYPE_CORE || type == TYPE_COMPAT || type == 
TYPE_ES) {
 GLuint prog = glCreateProgram();
+GLint param;
 
 for (unsigned i = 0; i < num_shaders; i++) {
 GLuint s = glCreateShader(shader[i].type);
 glShaderSource(s, 1, [i].text, [i].length);
 glCompileShader(s);
 
-GLint param;
 glGetShaderiv(s, GL_COMPILE_STATUS, );
 if (unlikely(!param)) {
 GLchar log[4096];
@@ -879,6 +889,38 @@ main(int argc, char **argv)
 }
 
 glLinkProgram(prog);
+
+glGetProgramiv(prog, GL_LINK_STATUS, );
+if (unlikely(!param)) {
+   GLchar log[4096];
+   GLsizei length;
+   glGetProgramInfoLog(prog, 4096, , log);
+
+   fprintf(stderr, "ERROR: failed to link progam:\n%s\n",
+   log);
+} else {
+   if (out_file[0] != 0) {
+  char *prog_buf = (char *)malloc(10*1024*1024);
+  GLenum format;
+  GLsizei length;
+  FILE *fp;
+
+  glGetProgramBinary(prog, 10*1024*1024, , , 
prog_buf);
+
+  param = glGetError();
+  if (param != GL_NO_ERROR) {
+ fprintf(stderr, "ERROR: failed to get Program 
Binary\n");
+  } else {
+ fp = fopen(out_file, "wb");
+ fprintf(stdout, "Binary program is generated (%d 
Byte).\n", length);
+ fprintf(stdout, "Binary Format is %d\n", format);
+ fprintf(stdout, "Now writing to the file\n");
+ fwrite(prog_buf, sizeof(char), length, fp);
+ fclose(fp);
+  }
+  free(prog_buf);
+   }
+}
 glDeleteProgram(prog);
 } else {
 for (unsigned i = 0; i < num_shaders; i++) {
-- 
2.16.1

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

[Mesa-dev] [PATCH shaderdb 1/3] intel_stub: override pci-id only if INTEL_DEVID_OVERRIDE is set

[Dongwon Kim]

To prevent a segfault, pci-id is set only if INTEL_DEVID_OVERRIDE exists.

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 intel_stub.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/intel_stub.c b/intel_stub.c
index ea88400..cf9ddff 100644
--- a/intel_stub.c
+++ b/intel_stub.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -174,6 +175,7 @@ ioctl(int fd, unsigned long request, ...)
va_list args;
void *argp;
struct stat buf;
+   char *pci_id;
 
va_start(args, request);
argp = va_arg(args, void *);
@@ -199,7 +201,13 @@ ioctl(int fd, unsigned long request, ...)
 *getparam->value = 1;
 break;
 case I915_PARAM_CHIPSET_ID:
-*getparam->value = 
strtod(getenv("INTEL_DEVID_OVERRIDE"), NULL);
+pci_id = getenv("INTEL_DEVID_OVERRIDE");
+
+if (pci_id)
+*getparam->value = strtod(pci_id, NULL);
+else
+return -EINVAL;
+
 break;
 case I915_PARAM_CMD_PARSER_VERSION:
 *getparam->value = 9;
-- 
2.16.1

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

[Mesa-dev] [PATCH shaderdb 0/3] offline building of shader program for a desired target

[Dongwon Kim]

This series of changes are for making shaderdb as a complete standalone 
compiler that
can create a shader program in binary form (using glGetProgramBinary), which 
can later
be loaded on the target system specified by user (glProgramBinary).

As a prerequisite, the patch "run: new '--pci-id' option for overriding pci-id"
was written to add support for other GEN architectures that is not listed in
run.c.

The first patch, "intel_stub: override pci-id only if INTEL_DEVID_OVERRIDE is 
set"
is for fixing a segfault problem when ./intel_run is executed without
INTEL_DEVID_OVERRIDE.

Dongwon Kim (3):
  intel_stub: override pci-id only if INTEL_DEVID_OVERRIDE is set
  run: new '--pci-id' option for overriding pci-id
  run: shader program file created via GetProgramBinary

 intel_stub.c | 10 ++-
 run.c| 88 +---
 2 files changed, 94 insertions(+), 4 deletions(-)

-- 
2.16.1

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

Re: [Mesa-dev] [PATCH] egl: reset 'ViewportInitialized' when unbinding current context

[Dongwon Kim]

Yeah, I understood your point. Manual updating of viewport and
drawable seems to be needed in this case unless we change
EGL specificiation. Thanks

On Tue, Aug 23, 2016 at 05:51:23PM +0100, Emil Velikov wrote:
> On 23 August 2016 at 17:36, Dongwon Kim <dongwon@intel.com> wrote:
> > I read that part in egl specification as well but I wasn't sure
> > if this statement covers the case when the context is unbound via
> > eglMakeCurrent call with NULL context then it is made current again.
> > The problem I saw was when the context is made current again especially
> > with a new surface with different dimension, there's no way the context
> > updates its viewport accordingly.
> >
> In that case one should adjust the viewport manually as mentioned in the spec 
> ?
> 
> Also note that the src/mesa/ code is _not_ EGL but EGL/GLX agnostic,
> so one should we
> quite careful that changes don't break in either one of the two ;-)
> 
> -Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] egl: reset 'ViewportInitialized' when unbinding current context

[Dongwon Kim]

I read that part in egl specification as well but I wasn't sure 
if this statement covers the case when the context is unbound via 
eglMakeCurrent call with NULL context then it is made current again.
The problem I saw was when the context is made current again especially 
with a new surface with different dimension, there's no way the context 
updates its viewport accordingly. 

On Tue, Aug 23, 2016 at 12:23:47PM +0100, Emil Velikov wrote:
> On 15 August 2016 at 23:58, Dongwon Kim <dongwon@intel.com> wrote:
> > 'ViewportInitialized' flag in gl_context has to be reset to '0'
> > when the current context is unbound via a eglMakeCurrent call with
> > all of 'NULL' resources (surfaces and context).
> >
> > This is to make sure the viewport of the context is re-initialized
> > when the same context is bound to new read and draw surfaces
> > (or same surfaces but with different size.) next time when the
> > context is made current again.
> >
> The spec is pretty clear about this:
> 
> "The first time a OpenGL or OpenGL ES context is made current the
> viewport and scissor dimensions are set to the size of the draw
> surface (as though glViewport(0, 0, w, h) and glScissor(0, 0, w, h)
> were called, where w and h are the width and height of the surface,
> respectively). However, the viewport and scissor dimensions are not
> modified when ctx is subsequently made current."
> 
> So unless coffee hasn't kicked this patch is wrong.
> Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] egl/dri2: Do not need to check return value from mtx_lock

[Dongwon Kim]

Sure, I will. Thanks

On Tue, Aug 23, 2016 at 11:39:52AM +0100, Emil Velikov wrote:
> Hi Dongwon,
> 
> On 15 August 2016 at 23:20, Dongwon Kim <dongwon@intel.com> wrote:
> > A new patch, "[PATCH] egl/dri2: remove error checks on return values from 
> > mtx_lock
> > and cnd_wait" containing additional clean-up has been submitted. Please 
> > disregard
> > this one.
> >
> Thanks for the update. For the future please add this is information
> in the new patch (after the --- line). Using [PATCH v2] and revision
> log would also be appreciated.
> 
> -Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] egl: reset 'ViewportInitialized' when unbinding current context

[Dongwon Kim]

'ViewportInitialized' flag in gl_context has to be reset to '0'
when the current context is unbound via a eglMakeCurrent call with
all of 'NULL' resources (surfaces and context).

This is to make sure the viewport of the context is re-initialized
when the same context is bound to new read and draw surfaces
(or same surfaces but with different size.) next time when the
context is made current again.

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 src/mesa/main/context.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 574c0fb..477e543 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -1642,6 +1642,10 @@ _mesa_make_current( struct gl_context *newCtx,
assert(_mesa_get_current_context() == newCtx);
 
if (!newCtx) {
+  /* We reset ViewportInitialized for new bindings of surfaces to the
+   * same context in a new _mesa_make_current call.
+   */
+  curCtx->ViewportInitialized = GL_FALSE;
   _glapi_set_dispatch(NULL);  /* none current */
}
else {
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH] egl/dri2: Do not need to check return value from mtx_lock

[Dongwon Kim]

A new patch, "[PATCH] egl/dri2: remove error checks on return values from 
mtx_lock
and cnd_wait" containing additional clean-up has been submitted. Please 
disregard 
this one.

On Thu, Jul 28, 2016 at 02:38:35PM -0700, Dongwon Kim wrote:
> This removes unnecessary error checks on return result of mtx_lock
> calls as in all other places in MESA source since there is no chance
> that mtx_lock returns any of error codes in current implementation.
> 
> Signed-off-by: Dongwon Kim <dongwon@intel.com>
> ---
>  src/egl/drivers/dri2/egl_dri2.c | 21 ++---
>  1 file changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index ac2be86..26b80d4 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -2578,10 +2578,7 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay 
> *dpy, _EGLSync *sync,
>  
>/* if timeout is EGL_FOREVER_KHR, it should wait without any timeout.*/
>if (timeout == EGL_FOREVER_KHR) {
> - if (mtx_lock(_sync->mutex)) {
> -ret = EGL_FALSE;
> -goto cleanup;
> - }
> + mtx_lock(_sync->mutex);
>  
>   ret = cnd_wait(_sync->cond, _sync->mutex);
>  
> @@ -2609,10 +2606,7 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay 
> *dpy, _EGLSync *sync,
> expire.nsec -= 10L;
>  }
>  
> -if (mtx_lock(_sync->mutex)) {
> -   ret = EGL_FALSE;
> -   goto cleanup;
> -}
> +mtx_lock(_sync->mutex);
>  
>  ret = cnd_timedwait(_sync->cond, _sync->mutex, 
> );
>  
> @@ -2632,15 +2626,12 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay 
> *dpy, _EGLSync *sync,
>break;
>}
>  
> - cleanup:
> -   dri2_egl_unref_sync(dri2_dpy, dri2_sync);
> +  dri2_egl_unref_sync(dri2_dpy, dri2_sync);
>  
> -   if (ret == EGL_FALSE) {
> -  _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
> -  return EGL_FALSE;
> -   }
> +  if (ret == EGL_FALSE)
> + _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
>  
> -   return ret;
> +  return ret;
>  }
>  
>  static EGLBoolean
> -- 
> 2.7.4
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] egl/dri2: Do not need to check return value from mtx_lock

[Dongwon Kim]

Emil,

I just submitted a new patch "[PATCH] egl/dri2: remove error checks on 
return values from mtx_lock and cnd_wait" that replaces the old one.
Please review the new one and disregard the original one, "egl/dri2: 
Do not need to check return value from mtx_lock".  

On Mon, Aug 15, 2016 at 02:45:04PM +0100, Emil Velikov wrote:
> Hi Kim,
> 
> On 28 July 2016 at 22:38, Dongwon Kim <dongwon@intel.com> wrote:
> > This removes unnecessary error checks on return result of mtx_lock
> > calls as in all other places in MESA source since there is no chance
> > that mtx_lock returns any of error codes in current implementation.
> >
> > Signed-off-by: Dongwon Kim <dongwon@intel.com>
> > ---
> >  src/egl/drivers/dri2/egl_dri2.c | 21 ++---
> >  1 file changed, 6 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/egl_dri2.c 
> > b/src/egl/drivers/dri2/egl_dri2.c
> > index ac2be86..26b80d4 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.c
> > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > @@ -2578,10 +2578,7 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay 
> > *dpy, _EGLSync *sync,
> >
> >/* if timeout is EGL_FOREVER_KHR, it should wait without any 
> > timeout.*/
> >if (timeout == EGL_FOREVER_KHR) {
> > - if (mtx_lock(_sync->mutex)) {
> > -ret = EGL_FALSE;
> > -goto cleanup;
> > - }
> > + mtx_lock(_sync->mutex);
> >
> >   ret = cnd_wait(_sync->cond, _sync->mutex);
> >
> > @@ -2609,10 +2606,7 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay 
> > *dpy, _EGLSync *sync,
> > expire.nsec -= 10L;
> >  }
> >
> > -if (mtx_lock(_sync->mutex)) {
> > -   ret = EGL_FALSE;
> > -   goto cleanup;
> > -}
> > +mtx_lock(_sync->mutex);
> >
> >  ret = cnd_timedwait(_sync->cond, _sync->mutex, 
> > );
> >
> > @@ -2632,15 +2626,12 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay 
> > *dpy, _EGLSync *sync,
> >break;
> >}
> >
> > - cleanup:
> > -   dri2_egl_unref_sync(dri2_dpy, dri2_sync);
> > +  dri2_egl_unref_sync(dri2_dpy, dri2_sync);
> >
> > -   if (ret == EGL_FALSE) {
> > -  _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
> > -  return EGL_FALSE;
> > -   }
> > +  if (ret == EGL_FALSE)
> > + _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
> >
> > -   return ret;
> > +  return ret;
> Patch looks great and although looking more into it - the original
> implementation is calling _eglError multiple times.
> Initially as the errors are detected and secondly in the cleanup path.
> Can we drop either one please ?
> 
> Aside: all the users of cnd_wait (its gallium wrapper or the core
> pthread function) don't bother checking the return value. Worth
> keeping/dropping the check ?
> 
> Regardless of which one(s) get nuked (or the cnd_wait comment) the patch is
> Reviewed-by: Emil Velikov <emil.veli...@collabora.com>
> 
> Thanks
> Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] egl/dri2: remove error checks on return values from mtx_lock and cnd_wait

[Dongwon Kim]

This removes unnecessary error checks on return result of mtx_lock
and cnd_wait calls as in all other places in MESA source since there
is no chance that any of these functions return any of error codes
in current implementation.

This patch also removes a redundent _eglError call that follows
EGL_FALSE check in the bottom of dri2_client_wait_sync.

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 src/egl/drivers/dri2/egl_dri2.c | 47 +++--
 1 file changed, 12 insertions(+), 35 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index ac2be86..8503482 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -2578,19 +2578,9 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay *dpy, 
_EGLSync *sync,
 
   /* if timeout is EGL_FOREVER_KHR, it should wait without any timeout.*/
   if (timeout == EGL_FOREVER_KHR) {
- if (mtx_lock(_sync->mutex)) {
-ret = EGL_FALSE;
-goto cleanup;
- }
-
- ret = cnd_wait(_sync->cond, _sync->mutex);
-
+ mtx_lock(_sync->mutex);
+ cnd_wait(_sync->cond, _sync->mutex);
  mtx_unlock(_sync->mutex);
-
- if (ret) {
-_eglError(EGL_BAD_PARAMETER, "eglClientWaitSyncKHR");
-ret = EGL_FALSE;
- }
   } else {
  /* if reusable sync has not been yet signaled */
  if (dri2_sync->base.SyncStatus != EGL_SIGNALED_KHR) {
@@ -2609,38 +2599,25 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay 
*dpy, _EGLSync *sync,
expire.nsec -= 10L;
 }
 
-if (mtx_lock(_sync->mutex)) {
-   ret = EGL_FALSE;
-   goto cleanup;
-}
-
+mtx_lock(_sync->mutex);
 ret = cnd_timedwait(_sync->cond, _sync->mutex, );
-
 mtx_unlock(_sync->mutex);
 
-if (ret)
-   if (ret == thrd_busy) {
-  if (dri2_sync->base.SyncStatus == EGL_UNSIGNALED_KHR) {
- ret = EGL_TIMEOUT_EXPIRED_KHR;
-  } else {
- _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
- ret = EGL_FALSE;
-  }
+if (ret == thrd_busy) {
+   if (dri2_sync->base.SyncStatus == EGL_UNSIGNALED_KHR) {
+  ret = EGL_TIMEOUT_EXPIRED_KHR;
+   } else {
+  _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
+  ret = EGL_FALSE;
}
+}
  }
   }
   break;
   }
+  dri2_egl_unref_sync(dri2_dpy, dri2_sync);
 
- cleanup:
-   dri2_egl_unref_sync(dri2_dpy, dri2_sync);
-
-   if (ret == EGL_FALSE) {
-  _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
-  return EGL_FALSE;
-   }
-
-   return ret;
+  return ret;
 }
 
 static EGLBoolean
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH] egl/dri2: Do not need to check return value from mtx_lock

[Dongwon Kim]

Yes, you are right. It is duplicate EGL_FALSE check and _eglError
call in the end of the function.. Also, I don't see any good reason
to check return value of cnd_wait (though we still need to check it in
timedwait case.). I will prepare a new patch with all of these 
taken into acocunt.

On Mon, Aug 15, 2016 at 02:45:04PM +0100, Emil Velikov wrote:
> Hi Kim,
> 
> On 28 July 2016 at 22:38, Dongwon Kim <dongwon@intel.com> wrote:
> > This removes unnecessary error checks on return result of mtx_lock
> > calls as in all other places in MESA source since there is no chance
> > that mtx_lock returns any of error codes in current implementation.
> >
> > Signed-off-by: Dongwon Kim <dongwon@intel.com>
> > ---
> >  src/egl/drivers/dri2/egl_dri2.c | 21 ++---
> >  1 file changed, 6 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/egl_dri2.c 
> > b/src/egl/drivers/dri2/egl_dri2.c
> > index ac2be86..26b80d4 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.c
> > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > @@ -2578,10 +2578,7 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay 
> > *dpy, _EGLSync *sync,
> >
> >/* if timeout is EGL_FOREVER_KHR, it should wait without any 
> > timeout.*/
> >if (timeout == EGL_FOREVER_KHR) {
> > - if (mtx_lock(_sync->mutex)) {
> > -ret = EGL_FALSE;
> > -goto cleanup;
> > - }
> > + mtx_lock(_sync->mutex);
> >
> >   ret = cnd_wait(_sync->cond, _sync->mutex);
> >
> > @@ -2609,10 +2606,7 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay 
> > *dpy, _EGLSync *sync,
> > expire.nsec -= 10L;
> >  }
> >
> > -if (mtx_lock(_sync->mutex)) {
> > -   ret = EGL_FALSE;
> > -   goto cleanup;
> > -}
> > +mtx_lock(_sync->mutex);
> >
> >  ret = cnd_timedwait(_sync->cond, _sync->mutex, 
> > );
> >
> > @@ -2632,15 +2626,12 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay 
> > *dpy, _EGLSync *sync,
> >break;
> >}
> >
> > - cleanup:
> > -   dri2_egl_unref_sync(dri2_dpy, dri2_sync);
> > +  dri2_egl_unref_sync(dri2_dpy, dri2_sync);
> >
> > -   if (ret == EGL_FALSE) {
> > -  _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
> > -  return EGL_FALSE;
> > -   }
> > +  if (ret == EGL_FALSE)
> > + _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
> >
> > -   return ret;
> > +  return ret;
> Patch looks great and although looking more into it - the original
> implementation is calling _eglError multiple times.
> Initially as the errors are detected and secondly in the cleanup path.
> Can we drop either one please ?
> 
> Aside: all the users of cnd_wait (its gallium wrapper or the core
> pthread function) don't bother checking the return value. Worth
> keeping/dropping the check ?
> 
> Regardless of which one(s) get nuked (or the cnd_wait comment) the patch is
> Reviewed-by: Emil Velikov <emil.veli...@collabora.com>
> 
> Thanks
> Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] egl/dri2: Do not need to check return value from mtx_lock

[Dongwon Kim]

This removes unnecessary error checks on return result of mtx_lock
calls as in all other places in MESA source since there is no chance
that mtx_lock returns any of error codes in current implementation.

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 src/egl/drivers/dri2/egl_dri2.c | 21 ++---
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index ac2be86..26b80d4 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -2578,10 +2578,7 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay *dpy, 
_EGLSync *sync,
 
   /* if timeout is EGL_FOREVER_KHR, it should wait without any timeout.*/
   if (timeout == EGL_FOREVER_KHR) {
- if (mtx_lock(_sync->mutex)) {
-ret = EGL_FALSE;
-goto cleanup;
- }
+ mtx_lock(_sync->mutex);
 
  ret = cnd_wait(_sync->cond, _sync->mutex);
 
@@ -2609,10 +2606,7 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay *dpy, 
_EGLSync *sync,
expire.nsec -= 10L;
 }
 
-if (mtx_lock(_sync->mutex)) {
-   ret = EGL_FALSE;
-   goto cleanup;
-}
+mtx_lock(_sync->mutex);
 
 ret = cnd_timedwait(_sync->cond, _sync->mutex, );
 
@@ -2632,15 +2626,12 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay 
*dpy, _EGLSync *sync,
   break;
   }
 
- cleanup:
-   dri2_egl_unref_sync(dri2_dpy, dri2_sync);
+  dri2_egl_unref_sync(dri2_dpy, dri2_sync);
 
-   if (ret == EGL_FALSE) {
-  _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
-  return EGL_FALSE;
-   }
+  if (ret == EGL_FALSE)
+ _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
 
-   return ret;
+  return ret;
 }
 
 static EGLBoolean
-- 
2.7.4

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

Re: [Mesa-dev] Fwd: New Defects reported by Coverity Scan for Mesa

[Dongwon Kim]

There are four different places where the program pointer may jump to 
'cleanup:' while the mutex is still possilby being locked. Two
of those cases are when mtx_lock(pthread_mutex_lock) fails to lock 
the mutex. However, this can't be a problem because the mutex is either 
not in "locked" state because current thread fails to acquire it or 
even if it's locked, it's owned by another thread.

The other two places are when mtx_unlock call fails to unlock the mutex. 
This is also not a problem. According to manpage, pthread_mutex_unlock
only can return EINVAL or EPERM. EINVAL means the mutex is not 
initialized. But if it's uninialized, the code should have already 
failed while trying to acquiring it. EPERM is also not an issue 
because this error code means current thread doesn't own the mutex. 
In other words, the function is not responsible for unlocking it.

On Wed, Apr 13, 2016 at 12:09:44PM -0700, Matt Turner wrote:
> Looks like some locking problems caused by commit 70299474. Please take a 
> look.
> 
> -- Forwarded message --
> From:  
> Date: Wed, Apr 13, 2016 at 8:37 AM
> Subject: New Defects reported by Coverity Scan for Mesa
> 
> 
> *** CID 1358496:(LOCK)
> /src/egl/drivers/dri2/egl_dri2.c: 2643 in dri2_client_wait_sync()
> 2637
> 2638  cleanup:
> 2639dri2_egl_unref_sync(dri2_dpy, dri2_sync);
> 2640
> 2641if (ret == EGL_FALSE) {
> 2642   _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
> >>> CID 1358496:(LOCK)
> >>> Returning without unlocking "dri2_sync->mutex".
> 2643   return EGL_FALSE;
> 2644}
> 2645
> 2646return ret;
> 2647 }
> 2648
> /src/egl/drivers/dri2/egl_dri2.c: 2643 in dri2_client_wait_sync()
> 2637
> 2638  cleanup:
> 2639dri2_egl_unref_sync(dri2_dpy, dri2_sync);
> 2640
> 2641if (ret == EGL_FALSE) {
> 2642   _eglError(EGL_BAD_ACCESS, "eglClientWaitSyncKHR");
> >>> CID 1358496:(LOCK)
> >>> Returning without unlocking "dri2_sync->mutex".
> 2643   return EGL_FALSE;
> 2644}
> 2645
> 2646return ret;
> 2647 }
> 2648
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3] egl: add EGL_KHR_reusable_sync to egl_dri

[Dongwon Kim]

Hi Emil,

I don't think I still have a chance to update
this to version 4 since this has already been pushed
according to Marek... However, I will follow up your 
suggestion with a separate patch once this is 
completely merged..

-DW

On Tue, Apr 05, 2016 at 03:09:41PM +0100, Emil Velikov wrote:
> Hi Dongwon,
> 
> On 5 April 2016 at 01:14, Dongwon Kim <dongwon@intel.com> wrote:
> > This patch enables an EGL extension, EGL_KHR_reusable_sync.
> > This new extension basically provides a way for multiple APIs or
> > threads to be excuted synchronously via a "reusable sync"
> > primitive shared by those threads/API calls.
> >
> > This was implemented based on the specification at
> >
> > https://www.khronos.org/registry/egl/extensions/KHR/EGL_KHR_reusable_sync.txt
> >
> > v2
> > - use thread functions defined in C11/threads.h instead of
> >   using direct pthread calls
> > - make the timeout set with reference to CLOCK_MONOTONIC
> > - cleaned up the way expiration time is calculated
> > - (bug fix) in dri2_client_wait_sync, case EGL_SYNC_CL_EVENT_KHR
> >   has been added.
> > - (bug fix) in dri2_destroy_sync, return from cond_broadcast
> >   call is now stored in 'err' intead of 'ret' to prevent 'ret'
> >   from being reset to 'EGL_FALSE' even in successful case
> > - corrected minor syntax problems
> >
> > v3
> > - dri2_egl_unref_sync now became 'void' type. No more error check
> >   is needed for this function call as a result.
> > - (bug fix) resolved issue with duplicated unlocking of display in
> >   eglClientWaitSync when type of sync is "EGL_KHR_REUSABLE_SYNC"
> >
> > Signed-off-by: Dongwon Kim <dongwon@intel.com>
> > ---
> >  src/egl/drivers/dri2/egl_dri2.c | 192 
> > ++--
> >  src/egl/drivers/dri2/egl_dri2.h |   2 +
> >  src/egl/main/eglapi.c   |  17 +++-
> >  src/egl/main/eglsync.c  |   3 +-
> >  4 files changed, 206 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/egl_dri2.c 
> > b/src/egl/drivers/dri2/egl_dri2.c
> > index 8f50f0c..490b040 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.c
> > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > @@ -38,6 +38,8 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> >  #ifdef HAVE_LIBDRM
> >  #include 
> >  #include 
> > @@ -623,6 +625,8 @@ dri2_setup_screen(_EGLDisplay *disp)
> >   disp->Extensions.KHR_cl_event2 = EGL_TRUE;
> > }
> >
> > +   disp->Extensions.KHR_reusable_sync = EGL_TRUE;
> > +
> > if (dri2_dpy->image) {
> >if (dri2_dpy->image->base.version >= 10 &&
> >dri2_dpy->image->getCapabilities != NULL) {
> > @@ -2394,7 +2398,12 @@ dri2_egl_unref_sync(struct dri2_egl_display 
> > *dri2_dpy,
> >  struct dri2_egl_sync *dri2_sync)
> >  {
> > if (p_atomic_dec_zero(_sync->refcount)) {
> > -  dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, 
> > dri2_sync->fence);
> > +  if (dri2_sync->base.Type == EGL_SYNC_REUSABLE_KHR)
> > + cnd_destroy(_sync->cond);
> > +
> > +  if (dri2_sync->fence)
> > + dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, 
> > dri2_sync->fence);
> > +
> >free(dri2_sync);
> > }
> >  }
> > @@ -2408,6 +2417,8 @@ dri2_create_sync(_EGLDriver *drv, _EGLDisplay *dpy,
> > struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
> > struct dri2_egl_context *dri2_ctx = dri2_egl_context(ctx);
> > struct dri2_egl_sync *dri2_sync;
> > +   EGLint ret;
> > +   pthread_condattr_t attr;
> >
> > dri2_sync = calloc(1, sizeof(struct dri2_egl_sync));
> > if (!dri2_sync) {
> > @@ -2450,6 +2461,37 @@ dri2_create_sync(_EGLDriver *drv, _EGLDisplay *dpy,
> >  dri2_sync->fence, 0, 0))
> >   dri2_sync->base.SyncStatus = EGL_SIGNALED_KHR;
> >break;
> > +
> > +   case EGL_SYNC_REUSABLE_KHR:
> > +  /* intialize attr */
> typo -> initialize
> 
> > +  ret = pthread_condattr_init();
> > +
> > +  if (ret) {
> > + _eglError(EGL_BAD_ACCESS, "eglCreateSyncKHR");
> > + free(dri2_sync);
> > + return NULL;
> > +  }
> > +
> > +  /* change clock attribute to CLOCK_MONOTONIC */
> > +  ret = pthread_c

[Mesa-dev] [PATCH v3] egl: add EGL_KHR_reusable_sync to egl_dri

[Dongwon Kim]

This patch enables an EGL extension, EGL_KHR_reusable_sync.
This new extension basically provides a way for multiple APIs or
threads to be excuted synchronously via a "reusable sync"
primitive shared by those threads/API calls.

This was implemented based on the specification at

https://www.khronos.org/registry/egl/extensions/KHR/EGL_KHR_reusable_sync.txt

v2
- use thread functions defined in C11/threads.h instead of
  using direct pthread calls
- make the timeout set with reference to CLOCK_MONOTONIC
- cleaned up the way expiration time is calculated
- (bug fix) in dri2_client_wait_sync, case EGL_SYNC_CL_EVENT_KHR
  has been added.
- (bug fix) in dri2_destroy_sync, return from cond_broadcast
  call is now stored in 'err' intead of 'ret' to prevent 'ret'
  from being reset to 'EGL_FALSE' even in successful case
- corrected minor syntax problems

v3
- dri2_egl_unref_sync now became 'void' type. No more error check
  is needed for this function call as a result.
- (bug fix) resolved issue with duplicated unlocking of display in
  eglClientWaitSync when type of sync is "EGL_KHR_REUSABLE_SYNC"

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 src/egl/drivers/dri2/egl_dri2.c | 192 ++--
 src/egl/drivers/dri2/egl_dri2.h |   2 +
 src/egl/main/eglapi.c   |  17 +++-
 src/egl/main/eglsync.c  |   3 +-
 4 files changed, 206 insertions(+), 8 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 8f50f0c..490b040 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -38,6 +38,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #ifdef HAVE_LIBDRM
 #include 
 #include 
@@ -623,6 +625,8 @@ dri2_setup_screen(_EGLDisplay *disp)
  disp->Extensions.KHR_cl_event2 = EGL_TRUE;
}
 
+   disp->Extensions.KHR_reusable_sync = EGL_TRUE;
+
if (dri2_dpy->image) {
   if (dri2_dpy->image->base.version >= 10 &&
   dri2_dpy->image->getCapabilities != NULL) {
@@ -2394,7 +2398,12 @@ dri2_egl_unref_sync(struct dri2_egl_display *dri2_dpy,
 struct dri2_egl_sync *dri2_sync)
 {
if (p_atomic_dec_zero(_sync->refcount)) {
-  dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, dri2_sync->fence);
+  if (dri2_sync->base.Type == EGL_SYNC_REUSABLE_KHR)
+ cnd_destroy(_sync->cond);
+
+  if (dri2_sync->fence)
+ dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, 
dri2_sync->fence);
+
   free(dri2_sync);
}
 }
@@ -2408,6 +2417,8 @@ dri2_create_sync(_EGLDriver *drv, _EGLDisplay *dpy,
struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
struct dri2_egl_context *dri2_ctx = dri2_egl_context(ctx);
struct dri2_egl_sync *dri2_sync;
+   EGLint ret;
+   pthread_condattr_t attr;
 
dri2_sync = calloc(1, sizeof(struct dri2_egl_sync));
if (!dri2_sync) {
@@ -2450,6 +2461,37 @@ dri2_create_sync(_EGLDriver *drv, _EGLDisplay *dpy,
 dri2_sync->fence, 0, 0))
  dri2_sync->base.SyncStatus = EGL_SIGNALED_KHR;
   break;
+
+   case EGL_SYNC_REUSABLE_KHR:
+  /* intialize attr */
+  ret = pthread_condattr_init();
+
+  if (ret) {
+ _eglError(EGL_BAD_ACCESS, "eglCreateSyncKHR");
+ free(dri2_sync);
+ return NULL;
+  }
+
+  /* change clock attribute to CLOCK_MONOTONIC */
+  ret = pthread_condattr_setclock(, CLOCK_MONOTONIC);
+
+  if (ret) {
+ _eglError(EGL_BAD_ACCESS, "eglCreateSyncKHR");
+ free(dri2_sync);
+ return NULL;
+  }
+
+  ret = pthread_cond_init(_sync->cond, );
+
+  if (ret) {
+ _eglError(EGL_BAD_ACCESS, "eglCreateSyncKHR");
+ free(dri2_sync);
+ return NULL;
+  }
+
+  /* initial status of reusable sync must be "unsignaled" */
+  dri2_sync->base.SyncStatus = EGL_UNSIGNALED_KHR;
+  break;
}
 
p_atomic_set(_sync->refcount, 1);
@@ -2461,9 +2503,27 @@ dri2_destroy_sync(_EGLDriver *drv, _EGLDisplay *dpy, 
_EGLSync *sync)
 {
struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
struct dri2_egl_sync *dri2_sync = dri2_egl_sync(sync);
+   EGLint ret = EGL_TRUE;
+   EGLint err;
+
+   /* if type of sync is EGL_SYNC_REUSABLE_KHR and it is not signaled yet,
+* then unlock all threads possibly blocked by the reusable sync before
+* destroying it.
+*/
+   if (dri2_sync->base.Type == EGL_SYNC_REUSABLE_KHR &&
+   dri2_sync->base.SyncStatus == EGL_UNSIGNALED_KHR) {
+  dri2_sync->base.SyncStatus = EGL_SIGNALED_KHR;
+  /* unblock all threads currently blocked by sync */
+  err = cnd_broadcast(_sync->cond);
 
+  if (err) {
+ _eglError(EGL_BAD_ACCESS, "eglDestroySyncKHR");
+ ret = EGL_FALSE;
+  }
+   }
dri2_egl_unr

[Mesa-dev] [PATCH v2] egl: add EGL_KHR_reusable_sync to egl_dri

[Dongwon Kim]

This patch enables an EGL extension, EGL_KHR_reusable_sync.
This new extension basically provides a way for multiple APIs or
threads to be excuted synchronously via a "reusable sync"
primitive shared by those threads/API calls.

This was implemented based on the specification at

https://www.khronos.org/registry/egl/extensions/KHR/EGL_KHR_reusable_sync.txt

v2
- use thread functions defined in C11/threads.h instead of
  using direct pthread calls
- make the timeout set with reference to CLOCK_MONOTONIC
- cleaned up the way expiration time is calculated
- (bug fix) in dri2_client_wait_sync, case EGL_SYNC_CL_EVENT_KHR
  has been added.
- (bug fix) in dri2_destroy_sync, return from cond_broadcast
  call is now stored in 'err' intead of 'ret' to prevent 'ret'
  from being reset to 'EGL_FALSE' even in successful case
- corrected minor syntax problems

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 src/egl/drivers/dri2/egl_dri2.c | 210 ++--
 src/egl/drivers/dri2/egl_dri2.h |   2 +
 src/egl/main/eglapi.c   |   8 ++
 src/egl/main/eglsync.c  |   3 +-
 4 files changed, 213 insertions(+), 10 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 8f50f0c..843cd53 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -38,6 +38,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #ifdef HAVE_LIBDRM
 #include 
 #include 
@@ -623,6 +625,8 @@ dri2_setup_screen(_EGLDisplay *disp)
  disp->Extensions.KHR_cl_event2 = EGL_TRUE;
}
 
+   disp->Extensions.KHR_reusable_sync = EGL_TRUE;
+
if (dri2_dpy->image) {
   if (dri2_dpy->image->base.version >= 10 &&
   dri2_dpy->image->getCapabilities != NULL) {
@@ -2389,14 +2393,22 @@ dri2_egl_ref_sync(struct dri2_egl_sync *sync)
p_atomic_inc(>refcount);
 }
 
-static void
+static EGLint
 dri2_egl_unref_sync(struct dri2_egl_display *dri2_dpy,
 struct dri2_egl_sync *dri2_sync)
 {
if (p_atomic_dec_zero(_sync->refcount)) {
-  dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, dri2_sync->fence);
+  if (dri2_sync->base.Type == EGL_SYNC_REUSABLE_KHR) {
+ cnd_destroy(_sync->cond);
+  }
+
+  if (dri2_sync->fence)
+ dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, 
dri2_sync->fence);
+
   free(dri2_sync);
}
+
+   return EGL_TRUE;
 }
 
 static _EGLSync *
@@ -2408,6 +2420,8 @@ dri2_create_sync(_EGLDriver *drv, _EGLDisplay *dpy,
struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
struct dri2_egl_context *dri2_ctx = dri2_egl_context(ctx);
struct dri2_egl_sync *dri2_sync;
+   EGLint ret;
+   pthread_condattr_t attr;
 
dri2_sync = calloc(1, sizeof(struct dri2_egl_sync));
if (!dri2_sync) {
@@ -2450,6 +2464,37 @@ dri2_create_sync(_EGLDriver *drv, _EGLDisplay *dpy,
 dri2_sync->fence, 0, 0))
  dri2_sync->base.SyncStatus = EGL_SIGNALED_KHR;
   break;
+
+   case EGL_SYNC_REUSABLE_KHR:
+  /* intialize attr */
+  ret = pthread_condattr_init();
+
+  if (ret) {
+ _eglError(EGL_BAD_ACCESS, "eglCreateSyncKHR");
+ free(dri2_sync);
+ return NULL;
+  }
+
+  /* change clock attribute to CLOCK_MONOTONIC */
+  ret = pthread_condattr_setclock(, CLOCK_MONOTONIC);
+
+  if (ret) {
+ _eglError(EGL_BAD_ACCESS, "eglCreateSyncKHR");
+ free(dri2_sync);
+ return NULL;
+  }
+
+  ret = pthread_cond_init(_sync->cond, );
+
+  if (ret) {
+ _eglError(EGL_BAD_ACCESS, "eglCreateSyncKHR");
+ free(dri2_sync);
+ return NULL;
+  }
+
+  /* initial status of reusable sync must be "unsignaled" */
+  dri2_sync->base.SyncStatus = EGL_UNSIGNALED_KHR;
+  break;
}
 
p_atomic_set(_sync->refcount, 1);
@@ -2461,9 +2506,33 @@ dri2_destroy_sync(_EGLDriver *drv, _EGLDisplay *dpy, 
_EGLSync *sync)
 {
struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
struct dri2_egl_sync *dri2_sync = dri2_egl_sync(sync);
+   EGLint ret = EGL_TRUE;
+   EGLint err;
 
-   dri2_egl_unref_sync(dri2_dpy, dri2_sync);
-   return EGL_TRUE;
+   /* if type of sync is EGL_SYNC_REUSABLE_KHR and it is not signaled yet,
+* then unlock all threads possibly blocked by the reusable sync before
+* destroying it.
+*/
+   if (dri2_sync->base.Type == EGL_SYNC_REUSABLE_KHR &&
+   dri2_sync->base.SyncStatus == EGL_UNSIGNALED_KHR) {
+  dri2_sync->base.SyncStatus = EGL_SIGNALED_KHR;
+  /* unblock all threads currently blocked by sync */
+  err = cnd_broadcast(_sync->cond);
+
+  if (err) {
+ _eglError(EGL_BAD_ACCESS, "eglDestroySyncKHR");
+ ret = EGL_FALSE;
+  }
+   }
+
+   err = dri2_egl_unref_sync(dr

[Mesa-dev] [PATCH] egl: adds EGL_KHR_reusable_sync to egl_dri

[Dongwon Kim]

This patch enables an EGL extension, EGL_KHR_reusable_sync.
This new extension basically provides a way for multiple APIs or
threads to be excuted synchronously via a "reusable sync"
primitive shared by those threads/API calls.

This was implemented based on the specification at

https://www.khronos.org/registry/egl/extensions/KHR/EGL_KHR_reusable_sync.txt

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 src/egl/drivers/dri2/egl_dri2.c | 197 ++--
 src/egl/drivers/dri2/egl_dri2.h |   2 +
 src/egl/main/eglapi.c   |   8 ++
 src/egl/main/eglsync.c  |   3 +-
 4 files changed, 200 insertions(+), 10 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 8f50f0c..78164e4 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -38,6 +38,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #ifdef HAVE_LIBDRM
 #include 
 #include 
@@ -623,6 +625,8 @@ dri2_setup_screen(_EGLDisplay *disp)
  disp->Extensions.KHR_cl_event2 = EGL_TRUE;
}
 
+   disp->Extensions.KHR_reusable_sync = EGL_TRUE;
+
if (dri2_dpy->image) {
   if (dri2_dpy->image->base.version >= 10 &&
   dri2_dpy->image->getCapabilities != NULL) {
@@ -2389,14 +2393,33 @@ dri2_egl_ref_sync(struct dri2_egl_sync *sync)
p_atomic_inc(>refcount);
 }
 
-static void
+static EGLint
 dri2_egl_unref_sync(struct dri2_egl_display *dri2_dpy,
 struct dri2_egl_sync *dri2_sync)
 {
+   EGLint ret;
+
if (p_atomic_dec_zero(_sync->refcount)) {
-  dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, dri2_sync->fence);
+  /* mutex and cond should be freed if not freed yet. */
+  if (dri2_sync->mutex)
+ free(dri2_sync->mutex);
+
+  if (dri2_sync->cond) {
+ ret = pthread_cond_destroy(dri2_sync->cond);
+
+ if (ret)
+return EGL_FALSE;
+
+ free(dri2_sync->cond);
+  }
+
+  if (dri2_sync->fence)
+ dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, 
dri2_sync->fence);
+
   free(dri2_sync);
}
+
+   return EGL_TRUE;
 }
 
 static _EGLSync *
@@ -2408,6 +2431,7 @@ dri2_create_sync(_EGLDriver *drv, _EGLDisplay *dpy,
struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
struct dri2_egl_context *dri2_ctx = dri2_egl_context(ctx);
struct dri2_egl_sync *dri2_sync;
+   EGLint ret;
 
dri2_sync = calloc(1, sizeof(struct dri2_egl_sync));
if (!dri2_sync) {
@@ -2450,6 +2474,23 @@ dri2_create_sync(_EGLDriver *drv, _EGLDisplay *dpy,
 dri2_sync->fence, 0, 0))
  dri2_sync->base.SyncStatus = EGL_SIGNALED_KHR;
   break;
+
+   case EGL_SYNC_REUSABLE_KHR:
+  dri2_sync->cond = calloc(1, sizeof(pthread_cond_t));
+  dri2_sync->mutex = calloc(1, sizeof(pthread_mutex_t));
+  ret = pthread_cond_init(dri2_sync->cond, NULL);
+
+  if (ret) {
+ _eglError(EGL_BAD_PARAMETER, "eglCreateSyncKHR");
+ free(dri2_sync->cond);
+ free(dri2_sync->mutex);
+ free(dri2_sync);
+ return NULL;
+  }
+
+  /* initial status of reusable sync must be "unsignaled" */
+  dri2_sync->base.SyncStatus = EGL_UNSIGNALED_KHR;
+  break;
}
 
p_atomic_set(_sync->refcount, 1);
@@ -2461,9 +2502,33 @@ dri2_destroy_sync(_EGLDriver *drv, _EGLDisplay *dpy, 
_EGLSync *sync)
 {
struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
struct dri2_egl_sync *dri2_sync = dri2_egl_sync(sync);
+   EGLint ret = EGL_TRUE;
+   EGLint err;
 
-   dri2_egl_unref_sync(dri2_dpy, dri2_sync);
-   return EGL_TRUE;
+   /* if type of sync is EGL_SYNC_REUSABLE_KHR and it is not signaled yet,
+* then unlock all threads possibly blocked by the reusable sync before
+* destroying it.
+*/
+   if (dri2_sync->base.Type == EGL_SYNC_REUSABLE_KHR &&
+   dri2_sync->base.SyncStatus == EGL_UNSIGNALED_KHR) {
+  dri2_sync->base.SyncStatus = EGL_SIGNALED_KHR;
+  /* unblock all threads currently blocked by sync */
+  ret = pthread_cond_broadcast(dri2_sync->cond);
+
+  if (ret) {
+ _eglError(EGL_BAD_PARAMETER, "eglDestroySyncKHR");
+ ret = EGL_FALSE;
+  }
+   }
+
+   err = dri2_egl_unref_sync(dri2_dpy, dri2_sync);
+
+   if (err == EGL_FALSE) {
+  _eglError(EGL_BAD_PARAMETER, "eglDestroySyncKHR");
+  ret = EGL_FALSE;
+   }
+
+   return ret;
 }
 
 static EGLint
@@ -2471,11 +2536,18 @@ dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay 
*dpy, _EGLSync *sync,
   EGLint flags, EGLTime timeout)
 {
_EGLContext *ctx = _eglGetCurrentContext();
+   struct dri2_egl_driver *dri2_drv = dri2_egl_driver(drv);
struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
struct dri2_egl_context *dri2_ct

[Mesa-dev] [PATCH] configure.ac: enable_asm=yes when x-compiling across same X86 arch

[Dongwon Kim]

Currently, configure script is forcing 'enable_asm' to be 'no'
whenever cross-compilation is performed on X86 host. This is
based on an assumption that target architecture is different
from host's (i.e. ARM). But there's always a case that we do
cross-compilation for target that is also X86 based just like
host in which same ASM codes will be supported. 'enable_asm'
should not be forced to be "no" anymore in this case.

v2: corrected commit message

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 configure.ac | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index b05f33d..0ad27c9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -710,8 +710,10 @@ test "x$enable_asm" = xno && AC_MSG_RESULT([no])
 if test "x$enable_asm" = xyes -a "x$cross_compiling" = xyes; then
 case "$host_cpu" in
 i?86 | x86_64 | amd64)
-enable_asm=no
-AC_MSG_RESULT([no, cross compiling])
+if test "x$host_cpu" != "x$target_cpu"; then
+enable_asm=no
+AC_MSG_RESULT([no, cross compiling])
+fi
 ;;
 esac
 fi
-- 
1.9.1

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

[Mesa-dev] [PATCH] configure.ac: enable_asm=yes when x-compiling across same X86 arch

[Dongwon Kim]

Currently, configure script is forcing 'enable_asm' to be 'no'
whenever cross-compilation is performed on X86 host. This is
based on an assumption that target architecture is different
from host's (i.e. ARM). But there's always a case that we do
cross-compile for target that is also X86 based just like host
in which same ASM codes will be supported. In other words,
'enable_asm' should not be forced to be "no" anymore in this
case.

This change removes this limitation by reseting enable_asm
if the host is one of X86 based architectures and target
has exactly same architecture with it in cross-compilation
environment.

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 configure.ac | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index b05f33d..0ad27c9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -710,8 +710,10 @@ test "x$enable_asm" = xno && AC_MSG_RESULT([no])
 if test "x$enable_asm" = xyes -a "x$cross_compiling" = xyes; then
 case "$host_cpu" in
 i?86 | x86_64 | amd64)
-enable_asm=no
-AC_MSG_RESULT([no, cross compiling])
+if test "x$host_cpu" != "x$target_cpu"; then
+enable_asm=no
+AC_MSG_RESULT([no, cross compiling])
+fi
 ;;
 esac
 fi
-- 
1.9.1

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

[Mesa-dev] [PATCH] egl: move Null check to eglGetSyncAttribKHR to prevent Segfault

[Dongwon Kim]

Null-check on "*value" is currently done in
_eglGetSyncAttrib, which is after eglGetSyncAttribKHR
attempts to copy data at 'value' to 'attrib'. Segfault
is enevitable if value==NULL in this case. Therefore,
null-check should be moved to beginning of
eglGetSyncAttribKHR to avoid any possible segfaults.

Signed-off-by: Dongwon Kim <dongwon@intel.com>
---
 src/egl/main/eglapi.c  | 10 --
 src/egl/main/eglsync.c |  3 ---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 323634e..32f6823 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -1555,8 +1555,14 @@ eglGetSyncAttrib(EGLDisplay dpy, EGLSync sync, EGLint 
attribute, EGLAttrib *valu
 static EGLBoolean EGLAPIENTRY
 eglGetSyncAttribKHR(EGLDisplay dpy, EGLSync sync, EGLint attribute, EGLint 
*value)
 {
-   EGLAttrib attrib = *value;
-   EGLBoolean result = eglGetSyncAttrib(dpy, sync, attribute, );
+   EGLAttrib attrib;
+   EGLBoolean result;
+
+   if (!value)
+  RETURN_EGL_ERROR(NULL, EGL_BAD_PARAMETER, EGL_FALSE);
+
+   attrib = *value;
+   result = eglGetSyncAttrib(dpy, sync, attribute, );
 
/* The EGL_KHR_fence_sync spec says this about eglGetSyncAttribKHR:
 *
diff --git a/src/egl/main/eglsync.c b/src/egl/main/eglsync.c
index 3019e6e..999cb48 100644
--- a/src/egl/main/eglsync.c
+++ b/src/egl/main/eglsync.c
@@ -144,9 +144,6 @@ EGLBoolean
 _eglGetSyncAttrib(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync,
   EGLint attribute, EGLAttrib *value)
 {
-   if (!value)
-  return _eglError(EGL_BAD_PARAMETER, "eglGetSyncAttribKHR");
-
switch (attribute) {
case EGL_SYNC_TYPE_KHR:
   *value = sync->Type;
-- 
1.9.1

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

[Mesa-dev] [PATCH] egl, wayland: RGB565 format support on Back-buffer

[Dongwon Kim]

From: Vivek Kasireddy vivek.kasire...@intel.com

In current code, color format is always hardcoded to
__DRI_IMAGE_FORMAT_ARGB when buffer or DRI image is
allocated in function calls, get_back_bo and dri2_get_buffers,
regardless of current target's color format. This problem
may leads to incorrect render pitch calculation, which
eventually ends up with wrong offset of pixels in
the frame buffer when the image is in different color format
from dri surf's, especially with different bpp. (e.g. RGB565-16bpp)

Attached code patch simply adds RGB565 and XRGB cases to two
functions noted above to resolve the issue.

v2: added a case of XRGB, format and bpp selection is done
via switch-case (not if-else anymore)

Signed-off-by: Vivek Kasireddy vivek.kasire...@intel.com
Signed-off-by: Dongwon Kim dongwon@intel.com
---
 src/egl/drivers/dri2/platform_wayland.c |   41 ---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_wayland.c 
b/src/egl/drivers/dri2/platform_wayland.c
index 3c34e07..07f68a2 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -292,6 +292,26 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
struct dri2_egl_display *dri2_dpy =
   dri2_egl_display(dri2_surf-base.Resource.Display);
int i;
+   unsigned int dri_image_format;
+
+   /* currently supports three WL DRM formats,
+* WL_DRM_FORMAT_ARGB, WL_DRM_FORMAT_XRGB,
+* and WL_DRM_FORMAT_RGB565
+*/
+   switch (dri2_surf-format) {
+   case WL_DRM_FORMAT_ARGB:
+  dri_image_format = __DRI_IMAGE_FORMAT_ARGB;
+  break;
+   case WL_DRM_FORMAT_XRGB:
+  dri_image_format = __DRI_IMAGE_FORMAT_XRGB;
+  break;
+   case WL_DRM_FORMAT_RGB565:
+  dri_image_format = __DRI_IMAGE_FORMAT_RGB565;
+  break;
+   default:
+  /* format is not supported */
+  return -1;
+   }
 
/* We always want to throttle to some event (either a frame callback or
 * a sync request) after the commit so that we can be sure the
@@ -322,7 +342,7 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
  dri2_dpy-image-createImage(dri2_dpy-dri_screen,
   dri2_surf-base.Width,
   dri2_surf-base.Height,
-  __DRI_IMAGE_FORMAT_ARGB,
+  dri_image_format,
   __DRI_IMAGE_USE_SHARE,
   NULL);
   dri2_surf-back-age = 0;
@@ -462,11 +482,26 @@ dri2_wl_get_buffers(__DRIdrawable * driDrawable,
 unsigned int *attachments, int count,
 int *out_count, void *loaderPrivate)
 {
+   struct dri2_egl_surface *dri2_surf = loaderPrivate;
unsigned int *attachments_with_format;
__DRIbuffer *buffer;
-   const unsigned int format = 32;
+   unsigned int bpp;
+
int i;
 
+   switch (dri2_surf-format) {
+   case WL_DRM_FORMAT_ARGB:
+   case WL_DRM_FORMAT_XRGB:
+  bpp = 32;
+  break;
+   case WL_DRM_FORMAT_RGB565:
+  bpp = 16;
+  break;
+   default:
+  /* format is not supported */
+  return NULL;
+   }
+
attachments_with_format = calloc(count, 2 * sizeof(unsigned int));
if (!attachments_with_format) {
   *out_count = 0;
@@ -475,7 +510,7 @@ dri2_wl_get_buffers(__DRIdrawable * driDrawable,
 
for (i = 0; i  count; ++i) {
   attachments_with_format[2*i] = attachments[i];
-  attachments_with_format[2*i + 1] = format;
+  attachments_with_format[2*i + 1] = bpp;
}
 
buffer =
-- 
1.7.9.5

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

[Mesa-dev] [PATCH] torus.c: Lighting effect is distorted when object is scaled up/down

[Dongwon Kim]

When torus object is scaled up/down via glScalef call the lighting
effect is incorrectly expressed on render target because normals'
length is also changed when vertices are scaled up/down.

This patch enables automatic normalization of normals, which
is one of well-known ways to avoid this kind of distortion.

Reference:
www.opengl.org/sdk/docs/man2/xhtml/glScale.xml

Notes

 If scale factors other than 1 are applied to the modelview matrix
 and lighting is enabled, lighting often appears wrong.
 In that case, enable automatic normalization of normals by
 calling glEnable with the argument GL_NORMALIZE.

Signed-off-by: Dongwon Kim dongwon@intel.com
---
 src/egl/opengles1/torus.c |4 
 1 file changed, 4 insertions(+)

diff --git a/src/egl/opengles1/torus.c b/src/egl/opengles1/torus.c
index 8f262b5..bb20670 100644
--- a/src/egl/opengles1/torus.c
+++ b/src/egl/opengles1/torus.c
@@ -358,6 +358,10 @@ init(void)
 
make_texture();
glEnable(GL_TEXTURE_2D);
+
+   /* Enabling automatic normalizing to prevent wrong expression of lighting
+  when torus is scaled down via glScalef */
+   glEnable(GL_NORMALIZE);
 }
 
 
-- 
1.7.9.5

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

[Mesa-dev] [PATCH] egl, wayland: RGB565 format support on Back-buffer

[Dongwon Kim]

In current code, color format is always hardcoded to __DRI_IMAGE_FORMAT_ARGB
when buffer or DRI image is allocated in function calls, get_back_bo and 
dri2_get_buffers,
regardless of current target's color format. This problem may leads to 
incorrect render
pitch calculation, which eventually ends up with wrong offset of pixels in the 
frame buffer
when the image is in different color format from dri surf's, especially with 
different bpp.
(e.g. 16bpp)

Attached code patch simply adds RGB565 case to two functions noted above to 
resolve
the issue.

Signed-off-by: Vivek Kasireddy vivek.kasire...@intel.com
Signed-off-by: Dongwon Kim dongwon@intel.com
---
 src/egl/drivers/dri2/platform_wayland.c |   10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_wayland.c 
b/src/egl/drivers/dri2/platform_wayland.c
index 3c34e07..78761e2 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -293,6 +293,11 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
   dri2_egl_display(dri2_surf-base.Resource.Display);
int i;
 
+   /* currently supports two DRI image formats, __DRI_IMAGE_FORMAT_RGB565 or 
__DRI_IMAGE_FORMAT_ARGB,
+* one of which is selected depending on dri surface format */
+   const unsigned int dri_image_format =
+(dri2_surf-format == WL_DRM_FORMAT_RGB565) ? 
__DRI_IMAGE_FORMAT_RGB565 : __DRI_IMAGE_FORMAT_ARGB;
+
/* We always want to throttle to some event (either a frame callback or
 * a sync request) after the commit so that we can be sure the
 * compositor has had a chance to handle it and send us a release event
@@ -322,7 +327,7 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
  dri2_dpy-image-createImage(dri2_dpy-dri_screen,
   dri2_surf-base.Width,
   dri2_surf-base.Height,
-  __DRI_IMAGE_FORMAT_ARGB,
+  dri_image_format,
   __DRI_IMAGE_USE_SHARE,
   NULL);
   dri2_surf-back-age = 0;
@@ -462,9 +467,10 @@ dri2_wl_get_buffers(__DRIdrawable * driDrawable,
 unsigned int *attachments, int count,
 int *out_count, void *loaderPrivate)
 {
+   struct dri2_egl_surface *dri2_surf = loaderPrivate;
unsigned int *attachments_with_format;
__DRIbuffer *buffer;
-   const unsigned int format = 32;
+   const unsigned int format = (dri2_surf-format == WL_DRM_FORMAT_RGB565) ? 
16 : 32;
int i;
 
attachments_with_format = calloc(count, 2 * sizeof(unsigned int));
-- 
1.7.9.5

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