Re: [Mesa-dev] [PATCH 04/22] nir/opcodes: Make unpack_half_2x16_split_* variable-width

[Kenneth Graunke]

On Friday, August 17, 2018 1:06:10 PM PDT Jason Ekstrand wrote:
> There is nothing inherent about these opcodes that requires them to only
> take scalars.  It's very convenient if we let them take vectors as well.
> ---
>  src/compiler/nir/nir_opcodes.py | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py
> index ed8e0ae9f39..4ef4ecc6f22 100644
> --- a/src/compiler/nir/nir_opcodes.py
> +++ b/src/compiler/nir/nir_opcodes.py
> @@ -304,10 +304,10 @@ unop_horiz("unpack_32_2x16", 2, tuint16, 1, tuint32,
>  # Lowered floating point unpacking operations.
>  
>  
> -unop_horiz("unpack_half_2x16_split_x", 1, tfloat32, 1, tuint32,
> -   "unpack_half_1x16((uint16_t)(src0.x & 0x))")
> -unop_horiz("unpack_half_2x16_split_y", 1, tfloat32, 1, tuint32,
> -   "unpack_half_1x16((uint16_t)(src0.x >> 16))")
> +unop_convert("unpack_half_2x16_split_x", tfloat32, tuint32,
> + "unpack_half_1x16((uint16_t)(src0 & 0x))")
> +unop_convert("unpack_half_2x16_split_y", tfloat32, tuint32,
> + "unpack_half_1x16((uint16_t)(src0 >> 16))")
>  
>  unop_convert("unpack_32_2x16_split_x", tuint16, tuint32, "src0")
>  unop_convert("unpack_32_2x16_split_y", tuint16, tuint32, "src0 >> 16")
> 

Reviewed-by: Kenneth Graunke 


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

Re: [Mesa-dev] [PATCH 14/22] intel/compiler: Do image load/store lowering to NIR

[Kenneth Graunke]

On Friday, August 17, 2018 1:06:20 PM PDT Jason Ekstrand wrote:
[snip]
> +# Intel-specific query for loading from the brw_image_param struct passed
> +# into the shader as a uniform.  The variable is a deref to the image
> +# variable. The const index specifies which of the six parameters to load.

This might be our first driver-specific intrinsics.  Some people make
big extensibility systems for that, where drivers can extend with their
own concepts.  But given that we all live in the same project, I think
this makes a lot of sense - just have everybody add their own here,
suffixed with a name they own (i.e. intel, amd, radv, ir3, whatever).

It's certainly nice and simple.

> +intrinsic("image_deref_load_param_intel", src_comp=[1], dest_comp=0,
> +  indices=[BASE], flags=[CAN_ELIMINATE, CAN_REORDER])
> +intrinsic("image_deref_load_raw_intel", src_comp=[1, 1], dest_comp=0,
> +  flags=[CAN_ELIMINATE, CAN_REORDER])
> +intrinsic("image_deref_store_raw_intel", src_comp=[1, 1, 0])

I don't think you want CAN_REORDER for the new load intrinsic...at
least, image_deref_load only has CAN_ELIMINATE.  It probably makes
sense for the two to match...

[snip]
> +static nir_ssa_def *
> +image_address(nir_builder *b, const struct gen_device_info *devinfo,
> +  nir_deref_instr *deref, nir_ssa_def *coord)
> +{
> +   coord = sanitize_image_coord(b, deref, coord);
> +
> +   nir_ssa_def *offset = load_image_param(b, deref, OFFSET);
> +   nir_ssa_def *tiling = load_image_param(b, deref, TILING);
> +   nir_ssa_def *stride = load_image_param(b, deref, STRIDE);
> +
> +   /* Shift the coordinates by the fixed surface offset.  It may be non-zero
> +* if the image is a single slice of a higher-dimensional surface, or if a
> +* non-zero mipmap level of the surface is bound to the pipeline.  The
> +* offset needs to be applied here rather than at surface state set-up 
> time
> +* because the desired slice-level may start mid-tile, so simply shifting
> +* the surface base address wouldn't give a well-formed tiled surface in
> +* the general case.
> +*/
> +   nir_ssa_def *xypos = (coord->num_components == 1) ?
> +nir_vec2(b, coord, nir_imm_int(b, 0)) :
> +nir_channels(b, coord, 0x3);
> +   xypos = nir_iadd(b, xypos, offset);
> +
> +   /* The layout of 3-D textures in memory is sort-of like a tiling
> +* format.  At each miplevel, the slices are arranged in rows of
> +* 2^level slices per row.  The slice row is stored in tmp.y and
> +* the slice within the row is stored in tmp.x.
> +*
> +* The layout of 2-D array textures and cubemaps is much simpler:
> +* Depending on whether the ARYSPC_LOD0 layout is in use it will be
> +* stored in memory as an array of slices, each one being a 2-D
> +* arrangement of miplevels, or as a 2D arrangement of miplevels,
> +* each one being an array of slices.  In either case the separation
> +* between slices of the same LOD is equal to the qpitch value
> +* provided as stride.w.
> +*
> +* This code can be made to handle either 2D arrays and 3D textures
> +* by passing in the miplevel as tile.z for 3-D textures and 0 in
> +* tile.z for 2-D array textures.
> +*
> +* See Volume 1 Part 1 of the Gen7 PRM, sections 6.18.4.7 "Surface
> +* Arrays" and 6.18.6 "3D Surfaces" for a more extensive discussion
> +* of the hardware 3D texture and 2D array layouts.
> +*/
> +   if (coord->num_components > 2) {
> +  /* Decompose z into a major (tmp.y) and a minor (tmp.x)
> +   * index.
> +   */
> +  nir_ssa_def *z = nir_channel(b, coord, 2);
> +  nir_ssa_def *z_x = nir_ubfe(b, z, nir_imm_int(b, 0),
> +  nir_channel(b, tiling, 2));
> +  nir_ssa_def *z_y = nir_ushr(b, z, nir_channel(b, tiling, 2));
> +
> +  /* Take into account the horizontal (tmp.x) and vertical (tmp.y)
> +   * slice offset.
> +   */
> +  xypos = nir_iadd(b, xypos, nir_imul(b, nir_vec2(b, z_x, z_y),
> + nir_channels(b, stride, 0xc)));
> +   }
> +
> +   nir_ssa_def *addr;
> +   if (coord->num_components > 1) {
> +  /* Calculate the major/minor x and y indices.  In order to
> +   * accommodate both X and Y tiling, the Y-major tiling format is
> +   * treated as being a bunch of narrow X-tiles placed next to each
> +   * other.  This means that the tile width for Y-tiling is actually
> +   * the width of one sub-column of the Y-major tile where each 4K
> +   * tile has 8 512B sub-columns.
> +   *
> +   * The major Y value is the row of tiles in which the pixel lives.
> +   * The major X value is the tile sub-column in which the pixel
> +   * lives; for X tiling, this is the same as the tile column, for Y
> +   * tiling, each tile has 8 sub-columns.  The minor X and Y indices
> +   * are the position within the 

[Mesa-dev] [PATCH 2/2] st/mesa, gallium: add a workaround for No Mans Sky

[Timothy Arceri]

The spec seems clear this is not allowed but the Nvidia binary
forces apps to add layout qualifiers so this works around the
issue for No Mans Sky until the CTS can be sorted out.
---
 src/gallium/auxiliary/pipe-loader/driinfo_gallium.h | 1 +
 src/gallium/include/state_tracker/st_api.h  | 1 +
 src/gallium/state_trackers/dri/dri_screen.c | 2 ++
 src/mesa/state_tracker/st_extensions.c  | 3 +++
 src/util/00-mesa-defaults.conf  | 1 +
 src/util/xmlpool/t_options.h| 5 +
 6 files changed, 13 insertions(+)

diff --git a/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h 
b/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
index b8f0fe64098..5f4305d91d7 100644
--- a/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
+++ b/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
@@ -29,6 +29,7 @@ DRI_CONF_SECTION_DEBUG
DRI_CONF_ALLOW_HIGHER_COMPAT_VERSION("false")
DRI_CONF_FORCE_GLSL_ABS_SQRT("false")
DRI_CONF_GLSL_CORRECT_DERIVATIVES_AFTER_DISCARD("false")
+   DRI_CONF_ALLOW_GLSL_LAYOUT_QUALIFIER_ON_FUNCTION_PARAMETERS("false")
 DRI_CONF_SECTION_END
 
 DRI_CONF_SECTION_MISCELLANEOUS
diff --git a/src/gallium/include/state_tracker/st_api.h 
b/src/gallium/include/state_tracker/st_api.h
index 8d386a82a63..61152e35468 100644
--- a/src/gallium/include/state_tracker/st_api.h
+++ b/src/gallium/include/state_tracker/st_api.h
@@ -228,6 +228,7 @@ struct st_config_options
boolean glsl_zero_init;
boolean force_glsl_abs_sqrt;
boolean allow_glsl_cross_stage_interpolation_mismatch;
+   boolean allow_glsl_layout_qualifier_on_function_parameters;
unsigned char config_options_sha1[20];
 };
 
diff --git a/src/gallium/state_trackers/dri/dri_screen.c 
b/src/gallium/state_trackers/dri/dri_screen.c
index 3e4de59a433..027e85024f0 100644
--- a/src/gallium/state_trackers/dri/dri_screen.c
+++ b/src/gallium/state_trackers/dri/dri_screen.c
@@ -85,6 +85,8 @@ dri_fill_st_options(struct dri_screen *screen)
   driQueryOptionb(optionCache, "force_glsl_abs_sqrt");
options->allow_glsl_cross_stage_interpolation_mismatch =
   driQueryOptionb(optionCache, 
"allow_glsl_cross_stage_interpolation_mismatch");
+   options->allow_glsl_layout_qualifier_on_function_parameters =
+  driQueryOptionb(optionCache, 
"allow_glsl_layout_qualifier_on_function_parameters");
 
driComputeOptionsSha1(optionCache, options->config_options_sha1);
 }
diff --git a/src/mesa/state_tracker/st_extensions.c 
b/src/mesa/state_tracker/st_extensions.c
index 8483f7a2a72..29a32513085 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -1255,6 +1255,9 @@ void st_init_extensions(struct pipe_screen *screen,
if (options->allow_glsl_relaxed_es)
   consts->AllowGLSLRelaxedES = GL_TRUE;
 
+   if (options->allow_glsl_layout_qualifier_on_function_parameters)
+  consts->AllowLayoutQualifiersOnFunctionParameters = GL_TRUE;
+
consts->MinMapBufferAlignment =
   screen->get_param(screen, PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT);
 
diff --git a/src/util/00-mesa-defaults.conf b/src/util/00-mesa-defaults.conf
index ad59efba50b..eb78b75e9b8 100644
--- a/src/util/00-mesa-defaults.conf
+++ b/src/util/00-mesa-defaults.conf
@@ -195,6 +195,7 @@ TODO: document the other workarounds.
 
 
 
+
 
 
 

[Mesa-dev] [PATCH 1/2] glsl: add a mechanism to allow layout qualifiers on function params

[Timothy Arceri]

The spec is quite clear this is not allowed:

From Section 4.4. (Layout Qualifiers) of the GLSL 4.60 spec:

   "Layout qualifiers can appear in several forms of declaration.
   They can appear as part of an interface block definition or
   block member, as shown in the grammar in the previous section.
   They can also appear with just an interface-qualifier to establish
   layouts of other declarations made with that qualifier:

  layout-qualifier interface-qualifier ;

   Or, they can appear with an individual variable declared with
   an interface qualifier:

  layout-qualifier interface-qualifier declaration ;"

From Section 4.10 (Memory Qualifiers) of the GLSL 4.60 spec:

   "Layout qualifiers cannot be used on formal function parameters,
   and layout qualification is not included in parameter matching."

However on the Nvidia binary driver they actually fail to compile
if image function params don't have a layout qualifier. This results
in applications such as No Mans Sky using layout qualifiers on params.

I've submitted a CTS test to expose this problem in the Nvidia driver
but until that is resolved this patch will help Mesa drivers work
around the issue.
---
 src/compiler/glsl/glsl_parser.yy | 17 +
 src/compiler/glsl/glsl_parser_extras.cpp |  2 ++
 src/compiler/glsl/glsl_parser_extras.h   |  1 +
 src/mesa/main/mtypes.h   |  5 +
 4 files changed, 25 insertions(+)

diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
index bc2571b6844..fd1592beca0 100644
--- a/src/compiler/glsl/glsl_parser.yy
+++ b/src/compiler/glsl/glsl_parser.yy
@@ -897,6 +897,23 @@ parameter_declarator:
   $$->identifier = $2;
   state->symbols->add_variable(new(state) ir_variable(NULL, $2, 
ir_var_auto));
}
+   | layout_qualifier type_specifier any_identifier
+   {
+  if (state->allow_layout_qualifier_on_function_parameter) {
+ void *ctx = state->linalloc;
+ $$ = new(ctx) ast_parameter_declarator();
+ $$->set_location_range(@2, @3);
+ $$->type = new(ctx) ast_fully_specified_type();
+ $$->type->set_location(@2);
+ $$->type->specifier = $2;
+ $$->identifier = $3;
+ state->symbols->add_variable(new(state) ir_variable(NULL, $3, 
ir_var_auto));
+  } else {
+ _mesa_glsl_error(&@1, state,
+  "is is not allowed on function parameter");
+ YYERROR;
+  }
+   }
| type_specifier any_identifier array_specifier
{
   void *ctx = state->linalloc;
diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
b/src/compiler/glsl/glsl_parser_extras.cpp
index 0a7d0d78b14..efd1a013dbd 100644
--- a/src/compiler/glsl/glsl_parser_extras.cpp
+++ b/src/compiler/glsl/glsl_parser_extras.cpp
@@ -311,6 +311,8 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct 
gl_context *_ctx,
   ctx->Const.AllowGLSLExtensionDirectiveMidShader;
this->allow_builtin_variable_redeclaration =
   ctx->Const.AllowGLSLBuiltinVariableRedeclaration;
+   this->allow_layout_qualifier_on_function_parameter =
+  ctx->Const.AllowLayoutQualifiersOnFunctionParameters;
 
this->cs_input_local_size_variable_specified = false;
 
diff --git a/src/compiler/glsl/glsl_parser_extras.h 
b/src/compiler/glsl/glsl_parser_extras.h
index 2c8353214aa..69aa6cf9cf3 100644
--- a/src/compiler/glsl/glsl_parser_extras.h
+++ b/src/compiler/glsl/glsl_parser_extras.h
@@ -866,6 +866,7 @@ struct _mesa_glsl_parse_state {
 
bool allow_extension_directive_midshader;
bool allow_builtin_variable_redeclaration;
+   bool allow_layout_qualifier_on_function_parameter;
 
/**
 * Known subroutine type declarations.
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 9d058cef6d9..1f640b063c0 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -3764,6 +3764,11 @@ struct gl_constants
 */
GLboolean AllowHigherCompatVersion;
 
+   /**
+* Allow layout qualifiers on function parameters.
+*/
+   GLboolean AllowLayoutQualifiersOnFunctionParameters;
+
/**
 * Force computing the absolute value for sqrt() and inversesqrt() to follow
 * D3D9 when apps rely on this behaviour.
-- 
2.17.1

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

[Mesa-dev] [PATCH] ac/radeonsi: fix CIK copy max size

[Dave Airlie]

From: Dave Airlie 

While adding transfer queues to radv, I started writing some tests,
the first test I wrote fell over copying a buffer larger than this
limit.

Checked AMDVLK and found the correct limit.

Cc: 
---
 src/amd/common/sid.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/amd/common/sid.h b/src/amd/common/sid.h
index 0671f7d3998..edb7d06afa6 100644
--- a/src/amd/common/sid.h
+++ b/src/amd/common/sid.h
@@ -9139,7 +9139,9 @@
 #defineCIK_SDMA_PACKET_SEMAPHORE   0x7
 #defineCIK_SDMA_PACKET_CONSTANT_FILL   0xb
 #defineCIK_SDMA_PACKET_SRBM_WRITE  0xe
-#defineCIK_SDMA_COPY_MAX_SIZE  0x3fffe0
+/* There is apparently an undocumented HW "feature" that
+   prevents the HW from copying past 256 bytes of (1 << 22) */
+#defineCIK_SDMA_COPY_MAX_SIZE  0x3fff00
 
 enum amd_cmp_class_flags {
S_NAN = 1 << 0,// Signaling NaN
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH 3/3] i965/screen: Allow modifiers on sRGB formats

[Jason Ekstrand]

On Tue, Aug 28, 2018 at 5:22 PM Jason Ekstrand  wrote:

> This effectively reverts a26693493570a9d0f0fba1be617e01ee7bfff4db which
> was a misguided attempt at protecting intel_query_dma_buf_modifiers from
> invalid formats.  Unfortunately, in some internal EGL cases, we can get
> an SRGB format validly in this function.  Rejecting such formats caused
> us to not allow CCS in some cases where we should have been allowing it.
>
> There's some question of whether or not we really should be using SRGB
> "fourcc" formats that aren't actually in drm_foucc.h but there's not
> much harm in allowing them through here.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107223
> Fixes: a26693493570 "i965/screen: Return false for unsupported..."
> ---
>  src/mesa/drivers/dri/i965/intel_screen.c | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index eaf5a3b9feb..ac1938f6204 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1274,9 +1274,9 @@ static bool
>  intel_image_format_is_supported(const struct gen_device_info *devinfo,
>  const struct intel_image_format *fmt)
>  {
> -   if (fmt->fourcc == __DRI_IMAGE_FOURCC_SARGB ||
> -   fmt->fourcc == __DRI_IMAGE_FOURCC_SABGR)
> -  return false;
> +   /* Currently, all formats with an intel_image_format are available on
> all
> +* platforms so there's really nothing to check there.
> +*/
>
>  #ifndef NDEBUG
> if (fmt->nplanes == 1) {
> @@ -1302,6 +1302,14 @@ intel_query_dma_buf_formats(__DRIscreen *_screen,
> int max,
> int num_formats = 0, i;
>
> for (i = 0; i < ARRAY_SIZE(intel_image_formats); i++) {
> +  /* These two formats are valid DRI formats but do not exist in
> +   * drm_fourcc.h in the Linux kernel.  We don't want to accidentally
> +   * advertise them through the EGL layer.
> +   */
> +  if (intel_image_formats[i].fourcc == __DRI_IMAGE_FOURCC_SARGB ||
> +  intel_image_formats[i].fourcc == __DRI_IMAGE_FOURCC_SABGR)
> + return false;
>

This should be a continue.  Fixed locally.


> +
>if (!intel_image_format_is_supported(>devinfo,
> _image_formats[i]))
>   continue;
> --
> 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: enable ARB_direct_state_access in compat for GL3.1+

[Timothy Arceri]

We could enable it for lower versions of GL but this allows us
to just use the existing version/extension checks that are already
used by the core profile.

v2: fix potential crash in no error path
---
 src/mapi/glapi/gen/apiexec.py| 194 +++
 src/mesa/main/arrayobj.c |   9 ++
 src/mesa/main/extensions_table.h |   2 +-
 src/mesa/main/fbobject.c |  13 ++-
 4 files changed, 114 insertions(+), 104 deletions(-)

diff --git a/src/mapi/glapi/gen/apiexec.py b/src/mapi/glapi/gen/apiexec.py
index b163d88549b..e2fc124be22 100644
--- a/src/mapi/glapi/gen/apiexec.py
+++ b/src/mapi/glapi/gen/apiexec.py
@@ -152,103 +152,103 @@ functions = {
 
 # OpenGL 4.5 / GL_ARB_direct_state_access.   Mesa can expose the extension
 # with core profile.
-"CreateTransformFeedbacks": exec_info(compatibility=45, core=31),
-"TransformFeedbackBufferBase": exec_info(compatibility=45, core=31),
-"TransformFeedbackBufferRange": exec_info(compatibility=45, core=31),
-"GetTransformFeedbackiv": exec_info(compatibility=45, core=31),
-"GetTransformFeedbacki_v": exec_info(compatibility=45, core=31),
-"GetTransformFeedbacki64_v": exec_info(compatibility=45, core=31),
-"CreateBuffers": exec_info(compatibility=45, core=31),
-"NamedBufferStorage": exec_info(compatibility=45, core=31),
-"NamedBufferData": exec_info(compatibility=45, core=31),
-"NamedBufferSubData": exec_info(compatibility=45, core=31),
-"CopyNamedBufferSubData": exec_info(compatibility=45, core=31),
-"ClearNamedBufferData": exec_info(compatibility=45, core=31),
-"ClearNamedBufferSubData": exec_info(compatibility=45, core=31),
-"MapNamedBuffer": exec_info(compatibility=45, core=31),
-"MapNamedBufferRange": exec_info(compatibility=45, core=31),
-"UnmapNamedBuffer": exec_info(compatibility=45, core=31),
-"FlushMappedNamedBufferRange": exec_info(compatibility=45, core=31),
-"GetNamedBufferParameteriv": exec_info(compatibility=45, core=31),
-"GetNamedBufferParameteri64v": exec_info(compatibility=45, core=31),
-"GetNamedBufferPointerv": exec_info(compatibility=45, core=31),
-"GetNamedBufferSubData": exec_info(compatibility=45, core=31),
-"CreateFramebuffers": exec_info(compatibility=45, core=31),
-"NamedFramebufferRenderbuffer": exec_info(compatibility=45, core=31),
-"NamedFramebufferParameteri": exec_info(compatibility=45, core=31),
-"NamedFramebufferTexture": exec_info(compatibility=45, core=31),
-"NamedFramebufferTextureLayer": exec_info(compatibility=45, core=31),
-"NamedFramebufferDrawBuffer": exec_info(compatibility=45, core=31),
-"NamedFramebufferDrawBuffers": exec_info(compatibility=45, core=31),
-"NamedFramebufferReadBuffer": exec_info(compatibility=45, core=31),
-"InvalidateNamedFramebufferData": exec_info(compatibility=45, core=31),
-"InvalidateNamedFramebufferSubData": exec_info(compatibility=45, core=31),
-"ClearNamedFramebufferiv": exec_info(compatibility=45, core=31),
-"ClearNamedFramebufferuiv": exec_info(compatibility=45, core=31),
-"ClearNamedFramebufferfv": exec_info(compatibility=45, core=31),
-"ClearNamedFramebufferfi": exec_info(compatibility=45, core=31),
-"BlitNamedFramebuffer": exec_info(compatibility=45, core=31),
-"CheckNamedFramebufferStatus": exec_info(compatibility=45, core=31),
-"GetNamedFramebufferParameteriv": exec_info(compatibility=45, core=31),
-"GetNamedFramebufferAttachmentParameteriv": exec_info(compatibility=45, 
core=31),
-"CreateRenderbuffers": exec_info(compatibility=45, core=31),
-"NamedRenderbufferStorage": exec_info(compatibility=45, core=31),
-"NamedRenderbufferStorageMultisample": exec_info(compatibility=45, 
core=31),
-"GetNamedRenderbufferParameteriv": exec_info(compatibility=45, core=31),
-"CreateTextures": exec_info(compatibility=45, core=31),
-"TextureBuffer": exec_info(compatibility=45, core=31),
-"TextureBufferRange": exec_info(compatibility=45, core=31),
-"TextureStorage1D": exec_info(compatibility=45, core=31),
-"TextureStorage2D": exec_info(compatibility=45, core=31),
-"TextureStorage3D": exec_info(compatibility=45, core=31),
-"TextureStorage2DMultisample": exec_info(compatibility=45, core=31),
-"TextureStorage3DMultisample": exec_info(compatibility=45, core=31),
-"TextureSubImage1D": exec_info(compatibility=45, core=31),
-"TextureSubImage2D": exec_info(compatibility=45, core=31),
-"TextureSubImage3D": exec_info(compatibility=45, core=31),
-"CompressedTextureSubImage1D": exec_info(compatibility=45, core=31),
-"CompressedTextureSubImage2D": exec_info(compatibility=45, core=31),
-"CompressedTextureSubImage3D": exec_info(compatibility=45, core=31),
-"CopyTextureSubImage1D": exec_info(compatibility=45, core=31),
-"CopyTextureSubImage2D": exec_info(compatibility=45, core=31),
-"CopyTextureSubImage3D": exec_info(compatibility=45, core=31),
-

[Mesa-dev] [PATCH] glsl: skip stringification in preprocessor if in unreachable branch

[Timothy Arceri]

This fixes compilation of some "No Mans Sky" shaders where the stringification
happens in branches intended for DX12.

---

 Piglit tests: https://patchwork.freedesktop.org/series/48850/

 src/compiler/glsl/glcpp/glcpp-lex.l | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/glcpp/glcpp-lex.l 
b/src/compiler/glsl/glcpp/glcpp-lex.l
index 9cfcc120222..fe5845acd4e 100644
--- a/src/compiler/glsl/glcpp/glcpp-lex.l
+++ b/src/compiler/glsl/glcpp/glcpp-lex.l
@@ -420,8 +420,10 @@ HEXADECIMAL_INTEGER0[xX][0-9a-fA-F]+[uU]?
 
/* This will catch any non-directive garbage after a HASH */
 {NONSPACE} {
-   BEGIN INITIAL;
-   RETURN_TOKEN (GARBAGE);
+   if (!parser->skipping) {
+   BEGIN INITIAL;
+   RETURN_TOKEN (GARBAGE);
+   }
 }
 
/* An identifier immediately followed by '(' */
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH] ac/surface: fix CMASK fast clear for NPOT textures with mipmapping on SI/CI/VI

[Marek Olšák]

BTW, radv doesn't use the function, so it will have this bug.

Marek

On Tue, Aug 28, 2018 at 5:47 PM, Bas Nieuwenhuizen
 wrote:
> Reviewed-by: Bas Nieuwenhuizen 
> On Tue, Aug 28, 2018 at 8:41 PM Marek Olšák  wrote:
>>
>> From: Marek Olšák 
>>
>> This fixes VM faults and corruption.
>>
>> Cc: 18.1 18.2 
>> ---
>>  src/amd/common/ac_surface.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/amd/common/ac_surface.c b/src/amd/common/ac_surface.c
>> index 2f4f0f8884f..94723dc9c09 100644
>> --- a/src/amd/common/ac_surface.c
>> +++ b/src/amd/common/ac_surface.c
>> @@ -581,22 +581,22 @@ void ac_compute_cmask(const struct radeon_info *info,
>> cl_width = 64;
>> cl_height = 64;
>> break;
>> default:
>> assert(0);
>> return;
>> }
>>
>> unsigned base_align = num_pipes * pipe_interleave_bytes;
>>
>> -   unsigned width = align(config->info.width, cl_width*8);
>> -   unsigned height = align(config->info.height, cl_height*8);
>> +   unsigned width = align(surf->u.legacy.level[0].nblk_x, cl_width*8);
>> +   unsigned height = align(surf->u.legacy.level[0].nblk_y, cl_height*8);
>> unsigned slice_elements = (width * height) / (8*8);
>>
>> /* Each element of CMASK is a nibble. */
>> unsigned slice_bytes = slice_elements / 2;
>>
>> surf->u.legacy.cmask_slice_tile_max = (width * height) / (128*128);
>> if (surf->u.legacy.cmask_slice_tile_max)
>> surf->u.legacy.cmask_slice_tile_max -= 1;
>>
>> unsigned num_layers;
>> --
>> 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 5/9] nir: Add a local dead write vars removal pass

[Jason Ekstrand]

On Mon, Aug 27, 2018 at 4:55 PM Caio Marcelo de Oliveira Filho <
caio.olive...@intel.com> wrote:

> (Disregard the incomplete mail, still adapting to notmuch-emacs).
>
> Jason Ekstrand  writes:
>
> >> +static nir_deref_path *
> >> +get_path(struct state *state, nir_deref_instr *deref)
> >> +{
> >> +   struct hash_entry *entry = _mesa_hash_table_search(state->paths,
> >> deref);
> >> +   if (!entry) {
> >> +  nir_deref_path *path = linear_zalloc_child(state->path_lin_ctx,
> >> sizeof(nir_deref_path));
> >> +  nir_deref_path_init(path, deref, state->mem_ctx);
> >> +  _mesa_hash_table_insert(state->paths, deref, path);
> >> +  return path;
> >> +   } else {
> >> +  return entry->data;
> >> +   }
> >> +}
> >>
> >
> > Do you have any proof that this actually helps?  The deref_path stuff was
> > designed to be put on the stack and absolutely as efficient as possible.
> > In the common case of a deref chain with only a couple of elements, I
> would
> > expect to actually be more work to look it up in a hash table.
>
> Storing those makes more sense if you read the next commit (the
> "global").  Since I've created the "local" commit as an aid for
> reviewing (perhaps a failure), I did not wanted to change it too much.
>
> When I wrote there were some savings when measuring executions with
> perf.
>
> (...)
>
> >> +static bool
> >> +remove_dead_write_vars_local(struct state *state, nir_block *block)
> >> +{
> >> +   bool progress = false;
> >> +
> >> +   struct util_dynarray unused_writes;
> >> +   util_dynarray_init(_writes, state->mem_ctx);
> >> +
> >> +   nir_foreach_instr_safe(instr, block) {
> >>
> >
> > It wouldn't hurt to add a case for call instructions which does a barrier
> > on everything I mentioned below as well as globals and locals.
>
> Makes sense.  But I don't get locals are affect?  Is this to cover the
> parameters being passed to the call?
>

Because a deref to a local might be passed in as a parameter.  This is the
way pass-by-reference works for SPIR-V.


> >
> >> +  if (instr->type != nir_instr_type_intrinsic)
> >> + continue;
> >> +
> >> +  nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr);
> >> +  switch (intrin->intrinsic) {
> >> +  case nir_intrinsic_barrier:
> >> +  case nir_intrinsic_memory_barrier: {
> >> + nir_variable_mode modes = ~(nir_var_local | nir_var_global |
> >> + nir_var_shader_in |
> nir_var_uniform);
> >>
> >
> > The only thing a barrier like this affects is shared, storage, and
> output.
> > Locals and globals can't cross between shader channels so there's no
> reason
> > to do anything with them on a barrier.  For inputs and uniforms, they're
> > never written anyway so there's no point in doing anything with them on a
> > barrier.
>
> This came from previous code, but except for "system values" it seems to
> do the right thing (note the ~).  Is the suggestion to use a "positive"
> enumeration, e.g.
>

Drp... I completely missed the ~.  Ignore my comment.  Yeah, we could throw
in system values. :D
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Dave Airlie]

From: Dave Airlie 

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

piglit/glsl-max-varyings test was causing this to happen, and
the renderer fix was breaking it.

v2: drop fprintf
---
 src/gallium/drivers/virgl/virgl_encode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/drivers/virgl/virgl_encode.c 
b/src/gallium/drivers/virgl/virgl_encode.c
index b56e1f5e428..ceb0c4c20d5 100644
--- a/src/gallium/drivers/virgl/virgl_encode.c
+++ b/src/gallium/drivers/virgl/virgl_encode.c
@@ -284,7 +284,7 @@ int virgl_encode_shader_state(struct virgl_context *ctx,
while (left_bytes) {
   uint32_t length, offlen;
   int hdr_len = base_hdr_size + (first_pass ? strm_hdr_size : 0);
-  if (ctx->cbuf->cdw + hdr_len + 1 > VIRGL_MAX_CMDBUF_DWORDS)
+  if (ctx->cbuf->cdw + hdr_len + 1 >= VIRGL_MAX_CMDBUF_DWORDS)
  ctx->base.flush(>base, NULL, 0);
 
   thispass = (VIRGL_MAX_CMDBUF_DWORDS - ctx->cbuf->cdw - hdr_len - 1) * 4;
-- 
2.14.3

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

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

[Dave Airlie]

From: Dave Airlie 

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

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

diff --git a/src/gallium/drivers/virgl/virgl_encode.c 
b/src/gallium/drivers/virgl/virgl_encode.c
index b56e1f5e428..c1082791af5 100644
--- a/src/gallium/drivers/virgl/virgl_encode.c
+++ b/src/gallium/drivers/virgl/virgl_encode.c
@@ -284,11 +284,12 @@ int virgl_encode_shader_state(struct virgl_context *ctx,
while (left_bytes) {
   uint32_t length, offlen;
   int hdr_len = base_hdr_size + (first_pass ? strm_hdr_size : 0);
-  if (ctx->cbuf->cdw + hdr_len + 1 > VIRGL_MAX_CMDBUF_DWORDS)
+  if (ctx->cbuf->cdw + hdr_len + 1 >= VIRGL_MAX_CMDBUF_DWORDS)
  ctx->base.flush(>base, NULL, 0);
 
   thispass = (VIRGL_MAX_CMDBUF_DWORDS - ctx->cbuf->cdw - hdr_len - 1) * 4;
 
+  fprintf(stderr, "this pass is %d %d %d\n", thispass, ctx->cbuf->cdw, 
hdr_len);
   length = MIN2(thispass, left_bytes);
   len = ((length + 3) / 4) + hdr_len;
 
-- 
2.14.3

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

[Mesa-dev] [PATCH V2] i965/icl: Set Enabled Texel Offset Precision Fix bit

[Anuj Phogat]

h/w specification requires this bit to be always set.

V2: Fix bit mask (Chris Wilson)

Suggested-by: Kenneth Graunke 
Signed-off-by: Anuj Phogat 
---
 src/mesa/drivers/dri/i965/brw_defines.h  | 4 
 src/mesa/drivers/dri/i965/brw_state_upload.c | 7 +++
 2 files changed, 11 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index 433314115b1..97a787a2ab3 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1673,6 +1673,10 @@ enum brw_pixel_shader_coverage_mask_mode {
 # define GLK_SCEC_BARRIER_MODE_3D_HULL (1 << 7)
 # define GLK_SCEC_BARRIER_MODE_MASKREG_MASK(1 << 7)
 
+#define HALF_SLICE_CHICKEN70xE194
+# define TEXEL_OFFSET_FIX_ENABLE   (1 << 1)
+# define TEXEL_OFFSET_FIX_MASK REG_MASK(1 << 1)
+
 #define GEN11_SAMPLER_MODE  0xE18C
 # define HEADERLESS_MESSAGE_FOR_PREEMPTABLE_CONTEXTS(1 << 5)
 # define HEADERLESS_MESSAGE_FOR_PREEMPTABLE_CONTEXTS_MASK   REG_MASK(1 << 5)
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c 
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index 2af4c45bc44..7f20579fb87 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -72,6 +72,13 @@ brw_upload_initial_gpu_state(struct brw_context *brw)
   brw_load_register_imm32(brw, GEN11_SAMPLER_MODE,
   HEADERLESS_MESSAGE_FOR_PREEMPTABLE_CONTEXTS_MASK 
|
   HEADERLESS_MESSAGE_FOR_PREEMPTABLE_CONTEXTS);
+
+  /* Bit 1 "Enabled Texel Offset Precision Fix" must be set in
+   * HALF_SLICE_CHICKEN7 register.
+   */
+  brw_load_register_imm32(brw, HALF_SLICE_CHICKEN7,
+  TEXEL_OFFSET_FIX_MASK |
+  TEXEL_OFFSET_FIX_ENABLE);
}
 
if (devinfo->gen == 10 || devinfo->gen == 11) {
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH 4/5 v2] i965/fs: Emit BRW_AOP_INC or BRW_AOP_DEC for imageAtomicAdd of +1 or -1

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand 

On Mon, Aug 27, 2018 at 3:55 PM Ian Romanick  wrote:

> From: Ian Romanick 
>
> v2: Refactor selection of atomic opcode to a separate function.
> Suggested by Jason.
>
> No changes on any other Intel platforms.
>
> Skylake
> total instructions in shared programs: 14304261 -> 14304241 (<.01%)
> instructions in affected programs: 1625 -> 1605 (-1.23%)
> helped: 4
> HURT: 0
> helped stats (abs) min: 1 max: 8 x̄: 5.00 x̃: 5
> helped stats (rel) min: 1.01% max: 14.29% x̄: 5.86% x̃: 4.07%
> 95% mean confidence interval for instructions value: -10.66 0.66
> 95% mean confidence interval for instructions %-change: -15.91% 4.19%
> Inconclusive result (value mean confidence interval includes 0).
>
> total cycles in shared programs: 527531226 -> 527531194 (<.01%)
> cycles in affected programs: 92204 -> 92172 (-0.03%)
> helped: 2
> HURT: 0
>
> Haswell and Broadwell had similar results. (Broadwell shown)
> total instructions in shared programs: 14615730 -> 14615710 (<.01%)
> instructions in affected programs: 1838 -> 1818 (-1.09%)
> helped: 4
> HURT: 0
> helped stats (abs) min: 1 max: 8 x̄: 5.00 x̃: 5
> helped stats (rel) min: 0.89% max: 13.04% x̄: 5.37% x̃: 3.78%
> 95% mean confidence interval for instructions value: -10.66 0.66
> 95% mean confidence interval for instructions %-change: -14.59% 3.85%
> Inconclusive result (value mean confidence interval includes 0).
>
> Signed-off-by: Ian Romanick 
> Reviewed-by: Caio Marcelo de Oliveira Filho 
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index 467aee0393f..15f34326c58 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -3899,10 +3899,16 @@ fs_visitor::nir_emit_intrinsic(const fs_builder
> , nir_intrinsic_instr *instr
>var->data.image.write_only ? GL_NONE : format);
>} else {
>   int op;
> + unsigned num_srcs = info->num_srcs;
>
>   switch (instr->intrinsic) {
>   case nir_intrinsic_image_deref_atomic_add:
> -op = BRW_AOP_ADD;
> +assert(num_srcs == 4);
> +
> +op = get_op_for_atomic_add(instr, 3);
> +
> +if (op != BRW_AOP_ADD)
> +   num_srcs = 3;
>  break;
>   case nir_intrinsic_image_deref_atomic_min:
>  op = (get_image_base_type(type) == BRW_REGISTER_TYPE_D ?
> @@ -3931,10 +3937,10 @@ fs_visitor::nir_emit_intrinsic(const fs_builder
> , nir_intrinsic_instr *instr
>  unreachable("Not reachable.");
>   }
>
> - const fs_reg src0 = (info->num_srcs >= 4 ?
> + const fs_reg src0 = (num_srcs >= 4 ?
>retype(get_nir_src(instr->src[3]),
> base_type) :
>fs_reg());
> - const fs_reg src1 = (info->num_srcs >= 5 ?
> + const fs_reg src1 = (num_srcs >= 5 ?
>retype(get_nir_src(instr->src[4]),
> base_type) :
>fs_reg());
>
> --
> 2.14.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/5 v2] i965/fs: Emit BRW_AOP_INC or BRW_AOP_DEC for atomicAdd of +1 or -1

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand 

On Mon, Aug 27, 2018 at 3:54 PM Ian Romanick  wrote:

> From: Ian Romanick 
>
> Funny story... a single shader was hurt for instructions, spills, fills.
> That same shader was also the most helped for cycles.  #GPUsAreWeird
>
> No changes on any other Intel platform.
>
> v2: Refactor selection of atomic opcode to a separate function.
> Suggested by Jason.
>
> Haswell, Broadwell, and Skylake had similar results. (Skylake shown)
> total instructions in shared programs: 14304116 -> 14304261 (<.01%)
> instructions in affected programs: 12776 -> 12921 (1.13%)
> helped: 19
> HURT: 1
> helped stats (abs) min: 1 max: 16 x̄: 2.32 x̃: 1
> helped stats (rel) min: 0.05% max: 7.27% x̄: 0.92% x̃: 0.55%
> HURT stats (abs)   min: 189 max: 189 x̄: 189.00 x̃: 189
> HURT stats (rel)   min: 4.87% max: 4.87% x̄: 4.87% x̃: 4.87%
> 95% mean confidence interval for instructions value: -12.83 27.33
> 95% mean confidence interval for instructions %-change: -1.57% 0.31%
> Inconclusive result (value mean confidence interval includes 0).
>
> total cycles in shared programs: 527552861 -> 527531226 (<.01%)
> cycles in affected programs: 1459195 -> 1437560 (-1.48%)
> helped: 16
> HURT: 2
> helped stats (abs) min: 2 max: 21328 x̄: 1353.69 x̃: 6
> helped stats (rel) min: 0.01% max: 5.29% x̄: 0.36% x̃: 0.03%
> HURT stats (abs)   min: 12 max: 12 x̄: 12.00 x̃: 12
> HURT stats (rel)   min: 0.03% max: 0.03% x̄: 0.03% x̃: 0.03%
> 95% mean confidence interval for cycles value: -3699.81 1295.92
> 95% mean confidence interval for cycles %-change: -0.94% 0.30%
> Inconclusive result (value mean confidence interval includes 0).
>
> total spills in shared programs: 8025 -> 8033 (0.10%)
> spills in affected programs: 208 -> 216 (3.85%)
> helped: 1
> HURT: 1
>
> total fills in shared programs: 10989 -> 11040 (0.46%)
> fills in affected programs: 444 -> 495 (11.49%)
> helped: 1
> HURT: 1
>
> Ivy Bridge
> total instructions in shared programs: 11709181 -> 11709153 (<.01%)
> instructions in affected programs: 3505 -> 3477 (-0.80%)
> helped: 3
> HURT: 0
> helped stats (abs) min: 1 max: 23 x̄: 9.33 x̃: 4
> helped stats (rel) min: 0.11% max: 1.16% x̄: 0.63% x̃: 0.61%
>
> total cycles in shared programs: 254741126 -> 254738801 (<.01%)
> cycles in affected programs: 919067 -> 916742 (-0.25%)
> helped: 3
> HURT: 0
> helped stats (abs) min: 21 max: 2144 x̄: 775.00 x̃: 160
> helped stats (rel) min: 0.03% max: 0.90% x̄: 0.32% x̃: 0.03%
>
> total spills in shared programs: 4536 -> 4533 (-0.07%)
> spills in affected programs: 40 -> 37 (-7.50%)
> helped: 1
> HURT: 0
>
> total fills in shared programs: 4819 -> 4813 (-0.12%)
> fills in affected programs: 94 -> 88 (-6.38%)
> helped: 1
> HURT: 0
>
> Signed-off-by: Ian Romanick 
> Reviewed-by: Caio Marcelo de Oliveira Filho  [v1]
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 27 +++
>  1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index 9c9df5ac09f..67c39a661ec 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -3604,6 +3604,21 @@ fs_visitor::nir_emit_fs_intrinsic(const fs_builder
> ,
> }
>  }
>
> +static int
> +get_op_for_atomic_add(nir_intrinsic_instr *instr, unsigned src)
> +{
> +   const nir_const_value *const val =
> nir_src_as_const_value(instr->src[src]);
> +
> +   if (val != NULL) {
> +  if (val->i32[0] == 1)
> + return BRW_AOP_INC;
> +  else if (val->i32[0] == -1)
> + return BRW_AOP_DEC;
> +   }
> +
> +   return BRW_AOP_ADD;
> +}
> +
>  void
>  fs_visitor::nir_emit_cs_intrinsic(const fs_builder ,
>nir_intrinsic_instr *instr)
> @@ -3660,7 +3675,7 @@ fs_visitor::nir_emit_cs_intrinsic(const fs_builder
> ,
> }
>
> case nir_intrinsic_shared_atomic_add:
> -  nir_emit_shared_atomic(bld, BRW_AOP_ADD, instr);
> +  nir_emit_shared_atomic(bld, get_op_for_atomic_add(instr, 1), instr);
>break;
> case nir_intrinsic_shared_atomic_imin:
>nir_emit_shared_atomic(bld, BRW_AOP_IMIN, instr);
> @@ -4378,7 +4393,7 @@ fs_visitor::nir_emit_intrinsic(const fs_builder
> , nir_intrinsic_instr *instr
> }
>
> case nir_intrinsic_ssbo_atomic_add:
> -  nir_emit_ssbo_atomic(bld, BRW_AOP_ADD, instr);
> +  nir_emit_ssbo_atomic(bld, get_op_for_atomic_add(instr, 2), instr);
>break;
> case nir_intrinsic_ssbo_atomic_imin:
>nir_emit_ssbo_atomic(bld, BRW_AOP_IMIN, instr);
> @@ -4888,7 +4903,9 @@ fs_visitor::nir_emit_ssbo_atomic(const fs_builder
> ,
> }
>
> fs_reg offset = get_nir_src(instr->src[1]);
> -   fs_reg data1 = get_nir_src(instr->src[2]);
> +   fs_reg data1;
> +   if (op != BRW_AOP_INC && op != BRW_AOP_DEC && op != BRW_AOP_PREDEC)
> +  data1 = get_nir_src(instr->src[2]);
> fs_reg data2;
> if (op == BRW_AOP_CMPWR)
>data2 = get_nir_src(instr->src[3]);
> @@ -4962,7 +4979,9 @@ 

Re: [Mesa-dev] [PATCH v4 7/7] nir: add loop unroll support for complex wrapper loops

[Timothy Arceri]

On 29/08/18 03:59, Jason Ekstrand wrote:
On Mon, Aug 27, 2018 at 4:10 AM Timothy Arceri > wrote:


In GLSL IR we cheat with switch statements and simply convert them
into loops with a single iteration. This allowed us to make use of
the existing jump instruction handling provided by the loop handing
code, it also allows dead code to be cleaned up once we have
wrapped the code in a loop.

However using loops in this way created previously unrollable loops
which limits further optimisations. Here we provide a way to unroll
loops that end in a break and have multiple other exits.

All shader-db changes are from the dolphin uber shaders. There is a
small amount of HURT shaders but in general the improvements far
exceed the HURT.

shader-db results IVB:

total instructions in shared programs: 10018187 -> 10016468 (-0.02%)
instructions in affected programs: 104080 -> 102361 (-1.65%)
helped: 36
HURT: 15

total cycles in shared programs: 220065064 -> 154529655 (-29.78%)
cycles in affected programs: 126063017 -> 60527608 (-51.99%)
helped: 51
HURT: 0

total loops in shared programs: 2515 -> 2308 (-8.23%)
loops in affected programs: 903 -> 696 (-22.92%)
helped: 51
HURT: 0

total spills in shared programs: 4370 -> 4124 (-5.63%)
spills in affected programs: 1397 -> 1151 (-17.61%)
helped: 9
HURT: 12

total fills in shared programs: 4581 -> 4419 (-3.54%)
fills in affected programs: 2201 -> 2039 (-7.36%)
helped: 9
HURT: 15
---
  src/compiler/nir/nir_opt_loop_unroll.c | 113 +
  1 file changed, 76 insertions(+), 37 deletions(-)

diff --git a/src/compiler/nir/nir_opt_loop_unroll.c
b/src/compiler/nir/nir_opt_loop_unroll.c
index 9c33267cb72..fa60523aac7 100644
--- a/src/compiler/nir/nir_opt_loop_unroll.c
+++ b/src/compiler/nir/nir_opt_loop_unroll.c
@@ -67,7 +67,6 @@ loop_prepare_for_unroll(nir_loop *loop)
     /* Remove continue if its the last instruction in the loop */
     nir_instr *last_instr =
nir_block_last_instr(nir_loop_last_block(loop));
     if (last_instr && last_instr->type == nir_instr_type_jump) {
-      assert(nir_instr_as_jump(last_instr)->type == nir_jump_continue);
        nir_instr_remove(last_instr);
     }
  }
@@ -474,54 +473,91 @@ complex_unroll(nir_loop *loop,
nir_loop_terminator *unlimit_term,
  static bool
  wrapper_unroll(nir_loop *loop)
  {
-   bool progress = false;
-
-   nir_block *blk_after_loop =
-      nir_cursor_current_block(nir_after_cf_node(>cf_node));
-
-   /* There may still be some single src phis following the loop that
-    * have not yet been cleaned up by another pass. Tidy those up
before
-    * unrolling the loop.
-    */
-   nir_foreach_instr_safe(instr, blk_after_loop) {
-      if (instr->type != nir_instr_type_phi)
-         break;
+   if (!list_empty(>info->loop_terminator_list)) {

-      nir_phi_instr *phi = nir_instr_as_phi(instr);
-      assert(exec_list_length(>srcs) == 1);
+      /* Unrolling a loop with a large number of exits can result in a
+       * large inrease in register pressure. For now we just skip
+       * unrolling if we have more than 3 exits (not including the
break
+       * at the end of the loop).
+       *
+       * TODO: Most loops that fit this pattern are simply switch
+       * statements that are converted to a loop to take advantage of
+       * exiting jump instruction handling. In this case we could make
+       * use of a binary seach pattern like we do in
+       * nir_lower_indirect_derefs(), this should allow us to
unroll the
+       * loops in an optimal way and should also avoid some of the
+       * register pressure that comes from simply nesting the
+       * terminators one after the other.
+       */
+      if (list_length(>info->loop_terminator_list) > 3)
+         return false;
+
+      loop_prepare_for_unroll(loop);
+
+      nir_cursor cursor = nir_after_block(nir_loop_last_block(loop));


Maybe call this loop_end; that's less generic than "cursor".


ok



+      list_for_each_entry(nir_loop_terminator, terminator,
+                          >info->loop_terminator_list,
+                          loop_terminator_link) {


I presume the terminator list is guaranteed to be sorted top to bottom?


yes



Reviewed-by: Jason Ekstrand >


Thanks!



+
+         /* Remove break from the terminator */
+         nir_instr *break_instr =
+            nir_block_last_instr(terminator->break_block);
+         nir_instr_remove(break_instr);
+
+         /* Pluck out the loop body. */
+         nir_cf_list loop_body;
+         nir_cf_extract(_body,
+ 

[Mesa-dev] [PATCH 2/3] egl/dri2: Guard against invalid fourcc formats

[Jason Ekstrand]

We already reject attempts to import images with invalid fourcc formats
but don't really guard the queries all that well.  This makes us error
out in any calls to eglQueryDmaBufModifiersEXT if the given format is
not a valid fourcc format.  We also add an assert to ensure that drivers
don't advertise any non-fourcc formats.

Cc: Daniel Stone 
Cc: mesa-sta...@lists.freedesktop.org
---
 src/egl/drivers/dri2/egl_dri2.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index daf535178ce..c5fa935657e 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -2382,6 +2382,18 @@ dri2_query_dma_buf_formats(_EGLDriver *drv, _EGLDisplay 
*disp,
 formats, count))
   return EGL_FALSE;
 
+   if (max > 0) {
+  /* Assert that all of the formats returned are actually fourcc formats.
+   * Some day, if we want the internal interface function to be able to
+   * return the fake fourcc formats defined in dri_interface.h, we'll have
+   * to do something more clever here to pair the list down to just real
+   * fourcc formats so that we don't leak the fake internal ones.
+   */
+  for (int i = 0; i < *count; i++) {
+ assert(dri2_num_fourcc_format_planes(formats[i]) > 0);
+  }
+   }
+
return EGL_TRUE;
 }
 
@@ -2392,6 +2404,9 @@ dri2_query_dma_buf_modifiers(_EGLDriver *drv, _EGLDisplay 
*disp, EGLint format,
 {
struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
 
+   if (dri2_num_fourcc_format_planes(format) == 0)
+  return _eglError(EGL_BAD_PARAMETER, "invalid fourcc format");
+
if (max < 0)
   return _eglError(EGL_BAD_PARAMETER, "invalid value for max count of 
formats");
 
-- 
2.17.1

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

[Mesa-dev] [PATCH 3/3] i965/screen: Allow modifiers on sRGB formats

[Jason Ekstrand]

This effectively reverts a26693493570a9d0f0fba1be617e01ee7bfff4db which
was a misguided attempt at protecting intel_query_dma_buf_modifiers from
invalid formats.  Unfortunately, in some internal EGL cases, we can get
an SRGB format validly in this function.  Rejecting such formats caused
us to not allow CCS in some cases where we should have been allowing it.

There's some question of whether or not we really should be using SRGB
"fourcc" formats that aren't actually in drm_foucc.h but there's not
much harm in allowing them through here.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107223
Fixes: a26693493570 "i965/screen: Return false for unsupported..."
---
 src/mesa/drivers/dri/i965/intel_screen.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
b/src/mesa/drivers/dri/i965/intel_screen.c
index eaf5a3b9feb..ac1938f6204 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1274,9 +1274,9 @@ static bool
 intel_image_format_is_supported(const struct gen_device_info *devinfo,
 const struct intel_image_format *fmt)
 {
-   if (fmt->fourcc == __DRI_IMAGE_FOURCC_SARGB ||
-   fmt->fourcc == __DRI_IMAGE_FOURCC_SABGR)
-  return false;
+   /* Currently, all formats with an intel_image_format are available on all
+* platforms so there's really nothing to check there.
+*/
 
 #ifndef NDEBUG
if (fmt->nplanes == 1) {
@@ -1302,6 +1302,14 @@ intel_query_dma_buf_formats(__DRIscreen *_screen, int 
max,
int num_formats = 0, i;
 
for (i = 0; i < ARRAY_SIZE(intel_image_formats); i++) {
+  /* These two formats are valid DRI formats but do not exist in
+   * drm_fourcc.h in the Linux kernel.  We don't want to accidentally
+   * advertise them through the EGL layer.
+   */
+  if (intel_image_formats[i].fourcc == __DRI_IMAGE_FOURCC_SARGB ||
+  intel_image_formats[i].fourcc == __DRI_IMAGE_FOURCC_SABGR)
+ return false;
+
   if (!intel_image_format_is_supported(>devinfo,
_image_formats[i]))
  continue;
-- 
2.17.1

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

[Mesa-dev] [PATCH 1/3] egl/dri2: Add a helper for the number of planes for a FOURCC format

[Jason Ekstrand]

This also serves as a convenient "is this a fourcc format" check as well
which we'll take advantage of in the next commit.

Cc: mesa-sta...@lists.freedesktop.org
---
 src/egl/drivers/dri2/egl_dri2.c | 32 +---
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 7fc7cb49703..daf535178ce 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -2224,13 +2224,13 @@ dri2_check_dma_buf_attribs(const _EGLImageAttribs 
*attrs)
return EGL_TRUE;
 }
 
-/* Returns the total number of file descriptors. Zero indicates an error. */
+/* Returns the total number of planes for the format or zero if it isn't a
+ * valid fourcc format.
+ */
 static unsigned
-dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
+dri2_num_fourcc_format_planes(EGLint format)
 {
-   unsigned plane_n;
-
-   switch (attrs->DMABufFourCC.Value) {
+   switch (format) {
case DRM_FORMAT_R8:
case DRM_FORMAT_RG88:
case DRM_FORMAT_GR88:
@@ -2278,14 +2278,14 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
case DRM_FORMAT_YVYU:
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
-  plane_n = 1;
-  break;
+  return 1;
+
case DRM_FORMAT_NV12:
case DRM_FORMAT_NV21:
case DRM_FORMAT_NV16:
case DRM_FORMAT_NV61:
-  plane_n = 2;
-  break;
+  return 2;
+
case DRM_FORMAT_YUV410:
case DRM_FORMAT_YVU410:
case DRM_FORMAT_YUV411:
@@ -2296,9 +2296,19 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
case DRM_FORMAT_YVU422:
case DRM_FORMAT_YUV444:
case DRM_FORMAT_YVU444:
-  plane_n = 3;
-  break;
+  return 3;
+
default:
+  return 0;
+   }
+}
+
+/* Returns the total number of file descriptors. Zero indicates an error. */
+static unsigned
+dri2_check_dma_buf_format(const _EGLImageAttribs *attrs)
+{
+   unsigned plane_n = dri2_num_fourcc_format_planes(attrs->DMABufFourCC.Value);
+   if (plane_n == 0) {
   _eglError(EGL_BAD_ATTRIBUTE, "invalid format");
   return 0;
}
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Ian Romanick]

On 08/28/2018 12:40 PM, Ian Romanick wrote:
> On 08/28/2018 12:09 PM, Rogovin, Kevin wrote:
>> Hi,
>>
>>> Is that tested?
>>
>> I have tested it in a simple shader test I made (i.e. not in a dedicated 
>> test suite such as dEQP, piglit or something else). The created assembly is 
>> identical. The specific example is a shader where I replace calls of 
>> beginFragmentShaderOrderingINTEL() with beginInvocationInterlockARB() and 
>> the created assembly is the same. Running with INTEL_DEBUG=fs produced 
>> identical output except the SSA NIR had the different opcode. AFAIK, the 
>> current Mesa implementation of the ARB extension does not in any way shape 
>> or form enforce any of "no control flow", "only once and only in main" 
>> requirements. Indeed, Mesa happily accepts code even without the 
>> endInvocationInterlockARB() call as well.  Personally, I am happy that it is 
>> more accepting rather than rejecting the code.
> 
> That's actually terrible and needs to be fixed ASAP.  That's how we end
> up with the "it works on " rubbish that has basically
> ruined GLSL.

Test cases sent to the piglit list.

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

Re: [Mesa-dev] [PATCH] mesa: allow GL_UNSIGNED_BYTE type for SNORM reads

[Andres Gomez]

This is:

Tested-by: Andres Gomez 


On Mon, 2018-08-27 at 14:46 +0300, Tapani Pälli wrote:
> OpenGL ES spec states:
>"For normalized fixed-point rendering surfaces, the combination format
> RGBA and type UNSIGNED_BYTE is accepted."
> 
> This fixes following failing VK-GL-CTS tests:
> 
>KHR-GLES3.packed_pixels.pbo_rectangle.rgba8_snorm
>KHR-GLES3.packed_pixels.rectangle.rgba8_snorm
>KHR-GLES3.packed_pixels.varied_rectangle.rgba8_snorm

Notice that, for me, it is not only fixing these 3 tests but also:

KHR-GLES3.packed_pixels.pbo_rectangle.r8_snorm
KHR-GLES3.packed_pixels.pbo_rectangle.rg8_snorm
KHR-GLES3.packed_pixels.rectangle.r8_snorm
KHR-GLES3.packed_pixels.rectangle.rg8_snorm
KHR-GLES3.packed_pixels.varied_rectangle.r8_snorm
KHR-GLES3.packed_pixels.varied_rectangle.rg8_snorm

As commented at:
https://bugs.freedesktop.org/show_bug.cgi?id=107658


> 
> Signed-off-by: Tapani Pälli 
> https://bugs.freedesktop.org/show_bug.cgi?id=107658
> Cc: mesa-sta...@lists.freedesktop.org
> ---
> 
> This is a partial fix to the bug. I believe there are 2 separate
> issues within reported bug and this fixes the first one.
> 
>  src/mesa/main/readpix.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> index 2cbb578a37f..556c860d393 100644
> --- a/src/mesa/main/readpix.c
> +++ b/src/mesa/main/readpix.c
> @@ -958,6 +958,15 @@ read_pixels_es3_error_check(struct gl_context *ctx, 
> GLenum format, GLenum type,
> return GL_NO_ERROR;
>   }
>}
> +  if (type == GL_UNSIGNED_BYTE) {
> + switch (internalFormat) {
> + case GL_R8_SNORM:
> + case GL_RG8_SNORM:
> + case GL_RGBA8_SNORM:
> +if (_mesa_has_EXT_render_snorm(ctx))
> +   return GL_NO_ERROR;
> + }
> +  }
>break;
> case GL_BGRA:
>/* GL_EXT_read_format_bgra */
-- 
Br,

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

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Jason Ekstrand]

On Tue, Aug 28, 2018 at 4:13 PM Rogovin, Kevin 
wrote:

> Hi,
>
> > You are completely missing the point.  Any test which tests the
> > GL_ARB_fragment_shader_interlock extension with a
> > beginInvocationInterlockARB() call inside of control-flow would be an
> > invalid test for GL_ARB_fragment_shader_interlock since that behavior is
> > undefined.  Therefore, any such test would be a bad test.  Unless we have
> > tests which test beginFragmentShaderOrderingINTEL() inside non-uniform
> > control-flow, it is insufficiently tested.  Therefore, any tests we may
> > have for GL_ARB_fragment_shader_interlock are either invalid use of the
> ARB
> > extension or they are insufficient to test the INTEL extension.
>
> My apologies, I misunderstood your question "is that tested?"; I thought
> the question was about if the assentation that the current implementation
> of beginFragmentShaderOrderingINTEL() and beginInvocationInterlockARB()
> were functionally interchangeable has been verified/tested.
>
> FWIW, a test where beginInvocationInterlockARB() is called within any
> control flow or for that matter not called from main(), the shader is
> supposed to be rejected according the spec. In a negative way, it is
> another test. Though, while acknowledging Ian's point that "works for
> vendor X" has caused a great deal of GLSL fragmentation, I still personally
> prefer to implementation to try to take what applications throw at them as
> much as reasonable possible. However, that is not my decision to make and
> whatever the community chooses is what it shall be.
>

If the spec says to reject it, then we should reject it and we should have
tests that ensure that we do so.  That's not really a matter of "take what
applications throw at them as much as reasonable possible", it's a matter
of implementing a spec.  The moment people start straying from the spec
because they think their implementation is more reasonable than what's
spec'd, it's no longer a spec.  In this case, that's exactly why the INTEL
extension exists.


> I agree with you that taking the current ARB test and then adding some
> non-uniform flow control (along with removing the layout() qualifiers) to
> it is the best way to go and I *think* Plamena has offered to do so.
>
> At this point, I can just let for it all to sort out and bow out of the
> discussion at this point as the series seems to have brought additional
> issues up that are out-of-scope for what I had in mind with the series
> (namely something small-simple to expose the extension and not enforcing
> the limitation of the ARB extension).
>

One of my fears here is whether or not the compiler will actually treat the
instruction as a barrier like it's supposed to.  We don't do a lot with
respect to optimization around barriers right now so it's probably ok.
However, the two extensions are sufficiently different from an API surface
perspective (even if the implementation is identical!) that we need to be
careful about making sure they're both tested.

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

[Mesa-dev] [PATCH] docs/relnotes: Add AMD_depth_clamp_separate for i965

[Sagar Ghuge]

Signed-off-by: Sagar Ghuge 
---
 docs/relnotes/18.3.0.html | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/relnotes/18.3.0.html b/docs/relnotes/18.3.0.html
index 71fb41ca86..6e5e3ef93b 100644
--- a/docs/relnotes/18.3.0.html
+++ b/docs/relnotes/18.3.0.html
@@ -51,6 +51,7 @@ Note: some of the new features are only available with 
certain drivers.
 
 
 
+GL_AMD_depth_clamp_separate on i965.
 GL_AMD_framebuffer_multisample_advanced on radeonsi.
 GL_AMD_gpu_shader_int64 on i965, nvc0, radeonsi.
 GL_AMD_multi_draw_indirect on all GL 4.x drivers.
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH] ac/surface: fix CMASK fast clear for NPOT textures with mipmapping on SI/CI/VI

[Bas Nieuwenhuizen]

Reviewed-by: Bas Nieuwenhuizen 
On Tue, Aug 28, 2018 at 8:41 PM Marek Olšák  wrote:
>
> From: Marek Olšák 
>
> This fixes VM faults and corruption.
>
> Cc: 18.1 18.2 
> ---
>  src/amd/common/ac_surface.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/amd/common/ac_surface.c b/src/amd/common/ac_surface.c
> index 2f4f0f8884f..94723dc9c09 100644
> --- a/src/amd/common/ac_surface.c
> +++ b/src/amd/common/ac_surface.c
> @@ -581,22 +581,22 @@ void ac_compute_cmask(const struct radeon_info *info,
> cl_width = 64;
> cl_height = 64;
> break;
> default:
> assert(0);
> return;
> }
>
> unsigned base_align = num_pipes * pipe_interleave_bytes;
>
> -   unsigned width = align(config->info.width, cl_width*8);
> -   unsigned height = align(config->info.height, cl_height*8);
> +   unsigned width = align(surf->u.legacy.level[0].nblk_x, cl_width*8);
> +   unsigned height = align(surf->u.legacy.level[0].nblk_y, cl_height*8);
> unsigned slice_elements = (width * height) / (8*8);
>
> /* Each element of CMASK is a nibble. */
> unsigned slice_bytes = slice_elements / 2;
>
> surf->u.legacy.cmask_slice_tile_max = (width * height) / (128*128);
> if (surf->u.legacy.cmask_slice_tile_max)
> surf->u.legacy.cmask_slice_tile_max -= 1;
>
> unsigned num_layers;
> --
> 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] i965/gen6/xfb: handle case where transform feedback is not active

[Andres Gomez]

Andrii, Samuel, should we also include this in the stable queues ?


On Wed, 2018-08-15 at 18:20 +0300, asimiklit.w...@gmail.com wrote:
> From: Andrii Simiklit 
> 
> When the SVBI Payload Enable is false I guess the register R1.4
> which contains the Maximum Streamed Vertex Buffer Index is filled by zero
> and GS stops to write transform feedback when the transform feedback 
> is not active.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107579
> Signed-off-by: Andrii Simiklit 
> ---
>  src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c 
> b/src/mesa/drivers/dri/i965/genX_state_upload.c
> index ea5ad55..0f82500 100644
> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> @@ -2806,7 +2806,7 @@ genX(upload_gs_state)(struct brw_context *brw)
>  #if GEN_GEN < 7
>   gs.SOStatisticsEnable = true;
>   if (gs_prog->info.has_transform_feedback_varyings)
> -gs.SVBIPayloadEnable = true;
> +gs.SVBIPayloadEnable = _mesa_is_xfb_active_and_unpaused(ctx);
>  
>   /* GEN6_GS_SPF_MODE and GEN6_GS_VECTOR_MASK_ENABLE are enabled as it
>* was previously done for gen6.
-- 
Br,

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

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Rogovin, Kevin]

Hi,

> You are completely missing the point.  Any test which tests the
> GL_ARB_fragment_shader_interlock extension with a
> beginInvocationInterlockARB() call inside of control-flow would be an
> invalid test for GL_ARB_fragment_shader_interlock since that behavior is
> undefined.  Therefore, any such test would be a bad test.  Unless we have
> tests which test beginFragmentShaderOrderingINTEL() inside non-uniform
> control-flow, it is insufficiently tested.  Therefore, any tests we may
> have for GL_ARB_fragment_shader_interlock are either invalid use of the ARB
> extension or they are insufficient to test the INTEL extension.

My apologies, I misunderstood your question "is that tested?"; I thought the 
question was about if the assentation that the current implementation of 
beginFragmentShaderOrderingINTEL() and beginInvocationInterlockARB() were 
functionally interchangeable has been verified/tested.

FWIW, a test where beginInvocationInterlockARB() is called within any control 
flow or for that matter not called from main(), the shader is supposed to be 
rejected according the spec. In a negative way, it is another test. Though, 
while acknowledging Ian's point that "works for vendor X" has caused a great 
deal of GLSL fragmentation, I still personally prefer to implementation to try 
to take what applications throw at them as much as reasonable possible. 
However, that is not my decision to make and whatever the community chooses is 
what it shall be.

I agree with you that taking the current ARB test and then adding some 
non-uniform flow control (along with removing the layout() qualifiers) to it is 
the best way to go and I *think* Plamena has offered to do so.

At this point, I can just let for it all to sort out and bow out of the 
discussion at this point as the series seems to have brought additional issues 
up that are out-of-scope for what I had in mind with the series (namely 
something small-simple to expose the extension and not enforcing the limitation 
of the ARB extension).

-Kevin

-Original Message-
From: Rogovin, Kevin 
Sent: Tuesday, August 28, 2018 10:10 PM
To: 'mesa-dev@lists.freedesktop.org' 
Cc: ja...@jlekstrand.net
Subject: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

Hi,

> Is that tested?

I have tested it in a simple shader test I made (i.e. not in a dedicated test 
suite such as dEQP, piglit or something else). The created assembly is 
identical. The specific example is a shader where I replace calls of 
beginFragmentShaderOrderingINTEL() with beginInvocationInterlockARB() and the 
created assembly is the same. Running with INTEL_DEBUG=fs produced identical 
output except the SSA NIR had the different opcode. AFAIK, the current Mesa 
implementation of the ARB extension does not in any way shape or form enforce 
any of "no control flow", "only once and only in main" requirements. Indeed, 
Mesa happily accepts code even without the endInvocationInterlockARB() call as 
well.  Personally, I am happy that it is more accepting rather than rejecting 
the code.

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

Re: [Mesa-dev] [PATCH 1/2] st/mesa: help fix stencil border color for GL_DEPTH_STENCIL textures

[Marek Olšák]

ping

On Mon, Aug 20, 2018 at 11:20 PM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> GL_STENCIL_INDEX uses GL_INTENSITY for the border color, which is nicer
> to hardware that doesn't read the stencil border value from the X channel.
>
> This fixes a bunch of dEQP tests on Vega & Raven.
>
> Cc: 18.1 18.2 
> ---
>  src/mesa/state_tracker/st_atom_sampler.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/mesa/state_tracker/st_atom_sampler.c 
> b/src/mesa/state_tracker/st_atom_sampler.c
> index 289856cd72d..27e4da31581 100644
> --- a/src/mesa/state_tracker/st_atom_sampler.c
> +++ b/src/mesa/state_tracker/st_atom_sampler.c
> @@ -156,20 +156,23 @@ st_convert_sampler(const struct st_context *st,
> /* For non-black borders... */
> if (/* This is true if wrap modes are using the border color: */
> (sampler->wrap_s | sampler->wrap_t | sampler->wrap_r) & 0x1 &&
> (msamp->BorderColor.ui[0] ||
>  msamp->BorderColor.ui[1] ||
>  msamp->BorderColor.ui[2] ||
>  msamp->BorderColor.ui[3])) {
>const GLboolean is_integer = texobj->_IsIntegerFormat;
>GLenum texBaseFormat = _mesa_base_tex_image(texobj)->_BaseFormat;
>
> +  if (texobj->StencilSampling)
> + texBaseFormat = GL_STENCIL_INDEX;
> +
>if (st->apply_texture_swizzle_to_border_color) {
>   const struct st_texture_object *stobj = 
> st_texture_object_const(texobj);
>   /* XXX: clean that up to not use the sampler view at all */
>   const struct st_sampler_view *sv = 
> st_texture_get_current_sampler_view(st, stobj);
>
>   if (sv) {
>  struct pipe_sampler_view *view = sv->view;
>  union pipe_color_union tmp;
>  const unsigned char swz[4] =
>  {
> --
> 2.17.1
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] i965/screen: Allow modifiers on sRGB formats

[Jason Ekstrand]

This effectively reverts a26693493570a9d0f0fba1be617e01ee7bfff4db which
was a misguided attempt at protecting intel_query_dma_buf_modifiers from
invalid formats.  Unfortunately, in some internal EGL cases, we can get
an SRGB format validly in this function.  Rejecting such formats caused
us to not allow CCS in some cases where we should have been allowing it.

There's some question of whether or not we really should be using SRGB
"fourcc" formats that aren't actually in drm_foucc.h but there's not
much harm in allowing them through here.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107223
Fixes: a26693493570 "i965/screen: Return false for unsupported..."
---
 src/mesa/drivers/dri/i965/intel_screen.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
b/src/mesa/drivers/dri/i965/intel_screen.c
index eaf5a3b9feb..ac1938f6204 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1274,9 +1274,9 @@ static bool
 intel_image_format_is_supported(const struct gen_device_info *devinfo,
 const struct intel_image_format *fmt)
 {
-   if (fmt->fourcc == __DRI_IMAGE_FOURCC_SARGB ||
-   fmt->fourcc == __DRI_IMAGE_FOURCC_SABGR)
-  return false;
+   /* Currently, all formats with an intel_image_format are available on all
+* platforms so there's really nothing to check there.
+*/
 
 #ifndef NDEBUG
if (fmt->nplanes == 1) {
@@ -1302,6 +1302,14 @@ intel_query_dma_buf_formats(__DRIscreen *_screen, int 
max,
int num_formats = 0, i;
 
for (i = 0; i < ARRAY_SIZE(intel_image_formats); i++) {
+  /* These two formats are valid DRI formats but do not exist in
+   * drm_fourcc.h in the Linux kernel.  We don't want to accidentally
+   * advertise them through the EGL layer.
+   */
+  if (intel_image_formats[i].fourcc == __DRI_IMAGE_FOURCC_SARGB ||
+  intel_image_formats[i].fourcc == __DRI_IMAGE_FOURCC_SABGR)
+ return false;
+
   if (!intel_image_format_is_supported(>devinfo,
_image_formats[i]))
  continue;
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Mark Janes]

Kevin,

Your email messages are missing the in-reply-to header, making this
thread completely incomprehensible on the mailing list.

Please fix your mail client to include the standard headers so we can
follow what you are saying.

-Mark

"Rogovin, Kevin"  writes:

> Hi,
>
>> Having the same underlying compiler intrinsic and having the same behavior
>> are not the same thing.  The INTEL extension allows strictly more
>> functionality than the ARB extension so it needs even more testing.  In
>> particular, it allows you to do the barrier in non-uniform control-flow
>> which is a very different thing than at the top of a shader like is allowed
>> by ARB_fragment_shader_interlock.  I expect the INTEL extension to need
>> substantially more testing than the ARB one.
>
> Just an FYI: the Mesa implementation for the ARB (and thus NV)
> extension accept shaders where the begin function takes place inside
> of control flow actually.
>
>  -Kevin
> ___
> 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] i965/icl: Set Enabled Texel Offset Precision Fix bit

[Anuj Phogat]

On Tue, Aug 28, 2018 at 10:57 AM Chris Wilson  wrote:
>
> Quoting Anuj Phogat (2018-08-28 18:53:59)
> > h/w specification requires this bit to be always set.
> >
> > Suggested-by: Kenneth Graunke 
> > Signed-off-by: Anuj Phogat 
> > ---
> >  src/mesa/drivers/dri/i965/brw_defines.h  | 4 
> >  src/mesa/drivers/dri/i965/brw_state_upload.c | 7 +++
> >  2 files changed, 11 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
> > b/src/mesa/drivers/dri/i965/brw_defines.h
> > index 433314115b1..1c73ddeb190 100644
> > --- a/src/mesa/drivers/dri/i965/brw_defines.h
> > +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> > @@ -1673,6 +1673,10 @@ enum brw_pixel_shader_coverage_mask_mode {
> >  # define GLK_SCEC_BARRIER_MODE_3D_HULL (1 << 7)
> >  # define GLK_SCEC_BARRIER_MODE_MASKREG_MASK(1 << 7)
> >
> > +#define HALF_SLICE_CHICKEN70xE194
> > +# define TEXEL_OFFSET_FIX_ENABLE   (1 << 1)
> > +# define TEXEL_OFFSET_FIX_MASK REG_MASK(1 << 7)
>
> That mask doesn't match the enable-bit.
>
Right. copy-paste error :(
v2 coming.

> It'll probably be quite useful to record all the registers you are
> setting as part of the global setup and read them back later to make
> sure they stuck.
That's a good idea Chris. I'll do it later in a separate series.
> -Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Ian Romanick]

On 08/28/2018 12:09 PM, Rogovin, Kevin wrote:
> Hi,
> 
>> Is that tested?
> 
> I have tested it in a simple shader test I made (i.e. not in a dedicated test 
> suite such as dEQP, piglit or something else). The created assembly is 
> identical. The specific example is a shader where I replace calls of 
> beginFragmentShaderOrderingINTEL() with beginInvocationInterlockARB() and the 
> created assembly is the same. Running with INTEL_DEBUG=fs produced identical 
> output except the SSA NIR had the different opcode. AFAIK, the current Mesa 
> implementation of the ARB extension does not in any way shape or form enforce 
> any of "no control flow", "only once and only in main" requirements. Indeed, 
> Mesa happily accepts code even without the endInvocationInterlockARB() call 
> as well.  Personally, I am happy that it is more accepting rather than 
> rejecting the code.

That's actually terrible and needs to be fixed ASAP.  That's how we end
up with the "it works on " rubbish that has basically
ruined GLSL.

> -Kevin
> ___
> 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 0/2] Implement INTEL_fragment_shader_ordering

[Jason Ekstrand]

On Tue, Aug 28, 2018 at 2:10 PM Rogovin, Kevin 
wrote:

> Hi,
>
> > Is that tested?
>
> I have tested it in a simple shader test I made (i.e. not in a dedicated
> test suite such as dEQP, piglit or something else). The created assembly is
> identical. The specific example is a shader where I replace calls of
> beginFragmentShaderOrderingINTEL() with beginInvocationInterlockARB() and
> the created assembly is the same. Running with INTEL_DEBUG=fs produced
> identical output except the SSA NIR had the different opcode. AFAIK, the
> current Mesa implementation of the ARB extension does not in any way shape
> or form enforce any of "no control flow", "only once and only in main"
> requirements. Indeed, Mesa happily accepts code even without the
> endInvocationInterlockARB() call as well.  Personally, I am happy that it
> is more accepting rather than rejecting the code.
>

You are completely missing the point.  Any test which tests the
GL_ARB_fragment_shader_interlock extension with a
beginInvocationInterlockARB() call inside of control-flow would be an
invalid test for GL_ARB_fragment_shader_interlock since that behavior is
undefined.  Therefore, any such test would be a bad test.  Unless we have
tests which test beginFragmentShaderOrderingINTEL() inside non-uniform
control-flow, it is insufficiently tested.  Therefore, any tests we may
have for GL_ARB_fragment_shader_interlock are either invalid use of the ARB
extension or they are insufficient to test the INTEL extension.

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

[Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Rogovin, Kevin]

Hi,

> Is that tested?

I have tested it in a simple shader test I made (i.e. not in a dedicated test 
suite such as dEQP, piglit or something else). The created assembly is 
identical. The specific example is a shader where I replace calls of 
beginFragmentShaderOrderingINTEL() with beginInvocationInterlockARB() and the 
created assembly is the same. Running with INTEL_DEBUG=fs produced identical 
output except the SSA NIR had the different opcode. AFAIK, the current Mesa 
implementation of the ARB extension does not in any way shape or form enforce 
any of "no control flow", "only once and only in main" requirements. Indeed, 
Mesa happily accepts code even without the endInvocationInterlockARB() call as 
well.  Personally, I am happy that it is more accepting rather than rejecting 
the code.

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

Re: [Mesa-dev] [PATCH] gallium: fix drivers to report something for PIPE_CAP_VIEWPORT_SUBPIXEL_BITS

[Roland Scheidegger]

This is not correct.

The problem here is that there's only a viewport subpixel accuracy
query, but the state tracker uses this to both set subpixel (which is
raterization subpixel accuracy) as well as viewport subpixel bits.
The former needs to be at least 4 (dx10 requires 8), whereas the latter
still is ok to be 0.

For instance, llvmpipe and probably a bunch of other drivers would have
8 subpixel bits for rasterization, but they use actual integers for
viewport bounds, hence 0 bits. It would be incorrect to report 8 bits
for viewport accuracy (even though without msaa it matters little).

So I suspect to fix this properly you'd need a new pipe cap bit (whereas
0 as default for viewport subpixels would be just fine, but not for
rasterization subpixel).

Roland



Am 28.08.2018 um 20:19 schrieb Marek Olšák:
> From: Marek Olšák 
> 
> ---
>  src/gallium/drivers/etnaviv/etnaviv_screen.c | 4 +++-
>  src/gallium/drivers/freedreno/freedreno_screen.c | 4 +++-
>  src/gallium/drivers/i915/i915_screen.c   | 4 +++-
>  src/gallium/drivers/llvmpipe/lp_screen.c | 3 ++-
>  src/gallium/drivers/nouveau/nv30/nv30_screen.c   | 3 ++-
>  src/gallium/drivers/nouveau/nv50/nv50_screen.c   | 4 +++-
>  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c   | 4 +++-
>  src/gallium/drivers/r300/r300_screen.c   | 5 -
>  src/gallium/drivers/softpipe/sp_screen.c | 3 ++-
>  src/gallium/drivers/svga/svga_screen.c   | 4 +++-
>  src/gallium/drivers/swr/swr_screen.cpp   | 4 +++-
>  src/gallium/drivers/v3d/v3d_screen.c | 4 +++-
>  src/gallium/drivers/vc4/vc4_screen.c | 4 +++-
>  src/gallium/drivers/virgl/virgl_screen.c | 3 ++-
>  14 files changed, 39 insertions(+), 14 deletions(-)
> 
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c 
> b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> index 9669bd2f601..4fe1c9ebead 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> @@ -162,20 +162,23 @@ etna_screen_get_param(struct pipe_screen *pscreen, enum 
> pipe_cap param)
>NON_POWER_OF_TWO); */
>  
> case PIPE_CAP_TEXTURE_SWIZZLE:
> case PIPE_CAP_PRIMITIVE_RESTART:
>return VIV_FEATURE(screen, chipMinorFeatures1, HALTI0);
>  
> case PIPE_CAP_ENDIANNESS:
>return PIPE_ENDIAN_LITTLE; /* on most Viv hw this is configurable 
> (feature
>  ENDIANNESS_CONFIG) */
>  
> +   case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
> +  return 8;
> +
> /* Unsupported features. */
> case PIPE_CAP_SEAMLESS_CUBE_MAP:
> case PIPE_CAP_COMPUTE: /* XXX supported on gc2000 */
> case PIPE_CAP_MIXED_COLORBUFFER_FORMATS: /* only one colorbuffer 
> supported, so mixing makes no sense */
> case PIPE_CAP_CONDITIONAL_RENDER: /* no occlusion queries */
> case PIPE_CAP_TGSI_INSTANCEID: /* no idea, really */
> case PIPE_CAP_START_INSTANCE: /* instancing not supported AFAIK */
> case PIPE_CAP_VERTEX_ELEMENT_INSTANCE_DIVISOR: /* instancing not 
> supported AFAIK */
> case PIPE_CAP_SHADER_STENCIL_EXPORT: /* Fragment shader cannot export 
> stencil value */
> case PIPE_CAP_MAX_DUAL_SOURCE_RENDER_TARGETS: /* no dual-source supported 
> */
> @@ -237,21 +240,20 @@ etna_screen_get_param(struct pipe_screen *pscreen, enum 
> pipe_cap param)
> case PIPE_CAP_SURFACE_REINTERPRET_BLOCKS:
> case PIPE_CAP_QUERY_BUFFER_OBJECT:
> case PIPE_CAP_QUERY_MEMORY_INFO:
> case PIPE_CAP_FRAMEBUFFER_NO_ATTACHMENT:
> case PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR:
> case PIPE_CAP_CULL_DISTANCE:
> case PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES:
> case PIPE_CAP_TGSI_VOTE:
> case PIPE_CAP_MAX_WINDOW_RECTANGLES:
> case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
> -   case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
> case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
> case PIPE_CAP_STREAM_OUTPUT_INTERLEAVE_BUFFERS:
> case PIPE_CAP_TGSI_CAN_READ_OUTPUTS:
> case PIPE_CAP_GLSL_OPTIMIZE_CONSERVATIVELY:
> case PIPE_CAP_TGSI_FS_FBFETCH:
> case PIPE_CAP_TGSI_MUL_ZERO_WINS:
> case PIPE_CAP_DOUBLES:
> case PIPE_CAP_INT64:
> case PIPE_CAP_INT64_DIVMOD:
> case PIPE_CAP_TGSI_TEX_TXF_LZ:
> diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c 
> b/src/gallium/drivers/freedreno/freedreno_screen.c
> index 489986703cd..394edbcd1fe 100644
> --- a/src/gallium/drivers/freedreno/freedreno_screen.c
> +++ b/src/gallium/drivers/freedreno/freedreno_screen.c
> @@ -291,20 +291,23 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum 
> pipe_cap param)
>   case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
>   if (is_a5xx(screen) || is_a6xx(screen))
>   return 4;
>   return 0;
>  
>   case PIPE_CAP_MAX_TEXTURE_GATHER_COMPONENTS:
>   if (is_a4xx(screen) || is_a5xx(screen) || is_a6xx(screen))
>   return 4;
>   return 0;
>  
> + 

[Mesa-dev] [PATCH] ac/surface: fix CMASK fast clear for NPOT textures with mipmapping on SI/CI/VI

[Marek Olšák]

From: Marek Olšák 

This fixes VM faults and corruption.

Cc: 18.1 18.2 
---
 src/amd/common/ac_surface.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/amd/common/ac_surface.c b/src/amd/common/ac_surface.c
index 2f4f0f8884f..94723dc9c09 100644
--- a/src/amd/common/ac_surface.c
+++ b/src/amd/common/ac_surface.c
@@ -581,22 +581,22 @@ void ac_compute_cmask(const struct radeon_info *info,
cl_width = 64;
cl_height = 64;
break;
default:
assert(0);
return;
}
 
unsigned base_align = num_pipes * pipe_interleave_bytes;
 
-   unsigned width = align(config->info.width, cl_width*8);
-   unsigned height = align(config->info.height, cl_height*8);
+   unsigned width = align(surf->u.legacy.level[0].nblk_x, cl_width*8);
+   unsigned height = align(surf->u.legacy.level[0].nblk_y, cl_height*8);
unsigned slice_elements = (width * height) / (8*8);
 
/* Each element of CMASK is a nibble. */
unsigned slice_bytes = slice_elements / 2;
 
surf->u.legacy.cmask_slice_tile_max = (width * height) / (128*128);
if (surf->u.legacy.cmask_slice_tile_max)
surf->u.legacy.cmask_slice_tile_max -= 1;
 
unsigned num_layers;
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Jason Ekstrand]

On Tue, Aug 28, 2018 at 1:23 PM Rogovin, Kevin 
wrote:

> Hi,
>
> > Having the same underlying compiler intrinsic and having the same
> behavior
> > are not the same thing.  The INTEL extension allows strictly more
> > functionality than the ARB extension so it needs even more testing.  In
> > particular, it allows you to do the barrier in non-uniform control-flow
> > which is a very different thing than at the top of a shader like is
> allowed
> > by ARB_fragment_shader_interlock.  I expect the INTEL extension to need
> > substantially more testing than the ARB one.
>
> Just an FYI: the Mesa implementation for the ARB (and thus NV) extension
> accept shaders where the begin function takes place inside of control flow
> actually.
>

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

[Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Rogovin, Kevin]

Hi,

> Having the same underlying compiler intrinsic and having the same behavior
> are not the same thing.  The INTEL extension allows strictly more
> functionality than the ARB extension so it needs even more testing.  In
> particular, it allows you to do the barrier in non-uniform control-flow
> which is a very different thing than at the top of a shader like is allowed
> by ARB_fragment_shader_interlock.  I expect the INTEL extension to need
> substantially more testing than the ARB one.

Just an FYI: the Mesa implementation for the ARB (and thus NV) extension accept 
shaders where the begin function takes place inside of control flow actually.

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

[Mesa-dev] [PATCH] gallium: fix drivers to report something for PIPE_CAP_VIEWPORT_SUBPIXEL_BITS

[Marek Olšák]

From: Marek Olšák 

---
 src/gallium/drivers/etnaviv/etnaviv_screen.c | 4 +++-
 src/gallium/drivers/freedreno/freedreno_screen.c | 4 +++-
 src/gallium/drivers/i915/i915_screen.c   | 4 +++-
 src/gallium/drivers/llvmpipe/lp_screen.c | 3 ++-
 src/gallium/drivers/nouveau/nv30/nv30_screen.c   | 3 ++-
 src/gallium/drivers/nouveau/nv50/nv50_screen.c   | 4 +++-
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c   | 4 +++-
 src/gallium/drivers/r300/r300_screen.c   | 5 -
 src/gallium/drivers/softpipe/sp_screen.c | 3 ++-
 src/gallium/drivers/svga/svga_screen.c   | 4 +++-
 src/gallium/drivers/swr/swr_screen.cpp   | 4 +++-
 src/gallium/drivers/v3d/v3d_screen.c | 4 +++-
 src/gallium/drivers/vc4/vc4_screen.c | 4 +++-
 src/gallium/drivers/virgl/virgl_screen.c | 3 ++-
 14 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c 
b/src/gallium/drivers/etnaviv/etnaviv_screen.c
index 9669bd2f601..4fe1c9ebead 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
@@ -162,20 +162,23 @@ etna_screen_get_param(struct pipe_screen *pscreen, enum 
pipe_cap param)
   NON_POWER_OF_TWO); */
 
case PIPE_CAP_TEXTURE_SWIZZLE:
case PIPE_CAP_PRIMITIVE_RESTART:
   return VIV_FEATURE(screen, chipMinorFeatures1, HALTI0);
 
case PIPE_CAP_ENDIANNESS:
   return PIPE_ENDIAN_LITTLE; /* on most Viv hw this is configurable 
(feature
 ENDIANNESS_CONFIG) */
 
+   case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
+  return 8;
+
/* Unsupported features. */
case PIPE_CAP_SEAMLESS_CUBE_MAP:
case PIPE_CAP_COMPUTE: /* XXX supported on gc2000 */
case PIPE_CAP_MIXED_COLORBUFFER_FORMATS: /* only one colorbuffer supported, 
so mixing makes no sense */
case PIPE_CAP_CONDITIONAL_RENDER: /* no occlusion queries */
case PIPE_CAP_TGSI_INSTANCEID: /* no idea, really */
case PIPE_CAP_START_INSTANCE: /* instancing not supported AFAIK */
case PIPE_CAP_VERTEX_ELEMENT_INSTANCE_DIVISOR: /* instancing not supported 
AFAIK */
case PIPE_CAP_SHADER_STENCIL_EXPORT: /* Fragment shader cannot export 
stencil value */
case PIPE_CAP_MAX_DUAL_SOURCE_RENDER_TARGETS: /* no dual-source supported */
@@ -237,21 +240,20 @@ etna_screen_get_param(struct pipe_screen *pscreen, enum 
pipe_cap param)
case PIPE_CAP_SURFACE_REINTERPRET_BLOCKS:
case PIPE_CAP_QUERY_BUFFER_OBJECT:
case PIPE_CAP_QUERY_MEMORY_INFO:
case PIPE_CAP_FRAMEBUFFER_NO_ATTACHMENT:
case PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR:
case PIPE_CAP_CULL_DISTANCE:
case PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES:
case PIPE_CAP_TGSI_VOTE:
case PIPE_CAP_MAX_WINDOW_RECTANGLES:
case PIPE_CAP_POLYGON_OFFSET_UNITS_UNSCALED:
-   case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
case PIPE_CAP_TGSI_ARRAY_COMPONENTS:
case PIPE_CAP_STREAM_OUTPUT_INTERLEAVE_BUFFERS:
case PIPE_CAP_TGSI_CAN_READ_OUTPUTS:
case PIPE_CAP_GLSL_OPTIMIZE_CONSERVATIVELY:
case PIPE_CAP_TGSI_FS_FBFETCH:
case PIPE_CAP_TGSI_MUL_ZERO_WINS:
case PIPE_CAP_DOUBLES:
case PIPE_CAP_INT64:
case PIPE_CAP_INT64_DIVMOD:
case PIPE_CAP_TGSI_TEX_TXF_LZ:
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c 
b/src/gallium/drivers/freedreno/freedreno_screen.c
index 489986703cd..394edbcd1fe 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -291,20 +291,23 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum 
pipe_cap param)
case PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT:
if (is_a5xx(screen) || is_a6xx(screen))
return 4;
return 0;
 
case PIPE_CAP_MAX_TEXTURE_GATHER_COMPONENTS:
if (is_a4xx(screen) || is_a5xx(screen) || is_a6xx(screen))
return 4;
return 0;
 
+   case PIPE_CAP_VIEWPORT_SUBPIXEL_BITS:
+   return 8;
+
/* Unsupported features. */
case PIPE_CAP_TGSI_FS_COORD_ORIGIN_LOWER_LEFT:
case PIPE_CAP_TGSI_FS_COORD_PIXEL_CENTER_HALF_INTEGER:
case PIPE_CAP_TGSI_CAN_COMPACT_CONSTANTS:
case PIPE_CAP_USER_VERTEX_BUFFERS:
case PIPE_CAP_QUERY_PIPELINE_STATISTICS:
case PIPE_CAP_TEXTURE_BORDER_COLOR_QUIRK:
case PIPE_CAP_TGSI_VS_LAYER_VIEWPORT:
case PIPE_CAP_TEXTURE_GATHER_SM5:
case PIPE_CAP_SAMPLE_SHADING:
@@ -328,21 +331,20 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum 
pipe_cap param)
case PIPE_CAP_TGSI_FS_POSITION_IS_SYSVAL:
case PIPE_CAP_TGSI_FS_FACE_IS_INTEGER_SYSVAL:
case PIPE_CAP_GENERATE_MIPMAP:
case PIPE_CAP_SURFACE_REINTERPRET_BLOCKS:
case PIPE_CAP_ROBUST_BUFFER_ACCESS_BEHAVIOR:
case PIPE_CAP_CULL_DISTANCE:
case PIPE_CAP_PRIMITIVE_RESTART_FOR_PATCHES:
case 

[Mesa-dev] [1/2] mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering.

[Rogovin, Kevin]

Hi all,

Patch addressing the missing enum warning is here: 
https://lists.freedesktop.org/archives/mesa-dev/2018-August/203796.html . Also, 
see my reply to Francisco why I think it is better to have a new intrinsic 
function for that.

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

[Mesa-dev] [PATCH] mesa/state_tracker: explicitely handle case ir_intrinsic_begin_fragment_shader_ordering in visit_generic_intrinsic()

[kevin . rogovin]

From: Kevin Rogovin 

---
 src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
index 7b96947c60..48a7b030ce 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
@@ -4068,6 +4068,7 @@ glsl_to_tgsi_visitor::visit(ir_call *ir)
case ir_intrinsic_generic_atomic_comp_swap:
case ir_intrinsic_begin_invocation_interlock:
case ir_intrinsic_end_invocation_interlock:
+   case ir_intrinsic_begin_fragment_shader_ordering:
   unreachable("Invalid intrinsic");
}
 }
-- 
2.17.1

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

Re: [Mesa-dev] [PATCH 3/3] anv: Claim to support depthBounds for ID games

[Jason Ekstrand]

On Tue, Aug 28, 2018 at 12:40 PM Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> On 27/08/2018 23:48, Jason Ekstrand wrote:
>
> On Mon, Aug 27, 2018 at 12:50 PM Lionel Landwerlin <
> lionel.g.landwer...@intel.com> wrote:
>
>> On 23/08/2018 16:13, Jason Ekstrand wrote:
>> > ---
>> >   src/intel/vulkan/anv_device.c | 9 +
>> >   1 file changed, 9 insertions(+)
>> >
>> > diff --git a/src/intel/vulkan/anv_device.c
>> b/src/intel/vulkan/anv_device.c
>> > index 0357fc7c0ea..5a63592f7ca 100644
>> > --- a/src/intel/vulkan/anv_device.c
>> > +++ b/src/intel/vulkan/anv_device.c
>> > @@ -854,6 +854,15 @@ void anv_GetPhysicalDeviceFeatures(
>> >  pFeatures->vertexPipelineStoresAndAtomics =
>> > pdevice->compiler->scalar_stage[MESA_SHADER_VERTEX] &&
>> > pdevice->compiler->scalar_stage[MESA_SHADER_GEOMETRY];
>> > +
>> > +   struct anv_app_info *app_info = >instance->app_info;
>> > +
>> > +   /* The new DOOM and Wolfenstein games require depthBounds without
>> > +* checking for it.  They seem to run fine without it so just claim
>> it's
>> > +* there and accept the consequences.
>> > +*/
>> > +   if (app_info->engine_name && strcmp(app_info->engine_name,
>> "idTech") == 0)
>> > +  pFeatures->depthBounds = true;
>> >   }
>> >
>> >   void anv_GetPhysicalDeviceFeatures2(
>>
>> I'm struggling to understand "require depthBounds with checking for it".
>> I thought one would check for the feature by reading the boolean you
>> just set. So if it's not checked why would it make a difference?
>>
>
> The Vulkan spec requires that we throw VK_ERROR_INITIALIZATION_FAILED if
> the client enables any features we have not advertised.  This gives us two
> options: 1) advertise support for depthBounds for idTech games or 2) Skip
> the check for depthBounds in CreateDevice and let it succeed even if it's
> enabled for idTech games.  This patch takes the former approach since it's
> easier.
>
> --Jason
>
>
> Oh thanks for the explanation.
> With the strdup() fix this series is :
>
> Reviewed-by: Lionel Landwerlin 
> 
>

Thanks!


> I remember talks about doing this kind of tricks for particular titles in
> a layer.
>
> It doesn't necessarily makes things easier for us, but are you still
> thinking about that layer mechanism?
>
If someone wants to write that layer, great.  If not, it's easier for us to
just do it in the driver.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v4 7/7] nir: add loop unroll support for complex wrapper loops

[Jason Ekstrand]

On Mon, Aug 27, 2018 at 4:10 AM Timothy Arceri 
wrote:

> In GLSL IR we cheat with switch statements and simply convert them
> into loops with a single iteration. This allowed us to make use of
> the existing jump instruction handling provided by the loop handing
> code, it also allows dead code to be cleaned up once we have
> wrapped the code in a loop.
>
> However using loops in this way created previously unrollable loops
> which limits further optimisations. Here we provide a way to unroll
> loops that end in a break and have multiple other exits.
>
> All shader-db changes are from the dolphin uber shaders. There is a
> small amount of HURT shaders but in general the improvements far
> exceed the HURT.
>
> shader-db results IVB:
>
> total instructions in shared programs: 10018187 -> 10016468 (-0.02%)
> instructions in affected programs: 104080 -> 102361 (-1.65%)
> helped: 36
> HURT: 15
>
> total cycles in shared programs: 220065064 -> 154529655 (-29.78%)
> cycles in affected programs: 126063017 -> 60527608 (-51.99%)
> helped: 51
> HURT: 0
>
> total loops in shared programs: 2515 -> 2308 (-8.23%)
> loops in affected programs: 903 -> 696 (-22.92%)
> helped: 51
> HURT: 0
>
> total spills in shared programs: 4370 -> 4124 (-5.63%)
> spills in affected programs: 1397 -> 1151 (-17.61%)
> helped: 9
> HURT: 12
>
> total fills in shared programs: 4581 -> 4419 (-3.54%)
> fills in affected programs: 2201 -> 2039 (-7.36%)
> helped: 9
> HURT: 15
> ---
>  src/compiler/nir/nir_opt_loop_unroll.c | 113 +
>  1 file changed, 76 insertions(+), 37 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_loop_unroll.c
> b/src/compiler/nir/nir_opt_loop_unroll.c
> index 9c33267cb72..fa60523aac7 100644
> --- a/src/compiler/nir/nir_opt_loop_unroll.c
> +++ b/src/compiler/nir/nir_opt_loop_unroll.c
> @@ -67,7 +67,6 @@ loop_prepare_for_unroll(nir_loop *loop)
> /* Remove continue if its the last instruction in the loop */
> nir_instr *last_instr =
> nir_block_last_instr(nir_loop_last_block(loop));
> if (last_instr && last_instr->type == nir_instr_type_jump) {
> -  assert(nir_instr_as_jump(last_instr)->type == nir_jump_continue);
>nir_instr_remove(last_instr);
> }
>  }
> @@ -474,54 +473,91 @@ complex_unroll(nir_loop *loop, nir_loop_terminator
> *unlimit_term,
>  static bool
>  wrapper_unroll(nir_loop *loop)
>  {
> -   bool progress = false;
> -
> -   nir_block *blk_after_loop =
> -  nir_cursor_current_block(nir_after_cf_node(>cf_node));
> -
> -   /* There may still be some single src phis following the loop that
> -* have not yet been cleaned up by another pass. Tidy those up before
> -* unrolling the loop.
> -*/
> -   nir_foreach_instr_safe(instr, blk_after_loop) {
> -  if (instr->type != nir_instr_type_phi)
> - break;
> +   if (!list_empty(>info->loop_terminator_list)) {
>
> -  nir_phi_instr *phi = nir_instr_as_phi(instr);
> -  assert(exec_list_length(>srcs) == 1);
> +  /* Unrolling a loop with a large number of exits can result in a
> +   * large inrease in register pressure. For now we just skip
> +   * unrolling if we have more than 3 exits (not including the break
> +   * at the end of the loop).
> +   *
> +   * TODO: Most loops that fit this pattern are simply switch
> +   * statements that are converted to a loop to take advantage of
> +   * exiting jump instruction handling. In this case we could make
> +   * use of a binary seach pattern like we do in
> +   * nir_lower_indirect_derefs(), this should allow us to unroll the
> +   * loops in an optimal way and should also avoid some of the
> +   * register pressure that comes from simply nesting the
> +   * terminators one after the other.
> +   */
> +  if (list_length(>info->loop_terminator_list) > 3)
> + return false;
> +
> +  loop_prepare_for_unroll(loop);
> +
> +  nir_cursor cursor = nir_after_block(nir_loop_last_block(loop));
>

Maybe call this loop_end; that's less generic than "cursor".


> +  list_for_each_entry(nir_loop_terminator, terminator,
> +  >info->loop_terminator_list,
> +  loop_terminator_link) {
>

I presume the terminator list is guaranteed to be sorted top to bottom?

Reviewed-by: Jason Ekstrand 


> +
> + /* Remove break from the terminator */
> + nir_instr *break_instr =
> +nir_block_last_instr(terminator->break_block);
> + nir_instr_remove(break_instr);
> +
> + /* Pluck out the loop body. */
> + nir_cf_list loop_body;
> + nir_cf_extract(_body,
> +nir_after_cf_node(>nif->cf_node),
> +cursor);
> +
> + /* Reinsert loop body into continue from block */
> + nir_cf_reinsert(_body,
> +
>  nir_after_block(terminator->continue_from_block));
> +
> + cursor = terminator->continue_from_then ?
> +   

Re: [Mesa-dev] [PATCH] i965/icl: Set Enabled Texel Offset Precision Fix bit

[Chris Wilson]

Quoting Anuj Phogat (2018-08-28 18:53:59)
> h/w specification requires this bit to be always set.
> 
> Suggested-by: Kenneth Graunke 
> Signed-off-by: Anuj Phogat 
> ---
>  src/mesa/drivers/dri/i965/brw_defines.h  | 4 
>  src/mesa/drivers/dri/i965/brw_state_upload.c | 7 +++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
> b/src/mesa/drivers/dri/i965/brw_defines.h
> index 433314115b1..1c73ddeb190 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -1673,6 +1673,10 @@ enum brw_pixel_shader_coverage_mask_mode {
>  # define GLK_SCEC_BARRIER_MODE_3D_HULL (1 << 7)
>  # define GLK_SCEC_BARRIER_MODE_MASKREG_MASK(1 << 7)
>  
> +#define HALF_SLICE_CHICKEN70xE194
> +# define TEXEL_OFFSET_FIX_ENABLE   (1 << 1)
> +# define TEXEL_OFFSET_FIX_MASK REG_MASK(1 << 7)

That mask doesn't match the enable-bit.

It'll probably be quite useful to record all the registers you are
setting as part of the global setup and read them back later to make
sure they stuck.
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 07/14] st/mesa: set ctx->Const.SubPixelBits

[Marek Olšák]

On Tue, Aug 28, 2018 at 7:40 AM, Jakob Bornecrantz  wrote:
> On Thu, Aug 9, 2018 at 12:57 AM Marek Olšák  wrote:
>>
>> From: Marek Olšák 
>
> This patch causes regressions in dEQP[1] on virgl running on a
> radeonSI device. Not a lot of drivers set
> PIPE_CAP_VIEWPORT_SUBPIXEL_BITS but SubPixelBits is by default set to
> 4, but this overwrites it without checking if the returned value is
> zero or not. Looking around it seems that a lot of other drivers just
> returns zero for PIPE_CAP_VIEWPORT_SUBPIXEL_BITS not just virgl, so
> this probably causes regressions on more drivers then virgl.

That's a good thing. It shows us that some drivers report some caps incorrectly.

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

[Mesa-dev] [PATCH] anv/icl: Set Enabled Texel Offset Precision Fix bit

[Anuj Phogat]

h/w specification requires this bit to be always set.

Suggested-by: Kenneth Graunke 
Signed-off-by: Anuj Phogat 
---
 src/intel/genxml/gen11.xml|  5 +
 src/intel/vulkan/genX_state.c | 14 ++
 2 files changed, 19 insertions(+)

diff --git a/src/intel/genxml/gen11.xml b/src/intel/genxml/gen11.xml
index 1b3befbbfc9..c69d7dc89c2 100644
--- a/src/intel/genxml/gen11.xml
+++ b/src/intel/genxml/gen11.xml
@@ -3640,4 +3640,9 @@
 
   
 
+  
+
+
+  
+
 
diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c
index d6ccd21524c..2f48a7e1995 100644
--- a/src/intel/vulkan/genX_state.c
+++ b/src/intel/vulkan/genX_state.c
@@ -172,6 +172,20 @@ genX(init_device_state)(struct anv_device *device)
   lri.RegisterOffset = GENX(SAMPLER_MODE_num);
   lri.DataDWord  = sampler_mode;
}
+
+   /* Bit 1 "Enabled Texel Offset Precision Fix" must be set in
+* HALF_SLICE_CHICKEN7 register.
+*/
+   uint32_t half_slice_chicken7;
+   anv_pack_struct(_slice_chicken7, GENX(HALF_SLICE_CHICKEN7),
+   .EnabledTexelOffsetPrecisionFix = true,
+   .EnabledTexelOffsetPrecisionFixMask = true);
+
+anv_batch_emit(, GENX(MI_LOAD_REGISTER_IMM), lri) {
+  lri.RegisterOffset = GENX(HALF_SLICE_CHICKEN7_num);
+  lri.DataDWord  = half_slice_chicken7;
+   }
+
 #endif
 
/* Set the "CONSTANT_BUFFER Address Offset Disable" bit, so
-- 
2.17.1

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

[Mesa-dev] [PATCH] i965/icl: Set Enabled Texel Offset Precision Fix bit

[Anuj Phogat]

h/w specification requires this bit to be always set.

Suggested-by: Kenneth Graunke 
Signed-off-by: Anuj Phogat 
---
 src/mesa/drivers/dri/i965/brw_defines.h  | 4 
 src/mesa/drivers/dri/i965/brw_state_upload.c | 7 +++
 2 files changed, 11 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index 433314115b1..1c73ddeb190 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1673,6 +1673,10 @@ enum brw_pixel_shader_coverage_mask_mode {
 # define GLK_SCEC_BARRIER_MODE_3D_HULL (1 << 7)
 # define GLK_SCEC_BARRIER_MODE_MASKREG_MASK(1 << 7)
 
+#define HALF_SLICE_CHICKEN70xE194
+# define TEXEL_OFFSET_FIX_ENABLE   (1 << 1)
+# define TEXEL_OFFSET_FIX_MASK REG_MASK(1 << 7)
+
 #define GEN11_SAMPLER_MODE  0xE18C
 # define HEADERLESS_MESSAGE_FOR_PREEMPTABLE_CONTEXTS(1 << 5)
 # define HEADERLESS_MESSAGE_FOR_PREEMPTABLE_CONTEXTS_MASK   REG_MASK(1 << 5)
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c 
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index 2af4c45bc44..7f20579fb87 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -72,6 +72,13 @@ brw_upload_initial_gpu_state(struct brw_context *brw)
   brw_load_register_imm32(brw, GEN11_SAMPLER_MODE,
   HEADERLESS_MESSAGE_FOR_PREEMPTABLE_CONTEXTS_MASK 
|
   HEADERLESS_MESSAGE_FOR_PREEMPTABLE_CONTEXTS);
+
+  /* Bit 1 "Enabled Texel Offset Precision Fix" must be set in
+   * HALF_SLICE_CHICKEN7 register.
+   */
+  brw_load_register_imm32(brw, HALF_SLICE_CHICKEN7,
+  TEXEL_OFFSET_FIX_MASK |
+  TEXEL_OFFSET_FIX_ENABLE);
}
 
if (devinfo->gen == 10 || devinfo->gen == 11) {
-- 
2.17.1

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

[Mesa-dev] [Bug 107477] [DXVK] Setting high shader quality in GTA V results in LLVM error

[bugzilla-daemon]

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

--- Comment #13 from Clément Guérin  ---
Let's try again:
https://send.firefox.com/download/0a9b6178c5/#mjwYjQDZZbGvFqAKq5fhig

-- 
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 3/3] anv: Claim to support depthBounds for ID games

[Lionel Landwerlin]

On 27/08/2018 23:48, Jason Ekstrand wrote:
On Mon, Aug 27, 2018 at 12:50 PM Lionel Landwerlin 
mailto:lionel.g.landwer...@intel.com>> 
wrote:


On 23/08/2018 16:13, Jason Ekstrand wrote:
> ---
>   src/intel/vulkan/anv_device.c | 9 +
>   1 file changed, 9 insertions(+)
>
> diff --git a/src/intel/vulkan/anv_device.c
b/src/intel/vulkan/anv_device.c
> index 0357fc7c0ea..5a63592f7ca 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -854,6 +854,15 @@ void anv_GetPhysicalDeviceFeatures(
>      pFeatures->vertexPipelineStoresAndAtomics =
>  pdevice->compiler->scalar_stage[MESA_SHADER_VERTEX] &&
>  pdevice->compiler->scalar_stage[MESA_SHADER_GEOMETRY];
> +
> +   struct anv_app_info *app_info = >instance->app_info;
> +
> +   /* The new DOOM and Wolfenstein games require depthBounds
without
> +    * checking for it.  They seem to run fine without it so
just claim it's
> +    * there and accept the consequences.
> +    */
> +   if (app_info->engine_name && strcmp(app_info->engine_name,
"idTech") == 0)
> +      pFeatures->depthBounds = true;
>   }
>
>   void anv_GetPhysicalDeviceFeatures2(

I'm struggling to understand "require depthBounds with checking
for it".
I thought one would check for the feature by reading the boolean you
just set. So if it's not checked why would it make a difference?


The Vulkan spec requires that we throw VK_ERROR_INITIALIZATION_FAILED 
if the client enables any features we have not advertised.  This gives 
us two options: 1) advertise support for depthBounds for idTech games 
or 2) Skip the check for depthBounds in CreateDevice and let it 
succeed even if it's enabled for idTech games.  This patch takes the 
former approach since it's easier.


--Jason


Oh thanks for the explanation.
With the strdup() fix this series is :

Reviewed-by: Lionel Landwerlin 

I remember talks about doing this kind of tricks for particular titles 
in a layer.


It doesn't necessarily makes things easier for us, but are you still 
thinking about that layer mechanism?



-

Lionel

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

Re: [Mesa-dev] [PATCH v4 5/7] nir/opt_loop_unroll: Remove unneeded phis if we make progress

[Jason Ekstrand]

I'm not a huge fan of calling copy-prop here but I also don't see an easy
way around it.

Reviewed-by: Jason Ekstrand 

On Mon, Aug 27, 2018 at 4:09 AM Timothy Arceri 
wrote:

> Now that SSA values can be derefs and they have special rules, we have
> to be a bit more careful about our LCSSA phis.  In particular, we need
> to clean up in case LCSSA ended up creating a phi node for a deref.
> This avoids validation issues with some CTS tests with the following
> patch, but its possible this we could also see the same problem with
> the existing unrolling passes.
> ---
>  src/compiler/nir/nir_opt_loop_unroll.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/src/compiler/nir/nir_opt_loop_unroll.c
> b/src/compiler/nir/nir_opt_loop_unroll.c
> index a1ad0e59814..e0e0b754716 100644
> --- a/src/compiler/nir/nir_opt_loop_unroll.c
> +++ b/src/compiler/nir/nir_opt_loop_unroll.c
> @@ -575,9 +575,17 @@ nir_opt_loop_unroll_impl(nir_function_impl *impl,
>  _nested_loop);
> }
>
> -   if (progress)
> +   if (progress) {
>nir_lower_regs_to_ssa_impl(impl);
>
> +  /* Calling nir_convert_loop_to_lcssa() adds extra phi nodes which
> may
> +   * not be valid if they're used for something such as a deref.
> +   *  Remove any unneeded phis.
> +   */
> +  nir_copy_prop(impl->function->shader);
> +  nir_opt_remove_phis_impl(impl);
> +   }
> +
> return progress;
>  }
>
> --
> 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 v4 6/7] nir: add loop unroll support for wrapper loops

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand 

On Mon, Aug 27, 2018 at 4:09 AM Timothy Arceri 
wrote:

> This adds support for unrolling the classic
>
> do {
> // ...
> } while (false)
>
> that is used to wrap multi-line macros. GLSL IR also wraps switch
> statements in a loop like this.
>
> shader-db results IVB:
>
> total loops in shared programs: 2515 -> 2512 (-0.12%)
> loops in affected programs: 33 -> 30 (-9.09%)
> helped: 3
> HURT: 0
> ---
>  src/compiler/nir/nir_opt_loop_unroll.c | 77 ++
>  1 file changed, 77 insertions(+)
>
> diff --git a/src/compiler/nir/nir_opt_loop_unroll.c
> b/src/compiler/nir/nir_opt_loop_unroll.c
> index e0e0b754716..9c33267cb72 100644
> --- a/src/compiler/nir/nir_opt_loop_unroll.c
> +++ b/src/compiler/nir/nir_opt_loop_unroll.c
> @@ -465,6 +465,65 @@ complex_unroll(nir_loop *loop, nir_loop_terminator
> *unlimit_term,
> _mesa_hash_table_destroy(remap_table, NULL);
>  }
>
> +/* Unrolls the classic wrapper loops e.g
> + *
> + *do {
> + *// ...
> + *} while (false)
> + */
> +static bool
> +wrapper_unroll(nir_loop *loop)
> +{
> +   bool progress = false;
> +
> +   nir_block *blk_after_loop =
> +  nir_cursor_current_block(nir_after_cf_node(>cf_node));
> +
> +   /* There may still be some single src phis following the loop that
> +* have not yet been cleaned up by another pass. Tidy those up before
> +* unrolling the loop.
> +*/
> +   nir_foreach_instr_safe(instr, blk_after_loop) {
> +  if (instr->type != nir_instr_type_phi)
> + break;
> +
> +  nir_phi_instr *phi = nir_instr_as_phi(instr);
> +  assert(exec_list_length(>srcs) == 1);
> +
> +  nir_phi_src *phi_src = exec_node_data(nir_phi_src,
> +
> exec_list_get_head(>srcs),
> +node);
> +
> +  nir_ssa_def_rewrite_uses(>dest.ssa, phi_src->src);
> +  nir_instr_remove(instr);
> +
> +  progress = true;
> +   }
> +
> +   nir_block *last_loop_blk = nir_loop_last_block(loop);
> +   if (nir_block_ends_in_break(last_loop_blk)) {
> +
> +  /* Remove break at end of the loop */
> +  nir_instr *break_instr = nir_block_last_instr(last_loop_blk);
> +  nir_instr_remove(break_instr);
> +
> +  /* Pluck out the loop body. */
> +  nir_cf_list loop_body;
> +  nir_cf_extract(_body,
> nir_before_block(nir_loop_first_block(loop)),
> + nir_after_block(nir_loop_last_block(loop)));
> +
> +  /* Reinsert loop body after the loop */
> +  nir_cf_reinsert(_body, nir_after_cf_node(>cf_node));
> +
> +  /* The loop has been unrolled so remove it. */
> +  nir_cf_node_remove(>cf_node);
> +
> +  progress = true;
> +   }
> +
> +   return progress;
> +}
> +
>  static bool
>  is_loop_small_enough_to_unroll(nir_shader *shader, nir_loop_info *li)
>  {
> @@ -516,6 +575,24 @@ process_loops(nir_shader *sh, nir_cf_node *cf_node,
> bool *has_nested_loop_out)
>  */
> if (!progress) {
>
> +  /* Check for the classic
> +   *
> +   *do {
> +   *// ...
> +   *} while (false)
> +   *
> +   * that is used to wrap multi-line macros. GLSL IR also wraps switch
> +   * statements in a loop like this.
> +   */
> +  if (loop->info->limiting_terminator == NULL &&
> +  list_empty(>info->loop_terminator_list) &&
> +  !loop->info->complex_loop) {
> +
> + progress = wrapper_unroll(loop);
> +
> + goto exit;
> +  }
> +
>if (has_nested_loop || loop->info->limiting_terminator == NULL)
>   goto exit;
>
> --
> 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] anv: blorp: support multiple aspect blits

[Lionel Landwerlin]

Thanks a lot!

Looks like reviewing patches on gitlab could really help ;)

-
Lionel

On 28/08/2018 18:18, Jason Ekstrand wrote:

Assuming nothing got lost in the indent, looks good to me.

Reviewed-by: Jason Ekstrand >


On Tue, Aug 28, 2018 at 6:27 AM Lionel Landwerlin 
mailto:lionel.g.landwer...@intel.com>> 
wrote:


Newer blit tests are enabling depth blits. We currently don't
support it but can do by iterating over the aspects masks (copy some
logic from the CopyImage function).

Signed-off-by: Lionel Landwerlin mailto:lionel.g.landwer...@intel.com>>
Fixes: 9f44745eca0e41 ("anv: Use blorp to implement VkBlitImage")
---
 src/intel/vulkan/anv_blorp.c | 145
++-
 1 file changed, 75 insertions(+), 70 deletions(-)

diff --git a/src/intel/vulkan/anv_blorp.c
b/src/intel/vulkan/anv_blorp.c
index cd67cc636b2..35b304f92b3 100644
--- a/src/intel/vulkan/anv_blorp.c
+++ b/src/intel/vulkan/anv_blorp.c
@@ -532,81 +532,86 @@ void anv_CmdBlitImage(
       const VkImageSubresourceLayers *src_res =
[r].srcSubresource;
       const VkImageSubresourceLayers *dst_res =
[r].dstSubresource;

-      get_blorp_surf_for_anv_image(cmd_buffer->device,
-                                   src_image, src_res->aspectMask,
-                                   srcImageLayout,
ISL_AUX_USAGE_NONE, );
-      get_blorp_surf_for_anv_image(cmd_buffer->device,
-                                   dst_image, dst_res->aspectMask,
-                                   dstImageLayout,
ISL_AUX_USAGE_NONE, );
-
-      struct anv_format_plane src_format =
-  anv_get_format_plane(_buffer->device->info,
src_image->vk_format,
-                              src_res->aspectMask,
src_image->tiling);
-      struct anv_format_plane dst_format =
-  anv_get_format_plane(_buffer->device->info,
dst_image->vk_format,
-                              dst_res->aspectMask,
dst_image->tiling);
-
-      unsigned dst_start, dst_end;
-      if (dst_image->type == VK_IMAGE_TYPE_3D) {
-         assert(dst_res->baseArrayLayer == 0);
-         dst_start = pRegions[r].dstOffsets[0].z;
-         dst_end = pRegions[r].dstOffsets[1].z;
-      } else {
-         dst_start = dst_res->baseArrayLayer;
-         dst_end = dst_start + anv_get_layerCount(dst_image,
dst_res);
-      }
-
-      unsigned src_start, src_end;
-      if (src_image->type == VK_IMAGE_TYPE_3D) {
-         assert(src_res->baseArrayLayer == 0);
-         src_start = pRegions[r].srcOffsets[0].z;
-         src_end = pRegions[r].srcOffsets[1].z;
-      } else {
-         src_start = src_res->baseArrayLayer;
-         src_end = src_start + anv_get_layerCount(src_image,
src_res);
-      }
-
-      bool flip_z = flip_coords(_start, _end, _start,
_end);
-      float src_z_step = (float)(src_end + 1 - src_start) /
-                         (float)(dst_end + 1 - dst_start);
+ assert(anv_image_aspects_compatible(src_res->aspectMask,
+ dst_res->aspectMask));
+
+      uint32_t aspect_bit;
+      anv_foreach_image_aspect_bit(aspect_bit, src_image,
src_res->aspectMask) {
+         get_blorp_surf_for_anv_image(cmd_buffer->device,
+                                      src_image, 1U << aspect_bit,
+                                      srcImageLayout,
ISL_AUX_USAGE_NONE, );
+         get_blorp_surf_for_anv_image(cmd_buffer->device,
+                                      dst_image, 1U << aspect_bit,
+                                      dstImageLayout,
ISL_AUX_USAGE_NONE, );
+
+         struct anv_format_plane src_format =
+ anv_get_format_plane(_buffer->device->info,
src_image->vk_format,
+                                 1U << aspect_bit,
src_image->tiling);
+         struct anv_format_plane dst_format =
+ anv_get_format_plane(_buffer->device->info,
dst_image->vk_format,
+                                 1U << aspect_bit,
dst_image->tiling);
+
+         unsigned dst_start, dst_end;
+         if (dst_image->type == VK_IMAGE_TYPE_3D) {
+            assert(dst_res->baseArrayLayer == 0);
+            dst_start = pRegions[r].dstOffsets[0].z;
+            dst_end = pRegions[r].dstOffsets[1].z;
+         } else {
+            dst_start = dst_res->baseArrayLayer;
+            dst_end = dst_start + anv_get_layerCount(dst_image,
dst_res);
+         }

-      if (flip_z) {
-         src_start = src_end;
-         src_z_step *= -1;
-      }
+         unsigned src_start, src_end;
+         if (src_image->type == VK_IMAGE_TYPE_3D) {
+            assert(src_res->baseArrayLayer == 0);
+            src_start = pRegions[r].srcOffsets[0].z;
+            

Re: [Mesa-dev] [PATCH] anv: blorp: support multiple aspect blits

[Jason Ekstrand]

Assuming nothing got lost in the indent, looks good to me.

Reviewed-by: Jason Ekstrand 

On Tue, Aug 28, 2018 at 6:27 AM Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> Newer blit tests are enabling depth blits. We currently don't
> support it but can do by iterating over the aspects masks (copy some
> logic from the CopyImage function).
>
> Signed-off-by: Lionel Landwerlin 
> Fixes: 9f44745eca0e41 ("anv: Use blorp to implement VkBlitImage")
> ---
>  src/intel/vulkan/anv_blorp.c | 145 ++-
>  1 file changed, 75 insertions(+), 70 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index cd67cc636b2..35b304f92b3 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -532,81 +532,86 @@ void anv_CmdBlitImage(
>const VkImageSubresourceLayers *src_res =
> [r].srcSubresource;
>const VkImageSubresourceLayers *dst_res =
> [r].dstSubresource;
>
> -  get_blorp_surf_for_anv_image(cmd_buffer->device,
> -   src_image, src_res->aspectMask,
> -   srcImageLayout, ISL_AUX_USAGE_NONE,
> );
> -  get_blorp_surf_for_anv_image(cmd_buffer->device,
> -   dst_image, dst_res->aspectMask,
> -   dstImageLayout, ISL_AUX_USAGE_NONE,
> );
> -
> -  struct anv_format_plane src_format =
> - anv_get_format_plane(_buffer->device->info,
> src_image->vk_format,
> -  src_res->aspectMask, src_image->tiling);
> -  struct anv_format_plane dst_format =
> - anv_get_format_plane(_buffer->device->info,
> dst_image->vk_format,
> -  dst_res->aspectMask, dst_image->tiling);
> -
> -  unsigned dst_start, dst_end;
> -  if (dst_image->type == VK_IMAGE_TYPE_3D) {
> - assert(dst_res->baseArrayLayer == 0);
> - dst_start = pRegions[r].dstOffsets[0].z;
> - dst_end = pRegions[r].dstOffsets[1].z;
> -  } else {
> - dst_start = dst_res->baseArrayLayer;
> - dst_end = dst_start + anv_get_layerCount(dst_image, dst_res);
> -  }
> -
> -  unsigned src_start, src_end;
> -  if (src_image->type == VK_IMAGE_TYPE_3D) {
> - assert(src_res->baseArrayLayer == 0);
> - src_start = pRegions[r].srcOffsets[0].z;
> - src_end = pRegions[r].srcOffsets[1].z;
> -  } else {
> - src_start = src_res->baseArrayLayer;
> - src_end = src_start + anv_get_layerCount(src_image, src_res);
> -  }
> -
> -  bool flip_z = flip_coords(_start, _end, _start,
> _end);
> -  float src_z_step = (float)(src_end + 1 - src_start) /
> - (float)(dst_end + 1 - dst_start);
> +  assert(anv_image_aspects_compatible(src_res->aspectMask,
> +  dst_res->aspectMask));
> +
> +  uint32_t aspect_bit;
> +  anv_foreach_image_aspect_bit(aspect_bit, src_image,
> src_res->aspectMask) {
> + get_blorp_surf_for_anv_image(cmd_buffer->device,
> +  src_image, 1U << aspect_bit,
> +  srcImageLayout, ISL_AUX_USAGE_NONE,
> );
> + get_blorp_surf_for_anv_image(cmd_buffer->device,
> +  dst_image, 1U << aspect_bit,
> +  dstImageLayout, ISL_AUX_USAGE_NONE,
> );
> +
> + struct anv_format_plane src_format =
> +anv_get_format_plane(_buffer->device->info,
> src_image->vk_format,
> + 1U << aspect_bit, src_image->tiling);
> + struct anv_format_plane dst_format =
> +anv_get_format_plane(_buffer->device->info,
> dst_image->vk_format,
> + 1U << aspect_bit, dst_image->tiling);
> +
> + unsigned dst_start, dst_end;
> + if (dst_image->type == VK_IMAGE_TYPE_3D) {
> +assert(dst_res->baseArrayLayer == 0);
> +dst_start = pRegions[r].dstOffsets[0].z;
> +dst_end = pRegions[r].dstOffsets[1].z;
> + } else {
> +dst_start = dst_res->baseArrayLayer;
> +dst_end = dst_start + anv_get_layerCount(dst_image, dst_res);
> + }
>
> -  if (flip_z) {
> - src_start = src_end;
> - src_z_step *= -1;
> -  }
> + unsigned src_start, src_end;
> + if (src_image->type == VK_IMAGE_TYPE_3D) {
> +assert(src_res->baseArrayLayer == 0);
> +src_start = pRegions[r].srcOffsets[0].z;
> +src_end = pRegions[r].srcOffsets[1].z;
> + } else {
> +src_start = src_res->baseArrayLayer;
> +src_end = src_start + anv_get_layerCount(src_image, src_res);
> + }
>
> -  unsigned src_x0 = pRegions[r].srcOffsets[0].x;
> -  unsigned src_x1 = pRegions[r].srcOffsets[1].x;
> -  unsigned dst_x0 = 

Re: [Mesa-dev] [1/2] mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering.

[Eric Engestrom]

On Tuesday, 2018-08-28 17:59:15 +0100, Rogovin, Kevin wrote:
> Hi,
> 
> Sighs; I missed that warning on the build since there is so much build
> output.

Fair enough; I'll send a few patches to drastically cut the build
warnings later today/tomorrow :)

> I can issue a small patch to handle the missing enum.

Sounds good for now, but you might want to consider Francisco's
suggestion:
https://lists.freedesktop.org/archives/mesa-dev/2018-August/203767.html
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Jason Ekstrand]

"Works on an app" is not and never has been an acceptable testing plan for
landing a new feature in Mesa.  There need to be tests in piglit, dEQP, or
one of the conformance suites.

--Jason

On Tue, Aug 28, 2018 at 11:15 AM Rogovin, Kevin 
wrote:

> Hi,
>
>  On the questions of tests: If people want, I can adapt the test in piglit
> for ARB_fragment_shader_interlock to this INTEL one. In general, I have an
> app/library that uses the extension and testing of that does definitely
> work on i965 (which should be utterly unsurprising since the produced
> assembly is identical). Indeed the current implementation in Mesa of
> ARB_fragment_shader_interlock is as flexible as the INTEL one since the
> compiler accepts code with the begin/end interlock anywhere since the only
> backend that supports it, i965, can easily accept it. I view the
> INTEL_fragment_shader_ordering implementation in a similar light as the
> NV_fragment_shader_interlock where it actually does not really do anything
> new, but signals to an application that the Mesa/i965 implementation allows
> more (and does it correctly) than the ARB/NV extensions restrict to.
>
> -Kevin
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Jason Ekstrand]

On Tue, Aug 28, 2018 at 11:22 AM Manolova, Plamena <
plamena.manol...@intel.com> wrote:

> Hi Mark,
> AFAIK there is no piglit test for this specific extension. However
> underneath the hood it reuses the
> functionality of ARB_fragment_shader_interlock, which has a test. I
> believe the only major difference between
> the two extensions is that unlike beginInvocationInterlockARB, 
> beginFragmentShaderOrderingINTEL
> can
> be called from functions other than main(). If necessary it would be
> pretty straightforward to reuse most of the code
> for the ARB_fragment_shader_interlock test to create one
> for INTEL_fragment_shader_ordering.
>

Having the same underlying compiler intrinsic and having the same behavior
are not the same thing.  The INTEL extension allows strictly more
functionality than the ARB extension so it needs even more testing.  In
particular, it allows you to do the barrier in non-uniform control-flow
which is a very different thing than at the top of a shader like is allowed
by ARB_fragment_shader_interlock.  I expect the INTEL extension to need
substantially more testing than the ARB one.

--Jason



> Thank you,
> Pam
>
> On Tue, Aug 28, 2018 at 6:41 PM Mark Janes  wrote:
>
>> Is there a piglit test that verifies that this feature works properly?
>>
>>  writes:
>>
>> > From: Kevin Rogovin 
>> >
>> > INTEL_fragment_shader_ordering provides the ability for shaders
>> > to issue a call to gaurnantee memory write operation ordering
>> > of overlapping pixels or samples. In contrast to
>> > ARB_fragment_shader_interlock, INTEL_fragment_shader_ordering
>> > instead of defining a critical region (which must be in main() and
>> > under no flow control) provides a single function that acts like
>> > a memory barrier that can be called under any control flow.
>> >
>> > Kevin Rogovin (2):
>> >   mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering.
>> >   i965: Add INTEL_fragment_shader_ordering support.
>> >
>> >  docs/relnotes/18.3.0.html|  1 +
>> >  src/compiler/glsl/builtin_functions.cpp  | 17 +
>> >  src/compiler/glsl/glsl_parser_extras.cpp |  1 +
>> >  src/compiler/glsl/glsl_parser_extras.h   |  2 ++
>> >  src/compiler/glsl/glsl_to_nir.cpp|  6 ++
>> >  src/compiler/glsl/ir.h   |  1 +
>> >  src/compiler/nir/nir_intrinsics.py   |  1 +
>> >  src/intel/compiler/brw_fs_nir.cpp|  1 +
>> >  src/mesa/drivers/dri/i965/intel_extensions.c |  1 +
>> >  src/mesa/main/extensions_table.h |  1 +
>> >  src/mesa/main/mtypes.h   |  1 +
>> >  11 files changed, 33 insertions(+)
>> >
>> > --
>> > 2.17.1
>> >
>> > ___
>> > mesa-dev mailing list
>> > mesa-dev@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [1/2] mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering.

[Rogovin, Kevin]

Hi,

Sighs; I missed that warning on the build since there is so much build output. 
I can issue a small patch to handle the missing enum.

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

Re: [Mesa-dev] [PATCH mesa] meson: consolidate langs lists

[Dylan Baker]

Quoting Eric Engestrom (2018-08-28 09:18:09)
> Signed-off-by: Eric Engestrom 
> ---
>  src/util/xmlpool/meson.build | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/xmlpool/meson.build b/src/util/xmlpool/meson.build
> index 3d2de0cdc3a5fc45d1e4..8bdabcb825b776d06182 100644
> --- a/src/util/xmlpool/meson.build
> +++ b/src/util/xmlpool/meson.build
> @@ -18,14 +18,20 @@
>  # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
> THE
>  # SOFTWARE.
>  
> +_langs = ['ca', 'es', 'de', 'nl', 'sv', 'fr']
> +
> +_langs_po_files = []
> +foreach lang : _langs
> +  _langs_po_files += files(lang + '.po')
> +endforeach
> +
>  xmlpool_options_h = custom_target(
>'xmlpool_options.h',
>input : ['gen_xmlpool.py', 't_options.h'],
>output : 'options.h',
>command : [
> -prog_python, '@INPUT@', meson.current_source_dir(),
> -'ca', 'es', 'de', 'nl', 'sv', 'fr',
> +prog_python, '@INPUT@', meson.current_source_dir(), _langs,
>],
>capture : true,
> -  depend_files : files('ca.po', 'es.po', 'de.po', 'nl.po', 'sv.po', 'fr.po'),
> +  depend_files : _langs_po_files,
>  )
> -- 
> Cheers,
>   Eric
> 

Reviewed-by: Dylan Baker 


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

Re: [Mesa-dev] [1/2] mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering.

[Eric Engestrom]

On Monday, 2018-08-27 09:54:23 +0300, kevin.rogo...@intel.com wrote:
> From: Kevin Rogovin 
> 
>This extension provides new GLSL built-in function
>beginFragmentShaderOrderingIntel() that guarantees
>(taking wording of GL_INTEL_fragment_shader_ordering
>extension) that any memory transactions issued by
>shader invocations from previous primitives mapped to
>same xy window coordinates (and same sample when
>per-sample shading is active), complete and are visible
>to the shader invocation that called
>beginFragmentShaderOrderingINTEL().
> 
>One advantage of INTEL_fragment_shader_ordering over
>ARB_fragment_shader_interlock is that it provides a
>function that operates as a memory barrie (instead
>of a defining a critcial section) that can be called
>under arbitary control flow from any function (in
>contrast the begin/end of ARB_fragment_shader_interlock
>may only be called once, from main(), under no control
>flow.
> 
> Signed-off-by: Kevin Rogovin 
> ---
>  src/compiler/glsl/builtin_functions.cpp  | 17 +
>  src/compiler/glsl/glsl_parser_extras.cpp |  1 +
>  src/compiler/glsl/glsl_parser_extras.h   |  2 ++
>  src/compiler/glsl/glsl_to_nir.cpp|  6 ++
>  src/compiler/glsl/ir.h   |  1 +
>  src/compiler/nir/nir_intrinsics.py   |  1 +
>  src/mesa/main/extensions_table.h |  1 +
>  src/mesa/main/mtypes.h   |  1 +
>  8 files changed, 30 insertions(+)
> 
> diff --git a/src/compiler/glsl/builtin_functions.cpp 
> b/src/compiler/glsl/builtin_functions.cpp
> index b601880686..5650365d1d 100644
> --- a/src/compiler/glsl/builtin_functions.cpp
> +++ b/src/compiler/glsl/builtin_functions.cpp
> @@ -525,6 +525,12 @@ supports_nv_fragment_shader_interlock(const 
> _mesa_glsl_parse_state *state)
> return state->NV_fragment_shader_interlock_enable;
>  }
>  
> +static bool
> +supports_intel_fragment_shader_ordering(const _mesa_glsl_parse_state *state)
> +{
> +   return state->INTEL_fragment_shader_ordering_enable;
> +}
> +
>  static bool
>  shader_clock(const _mesa_glsl_parse_state *state)
>  {
> @@ -1305,6 +1311,11 @@ builtin_builder::create_intrinsics()
> supports_arb_fragment_shader_interlock,
> ir_intrinsic_end_invocation_interlock), NULL);
>  
> +   add_function("__intrinsic_begin_fragment_shader_ordering",
> +_invocation_interlock_intrinsic(
> +   supports_intel_fragment_shader_ordering,
> +   ir_intrinsic_begin_fragment_shader_ordering), NULL);
> +
> add_function("__intrinsic_shader_clock",
>  _shader_clock_intrinsic(shader_clock,
>  glsl_type::uvec2_type),
> @@ -3419,6 +3430,12 @@ builtin_builder::create_builtins()
> supports_nv_fragment_shader_interlock),
>  NULL);
>  
> +   add_function("beginFragmentShaderOrderingINTEL",
> +_invocation_interlock(
> +   "__intrinsic_begin_fragment_shader_ordering",
> +   supports_intel_fragment_shader_ordering),
> +NULL);
> +
> add_function("anyInvocationARB",
>  _vote("__intrinsic_vote_any", vote),
>  NULL);
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
> b/src/compiler/glsl/glsl_parser_extras.cpp
> index 0a7d0d78b1..21d4122444 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -725,6 +725,7 @@ static const _mesa_glsl_extension 
> _mesa_glsl_supported_extensions[] = {
> EXT_AEP(EXT_texture_buffer),
> EXT_AEP(EXT_texture_cube_map_array),
> EXT(INTEL_conservative_rasterization),
> +   EXT(INTEL_fragment_shader_ordering),
> EXT(INTEL_shader_atomic_float_minmax),
> EXT(MESA_shader_integer_functions),
> EXT(NV_fragment_shader_interlock),
> diff --git a/src/compiler/glsl/glsl_parser_extras.h 
> b/src/compiler/glsl/glsl_parser_extras.h
> index 2c8353214a..e03b34d7d6 100644
> --- a/src/compiler/glsl/glsl_parser_extras.h
> +++ b/src/compiler/glsl/glsl_parser_extras.h
> @@ -812,6 +812,8 @@ struct _mesa_glsl_parse_state {
> bool EXT_texture_cube_map_array_warn;
> bool INTEL_conservative_rasterization_enable;
> bool INTEL_conservative_rasterization_warn;
> +   bool INTEL_fragment_shader_ordering_enable;
> +   bool INTEL_fragment_shader_ordering_warn;
> bool INTEL_shader_atomic_float_minmax_enable;
> bool INTEL_shader_atomic_float_minmax_warn;
> bool MESA_shader_integer_functions_enable;
> diff --git a/src/compiler/glsl/glsl_to_nir.cpp 
> b/src/compiler/glsl/glsl_to_nir.cpp
> index a53000f47e..efbb2317ac 100644
> --- a/src/compiler/glsl/glsl_to_nir.cpp
> +++ b/src/compiler/glsl/glsl_to_nir.cpp
> @@ -742,6 +742,9 @@ nir_visitor::visit(ir_call *ir)
>case ir_intrinsic_end_invocation_interlock:
>   op = nir_intrinsic_end_invocation_interlock;
>

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Mark Janes]

"Manolova, Plamena"  writes:

> Hi Mark,
> AFAIK there is no piglit test for this specific extension. However
> underneath the hood it reuses the functionality of
> ARB_fragment_shader_interlock, which has a test. I believe the only
> major difference between the two extensions is that unlike
> beginInvocationInterlockARB, beginFragmentShaderOrderingINTEL can be
> called from functions other than main(). If necessary it would be
> pretty straightforward to reuse most of the code for the
> ARB_fragment_shader_interlock test to create one for
> INTEL_fragment_shader_ordering.

Sounds like a good plan!

> Thank you,
> Pam
>
> On Tue, Aug 28, 2018 at 6:41 PM Mark Janes  wrote:
>
>> Is there a piglit test that verifies that this feature works properly?
>>
>>  writes:
>>
>> > From: Kevin Rogovin 
>> >
>> > INTEL_fragment_shader_ordering provides the ability for shaders
>> > to issue a call to gaurnantee memory write operation ordering
>> > of overlapping pixels or samples. In contrast to
>> > ARB_fragment_shader_interlock, INTEL_fragment_shader_ordering
>> > instead of defining a critical region (which must be in main() and
>> > under no flow control) provides a single function that acts like
>> > a memory barrier that can be called under any control flow.
>> >
>> > Kevin Rogovin (2):
>> >   mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering.
>> >   i965: Add INTEL_fragment_shader_ordering support.
>> >
>> >  docs/relnotes/18.3.0.html|  1 +
>> >  src/compiler/glsl/builtin_functions.cpp  | 17 +
>> >  src/compiler/glsl/glsl_parser_extras.cpp |  1 +
>> >  src/compiler/glsl/glsl_parser_extras.h   |  2 ++
>> >  src/compiler/glsl/glsl_to_nir.cpp|  6 ++
>> >  src/compiler/glsl/ir.h   |  1 +
>> >  src/compiler/nir/nir_intrinsics.py   |  1 +
>> >  src/intel/compiler/brw_fs_nir.cpp|  1 +
>> >  src/mesa/drivers/dri/i965/intel_extensions.c |  1 +
>> >  src/mesa/main/extensions_table.h |  1 +
>> >  src/mesa/main/mtypes.h   |  1 +
>> >  11 files changed, 33 insertions(+)
>> >
>> > --
>> > 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 0/2] Implement INTEL_fragment_shader_ordering

[Eero Tamminen]

Hi,

On 28.08.2018 19:15, Rogovin, Kevin wrote:

  On the questions of tests: If people want, I can adapt the test in piglit for 
ARB_fragment_shader_interlock to this INTEL one. In general, I have an 
app/library that uses the extension and testing of that does definitely work on 
i965 (which should be utterly unsurprising since the produced assembly is 
identical).


Did you mean this library:
https://01.org/fast-ui-draw
?


- Eero


Indeed the current implementation in Mesa of ARB_fragment_shader_interlock is 
as flexible as the INTEL one since the compiler accepts code with the begin/end 
interlock anywhere since the only backend that supports it, i965, can easily 
accept it. I view the INTEL_fragment_shader_ordering implementation in a 
similar light as the NV_fragment_shader_interlock where it actually does not 
really do anything new, but signals to an application that the Mesa/i965 
implementation allows more (and does it correctly) than the ARB/NV extensions 
restrict to.

-Kevin
___
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 0/2] Implement INTEL_fragment_shader_ordering

[Manolova, Plamena]

Hi Mark,
AFAIK there is no piglit test for this specific extension. However
underneath the hood it reuses the
functionality of ARB_fragment_shader_interlock, which has a test. I believe
the only major difference between
the two extensions is that unlike beginInvocationInterlockARB,
beginFragmentShaderOrderingINTEL
can
be called from functions other than main(). If necessary it would be pretty
straightforward to reuse most of the code
for the ARB_fragment_shader_interlock test to create one
for INTEL_fragment_shader_ordering.

Thank you,
Pam

On Tue, Aug 28, 2018 at 6:41 PM Mark Janes  wrote:

> Is there a piglit test that verifies that this feature works properly?
>
>  writes:
>
> > From: Kevin Rogovin 
> >
> > INTEL_fragment_shader_ordering provides the ability for shaders
> > to issue a call to gaurnantee memory write operation ordering
> > of overlapping pixels or samples. In contrast to
> > ARB_fragment_shader_interlock, INTEL_fragment_shader_ordering
> > instead of defining a critical region (which must be in main() and
> > under no flow control) provides a single function that acts like
> > a memory barrier that can be called under any control flow.
> >
> > Kevin Rogovin (2):
> >   mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering.
> >   i965: Add INTEL_fragment_shader_ordering support.
> >
> >  docs/relnotes/18.3.0.html|  1 +
> >  src/compiler/glsl/builtin_functions.cpp  | 17 +
> >  src/compiler/glsl/glsl_parser_extras.cpp |  1 +
> >  src/compiler/glsl/glsl_parser_extras.h   |  2 ++
> >  src/compiler/glsl/glsl_to_nir.cpp|  6 ++
> >  src/compiler/glsl/ir.h   |  1 +
> >  src/compiler/nir/nir_intrinsics.py   |  1 +
> >  src/intel/compiler/brw_fs_nir.cpp|  1 +
> >  src/mesa/drivers/dri/i965/intel_extensions.c |  1 +
> >  src/mesa/main/extensions_table.h |  1 +
> >  src/mesa/main/mtypes.h   |  1 +
> >  11 files changed, 33 insertions(+)
> >
> > --
> > 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 0/2] Implement INTEL_fragment_shader_ordering

[Rogovin, Kevin]

Hi,

Sorry for the doubly reply, but I wanted to add one more important bit in my 
thinking in addition to doing minimal code changes and following existing 
convention. The ARB/NV extensions defined a critical section where as the INTEL 
extension is just a barrier function. I suspect (but I do not know) that the 
NVIDIA hardware that supports the extension is implemented by some weird 
interfacing logic with the pixel backend and that implementation requires that 
there is just one critical section under no control flow whereas the INTEL is 
just a memory barrier. In that light, I think they should be 3 different 
intrinsics with the thinking that if the Nouveau driver adds support for the 
ARB/NV extension it will do the IR analysis to do what is needed for the 
critical-section style extension.

-Kevin

From: Rogovin, Kevin
Sent: Tuesday, August 28, 2018 7:05 PM
To: mesa-dev@lists.freedesktop.org
Cc: Jerez Plata, Francisco 
Subject: Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

Hi,

I followed the convention that was already present. The code from 
ARB/NV_fragment_shader_interlock had an intrinsic for begin critical section 
and an intrinsic for end critical section. I figured that since this extension 
has a completely different thinking (i.e. a memory barrier instead of a 
section) it warranted a new intrinsic. That and doing this way incurred the 
least amount of changes which I figured is always a good thing.

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

[Mesa-dev] [PATCH mesa] meson: consolidate langs lists

[Eric Engestrom]

Signed-off-by: Eric Engestrom 
---
 src/util/xmlpool/meson.build | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/util/xmlpool/meson.build b/src/util/xmlpool/meson.build
index 3d2de0cdc3a5fc45d1e4..8bdabcb825b776d06182 100644
--- a/src/util/xmlpool/meson.build
+++ b/src/util/xmlpool/meson.build
@@ -18,14 +18,20 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
+_langs = ['ca', 'es', 'de', 'nl', 'sv', 'fr']
+
+_langs_po_files = []
+foreach lang : _langs
+  _langs_po_files += files(lang + '.po')
+endforeach
+
 xmlpool_options_h = custom_target(
   'xmlpool_options.h',
   input : ['gen_xmlpool.py', 't_options.h'],
   output : 'options.h',
   command : [
-prog_python, '@INPUT@', meson.current_source_dir(),
-'ca', 'es', 'de', 'nl', 'sv', 'fr',
+prog_python, '@INPUT@', meson.current_source_dir(), _langs,
   ],
   capture : true,
-  depend_files : files('ca.po', 'es.po', 'de.po', 'nl.po', 'sv.po', 'fr.po'),
+  depend_files : _langs_po_files,
 )
-- 
Cheers,
  Eric

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

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Rogovin, Kevin]

Hi,

 On the questions of tests: If people want, I can adapt the test in piglit for 
ARB_fragment_shader_interlock to this INTEL one. In general, I have an 
app/library that uses the extension and testing of that does definitely work on 
i965 (which should be utterly unsurprising since the produced assembly is 
identical). Indeed the current implementation in Mesa of 
ARB_fragment_shader_interlock is as flexible as the INTEL one since the 
compiler accepts code with the begin/end interlock anywhere since the only 
backend that supports it, i965, can easily accept it. I view the 
INTEL_fragment_shader_ordering implementation in a similar light as the 
NV_fragment_shader_interlock where it actually does not really do anything new, 
but signals to an application that the Mesa/i965 implementation allows more 
(and does it correctly) than the ARB/NV extensions restrict to.

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

[Mesa-dev] [Bug 107727] [DXVK]Assassin's Creed Syndicate - Assertion "radv_image_view_init"

[bugzilla-daemon]

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

Bug ID: 107727
   Summary: [DXVK]Assassin's Creed Syndicate - Assertion
"radv_image_view_init"
   Product: Mesa
   Version: 18.2
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Vulkan/radeon
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: berilli...@gmail.com
QA Contact: mesa-dev@lists.freedesktop.org

Hello,

When i tried to launch Assassin's Creed : Syndicate, wine crashes after the
second loading, before to go to the main menu.

When i check in the output console log, i see that the crash appears after an
error about Mesa and RADV. It's an error about "radv_image_view_init" :

Ubisoft Game Launcher/games/Assassin's Creed Syndicate/ACS.exe:
../mesa-/src/amd/vulkan/radv_image.c :1092 : radv_image_view_init: 
assertion « range->baseArrayLayer + radv_get_layerCount(image, range) - 1 <=
radv_minify(image->info.depth, range->baseMipLevel) » failed.

System Spec :
- Gentoo x86_64
- Ryzen 1600
- AMD Rx580 8Gb
- Wine Staging 3.14
- LLVM 7 (tested with LLVM 8)
- Mesa 18.2-rc4 (testing with Mesa git)
- DXVK 0.70 (tested with nightly build)

-- 
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 v4 2/7] nir: propagates if condition evaluation down some alu chains

[Jason Ekstrand]

On Mon, Aug 27, 2018 at 4:09 AM Timothy Arceri 
wrote:

> v2:
>  - only allow nir_op_inot or nir_op_b2i when alu input is 1.
>  - use some helpers as suggested by Jason.
>
> shader-db IVB results:
>
> total instructions in shared programs: 9993483 -> 9993472 (-0.00%)
> instructions in affected programs: 1300 -> 1289 (-0.85%)
> helped: 11
> HURT: 0
>
> total cycles in shared programs: 219476091 -> 219476059 (-0.00%)
> cycles in affected programs: 7675 -> 7643 (-0.42%)
> helped: 10
> HURT: 1
> ---
>  src/compiler/nir/nir_opt_if.c | 135 --
>  1 file changed, 129 insertions(+), 6 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
> index 11c6693d302..6d81705f6b7 100644
> --- a/src/compiler/nir/nir_opt_if.c
> +++ b/src/compiler/nir/nir_opt_if.c
> @@ -403,9 +403,113 @@ evaluate_if_condition(nir_if *nif, nir_cursor
> cursor, uint32_t *value)
> }
>  }
>
> +/*
> + * This propagates if condition evaluation down the chain of some alu
> + * instructions. For example by checking the use of some of the following
> alu
> + * instruction we can eventually replace ssa_107 with NIR_TRUE.
> + *
> + *   loop {
> + *  block block_1:
> + *  vec1 32 ssa_85 = load_const (0x0002)
> + *  vec1 32 ssa_86 = ieq ssa_48, ssa_85
> + *  vec1 32 ssa_87 = load_const (0x0001)
> + *  vec1 32 ssa_88 = ieq ssa_48, ssa_87
> + *  vec1 32 ssa_89 = ior ssa_86, ssa_88
> + *  vec1 32 ssa_90 = ieq ssa_48, ssa_0
> + *  vec1 32 ssa_91 = ior ssa_89, ssa_90
> + *  if ssa_86 {
> + * block block_2:
> + * ...
> + *break
> + *  } else {
> + *block block_3:
> + *  }
> + *  block block_4:
> + *  if ssa_88 {
> + *block block_5:
> + * ...
> + *break
> + *  } else {
> + *block block_6:
> + *  }
> + *  block block_7:
> + *  if ssa_90 {
> + *block block_8:
> + * ...
> + *break
> + *  } else {
> + *block block_9:
> + *  }
> + *  block block_10:
> + *  vec1 32 ssa_107 = inot ssa_91
> + *  if ssa_107 {
> + *block block_11:
> + *break
> + *  } else {
> + *block block_12:
> + *  }
> + *   }
> + */
>  static bool
> -evaluate_condition_use(nir_if *nif, nir_src *use_src, void *mem_ctx,
> -   bool is_if_condition)
> +propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src,
> + nir_src *alu_use, nir_alu_instr *alu, void
> *mem_ctx,
> + bool is_if_condition)
> +{
> +   bool progress = false;
> +
> +   uint32_t bool_value;
> +   nir_cursor cursor = nir_before_src(alu_use, is_if_condition);
> +   if (nir_op_infos[alu->op].num_inputs == 1) {
>

Howa about just

if (alu->op == nir_op_inot || alu->op == nir_op_b2i) {
   if (evaluate_if_condition()) {
  replace_if_condition_with_const();
  return true;
   }
} else if (alu->op == alu_op_ior || alu_op_iand) {
   if (evaluate_if_condition()) {
  // stuff
  return true;
   }
}

return false;

Or, for that matter, it could be a switch statement.  My point is that
checking the number of sources and then checking the number of inputs seems
kind-of pointless.


> +  if ((alu->op == nir_op_inot || alu->op == nir_op_b2i) &&
> +  evaluate_if_condition(nif, cursor, _value)) {
> + replace_if_condition_use_with_const(alu_use, mem_ctx, cursor,
> + bool_value, is_if_condition);
>

This isn't correct.  We need to run nir_eval_const_opcode on it before
replacing the ALU destination with it.


> + progress = true;
> +  }
> +   } else {
> +  assert(alu->op == nir_op_ior || alu->op == nir_op_iand);
> +
> +  if (evaluate_if_condition(nif, cursor, _value)) {
> + nir_ssa_def *def[2];
> + for (unsigned i = 0; i < 2; i++) {
> +if (alu->src[i].src.ssa == use_src->ssa) {
> +   if (is_if_condition) {
> +  b->cursor =
> + nir_before_cf_node(_use->parent_if->cf_node);
> +   } else {
> +  b->cursor = nir_before_instr(alu_use->parent_instr);
> +   }
>

This should be nir_before_src


> +
> +   nir_const_value const_value;
> +   const_value.u32[0] = bool_value;
> +
> +   def[i] = nir_build_imm(b, 1, 32, const_value);
> +} else {
> +   def[i] = alu->src[i].src.ssa;
> +}
> + }
> +
> + nir_ssa_def *nalu =
> +nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
> +
> + /* Rewrite use to use new alu instruction */
> + nir_src new_src = nir_src_for_ssa(nalu);
> +
> + if (is_if_condition)
> +nir_if_rewrite_condition(alu_use->parent_if, new_src);
> + else
> +

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Rogovin, Kevin]

Hi,

I followed the convention that was already present. The code from 
ARB/NV_fragment_shader_interlock had an intrinsic for begin critical section 
and an intrinsic for end critical section. I figured that since this extension 
has a completely different thinking (i.e. a memory barrier instead of a 
section) it warranted a new intrinsic. That and doing this way incurred the 
least amount of changes which I figured is always a good thing.

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

Re: [Mesa-dev] [Mesa-stable] [PATCH] intel: decoder: unify MI_BB_START field naming

[Dylan Baker]

Quoting Lionel Landwerlin (2018-08-28 02:39:58)
> Yes, I think so. You asked on another commit too, both are related and 
> this depends on other commits from Jason.
> 
> Here is a list in order of cherry picking :
> 
> commit f430a37fa75f534c3a114b0ec546fa14f05f5da1
> Author: Lionel Landwerlin 
> Date:   Tue Aug 14 11:22:12 2018 +0100
> 
>      intel: decoder: unify MI_BB_START field naming
> 
> commit 2abd7ae189135eb5a1f530a3a1c9412d3a7e238d
> Author: Jason Ekstrand 
> Date:   Fri Aug 24 15:23:04 2018 -0500
> 
>      intel/decoder: Clean up field iteration and fix sub-dword fields
> 
> commit cbd4bc1346f7397242e157bb66099b950a8c5643
> Author: Jason Ekstrand 
> Date:   Fri Aug 24 16:04:03 2018 -0500
> 
>      intel/batch_decoder: Fix dynamic state printing
> 
> commit 70de31d0c106f58d6b7e6d5b79b8d90c1c112a3b
> Author: Jason Ekstrand 
> Date:   Fri Aug 24 16:05:08 2018 -0500
> 
>      intel/batch_decoder: Print blend states properly
> 
> 
> commit 440a988bd1478bb33dafcbb8575473bc643ae383
> Author: Lionel Landwerlin 
> Date:   Sat Aug 25 18:22:00 2018 +0100
> 
>      intel: decoder: handle 0 sized structs
> 
> Thanks,
> 
> -
> Lionel
> 
> On 27/08/2018 22:20, Andres Gomez wrote:
> > Lionel, should we also include this in the stable queues ?
> >
> > On Tue, 2018-08-14 at 11:26 +0100, Lionel Landwerlin wrote:
> >> The batch decoder looks for a field with a particular name to decide
> >> whether an MI_BB_START leads into a second batch buffer level. Because
> >> the names are different between Gen7.5/8 and the newer generation we
> >> fail that test and keep on reading (invalid) instructions.
> >>
> >> Signed-off-by: Lionel Landwerlin 
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
> >> ---
> >>   src/intel/genxml/gen75.xml | 6 +++---
> >>   src/intel/genxml/gen8.xml  | 6 +++---
> >>   src/intel/vulkan/anv_batch_chain.c | 2 +-
> >>   3 files changed, 7 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
> >> index 5b01fd45400..dfc3d891498 100644
> >> --- a/src/intel/genxml/gen75.xml
> >> +++ b/src/intel/genxml/gen75.xml
> >> @@ -2314,9 +2314,9 @@
> >> 
> >>>> default="0"/>
> >>>> default="49"/>
> >> -
> >> -  
> >> -  
> >> + >> type="uint">
> >> +  
> >> +  
> >>   
> >>   
> >>   
> >> diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
> >> index 4ed41d15612..330366b7ed0 100644
> >> --- a/src/intel/genxml/gen8.xml
> >> +++ b/src/intel/genxml/gen8.xml
> >> @@ -2553,9 +2553,9 @@
> >> 
> >>>> default="0"/>
> >>>> default="49"/>
> >> -
> >> -  
> >> -  
> >> + >> type="uint">
> >> +  
> >> +  
> >>   
> >>   
> >>   
> >> diff --git a/src/intel/vulkan/anv_batch_chain.c 
> >> b/src/intel/vulkan/anv_batch_chain.c
> >> index c47a81c8a4d..0f7c8325ea4 100644
> >> --- a/src/intel/vulkan/anv_batch_chain.c
> >> +++ b/src/intel/vulkan/anv_batch_chain.c
> >> @@ -531,7 +531,7 @@ emit_batch_buffer_start(struct anv_cmd_buffer 
> >> *cmd_buffer,
> >>  anv_batch_emit(_buffer->batch, GEN8_MI_BATCH_BUFFER_START, bbs) {
> >> bbs.DWordLength   = cmd_buffer->device->info.gen < 8 ?
> >> gen7_length : gen8_length;
> >> -  bbs._2ndLevelBatchBuffer  = _1stlevelbatch;
> >> +  bbs.SecondLevelBatchBuffer= Firstlevelbatch;
> >> bbs.AddressSpaceIndicator = ASI_PPGTT;
> >> bbs.BatchBufferStartAddress   = (struct anv_address) { bo, offset 
> >> };
> >>  }

Hi Lionel,

Only patches 1 and 3 of that list apply cleanly to the 18.1 branch, it looks
like maybe I need a few more patches for things to apply cleanly?

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 1/2] mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering.

[Francisco Jerez]

kevin.rogo...@intel.com writes:

> From: Kevin Rogovin 
>
>This extension provides new GLSL built-in function
>beginFragmentShaderOrderingIntel() that guarantees
>(taking wording of GL_INTEL_fragment_shader_ordering
>extension) that any memory transactions issued by
>shader invocations from previous primitives mapped to
>same xy window coordinates (and same sample when
>per-sample shading is active), complete and are visible
>to the shader invocation that called
>beginFragmentShaderOrderingINTEL().
>
>One advantage of INTEL_fragment_shader_ordering over
>ARB_fragment_shader_interlock is that it provides a
>function that operates as a memory barrie (instead
>of a defining a critcial section) that can be called
>under arbitary control flow from any function (in
>contrast the begin/end of ARB_fragment_shader_interlock
>may only be called once, from main(), under no control
>flow.
>
> Signed-off-by: Kevin Rogovin 
> ---
>  src/compiler/glsl/builtin_functions.cpp  | 17 +
>  src/compiler/glsl/glsl_parser_extras.cpp |  1 +
>  src/compiler/glsl/glsl_parser_extras.h   |  2 ++
>  src/compiler/glsl/glsl_to_nir.cpp|  6 ++
>  src/compiler/glsl/ir.h   |  1 +
>  src/compiler/nir/nir_intrinsics.py   |  1 +
>  src/mesa/main/extensions_table.h |  1 +
>  src/mesa/main/mtypes.h   |  1 +
>  8 files changed, 30 insertions(+)
>
> diff --git a/src/compiler/glsl/builtin_functions.cpp 
> b/src/compiler/glsl/builtin_functions.cpp
> index b601880686..5650365d1d 100644
> --- a/src/compiler/glsl/builtin_functions.cpp
> +++ b/src/compiler/glsl/builtin_functions.cpp
> @@ -525,6 +525,12 @@ supports_nv_fragment_shader_interlock(const 
> _mesa_glsl_parse_state *state)
> return state->NV_fragment_shader_interlock_enable;
>  }
>  
> +static bool
> +supports_intel_fragment_shader_ordering(const _mesa_glsl_parse_state *state)
> +{
> +   return state->INTEL_fragment_shader_ordering_enable;
> +}
> +
>  static bool
>  shader_clock(const _mesa_glsl_parse_state *state)
>  {
> @@ -1305,6 +1311,11 @@ builtin_builder::create_intrinsics()
> supports_arb_fragment_shader_interlock,
> ir_intrinsic_end_invocation_interlock), NULL);
>  
> +   add_function("__intrinsic_begin_fragment_shader_ordering",
> +_invocation_interlock_intrinsic(
> +   supports_intel_fragment_shader_ordering,
> +   ir_intrinsic_begin_fragment_shader_ordering), NULL);
> +
> add_function("__intrinsic_shader_clock",
>  _shader_clock_intrinsic(shader_clock,
>  glsl_type::uvec2_type),
> @@ -3419,6 +3430,12 @@ builtin_builder::create_builtins()
> supports_nv_fragment_shader_interlock),
>  NULL);
>  
> +   add_function("beginFragmentShaderOrderingINTEL",
> +_invocation_interlock(
> +   "__intrinsic_begin_fragment_shader_ordering",
> +   supports_intel_fragment_shader_ordering),
> +NULL);
> +
> add_function("anyInvocationARB",
>  _vote("__intrinsic_vote_any", vote),
>  NULL);
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp 
> b/src/compiler/glsl/glsl_parser_extras.cpp
> index 0a7d0d78b1..21d4122444 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -725,6 +725,7 @@ static const _mesa_glsl_extension 
> _mesa_glsl_supported_extensions[] = {
> EXT_AEP(EXT_texture_buffer),
> EXT_AEP(EXT_texture_cube_map_array),
> EXT(INTEL_conservative_rasterization),
> +   EXT(INTEL_fragment_shader_ordering),
> EXT(INTEL_shader_atomic_float_minmax),
> EXT(MESA_shader_integer_functions),
> EXT(NV_fragment_shader_interlock),
> diff --git a/src/compiler/glsl/glsl_parser_extras.h 
> b/src/compiler/glsl/glsl_parser_extras.h
> index 2c8353214a..e03b34d7d6 100644
> --- a/src/compiler/glsl/glsl_parser_extras.h
> +++ b/src/compiler/glsl/glsl_parser_extras.h
> @@ -812,6 +812,8 @@ struct _mesa_glsl_parse_state {
> bool EXT_texture_cube_map_array_warn;
> bool INTEL_conservative_rasterization_enable;
> bool INTEL_conservative_rasterization_warn;
> +   bool INTEL_fragment_shader_ordering_enable;
> +   bool INTEL_fragment_shader_ordering_warn;
> bool INTEL_shader_atomic_float_minmax_enable;
> bool INTEL_shader_atomic_float_minmax_warn;
> bool MESA_shader_integer_functions_enable;
> diff --git a/src/compiler/glsl/glsl_to_nir.cpp 
> b/src/compiler/glsl/glsl_to_nir.cpp
> index a53000f47e..efbb2317ac 100644
> --- a/src/compiler/glsl/glsl_to_nir.cpp
> +++ b/src/compiler/glsl/glsl_to_nir.cpp
> @@ -742,6 +742,9 @@ nir_visitor::visit(ir_call *ir)
>case ir_intrinsic_end_invocation_interlock:
>   op = nir_intrinsic_end_invocation_interlock;
>   break;
> +  case 

Re: [Mesa-dev] [PATCH 1/5] meson: Actually load translation files

[Dylan Baker]

Quoting Eric Engestrom (2018-08-28 07:34:26)
> On Friday, 2018-08-24 07:13:41 -0700, Dylan Baker wrote:
> > Currently we run the script but don't actually load any files, even in a
> > tarball where they exist.
> > 
> > Fixes: 3218056e0eb375eeda470058d06add1532acd6d4
> >("meson: Build i965 and dri stack")
> > ---
> >  src/util/xmlpool/meson.build | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/util/xmlpool/meson.build b/src/util/xmlpool/meson.build
> > index 346b1956a55..3d2de0cdc3a 100644
> > --- a/src/util/xmlpool/meson.build
> > +++ b/src/util/xmlpool/meson.build
> > @@ -22,7 +22,10 @@ xmlpool_options_h = custom_target(
> >'xmlpool_options.h',
> >input : ['gen_xmlpool.py', 't_options.h'],
> >output : 'options.h',
> > -  command : [prog_python, '@INPUT@', meson.current_source_dir()],
> > +  command : [
> > +prog_python, '@INPUT@', meson.current_source_dir(),
> > +'ca', 'es', 'de', 'nl', 'sv', 'fr',
> > +  ],
> >capture : true,
> >depend_files : files('ca.po', 'es.po', 'de.po', 'nl.po', 'sv.po', 
> > 'fr.po'),
> 
> Nit: I would put that lang list in an array and reuse it to generate the list
> of `depend_files` here, to keep it in sync, but that can be a follow up
> patch (happy to do it).

Let's follow up on that, since I'd like to have this for 18.1.7 so that
translations actually work in the 18.1 series :)

> 
> Reviewed-by: Eric Engestrom 

Thanks!

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


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

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Mark Janes]

Is there a piglit test that verifies that this feature works properly?

 writes:

> From: Kevin Rogovin 
>
> INTEL_fragment_shader_ordering provides the ability for shaders
> to issue a call to gaurnantee memory write operation ordering
> of overlapping pixels or samples. In contrast to
> ARB_fragment_shader_interlock, INTEL_fragment_shader_ordering
> instead of defining a critical region (which must be in main() and
> under no flow control) provides a single function that acts like
> a memory barrier that can be called under any control flow.
>
> Kevin Rogovin (2):
>   mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering.
>   i965: Add INTEL_fragment_shader_ordering support.
>
>  docs/relnotes/18.3.0.html|  1 +
>  src/compiler/glsl/builtin_functions.cpp  | 17 +
>  src/compiler/glsl/glsl_parser_extras.cpp |  1 +
>  src/compiler/glsl/glsl_parser_extras.h   |  2 ++
>  src/compiler/glsl/glsl_to_nir.cpp|  6 ++
>  src/compiler/glsl/ir.h   |  1 +
>  src/compiler/nir/nir_intrinsics.py   |  1 +
>  src/intel/compiler/brw_fs_nir.cpp|  1 +
>  src/mesa/drivers/dri/i965/intel_extensions.c |  1 +
>  src/mesa/main/extensions_table.h |  1 +
>  src/mesa/main/mtypes.h   |  1 +
>  11 files changed, 33 insertions(+)
>
> -- 
> 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 2/2] i965: Add INTEL_fragment_shader_ordering support.

[Jason Ekstrand]

Are there any tests for this?

--Jason

On Mon, Aug 27, 2018 at 1:54 AM  wrote:

> From: Kevin Rogovin 
>
> Adds suppport for INTEL_fragment_shader_ordering. We achieve
> the fragment ordering by using the same instruction as for
> beginInvocationInterlockARB() which is by issuing a memory
> fence via sendc.
>
> Signed-off-by: Kevin Rogovin 
> ---
>  docs/relnotes/18.3.0.html| 1 +
>  src/intel/compiler/brw_fs_nir.cpp| 1 +
>  src/mesa/drivers/dri/i965/intel_extensions.c | 1 +
>  3 files changed, 3 insertions(+)
>
> diff --git a/docs/relnotes/18.3.0.html b/docs/relnotes/18.3.0.html
> index afcb044817..71fb41ca86 100644
> --- a/docs/relnotes/18.3.0.html
> +++ b/docs/relnotes/18.3.0.html
> @@ -59,6 +59,7 @@ Note: some of the new features are only available with
> certain drivers.
>  GL_EXT_vertex_attrib_64bit on i965, nvc0, radeonsi.
>  GL_EXT_window_rectangles on radeonsi.
>  GL_KHR_texture_compression_astc_sliced_3d on radeonsi.
> +GL_INTEL_fragment_shader_ordering on i965.
>  GL_NV_fragment_shader_interlock on i965.
>  
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index 9c9df5ac09..62bff2a323 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -4836,6 +4836,7 @@ fs_visitor::nir_emit_intrinsic(const fs_builder
> , nir_intrinsic_instr *instr
>break;
> }
>
> +   case nir_intrinsic_begin_fragment_shader_ordering:
> case nir_intrinsic_begin_invocation_interlock: {
>const fs_builder ubld = bld.group(8, 0);
>const fs_reg tmp = ubld.vgrf(BRW_REGISTER_TYPE_UD, 2);
> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c
> b/src/mesa/drivers/dri/i965/intel_extensions.c
> index 0b137664b0..1ea8594c34 100644
> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
> @@ -247,6 +247,7 @@ intelInitExtensions(struct gl_context *ctx)
>ctx->Extensions.OES_primitive_bounding_box = true;
>ctx->Extensions.OES_texture_buffer = true;
>ctx->Extensions.ARB_fragment_shader_interlock = true;
> +  ctx->Extensions.INTEL_fragment_shader_ordering = true;
>
>if (can_do_pipelined_register_writes(brw->screen)) {
>   ctx->Extensions.ARB_draw_indirect = true;
> --
> 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 v4 1/7] nir: evaluate if condition uses inside the if branches

[Jason Ekstrand]

On Mon, Aug 27, 2018 at 4:09 AM Timothy Arceri 
wrote:

> Since we know what side of the branch we ended up on we can just
> replace the use with a constant.
>
> All the spill changes in shader-db are from Dolphin uber shaders,
> despite some small regressions the change is clearly positive.
>
> V2: insert new constant after any phis in the
> use->parent_instr->type == nir_instr_type_phi path.
>
> v3:
>  - use nir_after_block_before_jump() for inserting const
>  - check dominance of phi uses correctly
>
> v4:
>  - create some helpers as suggested by Jason.
>
> shader-db results IVB:
>
> total instructions in shared programs: 201 -> 9993483 (-0.06%)
> instructions in affected programs: 163235 -> 157517 (-3.50%)
> helped: 132
> HURT: 2
>
> total cycles in shared programs: 231670754 -> 219476091 (-5.26%)
> cycles in affected programs: 143424120 -> 131229457 (-8.50%)
> helped: 115
> HURT: 24
>
> total spills in shared programs: 4383 -> 4370 (-0.30%)
> spills in affected programs: 1656 -> 1643 (-0.79%)
> helped: 9
> HURT: 18
>
> total fills in shared programs: 4610 -> 4581 (-0.63%)
> fills in affected programs: 374 -> 345 (-7.75%)
> helped: 6
> HURT: 0
> ---
>  src/compiler/nir/nir.h|  22 +++
>  src/compiler/nir/nir_opt_if.c | 113 ++
>  2 files changed, 135 insertions(+)
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 009a6d60371..0caacd30173 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -2331,6 +2331,28 @@ nir_after_block_before_jump(nir_block *block)
> }
>  }
>
> +static inline nir_cursor
> +nir_before_src(nir_src *src, bool is_if_condition)
> +{
> +   if (is_if_condition) {
> +  nir_block *prev_block =
> + nir_cf_node_as_block(nir_cf_node_prev(>parent_if->cf_node));
> +  assert(!nir_block_ends_in_jump(prev_block));
> +  return nir_after_block(prev_block);
> +   } else if (src->parent_instr->type == nir_instr_type_phi) {
> +  nir_phi_instr *cond_phi = nir_instr_as_phi(src->parent_instr);
> +  nir_foreach_phi_src(phi_src, cond_phi) {
> + if (phi_src->src.ssa == src->ssa) {
> +return nir_after_block_before_jump(phi_src->pred);
> + }
> +  }
>

This works.  Some sort of container_of would be more efficient.  I guess
that wouldn't work if the source was a register indirect on a phi src but I
don't think that's valid.


> +
> +  unreachable("failed to find phi src");
>

If the source isn't a phi src (and container_of would fail) hitting an
unreachable is really nasty.  I don't think we will hit this case but I
also think that if we do, container_of is actually the safer option as
weird as that sounds.  That said, we should do something to ensure that we
get an assertion failure if it ever isn't the source of a phi.  What you
did above works.  If we did container_of, we should add an assert loop
somehow.


> +   } else {
> +  return nir_before_instr(src->parent_instr);
> +   }
> +}
> +
>  static inline nir_cursor
>  nir_before_cf_node(nir_cf_node *node)
>  {
> diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
> index dacf2d6c667..11c6693d302 100644
> --- a/src/compiler/nir/nir_opt_if.c
> +++ b/src/compiler/nir/nir_opt_if.c
> @@ -369,6 +369,76 @@ opt_if_loop_terminator(nir_if *nif)
> return true;
>  }
>
> +static void
> +replace_if_condition_use_with_const(nir_src *use, void *mem_ctx,
> +nir_cursor cursor, unsigned
> nir_boolean,
>

Mind making nir_boolean a uint32_t?

With that (and regardless of what way we go with container_of above), this
patch is

Reviewed-by: Jason Ekstrand 


> +bool if_condition)
> +{
> +   /* Create const */
> +   nir_load_const_instr *load = nir_load_const_instr_create(mem_ctx, 1,
> 32);
> +   load->value.u32[0] = nir_boolean;
> +   nir_instr_insert(cursor, >instr);
> +
> +   /* Rewrite use to use const */
> +   nir_src new_src = nir_src_for_ssa(>def);
> +
> +   if (if_condition)
> +  nir_if_rewrite_condition(use->parent_if, new_src);
> +   else
> +  nir_instr_rewrite_src(use->parent_instr, use, new_src);
> +}
> +
> +static bool
> +evaluate_if_condition(nir_if *nif, nir_cursor cursor, uint32_t *value)
> +{
> +   nir_block *use_block = nir_cursor_current_block(cursor);
> +   if (nir_block_dominates(nir_if_first_then_block(nif), use_block)) {
> +  *value = NIR_TRUE;
> +  return true;
> +   } else if (nir_block_dominates(nir_if_first_else_block(nif),
> use_block)) {
> +  *value = NIR_FALSE;
> +  return true;
> +   } else {
> +  return false;
> +   }
> +}
> +
> +static bool
> +evaluate_condition_use(nir_if *nif, nir_src *use_src, void *mem_ctx,
> +   bool is_if_condition)
> +{
> +   bool progress = false;
> +
> +   uint32_t value;
> +   nir_cursor cursor = nir_before_src(use_src, is_if_condition);
> +   if (evaluate_if_condition(nif, cursor, )) {
> +  

[Mesa-dev] [Bug 106976] Compilation failure due to missing xcb_randr_lease_t

[bugzilla-daemon]

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

Sergii Romantsov  changed:

   What|Removed |Added

 CC||b...@basnieuwenhuizen.nl

--- Comment #7 from Sergii Romantsov  ---
Hello,
could, please, somebody clarify: should mesa be supported on Ubuntu 16.04 and
with autoconf?
If yes, than could we review a patch, because seems patch makes some sense?

-- 
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 107146] swr doesn't compile with current LLVM 7.0 snapshots

[bugzilla-daemon]

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

--- Comment #3 from Alok Hota  ---
(In reply to Bernhard Rosenkraenzer from comment #0)
> createInstructionSimplifierPass() has been removed in LLVM. This is used in
> a few places in SWR; looks like this can safely be removed because it is
> always preceded by createInstructionCombiningPass() which seems to be a
> superset of what createInstructionSimplifierPass() used to do.
> 
> Fixing this just unmasks another problem though -
> Intrinsic::x86_fma_vfmadd_ps_256 is gone. Not sure how to replace that
> properly.

I can confirm we're seeing this issue as well.

(In reply to Philip Meulengracht from comment #2)
> Created attachment 140731 [details] [review]
> proposed patch
> 
> See my attachment for a proposed patch - I encounted this error too and
> removed the references to the vfmaddps, and I removed the SimplePass's due
> to them being unnecessary. Please feel free to correct me or the patch if
> it's wrong. 
> 
> Instead of the vfmaddps it will use mul add.

Thanks for the patch! I'm looking into this now.

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

Re: [Mesa-dev] [PATCH 1/5] meson: Actually load translation files

[Eric Engestrom]

On Friday, 2018-08-24 07:13:41 -0700, Dylan Baker wrote:
> Currently we run the script but don't actually load any files, even in a
> tarball where they exist.
> 
> Fixes: 3218056e0eb375eeda470058d06add1532acd6d4
>("meson: Build i965 and dri stack")
> ---
>  src/util/xmlpool/meson.build | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/util/xmlpool/meson.build b/src/util/xmlpool/meson.build
> index 346b1956a55..3d2de0cdc3a 100644
> --- a/src/util/xmlpool/meson.build
> +++ b/src/util/xmlpool/meson.build
> @@ -22,7 +22,10 @@ xmlpool_options_h = custom_target(
>'xmlpool_options.h',
>input : ['gen_xmlpool.py', 't_options.h'],
>output : 'options.h',
> -  command : [prog_python, '@INPUT@', meson.current_source_dir()],
> +  command : [
> +prog_python, '@INPUT@', meson.current_source_dir(),
> +'ca', 'es', 'de', 'nl', 'sv', 'fr',
> +  ],
>capture : true,
>depend_files : files('ca.po', 'es.po', 'de.po', 'nl.po', 'sv.po', 'fr.po'),

Nit: I would put that lang list in an array and reuse it to generate the list
of `depend_files` here, to keep it in sync, but that can be a follow up
patch (happy to do it).

Reviewed-by: Eric Engestrom 

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

Re: [Mesa-dev] [RFC 00/10] Let us welcome EGLDevice

[Emil Velikov]

Thanks Mathias,

On 28 August 2018 at 08:31, Mathias Fröhlich  wrote:
> Hi Emil,
>
> On Friday, 3 August 2018 14:44:16 CEST Emil Velikov wrote:
>> Hi all,
>>
>> This series implements the following extensions:
>>  - EGL_EXT_device_base
>>  - EGL_MESA_device_software
>>  - EGL_EXT_device_drm
>>  - EGL_platform_device
>>
>> As you know the APIs are used to enumerate, select and use EGLDevices.
>> The second extension (proposed by Ajax) defines a 'software' device,
>> alongside the existing DRM ones.
>>
>> While there are many usecases for this work, my primary interest is
>> allowing device selection, wrt testing. To achieve the goal, we would
>> need to finalise EGL_EXT_explicit_device (also proposed by Ajax).
>>
>> Piglit patches will be sent out shortly.
>>
>> Any feedback is greatly appreciated.
>
> From a higher point of view the approach looks good.
>
> To sum up, you basically associate an _EGLDevice with each _EGLDisplay
> where the _EGLDevice only contains the meta information where to
> find the device and the _EGLDisplay later contains open file descriptors
> to work with ...
>
It's the other way around - I associate each EGLDIsplay with a given EGLDevice.
The EGL_EXT_device_enumeration spec removed that (imho) bogus relation:

"Because the EGLDeviceEXT is a property of , any use of an
associated EGLDeviceEXT after  has been terminated gives
undefined results. Querying an EGL_DEVICE_EXT from  after a
call to eglTerminate() (and subsequent re-initialization) may
return a different value."


> Nevertheless, running the tests and proof of concept programs that I used
> back in the day brought up some questions and one crash.
>
> At first the Crash: The attached eglcontext-pbuffer.c which goes the pbuffer 
> approach
> instead of going surfaceless just dumps core in pbuffer creation using
> the patch series. I believe that it is legal what the program does, but may
> be you want to double check that too.
>
Off the top of my head, I think that's due to eglCreatePbufferSurface
instead of eglCreatePlatformPbufferSurfaceEXT.
I think we could wire that legacy API up, although the whole thing is
_really_ fiddly.

See my piglit patch 85e3b32b3260184853ec20b938574c26a0f39dd8 for some details.

> Then if I use the patch series on an account that has no DISPLAY set and no
> 'display server' running, eglInitialize fails in device_probe_device due to 
> at first
> opening the '.../card0' device and bailing out when this does not work.
> Means the current patch series goes via opening the primary node which
> shall not be accessible by everyone and from that derives the rendernode
> device which is finally used for _EGLDisplay initialization.
> Can we alternatively go directly to the rendernode in some way?
>
I think we could. Can you elaborate a bit more or provide an example
on the topic though.
I'm quite curious what's happening here - is there no card0 node,
open() fails, other?

> For patch #7, can you explain why dri already provides the right format?
> It's probably correct, but I am missing some bits of that context creation
> big picture to give a review.
>
The format used when to create a surface is passed to the driver,
which then calls back the loader with the exact same one.
Admittedly there's a ton of conversion back and forth (within the
driver) so it's not always clear.

Pretty much every other platform respects the provided format, hence
my cleanup patches ;-)
That said, I'm perfectly fine with pulling those patches (and updating
platform_device.c) if it will make things easier.

> And finally, in patch #2 you mention that you want to avoid larger patches
> and the announced extensions are not yet working as expected.
> May be you can announce the extensions in a separate patch that follows
> all the implementation patches? That probably helps people that want to
> bisect something using piglit.
>
It's actually patch 1/10 which "enables" the extensions.

You're right though - we could merge the 1/10 boilerplate with 2/10
and enable the extensions once the SW _and_ DRM extensions have
landed.
Might even a) split the helpers from 3/10 into a prep patch and b) add
the DRM _eglFindDevice instances of 3/10 into the DRM patch - 4/10.

Another thing that comes to mind is ... minimise the first device is
always SW assumption.
That would involve adding a couple of _eglDeviceSupports(dev,
_EGL_DEVICE_DRM) calls. It should make things less magical and clearer
for the next EGL device extension.

FTR swrast with platform_device is still WIP - I've been working
fixing up swrast recently. It requires a ton of refactoring and fixes
;-)

