Re: [Mesa-dev] [PATCH] android: st/mesa: fix building error due to sched_getcpu()

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Fri, Nov 30, 2018 at 7:03 PM Mauro Rossi  wrote:

> Android has cpufeatures library but pinning of threads is not supported
> PIPE_OS_LINUX code path causes build error due to sched_getcpu()
> unavailable
> thus we need to avoid setting HAVE_SCHED_GETCPU for Android
>
> Fixes: 48f2160 ("st/mesa: regularly re-pin driver threads to the CCX where
> the app thread is")
> Signed-off-by: Mauro Rossi 
> ---
>  src/mesa/state_tracker/st_draw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/state_tracker/st_draw.c
> b/src/mesa/state_tracker/st_draw.c
> index 5910ffa5bd..9600b1569d 100644
> --- a/src/mesa/state_tracker/st_draw.c
> +++ b/src/mesa/state_tracker/st_draw.c
> @@ -67,7 +67,7 @@
>  #include "draw/draw_context.h"
>  #include "cso_cache/cso_context.h"
>
> -#ifdef PIPE_OS_LINUX
> +#if defined(PIPE_OS_LINUX) && !defined(ANDROID)
>  #include 
>  #define HAVE_SCHED_GETCPU 1
>  #else
> --
> 2.19.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 v2 00/29] do not allow invalid texture-format enums

[Matt Turner]

On Fri, Nov 30, 2018 at 4:46 AM Francesco Ansanelli  wrote:
> Il giorno ven 30 nov 2018, 12:26 Erik Faye-Lund 
>  ha scritto:
>>
>> On Fri, 2018-11-23 at 11:53 +0100, Erik Faye-Lund wrote:
>> > OK, so here's a v2 of this series. These are the changes since v1:
>> > - Removed double-semicolons in patch #25
>> > - Removed default-case and questionable comment in patch #25
>> > - Dropped patch #30, as it would regress functionality on two drivers
>> >
>> > Please review :)
>>
>> Ping? Any takers?
>
> I read all the patches and didn't spot anything wrong, but please don't 
> consider this a proper review.

Thank you for speaking up. Even if you don't feel like your
Reviewed-by tag is sufficient it's still valuable! Feel free to give
it when you have reviewed patches to the best of your ability.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 03/29] mesa/main: clean up S3_s3tc check

[Ian Romanick]

