[Mesa-dev] [PATCH v1] egl/android: Implement the eglSwapinterval for Android.

[Zhongmin Wu]

Implement the eglSwapinterval for Android platform to
enable the async mode for some GFX benchmarks.

Signed-off-by: Zhongmin Wu 
---
 src/egl/drivers/dri2/platform_android.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/src/egl/drivers/dri2/platform_android.c 
b/src/egl/drivers/dri2/platform_android.c
index f6a24cd..09f1159 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -476,6 +476,19 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, 
_EGLSurface *surf)
return EGL_TRUE;
 }
 
+static EGLBoolean
+droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
+   _EGLSurface *surf, EGLint interval)
+{
+   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
+   struct ANativeWindow *window = dri2_surf->window;
+   if (window->setSwapInterval(window, interval)) {
+  return EGL_FALSE;
+   }
+   surf->SwapInterval = interval;
+   return EGL_TRUE;
+}
+
 static int
 update_buffers(struct dri2_egl_surface *dri2_surf)
 {
@@ -1300,6 +1313,7 @@ static const struct dri2_egl_display_vtbl 
droid_display_vtbl = {
.swap_buffers = droid_swap_buffers,
.swap_buffers_with_damage = dri2_fallback_swap_buffers_with_damage, /* 
Android implements the function */
.swap_buffers_region = dri2_fallback_swap_buffers_region,
+   .swap_interval = droid_swap_interval,
 #if ANDROID_API_LEVEL >= 23
.set_damage_region = droid_set_damage_region,
 #else
@@ -1443,6 +1457,8 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *dpy)
 
dri2_setup_screen(dpy);
 
+   dri2_setup_swap_interval(dpy, 1);
+
if (!droid_add_configs_for_visuals(drv, dpy)) {
   err = "DRI2: failed to add configs";
   goto cleanup;
-- 
2.7.4

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

Re: [Mesa-dev] [egl/android: Implement the eglSwapinterval for Android] egl/android: Implement the eglSwapinterval for Android.

[Wu, Zhongmin]

Hi Tomasz:
Thanks very much for your reply, I will re-submit the patch.
And I did not do the deqp testing yet, I am going to have a try.

-Original Message-
From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of 
Tomasz Figa
Sent: Tuesday, January 16, 2018 15:17 
To: Wu, Zhongmin 
Cc: Long, Zhifang ; Kps, Harish Krupo 
; Xu, Randy ; Chad Versace 
; Eric Engestrom ; Emil Velikov 
; Kondapally, Kalyan ; 
Bhardwaj, MunishX ; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [egl/android: Implement the eglSwapinterval for 
Android] egl/android: Implement the eglSwapinterval for Android.

Hi Zhongmin,

On Tue, Jan 16, 2018 at 4:07 PM, Wu, Zhongmin  wrote:
> Sorry, is there any comment about the below patch, Thanks very much! Or did I 
> miss something ?

I assumed this was sent by mistake. The subject doesn't look like a patch for 
review - it should have [PATCH] prefix. There was even a follow-up email 
(presumably generated by your mailing client) to cancel sending it.

Also please remove internal annotations, such as gerrit Change-Id, since they 
do not have any meaning for upstream purposes.

As for the change itself, it looks fine to me, +/- some style nitpicks, which I 
listed inline. Have you checked if there are no dEQP regressions (at least for 
the EGL suite)?

Best regards,
Tomasz

>
> -Original Message-
> From: Wu, Zhongmin
> Sent: Wednesday, January 3, 2018 10:11
> To: mesa-dev@lists.freedesktop.org
> Cc: Kondapally, Kalyan ; Palli, Tapani 
> ; Xu, Randy ; Long, 
> Zhifang ; Wu, Zhongmin 
> ; Rob Herring ; Tomasz Figa 
> ; Eric Engestrom ; Emil Velikov 
> ; Bhardwaj, MunishX 
> ; Kps, Harish Krupo 
> ; Chad Versace 
> Subject: [egl/android: Implement the eglSwapinterval for Android] 
> egl/android: Implement the eglSwapinterval for Android.
>
> From: Zhongmin Wu 
>
> Implement the eglSwapinterval for Android platform to enable the async mode 
> for some GFX benchmarks.
>
> Change-Id: I3576d8b92862719dae11c31e2adc2d77cb5a0b64
> Signed-off-by: Zhongmin Wu 
> ---
>  src/egl/drivers/dri2/platform_android.c | 15 +++
>  1 file changed, 15 insertions(+)
>
> diff --git a/src/egl/drivers/dri2/platform_android.c 
> b/src/egl/drivers/dri2/platform_android.c
> index f6a24cd..f9c74ee 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -476,6 +476,18 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay 
> *disp, _EGLSurface *surf)
> return EGL_TRUE;
>  }
>
> +static EGLBoolean droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
> +_EGLSurface *surf, EGLint interval) {

Please move the function name to new line, align the arguments with the top 
lines, if there is a need to wrap the lines and move the opening brace to new 
line, to match the coding style already used in the file.

> +

No need for this blank line.

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

Re: [Mesa-dev] [PATCH v4 03/10] nir/spirv: add gl_spirv_validation method

[Alejandro Piñeiro]

On 15/01/18 17:28, Jason Ekstrand wrote:
> On January 15, 2018 06:46:13 Alejandro Piñeiro 
> wrote:
>
>> ARB_gl_spirv adds the ability to use SPIR-V binaries, and a new
>> method, glSpecializeShader. From OpenGL 4.6 spec, section 7.2.1
>> "Shader Specialization", error table:
>>
>>    INVALID_VALUE is generated if  does not name a valid
>>    entry point for .
>>
>>    INVALID_VALUE is generated if any element of 
>>    refers to a specialization constant that does not exist in the
>>    shader module contained in .""
>>
>> But we are not really interested on creating the nir shader at that
>> point, and adding nir structures on the gl_program, so at that point
>> we are just interested on the error checking.
>>
>> So we add a new method focused on just checking those errors. It still
>> needs to parse the binary, but skips what it is not needed, and
>> doesn't create the nir shader.
>>
>> v2: rebase update (spirv_to_nir options added, changes on the warning
>>     logging, and others)
>> v3: include passing options on common initialization, doesn't call
>>     setjmp on common_initialization
>> ---
>>  src/compiler/spirv/nir_spirv.h    |   5 +
>>  src/compiler/spirv/spirv_to_nir.c | 191
>> ++
>>  2 files changed, 180 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/compiler/spirv/nir_spirv.h
>> b/src/compiler/spirv/nir_spirv.h
>> index a2c40e57d18..d2766abb7f9 100644
>> --- a/src/compiler/spirv/nir_spirv.h
>> +++ b/src/compiler/spirv/nir_spirv.h
>> @@ -41,6 +41,7 @@ struct nir_spirv_specialization {
>>    uint32_t data32;
>>    uint64_t data64;
>>     };
>> +   bool defined_on_module;
>>  };
>>
>>  enum nir_spirv_debug_level {
>> @@ -69,6 +70,10 @@ struct spirv_to_nir_options {
>>     } debug;
>>  };
>>
>> +bool gl_spirv_validation(const uint32_t *words, size_t word_count,
>> + struct nir_spirv_specialization *spec,
>> unsigned num_spec,
>> + gl_shader_stage stage, const char
>> *entry_point_name);
>> +
>>  nir_function *spirv_to_nir(const uint32_t *words, size_t word_count,
>>     struct nir_spirv_specialization
>> *specializations,
>>     unsigned num_specializations,
>> diff --git a/src/compiler/spirv/spirv_to_nir.c
>> b/src/compiler/spirv/spirv_to_nir.c
>> index c6df764682e..2143cd9df31 100644
>> --- a/src/compiler/spirv/spirv_to_nir.c
>> +++ b/src/compiler/spirv/spirv_to_nir.c
>> @@ -1332,6 +1332,7 @@ spec_constant_decoration_cb(struct vtn_builder
>> *b, struct vtn_value *v,
>>  const_value->data64 = b->specializations[i].data64;
>>   else
>>  const_value->data32 = b->specializations[i].data32;
>> + b->specializations[i].defined_on_module = true;
>>   return;
>>    }
>>     }
>> @@ -1366,7 +1367,13 @@ handle_workgroup_size_decoration_cb(struct
>> vtn_builder *b,
>>  const struct vtn_decoration *dec,
>>  void *data)
>>  {
>> +   /* This can happens if we are gl_spirv_validation. We can return
>> safely, as
>> +    * we don't need the workgroup info for such validation. */
>> +   if (b->shader == NULL)
>> +  return;
>
> I don't think that re-using these two functions is really buying us
> anything.  We could just make spec constant validation versions that
> just do what's needed there.

Ok, makes sense. I just reused them in order to add as less code as
possible.

>
>> +
>>     vtn_assert(member == -1);
>> +
>>     if (dec->decoration != SpvDecorationBuiltIn ||
>>     dec->literals[0] != SpvBuiltInWorkgroupSize)
>>    return;
>> @@ -3263,6 +3270,49 @@ vtn_handle_preamble_instruction(struct
>> vtn_builder *b, SpvOp opcode,
>>     return true;
>>  }
>>
>> +/*
>> + * gl_spirv validation. Just need to check for the entry point.
>> + */
>> +static bool
>> +vtn_validate_preamble_instruction(struct vtn_builder *b, SpvOp opcode,
>> +  const uint32_t *w, unsigned count)
>> +{
>> +   switch (opcode) {
>> +   /* The following opcodes are not needed for gl_spirv, so we can skip
>> +    * them.
>> +    */
>> +   case SpvOpSource:
>> +   case SpvOpSourceExtension:
>> +   case SpvOpSourceContinued:
>> +   case SpvOpExtension:
>> +   case SpvOpCapability:
>> +   case SpvOpExtInstImport:
>> +   case SpvOpMemoryModel:
>> +   case SpvOpString:
>> +   case SpvOpName:
>> +   case SpvOpMemberName:
>> +   case SpvOpExecutionMode:
>> +   case SpvOpDecorationGroup:
>> +   case SpvOpMemberDecorate:
>> +   case SpvOpGroupDecorate:
>> +   case SpvOpGroupMemberDecorate:
>> +  break;
>> +
>> +   case SpvOpEntryPoint:
>> +  vtn_handle_preamble_instruction(b, opcode, w, count);
>> +  break;
>> +
>> +   case SpvOpDecorate:
>> +  vtn_handle_decoration(b, opcode, w, count);
>> +  break;
>> +
>> +   default:
>> +  return false; /* End of preamble */
>> +   }
>> +
>> +   return true;

Re: [Mesa-dev] DRI Configurator replacement announcement

[Gert Wollny]

Hello Jean, 

Am Montag, den 15.01.2018, 20:15 + schrieb Jean Hertel:
> I have written a simply application like DRI Conf tool.
> It is written using GTKmm and C++.
Great! 

Unfortunately, it didn't link properly, so I send you a pull request to
correct the Boost_LOCALE dependency. 
  
  https://github.com/jlHertel/adriconf/pull/1

Then it crashed with std::bas_alloc that also printed a Python (?!) 
ValueError (opened an issue for that):

https://github.com/jlHertel/adriconf/issues/2

Because of this I could not really test the application. I'll see
whether I can find the culprit later.

> Main Features (apart from what is already available in driconf):
> - Automatic removal of invalid options (Options that the driver
> doesn't support at all)
> - Options that have the same value as the system wide options or
> driver default will be ignored
Makes sense. 

> - Applications with empty options (all options are the same as
> system-wide config or driver default) will be removed automatically
I don't think that is such a good idea, what happens if someone wants
to try out options to compare to the defaults? The application
shouldn't get removed automatically when the user sets all to default.

> Current TODOs:
[...]
> - Tests? Implementing testing for the software would be very nice
+1

> - Remove Boost dependency? (Currently boost.locale is used to get the
> ISO-639 language id, to properly parse Mesa3d translations)
If the OS provides a propper packaging system having one dependency
more should not be a problem. 

I'll take a look at the code itself later. 

Best, 
Gert 

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

Re: [Mesa-dev] [egl/android: Implement the eglSwapinterval for Android] egl/android: Implement the eglSwapinterval for Android.

[Tomasz Figa]

Hi Zhongmin,

On Tue, Jan 16, 2018 at 4:07 PM, Wu, Zhongmin  wrote:
> Sorry, is there any comment about the below patch, Thanks very much! Or did I 
> miss something ?

I assumed this was sent by mistake. The subject doesn't look like a
patch for review - it should have [PATCH] prefix. There was even a
follow-up email (presumably generated by your mailing client) to
cancel sending it.

Also please remove internal annotations, such as gerrit Change-Id,
since they do not have any meaning for upstream purposes.

As for the change itself, it looks fine to me, +/- some style
nitpicks, which I listed inline. Have you checked if there are no dEQP
regressions (at least for the EGL suite)?

Best regards,
Tomasz

>
> -Original Message-
> From: Wu, Zhongmin
> Sent: Wednesday, January 3, 2018 10:11
> To: mesa-dev@lists.freedesktop.org
> Cc: Kondapally, Kalyan ; Palli, Tapani 
> ; Xu, Randy ; Long, Zhifang 
> ; Wu, Zhongmin ; Rob Herring 
> ; Tomasz Figa ; Eric Engestrom 
> ; Emil Velikov ; Bhardwaj, 
> MunishX ; Kps, Harish Krupo 
> ; Chad Versace 
> Subject: [egl/android: Implement the eglSwapinterval for Android] 
> egl/android: Implement the eglSwapinterval for Android.
>
> From: Zhongmin Wu 
>
> Implement the eglSwapinterval for Android platform to enable the async mode 
> for some GFX benchmarks.
>
> Change-Id: I3576d8b92862719dae11c31e2adc2d77cb5a0b64
> Signed-off-by: Zhongmin Wu 
> ---
>  src/egl/drivers/dri2/platform_android.c | 15 +++
>  1 file changed, 15 insertions(+)
>
> diff --git a/src/egl/drivers/dri2/platform_android.c 
> b/src/egl/drivers/dri2/platform_android.c
> index f6a24cd..f9c74ee 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -476,6 +476,18 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay 
> *disp, _EGLSurface *surf)
> return EGL_TRUE;
>  }
>
> +static EGLBoolean droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
> +_EGLSurface *surf, EGLint interval) {

Please move the function name to new line, align the arguments with
the top lines, if there is a need to wrap the lines and move the
opening brace to new line, to match the coding style already used in
the file.

> +

No need for this blank line.

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

Re: [Mesa-dev] [egl/android: Implement the eglSwapinterval for Android] egl/android: Implement the eglSwapinterval for Android.

[Wu, Zhongmin]

Sorry, is there any comment about the below patch, Thanks very much! Or did I 
miss something ?

-Original Message-
From: Wu, Zhongmin 
Sent: Wednesday, January 3, 2018 10:11 
To: mesa-dev@lists.freedesktop.org
Cc: Kondapally, Kalyan ; Palli, Tapani 
; Xu, Randy ; Long, Zhifang 
; Wu, Zhongmin ; Rob Herring 
; Tomasz Figa ; Eric Engestrom 
; Emil Velikov ; Bhardwaj, MunishX 
; Kps, Harish Krupo ; 
Chad Versace 
Subject: [egl/android: Implement the eglSwapinterval for Android] egl/android: 
Implement the eglSwapinterval for Android.

From: Zhongmin Wu 

Implement the eglSwapinterval for Android platform to enable the async mode for 
some GFX benchmarks.

Change-Id: I3576d8b92862719dae11c31e2adc2d77cb5a0b64
Signed-off-by: Zhongmin Wu 
---
 src/egl/drivers/dri2/platform_android.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/egl/drivers/dri2/platform_android.c 
b/src/egl/drivers/dri2/platform_android.c
index f6a24cd..f9c74ee 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -476,6 +476,18 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, 
_EGLSurface *surf)
return EGL_TRUE;
 }
 
