Re: [Mesa-dev] [PATCH 2/2] freedreno: a2xx: fix crash when freeing context

[Erik Faye-Lund]

On Tue, Jun 26, 2018, 14:00 Rob Clark  wrote:

> On Tue, Jun 26, 2018 at 3:32 AM, Erik Faye-Lund 
> wrote:
> > On Wed, Jun 20, 2018 at 3:03 AM Jonathan Marek 
> wrote:
> >>
> >> Signed-off-by: Jonathan Marek 
> >> ---
> >>  src/gallium/drivers/freedreno/a2xx/fd2_program.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/src/gallium/drivers/freedreno/a2xx/fd2_program.c
> b/src/gallium/drivers/freedreno/a2xx/fd2_program.c
> >> index 9a77457251..834a7c7fcd 100644
> >> --- a/src/gallium/drivers/freedreno/a2xx/fd2_program.c
> >> +++ b/src/gallium/drivers/freedreno/a2xx/fd2_program.c
> >> @@ -54,6 +54,8 @@ create_shader(enum shader_t type)
> >>  static void
> >>  delete_shader(struct fd2_shader_stateobj *so)
> >>  {
> >> +   if (!so)
> >> +   return;
> >> ir2_shader_destroy(so->ir);
> >> free(so->tokens);
> >> free(so->bin);
> >
> > This seems to just put a band-aid on top of some other bug... Wouldn't
> > it be better to lift this out to the call-site instead?
>
> Mmm, then you just end up w/ the null check in two or three different
> spots, instead of one..
>

I meant in the affected call site, not all. Basically, I was trying to hint
that this shouldn't have happened in the first place, and should be fixed
where this unexpectedly becomes null...

>
> I would be a bit curious to see the call-stack of the crash (since I
> haven't seen similar issues with a3xx/a4xx/a5xx), but I think putting
> the null check here is reasonable.
>
> BR,
> -R
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] meson: do not search for needless deps

[Erik Faye-Lund]

On Thu, Oct 26, 2017 at 11:49 AM, Gert Wollny <gw.foss...@gmail.com> wrote:
>
> Am Mittwoch, den 25.10.2017, 10:24 +0200 schrieb Erik Faye-Lund:
>> If we don't want to use these deps, there's no good reason to search
>> for them in the first place. This should shave a bit of time for the
>> initial build.
>> ---
>>
>> This would be a way of dealing with Gert's suggestion. Goes on top
>> of the previous patch.
>>
>> Thoughts?
>>
>>  meson.build | 20 ++--
>>  1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index e842bb1652..201956c4c8 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -666,9 +666,13 @@ if with_glvnd
>>  endif
>>
>>  # TODO: make this conditional
>> -dep_valgrind = dependency('valgrind', required : false)
>> -if dep_valgrind.found() and with_valgrind
>> -  pre_args += '-DHAVE_VALGRIND'
>> +if with_valgrind
>> +  dep_valgrind = dependency('valgrind', required : false)
>> +  if dep_valgrind.found()
>> +pre_args += '-DHAVE_VALGRIND'
>> +  endif
>> +else
>> +  dep_valgrind = []
>>  endif
>>
>>  # pthread stubs. Lets not and say we didn't
>> @@ -681,9 +685,13 @@ dep_selinux = []
>>
>>  # TODO: llvm-prefix and llvm-shared-libs
>>
>> -dep_unwind = dependency('libunwind', required : false)
>> -if dep_unwind.found() and with_libunwind
>> -  pre_args += '-DHAVE_LIBUNWIND'
>> +if with_libunwind
>> +  dep_unwind = dependency('libunwind', required : false)
>> +  if dep_unwind.found()
>> +pre_args += '-DHAVE_LIBUNWIND'
>> +  endif
>> +else
>> +  dep_unwind = []
>>  endif
>>
>>  # TODO: flags for opengl, gles, dri
>
> Obviously, I like this more, but after reviewing what is done with the
> autotools, I think it would be preferable if meson would do the same
> i.e. with autotools
>
>   --enable-libunwind=no -> don't use libunwind
>   --enable-libunwind=yes -> require libunwind
>
> and if not given, then use it if available. The "force using libunwind"
> option is currently missing.
>
> I don't know yet how to use meson properly, but I guess one could
> achieve this  by something like (put in the apropriate files):
>
> option('with_libunwind',
> type : 'combo',
> choices : ['auto', 'yes', 'no'])
>
> if with_libunwind != 'no'
>   dep_unwind = dependency('libunwind',
>required : with_libunwind == 'yes')
>   if dep_unwind.found()
> pre_args += '-DHAVE_LIBUNWIND'
>   endif
> else
>   dep_unwind = []
> endif
>

Yeah, that would probably be even nicer, as it would allow someone to
*ensure* they're using libunwind / valgrind.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] meson: drop vulkan if no drivers are built

[Erik Faye-Lund]

This avoids the following build-error when building with emtpy
vulkan-drivers and without glx=dri:

Meson encountered an error in file src/vulkan/wsi/meson.build, line 30,
column 2:
Unknown variable "dep_xcb".

Signed-off-by: Erik Faye-Lund <kusmab...@gmail.com>
---
 src/meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/meson.build b/src/meson.build
index 9b1b0ae594..4b00ab910c 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -47,7 +47,9 @@ subdir('mapi')
 # TODO: osmesa
 subdir('compiler')
 subdir('egl/wayland/wayland-drm')
-subdir('vulkan')
+if with_any_vk
+  subdir('vulkan')
+endif
 subdir('amd')
 if with_gallium_vc4
   subdir('broadcom')
-- 
2.11.0

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

Re: [Mesa-dev] [PATCH] meson: drop vulkan if no drivers are built

[Erik Faye-Lund]

Yeah, I kinda got that feeling. Your approach seems better.

On Oct 27, 2017 19:22, "Dylan Baker" <dy...@pnwbakers.com> wrote:

This just papers over the actual problem, that dep_xcb isn't declared as an
empty list with the other glx dependencies.

Dylan

Quoting Erik Faye-Lund (2017-10-27 04:55:25)
> This avoids the following build-error when building with emtpy
> vulkan-drivers and without glx=dri:
>
> Meson encountered an error in file src/vulkan/wsi/meson.build, line 30,
> column 2:
> Unknown variable "dep_xcb".
>
> Signed-off-by: Erik Faye-Lund <kusmab...@gmail.com>
> ---
>  src/meson.build | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/meson.build b/src/meson.build
> index 9b1b0ae594..4b00ab910c 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -47,7 +47,9 @@ subdir('mapi')
>  # TODO: osmesa
>  subdir('compiler')
>  subdir('egl/wayland/wayland-drm')
> -subdir('vulkan')
> +if with_any_vk
> +  subdir('vulkan')
> +endif
>  subdir('amd')
>  if with_gallium_vc4
>subdir('broadcom')
> --
> 2.11.0
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] meson: declare an empty xcb dependency

[Erik Faye-Lund]

On Fri, Oct 27, 2017 at 7:25 PM, Dylan Baker <dy...@pnwbakers.com> wrote:
> This is needed in cases where xcb isn't actually needed as a dependency,
> but may still be included somewhere.
>
> cc: Erik Faye-Lund <kusmab...@gmail.com>
> cc: Eric Engestrom <eric.engest...@imgtec.com>
> Signed-off-by: Dylan Baker <dylanx.c.ba...@intel.com>

Tested-by: Erik Faye-Lund <kusmab...@gmail.com>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/9] glsl/lower_64bit: extract non-64bit sources from vectors.

[Erik Faye-Lund]

On Feb 1, 2018 04:35, "Dave Airlie"  wrote:

From: Dave Airlie 

In order to deal with conversions properly we need to extract
non-64bit sources from vectors instead of expanding them as
the 64-bit code does.

We need non-64bit sources for the 32->64 conversion functions.

Signed-off-by: Dave Airlie 
---
 src/compiler/glsl/lower_64bit.cpp | 38 ++

 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/src/compiler/glsl/lower_64bit.cpp b/src/compiler/glsl/lower_