On 11/30/2018 03:15 PM, Marek Olšák wrote:
> On Fri, Nov 30, 2018 at 5:57 PM Ian Romanick  > wrote:
> 
> On 11/30/2018 02:17 PM, Marek Olšák wrote:
> >
> >
> > On Fri, Nov 23, 2018 at 5:54 AM Erik Faye-Lund
> >  
>  >> wrote:
> >
> >     S3_s3tc is the extension that enables this functionality on
> desktop, so
> >     let's check for that one. The _mesa_has_S3_s3tc() helper already
> >     verifies the API according to the extension-table.
> >
> >     Signed-off-by: Erik Faye-Lund  
> >      >>
> >     ---
> >      src/mesa/main/glformats.c | 8 +++-
> >      1 file changed, 3 insertions(+), 5 deletions(-)
> >
> >     diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> >     index 9a507d11b96..b2c18aa6d94 100644
> >     --- a/src/mesa/main/glformats.c
> >     +++ b/src/mesa/main/glformats.c
> >     @@ -1352,8 +1352,7 @@ _mesa_is_compressed_format(const struct
> >     gl_context *ctx, GLenum format)
> >         case GL_RGB4_S3TC:
> >         case GL_RGBA_S3TC:
> >         case GL_RGBA4_S3TC:
> >     -      return _mesa_is_desktop_gl(ctx) &&
> >     -         ctx->Extensions.ANGLE_texture_compression_dxt;
> >     +      return _mesa_has_S3_s3tc(ctx);
> >         case GL_COMPRESSED_LUMINANCE_ALPHA_3DC_ATI:
> >            return ctx->API == API_OPENGL_COMPAT
> >               && ctx->Extensions.ATI_texture_compression_3dc;
> >     @@ -1378,9 +1377,8 @@ _mesa_is_compressed_format(const struct
> >     gl_context *ctx, GLenum format)
> >                */
> >               return ctx->Extensions.ANGLE_texture_compression_dxt;
> >            } else {
> >     -         return _mesa_is_desktop_gl(ctx)
> >     -            && ctx->Extensions.EXT_texture_sRGB
> >     -            && ctx->Extensions.EXT_texture_compression_s3tc;
> >     +         return _mesa_has_EXT_texture_sRGB(ctx) &&
> >     +            _mesa_has_S3_s3tc(ctx);
> >
> > This looks like it should be _mesa_has_EXT_texture_compression_s3tc.
> 
> I haven't looked at the code to verify, but I think
> _mesa_has_EXT_texture_compression_s3tc can be true in OpenGL ES, but
> _mesa_has_S3_s3tc cannot.  The original code has _mesa_is_desktop_gl, so
> we don't want to allow ES.  But I also thought _mesa_has_S3_s3tc wasn't
> allowed in core profile, so that might not be right either?
> 
> 
> They are all allowed by core & compat. EXT_texture_sRGB is only
> available in desktop GL, so any s3tc extension should work, but
> EXT_texture_sRGB defines interactions with EXT_texture_compression_s3tc,
> not S3_s3tc.

Ah.  That makes sense.

> Marek

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

[Mesa-dev] [PATCH] android: st/mesa: fix building error due to sched_getcpu()

[Mauro Rossi]

Android has cpufeatures library but pinning of threads is not supported
PIPE_OS_LINUX code path causes build error due to sched_getcpu() unavailable
thus we need to avoid setting HAVE_SCHED_GETCPU for Android

Fixes: 48f2160 ("st/mesa: regularly re-pin driver threads to the CCX where the 
app thread is")
Signed-off-by: Mauro Rossi 
---
 src/mesa/state_tracker/st_draw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_draw.c b/src/mesa/state_tracker/st_draw.c
index 5910ffa5bd..9600b1569d 100644
--- a/src/mesa/state_tracker/st_draw.c
+++ b/src/mesa/state_tracker/st_draw.c
@@ -67,7 +67,7 @@
 #include "draw/draw_context.h"
 #include "cso_cache/cso_context.h"
 
-#ifdef PIPE_OS_LINUX
+#if defined(PIPE_OS_LINUX) && !defined(ANDROID)
 #include 
 #define HAVE_SCHED_GETCPU 1
 #else
-- 
2.19.1

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

Re: [Mesa-dev] [PATCH 4/4] nir: detect more induction variables

[Thomas Helland]

I've done a couple passes over the patches now.
Neatly implemented and look correct to me.
With the two small nitpicks below correct this whole series is:

Reviewed-by: Thomas Helland 

Den ons. 28. nov. 2018 kl. 04:26 skrev Timothy Arceri :
>
> This adds allows loop analysis to detect inductions varibales that

This reads wierd. And s/varibales/variables

> are incremented in both branches of an if rather than in a main
> loop block. For example:
>
>loop {
>   block block_1:
>   /* preds: block_0 block_7 */
>   vec1 32 ssa_8 = phi block_0: ssa_4, block_7: ssa_20
>   vec1 32 ssa_9 = phi block_0: ssa_0, block_7: ssa_4
>   vec1 32 ssa_10 = phi block_0: ssa_1, block_7: ssa_4
>   vec1 32 ssa_11 = phi block_0: ssa_2, block_7: ssa_21
>   vec1 32 ssa_12 = phi block_0: ssa_3, block_7: ssa_22
>   vec4 32 ssa_13 = vec4 ssa_12, ssa_11, ssa_10, ssa_9
>   vec1 32 ssa_14 = ige ssa_8, ssa_5
>   /* succs: block_2 block_3 */
>   if ssa_14 {
>  block block_2:
>  /* preds: block_1 */
>  break
>  /* succs: block_8 */
>   } else {
>  block block_3:
>  /* preds: block_1 */
>  /* succs: block_4 */
>   }
>   block block_4:
>   /* preds: block_3 */
>   vec1 32 ssa_15 = ilt ssa_6, ssa_8
>   /* succs: block_5 block_6 */
>   if ssa_15 {
>  block block_5:
>  /* preds: block_4 */
>  vec1 32 ssa_16 = iadd ssa_8, ssa_7
>  vec1 32 ssa_17 = load_const (0x3f80 /* 1.00*/)
>  /* succs: block_7 */
>   } else {
>  block block_6:
>  /* preds: block_4 */
>  vec1 32 ssa_18 = iadd ssa_8, ssa_7
>  vec1 32 ssa_19 = load_const (0x3f80 /* 1.00*/)
>  /* succs: block_7 */
>   }
>   block block_7:
>   /* preds: block_5 block_6 */
>   vec1 32 ssa_20 = phi block_5: ssa_16, block_6: ssa_18
>   vec1 32 ssa_21 = phi block_5: ssa_17, block_6: ssa_4
>   vec1 32 ssa_22 = phi block_5: ssa_4, block_6: ssa_19
>   /* succs: block_1 */
>}
>
> Unfortunatly GCM could move the addition out of the if for us
> (making this patch unrequired) but we still cannot enable the GCM
> pass without regressions.
>
> This unrolls a loop in Rise of The Tomb Raider.
>
> vkpipeline-db results (VEGA):
>
> Totals from affected shaders:
> SGPRS: 88 -> 96 (9.09 %)
> VGPRS: 56 -> 52 (-7.14 %)
> Spilled SGPRs: 0 -> 0 (0.00 %)
> Spilled VGPRs: 0 -> 0 (0.00 %)
> Private memory VGPRs: 0 -> 0 (0.00 %)
> Scratch size: 0 -> 0 (0.00 %) dwords per thread
> Code Size: 2168 -> 4560 (110.33 %) bytes
> LDS: 0 -> 0 (0.00 %) blocks
> Max Waves: 4 -> 4 (0.00 %)
> Wait states: 0 -> 0 (0.00 %)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=32211
> ---
>  src/compiler/nir/nir_loop_analyze.c | 36 +
>  1 file changed, 36 insertions(+)
>
> diff --git a/src/compiler/nir/nir_loop_analyze.c 
> b/src/compiler/nir/nir_loop_analyze.c
> index 8903e15105..cf97d6bf06 100644
> --- a/src/compiler/nir/nir_loop_analyze.c
> +++ b/src/compiler/nir/nir_loop_analyze.c
> @@ -245,6 +245,42 @@ compute_induction_information(loop_info_state *state)
>   if (src_var->in_if_branch || src_var->in_nested_loop)
>  break;
>
> + /* Detect inductions varibales that are incremented in both branches

s/varibales/variables

> +  * of an unnested if rather than in a loop block.
> +  */
> + if (is_var_phi(src_var)) {
> +nir_phi_instr *src_phi =
> +   nir_instr_as_phi(src_var->def->parent_instr);
> +
> +nir_op alu_op;
> +nir_ssa_def *alu_srcs[2] = {0};
> +nir_foreach_phi_src(src2, src_phi) {
> +   nir_loop_variable *src_var2 =
> +  get_loop_var(src2->src.ssa, state);
> +
> +   if (!src_var2->in_if_branch || !is_var_alu(src_var2))
> +  break;
> +
> +   nir_alu_instr *alu =
> +  nir_instr_as_alu(src_var2->def->parent_instr);
> +   if (nir_op_infos[alu->op].num_inputs != 2)
> +  break;
> +
> +   if (alu->src[0].src.ssa == alu_srcs[0] &&
> +   alu->src[1].src.ssa == alu_srcs[1] &&
> +   alu->op == alu_op) {
> +  /* Both branches perform the same calculation so we can use
> +   * one of them to find the induction variable.
> +   */
> +  src_var = src_var2;
> +   } else {
> +  alu_srcs[0] = alu->src[0].src.ssa;
> +  alu_srcs[1] = alu->src[1].src.ssa;
> +  alu_op = alu->op;
> +   }
> +}
> + }
> +
>   if (!src_var->in_loop) {
>  biv->def_outside_loop = src_var;
>   } else if (is_var_alu(src_var)) {
> --
> 2.19.1
>
> ___
> mesa-dev mailing list
> 

Re: [Mesa-dev] [PATCH v2 2/5] gallium: Add new PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE

[Marek Olšák]

I don't plan to expose the GCN functionality. If I wanted to, I would add
the sample count into the rasterizer state, because it only affects
gl_SampleMaskIn.

Marek

On Fri, Nov 30, 2018 at 5:12 PM Rob Clark  wrote:

> I guess as long as it is just a single draw, it works out to the same
> thing.  Maybe to expose the GCN functionality we could do something
> like
>
>   PIPE_CAP_SURFACE_SAMPLE_COUNT:
> 0 - unsupported
> 1 - msaa per draw
> 2 - msaa per renderpass
>
> ??
>
> BR,
> -R
>
> On Fri, Nov 30, 2018 at 3:41 PM Marek Olšák  wrote:
> >
> > GCN calls it overrasterization multisampling, but it's really only
> useful for polygon smoothing, because there is no temporary buffer.
> >
> > Marek
> >
> > On Fri, Nov 30, 2018 at 3:35 PM Rob Clark  wrote:
> >>
> >> On Fri, Nov 30, 2018 at 3:25 PM Marek Olšák  wrote:
> >> >
> >> > Suggestions:
> >> > - PIPE_CAP_TILED_BASED_MSAA_OVERSAMPLING
> >> > - pipe_surface::tile_based_oversample_count
> >> >
> >> > I'm assuming this feature isn't possible without tile-based rendering.
> >>
> >> I guess technically an IMR could do it, with an internal temporary
> >> buffer not visible to the state-tracker.  (Not sure I could come up
> >> with a reason *why* you'd want to do that, but...)
> >>
> >> BR,
> >> -R
> >>
> >> >
> >> > Marek
> >> >
> >> > On Fri, Nov 30, 2018 at 1:23 PM Kristian Høgsberg <
> hoegsb...@gmail.com> wrote:
> >> >>
> >> >> On Fri, Nov 30, 2018 at 10:17 AM Marek Olšák 
> wrote:
> >> >> >
> >> >> > On Fri, Nov 30, 2018 at 1:13 PM Kristian Høgsberg <
> hoegsb...@gmail.com> wrote:
> >> >> >>
> >> >> >> On Fri, Nov 16, 2018 at 7:48 PM Marek Olšák 
> wrote:
> >> >> >> >
> >> >> >> > I think the name PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE is
> slightly misleading, because it doesn't imply anything about the OpenGL ES
> behavior, which is that a texture is multisampled in the cache, but
> single-sampled in memory. This should be mentioned somewhere.
> >> >> >>
> >> >> >> Not sure exactly which change you're recommending, but in
> retrospect,
> >> >> >> I think it would be better to name the cap in term of how it
> changes
> >> >> >> the gallium API instead of the GLES extension it enables. How
> about
> >> >> >> PIPE_CAP_SURFACE_SAMPLE_COUNT, to indicate that it allows
> overriding
> >> >> >> sample counts in pipe_surface?
> >> >> >
> >> >> >
> >> >> > That's an interesting idea, but how does it convey that
> multisampled surfaces are never multisampled in memory?
> >> >>
> >> >> How are you going to convey all that in a cap enum name? In general,
> >> >> the cap names are concise hints and not super descriptive - you have
> >> >> to go read the documentation to understand the semantics.
> >> >>
> >> >> Do you have a specific name you'd like to see or can we go with
> >> >> PIPE_CAP_SURFACE_SAMPLE_COUNT?
> >> >>
> >> >> >
> >> >> > Marek
> >> >
> >> > ___
> >> > 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 03/30] mesa/main: clean up S3_s3tc check

[Marek Olšák]

Assuming you use _mesa_has_EXT_texture_compression_s3tc in the second hunk,
the series is:

Reviewed-by: Marek Olšák 

Marek

On Mon, Nov 19, 2018 at 7:15 AM Erik Faye-Lund 
wrote:

> S3_s3tc is the extension that enables this functionality on desktop, so
> let's check for that one. The _mesa_has_S3_s3tc() helper already
> verifies the API according to the extension-table.
>
> Signed-off-by: Erik Faye-Lund 
> ---
>  src/mesa/main/glformats.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index 9a507d11b96..b2c18aa6d94 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -1352,8 +1352,7 @@ _mesa_is_compressed_format(const struct gl_context
> *ctx, GLenum format)
> case GL_RGB4_S3TC:
> case GL_RGBA_S3TC:
> case GL_RGBA4_S3TC:
> -  return _mesa_is_desktop_gl(ctx) &&
> - ctx->Extensions.ANGLE_texture_compression_dxt;
> +  return _mesa_has_S3_s3tc(ctx);
> case GL_COMPRESSED_LUMINANCE_ALPHA_3DC_ATI:
>return ctx->API == API_OPENGL_COMPAT
>   && ctx->Extensions.ATI_texture_compression_3dc;
> @@ -1378,9 +1377,8 @@ _mesa_is_compressed_format(const struct gl_context
> *ctx, GLenum format)
>*/
>   return ctx->Extensions.ANGLE_texture_compression_dxt;
>} else {
> - return _mesa_is_desktop_gl(ctx)
> -&& ctx->Extensions.EXT_texture_sRGB
> -&& ctx->Extensions.EXT_texture_compression_s3tc;
> + return _mesa_has_EXT_texture_sRGB(ctx) &&
> +_mesa_has_S3_s3tc(ctx);
>}
> case MESA_FORMAT_LAYOUT_FXT1:
>return _mesa_is_desktop_gl(ctx)
> --
> 2.19.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 v2 03/29] mesa/main: clean up S3_s3tc check

[Marek Olšák]

On Fri, Nov 30, 2018 at 5:57 PM Ian Romanick  wrote:

> On 11/30/2018 02:17 PM, Marek Olšák wrote:
> >
> >
> > On Fri, Nov 23, 2018 at 5:54 AM Erik Faye-Lund
> > mailto:erik.faye-l...@collabora.com>>
> wrote:
> >
> > S3_s3tc is the extension that enables this functionality on desktop,
> so
> > let's check for that one. The _mesa_has_S3_s3tc() helper already
> > verifies the API according to the extension-table.
> >
> > Signed-off-by: Erik Faye-Lund  > >
> > ---
> >  src/mesa/main/glformats.c | 8 +++-
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> > index 9a507d11b96..b2c18aa6d94 100644
> > --- a/src/mesa/main/glformats.c
> > +++ b/src/mesa/main/glformats.c
> > @@ -1352,8 +1352,7 @@ _mesa_is_compressed_format(const struct
> > gl_context *ctx, GLenum format)
> > case GL_RGB4_S3TC:
> > case GL_RGBA_S3TC:
> > case GL_RGBA4_S3TC:
> > -  return _mesa_is_desktop_gl(ctx) &&
> > - ctx->Extensions.ANGLE_texture_compression_dxt;
> > +  return _mesa_has_S3_s3tc(ctx);
> > case GL_COMPRESSED_LUMINANCE_ALPHA_3DC_ATI:
> >return ctx->API == API_OPENGL_COMPAT
> >   && ctx->Extensions.ATI_texture_compression_3dc;
> > @@ -1378,9 +1377,8 @@ _mesa_is_compressed_format(const struct
> > gl_context *ctx, GLenum format)
> >*/
> >   return ctx->Extensions.ANGLE_texture_compression_dxt;
> >} else {
> > - return _mesa_is_desktop_gl(ctx)
> > -&& ctx->Extensions.EXT_texture_sRGB
> > -&& ctx->Extensions.EXT_texture_compression_s3tc;
> > + return _mesa_has_EXT_texture_sRGB(ctx) &&
> > +_mesa_has_S3_s3tc(ctx);
> >
> > This looks like it should be _mesa_has_EXT_texture_compression_s3tc.
>
> I haven't looked at the code to verify, but I think
> _mesa_has_EXT_texture_compression_s3tc can be true in OpenGL ES, but
> _mesa_has_S3_s3tc cannot.  The original code has _mesa_is_desktop_gl, so
> we don't want to allow ES.  But I also thought _mesa_has_S3_s3tc wasn't
> allowed in core profile, so that might not be right either?
>

They are all allowed by core & compat. EXT_texture_sRGB is only available
in desktop GL, so any s3tc extension should work, but EXT_texture_sRGB
defines interactions with EXT_texture_compression_s3tc, not S3_s3tc.

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

Re: [Mesa-dev] [PATCH v2 03/29] mesa/main: clean up S3_s3tc check

[Ian Romanick]

On 11/30/2018 02:17 PM, Marek Olšák wrote:
> 
> 
> On Fri, Nov 23, 2018 at 5:54 AM Erik Faye-Lund
> mailto:erik.faye-l...@collabora.com>> wrote:
> 
> S3_s3tc is the extension that enables this functionality on desktop, so
> let's check for that one. The _mesa_has_S3_s3tc() helper already
> verifies the API according to the extension-table.
> 
> Signed-off-by: Erik Faye-Lund  >
> ---
>  src/mesa/main/glformats.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index 9a507d11b96..b2c18aa6d94 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -1352,8 +1352,7 @@ _mesa_is_compressed_format(const struct
> gl_context *ctx, GLenum format)
>     case GL_RGB4_S3TC:
>     case GL_RGBA_S3TC:
>     case GL_RGBA4_S3TC:
> -      return _mesa_is_desktop_gl(ctx) &&
> -         ctx->Extensions.ANGLE_texture_compression_dxt;
> +      return _mesa_has_S3_s3tc(ctx);
>     case GL_COMPRESSED_LUMINANCE_ALPHA_3DC_ATI:
>        return ctx->API == API_OPENGL_COMPAT
>           && ctx->Extensions.ATI_texture_compression_3dc;
> @@ -1378,9 +1377,8 @@ _mesa_is_compressed_format(const struct
> gl_context *ctx, GLenum format)
>            */
>           return ctx->Extensions.ANGLE_texture_compression_dxt;
>        } else {
> -         return _mesa_is_desktop_gl(ctx)
> -            && ctx->Extensions.EXT_texture_sRGB
> -            && ctx->Extensions.EXT_texture_compression_s3tc;
> +         return _mesa_has_EXT_texture_sRGB(ctx) &&
> +            _mesa_has_S3_s3tc(ctx);
> 
> This looks like it should be _mesa_has_EXT_texture_compression_s3tc.

I haven't looked at the code to verify, but I think
_mesa_has_EXT_texture_compression_s3tc can be true in OpenGL ES, but
_mesa_has_S3_s3tc cannot.  The original code has _mesa_is_desktop_gl, so
we don't want to allow ES.  But I also thought _mesa_has_S3_s3tc wasn't
allowed in core profile, so that might not be right either?

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

Re: [Mesa-dev] Representing explicit memory layouts in NIR

[Jason Ekstrand]

On Fri, Nov 30, 2018 at 4:39 PM Timothy Arceri 
wrote:

> On 1/12/18 9:11 am, Jason Ekstrand wrote:
> > All,
> >
> > This week, I've been working on trying to move UBO and SSBO access in
> > NIR over to deref instructions.  I'm hoping that this will allow us to
> > start doing alias analysis and copy-propagation on it.  The passes we
> > have in NIR *should* be able to work with SSBOs as long as
> > nir_compare_derefs does the right thing.
> >
> > # A story about derefs
> >
> > In that effort, I've run into a bit of a snag with how to represent the
> > layout information.  What we get in from SPIR-V for Vulkan is a byte
> > offset for every struct member and a byte stride for every array (and
> > pointer in the OpPtrAccessChain case).  For matrices, there is an
> > additional RowMajor boolean we need to track somewhere.  With OpenCL
> > memory access, you don't get these decorations but it's trivial to
> > translate the OpenCL layout (It's the same as C) to offset/stride when
> > creating the type.  I've come up with three different ways to represent
> > the information and they all have their own downsides:
> >
> > ## 1. Put the information on the glsl_type similar to how it's done in
> > SPIR-V
> >
> > This has the advantage of being fairly non-invasive to glsl_type.  A lot
> > of the fields we need are already there and the only real change is to
> > allow array types to have strides.  The downside is that the information
> > is often not where you want.  Arrays and structs are ok but, for
> > matrices, you have to go fishing all the way back to the struct type to
> > get the RowMajor and MatrixStride decorations.  (Thanks, SPIR-V...)
> > While this seems like a local annoyance, it actually destroys basically
> > all the advantages of having the information on the type and makes
> > lower_io a real pain.
> >
> > ## 2. Put the information on the type but do it properly
> >
> > In this version, we would put the matrix stride and RowMajor decoration
> > directly on the matrix type.  One obvious advantage here is that it
> > means no fishing for matrix type information.  Another is that, by
> > having the types specialized like this, the only way to change layouts
> > mid-deref-chain would be to have a cast.  Option 1 doesn't provide this
> > because matrix types are the same regardless of whether or not they're
> > declared RowMajor in the struct.  The downside to this option is that it
> > requires glsl_type surgery to make it work.  More on that in a bit.
> >
> > ## 3. Put the information directly on the deref
> >
> > Instead of putting the stride/offset information on the type, we just
> > put it on the deref as we build the deref chain.  This is easy enough to
> > do in spirv_to_nir and someone could also do it easily enough as a
> > lowering pass based on a type_size function.  This has the advantage of
> > simplicity because you don't have to modify glsl_type at all and
> > lowering is stupid-easy because all the information you need is right
> > there on the deref.  The downside, however, is that you alias analysis
> > is potentially harder because you don't have the nice guarantee that you
> > don't see a layout change without a type cast.  The other downside is
> > that we can't ever use copy_deref with anything bigger than a vector
> > because you don't know the sizes of any types and, unless spirv_to_nir
> > puts the offset/stride information on the deref, there's now way to
> > reconstruct it.
> >
> > I've prototyped both 1 and 3 so far and I definitely like 3 better than
> > 1 but it's not great.  I haven't prototyped 2 yet due to the issue
> > mentioned with glsl_type.
> >
> > Between 2 and 3, I really don't know how much we actually loose in terms
> > of our ability to do alias analysis.  I've written the alias analysis
> > for 3 and it isn't too bad.  I'm also not sure how much we would
> > actually loose from not being able to express whole-array or
> > whole-struct copies.  However, without a good reason otherwise, option 2
> > really seems like it's the best of all worlds
> >
> > # glsl_type surgery
> >
> > You want a good reason, eh?  You should have known this was coming...
> >
> > The problem with option 2 above is that it requires significant
> > glsl_type surgery to do it.  Putting decorations on matrices violates
> > one of the core principals of glsl_type, namely that all fundamental
> > types: scalars, vectors, matrices, images, and samplers are singletons.
> > Other types such as structs and arrays we build on-the-fly and cache
> > as-needed.  In order to do what we need for option 2 above, you have to
> > at least drop this for matrices and possibly vectors (the columns of a
> > row-major mat4 are vectors with a stride of 16).  Again, I see two
> options:
> >
> > ## A. Major rework of the guts of glsl_type
> >
> > Basically, get rid of the static singletons and just use the build
> > on-the-fly and cache model for everything.  This would mean that mat4 ==
> > mat4 is no 

Re: [Mesa-dev] [PATCH] android: amd/addrlib: update Mesa's copy of addrlib

[Haehnle, Nicolai]

We really need to reduce the number of build systems in Mesa :/

Reviewed-by: Nicolai Hähnle 

On 30.11.18 23:48, Mauro Rossi wrote:
> Needed to fix build error in addrlib in mesa for Android
> 
> Fixes: 776b911 ("amd/addrlib: update Mesa's copy of addrlib")
> Signed-off-by: Mauro Rossi 
> ---
>   src/amd/Android.addrlib.mk | 11 +--
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/src/amd/Android.addrlib.mk b/src/amd/Android.addrlib.mk
> index a37112d715..428fe19b20 100644
> --- a/src/amd/Android.addrlib.mk
> +++ b/src/amd/Android.addrlib.mk
> @@ -33,12 +33,11 @@ LOCAL_SRC_FILES := $(ADDRLIB_FILES)
>   LOCAL_C_INCLUDES := \
>   $(MESA_TOP)/src \
>   $(MESA_TOP)/src/amd/common \
> - $(MESA_TOP)/src/amd/addrlib \
> - $(MESA_TOP)/src/amd/addrlib/core \
> - $(MESA_TOP)/src/amd/addrlib/inc/chip/gfx9 \
> - $(MESA_TOP)/src/amd/addrlib/inc/chip/r800 \
> - $(MESA_TOP)/src/amd/addrlib/gfx9/chip \
> - $(MESA_TOP)/src/amd/addrlib/r800/chip
> + $(MESA_TOP)/src/amd/addrlib/inc \
> + $(MESA_TOP)/src/amd/addrlib/src \
> + $(MESA_TOP)/src/amd/addrlib/src/core \
> + $(MESA_TOP)/src/amd/addrlib/src/chip/gfx9 \
> + $(MESA_TOP)/src/amd/addrlib/src/chip/r800
>   
>   LOCAL_EXPORT_C_INCLUDE_DIRS := \
>   $(LOCAL_PATH) \
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Ian Romanick]

On 11/30/2018 02:36 PM, Jason Ekstrand wrote:
> On Fri, Nov 30, 2018 at 4:34 PM Ian Romanick  > wrote:
> 
> On 11/30/2018 01:29 PM, Jason Ekstrand wrote:
> > On Fri, Nov 30, 2018 at 3:18 PM Ian Romanick  
> > >> wrote:
> >
> >     On 11/29/2018 07:47 AM, Connor Abbott wrote:
> >     > On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand
> >     mailto:ja...@jlekstrand.net>
> >> wrote:
> >     >>
> >     >> Can you provide some context for this?  Those rules are already
> >     flagged "inexact" (that's what the ~ means) so they won't apply to
> >     anything that's "precise" or "invariant".
> >     >
> >     > I think the concern is that this isn't allowed in SPIR-V,
> even without
> >     > exact or invariant. We even go out of our way to do the
> correct thing
> >     > in the frontend by inserting an "&& a == a" or "|| a != a",
> but then
> >
> >     If you're that paranoid about it, why not just mark the
> operations are
> >     precise?  That's literally why it exists.
> >
> >     > opt_algebraic removes it with another rule and then this
> rule can flip
> >     > it from ordered to unordered. The spec says that operations
> don't have
> >     > to produce NaN, but it doesn't say anything on comparisons
> other than
> >     > the generic "everything must follow IEEE rules" and an entry
> in the
> >     > table that says "produces correct results." Then again, I
> can't find
> >     > anything in GLSL allowing these transforms either, so maybe
> we just
> >     > need to get rid of them.
> >
> >     What I hear you saying is, "The behavior isn't defined." 
> Unless you can
> >     point to a CTS test or an application that has incorrect
> behavior, I'm
> >     going to oppose removing this pretty strongly.  *Every* GLSL
> compiler
> >     does this.
> >
> >
> > The test case came from VKD3D which does D3D12 on Vulkan.  Someone
> > (Samuel, maybe?) was going to ask around and see if we can figure out
> > what D3D12's rules are.  It's possible that it requires IEEE or
> > something close.  If that's the case, as I said to Samuel on IRC,
> we're
> > probably looking at an extension.  I don't think we want a flag like
> > this that's set per-API.
> 
> Why isn't it sufficient to mark the operation as precise?  Was that on
> IRC, and I missed it?
> 
> It is also possible that we could improve the handling of 'if
> (!condition)' in the backend to make these transformations less
> important.  I have some patches for that somewhere.  They didn't really
> help with these transformations in place, and I never measured the
> result without these transformations.
> 
> I think we can and that would be better than this flag.

Ok.  I'll resurrect my patches next week.

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

[Mesa-dev] [PATCH] android: amd/addrlib: update Mesa's copy of addrlib

[Mauro Rossi]

Needed to fix build error in addrlib in mesa for Android

Fixes: 776b911 ("amd/addrlib: update Mesa's copy of addrlib")
Signed-off-by: Mauro Rossi 
---
 src/amd/Android.addrlib.mk | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/amd/Android.addrlib.mk b/src/amd/Android.addrlib.mk
index a37112d715..428fe19b20 100644
--- a/src/amd/Android.addrlib.mk
+++ b/src/amd/Android.addrlib.mk
@@ -33,12 +33,11 @@ LOCAL_SRC_FILES := $(ADDRLIB_FILES)
 LOCAL_C_INCLUDES := \
$(MESA_TOP)/src \
$(MESA_TOP)/src/amd/common \
-   $(MESA_TOP)/src/amd/addrlib \
-   $(MESA_TOP)/src/amd/addrlib/core \
-   $(MESA_TOP)/src/amd/addrlib/inc/chip/gfx9 \
-   $(MESA_TOP)/src/amd/addrlib/inc/chip/r800 \
-   $(MESA_TOP)/src/amd/addrlib/gfx9/chip \
-   $(MESA_TOP)/src/amd/addrlib/r800/chip
+   $(MESA_TOP)/src/amd/addrlib/inc \
+   $(MESA_TOP)/src/amd/addrlib/src \
+   $(MESA_TOP)/src/amd/addrlib/src/core \
+   $(MESA_TOP)/src/amd/addrlib/src/chip/gfx9 \
+   $(MESA_TOP)/src/amd/addrlib/src/chip/r800
 
 LOCAL_EXPORT_C_INCLUDE_DIRS := \
$(LOCAL_PATH) \
-- 
2.19.1

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

Re: [Mesa-dev] [PATCH 4/4] nir: detect more induction variables

[Ian Romanick]

On 11/29/2018 10:38 PM, Thomas Helland wrote:
> Den ons. 28. nov. 2018 kl. 10:23 skrev Timothy Arceri :
>>
>> On 28/11/18 6:52 pm, Thomas Helland wrote:
>>> Den ons. 28. nov. 2018 kl. 04:26 skrev Timothy Arceri 
>>> :

 This adds allows loop analysis to detect inductions varibales that
 are incremented in both branches of an if rather than in a main
 loop block. For example:

 loop {
block block_1:
/* preds: block_0 block_7 */
vec1 32 ssa_8 = phi block_0: ssa_4, block_7: ssa_20
vec1 32 ssa_9 = phi block_0: ssa_0, block_7: ssa_4
vec1 32 ssa_10 = phi block_0: ssa_1, block_7: ssa_4
vec1 32 ssa_11 = phi block_0: ssa_2, block_7: ssa_21
vec1 32 ssa_12 = phi block_0: ssa_3, block_7: ssa_22
vec4 32 ssa_13 = vec4 ssa_12, ssa_11, ssa_10, ssa_9
vec1 32 ssa_14 = ige ssa_8, ssa_5
/* succs: block_2 block_3 */
if ssa_14 {
   block block_2:
   /* preds: block_1 */
   break
   /* succs: block_8 */
} else {
   block block_3:
   /* preds: block_1 */
   /* succs: block_4 */
}
block block_4:
/* preds: block_3 */
vec1 32 ssa_15 = ilt ssa_6, ssa_8
/* succs: block_5 block_6 */
if ssa_15 {
   block block_5:
   /* preds: block_4 */
   vec1 32 ssa_16 = iadd ssa_8, ssa_7
   vec1 32 ssa_17 = load_const (0x3f80 /* 1.00*/)
   /* succs: block_7 */
} else {
   block block_6:
   /* preds: block_4 */
   vec1 32 ssa_18 = iadd ssa_8, ssa_7
   vec1 32 ssa_19 = load_const (0x3f80 /* 1.00*/)
   /* succs: block_7 */
}
block block_7:
/* preds: block_5 block_6 */
vec1 32 ssa_20 = phi block_5: ssa_16, block_6: ssa_18
vec1 32 ssa_21 = phi block_5: ssa_17, block_6: ssa_4
vec1 32 ssa_22 = phi block_5: ssa_4, block_6: ssa_19
/* succs: block_1 */
 }

 Unfortunatly GCM could move the addition out of the if for us
 (making this patch unrequired) but we still cannot enable the GCM
 pass without regressions.

>>>
>>> Just some questions / suggestions from my side for now.
>>> I'll try to take a closer look at the patch later today.
>>>
>>> While GCM would be nice, to me it seems that adding an
>>> if-opt instead, that pulls common code from both branches
>>> of an if out of the if on a more general basis, would get us
>>> this, plus a bunch of other benefits? As far as I can see there
>>> should never be negative impacts from pulling common code
>>> out like that, but I might be wrong. Did you look into that?
>>> I bet out did, I'm just interested in how that worked out.
>>
>> I didn't attempt this because pulling code out of the ifs can increase
>> register pressure. This is one of the problems we have with the GCM pass
>> currently, so for now I chose a more conservative approach.
>>
> 
> Yeah, of course. I'm being dumb. It looks better in source code,
> but as long as it does not lead to other optimizations it will only
> cause the live range of the add to intersect with that of the branch
> condition. The same amount of instructions will be executed
> either way.

Not necessarily.  If the condition is non-uniform, both branches can be
executed.

I don't think it would be too hard to do something simple that wouldn't
increase register pressure, but it should be a follow-up series.  I'm
just thinking off the top of my head...  Basically, if an operation is
the last use of at least one value, move it earlier if that will reduce
the range without increasing the range of the other operands.  In the
example above, moving the common iadd to right after the generation of
ssa_15 would reduce the live range of ssa_8.  Maybe?

>>> Since GCM is not yet where we want it to be, maybe we'd
>>> want to implement LICM? That obviously does not come
>>> into play with what this patch adresses, but it might help
>>> get a more accurate estimate of the cost/benefit of unrolling?
>>> (Invariant computations that will be CSE'd will not be
>>> counted multiple times). This might already be accounted
>>> for by counting the invariant computations only once?
>>
>> No we don't do anything like this currently. The GCM pass can pull
>> things out of loops also, but again we hit register pressure issues with
>> that pass.
>>
>> As far as I'm aware reducing invariants is not where we get most of our
>> wins from with unrolling. Removing indirect array access, improving
>> opportunities for constant folding (and a bunch of other passes), being
>> able to evaluate the unfolded loop with the surrounding code etc all
>> result in greater benefits.
>>
>> With the limits we place on making sure we don't unroll 

Re: [Mesa-dev] Representing explicit memory layouts in NIR

[Timothy Arceri]

On 1/12/18 9:11 am, Jason Ekstrand wrote:

All,

This week, I've been working on trying to move UBO and SSBO access in 
NIR over to deref instructions.  I'm hoping that this will allow us to 
start doing alias analysis and copy-propagation on it.  The passes we 
have in NIR *should* be able to work with SSBOs as long as 
nir_compare_derefs does the right thing.


# A story about derefs

In that effort, I've run into a bit of a snag with how to represent the 
layout information.  What we get in from SPIR-V for Vulkan is a byte 
offset for every struct member and a byte stride for every array (and 
pointer in the OpPtrAccessChain case).  For matrices, there is an 
additional RowMajor boolean we need to track somewhere.  With OpenCL 
memory access, you don't get these decorations but it's trivial to 
translate the OpenCL layout (It's the same as C) to offset/stride when 
creating the type.  I've come up with three different ways to represent 
the information and they all have their own downsides:


## 1. Put the information on the glsl_type similar to how it's done in 
SPIR-V


This has the advantage of being fairly non-invasive to glsl_type.  A lot 
of the fields we need are already there and the only real change is to 
allow array types to have strides.  The downside is that the information 
is often not where you want.  Arrays and structs are ok but, for 
matrices, you have to go fishing all the way back to the struct type to 
get the RowMajor and MatrixStride decorations.  (Thanks, SPIR-V...)  
While this seems like a local annoyance, it actually destroys basically 
all the advantages of having the information on the type and makes 
lower_io a real pain.


## 2. Put the information on the type but do it properly

In this version, we would put the matrix stride and RowMajor decoration 
directly on the matrix type.  One obvious advantage here is that it 
means no fishing for matrix type information.  Another is that, by 
having the types specialized like this, the only way to change layouts 
mid-deref-chain would be to have a cast.  Option 1 doesn't provide this 
because matrix types are the same regardless of whether or not they're 
declared RowMajor in the struct.  The downside to this option is that it 
requires glsl_type surgery to make it work.  More on that in a bit.


## 3. Put the information directly on the deref

Instead of putting the stride/offset information on the type, we just 
put it on the deref as we build the deref chain.  This is easy enough to 
do in spirv_to_nir and someone could also do it easily enough as a 
lowering pass based on a type_size function.  This has the advantage of 
simplicity because you don't have to modify glsl_type at all and 
lowering is stupid-easy because all the information you need is right 
there on the deref.  The downside, however, is that you alias analysis 
is potentially harder because you don't have the nice guarantee that you 
don't see a layout change without a type cast.  The other downside is 
that we can't ever use copy_deref with anything bigger than a vector 
because you don't know the sizes of any types and, unless spirv_to_nir 
puts the offset/stride information on the deref, there's now way to 
reconstruct it.


I've prototyped both 1 and 3 so far and I definitely like 3 better than 
1 but it's not great.  I haven't prototyped 2 yet due to the issue 
mentioned with glsl_type.


Between 2 and 3, I really don't know how much we actually loose in terms 
of our ability to do alias analysis.  I've written the alias analysis 
for 3 and it isn't too bad.  I'm also not sure how much we would 
actually loose from not being able to express whole-array or 
whole-struct copies.  However, without a good reason otherwise, option 2 
really seems like it's the best of all worlds


# glsl_type surgery

You want a good reason, eh?  You should have known this was coming...

The problem with option 2 above is that it requires significant 
glsl_type surgery to do it.  Putting decorations on matrices violates 
one of the core principals of glsl_type, namely that all fundamental 
types: scalars, vectors, matrices, images, and samplers are singletons.  
Other types such as structs and arrays we build on-the-fly and cache 
as-needed.  In order to do what we need for option 2 above, you have to 
at least drop this for matrices and possibly vectors (the columns of a 
row-major mat4 are vectors with a stride of 16).  Again, I see two options:


## A. Major rework of the guts of glsl_type

Basically, get rid of the static singletons and just use the build 
on-the-fly and cache model for everything.  This would mean that mat4 == 
mat4 is no longer guaranteed unless you know a priori that none of your 
types are decorated with layout information.  It would also be, not only 
a pile of work, but a single mega-patch.  I don't know of any way to 
make that change without just ripping it all up and putting it back 
together.


Do we really need to throw away the singleton 

Re: [Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Ian Romanick]

On 11/30/2018 01:36 PM, Bas Nieuwenhuizen wrote:
> On Fri, Nov 30, 2018 at 10:29 PM Jason Ekstrand  wrote:
>>
>> On Fri, Nov 30, 2018 at 3:18 PM Ian Romanick  wrote:
>>>
>>> On 11/29/2018 07:47 AM, Connor Abbott wrote:
 On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand  
 wrote:
>
> Can you provide some context for this?  Those rules are already flagged 
> "inexact" (that's what the ~ means) so they won't apply to anything 
> that's "precise" or "invariant".

 I think the concern is that this isn't allowed in SPIR-V, even without
 exact or invariant. We even go out of our way to do the correct thing
 in the frontend by inserting an "&& a == a" or "|| a != a", but then
>>>
>>> If you're that paranoid about it, why not just mark the operations are
>>> precise?  That's literally why it exists.
>>>
 opt_algebraic removes it with another rule and then this rule can flip
 it from ordered to unordered. The spec says that operations don't have
 to produce NaN, but it doesn't say anything on comparisons other than
 the generic "everything must follow IEEE rules" and an entry in the
 table that says "produces correct results." Then again, I can't find
 anything in GLSL allowing these transforms either, so maybe we just
 need to get rid of them.
>>>
>>> What I hear you saying is, "The behavior isn't defined."  Unless you can
>>> point to a CTS test or an application that has incorrect behavior, I'm
>>> going to oppose removing this pretty strongly.  *Every* GLSL compiler
>>> does this.
>>
>>
>> The test case came from VKD3D which does D3D12 on Vulkan.  Someone (Samuel, 
>> maybe?) was going to ask around and see if we can figure out what D3D12's 
>> rules are.  It's possible that it requires IEEE or something close.  If 
>> that's the case, as I said to Samuel on IRC, we're probably looking at an 
>> extension.  I don't think we want a flag like this that's set per-API.
> 
> What do you mean an extension? AFAIU the concern is that Vulkan SPIR-V
> is more restrictive than GLSL here, and disallows these optimization
> right? That makes a strong case that we should remove these rules for
> at least Vulkan. If that means writing a CTS test, maybe we should do
> just that?

Given the existence of mobile GPUs, it seems unlikely to me that Vulkan
SPIR-V is more strict about NaNs or denorms than GLSL or GLSL ES.  The
mobile vendors didn't have the extra DX requirements, so, for a variety
of valid reasons, they pushed back on making much of anything more strict.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Jason Ekstrand]

On Fri, Nov 30, 2018 at 4:34 PM Ian Romanick  wrote:

> On 11/30/2018 01:29 PM, Jason Ekstrand wrote:
> > On Fri, Nov 30, 2018 at 3:18 PM Ian Romanick  > > wrote:
> >
> > On 11/29/2018 07:47 AM, Connor Abbott wrote:
> > > On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand
> > mailto:ja...@jlekstrand.net>> wrote:
> > >>
> > >> Can you provide some context for this?  Those rules are already
> > flagged "inexact" (that's what the ~ means) so they won't apply to
> > anything that's "precise" or "invariant".
> > >
> > > I think the concern is that this isn't allowed in SPIR-V, even
> without
> > > exact or invariant. We even go out of our way to do the correct
> thing
> > > in the frontend by inserting an "&& a == a" or "|| a != a", but
> then
> >
> > If you're that paranoid about it, why not just mark the operations
> are
> > precise?  That's literally why it exists.
> >
> > > opt_algebraic removes it with another rule and then this rule can
> flip
> > > it from ordered to unordered. The spec says that operations don't
> have
> > > to produce NaN, but it doesn't say anything on comparisons other
> than
> > > the generic "everything must follow IEEE rules" and an entry in the
> > > table that says "produces correct results." Then again, I can't
> find
> > > anything in GLSL allowing these transforms either, so maybe we just
> > > need to get rid of them.
> >
> > What I hear you saying is, "The behavior isn't defined."  Unless you
> can
> > point to a CTS test or an application that has incorrect behavior,
> I'm
> > going to oppose removing this pretty strongly.  *Every* GLSL compiler
> > does this.
> >
> >
> > The test case came from VKD3D which does D3D12 on Vulkan.  Someone
> > (Samuel, maybe?) was going to ask around and see if we can figure out
> > what D3D12's rules are.  It's possible that it requires IEEE or
> > something close.  If that's the case, as I said to Samuel on IRC, we're
> > probably looking at an extension.  I don't think we want a flag like
> > this that's set per-API.
>
> Why isn't it sufficient to mark the operation as precise?  Was that on
> IRC, and I missed it?
>
> It is also possible that we could improve the handling of 'if
> (!condition)' in the backend to make these transformations less
> important.  I have some patches for that somewhere.  They didn't really
> help with these transformations in place, and I never measured the
> result without these transformations.
>

I think we can and that would be better than this flag.

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

Re: [Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Ian Romanick]

On 11/30/2018 01:29 PM, Jason Ekstrand wrote:
> On Fri, Nov 30, 2018 at 3:18 PM Ian Romanick  > wrote:
> 
> On 11/29/2018 07:47 AM, Connor Abbott wrote:
> > On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand
> mailto:ja...@jlekstrand.net>> wrote:
> >>
> >> Can you provide some context for this?  Those rules are already
> flagged "inexact" (that's what the ~ means) so they won't apply to
> anything that's "precise" or "invariant".
> >
> > I think the concern is that this isn't allowed in SPIR-V, even without
> > exact or invariant. We even go out of our way to do the correct thing
> > in the frontend by inserting an "&& a == a" or "|| a != a", but then
> 
> If you're that paranoid about it, why not just mark the operations are
> precise?  That's literally why it exists.
> 
> > opt_algebraic removes it with another rule and then this rule can flip
> > it from ordered to unordered. The spec says that operations don't have
> > to produce NaN, but it doesn't say anything on comparisons other than
> > the generic "everything must follow IEEE rules" and an entry in the
> > table that says "produces correct results." Then again, I can't find
> > anything in GLSL allowing these transforms either, so maybe we just
> > need to get rid of them.
> 
> What I hear you saying is, "The behavior isn't defined."  Unless you can
> point to a CTS test or an application that has incorrect behavior, I'm
> going to oppose removing this pretty strongly.  *Every* GLSL compiler
> does this.
> 
> 
> The test case came from VKD3D which does D3D12 on Vulkan.  Someone
> (Samuel, maybe?) was going to ask around and see if we can figure out
> what D3D12's rules are.  It's possible that it requires IEEE or
> something close.  If that's the case, as I said to Samuel on IRC, we're
> probably looking at an extension.  I don't think we want a flag like
> this that's set per-API.

Why isn't it sufficient to mark the operation as precise?  Was that on
IRC, and I missed it?

It is also possible that we could improve the handling of 'if
(!condition)' in the backend to make these transformations less
important.  I have some patches for that somewhere.  They didn't really
help with these transformations in place, and I never measured the
result without these transformations.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 03/29] mesa/main: clean up S3_s3tc check

[Marek Olšák]

On Fri, Nov 23, 2018 at 5:54 AM Erik Faye-Lund 
wrote:

> S3_s3tc is the extension that enables this functionality on desktop, so
> let's check for that one. The _mesa_has_S3_s3tc() helper already
> verifies the API according to the extension-table.
>
> Signed-off-by: Erik Faye-Lund 
> ---
>  src/mesa/main/glformats.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index 9a507d11b96..b2c18aa6d94 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -1352,8 +1352,7 @@ _mesa_is_compressed_format(const struct gl_context
> *ctx, GLenum format)
> case GL_RGB4_S3TC:
> case GL_RGBA_S3TC:
> case GL_RGBA4_S3TC:
> -  return _mesa_is_desktop_gl(ctx) &&
> - ctx->Extensions.ANGLE_texture_compression_dxt;
> +  return _mesa_has_S3_s3tc(ctx);
> case GL_COMPRESSED_LUMINANCE_ALPHA_3DC_ATI:
>return ctx->API == API_OPENGL_COMPAT
>   && ctx->Extensions.ATI_texture_compression_3dc;
> @@ -1378,9 +1377,8 @@ _mesa_is_compressed_format(const struct gl_context
> *ctx, GLenum format)
>*/
>   return ctx->Extensions.ANGLE_texture_compression_dxt;
>} else {
> - return _mesa_is_desktop_gl(ctx)
> -&& ctx->Extensions.EXT_texture_sRGB
> -&& ctx->Extensions.EXT_texture_compression_s3tc;
> + return _mesa_has_EXT_texture_sRGB(ctx) &&
> +_mesa_has_S3_s3tc(ctx);
>

This looks like it should be _mesa_has_EXT_texture_compression_s3tc.

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

Re: [Mesa-dev] [PATCH v2 2/5] gallium: Add new PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE

[Rob Clark]

I guess as long as it is just a single draw, it works out to the same
thing.  Maybe to expose the GCN functionality we could do something
like

  PIPE_CAP_SURFACE_SAMPLE_COUNT:
0 - unsupported
1 - msaa per draw
2 - msaa per renderpass

??

BR,
-R

On Fri, Nov 30, 2018 at 3:41 PM Marek Olšák  wrote:
>
> GCN calls it overrasterization multisampling, but it's really only useful for 
> polygon smoothing, because there is no temporary buffer.
>
> Marek
>
> On Fri, Nov 30, 2018 at 3:35 PM Rob Clark  wrote:
>>
>> On Fri, Nov 30, 2018 at 3:25 PM Marek Olšák  wrote:
>> >
>> > Suggestions:
>> > - PIPE_CAP_TILED_BASED_MSAA_OVERSAMPLING
>> > - pipe_surface::tile_based_oversample_count
>> >
>> > I'm assuming this feature isn't possible without tile-based rendering.
>>
>> I guess technically an IMR could do it, with an internal temporary
>> buffer not visible to the state-tracker.  (Not sure I could come up
>> with a reason *why* you'd want to do that, but...)
>>
>> BR,
>> -R
>>
>> >
>> > Marek
>> >
>> > On Fri, Nov 30, 2018 at 1:23 PM Kristian Høgsberg  
>> > wrote:
>> >>
>> >> On Fri, Nov 30, 2018 at 10:17 AM Marek Olšák  wrote:
>> >> >
>> >> > On Fri, Nov 30, 2018 at 1:13 PM Kristian Høgsberg  
>> >> > wrote:
>> >> >>
>> >> >> On Fri, Nov 16, 2018 at 7:48 PM Marek Olšák  wrote:
>> >> >> >
>> >> >> > I think the name PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE is slightly 
>> >> >> > misleading, because it doesn't imply anything about the OpenGL ES 
>> >> >> > behavior, which is that a texture is multisampled in the cache, but 
>> >> >> > single-sampled in memory. This should be mentioned somewhere.
>> >> >>
>> >> >> Not sure exactly which change you're recommending, but in retrospect,
>> >> >> I think it would be better to name the cap in term of how it changes
>> >> >> the gallium API instead of the GLES extension it enables. How about
>> >> >> PIPE_CAP_SURFACE_SAMPLE_COUNT, to indicate that it allows overriding
>> >> >> sample counts in pipe_surface?
>> >> >
>> >> >
>> >> > That's an interesting idea, but how does it convey that multisampled 
>> >> > surfaces are never multisampled in memory?
>> >>
>> >> How are you going to convey all that in a cap enum name? In general,
>> >> the cap names are concise hints and not super descriptive - you have
>> >> to go read the documentation to understand the semantics.
>> >>
>> >> Do you have a specific name you'd like to see or can we go with
>> >> PIPE_CAP_SURFACE_SAMPLE_COUNT?
>> >>
>> >> >
>> >> > Marek
>> >
>> > ___
>> > 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] Representing explicit memory layouts in NIR

[Jason Ekstrand]

All,

This week, I've been working on trying to move UBO and SSBO access in NIR
over to deref instructions.  I'm hoping that this will allow us to start
doing alias analysis and copy-propagation on it.  The passes we have in NIR
*should* be able to work with SSBOs as long as nir_compare_derefs does the
right thing.

# A story about derefs

In that effort, I've run into a bit of a snag with how to represent the
layout information.  What we get in from SPIR-V for Vulkan is a byte offset
for every struct member and a byte stride for every array (and pointer in
the OpPtrAccessChain case).  For matrices, there is an additional RowMajor
boolean we need to track somewhere.  With OpenCL memory access, you don't
get these decorations but it's trivial to translate the OpenCL layout (It's
the same as C) to offset/stride when creating the type.  I've come up with
three different ways to represent the information and they all have their
own downsides:

## 1. Put the information on the glsl_type similar to how it's done in
SPIR-V

This has the advantage of being fairly non-invasive to glsl_type.  A lot of
the fields we need are already there and the only real change is to allow
array types to have strides.  The downside is that the information is often
not where you want.  Arrays and structs are ok but, for matrices, you have
to go fishing all the way back to the struct type to get the RowMajor and
MatrixStride decorations.  (Thanks, SPIR-V...)  While this seems like a
local annoyance, it actually destroys basically all the advantages of
having the information on the type and makes lower_io a real pain.

## 2. Put the information on the type but do it properly

In this version, we would put the matrix stride and RowMajor decoration
directly on the matrix type.  One obvious advantage here is that it means
no fishing for matrix type information.  Another is that, by having the
types specialized like this, the only way to change layouts mid-deref-chain
would be to have a cast.  Option 1 doesn't provide this because matrix
types are the same regardless of whether or not they're declared RowMajor
in the struct.  The downside to this option is that it requires glsl_type
surgery to make it work.  More on that in a bit.

## 3. Put the information directly on the deref

Instead of putting the stride/offset information on the type, we just put
it on the deref as we build the deref chain.  This is easy enough to do in
spirv_to_nir and someone could also do it easily enough as a lowering pass
based on a type_size function.  This has the advantage of simplicity
because you don't have to modify glsl_type at all and lowering is
stupid-easy because all the information you need is right there on the
deref.  The downside, however, is that you alias analysis is potentially
harder because you don't have the nice guarantee that you don't see a
layout change without a type cast.  The other downside is that we can't
ever use copy_deref with anything bigger than a vector because you don't
know the sizes of any types and, unless spirv_to_nir puts the offset/stride
information on the deref, there's now way to reconstruct it.

I've prototyped both 1 and 3 so far and I definitely like 3 better than 1
but it's not great.  I haven't prototyped 2 yet due to the issue mentioned
with glsl_type.

Between 2 and 3, I really don't know how much we actually loose in terms of
our ability to do alias analysis.  I've written the alias analysis for 3
and it isn't too bad.  I'm also not sure how much we would actually loose
from not being able to express whole-array or whole-struct copies.
However, without a good reason otherwise, option 2 really seems like it's
the best of all worlds

# glsl_type surgery

You want a good reason, eh?  You should have known this was coming...

The problem with option 2 above is that it requires significant glsl_type
surgery to do it.  Putting decorations on matrices violates one of the core
principals of glsl_type, namely that all fundamental types: scalars,
vectors, matrices, images, and samplers are singletons.  Other types such
as structs and arrays we build on-the-fly and cache as-needed.  In order to
do what we need for option 2 above, you have to at least drop this for
matrices and possibly vectors (the columns of a row-major mat4 are vectors
with a stride of 16).  Again, I see two options:

## A. Major rework of the guts of glsl_type

Basically, get rid of the static singletons and just use the build
on-the-fly and cache model for everything.  This would mean that mat4 ==
mat4 is no longer guaranteed unless you know a priori that none of your
types are decorated with layout information.  It would also be, not only a
pile of work, but a single mega-patch.  I don't know of any way to make
that change without just ripping it all up and putting it back together.

## B. Make a new nir_type and make NIR use it

This seems a bit crazy at this point.  src/compiler/nir itself has over 200
references to glsl_type and that 

Re: [Mesa-dev] [PATCH 4/4] nir: detect more induction variables

[Timothy Arceri]

On 30/11/18 5:38 pm, Thomas Helland wrote:

Den ons. 28. nov. 2018 kl. 10:23 skrev Timothy Arceri :


On 28/11/18 6:52 pm, Thomas Helland wrote:

Den ons. 28. nov. 2018 kl. 04:26 skrev Timothy Arceri :


This adds allows loop analysis to detect inductions varibales that
are incremented in both branches of an if rather than in a main
loop block. For example:

 loop {
block block_1:
/* preds: block_0 block_7 */
vec1 32 ssa_8 = phi block_0: ssa_4, block_7: ssa_20
vec1 32 ssa_9 = phi block_0: ssa_0, block_7: ssa_4
vec1 32 ssa_10 = phi block_0: ssa_1, block_7: ssa_4
vec1 32 ssa_11 = phi block_0: ssa_2, block_7: ssa_21
vec1 32 ssa_12 = phi block_0: ssa_3, block_7: ssa_22
vec4 32 ssa_13 = vec4 ssa_12, ssa_11, ssa_10, ssa_9
vec1 32 ssa_14 = ige ssa_8, ssa_5
/* succs: block_2 block_3 */
if ssa_14 {
   block block_2:
   /* preds: block_1 */
   break
   /* succs: block_8 */
} else {
   block block_3:
   /* preds: block_1 */
   /* succs: block_4 */
}
block block_4:
/* preds: block_3 */
vec1 32 ssa_15 = ilt ssa_6, ssa_8
/* succs: block_5 block_6 */
if ssa_15 {
   block block_5:
   /* preds: block_4 */
   vec1 32 ssa_16 = iadd ssa_8, ssa_7
   vec1 32 ssa_17 = load_const (0x3f80 /* 1.00*/)
   /* succs: block_7 */
} else {
   block block_6:
   /* preds: block_4 */
   vec1 32 ssa_18 = iadd ssa_8, ssa_7
   vec1 32 ssa_19 = load_const (0x3f80 /* 1.00*/)
   /* succs: block_7 */
}
block block_7:
/* preds: block_5 block_6 */
vec1 32 ssa_20 = phi block_5: ssa_16, block_6: ssa_18
vec1 32 ssa_21 = phi block_5: ssa_17, block_6: ssa_4
vec1 32 ssa_22 = phi block_5: ssa_4, block_6: ssa_19
/* succs: block_1 */
 }

Unfortunatly GCM could move the addition out of the if for us
(making this patch unrequired) but we still cannot enable the GCM
pass without regressions.



Just some questions / suggestions from my side for now.
I'll try to take a closer look at the patch later today.

While GCM would be nice, to me it seems that adding an
if-opt instead, that pulls common code from both branches
of an if out of the if on a more general basis, would get us
this, plus a bunch of other benefits? As far as I can see there
should never be negative impacts from pulling common code
out like that, but I might be wrong. Did you look into that?
I bet out did, I'm just interested in how that worked out.


I didn't attempt this because pulling code out of the ifs can increase
register pressure. This is one of the problems we have with the GCM pass
currently, so for now I chose a more conservative approach.



Yeah, of course. I'm being dumb. It looks better in source code,
but as long as it does not lead to other optimizations it will only
cause the live range of the add to intersect with that of the branch
condition. The same amount of instructions will be executed
either way.



Since GCM is not yet where we want it to be, maybe we'd
want to implement LICM? That obviously does not come
into play with what this patch adresses, but it might help
get a more accurate estimate of the cost/benefit of unrolling?
(Invariant computations that will be CSE'd will not be
counted multiple times). This might already be accounted
for by counting the invariant computations only once?


No we don't do anything like this currently. The GCM pass can pull
things out of loops also, but again we hit register pressure issues with
that pass.

As far as I'm aware reducing invariants is not where we get most of our
wins from with unrolling. Removing indirect array access, improving
opportunities for constant folding (and a bunch of other passes), being
able to evaluate the unfolded loop with the surrounding code etc all
result in greater benefits.

With the limits we place on making sure we don't unroll large loops that
are going to cause register use issues, nobody has yet been able to show
that always unrolling loops is causing any harm, and it's certainly been
shown to help :)


Thanks for taking the time with my stupidity =) I'll try to take a look at
these patches later tonight =)