> thanks for approaching this topic!
>
Thank you for helping out with review and questions.

I'll try to find time and respin the series tomorrow.

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

Re: [Mesa-dev] [PATCH] i965/gen6/xfb: handle case where transform feedback is not active

[andrey simiklit]

Hello,

Thanks a lot)

Regards,
Andrii.

On Tue, Aug 28, 2018 at 4:35 PM Samuel Iglesias Gonsálvez <
sigles...@igalia.com> wrote:

> On Tuesday, August 28, 2018 2:02:07 PM CEST Samuel Iglesias Gonsálvez
> wrote:
> > I'll do it later today.
>
> Done!
>
> Sam
>
> >
> > Thanks for contributing!
> >
> > Sam
> >
> > On 28/08/18 13:59, andrey simiklit wrote:
> > > Hi all,
> > >
> > > Could somebody push it if it seems good for all?
> > >
> > > Regards,
> > > Andrii.
> > >
> > > On Thu, Aug 23, 2018 at 4:53 PM Samuel Iglesias Gonsálvez
> > >
> > > mailto:sigles...@igalia.com>> wrote:
> > > The patch seems fine to me. I also tested it on Intel CI and there
> > > were no regressions.
> > >
> > > Reviewed-by: Samuel Iglesias Gonsálvez 
> > > 
> > >
> > > Thanks,
> > >
> > > Sam
> > >
> > > On 15/08/18 17:20, asimiklit.w...@gmail.com
> > >
> > >  wrote:
> > >> From: Andrii Simiklit 
> > >> 
> > >>
> > >> When the SVBI Payload Enable is false I guess the register R1.4
> > >> which contains the Maximum Streamed Vertex Buffer Index is filled
> by
> > >> zero
> > >> and GS stops to write transform feedback when the transform
> feedback
> > >> is not active.
> > >>
> > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107579
> > >> Signed-off-by: Andrii Simiklit 
> > >> 
> > >> ---
> > >>
> > >>  src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +-
> > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
> > >> b/src/mesa/drivers/dri/i965/genX_state_upload.c index
> > >> ea5ad55..0f82500 100644
> > >> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> > >> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> > >> @@ -2806,7 +2806,7 @@ genX(upload_gs_state)(struct brw_context
> *brw)
> > >>
> > >>  #if GEN_GEN < 7
> > >>
> > >>   gs.SOStatisticsEnable = true;
> > >>   if (gs_prog->info.has_transform_feedback_varyings)
> > >>
> > >> -gs.SVBIPayloadEnable = true;
> > >> +gs.SVBIPayloadEnable =
> > >> _mesa_is_xfb_active_and_unpaused(ctx);
> > >>
> > >>   /* GEN6_GS_SPF_MODE and GEN6_GS_VECTOR_MASK_ENABLE are
> > >>   enabled as it
> > >>
> > >>* was previously done for gen6.
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 107477] [DXVK] Setting high shader quality in GTA V results in LLVM error

[bugzilla-daemon]

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

--- Comment #12 from Samuel Pitoiset  ---
Are you sure the link is correct? "This link has expired or never existed in
the first place!"

-- 
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] i965/gen6/xfb: handle case where transform feedback is not active

[Samuel Iglesias Gonsálvez]

On Tuesday, August 28, 2018 2:02:07 PM CEST Samuel Iglesias Gonsálvez wrote:
> I'll do it later today.

Done!

Sam

> 
> Thanks for contributing!
> 
> Sam
> 
> On 28/08/18 13:59, andrey simiklit wrote:
> > Hi all,
> > 
> > Could somebody push it if it seems good for all?
> > 
> > Regards,
> > Andrii.
> > 
> > On Thu, Aug 23, 2018 at 4:53 PM Samuel Iglesias Gonsálvez
> > 
> > mailto:sigles...@igalia.com>> wrote:
> > The patch seems fine to me. I also tested it on Intel CI and there
> > were no regressions.
> > 
> > Reviewed-by: Samuel Iglesias Gonsálvez 
> > 
> > 
> > Thanks,
> > 
> > Sam
> > 
> > On 15/08/18 17:20, asimiklit.w...@gmail.com
> > 
> >  wrote:
> >> From: Andrii Simiklit 
> >> 
> >> 
> >> When the SVBI Payload Enable is false I guess the register R1.4
> >> which contains the Maximum Streamed Vertex Buffer Index is filled by
> >> zero
> >> and GS stops to write transform feedback when the transform feedback
> >> is not active.
> >> 
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107579
> >> Signed-off-by: Andrii Simiklit 
> >> 
> >> ---
> >> 
> >>  src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c
> >> b/src/mesa/drivers/dri/i965/genX_state_upload.c index
> >> ea5ad55..0f82500 100644
> >> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> >> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> >> @@ -2806,7 +2806,7 @@ genX(upload_gs_state)(struct brw_context *brw)
> >> 
> >>  #if GEN_GEN < 7
> >>  
> >>   gs.SOStatisticsEnable = true;
> >>   if (gs_prog->info.has_transform_feedback_varyings)
> >> 
> >> -gs.SVBIPayloadEnable = true;
> >> +gs.SVBIPayloadEnable =
> >> _mesa_is_xfb_active_and_unpaused(ctx);
> >> 
> >>   /* GEN6_GS_SPF_MODE and GEN6_GS_VECTOR_MASK_ENABLE are
> >>   enabled as it
> >>   
> >>* was previously done for gen6.



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

Re: [Mesa-dev] [PATCH 1/4] mesa: use C99 initializer in get_gl_override()

[Eric Engestrom]

On Friday, 2018-08-24 14:05:57 +0100, Emil Velikov wrote:
> From: Emil Velikov 
> 
> The overrides array contains entries indexed on the gl_api enum.
> Use a C99 initializer to make it a bit more obvious.
> 
> Signed-off-by: Emil Velikov 

Patches 1-3 are:
Reviewed-by: Eric Engestrom 

Patch 4 look good code-wise, but I don't know if it's a good idea or
not, so I abstain.

> ---
>  src/mesa/main/version.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mesa/main/version.c b/src/mesa/main/version.c
> index 77ff51b6d9e..610ba2f08c5 100644
> --- a/src/mesa/main/version.c
> +++ b/src/mesa/main/version.c
> @@ -64,10 +64,10 @@ get_gl_override(gl_api api, int *version, bool 
> *fwd_context,
>bool fc_suffix;
>bool compat_suffix;
> } override[] = {
> -  { -1, false, false},
> -  { -1, false, false},
> -  { -1, false, false},
> -  { -1, false, false},
> +  [API_OPENGL_COMPAT] = { -1, false, false},
> +  [API_OPENGLES]  = { -1, false, false},
> +  [API_OPENGLES2] = { -1, false, false},
> +  [API_OPENGL_CORE]   = { -1, false, false},
> };
>  
> STATIC_ASSERT(ARRAY_SIZE(override) == API_OPENGL_LAST + 1);
> -- 
> 2.18.0
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 07/14] st/mesa: set ctx->Const.SubPixelBits

[Elie Tournier]

On Tue, Aug 28, 2018 at 12:40:25PM +0100, Jakob Bornecrantz wrote:
> On Thu, Aug 9, 2018 at 12:57 AM Marek Olšák  wrote:
> >
> > From: Marek Olšák 
> 
> This patch causes regressions in dEQP[1] on virgl running on a
> radeonSI device. Not a lot of drivers set
> PIPE_CAP_VIEWPORT_SUBPIXEL_BITS but SubPixelBits is by default set to
> 4, but this overwrites it without checking if the returned value is
> zero or not. Looking around it seems that a lot of other drivers just
> returns zero for PIPE_CAP_VIEWPORT_SUBPIXEL_BITS not just virgl, so
> this probably causes regressions on more drivers then virgl.

I can also see the regression on the intel driver.

> 
> Cheers, Jakob.
> ___
> 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] i965/gen6/xfb: handle case where transform feedback is not active

[Samuel Iglesias Gonsálvez]

I'll do it later today.

Thanks for contributing!

Sam


On 28/08/18 13:59, andrey simiklit wrote:
> Hi all,
>
> Could somebody push it if it seems good for all?
>
> Regards,
> Andrii.
>
> On Thu, Aug 23, 2018 at 4:53 PM Samuel Iglesias Gonsálvez
> mailto:sigles...@igalia.com>> wrote:
>
> The patch seems fine to me. I also tested it on Intel CI and there
> were no regressions.
>
> Reviewed-by: Samuel Iglesias Gonsálvez 
> 
>
> Thanks,
>
> Sam
>
> On 15/08/18 17:20, asimiklit.w...@gmail.com
>  wrote:
>> From: Andrii Simiklit  
>> 
>>
>> When the SVBI Payload Enable is false I guess the register R1.4
>> which contains the Maximum Streamed Vertex Buffer Index is filled by zero
>> and GS stops to write transform feedback when the transform feedback 
>> is not active.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107579
>> Signed-off-by: Andrii Simiklit 
>> 
>> ---
>>  src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c 
>> b/src/mesa/drivers/dri/i965/genX_state_upload.c
>> index ea5ad55..0f82500 100644
>> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
>> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
>> @@ -2806,7 +2806,7 @@ genX(upload_gs_state)(struct brw_context *brw)
>>  #if GEN_GEN < 7
>>   gs.SOStatisticsEnable = true;
>>   if (gs_prog->info.has_transform_feedback_varyings)
>> -gs.SVBIPayloadEnable = true;
>> +gs.SVBIPayloadEnable = 
>> _mesa_is_xfb_active_and_unpaused(ctx);
>>  
>>   /* GEN6_GS_SPF_MODE and GEN6_GS_VECTOR_MASK_ENABLE are enabled 
>> as it
>>* was previously done for gen6.
>



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

Re: [Mesa-dev] [PATCH] i965/gen6/xfb: handle case where transform feedback is not active

[andrey simiklit]

Hi all,

Could somebody push it if it seems good for all?

Regards,
Andrii.

On Thu, Aug 23, 2018 at 4:53 PM Samuel Iglesias Gonsálvez <
sigles...@igalia.com> wrote:

> The patch seems fine to me. I also tested it on Intel CI and there were no
> regressions.
>
> Reviewed-by: Samuel Iglesias Gonsálvez 
> 
> Thanks,
>
> Sam
>
> On 15/08/18 17:20, asimiklit.w...@gmail.com wrote:
>
> From: Andrii Simiklit  
>
> When the SVBI Payload Enable is false I guess the register R1.4
> which contains the Maximum Streamed Vertex Buffer Index is filled by zero
> and GS stops to write transform feedback when the transform feedback
> is not active.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107579
> Signed-off-by: Andrii Simiklit  
> 
> ---
>  src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c 
> b/src/mesa/drivers/dri/i965/genX_state_upload.c
> index ea5ad55..0f82500 100644
> --- a/src/mesa/drivers/dri/i965/genX_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c
> @@ -2806,7 +2806,7 @@ genX(upload_gs_state)(struct brw_context *brw)
>  #if GEN_GEN < 7
>   gs.SOStatisticsEnable = true;
>   if (gs_prog->info.has_transform_feedback_varyings)
> -gs.SVBIPayloadEnable = true;
> +gs.SVBIPayloadEnable = _mesa_is_xfb_active_and_unpaused(ctx);
>
>   /* GEN6_GS_SPF_MODE and GEN6_GS_VECTOR_MASK_ENABLE are enabled as it
>* was previously done for gen6.
>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 07/14] st/mesa: set ctx->Const.SubPixelBits

[Jakob Bornecrantz]

On Thu, Aug 9, 2018 at 12:57 AM Marek Olšák  wrote:
>
> From: Marek Olšák 

This patch causes regressions in dEQP[1] on virgl running on a
radeonSI device. Not a lot of drivers set
PIPE_CAP_VIEWPORT_SUBPIXEL_BITS but SubPixelBits is by default set to
4, but this overwrites it without checking if the returned value is
zero or not. Looking around it seems that a lot of other drivers just
returns zero for PIPE_CAP_VIEWPORT_SUBPIXEL_BITS not just virgl, so
this probably causes regressions on more drivers then virgl.

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

[Mesa-dev] [PATCH] anv: blorp: support multiple aspect blits

[Lionel Landwerlin]

Newer blit tests are enabling depth blits. We currently don't
support it but can do by iterating over the aspects masks (copy some
logic from the CopyImage function).

Signed-off-by: Lionel Landwerlin 
Fixes: 9f44745eca0e41 ("anv: Use blorp to implement VkBlitImage")
---
 src/intel/vulkan/anv_blorp.c | 145 ++-
 1 file changed, 75 insertions(+), 70 deletions(-)

diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
index cd67cc636b2..35b304f92b3 100644
--- a/src/intel/vulkan/anv_blorp.c
+++ b/src/intel/vulkan/anv_blorp.c
@@ -532,81 +532,86 @@ void anv_CmdBlitImage(
   const VkImageSubresourceLayers *src_res = [r].srcSubresource;
   const VkImageSubresourceLayers *dst_res = [r].dstSubresource;
 
-  get_blorp_surf_for_anv_image(cmd_buffer->device,
-   src_image, src_res->aspectMask,
-   srcImageLayout, ISL_AUX_USAGE_NONE, );
-  get_blorp_surf_for_anv_image(cmd_buffer->device,
-   dst_image, dst_res->aspectMask,
-   dstImageLayout, ISL_AUX_USAGE_NONE, );
-
-  struct anv_format_plane src_format =
- anv_get_format_plane(_buffer->device->info, src_image->vk_format,
-  src_res->aspectMask, src_image->tiling);
-  struct anv_format_plane dst_format =
- anv_get_format_plane(_buffer->device->info, dst_image->vk_format,
-  dst_res->aspectMask, dst_image->tiling);
-
-  unsigned dst_start, dst_end;
-  if (dst_image->type == VK_IMAGE_TYPE_3D) {
- assert(dst_res->baseArrayLayer == 0);
- dst_start = pRegions[r].dstOffsets[0].z;
- dst_end = pRegions[r].dstOffsets[1].z;
-  } else {
- dst_start = dst_res->baseArrayLayer;
- dst_end = dst_start + anv_get_layerCount(dst_image, dst_res);
-  }
-
-  unsigned src_start, src_end;
-  if (src_image->type == VK_IMAGE_TYPE_3D) {
- assert(src_res->baseArrayLayer == 0);
- src_start = pRegions[r].srcOffsets[0].z;
- src_end = pRegions[r].srcOffsets[1].z;
-  } else {
- src_start = src_res->baseArrayLayer;
- src_end = src_start + anv_get_layerCount(src_image, src_res);
-  }
-
-  bool flip_z = flip_coords(_start, _end, _start, _end);
-  float src_z_step = (float)(src_end + 1 - src_start) /
- (float)(dst_end + 1 - dst_start);
+  assert(anv_image_aspects_compatible(src_res->aspectMask,
+  dst_res->aspectMask));
+
+  uint32_t aspect_bit;
+  anv_foreach_image_aspect_bit(aspect_bit, src_image, src_res->aspectMask) 
{
+ get_blorp_surf_for_anv_image(cmd_buffer->device,
+  src_image, 1U << aspect_bit,
+  srcImageLayout, ISL_AUX_USAGE_NONE, 
);
+ get_blorp_surf_for_anv_image(cmd_buffer->device,
+  dst_image, 1U << aspect_bit,
+  dstImageLayout, ISL_AUX_USAGE_NONE, 
);
+
+ struct anv_format_plane src_format =
+anv_get_format_plane(_buffer->device->info, 
src_image->vk_format,
+ 1U << aspect_bit, src_image->tiling);
+ struct anv_format_plane dst_format =
+anv_get_format_plane(_buffer->device->info, 
dst_image->vk_format,
+ 1U << aspect_bit, dst_image->tiling);
+
+ unsigned dst_start, dst_end;
+ if (dst_image->type == VK_IMAGE_TYPE_3D) {
+assert(dst_res->baseArrayLayer == 0);
+dst_start = pRegions[r].dstOffsets[0].z;
+dst_end = pRegions[r].dstOffsets[1].z;
+ } else {
+dst_start = dst_res->baseArrayLayer;
+dst_end = dst_start + anv_get_layerCount(dst_image, dst_res);
+ }
 
-  if (flip_z) {
- src_start = src_end;
- src_z_step *= -1;
-  }
+ unsigned src_start, src_end;
+ if (src_image->type == VK_IMAGE_TYPE_3D) {
+assert(src_res->baseArrayLayer == 0);
+src_start = pRegions[r].srcOffsets[0].z;
+src_end = pRegions[r].srcOffsets[1].z;
+ } else {
+src_start = src_res->baseArrayLayer;
+src_end = src_start + anv_get_layerCount(src_image, src_res);
+ }
 
-  unsigned src_x0 = pRegions[r].srcOffsets[0].x;
-  unsigned src_x1 = pRegions[r].srcOffsets[1].x;
-  unsigned dst_x0 = pRegions[r].dstOffsets[0].x;
-  unsigned dst_x1 = pRegions[r].dstOffsets[1].x;
-  bool flip_x = flip_coords(_x0, _x1, _x0, _x1);
+ bool flip_z = flip_coords(_start, _end, _start, _end);
+ float src_z_step = (float)(src_end + 1 - src_start) /
+(float)(dst_end + 1 - dst_start);
 
-  unsigned src_y0 = pRegions[r].srcOffsets[0].y;
-  unsigned src_y1 = 

Re: [Mesa-dev] [PATCH 0/2] Implement INTEL_fragment_shader_ordering

[Manolova, Plamena]

Actually ignore my comment, I forgot that docs/features.txt is for
non-vendor specific
features only. I'll go ahead and push your patches. Sorry for the confusion.

Thanks,
Pam

On Tue, Aug 28, 2018 at 1:04 PM Manolova, Plamena <
plamena.manol...@intel.com> wrote:

> Hi Kevin,
> Could you also mark the extension as done in docs/features.txt? Otherwise
> the series looks good to me
> and with that tiny change it's Reviewed-by: Plamena Manolova <
> plamena.manol...@intel.com>
>
> Thanks,
> Pam
>
> On Mon, Aug 27, 2018 at 9:56 AM  wrote:
>
>> From: Kevin Rogovin 
>>
>> INTEL_fragment_shader_ordering provides the ability for shaders
>> to issue a call to gaurnantee memory write operation ordering
>> of overlapping pixels or samples. In contrast to
>> ARB_fragment_shader_interlock, INTEL_fragment_shader_ordering
>> instead of defining a critical region (which must be in main() and
>> under no flow control) provides a single function that acts like
>> a memory barrier that can be called under any control flow.
>>
>> Kevin Rogovin (2):
>>   mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering.
>>   i965: Add INTEL_fragment_shader_ordering support.
>>
>>  docs/relnotes/18.3.0.html|  1 +
>>  src/compiler/glsl/builtin_functions.cpp  | 17 +
>>  src/compiler/glsl/glsl_parser_extras.cpp |  1 +
>>  src/compiler/glsl/glsl_parser_extras.h   |  2 ++
>>  src/compiler/glsl/glsl_to_nir.cpp|  6 ++
>>  src/compiler/glsl/ir.h   |  1 +
>>  src/compiler/nir/nir_intrinsics.py   |  1 +
>>  src/intel/compiler/brw_fs_nir.cpp|  1 +
>>  src/mesa/drivers/dri/i965/intel_extensions.c |  1 +
>>  src/mesa/main/extensions_table.h |  1 +
>>  src/mesa/main/mtypes.h   |  1 +
>>  11 files changed, 33 insertions(+)
>>
>> --
>> 2.17.1
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 107654] Account request Kevin Rogovin

[bugzilla-daemon]

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

Daniel Stone  changed:

   What|Removed |Added

 CC||ja...@jlekstrand.net

--- Comment #4 from Daniel Stone  ---
Ken, Jason, someone - can you please change the Mesa docs so they reference
creating a GitLab account and requesting access there (Bugzilla, list, not sure
which you'd prefer), rather than attaching GPG keys etc for a legacy account? I
had a quick look at Mesa's docs to find out where that might be, but couldn't
see it.

-- 
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 0/2] Implement INTEL_fragment_shader_ordering

[Manolova, Plamena]

Hi Kevin,
Could you also mark the extension as done in docs/features.txt? Otherwise
the series looks good to me
and with that tiny change it's Reviewed-by: Plamena Manolova <
plamena.manol...@intel.com>

Thanks,
Pam

On Mon, Aug 27, 2018 at 9:56 AM  wrote:

> From: Kevin Rogovin 
>
> INTEL_fragment_shader_ordering provides the ability for shaders
> to issue a call to gaurnantee memory write operation ordering
> of overlapping pixels or samples. In contrast to
> ARB_fragment_shader_interlock, INTEL_fragment_shader_ordering
> instead of defining a critical region (which must be in main() and
> under no flow control) provides a single function that acts like
> a memory barrier that can be called under any control flow.
>
> Kevin Rogovin (2):
>   mesa: Add GL/GLSL plumbing for INTEL_fragment_shader_ordering.
>   i965: Add INTEL_fragment_shader_ordering support.
>
>  docs/relnotes/18.3.0.html|  1 +
>  src/compiler/glsl/builtin_functions.cpp  | 17 +
>  src/compiler/glsl/glsl_parser_extras.cpp |  1 +
>  src/compiler/glsl/glsl_parser_extras.h   |  2 ++
>  src/compiler/glsl/glsl_to_nir.cpp|  6 ++
>  src/compiler/glsl/ir.h   |  1 +
>  src/compiler/nir/nir_intrinsics.py   |  1 +
>  src/intel/compiler/brw_fs_nir.cpp|  1 +
>  src/mesa/drivers/dri/i965/intel_extensions.c |  1 +
>  src/mesa/main/extensions_table.h |  1 +
>  src/mesa/main/mtypes.h   |  1 +
>  11 files changed, 33 insertions(+)
>
> --
> 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] intel: decoder: handle 0 sized structs

[Lionel Landwerlin]

Thanks Andres,

Please see my response to Re: [Mesa-dev] [PATCH] intel: decoder: unify 
MI_BB_START field naming


On 27/08/2018 22:20, Andres Gomez wrote:

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


On Sat, 2018-08-25 at 18:23 +0100, Lionel Landwerlin wrote:

Gen7.5 has a BLEND_STATE of size 0 which includes a variable length
group. We did not deal with that very well, leading to an endless
loop.

Signed-off-by: Lionel Landwerlin 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
---
  src/intel/common/gen_decoder.c | 12 
  1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
index 9e46f271633..ec22b545492 100644
--- a/src/intel/common/gen_decoder.c
+++ b/src/intel/common/gen_decoder.c
@@ -997,7 +997,7 @@ gen_field_iterator_init(struct gen_field_iterator *iter,
 iter->p_bit = p_bit;
  
 int length = gen_group_get_length(iter->group, iter->p);

-   iter->p_end = length > 0 ? [length] : NULL;
+   iter->p_end = length >= 0 ? [length] : NULL;
 iter->print_colors = print_colors;
  }
  
@@ -1012,10 +1012,14 @@ gen_field_iterator_next(struct gen_field_iterator *iter)

   iter_start_field(iter, iter->group->next->fields);
  
bool result = iter_decode_field(iter);

-  if (iter->p_end)
- assert(result);
+  if (!result && iter->p_end) {
+ /* We're dealing with a non empty struct of length=0 (BLEND_STATE on
+  * Gen 7.5)
+  */
+ assert(iter->group->dw_length == 0);
+  }
  
