Re: [Mesa-dev] last call for autotools

[Rhys Kidd]

On Tue, 11 Dec 2018 at 16:46, Emil Velikov  wrote:

> On Tue, 11 Dec 2018 at 19:49, Dylan Baker  wrote:
> >
> > Quoting Emil Velikov (2018-12-11 10:48:56)
> > > On Tue, 11 Dec 2018 at 18:18, Eric Anholt  wrote:
> > > >
> > > > Emil Velikov  writes:
> > > >
> > > > > On Mon, 10 Dec 2018 at 23:11, Dylan Baker 
> wrote:
> > > > >>
> > > > >> Meson 0.49.0 has been out for a couple of days now, and I'd like
> to make the
> > > > >> final call for autotools. My patch is so massive that it's a huge
> pain to send
> > > > >> to the list, the latest versions is here:
> > > > >>
> https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools
> > > > >>
> > > > > Can you split this up a bit? I'm mostly conserved about a couple
> of things:
> > > > > - we're loosing multiple testing permutations with Travis -
> there's no
> > > > > meson equivalent
> > > > > - needed/missing bits are impossible to spot as-is
> > > > >
> > > > > Personally, I'd do something like:
> > > > >  - move .travis autotools permutations to meson
> > > > >  - docs changes
> > > > >  - rm Makefile{.*,}.am autogen.sh and configure.ac
> > > > >  - rm Makefile.sources
> > > > >  - .gitignore
> > > >
> > > > I don't think we should block on anything related to travis at this
> > > > point.  I wrote it initially hoping it would let us prevent build
> > > > breakages.  Instead, whenever I try to use travis to prevent build
> > > > breakage, I find that travis has already been broken for a while.
> > > >
> > > You must be really unlucky.
> > >
> > > > The CI needs to be integrated with our actual repository host, or it
> > > > doesn't work.  We should just delete .travis.yml and push the gitlab
> > > > stuff forward instead.
> > >
> > > If we have anything better in place (say gitlab CI/other) - sure.
> > > Removing something without a replacement in place is quite meh.
> > > That said, I think we agree that this is a topic for another day.
> > >
> > > HTH
> > > -Emil
> >
> > I'm actually finding myself agreeing that travis is just a pain. Could
> you help
> > me debug my WIP of porting the autotools tests to meson? They're just
> failing
> > randomly with no good debuging output:
> >
> > https://travis-ci.org/dcbaker/mesa/jobs/45289
> >
> If you don't mind I'll have a look first thing tomorrow morning. It's
> 9:43pm and I've yet to cook some food :-\
>

I was able to catch and fix most of the remaining meson / travis build
errors. RFC patch series on the mailing list that applies on top of Dylan's
WIP branch.

Hope that helps!


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

[Mesa-dev] [RFC PATCH 3/5] travis: radeonsi and radv require LLVM 7.0 (meson)

[Rhys Kidd]

Signed-off-by: Rhys Kidd 
---
 .travis.yml | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 60ef236e7be..070c9ef6773 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -81,20 +81,21 @@ matrix:
 - VULKAN_DRIVERS=""
 - 
GALLIUM_DRIVERS="swrast,swr,r300,r600,radeonsi,nouveau,i915,vc4,v3d,pl111,freedreno,etnaviv,imx,tegra,svga,virgl"
 - MESON_OPTIONS="-Dgallium-opencl=standalone -Dgallium-nine=true"
-- LLVM_VERSION=6.0
+- LLVM_VERSION=7
 - LLVM_CONFIG="llvm-config-${LLVM_VERSION}"
   addons:
 apt:
   sources:
-- llvm-toolchain-trusty-6.0
-# llvm-6 requires libstdc++4.9 which is not in main repo
+- sourceline: 'deb http://apt.llvm.org/trusty/ 
llvm-toolchain-trusty-7 main'
+  key_url: https://apt.llvm.org/llvm-snapshot.gpg.key
+# llvm-7 requires libstdc++4.9 which is not in main repo
 - ubuntu-toolchain-r-test
   packages:
 - libclc-dev
 # From sources above
-- llvm-6.0-dev
-- clang-6.0
-- libclang-6.0-dev
+- llvm-7-dev
+- clang-7
+- libclang-7-dev
 # Common
 - xz-utils
 - libexpat1-dev
-- 
2.19.1

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

[Mesa-dev] [RFC PATCH 5/5] meson: libfreedreno depends upon libdrm (for fence support)

[Rhys Kidd]

Error message building freedreno Gallium driver with meson:

  ../src/gallium/drivers/freedreno/freedreno_fence.c:27:21: fatal error: 
libsync.h: No such file or directory
   \#include 

Signed-off-by: Rhys Kidd 
---
 src/gallium/drivers/freedreno/meson.build | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/gallium/drivers/freedreno/meson.build 
b/src/gallium/drivers/freedreno/meson.build
index 366b1468bef..df3c743d41d 100644
--- a/src/gallium/drivers/freedreno/meson.build
+++ b/src/gallium/drivers/freedreno/meson.build
@@ -232,9 +232,7 @@ libfreedreno = static_library(
   include_directories : freedreno_includes,
   c_args : [freedreno_c_args, c_vis_args],
   cpp_args : [freedreno_cpp_args, cpp_vis_args],
-  dependencies : [
-idep_nir_headers
-  ],
+  dependencies : [dep_libdrm, idep_nir_headers],
 )
 
 driver_freedreno = declare_dependency(
-- 
2.19.1

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

[Mesa-dev] [RFC PATCH 2/5] travis: Add libclc-dev, clang and libclang-dev dependencies of clover

[Rhys Kidd]

Fixes: 98a3f027aaf ("travis: enable nine and clover")
Signed-off-by: Rhys Kidd 
---
 .travis.yml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index aec1b001083..60ef236e7be 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -90,8 +90,11 @@ matrix:
 # llvm-6 requires libstdc++4.9 which is not in main repo
 - ubuntu-toolchain-r-test
   packages:
+- libclc-dev
 # From sources above
 - llvm-6.0-dev
+- clang-6.0
+- libclang-6.0-dev
 # Common
 - xz-utils
 - libexpat1-dev
-- 
2.19.1

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

[Mesa-dev] [RFC PATCH 1/5] travis: fix python3.5 dependency for "meson Gallium Drivers"

[Rhys Kidd]

Error message:

  $ if test "x$BUILD" = xmeson; then sudo update-alternatives --install 
/usr/bin/python3 python3 /usr/bin/python3.5 10; pip3 install --user meson; pip3 
install --user mako; fi
  update-alternatives: error: alternative path /usr/bin/python3.5 doesn't exist
  Downloading/unpacking meson
Downloading meson-0.49.0.tar.gz (1.3MB): 1.3MB downloaded
Running setup.py (path:/tmp/pip_build_travis/meson/setup.py) egg_info for 
package meson
  Tried to install with an unsupported version of Python. Meson requires 
Python 3.5.0 or greater
  Complete output from command python setup.py egg_info:
  Tried to install with an unsupported version of Python. Meson requires 
Python 3.5.0 or greater
  
  Cleaning up...
  Command python setup.py egg_info failed with error code 1 in 
/tmp/pip_build_travis/meson
  Storing debug log for failure in /home/travis/.pip/pip.log

Fixes: 05edf75f42e ("wip")
Signed-off-by: Rhys Kidd 
---
 .travis.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.travis.yml b/.travis.yml
index fb86418b2b0..aec1b001083 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -98,6 +98,7 @@ matrix:
 - libx11-xcb-dev
 - libelf-dev
 - libunwind8-dev
+- python3.5
 - python3-pip
 
 before_install:
-- 
2.19.1

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

[Mesa-dev] [RFC PATCH 0/5] last call for autotools (meson travis fixes)

[Rhys Kidd]

Emil and Dylan,

I took a go at addressing the limited number of remaining meson-based
travis-ci errors. This series applies on top of the work Dylan circulated
yesterday, and which can be seen here:
 
>  Could you help me debug my WIP of porting the autotools tests
>  to meson? They're just failing randomly with no good debuging output:
>
>  https://travis-ci.org/dcbaker/mesa/jobs/45289

Dylan, if you pick these patches up you may want to squash those ones which
are 'Fixes' into the respective initial patch before any final push.

At this stage the failures are deterministic.

The remaining two known issues are:

  * -Dbuild-tests=true: anv is unable to build when this option is enabled.
This is unrelated to this series and occurs in master. At present meson
by default has different test coverage to autotools. See further here:
https://travis-ci.org/Echelon9/mesa/builds/466840665

  * gallium pipeloaders: There's an ongoing scoping problem with the LLVM
libraries as a dependency. I was unable to fix yet. See further here:
https://travis-ci.org/Echelon9/mesa/jobs/466861661#L9112   

Rhys Kidd (5):
  travis: fix python3.5 dependency for "meson Gallium Drivers"
  travis: Add libclc-dev, clang and libclang-dev dependencies of clover
  travis: radeonsi and radv require LLVM 7.0 (meson)
  travis: delete remaining scons residuals
  meson: libdfreedreno depends upon libdrm (for fence support)

 .travis.yml   | 25 ---
 src/gallium/drivers/freedreno/meson.build |  4 +---
 2 files changed, 10 insertions(+), 19 deletions(-)

-- 
2.19.1

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

[Mesa-dev] [RFC PATCH 4/5] travis: delete remaining scons residuals

[Rhys Kidd]

Fixes: 53d5129ded5 ("delete me: remove scons")
Signed-off-by: Rhys Kidd 
---
 .travis.yml | 12 
 1 file changed, 12 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 070c9ef6773..442afea3a74 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -138,12 +138,6 @@ install:
   pip3 install --user mako;
 fi
 
-  # Install a more modern scons from pip.
-  - if test "x$BUILD" = xscons; then
-  pip2 install --user "scons>=2.4";
-  pip2 install --user mako;
-fi
-
   # Install dependencies where we require specific versions (or where
   # disallowed by Travis CI's package whitelisting).
 
@@ -247,12 +241,6 @@ install:
 fi
 
 script:
-  - if test "x$BUILD" = xscons; then
-  test -n "$OVERRIDE_CC" && export CC="$OVERRIDE_CC";
-  test -n "$OVERRIDE_CXX" && export CXX="$OVERRIDE_CXX";
-  scons $SCONS_TARGET && eval $SCONS_CHECK_COMMAND;
-fi
-
   - |
 if test "x$BUILD" = xmeson; then
   MESON_OPTIONS="$MESON_OPTIONS -Dbuild-tests=true"
-- 
2.19.1

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

Re: [Mesa-dev] [PATCH v3] mesa: Fix GLES2 OES float texture framebuffer rendering.

[Tapani Pälli]

On 12/12/18 8:42 AM, Tapani Pälli wrote:



On 12/12/18 5:05 AM, Nick Kreeger wrote:

This change enables GLES2 to render float/half-float textures to a
framebuffer when the appropriate OES extensions are available.

This commit regressed OES GLES2 float texture rendering:
https://gitlab.freedesktop.org/mesa/mesa/commit/e333035c47a6a4cc88f0f9ca2bced500538bebae 



Which test/app got regressed? I'm curious because this commit also fixed 
a failing conformance test that explicitly tests that the fbo with float 
attachment should be marked incomplete.




I'm against this change because these same tests will start to fail if 
we allow this:


https://mesa-ci.01.org/tpalli/builds/652/group/63a9f0ea7bb98050796b649e85481845

I believe the rule is that you should be able to render to sized formats 
such as RGBA32F (with modern GLES) but not to unsized formats such as 
GL_FLOAT.




---
  src/mesa/main/fbobject.c | 20 +++-
  1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 23e4939619..cedfc3d81b 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -869,15 +869,17 @@ test_attachment_completeness(const struct 
gl_context *ctx, GLenum format,

  return;
   }
- /* OES_texture_float allows creation and use of floating point
-  * textures with GL_FLOAT, GL_HALF_FLOAT but it does not allow
-  * these textures to be used as a render target, this is 
done via
-  * GL_EXT_color_buffer(_half)_float with set of new sized 
types.

-  */
- if (_mesa_is_gles(ctx) && (texObj->_IsFloat || 
texObj->_IsHalfFloat)) {

-    att_incomplete("bad internal format");
-    att->Complete = GL_FALSE;
-    return;
+ if (_mesa_is_gles(ctx)) {
+   /**
+    * GL ES 2 will allow GL_FLOAT and GL_HALF_FLOAT to render 
as a
+    * target when the appropriate OES_* extensions are 
available.

+    */
+   if ((texObj->_IsHalfFloat && 
!_mesa_has_OES_texture_half_float(ctx)) ||
+   (texObj->_IsFloat && 
!_mesa_has_OES_texture_float(ctx))) {

+ att_incomplete("bad internal format");
+ att->Complete = GL_FALSE;
+ return;
+   }
   }
    }
    else if (format == GL_DEPTH) {


___
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 01/10] i965: Move down genX_upload_sbe in profiles.

[Mathias . Froehlich]

From: Mathias Fröhlich 

Avoid looping over all VARYING_SLOT_MAX urb_setup array
entries from genX_upload_sbe. Prepare an array indirection
to the active entries of urb_setup already in the compile
step. On upload only walk the active arrays.

v2: Use uint8_t to store the attribute numbers.

Signed-off-by: Mathias Fröhlich 
---
 src/intel/compiler/brw_compiler.h |  7 ++
 src/intel/compiler/brw_fs.cpp | 25 +++
 src/intel/compiler/brw_fs.h   |  2 ++
 src/intel/compiler/brw_fs_visitor.cpp |  1 +
 src/mesa/drivers/dri/i965/genX_state_upload.c |  7 +++---
 5 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/src/intel/compiler/brw_compiler.h 
b/src/intel/compiler/brw_compiler.h
index e4f4d83c8e..427a61fd70 100644
--- a/src/intel/compiler/brw_compiler.h
+++ b/src/intel/compiler/brw_compiler.h
@@ -751,6 +751,13 @@ struct brw_wm_prog_data {
 * For varying slots that are not used by the FS, the value is -1.
 */
int urb_setup[VARYING_SLOT_MAX];
+   /**
+* Cache structure into the urb_setup array above that contains the
+* attribute numbers of active varyings out of urb_setup.
+* The actual count is stored in urb_setup_attribs_count.
+*/
+   uint8_t urb_setup_attribs[VARYING_SLOT_MAX];
+   uint8_t urb_setup_attribs_count;
 };
 
 /** Returns the SIMD width corresponding to a given KSP index
diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
index 3125e5feb1..db76462ba7 100644
--- a/src/intel/compiler/brw_fs.cpp
+++ b/src/intel/compiler/brw_fs.cpp
@@ -1541,6 +1541,27 @@ fs_visitor::assign_curb_setup()
this->first_non_payload_grf = payload.num_regs + 
prog_data->curb_read_length;
 }
 
+/*
+ * Build up an array of indices into the urb_setup array that
+ * references the active entries of the urb_setup array.
+ * Used to accelerate walking the active entries of the urb_setup array
+ * on each upload.
+ */
+void
+brw_compute_urb_setup_index(struct brw_wm_prog_data *wm_prog_data)
+{
+   /* Make sure uint8_t is sufficient */
+   STATIC_ASSERT(VARYING_SLOT_MAX <= 0xff);
+   uint8_t index = 0;
+   for (uint8_t attr = 0; attr < VARYING_SLOT_MAX; attr++) {
+  int input_index = wm_prog_data->urb_setup[attr];
+  if (input_index < 0)
+ continue;
+  wm_prog_data->urb_setup_attribs[index++] = attr;
+   }
+   wm_prog_data->urb_setup_attribs_count = index;
+}
+
 void
 fs_visitor::calculate_urb_setup()
 {
@@ -1629,6 +1650,8 @@ fs_visitor::calculate_urb_setup()
}
 
prog_data->num_varying_inputs = urb_next;
+
+   brw_compute_urb_setup_index(prog_data);
 }
 
 void
@@ -6792,6 +6815,8 @@ gen9_ps_header_only_workaround(struct brw_wm_prog_data 
*wm_prog_data)
 
wm_prog_data->urb_setup[VARYING_SLOT_LAYER] = 0;
wm_prog_data->num_varying_inputs = 1;
+
+   brw_compute_urb_setup_index(wm_prog_data);
 }
 
 bool
diff --git a/src/intel/compiler/brw_fs.h b/src/intel/compiler/brw_fs.h
index 163c000882..7d3b271837 100644
--- a/src/intel/compiler/brw_fs.h
+++ b/src/intel/compiler/brw_fs.h
@@ -556,4 +556,6 @@ fs_reg setup_imm_ub(const brw::fs_builder &bld,
 enum brw_barycentric_mode brw_barycentric_mode(enum glsl_interp_mode mode,
nir_intrinsic_op op);
 
+void brw_compute_urb_setup_index(struct brw_wm_prog_data *wm_prog_data);
+
 #endif /* BRW_FS_H */
diff --git a/src/intel/compiler/brw_fs_visitor.cpp 
b/src/intel/compiler/brw_fs_visitor.cpp
index 51a0ca2374..510f0cac47 100644
--- a/src/intel/compiler/brw_fs_visitor.cpp
+++ b/src/intel/compiler/brw_fs_visitor.cpp
@@ -120,6 +120,7 @@ fs_visitor::emit_dummy_fs()
wm_prog_data->num_varying_inputs = devinfo->gen < 6 ? 1 : 0;
memset(wm_prog_data->urb_setup, -1,
   sizeof(wm_prog_data->urb_setup[0]) * VARYING_SLOT_MAX);
+   brw_compute_urb_setup_index(wm_prog_data);
 
/* We don't have any uniforms. */
stage_prog_data->nr_params = 0;
diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c 
b/src/mesa/drivers/dri/i965/genX_state_upload.c
index 8e3fcbf12e..f99dc2f206 100644
--- a/src/mesa/drivers/dri/i965/genX_state_upload.c
+++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
@@ -1168,12 +1168,11 @@ genX(calculate_attr_overrides)(const struct brw_context 
*brw,
 * BRW_NEW_PRIMITIVE | BRW_NEW_GS_PROG_DATA | BRW_NEW_TES_PROG_DATA
 */
bool drawing_points = brw_is_drawing_points(brw);
-
-   for (int attr = 0; attr < VARYING_SLOT_MAX; attr++) {
+   for (uint8_t index = 0; index < wm_prog_data->urb_setup_attribs_count; 
index++) {
+  uint8_t attr = wm_prog_data->urb_setup_attribs[index];
   int input_index = wm_prog_data->urb_setup[attr];
 
-  if (input_index < 0)
- continue;
+  assert(0 <= input_index);
 
   /* _NEW_POINT */
   bool point_sprite = false;
-- 
2.19.2

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

[Mesa-dev] [PATCH 03/10] i965: Split merge_inputs and clear_buffers.

[Mathias . Froehlich]

From: Mathias Fröhlich 

The merge_inputs function handles that part that changes when the
inputs change. The clear_buffers function triggers when we may need
a new upload. Thus the merge_inputs can be limited to be once
per brw_draw_prims.

Signed-off-by: Mathias Fröhlich 
---
 src/mesa/drivers/dri/i965/brw_draw.c | 30 
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_draw.c 
b/src/mesa/drivers/dri/i965/brw_draw.c
index b818a0d77a..9826bc8761 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -278,21 +278,34 @@ brw_emit_prim(struct brw_context *brw,
 
 
 static void
-brw_merge_inputs(struct brw_context *brw)
+brw_clear_buffers(struct brw_context *brw)
 {
-   const struct gen_device_info *devinfo = &brw->screen->devinfo;
-   const struct gl_context *ctx = &brw->ctx;
-   GLuint i;
-
-   for (i = 0; i < brw->vb.nr_buffers; i++) {
+   for (unsigned i = 0; i < brw->vb.nr_buffers; ++i) {
   brw_bo_unreference(brw->vb.buffers[i].bo);
   brw->vb.buffers[i].bo = NULL;
}
brw->vb.nr_buffers = 0;
 
+   for (unsigned i = 0; i < brw->vb.nr_enabled; ++i) {
+  brw->vb.enabled[i]->buffer = -1;
+   }
+#ifndef NDEBUG
+   for (unsigned i = 0; i < VERT_ATTRIB_MAX; i++) {
+  assert(brw->vb.inputs[i].buffer == -1);
+   }
+#endif
+}
+
+
+static void
+brw_merge_inputs(struct brw_context *brw)
+{
+   const struct gen_device_info *devinfo = &brw->screen->devinfo;
+   const struct gl_context *ctx = &brw->ctx;
+   GLuint i;
+
for (i = 0; i < VERT_ATTRIB_MAX; i++) {
   struct brw_vertex_element *input = &brw->vb.inputs[i];
-  input->buffer = -1;
   _mesa_draw_attrib_and_binding(ctx, i,
 &input->glattrib, &input->glbinding);
}
@@ -843,6 +856,7 @@ brw_prepare_drawing(struct gl_context *ctx,
 
/* Bind all inputs, derive varying and size information:
 */
+   brw_clear_buffers(brw);
brw_merge_inputs(brw);
 
brw->ib.ib = ib;
@@ -908,7 +922,7 @@ brw_draw_single_prim(struct gl_context *ctx,
   brw->baseinstance = prim->base_instance;
   if (prim_id > 0) { /* For i == 0 we just did this before the loop */
  brw->ctx.NewDriverState |= BRW_NEW_VERTICES;
- brw_merge_inputs(brw);
+ brw_clear_buffers(brw);
   }
}
 
-- 
2.19.2

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

[Mesa-dev] [PATCH 06/10] mesa: Remove now unused _mesa_draw_attrib_and_binding.

[Mathias . Froehlich]

From: Mathias Fröhlich 

Signed-off-by: Mathias Fröhlich 
---
 src/mesa/main/arrayobj.h | 19 ---
 1 file changed, 19 deletions(-)

diff --git a/src/mesa/main/arrayobj.h b/src/mesa/main/arrayobj.h
index ee87b4b6ba..49b3522f30 100644
--- a/src/mesa/main/arrayobj.h
+++ b/src/mesa/main/arrayobj.h
@@ -313,25 +313,6 @@ _mesa_draw_attrib(const struct gl_context *ctx, 
gl_vert_attrib attr)
 }
 
 
-/**
- * Return the attrib, binding pair for the given attribute.
- */
-static inline void
-_mesa_draw_attrib_and_binding(const struct gl_context *ctx, gl_vert_attrib 
attr,
-  const struct gl_array_attributes **attrib,
-  const struct gl_vertex_buffer_binding **binding)
-{
-   if (ctx->Array._DrawVAOEnabledAttribs & VERT_BIT(attr)) {
-  const struct gl_vertex_array_object *vao = ctx->Array._DrawVAO;
-  *attrib = _mesa_draw_array_attrib(vao, attr);
-  *binding = _mesa_draw_buffer_binding_from_attrib(vao, *attrib);
-   } else {
-  *attrib = _vbo_current_attrib(ctx, attr);
-  *binding = _vbo_current_binding(ctx);
-   }
-}
-
-
 /*
  * API functions
  */
-- 
2.19.2

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

[Mesa-dev] [PATCH 08/10] mesa: Provide gl_vertex_format accessors.

[Mathias . Froehlich]

From: Mathias Fröhlich 

Provide the same set of VAO and current value gl_vertex_format
accessor functions like we have for the gl_array_attributes.
For most purpose the vertex format is what we need.

Signed-off-by: Mathias Fröhlich 
---
 src/mesa/main/arrayobj.h | 21 +
 1 file changed, 21 insertions(+)

diff --git a/src/mesa/main/arrayobj.h b/src/mesa/main/arrayobj.h
index 55d233befb..70da371561 100644
--- a/src/mesa/main/arrayobj.h
+++ b/src/mesa/main/arrayobj.h
@@ -229,6 +229,17 @@ _mesa_draw_array_attrib(const struct 
gl_vertex_array_object *vao,
 }
 
 
+/**
+ * Return a vertex array vertex format provided the attribute number.
+ */
+static inline const struct gl_vertex_format*
+_mesa_draw_array_format(const struct gl_vertex_array_object *vao,
+gl_vert_attrib attr)
+{
+   return &_mesa_draw_array_attrib(vao, attr)->Format;
+}
+
+
 /**
  * Return vertex buffer binding provided an attribute number.
  */
@@ -288,6 +299,16 @@ _mesa_draw_current_attrib(const struct gl_context *ctx, 
gl_vert_attrib attr)
 }
 
 
+/**
+ * Return a current value vertex format provided the attribute number.
+ */
+static inline const struct gl_vertex_format*
+_mesa_draw_current_format(const struct gl_context *ctx, gl_vert_attrib attr)
+{
+   return &_vbo_current_attrib(ctx, attr)->Format;
+}
+
+
 /**
  * Return true if we have the VERT_ATTRIB_EDGEFLAG array enabled.
  */
-- 
2.19.2

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

[Mesa-dev] [PATCH 09/10] i965: Make use of the vertex format functions in i965.

[Mathias . Froehlich]

From: Mathias Fröhlich 

Signed-off-by: Mathias Fröhlich 
---
 src/mesa/drivers/dri/i965/brw_draw.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_draw.c 
b/src/mesa/drivers/dri/i965/brw_draw.c
index 6e4a0a0213..e0d17cd449 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -344,9 +344,8 @@ brw_merge_inputs(struct brw_context *brw)
   GLbitfield vaomask = vs_inputs & _mesa_draw_array_bits(ctx);
   while (vaomask) {
  const gl_vert_attrib i = u_bit_scan(&vaomask);
- const struct gl_array_attributes *glattrib
-= _mesa_draw_array_attrib(vao, i);
- const uint8_t wa_flags = get_wa_flags(&glattrib->Format);
+ const uint8_t wa_flags
+= get_wa_flags(_mesa_draw_array_format(vao, i));
 
  if (brw->vb.attrib_wa_flags[i] != wa_flags) {
 brw->vb.attrib_wa_flags[i] = wa_flags;
@@ -357,9 +356,8 @@ brw_merge_inputs(struct brw_context *brw)
   GLbitfield currmask = vs_inputs & _mesa_draw_current_bits(ctx);
   while (currmask) {
  const gl_vert_attrib i = u_bit_scan(&currmask);
- const struct gl_array_attributes *glattrib
-= _mesa_draw_current_attrib(ctx, i);
- const uint8_t wa_flags = get_wa_flags(&glattrib->Format);
+ const uint8_t wa_flags
+= get_wa_flags(_mesa_draw_current_format(ctx, i));
 
  if (brw->vb.attrib_wa_flags[i] != wa_flags) {
 brw->vb.attrib_wa_flags[i] = wa_flags;
-- 
2.19.2

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

[Mesa-dev] [PATCH 05/10] i965: Remove glbinding from brw_vertex_element.

[Mathias . Froehlich]

From: Mathias Fröhlich 

Signed-off-by: Mathias Fröhlich 
---
 src/mesa/drivers/dri/i965/brw_context.h | 1 -
 src/mesa/drivers/dri/i965/brw_draw.c| 7 ---
 2 files changed, 8 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index b278bdd477..704542c87b 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -446,7 +446,6 @@ struct brw_vertex_buffer {
 };
 struct brw_vertex_element {
const struct gl_array_attributes *glattrib;
-   const struct gl_vertex_buffer_binding *glbinding;
 
int buffer;
bool is_dual_slot;
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c 
b/src/mesa/drivers/dri/i965/brw_draw.c
index fe96609e1f..6e4a0a0213 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -332,13 +332,6 @@ brw_merge_inputs(struct brw_context *brw)
 {
const struct gen_device_info *devinfo = &brw->screen->devinfo;
const struct gl_context *ctx = &brw->ctx;
-   GLuint i;
-
-   for (i = 0; i < VERT_ATTRIB_MAX; i++) {
-  struct brw_vertex_element *input = &brw->vb.inputs[i];
-  _mesa_draw_attrib_and_binding(ctx, i,
-&input->glattrib, &input->glbinding);
-   }
 
if (devinfo->gen < 8 && !devinfo->is_haswell) {
   /* Prior to Haswell, the hardware can't natively support GL_FIXED or
-- 
2.19.2

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

[Mesa-dev] [PATCH 07/10] mesa: Remove now unused _mesa_draw_attrib.

[Mathias . Froehlich]

From: Mathias Fröhlich 

Signed-off-by: Mathias Fröhlich 
---
 src/mesa/main/arrayobj.h | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/src/mesa/main/arrayobj.h b/src/mesa/main/arrayobj.h
index 49b3522f30..55d233befb 100644
--- a/src/mesa/main/arrayobj.h
+++ b/src/mesa/main/arrayobj.h
@@ -298,21 +298,6 @@ _mesa_draw_edge_flag_array_enabled(const struct gl_context 
*ctx)
 }
 
 
-/**
- * Return the attrib for the given attribute.
- */
-static inline const struct gl_array_attributes*
-_mesa_draw_attrib(const struct gl_context *ctx, gl_vert_attrib attr)
-{
-   if (ctx->Array._DrawVAOEnabledAttribs & VERT_BIT(attr)) {
-  const struct gl_vertex_array_object *vao = ctx->Array._DrawVAO;
-  return _mesa_draw_array_attrib(vao, attr);
-   } else {
-  return _vbo_current_attrib(ctx, attr);
-   }
-}
-
-
 /*
  * API functions
  */
-- 
2.19.2

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

[Mesa-dev] [PATCH 04/10] i965: Reorder workaround flags computation.

[Mathias . Froehlich]

From: Mathias Fröhlich 

Vertex processing workaround flags can be split into
array and current vertex attributes. By that we
can use specific access functions for these different
vertex attribute kinds. This finally obsoletes
some access functions that I introduced last winter
for a smooth transition.

Signed-off-by: Mathias Fröhlich 
---
 src/mesa/drivers/dri/i965/brw_draw.c | 76 ++--
 1 file changed, 50 insertions(+), 26 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_draw.c 
b/src/mesa/drivers/dri/i965/brw_draw.c
index 9826bc8761..fe96609e1f 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -297,6 +297,36 @@ brw_clear_buffers(struct brw_context *brw)
 }
 
 
+static uint8_t get_wa_flags(const struct gl_vertex_format *glformat)
+{
+   uint8_t wa_flags = 0;
+
+   switch (glformat->Type) {
+
+   case GL_FIXED:
+  wa_flags = glformat->Size;
+  break;
+
+   case GL_INT_2_10_10_10_REV:
+  wa_flags |= BRW_ATTRIB_WA_SIGN;
+  /* fallthough */
+
+   case GL_UNSIGNED_INT_2_10_10_10_REV:
+  if (glformat->Format == GL_BGRA)
+ wa_flags |= BRW_ATTRIB_WA_BGRA;
+
+  if (glformat->Normalized)
+ wa_flags |= BRW_ATTRIB_WA_NORMALIZE;
+  else if (!glformat->Integer)
+ wa_flags |= BRW_ATTRIB_WA_SCALE;
+
+  break;
+   }
+
+   return wa_flags;
+}
+
+
 static void
 brw_merge_inputs(struct brw_context *brw)
 {
@@ -311,38 +341,32 @@ brw_merge_inputs(struct brw_context *brw)
}
 
if (devinfo->gen < 8 && !devinfo->is_haswell) {
-  uint64_t mask = ctx->VertexProgram._Current->info.inputs_read;
   /* Prior to Haswell, the hardware can't natively support GL_FIXED or
* 2_10_10_10_REV vertex formats.  Set appropriate workaround flags.
*/
-  while (mask) {
- const struct gl_vertex_format *glformat;
- uint8_t wa_flags = 0;
-
- i = u_bit_scan64(&mask);
- glformat = &brw->vb.inputs[i].glattrib->Format;
-
- switch (glformat->Type) {
-
- case GL_FIXED:
-wa_flags = glformat->Size;
-break;
-
- case GL_INT_2_10_10_10_REV:
-wa_flags |= BRW_ATTRIB_WA_SIGN;
-/* fallthough */
-
- case GL_UNSIGNED_INT_2_10_10_10_REV:
-if (glformat->Format == GL_BGRA)
-   wa_flags |= BRW_ATTRIB_WA_BGRA;
+  const struct gl_vertex_array_object *vao = ctx->Array._DrawVAO;
+  const uint64_t vs_inputs = ctx->VertexProgram._Current->info.inputs_read;
+  assert((vs_inputs & ~((uint64_t)VERT_BIT_ALL)) == 0);
 
-if (glformat->Normalized)
-   wa_flags |= BRW_ATTRIB_WA_NORMALIZE;
-else if (!glformat->Integer)
-   wa_flags |= BRW_ATTRIB_WA_SCALE;
+  GLbitfield vaomask = vs_inputs & _mesa_draw_array_bits(ctx);
+  while (vaomask) {
+ const gl_vert_attrib i = u_bit_scan(&vaomask);
+ const struct gl_array_attributes *glattrib
+= _mesa_draw_array_attrib(vao, i);
+ const uint8_t wa_flags = get_wa_flags(&glattrib->Format);
 
-break;
+ if (brw->vb.attrib_wa_flags[i] != wa_flags) {
+brw->vb.attrib_wa_flags[i] = wa_flags;
+brw->ctx.NewDriverState |= BRW_NEW_VS_ATTRIB_WORKAROUNDS;
  }
+  }
+
+  GLbitfield currmask = vs_inputs & _mesa_draw_current_bits(ctx);
+  while (currmask) {
+ const gl_vert_attrib i = u_bit_scan(&currmask);
+ const struct gl_array_attributes *glattrib
+= _mesa_draw_current_attrib(ctx, i);
+ const uint8_t wa_flags = get_wa_flags(&glattrib->Format);
 
  if (brw->vb.attrib_wa_flags[i] != wa_flags) {
 brw->vb.attrib_wa_flags[i] = wa_flags;
-- 
2.19.2

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

[Mesa-dev] [PATCH 02/10] i965: Use the VAOs binding information in array setup.

[Mathias . Froehlich]

From: Mathias Fröhlich 

The change basically reimplements array setup by walking
the gl_contex::Array._DrawVAO on a per binding sequence.
In this way we can make direct use of the application
provided minimum set of buffer objects and emit fewer relocs.

v2: Rebase onto:
compiler: Move double_inputs to gl_program::DualSlotInputs
v3: Rebase onto introduction of gl_vertex_format
v4: Reorder and extend patch series.

Signed-off-by: Mathias Fröhlich 
---
 src/mesa/drivers/dri/i965/brw_draw_upload.c | 366 ++--
 1 file changed, 174 insertions(+), 192 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c 
b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index dfbc45fe93..574e2281d9 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -400,30 +400,11 @@ brw_get_vertex_surface_type(struct brw_context *brw,
 
 static void
 copy_array_to_vbo_array(struct brw_context *brw,
-   struct brw_vertex_element *element,
-   int min, int max,
-   struct brw_vertex_buffer *buffer,
-   GLuint dst_stride)
+const GLubyte *const ptr, const int src_stride,
+int min, int max,
+struct brw_vertex_buffer *buffer, GLuint dst_stride)
 {
-   const struct gl_vertex_buffer_binding *glbinding = element->glbinding;
-   const struct gl_array_attributes *glattrib = element->glattrib;
-   const struct gl_vertex_format *glformat = &glattrib->Format;
-   const int src_stride = glbinding->Stride;
-
-   /* If the source stride is zero, we just want to upload the current
-* attribute once and set the buffer's stride to 0.  There's no need
-* to replicate it out.
-*/
-   if (src_stride == 0) {
-  brw_upload_data(&brw->upload, glattrib->Ptr, glformat->_ElementSize,
-  glformat->_ElementSize, &buffer->bo, &buffer->offset);
-
-  buffer->stride = 0;
-  buffer->size = glformat->_ElementSize;
-  return;
-   }
-
-   const unsigned char *src = glattrib->Ptr + min * src_stride;
+   const unsigned char *src = ptr + min * src_stride;
int count = max - min + 1;
GLuint size = count * dst_stride;
uint8_t *dst = brw_upload_space(&brw->upload, size, dst_stride,
@@ -436,7 +417,7 @@ copy_array_to_vbo_array(struct brw_context *brw,
 *
 * In this case, let's the dst with undefined values
 */
-   if (src != NULL) {
+   if (ptr != NULL) {
   if (dst_stride == src_stride) {
  memcpy(dst, src, size);
   } else {
@@ -461,19 +442,15 @@ brw_prepare_vertices(struct brw_context *brw)
/* BRW_NEW_VS_PROG_DATA */
const struct brw_vs_prog_data *vs_prog_data =
   brw_vs_prog_data(brw->vs.base.prog_data);
-   GLbitfield64 vs_inputs =
+   const GLbitfield64 vs_inputs64 =
   nir_get_single_slot_attribs_mask(vs_prog_data->inputs_read,
vp->DualSlotInputs);
-   const unsigned char *ptr = NULL;
-   GLuint interleaved = 0;
+   assert((vs_inputs64 & ~(GLbitfield64)VERT_BIT_ALL) == 0);
+   GLbitfield vs_inputs = (GLbitfield)vs_inputs64;
unsigned int min_index = brw->vb.min_index + brw->basevertex;
unsigned int max_index = brw->vb.max_index + brw->basevertex;
-   unsigned i;
int delta, j;
 
-   struct brw_vertex_element *upload[VERT_ATTRIB_MAX];
-   GLuint nr_uploads = 0;
-
/* _NEW_POLYGON
 *
 * On gen6+, edge flags don't end up in the VUE (either in or out of the
@@ -491,15 +468,13 @@ brw_prepare_vertices(struct brw_context *brw)
 
/* Accumulate the list of enabled arrays. */
brw->vb.nr_enabled = 0;
-   while (vs_inputs) {
-  const unsigned index = ffsll(vs_inputs) - 1;
-  assert(index < 64);
-
-  struct brw_vertex_element *input = &brw->vb.inputs[index];
-  input->is_dual_slot = (vp->DualSlotInputs & BITFIELD64_BIT(index)) != 0;
-  vs_inputs &= ~BITFIELD64_BIT(index);
+   GLbitfield mask = vs_inputs;
+   while (mask) {
+  gl_vert_attrib attr = u_bit_scan(&mask);
+  struct brw_vertex_element *input = &brw->vb.inputs[attr];
   brw->vb.enabled[brw->vb.nr_enabled++] = input;
}
+   assert(brw->vb.nr_enabled <= VERT_ATTRIB_MAX);
 
if (brw->vb.nr_enabled == 0)
   return;
@@ -507,134 +482,84 @@ brw_prepare_vertices(struct brw_context *brw)
if (brw->vb.nr_buffers)
   return;
 
-   /* The range of data in a given buffer represented as [min, max) */
-   struct intel_buffer_object *enabled_buffer[VERT_ATTRIB_MAX];
-   uint32_t buffer_range_start[VERT_ATTRIB_MAX];
-   uint32_t buffer_range_end[VERT_ATTRIB_MAX];
+   j = 0;
+   const struct gl_vertex_array_object *vao = ctx->Array._DrawVAO;
+
+   GLbitfield vbomask = vs_inputs & _mesa_draw_vbo_array_bits(ctx);
+   while (vbomask) {
+  const struct gl_vertex_buffer_binding *const glbinding =
+ _mesa_draw_buffer_binding(vao, ffs(vbomask) - 1);
+  const GLsizei stride = glbinding->St

[Mesa-dev] [PATCH 00/10] Make use of VAO information in i965 v4.

[Mathias . Froehlich]

From: Mathias Fröhlich 


Hi all,

The following series is a resend from last spring with some rebasing
on the way.

This change finally makes use of the binding/attribute information now
present in the VAO. The big part is basically a rewrite of brw_draw_upload
in a way that traverses in an outer loop the bindings and for each binding
the attached arrays. By this way the driver emits as few buffers as the VAO
allows. For an application that already configures the VAO with a minimal
set of buffers, this information is now routed through from the application
down into the backend and the hardware.
And there is the first in the series that remove the last VERT_ATTRIB_MAX
long loop in the fast draw path.

I did, up to now, not find a sensible way to split the big blob #2 into smaller
nicer to review chunks. Nevertheless, the patchset run through intels CI
system without introducing failures (With changes in logs frohlich/#85).

The piglit drawoverhead test improves from 1.5 million to 2 million
DrawElements calls and from 1.5 million to 2 million DrawArrays calls
on a Skylake GT2.

Note that the change also serves as a preparation for further cleanup
in the vbo module due to a different code flow for the current attribute values.

Please review

Thanks and best
Mathias


Mathias Fröhlich (10):
  i965: Move down genX_upload_sbe in profiles.
  i965: Use the VAOs binding information in array setup.
  i965: Split merge_inputs and clear_buffers.
  i965: Reorder workaround flags computation.
  i965: Remove glbinding from brw_vertex_element.
  mesa: Remove now unused _mesa_draw_attrib_and_binding.
  mesa: Remove now unused _mesa_draw_attrib.
  mesa: Provide gl_vertex_format accessors.
  i965: Make use of the vertex format functions in i965.
  i965: Use gl_vertex_format in brw_vertex_element.

 src/intel/compiler/brw_compiler.h |   7 +
 src/intel/compiler/brw_fs.cpp |  25 ++
 src/intel/compiler/brw_fs.h   |   2 +
 src/intel/compiler/brw_fs_visitor.cpp |   1 +
 src/mesa/drivers/dri/i965/brw_context.h   |   3 +-
 src/mesa/drivers/dri/i965/brw_draw.c  | 103 +++--
 src/mesa/drivers/dri/i965/brw_draw_upload.c   | 366 +-
 src/mesa/drivers/dri/i965/genX_state_upload.c |  27 +-
 src/mesa/main/arrayobj.h  |  51 +--
 9 files changed, 307 insertions(+), 278 deletions(-)

-- 
2.19.2

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

Re: [Mesa-dev] [PATCH v3] mesa: Fix GLES2 OES float texture framebuffer rendering.

[Tapani Pälli]

On 12/12/18 5:05 AM, Nick Kreeger wrote:

This change enables GLES2 to render float/half-float textures to a
framebuffer when the appropriate OES extensions are available.

This commit regressed OES GLES2 float texture rendering:
https://gitlab.freedesktop.org/mesa/mesa/commit/e333035c47a6a4cc88f0f9ca2bced500538bebae


Which test/app got regressed? I'm curious because this commit also fixed 
a failing conformance test that explicitly tests that the fbo with float 
attachment should be marked incomplete.




---
  src/mesa/main/fbobject.c | 20 +++-
  1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 23e4939619..cedfc3d81b 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -869,15 +869,17 @@ test_attachment_completeness(const struct gl_context 
*ctx, GLenum format,
  return;
   }
  
- /* OES_texture_float allows creation and use of floating point

-  * textures with GL_FLOAT, GL_HALF_FLOAT but it does not allow
-  * these textures to be used as a render target, this is done via
-  * GL_EXT_color_buffer(_half)_float with set of new sized types.
-  */
- if (_mesa_is_gles(ctx) && (texObj->_IsFloat || texObj->_IsHalfFloat)) 
{
-att_incomplete("bad internal format");
-att->Complete = GL_FALSE;
-return;
+ if (_mesa_is_gles(ctx)) {
+   /**
+* GL ES 2 will allow GL_FLOAT and GL_HALF_FLOAT to render as a
+* target when the appropriate OES_* extensions are available.
+*/
+   if ((texObj->_IsHalfFloat && 
!_mesa_has_OES_texture_half_float(ctx)) ||
+   (texObj->_IsFloat && !_mesa_has_OES_texture_float(ctx))) {
+ att_incomplete("bad internal format");
+ att->Complete = GL_FALSE;
+ return;
+   }
   }
}
else if (format == GL_DEPTH) {


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

Re: [Mesa-dev] [PATCH] nir: Document the function inlining process

[Matt Turner]

Thanks. This was very useful for me in the fp64 work.

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

[Mesa-dev] [Bug 109023] error: inlining failed in call to always_inline ‘__m512 _mm512_and_ps(__m512, __m512)’: target specific option mismatch

[bugzilla-daemon]

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

Bug ID: 109023
   Summary: error: inlining failed in call to always_inline
‘__m512 _mm512_and_ps(__m512, __m512)’: target
specific option mismatch
   Product: Mesa
   Version: git
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: Other
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: v...@freedesktop.org
QA Contact: mesa-dev@lists.freedesktop.org
CC: baker.dyla...@gmail.com

Build error with meson.

In file included from
/usr/lib/gcc/x86_64-redhat-linux/8/include/immintrin.h:57,
 from
/usr/lib/gcc/x86_64-redhat-linux/8/include/x86intrin.h:48,
 from ../src/gallium/drivers/swr/rasterizer/common/os.h:99,
 from ../src/gallium/drivers/swr/rasterizer/core/clip.cpp:31:
/usr/lib/gcc/x86_64-redhat-linux/8/include/avx512dqintrin.h: In static member
function ‘static SIMDImpl::SIMD512Impl::Float
SIMDImpl::SIMD512Impl::AVX512Impl::and_ps(SIMDImpl::SIMD512Impl::Float,
SIMDImpl::SIMD512Impl::Float)’:
/usr/lib/gcc/x86_64-redhat-linux/8/include/avx512dqintrin.h:564:1: error:
inlining failed in call to always_inline ‘__m512 _mm512_and_ps(__m512,
__m512)’: target specific option mismatch
 _mm512_and_ps (__m512 __A, __m512 __B)
 ^
In file included from
../src/gallium/drivers/swr/rasterizer/common/simdlib.hpp:172,
 from
../src/gallium/drivers/swr/rasterizer/common/simdintrin.h:28,
 from ../src/gallium/drivers/swr/rasterizer/core/clip.h:30,
 from ../src/gallium/drivers/swr/rasterizer/core/clip.cpp:32:
../src/gallium/drivers/swr/rasterizer/common/simdlib_512_avx512_core.inl:38:88:
note: called from here
 static SIMDINLINE Float SIMDCALL op(Float a, Float b) { return
_mm512_##intrin(a, b); }
   
^
../src/gallium/drivers/swr/rasterizer/common/simdlib_512_avx512_core.inl:39:28:
note: in expansion of macro ‘SIMD_WRAPPER_2_’
 #define SIMD_WRAPPER_2(op) SIMD_WRAPPER_2_(op, op)
^~~
../src/gallium/drivers/swr/rasterizer/common/simdlib_512_avx512_core.inl:122:1:
note: in expansion of macro ‘SIMD_WRAPPER_2’
 SIMD_WRAPPER_2(and_ps);// return a & b   (float treated as int)
 ^~

-- 
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] genxml: Consistently use a numeric "MOCS" field

[Jordan Justen]

Reviewed-by: Jordan Justen 

On 2018-12-11 20:23:06, Kenneth Graunke wrote:
> When we first started using genxml, we decided to represent MOCS as an
> actual structure, and pack values.  However, in many places, it was more
> convenient to use a numeric value rather than treating it as a struct,
> so we added secondary setters in a bunch of places as well.
> 
> We were not entirely consistent, either.  Some places only had one.
> Gen6 had both kinds of setters for STATE_BASE_ADDRESS, but newer gens
> only had the struct-based setters.  The names were sometimes "Constant
> Buffer Object Control State" instead of "Memory", making it harder to
> find.  Many had prefixes like "Vertex Buffer MOCS"...in a vertex buffer
> packet...which is a bit redundant.
> 
> On modern hardware, MOCS is simply an index into a table, but we were
> still carrying around the structure with an "Index to MOCS Table" field,
> in addition to the direct numeric setters.  This is clunky - we really
> just want a number on new hardware.
> 
> This patch eliminates the struct-based setters, and makes the numeric
> setters be consistently called "MOCS".  We leave the struct definition
> around on Gen7-8 for reference purposes, but it is unused.
> ---
>  src/intel/blorp/blorp_genX_exec.h |  2 +-
>  src/intel/genxml/gen10.xml| 53 +
>  src/intel/genxml/gen11.xml| 53 +
>  src/intel/genxml/gen6.xml | 28 ++-
>  src/intel/genxml/gen7.xml | 35 -
>  src/intel/genxml/gen75.xml| 38 --
>  src/intel/genxml/gen8.xml | 47 +---
>  src/intel/genxml/gen9.xml | 50 +---
>  src/intel/isl/isl_emit_depth_stencil.c|  6 +-
>  src/intel/vulkan/anv_private.h| 76 ---
>  src/intel/vulkan/gen7_cmd_buffer.c|  2 +-
>  src/intel/vulkan/gen8_cmd_buffer.c|  2 +-
>  src/intel/vulkan/genX_cmd_buffer.c| 19 +++--
>  src/intel/vulkan/genX_gpu_memcpy.c|  4 +-
>  src/intel/vulkan/genX_state.c |  6 +-
>  src/mesa/drivers/dri/i965/genX_state_upload.c | 14 ++--
>  16 files changed, 177 insertions(+), 258 deletions(-)
> 
> diff --git a/src/intel/blorp/blorp_genX_exec.h 
> b/src/intel/blorp/blorp_genX_exec.h
> index 065980616ec..42494ffbc86 100644
> --- a/src/intel/blorp/blorp_genX_exec.h
> +++ b/src/intel/blorp/blorp_genX_exec.h
> @@ -311,7 +311,7 @@ blorp_fill_vertex_buffer_state(struct blorp_batch *batch,
> vb[idx].BufferPitch = stride;
>  
>  #if GEN_GEN >= 6
> -   vb[idx].VertexBufferMOCS = addr.mocs;
> +   vb[idx].MOCS = addr.mocs;
>  #endif
>  
>  #if GEN_GEN >= 7
> diff --git a/src/intel/genxml/gen10.xml b/src/intel/genxml/gen10.xml
> index 2d3bc39b1b9..21cd8a17d91 100644
> --- a/src/intel/genxml/gen10.xml
> +++ b/src/intel/genxml/gen10.xml
> @@ -219,14 +219,9 @@
>  
>
>  
> -  
> -
> -  
> -
>
>  
> - type="MEMORY_OBJECT_CONTROL_STATE"/>
> -
> +
>  
>  
>  
> @@ -495,7 +490,6 @@
>   type="bool"/>
>   type="bool"/>
>   type="bool"/>
> - type="MEMORY_OBJECT_CONTROL_STATE"/>
>  
>  
>  
> @@ -993,7 +987,7 @@
>  
>   type="address"/>
>  
> - type="MEMORY_OBJECT_CONTROL_STATE"/>
> +
>   type="uint">
>
>  
> @@ -1085,7 +1079,7 @@
>   default="3"/>
>   default="0"/>
>   default="26"/>
> - type="MEMORY_OBJECT_CONTROL_STATE"/>
> +
>  
>   type="3DSTATE_CONSTANT_BODY"/>
>
> @@ -1095,7 +1089,7 @@
>   default="3"/>
>   default="0"/>
>   default="22"/>
> - type="MEMORY_OBJECT_CONTROL_STATE"/>
> +
>  
>   type="3DSTATE_CONSTANT_BODY"/>
>
> @@ -1105,7 +1099,7 @@
>   default="3"/>
>   default="0"/>
>   default="25"/>
> - type="MEMORY_OBJECT_CONTROL_STATE"/>
> +
>  
>   type="3DSTATE_CONSTANT_BODY"/>
>
> @@ -1116,7 +1110,7 @@
>   default="0"/>
>   default="23"/>
>   type="uint"/>
> - type="MEMORY_OBJECT_CONTROL_STATE"/>
> +
>  
>   type="3DSTATE_CONSTANT_BODY"/>
>
> @@ -1126,7 +1120,7 @@
>   default="3"/>
>   default="0"/>
>   default="21"/>
> - type="MEMORY_OBJECT_CONTROL_STATE"/>
> +
>  
>   type="3DSTATE_CONSTANT_BODY"/>
>
> @@ -1157,8 +1151,7 @@
>  
>  
>  
> - type="MEMORY_OBJECT_CONTROL_STATE"/>
> -
> +
>  
>
>
> @@ -1368,7 +1361,7 @@
>  
>   type="address"/>
>  
> - type="MEMORY_OBJECT_CONTROL_STATE"/>
> +
>  
>
>  
> @@ -1447,8 +1440,7 @@
>   default="0"/>
>   default="7"/>
>  
> - end="63" type="MEMORY_OBJECT_CONTROL_STATE"/>
> - type="uint"/>
> +
>  
>  
>  
> @@ -1511,8 +1503,7 @@
>
>
>  
> - type="MEMORY_OBJECT_CONTROL_STATE"/>
> -
> +
>   type="address"/>
>  

Re: [Mesa-dev] [PATCH 4/4] virgl: work around bad assumptions in virglrenderer

[Mathias Fröhlich]

Erik,

On Tuesday, 11 December 2018 15:29:49 CET Erik Faye-Lund wrote:
> On Tue, 2018-12-11 at 15:26 +0100, Erik Faye-Lund wrote:
> > Virglrenderer does the wrong thing when given an instance divisor;
> > it tries to use the element-index rather than the binding-index as
> > the argument to glVertexBindingDivisor(). This worked fine as long
> > as there was a 1:1 relationship between elements and bindings,
> > which was the case util 19a91841c34 "st/mesa: Use Array._DrawVAO in
> > st_atom_array.c.".
> > 
> > So let's detect instance divisors, and restore a 1:1 relationship in
> > that case. This will make old versions of virglrenderer behave
> > correctly. For newer versions, we can consider making a better
> > interface, where the instance divisor isn't specified per element,
> > but rather per binding. But let's save that for another day.
> > 
> > Signed-off-by: Erik Faye-Lund 

I don't exactly know what kind of coding standards and that you use in virgl.
But the patch series looks good and should help for the issues we observed.

One thing, may be. Do you want to add some documentation beside the
git log message why we do something surprising like replicating out the
buffers and assigning new buffer indices? Just something that allows
a reader to get an idea why non straight forward things happen here.
The git annotate references to the commit messages tend to vanish
over time behind further changes in the code.

> I suppose this should have had:
> 
> Fixes: 19a91841c34 "st/mesa: Use Array._DrawVAO in st_atom_array.c."
Probably at least for patch #3 and #4.

With or without such comment and for the series:
Reviewed-by: Mathias Fröhlich 

best

Mathias


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

[Mesa-dev] [PATCH] mesa: ES2 glReadPixels support for OES float extensions.

[Nick Kreeger]

The OES extensions for float/half-float allow glReadPixels to read
GL_RGBA values as GL_FLOAT for both floats and half-floats. This patch
ensures that ES2 context versions allows this if at least one of the
extensions is present.
---
 src/mesa/main/readpix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
index 556c860d39..59f14a6563 100644
--- a/src/mesa/main/readpix.c
+++ b/src/mesa/main/readpix.c
@@ -1084,7 +1084,8 @@ read_pixels(GLint x, GLint y, GLsizei width, GLsizei 
height, GLenum format,
  } else if (ctx->Version < 30) {
 err = _mesa_es_error_check_format_and_type(ctx, format, type, 2);
 if (err == GL_NO_ERROR) {
-   if (type == GL_FLOAT || type == GL_HALF_FLOAT_OES) {
+   if ((type == GL_FLOAT || type == GL_HALF_FLOAT_OES) &&
+   (!_mesa_has_OES_texture_float(ctx) && 
!_mesa_has_OES_texture_half_float(ctx))) {
   err = GL_INVALID_OPERATION;
}
 }
-- 
2.17.1

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

[Mesa-dev] [Bug 108946] High memory usage in Black Mesa

[bugzilla-daemon]

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

Ian Romanick  changed:

   What|Removed |Added

 Status|NEW |NEEDINFO
 CC||i...@freedesktop.org

--- Comment #6 from Ian Romanick  ---
(I'm quite sure I submitted a similar comment earlier today, but it seems to
have just vanished.)

I ran the API trace in Valgrind massif memory profiler.  As far as I can tell,
the application never calls glDeleteShader.  We don't release any of the memory
because the app hasn't told us that we can.  In that trace there is is about
2.6GB (on a 64-bit build of apitrace) of intermediate IR that could be released
by calling glDeleteShader on all the shaders that have been linked.

It's possible that this is a quirk of the API trace.  Someone would have to run
the app with Valgrind massif to say for sure.

(In reply to MGG from comment #0)
> Finally, while running an apitrace I detected a lot this errors: "major
> shader compiler issue 21156: 0:4(12): warning: extension
> `GL_EXT_gpu_shader4' unsupported in vertex shader". Not sure if it could be
> related with any leak though.

This is unrelated.  It's a bit misleading that apitrace lists this as a "major
shader compiler issue."  It is perfectly valid to enable extensions that the
driver does not support.  GLSL is intentionally designed so that shaders can
have multiple code paths that are used based on what extensions are available
at compile time.

-- 
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] nir: Document the function inlining process

[Karol Herbst]

sorry, totally forgot about that. Seems fine as it is and I have the
same within my CL branch (except the last
nir_lower_constant_initializers).

as I don't really know if all that is actually true, but because it
makes sense to me:
Acked-by: Karol Herbst 

I kind of wished we would have a helper function for this though like
"nir_convert_to_single_function(nir_shader *shader, nir_function
*entry_point)" which would do all of that already and we could just
call. Hopefully you can come up with a better name.
On Wed, Dec 12, 2018 at 4:34 AM Jason Ekstrand  wrote:
>
> ping
>
> On Mon, Oct 29, 2018 at 12:14 PM Jason Ekstrand  wrote:
>>
>> This has thrown a few people off recently and it's good to have the
>> process and all the rational for it documented somewhere.  A comment at
>> the top of nir_inline_functions seems as good a place as any.
>>
>> Cc: Matt Turner 
>> Cc: Karol Herbst 
>> ---
>>  src/compiler/nir/nir_inline_functions.c | 68 +
>>  1 file changed, 68 insertions(+)
>>
>> diff --git a/src/compiler/nir/nir_inline_functions.c 
>> b/src/compiler/nir/nir_inline_functions.c
>> index 06c90d93956..29474bb417b 100644
>> --- a/src/compiler/nir/nir_inline_functions.c
>> +++ b/src/compiler/nir/nir_inline_functions.c
>> @@ -132,6 +132,74 @@ inline_function_impl(nir_function_impl *impl, struct 
>> set *inlined)
>> return progress;
>>  }
>>
>> +/** A pass to inline all functions in a shader into their callers
>> + *
>> + * For most use-cases, function inlining is a multi-step process.  The 
>> general
>> + * pattern employed by SPIR-V consumers and others is as follows:
>> + *
>> + *  1. nir_lower_constant_initializers(shader, nir_var_local)
>> + *
>> + * This is needed because local variables from the callee are simply 
>> added
>> + * to the locals list for the caller and the information about where the
>> + * constant initializer logically happens is lost.  If the callee is
>> + * called in a loop, this can cause the variable to go from being
>> + * initialized once per loop iteration to being initialized once at the
>> + * top of the caller and values to persist from one invocation of the
>> + * callee to the next.  The simple solution to this problem is to get 
>> rid
>> + * of constant initializers before function inlining.
>> + *
>> + *  2. nir_lower_returns(shader)
>> + *
>> + * nir_inline_functions assumes that all functions end "naturally" by
>> + * execution reaching the end of the function without any return
>> + * instructions causing instant jumps to the end.  Thanks to NIR being
>> + * structured, we can't represent arbitrary jumps to various points in 
>> the
>> + * program which is what an early return in the callee would have to 
>> turn
>> + * into when we inline it into the caller.  Instead, we require returns 
>> to
>> + * be lowered which lets us just copy+paste the callee directly into the
>> + * caller.
>> + *
>> + *  3. nir_inline_functions(shader)
>> + *
>> + * This does the actual function inlining and the resulting shader will
>> + * contain no call instructions.
>> + *
>> + *  4. nir_copy_prop(shader)
>> + *
>> + * Most functions contain pointer parameters where the result of a deref
>> + * instruction is passed in as a parameter, loaded via a load_param
>> + * intrinsic, and then turned back into a deref via a cast.  Running 
>> copy
>> + * propagation gets rid of the intermediate steps and results in a whole
>> + * deref chain again.  This is currently required by a number of
>> + * optimizations and lowering passes at least for certain variable 
>> modes.
>> + *
>> + *  5. Loop over the functions and delete all but the main entrypoint.
>> + *
>> + * In the Intel Vulkan driver this looks like this:
>> + *
>> + *foreach_list_typed_safe(nir_function, func, node, 
>> &nir->functions) {
>> + *   if (func != entry_point)
>> + *  exec_node_remove(&func->node);
>> + *}
>> + *assert(exec_list_length(&nir->functions) == 1);
>> + *
>> + *While nir_inline_functions does get rid of all call instructions, it
>> + *doesn't get rid of any functions because it doesn't know what the 
>> "root
>> + *function" is.  Instead, it's up to the individual driver to know how 
>> to
>> + *decide on a root function and delete the rest.  With SPIR-V,
>> + *spirv_to_nir returns the root function and so we can just use == 
>> whereas
>> + *with GL, you may have to look for a function named "main".
>> + *
>> + *  6. nir_lower_constant_initializers(shader, ~nir_var_local)
>> + *
>> + * Lowering constant initializers on inputs, outputs, global variables,
>> + * etc. requires that we know the main entrypoint so that we know where 
>> to
>> + * initialize them.  Otherwise, we would have to assume that anything
>> + * could be a main entrypoint and initialize them at the start of every
>> + * function but

[Mesa-dev] [PATCH] genxml: Consistently use a numeric "MOCS" field

[Kenneth Graunke]

When we first started using genxml, we decided to represent MOCS as an
actual structure, and pack values.  However, in many places, it was more
convenient to use a numeric value rather than treating it as a struct,
so we added secondary setters in a bunch of places as well.

We were not entirely consistent, either.  Some places only had one.
Gen6 had both kinds of setters for STATE_BASE_ADDRESS, but newer gens
only had the struct-based setters.  The names were sometimes "Constant
Buffer Object Control State" instead of "Memory", making it harder to
find.  Many had prefixes like "Vertex Buffer MOCS"...in a vertex buffer
packet...which is a bit redundant.

On modern hardware, MOCS is simply an index into a table, but we were
still carrying around the structure with an "Index to MOCS Table" field,
in addition to the direct numeric setters.  This is clunky - we really
just want a number on new hardware.

This patch eliminates the struct-based setters, and makes the numeric
setters be consistently called "MOCS".  We leave the struct definition
around on Gen7-8 for reference purposes, but it is unused.
---
 src/intel/blorp/blorp_genX_exec.h |  2 +-
 src/intel/genxml/gen10.xml| 53 +
 src/intel/genxml/gen11.xml| 53 +
 src/intel/genxml/gen6.xml | 28 ++-
 src/intel/genxml/gen7.xml | 35 -
 src/intel/genxml/gen75.xml| 38 --
 src/intel/genxml/gen8.xml | 47 +---
 src/intel/genxml/gen9.xml | 50 +---
 src/intel/isl/isl_emit_depth_stencil.c|  6 +-
 src/intel/vulkan/anv_private.h| 76 ---
 src/intel/vulkan/gen7_cmd_buffer.c|  2 +-
 src/intel/vulkan/gen8_cmd_buffer.c|  2 +-
 src/intel/vulkan/genX_cmd_buffer.c| 19 +++--
 src/intel/vulkan/genX_gpu_memcpy.c|  4 +-
 src/intel/vulkan/genX_state.c |  6 +-
 src/mesa/drivers/dri/i965/genX_state_upload.c | 14 ++--
 16 files changed, 177 insertions(+), 258 deletions(-)

diff --git a/src/intel/blorp/blorp_genX_exec.h 
b/src/intel/blorp/blorp_genX_exec.h
index 065980616ec..42494ffbc86 100644
--- a/src/intel/blorp/blorp_genX_exec.h
+++ b/src/intel/blorp/blorp_genX_exec.h
@@ -311,7 +311,7 @@ blorp_fill_vertex_buffer_state(struct blorp_batch *batch,
vb[idx].BufferPitch = stride;
 
 #if GEN_GEN >= 6
-   vb[idx].VertexBufferMOCS = addr.mocs;
+   vb[idx].MOCS = addr.mocs;
 #endif
 
 #if GEN_GEN >= 7
diff --git a/src/intel/genxml/gen10.xml b/src/intel/genxml/gen10.xml
index 2d3bc39b1b9..21cd8a17d91 100644
--- a/src/intel/genxml/gen10.xml
+++ b/src/intel/genxml/gen10.xml
@@ -219,14 +219,9 @@
 
   
 
-  
-
-  
-
   
 
-
-
+
 
 
 
@@ -495,7 +490,6 @@
 
 
 
-
 
 
 
@@ -993,7 +987,7 @@
 
 
 
-
+
 
   
 
@@ -1085,7 +1079,7 @@
 
 
 
-
+
 
 
   
@@ -1095,7 +1089,7 @@
 
 
 
-
+
 
 
   
@@ -1105,7 +1099,7 @@
 
 
 
-
+
 
 
   
@@ -1116,7 +1110,7 @@
 
 
 
-
+
 
 
   
@@ -1126,7 +1120,7 @@
 
 
 
-
+
 
 
   
@@ -1157,8 +1151,7 @@
 
 
 
-
-
+
 
   
   
@@ -1368,7 +1361,7 @@
 
 
 
-
+
 
   
 
@@ -1447,8 +1440,7 @@
 
 
 
-
-
+
 
 
 
@@ -1511,8 +1503,7 @@
   
   
 
-
-
+
 
 
   
@@ -2068,8 +2059,7 @@
 
 
 
-
-
+
 
 
 
@@ -2104,8 +2094,7 @@
 
 
 
-
-
+
 
 
 
@@ -3318,20 +3307,20 @@
 
 
 
-
+
 
-
+
 
-
+
 
 
-
+
 
 
-
+
 
 
-
+
 
 
 
@@ -3342,11 +3331,11 @@
 
 
 
-
+
 
 
 
-
+
 
 
   
diff --git a/src/intel/genxml/gen11.xml b/src/intel/genxml/gen11.xml
index 1239ed011ed..54816458fc3 100644
--- a/src/intel/genxml/gen11.xml
+++ b/src/intel/genxml/gen11.xml
@@ -220,14 +220,9 @@
 
   
 
-  
-
-  
-
   
 
-
-
+
 
 
 
@@ -496,7 +491,6 @@
 
 
 
-
 
 
 
@@ -1012,7 +1006,7 @@
 
 
 
-
+
 
   
 
@@ -1104,7 +1098,7 @@
 
 
 
-
+
 
 
   
@@ -1114,7 +1108,7 @@
 
 
 
-
+
 
 
   
@@ -1124,7 +1118,7 @@
 
 
 
-
+
 
 
   
@@ -1135,7 +1129,7 @@
 
 
 
-
+
 
 
   
@@ -1145,7 +1139,7 @@
 
 
 
-
+
 
 
   
@@ -1176,8 +1170,7 @@
 
 
 
-
-
+
 
   
   
@@ -1386,7 +1379,7 @@
 
 
 
-
+
 
   
 
@@ -1463,8 +1456,7 @@
 
 
 
-
-
+
 
   

Re: [Mesa-dev] [PATCH] intel/blorp: Assert that we don't re-layout a compressed surface

[Jason Ekstrand]

ping

On Thu, Apr 5, 2018 at 12:50 PM Jason Ekstrand  wrote:

> ---
>  src/intel/blorp/blorp_blit.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/intel/blorp/blorp_blit.c b/src/intel/blorp/blorp_blit.c
> index 0757db0..0f9ecb3 100644
> --- a/src/intel/blorp/blorp_blit.c
> +++ b/src/intel/blorp/blorp_blit.c
> @@ -1395,6 +1395,9 @@ blorp_surf_convert_to_single_slice(const struct
> isl_device *isl_dev,
>  {
> bool ok UNUSED;
>
> +   /* It would be insane to try and do this on a compressed surface */
> +   assert(info->aux_usage == ISL_AUX_USAGE_NONE);
> +
> /* Just bail if we have nothing to do. */
> if (info->surf.dim == ISL_SURF_DIM_2D &&
> info->view.base_level == 0 && info->view.base_array_layer == 0 &&
> --
> 2.5.0.400.gff86faf
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir: Document the function inlining process

[Jason Ekstrand]

ping

On Mon, Oct 29, 2018 at 12:14 PM Jason Ekstrand 
wrote:

> This has thrown a few people off recently and it's good to have the
> process and all the rational for it documented somewhere.  A comment at
> the top of nir_inline_functions seems as good a place as any.
>
> Cc: Matt Turner 
> Cc: Karol Herbst 
> ---
>  src/compiler/nir/nir_inline_functions.c | 68 +
>  1 file changed, 68 insertions(+)
>
> diff --git a/src/compiler/nir/nir_inline_functions.c
> b/src/compiler/nir/nir_inline_functions.c
> index 06c90d93956..29474bb417b 100644
> --- a/src/compiler/nir/nir_inline_functions.c
> +++ b/src/compiler/nir/nir_inline_functions.c
> @@ -132,6 +132,74 @@ inline_function_impl(nir_function_impl *impl, struct
> set *inlined)
> return progress;
>  }
>
> +/** A pass to inline all functions in a shader into their callers
> + *
> + * For most use-cases, function inlining is a multi-step process.  The
> general
> + * pattern employed by SPIR-V consumers and others is as follows:
> + *
> + *  1. nir_lower_constant_initializers(shader, nir_var_local)
> + *
> + * This is needed because local variables from the callee are simply
> added
> + * to the locals list for the caller and the information about where
> the
> + * constant initializer logically happens is lost.  If the callee is
> + * called in a loop, this can cause the variable to go from being
> + * initialized once per loop iteration to being initialized once at
> the
> + * top of the caller and values to persist from one invocation of the
> + * callee to the next.  The simple solution to this problem is to get
> rid
> + * of constant initializers before function inlining.
> + *
> + *  2. nir_lower_returns(shader)
> + *
> + * nir_inline_functions assumes that all functions end "naturally" by
> + * execution reaching the end of the function without any return
> + * instructions causing instant jumps to the end.  Thanks to NIR being
> + * structured, we can't represent arbitrary jumps to various points
> in the
> + * program which is what an early return in the callee would have to
> turn
> + * into when we inline it into the caller.  Instead, we require
> returns to
> + * be lowered which lets us just copy+paste the callee directly into
> the
> + * caller.
> + *
> + *  3. nir_inline_functions(shader)
> + *
> + * This does the actual function inlining and the resulting shader
> will
> + * contain no call instructions.
> + *
> + *  4. nir_copy_prop(shader)
> + *
> + * Most functions contain pointer parameters where the result of a
> deref
> + * instruction is passed in as a parameter, loaded via a load_param
> + * intrinsic, and then turned back into a deref via a cast.  Running
> copy
> + * propagation gets rid of the intermediate steps and results in a
> whole
> + * deref chain again.  This is currently required by a number of
> + * optimizations and lowering passes at least for certain variable
> modes.
> + *
> + *  5. Loop over the functions and delete all but the main entrypoint.
> + *
> + * In the Intel Vulkan driver this looks like this:
> + *
> + *foreach_list_typed_safe(nir_function, func, node,
> &nir->functions) {
> + *   if (func != entry_point)
> + *  exec_node_remove(&func->node);
> + *}
> + *assert(exec_list_length(&nir->functions) == 1);
> + *
> + *While nir_inline_functions does get rid of all call instructions, it
> + *doesn't get rid of any functions because it doesn't know what the
> "root
> + *function" is.  Instead, it's up to the individual driver to know
> how to
> + *decide on a root function and delete the rest.  With SPIR-V,
> + *spirv_to_nir returns the root function and so we can just use ==
> whereas
> + *with GL, you may have to look for a function named "main".
> + *
> + *  6. nir_lower_constant_initializers(shader, ~nir_var_local)
> + *
> + * Lowering constant initializers on inputs, outputs, global
> variables,
> + * etc. requires that we know the main entrypoint so that we know
> where to
> + * initialize them.  Otherwise, we would have to assume that anything
> + * could be a main entrypoint and initialize them at the start of
> every
> + * function but that would clearly be wrong if any of those functions
> were
> + * ever called within another function.  Simply requiring a single-
> + * entrypoint function shader is the best way to make it well-defined.
> + */
>  bool
>  nir_inline_functions(nir_shader *shader)
>  {
> --
> 2.19.1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] mesa: Fix GLES2 OES float texture framebuffer rendering.

[Nick Kreeger]

Please ignore this patch - I accidentally had my conditionals backwards for
half/float float extension checking.

The V3 patch is the main one.

On Tue, Dec 11, 2018 at 7:02 PM Nick Kreeger  wrote:

> This change enables GLES2 to render float/half-float textures to a
> framebuffer when the appropriate OES extensions are available.
>
> This commit regressed OES GLES2 float texture rendering:
>
> https://gitlab.freedesktop.org/mesa/mesa/commit/e333035c47a6a4cc88f0f9ca2bced500538bebae
> ---
>  src/mesa/main/fbobject.c | 20 +++-
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index 23e4939619..8b83297a12 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -869,15 +869,17 @@ test_attachment_completeness(const struct gl_context
> *ctx, GLenum format,
>  return;
>   }
>
> - /* OES_texture_float allows creation and use of floating point
> -  * textures with GL_FLOAT, GL_HALF_FLOAT but it does not allow
> -  * these textures to be used as a render target, this is done via
> -  * GL_EXT_color_buffer(_half)_float with set of new sized types.
> -  */
> - if (_mesa_is_gles(ctx) && (texObj->_IsFloat ||
> texObj->_IsHalfFloat)) {
> -att_incomplete("bad internal format");
> -att->Complete = GL_FALSE;
> -return;
> + if (_mesa_is_gles(ctx)) {
> +   /**
> +* GL ES 2 will allow GL_FLOAT and GL_HALF_FLOAT to render as a
> +* target when the appropriate OES_* extensions are available.
> +*/
> +   if ((texObj->_IsFloat &&
> !_mesa_has_OES_texture_half_float(ctx)) ||
> +   (texObj->_IsHalfFloat &&
> !_mesa_has_OES_texture_float(ctx))) {
> + att_incomplete("bad internal format");
> + att->Complete = GL_FALSE;
> + return;
> +   }
>   }
>}
>else if (format == GL_DEPTH) {
> --
> 2.17.1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v3] mesa: Fix GLES2 OES float texture framebuffer rendering.

[Nick Kreeger]

This change enables GLES2 to render float/half-float textures to a
framebuffer when the appropriate OES extensions are available.

This commit regressed OES GLES2 float texture rendering:
https://gitlab.freedesktop.org/mesa/mesa/commit/e333035c47a6a4cc88f0f9ca2bced500538bebae
---
 src/mesa/main/fbobject.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 23e4939619..cedfc3d81b 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -869,15 +869,17 @@ test_attachment_completeness(const struct gl_context 
*ctx, GLenum format,
 return;
  }
 
- /* OES_texture_float allows creation and use of floating point
-  * textures with GL_FLOAT, GL_HALF_FLOAT but it does not allow
-  * these textures to be used as a render target, this is done via
-  * GL_EXT_color_buffer(_half)_float with set of new sized types.
-  */
- if (_mesa_is_gles(ctx) && (texObj->_IsFloat || texObj->_IsHalfFloat)) 
{
-att_incomplete("bad internal format");
-att->Complete = GL_FALSE;
-return;
+ if (_mesa_is_gles(ctx)) {
+   /**
+* GL ES 2 will allow GL_FLOAT and GL_HALF_FLOAT to render as a
+* target when the appropriate OES_* extensions are available.
+*/
+   if ((texObj->_IsHalfFloat && 
!_mesa_has_OES_texture_half_float(ctx)) ||
+   (texObj->_IsFloat && !_mesa_has_OES_texture_float(ctx))) {
+ att_incomplete("bad internal format");
+ att->Complete = GL_FALSE;
+ return;
+   }
  }
   }
   else if (format == GL_DEPTH) {
-- 
2.17.1

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

[Mesa-dev] [PATCH v2] mesa: Fix GLES2 OES float texture framebuffer rendering.

[Nick Kreeger]

This change enables GLES2 to render float/half-float textures to a
framebuffer when the appropriate OES extensions are available.

This commit regressed OES GLES2 float texture rendering:
https://gitlab.freedesktop.org/mesa/mesa/commit/e333035c47a6a4cc88f0f9ca2bced500538bebae
---
 src/mesa/main/fbobject.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 23e4939619..8b83297a12 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -869,15 +869,17 @@ test_attachment_completeness(const struct gl_context 
*ctx, GLenum format,
 return;
  }
 
- /* OES_texture_float allows creation and use of floating point
-  * textures with GL_FLOAT, GL_HALF_FLOAT but it does not allow
-  * these textures to be used as a render target, this is done via
-  * GL_EXT_color_buffer(_half)_float with set of new sized types.
-  */
- if (_mesa_is_gles(ctx) && (texObj->_IsFloat || texObj->_IsHalfFloat)) 
{
-att_incomplete("bad internal format");
-att->Complete = GL_FALSE;
-return;
+ if (_mesa_is_gles(ctx)) {
+   /**
+* GL ES 2 will allow GL_FLOAT and GL_HALF_FLOAT to render as a
+* target when the appropriate OES_* extensions are available.
+*/
+   if ((texObj->_IsFloat && !_mesa_has_OES_texture_half_float(ctx)) ||
+   (texObj->_IsHalfFloat && !_mesa_has_OES_texture_float(ctx))) {
+ att_incomplete("bad internal format");
+ att->Complete = GL_FALSE;
+ return;
+   }
  }
   }
   else if (format == GL_DEPTH) {
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)

[Jason Ekstrand]

On Tue, Dec 11, 2018 at 8:15 PM Ian Romanick  wrote:

> It's fairly common for Mesa developers to have several patch series on
> the mailing list at a time.  I believe most people will author these as
> a continuous stream with either implicit dependencies (i.e., commit
> messages in the second series with shader-db results that are impacted
> by the first series) or explicit dependencies (i.e., second series uses
> interfaces added or changed by the first).  When I do this, I still like
> to send the series out a separate sets of patches that accomplish
> separate, logical tasks.
>
> We can't be the only project where people work like this, so what is the
> common practice in other projects?  I've thought of several possible
> solutions, but each seems fatally flawed.
>
> - A single, possibly giant, MR defeats the ability to have separate
> logical work units.
>
> - Splitting into multiple MRs based from master may not be practical or
> possible without including some patches in both series.
>
> - Waiting to send the second series out may increase the lag in the
> review process or lead to the submitter forgetting to submit. :)
>

What I would do would be to create a series of MRs that are each based on
the one before.  It gets a little weird because they get longer and longer
but if you have some commentary in the MR header saying that it's based on
another one, it shouldn't be so bad.  If someone is doing patch-by-patch
review, they can easily enough just comment on the patches unique to that
MR.

Sadly, that suggestion won't work quite the way you want as I don't think
GitLab has a concept of dependent MRs.  Maybe that's another thing to add
to the feature request list?


> On 12/5/18 3:32 PM, Jordan Justen wrote:
> > This documents a process for using GitLab Merge Requests as an second
> > way to submit code changes for Mesa. Only one of the two methods is
> > allowed for each patch series.
> >
> > We will *not* require all patches to be emailed. Some code changes may
> > be reviewed and merged without any discussion on the mesa-dev email
> > list.
> >
> > v2:
> >  * No longer require email. Allow submitter to choose email or a
> >GitLab merge request.
> >  * Various feedback from Brian, Daniel, Dylan, Eric, Erik, Jason,
> >Matt, Michel and Rob.
> >
> > Signed-off-by: Jordan Justen 
> > ---
> >  docs/submittingpatches.html | 76 ++---
> >  1 file changed, 71 insertions(+), 5 deletions(-)
> >
> > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
> > index 92d954a2d09..21175988d0b 100644
> > --- a/docs/submittingpatches.html
> > +++ b/docs/submittingpatches.html
> > @@ -21,7 +21,7 @@
> >  Basic guidelines
> >  Patch formatting
> >  Testing Patches
> > -Mailing Patches
> > +Submitting Patches
> >  Reviewing Patches
> >  Nominating a commit for a stable branch
> >  Criteria for accepting patches to the stable
> branch
> > @@ -42,8 +42,10 @@ components.
> >  git bisect.)
> >  Patches should be properly formatted.
> >  Patches should be sufficiently tested before
> submitting.
> > -Patches should be submitted to mesa-dev
> > -for review using git send-email.
> > +Patches should be submitted
> > +to mesa-dev or with
> > +a merge request
> > +for review.
> >
> >  
> >
> > @@ -180,10 +182,19 @@ run.
> >  
> >
> >
> > -Mailing Patches
> > +Submitting Patches
> >
> >  
> > -Patches should be sent to the mesa-dev mailing list for review:
> > +Patches may be submitted to the Mesa project by
> > +email or with a
> > +GitLab merge request. To prevent
> > +duplicate code review, only use one method to submit your changes.
> > +
> > +
> > +Mailing Patches
> > +
> > +
> > +Patches may be sent to the mesa-dev mailing list for review:
> >  https://lists.freedesktop.org/mailman/listinfo/mesa-dev";>
> >  mesa-dev@lists.freedesktop.org.
> >  When submitting a patch make sure to use
> > @@ -217,8 +228,63 @@ disabled before sending your patches. (Note that
> you may need to contact
> >  your email administrator for this.)
> >  
> >
> > +GitLab Merge Requests
> > +
> > +
> > +  https://gitlab.freedesktop.org/mesa/mesa";>GitLab Merge
> > +  Requests (MR) can also be used to submit patches for Mesa.
> > +
> > +
> > +
> > +  If the MR may have interest for most of the Mesa community, you can
> > +  send an email to the mesa-dev email list including a link to the MR.
> > +  Don't send the patch to mesa-dev, just the MR link.
> > +
> > +
> > +  Add labels to your MR to help reviewers find it. For example:
> > +  
> > +Mesa changes affecting all drivers: mesa
> > +Hardware vendor specific code: amd, intel, nvidia, ...
> > +Driver specific code: anvil, freedreno, i965, iris, radeonsi,
> > +  radv, vc4, ...
> > +Other tag examples: gallium, util
> > +  
> > +
> > +
> > +  If you revise your patches based on code review and push an update
> > +  to your branch, you should maintain a clean history
> > +  in your patches. There should not be "fixup" patches i

Re: [Mesa-dev] [PATCH] etnaviv: fix resource usage tracking across different pipe_context's

[Marek Vasut]

On 12/11/2018 10:40 PM, Lukas F. Hartmann wrote:
> Hi,

Hi,

> I tested the patch on i.MX6QP with:
> Linux reform 4.20.0-rc2-00133-g1ce80e0fe98e-dirty #10 SMP Mon Nov 26
> 02:02:42 CET 2018 armv7l GNU/Linux
> 
> Running a recent Xorg built from source with modesetting driver and
> etnaviv.
> 
> I was getting segfaults after a few seconds of usage and tracked them
> down to a corrupt pointer (probably use-after-free):
> 
> http://dump.mntmn.com/marexpatch-trace1.txt.html
> 
> After looking at your patch for a while I came up with this addition
> (last line) in etna_resource.c, to remove the resource from the
> used_resources set when it is destroyed.
> 
> @@ -464,6 +469,9 @@ etna_resource_destroy(struct pipe_screen *pscreen,
> struct pipe_resource *prsc) {
> struct etna_resource *rsc = etna_resource(prsc);
>  
> +   _mesa_set_destroy(rsc->pending_ctx, NULL);
> +   _mesa_set_remove_key(etna_screen(pscreen)->used_resources, rsc);
> 
> I've been running with this version for a few days and it seems stable.
> The patch seems to do its job (tested qupzilla and qutebrowser, no
> more corrupted tiles). Just sometimes an occassional black tile shows
> up. So something is still up, but the patch is a perceived improvement.

Nice, thanks. Lemme try that here too.

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

Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)

[Ian Romanick]

It's fairly common for Mesa developers to have several patch series on
the mailing list at a time.  I believe most people will author these as
a continuous stream with either implicit dependencies (i.e., commit
messages in the second series with shader-db results that are impacted
by the first series) or explicit dependencies (i.e., second series uses
interfaces added or changed by the first).  When I do this, I still like
to send the series out a separate sets of patches that accomplish
separate, logical tasks.

We can't be the only project where people work like this, so what is the
common practice in other projects?  I've thought of several possible
solutions, but each seems fatally flawed.

- A single, possibly giant, MR defeats the ability to have separate
logical work units.

- Splitting into multiple MRs based from master may not be practical or
possible without including some patches in both series.

- Waiting to send the second series out may increase the lag in the
review process or lead to the submitter forgetting to submit. :)

On 12/5/18 3:32 PM, Jordan Justen wrote:
> This documents a process for using GitLab Merge Requests as an second
> way to submit code changes for Mesa. Only one of the two methods is
> allowed for each patch series.
> 
> We will *not* require all patches to be emailed. Some code changes may
> be reviewed and merged without any discussion on the mesa-dev email
> list.
> 
> v2:
>  * No longer require email. Allow submitter to choose email or a
>GitLab merge request.
>  * Various feedback from Brian, Daniel, Dylan, Eric, Erik, Jason,
>Matt, Michel and Rob.
> 
> Signed-off-by: Jordan Justen 
> ---
>  docs/submittingpatches.html | 76 ++---
>  1 file changed, 71 insertions(+), 5 deletions(-)
> 
> diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
> index 92d954a2d09..21175988d0b 100644
> --- a/docs/submittingpatches.html
> +++ b/docs/submittingpatches.html
> @@ -21,7 +21,7 @@
>  Basic guidelines
>  Patch formatting
>  Testing Patches
> -Mailing Patches
> +Submitting Patches
>  Reviewing Patches
>  Nominating a commit for a stable branch
>  Criteria for accepting patches to the stable 
> branch
> @@ -42,8 +42,10 @@ components.
>  git bisect.)
>  Patches should be properly formatted.
>  Patches should be sufficiently tested before 
> submitting.
> -Patches should be submitted to mesa-dev
> -for review using git send-email.
> +Patches should be submitted
> +to mesa-dev or with
> +a merge request
> +for review.
>  
>  
>  
> @@ -180,10 +182,19 @@ run.
>  
>  
>  
> -Mailing Patches
> +Submitting Patches
>  
>  
> -Patches should be sent to the mesa-dev mailing list for review:
> +Patches may be submitted to the Mesa project by
> +email or with a
> +GitLab merge request. To prevent
> +duplicate code review, only use one method to submit your changes.
> +
> +
> +Mailing Patches
> +
> +
> +Patches may be sent to the mesa-dev mailing list for review:
>  https://lists.freedesktop.org/mailman/listinfo/mesa-dev";>
>  mesa-dev@lists.freedesktop.org.
>  When submitting a patch make sure to use
> @@ -217,8 +228,63 @@ disabled before sending your patches. (Note that you may 
> need to contact
>  your email administrator for this.)
>  
>  
> +GitLab Merge Requests
> +
> +
> +  https://gitlab.freedesktop.org/mesa/mesa";>GitLab Merge
> +  Requests (MR) can also be used to submit patches for Mesa.
> +
> +
> +
> +  If the MR may have interest for most of the Mesa community, you can
> +  send an email to the mesa-dev email list including a link to the MR.
> +  Don't send the patch to mesa-dev, just the MR link.
> +
> +
> +  Add labels to your MR to help reviewers find it. For example:
> +  
> +Mesa changes affecting all drivers: mesa
> +Hardware vendor specific code: amd, intel, nvidia, ...
> +Driver specific code: anvil, freedreno, i965, iris, radeonsi,
> +  radv, vc4, ...
> +Other tag examples: gallium, util
> +  
> +
> +
> +  If you revise your patches based on code review and push an update
> +  to your branch, you should maintain a clean history
> +  in your patches. There should not be "fixup" patches in the history.
> +  The series should be buildable and functional after every commit
> +  whenever you push the branch.
> +
> +
> +  It is your responsibility to keep the MR alive and making progress,
> +  as there are no guarantees that a Mesa dev will independently take
> +  interest in it.
> +
> +
> +  Some other notes:
> +  
> +Make changes and update your branch based on feedback
> +Old, stale MR may be closed, but you can reopen it if you
> +  still want to pursue the changes
> +You should periodically check to see if your MR needs to be
> +  rebased
> +Make sure your MR is closed if your patches get pushed outside
> +  of GitLab
> +  
> +
> +
>  Reviewing Patches
>  
> +
> +  To participate in code review, you should monitor the
> +  https://lists.freedesktop.org/mailman/listinfo/

Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)

[Rob Clark]

On Tue, Dec 11, 2018 at 7:06 PM Jason Ekstrand  wrote:
>
> Ping?
>
> I see about 5 acks/reviews, 3 of which are from Intel which doesn't exactly 
> seem like overwhelming consensus.  However, we also haven't had any debate in 
> a while.
>
> I know some people are somewhat skeptical as to how well it will work but 
> they won't be able to see until we actually start experimenting with it which 
> we can't do until we allow MRs.  Personally, I say we should just turn on the 
> option in GitLab and people can start playing with it and see how it goes.  
> We can always decide later that it was a terrible idea and move all active 
> MRs back to the list.
>

I think some way to get cover-letters or similar (pref w/ link to mr)
to list would be nice, but I'm defn ok w/ 'lets try it and then fine
tune' approach.  I think worst case is I overlook mr notification spam
and someone has to poke me on irc, which is more or less the current
worst case...

Acked-by: Rob Clark 


> --Jason
>
> On Fri, Dec 7, 2018 at 2:24 PM Daniel Stone  wrote:
>>
>> Hi,
>>
>> On Sat, 8 Dec 2018 at 05:15, Eric Engestrom  wrote:
>> > On Friday, 2018-12-07 10:19:23 +0100, Erik Faye-Lund wrote:
>> > > Automated emails (and perhaps IRC bot) would be really nice.
>> >
>> > Agreed. Email would be great to help with the transition.
>> > There's work currently being done on GitLab to allow for mailing lists
>> > to be notified; this should cover 'new MR' as well.
>> > If we need this feature before GitLab is done, it should be possible to
>> > write a bot using the webhooks, just needs someone to take the time to
>> > do it :)
>> >
>> > For IRC, there's already some integration, but it's limited to notifying
>> > about git pushes for now:
>> > https://docs.gitlab.com/ee/user/project/integrations/irker.html
>> >
>> > There's an open issue about adding more events, but it hasn't seen much
>> > activity:
>> > https://gitlab.com/gitlab-org/gitlab-ce/issues/7965
>>
>> Wayland uses a couple of eventd plugins chained together:
>> https://github.com/sardemff7/git-eventc
>>
>> That notifies the channel when issues and MRs are opened or closed and
>> on push as well, including things like the labels. It's been pretty
>> useful so far.
>>
>> > > Even better if it could be hooked up to scripts/get_reviewer.pl, and
>> > > automatically CC "the right people".
>> >
>> > Side note, I've been rewriting that script, although I need to send v2
>> > out at some point:
>> > https://patchwork.freedesktop.org/patch/226256/
>> >
>> > I would be trivial to hook that into a bot we'd write, but I don't think
>> > GitLab has support for something like this. I just opened an issue about
>> > adding support directly in GitLab:
>> > https://gitlab.com/gitlab-org/gitlab-ce/issues/55035
>>
>> This already exists, as an EE-only feature called 'code owners':
>> https://docs.gitlab.com/ee/user/project/code_owners.html
>> https://gitlab.com/gitlab-org/gitlab-ee/issues/1012
>>
>> Cheers,
>> Daniel
>> ___
>> 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] docs: Document GitLab merge request process (email alternative)

[Bas Nieuwenhuizen]

On Wed, Dec 12, 2018 at 1:06 AM Jason Ekstrand  wrote:
>
> Ping?
>
> I see about 5 acks/reviews, 3 of which are from Intel which doesn't exactly 
> seem like overwhelming consensus.  However, we also haven't had any debate in 
> a while.

FWIW, I'm fine with it too.

Acked-by: Bas Nieuwenhuizen 


>
> I know some people are somewhat skeptical as to how well it will work but 
> they won't be able to see until we actually start experimenting with it which 
> we can't do until we allow MRs.  Personally, I say we should just turn on the 
> option in GitLab and people can start playing with it and see how it goes.  
> We can always decide later that it was a terrible idea and move all active 
> MRs back to the list.
>
> --Jason
>
> On Fri, Dec 7, 2018 at 2:24 PM Daniel Stone  wrote:
>>
>> Hi,
>>
>> On Sat, 8 Dec 2018 at 05:15, Eric Engestrom  wrote:
>> > On Friday, 2018-12-07 10:19:23 +0100, Erik Faye-Lund wrote:
>> > > Automated emails (and perhaps IRC bot) would be really nice.
>> >
>> > Agreed. Email would be great to help with the transition.
>> > There's work currently being done on GitLab to allow for mailing lists
>> > to be notified; this should cover 'new MR' as well.
>> > If we need this feature before GitLab is done, it should be possible to
>> > write a bot using the webhooks, just needs someone to take the time to
>> > do it :)
>> >
>> > For IRC, there's already some integration, but it's limited to notifying
>> > about git pushes for now:
>> > https://docs.gitlab.com/ee/user/project/integrations/irker.html
>> >
>> > There's an open issue about adding more events, but it hasn't seen much
>> > activity:
>> > https://gitlab.com/gitlab-org/gitlab-ce/issues/7965
>>
>> Wayland uses a couple of eventd plugins chained together:
>> https://github.com/sardemff7/git-eventc
>>
>> That notifies the channel when issues and MRs are opened or closed and
>> on push as well, including things like the labels. It's been pretty
>> useful so far.
>>
>> > > Even better if it could be hooked up to scripts/get_reviewer.pl, and
>> > > automatically CC "the right people".
>> >
>> > Side note, I've been rewriting that script, although I need to send v2
>> > out at some point:
>> > https://patchwork.freedesktop.org/patch/226256/
>> >
>> > I would be trivial to hook that into a bot we'd write, but I don't think
>> > GitLab has support for something like this. I just opened an issue about
>> > adding support directly in GitLab:
>> > https://gitlab.com/gitlab-org/gitlab-ce/issues/55035
>>
>> This already exists, as an EE-only feature called 'code owners':
>> https://docs.gitlab.com/ee/user/project/code_owners.html
>> https://gitlab.com/gitlab-org/gitlab-ee/issues/1012
>>
>> Cheers,
>> Daniel
>> ___
>> 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] [RFC PATCH 12/14] anv/allocator: Rework chunk return to the state pool.

[Jason Ekstrand]

On Tue, Dec 11, 2018 at 5:31 PM Rafael Antognolli <
rafael.antogno...@intel.com> wrote:

> On Mon, Dec 10, 2018 at 11:10:02PM -0600, Jason Ekstrand wrote:
> >
> >
> > On Mon, Dec 10, 2018 at 5:48 PM Rafael Antognolli <
> rafael.antogno...@intel.com>
> > wrote:
> >
> > On Mon, Dec 10, 2018 at 04:56:40PM -0600, Jason Ekstrand wrote:
> > > On Fri, Dec 7, 2018 at 6:06 PM Rafael Antognolli <
> > rafael.antogno...@intel.com>
> > > wrote:
> > >
> > > This commit tries to rework the code that split and returns
> chunks
> > back
> > > to the state pool, while still keeping the same logic.
> > >
> > > The original code would get a chunk larger than we need and
> split it
> > > into pool->block_size. Then it would return all but the first
> one,
> > and
> > > would split that first one into alloc_size chunks. Then it
> would keep
> > > the first one (for the allocation), and return the others back
> to the
> > > pool.
> > >
> > > The new anv_state_pool_return_chunk() function will take a
> chunk
> > (with
> > > the alloc_size part removed), and a small_size hint. It then
> splits
> > that
> > > chunk into pool->block_size'd chunks, and if there's some
> space still
> > > left, split that into small_size chunks. small_size in this
> case is
> > the
> > > same size as alloc_size.
> > >
> > > The idea is to keep the same logic, but make it in a way we
> can reuse
> > it
> > > to return other chunks to the pool when we are growing the
> buffer.
> > > ---
> > >  src/intel/vulkan/anv_allocator.c | 147
> > +--
> > >  1 file changed, 102 insertions(+), 45 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/anv_allocator.c
> b/src/intel/vulkan/
> > > anv_allocator.c
> > > index 31258e38635..bddeb4a0fbd 100644
> > > --- a/src/intel/vulkan/anv_allocator.c
> > > +++ b/src/intel/vulkan/anv_allocator.c
> > > @@ -994,6 +994,97 @@ anv_state_pool_get_bucket_size(uint32_t
> bucket)
> > > return 1 << size_log2;
> > >  }
> > >
> > > +/** Helper to create a chunk into the state table.
> > > + *
> > > + * It just creates 'count' entries into the state table and
> update
> > their
> > > sizes,
> > > + * offsets and maps, also pushing them as "free" states.
> > > + */
> > > +static void
> > > +anv_state_pool_return_blocks(struct anv_state_pool *pool,
> > > + uint32_t chunk_offset, uint32_t
> count,
> > > + uint32_t block_size)
> > > +{
> > > +   if (count == 0)
> > > +  return;
> > > +
> > > +   uint32_t st_idx = anv_state_table_add(&pool->table, count);
> > > +   for (int i = 0; i < count; i++) {
> > > +  /* update states that were added back to the state
> table */
> > > +  struct anv_state *state_i =
> anv_state_table_get(&pool->table,
> > > +  st_idx
> + i);
> > > +  state_i->alloc_size = block_size;
> > > +  state_i->offset = chunk_offset + block_size * i;
> > > +  struct anv_pool_map pool_map =
> anv_block_pool_map(&pool->
> > block_pool,
> > > +
> state_i->
> > offset);
> > > +  state_i->map = pool_map.map + pool_map.offset;
> > > +   }
> > > +
> > > +   uint32_t block_bucket =
> anv_state_pool_get_bucket(block_size);
> > > +
>  anv_state_table_push(&pool->buckets[block_bucket].free_list,
> > > +&pool->table, st_idx, count);
> > > +}
> > > +
> > > +static uint32_t
> > > +calculate_divisor(uint32_t size)
> > > +{
> > > +   uint32_t bucket = anv_state_pool_get_bucket(size);
> > > +
> > > +   while (bucket >= 0) {
> > > +  uint32_t bucket_size =
> anv_state_pool_get_bucket_size(bucket);
> > > +  if (size % bucket_size == 0)
> > > + return bucket_size;
> > > +   }
> > > +
> > > +   return 0;
> > > +}
> > > +
> > > +/** Returns a chunk of memory back to the state pool.
> > > + *
> > > + * If small_size is zero, we split chunk_size into pool->
> > block_size'd
> > > pieces,
> > > + * and return those. If there's some remaining 'rest' space
> > (chunk_size is
> > > not
> > > + * divisble by pool->block_size), then we find a bucket size
> that is
> > a
> > > divisor
> > > + * of that rest, and split the 'rest' into that size,
> returning it
> > to the
> > > pool.
> > > + *
> > > + * If small_size 

Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)

[Jason Ekstrand]

Ping?

I see about 5 acks/reviews, 3 of which are from Intel which doesn't exactly
seem like overwhelming consensus.  However, we also haven't had any debate
in a while.

I know some people are somewhat skeptical as to how well it will work but
they won't be able to see until we actually start experimenting with it
which we can't do until we allow MRs.  Personally, I say we should just
turn on the option in GitLab and people can start playing with it and see
how it goes.  We can always decide later that it was a terrible idea and
move all active MRs back to the list.

--Jason

On Fri, Dec 7, 2018 at 2:24 PM Daniel Stone  wrote:

> Hi,
>
> On Sat, 8 Dec 2018 at 05:15, Eric Engestrom 
> wrote:
> > On Friday, 2018-12-07 10:19:23 +0100, Erik Faye-Lund wrote:
> > > Automated emails (and perhaps IRC bot) would be really nice.
> >
> > Agreed. Email would be great to help with the transition.
> > There's work currently being done on GitLab to allow for mailing lists
> > to be notified; this should cover 'new MR' as well.
> > If we need this feature before GitLab is done, it should be possible to
> > write a bot using the webhooks, just needs someone to take the time to
> > do it :)
> >
> > For IRC, there's already some integration, but it's limited to notifying
> > about git pushes for now:
> > https://docs.gitlab.com/ee/user/project/integrations/irker.html
> >
> > There's an open issue about adding more events, but it hasn't seen much
> > activity:
> > https://gitlab.com/gitlab-org/gitlab-ce/issues/7965
>
> Wayland uses a couple of eventd plugins chained together:
> https://github.com/sardemff7/git-eventc
>
> That notifies the channel when issues and MRs are opened or closed and
> on push as well, including things like the labels. It's been pretty
> useful so far.
>
> > > Even better if it could be hooked up to scripts/get_reviewer.pl, and
> > > automatically CC "the right people".
> >
> > Side note, I've been rewriting that script, although I need to send v2
> > out at some point:
> > https://patchwork.freedesktop.org/patch/226256/
> >
> > I would be trivial to hook that into a bot we'd write, but I don't think
> > GitLab has support for something like this. I just opened an issue about
> > adding support directly in GitLab:
> > https://gitlab.com/gitlab-org/gitlab-ce/issues/55035
>
> This already exists, as an EE-only feature called 'code owners':
> https://docs.gitlab.com/ee/user/project/code_owners.html
> https://gitlab.com/gitlab-org/gitlab-ee/issues/1012
>
> Cheers,
> Daniel
> ___
> 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 4/6] mesa/st: wire up DiscardSubFramebuffer

[Ilia Mirkin]

On Tue, Dec 11, 2018 at 6:34 PM Rob Clark  wrote:
>
> On Tue, Dec 11, 2018 at 6:10 PM Ilia Mirkin  wrote:
> >
> > On Tue, Dec 11, 2018 at 5:50 PM Rob Clark  wrote:
> > >
> > > Signed-off-by: Rob Clark 
> > > ---
> > >  src/gallium/include/pipe/p_context.h | 11 +++
> > >  src/mesa/state_tracker/st_cb_fbo.c   | 26 ++
> > >  2 files changed, 37 insertions(+)
> > >
> > > diff --git a/src/gallium/include/pipe/p_context.h 
> > > b/src/gallium/include/pipe/p_context.h
> > > index d4e9179b78a..eb52c7e9a4e 100644
> > > --- a/src/gallium/include/pipe/p_context.h
> > > +++ b/src/gallium/include/pipe/p_context.h
> > > @@ -811,6 +811,17 @@ struct pipe_context {
> > > void (*invalidate_surface)(struct pipe_context *ctx,
> > >struct pipe_surface *surf);
> > >
> > > +   /**
> > > +* Invalidate a portion of a surface.  This is used to
> > > +*
> > > +* (1) implement glInvalidateSubFramebuffer() and friends
> > > +* (2) as a hint before a scissored clear (which is turned into 
> > > draw_vbo()
> > > +* that the cleared rect can be discarded
> > > +*/
> > > +   void (*invalidate_sub_surface)(struct pipe_context *ctx,
> > > +  struct pipe_surface *surf,
> > > +  const struct pipe_scissor_state *rect);
> >
> > I think pipe_box is a bit more natural here than scissor state? box is
> > used for transfers, etc. This could also enable you to to specify a
> > sub-3d area. And perhaps by passing a level, you could go back to this
> > being a resource (and just extend invalidate_resource)?
>
> I prefer surface to resource+params, as I mentioned in the other reply
> on this thread.
>
> I guess box is an option.. it is overkill for what the gl API wants
> and I can't immediately think of a use for this for more than a 2d
> slice of a higher dimensioned object, where pipe_surface is a perfect
> fit.  (Are there any GPUs that can render to a 3d render target?  I
> guess at least the intersection of tilers and GPUs that can is pretty
> small for now..)

Any DX10- or GL 3.2-capable GPU can render to multiple layers at once.
You bind a bunch of layers at a time.

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

Re: [Mesa-dev] [PATCH 1/6] mesa: wire up InvalidateFramebuffer

[Ian Romanick]

On 12/11/18 3:15 PM, Rob Clark wrote:
> On Tue, Dec 11, 2018 at 6:06 PM Ian Romanick  wrote:
>>
>> On 12/11/18 2:50 PM, Rob Clark wrote:
>>> And before someone actually starts implementing DiscardFramebuffer()
>>> lets rework the interface to something that is actually usable.
>>>
>>> Signed-off-by: Rob Clark 
>>> ---
>>>  src/mesa/main/dd.h   |  5 +--
>>>  src/mesa/main/fbobject.c | 79 ++--
>>>  2 files changed, 77 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
>>> index f14c3e04e91..1214eeaa474 100644
>>> --- a/src/mesa/main/dd.h
>>> +++ b/src/mesa/main/dd.h
>>> @@ -784,9 +784,8 @@ struct dd_function_table {
>>> GLint srcX0, GLint srcY0, GLint srcX1, GLint 
>>> srcY1,
>>> GLint dstX0, GLint dstY0, GLint dstX1, GLint 
>>> dstY1,
>>> GLbitfield mask, GLenum filter);
>>> -   void (*DiscardFramebuffer)(struct gl_context *ctx,
>>> -  GLenum target, GLsizei numAttachments,
>>> -  const GLenum *attachments);
>>> +   void (*DiscardFramebuffer)(struct gl_context *ctx, struct 
>>> gl_framebuffer *fb,
>>> +  struct gl_renderbuffer_attachment *att);
>>>
>>> /**
>>>  * \name Functions for GL_ARB_sample_locations
>>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>>> index 23e49396199..f931e8f76b1 100644
>>> --- a/src/mesa/main/fbobject.c
>>> +++ b/src/mesa/main/fbobject.c
>>> @@ -4637,6 +4637,67 @@ invalid_enum:
>>> return;
>>>  }
>>>
>>> +static struct gl_renderbuffer_attachment *
>>> +get_fb_attachment(struct gl_context *ctx, struct gl_framebuffer *fb,
>>> +  const GLenum attachment)
>>> +{
>>> +   GLint idx;
>>
>> Nancy Reagan says "Just say no... to silly GL types."  I'd also be
>> inclined to move this...
> 
> defn agree.. that just seemed to be the 'when in Rome' convention
> 
> But I don't mind bucking convention

This used to be the convention, but I think we stopped using GL types
for things that aren't exposed to the application some time ago.  I
think Eric (wisely) led that charge.  Then it became another one of
those cases where we didn't go back and change everything.

Which reminds me...

>>> +
>>> +   switch (attachment) {
>>> +   case GL_COLOR:
>>> +   case GL_COLOR_ATTACHMENT0_EXT:
>>> +   case GL_COLOR_ATTACHMENT1_EXT:
>>> +   case GL_COLOR_ATTACHMENT2_EXT:
>>> +   case GL_COLOR_ATTACHMENT3_EXT:
>>> +   case GL_COLOR_ATTACHMENT4_EXT:
>>> +   case GL_COLOR_ATTACHMENT5_EXT:
>>> +   case GL_COLOR_ATTACHMENT6_EXT:
>>> +   case GL_COLOR_ATTACHMENT7_EXT:
>>> +   case GL_COLOR_ATTACHMENT8_EXT:
>>> +   case GL_COLOR_ATTACHMENT9_EXT:
>>> +   case GL_COLOR_ATTACHMENT10_EXT:
>>> +   case GL_COLOR_ATTACHMENT11_EXT:
>>> +   case GL_COLOR_ATTACHMENT12_EXT:
>>> +   case GL_COLOR_ATTACHMENT13_EXT:
>>> +   case GL_COLOR_ATTACHMENT14_EXT:
>>> +   case GL_COLOR_ATTACHMENT15_EXT:

s/_EXT// here and elsewhere. :)

>>> +  if (attachment == GL_COLOR) {
>>> + idx = 0;
>>> +  } else {
>>> + idx = attachment - GL_COLOR_ATTACHMENT0_EXT;
>>> +  }
>>
>> ...here and do
>>
>>   const unsigned idx = attachment == GL_COLOR ? 0 : attachment - 
>> GL_COLOR_ATTACHMENT0_EXT;
>>
> 
> mostly just trying to keep it in 80(ish) columns

Yeah... I'll usually break these at the " = " if they get too long.  I
don't know if we actually have a proper coding style guideline for this.
 I know Matt does something different from what I do.  Not sure about
Ken or Jason. *shrug*

>> but that's just my personal preference.
>>
>>> +  return &fb->Attachment[BUFFER_COLOR0 + idx];
>>> +   case GL_DEPTH:
>>> +   case GL_DEPTH_ATTACHMENT_EXT:
>>> +   case GL_DEPTH_STENCIL_ATTACHMENT:
>>> +  return &fb->Attachment[BUFFER_DEPTH];
>>> +   case GL_STENCIL:
>>> +   case GL_STENCIL_ATTACHMENT_EXT:
>>> +  return &fb->Attachment[BUFFER_STENCIL];
>>> +   default:
>>> +  return NULL;
>>> +   }
>>> +}
>>> +
>>> +static void
>>> +discard_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
>>> +GLsizei numAttachments, const GLenum *attachments)
>>> +{
>>> +   GLint i;
>>> +
>>> +   if (!ctx->Driver.DiscardFramebuffer)
>>> +  return;
>>> +
>>> +   for (i = 0; i < numAttachments; i++) {
>>
>> I'd definitely move 'int i' inside the for.
> 
> will do
> 
>>
>> With at least s/GLint/int/ or similar, this patch is
>>
>> Reviewed-by: Ian Romanick 
> 
> thanks
> 
> BR,
> -R
> 
>>
>>> +  struct gl_renderbuffer_attachment *att =
>>> +get_fb_attachment(ctx, fb, attachments[i]);
>>> +
>>> +  if (!att)
>>> + continue;
>>> +
>>> +  ctx->Driver.DiscardFramebuffer(ctx, fb, att);
>>> +   }
>>> +}
>>>
>>>  void GLAPIENTRY
>>>  _mesa_InvalidateSubFramebuffer_no_error(GLenum target, GLsizei 
>>> numAttachments,
>>> @@ -4695,14 +4756,23 @@ _mesa_InvalidateNamedFramebufferSubData(GLuint 

Re: [Mesa-dev] [PATCH 4/6] mesa/st: wire up DiscardSubFramebuffer

[Rob Clark]

On Tue, Dec 11, 2018 at 6:10 PM Ilia Mirkin  wrote:
>
> On Tue, Dec 11, 2018 at 5:50 PM Rob Clark  wrote:
> >
> > Signed-off-by: Rob Clark 
> > ---
> >  src/gallium/include/pipe/p_context.h | 11 +++
> >  src/mesa/state_tracker/st_cb_fbo.c   | 26 ++
> >  2 files changed, 37 insertions(+)
> >
> > diff --git a/src/gallium/include/pipe/p_context.h 
> > b/src/gallium/include/pipe/p_context.h
> > index d4e9179b78a..eb52c7e9a4e 100644
> > --- a/src/gallium/include/pipe/p_context.h
> > +++ b/src/gallium/include/pipe/p_context.h
> > @@ -811,6 +811,17 @@ struct pipe_context {
> > void (*invalidate_surface)(struct pipe_context *ctx,
> >struct pipe_surface *surf);
> >
> > +   /**
> > +* Invalidate a portion of a surface.  This is used to
> > +*
> > +* (1) implement glInvalidateSubFramebuffer() and friends
> > +* (2) as a hint before a scissored clear (which is turned into 
> > draw_vbo()
> > +* that the cleared rect can be discarded
> > +*/
> > +   void (*invalidate_sub_surface)(struct pipe_context *ctx,
> > +  struct pipe_surface *surf,
> > +  const struct pipe_scissor_state *rect);
>
> I think pipe_box is a bit more natural here than scissor state? box is
> used for transfers, etc. This could also enable you to to specify a
> sub-3d area. And perhaps by passing a level, you could go back to this
> being a resource (and just extend invalidate_resource)?

I prefer surface to resource+params, as I mentioned in the other reply
on this thread.

I guess box is an option.. it is overkill for what the gl API wants
and I can't immediately think of a use for this for more than a 2d
slice of a higher dimensioned object, where pipe_surface is a perfect
fit.  (Are there any GPUs that can render to a 3d render target?  I
guess at least the intersection of tilers and GPUs that can is pretty
small for now..)

BR,
-R

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

Re: [Mesa-dev] [RFC PATCH 10/14] anv: Add clflush to states.

[Jason Ekstrand]

On Tue, Dec 11, 2018 at 5:32 PM Rafael Antognolli <
rafael.antogno...@intel.com> wrote:

> On Mon, Dec 10, 2018 at 01:52:15PM -0600, Jason Ekstrand wrote:
> > This seems very much over-the-top.  It would be better to either find the
> > specific bug or else just allocate the BOs we use for states as
> snooped.  See
> > also the anv_gem_set_caching call in genX_query.c.
>
> It seems we were missing some flushes in the states allocated in
> anv_shader_bin_create(). I added them there and apparently I don't need
> this patch anymore.
>

Glad you found it!

--Jason


> > On Fri, Dec 7, 2018 at 6:06 PM Rafael Antognolli <
> rafael.antogno...@intel.com>
> > wrote:
> >
> > TODO: This is just flushing the entire dynamic states on every
> execbuf.
> > Maybe it's too much. However, in theory we should be already flushing
> > the states as needed, but I think we didn't hit any bug due to the
> > coherence implied by userptr.
> > ---
> >  src/intel/vulkan/anv_batch_chain.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/
> > anv_batch_chain.c
> > index 65df28ccb91..99009679435 100644
> > --- a/src/intel/vulkan/anv_batch_chain.c
> > +++ b/src/intel/vulkan/anv_batch_chain.c
> > @@ -1366,6 +1366,10 @@ anv_reloc_list_add_dep(struct anv_cmd_buffer
> > *cmd_buffer,
> >
> > anv_block_pool_foreach_bo(bo_list, iter, bo) {
> >_mesa_set_add(relocs->deps, bo);
> > +  if (!cmd_buffer->device->info.has_llc) {
> > + for (uint32_t i = 0; i < bo->size; i += CACHELINE_SIZE)
> > +__builtin_ia32_clflush(bo->map + i);
> > +  }
> > }
> >  }
> >
> > --
> > 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 10/14] anv: Add clflush to states.

[Rafael Antognolli]

On Mon, Dec 10, 2018 at 01:52:15PM -0600, Jason Ekstrand wrote:
> This seems very much over-the-top.  It would be better to either find the
> specific bug or else just allocate the BOs we use for states as snooped.  See
> also the anv_gem_set_caching call in genX_query.c.

It seems we were missing some flushes in the states allocated in
anv_shader_bin_create(). I added them there and apparently I don't need
this patch anymore.

> On Fri, Dec 7, 2018 at 6:06 PM Rafael Antognolli 
> wrote:
> 
> TODO: This is just flushing the entire dynamic states on every execbuf.
> Maybe it's too much. However, in theory we should be already flushing
> the states as needed, but I think we didn't hit any bug due to the
> coherence implied by userptr.
> ---
>  src/intel/vulkan/anv_batch_chain.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/
> anv_batch_chain.c
> index 65df28ccb91..99009679435 100644
> --- a/src/intel/vulkan/anv_batch_chain.c
> +++ b/src/intel/vulkan/anv_batch_chain.c
> @@ -1366,6 +1366,10 @@ anv_reloc_list_add_dep(struct anv_cmd_buffer
> *cmd_buffer,
> 
> anv_block_pool_foreach_bo(bo_list, iter, bo) {
>_mesa_set_add(relocs->deps, bo);
> +  if (!cmd_buffer->device->info.has_llc) {
> + for (uint32_t i = 0; i < bo->size; i += CACHELINE_SIZE)
> +__builtin_ia32_clflush(bo->map + i);
> +  }
> }
>  }
> 
> --
> 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 4/6] mesa/st: wire up DiscardSubFramebuffer

[Rob Clark]

On Tue, Dec 11, 2018 at 6:08 PM Roland Scheidegger  wrote:
>
> Am 11.12.18 um 23:50 schrieb Rob Clark:
> > Signed-off-by: Rob Clark 
> > ---
> >  src/gallium/include/pipe/p_context.h | 11 +++
> >  src/mesa/state_tracker/st_cb_fbo.c   | 26 ++
> >  2 files changed, 37 insertions(+)
> >
> > diff --git a/src/gallium/include/pipe/p_context.h 
> > b/src/gallium/include/pipe/p_context.h
> > index d4e9179b78a..eb52c7e9a4e 100644
> > --- a/src/gallium/include/pipe/p_context.h
> > +++ b/src/gallium/include/pipe/p_context.h
> > @@ -811,6 +811,17 @@ struct pipe_context {
> > void (*invalidate_surface)(struct pipe_context *ctx,
> >struct pipe_surface *surf);
> >
> > +   /**
> > +* Invalidate a portion of a surface.  This is used to
> > +*
> > +* (1) implement glInvalidateSubFramebuffer() and friends
> > +* (2) as a hint before a scissored clear (which is turned into 
> > draw_vbo()
> > +* that the cleared rect can be discarded
> > +*/
> > +   void (*invalidate_sub_surface)(struct pipe_context *ctx,
> > +  struct pipe_surface *surf,
> > +  const struct pipe_scissor_state *rect);
> > +
>
> The same applies as to the previous change. Additionally, I think we
> really don't need 3 functions essentially doing the same thing. Ok I
> could see that maybe there's value passing in the surface directly
> (rather than mip levels, layers), but surely
> invalidate_surface/invalidate_sub_surface looks like overkill to me.
> Maybe pass in a NULL pointer for rect if you want to clear everything?

Will splitup gallium+docs and mesa/st, as you mentioned..

I guess rect==NULL is ok by me for avoiding invalidate_surface vs
invalidate_sub_surface

I'm not a huge fan of keeping this at the pipe_resource level and
passing layer/level/etc since in the driver it works out well to track
this at fd_surface rather than fd_resource level, and I don't want to
have to try and recover the surface from resource+params.

BR,
-R


>
> Roland
>
>
> > /**
> >  * Return information about unexpected device resets.
> >  */
> > diff --git a/src/mesa/state_tracker/st_cb_fbo.c 
> > b/src/mesa/state_tracker/st_cb_fbo.c
> > index 3ece1d4a9de..50c27ea51d9 100644
> > --- a/src/mesa/state_tracker/st_cb_fbo.c
> > +++ b/src/mesa/state_tracker/st_cb_fbo.c
> > @@ -774,6 +774,31 @@ st_discard_framebuffer(struct gl_context *ctx, struct 
> > gl_framebuffer *fb,
> >st->pipe->invalidate_surface(st->pipe, psurf);
> >  }
> >
> > +static void
> > +st_discard_sub_framebuffer(struct gl_context *ctx, struct gl_framebuffer 
> > *fb,
> > +   struct gl_renderbuffer_attachment *att, GLint x,
> > +   GLint y, GLsizei width, GLsizei height)
> > +{
> > +   struct st_context *st = st_context(ctx);
> > +   struct pipe_surface *psurf;
> > +
> > +   if (!att->Renderbuffer)
> > +  return;
> > +
> > +   psurf = st_renderbuffer(att->Renderbuffer)->surface;
> > +
> > +   if (st->pipe->invalidate_sub_surface) {
> > +  struct pipe_scissor_state rect;
> > +
> > +  rect.minx = x;
> > +  rect.maxx = x + width - 1;
> > +  rect.miny = y;
> > +  rect.maxy = y + height - 1;
> > +
> > +  st->pipe->invalidate_sub_surface(st->pipe, psurf, &rect);
> > +   }
> > +}
> > +
> >  /**
> >   * Called via glDrawBuffer.  We only provide this driver function so that 
> > we
> >   * can check if we need to allocate a new renderbuffer.  Specifically, we
> > @@ -952,6 +977,7 @@ st_init_fbo_functions(struct dd_function_table 
> > *functions)
> > functions->FinishRenderTexture = st_finish_render_texture;
> > functions->ValidateFramebuffer = st_validate_framebuffer;
> > functions->DiscardFramebuffer = st_discard_framebuffer;
> > +   functions->DiscardSubFramebuffer = st_discard_sub_framebuffer;
> >
> > functions->DrawBufferAllocate = st_DrawBufferAllocate;
> > functions->ReadBuffer = st_ReadBuffer;
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH 12/14] anv/allocator: Rework chunk return to the state pool.

[Rafael Antognolli]

On Mon, Dec 10, 2018 at 11:10:02PM -0600, Jason Ekstrand wrote:
> 
> 
> On Mon, Dec 10, 2018 at 5:48 PM Rafael Antognolli 
> 
> wrote:
> 
> On Mon, Dec 10, 2018 at 04:56:40PM -0600, Jason Ekstrand wrote:
> > On Fri, Dec 7, 2018 at 6:06 PM Rafael Antognolli <
> rafael.antogno...@intel.com>
> > wrote:
> >
> > This commit tries to rework the code that split and returns chunks
> back
> > to the state pool, while still keeping the same logic.
> >
> > The original code would get a chunk larger than we need and split it
> > into pool->block_size. Then it would return all but the first one,
> and
> > would split that first one into alloc_size chunks. Then it would 
> keep
> > the first one (for the allocation), and return the others back to 
> the
> > pool.
> >
> > The new anv_state_pool_return_chunk() function will take a chunk
> (with
> > the alloc_size part removed), and a small_size hint. It then splits
> that
> > chunk into pool->block_size'd chunks, and if there's some space 
> still
> > left, split that into small_size chunks. small_size in this case is
> the
> > same size as alloc_size.
> >
> > The idea is to keep the same logic, but make it in a way we can 
> reuse
> it
> > to return other chunks to the pool when we are growing the buffer.
> > ---
> >  src/intel/vulkan/anv_allocator.c | 147
> +--
> >  1 file changed, 102 insertions(+), 45 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/
> > anv_allocator.c
> > index 31258e38635..bddeb4a0fbd 100644
> > --- a/src/intel/vulkan/anv_allocator.c
> > +++ b/src/intel/vulkan/anv_allocator.c
> > @@ -994,6 +994,97 @@ anv_state_pool_get_bucket_size(uint32_t bucket)
> > return 1 << size_log2;
> >  }
> >
> > +/** Helper to create a chunk into the state table.
> > + *
> > + * It just creates 'count' entries into the state table and update
> their
> > sizes,
> > + * offsets and maps, also pushing them as "free" states.
> > + */
> > +static void
> > +anv_state_pool_return_blocks(struct anv_state_pool *pool,
> > + uint32_t chunk_offset, uint32_t count,
> > + uint32_t block_size)
> > +{
> > +   if (count == 0)
> > +  return;
> > +
> > +   uint32_t st_idx = anv_state_table_add(&pool->table, count);
> > +   for (int i = 0; i < count; i++) {
> > +  /* update states that were added back to the state table */
> > +  struct anv_state *state_i = anv_state_table_get(&pool->table,
> > +  st_idx + i);
> > +  state_i->alloc_size = block_size;
> > +  state_i->offset = chunk_offset + block_size * i;
> > +  struct anv_pool_map pool_map = anv_block_pool_map(&pool->
> block_pool,
> > +state_i->
> offset);
> > +  state_i->map = pool_map.map + pool_map.offset;
> > +   }
> > +
> > +   uint32_t block_bucket = anv_state_pool_get_bucket(block_size);
> > +   anv_state_table_push(&pool->buckets[block_bucket].free_list,
> > +&pool->table, st_idx, count);
> > +}
> > +
> > +static uint32_t
> > +calculate_divisor(uint32_t size)
> > +{
> > +   uint32_t bucket = anv_state_pool_get_bucket(size);
> > +
> > +   while (bucket >= 0) {
> > +  uint32_t bucket_size = 
> anv_state_pool_get_bucket_size(bucket);
> > +  if (size % bucket_size == 0)
> > + return bucket_size;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +/** Returns a chunk of memory back to the state pool.
> > + *
> > + * If small_size is zero, we split chunk_size into pool->
> block_size'd
> > pieces,
> > + * and return those. If there's some remaining 'rest' space
> (chunk_size is
> > not
> > + * divisble by pool->block_size), then we find a bucket size that 
> is
> a
> > divisor
> > + * of that rest, and split the 'rest' into that size, returning it
> to the
> > pool.
> > + *
> > + * If small_size is non-zero, we use it in two different ways:
> > + ** if it is larger than pool->block_size, we split the chunk
> into
> > + *small_size'd pieces, instead of pool->block_size'd ones.
> > + ** we also use it as the desired size to split the 'rest' 
> after
> we
> >  

Re: [Mesa-dev] [PATCH 2/6] mesa: wire up InvalidateSubFramebuffer

[Rob Clark]

On Tue, Dec 11, 2018 at 6:16 PM Ian Romanick  wrote:
>
> On 12/11/18 2:50 PM, Rob Clark wrote:
> > Signed-off-by: Rob Clark 
> > ---
> >  src/mesa/main/dd.h   |  3 +++
> >  src/mesa/main/fbobject.c | 34 +-
> >  2 files changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> > index 1214eeaa474..c7112677223 100644
> > --- a/src/mesa/main/dd.h
> > +++ b/src/mesa/main/dd.h
> > @@ -786,6 +786,9 @@ struct dd_function_table {
> > GLbitfield mask, GLenum filter);
> > void (*DiscardFramebuffer)(struct gl_context *ctx, struct 
> > gl_framebuffer *fb,
> >struct gl_renderbuffer_attachment *att);
> > +   void (*DiscardSubFramebuffer)(struct gl_context *ctx, struct 
> > gl_framebuffer *fb,
> > + struct gl_renderbuffer_attachment *att, 
> > GLint x,
> > + GLint y, GLsizei width, GLsizei height);
>
> After looking at the rest of the series... I wonder if some higher layer
> should be responsible for detecting the case where the subrect size of
> the DiscardSubFramebuffer is the size of the entire attachment (which
> may be different than the renderable size of the framebuffer) and call
> DiscardFramebuffer instead.  It seems like many implementations won't do
> anything for DiscardSubFramebuffer but will do something for
> DiscardFramebuffer.

Fair point, the DiscardSubFramebuffer() part could be harder or less
useful for other drivers.. smashing in something like this would be
trival:


diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 04fd7b8b943..f5b7562c9d7 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -4706,6 +4706,17 @@ discard_sub_framebuffer(struct gl_context *ctx,
struct gl_framebuffer *fb,
if (!ctx->Driver.DiscardSubFramebuffer)
   return;

+   /* In the trivial case, turn it into discard_framebuffer(), on the
+* premise that drivers are more likely to implement the simpler
+* interface:
+*/
+   if (x == 0 && y == 0 &&
+   width >= fb->Width &&
+   height >= fb->Height) {
+  discard_framebuffer(ctx, fb, numAttachments, attachments);
+  return;
+   }
+
for (int i = 0; i < numAttachments; i++) {
   struct gl_renderbuffer_attachment *att =
 get_fb_attachment(ctx, fb, attachments[i]);


BR,
-R

>
> Maybe just leave a note in discard_sub_framebuffer so that the first
> person working on a driver that would benefit from this will implement it?
>
> >
> > /**
> >  * \name Functions for GL_ARB_sample_locations
> > diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> > index f931e8f76b1..8ef5eb747c0 100644
> > --- a/src/mesa/main/fbobject.c
> > +++ b/src/mesa/main/fbobject.c
> > @@ -4699,12 +4699,41 @@ discard_framebuffer(struct gl_context *ctx, struct 
> > gl_framebuffer *fb,
> > }
> >  }
> >
> > +static void
> > +discard_sub_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> > +GLsizei numAttachments, const GLenum *attachments,
> > +GLint x, GLint y, GLsizei width, GLsizei height)
> > +{
> > +   GLint i;
> > +
> > +   if (!ctx->Driver.DiscardSubFramebuffer)
> > +  return;
> > +
> > +   for (i = 0; i < numAttachments; i++) {
> > +  struct gl_renderbuffer_attachment *att =
> > +get_fb_attachment(ctx, fb, attachments[i]);
> > +
> > +  if (!att)
> > + continue;
> > +
> > +  ctx->Driver.DiscardSubFramebuffer(ctx, fb, att, x, y, width, height);
> > +   }
> > +}
> > +
> >  void GLAPIENTRY
> >  _mesa_InvalidateSubFramebuffer_no_error(GLenum target, GLsizei 
> > numAttachments,
> >  const GLenum *attachments, GLint x,
> >  GLint y, GLsizei width, GLsizei 
> > height)
> >  {
> > -   /* no-op */
> > +   struct gl_framebuffer *fb;
> > +   GET_CURRENT_CONTEXT(ctx);
> > +
> > +   fb = get_framebuffer_target(ctx, target);
> > +   if (!fb)
> > +  return;
> > +
> > +   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
> > +   x, y, width, height);
> >  }
> >
> >
> > @@ -4727,6 +4756,9 @@ _mesa_InvalidateSubFramebuffer(GLenum target, GLsizei 
> > numAttachments,
> > invalidate_framebuffer_storage(ctx, fb, numAttachments, attachments,
> >x, y, width, height,
> >"glInvalidateSubFramebuffer");
> > +
> > +   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
> > +   x, y, width, height);
> >  }
> >
> >
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/6] mesa: wire up InvalidateFramebuffer

[Rob Clark]

On Tue, Dec 11, 2018 at 6:06 PM Ian Romanick  wrote:
>
> On 12/11/18 2:50 PM, Rob Clark wrote:
> > And before someone actually starts implementing DiscardFramebuffer()
> > lets rework the interface to something that is actually usable.
> >
> > Signed-off-by: Rob Clark 
> > ---
> >  src/mesa/main/dd.h   |  5 +--
> >  src/mesa/main/fbobject.c | 79 ++--
> >  2 files changed, 77 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> > index f14c3e04e91..1214eeaa474 100644
> > --- a/src/mesa/main/dd.h
> > +++ b/src/mesa/main/dd.h
> > @@ -784,9 +784,8 @@ struct dd_function_table {
> > GLint srcX0, GLint srcY0, GLint srcX1, GLint 
> > srcY1,
> > GLint dstX0, GLint dstY0, GLint dstX1, GLint 
> > dstY1,
> > GLbitfield mask, GLenum filter);
> > -   void (*DiscardFramebuffer)(struct gl_context *ctx,
> > -  GLenum target, GLsizei numAttachments,
> > -  const GLenum *attachments);
> > +   void (*DiscardFramebuffer)(struct gl_context *ctx, struct 
> > gl_framebuffer *fb,
> > +  struct gl_renderbuffer_attachment *att);
> >
> > /**
> >  * \name Functions for GL_ARB_sample_locations
> > diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> > index 23e49396199..f931e8f76b1 100644
> > --- a/src/mesa/main/fbobject.c
> > +++ b/src/mesa/main/fbobject.c
> > @@ -4637,6 +4637,67 @@ invalid_enum:
> > return;
> >  }
> >
> > +static struct gl_renderbuffer_attachment *
> > +get_fb_attachment(struct gl_context *ctx, struct gl_framebuffer *fb,
> > +  const GLenum attachment)
> > +{
> > +   GLint idx;
>
> Nancy Reagan says "Just say no... to silly GL types."  I'd also be
> inclined to move this...

defn agree.. that just seemed to be the 'when in Rome' convention

But I don't mind bucking convention

>
> > +
> > +   switch (attachment) {
> > +   case GL_COLOR:
> > +   case GL_COLOR_ATTACHMENT0_EXT:
> > +   case GL_COLOR_ATTACHMENT1_EXT:
> > +   case GL_COLOR_ATTACHMENT2_EXT:
> > +   case GL_COLOR_ATTACHMENT3_EXT:
> > +   case GL_COLOR_ATTACHMENT4_EXT:
> > +   case GL_COLOR_ATTACHMENT5_EXT:
> > +   case GL_COLOR_ATTACHMENT6_EXT:
> > +   case GL_COLOR_ATTACHMENT7_EXT:
> > +   case GL_COLOR_ATTACHMENT8_EXT:
> > +   case GL_COLOR_ATTACHMENT9_EXT:
> > +   case GL_COLOR_ATTACHMENT10_EXT:
> > +   case GL_COLOR_ATTACHMENT11_EXT:
> > +   case GL_COLOR_ATTACHMENT12_EXT:
> > +   case GL_COLOR_ATTACHMENT13_EXT:
> > +   case GL_COLOR_ATTACHMENT14_EXT:
> > +   case GL_COLOR_ATTACHMENT15_EXT:
> > +  if (attachment == GL_COLOR) {
> > + idx = 0;
> > +  } else {
> > + idx = attachment - GL_COLOR_ATTACHMENT0_EXT;
> > +  }
>
> ...here and do
>
>   const unsigned idx = attachment == GL_COLOR ? 0 : attachment - 
> GL_COLOR_ATTACHMENT0_EXT;
>

mostly just trying to keep it in 80(ish) columns

> but that's just my personal preference.
>
> > +  return &fb->Attachment[BUFFER_COLOR0 + idx];
> > +   case GL_DEPTH:
> > +   case GL_DEPTH_ATTACHMENT_EXT:
> > +   case GL_DEPTH_STENCIL_ATTACHMENT:
> > +  return &fb->Attachment[BUFFER_DEPTH];
> > +   case GL_STENCIL:
> > +   case GL_STENCIL_ATTACHMENT_EXT:
> > +  return &fb->Attachment[BUFFER_STENCIL];
> > +   default:
> > +  return NULL;
> > +   }
> > +}
> > +
> > +static void
> > +discard_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> > +GLsizei numAttachments, const GLenum *attachments)
> > +{
> > +   GLint i;
> > +
> > +   if (!ctx->Driver.DiscardFramebuffer)
> > +  return;
> > +
> > +   for (i = 0; i < numAttachments; i++) {
>
> I'd definitely move 'int i' inside the for.

will do

>
> With at least s/GLint/int/ or similar, this patch is
>
> Reviewed-by: Ian Romanick 

thanks

BR,
-R

>
> > +  struct gl_renderbuffer_attachment *att =
> > +get_fb_attachment(ctx, fb, attachments[i]);
> > +
> > +  if (!att)
> > + continue;
> > +
> > +  ctx->Driver.DiscardFramebuffer(ctx, fb, att);
> > +   }
> > +}
> >
> >  void GLAPIENTRY
> >  _mesa_InvalidateSubFramebuffer_no_error(GLenum target, GLsizei 
> > numAttachments,
> > @@ -4695,14 +4756,23 @@ _mesa_InvalidateNamedFramebufferSubData(GLuint 
> > framebuffer,
> > invalidate_framebuffer_storage(ctx, fb, numAttachments, attachments,
> >x, y, width, height,
> >"glInvalidateNamedFramebufferSubData");
> > -}
> >
> > +   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
> > +   x, y, width, height);
> > +}
> >
> >  void GLAPIENTRY
> >  _mesa_InvalidateFramebuffer_no_error(GLenum target, GLsizei numAttachments,
> >   const GLenum *attachments)
> >  {
> > -   /* no-op */
> > +   struct gl_framebuffer *fb;
> > +   GET_CURRENT_CONTEXT(ctx);

Re: [Mesa-dev] [PATCH 2/6] mesa: wire up InvalidateSubFramebuffer

[Ian Romanick]

On 12/11/18 2:50 PM, Rob Clark wrote:
> Signed-off-by: Rob Clark 
> ---
>  src/mesa/main/dd.h   |  3 +++
>  src/mesa/main/fbobject.c | 34 +-
>  2 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index 1214eeaa474..c7112677223 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -786,6 +786,9 @@ struct dd_function_table {
> GLbitfield mask, GLenum filter);
> void (*DiscardFramebuffer)(struct gl_context *ctx, struct gl_framebuffer 
> *fb,
>struct gl_renderbuffer_attachment *att);
> +   void (*DiscardSubFramebuffer)(struct gl_context *ctx, struct 
> gl_framebuffer *fb,
> + struct gl_renderbuffer_attachment *att, 
> GLint x,
> + GLint y, GLsizei width, GLsizei height);

After looking at the rest of the series... I wonder if some higher layer
should be responsible for detecting the case where the subrect size of
the DiscardSubFramebuffer is the size of the entire attachment (which
may be different than the renderable size of the framebuffer) and call
DiscardFramebuffer instead.  It seems like many implementations won't do
anything for DiscardSubFramebuffer but will do something for
DiscardFramebuffer.

Maybe just leave a note in discard_sub_framebuffer so that the first
person working on a driver that would benefit from this will implement it?

>  
> /**
>  * \name Functions for GL_ARB_sample_locations
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index f931e8f76b1..8ef5eb747c0 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -4699,12 +4699,41 @@ discard_framebuffer(struct gl_context *ctx, struct 
> gl_framebuffer *fb,
> }
>  }
>  
> +static void
> +discard_sub_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> +GLsizei numAttachments, const GLenum *attachments,
> +GLint x, GLint y, GLsizei width, GLsizei height)
> +{
> +   GLint i;
> +
> +   if (!ctx->Driver.DiscardSubFramebuffer)
> +  return;
> +
> +   for (i = 0; i < numAttachments; i++) {
> +  struct gl_renderbuffer_attachment *att =
> +get_fb_attachment(ctx, fb, attachments[i]);
> +
> +  if (!att)
> + continue;
> +
> +  ctx->Driver.DiscardSubFramebuffer(ctx, fb, att, x, y, width, height);
> +   }
> +}
> +
>  void GLAPIENTRY
>  _mesa_InvalidateSubFramebuffer_no_error(GLenum target, GLsizei 
> numAttachments,
>  const GLenum *attachments, GLint x,
>  GLint y, GLsizei width, GLsizei 
> height)
>  {
> -   /* no-op */
> +   struct gl_framebuffer *fb;
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   fb = get_framebuffer_target(ctx, target);
> +   if (!fb)
> +  return;
> +
> +   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
> +   x, y, width, height);
>  }
>  
>  
> @@ -4727,6 +4756,9 @@ _mesa_InvalidateSubFramebuffer(GLenum target, GLsizei 
> numAttachments,
> invalidate_framebuffer_storage(ctx, fb, numAttachments, attachments,
>x, y, width, height,
>"glInvalidateSubFramebuffer");
> +
> +   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
> +   x, y, width, height);
>  }
>  
>  
> 

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

Re: [Mesa-dev] [PATCH 4/6] mesa/st: wire up DiscardSubFramebuffer

[Ilia Mirkin]

On Tue, Dec 11, 2018 at 5:50 PM Rob Clark  wrote:
>
> Signed-off-by: Rob Clark 
> ---
>  src/gallium/include/pipe/p_context.h | 11 +++
>  src/mesa/state_tracker/st_cb_fbo.c   | 26 ++
>  2 files changed, 37 insertions(+)
>
> diff --git a/src/gallium/include/pipe/p_context.h 
> b/src/gallium/include/pipe/p_context.h
> index d4e9179b78a..eb52c7e9a4e 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -811,6 +811,17 @@ struct pipe_context {
> void (*invalidate_surface)(struct pipe_context *ctx,
>struct pipe_surface *surf);
>
> +   /**
> +* Invalidate a portion of a surface.  This is used to
> +*
> +* (1) implement glInvalidateSubFramebuffer() and friends
> +* (2) as a hint before a scissored clear (which is turned into draw_vbo()
> +* that the cleared rect can be discarded
> +*/
> +   void (*invalidate_sub_surface)(struct pipe_context *ctx,
> +  struct pipe_surface *surf,
> +  const struct pipe_scissor_state *rect);

I think pipe_box is a bit more natural here than scissor state? box is
used for transfers, etc. This could also enable you to to specify a
sub-3d area. And perhaps by passing a level, you could go back to this
being a resource (and just extend invalidate_resource)?

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

Re: [Mesa-dev] [PATCH 4/6] mesa/st: wire up DiscardSubFramebuffer

[Roland Scheidegger]

Am 11.12.18 um 23:50 schrieb Rob Clark:
> Signed-off-by: Rob Clark 
> ---
>  src/gallium/include/pipe/p_context.h | 11 +++
>  src/mesa/state_tracker/st_cb_fbo.c   | 26 ++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/src/gallium/include/pipe/p_context.h 
> b/src/gallium/include/pipe/p_context.h
> index d4e9179b78a..eb52c7e9a4e 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -811,6 +811,17 @@ struct pipe_context {
> void (*invalidate_surface)(struct pipe_context *ctx,
>struct pipe_surface *surf);
>  
> +   /**
> +* Invalidate a portion of a surface.  This is used to
> +*
> +* (1) implement glInvalidateSubFramebuffer() and friends
> +* (2) as a hint before a scissored clear (which is turned into draw_vbo()
> +* that the cleared rect can be discarded
> +*/
> +   void (*invalidate_sub_surface)(struct pipe_context *ctx,
> +  struct pipe_surface *surf,
> +  const struct pipe_scissor_state *rect);
> +

The same applies as to the previous change. Additionally, I think we
really don't need 3 functions essentially doing the same thing. Ok I
could see that maybe there's value passing in the surface directly
(rather than mip levels, layers), but surely
invalidate_surface/invalidate_sub_surface looks like overkill to me.
Maybe pass in a NULL pointer for rect if you want to clear everything?

Roland


> /**
>  * Return information about unexpected device resets.
>  */
> diff --git a/src/mesa/state_tracker/st_cb_fbo.c 
> b/src/mesa/state_tracker/st_cb_fbo.c
> index 3ece1d4a9de..50c27ea51d9 100644
> --- a/src/mesa/state_tracker/st_cb_fbo.c
> +++ b/src/mesa/state_tracker/st_cb_fbo.c
> @@ -774,6 +774,31 @@ st_discard_framebuffer(struct gl_context *ctx, struct 
> gl_framebuffer *fb,
>st->pipe->invalidate_surface(st->pipe, psurf);
>  }
>  
> +static void
> +st_discard_sub_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> +   struct gl_renderbuffer_attachment *att, GLint x,
> +   GLint y, GLsizei width, GLsizei height)
> +{
> +   struct st_context *st = st_context(ctx);
> +   struct pipe_surface *psurf;
> +
> +   if (!att->Renderbuffer)
> +  return;
> +
> +   psurf = st_renderbuffer(att->Renderbuffer)->surface;
> +
> +   if (st->pipe->invalidate_sub_surface) {
> +  struct pipe_scissor_state rect;
> +
> +  rect.minx = x;
> +  rect.maxx = x + width - 1;
> +  rect.miny = y;
> +  rect.maxy = y + height - 1;
> +
> +  st->pipe->invalidate_sub_surface(st->pipe, psurf, &rect);
> +   }
> +}
> +
>  /**
>   * Called via glDrawBuffer.  We only provide this driver function so that we
>   * can check if we need to allocate a new renderbuffer.  Specifically, we
> @@ -952,6 +977,7 @@ st_init_fbo_functions(struct dd_function_table *functions)
> functions->FinishRenderTexture = st_finish_render_texture;
> functions->ValidateFramebuffer = st_validate_framebuffer;
> functions->DiscardFramebuffer = st_discard_framebuffer;
> +   functions->DiscardSubFramebuffer = st_discard_sub_framebuffer;
>  
> functions->DrawBufferAllocate = st_DrawBufferAllocate;
> functions->ReadBuffer = st_ReadBuffer;
> 

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

Re: [Mesa-dev] [PATCH 2/6] mesa: wire up InvalidateSubFramebuffer

[Ian Romanick]

On 12/11/18 2:50 PM, Rob Clark wrote:
> Signed-off-by: Rob Clark 
> ---
>  src/mesa/main/dd.h   |  3 +++
>  src/mesa/main/fbobject.c | 34 +-
>  2 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index 1214eeaa474..c7112677223 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -786,6 +786,9 @@ struct dd_function_table {
> GLbitfield mask, GLenum filter);
> void (*DiscardFramebuffer)(struct gl_context *ctx, struct gl_framebuffer 
> *fb,
>struct gl_renderbuffer_attachment *att);
> +   void (*DiscardSubFramebuffer)(struct gl_context *ctx, struct 
> gl_framebuffer *fb,
> + struct gl_renderbuffer_attachment *att, 
> GLint x,
> + GLint y, GLsizei width, GLsizei height);
>  
> /**
>  * \name Functions for GL_ARB_sample_locations
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index f931e8f76b1..8ef5eb747c0 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -4699,12 +4699,41 @@ discard_framebuffer(struct gl_context *ctx, struct 
> gl_framebuffer *fb,
> }
>  }
>  
> +static void
> +discard_sub_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> +GLsizei numAttachments, const GLenum *attachments,
> +GLint x, GLint y, GLsizei width, GLsizei height)
> +{
> +   GLint i;
> +
> +   if (!ctx->Driver.DiscardSubFramebuffer)
> +  return;
> +
> +   for (i = 0; i < numAttachments; i++) {

Same comment here about move the declaration and changing the type.
With that,

Reviewed-by: Ian Romanick 

> +  struct gl_renderbuffer_attachment *att =
> +get_fb_attachment(ctx, fb, attachments[i]);
> +
> +  if (!att)
> + continue;
> +
> +  ctx->Driver.DiscardSubFramebuffer(ctx, fb, att, x, y, width, height);
> +   }
> +}
> +
>  void GLAPIENTRY
>  _mesa_InvalidateSubFramebuffer_no_error(GLenum target, GLsizei 
> numAttachments,
>  const GLenum *attachments, GLint x,
>  GLint y, GLsizei width, GLsizei 
> height)
>  {
> -   /* no-op */
> +   struct gl_framebuffer *fb;
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   fb = get_framebuffer_target(ctx, target);
> +   if (!fb)
> +  return;
> +
> +   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
> +   x, y, width, height);
>  }
>  
>  
> @@ -4727,6 +4756,9 @@ _mesa_InvalidateSubFramebuffer(GLenum target, GLsizei 
> numAttachments,
> invalidate_framebuffer_storage(ctx, fb, numAttachments, attachments,
>x, y, width, height,
>"glInvalidateSubFramebuffer");
> +
> +   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
> +   x, y, width, height);
>  }
>  
>  
> 

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

Re: [Mesa-dev] [PATCH 1/6] mesa: wire up InvalidateFramebuffer

[Ian Romanick]

On 12/11/18 2:50 PM, Rob Clark wrote:
> And before someone actually starts implementing DiscardFramebuffer()
> lets rework the interface to something that is actually usable.
> 
> Signed-off-by: Rob Clark 
> ---
>  src/mesa/main/dd.h   |  5 +--
>  src/mesa/main/fbobject.c | 79 ++--
>  2 files changed, 77 insertions(+), 7 deletions(-)
> 
> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
> index f14c3e04e91..1214eeaa474 100644
> --- a/src/mesa/main/dd.h
> +++ b/src/mesa/main/dd.h
> @@ -784,9 +784,8 @@ struct dd_function_table {
> GLint srcX0, GLint srcY0, GLint srcX1, GLint 
> srcY1,
> GLint dstX0, GLint dstY0, GLint dstX1, GLint 
> dstY1,
> GLbitfield mask, GLenum filter);
> -   void (*DiscardFramebuffer)(struct gl_context *ctx,
> -  GLenum target, GLsizei numAttachments,
> -  const GLenum *attachments);
> +   void (*DiscardFramebuffer)(struct gl_context *ctx, struct gl_framebuffer 
> *fb,
> +  struct gl_renderbuffer_attachment *att);
>  
> /**
>  * \name Functions for GL_ARB_sample_locations
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index 23e49396199..f931e8f76b1 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -4637,6 +4637,67 @@ invalid_enum:
> return;
>  }
>  
> +static struct gl_renderbuffer_attachment *
> +get_fb_attachment(struct gl_context *ctx, struct gl_framebuffer *fb,
> +  const GLenum attachment)
> +{
> +   GLint idx;

Nancy Reagan says "Just say no... to silly GL types."  I'd also be
inclined to move this...

> +
> +   switch (attachment) {
> +   case GL_COLOR:
> +   case GL_COLOR_ATTACHMENT0_EXT:
> +   case GL_COLOR_ATTACHMENT1_EXT:
> +   case GL_COLOR_ATTACHMENT2_EXT:
> +   case GL_COLOR_ATTACHMENT3_EXT:
> +   case GL_COLOR_ATTACHMENT4_EXT:
> +   case GL_COLOR_ATTACHMENT5_EXT:
> +   case GL_COLOR_ATTACHMENT6_EXT:
> +   case GL_COLOR_ATTACHMENT7_EXT:
> +   case GL_COLOR_ATTACHMENT8_EXT:
> +   case GL_COLOR_ATTACHMENT9_EXT:
> +   case GL_COLOR_ATTACHMENT10_EXT:
> +   case GL_COLOR_ATTACHMENT11_EXT:
> +   case GL_COLOR_ATTACHMENT12_EXT:
> +   case GL_COLOR_ATTACHMENT13_EXT:
> +   case GL_COLOR_ATTACHMENT14_EXT:
> +   case GL_COLOR_ATTACHMENT15_EXT:
> +  if (attachment == GL_COLOR) {
> + idx = 0;
> +  } else {
> + idx = attachment - GL_COLOR_ATTACHMENT0_EXT;
> +  }

...here and do

  const unsigned idx = attachment == GL_COLOR ? 0 : attachment - 
GL_COLOR_ATTACHMENT0_EXT;

but that's just my personal preference.

> +  return &fb->Attachment[BUFFER_COLOR0 + idx];
> +   case GL_DEPTH:
> +   case GL_DEPTH_ATTACHMENT_EXT:
> +   case GL_DEPTH_STENCIL_ATTACHMENT:
> +  return &fb->Attachment[BUFFER_DEPTH];
> +   case GL_STENCIL:
> +   case GL_STENCIL_ATTACHMENT_EXT:
> +  return &fb->Attachment[BUFFER_STENCIL];
> +   default:
> +  return NULL;
> +   }
> +}
> +
> +static void
> +discard_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> +GLsizei numAttachments, const GLenum *attachments)
> +{
> +   GLint i;
> +
> +   if (!ctx->Driver.DiscardFramebuffer)
> +  return;
> +
> +   for (i = 0; i < numAttachments; i++) {

I'd definitely move 'int i' inside the for.

With at least s/GLint/int/ or similar, this patch is

Reviewed-by: Ian Romanick 

> +  struct gl_renderbuffer_attachment *att =
> +get_fb_attachment(ctx, fb, attachments[i]);
> +
> +  if (!att)
> + continue;
> +
> +  ctx->Driver.DiscardFramebuffer(ctx, fb, att);
> +   }
> +}
>  
>  void GLAPIENTRY
>  _mesa_InvalidateSubFramebuffer_no_error(GLenum target, GLsizei 
> numAttachments,
> @@ -4695,14 +4756,23 @@ _mesa_InvalidateNamedFramebufferSubData(GLuint 
> framebuffer,
> invalidate_framebuffer_storage(ctx, fb, numAttachments, attachments,
>x, y, width, height,
>"glInvalidateNamedFramebufferSubData");
> -}
>  
> +   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
> +   x, y, width, height);
> +}
>  
>  void GLAPIENTRY
>  _mesa_InvalidateFramebuffer_no_error(GLenum target, GLsizei numAttachments,
>   const GLenum *attachments)
>  {
> -   /* no-op */
> +   struct gl_framebuffer *fb;
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   fb = get_framebuffer_target(ctx, target);
> +   if (!fb)
> +  return;
> +
> +   discard_framebuffer(ctx, fb, numAttachments, attachments);
>  }
>  
>  
> @@ -4738,6 +4808,8 @@ _mesa_InvalidateFramebuffer(GLenum target, GLsizei 
> numAttachments,
>ctx->Const.MaxViewportWidth,
>ctx->Const.MaxViewportHeight,
>"glInvalidateFramebuffer");
> +
> +   discard_framebuffer(ctx, f

Re: [Mesa-dev] [PATCH] gallium/aux: add is_unorm() helper

[Ilia Mirkin]

I was talking about Z24X8, which Roland confirmed would return true
here. I guess that's fine, but worth pointing out perhaps.
On Tue, Dec 11, 2018 at 5:48 PM Rob Clark  wrote:
>
> The z24s8 permutations are is_mixed, the x24s8 permutations are not
> and will show up as is_unorm() (would have been an issue for
> is_snorm() too except they are unsigned)
>
> The 10_10_10_2 formats are not mixed.
>
> In either case, I'm not sure is_norm() returning true is *incorrect*..
>
> BR,
> -R
>
> On Tue, Dec 11, 2018 at 4:21 PM Ilia Mirkin  wrote:
> >
> > So ... Z24 will end up with is_unorm() == true? [Just guessing --
> > assume it doesn't hae is_mixed == true.] Also, does RGB10A2 have mixed
> > set? If so, then it won't report unorm. Not 100% sure if is_mixed is
> > only for norm + int mixing.
> > On Tue, Dec 11, 2018 at 4:05 PM Rob Clark  wrote:
> > >
> > > We already had one for is_snorm() but not unorm.
> > >
> > > Signed-off-by: Rob Clark 
> > > ---
> > >  src/gallium/auxiliary/util/u_format.c | 21 +
> > >  src/gallium/auxiliary/util/u_format.h |  3 +++
> > >  2 files changed, 24 insertions(+)
> > >
> > > diff --git a/src/gallium/auxiliary/util/u_format.c 
> > > b/src/gallium/auxiliary/util/u_format.c
> > > index e43a619313e..231e89017b4 100644
> > > --- a/src/gallium/auxiliary/util/u_format.c
> > > +++ b/src/gallium/auxiliary/util/u_format.c
> > > @@ -169,6 +169,27 @@ util_format_is_snorm(enum pipe_format format)
> > >desc->channel[i].normalized;
> > >  }
> > >
> > > +/**
> > > + * Returns true if all non-void channels are normalized unsigned.
> > > + */
> > > +boolean
> > > +util_format_is_unorm(enum pipe_format format)
> > > +{
> > > +   const struct util_format_description *desc = 
> > > util_format_description(format);
> > > +   int i;
> > > +
> > > +   if (desc->is_mixed)
> > > +  return FALSE;
> > > +
> > > +   i = util_format_get_first_non_void_channel(format);
> > > +   if (i == -1)
> > > +  return FALSE;
> > > +
> > > +   return desc->channel[i].type == UTIL_FORMAT_TYPE_UNSIGNED &&
> > > +  !desc->channel[i].pure_integer &&
> > > +  desc->channel[i].normalized;
> > > +}
> > > +
> > >  boolean
> > >  util_format_is_snorm8(enum pipe_format format)
> > >  {
> > > diff --git a/src/gallium/auxiliary/util/u_format.h 
> > > b/src/gallium/auxiliary/util/u_format.h
> > > index 5bcfc1f1154..8dcc438a4a1 100644
> > > --- a/src/gallium/auxiliary/util/u_format.h
> > > +++ b/src/gallium/auxiliary/util/u_format.h
> > > @@ -726,6 +726,9 @@ util_format_is_pure_uint(enum pipe_format format);
> > >  boolean
> > >  util_format_is_snorm(enum pipe_format format);
> > >
> > > +boolean
> > > +util_format_is_unorm(enum pipe_format format);
> > > +
> > >  boolean
> > >  util_format_is_snorm8(enum pipe_format format);
> > >
> > > --
> > > 2.19.2
> > >
> > > ___
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/6] mesa/st: wire up DiscardFramebuffer

[Roland Scheidegger]

Am 11.12.18 um 23:50 schrieb Rob Clark:
> pipe_context::invalidate_resource() is so *almost* what we want, but
> with FBOs the fb can be a single layer/level of a pipe_resource, which
> makes ::invalidate_resource() not expressive enough.
> 
> Signed-off-by: Rob Clark 
> ---
>  src/gallium/include/pipe/p_context.h |  8 
>  src/mesa/state_tracker/st_cb_fbo.c   | 16 
>  2 files changed, 24 insertions(+)
> 
> diff --git a/src/gallium/include/pipe/p_context.h 
> b/src/gallium/include/pipe/p_context.h
> index e07b76d4f03..d4e9179b78a 100644
> --- a/src/gallium/include/pipe/p_context.h
> +++ b/src/gallium/include/pipe/p_context.h
> @@ -803,6 +803,14 @@ struct pipe_context {
> void (*invalidate_resource)(struct pipe_context *ctx,
> struct pipe_resource *resource);
>  
> +   /**
> +* Like ->invalidate_surface,
Like ->invalidate->resource?

gallium interface changes must use gallium: in the shortlog. Should
probably split up the patch into 2.
It's also missing the gallium/docs changes.

Roland



 but can invalidate a specific layer and level
> +* of a resource.  If the backing surf->texture has just a single layer 
> and
> +* level (like window system buffers) it is equiv to ->invalidate_resource
> +*/
> +   void (*invalidate_surface)(struct pipe_context *ctx,
> +  struct pipe_surface *surf);
> +
> /**
>  * Return information about unexpected device resets.
>  */
> diff --git a/src/mesa/state_tracker/st_cb_fbo.c 
> b/src/mesa/state_tracker/st_cb_fbo.c
> index 8901a8680ef..3ece1d4a9de 100644
> --- a/src/mesa/state_tracker/st_cb_fbo.c
> +++ b/src/mesa/state_tracker/st_cb_fbo.c
> @@ -758,6 +758,21 @@ st_validate_framebuffer(struct gl_context *ctx, struct 
> gl_framebuffer *fb)
> }
>  }
>  
> +static void
> +st_discard_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
> +   struct gl_renderbuffer_attachment *att)
> +{
> +   struct st_context *st = st_context(ctx);
> +   struct pipe_surface *psurf;
> +
> +   if (!att->Renderbuffer)
> +  return;
> +
> +   psurf = st_renderbuffer(att->Renderbuffer)->surface;
> +
> +   if (st->pipe->invalidate_surface)
> +  st->pipe->invalidate_surface(st->pipe, psurf);
> +}
>  
>  /**
>   * Called via glDrawBuffer.  We only provide this driver function so that we
> @@ -936,6 +951,7 @@ st_init_fbo_functions(struct dd_function_table *functions)
> functions->RenderTexture = st_render_texture;
> functions->FinishRenderTexture = st_finish_render_texture;
> functions->ValidateFramebuffer = st_validate_framebuffer;
> +   functions->DiscardFramebuffer = st_discard_framebuffer;
>  
> functions->DrawBufferAllocate = st_DrawBufferAllocate;
> functions->ReadBuffer = st_ReadBuffer;
> 

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

Re: [Mesa-dev] [PATCH] gallium/aux: add is_unorm() helper

[Roland Scheidegger]

Am 11.12.18 um 22:21 schrieb Ilia Mirkin:
> So ... Z24 will end up with is_unorm() == true? [Just guessing --
> assume it doesn't hae is_mixed == true.]
Z24X8 would return true, but Z24S8 would return false. (If you didn't
want that I guess you could check for colorspace.)

> Also, does RGB10A2 have mixed
> set? If so, then it won't report unorm. Not 100% sure if is_mixed is
> only for norm + int mixing.
Not sure which RGB10A2 variant you're refering to but there's only one
weird one (for bump maps) which would have mixed set. All others I think
always have components with the same type, so the one with unorm should
return true...

FWIW I don't think the combination of normalized & pure_integer is
really possible, so an explicit check for !pure_integer seems redundant.
But shouldn't hurt I suppose (and of course is_snorm does it the same).

Roland




> On Tue, Dec 11, 2018 at 4:05 PM Rob Clark  wrote:
>>
>> We already had one for is_snorm() but not unorm.
>>
>> Signed-off-by: Rob Clark 
>> ---
>>  src/gallium/auxiliary/util/u_format.c | 21 +
>>  src/gallium/auxiliary/util/u_format.h |  3 +++
>>  2 files changed, 24 insertions(+)
>>
>> diff --git a/src/gallium/auxiliary/util/u_format.c 
>> b/src/gallium/auxiliary/util/u_format.c
>> index e43a619313e..231e89017b4 100644
>> --- a/src/gallium/auxiliary/util/u_format.c
>> +++ b/src/gallium/auxiliary/util/u_format.c
>> @@ -169,6 +169,27 @@ util_format_is_snorm(enum pipe_format format)
>>desc->channel[i].normalized;
>>  }
>>
>> +/**
>> + * Returns true if all non-void channels are normalized unsigned.
>> + */
>> +boolean
>> +util_format_is_unorm(enum pipe_format format)
>> +{
>> +   const struct util_format_description *desc = 
>> util_format_description(format);
>> +   int i;
>> +
>> +   if (desc->is_mixed)
>> +  return FALSE;
>> +
>> +   i = util_format_get_first_non_void_channel(format);
>> +   if (i == -1)
>> +  return FALSE;
>> +
>> +   return desc->channel[i].type == UTIL_FORMAT_TYPE_UNSIGNED &&
>> +  !desc->channel[i].pure_integer &&
>> +  desc->channel[i].normalized;
>> +}
>> +
>>  boolean
>>  util_format_is_snorm8(enum pipe_format format)
>>  {
>> diff --git a/src/gallium/auxiliary/util/u_format.h 
>> b/src/gallium/auxiliary/util/u_format.h
>> index 5bcfc1f1154..8dcc438a4a1 100644
>> --- a/src/gallium/auxiliary/util/u_format.h
>> +++ b/src/gallium/auxiliary/util/u_format.h
>> @@ -726,6 +726,9 @@ util_format_is_pure_uint(enum pipe_format format);
>>  boolean
>>  util_format_is_snorm(enum pipe_format format);
>>
>> +boolean
>> +util_format_is_unorm(enum pipe_format format);
>> +
>>  boolean
>>  util_format_is_snorm8(enum pipe_format format);
>>
>> --
>> 2.19.2
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev&data=02%7C01%7Csroland%40vmware.com%7C4aca1244e0024a40746508d65faea953%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636801601117990736&sdata=5SfD%2BKxjb1guv08vn0GOyxCODeiehCLf0m%2BLAhEE%2BW8%3D&reserved=0
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev&data=02%7C01%7Csroland%40vmware.com%7C4aca1244e0024a40746508d65faea953%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636801601117990736&sdata=5SfD%2BKxjb1guv08vn0GOyxCODeiehCLf0m%2BLAhEE%2BW8%3D&reserved=0
> 

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

[Mesa-dev] [PATCH 5/6] mesa/st: use invalidate_sub_surface() for scissored clears

[Rob Clark]

Now that we have pipe_context::invalidate_sub_surface(), we can also
use this to hint to driver about scissored clears (which use draw_vbo()).
This is useful in particular for tilers because the driver can avoid
bringing (some) tiles back into the tile buffer from system memory.

Signed-off-by: Rob Clark 
---
 src/mesa/state_tracker/st_cb_clear.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/src/mesa/state_tracker/st_cb_clear.c 
b/src/mesa/state_tracker/st_cb_clear.c
index 88fc12789e3..e3af3532fc4 100644
--- a/src/mesa/state_tracker/st_cb_clear.c
+++ b/src/mesa/state_tracker/st_cb_clear.c
@@ -357,6 +357,18 @@ is_stencil_masked(struct gl_context *ctx, struct 
gl_renderbuffer *rb)
return (ctx->Stencil.WriteMask[0] & stencilMax) != stencilMax;
 }
 
+static void
+handle_scissor_invalidate(struct gl_context *ctx, struct gl_renderbuffer *rb)
+{
+   struct st_context *st = st_context(ctx);
+
+   if (st->pipe->invalidate_sub_surface &&
+   is_scissor_enabled(ctx, rb)) {
+  struct st_renderbuffer *strb = st_renderbuffer(rb);
+  st->pipe->invalidate_sub_surface(st->pipe, strb->surface,
+   &st->state.scissor[0]);
+   }
+}
 
 /**
  * Called via ctx->Driver.Clear()
@@ -415,6 +427,7 @@ st_Clear(struct gl_context *ctx, GLbitfield mask)
   struct st_renderbuffer *strb = st_renderbuffer(depthRb);
 
   if (strb->surface && ctx->Depth.Mask) {
+ handle_scissor_invalidate(ctx, depthRb);
  if (is_scissor_enabled(ctx, depthRb) ||
  is_window_rectangle_enabled(ctx))
 quad_buffers |= PIPE_CLEAR_DEPTH;
@@ -426,6 +439,7 @@ st_Clear(struct gl_context *ctx, GLbitfield mask)
   struct st_renderbuffer *strb = st_renderbuffer(stencilRb);
 
   if (strb->surface && !is_stencil_disabled(ctx, stencilRb)) {
+ handle_scissor_invalidate(ctx, stencilRb);
  if (is_scissor_enabled(ctx, stencilRb) ||
  is_window_rectangle_enabled(ctx) ||
  is_stencil_masked(ctx, stencilRb))
-- 
2.19.2

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

[Mesa-dev] [PATCH 1/6] mesa: wire up InvalidateFramebuffer

[Rob Clark]

And before someone actually starts implementing DiscardFramebuffer()
lets rework the interface to something that is actually usable.

Signed-off-by: Rob Clark 
---
 src/mesa/main/dd.h   |  5 +--
 src/mesa/main/fbobject.c | 79 ++--
 2 files changed, 77 insertions(+), 7 deletions(-)

diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
index f14c3e04e91..1214eeaa474 100644
--- a/src/mesa/main/dd.h
+++ b/src/mesa/main/dd.h
@@ -784,9 +784,8 @@ struct dd_function_table {
GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1,
GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1,
GLbitfield mask, GLenum filter);
-   void (*DiscardFramebuffer)(struct gl_context *ctx,
-  GLenum target, GLsizei numAttachments,
-  const GLenum *attachments);
+   void (*DiscardFramebuffer)(struct gl_context *ctx, struct gl_framebuffer 
*fb,
+  struct gl_renderbuffer_attachment *att);
 
/**
 * \name Functions for GL_ARB_sample_locations
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 23e49396199..f931e8f76b1 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -4637,6 +4637,67 @@ invalid_enum:
return;
 }
 
+static struct gl_renderbuffer_attachment *
+get_fb_attachment(struct gl_context *ctx, struct gl_framebuffer *fb,
+  const GLenum attachment)
+{
+   GLint idx;
+
+   switch (attachment) {
+   case GL_COLOR:
+   case GL_COLOR_ATTACHMENT0_EXT:
+   case GL_COLOR_ATTACHMENT1_EXT:
+   case GL_COLOR_ATTACHMENT2_EXT:
+   case GL_COLOR_ATTACHMENT3_EXT:
+   case GL_COLOR_ATTACHMENT4_EXT:
+   case GL_COLOR_ATTACHMENT5_EXT:
+   case GL_COLOR_ATTACHMENT6_EXT:
+   case GL_COLOR_ATTACHMENT7_EXT:
+   case GL_COLOR_ATTACHMENT8_EXT:
+   case GL_COLOR_ATTACHMENT9_EXT:
+   case GL_COLOR_ATTACHMENT10_EXT:
+   case GL_COLOR_ATTACHMENT11_EXT:
+   case GL_COLOR_ATTACHMENT12_EXT:
+   case GL_COLOR_ATTACHMENT13_EXT:
+   case GL_COLOR_ATTACHMENT14_EXT:
+   case GL_COLOR_ATTACHMENT15_EXT:
+  if (attachment == GL_COLOR) {
+ idx = 0;
+  } else {
+ idx = attachment - GL_COLOR_ATTACHMENT0_EXT;
+  }
+  return &fb->Attachment[BUFFER_COLOR0 + idx];
+   case GL_DEPTH:
+   case GL_DEPTH_ATTACHMENT_EXT:
+   case GL_DEPTH_STENCIL_ATTACHMENT:
+  return &fb->Attachment[BUFFER_DEPTH];
+   case GL_STENCIL:
+   case GL_STENCIL_ATTACHMENT_EXT:
+  return &fb->Attachment[BUFFER_STENCIL];
+   default:
+  return NULL;
+   }
+}
+
+static void
+discard_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
+GLsizei numAttachments, const GLenum *attachments)
+{
+   GLint i;
+
+   if (!ctx->Driver.DiscardFramebuffer)
+  return;
+
+   for (i = 0; i < numAttachments; i++) {
+  struct gl_renderbuffer_attachment *att =
+get_fb_attachment(ctx, fb, attachments[i]);
+
+  if (!att)
+ continue;
+
+  ctx->Driver.DiscardFramebuffer(ctx, fb, att);
+   }
+}
 
 void GLAPIENTRY
 _mesa_InvalidateSubFramebuffer_no_error(GLenum target, GLsizei numAttachments,
@@ -4695,14 +4756,23 @@ _mesa_InvalidateNamedFramebufferSubData(GLuint 
framebuffer,
invalidate_framebuffer_storage(ctx, fb, numAttachments, attachments,
   x, y, width, height,
   "glInvalidateNamedFramebufferSubData");
-}
 
+   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
+   x, y, width, height);
+}
 
 void GLAPIENTRY
 _mesa_InvalidateFramebuffer_no_error(GLenum target, GLsizei numAttachments,
  const GLenum *attachments)
 {
-   /* no-op */
+   struct gl_framebuffer *fb;
+   GET_CURRENT_CONTEXT(ctx);
+
+   fb = get_framebuffer_target(ctx, target);
+   if (!fb)
+  return;
+
+   discard_framebuffer(ctx, fb, numAttachments, attachments);
 }
 
 
@@ -4738,6 +4808,8 @@ _mesa_InvalidateFramebuffer(GLenum target, GLsizei 
numAttachments,
   ctx->Const.MaxViewportWidth,
   ctx->Const.MaxViewportHeight,
   "glInvalidateFramebuffer");
+
+   discard_framebuffer(ctx, fb, numAttachments, attachments);
 }
 
 
@@ -4824,8 +4896,7 @@ _mesa_DiscardFramebufferEXT(GLenum target, GLsizei 
numAttachments,
   }
}
 
-   if (ctx->Driver.DiscardFramebuffer)
-  ctx->Driver.DiscardFramebuffer(ctx, target, numAttachments, attachments);
+   discard_framebuffer(ctx, fb, numAttachments, attachments);
 
return;
 
-- 
2.19.2

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

[Mesa-dev] [PATCH 3/6] mesa/st: wire up DiscardFramebuffer

[Rob Clark]

pipe_context::invalidate_resource() is so *almost* what we want, but
with FBOs the fb can be a single layer/level of a pipe_resource, which
makes ::invalidate_resource() not expressive enough.

Signed-off-by: Rob Clark 
---
 src/gallium/include/pipe/p_context.h |  8 
 src/mesa/state_tracker/st_cb_fbo.c   | 16 
 2 files changed, 24 insertions(+)

diff --git a/src/gallium/include/pipe/p_context.h 
b/src/gallium/include/pipe/p_context.h
index e07b76d4f03..d4e9179b78a 100644
--- a/src/gallium/include/pipe/p_context.h
+++ b/src/gallium/include/pipe/p_context.h
@@ -803,6 +803,14 @@ struct pipe_context {
void (*invalidate_resource)(struct pipe_context *ctx,
struct pipe_resource *resource);
 
+   /**
+* Like ->invalidate_surface, but can invalidate a specific layer and level
+* of a resource.  If the backing surf->texture has just a single layer and
+* level (like window system buffers) it is equiv to ->invalidate_resource
+*/
+   void (*invalidate_surface)(struct pipe_context *ctx,
+  struct pipe_surface *surf);
+
/**
 * Return information about unexpected device resets.
 */
diff --git a/src/mesa/state_tracker/st_cb_fbo.c 
b/src/mesa/state_tracker/st_cb_fbo.c
index 8901a8680ef..3ece1d4a9de 100644
--- a/src/mesa/state_tracker/st_cb_fbo.c
+++ b/src/mesa/state_tracker/st_cb_fbo.c
@@ -758,6 +758,21 @@ st_validate_framebuffer(struct gl_context *ctx, struct 
gl_framebuffer *fb)
}
 }
 
+static void
+st_discard_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
+   struct gl_renderbuffer_attachment *att)
+{
+   struct st_context *st = st_context(ctx);
+   struct pipe_surface *psurf;
+
+   if (!att->Renderbuffer)
+  return;
+
+   psurf = st_renderbuffer(att->Renderbuffer)->surface;
+
+   if (st->pipe->invalidate_surface)
+  st->pipe->invalidate_surface(st->pipe, psurf);
+}
 
 /**
  * Called via glDrawBuffer.  We only provide this driver function so that we
@@ -936,6 +951,7 @@ st_init_fbo_functions(struct dd_function_table *functions)
functions->RenderTexture = st_render_texture;
functions->FinishRenderTexture = st_finish_render_texture;
functions->ValidateFramebuffer = st_validate_framebuffer;
+   functions->DiscardFramebuffer = st_discard_framebuffer;
 
functions->DrawBufferAllocate = st_DrawBufferAllocate;
functions->ReadBuffer = st_ReadBuffer;
-- 
2.19.2

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

[Mesa-dev] [PATCH 6/6] freedreno: support invalidate_{sub_}surface

[Rob Clark]

Signed-off-by: Rob Clark 
---
 src/gallium/drivers/freedreno/a6xx/fd6_gmem.c |  3 +-
 .../drivers/freedreno/freedreno_batch.c   |  1 +
 .../drivers/freedreno/freedreno_batch.h   |  8 +-
 .../drivers/freedreno/freedreno_draw.c| 99 ---
 .../drivers/freedreno/freedreno_resource.c| 41 
 .../drivers/freedreno/freedreno_state.h   | 10 ++
 .../drivers/freedreno/freedreno_surface.h | 18 
 7 files changed, 145 insertions(+), 35 deletions(-)

diff --git a/src/gallium/drivers/freedreno/a6xx/fd6_gmem.c 
b/src/gallium/drivers/freedreno/a6xx/fd6_gmem.c
index 8cda7d6ddae..0a82f69fc62 100644
--- a/src/gallium/drivers/freedreno/a6xx/fd6_gmem.c
+++ b/src/gallium/drivers/freedreno/a6xx/fd6_gmem.c
@@ -36,6 +36,7 @@
 #include "freedreno_draw.h"
 #include "freedreno_state.h"
 #include "freedreno_resource.h"
+#include "freedreno_surface.h"
 
 #include "fd6_gmem.h"
 #include "fd6_context.h"
@@ -901,7 +902,7 @@ emit_resolve_blit(struct fd_batch *batch,
 {
uint32_t info = 0;
 
-   if (!rsc->valid)
+   if (!fd_surface_valid(psurf))
return;
 
switch (buffer) {
diff --git a/src/gallium/drivers/freedreno/freedreno_batch.c 
b/src/gallium/drivers/freedreno/freedreno_batch.c
index eae2f68ce11..280a41a 100644
--- a/src/gallium/drivers/freedreno/freedreno_batch.c
+++ b/src/gallium/drivers/freedreno/freedreno_batch.c
@@ -78,6 +78,7 @@ batch_init(struct fd_batch *batch)
batch->cleared = 0;
batch->fast_cleared = 0;
batch->invalidated = 0;
+   batch->partial_invalidated = 0;
batch->restore = batch->resolve = 0;
batch->needs_flush = false;
batch->flushed = false;
diff --git a/src/gallium/drivers/freedreno/freedreno_batch.h 
b/src/gallium/drivers/freedreno/freedreno_batch.h
index a40d36094cd..b5c1c321bf0 100644
--- a/src/gallium/drivers/freedreno/freedreno_batch.h
+++ b/src/gallium/drivers/freedreno/freedreno_batch.h
@@ -95,7 +95,13 @@ struct fd_batch {
FD_BUFFER_DEPTH   = PIPE_CLEAR_DEPTH,
FD_BUFFER_STENCIL = PIPE_CLEAR_STENCIL,
FD_BUFFER_ALL = FD_BUFFER_COLOR | FD_BUFFER_DEPTH | 
FD_BUFFER_STENCIL,
-   } invalidated, cleared, fast_cleared, restore, resolve;
+   } invalidated, partial_invalidated, cleared, fast_cleared, restore, 
resolve;
+
+   /* for partially invalidated fb surfaces, track the invalidated rect: */
+   struct {
+   struct pipe_scissor_state zsbuf;
+   struct pipe_scissor_state cbuf[8];
+   } invalid;
 
/* is this a non-draw batch (ie compute/blit which has no pfb state)? */
bool nondraw : 1;
diff --git a/src/gallium/drivers/freedreno/freedreno_draw.c 
b/src/gallium/drivers/freedreno/freedreno_draw.c
index f17cb563063..07c3ecfa805 100644
--- a/src/gallium/drivers/freedreno/freedreno_draw.c
+++ b/src/gallium/drivers/freedreno/freedreno_draw.c
@@ -36,6 +36,7 @@
 #include "freedreno_context.h"
 #include "freedreno_fence.h"
 #include "freedreno_state.h"
+#include "freedreno_surface.h"
 #include "freedreno_resource.h"
 #include "freedreno_query_acc.h"
 #include "freedreno_query_hw.h"
@@ -57,6 +58,14 @@ resource_written(struct fd_batch *batch, struct 
pipe_resource *prsc)
fd_batch_resource_used(batch, fd_resource(prsc), true);
 }
 
+static void
+surface_valid(struct pipe_surface *psurf)
+{
+   struct fd_surface *surf = fd_surface(psurf);
+   surf->invalid = false;
+   surf->partial_invalid = false;
+}
+
 static void
 fd_draw_vbo(struct pipe_context *pctx, const struct pipe_draw_info *info)
 {
@@ -138,59 +147,78 @@ fd_draw_vbo(struct pipe_context *pctx, const struct 
pipe_draw_info *info)
mtx_lock(&ctx->screen->lock);
 
if (ctx->dirty & (FD_DIRTY_FRAMEBUFFER | FD_DIRTY_ZSA)) {
+   unsigned b = 0;
if (fd_depth_enabled(ctx)) {
-   if (fd_resource(pfb->zsbuf->texture)->valid) {
-   restore_buffers |= FD_BUFFER_DEPTH;
-   } else {
-   batch->invalidated |= FD_BUFFER_DEPTH;
-   }
-   buffers |= FD_BUFFER_DEPTH;
-   resource_written(batch, pfb->zsbuf->texture);
+   b |= FD_BUFFER_DEPTH;
batch->gmem_reason |= FD_GMEM_DEPTH_ENABLED;
}
 
if (fd_stencil_enabled(ctx)) {
-   if (fd_resource(pfb->zsbuf->texture)->valid) {
-   restore_buffers |= FD_BUFFER_STENCIL;
+   b |= FD_BUFFER_STENCIL;
+   batch->gmem_reason |= FD_GMEM_STENCIL_ENABLED;
+   }
+
+   if (b) {
+   if (fd_surface_valid(pfb->zsbuf)) {
+   restore_buffers |= b;
+   } else if (fd_surface_partially_valid(pfb->zsbuf)) {
+   restore_b

[Mesa-dev] [PATCH 4/6] mesa/st: wire up DiscardSubFramebuffer

[Rob Clark]

Signed-off-by: Rob Clark 
---
 src/gallium/include/pipe/p_context.h | 11 +++
 src/mesa/state_tracker/st_cb_fbo.c   | 26 ++
 2 files changed, 37 insertions(+)

diff --git a/src/gallium/include/pipe/p_context.h 
b/src/gallium/include/pipe/p_context.h
index d4e9179b78a..eb52c7e9a4e 100644
--- a/src/gallium/include/pipe/p_context.h
+++ b/src/gallium/include/pipe/p_context.h
@@ -811,6 +811,17 @@ struct pipe_context {
void (*invalidate_surface)(struct pipe_context *ctx,
   struct pipe_surface *surf);
 
+   /**
+* Invalidate a portion of a surface.  This is used to
+*
+* (1) implement glInvalidateSubFramebuffer() and friends
+* (2) as a hint before a scissored clear (which is turned into draw_vbo()
+* that the cleared rect can be discarded
+*/
+   void (*invalidate_sub_surface)(struct pipe_context *ctx,
+  struct pipe_surface *surf,
+  const struct pipe_scissor_state *rect);
+
/**
 * Return information about unexpected device resets.
 */
diff --git a/src/mesa/state_tracker/st_cb_fbo.c 
b/src/mesa/state_tracker/st_cb_fbo.c
index 3ece1d4a9de..50c27ea51d9 100644
--- a/src/mesa/state_tracker/st_cb_fbo.c
+++ b/src/mesa/state_tracker/st_cb_fbo.c
@@ -774,6 +774,31 @@ st_discard_framebuffer(struct gl_context *ctx, struct 
gl_framebuffer *fb,
   st->pipe->invalidate_surface(st->pipe, psurf);
 }
 
+static void
+st_discard_sub_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
+   struct gl_renderbuffer_attachment *att, GLint x,
+   GLint y, GLsizei width, GLsizei height)
+{
+   struct st_context *st = st_context(ctx);
+   struct pipe_surface *psurf;
+
+   if (!att->Renderbuffer)
+  return;
+
+   psurf = st_renderbuffer(att->Renderbuffer)->surface;
+
+   if (st->pipe->invalidate_sub_surface) {
+  struct pipe_scissor_state rect;
+
+  rect.minx = x;
+  rect.maxx = x + width - 1;
+  rect.miny = y;
+  rect.maxy = y + height - 1;
+
+  st->pipe->invalidate_sub_surface(st->pipe, psurf, &rect);
+   }
+}
+
 /**
  * Called via glDrawBuffer.  We only provide this driver function so that we
  * can check if we need to allocate a new renderbuffer.  Specifically, we
@@ -952,6 +977,7 @@ st_init_fbo_functions(struct dd_function_table *functions)
functions->FinishRenderTexture = st_finish_render_texture;
functions->ValidateFramebuffer = st_validate_framebuffer;
functions->DiscardFramebuffer = st_discard_framebuffer;
+   functions->DiscardSubFramebuffer = st_discard_sub_framebuffer;
 
functions->DrawBufferAllocate = st_DrawBufferAllocate;
functions->ReadBuffer = st_ReadBuffer;
-- 
2.19.2

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

Re: [Mesa-dev] [PATCH 10/12] virgl: modify how we handle GL_MAP_FLUSH_EXPLICIT_BIT

[Elie Tournier]

On Mon, Dec 10, 2018 at 10:20:36AM -0800, Gurchetan Singh wrote:
> Previously, we ignored the the glUnmap(..) operation and
> flushed before we flush the cbuf.  Now, let's just flush
> the data when we unmap.
> 
> Neither method is optimal, for example:
> 
> glMapBufferRange(.., 0, 100, GL_MAP_FLUSH_EXPLICIT_BIT)
> glFlushMappedBufferRange(.., 25, 30)
> glFlushMappedBufferRange(.., 65, 70)
> 
> We'll end up flushing 25 --> 70.  Maybe we can fix this later.
> 
> v2: Add fixme comment in the code (Elie)
Thanks.
I still have to run some regressions tests. They are a bit slow on my system.
So for now, the series is:
Acked-by: Elie Tournier 
> ---
>  src/gallium/drivers/virgl/virgl_buffer.c   | 46 +++---
>  src/gallium/drivers/virgl/virgl_context.c  | 34 +---
>  src/gallium/drivers/virgl/virgl_context.h  |  1 -
>  src/gallium/drivers/virgl/virgl_resource.h | 13 --
>  4 files changed, 25 insertions(+), 69 deletions(-)
> 
> diff --git a/src/gallium/drivers/virgl/virgl_buffer.c 
> b/src/gallium/drivers/virgl/virgl_buffer.c
> index d5d728735e..a20deab549 100644
> --- a/src/gallium/drivers/virgl/virgl_buffer.c
> +++ b/src/gallium/drivers/virgl/virgl_buffer.c
> @@ -33,7 +33,6 @@ static void virgl_buffer_destroy(struct pipe_screen *screen,
> struct virgl_screen *vs = virgl_screen(screen);
> struct virgl_buffer *vbuf = virgl_buffer(buf);
>  
> -   util_range_destroy(&vbuf->valid_buffer_range);
> vs->vws->resource_unref(vs->vws, vbuf->base.hw_res);
> FREE(vbuf);
>  }
> @@ -53,7 +52,7 @@ static void *virgl_buffer_transfer_map(struct pipe_context 
> *ctx,
> bool readback;
> bool doflushwait = false;
>  
> -   if ((usage & PIPE_TRANSFER_READ) && (vbuf->on_list == TRUE))
> +   if (usage & PIPE_TRANSFER_READ)
>doflushwait = true;
> else
>doflushwait = virgl_res_needs_flush_wait(vctx, &vbuf->base, usage);
> @@ -92,13 +91,19 @@ static void virgl_buffer_transfer_unmap(struct 
> pipe_context *ctx,
> struct virgl_buffer *vbuf = virgl_buffer(transfer->resource);
>  
> if (trans->base.usage & PIPE_TRANSFER_WRITE) {
> -  if (!(transfer->usage & PIPE_TRANSFER_FLUSH_EXPLICIT)) {
> - struct virgl_screen *vs = virgl_screen(ctx->screen);
> - vctx->num_transfers++;
> - vs->vws->transfer_put(vs->vws, vbuf->base.hw_res,
> -   &transfer->box, trans->base.stride, 
> trans->base.layer_stride, trans->offset, transfer->level);
> -
> +  struct virgl_screen *vs = virgl_screen(ctx->screen);
> +  if (transfer->usage & PIPE_TRANSFER_FLUSH_EXPLICIT) {
> + transfer->box.x += trans->range.start;
> + transfer->box.width = trans->range.end - trans->range.start;
> + trans->offset = transfer->box.x;
>}
> +
> +  vctx->num_transfers++;
> +  vs->vws->transfer_put(vs->vws, vbuf->base.hw_res,
> +&transfer->box, trans->base.stride,
> +trans->base.layer_stride, trans->offset,
> +transfer->level);
> +
> }
>  
> virgl_resource_destroy_transfer(vctx, trans);
> @@ -108,20 +113,19 @@ static void virgl_buffer_transfer_flush_region(struct 
> pipe_context *ctx,
> struct pipe_transfer 
> *transfer,
> const struct pipe_box *box)
>  {
> -   struct virgl_context *vctx = virgl_context(ctx);
> struct virgl_buffer *vbuf = virgl_buffer(transfer->resource);
> +   struct virgl_transfer *trans = virgl_transfer(transfer);
>  
> -   if (!vbuf->on_list) {
> -   struct pipe_resource *res = NULL;
> -
> -   list_addtail(&vbuf->flush_list, &vctx->to_flush_bufs);
> -   vbuf->on_list = TRUE;
> -   pipe_resource_reference(&res, &vbuf->base.u.b);
> -   }
> -
> -   util_range_add(&vbuf->valid_buffer_range, transfer->box.x + box->x,
> -  transfer->box.x + box->x + box->width);
> -
> +   /*
> +* FIXME: This is not optimal.  For example,
> +*
> +* glMapBufferRange(.., 0, 100, GL_MAP_FLUSH_EXPLICIT_BIT)
> +* glFlushMappedBufferRange(.., 25, 30)
> +* glFlushMappedBufferRange(.., 65, 70)
> +*
> +* We'll end up flushing 25 --> 70.
> +*/
> +   util_range_add(&trans->range, box->x, box->x + box->width);
> vbuf->base.clean = FALSE;
>  }
>  
> @@ -145,7 +149,6 @@ struct pipe_resource *virgl_buffer_create(struct 
> virgl_screen *vs,
> buf->base.u.b.screen = &vs->base;
> buf->base.u.vtbl = &virgl_buffer_vtbl;
> pipe_reference_init(&buf->base.u.b.reference, 1);
> -   util_range_init(&buf->valid_buffer_range);
> virgl_resource_layout(&buf->base.u.b, &buf->metadata);
>  
> vbind = pipe_to_virgl_bind(template->bind);
> @@ -155,6 +158,5 @@ struct pipe_resource *virgl_buffer_create(struct 
> virgl_screen *vs,
> template->width0, 1, 1, 1, 0, 
> 0,
> buf->metada

[Mesa-dev] [PATCH 2/6] mesa: wire up InvalidateSubFramebuffer

[Rob Clark]

Signed-off-by: Rob Clark 
---
 src/mesa/main/dd.h   |  3 +++
 src/mesa/main/fbobject.c | 34 +-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
index 1214eeaa474..c7112677223 100644
--- a/src/mesa/main/dd.h
+++ b/src/mesa/main/dd.h
@@ -786,6 +786,9 @@ struct dd_function_table {
GLbitfield mask, GLenum filter);
void (*DiscardFramebuffer)(struct gl_context *ctx, struct gl_framebuffer 
*fb,
   struct gl_renderbuffer_attachment *att);
+   void (*DiscardSubFramebuffer)(struct gl_context *ctx, struct gl_framebuffer 
*fb,
+ struct gl_renderbuffer_attachment *att, GLint 
x,
+ GLint y, GLsizei width, GLsizei height);
 
/**
 * \name Functions for GL_ARB_sample_locations
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index f931e8f76b1..8ef5eb747c0 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -4699,12 +4699,41 @@ discard_framebuffer(struct gl_context *ctx, struct 
gl_framebuffer *fb,
}
 }
 
+static void
+discard_sub_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb,
+GLsizei numAttachments, const GLenum *attachments,
+GLint x, GLint y, GLsizei width, GLsizei height)
+{
+   GLint i;
+
+   if (!ctx->Driver.DiscardSubFramebuffer)
+  return;
+
+   for (i = 0; i < numAttachments; i++) {
+  struct gl_renderbuffer_attachment *att =
+get_fb_attachment(ctx, fb, attachments[i]);
+
+  if (!att)
+ continue;
+
+  ctx->Driver.DiscardSubFramebuffer(ctx, fb, att, x, y, width, height);
+   }
+}
+
 void GLAPIENTRY
 _mesa_InvalidateSubFramebuffer_no_error(GLenum target, GLsizei numAttachments,
 const GLenum *attachments, GLint x,
 GLint y, GLsizei width, GLsizei height)
 {
-   /* no-op */
+   struct gl_framebuffer *fb;
+   GET_CURRENT_CONTEXT(ctx);
+
+   fb = get_framebuffer_target(ctx, target);
+   if (!fb)
+  return;
+
+   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
+   x, y, width, height);
 }
 
 
@@ -4727,6 +4756,9 @@ _mesa_InvalidateSubFramebuffer(GLenum target, GLsizei 
numAttachments,
invalidate_framebuffer_storage(ctx, fb, numAttachments, attachments,
   x, y, width, height,
   "glInvalidateSubFramebuffer");
+
+   discard_sub_framebuffer(ctx, fb, numAttachments, attachments,
+   x, y, width, height);
 }
 
 
-- 
2.19.2

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

Re: [Mesa-dev] [PATCH] gallium/aux: add is_unorm() helper

[Rob Clark]

The z24s8 permutations are is_mixed, the x24s8 permutations are not
and will show up as is_unorm() (would have been an issue for
is_snorm() too except they are unsigned)

The 10_10_10_2 formats are not mixed.

In either case, I'm not sure is_norm() returning true is *incorrect*..

BR,
-R

On Tue, Dec 11, 2018 at 4:21 PM Ilia Mirkin  wrote:
>
> So ... Z24 will end up with is_unorm() == true? [Just guessing --
> assume it doesn't hae is_mixed == true.] Also, does RGB10A2 have mixed
> set? If so, then it won't report unorm. Not 100% sure if is_mixed is
> only for norm + int mixing.
> On Tue, Dec 11, 2018 at 4:05 PM Rob Clark  wrote:
> >
> > We already had one for is_snorm() but not unorm.
> >
> > Signed-off-by: Rob Clark 
> > ---
> >  src/gallium/auxiliary/util/u_format.c | 21 +
> >  src/gallium/auxiliary/util/u_format.h |  3 +++
> >  2 files changed, 24 insertions(+)
> >
> > diff --git a/src/gallium/auxiliary/util/u_format.c 
> > b/src/gallium/auxiliary/util/u_format.c
> > index e43a619313e..231e89017b4 100644
> > --- a/src/gallium/auxiliary/util/u_format.c
> > +++ b/src/gallium/auxiliary/util/u_format.c
> > @@ -169,6 +169,27 @@ util_format_is_snorm(enum pipe_format format)
> >desc->channel[i].normalized;
> >  }
> >
> > +/**
> > + * Returns true if all non-void channels are normalized unsigned.
> > + */
> > +boolean
> > +util_format_is_unorm(enum pipe_format format)
> > +{
> > +   const struct util_format_description *desc = 
> > util_format_description(format);
> > +   int i;
> > +
> > +   if (desc->is_mixed)
> > +  return FALSE;
> > +
> > +   i = util_format_get_first_non_void_channel(format);
> > +   if (i == -1)
> > +  return FALSE;
> > +
> > +   return desc->channel[i].type == UTIL_FORMAT_TYPE_UNSIGNED &&
> > +  !desc->channel[i].pure_integer &&
> > +  desc->channel[i].normalized;
> > +}
> > +
> >  boolean
> >  util_format_is_snorm8(enum pipe_format format)
> >  {
> > diff --git a/src/gallium/auxiliary/util/u_format.h 
> > b/src/gallium/auxiliary/util/u_format.h
> > index 5bcfc1f1154..8dcc438a4a1 100644
> > --- a/src/gallium/auxiliary/util/u_format.h
> > +++ b/src/gallium/auxiliary/util/u_format.h
> > @@ -726,6 +726,9 @@ util_format_is_pure_uint(enum pipe_format format);
> >  boolean
> >  util_format_is_snorm(enum pipe_format format);
> >
> > +boolean
> > +util_format_is_unorm(enum pipe_format format);
> > +
> >  boolean
> >  util_format_is_snorm8(enum pipe_format format);
> >
> > --
> > 2.19.2
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 108946] High memory usage in Black Mesa

[bugzilla-daemon]

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

--- Comment #5 from Justin Mitzel  ---
Created attachment 142782
  --> https://bugs.freedesktop.org/attachment.cgi?id=142782&action=edit
BMS apitrace with -dev argument

-dev option creates static image on the menu-screen instead of an active scene
without that argument

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

Re: [Mesa-dev] [PATCH] etnaviv: fix resource usage tracking across different pipe_context's

[Lukas F. Hartmann]

Hi,

I tested the patch on i.MX6QP with:
Linux reform 4.20.0-rc2-00133-g1ce80e0fe98e-dirty #10 SMP Mon Nov 26
02:02:42 CET 2018 armv7l GNU/Linux

Running a recent Xorg built from source with modesetting driver and
etnaviv.

I was getting segfaults after a few seconds of usage and tracked them
down to a corrupt pointer (probably use-after-free):

http://dump.mntmn.com/marexpatch-trace1.txt.html

After looking at your patch for a while I came up with this addition
(last line) in etna_resource.c, to remove the resource from the
used_resources set when it is destroyed.

@@ -464,6 +469,9 @@ etna_resource_destroy(struct pipe_screen *pscreen,
struct pipe_resource *prsc) {
struct etna_resource *rsc = etna_resource(prsc);
 
+   _mesa_set_destroy(rsc->pending_ctx, NULL);
+   _mesa_set_remove_key(etna_screen(pscreen)->used_resources, rsc);

I've been running with this version for a few days and it seems stable.
The patch seems to do its job (tested qupzilla and qutebrowser, no
more corrupted tiles). Just sometimes an occassional black tile shows
up. So something is still up, but the patch is a perceived improvement.

Cheers
Lukas F. Hartmann (mntmn)
https://mntmn.com

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

Re: [Mesa-dev] last call for autotools

[Samuel Pitoiset]

On 12/11/18 7:18 PM, Eric Anholt wrote:

Emil Velikov  writes:


On Mon, 10 Dec 2018 at 23:11, Dylan Baker  wrote:


Meson 0.49.0 has been out for a couple of days now, and I'd like to make the
final call for autotools. My patch is so massive that it's a huge pain to send
to the list, the latest versions is here:
https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools


Can you split this up a bit? I'm mostly conserved about a couple of things:
- we're loosing multiple testing permutations with Travis - there's no
meson equivalent
- needed/missing bits are impossible to spot as-is

Personally, I'd do something like:
  - move .travis autotools permutations to meson
  - docs changes
  - rm Makefile{.*,}.am autogen.sh and configure.ac
  - rm Makefile.sources
  - .gitignore


I don't think we should block on anything related to travis at this
point.  I wrote it initially hoping it would let us prevent build
breakages.  Instead, whenever I try to use travis to prevent build
breakage, I find that travis has already been broken for a while.

The CI needs to be integrated with our actual repository host, or it
doesn't work.  We should just delete .travis.yml and push the gitlab
stuff forward instead.



Gitlab CI is probably the way to go.



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


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

[Mesa-dev] [Bug 108946] High memory usage in Black Mesa

[bugzilla-daemon]

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

--- Comment #4 from Justin Mitzel  ---
Created attachment 142781
  --> https://bugs.freedesktop.org/attachment.cgi?id=142781&action=edit
BMS apitrace with default arguments

-- 
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] last call for autotools

[Emil Velikov]

On Tue, 11 Dec 2018 at 19:49, Dylan Baker  wrote:
>
> Quoting Emil Velikov (2018-12-11 10:48:56)
> > On Tue, 11 Dec 2018 at 18:18, Eric Anholt  wrote:
> > >
> > > Emil Velikov  writes:
> > >
> > > > On Mon, 10 Dec 2018 at 23:11, Dylan Baker  wrote:
> > > >>
> > > >> Meson 0.49.0 has been out for a couple of days now, and I'd like to 
> > > >> make the
> > > >> final call for autotools. My patch is so massive that it's a huge pain 
> > > >> to send
> > > >> to the list, the latest versions is here:
> > > >> https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools
> > > >>
> > > > Can you split this up a bit? I'm mostly conserved about a couple of 
> > > > things:
> > > > - we're loosing multiple testing permutations with Travis - there's no
> > > > meson equivalent
> > > > - needed/missing bits are impossible to spot as-is
> > > >
> > > > Personally, I'd do something like:
> > > >  - move .travis autotools permutations to meson
> > > >  - docs changes
> > > >  - rm Makefile{.*,}.am autogen.sh and configure.ac
> > > >  - rm Makefile.sources
> > > >  - .gitignore
> > >
> > > I don't think we should block on anything related to travis at this
> > > point.  I wrote it initially hoping it would let us prevent build
> > > breakages.  Instead, whenever I try to use travis to prevent build
> > > breakage, I find that travis has already been broken for a while.
> > >
> > You must be really unlucky.
> >
> > > The CI needs to be integrated with our actual repository host, or it
> > > doesn't work.  We should just delete .travis.yml and push the gitlab
> > > stuff forward instead.
> >
> > If we have anything better in place (say gitlab CI/other) - sure.
> > Removing something without a replacement in place is quite meh.
> > That said, I think we agree that this is a topic for another day.
> >
> > HTH
> > -Emil
>
> I'm actually finding myself agreeing that travis is just a pain. Could you 
> help
> me debug my WIP of porting the autotools tests to meson? They're just failing
> randomly with no good debuging output:
>
> https://travis-ci.org/dcbaker/mesa/jobs/45289
>
If you don't mind I'll have a look first thing tomorrow morning. It's
9:43pm and I've yet to cook some food :-\

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

[Mesa-dev] [Bug 108946] High memory usage in Black Mesa

[bugzilla-daemon]

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

--- Comment #3 from Justin Mitzel  ---
Created attachment 142780
  --> https://bugs.freedesktop.org/attachment.cgi?id=142780&action=edit
Terminal Output upon executing bms.sh

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

[Mesa-dev] [Bug 108946] High memory usage in Black Mesa

[bugzilla-daemon]

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

--- Comment #2 from Justin Mitzel  ---
I can also confirm that this is not a Mesa 18.2 specific bug as I am running
Black Mesa on Mesa 19.0.0-devel. I will attach my own apitraces and terminal
output and hopefully this can be solved soon as the BM dev says that this
memory leak should be "Well known" to Mesa Devs.

-- 
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] [ANNOUNCE] mesa 18.3.1

[Emil Velikov]

Mesa 18.3.1 is now available.

This version disables the VK_EXT_pci_bus_info extension due to last
minute issues spotted in the specification.


Emil Velikov (3):
  docs: add sha256 checksums for 18.3.0
  Update version to 18.3.1
  docs: add release notes for 18.3.1

Jason Ekstrand (1):
  anv,radv: Disable VK_EXT_pci_bus_info

git tag: mesa-18.3.1

https://mesa.freedesktop.org/archive/mesa-18.3.1.tar.gz
MD5:  2de82245518020872fee4c2f9a8c709b  mesa-18.3.1.tar.gz
SHA1: 103cb6e8d52ea82ba30ecd546f4ca5c63ceef2e4  mesa-18.3.1.tar.gz
SHA256: 256d0c3d88e380c1b8e3fc5c6ac34001e3b7c30458b8b852407ec68b8ccd9fda  
mesa-18.3.1.tar.gz
SHA512: 
16e5b52246bcb8c014b59bf7d0ad77b0e350bca212c2ee3e2b8a66bbed59d2f8e2a557f210ea45f98db988039ebb348cb69acf77505fb8e33b29da5efb5307de
  mesa-18.3.1.tar.gz
PGP:  https://mesa.freedesktop.org/archive/mesa-18.3.1.tar.gz.sig

https://mesa.freedesktop.org/archive/mesa-18.3.1.tar.xz
MD5:  d60828056d77bfdbae0970f9b15fb1be  mesa-18.3.1.tar.xz
SHA1: 50ba2d37647fea77ea19416e8a6ffed34c313330  mesa-18.3.1.tar.xz
SHA256: 5b1f827d28684a25f6657289f8b7d47ac56395988c7ac23e0ec9a62b644bdc63  
mesa-18.3.1.tar.xz
SHA512: 
a68d39158cf1e868d70730d0641a0cfe4c6e5b3cd1bc0c47f54022402aca03503933084f6ddc722bf88c9b6d1281ba5c847ec4fed8092a9b33f90527d08e12db
  mesa-18.3.1.tar.xz
PGP:  https://mesa.freedesktop.org/archive/mesa-18.3.1.tar.xz.sig




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

Re: [Mesa-dev] [PATCH] gallium/aux: add is_unorm() helper

[Ilia Mirkin]

So ... Z24 will end up with is_unorm() == true? [Just guessing --
assume it doesn't hae is_mixed == true.] Also, does RGB10A2 have mixed
set? If so, then it won't report unorm. Not 100% sure if is_mixed is
only for norm + int mixing.
On Tue, Dec 11, 2018 at 4:05 PM Rob Clark  wrote:
>
> We already had one for is_snorm() but not unorm.
>
> Signed-off-by: Rob Clark 
> ---
>  src/gallium/auxiliary/util/u_format.c | 21 +
>  src/gallium/auxiliary/util/u_format.h |  3 +++
>  2 files changed, 24 insertions(+)
>
> diff --git a/src/gallium/auxiliary/util/u_format.c 
> b/src/gallium/auxiliary/util/u_format.c
> index e43a619313e..231e89017b4 100644
> --- a/src/gallium/auxiliary/util/u_format.c
> +++ b/src/gallium/auxiliary/util/u_format.c
> @@ -169,6 +169,27 @@ util_format_is_snorm(enum pipe_format format)
>desc->channel[i].normalized;
>  }
>
> +/**
> + * Returns true if all non-void channels are normalized unsigned.
> + */
> +boolean
> +util_format_is_unorm(enum pipe_format format)
> +{
> +   const struct util_format_description *desc = 
> util_format_description(format);
> +   int i;
> +
> +   if (desc->is_mixed)
> +  return FALSE;
> +
> +   i = util_format_get_first_non_void_channel(format);
> +   if (i == -1)
> +  return FALSE;
> +
> +   return desc->channel[i].type == UTIL_FORMAT_TYPE_UNSIGNED &&
> +  !desc->channel[i].pure_integer &&
> +  desc->channel[i].normalized;
> +}
> +
>  boolean
>  util_format_is_snorm8(enum pipe_format format)
>  {
> diff --git a/src/gallium/auxiliary/util/u_format.h 
> b/src/gallium/auxiliary/util/u_format.h
> index 5bcfc1f1154..8dcc438a4a1 100644
> --- a/src/gallium/auxiliary/util/u_format.h
> +++ b/src/gallium/auxiliary/util/u_format.h
> @@ -726,6 +726,9 @@ util_format_is_pure_uint(enum pipe_format format);
>  boolean
>  util_format_is_snorm(enum pipe_format format);
>
> +boolean
> +util_format_is_unorm(enum pipe_format format);
> +
>  boolean
>  util_format_is_snorm8(enum pipe_format format);
>
> --
> 2.19.2
>
> ___
> 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] gallium/aux: add is_unorm() helper

[Rob Clark]

We already had one for is_snorm() but not unorm.

Signed-off-by: Rob Clark 
---
 src/gallium/auxiliary/util/u_format.c | 21 +
 src/gallium/auxiliary/util/u_format.h |  3 +++
 2 files changed, 24 insertions(+)

diff --git a/src/gallium/auxiliary/util/u_format.c 
b/src/gallium/auxiliary/util/u_format.c
index e43a619313e..231e89017b4 100644
--- a/src/gallium/auxiliary/util/u_format.c
+++ b/src/gallium/auxiliary/util/u_format.c
@@ -169,6 +169,27 @@ util_format_is_snorm(enum pipe_format format)
   desc->channel[i].normalized;
 }
 
+/**
+ * Returns true if all non-void channels are normalized unsigned.
+ */
+boolean
+util_format_is_unorm(enum pipe_format format)
+{
+   const struct util_format_description *desc = 
util_format_description(format);
+   int i;
+
+   if (desc->is_mixed)
+  return FALSE;
+
+   i = util_format_get_first_non_void_channel(format);
+   if (i == -1)
+  return FALSE;
+
+   return desc->channel[i].type == UTIL_FORMAT_TYPE_UNSIGNED &&
+  !desc->channel[i].pure_integer &&
+  desc->channel[i].normalized;
+}
+
 boolean
 util_format_is_snorm8(enum pipe_format format)
 {
diff --git a/src/gallium/auxiliary/util/u_format.h 
b/src/gallium/auxiliary/util/u_format.h
index 5bcfc1f1154..8dcc438a4a1 100644
--- a/src/gallium/auxiliary/util/u_format.h
+++ b/src/gallium/auxiliary/util/u_format.h
@@ -726,6 +726,9 @@ util_format_is_pure_uint(enum pipe_format format);
 boolean
 util_format_is_snorm(enum pipe_format format);
 
+boolean
+util_format_is_unorm(enum pipe_format format);
+
 boolean
 util_format_is_snorm8(enum pipe_format format);
 
-- 
2.19.2

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

[Mesa-dev] [PATCH] radv: don't decompress HTILE for 16-bit depth surfaces on GFX8

[Samuel Pitoiset]

GFX8 only natively supports TC-compatible HTILE for 32-bit surfaces,
while GFX9 also supports 16-bit surfaces. Though, it's possible
to enable it for 16-bit depth surfaces if no Z planes are
compressed and the driver does that.

However, it appears that trying to decompress such a surface
ends up by reporting VM faults. This is probably because the
hardware uses 32-bit instead of 16-bit. Anyways, decompressing
a surface when no Z planes are compressed is just dumb.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107563
Signed-off-by: Samuel Pitoiset 
---
 src/amd/vulkan/radv_meta_decompress.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/amd/vulkan/radv_meta_decompress.c 
b/src/amd/vulkan/radv_meta_decompress.c
index fe8e114e91b..952765b16db 100644
--- a/src/amd/vulkan/radv_meta_decompress.c
+++ b/src/amd/vulkan/radv_meta_decompress.c
@@ -330,8 +330,15 @@ static void radv_process_depth_image_inplace(struct 
radv_cmd_buffer *cmd_buffer,
struct radv_meta_state *meta_state = &cmd_buffer->device->meta_state;
VkPipeline pipeline_h;
 
-   if (!radv_image_has_htile(image))
+   if (!radv_image_has_htile(image) ||
+   /* Don't decompress HTILE for 16-bit depth surfaces that are
+* TC-compatible because no Z planes are compressed on GFX8.
+*/
+   (cmd_buffer->device->physical_device->rad_info.chip_class == VI &&
+radv_image_is_tc_compat_htile(image) &&
+image->vk_format == VK_FORMAT_D16_UNORM)) {
return;
+   }
 
if (!meta_state->depth_decomp[samples_log2].decompress_pipeline) {
VkResult ret = create_pipeline(cmd_buffer->device, 
VK_NULL_HANDLE, samples,
-- 
2.20.0

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

[Mesa-dev] [Bug 107563] [RADV] Broken rendering in Unity demos

[bugzilla-daemon]

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

--- Comment #10 from Samuel Pitoiset  ---
This patch should fix the VM faults
https://patchwork.freedesktop.org/series/53914/

-- 
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] radv: adjust the VGT workaround for prim restart on GFX9

[Samuel Pitoiset]

On 12/11/18 8:57 PM, Marek Olšák wrote:
It's up to you as long as you're OK with downgraded performance of 
triangle strips with primitive restart.


I'm OK with it, at least for now. From my point of view, it's definitely 
better to avoid GPU hangs.


Bas, what's your opinion?



Marek

On Tue, Dec 11, 2018 at 10:08 AM Samuel Pitoiset 
mailto:samuel.pitoi...@gmail.com>> wrote:


ping?

After looking into this again today, I can't find any better solutions.
We should probably push this patch because at least two games are
affected. My opinion is that correctness is more important than
performance.

On 10/11/18 10:42 AM, Samuel Pitoiset wrote:
 > WD_SWITCH_ON_EOP seems to be the only workaround that fixes
 > the GPU hangs with Yakuza and The Evil Within on Vega. I don't
 > like as it might decrease geometry performance as pointed out
 > by Marek, but I don't know how to implement a better one.
 >
 > Cc: mesa-sta...@lists.freedesktop.org

 > Signed-off-by: Samuel Pitoiset mailto:samuel.pitoi...@gmail.com>>
 > ---
 >   src/amd/vulkan/radv_pipeline.c | 13 +++--
 >   1 file changed, 11 insertions(+), 2 deletions(-)
 >
 > diff --git a/src/amd/vulkan/radv_pipeline.c
b/src/amd/vulkan/radv_pipeline.c
 > index 426b417e172..2256b2c58e9 100644
 > --- a/src/amd/vulkan/radv_pipeline.c
 > +++ b/src/amd/vulkan/radv_pipeline.c
 > @@ -3412,14 +3412,23 @@
radv_compute_ia_multi_vgt_param_helpers(struct radv_pipeline *pipeline,
 >       }
 >
 >       /* Workaround for a VGT hang when strip primitive types are
used with
 > -      * primitive restart.
 > +      * primitive restart. This fixes GPU hangs with Yakuza and
The Evil
 > +      * Within, at least. Not sure if we can implement a better
workaround.
 >        */
 >       if (pipeline->graphics.prim_restart_enable &&
 >           (prim == V_008958_DI_PT_LINESTRIP ||
 >            prim == V_008958_DI_PT_TRISTRIP ||
 >            prim == V_008958_DI_PT_LINESTRIP_ADJ ||
 >            prim == V_008958_DI_PT_TRISTRIP_ADJ)) {
 > -             ia_multi_vgt_param.partial_vs_wave = true;
 > +             if (device->physical_device->rad_info.chip_class >=
GFX9) {
 > +                     /* XXX: This might decrease geometry
performance by 2x,
 > +                      * but this appears to be the only
workaround that
 > +                      * fixes the GPU hang.
 > +                      */
 > +                     ia_multi_vgt_param.wd_switch_on_eop = true;
 > +             } else {
 > +                     ia_multi_vgt_param.partial_vs_wave = true;
 > +             }
 >       }
 >
 >       ia_multi_vgt_param.base =
 >
___
mesa-stable mailing list
mesa-sta...@lists.freedesktop.org

https://lists.freedesktop.org/mailman/listinfo/mesa-stable


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

Re: [Mesa-dev] [Mesa-stable] [PATCH] radv: adjust the VGT workaround for prim restart on GFX9

[Marek Olšák]

It's up to you as long as you're OK with downgraded performance of triangle
strips with primitive restart.

Marek

On Tue, Dec 11, 2018 at 10:08 AM Samuel Pitoiset 
wrote:

> ping?
>
> After looking into this again today, I can't find any better solutions.
> We should probably push this patch because at least two games are
> affected. My opinion is that correctness is more important than
> performance.
>
> On 10/11/18 10:42 AM, Samuel Pitoiset wrote:
> > WD_SWITCH_ON_EOP seems to be the only workaround that fixes
> > the GPU hangs with Yakuza and The Evil Within on Vega. I don't
> > like as it might decrease geometry performance as pointed out
> > by Marek, but I don't know how to implement a better one.
> >
> > Cc: mesa-sta...@lists.freedesktop.org
> > Signed-off-by: Samuel Pitoiset 
> > ---
> >   src/amd/vulkan/radv_pipeline.c | 13 +++--
> >   1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/amd/vulkan/radv_pipeline.c
> b/src/amd/vulkan/radv_pipeline.c
> > index 426b417e172..2256b2c58e9 100644
> > --- a/src/amd/vulkan/radv_pipeline.c
> > +++ b/src/amd/vulkan/radv_pipeline.c
> > @@ -3412,14 +3412,23 @@ radv_compute_ia_multi_vgt_param_helpers(struct
> radv_pipeline *pipeline,
> >   }
> >
> >   /* Workaround for a VGT hang when strip primitive types are used
> with
> > -  * primitive restart.
> > +  * primitive restart. This fixes GPU hangs with Yakuza and The Evil
> > +  * Within, at least. Not sure if we can implement a better
> workaround.
> >*/
> >   if (pipeline->graphics.prim_restart_enable &&
> >   (prim == V_008958_DI_PT_LINESTRIP ||
> >prim == V_008958_DI_PT_TRISTRIP ||
> >prim == V_008958_DI_PT_LINESTRIP_ADJ ||
> >prim == V_008958_DI_PT_TRISTRIP_ADJ)) {
> > - ia_multi_vgt_param.partial_vs_wave = true;
> > + if (device->physical_device->rad_info.chip_class >= GFX9) {
> > + /* XXX: This might decrease geometry performance
> by 2x,
> > +  * but this appears to be the only workaround that
> > +  * fixes the GPU hang.
> > +  */
> > + ia_multi_vgt_param.wd_switch_on_eop = true;
> > + } else {
> > + ia_multi_vgt_param.partial_vs_wave = true;
> > + }
> >   }
> >
> >   ia_multi_vgt_param.base =
> >
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] last call for autotools

[Dylan Baker]

Quoting Emil Velikov (2018-12-11 10:48:56)
> On Tue, 11 Dec 2018 at 18:18, Eric Anholt  wrote:
> >
> > Emil Velikov  writes:
> >
> > > On Mon, 10 Dec 2018 at 23:11, Dylan Baker  wrote:
> > >>
> > >> Meson 0.49.0 has been out for a couple of days now, and I'd like to make 
> > >> the
> > >> final call for autotools. My patch is so massive that it's a huge pain 
> > >> to send
> > >> to the list, the latest versions is here:
> > >> https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools
> > >>
> > > Can you split this up a bit? I'm mostly conserved about a couple of 
> > > things:
> > > - we're loosing multiple testing permutations with Travis - there's no
> > > meson equivalent
> > > - needed/missing bits are impossible to spot as-is
> > >
> > > Personally, I'd do something like:
> > >  - move .travis autotools permutations to meson
> > >  - docs changes
> > >  - rm Makefile{.*,}.am autogen.sh and configure.ac
> > >  - rm Makefile.sources
> > >  - .gitignore
> >
> > I don't think we should block on anything related to travis at this
> > point.  I wrote it initially hoping it would let us prevent build
> > breakages.  Instead, whenever I try to use travis to prevent build
> > breakage, I find that travis has already been broken for a while.
> >
> You must be really unlucky.
> 
> > The CI needs to be integrated with our actual repository host, or it
> > doesn't work.  We should just delete .travis.yml and push the gitlab
> > stuff forward instead.
> 
> If we have anything better in place (say gitlab CI/other) - sure.
> Removing something without a replacement in place is quite meh.
> That said, I think we agree that this is a topic for another day.
> 
> HTH
> -Emil

I'm actually finding myself agreeing that travis is just a pain. Could you help
me debug my WIP of porting the autotools tests to meson? They're just failing
randomly with no good debuging output:

https://travis-ci.org/dcbaker/mesa/jobs/45289

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 v2] glx: mandate xf86vidmode only for "drm" dri platforms

[Eric Engestrom]

On Tuesday, 2018-12-11 17:22:03 +, Emil Velikov wrote:
> From: Emil Velikov 
> 
> Currently we have the three dri "platforms" - drm, apple and windows.
> 
> Since xf86vidmode is a thing only for the drm one, adjust the
> preprocessor guards and correctly check for the dependency.
> 
> v2: terminate the GLX_USE_WINDOWSGL hunk
> 
> Cc: Jon TURNEY 
> Cc: Dylan Baker 
> Cc: Eric Engestrom 
> Fixes: 5bc509363b6 ("glx: make xf86vidmode mandatory for direct rendering")
> Signed-off-by: Emil Velikov 
> ---
> This time it's actually tested ;-)
> ---
>  configure.ac  | 4 ++--
>  meson.build   | 4 ++--
>  src/glx/glxcmds.c | 6 --
>  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 5d3da4b7c48..9b437a252cc 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1716,6 +1716,8 @@ xdri)
>  if test x"$enable_dri" = xyes; then
> dri_modules="$dri_modules xcb-dri2 >= $XCBDRI2_REQUIRED"
>  fi
> +
> +dri_modules="$dri_modules xxf86vm"
>  fi
>  if test x"$dri_platform" = xapple ; then
>  DEFINES="$DEFINES -DGLX_USE_APPLEGL"
> @@ -1725,8 +1727,6 @@ xdri)
>  fi
>  fi
>  
> -dri_modules="$dri_modules xxf86vm"
> -
>  PKG_CHECK_MODULES([DRIGL], [$dri_modules])
>  GL_PC_REQ_PRIV="$GL_PC_REQ_PRIV $dri_modules"
>  X11_INCLUDES="$X11_INCLUDES $DRIGL_CFLAGS"
> diff --git a/meson.build b/meson.build
> index 4f6176b8d96..fe647f682c1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1349,7 +1349,6 @@ if with_platform_x11
>  dep_xdamage = dependency('xdamage', version : '>= 1.1')
>  dep_xfixes = dependency('xfixes')
>  dep_xcb_glx = dependency('xcb-glx', version : '>= 1.8.1')
> -dep_xxf86vm = dependency('xxf86vm')
>endif
>if (with_any_vk or with_glx == 'dri' or
> (with_gallium_vdpau or with_gallium_xvmc or with_gallium_va or
> @@ -1376,6 +1375,7 @@ if with_platform_x11
>if with_glx == 'dri'
>  if with_dri_platform == 'drm'
>dep_dri2proto = dependency('dri2proto', version : '>= 2.8')
> +  dep_xxf86vm = dependency('xxf86vm')
>  endif
>  dep_glproto = dependency('glproto', version : '>= 1.4.14')
>endif
> @@ -1426,8 +1426,8 @@ elif with_glx == 'dri'
>  'xcb-glx >= 1.8.1']
>if with_dri_platform == 'drm'
>  gl_priv_reqs += 'xcb-dri2 >= 1.8'
> +gl_priv_reqs += 'xxf86vm'
>endif
> -  gl_priv_reqs += 'xxf86vm'
>  endif
>  if dep_libdrm.found()
>gl_priv_reqs += 'libdrm >= 2.4.75'
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index b940c8ebdbe..d00c0d4816d 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -45,8 +45,10 @@
>  #include "apple/apple_glx.h"
>  #include "util/debug.h"
>  #else
> +#ifndef GLX_USE_WINDOWSGL
>  #include 
>  #include 
> +#endif /* GLX_USE_WINDOWSGL */

Actually `#elif !defined(...)` would've been nicer, but it doesn't
really matter; this looks all reasonable to me, so:
Acked-by: Eric Engestrom 

>  #endif
>  #endif
>  
> @@ -2081,7 +2083,7 @@ __glXGetSyncValuesOML(Display * dpy, GLXDrawable 
> drawable,
> return False;
>  }
>  
> -#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
> +#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL) && 
> !defined(GLX_USE_WINDOWSGL)
>  _X_HIDDEN GLboolean
>  __glxGetMscRate(struct glx_screen *psc,
>   int32_t * numerator, int32_t * denominator)
> @@ -2157,7 +2159,7 @@ _X_HIDDEN GLboolean
>  __glXGetMscRateOML(Display * dpy, GLXDrawable drawable,
> int32_t * numerator, int32_t * denominator)
>  {
> -#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
> +#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL) && 
> !defined(GLX_USE_WINDOWSGL)
> __GLXDRIdrawable *draw = GetGLXDRIDrawable(dpy, drawable);
>  
> if (draw == NULL)
> -- 
> 2.19.2
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] last call for autotools

[Emil Velikov]

On Tue, 11 Dec 2018 at 18:18, Eric Anholt  wrote:
>
> Emil Velikov  writes:
>
> > On Mon, 10 Dec 2018 at 23:11, Dylan Baker  wrote:
> >>
> >> Meson 0.49.0 has been out for a couple of days now, and I'd like to make 
> >> the
> >> final call for autotools. My patch is so massive that it's a huge pain to 
> >> send
> >> to the list, the latest versions is here:
> >> https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools
> >>
> > Can you split this up a bit? I'm mostly conserved about a couple of things:
> > - we're loosing multiple testing permutations with Travis - there's no
> > meson equivalent
> > - needed/missing bits are impossible to spot as-is
> >
> > Personally, I'd do something like:
> >  - move .travis autotools permutations to meson
> >  - docs changes
> >  - rm Makefile{.*,}.am autogen.sh and configure.ac
> >  - rm Makefile.sources
> >  - .gitignore
>
> I don't think we should block on anything related to travis at this
> point.  I wrote it initially hoping it would let us prevent build
> breakages.  Instead, whenever I try to use travis to prevent build
> breakage, I find that travis has already been broken for a while.
>
You must be really unlucky.

> The CI needs to be integrated with our actual repository host, or it
> doesn't work.  We should just delete .travis.yml and push the gitlab
> stuff forward instead.

If we have anything better in place (say gitlab CI/other) - sure.
Removing something without a replacement in place is quite meh.
That said, I think we agree that this is a topic for another day.

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

Re: [Mesa-dev] last call for autotools

[Eric Anholt]

Emil Velikov  writes:

> On Mon, 10 Dec 2018 at 23:11, Dylan Baker  wrote:
>>
>> Meson 0.49.0 has been out for a couple of days now, and I'd like to make the
>> final call for autotools. My patch is so massive that it's a huge pain to 
>> send
>> to the list, the latest versions is here:
>> https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools
>>
> Can you split this up a bit? I'm mostly conserved about a couple of things:
> - we're loosing multiple testing permutations with Travis - there's no
> meson equivalent
> - needed/missing bits are impossible to spot as-is
>
> Personally, I'd do something like:
>  - move .travis autotools permutations to meson
>  - docs changes
>  - rm Makefile{.*,}.am autogen.sh and configure.ac
>  - rm Makefile.sources
>  - .gitignore

I don't think we should block on anything related to travis at this
point.  I wrote it initially hoping it would let us prevent build
breakages.  Instead, whenever I try to use travis to prevent build
breakage, I find that travis has already been broken for a while.

The CI needs to be integrated with our actual repository host, or it
doesn't work.  We should just delete .travis.yml and push the gitlab
stuff forward instead.


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

Re: [Mesa-dev] [PATCH v2] glx: mandate xf86vidmode only for "drm" dri platforms

[Dylan Baker]

Quoting Emil Velikov (2018-12-11 09:22:03)
> From: Emil Velikov 
> 
> Currently we have the three dri "platforms" - drm, apple and windows.
> 
> Since xf86vidmode is a thing only for the drm one, adjust the
> preprocessor guards and correctly check for the dependency.
> 
> v2: terminate the GLX_USE_WINDOWSGL hunk
> 
> Cc: Jon TURNEY 
> Cc: Dylan Baker 
> Cc: Eric Engestrom 
> Fixes: 5bc509363b6 ("glx: make xf86vidmode mandatory for direct rendering")
> Signed-off-by: Emil Velikov 
> ---
> This time it's actually tested ;-)
> ---
>  configure.ac  | 4 ++--
>  meson.build   | 4 ++--
>  src/glx/glxcmds.c | 6 --
>  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 5d3da4b7c48..9b437a252cc 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1716,6 +1716,8 @@ xdri)
>  if test x"$enable_dri" = xyes; then
> dri_modules="$dri_modules xcb-dri2 >= $XCBDRI2_REQUIRED"
>  fi
> +
> +dri_modules="$dri_modules xxf86vm"
>  fi
>  if test x"$dri_platform" = xapple ; then
>  DEFINES="$DEFINES -DGLX_USE_APPLEGL"
> @@ -1725,8 +1727,6 @@ xdri)
>  fi
>  fi
>  
> -dri_modules="$dri_modules xxf86vm"
> -
>  PKG_CHECK_MODULES([DRIGL], [$dri_modules])
>  GL_PC_REQ_PRIV="$GL_PC_REQ_PRIV $dri_modules"
>  X11_INCLUDES="$X11_INCLUDES $DRIGL_CFLAGS"
> diff --git a/meson.build b/meson.build
> index 4f6176b8d96..fe647f682c1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1349,7 +1349,6 @@ if with_platform_x11
>  dep_xdamage = dependency('xdamage', version : '>= 1.1')
>  dep_xfixes = dependency('xfixes')
>  dep_xcb_glx = dependency('xcb-glx', version : '>= 1.8.1')
> -dep_xxf86vm = dependency('xxf86vm')
>endif
>if (with_any_vk or with_glx == 'dri' or
> (with_gallium_vdpau or with_gallium_xvmc or with_gallium_va or
> @@ -1376,6 +1375,7 @@ if with_platform_x11
>if with_glx == 'dri'
>  if with_dri_platform == 'drm'
>dep_dri2proto = dependency('dri2proto', version : '>= 2.8')
> +  dep_xxf86vm = dependency('xxf86vm')
>  endif
>  dep_glproto = dependency('glproto', version : '>= 1.4.14')
>endif
> @@ -1426,8 +1426,8 @@ elif with_glx == 'dri'
>  'xcb-glx >= 1.8.1']
>if with_dri_platform == 'drm'
>  gl_priv_reqs += 'xcb-dri2 >= 1.8'
> +gl_priv_reqs += 'xxf86vm'
>endif
> -  gl_priv_reqs += 'xxf86vm'

I'd really like to move to meson 0.46 so meson would just figure all of this out
for us... sigh

for the meson bits:
Reviewed-by: Dylan Baker 

>  endif
>  if dep_libdrm.found()
>gl_priv_reqs += 'libdrm >= 2.4.75'
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index b940c8ebdbe..d00c0d4816d 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -45,8 +45,10 @@
>  #include "apple/apple_glx.h"
>  #include "util/debug.h"
>  #else
> +#ifndef GLX_USE_WINDOWSGL
>  #include 
>  #include 
> +#endif /* GLX_USE_WINDOWSGL */
>  #endif
>  #endif
>  
> @@ -2081,7 +2083,7 @@ __glXGetSyncValuesOML(Display * dpy, GLXDrawable 
> drawable,
> return False;
>  }
>  
> -#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
> +#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL) && 
> !defined(GLX_USE_WINDOWSGL)
>  _X_HIDDEN GLboolean
>  __glxGetMscRate(struct glx_screen *psc,
> int32_t * numerator, int32_t * denominator)
> @@ -2157,7 +2159,7 @@ _X_HIDDEN GLboolean
>  __glXGetMscRateOML(Display * dpy, GLXDrawable drawable,
> int32_t * numerator, int32_t * denominator)
>  {
> -#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
> +#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL) && 
> !defined(GLX_USE_WINDOWSGL)
> __GLXDRIdrawable *draw = GetGLXDRIDrawable(dpy, drawable);
>  
> if (draw == NULL)
> -- 
> 2.19.2
> 


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 mesa] meson/vdpau: add missing soversion

[Dylan Baker]

Thanks for fixing this,
Reviewed-by: Dylan Baker 

Quoting Eric Engestrom (2018-12-11 09:13:54)
> This mirrors what autotools does in 
> src/gallium/state_trackers/vdpau/Makefile.am
> and src/gallium/targets/vdpau/Makefile.am:
> 
>   VDPAU_MAJOR = 1
>   VDPAU_MINOR = 0
>   libvdpau_gallium_la_LDFLAGS = -version-number $(VDPAU_MAJOR):$(VDPAU_MINOR)
> 
> Reported-by: Igor Gnatenko 
> Fixes: 68076b87474e7959c161 "meson: build gallium vdpau state tracker"
> Signed-off-by: Eric Engestrom 
> ---
>  src/gallium/state_trackers/vdpau/meson.build | 9 -
>  src/gallium/targets/vdpau/meson.build| 3 ++-
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/state_trackers/vdpau/meson.build 
> b/src/gallium/state_trackers/vdpau/meson.build
> index 9678b79ef6c952a48e45..28b98ae5369fa5c5ccf2 100644
> --- a/src/gallium/state_trackers/vdpau/meson.build
> +++ b/src/gallium/state_trackers/vdpau/meson.build
> @@ -18,13 +18,20 @@
>  # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
> THE
>  # SOFTWARE.
>  
> +VDPAU_MAJOR = 1
> +VDPAU_MINOR = 0
> +
>  libvdpau_st = static_library(
>'vdpau_st',
>files(
>  'bitmap.c', 'decode.c', 'device.c', 'ftab.c', 'htab.c', 'mixer.c',
>  'output.c', 'preemption.c', 'presentation.c', 'query.c', 'surface.c',
>),
> -  c_args : [c_vis_args, '-DVER_MAJOR=1', '-DVER_MINOR=0'],
> +  c_args : [
> +c_vis_args,
> +'-DVER_MAJOR=@0@'.format(VDPAU_MAJOR),
> +'-DVER_MINOR=@0@'.format(VDPAU_MINOR),
> +  ],
>include_directories : [
>  inc_include, inc_src, inc_util, inc_gallium, inc_gallium_aux,
>],
> diff --git a/src/gallium/targets/vdpau/meson.build 
> b/src/gallium/targets/vdpau/meson.build
> index 0c09b2b811429b5688b8..cfc616edf3eac3dd1540 100644
> --- a/src/gallium/targets/vdpau/meson.build
> +++ b/src/gallium/targets/vdpau/meson.build
> @@ -54,6 +54,7 @@ libvdpau_gallium = shared_library(
>  dep_thread, driver_r300, driver_r600, driver_radeonsi, driver_nouveau, 
> driver_tegra,
>],
>link_depends : vdpau_link_depends,
> +  soversion : '@0@.@1@.0'.format(VDPAU_MAJOR, VDPAU_MINOR),
>  )
>  foreach d : [[with_gallium_r300, 'r300'],
>   [with_gallium_r600, 'r600'],
> @@ -61,7 +62,7 @@ foreach d : [[with_gallium_r300, 'r300'],
>   [with_gallium_tegra, 'tegra'],
>   [with_gallium_nouveau, 'nouveau']]
>if d[0]
> -vdpau_drivers += 'libvdpau_@0@.so.1.0.0'.format(d[1])
> +vdpau_drivers += 'libvdpau_@0@.so.@1@.@2@.0'.format(d[1], VDPAU_MAJOR, 
> VDPAU_MINOR)
>endif
>  endforeach
>  
> -- 
> Cheers,
>   Eric
> 


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] glx: mandate xf86vidmode only for "drm" dri platforms

[Emil Velikov]

From: Emil Velikov 

Currently we have the three dri "platforms" - drm, apple and windows.

Since xf86vidmode is a thing only for the drm one, adjust the
preprocessor guards and correctly check for the dependency.

v2: terminate the GLX_USE_WINDOWSGL hunk

Cc: Jon TURNEY 
Cc: Dylan Baker 
Cc: Eric Engestrom 
Fixes: 5bc509363b6 ("glx: make xf86vidmode mandatory for direct rendering")
Signed-off-by: Emil Velikov 
---
This time it's actually tested ;-)
---
 configure.ac  | 4 ++--
 meson.build   | 4 ++--
 src/glx/glxcmds.c | 6 --
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5d3da4b7c48..9b437a252cc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1716,6 +1716,8 @@ xdri)
 if test x"$enable_dri" = xyes; then
dri_modules="$dri_modules xcb-dri2 >= $XCBDRI2_REQUIRED"
 fi
+
+dri_modules="$dri_modules xxf86vm"
 fi
 if test x"$dri_platform" = xapple ; then
 DEFINES="$DEFINES -DGLX_USE_APPLEGL"
@@ -1725,8 +1727,6 @@ xdri)
 fi
 fi
 
-dri_modules="$dri_modules xxf86vm"
-
 PKG_CHECK_MODULES([DRIGL], [$dri_modules])
 GL_PC_REQ_PRIV="$GL_PC_REQ_PRIV $dri_modules"
 X11_INCLUDES="$X11_INCLUDES $DRIGL_CFLAGS"
diff --git a/meson.build b/meson.build
index 4f6176b8d96..fe647f682c1 100644
--- a/meson.build
+++ b/meson.build
@@ -1349,7 +1349,6 @@ if with_platform_x11
 dep_xdamage = dependency('xdamage', version : '>= 1.1')
 dep_xfixes = dependency('xfixes')
 dep_xcb_glx = dependency('xcb-glx', version : '>= 1.8.1')
-dep_xxf86vm = dependency('xxf86vm')
   endif
   if (with_any_vk or with_glx == 'dri' or
(with_gallium_vdpau or with_gallium_xvmc or with_gallium_va or
@@ -1376,6 +1375,7 @@ if with_platform_x11
   if with_glx == 'dri'
 if with_dri_platform == 'drm'
   dep_dri2proto = dependency('dri2proto', version : '>= 2.8')
+  dep_xxf86vm = dependency('xxf86vm')
 endif
 dep_glproto = dependency('glproto', version : '>= 1.4.14')
   endif
@@ -1426,8 +1426,8 @@ elif with_glx == 'dri'
 'xcb-glx >= 1.8.1']
   if with_dri_platform == 'drm'
 gl_priv_reqs += 'xcb-dri2 >= 1.8'
+gl_priv_reqs += 'xxf86vm'
   endif
-  gl_priv_reqs += 'xxf86vm'
 endif
 if dep_libdrm.found()
   gl_priv_reqs += 'libdrm >= 2.4.75'
diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
index b940c8ebdbe..d00c0d4816d 100644
--- a/src/glx/glxcmds.c
+++ b/src/glx/glxcmds.c
@@ -45,8 +45,10 @@
 #include "apple/apple_glx.h"
 #include "util/debug.h"
 #else
+#ifndef GLX_USE_WINDOWSGL
 #include 
 #include 
+#endif /* GLX_USE_WINDOWSGL */
 #endif
 #endif
 
@@ -2081,7 +2083,7 @@ __glXGetSyncValuesOML(Display * dpy, GLXDrawable drawable,
return False;
 }
 
-#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
+#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL) && 
!defined(GLX_USE_WINDOWSGL)
 _X_HIDDEN GLboolean
 __glxGetMscRate(struct glx_screen *psc,
int32_t * numerator, int32_t * denominator)
@@ -2157,7 +2159,7 @@ _X_HIDDEN GLboolean
 __glXGetMscRateOML(Display * dpy, GLXDrawable drawable,
int32_t * numerator, int32_t * denominator)
 {
-#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
+#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL) && 
!defined(GLX_USE_WINDOWSGL)
__GLXDRIdrawable *draw = GetGLXDRIDrawable(dpy, drawable);
 
if (draw == NULL)
-- 
2.19.2

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

Re: [Mesa-dev] [PATCH mesa] meson/vdpau: add missing soversion

[Emil Velikov]

On Tue, 11 Dec 2018 at 17:14, Eric Engestrom  wrote:
>
> This mirrors what autotools does in 
> src/gallium/state_trackers/vdpau/Makefile.am
> and src/gallium/targets/vdpau/Makefile.am:
>
>   VDPAU_MAJOR = 1
>   VDPAU_MINOR = 0
>   libvdpau_gallium_la_LDFLAGS = -version-number $(VDPAU_MAJOR):$(VDPAU_MINOR)
>
> Reported-by: Igor Gnatenko 
> Fixes: 68076b87474e7959c161 "meson: build gallium vdpau state tracker"
> Signed-off-by: Eric Engestrom 

Thanks Eric.
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] last call for autotools

[Emil Velikov]

On Mon, 10 Dec 2018 at 23:11, Dylan Baker  wrote:
>
> Meson 0.49.0 has been out for a couple of days now, and I'd like to make the
> final call for autotools. My patch is so massive that it's a huge pain to send
> to the list, the latest versions is here:
> https://gitlab.freedesktop.org/dbaker/mesa/commits/delete-autotools
>
Can you split this up a bit? I'm mostly conserved about a couple of things:
- we're loosing multiple testing permutations with Travis - there's no
meson equivalent
- needed/missing bits are impossible to spot as-is

Personally, I'd do something like:
 - move .travis autotools permutations to meson
 - docs changes
 - rm Makefile{.*,}.am autogen.sh and configure.ac
 - rm Makefile.sources
 - .gitignore

git reset and git checkout --patch should make the above split a 5 min job ;-)

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

[Mesa-dev] [PATCH mesa] meson/vdpau: add missing soversion

[Eric Engestrom]

This mirrors what autotools does in src/gallium/state_trackers/vdpau/Makefile.am
and src/gallium/targets/vdpau/Makefile.am:

  VDPAU_MAJOR = 1
  VDPAU_MINOR = 0
  libvdpau_gallium_la_LDFLAGS = -version-number $(VDPAU_MAJOR):$(VDPAU_MINOR)

Reported-by: Igor Gnatenko 
Fixes: 68076b87474e7959c161 "meson: build gallium vdpau state tracker"
Signed-off-by: Eric Engestrom 
---
 src/gallium/state_trackers/vdpau/meson.build | 9 -
 src/gallium/targets/vdpau/meson.build| 3 ++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/gallium/state_trackers/vdpau/meson.build 
b/src/gallium/state_trackers/vdpau/meson.build
index 9678b79ef6c952a48e45..28b98ae5369fa5c5ccf2 100644
--- a/src/gallium/state_trackers/vdpau/meson.build
+++ b/src/gallium/state_trackers/vdpau/meson.build
@@ -18,13 +18,20 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
+VDPAU_MAJOR = 1
+VDPAU_MINOR = 0
+
 libvdpau_st = static_library(
   'vdpau_st',
   files(
 'bitmap.c', 'decode.c', 'device.c', 'ftab.c', 'htab.c', 'mixer.c',
 'output.c', 'preemption.c', 'presentation.c', 'query.c', 'surface.c',
   ),
-  c_args : [c_vis_args, '-DVER_MAJOR=1', '-DVER_MINOR=0'],
+  c_args : [
+c_vis_args,
+'-DVER_MAJOR=@0@'.format(VDPAU_MAJOR),
+'-DVER_MINOR=@0@'.format(VDPAU_MINOR),
+  ],
   include_directories : [
 inc_include, inc_src, inc_util, inc_gallium, inc_gallium_aux,
   ],
diff --git a/src/gallium/targets/vdpau/meson.build 
b/src/gallium/targets/vdpau/meson.build
index 0c09b2b811429b5688b8..cfc616edf3eac3dd1540 100644
--- a/src/gallium/targets/vdpau/meson.build
+++ b/src/gallium/targets/vdpau/meson.build
@@ -54,6 +54,7 @@ libvdpau_gallium = shared_library(
 dep_thread, driver_r300, driver_r600, driver_radeonsi, driver_nouveau, 
driver_tegra,
   ],
   link_depends : vdpau_link_depends,
+  soversion : '@0@.@1@.0'.format(VDPAU_MAJOR, VDPAU_MINOR),
 )
 foreach d : [[with_gallium_r300, 'r300'],
  [with_gallium_r600, 'r600'],
@@ -61,7 +62,7 @@ foreach d : [[with_gallium_r300, 'r300'],
  [with_gallium_tegra, 'tegra'],
  [with_gallium_nouveau, 'nouveau']]
   if d[0]
-vdpau_drivers += 'libvdpau_@0@.so.1.0.0'.format(d[1])
+vdpau_drivers += 'libvdpau_@0@.so.@1@.@2@.0'.format(d[1], VDPAU_MAJOR, 
VDPAU_MINOR)
   endif
 endforeach
 
-- 
Cheers,
  Eric

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

Re: [Mesa-dev] [PATCH 41/59] intel/compiler: split is_partial_write() into two variants

[Pohjolainen, Topi]

On Fri, Dec 07, 2018 at 03:30:11PM +0200, Pohjolainen, Topi wrote:
> On Tue, Dec 04, 2018 at 08:17:05AM +0100, Iago Toral Quiroga wrote:
> > This function is used in two different scenarios that for 32-bit
> > instructions are the same, but for 16-bit instructions are not.
> > 
> > One scenario is that in which we are working at a SIMD8 register
> > level and we need to know if a register is fully defined or written.
> > This is useful, for example, in the context of liveness analysis or
> > register allocation, where we work with units of registers.
> > 
> > The other scenario is that in which we want to know if an instruction
> > is writing a full scalar component or just some subset of it. This is
> > useful, for example, in the context of some optimization passes
> > like copy propagation.
> > 
> > For 32-bit instructions (or larger), a SIMD8 dispatch will always write
> > at least a full SIMD8 register (32B) if the write is not partial. The
> > function is_partial_write() checks this to determine if we have a partial
> > write. However, when we deal with 16-bit instructions, that logic disables
> > some optimizations that should be safe. For example, a SIMD8 16-bit MOV will
> > only update half of a SIMD register, but it is still a complete write of the
> > variable for a SIMD8 dispatch, so we should not prevent copy propagation in
> > this scenario because we don't write all 32 bytes in the SIMD register
> > or because the write starts at offset 16B (wehere we pack components Y or
> > W of 16-bit vectors).
> > 
> > This is a problem for SIMD8 executions (VS, TCS, TES, GS) of 16-bit
> > instructions, which lose a number of optimizations because of this, most
> > important of which is copy-propagation.
> > 
> > This patch splits is_partial_write() into is_partial_reg_write(), which
> > represents the current is_partial_write(), useful for things like
> > liveness analysis, and is_partial_var_write(), which considers
> > the dispatch size to check if we are writing a full variable (rather
> > than a full register) to decide if the write is partial or not, which
> > is what we really want in many optimization passes.

I actually started wondering why would liveness analysis and register
coalescing need to treat the 16-bit SIMD8 case differently than optimizations.
In virtual register space nothing would read or write the unused second half
of the register in case of 16-bit type and SIMD8.
Real register allocation in turn should be orthogonal to how things are
allocated in virtual space. And I guess even there we wouldn't be interested
of packing two 16-bit typed SIMD8 variables into one and same hardware
register. It is SIMD16 where we get more pressure into register space anyway
and there the 16-bit typed variables occupy full registers. In other words,
if things fit in SIMD16, would we bother packing things more tightly in
SIMD8? Or even if SIMD8 was the only option, would we be interested packing
channels for two variables in one hw reg even then?

Jason, we discussed this a little in the spring time.

As a recap my approach shortly. Instead of ignoring the second half of
registers case by case I addressed it more generally:

- changed all the open coded checks to use helpers,
- added a padding bit into fs_reg telling about the unused space,
- change nir -> fs step to set that bit for 16-bit typed regs
- and finally changed the helpers to consider the padding bit.

Now, I'm fine doing this case by case the way it is done here. I'm just
wondering if the split is needed, i.e., considering in some cases 16-bit SIMD8
virtual registers as half written and in some cases just ignoring the fact.

> > 
> > Then the patch goes on and rewrites all uses of is_partial_write() to use
> > one or the other version. Specifically, we use is_partial_var_write()
> > in the following places: copy propagation, cmod propagation, common
> > subexpression elimination, saturate propagation and sel peephole.
> > 
> > Notice that the semantics of is_partial_var_write() exactly match the
> > current implementation of is_partial_write() for anything that is
> > 32-bit or larger, so no changes are expected for 32-bit instructions.
> > 
> > Tested against ~5000 tests involving 16-bit instructions in CTS produced
> > the following changes in instruction counts:
> > 
> > Patched  | Master|%|
> > 
> > SIMD8  |621,900  |706,721| -12.00% |
> > 
> > SIMD16 | 93,252  | 93,252|   0.00% |
> > 
> > 
> > As expected, the change only affects SIMD8 dispatches.
> 
> I like this. But I think I want to try and rebase my fp16 work on top to see
> if there are any differences in the final assembly between this and my
> "register padding" scheme.
> 
> > ---
> >  src/intel/compiler/brw_fs.cpp | 31 +++
> >  .../compiler/brw_fs_cm

Re: [Mesa-dev] [PATCH 04/11] st/mesa: Use Array._DrawVAO in st_atom_array.c.

[Roland Scheidegger]

Am 11.12.18 um 10:37 schrieb Mathias Fröhlich:
> 
> Hey,
> 
> On Tuesday, 11 December 2018 10:19:47 CET Erik Faye-Lund wrote:
>> On Mon, 2018-12-10 at 18:23 +0100, Mathias Fröhlich wrote:
>>> Hi Erik,
>>>
>>> Not sure if this is our problem as I think that I only saw simple
>>> bindings with a zero instance divisor while debugging supertux kart.
>>>
>>> But at least I think that this is a problem in virglrenderer. The
>>> glVertexBindingDivisor is per binding and not per vertex attribute
>>> in OpenGL.
>>> ... you probably want to solve that differently, but for now this
>>> should
>>> quick band aid to pinpoint the problem that we observe.
>>>
>>> Does the attached patch to virglrenderer fix our problem?
>>>
>>
>> It does! Thanks a lot :)
>>
>> I'll find out what the proper fix is, and submit a patch in your name!
>> Again, thanks a lot :)
> 
> You are welcome! And I don't need credits. Its a bit of a pity that the
> vertex element/buffer structure in gallium is different than it is in OpenGL.
> OTOH, does it match the way it is done in DirectX?
Yes indeed. It actually seems D3d10 (11/12) is the odd man out here,
since GL, Vulkan, and even Metal have it per vertex buffer. But having
it per attribute is a more powerful representation (since you can have
multiple attribs per buffer, but not the other way round). Not sure
off-hand if d3d9 could already do it. I suppose if you'd be interested
only in gl state tracker you could ignore that the value could
potentially be different for different attributes but the same buffer,
otherwise you'd have to duplicate the binding in theory, although I
believe that hitting such a condition will be extremely rare (it
probably doesn't make much sense that someone would organize the data in
such a strange way that you have both per-vertex and per-instance (or
per-instance data with separate rates) in the same buffer).

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

Re: [Mesa-dev] [PATCH] glx: mandate xf86vidmode only for "drm" dri platforms

[Eric Engestrom]

On Tuesday, 2018-12-11 16:25:55 +, Emil Velikov wrote:
> From: Emil Velikov 
> 
> Currently we have the three dri "platforms" - drm, apple and windows.
> 
> Since xf86vidmode is a thing only for the drm one, adjust the
> preprocessor guards and correctly check for the dependency.
> 
> Cc: Jon TURNEY 
> Cc: Dylan Baker 
> Cc: Eric Engestrom 
> Fixes: 5bc509363b6 ("glx: make xf86vidmode mandatory for direct rendering")
> Signed-off-by: Emil Velikov 
> ---
>  configure.ac  | 4 ++--
>  meson.build   | 4 ++--
>  src/glx/glxcmds.c | 5 +++--
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 5d3da4b7c48..9b437a252cc 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1716,6 +1716,8 @@ xdri)
>  if test x"$enable_dri" = xyes; then
> dri_modules="$dri_modules xcb-dri2 >= $XCBDRI2_REQUIRED"
>  fi
> +
> +dri_modules="$dri_modules xxf86vm"
>  fi
>  if test x"$dri_platform" = xapple ; then
>  DEFINES="$DEFINES -DGLX_USE_APPLEGL"
> @@ -1725,8 +1727,6 @@ xdri)
>  fi
>  fi
>  
> -dri_modules="$dri_modules xxf86vm"
> -
>  PKG_CHECK_MODULES([DRIGL], [$dri_modules])
>  GL_PC_REQ_PRIV="$GL_PC_REQ_PRIV $dri_modules"
>  X11_INCLUDES="$X11_INCLUDES $DRIGL_CFLAGS"
> diff --git a/meson.build b/meson.build
> index 4f6176b8d96..fe647f682c1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1349,7 +1349,6 @@ if with_platform_x11
>  dep_xdamage = dependency('xdamage', version : '>= 1.1')
>  dep_xfixes = dependency('xfixes')
>  dep_xcb_glx = dependency('xcb-glx', version : '>= 1.8.1')
> -dep_xxf86vm = dependency('xxf86vm')
>endif
>if (with_any_vk or with_glx == 'dri' or
> (with_gallium_vdpau or with_gallium_xvmc or with_gallium_va or
> @@ -1376,6 +1375,7 @@ if with_platform_x11
>if with_glx == 'dri'
>  if with_dri_platform == 'drm'
>dep_dri2proto = dependency('dri2proto', version : '>= 2.8')
> +  dep_xxf86vm = dependency('xxf86vm')
>  endif
>  dep_glproto = dependency('glproto', version : '>= 1.4.14')
>endif
> @@ -1426,8 +1426,8 @@ elif with_glx == 'dri'
>  'xcb-glx >= 1.8.1']
>if with_dri_platform == 'drm'
>  gl_priv_reqs += 'xcb-dri2 >= 1.8'
> +gl_priv_reqs += 'xxf86vm'
>endif
> -  gl_priv_reqs += 'xxf86vm'
>  endif
>  if dep_libdrm.found()
>gl_priv_reqs += 'libdrm >= 2.4.75'
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index b940c8ebdbe..6632af1e002 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -45,6 +45,7 @@
>  #include "apple/apple_glx.h"
>  #include "util/debug.h"
>  #else
> +#ifndef GLX_USE_WINDOWSGL
>  #include 
>  #include 

Missing +#endif

>  #endif
> @@ -2081,7 +2082,7 @@ __glXGetSyncValuesOML(Display * dpy, GLXDrawable 
> drawable,
> return False;
>  }
>  
> -#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
> +#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL) && 
> !defined(GLX_USE_WINDOWSGL)
>  _X_HIDDEN GLboolean
>  __glxGetMscRate(struct glx_screen *psc,
>   int32_t * numerator, int32_t * denominator)
> @@ -2157,7 +2158,7 @@ _X_HIDDEN GLboolean
>  __glXGetMscRateOML(Display * dpy, GLXDrawable drawable,
> int32_t * numerator, int32_t * denominator)
>  {
> -#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
> +#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL) && 
> !defined(GLX_USE_WINDOWSGL)
> __GLXDRIdrawable *draw = GetGLXDRIDrawable(dpy, drawable);
>  
> if (draw == NULL)
> -- 
> 2.19.2
> 
> ___
> 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] glx: mandate xf86vidmode only for "drm" dri platforms

[Emil Velikov]

From: Emil Velikov 

Currently we have the three dri "platforms" - drm, apple and windows.

Since xf86vidmode is a thing only for the drm one, adjust the
preprocessor guards and correctly check for the dependency.

Cc: Jon TURNEY 
Cc: Dylan Baker 
Cc: Eric Engestrom 
Fixes: 5bc509363b6 ("glx: make xf86vidmode mandatory for direct rendering")
Signed-off-by: Emil Velikov 
---
 configure.ac  | 4 ++--
 meson.build   | 4 ++--
 src/glx/glxcmds.c | 5 +++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5d3da4b7c48..9b437a252cc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1716,6 +1716,8 @@ xdri)
 if test x"$enable_dri" = xyes; then
dri_modules="$dri_modules xcb-dri2 >= $XCBDRI2_REQUIRED"
 fi
+
+dri_modules="$dri_modules xxf86vm"
 fi
 if test x"$dri_platform" = xapple ; then
 DEFINES="$DEFINES -DGLX_USE_APPLEGL"
@@ -1725,8 +1727,6 @@ xdri)
 fi
 fi
 
-dri_modules="$dri_modules xxf86vm"
-
 PKG_CHECK_MODULES([DRIGL], [$dri_modules])
 GL_PC_REQ_PRIV="$GL_PC_REQ_PRIV $dri_modules"
 X11_INCLUDES="$X11_INCLUDES $DRIGL_CFLAGS"
diff --git a/meson.build b/meson.build
index 4f6176b8d96..fe647f682c1 100644
--- a/meson.build
+++ b/meson.build
@@ -1349,7 +1349,6 @@ if with_platform_x11
 dep_xdamage = dependency('xdamage', version : '>= 1.1')
 dep_xfixes = dependency('xfixes')
 dep_xcb_glx = dependency('xcb-glx', version : '>= 1.8.1')
-dep_xxf86vm = dependency('xxf86vm')
   endif
   if (with_any_vk or with_glx == 'dri' or
(with_gallium_vdpau or with_gallium_xvmc or with_gallium_va or
@@ -1376,6 +1375,7 @@ if with_platform_x11
   if with_glx == 'dri'
 if with_dri_platform == 'drm'
   dep_dri2proto = dependency('dri2proto', version : '>= 2.8')
+  dep_xxf86vm = dependency('xxf86vm')
 endif
 dep_glproto = dependency('glproto', version : '>= 1.4.14')
   endif
@@ -1426,8 +1426,8 @@ elif with_glx == 'dri'
 'xcb-glx >= 1.8.1']
   if with_dri_platform == 'drm'
 gl_priv_reqs += 'xcb-dri2 >= 1.8'
+gl_priv_reqs += 'xxf86vm'
   endif
-  gl_priv_reqs += 'xxf86vm'
 endif
 if dep_libdrm.found()
   gl_priv_reqs += 'libdrm >= 2.4.75'
diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
index b940c8ebdbe..6632af1e002 100644
--- a/src/glx/glxcmds.c
+++ b/src/glx/glxcmds.c
@@ -45,6 +45,7 @@
 #include "apple/apple_glx.h"
 #include "util/debug.h"
 #else
+#ifndef GLX_USE_WINDOWSGL
 #include 
 #include 
 #endif
@@ -2081,7 +2082,7 @@ __glXGetSyncValuesOML(Display * dpy, GLXDrawable drawable,
return False;
 }
 
-#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
+#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL) && 
!defined(GLX_USE_WINDOWSGL)
 _X_HIDDEN GLboolean
 __glxGetMscRate(struct glx_screen *psc,
int32_t * numerator, int32_t * denominator)
@@ -2157,7 +2158,7 @@ _X_HIDDEN GLboolean
 __glXGetMscRateOML(Display * dpy, GLXDrawable drawable,
int32_t * numerator, int32_t * denominator)
 {
-#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
+#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL) && 
!defined(GLX_USE_WINDOWSGL)
__GLXDRIdrawable *draw = GetGLXDRIDrawable(dpy, drawable);
 
if (draw == NULL)
-- 
2.19.2

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

Re: [Mesa-dev] [PATCH 1/2] Revert "glx: make xf86vidmode mandatory for direct rendering"

[Emil Velikov]

Hi Jon,

On Tue, 11 Dec 2018 at 15:22, Jon Turney  wrote:
>
> This reverts commit 5bc509363b6dbc42af72668fe500b6aec988dbf0.
>
Right, I've forgot Cygwin has direct GLX. Not what exactly what we
usually consider direct, regardless.

A far simpler and more accurate is to add "&& !defined(__CYGWIN__)"
just after the Apple one.
I'll send a patch for that in a moment.

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

Re: [Mesa-dev] [PATCH] glapi: Fix DispatchSanity_test

[Kristian Høgsberg]

On Tue, Dec 11, 2018 at 3:33 AM Emil Velikov  wrote:
>
> On Mon, 10 Dec 2018 at 18:56, Kristian Høgsberg  wrote:
> >
> > On Mon, Dec 10, 2018 at 7:38 AM Emil Velikov  
> > wrote:
> > >
> > > On Fri, 7 Dec 2018 at 23:48, Kristian H. Kristensen  
> > > wrote:
> > > >
> > > > ---
> > > >  src/mapi/glapi/gen/EXT_multisampled_render_to_texture.xml | 2 +-
> > > >  src/mapi/glapi/gen/es_EXT.xml | 2 ++
> > > >  src/mapi/glapi/gen/gl_API.xml | 2 --
> > > >  src/mesa/main/tests/dispatch_sanity.cpp   | 3 +++
> > > >  4 files changed, 6 insertions(+), 3 deletions(-)
> > > >
> > > There's a bit more to it. I'm testing something up and will send a
> > > patch shortly.
> >
> > You could also just review this patch and tell me what's missing.
> > Either way, we need to fix the test suite, so let's close this soon.
> > I'll commit this end of today if I don't hear from you.
> >
> As you can see in v2 - tracking down exactly what works and is
> reasonable took quite a bit.
> Fwiw I've kept you as the patch author, although I feel that's not
> your main concern.

No, I'm not to worried about that, I just don't feel good about the
"let me rewrite your patch for you" style of review. But I appreciate
your help in this case, thanks.

Kristian

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

Re: [Mesa-dev] [PATCH] svga: Enable rendering of float/half-float

[Brian Paul]

On 12/10/2018 02:36 PM, Nick Kreeger wrote:
> In GLES2 - if extensions are present, float and half-float textures can
> be used for rendering. This change enables the svga driver to handle
> rendering these types.
> ---
>   src/gallium/drivers/svga/svga_screen.c | 6 --
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/drivers/svga/svga_screen.c 
> b/src/gallium/drivers/svga/svga_screen.c
> index 95dde8b..3dcfc6b 100644
> --- a/src/gallium/drivers/svga/svga_screen.c
> +++ b/src/gallium/drivers/svga/svga_screen.c
> @@ -351,6 +351,10 @@ svga_get_param(struct pipe_screen *screen, enum pipe_cap 
> param)
>  case PIPE_CAP_MAX_TEXTURE_GATHER_COMPONENTS:
> return sws->have_sm4_1 ? 1 : 0; /* only single-channel textures */
>   
> +   case PIPE_CAP_TEXTURE_FLOAT_LINEAR:
> +   case PIPE_CAP_TEXTURE_HALF_FLOAT_LINEAR:
> +  return 1;
> +
>  /* Unsupported features */
>  case PIPE_CAP_TEXTURE_MIRROR_CLAMP:
>  case PIPE_CAP_TEXTURE_MIRROR_CLAMP_TO_EDGE:
> @@ -427,8 +431,6 @@ svga_get_param(struct pipe_screen *screen, enum pipe_cap 
> param)
>  case PIPE_CAP_RESOURCE_FROM_USER_MEMORY:
>  case PIPE_CAP_DEVICE_RESET_STATUS_QUERY:
>  case PIPE_CAP_MAX_SHADER_PATCH_VARYINGS:
> -   case PIPE_CAP_TEXTURE_FLOAT_LINEAR:
> -   case PIPE_CAP_TEXTURE_HALF_FLOAT_LINEAR:
>  case PIPE_CAP_DEPTH_BOUNDS_TEST:
>  case PIPE_CAP_TGSI_TXQS:
>  case PIPE_CAP_SHAREABLE_SHADERS:
> 


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

Re: [Mesa-dev] [PATCH 2/2] glx: make xf86vidmode mandatory for direct rendering

[Jon Turney]

On 16/11/2018 15:12, Eric Engestrom wrote:

On Friday, 2018-11-16 13:59:01 +, Emil Velikov wrote:

From: Emil Velikov 

Currently we detect the module and if missing, the glXGetMsc* API is
effectively a stub, always returning false.

This is what effectively has been happening with our meson build :-(


Oops... you're right: s/HAVE_XF86VIDMODE/XF86VIDMODE/ typo which was never
caught because dep_xxf86vm was missing from libglx's dependencies :/




Reviewed-by: Eric Engestrom 
Fixes: a47c525f3281a2753180e "meson: build glx"



Thus users have no chance of using it - they cannot even distinguish
if the failure is due to a misconfigured build.

There's no reason for keeping xf86vidmode optional - it has been
available in all distributions for years.


If you have an actual year number on that it might be worth writing it
down in the commit message; not important if you don't.


Sadly, this turns out to be not quite true.

I've sent patches to revert and fix this the other way.

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

[Mesa-dev] [PATCH 2/2] Fix typo preventing xxf86vm being used in meson build

[Jon Turney]

Also add now needed xxf86vm to the dependencies of libGL

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

diff --git a/src/glx/meson.build b/src/glx/meson.build
index 1de35fca6bc..2507993ed52 100644
--- a/src/glx/meson.build
+++ b/src/glx/meson.build
@@ -137,7 +137,7 @@ gl_lib_cargs = [
 ]
 
 if dep_xxf86vm.found()
-  gl_lib_cargs += '-DHAVE_XF86VIDMODE'
+  gl_lib_cargs += '-DXF86VIDMODE'
 endif
 
 libglx = static_library(
@@ -166,7 +166,7 @@ if with_glx == 'dri'
 link_args : [ld_args_bsymbolic, ld_args_gc_sections, extra_ld_args_libgl],
 dependencies : [
   dep_libdrm, dep_dl, dep_m, dep_thread, dep_x11, dep_xcb_glx, dep_xcb,
-  dep_x11_xcb, dep_xcb_dri2, dep_xext, dep_xfixes, dep_xdamage,
+  dep_x11_xcb, dep_xcb_dri2, dep_xext, dep_xfixes, dep_xdamage, 
dep_xxf86vm,
   extra_deps_libgl,
 ],
 version : gl_lib_version,
-- 
2.17.0

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

[Mesa-dev] [PATCH 1/2] Revert "glx: make xf86vidmode mandatory for direct rendering"

[Jon Turney]

This reverts commit 5bc509363b6dbc42af72668fe500b6aec988dbf0.

Signed-off-by: Jon Turney 
---
 configure.ac| 12 +++-
 meson.build |  6 --
 src/glx/Makefile.am |  5 +
 src/glx/SConscript  |  5 -
 src/glx/glxcmds.c   |  7 ++-
 src/glx/meson.build |  6 +-
 6 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5d3da4b7c48..f42e36c0658 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1725,7 +1725,11 @@ xdri)
 fi
 fi
 
-dri_modules="$dri_modules xxf86vm"
+# add xf86vidmode if available
+PKG_CHECK_MODULES([XF86VIDMODE], [xxf86vm], HAVE_XF86VIDMODE=yes, 
HAVE_XF86VIDMODE=no)
+if test "$HAVE_XF86VIDMODE" = yes ; then
+dri_modules="$dri_modules xxf86vm"
+fi
 
 PKG_CHECK_MODULES([DRIGL], [$dri_modules])
 GL_PC_REQ_PRIV="$GL_PC_REQ_PRIV $dri_modules"
@@ -1738,6 +1742,10 @@ xdri)
 ;;
 esac
 
+# This is outside the case (above) so that it is invoked even for non-GLX
+# builds.
+AM_CONDITIONAL(HAVE_XF86VIDMODE, test "x$HAVE_XF86VIDMODE" = xyes)
+
 GLESv1_CM_LIB_DEPS="$LIBDRM_LIBS -lm $PTHREAD_LIBS $DLOPEN_LIBS"
 GLESv1_CM_PC_LIB_PRIV="-lm $PTHREAD_LIBS $DLOPEN_LIBS"
 GLESv2_LIB_DEPS="$LIBDRM_LIBS -lm $PTHREAD_LIBS $DLOPEN_LIBS"
@@ -1754,6 +1762,8 @@ AC_SUBST([GLESv1_CM_PC_LIB_PRIV])
 AC_SUBST([GLESv2_LIB_DEPS])
 AC_SUBST([GLESv2_PC_LIB_PRIV])
 
+AC_SUBST([HAVE_XF86VIDMODE])
+
 dnl
 dnl More GLX setup
 dnl
diff --git a/meson.build b/meson.build
index 4f6176b8d96..c21b7b826e0 100644
--- a/meson.build
+++ b/meson.build
@@ -1349,7 +1349,7 @@ if with_platform_x11
 dep_xdamage = dependency('xdamage', version : '>= 1.1')
 dep_xfixes = dependency('xfixes')
 dep_xcb_glx = dependency('xcb-glx', version : '>= 1.8.1')
-dep_xxf86vm = dependency('xxf86vm')
+dep_xxf86vm = dependency('xxf86vm', required : false)
   endif
   if (with_any_vk or with_glx == 'dri' or
(with_gallium_vdpau or with_gallium_xvmc or with_gallium_va or
@@ -1427,11 +1427,13 @@ elif with_glx == 'dri'
   if with_dri_platform == 'drm'
 gl_priv_reqs += 'xcb-dri2 >= 1.8'
   endif
-  gl_priv_reqs += 'xxf86vm'
 endif
 if dep_libdrm.found()
   gl_priv_reqs += 'libdrm >= 2.4.75'
 endif
+if dep_xxf86vm.found()
+  gl_priv_reqs += 'xxf86vm'
+endif
 
 gl_priv_libs = []
 if dep_thread.found()
diff --git a/src/glx/Makefile.am b/src/glx/Makefile.am
index a66957d609b..d208ce14bb7 100644
--- a/src/glx/Makefile.am
+++ b/src/glx/Makefile.am
@@ -24,6 +24,10 @@ SUBDIRS =
 
 EXTRA_DIST = SConscript meson.build
 
+if HAVE_XF86VIDMODE
+EXTRA_DEFINES_XF86VIDMODE = -DXF86VIDMODE
+endif
+
 AM_CFLAGS = \
-I$(top_srcdir)/include \
-I$(top_srcdir)/include/GL/internal \
@@ -34,6 +38,7 @@ AM_CFLAGS = \
-I$(top_builddir)/src/mapi/glapi \
-I$(top_srcdir)/src/mapi/glapi \
$(VISIBILITY_CFLAGS) \
+   $(EXTRA_DEFINES_XF86VIDMODE) \
-D_REENTRANT \
$(DEFINES) \
$(LIBDRM_CFLAGS) \
diff --git a/src/glx/SConscript b/src/glx/SConscript
index ce25a1faa84..7555fb0568c 100644
--- a/src/glx/SConscript
+++ b/src/glx/SConscript
@@ -35,7 +35,10 @@ env.Prepend(LIBS = [
 env.PkgUseModules('X11')
 env.PkgUseModules('XCB')
 env.PkgUseModules('DRM')
-env.PkgUseModules('XF86VIDMODE')
+
+if env['HAVE_XF86VIDMODE']:
+env.Append(CPPDEFINES = ['XF86VIDMODE'])
+env.PkgUseModules('XF86VIDMODE')
 
 sources = [
 'clientattrib.c',
diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
index b940c8ebdbe..3ed960fbf3c 100644
--- a/src/glx/glxcmds.c
+++ b/src/glx/glxcmds.c
@@ -46,9 +46,11 @@
 #include "util/debug.h"
 #else
 #include 
+#ifdef XF86VIDMODE
 #include 
 #endif
 #endif
+#endif
 
 #include 
 #include 
@@ -2086,6 +2088,7 @@ _X_HIDDEN GLboolean
 __glxGetMscRate(struct glx_screen *psc,
int32_t * numerator, int32_t * denominator)
 {
+#ifdef XF86VIDMODE
XF86VidModeModeLine mode_line;
int dot_clock;
int i;
@@ -2132,6 +2135,8 @@ __glxGetMscRate(struct glx_screen *psc,
 
   return True;
}
+   else
+#endif
 
return False;
 }
@@ -2157,7 +2162,7 @@ _X_HIDDEN GLboolean
 __glXGetMscRateOML(Display * dpy, GLXDrawable drawable,
int32_t * numerator, int32_t * denominator)
 {
-#if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
+#if defined( GLX_DIRECT_RENDERING ) && defined( XF86VIDMODE )
__GLXDRIdrawable *draw = GetGLXDRIDrawable(dpy, drawable);
 
if (draw == NULL)
diff --git a/src/glx/meson.build b/src/glx/meson.build
index 3fd74439b11..1de35fca6bc 100644
--- a/src/glx/meson.build
+++ b/src/glx/meson.build
@@ -136,6 +136,10 @@ gl_lib_cargs = [
   '-D_REENTRANT',
 ]
 
+if dep_xxf86vm.found()
+  gl_lib_cargs += '-DHAVE_XF86VIDMODE'
+endif
+
 libglx = static_library(
   'glx',
   [files_libglx, glx_generated],
@@ -162,7 +166,7 @@ if with_glx == 'dri'
 link_args : [ld_args_bsymbolic, ld_args_gc_sections, extra_ld_args_libgl],
 dependencies : [
   dep_libdrm, dep_dl, dep_m, dep_thread, dep_x11, dep_xcb_glx, dep

[Mesa-dev] [PATCH 0/2] Restore optional xf86vidmode

[Jon Turney]

Restore optional xf86vidmode, and fix the meson build to actually use it.

Jon Turney (2):
  Revert "glx: make xf86vidmode mandatory for direct rendering"
  Fix typo preventing xxf86vm being used in meson build

 configure.ac| 12 +++-
 meson.build |  6 --
 src/glx/Makefile.am |  5 +
 src/glx/SConscript  |  5 -
 src/glx/glxcmds.c   |  7 ++-
 src/glx/meson.build |  4 
 6 files changed, 34 insertions(+), 5 deletions(-)

-- 
2.17.0

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

Re: [Mesa-dev] [PATCH] radv: adjust the VGT workaround for prim restart on GFX9

[Samuel Pitoiset]

ping?

After looking into this again today, I can't find any better solutions. 
We should probably push this patch because at least two games are 
affected. My opinion is that correctness is more important than performance.


On 10/11/18 10:42 AM, Samuel Pitoiset wrote:

WD_SWITCH_ON_EOP seems to be the only workaround that fixes
the GPU hangs with Yakuza and The Evil Within on Vega. I don't
like as it might decrease geometry performance as pointed out
by Marek, but I don't know how to implement a better one.

Cc: mesa-sta...@lists.freedesktop.org
Signed-off-by: Samuel Pitoiset 
---
  src/amd/vulkan/radv_pipeline.c | 13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index 426b417e172..2256b2c58e9 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -3412,14 +3412,23 @@ radv_compute_ia_multi_vgt_param_helpers(struct 
radv_pipeline *pipeline,
}
  
  	/* Workaround for a VGT hang when strip primitive types are used with

-* primitive restart.
+* primitive restart. This fixes GPU hangs with Yakuza and The Evil
+* Within, at least. Not sure if we can implement a better workaround.
 */
if (pipeline->graphics.prim_restart_enable &&
(prim == V_008958_DI_PT_LINESTRIP ||
 prim == V_008958_DI_PT_TRISTRIP ||
 prim == V_008958_DI_PT_LINESTRIP_ADJ ||
 prim == V_008958_DI_PT_TRISTRIP_ADJ)) {
-   ia_multi_vgt_param.partial_vs_wave = true;
+   if (device->physical_device->rad_info.chip_class >= GFX9) {
+   /* XXX: This might decrease geometry performance by 2x,
+* but this appears to be the only workaround that
+* fixes the GPU hang.
+*/
+   ia_multi_vgt_param.wd_switch_on_eop = true;
+   } else {
+   ia_multi_vgt_param.partial_vs_wave = true;
+   }
}
  
  	ia_multi_vgt_param.base =



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