All valid questions :) Thanks for taking a look.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Jason Ekstrand]

On Fri, Nov 30, 2018 at 3:37 PM Bas Nieuwenhuizen 
wrote:

> On Fri, Nov 30, 2018 at 10:29 PM Jason Ekstrand 
> wrote:
> >
> > On Fri, Nov 30, 2018 at 3:18 PM Ian Romanick 
> wrote:
> >>
> >> On 11/29/2018 07:47 AM, Connor Abbott wrote:
> >> > On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand 
> wrote:
> >> >>
> >> >> Can you provide some context for this?  Those rules are already
> flagged "inexact" (that's what the ~ means) so they won't apply to anything
> that's "precise" or "invariant".
> >> >
> >> > I think the concern is that this isn't allowed in SPIR-V, even without
> >> > exact or invariant. We even go out of our way to do the correct thing
> >> > in the frontend by inserting an "&& a == a" or "|| a != a", but then
> >>
> >> If you're that paranoid about it, why not just mark the operations are
> >> precise?  That's literally why it exists.
> >>
> >> > opt_algebraic removes it with another rule and then this rule can flip
> >> > it from ordered to unordered. The spec says that operations don't have
> >> > to produce NaN, but it doesn't say anything on comparisons other than
> >> > the generic "everything must follow IEEE rules" and an entry in the
> >> > table that says "produces correct results." Then again, I can't find
> >> > anything in GLSL allowing these transforms either, so maybe we just
> >> > need to get rid of them.
> >>
> >> What I hear you saying is, "The behavior isn't defined."  Unless you can
> >> point to a CTS test or an application that has incorrect behavior, I'm
> >> going to oppose removing this pretty strongly.  *Every* GLSL compiler
> >> does this.
> >
> >
> > The test case came from VKD3D which does D3D12 on Vulkan.  Someone
> (Samuel, maybe?) was going to ask around and see if we can figure out what
> D3D12's rules are.  It's possible that it requires IEEE or something
> close.  If that's the case, as I said to Samuel on IRC, we're probably
> looking at an extension.  I don't think we want a flag like this that's set
> per-API.
>
> What do you mean an extension? AFAIU the concern is that Vulkan SPIR-V
> is more restrictive than GLSL here, and disallows these optimization
> right? That makes a strong case that we should remove these rules for
> at least Vulkan. If that means writing a CTS test, maybe we should do
> just that?
>

I know there were some discussions around this.  I don't remember what the
outcome was.  It's possible that it really was "Vulkan is just more
restrictive" but I don't remember.  I know there are CTS tests for these
opcodes and, last I checked, we're passing the CTS so maybe those tests
aren't mustpass?  I'd have to go digging.

--Jason


> >
> >>
> >> >> On Thu, Nov 29, 2018 at 9:18 AM Samuel Pitoiset <
> samuel.pitoi...@gmail.com> wrote:
> >> >>>
> >> >>> It's correct in GLSL because the behaviour is undefined in
> >> >>> presence of NaNs. But this seems incorrect in Vulkan.
> >> >>>
> >> >>> Signed-off-by: Samuel Pitoiset 
> >> >>> ---
> >> >>>  src/compiler/nir/nir.h| 6 ++
> >> >>>  src/compiler/nir/nir_opt_algebraic.py | 8 
> >> >>>  2 files changed, 10 insertions(+), 4 deletions(-)
> >> >>>
> >> >>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> >> >>> index db935c8496b..4107c293962 100644
> >> >>> --- a/src/compiler/nir/nir.h
> >> >>> +++ b/src/compiler/nir/nir.h
> >> >>> @@ -2188,6 +2188,12 @@ typedef struct nir_shader_compiler_options {
> >> >>> /* Set if nir_lower_wpos_ytransform() should also invert
> gl_PointCoord. */
> >> >>> bool lower_wpos_pntc;
> >> >>>
> >> >>> +   /* If false, lower ~inot(flt(a,b)) -> fge(a,b) and variants.
> >> >>> +* In presence of NaNs, this is correct in GLSL because the
> behaviour is
> >> >>> +* undefined. In Vulkan, doing these transformations is
> incorrect.
> >> >>> +*/
> >> >>> +   bool exact_float_comparisons;
> >> >>> +
> >> >>> /**
> >> >>>  * Should nir_lower_io() create load_interpolated_input
> intrinsics?
> >> >>>  *
> >> >>> diff --git a/src/compiler/nir/nir_opt_algebraic.py
> b/src/compiler/nir/nir_opt_algebraic.py
> >> >>> index f2a7be0c403..3750874407b 100644
> >> >>> --- a/src/compiler/nir/nir_opt_algebraic.py
> >> >>> +++ b/src/compiler/nir/nir_opt_algebraic.py
> >> >>> @@ -154,10 +154,10 @@ optimizations = [
> >> >>> (('ishl', ('imul', a, '#b'), '#c'), ('imul', a, ('ishl', b,
> c))),
> >> >>>
> >> >>> # Comparison simplifications
> >> >>> -   (('~inot', ('flt', a, b)), ('fge', a, b)),
> >> >>> -   (('~inot', ('fge', a, b)), ('flt', a, b)),
> >> >>> -   (('~inot', ('feq', a, b)), ('fne', a, b)),
> >> >>> -   (('~inot', ('fne', a, b)), ('feq', a, b)),
> >> >>> +   (('~inot', ('flt', a, b)), ('fge', a, b),
> '!options->exact_float_comparisons'),
> >> >>> +   (('~inot', ('fge', a, b)), ('flt', a, b),
> '!options->exact_float_comparisons'),
> >> >>> +   (('~inot', ('feq', a, b)), ('fne', a, b),
> '!options->exact_float_comparisons'),
> >> >>> +   (('~inot', ('fne', a, b)), ('feq', a, b),
> 

Re: [Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Bas Nieuwenhuizen]

On Fri, Nov 30, 2018 at 10:29 PM Jason Ekstrand  wrote:
>
> On Fri, Nov 30, 2018 at 3:18 PM Ian Romanick  wrote:
>>
>> On 11/29/2018 07:47 AM, Connor Abbott wrote:
>> > On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand  
>> > wrote:
>> >>
>> >> Can you provide some context for this?  Those rules are already flagged 
>> >> "inexact" (that's what the ~ means) so they won't apply to anything 
>> >> that's "precise" or "invariant".
>> >
>> > I think the concern is that this isn't allowed in SPIR-V, even without
>> > exact or invariant. We even go out of our way to do the correct thing
>> > in the frontend by inserting an "&& a == a" or "|| a != a", but then
>>
>> If you're that paranoid about it, why not just mark the operations are
>> precise?  That's literally why it exists.
>>
>> > opt_algebraic removes it with another rule and then this rule can flip
>> > it from ordered to unordered. The spec says that operations don't have
>> > to produce NaN, but it doesn't say anything on comparisons other than
>> > the generic "everything must follow IEEE rules" and an entry in the
>> > table that says "produces correct results." Then again, I can't find
>> > anything in GLSL allowing these transforms either, so maybe we just
>> > need to get rid of them.
>>
>> What I hear you saying is, "The behavior isn't defined."  Unless you can
>> point to a CTS test or an application that has incorrect behavior, I'm
>> going to oppose removing this pretty strongly.  *Every* GLSL compiler
>> does this.
>
>
> The test case came from VKD3D which does D3D12 on Vulkan.  Someone (Samuel, 
> maybe?) was going to ask around and see if we can figure out what D3D12's 
> rules are.  It's possible that it requires IEEE or something close.  If 
> that's the case, as I said to Samuel on IRC, we're probably looking at an 
> extension.  I don't think we want a flag like this that's set per-API.

What do you mean an extension? AFAIU the concern is that Vulkan SPIR-V
is more restrictive than GLSL here, and disallows these optimization
right? That makes a strong case that we should remove these rules for
at least Vulkan. If that means writing a CTS test, maybe we should do
just that?


>
>>
>> >> On Thu, Nov 29, 2018 at 9:18 AM Samuel Pitoiset 
>> >>  wrote:
>> >>>
>> >>> It's correct in GLSL because the behaviour is undefined in
>> >>> presence of NaNs. But this seems incorrect in Vulkan.
>> >>>
>> >>> Signed-off-by: Samuel Pitoiset 
>> >>> ---
>> >>>  src/compiler/nir/nir.h| 6 ++
>> >>>  src/compiler/nir/nir_opt_algebraic.py | 8 
>> >>>  2 files changed, 10 insertions(+), 4 deletions(-)
>> >>>
>> >>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> >>> index db935c8496b..4107c293962 100644
>> >>> --- a/src/compiler/nir/nir.h
>> >>> +++ b/src/compiler/nir/nir.h
>> >>> @@ -2188,6 +2188,12 @@ typedef struct nir_shader_compiler_options {
>> >>> /* Set if nir_lower_wpos_ytransform() should also invert 
>> >>> gl_PointCoord. */
>> >>> bool lower_wpos_pntc;
>> >>>
>> >>> +   /* If false, lower ~inot(flt(a,b)) -> fge(a,b) and variants.
>> >>> +* In presence of NaNs, this is correct in GLSL because the 
>> >>> behaviour is
>> >>> +* undefined. In Vulkan, doing these transformations is 
>> >>> incorrect.
>> >>> +*/
>> >>> +   bool exact_float_comparisons;
>> >>> +
>> >>> /**
>> >>>  * Should nir_lower_io() create load_interpolated_input intrinsics?
>> >>>  *
>> >>> diff --git a/src/compiler/nir/nir_opt_algebraic.py 
>> >>> b/src/compiler/nir/nir_opt_algebraic.py
>> >>> index f2a7be0c403..3750874407b 100644
>> >>> --- a/src/compiler/nir/nir_opt_algebraic.py
>> >>> +++ b/src/compiler/nir/nir_opt_algebraic.py
>> >>> @@ -154,10 +154,10 @@ optimizations = [
>> >>> (('ishl', ('imul', a, '#b'), '#c'), ('imul', a, ('ishl', b, c))),
>> >>>
>> >>> # Comparison simplifications
>> >>> -   (('~inot', ('flt', a, b)), ('fge', a, b)),
>> >>> -   (('~inot', ('fge', a, b)), ('flt', a, b)),
>> >>> -   (('~inot', ('feq', a, b)), ('fne', a, b)),
>> >>> -   (('~inot', ('fne', a, b)), ('feq', a, b)),
>> >>> +   (('~inot', ('flt', a, b)), ('fge', a, b), 
>> >>> '!options->exact_float_comparisons'),
>> >>> +   (('~inot', ('fge', a, b)), ('flt', a, b), 
>> >>> '!options->exact_float_comparisons'),
>> >>> +   (('~inot', ('feq', a, b)), ('fne', a, b), 
>> >>> '!options->exact_float_comparisons'),
>> >>> +   (('~inot', ('fne', a, b)), ('feq', a, b), 
>> >>> '!options->exact_float_comparisons'),
>> >>
>> >>
>> >> The feq/fne one is actually completely safe.  fne is defined to be !feq 
>> >> even when NaN is considered.
>> >>
>> >> --Jasoan
>> >>
>> >>>
>> >>> (('inot', ('ilt', a, b)), ('ige', a, b)),
>> >>> (('inot', ('ult', a, b)), ('uge', a, b)),
>> >>> (('inot', ('ige', a, b)), ('ilt', a, b)),
>> >>> --
>> >>> 2.19.2
>> >>>
>> >>> ___
>> >>> mesa-dev mailing list
>> >>> mesa-dev@lists.freedesktop.org
>> >>> 

Re: [Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Jason Ekstrand]

On Fri, Nov 30, 2018 at 3:18 PM Ian Romanick  wrote:

> On 11/29/2018 07:47 AM, Connor Abbott wrote:
> > On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand 
> wrote:
> >>
> >> Can you provide some context for this?  Those rules are already flagged
> "inexact" (that's what the ~ means) so they won't apply to anything that's
> "precise" or "invariant".
> >
> > I think the concern is that this isn't allowed in SPIR-V, even without
> > exact or invariant. We even go out of our way to do the correct thing
> > in the frontend by inserting an "&& a == a" or "|| a != a", but then
>
> If you're that paranoid about it, why not just mark the operations are
> precise?  That's literally why it exists.
>
> > opt_algebraic removes it with another rule and then this rule can flip
> > it from ordered to unordered. The spec says that operations don't have
> > to produce NaN, but it doesn't say anything on comparisons other than
> > the generic "everything must follow IEEE rules" and an entry in the
> > table that says "produces correct results." Then again, I can't find
> > anything in GLSL allowing these transforms either, so maybe we just
> > need to get rid of them.
>
> What I hear you saying is, "The behavior isn't defined."  Unless you can
> point to a CTS test or an application that has incorrect behavior, I'm
> going to oppose removing this pretty strongly.  *Every* GLSL compiler
> does this.
>

The test case came from VKD3D which does D3D12 on Vulkan.  Someone (Samuel,
maybe?) was going to ask around and see if we can figure out what D3D12's
rules are.  It's possible that it requires IEEE or something close.  If
that's the case, as I said to Samuel on IRC, we're probably looking at an
extension.  I don't think we want a flag like this that's set per-API.


> >> On Thu, Nov 29, 2018 at 9:18 AM Samuel Pitoiset <
> samuel.pitoi...@gmail.com> wrote:
> >>>
> >>> It's correct in GLSL because the behaviour is undefined in
> >>> presence of NaNs. But this seems incorrect in Vulkan.
> >>>
> >>> Signed-off-by: Samuel Pitoiset 
> >>> ---
> >>>  src/compiler/nir/nir.h| 6 ++
> >>>  src/compiler/nir/nir_opt_algebraic.py | 8 
> >>>  2 files changed, 10 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> >>> index db935c8496b..4107c293962 100644
> >>> --- a/src/compiler/nir/nir.h
> >>> +++ b/src/compiler/nir/nir.h
> >>> @@ -2188,6 +2188,12 @@ typedef struct nir_shader_compiler_options {
> >>> /* Set if nir_lower_wpos_ytransform() should also invert
> gl_PointCoord. */
> >>> bool lower_wpos_pntc;
> >>>
> >>> +   /* If false, lower ~inot(flt(a,b)) -> fge(a,b) and variants.
> >>> +* In presence of NaNs, this is correct in GLSL because the
> behaviour is
> >>> +* undefined. In Vulkan, doing these transformations is
> incorrect.
> >>> +*/
> >>> +   bool exact_float_comparisons;
> >>> +
> >>> /**
> >>>  * Should nir_lower_io() create load_interpolated_input intrinsics?
> >>>  *
> >>> diff --git a/src/compiler/nir/nir_opt_algebraic.py
> b/src/compiler/nir/nir_opt_algebraic.py
> >>> index f2a7be0c403..3750874407b 100644
> >>> --- a/src/compiler/nir/nir_opt_algebraic.py
> >>> +++ b/src/compiler/nir/nir_opt_algebraic.py
> >>> @@ -154,10 +154,10 @@ optimizations = [
> >>> (('ishl', ('imul', a, '#b'), '#c'), ('imul', a, ('ishl', b, c))),
> >>>
> >>> # Comparison simplifications
> >>> -   (('~inot', ('flt', a, b)), ('fge', a, b)),
> >>> -   (('~inot', ('fge', a, b)), ('flt', a, b)),
> >>> -   (('~inot', ('feq', a, b)), ('fne', a, b)),
> >>> -   (('~inot', ('fne', a, b)), ('feq', a, b)),
> >>> +   (('~inot', ('flt', a, b)), ('fge', a, b),
> '!options->exact_float_comparisons'),
> >>> +   (('~inot', ('fge', a, b)), ('flt', a, b),
> '!options->exact_float_comparisons'),
> >>> +   (('~inot', ('feq', a, b)), ('fne', a, b),
> '!options->exact_float_comparisons'),
> >>> +   (('~inot', ('fne', a, b)), ('feq', a, b),
> '!options->exact_float_comparisons'),
> >>
> >>
> >> The feq/fne one is actually completely safe.  fne is defined to be !feq
> even when NaN is considered.
> >>
> >> --Jasoan
> >>
> >>>
> >>> (('inot', ('ilt', a, b)), ('ige', a, b)),
> >>> (('inot', ('ult', a, b)), ('uge', a, b)),
> >>> (('inot', ('ige', a, b)), ('ilt', a, b)),
> >>> --
> >>> 2.19.2
> >>>
> >>> ___
> >>> mesa-dev mailing list
> >>> mesa-dev@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >>
> >> ___
> >> mesa-dev mailing list
> >> mesa-dev@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org

Re: [Mesa-dev] [PATCH] docs: Document and *require* usage of Signed-off-by

[Ian Romanick]

On 11/30/2018 12:07 PM, Eric Anholt wrote:
> Ian Romanick  writes:
> 
>> On 11/29/2018 03:53 PM, Eric Anholt wrote:
>>> e<#secure method=pgpmime mode=sign>
>>> Erik Faye-Lund  writes:
>>>
 On Wed, 2018-11-28 at 13:43 -0800, Eric Anholt wrote:
> Jordan Justen  writes:
>
>> This adds the "Developer's Certificate of Origin 1.1" from the
>> Linux
>> kernel. It indicates that by using Signed-off-by you are certifying
>> that your patch meets the DCO 1.1 guidelines.
>>
>> It also changes Signed-off-by from being optional to being
>> required.
>>
>> Signed-off-by: Jordan Justen 
>> Cc: Matt Turner 
>
> What problem for our project is solved by signed-off-by?  Do you
> think
> that it has actually reduced the incidence of people submitting code
> they don't have permission to submit in the projects where it's been
> used?  I know the behavior in the kernel is that people submit a
> patch,
> it's missing s-o-b, so it gets bounced, then they maybe add s-o-b or
> just give up.  I don't think anyone stops and says "Wow, that's a
> good
> question.  Maybe I don't have permission to distribute this after
> all?"
>
> /me sees s-o-b as basically just a linux kernel hazing ritual

 I don't think that's the purpose of s-o-b in the Kernel. AFAIK, the
 reason for the s-o-b is to have a paper-trail for how a patch landed in
 the kernel; who it went through etc.
>>>
>>> I get the *idea*, I just believe the idea fails in practice.  The S-O-B
>>> idea came from "wouldn't it be nice if we could make everyone think
>>> about this issue that is important to us" but what we actually got
>>> instead is "your patch gets bounced if you don't have a s-o-b on until
>>> you slap one on and resend."
>>
>> You're thinking like an engineer, but the s-o-b is the spawn of lawyers.
>>  Supposedly, having someone say "I had the rights to contribute this
>> code," even if they didn't think about what that meant, enables you to
>> pass that blame along because that person showed intent.  You may not
>> have read and thought about every page of your mortgage, but you can
>> still be held accountable for every word.  Having an author tag or a
>> committer tag does not show that same intent.  In a legal context, the
>> s-o-b shifts the onus of auditing code ownership from the distributor to
>> the submitter.  When a distributor, like IBM in the SCO case, get sued,
>> they can plausibly claim that all of the code they distributed was
>> vouched for.  If someone submits code that they do not have the right to
>> submit and provides false testimony, then that's on the submitter.
> 
> That's an interesting legal theory.  I can't comment on it, not being a
> lawyer, and I haven't seen lawyers comment on it, either.  What I do see
> for the origin of the DCO is:
> 
> https://lkml.org/lkml/2004/5/23/10
> 
> which looks a lot more like what I was describing (an engineer trying to
> solve a social problem with engineering) than something that came out of
> a lawyers.

I just remember various lawyers at IBM explaining it to us, and they
seemed to think it was the best idea since beer in a bottle. *shrug*

Either way, it was all a long, long time ago, and I don't think any of
it really matters for Mesa.  Even though I've been doing 'git commit -s'
for years, I don't think there's a compelling to require it for Mesa.
It's just part of my work flow.



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 v3 2/5] gallium: Add new PIPE_CAP_SURFACE_SAMPLE_COUNT

[Eric Anholt]

"Kristian H. Kristensen"  writes:

> This new pipe cap and the new nr_samples field in pipe_surface lets a
> state tracker bind a render target with a different sample count than
> the resource. This allows for implementing
> EXT_multisampled_render_to_texture and
> EXT_multisampled_render_to_texture2.
>
> Signed-off-by: Kristian H. Kristensen 

> diff --git a/src/gallium/docs/source/screen.rst 
> b/src/gallium/docs/source/screen.rst
> index 0abd164494c..cf2ce33b87f 100644
> --- a/src/gallium/docs/source/screen.rst
> +++ b/src/gallium/docs/source/screen.rst
> @@ -477,6 +477,9 @@ subpixel precision bias in bits during conservative 
> rasterization.
>0 means no limit.
>  * ``PIPE_CAP_MAX_VERTEX_ELEMENT_SRC_OFFSET``: The maximum supported value for
>of pipe_vertex_element::src_offset.
> +* ``PIPE_CAP_SURFACEA_SAMPLE_COUNT_TEXTURE``: Whether the driver
> +  supports pipe_surface overrides of resource nr_samples. If set, will
> +  enable EXT_multisampled_render_to_texture.

s/SURFACEA/SURFACE/

>  
>  .. _pipe_capf:
>  
> diff --git a/src/gallium/include/pipe/p_defines.h 
> b/src/gallium/include/pipe/p_defines.h
> index e99895d30d8..6d96f1ccb5b 100644
> --- a/src/gallium/include/pipe/p_defines.h
> +++ b/src/gallium/include/pipe/p_defines.h
> @@ -832,6 +832,7 @@ enum pipe_cap
> PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS,
> PIPE_CAP_MAX_TEXTURE_UPLOAD_MEMORY_BUDGET,
> PIPE_CAP_MAX_VERTEX_ELEMENT_SRC_OFFSET,
> +   PIPE_CAP_SURFACE_SAMPLE_COUNT,
>  };
>  
>  /**
> diff --git a/src/gallium/include/pipe/p_state.h 
> b/src/gallium/include/pipe/p_state.h
> index fd670345aad..89cffb15bd8 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -443,6 +443,12 @@ struct pipe_surface
> uint16_t width;   /**< logical width in pixels */
> uint16_t height;  /**< logical height in pixels */
>  
> +   /** Number of samples for the surface.  This can be different from the
> +* resource nr_samples when the resource is bound using
> +* FramebufferTexture2DMultisampleEXT.
> +*/
> +   unsigned nr_samples:8;

Don't you mean:

/**
 * Number of samples for the surface.  This will be 0 if rendering
 * should use the resource's nr_samples, or another value if the resource
 * is bound using FramebufferTexture2DMultisampleEXT
 */

Other than that, 1-3 are:

Reviewed-by: Eric Anholt 


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

Re: [Mesa-dev] [PATCH 1/2] nir: add a compiler option for disabling float comparison simplifications

[Ian Romanick]

On 11/29/2018 07:47 AM, Connor Abbott wrote:
> On Thu, Nov 29, 2018 at 4:22 PM Jason Ekstrand  wrote:
>>
>> Can you provide some context for this?  Those rules are already flagged 
>> "inexact" (that's what the ~ means) so they won't apply to anything that's 
>> "precise" or "invariant".
> 
> I think the concern is that this isn't allowed in SPIR-V, even without
> exact or invariant. We even go out of our way to do the correct thing
> in the frontend by inserting an "&& a == a" or "|| a != a", but then

If you're that paranoid about it, why not just mark the operations are
precise?  That's literally why it exists.

> opt_algebraic removes it with another rule and then this rule can flip
> it from ordered to unordered. The spec says that operations don't have
> to produce NaN, but it doesn't say anything on comparisons other than
> the generic "everything must follow IEEE rules" and an entry in the
> table that says "produces correct results." Then again, I can't find
> anything in GLSL allowing these transforms either, so maybe we just
> need to get rid of them.

What I hear you saying is, "The behavior isn't defined."  Unless you can
point to a CTS test or an application that has incorrect behavior, I'm
going to oppose removing this pretty strongly.  *Every* GLSL compiler
does this.

>> On Thu, Nov 29, 2018 at 9:18 AM Samuel Pitoiset  
>> wrote:
>>>
>>> It's correct in GLSL because the behaviour is undefined in
>>> presence of NaNs. But this seems incorrect in Vulkan.
>>>
>>> Signed-off-by: Samuel Pitoiset 
>>> ---
>>>  src/compiler/nir/nir.h| 6 ++
>>>  src/compiler/nir/nir_opt_algebraic.py | 8 
>>>  2 files changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>>> index db935c8496b..4107c293962 100644
>>> --- a/src/compiler/nir/nir.h
>>> +++ b/src/compiler/nir/nir.h
>>> @@ -2188,6 +2188,12 @@ typedef struct nir_shader_compiler_options {
>>> /* Set if nir_lower_wpos_ytransform() should also invert gl_PointCoord. 
>>> */
>>> bool lower_wpos_pntc;
>>>
>>> +   /* If false, lower ~inot(flt(a,b)) -> fge(a,b) and variants.
>>> +* In presence of NaNs, this is correct in GLSL because the 
>>> behaviour is
>>> +* undefined. In Vulkan, doing these transformations is incorrect.
>>> +*/
>>> +   bool exact_float_comparisons;
>>> +
>>> /**
>>>  * Should nir_lower_io() create load_interpolated_input intrinsics?
>>>  *
>>> diff --git a/src/compiler/nir/nir_opt_algebraic.py 
>>> b/src/compiler/nir/nir_opt_algebraic.py
>>> index f2a7be0c403..3750874407b 100644
>>> --- a/src/compiler/nir/nir_opt_algebraic.py
>>> +++ b/src/compiler/nir/nir_opt_algebraic.py
>>> @@ -154,10 +154,10 @@ optimizations = [
>>> (('ishl', ('imul', a, '#b'), '#c'), ('imul', a, ('ishl', b, c))),
>>>
>>> # Comparison simplifications
>>> -   (('~inot', ('flt', a, b)), ('fge', a, b)),
>>> -   (('~inot', ('fge', a, b)), ('flt', a, b)),
>>> -   (('~inot', ('feq', a, b)), ('fne', a, b)),
>>> -   (('~inot', ('fne', a, b)), ('feq', a, b)),
>>> +   (('~inot', ('flt', a, b)), ('fge', a, b), 
>>> '!options->exact_float_comparisons'),
>>> +   (('~inot', ('fge', a, b)), ('flt', a, b), 
>>> '!options->exact_float_comparisons'),
>>> +   (('~inot', ('feq', a, b)), ('fne', a, b), 
>>> '!options->exact_float_comparisons'),
>>> +   (('~inot', ('fne', a, b)), ('feq', a, b), 
>>> '!options->exact_float_comparisons'),
>>
>>
>> The feq/fne one is actually completely safe.  fne is defined to be !feq even 
>> when NaN is considered.
>>
>> --Jasoan
>>
>>>
>>> (('inot', ('ilt', a, b)), ('ige', a, b)),
>>> (('inot', ('ult', a, b)), ('uge', a, b)),
>>> (('inot', ('ige', a, b)), ('ilt', a, b)),
>>> --
>>> 2.19.2
>>>
>>> ___
>>> mesa-dev mailing list
>>> mesa-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> ___
> mesa-dev 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] Lets talk about autotools

[Jason Ekstrand]

On Fri, Nov 30, 2018 at 2:45 PM Matt Turner  wrote:

> On Fri, Nov 30, 2018 at 2:28 AM Gert Wollny  wrote:
> > Am Donnerstag, den 29.11.2018, 17:44 + schrieb Emil Velikov:
> > > Hi all,
> > >
> > > I can see why people may opt to not use or maintain the autotools
> > > build. Although I would kindly ask that we do not remove it just yet.
> >
> > I second that, I think the process of removing autotools should be a
> > two-step procedure. i.e. prior to ripping autotools support out there
> > should be one or two releases that deprecates it, e.g. by changing
> > configure that it (1) needs an extra flag to run, and (2) when run
> > without this flag it would just print a message about the meson build
> > system, the deprecation of autotools (making it clear when it will be
> > removed), and information how to still run autotools with this extra
> > flag. If qwe
> >
> > A rationale is that with a release that only has mesa there is a high
> > chance that people not directly involved with the project, and that
> > don't follow git but only use the releases hit corner cases when
> > building mesa that we or maintainers for distributions might not be
> > aware of. Still having something around that is known to work would be
> > good for them, so they can report a bug against the meson build system
> > and still get their work done by easily switching to the autotools for
> > the time being.
>
> I've been using Meson to build Mesa in Gentoo for a ~6 months now, and
> I've reported (and Dylan has fixed) a handful of corner case bugs.
> Dylan is super responsive, usually having a fix within a day. It's
> been great, really.
>
> And we've been discussing removing autotools for more than a year now.
>
> I think we've been plenty conservative and there's nothing to be
> gained / no problems to be avoided by delaying further. (And again, if
> someone comes out of the woodwork with a bug in the Meson build, Dylan
> will fix it within a day in my experience)
>

I fully agree.  If we wait another 6 months, someone will make exactly the
same argument because surely there's someone out there who hasn't tried out
meson yet.  The only way to force every one of our users to give meson a
try and report the last handful of bugs is to delete autotools from the
tree.  It's been very stable for some time now; let's just pull the trigger.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Lets talk about autotools

[Matt Turner]

On Fri, Nov 30, 2018 at 2:28 AM Gert Wollny  wrote:
> Am Donnerstag, den 29.11.2018, 17:44 + schrieb Emil Velikov:
> > Hi all,
> >
> > I can see why people may opt to not use or maintain the autotools
> > build. Although I would kindly ask that we do not remove it just yet.
>
> I second that, I think the process of removing autotools should be a
> two-step procedure. i.e. prior to ripping autotools support out there
> should be one or two releases that deprecates it, e.g. by changing
> configure that it (1) needs an extra flag to run, and (2) when run
> without this flag it would just print a message about the meson build
> system, the deprecation of autotools (making it clear when it will be
> removed), and information how to still run autotools with this extra
> flag. If qwe
>
> A rationale is that with a release that only has mesa there is a high
> chance that people not directly involved with the project, and that
> don't follow git but only use the releases hit corner cases when
> building mesa that we or maintainers for distributions might not be
> aware of. Still having something around that is known to work would be
> good for them, so they can report a bug against the meson build system
> and still get their work done by easily switching to the autotools for
> the time being.

I've been using Meson to build Mesa in Gentoo for a ~6 months now, and
I've reported (and Dylan has fixed) a handful of corner case bugs.
Dylan is super responsive, usually having a fix within a day. It's
been great, really.

And we've been discussing removing autotools for more than a year now.

I think we've been plenty conservative and there's nothing to be
gained / no problems to be avoided by delaying further. (And again, if
someone comes out of the woodwork with a bug in the Meson build, Dylan
will fix it within a day in my experience)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/5] gallium: Add new PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE

[Marek Olšák]

GCN calls it overrasterization multisampling, but it's really only useful
for polygon smoothing, because there is no temporary buffer.

Marek

On Fri, Nov 30, 2018 at 3:35 PM Rob Clark  wrote:

> On Fri, Nov 30, 2018 at 3:25 PM Marek Olšák  wrote:
> >
> > Suggestions:
> > - PIPE_CAP_TILED_BASED_MSAA_OVERSAMPLING
> > - pipe_surface::tile_based_oversample_count
> >
> > I'm assuming this feature isn't possible without tile-based rendering.
>
> I guess technically an IMR could do it, with an internal temporary
> buffer not visible to the state-tracker.  (Not sure I could come up
> with a reason *why* you'd want to do that, but...)
>
> BR,
> -R
>
> >
> > Marek
> >
> > On Fri, Nov 30, 2018 at 1:23 PM Kristian Høgsberg 
> wrote:
> >>
> >> On Fri, Nov 30, 2018 at 10:17 AM Marek Olšák  wrote:
> >> >
> >> > On Fri, Nov 30, 2018 at 1:13 PM Kristian Høgsberg <
> hoegsb...@gmail.com> wrote:
> >> >>
> >> >> On Fri, Nov 16, 2018 at 7:48 PM Marek Olšák 
> wrote:
> >> >> >
> >> >> > I think the name PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE is
> slightly misleading, because it doesn't imply anything about the OpenGL ES
> behavior, which is that a texture is multisampled in the cache, but
> single-sampled in memory. This should be mentioned somewhere.
> >> >>
> >> >> Not sure exactly which change you're recommending, but in retrospect,
> >> >> I think it would be better to name the cap in term of how it changes
> >> >> the gallium API instead of the GLES extension it enables. How about
> >> >> PIPE_CAP_SURFACE_SAMPLE_COUNT, to indicate that it allows overriding
> >> >> sample counts in pipe_surface?
> >> >
> >> >
> >> > That's an interesting idea, but how does it convey that multisampled
> surfaces are never multisampled in memory?
> >>
> >> How are you going to convey all that in a cap enum name? In general,
> >> the cap names are concise hints and not super descriptive - you have
> >> to go read the documentation to understand the semantics.
> >>
> >> Do you have a specific name you'd like to see or can we go with
> >> PIPE_CAP_SURFACE_SAMPLE_COUNT?
> >>
> >> >
> >> > Marek
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/5] gallium: Add new PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE

[Rob Clark]

On Fri, Nov 30, 2018 at 3:25 PM Marek Olšák  wrote:
>
> Suggestions:
> - PIPE_CAP_TILED_BASED_MSAA_OVERSAMPLING
> - pipe_surface::tile_based_oversample_count
>
> I'm assuming this feature isn't possible without tile-based rendering.

I guess technically an IMR could do it, with an internal temporary
buffer not visible to the state-tracker.  (Not sure I could come up
with a reason *why* you'd want to do that, but...)

BR,
-R

>
> Marek
>
> On Fri, Nov 30, 2018 at 1:23 PM Kristian Høgsberg  wrote:
>>
>> On Fri, Nov 30, 2018 at 10:17 AM Marek Olšák  wrote:
>> >
>> > On Fri, Nov 30, 2018 at 1:13 PM Kristian Høgsberg  
>> > wrote:
>> >>
>> >> On Fri, Nov 16, 2018 at 7:48 PM Marek Olšák  wrote:
>> >> >
>> >> > I think the name PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE is slightly 
>> >> > misleading, because it doesn't imply anything about the OpenGL ES 
>> >> > behavior, which is that a texture is multisampled in the cache, but 
>> >> > single-sampled in memory. This should be mentioned somewhere.
>> >>
>> >> Not sure exactly which change you're recommending, but in retrospect,
>> >> I think it would be better to name the cap in term of how it changes
>> >> the gallium API instead of the GLES extension it enables. How about
>> >> PIPE_CAP_SURFACE_SAMPLE_COUNT, to indicate that it allows overriding
>> >> sample counts in pipe_surface?
>> >
>> >
>> > That's an interesting idea, but how does it convey that multisampled 
>> > surfaces are never multisampled in memory?
>>
>> How are you going to convey all that in a cap enum name? In general,
>> the cap names are concise hints and not super descriptive - you have
>> to go read the documentation to understand the semantics.
>>
>> Do you have a specific name you'd like to see or can we go with
>> PIPE_CAP_SURFACE_SAMPLE_COUNT?
>>
>> >
>> > Marek
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/5] gallium: Add new PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE

[Marek Olšák]

Suggestions:
- PIPE_CAP_TILED_BASED_MSAA_OVERSAMPLING
- pipe_surface::tile_based_oversample_count

I'm assuming this feature isn't possible without tile-based rendering.

Marek

On Fri, Nov 30, 2018 at 1:23 PM Kristian Høgsberg 
wrote:

> On Fri, Nov 30, 2018 at 10:17 AM Marek Olšák  wrote:
> >
> > On Fri, Nov 30, 2018 at 1:13 PM Kristian Høgsberg 
> wrote:
> >>
> >> On Fri, Nov 16, 2018 at 7:48 PM Marek Olšák  wrote:
> >> >
> >> > I think the name PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE is slightly
> misleading, because it doesn't imply anything about the OpenGL ES behavior,
> which is that a texture is multisampled in the cache, but single-sampled in
> memory. This should be mentioned somewhere.
> >>
> >> Not sure exactly which change you're recommending, but in retrospect,
> >> I think it would be better to name the cap in term of how it changes
> >> the gallium API instead of the GLES extension it enables. How about
> >> PIPE_CAP_SURFACE_SAMPLE_COUNT, to indicate that it allows overriding
> >> sample counts in pipe_surface?
> >
> >
> > That's an interesting idea, but how does it convey that multisampled
> surfaces are never multisampled in memory?
>
> How are you going to convey all that in a cap enum name? In general,
> the cap names are concise hints and not super descriptive - you have
> to go read the documentation to understand the semantics.
>
> Do you have a specific name you'd like to see or can we go with
> PIPE_CAP_SURFACE_SAMPLE_COUNT?
>
> >
> > Marek
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Lets talk about autotools

[Dave Airlie]

On Wed., 28 Nov. 2018, 03:06 Matt Turner  On Tue, Nov 27, 2018 at 1:13 AM Timo Aaltonen  wrote:
> >
> > On 17.11.2018 6.04, Dylan Baker wrote:
> > > Quoting Dylan Baker (2018-09-17 09:44:07)
> > >> I feel like for !windows meson is in good enough shape at this point
> that we
> > >> can start having the discussion about deleting the autotools build.
> So, is there
> > >> anything left that autotools can do that meson cannot (that we
> actually want to
> > >> implement)? And, what is a reasonable time-table to remove the
> autotools build?
> > >> I think we could reasonably remove it as soon as 18.3 if others felt
> confident
> > >> that it would work for them.
> > >>
> > >> Dylan
> > >
> > > Okay, time for an update on things and a chance to talk about what
> else we need.
> > >
> > > Support for llvm-config (and any binary, actually) overriding has
> landed in
> > > meson, and will be present in the 0.49.0 release, which is due out
> December 9th.
> >
> > Hi, just a note that Ubuntu 18.04 LTS ships with meson 0.45.1 and will
> > get Mesa backports up until and including 20.0.x, so I wonder how
> > complex these required new features in meson are to be backported, or
> > perhaps easily worked around? Backporting a whole new version of meson
> > might not happen..
>
> I understand the LTS concept, but what's the value in never upgrading
> something like a build tool like Meson? Yeah, new versions give a
> possibility of regressions, but with something evolving as quickly as
> Meson the version available in April 2018 becomes less useful for its
> intended purpose with each passing month...
>

Is meson guaranteeing backwards compatibility? Will meson in 2 years build
a package built with meson now?

If I have mesa and another package using meson 0.48, and I update meson to
0.52 will the other package still rebuild fine for things like security
updates?

Dave.

> ___
> 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] docs: Document and *require* usage of Signed-off-by

[Eric Anholt]

Ian Romanick  writes:

> On 11/29/2018 03:53 PM, Eric Anholt wrote:
>> e<#secure method=pgpmime mode=sign>
>> Erik Faye-Lund  writes:
>> 
>>> On Wed, 2018-11-28 at 13:43 -0800, Eric Anholt wrote:
 Jordan Justen  writes:

> This adds the "Developer's Certificate of Origin 1.1" from the
> Linux
> kernel. It indicates that by using Signed-off-by you are certifying
> that your patch meets the DCO 1.1 guidelines.
>
> It also changes Signed-off-by from being optional to being
> required.
>
> Signed-off-by: Jordan Justen 
> Cc: Matt Turner 

 What problem for our project is solved by signed-off-by?  Do you
 think
 that it has actually reduced the incidence of people submitting code
 they don't have permission to submit in the projects where it's been
 used?  I know the behavior in the kernel is that people submit a
 patch,
 it's missing s-o-b, so it gets bounced, then they maybe add s-o-b or
 just give up.  I don't think anyone stops and says "Wow, that's a
 good
 question.  Maybe I don't have permission to distribute this after
 all?"

 /me sees s-o-b as basically just a linux kernel hazing ritual
>>>
>>> I don't think that's the purpose of s-o-b in the Kernel. AFAIK, the
>>> reason for the s-o-b is to have a paper-trail for how a patch landed in
>>> the kernel; who it went through etc.
>> 
>> I get the *idea*, I just believe the idea fails in practice.  The S-O-B
>> idea came from "wouldn't it be nice if we could make everyone think
>> about this issue that is important to us" but what we actually got
>> instead is "your patch gets bounced if you don't have a s-o-b on until
>> you slap one on and resend."
>
> You're thinking like an engineer, but the s-o-b is the spawn of lawyers.
>  Supposedly, having someone say "I had the rights to contribute this
> code," even if they didn't think about what that meant, enables you to
> pass that blame along because that person showed intent.  You may not
> have read and thought about every page of your mortgage, but you can
> still be held accountable for every word.  Having an author tag or a
> committer tag does not show that same intent.  In a legal context, the
> s-o-b shifts the onus of auditing code ownership from the distributor to
> the submitter.  When a distributor, like IBM in the SCO case, get sued,
> they can plausibly claim that all of the code they distributed was
> vouched for.  If someone submits code that they do not have the right to
> submit and provides false testimony, then that's on the submitter.

That's an interesting legal theory.  I can't comment on it, not being a
lawyer, and I haven't seen lawyers comment on it, either.  What I do see
for the origin of the DCO is:

https://lkml.org/lkml/2004/5/23/10

which looks a lot more like what I was describing (an engineer trying to
solve a social problem with engineering) than something that came out of
a lawyers.


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

[Mesa-dev] [PATCH v3 0/5] EXT_multisampled_render_to_texture and MSAA for a6xx

[Kristian H. Kristensen]

Here's v3 of the series. I've updated to account for most of the comments, 
except
I went back and forth and then ultimately back again on Roladnd suggestion.
It may be more intuitive to always have surf->nr_samples be the number of 
samples
but that means we'll have to go update all state trackers to set it in the 
template
and modify all drivers to copy it in the constructor.  At the same time, having
surf->nr_samples be 0 if it wasn't attached with 
FramebufferTexture2DMultisampleEXT
also seems like perfectly fine semantics, and doesn't require updating state 
trackers
or drivers that don't care or support this feature.

Kristian



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

[Mesa-dev] [PATCH v3 1/5] mesa: Add core support for EXT_multisampled_render_to_texture{, 2}

[Kristian H. Kristensen]

This also turns on EXT_multisampled_render_to_texture which is a
subset of EXT_multisampled_render_to_texture2, allowing only
COLOR_ATTACHMENT0.

Signed-off-by: Kristian H. Kristensen 
---
 .../EXT_multisampled_render_to_texture.xml| 34 +++
 src/mapi/glapi/gen/Makefile.am|  1 +
 src/mapi/glapi/gen/gl_API.xml |  2 +
 src/mapi/glapi/gen/meson.build|  1 +
 src/mesa/drivers/common/meta.c|  2 +-
 src/mesa/main/extensions_table.h  |  2 +
 src/mesa/main/fbobject.c  | 57 ++-
 src/mesa/main/fbobject.h  |  8 ++-
 src/mesa/main/glheader.h  |  3 +
 src/mesa/main/mtypes.h|  2 +
 10 files changed, 96 insertions(+), 16 deletions(-)
 create mode 100644 src/mapi/glapi/gen/EXT_multisampled_render_to_texture.xml

diff --git a/src/mapi/glapi/gen/EXT_multisampled_render_to_texture.xml 
b/src/mapi/glapi/gen/EXT_multisampled_render_to_texture.xml
new file mode 100644
index 000..555b008bd33
--- /dev/null
+++ b/src/mapi/glapi/gen/EXT_multisampled_render_to_texture.xml
@@ -0,0 +1,34 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
index 6e0ee1e1687..40538b0ff2e 100644
--- a/src/mapi/glapi/gen/Makefile.am
+++ b/src/mapi/glapi/gen/Makefile.am
@@ -200,6 +200,7 @@ API_XML = \
EXT_external_objects_fd.xml \
EXT_framebuffer_object.xml \
EXT_gpu_shader4.xml \
+   EXT_multisampled_render_to_texture.xml \
EXT_packed_depth_stencil.xml \
EXT_provoking_vertex.xml \
EXT_separate_shader_objects.xml \
diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
index f4d0808f13b..f1def8090de 100644
--- a/src/mapi/glapi/gen/gl_API.xml
+++ b/src/mapi/glapi/gen/gl_API.xml
@@ -8175,6 +8175,8 @@
 
 http://www.w3.org/2001/XInclude"/>
 
+http://www.w3.org/2001/XInclude"/>
+
 http://www.w3.org/2001/XInclude"/>
 
 
diff --git a/src/mapi/glapi/gen/meson.build b/src/mapi/glapi/gen/meson.build
index f494e9707b6..8cc163b2989 100644
--- a/src/mapi/glapi/gen/meson.build
+++ b/src/mapi/glapi/gen/meson.build
@@ -107,6 +107,7 @@ api_xml_files = files(
   'EXT_external_objects_fd.xml',
   'EXT_framebuffer_object.xml',
   'EXT_gpu_shader4.xml',
+  'EXT_multisampled_render_to_texture.xml',
   'EXT_packed_depth_stencil.xml',
   'EXT_provoking_vertex.xml',
   'EXT_separate_shader_objects.xml',
diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
index 4392c4bbd88..3515e312023 100644
--- a/src/mesa/drivers/common/meta.c
+++ b/src/mesa/drivers/common/meta.c
@@ -127,7 +127,7 @@ _mesa_meta_framebuffer_texture_image(struct gl_context *ctx,
assert(att);
 
_mesa_framebuffer_texture(ctx, fb, attachment, att, texObj, texTarget,
- level, layer, false);
+ level, att->NumSamples, layer, false);
 }
 
 static struct gl_shader *
diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h
index dd846d7bc5c..306f6a78c17 100644
--- a/src/mesa/main/extensions_table.h
+++ b/src/mesa/main/extensions_table.h
@@ -241,6 +241,8 @@ EXT(EXT_map_buffer_range, 
ARB_map_buffer_range
 EXT(EXT_memory_object   , EXT_memory_object
  , GLL, GLC,  x , ES2, 2017)
 EXT(EXT_memory_object_fd, EXT_memory_object_fd 
  , GLL, GLC,  x , ES2, 2017)
 EXT(EXT_multi_draw_arrays   , dummy_true   
  , GLL,  x , ES1, ES2, 1999)
+EXT(EXT_multisampled_render_to_texture  , 
EXT_multisampled_render_to_texture ,  x ,  x ,  x , ES2, 2016)
+EXT(EXT_multisampled_render_to_texture2 , 
EXT_multisampled_render_to_texture ,  x ,  x ,  x , ES2, 2016)
 EXT(EXT_occlusion_query_boolean , ARB_occlusion_query2 
  ,  x ,  x ,  x , ES2, 2011)
 EXT(EXT_packed_depth_stencil, dummy_true   
  , GLL, GLC,  x ,  x , 2005)
 EXT(EXT_packed_float, EXT_packed_float 
  , GLL, GLC,  x ,  x , 2004)
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 68e0daf3423..23e49396199 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -497,8 +497,8 @@ set_texture_attachment(struct gl_context *ctx,
struct gl_framebuffer *fb,
struct gl_renderbuffer_attachment *att,
struct gl_texture_object *texObj,
-   GLenum texTarget, GLuint level, GLuint layer,
-   GLboolean layered)
+   GLenum texTarget, GLuint level, GLsizei samples,
+   GLuint layer, GLboolean layered)
 {
struct gl_renderbuffer *rb = 

[Mesa-dev] [PATCH v3 3/5] st/mesa: Add support for EXT_multisampled_render_to_texture

[Kristian H. Kristensen]

In gallium, we model the attachment sample count as a new nr_samples
field in pipe_surface. A driver can indicate support for the extension
using the new pipe cap, PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE.

Signed-off-by: Kristian H. Kristensen 
---
 src/mesa/state_tracker/st_cb_fbo.c | 3 +++
 src/mesa/state_tracker/st_cb_fbo.h | 1 +
 src/mesa/state_tracker/st_extensions.c | 1 +
 3 files changed, 5 insertions(+)

diff --git a/src/mesa/state_tracker/st_cb_fbo.c 
b/src/mesa/state_tracker/st_cb_fbo.c
index 0e535257cb4..8901a8680ef 100644
--- a/src/mesa/state_tracker/st_cb_fbo.c
+++ b/src/mesa/state_tracker/st_cb_fbo.c
@@ -516,6 +516,7 @@ st_update_renderbuffer_surface(struct st_context *st,
surf->texture != resource ||
surf->width != rtt_width ||
surf->height != rtt_height ||
+   surf->nr_samples != strb->rtt_nr_samples ||
surf->u.tex.level != level ||
surf->u.tex.first_layer != first_layer ||
surf->u.tex.last_layer != last_layer) {
@@ -523,6 +524,7 @@ st_update_renderbuffer_surface(struct st_context *st,
   struct pipe_surface surf_tmpl;
   memset(_tmpl, 0, sizeof(surf_tmpl));
   surf_tmpl.format = format;
+  surf_tmpl.nr_samples = strb->rtt_nr_samples;
   surf_tmpl.u.tex.level = level;
   surf_tmpl.u.tex.first_layer = first_layer;
   surf_tmpl.u.tex.last_layer = last_layer;
@@ -572,6 +574,7 @@ st_render_texture(struct gl_context *ctx,
strb->rtt_face = att->CubeMapFace;
strb->rtt_slice = att->Zoffset;
strb->rtt_layered = att->Layered;
+   strb->rtt_nr_samples = att->NumSamples;
pipe_resource_reference(>texture, pt);
 
st_update_renderbuffer_surface(st, strb);
diff --git a/src/mesa/state_tracker/st_cb_fbo.h 
b/src/mesa/state_tracker/st_cb_fbo.h
index 345c11442c6..046f01713ce 100644
--- a/src/mesa/state_tracker/st_cb_fbo.h
+++ b/src/mesa/state_tracker/st_cb_fbo.h
@@ -69,6 +69,7 @@ struct st_renderbuffer
boolean is_rtt; /**< whether Driver.RenderTexture was called */
unsigned rtt_face, rtt_slice;
boolean rtt_layered; /**< whether glFramebufferTexture was called */
+   unsigned rtt_nr_samples; /**< from FramebufferTexture2DMultisampleEXT */
 };
 
 
diff --git a/src/mesa/state_tracker/st_extensions.c 
b/src/mesa/state_tracker/st_extensions.c
index 16889074f66..3cb1a646f38 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -740,6 +740,7 @@ void st_init_extensions(struct pipe_screen *screen,
   { o(EXT_draw_buffers2),PIPE_CAP_INDEP_BLEND_ENABLE   
},
   { o(EXT_memory_object),PIPE_CAP_MEMOBJ   
},
   { o(EXT_memory_object_fd), PIPE_CAP_MEMOBJ   
},
+  { o(EXT_multisampled_render_to_texture), PIPE_CAP_SURFACE_SAMPLE_COUNT   
},
   { o(EXT_semaphore),PIPE_CAP_FENCE_SIGNAL 
},
   { o(EXT_semaphore_fd), PIPE_CAP_FENCE_SIGNAL 
},
   { o(EXT_texture_array),PIPE_CAP_MAX_TEXTURE_ARRAY_LAYERS 
},
-- 
2.18.1

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

[Mesa-dev] [PATCH v3 4/5] freedreno/a6xx: MSAA

[Kristian H. Kristensen]

From: Rob Clark 

Signed-off-by: Rob Clark 
---
 .../drivers/freedreno/a6xx/fd6_blend.c|  2 +
 .../drivers/freedreno/a6xx/fd6_context.c  |  2 +
 src/gallium/drivers/freedreno/a6xx/fd6_emit.c | 12 --
 src/gallium/drivers/freedreno/a6xx/fd6_gmem.c | 41 +++
 .../drivers/freedreno/a6xx/fd6_rasterizer.c   |  4 +-
 .../drivers/freedreno/a6xx/fd6_screen.c   | 24 +--
 .../drivers/freedreno/a6xx/fd6_texture.c  |  1 +
 .../drivers/freedreno/adreno_common.xml.h |  1 +
 .../drivers/freedreno/freedreno_resource.c|  9 
 .../drivers/freedreno/freedreno_util.h|  2 +
 10 files changed, 74 insertions(+), 24 deletions(-)

diff --git a/src/gallium/drivers/freedreno/a6xx/fd6_blend.c 
b/src/gallium/drivers/freedreno/a6xx/fd6_blend.c
index 185b061cd1e..f888e162cf9 100644
--- a/src/gallium/drivers/freedreno/a6xx/fd6_blend.c
+++ b/src/gallium/drivers/freedreno/a6xx/fd6_blend.c
@@ -138,8 +138,10 @@ fd6_blend_state_create(struct pipe_context *pctx,
}
 
so->rb_blend_cntl = A6XX_RB_BLEND_CNTL_ENABLE_BLEND(mrt_blend) |
+   COND(cso->alpha_to_coverage, 
A6XX_RB_BLEND_CNTL_ALPHA_TO_COVERAGE) |
COND(cso->independent_blend_enable, 
A6XX_RB_BLEND_CNTL_INDEPENDENT_BLEND);
so->sp_blend_cntl = A6XX_SP_BLEND_CNTL_UNK8 |
+   COND(cso->alpha_to_coverage, 
A6XX_SP_BLEND_CNTL_ALPHA_TO_COVERAGE) |
COND(mrt_blend, A6XX_SP_BLEND_CNTL_ENABLED);
 
return so;
diff --git a/src/gallium/drivers/freedreno/a6xx/fd6_context.c 
b/src/gallium/drivers/freedreno/a6xx/fd6_context.c
index 3282b7d86cf..35fd03c3d99 100644
--- a/src/gallium/drivers/freedreno/a6xx/fd6_context.c
+++ b/src/gallium/drivers/freedreno/a6xx/fd6_context.c
@@ -104,6 +104,8 @@ fd6_context_create(struct pipe_screen *pscreen, void *priv, 
unsigned flags)
if (!pctx)
return NULL;
 
+   util_blitter_set_texture_multisample(fd6_ctx->base.blitter, true);
+
/* fd_context_init overwrites delete_rasterizer_state, so set this
 * here. */
pctx->delete_rasterizer_state = fd6_rasterizer_state_delete;
diff --git a/src/gallium/drivers/freedreno/a6xx/fd6_emit.c 
b/src/gallium/drivers/freedreno/a6xx/fd6_emit.c
index 70b93340e77..c4d43c22f99 100644
--- a/src/gallium/drivers/freedreno/a6xx/fd6_emit.c
+++ b/src/gallium/drivers/freedreno/a6xx/fd6_emit.c
@@ -879,14 +879,18 @@ fd6_emit_state(struct fd_ringbuffer *ring, struct 
fd6_emit *emit)
OUT_RING(ring, blend_control);
}
 
-   OUT_PKT4(ring, REG_A6XX_RB_BLEND_CNTL, 1);
-   OUT_RING(ring, blend->rb_blend_cntl |
-   A6XX_RB_BLEND_CNTL_SAMPLE_MASK(0x));
-
OUT_PKT4(ring, REG_A6XX_SP_BLEND_CNTL, 1);
OUT_RING(ring, blend->sp_blend_cntl);
}
 
+   if (dirty & (FD_DIRTY_BLEND | FD_DIRTY_SAMPLE_MASK)) {
+   struct fd6_blend_stateobj *blend = 
fd6_blend_stateobj(ctx->blend);
+
+   OUT_PKT4(ring, REG_A6XX_RB_BLEND_CNTL, 1);
+   OUT_RING(ring, blend->rb_blend_cntl |
+   
A6XX_RB_BLEND_CNTL_SAMPLE_MASK(ctx->sample_mask));
+   }
+
if (dirty & FD_DIRTY_BLEND_COLOR) {
struct pipe_blend_color *bcolor = >blend_color;
 
diff --git a/src/gallium/drivers/freedreno/a6xx/fd6_gmem.c 
b/src/gallium/drivers/freedreno/a6xx/fd6_gmem.c
index 94ad6641718..8cda7d6ddae 100644
--- a/src/gallium/drivers/freedreno/a6xx/fd6_gmem.c
+++ b/src/gallium/drivers/freedreno/a6xx/fd6_gmem.c
@@ -89,7 +89,7 @@ emit_mrt(struct fd_ringbuffer *ring, struct 
pipe_framebuffer_state *pfb,
offset = fd_resource_offset(rsc, psurf->u.tex.level,

psurf->u.tex.first_layer);
 
-   stride = slice->pitch * rsc->cpp;
+   stride = slice->pitch * rsc->cpp * pfb->samples;
 
debug_assert(psurf->u.tex.first_layer == 
psurf->u.tex.last_layer);
debug_assert((offset + slice->size0) <= fd_bo_size(rsc->bo));
@@ -414,23 +414,27 @@ emit_binning_pass(struct fd_batch *batch)
 }
 
 static void
-disable_msaa(struct fd_ringbuffer *ring)
+emit_msaa(struct fd_ringbuffer *ring, unsigned nr)
 {
-   // TODO MSAA
+   enum a3xx_msaa_samples samples = fd_msaa_samples(nr);
+
OUT_PKT4(ring, REG_A6XX_SP_TP_RAS_MSAA_CNTL, 2);
-   OUT_RING(ring, A6XX_SP_TP_RAS_MSAA_CNTL_SAMPLES(MSAA_ONE));
-   OUT_RING(ring, A6XX_SP_TP_DEST_MSAA_CNTL_SAMPLES(MSAA_ONE) |
-A6XX_SP_TP_DEST_MSAA_CNTL_MSAA_DISABLE);
+   OUT_RING(ring, A6XX_SP_TP_RAS_MSAA_CNTL_SAMPLES(samples));
+   OUT_RING(ring, A6XX_SP_TP_DEST_MSAA_CNTL_SAMPLES(samples) |
+COND(samples == MSAA_ONE, 
A6XX_SP_TP_DEST_MSAA_CNTL_MSAA_DISABLE));
 
OUT_PKT4(ring, REG_A6XX_GRAS_RAS_MSAA_CNTL, 2);
-   OUT_RING(ring, 

[Mesa-dev] [PATCH v3 5/5] freedreno: Add support for EXT_multisampled_render_to_texture

[Kristian H. Kristensen]

There is not much to do in freedreno - tile layout and multisample
state for gmem renderings is programmed based on the pfb sample count,
while resolve blits take the destination sample count from the resource.

Signed-off-by: Kristian H. Kristensen 
---
 src/gallium/drivers/freedreno/freedreno_batch_cache.c | 4 +++-
 src/gallium/drivers/freedreno/freedreno_screen.c  | 3 +++
 src/gallium/drivers/freedreno/freedreno_surface.c | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/freedreno/freedreno_batch_cache.c 
b/src/gallium/drivers/freedreno/freedreno_batch_cache.c
index 408d48ccdb6..45cd9c172d3 100644
--- a/src/gallium/drivers/freedreno/freedreno_batch_cache.c
+++ b/src/gallium/drivers/freedreno/freedreno_batch_cache.c
@@ -81,7 +81,8 @@ struct key {
struct {
struct pipe_resource *texture;
union pipe_surface_desc u;
-   uint16_t pos, format;
+   uint8_t pos, samples;
+   uint16_t format;
} surf[0];
 };
 
@@ -401,6 +402,7 @@ key_surf(struct key *key, unsigned idx, unsigned pos, 
struct pipe_surface *psurf
key->surf[idx].texture = psurf->texture;
key->surf[idx].u = psurf->u;
key->surf[idx].pos = pos;
+   key->surf[idx].samples = psurf->nr_samples;
key->surf[idx].format = psurf->format;
 }
 
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c 
b/src/gallium/drivers/freedreno/freedreno_screen.c
index ab83487aef8..03b358782c1 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -225,6 +225,9 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum 
pipe_cap param)
case PIPE_CAP_TEXTURE_MULTISAMPLE:
return is_a5xx(screen) || is_a6xx(screen);
 
+   case PIPE_CAP_SURFACE_SAMPLE_COUNT:
+   return is_a6xx(screen);
+
case PIPE_CAP_DEPTH_CLIP_DISABLE:
return is_a3xx(screen) || is_a4xx(screen);
 
diff --git a/src/gallium/drivers/freedreno/freedreno_surface.c 
b/src/gallium/drivers/freedreno/freedreno_surface.c
index 6f415f69993..24da54798b6 100644
--- a/src/gallium/drivers/freedreno/freedreno_surface.c
+++ b/src/gallium/drivers/freedreno/freedreno_surface.c
@@ -53,6 +53,7 @@ fd_create_surface(struct pipe_context *pctx,
psurf->format = surf_tmpl->format;
psurf->width = u_minify(ptex->width0, level);
psurf->height = u_minify(ptex->height0, level);
+   psurf->nr_samples = surf_tmpl->nr_samples;
 
if (ptex->target == PIPE_BUFFER) {
psurf->u.buf.first_element = surf_tmpl->u.buf.first_element;
-- 
2.18.1

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

[Mesa-dev] [PATCH v3 2/5] gallium: Add new PIPE_CAP_SURFACE_SAMPLE_COUNT

[Kristian H. Kristensen]

This new pipe cap and the new nr_samples field in pipe_surface lets a
state tracker bind a render target with a different sample count than
the resource. This allows for implementing
EXT_multisampled_render_to_texture and
EXT_multisampled_render_to_texture2.

Signed-off-by: Kristian H. Kristensen 
---
 src/gallium/auxiliary/util/u_framebuffer.c | 10 --
 src/gallium/docs/source/screen.rst |  3 +++
 src/gallium/include/pipe/p_defines.h   |  1 +
 src/gallium/include/pipe/p_state.h |  6 ++
 4 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/gallium/auxiliary/util/u_framebuffer.c 
b/src/gallium/auxiliary/util/u_framebuffer.c
index 5bafddc726f..f569511393b 100644
--- a/src/gallium/auxiliary/util/u_framebuffer.c
+++ b/src/gallium/auxiliary/util/u_framebuffer.c
@@ -229,13 +229,19 @@ util_framebuffer_get_num_samples(const struct 
pipe_framebuffer_state *fb)
if (!(fb->nr_cbufs || fb->zsbuf))
   return MAX2(fb->samples, 1);
 
+   /**
+* If a driver doesn't advertise PIPE_CAP_SURFACE_SAMPLE_COUNT,
+* pipe_surface::nr_samples will always be 0.
+*/
for (i = 0; i < fb->nr_cbufs; i++) {
   if (fb->cbufs[i]) {
- return MAX2(1, fb->cbufs[i]->texture->nr_samples);
+ return MAX3(1, fb->cbufs[i]->texture->nr_samples,
+ fb->cbufs[i]->nr_samples);
   }
}
if (fb->zsbuf) {
-  return MAX2(1, fb->zsbuf->texture->nr_samples);
+  return MAX3(1, fb->zsbuf->texture->nr_samples,
+  fb->zsbuf->nr_samples);
}
 
return 1;
diff --git a/src/gallium/docs/source/screen.rst 
b/src/gallium/docs/source/screen.rst
index 0abd164494c..cf2ce33b87f 100644
--- a/src/gallium/docs/source/screen.rst
+++ b/src/gallium/docs/source/screen.rst
@@ -477,6 +477,9 @@ subpixel precision bias in bits during conservative 
rasterization.
   0 means no limit.
 * ``PIPE_CAP_MAX_VERTEX_ELEMENT_SRC_OFFSET``: The maximum supported value for
   of pipe_vertex_element::src_offset.
+* ``PIPE_CAP_SURFACEA_SAMPLE_COUNT_TEXTURE``: Whether the driver
+  supports pipe_surface overrides of resource nr_samples. If set, will
+  enable EXT_multisampled_render_to_texture.
 
 .. _pipe_capf:
 
diff --git a/src/gallium/include/pipe/p_defines.h 
b/src/gallium/include/pipe/p_defines.h
index e99895d30d8..6d96f1ccb5b 100644
--- a/src/gallium/include/pipe/p_defines.h
+++ b/src/gallium/include/pipe/p_defines.h
@@ -832,6 +832,7 @@ enum pipe_cap
PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS,
PIPE_CAP_MAX_TEXTURE_UPLOAD_MEMORY_BUDGET,
PIPE_CAP_MAX_VERTEX_ELEMENT_SRC_OFFSET,
+   PIPE_CAP_SURFACE_SAMPLE_COUNT,
 };
 
 /**
diff --git a/src/gallium/include/pipe/p_state.h 
b/src/gallium/include/pipe/p_state.h
index fd670345aad..89cffb15bd8 100644
--- a/src/gallium/include/pipe/p_state.h
+++ b/src/gallium/include/pipe/p_state.h
@@ -443,6 +443,12 @@ struct pipe_surface
uint16_t width;   /**< logical width in pixels */
uint16_t height;  /**< logical height in pixels */
 
+   /** Number of samples for the surface.  This can be different from the
+* resource nr_samples when the resource is bound using
+* FramebufferTexture2DMultisampleEXT.
+*/
+   unsigned nr_samples:8;
+
union pipe_surface_desc u;
 };
 
-- 
2.18.1

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

Re: [Mesa-dev] [PATCH v2 2/5] gallium: Add new PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE

[Kristian Høgsberg]

On Fri, Nov 30, 2018 at 10:17 AM Marek Olšák  wrote:
>
> On Fri, Nov 30, 2018 at 1:13 PM Kristian Høgsberg  wrote:
>>
>> On Fri, Nov 16, 2018 at 7:48 PM Marek Olšák  wrote:
>> >
>> > I think the name PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE is slightly 
>> > misleading, because it doesn't imply anything about the OpenGL ES 
>> > behavior, which is that a texture is multisampled in the cache, but 
>> > single-sampled in memory. This should be mentioned somewhere.
>>
>> Not sure exactly which change you're recommending, but in retrospect,
>> I think it would be better to name the cap in term of how it changes
>> the gallium API instead of the GLES extension it enables. How about
>> PIPE_CAP_SURFACE_SAMPLE_COUNT, to indicate that it allows overriding
>> sample counts in pipe_surface?
>
>
> That's an interesting idea, but how does it convey that multisampled surfaces 
> are never multisampled in memory?

How are you going to convey all that in a cap enum name? In general,
the cap names are concise hints and not super descriptive - you have
to go read the documentation to understand the semantics.

Do you have a specific name you'd like to see or can we go with
PIPE_CAP_SURFACE_SAMPLE_COUNT?

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

[Mesa-dev] [Bug 108914] blocky shadow artifacts in The Forest with DXVK, RADV_DEBUG=nohiz fixes this

[bugzilla-daemon]

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

--- Comment #10 from tempel.jul...@gmail.com ---
Created attachment 142679
  --> https://bugs.freedesktop.org/attachment.cgi?id=142679=edit
The Forest renderdoc log part 4

-- 
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 v2 2/5] gallium: Add new PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE

[Marek Olšák]

On Fri, Nov 30, 2018 at 1:13 PM Kristian Høgsberg 
wrote:

> On Fri, Nov 16, 2018 at 7:48 PM Marek Olšák  wrote:
> >
> > I think the name PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE is slightly
> misleading, because it doesn't imply anything about the OpenGL ES behavior,
> which is that a texture is multisampled in the cache, but single-sampled in
> memory. This should be mentioned somewhere.
>
> Not sure exactly which change you're recommending, but in retrospect,
> I think it would be better to name the cap in term of how it changes
> the gallium API instead of the GLES extension it enables. How about
> PIPE_CAP_SURFACE_SAMPLE_COUNT, to indicate that it allows overriding
> sample counts in pipe_surface?
>

That's an interesting idea, but how does it convey that multisampled
surfaces are never multisampled in memory?

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

Re: [Mesa-dev] [PATCH v2 2/5] gallium: Add new PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE

[Kristian Høgsberg]

On Fri, Nov 16, 2018 at 7:48 PM Marek Olšák  wrote:
>
> I think the name PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE is slightly 
> misleading, because it doesn't imply anything about the OpenGL ES behavior, 
> which is that a texture is multisampled in the cache, but single-sampled in 
> memory. This should be mentioned somewhere.

Not sure exactly which change you're recommending, but in retrospect,
I think it would be better to name the cap in term of how it changes
the gallium API instead of the GLES extension it enables. How about
PIPE_CAP_SURFACE_SAMPLE_COUNT, to indicate that it allows overriding
sample counts in pipe_surface?

>
> Marek
>
>
> On Tue, Nov 6, 2018 at 5:19 PM Kristian H. Kristensen  
> wrote:
>>
>> This new pipe cap and the new nr_samples field in pipe_surface lets a
>> state tracker bind a render target with a different sample count than
>> the resource. This allows for implementing
>> EXT_multisampled_render_to_texture and
>> EXT_multisampled_render_to_texture2.
>>
>> Signed-off-by: Kristian H. Kristensen 
>> ---
>>  src/gallium/auxiliary/util/u_framebuffer.c | 10 --
>>  src/gallium/docs/source/screen.rst |  3 +++
>>  src/gallium/include/pipe/p_defines.h   |  1 +
>>  src/gallium/include/pipe/p_state.h |  6 ++
>>  4 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/util/u_framebuffer.c 
>> b/src/gallium/auxiliary/util/u_framebuffer.c
>> index 5bafddc726f..127623a7677 100644
>> --- a/src/gallium/auxiliary/util/u_framebuffer.c
>> +++ b/src/gallium/auxiliary/util/u_framebuffer.c
>> @@ -229,13 +229,19 @@ util_framebuffer_get_num_samples(const struct 
>> pipe_framebuffer_state *fb)
>> if (!(fb->nr_cbufs || fb->zsbuf))
>>return MAX2(fb->samples, 1);
>>
>> +   /**
>> +* If a driver doesn't advertise PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE,
>> +* pipe_surface::nr_samples will always be 0.
>> +*/
>> for (i = 0; i < fb->nr_cbufs; i++) {
>>if (fb->cbufs[i]) {
>> - return MAX2(1, fb->cbufs[i]->texture->nr_samples);
>> + return MAX3(1, fb->cbufs[i]->texture->nr_samples,
>> + fb->cbufs[i]->nr_samples);
>>}
>> }
>> if (fb->zsbuf) {
>> -  return MAX2(1, fb->zsbuf->texture->nr_samples);
>> +  return MAX3(1, fb->zsbuf->texture->nr_samples,
>> +  fb->zsbuf->nr_samples);
>> }
>>
>> return 1;
>> diff --git a/src/gallium/docs/source/screen.rst 
>> b/src/gallium/docs/source/screen.rst
>> index 0abd164494c..2a062a7027c 100644
>> --- a/src/gallium/docs/source/screen.rst
>> +++ b/src/gallium/docs/source/screen.rst
>> @@ -477,6 +477,9 @@ subpixel precision bias in bits during conservative 
>> rasterization.
>>0 means no limit.
>>  * ``PIPE_CAP_MAX_VERTEX_ELEMENT_SRC_OFFSET``: The maximum supported value 
>> for
>>of pipe_vertex_element::src_offset.
>> +* ``PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE``: Whether the driver
>> +  supports pipe_surface overrides of resource nr_samples. If set, will
>> +  enable EXT_multisampled_render_to_texture.
>>
>>  .. _pipe_capf:
>>
>> diff --git a/src/gallium/include/pipe/p_defines.h 
>> b/src/gallium/include/pipe/p_defines.h
>> index dacedf5b936..0ecfaf3ba5e 100644
>> --- a/src/gallium/include/pipe/p_defines.h
>> +++ b/src/gallium/include/pipe/p_defines.h
>> @@ -823,6 +823,7 @@ enum pipe_cap
>> PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS,
>> PIPE_CAP_MAX_TEXTURE_UPLOAD_MEMORY_BUDGET,
>> PIPE_CAP_MAX_VERTEX_ELEMENT_SRC_OFFSET,
>> +   PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE,
>>  };
>>
>>  /**
>> diff --git a/src/gallium/include/pipe/p_state.h 
>> b/src/gallium/include/pipe/p_state.h
>> index fd670345aad..89cffb15bd8 100644
>> --- a/src/gallium/include/pipe/p_state.h
>> +++ b/src/gallium/include/pipe/p_state.h
>> @@ -443,6 +443,12 @@ struct pipe_surface
>> uint16_t width;   /**< logical width in pixels */
>> uint16_t height;  /**< logical height in pixels */
>>
>> +   /** Number of samples for the surface.  This can be different from the
>> +* resource nr_samples when the resource is bound using
>> +* FramebufferTexture2DMultisampleEXT.
>> +*/
>> +   unsigned nr_samples:8;
>> +
>> union pipe_surface_desc u;
>>  };
>>
>> --
>> 2.19.1.930.g4563a0d9d0-goog
>>
>> ___
>> 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 108914] blocky shadow artifacts in The Forest with DXVK, RADV_DEBUG=nohiz fixes this