-  return true;

+  return result;
 }
  
 if (!iter_advance_field(iter))



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

Re: [Mesa-dev] [PATCH] intel: decoder: unify MI_BB_START field naming

[Lionel Landwerlin]

Yes, I think so. You asked on another commit too, both are related and 
this depends on other commits from Jason.


Here is a list in order of cherry picking :

commit f430a37fa75f534c3a114b0ec546fa14f05f5da1
Author: Lionel Landwerlin 
Date:   Tue Aug 14 11:22:12 2018 +0100

    intel: decoder: unify MI_BB_START field naming

commit 2abd7ae189135eb5a1f530a3a1c9412d3a7e238d
Author: Jason Ekstrand 
Date:   Fri Aug 24 15:23:04 2018 -0500

    intel/decoder: Clean up field iteration and fix sub-dword fields

commit cbd4bc1346f7397242e157bb66099b950a8c5643
Author: Jason Ekstrand 
Date:   Fri Aug 24 16:04:03 2018 -0500

    intel/batch_decoder: Fix dynamic state printing

commit 70de31d0c106f58d6b7e6d5b79b8d90c1c112a3b
Author: Jason Ekstrand 
Date:   Fri Aug 24 16:05:08 2018 -0500

    intel/batch_decoder: Print blend states properly


