Re: [Mesa-dev] [PATCH 01/14] mesa: remove incorrect change for EXT_disjoint_timer_query

[Tapani Pälli]

On 08/09/2018 06:55 PM, Marek Olšák wrote:

On Thu, Aug 9, 2018 at 1:47 AM, Tapani Pälli  wrote:

Hi Marek;

On 08/09/2018 02:55 AM, Marek Olšák wrote:


From: Marek Olšák 

---
   src/mesa/main/queryobj.c | 3 +--
   1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
index 7547fa1bb4d..e97a0138e96 100644
--- a/src/mesa/main/queryobj.c
+++ b/src/mesa/main/queryobj.c
@@ -815,22 +815,21 @@ get_query_object(struct gl_context *ctx, const char
*func,
  if (_mesa_is_gles(ctx) &&
  (pname != GL_QUERY_RESULT && pname != GL_QUERY_RESULT_AVAILABLE))
{
 _mesa_error(ctx, GL_INVALID_ENUM, "%s(%s)", func,
 _mesa_enum_to_string(pname));
 return;
  }
if (buf && buf != ctx->Shared->NullBufferObj) {
 bool is_64bit = ptype == GL_INT64_ARB ||
ptype == GL_UNSIGNED_INT64_ARB;
-  if (!ctx->Extensions.ARB_query_buffer_object &&
-  !ctx->Extensions.EXT_disjoint_timer_query) {
+  if (!ctx->Extensions.ARB_query_buffer_object) {
_mesa_error(ctx, GL_INVALID_OPERATION, "%s(not supported)",
func);



Can you explain what was the trouble with this change? I don't recall much
why this particular change was added but the EXT_disjoint_timer_query spec
adds support for int64 and uint64 GL types and params to be used in queries.
I can run some tests to check this.


The conditional is in a block that is entered if a query buffer object
is bound. EXT_disjoint_timer_query doesn't support query buffer
objects. I don't see any connection with 64-bit types there.



Right, you are correct. This block is only for buffer objects, this patch is

Reviewed-by: Tapani Pälli 

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

Re: [Mesa-dev] [PATCH] glsl: handle error case with ast_post_inc, ast_post_dec

[Tapani Pälli]

On 08/09/2018 03:24 PM, Andres Gomez wrote:

Tapani, should we also include this in the stable queues ?


I was not sure if this is 'critical enough' for stable, but yes I'm OK 
including it there!


Thanks;


On Tue, 2018-08-07 at 08:20 +0300, Tapani Pälli wrote:

Return ir_rvalue::error_value with ast_post_inc, ast_post_dec if
parser error was emitted previously. This way process_array_size
won't see bogus IR generated like with commit 9c676a64273.

Signed-off-by: Tapani Pälli 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98699
---
  src/compiler/glsl/ast_to_hir.cpp | 5 +
  1 file changed, 5 insertions(+)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 74160ec142b..5d3f10b6823 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -1928,6 +1928,11 @@ ast_expression::do_hir(exec_list *instructions,
  
error_emitted = op[0]->type->is_error() || op[1]->type->is_error();
  
+  if (error_emitted) {

+ result = ir_rvalue::error_value(ctx);
+ break;
+  }
+
type = arithmetic_result_type(op[0], op[1], false, state, & loc);
  
ir_rvalue *temp_rhs;

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

Re: [Mesa-dev] [PATCH 1/3] gitlab-ci: build Mesa using GitLab CI

[Eric Anholt]

"Juan A. Suarez Romero"  writes:

> Creates different Docker images containing Mesa built with different
> tools (autotools, meson, scons, etc).
>
> The build is done in 3 levels: the first level creates a base image
> with all the requirements to build Mesa.
>
> The second level (based of the first one), builds different images with
> different versions of LLVM. As Gallium drivers heavily relies on LLVM,
> this will help to test the build with different LLVM versions.
>
> Finally, the latest level creates different images versions of Mesa.
> The main differences is the tool to build them: autotools, meson, scons,
> building Gallium drivers with different LLVM versions, and so on.
>
> As the purpose is just to test that everything can be built correctly,
> all the images are discarded, except one (the autotools), which is
> stored in the registry. Thus, anyone can just pull it locally and test
> against their local system.
>
> In order to build the images, Rocker is used. This is a tool that
> extends the Dockerfiles with new features that are quite interested
> here. The main features we use is the support for templating, and the
> support for mounting external directories during the image building.
> This help to use tools like ccache to improve the build speed.
>
> Signed-off-by: Juan A. Suarez Romero 
> ---
>  .gitlab-ci.yml| 177 +
>  gitlab-ci/Rockerfile.base | 199 ++
>  gitlab-ci/Rockerfile.llvm |  57 +++
>  gitlab-ci/Rockerfile.mesa | 145 +++
>  4 files changed, 578 insertions(+)
>  create mode 100644 .gitlab-ci.yml
>  create mode 100644 gitlab-ci/Rockerfile.base
>  create mode 100644 gitlab-ci/Rockerfile.llvm
>  create mode 100644 gitlab-ci/Rockerfile.mesa
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> new file mode 100644
> index 000..5cee333dd45
> --- /dev/null
> +++ b/.gitlab-ci.yml
> @@ -0,0 +1,177 @@
> +image: docker:latest
> +
> +services:
> +  - docker:dind
> +
> +stages:
> +  - base
> +  - llvm
> +  - mesa
> +
> +variables:
> +  DOCKER_IMAGE: $CI_REGISTRY_IMAGE
> +  CCACHE_DIR: $CI_PROJECT_DIR/../ccache
> +  LLVM: "6.0"
> +
> +cache:
> +  paths:
> +- ccache/
> +  key: "$CI_JOB_STAGE"
> +
> +before_script:
> +  - mkdir -p ccache
> +  - rm -fr ../ccache
> +  - mv ccache ../
> +  - export MAKEFLAGS=-j$(nproc)
> +  - apk --no-cache add libc6-compat
> +  - wget 
> https://github.com/grammarly/rocker/releases/download/1.3.1/rocker-1.3.1-linux_amd64.tar.gz
> +  - tar xvf rocker-1.3.1-linux_amd64.tar.gz
> +  - rm rocker-1.3.1-linux_amd64.tar.gz
> +  - mv rocker ..
> +  - docker login -u gitlab-ci-token -p $CI_BUILD_TOKEN $CI_REGISTRY
> +
> +after_script:
> +  - mv ../ccache ./
> +
> +.build_llvm: _llvm
> +  stage: llvm
> +  cache: {}
> +  script:
> +- ../rocker build -f gitlab-ci/Rockerfile.llvm --var LLVM=$LLVM
> +- docker push $CI_REGISTRY_IMAGE:llvm-$LLVM
> +
> +.build_mesa: _mesa
> +  stage: mesa
> +  script:
> +- ../rocker build -f gitlab-ci/Rockerfile.mesa --var BUILD=$BUILD --var 
> LLVM=$LLVM --var TAG=$CI_COMMIT_REF_SLUG .
> +
> +base:
> +  stage: base
> +  script:
> +- DOCKERFILE_SHA256=$(cat gitlab-ci/Rockerfile.base | sha256sum | cut 
> -c-64)
> +- IMAGE_DOCKERFILE_SHA256=$(./gitlab-ci/inspect-remote-image.sh 
> gitlab-ci-token $CI_BUILD_TOKEN $CI_PROJECT_PATH "base" 
> ".config.Labels[\"dockerfile.sha256\"]" || echo -n "")
> +- if [ "$DOCKERFILE_SHA256" != "$IMAGE_DOCKERFILE_SHA256" ] ; then 
> FORCE_BUILD=true ; fi
> +- if [ "$FORCE_BUILD" ] ; then ../rocker build -f 
> gitlab-ci/Rockerfile.base --var DOCKERFILE_SHA256=$DOCKERFILE_SHA256 ; fi
> +- if [ "$FORCE_BUILD" ] ; then docker push $CI_REGISTRY_IMAGE:base ; fi

I think this patch file was a previous version, as patch 2 removes lines
that aren't in this block and replaces them with these ones?

> +llvm:3.3:
> +  variables:
> +LLVM: "3.3"
> +  <<: *build_llvm

How big do all these images end up being?  Do we have any size limits on
what our CI can be uploading?

> diff --git a/gitlab-ci/Rockerfile.base b/gitlab-ci/Rockerfile.base
> new file mode 100644
> index 000..a0cb5e5290d
> --- /dev/null
> +++ b/gitlab-ci/Rockerfile.base
> @@ -0,0 +1,199 @@
> +#
> +# Base image for building Mesa.
> +#
> +# ~~~
> +#  rocker build -f Rockerfile.base [--attach] [--pull]
> +# ~~~
> +#
> +# Environment variables that are used in the build:
> +#  - DOCKER_IMAGE: name of the final image to be tagged (default: mesa:base)
> +#  - MAKEFLAGS: flags to pass to make (e.g., "-j8")
> +#  - CCACHE_DIR: ccache directory (e.g, ~/.ccache)
> +#
> +
> +{{ $image := (or .Env.DOCKER_IMAGE "mesa") }}
> +
> +FROM ubuntu:xenial
> +
> +LABEL maintainer "Juan A. Suarez Romero "
> +
> +ENV LC_ALL=C.UTF-8
> +
> +RUN apt-get update   
>   \
> +  && apt-get --no-install-recommends -y install autoconf gcc g++ sudo cmake 
> patch git  \
> +

Re: [Mesa-dev] [PATCH] intel/isl: Avoid tiling on 16K-wide render targets

[Nanley Chery]

On Thu, Aug 09, 2018 at 04:26:06PM +0300, Andres Gomez wrote:
> Ugh!
> 
> Unfortunately, as I've commented at:
> https://bugs.freedesktop.org/show_bug.cgi?id=107359
> 
> This is now breaking these other CTS tests:
> GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_framebuffers_different_formats
> GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_framebuffers_different_origins
> GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_framebuffers_different_sample_count
> GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_framebuffers_different_sizes
> GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_read_buffer_different_formats
> GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_read_buffer_different_origins
> GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_read_buffer_different_sizes
> GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_functionality_multisampled_to_singlesampled_blit
> 
> 

Sorry about that, I must not have tested this in CI. I'll send out a
better tested v2.

-Nanley

> On Mon, 2018-07-30 at 19:25 +0300, Andres Gomez wrote:
> > That was quick! ☺
> > 
> > On Fri, 2018-07-27 at 16:02 -0700, Nanley Chery wrote:
> > > Fix rendering issues on BDW and SKL.
> > > 
> > > Fixes 0288fe8d0417730bdd5b3477130dd1dc32bdbcd3
> > > ("i965/miptree: Use the correct BLT pitch")
> > 
> > I'd add here some lines listing the tests fixed by this patch.
> > 
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107359
> > > Cc: 
> > 
> > This is:
> > 
> > Tested-by: Andres Gomez 
> > 
> > > ---
> > > 
> > > We could probably add an assert when filling out the surface state, but
> > > I think BLORP would need a non-trivial amount of work done as a
> > > prerequisite. I'm thinking specifically of the cases where we bind a
> > > depth buffer as a render target.
> > > 
> > > I won't be able to push anything until about a week from EOD today, so
> > > if this does end up getting reviewed, please feel free to push it.
> > > 
> > >  src/intel/isl/isl_gen7.c | 19 +++
> > >  1 file changed, 19 insertions(+)
> > > 
> > > diff --git a/src/intel/isl/isl_gen7.c b/src/intel/isl/isl_gen7.c
> > > index 4fa9851233f..2d85f4b568d 100644
> > > --- a/src/intel/isl/isl_gen7.c
> > > +++ b/src/intel/isl/isl_gen7.c
> > > @@ -294,6 +294,25 @@ isl_gen6_filter_tiling(const struct isl_device *dev,
> > >  */
> > > if (ISL_DEV_GEN(dev) < 7 && isl_format_get_layout(info->format)->bpb 
> > > >= 128)
> > >*flags &= ~ISL_TILING_Y0_BIT;
> > > +
> > > +   /* From the BDW and SKL PRMs, Volume 2d,
> > > +* RENDER_SURFACE_STATE::Width - Programming Notes:
> > > +*
> > > +*   A known issue exists if a primitive is rendered to the first 2 
> > > rows and
> > > +*   last 2 columns of a 16K width surface. If any geometry is drawn 
> > > inside
> > > +*   this square it will be copied to column X=2 and X=3 (arrangement 
> > > on Y
> > > +*   position will stay the same). If any geometry exceeds the 
> > > boundaries of
> > > +*   this 2x2 region it will be drawn normally. The issue also only 
> > > occurs
> > > +*   if the surface has TileMode != Linear.
> > > +*
> > > +* To prevent this rendering corruption, only allow linear tiling for
> > > +* surfaces with widths greater than 16K-2 pixels.
> > > +*/
> > > +   if (info->width > 16382 &&
> > > +   info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT &&
> > > +   (ISL_DEV_GEN(dev) == 8 || dev->info->is_skylake)) {
> > > +  *flags &= ISL_TILING_LINEAR_BIT;
> > > +   }
> > >  }
> > >  
> > >  void
> -- 
> Br,
> 
> Andres
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] radv: Fix missing Android platform define.

[Bas Nieuwenhuizen]

CC: 
---
 src/amd/vulkan/Android.mk  | 2 ++
 src/amd/vulkan/Makefile.am | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/amd/vulkan/Android.mk b/src/amd/vulkan/Android.mk
index cee3744f40b..51b03561fa7 100644
--- a/src/amd/vulkan/Android.mk
+++ b/src/amd/vulkan/Android.mk
@@ -62,6 +62,7 @@ LOCAL_SRC_FILES := \
$(VULKAN_FILES)
 
 LOCAL_CFLAGS += -DFORCE_BUILD_AMDGPU   # instructs LLVM to declare 
LLVMInitializeAMDGPU* functions
+LOCAL_CFLAGS += -DVK_USE_PLATFORM_ANDROID_KHR
 
 $(call mesa-build-with-llvm)
 
@@ -140,6 +141,7 @@ LOCAL_SRC_FILES := \
$(VULKAN_ANDROID_FILES)
 
 LOCAL_CFLAGS += -DFORCE_BUILD_AMDGPU   # instructs LLVM to declare 
LLVMInitializeAMDGPU* functions
+LOCAL_CFLAGS += -DVK_USE_PLATFORM_ANDROID_KHR
 
 $(call mesa-build-with-llvm)
 
diff --git a/src/amd/vulkan/Makefile.am b/src/amd/vulkan/Makefile.am
index e7ccc58a28e..e28f032cbee 100644
--- a/src/amd/vulkan/Makefile.am
+++ b/src/amd/vulkan/Makefile.am
@@ -124,7 +124,7 @@ VULKAN_LIB_DEPS += \
 endif
 
 if HAVE_PLATFORM_ANDROID
-AM_CPPFLAGS += $(ANDROID_CPPFLAGS)
+AM_CPPFLAGS += $(ANDROID_CPPFLAGS) -DVK_USE_PLATFORM_ANDROID_KHR
 AM_CFLAGS += $(ANDROID_CFLAGS)
 VULKAN_LIB_DEPS += $(ANDROID_LIBS)
 VULKAN_SOURCES += $(VULKAN_ANDROID_FILES)
-- 
2.18.0

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

Re: [Mesa-dev] [PATCH 3/4] anv: Use central api generation scripts.

[Dylan Baker]

Quoting Chad Versace (2018-08-09 10:37:33)
> On Tue 07 Aug 2018, Dylan Baker wrote:
> > Quoting Bas Nieuwenhuizen (2018-08-07 16:14:33) 
> > 
> > 
> >   
> > >   
> > >   
> > >   
> > >   
> > >  anv_extensions_c = custom_target(
> > >   
> > >   
> > >   
> > > @@ -36,10 +37,11 @@ anv_extensions_c = custom_target( 
> > >   
> > >   
> > >   
> > >input : ['anv_extensions_gen.py', vk_api_xml], 
> > >   
> > >   
> > >   
> > >output : 'anv_extensions.c',   
> > >   
> > >   
> > >   
> > >command : [
> > >   
> > >   
> > >   
> > > +   'env', 'PYTHONPATH=@0@'.format(join_paths(meson.source_root(), 
> > > 'src/vulkan/util/')), 
> > >   
> > >  
> > 
> > This is really gross, you're adding a dependency on a unix console command. 
> > I   
> > 
> >   
> > know that anv is only built on Unix-like oses, but this will eventually end 
> > up  
> > 
> >   
> > being used in some code that needs to run on Windows (or mac, does mac have 
> > 
> > 
> >   
> > env?).  
> > 
> > 
> >   
> > 
> > I know that some people will object, but IMHO a better solution than 
> > mucking 
> > 
> >  
> > with the python path (either through sys.path or through PYTHONPATH, is to  
> > 
> > 
> >   
> > put all of the generators in a src/generators directory and be done with 
> > it. 
> > 
> >  
> > Sure the intel specific bits (for example) aren't in the src/intel folder, 
> > that's a small price to avoid having to call env just to run a python 
> > script.
> 
> Dylan, I think we should avoid introducing complexity in the build
> system for the benefit of operating systems not supported by the driver.
> That feels like a serious premature optimazation, to me.  Anvil's usage
> of 

Re: [Mesa-dev] [PATCH v2 7/9] python: Simplify list sorting

[Dylan Baker]

All patches up to here (including this one) have been pushed to master. I had
comments on patch 8, and I want to follow up with our CI team to make sure we
have all the dependencies for python 3 in our CI.

Dylan

Quoting Mathieu Bridon (2018-08-09 01:27:24)
> Instead of copying the list, then sorting the copy in-place, we can just
> get a new sorted copy directly.
> 
> Signed-off-by: Mathieu Bridon 
> ---
>  src/mapi/mapi_abi.py | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mapi/mapi_abi.py b/src/mapi/mapi_abi.py
> index d4c48ec430..dc48fa5935 100644
> --- a/src/mapi/mapi_abi.py
> +++ b/src/mapi/mapi_abi.py
> @@ -291,8 +291,7 @@ class ABIPrinter(object):
>  self.entries = entries
>  
>  # sort entries by their names
> -self.entries_sorted_by_names = self.entries[:]
> -self.entries_sorted_by_names.sort(key=attrgetter('name'))
> +self.entries_sorted_by_names = sorted(self.entries, 
> key=attrgetter('name'))
>  
>  self.indent = ' ' * 3
>  self.noop_warn = 'noop_warn'
> @@ -441,8 +440,7 @@ class ABIPrinter(object):
>  def c_stub_string_pool(self):
>  """Return the string pool for use by stubs."""
>  # sort entries by their names
> -sorted_entries = self.entries[:]
> -sorted_entries.sort(key=attrgetter('name'))
> +sorted_entries = sorted(self.entries, key=attrgetter('name'))
>  
>  pool = []
>  offsets = {}
> -- 
> 2.17.1
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

Re: [Mesa-dev] [PATCH 3/4] anv: Use central api generation scripts.

[Bas Nieuwenhuizen]

On Thu, Aug 9, 2018 at 7:48 PM, Chad Versace  wrote:
> On Wed 08 Aug 2018, Bas Nieuwenhuizen wrote:
>> This became kind of messy as python imports cannot really look up
>> parent/sibling directories. I saw some scripts use sys.path but
>> that became even more messy due to import locations.
>>
>> I also move the selections of the dispatch table out of the
>> generation script because it is not easily shared, and generating
>> it did not really win anything anyway.
>> ---
>>  src/intel/Android.vulkan.mk |   9 +
>>  src/intel/Makefile.vulkan.am|  25 +-
>>  src/intel/vulkan/anv_device.c   |  46 ++
>>  src/intel/vulkan/anv_entrypoints_gen.py | 537 +---
>>  src/intel/vulkan/anv_extensions.py  |  68 +--
>>  src/intel/vulkan/anv_extensions_gen.py  | 177 +---
>>  src/intel/vulkan/meson.build|  15 +-
>>  src/vulkan/util/vk_extensions.py|   5 +-
>>  8 files changed, 98 insertions(+), 784 deletions(-)
>>
>> diff --git a/src/intel/Android.vulkan.mk b/src/intel/Android.vulkan.mk
>
>
>>  $(intermediates)/vulkan/anv_entrypoints.h: $(intermediates)/vulkan/dummy.c
>> + PYTHONPATH=$(MESA_TOP)/src/vulkan/util \
>>   $(VK_ENTRYPOINTS_SCRIPT) \
>>   --outdir $(dir $@) \
>>   --xml $(MESA_TOP)/src/vulkan/registry/vk.xml
>
> Yes, modifying PYTHONPATH is messy, but it seems to me that it's the the
> least-messy way.
>
> I'm no expert on build systems, but I think it's wrong to clobber
> PYTHONPATH. Instead, you should prepend the dir to PYTHONPATH.
>
> For example, on my machine, PYTHONPATH is already set:
>
> $ echo $PYTHONPATH
> /usr/local/buildtools/current/sitecustomize
>
> So, this is more correct...
>
>$(intermediates)/vulkan/anv_entrypoints.h: $(intermediates)/vulkan/dummy.c
> PYTHONPATH="$(MESA_TOP)/src/vulkan/util:$${PYTHONPATH}"
> $(VK_ENTRYPOINTS_SCRIPT) \
> --outdir $(dir $@) \
> --xml $(MESA_TOP)/src/vulkan/registry/vk.xml
>
> but runs the risk of accidentally inserting $PWD into PYTHONPATH, because
> Python interprets each empty path in PYTHONPATH as equivalent to $PWD.
>
> So... perhaps it is better to modify sys.path instead of PYTHONPATH. It is
> definitely safer.

I'm also not sure I can modify the PYTHONPATH in meson.

The main problem with sys.path is that the {radv,anv}_extensions use
quite a bit of helpers in vk_extensions.py, and the generators use
quite a bit of {radv,anv}_extensions. However, with sys.path we change
the path in the main function after changing the arguments (note a
relative path is relative from the working directory, not the source
directory). This means we have to use late imports too and likely not
a global scope, which makes things messy. I'll look a bit more into
it.

Do people prefer the shared directory or using sys.path?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/4] vulkan: Add central copy of entrypoints/extensions code.

[Bas Nieuwenhuizen]

On Thu, Aug 9, 2018 at 7:49 PM, Chad Versace  wrote:
> On Wed 08 Aug 2018, Bas Nieuwenhuizen wrote:
>> ---
>>  src/vulkan/Makefile.am|   3 +
>>  src/vulkan/util/meson.build   |   2 +
>>  src/vulkan/util/vk_entrypoints_gen.py | 515 ++
>>  src/vulkan/util/vk_extensions.py  |  92 +
>>  src/vulkan/util/vk_extensions_gen.py  | 205 ++
>>  5 files changed, 817 insertions(+)
>>  create mode 100644 src/vulkan/util/vk_entrypoints_gen.py
>>  create mode 100644 src/vulkan/util/vk_extensions.py
>>  create mode 100644 src/vulkan/util/vk_extensions_gen.py
>
>
>> +class VkVersion:
>> +def __init__(self, string):
>> +split = string.split('.')
>> +self.major = int(split[0])
>> +self.minor = int(split[1])
>> +if len(split) > 2:
>> +assert len(split) == 3
>> +self.patch = int(split[2])
>> +else:
>> +self.patch = None
>> +
>> +# Sanity check.  The range bits are required by the definition of 
>> the
>> +# VK_MAKE_VERSION macro
>> +assert self.major < 1024 and self.minor < 1024
>> +assert self.patch is None or self.patch < 4096
>> +assert(str(self) == string)
>> +
>> +def __str__(self):
>> +ver_list = [str(self.major), str(self.minor)]
>> +if self.patch is not None:
>> +ver_list.append(str(self.patch))
>> +return '.'.join(ver_list)
>> +
>> +def c_vk_version(self):
>> +patch = self.patch if self.patch is not None else 0
>> +ver_list = [str(self.major), str(self.minor), str(patch)]
>> +return 'VK_MAKE_VERSION(' + ', '.join(ver_list) + ')'
>> +
>> +def __int_ver(self):
>> +# This is just an expansion of VK_VERSION
>> +patch = self.patch if self.patch is not None else 0
>> +return (self.major << 22) | (self.minor << 12) | patch
>> +
>> +def __cmp__(self, other):
>
> Patch 3 replaces __cmp__ here with __gt__. Was that on purpose?

No, a rebase error, the __cmp__ here effectively reverts an upstream
patch in the copied file, but my fixup ended up in the wrong patch.

>
>> +# If only one of them has a patch version, "ignore" it by making
>> +# other's patch version match self.
>> +if (self.patch is None) != (other.patch is None):
>> +other = copy.copy(other)
>> +other.patch = self.patch
>> +
>> +return self.__int_ver().__cmp__(other.__int_ver())
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v3 2/4] gm107/ir: add support for OP_XMAD on GM107+

[Karol Herbst]

On Mon, Jul 23, 2018 at 12:40 PM, Rhys Perry  wrote:
> Signed-off-by: Rhys Perry 
> ---
>  .../drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp | 65 
> ++
>  .../nouveau/codegen/nv50_ir_target_gm107.cpp   |  6 +-
>  .../nouveau/codegen/nv50_ir_target_nvc0.cpp|  1 +
>  3 files changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp 
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> index 1d31f181e4..c3d7be0f0e 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> @@ -156,6 +156,7 @@ private:
> void emitIMUL();
> void emitIMAD();
> void emitISCADD();
> +   void emitXMAD();
> void emitIMNMX();
> void emitICMP();
> void emitISET();
> @@ -1892,6 +1893,67 @@ CodeEmitterGM107::emitISCADD()
> emitGPR (0x00, insn->def(0));
>  }
>
> +void
> +CodeEmitterGM107::emitXMAD()
> +{
> +   assert(insn->src(0).getFile() == FILE_GPR);
> +
> +   bool constbuf = false;
> +   bool psl_mrg = true;
> +   bool immediate = false;
> +   if (insn->src(2).getFile() == FILE_MEMORY_CONST) {
> +  assert(insn->src(1).getFile() == FILE_GPR);
> +  constbuf = true;
> +  psl_mrg = false;
> +  emitInsn(0x5100);
> +  emitGPR(0x27, insn->src(1));
> +  emitCBUF(0x22, -1, 0x14, 16, 2, insn->src(2));
> +   } else if (insn->src(1).getFile() == FILE_MEMORY_CONST) {
> +  assert(insn->src(2).getFile() == FILE_GPR);
> +  constbuf = true;
> +  emitInsn(0x4e00);
> +  emitCBUF(0x22, -1, 0x14, 16, 2, insn->src(1));
> +  emitGPR(0x27, insn->src(2));
> +   } else if (insn->src(1).getFile() == FILE_IMMEDIATE) {
> +  assert(insn->src(2).getFile() == FILE_GPR);
> +  assert(!(insn->subOp & NV50_IR_SUBOP_XMAD_H1(1)));
> +  immediate = false;

has to be immediate = true;

> +  emitInsn(0x3600);
> +  emitIMMD(0x14, 19, insn->src(1));

we can only do 16 bit sized immediates with XMAD I think. I think we
also have to adjust the target so that those don't get load
propagated? How does this works out for mul/mad -> XMAD conversions
anyway? We might want to recheck that we actually do the right thing
there actually (or maybe it doesn't come up, still, would be nice to
fix it inside the target in case it is actually buggy).

> +  emitGPR(0x27, insn->src(2));
> +   } else {
> +  assert(insn->src(1).getFile() == FILE_GPR);
> +  assert(insn->src(2).getFile() == FILE_GPR);
> +  emitInsn(0x5b00);
> +  emitGPR(0x14, insn->src(1));
> +  emitGPR(0x27, insn->src(2));
> +   }
> +
> +   if (psl_mrg)
> +  emitField(constbuf ? 0x37 : 0x24, 2, insn->subOp & 0x3);
> +
> +   unsigned cmode = (insn->subOp & NV50_IR_SUBOP_XMAD_CMODE_MASK);
> +   cmode >>= NV50_IR_SUBOP_XMAD_CMODE_SHIFT;
> +   emitField(0x32, constbuf ? 2 : 3, cmode);
> +
> +   emitX(constbuf ? 0x36 : 0x26);
> +   emitCC(0x2f);
> +
> +   emitGPR(0x0, insn->def(0));
> +   emitGPR(0x8, insn->src(0));
> +
> +   // source flags
> +   if (isSignedType(insn->sType)) {
> +  uint16_t h1s = insn->subOp & NV50_IR_SUBOP_XMAD_H1_MASK;
> +  emitField(0x30, 2, h1s >> NV50_IR_SUBOP_XMAD_H1_SHIFT);
> +   }
> +   emitField(0x35, 1, insn->subOp & NV50_IR_SUBOP_XMAD_H1(0) ? 1 : 0);
> +   if (!immediate) {
> +  bool h1 = insn->subOp & NV50_IR_SUBOP_XMAD_H1(1);
> +  emitField(constbuf ? 0x34 : 0x23, 1, h1);
> +   }
> +}
> +
>  void
>  CodeEmitterGM107::emitIMNMX()
>  {
> @@ -3266,6 +3328,9 @@ CodeEmitterGM107::emitInstruction(Instruction *i)
> case OP_SHLADD:
>emitISCADD();
>break;
> +   case OP_XMAD:
> +  emitXMAD();
> +  break;
> case OP_MIN:
> case OP_MAX:
>if (isFloatType(insn->dType)) {
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_gm107.cpp 
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_gm107.cpp
> index 7293fb27dd..bb1c234c43 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_gm107.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_gm107.cpp
> @@ -60,8 +60,11 @@ TargetGM107::isOpSupported(operation op, DataType ty) const
> case OP_SQRT:
> case OP_DIV:
> case OP_MOD:
> -   case OP_XMAD:
>return false;
> +   case OP_XMAD:
> +  if (isFloatType(ty))
> + return false;
> +  break;
> default:
>break;
> }
> @@ -231,6 +234,7 @@ TargetGM107::getLatency(const Instruction *insn) const
> case OP_SUB:
> case OP_VOTE:
> case OP_XOR:
> +   case OP_XMAD:
>if (insn->dType != TYPE_F64)
>   return 6;
>break;
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.cpp 
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.cpp
> index 7e66d2950b..5257f353e4 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nvc0.cpp
> @@ -161,6 

Re: [Mesa-dev] [PATCH v2 8/9] python: Rework bytes/unicode string handling

[Dylan Baker]

This doesn't work with python 2. It's also pointed out to me that we don't
handle translations in meson at all...

Here's the traceback:
make[5]: Entering directory 
'/home/jenkins/workspace/Leeroy_3/repos/mesa/build_m64/src/util/xmlpool'
Updating (ca) ca/LC_MESSAGES/options.mo from 
/home/jenkins/workspace/Leeroy_3/repos/mesa/src/util/xmlpool/ca.po.
Updating (de) de/LC_MESSAGES/options.mo from 
/home/jenkins/workspace/Leeroy_3/repos/mesa/src/util/xmlpool/de.po.
Updating (es) es/LC_MESSAGES/options.mo from 
/home/jenkins/workspace/Leeroy_3/repos/mesa/src/util/xmlpool/es.po.
Updating (nl) nl/LC_MESSAGES/options.mo from 
/home/jenkins/workspace/Leeroy_3/repos/mesa/src/util/xmlpool/nl.po.
Updating (fr) fr/LC_MESSAGES/options.mo from 
/home/jenkins/workspace/Leeroy_3/repos/mesa/src/util/xmlpool/fr.po.
Updating (sv) sv/LC_MESSAGES/options.mo from 
/home/jenkins/workspace/Leeroy_3/repos/mesa/src/util/xmlpool/sv.po.
  GEN  options.h
Traceback (most recent call last):
  File 
"/home/jenkins/workspace/Leeroy_3/repos/mesa/src/util/xmlpool/gen_xmlpool.py", 
line 210, in 
expandMatches ([matchDESC], translations)
  File 
"/home/jenkins/workspace/Leeroy_3/repos/mesa/src/util/xmlpool/gen_xmlpool.py", 
line 133, in expandMatches
text = (matches[0].expand (r'\1' + lang + r'\3"' + text + r'"\7') + suffix)
  File "/usr/lib/python2.7/re.py", line 282, in _expand
return sre_parse.expand_template(template, match)
  File "/usr/lib/python2.7/sre_parse.py", line 862, in expand_template
return sep.join(literals)
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 9: ordinal 
not in range(128)
Makefile:720: recipe for target 'options.h' failed

I'm running everything up through this through our CI again, but assuming
everything still looks good I'll be merging everything but this patch and the
next patch today.

For every patch up to this point:
Reviewed-by: Dylan Baker 

Quoting Mathieu Bridon (2018-08-09 01:27:25)
> In both Python 2 and 3, opening a file without specifying the mode will
> open it for reading in text mode ('r').
> 
> On Python 2, the read() method of a file object opened in mode 'r' will
> return byte strings, while on Python 3 it will return unicode strings.
> 
> Explicitly specifying the binary mode ('rb') then decoding the byte
> string means we always handle unicode strings on both Python 2 and 3.
> 
> Which in turns means all re.match(line) will return unicode strings as
> well.
> 
> If we also make expandCString return unicode strings, we don't need the
> call to the unicode() constructor any more.
> 
> We were using the ugettext() method because it always returns unicode
> strings in Python 2, contrarily to the gettext() one which returns
> strings in the same type as its input. The ugettext() method doesn't
> exist on Python 3, so we must use the gettext() one.
> 
> This is fine now that we know we only pass unicode strings to gettext().
> (the return values of expandCString)
> 
> The last hurdles are that Python 3 doesn't let us concatenate unicode
> and byte strings directly, and that Python 2's stdout wants encoded byte
> strings while Python 3's want unicode strings.
> 
> With these changes, the script gives the same output on both Python 2
> and 3.
> 
> Signed-off-by: Mathieu Bridon 
> ---
>  src/util/xmlpool/gen_xmlpool.py | 35 +++--
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/src/util/xmlpool/gen_xmlpool.py b/src/util/xmlpool/gen_xmlpool.py
> index b0db183854..db20e2767f 100644
> --- a/src/util/xmlpool/gen_xmlpool.py
> +++ b/src/util/xmlpool/gen_xmlpool.py
> @@ -60,7 +60,7 @@ def expandCString (s):
>  octa = False
>  num = 0
>  digits = 0
> -r = ''
> +r = u''
>  while i < len(s):
>  if not escape:
>  if s[i] == '\\':
> @@ -128,16 +128,29 @@ def expandMatches (matches, translations, end=None):
>  if len(matches) == 1 and i < len(translations) and \
> not matches[0].expand (r'\7').endswith('\\'):
>  suffix = ' \\'
> -# Expand the description line. Need to use ugettext in order to allow
> -# non-ascii unicode chars in the original English descriptions.
> -text = escapeCString (trans.ugettext (unicode (expandCString (
> -matches[0].expand (r'\5')), "utf-8"))).encode("utf-8")
> -print(matches[0].expand (r'\1' + lang + r'\3"' + text + r'"\7') + 
> suffix)
> +text = escapeCString (trans.gettext (expandCString (
> +matches[0].expand (r'\5'
> +text = (matches[0].expand (r'\1' + lang + r'\3"' + text + r'"\7') + 
> suffix)
> +
> +# In Python 2, stdout expects encoded byte strings, or else it will
> +# encode them with the ascii 'codec'
> +if sys.version_info.major == 2:
> +text = text.encode('utf-8')
> +
> +print(text)
> +
>  # Expand any subsequent enum lines
>  for match in matches[1:]:
> -

Re: [Mesa-dev] [PATCH] st/mesa: Don't set atomic counter size != 0 with atomic buffers == 0.

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Wed, Aug 1, 2018 at 7:07 PM, Eric Anholt  wrote:
> This is just asking for tests to get confused about the HW supporting
> atomics in this shader stage or not, such as
> dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_expression_vertex.
> ---
>  src/mesa/state_tracker/st_extensions.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/state_tracker/st_extensions.c 
> b/src/mesa/state_tracker/st_extensions.c
> index dbaf7f6f8fe5..cd17f026fed2 100644
> --- a/src/mesa/state_tracker/st_extensions.c
> +++ b/src/mesa/state_tracker/st_extensions.c
> @@ -243,9 +243,10 @@ void st_init_limits(struct pipe_screen *screen,
>   pc->MaxAtomicBuffers = screen->get_shader_param(screen, sh, 
> PIPE_SHADER_CAP_MAX_HW_ATOMIC_COUNTER_BUFFERS);
>   pc->MaxShaderStorageBlocks = screen->get_shader_param(screen, sh, 
> PIPE_SHADER_CAP_MAX_SHADER_BUFFERS);
>} else {
> - pc->MaxAtomicCounters = MAX_ATOMIC_COUNTERS;
>   pc->MaxAtomicBuffers = screen->get_shader_param(screen, sh, 
> PIPE_SHADER_CAP_MAX_SHADER_BUFFERS) / 2;
>   pc->MaxShaderStorageBlocks = pc->MaxAtomicBuffers;
> + if (pc->MaxAtomicBuffers)
> +pc->MaxAtomicCounters = MAX_ATOMIC_COUNTERS;
>}
>pc->MaxImageUniforms = screen->get_shader_param(
>  screen, sh, PIPE_SHADER_CAP_MAX_SHADER_IMAGES);
> --
> 2.18.0
>
> ___
> 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] [RFC][PATCH 4/5] mesa: implement glGet for AMD_depth_clamp_separate

[Marek Olšák]

Note that the spec says:
"Querying DEPTH_CLAMP will return TRUE if DEPTH_CLAMP_NEAR_AMD _or_
DEPTH_CLAMP_FAR_AMD is enabled."

Marek

On Wed, Aug 1, 2018 at 11:31 PM, Sagar Ghuge  wrote:
> Signed-off-by: Sagar Ghuge 
> ---
>  src/mesa/main/get.c  | 1 +
>  src/mesa/main/get_hash_params.py | 5 +
>  2 files changed, 6 insertions(+)
>
> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
> index db0079beb5..b310f014b7 100644
> --- a/src/mesa/main/get.c
> +++ b/src/mesa/main/get.c
> @@ -468,6 +468,7 @@ EXTRA_EXT(NV_texture_rectangle);
>  EXTRA_EXT(EXT_stencil_two_side);
>  EXTRA_EXT(EXT_depth_bounds_test);
>  EXTRA_EXT(ARB_depth_clamp);
> +EXTRA_EXT(AMD_depth_clamp_separate);
>  EXTRA_EXT(ATI_fragment_shader);
>  EXTRA_EXT(EXT_provoking_vertex);
>  EXTRA_EXT(ARB_fragment_shader);
> diff --git a/src/mesa/main/get_hash_params.py 
> b/src/mesa/main/get_hash_params.py
> index 618e306e50..edf3c982d0 100644
> --- a/src/mesa/main/get_hash_params.py
> +++ b/src/mesa/main/get_hash_params.py
> @@ -982,6 +982,11 @@ descriptor=[
>
>  # GL_ARB_indirect_parameters
>[ "PARAMETER_BUFFER_BINDING_ARB", "LOC_CUSTOM, TYPE_INT, 0, 
> extra_ARB_indirect_parameters" ],
> +
> +# GL 4.1
> +# GL_AMD_depth_clamp_separate
> +  [ "DEPTH_CLAMP_NEAR_AMD", "CONTEXT_BOOL(Transform.DepthClampNear), 
> extra_AMD_depth_clamp_separate" ],
> +  [ "DEPTH_CLAMP_FAR_AMD", "CONTEXT_BOOL(Transform.DepthClampFar), 
> extra_AMD_depth_clamp_separate" ],
>  ]},
>
>  ]
> --
> 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] [RFC][PATCH 3/5] mesa: Add support for AMD_depth_clamp_separate

[Sagar Ghuge]

On 08/09/2018 01:09 PM, Marek Olšák wrote:
> On Wed, Aug 1, 2018 at 11:31 PM, Sagar Ghuge  wrote:
>> enable _mesa_PushAttrib() and _mesa_PopAttrib()
>> to handle GL_DEPTH_CLAMP_NEAR_AMD and
>> GL_DEPTH_CLAMP_FAR_AMD tokens.
>>
>> Signed-off-by: Sagar Ghuge 
>> ---
>>  src/mesa/main/attrib.c | 16 
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
>> index cbe93ab6fa..d9f165b428 100644
>> --- a/src/mesa/main/attrib.c
>> +++ b/src/mesa/main/attrib.c
>> @@ -73,6 +73,8 @@ struct gl_enable_attrib
>> GLboolean ColorMaterial;
>> GLboolean CullFace;
>> GLboolean DepthClamp;
>> +   GLboolean DepthClampNear;
>> +   GLboolean DepthClampFar;
> 
> The first patch uses this. Also, DepthClamp can be removed, because
> DepthClampNear+Far replace it, right?

Yes, that's true. 

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

Re: [Mesa-dev] [RFC][PATCH 0/5] mesa: Add types for AMD_depth_clamp_separate.

[Marek Olšák]

On Thu, Aug 2, 2018 at 2:44 PM, Ian Romanick  wrote:
> On 08/02/2018 11:30 AM, Ian Romanick wrote:
>> On 08/01/2018 08:31 PM, Sagar Ghuge wrote:
>>> Add some basic types and storage for the
>>> AMD_depth_clamp_separate extension.
>
> I mentioned this on patch 5, but you should word wrap the commit message
> to 70 or 72 columns.
>
> More substantive comments are below...
>
>>> Signed-off-by: Sagar Ghuge 
>>> ---
>>>  include/GL/glcorearb.h   | 2 ++
>>>  src/mesa/main/extensions_table.h | 1 +
>>>  src/mesa/main/mtypes.h   | 9 +
>>>  3 files changed, 12 insertions(+)
>>>
>>> diff --git a/include/GL/glcorearb.h b/include/GL/glcorearb.h
>>> index a78bbb6e18..d73ca5a8df 100644
>>> --- a/include/GL/glcorearb.h
>>> +++ b/include/GL/glcorearb.h
>>> @@ -1558,6 +1558,8 @@ typedef int64_t GLint64;
>>>  #define GL_MAX_FRAGMENT_INPUT_COMPONENTS  0x9125
>>>  #define GL_CONTEXT_PROFILE_MASK   0x9126
>>>  #define GL_DEPTH_CLAMP0x864F
>>> +#define GL_DEPTH_CLAMP_NEAR_AMD   0x901E
>>> +#define GL_DEPTH_CLAMP_FAR_AMD0x901F
>>>  #define GL_QUADS_FOLLOW_PROVOKING_VERTEX_CONVENTION 0x8E4C
>>>  #define GL_FIRST_VERTEX_CONVENTION0x8E4D
>>>  #define GL_LAST_VERTEX_CONVENTION 0x8E4E
>>
>> We should just import the updated versions of the Khronos headers.  I
>> think Marek sent out a patch to do this.  Does that work?
>>
>>> diff --git a/src/mesa/main/extensions_table.h 
>>> b/src/mesa/main/extensions_table.h
>>> index 3f01896cae..8dc668e087 100644
>>> --- a/src/mesa/main/extensions_table.h
>>> +++ b/src/mesa/main/extensions_table.h
>>> @@ -9,6 +9,7 @@
>>>  EXT(3DFX_texture_compression_FXT1   , 
>>> TDFX_texture_compression_FXT1  , GLL, GLC,  x ,  x , 1999)
>>>
>>>  EXT(AMD_conservative_depth  , ARB_conservative_depth   
>>>   , GLL, GLC,  x ,  x , 2009)
>>> +EXT(AMD_depth_clamp_separate, AMD_depth_clamp_separate 
>>>   ,  x , GLC,  x ,  x , 2009)
>>>  EXT(AMD_draw_buffers_blend  , ARB_draw_buffers_blend   
>>>   , GLL, GLC,  x ,  x , 2009)
>>>  EXT(AMD_performance_monitor , AMD_performance_monitor  
>>>   , GLL, GLC,  x , ES2, 2007)
>>>  EXT(AMD_pinned_memory   , AMD_pinned_memory
>>>   , GLL, GLC,  x ,  x , 2013)
>>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>>> index d71872835d..406746a84c 100644
>>> --- a/src/mesa/main/mtypes.h
>>> +++ b/src/mesa/main/mtypes.h
>>> @@ -1280,6 +1280,8 @@ struct gl_transform_attrib
>>> GLboolean RescaleNormals;/**< 
>>> GL_EXT_rescale_normal */
>>> GLboolean RasterPositionUnclipped;   /**< GL_IBM_rasterpos_clip 
>>> */
>>> GLboolean DepthClamp;/**< GL_ARB_depth_clamp */
>>> +   GLboolean DepthClampNear;/**< 
>>> GL_AMD_depth_clamp_separate */
>>> +   GLboolean DepthClampFar; /**< 
>>> GL_AMD_depth_clamp_separate */
>
> I think we actually need two more flags here: _DepthClampNear and
> _DepthClampFar.  The spec is a little unclear, so you may need to test
> on some AMD closed-source drivers.  Specifically, the spec says
>
> "In addition to DEPTH_CLAMP_NEAR_AMD and DEPTH_CLAMP_FAR_AMD, the
> token DEPTH_CLAMP may be used to simultaneously enable or disable
> depth clamping at both the near and far planes."
>
> Based on that, I'm not sure what you're supposed to get if you do:
>
> glDisable(GL_DEPTH_CLAMP_NEAR_AMD);
> glEnable(GL_DEPTH_CLAMP);
> glGetIntegerv(GL_DEPTH_CLAMP_NEAR_AMD, );
>
> Should v contain GL_TRUE or GL_FALSE?  It seems clear that rendering
> should have the near plane clamped.
>
> Depending on the results of testing on AMD drivers, we either need
> enable / disable of GL_DEPTH_CLAMP to set / reset
> gl_transform_attrib::DepthClampNear and
> gl_transform_attrib::DepthClampFar or we need to add _DepthClampNear and
> _DepthClampFar that get modified.  In the latter case, the driver would
> only ever look at _DepthClampNear and _DepthClampFar.
>
>>> /** GL_ARB_clip_control */
>>> GLenum16 ClipOrigin;   /**< GL_LOWER_LEFT or GL_UPPER_LEFT */
>>> GLenum16 ClipDepthMode;/**< GL_NEGATIVE_ONE_TO_ONE or GL_ZERO_TO_ONE */
>>> @@ -4235,6 +4237,7 @@ struct gl_extensions
>>> GLboolean OES_texture_view;
>>> GLboolean OES_viewport_array;
>>> /* vendor extensions */
>>> +   GLboolean AMD_depth_clamp_separate;
>>> GLboolean AMD_performance_monitor;
>>> GLboolean AMD_pinned_memory;
>>> GLboolean AMD_seamless_cubemap_per_texture;
>>> @@ -4577,6 +4580,12 @@ struct gl_driver_flags
>>> /** gl_context::Transform::DepthClamp */
>>> uint64_t NewDepthClamp;
>>>
>>> +   /** gl_context::Transform::DepthClampNear */
>>> +   uint64_t NewDepthClampNear;
>>> +
>>> +   /** gl_context::Transform::DepthClampFar */
>>> +   uint64_t NewDepthClampFar;
>>> +
>
> I don't think we 

Re: [Mesa-dev] [RFC][PATCH 3/5] mesa: Add support for AMD_depth_clamp_separate

[Marek Olšák]

On Wed, Aug 1, 2018 at 11:31 PM, Sagar Ghuge  wrote:
> enable _mesa_PushAttrib() and _mesa_PopAttrib()
> to handle GL_DEPTH_CLAMP_NEAR_AMD and
> GL_DEPTH_CLAMP_FAR_AMD tokens.
>
> Signed-off-by: Sagar Ghuge 
> ---
>  src/mesa/main/attrib.c | 16 
>  1 file changed, 16 insertions(+)
>
> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
> index cbe93ab6fa..d9f165b428 100644
> --- a/src/mesa/main/attrib.c
> +++ b/src/mesa/main/attrib.c
> @@ -73,6 +73,8 @@ struct gl_enable_attrib
> GLboolean ColorMaterial;
> GLboolean CullFace;
> GLboolean DepthClamp;
> +   GLboolean DepthClampNear;
> +   GLboolean DepthClampFar;

The first patch uses this. Also, DepthClamp can be removed, because
DepthClampNear+Far replace it, right?

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

Re: [Mesa-dev] [PATCH] intel: Switch the order of the 2x MSAA sample positions

[Anuj Phogat]

On Wed, Aug 8, 2018 at 11:31 AM Jason Ekstrand  wrote:
>
> The Vulkan 1.1.82 spec flipped the order to better match D3D.
>
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/intel/blorp/blorp_blit.c   | 11 ++-
>  src/intel/common/gen_sample_positions.h|  8 
>  src/mesa/drivers/dri/i965/brw_multisample_state.h  |  8 
>  src/mesa/drivers/dri/i965/gen6_multisample_state.c |  4 ++--
>  4 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/src/intel/blorp/blorp_blit.c b/src/intel/blorp/blorp_blit.c
> index 561897894c3..013f7a14fa2 100644
> --- a/src/intel/blorp/blorp_blit.c
> +++ b/src/intel/blorp/blorp_blit.c
> @@ -776,6 +776,13 @@ blorp_nir_manual_blend_bilinear(nir_builder *b, 
> nir_ssa_def *pos,
> * grid of samples with in a pixel. Sample number layout shows the
> * rectangular grid of samples roughly corresponding to the real sample
> * locations with in a pixel.
> +   *
> +   * In the case of 2x MSAA, the layout of sample indices is reversed 
> from
> +   * the layout of sample numbers:
It is not clear from this comment if below layout is for sample index or sample
number. Adding "sample number layout:" on top of it will help.
> +   *   -
> +   *   | 1 | 0 |
> +   *   -
> +   *
> * In case of 4x MSAA, layout of sample indices matches the layout of
> * sample numbers:
> *   -
> @@ -819,7 +826,9 @@ blorp_nir_manual_blend_bilinear(nir_builder *b, 
> nir_ssa_def *pos,
>  key->x_scale * key->y_scale));
>sample = nir_f2i32(b, sample);
>
> -  if (tex_samples == 8) {
> +  if (tex_samples == 2) {
> + sample = nir_isub(b, nir_imm_int(b, 1), sample);
> +  } else if (tex_samples == 8) {
>   sample = nir_iand(b, nir_ishr(b, nir_imm_int(b, 0x64210573),
> nir_ishl(b, sample, nir_imm_int(b, 
> 2))),
> nir_imm_int(b, 0xf));
> diff --git a/src/intel/common/gen_sample_positions.h 
> b/src/intel/common/gen_sample_positions.h
> index f0ce95dd1fb..da48dcb5ed0 100644
> --- a/src/intel/common/gen_sample_positions.h
> +++ b/src/intel/common/gen_sample_positions.h
> @@ -42,10 +42,10 @@ prefix##0YOffset   = 0.5;
>   * c   1
>   */
>  #define GEN_SAMPLE_POS_2X(prefix) \
> -prefix##0XOffset   = 0.25; \
> -prefix##0YOffset   = 0.25; \
> -prefix##1XOffset   = 0.75; \
> -prefix##1YOffset   = 0.75;
> +prefix##0XOffset   = 0.75; \
> +prefix##0YOffset   = 0.75; \
> +prefix##1XOffset   = 0.25; \
> +prefix##1YOffset   = 0.25;
>
>  /**
>   * Sample positions:
> diff --git a/src/mesa/drivers/dri/i965/brw_multisample_state.h 
> b/src/mesa/drivers/dri/i965/brw_multisample_state.h
> index 6cf324e561c..2142a17a484 100644
> --- a/src/mesa/drivers/dri/i965/brw_multisample_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_multisample_state.h
> @@ -38,13 +38,13 @@
>  /**
>   * 1x MSAA has a single sample at the center: (0.5, 0.5) -> (0x8, 0x8).
>   *
> - * 2x MSAA sample positions are (0.25, 0.25) and (0.75, 0.75):
> + * 2x MSAA sample positions are (0.75, 0.75) and (0.25, 0.25):
>   *   4 c
> - * 4 0
> - * c   1
> + * 4 1
> + * c   0
>   */
>  static const uint32_t
> -brw_multisample_positions_1x_2x = 0x0088cc44;
> +brw_multisample_positions_1x_2x = 0x008844cc;
>
>  /**
>   * Sample positions:
> diff --git a/src/mesa/drivers/dri/i965/gen6_multisample_state.c 
> b/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> index bfa84fb9b77..78ff3942075 100644
> --- a/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> @@ -70,7 +70,7 @@ gen6_get_sample_position(struct gl_context *ctx,
>   *
>   * 2X MSAA sample index / number layout
You should update this comment matching it up with comment in blorp and
add  "sample number layout:" on top of below layout. I would leave it up to
you if you also want to draw sample index layout here just for uniformity.
>   *   -
> - *   | 0 | 1 |
> + *   | 1 | 0 |
>   *   -
>   *
>   * 4X MSAA sample index / number layout
> @@ -107,7 +107,7 @@ gen6_get_sample_position(struct gl_context *ctx,
>  void
>  gen6_set_sample_maps(struct gl_context *ctx)
>  {
> -   uint8_t map_2x[2] = {0, 1};
> +   uint8_t map_2x[2] = {1, 0};
> uint8_t map_4x[4] = {0, 1, 2, 3};
> uint8_t map_8x[8] = {3, 7, 5, 0, 1, 2, 4, 6};
> uint8_t map_16x[16] = { 15, 10, 9, 7, 4, 1, 3, 13,
> --
> 2.17.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

With above changes
Reviewed-by: Anuj Phogat 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH libdrm] drm/i915/cfl: Add a new CFL PCI ID.

[Souza, Jose]

On Wed, 2018-08-08 at 22:41 -0700, Rodrigo Vivi wrote:
> One more CFL ID added to spec.
> 
> Align with kernel commit d0e062ebb3a4 ("drm/i915/cfl:
> Add a new CFL PCI ID.")
> 

Reviewed-by: José Roberto de Souza 

> Cc: José Roberto de Souza 
> Signed-off-by: Rodrigo Vivi 
> ---
>  intel/intel_chipset.h | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
> index 583d6447..4a34b7be 100644
> --- a/intel/intel_chipset.h
> +++ b/intel/intel_chipset.h
> @@ -230,7 +230,8 @@
>  #define PCI_CHIP_COFFEELAKE_S_GT2_1 0x3E91
>  #define PCI_CHIP_COFFEELAKE_S_GT2_2 0x3E92
>  #define PCI_CHIP_COFFEELAKE_S_GT2_3 0x3E96
> -#define PCI_CHIP_COFFEELAKE_S_GT2_4 0x3E9A
> +#define PCI_CHIP_COFFEELAKE_S_GT2_4 0x3E98
> +#define PCI_CHIP_COFFEELAKE_S_GT2_5 0x3E9A
>  #define PCI_CHIP_COFFEELAKE_H_GT2_1 0x3E9B
>  #define PCI_CHIP_COFFEELAKE_H_GT2_2 0x3E94
>  #define PCI_CHIP_COFFEELAKE_U_GT2_1 0x3EA9
> @@ -509,7 +510,8 @@
>   (devid) ==
> PCI_CHIP_COFFEELAKE_S_GT2_1 || \
>   (devid) ==
> PCI_CHIP_COFFEELAKE_S_GT2_2 || \
>   (devid) ==
> PCI_CHIP_COFFEELAKE_S_GT2_3 || \
> - (devid) ==
> PCI_CHIP_COFFEELAKE_S_GT2_4)
> + (devid) ==
> PCI_CHIP_COFFEELAKE_S_GT2_4 || \
> + (devid) ==
> PCI_CHIP_COFFEELAKE_S_GT2_5)
>  
>  #define IS_CFL_H(devid) ((devid) ==
> PCI_CHIP_COFFEELAKE_H_GT2_1 || \
>   (devid) ==
> PCI_CHIP_COFFEELAKE_H_GT2_2)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Why are pbuffers backed by a pixmaps (Was Re: [PATCH] egl: Fix leak of X11 pixmaps backing pbuffers in DRI3.)

[Adam Jackson]

On Thu, 2018-08-09 at 18:22 +0100, Emil Velikov wrote:

> In the GLX case, it's required due to server-side rendering. One needs
> a separate primitive for each pbuffer, thus the information can be
> passed long the wire.

I can't parse this. "Primitive"?

So, backstory time. GLX_SGIX_pbuffer was the genesis of pbuffers.
Here's what it has to say about how a pbuffer is unlike other GLX
drawables (some of which is a comment about how things happened to work
on IRIX):

"GLXPbuffers are equivalent to GLXPixmaps with the following
exceptions:

1.  There is no associated X pixmap. Also, since a GLXPbuffer is a GLX
resource, it may not be possible to render to it using X or an 
X extension other than GLX.

2.  The format of the color buffers and the type and size of any
associated ancillary buffers for a GLXPbuffer can only be
described with a GLXFBConfig -- an X Visual cannot be used.

3.  It is possible to create a GLXPbuffer whose contents may be 
asynchronously lost at any time.

4.  GLXPbuffers can be rendered to using either direct or indirect
rendering contexts.

5.  The allocation of a GLXPbuffer can fail if there are insufficient
resources (i.e., all the pbuffer memory has been allocated and 
the implementation does not virtualize pbuffer memory.)"

In contrast, a GLXPixmap _must_ be renderable by X11, cannot lose its
contents, and _may_ not be renderable by direct contexts. All of this
dates to like 1997, so we didn't have FBOs yet, and any rendering
surface would have been allocated by "the server" [1]. That extension
was merged into GLX 1.3 pretty much unchanged, and GLX 1.3 was 1998. 

Xorg didn't get GLX 1.3 working until like 2007 [2]. As an
implementation choice, Xorg _does_ back pbuffers with pixmaps, both in
that it calls ->CreatePixmap internally to make them and that it tracks
them as actual pixmaps [3]. We never clobber pbuffers, we have no
problem rendering to pbuffers from X11, and we have no problem
rendering to GLXPixmaps from direct contexts. Effectively, other than
the resource type, the difference between a pbuffer and a glxpixmap is
that the latter takes two XIDs and two resource creation calls.

EGL dates to 2003, which means it predates even EXT_framebuffer_object,
so the pbuffer bit of GLX was kept in the new API. The EGL spec says a
pbuffer has no associated native window or native window type - no
kidding, it's not a window - but it does not require that a pbuffer
have no native object backing it at all.

Now, direct rendering in GLX is underspecified; the implementation is
free to use whatever subset of GLX, and whatever private protocol, it
wants. In principle the client could allocate the pbuffer's storage
from the "server" (the drm, here), and pass that handle and its new XID
name to the X server to bind them together so indirect contexts can
name it as well.

An EGL implementation could take even more liberties. Even on an X11
window system there's no intrinsic reason that an EGL pbuffer need to
exist as a native winsys object; all that's required is that pbuffers
work with any API that takes an EGLSurface parameter. You might choose
to mirror EGL surfaces as GLX objects, or not, whatever your
implementation finds convenient. In practice, we back pbuffers with
pixmaps because we also back pixmaps with pixmaps, and there's no
reason to make those paths diverge more than they have to.

[1] - Even in this old IRIXy context, "the server" doesn't necessarily
mean the X server, it means the rendering services provided by the OS;
but as pbuffers have an XID, the X server is where this allocation
happens from the GLX app's perspective, regardless of what OS services
xserver (or libGL) invokes to make that happen.

[2] - Some of that was embarassment, as GLX_EXT_texture_from_pixmap
technically requires 1.3, and we were cheating. Some of it was that the
GLX code at this point was under SGI FreeB 1.1, and we were all
pretending that was okay to distribute, but were hesitant to make
changes due to the license terms being unpleasant.

[3] - XDestroyPixmap(glXCreatePbuffer()) ought to throw GLXBadDrawable,
not succeed, so this latter behavior is a bug and probably a spec
violation.

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

Re: [Mesa-dev] [PATCH RFC] st/mesa: check st_context in st_renderbuffer_delete()

[Dave Airlie]

Sounds like a missing make current somewhere.

Dave.

On Fri., 10 Aug. 2018, 03:28 Marek Olšák,  wrote:

> This will leak the renderbuffer, but that's not the biggest problem.
>
> In your bug report, you said that the renderbuffer was created by
> intel_new_renderbuffer, but this change is for st/mesa. Something is
> horribly wrong here. The intel driver should not ever end up in
> st/mesa, because st/mesa is a different driver. What is going on here?
>
>
> Marek
>
> On Thu, Aug 2, 2018 at 8:29 AM, Olivier Fourdan 
> wrote:
> > st_renderbuffer_delete() can segfault if we get a non-NULL context
> > pointer but if the st_context is NULL:
> >
> >   Thread 1 "Xwayland" received signal SIGSEGV, Segmentation fault.
> >   in st_renderbuffer_delete () at state_tracker/st_cb_fbo.c:241
> >   241 pipe_surface_release(st->pipe, >surface_srgb);
> >   (gdb) bt
> >   #0  st_renderbuffer_delete () at state_tracker/st_cb_fbo.c:241
> >   #1  _mesa_reference_renderbuffer_ () at main/renderbuffer.c:212
> >   #2  _mesa_reference_renderbuffer () at main/renderbuffer.h:72
> >   #3  _mesa_free_framebuffer_data (0) at main/framebuffer.c:229
> >   #4  _mesa_destroy_framebuffer () at main/framebuffer.c:207
> >   #5  _mesa_reference_framebuffer_ () at main/framebuffer.c:265
> >   #6  _mesa_reference_framebuffer () at main/framebuffer.h:63
> >   #7  _mesa_free_context_data () at main/context.c:1326
> >   #8  st_destroy_context () at state_tracker/st_context.c:653
> >   #9  dri_destroy_context () at dri_context.c:239
> >   #10 driDestroyContext () at dri_util.c:524
> >   #11 __glXDRIcontextDestroy () at glxdriswrast.c:132
> >   #12 __glXFreeContext () at glxext.c:190
> >   #13 ContextGone () at glxext.c:82
> >   #14 doFreeResource () at resource.c:880
> >   #15 FreeResourceByType () at resource.c:941
> >   #16 __glXDisp_DestroyContext () at glxcmds.c:437
> >   #17 dispatch_DestroyContext () at vnd_dispatch_stubs.c:82
> >   #18 Dispatch () at dispatch.c:478
> >   #19 dix_main () at main.c:276
> >   #20 __libc_start_main () from /lib64/libc.so.6
> >   #21 _start () at glxcmds.c:125
> >
> >   (gdb) p st
> >   $1 = (struct st_context *) 0x0
> >
> > Check for a non-NULL st_context pointer as well to avoid the crash.
> >
> > Bugzilla: https://bugzilla.redhat.com/1611140
> > Signed-off-by: Olivier Fourdan 
> > ---
> >  Note: This fixes several bug reported downstream, like:
> >   https://bugzilla.redhat.com/1611140
> >   https://bugs.launchpad.net/ubuntu/+source/xorg-server/+bug/1762971
> >   https://bugs.launchpad.net/ubuntu/+source/mesa/+bug/1754693
> >   etc.
> >  I don't know what this client actually does, but whatever it is it
> should
> >  not crash Xwayland because of Mesa...
> >  I tested this fix against the given reproducer (run snap on
> Wayland/Xwayland)
> >  and it works.
> >
> >  src/mesa/state_tracker/st_cb_fbo.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/state_tracker/st_cb_fbo.c
> b/src/mesa/state_tracker/st_cb_fbo.c
> > index 73414fdfa1..856d213b73 100644
> > --- a/src/mesa/state_tracker/st_cb_fbo.c
> > +++ b/src/mesa/state_tracker/st_cb_fbo.c
> > @@ -238,8 +238,10 @@ st_renderbuffer_delete(struct gl_context *ctx,
> struct gl_renderbuffer *rb)
> > struct st_renderbuffer *strb = st_renderbuffer(rb);
> > if (ctx) {
> >struct st_context *st = st_context(ctx);
> > -  pipe_surface_release(st->pipe, >surface_srgb);
> > -  pipe_surface_release(st->pipe, >surface_linear);
> > +  if (st) {
> > + pipe_surface_release(st->pipe, >surface_srgb);
> > + pipe_surface_release(st->pipe, >surface_linear);
> > +  }
> >strb->surface = NULL;
> > }
> > pipe_resource_reference(>texture, NULL);
> > --
> > 2.17.1
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] Gallium/tgsi: Correct signedness of return value of bit operations

[Roland Scheidegger]

Am 09.08.2018 um 19:28 schrieb Gert Wollny:
> From: Gert Wollny 
> 
> The GLSL operations findLSB, findMSB, and countBits always return
> a signed integer type. Let TGSI reflect this.
> 
> v2: Properly set values in infer_(src|dst)_type   (Thanks Roland
> Schneideregger for pointing out problems with my 1st approach)
That's "Scheidegger" :).

> 
> Signed-off-by: Gert Wollny 
> ---
>  src/gallium/auxiliary/tgsi/tgsi_info.c | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c 
> b/src/gallium/auxiliary/tgsi/tgsi_info.c
> index bbe1a21e43..ff58b4c22e 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
> @@ -150,9 +150,6 @@ tgsi_opcode_infer_type(enum tgsi_opcode opcode)
> case TGSI_OPCODE_UBFE:
> case TGSI_OPCODE_BFI:
> case TGSI_OPCODE_BREV:
> -   case TGSI_OPCODE_POPC:
> -   case TGSI_OPCODE_LSB:
> -   case TGSI_OPCODE_UMSB:
> case TGSI_OPCODE_IMG2HND:
> case TGSI_OPCODE_SAMP2HND:
>return TGSI_TYPE_UNSIGNED;
> @@ -269,6 +266,7 @@ tgsi_opcode_infer_src_type(enum tgsi_opcode opcode, uint 
> src_idx)
> case TGSI_OPCODE_UP2H:
> case TGSI_OPCODE_U2I64:
> case TGSI_OPCODE_MEMBAR:
> +   case TGSI_OPCODE_UMSB:
>return TGSI_TYPE_UNSIGNED;
> case TGSI_OPCODE_IMUL_HI:
> case TGSI_OPCODE_I2F:
> @@ -324,5 +322,12 @@ tgsi_opcode_infer_dst_type(enum tgsi_opcode opcode, uint 
> dst_idx)
> if (dst_idx == 1 && opcode == TGSI_OPCODE_DFRACEXP)
>return TGSI_TYPE_SIGNED;
>  
> -   return tgsi_opcode_infer_type(opcode);
> +   switch (opcode) {
> +   case TGSI_OPCODE_LSB:
> +   case TGSI_OPCODE_POPC:
> +   case TGSI_OPCODE_UMSB:
> +  return TGSI_TYPE_SIGNED;
> +   default:
> +  return tgsi_opcode_infer_type(opcode);
> +   }
So, I don't think that's really the correct approach to treat them
differently than other opcodes. I mean, if you want to change the type
of them, just add the 3 of them to infer_type like you had in your first
patch, so the type of the instruction is the same as the dst type. (Plus
of course the umsb to unsigned infer_src_type like in this patch, since
this opcode really takes unsigned operands and there's a separate opcode
for signed operands). For such a patch, you have my R-b...

Roland


>  }
> 

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

Re: [Mesa-dev] [PATCH 1/4] vulkan: Add central copy of entrypoints/extensions code.

[Chad Versace]

On Wed 08 Aug 2018, Bas Nieuwenhuizen wrote:
> ---
>  src/vulkan/Makefile.am|   3 +
>  src/vulkan/util/meson.build   |   2 +
>  src/vulkan/util/vk_entrypoints_gen.py | 515 ++
>  src/vulkan/util/vk_extensions.py  |  92 +
>  src/vulkan/util/vk_extensions_gen.py  | 205 ++
>  5 files changed, 817 insertions(+)
>  create mode 100644 src/vulkan/util/vk_entrypoints_gen.py
>  create mode 100644 src/vulkan/util/vk_extensions.py
>  create mode 100644 src/vulkan/util/vk_extensions_gen.py


> +class VkVersion:
> +def __init__(self, string):
> +split = string.split('.')
> +self.major = int(split[0])
> +self.minor = int(split[1])
> +if len(split) > 2:
> +assert len(split) == 3
> +self.patch = int(split[2])
> +else:
> +self.patch = None
> +
> +# Sanity check.  The range bits are required by the definition of the
> +# VK_MAKE_VERSION macro
> +assert self.major < 1024 and self.minor < 1024
> +assert self.patch is None or self.patch < 4096
> +assert(str(self) == string)
> +
> +def __str__(self):
> +ver_list = [str(self.major), str(self.minor)]
> +if self.patch is not None:
> +ver_list.append(str(self.patch))
> +return '.'.join(ver_list)
> +
> +def c_vk_version(self):
> +patch = self.patch if self.patch is not None else 0
> +ver_list = [str(self.major), str(self.minor), str(patch)]
> +return 'VK_MAKE_VERSION(' + ', '.join(ver_list) + ')'
> +
> +def __int_ver(self):
> +# This is just an expansion of VK_VERSION
> +patch = self.patch if self.patch is not None else 0
> +return (self.major << 22) | (self.minor << 12) | patch
> +
> +def __cmp__(self, other):

Patch 3 replaces __cmp__ here with __gt__. Was that on purpose?

> +# If only one of them has a patch version, "ignore" it by making
> +# other's patch version match self.
> +if (self.patch is None) != (other.patch is None):
> +other = copy.copy(other)
> +other.patch = self.patch
> +
> +return self.__int_ver().__cmp__(other.__int_ver())
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/4] Merge vulkan API generators.

[Chad Versace]

On Wed 08 Aug 2018, Bas Nieuwenhuizen wrote:
> radv was always just mirroring a derived version of the anv
> version, sometimes hacked together and sometimes very behind.
> 
> As we grow more vulkan drivers this repetition makes even less
> sense, so lets merge them. I took the anv generators as the
> template and made radv use them.
> 
> This includes some messy stuff in the build system due to 
> difficulties with python includes. I tested with meson and
> autotools. Android.mk is updated but not tested.
> 
> Bas Nieuwenhuizen (4):
>   vulkan: Add central copy of entrypoints/extensions code.
>   vulkan/util: Add support to not generate the trampolines.
>   anv: Use central api generation scripts.
>   radv: Integrate with common generators.

git-am complains that patches 3 and 4 insert trailing whitespace.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/4] anv: Use central api generation scripts.

[Chad Versace]

On Wed 08 Aug 2018, Bas Nieuwenhuizen wrote:
> This became kind of messy as python imports cannot really look up
> parent/sibling directories. I saw some scripts use sys.path but
> that became even more messy due to import locations.
> 
> I also move the selections of the dispatch table out of the
> generation script because it is not easily shared, and generating
> it did not really win anything anyway.
> ---
>  src/intel/Android.vulkan.mk |   9 +
>  src/intel/Makefile.vulkan.am|  25 +-
>  src/intel/vulkan/anv_device.c   |  46 ++
>  src/intel/vulkan/anv_entrypoints_gen.py | 537 +---
>  src/intel/vulkan/anv_extensions.py  |  68 +--
>  src/intel/vulkan/anv_extensions_gen.py  | 177 +---
>  src/intel/vulkan/meson.build|  15 +-
>  src/vulkan/util/vk_extensions.py|   5 +-
>  8 files changed, 98 insertions(+), 784 deletions(-)
> 
> diff --git a/src/intel/Android.vulkan.mk b/src/intel/Android.vulkan.mk


>  $(intermediates)/vulkan/anv_entrypoints.h: $(intermediates)/vulkan/dummy.c
> + PYTHONPATH=$(MESA_TOP)/src/vulkan/util \
>   $(VK_ENTRYPOINTS_SCRIPT) \
>   --outdir $(dir $@) \
>   --xml $(MESA_TOP)/src/vulkan/registry/vk.xml

Yes, modifying PYTHONPATH is messy, but it seems to me that it's the the
least-messy way.

I'm no expert on build systems, but I think it's wrong to clobber
PYTHONPATH. Instead, you should prepend the dir to PYTHONPATH.

For example, on my machine, PYTHONPATH is already set:

$ echo $PYTHONPATH
/usr/local/buildtools/current/sitecustomize

So, this is more correct...

   $(intermediates)/vulkan/anv_entrypoints.h: $(intermediates)/vulkan/dummy.c
PYTHONPATH="$(MESA_TOP)/src/vulkan/util:$${PYTHONPATH}"
$(VK_ENTRYPOINTS_SCRIPT) \
--outdir $(dir $@) \
--xml $(MESA_TOP)/src/vulkan/registry/vk.xml

but runs the risk of accidentally inserting $PWD into PYTHONPATH, because
Python interprets each empty path in PYTHONPATH as equivalent to $PWD.

So... perhaps it is better to modify sys.path instead of PYTHONPATH. It is
definitely safer.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/4] anv: Use central api generation scripts.

[Chad Versace]

On Tue 07 Aug 2018, Dylan Baker wrote:
> Quoting Bas Nieuwenhuizen (2018-08-07 16:14:33)   
>   
>   
> 
> > 
> > 
> > 
> > 
> >  anv_extensions_c = custom_target(  
> > 
> > 
> > 
> > @@ -36,10 +37,11 @@ anv_extensions_c = custom_target(   
> > 
> > 
> > 
> >input : ['anv_extensions_gen.py', vk_api_xml],   
> > 
> > 
> > 
> >output : 'anv_extensions.c', 
> > 
> > 
> > 
> >command : [  
> > 
> > 
> > 
> > +   'env', 'PYTHONPATH=@0@'.format(join_paths(meson.source_root(), 
> > 'src/vulkan/util/')),   
> > 
> >  
> 
> This is really gross, you're adding a dependency on a unix console command. I 
>   
>   
> 
> know that anv is only built on Unix-like oses, but this will eventually end 
> up
>   
>   
> being used in some code that needs to run on Windows (or mac, does mac have   
>   
>   
> 
> env?).
>   
>   
> 
> 
> I know that some people will object, but IMHO a better solution than mucking  
>   
>   
> 
> with the python path (either through sys.path or through PYTHONPATH, is to
>   
>   
> 
> put all of the generators in a src/generators directory and be done with it.  
>   
>   
> 
> Sure the intel specific bits (for example) aren't in the src/intel folder, 
> that's a small price to avoid having to call env just to run a python script.

Dylan, I think we should avoid introducing complexity in the build
system for the benefit of operating systems not supported by the driver.
That feels like a serious premature optimazation, to me.  Anvil's usage
of ioctls is highly specific to Linux/Unix, will not work on MacOS, and
definitely does not work on Windows.

(By the way, MacOS has the 'env' command).

Re: [Mesa-dev] [PATCH v3 08/48] meson: fix dl detection on non cygwin windows

[Dylan Baker]

Quoting Eric Engestrom (2018-08-09 08:36:55)
> On Monday, 2018-08-06 17:50:48 -0700, Dylan Baker wrote:
> > ---
> >  meson.build | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/meson.build b/meson.build
> > index c7dd5ddfec6..788021c05e9 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1027,9 +1027,9 @@ endif
> >  if cc.has_function('dlopen')
> 
> This check is not needed on windows, is it? It will always fail afaict.
> 
> How about this?
>   if host == windows or cc.has_function(dlopen)
> 
> >dep_dl = null_dep
> >  else
> > -  dep_dl = cc.find_library('dl')
> > +  dep_dl = cc.find_library('dl', required : host_machine.system() != 
> > 'windows')
> >  endif
> > -if cc.has_function('dladdr', dependencies : dep_dl)
> > +if host_machine.system() != 'windows' and cc.has_function('dladdr', 
> > dependencies : dep_dl)
> ># This is really only required for megadrivers
> >pre_args += '-DHAVE_DLADDR'
> >  endif

How about:
# check for dl support
dep_dl = null_dep
if host_machine.system() != 'windows'
  if not cc.has_function('dlopen')
dep_dl = cc.find_library('dl')
  endif
  if cc.has_function('dladdr', dependencies : dep_dl)
# This is really only required for megadrivers
pre_args += '-DHAVE_DLADDR'
  endif
endif

> 
> With that, 1-4 and 7-8 are:
> Reviewed-by: Eric Engestrom 
> 
> > -- 
> > 2.18.0
> > 
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

[Mesa-dev] [PATCH v2] Gallium/tgsi: Correct signedness of return value of bit operations

[Gert Wollny]

From: Gert Wollny 

The GLSL operations findLSB, findMSB, and countBits always return
a signed integer type. Let TGSI reflect this.

v2: Properly set values in infer_(src|dst)_type   (Thanks Roland
Schneideregger for pointing out problems with my 1st approach)

Signed-off-by: Gert Wollny 
---
 src/gallium/auxiliary/tgsi/tgsi_info.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c 
b/src/gallium/auxiliary/tgsi/tgsi_info.c
index bbe1a21e43..ff58b4c22e 100644
--- a/src/gallium/auxiliary/tgsi/tgsi_info.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
@@ -150,9 +150,6 @@ tgsi_opcode_infer_type(enum tgsi_opcode opcode)
case TGSI_OPCODE_UBFE:
case TGSI_OPCODE_BFI:
case TGSI_OPCODE_BREV:
-   case TGSI_OPCODE_POPC:
-   case TGSI_OPCODE_LSB:
-   case TGSI_OPCODE_UMSB:
case TGSI_OPCODE_IMG2HND:
case TGSI_OPCODE_SAMP2HND:
   return TGSI_TYPE_UNSIGNED;
@@ -269,6 +266,7 @@ tgsi_opcode_infer_src_type(enum tgsi_opcode opcode, uint 
src_idx)
case TGSI_OPCODE_UP2H:
case TGSI_OPCODE_U2I64:
case TGSI_OPCODE_MEMBAR:
+   case TGSI_OPCODE_UMSB:
   return TGSI_TYPE_UNSIGNED;
case TGSI_OPCODE_IMUL_HI:
case TGSI_OPCODE_I2F:
@@ -324,5 +322,12 @@ tgsi_opcode_infer_dst_type(enum tgsi_opcode opcode, uint 
dst_idx)
if (dst_idx == 1 && opcode == TGSI_OPCODE_DFRACEXP)
   return TGSI_TYPE_SIGNED;
 
-   return tgsi_opcode_infer_type(opcode);
+   switch (opcode) {
+   case TGSI_OPCODE_LSB:
+   case TGSI_OPCODE_POPC:
+   case TGSI_OPCODE_UMSB:
+  return TGSI_TYPE_SIGNED;
+   default:
+  return tgsi_opcode_infer_type(opcode);
+   }
 }
-- 
2.16.4


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

Re: [Mesa-dev] [PATCH RFC] st/mesa: check st_context in st_renderbuffer_delete()

[Marek Olšák]

This will leak the renderbuffer, but that's not the biggest problem.

In your bug report, you said that the renderbuffer was created by
intel_new_renderbuffer, but this change is for st/mesa. Something is
horribly wrong here. The intel driver should not ever end up in
st/mesa, because st/mesa is a different driver. What is going on here?

Marek

On Thu, Aug 2, 2018 at 8:29 AM, Olivier Fourdan  wrote:
> st_renderbuffer_delete() can segfault if we get a non-NULL context
> pointer but if the st_context is NULL:
>
>   Thread 1 "Xwayland" received signal SIGSEGV, Segmentation fault.
>   in st_renderbuffer_delete () at state_tracker/st_cb_fbo.c:241
>   241 pipe_surface_release(st->pipe, >surface_srgb);
>   (gdb) bt
>   #0  st_renderbuffer_delete () at state_tracker/st_cb_fbo.c:241
>   #1  _mesa_reference_renderbuffer_ () at main/renderbuffer.c:212
>   #2  _mesa_reference_renderbuffer () at main/renderbuffer.h:72
>   #3  _mesa_free_framebuffer_data (0) at main/framebuffer.c:229
>   #4  _mesa_destroy_framebuffer () at main/framebuffer.c:207
>   #5  _mesa_reference_framebuffer_ () at main/framebuffer.c:265
>   #6  _mesa_reference_framebuffer () at main/framebuffer.h:63
>   #7  _mesa_free_context_data () at main/context.c:1326
>   #8  st_destroy_context () at state_tracker/st_context.c:653
>   #9  dri_destroy_context () at dri_context.c:239
>   #10 driDestroyContext () at dri_util.c:524
>   #11 __glXDRIcontextDestroy () at glxdriswrast.c:132
>   #12 __glXFreeContext () at glxext.c:190
>   #13 ContextGone () at glxext.c:82
>   #14 doFreeResource () at resource.c:880
>   #15 FreeResourceByType () at resource.c:941
>   #16 __glXDisp_DestroyContext () at glxcmds.c:437
>   #17 dispatch_DestroyContext () at vnd_dispatch_stubs.c:82
>   #18 Dispatch () at dispatch.c:478
>   #19 dix_main () at main.c:276
>   #20 __libc_start_main () from /lib64/libc.so.6
>   #21 _start () at glxcmds.c:125
>
>   (gdb) p st
>   $1 = (struct st_context *) 0x0
>
> Check for a non-NULL st_context pointer as well to avoid the crash.
>
> Bugzilla: https://bugzilla.redhat.com/1611140
> Signed-off-by: Olivier Fourdan 
> ---
>  Note: This fixes several bug reported downstream, like:
>   https://bugzilla.redhat.com/1611140
>   https://bugs.launchpad.net/ubuntu/+source/xorg-server/+bug/1762971
>   https://bugs.launchpad.net/ubuntu/+source/mesa/+bug/1754693
>   etc.
>  I don't know what this client actually does, but whatever it is it should
>  not crash Xwayland because of Mesa...
>  I tested this fix against the given reproducer (run snap on Wayland/Xwayland)
>  and it works.
>
>  src/mesa/state_tracker/st_cb_fbo.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_fbo.c 
> b/src/mesa/state_tracker/st_cb_fbo.c
> index 73414fdfa1..856d213b73 100644
> --- a/src/mesa/state_tracker/st_cb_fbo.c
> +++ b/src/mesa/state_tracker/st_cb_fbo.c
> @@ -238,8 +238,10 @@ st_renderbuffer_delete(struct gl_context *ctx, struct 
> gl_renderbuffer *rb)
> struct st_renderbuffer *strb = st_renderbuffer(rb);
> if (ctx) {
>struct st_context *st = st_context(ctx);
> -  pipe_surface_release(st->pipe, >surface_srgb);
> -  pipe_surface_release(st->pipe, >surface_linear);
> +  if (st) {
> + pipe_surface_release(st->pipe, >surface_srgb);
> + pipe_surface_release(st->pipe, >surface_linear);
> +  }
>strb->surface = NULL;
> }
> pipe_resource_reference(>texture, NULL);
> --
> 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 v3 20/48] meson: build graw-gdi target

[Dylan Baker]

Quoting Eric Anholt (2018-08-07 11:14:16)
> Dylan Baker  writes:
> 
> > ---
> >  src/gallium/meson.build  |  4 ++-
> >  src/gallium/targets/graw-gdi/meson.build | 36 
> >  2 files changed, 39 insertions(+), 1 deletion(-)
> >  create mode 100644 src/gallium/targets/graw-gdi/meson.build
> >
> > diff --git a/src/gallium/meson.build b/src/gallium/meson.build
> > index 5019477788b..e4e0b88e7fc 100644
> > --- a/src/gallium/meson.build
> > +++ b/src/gallium/meson.build
> > @@ -197,7 +197,9 @@ if with_platform_windows
> >  endif
> >  if with_tests
> >subdir('targets/graw-null')
> > -  if with_glx == 'gallium-xlib'
> > +  if with_platform_windows
> > +subdir('targets/graw-gdi')
> > +  elif with_glx == 'gallium-xlib'
> >  subdir('targets/graw-xlib')
> >endif
> >subdir('tests')
> > diff --git a/src/gallium/targets/graw-gdi/meson.build 
> > b/src/gallium/targets/graw-gdi/meson.build
> > new file mode 100644
> > index 000..e04b454ab53
> > --- /dev/null
> > +++ b/src/gallium/targets/graw-gdi/meson.build
> > @@ -0,0 +1,36 @@
> > +# Copyright © 2018 Intel Corporation
> > +
> > +# Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > +# of this software and associated documentation files (the "Software"), to 
> > deal
> > +# in the Software without restriction, including without limitation the 
> > rights
> > +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > +# copies of the Software, and to permit persons to whom the Software is
> > +# furnished to do so, subject to the following conditions:
> > +
> > +# The above copyright notice and this permission notice shall be included 
> > in
> > +# all copies or substantial portions of the Software.
> > +
> > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> > THE
> > +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> > FROM,
> > +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
> > IN THE
> > +# SOFTWARE.
> > +
> > +libgraw_gdi = shared_library(
> > +  'graw',
> > +  'graw_gdi.c',
> > +  include_directories : [
> > +inc_include, inc_src, inc_gallium, inc_gallium_aux, 
> > inc_gallium_drivers,
> > +inc_gallium_winsys_sw,
> > +  ],
> > +  link_with : [
> > +libgraw_util, libmesa_util, libgallium, libwsgdi,
> > +  ],
> > +  dependencies : [
> > +dep_ws2_32, driver_swrast,
> > +  ],
> > +)
> 
> Looks like this is missing GALLIUM_SOFTPIPE/GALLIUM_LLVMPIPE
> definitions.

driver_swrast will have the correct defines depending on whether softpipe or
llvmpipe is being built.


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

Re: [Mesa-dev] [PATCH v3 19/48] meson: build libgl-gdi target

[Dylan Baker]

Quoting Eric Anholt (2018-08-07 11:12:27)
> Dylan Baker  writes:
> 
> > ---
> >  src/gallium/meson.build   |  1 +
> >  src/gallium/targets/libgl-gdi/meson.build | 44 +++
> >  2 files changed, 45 insertions(+)
> >  create mode 100644 src/gallium/targets/libgl-gdi/meson.build
> >
> > diff --git a/src/gallium/meson.build b/src/gallium/meson.build
> > index a4f28dc4757..5019477788b 100644
> > --- a/src/gallium/meson.build
> > +++ b/src/gallium/meson.build
> > @@ -193,6 +193,7 @@ if with_gallium_st_nine
> >  endif
> >  if with_platform_windows
> >subdir('state_trackers/wgl')
> > +  subdir('targets/libgl-gdi')
> >  endif
> >  if with_tests
> >subdir('targets/graw-null')
> > diff --git a/src/gallium/targets/libgl-gdi/meson.build 
> > b/src/gallium/targets/libgl-gdi/meson.build
> > new file mode 100644
> > index 000..63cc40b97bc
> > --- /dev/null
> > +++ b/src/gallium/targets/libgl-gdi/meson.build
> > @@ -0,0 +1,44 @@
> > +# Copyright © 2018 Intel Corporation
> > +
> > +# Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > +# of this software and associated documentation files (the "Software"), to 
> > deal
> > +# in the Software without restriction, including without limitation the 
> > rights
> > +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> > +# copies of the Software, and to permit persons to whom the Software is
> > +# furnished to do so, subject to the following conditions:
> > +
> > +# The above copyright notice and this permission notice shall be included 
> > in
> > +# all copies or substantial portions of the Software.
> > +
> > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> > THE
> > +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> > FROM,
> > +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
> > IN THE
> > +# SOFTWARE.
> > +
> > +if cc.get_id() == 'gcc' and host_machine.cpu_family() == 'x86_64'
> > +  ogldef = files('../../state_trackers/wgl/opengl32.mingw.def')[0]
> > +else
> > +  ogldef = files('../../state_trackers/wgl/opengl32.def')[0]
> > +endif
> 
> I think you flipped the polarity of the x86_64 check.  Also, please copy
> over the comment explaining what this is about.

You are indeed correct.

> 
> > +libopengl32 = shared_library(
> > +  'opengl32',
> > +  ['libgl_gdi.c'],
> > +  vs_module_defs : ogldef,
> > +  include_directories : [
> > +inc_common, inc_wgl, inc_gallium_winsys_sw, inc_gallium_drivers,
> > +  ],
> > +  link_whole : [libwgl],
> > +  link_with : [
> > +libmesa_util, libgallium, libglsl, libmesa_gallium, libwsgdi,
> > +libglapi_static, libglapi
> > +  ],
> > +  dependencies : [
> > +dep_ws2_32, idep_nir, driver_swrast, driver_swr,
> > +  ],
> > +  name_prefix : '',  # otherwise mingw will create libopengl32.dll
> > +  install : true,
> 
> Looks like you're missing the HAVE_SWR, HAVE_LLVMPIPE definitions.

driver_swrast and driver_swr provide those definitions (driver_swrast is
llvmpipe if that's enabled otherwise it's softpipe).


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

[Mesa-dev] Why are pbuffers backed by a pixmaps (Was Re: [PATCH] egl: Fix leak of X11 pixmaps backing pbuffers in DRI3.)

[Emil Velikov]

Hi Adam,

On 9 August 2018 at 17:41, Adam Jackson  wrote:
> On Thu, 2018-08-09 at 17:20 +0100, Emil Velikov wrote:
>
>> If you have a moment, I'd be interested to know why we're creating a X
>> primitive for pbuffer surface.
>> IIRC pbuffers are used of off-screen rendering, thus having zero
>> knowledge/dependency on the underlying platform.
>
> pbuffers might be offscreen, but they're still window-system objects.
>
Double-checking we're using the same terminology.

Window-system: EGL/GLX/WGL
Underlying platform: X/Wayland/Android/etc, when EGL is used

Agreed, they are window-system objects. Although I cannot see any
reason why we'd need a pixmap here (EGL).
In the GLX case, it's required due to server-side rendering. One needs
a separate primitive for each pbuffer, thus the information can be
passed long the wire.

Feel free to share links which might shed some light.

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

Re: [Mesa-dev] [PATCH] Gallium/tgsi: Correct signdness of return value of bit operations

[Roland Scheidegger]

Am 09.08.2018 um 19:09 schrieb Gert Wollny:
> Am Donnerstag, den 09.08.2018, 18:52 +0200 schrieb Roland Scheidegger:
>> Am 09.08.2018 um 18:18 schrieb Gert Wollny:
>>> Am Donnerstag, den 09.08.2018, 17:10 +0200 schrieb Roland
>>> Scheidegger:
 This is incorrect for umsb.
>>>
>>> Hmm, according to the TGSI doc all of those operations including
>>> UMSB are supposed to return -1 if no bits are set [1], for me that
>>> implies that their return type should be signed. 
>>>
>>> [1] https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2
>>> Fgallium.readthedocs.io%2Fen%2Flatest%2Ftgsi.html%23opcode-
>>> UMSBdata=02%7C01%7Csroland%40vmware.com%7C7dabc2002d7c4ece269d
>>> 08d5fe13cba8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636694283
>>> 339615256sdata=j5BGumW2g%2Bj8NtLAxyy6ZFCoCDsbfMqgrkjtWtIlQBQ%3
>>> Dreserved=0
>>
>> Yes, I guess that's why glsl has them defined as signed.
>> But you could just as well say the definition is they return unsigned
>> 0x (tgsi is really more like d3d10 asm there, so as you know
>> the register file is untyped, and d3d10 says to return 0x for
>> such things, not saying what type this is at all).
>> tgsi doesn't really directly have to mirror glsl opcodes, and
>> certainly not in cases where this amounts to just cosmetic
>> differences.
> Well, it's not that cosmetic when you look at virglrenderer where TGSI
> gets translated back to GLSL. Obviously there one can also force a
> certain return type in other was,, and this is what I initally
> proposed, but Dave asked whether this could also be done via the
> infer_type mechanism, so I did this and to limit the amount the
> virglrenderer/gallium and the mesa/gallium diverge, I also proposed
> this here too. (I added Dave directly to the loop in case he wants to
> add something).  
> 
>> And personally, I prefer them to all be unsigned, because bitops on
>> signed is just always looking crazy. 
> I can understand this, but in the case of the return value I don't
> really see declaring it as signed would be a bad thing.
Yes, as I said I can live with it. If it helps something alright.



> 
> A different approch would then look more or less like this: 
> 
> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
> @@ -150,9 +150,6 @@ tgsi_opcode_infer_type(enum tgsi_opcode opcode)
> case TGSI_OPCODE_UBFE:
> case TGSI_OPCODE_BFI:
> case TGSI_OPCODE_BREV:
> -   case TGSI_OPCODE_POPC:
> -   case TGSI_OPCODE_LSB:
> -   case TGSI_OPCODE_UMSB:
> case TGSI_OPCODE_IMG2HND:
> case TGSI_OPCODE_SAMP2HND:
>return TGSI_TYPE_UNSIGNED;
> @@ -274,6 +271,7 @@ tgsi_opcode_infer_src_type(enum tgsi_opcode opcode,
> uint src_idx)
> case TGSI_OPCODE_I2F:
> case TGSI_OPCODE_I2D:
> case TGSI_OPCODE_I2I64:
> +   case TGSI_OPCODE_UMSB:
You probably wanted to add that to the unsigned section...

Roland



>return TGSI_TYPE_SIGNED;
> case TGSI_OPCODE_ARL:
> case TGSI_OPCODE_ARR:
> @@ -324,5 +322,12 @@ tgsi_opcode_infer_dst_type(enum tgsi_opcode
> opcode, uint dst_idx)
> if (dst_idx == 1 && opcode == TGSI_OPCODE_DFRACEXP)
>return TGSI_TYPE_SIGNED;
>  
> -   return tgsi_opcode_infer_type(opcode);
> +   switch (opcode) {
> +   case TGSI_OPCODE_LSB:
> +   case TGSI_OPCODE_POPC:
> +   case TGSI_OPCODE_UMSB:
> +  return TGSI_TYPE_SIGNED;
> +   default:
> +  return tgsi_opcode_infer_type(opcode);
> +   }
>  }
> 
> Best, 
> Gert
> 

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

Re: [Mesa-dev] [PATCH] Gallium/tgsi: Correct signdness of return value of bit operations

[Gert Wollny]

Am Donnerstag, den 09.08.2018, 18:52 +0200 schrieb Roland Scheidegger:
> Am 09.08.2018 um 18:18 schrieb Gert Wollny:
> > Am Donnerstag, den 09.08.2018, 17:10 +0200 schrieb Roland
> > Scheidegger:
> > > This is incorrect for umsb.
> > 
> > Hmm, according to the TGSI doc all of those operations including
> > UMSB are supposed to return -1 if no bits are set [1], for me that
> > implies that their return type should be signed. 
> > 
> > [1] https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2
> > Fgallium.readthedocs.io%2Fen%2Flatest%2Ftgsi.html%23opcode-
> > UMSBdata=02%7C01%7Csroland%40vmware.com%7C7dabc2002d7c4ece269d
> > 08d5fe13cba8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636694283
> > 339615256sdata=j5BGumW2g%2Bj8NtLAxyy6ZFCoCDsbfMqgrkjtWtIlQBQ%3
> > Dreserved=0
> 
> Yes, I guess that's why glsl has them defined as signed.
> But you could just as well say the definition is they return unsigned
> 0x (tgsi is really more like d3d10 asm there, so as you know
> the register file is untyped, and d3d10 says to return 0x for
> such things, not saying what type this is at all).
> tgsi doesn't really directly have to mirror glsl opcodes, and
> certainly not in cases where this amounts to just cosmetic
> differences.
Well, it's not that cosmetic when you look at virglrenderer where TGSI
gets translated back to GLSL. Obviously there one can also force a
certain return type in other was,, and this is what I initally
proposed, but Dave asked whether this could also be done via the
infer_type mechanism, so I did this and to limit the amount the
virglrenderer/gallium and the mesa/gallium diverge, I also proposed
this here too. (I added Dave directly to the loop in case he wants to
add something).  

> And personally, I prefer them to all be unsigned, because bitops on
> signed is just always looking crazy. 
I can understand this, but in the case of the return value I don't
really see declaring it as signed would be a bad thing. 

A different approch would then look more or less like this: 

--- a/src/gallium/auxiliary/tgsi/tgsi_info.c
+++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
@@ -150,9 +150,6 @@ tgsi_opcode_infer_type(enum tgsi_opcode opcode)
case TGSI_OPCODE_UBFE:
case TGSI_OPCODE_BFI:
case TGSI_OPCODE_BREV:
-   case TGSI_OPCODE_POPC:
-   case TGSI_OPCODE_LSB:
-   case TGSI_OPCODE_UMSB:
case TGSI_OPCODE_IMG2HND:
case TGSI_OPCODE_SAMP2HND:
   return TGSI_TYPE_UNSIGNED;
@@ -274,6 +271,7 @@ tgsi_opcode_infer_src_type(enum tgsi_opcode opcode,
uint src_idx)
case TGSI_OPCODE_I2F:
case TGSI_OPCODE_I2D:
case TGSI_OPCODE_I2I64:
+   case TGSI_OPCODE_UMSB:
   return TGSI_TYPE_SIGNED;
case TGSI_OPCODE_ARL:
case TGSI_OPCODE_ARR:
@@ -324,5 +322,12 @@ tgsi_opcode_infer_dst_type(enum tgsi_opcode
opcode, uint dst_idx)
if (dst_idx == 1 && opcode == TGSI_OPCODE_DFRACEXP)
   return TGSI_TYPE_SIGNED;
 
-   return tgsi_opcode_infer_type(opcode);
+   switch (opcode) {
+   case TGSI_OPCODE_LSB:
+   case TGSI_OPCODE_POPC:
+   case TGSI_OPCODE_UMSB:
+  return TGSI_TYPE_SIGNED;
+   default:
+  return tgsi_opcode_infer_type(opcode);
+   }
 }

Best, 
Gert

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

Re: [Mesa-dev] [PATCH] mesa/st: ETC2 now uses R8G8B8A8_SRGB as fallback

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Thu, Aug 9, 2018 at 6:46 AM, Gert Wollny  wrote:
> The check for ETC2 compatibility was not updated when the fallback
> format was changed.
>
> Fixes: 71867a0a61cea20bf3f6115692e70b0d60f0b70d
>st/mesa: Fall back to R8G8B8A8_SRGB for ETC2
>
> Signed-off-by: Gert Wollny 
> ---
>  src/mesa/state_tracker/st_extensions.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/state_tracker/st_extensions.c 
> b/src/mesa/state_tracker/st_extensions.c
> index 1c01495e93..db08d5169a 100644
> --- a/src/mesa/state_tracker/st_extensions.c
> +++ b/src/mesa/state_tracker/st_extensions.c
> @@ -1318,7 +1318,7 @@ void st_init_extensions(struct pipe_screen *screen,
> screen->is_format_supported(screen, PIPE_FORMAT_R8G8B8A8_UNORM,
> PIPE_TEXTURE_2D, 0, 0,
> PIPE_BIND_SAMPLER_VIEW) &&
> -   screen->is_format_supported(screen, PIPE_FORMAT_B8G8R8A8_SRGB,
> +   screen->is_format_supported(screen, PIPE_FORMAT_R8G8B8A8_SRGB,
> PIPE_TEXTURE_2D, 0, 0,
> PIPE_BIND_SAMPLER_VIEW) &&
> screen->is_format_supported(screen, PIPE_FORMAT_R16_UNORM,
> --
> 2.17.1
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] Gallium/tgsi: Correct signdness of return value of bit operations

[Roland Scheidegger]

Am 09.08.2018 um 18:18 schrieb Gert Wollny:
> Am Donnerstag, den 09.08.2018, 17:10 +0200 schrieb Roland Scheidegger:
>> This is incorrect for umsb.
> Hmm, according to the TGSI doc all of those operations including UMSB
> are supposed to return -1 if no bits are set [1], for me that implies
> that their return type should be signed. 
> 
> [1] 
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgallium.readthedocs.io%2Fen%2Flatest%2Ftgsi.html%23opcode-UMSBdata=02%7C01%7Csroland%40vmware.com%7C7dabc2002d7c4ece269d08d5fe13cba8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636694283339615256sdata=j5BGumW2g%2Bj8NtLAxyy6ZFCoCDsbfMqgrkjtWtIlQBQ%3Dreserved=0
Yes, I guess that's why glsl has them defined as signed.
But you could just as well say the definition is they return unsigned
0x (tgsi is really more like d3d10 asm there, so as you know the
register file is untyped, and d3d10 says to return 0x for such
things, not saying what type this is at all).
tgsi doesn't really directly have to mirror glsl opcodes, and certainly
not in cases where this amounts to just cosmetic differences.
And personally, I prefer them to all be unsigned, because bitops on
signed is just always looking crazy.
Although ignoring the type that way is really something which only works
easily because the type width is always the same, even if it's really
doing the same thing anyway.

> 
>> If you really want to move so the dst type is signed, you need to add
>> it to infer_src_type so the src args are unsigned.
> I see the control flow now, so I guess I actually should add these
> three cases to infer_dst_type, and also add the proper src type for
> umsb.
> 
>> FWIW I'd like to think that all 3 of these always operate on unsigned
>> data (even if they might return signed data), imho they are a lot
>> more easier to understand that way (I mean the signed msb you have to
>> look up what the hell it actually does, bit operations on unsigned
>> data are just easier to understand). But of course it doesn't really
>> make a difference, so I can live with the other 2 if the src type
>> indicates signed.
> GLSL bitCount and findLSB are both operating on both types, and I
> assume that these are actually translated to these two TGSI
> instructions, so the src type should not be set,
Well that doesn't really matter, if you don't set them they will be
implicitly be the same type as the instruction type.

Roland



> 
> thanks for the pointers,
> Gert 
> 
>>
>> Roland
>>
>>
>> Am 09.08.2018 um 10:43 schrieb Gert Wollny:
>>> The GLSL functions  findLSB, findMSB, and countBits always return
>>> a signed integer type. Let the according TGSI operations reflect
>>> this.
>>>
>>> Signed-off-by: Gert Wollny 
>>> ---
>>>  src/gallium/auxiliary/tgsi/tgsi_info.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c
>>> b/src/gallium/auxiliary/tgsi/tgsi_info.c
>>> index bbe1a21e43..a8c2bbe663 100644
>>> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
>>> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
>>> @@ -150,9 +150,6 @@ tgsi_opcode_infer_type(enum tgsi_opcode opcode)
>>> case TGSI_OPCODE_UBFE:
>>> case TGSI_OPCODE_BFI:
>>> case TGSI_OPCODE_BREV:
>>> -   case TGSI_OPCODE_POPC:
>>> -   case TGSI_OPCODE_LSB:
>>> -   case TGSI_OPCODE_UMSB:
>>> case TGSI_OPCODE_IMG2HND:
>>> case TGSI_OPCODE_SAMP2HND:
>>>return TGSI_TYPE_UNSIGNED;
>>> @@ -188,6 +185,9 @@ tgsi_opcode_infer_type(enum tgsi_opcode opcode)
>>> case TGSI_OPCODE_U64SGE:
>>> case TGSI_OPCODE_I64SLT:
>>> case TGSI_OPCODE_I64SGE:
>>> +   case TGSI_OPCODE_LSB:
>>> +   case TGSI_OPCODE_POPC:
>>> +   case TGSI_OPCODE_UMSB:
>>>return TGSI_TYPE_SIGNED;
>>> case TGSI_OPCODE_DADD:
>>> case TGSI_OPCODE_DABS:
>>>
>>
>>

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

Re: [Mesa-dev] [PATCH] egl: Fix leak of X11 pixmaps backing pbuffers in DRI3.

[Adam Jackson]

On Thu, 2018-08-09 at 17:20 +0100, Emil Velikov wrote:

> If you have a moment, I'd be interested to know why we're creating a X
> primitive for pbuffer surface.
> IIRC pbuffers are used of off-screen rendering, thus having zero
> knowledge/dependency on the underlying platform.

pbuffers might be offscreen, but they're still window-system objects.

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

Re: [Mesa-dev] [Mesa-stable] [PATCH] glx: GLX_MESA_multithread_makecurrent is direct-only

[Adam Jackson]

On Thu, 2018-08-09 at 15:54 +0300, Andres Gomez wrote:
> Adam, which is the status of this patch? Is this effectively dropped?

It's still valid, and still not very important. I'd just forgotten it.
Merged now, thanks for the reminder:

remote: remote: I: patch #191858 updated using rev 
63a6b719d98fb1ad58ae93c2de859e6d4bfa8b8b
remote: remote: I: 1 patch(es) updated to state Accepted.
remote: remote:   % Total% Received % Xferd  Average Speed   TimeTime   
  Time  Current
remote: remote:  Dload  Upload   Total   Spent  
  Left  Speed
remote: remote:   0 00 00 0  0  0 --:--:-- --:--:-- 
--:--:-- 0
remote: To ssh://kemper.freedesktop.org/git/mesa/mesa
remote:fcf259ef97..63a6b719d9  63a6b719d98fb1ad58ae93c2de859e6d4bfa8b8b -> 
master
To gitlab.freedesktop.org:mesa/mesa.git
   fcf259ef97f..63a6b719d98  master -> master

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

Re: [Mesa-dev] [Mesa-stable] [PATCH] glx: use correct table offset for glAreTexturesResidentEXT

[Emil Velikov]

On 9 August 2018 at 13:51, Andres Gomez  wrote:
> Emil, this patch has been stalled in the -stable ML for quite some time
> without update.
>
> Unless you say otherwise, I will just ignore it at this point and trust
> that you will also Cc -stable in the future, in case you come with
> another version.
>
Agreed.

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

Re: [Mesa-dev] [Mesa-stable] [PATCH 07/10] gbm: unify error handling in gbm_dri_bo_import()

[Emil Velikov]

On 9 August 2018 at 13:53, Andres Gomez  wrote:
> Emil, this patch never landed in master (nor got a R-b).
>
> Is this still relevant? Could you manage to get somebody to review it?
>
> I'd do it myself but I'm quite ignorant on the GBM bits.
>
I need to respin the whole series since a few corner-cases are not handled.

Let's drop it on your end for now. Updated series will follow in due time.

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

Re: [Mesa-dev] [Mesa-stable] [PATCH 4/8] egl: rework input validation order in _eglCreateWindowSurfaceCommon

[Emil Velikov]

On 9 August 2018 at 13:58, Andres Gomez  wrote:
> Emil, this patch has been stalled in the -stable ML for quite some time
> without update.
>
> Unless you say otherwise, I will just ignore it at this point and trust
> that you will also Cc -stable in the future, in case you come with
> another version.
>
Ack. I will chase the dEQP people first to merge the test fixes and
only after that respin this patch.

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

Re: [Mesa-dev] [PATCH] egl: Fix leak of X11 pixmaps backing pbuffers in DRI3.

[Emil Velikov]

On 7 August 2018 at 20:32, Eric Anholt  wrote:
> This is basically copied from the DRI2 destroy path.  Without this,
> Raspberry Pi would quickly run out of CMA during the EGL tests in the CTS
> due to all the pixmaps laying around.
>
> Fixes: f35198badeb9 ("egl/x11: Implement dri3 support with loader's dri3 
> helper")
Spot on - mirrors the xcb_create_pixmap in dri3_create_surface.

Reviewed-by: Emil Velikov 

If you have a moment, I'd be interested to know why we're creating a X
primitive for pbuffer surface.
IIRC pbuffers are used of off-screen rendering, thus having zero
knowledge/dependency on the underlying platform.

Gut suggests that can drop the pixmap, but I'm likely missing something.

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

Re: [Mesa-dev] [PATCH] Gallium/tgsi: Correct signdness of return value of bit operations

[Gert Wollny]

Am Donnerstag, den 09.08.2018, 17:10 +0200 schrieb Roland Scheidegger:
> This is incorrect for umsb.
Hmm, according to the TGSI doc all of those operations including UMSB
are supposed to return -1 if no bits are set [1], for me that implies
that their return type should be signed. 

[1] http://gallium.readthedocs.io/en/latest/tgsi.html#opcode-UMSB

> If you really want to move so the dst type is signed, you need to add
> it to infer_src_type so the src args are unsigned.
I see the control flow now, so I guess I actually should add these
three cases to infer_dst_type, and also add the proper src type for
umsb.

> FWIW I'd like to think that all 3 of these always operate on unsigned
> data (even if they might return signed data), imho they are a lot
> more easier to understand that way (I mean the signed msb you have to
> look up what the hell it actually does, bit operations on unsigned
> data are just easier to understand). But of course it doesn't really
> make a difference, so I can live with the other 2 if the src type
> indicates signed.
GLSL bitCount and findLSB are both operating on both types, and I
assume that these are actually translated to these two TGSI
instructions, so the src type should not be set, 

thanks for the pointers,
Gert 

> 
> Roland
> 
> 
> Am 09.08.2018 um 10:43 schrieb Gert Wollny:
> > The GLSL functions  findLSB, findMSB, and countBits always return
> > a signed integer type. Let the according TGSI operations reflect
> > this.
> > 
> > Signed-off-by: Gert Wollny 
> > ---
> >  src/gallium/auxiliary/tgsi/tgsi_info.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c
> > b/src/gallium/auxiliary/tgsi/tgsi_info.c
> > index bbe1a21e43..a8c2bbe663 100644
> > --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
> > +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
> > @@ -150,9 +150,6 @@ tgsi_opcode_infer_type(enum tgsi_opcode opcode)
> > case TGSI_OPCODE_UBFE:
> > case TGSI_OPCODE_BFI:
> > case TGSI_OPCODE_BREV:
> > -   case TGSI_OPCODE_POPC:
> > -   case TGSI_OPCODE_LSB:
> > -   case TGSI_OPCODE_UMSB:
> > case TGSI_OPCODE_IMG2HND:
> > case TGSI_OPCODE_SAMP2HND:
> >return TGSI_TYPE_UNSIGNED;
> > @@ -188,6 +185,9 @@ tgsi_opcode_infer_type(enum tgsi_opcode opcode)
> > case TGSI_OPCODE_U64SGE:
> > case TGSI_OPCODE_I64SLT:
> > case TGSI_OPCODE_I64SGE:
> > +   case TGSI_OPCODE_LSB:
> > +   case TGSI_OPCODE_POPC:
> > +   case TGSI_OPCODE_UMSB:
> >return TGSI_TYPE_SIGNED;
> > case TGSI_OPCODE_DADD:
> > case TGSI_OPCODE_DABS:
> > 
> 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 14/14] radeonsi: increase the maximum UBO size to 2 GB

[Marek Olšák]

piglit testing the max uniform block size:

Before:

FRAG
PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1
DCL OUT[0], COLOR
DCL CONST[0][0]
DCL CONST[1][0..4095]
DCL TEMP[0], LOCAL
IMM[0] UINT32 {0, 16, 0, 0}
  0: UMUL TEMP[0].x, CONST[0][0]., IMM[0].
  1: LOAD OUT[0], CONSTBUF[1], TEMP[0].
  2: END


After:

FRAG
PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1
DCL OUT[0], COLOR
DCL CONST[0][0]
DCL CONST[1][0..65527]
DCL TEMP[0], LOCAL
IMM[0] UINT32 {0, 16, 0, 0}
  0: UMUL TEMP[0].x, CONST[0][0]., IMM[0].
  1: LOAD OUT[0], CONSTBUF[1], TEMP[0].
  2: END

CONST[1] is fairly small, but the UBO has 2GB, and CONST[0][0]
contains 134217719, so the final offset is 2147483504.

Marek

On Thu, Aug 9, 2018 at 11:57 AM, Marek Olšák  wrote:
> On Thu, Aug 9, 2018 at 1:35 AM, Roland Scheidegger  wrote:
>> I'm not quite convinced you can really use huge ubos safely? At least
>> direct addressing in tgsi can't work (you've only got a 16bit register
>> index, and it's signed too).
>
> UBOs use the LOAD instruction if PIPE_CAP_LOAD_CONSTBUF is supported.
>
> Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [Mesa-stable] [PATCH v4 1/2] wayland/egl: initialize window surface size to window size

[Eric Engestrom]

On Thursday, 2018-08-09 09:03:02 -0700, Dylan Baker wrote:
> Quoting Juan A. Suarez Romero (2018-08-07 08:49:36)
> > When creating a windows surface with eglCreateWindowSurface(), the
> > width and height returned by eglQuerySurface(EGL_{WIDTH,HEIGHT}) is
> > invalid until buffers are updated (like calling glClear()).
> > 
> > But according to EGL 1.5 spec, section 3.5.6 ("Surface Attributes"):
> > 
> >   "Querying EGL_WIDTH and EGL_HEIGHT returns respectively the width and
> >height, in pixels, of the surface. For a window or pixmap surface,
> >these values are initially equal to the width and height of the
> >native window or pixmap with respect to which the surface was
> >created"
> > 
> > This fixes dEQP-EGL.functional.color_clears.* CTS tests
> > 
> > v2:
> > - Do not modify attached_{width,height} (Daniel)
> > - Do not update size on resizing window (Brendan)
> > 
> > CC: Daniel Stone 
> > CC: Brendan King 
> > CC: mesa-sta...@lists.freedesktop.org
> > Tested-by: Eric Engestrom 
> > ---
> >  src/egl/drivers/dri2/platform_wayland.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/src/egl/drivers/dri2/platform_wayland.c 
> > b/src/egl/drivers/dri2/platform_wayland.c
> > index dca099500a8..a5d43094cf3 100644
> > --- a/src/egl/drivers/dri2/platform_wayland.c
> > +++ b/src/egl/drivers/dri2/platform_wayland.c
> > @@ -258,6 +258,9 @@ dri2_wl_create_window_surface(_EGLDriver *drv, 
> > _EGLDisplay *disp,
> >goto cleanup_surf;
> > }
> >  
> > +   dri2_surf->base.Width = window->width;
> > +   dri2_surf->base.Height = window->height;
> > +
> > visual_idx = dri2_wl_visual_idx_from_config(dri2_dpy, config);
> > assert(visual_idx != -1);
> >  
> > -- 
> > 2.17.1
> > 
> 
> Hi Juan,
> 
> There was a minor conflict when I pulled this into staging/18.1, I'm pretty
> confident that I resolved it correctly, but if you wouldn't mind taking a look
> I'd appreciate it.

For easier lookup, the 18.1 commit is 4395919bd95501a13ba2, and the
rebase looks correct to me too :)

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

Re: [Mesa-dev] [PATCH 14/14] radeonsi: increase the maximum UBO size to 2 GB

[Roland Scheidegger]

Am 09.08.2018 um 17:57 schrieb Marek Olšák:
> On Thu, Aug 9, 2018 at 1:35 AM, Roland Scheidegger  wrote:
>> I'm not quite convinced you can really use huge ubos safely? At least
>> direct addressing in tgsi can't work (you've only got a 16bit register
>> index, and it's signed too).
> 
> UBOs use the LOAD instruction if PIPE_CAP_LOAD_CONSTBUF is supported.
> 
> Marek
> 

Ahh I thought you could still get "ordinary" constant buffer addressing.
But you're right it shouldn't be an issue then. Maybe the sanity stuff
doesn't take this into account (although I wouldn't know why the limit
there would be 4096 exactly).

Roland

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

Re: [Mesa-dev] [PATCH v3 09/48] meson: build getopt when using msvc

[Eric Engestrom]

On Monday, 2018-08-06 17:50:49 -0700, Dylan Baker wrote:
> completely untested
> ---
>  src/getopt/meson.build | 29 +
>  src/meson.build|  5 +
>  2 files changed, 34 insertions(+)
>  create mode 100644 src/getopt/meson.build
> 
> diff --git a/src/getopt/meson.build b/src/getopt/meson.build
> new file mode 100644
> index 000..5e106a6bc60
> --- /dev/null
> +++ b/src/getopt/meson.build
> @@ -0,0 +1,29 @@
> +# Copyright © 2018 Intel Corporation
> +
> +# Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> +# of this software and associated documentation files (the "Software"), to 
> deal
> +# in the Software without restriction, including without limitation the 
> rights
> +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> +# copies of the Software, and to permit persons to whom the Software is
> +# furnished to do so, subject to the following conditions:
> +
> +# The above copyright notice and this permission notice shall be included in
> +# all copies or substantial portions of the Software.
> +
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
> THE
> +# SOFTWARE.
> +
> +libgetopt = static_library(
> +  'getopt',
> +  ['getopt_long.c'],

nit-picky nit: plain 'getopt_long.c' like we do in other single-file places

> +)
> +
> +idep_getopt = declare_dependency(
> +  link_with : libgetopt,
> +  include_directories : include_directories('.', is_system : true),
> +)
> diff --git a/src/meson.build b/src/meson.build
> index 6213b7d8a36..bc0508bce3f 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -47,6 +47,11 @@ sha1_h = custom_target(
>  )
>  
>  subdir('gtest')
> +if cc.get_id() == 'msvc'
> +  subdir('getopt')
> +else
> +  idep_getopt = []

null_dep

Looks reasonable otherwise:
Reviewed-by: Eric Engestrom 

> +endif
>  subdir('util')
>  subdir('mapi')
>  # TODO: opengl
> -- 
> 2.18.0
> 
> ___
> 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] [Mesa-stable] [PATCH v4 1/2] wayland/egl: initialize window surface size to window size

[Dylan Baker]

Quoting Juan A. Suarez Romero (2018-08-07 08:49:36)
> When creating a windows surface with eglCreateWindowSurface(), the
> width and height returned by eglQuerySurface(EGL_{WIDTH,HEIGHT}) is
> invalid until buffers are updated (like calling glClear()).
> 
> But according to EGL 1.5 spec, section 3.5.6 ("Surface Attributes"):
> 
>   "Querying EGL_WIDTH and EGL_HEIGHT returns respectively the width and
>height, in pixels, of the surface. For a window or pixmap surface,
>these values are initially equal to the width and height of the
>native window or pixmap with respect to which the surface was
>created"
> 
> This fixes dEQP-EGL.functional.color_clears.* CTS tests
> 
> v2:
> - Do not modify attached_{width,height} (Daniel)
> - Do not update size on resizing window (Brendan)
> 
> CC: Daniel Stone 
> CC: Brendan King 
> CC: mesa-sta...@lists.freedesktop.org
> Tested-by: Eric Engestrom 
> ---
>  src/egl/drivers/dri2/platform_wayland.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/egl/drivers/dri2/platform_wayland.c 
> b/src/egl/drivers/dri2/platform_wayland.c
> index dca099500a8..a5d43094cf3 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -258,6 +258,9 @@ dri2_wl_create_window_surface(_EGLDriver *drv, 
> _EGLDisplay *disp,
>goto cleanup_surf;
> }
>  
> +   dri2_surf->base.Width = window->width;
> +   dri2_surf->base.Height = window->height;
> +
> visual_idx = dri2_wl_visual_idx_from_config(dri2_dpy, config);
> assert(visual_idx != -1);
>  
> -- 
> 2.17.1
> 

Hi Juan,

There was a minor conflict when I pulled this into staging/18.1, I'm pretty
confident that I resolved it correctly, but if you wouldn't mind taking a look
I'd appreciate it.

Thanks,
Dylan


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

Re: [Mesa-dev] [PATCH 14/14] radeonsi: increase the maximum UBO size to 2 GB

[Marek Olšák]

On Thu, Aug 9, 2018 at 1:35 AM, Roland Scheidegger  wrote:
> I'm not quite convinced you can really use huge ubos safely? At least
> direct addressing in tgsi can't work (you've only got a 16bit register
> index, and it's signed too).

UBOs use the LOAD instruction if PIPE_CAP_LOAD_CONSTBUF is supported.

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

Re: [Mesa-dev] [PATCH 01/14] mesa: remove incorrect change for EXT_disjoint_timer_query

[Marek Olšák]

On Thu, Aug 9, 2018 at 1:47 AM, Tapani Pälli  wrote:
> Hi Marek;
>
> On 08/09/2018 02:55 AM, Marek Olšák wrote:
>>
>> From: Marek Olšák 
>>
>> ---
>>   src/mesa/main/queryobj.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
>> index 7547fa1bb4d..e97a0138e96 100644
>> --- a/src/mesa/main/queryobj.c
>> +++ b/src/mesa/main/queryobj.c
>> @@ -815,22 +815,21 @@ get_query_object(struct gl_context *ctx, const char
>> *func,
>>  if (_mesa_is_gles(ctx) &&
>>  (pname != GL_QUERY_RESULT && pname != GL_QUERY_RESULT_AVAILABLE))
>> {
>> _mesa_error(ctx, GL_INVALID_ENUM, "%s(%s)", func,
>> _mesa_enum_to_string(pname));
>> return;
>>  }
>>if (buf && buf != ctx->Shared->NullBufferObj) {
>> bool is_64bit = ptype == GL_INT64_ARB ||
>>ptype == GL_UNSIGNED_INT64_ARB;
>> -  if (!ctx->Extensions.ARB_query_buffer_object &&
>> -  !ctx->Extensions.EXT_disjoint_timer_query) {
>> +  if (!ctx->Extensions.ARB_query_buffer_object) {
>>_mesa_error(ctx, GL_INVALID_OPERATION, "%s(not supported)",
>> func);
>
>
> Can you explain what was the trouble with this change? I don't recall much
> why this particular change was added but the EXT_disjoint_timer_query spec
> adds support for int64 and uint64 GL types and params to be used in queries.
> I can run some tests to check this.

The conditional is in a block that is entered if a query buffer object
is bound. EXT_disjoint_timer_query doesn't support query buffer
objects. I don't see any connection with 64-bit types there.

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

Re: [Mesa-dev] [PATCH v2] vulkan: simplify VK_USE_PLATFORM_*_KHR handling

[Emil Velikov]

On 9 August 2018 at 15:46, Eric Engestrom  wrote:
> On Tuesday, 2018-08-07 18:17:09 +0100, Emil Velikov wrote:
>> From: Emil Velikov 
>>
>> Instead of having multiple guards littered through the code, simply
>> introduce static inline no-op functions when the respective macros are
>> not set.
>>
>> Inspired by the same convention from the kernel.
>>
>> v2: Also handle PLATFORM_DISPLAY
>>
>> Signed-off-by: Emil Velikov 
>> Reviewed-by: Eric Engestrom  (v1)
>> ---
>>  src/vulkan/wsi/wsi_common.c | 12 
>>  src/vulkan/wsi/wsi_common_private.h | 47 +
>>  2 files changed, 47 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
>> index f2d90a6bba2..d2ba7871a1d 100644
>> --- a/src/vulkan/wsi/wsi_common.c
>> +++ b/src/vulkan/wsi/wsi_common.c
>> @@ -80,23 +80,17 @@ wsi_device_init(struct wsi_device *wsi,
>> WSI_GET_CB(WaitForFences);
>>  #undef WSI_GET_CB
>>
>> -#ifdef VK_USE_PLATFORM_XCB_KHR
>> result = wsi_x11_init_wsi(wsi, alloc);
>> if (result != VK_SUCCESS)
>>goto fail;
>> -#endif
>>
>> -#ifdef VK_USE_PLATFORM_WAYLAND_KHR
>> result = wsi_wl_init_wsi(wsi, alloc, pdevice);
>> if (result != VK_SUCCESS)
>>goto fail;
>> -#endif
>>
>> -#ifdef VK_USE_PLATFORM_DISPLAY_KHR
>> result = wsi_display_init_wsi(wsi, alloc, display_fd);
>> if (result != VK_SUCCESS)
>>goto fail;
>> -#endif
>>
>> return VK_SUCCESS;
>>
>> @@ -109,15 +103,9 @@ void
>>  wsi_device_finish(struct wsi_device *wsi,
>>const VkAllocationCallbacks *alloc)
>>  {
>> -#ifdef VK_USE_PLATFORM_DISPLAY_KHR
>> wsi_display_finish_wsi(wsi, alloc);
>> -#endif
>> -#ifdef VK_USE_PLATFORM_WAYLAND_KHR
>> wsi_wl_finish_wsi(wsi, alloc);
>> -#endif
>> -#ifdef VK_USE_PLATFORM_XCB_KHR
>> wsi_x11_finish_wsi(wsi, alloc);
>> -#endif
>>  }
>>
>>  VkResult
>> diff --git a/src/vulkan/wsi/wsi_common_private.h 
>> b/src/vulkan/wsi/wsi_common_private.h
>> index 9f2aacd6560..7dc1554e38d 100644
>> --- a/src/vulkan/wsi/wsi_common_private.h
>> +++ b/src/vulkan/wsi/wsi_common_private.h
>> @@ -128,17 +128,49 @@ struct wsi_interface {
>>  struct wsi_swapchain **swapchain);
>>  };
>>
>> +#ifdef VK_USE_PLATFORM_XCB_KHR
>>  VkResult wsi_x11_init_wsi(struct wsi_device *wsi_device,
>>const VkAllocationCallbacks *alloc);
>>  void wsi_x11_finish_wsi(struct wsi_device *wsi_device,
>>  const VkAllocationCallbacks *alloc);
>> +#else
>> +static inline
>> +VkResult wsi_x11_init_wsi(struct wsi_device *wsi_device,
>> +  const VkAllocationCallbacks *alloc)
>> +{
>> +   return VK_SUCCESS;
>> +}
>> +
>> +static inline
>> +void wsi_x11_finish_wsi(struct wsi_device *wsi_device,
>> +const VkAllocationCallbacks *alloc)
>> +{
>> +}
>> +#endif
>> +
>> +#ifdef VK_USE_PLATFORM_WAYLAND_KHR
>>  VkResult wsi_wl_init_wsi(struct wsi_device *wsi_device,
>>   const VkAllocationCallbacks *alloc,
>>   VkPhysicalDevice physical_device);
>>  void wsi_wl_finish_wsi(struct wsi_device *wsi_device,
>> const VkAllocationCallbacks *alloc);
>> +#else
>> +static inline
>> +VkResult wsi_wl_init_wsi(struct wsi_device *wsi_device,
>> + const VkAllocationCallbacks *alloc,
>> + VkPhysicalDevice physical_device)
>> +{
>> +   return VK_SUCCESS;
>> +}
>>
>> +static inline
>> +void wsi_wl_finish_wsi(struct wsi_device *wsi_device,
>> +   const VkAllocationCallbacks *alloc)
>> +{
>> +}
>> +#endif
>>
>> +#ifdef VK_USE_PLATFORM_DISPLAY_KHR
>>  VkResult
>>  wsi_display_init_wsi(struct wsi_device *wsi_device,
>>   const VkAllocationCallbacks *alloc,
>> @@ -147,6 +179,21 @@ wsi_display_init_wsi(struct wsi_device *wsi_device,
>>  void
>>  wsi_display_finish_wsi(struct wsi_device *wsi_device,
>> const VkAllocationCallbacks *alloc);
>> +#else
>> +static inline VkResult
>> +wsi_display_init_wsi(struct wsi_device *wsi_device,
>
> To be consistent, `VkResult` (and `void` in the finish one below) should
> be on the same line as the function name.
>
Just following existing wsi_display_init_wsi declaration.

> That said, this is actually much more code and more complicated than the
> current state, and let's be honest: nobody will add another wsi_device_init()
> or anything else that would add a second caller for any of these, so
> I actually retract my r-b: I think it's better as it currently is :)
>
Ack. It is fairly bike-sheddy solution, so I'm glad it's settled
sooner than later ;-)

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

Re: [Mesa-dev] [PATCH 10/14] tgsi/ureg: don't call tgsi_sanity when it's too slow

[Marek Olšák]

On Thu, Aug 9, 2018 at 1:28 AM, Roland Scheidegger  wrote:
> Am 09.08.2018 um 01:55 schrieb Marek Olšák:
>> From: Marek Olšák 
>>
>> ---
>>  src/gallium/auxiliary/tgsi/tgsi_ureg.c | 13 -
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.c 
>> b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> index 92c98c763eb..c1c8851486e 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> @@ -2099,21 +2099,32 @@ const struct tgsi_token *ureg_finalize( struct 
>> ureg_program *ureg )
>>
>> tokens = >domain[DOMAIN_DECL].tokens[0].token;
>>
>> if (0) {
>>debug_printf("%s: emitted shader %d tokens:\n", __FUNCTION__,
>> ureg->domain[DOMAIN_DECL].count);
>>tgsi_dump( tokens, 0 );
>> }
>>
>>  #if DEBUG
>> -   if (tokens && !tgsi_sanity_check(tokens)) {
>> +   /* tgsi_sanity doesn't seem to return if there are too many constants. */
>> +   bool too_many_constants = false;
>> +   for (unsigned i = 0; i < ARRAY_SIZE(ureg->const_decls); i++) {
>> +  for (unsigned j = 0; j < ureg->const_decls[i].nr_constant_ranges; 
>> j++) {
>> + if (ureg->const_decls[i].constant_range[j].last > 4096) {
>> +too_many_constants = true;
>> +break;
>> + }
>> +  }
>> +   }
> Err, is it actually too slow, is there a bug in sanity checking or are you
> making it pass sanity but tgsi emitting garbage?

I don't know. It's possible that TGSI doesn't like when there are too
many CONST declarations, perhaps too many to fit into the declaration
and source operand tokens. On the other hand, we have
PIPE_CAP_LOAD_CONSTBUF, which switches glsl_to_tgsi to use LOAD, so
the CONST limit doesn't apply. It's possible that TGSI is indeed
incorrect with regard to CONST file usage, but I don't care as long as
piglit tests pass for large UBOs.

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

Re: [Mesa-dev] [PATCH v3 08/48] meson: fix dl detection on non cygwin windows

[Eric Engestrom]

On Monday, 2018-08-06 17:50:48 -0700, Dylan Baker wrote:
> ---
>  meson.build | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index c7dd5ddfec6..788021c05e9 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1027,9 +1027,9 @@ endif
>  if cc.has_function('dlopen')

This check is not needed on windows, is it? It will always fail afaict.

How about this?
  if host == windows or cc.has_function(dlopen)

>dep_dl = null_dep
>  else
> -  dep_dl = cc.find_library('dl')
> +  dep_dl = cc.find_library('dl', required : host_machine.system() != 
> 'windows')
>  endif
> -if cc.has_function('dladdr', dependencies : dep_dl)
> +if host_machine.system() != 'windows' and cc.has_function('dladdr', 
> dependencies : dep_dl)
># This is really only required for megadrivers
>pre_args += '-DHAVE_DLADDR'
>  endif

With that, 1-4 and 7-8 are:
Reviewed-by: Eric Engestrom 

> -- 
> 2.18.0
> 
> ___
> 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] Gallium/tgsi: Correct signdness of return value of bit operations

[Roland Scheidegger]

This is incorrect for umsb.
If you really want to move so the dst type is signed, you need to add it
to infer_src_type so the src args are unsigned.
FWIW I'd like to think that all 3 of these always operate on unsigned
data (even if they might return signed data), imho they are a lot more
easier to understand that way (I mean the signed msb you have to look up
what the hell it actually does, bit operations on unsigned data are just
easier to understand). But of course it doesn't really make a
difference, so I can live with the other 2 if the src type indicates signed.

Roland


Am 09.08.2018 um 10:43 schrieb Gert Wollny:
> The GLSL functions  findLSB, findMSB, and countBits always return
> a signed integer type. Let the according TGSI operations reflect this.
> 
> Signed-off-by: Gert Wollny 
> ---
>  src/gallium/auxiliary/tgsi/tgsi_info.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c 
> b/src/gallium/auxiliary/tgsi/tgsi_info.c
> index bbe1a21e43..a8c2bbe663 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
> @@ -150,9 +150,6 @@ tgsi_opcode_infer_type(enum tgsi_opcode opcode)
> case TGSI_OPCODE_UBFE:
> case TGSI_OPCODE_BFI:
> case TGSI_OPCODE_BREV:
> -   case TGSI_OPCODE_POPC:
> -   case TGSI_OPCODE_LSB:
> -   case TGSI_OPCODE_UMSB:
> case TGSI_OPCODE_IMG2HND:
> case TGSI_OPCODE_SAMP2HND:
>return TGSI_TYPE_UNSIGNED;
> @@ -188,6 +185,9 @@ tgsi_opcode_infer_type(enum tgsi_opcode opcode)
> case TGSI_OPCODE_U64SGE:
> case TGSI_OPCODE_I64SLT:
> case TGSI_OPCODE_I64SGE:
> +   case TGSI_OPCODE_LSB:
> +   case TGSI_OPCODE_POPC:
> +   case TGSI_OPCODE_UMSB:
>return TGSI_TYPE_SIGNED;
> case TGSI_OPCODE_DADD:
> case TGSI_OPCODE_DABS:
> 

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

Re: [Mesa-dev] [PATCH] egl: Fix leak of X11 pixmaps backing pbuffers in DRI3.

[Eric Engestrom]

On Tuesday, 2018-08-07 12:32:04 -0700, Eric Anholt wrote:
> This is basically copied from the DRI2 destroy path.  Without this,
> Raspberry Pi would quickly run out of CMA during the EGL tests in the CTS
> due to all the pixmaps laying around.
> 
> Fixes: f35198badeb9 ("egl/x11: Implement dri3 support with loader's dri3 
> helper")

Good catch, you're right!
Reviewed-by: Eric Engestrom 

> ---
>  src/egl/drivers/dri2/platform_x11_dri3.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/egl/drivers/dri2/platform_x11_dri3.c 
> b/src/egl/drivers/dri2/platform_x11_dri3.c
> index c3c9c2dd45d6..e1967422f0a8 100644
> --- a/src/egl/drivers/dri2/platform_x11_dri3.c
> +++ b/src/egl/drivers/dri2/platform_x11_dri3.c
> @@ -107,12 +107,17 @@ static const struct loader_dri3_vtable egl_dri3_vtable 
> = {
>  static EGLBoolean
>  dri3_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
>  {
> +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> struct dri3_egl_surface *dri3_surf = dri3_egl_surface(surf);
> +   xcb_drawable_t drawable = dri3_surf->loader_drawable.drawable;
>  
> (void) drv;
>  
> loader_dri3_drawable_fini(_surf->loader_drawable);
>  
> +   if (surf->Type == EGL_PBUFFER_BIT)
> +  xcb_free_pixmap (dri2_dpy->conn, drawable);
> +
> dri2_fini_surface(surf);
> free(surf);
>  
> -- 
> 2.18.0
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] vulkan: simplify VK_USE_PLATFORM_*_KHR handling

[Eric Engestrom]

On Tuesday, 2018-08-07 18:17:09 +0100, Emil Velikov wrote:
> From: Emil Velikov 
> 
> Instead of having multiple guards littered through the code, simply
> introduce static inline no-op functions when the respective macros are
> not set.
> 
> Inspired by the same convention from the kernel.
> 
> v2: Also handle PLATFORM_DISPLAY
> 
> Signed-off-by: Emil Velikov 
> Reviewed-by: Eric Engestrom  (v1)
> ---
>  src/vulkan/wsi/wsi_common.c | 12 
>  src/vulkan/wsi/wsi_common_private.h | 47 +
>  2 files changed, 47 insertions(+), 12 deletions(-)
> 
> diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
> index f2d90a6bba2..d2ba7871a1d 100644
> --- a/src/vulkan/wsi/wsi_common.c
> +++ b/src/vulkan/wsi/wsi_common.c
> @@ -80,23 +80,17 @@ wsi_device_init(struct wsi_device *wsi,
> WSI_GET_CB(WaitForFences);
>  #undef WSI_GET_CB
>  
> -#ifdef VK_USE_PLATFORM_XCB_KHR
> result = wsi_x11_init_wsi(wsi, alloc);
> if (result != VK_SUCCESS)
>goto fail;
> -#endif
>  
> -#ifdef VK_USE_PLATFORM_WAYLAND_KHR
> result = wsi_wl_init_wsi(wsi, alloc, pdevice);
> if (result != VK_SUCCESS)
>goto fail;
> -#endif
>  
> -#ifdef VK_USE_PLATFORM_DISPLAY_KHR
> result = wsi_display_init_wsi(wsi, alloc, display_fd);
> if (result != VK_SUCCESS)
>goto fail;
> -#endif
>  
> return VK_SUCCESS;
>  
> @@ -109,15 +103,9 @@ void
>  wsi_device_finish(struct wsi_device *wsi,
>const VkAllocationCallbacks *alloc)
>  {
> -#ifdef VK_USE_PLATFORM_DISPLAY_KHR
> wsi_display_finish_wsi(wsi, alloc);
> -#endif
> -#ifdef VK_USE_PLATFORM_WAYLAND_KHR
> wsi_wl_finish_wsi(wsi, alloc);
> -#endif
> -#ifdef VK_USE_PLATFORM_XCB_KHR
> wsi_x11_finish_wsi(wsi, alloc);
> -#endif
>  }
>  
>  VkResult
> diff --git a/src/vulkan/wsi/wsi_common_private.h 
> b/src/vulkan/wsi/wsi_common_private.h
> index 9f2aacd6560..7dc1554e38d 100644
> --- a/src/vulkan/wsi/wsi_common_private.h
> +++ b/src/vulkan/wsi/wsi_common_private.h
> @@ -128,17 +128,49 @@ struct wsi_interface {
>  struct wsi_swapchain **swapchain);
>  };
>  
> +#ifdef VK_USE_PLATFORM_XCB_KHR
>  VkResult wsi_x11_init_wsi(struct wsi_device *wsi_device,
>const VkAllocationCallbacks *alloc);
>  void wsi_x11_finish_wsi(struct wsi_device *wsi_device,
>  const VkAllocationCallbacks *alloc);
> +#else
> +static inline
> +VkResult wsi_x11_init_wsi(struct wsi_device *wsi_device,
> +  const VkAllocationCallbacks *alloc)
> +{
> +   return VK_SUCCESS;
> +}
> +
> +static inline
> +void wsi_x11_finish_wsi(struct wsi_device *wsi_device,
> +const VkAllocationCallbacks *alloc)
> +{
> +}
> +#endif
> +
> +#ifdef VK_USE_PLATFORM_WAYLAND_KHR
>  VkResult wsi_wl_init_wsi(struct wsi_device *wsi_device,
>   const VkAllocationCallbacks *alloc,
>   VkPhysicalDevice physical_device);
>  void wsi_wl_finish_wsi(struct wsi_device *wsi_device,
> const VkAllocationCallbacks *alloc);
> +#else
> +static inline
> +VkResult wsi_wl_init_wsi(struct wsi_device *wsi_device,
> + const VkAllocationCallbacks *alloc,
> + VkPhysicalDevice physical_device)
> +{
> +   return VK_SUCCESS;
> +}
>  
> +static inline
> +void wsi_wl_finish_wsi(struct wsi_device *wsi_device,
> +   const VkAllocationCallbacks *alloc)
> +{
> +}
> +#endif
>  
> +#ifdef VK_USE_PLATFORM_DISPLAY_KHR
>  VkResult
>  wsi_display_init_wsi(struct wsi_device *wsi_device,
>   const VkAllocationCallbacks *alloc,
> @@ -147,6 +179,21 @@ wsi_display_init_wsi(struct wsi_device *wsi_device,
>  void
>  wsi_display_finish_wsi(struct wsi_device *wsi_device,
> const VkAllocationCallbacks *alloc);
> +#else
> +static inline VkResult
> +wsi_display_init_wsi(struct wsi_device *wsi_device,

To be consistent, `VkResult` (and `void` in the finish one below) should
be on the same line as the function name.

That said, this is actually much more code and more complicated than the
current state, and let's be honest: nobody will add another wsi_device_init()
or anything else that would add a second caller for any of these, so
I actually retract my r-b: I think it's better as it currently is :)

> + const VkAllocationCallbacks *alloc,
> + int display_fd)
> +{
> +   return VK_SUCCESS;
> +}
> +
> +static inline void
> +wsi_display_finish_wsi(struct wsi_device *wsi_device,
> +   const VkAllocationCallbacks *alloc)
> +{
> +}
> +#endif
>  
>  #define WSI_DEFINE_NONDISP_HANDLE_CASTS(__wsi_type, __VkType)  \
> \
> -- 
> 2.18.0
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> 

[Mesa-dev] [Bug 107533] Please restore --with-{gl, osmesa}-lib-name options

[bugzilla-daemon]

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

Brad King  changed:

   What|Removed |Added

 Resolution|FIXED   |WONTFIX

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

[Mesa-dev] [Bug 107533] Please restore --with-{gl, osmesa}-lib-name options

[bugzilla-daemon]

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

Brad King  changed:

   What|Removed |Added

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

--- Comment #3 from Brad King  ---
> an arbitrary name many have worked in a particular combination

Yes, I use:

./autogen.sh \
  --prefix="$prefix" \
  --disable-dri \
  --disable-egl \
  --disable-gbm \
  --disable-gles1 \
  --disable-gles2 \
  --disable-shared-glapi \
  --with-platforms=x11 \
  --enable-glx=gallium-xlib \
  --enable-gallium-osmesa \
  --with-gallium-drivers=swrast \
  --enable-gallium-llvm=yes \
LLVM_CONFIG=$llvm_config \
  --enable-llvm-shared-libs \
  --with-gl-lib-name=$gl_name \
  --with-osmesa-lib-name=$osmesa_name

> but the rest were broken

Ah, okay.

> is a rm && patchelf, post-install.

In principle 'lib/pkgconfig/*.pc' and 'lib/*.la' need to be updated
too but I don't need those for my use case.  I've updated my scripts
to rename post-install, patchelf, and revise symlinks.

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

Re: [Mesa-dev] [PATCH v2 7/9] python: Simplify list sorting

[Eric Engestrom]

On Thursday, 2018-08-09 10:27:24 +0200, Mathieu Bridon wrote:
> Instead of copying the list, then sorting the copy in-place, we can just
> get a new sorted copy directly.
> 
> Signed-off-by: Mathieu Bridon 

4-7 are
Reviewed-by: Eric Engestrom 

> ---
>  src/mapi/mapi_abi.py | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mapi/mapi_abi.py b/src/mapi/mapi_abi.py
> index d4c48ec430..dc48fa5935 100644
> --- a/src/mapi/mapi_abi.py
> +++ b/src/mapi/mapi_abi.py
> @@ -291,8 +291,7 @@ class ABIPrinter(object):
>  self.entries = entries
>  
>  # sort entries by their names
> -self.entries_sorted_by_names = self.entries[:]
> -self.entries_sorted_by_names.sort(key=attrgetter('name'))
> +self.entries_sorted_by_names = sorted(self.entries, 
> key=attrgetter('name'))
>  
>  self.indent = ' ' * 3
>  self.noop_warn = 'noop_warn'
> @@ -441,8 +440,7 @@ class ABIPrinter(object):
>  def c_stub_string_pool(self):
>  """Return the string pool for use by stubs."""
>  # sort entries by their names
> -sorted_entries = self.entries[:]
> -sorted_entries.sort(key=attrgetter('name'))
> +sorted_entries = sorted(self.entries, key=attrgetter('name'))
>  
>  pool = []
>  offsets = {}
> -- 
> 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 07/10] mesa/glspirv: compute double inputs and remap attributes

[Alejandro Piñeiro]

input locations used by input attributes are not handled in the same
way in OpenGL vs Vulkan. There is a detailed explanation of such
differences on the following commit:

c2acf97fcc9b32eaa9778771282758e5652a8ad4

So with this commit, the same adjustment that is done after
glsl_to_nir, is being done after spirv_to_nir, when it is used on
OpenGL (ARB_gl_spirv).
---
 src/mesa/main/glspirv.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 87075a547cd..7af73efd589 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -182,6 +182,20 @@ _mesa_spirv_link_shaders(struct gl_context *ctx, struct 
gl_shader_program *prog)
   prog->last_vert_prog = prog->_LinkedShaders[last_vert_stage - 
1]->Program;
 }
 
+static void
+nir_compute_double_inputs(nir_shader *shader,
+  const nir_shader_compiler_options *options)
+{
+   nir_foreach_variable(var, >inputs) {
+  if (glsl_type_is_dual_slot(glsl_without_array(var->type))) {
+ for (uint i = 0; i < glsl_count_attribute_slots(var->type, true); 
i++) {
+uint64_t bitfield = BITFIELD64_BIT(var->data.location + i);
+shader->info.vs.double_inputs |= bitfield;
+ }
+  }
+   }
+}
+
 nir_shader *
 _mesa_spirv_to_nir(struct gl_context *ctx,
const struct gl_shader_program *prog,
@@ -246,6 +260,11 @@ _mesa_spirv_to_nir(struct gl_context *ctx,
NIR_PASS_V(nir, nir_split_var_copies);
NIR_PASS_V(nir, nir_split_per_member_structs);
 
+   if (nir->info.stage == MESA_SHADER_VERTEX) {
+  nir_compute_double_inputs(nir, options);
+  nir_remap_attributes(nir, options);
+   }
+
return nir;
 }
 
-- 
2.14.1

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

[Mesa-dev] [PATCH 09/10] mesa/glspirv: Set separate_shader on shader_info

[Alejandro Piñeiro]

From: Neil Roberts 

The value is copied from the gl_program. If we don’t do this then it
will get reset back to zero in brw_shader_gather_info. This isn’t a
problem for GLSL because in that case the nir_shader is initialised
with a copy of the shader_info from the gl_program.
---
 src/mesa/main/glspirv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 5a2d6a4bb2a..4fc80b72181 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -252,6 +252,8 @@ _mesa_spirv_to_nir(struct gl_context *ctx,
   prog->Name);
nir_validate_shader(nir);
 
+   nir->info.separate_shader = linked_shader->Program->info.separate_shader;
+
/* We have to lower away local constant initializers right before we
 * inline functions.  That way they get properly initialized at the top
 * of the function and not at the top of its caller.
-- 
2.14.1

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

[Mesa-dev] [PATCH 08/10] mesa/glspirv: pick off the only entry point we need

[Alejandro Piñeiro]

From: Iago Toral Quiroga 

This is the same we do for vulkan drivers

This is needed to pass the following CTS test:
KHR-GL45.gl_spirv.spirv_modules_shader_binary_multiple_shader_objects_test
---
 src/mesa/main/glspirv.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index 7af73efd589..5a2d6a4bb2a 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -252,8 +252,23 @@ _mesa_spirv_to_nir(struct gl_context *ctx,
   prog->Name);
nir_validate_shader(nir);
 
+   /* We have to lower away local constant initializers right before we
+* inline functions.  That way they get properly initialized at the top
+* of the function and not at the top of its caller.
+*/
+   NIR_PASS_V(nir, nir_lower_constant_initializers, nir_var_local);
+   NIR_PASS_V(nir, nir_lower_returns);
+   NIR_PASS_V(nir, nir_inline_functions);
NIR_PASS_V(nir, nir_copy_prop);
 
+   /* Pick off the single entrypoint that we want */
+   foreach_list_typed_safe(nir_function, func, node, >functions) {
+  if (func != entry_point)
+ exec_node_remove(>node);
+   }
+   assert(exec_list_length(>functions) == 1);
+   entry_point->name = ralloc_strdup(entry_point, "main");
+
/* Split member structs.  We do this before lower_io_to_temporaries so that
 * it doesn't lower system values to temporaries by accident.
 */
-- 
2.14.1

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

[Mesa-dev] [PATCH 04/10] nir/linker: take into account hidden uniforms

[Alejandro Piñeiro]

So they are not exposed through the introspection API.

It is worth to note that the number of hidden uniforms of GLSL linking
vs SPIR-V linking would be somewhat different due the differen order
of the nir lowerings/optimizations.

For example: gl_FbWposYTransform. This is introduced as part of
nir_lower_wpos_ytransform. On GLSL that is executed after the IR-based
linking. So that means that on GLSL the UniformStorage will not
include this uniform. With the SPIR-V linking, that uniform is already
present, but marked as hidden. So it will be included on the
UniformStorage, but as hidden.

One alternative would create a special how_declared for that case, but
seemed an overkill. Using hidden should be ok as far as it is used
properly.
---
 src/compiler/glsl/gl_nir_link_uniforms.c | 5 -
 src/compiler/glsl/gl_nir_linker.c| 4 
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/compiler/glsl/gl_nir_link_uniforms.c 
b/src/compiler/glsl/gl_nir_link_uniforms.c
index f729fa988e2..1573e30c41e 100644
--- a/src/compiler/glsl/gl_nir_link_uniforms.c
+++ b/src/compiler/glsl/gl_nir_link_uniforms.c
@@ -365,6 +365,10 @@ nir_link_uniform(struct gl_context *ctx,
  uniform->remap_location = UNMAPPED_UNIFORM_LOC;
   }
 
+  uniform->hidden = state->current_var->data.how_declared == 
nir_var_hidden;
+  if (uniform->hidden)
+ state->num_hidden_uniforms++;
+
   /* @FIXME: the initialization of the following will be done as we
* implement support for their specific features, like SSBO, atomics,
* etc.
@@ -374,7 +378,6 @@ nir_link_uniform(struct gl_context *ctx,
   uniform->matrix_stride = -1;
   uniform->array_stride = -1;
   uniform->row_major = false;
-  uniform->hidden = false;
   uniform->builtin = false;
   uniform->is_shader_storage = false;
   uniform->atomic_buffer_index = -1;
diff --git a/src/compiler/glsl/gl_nir_linker.c 
b/src/compiler/glsl/gl_nir_linker.c
index d09a2c0a6c5..547549bc4e0 100644
--- a/src/compiler/glsl/gl_nir_linker.c
+++ b/src/compiler/glsl/gl_nir_linker.c
@@ -56,6 +56,10 @@ nir_build_program_resource_list(struct gl_context *ctx,
for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
   struct gl_uniform_storage *uniform = >data->UniformStorage[i];
 
+  /* Do not add uniforms internally used by Mesa. */
+  if (uniform->hidden)
+ continue;
+
   if (!link_util_add_program_resource(prog, resource_set, GL_UNIFORM, 
uniform,
   uniform->active_shader_mask)) {
  return;
-- 
2.14.1

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

[Mesa-dev] [PATCH 10/10] i965/nir: Use the nir copy of shader_info to handle gl_PatchVerticesIn

[Alejandro Piñeiro]

From: Neil Roberts 

Instead of using the copy of shader_info stored in gl_program, it now
uses the one in nir_shader. This is needed for SPIR-V because the
info.tess.tcs_vertices_out is filled in via _mesa_spirv_to_nir which
happens much later than with a GLSL shader. The copy of shader_data in
gl_program is only updated later via brw_shader_gather_info but that
is too late.

For GLSL this shouldn't create any problems because the nir copy of
the shader_info is immediately copied from the gl_program in
glsl_to_nir.

v2: updated after commit "i965: Combine both gl_PatchVerticesIn
lowering passes." (488972) (Alejandro Piñeiro)
---
 src/mesa/drivers/dri/i965/brw_program.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_program.c 
b/src/mesa/drivers/dri/i965/brw_program.c
index 7adb75d0eaa..a669814d0d2 100644
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -116,7 +116,7 @@ brw_create_nir(struct brw_context *brw,
   struct gl_linked_shader *tcs =
  shader_prog->_LinkedShaders[MESA_SHADER_TESS_CTRL];
   uint32_t static_patch_vertices =
- tcs ? tcs->Program->info.tess.tcs_vertices_out : 0;
+ tcs ? tcs->Program->nir->info.tess.tcs_vertices_out : 0;
   static const gl_state_index16 tokens[STATE_LENGTH] =
  { STATE_INTERNAL, STATE_TES_PATCH_VERTICES_IN };
   nir_lower_patch_vertices(nir, static_patch_vertices, tokens);
-- 
2.14.1

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

[Mesa-dev] [PATCH 06/10] nir/glsl: make nir_remap_attributes public

[Alejandro Piñeiro]

As we plan to reuse it for ARB_gl_spirv implementation.
---
 src/compiler/glsl/glsl_to_nir.cpp | 17 -
 src/compiler/nir/nir.c| 24 
 src/compiler/nir/nir.h|  3 +++
 3 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/src/compiler/glsl/glsl_to_nir.cpp 
b/src/compiler/glsl/glsl_to_nir.cpp
index 1e4d9f9d3c8..b1d1da89111 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -128,23 +128,6 @@ private:
 
 } /* end of anonymous namespace */
 
-static void
-nir_remap_attributes(nir_shader *shader,
- const nir_shader_compiler_options *options)
-{
-   if (options->vs_inputs_dual_locations) {
-  nir_foreach_variable(var, >inputs) {
- var->data.location +=
-_mesa_bitcount_64(shader->info.vs.double_inputs &
-  BITFIELD64_MASK(var->data.location));
-  }
-   }
-
-   /* Once the remap is done, reset double_inputs_read, so later it will have
-* which location/slots are doubles */
-   shader->info.vs.double_inputs = 0;
-}
-
 nir_shader *
 glsl_to_nir(const struct gl_shader_program *shader_prog,
 gl_shader_stage stage,
diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
index 5e76654ca3d..e12aa5d80f5 100644
--- a/src/compiler/nir/nir.c
+++ b/src/compiler/nir/nir.c
@@ -32,6 +32,9 @@
 #include 
 #include 
 
+#include "main/imports.h" /* _mesa_bitcount_64 */
+#include "main/menums.h" /* BITFIELD64_MASK */
+
 nir_shader *
 nir_shader_create(void *mem_ctx,
   gl_shader_stage stage,
@@ -1847,3 +1850,24 @@ nir_system_value_from_intrinsic(nir_intrinsic_op intrin)
   unreachable("intrinsic doesn't produce a system value");
}
 }
+
+/* OpenGL utility method that remaps the location attributes if they are
+ * doubles. Not needed for vulkan due the differences on the input location
+ * count for doubles on vulkan vs OpenGL
+ */
+void
+nir_remap_attributes(nir_shader *shader,
+ const nir_shader_compiler_options *options)
+{
+   if (options->vs_inputs_dual_locations) {
+  nir_foreach_variable(var, >inputs) {
+ var->data.location +=
+_mesa_bitcount_64(shader->info.vs.double_inputs &
+  BITFIELD64_MASK(var->data.location));
+  }
+   }
+
+   /* Once the remap is done, reset double_inputs_read, so later it will have
+* which location/slots are doubles */
+   shader->info.vs.double_inputs = 0;
+}
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index 1ccbccc8bbb..d0fa693884b 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -2999,6 +2999,9 @@ bool nir_opt_conditional_discard(nir_shader *shader);
 
 void nir_sweep(nir_shader *shader);
 
+void nir_remap_attributes(nir_shader *shader,
+  const nir_shader_compiler_options *options);
+
 nir_intrinsic_op nir_intrinsic_from_system_value(gl_system_value val);
 gl_system_value nir_system_value_from_intrinsic(nir_intrinsic_op intrin);
 
-- 
2.14.1

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

[Mesa-dev] [PATCH 05/10] nir/lower_samplers: don't assume a deref for both texture and sampler srcs

[Alejandro Piñeiro]

After commit "nir: Use derefs in nir_lower_samplers"
(75286c2d083cdbdfb202a93349e567df0441d5f7) assumes one deref for both
the texture and the sampler. However there are cases (on OpenGL, using
ARB_gl_spirv) where SPIR-V is not providing a sampler, like for
texture query levels ops. Although we could make spirv_to_nir to
provide a sampler deref for those cases, it is not really needed, and
wrong from the Vulkan point of view.

This patch fixes the following (borrowed) tests run on SPIR-V mode:
  arb_compute_shader/execution/basic-texelFetch.shader_test
  
arb_gpu_shader5/execution/sampler_array_indexing/fs-simple-texture-size.shader_test
  arb_texture_query_levels/execution/fs-baselevel.shader_test
  arb_texture_query_levels/execution/fs-maxlevel.shader_test
  arb_texture_query_levels/execution/fs-miptree.shader_test
  arb_texture_query_levels/execution/fs-nomips.shader_test
  arb_texture_query_levels/execution/vs-baselevel.shader_test
  arb_texture_query_levels/execution/vs-maxlevel.shader_test
  arb_texture_query_levels/execution/vs-miptree.shader_test
  arb_texture_query_levels/execution/vs-nomips.shader_test
  glsl-1.30/execution/fs-textureSize-compare.shader_test
---
 src/compiler/glsl/gl_nir_lower_samplers.c | 83 ---
 1 file changed, 55 insertions(+), 28 deletions(-)

diff --git a/src/compiler/glsl/gl_nir_lower_samplers.c 
b/src/compiler/glsl/gl_nir_lower_samplers.c
index 43fe318a835..1b50b10d345 100644
--- a/src/compiler/glsl/gl_nir_lower_samplers.c
+++ b/src/compiler/glsl/gl_nir_lower_samplers.c
@@ -103,48 +103,75 @@ calc_sampler_offsets(nir_builder *b, nir_ssa_def *ptr,
   shader_program->data->UniformStorage[location].opaque[stage].index;
 }
 
+static void
+lower_tex_src_to_offset(nir_builder *b,
+nir_tex_instr *instr, unsigned src_idx,
+unsigned *index, unsigned *array_size,
+const struct gl_shader_program *shader_program)
+{
+   nir_ssa_def *indirect;
+   unsigned base_offset, array_elements;
+   nir_tex_src *src = >src[src_idx];
+   bool is_sampler = src->src_type == nir_tex_src_sampler_deref;
+
+   calc_sampler_offsets(b, src->src.ssa, shader_program, _offset,
+, _elements);
+   if (indirect) {
+  nir_instr_rewrite_src(>instr, >src,
+nir_src_for_ssa(indirect));
+
+  src->src_type = is_sampler ?
+ nir_tex_src_sampler_offset :
+ nir_tex_src_texture_offset;
+
+  instr->texture_array_size = array_elements;
+   } else {
+  nir_tex_instr_remove_src(instr, src_idx);
+   }
+
+   if (index)
+  *index = base_offset;
+
+   if (array_size)
+  *array_size = array_elements;
+}
+
 static bool
 lower_sampler(nir_builder *b, nir_tex_instr *instr,
   const struct gl_shader_program *shader_program)
 {
int texture_idx =
   nir_tex_instr_src_index(instr, nir_tex_src_texture_deref);
-   int sampler_idx =
-  nir_tex_instr_src_index(instr, nir_tex_src_sampler_deref);
 
-   if (texture_idx < 0)
-  return false;
+   if (texture_idx >= 0) {
+  unsigned texture_index;
+  unsigned texture_array_size;
 
-   assert(texture_idx >= 0 && sampler_idx >= 0);
-   assert(instr->src[texture_idx].src.is_ssa);
-   assert(instr->src[sampler_idx].src.is_ssa);
-   assert(instr->src[texture_idx].src.ssa == instr->src[sampler_idx].src.ssa);
+  b->cursor = nir_before_instr(>instr);
 
-   b->cursor = nir_before_instr(>instr);
+  lower_tex_src_to_offset(b, instr, texture_idx,
+  _index, _array_size,
+  shader_program);
 
-   unsigned base_offset, array_elements;
-   nir_ssa_def *indirect;
-   calc_sampler_offsets(b, instr->src[texture_idx].src.ssa, shader_program,
-_offset, , _elements);
+  instr->texture_index = texture_index;
+  instr->texture_array_size = texture_array_size;
+   }
 
-   instr->texture_index = base_offset;
-   instr->sampler_index = base_offset;
-   if (indirect) {
-  nir_instr_rewrite_src(>instr, >src[texture_idx].src,
-nir_src_for_ssa(indirect));
-  instr->src[texture_idx].src_type = nir_tex_src_texture_offset;
-  nir_instr_rewrite_src(>instr, >src[sampler_idx].src,
-nir_src_for_ssa(indirect));
-  instr->src[sampler_idx].src_type = nir_tex_src_sampler_offset;
+   int sampler_idx =
+  nir_tex_instr_src_index(instr, nir_tex_src_sampler_deref);
 
-  instr->texture_array_size = array_elements;
-   } else {
-  nir_tex_instr_remove_src(instr, texture_idx);
-  /* The sampler index may have changed */
-  sampler_idx = nir_tex_instr_src_index(instr, nir_tex_src_sampler_deref);
-  nir_tex_instr_remove_src(instr, sampler_idx);
+   if (sampler_idx >= 0) {
+  unsigned sampler_index;
+
+  lower_tex_src_to_offset(b, instr, sampler_idx,
+  _index, NULL,
+  

[Mesa-dev] [PATCH 03/10] nir: add how_declared to nir_variable.data

[Alejandro Piñeiro]

Equivalent to the already existing how_declared at GLSL IR. The only
difference is that we are not adding all the declaration_type
available on GLSL, only the one that we will use on the short term. We
would add more mode if needed on the future.
---
 src/compiler/nir/nir.c   |  1 +
 src/compiler/nir/nir.h   | 24 
 src/compiler/nir/nir_lower_wpos_ytransform.c |  2 +-
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
index a849664134f..5e76654ca3d 100644
--- a/src/compiler/nir/nir.c
+++ b/src/compiler/nir/nir.c
@@ -162,6 +162,7 @@ nir_variable_create(nir_shader *shader, nir_variable_mode 
mode,
var->name = ralloc_strdup(var, name);
var->type = type;
var->data.mode = mode;
+   var->data.how_declared = nir_var_declared_normally;
 
if ((mode == nir_var_shader_in &&
 shader->info.stage != MESA_SHADER_VERTEX) ||
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
index bca6a32c956..1ccbccc8bbb 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -162,6 +162,22 @@ typedef enum {
 nir_depth_layout_unchanged
 } nir_depth_layout;
 
+/**
+ * Enum keeping track of how a variable was declared.
+ */
+typedef enum {
+   /**
+* Normal declaration.
+*/
+   nir_var_declared_normally = 0,
+
+   /**
+* Variable is implicitly generated by the compiler and should not be
+* visible via the API.
+*/
+   nir_var_hidden,
+} nir_var_declaration_type;
+
 /**
  * Either a uniform, global variable, shader input, or shader output. Based on
  * ir_variable - it should be easy to translate between the two.
@@ -349,6 +365,14 @@ typedef struct nir_variable {
*/
   unsigned xfb_stride;
 
+  /**
+   * How the variable was declared.  See nir_var_declaration_type.
+   *
+   * This is used to detect variables generated by the compiler, so should
+   * not be visible via the API.
+   */
+  unsigned how_declared:2;
+
   /**
* ARB_shader_image_load_store qualifiers.
*/
diff --git a/src/compiler/nir/nir_lower_wpos_ytransform.c 
b/src/compiler/nir/nir_lower_wpos_ytransform.c
index fc61beb7872..444e211b680 100644
--- a/src/compiler/nir/nir_lower_wpos_ytransform.c
+++ b/src/compiler/nir/nir_lower_wpos_ytransform.c
@@ -61,7 +61,7 @@ get_transform(lower_wpos_ytransform_state *state)
   var->state_slots[0].swizzle = SWIZZLE_XYZW;
   memcpy(var->state_slots[0].tokens, state->options->state_tokens,
  sizeof(var->state_slots[0].tokens));
-
+  var->data.how_declared = nir_var_hidden;
   state->transform = var;
}
return nir_load_var(>b, state->transform);
-- 
2.14.1

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

[Mesa-dev] [PATCH 02/10] spirv: Make VertexIndex and VertexId both non-zero-based

[Alejandro Piñeiro]

From: Neil Roberts 

GLSL has gl_VertexID which is supposed to be non-zero-based.

SPIR-V has both VertexIndex and VertexId builtins whose meanings are
defined by the APIs.

Vulkan defines VertexIndex as being non-zero-based. I can’t find
any mention of what VertexId is supposed to be.

GL_ARB_spirv removes VertexIndex and defines VertexId to be the same
as gl_VertexId (which is alo non-zero-based).

Previously in Mesa it was treating VertexIndex as non-zero-based and
VertexId as zero-based, so it was breaking for GL. This behaviour was
apparently based on Khronos bug 14255. However that bug doesn’t seem
to have made a final decision for VertexId.

Assuming there really is no other definition for VertexId for Vulkan
it seems better to just make them both have the same
value.
---
 src/compiler/spirv/vtn_variables.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/compiler/spirv/vtn_variables.c 
b/src/compiler/spirv/vtn_variables.c
index 8dab86abd74..b92bda59828 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -1011,15 +1011,16 @@ vtn_get_builtin_location(struct vtn_builder *b,
case SpvBuiltInCullDistance:
   *location = VARYING_SLOT_CULL_DIST0;
   break;
-   case SpvBuiltInVertexIndex:
-  *location = SYSTEM_VALUE_VERTEX_ID;
-  set_mode_system_value(b, mode);
-  break;
case SpvBuiltInVertexId:
-  /* Vulkan defines VertexID to be zero-based and reserves the new
-   * builtin keyword VertexIndex to indicate the non-zero-based value.
+   case SpvBuiltInVertexIndex:
+  /* For whatever reason, both of these are defined in the SPIR-V spec.
+   * The Vulkan spec defines VertexIndex to be non-zero-based and doesn’t
+   * mention VertexId. The ARB_gl_spirv spec defines VertexId to be the
+   * same as gl_VertexID, which is non-zero-based, and removes
+   * VertexIndex. Assuming there is no use for VertexId in Vulkan yet, we
+   * can just make them both be the same.
*/
-  *location = SYSTEM_VALUE_VERTEX_ID_ZERO_BASE;
+  *location = SYSTEM_VALUE_VERTEX_ID;
   set_mode_system_value(b, mode);
   break;
case SpvBuiltInInstanceIndex:
-- 
2.14.1

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

[Mesa-dev] [PATCH 01/10] spirv: fill info.gs.input_primitive too

[Alejandro Piñeiro]

info.gs.output_primitive was already being filled. Not sure why this
is not needed on Vulkan, but we found to be needed for
ARB_gl_spirv. Specifically, this is needed to get the following test
passing:

KHR-GL45.gl_spirv.spirv_validation_builtin_variable_decorations_test
---
 src/compiler/spirv/spirv_to_nir.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/compiler/spirv/spirv_to_nir.c 
b/src/compiler/spirv/spirv_to_nir.c
index 32ebdd78a1f..b5ec2de7bf9 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -3690,6 +3690,8 @@ vtn_handle_execution_mode(struct vtn_builder *b, struct 
vtn_value *entry_point,
  vtn_assert(b->shader->info.stage == MESA_SHADER_GEOMETRY);
  b->shader->info.gs.vertices_in =
 vertices_in_from_spv_execution_mode(b, mode->exec_mode);
+ b->shader->info.gs.input_primitive =
+gl_primitive_from_spv_execution_mode(b, mode->exec_mode);
   }
   break;
 
-- 
2.14.1

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

[Mesa-dev] [PATCH 00/10] ARB_gl_spirv series 5: some miscellaneous patches

[Alejandro Piñeiro]

Hi,

this is the fith series for the ongoing support for ARB_gl_spirv. This
time there is not a common topic for all those patches. They are
patches that we think that are ready, and would be good to send to
review in advance, while we finish other main subfeatures.

The tree for this series can be found on the following repository:
https://github.com/Igalia/mesa/tree/arb_gl_spirv-series5-misc-v1

And can be tested with the following piglit branch:
https://github.com/Igalia/piglit/tree/arb_gl_spirv-series4-misc-v1

Although that piglit branch, compared with the previous series, just
add some double tests. Some of the patches of this series are already
needed/tested by some CTS tests, so we didn't add a piglit test for
now. There are some comments about those CTS tests on some patches.

Having said so, with this branch (plus the patches already on master,
of course), if we made a full run borrowing as many piglit tests as
possible (something that we didn't propose to be the rule, but just
give the tools to replicate for developers), we get the following
numbers with the i965 driver (SKL):

   [34453/34453] skip: 5877, pass: 28224, fail: 196, crash: 156

Numbers that are somewhat inflated due the big amount of va64 tests,
but in any case, and taking into account the lack of ubo/ssbo support,
are promising.

FWIW, the numbers with the development branch are the following:

   [34453/34453] skip: 5877, pass: 28352, fail: 211, crash: 13

Comparing, it is 154 tests fixes, most of them due the wip (but almost
complete) ubo/ssbo support, and some validations.

BR

Alejandro Piñeiro (6):
  spirv: fill info.gs.input_primitive too
  nir: add how_declared to nir_variable.data
  nir/linker: take into account hidden uniforms
  nir/lower_samplers: don't assume a deref for both texture and sampler
srcs
  nir/glsl: make nir_remap_attributes public
  mesa/glspirv: compute double inputs and remap attributes

Iago Toral Quiroga (1):
  mesa/glspirv: pick off the only entry point we need

Neil Roberts (3):
  spirv: Make VertexIndex and VertexId both non-zero-based
  mesa/glspirv: Set separate_shader on shader_info
  i965/nir: Use the nir copy of shader_info to handle gl_PatchVerticesIn

 src/compiler/glsl/gl_nir_link_uniforms.c |  5 +-
 src/compiler/glsl/gl_nir_linker.c|  4 ++
 src/compiler/glsl/gl_nir_lower_samplers.c| 83 ++--
 src/compiler/glsl/glsl_to_nir.cpp| 17 --
 src/compiler/nir/nir.c   | 25 +
 src/compiler/nir/nir.h   | 27 +
 src/compiler/nir/nir_lower_wpos_ytransform.c |  2 +-
 src/compiler/spirv/spirv_to_nir.c|  2 +
 src/compiler/spirv/vtn_variables.c   | 15 ++---
 src/mesa/drivers/dri/i965/brw_program.c  |  2 +-
 src/mesa/main/glspirv.c  | 36 
 11 files changed, 163 insertions(+), 55 deletions(-)

-- 
2.14.1

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

Re: [Mesa-dev] [Mesa-stable] [PATCH] radv: Unset ZRANGE_PRECISION when depth was zeroed

[Andres Gomez]

On Thu, 2018-08-09 at 15:33 +0200, Bas Nieuwenhuizen wrote:
> On Thu, Aug 9, 2018 at 2:56 PM, Andres Gomez  wrote:
> > Bas, James, did you eventually come with a resolution for this? Can I
> > just ignore this nominated patch in the -stable ML?
> 
> The fix in this patch landed as part of

Great! Thanks ☺

> 
> commit 68dead112e710b261ad33604175d635dec6afd34
> Author: Samuel Pitoiset 
> Date:   Wed Jun 13 14:27:40 2018 +0200
> 
> radv: update the ZRANGE_PRECISION value for the TC-compat bug
> 
> On GFX8+, there is a bug that affects TC-compatible depth surfaces
> when the ZRange is not reset after LateZ kills pixels.
> 
> The workaround is to always set DB_Z_INFO.ZRANGE_PRECISION to match
> the last fast clear value. Because the value is set to 1 by default,
> we only need to update it when clearing Z to 0.0.
> 
> We also need to set the depth clear regs and to update
> ZRANGE_PRECISION when initializing a TC-compat depth image to 0.
> 
> Original patch from James Legg.
> 
> This fixes random CTS fails with
> dEQP-VK.renderpass.suballocation.formats.d32_sfloat_s8_uint.input.*
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105396
> CC: 
> Signed-off-by: Samuel Pitoiset 
> Reviewed-by: Bas Nieuwenhuizen 
> 
> 
> > 
> > On Wed, 2018-03-28 at 15:28 +0200, Bas Nieuwenhuizen wrote:
> > > No final resolution yet.
> > > 
> > > I was trying to fix my minor comment, but looks like I have a bunch of
> > > CTS regressions here with the original patch, so still working on it.
> > > 
> > > On Tue, Mar 27, 2018 at 2:03 PM, Juan A. Suarez Romero
> > >  wrote:
> > > > On Thu, 2018-03-22 at 12:31 +, James Legg wrote:
> > > > > On Thu, 2018-03-22 at 02:36 +0100, Bas Nieuwenhuizen wrote:
> > > > > > On Thu, Mar 8, 2018 at 12:59 PM, James Legg 
> > > > > >  wrote:
> > > > > > > This avoids bug 105396 somehow. I suspect it is a VI and GFX9 
> > > > > > > hardware
> > > > > > > bug which PAL calls WaTcCompatZRange, but I don't know for sure.
> > > > > > > 
> > > > > > > In the VK_FORMAT_D32_SFLOAT case, TILE_STENCIL_DISABLE is not set 
> > > > > > > for
> > > > > > > tc compatible image formats regardless of not having a stencil 
> > > > > > > aspect.
> > > > > > > If TILE_STENCIL_DISABLE was set, ZRANGE_PRECISION would have no 
> > > > > > > effect
> > > > > > > and the bug would occur again.
> > > > > > > 
> > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105396
> > > > > > > CC: 
> > > > > > > CC: Dave Airlie 
> > > > > > > CC: Bas Nieuwenhuizen 
> > > > > > > CC: Samuel Pitoiset 
> > > > > > > ---
> > > > > > >  src/amd/vulkan/radv_cmd_buffer.c | 52 
> > > > > > > +---
> > > > > > >  1 file changed, 49 insertions(+), 3 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
> > > > > > > b/src/amd/vulkan/radv_cmd_buffer.c
> > > > > > > index 3e0ed0e9a9..89e31a0347 100644
> > > > > > > --- a/src/amd/vulkan/radv_cmd_buffer.c
> > > > > > > +++ b/src/amd/vulkan/radv_cmd_buffer.c
> > > > > > > @@ -915,6 +915,37 @@ radv_emit_fb_ds_state(struct radv_cmd_buffer 
> > > > > > > *cmd_buffer,
> > > > > > > 
> > > > > > > }
> > > > > > > 
> > > > > > > +   if (image->surface.htile_size)
> > > > > > > +   {
> > > > > > > +   /* If the last depth clear value was 0.0f, set 
> > > > > > > ZRANGE_PRECISION
> > > > > > > +* to 0 in dp_z_info for more accuracy with 
> > > > > > > reverse depth; and
> > > > > > > +* to avoid 
> > > > > > > https://bugs.freedesktop.org/show_bug.cgi?id=105396.
> > > > > > > +* Otherwise, we leave it set to 1.
> > > > > > > +*/
> > > > > > > +   radeon_emit(cmd_buffer->cs, PKT3(PKT3_COND_WRITE, 
> > > > > > > 7, 0));
> > > > > > > +
> > > > > > > +   const uint32_t write_space = 0 << 8;/* 
> > > > > > > register */
> > > > > > > +   const uint32_t poll_space = 1 << 4; /* memory 
> > > > > > > */
> > > > > > > +   const uint32_t function = 3 << 0;   /* equal 
> > > > > > > to the reference */
> > > > > > > +   const uint32_t options = write_space | poll_space 
> > > > > > > | function;
> > > > > > > +   radeon_emit(cmd_buffer->cs, options);
> > > > > > > +
> > > > > > > +   /* poll address - location of the depth clear 
> > > > > > > value */
> > > > > > > +   uint64_t va = radv_buffer_get_va(image->bo);
> > > > > > > +   va += image->offset + image->clear_value_offset;
> > > > > > > +   radeon_emit(cmd_buffer->cs, va);
> > > > > > > +   radeon_emit(cmd_buffer->cs, va >> 32);
> > > > > > > +
> > > > > > > +   radeon_emit(cmd_buffer->cs, fui(0.0f)); 
> > > > > > > /* reference value */
> > > > > > > +   radeon_emit(cmd_buffer->cs, (uint32_t)-1);  
> > > > > > > /* comparison mask */
> > 

Re: [Mesa-dev] [Mesa-stable] [PATCH] radv: Unset ZRANGE_PRECISION when depth was zeroed

[Bas Nieuwenhuizen]

On Thu, Aug 9, 2018 at 2:56 PM, Andres Gomez  wrote:
> Bas, James, did you eventually come with a resolution for this? Can I
> just ignore this nominated patch in the -stable ML?

The fix in this patch landed as part of

commit 68dead112e710b261ad33604175d635dec6afd34
Author: Samuel Pitoiset 
Date:   Wed Jun 13 14:27:40 2018 +0200

radv: update the ZRANGE_PRECISION value for the TC-compat bug

On GFX8+, there is a bug that affects TC-compatible depth surfaces
when the ZRange is not reset after LateZ kills pixels.

The workaround is to always set DB_Z_INFO.ZRANGE_PRECISION to match
the last fast clear value. Because the value is set to 1 by default,
we only need to update it when clearing Z to 0.0.

We also need to set the depth clear regs and to update
ZRANGE_PRECISION when initializing a TC-compat depth image to 0.

Original patch from James Legg.

This fixes random CTS fails with
dEQP-VK.renderpass.suballocation.formats.d32_sfloat_s8_uint.input.*

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105396
CC: 
Signed-off-by: Samuel Pitoiset 
Reviewed-by: Bas Nieuwenhuizen 


>
> On Wed, 2018-03-28 at 15:28 +0200, Bas Nieuwenhuizen wrote:
>> No final resolution yet.
>>
>> I was trying to fix my minor comment, but looks like I have a bunch of
>> CTS regressions here with the original patch, so still working on it.
>>
>> On Tue, Mar 27, 2018 at 2:03 PM, Juan A. Suarez Romero
>>  wrote:
>> > On Thu, 2018-03-22 at 12:31 +, James Legg wrote:
>> > > On Thu, 2018-03-22 at 02:36 +0100, Bas Nieuwenhuizen wrote:
>> > > > On Thu, Mar 8, 2018 at 12:59 PM, James Legg 
>> > > >  wrote:
>> > > > > This avoids bug 105396 somehow. I suspect it is a VI and GFX9 
>> > > > > hardware
>> > > > > bug which PAL calls WaTcCompatZRange, but I don't know for sure.
>> > > > >
>> > > > > In the VK_FORMAT_D32_SFLOAT case, TILE_STENCIL_DISABLE is not set for
>> > > > > tc compatible image formats regardless of not having a stencil 
>> > > > > aspect.
>> > > > > If TILE_STENCIL_DISABLE was set, ZRANGE_PRECISION would have no 
>> > > > > effect
>> > > > > and the bug would occur again.
>> > > > >
>> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105396
>> > > > > CC: 
>> > > > > CC: Dave Airlie 
>> > > > > CC: Bas Nieuwenhuizen 
>> > > > > CC: Samuel Pitoiset 
>> > > > > ---
>> > > > >  src/amd/vulkan/radv_cmd_buffer.c | 52 
>> > > > > +---
>> > > > >  1 file changed, 49 insertions(+), 3 deletions(-)
>> > > > >
>> > > > > diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
>> > > > > b/src/amd/vulkan/radv_cmd_buffer.c
>> > > > > index 3e0ed0e9a9..89e31a0347 100644
>> > > > > --- a/src/amd/vulkan/radv_cmd_buffer.c
>> > > > > +++ b/src/amd/vulkan/radv_cmd_buffer.c
>> > > > > @@ -915,6 +915,37 @@ radv_emit_fb_ds_state(struct radv_cmd_buffer 
>> > > > > *cmd_buffer,
>> > > > >
>> > > > > }
>> > > > >
>> > > > > +   if (image->surface.htile_size)
>> > > > > +   {
>> > > > > +   /* If the last depth clear value was 0.0f, set 
>> > > > > ZRANGE_PRECISION
>> > > > > +* to 0 in dp_z_info for more accuracy with reverse 
>> > > > > depth; and
>> > > > > +* to avoid 
>> > > > > https://bugs.freedesktop.org/show_bug.cgi?id=105396.
>> > > > > +* Otherwise, we leave it set to 1.
>> > > > > +*/
>> > > > > +   radeon_emit(cmd_buffer->cs, PKT3(PKT3_COND_WRITE, 7, 
>> > > > > 0));
>> > > > > +
>> > > > > +   const uint32_t write_space = 0 << 8;/* register 
>> > > > > */
>> > > > > +   const uint32_t poll_space = 1 << 4; /* memory */
>> > > > > +   const uint32_t function = 3 << 0;   /* equal to 
>> > > > > the reference */
>> > > > > +   const uint32_t options = write_space | poll_space | 
>> > > > > function;
>> > > > > +   radeon_emit(cmd_buffer->cs, options);
>> > > > > +
>> > > > > +   /* poll address - location of the depth clear value 
>> > > > > */
>> > > > > +   uint64_t va = radv_buffer_get_va(image->bo);
>> > > > > +   va += image->offset + image->clear_value_offset;
>> > > > > +   radeon_emit(cmd_buffer->cs, va);
>> > > > > +   radeon_emit(cmd_buffer->cs, va >> 32);
>> > > > > +
>> > > > > +   radeon_emit(cmd_buffer->cs, fui(0.0f)); /* 
>> > > > > reference value */
>> > > > > +   radeon_emit(cmd_buffer->cs, (uint32_t)-1);  /* 
>> > > > > comparison mask */
>> > > > > +   radeon_emit(cmd_buffer->cs, R_028040_DB_Z_INFO >> 
>> > > > > 2); /* write address low */
>> > > > > +   radeon_emit(cmd_buffer->cs, 0u);/* 
>> > > > > write address high */
>> > > > > +
>> > > > > +   /* The value to write data when the condition passes 
>> > > > > */
>> > > > > +   uint32_t db_z_info_clear_zero = db_z_info & 
>> > > 

Re: [Mesa-dev] [PATCH] intel/isl: Avoid tiling on 16K-wide render targets

[Andres Gomez]

Ugh!

Unfortunately, as I've commented at:
https://bugs.freedesktop.org/show_bug.cgi?id=107359

This is now breaking these other CTS tests:
GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_framebuffers_different_formats
GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_framebuffers_different_origins
GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_framebuffers_different_sample_count
GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_framebuffers_different_sizes
GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_read_buffer_different_formats
GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_read_buffer_different_origins
GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_error_blitframebuffer_multisampled_read_buffer_different_sizes
GTF-GL46.gtf30.GL3Tests.framebuffer_blit.framebuffer_blit_functionality_multisampled_to_singlesampled_blit


On Mon, 2018-07-30 at 19:25 +0300, Andres Gomez wrote:
> That was quick! ☺
> 
> On Fri, 2018-07-27 at 16:02 -0700, Nanley Chery wrote:
> > Fix rendering issues on BDW and SKL.
> > 
> > Fixes 0288fe8d0417730bdd5b3477130dd1dc32bdbcd3
> > ("i965/miptree: Use the correct BLT pitch")
> 
> I'd add here some lines listing the tests fixed by this patch.
> 
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107359
> > Cc: 
> 
> This is:
> 
> Tested-by: Andres Gomez 
> 
> > ---
> > 
> > We could probably add an assert when filling out the surface state, but
> > I think BLORP would need a non-trivial amount of work done as a
> > prerequisite. I'm thinking specifically of the cases where we bind a
> > depth buffer as a render target.
> > 
> > I won't be able to push anything until about a week from EOD today, so
> > if this does end up getting reviewed, please feel free to push it.
> > 
> >  src/intel/isl/isl_gen7.c | 19 +++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/src/intel/isl/isl_gen7.c b/src/intel/isl/isl_gen7.c
> > index 4fa9851233f..2d85f4b568d 100644
> > --- a/src/intel/isl/isl_gen7.c
> > +++ b/src/intel/isl/isl_gen7.c
> > @@ -294,6 +294,25 @@ isl_gen6_filter_tiling(const struct isl_device *dev,
> >  */
> > if (ISL_DEV_GEN(dev) < 7 && isl_format_get_layout(info->format)->bpb >= 
> > 128)
> >*flags &= ~ISL_TILING_Y0_BIT;
> > +
> > +   /* From the BDW and SKL PRMs, Volume 2d,
> > +* RENDER_SURFACE_STATE::Width - Programming Notes:
> > +*
> > +*   A known issue exists if a primitive is rendered to the first 2 
> > rows and
> > +*   last 2 columns of a 16K width surface. If any geometry is drawn 
> > inside
> > +*   this square it will be copied to column X=2 and X=3 (arrangement 
> > on Y
> > +*   position will stay the same). If any geometry exceeds the 
> > boundaries of
> > +*   this 2x2 region it will be drawn normally. The issue also only 
> > occurs
> > +*   if the surface has TileMode != Linear.
> > +*
> > +* To prevent this rendering corruption, only allow linear tiling for
> > +* surfaces with widths greater than 16K-2 pixels.
> > +*/
> > +   if (info->width > 16382 &&
> > +   info->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT &&
> > +   (ISL_DEV_GEN(dev) == 8 || dev->info->is_skylake)) {
> > +  *flags &= ISL_TILING_LINEAR_BIT;
> > +   }
> >  }
> >  
> >  void
-- 
Br,

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

Re: [Mesa-dev] [PATCH v2] drirc: Allow extension midshader for Metro Redux

[Vadym Shovkoplias]

Hi Andreas,

This change shouldn't brake anything. So yes, it can be included in stable.

On Thu, Aug 9, 2018 at 3:44 PM, Andres Gomez  wrote:

> Vadym, should we also include this in the stable queues ?
>
> On Mon, 2018-08-06 at 15:52 +0300, vadym.shovkoplias wrote:
> > This fixes both Metro 2033 Redux and Metro Last Light Redux
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99730
> > Signed-off-by: Eero Tamminen 
> > Signed-off-by: Vadym Shovkoplias 
> > ---
> >  src/util/drirc | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/src/util/drirc b/src/util/drirc
> > index 8ece875e34..c4f9e060f3 100644
> > --- a/src/util/drirc
> > +++ b/src/util/drirc
> > @@ -120,6 +120,10 @@ TODO: document the other workarounds.
> >   value="true" />
> >  
> >
> > + executable="metro">
> > + value="true" />
> > +
> > +
> >  
> >  
> >  
> --
> Br,
>
> Andres
>



-- 

Vadym Shovkoplias | Senior Software Engineer
GlobalLogic
P +380.57.766.7667  M +3.8050.931.7304  S vadym.shovkoplias
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 107533] Please restore --with-{gl, osmesa}-lib-name options

[bugzilla-daemon]

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

--- Comment #2 from Emil Velikov  ---
You're correct - the SONAME does stay intact. One could trivially change that
with patchelf.

Let's take a look at the following combinations:
 1) standard vs GLVND
names: GL
symbols: standard

 2) GLVND
names: GLX_mesa
symbols: (should be) limited set != standard

 3) mangling
name: Mangled + (GL, GLX_mesa?)
symbols: standard prefixed with m

 4) arbitrary name
name: ?? does it it interact with mangled and/or GLVND?
symbols: standard - how do we ensure no symbol collision?

That said, an arbitrary name many have worked in a particular combination, but
the rest were broken :-(

Attempting to have all the cases working is O(N), while 4) is a rm && patchelf,
post-install.

Can see that being slightly annoying, but I hope you'll agreed on the goal.

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

Re: [Mesa-dev] [Mesa-stable] [PATCH 1/8] radv: don't flush src stages when dstStageMask == BOTTOM_OF_PIPE

[Andres Gomez]

Fredrik, which is the status of this series? Several patches got R-b
but nothing has landed so far. Are you in need of more reviews for the
rest of the patches in the series?

On Tue, 2018-06-26 at 23:49 +0200, Fredrik Höglund wrote:
> The Vulkan specification says:
> 
>"An execution dependency with only VK_PIPELINE_STAGE_BOTTOM_OF_-
> PIPE_BIT in the destination stage mask [...] does not delay
> processing of subsequent commands."
> 
> Cc: 
> Signed-off-by: Fredrik Höglund 
> ---
>  src/amd/vulkan/radv_cmd_buffer.c | 3 ++-
>  src/amd/vulkan/radv_pass.c   | 6 --
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
> b/src/amd/vulkan/radv_cmd_buffer.c
> index 110a9a960a9..5bfcba28d83 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -4197,7 +4197,8 @@ void radv_CmdPipelineBarrier(
>   image);
>   }
>  
> - radv_stage_flush(cmd_buffer, srcStageMask);
> + if (destStageMask != VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT)
> + radv_stage_flush(cmd_buffer, srcStageMask);
>   cmd_buffer->state.flush_bits |= src_flush_bits;
>  
>   for (uint32_t i = 0; i < imageMemoryBarrierCount; i++) {
> diff --git a/src/amd/vulkan/radv_pass.c b/src/amd/vulkan/radv_pass.c
> index 15fee444cdc..7a0dca09496 100644
> --- a/src/amd/vulkan/radv_pass.c
> +++ b/src/amd/vulkan/radv_pass.c
> @@ -174,11 +174,13 @@ VkResult radv_CreateRenderPass(
>   for (unsigned i = 0; i < pCreateInfo->dependencyCount; ++i) {
>   uint32_t dst = pCreateInfo->pDependencies[i].dstSubpass;
>   if (dst == VK_SUBPASS_EXTERNAL) {
> - pass->end_barrier.src_stage_mask = 
> pCreateInfo->pDependencies[i].srcStageMask;
> + if (pCreateInfo->pDependencies[i].dstStageMask != 
> VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT)
> + pass->end_barrier.src_stage_mask = 
> pCreateInfo->pDependencies[i].srcStageMask;
>   pass->end_barrier.src_access_mask = 
> pCreateInfo->pDependencies[i].srcAccessMask;
>   pass->end_barrier.dst_access_mask = 
> pCreateInfo->pDependencies[i].dstAccessMask;
>   } else {
> - pass->subpasses[dst].start_barrier.src_stage_mask = 
> pCreateInfo->pDependencies[i].srcStageMask;
> + if (pCreateInfo->pDependencies[i].dstStageMask != 
> VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT)
> + 
> pass->subpasses[dst].start_barrier.src_stage_mask = 
> pCreateInfo->pDependencies[i].srcStageMask;
>   pass->subpasses[dst].start_barrier.src_access_mask = 
> pCreateInfo->pDependencies[i].srcAccessMask;
>   pass->subpasses[dst].start_barrier.dst_access_mask = 
> pCreateInfo->pDependencies[i].dstAccessMask;
>   }
-- 
Br,

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

Re: [Mesa-dev] [Mesa-stable] [PATCH 4/8] egl: rework input validation order in _eglCreateWindowSurfaceCommon

[Andres Gomez]

Emil, this patch has been stalled in the -stable ML for quite some time
without update.

Unless you say otherwise, I will just ignore it at this point and trust
that you will also Cc -stable in the future, in case you come with
another version.

On Wed, 2017-09-27 at 17:36 +0200, Juan A. Suarez Romero wrote:
> On Wed, 2017-09-27 at 16:15 +0100, Emil Velikov wrote:
> > On 26 September 2017 at 18:08, Juan A. Suarez Romero
> >  wrote:
> > > On Wed, 2017-09-06 at 15:07 +0100, Emil Velikov wrote:
> > > > On 5 August 2017 at 00:25, Emil Velikov  
> > > > wrote:
> > > > > From: Emil Velikov 
> > > > > 
> > > > > As mentioned in previous commit the negative tests in dEQP expect the
> > > > > arguments to be evaluated in particular order.
> > > > > 
> > > > > Namely - first the dpy, then the config, followed by the 
> > > > > surface/window.
> > > > > 
> > > > > Move the check further down or executing the test below will produce
> > > > > the following error.
> > > > > 
> > > > >dEQP-EGL.functional.negative_api.create_pbuffer_surface
> > > > > 
> > > > > 
> > > > >
> > > > >   eglCreateWindowSurface(0x9bfff0f150, 0x, 
> > > > > 0x, { EGL_NONE });
> > > > >   // 0x returned
> > > > >   // ERROR expected: EGL_BAD_CONFIG, Got: 
> > > > > EGL_BAD_NATIVE_WINDOW
> > > > >
> > > > > 
> > > > 
> > > > FTR dEQP has been "fixed" (by removing the test all together) to not
> > > > generate the above error. At the same the Pixman equivalent is still
> > > > buggy, hence the v2 of commit
> > > > df8efd5b74d45e2bfb977a92dcd3db86abd6b143.
> > > > 
> > > 
> > > Emil, does it mean this patch is superseded? I'm interesting in knowing
> > > the situation as this was tagged to be included in stable release.
> > > 
> > 
> > I'd like to keep this open and eventually merge it.
> > The dEQP patches need 'a bit' of polishing.
> > 
> 
> Sure. I'll skip it for next release. Thanks
> 
>   J.A.
> 
> > -Emil
> > ___
> > mesa-stable mailing list
> > mesa-sta...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-stable
> 
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
-- 
Br,

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

Re: [Mesa-dev] [Mesa-stable] [PATCH] radv: Unset ZRANGE_PRECISION when depth was zeroed

[Andres Gomez]

Bas, James, did you eventually come with a resolution for this? Can I
just ignore this nominated patch in the -stable ML?

On Wed, 2018-03-28 at 15:28 +0200, Bas Nieuwenhuizen wrote:
> No final resolution yet.
> 
> I was trying to fix my minor comment, but looks like I have a bunch of
> CTS regressions here with the original patch, so still working on it.
> 
> On Tue, Mar 27, 2018 at 2:03 PM, Juan A. Suarez Romero
>  wrote:
> > On Thu, 2018-03-22 at 12:31 +, James Legg wrote:
> > > On Thu, 2018-03-22 at 02:36 +0100, Bas Nieuwenhuizen wrote:
> > > > On Thu, Mar 8, 2018 at 12:59 PM, James Legg 
> > > >  wrote:
> > > > > This avoids bug 105396 somehow. I suspect it is a VI and GFX9 hardware
> > > > > bug which PAL calls WaTcCompatZRange, but I don't know for sure.
> > > > > 
> > > > > In the VK_FORMAT_D32_SFLOAT case, TILE_STENCIL_DISABLE is not set for
> > > > > tc compatible image formats regardless of not having a stencil aspect.
> > > > > If TILE_STENCIL_DISABLE was set, ZRANGE_PRECISION would have no effect
> > > > > and the bug would occur again.
> > > > > 
> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105396
> > > > > CC: 
> > > > > CC: Dave Airlie 
> > > > > CC: Bas Nieuwenhuizen 
> > > > > CC: Samuel Pitoiset 
> > > > > ---
> > > > >  src/amd/vulkan/radv_cmd_buffer.c | 52 
> > > > > +---
> > > > >  1 file changed, 49 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/src/amd/vulkan/radv_cmd_buffer.c 
> > > > > b/src/amd/vulkan/radv_cmd_buffer.c
> > > > > index 3e0ed0e9a9..89e31a0347 100644
> > > > > --- a/src/amd/vulkan/radv_cmd_buffer.c
> > > > > +++ b/src/amd/vulkan/radv_cmd_buffer.c
> > > > > @@ -915,6 +915,37 @@ radv_emit_fb_ds_state(struct radv_cmd_buffer 
> > > > > *cmd_buffer,
> > > > > 
> > > > > }
> > > > > 
> > > > > +   if (image->surface.htile_size)
> > > > > +   {
> > > > > +   /* If the last depth clear value was 0.0f, set 
> > > > > ZRANGE_PRECISION
> > > > > +* to 0 in dp_z_info for more accuracy with reverse 
> > > > > depth; and
> > > > > +* to avoid 
> > > > > https://bugs.freedesktop.org/show_bug.cgi?id=105396.
> > > > > +* Otherwise, we leave it set to 1.
> > > > > +*/
> > > > > +   radeon_emit(cmd_buffer->cs, PKT3(PKT3_COND_WRITE, 7, 
> > > > > 0));
> > > > > +
> > > > > +   const uint32_t write_space = 0 << 8;/* register */
> > > > > +   const uint32_t poll_space = 1 << 4; /* memory */
> > > > > +   const uint32_t function = 3 << 0;   /* equal to 
> > > > > the reference */
> > > > > +   const uint32_t options = write_space | poll_space | 
> > > > > function;
> > > > > +   radeon_emit(cmd_buffer->cs, options);
> > > > > +
> > > > > +   /* poll address - location of the depth clear value */
> > > > > +   uint64_t va = radv_buffer_get_va(image->bo);
> > > > > +   va += image->offset + image->clear_value_offset;
> > > > > +   radeon_emit(cmd_buffer->cs, va);
> > > > > +   radeon_emit(cmd_buffer->cs, va >> 32);
> > > > > +
> > > > > +   radeon_emit(cmd_buffer->cs, fui(0.0f)); /* 
> > > > > reference value */
> > > > > +   radeon_emit(cmd_buffer->cs, (uint32_t)-1);  /* 
> > > > > comparison mask */
> > > > > +   radeon_emit(cmd_buffer->cs, R_028040_DB_Z_INFO >> 2); 
> > > > > /* write address low */
> > > > > +   radeon_emit(cmd_buffer->cs, 0u);/* 
> > > > > write address high */
> > > > > +
> > > > > +   /* The value to write data when the condition passes 
> > > > > */
> > > > > +   uint32_t db_z_info_clear_zero = db_z_info & 
> > > > > C_028040_ZRANGE_PRECISION;
> > > > > +   radeon_emit(cmd_buffer->cs, db_z_info_clear_zero);
> > > > > +   }
> > > > > +
> > > > > radeon_set_context_reg(cmd_buffer->cs, 
> > > > > R_028B78_PA_SU_POLY_OFFSET_DB_FMT_CNTL,
> > > > >ds->pa_su_poly_offset_db_fmt_cntl);
> > > > >  }
> > > > > @@ -3479,7 +3510,8 @@ void radv_CmdEndRenderPass(
> > > > > 
> > > > >  /*
> > > > >   * For HTILE we have the following interesting clear words:
> > > > > - *   0xf30f: Uncompressed, full depth range, for depth+stencil 
> > > > > HTILE
> > > > > + *   0xf30f: Uncompressed, full depth range, for depth+stencil 
> > > > > HTILE when ZRANGE_PRECISION is 1
> > > > > + *   0x0003f30f: Uncompressed, full depth range, for depth+stencil 
> > > > > HTILE when ZRANGE_PRECISION is 0
> > > > >   *   0xfffc000f: Uncompressed, full depth range, for depth only 
> > > > > HTILE.
> > > > >   *   0xfff0: Clear depth to 1.0
> > > > >   *   0x: Clear depth to 0.0
> > > > > @@ -3528,8 +3560,22 @@ static void 
> > > > > radv_handle_depth_image_transition(struct radv_cmd_buffer *cmd_buffe
> > > > >

Re: [Mesa-dev] [PATCH] swr: bump minimum supported LLVM version to 6.0

[Cherniak, Bruce]

We agree.
Reviewed-by: Bruce Cherniak  

> On Aug 8, 2018, at 6:50 AM, Juan A. Suarez Romero  wrote:
> 
> On Mon, 2018-08-06 at 11:52 +0200, Juan A. Suarez Romero wrote:
>> RADV now requires LLVM 6.0 or greater, and thus we can't build dist
>> tarball because swr requires LLVM 5.0.
>> 
>> Let's bump required LLVM to 6.0 in swr too.
>> 
>> Fixes: fd1121e839 ("amd: remove support for LLVM 5.0")
>> Cc: Tim Rowley 
>> Cc: Emil Velikov 
>> Cc: Dylan Baker 
>> Cc: Eric Engestrom 
> 
> CC: "Cherniak, Bruce" 
> 
>> ---
>> configure.ac| 7 +++
>> src/gallium/drivers/swr/Makefile.am | 2 +-
>> src/gallium/drivers/swr/SConscript  | 4 ++--
>> 3 files changed, 6 insertions(+), 7 deletions(-)
>> 
>> diff --git a/configure.ac b/configure.ac
>> index 7d898aeda9e..10d37584696 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -110,7 +110,7 @@ LLVM_REQUIRED_OPENCL=3.9.0
>> LLVM_REQUIRED_R600=3.9.0
>> LLVM_REQUIRED_RADEONSI=6.0.0
>> LLVM_REQUIRED_RADV=6.0.0
>> -LLVM_REQUIRED_SWR=5.0.0
>> +LLVM_REQUIRED_SWR=6.0.0
>> 
>> dnl Check for progs
>> AC_PROG_CPP
>> @@ -2797,9 +2797,8 @@ if test -n "$with_gallium_drivers"; then
>> fi
>> 
>> # XXX: Keep in sync with LLVM_REQUIRED_SWR
>> -AM_CONDITIONAL(SWR_INVALID_LLVM_VERSION, test "x$LLVM_VERSION" != x5.0.0 -a 
>> \
>> -  "x$LLVM_VERSION" != x5.0.1 -a 
>> \
>> -  "x$LLVM_VERSION" != x5.0.2)
>> +AM_CONDITIONAL(SWR_INVALID_LLVM_VERSION, test "x$LLVM_VERSION" != x6.0.0 -a 
>> \
>> +  "x$LLVM_VERSION" != x6.0.1)
>> 
>> if test "x$enable_llvm" = "xyes" -a "$with_gallium_drivers"; then
>> llvm_require_version $LLVM_REQUIRED_GALLIUM "gallium"
>> diff --git a/src/gallium/drivers/swr/Makefile.am 
>> b/src/gallium/drivers/swr/Makefile.am
>> index 5cc3f77478a..d20a6bdbed3 100644
>> --- a/src/gallium/drivers/swr/Makefile.am
>> +++ b/src/gallium/drivers/swr/Makefile.am
>> @@ -375,7 +375,7 @@ include $(top_srcdir)/install-gallium-links.mk
>> dist-hook:
>> if SWR_INVALID_LLVM_VERSION
>>  @echo "*"
>> -@echo "LLVM 5.0.x required to create the tarball"
>> +@echo "LLVM 6.0.x required to create the tarball"
>>  @echo "*"
>>  @test
>> endif
>> diff --git a/src/gallium/drivers/swr/SConscript 
>> b/src/gallium/drivers/swr/SConscript
>> index 224372eb3f5..a89d02c5db0 100644
>> --- a/src/gallium/drivers/swr/SConscript
>> +++ b/src/gallium/drivers/swr/SConscript
>> @@ -12,8 +12,8 @@ if not env['llvm']:
>> env['swr'] = False
>> Return()
>> 
>> -if env['LLVM_VERSION'] < distutils.version.LooseVersion('5.0'):
>> -print("warning: swr requires LLVM >= 5.0: not building swr")
>> +if env['LLVM_VERSION'] < distutils.version.LooseVersion('6.0'):
>> +print("warning: swr requires LLVM >= 6.0: not building swr")
>> env['swr'] = False
>> Return()
>> 
> 

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

Re: [Mesa-dev] [Mesa-stable] [PATCH] glx: GLX_MESA_multithread_makecurrent is direct-only

[Andres Gomez]

Adam, which is the status of this patch? Is this effectively dropped?

On Wed, 2017-12-06 at 16:13 -0500, Adam Jackson wrote:
> On Wed, 2017-12-06 at 15:01 -0500, Ian Romanick wrote:
> > On 12/06/2017 10:32 AM, Emil Velikov wrote:
> > > On 5 December 2017 at 16:10, Adam Jackson  wrote:
> > > > This extension is not defined for indirect contexts. Marking it as
> > > > "client only", as the old code did here, would make the extension
> > > > available in indirect contexts, even though the server would certainly
> > > > not have it in its extension list.
> > > > 
> > > > Cc: 
> > > > Signed-off-by: Adam Jackson 
> > > 
> > > Reviewed-by: Emil Velikov 
> > > 
> > > Unrelated: reportedly only cairo is using the extension, so could we
> > > consider the extension obsolete?
> > 
> > It's not too surprising that only Cairo is using it.  IIRC, Eric
> > specifically made this extension for Cairo, and it was a pretty big perf
> > win at the time.
> 
> I think at this point most of the effect could be achieved with no-
> flush contexts, but yeah.
> 
> > I had wanted to test this patch, but... does LIBGL_ALWAYS_INDIRECT=y
> > just not work any more?  With the distro Mesa I get:
> 
> It works, that's the server telling you it doesn't support indirect
> rendering. We turned that off by default a few releases ago as being
> slow and underfeatured and CVE-prone. Start your server with +iglx or
> with this in xorg.conf:
> 
> Section "ServerFlags"
>   Option "IndirectGLX" "true"
> EndSection
> 
> - ajax
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
-- 
Br,

Andres

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

Re: [Mesa-dev] [Mesa-stable] [PATCH 07/10] gbm: unify error handling in gbm_dri_bo_import()

[Andres Gomez]

Emil, this patch never landed in master (nor got a R-b).

Is this still relevant? Could you manage to get somebody to review it?

I'd do it myself but I'm quite ignorant on the GBM bits.

On Mon, 2017-10-16 at 17:04 +0100, Emil Velikov wrote:
> From: Emil Velikov 
> 
> Fold the error handling for image creation/duplication in a single
> place.
> 
> Effectively providing the same errno across the board and plugging a
> memory leak in the GBM_BO_IMPORT_WL_BUFFER case.
> 
> Cc: mesa-sta...@lists.freedesktop.org
> Signed-off-by: Emil Velikov 
> ---
>  src/gbm/backends/dri/gbm_dri.c | 35 +--
>  1 file changed, 13 insertions(+), 22 deletions(-)
> 
> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
> index d7cf01fc6b4..c39ee9c9a53 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -888,9 +888,9 @@ gbm_dri_bo_import(struct gbm_device *gbm,
>  {
> struct gbm_dri_device *dri = gbm_dri_device(gbm);
> struct gbm_dri_bo *bo;
> -   __DRIimage *image;
> +   __DRIimage *image = NULL;
> unsigned dri_use = 0;
> -   int gbm_format;
> +   int gbm_format = 0;
> unsigned query; /* EGLBoolean, but we cannot include the header */
>  
> if (dri->image == NULL) {
> @@ -935,17 +935,9 @@ gbm_dri_bo_import(struct gbm_device *gbm,
>image = dri->lookup_image(dri->screen, buffer, dri->lookup_user_data);
>image = dri->image->dupImage(image, NULL);
>query = dri->image->queryImage(image, __DRI_IMAGE_ATTRIB_FORMAT, 
> _format);
> -  if (!query) {
> - errno = EINVAL;
> - dri->image->destroyImage(image);
> +  if (!query)
>   break;
> -  }
>gbm_format = gbm_dri_to_gbm_format(dri_format);
> -  if (gbm_format == 0) {
> - errno = EINVAL;
> - dri->image->destroyImage(image);
> - return NULL;
> -  }
>break;
> }
>  
> @@ -973,10 +965,6 @@ gbm_dri_bo_import(struct gbm_device *gbm,
>   _data->fd, 1,
>   , ,
>   NULL);
> -  if (image == NULL) {
> - errno = EINVAL;
> - return NULL;
> -  }
>gbm_format = fd_data->format;
>break;
> }
> @@ -1008,20 +996,23 @@ gbm_dri_bo_import(struct gbm_device *gbm,
>fd_data->offsets,
>0, 0, 0, 0,
>, NULL);
> -  if (image == NULL) {
> - errno = ENOSYS;
> - return NULL;
> -  }
> -
>gbm_format = fourcc;
>break;
> }
>  
> default:
> -  errno = ENOSYS;
> -  return NULL;
> +  break;
> }
>  
> +   if (image == NULL) {
> +  errno = EINVAL;
> +  return NULL;
> +   }
> +   if (gbm_format == 0) {
> +  errno = EINVAL;
> +  dri->image->destroyImage(image);
> +  return NULL;
> +   }
>  
> bo = calloc(1, sizeof *bo);
> if (bo == NULL) {
-- 
Br,

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

Re: [Mesa-dev] [Mesa-stable] [PATCH] glx: use correct table offset for glAreTexturesResidentEXT

[Andres Gomez]

Emil, this patch has been stalled in the -stable ML for quite some time
without update.

Unless you say otherwise, I will just ignore it at this point and trust
that you will also Cc -stable in the future, in case you come with
another version.

On Tue, 2017-09-26 at 18:52 +0200, Juan A. Suarez Romero wrote:
> On Tue, 2017-07-25 at 11:43 +0100, Emil Velikov wrote:
> > On 25 July 2017 at 09:12, Nicolai Hähnle  wrote:
> > > On 21.07.2017 16:57, Emil Velikov wrote:
> > > > 
> > > > From: Emil Velikov 
> > > > 
> > > > The correct offset is 322 as opposed to 332.
> > > > Broken since the rework of GET_DISPATCH back in ~2012.
> > > 
> > > 
> > > This is kind of amazing. How did you run into this?
> > > 
> > 
> > Looking at [truly] vendor neutral indirect libGLX, as opposed to the
> > current solution.
> > 
> > > Maybe add a simple touch test to piglit?
> > > 
> > 
> > Ack, will look into.
> > 
> > > Maybe use _gloffset_AreTexturesResident (from dispatch.h) instead of 
> > > using a
> > > magic number?
> > > 
> > 
> > As the offending commit 99fee476a102 says "... can't use
> > src/mesa/main/dispatch.h ..."
> > 
> > At the same time, the function was moved outside of the python
> > generator(s) to address a out of bounds access - see commit
> > 13f96c5401f.
> > Sadly the same issue exists elsewhere - so I'm looking into addressing
> > that once and for all, and purging the handwritten function.
> > 
> > That's my evil plan ;-)
> 
> Emil, how's going your evil plan? :)
> 
>   J.A.
> 
> > 
> > -Emil
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
-- 
Br,

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

Re: [Mesa-dev] [Mesa-stable] [PATCH] meta: Fix BlitFramebuffer temp texture setup

[Andres Gomez]

Ville, this patch has been stalled in the -stable ML for quite some
time without update.

Unless you say otherwise, I will just ignore it at this point and trust
that you will also Cc -stable in the future, in case you come with
another version.

On Tue, 2017-09-26 at 18:49 +0200, Juan A. Suarez Romero wrote:
> On Tue, 2017-07-11 at 15:42 +0300, Ville Syrjälä wrote:
> > On Mon, Jul 10, 2017 at 11:42:18PM +0300, Andres Gomez wrote:
> > > Ville, has this patch fallen through the cracks ?
> > 
> > Nope. I've still been looking into the issue whenever I've had a
> > few minutes to spare. I think I have it more or les figured out
> > at this stage, but I'll need to respin this patch, and clean up
> > some further patches to fix this stuff properly for i915.
> > 
> 
> Any news on this patch?
> 
>   J.A.
> 
> > > 
> > > On Fri, 2017-06-23 at 14:58 +0300, ville.syrj...@linux.intel.com wrote:
> > > > From: Ville Syrjälä 
> > > > 
> > > > Pass the correct src coordinates to CopyTexSubImage()
> > > > when creating the temporary texture, and also take care to adjust
> > > > flipX/Y if the original src coordinates were flipped compared to
> > > > the new temporary texture src coordinates.
> > > > 
> > > > This fixes all the flip_src_x/y tests in
> > > > piglit.spec.arb_framebuffer_object.fbo-blit-stretch on i915, but
> > > > we're still left with the some failures in the stretch tests.
> > > > 
> > > > It looks to me like commit b702233f53d6 ("meta: Refactor the
> > > > BlitFramebuffer color CopyTexImage fallback.") most likely
> > > > broke this codepath.
> > > > 
> > > > Cc: mesa-sta...@lists.freedesktop.org
> > > > Cc: Eric Anholt 
> > > > Cc: Kenneth Graunke 
> > > > Cc: Ian Romanick 
> > > > Cc: Anuj Phogat 
> > > > Fixes: b702233f53d6 ("meta: Refactor the BlitFramebuffer color 
> > > > CopyTexImage fallback.")
> > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=101414
> > > > Signed-off-by: Ville Syrjälä 
> > > > ---
> > > >  src/mesa/drivers/common/meta_blit.c | 8 ++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/src/mesa/drivers/common/meta_blit.c 
> > > > b/src/mesa/drivers/common/meta_blit.c
> > > > index 7adad469aceb..7262ecdfaf13 100644
> > > > --- a/src/mesa/drivers/common/meta_blit.c
> > > > +++ b/src/mesa/drivers/common/meta_blit.c
> > > > @@ -680,12 +680,16 @@ blitframebuffer_texture(struct gl_context *ctx,
> > > >}
> > > >  
> > > >_mesa_meta_setup_copypix_texture(ctx, meta_temp_texture,
> > > > -   srcX0, srcY0,
> > > > +   MIN2(srcX0, srcX1),
> > > > +   MIN2(srcY0, srcY1),
> > > > srcW, srcH,
> > > > tex_base_format,
> > > > filter);
> > > >  
> > > > -
> > > > +  if (srcX0 > srcX1)
> > > > + flipX = -flipX;
> > > > +  if (srcY0 > srcY1)
> > > > + flipY = -flipY;
> > > >srcX0 = 0;
> > > >srcY0 = 0;
> > > >srcX1 = srcW;
> > > 
> > > -- 
> > > Br,
> > > 
> > > Andres
> > 
> > 
> 
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
-- 
Br,

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

Re: [Mesa-dev] [Mesa-stable] [PATCH 1/3] anv: Stop racing relocation offsets

[Andres Gomez]

Jason, this patch has been stalled in the -stable ML for quite some
time without update.

Unless you say otherwise, I will just ignore it at this point and trust
that you will also Cc -stable in the future, in case you come with
another version.

On Tue, 2017-09-26 at 12:31 +0200, Juan A. Suarez Romero wrote:
> On Mon, 2017-06-26 at 22:39 +0300, Andres Gomez wrote:
> > Jason, it doesn't seem like this patch has landed in master. Are you in
> > need of review or is it that this has been superseded?
> > 
> 
> 
> Gently ping to know what is the status for this patch.
> 
> Thaks in advance.
> 
> 
>   J.A.
> 
> > Thanks!
> > 
> > On Wed, 2017-05-10 at 16:08 -0700, Jason Ekstrand wrote:
> > > One of the key invariants of the relocation system is the
> > > presumed_offset field.  The assumption is made that the value currently
> > > in the address to be relocated agrees with the presumed_offset field.
> > > If presumed_offset is equal to the offset of the BO, the kernel will
> > > skip the relocation assuming that the value is already correct.
> > > 
> > > Our initial implementation of relocation handling had a race where we
> > > would read bo->offset once when we wrote the relocation entry and again
> > > when we filled out actual address.
> > > 
> > > Found with helgrind
> > > 
> > > Cc: "17.0 17.1" 
> > > ---
> > >  src/intel/vulkan/anv_batch_chain.c | 21 +
> > >  src/intel/vulkan/anv_private.h |  2 +-
> > >  src/intel/vulkan/genX_blorp_exec.c |  5 -
> > >  src/intel/vulkan/genX_cmd_buffer.c |  7 +--
> > >  4 files changed, 27 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/src/intel/vulkan/anv_batch_chain.c 
> > > b/src/intel/vulkan/anv_batch_chain.c
> > > index 9def174..13303b1 100644
> > > --- a/src/intel/vulkan/anv_batch_chain.c
> > > +++ b/src/intel/vulkan/anv_batch_chain.c
> > > @@ -143,7 +143,8 @@ anv_reloc_list_grow(struct anv_reloc_list *list,
> > >  VkResult
> > >  anv_reloc_list_add(struct anv_reloc_list *list,
> > > const VkAllocationCallbacks *alloc,
> > > -   uint32_t offset, struct anv_bo *target_bo, uint32_t 
> > > delta)
> > > +   uint32_t offset, struct anv_bo *target_bo, uint32_t 
> > > delta,
> > > +   uint64_t *bo_offset_out)
> > >  {
> > > struct drm_i915_gem_relocation_entry *entry;
> > > int index;
> > > @@ -155,6 +156,14 @@ anv_reloc_list_add(struct anv_reloc_list *list,
> > > if (result != VK_SUCCESS)
> > >return result;
> > >  
> > > +   /* Read the BO offset once.  This same value will be used in the 
> > > relocation
> > > +* entry and passed back to the caller for it to use when it writes 
> > > the
> > > +* actual value.  This guarantees that the two values match even if 
> > > there
> > > +* is a data race between now and when the caller gets around to 
> > > writing
> > > +* the address into the BO.
> > > +*/
> > > +   uint64_t presumed_offset = target_bo->offset;
> > > +
> > > /* XXX: Can we use I915_EXEC_HANDLE_LUT? */
> > > index = list->num_relocs++;
> > > list->reloc_bos[index] = target_bo;
> > > @@ -162,11 +171,13 @@ anv_reloc_list_add(struct anv_reloc_list *list,
> > > entry->target_handle = target_bo->gem_handle;
> > > entry->delta = delta;
> > > entry->offset = offset;
> > > -   entry->presumed_offset = target_bo->offset;
> > > +   entry->presumed_offset = presumed_offset;
> > > entry->read_domains = domain;
> > > entry->write_domain = domain;
> > > VG(VALGRIND_CHECK_MEM_IS_DEFINED(entry, sizeof(*entry)));
> > >  
> > > +   *bo_offset_out = presumed_offset;
> > > +
> > > return VK_SUCCESS;
> > >  }
> > >  
> > > @@ -218,14 +229,16 @@ uint64_t
> > >  anv_batch_emit_reloc(struct anv_batch *batch,
> > >   void *location, struct anv_bo *bo, uint32_t delta)
> > >  {
> > > +   uint64_t bo_offset;
> > > VkResult result = anv_reloc_list_add(batch->relocs, batch->alloc,
> > > -location - batch->start, bo, 
> > > delta);
> > > +location - batch->start, bo, 
> > > delta,
> > > +_offset);
> > > if (result != VK_SUCCESS) {
> > >anv_batch_set_error(batch, result);
> > >return 0;
> > > }
> > >  
> > > -   return bo->offset + delta;
> > > +   return bo_offset + delta;
> > >  }
> > >  
> > >  void
> > > diff --git a/src/intel/vulkan/anv_private.h 
> > > b/src/intel/vulkan/anv_private.h
> > > index 9b0dd67..1686da8 100644
> > > --- a/src/intel/vulkan/anv_private.h
> > > +++ b/src/intel/vulkan/anv_private.h
> > > @@ -825,7 +825,7 @@ void anv_reloc_list_finish(struct anv_reloc_list 
> > > *list,
> > >  VkResult anv_reloc_list_add(struct anv_reloc_list *list,
> > >  const VkAllocationCallbacks *alloc,
> > >  uint32_t offset, struct anv_bo *target_bo,
> > > -   

Re: [Mesa-dev] [PATCH v2] drirc: Allow extension midshader for Metro Redux

[Andres Gomez]

Vadym, should we also include this in the stable queues ?

On Mon, 2018-08-06 at 15:52 +0300, vadym.shovkoplias wrote:
> This fixes both Metro 2033 Redux and Metro Last Light Redux
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99730
> Signed-off-by: Eero Tamminen 
> Signed-off-by: Vadym Shovkoplias 
> ---
>  src/util/drirc | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/util/drirc b/src/util/drirc
> index 8ece875e34..c4f9e060f3 100644
> --- a/src/util/drirc
> +++ b/src/util/drirc
> @@ -120,6 +120,10 @@ TODO: document the other workarounds.
>   value="true" />
>  
>  
> + executable="metro">
> + value="true" />
> +
> +
>  
>  
>  
-- 
Br,

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

Re: [Mesa-dev] [PATCH 1/4] tegra: fix memory leak

[Emil Velikov]

On 9 August 2018 at 12:20, Christian Gmeiner
 wrote:
> Am Do., 9. Aug. 2018 um 12:23 Uhr schrieb Emil Velikov
> :
>>
>> On 9 August 2018 at 06:12, Christian Gmeiner
>>  wrote:
>> > Signed-off-by: Christian Gmeiner 
>> > ---
>> >  src/gallium/drivers/tegra/tegra_screen.c | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/src/gallium/drivers/tegra/tegra_screen.c 
>> > b/src/gallium/drivers/tegra/tegra_screen.c
>> > index 034ea271ee..361ec034de 100644
>> > --- a/src/gallium/drivers/tegra/tegra_screen.c
>> > +++ b/src/gallium/drivers/tegra/tegra_screen.c
>> > @@ -198,6 +198,7 @@ static int tegra_open_render_node(void)
>> >
>> >   version = drmGetVersion(fd);
>> >   if (!version) {
>> > +drmFreeVersion(version);
>> This should be in the next if hunk - the strcmp() call.
>
> Yeah... that is what I wanted initially - but hacking to early in the
> morning (and in the train)
> does not seems to be my thing.
>
You've done it correctly on 2/4 so I'd imagine a shaky train got to you.

With that fixed, this patch is
Fixes: 1755f608f52 ("tegra: Initial support")
Reviewed-by: Emil Velikov 

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

Re: [Mesa-dev] [PATCH] glsl: handle error case with ast_post_inc, ast_post_dec

[Andres Gomez]

Tapani, should we also include this in the stable queues ?

On Tue, 2018-08-07 at 08:20 +0300, Tapani Pälli wrote:
> Return ir_rvalue::error_value with ast_post_inc, ast_post_dec if
> parser error was emitted previously. This way process_array_size
> won't see bogus IR generated like with commit 9c676a64273.
> 
> Signed-off-by: Tapani Pälli 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98699
> ---
>  src/compiler/glsl/ast_to_hir.cpp | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/compiler/glsl/ast_to_hir.cpp 
> b/src/compiler/glsl/ast_to_hir.cpp
> index 74160ec142b..5d3f10b6823 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -1928,6 +1928,11 @@ ast_expression::do_hir(exec_list *instructions,
>  
>error_emitted = op[0]->type->is_error() || op[1]->type->is_error();
>  
> +  if (error_emitted) {
> + result = ir_rvalue::error_value(ctx);
> + break;
> +  }
> +
>type = arithmetic_result_type(op[0], op[1], false, state, & loc);
>  
>ir_rvalue *temp_rhs;
-- 
Br,

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

Re: [Mesa-dev] [PATCH 4/4] imx: make use of loader_open_render_node(..) helper

[Emil Velikov]

On 9 August 2018 at 06:12, Christian Gmeiner
 wrote:
> Gets rid of hard-coded gpu device path.
>
> Signed-off-by: Christian Gmeiner 
> ---
3/4 and 4/4 are
Reviewed-by: Emil Velikov 

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

Re: [Mesa-dev] [PATCH 2/4] loader: add loader_open_render_node(..)

[Emil Velikov]

On 9 August 2018 at 06:12, Christian Gmeiner
 wrote:
> This helper is almost a 1:1 copy of tegra_open_render_node().
>
> Signed-off-by: Christian Gmeiner 
> ---
>  src/loader/loader.c | 65 +
>  src/loader/loader.h |  3 +++
>  2 files changed, 68 insertions(+)
>
> diff --git a/src/loader/loader.c b/src/loader/loader.c
> index 43275484cc..60b5d71083 100644
> --- a/src/loader/loader.c
> +++ b/src/loader/loader.c
> @@ -83,6 +83,65 @@ loader_open_device(const char *device_name)
>  }
>
A few 'thinking out loud' comments:
 - worth keeping node type and bus type as arguments
 - using devices[64] will simplify/speed things up
 - s/open/loader_open_device/ to deal with funky O_CLOEXEC corner-cases

That can follow at a later stage. As-is
Reviewed-by: Emil Velikov 

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

Re: [Mesa-dev] [PATCH] intel/decoder: fix the possible out of bounds group_iter

[andrey simiklit]

Hi,

Sorry I missed the main thought here.
The "gen_group_get_length" function returns *int*
but the "iter_group_offset_bits" function returns *uint32_t*
So *uint32_t*(*int*(-32)) = *0xFFE0U* and it looks like unexpected
behavior for me:
iter_group_offset_bits(iter, iter->group_iter + 1) < *0xFFE0U*;

Regards,
Andrii.

On Thu, Aug 9, 2018 at 2:35 PM, Andrii Simiklit 
wrote:

> The "gen_group_get_length" function can return a negative value
> and it can lead to the out of bounds group_iter.
>
> Signed-off-by: Andrii Simiklit 
> ---
>  src/intel/common/gen_decoder.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder
> .c
> index ec0a486..f09bd87 100644
> --- a/src/intel/common/gen_decoder.c
> +++ b/src/intel/common/gen_decoder.c
> @@ -803,8 +803,10 @@ static bool
>  iter_more_groups(const struct gen_field_iterator *iter)
>  {
> if (iter->group->variable) {
> -  return iter_group_offset_bits(iter, iter->group_iter + 1) <
> -  (gen_group_get_length(iter->group, iter->p) * 32);
> +  const int length = gen_group_get_length(iter->group, iter->p);
> +  return length > 0 &&
> + iter_group_offset_bits(iter, iter->group_iter + 1) <
> +  (length * 32);
> } else {
>return (iter->group_iter + 1) < iter->group->group_count ||
>   iter->group->next != NULL;
> --
> 2.7.4
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 106394] Black Mesa cannot compile shaders because of missing GL_EXT_gpu_shader4

[bugzilla-daemon]

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

mirh  changed:

   What|Removed |Added

 CC||m...@protonmail.ch

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

[Mesa-dev] [PATCH] intel/decoder: fix the possible out of bounds group_iter

[Andrii Simiklit]

The "gen_group_get_length" function can return a negative value
and it can lead to the out of bounds group_iter.

Signed-off-by: Andrii Simiklit 
---
 src/intel/common/gen_decoder.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
index ec0a486..f09bd87 100644
--- a/src/intel/common/gen_decoder.c
+++ b/src/intel/common/gen_decoder.c
@@ -803,8 +803,10 @@ static bool
 iter_more_groups(const struct gen_field_iterator *iter)
 {
if (iter->group->variable) {
-  return iter_group_offset_bits(iter, iter->group_iter + 1) <
-  (gen_group_get_length(iter->group, iter->p) * 32);
+  const int length = gen_group_get_length(iter->group, iter->p);
+  return length > 0 &&
+ iter_group_offset_bits(iter, iter->group_iter + 1) <
+  (length * 32);
} else {
   return (iter->group_iter + 1) < iter->group->group_count ||
  iter->group->next != NULL;
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH 1/4] tegra: fix memory leak

[Christian Gmeiner]

Am Do., 9. Aug. 2018 um 12:23 Uhr schrieb Emil Velikov
:
>
> On 9 August 2018 at 06:12, Christian Gmeiner
>  wrote:
> > Signed-off-by: Christian Gmeiner 
> > ---
> >  src/gallium/drivers/tegra/tegra_screen.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/src/gallium/drivers/tegra/tegra_screen.c 
> > b/src/gallium/drivers/tegra/tegra_screen.c
> > index 034ea271ee..361ec034de 100644
> > --- a/src/gallium/drivers/tegra/tegra_screen.c
> > +++ b/src/gallium/drivers/tegra/tegra_screen.c
> > @@ -198,6 +198,7 @@ static int tegra_open_render_node(void)
> >
> >   version = drmGetVersion(fd);
> >   if (!version) {
> > +drmFreeVersion(version);
> This should be in the next if hunk - the strcmp() call.

Yeah... that is what I wanted initially - but hacking to early in the
morning (and in the train)
does not seems to be my thing.

-- 
greets
--
Christian Gmeiner, MSc

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

[Mesa-dev] [Bug 107533] Please restore --with-{gl, osmesa}-lib-name options

[bugzilla-daemon]

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

--- Comment #1 from Brad King  ---
See also commit 27382c0f7ba2ae826531ba4c254741b2a9df1882.

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

[Mesa-dev] [Bug 107533] Please restore --with-{gl, osmesa}-lib-name options

[bugzilla-daemon]

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

Bug ID: 107533
   Summary: Please restore --with-{gl, osmesa}-lib-name options
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Other
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: brad.k...@kitware.com
QA Contact: mesa-dev@lists.freedesktop.org

Commit d5ac23647110fd530f9bf5002762587be446866d removed these options claiming
that they were broken and used only in association with mangling symbols.  This
is incorrect.  I added the options in commit
f3cdcb839f534a3062864b06ec6689717ed102a1 with a commit message explaining they
are not associated with symbol mangling.  I've been using the options for years
and they work well.

The suggested alternative of renaming libraries manually is not as easy as
claimed.  The reason for mangling the library name is so that the SONAME field
is not just 'libGL.so.1'.  The goal is to ensure that the mesa library is used
at runtime and not some other libGL by accident.  A simple file rename does not
change the soname.

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

Re: [Mesa-dev] [PATCH 0/4] Merge vulkan API generators.

[Tapani Pälli]

On 08/08/2018 06:43 PM, Tapani Pälli wrote:



On 08.08.2018 17:31, Lionel Landwerlin wrote:

On 08/08/18 12:05, Lionel Landwerlin wrote:

On 08/08/18 00:14, Bas Nieuwenhuizen wrote:

radv was always just mirroring a derived version of the anv
version, sometimes hacked together and sometimes very behind.

As we grow more vulkan drivers this repetition makes even less
sense, so lets merge them. I took the anv generators as the
template and made radv use them.

This includes some messy stuff in the build system due to
difficulties with python includes. I tested with meson and
autotools. Android.mk is updated but not tested.


We have an android build target in our CI. I can give this series a go.

-
Lionel


Oh my bad, we don't actually have the Vulkan driver on Android as a 
target (just GL atm).




I can try building with these changes on Android tomorrow.



I did this and anv builds and works fine on Android with these changes.

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

[Mesa-dev] [PATCH] mesa/st: ETC2 now uses R8G8B8A8_SRGB as fallback

[Gert Wollny]

The check for ETC2 compatibility was not updated when the fallback
format was changed.

Fixes: 71867a0a61cea20bf3f6115692e70b0d60f0b70d
   st/mesa: Fall back to R8G8B8A8_SRGB for ETC2

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

diff --git a/src/mesa/state_tracker/st_extensions.c 
b/src/mesa/state_tracker/st_extensions.c
index 1c01495e93..db08d5169a 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -1318,7 +1318,7 @@ void st_init_extensions(struct pipe_screen *screen,
screen->is_format_supported(screen, PIPE_FORMAT_R8G8B8A8_UNORM,
PIPE_TEXTURE_2D, 0, 0,
PIPE_BIND_SAMPLER_VIEW) &&
-   screen->is_format_supported(screen, PIPE_FORMAT_B8G8R8A8_SRGB,
+   screen->is_format_supported(screen, PIPE_FORMAT_R8G8B8A8_SRGB,
PIPE_TEXTURE_2D, 0, 0,
PIPE_BIND_SAMPLER_VIEW) &&
screen->is_format_supported(screen, PIPE_FORMAT_R16_UNORM,
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH] i965: Add a new CFL PCI ID.

[Lionel Landwerlin]

Reviewed-by: Lionel Landwerlin 

On 09/08/18 06:44, Rodrigo Vivi wrote:

One more CFL ID added to spec.

Align with kernel commit d0e062ebb3a4 ("drm/i915/cfl:
Add a new CFL PCI ID.")

Cc: José Roberto de Souza 
Cc: Anuj Phogat 
Signed-off-by: Rodrigo Vivi 
---
  include/pci_ids/i965_pci_ids.h | 1 +
  1 file changed, 1 insertion(+)

diff --git a/include/pci_ids/i965_pci_ids.h b/include/pci_ids/i965_pci_ids.h
index 82e4a549e0..bced44e288 100644
--- a/include/pci_ids/i965_pci_ids.h
+++ b/include/pci_ids/i965_pci_ids.h
@@ -174,6 +174,7 @@ CHIPSET(0x3EA4, cfl_gt1, "Intel(R) HD Graphics (Coffeelake 2x6 
GT1)")
  CHIPSET(0x3E91, cfl_gt2, "Intel(R) UHD Graphics 630 (Coffeelake 3x8 GT2)")
  CHIPSET(0x3E92, cfl_gt2, "Intel(R) UHD Graphics 630 (Coffeelake 3x8 GT2)")
  CHIPSET(0x3E96, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
+CHIPSET(0x3E98, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
  CHIPSET(0x3E9A, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
  CHIPSET(0x3E9B, cfl_gt2, "Intel(R) UHD Graphics 630 (Coffeelake 3x8 GT2)")
  CHIPSET(0x3E94, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")



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

Re: [Mesa-dev] [PATCH 1/4] tegra: fix memory leak

[Emil Velikov]

On 9 August 2018 at 06:12, Christian Gmeiner
 wrote:
> Signed-off-by: Christian Gmeiner 
> ---
>  src/gallium/drivers/tegra/tegra_screen.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/gallium/drivers/tegra/tegra_screen.c 
> b/src/gallium/drivers/tegra/tegra_screen.c
> index 034ea271ee..361ec034de 100644
> --- a/src/gallium/drivers/tegra/tegra_screen.c
> +++ b/src/gallium/drivers/tegra/tegra_screen.c
> @@ -198,6 +198,7 @@ static int tegra_open_render_node(void)
>
>   version = drmGetVersion(fd);
>   if (!version) {
> +drmFreeVersion(version);
This should be in the next if hunk - the strcmp() call.

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

[Mesa-dev] [AppVeyor] mesa master #8562 completed

[AppVeyor]

Build mesa 8562 completed



Commit e0de26eacc by vadym.shovkoplias on 8/6/2018 12:52 PM:

drirc: Allow extension midshader for Metro Redux\n\nThis fixes both Metro 2033 Redux and Metro Last Light Redux\n\nBugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99730\nSigned-off-by: Eero Tamminen \nSigned-off-by: Vadym Shovkoplias \nReviewed-by: Tapani Pälli 


Configure your notification preferences

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

Re: [Mesa-dev] [PATCH] intel: Switch the order of the 2x MSAA sample positions

[Iago Toral]

Reviewed-by: Iago Toral Quiroga 

On Wed, 2018-08-08 at 11:30 -0700, Jason Ekstrand wrote:
> The Vulkan 1.1.82 spec flipped the order to better match D3D.
> 
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/intel/blorp/blorp_blit.c   | 11 ++-
>  src/intel/common/gen_sample_positions.h|  8 
>  src/mesa/drivers/dri/i965/brw_multisample_state.h  |  8 
>  src/mesa/drivers/dri/i965/gen6_multisample_state.c |  4 ++--
>  4 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/src/intel/blorp/blorp_blit.c
> b/src/intel/blorp/blorp_blit.c
> index 561897894c3..013f7a14fa2 100644
> --- a/src/intel/blorp/blorp_blit.c
> +++ b/src/intel/blorp/blorp_blit.c
> @@ -776,6 +776,13 @@ blorp_nir_manual_blend_bilinear(nir_builder *b,
> nir_ssa_def *pos,
> * grid of samples with in a pixel. Sample number layout shows
> the
> * rectangular grid of samples roughly corresponding to the
> real sample
> * locations with in a pixel.
> +   *
> +   * In the case of 2x MSAA, the layout of sample indices is
> reversed from
> +   * the layout of sample numbers:
> +   *   -
> +   *   | 1 | 0 |
> +   *   -
> +   *
> * In case of 4x MSAA, layout of sample indices matches the
> layout of
> * sample numbers:
> *   -
> @@ -819,7 +826,9 @@ blorp_nir_manual_blend_bilinear(nir_builder *b,
> nir_ssa_def *pos,
>  key->x_scale * key-
> >y_scale));
>sample = nir_f2i32(b, sample);
>  
> -  if (tex_samples == 8) {
> +  if (tex_samples == 2) {
> + sample = nir_isub(b, nir_imm_int(b, 1), sample);
> +  } else if (tex_samples == 8) {
>   sample = nir_iand(b, nir_ishr(b, nir_imm_int(b,
> 0x64210573),
> nir_ishl(b, sample,
> nir_imm_int(b, 2))),
> nir_imm_int(b, 0xf));
> diff --git a/src/intel/common/gen_sample_positions.h
> b/src/intel/common/gen_sample_positions.h
> index f0ce95dd1fb..da48dcb5ed0 100644
> --- a/src/intel/common/gen_sample_positions.h
> +++ b/src/intel/common/gen_sample_positions.h
> @@ -42,10 +42,10 @@ prefix##0YOffset   = 0.5;
>   * c   1
>   */
>  #define GEN_SAMPLE_POS_2X(prefix) \
> -prefix##0XOffset   = 0.25; \
> -prefix##0YOffset   = 0.25; \
> -prefix##1XOffset   = 0.75; \
> -prefix##1YOffset   = 0.75;
> +prefix##0XOffset   = 0.75; \
> +prefix##0YOffset   = 0.75; \
> +prefix##1XOffset   = 0.25; \
> +prefix##1YOffset   = 0.25;
>  
>  /**
>   * Sample positions:
> diff --git a/src/mesa/drivers/dri/i965/brw_multisample_state.h
> b/src/mesa/drivers/dri/i965/brw_multisample_state.h
> index 6cf324e561c..2142a17a484 100644
> --- a/src/mesa/drivers/dri/i965/brw_multisample_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_multisample_state.h
> @@ -38,13 +38,13 @@
>  /**
>   * 1x MSAA has a single sample at the center: (0.5, 0.5) -> (0x8,
> 0x8).
>   *
> - * 2x MSAA sample positions are (0.25, 0.25) and (0.75, 0.75):
> + * 2x MSAA sample positions are (0.75, 0.75) and (0.25, 0.25):
>   *   4 c
> - * 4 0
> - * c   1
> + * 4 1
> + * c   0
>   */
>  static const uint32_t
> -brw_multisample_positions_1x_2x = 0x0088cc44;
> +brw_multisample_positions_1x_2x = 0x008844cc;
>  
>  /**
>   * Sample positions:
> diff --git a/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> b/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> index bfa84fb9b77..78ff3942075 100644
> --- a/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> @@ -70,7 +70,7 @@ gen6_get_sample_position(struct gl_context *ctx,
>   *
>   * 2X MSAA sample index / number layout
>   *   -
> - *   | 0 | 1 |
> + *   | 1 | 0 |
>   *   -
>   *
>   * 4X MSAA sample index / number layout
> @@ -107,7 +107,7 @@ gen6_get_sample_position(struct gl_context *ctx,
>  void
>  gen6_set_sample_maps(struct gl_context *ctx)
>  {
> -   uint8_t map_2x[2] = {0, 1};
> +   uint8_t map_2x[2] = {1, 0};
> uint8_t map_4x[4] = {0, 1, 2, 3};
> uint8_t map_8x[8] = {3, 7, 5, 0, 1, 2, 4, 6};
> uint8_t map_16x[16] = { 15, 10, 9, 7, 4, 1, 3, 13,
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev