Re: [Mesa-dev] [PATCH] anv/android: we need git_sha1.h in include paths
ping ... this fixes Android build as anv_device.c started to include "git_sha1.h" but build does not currently pass the path to this header. On 10/3/18 1:46 PM, Tapani Pälli wrote: Fixes: e4538b9 "anv: Implement VK_KHR_driver_properties" Signed-off-by: Tapani Pälli --- src/intel/Android.vulkan.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/src/intel/Android.vulkan.mk b/src/intel/Android.vulkan.mk index 8dc20149784..03120cf48a0 100644 --- a/src/intel/Android.vulkan.mk +++ b/src/intel/Android.vulkan.mk @@ -246,6 +246,7 @@ LOCAL_C_INCLUDES := \ LOCAL_WHOLE_STATIC_LIBRARIES := \ libmesa_anv_entrypoints \ libmesa_genxml \ + libmesa_git_sha1 \ libmesa_vulkan_util # The rule generates both C and H files, but due to some strange ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 108311] Query buffer object support is broken on r600.
https://bugs.freedesktop.org/show_bug.cgi?id=108311 --- Comment #3 from Andrew Wesie --- (In reply to Andrew Wesie from comment #2) > (In reply to Dave Airlie from comment #1) > > Created attachment 141989 [details] [review] [review] > > set larger alignment for tmp buffer offset > > > > Does this patch work as an alternate? > > It looks like it should work but I'll test it with real hw. > I confirmed the new patch fixes the bug with my test gpu (HD 5700 series). -- You are receiving this mail because: You are on the CC list for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 108311] Query buffer object support is broken on r600.
https://bugs.freedesktop.org/show_bug.cgi?id=108311 --- Comment #2 from Andrew Wesie --- (In reply to Dave Airlie from comment #1) > Created attachment 141989 [details] [review] > set larger alignment for tmp buffer offset > > Does this patch work as an alternate? It looks like it should work but I'll test it with real hw. Any reason you prefer this patch? It seems like it would use more heap space without any notable benefits (e.g. should it have better performance characteristics?). -- You are receiving this mail because: You are on the CC list for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 108311] Query buffer object support is broken on r600.
https://bugs.freedesktop.org/show_bug.cgi?id=108311 --- Comment #1 from Dave Airlie --- Created attachment 141989 --> https://bugs.freedesktop.org/attachment.cgi?id=141989=edit set larger alignment for tmp buffer offset Does this patch work as an alternate? -- You are receiving this mail because: You are on the CC list for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: remove redundant es_shader checks
Assuming there were no CI failures, this patch is Reviewed-by: Ian Romanick On 10/10/2018 05:25 PM, Timothy Arceri wrote: > The es check is already covered by the is_version() check. > --- > src/compiler/glsl/ast_to_hir.cpp | 4 > src/compiler/glsl_types.cpp | 2 +- > 2 files changed, 1 insertion(+), 5 deletions(-) > > diff --git a/src/compiler/glsl/ast_to_hir.cpp > b/src/compiler/glsl/ast_to_hir.cpp > index 1082d6c91cf..77fe0afef86 100644 > --- a/src/compiler/glsl/ast_to_hir.cpp > +++ b/src/compiler/glsl/ast_to_hir.cpp > @@ -314,10 +314,6 @@ apply_implicit_conversion(const glsl_type *to, ir_rvalue > * , > if (!state->is_version(120, 0)) >return false; > > - /* ESSL does not allow implicit conversions */ > - if (state->es_shader) > - return false; > - > /* From page 27 (page 33 of the PDF) of the GLSL 1.50 spec: > * > *"There are no implicit array or structure conversions. For > diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp > index ca5368aa53f..70bce6ace8e 100644 > --- a/src/compiler/glsl_types.cpp > +++ b/src/compiler/glsl_types.cpp > @@ -1425,7 +1425,7 @@ glsl_type::can_implicitly_convert_to(const glsl_type > *desired, > * state, we're doing intra-stage function linking where these checks have > * already been done. > */ > - if (state && (state->es_shader || !state->is_version(120, 0))) > + if (state && !state->is_version(120, 0)) >return false; > > /* There is no conversion among matrix types. */ > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/compiler: Fix nir_op_b2[fi] with 64-bit result on Gen8 LP and Gen9 LP
On 10/10/2018 05:17 PM, Jason Ekstrand wrote:
> Do we need a fix for vec4?
I had expected that we would, and I even wrote a patch for that. It
seems that the Atom GPUs that use the vec4 backend don't have the same
restriction. There weren't any failures on BYT caused by a68dd47b911,
and I couldn't trigger any validator errors locally using
INTEL_DEVID_OVERRIDE with a bunch of different Atom PCI IDs.
> On October 10, 2018 17:17:17 "Ian Romanick" wrote:
>
>> From: Jason Ekstrand
>>
>> Several of the Atom GPUs have additional restrictions on alignment when
>> moving < 64-bit source to a 64-bit destination. All of the nir_op_*2*64
>> code generation paths respected this, but nir_op_b2[fi] did not.
>>
>> Previous to commit a68dd47b911 it was not possible to generate such an
>> instruction from the GLSL path. It may have been possible from SPIR-V,
>> but it's not clear. The aforementioned patch converts a 64-bit
>> nir_op_fsign into a sequence of operations including a nir_op_b2f with a
>> 64-bit result. This "just works" everywhere except these Atom parts.
>>
>> This problem was not detected during normal CI testing because the Atom
>> parts are not included in developer builds.
>>
>> v2 (idr): Make the patch compile, and make some cosmetic changes. Add a
>> commit message.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108319
>> Fixes: a68dd47b911 "nir/algebraic: Simplify fsat of fsign"
>> ---
>> src/intel/compiler/brw_fs_nir.cpp | 10 +-
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/intel/compiler/brw_fs_nir.cpp
>> b/src/intel/compiler/brw_fs_nir.cpp
>> index 12b087a5ec0..7930205d659 100644
>> --- a/src/intel/compiler/brw_fs_nir.cpp
>> +++ b/src/intel/compiler/brw_fs_nir.cpp
>> @@ -793,6 +793,11 @@ fs_visitor::nir_emit_alu(const fs_builder ,
>> nir_alu_instr *instr)
>> inst->saturate = instr->dest.saturate;
>> break;
>>
>> + case nir_op_b2i:
>> + case nir_op_b2f:
>> + op[0].type = BRW_REGISTER_TYPE_D;
>> + op[0].negate = !op[0].negate;
>> + /* fallthrough */
>> case nir_op_f2f64:
>> case nir_op_f2i64:
>> case nir_op_f2u64:
>> @@ -1213,11 +1218,6 @@ fs_visitor::nir_emit_alu(const fs_builder ,
>> nir_alu_instr *instr)
>> inst->saturate = instr->dest.saturate;
>> break;
>>
>> - case nir_op_b2i:
>> - case nir_op_b2f:
>> - bld.MOV(result, negate(op[0]));
>> - break;
>> -
>> case nir_op_i2b:
>> case nir_op_f2b: {
>> uint32_t bit_size = nir_src_bit_size(instr->src[0].src);
>> --
>> 2.14.4
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: remove redundant es_shader checks
The es check is already covered by the is_version() check. --- src/compiler/glsl/ast_to_hir.cpp | 4 src/compiler/glsl_types.cpp | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp index 1082d6c91cf..77fe0afef86 100644 --- a/src/compiler/glsl/ast_to_hir.cpp +++ b/src/compiler/glsl/ast_to_hir.cpp @@ -314,10 +314,6 @@ apply_implicit_conversion(const glsl_type *to, ir_rvalue * , if (!state->is_version(120, 0)) return false; - /* ESSL does not allow implicit conversions */ - if (state->es_shader) - return false; - /* From page 27 (page 33 of the PDF) of the GLSL 1.50 spec: * *"There are no implicit array or structure conversions. For diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp index ca5368aa53f..70bce6ace8e 100644 --- a/src/compiler/glsl_types.cpp +++ b/src/compiler/glsl_types.cpp @@ -1425,7 +1425,7 @@ glsl_type::can_implicitly_convert_to(const glsl_type *desired, * state, we're doing intra-stage function linking where these checks have * already been done. */ - if (state && (state->es_shader || !state->is_version(120, 0))) + if (state && !state->is_version(120, 0)) return false; /* There is no conversion among matrix types. */ -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/compiler: Fix nir_op_b2[fi] with 64-bit result on Gen8 LP and Gen9 LP
Do we need a fix for vec4?
On October 10, 2018 17:17:17 "Ian Romanick" wrote:
From: Jason Ekstrand
Several of the Atom GPUs have additional restrictions on alignment when
moving < 64-bit source to a 64-bit destination. All of the nir_op_*2*64
code generation paths respected this, but nir_op_b2[fi] did not.
Previous to commit a68dd47b911 it was not possible to generate such an
instruction from the GLSL path. It may have been possible from SPIR-V,
but it's not clear. The aforementioned patch converts a 64-bit
nir_op_fsign into a sequence of operations including a nir_op_b2f with a
64-bit result. This "just works" everywhere except these Atom parts.
This problem was not detected during normal CI testing because the Atom
parts are not included in developer builds.
v2 (idr): Make the patch compile, and make some cosmetic changes. Add a
commit message.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108319
Fixes: a68dd47b911 "nir/algebraic: Simplify fsat of fsign"
---
src/intel/compiler/brw_fs_nir.cpp | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/intel/compiler/brw_fs_nir.cpp
b/src/intel/compiler/brw_fs_nir.cpp
index 12b087a5ec0..7930205d659 100644
--- a/src/intel/compiler/brw_fs_nir.cpp
+++ b/src/intel/compiler/brw_fs_nir.cpp
@@ -793,6 +793,11 @@ fs_visitor::nir_emit_alu(const fs_builder ,
nir_alu_instr *instr)
inst->saturate = instr->dest.saturate;
break;
+ case nir_op_b2i:
+ case nir_op_b2f:
+ op[0].type = BRW_REGISTER_TYPE_D;
+ op[0].negate = !op[0].negate;
+ /* fallthrough */
case nir_op_f2f64:
case nir_op_f2i64:
case nir_op_f2u64:
@@ -1213,11 +1218,6 @@ fs_visitor::nir_emit_alu(const fs_builder ,
nir_alu_instr *instr)
inst->saturate = instr->dest.saturate;
break;
- case nir_op_b2i:
- case nir_op_b2f:
- bld.MOV(result, negate(op[0]));
- break;
-
case nir_op_i2b:
case nir_op_f2b: {
uint32_t bit_size = nir_src_bit_size(instr->src[0].src);
--
2.14.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] anv/gen9+: Initialize new fields in STATE_BASE_ADDRESS
On 2018-10-10 14:38:23, Rafael Antognolli wrote:
> On Wed, Oct 10, 2018 at 02:04:11PM -0700, Jordan Justen wrote:
> > On 2018-10-10 13:45:13, Rafael Antognolli wrote:
> > > On Wed, Oct 10, 2018 at 01:39:25PM -0700, Jordan Justen wrote:
> > > > Ref: 263b584d5e4 "i965/skl: Emit extra zeros in STATE_BASE_ADDRESS on
> > > > Skylake."
> > > > Signed-off-by: Jordan Justen
> > > > ---
> > > > src/intel/vulkan/genX_cmd_buffer.c | 12
> > > > 1 file changed, 12 insertions(+)
> > > >
> > > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > > > b/src/intel/vulkan/genX_cmd_buffer.c
> > > > index c3a7e5c83c3..43a02f22567 100644
> > > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > > @@ -121,6 +121,18 @@ genX(cmd_buffer_emit_state_base_address)(struct
> > > > anv_cmd_buffer *cmd_buffer)
> > > >sba.IndirectObjectBufferSizeModifyEnable = true;
> > > >sba.InstructionBufferSize = 0xf;
> > > >sba.InstructionBuffersizeModifyEnable = true;
> > > > +# endif
> > > > +# if (GEN_GEN >= 9)
> > > > + sba.BindlessSurfaceStateBaseAddress = (struct anv_address) {
> > > > NULL, 0 };
> > > > + sba.BindlessSurfaceStateMemoryObjectControlState = GENX(MOCS);
> > > > + sba.BindlessSurfaceStateBaseAddressModifyEnable = true;
> > > > + sba.BindlessSurfaceStateSize = 0;
> > > > +# endif
> > > > +# if (GEN_GEN >= 10)
> > > > + sba.BindlessSamplerStateBaseAddress = (struct anv_address) {
> > > > NULL, 0 };
> > > > + sba.BindlessSamplerStateMemoryObjectControlState = GENX(MOCS);
> > > > + sba.BindlessSamplerStateBaseAddressModifyEnable = true;
> > > > + sba.BindlessSamplerStateBufferSize = 0;
> > >
> > > Do we really need to set all of these fields? AFAIK the ones we don't
> > > set should be left as 0's anyway, so at least the Address and BufferSize
> > > should be fine to be left out. I think the MOCS field should be fine
> > > too, since we are not setting any pointer here. Unless you want to
> > > be really explicit...
> >
> > Yeah. I don't know that it is helpful since the genxml already sets
> > the packet length, and I guess things should be zero by default. Maybe
> > it will make it a little easier to find for bindless in the future?
> >
> > Regarding Jason's comment about the enable bit, I was following Ken's
> > referenced commit (263b584d5e4) for the similar field in gen9+ on
> > i965. Maybe it is good to actually force the write to explicitly set
> > the size to 0?
>
> Yeah, my understanding is that we should set the "modify" bit, so it
> will actually set the address and size to 0.
>
> > I guess setting MOCS does not follow what Ken did in i965.
> >
> > If we actually do want to set the enable bit, then it might be good to
> > also leave the fields being explicitly set to zero.
> >
> > My preference would be to just set the fields explicitly. Since we
> > only specify this packet in one place, it doesn't seem like it adds
> > too much verbosity.
>
> On most of the genxml code I've seen, we only set the fields that are
> not zeroed by default.
I think there are exceptions:
$ git grep -Ee "\..* = 0;" src/intel/vulkan/genX_*
I think the rule is more like: if setting the field to 0 is notable,
then it's better to explicitly set it for informational purposes.
If the 'enable' bit is set, then I think think the fields that will be
updated are notable. If the 'enable' bit was not set, then maybe the
fields are not important. (In that case, perhaps the patch should be
dropped entirely.)
Anyway, if Jason doesn't have any further input, I'll go with your
suggestion of dropping the zeroed fields.
-Jordan
> And if the name of these fields change or
> something in a future generation, assuming we are still not using them,
> it's easier to just change the xml for that gen.
>
> So to keep the code consistent with the rest, I would leave it out, but
> regardless of what you choose,
>
> Reviewed-by: Rafael Antognolli
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] anv: Clear WM_HZ_OP overrides in init_device_state
This is basically a port of commit,
3ade766684933ac84e41634429fb693f85353c11
("i965: Disable 3DSTATE_WM_HZ_OP fields.")
The BDW+ docs describe how to use the 3DSTATE_WM_HZ_OP instruction in
the section titled, "Optimized Depth Buffer Clear and/or Stencil Buffer
Clear." It mentions that the packet overrides GPU state for the clear
operation and needs to be reset to 0s to clear the overrides. Depending
on the kernel, we may not get a context with the GPU state for this
packet zeroed. Do it ourselves just in case.
Prevents a number of GPU hangs when running crucible on ICL. I tried to
get the exact number of hangs that occurs without this patch, but was
unsuccessful. The test machine became unresponsive before completing the
full run.
Reviewed-by: Kenneth Graunke
---
src/intel/vulkan/genX_state.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/intel/vulkan/genX_state.c b/src/intel/vulkan/genX_state.c
index 75bcd96d78a..42800a2581e 100644
--- a/src/intel/vulkan/genX_state.c
+++ b/src/intel/vulkan/genX_state.c
@@ -157,6 +157,16 @@ genX(init_device_state)(struct anv_device *device)
GEN_SAMPLE_POS_16X(sp._16xSample);
#endif
}
+
+ /* The BDW+ docs describe how to use the 3DSTATE_WM_HZ_OP instruction in the
+* section titled, "Optimized Depth Buffer Clear and/or Stencil Buffer
+* Clear." It mentions that the packet overrides GPU state for the clear
+* operation and needs to be reset to 0s to clear the overrides. Depending
+* on the kernel, we may not get a context with the state for this packet
+* zeroed. Do it ourselves just in case. We've observed this to prevent a
+* number of GPU hangs on ICL.
+*/
+ anv_batch_emit(, GENX(3DSTATE_WM_HZ_OP), hzp);
#endif
#if GEN_GEN == 10
--
2.19.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] i965/miptree: Use enum instead of boolean.
ISL_AUX_USAGE_NONE happens to be the same as "false", but let's do the
right thing and use the enum.
v2: fix intel_miptree_finish_depth too (Caio)
Reviewed-by: Dylan Baker
Reviewed-by: Caio Marcelo de Oliveira Filho
Reviewed-by: Jason Ekstrand
---
I just added the finish_depth() fix in the same patch and kept the rb's,
since it's a one-liner. And imho it makes sense to have all the fixes in
a single commit. Hopefully it's not an issue.
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 2 +-
src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index e32641f4098..69b7a96b9c7 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -2727,7 +2727,7 @@ intel_miptree_finish_depth(struct brw_context *brw,
{
if (depth_written) {
intel_miptree_finish_write(brw, mt, level, start_layer, layer_count,
- mt->aux_buf != NULL);
+ mt->aux_usage);
}
}
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index 708757c47b8..b0333655ad5 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -613,9 +613,10 @@ intel_miptree_access_raw(struct brw_context *brw,
uint32_t level, uint32_t layer,
bool write)
{
- intel_miptree_prepare_access(brw, mt, level, 1, layer, 1, false, false);
+ intel_miptree_prepare_access(brw, mt, level, 1, layer, 1,
+ISL_AUX_USAGE_NONE, false);
if (write)
- intel_miptree_finish_write(brw, mt, level, layer, 1, false);
+ intel_miptree_finish_write(brw, mt, level, layer, 1, ISL_AUX_USAGE_NONE);
}
enum isl_aux_usage
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/compiler: Fix nir_op_b2[fi] with 64-bit result on Gen8 LP and Gen9 LP
From: Jason Ekstrand
Several of the Atom GPUs have additional restrictions on alignment when
moving < 64-bit source to a 64-bit destination. All of the nir_op_*2*64
code generation paths respected this, but nir_op_b2[fi] did not.
Previous to commit a68dd47b911 it was not possible to generate such an
instruction from the GLSL path. It may have been possible from SPIR-V,
but it's not clear. The aforementioned patch converts a 64-bit
nir_op_fsign into a sequence of operations including a nir_op_b2f with a
64-bit result. This "just works" everywhere except these Atom parts.
This problem was not detected during normal CI testing because the Atom
parts are not included in developer builds.
v2 (idr): Make the patch compile, and make some cosmetic changes. Add a
commit message.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108319
Fixes: a68dd47b911 "nir/algebraic: Simplify fsat of fsign"
---
src/intel/compiler/brw_fs_nir.cpp | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/intel/compiler/brw_fs_nir.cpp
b/src/intel/compiler/brw_fs_nir.cpp
index 12b087a5ec0..7930205d659 100644
--- a/src/intel/compiler/brw_fs_nir.cpp
+++ b/src/intel/compiler/brw_fs_nir.cpp
@@ -793,6 +793,11 @@ fs_visitor::nir_emit_alu(const fs_builder ,
nir_alu_instr *instr)
inst->saturate = instr->dest.saturate;
break;
+ case nir_op_b2i:
+ case nir_op_b2f:
+ op[0].type = BRW_REGISTER_TYPE_D;
+ op[0].negate = !op[0].negate;
+ /* fallthrough */
case nir_op_f2f64:
case nir_op_f2i64:
case nir_op_f2u64:
@@ -1213,11 +1218,6 @@ fs_visitor::nir_emit_alu(const fs_builder ,
nir_alu_instr *instr)
inst->saturate = instr->dest.saturate;
break;
- case nir_op_b2i:
- case nir_op_b2f:
- bld.MOV(result, negate(op[0]));
- break;
-
case nir_op_i2b:
case nir_op_f2b: {
uint32_t bit_size = nir_src_bit_size(instr->src[0].src);
--
2.14.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/5] anv: Drop some VK_IMAGE_TILING_OPTIMAL checks
On Mon 01 Oct 2018, Jason Ekstrand wrote:
> The DRM format modifiers extension adds a TILING_DRM_FORMAT_MODIFIER
> which will be used for modifiers so we can no longer use OPTIMAL to
> indicate tiled inside the driver.
> ---
> src/intel/vulkan/anv_formats.c | 2 +-
> src/intel/vulkan/anv_image.c | 6 +++---
> src/intel/vulkan/genX_cmd_buffer.c | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
This patch needs to also fixup some places that test tiling ==
VK_IMAGE_TILING_LINEAR. I found at least one such place in
anv_formats.c. The patch also needs to fixup any functions that use
early returns that are conditioned (perhaps indirectly) on tiling;
anv_get_format_plane() comes to mind.
I quickly tried, as an experiment, to expand this patch into a correct
patch. After a few minutes of typing, I concluded that this patch series
takes the wrong order-of-implementation approach.
For example, what happens to all the calls to anv_get_format_plane() in
anv_blorp.c? Those need fixing too. Simply fixing the tiling checks is
not enough, especially because VK_IMAGE_TILING_DRM_FORMAT_MODIFIER_EXT
allows DRM_FORMAT_MOD_LINEAR.
Instead, I think the right way to do this is to incrementally, following
the callchains bottom-up, teach each function to understand
VK_EXT_image_drm_format_modifier. Only when that's complete, then move
WSI to the new structs. Otherwise, there is too much room for
accidential implementation gaps; gaps that may not hurt WSI, but will
make it more difficult to discern what-works-and-what-doesn't while
implementing the full extension.
Just now, I tried writing a patch that starts at the bottom of the
callchain, anv_get_format_plane(), and teaches it about modifiers. The
patch is more invasive than expected. Maybe the patch is messy because
more preliminary cleanups are needed. I'm unsure; I need to take a break
and revisit it in the morning.
>
> diff --git a/src/intel/vulkan/anv_formats.c b/src/intel/vulkan/anv_formats.c
> index 33faf7cc37f..d44aae1c127 100644
> --- a/src/intel/vulkan/anv_formats.c
> +++ b/src/intel/vulkan/anv_formats.c
> @@ -508,7 +508,7 @@ get_image_format_features(const struct gen_device_info
> *devinfo,
>return 0;
>
> struct anv_format_plane base_plane_format = plane_format;
> - if (vk_tiling == VK_IMAGE_TILING_OPTIMAL) {
> + if (vk_tiling != VK_IMAGE_TILING_LINEAR) {
>base_plane_format = anv_get_format_plane(devinfo, vk_format,
> VK_IMAGE_ASPECT_COLOR_BIT,
> VK_IMAGE_TILING_LINEAR);
> diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c
> index b0d8c560adb..70594d6c053 100644
> --- a/src/intel/vulkan/anv_image.c
> +++ b/src/intel/vulkan/anv_image.c
> @@ -334,7 +334,7 @@ make_surface(const struct anv_device *dev,
> bool needs_shadow = false;
> if (dev->info.gen <= 8 &&
> (vk_info->flags & VK_IMAGE_CREATE_BLOCK_TEXEL_VIEW_COMPATIBLE_BIT) &&
> - vk_info->tiling == VK_IMAGE_TILING_OPTIMAL) {
> + vk_info->tiling != VK_IMAGE_TILING_LINEAR) {
>assert(isl_format_is_compressed(plane_format.isl_format));
>tiling_flags = ISL_TILING_LINEAR_BIT;
>needs_shadow = true;
> @@ -829,7 +829,7 @@ anv_layout_to_aux_usage(const struct gen_device_info *
> const devinfo,
>return ISL_AUX_USAGE_NONE;
>
> /* All images that use an auxiliary surface are required to be tiled. */
> - assert(image->tiling == VK_IMAGE_TILING_OPTIMAL);
> + assert(image->planes[plane].surface.isl.tiling != ISL_TILING_LINEAR);
>
> /* Stencil has no aux */
> assert(aspect != VK_IMAGE_ASPECT_STENCIL_BIT);
> @@ -953,7 +953,7 @@ anv_layout_to_fast_clear_type(const struct
> gen_device_info * const devinfo,
>return ANV_FAST_CLEAR_NONE;
>
> /* All images that use an auxiliary surface are required to be tiled. */
> - assert(image->tiling == VK_IMAGE_TILING_OPTIMAL);
> + assert(image->planes[plane].surface.isl.tiling != ISL_TILING_LINEAR);
>
> /* Stencil has no aux */
> assert(aspect != VK_IMAGE_ASPECT_STENCIL_BIT);
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index 099c30f3d66..821506761e2 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -967,7 +967,7 @@ transition_color_buffer(struct anv_cmd_buffer *cmd_buffer,
> if (base_layer >= anv_image_aux_layers(image, aspect, base_level))
>return;
>
> - assert(image->tiling == VK_IMAGE_TILING_OPTIMAL);
> + assert(image->planes[plane].surface.isl.tiling != ISL_TILING_LINEAR);
>
> if (initial_layout == VK_IMAGE_LAYOUT_UNDEFINED ||
> initial_layout == VK_IMAGE_LAYOUT_PREINITIALIZED) {
> --
> 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] nir: fix compacting varyings when XFB outputs are present
On 11/10/18 1:46 am, Samuel Pitoiset wrote:
We shouldn't try to compact any varyings known as always
active IO, especially XFB outputs. For example, if one
component of an xfb output is also used as input varying
in the next stage, it shouldn't be compacted.
This small helper just marks all XFB varyings as always_active_io
in the consumer to not compact them.
v2: add a little helper
Signed-off-by: Samuel Pitoiset
---
src/compiler/nir/nir_linking_helpers.c | 33 ++
1 file changed, 33 insertions(+)
diff --git a/src/compiler/nir/nir_linking_helpers.c
b/src/compiler/nir/nir_linking_helpers.c
index 88014e9a1d..433729bd79 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -480,6 +480,36 @@ compact_components(nir_shader *producer, nir_shader
*consumer, uint8_t *comps,
>info.outputs_read);
}
+/* Mark XFB varyings as always_active_io in the consumer to not compact them. */
+static void
+link_xfb_varyings(nir_shader *producer, nir_shader *consumer)
+{
+ nir_variable *input_vars[32] = {};
nir_variable *input_vars[MAX_VARYING] = {};
+
+ nir_foreach_variable(var, >inputs) {
+ if (var->data.location >= VARYING_SLOT_VAR0 &&
+ var->data.location - VARYING_SLOT_VAR0 < 32) {
var->data.location - VARYING_SLOT_VAR0 < MAX_VARYING) {
+
+ unsigned location = var->data.location - VARYING_SLOT_VAR0;
+ input_vars[location] = var;
+ }
+ }
+
+ nir_foreach_variable(var, >outputs) {
+ if (var->data.location >= VARYING_SLOT_VAR0 &&
+ var->data.location - VARYING_SLOT_VAR0 < 32) {
var->data.location - VARYING_SLOT_VAR0 < MAX_VARYING) {
+
+ if (!var->data.explicit_xfb_buffer)
+continue;
+
+ unsigned location = var->data.location - VARYING_SLOT_VAR0;
+ if (input_vars[location]) {
+input_vars[location]->data.always_active_io = true;
+ }
+ }
+ }
+}
+
/* We assume that this has been called more-or-less directly after
* remove_unused_varyings. At this point, all of the varyings that we
* aren't going to be using have been completely removed and the
@@ -501,6 +531,9 @@ nir_compact_varyings(nir_shader *producer, nir_shader
*consumer,
uint8_t interp_type[32] = {0};
uint8_t interp_loc[32] = {0};
Hmm, looks like I should fix up these magic numbers also. I was planning
on changing this code when adding patch support (which I still haven't
got round to).
+ if (producer->info.has_transform_feedback_varyings)
+ link_xfb_varyings(producer, consumer);
I think you need to expose link_xfb_varyings() externally and call it in
radv_link_shaders() before nir_lower_io_arrays_to_elements()
e.g
if (ordered_shaders[i]->info.has_transform_feedback_varyings)
nir_link_xfb_varyings(ordered_shaders[i], ordered_shaders[i - 1]);
With these changes this patch is:
Reviewed-by: Timothy Arceri
+
get_slot_component_masks_and_interp_types(>outputs, comps,
interp_type, interp_loc,
producer->info.stage,
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] anv/gen9+: Initialize new fields in STATE_BASE_ADDRESS
On Wed, Oct 10, 2018 at 02:04:11PM -0700, Jordan Justen wrote:
> On 2018-10-10 13:45:13, Rafael Antognolli wrote:
> > On Wed, Oct 10, 2018 at 01:39:25PM -0700, Jordan Justen wrote:
> > > Ref: 263b584d5e4 "i965/skl: Emit extra zeros in STATE_BASE_ADDRESS on
> > > Skylake."
> > > Signed-off-by: Jordan Justen
> > > ---
> > > src/intel/vulkan/genX_cmd_buffer.c | 12
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > > b/src/intel/vulkan/genX_cmd_buffer.c
> > > index c3a7e5c83c3..43a02f22567 100644
> > > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > > @@ -121,6 +121,18 @@ genX(cmd_buffer_emit_state_base_address)(struct
> > > anv_cmd_buffer *cmd_buffer)
> > >sba.IndirectObjectBufferSizeModifyEnable = true;
> > >sba.InstructionBufferSize = 0xf;
> > >sba.InstructionBuffersizeModifyEnable = true;
> > > +# endif
> > > +# if (GEN_GEN >= 9)
> > > + sba.BindlessSurfaceStateBaseAddress = (struct anv_address) { NULL,
> > > 0 };
> > > + sba.BindlessSurfaceStateMemoryObjectControlState = GENX(MOCS);
> > > + sba.BindlessSurfaceStateBaseAddressModifyEnable = true;
> > > + sba.BindlessSurfaceStateSize = 0;
> > > +# endif
> > > +# if (GEN_GEN >= 10)
> > > + sba.BindlessSamplerStateBaseAddress = (struct anv_address) { NULL,
> > > 0 };
> > > + sba.BindlessSamplerStateMemoryObjectControlState = GENX(MOCS);
> > > + sba.BindlessSamplerStateBaseAddressModifyEnable = true;
> > > + sba.BindlessSamplerStateBufferSize = 0;
> >
> > Do we really need to set all of these fields? AFAIK the ones we don't
> > set should be left as 0's anyway, so at least the Address and BufferSize
> > should be fine to be left out. I think the MOCS field should be fine
> > too, since we are not setting any pointer here. Unless you want to
> > be really explicit...
>
> Yeah. I don't know that it is helpful since the genxml already sets
> the packet length, and I guess things should be zero by default. Maybe
> it will make it a little easier to find for bindless in the future?
>
> Regarding Jason's comment about the enable bit, I was following Ken's
> referenced commit (263b584d5e4) for the similar field in gen9+ on
> i965. Maybe it is good to actually force the write to explicitly set
> the size to 0?
Yeah, my understanding is that we should set the "modify" bit, so it
will actually set the address and size to 0.
> I guess setting MOCS does not follow what Ken did in i965.
>
> If we actually do want to set the enable bit, then it might be good to
> also leave the fields being explicitly set to zero.
>
> My preference would be to just set the fields explicitly. Since we
> only specify this packet in one place, it doesn't seem like it adds
> too much verbosity.
On most of the genxml code I've seen, we only set the fields that are
not zeroed by default. And if the name of these fields change or
something in a future generation, assuming we are still not using them,
it's easier to just change the xml for that gen.
So to keep the code consistent with the rest, I would leave it out, but
regardless of what you choose,
Reviewed-by: Rafael Antognolli
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3 1/2] i965/batch: avoid reverting batch buffer if saved state is an empty
On 2018-09-12 09:05:44, wrote:
> From: Andrii Simiklit
>
> There's no point reverting to the last saved point if that save point is
> the empty batch, we will just repeat ourselves.
>
> CC: Chris Wilson
> Fixes: 3faf56ffbdeb "intel: Add an interface for saving/restoring
> the batchbuffer state."
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107626
> ---
> src/mesa/drivers/dri/i965/brw_compute.c | 3 ++-
> src/mesa/drivers/dri/i965/brw_draw.c | 3 ++-
> src/mesa/drivers/dri/i965/genX_blorp_exec.c | 3 ++-
> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 7 +++
> src/mesa/drivers/dri/i965/intel_batchbuffer.h | 1 +
> 5 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_compute.c
> b/src/mesa/drivers/dri/i965/brw_compute.c
> index de08fc3..5c8e3a5 100644
> --- a/src/mesa/drivers/dri/i965/brw_compute.c
> +++ b/src/mesa/drivers/dri/i965/brw_compute.c
> @@ -167,7 +167,7 @@ static void
> brw_dispatch_compute_common(struct gl_context *ctx)
> {
> struct brw_context *brw = brw_context(ctx);
> - bool fail_next = false;
> + bool fail_next;
>
> if (!_mesa_check_conditional_render(ctx))
>return;
> @@ -185,6 +185,7 @@ brw_dispatch_compute_common(struct gl_context *ctx)
> intel_batchbuffer_require_space(brw, 600);
> brw_require_statebuffer_space(brw, 2500);
> intel_batchbuffer_save_state(brw);
> + fail_next = intel_batchbuffer_saved_state_is_empty(brw);
>
> retry:
> brw->batch.no_wrap = true;
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> b/src/mesa/drivers/dri/i965/brw_draw.c
> index 8536c04..19ee396 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -885,7 +885,7 @@ brw_draw_single_prim(struct gl_context *ctx,
> {
> struct brw_context *brw = brw_context(ctx);
> const struct gen_device_info *devinfo = >screen->devinfo;
> - bool fail_next = false;
> + bool fail_next;
>
> /* Flag BRW_NEW_DRAW_CALL on every draw. This allows us to have
> * atoms that happen on every draw call.
> @@ -898,6 +898,7 @@ brw_draw_single_prim(struct gl_context *ctx,
> intel_batchbuffer_require_space(brw, 1500);
> brw_require_statebuffer_space(brw, 2400);
> intel_batchbuffer_save_state(brw);
> + fail_next = intel_batchbuffer_saved_state_is_empty(brw);
It seems like this will cause the WARN_ONCE to be hit incorrectly.
What about adding a 'bool empty_state', and checking that before
fail_next in the code that follows. If empty_state is true, I guess
you want to flush, but not emit the WARN_ONCE?
With that change,
series Reviewed-by: Jordan Justen
As always, it could be nice to get an r-b, or acked-by from Ken. :)
-Jordan
>
> if (brw->num_instances != prim->num_instances ||
> brw->basevertex != prim->basevertex ||
> diff --git a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> index 34bfcad..fd9ce93 100644
> --- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> +++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> @@ -268,7 +268,7 @@ genX(blorp_exec)(struct blorp_batch *batch,
> assert(batch->blorp->driver_ctx == batch->driver_batch);
> struct brw_context *brw = batch->driver_batch;
> struct gl_context *ctx = >ctx;
> - bool check_aperture_failed_once = false;
> + bool check_aperture_failed_once;
>
> #if GEN_GEN >= 11
> /* The PIPE_CONTROL command description says:
> @@ -309,6 +309,7 @@ retry:
> intel_batchbuffer_require_space(brw, 1400);
> brw_require_statebuffer_space(brw, 600);
> intel_batchbuffer_save_state(brw);
> + check_aperture_failed_once = intel_batchbuffer_saved_state_is_empty(brw);
> brw->batch.no_wrap = true;
>
> #if GEN_GEN == 6
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 4363b14..2dc6eb8 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -299,6 +299,13 @@ intel_batchbuffer_save_state(struct brw_context *brw)
> brw->batch.saved.exec_count = brw->batch.exec_count;
> }
>
> +bool
> +intel_batchbuffer_saved_state_is_empty(struct brw_context *brw)
> +{
> + struct intel_batchbuffer *batch = >batch;
> + return (batch->saved.map_next == batch->batch.map);
> +}
> +
> void
> intel_batchbuffer_reset_to_saved(struct brw_context *brw)
> {
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.h
> b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
> index 0632142..91720da 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.h
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
> @@ -24,6 +24,7 @@ struct intel_batchbuffer;
> void intel_batchbuffer_init(struct brw_context *brw);
> void intel_batchbuffer_free(struct intel_batchbuffer *batch);
> void intel_batchbuffer_save_state(struct brw_context *brw);
> +bool
Re: [Mesa-dev] [PATCH 1/2] anv/gen9+: Initialize new fields in STATE_BASE_ADDRESS
On 2018-10-10 13:45:13, Rafael Antognolli wrote:
> On Wed, Oct 10, 2018 at 01:39:25PM -0700, Jordan Justen wrote:
> > Ref: 263b584d5e4 "i965/skl: Emit extra zeros in STATE_BASE_ADDRESS on
> > Skylake."
> > Signed-off-by: Jordan Justen
> > ---
> > src/intel/vulkan/genX_cmd_buffer.c | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> > b/src/intel/vulkan/genX_cmd_buffer.c
> > index c3a7e5c83c3..43a02f22567 100644
> > --- a/src/intel/vulkan/genX_cmd_buffer.c
> > +++ b/src/intel/vulkan/genX_cmd_buffer.c
> > @@ -121,6 +121,18 @@ genX(cmd_buffer_emit_state_base_address)(struct
> > anv_cmd_buffer *cmd_buffer)
> >sba.IndirectObjectBufferSizeModifyEnable = true;
> >sba.InstructionBufferSize = 0xf;
> >sba.InstructionBuffersizeModifyEnable = true;
> > +# endif
> > +# if (GEN_GEN >= 9)
> > + sba.BindlessSurfaceStateBaseAddress = (struct anv_address) { NULL, 0
> > };
> > + sba.BindlessSurfaceStateMemoryObjectControlState = GENX(MOCS);
> > + sba.BindlessSurfaceStateBaseAddressModifyEnable = true;
> > + sba.BindlessSurfaceStateSize = 0;
> > +# endif
> > +# if (GEN_GEN >= 10)
> > + sba.BindlessSamplerStateBaseAddress = (struct anv_address) { NULL, 0
> > };
> > + sba.BindlessSamplerStateMemoryObjectControlState = GENX(MOCS);
> > + sba.BindlessSamplerStateBaseAddressModifyEnable = true;
> > + sba.BindlessSamplerStateBufferSize = 0;
>
> Do we really need to set all of these fields? AFAIK the ones we don't
> set should be left as 0's anyway, so at least the Address and BufferSize
> should be fine to be left out. I think the MOCS field should be fine
> too, since we are not setting any pointer here. Unless you want to
> be really explicit...
Yeah. I don't know that it is helpful since the genxml already sets
the packet length, and I guess things should be zero by default. Maybe
it will make it a little easier to find for bindless in the future?
Regarding Jason's comment about the enable bit, I was following Ken's
referenced commit (263b584d5e4) for the similar field in gen9+ on
i965. Maybe it is good to actually force the write to explicitly set
the size to 0?
I guess setting MOCS does not follow what Ken did in i965.
If we actually do want to set the enable bit, then it might be good to
also leave the fields being explicitly set to zero.
My preference would be to just set the fields explicitly. Since we
only specify this packet in one place, it doesn't seem like it adds
too much verbosity.
-Jordan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i965/gen10+: Initialize new fields in STATE_BASE_ADDRESS
On Wed, Oct 10, 2018 at 01:39:26PM -0700, Jordan Justen wrote:
> Ref: 263b584d5e4 "i965/skl: Emit extra zeros in STATE_BASE_ADDRESS on
> Skylake."
> Signed-off-by: Jordan Justen
> ---
> src/mesa/drivers/dri/i965/brw_misc_state.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
> b/src/mesa/drivers/dri/i965/brw_misc_state.c
> index 0895e1f2b7f..9bff2c8ac92 100644
> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> @@ -688,7 +688,7 @@ brw_upload_state_base_address(struct brw_context *brw)
> * to the bottom 4GB.
> */
>uint32_t mocs_wb = devinfo->gen >= 9 ? SKL_MOCS_WB : BDW_MOCS_WB;
> - int pkt_len = devinfo->gen >= 9 ? 19 : 16;
> + int pkt_len = devinfo->gen >= 10 ? 22 : (devinfo->gen >= 9 ? 19 : 16);
>
>BEGIN_BATCH(pkt_len);
>OUT_BATCH(CMD_STATE_BASE_ADDRESS << 16 | (pkt_len - 2));
> @@ -718,6 +718,11 @@ brw_upload_state_base_address(struct brw_context *brw)
> OUT_BATCH(0);
> OUT_BATCH(0);
>}
> + if (devinfo->gen >= 10) {
> + OUT_BATCH(1);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + }
Reviewed-by: Rafael Antognolli
>ADVANCE_BATCH();
> } else if (devinfo->gen >= 6) {
>uint8_t mocs = devinfo->gen == 7 ? GEN7_MOCS_L3 : 0;
> --
> 2.19.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] anv/gen9+: Initialize new fields in STATE_BASE_ADDRESS
On Wed, Oct 10, 2018 at 01:39:25PM -0700, Jordan Justen wrote:
> Ref: 263b584d5e4 "i965/skl: Emit extra zeros in STATE_BASE_ADDRESS on
> Skylake."
> Signed-off-by: Jordan Justen
> ---
> src/intel/vulkan/genX_cmd_buffer.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index c3a7e5c83c3..43a02f22567 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -121,6 +121,18 @@ genX(cmd_buffer_emit_state_base_address)(struct
> anv_cmd_buffer *cmd_buffer)
>sba.IndirectObjectBufferSizeModifyEnable = true;
>sba.InstructionBufferSize = 0xf;
>sba.InstructionBuffersizeModifyEnable = true;
> +# endif
> +# if (GEN_GEN >= 9)
> + sba.BindlessSurfaceStateBaseAddress = (struct anv_address) { NULL, 0 };
> + sba.BindlessSurfaceStateMemoryObjectControlState = GENX(MOCS);
> + sba.BindlessSurfaceStateBaseAddressModifyEnable = true;
> + sba.BindlessSurfaceStateSize = 0;
> +# endif
> +# if (GEN_GEN >= 10)
> + sba.BindlessSamplerStateBaseAddress = (struct anv_address) { NULL, 0 };
> + sba.BindlessSamplerStateMemoryObjectControlState = GENX(MOCS);
> + sba.BindlessSamplerStateBaseAddressModifyEnable = true;
> + sba.BindlessSamplerStateBufferSize = 0;
Do we really need to set all of these fields? AFAIK the ones we don't
set should be left as 0's anyway, so at least the Address and BufferSize
should be fine to be left out. I think the MOCS field should be fine
too, since we are not setting any pointer here. Unless you want to
be really explicit...
> # endif
> }
>
> --
> 2.19.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] anv/gen9+: Initialize new fields in STATE_BASE_ADDRESS
Do we need to set the enable bits? If not, just extending the struct in
genxml should be sufficient.
On Wed, Oct 10, 2018 at 3:39 PM Jordan Justen
wrote:
> Ref: 263b584d5e4 "i965/skl: Emit extra zeros in STATE_BASE_ADDRESS on
> Skylake."
> Signed-off-by: Jordan Justen
> ---
> src/intel/vulkan/genX_cmd_buffer.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c
> b/src/intel/vulkan/genX_cmd_buffer.c
> index c3a7e5c83c3..43a02f22567 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -121,6 +121,18 @@ genX(cmd_buffer_emit_state_base_address)(struct
> anv_cmd_buffer *cmd_buffer)
>sba.IndirectObjectBufferSizeModifyEnable = true;
>sba.InstructionBufferSize = 0xf;
>sba.InstructionBuffersizeModifyEnable = true;
> +# endif
> +# if (GEN_GEN >= 9)
> + sba.BindlessSurfaceStateBaseAddress = (struct anv_address) { NULL,
> 0 };
> + sba.BindlessSurfaceStateMemoryObjectControlState = GENX(MOCS);
> + sba.BindlessSurfaceStateBaseAddressModifyEnable = true;
> + sba.BindlessSurfaceStateSize = 0;
> +# endif
> +# if (GEN_GEN >= 10)
> + sba.BindlessSamplerStateBaseAddress = (struct anv_address) { NULL,
> 0 };
> + sba.BindlessSamplerStateMemoryObjectControlState = GENX(MOCS);
> + sba.BindlessSamplerStateBaseAddressModifyEnable = true;
> + sba.BindlessSamplerStateBufferSize = 0;
> # endif
> }
>
> --
> 2.19.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Update STATE_BASE_ADDRESS length for gen11+.
Please ignore this patch, Jordan's version is the correct one.
On Wed, Oct 10, 2018 at 01:30:52PM -0700, Rafael Antognolli wrote:
> Starting in gen11, we have 3 more dwords used for Bindless Sampler State
> pointer and size.
>
> Cc: Anuj Phogat
>
> ---
> src/mesa/drivers/dri/i965/brw_misc_state.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
> b/src/mesa/drivers/dri/i965/brw_misc_state.c
> index 0895e1f2b7f..965fbb10c4d 100644
> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> @@ -688,7 +688,8 @@ brw_upload_state_base_address(struct brw_context *brw)
> * to the bottom 4GB.
> */
>uint32_t mocs_wb = devinfo->gen >= 9 ? SKL_MOCS_WB : BDW_MOCS_WB;
> - int pkt_len = devinfo->gen >= 9 ? 19 : 16;
> + const int pkt_len =
> + devinfo->gen >= 9 ? (devinfo->gen >= 11 ? 22 : 19) : 16;
>
>BEGIN_BATCH(pkt_len);
>OUT_BATCH(CMD_STATE_BASE_ADDRESS << 16 | (pkt_len - 2));
> @@ -717,6 +718,12 @@ brw_upload_state_base_address(struct brw_context *brw)
> OUT_BATCH(1);
> OUT_BATCH(0);
> OUT_BATCH(0);
> + if (devinfo->gen >= 11) {
> +/* Bindless Sampler State */
> +OUT_BATCH(0);
> +OUT_BATCH(0);
> +OUT_BATCH(0);
> + }
>}
>ADVANCE_BATCH();
> } else if (devinfo->gen >= 6) {
> --
> 2.17.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] anv/gen9+: Initialize new fields in STATE_BASE_ADDRESS
Ref: 263b584d5e4 "i965/skl: Emit extra zeros in STATE_BASE_ADDRESS on Skylake."
Signed-off-by: Jordan Justen
---
src/intel/vulkan/genX_cmd_buffer.c | 12
1 file changed, 12 insertions(+)
diff --git a/src/intel/vulkan/genX_cmd_buffer.c
b/src/intel/vulkan/genX_cmd_buffer.c
index c3a7e5c83c3..43a02f22567 100644
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -121,6 +121,18 @@ genX(cmd_buffer_emit_state_base_address)(struct
anv_cmd_buffer *cmd_buffer)
sba.IndirectObjectBufferSizeModifyEnable = true;
sba.InstructionBufferSize = 0xf;
sba.InstructionBuffersizeModifyEnable = true;
+# endif
+# if (GEN_GEN >= 9)
+ sba.BindlessSurfaceStateBaseAddress = (struct anv_address) { NULL, 0 };
+ sba.BindlessSurfaceStateMemoryObjectControlState = GENX(MOCS);
+ sba.BindlessSurfaceStateBaseAddressModifyEnable = true;
+ sba.BindlessSurfaceStateSize = 0;
+# endif
+# if (GEN_GEN >= 10)
+ sba.BindlessSamplerStateBaseAddress = (struct anv_address) { NULL, 0 };
+ sba.BindlessSamplerStateMemoryObjectControlState = GENX(MOCS);
+ sba.BindlessSamplerStateBaseAddressModifyEnable = true;
+ sba.BindlessSamplerStateBufferSize = 0;
# endif
}
--
2.19.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] i965/gen10+: Initialize new fields in STATE_BASE_ADDRESS
Ref: 263b584d5e4 "i965/skl: Emit extra zeros in STATE_BASE_ADDRESS on Skylake."
Signed-off-by: Jordan Justen
---
src/mesa/drivers/dri/i965/brw_misc_state.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
b/src/mesa/drivers/dri/i965/brw_misc_state.c
index 0895e1f2b7f..9bff2c8ac92 100644
--- a/src/mesa/drivers/dri/i965/brw_misc_state.c
+++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
@@ -688,7 +688,7 @@ brw_upload_state_base_address(struct brw_context *brw)
* to the bottom 4GB.
*/
uint32_t mocs_wb = devinfo->gen >= 9 ? SKL_MOCS_WB : BDW_MOCS_WB;
- int pkt_len = devinfo->gen >= 9 ? 19 : 16;
+ int pkt_len = devinfo->gen >= 10 ? 22 : (devinfo->gen >= 9 ? 19 : 16);
BEGIN_BATCH(pkt_len);
OUT_BATCH(CMD_STATE_BASE_ADDRESS << 16 | (pkt_len - 2));
@@ -718,6 +718,11 @@ brw_upload_state_base_address(struct brw_context *brw)
OUT_BATCH(0);
OUT_BATCH(0);
}
+ if (devinfo->gen >= 10) {
+ OUT_BATCH(1);
+ OUT_BATCH(0);
+ OUT_BATCH(0);
+ }
ADVANCE_BATCH();
} else if (devinfo->gen >= 6) {
uint8_t mocs = devinfo->gen == 7 ? GEN7_MOCS_L3 : 0;
--
2.19.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Update STATE_BASE_ADDRESS length for gen11+.
Starting in gen11, we have 3 more dwords used for Bindless Sampler State
pointer and size.
Cc: Anuj Phogat
---
src/mesa/drivers/dri/i965/brw_misc_state.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
b/src/mesa/drivers/dri/i965/brw_misc_state.c
index 0895e1f2b7f..965fbb10c4d 100644
--- a/src/mesa/drivers/dri/i965/brw_misc_state.c
+++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
@@ -688,7 +688,8 @@ brw_upload_state_base_address(struct brw_context *brw)
* to the bottom 4GB.
*/
uint32_t mocs_wb = devinfo->gen >= 9 ? SKL_MOCS_WB : BDW_MOCS_WB;
- int pkt_len = devinfo->gen >= 9 ? 19 : 16;
+ const int pkt_len =
+ devinfo->gen >= 9 ? (devinfo->gen >= 11 ? 22 : 19) : 16;
BEGIN_BATCH(pkt_len);
OUT_BATCH(CMD_STATE_BASE_ADDRESS << 16 | (pkt_len - 2));
@@ -717,6 +718,12 @@ brw_upload_state_base_address(struct brw_context *brw)
OUT_BATCH(1);
OUT_BATCH(0);
OUT_BATCH(0);
+ if (devinfo->gen >= 11) {
+/* Bindless Sampler State */
+OUT_BATCH(0);
+OUT_BATCH(0);
+OUT_BATCH(0);
+ }
}
ADVANCE_BATCH();
} else if (devinfo->gen >= 6) {
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 10/11] nir: Take call instruction into account in copy_prop_vars
On Sat, Sep 15, 2018 at 12:45 AM Caio Marcelo de Oliveira Filho <
caio.olive...@intel.com> wrote:
> Calls are not used yet (functions are inlined), but since new code is
> already taking them into account, do it here too. The convention here
> and in other places is that no writable memory is assumed to remain
> unchanged, as well as global variables.
>
> Also, explicitly state the modes affected (instead of using the
> reverse logic) in one of the apply_for_barrier_modes calls.
>
> Suggested (indirectly) by Jason.
> ---
>
> Jason suggested this for the other pass, so doing this here too.
>
> src/compiler/nir/nir_opt_copy_prop_vars.c | 17 +++--
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_copy_prop_vars.c
> b/src/compiler/nir/nir_opt_copy_prop_vars.c
> index 5276aa176d8..f58abfbb69f 100644
> --- a/src/compiler/nir/nir_opt_copy_prop_vars.c
> +++ b/src/compiler/nir/nir_opt_copy_prop_vars.c
> @@ -404,6 +404,14 @@ copy_prop_vars_block(struct copy_prop_var_state
> *state,
>copy_entry_remove(state, iter);
>
> nir_foreach_instr_safe(instr, block) {
> + if (instr->type == nir_instr_type_call) {
> + apply_barrier_for_modes(copies, nir_var_shader_out |
> + nir_var_global |
> + nir_var_shader_storage |
> + nir_var_shared);
>
As I commented on the dead writes, pass, I think locals should be included
here. Other than that,
Reviewed-by: Jason Ekstrand
> + continue;
> + }
> +
>if (instr->type != nir_instr_type_intrinsic)
> continue;
>
> @@ -411,12 +419,9 @@ copy_prop_vars_block(struct copy_prop_var_state
> *state,
>switch (intrin->intrinsic) {
>case nir_intrinsic_barrier:
>case nir_intrinsic_memory_barrier:
> - /* If we hit a barrier, we need to trash everything that may
> possibly
> - * be accessible to another thread. Locals, globals, and things
> of
> - * the like are safe, however.
> - */
> - apply_barrier_for_modes(state, ~(nir_var_local | nir_var_global |
> - nir_var_shader_in |
> nir_var_uniform));
> + apply_barrier_for_modes(copies, nir_var_shader_out |
> + nir_var_shader_storage |
> + nir_var_shared);
>
Yeah, this is better.
> break;
>
>case nir_intrinsic_emit_vertex:
> --
> 2.19.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/11] nir: Add tests for copy propagation of derefs
On Sat, Sep 15, 2018 at 12:46 AM Caio Marcelo de Oliveira Filho <
caio.olive...@intel.com> wrote:
> Also tests for removal of redundant loads, that we currently handle as
> part of the copy propagation.
> ---
> src/compiler/nir/tests/vars_tests.cpp | 300 ++
> 1 file changed, 300 insertions(+)
>
> diff --git a/src/compiler/nir/tests/vars_tests.cpp
> b/src/compiler/nir/tests/vars_tests.cpp
> index cdd2a17fe92..b1fa04b5cb9 100644
> --- a/src/compiler/nir/tests/vars_tests.cpp
> +++ b/src/compiler/nir/tests/vars_tests.cpp
> @@ -140,11 +140,131 @@ nir_imm_ivec2(nir_builder *build, int x, int y)
> }
>
> /* Allow grouping the tests while still sharing the helpers. */
> +class nir_redundant_load_vars_test : public nir_vars_test {};
> class nir_copy_prop_vars_test : public nir_vars_test {};
> class nir_dead_write_vars_test : public nir_vars_test {};
>
> } // namespace
>
> +TEST_F(nir_redundant_load_vars_test, duplicated_load)
> +{
> + /* Load a variable twice in the same block. One should be removed. */
> +
> + nir_variable *in = create_int(nir_var_shader_in, "in");
> + nir_variable **out = create_many_int(nir_var_shader_out, "out", 2);
> +
> + nir_store_var(b, out[0], nir_load_var(b, in), 1);
> + nir_store_var(b, out[1], nir_load_var(b, in), 1);
> +
> + nir_validate_shader(b->shader);
> +
> + ASSERT_EQ(count_intrinsics(nir_intrinsic_load_deref), 2);
> +
> + bool progress = nir_opt_copy_prop_vars(b->shader);
> + EXPECT_TRUE(progress);
> +
> + nir_validate_shader(b->shader);
> +
> + ASSERT_EQ(count_intrinsics(nir_intrinsic_load_deref), 1);
> +}
> +
> +TEST_F(nir_redundant_load_vars_test,
> DISABLED_duplicated_load_in_two_blocks)
> +{
> + /* Load a variable twice in different blocks. One should be removed.
> */
> +
> + nir_variable *in = create_int(nir_var_shader_in, "in");
> + nir_variable **out = create_many_int(nir_var_shader_out, "out", 2);
> +
> + nir_store_var(b, out[0], nir_load_var(b, in), 1);
> +
> + /* Forces the stores to be in different blocks. */
> + nir_pop_if(b, nir_push_if(b, nir_imm_int(b, 0)));
> +
> + nir_store_var(b, out[1], nir_load_var(b, in), 1);
> +
> + nir_validate_shader(b->shader);
> +
> + ASSERT_EQ(count_intrinsics(nir_intrinsic_load_deref), 2);
> +
> + bool progress = nir_opt_copy_prop_vars(b->shader);
> + EXPECT_TRUE(progress);
> +
> + nir_validate_shader(b->shader);
> +
> + ASSERT_EQ(count_intrinsics(nir_intrinsic_load_deref), 1);
> +}
> +
> +TEST_F(nir_redundant_load_vars_test, DISABLED_invalidate_inside_if_block)
> +{
> + /* Load variables, then write to some of then in different branches of
> the
> +* if statement. They should be invalidated accordingly.
> +*/
> +
> + nir_variable **g = create_many_int(nir_var_global, "g", 3);
> + nir_variable **out = create_many_int(nir_var_shader_out, "out", 3);
> +
> + nir_load_var(b, g[0]);
> + nir_load_var(b, g[1]);
> + nir_load_var(b, g[2]);
> +
> + nir_if *if_stmt = nir_push_if(b, nir_imm_int(b, 0));
> + nir_store_var(b, g[0], nir_imm_int(b, 10), 1);
> +
> + nir_push_else(b, if_stmt);
> + nir_store_var(b, g[1], nir_imm_int(b, 20), 1);
> +
> + nir_pop_if(b, if_stmt);
> +
> + nir_store_var(b, out[0], nir_load_var(b, g[0]), 1);
> + nir_store_var(b, out[1], nir_load_var(b, g[1]), 1);
> + nir_store_var(b, out[2], nir_load_var(b, g[2]), 1);
> +
> + nir_validate_shader(b->shader);
> +
> + bool progress = nir_opt_copy_prop_vars(b->shader);
> + EXPECT_TRUE(progress);
> +
> + /* There are 3 initial loads, plus 2 loads for the values invalidated
> +* inside the if statement.
> +*/
> + ASSERT_EQ(count_intrinsics(nir_intrinsic_load_deref), 5);
> +
> + /* We only load g[2] once. */
> + unsigned g2_load_count = 0;
> + nir_intrinsic_instr *load = NULL;
> + for (int i = 0; i < 5; i++) {
> + load = find_next_intrinsic(nir_intrinsic_load_deref, load);
> + if (nir_intrinsic_get_var(load, 0) == g[2])
> + g2_load_count++;
> + }
> + EXPECT_EQ(g2_load_count, 1);
> +}
> +
> +TEST_F(nir_redundant_load_vars_test,
> invalidate_live_load_in_the_end_of_loop)
> +{
> + /* Invalidating a load in the end of loop body will apply to the whole
> loop
> +* body.
> +*/
> +
> + nir_variable *v = create_int(nir_var_shader_storage, "v");
> +
> + nir_load_var(b, v);
> +
> + nir_loop *loop = nir_push_loop(b);
> +
> + nir_if *if_stmt = nir_push_if(b, nir_imm_int(b, 0));
> + nir_jump(b, nir_jump_break);
> + nir_pop_if(b, if_stmt);
> +
> + nir_load_var(b, v);
> + nir_store_var(b, v, nir_imm_int(b, 10), 1);
> +
> + nir_pop_loop(b, loop);
> +
> + bool progress = nir_opt_copy_prop_vars(b->shader);
> + ASSERT_FALSE(progress);
> +}
> +
> TEST_F(nir_copy_prop_vars_test, simple_copies)
> {
> nir_variable *in = create_int(nir_var_shader_in, "in");
> @@ -199,6 +319,186 @@ TEST_F(nir_copy_prop_vars_test, simple_store_load)
> }
> }
>
> +TEST_F(nir_copy_prop_vars_test, store_store_load)
> +{
Re: [Mesa-dev] [PATCH 08/11] nir: Remove handling of dead writes from copy_prop_vars
6-8 are
Reviewed-by: Jason Ekstrand
On Sat, Sep 15, 2018 at 12:45 AM Caio Marcelo de Oliveira Filho <
caio.olive...@intel.com> wrote:
> These are covered by another pass now.
> ---
> src/compiler/nir/nir_opt_copy_prop_vars.c | 84 +++
> 1 file changed, 8 insertions(+), 76 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_copy_prop_vars.c
> b/src/compiler/nir/nir_opt_copy_prop_vars.c
> index 9fecaf0eeec..5276aa176d8 100644
> --- a/src/compiler/nir/nir_opt_copy_prop_vars.c
> +++ b/src/compiler/nir/nir_opt_copy_prop_vars.c
> @@ -38,10 +38,7 @@
> * 1) Copy-propagation on variables that have indirect access. This
> includes
> * propagating from indirect stores into indirect loads.
> *
> - * 2) Dead code elimination of store_var and copy_var intrinsics based on
> - * killed destination values.
> - *
> - * 3) Removal of redundant load_deref intrinsics. We can't trust
> regular CSE
> + * 2) Removal of redundant load_deref intrinsics. We can't trust
> regular CSE
> * to do this because it isn't aware of variable writes that may
> alias the
> * value and make the former load invalid.
> *
> @@ -51,6 +48,8 @@
> * rapidly get out of hand. Fortunately, for anything that is only ever
> * accessed directly, we get SSA based copy-propagation which is extremely
> * powerful so this isn't that great a loss.
> + *
> + * Removal of dead writes to variables is handled by another pass.
> */
>
> struct value {
> @@ -64,9 +63,6 @@ struct value {
> struct copy_entry {
> struct list_head link;
>
> - nir_instr *store_instr[4];
> -
> - unsigned comps_may_be_read;
> struct value src;
>
> nir_deref_instr *dst;
> @@ -114,44 +110,6 @@ copy_entry_remove(struct copy_prop_var_state *state,
> struct copy_entry *entry)
> list_add(>link, >copy_free_list);
> }
>
> -static void
> -remove_dead_writes(struct copy_prop_var_state *state,
> - struct copy_entry *entry, unsigned write_mask)
> -{
> - /* We're overwriting another entry. Some of it's components may not
> -* have been read yet and, if that's the case, we may be able to delete
> -* some instructions but we have to be careful.
> -*/
> - unsigned dead_comps = write_mask & ~entry->comps_may_be_read;
> -
> - for (unsigned mask = dead_comps; mask;) {
> - unsigned i = u_bit_scan();
> -
> - nir_instr *instr = entry->store_instr[i];
> -
> - /* We may have already deleted it on a previous iteration */
> - if (!instr)
> - continue;
> -
> - /* See if this instr is used anywhere that it's not dead */
> - bool keep = false;
> - for (unsigned j = 0; j < 4; j++) {
> - if (entry->store_instr[j] == instr) {
> -if (dead_comps & (1 << j)) {
> - entry->store_instr[j] = NULL;
> -} else {
> - keep = true;
> -}
> - }
> - }
> -
> - if (!keep) {
> - nir_instr_remove(instr);
> - state->progress = true;
> - }
> - }
> -}
> -
> static struct copy_entry *
> lookup_entry_for_deref(struct copy_prop_var_state *state,
> nir_deref_instr *deref,
> @@ -165,16 +123,6 @@ lookup_entry_for_deref(struct copy_prop_var_state
> *state,
> return NULL;
> }
>
> -static void
> -mark_aliased_entries_as_read(struct copy_prop_var_state *state,
> - nir_deref_instr *deref, unsigned components)
> -{
> - list_for_each_entry(struct copy_entry, iter, >copies, link) {
> - if (nir_compare_derefs(iter->dst, deref) & nir_derefs_may_alias_bit)
> - iter->comps_may_be_read |= components;
> - }
> -}
> -
> static struct copy_entry *
> get_entry_and_kill_aliases(struct copy_prop_var_state *state,
> nir_deref_instr *deref,
> @@ -191,11 +139,6 @@ get_entry_and_kill_aliases(struct copy_prop_var_state
> *state,
>}
>
>nir_deref_compare_result comp = nir_compare_derefs(iter->dst,
> deref);
> - /* This is a store operation. If we completely overwrite some
> value, we
> - * want to delete any dead writes that may be present.
> - */
> - if (comp & nir_derefs_b_contains_a_bit)
> - remove_dead_writes(state, iter, write_mask);
>
>if (comp & nir_derefs_equal_bit) {
> assert(entry == NULL);
> @@ -228,25 +171,19 @@ apply_barrier_for_modes(struct copy_prop_var_state
> *state,
>
> static void
> store_to_entry(struct copy_prop_var_state *state, struct copy_entry
> *entry,
> - const struct value *value, unsigned write_mask,
> - nir_instr *store_instr)
> + const struct value *value, unsigned write_mask)
> {
> - entry->comps_may_be_read &= ~write_mask;
> if (value->is_ssa) {
>entry->src.is_ssa = true;
>/* Only overwrite the written components */
>for (unsigned i = 0; i < 4; i++) {
> - if (write_mask & (1 << i)) {
> -
Re: [Mesa-dev] [PATCH 05/11] nir: Separate dead write removal into its own pass
On Sat, Sep 15, 2018 at 12:46 AM Caio Marcelo de Oliveira Filho <
caio.olive...@intel.com> wrote:
> Instead of doing this as part of the existing copy_prop_vars pass.
>
> Separation makes easier to expand the scope of both passes to be more
> than per-block. For copy propagation, the information about valid
> copies comes from previous instructions; while the dead write removal
> depends on information from later instructions ("have any instruction
> used this deref before overwrite it?").
>
> Also change the tests to use this pass (instead of copy prop vars).
> Note that the disabled tests continue to fail, since the standalone
> pass is still per-block.
>
> v2: Remove entries from dynarray instead of marking items as
> deleted. Use foreach_reverse. (Caio)
>
> (all from Jason)
> Do not cache nir_deref_path. Not worthy for this patch.
> Clear unused writes when hitting a call instruction.
> Clean up enumeration of modes for barriers.
> Move metadata calls to the inner function.
> ---
> src/compiler/Makefile.sources | 1 +
> src/compiler/nir/meson.build | 1 +
> src/compiler/nir/nir.h | 2 +
> src/compiler/nir/nir_opt_dead_write_vars.c | 216 +
> src/compiler/nir/tests/vars_tests.cpp | 3 -
> 5 files changed, 220 insertions(+), 3 deletions(-)
> create mode 100644 src/compiler/nir/nir_opt_dead_write_vars.c
>
> diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
> index d3b06564832..b65bb9b80b9 100644
> --- a/src/compiler/Makefile.sources
> +++ b/src/compiler/Makefile.sources
> @@ -274,6 +274,7 @@ NIR_FILES = \
> nir/nir_opt_cse.c \
> nir/nir_opt_dce.c \
> nir/nir_opt_dead_cf.c \
> + nir/nir_opt_dead_write_vars.c \
> nir/nir_opt_find_array_copies.c \
> nir/nir_opt_gcm.c \
> nir/nir_opt_global_to_local.c \
> diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
> index 1a7fa2d3327..d8f65640004 100644
> --- a/src/compiler/nir/meson.build
> +++ b/src/compiler/nir/meson.build
> @@ -158,6 +158,7 @@ files_libnir = files(
>'nir_opt_cse.c',
>'nir_opt_dce.c',
>'nir_opt_dead_cf.c',
> + 'nir_opt_dead_write_vars.c',
>'nir_opt_find_array_copies.c',
>'nir_opt_gcm.c',
>'nir_opt_global_to_local.c',
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 599f469a714..80d145cac1e 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -3030,6 +3030,8 @@ bool nir_opt_dce(nir_shader *shader);
>
> bool nir_opt_dead_cf(nir_shader *shader);
>
> +bool nir_opt_dead_write_vars(nir_shader *shader);
> +
> bool nir_opt_find_array_copies(nir_shader *shader);
>
> bool nir_opt_gcm(nir_shader *shader, bool value_number);
> diff --git a/src/compiler/nir/nir_opt_dead_write_vars.c
> b/src/compiler/nir/nir_opt_dead_write_vars.c
> new file mode 100644
> index 000..5a3145875cb
> --- /dev/null
> +++ b/src/compiler/nir/nir_opt_dead_write_vars.c
> @@ -0,0 +1,216 @@
> +/*
> + * Copyright © 2018 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (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.
> + */
> +
> +#include "nir.h"
> +#include "nir_builder.h"
> +#include "nir_deref.h"
> +
> +#include "util/u_dynarray.h"
> +
> +/**
> + * Elimination of dead writes based on derefs.
> + *
> + * Dead writes are stores and copies that write to a deref, which then
> gets
> + * another write before it was used (read or sourced for a copy). Those
> + * writes can be removed since they don't affect anything.
> + *
> + * For derefs that refer to a memory area that can be read after the
> program,
> + * the last write is considered used. The presence of certain
> instructions
> + * may also cause writes to be considered used, e.g. memory barrier (in
> this case
> + * the value must be written as other thread might
Re: [Mesa-dev] [PATCH] st/va: use provided sizes and coords for vlVaGetImage
Acked-by: Christian König
Am 10.10.2018 um 21:18 schrieb Ilia Mirkin:
Reviewed-by: Ilia Mirkin
On Wed, Oct 10, 2018 at 3:12 PM wrote:
From: Boyuan Zhang
vlVaGetImage should respect the width, height, and coordinates x and y that
passed in. Therefore, pipe_box should be created with the passed in values
instead of surface width/height.
v2: add input size check, return error when size out of bounds
v3: fix the size check for vaimage
v4: add size adjustment for x and y coordinates
Signed-off-by: Boyuan Zhang
Cc: "18.2"
Reviewed-by: Leo Liu
---
src/gallium/state_trackers/va/image.c | 31 ---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/src/gallium/state_trackers/va/image.c
b/src/gallium/state_trackers/va/image.c
index 3f892c9..807fc83 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -353,6 +353,23 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface,
int x, int y,
return VA_STATUS_ERROR_INVALID_IMAGE;
}
+ if (x < 0 || y < 0) {
+ mtx_unlock(>mutex);
+ return VA_STATUS_ERROR_INVALID_PARAMETER;
+ }
+
+ if (x + width > surf->templat.width ||
+ y + height > surf->templat.height) {
+ mtx_unlock(>mutex);
+ return VA_STATUS_ERROR_INVALID_PARAMETER;
+ }
+
+ if (width > vaimage->width ||
+ height > vaimage->height) {
+ mtx_unlock(>mutex);
+ return VA_STATUS_ERROR_INVALID_PARAMETER;
+ }
+
img_buf = handle_table_get(drv->htab, vaimage->buf);
if (!img_buf) {
mtx_unlock(>mutex);
@@ -400,11 +417,19 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface,
int x, int y,
}
for (i = 0; i < vaimage->num_planes; i++) {
- unsigned width, height;
+ unsigned box_w = align(width, 2);
+ unsigned box_h = align(height, 2);
+ unsigned box_x = x & ~1;
+ unsigned box_y = y & ~1;
if (!views[i]) continue;
- vlVaVideoSurfaceSize(surf, i, , );
+ vl_video_buffer_adjust_size(_w, _h, i,
+ surf->templat.chroma_format,
+ surf->templat.interlaced);
+ vl_video_buffer_adjust_size(_x, _y, i,
+ surf->templat.chroma_format,
+ surf->templat.interlaced);
for (j = 0; j < views[i]->texture->array_size; ++j) {
- struct pipe_box box = {0, 0, j, width, height, 1};
+ struct pipe_box box = {box_x, box_y, j, box_w, box_h, 1};
struct pipe_transfer *transfer;
uint8_t *map;
map = drv->pipe->transfer_map(drv->pipe, views[i]->texture, 0,
--
2.7.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/va: use provided sizes and coords for vlVaGetImage
Reviewed-by: Ilia Mirkin
On Wed, Oct 10, 2018 at 3:12 PM wrote:
>
> From: Boyuan Zhang
>
> vlVaGetImage should respect the width, height, and coordinates x and y that
> passed in. Therefore, pipe_box should be created with the passed in values
> instead of surface width/height.
>
> v2: add input size check, return error when size out of bounds
> v3: fix the size check for vaimage
> v4: add size adjustment for x and y coordinates
>
> Signed-off-by: Boyuan Zhang
> Cc: "18.2"
> Reviewed-by: Leo Liu
> ---
> src/gallium/state_trackers/va/image.c | 31 ---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/image.c
> b/src/gallium/state_trackers/va/image.c
> index 3f892c9..807fc83 100644
> --- a/src/gallium/state_trackers/va/image.c
> +++ b/src/gallium/state_trackers/va/image.c
> @@ -353,6 +353,23 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface,
> int x, int y,
>return VA_STATUS_ERROR_INVALID_IMAGE;
> }
>
> + if (x < 0 || y < 0) {
> + mtx_unlock(>mutex);
> + return VA_STATUS_ERROR_INVALID_PARAMETER;
> + }
> +
> + if (x + width > surf->templat.width ||
> + y + height > surf->templat.height) {
> + mtx_unlock(>mutex);
> + return VA_STATUS_ERROR_INVALID_PARAMETER;
> + }
> +
> + if (width > vaimage->width ||
> + height > vaimage->height) {
> + mtx_unlock(>mutex);
> + return VA_STATUS_ERROR_INVALID_PARAMETER;
> + }
> +
> img_buf = handle_table_get(drv->htab, vaimage->buf);
> if (!img_buf) {
>mtx_unlock(>mutex);
> @@ -400,11 +417,19 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface,
> int x, int y,
> }
>
> for (i = 0; i < vaimage->num_planes; i++) {
> - unsigned width, height;
> + unsigned box_w = align(width, 2);
> + unsigned box_h = align(height, 2);
> + unsigned box_x = x & ~1;
> + unsigned box_y = y & ~1;
>if (!views[i]) continue;
> - vlVaVideoSurfaceSize(surf, i, , );
> + vl_video_buffer_adjust_size(_w, _h, i,
> + surf->templat.chroma_format,
> + surf->templat.interlaced);
> + vl_video_buffer_adjust_size(_x, _y, i,
> + surf->templat.chroma_format,
> + surf->templat.interlaced);
>for (j = 0; j < views[i]->texture->array_size; ++j) {
> - struct pipe_box box = {0, 0, j, width, height, 1};
> + struct pipe_box box = {box_x, box_y, j, box_w, box_h, 1};
> struct pipe_transfer *transfer;
> uint8_t *map;
> map = drv->pipe->transfer_map(drv->pipe, views[i]->texture, 0,
> --
> 2.7.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/va: use provided sizes and coords for vlVaGetImage
From: Boyuan Zhang
vlVaGetImage should respect the width, height, and coordinates x and y that
passed in. Therefore, pipe_box should be created with the passed in values
instead of surface width/height.
v2: add input size check, return error when size out of bounds
v3: fix the size check for vaimage
v4: add size adjustment for x and y coordinates
Signed-off-by: Boyuan Zhang
Cc: "18.2"
Reviewed-by: Leo Liu
---
src/gallium/state_trackers/va/image.c | 31 ---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/src/gallium/state_trackers/va/image.c
b/src/gallium/state_trackers/va/image.c
index 3f892c9..807fc83 100644
--- a/src/gallium/state_trackers/va/image.c
+++ b/src/gallium/state_trackers/va/image.c
@@ -353,6 +353,23 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface,
int x, int y,
return VA_STATUS_ERROR_INVALID_IMAGE;
}
+ if (x < 0 || y < 0) {
+ mtx_unlock(>mutex);
+ return VA_STATUS_ERROR_INVALID_PARAMETER;
+ }
+
+ if (x + width > surf->templat.width ||
+ y + height > surf->templat.height) {
+ mtx_unlock(>mutex);
+ return VA_STATUS_ERROR_INVALID_PARAMETER;
+ }
+
+ if (width > vaimage->width ||
+ height > vaimage->height) {
+ mtx_unlock(>mutex);
+ return VA_STATUS_ERROR_INVALID_PARAMETER;
+ }
+
img_buf = handle_table_get(drv->htab, vaimage->buf);
if (!img_buf) {
mtx_unlock(>mutex);
@@ -400,11 +417,19 @@ vlVaGetImage(VADriverContextP ctx, VASurfaceID surface,
int x, int y,
}
for (i = 0; i < vaimage->num_planes; i++) {
- unsigned width, height;
+ unsigned box_w = align(width, 2);
+ unsigned box_h = align(height, 2);
+ unsigned box_x = x & ~1;
+ unsigned box_y = y & ~1;
if (!views[i]) continue;
- vlVaVideoSurfaceSize(surf, i, , );
+ vl_video_buffer_adjust_size(_w, _h, i,
+ surf->templat.chroma_format,
+ surf->templat.interlaced);
+ vl_video_buffer_adjust_size(_x, _y, i,
+ surf->templat.chroma_format,
+ surf->templat.interlaced);
for (j = 0; j < views[i]->texture->array_size; ++j) {
- struct pipe_box box = {0, 0, j, width, height, 1};
+ struct pipe_box box = {box_x, box_y, j, box_w, box_h, 1};
struct pipe_transfer *transfer;
uint8_t *map;
map = drv->pipe->transfer_map(drv->pipe, views[i]->texture, 0,
--
2.7.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/miptree: Use enum instead of boolean.
Oops... My bad.
Reviewed-by: Jason Ekstrand
On Wed, Oct 10, 2018 at 12:42 PM Rafael Antognolli <
rafael.antogno...@intel.com> wrote:
> ISL_AUX_USAGE_NONE happens to be the same as "false", but let's do the
> right thing and use the enum.
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 708757c47b8..b0333655ad5 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -613,9 +613,10 @@ intel_miptree_access_raw(struct brw_context *brw,
> uint32_t level, uint32_t layer,
> bool write)
> {
> - intel_miptree_prepare_access(brw, mt, level, 1, layer, 1, false,
> false);
> + intel_miptree_prepare_access(brw, mt, level, 1, layer, 1,
> +ISL_AUX_USAGE_NONE, false);
> if (write)
> - intel_miptree_finish_write(brw, mt, level, layer, 1, false);
> + intel_miptree_finish_write(brw, mt, level, layer, 1,
> ISL_AUX_USAGE_NONE);
> }
>
> enum isl_aux_usage
> --
> 2.17.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 06/12] util: Add tests for fast integer division by constants
I just landed the first 6 patches as well as a fix for one intel compiler
bug uncovered by the series. You can go ahead and rebase your former
series. If you feel the need for a separate 32-bit version of
compute_fast_udiv_info, I'll happily reivew the patch.
--Jason
On Mon, Oct 8, 2018 at 11:11 AM Jason Ekstrand wrote:
> On Sat, Oct 6, 2018 at 1:52 PM Marek Olšák wrote:
>
>> With my comments addressed, patches 2 - 6 are:
>>
>> Reviewed-by: Marek Olšák
>>
>
> Thanks! Unfortunately, the tests require patch 1 so it'd be nice if that
> got review by someone. I'd also be happy to pull in someone else's more
> vetted code for large integer multiplication but I had trouble finding any
> that was liberally lisenced. Maybe I just didn't look hard enough?
>
>
>> Since I will need to compute the division terms during draw calls, I
>> may need to switch the math to uint32_t for my case (e.g. via a C++
>> template).
>>
>
> There's very little change in the 32 vs. 64-bit versions but if you're
> worried about the 64-bit arithmetic, it'd be easy enough to have a version
> that does 32-bit arithmetic and only use the 64-bit version when actually
> needed.
>
> --Jason
>
>
>> Marek
>>
>> On Sat, Oct 6, 2018 at 12:11 AM Jason Ekstrand
>> wrote:
>> >> While I generally trust rediculousfish to have done his homework, we've
>> > made some adjustments to suite the needs of mesa and it'd be good to
>> > test those. Also, there's no better place than unit tests to clearly
>> > document the different edge cases of the different methods.
>> > ---
>> > configure.ac | 1 +
>> > src/util/Makefile.am | 3 +-
>> > src/util/meson.build | 1 +
>> > src/util/tests/fast_idiv_by_const/Makefile.am | 43 ++
>> > .../fast_idiv_by_const_test.cpp | 472 ++
>> > src/util/tests/fast_idiv_by_const/meson.build | 30 ++
>> > 6 files changed, 549 insertions(+), 1 deletion(-)
>> > create mode 100644 src/util/tests/fast_idiv_by_const/Makefile.am
>> > create mode 100644
>> src/util/tests/fast_idiv_by_const/fast_idiv_by_const_test.cpp
>> > create mode 100644 src/util/tests/fast_idiv_by_const/meson.build
>> >
>> > diff --git a/configure.ac b/configure.ac
>> > index 34689826c98..7b0b2b20ba2 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -3198,6 +3198,7 @@ AC_CONFIG_FILES([Makefile
>> > src/util/tests/hash_table/Makefile
>> > src/util/tests/set/Makefile
>> > src/util/tests/string_buffer/Makefile
>> > + src/util/tests/uint_inverse/Makefile
>> > src/util/tests/vma/Makefile
>> > src/util/xmlpool/Makefile
>> > src/vulkan/Makefile])
>> > diff --git a/src/util/Makefile.am b/src/util/Makefile.am
>> > index d79f2b320be..9e633bf65d5 100644
>> > --- a/src/util/Makefile.am
>> > +++ b/src/util/Makefile.am
>> > @@ -24,7 +24,8 @@ SUBDIRS = . \
>> > tests/fast_idiv_by_const \
>> > tests/hash_table \
>> > tests/string_buffer \
>> > - tests/set
>> > + tests/set \
>> > + tests/uint_inverse
>> >
>> > if HAVE_STD_CXX11
>> > SUBDIRS += tests/vma
>> > diff --git a/src/util/meson.build b/src/util/meson.build
>> > index cdbad98e7cb..49d84c16ebe 100644
>> > --- a/src/util/meson.build
>> > +++ b/src/util/meson.build
>> > @@ -170,6 +170,7 @@ if with_tests
>> > )
>> >)
>> >
>> > + subdir('tests/fast_idiv_by_const')
>> >subdir('tests/hash_table')
>> >subdir('tests/string_buffer')
>> >subdir('tests/vma')
>> > diff --git a/src/util/tests/fast_idiv_by_const/Makefile.am
>> b/src/util/tests/fast_idiv_by_const/Makefile.am
>> > new file mode 100644
>> > index 000..1ebee09f59b
>> > --- /dev/null
>> > +++ b/src/util/tests/fast_idiv_by_const/Makefile.am
>> > @@ -0,0 +1,43 @@
>> > +# Copyright © 2018 Intel
>> > +#
>> > +# 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
>> >
Re: [Mesa-dev] [PATCH] i965/miptree: Use enum instead of boolean.
On Wed, Oct 10, 2018 at 10:41:43AM -0700, Rafael Antognolli wrote: > ISL_AUX_USAGE_NONE happens to be the same as "false", but let's do the > right thing and use the enum. > --- Reviewed-by: Caio Marcelo de Oliveira Filho I think intel_miptree_finish_depth() in intel_mipmap_tree.c might need a similar change. Caio ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/miptree: Use enum instead of boolean.
Reviewed-by: Dylan Baker
Quoting Rafael Antognolli (2018-10-10 10:41:43)
> ISL_AUX_USAGE_NONE happens to be the same as "false", but let's do the
> right thing and use the enum.
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 708757c47b8..b0333655ad5 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -613,9 +613,10 @@ intel_miptree_access_raw(struct brw_context *brw,
> uint32_t level, uint32_t layer,
> bool write)
> {
> - intel_miptree_prepare_access(brw, mt, level, 1, layer, 1, false, false);
> + intel_miptree_prepare_access(brw, mt, level, 1, layer, 1,
> +ISL_AUX_USAGE_NONE, false);
> if (write)
> - intel_miptree_finish_write(brw, mt, level, layer, 1, false);
> + intel_miptree_finish_write(brw, mt, level, layer, 1,
> ISL_AUX_USAGE_NONE);
> }
>
> enum isl_aux_usage
> --
> 2.17.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] meson: Don't allow building EGL on Windows or MacOS
Quoting Eric Engestrom (2018-10-10 10:15:35)
> On Wednesday, 2018-10-10 09:22:46 -0700, Dylan Baker wrote:
> > Quoting Eric Engestrom (2018-10-04 07:54:07)
> > > On Wednesday, 2018-10-03 11:05:36 -0700, Dylan Baker wrote:
> > > > Quoting Dylan Baker (2018-10-03 10:35:45)
> > > > > Currently mesa only supports EGL on Unix like systems, cygwin, and
> > > > > haiku. Meson should actually enforce this. This fixes the default
> > > > > build
> > > > > on MacOS.
> > > > >
> > > > > v2: - invert the condition, mark darwin and windows as not supported
> > > > > instead of trying to mark what is supported.
> > > > >
> > > > > CC: 18.2
> > > > > ---
> > > > > meson.build | 7 ++-
> > > > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/meson.build b/meson.build
> > > > > index e4b9f04949c..2894c4931ac 100644
> > > > > --- a/meson.build
> > > > > +++ b/meson.build
> > > > > @@ -306,7 +306,10 @@ endif
> > > > >
> > > > > _egl = get_option('egl')
> > > > > if _egl == 'auto'
> > > > > - with_egl = with_dri and with_shared_glapi and with_platforms
> > > > > + with_egl = (
> > > > > +not ['darwin', 'windows'].contains(host_machine.system() and
> > > >^
> > > > There's a missing brace here, I forgot to commit that change before I
> > > > sent the
> > > > patch :( I've squashed that and saved as a v3 locally.
> > > >
> > > > > +with_dri and with_shared_glapi and with_platforms
> > > > > + )
> > > > > elif _egl == 'true'
> > > > >if not with_dri
> > > > > error('EGL requires dri')
> > > > > @@ -316,6 +319,8 @@ elif _egl == 'true'
> > > > > error('No platforms specified, consider
> > > > > -Dplatforms=drm,x11,surfaceless at least')
> > > > >elif not ['disabled', 'dri'].contains(with_glx)
> > > > > error('EGL requires dri, but a GLX is being built without dri')
> > > > > + elif ['darwin', 'windows'].contains(host_machine.system())
> > > > > +error('EGL is not valid on systems that don\'t use KMS except
> > > > > Haiku.')
> > >
> > > I usually use `'''` when I need to put a `'` in the string :)
> >
> > I guess I really should update this comment to say "something like "EGL is
> > not
> > available on Windows or MacOS", since that's what the patch actually does...
> >
> > Does your review still sand with that change?
>
> Of course!
Thanks!
>
> >
> > > Reviewed-by: Eric Engestrom
> > >
> > > > >endif
> > > > >with_egl = true
> > > > > else
> > > > > --
> > > > > 2.19.0
> > > > >
signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965/miptree: Use enum instead of boolean.
ISL_AUX_USAGE_NONE happens to be the same as "false", but let's do the
right thing and use the enum.
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index 708757c47b8..b0333655ad5 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -613,9 +613,10 @@ intel_miptree_access_raw(struct brw_context *brw,
uint32_t level, uint32_t layer,
bool write)
{
- intel_miptree_prepare_access(brw, mt, level, 1, layer, 1, false, false);
+ intel_miptree_prepare_access(brw, mt, level, 1, layer, 1,
+ISL_AUX_USAGE_NONE, false);
if (write)
- intel_miptree_finish_write(brw, mt, level, layer, 1, false);
+ intel_miptree_finish_write(brw, mt, level, layer, 1, ISL_AUX_USAGE_NONE);
}
enum isl_aux_usage
--
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] meson: Don't allow building EGL on Windows or MacOS
On Wednesday, 2018-10-10 09:22:46 -0700, Dylan Baker wrote:
> Quoting Eric Engestrom (2018-10-04 07:54:07)
> > On Wednesday, 2018-10-03 11:05:36 -0700, Dylan Baker wrote:
> > > Quoting Dylan Baker (2018-10-03 10:35:45)
> > > > Currently mesa only supports EGL on Unix like systems, cygwin, and
> > > > haiku. Meson should actually enforce this. This fixes the default build
> > > > on MacOS.
> > > >
> > > > v2: - invert the condition, mark darwin and windows as not supported
> > > > instead of trying to mark what is supported.
> > > >
> > > > CC: 18.2
> > > > ---
> > > > meson.build | 7 ++-
> > > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/meson.build b/meson.build
> > > > index e4b9f04949c..2894c4931ac 100644
> > > > --- a/meson.build
> > > > +++ b/meson.build
> > > > @@ -306,7 +306,10 @@ endif
> > > >
> > > > _egl = get_option('egl')
> > > > if _egl == 'auto'
> > > > - with_egl = with_dri and with_shared_glapi and with_platforms
> > > > + with_egl = (
> > > > +not ['darwin', 'windows'].contains(host_machine.system() and
> > >^
> > > There's a missing brace here, I forgot to commit that change before I
> > > sent the
> > > patch :( I've squashed that and saved as a v3 locally.
> > >
> > > > +with_dri and with_shared_glapi and with_platforms
> > > > + )
> > > > elif _egl == 'true'
> > > >if not with_dri
> > > > error('EGL requires dri')
> > > > @@ -316,6 +319,8 @@ elif _egl == 'true'
> > > > error('No platforms specified, consider
> > > > -Dplatforms=drm,x11,surfaceless at least')
> > > >elif not ['disabled', 'dri'].contains(with_glx)
> > > > error('EGL requires dri, but a GLX is being built without dri')
> > > > + elif ['darwin', 'windows'].contains(host_machine.system())
> > > > +error('EGL is not valid on systems that don\'t use KMS except
> > > > Haiku.')
> >
> > I usually use `'''` when I need to put a `'` in the string :)
>
> I guess I really should update this comment to say "something like "EGL is not
> available on Windows or MacOS", since that's what the patch actually does...
>
> Does your review still sand with that change?
Of course!
>
> > Reviewed-by: Eric Engestrom
> >
> > > >endif
> > > >with_egl = true
> > > > else
> > > > --
> > > > 2.19.0
> > > >
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] meson: Don't allow building EGL on Windows or MacOS
Quoting Eric Engestrom (2018-10-04 07:54:07)
> On Wednesday, 2018-10-03 11:05:36 -0700, Dylan Baker wrote:
> > Quoting Dylan Baker (2018-10-03 10:35:45)
> > > Currently mesa only supports EGL on Unix like systems, cygwin, and
> > > haiku. Meson should actually enforce this. This fixes the default build
> > > on MacOS.
> > >
> > > v2: - invert the condition, mark darwin and windows as not supported
> > > instead of trying to mark what is supported.
> > >
> > > CC: 18.2
> > > ---
> > > meson.build | 7 ++-
> > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/meson.build b/meson.build
> > > index e4b9f04949c..2894c4931ac 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -306,7 +306,10 @@ endif
> > >
> > > _egl = get_option('egl')
> > > if _egl == 'auto'
> > > - with_egl = with_dri and with_shared_glapi and with_platforms
> > > + with_egl = (
> > > +not ['darwin', 'windows'].contains(host_machine.system() and
> >^
> > There's a missing brace here, I forgot to commit that change before I sent
> > the
> > patch :( I've squashed that and saved as a v3 locally.
> >
> > > +with_dri and with_shared_glapi and with_platforms
> > > + )
> > > elif _egl == 'true'
> > >if not with_dri
> > > error('EGL requires dri')
> > > @@ -316,6 +319,8 @@ elif _egl == 'true'
> > > error('No platforms specified, consider
> > > -Dplatforms=drm,x11,surfaceless at least')
> > >elif not ['disabled', 'dri'].contains(with_glx)
> > > error('EGL requires dri, but a GLX is being built without dri')
> > > + elif ['darwin', 'windows'].contains(host_machine.system())
> > > +error('EGL is not valid on systems that don\'t use KMS except
> > > Haiku.')
>
> I usually use `'''` when I need to put a `'` in the string :)
I guess I really should update this comment to say "something like "EGL is not
available on Windows or MacOS", since that's what the patch actually does...
Does your review still sand with that change?
> Reviewed-by: Eric Engestrom
>
> > >endif
> > >with_egl = true
> > > else
> > > --
> > > 2.19.0
> > >
signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/5] anv: add missing unlock in error path.
On Wed, Oct 10, 2018 at 9:47 AM Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:
> Oh dear...
>
> Reviewed-by: Lionel Landwerlin
>
> Eric, Jason : Could it be the wsi CTS test you've seen locking up forever?
>
I don't think so. It blocks in a different place. In any case, good catch!
Reviewed-by: Jason Ekstrand
> On 05/10/2018 01:00, Dave Airlie wrote:
> > From: Dave Airlie
> >
> > Not going to matter, but be consistent.
> >
> > Found by coverity
> > ---
> > src/intel/vulkan/anv_allocator.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/src/intel/vulkan/anv_allocator.c
> b/src/intel/vulkan/anv_allocator.c
> > index f62d48ae3fe..67f2f73aa11 100644
> > --- a/src/intel/vulkan/anv_allocator.c
> > +++ b/src/intel/vulkan/anv_allocator.c
> > @@ -1358,6 +1358,7 @@ anv_bo_cache_import(struct anv_device *device,
> > if ((new_flags & EXEC_OBJECT_PINNED) &&
> > (bo->bo.flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS) !=
> > (bo_flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS)) {
> > + pthread_mutex_unlock(>mutex);
> >return vk_errorf(device->instance, NULL,
> > VK_ERROR_INVALID_EXTERNAL_HANDLE,
> > "The same BO was imported on two different
> heaps");
>
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] anv: Use separate MOCS settings for external BOs
Looks good to me.
On Wed, Oct 10, 2018 at 10:52 AM Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:
> On 10/10/2018 16:34, Juan A. Suarez Romero wrote:
> > On Tue, 2018-10-02 at 16:11 -0500, Jason Ekstrand wrote:
> >> On Broadwell and above, we have to use different MOCS settings to allow
> >> the kernel to take over and disable caching when needed for external
> >> buffers. On Broadwell, this is especially important because the kernel
> >> can't disable eLLC so we have to do it in userspace. We very badly
> >> don't want to do that on everything so we need separate MOCS for
> >> external and internal BOs.
> >>
> >> In order to do this, we add an anv-specific BO flag for "external" and
> >> use that to distinguish between buffers which may be shared with other
> >> processes and/or display and those which are entirely internal. That,
> >> together with an anv_mocs_for_bo helper lets us choose the right MOCS
> >> settings for each BO use.
> >>
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99507
> >> Cc: mesa-sta...@lists.freedesktop.org
> >
> > Unfortunately this didn't apply cleanly on 18.2 branch. I've resolved the
> > trivial conflicts, but it would be good if you can check if everything is
> > correct.
> >
> > The fixed commit in 18.2 is
> >
> >
> >
> https://gitlab.freedesktop.org/mesa/mesa/commit/8927cf03bbb64d0be1fbb68f1a505b81d3e8ba26
> >
> >
> > Thanks in advance!
>
> Hi Juan,
>
> This backport looks good to me.
>
> Thanks!
>
> -
> Lionel
>
>
> >
> >
> > J.A.
> >
> >> ---
> >> src/intel/vulkan/anv_allocator.c | 12 --
> >> src/intel/vulkan/anv_batch_chain.c | 2 +-
> >> src/intel/vulkan/anv_blorp.c | 15 ++--
> >> src/intel/vulkan/anv_device.c | 9 +--
> >> src/intel/vulkan/anv_image.c | 5 ++--
> >> src/intel/vulkan/anv_intel.c | 2 +-
> >> src/intel/vulkan/anv_private.h | 38 +++---
> >> src/intel/vulkan/gen7_cmd_buffer.c | 3 ++-
> >> src/intel/vulkan/gen8_cmd_buffer.c | 3 ++-
> >> src/intel/vulkan/genX_cmd_buffer.c | 18 +++---
> >> src/intel/vulkan/genX_gpu_memcpy.c | 5 ++--
> >> src/intel/vulkan/genX_state.c | 6 +
> >> 12 files changed, 80 insertions(+), 38 deletions(-)
> >>
> >> diff --git a/src/intel/vulkan/anv_allocator.c
> b/src/intel/vulkan/anv_allocator.c
> >> index ab01d46cbeb..f62d48ae3fe 100644
> >> --- a/src/intel/vulkan/anv_allocator.c
> >> +++ b/src/intel/vulkan/anv_allocator.c
> >> @@ -1253,7 +1253,8 @@ anv_bo_cache_lookup(struct anv_bo_cache *cache,
> uint32_t gem_handle)
> >> (EXEC_OBJECT_WRITE | \
> >> EXEC_OBJECT_ASYNC | \
> >> EXEC_OBJECT_SUPPORTS_48B_ADDRESS | \
> >> -EXEC_OBJECT_PINNED)
> >> +EXEC_OBJECT_PINNED | \
> >> +ANV_BO_EXTERNAL)
> >>
> >> VkResult
> >> anv_bo_cache_alloc(struct anv_device *device,
> >> @@ -1311,6 +1312,7 @@ anv_bo_cache_import(struct anv_device *device,
> >> struct anv_bo **bo_out)
> >> {
> >> assert(bo_flags == (bo_flags & ANV_BO_CACHE_SUPPORTED_FLAGS));
> >> + assert(bo_flags & ANV_BO_EXTERNAL);
> >>
> >> pthread_mutex_lock(>mutex);
> >>
> >> @@ -1327,7 +1329,7 @@ anv_bo_cache_import(struct anv_device *device,
> >> * client has imported a BO twice in different ways and they
> get what
> >> * they have coming.
> >> */
> >> - uint64_t new_flags = 0;
> >> + uint64_t new_flags = ANV_BO_EXTERNAL;
> >> new_flags |= (bo->bo.flags | bo_flags) & EXEC_OBJECT_WRITE;
> >> new_flags |= (bo->bo.flags & bo_flags) & EXEC_OBJECT_ASYNC;
> >> new_flags |= (bo->bo.flags & bo_flags) &
> EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> >> @@ -1411,6 +1413,12 @@ anv_bo_cache_export(struct anv_device *device,
> >> assert(anv_bo_cache_lookup(cache, bo_in->gem_handle) == bo_in);
> >> struct anv_cached_bo *bo = (struct anv_cached_bo *)bo_in;
> >>
> >> + /* This BO must have been flagged external in order for us to be
> able
> >> +* to export it. This is done based on external options passed into
> >> +* anv_AllocateMemory.
> >> +*/
> >> + assert(bo->bo.flags & ANV_BO_EXTERNAL);
> >> +
> >> int fd = anv_gem_handle_to_fd(device, bo->bo.gem_handle);
> >> if (fd < 0)
> >> return vk_error(VK_ERROR_TOO_MANY_OBJECTS);
> >> diff --git a/src/intel/vulkan/anv_batch_chain.c
> b/src/intel/vulkan/anv_batch_chain.c
> >> index 0f7c8325ea4..3e13553ac18 100644
> >> --- a/src/intel/vulkan/anv_batch_chain.c
> >> +++ b/src/intel/vulkan/anv_batch_chain.c
> >> @@ -1088,7 +1088,7 @@ anv_execbuf_add_bo(struct anv_execbuf *exec,
> >> obj->relocs_ptr = 0;
> >> obj->alignment = 0;
> >> obj->offset = bo->offset;
> >> - obj->flags = bo->flags | extra_flags;
> >> + obj->flags = (bo->flags & ~ANV_BO_FLAG_MASK) | extra_flags;
> >> obj->rsvd1 = 0;
> >> obj->rsvd2 = 0;
> >> }
> >> diff --git a/src/intel/vulkan/anv_blorp.c
Re: [Mesa-dev] [Mesa-stable] [PATCH] anv: Use separate MOCS settings for external BOs
On 10/10/2018 16:34, Juan A. Suarez Romero wrote:
On Tue, 2018-10-02 at 16:11 -0500, Jason Ekstrand wrote:
On Broadwell and above, we have to use different MOCS settings to allow
the kernel to take over and disable caching when needed for external
buffers. On Broadwell, this is especially important because the kernel
can't disable eLLC so we have to do it in userspace. We very badly
don't want to do that on everything so we need separate MOCS for
external and internal BOs.
In order to do this, we add an anv-specific BO flag for "external" and
use that to distinguish between buffers which may be shared with other
processes and/or display and those which are entirely internal. That,
together with an anv_mocs_for_bo helper lets us choose the right MOCS
settings for each BO use.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99507
Cc: mesa-sta...@lists.freedesktop.org
Unfortunately this didn't apply cleanly on 18.2 branch. I've resolved the
trivial conflicts, but it would be good if you can check if everything is
correct.
The fixed commit in 18.2 is
https://gitlab.freedesktop.org/mesa/mesa/commit/8927cf03bbb64d0be1fbb68f1a505b81d3e8ba26
Thanks in advance!
Hi Juan,
This backport looks good to me.
Thanks!
-
Lionel
J.A.
---
src/intel/vulkan/anv_allocator.c | 12 --
src/intel/vulkan/anv_batch_chain.c | 2 +-
src/intel/vulkan/anv_blorp.c | 15 ++--
src/intel/vulkan/anv_device.c | 9 +--
src/intel/vulkan/anv_image.c | 5 ++--
src/intel/vulkan/anv_intel.c | 2 +-
src/intel/vulkan/anv_private.h | 38 +++---
src/intel/vulkan/gen7_cmd_buffer.c | 3 ++-
src/intel/vulkan/gen8_cmd_buffer.c | 3 ++-
src/intel/vulkan/genX_cmd_buffer.c | 18 +++---
src/intel/vulkan/genX_gpu_memcpy.c | 5 ++--
src/intel/vulkan/genX_state.c | 6 +
12 files changed, 80 insertions(+), 38 deletions(-)
diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_allocator.c
index ab01d46cbeb..f62d48ae3fe 100644
--- a/src/intel/vulkan/anv_allocator.c
+++ b/src/intel/vulkan/anv_allocator.c
@@ -1253,7 +1253,8 @@ anv_bo_cache_lookup(struct anv_bo_cache *cache, uint32_t
gem_handle)
(EXEC_OBJECT_WRITE | \
EXEC_OBJECT_ASYNC | \
EXEC_OBJECT_SUPPORTS_48B_ADDRESS | \
-EXEC_OBJECT_PINNED)
+EXEC_OBJECT_PINNED | \
+ANV_BO_EXTERNAL)
VkResult
anv_bo_cache_alloc(struct anv_device *device,
@@ -1311,6 +1312,7 @@ anv_bo_cache_import(struct anv_device *device,
struct anv_bo **bo_out)
{
assert(bo_flags == (bo_flags & ANV_BO_CACHE_SUPPORTED_FLAGS));
+ assert(bo_flags & ANV_BO_EXTERNAL);
pthread_mutex_lock(>mutex);
@@ -1327,7 +1329,7 @@ anv_bo_cache_import(struct anv_device *device,
* client has imported a BO twice in different ways and they get what
* they have coming.
*/
- uint64_t new_flags = 0;
+ uint64_t new_flags = ANV_BO_EXTERNAL;
new_flags |= (bo->bo.flags | bo_flags) & EXEC_OBJECT_WRITE;
new_flags |= (bo->bo.flags & bo_flags) & EXEC_OBJECT_ASYNC;
new_flags |= (bo->bo.flags & bo_flags) &
EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
@@ -1411,6 +1413,12 @@ anv_bo_cache_export(struct anv_device *device,
assert(anv_bo_cache_lookup(cache, bo_in->gem_handle) == bo_in);
struct anv_cached_bo *bo = (struct anv_cached_bo *)bo_in;
+ /* This BO must have been flagged external in order for us to be able
+* to export it. This is done based on external options passed into
+* anv_AllocateMemory.
+*/
+ assert(bo->bo.flags & ANV_BO_EXTERNAL);
+
int fd = anv_gem_handle_to_fd(device, bo->bo.gem_handle);
if (fd < 0)
return vk_error(VK_ERROR_TOO_MANY_OBJECTS);
diff --git a/src/intel/vulkan/anv_batch_chain.c
b/src/intel/vulkan/anv_batch_chain.c
index 0f7c8325ea4..3e13553ac18 100644
--- a/src/intel/vulkan/anv_batch_chain.c
+++ b/src/intel/vulkan/anv_batch_chain.c
@@ -1088,7 +1088,7 @@ anv_execbuf_add_bo(struct anv_execbuf *exec,
obj->relocs_ptr = 0;
obj->alignment = 0;
obj->offset = bo->offset;
- obj->flags = bo->flags | extra_flags;
+ obj->flags = (bo->flags & ~ANV_BO_FLAG_MASK) | extra_flags;
obj->rsvd1 = 0;
obj->rsvd2 = 0;
}
diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
index fa7936d0981..29ed6b2ee35 100644
--- a/src/intel/vulkan/anv_blorp.c
+++ b/src/intel/vulkan/anv_blorp.c
@@ -156,7 +156,7 @@ get_blorp_surf_for_anv_buffer(struct anv_device *device,
.addr = {
.buffer = buffer->address.bo,
.offset = buffer->address.offset + offset,
- .mocs = device->default_mocs,
+ .mocs = anv_mocs_for_bo(device, buffer->address.bo),
},
};
@@ -209,7 +209,7 @@ get_blorp_surf_for_anv_image(const struct anv_device *device,
.addr = {
.buffer = image->planes[plane].address.bo,
.offset =
Re: [Mesa-dev] [Mesa-stable] [PATCH] glsl: ignore trailing whitespace when define redefined
On Wed, 2018-10-10 at 11:03 +1100, Timothy Arceri wrote:
> The Nvidia/AMD binary drivers allow this, as does GCC.
>
> This fixes shader compilation issues in the latest update of
> No Mans Sky.
>
> Cc: mesa-sta...@lists.freedesktop.org
This patch landed in upstream without the CC: mesa-stable.
I guess this was done on purpose, and that the patch is not intended to be
applied in 18.2. So I'll ignore it. If not, please, let me know.
Thanks.
J.A.
> ---
> src/compiler/glsl/glcpp/glcpp-parse.y | 14 ++
> .../glsl/glcpp/tests/122-redefine-whitespace.c | 4
> .../glcpp/tests/122-redefine-whitespace.c.expected | 10 +++---
> 3 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y
> b/src/compiler/glsl/glcpp/glcpp-parse.y
> index 4be5cfa3d54..1c095cb66f9 100644
> --- a/src/compiler/glsl/glcpp/glcpp-parse.y
> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y
> @@ -1074,6 +1074,20 @@ _token_list_equal_ignoring_space(token_list_t *a,
> token_list_t *b)
>
> while (1)
> {
> + if (node_a == NULL && node_b == NULL)
> + break;
> +
> + /* Ignore trailing whitespace */
> + if (node_a == NULL && node_b->token->type == SPACE) {
> + while (node_b && node_b->token->type == SPACE)
> +node_b = node_b->next;
> + }
> +
> + if (node_b == NULL && node_a->token->type == SPACE) {
> + while (node_a && node_a->token->type == SPACE)
> +node_a = node_a->next;
> + }
> +
>if (node_a == NULL && node_b == NULL)
> break;
>
> diff --git a/src/compiler/glsl/glcpp/tests/122-redefine-whitespace.c
> b/src/compiler/glsl/glcpp/tests/122-redefine-whitespace.c
> index ae7ea09f67e..2b084e0960a 100644
> --- a/src/compiler/glsl/glcpp/tests/122-redefine-whitespace.c
> +++ b/src/compiler/glsl/glcpp/tests/122-redefine-whitespace.c
> @@ -2,6 +2,7 @@
> #define TWO ( 1+1 )
> #define FOUR (2 + 2)
> #define SIX (3 + 3)
> +#define EIGHT (8 + 8)
>
> /* Redefinitions with whitespace in same places, but different amounts, (so
> no
> * error). */
> @@ -9,6 +10,9 @@
> #define FOUR(2 + 2)
> #define SIX (3/*comment is whitespace*/+ /* collapsed */ /* to */ /* one
> */ /* space */ 3)
>
> +/* Trailing whitespace (no error) */
> +#define EIGHT (8 + 8)
> +
> /* Redefinitions with whitespace in different places. Each of these should
> * trigger an error. */
> #define TWO (1 + 1)
> diff --git a/src/compiler/glsl/glcpp/tests/122-redefine-whitespace.c.expected
> b/src/compiler/glsl/glcpp/tests/122-redefine-whitespace.c.expected
> index 602bdef94c2..766849e34a9 100644
> --- a/src/compiler/glsl/glcpp/tests/122-redefine-whitespace.c.expected
> +++ b/src/compiler/glsl/glcpp/tests/122-redefine-whitespace.c.expected
> @@ -1,14 +1,15 @@
> -0:14(9): preprocessor error: Redefinition of macro TWO
> +0:18(9): preprocessor error: Redefinition of macro TWO
>
> -0:15(9): preprocessor error: Redefinition of macro FOUR
> +0:19(9): preprocessor error: Redefinition of macro FOUR
>
> -0:16(9): preprocessor error: Redefinition of macro SIX
> +0:20(9): preprocessor error: Redefinition of macro SIX
>
>
>
>
>
>
> +
>
>
>
> @@ -18,5 +19,8 @@
>
>
>
> +
> +
> +
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] anv: Use separate MOCS settings for external BOs
On Tue, 2018-10-02 at 16:11 -0500, Jason Ekstrand wrote:
> On Broadwell and above, we have to use different MOCS settings to allow
> the kernel to take over and disable caching when needed for external
> buffers. On Broadwell, this is especially important because the kernel
> can't disable eLLC so we have to do it in userspace. We very badly
> don't want to do that on everything so we need separate MOCS for
> external and internal BOs.
>
> In order to do this, we add an anv-specific BO flag for "external" and
> use that to distinguish between buffers which may be shared with other
> processes and/or display and those which are entirely internal. That,
> together with an anv_mocs_for_bo helper lets us choose the right MOCS
> settings for each BO use.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99507
> Cc: mesa-sta...@lists.freedesktop.org
Unfortunately this didn't apply cleanly on 18.2 branch. I've resolved the
trivial conflicts, but it would be good if you can check if everything is
correct.
The fixed commit in 18.2 is
https://gitlab.freedesktop.org/mesa/mesa/commit/8927cf03bbb64d0be1fbb68f1a505b81d3e8ba26
Thanks in advance!
J.A.
> ---
> src/intel/vulkan/anv_allocator.c | 12 --
> src/intel/vulkan/anv_batch_chain.c | 2 +-
> src/intel/vulkan/anv_blorp.c | 15 ++--
> src/intel/vulkan/anv_device.c | 9 +--
> src/intel/vulkan/anv_image.c | 5 ++--
> src/intel/vulkan/anv_intel.c | 2 +-
> src/intel/vulkan/anv_private.h | 38 +++---
> src/intel/vulkan/gen7_cmd_buffer.c | 3 ++-
> src/intel/vulkan/gen8_cmd_buffer.c | 3 ++-
> src/intel/vulkan/genX_cmd_buffer.c | 18 +++---
> src/intel/vulkan/genX_gpu_memcpy.c | 5 ++--
> src/intel/vulkan/genX_state.c | 6 +
> 12 files changed, 80 insertions(+), 38 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_allocator.c
> b/src/intel/vulkan/anv_allocator.c
> index ab01d46cbeb..f62d48ae3fe 100644
> --- a/src/intel/vulkan/anv_allocator.c
> +++ b/src/intel/vulkan/anv_allocator.c
> @@ -1253,7 +1253,8 @@ anv_bo_cache_lookup(struct anv_bo_cache *cache,
> uint32_t gem_handle)
> (EXEC_OBJECT_WRITE | \
> EXEC_OBJECT_ASYNC | \
> EXEC_OBJECT_SUPPORTS_48B_ADDRESS | \
> -EXEC_OBJECT_PINNED)
> +EXEC_OBJECT_PINNED | \
> +ANV_BO_EXTERNAL)
>
> VkResult
> anv_bo_cache_alloc(struct anv_device *device,
> @@ -1311,6 +1312,7 @@ anv_bo_cache_import(struct anv_device *device,
> struct anv_bo **bo_out)
> {
> assert(bo_flags == (bo_flags & ANV_BO_CACHE_SUPPORTED_FLAGS));
> + assert(bo_flags & ANV_BO_EXTERNAL);
>
> pthread_mutex_lock(>mutex);
>
> @@ -1327,7 +1329,7 @@ anv_bo_cache_import(struct anv_device *device,
> * client has imported a BO twice in different ways and they get what
> * they have coming.
> */
> - uint64_t new_flags = 0;
> + uint64_t new_flags = ANV_BO_EXTERNAL;
>new_flags |= (bo->bo.flags | bo_flags) & EXEC_OBJECT_WRITE;
>new_flags |= (bo->bo.flags & bo_flags) & EXEC_OBJECT_ASYNC;
>new_flags |= (bo->bo.flags & bo_flags) &
> EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> @@ -1411,6 +1413,12 @@ anv_bo_cache_export(struct anv_device *device,
> assert(anv_bo_cache_lookup(cache, bo_in->gem_handle) == bo_in);
> struct anv_cached_bo *bo = (struct anv_cached_bo *)bo_in;
>
> + /* This BO must have been flagged external in order for us to be able
> +* to export it. This is done based on external options passed into
> +* anv_AllocateMemory.
> +*/
> + assert(bo->bo.flags & ANV_BO_EXTERNAL);
> +
> int fd = anv_gem_handle_to_fd(device, bo->bo.gem_handle);
> if (fd < 0)
>return vk_error(VK_ERROR_TOO_MANY_OBJECTS);
> diff --git a/src/intel/vulkan/anv_batch_chain.c
> b/src/intel/vulkan/anv_batch_chain.c
> index 0f7c8325ea4..3e13553ac18 100644
> --- a/src/intel/vulkan/anv_batch_chain.c
> +++ b/src/intel/vulkan/anv_batch_chain.c
> @@ -1088,7 +1088,7 @@ anv_execbuf_add_bo(struct anv_execbuf *exec,
>obj->relocs_ptr = 0;
>obj->alignment = 0;
>obj->offset = bo->offset;
> - obj->flags = bo->flags | extra_flags;
> + obj->flags = (bo->flags & ~ANV_BO_FLAG_MASK) | extra_flags;
>obj->rsvd1 = 0;
>obj->rsvd2 = 0;
> }
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index fa7936d0981..29ed6b2ee35 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -156,7 +156,7 @@ get_blorp_surf_for_anv_buffer(struct anv_device *device,
>.addr = {
> .buffer = buffer->address.bo,
> .offset = buffer->address.offset + offset,
> - .mocs = device->default_mocs,
> + .mocs = anv_mocs_for_bo(device, buffer->address.bo),
>},
> };
>
> @@ -209,7 +209,7 @@ get_blorp_surf_for_anv_image(const struct anv_device
> *device,
>.addr = {
> .buffer =
[Mesa-dev] [Bug 108312] Failed to allocate buffer on big endian machine - buffer size not byte swapped
https://bugs.freedesktop.org/show_bug.cgi?id=108312 --- Comment #1 from Bas Vermeulen --- I've tracked it down to src/gallium/drivers/radeonsi/si_compute.c, functions si_switch_compute_shader and/or si_launch_grid, where amd_kernel_code_t is available in just little endian. A big endian system will read all the values inside the amd_kernel_code_t wrong (amd_kernel_code_t uses little endian). -- 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 5/5] anv: add missing unlock in error path.
Oh dear...
Reviewed-by: Lionel Landwerlin
Eric, Jason : Could it be the wsi CTS test you've seen locking up forever?
On 05/10/2018 01:00, Dave Airlie wrote:
From: Dave Airlie
Not going to matter, but be consistent.
Found by coverity
---
src/intel/vulkan/anv_allocator.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/intel/vulkan/anv_allocator.c b/src/intel/vulkan/anv_allocator.c
index f62d48ae3fe..67f2f73aa11 100644
--- a/src/intel/vulkan/anv_allocator.c
+++ b/src/intel/vulkan/anv_allocator.c
@@ -1358,6 +1358,7 @@ anv_bo_cache_import(struct anv_device *device,
if ((new_flags & EXEC_OBJECT_PINNED) &&
(bo->bo.flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS) !=
(bo_flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS)) {
+ pthread_mutex_unlock(>mutex);
return vk_errorf(device->instance, NULL,
VK_ERROR_INVALID_EXTERNAL_HANDLE,
"The same BO was imported on two different heaps");
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] nir: fix compacting varyings when XFB outputs are present
We shouldn't try to compact any varyings known as always
active IO, especially XFB outputs. For example, if one
component of an xfb output is also used as input varying
in the next stage, it shouldn't be compacted.
This small helper just marks all XFB varyings as always_active_io
in the consumer to not compact them.
v2: add a little helper
Signed-off-by: Samuel Pitoiset
---
src/compiler/nir/nir_linking_helpers.c | 33 ++
1 file changed, 33 insertions(+)
diff --git a/src/compiler/nir/nir_linking_helpers.c
b/src/compiler/nir/nir_linking_helpers.c
index 88014e9a1d..433729bd79 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -480,6 +480,36 @@ compact_components(nir_shader *producer, nir_shader
*consumer, uint8_t *comps,
>info.outputs_read);
}
+/* Mark XFB varyings as always_active_io in the consumer to not compact them.
*/
+static void
+link_xfb_varyings(nir_shader *producer, nir_shader *consumer)
+{
+ nir_variable *input_vars[32] = {};
+
+ nir_foreach_variable(var, >inputs) {
+ if (var->data.location >= VARYING_SLOT_VAR0 &&
+ var->data.location - VARYING_SLOT_VAR0 < 32) {
+
+ unsigned location = var->data.location - VARYING_SLOT_VAR0;
+ input_vars[location] = var;
+ }
+ }
+
+ nir_foreach_variable(var, >outputs) {
+ if (var->data.location >= VARYING_SLOT_VAR0 &&
+ var->data.location - VARYING_SLOT_VAR0 < 32) {
+
+ if (!var->data.explicit_xfb_buffer)
+continue;
+
+ unsigned location = var->data.location - VARYING_SLOT_VAR0;
+ if (input_vars[location]) {
+input_vars[location]->data.always_active_io = true;
+ }
+ }
+ }
+}
+
/* We assume that this has been called more-or-less directly after
* remove_unused_varyings. At this point, all of the varyings that we
* aren't going to be using have been completely removed and the
@@ -501,6 +531,9 @@ nir_compact_varyings(nir_shader *producer, nir_shader
*consumer,
uint8_t interp_type[32] = {0};
uint8_t interp_loc[32] = {0};
+ if (producer->info.has_transform_feedback_varyings)
+ link_xfb_varyings(producer, consumer);
+
get_slot_component_masks_and_interp_types(>outputs, comps,
interp_type, interp_loc,
producer->info.stage,
--
2.19.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] radv: add a workaround for a VGT hang with prim restart and strips
Otherwise, Yakuza and The Evil Within hang the GPU with DXVK.
Suggested by Marek.
Cc: mesa-sta...@lists.freedesktop.org
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_pipeline.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index b39e8b62bb6..84f658f08c4 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -3411,6 +3411,17 @@ radv_compute_ia_multi_vgt_param_helpers(struct
radv_pipeline *pipeline,
}
}
+ /* Workaround for a VGT hang when strip primitive types are used with
+* primitive restart.
+*/
+ if (pipeline->graphics.prim_restart_enable &&
+ (prim == V_008958_DI_PT_LINESTRIP ||
+prim == V_008958_DI_PT_TRISTRIP ||
+prim == V_008958_DI_PT_LINESTRIP_ADJ ||
+prim == V_008958_DI_PT_TRISTRIP_ADJ)) {
+ ia_multi_vgt_param.partial_vs_wave = true;
+ }
+
ia_multi_vgt_param.base =
S_028AA8_PRIMGROUP_SIZE(ia_multi_vgt_param.primgroup_size - 1) |
/* The following field was moved to VGT_SHADER_STAGES_EN in
GFX9. */
--
2.19.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Check the subroutine associated functions names
Reviewed-by: Tapani Pälli
On 10/9/18 7:09 PM, Vadym Shovkoplias wrote:
Adding compile time check for subroutine functions with
the same names. Similar check for intrastage linking was already
landed in commit 5f0567a4f60.
From Section 6.1.2 (Subroutines) of the GLSL 4.00 specification
"A program will fail to compile or link if any shader
or stage contains two or more functions with the same
name if the name is associated with a subroutine type."
Fixes:
* no-overloads.vert
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108109
Signed-off-by: Vadym Shovkoplias
---
src/compiler/glsl/ast_to_hir.cpp | 36
1 file changed, 36 insertions(+)
diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 1082d6c91c..6790f00526 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -66,6 +66,9 @@ using namespace ir_builder;
static void
detect_conflicting_assignments(struct _mesa_glsl_parse_state *state,
exec_list *instructions);
+static void
+verify_subroutine_associated_funcs(struct _mesa_glsl_parse_state *state);
+
static void
remove_per_vertex_blocks(exec_list *instructions,
_mesa_glsl_parse_state *state, ir_variable_mode
mode);
@@ -155,6 +158,7 @@ _mesa_ast_to_hir(exec_list *instructions, struct
_mesa_glsl_parse_state *state)
foreach_list_typed (ast_node, ast, link, & state->translation_unit)
ast->hir(instructions, state);
+ verify_subroutine_associated_funcs(state);
detect_recursion_unlinked(state, instructions);
detect_conflicting_assignments(state, instructions);
@@ -8684,6 +8688,38 @@ detect_conflicting_assignments(struct _mesa_glsl_parse_state *state,
}
}
+static void
+verify_subroutine_associated_funcs(struct _mesa_glsl_parse_state *state)
+{
+ YYLTYPE loc;
+ memset(, 0, sizeof(loc));
+
+ /* Section 6.1.2 (Subroutines) of the GLSL 4.00 spec says:
+*
+* "A program will fail to compile or link if any shader
+*or stage contains two or more functions with the same
+*name if the name is associated with a subroutine type."
+*/
+
+ for (int i = 0; i < state->num_subroutines; i++) {
+ unsigned definitions = 0;
+ ir_function *fn = state->subroutines[i];
+ /* Calculate number of function definitions with the same name */
+ foreach_in_list(ir_function_signature, sig, >signatures) {
+ if (sig->is_defined) {
+if (++definitions > 1) {
+ _mesa_glsl_error(, state,
+ "%s shader contains two or more function "
+ "definitions with name `%s', which is "
+ "associated with a subroutine type.\n",
+ _mesa_shader_stage_to_string(state->stage),
+ fn->name);
+ return;
+}
+ }
+ }
+ }
+}
static void
remove_per_vertex_blocks(exec_list *instructions,
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl/linker: Change the format of spec quotation
Reviewed-by: Tapani Pälli
On 10/10/18 1:51 PM, Vadym Shovkoplias wrote:
Also there is no "OpenGL ES Shading Language 4.00" spec,
so change it to GLSL 4.00 spec.
Signed-off-by: Vadym Shovkoplias
---
src/compiler/glsl/linker.cpp | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 2f4c886054..7db34ebf95 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -4648,12 +4648,11 @@ verify_subroutine_associated_funcs(struct
gl_shader_program *prog)
gl_program *p = prog->_LinkedShaders[i]->Program;
glsl_symbol_table *symbols = prog->_LinkedShaders[i]->symbols;
- /*
- * From OpenGL ES Shading Language 4.00 specification
- * (6.1.2 Subroutines):
- * "A program will fail to compile or link if any shader
- * or stage contains two or more functions with the same
- * name if the name is associated with a subroutine type."
+ /* Section 6.1.2 (Subroutines) of the GLSL 4.00 spec says:
+ *
+ * "A program will fail to compile or link if any shader
+ *or stage contains two or more functions with the same
+ *name if the name is associated with a subroutine type."
*/
for (unsigned j = 0; j < p->sh.NumSubroutineFunctions; j++) {
unsigned definitions = 0;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 100960] Special block from Minecraft mod rendered out of place
https://bugs.freedesktop.org/show_bug.cgi?id=100960 --- Comment #16 from Sergii Romantsov --- Created attachment 141974 --> https://bugs.freedesktop.org/attachment.cgi?id=141974=edit zero-vector-rotate.trace.tar.gz -- 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 100960] Special block from Minecraft mod rendered out of place
https://bugs.freedesktop.org/show_bug.cgi?id=100960 --- Comment #15 from Sergii Romantsov --- Created attachment 141973 --> https://bugs.freedesktop.org/attachment.cgi?id=141973=edit Mesa_zero-vector-rotate.png -- 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 100960] Special block from Minecraft mod rendered out of place
https://bugs.freedesktop.org/show_bug.cgi?id=100960 --- Comment #14 from Sergii Romantsov --- Thanks, Fabian. Your image looks the same as mesa with patch https://patchwork.freedesktop.org/patch/249043/ v3 Proposed additionally piglit-test: https://patchwork.freedesktop.org/patch/255629/ Apitrace to check on other platforms: zero-vector-rotate.trace.tar.gz The final version of test gives image: Mesa_zero-vector-rotate.png -- 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] [PATCH] glsl/linker: Change the format of spec quotation
Also there is no "OpenGL ES Shading Language 4.00" spec,
so change it to GLSL 4.00 spec.
Signed-off-by: Vadym Shovkoplias
---
src/compiler/glsl/linker.cpp | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 2f4c886054..7db34ebf95 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -4648,12 +4648,11 @@ verify_subroutine_associated_funcs(struct
gl_shader_program *prog)
gl_program *p = prog->_LinkedShaders[i]->Program;
glsl_symbol_table *symbols = prog->_LinkedShaders[i]->symbols;
- /*
- * From OpenGL ES Shading Language 4.00 specification
- * (6.1.2 Subroutines):
- * "A program will fail to compile or link if any shader
- * or stage contains two or more functions with the same
- * name if the name is associated with a subroutine type."
+ /* Section 6.1.2 (Subroutines) of the GLSL 4.00 spec says:
+ *
+ * "A program will fail to compile or link if any shader
+ *or stage contains two or more functions with the same
+ *name if the name is associated with a subroutine type."
*/
for (unsigned j = 0; j < p->sh.NumSubroutineFunctions; j++) {
unsigned definitions = 0;
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3] mesa: rotation of 0-vector
Hello,
i've updated a test for the latest version of mesa-patch:
https://patchwork.freedesktop.org/patch/255629/
The image should looks for any driver (Windows, Nvidia, mesa) like this:
[image: Screenshot from 2018-10-10 13-47-56.png]
On Fri, Sep 21, 2018 at 4:21 PM Sergii Romantsov
wrote:
> Specification doesn't define behaviour for rotation of 0-vector.
> In https://github.com/KhronosGroup/OpenGL-API/issues/41 said that
> behaviour is undefined and agreed that it would be fine for
> implementation to do something useful for this.
> Windows and Nvidia drivers have a workaround for that.
> For compatibility proposed optimized version of computations.
> Specification defines a formula of rotation (see "OpenGL 4.6
> (Compatibility Profile) - May 14, 2018", paragraph "12.1.1 Matrices").
>
> Optimized formula will look so:
>
>R = cos(angle) * I
>
> That is equavalent to logic that magnitude of (0,0,0)-vector is 1.0.
>
> -v2: logic moved to _math_matrix_rotate
>
> -v3: general optimization of computations
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100960
> Signed-off-by: Sergii Romantsov
> ---
> src/mesa/math/m_matrix.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/src/mesa/math/m_matrix.c b/src/mesa/math/m_matrix.c
> index 57a4953..3eef122 100644
> --- a/src/mesa/math/m_matrix.c
> +++ b/src/mesa/math/m_matrix.c
> @@ -824,6 +824,18 @@ _math_matrix_rotate( GLmatrix *mat,
> M(1,0) = s;
> }
> }
> + else {
> +/* https://bugs.freedesktop.org/show_bug.cgi?id=100960
> + * https://github.com/KhronosGroup/OpenGL-API/issues/41
> + * Here we will treat magnitude as 1.0 if it really 0.0.
> + * So that is kind of workaround for empty-vectors to have
> + * compatibility with Windows and Nvidia drivers.
> + */
> +optimized = GL_TRUE;
> +M(0,0) = c;
> +M(1,1) = c;
> +M(2,2) = c;
> + }
>}
>else if (z == 0.0F) {
> optimized = GL_TRUE;
> --
> 2.7.4
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: Add a pass for gathering transform feedback info
After the experience of using it, and reading it, the patch LGTM. I
still have some issues while trying to use this pass, but they are
mostly glslang bugs, or things that I suspect is a problem on a
different pass or on our linking code, that are better to talk in a
different thread.
So this patch:
Reviewed-by: Alejandro Piñeiro
On 05/10/18 16:13, Jason Ekstrand wrote:
> This is different from the GL_ARB_spirv pass because it generates a much
> simpler data structure that isn't tied to OpenGL and mtypes.h.
> ---
> src/compiler/Makefile.sources | 4 +-
> src/compiler/nir/meson.build | 2 +
> src/compiler/nir/nir_gather_xfb_info.c | 150 +
> src/compiler/nir/nir_xfb_info.h| 59 ++
> 4 files changed, 214 insertions(+), 1 deletion(-)
> create mode 100644 src/compiler/nir/nir_gather_xfb_info.c
> create mode 100644 src/compiler/nir/nir_xfb_info.h
>
> diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
> index d3b06564832..46ed5e47b46 100644
> --- a/src/compiler/Makefile.sources
> +++ b/src/compiler/Makefile.sources
> @@ -216,6 +216,7 @@ NIR_FILES = \
> nir/nir_format_convert.h \
> nir/nir_from_ssa.c \
> nir/nir_gather_info.c \
> + nir/nir_gather_xfb_info.c \
> nir/nir_gs_count_vertices.c \
> nir/nir_inline_functions.c \
> nir/nir_instr_set.c \
> @@ -307,7 +308,8 @@ NIR_FILES = \
> nir/nir_validate.c \
> nir/nir_vla.h \
> nir/nir_worklist.c \
> - nir/nir_worklist.h
> + nir/nir_worklist.h \
> + nir/nir_xfb_info.h
>
> SPIRV_GENERATED_FILES = \
> spirv/spirv_info.c \
> diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
> index 090aa7a628f..b416e561eb0 100644
> --- a/src/compiler/nir/meson.build
> +++ b/src/compiler/nir/meson.build
> @@ -100,6 +100,7 @@ files_libnir = files(
>'nir_format_convert.h',
>'nir_from_ssa.c',
>'nir_gather_info.c',
> + 'nir_gather_xfb_info.c',
>'nir_gs_count_vertices.c',
>'nir_inline_functions.c',
>'nir_instr_set.c',
> @@ -192,6 +193,7 @@ files_libnir = files(
>'nir_vla.h',
>'nir_worklist.c',
>'nir_worklist.h',
> + 'nir_xfb_info.h',
>'../spirv/GLSL.ext.AMD.h',
>'../spirv/GLSL.std.450.h',
>'../spirv/gl_spirv.c',
> diff --git a/src/compiler/nir/nir_gather_xfb_info.c
> b/src/compiler/nir/nir_gather_xfb_info.c
> new file mode 100644
> index 000..a53703bb9bf
> --- /dev/null
> +++ b/src/compiler/nir/nir_gather_xfb_info.c
> @@ -0,0 +1,150 @@
> +/*
> + * Copyright © 2018 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (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.
> + */
> +
> +#include "nir_xfb_info.h"
> +
> +#include
> +
> +static void
> +add_var_xfb_outputs(nir_xfb_info *xfb,
> +nir_variable *var,
> +unsigned *location,
> +unsigned *offset,
> +const struct glsl_type *type)
> +{
> + if (glsl_type_is_array(type) || glsl_type_is_matrix(type)) {
> + unsigned length = glsl_get_length(type);
> + const struct glsl_type *child_type = glsl_get_array_element(type);
> + for (unsigned i = 0; i < length; i++)
> + add_var_xfb_outputs(xfb, var, location, offset, child_type);
> + } else if (glsl_type_is_struct(type)) {
> + unsigned length = glsl_get_length(type);
> + for (unsigned i = 0; i < length; i++) {
> + const struct glsl_type *child_type = glsl_get_struct_field(type, i);
> + add_var_xfb_outputs(xfb, var, location, offset, child_type);
> + }
> + } else {
> + assert(var->data.xfb_buffer < NIR_MAX_XFB_BUFFERS);
> + if (xfb->buffers_written & (1 << var->data.xfb_buffer)) {
> + assert(xfb->strides[var->data.xfb_buffer] == var->data.xfb_stride);
> + assert(xfb->buffer_to_stream[var->data.xfb_buffer] ==
> var->data.stream);
>
[Mesa-dev] [Bug 108245] RADV/Vega: Low mip levels of large BCn textures get corrupted by vkCmdCopyBufferToImage
https://bugs.freedesktop.org/show_bug.cgi?id=108245 --- Comment #3 from Samuel Pitoiset --- I can confirm that vega fails. I will try to look at it today. -- 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] nir: fix compacting varyings when XFB outputs are present
On 10/10/18 10:34 AM, Timothy Arceri wrote:
On 10/10/18 7:25 pm, Samuel Pitoiset wrote:
On 10/10/18 10:14 AM, Timothy Arceri wrote:
On 10/10/18 6:44 pm, Samuel Pitoiset wrote:
We shouldn't try to compact any varyings known as always
active IO, especially XFB outputs. For example, if one
component of an xfb output is also used as input varying
in the next stage, it shouldn't be compacted.
Because we look at the input varyings from the consumer
stage, we don't know if one of them is an XFB output. One
solution is to mark all components as used when
always_active_io is true to avoid wrong remapping.
Signed-off-by: Samuel Pitoiset
---
src/compiler/nir/nir_linking_helpers.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/compiler/nir/nir_linking_helpers.c
b/src/compiler/nir/nir_linking_helpers.c
index 85712a7cb1..88014e9a1d 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -236,6 +236,15 @@
get_slot_component_masks_and_interp_types(struct exec_list *var_list,
get_interp_type(var, default_to_smooth_interp);
interp_loc[location + i] = get_interp_loc(var);
+ if (var->data.always_active_io) {
+ /* Mark all components as used to avoid repacting
xfb varyings
+ * wrongly. For instance, if one component of an xfb
output is
+ * also used as input varying in the next stage.
+ */
+ comps[location + i] |= 0xf;
+ continue;
+ }
In GLSL we specifically mark the corresponding inputs as
always_active_io. I think we should try to do that for spirv also
otherwise we are going to run into issues with the other lowering
passes that check always_active_io such as scalar/array lowering.
https://cgit.freedesktop.org/mesa/mesa/commit/?id=a1bc1523408a305b14a8c297ea93a28bb12cee5d
Should be enough?
I haven't read the spec but if its the same as GLSL that probably only
sets it on outputs.
How does the GLSL compiler handle that then? Does it somehow propagate
always_active_io to the input varyings of the next stage?
+
if (dual_slot) {
if (i & 1) {
comps[location + i] |= ((1 << comps_slot2) - 1);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 1/2] spirv/nir: handle memory access qualifiers for SSBO loads/stores
v2: - change how the access qualifiers are accumulated
v3: - duplicate members in struct_member_decoration_cb()
- handle access qualifiers on variables
- remove access qualifiers handling in _vtn_variable_load_store()
- fix setting access qualifiers on type->array_element
Signed-off-by: Samuel Pitoiset
---
src/compiler/nir/nir_intrinsics.py | 4 +--
src/compiler/spirv/spirv_to_nir.c | 24 +++--
src/compiler/spirv/vtn_private.h | 9 +
src/compiler/spirv/vtn_variables.c | 54 +-
4 files changed, 77 insertions(+), 14 deletions(-)
diff --git a/src/compiler/nir/nir_intrinsics.py
b/src/compiler/nir/nir_intrinsics.py
index b06b38fc2c..ec3049ca06 100644
--- a/src/compiler/nir/nir_intrinsics.py
+++ b/src/compiler/nir/nir_intrinsics.py
@@ -565,7 +565,7 @@ intrinsic("load_interpolated_input", src_comp=[2, 1],
dest_comp=0,
indices=[BASE, COMPONENT], flags=[CAN_ELIMINATE, CAN_REORDER])
# src[] = { buffer_index, offset }. No const_index
-load("ssbo", 2, flags=[CAN_ELIMINATE])
+load("ssbo", 2, flags=[CAN_ELIMINATE], indices=[ACCESS])
# src[] = { offset }. const_index[] = { base, component }
load("output", 1, [BASE, COMPONENT], flags=[CAN_ELIMINATE])
# src[] = { vertex, offset }. const_index[] = { base }
@@ -591,6 +591,6 @@ store("output", 2, [BASE, WRMASK, COMPONENT])
# const_index[] = { base, write_mask, component }
store("per_vertex_output", 3, [BASE, WRMASK, COMPONENT])
# src[] = { value, block_index, offset }. const_index[] = { write_mask }
-store("ssbo", 3, [WRMASK])
+store("ssbo", 3, [WRMASK, ACCESS])
# src[] = { value, offset }. const_index[] = { base, write_mask }
store("shared", 2, [BASE, WRMASK])
diff --git a/src/compiler/spirv/spirv_to_nir.c
b/src/compiler/spirv/spirv_to_nir.c
index 2ad83196e4..37a801037b 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -668,6 +668,16 @@ mutable_matrix_member(struct vtn_builder *b, struct
vtn_type *type, int member)
return type;
}
+static void
+vtn_handle_access_qualifier(struct vtn_builder *b, struct vtn_type *type,
+int member, enum gl_access_qualifier access)
+{
+ type->members[member] = vtn_type_copy(b, type->members[member]);
+ type = type->members[member];
+
+ type->access |= access;
+}
+
static void
struct_member_decoration_cb(struct vtn_builder *b,
struct vtn_value *val, int member,
@@ -681,13 +691,21 @@ struct_member_decoration_cb(struct vtn_builder *b,
assert(member < ctx->num_fields);
switch (dec->decoration) {
+ case SpvDecorationRelaxedPrecision:
+ case SpvDecorationUniform:
+ break; /* FIXME: Do nothing with this for now. */
case SpvDecorationNonWritable:
+ vtn_handle_access_qualifier(b, ctx->type, member, ACCESS_NON_WRITEABLE);
+ break;
case SpvDecorationNonReadable:
- case SpvDecorationRelaxedPrecision:
+ vtn_handle_access_qualifier(b, ctx->type, member, ACCESS_NON_READABLE);
+ break;
case SpvDecorationVolatile:
+ vtn_handle_access_qualifier(b, ctx->type, member, ACCESS_VOLATILE);
+ break;
case SpvDecorationCoherent:
- case SpvDecorationUniform:
- break; /* FIXME: Do nothing with this for now. */
+ vtn_handle_access_qualifier(b, ctx->type, member, ACCESS_COHERENT);
+ break;
case SpvDecorationNoPerspective:
ctx->fields[member].interpolation = INTERP_MODE_NOPERSPECTIVE;
break;
diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h
index a31202d129..da7a04ce59 100644
--- a/src/compiler/spirv/vtn_private.h
+++ b/src/compiler/spirv/vtn_private.h
@@ -296,6 +296,9 @@ struct vtn_type {
/* for arrays, matrices and pointers, the array stride */
unsigned stride;
+ /* Access qualifiers */
+ enum gl_access_qualifier access;
+
union {
/* Members for scalar, vector, and array-like types */
struct {
@@ -457,6 +460,9 @@ struct vtn_pointer {
/** A (block_index, offset) pair representing a UBO or SSBO position. */
struct nir_ssa_def *block_index;
struct nir_ssa_def *offset;
+
+ /* Access qualifiers */
+ enum gl_access_qualifier access;
};
struct vtn_variable {
@@ -488,6 +494,9 @@ struct vtn_variable {
* hack at some point in the future.
*/
struct vtn_pointer *copy_prop_sampler;
+
+ /* Access qualifiers. */
+ enum gl_access_qualifier access;
};
struct vtn_image_pointer {
diff --git a/src/compiler/spirv/vtn_variables.c
b/src/compiler/spirv/vtn_variables.c
index 636fdb8689..cc3438bff2 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -89,6 +89,7 @@ vtn_access_chain_pointer_dereference(struct vtn_builder *b,
struct vtn_access_chain *chain =
vtn_access_chain_extend(b, base->chain, deref_chain->length);
struct vtn_type *type = base->type;
+ enum gl_access_qualifier access = base->access;
/* OpPtrAccessChain is only allowed on
Re: [Mesa-dev] [PATCH] nir: fix compacting varyings when XFB outputs are present
On 10/10/18 7:25 pm, Samuel Pitoiset wrote:
On 10/10/18 10:14 AM, Timothy Arceri wrote:
On 10/10/18 6:44 pm, Samuel Pitoiset wrote:
We shouldn't try to compact any varyings known as always
active IO, especially XFB outputs. For example, if one
component of an xfb output is also used as input varying
in the next stage, it shouldn't be compacted.
Because we look at the input varyings from the consumer
stage, we don't know if one of them is an XFB output. One
solution is to mark all components as used when
always_active_io is true to avoid wrong remapping.
Signed-off-by: Samuel Pitoiset
---
src/compiler/nir/nir_linking_helpers.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/compiler/nir/nir_linking_helpers.c
b/src/compiler/nir/nir_linking_helpers.c
index 85712a7cb1..88014e9a1d 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -236,6 +236,15 @@ get_slot_component_masks_and_interp_types(struct
exec_list *var_list,
get_interp_type(var, default_to_smooth_interp);
interp_loc[location + i] = get_interp_loc(var);
+ if (var->data.always_active_io) {
+ /* Mark all components as used to avoid repacting xfb
varyings
+ * wrongly. For instance, if one component of an xfb
output is
+ * also used as input varying in the next stage.
+ */
+ comps[location + i] |= 0xf;
+ continue;
+ }
In GLSL we specifically mark the corresponding inputs as
always_active_io. I think we should try to do that for spirv also
otherwise we are going to run into issues with the other lowering
passes that check always_active_io such as scalar/array lowering.
https://cgit.freedesktop.org/mesa/mesa/commit/?id=a1bc1523408a305b14a8c297ea93a28bb12cee5d
Should be enough?
I haven't read the spec but if its the same as GLSL that probably only
sets it on outputs.
+
if (dual_slot) {
if (i & 1) {
comps[location + i] |= ((1 << comps_slot2) - 1);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: fix compacting varyings when XFB outputs are present
On 10/10/18 10:14 AM, Timothy Arceri wrote:
On 10/10/18 6:44 pm, Samuel Pitoiset wrote:
We shouldn't try to compact any varyings known as always
active IO, especially XFB outputs. For example, if one
component of an xfb output is also used as input varying
in the next stage, it shouldn't be compacted.
Because we look at the input varyings from the consumer
stage, we don't know if one of them is an XFB output. One
solution is to mark all components as used when
always_active_io is true to avoid wrong remapping.
Signed-off-by: Samuel Pitoiset
---
src/compiler/nir/nir_linking_helpers.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/compiler/nir/nir_linking_helpers.c
b/src/compiler/nir/nir_linking_helpers.c
index 85712a7cb1..88014e9a1d 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -236,6 +236,15 @@ get_slot_component_masks_and_interp_types(struct
exec_list *var_list,
get_interp_type(var, default_to_smooth_interp);
interp_loc[location + i] = get_interp_loc(var);
+ if (var->data.always_active_io) {
+ /* Mark all components as used to avoid repacting xfb
varyings
+ * wrongly. For instance, if one component of an xfb
output is
+ * also used as input varying in the next stage.
+ */
+ comps[location + i] |= 0xf;
+ continue;
+ }
In GLSL we specifically mark the corresponding inputs as
always_active_io. I think we should try to do that for spirv also
otherwise we are going to run into issues with the other lowering passes
that check always_active_io such as scalar/array lowering.
https://cgit.freedesktop.org/mesa/mesa/commit/?id=a1bc1523408a305b14a8c297ea93a28bb12cee5d
Should be enough?
+
if (dual_slot) {
if (i & 1) {
comps[location + i] |= ((1 << comps_slot2) - 1);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: fix compacting varyings when XFB outputs are present
On 10/10/18 6:44 pm, Samuel Pitoiset wrote:
We shouldn't try to compact any varyings known as always
active IO, especially XFB outputs. For example, if one
component of an xfb output is also used as input varying
in the next stage, it shouldn't be compacted.
Because we look at the input varyings from the consumer
stage, we don't know if one of them is an XFB output. One
solution is to mark all components as used when
always_active_io is true to avoid wrong remapping.
Signed-off-by: Samuel Pitoiset
---
src/compiler/nir/nir_linking_helpers.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/compiler/nir/nir_linking_helpers.c
b/src/compiler/nir/nir_linking_helpers.c
index 85712a7cb1..88014e9a1d 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -236,6 +236,15 @@ get_slot_component_masks_and_interp_types(struct exec_list
*var_list,
get_interp_type(var, default_to_smooth_interp);
interp_loc[location + i] = get_interp_loc(var);
+if (var->data.always_active_io) {
+ /* Mark all components as used to avoid repacting xfb varyings
+* wrongly. For instance, if one component of an xfb output is
+* also used as input varying in the next stage.
+*/
+ comps[location + i] |= 0xf;
+ continue;
+}
In GLSL we specifically mark the corresponding inputs as
always_active_io. I think we should try to do that for spirv also
otherwise we are going to run into issues with the other lowering passes
that check always_active_io such as scalar/array lowering.
+
if (dual_slot) {
if (i & 1) {
comps[location + i] |= ((1 << comps_slot2) - 1);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: Add a pass for gathering transform feedback info
I'm aware of the Offset decoration discussions, but I'm fine with that
pass for now anyway. It does the job and RADV can use it.
Having the offset explicitely declared is the way to go in my opinion,
and that doesn't require extra compiler work. So, patch is:
Reviewed-by: Samuel Pitoiset
On 10/5/18 4:13 PM, Jason Ekstrand wrote:
This is different from the GL_ARB_spirv pass because it generates a much
simpler data structure that isn't tied to OpenGL and mtypes.h.
---
src/compiler/Makefile.sources | 4 +-
src/compiler/nir/meson.build | 2 +
src/compiler/nir/nir_gather_xfb_info.c | 150 +
src/compiler/nir/nir_xfb_info.h| 59 ++
4 files changed, 214 insertions(+), 1 deletion(-)
create mode 100644 src/compiler/nir/nir_gather_xfb_info.c
create mode 100644 src/compiler/nir/nir_xfb_info.h
diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
index d3b06564832..46ed5e47b46 100644
--- a/src/compiler/Makefile.sources
+++ b/src/compiler/Makefile.sources
@@ -216,6 +216,7 @@ NIR_FILES = \
nir/nir_format_convert.h \
nir/nir_from_ssa.c \
nir/nir_gather_info.c \
+ nir/nir_gather_xfb_info.c \
nir/nir_gs_count_vertices.c \
nir/nir_inline_functions.c \
nir/nir_instr_set.c \
@@ -307,7 +308,8 @@ NIR_FILES = \
nir/nir_validate.c \
nir/nir_vla.h \
nir/nir_worklist.c \
- nir/nir_worklist.h
+ nir/nir_worklist.h \
+ nir/nir_xfb_info.h
SPIRV_GENERATED_FILES = \
spirv/spirv_info.c \
diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
index 090aa7a628f..b416e561eb0 100644
--- a/src/compiler/nir/meson.build
+++ b/src/compiler/nir/meson.build
@@ -100,6 +100,7 @@ files_libnir = files(
'nir_format_convert.h',
'nir_from_ssa.c',
'nir_gather_info.c',
+ 'nir_gather_xfb_info.c',
'nir_gs_count_vertices.c',
'nir_inline_functions.c',
'nir_instr_set.c',
@@ -192,6 +193,7 @@ files_libnir = files(
'nir_vla.h',
'nir_worklist.c',
'nir_worklist.h',
+ 'nir_xfb_info.h',
'../spirv/GLSL.ext.AMD.h',
'../spirv/GLSL.std.450.h',
'../spirv/gl_spirv.c',
diff --git a/src/compiler/nir/nir_gather_xfb_info.c
b/src/compiler/nir/nir_gather_xfb_info.c
new file mode 100644
index 000..a53703bb9bf
--- /dev/null
+++ b/src/compiler/nir/nir_gather_xfb_info.c
@@ -0,0 +1,150 @@
+/*
+ * Copyright © 2018 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (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.
+ */
+
+#include "nir_xfb_info.h"
+
+#include
+
+static void
+add_var_xfb_outputs(nir_xfb_info *xfb,
+nir_variable *var,
+unsigned *location,
+unsigned *offset,
+const struct glsl_type *type)
+{
+ if (glsl_type_is_array(type) || glsl_type_is_matrix(type)) {
+ unsigned length = glsl_get_length(type);
+ const struct glsl_type *child_type = glsl_get_array_element(type);
+ for (unsigned i = 0; i < length; i++)
+ add_var_xfb_outputs(xfb, var, location, offset, child_type);
+ } else if (glsl_type_is_struct(type)) {
+ unsigned length = glsl_get_length(type);
+ for (unsigned i = 0; i < length; i++) {
+ const struct glsl_type *child_type = glsl_get_struct_field(type, i);
+ add_var_xfb_outputs(xfb, var, location, offset, child_type);
+ }
+ } else {
+ assert(var->data.xfb_buffer < NIR_MAX_XFB_BUFFERS);
+ if (xfb->buffers_written & (1 << var->data.xfb_buffer)) {
+ assert(xfb->strides[var->data.xfb_buffer] == var->data.xfb_stride);
+ assert(xfb->buffer_to_stream[var->data.xfb_buffer] ==
var->data.stream);
+ } else {
+ xfb->buffers_written |= (1 << var->data.xfb_buffer);
+ xfb->strides[var->data.xfb_buffer] = var->data.xfb_stride;
+ xfb->buffer_to_stream[var->data.xfb_buffer] =
Re: [Mesa-dev] [PATCH] dri: Include kernel release in vendor info
Quoting Ian Romanick (2018-10-10 00:24:00)
> On 10/09/2018 06:24 AM, Chris Wilson wrote:
> > The userspace driver does not exist in isolation and occasionally
> > depends on kernel uapi, and so it is useful in bug reports to include
> > that information. (radeonsi, r600 and radv already include utsname)
> >
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=108282
> > ---
> > src/mesa/drivers/dri/common/utils.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/common/utils.c
> > b/src/mesa/drivers/dri/common/utils.c
> > index 5a66bcf8e05..4d0d654dbfd 100644
> > --- a/src/mesa/drivers/dri/common/utils.c
> > +++ b/src/mesa/drivers/dri/common/utils.c
> > @@ -34,6 +34,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +
> > #include "main/macros.h"
> > #include "main/mtypes.h"
> > #include "main/cpuinfo.h"
> > @@ -77,11 +79,15 @@ unsigned
> > driGetRendererString( char * buffer, const char * hardware_name,
> > GLuint agp_mode )
> > {
> > + struct utsname uts;
> > unsigned offset;
> > char *cpu;
> >
> > offset = sprintf( buffer, "Mesa DRI %s", hardware_name );
> >
> > + if (uname() == 0)
> > + offset += sprintf(buffer + offset, " [%s]", uts.release);
>
> Is there any kind of a limit on how big uts.release can be? All of the
> callers pass a fixed-size buffer into the function, and the function has
> no way to know how big that buffer is.
Currently defined at 64 + '\0'. The function can always be replaced by
one that does know buflen.
-Chris
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 108109] [GLSL] no-overloads.vert fails
https://bugs.freedesktop.org/show_bug.cgi?id=108109 --- Comment #3 from vadym --- Additional check to compiler added: https://patchwork.freedesktop.org/patch/255542/ -- You are receiving this mail because: You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 108312] Failed to allocate buffer on big endian machine - buffer size not byte swapped
https://bugs.freedesktop.org/show_bug.cgi?id=108312 Bas Vermeulen changed: What|Removed |Added OS|All |Linux (All) Hardware|Other |PowerPC -- 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 108312] Failed to allocate buffer on big endian machine - buffer size not byte swapped
https://bugs.freedesktop.org/show_bug.cgi?id=108312 Bug ID: 108312 Summary: Failed to allocate buffer on big endian machine - buffer size not byte swapped Product: Mesa Version: 18.2 Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Gallium/StateTracker/Clover Assignee: mesa-dev@lists.freedesktop.org Reporter: b...@daedalean.ai QA Contact: mesa-dev@lists.freedesktop.org Some changes between 18.0.5 and 18.2.0 caused my big endian PowerPC to no longer be able to allocate buffers using OpenCL. radeon complains about failing to allocate a buffer of 2686976000 bytes where I allocate 10400 bytes. The size of the buffer seems to not be byte swapped. I will try to pin down the commit that caused this and create a patch to fix it. -- 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] [PATCH] nir: fix compacting varyings when XFB outputs are present
We shouldn't try to compact any varyings known as always
active IO, especially XFB outputs. For example, if one
component of an xfb output is also used as input varying
in the next stage, it shouldn't be compacted.
Because we look at the input varyings from the consumer
stage, we don't know if one of them is an XFB output. One
solution is to mark all components as used when
always_active_io is true to avoid wrong remapping.
Signed-off-by: Samuel Pitoiset
---
src/compiler/nir/nir_linking_helpers.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/compiler/nir/nir_linking_helpers.c
b/src/compiler/nir/nir_linking_helpers.c
index 85712a7cb1..88014e9a1d 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -236,6 +236,15 @@ get_slot_component_masks_and_interp_types(struct exec_list
*var_list,
get_interp_type(var, default_to_smooth_interp);
interp_loc[location + i] = get_interp_loc(var);
+if (var->data.always_active_io) {
+ /* Mark all components as used to avoid repacting xfb varyings
+* wrongly. For instance, if one component of an xfb output is
+* also used as input varying in the next stage.
+*/
+ comps[location + i] |= 0xf;
+ continue;
+}
+
if (dual_slot) {
if (i & 1) {
comps[location + i] |= ((1 << comps_slot2) - 1);
--
2.19.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 108245] RADV/Vega: Low mip levels of large BCn textures get corrupted by vkCmdCopyBufferToImage
https://bugs.freedesktop.org/show_bug.cgi?id=108245 --- Comment #2 from Alex Smith --- This also seems to be sometimes causing VM faults - I get a bunch of faults in the game that's affected by this bug, which go away if I skip vkCmdCopyBufferToImage calls which would get corrupted. -- 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 108311] Query buffer object support is broken on r600.
https://bugs.freedesktop.org/show_bug.cgi?id=108311 Andrew Wesie changed: What|Removed |Added CC||mesa-dev@lists.freedesktop. ||org -- You are receiving this mail because: You are on the CC list for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