[bugzilla-daemon]

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

--- Comment #9 from tempel.jul...@gmail.com ---
Created attachment 142676
  --> https://bugs.freedesktop.org/attachment.cgi?id=142676=edit
The Forest renderdoc log part 3

-- 
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 v2 2/5] gallium: Add new PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE

[Kristian Høgsberg]

On Tue, Nov 6, 2018 at 6:26 PM Roland Scheidegger  wrote:
>
> Am 07.11.18 um 00:03 schrieb Kristian Høgsberg:
> > On Tue, Nov 6, 2018 at 2:44 PM Axel Davy  wrote:
> >>
> >> Hi,
> >>
> >> Is there anything to be done in the nine state trackers (or other state
> >> trackers).
> >>
> >> Nine uses create_surface. Should it expect the field to be filled
> >> properly by the driver ?
> >
> > Nothing is required from any state tracker, but if your API has an
> > extension like EXT_multisampled_render_to_texture, and the driver has
> > this new capability, you can set pipe_surface::nr_samples. The driver
> > will then render with that many samples internally and transparently
> > resolve the rendering to the (non-MSAA) resource.
> >
> >> On 06/11/2018 23:09, Kristian H. Kristensen wrote:
> >>> +   /**
> >>> +* If a driver doesn't advertise 
> >>> PIPE_CAP_MULTISAMPLED_RENDER_TO_TEXTURE,
> >>> +* pipe_surface::nr_samples will always be 0.
> >>> +*/
> >> The above comment should be added to the comment below.
> >>> +   /** Number of samples for the surface.  This can be different from the
> >>> +* resource nr_samples when the resource is bound using
> >>> +* FramebufferTexture2DMultisampleEXT.
> >>> +*/
> >>> +   unsigned nr_samples:8;
> >
> > Hm, I probably need to reword that a bit, since it implies the surface
> > sample count can be same as the resource, when it is only ever either
> > surface samples = 0 or surface samples > 1 with resource samples = 1.
> Wouldn't it be more logical if you rather adjust the code to match the
> comment? That is, the surface would inherit the sample count of the
> resource by default, but can be set to something different for this
> extension.