commit 440a988bd1478bb33dafcbb8575473bc643ae383
Author: Lionel Landwerlin 
Date:   Sat Aug 25 18:22:00 2018 +0100

    intel: decoder: handle 0 sized structs

Thanks,

-
Lionel

On 27/08/2018 22:20, Andres Gomez wrote:

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

On Tue, 2018-08-14 at 11:26 +0100, Lionel Landwerlin wrote:

The batch decoder looks for a field with a particular name to decide
whether an MI_BB_START leads into a second batch buffer level. Because
the names are different between Gen7.5/8 and the newer generation we
fail that test and keep on reading (invalid) instructions.

Signed-off-by: Lionel Landwerlin 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
---
  src/intel/genxml/gen75.xml | 6 +++---
  src/intel/genxml/gen8.xml  | 6 +++---
  src/intel/vulkan/anv_batch_chain.c | 2 +-
  3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
index 5b01fd45400..dfc3d891498 100644
--- a/src/intel/genxml/gen75.xml
+++ b/src/intel/genxml/gen75.xml
@@ -2314,9 +2314,9 @@

  
  
-
-  
-  
+
+  
+  
  
  
  
diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
index 4ed41d15612..330366b7ed0 100644
--- a/src/intel/genxml/gen8.xml
+++ b/src/intel/genxml/gen8.xml
@@ -2553,9 +2553,9 @@

  
  