+static EGLBoolean droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
+_EGLSurface *surf, EGLint interval) {
+
+   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
+   struct ANativeWindow *window = dri2_surf->window;
+   if (window->setSwapInterval(window, interval)) {
+  return EGL_FALSE;
+   }
+   surf->SwapInterval = interval;
+   return EGL_TRUE;
+}
+
 static int
 update_buffers(struct dri2_egl_surface *dri2_surf)  { @@ -1300,6 +1312,7 @@ 
static const struct dri2_egl_display_vtbl droid_display_vtbl = {
.swap_buffers = droid_swap_buffers,
.swap_buffers_with_damage = dri2_fallback_swap_buffers_with_damage, /* 
Android implements the function */
.swap_buffers_region = dri2_fallback_swap_buffers_region,
+   .swap_interval = droid_swap_interval,
 #if ANDROID_API_LEVEL >= 23
.set_damage_region = droid_set_damage_region,  #else @@ -1443,6 +1456,8 @@ 
dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *dpy)
 
dri2_setup_screen(dpy);
 
+   dri2_setup_swap_interval(dpy, 1);
+
if (!droid_add_configs_for_visuals(drv, dpy)) {
   err = "DRI2: failed to add configs";
   goto cleanup;
--
2.7.4

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

[Mesa-dev] [PATCH] ac: fix nir_intrinsic_get_buffer_size for radeonsi

[Timothy Arceri]

---
 src/amd/common/ac_nir_to_llvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 4a80748e4e..0940dc82d8 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -2401,9 +2401,9 @@ static LLVMValueRef visit_load_push_constant(struct 
nir_to_llvm_context *ctx,
 static LLVMValueRef visit_get_buffer_size(struct ac_nir_context *ctx,
   const nir_intrinsic_instr *instr)
 {
-   LLVMValueRef ptr = get_src(ctx, instr->src[0]);
+   LLVMValueRef index = get_src(ctx, instr->src[0]);
 
-   return get_buffer_size(ctx, LLVMBuildLoad(ctx->ac.builder, ptr, ""), 
false);
+   return get_buffer_size(ctx, ctx->abi->load_ssbo(ctx->abi, index, 
false), false);
 }
 static void visit_store_ssbo(struct ac_nir_context *ctx,
  nir_intrinsic_instr *instr)
-- 
2.14.3

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

[Mesa-dev] [PATCH] ac: fix buffer overflow bug in 64bit SSBO loads

[Timothy Arceri]

Fixes: 441ee1e65b04 "radv/ac: Implement Float64 SSBO loads"
---
 src/amd/common/ac_nir_to_llvm.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 6467ed66ae..4a80748e4e 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -2585,8 +2585,11 @@ static LLVMValueRef visit_load_buffer(struct 
ac_nir_context *ctx,
ctx->ac.i1false,
};
 
-   results[i] = ac_build_intrinsic(>ac, load_name, data_type, 
params, 5, 0);
+   int idx = i;
+   if (instr->dest.ssa.bit_size == 64)
+   idx = i > 1 ? 1 : 0;
 
+   results[idx] = ac_build_intrinsic(>ac, load_name, 
data_type, params, 5, 0);
}
 
assume(results[0]);
-- 
2.14.3

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

[Mesa-dev] [Bug 103699] Latest mesa breaks firefox on kde plasma with compositing on

[bugzilla-daemon]

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

Tapani Pälli  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|ASSIGNED|RESOLVED

--- Comment #29 from Tapani Pälli  ---
I haven't been able to reproduce this after following commit landed to Xorg and
I haven't received any new bugs on this subject. Resolving as fixed, please
reopen if this still occurs.

--- 8< ---
commit c2954b16c8730c7ed8441fd8dba25900f3aed265
Author: Tapani Pälli 
Date:   Tue Nov 28 09:23:29 2017 +0200

glx: do not pick sRGB config for 32-bit RGBA visual

This fixes blending issues seen with kwin and gnome-shell when
32bit visual has sRGB capability set.

Reviewed-by: Adam Jackson 
Signed-off-by: Tapani Pälli 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103699
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103646
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103655

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

Re: [Mesa-dev] [PATCH] st/dri: Default ALLOW_RGB10_CONFIGS to false on gallium as well.

[Tapani Pälli]

Reviewed-by: Tapani Pälli 

On 16.01.2018 06:39, Mario Kleiner wrote:

For consistency with the i965 default of "off".

Signed-off-by: Mario Kleiner 
---
  src/gallium/auxiliary/pipe-loader/driinfo_gallium.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h 
b/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
index 505aae4..446bc06 100644
--- a/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
+++ b/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
@@ -32,5 +32,5 @@ DRI_CONF_SECTION_END
  DRI_CONF_SECTION_MISCELLANEOUS
 DRI_CONF_ALWAYS_HAVE_DEPTH_BUFFER("false")
 DRI_CONF_GLSL_ZERO_INIT("false")
-   DRI_CONF_ALLOW_RGB10_CONFIGS("true")
+   DRI_CONF_ALLOW_RGB10_CONFIGS("false")
  DRI_CONF_SECTION_END


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

[Mesa-dev] [PATCH] st/dri: Default ALLOW_RGB10_CONFIGS to false on gallium as well.

[Mario Kleiner]

For consistency with the i965 default of "off".

Signed-off-by: Mario Kleiner 
---
 src/gallium/auxiliary/pipe-loader/driinfo_gallium.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h 
b/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
index 505aae4..446bc06 100644
--- a/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
+++ b/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
@@ -32,5 +32,5 @@ DRI_CONF_SECTION_END
 DRI_CONF_SECTION_MISCELLANEOUS
DRI_CONF_ALWAYS_HAVE_DEPTH_BUFFER("false")
DRI_CONF_GLSL_ZERO_INIT("false")
-   DRI_CONF_ALLOW_RGB10_CONFIGS("true")
+   DRI_CONF_ALLOW_RGB10_CONFIGS("false")
 DRI_CONF_SECTION_END
-- 
2.7.4

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

[Mesa-dev] [PATCH] draw: fix vsplit code when the (post-bias) index value is -1

[sroland]

From: Roland Scheidegger 

vsplit_add_cache uses the post-bias index for hashing, but the
vsplit_add_cache_uint/ushort/ubyte ones used the pre-bias index, therefore
the code for handling the special case (because -1 matches the initialization
value of the cache) wasn't actually working.
Commit 78a997f72841310620d18daa9015633343d04db1 actually simplified the
cache logic somewhat, but it looks like this particular problem carried over
(and duplicated to the ushort/ubyte cases, since before only uint needed it).
This could lead to the vsplit cache doing the wrong thing, in particular
later fetch_info might indicate there are 0 values to fetch. This only really
affected edge cases which were bogus to begin with, but it could lead to a
crash with the jit vertex shader, since it cannot handle this case correctly
(the count loop is always executed at least once and we would not allocate
any memory for the shader outputs), so add another assert to catch it there.
---
 src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c | 1 +
 src/gallium/auxiliary/draw/draw_pt_vsplit.c| 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c 
b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
index c6492a1..5e0c562 100644
--- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
+++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
@@ -368,6 +368,7 @@ llvm_pipeline_generic(struct draw_pt_middle_end *middle,
unsigned start_or_maxelt, vid_base;
const unsigned *elts;
 
+   assert(fetch_info->count > 0);
llvm_vert_info.count = fetch_info->count;
llvm_vert_info.vertex_size = fpme->vertex_size;
llvm_vert_info.stride = fpme->vertex_size;
diff --git a/src/gallium/auxiliary/draw/draw_pt_vsplit.c 
b/src/gallium/auxiliary/draw/draw_pt_vsplit.c
index a68d5bf..3ff077b 100644
--- a/src/gallium/auxiliary/draw/draw_pt_vsplit.c
+++ b/src/gallium/auxiliary/draw/draw_pt_vsplit.c
@@ -133,7 +133,7 @@ vsplit_add_cache_ubyte(struct vsplit_frontend *vsplit, 
const ubyte *elts,
VSPLIT_CREATE_IDX(elts, start, fetch, elt_bias);
/* unlike the uint case this can only happen with elt_bias */
if (elt_bias && elt_idx == DRAW_MAX_FETCH_IDX && 
!vsplit->cache.has_max_fetch) {
-  unsigned hash = fetch % MAP_SIZE;
+  unsigned hash = elt_idx % MAP_SIZE;
   vsplit->cache.fetches[hash] = 0;
   vsplit->cache.has_max_fetch = TRUE;
}
@@ -148,7 +148,7 @@ vsplit_add_cache_ushort(struct vsplit_frontend *vsplit, 
const ushort *elts,
VSPLIT_CREATE_IDX(elts, start, fetch, elt_bias);
/* unlike the uint case this can only happen with elt_bias */
if (elt_bias && elt_idx == DRAW_MAX_FETCH_IDX && 
!vsplit->cache.has_max_fetch) {
-  unsigned hash = fetch % MAP_SIZE;
+  unsigned hash = elt_idx % MAP_SIZE;
   vsplit->cache.fetches[hash] = 0;
   vsplit->cache.has_max_fetch = TRUE;
}
@@ -168,7 +168,7 @@ vsplit_add_cache_uint(struct vsplit_frontend *vsplit, const 
uint *elts,
VSPLIT_CREATE_IDX(elts, start, fetch, elt_bias);
/* Take care for DRAW_MAX_FETCH_IDX (since cache is initialized to -1). */
if (elt_idx == DRAW_MAX_FETCH_IDX && !vsplit->cache.has_max_fetch) {
-  unsigned hash = fetch % MAP_SIZE;
+  unsigned hash = elt_idx % MAP_SIZE;
   /* force update - any value will do except DRAW_MAX_FETCH_IDX */
   vsplit->cache.fetches[hash] = 0;
   vsplit->cache.has_max_fetch = TRUE;
-- 
2.7.4

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

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

[Grazvydas Ignotas]

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

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

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

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

[Grazvydas Ignotas]

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

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

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

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

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

Re: [Mesa-dev] [PATCH 1/2] meson: Fix configuring dri glx with only gallium drivers

[Adam Jackson]

On Mon, 2018-01-15 at 20:01 +, Jon Turney wrote:
> On 12/01/2018 17:25, Dylan Baker wrote:
> > meson considers classic swrast to be a dri driver, I know it's not exactly
> > accurate, but, at least for me, it made the build system easier to reason 
> > about.
> 
> I think maybe the point here is that '-Ddri-drivers= -Dgallium-drivers= 
> -Dglx=dri' (or at least, it's autotools equivalent) is a valid 
> configuration, and gets you a libGL that falls back to indirect, since 
> no swrast or real DRI driver can be loaded[*]. (Maybe it's even the only 
> way to get that?)

I mean, it'd be valid to build libGL with -DGLX_DIRECT_RENDERING and
just not build any drivers, it'd be able to load the ones you've
already built or that came with your OS. But that's probably an even
less common thing to attempt than indirect-only.

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

[Mesa-dev] DRI Configurator replacement announcement

[Jean Hertel]

Hello,

I have written a simply application like DRI Conf tool.
It is written using GTKmm and C++.

Main Features (apart from what is already available in driconf):
- Automatic removal of invalid options (Options that the driver doesn't support 
at all)
- Options that have the same value as the system wide options or driver default 
will be ignored
- Applications with empty options (all options are the same as system-wide 
config or driver default) will be removed automatically

Current TODOs:
- Properly support a system without X (wayland systems?) Currently the glX 
functions mandate a Xlib display object. There must be another way to get the 
driver options
- Properly deal with PRIME setups (how do we get more information from the 
driver? hardware ids?)
- Some code cleanups. I'm not very experienced with C++ yet (Maybe split the 
GUI class, as it is very big and not very readable)
- Tests? Implementing testing for the software would be very nice
- Remove Boost dependency? (Currently boost.locale is used to get the ISO-639 
language id, to properly parse Mesa3d translations)

My main motivation for this project is to learn C++/GTKmm and a little bit more 
about Mesa itself, so if you read the code, be carefull that hidden bugs can be 
there.

Any feedback or hint on how to solve the TODOs list is very welcome.
Also, as this is a project for learning, feel free to point anything wrong with 
the code. I will be very happy to fix it and learn more.

Source is under github: https://github.com/jlHertel/adriconf

Kind Regards,
Jean Hertel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] meson: Fix configuring dri glx with only gallium drivers

[Jon Turney]

On 12/01/2018 17:25, Dylan Baker wrote:

meson considers classic swrast to be a dri driver, I know it's not exactly
accurate, but, at least for me, it made the build system easier to reason about.


I think maybe the point here is that '-Ddri-drivers= -Dgallium-drivers= 
-Dglx=dri' (or at least, it's autotools equivalent) is a valid 
configuration, and gets you a libGL that falls back to indirect, since 
no swrast or real DRI driver can be loaded[*]. (Maybe it's even the only 
way to get that?)


[*] on Windows, it also hits some platform-specific client-side 
rendering first; on OSX, that's all it does (due to some badness I have 
some half-finished patches to fix...)



Quoting Adam Jackson (2018-01-12 09:06:37)

On Fri, 2018-01-12 at 13:18 +, Jon Turney wrote:

'meson -Ddri-drivers= -Dgallium-drivers=swrast -Dglx=dri' fails with 'dri
based GLX requires at least one DRI driver'

Signed-off-by: Jon Turney 
---
  meson.build | 2 +-
  src/glx/meson.build | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 77e4e894b23..dd8e6145edb 100644
--- a/meson.build
+++ b/meson.build
@@ -323,7 +323,7 @@ if with_glx != 'disabled'
  if with_dri
error('xlib conflicts with any dri driver')
  endif
-  elif with_glx == 'dri' and not with_dri
+  elif with_glx == 'dri' and not (with_dri or with_gallium)
  error('dri based GLX requires at least one DRI driver')


We should just remove this check. libGL doesn't actually require a DRI
driver, and at least on OSX there's no DRI driver you could possibly
build.

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

[Mesa-dev] [Bug 104490] [radeonsi/290x] Dota2 fails to start (can't create opengl context)

[bugzilla-daemon]

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

--- Comment #11 from Adam Jackson  ---
I've reverted these for now. I'm reasonably sure that the issue is a skew
between X server and Mesa, such that the app thinks it can create a no-flush
context but either the server or libGL disagrees and throws an error. This
shouldn't be terribly hard to track down for someone with the time and
motivation; at the moment I lack the time.

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

Re: [Mesa-dev] [PATCH 1/2] meson: Fix configuring dri glx with only gallium drivers

[Jon Turney]

On 12/01/2018 17:33, Dylan Baker wrote:

Maybe this is correct, but it makes me nervous treating with_gallium as
equivalent to with_dri, since gallium drivers can be built dri-less
(gallium-xlib, and some other configurations on windows). I think something
like:

   with_glx = get_option('glx')
   if with_glx == 'auto'
 if with_dri
   with_glx = 'dri'
 elif with_gallium
   # Even when building just gallium drivers the user probably wants dri
   with_glx = 'dri'
   with_dri = true
 elif with_platform_x11 and with_any_opengl and not with_any_vk
   # The automatic behavior should not be to turn on xlib based glx when
   # building only vulkan drivers
   with_glx = 'xlib'
 else
   with_glx = 'disabled'
 endif
+ elif with_glx == 'dri'
+   if with_gallium
+ with_dri = true
+   endif
   endif


Would achieve the correct result, be simpler, and avoid accidentally adding dri
sources when we shouldn't.


Ah, yes.  I'd completely failed to spot that in the 'auto' case above.

How about the attached?
From f6d27e04bd7d8581b2cb723edaf6449eddb77cc8 Mon Sep 17 00:00:00 2001
From: Jon Turney 
Date: Mon, 15 Jan 2018 19:39:46 +
Subject: [PATCH] meson: Set with_dri from with_gallium when DRI glx is
 explicitly configured

Set with_dri from with_gallium when DRI GLX is explicitly configured, as
well as when DRI GLX is chosen automatically.

Signed-off-by: Jon Turney 
---
 meson.build | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 77e4e894b23..7bc4983d96e 100644
--- a/meson.build
+++ b/meson.build
@@ -248,7 +248,6 @@ if with_glx == 'auto'
   elif with_gallium
 # Even when building just gallium drivers the user probably wants dri
 with_glx = 'dri'
-with_dri = true
   elif with_platform_x11 and with_any_opengl and not with_any_vk
 # The automatic behavior should not be to turn on xlib based glx when
 # building only vulkan drivers
@@ -257,6 +256,11 @@ if with_glx == 'auto'
 with_glx = 'disabled'
   endif
 endif
+if with_glx == 'dri'
+   if with_gallium
+  with_dri = true
+   endif
+endif
 
 if not (with_dri or with_gallium or with_glx == 'xlib' or with_glx == 
'gallium-xlib')
   with_gles1 = false
-- 
2.15.1

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

Re: [Mesa-dev] [PATCH 2/4] i965/miptree: Use the tiling from the modifier instead of the BO

[Jason Ekstrand]

On Mon, Jan 15, 2018 at 5:47 AM, Pohjolainen, Topi <
topi.pohjolai...@gmail.com> wrote:

> On Thu, Jan 11, 2018 at 05:40:51PM -0800, Jason Ekstrand wrote:
> > From: Jason Ekstrand 
> >
> > This fixes a bug where we were taking the tiling from the BO regardless
> > of what the modifier said.  When we got images in from Vulkan where it
> > doesn't set the tiling on the BO, we would treat them as linear even
> > though the modifier expressly said to treat it as Y-tiled.
>
> I noticed that I didn't get the tiling from Vulkan when I played with
> ext_memory_object. Hence I only ran my new piglit test with linear tiling.
> I was about to ask how do we pass the tiling from Vulkan to GL?
>

That's where things get tricky... The way this is supposed to work is that
you specify TILING_OPTIMAL in Vulkan and then the two drivers are supposed
to just magically make the same choice.  Even better, they're supposed to
magically make the same choice for everything: tiling, alignment
parameters, stride, qpitch, etc.  This is why we needed to switch GL over
to ISL prior to even attempting to implement that extension.  One other
option would be to store the isl_surf somewhere in the BO.  However, even
there, the Vulkan driver needs to create an isl_surf that the GL driver
knows what to do with so all of the GL driver's restrictions apply.

In order for this to all work, we need to look at every additional
restriction in the GL driver and either get rid of it somehow, or move it
into ISL.  I have a strong preference for getting rid of things over moving
stuff to ISL if we can help it.  For instance, we may have to use the
blitter less to get rid of our linear fall-back for wide surfaces.


> Anyway here patches 1 and 2 are:
>
> Reviewed-by: Topi Pohjolainen 
>

Thanks!


> >
> > Cc: mesa-sta...@lists.freedesktop.org
> > Reviewed-by: Daniel Stone 
> > ---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 9 +
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index a0474ef..a9c2810 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -986,7 +986,11 @@ intel_miptree_create_for_dri_image(struct
> brw_context *brw,
> > uint32_t bo_tiling, bo_swizzle;
> > brw_bo_get_tiling(image->bo, _tiling, _swizzle);
> >
> > -   const enum isl_tiling tiling = isl_tiling_from_i915_tiling(
> bo_tiling);
> > +   const struct isl_drm_modifier_info *mod_info =
> > +  isl_drm_modifier_get_info(image->modifier);
> > +
> > +   const enum isl_tiling tiling =
> > +  mod_info ? mod_info->tiling : isl_tiling_from_i915_tiling(
> bo_tiling);
> >
> > if (image->planar_format && image->planar_format->nplanes > 1)
> >return miptree_create_for_planar_image(brw, image, target,
> tiling);
> > @@ -1010,9 +1014,6 @@ intel_miptree_create_for_dri_image(struct
> brw_context *brw,
> > if (!brw->ctx.TextureFormatSupported[format])
> >return NULL;
> >
> > -   const struct isl_drm_modifier_info *mod_info =
> > -  isl_drm_modifier_get_info(image->modifier);
> > -
> > enum intel_miptree_create_flags mt_create_flags = 0;
> >
> > /* If this image comes in from a window system, we have different
> > --
> > 2.5.0.400.gff86faf
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] vbo: optimize some display list drawing (v2)

[Brian Paul]

The vbo_save_vertex_list structure records one or more glBegin/End
primitives which all have the same vertex format.

To draw these primitives, we setup the vertex array state, then
issue the drawing command.  Before, the 'start' vertex was typically
zero and we used the vertex array pointer to indicate where the
vertex data starts.

This patch checks if the vertex buffer offset is an exact multiple of
the vertex size.  If so, that means we can use zero-based vertex array
pointers and use the draw's start value to indicate where the vertex
data starts.

This means a series of display list drawing commands may have
identical vertex array state.  This will get filtered out by the
Gallium CSO module so we can issue a tight series of drawing commands
without state changes to the device.

Note that this also works for a series of glCallList commands (not
just one list that contains multiple glBegin/End pairs).

No Piglit or conform changes.

v2: minor fixes suggested by Ian.
---
 src/mesa/vbo/vbo_save.h  | 14 ++
 src/mesa/vbo/vbo_save_api.c  | 14 ++
 src/mesa/vbo/vbo_save_draw.c | 12 
 3 files changed, 40 insertions(+)

diff --git a/src/mesa/vbo/vbo_save.h b/src/mesa/vbo/vbo_save.h
index 9d13e0a..274d667 100644
--- a/src/mesa/vbo/vbo_save.h
+++ b/src/mesa/vbo/vbo_save.h
@@ -86,6 +86,20 @@ struct vbo_save_vertex_list {
struct vbo_save_primitive_store *prim_store;
 };
 
+
+/**
+ * Is the vertex lists's buffer offset an exact multiple of the
+ * vertex size (in bytes)?  This is used to check for a vertex array /
+ * drawing optimization.
+ */
+static inline bool
+aligned_vertex_buffer_offset(const struct vbo_save_vertex_list *node)
+{
+   unsigned vertex_size = node->vertex_size * sizeof(GLfloat); /* in bytes */
+   return vertex_size != 0 && node->buffer_offset % vertex_size == 0;
+}
+
+
 /* These buffers should be a reasonable size to support upload to
  * hardware.  Current vbo implementation will re-upload on any
  * changes, so don't make too big or apps which dynamically create
diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c
index 42d883f..1c57544 100644
--- a/src/mesa/vbo/vbo_save_api.c
+++ b/src/mesa/vbo/vbo_save_api.c
@@ -546,6 +546,20 @@ compile_vertex_list(struct gl_context *ctx)
   save->prim_store = alloc_prim_store();
}
 
+   /*
+* If the vertex buffer offset is a multiple of the vertex size,
+* we can use the _mesa_prim::start value to indicate where the
+* vertices starts, instead of the buffer offset.  Also see the
+* bind_vertex_list() function.
+*/
+   if (aligned_vertex_buffer_offset(node)) {
+  const unsigned start_offset =
+ node->buffer_offset / (node->vertex_size * sizeof(GLfloat));
+  for (unsigned i = 0; i < save->prim_count; i++) {
+ save->prims[i].start += start_offset;
+  }
+   }
+
/* Reset our structures for the next run of vertices:
 */
reset_counters(ctx);
diff --git a/src/mesa/vbo/vbo_save_draw.c b/src/mesa/vbo/vbo_save_draw.c
index 1694a04..b63a9a8 100644
--- a/src/mesa/vbo/vbo_save_draw.c
+++ b/src/mesa/vbo/vbo_save_draw.c
@@ -146,6 +146,18 @@ bind_vertex_list(struct gl_context *ctx,
memcpy(node_attrsz, node->attrsz, sizeof(node->attrsz));
memcpy(node_attrtype, node->attrtype, sizeof(node->attrtype));
 
+   if (aligned_vertex_buffer_offset(node)) {
+  /* The vertex size is an exact multiple of the buffer offset.
+   * This means that we can use zero-based vertex attribute pointers
+   * and specify the start of the primitive with the _mesa_prim::start
+   * field.  This results in issuing several draw calls with identical
+   * vertex attribute information.  This can result in fewer state
+   * changes in drivers.  In particular, the Gallium CSO module will
+   * filter out redundant vertex buffer changes.
+   */
+  buffer_offset = 0;
+   }
+
/* Install the default (ie Current) attributes first, then overlay
 * all active ones.
 */
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 17/17] vbo: optimize some display list drawing

[Brian Paul]

On 01/12/2018 03:52 PM, Ian Romanick wrote:

On 01/12/2018 02:23 PM, Brian Paul wrote:

The vbo_save_vertex_list structure records one or more glBegin/End
primitives which all have the same vertex format.

To draw these primitives, we setup the vertex array state, then
issue the drawing command.  Before, the 'start' vertex was typically
zero and we used the vertex array pointer to indicate where the
vertex data starts.

This patch checks if the vertex buffer offset is an exact multiple of
the vertex size.  If so, that means we can use zero-based vertex array
pointers and use the draw's start value to indicate where the vertex
data starts.

This means a series of display list drawing commands may have
identical vertex array state.  This will get filtered out by the
Gallium CSO module so we can issue a tight series of drawing commands
without state changes to the device.

Note that this also works for a series of glCallList commands (not
just one list that contains multiple glBegin/End pairs).

No Piglit or conform changes.


Do you know if any of these tests actually hit these paths?


I don't.  I ran Piglit/conform just to be sure there were no regressions.



 I always
worry about changes to display list handing because there is so little
testing. :(


I test a number of Windows CAD apps/etc which use display lists.  In 
fact, this change is a result of trying to optimize one of those.  It's 
pretty hairy code, but I think I understand it fairly well now.




A few nits below.


---
  src/mesa/vbo/vbo_save.h  | 17 +
  src/mesa/vbo/vbo_save_api.c  | 15 +++
  src/mesa/vbo/vbo_save_draw.c | 12 
  3 files changed, 44 insertions(+)

diff --git a/src/mesa/vbo/vbo_save.h b/src/mesa/vbo/vbo_save.h
index 9d13e0a..468a04a 100644
--- a/src/mesa/vbo/vbo_save.h
+++ b/src/mesa/vbo/vbo_save.h
@@ -86,6 +86,23 @@ struct vbo_save_vertex_list {
 struct vbo_save_primitive_store *prim_store;
  };
  
+

+/**
+ * Is the vertex lists's buffer offset an exact multiple of the
+ * vertex size (in bytes)?  This is used to check for a vertex array /
+ * drawing optimization.
+ */
+static inline bool
+aligned_vertex_buffer_offset(const struct vbo_save_vertex_list *node)
+{
+   unsigned vertex_size = node->vertex_size * sizeof(GLfloat); /* in bytes */
+   if (vertex_size)
+  return node->buffer_offset % vertex_size == 0;
+   else
+  return false;


I think this would be more clear as

return vertex_size != 0 && node->buffer_offset % vertex_size == 0;


Ok.





+}
+
+
  /* These buffers should be a reasonable size to support upload to
   * hardware.  Current vbo implementation will re-upload on any
   * changes, so don't make too big or apps which dynamically create
diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c
index 42d883f..b9d382a 100644
--- a/src/mesa/vbo/vbo_save_api.c
+++ b/src/mesa/vbo/vbo_save_api.c
@@ -546,6 +546,21 @@ compile_vertex_list(struct gl_context *ctx)
save->prim_store = alloc_prim_store();
 }
  
+   /**


The Doxygen /** start marker doesn't do anything useful here.


right.





+* If the vertex buffer offset is a multiple of the vertex size,
+* we can use the _mesa_prim::start value to indicate where the
+* vertices starts, instead of the buffer offset.  Also see the
+* bind_vertex_list() function.
+*/
+   if (aligned_vertex_buffer_offset(node)) {
+  const unsigned start_offset =
+ node->buffer_offset / (node->vertex_size * sizeof(GLfloat));
+  unsigned i;
+  for (i = 0; i < save->prim_count; i++) {
+ save->prims[i].start += start_offset;
+  }


I believe the currently common style is:

   for (unsigned i = 0; i < save->prim_count; i++)
  save->prims[i].start += start_offset;


Yeah, I'm stuck on old habits.

v2 coming.

-Brian




+   }
+
 /* Reset our structures for the next run of vertices:
  */
 reset_counters(ctx);
diff --git a/src/mesa/vbo/vbo_save_draw.c b/src/mesa/vbo/vbo_save_draw.c
index 1694a04..b63a9a8 100644
--- a/src/mesa/vbo/vbo_save_draw.c
+++ b/src/mesa/vbo/vbo_save_draw.c
@@ -146,6 +146,18 @@ bind_vertex_list(struct gl_context *ctx,
 memcpy(node_attrsz, node->attrsz, sizeof(node->attrsz));
 memcpy(node_attrtype, node->attrtype, sizeof(node->attrtype));
  
+   if (aligned_vertex_buffer_offset(node)) {

+  /* The vertex size is an exact multiple of the buffer offset.
+   * This means that we can use zero-based vertex attribute pointers
+   * and specify the start of the primitive with the _mesa_prim::start
+   * field.  This results in issuing several draw calls with identical
+   * vertex attribute information.  This can result in fewer state
+   * changes in drivers.  In particular, the Gallium CSO module will
+   * filter out redundant vertex buffer changes.
+   */
+  buffer_offset = 0;
+   }
+
 /* Install the default (ie Current) attributes first, then overlay
  * all 

Re: [Mesa-dev] [PATCH 11/17] anv: Separate compute and graphics descriptor sets

[Pohjolainen, Topi]

On Mon, Jan 15, 2018 at 10:47:08AM -0800, Jason Ekstrand wrote:
> On Mon, Jan 15, 2018 at 7:11 AM, Pohjolainen, Topi <
> topi.pohjolai...@gmail.com> wrote:
> 
> > On Fri, Dec 15, 2017 at 05:09:09PM -0800, Jason Ekstrand wrote:
> > > The Vulkan spec says:
> > >
> > > "pipelineBindPoint is a VkPipelineBindPoint indicating whether the
> > > descriptors will be used by graphics pipelines or compute pipelines.
> > > There is a separate set of bind points for each of graphics and
> > > compute, so binding one does not disturb the other."
> > >
> > > Up until now, we've been ignoring the pipeline bind point and had just
> > > one bind point for everything.  This commit separates things out into
> > > separate bind points.
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102897
> > > ---
> > >  src/intel/vulkan/anv_cmd_buffer.c | 65
> > ++-
> > >  src/intel/vulkan/anv_descriptor_set.c |  2 ++
> > >  src/intel/vulkan/anv_private.h| 11 +++---
> > >  src/intel/vulkan/genX_cmd_buffer.c| 24 +++--
> > >  4 files changed, 70 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/anv_cmd_buffer.c
> > b/src/intel/vulkan/anv_cmd_buffer.c
> > > index 636f515..9720e7e 100644
> > > --- a/src/intel/vulkan/anv_cmd_buffer.c
> > > +++ b/src/intel/vulkan/anv_cmd_buffer.c
> > > @@ -124,12 +124,20 @@ anv_cmd_state_init(struct anv_cmd_buffer
> > *cmd_buffer)
> > >  }
> > >
> > >  static void
> > > +anv_cmd_pipeline_state_finish(struct anv_cmd_buffer *cmd_buffer,
> > > +  struct anv_cmd_pipeline_state *pipe_state)
> > > +{
> > > +   for (uint32_t i = 0; i < ARRAY_SIZE(pipe_state->push_descriptors);
> > i++)
> > > +  vk_free(_buffer->pool->alloc, pipe_state->push_descriptors[
> > i]);
> > > +}
> > > +
> > > +static void
> > >  anv_cmd_state_finish(struct anv_cmd_buffer *cmd_buffer)
> > >  {
> > > struct anv_cmd_state *state = _buffer->state;
> > >
> > > -   for (uint32_t i = 0; i < ARRAY_SIZE(state->push_descriptors); i++)
> > > -  vk_free(_buffer->pool->alloc, state->push_descriptors[i]);
> > > +   anv_cmd_pipeline_state_finish(cmd_buffer, >gfx.base);
> > > +   anv_cmd_pipeline_state_finish(cmd_buffer, >compute.base);
> > >
> > > for (uint32_t i = 0; i < MESA_SHADER_STAGES; i++)
> > >vk_free(_buffer->pool->alloc, state->push_constants[i]);
> > > @@ -495,6 +503,7 @@ void anv_CmdSetStencilReference(
> > >
> > >  static void
> > >  anv_cmd_buffer_bind_descriptor_set(struct anv_cmd_buffer *cmd_buffer,
> > > +   VkPipelineBindPoint bind_point,
> > > struct anv_pipeline_layout *layout,
> > > uint32_t set_index,
> > > struct anv_descriptor_set *set,
> > > @@ -504,7 +513,14 @@ anv_cmd_buffer_bind_descriptor_set(struct
> > anv_cmd_buffer *cmd_buffer,
> > > struct anv_descriptor_set_layout *set_layout =
> > >layout->set[set_index].layout;
> > >
> > > -   cmd_buffer->state.descriptors[set_index] = set;
> > > +   struct anv_cmd_pipeline_state *pipe_state;
> > > +   if (bind_point == VK_PIPELINE_BIND_POINT_COMPUTE) {
> > > +  pipe_state = _buffer->state.compute.base;
> > > +   } else {
> > > +  assert(bind_point == VK_PIPELINE_BIND_POINT_GRAPHICS);
> > > +  pipe_state = _buffer->state.gfx.base;
> > > +   }
> > > +   pipe_state->descriptors[set_index] = set;
> > >
> > > if (dynamic_offsets) {
> > >if (set_layout->dynamic_offset_count > 0) {
> > > @@ -514,9 +530,9 @@ anv_cmd_buffer_bind_descriptor_set(struct
> > anv_cmd_buffer *cmd_buffer,
> > >   /* Assert that everything is in range */
> > >   assert(set_layout->dynamic_offset_count <=
> > *dynamic_offset_count);
> > >   assert(dynamic_offset_start + set_layout->dynamic_offset_count
> > <=
> > > -ARRAY_SIZE(cmd_buffer->state.dynamic_offsets));
> > > +ARRAY_SIZE(pipe_state->dynamic_offsets));
> > >
> > > - typed_memcpy(_buffer->state.dynamic_offsets[dynamic_
> > offset_start],
> > > + typed_memcpy(_state->dynamic_offsets[dynamic_
> > offset_start],
> > >*dynamic_offsets, set_layout->dynamic_offset_
> > count);
> > >
> > >   *dynamic_offsets += set_layout->dynamic_offset_count;
> > > @@ -524,7 +540,13 @@ anv_cmd_buffer_bind_descriptor_set(struct
> > anv_cmd_buffer *cmd_buffer,
> > >}
> > > }
> > >
> > > -   cmd_buffer->state.descriptors_dirty |= set_layout->shader_stages;
> > > +   if (bind_point == VK_PIPELINE_BIND_POINT_COMPUTE) {
> > > +  cmd_buffer->state.descriptors_dirty |=
> > VK_SHADER_STAGE_COMPUTE_BIT;
> > > +   } else {
> > > +  assert(bind_point == VK_PIPELINE_BIND_POINT_GRAPHICS);
> > > +  cmd_buffer->state.descriptors_dirty |=
> > > + set_layout->shader_stages & VK_SHADER_STAGE_ALL_GRAPHICS;
> >
> > Should we put () around 

Re: [Mesa-dev] [PATCH 01/10] mesa: Also track a remapped version of the color logic op

[Brian Paul]

On 01/12/2018 03:56 PM, Ian Romanick wrote:

From: Ian Romanick 

With the exception of NVIDIA hardware, these are is the values that all
hardware and Gallium want.  The remapping is currently implemented in at
least 6 places.  This starts the process of consolidating to a single
place.

Signed-off-by: Ian Romanick 
---
  src/mesa/main/blend.c  | 22 ++
  src/mesa/main/mtypes.h | 29 +
  2 files changed, 51 insertions(+)

diff --git a/src/mesa/main/blend.c b/src/mesa/main/blend.c
index 01721ab..f47b102 100644
--- a/src/mesa/main/blend.c
+++ b/src/mesa/main/blend.c
@@ -849,6 +849,26 @@ _mesa_AlphaFunc( GLenum func, GLclampf ref )
 }
  }
  
+static const enum color_logic_ops color_logicop_mapping[16] = {

+   COLOR_LOGICOP_CLEAR,
+   COLOR_LOGICOP_AND,
+   COLOR_LOGICOP_AND_REVERSE,
+   COLOR_LOGICOP_COPY,
+   COLOR_LOGICOP_AND_INVERTED,
+   COLOR_LOGICOP_NOOP,
+   COLOR_LOGICOP_XOR,
+   COLOR_LOGICOP_OR,
+   COLOR_LOGICOP_NOR,
+   COLOR_LOGICOP_EQUIV,
+   COLOR_LOGICOP_INVERT,
+   COLOR_LOGICOP_OR_REVERSE,
+   COLOR_LOGICOP_COPY_INVERTED,
+   COLOR_LOGICOP_OR_INVERTED,
+   COLOR_LOGICOP_NAND,
+   COLOR_LOGICOP_SET
+};
+
+#define GLenum_to_color_logicop(x) color_logicop_mapping[x & 0x0f]
  
  static ALWAYS_INLINE void

  logic_op(struct gl_context *ctx, GLenum opcode, bool no_error)
@@ -884,6 +904,7 @@ logic_op(struct gl_context *ctx, GLenum opcode, bool 
no_error)
 FLUSH_VERTICES(ctx, ctx->DriverFlags.NewLogicOp ? 0 : _NEW_COLOR);
 ctx->NewDriverState |= ctx->DriverFlags.NewLogicOp;
 ctx->Color.LogicOp = opcode;
+   ctx->Color._LogicOp = GLenum_to_color_logicop(opcode);
  
 if (ctx->Driver.LogicOpcode)

ctx->Driver.LogicOpcode(ctx, opcode);
@@ -1189,6 +1210,7 @@ void _mesa_init_color( struct gl_context * ctx )
 ctx->Color.IndexLogicOpEnabled = GL_FALSE;
 ctx->Color.ColorLogicOpEnabled = GL_FALSE;
 ctx->Color.LogicOp = GL_COPY;
+   ctx->Color._LogicOp = COLOR_LOGICOP_COPY;
 ctx->Color.DitherFlag = GL_TRUE;
  
 /* GL_FRONT is not possible on GLES. Instead GL_BACK will render to either

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 226eb94..2fbfd27 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -418,6 +418,34 @@ union gl_color_union
 GLuint ui[4];
  };
  
+/**

+ * Remapped color logical operations
+ *
+ * With the exception of NVIDIA hardware, which consumes the OpenGL enumerants
+ * directly, everything wants this mapping of color logical operations.
+ *
+ * Fun fact: These values are just the bit-reverse of the low-nibble of the GL
+ * enumerant values (i.e., `GL_NOOP & 0x0f` is `b0101' while
+ * \c COLOR_LOGICOP_NOOP is `b1010`).
+ */
+enum PACKED color_logic_ops {


A name such as gl_logicop_mode would be more in line with other mesa 
enum types.




+   COLOR_LOGICOP_CLEAR = 0,
+   COLOR_LOGICOP_NOR = 1,
+   COLOR_LOGICOP_AND_INVERTED = 2,
+   COLOR_LOGICOP_COPY_INVERTED = 3,
+   COLOR_LOGICOP_AND_REVERSE = 4,
+   COLOR_LOGICOP_INVERT = 5,
+   COLOR_LOGICOP_XOR = 6,
+   COLOR_LOGICOP_NAND = 7,
+   COLOR_LOGICOP_AND = 8,
+   COLOR_LOGICOP_EQUIV = 9,
+   COLOR_LOGICOP_NOOP = 10,
+   COLOR_LOGICOP_OR_INVERTED = 11,
+   COLOR_LOGICOP_COPY = 12,
+   COLOR_LOGICOP_OR_REVERSE = 13,
+   COLOR_LOGICOP_OR = 14,
+   COLOR_LOGICOP_SET = 15
+};
  
  /**

   * Color buffer attribute group (GL_COLOR_BUFFER_BIT).
@@ -493,6 +521,7 @@ struct gl_colorbuffer_attrib
 GLboolean IndexLogicOpEnabled; /**< Color index logic op enabled flag 
*/
 GLboolean ColorLogicOpEnabled; /**< RGBA logic op enabled flag */
 GLenum LogicOp;/**< Logic operator */
+   enum color_logic_ops _LogicOp;
  
 /*@}*/
  



For patches 1,3,4, Reviewed-by: Brian Paul 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 11/17] anv: Separate compute and graphics descriptor sets

[Jason Ekstrand]

On Mon, Jan 15, 2018 at 7:11 AM, Pohjolainen, Topi <
topi.pohjolai...@gmail.com> wrote:

> On Fri, Dec 15, 2017 at 05:09:09PM -0800, Jason Ekstrand wrote:
> > The Vulkan spec says:
> >
> > "pipelineBindPoint is a VkPipelineBindPoint indicating whether the
> > descriptors will be used by graphics pipelines or compute pipelines.
> > There is a separate set of bind points for each of graphics and
> > compute, so binding one does not disturb the other."
> >
> > Up until now, we've been ignoring the pipeline bind point and had just
> > one bind point for everything.  This commit separates things out into
> > separate bind points.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102897
> > ---
> >  src/intel/vulkan/anv_cmd_buffer.c | 65
> ++-
> >  src/intel/vulkan/anv_descriptor_set.c |  2 ++
> >  src/intel/vulkan/anv_private.h| 11 +++---
> >  src/intel/vulkan/genX_cmd_buffer.c| 24 +++--
> >  4 files changed, 70 insertions(+), 32 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_cmd_buffer.c
> b/src/intel/vulkan/anv_cmd_buffer.c
> > index 636f515..9720e7e 100644
> > --- a/src/intel/vulkan/anv_cmd_buffer.c
> > +++ b/src/intel/vulkan/anv_cmd_buffer.c
> > @@ -124,12 +124,20 @@ anv_cmd_state_init(struct anv_cmd_buffer
> *cmd_buffer)
> >  }
> >
> >  static void
> > +anv_cmd_pipeline_state_finish(struct anv_cmd_buffer *cmd_buffer,
> > +  struct anv_cmd_pipeline_state *pipe_state)
> > +{
> > +   for (uint32_t i = 0; i < ARRAY_SIZE(pipe_state->push_descriptors);
> i++)
> > +  vk_free(_buffer->pool->alloc, pipe_state->push_descriptors[
> i]);
> > +}
> > +
> > +static void
> >  anv_cmd_state_finish(struct anv_cmd_buffer *cmd_buffer)
> >  {
> > struct anv_cmd_state *state = _buffer->state;
> >
> > -   for (uint32_t i = 0; i < ARRAY_SIZE(state->push_descriptors); i++)
> > -  vk_free(_buffer->pool->alloc, state->push_descriptors[i]);
> > +   anv_cmd_pipeline_state_finish(cmd_buffer, >gfx.base);
> > +   anv_cmd_pipeline_state_finish(cmd_buffer, >compute.base);
> >
> > for (uint32_t i = 0; i < MESA_SHADER_STAGES; i++)
> >vk_free(_buffer->pool->alloc, state->push_constants[i]);
> > @@ -495,6 +503,7 @@ void anv_CmdSetStencilReference(
> >
> >  static void
> >  anv_cmd_buffer_bind_descriptor_set(struct anv_cmd_buffer *cmd_buffer,
> > +   VkPipelineBindPoint bind_point,
> > struct anv_pipeline_layout *layout,
> > uint32_t set_index,
> > struct anv_descriptor_set *set,
> > @@ -504,7 +513,14 @@ anv_cmd_buffer_bind_descriptor_set(struct
> anv_cmd_buffer *cmd_buffer,
> > struct anv_descriptor_set_layout *set_layout =
> >layout->set[set_index].layout;
> >
> > -   cmd_buffer->state.descriptors[set_index] = set;
> > +   struct anv_cmd_pipeline_state *pipe_state;
> > +   if (bind_point == VK_PIPELINE_BIND_POINT_COMPUTE) {
> > +  pipe_state = _buffer->state.compute.base;
> > +   } else {
> > +  assert(bind_point == VK_PIPELINE_BIND_POINT_GRAPHICS);
> > +  pipe_state = _buffer->state.gfx.base;
> > +   }
> > +   pipe_state->descriptors[set_index] = set;
> >
> > if (dynamic_offsets) {
> >if (set_layout->dynamic_offset_count > 0) {
> > @@ -514,9 +530,9 @@ anv_cmd_buffer_bind_descriptor_set(struct
> anv_cmd_buffer *cmd_buffer,
> >   /* Assert that everything is in range */
> >   assert(set_layout->dynamic_offset_count <=
> *dynamic_offset_count);
> >   assert(dynamic_offset_start + set_layout->dynamic_offset_count
> <=
> > -ARRAY_SIZE(cmd_buffer->state.dynamic_offsets));
> > +ARRAY_SIZE(pipe_state->dynamic_offsets));
> >
> > - typed_memcpy(_buffer->state.dynamic_offsets[dynamic_
> offset_start],
> > + typed_memcpy(_state->dynamic_offsets[dynamic_
> offset_start],
> >*dynamic_offsets, set_layout->dynamic_offset_
> count);
> >
> >   *dynamic_offsets += set_layout->dynamic_offset_count;
> > @@ -524,7 +540,13 @@ anv_cmd_buffer_bind_descriptor_set(struct
> anv_cmd_buffer *cmd_buffer,
> >}
> > }
> >
> > -   cmd_buffer->state.descriptors_dirty |= set_layout->shader_stages;
> > +   if (bind_point == VK_PIPELINE_BIND_POINT_COMPUTE) {
> > +  cmd_buffer->state.descriptors_dirty |=
> VK_SHADER_STAGE_COMPUTE_BIT;
> > +   } else {
> > +  assert(bind_point == VK_PIPELINE_BIND_POINT_GRAPHICS);
> > +  cmd_buffer->state.descriptors_dirty |=
> > + set_layout->shader_stages & VK_SHADER_STAGE_ALL_GRAPHICS;
>
> Should we put () around the right hand side? We seem to be using that
> elsewhere.
>

Meh.  I think it's fairly obvious what's going on.  I do sometimes insist
on wraping == statements in () because a = b == c is something I find hard
to read but a = b & c seems obvious to me.


> > +   }
> >  }
> >
> >  void 

Re: [Mesa-dev] [PATCH] glx: fix non-dri build

[Samuel Thibault]

Nicolai Hähnle, on lun. 15 janv. 2018 15:07:03 +0100, wrote:
> On 13.01.2018 12:36, Samuel Thibault wrote:
> > glXGetDriverConfig parameters do not provide a context to dynamically
> > check for the presence of the function, so the dispatcher directly calls
> > glXGetDriverConfig, but in non-dri builds dri_glx.c didn't provide
> > glXGetDriverConfig.
> > 
> > This change makes it provide a NULL-returning stub in non-dri builds.
> > 
> > Fixes: 84f764a7591 "glxglvnddispatch: Add missing dispatch for 
> > GetDriverConfig"
> 
> Would it be possible to instead modify dispatch_GetDriverConfig with an:
> 
> #if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
>...
> #else
>return NULL;
> #endif

Sure!  There is just one thing: src/glx/g_glxglvnddispatchfuncs.c reads 

 * THIS FILE IS AUTOMATICALLY GENERATED BY gen_scrn_dispatch.pl
 * DO NOT EDIT!!

I didn't find that script...

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

[Mesa-dev] [PATCHv2] glx: fix non-dri build

[Samuel Thibault]

glXGetDriverConfig parameters do not provide a context to dynamically
check for the presence of the function, so the dispatcher directly calls
glXGetDriverConfig, but in non-dri builds dri_glx.c didn't provide
glXGetDriverConfig.

This change make it just return NULL in that case.

Fixes: 84f764a7591 "glxglvnddispatch: Add missing dispatch for GetDriverConfig

---
Difference between v1 and v2: just modify the call in
dispatch_GetDriverConfig rather than adding glXGetDriverConfig and
always adding dri_glx to build system.
---
 src/glx/g_glxglvnddispatchfuncs.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/glx/g_glxglvnddispatchfuncs.c 
b/src/glx/g_glxglvnddispatchfuncs.c
index 56d894eda..5b65afc86 100644
--- a/src/glx/g_glxglvnddispatchfuncs.c
+++ b/src/glx/g_glxglvnddispatchfuncs.c
@@ -338,11 +338,15 @@ static Display *dispatch_GetCurrentDisplayEXT(void)
 
 static const char *dispatch_GetDriverConfig(const char *driverName)
 {
+#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
 /*
  * The options are constant for a given driverName, so we do not need
  * a context (and apps expect to be able to call this without one).
  */
 return glXGetDriverConfig(driverName);
+#else
+return NULL;
+#endif
 }
 
 
-- 
2.15.1

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

Re: [Mesa-dev] [PATCHv2] glx: fix non-dri build

[Hans de Goede]

Hi,

On 15-01-18 15:38, Samuel Thibault wrote:

glXGetDriverConfig parameters do not provide a context to dynamically
check for the presence of the function, so the dispatcher directly calls
glXGetDriverConfig, but in non-dri builds dri_glx.c didn't provide
glXGetDriverConfig.

This change make it just return NULL in that case.

Fixes: 84f764a7591 "glxglvnddispatch: Add missing dispatch for GetDriverConfig


Seems sensible to me and thank you for fixing this up after me :)   :

Reviewed-by: Hans de Goede 

Regards,

Hans





---
Difference between v1 and v2: just modify the call in
dispatch_GetDriverConfig rather than adding glXGetDriverConfig and
always adding dri_glx to build system.
---
  src/glx/g_glxglvnddispatchfuncs.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/src/glx/g_glxglvnddispatchfuncs.c 
b/src/glx/g_glxglvnddispatchfuncs.c
index 56d894eda..5b65afc86 100644
--- a/src/glx/g_glxglvnddispatchfuncs.c
+++ b/src/glx/g_glxglvnddispatchfuncs.c
@@ -338,11 +338,15 @@ static Display *dispatch_GetCurrentDisplayEXT(void)
  
  static const char *dispatch_GetDriverConfig(const char *driverName)

  {
+#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
  /*
   * The options are constant for a given driverName, so we do not need
   * a context (and apps expect to be able to call this without one).
   */
  return glXGetDriverConfig(driverName);
+#else
+return NULL;
+#endif
  }
  
  


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

Re: [Mesa-dev] [RFC libdrm 0/5] Move alloc_handle_t from gralloc impls.

[Robert Foss]

Hey Tomasz,

On 01/15/2018 04:03 PM, Tomasz Figa wrote:

On Tue, Jan 16, 2018 at 12:00 AM, Rob Herring  wrote:

On Mon, Jan 15, 2018 at 7:09 AM, Robert Foss  wrote:

Hey,

On 01/13/2018 12:49 AM, Gurchetan Singh wrote:


 We can define accessor functions too (not ptrs), then the struct is
opaque
 and you can do your own accessor implementation if aligning is not
possible
 or desired.


Accessor functions in libdrm sound good to me.



Alright, this seems straight forward enough. As for the accessor
implementations, does anyone mind if I start out with support for multiple
planes even if the buffer handle currently doesn't contain multi plane
support
in various fields (fds, strides, offsets, etc.).


That would be good. Once we convert over to the accessors in users,
then we can change the handle.


Sounds good to me. FYI the handle used by cros_gralloc can already
describe multiple planes.


Yep, so the next step would be to add support for the multi plane fields that 
currently are implemented in (the google) cros_gralloc, but also the fields that 
currently don't exist at all like offset and pixel_stride.



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

Re: [Mesa-dev] [PATCH v4 03/10] nir/spirv: add gl_spirv_validation method

[Jason Ekstrand]

On January 15, 2018 06:46:13 Alejandro Piñeiro  wrote:


ARB_gl_spirv adds the ability to use SPIR-V binaries, and a new
method, glSpecializeShader. From OpenGL 4.6 spec, section 7.2.1
"Shader Specialization", error table:

   INVALID_VALUE is generated if  does not name a valid
   entry point for .

   INVALID_VALUE is generated if any element of 
   refers to a specialization constant that does not exist in the
   shader module contained in .""

But we are not really interested on creating the nir shader at that
point, and adding nir structures on the gl_program, so at that point
we are just interested on the error checking.

So we add a new method focused on just checking those errors. It still
needs to parse the binary, but skips what it is not needed, and
doesn't create the nir shader.

v2: rebase update (spirv_to_nir options added, changes on the warning
logging, and others)
v3: include passing options on common initialization, doesn't call
setjmp on common_initialization
---
 src/compiler/spirv/nir_spirv.h|   5 +
 src/compiler/spirv/spirv_to_nir.c | 191 ++
 2 files changed, 180 insertions(+), 16 deletions(-)

diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_spirv.h
index a2c40e57d18..d2766abb7f9 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -41,6 +41,7 @@ struct nir_spirv_specialization {
   uint32_t data32;
   uint64_t data64;
};
+   bool defined_on_module;
 };

 enum nir_spirv_debug_level {
@@ -69,6 +70,10 @@ struct spirv_to_nir_options {
} debug;
 };

+bool gl_spirv_validation(const uint32_t *words, size_t word_count,
+ struct nir_spirv_specialization *spec, unsigned 
num_spec,

+ gl_shader_stage stage, const char *entry_point_name);
+
 nir_function *spirv_to_nir(const uint32_t *words, size_t word_count,
struct nir_spirv_specialization *specializations,
unsigned num_specializations,
diff --git a/src/compiler/spirv/spirv_to_nir.c 
b/src/compiler/spirv/spirv_to_nir.c

index c6df764682e..2143cd9df31 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -1332,6 +1332,7 @@ spec_constant_decoration_cb(struct vtn_builder *b, 
struct vtn_value *v,

 const_value->data64 = b->specializations[i].data64;
  else
 const_value->data32 = b->specializations[i].data32;
+ b->specializations[i].defined_on_module = true;
  return;
   }
}
@@ -1366,7 +1367,13 @@ handle_workgroup_size_decoration_cb(struct 
vtn_builder *b,

 const struct vtn_decoration *dec,
 void *data)
 {
+   /* This can happens if we are gl_spirv_validation. We can return safely, as
+* we don't need the workgroup info for such validation. */
+   if (b->shader == NULL)
+  return;


I don't think that re-using these two functions is really buying us 
anything.  We could just make spec constant validation versions that just 
do what's needed there.



+
vtn_assert(member == -1);
+
if (dec->decoration != SpvDecorationBuiltIn ||
dec->literals[0] != SpvBuiltInWorkgroupSize)
   return;
@@ -3263,6 +3270,49 @@ vtn_handle_preamble_instruction(struct vtn_builder 
*b, SpvOp opcode,

return true;
 }

+/*
+ * gl_spirv validation. Just need to check for the entry point.
+ */
+static bool
+vtn_validate_preamble_instruction(struct vtn_builder *b, SpvOp opcode,
+  const uint32_t *w, unsigned count)
+{
+   switch (opcode) {
+   /* The following opcodes are not needed for gl_spirv, so we can skip
+* them.
+*/
+   case SpvOpSource:
+   case SpvOpSourceExtension:
+   case SpvOpSourceContinued:
+   case SpvOpExtension:
+   case SpvOpCapability:
+   case SpvOpExtInstImport:
+   case SpvOpMemoryModel:
+   case SpvOpString:
+   case SpvOpName:
+   case SpvOpMemberName:
+   case SpvOpExecutionMode:
+   case SpvOpDecorationGroup:
+   case SpvOpMemberDecorate:
+   case SpvOpGroupDecorate:
+   case SpvOpGroupMemberDecorate:
+  break;
+
+   case SpvOpEntryPoint:
+  vtn_handle_preamble_instruction(b, opcode, w, count);
+  break;
+
+   case SpvOpDecorate:
+  vtn_handle_decoration(b, opcode, w, count);
+  break;
+
+   default:
+  return false; /* End of preamble */
+   }
+
+   return true;
+}
+
 static void
 vtn_handle_execution_mode(struct vtn_builder *b, struct vtn_value *entry_point,
   const struct vtn_decoration *mode, void *data)
@@ -3473,6 +3523,22 @@ vtn_handle_variable_or_type_instruction(struct 
vtn_builder *b, SpvOp opcode,

 }

 static bool
+vtn_handle_constant_or_type_instruction(struct vtn_builder *b, SpvOp opcode,
+const uint32_t *w, unsigned count)
+{
+   switch (opcode) {
+   case SpvOpUndef:
+   case 

[Mesa-dev] [PATCH 3/3] mesa/program/prog_optimize.c: Silence two warnings

[Gert Wollny]

Specifically, -Wsign-compare (explicite cast) and -Wunused-param (annotate)

Signed-off-by: Gert Wollny 
---
 src/mesa/program/prog_optimize.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/mesa/program/prog_optimize.c b/src/mesa/program/prog_optimize.c
index 6a228ba258..f6304c3dad 100644
--- a/src/mesa/program/prog_optimize.c
+++ b/src/mesa/program/prog_optimize.c
@@ -925,7 +925,7 @@ update_interval(GLint intBegin[], GLint intEnd[],
 * of the outermost loop that doesn't contain its definition.
 */
for (i = 0; i < loopStackDepth; i++) {
-  if (intBegin[index] < loopStack[i].Start) {
+  if (intBegin[index] < (GLint)loopStack[i].Start) {
 end = loopStack[i].End;
 break;
   }
@@ -1312,7 +1312,7 @@ _mesa_simplify_cmp(struct gl_program * program)
  * instructions, temp regs, etc.
  */
 void
-_mesa_optimize_program(struct gl_context *ctx, struct gl_program *program,
+_mesa_optimize_program(UNUSED struct gl_context *ctx, struct gl_program 
*program,
void *mem_ctx)
 {
GLboolean any_change;
-- 
2.13.6

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

[Mesa-dev] [PATCH 1/3] mesa: make Visual.samples to be of type unsigned int and propagate this

[Gert Wollny]

According to the ARB_multisample Visual.samples is a non-negative Integer.
Consequently define it, and related functions and values as such and fail
in glx/choose_visual if a negative number is given.

Signed-off-by: Gert Wollny 
---
The patch was motivated by Emil: 
https://lists.freedesktop.org/archives/mesa-dev/2017-November/177218.html

It should be noted that the sample count was already converted to unsigned in 
many places, e.g. in XMesaCreateVisual when passing it to 
pipe_screen::is_format_supported. 

PS: I have not commit rights.

 src/gallium/include/state_tracker/st_api.h| 2 +-
 src/gallium/state_trackers/glx/xlib/glx_api.c | 8 ++--
 src/mesa/main/context.c   | 4 ++--
 src/mesa/main/context.h   | 4 ++--
 src/mesa/main/mtypes.h| 2 +-
 src/mesa/main/multisample.c   | 2 +-
 6 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/gallium/include/state_tracker/st_api.h 
b/src/gallium/include/state_tracker/st_api.h
index f95f65f156..ec6e7844b8 100644
--- a/src/gallium/include/state_tracker/st_api.h
+++ b/src/gallium/include/state_tracker/st_api.h
@@ -202,7 +202,7 @@ struct st_visual
enum pipe_format color_format;
enum pipe_format depth_stencil_format;
enum pipe_format accum_format;
-   int samples;
+   unsigned samples;
 
/**
 * Desired render buffer.
diff --git a/src/gallium/state_trackers/glx/xlib/glx_api.c 
b/src/gallium/state_trackers/glx/xlib/glx_api.c
index c473a0fe54..bb8afe0e6f 100644
--- a/src/gallium/state_trackers/glx/xlib/glx_api.c
+++ b/src/gallium/state_trackers/glx/xlib/glx_api.c
@@ -181,7 +181,7 @@ save_glx_visual( Display *dpy, XVisualInfo *vinfo,
  GLint depth_size, GLint stencil_size,
  GLint accumRedSize, GLint accumGreenSize,
  GLint accumBlueSize, GLint accumAlphaSize,
- GLint level, GLint numAuxBuffers, GLint num_samples )
+ GLint level, GLint numAuxBuffers, GLuint num_samples )
 {
GLboolean ximageFlag = GL_TRUE;
XMesaVisual xmvis;
@@ -996,7 +996,11 @@ choose_visual( Display *dpy, int screen, const int *list, 
GLboolean fbConfig )
 
(void) caveat;
 
-
+   if (num_samples < 0) {
+   _mesa_warning(NULL, "GLX_SAMPLES_ARB: number of samples must 
not be negative");
+   return NULL;
+   }
+   
/*
 * Since we're only simulating the GLX extension this function will never
 * find any real GL visuals.  Instead, all we can do is try to find an RGB
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 53261fea51..a52c98112e 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -227,7 +227,7 @@ _mesa_create_visual( GLboolean dbFlag,
  GLint accumGreenBits,
  GLint accumBlueBits,
  GLint accumAlphaBits,
- GLint numSamples )
+ GLuint numSamples )
 {
struct gl_config *vis = CALLOC_STRUCT(gl_config);
if (vis) {
@@ -269,7 +269,7 @@ _mesa_initialize_visual( struct gl_config *vis,
  GLint accumGreenBits,
  GLint accumBlueBits,
  GLint accumAlphaBits,
- GLint numSamples )
+ GLuint numSamples )
 {
assert(vis);
 
diff --git a/src/mesa/main/context.h b/src/mesa/main/context.h
index 17fb86c323..5d9e2ede47 100644
--- a/src/mesa/main/context.h
+++ b/src/mesa/main/context.h
@@ -79,7 +79,7 @@ _mesa_create_visual( GLboolean dbFlag,
  GLint accumGreenBits,
  GLint accumBlueBits,
  GLint accumAlphaBits,
- GLint numSamples );
+ GLuint numSamples );
 
 extern GLboolean
 _mesa_initialize_visual( struct gl_config *v,
@@ -95,7 +95,7 @@ _mesa_initialize_visual( struct gl_config *v,
  GLint accumGreenBits,
  GLint accumBlueBits,
  GLint accumAlphaBits,
- GLint numSamples );
+ GLuint numSamples );
 
 extern void
 _mesa_destroy_visual( struct gl_config *vis );
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 226eb94da9..d33bf08bdf 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -243,7 +243,7 @@ struct gl_config
 
/* ARB_multisample / SGIS_multisample */
GLint sampleBuffers;
-   GLint samples;
+   GLuint samples;
 
/* SGIX_pbuffer / GLX 1.3 */
GLint maxPbufferWidth;
diff --git a/src/mesa/main/multisample.c b/src/mesa/main/multisample.c
index 8ede94b745..dfe6a37142 100644
--- a/src/mesa/main/multisample.c
+++ b/src/mesa/main/multisample.c
@@ -87,7 +87,7 @@ _mesa_GetMultisamplefv(GLenum pname, GLuint index, GLfloat * 
val)
 
switch (pname) {
case GL_SAMPLE_POSITION: {
-  if ((int) index >= 

[Mesa-dev] [PATCH 2/3] mesa/program/prog_execute: silence a -Wunused-param warning by annotation

[Gert Wollny]

Signed-off-by: Gert Wollny 
---
 src/mesa/program/prog_execute.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/program/prog_execute.c b/src/mesa/program/prog_execute.c
index 1268476279..4950420fbc 100644
--- a/src/mesa/program/prog_execute.c
+++ b/src/mesa/program/prog_execute.c
@@ -222,7 +222,7 @@ fetch_vector4(const struct prog_src_register *source,
  * XXX this currently only works for fragment program input attribs.
  */
 static void
-fetch_vector4_deriv(struct gl_context * ctx,
+fetch_vector4_deriv(UNUSED struct gl_context * ctx,
 const struct prog_src_register *source,
 const struct gl_program_machine *machine,
 char xOrY, GLfloat result[4])
-- 
2.13.6

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

Re: [Mesa-dev] [PATCH 1/7] gallium: Refactor out vl_put_screen and vl_get_screen

[Leo Liu]

Hi lists,


If there is no more questions, and no objection, I would like to commit 
this new OMX st. to upstream.


@Gurkipal, If you can send me or lists your rebased patches set for 
committing, that would be appreciated.



Leo


On 12/04/2017 08:58 AM, Leo Liu wrote:




On 12/03/2017 10:04 AM, Gurkirpal Singh wrote:

I sent the modified patches in another thread a while ago.
Please review in case got missed.

Please be patient for a few days to see if any other comments.
After then please rebase, add rb/ab to your patches, and send them to me,
I will commit them for you.

Leo




On Thu, Nov 30, 2017 at 7:35 PM, Leo Liu > wrote:




On 11/30/2017 06:22 AM, Julien Isorce wrote:

Hi Gurkirpal,

> Before refactoring process both the state trackers were in independent 
directories.
> During earlier refactoring effort we decided to keep that directory 
structure so it made
> sense to move them to auxiliary code. After that I moved them both under 
st/omx.
> Since there could be a chance of it being useful out of st/omx, I left 
the decision to
> keep it or move it back to st/omx to the mailing list.

Yes please move it back to st/omx for the reasons you said, i.e.
there will now 2
sub directories, st/omx/bellagio and st/omx/tizonia and common
code in st/omx.

Yes. Please move them back to st/omx.

With that fixed, the series are:

Acked-by: Leo Liu  

Thanks for the work!

Leo





Another reason is that the env var "OMX_RENDER_NODE" mentions OMX.

Thx!
Julien


On 29 November 2017 at 04:02, Gurkirpal Singh
> wrote:

---
 src/gallium/auxiliary/Makefile.sources            |   2 +
 src/gallium/auxiliary/vl/vl_screen.c              | 107
+
 src/gallium/auxiliary/vl/vl_screen.h              |  33 +++
 .../state_trackers/omx_bellagio/entrypoint.c      |  83

 .../state_trackers/omx_bellagio/entrypoint.h      |   3 -
 src/gallium/state_trackers/omx_bellagio/vid_dec.c |   5 +-
 src/gallium/state_trackers/omx_bellagio/vid_enc.c |   5 +-
 7 files changed, 148 insertions(+), 90 deletions(-)
 create mode 100644 src/gallium/auxiliary/vl/vl_screen.c
 create mode 100644 src/gallium/auxiliary/vl/vl_screen.h

diff --git a/src/gallium/auxiliary/Makefile.sources
b/src/gallium/auxiliary/Makefile.sources
index f40c472..35e89f9 100644
--- a/src/gallium/auxiliary/Makefile.sources
+++ b/src/gallium/auxiliary/Makefile.sources
@@ -343,6 +343,8 @@ VL_SOURCES := \
        vl/vl_mpeg12_decoder.c \
        vl/vl_mpeg12_decoder.h \
        vl/vl_rbsp.h \
+       vl/vl_screen.c \
+       vl/vl_screen.h \
        vl/vl_types.h \
        vl/vl_vertex_buffers.c \
        vl/vl_vertex_buffers.h \
diff --git a/src/gallium/auxiliary/vl/vl_screen.c
b/src/gallium/auxiliary/vl/vl_screen.c
new file mode 100644
index 000..7192802
--- /dev/null
+++ b/src/gallium/auxiliary/vl/vl_screen.c
@@ -0,0 +1,107 @@

+/**
+ *
+ * Permission is hereby granted, free of charge, to any
person obtaining a
+ * copy of this software and associated documentation files
(the
+ * "Software"), to deal in the Software without
restriction, including
+ * without limitation the rights to use, copy, modify,
merge, publish,
+ * distribute, sub license, and/or sell copies of the
Software, and to
+ * permit persons to whom the Software is furnished to do
so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice
(including the
+ * next paragraph) shall be included in all copies or
substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
NON-INFRINGEMENT.
+ * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+

**/
+
+#include 
+#include 
+#include 
 

Re: [Mesa-dev] [PATCH 29/29] anv: Use blorp_ccs_ambiguate instead of fast-clears

[Pohjolainen, Topi]

On Sat, Jan 13, 2018 at 10:33:29AM +0200, Pohjolainen, Topi wrote:
> On Mon, Nov 27, 2017 at 07:06:19PM -0800, Jason Ekstrand wrote:
> > Even though the blorp pass looks a bit on the sketchy side, the end
> > result in the Vulkan driver is very nice.  Instead of having this weird
> > case where you do a fast clear and then maybe have to resolve, we just
> > do the ambiguate and are done with it.  The ambiguate does exactly what
> > we want of setting all the CCS values to 0 which puts it inot the

 into

> > pass-through state.
> 
> For me there wasn't enough context here to understand fully what is going on.
> Looking in a tree made it clearer why we don't need to bother about the
> current color values themselves.
> 
> Reviewed-by: Topi Pohjolainen 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 00/17] anv: Rework state tracking to better separate

[Pohjolainen, Topi]

On Fri, Dec 15, 2017 at 05:08:58PM -0800, Jason Ekstrand wrote:
> This series is intended to address a bug filed in September:
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=102897
> 
> Unfortunately, the fix is either a lot of patches or very messy.  This
> series (as is common for me) takes the lots of patches approach.  The
> general idea is to break out a large chunk of anv_cmd_state into sub-
> structs for graphics and compute.  This way, it's very clear when you're
> accessing any bit of state that you're pulling from one or the other.  By
> giving these a base struct, we can also make clear that a certain set of
> states are are per-pipeline-bind-point.  In order to reduce churn in the
> patches which actually move state from one struct to another, there are
> several patches which just make us make better use of helper functions and
> temporary variables.

I had a small nit in patch 11 but otherwise I couldn't find anything amiss:

Reviewed-by: Topi Pohjolainen 

> 
> Jason Ekstrand (17):
>   anv/pipeline: Don't assert on more than 32 samplers
>   anv/cmd_state: Drop the scratch_size field
>   anv/cmd_buffer: Get rid of the meta query workaround
>   anv/cmd_buffer: Rework anv_cmd_state_reset
>   anv/cmd_buffer: Use some pre-existing pipeline temporaries
>   anv/cmd_buffer: Add substructs to anv_cmd_state for graphics and
> compute
>   anv: Remove semicolons from vk_error[f] definitions
>   anv/cmd_buffer: Refactor ensure_push_descriptor_set
>   anv/cmd_buffer: Add a helper for binding descriptor sets
>   anv/cmd_buffer: Use anv_descriptor_for_binding for samplers
>   anv: Separate compute and graphics descriptor sets
>   anv/cmd_buffer: Move dirty bits into anv_cmd_*_state
>   anv/cmd_buffer: Move vb_dirty bits into anv_cmd_graphics_state
>   anv/cmd_buffer: Use a temporary variable for dynamic state
>   anv/cmd_buffer: Move dynamic state to graphics state
>   anv/cmd_buffer: Move num_workgroups to compute state
>   anv/cmd_buffer: Move gen7 index buffer state to graphics state
> 
>  src/intel/vulkan/anv_cmd_buffer.c | 274 
> +++---
>  src/intel/vulkan/anv_descriptor_set.c |   2 +
>  src/intel/vulkan/anv_private.h|  85 ---
>  src/intel/vulkan/gen7_cmd_buffer.c|  67 -
>  src/intel/vulkan/gen8_cmd_buffer.c|  91 ++-
>  src/intel/vulkan/genX_blorp_exec.c|   4 +-
>  src/intel/vulkan/genX_cmd_buffer.c| 121 ---
>  src/intel/vulkan/genX_gpu_memcpy.c|   2 +-
>  src/intel/vulkan/genX_pipeline.c  |   8 +-
>  src/intel/vulkan/genX_query.c |  14 --
>  10 files changed, 369 insertions(+), 299 deletions(-)
> 
> -- 
> 2.5.0.400.gff86faf
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 11/17] anv: Separate compute and graphics descriptor sets

[Pohjolainen, Topi]

On Fri, Dec 15, 2017 at 05:09:09PM -0800, Jason Ekstrand wrote:
> The Vulkan spec says:
> 
> "pipelineBindPoint is a VkPipelineBindPoint indicating whether the
> descriptors will be used by graphics pipelines or compute pipelines.
> There is a separate set of bind points for each of graphics and
> compute, so binding one does not disturb the other."
> 
> Up until now, we've been ignoring the pipeline bind point and had just
> one bind point for everything.  This commit separates things out into
> separate bind points.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102897
> ---
>  src/intel/vulkan/anv_cmd_buffer.c | 65 
> ++-
>  src/intel/vulkan/anv_descriptor_set.c |  2 ++
>  src/intel/vulkan/anv_private.h| 11 +++---
>  src/intel/vulkan/genX_cmd_buffer.c| 24 +++--
>  4 files changed, 70 insertions(+), 32 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_cmd_buffer.c 
> b/src/intel/vulkan/anv_cmd_buffer.c
> index 636f515..9720e7e 100644
> --- a/src/intel/vulkan/anv_cmd_buffer.c
> +++ b/src/intel/vulkan/anv_cmd_buffer.c
> @@ -124,12 +124,20 @@ anv_cmd_state_init(struct anv_cmd_buffer *cmd_buffer)
>  }
>  
>  static void
> +anv_cmd_pipeline_state_finish(struct anv_cmd_buffer *cmd_buffer,
> +  struct anv_cmd_pipeline_state *pipe_state)
> +{
> +   for (uint32_t i = 0; i < ARRAY_SIZE(pipe_state->push_descriptors); i++)
> +  vk_free(_buffer->pool->alloc, pipe_state->push_descriptors[i]);
> +}
> +
> +static void
>  anv_cmd_state_finish(struct anv_cmd_buffer *cmd_buffer)
>  {
> struct anv_cmd_state *state = _buffer->state;
>  
> -   for (uint32_t i = 0; i < ARRAY_SIZE(state->push_descriptors); i++)
> -  vk_free(_buffer->pool->alloc, state->push_descriptors[i]);
> +   anv_cmd_pipeline_state_finish(cmd_buffer, >gfx.base);
> +   anv_cmd_pipeline_state_finish(cmd_buffer, >compute.base);
>  
> for (uint32_t i = 0; i < MESA_SHADER_STAGES; i++)
>vk_free(_buffer->pool->alloc, state->push_constants[i]);
> @@ -495,6 +503,7 @@ void anv_CmdSetStencilReference(
>  
>  static void
>  anv_cmd_buffer_bind_descriptor_set(struct anv_cmd_buffer *cmd_buffer,
> +   VkPipelineBindPoint bind_point,
> struct anv_pipeline_layout *layout,
> uint32_t set_index,
> struct anv_descriptor_set *set,
> @@ -504,7 +513,14 @@ anv_cmd_buffer_bind_descriptor_set(struct anv_cmd_buffer 
> *cmd_buffer,
> struct anv_descriptor_set_layout *set_layout =
>layout->set[set_index].layout;
>  
> -   cmd_buffer->state.descriptors[set_index] = set;
> +   struct anv_cmd_pipeline_state *pipe_state;
> +   if (bind_point == VK_PIPELINE_BIND_POINT_COMPUTE) {
> +  pipe_state = _buffer->state.compute.base;
> +   } else {
> +  assert(bind_point == VK_PIPELINE_BIND_POINT_GRAPHICS);
> +  pipe_state = _buffer->state.gfx.base;
> +   }
> +   pipe_state->descriptors[set_index] = set;
>  
> if (dynamic_offsets) {
>if (set_layout->dynamic_offset_count > 0) {
> @@ -514,9 +530,9 @@ anv_cmd_buffer_bind_descriptor_set(struct anv_cmd_buffer 
> *cmd_buffer,
>   /* Assert that everything is in range */
>   assert(set_layout->dynamic_offset_count <= *dynamic_offset_count);
>   assert(dynamic_offset_start + set_layout->dynamic_offset_count <=
> -ARRAY_SIZE(cmd_buffer->state.dynamic_offsets));
> +ARRAY_SIZE(pipe_state->dynamic_offsets));
>  
> - 
> typed_memcpy(_buffer->state.dynamic_offsets[dynamic_offset_start],
> + typed_memcpy(_state->dynamic_offsets[dynamic_offset_start],
>*dynamic_offsets, set_layout->dynamic_offset_count);
>  
>   *dynamic_offsets += set_layout->dynamic_offset_count;
> @@ -524,7 +540,13 @@ anv_cmd_buffer_bind_descriptor_set(struct anv_cmd_buffer 
> *cmd_buffer,
>}
> }
>  
> -   cmd_buffer->state.descriptors_dirty |= set_layout->shader_stages;
> +   if (bind_point == VK_PIPELINE_BIND_POINT_COMPUTE) {
> +  cmd_buffer->state.descriptors_dirty |= VK_SHADER_STAGE_COMPUTE_BIT;
> +   } else {
> +  assert(bind_point == VK_PIPELINE_BIND_POINT_GRAPHICS);
> +  cmd_buffer->state.descriptors_dirty |=
> + set_layout->shader_stages & VK_SHADER_STAGE_ALL_GRAPHICS;

Should we put () around the right hand side? We seem to be using that
elsewhere.

> +   }
>  }
>  
>  void anv_CmdBindDescriptorSets(
> @@ -544,8 +566,8 @@ void anv_CmdBindDescriptorSets(
>  
> for (uint32_t i = 0; i < descriptorSetCount; i++) {
>ANV_FROM_HANDLE(anv_descriptor_set, set, pDescriptorSets[i]);
> -  anv_cmd_buffer_bind_descriptor_set(cmd_buffer, layout,
> - firstSet + i, set,
> +  anv_cmd_buffer_bind_descriptor_set(cmd_buffer, pipelineBindPoint,
> + layout, firstSet + 

Re: [Mesa-dev] [RFC libdrm 0/5] Move alloc_handle_t from gralloc impls.

[Tomasz Figa]

On Tue, Jan 16, 2018 at 12:00 AM, Rob Herring  wrote:
> On Mon, Jan 15, 2018 at 7:09 AM, Robert Foss  
> wrote:
>> Hey,
>>
>> On 01/13/2018 12:49 AM, Gurchetan Singh wrote:
>>>
>>> We can define accessor functions too (not ptrs), then the struct is
>>> opaque
>>> and you can do your own accessor implementation if aligning is not
>>> possible
>>> or desired.
>>>
>>>
>>> Accessor functions in libdrm sound good to me.
>>
>>
>> Alright, this seems straight forward enough. As for the accessor
>> implementations, does anyone mind if I start out with support for multiple
>> planes even if the buffer handle currently doesn't contain multi plane
>> support
>> in various fields (fds, strides, offsets, etc.).
>
> That would be good. Once we convert over to the accessors in users,
> then we can change the handle.

Sounds good to me. FYI the handle used by cros_gralloc can already
describe multiple planes.

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

Re: [Mesa-dev] [RFC libdrm 0/5] Move alloc_handle_t from gralloc impls.

[Rob Herring]

On Mon, Jan 15, 2018 at 7:09 AM, Robert Foss  wrote:
> Hey,
>
> On 01/13/2018 12:49 AM, Gurchetan Singh wrote:
>>
>> We can define accessor functions too (not ptrs), then the struct is
>> opaque
>> and you can do your own accessor implementation if aligning is not
>> possible
>> or desired.
>>
>>
>> Accessor functions in libdrm sound good to me.
>
>
> Alright, this seems straight forward enough. As for the accessor
> implementations, does anyone mind if I start out with support for multiple
> planes even if the buffer handle currently doesn't contain multi plane
> support
> in various fields (fds, strides, offsets, etc.).

That would be good. Once we convert over to the accessors in users,
then we can change the handle.

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

[Mesa-dev] [PATCH v4 3/5] spirv_extensions: define spirv_extensions_supported

[Alejandro Piñeiro]

Add a struct to maintain which SPIR-V extensions are supported, and an
utility method to initialize it based on
nir_spirv_supported_capabilities.

v2:
  * Fixing code style (Ian Romanick)
  * Adding a prefix (spirv) to fill_supported_spirv_extensions (Ian Romanick)

v3: rebase update (nir_spirv_supported_extensions renamed)
---
 src/compiler/spirv/spirv_extensions.c | 32 
 src/compiler/spirv/spirv_extensions.h | 13 +
 2 files changed, 45 insertions(+)

diff --git a/src/compiler/spirv/spirv_extensions.c 
b/src/compiler/spirv/spirv_extensions.c
index 3acbe28408a..dd8df817c8c 100644
--- a/src/compiler/spirv/spirv_extensions.c
+++ b/src/compiler/spirv/spirv_extensions.c
@@ -21,6 +21,7 @@
  * IN THE SOFTWARE.
  */
 
+#include 
 #include "spirv_extensions.h"
 #include "util/macros.h"
 
@@ -44,3 +45,34 @@ spirv_extensions_to_string(enum SpvExtension ext)
 
return "unknown";
 }
+
+/**
+ * Sets the supported flags for known SPIR-V extensions based on the
+ * capabilites supported (spirv capabilities based on the spirv to nir
+ * support).
+ *
+ * One could argue that makes more sense in the other way around, as from the
+ * spec pov capabilities are enable for a given extension. But from our pov,
+ * we support or not (depending on the driver) some given capability, and
+ * spirv_to_nir check for capabilities not extensions. Also we usually fill
+ * first the supported capabilities, that are not always related to an
+ * extension.
+ */
+void
+spirv_fill_supported_spirv_extensions(struct spirv_supported_extensions *ext,
+  const struct 
spirv_supported_capabilities *cap)
+{
+   for (unsigned i = 0; i < SPV_EXTENSIONS_COUNT; i++)
+  ext->supported[i] = false;
+
+   ext->count = 0;
+
+   ext->supported[SPV_KHR_shader_draw_parameters] = cap->draw_parameters;
+   ext->supported[SPV_KHR_multiview] = cap->multiview;
+   ext->supported[SPV_KHR_variable_pointers] = cap->variable_pointers;
+
+   for (unsigned i = 0; i < SPV_EXTENSIONS_COUNT; i++) {
+  if (ext->supported[i])
+ ext->count++;
+   }
+}
diff --git a/src/compiler/spirv/spirv_extensions.h 
b/src/compiler/spirv/spirv_extensions.h
index 478b128e1da..d26882a8d4c 100644
--- a/src/compiler/spirv/spirv_extensions.h
+++ b/src/compiler/spirv/spirv_extensions.h
@@ -24,6 +24,8 @@
 #ifndef _SPIRV_EXTENSIONS_H_
 #define _SPIRV_EXTENSIONS_H_
 
+#include "compiler/shader_info.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -40,8 +42,19 @@ enum SpvExtension {
SPV_EXTENSIONS_COUNT
 };
 
+struct spirv_supported_extensions {
+   /** Flags the supported extensions. Array to make it easier to iterate. */
+   bool supported[SPV_EXTENSIONS_COUNT];
+
+   /** Number of supported extensions */
+   unsigned int count;
+};
+
 const char *spirv_extensions_to_string(enum SpvExtension ext);
 
+void spirv_fill_supported_spirv_extensions(struct spirv_supported_extensions 
*ext,
+   const struct 
spirv_supported_capabilities *cap);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.11.0

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

[Mesa-dev] [PATCH v4 2/5] spirv_extensions: add list of extensions and to_string method

[Alejandro Piñeiro]

Ideally this should be generated somehow. One option would be gather
all the extension dependencies listed on the core grammar, but there
would be the possibility of not including some of the extensions.

Note that spirv-tools is doing it just slightly better, as it has a
hardcoded list of extensions manually took from the registry, that
they parse to get the enum and the to_string method (see
generate_grammar_tables.py).

v2:
  * Use a macro to improve readability. (Tapani Pälli)
  * Add unreachable on the switch, no default (Eric Engestrom)
  * No typedef enum (Ian Romanick)
  * Sort extensions names (Ian Romanick)
  * Don't add extensions unlikely to be supported by Mesa at any point
(Ian Romanick)

v3: rebase update
---
 src/compiler/Makefile.sources |  2 ++
 src/compiler/nir/meson.build  |  2 ++
 src/compiler/spirv/spirv_extensions.c | 46 
 src/compiler/spirv/spirv_extensions.h | 49 +++
 4 files changed, 99 insertions(+)
 create mode 100644 src/compiler/spirv/spirv_extensions.c
 create mode 100644 src/compiler/spirv/spirv_extensions.h

diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index d3f746f5f94..7961841bc79 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -297,6 +297,8 @@ SPIRV_FILES = \
spirv/GLSL.std.450.h \
spirv/nir_spirv.h \
spirv/spirv.h \
+   spirv/spirv_extensions.c \
+   spirv/spirv_extensions.h \
spirv/spirv_info.h \
spirv/spirv_to_nir.c \
spirv/vtn_alu.c \
diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
index b5f27ad667b..4f7123e6ece 100644
--- a/src/compiler/nir/meson.build
+++ b/src/compiler/nir/meson.build
@@ -185,6 +185,8 @@ files_libnir = files(
   '../spirv/GLSL.std.450.h',
   '../spirv/nir_spirv.h',
   '../spirv/spirv.h',
+  '../spirv/spirv_extensions.c',
+  '../spirv/spirv_extensions.h',
   '../spirv/spirv_info.h',
   '../spirv/spirv_to_nir.c',
   '../spirv/vtn_alu.c',
diff --git a/src/compiler/spirv/spirv_extensions.c 
b/src/compiler/spirv/spirv_extensions.c
new file mode 100644
index 000..3acbe28408a
--- /dev/null
+++ b/src/compiler/spirv/spirv_extensions.c
@@ -0,0 +1,46 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "spirv_extensions.h"
+#include "util/macros.h"
+
+const char *
+spirv_extensions_to_string(enum SpvExtension ext)
+{
+#define STR(x) case x: return #x;
+   switch (ext) {
+   STR(SPV_KHR_16bit_storage);
+   STR(SPV_KHR_device_group);
+   STR(SPV_KHR_multiview);
+   STR(SPV_KHR_shader_ballot);
+   STR(SPV_KHR_shader_draw_parameters);
+   STR(SPV_KHR_storage_buffer_storage_class);
+   STR(SPV_KHR_subgroup_vote);
+   STR(SPV_KHR_variable_pointers);
+   case SPV_EXTENSIONS_COUNT:
+  unreachable("Unknown SPIR-V extension");
+   }
+#undef STR
+
+   return "unknown";
+}
diff --git a/src/compiler/spirv/spirv_extensions.h 
b/src/compiler/spirv/spirv_extensions.h
new file mode 100644
index 000..478b128e1da
--- /dev/null
+++ b/src/compiler/spirv/spirv_extensions.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF 

[Mesa-dev] [PATCH v4 5/5] spirv_extensions: i965: initialize SPIR-V extensions

[Alejandro Piñeiro]

v2: Rebase update after changes on previous patches.
---
 src/mesa/drivers/dri/i965/brw_context.c | 9 -
 src/mesa/main/context.c | 2 ++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
b/src/mesa/drivers/dri/i965/brw_context.c
index 8bcd04ffd93..ac1afcb6b36 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -76,6 +76,7 @@
 #include "isl/isl.h"
 
 #include "compiler/spirv/nir_spirv.h"
+#include "compiler/spirv/spirv_extensions.h"
 /***
  * Mesa's Driver Functions
  ***/
@@ -1089,9 +1090,15 @@ brwCreateContext(gl_api api,
_mesa_compute_version(ctx);
 
/* GL_ARB_gl_spirv */
-   if (ctx->Version >= 33)
+   if (ctx->Version >= 33) {
   brw_initialize_spirv_supported_capabilities(brw);
 
+  /* GL_ARB_spirv_extensions */
+  ctx->Const.SpirVExtensions = MALLOC_STRUCT(spirv_supported_extensions);
+  spirv_fill_supported_spirv_extensions(ctx->Const.SpirVExtensions,
+ >Const.SpirVCapabilities);
+   }
+
_mesa_initialize_dispatch_tables(ctx);
_mesa_initialize_vbo_vtxfmt(ctx);
 
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 53261fea51b..eb2e6516251 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -1377,6 +1377,8 @@ _mesa_free_context_data( struct gl_context *ctx )
if (ctx == _mesa_get_current_context()) {
   _mesa_make_current(NULL, NULL, NULL);
}
+
+   free(ctx->Const.SpirVExtensions);
 }
 
 
-- 
2.11.0

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

[Mesa-dev] [PATCH v4 4/5] spirv_extensions: add spirv_supported_extensions on gl_constants

[Alejandro Piñeiro]

We can use it to get real values for ARB_spirv_extensions methods.

v2: Rebase update after changes on previous patches.
---
 src/mesa/main/mtypes.h   |  3 +++
 src/mesa/main/spirv_extensions.c | 20 +++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 5359a3f9971..e2bfb4bf2c0 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4050,6 +4050,9 @@ struct gl_constants
 
/** GL_ARB_gl_spirv */
struct spirv_supported_capabilities SpirVCapabilities;
+
+   /** GL_ARB_spirv_extensions */
+   struct spirv_supported_extensions *SpirVExtensions;
 };
 
 
diff --git a/src/mesa/main/spirv_extensions.c b/src/mesa/main/spirv_extensions.c
index 40a89c133aa..2bb29461fd4 100644
--- a/src/mesa/main/spirv_extensions.c
+++ b/src/mesa/main/spirv_extensions.c
@@ -27,16 +27,34 @@
  */
 
 #include "spirv_extensions.h"
+#include "compiler/spirv/spirv_extensions.h"
 
 GLuint
 _mesa_get_spirv_extension_count(struct gl_context *ctx)
 {
-   return 0;
+   if (ctx->Const.SpirVExtensions == NULL)
+  return 0;
+
+   return ctx->Const.SpirVExtensions->count;
 }
 
 const GLubyte *
 _mesa_get_enabled_spirv_extension(struct gl_context *ctx,
   GLuint index)
 {
+   unsigned int n = 0;
+
+   if (ctx->Const.SpirVExtensions == NULL)
+  return (const GLubyte *) 0;
+
+   for (unsigned int i = 0; i < SPV_EXTENSIONS_COUNT; i++) {
+  if (ctx->Const.SpirVExtensions->supported[i]) {
+ if (n == index)
+return (const GLubyte *) spirv_extensions_to_string(i);
+ else
+n++;
+  }
+   }
+
return (const GLubyte *) 0;
 }
-- 
2.11.0

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

[Mesa-dev] [PATCH v4 0/5] ARB_spirv_extensions support in Mesa and i965

[Alejandro Piñeiro]

Hi,

This series is the latest version of the support for
ARB_spirv_extensions on i965. The patches are basically the same that
v3 series we sent one month ago [1] but rebased against today master,
and removing the patch that enabled the extension on i965.

As with v3 it was split from a previous series that included also
support for gl_spirv, but since this is an independent extension, we
have chosen to send it separately to streamline review of the two
series.

This extension however depends on gl_spirv, so it should be applied on
top of the v4 series I just sent for gl_spirv
https://lists.freedesktop.org/archives/mesa-dev/2018-January/182018.html
if someone wants to try it.

Although all the patches gathered some feedback, the only with a Rb is
the first one.

A tree of this series can be found at
.

Thanks for reviewing

[1] https://lists.freedesktop.org/archives/mesa-dev/2017-December/180066.html

Alejandro Piñeiro (5):
  spirv_extensions: add GL_ARB_spirv_extensions boilerplate
  spirv_extensions: add list of extensions and to_string method
  spirv_extensions: define spirv_extensions_supported
  spirv_extensions: add spirv_supported_extensions on gl_constants
  spirv_extensions: i965: initialize SPIR-V extensions

 src/compiler/Makefile.sources   |  2 +
 src/compiler/nir/meson.build|  2 +
 src/compiler/spirv/spirv_extensions.c   | 78 +
 src/compiler/spirv/spirv_extensions.h   | 62 +++
 src/mapi/glapi/gen/ARB_spirv_extensions.xml | 13 +
 src/mapi/glapi/gen/Makefile.am  |  1 +
 src/mapi/glapi/gen/gl_API.xml   |  4 ++
 src/mapi/glapi/gen/meson.build  |  1 +
 src/mesa/Makefile.sources   |  2 +
 src/mesa/drivers/dri/i965/brw_context.c |  9 +++-
 src/mesa/main/context.c |  2 +
 src/mesa/main/extensions_table.h|  1 +
 src/mesa/main/get.c |  6 +++
 src/mesa/main/get_hash_params.py|  3 ++
 src/mesa/main/getstring.c   | 12 +
 src/mesa/main/mtypes.h  |  4 ++
 src/mesa/main/spirv_extensions.c| 60 ++
 src/mesa/main/spirv_extensions.h| 49 ++
 src/mesa/meson.build|  2 +
 19 files changed, 312 insertions(+), 1 deletion(-)
 create mode 100644 src/compiler/spirv/spirv_extensions.c
 create mode 100644 src/compiler/spirv/spirv_extensions.h
 create mode 100644 src/mapi/glapi/gen/ARB_spirv_extensions.xml
 create mode 100644 src/mesa/main/spirv_extensions.c
 create mode 100644 src/mesa/main/spirv_extensions.h

-- 
2.11.0

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

[Mesa-dev] [PATCH v4 1/5] spirv_extensions: add GL_ARB_spirv_extensions boilerplate

[Alejandro Piñeiro]

v2:
  * Mention extension gap at gl_API.xml (Emil Velikov)
  * Bail with INVALID_ENUM if extension not available on getStringi (Emil 
Velikov)
  * Use EXTRA_EXT macro when defining the extension at
get.c/get_hash_params.py (Emil Velikov)
  * Rename source files (spirvextensions.[ch] -> spirv_extensions.[ch]) (Ian)

Reviewed-by: Ian Romanick 
---
 src/mapi/glapi/gen/ARB_spirv_extensions.xml | 13 
 src/mapi/glapi/gen/Makefile.am  |  1 +
 src/mapi/glapi/gen/gl_API.xml   |  4 +++
 src/mapi/glapi/gen/meson.build  |  1 +
 src/mesa/Makefile.sources   |  2 ++
 src/mesa/main/extensions_table.h|  1 +
 src/mesa/main/get.c |  6 
 src/mesa/main/get_hash_params.py|  3 ++
 src/mesa/main/getstring.c   | 12 +++
 src/mesa/main/mtypes.h  |  1 +
 src/mesa/main/spirv_extensions.c| 42 +
 src/mesa/main/spirv_extensions.h| 49 +
 src/mesa/meson.build|  2 ++
 13 files changed, 137 insertions(+)
 create mode 100644 src/mapi/glapi/gen/ARB_spirv_extensions.xml
 create mode 100644 src/mesa/main/spirv_extensions.c
 create mode 100644 src/mesa/main/spirv_extensions.h

diff --git a/src/mapi/glapi/gen/ARB_spirv_extensions.xml 
b/src/mapi/glapi/gen/ARB_spirv_extensions.xml
new file mode 100644
index 000..103393104c2
--- /dev/null
+++ b/src/mapi/glapi/gen/ARB_spirv_extensions.xml
@@ -0,0 +1,13 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
index 35e37e95a9f..9a7a268adbf 100644
--- a/src/mapi/glapi/gen/Makefile.am
+++ b/src/mapi/glapi/gen/Makefile.am
@@ -167,6 +167,7 @@ API_XML = \
ARB_shader_subroutine.xml \
ARB_shader_storage_buffer_object.xml \
ARB_sparse_buffer.xml \
+   ARB_spirv_extensions.xml \
ARB_sync.xml \
ARB_tessellation_shader.xml \
ARB_texture_barrier.xml \
diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index d13a3bfd83d..240be0a5f63 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -8404,6 +8404,10 @@
 
 http://www.w3.org/2001/XInclude"/>
 
+
+
+http://www.w3.org/2001/XInclude"/>
+
 
 
 
diff --git a/src/mapi/glapi/gen/meson.build b/src/mapi/glapi/gen/meson.build
index a6a93cc83be..bfc766f7944 100644
--- a/src/mapi/glapi/gen/meson.build
+++ b/src/mapi/glapi/gen/meson.build
@@ -75,6 +75,7 @@ api_xml_files = files(
   'ARB_shader_subroutine.xml',
   'ARB_shader_storage_buffer_object.xml',
   'ARB_sparse_buffer.xml',
+  'ARB_spirv_extensions.xml',
   'ARB_sync.xml',
   'ARB_tessellation_shader.xml',
   'ARB_texture_barrier.xml',
diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
index 53fa486364d..8a41ed1eeef 100644
--- a/src/mesa/Makefile.sources
+++ b/src/mesa/Makefile.sources
@@ -205,6 +205,8 @@ MAIN_FILES = \
main/shader_query.cpp \
main/shared.c \
main/shared.h \
+   main/spirv_extensions.c \
+   main/spirv_extensions.h \
main/state.c \
main/state.h \
main/stencil.c \
diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
index 3dec6ea12f3..caae3364de4 100644
--- a/src/mesa/main/extensions_table.h
+++ b/src/mesa/main/extensions_table.h
@@ -129,6 +129,7 @@ EXT(ARB_shading_language_420pack, 
ARB_shading_language_420pack
 EXT(ARB_shading_language_packing, ARB_shading_language_packing 
  , GLL, GLC,  x ,  x , 2011)
 EXT(ARB_shadow  , ARB_shadow   
  , GLL,  x ,  x ,  x , 2001)
 EXT(ARB_sparse_buffer   , ARB_sparse_buffer
  , GLL, GLC,  x ,  x , 2014)
+EXT(ARB_spirv_extensions, ARB_spirv_extensions 
  ,  x,  GLC,  x ,  x , 2016)
 EXT(ARB_stencil_texturing   , ARB_stencil_texturing
  , GLL, GLC,  x ,  x , 2012)
 EXT(ARB_sync, ARB_sync 
  , GLL, GLC,  x ,  x , 2003)
 EXT(ARB_tessellation_shader , ARB_tessellation_shader  
  ,  x , GLC,  x ,  x , 2009)
diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index 7f2d72aa4bd..25e5566f981 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -34,6 +34,7 @@
 #include "get.h"
 #include "macros.h"
 #include "mtypes.h"
+#include "spirv_extensions.h"
 #include "state.h"
 #include "texcompress.h"
 #include "texstate.h"
@@ -501,6 +502,7 @@ EXTRA_EXT(OES_primitive_bounding_box);
 EXTRA_EXT(ARB_compute_variable_group_size);
 EXTRA_EXT(KHR_robustness);
 EXTRA_EXT(ARB_sparse_buffer);
+EXTRA_EXT(ARB_spirv_extensions);
 
 static const int
 extra_ARB_color_buffer_float_or_glcore[] = {
@@ -1166,6 +1168,10 @@ find_custom_value(struct gl_context *ctx, const 

[Mesa-dev] [PATCH v4 10/10] i965: Don't call process_glsl_ir() for SPIR-V shaders

[Alejandro Piñeiro]

From: Eduardo Lima Mitev 

v2: Use 'spirv_data' from gl_linked_shader instead, to check if shader
   is SPIR-V. (Timothy Arceri)

Reviewed-by: Timothy Arceri 
---
 src/mesa/drivers/dri/i965/brw_link.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp 
b/src/mesa/drivers/dri/i965/brw_link.cpp
index 64267671c05..a010aadf2a5 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -236,7 +236,8 @@ brw_link_shader(struct gl_context *ctx, struct 
gl_shader_program *shProg)
   struct gl_program *prog = shader->Program;
   prog->Parameters = _mesa_new_parameter_list();
 
-  process_glsl_ir(brw, shProg, shader);
+  if (!shader->spirv_data)
+ process_glsl_ir(brw, shProg, shader);
 
   _mesa_copy_linked_program_data(shProg, shader);
 
-- 
2.11.0

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

[Mesa-dev] [PATCH v4 06/10] mesa/glspirv: Add _mesa_spirv_link_shaders() function

[Alejandro Piñeiro]

From: Eduardo Lima Mitev 

This is the equivalent to link_shaders() from
src/compiler/glsl/linker.cpp, but for SPIR-V programs. It just
creates the program and its gl_linked_shader objects, giving drivers
the opportunity to implement any linking of SPIR-V shaders they choose,
at a later stage.

v2: Bail out if we see more that one shader for the same stage, and add
   a corresponding comment. (Timothy Arceri)

v3: * Adds also a linker error log to the condition above, with a reference
   to the specification issue. (Timothy Arceri)
* Squash with the patch adding the function boilerplate (Timothy Arceri)

Reviewed-by: Timothy Arceri 
---
 src/mesa/main/glspirv.c | 71 +
 src/mesa/main/glspirv.h |  4 +++
 2 files changed, 75 insertions(+)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index e8370e4c6f2..baed58380a8 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -28,6 +28,8 @@
 #include "compiler/nir/nir.h"
 #include "compiler/spirv/nir_spirv.h"
 
+#include "program/program.h"
+
 #include "util/u_atomic.h"
 
 void
@@ -103,6 +105,75 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
}
 }
 
+/**
+ * This is the equivalent to compiler/glsl/linker.cpp::link_shaders()
+ * but for SPIR-V programs.
+ *
+ * This method just creates the gl_linked_shader structs with a reference to
+ * the SPIR-V data collected during previous steps.
+ *
+ * The real linking happens later in the driver-specifc call LinkShader().
+ * This is so backends can implement different linking strategies for
+ * SPIR-V programs.
+ */
+void
+_mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program 
*prog)
+{
+   prog->data->LinkStatus = linking_success;
+   prog->data->Validated = false;
+
+   for (unsigned i = 0; i < prog->NumShaders; i++) {
+  struct gl_shader *shader = prog->Shaders[i];
+  gl_shader_stage shader_type = shader->Stage;
+
+  /* We only support one shader per stage. The gl_spirv spec doesn't seem
+   * to prevent this, but the way the API is designed, requiring all 
shaders
+   * to be specialized with an entry point, makes supporting this quite
+   * undefined.
+   *
+   * TODO: Turn this into a proper error once the spec bug
+   *  is resolved.
+   */
+  if (prog->_LinkedShaders[shader_type]) {
+ ralloc_strcat(>data->InfoLog,
+   "\nError trying to link more than one SPIR-V shader "
+   "per stage.\n");
+ prog->data->LinkStatus = linking_failure;
+ return;
+  }
+
+  assert(shader->spirv_data);
+
+  struct gl_linked_shader *linked = rzalloc(NULL, struct gl_linked_shader);
+  linked->Stage = shader_type;
+
+  /* Create program and attach it to the linked shader */
+  struct gl_program *gl_prog =
+ ctx->Driver.NewProgram(ctx,
+_mesa_shader_stage_to_program(shader_type),
+prog->Name, false);
+  if (!gl_prog) {
+ prog->data->LinkStatus = linking_failure;
+ _mesa_delete_linked_shader(ctx, linked);
+ return;
+  }
+
+  _mesa_reference_shader_program_data(ctx,
+  _prog->sh.data,
+  prog->data);
+
+  /* Don't use _mesa_reference_program() just take ownership */
+  linked->Program = gl_prog;
+
+  /* Reference the SPIR-V data from shader to the linked shader */
+  _mesa_shader_spirv_data_reference(>spirv_data,
+shader->spirv_data);
+
+  prog->_LinkedShaders[shader_type] = linked;
+  prog->data->linked_stages |= 1 << shader_type;
+   }
+}
+
 void GLAPIENTRY
 _mesa_SpecializeShaderARB(GLuint shader,
   const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index ba281f68bef..0f03b75c111 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -76,6 +76,10 @@ _mesa_spirv_shader_binary(struct gl_context *ctx,
   unsigned n, struct gl_shader **shaders,
   const void* binary, size_t length);
 
+void
+_mesa_spirv_link_shaders(struct gl_context *ctx,
+ struct gl_shader_program *prog);
+
 /**
  * \name API functions
  */
-- 
2.11.0

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

[Mesa-dev] [PATCH v4 08/10] mesa/glspirv: Add a _mesa_spirv_to_nir() function

[Alejandro Piñeiro]

From: Eduardo Lima Mitev 

This is basically a wrapper around spirv_to_nir() that includes
arguments setup and post-conversion validation.

v2: * Rebase update (SpirVCapabilities not a pointer anymore,
spirv_to_nir_options added, and others).
* Code-style improvements and remove debug hunk. (Timothy Arceri)

Reviewed-by: Timothy Arceri 
---
 src/mesa/main/glspirv.c | 58 +
 src/mesa/main/glspirv.h |  7 ++
 2 files changed, 65 insertions(+)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index baed58380a8..a5a2254bf9c 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -174,6 +174,64 @@ _mesa_spirv_link_shaders(struct gl_context *ctx, struct 
gl_shader_program *prog)
}
 }
 
+nir_shader *
+_mesa_spirv_to_nir(struct gl_context *ctx,
+   const struct gl_shader_program *prog,
+   gl_shader_stage stage,
+   const nir_shader_compiler_options *options)
+{
+   nir_shader *nir = NULL;
+
+   struct gl_linked_shader *linked_shader = prog->_LinkedShaders[stage];
+   assert (linked_shader);
+
+   struct gl_shader_spirv_data *spirv_data = linked_shader->spirv_data;
+   assert(spirv_data);
+
+   struct gl_spirv_module *spirv_module = spirv_data->SpirVModule;
+   assert (spirv_module != NULL);
+
+   const char *entry_point_name = spirv_data->SpirVEntryPoint;
+   assert(entry_point_name);
+
+   struct nir_spirv_specialization *spec_entries =
+  calloc(sizeof(*spec_entries),
+ spirv_data->NumSpecializationConstants);
+
+   for (unsigned i = 0; i < spirv_data->NumSpecializationConstants; ++i) {
+  spec_entries[i].id = spirv_data->SpecializationConstantsIndex[i];
+  spec_entries[i].data32 = spirv_data->SpecializationConstantsValue[i];
+  spec_entries[i].defined_on_module = false;
+   }
+
+   const struct spirv_to_nir_options spirv_options = {
+  .caps = ctx->Const.SpirVCapabilities
+   };
+
+   nir_function *entry_point =
+  spirv_to_nir((const uint32_t *) _module->Binary[0],
+   spirv_module->Length / 4,
+   spec_entries, spirv_data->NumSpecializationConstants,
+   stage, entry_point_name,
+   _options,
+   options);
+   free(spec_entries);
+
+   assert (entry_point);
+   nir = entry_point->shader;
+   assert(nir->info.stage == stage);
+
+   nir->options = options;
+
+   nir->info.name =
+  ralloc_asprintf(nir, "SPIRV:%s:%d",
+  _mesa_shader_stage_to_abbrev(nir->info.stage),
+  prog->Name);
+   nir_validate_shader(nir);
+
+   return nir;
+}
+
 void GLAPIENTRY
 _mesa_SpecializeShaderARB(GLuint shader,
   const GLchar *pEntryPoint,
diff --git a/src/mesa/main/glspirv.h b/src/mesa/main/glspirv.h
index 0f03b75c111..81626ce75b5 100644
--- a/src/mesa/main/glspirv.h
+++ b/src/mesa/main/glspirv.h
@@ -24,6 +24,7 @@
 #ifndef GLSPIRV_H
 #define GLSPIRV_H
 
+#include "compiler/nir/nir.h"
 #include "mtypes.h"
 
 #ifdef __cplusplus
@@ -80,6 +81,12 @@ void
 _mesa_spirv_link_shaders(struct gl_context *ctx,
  struct gl_shader_program *prog);
 
+nir_shader *
+_mesa_spirv_to_nir(struct gl_context *ctx,
+   const struct gl_shader_program *prog,
+   gl_shader_stage stage,
+   const nir_shader_compiler_options *options);
+
 /**
  * \name API functions
  */
-- 
2.11.0

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

[Mesa-dev] [PATCH v4 07/10] mesa/program: Link SPIR-V shaders using the SPIR-V code-path

[Alejandro Piñeiro]

From: Eduardo Lima Mitev 

---
 src/mesa/program/ir_to_mesa.cpp | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index 29198509a6c..5d56c2ef44a 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -36,6 +36,7 @@
 #include "main/shaderapi.h"
 #include "main/shaderobj.h"
 #include "main/uniforms.h"
+#include "main/glspirv.h"
 #include "compiler/glsl/ast.h"
 #include "compiler/glsl/ir.h"
 #include "compiler/glsl/ir_expression_flattening.h"
@@ -3112,7 +3113,10 @@ _mesa_glsl_link_shader(struct gl_context *ctx, struct 
gl_shader_program *prog)
}
 
if (prog->data->LinkStatus) {
-  link_shaders(ctx, prog);
+  if (!spirv)
+ link_shaders(ctx, prog);
+  else
+ _mesa_spirv_link_shaders(ctx, prog);
}
 
/* If LinkStatus is linking_success, then reset sampler validated to true.
-- 
2.11.0

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

[Mesa-dev] [PATCH v4 04/10] mesa: Implement glSpecializeShaderARB

[Alejandro Piñeiro]

From: Nicolai Hähnle 

v2: * Use gl_spirv_validation instead of spirv_to_nir.
   This method just validates the shader. The conversion to NIR will
   happen later, during linking. (Alejandro Piñeiro)

* Use gl_shader_spirv_data struct to store the SPIR-V data.
   (Eduardo Lima)

* Use the 'spirv_data' member to tell if the gl_shader is
   a SPIR-V shader, instead of a dedicated flag. (Timothy Arceri)
---
 src/mesa/main/glspirv.c | 107 +++-
 1 file changed, 105 insertions(+), 2 deletions(-)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 81303057d05..e8370e4c6f2 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -23,6 +23,11 @@
 
 #include "glspirv.h"
 #include "errors.h"
+#include "shaderobj.h"
+
+#include "compiler/nir/nir.h"
+#include "compiler/spirv/nir_spirv.h"
+
 #include "util/u_atomic.h"
 
 void
@@ -106,7 +111,105 @@ _mesa_SpecializeShaderARB(GLuint shader,
   const GLuint *pConstantValue)
 {
GET_CURRENT_CONTEXT(ctx);
+   struct gl_shader *sh;
+   bool has_entry_point;
+   struct nir_spirv_specialization *spec_entries = NULL;
+
+   if (!ctx->Extensions.ARB_gl_spirv) {
+  _mesa_error(ctx, GL_INVALID_OPERATION, "glSpecializeShaderARB");
+  return;
+   }
+
+   sh = _mesa_lookup_shader_err(ctx, shader, "glSpecializeShaderARB");
+   if (!sh)
+  return;
+
+   if (!sh->spirv_data) {
+  _mesa_error(ctx, GL_INVALID_OPERATION,
+  "glSpecializeShaderARB(not SPIR-V)");
+  return;
+   }
+
+   if (sh->CompileStatus) {
+  _mesa_error(ctx, GL_INVALID_OPERATION,
+  "glSpecializeShaderARB(already specialized)");
+  return;
+   }
+
+   struct gl_shader_spirv_data *spirv_data = sh->spirv_data;
+
+   /* From the GL_ARB_gl_spirv spec:
+*
+*"The OpenGL API expects the SPIR-V module to have already been
+* validated, and can return an error if it discovers anything invalid
+* in the module. An invalid SPIR-V module is allowed to result in
+* undefined behavior."
+*
+* However, the following errors still need to be detected (from the same
+* spec):
+*
+*"INVALID_VALUE is generated if  does not name a valid
+* entry point for .
+*
+* INVALID_VALUE is generated if any element of 
+* refers to a specialization constant that does not exist in the
+* shader module contained in ."
+*
+* We cannot flag those errors a-priori because detecting them requires
+* parsing the module. However, flagging them during specialization is okay,
+* since it makes no difference in terms of application-visible state.
+*/
+   spec_entries = calloc(sizeof(*spec_entries), numSpecializationConstants);
+
+   for (unsigned i = 0; i < numSpecializationConstants; ++i) {
+  spec_entries[i].id = pConstantIndex[i];
+  spec_entries[i].data32 = pConstantValue[i];
+  spec_entries[i].defined_on_module = false;
+   }
+
+   has_entry_point =
+  gl_spirv_validation((uint32_t *)_data->SpirVModule->Binary[0],
+  spirv_data->SpirVModule->Length / 4,
+  spec_entries, numSpecializationConstants,
+  sh->Stage, pEntryPoint);
+
+   /* See previous spec comment */
+   if (!has_entry_point) {
+  _mesa_error(ctx, GL_INVALID_VALUE,
+  "glSpecializeShaderARB(\"%s\" is not a valid entry point"
+  " for shader)", pEntryPoint);
+  goto end;
+   }
+
+   for (unsigned i = 0; i < numSpecializationConstants; ++i) {
+  if (spec_entries[i].defined_on_module == false) {
+ _mesa_error(ctx, GL_INVALID_VALUE,
+ "glSpecializeShaderARB(constant \"%i\" does not exist "
+ "in shader)", spec_entries[i].id);
+ goto end;
+  }
+   }
+
+   spirv_data->SpirVEntryPoint = ralloc_strdup(spirv_data, pEntryPoint);
+
+   /* Note that we didn't make a real compilation of the module (spirv_to_nir),
+* but just checked some error conditions. Real "compilation" will be done
+* later, upon linking.
+*/
+   sh->CompileStatus = compile_success;
+
+   spirv_data->NumSpecializationConstants = numSpecializationConstants;
+   spirv_data->SpecializationConstantsIndex =
+  rzalloc_array_size(spirv_data, sizeof(GLuint),
+ numSpecializationConstants);
+   spirv_data->SpecializationConstantsValue =
+  rzalloc_array_size(spirv_data, sizeof(GLuint),
+ numSpecializationConstants);
+   for (unsigned i = 0; i < numSpecializationConstants; ++i) {
+  spirv_data->SpecializationConstantsIndex[i] = pConstantIndex[i];
+  spirv_data->SpecializationConstantsValue[i] = pConstantValue[i];
+   }
 
-   /* Just return GL_INVALID_OPERATION error while this is boilerplate */
-   _mesa_error(ctx, GL_INVALID_OPERATION, "SpecializeShaderARB");
+ end:
+   

[Mesa-dev] [PATCH v4 09/10] i965: Call spirv_to_nir() instead of glsl_to_nir() for SPIR-V shaders

[Alejandro Piñeiro]

From: Eduardo Lima Mitev 

This is the main fork of the shader compilation code-path, where a NIR
shader is obtained by calling spirv_to_nir() or glsl_to_nir(),
depending on its nature..

v2: Use 'spirv_data' member from gl_linked_shader to know which method
   to call. (Timothy Arceri)

Reviewed-by: Timothy Arceri 
---
 src/mesa/drivers/dri/i965/brw_program.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_program.c 
b/src/mesa/drivers/dri/i965/brw_program.c
index 7fae22c6207..7e54bab0522 100644
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -31,6 +31,7 @@
 
 #include 
 #include "main/imports.h"
+#include "main/glspirv.h"
 #include "program/prog_parameter.h"
 #include "program/prog_print.h"
 #include "program/prog_to_nir.h"
@@ -74,9 +75,14 @@ brw_create_nir(struct brw_context *brw,
   ctx->Const.ShaderCompilerOptions[stage].NirOptions;
nir_shader *nir;
 
-   /* First, lower the GLSL IR or Mesa IR to NIR */
+   /* First, lower the GLSL/Mesa IR or SPIR-V to NIR */
if (shader_prog) {
-  nir = glsl_to_nir(shader_prog, stage, options);
+  if (shader_prog->_LinkedShaders[stage]->spirv_data)
+ nir = _mesa_spirv_to_nir(ctx, shader_prog, stage, options);
+  else
+ nir = glsl_to_nir(shader_prog, stage, options);
+  assert (nir);
+
   nir_remove_dead_variables(nir, nir_var_shader_in | nir_var_shader_out);
   nir_lower_returns(nir);
   nir_validate_shader(nir);
-- 
2.11.0

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

[Mesa-dev] [PATCH v4 05/10] mesa: Add a reference to gl_shader_spirv_data to gl_linked_shader

[Alejandro Piñeiro]

From: Eduardo Lima Mitev 

This is a reference to the spirv_data object stored in gl_shader, which
stores shader SPIR-V data that is needed during linking too.

Reviewed-by: Timothy Arceri 
---
 src/mesa/main/mtypes.h| 8 
 src/mesa/main/shaderobj.c | 1 +
 2 files changed, 9 insertions(+)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 75878049c1e..dda7cd29df8 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2545,6 +2545,14 @@ struct gl_linked_shader
struct exec_list *packed_varyings;
struct exec_list *fragdata_arrays;
struct glsl_symbol_table *symbols;
+
+   /**
+* ARB_gl_spirv related data.
+*
+* This is actually a reference to the gl_shader::spirv_data, which
+* stores information that is also needed during linking.
+*/
+   struct gl_shader_spirv_data *spirv_data;
 };
 
 
diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
index 5c1cdd6b27a..834e2a92ec4 100644
--- a/src/mesa/main/shaderobj.c
+++ b/src/mesa/main/shaderobj.c
@@ -137,6 +137,7 @@ void
 _mesa_delete_linked_shader(struct gl_context *ctx,
struct gl_linked_shader *sh)
 {
+   _mesa_shader_spirv_data_reference(>spirv_data, NULL);
_mesa_reference_program(ctx, >Program, NULL);
ralloc_free(sh);
 }
-- 
2.11.0

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

[Mesa-dev] [PATCH v4 02/10] i965: initialize SPIR-V capabilities

[Alejandro Piñeiro]

Needed for ARB_gl_spirv. Right now those are the same that the intel
vulkan driver, but those are not shared. From the ARB_spirv_extensions
spec:

   "3. If a new GL extension is added that includes SPIR-V support via
   a new SPIR-V extension does it's SPIR-V extension also get
   enumerated by the SPIR_V_EXTENSIONS_ARB query?.

   RESOLVED. Yes. It's good to include it for consistency. Any SPIR-V
   functionality supported beyond the SPIR-V version that is required
   for the GL API version should be enumerated."

Reading between lines, there is the possibility of specific GL
extensions enabling specific SPIR-V extensions (so capabilities). That
would mean that it is possible that OpenGL and Vulkan not having the
same capabilities supported, even for the same driver. So for now we
keep them separate. Perhaps in the future it is better to keep them
the same and synced.

Note: we initialize SPIR-V capabilities at brwCreateContext instead of
the usual brw_initialize_context_constants because we want to do that
only for version >= 3.3. At brw_initialize_context_constans GL version
is still not computed.

v2:
   * Rebase update (SpirVCapabilities not a pointer anymore)
   * Fill spirv capabilities for OpenGL >= 3.3 (Ian Romanick)
---
 src/mesa/drivers/dri/i965/brw_context.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
b/src/mesa/drivers/dri/i965/brw_context.c
index f765cff76b9..8bcd04ffd93 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -75,6 +75,7 @@
 #include "util/debug.h"
 #include "isl/isl.h"
 
+#include "compiler/spirv/nir_spirv.h"
 /***
  * Mesa's Driver Functions
  ***/
@@ -339,6 +340,21 @@ brw_init_driver_functions(struct brw_context *brw,
 }
 
 static void
+brw_initialize_spirv_supported_capabilities(struct brw_context *brw)
+{
+   const struct gen_device_info *devinfo = >screen->devinfo;
+   struct gl_context *ctx = >ctx;
+
+   ctx->Const.SpirVCapabilities.float64 = devinfo->gen >= 8;
+   ctx->Const.SpirVCapabilities.int64 = devinfo->gen >= 8;
+   ctx->Const.SpirVCapabilities.tessellation = true;
+   ctx->Const.SpirVCapabilities.draw_parameters = true;
+   ctx->Const.SpirVCapabilities.image_write_without_format = true;
+   ctx->Const.SpirVCapabilities.multiview = true;
+   ctx->Const.SpirVCapabilities.variable_pointers = true;
+}
+
+static void
 brw_initialize_context_constants(struct brw_context *brw)
 {
const struct gen_device_info *devinfo = >screen->devinfo;
@@ -1072,6 +1088,10 @@ brwCreateContext(gl_api api,
_mesa_override_extensions(ctx);
_mesa_compute_version(ctx);
 
+   /* GL_ARB_gl_spirv */
+   if (ctx->Version >= 33)
+  brw_initialize_spirv_supported_capabilities(brw);
+
_mesa_initialize_dispatch_tables(ctx);
_mesa_initialize_vbo_vtxfmt(ctx);
 
-- 
2.11.0

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

[Mesa-dev] [PATCH v4 01/10] mesa: add gl_constants::SpirVCapabilities

[Alejandro Piñeiro]

From: Nicolai Hähnle 

For drivers to declare which SPIR-V features they support.

v2: Don't use a pointer (Ian Romanick)
---
 src/mesa/main/mtypes.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 226eb94da91..75878049c1e 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4039,6 +4039,9 @@ struct gl_constants
 
/** GL_ARB_get_program_binary */
GLuint NumProgramBinaryFormats;
+
+   /** GL_ARB_gl_spirv */
+   struct spirv_supported_capabilities SpirVCapabilities;
 };
 
 
-- 
2.11.0

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

[Mesa-dev] [PATCH v4 03/10] nir/spirv: add gl_spirv_validation method

[Alejandro Piñeiro]

ARB_gl_spirv adds the ability to use SPIR-V binaries, and a new
method, glSpecializeShader. From OpenGL 4.6 spec, section 7.2.1
"Shader Specialization", error table:

   INVALID_VALUE is generated if  does not name a valid
   entry point for .

   INVALID_VALUE is generated if any element of 
   refers to a specialization constant that does not exist in the
   shader module contained in .""

But we are not really interested on creating the nir shader at that
point, and adding nir structures on the gl_program, so at that point
we are just interested on the error checking.

So we add a new method focused on just checking those errors. It still
needs to parse the binary, but skips what it is not needed, and
doesn't create the nir shader.

v2: rebase update (spirv_to_nir options added, changes on the warning
logging, and others)
v3: include passing options on common initialization, doesn't call
setjmp on common_initialization
---
 src/compiler/spirv/nir_spirv.h|   5 +
 src/compiler/spirv/spirv_to_nir.c | 191 ++
 2 files changed, 180 insertions(+), 16 deletions(-)

diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_spirv.h
index a2c40e57d18..d2766abb7f9 100644
--- a/src/compiler/spirv/nir_spirv.h
+++ b/src/compiler/spirv/nir_spirv.h
@@ -41,6 +41,7 @@ struct nir_spirv_specialization {
   uint32_t data32;
   uint64_t data64;
};
+   bool defined_on_module;
 };
 
 enum nir_spirv_debug_level {
@@ -69,6 +70,10 @@ struct spirv_to_nir_options {
} debug;
 };
 
+bool gl_spirv_validation(const uint32_t *words, size_t word_count,
+ struct nir_spirv_specialization *spec, unsigned 
num_spec,
+ gl_shader_stage stage, const char *entry_point_name);
+
 nir_function *spirv_to_nir(const uint32_t *words, size_t word_count,
struct nir_spirv_specialization *specializations,
unsigned num_specializations,
diff --git a/src/compiler/spirv/spirv_to_nir.c 
b/src/compiler/spirv/spirv_to_nir.c
index c6df764682e..2143cd9df31 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -1332,6 +1332,7 @@ spec_constant_decoration_cb(struct vtn_builder *b, struct 
vtn_value *v,
 const_value->data64 = b->specializations[i].data64;
  else
 const_value->data32 = b->specializations[i].data32;
+ b->specializations[i].defined_on_module = true;
  return;
   }
}
@@ -1366,7 +1367,13 @@ handle_workgroup_size_decoration_cb(struct vtn_builder 
*b,
 const struct vtn_decoration *dec,
 void *data)
 {
+   /* This can happens if we are gl_spirv_validation. We can return safely, as
+* we don't need the workgroup info for such validation. */
+   if (b->shader == NULL)
+  return;
+
vtn_assert(member == -1);
+
if (dec->decoration != SpvDecorationBuiltIn ||
dec->literals[0] != SpvBuiltInWorkgroupSize)
   return;
@@ -3263,6 +3270,49 @@ vtn_handle_preamble_instruction(struct vtn_builder *b, 
SpvOp opcode,
return true;
 }
 
+/*
+ * gl_spirv validation. Just need to check for the entry point.
+ */
+static bool
+vtn_validate_preamble_instruction(struct vtn_builder *b, SpvOp opcode,
+  const uint32_t *w, unsigned count)
+{
+   switch (opcode) {
+   /* The following opcodes are not needed for gl_spirv, so we can skip
+* them.
+*/
+   case SpvOpSource:
+   case SpvOpSourceExtension:
+   case SpvOpSourceContinued:
+   case SpvOpExtension:
+   case SpvOpCapability:
+   case SpvOpExtInstImport:
+   case SpvOpMemoryModel:
+   case SpvOpString:
+   case SpvOpName:
+   case SpvOpMemberName:
+   case SpvOpExecutionMode:
+   case SpvOpDecorationGroup:
+   case SpvOpMemberDecorate:
+   case SpvOpGroupDecorate:
+   case SpvOpGroupMemberDecorate:
+  break;
+
+   case SpvOpEntryPoint:
+  vtn_handle_preamble_instruction(b, opcode, w, count);
+  break;
+
+   case SpvOpDecorate:
+  vtn_handle_decoration(b, opcode, w, count);
+  break;
+
+   default:
+  return false; /* End of preamble */
+   }
+
+   return true;
+}
+
 static void
 vtn_handle_execution_mode(struct vtn_builder *b, struct vtn_value *entry_point,
   const struct vtn_decoration *mode, void *data)
@@ -3473,6 +3523,22 @@ vtn_handle_variable_or_type_instruction(struct 
vtn_builder *b, SpvOp opcode,
 }
 
 static bool
+vtn_handle_constant_or_type_instruction(struct vtn_builder *b, SpvOp opcode,
+const uint32_t *w, unsigned count)
+{
+   switch (opcode) {
+   case SpvOpUndef:
+   case SpvOpVariable:
+  break;
+
+   default:
+  return vtn_handle_variable_or_type_instruction(b, opcode, w, count);
+   }
+
+   return true;
+}
+
+static bool
 vtn_handle_body_instruction(struct vtn_builder *b, SpvOp opcode,
 

[Mesa-dev] [PATCH v4 00/10] Initial gl_spirv support in Mesa and i965

[Alejandro Piñeiro]

Hi,

This is the 4rd version of the series adding initial support for
ARB_gl_spirv. This series is equal to v3 we sent one month ago [1],
but rebased against today master.

As with v3, we are splitting the series, so after this series we will
send one for ARB_spirv_extensions.

Notice also that some patches from version 2 were merged in
master. These were already reviewed favorably and were fairly
independent from the rest of the series.

There are still 5 patches in this new series with a Reviewed-by tag
that we didn't merge yet because we consider they should go in with
the rest of the series. The patches missing review are 01, 02, 03, 04
and 07.

As usual, a git tree containing this series can be found at
 and the
larger, work-in-progress, often force-pushed series at
.

Thanks for reviewing

[1] https://lists.freedesktop.org/archives/mesa-dev/2017-December/180039.html


Alejandro Piñeiro (2):
  i965: initialize SPIR-V capabilities
  nir/spirv: add gl_spirv_validation method

Eduardo Lima Mitev (6):
  mesa: Add a reference to gl_shader_spirv_data to gl_linked_shader
  mesa/glspirv: Add _mesa_spirv_link_shaders() function
  mesa/program: Link SPIR-V shaders using the SPIR-V code-path
  mesa/glspirv: Add a _mesa_spirv_to_nir() function
  i965: Call spirv_to_nir() instead of glsl_to_nir() for SPIR-V shaders
  i965: Don't call process_glsl_ir() for SPIR-V shaders

Nicolai Hähnle (2):
  mesa: add gl_constants::SpirVCapabilities
  mesa: Implement glSpecializeShaderARB

 src/compiler/spirv/nir_spirv.h  |   5 +
 src/compiler/spirv/spirv_to_nir.c   | 191 +++---
 src/mesa/drivers/dri/i965/brw_context.c |  20 +++
 src/mesa/drivers/dri/i965/brw_link.cpp  |   3 +-
 src/mesa/drivers/dri/i965/brw_program.c |  10 +-
 src/mesa/main/glspirv.c | 236 +++-
 src/mesa/main/glspirv.h |  11 ++
 src/mesa/main/mtypes.h  |  11 ++
 src/mesa/main/shaderobj.c   |   1 +
 src/mesa/program/ir_to_mesa.cpp |   6 +-
 10 files changed, 472 insertions(+), 22 deletions(-)

-- 
2.11.0

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

Re: [Mesa-dev] [PATCHv2] glx: fix non-dri build

[Nicolai Hähnle]

On 15.01.2018 15:38, Samuel Thibault wrote:

glXGetDriverConfig parameters do not provide a context to dynamically
check for the presence of the function, so the dispatcher directly calls
glXGetDriverConfig, but in non-dri builds dri_glx.c didn't provide
glXGetDriverConfig.

This change make it just return NULL in that case.

Fixes: 84f764a7591 "glxglvnddispatch: Add missing dispatch for GetDriverConfig

---
Difference between v1 and v2: just modify the call in
dispatch_GetDriverConfig rather than adding glXGetDriverConfig and
always adding dri_glx to build system.


Thanks! We usually add the version difference as part of the commit message.

Reviewed-by: Nicolai Hähnle 

You may want to wait a while though to see if somebody else speaks up.

Cheers,
Nicolai



---
  src/glx/g_glxglvnddispatchfuncs.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/src/glx/g_glxglvnddispatchfuncs.c 
b/src/glx/g_glxglvnddispatchfuncs.c
index 56d894eda..5b65afc86 100644
--- a/src/glx/g_glxglvnddispatchfuncs.c
+++ b/src/glx/g_glxglvnddispatchfuncs.c
@@ -338,11 +338,15 @@ static Display *dispatch_GetCurrentDisplayEXT(void)
  
  static const char *dispatch_GetDriverConfig(const char *driverName)

  {
+#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
  /*
   * The options are constant for a given driverName, so we do not need
   * a context (and apps expect to be able to call this without one).
   */
  return glXGetDriverConfig(driverName);
+#else
+return NULL;
+#endif
  }
  
  



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

Re: [Mesa-dev] [PATCH] glx: fix non-dri build

[Nicolai Hähnle]

On 15.01.2018 15:15, Samuel Thibault wrote:

Nicolai Hähnle, on lun. 15 janv. 2018 15:07:03 +0100, wrote:

On 13.01.2018 12:36, Samuel Thibault wrote:

glXGetDriverConfig parameters do not provide a context to dynamically
check for the presence of the function, so the dispatcher directly calls
glXGetDriverConfig, but in non-dri builds dri_glx.c didn't provide
glXGetDriverConfig.

This change makes it provide a NULL-returning stub in non-dri builds.

Fixes: 84f764a7591 "glxglvnddispatch: Add missing dispatch for GetDriverConfig"


Would it be possible to instead modify dispatch_GetDriverConfig with an:

#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
...
#else
return NULL;
#endif


Sure!  There is just one thing: src/glx/g_glxglvnddispatchfuncs.c reads

  * THIS FILE IS AUTOMATICALLY GENERATED BY gen_scrn_dispatch.pl
  * DO NOT EDIT!!

I didn't find that script...


Good point.

Then again, that file isn't actually generated as part of the build (and 
as you write, that Perl script is missing), and the problem you're 
trying to fix originates in a manual edit of that file.


So I vote to fix this by changing the original manual edit, though I'd 
welcome other suggestions, especially ones that fix the confusion around 
whether that file is automatically generated...


Cheers,
Nicolai




Samuel



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

Re: [Mesa-dev] [PATCH] i965/miptree: Refactor CCS_E and CCS_D cases in render_aux_usage

[Pohjolainen, Topi]

On Sun, Dec 17, 2017 at 08:03:45PM -0800, Jason Ekstrand wrote:
> This commit unifies the CCS_E and CCS_D cases.  This should fix a couple
> of subtle issues.  One is that when you use INTEL_DEBUG=norbc to disable
> CCS_E, we don't get the sRGB blending workaround.  By unifying the code,
> we give CCS_D that workaround as well.
> 
> The second issue fixed by this refactor is that the blending workaround
> was appears to be enabled on all gens but really only applies on gen9.

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

Re: [Mesa-dev] [PATCH] glx: fix non-dri build

[Nicolai Hähnle]

On 13.01.2018 12:36, Samuel Thibault wrote:

glXGetDriverConfig parameters do not provide a context to dynamically
check for the presence of the function, so the dispatcher directly calls
glXGetDriverConfig, but in non-dri builds dri_glx.c didn't provide
glXGetDriverConfig.

This change makes it provide a NULL-returning stub in non-dri builds.

Fixes: 84f764a7591 "glxglvnddispatch: Add missing dispatch for GetDriverConfig"


Would it be possible to instead modify dispatch_GetDriverConfig with an:

#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
   ...
#else
   return NULL;
#endif

That seems a lighter-weight approach to fixing the build error.

Cheers,
Nicolai




---
Compiling dri_glx.c in non-dri builds might be frowned upon. I'll be
happy to move the glXGetDriverConfig to another file if somebody tells
me which file would be the proper place.
---
  src/glx/Makefile.am |  2 +-
  src/glx/dri_glx.c   | 11 ++-
  src/glx/glxclient.h |  4 ++--
  3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/glx/Makefile.am b/src/glx/Makefile.am
index 5448a0907..0ae403403 100644
--- a/src/glx/Makefile.am
+++ b/src/glx/Makefile.am
@@ -59,6 +59,7 @@ libglx_la_SOURCES = \
clientinfo.c \
compsize.c \
create_context.c \
+   dri_glx.c \
eval.c \
glxclient.h \
glxcmds.c \
@@ -123,7 +124,6 @@ libglx_la_SOURCES += \
dri2_glx.c \
dri2.h \
dri2_priv.h \
-   dri_glx.c \
dri_sarea.h \
XF86dri.c \
xf86dri.h \
diff --git a/src/glx/dri_glx.c b/src/glx/dri_glx.c
index 5c4346cec..893cb4acd 100644
--- a/src/glx/dri_glx.c
+++ b/src/glx/dri_glx.c
@@ -32,12 +32,13 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
   *
   */
  
+#include "glxclient.h"

+
  #if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
  
  #include 

  #include 
  #include 
-#include "glxclient.h"
  #include "xf86dri.h"
  #include "dri2.h"
  #include "dri_sarea.h"
@@ -1020,4 +1021,12 @@ driCreateDisplay(Display * dpy)
 return >base;
  }
  
+#else /* GLX_DIRECT_RENDERING */

+
+_GLX_PUBLIC const char *
+glXGetDriverConfig(const char *driverName)
+{
+  return NULL;
+}
+
  #endif /* GLX_DIRECT_RENDERING */
diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h
index f3a36cf10..a1925a5fe 100644
--- a/src/glx/glxclient.h
+++ b/src/glx/glxclient.h
@@ -169,10 +169,10 @@ extern unsigned dri2GetSwapEventType(Display *dpy, XID 
drawable);
  */
  extern const char *glXGetScreenDriver(Display * dpy, int scrNum);
  
-extern const char *glXGetDriverConfig(const char *driverName);

-
  #endif
  
+extern const char *glXGetDriverConfig(const char *driverName);

+
  //
  
  #define __GL_CLIENT_ATTRIB_STACK_DEPTH 16




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

[Mesa-dev] [PATCH 3/3] ac: set no-signed-zeros-fp-math when RADV_DEBUG="unsafemath" is used

[Samuel Pitoiset]

This is an optimisation that is recommended by Matt Arsenault,
and used by RadeonSI, but it's not compatible with Vulkan.

Note that AC_FLOAT_MODE_UNSAFE_FP_MATH includes the no signed
zeros flag in LLVM.

v2: - enable nsz with unsafe math

Signed-off-by: Samuel Pitoiset 
---
 src/amd/common/ac_nir_to_llvm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 46bb4decab..6e95a6a84e 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -352,6 +352,9 @@ create_llvm_function(LLVMContextRef ctx, LLVMModuleRef 
module,
LLVMAddTargetDependentFunctionAttr(main_function,
   "unsafe-fp-math",
   "true");
+   LLVMAddTargetDependentFunctionAttr(main_function,
+  "no-signed-zeros-fp-math",
+  "true");
}
return main_function;
 }
-- 
2.15.1

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

[Mesa-dev] [PATCH 2/3] ac: set fast math flags when RADV_DEBUG="unsafemath" is used

[Samuel Pitoiset]

When that debug option is not used, we use the default float mode
because the no signed zeros optimisation is not Vulkan compatible.

Signed-off-by: Samuel Pitoiset 
---
 src/amd/common/ac_nir_to_llvm.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 2034039543..46bb4decab 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -6681,7 +6681,11 @@ LLVMModuleRef 
ac_translate_nir_to_llvm(LLVMTargetMachineRef tm,
LLVMDisposeTargetData(data_layout);
LLVMDisposeMessage(data_layout_str);
 
-   ctx.builder = LLVMCreateBuilderInContext(ctx.context);
+   enum ac_float_mode float_mode =
+   options->unsafe_math ? AC_FLOAT_MODE_UNSAFE_FP_MATH :
+  AC_FLOAT_MODE_DEFAULT;
+
+   ctx.builder = ac_create_builder(ctx.context, float_mode);
ctx.ac.builder = ctx.builder;
 
memset(shader_info, 0, sizeof(*shader_info));
@@ -7093,7 +7097,11 @@ void ac_create_gs_copy_shader(LLVMTargetMachineRef tm,
ctx.is_gs_copy_shader = true;
LLVMSetTarget(ctx.module, "amdgcn--");
 
-   ctx.builder = LLVMCreateBuilderInContext(ctx.context);
+   enum ac_float_mode float_mode =
+   options->unsafe_math ? AC_FLOAT_MODE_UNSAFE_FP_MATH :
+  AC_FLOAT_MODE_DEFAULT;
+
+   ctx.builder = ac_create_builder(ctx.context, float_mode);
ctx.ac.builder = ctx.builder;
ctx.stage = MESA_SHADER_VERTEX;
 
-- 
2.15.1

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

[Mesa-dev] [PATCH 1/3] ac: import lp_create_builder() from gallivm

[Samuel Pitoiset]

Signed-off-by: Samuel Pitoiset 
---
 src/amd/common/ac_llvm_helper.cpp  | 30 ++
 src/amd/common/ac_llvm_util.h  |  9 +++
 src/gallium/auxiliary/gallivm/lp_bld_misc.cpp  | 29 -
 src/gallium/auxiliary/gallivm/lp_bld_misc.h|  9 ---
 .../drivers/radeonsi/si_shader_tgsi_setup.c|  8 +++---
 5 files changed, 43 insertions(+), 42 deletions(-)

diff --git a/src/amd/common/ac_llvm_helper.cpp 
b/src/amd/common/ac_llvm_helper.cpp
index 4db703622c..e42d00280b 100644
--- a/src/amd/common/ac_llvm_helper.cpp
+++ b/src/amd/common/ac_llvm_helper.cpp
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #if HAVE_LLVM < 0x0500
 namespace llvm {
@@ -80,3 +81,32 @@ bool ac_llvm_is_function(LLVMValueRef v)
return llvm::isa(llvm::unwrap(v));
 #endif
 }
+
+LLVMBuilderRef ac_create_builder(LLVMContextRef ctx,
+enum ac_float_mode float_mode)
+{
+   LLVMBuilderRef builder = LLVMCreateBuilderInContext(ctx);
+
+#if HAVE_LLVM >= 0x0308
+   llvm::FastMathFlags flags;
+
+   switch (float_mode) {
+   case AC_FLOAT_MODE_DEFAULT:
+   break;
+   case AC_FLOAT_MODE_NO_SIGNED_ZEROS_FP_MATH:
+   flags.setNoSignedZeros();
+   llvm::unwrap(builder)->setFastMathFlags(flags);
+   break;
+   case AC_FLOAT_MODE_UNSAFE_FP_MATH:
+#if HAVE_LLVM >= 0x0600
+   flags.setFast();
+#else
+   flags.setUnsafeAlgebra();
+#endif
+   llvm::unwrap(builder)->setFastMathFlags(flags);
+   break;
+   }
+#endif
+
+   return builder;
+}
diff --git a/src/amd/common/ac_llvm_util.h b/src/amd/common/ac_llvm_util.h
index 61bcc4e54e..84fcbf111c 100644
--- a/src/amd/common/ac_llvm_util.h
+++ b/src/amd/common/ac_llvm_util.h
@@ -62,6 +62,12 @@ enum ac_target_machine_options {
AC_TM_PROMOTE_ALLOCA_TO_SCRATCH = (1 << 4),
 };
 
+enum ac_float_mode {
+   AC_FLOAT_MODE_DEFAULT,
+   AC_FLOAT_MODE_NO_SIGNED_ZEROS_FP_MATH,
+   AC_FLOAT_MODE_UNSAFE_FP_MATH,
+};
+
 const char *ac_get_llvm_processor_name(enum radeon_family family);
 LLVMTargetMachineRef ac_create_target_machine(enum radeon_family family, enum 
ac_target_machine_options tm_options);
 
@@ -77,6 +83,9 @@ void ac_dump_module(LLVMModuleRef module);
 LLVMValueRef ac_llvm_get_called_value(LLVMValueRef call);
 bool ac_llvm_is_function(LLVMValueRef v);
 
+LLVMBuilderRef ac_create_builder(LLVMContextRef ctx,
+enum ac_float_mode float_mode);
+
 void
 ac_llvm_add_target_dep_function_attr(LLVMValueRef F,
 const char *name, int value);
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp 
b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
index 1319407290..79dbedbb56 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
+++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
@@ -813,32 +813,3 @@ lp_is_function(LLVMValueRef v)
return llvm::isa(llvm::unwrap(v));
 #endif
 }
-
-extern "C" LLVMBuilderRef
-lp_create_builder(LLVMContextRef ctx, enum lp_float_mode float_mode)
-{
-   LLVMBuilderRef builder = LLVMCreateBuilderInContext(ctx);
-
-#if HAVE_LLVM >= 0x0308
-   llvm::FastMathFlags flags;
-
-   switch (float_mode) {
-   case LP_FLOAT_MODE_DEFAULT:
-  break;
-   case LP_FLOAT_MODE_NO_SIGNED_ZEROS_FP_MATH:
-  flags.setNoSignedZeros();
-  llvm::unwrap(builder)->setFastMathFlags(flags);
-  break;
-   case LP_FLOAT_MODE_UNSAFE_FP_MATH:
-#if HAVE_LLVM >= 0x0600
-  flags.setFast();
-#else
-  flags.setUnsafeAlgebra();
-#endif
-  llvm::unwrap(builder)->setFastMathFlags(flags);
-  break;
-   }
-#endif
-
-   return builder;
-}
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.h 
b/src/gallium/auxiliary/gallivm/lp_bld_misc.h
index 1b725d10d7..ca5ba5c44f 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_misc.h
+++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.h
@@ -76,15 +76,6 @@ lp_get_called_value(LLVMValueRef call);
 extern bool
 lp_is_function(LLVMValueRef v);
 
-enum lp_float_mode {
-   LP_FLOAT_MODE_DEFAULT,
-   LP_FLOAT_MODE_NO_SIGNED_ZEROS_FP_MATH,
-   LP_FLOAT_MODE_UNSAFE_FP_MATH,
-};
-
-extern LLVMBuilderRef
-lp_create_builder(LLVMContextRef ctx, enum lp_float_mode float_mode);
-
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c 
b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
index 2ca036e67d..fc141ca1e0 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
@@ -1160,11 +1160,11 @@ void si_llvm_context_init(struct si_shader_context *ctx,
LLVMDisposeMessage(data_layout_str);
 
bool unsafe_fpmath = (sscreen->debug_flags & DBG(UNSAFE_MATH)) != 0;
-   enum lp_float_mode float_mode =
-   unsafe_fpmath ? LP_FLOAT_MODE_UNSAFE_FP_MATH :
- 

Re: [Mesa-dev] [PATCH 2/4] i965/miptree: Use the tiling from the modifier instead of the BO

[Pohjolainen, Topi]

On Thu, Jan 11, 2018 at 05:40:51PM -0800, Jason Ekstrand wrote:
> From: Jason Ekstrand 
> 
> This fixes a bug where we were taking the tiling from the BO regardless
> of what the modifier said.  When we got images in from Vulkan where it
> doesn't set the tiling on the BO, we would treat them as linear even
> though the modifier expressly said to treat it as Y-tiled.

I noticed that I didn't get the tiling from Vulkan when I played with
ext_memory_object. Hence I only ran my new piglit test with linear tiling.
I was about to ask how do we pass the tiling from Vulkan to GL?


Anyway here patches 1 and 2 are:

Reviewed-by: Topi Pohjolainen 

> 
> Cc: mesa-sta...@lists.freedesktop.org
> Reviewed-by: Daniel Stone 
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index a0474ef..a9c2810 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -986,7 +986,11 @@ intel_miptree_create_for_dri_image(struct brw_context 
> *brw,
> uint32_t bo_tiling, bo_swizzle;
> brw_bo_get_tiling(image->bo, _tiling, _swizzle);
>  
> -   const enum isl_tiling tiling = isl_tiling_from_i915_tiling(bo_tiling);
> +   const struct isl_drm_modifier_info *mod_info =
> +  isl_drm_modifier_get_info(image->modifier);
> +
> +   const enum isl_tiling tiling =
> +  mod_info ? mod_info->tiling : isl_tiling_from_i915_tiling(bo_tiling);
>  
> if (image->planar_format && image->planar_format->nplanes > 1)
>return miptree_create_for_planar_image(brw, image, target, tiling);
> @@ -1010,9 +1014,6 @@ intel_miptree_create_for_dri_image(struct brw_context 
> *brw,
> if (!brw->ctx.TextureFormatSupported[format])
>return NULL;
>  
> -   const struct isl_drm_modifier_info *mod_info =
> -  isl_drm_modifier_get_info(image->modifier);
> -
> enum intel_miptree_create_flags mt_create_flags = 0;
>  
> /* If this image comes in from a window system, we have different
> -- 
> 2.5.0.400.gff86faf
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 06/29] anv/image: Add a helper for determining when fast clears are supported

[Pohjolainen, Topi]

On Sat, Jan 13, 2018 at 11:11:35AM -0800, Jason Ekstrand wrote:
> Sorry for all the list spam, but I'm sort of thinking out-loud and writing
> it on the list for all to read.
> 
> I'm thinking that what we want this list to return is not a bool but an enum
> 
> /* The ordering of this enum is important */
> enum anv_fast_clear_support {
>ANV_FAST_CLEAR_NONE = 0,
>ANV_FAST_CLEAR_ZERO_ONLY = 1,
>ANV_FAST_CLEAR_ANY = 2,
> };
> 
> And then the predicate for whether or not to do the resolve becomes
> (has_compression && !supports_compression) || (image_fast_clear >
> supported_fast_clear).  I still haven't quite figured out what to do with
> MI_PREDICATE to make this work out.  I'm sure it's possible with MI math
> but maybe I can come up with something clever that doesn't require that.
> In the worst case, we only have to deal with this complexity on gen9+ where
> we have MI_MATH so it'll be ok.
> 
> Does this seem like a reasonable plan?

Sounds good to me at least.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC libdrm 0/5] Move alloc_handle_t from gralloc impls.

[Robert Foss]

Hey,

On 01/13/2018 12:49 AM, Gurchetan Singh wrote:

We can define accessor functions too (not ptrs), then the struct is opaque
and you can do your own accessor implementation if aligning is not possible
or desired.


Accessor functions in libdrm sound good to me.


Alright, this seems straight forward enough. As for the accessor 
implementations, does anyone mind if I start out with support for multiple 
planes even if the buffer handle currently doesn't contain multi plane support

in various fields (fds, strides, offsets, etc.).


Rob.



On Fri, Jan 12, 2018 at 11:44 AM, Rob Herring > wrote:


On Fri, Jan 12, 2018 at 2:29 AM, Tomasz Figa > wrote:
 > Hi Rob,
 >
 > On Fri, Jan 12, 2018 at 5:26 AM, Robert Foss > wrote:
 >> Heya,
 >>
 >>
 >> On 12/22/17 1:09 PM, Tomasz Figa wrote:
 >>>
 >>> On Fri, Dec 22, 2017 at 10:09 AM, Gurchetan Singh
 >>> > 
wrote:
 
  So the plan is for alloc_handle_t to not be sub-classed by the
  implementations, but have all necessary information that an
  implementation
  would need?
 
  If so, how do we reconcile the implementation specific information 
that
  is
  often in the handle:
 
 
 

https://github.com/intel/minigbm/blob/master/cros_gralloc/cros_gralloc_handle.h


  [consumer_usage, producer_usage, yuv_color_range, is_updated etc.]
 
 
 

https://chromium.googlesource.com/chromiumos/platform/minigbm/+/master/cros_gralloc/cros_gralloc_handle.h


  [use_flags, pixel_stride]
 
  In our case, removing our minigbm specific use flags from the handle
  would
  add complexity to our (*registerBuffer) path.
 
  On Thu, Dec 21, 2017 at 10:14 AM, Rob Herring > wrote:
 >
 >
 > On Wed, Dec 13, 2017 at 5:02 PM, Gurchetan Singh
 > > 
wrote:
 >>
 >> Hi Robert,
 >>
 >> Thanks for looking into this!  We need to decide if we want:
 >>
 >> (1) A common struct that implementations can subclass, i.e:
 >>
 >> struct blah_gralloc_handle {
 >>      alloc_handle_t alloc_handle;
 >>      int x, y, z;
 >>      
 >> }
 >>
 >> (2) An accessor library that vendors can implement, i.e:
 >>
 >> struct drmAndroidHandleInfo {
 >>     uint32_t (*get_fourcc)(buffer_handle_t handle);
 >>     uint32_t (*get_stride)(buffer_handle_t handle, uint32_t plane);
 >>     uint32_t (*get_offsets)(buffer_handle_t handle, uint32_t plane);
 >>     uint64_t (*get_modifier)(buffer_handle_t handle);
 >> };
 >>
 >>  From my perspective as someone who has to maintain the minigbm 
gralloc
 >> implementation, (2) is preferable since:
 >
 >
 > Yeah, I'd prefer not to encourage 1 as the default.
 >
 >>
 >> So I had a look into implementing this,
 >
 > Thanks!
 >
 >> and using function pointers is
 >> problematic due to this struct being passed between processes which 
would
 >> prevent mesa calling a function assigned in gbm_gralloc for example.
 >
 > Why would be this struct passed between processes?
 >
 > The only data being exchanged between processes using gralloc is the
 > handle and it isn't actually exchanged directly, but the exporting
 > process will flatten it and send through Binder, while the importing
 > one will have it unflattened and then the gralloc implementation
 > called on it (the register buffer operation), which could update any
 > per-process data in the handle. (But still, why would we need to
 > include the function pointers there?)
 >
 >>
 >> It could be used to provide runtime support for multiple gralloc
 >> implementations, but that seems to about the only advantage to adding 
FPs.
 >>
 >> Am I missing a good usecase for FPs? If not I think the added
 >> complexity/cruft is more problematic than good.
 >>
 >> Any thoughts?
 >>
 >
 > I guess it might not be a big deal whether FPs or structs are used, as
 > long as they are not directly embedded in the handle, which we don't
 > want constraints on.

Why no constraints? Is converging on a common handle 

[Mesa-dev] [PATCH 2/7] egl: add support for EGL_ANDROID_blob_cache

[Tapani Pälli]

Signed-off-by: Tapani Pälli 
---
 src/egl/drivers/dri2/egl_dri2.c | 43 +
 src/egl/drivers/dri2/egl_dri2.h |  4 
 src/egl/main/eglapi.c   | 29 +++
 src/egl/main/eglapi.h   |  4 
 src/egl/main/egldisplay.h   |  3 +++
 src/egl/main/eglentrypoint.h|  1 +
 6 files changed, 84 insertions(+)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index d5a4f72e86..f9d0223fe2 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -458,6 +458,7 @@ static const struct dri2_extension_match 
optional_core_extensions[] = {
{ __DRI2_INTEROP, 1, offsetof(struct dri2_egl_display, interop) },
{ __DRI_IMAGE, 1, offsetof(struct dri2_egl_display, image) },
{ __DRI2_FLUSH_CONTROL, 1, offsetof(struct dri2_egl_display, flush_control) 
},
+   { __DRI2_BLOB, 1, offsetof(struct dri2_egl_display, blob) },
{ NULL, 0, 0 }
 };
 
@@ -727,6 +728,9 @@ dri2_setup_screen(_EGLDisplay *disp)
   }
}
 
+   if (dri2_dpy->blob)
+  disp->Extensions.ANDROID_blob_cache = EGL_TRUE;
+
disp->Extensions.KHR_reusable_sync = EGL_TRUE;
 
if (dri2_dpy->image) {
@@ -1470,6 +1474,26 @@ dri2_surf_update_fence_fd(_EGLContext *ctx,
dri2_surface_set_out_fence_fd(surf, fence_fd);
 }
 
+static void
+update_blob_cache_functions(struct dri2_egl_display *dri2_dpy,
+struct dri2_egl_context *dri2_ctx)
+{
+   if (!dri2_dpy || !dri2_ctx)
+  return;
+
+   /* No blob support. */
+   if (!dri2_dpy->blob)
+  return;
+
+   /* No functions to set. */
+   if (!dri2_dpy->blob_cache_set)
+  return;
+
+   dri2_dpy->blob->set_cache_funcs(dri2_ctx->dri_context,
+   dri2_dpy->blob_cache_set,
+   dri2_dpy->blob_cache_get);
+}
+
 /**
  * Called via eglMakeCurrent(), drv->API.MakeCurrent().
  */
@@ -1499,6 +1523,9 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, 
_EGLSurface *dsurf,
if (old_ctx)
   dri2_gl_flush();
 
+   /* Make sure cache functions are set for new context. */
+   update_blob_cache_functions(dri2_dpy, dri2_ctx);
+
ddraw = (dsurf) ? dri2_dpy->vtbl->get_dri_drawable(dsurf) : NULL;
rdraw = (rsurf) ? dri2_dpy->vtbl->get_dri_drawable(rsurf) : NULL;
cctx = (dri2_ctx) ? dri2_ctx->dri_context : NULL;
@@ -3016,6 +3043,21 @@ dri2_dup_native_fence_fd(_EGLDriver *drv, _EGLDisplay 
*dpy, _EGLSync *sync)
return dup(sync->SyncFd);
 }
 
+static void
+dri2_set_blob_cache_funcs(_EGLDriver *drv, _EGLDisplay *dpy,
+  EGLSetBlobFuncANDROID set,
+  EGLGetBlobFuncANDROID get)
+{
+   _EGLContext *ctx = _eglGetCurrentContext();
+   struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
+   struct dri2_egl_context *dri2_ctx = dri2_egl_context(ctx);
+
+   dri2_dpy->blob_cache_set = set;
+   dri2_dpy->blob_cache_get = get;
+
+   update_blob_cache_functions(dri2_dpy, dri2_ctx);
+}
+
 static EGLint
 dri2_client_wait_sync(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSync *sync,
   EGLint flags, EGLTime timeout)
@@ -3234,6 +3276,7 @@ _eglBuiltInDriver(void)
dri2_drv->API.GLInteropQueryDeviceInfo = dri2_interop_query_device_info;
dri2_drv->API.GLInteropExportObject = dri2_interop_export_object;
dri2_drv->API.DupNativeFenceFDANDROID = dri2_dup_native_fence_fd;
+   dri2_drv->API.SetBlobCacheFuncsANDROID = dri2_set_blob_cache_funcs;
 
dri2_drv->Name = "DRI2";
 
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index cc76c73eab..a6777ad3f1 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -171,6 +171,7 @@ struct dri2_egl_display
const __DRInoErrorExtension*no_error;
const __DRI2configQueryExtension *config;
const __DRI2fenceExtension *fence;
+   const __DRI2blobExtension *blob;
const __DRI2rendererQueryExtension *rendererQuery;
const __DRI2interopExtension *interop;
int   fd;
@@ -230,6 +231,9 @@ struct dri2_egl_display
 
bool  is_render_node;
bool  is_different_gpu;
+
+   EGLSetBlobFuncANDROID blob_cache_set;
+   EGLGetBlobFuncANDROID blob_cache_get;
 };
 
 struct dri2_egl_context
diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 5110688f2d..b8d64a913c 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -476,6 +476,7 @@ _eglCreateExtensionsString(_EGLDisplay *dpy)
char *exts = dpy->ExtensionsString;
 
/* Please keep these sorted alphabetically. */
+   _EGL_CHECK_EXTENSION(ANDROID_blob_cache);
_EGL_CHECK_EXTENSION(ANDROID_framebuffer_target);
_EGL_CHECK_EXTENSION(ANDROID_image_native_buffer);
_EGL_CHECK_EXTENSION(ANDROID_native_fence_sync);
@@ -2522,6 +2523,34 @@ eglQueryDmaBufModifiersEXT(EGLDisplay dpy, EGLint 
format, EGLint max_modifiers,
RETURN_EGL_EVAL(disp, 

[Mesa-dev] [PATCH 5/7] i965: add __DRI2_BLOB support and set cache functions

[Tapani Pälli]

Signed-off-by: Tapani Pälli 
---
 src/mesa/drivers/dri/i965/intel_screen.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
b/src/mesa/drivers/dri/i965/intel_screen.c
index 08032c9b22..4c19304f14 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -36,6 +36,7 @@
 #include "main/version.h"
 #include "swrast/s_renderbuffer.h"
 #include "util/ralloc.h"
+#include "util/disk_cache.h"
 #include "brw_defines.h"
 #include "brw_state.h"
 #include "compiler/nir/nir.h"
@@ -1484,6 +1485,19 @@ brw_query_renderer_string(__DRIscreen *dri_screen,
return -1;
 }
 
+static void
+brw_set_cache_funcs(__DRIcontext *dri_ctx,
+__DRIblobCacheSet set, __DRIblobCacheGet get)
+{
+   struct brw_context *brw = dri_ctx->driverPrivate;
+   struct gl_context *ctx = >ctx;
+
+   if (!ctx->Cache)
+  return;
+
+   disk_cache_set_callbacks(ctx->Cache, set, get);
+}
+
 static const __DRI2rendererQueryExtension intelRendererQueryExtension = {
.base = { __DRI2_RENDERER_QUERY, 1 },
 
@@ -1495,6 +1509,11 @@ static const __DRIrobustnessExtension dri2Robustness = {
.base = { __DRI2_ROBUSTNESS, 1 }
 };
 
+static const __DRI2blobExtension intelBlobExtension = {
+   .base = { __DRI2_BLOB, 1 },
+   .set_cache_funcs = brw_set_cache_funcs
+};
+
 static const __DRIextension *screenExtensions[] = {
 ,
 ,
@@ -1504,6 +1523,7 @@ static const __DRIextension *screenExtensions[] = {
 ,
 ,
 ,
+,
 NULL
 };
 
@@ -1517,6 +1537,7 @@ static const __DRIextension 
*intelRobustScreenExtensions[] = {
 ,
 ,
 ,
+,
 NULL
 };
 
-- 
2.14.3

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

[Mesa-dev] [PATCH 6/7] android: ignore MESA_GLSL_CACHE_DISABLE setting

[Tapani Pälli]

Signed-off-by: Tapani Pälli 
---
 src/mesa/drivers/dri/i965/brw_disk_cache.c | 2 ++
 src/util/disk_cache.c  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_disk_cache.c 
b/src/mesa/drivers/dri/i965/brw_disk_cache.c
index 65bb52726e..4df4504666 100644
--- a/src/mesa/drivers/dri/i965/brw_disk_cache.c
+++ b/src/mesa/drivers/dri/i965/brw_disk_cache.c
@@ -405,8 +405,10 @@ void
 brw_disk_cache_init(struct brw_context *brw)
 {
 #ifdef ENABLE_SHADER_CACHE
+#ifndef ANDROID
if (env_var_as_boolean("MESA_GLSL_CACHE_DISABLE", true))
   return;
+#endif
 
char renderer[10];
MAYBE_UNUSED int len = snprintf(renderer, sizeof(renderer), "i965_%04x",
diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
index d7891e3b70..3c98089e69 100644
--- a/src/util/disk_cache.c
+++ b/src/util/disk_cache.c
@@ -208,9 +208,11 @@ disk_cache_create(const char *gpu_name, const char 
*timestamp,
if (local == NULL)
   goto fail;
 
+#ifndef ANDROID
/* At user request, disable shader cache entirely. */
if (env_var_as_boolean("MESA_GLSL_CACHE_DISABLE", false))
   goto fail;
+#endif
 
/* Determine path for cache based on the first defined name as follows:
 *
-- 
2.14.3

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

[Mesa-dev] [PATCH 4/7] disk cache: support setting MESA_GLSL_CACHE_DIR at compile time

[Tapani Pälli]

Signed-off-by: Tapani Pälli 
---
 src/util/disk_cache.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
index b71363bcf3..d7891e3b70 100644
--- a/src/util/disk_cache.c
+++ b/src/util/disk_cache.c
@@ -47,6 +47,7 @@
 #include "util/mesa-sha1.h"
 #include "util/ralloc.h"
 #include "main/errors.h"
+#include "main/macros.h"
 
 #include "disk_cache.h"
 
@@ -213,7 +214,8 @@ disk_cache_create(const char *gpu_name, const char 
*timestamp,
 
/* Determine path for cache based on the first defined name as follows:
 *
-*   $MESA_GLSL_CACHE_DIR
+*   $MESA_GLSL_CACHE_DIR as environment variable
+*   $MESA_GLSL_CACHE_DIR as compile time option
 *   $XDG_CACHE_HOME/mesa_shader_cache
 *   /.cache/mesa_shader_cache
 */
@@ -227,6 +229,15 @@ disk_cache_create(const char *gpu_name, const char 
*timestamp,
  goto fail;
}
 
+#ifdef MESA_GLSL_CACHE_DIR
+#define STR(x) STRINGIFY(x)
+   path = concatenate_and_mkdir(local, STR(MESA_GLSL_CACHE_DIR),
+CACHE_DIR_NAME);
+   if (path == NULL)
+  goto fail;
+#undef STR
+#endif
+
if (path == NULL) {
   char *xdg_cache_home = getenv("XDG_CACHE_HOME");
 
-- 
2.14.3

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

[Mesa-dev] [PATCH 3/7] disk cache: add callback functionality

[Tapani Pälli]

Signed-off-by: Tapani Pälli 
---
 src/util/disk_cache.c | 39 +++
 src/util/disk_cache.h | 19 +++
 2 files changed, 58 insertions(+)

diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
index 7ebfa8c045..b71363bcf3 100644
--- a/src/util/disk_cache.c
+++ b/src/util/disk_cache.c
@@ -99,6 +99,9 @@ struct disk_cache {
/* Driver cache keys. */
uint8_t *driver_keys_blob;
size_t driver_keys_blob_size;
+
+   disk_cache_set_cb blob_set_cb;
+   disk_cache_get_cb blob_get_cb;
 };
 
 struct disk_cache_put_job {
@@ -995,6 +998,11 @@ disk_cache_put(struct disk_cache *cache, const cache_key 
key,
const void *data, size_t size,
struct cache_item_metadata *cache_item_metadata)
 {
+   if (cache->blob_set_cb) {
+  cache->blob_set_cb(key, CACHE_KEY_SIZE, data, size);
+  return;
+   }
+
struct disk_cache_put_job *dc_job =
   create_put_job(cache, key, data, size, cache_item_metadata);
 
@@ -1057,6 +1065,29 @@ disk_cache_get(struct disk_cache *cache, const cache_key 
key, size_t *size)
if (size)
   *size = 0;
 
+   if (cache->blob_get_cb) {
+/* This is what Android EGL defines as the maxValueSize in egl_cache_t
+ * class implementation.
+ */
+#define MAX_BLOB_SIZE 64 * 1024
+  void *blob = malloc(MAX_BLOB_SIZE);
+  if (!blob)
+ return NULL;
+
+  signed long bytes =
+ cache->blob_get_cb(key, CACHE_KEY_SIZE, blob, MAX_BLOB_SIZE);
+
+  if (!bytes) {
+ free(blob);
+ return NULL;
+  }
+
+  if (size)
+ *size = bytes;
+  return blob;
+#undef MAX_BLOB_SIZE
+   }
+
filename = get_cache_file(cache, key);
if (filename == NULL)
   goto fail;
@@ -1209,4 +1240,12 @@ disk_cache_compute_key(struct disk_cache *cache, const 
void *data, size_t size,
_mesa_sha1_final(, key);
 }
 
+void
+disk_cache_set_callbacks(struct disk_cache *cache, disk_cache_set_cb set,
+ disk_cache_get_cb get)
+{
+   cache->blob_set_cb = set;
+   cache->blob_get_cb = get;
+}
+
 #endif /* ENABLE_SHADER_CACHE */
diff --git a/src/util/disk_cache.h b/src/util/disk_cache.h
index 488b297ead..3fae8a1358 100644
--- a/src/util/disk_cache.h
+++ b/src/util/disk_cache.h
@@ -50,6 +50,14 @@ typedef uint8_t cache_key[CACHE_KEY_SIZE];
 #define CACHE_ITEM_TYPE_UNKNOWN  0x0
 #define CACHE_ITEM_TYPE_GLSL 0x1
 
+typedef void
+(*disk_cache_set_cb) (const void *key, signed long keySize,
+  const void *value, signed long valueSize);
+
+typedef signed long
+(*disk_cache_get_cb) (const void *key, signed long keySize,
+  void *value, signed long valueSize);
+
 struct cache_item_metadata {
/**
 * The cache item type. This could be used to identify a GLSL cache item,
@@ -207,6 +215,10 @@ void
 disk_cache_compute_key(struct disk_cache *cache, const void *data, size_t size,
cache_key key);
 
+void
+disk_cache_set_callbacks(struct disk_cache *cache, disk_cache_set_cb set,
+ disk_cache_get_cb get);
+
 #else
 
 static inline struct disk_cache *
@@ -260,6 +272,13 @@ disk_cache_compute_key(struct disk_cache *cache, const 
void *data, size_t size,
return;
 }
 
+static inline void
+disk_cache_set_callbacks(struct disk_cache *cache, disk_cache_set_cb set,
+ disk_cache_get_cb get)
+{
+   return;
+}
+
 #endif /* ENABLE_SHADER_CACHE */
 
 #ifdef __cplusplus
-- 
2.14.3

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

[Mesa-dev] [PATCH 0/7] EGL_ANDROID_blob_cache

[Tapani Pälli]

Hello;

Here's a refactored series of EGL_ANDROID_blob_cache. Now cache 
functions are stored in disk_cache struct and the functionality 
called within existing disk_cache put/get code. Problems/errors 
that existed with earlier series are gone.

On Android cache index file is created to MESA_GLSL_CACHE_DIR 
and blobs are  generated under '/data/user_de/0' in application 
specific paths:

androidia_64:/ # find /data/user_de/0/ -name *shader*
/data/user_de/0/com.android.settings/code_cache/com.android.opengl.shaders_cache
/data/user_de/0/com.android.gallery3d/code_cache/com.android.opengl.shaders_cache
/data/user_de/0/com.android.systemui/code_cache/com.android.opengl.shaders_cache
/data/user_de/0/com.rovio.angrybirdsspace.ads/code_cache/com.android.opengl.shaders_cache

(this part is managed by Android but may be interesting to know).

Also SurfaceFlinger manages its own cache as seen in the log output:
01-15 07:40:26.329  2129  2129 D SurfaceFlinger: shader cache generated - 24 
shaders in 57.687504 ms

I'm not sure if /sdcard is sane default but I've tried everything 
else (/cache, /data/cache) and failed because of permission errors.

Thanks;

Tapani Pälli (7):
  dri: add interface for EGL_ANDROID_blob_cache extension
  egl: add support for EGL_ANDROID_blob_cache
  disk cache: add callback functionality
  disk cache: support setting MESA_GLSL_CACHE_DIR at compile time
  i965: add __DRI2_BLOB support and set cache functions
  android: ignore MESA_GLSL_CACHE_DISABLE setting
  android: set '/sdcard/' as MESA_GLSL_CACHE_DIR by default

 Android.common.mk  |  1 +
 include/GL/internal/dri_interface.h| 26 +-
 src/egl/drivers/dri2/egl_dri2.c| 43 
 src/egl/drivers/dri2/egl_dri2.h|  4 +++
 src/egl/main/eglapi.c  | 29 
 src/egl/main/eglapi.h  |  4 +++
 src/egl/main/egldisplay.h  |  3 ++
 src/egl/main/eglentrypoint.h   |  1 +
 src/mesa/drivers/dri/i965/brw_disk_cache.c |  2 ++
 src/mesa/drivers/dri/i965/intel_screen.c   | 21 
 src/util/disk_cache.c  | 54 +-
 src/util/disk_cache.h  | 19 +++
 12 files changed, 205 insertions(+), 2 deletions(-)

-- 
2.14.3

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

[Mesa-dev] [PATCH 7/7] android: set '/sdcard/' as MESA_GLSL_CACHE_DIR by default

[Tapani Pälli]

This can/should be modified depending on needs. AFAIK by default,
this is the only path that can be read/written to by anyone.

Signed-off-by: Tapani Pälli 
---
 Android.common.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Android.common.mk b/Android.common.mk
index 52dc7bff3b..7edbbfc0f2 100644
--- a/Android.common.mk
+++ b/Android.common.mk
@@ -47,6 +47,7 @@ LOCAL_CFLAGS += \
 LOCAL_CFLAGS += \
-DANDROID_API_LEVEL=$(PLATFORM_SDK_VERSION) \
-DENABLE_SHADER_CACHE \
+   -DMESA_GLSL_CACHE_DIR="/sdcard" \
-D__STDC_CONSTANT_MACROS \
-D__STDC_LIMIT_MACROS \
-DHAVE___BUILTIN_EXPECT \
-- 
2.14.3

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

[Mesa-dev] [PATCH 1/7] dri: add interface for EGL_ANDROID_blob_cache extension

[Tapani Pälli]

Signed-off-by: Tapani Pälli 
---
 include/GL/internal/dri_interface.h | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/include/GL/internal/dri_interface.h 
b/include/GL/internal/dri_interface.h
index 34a5c9fb01..de367d8f77 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -82,7 +82,7 @@ typedef struct __DRI2flushExtensionRec
__DRI2flushExtension;
 typedef struct __DRI2throttleExtensionRec  __DRI2throttleExtension;
 typedef struct __DRI2fenceExtensionRec  __DRI2fenceExtension;
 typedef struct __DRI2interopExtensionRec   __DRI2interopExtension;
-
+typedef struct __DRI2blobExtensionRec   __DRI2blobExtension;
 
 typedef struct __DRIimageLoaderExtensionRec __DRIimageLoaderExtension;
 typedef struct __DRIimageDriverExtensionRec __DRIimageDriverExtension;
@@ -336,6 +336,30 @@ struct __DRI2throttleExtensionRec {
enum __DRI2throttleReason reason);
 };
 
+/**
+ * Extension for EGL_ANDROID_blob_cache
+ */
+
+#define __DRI2_BLOB "DRI2_Blob"
+#define __DRI2_BLOB_VERSION 1
+
+typedef void
+(*__DRIblobCacheSet) (const void *key, signed long keySize,
+  const void *value, signed long valueSize);
+
+typedef signed long
+(*__DRIblobCacheGet) (const void *key, signed long keySize,
+  void *value, signed long valueSize);
+
+struct __DRI2blobExtensionRec {
+   __DRIextension base;
+
+   /**
+* Set cache functions for setting and getting cache entries.
+*/
+   void (*set_cache_funcs) (__DRIcontext *ctx,
+__DRIblobCacheSet set, __DRIblobCacheGet get);
+};
 
 /**
  * Extension for fences / synchronization objects.
-- 
2.14.3

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

Re: [Mesa-dev] [PATCH] u_thread: Use pthread_setname_np on linux only

[Jose Fonseca]

On 13/01/18 11:33, Samuel Thibault wrote:

pthread_setname_np was added in glibc 2.12 for the Linux port only, other
ports do not necessarily have it.
---
  src/util/u_thread.h | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/util/u_thread.h b/src/util/u_thread.h
index 26cc0b093..8c6e0bdc5 100644
--- a/src/util/u_thread.h
+++ b/src/util/u_thread.h
@@ -62,7 +62,8 @@ static inline void u_thread_setname( const char *name )
  {
  #if defined(HAVE_PTHREAD)
  #  if defined(__GNU_LIBRARY__) && defined(__GLIBC__) && defined(__GLIBC_MINOR__) 
&& \
-  (__GLIBC__ >= 3 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 12))
+  (__GLIBC__ >= 3 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 12)) && \
+  defined(__linux__)
 pthread_setname_np(pthread_self(), name);
  #  endif
  #endif



Looks good to me.

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

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

[Eric Engestrom]

On Sunday, 2018-01-14 23:59:48 +0200, Grazvydas Ignotas wrote:
> Trivial. Found by Coccinelle.

Series is:
Reviewed-by: Eric Engestrom 

> ---
>  src/mesa/drivers/osmesa/osmesa.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/osmesa/osmesa.c 
> b/src/mesa/drivers/osmesa/osmesa.c
> index 1df3da4..e0f87b8 100644
> --- a/src/mesa/drivers/osmesa/osmesa.c
> +++ b/src/mesa/drivers/osmesa/osmesa.c
> @@ -212,11 +212,10 @@ osmesa_choose_line_function( struct gl_context *ctx )
> */
>return NULL;
> }
>  
> if (ctx->RenderMode != GL_RENDER ||
> -   ctx->Line.SmoothFlag ||
> ctx->Texture._MaxEnabledTexImageUnit == -1 ||
> ctx->Light.ShadeModel != GL_FLAT ||
> ctx->Line.Width != 1.0F ||
> ctx->Line.StippleFlag ||
> ctx->Line.SmoothFlag) {
> -- 
> 2.7.4
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v3 2/2] anv: return VK_ERROR_OUT_OF_DEVICE_MEMORY when surface size is out of HW limits

[Samuel Iglesias Gonsálvez]

Signed-off-by: Samuel Iglesias Gonsálvez 
---
 src/intel/vulkan/anv_image.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
index 4d13e05e11f..401de16ddc5 100644
--- a/src/intel/vulkan/anv_image.c
+++ b/src/intel/vulkan/anv_image.c
@@ -340,6 +340,9 @@ make_surface(const struct anv_device *dev,
 */
assert(ok);
 
+   if (anv_surf->isl.size == UINT64_MAX)
+  return VK_ERROR_OUT_OF_DEVICE_MEMORY;
+
image->planes[plane].aux_usage = ISL_AUX_USAGE_NONE;
 
add_surface(image, anv_surf, plane);
-- 
2.14.1

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

[Mesa-dev] [PATCH v3 1/2] isl: don't crash when creating a huge image

[Samuel Iglesias Gonsálvez]

The HW has some limits but, according to the spec, we can create
the image as it has not yet any memory backing it. This patch
logs a debug error and set the size to the UINT64_MAX in order to
avoid allocating actual memory later.

Fixes the crashes on BDW for the following tests:

dEQP-VK.pipeline.render_to_image.core.2d_array.huge.*
dEQP-VK.pipeline.render_to_image.core.cube_array.huge.*

Signed-off-by: Samuel Iglesias Gonsálvez 
---
 src/intel/isl/isl.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
index 59f512fc050..cd7f2fcd4cb 100644
--- a/src/intel/isl/isl.c
+++ b/src/intel/isl/isl.c
@@ -26,6 +26,7 @@
 #include 
 
 #include "genxml/genX_bits.h"
+#include "common/intel_log.h"
 
 #include "isl.h"
 #include "isl_gen4.h"
@@ -1481,8 +1482,10 @@ isl_surf_init_s(const struct isl_device *dev,
*
* This comment is applicable to all Pre-gen9 platforms.
*/
-  if (size > (uint64_t) 1 << 31)
- return false;
+  if (size > (uint64_t) 1 << 31) {
+ intel_logd("%s: Surface size is bigger than the supported by the HW: 
%ld > (1 << 31)", __func__, size);
+ size = UINT64_MAX;
+  }
} else {
   /* From the Skylake PRM Vol 5, Maximum Surface Size in Bytes:
*"In addition to restrictions on maximum height, width, and depth,
@@ -1490,8 +1493,10 @@ isl_surf_init_s(const struct isl_device *dev,
* All pixels within the surface must be contained within 2^38 bytes
* of the base address."
*/
-  if (size > (uint64_t) 1 << 38)
- return false;
+  if (size > (uint64_t) 1 << 38) {
+ intel_logd("%s: Surface size is bigger than the supported by the HW: 
%ld > (1 << 38)", __func__, size);
+ size = UINT64_MAX;
+  }
}
 
*surf = (struct isl_surf) {
-- 
2.14.1

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

Re: [Mesa-dev] [PATCH] glsl/linker: link-error using the same name in unnamed block and outside

[Tapani Pälli]

On 01/15/2018 12:56 PM, Juan A. Suarez Romero wrote:

On Thu, 2018-01-11 at 16:51 +0100, Juan A. Suarez Romero wrote:

On Wed, 2018-01-10 at 14:04 +0200, Tapani Pälli wrote:


On 01/08/2018 03:19 PM, Juan A. Suarez Romero wrote:

Please, could someone take a look at this patch? Thanks in advance.



This looks correct to me. Is there some dEQP/Piglit test for this situation?



Yes. It fixes KHR-GL*.shaders.uniform_block.common.name_matching.

I'll include the tests in the commit message.



Tapani, with the previous comment about adding the fixed test in the
commit message, is this Rb ?


Yes, r-b!


Thanks!



J.A.




J.A.

On Mon, 2017-12-04 at 17:35 +0100, Juan A. Suarez Romero wrote:

According with OpenGL GLSL 4.20 spec, section 4.3.9, page 57:

 "It is a link-time error if any particular shader interface
  contains:
- two different blocks, each having no instance name, and each
  having a member of the same name, or
- a variable outside a block, and a block with no instance name,
  where the variable has the same name as a member in the block."

This means that it is a link error if for example we have a vertex
shader with the following definition.

"layout(location=0) uniform Data { float a; float b; };"

and a fragment shader with:

"uniform float a;"

As in both cases we refer to both uniforms as "a", and thus using
glGetUniformLocation() wouldn't know which one we mean.
---
   src/compiler/glsl/linker.cpp | 23 +++
   1 file changed, 23 insertions(+)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 33fd76deae9..b6de7b54ae3 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -,6 +,29 @@ cross_validate_globals(struct gl_shader_program *prog,
   return;
}
   
+ /* In OpenGL GLSL 4.20 spec, section 4.3.9, page 57:

+  *
+  *   "It is a link-time error if any particular shader interface
+  *contains:
+  *
+  *- two different blocks, each having no instance name, and each
+  *  having a member of the same name, or
+  *
+  *- a variable outside a block, and a block with no instance name,
+  *  where the variable has the same name as a member in the 
block."
+  */
+ if (var->data.mode == existing->data.mode &&
+ var->get_interface_type() != existing->get_interface_type()) {
+linker_error(prog, "declarations for %s `%s` are in "
+ "%s and %s\n",
+ mode_string(var), var->name,
+ existing->get_interface_type() ?
+   existing->get_interface_type()->name : "outside a 
block",
+ var->get_interface_type() ?
+   var->get_interface_type()->name : "outside a 
block");
+
+return;
+ }
/* Only in GLSL ES 3.10, the precision qualifier should not match
 * between block members defined in matched block names within a
 * shader interface.


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






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

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

[Mesa-dev] [Bug 104625] semicolon after if

[bugzilla-daemon]

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

--- Comment #1 from Thomas Hellström  ---
Thanks. This looks like a leftover debug hack.

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

[Mesa-dev] [Bug 104329] Vulkan app crashes GPU

[bugzilla-daemon]

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

neel84...@gmail.com changed:

   What|Removed |Added

 CC||neel84...@gmail.com

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

Re: [Mesa-dev] [PATCH] glsl/linker: link-error using the same name in unnamed block and outside

[Juan A. Suarez Romero]

On Thu, 2018-01-11 at 16:51 +0100, Juan A. Suarez Romero wrote:
> On Wed, 2018-01-10 at 14:04 +0200, Tapani Pälli wrote:
> > 
> > On 01/08/2018 03:19 PM, Juan A. Suarez Romero wrote:
> > > Please, could someone take a look at this patch? Thanks in advance.
> > > 
> > 
> > This looks correct to me. Is there some dEQP/Piglit test for this situation?
> 
> 
> Yes. It fixes KHR-GL*.shaders.uniform_block.common.name_matching.
> 
> I'll include the tests in the commit message.
> 

Tapani, with the previous comment about adding the fixed test in the
commit message, is this Rb ?

Thanks!


>   J.A.
> 
> > 
> > >   J.A.
> > > 
> > > On Mon, 2017-12-04 at 17:35 +0100, Juan A. Suarez Romero wrote:
> > > > According with OpenGL GLSL 4.20 spec, section 4.3.9, page 57:
> > > > 
> > > > "It is a link-time error if any particular shader interface
> > > >  contains:
> > > >- two different blocks, each having no instance name, and each
> > > >  having a member of the same name, or
> > > >- a variable outside a block, and a block with no instance name,
> > > >  where the variable has the same name as a member in the block."
> > > > 
> > > > This means that it is a link error if for example we have a vertex
> > > > shader with the following definition.
> > > > 
> > > >"layout(location=0) uniform Data { float a; float b; };"
> > > > 
> > > > and a fragment shader with:
> > > > 
> > > >"uniform float a;"
> > > > 
> > > > As in both cases we refer to both uniforms as "a", and thus using
> > > > glGetUniformLocation() wouldn't know which one we mean.
> > > > ---
> > > >   src/compiler/glsl/linker.cpp | 23 +++
> > > >   1 file changed, 23 insertions(+)
> > > > 
> > > > diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> > > > index 33fd76deae9..b6de7b54ae3 100644
> > > > --- a/src/compiler/glsl/linker.cpp
> > > > +++ b/src/compiler/glsl/linker.cpp
> > > > @@ -,6 +,29 @@ cross_validate_globals(struct gl_shader_program 
> > > > *prog,
> > > >   return;
> > > >}
> > > >   
> > > > + /* In OpenGL GLSL 4.20 spec, section 4.3.9, page 57:
> > > > +  *
> > > > +  *   "It is a link-time error if any particular shader 
> > > > interface
> > > > +  *contains:
> > > > +  *
> > > > +  *- two different blocks, each having no instance name, 
> > > > and each
> > > > +  *  having a member of the same name, or
> > > > +  *
> > > > +  *- a variable outside a block, and a block with no 
> > > > instance name,
> > > > +  *  where the variable has the same name as a member in 
> > > > the block."
> > > > +  */
> > > > + if (var->data.mode == existing->data.mode &&
> > > > + var->get_interface_type() != 
> > > > existing->get_interface_type()) {
> > > > +linker_error(prog, "declarations for %s `%s` are in "
> > > > + "%s and %s\n",
> > > > + mode_string(var), var->name,
> > > > + existing->get_interface_type() ?
> > > > +   existing->get_interface_type()->name : 
> > > > "outside a block",
> > > > + var->get_interface_type() ?
> > > > +   var->get_interface_type()->name : "outside 
> > > > a block");
> > > > +
> > > > +return;
> > > > + }
> > > >/* Only in GLSL ES 3.10, the precision qualifier should not 
> > > > match
> > > > * between block members defined in matched block names 
> > > > within a
> > > > * shader interface.
> > > 
> > > ___
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > 
> > 
> > 
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 1/4] anv/query: implement multiview interactions

[Iago Toral]

This is still awaiting review. Any takers?

On Mon, 2018-01-08 at 13:57 +0100, Iago Toral Quiroga wrote:
> From the Vulkan spec with KHX extensions:
> 
>   "If queries are used while executing a render pass instance that
> has
>multiview enabled, the query uses N consecutive query indices
>in the query pool (starting at query) where N is the number of
> bits
>set in the view mask in the subpass the query is used in.
> 
>How the numerical results of the query are distributed among the
>queries is implementation-dependent. For example, some
> implementations
>may write each view's results to a distinct query, while other
>implementations may write the total result to the first query and
> write
>zero to the other queries. However, the sum of the results in all
> the
>queries must accurately reflect the total result of the query
> summed
>over all views. Applications can sum the results from all the
> queries to
>compute the total result."
> 
> In our case we only really emit a single query (in the first query
> index)
> that stores the aggregated result for all views, but we still need to
> manage
> availability for all the other query indices involved, even if we
> don't
> actually use them.
> 
> This is relevant when clients call vkGetQueryPoolResults and pass all
> N
> queries to retrieve the results. In that scenario, without this
> patch,
> we will never see queries other than the first being available since
> we
> never emit them.
> 
> Fixes test failures in some work-in-progress CTS multiview+query
> tests.
> ---
>  src/intel/vulkan/genX_query.c | 36
> 
>  1 file changed, 36 insertions(+)
> 
> diff --git a/src/intel/vulkan/genX_query.c
> b/src/intel/vulkan/genX_query.c
> index 7683d0d1e3..231c605b6b 100644
> --- a/src/intel/vulkan/genX_query.c
> +++ b/src/intel/vulkan/genX_query.c
> @@ -462,6 +462,24 @@ void genX(CmdEndQuery)(
> default:
>unreachable("");
> }
> +
> +   /* When multiview is active the spec requires that N consecutive
> query
> +* indices are used, where N is the number of active views in the
> subpass.
> +* The spec allows that we only write the results to one of the
> queries
> +* but we still need to manage result availability for all the
> query indices.
> +* Since we only emit a single query for all active views in the
> +* first index, mark the other query indices as being already
> available
> +* with result 0.
> +*/
> +   if (!cmd_buffer->state.subpass || !cmd_buffer->state.subpass-
> >view_mask)
> +  return;
> +
> +   uint32_t num_queries = _mesa_bitcount(cmd_buffer->state.subpass-
> >view_mask);
> +   for (uint32_t i = 1; i < num_queries; i++) {
> +  uint64_t *slot = pool->bo.map + (query + i) * pool->stride;
> +  slot[0] = 1;
> +  memset([1], 0, sizeof(uint64_t) * pool->stride);
> +   }
>  }
>  
>  #define TIMESTAMP 0x2358
> @@ -504,6 +522,24 @@ void genX(CmdWriteTimestamp)(
> }
>  
> emit_query_availability(cmd_buffer, >bo, offset);
> +
> +   /* When multiview is active the spec requires that N consecutive
> query
> +* indices are used, where N is the number of active views in the
> subpass.
> +* The spec allows that we only write the results to one of the
> queries
> +* but we still need to manage result availability for all the
> query indices.
> +* Since we only emit a single query for all active views in the
> +* first index, mark the other query indices as being already
> available
> +* with result 0.
> +*/
> +   if (!cmd_buffer->state.subpass || !cmd_buffer->state.subpass-
> >view_mask)
> +  return;
> +
> +   uint32_t num_queries = _mesa_bitcount(cmd_buffer->state.subpass-
> >view_mask);
> +   for (uint32_t i = 1; i < num_queries; i++) {
> +  uint64_t *slot = pool->bo.map + (query + i) * pool->stride;
> +  slot[0] = 1;
> +  memset([1], 0, sizeof(uint64_t) * pool->stride);
> +   }
>  }
>  
>  #if GEN_GEN > 7 || GEN_IS_HASWELL
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 1/3] anv/blorp: only clear enabled views when multiview is used

[Iago Toral]

This series is still awaiting review, any takers?

On Fri, 2018-01-05 at 17:38 +0100, Iago Toral Quiroga wrote:
> ---
>  src/intel/vulkan/anv_blorp.c | 55 
> 
>  1 file changed, 40 insertions(+), 15 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_blorp.c
> b/src/intel/vulkan/anv_blorp.c
> index e244468e03..18fa4a4ae5 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -1203,25 +1203,50 @@ anv_cmd_buffer_clear_subpass(struct
> anv_cmd_buffer *cmd_buffer)
>  ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT |
> ANV_PIPE_CS_STALL_BIT;
>  
>   assert(image->n_planes == 1);
> - blorp_fast_clear(, , iview-
> >planes[0].isl.format,
> -  iview->planes[0].isl.base_level,
> -  iview->planes[0].isl.base_array_layer, fb-
> >layers,
> -  render_area.offset.x,
> render_area.offset.y,
> -  render_area.offset.x +
> render_area.extent.width,
> -  render_area.offset.y +
> render_area.extent.height);
> -
> + if (cmd_state->subpass->view_mask) {
> +uint32_t view_idx;
> +for_each_bit(view_idx, cmd_state->subpass->view_mask) {
> +   blorp_fast_clear(, , iview-
> >planes[0].isl.format,
> +iview->planes[0].isl.base_level,
> +view_idx, 1,
> +render_area.offset.x,
> render_area.offset.y,
> +render_area.offset.x +
> render_area.extent.width,
> +render_area.offset.y +
> render_area.extent.height);
> +}
> + } else {
> +blorp_fast_clear(, , iview-
> >planes[0].isl.format,
> + iview->planes[0].isl.base_level,
> + iview->planes[0].isl.base_array_layer,
> fb->layers,
> + render_area.offset.x,
> render_area.offset.y,
> + render_area.offset.x +
> render_area.extent.width,
> + render_area.offset.y +
> render_area.extent.height);
> + }
>   cmd_buffer->state.pending_pipe_bits |=
>  ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT |
> ANV_PIPE_CS_STALL_BIT;
>} else {
>   assert(image->n_planes == 1);
> - blorp_clear(, , iview->planes[0].isl.format,
> - anv_swizzle_for_render(iview-
> >planes[0].isl.swizzle),
> - iview->planes[0].isl.base_level,
> - iview->planes[0].isl.base_array_layer, fb-
> >layers,
> - render_area.offset.x, render_area.offset.y,
> - render_area.offset.x +
> render_area.extent.width,
> - render_area.offset.y +
> render_area.extent.height,
> - vk_to_isl_color(att_state->clear_value.color),
> NULL);
> + if (cmd_state->subpass->view_mask) {
> +uint32_t view_idx;
> +for_each_bit(view_idx, cmd_state->subpass->view_mask) {
> +   blorp_clear(, , iview-
> >planes[0].isl.format,
> +   anv_swizzle_for_render(iview-
> >planes[0].isl.swizzle),
> +   iview->planes[0].isl.base_level,
> +   view_idx, 1,
> +   render_area.offset.x,
> render_area.offset.y,
> +   render_area.offset.x +
> render_area.extent.width,
> +   render_area.offset.y +
> render_area.extent.height,
> +   vk_to_isl_color(att_state-
> >clear_value.color), NULL);
> +}
> + } else {
> +blorp_clear(, , iview->planes[0].isl.format,
> +anv_swizzle_for_render(iview-
> >planes[0].isl.swizzle),
> +iview->planes[0].isl.base_level,
> +iview->planes[0].isl.base_array_layer, fb-
> >layers,
> +render_area.offset.x, render_area.offset.y,
> +render_area.offset.x +
> render_area.extent.width,
> +render_area.offset.y +
> render_area.extent.height,
> +vk_to_isl_color(att_state-
> >clear_value.color), NULL);
> + }
>}
>  
>att_state->pending_clear_aspects = 0;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glx: fix non-dri build

[Samuel Thibault]

Dylan Baker, on dim. 14 janv. 2018 09:37:49 -0800, wrote:
> I don't know enough about glx to know if this is correct, but you'll need to
> add the c files to the meson.build as well

Oh, mesa now also uses meson. Ok, now commited that to my tree.

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