Yeah, that should work. I wanted to avoid rewriting every gallium
driver to look at surf->nr_samples instead of
surf->texture->nr_samples, but since they're the same without the new
cap, that shouldn't be necessary.

>
> > Kristian
> >
> >>> +
> >>>  union pipe_surface_desc u;
> >>>   };
> >>>
> >>
> >>
> >> Yours,
> >>
> >>
> >> Axel Davy
> >>
> >> ___
> >> mesa-dev mailing list
> >> mesa-dev@lists.freedesktop.org
> >> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-devdata=02%7C01%7Csroland%40vmware.com%7Cbd68e613af17447b3eae08d6443c12d3%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636771422156943417sdata=pji9JMcMB0DQyRIzske1nXJyCpneZ4RxITU9ov2A92o%3Dreserved=0
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-devdata=02%7C01%7Csroland%40vmware.com%7Cbd68e613af17447b3eae08d6443c12d3%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636771422156943417sdata=pji9JMcMB0DQyRIzske1nXJyCpneZ4RxITU9ov2A92o%3Dreserved=0
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 108914] blocky shadow artifacts in The Forest with DXVK, RADV_DEBUG=nohiz fixes this

[bugzilla-daemon]

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

--- Comment #8 from tempel.jul...@gmail.com ---
Created attachment 142675
  --> https://bugs.freedesktop.org/attachment.cgi?id=142675=edit