-
-  
-  
+
+  
+  
  
  
  
diff --git a/src/intel/vulkan/anv_batch_chain.c 
b/src/intel/vulkan/anv_batch_chain.c
index c47a81c8a4d..0f7c8325ea4 100644
--- a/src/intel/vulkan/anv_batch_chain.c
+++ b/src/intel/vulkan/anv_batch_chain.c
@@ -531,7 +531,7 @@ emit_batch_buffer_start(struct anv_cmd_buffer *cmd_buffer,
 anv_batch_emit(_buffer->batch, GEN8_MI_BATCH_BUFFER_START, bbs) {
bbs.DWordLength   = cmd_buffer->device->info.gen < 8 ?
gen7_length : gen8_length;
-  bbs._2ndLevelBatchBuffer  = _1stlevelbatch;
+  bbs.SecondLevelBatchBuffer= Firstlevelbatch;
bbs.AddressSpaceIndicator = ASI_PPGTT;
bbs.BatchBufferStartAddress   = (struct anv_address) { bo, offset };
 }



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

Re: [Mesa-dev] [PATCH] nir: Remove outdated comment

[Alejandro Piñeiro]

On 28/08/18 03:18, Jason Ekstrand wrote:
> Reviewed-by: Jason Ekstrand  >
>
> On Mon, Aug 27, 2018 at 5:20 PM Caio Marcelo de Oliveira Filho
> mailto:caio.olive...@intel.com>> wrote:
>
> ---
>
> The move of comapre functions landed before the suggestion to remove
>

typo: comapre => compare

> the comment, so removing it now.
>
>  src/compiler/nir/nir_deref.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/src/compiler/nir/nir_deref.c
> b/src/compiler/nir/nir_deref.c
> index c8851688f9d..097ea8f1046 100644
> --- a/src/compiler/nir/nir_deref.c
> +++ b/src/compiler/nir/nir_deref.c
> @@ -271,9 +271,6 @@ nir_fixup_deref_modes(nir_shader *shader)
>     }
>  }
>
> -/** Returns true if the storage referrenced to by deref
> completely contains
> - * the storage referenced by sub.
> - */
>  nir_deref_compare_result
>  nir_compare_deref_paths(nir_deref_path *a_path,
>                          nir_deref_path *b_path)
> -- 
> 2.18.0
>
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Mesa-dev] [Bug 107698] Ubuntu 18.10 kernel 4.18rc1-4.18.6 or kernel 4.19rc1