64bit.cpp
index ac62d1db1e..c7c6d1cb31 100644
--- a/src/compiler/glsl/lower_64bit.cpp
+++ b/src/compiler/glsl/lower_64bit.cpp
@@ -52,6 +52,7 @@ using namespace ir_builder;

 namespace lower_64bit {
 void expand_source(ir_factory &, ir_rvalue *val, ir_variable
**expanded_src);
+void extract_source(ir_factory &, ir_rvalue *val, ir_variable
**extracted_src);

 ir_dereference_variable *compact_destination(ir_factory &,
  const glsl_type *type,
@@ -226,6 +227,25 @@ lower_64bit::expand_source(ir_factory ,
   expanded_src[i] = expanded_src[0];
 }

+void
+lower_64bit::extract_source(ir_factory ,
+ir_rvalue *val,
+ir_variable **extracted_src)
+{
+   ir_variable *const temp = body.make_temp(val->type, "tmp");
+
+   body.emit(assign(temp, val));
+   unsigned i;
+   for (i = 0; i < val->type->vector_elements; i++) {
+  extracted_src[i] = body.make_temp(val->type->get_scalar_type(),
"extracted_source");
+
+  body.emit(assign(extracted_src[i], swizzle(temp, i, 1)));
+   }
+
+   for (/* empty */; i < 4; i++)
+  extracted_src[i] = extracted_src[0];
+}
+
 /**
  * Convert a series of uvec2 results into a single 64-bit integer vector
  */
@@ -262,14 +282,24 @@ lower_64bit::lower_op_to_function_call(ir_instruction
*base_ir,
void *const mem_ctx = ralloc_parent(ir);
exec_list instructions;
unsigned source_components = 0;
-   const glsl_type *const result_type =
-  ir->type->base_type == GLSL_TYPE_UINT64
-  ? glsl_type::uvec2_type : glsl_type::ivec2_type;
+   const glsl_type *result_type;
+
+   if (ir->type->is_64bit()) {
+  if (ir->type->base_type == GLSL_TYPE_UINT64 ||
+  ir->type->base_type == GLSL_TYPE_DOUBLE)
+ result_type = glsl_type::uvec2_type;
+  else
+ result_type = glsl_type::ivec2_type;
+   } else
+  result_type = ir->type->get_scalar_type();

ir_factory body(, mem_ctx);

for (unsigned i = 0; i < num_operands; i++) {
-  expand_source(body, ir->operands[i], src[i]);
+  if (ir->operands[i]->type->is_64bit())
+ expand_source(body, ir->operands[i], src[i]);
+  else
+ extract_source(body, ir->operands[i], src[i]);


This change looks like a nop to me. Was the different sides of the else
supposed to do different things?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 5/5] tegra: Initial support

[Erik Faye-Lund]

On Thu, Feb 22, 2018 at 2:23 PM, Thierry Reding
 wrote:
> On Wed, Feb 21, 2018 at 05:01:02PM +, Emil Velikov wrote:
>> Hi Thierry,
>>
>> On 21 February 2018 at 15:30, Thierry Reding  
>> wrote:
>> > +static const char *
>> > +tegra_screen_get_name(struct pipe_screen *pscreen)
>> > +{
>> > +   return "tegra";
>> > +}
>> > +
>> > +static const char *
>> > +tegra_screen_get_vendor(struct pipe_screen *pscreen)
>> > +{
>> > +   return "NVIDIA";
>> > +}
>> > +
>> > +static const char *
>> > +tegra_screen_get_device_vendor(struct pipe_screen *pscreen)
>> > +{
>> > +   return "NVIDIA";
>> > +}
>> > +
>> As-is these might be a bit fiddly, but will do for now.
>> For example - how will devs distinguish between the closed-source
>> driver and Mesa.
>
> Good point. Let me check what exactly we use in the closed-source driver
> and then come up with a proposal.
>
> I think perhaps a good choice for the vendor would be "grate". Even
> though this driver isn't part of the grate project, I'm hoping that we
> will eventually see Erik's and Dmitry's work merged into this.
>
> Adding Erik and Dmitry to get their opinion.
>

It's not entirely clear to me what the most natural boundary between
Tegra 2/3/4 (the GPUs the grate-project targets) and Tegra K1 would
be. The later Tegras are so fundamentally different in how they
work...

Should they share the implementation of tegra_screen_create? From a
quick look, it doesn't seem like it (mostly K1 specifics going on
there, it seems), but I could be wrong. And if it shouldn't perhaps it
should simply be a separate gallium-driver ("grate" vs "tegra"
maybe?)... We can probably share some code, but we can do that without
sharing the driver, just like intel, amd and broadcom does with just a
shared folder at src/nvidia or something for utilities...

I don't know, just trying to avoid having to add too many conditionals here...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] meson: use correct keyword to fix a meson warning

[Erik Faye-Lund]

On 02. aug. 2018 15:58, Jon Turney wrote:

With a sufficently recent meson, the following warning is produced:

WARNING: Passed invalid keyword argument "extra_args".
WARNING: This will become a hard error in the future.

It seems that compiler.links(args:) is meant here.

Signed-off-by: Jon Turney 



Perhaps add this:

Fixes: d1992255bb2 ("meson: Add build Intel "anv" vulkan driver")

...or CC mesa-sta...@lists.freedesktop.org to get this included in the 
18.2 release?


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

[Mesa-dev] [PATCH v2 1/2] virgl: rename msaa_sample_positions -> sample_locations

[Erik Faye-Lund]

This matches what this field is called in virglrenderer's copy of
this.

While we're at it, fixup the indentation.

This reduces the diff between the two different versions of
virgl_hw.h, and should make it easier to upgrade the file in
the future.
---
 src/gallium/drivers/virgl/virgl_context.c | 8 
 src/gallium/drivers/virgl/virgl_hw.h  | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_context.c 
b/src/gallium/drivers/virgl/virgl_context.c
index 74b232fe6c..832bd05efc 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -996,13 +996,13 @@ static void virgl_get_sample_position(struct pipe_context 
*ctx,
   out_value[0] = out_value[1] = 0.5f;
   return;
} else if (sample_count == 2) {
-  bits = vs->caps.caps.v2.msaa_sample_positions[0] >> (8 * index);
+  bits = vs->caps.caps.v2.sample_locations[0] >> (8 * index);
} else if (sample_count <= 4) {
-  bits = vs->caps.caps.v2.msaa_sample_positions[1] >> (8 * index);
+  bits = vs->caps.caps.v2.sample_locations[1] >> (8 * index);
} else if (sample_count <= 8) {
-  bits = vs->caps.caps.v2.msaa_sample_positions[2 + (index >> 2)] >> (8 * 
(index & 3));
+  bits = vs->caps.caps.v2.sample_locations[2 + (index >> 2)] >> (8 * 
(index & 3));
} else if (sample_count <= 16) {
-  bits = vs->caps.caps.v2.msaa_sample_positions[4 + (index >> 2)] >> (8 * 
(index & 3));
+  bits = vs->caps.caps.v2.sample_locations[4 + (index >> 2)] >> (8 * 
(index & 3));
}
out_value[0] = ((bits >> 4) & 0xf) / 16.0f;
out_value[1] = (bits & 0xf) / 16.0f;
diff --git a/src/gallium/drivers/virgl/virgl_hw.h 
b/src/gallium/drivers/virgl/virgl_hw.h
index 4469515dd1..059888b13b 100644
--- a/src/gallium/drivers/virgl/virgl_hw.h
+++ b/src/gallium/drivers/virgl/virgl_hw.h
@@ -303,7 +303,7 @@ struct virgl_caps_v2 {
 uint32_t uniform_buffer_offset_alignment;
 uint32_t shader_buffer_offset_alignment;
 uint32_t capability_bits;
-uint32_t msaa_sample_positions[8];
+uint32_t sample_locations[8];
 uint32_t max_vertex_attrib_stride;
 uint32_t max_shader_buffer_frag_compute;
 uint32_t max_shader_buffer_other_stages;
-- 
2.18.0

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

[Mesa-dev] [PATCH v2 0/2] virgl: synchronize virgl_hw.h with virglrenderer

[Erik Faye-Lund]

Here's a few patches to get the delta between our copy of virgl_hw.h and the
copy in the virglrenderer repository as small as possible.

The goal is to make it easier to track this file in the future by simply
copying the new version of the file on top of the old.

Changes since v2:
- Rebased on master
- Dropped "virgl: move bind-flags to virgl_winsys.h" as discussed in
  review.
- Updated "virgl: update virgl_hw.h from virglrenderer" to the latest
  master from virglrenderer.

Erik Faye-Lund (2):
  virgl: rename msaa_sample_positions -> sample_locations
  virgl: update virgl_hw.h from virglrenderer

 src/gallium/drivers/virgl/virgl_context.c |  8 +++
 src/gallium/drivers/virgl/virgl_hw.h  | 29 +--
 2 files changed, 31 insertions(+), 6 deletions(-)

-- 
2.18.0

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

[Mesa-dev] [PATCH v2 2/2] virgl: update virgl_hw.h from virglrenderer

[Erik Faye-Lund]

This just makes sure we're currently up-to-date with what
virglrenderer has.
---
 src/gallium/drivers/virgl/virgl_hw.h | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/virgl/virgl_hw.h 
b/src/gallium/drivers/virgl/virgl_hw.h
index 059888b13b..51a325548b 100644
--- a/src/gallium/drivers/virgl/virgl_hw.h
+++ b/src/gallium/drivers/virgl/virgl_hw.h
@@ -37,6 +37,7 @@ enum virgl_formats {
VIRGL_FORMAT_B5G5R5A1_UNORM  = 5,
VIRGL_FORMAT_B4G4R4A4_UNORM  = 6,
VIRGL_FORMAT_B5G6R5_UNORM= 7,
+   VIRGL_FORMAT_R10G10B10A2_UNORM   = 8,
VIRGL_FORMAT_L8_UNORM= 9,/**< ubyte luminance */
VIRGL_FORMAT_A8_UNORM= 10,   /**< ubyte alpha */
VIRGL_FORMAT_L8A8_UNORM  = 12,   /**< ubyte alpha, luminance */
@@ -112,6 +113,8 @@ enum virgl_formats {
VIRGL_FORMAT_B10G10R10A2_UNORM   = 131,
VIRGL_FORMAT_R8G8B8X8_UNORM  = 134,
VIRGL_FORMAT_B4G4R4X4_UNORM  = 135,
+   VIRGL_FORMAT_X24S8_UINT  = 136,
+   VIRGL_FORMAT_S8X24_UINT  = 137,
VIRGL_FORMAT_B2G3R3_UNORM= 139,
 
VIRGL_FORMAT_L16A16_UNORM= 140,
@@ -186,14 +189,29 @@ enum virgl_formats {
VIRGL_FORMAT_L32_SINT= 223,
VIRGL_FORMAT_L32A32_SINT = 224,
 
-   VIRGL_FORMAT_B10G10R10A2_UINT= 225, 
+   VIRGL_FORMAT_B10G10R10A2_UINT= 225,
VIRGL_FORMAT_R8G8B8X8_SNORM  = 229,
 
VIRGL_FORMAT_R8G8B8X8_SRGB   = 230,
 
+   VIRGL_FORMAT_R8G8B8X8_UINT   = 231,
+   VIRGL_FORMAT_R8G8B8X8_SINT   = 232,
VIRGL_FORMAT_B10G10R10X2_UNORM   = 233,
VIRGL_FORMAT_R16G16B16X16_UNORM  = 234,
VIRGL_FORMAT_R16G16B16X16_SNORM  = 235,
+   VIRGL_FORMAT_R16G16B16X16_FLOAT  = 236,
+   VIRGL_FORMAT_R16G16B16X16_UINT   = 237,
+   VIRGL_FORMAT_R16G16B16X16_SINT   = 238,
+
+   VIRGL_FORMAT_R10G10B10A2_UINT= 253,
+
+   VIRGL_FORMAT_BPTC_RGBA_UNORM = 255,
+   VIRGL_FORMAT_BPTC_SRGBA  = 256,
+   VIRGL_FORMAT_BPTC_RGB_FLOAT  = 257,
+   VIRGL_FORMAT_BPTC_RGB_UFLOAT = 258,
+
+   VIRGL_FORMAT_R10G10B10X2_UNORM   = 308,
+   VIRGL_FORMAT_A4B4G4R4_UNORM  = 311,
VIRGL_FORMAT_MAX,
 };
 
@@ -205,6 +223,9 @@ enum virgl_formats {
 #define VIRGL_CAP_COPY_IMAGE   (1 << 3)
 #define VIRGL_CAP_TGSI_PRECISE (1 << 4)
 
+/* virgl bind flags - these are compatible with mesa 10.5 gallium.
+ * but are fixed, no other should be passed to virgl either.
+ */
 #define VIRGL_BIND_DEPTH_STENCIL (1 << 0)
 #define VIRGL_BIND_RENDER_TARGET (1 << 1)
 #define VIRGL_BIND_SAMPLER_VIEW  (1 << 3)
@@ -279,6 +300,10 @@ struct virgl_caps_v1 {
 uint32_t max_texture_gather_components;
 };
 
+/*
+ * This struct should be growable when used in capset 2,
+ * so we shouldn't have to add a v3 ever.
+ */
 struct virgl_caps_v2 {
 struct virgl_caps_v1 v1;
 float min_aliased_point_size;
-- 
2.18.0

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

Re: [Mesa-dev] [PATCH v2 0/2] virgl: synchronize virgl_hw.h with virglrenderer

[Erik Faye-Lund]

Ping?


On 27. juli 2018 09:18, Erik Faye-Lund wrote:

Here's a few patches to get the delta between our copy of virgl_hw.h and the
copy in the virglrenderer repository as small as possible.

The goal is to make it easier to track this file in the future by simply
copying the new version of the file on top of the old.

Changes since v2:
- Rebased on master
- Dropped "virgl: move bind-flags to virgl_winsys.h" as discussed in
   review.
- Updated "virgl: update virgl_hw.h from virglrenderer" to the latest
   master from virglrenderer.

Erik Faye-Lund (2):
   virgl: rename msaa_sample_positions -> sample_locations
   virgl: update virgl_hw.h from virglrenderer

  src/gallium/drivers/virgl/virgl_context.c |  8 +++
  src/gallium/drivers/virgl/virgl_hw.h  | 29 +--
  2 files changed, 31 insertions(+), 6 deletions(-)



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

Re: [Mesa-dev] [PATCH] virgl: add ARB_shader_clock support

[Erik Faye-Lund]

Reviewed-by: Erik Faye-Lund 


On 06. aug. 2018 22:23, Dave Airlie wrote:

From: Dave Airlie 

---
  src/gallium/drivers/virgl/virgl_hw.h | 1 +
  src/gallium/drivers/virgl/virgl_screen.c | 3 ++-
  2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/virgl/virgl_hw.h 
b/src/gallium/drivers/virgl/virgl_hw.h
index 02cedcd0dc0..978839ad4fd 100644
--- a/src/gallium/drivers/virgl/virgl_hw.h
+++ b/src/gallium/drivers/virgl/virgl_hw.h
@@ -210,6 +210,7 @@ enum virgl_formats {
  #define VIRGL_CAP_FB_NO_ATTACH (1 << 8)
  #define VIRGL_CAP_ROBUST_BUFFER_ACCESS (1 << 9)
  #define VIRGL_CAP_TGSI_FBFETCH (1 << 10)
+#define VIRGL_CAP_SHADER_CLOCK (1 << 11)
  
  #define VIRGL_BIND_DEPTH_STENCIL (1 << 0)

  #define VIRGL_BIND_RENDER_TARGET (1 << 1)
diff --git a/src/gallium/drivers/virgl/virgl_screen.c 
b/src/gallium/drivers/virgl/virgl_screen.c
index e17d257fab1..421fde5249d 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -231,6 +231,8 @@ virgl_get_param(struct pipe_screen *screen, enum pipe_cap 
param)
return vscreen->caps.caps.v2.capability_bits & 
VIRGL_CAP_ROBUST_BUFFER_ACCESS;
 case PIPE_CAP_TGSI_FS_FBFETCH:
return vscreen->caps.caps.v2.capability_bits & VIRGL_CAP_TGSI_FBFETCH;
+   case PIPE_CAP_TGSI_CLOCK:
+  return vscreen->caps.caps.v2.capability_bits & VIRGL_CAP_SHADER_CLOCK;
 case PIPE_CAP_TEXTURE_GATHER_SM5:
 case PIPE_CAP_BUFFER_MAP_PERSISTENT_COHERENT:
 case PIPE_CAP_FAKE_SW_MSAA:
@@ -274,7 +276,6 @@ virgl_get_param(struct pipe_screen *screen, enum pipe_cap 
param)
 case PIPE_CAP_INT64:
 case PIPE_CAP_INT64_DIVMOD:
 case PIPE_CAP_TGSI_TEX_TXF_LZ:
-   case PIPE_CAP_TGSI_CLOCK:
 case PIPE_CAP_POLYGON_MODE_FILL_RECTANGLE:
 case PIPE_CAP_SPARSE_BUFFER_PAGE_SIZE:
 case PIPE_CAP_TGSI_BALLOT:


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

[Mesa-dev] [PATCH v2] i965: do not emit empty surface state

[Erik Faye-Lund]

If called with an empty size, brw_emit_buffer_surface_state asserts.
We already have a dedicated helper for uploading nothing, so let's use
that instead.

Signed-off-by: Erik Faye-Lund 
---

Here's an updated patch.

v2: call emit_null_surface_state to ensure out_offset is initialized
properly. (Lionel)

 src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 9397b637c7..2aef0ef59f 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1322,6 +1322,11 @@ upload_buffer_surface(struct brw_context *brw,
   if (!binding->AutomaticSize)
  size = MIN2(size, binding->Size);
 
+  if (size == 0) {
+ emit_null_surface_state(brw, NULL, out_offset);
+ return;
+  }
+
   struct intel_buffer_object *iobj =
  intel_buffer_object(binding->BufferObject);
   struct brw_bo *bo =
-- 
2.18.0

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

Re: [Mesa-dev] [PATCH 1/2] virgl: do not use SP_MAX_TEXTURE_*_LEVELS defines

[Erik Faye-Lund]

If you are referring to PIPE_CAP_MAX_TEXTURE_ARRAY_LAYERS, then that's 
already taken care of for virgl...


On Tue, Aug 14, 2018 at 6:00 PM, Marek Olšák  wrote:
I'm saying that the limit should be 16k except for the layer/slice 
dimension where it can be 2k. See radeonsi.


Marek

On Tue, Aug 14, 2018, 11:57 AM Erik Faye-Lund 
 wrote:
What are you referring to, exactly? In OpenGL, texture size-limits 
are per axis, not in total...


On Tue, Aug 14, 2018 at 5:43 PM, Marek Olšák  
wrote:

You could have a 3d or cube texture where 1 or 2 dimensions are 16k.

Marek

On Tue, Aug 14, 2018, 9:02 AM Erik Faye-Lund 
 wrote:
These macro-names are also used for softpipe, so let's avoid 
confusion
by avoiding them. Besides, they are just used in one place in 
virgl, so

let's just inline them into the place they are used instead.

While we're at it, fixup an error in the comment for the 3D 
version.
Mesa subtracts computes max-size by doing by 2^(n-1), which means 
this

should be 256 cubed, not 512 cubed. The other comments are correct.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_screen.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_screen.c 
b/src/gallium/drivers/virgl/virgl_screen.c

index 87ce3b7355..0ac976acbd 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -35,10 +35,6 @@
 #include "virgl_public.h"
 #include "virgl_context.h"

-#define SP_MAX_TEXTURE_2D_LEVELS 15  /* 16K x 16K */
-#define SP_MAX_TEXTURE_3D_LEVELS 9   /* 512 x 512 x 512 */
-#define SP_MAX_TEXTURE_CUBE_LEVELS 13  /* 4K x 4K */
-
 static const char *
 virgl_get_vendor(struct pipe_screen *screen)
 {
@@ -76,11 +72,11 @@ virgl_get_param(struct pipe_screen *screen, 
enum pipe_cap param)

case PIPE_CAP_TEXTURE_SWIZZLE:
   return 1;
case PIPE_CAP_MAX_TEXTURE_2D_LEVELS:
-  return SP_MAX_TEXTURE_2D_LEVELS;
+  return 15; /* 16K x 16K */
case PIPE_CAP_MAX_TEXTURE_3D_LEVELS:
-  return SP_MAX_TEXTURE_3D_LEVELS;
+  return 9; /* 256 x 256 x 256 */
case PIPE_CAP_MAX_TEXTURE_CUBE_LEVELS:
-  return SP_MAX_TEXTURE_CUBE_LEVELS;
+  return 13; /* 4K x 4K */
case PIPE_CAP_BLEND_EQUATION_SEPARATE:
   return 1;
case PIPE_CAP_INDEP_BLEND_ENABLE:
--
2.17.1

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

[Mesa-dev] [PATCH] mesa/st/glsl_to_tgsi: fixup copy-paste mistake

[Erik Faye-Lund]

This is clearly a copy-paste error; if we validate the reladdr2-pointer,
we don't want to traverse to the reladdr-pointer. Especially since the
check above shows that reladdr could be NULL here.

Noticed by Coverity.

CID: 1438389, 1438390
Fixes: 568bda2f2d3 ("mesa/st/glsl_to_tgsi: Split arrays whose elements are only 
accessed directly")
Signed-off-by: Erik Faye-Lund 
---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index a2083a4f85..2b9183abbb 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -5493,7 +5493,7 @@ void test_indirect_access(const st_reg& reg, bool 
*has_indirect_access)
 if (reg.reladdr)
test_indirect_access(*reg.reladdr, has_indirect_access);
 if (reg.reladdr2)
-   test_indirect_access(*reg.reladdr, has_indirect_access);
+   test_indirect_access(*reg.reladdr2, has_indirect_access);
   }
}
 }
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH] virgl: bump 3D texture limit to 2048, as GL4.1 requires

[Erik Faye-Lund]

On Tue, Aug 14, 2018 at 5:47 AM, Gurchetan Singh 
 wrote:

This is virgl analogue to cf6dad.

Fixes:
  dEQP-GLES31.functional.image_load_store.3d.atomic.*

Example test cases:
  
dEQP-GLES31.functional.image_load_store.3d.atomic.add_r32ui_return_value
  
dEQP-GLES31.functional.image_load_store.3d.atomic.min_r32ui_return_value
  
dEQP-GLES31.functional.image_load_store.3d.atomic.max_r32ui_return_value
  
dEQP-GLES31.functional.image_load_store.3d.atomic.and_r32ui_return_value

---
 src/gallium/drivers/virgl/virgl_screen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/virgl/virgl_screen.c 
b/src/gallium/drivers/virgl/virgl_screen.c

index 421fde5249..745a502189 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -36,7 +36,7 @@
 #include "virgl_context.h"

 #define SP_MAX_TEXTURE_2D_LEVELS 15  /* 16K x 16K */
-#define SP_MAX_TEXTURE_3D_LEVELS 9   /* 512 x 512 x 512 */
+#define SP_MAX_TEXTURE_3D_LEVELS 12   /* 2048 x 2048 x 2048 */
 #define SP_MAX_TEXTURE_CUBE_LEVELS 13  /* 4K x 4K */



I don't think this is a good idea.

OpenGL ES 3.1 only requires 256^3 support, so this is going to fail 
when running on such hardware. I'm working on patches that actually 
pass what the host supports instead.


But, a puzzling details is that these tests don't use more than 512 in 
either direction. They use max 320 (= 64 * 5). And mesa seems to think 
256 is the actual max here. So something is going wrong between us 
reportig this to mesa and


There's also a bug in the test, where it tests size that are out of 
spec without checking the max-size, it seems. I have a patch for this 
also.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] i965: do not emit empty surface state

[Erik Faye-Lund]

Quoting the original patch:

---8<---
This fixes an assert that triggers for me when running
dEQP-GLES31.functional.shaders.opaque_type_indexing.ssbo.const_literal_vertex
on top of a debug-build of mesa on top of i965. Since setting up a virgl
setup is rather convoluted, here's a piglit shader-test that reproduce 
it

directly on top of a debug-build of mesa on i965:

https://gitlab.freedesktop.org/kusma/piglit/tree/ssbo-zero-size
---8<---

So yeah, that last link should give you an application that reproduce 
this. Do note that this only triggers with a debug-build of Mesa.


I might not have phrased this clearly in the quoted text, but the 
initial reproduction was from within a virual machine running virgl, 
which crashed the VM. That's a pretty convoluted setup, so the 
stand-alone virgl case should be much easier to work with.



On Wed, Aug 15, 2018 at 12:11 AM, Lionel Landwerlin 
 wrote:

Hey Erik,

Out of curiosity, what app/test managed to run into this assert?
We could have some test added :)

Thanks,

-
Lionel

On 08/08/18 09:34, Lionel Landwerlin wrote:

On 07/08/18 20:31, Erik Faye-Lund wrote:

If called with an empty size, brw_emit_buffer_surface_state asserts.
We already have a dedicated helper for uploading nothing, so let's 
use

that instead.

Signed-off-by: Erik Faye-Lund 


Looks good to me. Maybe Ken can confirm?

Reviewed-by: Lionel Landwerlin 


---

Here's an updated patch.

v2: call emit_null_surface_state to ensure out_offset is initialized
 properly. (Lionel)

  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c

index 9397b637c7..2aef0ef59f 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1322,6 +1322,11 @@ upload_buffer_surface(struct brw_context 
*brw,

if (!binding->AutomaticSize)
   size = MIN2(size, binding->Size);
  +  if (size == 0) {
+ emit_null_surface_state(brw, NULL, out_offset);
+ return;
+  }
+
struct intel_buffer_object *iobj =
   intel_buffer_object(binding->BufferObject);
struct brw_bo *bo =



___
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] i965: do not emit empty sampler state

[Erik Faye-Lund]

If called with an empty size, brw_emit_buffer_surface_state asserts.
And since doing a zero-size upload is really just a lot of work for
no gain, let's just avoid the call in the first place.

Signed-off-by: Erik Faye-Lund 
---
This fixes an assert that triggers for me when running
dEQP-GLES31.functional.shaders.opaque_type_indexing.ssbo.const_literal_vertex
on top of a debug-build of mesa on top of i965. Since setting up a virgl
setup is rather convoluted, here's a piglit shader-test that reproduce it
directly on top of a debug-build of mesa on i965:

https://gitlab.freedesktop.org/kusma/piglit/tree/ssbo-zero-size

 .../drivers/dri/i965/brw_wm_surface_state.c| 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 9397b637c7..0da7f97218 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1322,14 +1322,16 @@ upload_buffer_surface(struct brw_context *brw,
   if (!binding->AutomaticSize)
  size = MIN2(size, binding->Size);
 
-  struct intel_buffer_object *iobj =
- intel_buffer_object(binding->BufferObject);
-  struct brw_bo *bo =
- intel_bufferobj_buffer(brw, iobj, binding->Offset, size,
-(reloc_flags & RELOC_WRITE) != 0);
-
-  brw_emit_buffer_surface_state(brw, out_offset, bo, binding->Offset,
-format, size, 1, reloc_flags);
+  if (size > 0) {
+ struct intel_buffer_object *iobj =
+intel_buffer_object(binding->BufferObject);
+ struct brw_bo *bo =
+intel_bufferobj_buffer(brw, iobj, binding->Offset, size,
+   (reloc_flags & RELOC_WRITE) != 0);
+
+ brw_emit_buffer_surface_state(brw, out_offset, bo, binding->Offset,
+   format, size, 1, reloc_flags);
+  }
}
 }
 
-- 
2.18.0

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

Re: [Mesa-dev] [PATCH] i965: do not emit empty sampler state

[Erik Faye-Lund]

On 06. aug. 2018 16:15, Lionel Landwerlin wrote:

Hey Erik,

I don't think this is right, because by not filling out_offset, you're 
leaving it at 0 which makes the entry in the binding table point to a 
dynamic state base address.

I would emit the null surface instead.



Hmm, good point. I'll rework it and resend.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] virgl: do not use SP_MAX_TEXTURE_*_LEVELS defines

[Erik Faye-Lund]

These macro-names are also used for softpipe, so let's avoid confusion
by avoiding them. Besides, they are just used in one place in virgl, so
let's just inline them into the place they are used instead.

While we're at it, fixup an error in the comment for the 3D version.
Mesa subtracts computes max-size by doing by 2^(n-1), which means this
should be 256 cubed, not 512 cubed. The other comments are correct.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_screen.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_screen.c 
b/src/gallium/drivers/virgl/virgl_screen.c
index 87ce3b7355..0ac976acbd 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -35,10 +35,6 @@
 #include "virgl_public.h"
 #include "virgl_context.h"
 
-#define SP_MAX_TEXTURE_2D_LEVELS 15  /* 16K x 16K */
-#define SP_MAX_TEXTURE_3D_LEVELS 9   /* 512 x 512 x 512 */
-#define SP_MAX_TEXTURE_CUBE_LEVELS 13  /* 4K x 4K */
-
 static const char *
 virgl_get_vendor(struct pipe_screen *screen)
 {
@@ -76,11 +72,11 @@ virgl_get_param(struct pipe_screen *screen, enum pipe_cap 
param)
case PIPE_CAP_TEXTURE_SWIZZLE:
   return 1;
case PIPE_CAP_MAX_TEXTURE_2D_LEVELS:
-  return SP_MAX_TEXTURE_2D_LEVELS;
+  return 15; /* 16K x 16K */
case PIPE_CAP_MAX_TEXTURE_3D_LEVELS:
-  return SP_MAX_TEXTURE_3D_LEVELS;
+  return 9; /* 256 x 256 x 256 */
case PIPE_CAP_MAX_TEXTURE_CUBE_LEVELS:
-  return SP_MAX_TEXTURE_CUBE_LEVELS;
+  return 13; /* 4K x 4K */
case PIPE_CAP_BLEND_EQUATION_SEPARATE:
   return 1;
case PIPE_CAP_INDEP_BLEND_ENABLE:
-- 
2.17.1

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

[Mesa-dev] [PATCH 2/2] virgl: report actual max-texture sizes

[Erik Faye-Lund]

Instead of doing conservative guesses, we should report the max levels
based on the max sizes we get from GL on the host.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_hw.h | 3 +++
 src/gallium/drivers/virgl/virgl_screen.c | 7 +++
 2 files changed, 10 insertions(+)

diff --git a/src/gallium/drivers/virgl/virgl_hw.h 
b/src/gallium/drivers/virgl/virgl_hw.h
index b56f554b00..787452d328 100644
--- a/src/gallium/drivers/virgl/virgl_hw.h
+++ b/src/gallium/drivers/virgl/virgl_hw.h
@@ -347,6 +347,9 @@ struct virgl_caps_v2 {
 uint32_t max_compute_shared_memory_size;
 uint32_t max_compute_grid_size[3];
 uint32_t max_compute_block_size[3];
+uint32_t max_texture_2d_size;
+uint32_t max_texture_3d_size;
+uint32_t max_texture_cube_size;
 };
 
 union virgl_caps {
diff --git a/src/gallium/drivers/virgl/virgl_screen.c 
b/src/gallium/drivers/virgl/virgl_screen.c
index 0ac976acbd..86063c66aa 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -24,6 +24,7 @@
 #include "util/u_format.h"
 #include "util/u_format_s3tc.h"
 #include "util/u_video.h"
+#include "util/u_math.h"
 #include "util/os_time.h"
 #include "pipe/p_defines.h"
 #include "pipe/p_screen.h"
@@ -72,10 +73,16 @@ virgl_get_param(struct pipe_screen *screen, enum pipe_cap 
param)
case PIPE_CAP_TEXTURE_SWIZZLE:
   return 1;
case PIPE_CAP_MAX_TEXTURE_2D_LEVELS:
+  if (vscreen->caps.caps.v2.max_texture_2d_size)
+ return 1 + util_logbase2(vscreen->caps.caps.v2.max_texture_2d_size);
   return 15; /* 16K x 16K */
case PIPE_CAP_MAX_TEXTURE_3D_LEVELS:
+  if (vscreen->caps.caps.v2.max_texture_3d_size)
+ return 1 + util_logbase2(vscreen->caps.caps.v2.max_texture_3d_size);
   return 9; /* 256 x 256 x 256 */
case PIPE_CAP_MAX_TEXTURE_CUBE_LEVELS:
+  if (vscreen->caps.caps.v2.max_texture_cube_size)
+ return 1 + util_logbase2(vscreen->caps.caps.v2.max_texture_cube_size);
   return 13; /* 4K x 4K */
case PIPE_CAP_BLEND_EQUATION_SEPARATE:
   return 1;
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH 2/2] virgl: report actual max-texture sizes

[Erik Faye-Lund]

I forgot to mention, this patch depends on a virglrenderer MR that 
hasn't been merged yet. So it shouldn't be merged before the MR has 
landed.


https://gitlab.freedesktop.org/virgl/virglrenderer/merge_requests/25


On Tue, Aug 14, 2018 at 3:02 PM, Erik Faye-Lund 
 wrote:

Instead of doing conservative guesses, we should report the max levels
based on the max sizes we get from GL on the host.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_hw.h | 3 +++
 src/gallium/drivers/virgl/virgl_screen.c | 7 +++
 2 files changed, 10 insertions(+)

diff --git a/src/gallium/drivers/virgl/virgl_hw.h 
b/src/gallium/drivers/virgl/virgl_hw.h

index b56f554b00..787452d328 100644
--- a/src/gallium/drivers/virgl/virgl_hw.h
+++ b/src/gallium/drivers/virgl/virgl_hw.h
@@ -347,6 +347,9 @@ struct virgl_caps_v2 {
 uint32_t max_compute_shared_memory_size;
 uint32_t max_compute_grid_size[3];
 uint32_t max_compute_block_size[3];
+uint32_t max_texture_2d_size;
+uint32_t max_texture_3d_size;
+uint32_t max_texture_cube_size;
 };

 union virgl_caps {
diff --git a/src/gallium/drivers/virgl/virgl_screen.c 
b/src/gallium/drivers/virgl/virgl_screen.c

index 0ac976acbd..86063c66aa 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -24,6 +24,7 @@
 #include "util/u_format.h"
 #include "util/u_format_s3tc.h"
 #include "util/u_video.h"
+#include "util/u_math.h"
 #include "util/os_time.h"
 #include "pipe/p_defines.h"
 #include "pipe/p_screen.h"
@@ -72,10 +73,16 @@ virgl_get_param(struct pipe_screen *screen, enum 
pipe_cap param)

case PIPE_CAP_TEXTURE_SWIZZLE:
   return 1;
case PIPE_CAP_MAX_TEXTURE_2D_LEVELS:
+  if (vscreen->caps.caps.v2.max_texture_2d_size)
+ return 1 + 
util_logbase2(vscreen->caps.caps.v2.max_texture_2d_size);

   return 15; /* 16K x 16K */
case PIPE_CAP_MAX_TEXTURE_3D_LEVELS:
+  if (vscreen->caps.caps.v2.max_texture_3d_size)
+ return 1 + 
util_logbase2(vscreen->caps.caps.v2.max_texture_3d_size);

   return 9; /* 256 x 256 x 256 */
case PIPE_CAP_MAX_TEXTURE_CUBE_LEVELS:
+  if (vscreen->caps.caps.v2.max_texture_cube_size)
+ return 1 + 
util_logbase2(vscreen->caps.caps.v2.max_texture_cube_size);

   return 13; /* 4K x 4K */
case PIPE_CAP_BLEND_EQUATION_SEPARATE:
   return 1;
--
2.17.1

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

Re: [Mesa-dev] [PATCH 1/2] virgl: do not use SP_MAX_TEXTURE_*_LEVELS defines

[Erik Faye-Lund]

What are you referring to, exactly? In OpenGL, texture size-limits are 
per axis, not in total...


On Tue, Aug 14, 2018 at 5:43 PM, Marek Olšák  wrote:

You could have a 3d or cube texture where 1 or 2 dimensions are 16k.

Marek

On Tue, Aug 14, 2018, 9:02 AM Erik Faye-Lund 
 wrote:
These macro-names are also used for softpipe, so let's avoid 
confusion
by avoiding them. Besides, they are just used in one place in virgl, 
so

let's just inline them into the place they are used instead.

While we're at it, fixup an error in the comment for the 3D version.
Mesa subtracts computes max-size by doing by 2^(n-1), which means 
this

should be 256 cubed, not 512 cubed. The other comments are correct.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_screen.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_screen.c 
b/src/gallium/drivers/virgl/virgl_screen.c

index 87ce3b7355..0ac976acbd 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -35,10 +35,6 @@
 #include "virgl_public.h"
 #include "virgl_context.h"

-#define SP_MAX_TEXTURE_2D_LEVELS 15  /* 16K x 16K */
-#define SP_MAX_TEXTURE_3D_LEVELS 9   /* 512 x 512 x 512 */
-#define SP_MAX_TEXTURE_CUBE_LEVELS 13  /* 4K x 4K */
-
 static const char *
 virgl_get_vendor(struct pipe_screen *screen)
 {
@@ -76,11 +72,11 @@ virgl_get_param(struct pipe_screen *screen, enum 
pipe_cap param)

case PIPE_CAP_TEXTURE_SWIZZLE:
   return 1;
case PIPE_CAP_MAX_TEXTURE_2D_LEVELS:
-  return SP_MAX_TEXTURE_2D_LEVELS;
+  return 15; /* 16K x 16K */
case PIPE_CAP_MAX_TEXTURE_3D_LEVELS:
-  return SP_MAX_TEXTURE_3D_LEVELS;
+  return 9; /* 256 x 256 x 256 */
case PIPE_CAP_MAX_TEXTURE_CUBE_LEVELS:
-  return SP_MAX_TEXTURE_CUBE_LEVELS;
+  return 13; /* 4K x 4K */
case PIPE_CAP_BLEND_EQUATION_SEPARATE:
   return 1;
case PIPE_CAP_INDEP_BLEND_ENABLE:
--
2.17.1

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

[Mesa-dev] [PATCH] glsl_to_tgsi: plumb image writable through to driver

[Erik Faye-Lund]

The virgl driver cares about the writable-flag on image definitions,
because it re-emits GLSL from the TGSI. However, so far it was hardcoded
to true in glsl_to_tgsi, which cause problems when virglrenderer is
running on top of GLES 3.1, where not all formats are supported for
writable images.

Signed-off-by: Erik Faye-Lund 
---

This patch is needed for this patch-series in virglrenderer to work
properly:

https://patchwork.freedesktop.org/series/47775/

 src/mesa/state_tracker/st_glsl_to_tgsi.cpp| 19 ++-
 .../state_tracker/st_glsl_to_tgsi_private.h   |  1 +
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index aec5330917..55984ff0c7 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -194,6 +194,7 @@ public:
int images_used;
enum tgsi_texture_type image_targets[PIPE_MAX_SHADER_IMAGES];
enum pipe_format image_formats[PIPE_MAX_SHADER_IMAGES];
+   bool image_wr[PIPE_MAX_SHADER_IMAGES];
bool indirect_addr_consts;
int wpos_transform_const;
 
@@ -3678,7 +3679,8 @@ glsl_to_tgsi_visitor::visit_shared_intrinsic(ir_call *ir)
 static void
 get_image_qualifiers(ir_dereference *ir, const glsl_type **type,
  bool *memory_coherent, bool *memory_volatile,
- bool *memory_restrict, unsigned *image_format)
+ bool *memory_restrict, bool *memory_read_only,
+ unsigned *image_format)
 {
 
switch (ir->ir_type) {
@@ -3694,6 +3696,8 @@ get_image_qualifiers(ir_dereference *ir, const glsl_type 
**type,
  struct_type->fields.structure[fild_idx].memory_volatile;
   *memory_restrict =
  struct_type->fields.structure[fild_idx].memory_restrict;
+  *memory_read_only =
+ struct_type->fields.structure[fild_idx].memory_read_only;
   *image_format =
  struct_type->fields.structure[fild_idx].image_format;
   break;
@@ -3703,7 +3707,7 @@ get_image_qualifiers(ir_dereference *ir, const glsl_type 
**type,
   ir_dereference_array *deref_arr = ir->as_dereference_array();
   get_image_qualifiers((ir_dereference *)deref_arr->array, type,
memory_coherent, memory_volatile, memory_restrict,
-   image_format);
+   memory_read_only, image_format);
   break;
}
 
@@ -3714,6 +3718,7 @@ get_image_qualifiers(ir_dereference *ir, const glsl_type 
**type,
   *memory_coherent = var->data.memory_coherent;
   *memory_volatile = var->data.memory_volatile;
   *memory_restrict = var->data.memory_restrict;
+  *memory_read_only = var->data.memory_read_only;
   *image_format = var->data.image_format;
   break;
}
@@ -3731,12 +3736,13 @@ glsl_to_tgsi_visitor::visit_image_intrinsic(ir_call *ir)
ir_dereference *img = (ir_dereference *)param;
const ir_variable *imgvar = img->variable_referenced();
unsigned sampler_array_size = 1, sampler_base = 0;
-   bool memory_coherent = false, memory_volatile = false, memory_restrict = 
false;
+   bool memory_coherent = false, memory_volatile = false,
+memory_restrict = false, memory_read_only = false;
unsigned image_format = 0;
const glsl_type *type = NULL;
 
get_image_qualifiers(img, , _coherent, _volatile,
-_restrict, _format);
+_restrict, _read_only, _format);
 
st_src_reg reladdr;
st_src_reg image(PROGRAM_IMAGE, 0, GLSL_TYPE_UINT);
@@ -3875,6 +3881,7 @@ glsl_to_tgsi_visitor::visit_image_intrinsic(ir_call *ir)
inst->tex_target = type->sampler_index();
inst->image_format = st_mesa_format_to_pipe_format(st_context(ctx),
  _mesa_get_shader_image_format(image_format));
+   inst->read_only = memory_read_only;
 
if (memory_coherent)
   inst->buffer_access |= TGSI_MEMORY_COHERENT;
@@ -4675,6 +4682,7 @@ count_resources(glsl_to_tgsi_visitor *v, gl_program *prog)
v->image_targets[idx] =
   st_translate_texture_target(inst->tex_target, false);
v->image_formats[idx] = inst->image_format;
+   v->image_wr[idx] = !inst->read_only;
 }
  }
   }
@@ -6770,7 +6778,8 @@ st_translate_program(
  t->images[i] = ureg_DECL_image(ureg, i,
 program->image_targets[i],
 program->image_formats[i],
-true, false);
+program->image_wr[i],
+false);
   }
}
 
diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_private.h 
b/src/mesa/state_tracker/st_glsl_to_tgsi_private.h
index fccb7041cf..356d029f47 100644
--- a/src/mesa/state_trac

Re: [Mesa-dev] [PATCH] virgl: don't send a shader create with no data.

[Erik Faye-Lund]

On on., aug. 29, 2018 at 12:34 AM, Dave Airlie  
wrote:

From: Dave Airlie 

This fixes the situation where we'd send a shader with just the
header and no data.

piglit/glsl-max-varyings test was causing this to happen, and
the renderer fix was breaking it.
---
 src/gallium/drivers/virgl/virgl_encode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/virgl/virgl_encode.c 
b/src/gallium/drivers/virgl/virgl_encode.c

index b56e1f5e428..c1082791af5 100644
--- a/src/gallium/drivers/virgl/virgl_encode.c
+++ b/src/gallium/drivers/virgl/virgl_encode.c
@@ -284,11 +284,12 @@ int virgl_encode_shader_state(struct 
virgl_context *ctx,

while (left_bytes) {
   uint32_t length, offlen;
   int hdr_len = base_hdr_size + (first_pass ? strm_hdr_size : 0);
-  if (ctx->cbuf->cdw + hdr_len + 1 > VIRGL_MAX_CMDBUF_DWORDS)
+  if (ctx->cbuf->cdw + hdr_len + 1 >= VIRGL_MAX_CMDBUF_DWORDS)


Just a suggestion (doesn't affect the rb): You could also just remove 
the "+ 1" for the same effect, no?



  ctx->base.flush(>base, NULL, 0);

   thispass = (VIRGL_MAX_CMDBUF_DWORDS - ctx->cbuf->cdw - hdr_len 
- 1) * 4;


+  fprintf(stderr, "this pass is %d %d %d\n", thispass, 
ctx->cbuf->cdw, hdr_len);


Please either drop this line, or change it to:

if (virgl_debug & VIRGL_DEBUG_VERBOSE)
  debug_printf("this pass is %d %d %d\n", thispass, ctx->cbuf->cdw, 
hdr_len);


...instead.

Otherwise we'll get spewing on stderr, which has been known to cause 
issues for some test-suite runners.


With that fixed:

Reviewed-by: Erik Faye-Lund 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/4] virgl: introduce $VIRGL_DEBUG=verbose

[Erik Faye-Lund]

Sorry, made a mistake when sending this series out. I'll resend 
properly soon.


On Mon, Aug 20, 2018 at 2:03 PM, Erik Faye-Lund 
 wrote:

This adds an environment-varaible that can be used for driver-specific
flags, as well as a flag for it to enable verbose output.

While we're at it, quiet some overly chatty debug-output by default.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_context.c | 6 --
 src/gallium/drivers/virgl/virgl_encode.c  | 3 ++-
 src/gallium/drivers/virgl/virgl_screen.c  | 9 +
 src/gallium/drivers/virgl/virgl_screen.h  | 3 +++
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_context.c 
b/src/gallium/drivers/virgl/virgl_context.c

index dc1dd2d3c2..edc03f5dcf 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -1115,8 +1115,10 @@ static void virgl_get_sample_position(struct 
pipe_context *ctx,

}
out_value[0] = ((bits >> 4) & 0xf) / 16.0f;
out_value[1] = (bits & 0xf) / 16.0f;
-   debug_printf("VIRGL: sample postion [%2d/%2d] = (%f, %f)\n",
-   index, sample_count, out_value[0], out_value[1]);
+
+   if (virgl_debug & VIRGL_DEBUG_VERBOSE)
+  debug_printf("VIRGL: sample postion [%2d/%2d] = (%f, %f)\n",
+   index, sample_count, out_value[0], out_value[1]);
 }

 struct pipe_context *virgl_context_create(struct pipe_screen 
*pscreen,
diff --git a/src/gallium/drivers/virgl/virgl_encode.c 
b/src/gallium/drivers/virgl/virgl_encode.c

index 6b0077ac0c..ea544fe5cd 100644
--- a/src/gallium/drivers/virgl/virgl_encode.c
+++ b/src/gallium/drivers/virgl/virgl_encode.c
@@ -261,7 +261,8 @@ int virgl_encode_shader_state(struct 
virgl_context *ctx,


   bret = tgsi_dump_str(tokens, TGSI_DUMP_FLOAT_AS_HEX, str, 
str_total_size);

   if (bret == false) {
- debug_printf("Failed to translate shader in available space 
- trying again\n");

+ if (virgl_debug & VIRGL_DEBUG_VERBOSE)
+debug_printf("Failed to translate shader in available 
space - trying again\n");

  old_size = str_total_size;
  str_total_size = 65536 * ++retry_size;
  str = REALLOC(str, old_size, str_total_size);
diff --git a/src/gallium/drivers/virgl/virgl_screen.c 
b/src/gallium/drivers/virgl/virgl_screen.c

index 86063c66aa..61147c423c 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -36,6 +36,13 @@
 #include "virgl_public.h"
 #include "virgl_context.h"

+int virgl_debug = 0;
+static const struct debug_named_value debug_options[] = {
+   { "verbose", VIRGL_DEBUG_VERBOSE, NULL },
+   DEBUG_NAMED_VALUE_END
+};
+DEBUG_GET_ONCE_FLAGS_OPTION(virgl_debug, "VIRGL_DEBUG", 
debug_options, 0)

+
 static const char *
 virgl_get_vendor(struct pipe_screen *screen)
 {
@@ -724,6 +731,8 @@ virgl_create_screen(struct virgl_winsys *vws)
if (!screen)
   return NULL;

+   virgl_debug = debug_get_option_virgl_debug();
+
screen->vws = vws;
screen->base.get_name = virgl_get_name;
screen->base.get_vendor = virgl_get_vendor;
diff --git a/src/gallium/drivers/virgl/virgl_screen.h 
b/src/gallium/drivers/virgl/virgl_screen.h

index dcf5816d60..8334d16242 100644
--- a/src/gallium/drivers/virgl/virgl_screen.h
+++ b/src/gallium/drivers/virgl/virgl_screen.h
@@ -27,6 +27,9 @@
 #include "util/slab.h"
 #include "virgl_winsys.h"

+#define VIRGL_DEBUG_VERBOSE 1
+extern int virgl_debug;
+
 struct virgl_screen {
struct pipe_screen base;

--
2.17.1

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

[Mesa-dev] [PATCH 2/4] virgl: replace fprintf-call with debug_printf

[Erik Faye-Lund]

This is the only direct call-site for fprintf in virgl; all other
call-sites call debug_printf instead. So let's follow in style here.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_encode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/virgl/virgl_encode.c 
b/src/gallium/drivers/virgl/virgl_encode.c
index 190c338f45..6b0077ac0c 100644
--- a/src/gallium/drivers/virgl/virgl_encode.c
+++ b/src/gallium/drivers/virgl/virgl_encode.c
@@ -261,7 +261,7 @@ int virgl_encode_shader_state(struct virgl_context *ctx,
 
   bret = tgsi_dump_str(tokens, TGSI_DUMP_FLOAT_AS_HEX, str, 
str_total_size);
   if (bret == false) {
- fprintf(stderr, "Failed to translate shader in available space - 
trying again\n");
+ debug_printf("Failed to translate shader in available space - trying 
again\n");
  old_size = str_total_size;
  str_total_size = 65536 * ++retry_size;
  str = REALLOC(str, old_size, str_total_size);
-- 
2.17.1

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

[Mesa-dev] [PATCH 0/4] virgl: debug printing cleanups

[Erik Faye-Lund]

Here's some patches that cleans up debug-printing in virgl a bit, by
using the existing debug environment-variable infrastructure so things
can be turned on and off explicitly.

This version introduce two flags:
- verbose: enables verbose output. This is useful to get information on
  some internal details, which would usually be overly chatty.
- tgsi: outputs the textual TGSI representation. This is useful for
  debugging virglrenderer when it's often useful to see what TGSI
  triggered for instance a malformed shader.

Both of these are flags for the VIRGL_DEBUG environement variable.

Erik Faye-Lund (4):
  virgl: delete commented out fprintf-call
  virgl: replace fprintf-call with debug_printf
  virgl: introduce $VIRGL_DEBUG=verbose
  virgl: add debug-switch to output TGSI

 src/gallium/drivers/virgl/virgl_context.c |  6 --
 src/gallium/drivers/virgl/virgl_encode.c  |  6 +-
 src/gallium/drivers/virgl/virgl_encode.h  |  1 -
 src/gallium/drivers/virgl/virgl_screen.c  | 10 ++
 src/gallium/drivers/virgl/virgl_screen.h  |  4 
 5 files changed, 23 insertions(+), 4 deletions(-)

-- 
2.17.1

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

[Mesa-dev] [PATCH 1/4] virgl: delete commented out fprintf-call

[Erik Faye-Lund]

This is just debug-cruft left over. Let's just get rid of it.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_encode.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/gallium/drivers/virgl/virgl_encode.h 
b/src/gallium/drivers/virgl/virgl_encode.h
index 749cd33012..999123f426 100644
--- a/src/gallium/drivers/virgl/virgl_encode.h
+++ b/src/gallium/drivers/virgl/virgl_encode.h
@@ -70,7 +70,6 @@ static inline void virgl_encoder_write_block(struct 
virgl_cmd_buf *state,
int x;
memcpy(state->buf + state->cdw, ptr, len);
x = (len % 4);
-//   fprintf(stderr, "[%d] block %d x is %d\n", state->cdw, len, x);
if (x) {
   uint8_t *mp = (uint8_t *)(state->buf + state->cdw);
   mp += len;
-- 
2.17.1

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

[Mesa-dev] [PATCH 3/4] virgl: introduce $VIRGL_DEBUG=verbose

[Erik Faye-Lund]

This adds an environment-varaible that can be used for driver-specific
flags, as well as a flag for it to enable verbose output.

While we're at it, quiet some overly chatty debug-output by default.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_context.c | 6 --
 src/gallium/drivers/virgl/virgl_encode.c  | 3 ++-
 src/gallium/drivers/virgl/virgl_screen.c  | 9 +
 src/gallium/drivers/virgl/virgl_screen.h  | 3 +++
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_context.c 
b/src/gallium/drivers/virgl/virgl_context.c
index dc1dd2d3c2..edc03f5dcf 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -1115,8 +1115,10 @@ static void virgl_get_sample_position(struct 
pipe_context *ctx,
}
out_value[0] = ((bits >> 4) & 0xf) / 16.0f;
out_value[1] = (bits & 0xf) / 16.0f;
-   debug_printf("VIRGL: sample postion [%2d/%2d] = (%f, %f)\n",
-   index, sample_count, out_value[0], out_value[1]);
+
+   if (virgl_debug & VIRGL_DEBUG_VERBOSE)
+  debug_printf("VIRGL: sample postion [%2d/%2d] = (%f, %f)\n",
+   index, sample_count, out_value[0], out_value[1]);
 }
 
 struct pipe_context *virgl_context_create(struct pipe_screen *pscreen,
diff --git a/src/gallium/drivers/virgl/virgl_encode.c 
b/src/gallium/drivers/virgl/virgl_encode.c
index 6b0077ac0c..ea544fe5cd 100644
--- a/src/gallium/drivers/virgl/virgl_encode.c
+++ b/src/gallium/drivers/virgl/virgl_encode.c
@@ -261,7 +261,8 @@ int virgl_encode_shader_state(struct virgl_context *ctx,
 
   bret = tgsi_dump_str(tokens, TGSI_DUMP_FLOAT_AS_HEX, str, 
str_total_size);
   if (bret == false) {
- debug_printf("Failed to translate shader in available space - trying 
again\n");
+ if (virgl_debug & VIRGL_DEBUG_VERBOSE)
+debug_printf("Failed to translate shader in available space - 
trying again\n");
  old_size = str_total_size;
  str_total_size = 65536 * ++retry_size;
  str = REALLOC(str, old_size, str_total_size);
diff --git a/src/gallium/drivers/virgl/virgl_screen.c 
b/src/gallium/drivers/virgl/virgl_screen.c
index 86063c66aa..61147c423c 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -36,6 +36,13 @@
 #include "virgl_public.h"
 #include "virgl_context.h"
 
+int virgl_debug = 0;
+static const struct debug_named_value debug_options[] = {
+   { "verbose", VIRGL_DEBUG_VERBOSE, NULL },
+   DEBUG_NAMED_VALUE_END
+};
+DEBUG_GET_ONCE_FLAGS_OPTION(virgl_debug, "VIRGL_DEBUG", debug_options, 0)
+
 static const char *
 virgl_get_vendor(struct pipe_screen *screen)
 {
@@ -724,6 +731,8 @@ virgl_create_screen(struct virgl_winsys *vws)
if (!screen)
   return NULL;
 
+   virgl_debug = debug_get_option_virgl_debug();
+
screen->vws = vws;
screen->base.get_name = virgl_get_name;
screen->base.get_vendor = virgl_get_vendor;
diff --git a/src/gallium/drivers/virgl/virgl_screen.h 
b/src/gallium/drivers/virgl/virgl_screen.h
index dcf5816d60..8334d16242 100644
--- a/src/gallium/drivers/virgl/virgl_screen.h
+++ b/src/gallium/drivers/virgl/virgl_screen.h
@@ -27,6 +27,9 @@
 #include "util/slab.h"
 #include "virgl_winsys.h"
 
+#define VIRGL_DEBUG_VERBOSE 1
+extern int virgl_debug;
+
 struct virgl_screen {
struct pipe_screen base;
 
-- 
2.17.1

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

[Mesa-dev] [PATCH 1/4] virgl: delete commented out fprintf-call

[Erik Faye-Lund]

This is just debug-cruft left over. Let's just get rid of it.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_encode.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/gallium/drivers/virgl/virgl_encode.h 
b/src/gallium/drivers/virgl/virgl_encode.h
index 749cd33012..999123f426 100644
--- a/src/gallium/drivers/virgl/virgl_encode.h
+++ b/src/gallium/drivers/virgl/virgl_encode.h
@@ -70,7 +70,6 @@ static inline void virgl_encoder_write_block(struct 
virgl_cmd_buf *state,
int x;
memcpy(state->buf + state->cdw, ptr, len);
x = (len % 4);
-//   fprintf(stderr, "[%d] block %d x is %d\n", state->cdw, len, x);
if (x) {
   uint8_t *mp = (uint8_t *)(state->buf + state->cdw);
   mp += len;
-- 
2.17.1

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

[Mesa-dev] [PATCH 2/4] virgl: replace fprintf-call with debug_printf

[Erik Faye-Lund]

This is the only direct call-site for fprintf in virgl; all other
call-sites call debug_printf instead. So let's follow in style here.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_encode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/virgl/virgl_encode.c 
b/src/gallium/drivers/virgl/virgl_encode.c
index 190c338f45..6b0077ac0c 100644
--- a/src/gallium/drivers/virgl/virgl_encode.c
+++ b/src/gallium/drivers/virgl/virgl_encode.c
@@ -261,7 +261,7 @@ int virgl_encode_shader_state(struct virgl_context *ctx,
 
   bret = tgsi_dump_str(tokens, TGSI_DUMP_FLOAT_AS_HEX, str, 
str_total_size);
   if (bret == false) {
- fprintf(stderr, "Failed to translate shader in available space - 
trying again\n");
+ debug_printf("Failed to translate shader in available space - trying 
again\n");
  old_size = str_total_size;
  str_total_size = 65536 * ++retry_size;
  str = REALLOC(str, old_size, str_total_size);
-- 
2.17.1

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

[Mesa-dev] [PATCH 4/4] virgl: add debug-switch to output TGSI

[Erik Faye-Lund]

This is quite useful for debugging shader-transpiling issues in
virglrenderer.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_encode.c | 3 +++
 src/gallium/drivers/virgl/virgl_screen.c | 1 +
 src/gallium/drivers/virgl/virgl_screen.h | 1 +
 3 files changed, 5 insertions(+)

diff --git a/src/gallium/drivers/virgl/virgl_encode.c 
b/src/gallium/drivers/virgl/virgl_encode.c
index ea544fe5cd..4e99dda0f7 100644
--- a/src/gallium/drivers/virgl/virgl_encode.c
+++ b/src/gallium/drivers/virgl/virgl_encode.c
@@ -274,6 +274,9 @@ int virgl_encode_shader_state(struct virgl_context *ctx,
if (bret == false)
   return -1;
 
+   if (virgl_debug & VIRGL_DEBUG_TGSI)
+  debug_printf("TGSI:\n---8<---\n%s\n---8<---\n", str);
+
shader_len = strlen(str) + 1;
 
left_bytes = shader_len;
diff --git a/src/gallium/drivers/virgl/virgl_screen.c 
b/src/gallium/drivers/virgl/virgl_screen.c
index 61147c423c..d9b50c9cca 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -39,6 +39,7 @@
 int virgl_debug = 0;
 static const struct debug_named_value debug_options[] = {
{ "verbose", VIRGL_DEBUG_VERBOSE, NULL },
+   { "tgsi", VIRGL_DEBUG_TGSI, NULL },
DEBUG_NAMED_VALUE_END
 };
 DEBUG_GET_ONCE_FLAGS_OPTION(virgl_debug, "VIRGL_DEBUG", debug_options, 0)
diff --git a/src/gallium/drivers/virgl/virgl_screen.h 
b/src/gallium/drivers/virgl/virgl_screen.h
index 8334d16242..719f5166d7 100644
--- a/src/gallium/drivers/virgl/virgl_screen.h
+++ b/src/gallium/drivers/virgl/virgl_screen.h
@@ -28,6 +28,7 @@
 #include "virgl_winsys.h"
 
 #define VIRGL_DEBUG_VERBOSE 1
+#define VIRGL_DEBUG_TGSI2
 extern int virgl_debug;
 
 struct virgl_screen {
-- 
2.17.1

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

[Mesa-dev] [PATCH 3/4] virgl: introduce $VIRGL_DEBUG=verbose

[Erik Faye-Lund]

This adds an environment-varaible that can be used for driver-specific
flags, as well as a flag for it to enable verbose output.

While we're at it, quiet some overly chatty debug-output by default.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_context.c | 6 --
 src/gallium/drivers/virgl/virgl_encode.c  | 3 ++-
 src/gallium/drivers/virgl/virgl_screen.c  | 9 +
 src/gallium/drivers/virgl/virgl_screen.h  | 3 +++
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_context.c 
b/src/gallium/drivers/virgl/virgl_context.c
index dc1dd2d3c2..edc03f5dcf 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -1115,8 +1115,10 @@ static void virgl_get_sample_position(struct 
pipe_context *ctx,
}
out_value[0] = ((bits >> 4) & 0xf) / 16.0f;
out_value[1] = (bits & 0xf) / 16.0f;
-   debug_printf("VIRGL: sample postion [%2d/%2d] = (%f, %f)\n",
-   index, sample_count, out_value[0], out_value[1]);
+
+   if (virgl_debug & VIRGL_DEBUG_VERBOSE)
+  debug_printf("VIRGL: sample postion [%2d/%2d] = (%f, %f)\n",
+   index, sample_count, out_value[0], out_value[1]);
 }
 
 struct pipe_context *virgl_context_create(struct pipe_screen *pscreen,
diff --git a/src/gallium/drivers/virgl/virgl_encode.c 
b/src/gallium/drivers/virgl/virgl_encode.c
index 6b0077ac0c..ea544fe5cd 100644
--- a/src/gallium/drivers/virgl/virgl_encode.c
+++ b/src/gallium/drivers/virgl/virgl_encode.c
@@ -261,7 +261,8 @@ int virgl_encode_shader_state(struct virgl_context *ctx,
 
   bret = tgsi_dump_str(tokens, TGSI_DUMP_FLOAT_AS_HEX, str, 
str_total_size);
   if (bret == false) {
- debug_printf("Failed to translate shader in available space - trying 
again\n");
+ if (virgl_debug & VIRGL_DEBUG_VERBOSE)
+debug_printf("Failed to translate shader in available space - 
trying again\n");
  old_size = str_total_size;
  str_total_size = 65536 * ++retry_size;
  str = REALLOC(str, old_size, str_total_size);
diff --git a/src/gallium/drivers/virgl/virgl_screen.c 
b/src/gallium/drivers/virgl/virgl_screen.c
index 86063c66aa..61147c423c 100644
--- a/src/gallium/drivers/virgl/virgl_screen.c
+++ b/src/gallium/drivers/virgl/virgl_screen.c
@@ -36,6 +36,13 @@
 #include "virgl_public.h"
 #include "virgl_context.h"
 
+int virgl_debug = 0;
+static const struct debug_named_value debug_options[] = {
+   { "verbose", VIRGL_DEBUG_VERBOSE, NULL },
+   DEBUG_NAMED_VALUE_END
+};
+DEBUG_GET_ONCE_FLAGS_OPTION(virgl_debug, "VIRGL_DEBUG", debug_options, 0)
+
 static const char *
 virgl_get_vendor(struct pipe_screen *screen)
 {
@@ -724,6 +731,8 @@ virgl_create_screen(struct virgl_winsys *vws)
if (!screen)
   return NULL;
 
+   virgl_debug = debug_get_option_virgl_debug();
+
screen->vws = vws;
screen->base.get_name = virgl_get_name;
screen->base.get_vendor = virgl_get_vendor;
diff --git a/src/gallium/drivers/virgl/virgl_screen.h 
b/src/gallium/drivers/virgl/virgl_screen.h
index dcf5816d60..8334d16242 100644
--- a/src/gallium/drivers/virgl/virgl_screen.h
+++ b/src/gallium/drivers/virgl/virgl_screen.h
@@ -27,6 +27,9 @@
 #include "util/slab.h"
 #include "virgl_winsys.h"
 
+#define VIRGL_DEBUG_VERBOSE 1
+extern int virgl_debug;
+
 struct virgl_screen {
struct pipe_screen base;
 
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH 4/4] virgl: add debug-switch to output TGSI

[Erik Faye-Lund]

On ma., aug. 27, 2018 at 9:49 AM, Gert Wollny 
 wrote:

Am Montag, den 20.08.2018, 14:10 +0200 schrieb Erik Faye-Lund:

 This is quite useful for debugging shader-transpiling issues in
 virglrenderer.

Isn't this coverted by ST_DEBUG=tgsi?



Ah, I wasn't even aware of this.

There's one minor difference here, though: virgl does some 
transformations on the TGSI before passing it to the host. It can 
sometimes be useful to see the difference.



Also, virglrenderer has a variable vrend_dump_shaders in
vrend_renderer.c that enables dumping all the TGSI + the created GLSL
shaders.


Yeah, and that's also useful. But there's two differences here:
- vrend_dump_shaders dumps the TGSI *after* parsing, comparing the 
over-the-wire TGSI and the parsed TGSI has helped me in the past to 
find flags being culled during parsing etc in the past.
- vrend_dump_shaders needs a recompile of virglrenderer, which makes it 
a lot more inconvenient from a turn-around point of view if running on 
qemu for instance.


I don't know, perhaps this is a bit too many similar features? I can 
certainly drop this patch for now.


Another (maybe more useful) option could be to allow a per-client 
override of the vrend_dump_shaders-functionality that could be enabled 
from the host?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH/RFC] glsl: allow redeclaring variables as 'precise invariant'

[Erik Faye-Lund]

There's seems to be nothing in the GLSL (ES) specifications that diallow
redeclaring a variable as both 'precise' and 'invariant' in the same
statement. But the way the parse-rules are structured this fails to
parse, because this is handled in single_declaration, which has an
exhaustive list of qualifiers here, and it does not include an option
with both.

So let's factor out the precise/invariant handling from the
type_qualifier rule so we can reuse it, as there's some intricate
subtleties here.

For reference: glslangValidator already allows this.

Signed-off-by: Erik Faye-Lund 
---

My yacc/bison skills aren't all that, so I wouldn't be too surprised if
this the wrong approach. This is why I'm posting this as an RFC. I'm
hoping someone can tell me if this is the right kind of approach or not.

This fixes a failure to compile a shader that appears in virgl after
an invariant output has been lowered to precise TGSI-opcodes, and
re-emitted GLSL for it (in virglrenderer). The problematic shader comes from
dEQP-GLES2.functional.shaders.invariance.highp.subexpression_precision_lowp.

 src/compiler/glsl/glsl_parser.yy | 77 +++-
 1 file changed, 36 insertions(+), 41 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
index cb7376995d..042e654a26 100644
--- a/src/compiler/glsl/glsl_parser.yy
+++ b/src/compiler/glsl/glsl_parser.yy
@@ -187,6 +187,7 @@ static bool match_layout_qualifier(const char *s1, const 
char *s2,
 %type  statement_list
 %type  simple_statement
 %type  precision_qualifier
+%type  invariant_or_precise_qualifier
 %type  type_qualifier
 %type  auxiliary_storage_qualifier
 %type  storage_qualifier
@@ -1106,7 +1107,7 @@ single_declaration:
   $$->declarations.push_tail(>link);
   state->symbols->add_variable(new(state) ir_variable(NULL, $2, 
ir_var_auto));
}
-   | INVARIANT variable_identifier
+   | invariant_or_precise_qualifier variable_identifier
{
   void *ctx = state->linalloc;
   ast_declaration *decl = new(ctx) ast_declaration($2, NULL, NULL);
@@ -1114,19 +1115,8 @@ single_declaration:
 
   $$ = new(ctx) ast_declarator_list(NULL);
   $$->set_location_range(@1, @2);
-  $$->invariant = true;
-
-  $$->declarations.push_tail(>link);
-   }
-   | PRECISE variable_identifier
-   {
-  void *ctx = state->linalloc;
-  ast_declaration *decl = new(ctx) ast_declaration($2, NULL, NULL);
-  decl->set_location(@2);
-
-  $$ = new(ctx) ast_declarator_list(NULL);
-  $$->set_location_range(@1, @2);
-  $$->precise = true;
+  $$->invariant = $1.flags.q.invariant;
+  $$->precise = $1.flags.q.precise;
 
   $$->declarations.push_tail(>link);
}
@@ -1889,7 +1879,7 @@ interpolation_qualifier:
}
;
 
-type_qualifier:
+invariant_or_precise_qualifier:
/* Single qualifiers */
INVARIANT
{
@@ -1901,31 +1891,7 @@ type_qualifier:
   memset(& $$, 0, sizeof($$));
   $$.flags.q.precise = 1;
}
-   | auxiliary_storage_qualifier
-   | storage_qualifier
-   | interpolation_qualifier
-   | layout_qualifier
-   | memory_qualifier
-   | subroutine_qualifier
-   | precision_qualifier
-   {
-  memset(&$$, 0, sizeof($$));
-  $$.precision = $1;
-   }
-
-   /* Multiple qualifiers:
-* In GLSL 4.20, these can be specified in any order.  In earlier versions,
-* they appear in this order (see GLSL 1.50 section 4.7 & comments below):
-*
-*invariant interpolation auxiliary storage precision  ...or...
-*layout storage precision
-*
-* Each qualifier's rule ensures that the accumulated qualifiers on the 
right
-* side don't contain any that must appear on the left hand side.
-* For example, when processing a storage qualifier, we check that there are
-* no auxiliary, interpolation, layout, invariant, or precise qualifiers to 
the right.
-*/
-   | PRECISE type_qualifier
+   | PRECISE invariant_or_precise_qualifier
{
   if ($2.flags.q.precise)
  _mesa_glsl_error(&@1, state, "duplicate \"precise\" qualifier");
@@ -1933,7 +1899,7 @@ type_qualifier:
   $$ = $2;
   $$.flags.q.precise = 1;
}
-   | INVARIANT type_qualifier
+   | INVARIANT invariant_or_precise_qualifier
{
   if ($2.flags.q.invariant)
  _mesa_glsl_error(&@1, state, "duplicate \"invariant\" qualifier");
@@ -1958,6 +1924,35 @@ type_qualifier:
   if (state->is_version(430, 300) && $$.flags.q.in)
  _mesa_glsl_error(&@1, state, "invariant qualifiers cannot be used 
with shader inputs");
}
+
+type_qualifier:
+   /* Single qualifiers */
+   invariant_or_precise_qualifier
+   | auxiliary_storage_qualifier
+   | storage_qualifier
+   | interpolation_qualifier
+   | layout_qualifier
+   | memory_qualifier
+   | subroutine_qualifier
+   | precision_qualifier
+   {
+   

Re: [Mesa-dev] [PATCH/RFC] glsl: allow redeclaring variables as 'precise invariant'

[Erik Faye-Lund]

Ping?

On on., aug. 22, 2018 at 7:34 PM, Erik Faye-Lund 
 wrote:
There's seems to be nothing in the GLSL (ES) specifications that 
diallow

redeclaring a variable as both 'precise' and 'invariant' in the same
statement. But the way the parse-rules are structured this fails to
parse, because this is handled in single_declaration, which has an
exhaustive list of qualifiers here, and it does not include an option
with both.

So let's factor out the precise/invariant handling from the
type_qualifier rule so we can reuse it, as there's some intricate
subtleties here.

For reference: glslangValidator already allows this.

Signed-off-by: Erik Faye-Lund 
---

My yacc/bison skills aren't all that, so I wouldn't be too surprised 
if

this the wrong approach. This is why I'm posting this as an RFC. I'm
hoping someone can tell me if this is the right kind of approach or 
not.


This fixes a failure to compile a shader that appears in virgl after
an invariant output has been lowered to precise TGSI-opcodes, and
re-emitted GLSL for it (in virglrenderer). The problematic shader 
comes from

dEQP-GLES2.functional.shaders.invariance.highp.subexpression_precision_lowp.

 src/compiler/glsl/glsl_parser.yy | 77 
+++-

 1 file changed, 36 insertions(+), 41 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser.yy 
b/src/compiler/glsl/glsl_parser.yy

index cb7376995d..042e654a26 100644
--- a/src/compiler/glsl/glsl_parser.yy
+++ b/src/compiler/glsl/glsl_parser.yy
@@ -187,6 +187,7 @@ static bool match_layout_qualifier(const char 
*s1, const char *s2,

 %type  statement_list
 %type  simple_statement
 %type  precision_qualifier
+%type  invariant_or_precise_qualifier
 %type  type_qualifier
 %type  auxiliary_storage_qualifier
 %type  storage_qualifier
@@ -1106,7 +1107,7 @@ single_declaration:
   $$->declarations.push_tail(>link);
   state->symbols->add_variable(new(state) ir_variable(NULL, $2, 
ir_var_auto));

}
-   | INVARIANT variable_identifier
+   | invariant_or_precise_qualifier variable_identifier
{
   void *ctx = state->linalloc;
   ast_declaration *decl = new(ctx) ast_declaration($2, NULL, 
NULL);

@@ -1114,19 +1115,8 @@ single_declaration:

   $$ = new(ctx) ast_declarator_list(NULL);
   $$->set_location_range(@1, @2);
-  $$->invariant = true;
-
-  $$->declarations.push_tail(>link);
-   }
-   | PRECISE variable_identifier
-   {
-  void *ctx = state->linalloc;
-  ast_declaration *decl = new(ctx) ast_declaration($2, NULL, 
NULL);

-  decl->set_location(@2);
-
-  $$ = new(ctx) ast_declarator_list(NULL);
-  $$->set_location_range(@1, @2);
-  $$->precise = true;
+  $$->invariant = $1.flags.q.invariant;
+  $$->precise = $1.flags.q.precise;

   $$->declarations.push_tail(>link);
}
@@ -1889,7 +1879,7 @@ interpolation_qualifier:
}
;

-type_qualifier:
+invariant_or_precise_qualifier:
/* Single qualifiers */
INVARIANT
{
@@ -1901,31 +1891,7 @@ type_qualifier:
   memset(& $$, 0, sizeof($$));
   $$.flags.q.precise = 1;
}
-   | auxiliary_storage_qualifier
-   | storage_qualifier
-   | interpolation_qualifier
-   | layout_qualifier
-   | memory_qualifier
-   | subroutine_qualifier
-   | precision_qualifier
-   {
-  memset(&$$, 0, sizeof($$));
-  $$.precision = $1;
-   }
-
-   /* Multiple qualifiers:
-* In GLSL 4.20, these can be specified in any order.  In earlier 
versions,
-* they appear in this order (see GLSL 1.50 section 4.7 & 
comments below):

-*
-*invariant interpolation auxiliary storage precision  
...or...

-*layout storage precision
-*
-* Each qualifier's rule ensures that the accumulated qualifiers 
on the right

-* side don't contain any that must appear on the left hand side.
-* For example, when processing a storage qualifier, we check 
that there are
-* no auxiliary, interpolation, layout, invariant, or precise 
qualifiers to the right.

-*/
-   | PRECISE type_qualifier
+   | PRECISE invariant_or_precise_qualifier
{
   if ($2.flags.q.precise)
  _mesa_glsl_error(&@1, state, "duplicate \"precise\" 
qualifier");

@@ -1933,7 +1899,7 @@ type_qualifier:
   $$ = $2;
   $$.flags.q.precise = 1;
}
-   | INVARIANT type_qualifier
+   | INVARIANT invariant_or_precise_qualifier
{
   if ($2.flags.q.invariant)
  _mesa_glsl_error(&@1, state, "duplicate \"invariant\" 
qualifier");

@@ -1958,6 +1924,35 @@ type_qualifier:
   if (state->is_version(430, 300) && $$.flags.q.in)
  _mesa_glsl_error(&@1, state, "invariant qualifiers cannot 
be used with shader inputs");

}
+
+type_qualifier:
+   /* Single qualifiers */
+   invariant_or_precise_qualifier
+   | auxiliary_storage_qualifier
+   | storage_qualifier
+   | interpolation_qualifi

[Mesa-dev] [PATCH] virgl: implement set_min_samples

[Erik Faye-Lund]

This allows us to implement glMinSampleShading correctly, which up
until now just got ignored.

Signed-off-by: Erik Faye-Lund 
---

This implements the mesa-side of VIRGL_CCMD_SET_MIN_SAMPLES, which
is already in the master-branch of virglre...

 src/gallium/drivers/virgl/virgl_context.c  | 12 
 src/gallium/drivers/virgl/virgl_encode.c   |  7 +++
 src/gallium/drivers/virgl/virgl_encode.h   |  3 +++
 src/gallium/drivers/virgl/virgl_hw.h   |  1 +
 src/gallium/drivers/virgl/virgl_protocol.h |  5 +
 5 files changed, 28 insertions(+)

diff --git a/src/gallium/drivers/virgl/virgl_context.c 
b/src/gallium/drivers/virgl/virgl_context.c
index d985bf906f..6f0695ee2c 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -844,6 +844,17 @@ static void virgl_set_sample_mask(struct pipe_context *ctx,
virgl_encoder_set_sample_mask(vctx, sample_mask);
 }
 
+static void virgl_set_min_samples(struct pipe_context *ctx,
+ unsigned min_samples)
+{
+   struct virgl_context *vctx = virgl_context(ctx);
+   struct virgl_screen *rs = virgl_screen(ctx->screen);
+
+   if (!(rs->caps.caps.v2.capability_bits & VIRGL_CAP_SET_MIN_SAMPLES))
+  return;
+   virgl_encoder_set_min_samples(vctx, min_samples);
+}
+
 static void virgl_set_clip_state(struct pipe_context *ctx,
 const struct pipe_clip_state *clip)
 {
@@ -1025,6 +1036,7 @@ struct pipe_context *virgl_context_create(struct 
pipe_screen *pscreen,
vctx->base.set_polygon_stipple = virgl_set_polygon_stipple;
vctx->base.set_scissor_states = virgl_set_scissor_states;
vctx->base.set_sample_mask = virgl_set_sample_mask;
+   vctx->base.set_min_samples = virgl_set_min_samples;
vctx->base.set_stencil_ref = virgl_set_stencil_ref;
vctx->base.set_clip_state = virgl_set_clip_state;
 
diff --git a/src/gallium/drivers/virgl/virgl_encode.c 
b/src/gallium/drivers/virgl/virgl_encode.c
index 6b800d3d07..c7c6b1e7d3 100644
--- a/src/gallium/drivers/virgl/virgl_encode.c
+++ b/src/gallium/drivers/virgl/virgl_encode.c
@@ -726,6 +726,13 @@ void virgl_encoder_set_sample_mask(struct virgl_context 
*ctx,
virgl_encoder_write_dword(ctx->cbuf, sample_mask);
 }
 
+void virgl_encoder_set_min_samples(struct virgl_context *ctx,
+  unsigned min_samples)
+{
+   virgl_encoder_write_cmd_dword(ctx, VIRGL_CMD0(VIRGL_CCMD_SET_MIN_SAMPLES, 
0, VIRGL_SET_MIN_SAMPLES_SIZE));
+   virgl_encoder_write_dword(ctx->cbuf, min_samples);
+}
+
 void virgl_encoder_set_clip_state(struct virgl_context *ctx,
  const struct pipe_clip_state *clip)
 {
diff --git a/src/gallium/drivers/virgl/virgl_encode.h 
b/src/gallium/drivers/virgl/virgl_encode.h
index 837075ea48..21c506eb56 100644
--- a/src/gallium/drivers/virgl/virgl_encode.h
+++ b/src/gallium/drivers/virgl/virgl_encode.h
@@ -211,6 +211,9 @@ void virgl_encoder_set_polygon_stipple(struct virgl_context 
*ctx,
 void virgl_encoder_set_sample_mask(struct virgl_context *ctx,
   unsigned sample_mask);
 
+void virgl_encoder_set_min_samples(struct virgl_context *ctx,
+  unsigned min_samples);
+
 void virgl_encoder_set_clip_state(struct virgl_context *ctx,
  const struct pipe_clip_state *clip);
 
diff --git a/src/gallium/drivers/virgl/virgl_hw.h 
b/src/gallium/drivers/virgl/virgl_hw.h
index 157267558a..118249e695 100644
--- a/src/gallium/drivers/virgl/virgl_hw.h
+++ b/src/gallium/drivers/virgl/virgl_hw.h
@@ -201,6 +201,7 @@ enum virgl_formats {
 #define VIRGL_CAP_NONE 0
 #define VIRGL_CAP_TGSI_INVARIANT   (1 << 0)
 #define VIRGL_CAP_TEXTURE_VIEW (1 << 1)
+#define VIRGL_CAP_SET_MIN_SAMPLES  (1 << 2)
 
 #define VIRGL_BIND_DEPTH_STENCIL (1 << 0)
 #define VIRGL_BIND_RENDER_TARGET (1 << 1)
diff --git a/src/gallium/drivers/virgl/virgl_protocol.h 
b/src/gallium/drivers/virgl/virgl_protocol.h
index bd5a8b4043..53493d5c15 100644
--- a/src/gallium/drivers/virgl/virgl_protocol.h
+++ b/src/gallium/drivers/virgl/virgl_protocol.h
@@ -85,6 +85,7 @@ enum virgl_context_cmd {
VIRGL_CCMD_BIND_SHADER,
 
VIRGL_CCMD_SET_TESS_STATE,
+   VIRGL_CCMD_SET_MIN_SAMPLES,
 };
 
 /*
@@ -486,4 +487,8 @@ enum virgl_context_cmd {
 /* tess state */
 #define VIRGL_TESS_STATE_SIZE 6
 
+/* set min samples */
+#define VIRGL_SET_MIN_SAMPLES_SIZE 1
+#define VIRGL_SET_MIN_SAMPLES_MASK 1
+
 #endif
-- 
2.18.0.rc2

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

[Mesa-dev] [PATCH 0/3] virgl: synchronize virgl_hw.h with virglrenderer

[Erik Faye-Lund]

Here's a few patches to get the delta between our copy of virgl_hw.h and the
copy in the virglrenderer repository as small as possible.

The goal is to make it easier to track this file in the future by simply
copying the new version of the file on top of the old.

I've left out one new cap that currently exists in master
(VIRGL_CAP_SET_MIN_SAMPLES), for two reasons:
reasons:

1. It wasn't yet in master when I generated this patch-set
2. I'm about to send out a patch-set that adds it (and respects it)
   separately.

Erik Faye-Lund (3):
  virgl: move bind-flags to virgl_winsys.h
  virgl: rename msaa_sample_positions -> sample_locations
  virgl: update virgl_hw.h from virglrenderer

 src/gallium/drivers/virgl/virgl_context.c  |  8 +++---
 src/gallium/drivers/virgl/virgl_hw.h   | 33 +-
 src/gallium/drivers/virgl/virgl_resource.h |  2 ++
 src/gallium/drivers/virgl/virgl_winsys.h   | 12 
 4 files changed, 37 insertions(+), 18 deletions(-)

-- 
2.18.0.rc2

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

[Mesa-dev] [PATCH 3/3] virgl: update virgl_hw.h from virglrenderer

[Erik Faye-Lund]

This just makes sure we're currently up-to-date with what
virglrenderer has.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_hw.h | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/virgl/virgl_hw.h 
b/src/gallium/drivers/virgl/virgl_hw.h
index 086757def6..ff6a3430e5 100644
--- a/src/gallium/drivers/virgl/virgl_hw.h
+++ b/src/gallium/drivers/virgl/virgl_hw.h
@@ -37,6 +37,7 @@ enum virgl_formats {
VIRGL_FORMAT_B5G5R5A1_UNORM  = 5,
VIRGL_FORMAT_B4G4R4A4_UNORM  = 6,
VIRGL_FORMAT_B5G6R5_UNORM= 7,
+   VIRGL_FORMAT_R10G10B10A2_UNORM   = 8,
VIRGL_FORMAT_L8_UNORM= 9,/**< ubyte luminance */
VIRGL_FORMAT_A8_UNORM= 10,   /**< ubyte alpha */
VIRGL_FORMAT_L8A8_UNORM  = 12,   /**< ubyte alpha, luminance */
@@ -112,6 +113,8 @@ enum virgl_formats {
VIRGL_FORMAT_B10G10R10A2_UNORM   = 131,
VIRGL_FORMAT_R8G8B8X8_UNORM  = 134,
VIRGL_FORMAT_B4G4R4X4_UNORM  = 135,
+   VIRGL_FORMAT_X24S8_UINT  = 136,
+   VIRGL_FORMAT_S8X24_UINT  = 137,
VIRGL_FORMAT_B2G3R3_UNORM= 139,
 
VIRGL_FORMAT_L16A16_UNORM= 140,
@@ -186,7 +189,7 @@ enum virgl_formats {
VIRGL_FORMAT_L32_SINT= 223,
VIRGL_FORMAT_L32A32_SINT = 224,
 
-   VIRGL_FORMAT_B10G10R10A2_UINT= 225, 
+   VIRGL_FORMAT_B10G10R10A2_UINT= 225,
VIRGL_FORMAT_R8G8B8X8_SNORM  = 229,
 
VIRGL_FORMAT_R8G8B8X8_SRGB   = 230,
@@ -194,6 +197,16 @@ enum virgl_formats {
VIRGL_FORMAT_B10G10R10X2_UNORM   = 233,
VIRGL_FORMAT_R16G16B16X16_UNORM  = 234,
VIRGL_FORMAT_R16G16B16X16_SNORM  = 235,
+
+   VIRGL_FORMAT_R10G10B10A2_UINT= 253,
+
+   VIRGL_FORMAT_BPTC_RGBA_UNORM = 255,
+   VIRGL_FORMAT_BPTC_SRGBA  = 256,
+   VIRGL_FORMAT_BPTC_RGB_FLOAT  = 257,
+   VIRGL_FORMAT_BPTC_RGB_UFLOAT = 258,
+
+   VIRGL_FORMAT_R10G10B10X2_UNORM   = 308,
+   VIRGL_FORMAT_A4B4G4R4_UNORM  = 311,
VIRGL_FORMAT_MAX,
 };
 
@@ -263,6 +276,10 @@ struct virgl_caps_v1 {
 uint32_t max_texture_gather_components;
 };
 
+/*
+ * This struct should be growable when used in capset 2,
+ * so we shouldn't have to add a v3 ever.
+ */
 struct virgl_caps_v2 {
 struct virgl_caps_v1 v1;
 float min_aliased_point_size;
-- 
2.18.0.rc2

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

[Mesa-dev] [PATCH 1/3] virgl: move bind-flags to virgl_winsys.h

[Erik Faye-Lund]

virglrenderer's virgl_hw.h doesn't contain these, and it's the
virgl winsys that cares about these, so let's move them there.

This reduces the diff between the two different versions of
virgl_hw.h, and should make it easier to upgrade the file in
the future.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_hw.h   | 12 
 src/gallium/drivers/virgl/virgl_resource.h |  2 ++
 src/gallium/drivers/virgl/virgl_winsys.h   | 12 
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_hw.h 
b/src/gallium/drivers/virgl/virgl_hw.h
index 157267558a..d0df23e2f6 100644
--- a/src/gallium/drivers/virgl/virgl_hw.h
+++ b/src/gallium/drivers/virgl/virgl_hw.h
@@ -202,18 +202,6 @@ enum virgl_formats {
 #define VIRGL_CAP_TGSI_INVARIANT   (1 << 0)
 #define VIRGL_CAP_TEXTURE_VIEW (1 << 1)
 
-#define VIRGL_BIND_DEPTH_STENCIL (1 << 0)
-#define VIRGL_BIND_RENDER_TARGET (1 << 1)
-#define VIRGL_BIND_SAMPLER_VIEW  (1 << 3)
-#define VIRGL_BIND_VERTEX_BUFFER (1 << 4)
-#define VIRGL_BIND_INDEX_BUFFER  (1 << 5)
-#define VIRGL_BIND_CONSTANT_BUFFER (1 << 6)
-#define VIRGL_BIND_DISPLAY_TARGET (1 << 7)
-#define VIRGL_BIND_STREAM_OUTPUT (1 << 11)
-#define VIRGL_BIND_CURSOR(1 << 16)
-#define VIRGL_BIND_CUSTOM(1 << 17)
-#define VIRGL_BIND_SCANOUT   (1 << 18)
-
 struct virgl_caps_bool_set1 {
 unsigned indep_blend_enable:1;
 unsigned indep_blend_func:1;
diff --git a/src/gallium/drivers/virgl/virgl_resource.h 
b/src/gallium/drivers/virgl/virgl_resource.h
index bab9bcb9b4..d9ad4733b4 100644
--- a/src/gallium/drivers/virgl/virgl_resource.h
+++ b/src/gallium/drivers/virgl/virgl_resource.h
@@ -30,6 +30,8 @@
 #include "util/u_transfer.h"
 
 #include "virgl_hw.h"
+#include "virgl_winsys.h"
+
 #define VR_MAX_TEXTURE_2D_LEVELS 15
 
 struct winsys_handle;
diff --git a/src/gallium/drivers/virgl/virgl_winsys.h 
b/src/gallium/drivers/virgl/virgl_winsys.h
index 99ab4d3840..acd68c060e 100644
--- a/src/gallium/drivers/virgl/virgl_winsys.h
+++ b/src/gallium/drivers/virgl/virgl_winsys.h
@@ -42,6 +42,18 @@ struct virgl_cmd_buf {
uint32_t *buf;
 };
 
+#define VIRGL_BIND_DEPTH_STENCIL (1 << 0)
+#define VIRGL_BIND_RENDER_TARGET (1 << 1)
+#define VIRGL_BIND_SAMPLER_VIEW  (1 << 3)
+#define VIRGL_BIND_VERTEX_BUFFER (1 << 4)
+#define VIRGL_BIND_INDEX_BUFFER  (1 << 5)
+#define VIRGL_BIND_CONSTANT_BUFFER (1 << 6)
+#define VIRGL_BIND_DISPLAY_TARGET (1 << 7)
+#define VIRGL_BIND_STREAM_OUTPUT (1 << 11)
+#define VIRGL_BIND_CURSOR(1 << 16)
+#define VIRGL_BIND_CUSTOM(1 << 17)
+#define VIRGL_BIND_SCANOUT   (1 << 18)
+
 struct virgl_winsys {
unsigned pci_id;
 
-- 
2.18.0.rc2

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

[Mesa-dev] [PATCH 2/3] virgl: rename msaa_sample_positions -> sample_locations

[Erik Faye-Lund]

This matches what this field is called in virglrenderer's copy of
this.

While we're at it, fixup the indentation.

This reduces the diff between the two different versions of
virgl_hw.h, and should make it easier to upgrade the file in
the future.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_context.c | 8 
 src/gallium/drivers/virgl/virgl_hw.h  | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_context.c 
b/src/gallium/drivers/virgl/virgl_context.c
index d985bf906f..b1e2f3ee42 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -942,13 +942,13 @@ static void virgl_get_sample_position(struct pipe_context 
*ctx,
   out_value[0] = out_value[1] = 0.5f;
   return;
} else if (sample_count == 2) {
-  bits = vs->caps.caps.v2.msaa_sample_positions[0] >> (8 * index);
+  bits = vs->caps.caps.v2.sample_locations[0] >> (8 * index);
} else if (sample_count < 4) {
-  bits = vs->caps.caps.v2.msaa_sample_positions[1] >> (8 * index);
+  bits = vs->caps.caps.v2.sample_locations[1] >> (8 * index);
} else if (sample_count < 8) {
-  bits = vs->caps.caps.v2.msaa_sample_positions[2 + (index >> 2)] >> (8 * 
(index & 3));
+  bits = vs->caps.caps.v2.sample_locations[2 + (index >> 2)] >> (8 * 
(index & 3));
} else if (sample_count < 8) {
-  bits = vs->caps.caps.v2.msaa_sample_positions[4 + (index >> 2)] >> (8 * 
(index & 3));
+  bits = vs->caps.caps.v2.sample_locations[4 + (index >> 2)] >> (8 * 
(index & 3));
}
out_value[0] = ((bits >> 4) & 0xf) / 16.0f;
out_value[1] = (bits & 0xf) / 16.0f;
diff --git a/src/gallium/drivers/virgl/virgl_hw.h 
b/src/gallium/drivers/virgl/virgl_hw.h
index d0df23e2f6..086757def6 100644
--- a/src/gallium/drivers/virgl/virgl_hw.h
+++ b/src/gallium/drivers/virgl/virgl_hw.h
@@ -287,7 +287,7 @@ struct virgl_caps_v2 {
 uint32_t uniform_buffer_offset_alignment;
 uint32_t shader_buffer_offset_alignment;
 uint32_t capability_bits;
-   uint32_t msaa_sample_positions[8];
+uint32_t sample_locations[8];
 };
 
 union virgl_caps {
-- 
2.18.0.rc2

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

Re: [Mesa-dev] [PATCH] mesa/virgl: Fix off-by-one and copy-paste error in multisample position evaluation

[Erik Faye-Lund]

Just a heads-up, this patch conflicts with this one:

https://patchwork.freedesktop.org/patch/239169/

Your were sent out first, so I'll be happy to rebase mine when this has 
landed...


On 13. juli 2018 14:46, Gert Wollny wrote:

Fixes: 91f48cdfe5c817158c533a8f67c60e9aabbe4479
virgl: Add support for glGetMultisample
Signed-off-by: Gert Wollny 
---
  src/gallium/drivers/virgl/virgl_context.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_context.c 
b/src/gallium/drivers/virgl/virgl_context.c
index d985bf906f..f5e3832d99 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -943,11 +943,11 @@ static void virgl_get_sample_position(struct pipe_context 
*ctx,
return;
 } else if (sample_count == 2) {
bits = vs->caps.caps.v2.msaa_sample_positions[0] >> (8 * index);
-   } else if (sample_count < 4) {
+   } else if (sample_count <= 4) {
bits = vs->caps.caps.v2.msaa_sample_positions[1] >> (8 * index);
-   } else if (sample_count < 8) {
+   } else if (sample_count <= 8) {
bits = vs->caps.caps.v2.msaa_sample_positions[2 + (index >> 2)] >> (8 * 
(index & 3));
-   } else if (sample_count < 8) {
+   } else if (sample_count <= 16) {
bits = vs->caps.caps.v2.msaa_sample_positions[4 + (index >> 2)] >> (8 * 
(index & 3));
 }
 out_value[0] = ((bits >> 4) & 0xf) / 16.0f;


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

Re: [Mesa-dev] [PATCH] mesa/virgl: Fix off-by-one and copy-paste error in multisample position evaluation

[Erik Faye-Lund]

On 13. juli 2018 14:46, Gert Wollny wrote:

Fixes: 91f48cdfe5c817158c533a8f67c60e9aabbe4479
virgl: Add support for glGetMultisample
Signed-off-by: Gert Wollny 
---
  src/gallium/drivers/virgl/virgl_context.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_context.c 
b/src/gallium/drivers/virgl/virgl_context.c
index d985bf906f..f5e3832d99 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -943,11 +943,11 @@ static void virgl_get_sample_position(struct pipe_context 
*ctx,
return;
 } else if (sample_count == 2) {
bits = vs->caps.caps.v2.msaa_sample_positions[0] >> (8 * index);
-   } else if (sample_count < 4) {
+   } else if (sample_count <= 4) {
bits = vs->caps.caps.v2.msaa_sample_positions[1] >> (8 * index);
-   } else if (sample_count < 8) {
+   } else if (sample_count <= 8) {
bits = vs->caps.caps.v2.msaa_sample_positions[2 + (index >> 2)] >> (8 * 
(index & 3));
-   } else if (sample_count < 8) {
+   } else if (sample_count <= 16) {


This isn't technically speaking an off-by-one... perhaps adjust the 
commit message a bit?



bits = vs->caps.caps.v2.msaa_sample_positions[4 + (index >> 2)] >> (8 * 
(index & 3));
     }
 out_value[0] = ((bits >> 4) & 0xf) / 16.0f;


Reviewed-by: Erik Faye-Lund 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] virgl: remove unused stride-arguments

[Erik Faye-Lund]

On Jul 23, 2018 22:05, Dave Airlie  wrote:On 20 July 2018 at 13:39, Gurchetan Singh  wrote:

> Reviewed-by: Gurchetan Singh 

> On Wed, Jul 18, 2018 at 4:06 AM Erik Faye-Lund

>  wrote:

>>

>> The IOCTLs doesn't pass this along, so computing them in the first

>> place is kinda pointless.

>>

>> Signed-off-by: Erik Faye-Lund 

>> ---

>>

>> This is just a cleanup I noticed based on some discussion with Gert.

>>

>> A question is, what code here expects this stride to be respected? The

>> call-sites in virgl_*_transfer_map and virgl_*_transfer_unmap kinda

>> looks like they do... They'll get a bit of a surprise here, no?



>>

>> Anyway, this is already broken, so I think this should be OK. But

>> perhaps this patch shows some code-paths that need some love?



I reverted this as it didn't fixup vtest, and it introduced build time warnings.



I agree this should get some more investigation but make sure vtest and

drm backends don't regress.



Dave.


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

[Mesa-dev] [PATCH] forward precise-flag if supported

[Erik Faye-Lund]

New versions of virglrenderer supports the precise-flag, so let's
forward it from TGSI if that's the case.

This fixes a few dEQP-GLES31 tests:
- dEQP-GLES31.functional.tessellation.common_edge.quads_equal_spacing_precise
- 
dEQP-GLES31.functional.tessellation.common_edge.quads_fractional_even_spacing_precise
- 
dEQP-GLES31.functional.tessellation.common_edge.quads_fractional_odd_spacing_precise
- 
dEQP-GLES31.functional.tessellation.common_edge.triangles_equal_spacing_precise
- 
dEQP-GLES31.functional.tessellation.common_edge.triangles_fractional_even_spacing_precise
- 
dEQP-GLES31.functional.tessellation.common_edge.triangles_fractional_odd_spacing_precise

Signed-off-by: Erik Faye-Lund 
---

This matches this virglrenderer-series, which has been merged upstrea:
https://patchwork.freedesktop.org/series/46361/

 src/gallium/drivers/virgl/virgl_hw.h   | 1 +
 src/gallium/drivers/virgl/virgl_tgsi.c | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/virgl/virgl_hw.h 
b/src/gallium/drivers/virgl/virgl_hw.h
index ee6aa68c5a..8f4f0ab6d8 100644
--- a/src/gallium/drivers/virgl/virgl_hw.h
+++ b/src/gallium/drivers/virgl/virgl_hw.h
@@ -203,6 +203,7 @@ enum virgl_formats {
 #define VIRGL_CAP_TEXTURE_VIEW (1 << 1)
 #define VIRGL_CAP_SET_MIN_SAMPLES  (1 << 2)
 #define VIRGL_CAP_COPY_IMAGE   (1 << 3)
+#define VIRGL_CAP_TGSI_PRECISE (1 << 4)
 
 #define VIRGL_BIND_DEPTH_STENCIL (1 << 0)
 #define VIRGL_BIND_RENDER_TARGET (1 << 1)
diff --git a/src/gallium/drivers/virgl/virgl_tgsi.c 
b/src/gallium/drivers/virgl/virgl_tgsi.c
index ff5abf6ddb..d1f785d4d2 100644
--- a/src/gallium/drivers/virgl/virgl_tgsi.c
+++ b/src/gallium/drivers/virgl/virgl_tgsi.c
@@ -31,6 +31,7 @@
 struct virgl_transform_context {
struct tgsi_transform_context base;
bool cull_enabled;
+   bool has_precise;
 };
 
 static void
@@ -76,7 +77,8 @@ static void
 virgl_tgsi_transform_instruction(struct tgsi_transform_context *ctx,
 struct tgsi_full_instruction *inst)
 {
-   if (inst->Instruction.Precise)
+   struct virgl_transform_context *vtctx = (struct virgl_transform_context 
*)ctx;
+   if (!vtctx->has_precise && inst->Instruction.Precise)
   inst->Instruction.Precise = 0;
 
for (unsigned i = 0; i < inst->Instruction.NumSrcRegs; i++) {
@@ -104,6 +106,7 @@ struct tgsi_token *virgl_tgsi_transform(struct 
virgl_context *vctx, const struct
transform.base.transform_property = virgl_tgsi_transform_property;
transform.base.transform_instruction = virgl_tgsi_transform_instruction;
transform.cull_enabled = vscreen->caps.caps.v1.bset.has_cull;
+   transform.has_precise = vscreen->caps.caps.v2.capability_bits & 
VIRGL_CAP_TGSI_PRECISE;
tgsi_transform_shader(tokens_in, new_tokens, newLen, );
 
return new_tokens;
-- 
2.18.0

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

[Mesa-dev] [PATCH] gles: report maximum vertex-attrib stride to guest

[Erik Faye-Lund]

Similar to e387116, we also need to report this for GLES hosts.

Signed-off-by: Erik Faye-Lund 
---

Without this, there's no chance for GLES hosts to get GLES3.1 support.

 src/vrend_renderer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
index 9fb2f92..405993a 100644
--- a/src/vrend_renderer.c
+++ b/src/vrend_renderer.c
@@ -7396,6 +7396,9 @@ static void vrend_renderer_fill_caps_gles(uint32_t set, 
UNUSED uint32_t version,
if (gles_ver >= 31)
   glGetIntegerv(GL_SHADER_STORAGE_BUFFER_OFFSET_ALIGNMENT, 
(GLint*)>v2.shader_buffer_offset_alignment);
 
+   if (gles_ver >= 31)
+  glGetIntegerv(GL_MAX_VERTEX_ATTRIB_STRIDE, 
(GLint*)>v2.max_vertex_attrib_stride);
+
/* Not available on GLES */
caps->v2.texture_buffer_offset_alignment = 0;
 
-- 
2.18.0

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

Re: [Mesa-dev] [PATCH] gles: report maximum vertex-attrib stride to guest

[Erik Faye-Lund]

Sorry, send wrong. Please ignore.


On 19. juli 2018 17:58, Erik Faye-Lund wrote:

Similar to e387116, we also need to report this for GLES hosts.

Signed-off-by: Erik Faye-Lund 
---

Without this, there's no chance for GLES hosts to get GLES3.1 support.

  src/vrend_renderer.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
index 9fb2f92..405993a 100644
--- a/src/vrend_renderer.c
+++ b/src/vrend_renderer.c
@@ -7396,6 +7396,9 @@ static void vrend_renderer_fill_caps_gles(uint32_t set, 
UNUSED uint32_t version,
 if (gles_ver >= 31)
glGetIntegerv(GL_SHADER_STORAGE_BUFFER_OFFSET_ALIGNMENT, 
(GLint*)>v2.shader_buffer_offset_alignment);
  
+   if (gles_ver >= 31)

+  glGetIntegerv(GL_MAX_VERTEX_ATTRIB_STRIDE, 
(GLint*)>v2.max_vertex_attrib_stride);
+
 /* Not available on GLES */
 caps->v2.texture_buffer_offset_alignment = 0;
  


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

Re: [Mesa-dev] [PATCH 1/3] virgl: move bind-flags to virgl_winsys.h

[Erik Faye-Lund]

On 16. juli 2018 13:34, Gert Wollny wrote:

Am Montag, den 16.07.2018, 12:23 +0200 schrieb Erik Faye-Lund:

virglrenderer's virgl_hw.h doesn't contain these, and it's the
virgl winsys that cares about these, so let's move them there.

This reduces the diff between the two different versions of
virgl_hw.h, and should make it easier to upgrade the file in
the future.

Signed-off-by: Erik Faye-Lund 
---
  src/gallium/drivers/virgl/virgl_hw.h   | 12 
  src/gallium/drivers/virgl/virgl_resource.h |  2 ++
  src/gallium/drivers/virgl/virgl_winsys.h   | 12 
  3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_hw.h
b/src/gallium/drivers/virgl/virgl_hw.h
index 157267558a..d0df23e2f6 100644
--- a/src/gallium/drivers/virgl/virgl_hw.h
+++ b/src/gallium/drivers/virgl/virgl_hw.h
@@ -202,18 +202,6 @@ enum virgl_formats {
  #define VIRGL_CAP_TGSI_INVARIANT   (1 << 0)
  #define VIRGL_CAP_TEXTURE_VIEW (1 << 1)
  
-#define VIRGL_BIND_DEPTH_STENCIL (1 << 0)

-#define VIRGL_BIND_RENDER_TARGET (1 << 1)
-#define VIRGL_BIND_SAMPLER_VIEW  (1 << 3)
-#define VIRGL_BIND_VERTEX_BUFFER (1 << 4)
-#define VIRGL_BIND_INDEX_BUFFER  (1 << 5)
-#define VIRGL_BIND_CONSTANT_BUFFER (1 << 6)
-#define VIRGL_BIND_DISPLAY_TARGET (1 << 7)
-#define VIRGL_BIND_STREAM_OUTPUT (1 << 11)
-#define VIRGL_BIND_CURSOR(1 << 16)
-#define VIRGL_BIND_CUSTOM(1 << 17)
-#define VIRGL_BIND_SCANOUT   (1 << 18)
-

In virglrenderer there is a set of VREND_RES_* that are mostly the
same, so I assume that they should be in sync (I wouldn't bet on it
though). If it can be resolved that these two sets are independent of
each other then the patch is
   Reviewed-By: Gert Wollny 

Otherwise it might be better to move the VREND_RES_* defines into
virgl_hw.h in virglrenderer and rename so that at one point the two
virgl_hw.h files will have the same content.


Oh, thanks for noticing. Yeah, these need to be the same. They get 
passed through virgl_drm_winsys_resource_cache_create() to the 
VIRTGPU_RESOURCE_CREATEioctl, to VIRTIO_GPU_CMD_RESOURCE_CREATE_3D in 
qemu, and in turn gets interpreted by vrend_renderer_resource_create...


So yeah, these needs to be in sync. I guess the pragmatic choice would 
be to use these definitions in virglrenderer's copy here. I'll send a 
new proposal to virglrenderer for this bit, and drop this patch from 
this series.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] gallium: initialize ureg_dst::Invariant bit

[Erik Faye-Lund]

When this bit was added, it seems the some initialization code
was omitted by mistake.

Since stack-variables have kinda random contents, and we don't
zero initialize the whole struct in these code-paths, we end up
getting random-ish values for this bit.

Spotted by Coverity in the following CIDs:
- 1438115
- 1438123
- 1438130

Fixes: 70425bcfe63c4e9191809659d019ec4af923595d ("gallium: plumb
invariant output attrib thru TGSI")
---
 src/gallium/auxiliary/tgsi/tgsi_ureg.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.h 
b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
index 4d4a954529..c974ed0206 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
+++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
@@ -1019,6 +1019,7 @@ ureg_dst_array_register(unsigned file,
dst.DimIndIndex = 0;
dst.DimIndSwizzle = 0;
dst.ArrayID = array_id;
+   dst.Invariant = 0;
 
return dst;
 }
@@ -1050,6 +1051,7 @@ ureg_dst( struct ureg_src src )
dst.DimIndIndex = src.DimIndIndex;
dst.DimIndSwizzle = src.DimIndSwizzle;
dst.ArrayID = src.ArrayID;
+   dst.Invariant = 0;
 
return dst;
 }
@@ -1141,6 +1143,7 @@ ureg_dst_undef( void )
dst.DimIndIndex = 0;
dst.DimIndSwizzle = 0;
dst.ArrayID = 0;
+   dst.Invariant = 0;
 
return dst;
 }
-- 
2.18.0

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

[Mesa-dev] [PATCH] virgl: remove unused stride-arguments

[Erik Faye-Lund]

The IOCTLs doesn't pass this along, so computing them in the first
place is kinda pointless.

Signed-off-by: Erik Faye-Lund 
---

This is just a cleanup I noticed based on some discussion with Gert.

A question is, what code here expects this stride to be respected? The
call-sites in virgl_*_transfer_map and virgl_*_transfer_unmap kinda
looks like they do... They'll get a bit of a surprise here, no?

Anyway, this is already broken, so I think this should be OK. But
perhaps this patch shows some code-paths that need some love?

 src/gallium/drivers/virgl/virgl_buffer.c  |  4 ++--
 src/gallium/drivers/virgl/virgl_context.c |  2 +-
 src/gallium/drivers/virgl/virgl_texture.c | 24 ++-
 src/gallium/drivers/virgl/virgl_winsys.h  |  2 --
 .../winsys/virgl/drm/virgl_drm_winsys.c   |  6 -
 5 files changed, 5 insertions(+), 33 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_buffer.c 
b/src/gallium/drivers/virgl/virgl_buffer.c
index 2e63aebc72..97b2854b9c 100644
--- a/src/gallium/drivers/virgl/virgl_buffer.c
+++ b/src/gallium/drivers/virgl/virgl_buffer.c
@@ -77,7 +77,7 @@ static void *virgl_buffer_transfer_map(struct pipe_context 
*ctx,
 
readback = virgl_res_needs_readback(vctx, >base, usage);
if (readback)
-  vs->vws->transfer_get(vs->vws, vbuf->base.hw_res, box, 
trans->base.stride, trans->base.layer_stride, offset, level);
+  vs->vws->transfer_get(vs->vws, vbuf->base.hw_res, box, offset, level);
 
if (!(usage & PIPE_TRANSFER_UNSYNCHRONIZED))
   doflushwait = true;
@@ -109,7 +109,7 @@ static void virgl_buffer_transfer_unmap(struct pipe_context 
*ctx,
  vbuf->base.clean = FALSE;
  vctx->num_transfers++;
  vs->vws->transfer_put(vs->vws, vbuf->base.hw_res,
-   >box, trans->base.stride, 
trans->base.layer_stride, trans->offset, transfer->level);
+   >box, trans->offset, transfer->level);
 
   }
}
diff --git a/src/gallium/drivers/virgl/virgl_context.c 
b/src/gallium/drivers/virgl/virgl_context.c
index ee28680b8f..19bc23dd1e 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -71,7 +71,7 @@ static void virgl_buffer_flush(struct virgl_context *vctx,
 
vctx->num_transfers++;
rs->vws->transfer_put(rs->vws, vbuf->base.hw_res,
- , 0, 0, box.x, 0);
+ , box.x, 0);
 
util_range_set_empty(>valid_buffer_range);
 }
diff --git a/src/gallium/drivers/virgl/virgl_texture.c 
b/src/gallium/drivers/virgl/virgl_texture.c
index 150a5ebd8c..485b7cf1a7 100644
--- a/src/gallium/drivers/virgl/virgl_texture.c
+++ b/src/gallium/drivers/virgl/virgl_texture.c
@@ -138,7 +138,6 @@ static void *virgl_texture_transfer_map(struct pipe_context 
*ctx,
const unsigned h = u_minify(vtex->base.u.b.height0, level);
const unsigned nblocksy = util_format_get_nblocksy(format, h);
bool is_depth = 
util_format_has_depth(util_format_description(resource->format));
-   uint32_t l_stride;
bool doflushwait;
 
doflushwait = virgl_res_needs_flush_wait(vctx, >base, usage);
@@ -156,15 +155,6 @@ static void *virgl_texture_transfer_map(struct 
pipe_context *ctx,
trans->base.stride = vtex->stride[level];
trans->base.layer_stride = trans->base.stride * nblocksy;
 
-   if (resource->target != PIPE_TEXTURE_3D &&
-   resource->target != PIPE_TEXTURE_CUBE &&
-   resource->target != PIPE_TEXTURE_1D_ARRAY &&
-   resource->target != PIPE_TEXTURE_2D_ARRAY &&
-   resource->target != PIPE_TEXTURE_CUBE_ARRAY)
-  l_stride = 0;
-   else
-  l_stride = trans->base.layer_stride;
-
if (is_depth && resource->nr_samples > 1) {
   struct pipe_resource tmp_resource;
   virgl_init_temp_resource_from_box(_resource, resource, box,
@@ -188,7 +178,7 @@ static void *virgl_texture_transfer_map(struct pipe_context 
*ctx,
 
readback = virgl_res_needs_readback(vctx, >base, usage);
if (readback)
-  vs->vws->transfer_get(vs->vws, hw_res, box, trans->base.stride, 
l_stride, offset, level);
+  vs->vws->transfer_get(vs->vws, hw_res, box, offset, level);
 
if (doflushwait || readback)
   vs->vws->resource_wait(vs->vws, vtex->base.hw_res);
@@ -210,16 +200,6 @@ static void virgl_texture_transfer_unmap(struct 
pipe_context *ctx,
struct virgl_context *vctx = virgl_context(ctx);
struct virgl_transfer *trans = virgl_transfer(transfer);
struct virgl_texture *vtex = virgl_texture(transfer->resource);
-   uint32_t l_stride;
-
-   if (transfer->resource->target != PIPE_TEXTURE_3D &&
-   transfer->resource->target != PIPE_TEXTURE_CUBE &&
-   transfer->resource->target != PIPE_TEXTURE_1D_ARR

[Mesa-dev] [PATCH 2/4] virgl: drop needless return-code

[Erik Faye-Lund]

We always return TRUE, and we never check the return-value. Let's
just drop the return value instead.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_texture.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_texture.c 
b/src/gallium/drivers/virgl/virgl_texture.c
index 71c0e9da7f..2cee412665 100644
--- a/src/gallium/drivers/virgl/virgl_texture.c
+++ b/src/gallium/drivers/virgl/virgl_texture.c
@@ -240,7 +240,7 @@ static void virgl_texture_transfer_unmap(struct 
pipe_context *ctx,
 }
 
 
-static boolean
+static void
 vrend_resource_layout(struct virgl_texture *res,
   uint32_t *total_size)
 {
@@ -276,7 +276,6 @@ vrend_resource_layout(struct virgl_texture *res,
   *total_size = buffer_size;
else /* don't create guest backing store for MSAA */
   *total_size = 0;
-   return TRUE;
 }
 
 static boolean virgl_texture_get_handle(struct pipe_screen *screen,
-- 
2.17.1

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

[Mesa-dev] [PATCH 0/4] virgil: fix crash when mapping ms-texture

[Erik Faye-Lund]

Here's a series of patches to address a crash when mapping multisampled
textures in piglit. The crash can easily be reproduce using the
following command:

bin/copyteximage 2D -samples=2 -auto

However, it's not clear to me if this is the right fix or not; the
resource creation code and the resource mapping code is inconsistent in
dealing with backing-storage. I decided to go for the widest definition
here, but it's possible the narrower one would be more appropriate.

The first three patches are smaller cleanups that were noticed while
debugging this, and not directly related to the crash.

It's also worth mentioning that the test doesn't pass after this
either, but at least it doesn't crash. The non-multisample versions of
this test doesn't pass either, so I think the failure is unrelated to
the presence of backing-store.

Dave, what are your thoughts here? It seems you introduced this
inconsistency in the very first commit to virgl. Was there some
expectation of some third, non-depth handling case here that just didn't
get noticed until now?

Erik Faye-Lund (4):
  virgl: free trans on map-error
  virgl: drop needless return-code
  virgl: remove dead code
  virgl: do not map zero-sized resource

 src/gallium/drivers/virgl/virgl_texture.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

-- 
2.17.1

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

[Mesa-dev] [PATCH 3/4] virgl: remove dead code

[Erik Faye-Lund]

We don't use the size we calculate in this function, so let's just
drop the calculation

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_texture.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_texture.c 
b/src/gallium/drivers/virgl/virgl_texture.c
index 2cee412665..88c2f38483 100644
--- a/src/gallium/drivers/virgl/virgl_texture.c
+++ b/src/gallium/drivers/virgl/virgl_texture.c
@@ -311,15 +311,11 @@ virgl_texture_from_handle(struct virgl_screen *vs,
   const struct pipe_resource *template,
   struct winsys_handle *whandle)
 {
-   struct virgl_texture *tex;
-   uint32_t size;
-
-   tex = CALLOC_STRUCT(virgl_texture);
+   struct virgl_texture *tex = CALLOC_STRUCT(virgl_texture);
tex->base.u.b = *template;
tex->base.u.b.screen = >base;
pipe_reference_init(>base.u.b.reference, 1);
tex->base.u.vtbl = _texture_vtbl;
-   vrend_resource_layout(tex, );
 
tex->base.hw_res = vs->vws->resource_create_from_handle(vs->vws, whandle);
return >base.u.b;
-- 
2.17.1

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

[Mesa-dev] [PATCH 4/4] virgl: do not map zero-sized resource

[Erik Faye-Lund]

When creating textures, we avoid creating backing-store for all
multisampled textures, not just depth buffers.

So we can't try to map them later. That's just going to fail. So
let's take the blit-based code-path that seems to avoid this problem.

This make this piglit test-case no longer crash (although it still
fails):

bin/copyteximage 2D -samples=2 -auto

Signed-off-by: Erik Faye-Lund 
---

This is the patch that I'm a bit unsure about. More details in the
cover letter.

 src/gallium/drivers/virgl/virgl_texture.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/gallium/drivers/virgl/virgl_texture.c 
b/src/gallium/drivers/virgl/virgl_texture.c
index 88c2f38483..0902f9b817 100644
--- a/src/gallium/drivers/virgl/virgl_texture.c
+++ b/src/gallium/drivers/virgl/virgl_texture.c
@@ -137,7 +137,6 @@ static void *virgl_texture_transfer_map(struct pipe_context 
*ctx,
struct virgl_hw_res *hw_res;
const unsigned h = u_minify(vtex->base.u.b.height0, level);
const unsigned nblocksy = util_format_get_nblocksy(format, h);
-   bool is_depth = 
util_format_has_depth(util_format_description(resource->format));
uint32_t l_stride;
bool doflushwait;
 
@@ -165,7 +164,7 @@ static void *virgl_texture_transfer_map(struct pipe_context 
*ctx,
else
   l_stride = trans->base.layer_stride;
 
-   if (is_depth && resource->nr_samples > 1) {
+   if (resource->nr_samples > 1) {
   struct pipe_resource tmp_resource;
   virgl_init_temp_resource_from_box(_resource, resource, box,
 level, 0);
-- 
2.17.1

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

[Mesa-dev] [PATCH 1/4] virgl: free trans on map-error

[Erik Faye-Lund]

When we fail to map memory, we should also free trans to avoid
leaking memory.

Noticed while reading code.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/virgl/virgl_texture.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/gallium/drivers/virgl/virgl_texture.c 
b/src/gallium/drivers/virgl/virgl_texture.c
index 150a5ebd8c..71c0e9da7f 100644
--- a/src/gallium/drivers/virgl/virgl_texture.c
+++ b/src/gallium/drivers/virgl/virgl_texture.c
@@ -195,6 +195,7 @@ static void *virgl_texture_transfer_map(struct pipe_context 
*ctx,
 
ptr = vs->vws->resource_map(vs->vws, hw_res);
if (!ptr) {
+  slab_free(>texture_transfer_pool, trans);
   return NULL;
}
 
-- 
2.17.1

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

[Mesa-dev] [PATCH] winsys/virgl: avoid unintended behavior

[Erik Faye-Lund]

If we end up never taking the loop that writes ret, we can end up with
an uninitialized value, and if we're *really* unlucky, that value can
be -1, causing us to go down an error-path instead of a success path.

This was obviously not intended, so let's just initialize this to zero.

Noticed by Valgrind:

Conditional jump or move depends on uninitialised value(s)
   at 0xBA640A0: virgl_drm_winsys_resource_cache_create (virgl_drm_winsys.c:348)
   by 0xBA62FCF: virgl_buffer_create (virgl_buffer.c:170)
   by 0xBA605AC: virgl_resource_create (virgl_resource.c:60)
   by 0xBCF816F: bufferobj_data (st_cb_bufferobjects.c:344)
   by 0xBCF816F: st_bufferobj_data (st_cb_bufferobjects.c:390)
   by 0xBB7E836: vbo_use_buffer_objects (vbo_exec_api.c:1136)
   by 0xBCFCC6E: st_create_context_priv (st_context.c:414)
   by 0xBCFD3CD: st_create_context (st_context.c:590)
   by 0xBBB30CA: st_api_create_context (st_manager.c:896)
   by 0xB981E76: dri_create_context (dri_context.c:155)
   by 0xB97BDCE: driCreateContextAttribs (dri_util.c:473)
   by 0x5288331: dri3_create_context_attribs (dri3_glx.c:309)
   by 0x5264D64: glXCreateContextAttribsARB (create_context.c:78)

Signed-off-by: Erik Faye-Lund 
---
This seems analogous to some patches from Gert recently.

 src/gallium/winsys/virgl/drm/virgl_drm_winsys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c 
b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
index 612d717b99..98e0e99f66 100644
--- a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
+++ b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
@@ -313,7 +313,7 @@ virgl_drm_winsys_resource_cache_create(struct virgl_winsys 
*qws,
struct virgl_hw_res *res, *curr_res;
struct list_head *curr, *next;
int64_t now;
-   int ret;
+   int ret = 0;
 
/* only store binds for vertex/index/const buffers */
if (bind != VIRGL_BIND_CONSTANT_BUFFER && bind != VIRGL_BIND_INDEX_BUFFER &&
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH] winsys/virgl: avoid unintended behavior

[Erik Faye-Lund]

On to., sep. 6, 2018 at 3:00 PM, Emil Velikov 
 wrote:

Hi Erik,

On 6 September 2018 at 11:48, Erik Faye-Lund
 wrote:
 If we end up never taking the loop that writes ret, we can end up 
with
 an uninitialized value, and if we're *really* unlucky, that value 
can
 be -1, causing us to go down an error-path instead of a success 
path.


 This was obviously not intended, so let's just initialize this to 
zero.



Not that versed in virgl to know if one should initialize to 0 or 1.


The way I read the code, any value that isn't -1 will do, as that 
avoids us from going down the error-path.




In either case please use the following tag.
Should help with picking this fix to the right stable branches.

Fixes: a8987b88ff1 ("virgl: add driver for virtio-gpu 3D (v2)")


Thanks, fixed locally.



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


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

Re: [Mesa-dev] [PATCH] virgl: Add command and flags to initiate debugging on the host

[Erik Faye-Lund]

On to., sep. 6, 2018 at 3:25 PM, Emil Velikov 
 wrote:
On 5 September 2018 at 16:34, Gert Wollny  
wrote:



 +
 +enum virgl_debug_flags {
 +   debug_shader_tgsi = 1 << 0,
 +   debug_shader_glsl = 1 << 1,
 +   debug_shader_streamout = 1 << 2,
 +   debug_shader = debug_shader_tgsi | debug_shader_glsl | 
debug_shader_streamout,

 +   debug_cmd = 1 << 3,
 +   debug_object = 1 << 4,
 +   debug_blit = 1 << 5,
 +   debug_copy_resource = 1 << 6,
 +   /* debug_features = 1 << 7, can't be triggered from the guest, 
because

 +* the code is run there before the guest can send commands */
 +   debug_all = (1 << 8) - 1,
 +};
 +

Jfyi: These are effectively part of the ABI/protocol. A host/guest
definitions mismatch will result in the wrong debug being printed.
Ohh "the horror" I know, hence the jfyi ;-)


Good point. I think they should be moved to virgl_hw.h instead, as they 
need to be kept in sync.


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

Re: [Mesa-dev] [PATCH] virgl: Add command and flags to initiate debugging on the host

[Erik Faye-Lund]

On on., sep. 5, 2018 at 5:34 PM, Gert Wollny  
wrote:

From: Gert Wollny 

On the host VREND_DEBUG=guestallow must be set to let the guest 
override

the debug flags.

Signed-off-by: Gert Wollny 
---
The corresponding code for virglrenderer can be found in this MR:
https://gitlab.freedesktop.org/virgl/virglrenderer/merge_requests/39

 src/gallium/drivers/virgl/virgl_context.c  | 17 +
 src/gallium/drivers/virgl/virgl_encode.c   |  8 
 src/gallium/drivers/virgl/virgl_encode.h   |  3 +++
 src/gallium/drivers/virgl/virgl_protocol.h |  1 +
 src/gallium/drivers/virgl/virgl_screen.h   | 15 +++
 5 files changed, 44 insertions(+)

diff --git a/src/gallium/drivers/virgl/virgl_context.c 
b/src/gallium/drivers/virgl/virgl_context.c

index 4511bf3b2f..087055b602 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -1157,6 +1157,20 @@ static void virgl_get_sample_position(struct 
pipe_context *ctx,

index, sample_count, out_value[0], out_value[1]);
 }

+static const struct debug_named_value host_debug_options[] = {
+   { "tgsi", debug_shader_tgsi , "Print TGSI on host"},
+   { "glsl", debug_shader_glsl, "Print GLSL shaders created from 
TGSI"},
+   { "glsl", debug_shader_streamout, "Print GLSL shaders created 
from TGSI"},


Typo? Shouldn't the string be "streamout", and the description be 
something else?


With that fixed:

Reviewed-by: Erik Faye-Lund 


+   { "shader", debug_shader, "Print TGSI and created GLSL shaders"},
+   { "cmd", debug_cmd, "Print incoming commands"},
+   { "obj", debug_object, "Print object creation"},
+   { "blit", debug_blit, "Debug blit code path"},
+   { "copyres", debug_copy_resource, "Debug copy resource code 
path"},

+   { "all", debug_all, "Enable all debugging output"},
+   DEBUG_NAMED_VALUE_END
+};
+DEBUG_GET_ONCE_FLAGS_OPTION(virgl_host_debug, "VIRGL_HOST_DEBUG", 
host_debug_options, 0)

+
 struct pipe_context *virgl_context_create(struct pipe_screen 
*pscreen,

   void *priv,
   unsigned flags)
@@ -1268,6 +1282,9 @@ struct pipe_context 
*virgl_context_create(struct pipe_screen *pscreen,

virgl_encoder_create_sub_ctx(vctx, vctx->hw_sub_ctx_id);

virgl_encoder_set_sub_ctx(vctx, vctx->hw_sub_ctx_id);
+
+   virgl_encode_host_debug_flags(vctx, 
(unsigned)debug_get_option_virgl_host_debug());

+
return >base;
 fail:
return NULL;
diff --git a/src/gallium/drivers/virgl/virgl_encode.c 
b/src/gallium/drivers/virgl/virgl_encode.c

index 29920b22be..ff21eb4bda 100644
--- a/src/gallium/drivers/virgl/virgl_encode.c
+++ b/src/gallium/drivers/virgl/virgl_encode.c
@@ -1044,3 +1044,11 @@ int virgl_encode_texture_barrier(struct 
virgl_context *ctx,

virgl_encoder_write_dword(ctx->cbuf, flags);
return 0;
 }
+
+int virgl_encode_host_debug_flags(struct virgl_context *ctx,
+  unsigned flags)
+{
+   virgl_encoder_write_cmd_dword(ctx, 
VIRGL_CMD0(VIRGL_CCMD_SET_DEBUG_FLAGS, 0, 1));

+   virgl_encoder_write_dword(ctx->cbuf, flags);
+   return 0;
+}
diff --git a/src/gallium/drivers/virgl/virgl_encode.h 
b/src/gallium/drivers/virgl/virgl_encode.h

index 40e62d453b..330c59f366 100644
--- a/src/gallium/drivers/virgl/virgl_encode.h
+++ b/src/gallium/drivers/virgl/virgl_encode.h
@@ -276,4 +276,7 @@ int virgl_encode_launch_grid(struct virgl_context 
*ctx,

  const struct pipe_grid_info *grid_info);
 int virgl_encode_texture_barrier(struct virgl_context *ctx,
  unsigned flags);
+
+int virgl_encode_host_debug_flags(struct virgl_context *ctx,
+  unsigned flags);
 #endif
diff --git a/src/gallium/drivers/virgl/virgl_protocol.h 
b/src/gallium/drivers/virgl/virgl_protocol.h

index 8d99c5ed47..3373121bf7 100644
--- a/src/gallium/drivers/virgl/virgl_protocol.h
+++ b/src/gallium/drivers/virgl/virgl_protocol.h
@@ -92,6 +92,7 @@ enum virgl_context_cmd {
VIRGL_CCMD_SET_FRAMEBUFFER_STATE_NO_ATTACH,
VIRGL_CCMD_TEXTURE_BARRIER,
VIRGL_CCMD_SET_ATOMIC_BUFFERS,
+   VIRGL_CCMD_SET_DEBUG_FLAGS,
 };

 /*
diff --git a/src/gallium/drivers/virgl/virgl_screen.h 
b/src/gallium/drivers/virgl/virgl_screen.h

index 719f5166d7..939c782c83 100644
--- a/src/gallium/drivers/virgl/virgl_screen.h
+++ b/src/gallium/drivers/virgl/virgl_screen.h
@@ -29,6 +29,21 @@

 #define VIRGL_DEBUG_VERBOSE 1
 #define VIRGL_DEBUG_TGSI2
+
+enum virgl_debug_flags {
+   debug_shader_tgsi = 1 << 0,
+   debug_shader_glsl = 1 << 1,
+   debug_shader_streamout = 1 << 2,
+   debug_shader = debug_shader_tgsi | debug_shader_glsl | 
debug_shader_streamout,

+   debug_cmd = 1 << 3,
+   debug

[Mesa-dev] [PATCH 0/3] verify max vertex attrib stride

[Erik Faye-Lund]

OpenGL 4.4 and OpenGL ES 3.1 both require the maximum
vertex attrib stride to be at least 2048. If this isn't
the case, we shouldn't expose these API versions.

Unfortunately, the r600 driver only supports 2047. To
avoid regressions in the supported GL version, the
first patch modifies the returned value.

I'm not sure if that last one is a good idea or not, as
it's strictly speaking non-conformant. But applications
won't expect GL errors generated when using strides of
2048 either, which is what currently happens.

The initial motivation for this patch-series is to avoid
exposing a too high spec version in virgl and then get
dEQP failures when running on old hardware. The virgl
specific bits are being sent separately, because they
depend on some other not-yet-upstream things ATM.

Thoughts?

Erik Faye-Lund (3):
  r600: report incorrect max-vertex-attrib for GL 4.4
  mesa: verify MaxVertexAttribStride for GL 4.4
  mesa: verify MaxVertexAttribStride for GLES 3.1

 src/gallium/drivers/r600/r600_pipe.c | 3 ++-
 src/mesa/main/version.c  | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.18.0.rc2

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

[Mesa-dev] [PATCH 3/3] mesa: verify MaxVertexAttribStride for GLES 3.1

[Erik Faye-Lund]

The OpenGL 3.1 specification, table Table 20.41 ("Implementation
Dependent Values"), defines the minimum-maximum value for
MAX_VERTEX_ATTRIB_STRIDE to be 2048.

So we shouldn't enable OpenGL ES 3.1 on implementations where this
isn't the case. Let's add a check for this

Signed-off-by: Erik Faye-Lund 
---
 src/mesa/main/version.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
index 0b0d5b79d6..58e68b4772 100644
--- a/src/mesa/main/version.c
+++ b/src/mesa/main/version.c
@@ -530,6 +530,7 @@ compute_version_es2(const struct gl_extensions *extensions,
const bool es31_compute_shader =
   consts->MaxComputeWorkGroupInvocations >= 128;
const bool ver_3_1 = (ver_3_0 &&
+ consts->MaxVertexAttribStride >= 2048 &&
  extensions->ARB_arrays_of_arrays &&
  es31_compute_shader &&
  extensions->ARB_draw_indirect &&
-- 
2.18.0.rc2

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

[Mesa-dev] [PATCH 1/3] r600: report incorrect max-vertex-attrib for GL 4.4

[Erik Faye-Lund]

OpenGL 4.4 requires a max vertex attrib of 2048 or higher, but
r600 only supports 2047. Technically, this makes it an GL4.3 GPU,
but it's currently exposing GL4.4.

To avoid regressing the GL version supported in the following
patches, let's just lie and pretend like we support 2048. Any
applications using 2048 are already broken anyway.

Signed-off-by: Erik Faye-Lund 
---
 src/gallium/drivers/r600/r600_pipe.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/r600/r600_pipe.c 
b/src/gallium/drivers/r600/r600_pipe.c
index bc5660d6d1..2518032145 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -470,7 +470,8 @@ static int r600_get_param(struct pipe_screen* pscreen, enum 
pipe_cap param)
return family >= CHIP_CEDAR ? 4 : 1;
 
case PIPE_CAP_MAX_VERTEX_ATTRIB_STRIDE:
-   return 2047;
+   /* Should be 2047, but 2048 is a requirement for GL 4.4 */
+   return 2048;
 
/* Texturing. */
case PIPE_CAP_MAX_TEXTURE_2D_LEVELS:
-- 
2.18.0.rc2

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

[Mesa-dev] [PATCH 2/3] mesa: verify MaxVertexAttribStride for GL 4.4

[Erik Faye-Lund]

The OpenGL 4.4 specification, table Table 23.55 ("Implementation
Dependent Values"), defines the minimum-maximum value for
MAX_VERTEX_ATTRIB_STRIDE to be 2048.

So we shouldn't enable OpenGL 4.4 on implementations where this isn't
the case. Let's add a check for this.

Signed-off-by: Erik Faye-Lund 
---
 src/mesa/main/version.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
index 2ba4c0b57f..0b0d5b79d6 100644
--- a/src/mesa/main/version.c
+++ b/src/mesa/main/version.c
@@ -369,6 +369,7 @@ compute_version(const struct gl_extensions *extensions,
  extensions->ARB_texture_view);
const bool ver_4_4 = (ver_4_3 &&
  consts->GLSLVersion >= 440 &&
+ consts->MaxVertexAttribStride >= 2048 &&
  extensions->ARB_buffer_storage &&
  extensions->ARB_clear_texture &&
  extensions->ARB_enhanced_layouts &&
-- 
2.18.0.rc2

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

Re: [Mesa-dev] [RFC PATCH] mesa/st: don't prematurely optimize for render targets when on virgl

[Erik Faye-Lund]

On 10. juli 2018 15:42, Gert Wollny wrote:

For three component textures virgl faces the problem that the host driver
may report that these can not be used as a render target, and when the
client requests such a texture a four-componet texture will be choosen
even if only a sampler view was requested. One example where this happens
is with the Intel i965 driver that doesn't support RGB32* as render target.
The result is that when allocating a GL_RGB32F and a GL_RGB32I texture, and
then glCopyImageSubData is called for these two texture, gallium will fail
with an assertion, because it reports a different per pixel bit count.

Therefore, when using the virgl driver, don't try to enable BIND_RENDER_TARGET
for RGB textures that were requested with only BIND_SAMPLER_VIEW.

Signed-off-by: Gert Wollny 
---

I'm aware that instead of using the device ID, I should probably add a new caps
flag, but apart from that I was wondering whether there may be better approaches
to achieve the same goal: The a texture is allocated with the internal format
as closely as possible to the requested one. Especially it shouldn't change the
percieved pixel bit count.

In fact, I was a bit surprised to see that the assertions regarding the
different sizes was hit in st_copy_image:307 (swizzled_copy). It seems that
there is some check missing that should redirect the copy in such a case.

Many thanks for any comments,
Gert

  src/mesa/state_tracker/st_format.c | 22 +++---
  1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/mesa/state_tracker/st_format.c 
b/src/mesa/state_tracker/st_format.c
index 9ae796eca9..2d8ff756a9 100644
--- a/src/mesa/state_tracker/st_format.c
+++ b/src/mesa/state_tracker/st_format.c
@@ -2285,19 +2285,27 @@ st_ChooseTextureFormat(struct gl_context *ctx, GLenum 
target,
  
 /* GL textures may wind up being render targets, but we don't know

  * that in advance.  Specify potential render target flags now for formats
-* that we know should always be renderable.
+* that we know should always be renderable, except when we are on virgl,
+* we don't try this for three component textures, because the host might
+* not support rendering to them, and then Gallium chooses a four component
+* internal format and calls to e.g. glCopyImageSubData will fail for format
+* that should be compatible.
  */
 bindings = PIPE_BIND_SAMPLER_VIEW;
 if (_mesa_is_depth_or_stencil_format(internalFormat))
bindings |= PIPE_BIND_DEPTH_STENCIL;
-   else if (is_renderbuffer || internalFormat == 3 || internalFormat == 4 ||
-internalFormat == GL_RGB || internalFormat == GL_RGBA ||
-internalFormat == GL_RGB8 || internalFormat == GL_RGBA8 ||
+   else if (is_renderbuffer  ||
+internalFormat == GL_RGBA ||
+internalFormat == GL_RGBA8 ||
  internalFormat == GL_BGRA ||
-internalFormat == GL_RGB16F ||
  internalFormat == GL_RGBA16F ||
-internalFormat == GL_RGB32F ||
-internalFormat == GL_RGBA32F)
+internalFormat == GL_RGBA32F ||
+((st->pipe->screen->get_param(st->pipe->screen, PIPE_CAP_DEVICE_ID) != 
0x1010) &&
+ (internalFormat == 3 || internalFormat == 4 ||
+  internalFormat == GL_RGB ||
+  internalFormat == GL_RGB8 ||
+  internalFormat == GL_RGB16F ||
+  internalFormat == GL_RGB32F )))
bindings |= PIPE_BIND_RENDER_TARGET;


I don't think this is correct. The problem is that the spec defines 
GL_RGB et al as color-renderable, and in OpenGL any color-renderable 
texture can become a render-target at any point. So the driver *has* to 
be prepared for rendering to GL_RGB.


The OpenGL 4.6 spec, section 9.4 "Framebuffer completeness" has this to say:

"An internal format is color-renderable if it is RED, RG, RGB, RGBA, or 
one of the sized internal formats from table 8.12 whose “CR” 
(color-renderable) column is checked in that table"


So, all RGB formats must be color-renderable in OpenGL. For OpenGL ES, I 
think this is slightly different, where color-renderable guarantees for 
RGB-textures are extensions for at least ES 2.0 IIRC. So *perhaps* we 
could get away with something like this for that API.

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

Re: [Mesa-dev] [RFC PATCH] mesa/st: don't prematurely optimize for render targets when on virgl

[Erik Faye-Lund]

On 10. juli 2018 16:23, Gert Wollny wrote:

Am Dienstag, den 10.07.2018, 16:01 +0200 schrieb Erik Faye-Lund:

On 10. juli 2018 15:42, Gert Wollny wrote:

For three component textures virgl faces the problem that the host
driver
may report that these can not be used as a render target, and when
the
client requests such a texture a four-componet texture will be
choosen
even if only a sampler view was requested. One example where this
happens
is with the Intel i965 driver that doesn't support RGB32* as render
target.
The result is that when allocating a GL_RGB32F and a GL_RGB32I
texture, and
then glCopyImageSubData is called for these two texture, gallium
will fail
with an assertion, because it reports a different per pixel bit
count.

Therefore, when using the virgl driver, don't try to enable
BIND_RENDER_TARGET
for RGB textures that were requested with only BIND_SAMPLER_VIEW.

Signed-off-by: Gert Wollny 
---

I'm aware that instead of using the device ID, I should probably
add a new caps
flag, but apart from that I was wondering whether there may be
better approaches
to achieve the same goal: The a texture is allocated with the
internal format
as closely as possible to the requested one. Especially it
shouldn't change the
percieved pixel bit count.

In fact, I was a bit surprised to see that the assertions regarding
the
different sizes was hit in st_copy_image:307 (swizzled_copy). It
seems that
there is some check missing that should redirect the copy in such a
case.

Many thanks for any comments,
Gert

   src/mesa/state_tracker/st_format.c | 22 +++---
   1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/mesa/state_tracker/st_format.c
b/src/mesa/state_tracker/st_format.c
index 9ae796eca9..2d8ff756a9 100644
--- a/src/mesa/state_tracker/st_format.c
+++ b/src/mesa/state_tracker/st_format.c
@@ -2285,19 +2285,27 @@ st_ChooseTextureFormat(struct gl_context
*ctx, GLenum target,
   
  /* GL textures may wind up being render targets, but we don't

know
   * that in advance.  Specify potential render target flags now
for formats
-* that we know should always be renderable.
+* that we know should always be renderable, except when we are
on virgl,
+* we don't try this for three component textures, because the
host might
+* not support rendering to them, and then Gallium chooses a
four component
+* internal format and calls to e.g. glCopyImageSubData will
fail for format
+* that should be compatible.
   */
  bindings = PIPE_BIND_SAMPLER_VIEW;
  if (_mesa_is_depth_or_stencil_format(internalFormat))
 bindings |= PIPE_BIND_DEPTH_STENCIL;
-   else if (is_renderbuffer || internalFormat == 3 ||
internalFormat == 4 ||
-internalFormat == GL_RGB || internalFormat == GL_RGBA
||
-internalFormat == GL_RGB8 || internalFormat ==
GL_RGBA8 ||
+   else if (is_renderbuffer  ||
+internalFormat == GL_RGBA ||
+internalFormat == GL_RGBA8 ||
   internalFormat == GL_BGRA ||
-internalFormat == GL_RGB16F ||
   internalFormat == GL_RGBA16F ||
-internalFormat == GL_RGB32F ||
-internalFormat == GL_RGBA32F)
+internalFormat == GL_RGBA32F ||
+((st->pipe->screen->get_param(st->pipe->screen,
PIPE_CAP_DEVICE_ID) != 0x1010) &&
+ (internalFormat == 3 || internalFormat == 4 ||
+  internalFormat == GL_RGB ||
+  internalFormat == GL_RGB8 ||
+  internalFormat == GL_RGB16F ||
+  internalFormat == GL_RGB32F )))
 bindings |= PIPE_BIND_RENDER_TARGET;

I don't think this is correct.

I'm also quite sure that this would just be a work-around-something ...


The problem is that the spec defines GL_RGB et al as color-
renderable, and in OpenGL any color-renderable texture can become a
render-target at any point. So the driver *has* to be prepared for
rendering to GL_RGB.

The OpenGL 4.6 spec, section 9.4 "Framebuffer completeness" has this
to say:

"An internal format is color-renderable if it is RED, RG, RGB, RGBA,
or one of the sized internal formats from table 8.12 whose “CR”
(color-renderable) column is checked in that table"

Since this is also written in the 4.5 spec, and the Intel driver
advertises this version, but the test to attach such a texture to a
frame buffer device results in an incomplete framebuffer, the Intel
driver is not up to spec or so it would seem ...


OpenGL has this other excape hatch, where it alllows drivers to say 
FRAMEBUFFER_UNSUPPORTED for basically any combination of attachments it 
doesn't like. Perhaps the Intel is using this as a way out? If so, we 
might have to be prepared to do something similar.


I don't think Gallium has a way of rejecting framebuffers in this way, 
though. So if that's what's going on, we might have to introduce one.



I wonder at which point this was introduced, 3.3 also has it,

Re: [Mesa-dev] How to apply patches to upstream mesa 9.0.3

[Erik Faye-Lund]

On 11. juli 2018 10:22, Thella Shyam Kumar wrote:

Hi All,

We are using an x86 based platform and we are needing changes in mesa 
9.0.3 which is coming with AOSP 7.1.2r36
If we want the changes in mesa in AOSP, our understanding is that 
these changes should be upstreamed in freedesktop.org 
's mesa 9.0.3 because AOSP gets the code from 
there.


So firstly will you allow changes into mesa 9.0.3 as it is very old now?


No. Mesa 9.0.3 is already released, so that ship has sailed. And I'm 
pretty sure there's no plans for a 9.0.4, considering the last 9.x 
release happened almost 5 years ago. I think you're going to have to 
look into upgrading (or patching) Mesa in AOSP instead.


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

Re: [Mesa-dev] [PATCH v2 25/49] docs/meson.html: fix numerous issues spotted by xmllint

[Erik Faye-Lund]

On 11. juli 2018 01:17, Dylan Baker wrote:

---
  docs/meson.html | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)


This is a HTML-document, not an XML document or XHTML. What xmllint 
thinks should be irrelevant.



diff --git a/docs/meson.html b/docs/meson.html
index b64ca2ec35e..d4b1861ff26 100644
--- a/docs/meson.html
+++ b/docs/meson.html
@@ -1,9 +1,9 @@
  http://www.w3.org/TR/html4/loose.dtd;>
  
  
-  
+  


No, meta-tags should not have a trailing slash:
https://www.w3.org/TR/html401/struct/global.html#h-7.4.4


Compilation and Installation using Meson
-  
+  


Link tags shouldn't have trailing slashes either:
https://www.w3.org/TR/html401/struct/links.html#edef-LINK


  
  
  
@@ -247,7 +247,6 @@ is unrelated to the buildtype; setting the latter to

  
  
  
-
  
  
  


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

Re: [Mesa-dev] [PATCH v2 41/49] meosn: don't build gallium trivial tests on windows

[Erik Faye-Lund]

Typo in the subject, "meosn" -> "meson".


On 11. juli 2018 01:18, Dylan Baker wrote:

They require the pipe-loaders, which require xmlconfig, which doesn't
build with msvc.
---
  src/gallium/tests/meson.build | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/gallium/tests/meson.build b/src/gallium/tests/meson.build
index 0ee04350c87..15b9f549647 100644
--- a/src/gallium/tests/meson.build
+++ b/src/gallium/tests/meson.build
@@ -18,7 +18,10 @@
  # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
THE
  # SOFTWARE.
  
-subdir('trivial')

+if not with_platform_windows
+  # pipe-loader doesn't build on windows.
+  subdir('trivial')
+endif
  if with_gallium_softpipe
subdir('unit')
  endif


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

Re: [Mesa-dev] [PATCH 0/3] verify max vertex attrib stride

[Erik Faye-Lund]

On 06. juli 2018 18:43, Roland Scheidegger wrote:

Am 06.07.2018 um 12:03 schrieb Erik Faye-Lund:

OpenGL 4.4 and OpenGL ES 3.1 both require the maximum
vertex attrib stride to be at least 2048. If this isn't
the case, we shouldn't expose these API versions.

Unfortunately, the r600 driver only supports 2047. To
avoid regressions in the supported GL version, the
first patch modifies the returned value.

I'm not sure if that last one is a good idea or not, as
it's strictly speaking non-conformant. But applications
won't expect GL errors generated when using strides of
2048 either, which is what currently happens.

The initial motivation for this patch-series is to avoid
exposing a too high spec version in virgl and then get
dEQP failures when running on old hardware. The virgl
specific bits are being sent separately, because they
depend on some other not-yet-upstream things ATM.

Thoughts?

Erik Faye-Lund (3):
   r600: report incorrect max-vertex-attrib for GL 4.4
   mesa: verify MaxVertexAttribStride for GL 4.4
   mesa: verify MaxVertexAttribStride for GLES 3.1

  src/gallium/drivers/r600/r600_pipe.c | 3 ++-
  src/mesa/main/version.c  | 2 ++
  2 files changed, 4 insertions(+), 1 deletion(-)


Personally I think it's _much_ better to lie about the supported GL
version rather than the maximum vertex attrib stride (I don't know if
dEQP would actually have a test which tests for the max stride also
working than just being advertized, which would be way more relevant and
fail in any case).


I don't think I agree in this case; OpenGL 4.4 applications that use
*exactly* strides of 2048probably won't even check the max-stride, as it's
guaranteed to work by the standard anyway. Applications that use *more*
might check for this value, but as the query was introduced in 4.4 it's
likely that applications will just get a gl-error down an unexpected
code-path, which can lead to the wrong attribute bound (possibly pointing
to client-memory, which can lead to segfaults). Lying about the max
attribute stride will "just" lead to rendering glitches.

But even *if* it was better to lie about the GL version, I don't think we
have a good way to contain something like this to just the driver in
question, and I *certainly* don't think it's better to have hacks for a
single driver in the generic code that affects *all* drivers. But if we
were to lie about the GL version rather than the max-stride, we'd IMO need
to introduce something like a driver-switch to control the GL version,
which just seems like a bad idea to me, as it's easy to get stuck behind
on an old version for artificial reasons, or to miss something else
important and get an artificially high version (and risk breaking apps
on working systems when fixing it again).

There's of course a third option here, which is to go by the book and
leave r600 on OpenGL 4.3 / OpenGL ES 3.0. That feels like the "correct
thing to do" to me, but I'm sure someone will be annoyed if they can't
play the games they've bought already before. But perhaps it's better for
the users to work around this on applications where it's known to work,
than having *any* sort of deceptive code in here?


(Because if you're going to use stride 2048, it is hugely unlikely you'd
just rely on GL 4.4 for that, hence it wouldn't work with older versions
of GL anyway neither, where ALL strides are just supposed to work.)


I think what ARB realized for GL 4.4 is that not all implementations
support all strides. In that light, I think it's reasonable to say
that for GL 4.3 and before, an *undefined* stride was supposed to
work.


Roland

FWIW, I think it should be possible to work around that limitation on
r600. Since vertex fetch just uses a fetch shader, you could have a
shader key bit indicating strides of 2048, program a stride of 1024
instead and multiply the index to fetch by 2 in the shader. Of course
that could cause shader recompiles (never in practice...) and there's
some overhead associated with it, so might not be worth it...



I don't think it would be a very high priority thing to fix, but it would
certainly be interesting. Sadly, I don't have an r600 card myself, so I
can't really work on that.

Another option would be to do some CPU-side reshuffling of vertex-
attributes that are overly distant. Dunno.

If we decided to go down the "let's stick to the spec"-approach here, one
of these approaches might be appropriate to regain the 4.4 feature...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH V2] Check if the window is non-NULL before setting swap interval.

[Erik Faye-Lund]

On Thu, Jul 5, 2018 at 7:02 PM Emil Velikov  wrote:
>
> On 5 July 2018 at 17:17, Eric Engestrom  wrote:
> > On Thursday, 2018-07-05 14:43:02 +0100, Emil Velikov wrote:
> >> On 5 July 2018 at 10:53, Eric Engestrom  wrote:
> >> > On Monday, 2018-07-02 14:12:44 +0530, samiuddi wrote:
> >> >> This fixes crash due to NULL window when swap interval is set
> >> >> for pbuffer surface.
> >> >>
> >> >> Test: CtsDisplayTestCases pass
> >> >>
> >> >> Signed-off-by: samiuddi 
> >> >> ---
> >> >>
> >> >> Kindly ignore this patch
> >> >> https://lists.freedesktop.org/archives/mesa-dev/2018-July/199098.html
> >> >>
> >> >>  src/egl/drivers/dri2/platform_android.c | 2 +-
> >> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/src/egl/drivers/dri2/platform_android.c 
> >> >> b/src/egl/drivers/dri2/platform_android.c
> >> >> index ca8708a..b5b960a 100644
> >> >> --- a/src/egl/drivers/dri2/platform_android.c
> >> >> +++ b/src/egl/drivers/dri2/platform_android.c
> >> >> @@ -485,7 +485,7 @@ droid_swap_interval(_EGLDriver *drv, _EGLDisplay 
> >> >> *dpy,
> >> >> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> >> >> struct ANativeWindow *window = dri2_surf->window;
> >> >>
> >> >> -   if (window->setSwapInterval(window, interval))
> >> >> +   if (window && window->setSwapInterval(window, interval))
> >> >>return EGL_FALSE;
> >> >
> >> > Shouldn't we return false if we couldn't set the swap interval?
> >> >
> >> > I think this should be:
> >> >if (!window || window->setSwapInterval(window, interval))
> >> >   return EGL_FALSE;
> >> >
> >> Changing the patch as above will lead to eglSwapInterval consistently
> >> failing for pbuffer surfaces.
> >
> > I'm not that familiar with pbuffers, but does SwapInterval really make
> > sense for them? I thought you couldn't swap a pbuffer.
> >
> > If so, failing an invalid op seems like the right thing to do.
> > Otherwise, I guess sure, no-op returning success is fine.
> >
> Looking at eglSwapInterval manpage [1] (with my annotation):
> "eglSwapInterval — specifies the minimum number of video frame periods
> per buffer swap for the _window_ associated with the current context."
> While the spec does not mention window/pbuffer/pixmap at all - it
> extensively refers to eglSwapBuffers.
>
> Wrt eglSwapBuffers manpage [2] and spec are consistent:
>
> "If surface is a pixel buffer or a pixmap, eglSwapBuffers has no
> effect, and no error is generated."
>
> Perhaps it's wrong to set eglSwapInterval for !window surfaces, but
> its sibling (eglSwapBuffers) does not error out.
> Hence I doubt many users make a distinction between window and pbuffer
> surfaces for eglSwap*.
>
> Worth bringing to the WG meeting - it' planned for 1st August.
>

As I pointed out when I proposed this variant here:
https://patchwork.freedesktop.org/patch/219313/

"Also, I don't think EGL_FALSE is the right return-value, as it doesn't
seem the EGL 1.5 spec defines any such error. Also, for instance
dri2_swap_interval returns EGL_TRUE when there's no driver-function,
which further backs the "silent failure" in this case IMO."

So I think EGL_TRUE is the correct return-value for now. If the spec
gets changed, we can of course update our implementation.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] virgl: adjust strides when mapping temp-resources

[Erik Faye-Lund]

When we're mapping temp-resources, we clip the resource to the
transfer-box, which means the stride might not be correct any more.

So let's update the stride from the temp-resource, and recompute the
layer-stride.

This fixes crashes when running dEQP with --deqp-gl-config-name=rgbad24s8ms4

Signed-off-by: Erik Faye-Lund 
Fixes: a8987b88ff1 "virgl: add driver for virtio-gpu 3D (v2)"
---
I noticed this crash while testing this patch:
https://patchwork.freedesktop.org/patch/248805/

 src/gallium/drivers/virgl/virgl_texture.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/gallium/drivers/virgl/virgl_texture.c 
b/src/gallium/drivers/virgl/virgl_texture.c
index 0902f9b817..9edf0ced0c 100644
--- a/src/gallium/drivers/virgl/virgl_texture.c
+++ b/src/gallium/drivers/virgl/virgl_texture.c
@@ -176,6 +176,8 @@ static void *virgl_texture_transfer_map(struct pipe_context 
*ctx,
   /* we want to do a resolve blit into the temporary */
   hw_res = trans->resolve_tmp->hw_res;
   offset = 0;
+  trans->base.stride = ((struct 
virgl_texture*)trans->resolve_tmp)->stride[level];
+  trans->base.layer_stride = trans->base.stride * nblocksy;
} else {
   offset = vrend_get_tex_image_offset(vtex, level, box->z);
 
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH 00/18] [RFC] Pointer specific data structures

[Erik Faye-Lund]

On Wed, Apr 11, 2018 at 8:48 PM, Thomas Helland
 wrote:
> This series came about when I saw a talk online, while simultaneously
> being annoyd about the needless waste of memory in our set as reported
> by pahole. I have previously made some patches that changed our hash
> table from a reprobing one to a quadratic probing one, in the name of
> lower overhead and better cache locality, but I was not quite satisfied.
>
> I'm sending this series out now, as it seems like an ideal time since
> Timothy is working at reducing our compile times. Further details about
> the implementation and its advantages are described in the patches.
> I've found this to give a reduction in shader-db runtime of about 2%,
> but I have to do some more testing on my main computer, as my laptop
> is showing its age with some terrible thermal issues.
>
> This special cases on pointers, as that is a very common usecase.
> This allows us to drop some comparisons, and reduce the total size
> of our hash table to 70% or our current and the set to 50%. It uses
> linear probing and power-of-two table sizes to get good cache locality.
> In the pointer_map caes it moves the stored hashes out into it's own
> array for even better cache locality.
>
> I'm not sure if we want another set and map amongst our utils,
> but the patch series is simple enough, and complete enough,
> that I thought I could share it for some inital comments.

This approach gives me a bad feeling. Using memory addresses for
storage ordering in a compiler is not quite nice; it can easily mask
spurious bugs, and have a compiler produce different result each run.
Such compilers are not nice to work with. I've seen *exactly* this
use-case go wrong in the past.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3] virgl: Add support for glGetMultisample

[Erik Faye-Lund]

On Thu, Jun 28, 2018 at 5:54 PM Gert Wollny  wrote:
>
> Am Donnerstag, den 28.06.2018, 17:40 +0200 schrieb Erik Faye-Lund:
> > On Thu, Jun 28, 2018 at 5:31 PM Gert Wollny  > m> wrote:
> > >
> > > There are two aspects:
> > >
> > > For each number of samples there is indeed a fixes set of sample
> > > positions that only depends on the hardware. The set corresponding
> > > to the requested number of samples is used when the surface is
> > > created with GL_TRUE for the "fixedsamplelocations" parameter.
> >
> > What I'm trying to say is that the concept of a global,
> > hardware-dependent set of sample-positions isn't a thing.
> I'm not sure what you are getting at ...

See below...

> >
> > These are not the same, and querying the positions of the highest
> > sample-count mode isn't going to apply to any other mode. There
> > simply isn't a concept of "the hardware's sample locations". They
> > depend on the MSAA mode (effectively sample-count). This is exactly
> > why you need to create a dummy FBO to query this; you need to know
> > the sample count of the mode to answer this.
> And this is exactly what I'm doing on the host (see the patch against
> virglrenderer I linked before) to get the values into
> vs->caps.caps.v2.msaa_sample_positions: I go through the supported
> sample counts 2, 4, 8, up to 16 (if supported), store the positions,
> and pass them to the guest. I looked at the radeonsi, r600, and the
> intel driver, and they all define sample position sets for these sample
> numbers (only up to 8 for r600). If one requests a number that is not
> amongst these, then the sample positions for the next higher sample
> count are returned, (eg. for 13 sample one gets the first 13 positions
> for 16 samples).

Right, thanks for clarifying. I misread the code, and thought you
discarded the old set of samples after finding a bigger set of
samples, thus assuming any mode was a subset of the biggest mode.

It still seems kinda strange (and fragile) to me to try to enumerate
all possible sample locations up-front instead of querying a given
texture for it's sample-locations.

For instance, I don't think there's anything in the spec backing the
correctness of the picking the 13 first positions from the 16 sample
mode strategy. That just happens to work on these three drivers you've
checked right now. It might not for future hardware, nor other
drivers. And I wouldn't be too surprised if nasty details like
framebuffer transforms (y-flipping in stuff like backbuffer vs FBO,
renderbuffers, pbuffers, scanout rotations, the recently propsed
y-flip extensions, you name it) could in fact "secretly" modify what
the correct values are.

This is of course just my two cents.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3] virgl: Add support for glGetMultisample

[Erik Faye-Lund]

On Thu, Jun 28, 2018 at 7:49 PM Gert Wollny  wrote:
>
> Am Donnerstag, den 28.06.2018, 18:09 +0200 schrieb Erik Faye-Lund:
> > It still seems kinda strange (and fragile) to me to try to enumerate
> > all possible sample locations up-front instead of querying a given
> > texture for it's sample-locations.
> With virgl, querying a texture for host-side information is quite
> costly, so if we can get aways with one lookup when starting qemu, then
> IMHO this is the preferred way to go.
>
> > For instance, I don't think there's anything in the spec backing the
> > correctness of the picking the 13 first positions from the 16 sample
> > mode strategy. That just happens to work on these three drivers
> > you've checked right now. It might not for future hardware, nor other
> > drivers. And I wouldn't be too surprised if nasty details like
> > framebuffer transforms (y-flipping in stuff like backbuffer vs FBO,
> > renderbuffers, pbuffers, scanout rotations, the recently propsed
> > y-flip extensions, you name it) could in fact "secretly" modify what
> > the correct values are.
> I think from the performance point of view it is a reasonable approach
> to query these values once and re-use them.

Yes, I just don't think it is from a correctness point of view. And
IMO, correctness concerns should always trump performance.

Is really calling glGetMultisample a hot code-path in any use-case
that matters? I'm having a hard time accepting that without some
data...

> If this turns out to be a
> problem then we can still think of another solution, but preferable one
> where one doesn't have to go through all the stack for each coordinate
> pair.

I would rather start with a correct solution, and try to optimize that
than start with one with questionable correctness and try to match the
correctness with specific implementations as seems to be what's going
on here.

There's probably also other things that can be done to speed this up.
Perhaps the gallium interface should be changed to get all
sample-pairs in an array instead, and cache that? We could even go so
far as to introduce an array-getter version of this as an extension to
avoid having to call into the guest method many times when it comes
from a mesa-driver...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] mesa: MESA_framebuffer_flip_y extension [v2]

[Erik Faye-Lund]

On Thu, Jun 28, 2018 at 11:12 PM Fritz Koenig  wrote:
>
> Adds an extension to glFramebufferParameteri
> that will specify if the framebuffer is vertically
> flipped. Historically system framebuffers are
> vertically flipped and user framebuffers are not.
> Checking to see the state was done by looking at
> the name field.  This adds an explicit field.
>
> v2:
> * updated spec language [for chadv]
> * correctly specifying ES 3.1 [for chadv]
> * refactor access to rb->Name [for jason]
> * handle GetFramebufferParameteriv [for chadv]
> ---
>  docs/specs/MESA_framebuffer_flip_y.spec| 84 ++
>  include/GLES2/gl2ext.h |  5 ++
>  src/mapi/glapi/registry/gl.xml |  6 ++
>  src/mesa/drivers/dri/i915/intel_fbo.c  |  7 +-
>  src/mesa/drivers/dri/i965/intel_fbo.c  |  7 +-
>  src/mesa/drivers/dri/nouveau/nouveau_fbo.c |  7 +-
>  src/mesa/drivers/dri/radeon/radeon_fbo.c   |  7 +-
>  src/mesa/drivers/dri/radeon/radeon_span.c  |  9 ++-
>  src/mesa/drivers/dri/swrast/swrast.c   |  7 +-
>  src/mesa/drivers/osmesa/osmesa.c   |  5 +-
>  src/mesa/drivers/x11/xm_buffer.c   |  3 +-
>  src/mesa/drivers/x11/xmesaP.h  |  3 +-
>  src/mesa/main/accum.c  | 17 +++--
>  src/mesa/main/dd.h |  3 +-
>  src/mesa/main/extensions_table.h   |  1 +
>  src/mesa/main/fbobject.c   | 18 -
>  src/mesa/main/framebuffer.c|  1 +
>  src/mesa/main/glheader.h   |  3 +
>  src/mesa/main/mtypes.h |  3 +
>  src/mesa/main/readpix.c| 20 +++---
>  src/mesa/state_tracker/st_cb_fbo.c |  7 +-
>  src/mesa/swrast/s_blit.c   | 17 +++--
>  src/mesa/swrast/s_clear.c  |  3 +-
>  src/mesa/swrast/s_copypix.c| 11 +--
>  src/mesa/swrast/s_depth.c  |  6 +-
>  src/mesa/swrast/s_drawpix.c| 26 ---
>  src/mesa/swrast/s_renderbuffer.c   |  6 +-
>  src/mesa/swrast/s_renderbuffer.h   |  3 +-
>  src/mesa/swrast/s_stencil.c|  3 +-
>  29 files changed, 241 insertions(+), 57 deletions(-)
>  create mode 100644 docs/specs/MESA_framebuffer_flip_y.spec
>

I think this needs to update the _mesa_is_winsys_fbo-check in
_mesa_GetMultisamplefv in src/mesa/main/multisample.c to flip the
sample-positions as well...
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] gallium/u_vbuf: drop min/max-scanning for empty indirect draws

[Erik Faye-Lund]

When building with asserts enabled, we'll end up triggering an assert
in pipe_buffer_map_range down this code-path, due to trying to map
an empty range. Even if we avoid that, we'll trigger another assert
a bit later, because u_vbuf_get_minmax_index returns a min-index of
-1 here, which gets promoted to an unsigned value, and gives us an
out-of-bounds buffer-mapping offset.

Since we can't really have a well-defined min/max range here when
the range is empty anyway, we should just drop this dance in the
first place. After all, no rendering is going to be produced.

This fixes a crash in dEQP-GLES31.functional.draw_indirect.random.0
on VirGL for me.

Signed-off-by: Erik Faye-Lund 
---
I noticed this while debugging something else, so I thought I'd send
a patch upstream, as the problem doesn't seem unique to my usecase.

 src/gallium/auxiliary/util/u_vbuf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/gallium/auxiliary/util/u_vbuf.c 
b/src/gallium/auxiliary/util/u_vbuf.c
index 42f37c7574..76a1d143d9 100644
--- a/src/gallium/auxiliary/util/u_vbuf.c
+++ b/src/gallium/auxiliary/util/u_vbuf.c
@@ -1183,6 +1183,9 @@ void u_vbuf_draw_vbo(struct u_vbuf *mgr, const struct 
pipe_draw_info *info)
   new_info.start = data[2];
   pipe_buffer_unmap(pipe, transfer);
   new_info.indirect = NULL;
+
+  if (!new_info.count)
+ return;
}
 
if (new_info.index_size) {
-- 
2.18.0.rc2

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

[Mesa-dev] [PATCH] gallium/u_vbuf: drop min/max-scanning for empty indirect draws

[Erik Faye-Lund]

When building with asserts enabled, we'll end up triggering an assert
in pipe_buffer_map_range down this code-path, due to trying to map
an empty range. Even if we avoid that, we'll trigger another assert
a bit later, because u_vbuf_get_minmax_index returns a min-index of
-1 here, which gets promoted to an unsigned value, and gives us an
out-of-bounds buffer-mapping offset.

Since we can't really have a well-defined min/max range here when
the range is empty anyway, we should just drop this dance in the
first place. After all, no rendering is going to be produced.

This fixes a crash in dEQP-GLES31.functional.draw_indirect.random.0
on VirGL for me.

Signed-off-by: Erik Faye-Lund 
---

This is a resend of a mail that didn't reach the mailing-list yet.
Sorry if it appears twice for someone!

 src/gallium/auxiliary/util/u_vbuf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/gallium/auxiliary/util/u_vbuf.c 
b/src/gallium/auxiliary/util/u_vbuf.c
index 42f37c7574..76a1d143d9 100644
--- a/src/gallium/auxiliary/util/u_vbuf.c
+++ b/src/gallium/auxiliary/util/u_vbuf.c
@@ -1183,6 +1183,9 @@ void u_vbuf_draw_vbo(struct u_vbuf *mgr, const struct 
pipe_draw_info *info)
   new_info.start = data[2];
   pipe_buffer_unmap(pipe, transfer);
   new_info.indirect = NULL;
+
+  if (!new_info.count)
+ return;
}
 
if (new_info.index_size) {
-- 
2.18.0.rc2

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

Re: [Mesa-dev] [PATCH v3] virgl: Add support for glGetMultisample

[Erik Faye-Lund]

On Thu, Jun 28, 2018 at 5:31 PM Gert Wollny  wrote:
>
> Am Donnerstag, den 28.06.2018, 17:09 +0200 schrieb Erik Faye-Lund:
> > Unless I'm misunderstanding, this seems to indicate that the hardware
> > has a fixed set of sample positions, which I don't think is true for
> > most hardware. Instead, the sample locations is a function of the
> > multisampling mode for a given surface...
> There are two aspects:
>
> For each number of samples there is indeed a fixes set of sample
> positions that only depends on the hardware. The set corresponding to
> the requested number of samples is used when the surface is created
> with GL_TRUE for the "fixedsamplelocations" parameter.

What I'm trying to say is that the concept of a global,
hardware-dependent set of sample-positions isn't a thing.

For instance, a single-sample multi-sample mode usually has the first
(and only sample) in the center of the pixel, whereas a 4xMSAA pattern
usually has four samples in a rotated grid, none of them in the center
of the pixel.

These are not the same, and querying the positions of the highest
sample-count mode isn't going to apply to any other mode. There simply
isn't a concept of "the hardware's sample locations". They depend on
the MSAA mode (effectively sample-count). This is exactly why you need
to create a dummy FBO to query this; you need to know the sample count
of the mode to answer this.

The fixedsamplelocations=false complication is entirely orthogonal to this.

> If this parameter is set to false, then all bets are off and the sample
> positions can even vary over the surface area. For this case
> glGetMultisample is kind of useless, for the other case one can query
> all sets once on the host and then re-use these values (see this patch
> for the host side: https://patchwork.freedesktop.org/patch/233354/)
>
> best,
> Gert
>
> >
> > On Thu, Jun 28, 2018 at 3:45 PM Gert Wollny  > m> wrote:
> > >
> > > Use caps to obtain the multisample sample positions for up to 16
> > > positions and implement the according Gallium interface.
> > >
> > > Fixes (when run on GL host):
> > > dEQP-
> > > GLES31.functional.texture.multisample.samples_1.sample_position
> > > dEQP-
> > > GLES31.functional.texture.multisample.samples_2.sample_position
> > > dEQP-
> > > GLES31.functional.texture.multisample.samples_3.sample_position
> > > dEQP-
> > > GLES31.functional.texture.multisample.samples_4.sample_position
> > > dEQP-
> > > GLES31.functional.texture.multisample.samples_8.sample_position
> > > dEQP-
> > > GLES31.functional.texture.multisample.samples_10.sample_position
> > > dEQP-
> > > GLES31.functional.texture.multisample.samples_12.sample_position
> > > dEQP-
> > > GLES31.functional.texture.multisample.samples_13.sample_position
> > > dEQP-
> > > GLES31.functional.texture.multisample.samples_16.sample_position
> > >
> > > v2: remove unrelated chunk (thanks Ilia Mirkin)
> > > v3: - also return positions for intermediate sample counts
> > > - fix unused varible warning
> > > - update description
> > >
> > > Signed-off-by: Gert Wollny 
> > > ---
> > > I left the debug_printf in there, because this patch (together with
> > > the
> > > related virglrenderer patch) is not sufficient to fix above tests
> > > on a GLES
> > > host.
> > >
> > >  src/gallium/drivers/virgl/virgl_context.c | 38
> > > +++
> > >  src/gallium/drivers/virgl/virgl_hw.h  |  1 +
> > >  2 files changed, 39 insertions(+)
> > >
> > > diff --git a/src/gallium/drivers/virgl/virgl_context.c
> > > b/src/gallium/drivers/virgl/virgl_context.c
> > > index e6f8dc8525..43c141e42d 100644
> > > --- a/src/gallium/drivers/virgl/virgl_context.c
> > > +++ b/src/gallium/drivers/virgl/virgl_context.c
> > > @@ -920,6 +920,42 @@ virgl_context_destroy( struct pipe_context
> > > *ctx )
> > > FREE(vctx);
> > >  }
> > >
> > > +static void virgl_get_sample_position(struct pipe_context *ctx,
> > > +  unsigned sample_count,
> > > +  unsigned index,
> > > +  float *out_value)
> > > +{
> > > +   struct virgl_context *vctx = virgl_context(ctx);
> > > +   struct virgl_screen *vs = virgl_screen(vctx->base.screen);
> > > +
> > > +   if (sample_count >

Re: [Mesa-dev] [PATCH v3] virgl: Add support for glGetMultisample

[Erik Faye-Lund]

Unless I'm misunderstanding, this seems to indicate that the hardware
has a fixed set of sample positions, which I don't think is true for
most hardware. Instead, the sample locations is a function of the
multisampling mode for a given surface...

On Thu, Jun 28, 2018 at 3:45 PM Gert Wollny  wrote:
>
> Use caps to obtain the multisample sample positions for up to 16
> positions and implement the according Gallium interface.
>
> Fixes (when run on GL host):
> dEQP-GLES31.functional.texture.multisample.samples_1.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_2.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_3.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_4.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_8.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_10.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_12.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_13.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_16.sample_position
>
> v2: remove unrelated chunk (thanks Ilia Mirkin)
> v3: - also return positions for intermediate sample counts
> - fix unused varible warning
> - update description
>
> Signed-off-by: Gert Wollny 
> ---
> I left the debug_printf in there, because this patch (together with the
> related virglrenderer patch) is not sufficient to fix above tests on a GLES
> host.
>
>  src/gallium/drivers/virgl/virgl_context.c | 38 
> +++
>  src/gallium/drivers/virgl/virgl_hw.h  |  1 +
>  2 files changed, 39 insertions(+)
>
> diff --git a/src/gallium/drivers/virgl/virgl_context.c 
> b/src/gallium/drivers/virgl/virgl_context.c
> index e6f8dc8525..43c141e42d 100644
> --- a/src/gallium/drivers/virgl/virgl_context.c
> +++ b/src/gallium/drivers/virgl/virgl_context.c
> @@ -920,6 +920,42 @@ virgl_context_destroy( struct pipe_context *ctx )
> FREE(vctx);
>  }
>
> +static void virgl_get_sample_position(struct pipe_context *ctx,
> +  unsigned sample_count,
> +  unsigned index,
> +  float *out_value)
> +{
> +   struct virgl_context *vctx = virgl_context(ctx);
> +   struct virgl_screen *vs = virgl_screen(vctx->base.screen);
> +
> +   if (sample_count > vs->caps.caps.v1.max_samples) {
> +  debug_printf("VIRGL: requested %d MSAA samples, but only %d 
> supported\n",
> +   sample_count, vs->caps.caps.v1.max_samples);
> +  return;
> +   }
> +
> +   /* The following is basically copied from dri/i965gen6_get_sample_position
> +* The only addition is that we hold the msaa positions for all sample
> +* counts in a flat array. */
> +   uint32_t bits = 0;
> +   if (sample_count == 1) {
> +  out_value[0] = out_value[1] = 0.5f;
> +  return;
> +   } else if (sample_count == 2) {
> +  bits = vs->caps.caps.v2.msaa_sample_positions[0] >> (8 * index);
> +   } else if (sample_count < 4) {
> +  bits = vs->caps.caps.v2.msaa_sample_positions[1] >> (8 * index);
> +   } else if (sample_count < 8) {
> +  bits = vs->caps.caps.v2.msaa_sample_positions[2 + (index >> 2)] >> (8 
> * (index & 3));
> +   } else if (sample_count < 8) {
> +  bits = vs->caps.caps.v2.msaa_sample_positions[4 + (index >> 2)] >> (8 
> * (index & 3));
> +   }
> +   out_value[0] = ((bits >> 4) & 0xf) / 16.0f;
> +   out_value[1] = (bits & 0xf) / 16.0f;
> +   debug_printf("VIRGL: sample postion [%2d/%2d] = (%f, %f)\n",
> +index, sample_count, out_value[0], out_value[1]);
> +}
> +
>  struct pipe_context *virgl_context_create(struct pipe_screen *pscreen,
>void *priv,
>unsigned flags)
> @@ -994,6 +1030,8 @@ struct pipe_context *virgl_context_create(struct 
> pipe_screen *pscreen,
>
> vctx->base.set_blend_color = virgl_set_blend_color;
>
> +   vctx->base.get_sample_position = virgl_get_sample_position;
> +
> vctx->base.resource_copy_region = virgl_resource_copy_region;
> vctx->base.flush_resource = virgl_flush_resource;
> vctx->base.blit =  virgl_blit;
> diff --git a/src/gallium/drivers/virgl/virgl_hw.h 
> b/src/gallium/drivers/virgl/virgl_hw.h
> index ee58520f9b..82cbb8aed1 100644
> --- a/src/gallium/drivers/virgl/virgl_hw.h
> +++ b/src/gallium/drivers/virgl/virgl_hw.h
> @@ -298,6 +298,7 @@ struct virgl_caps_v2 {
>  uint32_t uniform_buffer_offset_alignment;
>  uint32_t shader_buffer_offset_alignment;
>  uint32_t capability_bits;
> +uint32_t msaa_sample_positions[8];
>  };
>
>  union virgl_caps {
> --
> 2.16.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___

Re: [Mesa-dev] [PATCH v4] virgl: Add support for glGetMultisample

[Erik Faye-Lund]

On Fri, Jun 29, 2018 at 12:52 PM Erik Faye-Lund  wrote:
>
> On Fri, Jun 29, 2018 at 12:39 PM Gert Wollny  
> wrote:
> >
> > Use caps to obtain the multisample sample positions for up to 16
> > positions and implement the according Gallium interface.
> >
> > This implemenation (plus its counterpart in virglrenderer) assume that
> > the fixed sample position are always the same for a given number of samples
> > over the whole live time of a qemu session. It also assumes that sample
> > series are only given for 2, 4, 8, and 16 samples, and for intermediate
> > numbers N of samples the next higher supported set from above list is picked
> > and the sample positions for the first N samples are returned accordingly.
> >
> > Fixes (when run on GL host):
> > dEQP-GLES31.functional.texture.multisample.samples_1.sample_position
> > dEQP-GLES31.functional.texture.multisample.samples_2.sample_position
> > dEQP-GLES31.functional.texture.multisample.samples_3.sample_position
> > dEQP-GLES31.functional.texture.multisample.samples_4.sample_position
> > dEQP-GLES31.functional.texture.multisample.samples_8.sample_position
> > dEQP-GLES31.functional.texture.multisample.samples_10.sample_position
> > dEQP-GLES31.functional.texture.multisample.samples_12.sample_position
> > dEQP-GLES31.functional.texture.multisample.samples_13.sample_position
> > dEQP-GLES31.functional.texture.multisample.samples_16.sample_position
> >
> > v2: remove unrelated chunk (thanks Ilia Mirkin)
> > v3: - also return positions for intermediate sample counts
> > - fix unused varible warning
> >     - update description
> > v4: explain better what this patch assumes and how it handles sample numbers
> > that are not directly advertised (thanks go to Erik Faye-Lund for making
> > me aware that this should be documented)
> >
> > Signed-off-by: Gert Wollny 
> > ---
> > I left the debug_printf in there, because this patch (together with the
> > related virglrenderer patch) is not sufficient to fix above tests on a GLES
> > host.
> >
> >  src/gallium/drivers/virgl/virgl_context.c | 38 
> > +++
> >  src/gallium/drivers/virgl/virgl_hw.h  |  1 +
> >  2 files changed, 39 insertions(+)
> >
> > diff --git a/src/gallium/drivers/virgl/virgl_context.c 
> > b/src/gallium/drivers/virgl/virgl_context.c
> > index e6f8dc8525..43c141e42d 100644
> > --- a/src/gallium/drivers/virgl/virgl_context.c
> > +++ b/src/gallium/drivers/virgl/virgl_context.c
> > @@ -920,6 +920,42 @@ virgl_context_destroy( struct pipe_context *ctx )
> > FREE(vctx);
> >  }
> >
> > +static void virgl_get_sample_position(struct pipe_context *ctx,
> > +  unsigned sample_count,
> > +  unsigned index,
> > +  float *out_value)
> > +{
> > +   struct virgl_context *vctx = virgl_context(ctx);
> > +   struct virgl_screen *vs = virgl_screen(vctx->base.screen);
> > +
> > +   if (sample_count > vs->caps.caps.v1.max_samples) {
> > +  debug_printf("VIRGL: requested %d MSAA samples, but only %d 
> > supported\n",
> > +   sample_count, vs->caps.caps.v1.max_samples);
> > +  return;
> > +   }
> > +
> > +   /* The following is basically copied from 
> > dri/i965gen6_get_sample_position
> > +* The only addition is that we hold the msaa positions for all sample
> > +* counts in a flat array. */
> > +   uint32_t bits = 0;
> > +   if (sample_count == 1) {
> > +  out_value[0] = out_value[1] = 0.5f;
> > +  return;
> > +   } else if (sample_count == 2) {
> > +  bits = vs->caps.caps.v2.msaa_sample_positions[0] >> (8 * index);
> > +   } else if (sample_count < 4) {
> > +  bits = vs->caps.caps.v2.msaa_sample_positions[1] >> (8 * index);
> > +   } else if (sample_count < 8) {
> > +  bits = vs->caps.caps.v2.msaa_sample_positions[2 + (index >> 2)] >> 
> > (8 * (index & 3));
> > +   } else if (sample_count < 8) {
> > +  bits = vs->caps.caps.v2.msaa_sample_positions[4 + (index >> 2)] >> 
> > (8 * (index & 3));
> > +   }
> > +   out_value[0] = ((bits >> 4) & 0xf) / 16.0f;
> > +   out_value[1] = (bits & 0xf) / 16.0f;
> > +   debug_printf("VIRGL: sample postion [%2d/%2d] = (%f, %f)\n",
> > +index, sample_count, out_value[0], out_value[1]);
>

Re: [Mesa-dev] [PATCH v4] virgl: Add support for glGetMultisample

[Erik Faye-Lund]

On Fri, Jun 29, 2018 at 12:39 PM Gert Wollny  wrote:
>
> Use caps to obtain the multisample sample positions for up to 16
> positions and implement the according Gallium interface.
>
> This implemenation (plus its counterpart in virglrenderer) assume that
> the fixed sample position are always the same for a given number of samples
> over the whole live time of a qemu session. It also assumes that sample
> series are only given for 2, 4, 8, and 16 samples, and for intermediate
> numbers N of samples the next higher supported set from above list is picked
> and the sample positions for the first N samples are returned accordingly.
>
> Fixes (when run on GL host):
> dEQP-GLES31.functional.texture.multisample.samples_1.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_2.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_3.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_4.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_8.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_10.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_12.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_13.sample_position
> dEQP-GLES31.functional.texture.multisample.samples_16.sample_position
>
> v2: remove unrelated chunk (thanks Ilia Mirkin)
> v3: - also return positions for intermediate sample counts
> - fix unused varible warning
> - update description
> v4: explain better what this patch assumes and how it handles sample numbers
> that are not directly advertised (thanks go to Erik Faye-Lund for making
> me aware that this should be documented)
>
> Signed-off-by: Gert Wollny 
> ---
> I left the debug_printf in there, because this patch (together with the
> related virglrenderer patch) is not sufficient to fix above tests on a GLES
> host.
>
>  src/gallium/drivers/virgl/virgl_context.c | 38 
> +++
>  src/gallium/drivers/virgl/virgl_hw.h  |  1 +
>  2 files changed, 39 insertions(+)
>
> diff --git a/src/gallium/drivers/virgl/virgl_context.c 
> b/src/gallium/drivers/virgl/virgl_context.c
> index e6f8dc8525..43c141e42d 100644
> --- a/src/gallium/drivers/virgl/virgl_context.c
> +++ b/src/gallium/drivers/virgl/virgl_context.c
> @@ -920,6 +920,42 @@ virgl_context_destroy( struct pipe_context *ctx )
> FREE(vctx);
>  }
>
> +static void virgl_get_sample_position(struct pipe_context *ctx,
> +  unsigned sample_count,
> +  unsigned index,
> +  float *out_value)
> +{
> +   struct virgl_context *vctx = virgl_context(ctx);
> +   struct virgl_screen *vs = virgl_screen(vctx->base.screen);
> +
> +   if (sample_count > vs->caps.caps.v1.max_samples) {
> +  debug_printf("VIRGL: requested %d MSAA samples, but only %d 
> supported\n",
> +   sample_count, vs->caps.caps.v1.max_samples);
> +  return;
> +   }
> +
> +   /* The following is basically copied from dri/i965gen6_get_sample_position
> +* The only addition is that we hold the msaa positions for all sample
> +* counts in a flat array. */
> +   uint32_t bits = 0;
> +   if (sample_count == 1) {
> +  out_value[0] = out_value[1] = 0.5f;
> +  return;
> +   } else if (sample_count == 2) {
> +  bits = vs->caps.caps.v2.msaa_sample_positions[0] >> (8 * index);
> +   } else if (sample_count < 4) {
> +  bits = vs->caps.caps.v2.msaa_sample_positions[1] >> (8 * index);
> +   } else if (sample_count < 8) {
> +  bits = vs->caps.caps.v2.msaa_sample_positions[2 + (index >> 2)] >> (8 
> * (index & 3));
> +   } else if (sample_count < 8) {
> +  bits = vs->caps.caps.v2.msaa_sample_positions[4 + (index >> 2)] >> (8 
> * (index & 3));
> +   }
> +   out_value[0] = ((bits >> 4) & 0xf) / 16.0f;
> +   out_value[1] = (bits & 0xf) / 16.0f;
> +   debug_printf("VIRGL: sample postion [%2d/%2d] = (%f, %f)\n",
> +index, sample_count, out_value[0], out_value[1]);

Are you sure that an unconditional debug_printf in the success case
here is a good idea?

AFAIK, most debug_printfs are either in error paths, or guarded by a
compile-time flag, a run-time flag (typically environment variables)
or something like that.

The most kosher way of doing this would be with a driver-specific
debug-flag. Sadly, it seems there's no such flag in virgl yet for
this, and maybe this is a too small detail to bother with this? If so,
quite a lot of places just throw an #if 0 around it so it's easy to
enable for developers in the future...

Either way:
Reviewed-by: Erik Faye-Lund 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] mesa: allow EXT_texture_compression_s3tc on ES2

[Erik Faye-Lund]

The extension is written against both GL 1.2 and GLES 2.0, so let's
also enable it on GLES 2.0.

Signed-off-by: Erik Faye-Lund 
---
 src/mesa/main/extensions_table.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
index 09bf923bd0e..47db1583135 100644
--- a/src/mesa/main/extensions_table.h
+++ b/src/mesa/main/extensions_table.h
@@ -278,7 +278,7 @@ EXT(EXT_texture_buffer  , 
OES_texture_buffer
 EXT(EXT_texture_compression_dxt1, ANGLE_texture_compression_dxt
  , GLL, GLC, ES1, ES2, 2004)
 EXT(EXT_texture_compression_latc, EXT_texture_compression_latc 
  , GLL,  x ,  x ,  x , 2006)
 EXT(EXT_texture_compression_rgtc, ARB_texture_compression_rgtc 
  , GLL, GLC,  x ,  x , 2004)
-EXT(EXT_texture_compression_s3tc, EXT_texture_compression_s3tc 
  , GLL, GLC,  x ,  x , 2000)
+EXT(EXT_texture_compression_s3tc, EXT_texture_compression_s3tc 
  , GLL, GLC,  x , ES2, 2000)
 EXT(EXT_texture_cube_map, ARB_texture_cube_map 
  , GLL,  x ,  x ,  x , 2001)
 EXT(EXT_texture_cube_map_array  , OES_texture_cube_map_array   
  ,  x ,  x ,  x ,  31, 2014)
 EXT(EXT_texture_edge_clamp  , dummy_true   
  , GLL,  x ,  x ,  x , 1997)
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH] mesa: allow EXT_texture_compression_s3tc on ES2

[Erik Faye-Lund]

On Oct 18, 2018 18:21, Ilia Mirkin  wrote:Have you verified that this works OK on GLES2? This extension enables

online compression, which isn't normally available in GLES, and I

wouldn't be surprised if the current mesa TexImage handlers prevent

it.


The way I read the extension spec, it doesn't enable online compression on gles2. And from testing, it seems like mesa does the right thing. I'll double check, though...
  -ilia

On Thu, Oct 18, 2018 at 11:05 AM Erik Faye-Lund

 wrote:

>

> The extension is written against both GL 1.2 and GLES 2.0, so let's

> also enable it on GLES 2.0.

>

> Signed-off-by: Erik Faye-Lund 

> ---

>  src/mesa/main/extensions_table.h | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h

> index 09bf923bd0e..47db1583135 100644

> --- a/src/mesa/main/extensions_table.h

> +++ b/src/mesa/main/extensions_table.h

> @@ -278,7 +278,7 @@ EXT(EXT_texture_buffer  , OES_texture_buffer

>  EXT(EXT_texture_compression_dxt1    , ANGLE_texture_compression_dxt  , GLL, GLC, ES1, ES2, 2004)

>  EXT(EXT_texture_compression_latc    , EXT_texture_compression_latc   , GLL,  x ,  x ,  x , 2006)

>  EXT(EXT_texture_compression_rgtc    , ARB_texture_compression_rgtc   , GLL, GLC,  x ,  x , 2004)

> -EXT(EXT_texture_compression_s3tc    , EXT_texture_compression_s3tc   , GLL, GLC,  x ,  x , 2000)

> +EXT(EXT_texture_compression_s3tc    , EXT_texture_compression_s3tc   , GLL, GLC,  x , ES2, 2000)

>  EXT(EXT_texture_cube_map    , ARB_texture_cube_map   , GLL,  x ,  x ,  x , 2001)

>  EXT(EXT_texture_cube_map_array  , OES_texture_cube_map_array ,  x ,  x ,  x ,  31, 2014)

>  EXT(EXT_texture_edge_clamp  , dummy_true , GLL,  x ,  x ,  x , 1997)

> --

> 2.17.1

>

> ___

> mesa-dev mailing list

> mesa-dev@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

Re: [Mesa-dev] [PATCH] mesa: allow EXT_texture_compression_s3tc on ES2

[Erik Faye-Lund]

On Thu, 2018-10-18 at 14:31 -0400, Ilia Mirkin wrote:
> On Thu, Oct 18, 2018 at 2:26 PM Erik Faye-Lund
>  wrote:
> > On Oct 18, 2018 18:21, Ilia Mirkin  wrote:
> > 
> > Have you verified that this works OK on GLES2? This extension
> > enables
> > online compression, which isn't normally available in GLES, and I
> > wouldn't be surprised if the current mesa TexImage handlers prevent
> > it.
> > 
> > The way I read the extension spec, it doesn't enable online
> > compression on gles2. And from testing, it seems like mesa does the
> > right thing. I'll double check, though...
> 
> """
> In extended OpenGL ES 2.0.25 these new tokens are accepted by the
>  parameter of TexImage2D, CompressedTexImage2D
> and
> the 
> parameter of CompressedTexSubImage2D.
> """
> 
> and in the ES 2 section of the modifications:
> 
> """
> Modify Section 3.7.2, Alternate Texture Image Specification
> Commands
> 
> (add to the end of section)
> 
> When the internal format of the texture object is
> COMPRESSED_RGB_S3TC_DXT1_EXT, COMPRESSED_RGBA_S3TC_DXT1_EXT,
> COMPRESSED_RGBA_S3TC_DXT3_EXT, or COMPRESSED_RGBA_S3TC_DXT5_EXT,
> the
> update region specified in TexSubImage2D must be aligned to 4x4
> pixel blocks. If  or  are not multiples of 4 an
> INVALID_OPERATION error is generated. If  is not a
> multiple
> of 4 and  +  is not equal to the width of the LOD
> then an INVALID_OPERATION error is generated.  If  is not
> a multiple of 4 and  +  is not equal to the
> height of the LOD then an INVALID_OPERATION error is generated.
> """
> 
> Since you can pass the compressed internal format to TexImage2D, that
> implies online compression.
> 

Right. Yeah, I think you're right, and this needs some more work.

Thanks for pointing that out.

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

Re: [Mesa-dev] [PATCH] mesa: allow EXT_texture_compression_s3tc on ES2

[Erik Faye-Lund]

On Thu, 2018-10-18 at 15:42 -0400, Marek Olšák wrote:
> I think you need something like this:
> 
https://cgit.freedesktop.org/~mareko/mesa/commit/?h=amd-extension-pack=ad774f9db1d735811a8d830ad90a2f8208aa0a7b
> 

Thanks, this looks correct to me.

I've ported some of the piglit s3tc tests to gles2, and they all seem
to pass with your patch:

https://patchwork.freedesktop.org/series/51467/

Do you mind if I add a "Tested-by"-tag and submit a v2 with your patch
instead?

> I also have:
> rgtc: 
> https://cgit.freedesktop.org/~mareko/mesa/commit/?h=amd-extension-pack=c9c0ffc2d40f0119fb31bf0515f321cd877090dd
> bptc: 
> https://cgit.freedesktop.org/~mareko/mesa/commit/?h=amd-extension-pack=111255b6f8b85119da2c5d29dc19abc422fd7a12
> 
> and no time to test them.
> 
> Marek
> 
> On Thu, Oct 18, 2018 at 11:05 AM Erik Faye-Lund <
> erik.faye-l...@collabora.com> wrote:
> > The extension is written against both GL 1.2 and GLES 2.0, so let's
> > also enable it on GLES 2.0.
> > 
> > Signed-off-by: Erik Faye-Lund 
> > ---
> >  src/mesa/main/extensions_table.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/main/extensions_table.h
> > b/src/mesa/main/extensions_table.h
> > index 09bf923bd0e..47db1583135 100644
> > --- a/src/mesa/main/extensions_table.h
> > +++ b/src/mesa/main/extensions_table.h
> > @@ -278,7 +278,7 @@ EXT(EXT_texture_buffer  ,
> > OES_texture_buffer
> >  EXT(EXT_texture_compression_dxt1,
> > ANGLE_texture_compression_dxt  , GLL, GLC, ES1, ES2, 2004)
> >  EXT(EXT_texture_compression_latc,
> > EXT_texture_compression_latc   , GLL,  x ,  x ,  x , 2006)
> >  EXT(EXT_texture_compression_rgtc,
> > ARB_texture_compression_rgtc   , GLL, GLC,  x ,  x , 2004)
> > -EXT(EXT_texture_compression_s3tc,
> > EXT_texture_compression_s3tc   , GLL, GLC,  x ,  x , 2000)
> > +EXT(EXT_texture_compression_s3tc,
> > EXT_texture_compression_s3tc   , GLL, GLC,  x , ES2, 2000)
> >  EXT(EXT_texture_cube_map,
> > ARB_texture_cube_map   , GLL,  x ,  x ,  x , 2001)
> >  EXT(EXT_texture_cube_map_array  ,
> > OES_texture_cube_map_array ,  x ,  x ,  x ,  31, 2014)
> >  EXT(EXT_texture_edge_clamp  , dummy_true 
> >, GLL,  x ,  x ,  x , 1997)

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

Re: [Mesa-dev] [PATCH 3/5] glsl: fall back to inexact function-match

[Erik Faye-Lund]

On Wed, 2018-10-31 at 15:46 +0200, Tapani Pälli wrote:
> 
> On 10/30/18 7:11 PM, Erik Faye-Lund wrote:
> > In GLES, we currently either need an exact match with a local
> > function,
> > or an exact match with a builtin.
> > 
> > However, if we add support for implicit conversions for GLES
> > shaders,
> > we also need to fall back to a non-exact match in the case where
> > there
> > were no builtin match either.
> > 
> > Luckily, we already have a variable ready with this, so let's just
> > return it if the builtin-search failed.
> > 
> > Signed-off-by: Erik Faye-Lund 
> > ---
> >   src/compiler/glsl/ast_function.cpp | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/compiler/glsl/ast_function.cpp
> > b/src/compiler/glsl/ast_function.cpp
> > index 1fa3f7561ae..50fd8bc5f5d 100644
> > --- a/src/compiler/glsl/ast_function.cpp
> > +++ b/src/compiler/glsl/ast_function.cpp
> > @@ -667,7 +667,7 @@ match_function_by_name(const char *name,
> >  /* Local shader has no exact candidates; check the built-ins.
> > */
> >  _mesa_glsl_initialize_builtin_functions();
> >  sig = _mesa_glsl_find_builtin_function(state, name,
> > actual_parameters);
> > -   return sig;
> > +   return sig ? sig : local_sig;
> 
> I think this would deserve a comment about 'local_sig', it did not
> seem 
> obvious ... maybe something like:
> 
> /* Variable local_sig contains the result from 
> choose_best_inexact_overload(). */
> 

Yeah, good point. I've changed it to this, which feels like it explains
the point without spoon-feeding the reader too much... How does that
sound?

---8<---
diff --git a/src/compiler/glsl/ast_function.cpp
b/src/compiler/glsl/ast_function.cpp
index 1fa3f7561ae..6aa30400060 100644
--- a/src/compiler/glsl/ast_function.cpp
+++ b/src/compiler/glsl/ast_function.cpp
@@ -667,7 +667,11 @@ match_function_by_name(const char *name,
/* Local shader has no exact candidates; check the built-ins. */
_mesa_glsl_initialize_builtin_functions();
sig = _mesa_glsl_find_builtin_function(state, name,
actual_parameters);
-   return sig;
+
+   /* if _mesa_glsl_find_builtin_function failed, fall back to the
result
+* of choose_best_inexact_overload() instead
+*/
+   return sig ? sig : local_sig;
 }
 
 static ir_function_signature *
---8<---


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

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

Re: [Mesa-dev] [PATCH 3/5] glsl: fall back to inexact function-match

[Erik Faye-Lund]

On Wed, 2018-10-31 at 12:01 -0400, Ilia Mirkin wrote:
> I had to do a double (or triple) take on this logic as well. Part of
> the subtlety is that the fallback only applies for ES when there's a
> match but no exact match. Probably good to mention this.

Yeah, that makes sense. I thought I mentioneded this in the commit
message, but perhaps you want that to be more explicit than the "In
GLES, "-introduction?

How about I simply add something like "This fallback should only affect
GLES." at the end of the commit message?

> On Wed, Oct 31, 2018 at 11:13 AM Erik Faye-Lund
>  wrote:
> > 
> > On Wed, 2018-10-31 at 15:46 +0200, Tapani Pälli wrote:
> > > 
> > > On 10/30/18 7:11 PM, Erik Faye-Lund wrote:
> > > > In GLES, we currently either need an exact match with a local
> > > > function,
> > > > or an exact match with a builtin.
> > > > 
> > > > However, if we add support for implicit conversions for GLES
> > > > shaders,
> > > > we also need to fall back to a non-exact match in the case
> > > > where
> > > > there
> > > > were no builtin match either.
> > > > 
> > > > Luckily, we already have a variable ready with this, so let's
> > > > just
> > > > return it if the builtin-search failed.
> > > > 
> > > > Signed-off-by: Erik Faye-Lund 
> > > > ---
> > > >   src/compiler/glsl/ast_function.cpp | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/compiler/glsl/ast_function.cpp
> > > > b/src/compiler/glsl/ast_function.cpp
> > > > index 1fa3f7561ae..50fd8bc5f5d 100644
> > > > --- a/src/compiler/glsl/ast_function.cpp
> > > > +++ b/src/compiler/glsl/ast_function.cpp
> > > > @@ -667,7 +667,7 @@ match_function_by_name(const char *name,
> > > >  /* Local shader has no exact candidates; check the built-
> > > > ins.
> > > > */
> > > >  _mesa_glsl_initialize_builtin_functions();
> > > >  sig = _mesa_glsl_find_builtin_function(state, name,
> > > > actual_parameters);
> > > > -   return sig;
> > > > +   return sig ? sig : local_sig;
> > > 
> > > I think this would deserve a comment about 'local_sig', it did
> > > not
> > > seem
> > > obvious ... maybe something like:
> > > 
> > > /* Variable local_sig contains the result from
> > > choose_best_inexact_overload(). */
> > > 
> > 
> > Yeah, good point. I've changed it to this, which feels like it
> > explains
> > the point without spoon-feeding the reader too much... How does
> > that
> > sound?
> > 
> > ---8<---
> > diff --git a/src/compiler/glsl/ast_function.cpp
> > b/src/compiler/glsl/ast_function.cpp
> > index 1fa3f7561ae..6aa30400060 100644
> > --- a/src/compiler/glsl/ast_function.cpp
> > +++ b/src/compiler/glsl/ast_function.cpp
> > @@ -667,7 +667,11 @@ match_function_by_name(const char *name,
> > /* Local shader has no exact candidates; check the built-ins.
> > */
> > _mesa_glsl_initialize_builtin_functions();
> > sig = _mesa_glsl_find_builtin_function(state, name,
> > actual_parameters);
> > -   return sig;
> > +
> > +   /* if _mesa_glsl_find_builtin_function failed, fall back to the
> > result
> > +* of choose_best_inexact_overload() instead
> > +*/
> > +   return sig ? sig : local_sig;
> >  }
> > 
> >  static ir_function_signature *
> > ---8<---
> > 
> > 
> > > >   }
> > > > 
> > > >   static ir_function_signature *
> > > > 
> > > 
> > > ___
> > > 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 3/5] glsl: fall back to inexact function-match

[Erik Faye-Lund]

On Thu, 2018-11-01 at 07:49 +0200, Tapani Pälli wrote:
> 
> On 10/31/18 7:03 PM, Ilia Mirkin wrote:
> > On Wed, Oct 31, 2018 at 12:37 PM Erik Faye-Lund
> >  wrote:
> > > 
> > > On Wed, 2018-10-31 at 12:01 -0400, Ilia Mirkin wrote:
> > > > I had to do a double (or triple) take on this logic as well.
> > > > Part of
> > > > the subtlety is that the fallback only applies for ES when
> > > > there's a
> > > > match but no exact match. Probably good to mention this.
> > > 
> > > Yeah, that makes sense. I thought I mentioneded this in the
> > > commit
> > > message, but perhaps you want that to be more explicit than the
> > > "In
> > > GLES, "-introduction?
> > > 
> > > How about I simply add something like "This fallback should only
> > > affect
> > > GLES." at the end of the commit message?
> > 
> > Yes, you did mention it in the commit message. But that's unlikely
> > to
> > be visible when reading this code. I was thinking something in this
> > function's comments would be nice.
> > 
> 
> Sounds good, I wouldn't be worried about saying it aloud, at least I 
> would love some 'spoon-feeding comments' now and then with compiler
> and 
> linker parts :)
> 
> 

OK, fair enough. How about this?

---8<---
--- a/src/compiler/glsl/ast_function.cpp
+++ b/src/compiler/glsl/ast_function.cpp
@@ -667,7 +667,12 @@ match_function_by_name(const char *name,
/* Local shader has no exact candidates; check the built-ins. */
_mesa_glsl_initialize_builtin_functions();
sig = _mesa_glsl_find_builtin_function(state, name,
actual_parameters);
-   return sig;
+
+   /* if _mesa_glsl_find_builtin_function failed, fall back to th
e result
+* of choose_best_inexact_overload() instead. This should only
affect
+* GLES.
+*/
+   return sig ? sig : local_sig;
 }
 
 static ir_function_signature *
---8<---

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

Re: [Mesa-dev] [PATCH] mesa: expose EXT_texture_compression_s3tc on GLES

[Erik Faye-Lund]

On Tue, 2018-10-30 at 08:20 -0400, Ilia Mirkin wrote:
> On Tue, Oct 30, 2018 at 7:59 AM Erik Faye-Lund
>  wrote:
> > 
> > On Tue, 2018-10-30 at 07:44 -0400, Ilia Mirkin wrote:
> > > On Thu, Oct 25, 2018 at 6:59 AM Erik Faye-Lund
> > >  wrote:
> > > > 
> > > > From: Marek Olšák 
> > > > 
> > > > The spec was modified to support GLES.
> > > > 
> > > > Tested-by: Erik Faye-Lund 
> > > > ---
> > > > This replaces this patch:
> > > > https://patchwork.freedesktop.org/patch/257423/
> > > > 
> > > >  docs/relnotes/18.3.0.html|  1 +
> > > >  src/mesa/main/extensions_table.h |  2 +-
> > > >  src/mesa/main/glformats.c| 11 +++
> > > >  3 files changed, 13 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/docs/relnotes/18.3.0.html
> > > > b/docs/relnotes/18.3.0.html
> > > > index 5874d3fa330..e0061872de4 100644
> > > > --- a/docs/relnotes/18.3.0.html
> > > > +++ b/docs/relnotes/18.3.0.html
> > > > @@ -57,6 +57,7 @@ Note: some of the new features are only
> > > > available
> > > > with certain drivers.
> > > >  GL_AMD_multi_draw_indirect on all GL 4.x drivers.
> > > >  GL_AMD_query_buffer_object on i965, nvc0, r600,
> > > > radeonsi.
> > > >  GL_EXT_disjoint_timer_query on radeonsi and most other
> > > > Gallium
> > > > drivers (ES extension)
> > > > +GL_EXT_texture_compression_s3tc on all drivers (ES
> > > > extension)
> > > >  GL_EXT_vertex_attrib_64bit on i965, nvc0, radeonsi.
> > > >  GL_EXT_window_rectangles on radeonsi.
> > > >  GL_KHR_texture_compression_astc_sliced_3d on
> > > > radeonsi.
> > > > diff --git a/src/mesa/main/extensions_table.h
> > > > b/src/mesa/main/extensions_table.h
> > > > index 09bf923bd0e..47db1583135 100644
> > > > --- a/src/mesa/main/extensions_table.h
> > > > +++ b/src/mesa/main/extensions_table.h
> > > > @@ -278,7 +278,7 @@
> > > > EXT(EXT_texture_buffer  ,
> > > > OES_texture_buffer
> > > >  EXT(EXT_texture_compression_dxt1,
> > > > ANGLE_texture_compression_dxt  , GLL, GLC, ES1, ES2,
> > > > 2004)
> > > >  EXT(EXT_texture_compression_latc,
> > > > EXT_texture_compression_latc   , GLL,  x ,  x ,  x ,
> > > > 2006)
> > > >  EXT(EXT_texture_compression_rgtc,
> > > > ARB_texture_compression_rgtc   , GLL, GLC,  x ,  x ,
> > > > 2004)
> > > > -EXT(EXT_texture_compression_s3tc,
> > > > EXT_texture_compression_s3tc   , GLL, GLC,  x ,  x ,
> > > > 2000)
> > > > +EXT(EXT_texture_compression_s3tc,
> > > > EXT_texture_compression_s3tc   , GLL, GLC,  x , ES2,
> > > > 2000)
> > > >  EXT(EXT_texture_cube_map,
> > > > ARB_texture_cube_map   , GLL,  x ,  x ,  x ,
> > > > 2001)
> > > >  EXT(EXT_texture_cube_map_array  ,
> > > > OES_texture_cube_map_array ,  x ,  x ,  x ,  31,
> > > > 2014)
> > > >  EXT(EXT_texture_edge_clamp  ,
> > > > dummy_true , GLL,  x ,  x ,  x ,
> > > > 1997)
> > > > diff --git a/src/mesa/main/glformats.c
> > > > b/src/mesa/main/glformats.c
> > > > index 6cb3435dea2..f8fc36e9311 100644
> > > > --- a/src/mesa/main/glformats.c
> > > > +++ b/src/mesa/main/glformats.c
> > > > @@ -2803,6 +2803,17 @@
> > > > _mesa_es3_error_check_format_and_type(const
> > > > struct gl_context *ctx,
> > > >internalFormat = effectiveInternalFormat;
> > > > }
> > > > 
> > > > +   /* The GLES variant of EXT_texture_compression_s3tc is very
> > > > vague and
> > > > +* doesn't list valid types. Just do exactly what the spec
> > > > says.
> > > > +*/
> > > > +   if (ctx->Extensions.EXT_texture_compression_s3tc &&
> > > > +   (internalFormat == GL_COMPRESSED_RGB_S3TC_DXT1_EXT ||
> > > > +internalFormat == GL_COMPRESSED_RGBA_S3TC_DXT1_EXT ||
> > > > +internalFormat == GL_COMPRESSED_RGBA_S3TC_DXT3_EXT ||
> > > > +internalFor

Re: [Mesa-dev] [PATCH] mesa: expose EXT_texture_compression_s3tc on GLES

[Erik Faye-Lund]

On Tue, 2018-10-30 at 07:44 -0400, Ilia Mirkin wrote:
> On Thu, Oct 25, 2018 at 6:59 AM Erik Faye-Lund
>  wrote:
> > 
> > From: Marek Olšák 
> > 
> > The spec was modified to support GLES.
> > 
> > Tested-by: Erik Faye-Lund 
> > ---
> > This replaces this patch:
> > https://patchwork.freedesktop.org/patch/257423/
> > 
> >  docs/relnotes/18.3.0.html|  1 +
> >  src/mesa/main/extensions_table.h |  2 +-
> >  src/mesa/main/glformats.c| 11 +++
> >  3 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/docs/relnotes/18.3.0.html b/docs/relnotes/18.3.0.html
> > index 5874d3fa330..e0061872de4 100644
> > --- a/docs/relnotes/18.3.0.html
> > +++ b/docs/relnotes/18.3.0.html
> > @@ -57,6 +57,7 @@ Note: some of the new features are only available
> > with certain drivers.
> >  GL_AMD_multi_draw_indirect on all GL 4.x drivers.
> >  GL_AMD_query_buffer_object on i965, nvc0, r600, radeonsi.
> >  GL_EXT_disjoint_timer_query on radeonsi and most other Gallium
> > drivers (ES extension)
> > +GL_EXT_texture_compression_s3tc on all drivers (ES
> > extension)
> >  GL_EXT_vertex_attrib_64bit on i965, nvc0, radeonsi.
> >  GL_EXT_window_rectangles on radeonsi.
> >  GL_KHR_texture_compression_astc_sliced_3d on radeonsi.
> > diff --git a/src/mesa/main/extensions_table.h
> > b/src/mesa/main/extensions_table.h
> > index 09bf923bd0e..47db1583135 100644
> > --- a/src/mesa/main/extensions_table.h
> > +++ b/src/mesa/main/extensions_table.h
> > @@ -278,7 +278,7 @@ EXT(EXT_texture_buffer  ,
> > OES_texture_buffer
> >  EXT(EXT_texture_compression_dxt1,
> > ANGLE_texture_compression_dxt  , GLL, GLC, ES1, ES2, 2004)
> >  EXT(EXT_texture_compression_latc,
> > EXT_texture_compression_latc   , GLL,  x ,  x ,  x , 2006)
> >  EXT(EXT_texture_compression_rgtc,
> > ARB_texture_compression_rgtc   , GLL, GLC,  x ,  x , 2004)
> > -EXT(EXT_texture_compression_s3tc,
> > EXT_texture_compression_s3tc   , GLL, GLC,  x ,  x , 2000)
> > +EXT(EXT_texture_compression_s3tc,
> > EXT_texture_compression_s3tc   , GLL, GLC,  x , ES2, 2000)
> >  EXT(EXT_texture_cube_map,
> > ARB_texture_cube_map   , GLL,  x ,  x ,  x , 2001)
> >  EXT(EXT_texture_cube_map_array  ,
> > OES_texture_cube_map_array ,  x ,  x ,  x ,  31, 2014)
> >  EXT(EXT_texture_edge_clamp  ,
> > dummy_true , GLL,  x ,  x ,  x , 1997)
> > diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> > index 6cb3435dea2..f8fc36e9311 100644
> > --- a/src/mesa/main/glformats.c
> > +++ b/src/mesa/main/glformats.c
> > @@ -2803,6 +2803,17 @@ _mesa_es3_error_check_format_and_type(const
> > struct gl_context *ctx,
> >internalFormat = effectiveInternalFormat;
> > }
> > 
> > +   /* The GLES variant of EXT_texture_compression_s3tc is very
> > vague and
> > +* doesn't list valid types. Just do exactly what the spec
> > says.
> > +*/
> > +   if (ctx->Extensions.EXT_texture_compression_s3tc &&
> > +   (internalFormat == GL_COMPRESSED_RGB_S3TC_DXT1_EXT ||
> > +internalFormat == GL_COMPRESSED_RGBA_S3TC_DXT1_EXT ||
> > +internalFormat == GL_COMPRESSED_RGBA_S3TC_DXT3_EXT ||
> > +internalFormat == GL_COMPRESSED_RGBA_S3TC_DXT5_EXT))
> > +  return format == GL_RGB || format == GL_RGBA ? GL_NO_ERROR :
> > + GL_INVALID_OP
> > ERATION;
> 
> Presumably GL_RGB only for GL_COMPRESSED_RGB_S3TC_DXT1_EXT and
> GL_RGBA
> for the others?
> 

Actually, this is AFAICT what the comment refers to. The extension spec
says:

"
Components are then selected from the resulting R, G, B, or A
values to obtain a texture with the base internal format specified 
by , which must match  except when 
is TEXTURE_2D and  is one of the following
compressed formats: COMPRESSED_RGB_S3TC_DXT1_EXT,
COMPRESSED_RGBA_S3TC_DXT1_EXT, COMPRESSED_RGBA_S3TC_DXT3_EXT, or
COMPRESSED_RGBA_S3TC_DXT5_EXT. In this case, conversion from only
RGB and RGBA formats are supported during texture image processing.
 values other than RBA or RGBA will result in the
INVALID_OPERATION error. In all other cases where 
does not match , the error INVALID_OPERATION is generated.
"

This implies, as far as I can read, that RGB <-> RGBA is allowed. It's
a bit strang

[Mesa-dev] [PATCH 2/5] glsl: add has_implicit_uint_to_int_conversion()-helper

[Erik Faye-Lund]

This makes the code a bit easier to read, as well as reduces repetition,
especially when we add support for EXT_shader_implicit_conversions.

Signed-off-by: Erik Faye-Lund 
---
 src/compiler/glsl/ast_to_hir.cpp   | 3 +--
 src/compiler/glsl/glsl_parser_extras.h | 7 +++
 src/compiler/glsl_types.cpp| 3 +--
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 6d4b2c6aa5d..7f30a708855 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -250,8 +250,7 @@ get_implicit_conversion_operation(const glsl_type *to, 
const glsl_type *from,
   }
 
case GLSL_TYPE_UINT:
-  if (!state->is_version(400, 0) && !state->ARB_gpu_shader5_enable
-  && !state->MESA_shader_integer_functions_enable)
+  if (!state->has_implicit_uint_to_int_conversion())
  return (ir_expression_operation)0;
   switch (from->base_type) {
  case GLSL_TYPE_INT: return ir_unop_i2u;
diff --git a/src/compiler/glsl/glsl_parser_extras.h 
b/src/compiler/glsl/glsl_parser_extras.h
index edd41601c35..e1144a19c15 100644
--- a/src/compiler/glsl/glsl_parser_extras.h
+++ b/src/compiler/glsl/glsl_parser_extras.h
@@ -349,6 +349,13 @@ struct _mesa_glsl_parse_state {
   return is_version(120, 0);
}
 
+   bool has_implicit_uint_to_int_conversion() const
+   {
+  return ARB_gpu_shader5_enable ||
+ MESA_shader_integer_functions_enable ||
+ is_version(400, 0);
+   }
+
void process_version_directive(YYLTYPE *locp, int version,
   const char *ident);
 
diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
index bcc36e50d7f..e6262371bd0 100644
--- a/src/compiler/glsl_types.cpp
+++ b/src/compiler/glsl_types.cpp
@@ -1446,8 +1446,7 @@ glsl_type::can_implicitly_convert_to(const glsl_type 
*desired,
 * state-dependent checks have already happened though, so allow anything
 * that's allowed in any shader version.
 */
-   if ((!state || state->is_version(400, 0) || state->ARB_gpu_shader5_enable ||
-state->MESA_shader_integer_functions_enable) &&
+   if ((!state || state->has_implicit_uint_to_int_conversion()) &&
  desired->base_type == GLSL_TYPE_UINT && this->base_type == 
GLSL_TYPE_INT)
   return true;
 
-- 
2.17.2

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

[Mesa-dev] [PATCH 3/5] glsl: fall back to inexact function-match

[Erik Faye-Lund]

In GLES, we currently either need an exact match with a local function,
or an exact match with a builtin.

However, if we add support for implicit conversions for GLES shaders,
we also need to fall back to a non-exact match in the case where there
were no builtin match either.

Luckily, we already have a variable ready with this, so let's just
return it if the builtin-search failed.

Signed-off-by: Erik Faye-Lund 
---
 src/compiler/glsl/ast_function.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_function.cpp 
b/src/compiler/glsl/ast_function.cpp
index 1fa3f7561ae..50fd8bc5f5d 100644
--- a/src/compiler/glsl/ast_function.cpp
+++ b/src/compiler/glsl/ast_function.cpp
@@ -667,7 +667,7 @@ match_function_by_name(const char *name,
/* Local shader has no exact candidates; check the built-ins. */
_mesa_glsl_initialize_builtin_functions();
sig = _mesa_glsl_find_builtin_function(state, name, actual_parameters);
-   return sig;
+   return sig ? sig : local_sig;
 }
 
 static ir_function_signature *
-- 
2.17.2

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

[Mesa-dev] [PATCH 4/5] mesa/glsl: add support for EXT_shader_implicit_conversions

[Erik Faye-Lund]

EXT_shader_implicit_conversions adds support for implicit conversions
for GLES 3.1 and above.

This is essentially a subset of ARB_gpu_shader5, and augments
OES_gpu_shader5.

Signed-off-by: Erik Faye-Lund 
---
 src/compiler/glsl/glsl_parser_extras.cpp | 1 +
 src/compiler/glsl/glsl_parser_extras.h   | 5 -
 src/compiler/glsl/ir_function.cpp| 3 ++-
 src/mesa/main/extensions_table.h | 1 +
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 1bdd7c4bf17..2a58908c226 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -718,6 +718,7 @@ static const _mesa_glsl_extension 
_mesa_glsl_supported_extensions[] = {
EXT(EXT_separate_shader_objects),
EXT(EXT_shader_framebuffer_fetch),
EXT(EXT_shader_framebuffer_fetch_non_coherent),
+   EXT(EXT_shader_implicit_conversions),
EXT(EXT_shader_integer_mix),
EXT_AEP(EXT_shader_io_blocks),
EXT(EXT_shader_samples_identical),
diff --git a/src/compiler/glsl/glsl_parser_extras.h 
b/src/compiler/glsl/glsl_parser_extras.h
index e1144a19c15..985200ecab5 100644
--- a/src/compiler/glsl/glsl_parser_extras.h
+++ b/src/compiler/glsl/glsl_parser_extras.h
@@ -346,13 +346,14 @@ struct _mesa_glsl_parse_state {
 
bool has_implicit_conversions() const
{
-  return is_version(120, 0);
+  return EXT_shader_implicit_conversions_enable || is_version(120, 0);
}
 
bool has_implicit_uint_to_int_conversion() const
{
   return ARB_gpu_shader5_enable ||
  MESA_shader_integer_functions_enable ||
+ EXT_shader_implicit_conversions_enable ||
  is_version(400, 0);
}
 
@@ -806,6 +807,8 @@ struct _mesa_glsl_parse_state {
bool EXT_shader_framebuffer_fetch_warn;
bool EXT_shader_framebuffer_fetch_non_coherent_enable;
bool EXT_shader_framebuffer_fetch_non_coherent_warn;
+   bool EXT_shader_implicit_conversions_enable;
+   bool EXT_shader_implicit_conversions_warn;
bool EXT_shader_integer_mix_enable;
bool EXT_shader_integer_mix_warn;
bool EXT_shader_io_blocks_enable;
diff --git a/src/compiler/glsl/ir_function.cpp 
b/src/compiler/glsl/ir_function.cpp
index 3ee0d170fb2..97262f0f4b9 100644
--- a/src/compiler/glsl/ir_function.cpp
+++ b/src/compiler/glsl/ir_function.cpp
@@ -274,7 +274,8 @@ choose_best_inexact_overload(_mesa_glsl_parse_state *state,
 * assume everything supported in any GLSL version is available.
 */
if (!state || state->is_version(400, 0) || state->ARB_gpu_shader5_enable ||
-   state->MESA_shader_integer_functions_enable) {
+   state->MESA_shader_integer_functions_enable ||
+   state->EXT_shader_implicit_conversions_enable) {
   for (ir_function_signature **sig = matches; sig < matches + num_matches; 
sig++) {
  if (is_best_inexact_overload(actual_parameters, matches, num_matches, 
*sig))
 return *sig;
diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
index 47db1583135..e3bf7c9fcee 100644
--- a/src/mesa/main/extensions_table.h
+++ b/src/mesa/main/extensions_table.h
@@ -261,6 +261,7 @@ EXT(EXT_separate_shader_objects , dummy_true
 EXT(EXT_separate_specular_color , dummy_true   
  , GLL,  x ,  x ,  x , 1997)
 EXT(EXT_shader_framebuffer_fetch, EXT_shader_framebuffer_fetch 
  , GLL, GLC,  x , ES2, 2013)
 EXT(EXT_shader_framebuffer_fetch_non_coherent, 
EXT_shader_framebuffer_fetch_non_coherent, GLL, GLC,  x, ES2, 2018)
+EXT(EXT_shader_implicit_conversions , dummy_true   
  ,  x ,  x ,  x ,  31, 2013)
 EXT(EXT_shader_integer_mix  , EXT_shader_integer_mix   
  , GLL, GLC,  x ,  30, 2013)
 EXT(EXT_shader_io_blocks, dummy_true   
  ,  x ,  x ,  x ,  31, 2014)
 EXT(EXT_shader_samples_identical, EXT_shader_samples_identical 
  , GLL, GLC,  x ,  31, 2015)
-- 
2.17.2

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

[Mesa-dev] [PATCH 5/5] glsl: do not allow implicit casts of unsized array initializers

[Erik Faye-Lund]

The GLSL 4.6 specification (section 4.1.14. "Implicit Conversions")
says:

  "There are no implicit array or structure conversions. For
   example, an array of int cannot be implicitly converted to an
   array of float."

So let's add a check in place when assigning array initializers to
implicitly sized arrays, to avoid incorrectly allowing code on the
form:

int[] foo = float[](1.0, 2.0, 3.0)

This fixes the following dEQP test-cases:
- 
dEQP-GLES31.functional.shaders.implicit_conversions.es31.invalid.arrays.int_to_float_vertex
- 
dEQP-GLES31.functional.shaders.implicit_conversions.es31.invalid.arrays.int_to_float_fragment
- 
dEQP-GLES31.functional.shaders.implicit_conversions.es31.invalid.arrays.int_to_uint_vertex
- 
dEQP-GLES31.functional.shaders.implicit_conversions.es31.invalid.arrays.int_to_uint_fragment
- 
dEQP-GLES31.functional.shaders.implicit_conversions.es31.invalid.arrays.uint_to_float_vertex
- 
dEQP-GLES31.functional.shaders.implicit_conversions.es31.invalid.arrays.uint_to_float_fragment
- 
dEQP-GLES31.functional.shaders.implicit_conversions.es32.invalid.arrays.int_to_float_vertex
- 
dEQP-GLES31.functional.shaders.implicit_conversions.es32.invalid.arrays.int_to_float_fragment
- 
dEQP-GLES31.functional.shaders.implicit_conversions.es32.invalid.arrays.int_to_uint_vertex
- 
dEQP-GLES31.functional.shaders.implicit_conversions.es32.invalid.arrays.int_to_uint_fragment
- 
dEQP-GLES31.functional.shaders.implicit_conversions.es32.invalid.arrays.uint_to_float_vertex
- 
dEQP-GLES31.functional.shaders.implicit_conversions.es32.invalid.arrays.uint_to_float_fragment

Signed-off-by: Erik Faye-Lund 
---
 src/compiler/glsl/ast_to_hir.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 7f30a708855..7671041a454 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -891,7 +891,8 @@ validate_assignment(struct _mesa_glsl_parse_state *state,
}
if (unsized_array) {
   if (is_initializer) {
- return rhs;
+ if (rhs->type->get_scalar_type() == lhs->type->get_scalar_type())
+return rhs;
   } else {
  _mesa_glsl_error(, state,
   "implicitly sized arrays cannot be assigned");
-- 
2.17.2

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

[Mesa-dev] [PATCH 1/5] glsl: add has_implicit_conversions()-helper

[Erik Faye-Lund]

This makes the code a bit easier to read, as well as will reduce
repetition when we add support for EXT_shader_implicit_conversions.

Signed-off-by: Erik Faye-Lund 
---
 src/compiler/glsl/ast_to_hir.cpp   | 2 +-
 src/compiler/glsl/glsl_parser_extras.h | 5 +
 src/compiler/glsl_types.cpp| 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 084b7021a9f..6d4b2c6aa5d 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -315,7 +315,7 @@ apply_implicit_conversion(const glsl_type *to, ir_rvalue * 
,
   return true;
 
/* Prior to GLSL 1.20, there are no implicit conversions */
-   if (!state->is_version(120, 0))
+   if (!state->has_implicit_conversions())
   return false;
 
/* From page 27 (page 33 of the PDF) of the GLSL 1.50 spec:
diff --git a/src/compiler/glsl/glsl_parser_extras.h 
b/src/compiler/glsl/glsl_parser_extras.h
index 966d848509c..edd41601c35 100644
--- a/src/compiler/glsl/glsl_parser_extras.h
+++ b/src/compiler/glsl/glsl_parser_extras.h
@@ -344,6 +344,11 @@ struct _mesa_glsl_parse_state {
   return ARB_bindless_texture_enable;
}
 
+   bool has_implicit_conversions() const
+   {
+  return is_version(120, 0);
+   }
+
void process_version_directive(YYLTYPE *locp, int version,
   const char *ident);
 
diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
index 70bce6ace8e..bcc36e50d7f 100644
--- a/src/compiler/glsl_types.cpp
+++ b/src/compiler/glsl_types.cpp
@@ -1425,7 +1425,7 @@ glsl_type::can_implicitly_convert_to(const glsl_type 
*desired,
 * state, we're doing intra-stage function linking where these checks have
 * already been done.
 */
-   if (state && !state->is_version(120, 0))
+   if (state && !state->has_implicit_conversions())
   return false;
 
/* There is no conversion among matrix types. */
-- 
2.17.2

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

[Mesa-dev] [PATCH 0/5] add support for EXT_shader_implicit_conversions

[Erik Faye-Lund]

EXT_shader_implicit_conversions is a useful extension that adds implicit
conversions to OpenGL ES 3.1. Since it's tested excensively in dEQP, and
Mesa already has support for implicit conversions, it seems reasonable to
allow for the extension. This ended up mostly as code-cleanups anyway.

While enabling this, one bug was discorvered due to a failing dEQP test
(see the last patch).

This makes 2068 dEQP-GLE31 tests go from NotSupported to Pass on i965.

No piglit regressions observed.

Erik Faye-Lund (5):
  glsl: add has_implicit_conversions()-helper
  glsl: add has_implicit_uint_to_int_conversion()-helper
  glsl: fall back to inexact function-match
  mesa/glsl: add support for EXT_shader_implicit_conversions
  glsl: do not allow implicit casts of unsized array initializers

 src/compiler/glsl/ast_function.cpp   |  2 +-
 src/compiler/glsl/ast_to_hir.cpp |  8 
 src/compiler/glsl/glsl_parser_extras.cpp |  1 +
 src/compiler/glsl/glsl_parser_extras.h   | 15 +++
 src/compiler/glsl/ir_function.cpp|  3 ++-
 src/compiler/glsl_types.cpp  |  5 ++---
 src/mesa/main/extensions_table.h |  1 +
 7 files changed, 26 insertions(+), 9 deletions(-)

-- 
2.17.2

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

Re: [Mesa-dev] [PATCH] egl_dri2: check if driver_name is NULL before releasing it

[Erik Faye-Lund]

On Tue, 2018-10-30 at 18:28 +0800, Zhaowei YUan wrote:
> I don't think it's fine, usually, freeing an NULL pointer will cause 
> unexpected errors. It's better to check this for the robustness.
> 

From http://pubs.opengroup.org/onlinepubs/009695399/functions/free.html
:

"If ptr is a null pointer, no action shall occur."

So, it should be fine. If your system free does something when passed a
null-pointer, it sounds like there's a problem with your system. This
assumption is made all over the place, not just in Mesa.

> On 10/30/18 3:17 PM, Tapani Pälli wrote:
> > On 10/30/18 8:26 AM, Zhaowei Yuan wrote:
> > > Pointer dri2_dpy->driver_name is probably NULL when calling
> > > dri2_display_destory, check this before releasing it.
> > 
> > It's fine for it to be NULL though and it looks like all the
> > drivers set 
> > it correctly, there is no need for such check. Does this change
> > fix 
> > something for you, what was the motivation for this change?
> > 
> > (same applies for the device_name patch)
> > 
> > > Signed-off-by: Zhaowei Yuan 
> > > ---
> > >   src/egl/drivers/dri2/egl_dri2.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/egl/drivers/dri2/egl_dri2.c 
> > > b/src/egl/drivers/dri2/egl_dri2.c
> > > index c5fa935..54cc334 100644
> > > --- a/src/egl/drivers/dri2/egl_dri2.c
> > > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > > @@ -967,7 +967,8 @@ dri2_display_destroy(_EGLDisplay *disp)
> > > close(dri2_dpy->fd);
> > >  if (dri2_dpy->driver)
> > > dlclose(dri2_dpy->driver);
> > > -   free(dri2_dpy->driver_name);
> > > +   if (dri2_dpy->driver_name)
> > > +  free(dri2_dpy->driver_name);
> > >   #ifdef HAVE_WAYLAND_PLATFORM
> > >  free(dri2_dpy->device_name);
> > > 
> > 
> > ___
> > 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] mesa: expose EXT_texture_compression_s3tc on GLES

[Erik Faye-Lund]

On Fri, 2018-10-26 at 13:14 +0200, Erik Faye-Lund wrote:
> On Thu, 2018-10-25 at 10:45 -0400, Ilia Mirkin wrote:
> > Please confirm that this passes the piglit tests you sent to the
> > list
> > when run with ES2 forced, i.e. not ES3.
> > (MESA_GLES_VERSION_OVERRIDE=2.0 iirc.) I'm concerned that the extra
> > logic was only added to _mesa_es3_error_check_bla and not the es2
> > paths.
> 
> Yeah, the test still passes.
> 
> The _mesa_es3_error_check_format_and_type function seems to be a bit
> unfortunately named; it seems to be used for *all* es versions. I
> don't
> know why it's named like it is.
> 

So, can I perhaps have your r-b? Or is there something else that needs
to be in place for this to move forward?

> But thanks for pointing this out to me; testing this revealed a
> porting-bug, where I tried to use glGetTexLevelParameteriv on gles2,
> which isn't supported. I'll send out and updated version of the
> piglit 
> patches for that also.
> 
> > On Thu, Oct 25, 2018 at 6:59 AM Erik Faye-Lund
> >  wrote:
> > > 
> > > From: Marek Olšák 
> > > 
> > > The spec was modified to support GLES.
> > > 
> > > Tested-by: Erik Faye-Lund 
> > > ---
> > > This replaces this patch:
> > > https://patchwork.freedesktop.org/patch/257423/
> > > 
> > >  docs/relnotes/18.3.0.html|  1 +
> > >  src/mesa/main/extensions_table.h |  2 +-
> > >  src/mesa/main/glformats.c| 11 +++
> > >  3 files changed, 13 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/docs/relnotes/18.3.0.html
> > > b/docs/relnotes/18.3.0.html
> > > index 5874d3fa330..e0061872de4 100644
> > > --- a/docs/relnotes/18.3.0.html
> > > +++ b/docs/relnotes/18.3.0.html
> > > @@ -57,6 +57,7 @@ Note: some of the new features are only
> > > available
> > > with certain drivers.
> > >  GL_AMD_multi_draw_indirect on all GL 4.x drivers.
> > >  GL_AMD_query_buffer_object on i965, nvc0, r600,
> > > radeonsi.
> > >  GL_EXT_disjoint_timer_query on radeonsi and most other
> > > Gallium
> > > drivers (ES extension)
> > > +GL_EXT_texture_compression_s3tc on all drivers (ES
> > > extension)
> > >  GL_EXT_vertex_attrib_64bit on i965, nvc0, radeonsi.
> > >  GL_EXT_window_rectangles on radeonsi.
> > >  GL_KHR_texture_compression_astc_sliced_3d on radeonsi.
> > > diff --git a/src/mesa/main/extensions_table.h
> > > b/src/mesa/main/extensions_table.h
> > > index 09bf923bd0e..47db1583135 100644
> > > --- a/src/mesa/main/extensions_table.h
> > > +++ b/src/mesa/main/extensions_table.h
> > > @@ -278,7 +278,7 @@ EXT(EXT_texture_buffer  ,
> > > OES_texture_buffer
> > >  EXT(EXT_texture_compression_dxt1,
> > > ANGLE_texture_compression_dxt  , GLL, GLC, ES1, ES2,
> > > 2004)
> > >  EXT(EXT_texture_compression_latc,
> > > EXT_texture_compression_latc   , GLL,  x ,  x ,  x ,
> > > 2006)
> > >  EXT(EXT_texture_compression_rgtc,
> > > ARB_texture_compression_rgtc   , GLL, GLC,  x ,  x ,
> > > 2004)
> > > -EXT(EXT_texture_compression_s3tc,
> > > EXT_texture_compression_s3tc   , GLL, GLC,  x ,  x ,
> > > 2000)
> > > +EXT(EXT_texture_compression_s3tc,
> > > EXT_texture_compression_s3tc   , GLL, GLC,  x , ES2,
> > > 2000)
> > >  EXT(EXT_texture_cube_map,
> > > ARB_texture_cube_map   , GLL,  x ,  x ,  x ,
> > > 2001)
> > >  EXT(EXT_texture_cube_map_array  ,
> > > OES_texture_cube_map_array ,  x ,  x ,  x ,  31,
> > > 2014)
> > >  EXT(EXT_texture_edge_clamp  ,
> > > dummy_true , GLL,  x ,  x ,  x ,
> > > 1997)
> > > diff --git a/src/mesa/main/glformats.c
> > > b/src/mesa/main/glformats.c
> > > index 6cb3435dea2..f8fc36e9311 100644
> > > --- a/src/mesa/main/glformats.c
> > > +++ b/src/mesa/main/glformats.c
> > > @@ -2803,6 +2803,17 @@
> > > _mesa_es3_error_check_format_and_type(const
> > > struct gl_context *ctx,
> > >internalFormat = effectiveInternalFormat;
> > > }
> > > 
> > > +   /* The GLES variant of EXT_texture_compression_s3tc is very
> > > vague and
> > > +* doesn't list valid types. Just do exactly what the spec
> > > sa