The Forest renderdoc log part 2

-- 
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 108914] blocky shadow artifacts in The Forest with DXVK, RADV_DEBUG=nohiz fixes this

[bugzilla-daemon]

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

--- Comment #7 from tempel.jul...@gmail.com ---
Created attachment 142674
  --> https://bugs.freedesktop.org/attachment.cgi?id=142674=edit
The Forest renderdoc log part 1

-- 
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] docs: Document optional GitLab code review process

[Jason Ekstrand]

On Fri, Nov 30, 2018 at 11:27 AM Jason Ekstrand 
wrote:

> On Fri, Nov 30, 2018 at 10:55 AM Michel Dänzer  wrote:
>
>> On 2018-11-30 4:57 p.m., Daniel Stone wrote:
>> > On Wed, 28 Nov 2018 at 17:23, Dylan Baker  wrote:
>> >
>> >> Personally speaking, I think that better next steps for gitlab
>> integration are:
>> >> - migrate from bugzilla to gitlab issues
>> >
>> > This is currently held up by a mutual death grip: both AMD and Intel
>> > want to be able to move or reassign issues between kernel / Mesa / X11
>> > within Bugzilla, so have requested that nothing moves until everything
>> > moves. I don't know whether that has to be one big flag day or whether
>> > the driver teams would find it acceptable for all three components to
>> > have a documented plan with a timeline on it. Intel also have some
>> > pretty heavy scripting and tooling around Bugzilla which would need to
>> > be modified to track GitLab instead.
>> >
>> > From an infrastructure point of view though, Bugzilla is getting less
>> > and less acceptable to run as a service.
>>
>> Well, getting rid of Bugzilla entirely requires migrating all remaining
>> bugs somewhere else anyway. :) As long as kernel / Mesa /
>> xf86-video-amdgpu/ati bugs are all migrated to GitLab issues, that
>> should be fine from an AMD perspective FWIW.
>>
>
> The ability to cross-link and migrate to/from mesa has been somewhat
> useful for us.  However, I don't know that it's really *that* useful when
> you can easily create a GitLab issue and give a link to BZ or copy the bug
> text or something.  Also, given that GitLab doesn't really let you migrate
> issues (you can link them), I don't know that we're loosing much that we
> wouldn't loose anyway.  I think we have far too much ado about a fairly
> small thing.
>