[bugzilla-daemon]

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

Michel Dänzer  changed:

   What|Removed |Added

 CC||harry.wentl...@amd.com,
   ||sunpeng...@amd.com

--- Comment #4 from Michel Dänzer  ---
Does amdgpu.dc=0 on the kernel command line avoid the black screen?

Does the screen go black when the amdgpu driver is loaded, or only when the
login screen is supposed to start?

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

[Mesa-dev] [Bug 105731] linker error "fragment shader input ... has no matching output in the previous stage" when previous stage's output declaration in a separate shader object

[bugzilla-daemon]

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

--- Comment #1 from vadym  ---
Patch: https://patchwork.freedesktop.org/patch/246171/

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

Re: [Mesa-dev] [PATCH] glsl/linker: Link all out vars from a shader objects on a single stage

[Vadim Shovkoplias]

Hi Timothy,

Thanks for the review! Space was removed. Can you please push this patch
since I haven't got write permissions ?

Regards,
Vadym

вт, 28 авг. 2018 г. в 10:32, Vadym Shovkoplias :

> From: "vadym.shovkoplias" 
>
> During intra stage linking some out variables can be dropped because
> it is not used in a shader with the main function. But these out vars
> can be referenced on later stages which can lead to further linking
> errors.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105731
> Signed-off-by: Vadym Shovkoplias 
> ---
>  src/compiler/glsl/linker.cpp | 37 
>  1 file changed, 37 insertions(+)
>
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 3ce78fe642..dbd76b7fcc 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -2187,6 +2187,40 @@ link_cs_input_layout_qualifiers(struct
> gl_shader_program *prog,
> }
>  }
>
> +/**
> + * Link all out variables on a single stage which are not
> + * directly used in a shader with the main function.
> + */
> +static void
> +link_output_variables(struct gl_linked_shader *linked_shader,
> +  struct gl_shader **shader_list,
> +  unsigned num_shaders)
> +{
> +   struct glsl_symbol_table *symbols = linked_shader->symbols;
> +
> +   for (unsigned i = 0; i < num_shaders; i++) {
> +
> +  /* Skip shader object with main function */
> +  if (shader_list[i]->symbols->get_function("main"))
> + continue;
> +
> +  foreach_in_list (ir_instruction, ir, shader_list[i]->ir) {
> + if (ir->ir_type != ir_type_variable)
> +continue;
> +
> + ir_variable *const var = (ir_variable *) ir;
> +
> + if (var->data.mode == ir_var_shader_out &&
> +   !symbols->get_variable(var->name)) {
> +symbols->add_variable(var);
> +linked_shader->ir->push_head(var);
> + }
> +  }
> +   }
> +
> +   return;
> +}
> +
>
>  /**
>   * Combine a group of shaders for a single stage to generate a linked
> shader
> @@ -2352,6 +2386,9 @@ link_intrastage_shaders(void *mem_ctx,
>return NULL;
> }
>
> +   if (linked->Stage != MESA_SHADER_FRAGMENT)
> +  link_output_variables(linked, shader_list, num_shaders);
> +
> /* Make a pass over all variable declarations to ensure that arrays
> with
>  * unspecified sizes have a size specified.  The size is inferred from
> the
>  * max_array_access field.
> --
> 2.18.0
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] glsl/linker: Link all out vars from a shader objects on a single stage

[Vadym Shovkoplias]

From: "vadym.shovkoplias" 

During intra stage linking some out variables can be dropped because
it is not used in a shader with the main function. But these out vars
can be referenced on later stages which can lead to further linking
errors.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105731
Signed-off-by: Vadym Shovkoplias 
---
 src/compiler/glsl/linker.cpp | 37 
 1 file changed, 37 insertions(+)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 3ce78fe642..dbd76b7fcc 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -2187,6 +2187,40 @@ link_cs_input_layout_qualifiers(struct gl_shader_program 
*prog,
}
 }
 
+/**
+ * Link all out variables on a single stage which are not
+ * directly used in a shader with the main function.
+ */
+static void
+link_output_variables(struct gl_linked_shader *linked_shader,
+  struct gl_shader **shader_list,
+  unsigned num_shaders)
+{
+   struct glsl_symbol_table *symbols = linked_shader->symbols;
+
+   for (unsigned i = 0; i < num_shaders; i++) {
+
+  /* Skip shader object with main function */
+  if (shader_list[i]->symbols->get_function("main"))
+ continue;
+
+  foreach_in_list (ir_instruction, ir, shader_list[i]->ir) {
+ if (ir->ir_type != ir_type_variable)
+continue;
+
+ ir_variable *const var = (ir_variable *) ir;
+
+ if (var->data.mode == ir_var_shader_out &&
+   !symbols->get_variable(var->name)) {
+symbols->add_variable(var);
+linked_shader->ir->push_head(var);
+ }
+  }
+   }
+
+   return;
+}
+
 
 /**
  * Combine a group of shaders for a single stage to generate a linked shader
@@ -2352,6 +2386,9 @@ link_intrastage_shaders(void *mem_ctx,
   return NULL;
}
 
+   if (linked->Stage != MESA_SHADER_FRAGMENT)
+  link_output_variables(linked, shader_list, num_shaders);
+
/* Make a pass over all variable declarations to ensure that arrays with
 * unspecified sizes have a size specified.  The size is inferred from the
 * max_array_access field.
-- 
2.18.0

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

Re: [Mesa-dev] [RFC 00/10] Let us welcome EGLDevice

[Mathias Fröhlich]

Hi Emil,

On Friday, 3 August 2018 14:44:16 CEST Emil Velikov wrote:
> Hi all,
> 
> This series implements the following extensions:
>  - EGL_EXT_device_base
>  - EGL_MESA_device_software
>  - EGL_EXT_device_drm
>  - EGL_platform_device
> 
> As you know the APIs are used to enumerate, select and use EGLDevices.
> The second extension (proposed by Ajax) defines a 'software' device,
> alongside the existing DRM ones.
> 
> While there are many usecases for this work, my primary interest is
> allowing device selection, wrt testing. To achieve the goal, we would
> need to finalise EGL_EXT_explicit_device (also proposed by Ajax).
> 
> Piglit patches will be sent out shortly.
> 
> Any feedback is greatly appreciated.

>From a higher point of view the approach looks good.

To sum up, you basically associate an _EGLDevice with each _EGLDisplay
where the _EGLDevice only contains the meta information where to
find the device and the _EGLDisplay later contains open file descriptors
to work with ...

Nevertheless, running the tests and proof of concept programs that I used
back in the day brought up some questions and one crash.

At first the Crash: The attached eglcontext-pbuffer.c which goes the pbuffer 
approach
instead of going surfaceless just dumps core in pbuffer creation using
the patch series. I believe that it is legal what the program does, but may
be you want to double check that too.

Then if I use the patch series on an account that has no DISPLAY set and no
'display server' running, eglInitialize fails in device_probe_device due to at 
first
opening the '.../card0' device and bailing out when this does not work.
Means the current patch series goes via opening the primary node which
shall not be accessible by everyone and from that derives the rendernode
device which is finally used for _EGLDisplay initialization.
Can we alternatively go directly to the rendernode in some way?

For patch #7, can you explain why dri already provides the right format?
It's probably correct, but I am missing some bits of that context creation
big picture to give a review.

And finally, in patch #2 you mention that you want to avoid larger patches
and the announced extensions are not yet working as expected.
May be you can announce the extensions in a separate patch that follows
all the implementation patches? That probably helps people that want to
bisect something using piglit.

thanks for approaching this topic!

best
Mathias


> 
> Thanks
> Emil
> 
> Emil Velikov (9):
>   egl: add simple EGL_EXT_device_base implementation
>   egl: add EGL_MESA_device_software support
>   egl: add EGL_EXT_device_drm support
>   meson: egl: group dri2 bits separately from haiku
>   egl/surfaceless: inline surfaceless_alloc_image()
>   egl/surfaceless: honour the format passed to getBuffers
>   egl/surfaceless: remove no longer used dri2_egl_surface::visual
>   egl: add optional plat_opt to _eglFindDisplay()
>   egl: add EGL_platform_device support
> 
> Jonny Lamb (1):
>   egl: add initial boilerplate for EGL_EXT_device_base
> 
>  src/egl/Makefile.am |   3 +
>  src/egl/Makefile.sources|   2 +
>  src/egl/drivers/dri2/egl_dri2.c |   3 +
>  src/egl/drivers/dri2/egl_dri2.h |  13 +-
>  src/egl/drivers/dri2/platform_android.c |   9 +
>  src/egl/drivers/dri2/platform_device.c  | 380 
>  src/egl/drivers/dri2/platform_drm.c |   9 +
>  src/egl/drivers/dri2/platform_surfaceless.c |  37 +-
>  src/egl/drivers/dri2/platform_wayland.c |  18 +
>  src/egl/drivers/dri2/platform_x11.c |  27 ++
>  src/egl/drivers/haiku/egl_haiku.cpp |   8 +
>  src/egl/main/eglapi.c   |  79 +++-
>  src/egl/main/egldevice.c| 306 
>  src/egl/main/egldevice.h|  83 +
>  src/egl/main/egldisplay.c   | 119 +-
>  src/egl/main/egldisplay.h   |  14 +-
>  src/egl/main/eglentrypoint.h|   4 +
>  src/egl/main/eglglobals.c   |  10 +-
>  src/egl/main/eglglobals.h   |   2 +
>  src/egl/main/egltypedefs.h  |   2 +
>  src/egl/meson.build |  68 ++--
>  21 files changed, 1120 insertions(+), 76 deletions(-)
>  create mode 100644 src/egl/drivers/dri2/platform_device.c
>  create mode 100644 src/egl/main/egldevice.c
>  create mode 100644 src/egl/main/egldevice.h
> 
> 

// For RTLD_DEFAULT
#define _GNU_SOURCE

#include 
#include 
#include 
#include 

// open/close for gbm
#include 
#include 
#include 
#include 
#include 

#include 
#include 
#include 

static int
check_extension(EGLDisplay dpy, const char* name)
{
const char *extensions = eglQueryString(dpy, EGL_EXTENSIONS);
// It does not hurt to check this always, but by spec only
// required if we look for client extensions
if (/*dpy == EGL_NO_DISPLAY && */ !extensions)
  return 0;