Also, I should say that the Intel Mesa team has zero tooling around
bugzilla so if someone is going to move first, it makes sense for it to be
Mesa.

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

Re: [Mesa-dev] [PATCH] docs: Document optional GitLab code review process

[Jason Ekstrand]

On Fri, Nov 30, 2018 at 10:55 AM Michel Dänzer  wrote:

> On 2018-11-30 4:57 p.m., Daniel Stone wrote:
> > On Wed, 28 Nov 2018 at 17:23, Dylan Baker  wrote:
> >
> >> Personally speaking, I think that better next steps for gitlab
> integration are:
> >> - migrate from bugzilla to gitlab issues
> >
> > This is currently held up by a mutual death grip: both AMD and Intel
> > want to be able to move or reassign issues between kernel / Mesa / X11
> > within Bugzilla, so have requested that nothing moves until everything
> > moves. I don't know whether that has to be one big flag day or whether
> > the driver teams would find it acceptable for all three components to
> > have a documented plan with a timeline on it. Intel also have some
> > pretty heavy scripting and tooling around Bugzilla which would need to
> > be modified to track GitLab instead.
> >
> > From an infrastructure point of view though, Bugzilla is getting less
> > and less acceptable to run as a service.
>
> Well, getting rid of Bugzilla entirely requires migrating all remaining
> bugs somewhere else anyway. :) As long as kernel / Mesa /
> xf86-video-amdgpu/ati bugs are all migrated to GitLab issues, that
> should be fine from an AMD perspective FWIW.
>

The ability to cross-link and migrate to/from mesa has been somewhat useful
for us.  However, I don't know that it's really *that* useful when you can
easily create a GitLab issue and give a link to BZ or copy the bug text or
something.  Also, given that GitLab doesn't really let you migrate issues
(you can link them), I don't know that we're loosing much that we wouldn't
loose anyway.  I think we have far too much ado about a fairly small thing.

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

[Mesa-dev] [Bug 108914] blocky shadow artifacts in The Forest with DXVK, RADV_DEBUG=nohiz fixes this

[bugzilla-daemon]

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

--- Comment #6 from tempel.jul...@gmail.com ---
Thanks, found it. Compressing and uploading it right now.

-- 
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] docs: Document optional GitLab code review process

[Michel Dänzer]

On 2018-11-30 4:57 p.m., Daniel Stone wrote:
> On Wed, 28 Nov 2018 at 17:23, Dylan Baker  wrote:
> 
>> Personally speaking, I think that better next steps for gitlab integration 
>> are:
>> - migrate from bugzilla to gitlab issues
> 
> This is currently held up by a mutual death grip: both AMD and Intel
> want to be able to move or reassign issues between kernel / Mesa / X11
> within Bugzilla, so have requested that nothing moves until everything
> moves. I don't know whether that has to be one big flag day or whether
> the driver teams would find it acceptable for all three components to
> have a documented plan with a timeline on it. Intel also have some
> pretty heavy scripting and tooling around Bugzilla which would need to
> be modified to track GitLab instead.
> 
> From an infrastructure point of view though, Bugzilla is getting less
> and less acceptable to run as a service.

Well, getting rid of Bugzilla entirely requires migrating all remaining
bugs somewhere else anyway. :) As long as kernel / Mesa /
xf86-video-amdgpu/ati bugs are all migrated to GitLab issues, that
should be fine from an AMD perspective FWIW.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [Bug 108914] blocky shadow artifacts in The Forest with DXVK, RADV_DEBUG=nohiz fixes this

[bugzilla-daemon]

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

Danylo  changed:

   What|Removed |Added

 CC||danylo.pilia...@gmail.com

--- Comment #5 from Danylo  ---
Should be in /tmp/RenderDoc/

-- 
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 108914] blocky shadow artifacts in The Forest with DXVK, RADV_DEBUG=nohiz fixes this

[bugzilla-daemon]

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

--- Comment #4 from tempel.jul...@gmail.com ---
I launched "renderdoccmd capture wine64 TheForest.exe" and saved a capture via
button. Stupid question now: Where is 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] docs: Document and *require* usage of Signed-off-by

[Ian Romanick]

On 11/29/2018 03:53 PM, Eric Anholt wrote:
> e<#secure method=pgpmime mode=sign>
> Erik Faye-Lund  writes:
> 
>> On Wed, 2018-11-28 at 13:43 -0800, Eric Anholt wrote:
>>> Jordan Justen  writes:
>>>
 This adds the "Developer's Certificate of Origin 1.1" from the
 Linux
 kernel. It indicates that by using Signed-off-by you are certifying
 that your patch meets the DCO 1.1 guidelines.

 It also changes Signed-off-by from being optional to being
 required.

 Signed-off-by: Jordan Justen 
 Cc: Matt Turner 
>>>
>>> What problem for our project is solved by signed-off-by?  Do you
>>> think
>>> that it has actually reduced the incidence of people submitting code
>>> they don't have permission to submit in the projects where it's been
>>> used?  I know the behavior in the kernel is that people submit a
>>> patch,
>>> it's missing s-o-b, so it gets bounced, then they maybe add s-o-b or
>>> just give up.  I don't think anyone stops and says "Wow, that's a
>>> good
>>> question.  Maybe I don't have permission to distribute this after
>>> all?"
>>>
>>> /me sees s-o-b as basically just a linux kernel hazing ritual
>>
>> I don't think that's the purpose of s-o-b in the Kernel. AFAIK, the
>> reason for the s-o-b is to have a paper-trail for how a patch landed in
>> the kernel; who it went through etc.
> 
> I get the *idea*, I just believe the idea fails in practice.  The S-O-B
> idea came from "wouldn't it be nice if we could make everyone think
> about this issue that is important to us" but what we actually got
> instead is "your patch gets bounced if you don't have a s-o-b on until
> you slap one on and resend."

You're thinking like an engineer, but the s-o-b is the spawn of lawyers.
 Supposedly, having someone say "I had the rights to contribute this
code," even if they didn't think about what that meant, enables you to
pass that blame along because that person showed intent.  You may not
have read and thought about every page of your mortgage, but you can
still be held accountable for every word.  Having an author tag or a
committer tag does not show that same intent.  In a legal context, the
s-o-b shifts the onus of auditing code ownership from the distributor to
the submitter.  When a distributor, like IBM in the SCO case, get sued,
they can plausibly claim that all of the code they distributed was
vouched for.  If someone submits code that they do not have the right to
submit and provides false testimony, then that's on the submitter.

S-o-b for that purpose always seemed pretty lame to me (from any
perspective) because patches aren't digitally signed.  Anyone can add
any s-o-b to any patch, spoof the sending address, etc.

The s-o-b is also mostly an anachronism of taking patches via mailing
list.  Projects that exclusively use GitHub or similar have everyone
sign an SLA, and every patch that comes in through the submission
mechanism, guarded by a login, is implicitly signed.

> We've already got 1-2 people to contact if there's a question about
> authorship, from the committer and author fields.

That's one thing that annoys me from time to time about git.  Five
people may have worked on a patch, but it only shows one author.

> ___
> 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 5/5] intel/tools: make sure the binary file is properly read

[andrey simiklit]

On Fri, Nov 30, 2018 at 5:49 PM Eric Engestrom 
wrote:

> On Friday, 2018-11-30 15:47:11 +, Lionel Landwerlin wrote:
> > On 14/11/2018 16:30, asimiklit.w...@gmail.com wrote:
> > > From: Andrii Simiklit 
> > >
> > > 1. tools/i965_disasm.c:58:4: warning:
> > >   ignoring return value of ‘fread’,
> > >   declared with attribute warn_unused_result
> > >   fread(assembly, *end, 1, fp);
> > >
> > > v2: Fixed incorrect return value check.
> > > ( Eric Engestrom  )
> > >
> > > v3: Zero size file check placed before fread with exit()
> > > ( Eric Engestrom  )
> > >
> > > v4: - Title is changed.
> > >  - The 'size' variable was moved to top of a function scope.
> > >  - The assertion was replaced by the proper error handling.
> > >  - The error message on a caller side was fixed.
> > > ( Eric Engestrom  )
> > >
> > > Signed-off-by: Andrii Simiklit 
> >
> >
> > With the nit below :
> >
> >
> > Reviewed-by: Lionel Landwerlin 
>
> I'll change that as I push it in a minute :)
> Reviewed-by: Eric Engestrom 
>

Thanks a lot for reviews :)


>
> >
> >
> > > ---
> > >   src/intel/tools/i965_disasm.c | 16 +---
> > >   1 file changed, 13 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/src/intel/tools/i965_disasm.c
> b/src/intel/tools/i965_disasm.c
> > > index 73a6760fc1..0efbdab706 100644
> > > --- a/src/intel/tools/i965_disasm.c
> > > +++ b/src/intel/tools/i965_disasm.c
> > > @@ -47,17 +47,23 @@ i965_disasm_get_file_size(FILE *fp)
> > >   static void *
> > >   i965_disasm_read_binary(FILE *fp, size_t *end)
> > >   {
> > > +   size_t size;
> > >  void *assembly;
> > >  *end = i965_disasm_get_file_size(fp);
> > > +   if (!*end)
> > > +  return NULL;
> > >  assembly = malloc(*end + 1);
> > >  if (assembly == NULL)
> > > return NULL;
> > > -   fread(assembly, *end, 1, fp);
> > > +   size = fread(assembly, *end, 1, fp);
> > >  fclose(fp);
> > > -
> > > +   if (!size) {
> > > +  free(assembly);
> > > +  return NULL;
> > > +   }
> > >  return assembly;
> > >   }
> > > @@ -167,7 +173,11 @@ int main(int argc, char *argv[])
> > >  assembly = i965_disasm_read_binary(fp, );
> > >  if (!assembly) {
> > > -  fprintf(stderr, "Unable to allocate buffer to read binary
> file\n");
> > > +  if(end)
> > if (end)
> > > +fprintf(stderr, "Unable to allocate buffer to read binary
> file\n");
> > > +  else
> > > +fprintf(stderr, "Input file is empty\n");
> > > +
> > > exit(EXIT_FAILURE);
> > >  }
> >
> >
> ___
> 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] docs: Document optional GitLab code review process

[Daniel Stone]

Hi all,
Thanks for the CC. I'm on a sabbatical until mid-January; I'll be
around but not following the lists/etc as actively as before. Please
feel free to liberally CC me (on this address, not work) or poke me on
IRC if there's something I should see or could contribute to. I'll
have limited time for the next week or so, but should be able to do
more on it next month, albeit in Australian timezone.

On Wed, 28 Nov 2018 at 17:23, Dylan Baker  wrote:
> Quoting Matt Turner (2018-11-27 19:20:09)
> > Discussion point: I think attempting to have simultaneous review in
> > two places is a recipe for wasted time. Strawman: maybe we can only
> > email the cover letter to the mailing list and include in it a link to
> > the MR?
>
> I think it's a *really* bad idea to allow both. Some people will immediately
> begin using MR/PR's only and never read the list, others will never check
> MR/PRs and only use the list. It'll leave the rest of us forced to use both. 
> We
> should either go all in and archive the mailing list and use only gitlab, or 
> not
> use PR/MR's IMHO.

I fully agree, for all the reasons listed here. It imposes a much
higher overhead on contributors, and means that reviewers effectively
need to work their way through both interfaces anyway. Since you don't
get stable links until you push, in order to link between the two, you
would need to submit a MR to GitLab, capture the URL, include that in
your cover letter / patches, git-send-email, then capture the message
ID / archive link and go back to GitLab to edit your cover letter and
insert the link. Not to mention that it would basically destroy the
usefulness of Patchwork overnight.

Weston has a pretty conservative development workflow, but even then
we've had near-zero pushback, and saw a pretty big leap in
contributions from people who have been sitting on patchsets for a
long time but not bothered to submit them when we had review on the
list. I was genuinely surprised at how big the uptick was.

> > > +  Never use the merge button on the GitLab page to push patches
> >
> > Can we disable this in Gitlab? If the button is there, people *will*
> > accidentally press it.
>
> I think Daniel was working on getting a "rebase and push" button like github 
> has
> so that we could use the button, CC'd.

This is already there, and can be configured per-repo. Under settings
-> merge requests, choose this as your merge strategy:
'Fast-forward merge
No merge commits are created and all merges are fast-forwarded, which
means that merging is only allowed if the branch could be
fast-forwarded.
When fast-forward merge is not possible, the user is given the option
to rebase.'

If the user selects the 'allow commits from members who can merge to
the target branch' checkbox when submitting the MR, reviewers can
themselves request a rebase. We have FF-only MRs in Weston, and when I
want to land a contribution after review, I hit rebase followed by
'merge after pipeline succeeds', which does what it says on the box,
i.e. waits until CI has finished and then automatically lands it.

There are a couple of rough edges here. First, users have to
explicitly select the checkbox for this to happen; an upstream feature
request to make this the default or even mandatory is at
https://gitlab.com/gitlab-org/gitlab-ce/issues/49323. Secondly, if
something else lands in the target branch between the rebase starting
and CI finishing (i.e. it's no longer an FF landing by the time CI
reports success), the reviewer has to request the rebase again. Jason
was discussing having GitLab busy-loop, running rebase+CI in a loop
until it could actually land. I don't have a link to hand for this
though.

> > I'm not sure if it's a custom thing or what (I can find out) but I'd
> > much prefer to automate things if we can. Just like patchwork, people
> > *will* forget to close merge requests if it's possible. (And people
> > currently *do* forget to close bugzilla bugs)
>
> Or we could just use gitlab's issue tracker, which will automatically close
> issues when a commit pushed to master has a tag like `Fixes: #1234` or
> `Closes: #1234`.

Right, and obviously if you land a MR through the web UI then it's
automatically closed.

> Personally speaking, I think that better next steps for gitlab integration 
> are:
> - migrate from bugzilla to gitlab issues

This is currently held up by a mutual death grip: both AMD and Intel
want to be able to move or reassign issues between kernel / Mesa / X11
within Bugzilla, so have requested that nothing moves until everything
moves. I don't know whether that has to be one big flag day or whether
the driver teams would find it acceptable for all three components to
have a documented plan with a timeline on it. Intel also have some
pretty heavy scripting and tooling around Bugzilla which would need to
be modified to track GitLab instead.

From an infrastructure point of view though, Bugzilla is getting less
and less acceptable to run as a service. It's no 

Re: [Mesa-dev] [PATCH v4 5/5] intel/tools: make sure the binary file is properly read

[Eric Engestrom]

On Friday, 2018-11-30 15:47:11 +, Lionel Landwerlin wrote:
> On 14/11/2018 16:30, asimiklit.w...@gmail.com wrote:
> > From: Andrii Simiklit 
> > 
> > 1. tools/i965_disasm.c:58:4: warning:
> >   ignoring return value of ‘fread’,
> >   declared with attribute warn_unused_result
> >   fread(assembly, *end, 1, fp);
> > 
> > v2: Fixed incorrect return value check.
> > ( Eric Engestrom  )
> > 
> > v3: Zero size file check placed before fread with exit()
> > ( Eric Engestrom  )
> > 
> > v4: - Title is changed.
> >  - The 'size' variable was moved to top of a function scope.
> >  - The assertion was replaced by the proper error handling.
> >  - The error message on a caller side was fixed.
> > ( Eric Engestrom  )
> > 
> > Signed-off-by: Andrii Simiklit 
> 
> 
> With the nit below :
> 
> 
> Reviewed-by: Lionel Landwerlin 

I'll change that as I push it in a minute :)
Reviewed-by: Eric Engestrom 

> 
> 
> > ---
> >   src/intel/tools/i965_disasm.c | 16 +---
> >   1 file changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c
> > index 73a6760fc1..0efbdab706 100644
> > --- a/src/intel/tools/i965_disasm.c
> > +++ b/src/intel/tools/i965_disasm.c
> > @@ -47,17 +47,23 @@ i965_disasm_get_file_size(FILE *fp)
> >   static void *
> >   i965_disasm_read_binary(FILE *fp, size_t *end)
> >   {
> > +   size_t size;
> >  void *assembly;
> >  *end = i965_disasm_get_file_size(fp);
> > +   if (!*end)
> > +  return NULL;
> >  assembly = malloc(*end + 1);
> >  if (assembly == NULL)
> > return NULL;
> > -   fread(assembly, *end, 1, fp);
> > +   size = fread(assembly, *end, 1, fp);
> >  fclose(fp);
> > -
> > +   if (!size) {
> > +  free(assembly);
> > +  return NULL;
> > +   }
> >  return assembly;
> >   }
> > @@ -167,7 +173,11 @@ int main(int argc, char *argv[])
> >  assembly = i965_disasm_read_binary(fp, );
> >  if (!assembly) {
> > -  fprintf(stderr, "Unable to allocate buffer to read binary file\n");
> > +  if(end)
> if (end)
> > +fprintf(stderr, "Unable to allocate buffer to read binary file\n");
> > +  else
> > +fprintf(stderr, "Input file is empty\n");
> > +
> > exit(EXIT_FAILURE);
> >  }
> 
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v4 5/5] intel/tools: make sure the binary file is properly read

[Lionel Landwerlin]

On 14/11/2018 16:30, asimiklit.w...@gmail.com wrote:

From: Andrii Simiklit 

1. tools/i965_disasm.c:58:4: warning:
  ignoring return value of ‘fread’,
  declared with attribute warn_unused_result
  fread(assembly, *end, 1, fp);

v2: Fixed incorrect return value check.
( Eric Engestrom  )

v3: Zero size file check placed before fread with exit()
( Eric Engestrom  )

v4: - Title is changed.
 - The 'size' variable was moved to top of a function scope.
 - The assertion was replaced by the proper error handling.
 - The error message on a caller side was fixed.
( Eric Engestrom  )

Signed-off-by: Andrii Simiklit 



With the nit below :


Reviewed-by: Lionel Landwerlin 



---
  src/intel/tools/i965_disasm.c | 16 +---
  1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c
index 73a6760fc1..0efbdab706 100644
--- a/src/intel/tools/i965_disasm.c
+++ b/src/intel/tools/i965_disasm.c
@@ -47,17 +47,23 @@ i965_disasm_get_file_size(FILE *fp)
  static void *
  i965_disasm_read_binary(FILE *fp, size_t *end)
  {
+   size_t size;
 void *assembly;
  
 *end = i965_disasm_get_file_size(fp);

+   if (!*end)
+  return NULL;
  
 assembly = malloc(*end + 1);

 if (assembly == NULL)
return NULL;
  
-   fread(assembly, *end, 1, fp);

+   size = fread(assembly, *end, 1, fp);
 fclose(fp);
-
+   if (!size) {
+  free(assembly);
+  return NULL;
+   }
 return assembly;
  }
  
@@ -167,7 +173,11 @@ int main(int argc, char *argv[])
  
 assembly = i965_disasm_read_binary(fp, );

 if (!assembly) {
-  fprintf(stderr, "Unable to allocate buffer to read binary file\n");
+  if(end)

if (end)

+fprintf(stderr, "Unable to allocate buffer to read binary file\n");
+  else
+fprintf(stderr, "Input file is empty\n");
+
exit(EXIT_FAILURE);
 }
  



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