// Not great as we match 

Re: [Mesa-dev] [PATCH] glsl/linker: Allow unused in blocks which are not declated on previous stage

[Vadym Shovkoplias]

Hi Andreas,

Similar patch for varyings linking was pushed 4 years ago, so I think this
patch should be also stable.


On Tue, Aug 28, 2018 at 12:20 AM, Andres Gomez  wrote:

> Vadym, should we also include this in the stable queues ?
>
>
> On Mon, 2018-08-20 at 16:31 +0300, vadym.shovkoplias wrote:
> > From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
> >
> > "Only the input variables that are actually read need to be written
> >  by the previous stage; it is allowed to have superfluous
> >  declarations of input variables."
> >
> > Fixes:
> > * interstage-multiple-shader-objects.shader_test
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101247
> > Signed-off-by: Vadym Shovkoplias 
> > ---
> >  src/compiler/glsl/link_interface_blocks.cpp | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/compiler/glsl/link_interface_blocks.cpp
> b/src/compiler/glsl/link_interface_blocks.cpp
> > index e5eca9460e..801fbcd5d9 100644
> > --- a/src/compiler/glsl/link_interface_blocks.cpp
> > +++ b/src/compiler/glsl/link_interface_blocks.cpp
> > @@ -417,9 +417,15 @@ validate_interstage_inout_blocks(struct
> gl_shader_program *prog,
> > * write to any of the pre-defined outputs (e.g. if the vertex
> shader
> > * does not write to gl_Position, etc), which is allowed and
> results in
> > * undefined behavior.
> > +   *
> > +   * From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
> > +   *
> > +   *"Only the input variables that are actually read need to be
> written
> > +   * by the previous stage; it is allowed to have superfluous
> > +   * declarations of input variables."
> > */
> >if (producer_def == NULL &&
> > -  !is_builtin_gl_in_block(var, consumer->Stage)) {
> > +  !is_builtin_gl_in_block(var, consumer->Stage) &&
> var->data.used) {
> >   linker_error(prog, "Input block `%s' is not an output of "
> >"the previous stage\n",
> var->get_interface_type()->name);
> >   return;
> --
> Br,
>
> Andres
>



-- 

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

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

[Mesa-dev] [Bug 107655] X segfaults on startup in r300_dri.so, making system unusable

[bugzilla-daemon]

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

Michal Srb  changed:

   What|Removed |Added

 CC||michal...@gmail.com

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