[Mesa-dev] [Bug 108914] blocky shadow artifacts in The Forest with DXVK, RADV_DEBUG=nohiz fixes this

[bugzilla-daemon]

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

--- Comment #3 from tempel.jul...@gmail.com ---
Thanks for your quick reply. I can definitely try this, but since I'm not
experienced in that, I can't promise to do it before the holidays.

-- 
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 108914] blocky shadow artifacts in The Forest with DXVK, RADV_DEBUG=nohiz fixes this

[bugzilla-daemon]

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

--- Comment #2 from Samuel Pitoiset  ---
Thanks for the report. Can you try to record a renderdoc capture please?

-- 
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 108914] blocky shadow artifacts in The Forest with DXVK, RADV_DEBUG=nohiz fixes this

[bugzilla-daemon]

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

--- Comment #1 from tempel.jul...@gmail.com ---
Forgot to add the used versions:
mesa-git 19.0.0_devel.105999.89b4798c06
llvm-svn 8.0.0svn_r347844

But happens also with latest stable versions.

-- 
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 108914] blocky shadow artifacts in The Forest with DXVK, RADV_DEBUG=nohiz fixes this

[bugzilla-daemon]

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

Bug ID: 108914
   Summary: blocky shadow artifacts in The Forest with DXVK,
RADV_DEBUG=nohiz fixes this
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Vulkan/radeon
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: tempel.jul...@gmail.com
QA Contact: mesa-dev@lists.freedesktop.org

Created attachment 142665
  --> https://bugs.freedesktop.org/attachment.cgi?id=142665=edit
screenshot showing the artifacts with medium shadow details

Shadows in the game The Forest show blocky artifacts, especially at lower
detail settings like "medium".
The corruption disappears when using the RADV_DEBUG=nohiz variable. amdvlk
doesn't show this issue either.

The game should work by simply installing Steam into Wine and then install the
game normally inside Steam. Wine-Staging should be used to avoid some mouse
control issues.

-- 
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] miptree: test ability to use upside down miptree

[andrey simiklit]

Hello,

Could somebody take a look at this patch?

Thanks a lot,
Andrii.

On Tue, Nov 6, 2018 at 12:33 PM andrey simiklit 
wrote:

> Hi all,
>
> Have somebody some suggestions for this test?
> This patch is needed to test an issue (assertion in mesa) which will be
> fixed by this mesa patch:
> https://patchwork.freedesktop.org/patch/254397/
>
> Regards,
> Andrii.
>
> On Tue, Oct 23, 2018 at 4:44 PM  wrote:
>
>> From: Andrii Simiklit 
>>
>> Test that usage of upside down miptree doesn't cause assertion
>>
>> The miptree:
>>
>> level 0 = 1x1
>> level 1 = 2x2
>> level 2 = 4x4
>> ...
>> level n = NxN
>>
>> should be acceptable for case when we don't use a min filter.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107987
>> Signed-off-by: Andrii Simiklit 
>> ---
>>  tests/opengl.py   |   1 +
>>  tests/texturing/CMakeLists.gl.txt |   1 +
>>  tests/texturing/tex-upside-down-miptree.c | 171 ++
>>  3 files changed, 173 insertions(+)
>>  create mode 100644 tests/texturing/tex-upside-down-miptree.c
>>
>> diff --git a/tests/opengl.py b/tests/opengl.py
>> index f7e408cd5..f6a38e40e 100644
>> --- a/tests/opengl.py
>> +++ b/tests/opengl.py
>> @@ -704,6 +704,7 @@ with profile.test_list.group_manager(
>>  g(['getteximage-targets', '1D'])
>>  g(['getteximage-targets', '2D'])
>>  g(['teximage-scale-bias'])
>> +g(['tex-upside-down-miptree'])
>>  add_msaa_visual_plain_tests(g, ['draw-pixels'])
>>  add_msaa_visual_plain_tests(g, ['read-front'], run_concurrent=False)
>>  add_msaa_visual_plain_tests(g, ['read-front', 'clear-front-first'],
>> diff --git a/tests/texturing/CMakeLists.gl.txt
>> b/tests/texturing/CMakeLists.gl.txt
>> index e5d41e432..02b572c79 100644
>> --- a/tests/texturing/CMakeLists.gl.txt
>> +++ b/tests/texturing/CMakeLists.gl.txt
>> @@ -98,5 +98,6 @@ piglit_add_executable (texture-al texture-al.c)
>>  piglit_add_executable (texture-rg texture-rg.c)
>>  piglit_add_executable (teximage-colors teximage-colors.c)
>>  piglit_add_executable (zero-tex-coord zero-tex-coord.c)
>> +piglit_add_executable (tex-upside-down-miptree tex-upside-down-miptree.c)
>>
>>  # vim: ft=cmake:
>> diff --git a/tests/texturing/tex-upside-down-miptree.c
>> b/tests/texturing/tex-upside-down-miptree.c
>> new file mode 100644
>> index 0..2d10fb49a
>> --- /dev/null
>> +++ b/tests/texturing/tex-upside-down-miptree.c
>> @@ -0,0 +1,171 @@
>> +/*
>> + * Copyright (c) 2018 Andrii Simiklit
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining
>> a
>> + * copy of this software and associated documentation files (the
>> "Software"),
>> + * to deal in the Software without restriction, including without
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the
>> next
>> + * paragraph) shall be included in all copies or substantial portions of
>> the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Authors:
>> + *Andrii Simiklit 
>> + *
>> + */
>> +
>> +/**
>> + * Test what there no an assertion when we use upside down miptree and
>> + * GL_TEXTURE_MIN_FILTER is GL_LINEAR, base level is not 0
>> + * Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107987
>> + */
>> +
>> +#include "piglit-util-gl.h"
>> +#define TW 64
>> +#define TH 64
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +
>> +   config.supports_gl_compat_version = 10;
>> +
>> +   config.window_visual = PIGLIT_GL_VISUAL_RGBA |
>> PIGLIT_GL_VISUAL_DOUBLE;
>> +   config.khr_no_error_support = PIGLIT_NO_ERRORS;
>> +
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +static unsigned nlevels = 0;
>> +
>> +static void
>> +get_rect_bounds(int pos, int *x, int *y, int *w, int *h)
>> +{
>> +   *x = pos * (piglit_width / 3) + 5;
>> +   *y = 5;
>> +   *w = piglit_width / 3 - 10;
>> +   *h = piglit_height - 10;
>> +}
>> +
>> +
>> +static void
>> +draw_rect(int pos)
>> +{
>> +   int x, y, w, h;
>> +   get_rect_bounds(pos, , , , );
>> +   piglit_draw_rect_tex(x, y, w, h, 0, 0, 1, 1);
>> +}
>> +
>> +
>> +static GLboolean
>> +probe_pos(int pos, const GLfloat expected[4])
>> +{
>> +   int x, y, w, h;
>> +   get_rect_bounds(pos, , , , );
>> +   return 

[Mesa-dev] [Bug 108900] Non-recoverable GPU hangs with GfxBench v5 Aztec Ruins Vulkan test

[bugzilla-daemon]

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

Eero Tamminen  changed:

   What|Removed |Added

   Priority|medium  |high
   Severity|normal  |critical
Summary|GPU hangs with GfxBench v5  |Non-recoverable GPU hangs
   |Aztec Ruins Vulkan test |with GfxBench v5 Aztec
   ||Ruins Vulkan test

--- Comment #1 from Eero Tamminen  ---
Yes, this messes also other things, not just 3D (after this issue, script using
pycurl to upload test results, will just sit in poll() instead of working, so I
think something on kernel side gets corrupted).

-- 
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 5/5] intel/tools: make sure the binary file is properly read

[andrey simiklit]

Hello,

Could you please take a look on this v4 if it is possible?

Thanks,
Andrii.

On Wed, Nov 14, 2018 at 6:30 PM  wrote:

> From: Andrii Simiklit 
>
> 1. tools/i965_disasm.c:58:4: warning:
>  ignoring return value of ‘fread’,
>  declared with attribute warn_unused_result
>  fread(assembly, *end, 1, fp);
>
> v2: Fixed incorrect return value check.
>( Eric Engestrom  )
>
> v3: Zero size file check placed before fread with exit()
>( Eric Engestrom  )
>
> v4: - Title is changed.
> - The 'size' variable was moved to top of a function scope.
> - The assertion was replaced by the proper error handling.
> - The error message on a caller side was fixed.
>( Eric Engestrom  )
>
> Signed-off-by: Andrii Simiklit 
> ---
>  src/intel/tools/i965_disasm.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/src/intel/tools/i965_disasm.c b/src/intel/tools/i965_disasm.c
> index 73a6760fc1..0efbdab706 100644
> --- a/src/intel/tools/i965_disasm.c
> +++ b/src/intel/tools/i965_disasm.c
> @@ -47,17 +47,23 @@ i965_disasm_get_file_size(FILE *fp)
>  static void *
>  i965_disasm_read_binary(FILE *fp, size_t *end)
>  {
> +   size_t size;
> void *assembly;
>
> *end = i965_disasm_get_file_size(fp);
> +   if (!*end)
> +  return NULL;
>
> assembly = malloc(*end + 1);
> if (assembly == NULL)
>return NULL;
>
> -   fread(assembly, *end, 1, fp);
> +   size = fread(assembly, *end, 1, fp);
> fclose(fp);
> -
> +   if (!size) {
> +  free(assembly);
> +  return NULL;
> +   }
> return assembly;
>  }
>
> @@ -167,7 +173,11 @@ int main(int argc, char *argv[])
>
> assembly = i965_disasm_read_binary(fp, );
> if (!assembly) {
> -  fprintf(stderr, "Unable to allocate buffer to read binary file\n");
> +  if(end)
> +fprintf(stderr, "Unable to allocate buffer to read binary
> file\n");
> +  else
> +fprintf(stderr, "Input file is empty\n");
> +

   exit(EXIT_FAILURE);
> }
>
> --
> 2.17.1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 00/29] do not allow invalid texture-format enums

[Francesco Ansanelli]

Il giorno ven 30 nov 2018, 12:26 Erik Faye-Lund <
erik.faye-l...@collabora.com> ha scritto:

> On Fri, 2018-11-23 at 11:53 +0100, Erik Faye-Lund wrote:
> > OK, so here's a v2 of this series. These are the changes since v1:
> > - Removed double-semicolons in patch #25
> > - Removed default-case and questionable comment in patch #25
> > - Dropped patch #30, as it would regress functionality on two drivers
> >
> > Please review :)
>
> Ping? Any takers?
>
I read all the patches and didn't spot anything wrong, but please don't
consider this a proper review.

>
> > Erik Faye-Lund (29):
> >   mesa/main: make _mesa_has_tessellation return bool
> >   mesa/main: rename format-check function
> >   mesa/main: clean up S3_s3tc check
> >   mesa/main: clean up OES_texture_float_linear check
> >   mesa/main: clean up ES2_compatibility check
> >   mesa/main: clean up integer texture check
> >   mesa/main: use _mesa_has_FOO_bar for compressed format checks
> >   mesa/main: do not allow s3tc enums on gles1
> >   mesa/main: do not allow etc2 enums on gles1
> >   mesa/main: do not allow astc enums on gles1
> >   mesa/main: do not allow depth-texture enums on gles1
> >   mesa/main: do not allow stencil-texture enums on gles1
> >   mesa/main: do not allow ARB_texture_rgb10_a2ui enums before gles3
> >   mesa/main: do not allow integer-texture enums before gles3
> >   mesa/main: do not allow ARB_depth_buffer_float enums before gles3
> >   mesa/main: do not allow EXT_packed_float enums before gles3
> >   mesa/main: do not allow rg-textures enums before gles3
> >   mesa/main: do not allow EXT_texture_shared_exponent enums before
> > gles3
> >   mesa/main: do not allow MESA_ycbcr_texture enums on gles
> >   mesa/main: do not allow type_2_10_10_10_REV enums before gles3
> >   mesa/main: do not allow floating-point texture enums on gles1
> >   mesa/main: do not allow snorm-texture enums before gles3
> >   mesa/main: do not allow sRGB texture enums before gles3
> >   mesa/main: do not allow EXT_texture_sRGB_R8 enums before gles3
> >   mesa/main: split float-texture support checking in two
> >   mesa/main: require EXT_texture_type_2_10_10_10_REV for gles3
> >   mesa/main: require EXT_texture_sRGB for gles3
> >   mesa/st: do not probe for the same texture-formats twice
> >   mesa/main: do not require float-texture filtering for es3
> >
> >  src/mesa/main/context.h|  59 ++-
> >  src/mesa/main/glformats.c  | 221 +
> > 
> >  src/mesa/main/glformats.h  |   6 +-
> >  src/mesa/main/teximage.c   |   4 +-
> >  src/mesa/main/version.c|   8 +-
> >  src/mesa/state_tracker/st_extensions.c |   8 +-
> >  6 files changed, 184 insertions(+), 122 deletions(-)
> >
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 00/29] do not allow invalid texture-format enums

[Erik Faye-Lund]

On Fri, 2018-11-23 at 11:53 +0100, Erik Faye-Lund wrote:
> OK, so here's a v2 of this series. These are the changes since v1:
> - Removed double-semicolons in patch #25
> - Removed default-case and questionable comment in patch #25
> - Dropped patch #30, as it would regress functionality on two drivers
> 
> Please review :)

Ping? Any takers?

> Erik Faye-Lund (29):
>   mesa/main: make _mesa_has_tessellation return bool
>   mesa/main: rename format-check function
>   mesa/main: clean up S3_s3tc check
>   mesa/main: clean up OES_texture_float_linear check
>   mesa/main: clean up ES2_compatibility check
>   mesa/main: clean up integer texture check
>   mesa/main: use _mesa_has_FOO_bar for compressed format checks
>   mesa/main: do not allow s3tc enums on gles1
>   mesa/main: do not allow etc2 enums on gles1
>   mesa/main: do not allow astc enums on gles1
>   mesa/main: do not allow depth-texture enums on gles1
>   mesa/main: do not allow stencil-texture enums on gles1
>   mesa/main: do not allow ARB_texture_rgb10_a2ui enums before gles3
>   mesa/main: do not allow integer-texture enums before gles3
>   mesa/main: do not allow ARB_depth_buffer_float enums before gles3
>   mesa/main: do not allow EXT_packed_float enums before gles3
>   mesa/main: do not allow rg-textures enums before gles3
>   mesa/main: do not allow EXT_texture_shared_exponent enums before
> gles3
>   mesa/main: do not allow MESA_ycbcr_texture enums on gles
>   mesa/main: do not allow type_2_10_10_10_REV enums before gles3
>   mesa/main: do not allow floating-point texture enums on gles1
>   mesa/main: do not allow snorm-texture enums before gles3
>   mesa/main: do not allow sRGB texture enums before gles3
>   mesa/main: do not allow EXT_texture_sRGB_R8 enums before gles3
>   mesa/main: split float-texture support checking in two
>   mesa/main: require EXT_texture_type_2_10_10_10_REV for gles3
>   mesa/main: require EXT_texture_sRGB for gles3
>   mesa/st: do not probe for the same texture-formats twice
>   mesa/main: do not require float-texture filtering for es3
> 
>  src/mesa/main/context.h|  59 ++-
>  src/mesa/main/glformats.c  | 221 +
> 
>  src/mesa/main/glformats.h  |   6 +-
>  src/mesa/main/teximage.c   |   4 +-
>  src/mesa/main/version.c|   8 +-
>  src/mesa/state_tracker/st_extensions.c |   8 +-
>  6 files changed, 184 insertions(+), 122 deletions(-)
> 

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

Re: [Mesa-dev] [PATCH 1/3] virgl: quadruple command buffer size

[Erik Faye-Lund]

On Thu, 2018-11-29 at 18:09 -0800, Gurchetan Singh wrote:
> Thanks!  Gentle ping for push from committers..

Pushed, thanks :)

> On Mon, Nov 26, 2018 at 11:52 AM Erik Faye-Lund
>  wrote:
> > On Mon, 2018-11-26 at 09:54 -0800, Gurchetan Singh wrote:
> > > Tested running WebGL aquarium on Nvidia host (10,000 fishes)
> > > 
> > > This moves us from 7 fps to 9 fps.  After quadrupling,
> > > performance
> > > gains diminish.
> > > 
> > > v2: Remove change ID (Erik)
> > > 
> > > Tested-By: Gert Wollny 
> > > 
> > 
> > For the series:
> > 
> > Reviewed-by: Erik Faye-Lund 
> > 
> ___
> 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] Lets talk about autotools

[Gert Wollny]

Hello all, 

Am Donnerstag, den 29.11.2018, 17:44 + schrieb Emil Velikov:
> Hi all,
> 
> I can see why people may opt to not use or maintain the autotools
> build. Although I would kindly ask that we do not remove it just yet.

I second that, I think the process of removing autotools should be a
two-step procedure. i.e. prior to ripping autotools support out there
should be one or two releases that deprecates it, e.g. by changing
configure that it (1) needs an extra flag to run, and (2) when run
without this flag it would just print a message about the meson build
system, the deprecation of autotools (making it clear when it will be
removed), and information how to still run autotools with this extra
flag. If qwe 

A rationale is that with a release that only has mesa there is a high
chance that people not directly involved with the project, and that
don't follow git but only use the releases hit corner cases when
building mesa that we or maintainers for distributions might not be
aware of. Still having something around that is known to work would be
good for them, so they can report a bug against the meson build system
and still get their work done by easily switching to the autotools for
the time being.

Best, 
Gert

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

Re: [Mesa-dev] [PATCH 0/2] debug feature to dump "get configs" and "chosen configs"

[Silvestrs Timofejevs]

On Fri, Nov 09, 2018 at 06:04:10PM +, Silvestrs Timofejevs wrote:
> This patch series provides an easy way to see what configs
> have been returned by the 'eglGetConfigs' and 'eglChooseConfig'
> functions, and give an overview of config attributes.
> 
> Silvestrs Timofejevs (2):
>   egl: introduce a log level getter function
>   egl: add config debug printout
> 
>  src/egl/Makefile.sources  |   4 +-
>  src/egl/main/eglconfig.c  |  20 ++-
>  src/egl/main/eglconfigdebug.c | 277 
> ++
>  src/egl/main/eglconfigdebug.h |  55 +
>  src/egl/main/egllog.c |   9 ++
>  src/egl/main/egllog.h |   4 +
>  src/egl/meson.build   |   2 +
>  7 files changed, 366 insertions(+), 5 deletions(-)
>  create mode 100644 src/egl/main/eglconfigdebug.c
>  create mode 100644 src/egl/main/eglconfigdebug.h
> 
> -- 
> 2.7.4
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Hi,

just wondering if anyone had a chance to have a look at this patch?

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

Re: [Mesa-dev] [PATCH] docs: Document and *require* usage of Signed-off-by

[Jordan Justen]

On 2018-11-29 15:53:09, Eric Anholt wrote:
> e<#secure method=pgpmime mode=sign>
> Erik Faye-Lund  writes:
> 
> > On Wed, 2018-11-28 at 13:43 -0800, Eric Anholt wrote:
> >> Jordan Justen  writes:
> >> 
> >> > This adds the "Developer's Certificate of Origin 1.1" from the
> >> > Linux
> >> > kernel. It indicates that by using Signed-off-by you are certifying
> >> > that your patch meets the DCO 1.1 guidelines.
> >> > 
> >> > It also changes Signed-off-by from being optional to being
> >> > required.
> >> > 
> >> > Signed-off-by: Jordan Justen 
> >> > Cc: Matt Turner 
> >> 
> >> What problem for our project is solved by signed-off-by?  Do you
> >> think
> >> that it has actually reduced the incidence of people submitting code
> >> they don't have permission to submit in the projects where it's been
> >> used?  I know the behavior in the kernel is that people submit a
> >> patch,
> >> it's missing s-o-b, so it gets bounced, then they maybe add s-o-b or
> >> just give up.  I don't think anyone stops and says "Wow, that's a
> >> good
> >> question.  Maybe I don't have permission to distribute this after
> >> all?"
> >> 
> >> /me sees s-o-b as basically just a linux kernel hazing ritual
> >
> > I don't think that's the purpose of s-o-b in the Kernel. AFAIK, the
> > reason for the s-o-b is to have a paper-trail for how a patch landed in
> > the kernel; who it went through etc.
> 
> I get the *idea*, I just believe the idea fails in practice.  The S-O-B
> idea came from "wouldn't it be nice if we could make everyone think
> about this issue that is important to us" but what we actually got
> instead is "your patch gets bounced if you don't have a s-o-b on until
> you slap one on and resend."

Yeah, I can see how that can happen in some cases. Hopefully the
feedback to the contributor would be, go read about patch signing in
docs/submittingpatches.html rather than just "add Signed-off-by".

It sounds like the biggest drawback is that some casual contributors
might be turned off from making the contribution because they need to
read the DCO and revise their patch. Could it be argued that such
contributions are not neccessarily worth the time then? If they treat
a code contribution this cavalierly, then maybe a bug report would be
more their speed?

> We've already got 1-2 people to contact if there's a question about
> authorship, from the committer and author fields.

There's also the missed opportunity for them to think about the
license issue. (Maybe not 100% will seriously think about it, but will
it be 0%?)

Sometimes I'm shocked about how many projects I find on github with
absolutely no license in sight. It's like, oh that's cool, but I guess
it's not usable by anyone for anything. :\ So, maybe it's not so bad
to add this if we might also be inviting in the pull/merge request
crowd? :)

It seems like you are annoyed, or at least skeptical about this. Is
that a "NAK", or "why?", or "alright, but it's a waste of time"?

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

Re: [Mesa-dev] Lets talk about autotools

[Pekka Paalanen]

On Thu, 29 Nov 2018 17:44:47 +
Emil Velikov  wrote:

> Hi all,
> 
> I can see why people may opt to not use or maintain the autotools build.
> Although I would kindly ask that we do not remove it just yet.
> 
> In Mesa, we have different parts not used by different teams. As such
> we tend to remove stuff when nobody is around to maintain it anymore.
> 
> That said, I'm planning to continue maintaining it and would appreciate
> if we keep it in-tree.
> 
> As people may be concerned about bugreports and alike we can trivially
> add a warning (as configure is invoked) to forwards any issues to my
> email. Additionally (or alternatively) we can have an autotools bugzilla
> category with me as the default assignee.
> 
> What do you guys think?

Hi Emil,

could you explain your motivation behind keeping the autotools build
around? It seems like the points you made earlier are mooted by now.

Do you intend to remove autotools build from all CI, so that you alone
will be the lone user of it, meaning that no other developer should
care about its existence at all?

Wanting to keep it in-tree does not sound like it, but I wanted to
confirm what your expectations are from other developers.


Thanks,
pq


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