[Mesa-dev] [PATCH] radv: don't expose linear depth surfaces on SI/CIK/VI either.
From: Dave Airlie
ac_surface.c: gfx6_compute_surface says
/* DB doesn't support linear layouts. */
Now if we expose linear depth and create a linear depth image
and use CmdCopyImage to copy into it, we can't map the underlying
memory and read it linearly which I think should work.
---
src/amd/vulkan/radv_formats.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/amd/vulkan/radv_formats.c b/src/amd/vulkan/radv_formats.c
index 6253c27b95d..e1b4b5e830f 100644
--- a/src/amd/vulkan/radv_formats.c
+++ b/src/amd/vulkan/radv_formats.c
@@ -645,9 +645,8 @@ radv_physical_device_get_format_properties(struct
radv_physical_device *physical
if (radv_is_filter_minmax_format_supported(format))
tiled |=
VK_FORMAT_FEATURE_SAMPLED_IMAGE_FILTER_MINMAX_BIT_EXT;
- /* GFX9 doesn't support linear depth surfaces */
- if (physical_device->rad_info.chip_class >= GFX9)
- linear = 0;
+ /* Don't support linear depth surfaces */
+ linear = 0;
}
} else {
bool linear_sampling;
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeonsi: fix tess/gs fetchs for new swizzle.
Reviewed-by: Marek Olšák
Marek
On Thu, Aug 30, 2018 at 7:29 PM, Dave Airlie wrote:
> From: Dave Airlie
>
> I have piglit results from my machine, but I must have messed up,
> and not built mesa in between properly.
>
> Fixes: bb17ae49ee2 (gallivm: allow to pass two swizzles into fetches.)
> ---
> src/gallium/drivers/radeonsi/si_shader.c | 13 -
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_shader.c
> b/src/gallium/drivers/radeonsi/si_shader.c
> index d8930bfd50e..8cadcf2079b 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.c
> +++ b/src/gallium/drivers/radeonsi/si_shader.c
> @@ -1204,11 +1204,11 @@ static LLVMValueRef get_tess_ring_descriptor(struct
> si_shader_context *ctx,
> static LLVMValueRef fetch_input_tcs(
> struct lp_build_tgsi_context *bld_base,
> const struct tgsi_full_src_register *reg,
> - enum tgsi_opcode_type type, unsigned swizzle)
> + enum tgsi_opcode_type type, unsigned swizzle_in)
> {
> struct si_shader_context *ctx = si_shader_context(bld_base);
> LLVMValueRef dw_addr, stride;
> -
> + unsigned swizzle = swizzle_in & 0x;
> stride = get_tcs_in_vertex_dw_stride(ctx);
> dw_addr = get_tcs_in_current_patch_offset(ctx);
> dw_addr = get_dw_address(ctx, NULL, reg, stride, dw_addr);
> @@ -1289,10 +1289,11 @@ static LLVMValueRef si_nir_load_tcs_varyings(struct
> ac_shader_abi *abi,
> static LLVMValueRef fetch_output_tcs(
> struct lp_build_tgsi_context *bld_base,
> const struct tgsi_full_src_register *reg,
> - enum tgsi_opcode_type type, unsigned swizzle)
> + enum tgsi_opcode_type type, unsigned swizzle_in)
> {
> struct si_shader_context *ctx = si_shader_context(bld_base);
> LLVMValueRef dw_addr, stride;
> + unsigned swizzle = (swizzle_in & 0x);
>
> if (reg->Register.Dimension) {
> stride = get_tcs_out_vertex_dw_stride(ctx);
> @@ -1309,10 +1310,11 @@ static LLVMValueRef fetch_output_tcs(
> static LLVMValueRef fetch_input_tes(
> struct lp_build_tgsi_context *bld_base,
> const struct tgsi_full_src_register *reg,
> - enum tgsi_opcode_type type, unsigned swizzle)
> + enum tgsi_opcode_type type, unsigned swizzle_in)
> {
> struct si_shader_context *ctx = si_shader_context(bld_base);
> LLVMValueRef base, addr;
> + unsigned swizzle = (swizzle_in & 0x);
>
> base = LLVMGetParam(ctx->main_fn, ctx->param_tcs_offchip_offset);
> addr = get_tcs_tes_buffer_address_from_reg(ctx, NULL, reg);
> @@ -1696,10 +1698,11 @@ static LLVMValueRef fetch_input_gs(
> struct lp_build_tgsi_context *bld_base,
> const struct tgsi_full_src_register *reg,
> enum tgsi_opcode_type type,
> - unsigned swizzle)
> + unsigned swizzle_in)
> {
> struct si_shader_context *ctx = si_shader_context(bld_base);
> struct tgsi_shader_info *info = >shader->selector->info;
> + unsigned swizzle = swizzle_in & 0x;
>
> unsigned semantic_name =
> info->input_semantic_name[reg->Register.Index];
> if (swizzle != ~0 && semantic_name == TGSI_SEMANTIC_PRIMID)
> --
> 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] radeonsi: fix regression in indirect input swizzles.
Reviewed-by: Marek Olšák
Marek
On Thu, Aug 30, 2018 at 8:13 PM, Dave Airlie wrote:
> From: Dave Airlie
>
> This fixes:
> tests/spec/arb_enhanced_layouts/execution/component-layout/vs-fs-array-dvec3.shader_test
> since I reworked the 64-bit swizzles.
>
> Fixes: bb17ae49ee2 (gallivm: allow to pass two swizzles into fetches.)
> ---
> src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
> b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
> index d48eda1b100..3ec919dd23b 100644
> --- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
> +++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
> @@ -317,18 +317,21 @@ static LLVMValueRef
> emit_array_fetch(struct lp_build_tgsi_context *bld_base,
> unsigned File, enum tgsi_opcode_type type,
> struct tgsi_declaration_range range,
> -unsigned swizzle)
> +unsigned swizzle_in)
> {
> struct si_shader_context *ctx = si_shader_context(bld_base);
> unsigned i, size = range.Last - range.First + 1;
> LLVMTypeRef vec = LLVMVectorType(tgsi2llvmtype(bld_base, type), size);
> LLVMValueRef result = LLVMGetUndef(vec);
> -
> + unsigned swizzle = swizzle_in;
> struct tgsi_full_src_register tmp_reg = {};
> tmp_reg.Register.File = File;
> + if (tgsi_type_is_64bit(type))
> + swizzle |= (swizzle_in + 1) << 16;
>
> for (i = 0; i < size; ++i) {
> tmp_reg.Register.Index = i + range.First;
> +
> LLVMValueRef temp = si_llvm_emit_fetch(bld_base, _reg,
> type, swizzle);
> result = LLVMBuildInsertElement(ctx->ac.builder, result, temp,
> LLVMConstInt(ctx->i32, i, 0), "array_vector");
> --
> 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] mesa: ignore VAO IDs equal to 0 in glDeleteVertexArrays
Sadly, there are no tests for this.
Marek
On Thu, Aug 30, 2018 at 6:24 PM, Ian Romanick wrote:
> This patch is
>
> Reviewed-by: Ian Romanick
>
> Is there a piglit test? I wonder how many other glDeleteFoo functions
> mishandle the id=0 case. :(
>
> On 08/30/2018 12:16 PM, Marek Olšák wrote:
>> From: Marek Olšák
>>
>> This fixes a firefox crash.
>>
>> Fixes: 781a78914c798dc64005b37c6ca1224ce06803fc
>> ---
>> src/mesa/main/arrayobj.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
>> index a23031fe182..6e0665c0e5d 100644
>> --- a/src/mesa/main/arrayobj.c
>> +++ b/src/mesa/main/arrayobj.c
>> @@ -1007,20 +1007,24 @@ _mesa_BindVertexArray(GLuint id)
>> *
>> * \param n Number of array objects to delete.
>> * \param idsArray of \c n array object IDs.
>> */
>> static void
>> delete_vertex_arrays(struct gl_context *ctx, GLsizei n, const GLuint *ids)
>> {
>> GLsizei i;
>>
>> for (i = 0; i < n; i++) {
>> + /* IDs equal to 0 should be silently ignored. */
>> + if (!ids[i])
>> + continue;
>> +
>>struct gl_vertex_array_object *obj = _mesa_lookup_vao(ctx, ids[i]);
>>
>>if (obj) {
>> assert(obj->Name == ids[i]);
>>
>> /* If the array object is currently bound, the spec says "the
>> binding
>>* for that object reverts to zero and the default vertex array
>>* becomes current."
>>*/
>> if (obj == ctx->Array.VAO)
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 7/7] radv: remove dead code in scan_shader_output_decl()
Assuming it all has no CTS regressions, and GTAV works.
Reviewed-by: Dave Airlie for the series.
On Thu, 30 Aug 2018 at 18:59, Samuel Pitoiset wrote:
>
> Never used.
>
> Signed-off-by: Samuel Pitoiset
> ---
> src/amd/vulkan/radv_nir_to_llvm.c | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_nir_to_llvm.c
> b/src/amd/vulkan/radv_nir_to_llvm.c
> index b2411fe38b..af34c548c1 100644
> --- a/src/amd/vulkan/radv_nir_to_llvm.c
> +++ b/src/amd/vulkan/radv_nir_to_llvm.c
> @@ -2241,8 +2241,6 @@ scan_shader_output_decl(struct radv_shader_context *ctx,
> stage == MESA_SHADER_TESS_EVAL ||
> stage == MESA_SHADER_GEOMETRY) {
> if (idx == VARYING_SLOT_CLIP_DIST0) {
> - int length = shader->info.clip_distance_array_size +
> -shader->info.cull_distance_array_size;
> if (stage == MESA_SHADER_VERTEX) {
> ctx->shader_info->vs.outinfo.clip_dist_mask =
> (1 << shader->info.clip_distance_array_size) - 1;
> ctx->shader_info->vs.outinfo.cull_dist_mask =
> (1 << shader->info.cull_distance_array_size) - 1;
> @@ -2254,10 +2252,6 @@ scan_shader_output_decl(struct radv_shader_context
> *ctx,
> ctx->shader_info->tes.outinfo.cull_dist_mask
> <<= shader->info.clip_distance_array_size;
> }
>
> - if (length > 4)
> - attrib_count = 2;
> - else
> - attrib_count = 1;
> mask_attribs = 1ull << idx;
> }
> }
> --
> 2.18.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 54805] gl_ClipVertex support horribly broken with software TNL
https://bugs.freedesktop.org/show_bug.cgi?id=54805 Timothy Arceri changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |WONTFIX --- Comment #10 from Timothy Arceri --- As it turns out this wasn't a regression and its unlikely anyone is going to work on improving software TNL I'm going to close this for now. -- You are receiving this mail because: 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] gallivm/radeonsi: allow to pass two swizzles into fetches.
On Fri., 31 Aug. 2018, 01:22 Michel Dänzer, wrote: > > On 2018-08-27 11:16 p.m., Dave Airlie wrote: > > From: Dave Airlie > > > > This hijacks the top 16-bits of swizzle, to pass in the swizzle > > for the second channel. > > > > This fixes handling .yx swizzles of 64-bit values. > > > > This should fixup radeonsi and llvmpipe. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107524 > > This change broke a bunch of piglit tests for me with radeonsi on > Bonair > Wierd I did piglit runs locally, but I must have screwed them up somehow. I've sent two patches to fix up the regressions, thanks for finding and reporting them! Dave. > spec@arb_enhanced_layouts@execution@component-layout@vs-fs-array-dvec3 > spec@arb_tessellation_shader@execution@dvec2-vs-tcs-tes > spec@arb_tessellation_shader@execution@double-array-vs-tcs-tes > spec@arb_tessellation_shader@execution@double-vs-tcs-tes > spec@arb_tessellation_shader@execution@dvec3-vs-tcs-tes > spec@arb_tessellation_shader@execution@variable-indexing@tes-input-array-dvec4-index-rd > spec@arb_tessellation_shader@execution@variable-indexing@vs-output-array-dvec4-index-wr-before-tcs > spec@arb_tessellation_shader@execution@variable-indexing@tcs-output-array-dvec4-index-wr > spec@arb_tessellation_shader@execution@variable-indexing@tcs-input-array-dvec4-index-rd > > > -- > Earthling Michel Dänzer | http://www.amd.com > Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] radeonsi: fix regression in indirect input swizzles.
From: Dave Airlie
This fixes:
tests/spec/arb_enhanced_layouts/execution/component-layout/vs-fs-array-dvec3.shader_test
since I reworked the 64-bit swizzles.
Fixes: bb17ae49ee2 (gallivm: allow to pass two swizzles into fetches.)
---
src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
index d48eda1b100..3ec919dd23b 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_setup.c
@@ -317,18 +317,21 @@ static LLVMValueRef
emit_array_fetch(struct lp_build_tgsi_context *bld_base,
unsigned File, enum tgsi_opcode_type type,
struct tgsi_declaration_range range,
-unsigned swizzle)
+unsigned swizzle_in)
{
struct si_shader_context *ctx = si_shader_context(bld_base);
unsigned i, size = range.Last - range.First + 1;
LLVMTypeRef vec = LLVMVectorType(tgsi2llvmtype(bld_base, type), size);
LLVMValueRef result = LLVMGetUndef(vec);
-
+ unsigned swizzle = swizzle_in;
struct tgsi_full_src_register tmp_reg = {};
tmp_reg.Register.File = File;
+ if (tgsi_type_is_64bit(type))
+ swizzle |= (swizzle_in + 1) << 16;
for (i = 0; i < size; ++i) {
tmp_reg.Register.Index = i + range.First;
+
LLVMValueRef temp = si_llvm_emit_fetch(bld_base, _reg,
type, swizzle);
result = LLVMBuildInsertElement(ctx->ac.builder, result, temp,
LLVMConstInt(ctx->i32, i, 0), "array_vector");
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC] compiler: Move double_inputs to gl_program::DualSlotInputs
Previously, we had two field in shader_info: double_inputs_read and
double_inputs. Presumably, the one was for all double inputs that are
read and the other is all that exist. However, because nir_gather_info
regenerates these two values, there is a possibility, if a variable gets
deleted, that the value of double_inputs could change over time. This
is a problem because double_inputs is used to remap the input locations
to a two-slot-per-dvec3/4 scheme for i965. If that mapping were to
change between glsl_to_nir and back-end state setup, we would fall over
when trying to map the NIR outputs back onto the GL location space.
This commit changes the way slot re-mapping works. Instead of the
double_inputs field in shader_info, it adds a DualSlotInputs bitfield to
gl_program. By having it in gl_program, we more easily guarantee that
NIR passes won't touch it after it's been set. It also makes more sense
to put it in a GL data structure since it's really a mapping from GL
slots to back-end and/or NIR slots and not really a NIR shader thing.
This shouldn't affect gallium drivers or radv but I have yet to actually
test it on any of them.
Cc: Kenneth Graunke
Cc: Timothy Arceri
---
src/compiler/glsl/glsl_to_nir.cpp | 16 +++--
src/compiler/glsl/ir_set_program_inouts.cpp | 2 +-
src/compiler/glsl/serialize.cpp | 2 ++
src/compiler/nir/nir.c| 36 ---
src/compiler/nir/nir.h| 5 +--
src/compiler/nir/nir_gather_info.c| 6
src/compiler/shader_info.h| 3 --
src/mesa/drivers/dri/i965/brw_draw_upload.c | 19 +-
src/mesa/drivers/dri/i965/genX_state_upload.c | 1 +
src/mesa/main/glspirv.c | 20 +++
src/mesa/main/mtypes.h| 15
src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +-
src/mesa/state_tracker/st_glsl_to_tgsi.cpp| 2 +-
src/mesa/state_tracker/st_program.c | 3 +-
14 files changed, 66 insertions(+), 66 deletions(-)
diff --git a/src/compiler/glsl/glsl_to_nir.cpp
b/src/compiler/glsl/glsl_to_nir.cpp
index f38d280d406..d22f4a58dd4 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -149,8 +149,11 @@ glsl_to_nir(const struct gl_shader_program *shader_prog,
* two locations. For instance, if we have in the IR code a dvec3 attr0 in
* location 0 and vec4 attr1 in location 1, in NIR attr0 will use
* locations/slots 0 and 1, and attr1 will use location/slot 2 */
- if (shader->info.stage == MESA_SHADER_VERTEX)
- nir_remap_attributes(shader, options);
+ if (shader->info.stage == MESA_SHADER_VERTEX) {
+ sh->Program->DualSlotInputs = nir_get_dual_slot_attributes(shader);
+ if (options->vs_inputs_dual_locations)
+ nir_remap_dual_slot_attributes(shader, sh->Program->DualSlotInputs);
+ }
shader->info.name = ralloc_asprintf(shader, "GLSL%d", shader_prog->Name);
if (shader_prog->Label)
@@ -344,15 +347,6 @@ nir_visitor::visit(ir_variable *ir)
var->data.compact = ir->type->without_array()->is_scalar();
}
}
-
- /* Mark all the locations that require two slots */
- if (shader->info.stage == MESA_SHADER_VERTEX &&
- glsl_type_is_dual_slot(glsl_without_array(var->type))) {
- for (unsigned i = 0; i < glsl_count_attribute_slots(var->type, true);
i++) {
-uint64_t bitfield = BITFIELD64_BIT(var->data.location + i);
-shader->info.vs.double_inputs |= bitfield;
- }
- }
break;
case ir_var_shader_out:
diff --git a/src/compiler/glsl/ir_set_program_inouts.cpp
b/src/compiler/glsl/ir_set_program_inouts.cpp
index ba1e44167c3..a3cb19479b8 100644
--- a/src/compiler/glsl/ir_set_program_inouts.cpp
+++ b/src/compiler/glsl/ir_set_program_inouts.cpp
@@ -118,7 +118,7 @@ mark(struct gl_program *prog, ir_variable *var, int offset,
int len,
/* double inputs read is only for vertex inputs */
if (stage == MESA_SHADER_VERTEX &&
var->type->without_array()->is_dual_slot())
-prog->info.vs.double_inputs_read |= bitfield;
+prog->DualSlotInputs |= bitfield;
if (stage == MESA_SHADER_FRAGMENT) {
prog->info.fs.uses_sample_qualifier |= var->data.sample;
diff --git a/src/compiler/glsl/serialize.cpp b/src/compiler/glsl/serialize.cpp
index 889038fb5e2..267700e7e78 100644
--- a/src/compiler/glsl/serialize.cpp
+++ b/src/compiler/glsl/serialize.cpp
@@ -1035,6 +1035,7 @@ write_shader_metadata(struct blob *metadata,
gl_linked_shader *shader)
struct gl_program *glprog = shader->Program;
unsigned i;
+ blob_write_uint64(metadata, glprog->DualSlotInputs);
blob_write_bytes(metadata, glprog->TexturesUsed,
sizeof(glprog->TexturesUsed));
blob_write_uint64(metadata, glprog->SamplersUsed);
@@ -1088,6 +1089,7 @@ read_shader_metadata(struct
[Mesa-dev] [Bug 107670] Massive slowdown under specific memcpy implementations (32bit, no-SIMD, backward copy).
https://bugs.freedesktop.org/show_bug.cgi?id=107670 Timothy Arceri changed: What|Removed |Added Status|NEW |NEEDINFO --- Comment #8 from Timothy Arceri --- (In reply to Eero Tamminen from comment #6) > (In reply to Timothy Arceri from comment #1) > > There already is asm optimized version of memcpy() in glibc. Why would we > > want to reinvent that in Mesa? > > > > glibc should pick the right implementation for you system. > > How would memcpy() know that the destination is mapped to PCI-E address > space i.e. gets transparently transferred over the PCI-E bus (which has its > own performance constraints)? "The slowdown could be observed if non-SIMD version of the glibc-2.27 function is used (like the one that comes with the 32 bit Slackware-current). Using SSE2 memcpy seems to avoid this problem" Glib should select the SSE2 (or better) version of memcpy. If Slackware doesn't ship and SSE2 support for glibc I don't see how this is Mesas fault. If I'm misunderstanding somthing please clarify. Otherwise I'm inclined to close this as won't fix. -- 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 107457] [Tracker] Mesa 18.2 release tracker
https://bugs.freedesktop.org/show_bug.cgi?id=107457 Bug 107457 depends on bug 106865, which changed state. Bug 106865 Summary: [GLK] piglit.spec.ext_framebuffer_multisample.accuracy stencil tests fail https://bugs.freedesktop.org/show_bug.cgi?id=106865 What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED -- 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 106156] [TRACKER] Mesa 18.2 feature tracker
https://bugs.freedesktop.org/show_bug.cgi?id=106156 Bug 106156 depends on bug 106865, which changed state. Bug 106865 Summary: [GLK] piglit.spec.ext_framebuffer_multisample.accuracy stencil tests fail https://bugs.freedesktop.org/show_bug.cgi?id=106865 What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] i965/gen7_urb: Re-emit PUSH_CONSTANT_ALLOC on some gen9
On Thu, Aug 30, 2018 at 02:37:40PM -0700, Kenneth Graunke wrote:
> On Wednesday, August 29, 2018 1:38:51 PM PDT Nanley Chery wrote:
> > According to internal docs, some gen9 platforms have a pixel shader push
> > constant synchronization issue. Although not listed among said
> > platforms, this issue seems to be present on the GeminiLake 2x6's we've
> > tested.
> >
> > We consider the available workarounds to be too detrimental on
> > performance. Instead, we mitigate the issue by applying part of one of
> > the workarounds. Re-emit PUSH_CONSTANT_ALLOC at the top of every batch
> > (as suggested by Ken).
> >
> > Fixes ext_framebuffer_multisample-accuracy piglit test failures with the
> > following options:
> > * 6 depth_draw small depthstencil
> > * 8 stencil_draw small depthstencil
> > * 6 stencil_draw small depthstencil
> > * 8 depth_resolve small
> > * 6 stencil_resolve small depthstencil
> > * 4 stencil_draw small depthstencil
> > * 16 stencil_draw small depthstencil
> > * 16 depth_draw small depthstencil
> > * 2 stencil_resolve small depthstencil
> > * 6 stencil_draw small
> > * all_samples stencil_draw small
> > * 2 depth_draw small depthstencil
> > * all_samples depth_draw small depthstencil
> > * all_samples stencil_resolve small
> > * 4 depth_draw small depthstencil
> > * all_samples depth_draw small
> > * all_samples stencil_draw small depthstencil
> > * 4 stencil_resolve small depthstencil
> > * 4 depth_resolve small depthstencil
> > * all_samples stencil_resolve small depthstencil
> >
> > v2: Include more platforms in WA (Ken).
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106865
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93355
> > Cc:
> > Tested-by: Mark Janes
> > ---
> > src/mesa/drivers/dri/i965/gen7_urb.c | 28
> > 1 file changed, 28 insertions(+)
> >
> > I'm not sure I have enough information about what's happening in the HW
> > to create a piglit test for this issue.
> >
> > diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c
> > b/src/mesa/drivers/dri/i965/gen7_urb.c
> > index 2e5f8e60ba9..e7259fc1b8d 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_urb.c
> > +++ b/src/mesa/drivers/dri/i965/gen7_urb.c
> > @@ -118,6 +118,33 @@ gen7_emit_push_constant_state(struct brw_context *brw,
> > unsigned vs_size,
> > const struct gen_device_info *devinfo = >screen->devinfo;
> > unsigned offset = 0;
> >
> > + /* From the SKL PRM, Workarounds section (#878):
> > +*
> > +*Push constant buffer corruption possible. WA: Insert 2 zero-length
> > +*PushConst_PS before every intended PushConst_PS update, issue a
> > +*NULLPRIM after each of the zero len PC update to make sure CS
> > commits
> > +*them.
> > +*
> > +* This workaround is attempting to solve a pixel shader push constant
> > +* synchronization issue.
> > +*
> > +* There's an unpublished WA that involves re-emitting
> > +* 3DSTATE_PUSH_CONSTANT_ALLOC_PS for every 500-ish 3DSTATE_CONSTANT_PS
> > +* packets. Since our counting methods may not be reliable due to
> > +* context-switching and pre-emption, we instead choose to approximate
> > this
> > +* behavior by re-emitting the packet at the top of the batch.
> > +*/
> > + if (brw->ctx.NewDriverState == BRW_NEW_BATCH) {
> > + /* SKL GT2 and GLK 2x6 have reliably demonstrated this issue thus
> > far.
> > +* We've also seen some intermittent failures from SKL GT4 and BXT
> > in
> > +* the past.
> > +*/
> > + if (!devinfo->is_skylake &&
> > + !devinfo->is_broxton &&
> > + !devinfo->is_geminilake)
> > + return;
> > + }
> > +
> > BEGIN_BATCH(10);
> > OUT_BATCH(_3DSTATE_PUSH_CONSTANT_ALLOC_VS << 16 | (2 - 2));
> > OUT_BATCH(vs_size | offset << GEN7_PUSH_CONSTANT_BUFFER_OFFSET_SHIFT);
> > @@ -154,6 +181,7 @@ const struct brw_tracked_state gen7_push_constant_space
> > = {
> > .dirty = {
> >.mesa = 0,
> >.brw = BRW_NEW_CONTEXT |
> > + BRW_NEW_BATCH | /* Push constant workaround */
> > BRW_NEW_GEOMETRY_PROGRAM |
> > BRW_NEW_TESS_PROGRAMS,
> > },
> >
>
> Not sure we can do much better than this. Thanks for taking care of
> this, Nanley.
>
> Reviewed-by: Kenneth Graunke
Same here. Thanks.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] radeonsi: fix tess/gs fetchs for new swizzle.
From: Dave Airlie
I have piglit results from my machine, but I must have messed up,
and not built mesa in between properly.
Fixes: bb17ae49ee2 (gallivm: allow to pass two swizzles into fetches.)
---
src/gallium/drivers/radeonsi/si_shader.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/gallium/drivers/radeonsi/si_shader.c
b/src/gallium/drivers/radeonsi/si_shader.c
index d8930bfd50e..8cadcf2079b 100644
--- a/src/gallium/drivers/radeonsi/si_shader.c
+++ b/src/gallium/drivers/radeonsi/si_shader.c
@@ -1204,11 +1204,11 @@ static LLVMValueRef get_tess_ring_descriptor(struct
si_shader_context *ctx,
static LLVMValueRef fetch_input_tcs(
struct lp_build_tgsi_context *bld_base,
const struct tgsi_full_src_register *reg,
- enum tgsi_opcode_type type, unsigned swizzle)
+ enum tgsi_opcode_type type, unsigned swizzle_in)
{
struct si_shader_context *ctx = si_shader_context(bld_base);
LLVMValueRef dw_addr, stride;
-
+ unsigned swizzle = swizzle_in & 0x;
stride = get_tcs_in_vertex_dw_stride(ctx);
dw_addr = get_tcs_in_current_patch_offset(ctx);
dw_addr = get_dw_address(ctx, NULL, reg, stride, dw_addr);
@@ -1289,10 +1289,11 @@ static LLVMValueRef si_nir_load_tcs_varyings(struct
ac_shader_abi *abi,
static LLVMValueRef fetch_output_tcs(
struct lp_build_tgsi_context *bld_base,
const struct tgsi_full_src_register *reg,
- enum tgsi_opcode_type type, unsigned swizzle)
+ enum tgsi_opcode_type type, unsigned swizzle_in)
{
struct si_shader_context *ctx = si_shader_context(bld_base);
LLVMValueRef dw_addr, stride;
+ unsigned swizzle = (swizzle_in & 0x);
if (reg->Register.Dimension) {
stride = get_tcs_out_vertex_dw_stride(ctx);
@@ -1309,10 +1310,11 @@ static LLVMValueRef fetch_output_tcs(
static LLVMValueRef fetch_input_tes(
struct lp_build_tgsi_context *bld_base,
const struct tgsi_full_src_register *reg,
- enum tgsi_opcode_type type, unsigned swizzle)
+ enum tgsi_opcode_type type, unsigned swizzle_in)
{
struct si_shader_context *ctx = si_shader_context(bld_base);
LLVMValueRef base, addr;
+ unsigned swizzle = (swizzle_in & 0x);
base = LLVMGetParam(ctx->main_fn, ctx->param_tcs_offchip_offset);
addr = get_tcs_tes_buffer_address_from_reg(ctx, NULL, reg);
@@ -1696,10 +1698,11 @@ static LLVMValueRef fetch_input_gs(
struct lp_build_tgsi_context *bld_base,
const struct tgsi_full_src_register *reg,
enum tgsi_opcode_type type,
- unsigned swizzle)
+ unsigned swizzle_in)
{
struct si_shader_context *ctx = si_shader_context(bld_base);
struct tgsi_shader_info *info = >shader->selector->info;
+ unsigned swizzle = swizzle_in & 0x;
unsigned semantic_name = info->input_semantic_name[reg->Register.Index];
if (swizzle != ~0 && semantic_name == TGSI_SEMANTIC_PRIMID)
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: ignore VAO IDs equal to 0 in glDeleteVertexArrays
This patch is
Reviewed-by: Ian Romanick
Is there a piglit test? I wonder how many other glDeleteFoo functions
mishandle the id=0 case. :(
On 08/30/2018 12:16 PM, Marek Olšák wrote:
> From: Marek Olšák
>
> This fixes a firefox crash.
>
> Fixes: 781a78914c798dc64005b37c6ca1224ce06803fc
> ---
> src/mesa/main/arrayobj.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
> index a23031fe182..6e0665c0e5d 100644
> --- a/src/mesa/main/arrayobj.c
> +++ b/src/mesa/main/arrayobj.c
> @@ -1007,20 +1007,24 @@ _mesa_BindVertexArray(GLuint id)
> *
> * \param n Number of array objects to delete.
> * \param idsArray of \c n array object IDs.
> */
> static void
> delete_vertex_arrays(struct gl_context *ctx, GLsizei n, const GLuint *ids)
> {
> GLsizei i;
>
> for (i = 0; i < n; i++) {
> + /* IDs equal to 0 should be silently ignored. */
> + if (!ids[i])
> + continue;
> +
>struct gl_vertex_array_object *obj = _mesa_lookup_vao(ctx, ids[i]);
>
>if (obj) {
> assert(obj->Name == ids[i]);
>
> /* If the array object is currently bound, the spec says "the
> binding
>* for that object reverts to zero and the default vertex array
>* becomes current."
>*/
> if (obj == ctx->Array.VAO)
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] intel: aubinator: Adding missed platforms to the error message.
Many new platforms got added to gen_device_name_to_pci_device_id()
but the error message inside aubinator didn't reflected those
changes. So syncing on the same order to be sure that we are not
missing any now.
Cc: Anuj Phogat
Cc: Matt Turner
Cc: Jordan Justen
Signed-off-by: Rodrigo Vivi
---
src/intel/tools/aubinator.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
index c22d191f14..edd11fe0f5 100644
--- a/src/intel/tools/aubinator.c
+++ b/src/intel/tools/aubinator.c
@@ -280,8 +280,9 @@ int main(int argc, char *argv[])
case 'g': {
const int id = gen_device_name_to_pci_device_id(optarg);
if (id < 0) {
-fprintf(stderr, "can't parse gen: '%s', expected ivb, byt, hsw, "
- "bdw, chv, skl, kbl or bxt\n", optarg);
+fprintf(stderr, "can't parse gen: '%s', expected brw, g4x, ilk, "
+"snb, ivb, hsw, byt, bdw, chv, skl, bxt, kbl, "
+"glk, cfl, cnl, icl", optarg);
exit(EXIT_FAILURE);
} else {
pci_id = id;
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] intel: Introducing Whiskey Lake platform
Whiskey Lake uses the same gen graphics as Coffe Lake, including some
ids that were previously marked as reserved on Coffe Lake, but that
now are moved to WHL page.
This follows the ids and approach used on kernel's commit
b9be78531d27 ("drm/i915/whl: Introducing Whiskey Lake platform")
Cc: José Roberto de Souza
Cc: Anuj Phogat
Signed-off-by: Rodrigo Vivi
---
include/pci_ids/i965_pci_ids.h | 10 +-
src/intel/compiler/test_eu_validate.cpp | 1 +
src/intel/dev/gen_device_info.c | 1 +
src/intel/tools/aubinator.c | 2 +-
4 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/include/pci_ids/i965_pci_ids.h b/include/pci_ids/i965_pci_ids.h
index 4efac638e9..d151b3dd0e 100644
--- a/include/pci_ids/i965_pci_ids.h
+++ b/include/pci_ids/i965_pci_ids.h
@@ -170,8 +170,6 @@ CHIPSET(0x3185, glk_2x6, "Intel(R) UHD Graphics 600
(Geminilake 2x6)")
CHIPSET(0x3E90, cfl_gt1, "Intel(R) UHD Graphics 610 (Coffeelake 2x6 GT1)")
CHIPSET(0x3E93, cfl_gt1, "Intel(R) UHD Graphics 610 (Coffeelake 2x6 GT1)")
CHIPSET(0x3E99, cfl_gt1, "Intel(R) HD Graphics (Coffeelake 2x6 GT1)")
-CHIPSET(0x3EA1, cfl_gt1, "Intel(R) HD Graphics (Coffeelake 2x6 GT1)")
-CHIPSET(0x3EA4, cfl_gt1, "Intel(R) HD Graphics (Coffeelake 2x6 GT1)")
CHIPSET(0x3E91, cfl_gt2, "Intel(R) UHD Graphics 630 (Coffeelake 3x8 GT2)")
CHIPSET(0x3E92, cfl_gt2, "Intel(R) UHD Graphics 630 (Coffeelake 3x8 GT2)")
CHIPSET(0x3E96, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
@@ -179,14 +177,16 @@ CHIPSET(0x3E98, cfl_gt2, "Intel(R) HD Graphics
(Coffeelake 3x8 GT2)")
CHIPSET(0x3E9A, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
CHIPSET(0x3E9B, cfl_gt2, "Intel(R) UHD Graphics 630 (Coffeelake 3x8 GT2)")
CHIPSET(0x3E94, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
-CHIPSET(0x3EA0, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
-CHIPSET(0x3EA3, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
CHIPSET(0x3EA9, cfl_gt2, "Intel(R) HD Graphics (Coffeelake 3x8 GT2)")
-CHIPSET(0x3EA2, cfl_gt3, "Intel(R) HD Graphics (Coffeelake 3x8 GT3)")
CHIPSET(0x3EA5, cfl_gt3, "Intel(R) HD Graphics (Coffeelake 3x8 GT3)")
CHIPSET(0x3EA6, cfl_gt3, "Intel(R) HD Graphics (Coffeelake 3x8 GT3)")
CHIPSET(0x3EA7, cfl_gt3, "Intel(R) HD Graphics (Coffeelake 3x8 GT3)")
CHIPSET(0x3EA8, cfl_gt3, "Intel(R) HD Graphics (Coffeelake 3x8 GT3)")
+CHIPSET(0x3EA1, cfl_gt1, "Intel(R) HD Graphics (Whiskey Lake 2x6 GT1)")
+CHIPSET(0x3EA0, cfl_gt2, "Intel(R) HD Graphics (Whiskey Lake 3x8 GT2)")
+CHIPSET(0x3EA2, cfl_gt3, "Intel(R) HD Graphics (Whiskey Lake 3x8 GT3)")
+CHIPSET(0x3EA3, cfl_gt3, "Intel(R) HD Graphics (Whiskey Lake 3x8 GT3)")
+CHIPSET(0x3EA4, cfl_gt3, "Intel(R) HD Graphics (Whiskey Lake 3x8 GT3)")
CHIPSET(0x5A49, cnl_2x8, "Intel(R) HD Graphics (Cannonlake 2x8 GT0.5)")
CHIPSET(0x5A4A, cnl_2x8, "Intel(R) HD Graphics (Cannonlake 2x8 GT0.5)")
CHIPSET(0x5A41, cnl_3x8, "Intel(R) HD Graphics (Cannonlake 3x8 GT1)")
diff --git a/src/intel/compiler/test_eu_validate.cpp
b/src/intel/compiler/test_eu_validate.cpp
index 744ae5806d..73300b2312 100644
--- a/src/intel/compiler/test_eu_validate.cpp
+++ b/src/intel/compiler/test_eu_validate.cpp
@@ -43,6 +43,7 @@ static const struct gen_info {
{ "aml", },
{ "glk", },
{ "cfl", },
+ { "whl", },
{ "cnl", },
{ "icl", },
};
diff --git a/src/intel/dev/gen_device_info.c b/src/intel/dev/gen_device_info.c
index 3cece52a04..0f12d17a84 100644
--- a/src/intel/dev/gen_device_info.c
+++ b/src/intel/dev/gen_device_info.c
@@ -60,6 +60,7 @@ gen_device_name_to_pci_device_id(const char *name)
{ "aml", 0x591C },
{ "glk", 0x3185 },
{ "cfl", 0x3E9B },
+ { "whl", 0x3EA1 },
{ "cnl", 0x5a52 },
{ "icl", 0x8a52 },
};
diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
index 55672fa073..7de20f3d7a 100644
--- a/src/intel/tools/aubinator.c
+++ b/src/intel/tools/aubinator.c
@@ -282,7 +282,7 @@ int main(int argc, char *argv[])
if (id < 0) {
fprintf(stderr, "can't parse gen: '%s', expected brw, g4x, ilk, "
"snb, ivb, hsw, byt, bdw, chv, skl, bxt, kbl, "
-"aml, glk, cfl, cnl, icl", optarg);
+"aml, glk, cfl, whl, cnl, icl", optarg);
exit(EXIT_FAILURE);
} else {
pci_id = id;
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] intel: Introducing Amber Lake platform
Amber Lake uses the same gen graphics as Kaby Lake, including a id
that were previously marked as reserved on Kaby Lake, but that
now is moved to AML page.
This follows the ids and approach used on kernel's commit
e364672477a1 ("drm/i915/aml: Introducing Amber Lake platform")
Reported-by: Timo Aaltonen
Cc: José Roberto de Souza
Cc: Anuj Phogat
Signed-off-by: Rodrigo Vivi
---
include/pci_ids/i965_pci_ids.h | 3 ++-
src/intel/compiler/test_eu_validate.cpp | 1 +
src/intel/dev/gen_device_info.c | 1 +
src/intel/tools/aubinator.c | 2 +-
4 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/pci_ids/i965_pci_ids.h b/include/pci_ids/i965_pci_ids.h
index bced44e288..4efac638e9 100644
--- a/include/pci_ids/i965_pci_ids.h
+++ b/include/pci_ids/i965_pci_ids.h
@@ -156,7 +156,6 @@ CHIPSET(0x5912, kbl_gt2, "Intel(R) HD Graphics 630 (Kaby
Lake GT2)")
CHIPSET(0x5916, kbl_gt2, "Intel(R) HD Graphics 620 (Kaby Lake GT2)")
CHIPSET(0x591A, kbl_gt2, "Intel(R) HD Graphics P630 (Kaby Lake GT2)")
CHIPSET(0x591B, kbl_gt2, "Intel(R) HD Graphics 630 (Kaby Lake GT2)")
-CHIPSET(0x591C, kbl_gt2, "Intel(R) Kaby Lake GT2")
CHIPSET(0x591D, kbl_gt2, "Intel(R) HD Graphics P630 (Kaby Lake GT2)")
CHIPSET(0x591E, kbl_gt2, "Intel(R) HD Graphics 615 (Kaby Lake GT2)")
CHIPSET(0x5921, kbl_gt2, "Intel(R) Kabylake GT2F")
@@ -164,6 +163,8 @@ CHIPSET(0x5923, kbl_gt3, "Intel(R) Kabylake GT3")
CHIPSET(0x5926, kbl_gt3, "Intel(R) Iris Plus Graphics 640 (Kaby Lake GT3e)")
CHIPSET(0x5927, kbl_gt3, "Intel(R) Iris Plus Graphics 650 (Kaby Lake GT3e)")
CHIPSET(0x593B, kbl_gt4, "Intel(R) Kabylake GT4")
+CHIPSET(0x591C, kbl_gt2, "Intel(R) Amber Lake GT2")
+CHIPSET(0x87C0, kbl_gt2, "Intel(R) Amber Lake GT2")
CHIPSET(0x3184, glk, "Intel(R) UHD Graphics 605 (Geminilake)")
CHIPSET(0x3185, glk_2x6, "Intel(R) UHD Graphics 600 (Geminilake 2x6)")
CHIPSET(0x3E90, cfl_gt1, "Intel(R) UHD Graphics 610 (Coffeelake 2x6 GT1)")
diff --git a/src/intel/compiler/test_eu_validate.cpp
b/src/intel/compiler/test_eu_validate.cpp
index b132b87a1a..744ae5806d 100644
--- a/src/intel/compiler/test_eu_validate.cpp
+++ b/src/intel/compiler/test_eu_validate.cpp
@@ -40,6 +40,7 @@ static const struct gen_info {
{ "skl", },
{ "bxt", },
{ "kbl", },
+ { "aml", },
{ "glk", },
{ "cfl", },
{ "cnl", },
diff --git a/src/intel/dev/gen_device_info.c b/src/intel/dev/gen_device_info.c
index b0ae4d1803..3cece52a04 100644
--- a/src/intel/dev/gen_device_info.c
+++ b/src/intel/dev/gen_device_info.c
@@ -57,6 +57,7 @@ gen_device_name_to_pci_device_id(const char *name)
{ "skl", 0x1912 },
{ "bxt", 0x5A85 },
{ "kbl", 0x5912 },
+ { "aml", 0x591C },
{ "glk", 0x3185 },
{ "cfl", 0x3E9B },
{ "cnl", 0x5a52 },
diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
index edd11fe0f5..55672fa073 100644
--- a/src/intel/tools/aubinator.c
+++ b/src/intel/tools/aubinator.c
@@ -282,7 +282,7 @@ int main(int argc, char *argv[])
if (id < 0) {
fprintf(stderr, "can't parse gen: '%s', expected brw, g4x, ilk, "
"snb, ivb, hsw, byt, bdw, chv, skl, bxt, kbl, "
-"glk, cfl, cnl, icl", optarg);
+"aml, glk, cfl, cnl, icl", optarg);
exit(EXIT_FAILURE);
} else {
pci_id = id;
--
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] i965/gen7_urb: Re-emit PUSH_CONSTANT_ALLOC on some gen9
On Wednesday, August 29, 2018 1:38:51 PM PDT Nanley Chery wrote:
> According to internal docs, some gen9 platforms have a pixel shader push
> constant synchronization issue. Although not listed among said
> platforms, this issue seems to be present on the GeminiLake 2x6's we've
> tested.
>
> We consider the available workarounds to be too detrimental on
> performance. Instead, we mitigate the issue by applying part of one of
> the workarounds. Re-emit PUSH_CONSTANT_ALLOC at the top of every batch
> (as suggested by Ken).
>
> Fixes ext_framebuffer_multisample-accuracy piglit test failures with the
> following options:
> * 6 depth_draw small depthstencil
> * 8 stencil_draw small depthstencil
> * 6 stencil_draw small depthstencil
> * 8 depth_resolve small
> * 6 stencil_resolve small depthstencil
> * 4 stencil_draw small depthstencil
> * 16 stencil_draw small depthstencil
> * 16 depth_draw small depthstencil
> * 2 stencil_resolve small depthstencil
> * 6 stencil_draw small
> * all_samples stencil_draw small
> * 2 depth_draw small depthstencil
> * all_samples depth_draw small depthstencil
> * all_samples stencil_resolve small
> * 4 depth_draw small depthstencil
> * all_samples depth_draw small
> * all_samples stencil_draw small depthstencil
> * 4 stencil_resolve small depthstencil
> * 4 depth_resolve small depthstencil
> * all_samples stencil_resolve small depthstencil
>
> v2: Include more platforms in WA (Ken).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106865
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93355
> Cc:
> Tested-by: Mark Janes
> ---
> src/mesa/drivers/dri/i965/gen7_urb.c | 28
> 1 file changed, 28 insertions(+)
>
> I'm not sure I have enough information about what's happening in the HW
> to create a piglit test for this issue.
>
> diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c
> b/src/mesa/drivers/dri/i965/gen7_urb.c
> index 2e5f8e60ba9..e7259fc1b8d 100644
> --- a/src/mesa/drivers/dri/i965/gen7_urb.c
> +++ b/src/mesa/drivers/dri/i965/gen7_urb.c
> @@ -118,6 +118,33 @@ gen7_emit_push_constant_state(struct brw_context *brw,
> unsigned vs_size,
> const struct gen_device_info *devinfo = >screen->devinfo;
> unsigned offset = 0;
>
> + /* From the SKL PRM, Workarounds section (#878):
> +*
> +*Push constant buffer corruption possible. WA: Insert 2 zero-length
> +*PushConst_PS before every intended PushConst_PS update, issue a
> +*NULLPRIM after each of the zero len PC update to make sure CS
> commits
> +*them.
> +*
> +* This workaround is attempting to solve a pixel shader push constant
> +* synchronization issue.
> +*
> +* There's an unpublished WA that involves re-emitting
> +* 3DSTATE_PUSH_CONSTANT_ALLOC_PS for every 500-ish 3DSTATE_CONSTANT_PS
> +* packets. Since our counting methods may not be reliable due to
> +* context-switching and pre-emption, we instead choose to approximate
> this
> +* behavior by re-emitting the packet at the top of the batch.
> +*/
> + if (brw->ctx.NewDriverState == BRW_NEW_BATCH) {
> + /* SKL GT2 and GLK 2x6 have reliably demonstrated this issue thus far.
> +* We've also seen some intermittent failures from SKL GT4 and BXT in
> +* the past.
> +*/
> + if (!devinfo->is_skylake &&
> + !devinfo->is_broxton &&
> + !devinfo->is_geminilake)
> + return;
> + }
> +
> BEGIN_BATCH(10);
> OUT_BATCH(_3DSTATE_PUSH_CONSTANT_ALLOC_VS << 16 | (2 - 2));
> OUT_BATCH(vs_size | offset << GEN7_PUSH_CONSTANT_BUFFER_OFFSET_SHIFT);
> @@ -154,6 +181,7 @@ const struct brw_tracked_state gen7_push_constant_space =
> {
> .dirty = {
>.mesa = 0,
>.brw = BRW_NEW_CONTEXT |
> + BRW_NEW_BATCH | /* Push constant workaround */
> BRW_NEW_GEOMETRY_PROGRAM |
> BRW_NEW_TESS_PROGRAMS,
> },
>
Not sure we can do much better than this. Thanks for taking care of
this, Nanley.
Reviewed-by: Kenneth Graunke
signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 107765] [regression] Batman Arkham City crashes with DXVK under wine
https://bugs.freedesktop.org/show_bug.cgi?id=107765 Bug ID: 107765 Summary: [regression] Batman Arkham City crashes with DXVK under wine Product: Mesa Version: 18.2 Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Vulkan/radeon Assignee: mesa-dev@lists.freedesktop.org Reporter: farmboy0+freedesk...@googlemail.com QA Contact: mesa-dev@lists.freedesktop.org ../mesa-18.2.0-rc5/src/amd/vulkan/radv_device.c:3903: FINISHME: Illegal color teamapps\common\Batman Arkham City GOTY\Binaries\Win32\BatmanAC.exe: ../mesa-18.2.0-rc5/src/amd/vulkan/radv_pipeline.c:486: si_choose_spi_color_format: Assertion `!"unhandled blend format"' failed. -- 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] mesa: ignore VAO IDs equal to 0 in glDeleteVertexArrays
From: Marek Olšák
This fixes a firefox crash.
Fixes: 781a78914c798dc64005b37c6ca1224ce06803fc
---
src/mesa/main/arrayobj.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
index a23031fe182..6e0665c0e5d 100644
--- a/src/mesa/main/arrayobj.c
+++ b/src/mesa/main/arrayobj.c
@@ -1007,20 +1007,24 @@ _mesa_BindVertexArray(GLuint id)
*
* \param n Number of array objects to delete.
* \param idsArray of \c n array object IDs.
*/
static void
delete_vertex_arrays(struct gl_context *ctx, GLsizei n, const GLuint *ids)
{
GLsizei i;
for (i = 0; i < n; i++) {
+ /* IDs equal to 0 should be silently ignored. */
+ if (!ids[i])
+ continue;
+
struct gl_vertex_array_object *obj = _mesa_lookup_vao(ctx, ids[i]);
if (obj) {
assert(obj->Name == ids[i]);
/* If the array object is currently bound, the spec says "the binding
* for that object reverts to zero and the default vertex array
* becomes current."
*/
if (obj == ctx->Array.VAO)
--
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/nir: Lowering image loads and stores trashes all metadata
Thanks! On Thu, Aug 30, 2018 at 1:41 PM Caio Marcelo de Oliveira Filho < caio.olive...@intel.com> wrote: > Jason Ekstrand writes: > > > This fixes the GL_ARB_fragment_shader_interlock piglit test on gen8 > > platforms where the lack of metadata dirtying was causing another pass > > to accidentally delete a much needed loop. > > > > Cc: Kenneth Graunke > > https://bugs.freedesktop.org/show_bug.cgi?id=107745 > > Fixes: 37f7983bcca1 "intel/compiler: Do image load/store lowering..." > > --- > > src/intel/compiler/brw_nir_lower_image_load_store.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > Reviewed-by: Caio Marcelo de Oliveira Filho > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] intel/nir: Lowering image loads and stores trashes all metadata
Jason Ekstrand writes: > This fixes the GL_ARB_fragment_shader_interlock piglit test on gen8 > platforms where the lack of metadata dirtying was causing another pass > to accidentally delete a much needed loop. > > Cc: Kenneth Graunke > https://bugs.freedesktop.org/show_bug.cgi?id=107745 > Fixes: 37f7983bcca1 "intel/compiler: Do image load/store lowering..." > --- > src/intel/compiler/brw_nir_lower_image_load_store.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Caio Marcelo de Oliveira Filho ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] intel/nir: Lowering image loads and stores trashes all metadata
This fixes the GL_ARB_fragment_shader_interlock piglit test on gen8 platforms where the lack of metadata dirtying was causing another pass to accidentally delete a much needed loop. Cc: Kenneth Graunke https://bugs.freedesktop.org/show_bug.cgi?id=107745 Fixes: 37f7983bcca1 "intel/compiler: Do image load/store lowering..." --- src/intel/compiler/brw_nir_lower_image_load_store.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/intel/compiler/brw_nir_lower_image_load_store.c b/src/intel/compiler/brw_nir_lower_image_load_store.c index de6f7683be4..e8083a80cb7 100644 --- a/src/intel/compiler/brw_nir_lower_image_load_store.c +++ b/src/intel/compiler/brw_nir_lower_image_load_store.c @@ -817,8 +817,8 @@ brw_nir_lower_image_load_store(nir_shader *shader, } } - nir_metadata_preserve(function->impl, nir_metadata_block_index | -nir_metadata_dominance); + if (progress) + nir_metadata_preserve(function->impl, nir_metadata_none); } return progress; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/7] gallium: add PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER{S, _BUFFERS}
On Thu, Aug 30, 2018 at 03:40:16PM +0200, Erik Faye-Lund wrote:
> This moves the evergreen-specific max-sizes out as a driver-cap, so
> other drivers with less strict requirements also can use hw-atomics.
>
> Remove ssbo_atomic as it's no longer needed.
>
> We should now be able to use hw-atomics for some stages and not for
> other, if needed.
>
> Signed-off-by: Erik Faye-Lund
Etnaviv part
Reviewed-by: Wladimir J. van der Laan
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c
> b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> index 108b97d35c..95166a2db1 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
> @@ -372,6 +372,11 @@ etna_screen_get_param(struct pipe_screen *pscreen, enum
> pipe_cap param)
>return 0;
> case PIPE_CAP_UMA:
>return 1;
> +
> + /* hw atomic counters */
> + case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTERS:
> + case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS:
> + return 0;
> }
>
> debug_printf("unknown param %d", param);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/7] gallium: add PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS
On Thu, Aug 30, 2018 at 03:40:15PM +0200, Erik Faye-Lund wrote: > This gets rid of a r600 specific hack in the state-tracker, and prepares > for other drivers to be able to use hw-atomics. > > While we're at it, clean up some indentation in the various drivers. > > Signed-off-by: Erik Faye-Lund Etnaviv part Reviewed-by: Wladimir J. van der Laan > diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c > b/src/gallium/drivers/etnaviv/etnaviv_screen.c > index 9669bd2f60..108b97d35c 100644 > --- a/src/gallium/drivers/etnaviv/etnaviv_screen.c > +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c > @@ -289,8 +289,11 @@ etna_screen_get_param(struct pipe_screen *pscreen, enum > pipe_cap param) > case PIPE_CAP_MAX_GS_INVOCATIONS: >return 32; > > + /* shader buffer objects */ > case PIPE_CAP_MAX_SHADER_BUFFER_SIZE: >return 1 << 27; > + case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS: > + return 0; > > /* Stream output. */ > case PIPE_CAP_MAX_STREAM_OUTPUT_BUFFERS: ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 107457] [Tracker] Mesa 18.2 release tracker
https://bugs.freedesktop.org/show_bug.cgi?id=107457 Bug 107457 depends on bug 107223, which changed state. Bug 107223 Summary: [GEN9+] 50% perf drop in SynMark Fill* tests (E2E RBC gets disabled?) https://bugs.freedesktop.org/show_bug.cgi?id=107223 What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED -- 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] pipe-loader: add a dup() in pipe_loader_sw_probe_kms
From: Emil Velikov
The pipe_loader_release API closes the fd given, even if the pipe-loader
should _not_ take ownership of it.
With earlier commit we fixed pipe_loader_drm_probe_fd, and now with
cover the final piece.
Note that unlike the DRM case, here the caller _did_ forget to dup
before using it ... most likely leading to all sorts of fun.
Don't forget the close in the error path. Seems like the things are a
bit leaky/asymmetrical with the semi-recent config work. But we can shave
that yak another day ;-)
Signed-off-by: Emil Velikov
---
Strictly speaking we could add the dup() in st/dri sending that for stable.
Since there aren't that many users of kms_swarast to actually notice the
problem, I went with only one patch ;-)
Can rework if people prefer.
---
src/gallium/auxiliary/pipe-loader/pipe_loader.h| 3 +++
src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c | 11 +--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
index cbc9f3af9b1..be40f98d5fc 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader.h
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader.h
@@ -146,6 +146,9 @@ pipe_loader_sw_probe_dri(struct pipe_loader_device **devs,
*
* This function is platform-specific.
*
+ * Function does not take ownership of the fd, but duplicates it locally.
+ * The local fd is closed during pipe_loader_release.
+ *
* \sa pipe_loader_probe
*/
bool
diff --git a/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c
b/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c
index 84894c0caf6..d387ce90d32 100644
--- a/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c
@@ -25,6 +25,10 @@
*
**/
+#ifdef HAVE_PIPE_LOADER_KMS
+#include
+#endif
+
#include "pipe_loader_priv.h"
#include "util/u_memory.h"
@@ -171,11 +175,12 @@ pipe_loader_sw_probe_kms(struct pipe_loader_device
**devs, int fd)
if (!pipe_loader_sw_probe_init_common(sdev))
goto fail;
- sdev->fd = fd;
+ if (fd < 0 || (sdev->fd = fcntl(fd, F_DUPFD_CLOEXEC, 3)) < 0)
+ goto fail;
for (i = 0; sdev->dd->winsys[i].name; i++) {
if (strcmp(sdev->dd->winsys[i].name, "kms_dri") == 0) {
- sdev->ws = sdev->dd->winsys[i].create_winsys(fd);
+ sdev->ws = sdev->dd->winsys[i].create_winsys(sdev->fd);
break;
}
}
@@ -187,6 +192,8 @@ pipe_loader_sw_probe_kms(struct pipe_loader_device **devs,
int fd)
fail:
pipe_loader_sw_probe_teardown_common(sdev);
+ if (sdev->fd != -1)
+ close(sdev->fd);
FREE(sdev);
return false;
}
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/4] u_vbuf: Fix leak
Reported by Coverity: data is heap-allocated, but only freed in the info->index_size != 0 branch. CID: 1438238 Signed-off-by: Ernestas Kulik --- src/gallium/auxiliary/util/u_vbuf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gallium/auxiliary/util/u_vbuf.c b/src/gallium/auxiliary/util/u_vbuf.c index a7a8a3be21..f721613cbc 100644 --- a/src/gallium/auxiliary/util/u_vbuf.c +++ b/src/gallium/auxiliary/util/u_vbuf.c @@ -1334,6 +1334,7 @@ void u_vbuf_draw_vbo(struct u_vbuf *mgr, const struct pipe_draw_info *info) end_vertex = MAX2(end_vertex, start + count); end_instance = MAX2(end_instance, start_instance + instance_count); } + free(data); /* Set the final counts. */ new_info.count = end_vertex - new_info.start; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/4] vc4: Fix leak
Reported by Coverity: in the case where there exist hardware and non-hardware queries, the code does not jump to err_free_query and leaks the query. CID: 1430194 Signed-off-by: Ernestas Kulik --- src/gallium/drivers/vc4/vc4_query.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/drivers/vc4/vc4_query.c b/src/gallium/drivers/vc4/vc4_query.c index 6e4681e93c..f08785f457 100644 --- a/src/gallium/drivers/vc4/vc4_query.c +++ b/src/gallium/drivers/vc4/vc4_query.c @@ -132,7 +132,7 @@ vc4_create_batch_query(struct pipe_context *pctx, unsigned num_queries, /* We can't mix HW and non-HW queries. */ if (nhwqueries && nhwqueries != num_queries) -return NULL; +goto err_free_query; if (!nhwqueries) return (struct pipe_query *)query; -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/4] v3d: Fix leak
Reported by Coverity: in the case of unsupported modifier request, the
code does not jump to the “fail” label to destroy the acquired resource.
CID: 1435704
Signed-off-by: Ernestas Kulik
---
src/gallium/drivers/v3d/v3d_resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/drivers/v3d/v3d_resource.c
b/src/gallium/drivers/v3d/v3d_resource.c
index 8bf6a97c39..cdb6b4861c 100644
--- a/src/gallium/drivers/v3d/v3d_resource.c
+++ b/src/gallium/drivers/v3d/v3d_resource.c
@@ -669,7 +669,7 @@ v3d_resource_create_with_modifiers(struct pipe_screen
*pscreen,
rsc->tiled = false;
} else {
fprintf(stderr, "Unsupported modifier requested\n");
-return NULL;
+goto fail;
}
rsc->internal_format = prsc->format;
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/4] Coverity resource leak fixes
This patch series takes care of several reported resource leaks. Ernestas Kulik (4): glsl_to_tgsi: Fix potential leak u_vbuf: Fix leak v3d: Fix leak vc4: Fix leak src/gallium/auxiliary/util/u_vbuf.c| 1 + src/gallium/drivers/v3d/v3d_resource.c | 2 +- src/gallium/drivers/vc4/vc4_query.c| 2 +- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 7 --- 4 files changed, 7 insertions(+), 5 deletions(-) -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/4] glsl_to_tgsi: Fix potential leak
Reported by Coverity: arr_live_ranges is freed in a different branch than the one in which it was allocated. CID: 1438391 Signed-off-by: Ernestas Kulik --- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 7b96947c60..68573f628d 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -5616,10 +5616,11 @@ glsl_to_tgsi_visitor::merge_registers(void) this->next_array = merge_arrays(this->next_array, this->array_sizes, >instructions, arr_live_ranges); - - if (arr_live_ranges) -delete[] arr_live_ranges; } + + if (arr_live_ranges) + delete[] arr_live_ranges; + ralloc_free(reg_live_ranges); } -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] intel: decoder: unify MI_BB_START field naming
On 30/08/2018 16:23, Dylan Baker wrote:
Quoting Lionel Landwerlin (2018-08-30 06:58:39)
On 28/08/2018 17:01, Dylan Baker wrote:
Quoting Lionel Landwerlin (2018-08-28 02:39:58)
Yes, I think so. You asked on another commit too, both are related and
this depends on other commits from Jason.
Here is a list in order of cherry picking :
commit f430a37fa75f534c3a114b0ec546fa14f05f5da1
Author: Lionel Landwerlin
Date: Tue Aug 14 11:22:12 2018 +0100
intel: decoder: unify MI_BB_START field naming
commit 2abd7ae189135eb5a1f530a3a1c9412d3a7e238d
Author: Jason Ekstrand
Date: Fri Aug 24 15:23:04 2018 -0500
intel/decoder: Clean up field iteration and fix sub-dword fields
commit cbd4bc1346f7397242e157bb66099b950a8c5643
Author: Jason Ekstrand
Date: Fri Aug 24 16:04:03 2018 -0500
intel/batch_decoder: Fix dynamic state printing
commit 70de31d0c106f58d6b7e6d5b79b8d90c1c112a3b
Author: Jason Ekstrand
Date: Fri Aug 24 16:05:08 2018 -0500
intel/batch_decoder: Print blend states properly
commit 440a988bd1478bb33dafcbb8575473bc643ae383
Author: Lionel Landwerlin
Date: Sat Aug 25 18:22:00 2018 +0100
intel: decoder: handle 0 sized structs
Thanks,
-
Lionel
On 27/08/2018 22:20, Andres Gomez wrote:
Lionel, should we also include this in the stable queues ?
On Tue, 2018-08-14 at 11:26 +0100, Lionel Landwerlin wrote:
The batch decoder looks for a field with a particular name to decide
whether an MI_BB_START leads into a second batch buffer level. Because
the names are different between Gen7.5/8 and the newer generation we
fail that test and keep on reading (invalid) instructions.
Signed-off-by: Lionel Landwerlin
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
---
src/intel/genxml/gen75.xml | 6 +++---
src/intel/genxml/gen8.xml | 6 +++---
src/intel/vulkan/anv_batch_chain.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
index 5b01fd45400..dfc3d891498 100644
--- a/src/intel/genxml/gen75.xml
+++ b/src/intel/genxml/gen75.xml
@@ -2314,9 +2314,9 @@
-
-
-
+
+
+
diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
index 4ed41d15612..330366b7ed0 100644
--- a/src/intel/genxml/gen8.xml
+++ b/src/intel/genxml/gen8.xml
@@ -2553,9 +2553,9 @@
-
-
-
+
+
+
diff --git a/src/intel/vulkan/anv_batch_chain.c
b/src/intel/vulkan/anv_batch_chain.c
index c47a81c8a4d..0f7c8325ea4 100644
--- a/src/intel/vulkan/anv_batch_chain.c
+++ b/src/intel/vulkan/anv_batch_chain.c
@@ -531,7 +531,7 @@ emit_batch_buffer_start(struct anv_cmd_buffer *cmd_buffer,
anv_batch_emit(_buffer->batch, GEN8_MI_BATCH_BUFFER_START, bbs) {
bbs.DWordLength = cmd_buffer->device->info.gen < 8 ?
gen7_length : gen8_length;
- bbs._2ndLevelBatchBuffer = _1stlevelbatch;
+ bbs.SecondLevelBatchBuffer= Firstlevelbatch;
bbs.AddressSpaceIndicator = ASI_PPGTT;
bbs.BatchBufferStartAddress = (struct anv_address) { bo, offset };
}
Hi Lionel,
Only patches 1 and 3 of that list apply cleanly to the 18.1 branch, it looks
like maybe I need a few more patches for things to apply cleanly?
Dylan
Yeah... Looks like this might pull in the world...
Thinking about it again, how much do we need INTEL_DEBUG=batch fixed in
stable?
-
Lionel
If it applies cleanly I think it's fine, but I don't think it's worth it if it's
going to require a lot of other patches. So just drop this for 18.1?
Dylan
Yes, let's just drop it.
Thanks,
-
Lionel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] mesa: allow GL_UNSIGNED_BYTE type for SNORM reads
Quoting Tapani Pälli (2018-08-29 21:26:08)
>
>
> On 08/29/2018 06:22 PM, Dylan Baker wrote:
> > Quoting Tapani Pälli (2018-08-27 04:46:37)
> >> OpenGL ES spec states:
> >> "For normalized fixed-point rendering surfaces, the combination format
> >> RGBA and type UNSIGNED_BYTE is accepted."
> >>
> >> This fixes following failing VK-GL-CTS tests:
> >>
> >> KHR-GLES3.packed_pixels.pbo_rectangle.rgba8_snorm
> >> KHR-GLES3.packed_pixels.rectangle.rgba8_snorm
> >> KHR-GLES3.packed_pixels.varied_rectangle.rgba8_snorm
> >>
> >> Signed-off-by: Tapani Pälli
> >> https://bugs.freedesktop.org/show_bug.cgi?id=107658
> >> Cc: mesa-sta...@lists.freedesktop.org
> >> ---
> >>
> >> This is a partial fix to the bug. I believe there are 2 separate
> >> issues within reported bug and this fixes the first one.
> >>
> >> src/mesa/main/readpix.c | 9 +
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
> >> index 2cbb578a37f..556c860d393 100644
> >> --- a/src/mesa/main/readpix.c
> >> +++ b/src/mesa/main/readpix.c
> >> @@ -958,6 +958,15 @@ read_pixels_es3_error_check(struct gl_context *ctx,
> >> GLenum format, GLenum type,
> >> return GL_NO_ERROR;
> >>}
> >> }
> >> + if (type == GL_UNSIGNED_BYTE) {
> >> + switch (internalFormat) {
> >> + case GL_R8_SNORM:
> >> + case GL_RG8_SNORM:
> >> + case GL_RGBA8_SNORM:
> >> +if (_mesa_has_EXT_render_snorm(ctx))
> >> + return GL_NO_ERROR;
> >> + }
> >> + }
> >> break;
> >> case GL_BGRA:
> >> /* GL_EXT_read_format_bgra */
> >> --
> >> 2.14.4
> >>
> >
> > Hi Tapani,
> >
> > This doesn't apply cleanly to 18.1 because "mesa: enable EXT_render_snorm
> > extension" isn't present on the branch. Does it still make sense to pull
> > this
> > into 18.1?
> >
>
> Ah nope, patch makes sense only with EXT_render_snorm.
>
> // Tapani
Cool. Thanks for following up, I've added this to the ignore list for 18.1.
Dylan
signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] anv: blorp: support multiple aspect blits
Quoting Lionel Landwerlin (2018-08-30 06:42:06)
> Newer blit tests are enabling depth blits. We currently don't
> support it but can do by iterating over the aspects masks (copy some
> logic from the CopyImage function).
>
> Signed-off-by: Lionel Landwerlin
> Fixes: 9f44745eca0e41 ("anv: Use blorp to implement VkBlitImage")
> Reviewed-by: Jason Ekstrand
> (cherry picked from commit 5a1c23d1502d275c4d554c586bf029e66131f4ac)
> ---
> src/intel/vulkan/anv_blorp.c | 146 ++-
> 1 file changed, 75 insertions(+), 71 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index 68e2ed65c29..37f68790889 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -533,82 +533,86 @@ void anv_CmdBlitImage(
>const VkImageSubresourceLayers *src_res = [r].srcSubresource;
>const VkImageSubresourceLayers *dst_res = [r].dstSubresource;
>
> - get_blorp_surf_for_anv_image(cmd_buffer->device,
> - src_image, src_res->aspectMask,
> - srcImageLayout, ISL_AUX_USAGE_NONE, );
> - get_blorp_surf_for_anv_image(cmd_buffer->device,
> - dst_image, dst_res->aspectMask,
> - dstImageLayout, ISL_AUX_USAGE_NONE, );
> -
> - struct anv_format_plane src_format =
> - anv_get_format_plane(_buffer->device->info,
> src_image->vk_format,
> - src_res->aspectMask, src_image->tiling);
> - struct anv_format_plane dst_format =
> - anv_get_format_plane(_buffer->device->info,
> dst_image->vk_format,
> - dst_res->aspectMask, dst_image->tiling);
> -
> - unsigned dst_start, dst_end;
> - if (dst_image->type == VK_IMAGE_TYPE_3D) {
> - assert(dst_res->baseArrayLayer == 0);
> - dst_start = pRegions[r].dstOffsets[0].z;
> - dst_end = pRegions[r].dstOffsets[1].z;
> - } else {
> - dst_start = dst_res->baseArrayLayer;
> - dst_end = dst_start + anv_get_layerCount(dst_image, dst_res);
> - }
> -
> - unsigned src_start, src_end;
> - if (src_image->type == VK_IMAGE_TYPE_3D) {
> - assert(src_res->baseArrayLayer == 0);
> - src_start = pRegions[r].srcOffsets[0].z;
> - src_end = pRegions[r].srcOffsets[1].z;
> - } else {
> - src_start = src_res->baseArrayLayer;
> - src_end = src_start + anv_get_layerCount(src_image, src_res);
> - }
> -
> - bool flip_z = flip_coords(_start, _end, _start, _end);
> - float src_z_step = (float)(src_end + 1 - src_start) /
> - (float)(dst_end + 1 - dst_start);
> + assert(anv_image_aspects_compatible(src_res->aspectMask,
> + dst_res->aspectMask));
> +
> + uint32_t aspect_bit;
> + anv_foreach_image_aspect_bit(aspect_bit, src_image,
> src_res->aspectMask) {
> + get_blorp_surf_for_anv_image(cmd_buffer->device,
> + src_image, 1U << aspect_bit,
> + srcImageLayout, ISL_AUX_USAGE_NONE,
> );
> + get_blorp_surf_for_anv_image(cmd_buffer->device,
> + dst_image, 1U << aspect_bit,
> + dstImageLayout, ISL_AUX_USAGE_NONE,
> );
> +
> + struct anv_format_plane src_format =
> +anv_get_format_plane(_buffer->device->info,
> src_image->vk_format,
> + 1U << aspect_bit, src_image->tiling);
> + struct anv_format_plane dst_format =
> +anv_get_format_plane(_buffer->device->info,
> dst_image->vk_format,
> + 1U << aspect_bit, dst_image->tiling);
> +
> + unsigned dst_start, dst_end;
> + if (dst_image->type == VK_IMAGE_TYPE_3D) {
> +assert(dst_res->baseArrayLayer == 0);
> +dst_start = pRegions[r].dstOffsets[0].z;
> +dst_end = pRegions[r].dstOffsets[1].z;
> + } else {
> +dst_start = dst_res->baseArrayLayer;
> +dst_end = dst_start + anv_get_layerCount(dst_image, dst_res);
> + }
>
> - if (flip_z) {
> - src_start = src_end;
> - src_z_step *= -1;
> - }
> + unsigned src_start, src_end;
> + if (src_image->type == VK_IMAGE_TYPE_3D) {
> +assert(src_res->baseArrayLayer == 0);
> +src_start = pRegions[r].srcOffsets[0].z;
> +src_end = pRegions[r].srcOffsets[1].z;
> + } else {
> +src_start = src_res->baseArrayLayer;
> +src_end = src_start + anv_get_layerCount(src_image, src_res);
> + }
>
> - unsigned src_x0 = pRegions[r].srcOffsets[0].x;
> - unsigned src_x1 = pRegions[r].srcOffsets[1].x;
> - unsigned dst_x0 = pRegions[r].dstOffsets[0].x;
> - unsigned
Re: [Mesa-dev] [Mesa-stable] [PATCH] intel: decoder: unify MI_BB_START field naming
Quoting Lionel Landwerlin (2018-08-30 06:58:39)
> On 28/08/2018 17:01, Dylan Baker wrote:
> > Quoting Lionel Landwerlin (2018-08-28 02:39:58)
> >> Yes, I think so. You asked on another commit too, both are related and
> >> this depends on other commits from Jason.
> >>
> >> Here is a list in order of cherry picking :
> >>
> >> commit f430a37fa75f534c3a114b0ec546fa14f05f5da1
> >> Author: Lionel Landwerlin
> >> Date: Tue Aug 14 11:22:12 2018 +0100
> >>
> >> intel: decoder: unify MI_BB_START field naming
> >>
> >> commit 2abd7ae189135eb5a1f530a3a1c9412d3a7e238d
> >> Author: Jason Ekstrand
> >> Date: Fri Aug 24 15:23:04 2018 -0500
> >>
> >> intel/decoder: Clean up field iteration and fix sub-dword fields
> >>
> >> commit cbd4bc1346f7397242e157bb66099b950a8c5643
> >> Author: Jason Ekstrand
> >> Date: Fri Aug 24 16:04:03 2018 -0500
> >>
> >> intel/batch_decoder: Fix dynamic state printing
> >>
> >> commit 70de31d0c106f58d6b7e6d5b79b8d90c1c112a3b
> >> Author: Jason Ekstrand
> >> Date: Fri Aug 24 16:05:08 2018 -0500
> >>
> >> intel/batch_decoder: Print blend states properly
> >>
> >>
> >> commit 440a988bd1478bb33dafcbb8575473bc643ae383
> >> Author: Lionel Landwerlin
> >> Date: Sat Aug 25 18:22:00 2018 +0100
> >>
> >> intel: decoder: handle 0 sized structs
> >>
> >> Thanks,
> >>
> >> -
> >> Lionel
> >>
> >> On 27/08/2018 22:20, Andres Gomez wrote:
> >>> Lionel, should we also include this in the stable queues ?
> >>>
> >>> On Tue, 2018-08-14 at 11:26 +0100, Lionel Landwerlin wrote:
> The batch decoder looks for a field with a particular name to decide
> whether an MI_BB_START leads into a second batch buffer level. Because
> the names are different between Gen7.5/8 and the newer generation we
> fail that test and keep on reading (invalid) instructions.
>
> Signed-off-by: Lionel Landwerlin
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
> ---
> src/intel/genxml/gen75.xml | 6 +++---
> src/intel/genxml/gen8.xml | 6 +++---
> src/intel/vulkan/anv_batch_chain.c | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
> index 5b01fd45400..dfc3d891498 100644
> --- a/src/intel/genxml/gen75.xml
> +++ b/src/intel/genxml/gen75.xml
> @@ -2314,9 +2314,9 @@
>
> default="0"/>
> default="49"/>
> - type="uint">
> -
> -
> + type="uint">
> +
> +
>
>
>
> diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
> index 4ed41d15612..330366b7ed0 100644
> --- a/src/intel/genxml/gen8.xml
> +++ b/src/intel/genxml/gen8.xml
> @@ -2553,9 +2553,9 @@
>
> default="0"/>
> default="49"/>
> - type="uint">
> -
> -
> + type="uint">
> +
> +
>
>
>
> diff --git a/src/intel/vulkan/anv_batch_chain.c
> b/src/intel/vulkan/anv_batch_chain.c
> index c47a81c8a4d..0f7c8325ea4 100644
> --- a/src/intel/vulkan/anv_batch_chain.c
> +++ b/src/intel/vulkan/anv_batch_chain.c
> @@ -531,7 +531,7 @@ emit_batch_buffer_start(struct anv_cmd_buffer
> *cmd_buffer,
> anv_batch_emit(_buffer->batch, GEN8_MI_BATCH_BUFFER_START,
> bbs) {
> bbs.DWordLength = cmd_buffer->device->info.gen <
> 8 ?
> gen7_length : gen8_length;
> - bbs._2ndLevelBatchBuffer = _1stlevelbatch;
> + bbs.SecondLevelBatchBuffer= Firstlevelbatch;
> bbs.AddressSpaceIndicator = ASI_PPGTT;
> bbs.BatchBufferStartAddress = (struct anv_address) { bo,
> offset };
> }
> > Hi Lionel,
> >
> > Only patches 1 and 3 of that list apply cleanly to the 18.1 branch, it looks
> > like maybe I need a few more patches for things to apply cleanly?
> >
> > Dylan
>
> Yeah... Looks like this might pull in the world...
> Thinking about it again, how much do we need INTEL_DEBUG=batch fixed in
> stable?
>
> -
> Lionel
If it applies cleanly I think it's fine, but I don't think it's worth it if it's
going to require a lot of other patches. So just drop this for 18.1?
Dylan
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: Deny persistent mappings of incoherent GTT mmaps
On more recent HW, the indirect writes via the GGTT are internally
buffered presenting a nuisance to trying to use them for persistent
client maps. (We cannot guarantee that any write by the client will
then be visible to either the GPU or third parties in a timely manner,
leading to corruption.) Fortunately, we try very hard not to even use
the GGTT for anything and even less so for persistent mmaps, so their
loss is unlikely to be noticed.
No piglits harmed.
Cc: Kenneth Graunke
Cc: Lionel Landwerlin
Cc: Joonas Lahtinen
---
include/drm-uapi/i915_drm.h | 22 +++
src/mesa/drivers/dri/i965/brw_bufmgr.c | 36
src/mesa/drivers/dri/i965/intel_screen.c | 3 ++
src/mesa/drivers/dri/i965/intel_screen.h | 1 +
4 files changed, 62 insertions(+)
diff --git a/include/drm-uapi/i915_drm.h b/include/drm-uapi/i915_drm.h
index 16e452aa12d..268b585f8a4 100644
--- a/include/drm-uapi/i915_drm.h
+++ b/include/drm-uapi/i915_drm.h
@@ -529,6 +529,28 @@ typedef struct drm_i915_irq_wait {
*/
#define I915_PARAM_CS_TIMESTAMP_FREQUENCY 51
+/*
+ * Once upon a time we supposed that writes through the GGTT would be
+ * immediately in physical memory (once flushed out of the CPU path). However,
+ * on a few different processors and chipsets, this is not necessarily the case
+ * as the writes appear to be buffered internally. Thus a read of the backing
+ * storage (physical memory) via a different path (with different physical tags
+ * to the indirect write via the GGTT) will see stale values from before
+ * the GGTT write. Inside the kernel, we can for the most part keep track of
+ * the different read/write domains in use (e.g. set-domain), but the
assumption
+ * of coherency is baked into the ABI, hence reporting its true state in this
+ * parameter.
+ *
+ * Reports true when writes via mmap_gtt are immediately visible following an
+ * lfence to flush the WCB.
+ *
+ * Reports false when writes via mmap_gtt are indeterminately delayed in an in
+ * internal buffer and are _not_ immediately visible to third parties accessing
+ * directly via mmap_cpu/mmap_wc. Use of mmap_gtt as part of an IPC
+ * communications channel when reporting false is strongly disadvised.
+ */
+#define I915_PARAM_MMAP_GTT_COHERENT 52
+
typedef struct drm_i915_getparam {
__s32 param;
/*
diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c
b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index f1675b191c1..6955c5c890c 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -1068,6 +1068,19 @@ brw_bo_map_wc(struct brw_context *brw, struct brw_bo
*bo, unsigned flags)
return bo->map_wc;
}
+static void
+bo_set_domain(struct brw_bo *bo, unsigned int read, unsigned int write)
+{
+ struct brw_bufmgr *bufmgr = bo->bufmgr;
+
+ struct drm_i915_gem_set_domain arg = {
+ .handle = bo->gem_handle,
+ .read_domains = read,
+ .write_domain = write,
+ };
+ drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_SET_DOMAIN, );
+}
+
/**
* Perform an uncached mapping via the GTT.
*
@@ -1095,6 +1108,25 @@ brw_bo_map_gtt(struct brw_context *brw, struct brw_bo
*bo, unsigned flags)
{
struct brw_bufmgr *bufmgr = bo->bufmgr;
+ /* Once upon a time we believed that there was no internal buffering of
+* the indirect writes via the Global GTT; that is once flushed from
+* the processor the write would be immediately visible to any one
+* else reading that memory location be they the GPU, kernel or another
+* client. As it turns out, on modern hardware there is an internal buffer
+* that cannot be directly flushed (e.g. using the sfence one would normally
+* use to flush the WCB) and so far the w/a requires us to do an uncached
+* mmio read, quite expensive and requires user cooperation. That is we
+* cannot simply support persistent user access to the GTT mmap buffers
+* as we have no means of flushing their writes in a timely manner.
+*/
+ if (flags & MAP_PERSISTENT &&
+ flags & MAP_COHERENT &&
+ flags & MAP_WRITE &&
+ !(brw->screen->kernel_features & KERNEL_ALLOWS_COHERENT_MMAP_GTT)) {
+ DBG("bo_map_gtt: rejected attempt to make a coherent, persistent and
writable GGTT mmap, %d (%s)\n", bo->gem_handle, bo->name);
+ return NULL;
+ }
+
/* Get a mapping of the buffer if we haven't before. */
if (bo->map_gtt == NULL) {
DBG("bo_map_gtt: mmap %d (%s)\n", bo->gem_handle, bo->name);
@@ -1138,6 +1170,10 @@ brw_bo_map_gtt(struct brw_context *brw, struct brw_bo
*bo, unsigned flags)
if (!(flags & MAP_ASYNC)) {
bo_wait_with_stall_warning(brw, bo, "GTT mapping");
}
+ if (flags & MAP_WRITE &&
+ !(brw->screen->kernel_features & KERNEL_ALLOWS_COHERENT_MMAP_GTT)) {
+ bo_set_domain(bo, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+ }
return bo->map_gtt;
}
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
Re: [Mesa-dev] [PATCH] gallivm/radeonsi: allow to pass two swizzles into fetches.
On 2018-08-27 11:16 p.m., Dave Airlie wrote: > From: Dave Airlie > > This hijacks the top 16-bits of swizzle, to pass in the swizzle > for the second channel. > > This fixes handling .yx swizzles of 64-bit values. > > This should fixup radeonsi and llvmpipe. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107524 This change broke a bunch of piglit tests for me with radeonsi on Bonaire: spec@arb_enhanced_layouts@execution@component-layout@vs-fs-array-dvec3 spec@arb_tessellation_shader@execution@dvec2-vs-tcs-tes spec@arb_tessellation_shader@execution@double-array-vs-tcs-tes spec@arb_tessellation_shader@execution@double-vs-tcs-tes spec@arb_tessellation_shader@execution@dvec3-vs-tcs-tes spec@arb_tessellation_shader@execution@variable-indexing@tes-input-array-dvec4-index-rd spec@arb_tessellation_shader@execution@variable-indexing@vs-output-array-dvec4-index-wr-before-tcs spec@arb_tessellation_shader@execution@variable-indexing@tcs-output-array-dvec4-index-wr spec@arb_tessellation_shader@execution@variable-indexing@tcs-input-array-dvec4-index-rd -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] docs: update calendar to extended the 18.1 cycle by one more release
Due to having 2 additional RCs for 18.2. Cc: Dylan Baker Cc: Juan A. Suarez Cc: Emil Velikov Signed-off-by: Andres Gomez --- docs/release-calendar.html | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/release-calendar.html b/docs/release-calendar.html index d4c9ab86967..2e3bb8a 100644 --- a/docs/release-calendar.html +++ b/docs/release-calendar.html @@ -39,14 +39,20 @@ if you'd like to nominate a patch in the next stable release. Notes -18.1 +18.1 2018-09-07 18.1.8 Dylan Baker + + + +2018-09-21 +18.1.9 +Dylan Baker Last planned 18.1.x release -18.2 +18.2 2018-09-05 18.2.0-rc6 Andres Gomez -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2] Check if the window is non-NULL before setting swap interval.
On 30 August 2018 at 11:41, Eric Engestrom wrote: > On Thursday, 2018-08-30 17:55:14 +0900, Tomasz Figa wrote: >> Hi Erik, Emil, Eric, >> >> On Tue, Jul 10, 2018 at 12:41 AM Erik Faye-Lund wrote: >> > >> > On Thu, Jul 5, 2018 at 7:02 PM Emil Velikov >> > wrote: >> > > >> > > On 5 July 2018 at 17:17, Eric Engestrom wrote: >> > > > On Thursday, 2018-07-05 14:43:02 +0100, Emil Velikov wrote: >> > > >> On 5 July 2018 at 10:53, Eric Engestrom >> > > >> wrote: >> > > >> > On Monday, 2018-07-02 14:12:44 +0530, samiuddi wrote: >> > > >> >> This fixes crash due to NULL window when swap interval is set >> > > >> >> for pbuffer surface. >> > > >> >> >> > > >> >> Test: CtsDisplayTestCases pass >> > > >> >> >> > > >> >> Signed-off-by: samiuddi >> > > >> >> --- >> > > >> >> >> > > >> >> Kindly ignore this patch >> > > >> >> https://lists.freedesktop.org/archives/mesa-dev/2018-July/199098.html >> > > >> >> >> > > >> >> src/egl/drivers/dri2/platform_android.c | 2 +- >> > > >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> > > >> >> >> > > >> >> diff --git a/src/egl/drivers/dri2/platform_android.c >> > > >> >> b/src/egl/drivers/dri2/platform_android.c >> > > >> >> index ca8708a..b5b960a 100644 >> > > >> >> --- a/src/egl/drivers/dri2/platform_android.c >> > > >> >> +++ b/src/egl/drivers/dri2/platform_android.c >> > > >> >> @@ -485,7 +485,7 @@ droid_swap_interval(_EGLDriver *drv, >> > > >> >> _EGLDisplay *dpy, >> > > >> >> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); >> > > >> >> struct ANativeWindow *window = dri2_surf->window; >> > > >> >> >> > > >> >> - if (window->setSwapInterval(window, interval)) >> > > >> >> + if (window && window->setSwapInterval(window, interval)) >> > > >> >>return EGL_FALSE; >> > > >> > >> > > >> > Shouldn't we return false if we couldn't set the swap interval? >> > > >> > >> > > >> > I think this should be: >> > > >> >if (!window || window->setSwapInterval(window, interval)) >> > > >> > return EGL_FALSE; >> > > >> > >> > > >> Changing the patch as above will lead to eglSwapInterval consistently >> > > >> failing for pbuffer surfaces. >> > > > >> > > > I'm not that familiar with pbuffers, but does SwapInterval really make >> > > > sense for them? I thought you couldn't swap a pbuffer. >> > > > >> > > > If so, failing an invalid op seems like the right thing to do. >> > > > Otherwise, I guess sure, no-op returning success is fine. >> > > > >> > > Looking at eglSwapInterval manpage [1] (with my annotation): >> > > "eglSwapInterval — specifies the minimum number of video frame periods >> > > per buffer swap for the _window_ associated with the current context." >> > > While the spec does not mention window/pbuffer/pixmap at all - it >> > > extensively refers to eglSwapBuffers. >> > > >> > > Wrt eglSwapBuffers manpage [2] and spec are consistent: >> > > >> > > "If surface is a pixel buffer or a pixmap, eglSwapBuffers has no >> > > effect, and no error is generated." >> > > >> > > Perhaps it's wrong to set eglSwapInterval for !window surfaces, but >> > > its sibling (eglSwapBuffers) does not error out. >> > > Hence I doubt many users make a distinction between window and pbuffer >> > > surfaces for eglSwap*. >> > > >> > > Worth bringing to the WG meeting - it' planned for 1st August. >> > > >> > >> > As I pointed out when I proposed this variant here: >> > https://patchwork.freedesktop.org/patch/219313/ >> > >> > "Also, I don't think EGL_FALSE is the right return-value, as it doesn't >> > seem the EGL 1.5 spec defines any such error. Also, for instance >> > dri2_swap_interval returns EGL_TRUE when there's no driver-function, >> > which further backs the "silent failure" in this case IMO." >> > >> > So I think EGL_TRUE is the correct return-value for now. If the spec >> > gets changed, we can of course update our implementation. >> >> What happens to this patch in the end? It looks like we're observing a >> similar problem in Chrome OS. >> >> Emil, was there any follow-up on the WG meeting? > > Meeting was cancelled, but I raised the issue here: > https://gitlab.khronos.org/egl/API/merge_requests/17 > > Right now we have ARM saying they return false + error and NVIDIA saying > they return true + no error and that ARM has a bug. > I think another party adding their opinion might nudge it forward :) > Fwiw I put forward a suggestion to add a workaround in the Android/CrOS libEGL wrapper for the ARM drivers. Although one could also consider the Nvidia/Mesa drivers as non-compliant and add a workaround for those instead. I have to admit it's not pretty, but it seems consistent with all the other workarounds in the wrapper. As Eric said - input from another party should, would be great. HTH Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 00/10] Let us welcome EGLDevice
Hi Emil,
> Correct. We had a number of bugs (one of which breaking MT apps) that
> makes me cautious.
Working with environment variables basically is often asking for problems. The
thread race condition ones being the most prominent.
To overwrite something - especially for debugging - its ok to use the
environment IMO. But there should always be a non environment variable API for
normal use.
> Agreed - will need to take a closer look why things crash, while the
> piglit test works.
> Might even add a piglit subtest.
That would be great.
Once the development machine is restored form backup, I should have one more
piglit test that I wrote once for that basic set of extensions. Mostly
overlapping with what you committed lately, but may be still useful.
> >> > Then if I use the patch series on an account that has no DISPLAY set
> >> > and
> >> > no
> >> > 'display server' running, eglInitialize fails in device_probe_device
> >> > due
> >> > to at first opening the '.../card0' device and bailing out when this
> >> > does
> >> > not work. Means the current patch series goes via opening the primary
> >> > node which shall not be accessible by everyone and from that derives
> >> > the
> >> > rendernode device which is finally used for _EGLDisplay initialization.
> >> > Can we alternatively go directly to the rendernode in some way?
> >>
> >> I think we could. Can you elaborate a bit more or provide an example
> >> on the topic though.
> >> I'm quite curious what's happening here - is there no card0 node,
> >> open() fails, other?
> >
> > It's just the file permissions:
> >
> > $ ls -l /dev/dri/{card*,render*}
> > crw-rw+ 1 root video 226, 0 Aug 30 08:28 /dev/dri/card0
> > crw-rw-rw- 1 root render 226, 128 Aug 30 08:28 /dev/dri/renderD128
> >
> > which is pretty much what I expect from the basic idea of render nodes.
> > As long as you are the one logged into the console you can access card0
> > via an dynamically added acl. But if you are not logged into the console,
> > which is at least one of the major driving use cases of the device
> > enumeration extension, the current implementation fails with opening
> > card0.
>
> IIRC when someone gets logged in systemd/logind/others sets up the ACL
> - both ownership and permissions.
> So an open(/dev/dri/card0) will be fine - be that from tty or the DE
> terminal emulator.
>
> Can you share a brief how-to reproduce?
ssh different-machine.somewhere
Then you will see that you are not added to the card0 acl as you are not
logged to any console.
Basically this set of extensions should give anyone who has access to the cpu
to compute something from his data also access to the gpu to compute/render
some pictures from his data.
The practical use case is really: You sit in Central Europe with your CAD
model and you prepared that for some simulation. That simulation will later
run on a compute cluster located anywhere but central europe. May be think of
a country where cooling is easier and electricity is cheaper. You copy your
prepared model to one cluster file system there and start to produce terabytes
of data on that cluser filesystem. Either your postprocessing runs then later
on on some machine with a GPU or your simulation program - mostly using MPI -
transfers data for postprocessing to some GPU nodes inside the MPI program on
the cluster and generates some images that you will take a look afterwards.
Usually the images are some megabytes that you can copy back to central
europe.
Also please don't nail that argument down to ssh. You can find very creative
solutions in high performance computing on different machines. Some of them
still use telnet or rsh to spawn their sub processes. Some queuing systems
have custom daemons and protocols to start compute processes onto some compute
node.
Long story short, you just cannot assume that you have access to card*.
> Ouch. Hope you have backups for your important bits.
Yes, I have. I will loose hand full of mails from until the previous evening,
but appart from that, nothing else. Thanks for asking!
best
Mathias
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH] intel: decoder: unify MI_BB_START field naming
On 28/08/2018 17:01, Dylan Baker wrote:
Quoting Lionel Landwerlin (2018-08-28 02:39:58)
Yes, I think so. You asked on another commit too, both are related and
this depends on other commits from Jason.
Here is a list in order of cherry picking :
commit f430a37fa75f534c3a114b0ec546fa14f05f5da1
Author: Lionel Landwerlin
Date: Tue Aug 14 11:22:12 2018 +0100
intel: decoder: unify MI_BB_START field naming
commit 2abd7ae189135eb5a1f530a3a1c9412d3a7e238d
Author: Jason Ekstrand
Date: Fri Aug 24 15:23:04 2018 -0500
intel/decoder: Clean up field iteration and fix sub-dword fields
commit cbd4bc1346f7397242e157bb66099b950a8c5643
Author: Jason Ekstrand
Date: Fri Aug 24 16:04:03 2018 -0500
intel/batch_decoder: Fix dynamic state printing
commit 70de31d0c106f58d6b7e6d5b79b8d90c1c112a3b
Author: Jason Ekstrand
Date: Fri Aug 24 16:05:08 2018 -0500
intel/batch_decoder: Print blend states properly
commit 440a988bd1478bb33dafcbb8575473bc643ae383
Author: Lionel Landwerlin
Date: Sat Aug 25 18:22:00 2018 +0100
intel: decoder: handle 0 sized structs
Thanks,
-
Lionel
On 27/08/2018 22:20, Andres Gomez wrote:
Lionel, should we also include this in the stable queues ?
On Tue, 2018-08-14 at 11:26 +0100, Lionel Landwerlin wrote:
The batch decoder looks for a field with a particular name to decide
whether an MI_BB_START leads into a second batch buffer level. Because
the names are different between Gen7.5/8 and the newer generation we
fail that test and keep on reading (invalid) instructions.
Signed-off-by: Lionel Landwerlin
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
---
src/intel/genxml/gen75.xml | 6 +++---
src/intel/genxml/gen8.xml | 6 +++---
src/intel/vulkan/anv_batch_chain.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/intel/genxml/gen75.xml b/src/intel/genxml/gen75.xml
index 5b01fd45400..dfc3d891498 100644
--- a/src/intel/genxml/gen75.xml
+++ b/src/intel/genxml/gen75.xml
@@ -2314,9 +2314,9 @@
-
-
-
+
+
+
diff --git a/src/intel/genxml/gen8.xml b/src/intel/genxml/gen8.xml
index 4ed41d15612..330366b7ed0 100644
--- a/src/intel/genxml/gen8.xml
+++ b/src/intel/genxml/gen8.xml
@@ -2553,9 +2553,9 @@
-
-
-
+
+
+
diff --git a/src/intel/vulkan/anv_batch_chain.c
b/src/intel/vulkan/anv_batch_chain.c
index c47a81c8a4d..0f7c8325ea4 100644
--- a/src/intel/vulkan/anv_batch_chain.c
+++ b/src/intel/vulkan/anv_batch_chain.c
@@ -531,7 +531,7 @@ emit_batch_buffer_start(struct anv_cmd_buffer *cmd_buffer,
anv_batch_emit(_buffer->batch, GEN8_MI_BATCH_BUFFER_START, bbs) {
bbs.DWordLength = cmd_buffer->device->info.gen < 8 ?
gen7_length : gen8_length;
- bbs._2ndLevelBatchBuffer = _1stlevelbatch;
+ bbs.SecondLevelBatchBuffer= Firstlevelbatch;
bbs.AddressSpaceIndicator = ASI_PPGTT;
bbs.BatchBufferStartAddress = (struct anv_address) { bo, offset };
}
Hi Lionel,
Only patches 1 and 3 of that list apply cleanly to the 18.1 branch, it looks
like maybe I need a few more patches for things to apply cleanly?
Dylan
Yeah... Looks like this might pull in the world...
Thinking about it again, how much do we need INTEL_DEBUG=batch fixed in
stable?
-
Lionel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/dri: use the FREE wrapper for the dri screen
On 08/30/2018 07:32 AM, Eric Engestrom wrote: On Thursday, 2018-08-30 11:12:11 +0100, Emil Velikov wrote: From: Emil Velikov The memory is allocated with the uppercase wrapper. Tear down should match that. You're right, in dri2_init_screen() / dri_kms_init_screen(), but looking at the history this used to be FREE() and Brian replaced a bunch of them with free() in fe72a069d1fcce943f315 "mesa: s/FREE/free/". Cc'ing him to check if going back is the right thing, or if maybe instead it's the CALLOC() in dri2.c that should be replaced with calloc()? Unfortunately, there's two different definitions of CALLOC_STRUCT: src/gallium/auxiliary/util/u_memory.h:58:#define CALLOC_STRUCT(T) (struct T *) CALLOC(1, sizeof(struct T)) src/mesa/main/imports.h:58:#define CALLOC_STRUCT(T) (struct T *) calloc(1, sizeof(struct T)) (by amazing coincidence they're on the same line number!) The former calls CALLOC() which wraps os_calloc() while the later calls calloc(). You just have to be sure to use free/FREE which matches the underlying CALLOC(). After only a quick look it's not obvious to me which is correct here but I'm sure you can figure it out. -Brian Cc: mesa-sta...@lists.freedesktop.org Signed-off-by: Emil Velikov --- src/gallium/state_trackers/dri/dri_screen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/state_trackers/dri/dri_screen.c b/src/gallium/state_trackers/dri/dri_screen.c index 3e4de59a433..98b02678c81 100644 --- a/src/gallium/state_trackers/dri/dri_screen.c +++ b/src/gallium/state_trackers/dri/dri_screen.c @@ -484,7 +484,7 @@ dri_destroy_screen(__DRIscreen * sPriv) pipe_loader_release(>dev, 1); - free(screen); + FREE(screen); sPriv->driverPrivate = NULL; sPriv->extensions = NULL; } -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-devdata=02%7C01%7Cbrianp%40vmware.com%7Cb3cdaba097544ad60e3d08d60e7d0603%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636712327476193623sdata=PvdH0sHKaHCn8XfN8ToksT3dXgs6KUknYAUcHTnIQzo%3Dreserved=0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] anv: blorp: support multiple aspect blits
Newer blit tests are enabling depth blits. We currently don't
support it but can do by iterating over the aspects masks (copy some
logic from the CopyImage function).
Signed-off-by: Lionel Landwerlin
Fixes: 9f44745eca0e41 ("anv: Use blorp to implement VkBlitImage")
Reviewed-by: Jason Ekstrand
(cherry picked from commit 5a1c23d1502d275c4d554c586bf029e66131f4ac)
---
src/intel/vulkan/anv_blorp.c | 146 ++-
1 file changed, 75 insertions(+), 71 deletions(-)
diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
index 68e2ed65c29..37f68790889 100644
--- a/src/intel/vulkan/anv_blorp.c
+++ b/src/intel/vulkan/anv_blorp.c
@@ -533,82 +533,86 @@ void anv_CmdBlitImage(
const VkImageSubresourceLayers *src_res = [r].srcSubresource;
const VkImageSubresourceLayers *dst_res = [r].dstSubresource;
- get_blorp_surf_for_anv_image(cmd_buffer->device,
- src_image, src_res->aspectMask,
- srcImageLayout, ISL_AUX_USAGE_NONE, );
- get_blorp_surf_for_anv_image(cmd_buffer->device,
- dst_image, dst_res->aspectMask,
- dstImageLayout, ISL_AUX_USAGE_NONE, );
-
- struct anv_format_plane src_format =
- anv_get_format_plane(_buffer->device->info, src_image->vk_format,
- src_res->aspectMask, src_image->tiling);
- struct anv_format_plane dst_format =
- anv_get_format_plane(_buffer->device->info, dst_image->vk_format,
- dst_res->aspectMask, dst_image->tiling);
-
- unsigned dst_start, dst_end;
- if (dst_image->type == VK_IMAGE_TYPE_3D) {
- assert(dst_res->baseArrayLayer == 0);
- dst_start = pRegions[r].dstOffsets[0].z;
- dst_end = pRegions[r].dstOffsets[1].z;
- } else {
- dst_start = dst_res->baseArrayLayer;
- dst_end = dst_start + anv_get_layerCount(dst_image, dst_res);
- }
-
- unsigned src_start, src_end;
- if (src_image->type == VK_IMAGE_TYPE_3D) {
- assert(src_res->baseArrayLayer == 0);
- src_start = pRegions[r].srcOffsets[0].z;
- src_end = pRegions[r].srcOffsets[1].z;
- } else {
- src_start = src_res->baseArrayLayer;
- src_end = src_start + anv_get_layerCount(src_image, src_res);
- }
-
- bool flip_z = flip_coords(_start, _end, _start, _end);
- float src_z_step = (float)(src_end + 1 - src_start) /
- (float)(dst_end + 1 - dst_start);
+ assert(anv_image_aspects_compatible(src_res->aspectMask,
+ dst_res->aspectMask));
+
+ uint32_t aspect_bit;
+ anv_foreach_image_aspect_bit(aspect_bit, src_image, src_res->aspectMask)
{
+ get_blorp_surf_for_anv_image(cmd_buffer->device,
+ src_image, 1U << aspect_bit,
+ srcImageLayout, ISL_AUX_USAGE_NONE,
);
+ get_blorp_surf_for_anv_image(cmd_buffer->device,
+ dst_image, 1U << aspect_bit,
+ dstImageLayout, ISL_AUX_USAGE_NONE,
);
+
+ struct anv_format_plane src_format =
+anv_get_format_plane(_buffer->device->info,
src_image->vk_format,
+ 1U << aspect_bit, src_image->tiling);
+ struct anv_format_plane dst_format =
+anv_get_format_plane(_buffer->device->info,
dst_image->vk_format,
+ 1U << aspect_bit, dst_image->tiling);
+
+ unsigned dst_start, dst_end;
+ if (dst_image->type == VK_IMAGE_TYPE_3D) {
+assert(dst_res->baseArrayLayer == 0);
+dst_start = pRegions[r].dstOffsets[0].z;
+dst_end = pRegions[r].dstOffsets[1].z;
+ } else {
+dst_start = dst_res->baseArrayLayer;
+dst_end = dst_start + anv_get_layerCount(dst_image, dst_res);
+ }
- if (flip_z) {
- src_start = src_end;
- src_z_step *= -1;
- }
+ unsigned src_start, src_end;
+ if (src_image->type == VK_IMAGE_TYPE_3D) {
+assert(src_res->baseArrayLayer == 0);
+src_start = pRegions[r].srcOffsets[0].z;
+src_end = pRegions[r].srcOffsets[1].z;
+ } else {
+src_start = src_res->baseArrayLayer;
+src_end = src_start + anv_get_layerCount(src_image, src_res);
+ }
- unsigned src_x0 = pRegions[r].srcOffsets[0].x;
- unsigned src_x1 = pRegions[r].srcOffsets[1].x;
- unsigned dst_x0 = pRegions[r].dstOffsets[0].x;
- unsigned dst_x1 = pRegions[r].dstOffsets[1].x;
- bool flip_x = flip_coords(_x0, _x1, _x0, _x1);
+ bool flip_z = flip_coords(_start, _end, _start, _end);
+ float src_z_step = (float)(src_end + 1 - src_start) /
+(float)(dst_end + 1 - dst_start);
-
[Mesa-dev] [PATCH 6/7] virgl: update minor differences to upstream header
virgl_protocol.h is considered to have it's upstream in the
virglrenderer repository, and somehow these minor differences has
crept in.
Let's sync with the upstream to avoid this.
Signed-off-by: Erik Faye-Lund
---
src/gallium/drivers/virgl/virgl_protocol.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gallium/drivers/virgl/virgl_protocol.h
b/src/gallium/drivers/virgl/virgl_protocol.h
index 0a41c0174f..dd1a4ee54d 100644
--- a/src/gallium/drivers/virgl/virgl_protocol.h
+++ b/src/gallium/drivers/virgl/virgl_protocol.h
@@ -83,7 +83,6 @@ enum virgl_context_cmd {
VIRGL_CCMD_CREATE_SUB_CTX,
VIRGL_CCMD_DESTROY_SUB_CTX,
VIRGL_CCMD_BIND_SHADER,
-
VIRGL_CCMD_SET_TESS_STATE,
VIRGL_CCMD_SET_MIN_SAMPLES,
VIRGL_CCMD_SET_SHADER_BUFFERS,
@@ -521,6 +520,7 @@ enum virgl_context_cmd {
#define VIRGL_MEMORY_BARRIER_SIZE 1
#define VIRGL_MEMORY_BARRIER_FLAGS 1
+/* launch grid */
#define VIRGL_LAUNCH_GRID_SIZE 8
#define VIRGL_LAUNCH_BLOCK_X 1
#define VIRGL_LAUNCH_BLOCK_Y 2
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 7/7] virgl: use hw-atomics instead of in-ssbo ones
From: Tomeu Vizoso
Emulating atomics on top of ssbos can lead to too small max SSBO count,
so let's use the hw-atomics mechanism to expose atomic buffers instead.
Signed-off-by: Erik Faye-Lund
---
src/gallium/drivers/virgl/virgl_context.c | 37 ++
src/gallium/drivers/virgl/virgl_context.h | 2 ++
src/gallium/drivers/virgl/virgl_encode.c | 23 ++
src/gallium/drivers/virgl/virgl_encode.h | 3 ++
src/gallium/drivers/virgl/virgl_hw.h | 5 +++
src/gallium/drivers/virgl/virgl_protocol.h | 9 ++
src/gallium/drivers/virgl/virgl_screen.c | 14 +---
7 files changed, 88 insertions(+), 5 deletions(-)
diff --git a/src/gallium/drivers/virgl/virgl_context.c
b/src/gallium/drivers/virgl/virgl_context.c
index edc03f5dcf..30cd0e4331 100644
--- a/src/gallium/drivers/virgl/virgl_context.c
+++ b/src/gallium/drivers/virgl/virgl_context.c
@@ -196,6 +196,19 @@ static void virgl_attach_res_shader_images(struct
virgl_context *vctx,
}
}
+static void virgl_attach_res_atomic_buffers(struct virgl_context *vctx)
+{
+ struct virgl_winsys *vws = virgl_screen(vctx->base.screen)->vws;
+ struct virgl_resource *res;
+ unsigned i;
+ for (i = 0; i < PIPE_MAX_SHADER_BUFFERS; i++) {
+ res = virgl_resource(vctx->atomic_buffers[i]);
+ if (res) {
+ vws->emit_res(vws, vctx->cbuf, res->hw_res, FALSE);
+ }
+ }
+}
+
/*
* after flushing, the hw context still has a bunch of
* resources bound, so we need to rebind those here.
@@ -214,6 +227,7 @@ static void virgl_reemit_res(struct virgl_context *vctx)
virgl_attach_res_shader_buffers(vctx, shader_type);
virgl_attach_res_shader_images(vctx, shader_type);
}
+ virgl_attach_res_atomic_buffers(vctx);
virgl_attach_res_vertex_buffers(vctx);
virgl_attach_res_so_targets(vctx);
}
@@ -952,6 +966,28 @@ static void virgl_blit(struct pipe_context *ctx,
blit);
}
+static void virgl_set_hw_atomic_buffers(struct pipe_context *ctx,
+ unsigned start_slot,
+ unsigned count,
+ const struct pipe_shader_buffer
*buffers)
+{
+ struct virgl_context *vctx = virgl_context(ctx);
+
+ for (unsigned i = 0; i < count; i++) {
+ unsigned idx = start_slot + i;
+
+ if (buffers) {
+ if (buffers[i].buffer) {
+pipe_resource_reference(>atomic_buffers[idx],
+buffers[i].buffer);
+continue;
+ }
+ }
+ pipe_resource_reference(>atomic_buffers[idx], NULL);
+ }
+ virgl_encode_set_hw_atomic_buffers(vctx, start_slot, count, buffers);
+}
+
static void virgl_set_shader_buffers(struct pipe_context *ctx,
enum pipe_shader_type shader,
unsigned start_slot, unsigned count,
@@ -1209,6 +1245,7 @@ struct pipe_context *virgl_context_create(struct
pipe_screen *pscreen,
vctx->base.blit = virgl_blit;
vctx->base.set_shader_buffers = virgl_set_shader_buffers;
+ vctx->base.set_hw_atomic_buffers = virgl_set_hw_atomic_buffers;
vctx->base.set_shader_images = virgl_set_shader_images;
vctx->base.memory_barrier = virgl_memory_barrier;
diff --git a/src/gallium/drivers/virgl/virgl_context.h
b/src/gallium/drivers/virgl/virgl_context.h
index 38d1f450e1..20988baa3c 100644
--- a/src/gallium/drivers/virgl/virgl_context.h
+++ b/src/gallium/drivers/virgl/virgl_context.h
@@ -75,6 +75,8 @@ struct virgl_context {
int num_draws;
struct list_head to_flush_bufs;
+ struct pipe_resource *atomic_buffers[PIPE_MAX_HW_ATOMIC_BUFFERS];
+
struct primconvert_context *primconvert;
uint32_t hw_sub_ctx_id;
};
diff --git a/src/gallium/drivers/virgl/virgl_encode.c
b/src/gallium/drivers/virgl/virgl_encode.c
index bcb14d8939..c9cc099061 100644
--- a/src/gallium/drivers/virgl/virgl_encode.c
+++ b/src/gallium/drivers/virgl/virgl_encode.c
@@ -958,6 +958,29 @@ int virgl_encode_set_shader_buffers(struct virgl_context
*ctx,
return 0;
}
+int virgl_encode_set_hw_atomic_buffers(struct virgl_context *ctx,
+ unsigned start_slot, unsigned count,
+ const struct pipe_shader_buffer
*buffers)
+{
+ int i;
+ virgl_encoder_write_cmd_dword(ctx,
VIRGL_CMD0(VIRGL_CCMD_SET_ATOMIC_BUFFERS, 0,
VIRGL_SET_ATOMIC_BUFFER_SIZE(count)));
+
+ virgl_encoder_write_dword(ctx->cbuf, start_slot);
+ for (i = 0; i < count; i++) {
+ if (buffers) {
+struct virgl_resource *res = virgl_resource(buffers[i].buffer);
+virgl_encoder_write_dword(ctx->cbuf, buffers[i].buffer_offset);
+virgl_encoder_write_dword(ctx->cbuf, buffers[i].buffer_size);
+virgl_encoder_write_res(ctx, res);
+ } else {
+virgl_encoder_write_dword(ctx->cbuf, 0);
+virgl_encoder_write_dword(ctx->cbuf, 0);
+
[Mesa-dev] [PATCH 5/7] gallium: add PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER{S, _BUFFERS}
This moves the evergreen-specific max-sizes out as a driver-cap, so
other drivers with less strict requirements also can use hw-atomics.
Remove ssbo_atomic as it's no longer needed.
We should now be able to use hw-atomics for some stages and not for
other, if needed.
Signed-off-by: Erik Faye-Lund
---
src/gallium/drivers/etnaviv/etnaviv_screen.c | 5 +
.../drivers/freedreno/freedreno_screen.c | 5 +
.../drivers/nouveau/nv30/nv30_screen.c| 2 ++
.../drivers/nouveau/nv50/nv50_screen.c| 2 ++
.../drivers/nouveau/nvc0/nvc0_screen.c| 2 ++
src/gallium/drivers/r300/r300_screen.c| 2 ++
src/gallium/drivers/r600/r600_pipe.c | 9 +
src/gallium/drivers/radeonsi/si_get.c | 2 ++
src/gallium/drivers/svga/svga_screen.c| 2 ++
src/gallium/drivers/v3d/v3d_screen.c | 2 ++
src/gallium/drivers/vc4/vc4_screen.c | 2 ++
src/gallium/drivers/virgl/virgl_screen.c | 2 ++
src/gallium/include/pipe/p_defines.h | 2 ++
src/mesa/state_tracker/st_extensions.c| 20 +--
14 files changed, 49 insertions(+), 10 deletions(-)
diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c
b/src/gallium/drivers/etnaviv/etnaviv_screen.c
index 108b97d35c..95166a2db1 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
@@ -372,6 +372,11 @@ etna_screen_get_param(struct pipe_screen *pscreen, enum
pipe_cap param)
return 0;
case PIPE_CAP_UMA:
return 1;
+
+ /* hw atomic counters */
+ case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTERS:
+ case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS:
+ return 0;
}
debug_printf("unknown param %d", param);
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c
b/src/gallium/drivers/freedreno/freedreno_screen.c
index af44ab698e..e6dfc2e48e 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -491,6 +491,11 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum
pipe_cap param)
return 1;
case PIPE_CAP_NATIVE_FENCE_FD:
return fd_device_version(screen->dev) >= FD_VERSION_FENCE_FD;
+
+ /* hw atomic counters */
+ case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTERS:
+ case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS:
+ return 0;
}
debug_printf("unknown param %d\n", param);
return 0;
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
index d52d8f3988..3fdbadb6c6 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
@@ -239,6 +239,8 @@ nv30_screen_get_param(struct pipe_screen *pscreen, enum
pipe_cap param)
case PIPE_CAP_CONSERVATIVE_RASTER_POST_DEPTH_COVERAGE:
case PIPE_CAP_MAX_CONSERVATIVE_RASTER_SUBPIXEL_PRECISION_BIAS:
case PIPE_CAP_PROGRAMMABLE_SAMPLE_LOCATIONS:
+ case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTERS:
+ case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS:
return 0;
case PIPE_CAP_MAX_GS_INVOCATIONS:
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
index cd36795e56..a6dda5dfa0 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
@@ -293,6 +293,8 @@ nv50_screen_get_param(struct pipe_screen *pscreen, enum
pipe_cap param)
case PIPE_CAP_CONSERVATIVE_RASTER_POST_DEPTH_COVERAGE:
case PIPE_CAP_MAX_CONSERVATIVE_RASTER_SUBPIXEL_PRECISION_BIAS:
case PIPE_CAP_PROGRAMMABLE_SAMPLE_LOCATIONS:
+ case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTERS:
+ case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS:
return 0;
case PIPE_CAP_MAX_GS_INVOCATIONS:
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index 446e30dcc8..b628b24d76 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -322,6 +322,8 @@ nvc0_screen_get_param(struct pipe_screen *pscreen, enum
pipe_cap param)
case PIPE_CAP_CONSTBUF0_FLAGS:
case PIPE_CAP_PACKED_UNIFORMS:
case PIPE_CAP_CONSERVATIVE_RASTER_PRE_SNAP_POINTS_LINES:
+ case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTERS:
+ case PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER_BUFFERS:
return 0;
case PIPE_CAP_VENDOR_ID:
diff --git a/src/gallium/drivers/r300/r300_screen.c
b/src/gallium/drivers/r300/r300_screen.c
index d27c2b8f1d..f29d5244ff 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -261,6 +261,8 @@ static int r300_get_param(struct pipe_screen* pscreen, enum
pipe_cap param)
case PIPE_CAP_CONSERVATIVE_RASTER_POST_DEPTH_COVERAGE:
case PIPE_CAP_MAX_CONSERVATIVE_RASTER_SUBPIXEL_PRECISION_BIAS:
case
[Mesa-dev] [PATCH 1/7] st/mesa: use real bool for can_ubo
We're doing full c99 now, so there's no point in using the old boolean
type.
Signed-off-by: Erik Faye-Lund
---
This is not technically nessecary for the series, but IMO a nice cleanup
nevertheless.
src/mesa/state_tracker/st_extensions.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/mesa/state_tracker/st_extensions.c
b/src/mesa/state_tracker/st_extensions.c
index 29a3251308..8cb80f9932 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -81,7 +81,7 @@ void st_init_limits(struct pipe_screen *screen,
{
int supported_irs;
unsigned sh;
- boolean can_ubo = TRUE;
+ bool can_ubo = true;
int temp;
bool ssbo_atomic = true;
@@ -160,7 +160,7 @@ void st_init_limits(struct pipe_screen *screen,
c->MaxUniformBlockSize = MIN2(c->MaxUniformBlockSize, INT_MAX - 127);
if (c->MaxUniformBlockSize < 16384) {
- can_ubo = FALSE;
+ can_ubo = false;
}
for (sh = 0; sh < PIPE_SHADER_TYPES; ++sh) {
@@ -301,7 +301,7 @@ void st_init_limits(struct pipe_screen *screen,
if (pc->MaxNativeInstructions &&
(options->EmitNoIndirectUniform || pc->MaxUniformBlocks < 12)) {
- can_ubo = FALSE;
+ can_ubo = false;
}
if (options->EmitNoLoops)
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/7] gallium: add PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS
This gets rid of a r600 specific hack in the state-tracker, and prepares
for other drivers to be able to use hw-atomics.
While we're at it, clean up some indentation in the various drivers.
Signed-off-by: Erik Faye-Lund
---
src/gallium/drivers/etnaviv/etnaviv_screen.c | 3 +++
src/gallium/drivers/freedreno/freedreno_screen.c | 3 +++
src/gallium/drivers/nouveau/nv30/nv30_screen.c | 2 ++
src/gallium/drivers/nouveau/nv50/nv50_screen.c | 2 ++
src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 2 ++
src/gallium/drivers/r300/r300_screen.c | 6 +-
src/gallium/drivers/r600/r600_pipe.c | 4
src/gallium/drivers/radeonsi/si_get.c| 2 ++
src/gallium/drivers/svga/svga_screen.c | 2 ++
src/gallium/drivers/v3d/v3d_screen.c | 8 ++--
src/gallium/drivers/vc4/vc4_screen.c | 8 ++--
src/gallium/drivers/virgl/virgl_screen.c | 2 ++
src/gallium/include/pipe/p_defines.h | 1 +
src/mesa/state_tracker/st_extensions.c | 9 +
14 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c
b/src/gallium/drivers/etnaviv/etnaviv_screen.c
index 9669bd2f60..108b97d35c 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_screen.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c
@@ -289,8 +289,11 @@ etna_screen_get_param(struct pipe_screen *pscreen, enum
pipe_cap param)
case PIPE_CAP_MAX_GS_INVOCATIONS:
return 32;
+ /* shader buffer objects */
case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
return 1 << 27;
+ case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS:
+ return 0;
/* Stream output. */
case PIPE_CAP_MAX_STREAM_OUTPUT_BUFFERS:
diff --git a/src/gallium/drivers/freedreno/freedreno_screen.c
b/src/gallium/drivers/freedreno/freedreno_screen.c
index 489986703c..af44ab698e 100644
--- a/src/gallium/drivers/freedreno/freedreno_screen.c
+++ b/src/gallium/drivers/freedreno/freedreno_screen.c
@@ -376,8 +376,11 @@ fd_screen_get_param(struct pipe_screen *pscreen, enum
pipe_cap param)
case PIPE_CAP_MAX_GS_INVOCATIONS:
return 32;
+ /* shader buffer objects */
case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
return 1 << 27;
+ case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS:
+ return 0;
case PIPE_CAP_CONTEXT_PRIORITY_MASK:
return screen->priority_mask;
diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
index da7ebecd5d..d52d8f3988 100644
--- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
+++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
@@ -245,6 +245,8 @@ nv30_screen_get_param(struct pipe_screen *pscreen, enum
pipe_cap param)
return 32;
case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
return 1 << 27;
+ case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS:
+ return 0;
case PIPE_CAP_VENDOR_ID:
return 0x10de;
case PIPE_CAP_DEVICE_ID: {
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
index 0007a2b957..cd36795e56 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
@@ -299,6 +299,8 @@ nv50_screen_get_param(struct pipe_screen *pscreen, enum
pipe_cap param)
return 32;
case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
return 1 << 27;
+ case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS:
+ return 0;
case PIPE_CAP_VENDOR_ID:
return 0x10de;
case PIPE_CAP_DEVICE_ID: {
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index 4701b768be..446e30dcc8 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -272,6 +272,8 @@ nvc0_screen_get_param(struct pipe_screen *pscreen, enum
pipe_cap param)
return 32;
case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
return 1 << 27;
+ case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS:
+ return 0;
case PIPE_CAP_CONSERVATIVE_RASTER_PRE_SNAP_TRIANGLES:
return class_3d >= GP100_3D_CLASS;
case PIPE_CAP_SEAMLESS_CUBE_MAP_PER_TEXTURE:
diff --git a/src/gallium/drivers/r300/r300_screen.c
b/src/gallium/drivers/r300/r300_screen.c
index 01a95d98dc..d27c2b8f1d 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -265,8 +265,12 @@ static int r300_get_param(struct pipe_screen* pscreen,
enum pipe_cap param)
case PIPE_CAP_MAX_GS_INVOCATIONS:
return 32;
- case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
+
+/* shader buffer objects */
+case PIPE_CAP_MAX_SHADER_BUFFER_SIZE:
return 1 << 27;
+case PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS:
+ return 0;
/* SWTCL-only features. */
case PIPE_CAP_PRIMITIVE_RESTART:
diff --git a/src/gallium/drivers/r600/r600_pipe.c
[Mesa-dev] [PATCH 2/7] st/mesa: clean up atomic vs ssbo code
This makes the code a bit easier to follow; we first set up
MaxShaderStorageBlocks, then we either set up a dedicated
MaxAtomicBuffers, or we split MaxShaderStorageBlocks in two.
While we're at it, also make the SSBO-splitting code tolerate the
hypothetical case of having an odd number of SSBOs without incorrectly
dropping the last SSBO.
This has the nice result that the SSBOs and atomic buffers are dealt
with almost completely orthogonally, easing some upcoming patches.
Signed-off-by: Erik Faye-Lund
---
src/mesa/state_tracker/st_extensions.c | 27 --
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/src/mesa/state_tracker/st_extensions.c
b/src/mesa/state_tracker/st_extensions.c
index 8cb80f9932..83fc09f52b 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -241,6 +241,10 @@ void st_init_limits(struct pipe_screen *screen,
pc->MaxUniformComponents +
(uint64_t)c->MaxUniformBlockSize / 4 * pc->MaxUniformBlocks;
+ pc->MaxShaderStorageBlocks =
+ screen->get_shader_param(screen, sh,
+ PIPE_SHADER_CAP_MAX_SHADER_BUFFERS);
+
temp = screen->get_shader_param(screen, sh,
PIPE_SHADER_CAP_MAX_HW_ATOMIC_COUNTERS);
if (temp) {
/*
@@ -250,11 +254,14 @@ void st_init_limits(struct pipe_screen *screen,
ssbo_atomic = false;
pc->MaxAtomicCounters = temp;
pc->MaxAtomicBuffers = screen->get_shader_param(screen, sh,
PIPE_SHADER_CAP_MAX_HW_ATOMIC_COUNTER_BUFFERS);
- pc->MaxShaderStorageBlocks = screen->get_shader_param(screen, sh,
PIPE_SHADER_CAP_MAX_SHADER_BUFFERS);
} else {
pc->MaxAtomicCounters = MAX_ATOMIC_COUNTERS;
- pc->MaxAtomicBuffers = screen->get_shader_param(screen, sh,
PIPE_SHADER_CAP_MAX_SHADER_BUFFERS) / 2;
- pc->MaxShaderStorageBlocks = pc->MaxAtomicBuffers;
+ /*
+ * without separate atomic counters, reserve half of the available
+ * SSBOs for atomic buffers, and the other half for normal SSBOs.
+ */
+ pc->MaxAtomicBuffers = pc->MaxShaderStorageBlocks / 2;
+ pc->MaxShaderStorageBlocks -= pc->MaxAtomicBuffers;
}
pc->MaxImageUniforms = screen->get_shader_param(
screen, sh, PIPE_SHADER_CAP_MAX_SHADER_IMAGES);
@@ -465,9 +472,17 @@ void st_init_limits(struct pipe_screen *screen,
screen->get_param(screen, PIPE_CAP_SHADER_BUFFER_OFFSET_ALIGNMENT);
if (c->ShaderStorageBufferOffsetAlignment) {
/* for hw atomic counters leaves these at default for now */
- if (ssbo_atomic)
- c->MaxCombinedShaderStorageBlocks = c->MaxShaderStorageBufferBindings
=
-c->MaxCombinedAtomicBuffers;
+ if (ssbo_atomic) {
+ c->MaxCombinedShaderStorageBlocks =
+c->Program[MESA_SHADER_VERTEX].MaxShaderStorageBlocks +
+c->Program[MESA_SHADER_TESS_CTRL].MaxShaderStorageBlocks +
+c->Program[MESA_SHADER_TESS_EVAL].MaxShaderStorageBlocks +
+c->Program[MESA_SHADER_GEOMETRY].MaxShaderStorageBlocks +
+c->Program[MESA_SHADER_FRAGMENT].MaxShaderStorageBlocks;
+ assert(c->MaxCombinedShaderStorageBlocks <
MAX_COMBINED_SHADER_STORAGE_BUFFERS);
+ }
+ c->MaxShaderStorageBufferBindings = c->MaxCombinedShaderStorageBlocks;
+
c->MaxCombinedShaderOutputResources +=
c->MaxCombinedShaderStorageBlocks;
c->MaxShaderStorageBlockSize =
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/7] st/mesa: simplify MaxAtomicBufferSize-logic
MaxAtomicCounters has already been assigned in the loop above in the
ssbo_atomic = true case, so this will calculate the same value as the
default.
While we're at it, fixup indentation on the MaxAtomicBufferBindings
assign.
Signed-off-by: Erik Faye-Lund
---
src/mesa/state_tracker/st_extensions.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/mesa/state_tracker/st_extensions.c
b/src/mesa/state_tracker/st_extensions.c
index 83fc09f52b..9ecdd26edd 100644
--- a/src/mesa/state_tracker/st_extensions.c
+++ b/src/mesa/state_tracker/st_extensions.c
@@ -441,12 +441,11 @@ void st_init_limits(struct pipe_screen *screen,
c->NumProgramBinaryFormats = 1;
c->MaxAtomicBufferBindings =
- c->Program[MESA_SHADER_FRAGMENT].MaxAtomicBuffers;
+ c->Program[MESA_SHADER_FRAGMENT].MaxAtomicBuffers;
+ c->MaxAtomicBufferSize =
+ c->Program[MESA_SHADER_FRAGMENT].MaxAtomicCounters * ATOMIC_COUNTER_SIZE;
if (!ssbo_atomic) {
- /* for separate atomic buffers - there atomic buffer size will be
- limited */
- c->MaxAtomicBufferSize =
c->Program[MESA_SHADER_FRAGMENT].MaxAtomicCounters * ATOMIC_COUNTER_SIZE;
/* on all HW with separate atomic (evergreen) the following
lines are true. not sure it's worth adding CAPs for this at this
stage. */
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/7] gallium/virgl: use hw-atomic buffers
Here's a patch-series that cleans up some evergreen-specific gallium
assumptions regarding hw-atomic buffers, as well as uses them on virgl
to avoid reporting synthetically few SSBOs.
The goal is really for gallium drivers to accrately report the exact
amount of SSBOs and hw-atomic buffers they suppport, without any artifical
limitations.
Ideally, I'd submit the last two patches separately, but I left them in
here to show the motivation.
This depends on this virglrenderer MR to be effective:
https://gitlab.freedesktop.org/virgl/virglrenderer/merge_requests/12
This is the final step currently needed to pass dEQP GLES-3.1 (as well as
GLES-3.2) using virgl/virglrenderer on i965 hardware.
Erik Faye-Lund (6):
st/mesa: use real bool for can_ubo
st/mesa: clean up atomic vs ssbo code
st/mesa: simplify MaxAtomicBufferSize-logic
gallium: add PIPE_CAP_MAX_COMBINED_SHADER_BUFFERS
gallium: add PIPE_CAP_MAX_COMBINED_HW_ATOMIC_COUNTER{S,_BUFFERS}
virgl: update minor differences to upstream header
Tomeu Vizoso (1):
virgl: use hw-atomics instead of in-ssbo ones
src/gallium/drivers/etnaviv/etnaviv_screen.c | 8 +++
.../drivers/freedreno/freedreno_screen.c | 8 +++
.../drivers/nouveau/nv30/nv30_screen.c| 4 ++
.../drivers/nouveau/nv50/nv50_screen.c| 4 ++
.../drivers/nouveau/nvc0/nvc0_screen.c| 4 ++
src/gallium/drivers/r300/r300_screen.c| 8 ++-
src/gallium/drivers/r600/r600_pipe.c | 13
src/gallium/drivers/radeonsi/si_get.c | 4 ++
src/gallium/drivers/svga/svga_screen.c| 4 ++
src/gallium/drivers/v3d/v3d_screen.c | 10 ++-
src/gallium/drivers/vc4/vc4_screen.c | 10 ++-
src/gallium/drivers/virgl/virgl_context.c | 37 +++
src/gallium/drivers/virgl/virgl_context.h | 2 +
src/gallium/drivers/virgl/virgl_encode.c | 23 +++
src/gallium/drivers/virgl/virgl_encode.h | 3 +
src/gallium/drivers/virgl/virgl_hw.h | 5 ++
src/gallium/drivers/virgl/virgl_protocol.h| 11 +++-
src/gallium/drivers/virgl/virgl_screen.c | 12 +++-
src/gallium/include/pipe/p_defines.h | 3 +
src/mesa/state_tracker/st_extensions.c| 65 ---
20 files changed, 205 insertions(+), 33 deletions(-)
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/dri: use the FREE wrapper for the dri screen
On Thursday, 2018-08-30 11:12:11 +0100, Emil Velikov wrote: > From: Emil Velikov > > The memory is allocated with the uppercase wrapper. Tear down should > match that. You're right, in dri2_init_screen() / dri_kms_init_screen(), but looking at the history this used to be FREE() and Brian replaced a bunch of them with free() in fe72a069d1fcce943f315 "mesa: s/FREE/free/". Cc'ing him to check if going back is the right thing, or if maybe instead it's the CALLOC() in dri2.c that should be replaced with calloc()? > > Cc: mesa-sta...@lists.freedesktop.org > Signed-off-by: Emil Velikov > --- > src/gallium/state_trackers/dri/dri_screen.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/gallium/state_trackers/dri/dri_screen.c > b/src/gallium/state_trackers/dri/dri_screen.c > index 3e4de59a433..98b02678c81 100644 > --- a/src/gallium/state_trackers/dri/dri_screen.c > +++ b/src/gallium/state_trackers/dri/dri_screen.c > @@ -484,7 +484,7 @@ dri_destroy_screen(__DRIscreen * sPriv) > > pipe_loader_release(>dev, 1); > > - free(screen); > + FREE(screen); > sPriv->driverPrivate = NULL; > sPriv->extensions = NULL; > } > -- > 2.18.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 4/5] gitlab-ci: update base + llvm images with scheduled pipelines
On 30 August 2018 at 11:41, Juan A. Suarez Romero wrote: > On Wed, 2018-08-29 at 14:16 +0100, Emil Velikov wrote: >> On 29 August 2018 at 11:12, Juan A. Suarez Romero >> wrote: >> > Use scheduled pipelines to update both the base and the LLVM images. >> > >> > This way allows to have an updated version of the base images even when >> > the respect Rockerfiles keep the same. >> > >> >> Grammar seems off. >> > > After re-reading it, you're right :) > >> Please include an example when a scheduled pipeline is needed/used. >> > > As base images are only re-built when there's a change in the Dockerfile, if > Ubuntu updates the base image or some of the packages we won't get them until > we > change the Dockerfile, which is something that doesn't happen very frequently. > > > Thus, we use scheduled pipelines: every week or so, we force the full re-build > of the base images, that are re-built even if the Dockerfile dind't change. > I see - nicely done. Thank you. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 1/5] gitlab-ci: build Mesa using GitLab CI
Hi Juan, Something I should have mentioned ealier: Pardon if my question are a bit rough - my rocker/docker experience is limited. On 30 August 2018 at 11:22, Juan A. Suarez Romero wrote: > On Wed, 2018-08-29 at 14:11 +0100, Emil Velikov wrote: >> Hi Juan, >> >> I've shared a number of suggestions. I'll leave that to you if they >> will be in v3 or patches on top. >> >> On 29 August 2018 at 11:12, Juan A. Suarez Romero >> wrote: >> >> > In order to build the images, Rocker is used. This is a tool that >> > extends the Dockerfiles with new features that are quite interested >> > here. The main features we use is the support for templating, and the >> > support for mounting external directories during the image building. >> > This help to use tools like ccache to improve the build speed. >> > >> >> I think that gitlab-ci supports templating - not sure about mounting >> external directories. > > > We need the templating in the Dockerfile, not in the gitlab-ci itself. Same > for > mounting external directories: we need mounting (not a real requirement, but > speeds up the build step) inside the docker build process. > Simply put, one could see .gitlab-ci.xml as a form of dockerfile, since the gitlab-ci uses Docker. Former is capable of storing artefacts, although I'd imagine the extra docker/rocker here is used solely to share and reuse the artefacts and dependencies. Am I in the right ballpark? Asking all these questions since the double-docker does seem a bit strange. >> > +before_script: >> > + - mkdir -p ccache >> > + - rm -fr ../ccache >> > + - mv ccache ../ >> >> nit: how does this look >> rm -rf ../ccache >> mkdir -p ../ccache >> > > It wouldn't work. > Hmm something sounds badly broken. I would imagine you (or perhaps me) misread some the .. in the commands. >> Although why do we .. in the first place? Mind adding a comment? > > > Yup. First of all, we cache the ccache directory in GitLab, so when > re-building > a Docker image we save time. > > GitLab only caches directories that are in the same path as the git > clone/fetch > is done. That is, it restores/saves directories that are in the same place as > the full Mesa code. > > But we don't want to have the ccache there, because when building the image we > add all the cloned content. And this would add inside the image the full > ccache. > We don't want this, but just mount it externally. > AKA keeping the docker images small, while sharing the ccache. Makes sense. > Thus, we move the restored ccache to a different place at the beginning (the > "mv > ccache ../" in the before_script) and move back at the end so GitLab can store > the new cache (the "mv ../ccache ./" in the after_script). > > As the very first time there is no ccache to restore, we ensure there's > always a > ccache directory (hence, the mkdir call). The "rmdir ../ccache" is to ensure > there's nothing there before doing the move. I think this is a safe measure, > as > usually there shouldn't be nothing there. > I think that a cancelled job may leave a ../ccache around. >> >> I'm fairly sure we can drop anything older than 3.9 through the series. >> Jose was pretty clear that they're aiming/moved to llvm 5.0 >> > > I'm fine with this. We test so many versions because those are the minimum > required versions according to configure.ac. > > If we think it is not needed to check older versions, we can remove them. On > the > other hand, we could just keep them when testing release branches, and remove > when testing master. > Good call. Thinking about it - keeping the LLVM bits in your patches as-is and tweaking the version (in configure/travis/gitlab-ci/etc) would be a better. > >> >> > +RUN apt-get update >> > \ >> > + && apt-get --no-install-recommends -y install autoconf automake gcc g++ >> > libtool-bin \ >> > +pkg-config gettext ccache make scons bison flex sudo git wget bzip2 >> > xz-utils \ >> > +libclc-dev libelf-dev libexpat1-dev libffi-dev libomxil-bellagio-dev >> > \ >> > +libpciaccess-dev libx11-xcb-dev libxdamage-dev libxml2-dev >> > libxrender-dev \ >> > +libxvmc-dev libunwind-dev zlib1g-dev python-pip python-setuptools >> > python-wheel \ >> > +python3-pip python3-setuptools python3-wheel >> > \ >> > + && rm -fr /var/lib/apt/lists/* >> >> Why do we need the rm after apt-get? >> >> > > This is suggested by Docker as ag ood pattern to install packages in Docker > images. We update + install package + remove the files generated by the update > everything in a single command, so we keep the Docker image size reduced to > the > minimum. > Fair enough. I was thinking about the duplicated "sync the local package DB". Which is admittedly nothing comparing to keeping the images small. >> > +USER local >> > + >> >> A user with name "local" sounds a bit strange. Is that the
Re: [Mesa-dev] [RFC 00/10] Let us welcome EGLDevice
On 30 August 2018 at 10:54, Mathias Fröhlich wrote:
> On Tuesday, 28 August 2018 16:22:33 CEST Emil Velikov wrote:
>> > From a higher point of view the approach looks good.
>> >
>> > To sum up, you basically associate an _EGLDevice with each _EGLDisplay
>> > where the _EGLDevice only contains the meta information where to
>> > find the device and the _EGLDisplay later contains open file descriptors
>> > to work with ...
>>
>> It's the other way around - I associate each EGLDIsplay with a given
>> EGLDevice. The EGL_EXT_device_enumeration spec removed that (imho) bogus
>> relation:
>>
>> "Because the EGLDeviceEXT is a property of , any use of an
>> associated EGLDeviceEXT after has been terminated gives
>> undefined results. Querying an EGL_DEVICE_EXT from after a
>> call to eglTerminate() (and subsequent re-initialization) may
>> return a different value."
>
> Ok, wording difference. The _EGLDisplay has a member that points to an
> _EGLDevice. So each display has a device.
> But the list of _EGLDevices exists independent of the list of _EGLDisplays.
> Sounds plausible to me.
>
Precisely.
>> > Nevertheless, running the tests and proof of concept programs that I used
>> > back in the day brought up some questions and one crash.
>> >
>> > At first the Crash: The attached eglcontext-pbuffer.c which goes the
>> > pbuffer approach instead of going surfaceless just dumps core in pbuffer
>> > creation using the patch series. I believe that it is legal what the
>> > program does, but may be you want to double check that too.
>>
>> Off the top of my head, I think that's due to eglCreatePbufferSurface
>> instead of eglCreatePlatformPbufferSurfaceEXT.
>> I think we could wire that legacy API up, although the whole thing is
>> _really_ fiddly.
>>
>> See my piglit patch 85e3b32b3260184853ec20b938574c26a0f39dd8 for some
>> details.
>
> Can you explain that in more detail?
>
> Don't we have a initialized _EGLDisplay in our hands that does not require any
> further 'guessing of the underlying platform'.
>
Correct. We had a number of bugs (one of which breaking MT apps) that
makes me cautious.
> May be I am missing the latest development in this area, but I could not find
> any pointer to eglCreatePlatformPbufferSurfaceEXT in the wild. May be you can
> help out here?
>
You're correct - there isn't one. I should have checked the spec
because speaking.
> Anyhow, IMO it must not crash with a null pointer dereference even with the
> first patch series.
>
Agreed - will need to take a closer look why things crash, while the
piglit test works.
Might even add a piglit subtest.
>> > Then if I use the patch series on an account that has no DISPLAY set and
>> > no
>> > 'display server' running, eglInitialize fails in device_probe_device due
>> > to at first opening the '.../card0' device and bailing out when this does
>> > not work. Means the current patch series goes via opening the primary
>> > node which shall not be accessible by everyone and from that derives the
>> > rendernode device which is finally used for _EGLDisplay initialization.
>> > Can we alternatively go directly to the rendernode in some way?
>>
>> I think we could. Can you elaborate a bit more or provide an example
>> on the topic though.
>> I'm quite curious what's happening here - is there no card0 node,
>> open() fails, other?
>
> It's just the file permissions:
>
> $ ls -l /dev/dri/{card*,render*}
> crw-rw+ 1 root video 226, 0 Aug 30 08:28 /dev/dri/card0
> crw-rw-rw- 1 root render 226, 128 Aug 30 08:28 /dev/dri/renderD128
>
> which is pretty much what I expect from the basic idea of render nodes.
> As long as you are the one logged into the console you can access card0 via an
> dynamically added acl. But if you are not logged into the console, which is at
> least one of the major driving use cases of the device enumeration extension,
> the current implementation fails with opening card0.
>
IIRC when someone gets logged in systemd/logind/others sets up the ACL
- both ownership and permissions.
So an open(/dev/dri/card0) will be fine - be that from tty or the DE
terminal emulator.
Can you share a brief how-to reproduce?
>
>> > For patch #7, can you explain why dri already provides the right format?
>> > It's probably correct, but I am missing some bits of that context creation
>> > big picture to give a review.
>>
>> The format used when to create a surface is passed to the driver,
>> which then calls back the loader with the exact same one.
>> Admittedly there's a ton of conversion back and forth (within the
>> driver) so it's not always clear.
>>
>> Pretty much every other platform respects the provided format, hence
>> my cleanup patches ;-)
>> That said, I'm perfectly fine with pulling those patches (and updating
>> platform_device.c) if it will make things easier.
>
> Thanks for that explanation.
> I see, it is the getBuffers call that routes that back into the loader.
> This is to align with the behavior of
[Mesa-dev] [PATCH v2] egl/wayland: do not leak wl_buffer when it is locked
If color buffer is locked, do not set its wayland buffer to NULL;
otherwise it can not be freed later.
Rather, flag it in order to destroy it later on the release event.
v2: instruct release event to unlock only or free wl_buffer too (Daniel)
This also fixes dEQP-EGL.functional.swap_buffers_with_damage.* tests.
CC: Daniel Stone
---
src/egl/drivers/dri2/egl_dri2.h | 1 +
src/egl/drivers/dri2/platform_wayland.c | 22 +++---
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index d1e4e8dcf22..349f66a3506 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -297,6 +297,7 @@ struct dri2_egl_surface
struct {
#ifdef HAVE_WAYLAND_PLATFORM
struct wl_buffer *wl_buffer;
+ boolwl_release;
__DRIimage *dri_image;
/* for is_different_gpu case. NULL else */
__DRIimage *linear_copy;
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index 11c57de0f89..03a3e0993b0 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -182,9 +182,12 @@ wl_buffer_release(void *data, struct wl_buffer *buffer)
if (dri2_surf->color_buffers[i].wl_buffer == buffer)
break;
- if (i == ARRAY_SIZE(dri2_surf->color_buffers)) {
+ assert (i < ARRAY_SIZE(dri2_surf->color_buffers));
+
+ if (dri2_surf->color_buffers[i].wl_release) {
wl_buffer_destroy(buffer);
- return;
+ dri2_surf->color_buffers[i].wl_release = false;
+ dri2_surf->color_buffers[i].wl_buffer = NULL;
}
dri2_surf->color_buffers[i].locked = false;
@@ -425,9 +428,14 @@ dri2_wl_release_buffers(struct dri2_egl_surface *dri2_surf)
dri2_egl_display(dri2_surf->base.Resource.Display);
for (int i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
- if (dri2_surf->color_buffers[i].wl_buffer &&
- !dri2_surf->color_buffers[i].locked)
- wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
+ if (dri2_surf->color_buffers[i].wl_buffer) {
+ if (dri2_surf->color_buffers[i].locked) {
+dri2_surf->color_buffers[i].wl_release = true;
+ } else {
+wl_buffer_destroy(dri2_surf->color_buffers[i].wl_buffer);
+dri2_surf->color_buffers[i].wl_buffer = NULL;
+ }
+ }
if (dri2_surf->color_buffers[i].dri_image)
dri2_dpy->image->destroyImage(dri2_surf->color_buffers[i].dri_image);
if (dri2_surf->color_buffers[i].linear_copy)
@@ -436,11 +444,9 @@ dri2_wl_release_buffers(struct dri2_egl_surface *dri2_surf)
munmap(dri2_surf->color_buffers[i].data,
dri2_surf->color_buffers[i].data_size);
- dri2_surf->color_buffers[i].wl_buffer = NULL;
dri2_surf->color_buffers[i].dri_image = NULL;
dri2_surf->color_buffers[i].linear_copy = NULL;
dri2_surf->color_buffers[i].data = NULL;
- dri2_surf->color_buffers[i].locked = false;
}
if (dri2_dpy->dri2)
@@ -968,6 +974,8 @@ dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
dri2_surf->current->wl_buffer =
create_wl_buffer(dri2_dpy, dri2_surf, image);
+ dri2_surf->current->wl_release = false;
+
wl_buffer_add_listener(dri2_surf->current->wl_buffer,
_buffer_listener, dri2_surf);
}
--
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] radv: fix passing clip/cull distances from VS to PS
okay, r-b with that.
On Thu, Aug 30, 2018 at 1:39 PM Samuel Pitoiset
wrote:
>
>
>
> On 8/30/18 12:30 PM, Bas Nieuwenhuizen wrote:
> > On Wed, Aug 29, 2018 at 10:55 PM Samuel Pitoiset
> > wrote:
> >>
> >> CTS doesn't test input clip/cull distances for the fragment
> >> shader stage, which explains why this was totally broken. I
> >> wrote a simple test locally that works now.
> >>
> >> This fixes a crash with GTA V and DXVK.
> >>
> >> Note that we are exporting unused parameters from the vertex
> >> shader now, but this can't be optimized easily because we don't
> >> keep the fragment shader info...
> >>
> >> Cc: mesa-sta...@lists.freedesktop.org
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107477
> >> Signed-off-by: Samuel Pitoiset
> >> ---
> >> src/amd/vulkan/radv_nir_to_llvm.c | 30 +-
> >> src/amd/vulkan/radv_pipeline.c| 16
> >> src/amd/vulkan/radv_shader.h | 1 +
> >> src/amd/vulkan/radv_shader_info.c | 4
> >> 4 files changed, 50 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/amd/vulkan/radv_nir_to_llvm.c
> >> b/src/amd/vulkan/radv_nir_to_llvm.c
> >> index 4940e3230f..d7cd8cc069 100644
> >> --- a/src/amd/vulkan/radv_nir_to_llvm.c
> >> +++ b/src/amd/vulkan/radv_nir_to_llvm.c
> >> @@ -2098,9 +2098,10 @@ handle_fs_input_decl(struct radv_shader_context
> >> *ctx,
> >> int idx = variable->data.location;
> >> unsigned attrib_count =
> >> glsl_count_attribute_slots(variable->type, false);
> >> LLVMValueRef interp = NULL;
> >> + uint64_t mask;
> >>
> >> variable->data.driver_location = idx * 4;
> >> - ctx->input_mask |= ((1ull << attrib_count) - 1) <<
> >> variable->data.location;
> >> + mask = ((1ull << attrib_count) - 1) << variable->data.location;
> >>
> >> if (glsl_get_base_type(glsl_without_array(variable->type)) ==
> >> GLSL_TYPE_FLOAT) {
> >> unsigned interp_type;
> >> @@ -2121,6 +2122,15 @@ handle_fs_input_decl(struct radv_shader_context
> >> *ctx,
> >> for (unsigned i = 0; i < attrib_count; ++i)
> >> ctx->inputs[ac_llvm_reg_index_soa(idx + i, 0)] = interp;
> >>
> >> + if (idx == VARYING_SLOT_CLIP_DIST0) {
> >> + /* Do not account for the number of components inside the
> >> array
> >> +* of clip/cull distances because this might wrongly set
> >> other
> >> +* bits like primitive ID or layer.
> >> +*/
> >> + mask = 1ull << VARYING_SLOT_CLIP_DIST0;
> >> + }
> >> +
> >> + ctx->input_mask |= mask;
> >> }
> >>
> >> static void
> >> @@ -2187,6 +2197,17 @@ handle_fs_inputs(struct radv_shader_context *ctx,
> >> if (LLVMIsUndef(interp_param))
> >> ctx->shader_info->fs.flat_shaded_mask |=
> >> 1u << index;
> >> ++index;
> >> + } else if (i == VARYING_SLOT_CLIP_DIST0) {
> >> + int length =
> >> ctx->shader_info->info.ps.num_input_clips_culls;
> >> +
> >> + for (unsigned j = 0; j < length; j += 4) {
> >> + inputs = ctx->inputs +
> >> ac_llvm_reg_index_soa(i, j);
> >> +
> >> + interp_param = *inputs;
> >> + interp_fs_input(ctx, index, interp_param,
> >> + ctx->abi.prim_mask,
> >> inputs);
> >> + ++index;
> >> + }
> >> } else if (i == VARYING_SLOT_POS) {
> >> for(int i = 0; i < 3; ++i)
> >> inputs[i] = ctx->abi.frag_pos[i];
> >> @@ -2482,6 +2503,13 @@ handle_vs_outputs_post(struct radv_shader_context
> >> *ctx,
> >> memcpy(_args[target - V_008DFC_SQ_EXP_POS],
> >> , sizeof(args));
> >>
> >> + /* Export the clip/cull distances values to the next
> >> stage. */
> >> + radv_export_param(ctx, param_count, [0], 0xf);
> >> + outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST0] =
> >> param_count++;
> >> + if (ctx->num_output_clips + ctx->num_output_culls > 4) {
> >> + radv_export_param(ctx, param_count, [4],
> >> 0xf);
> >> +
> >> outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST1] = param_count++;
> >> + }
> >> }
> >>
> >> LLVMValueRef pos_values[4] = {ctx->ac.f32_0, ctx->ac.f32_0,
> >> ctx->ac.f32_0, ctx->ac.f32_1};
> >> diff --git a/src/amd/vulkan/radv_pipeline.c
> >> b/src/amd/vulkan/radv_pipeline.c
> >> index e63c481d1e..0303642d7e 100644
> >> --- a/src/amd/vulkan/radv_pipeline.c
> >> +++ b/src/amd/vulkan/radv_pipeline.c
> >> @@ -3052,6 +3052,22 @@ radv_pipeline_generate_ps_inputs(struct
> >> radeon_cmdbuf *cs,
> >>
Re: [Mesa-dev] [PATCH v2] radv: fix passing clip/cull distances from VS to PS
On 8/30/18 12:30 PM, Bas Nieuwenhuizen wrote:
On Wed, Aug 29, 2018 at 10:55 PM Samuel Pitoiset
wrote:
CTS doesn't test input clip/cull distances for the fragment
shader stage, which explains why this was totally broken. I
wrote a simple test locally that works now.
This fixes a crash with GTA V and DXVK.
Note that we are exporting unused parameters from the vertex
shader now, but this can't be optimized easily because we don't
keep the fragment shader info...
Cc: mesa-sta...@lists.freedesktop.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107477
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_nir_to_llvm.c | 30 +-
src/amd/vulkan/radv_pipeline.c| 16
src/amd/vulkan/radv_shader.h | 1 +
src/amd/vulkan/radv_shader_info.c | 4
4 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/src/amd/vulkan/radv_nir_to_llvm.c
b/src/amd/vulkan/radv_nir_to_llvm.c
index 4940e3230f..d7cd8cc069 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -2098,9 +2098,10 @@ handle_fs_input_decl(struct radv_shader_context *ctx,
int idx = variable->data.location;
unsigned attrib_count = glsl_count_attribute_slots(variable->type,
false);
LLVMValueRef interp = NULL;
+ uint64_t mask;
variable->data.driver_location = idx * 4;
- ctx->input_mask |= ((1ull << attrib_count) - 1) <<
variable->data.location;
+ mask = ((1ull << attrib_count) - 1) << variable->data.location;
if (glsl_get_base_type(glsl_without_array(variable->type)) ==
GLSL_TYPE_FLOAT) {
unsigned interp_type;
@@ -2121,6 +2122,15 @@ handle_fs_input_decl(struct radv_shader_context *ctx,
for (unsigned i = 0; i < attrib_count; ++i)
ctx->inputs[ac_llvm_reg_index_soa(idx + i, 0)] = interp;
+ if (idx == VARYING_SLOT_CLIP_DIST0) {
+ /* Do not account for the number of components inside the array
+* of clip/cull distances because this might wrongly set other
+* bits like primitive ID or layer.
+*/
+ mask = 1ull << VARYING_SLOT_CLIP_DIST0;
+ }
+
+ ctx->input_mask |= mask;
}
static void
@@ -2187,6 +2197,17 @@ handle_fs_inputs(struct radv_shader_context *ctx,
if (LLVMIsUndef(interp_param))
ctx->shader_info->fs.flat_shaded_mask |= 1u <<
index;
++index;
+ } else if (i == VARYING_SLOT_CLIP_DIST0) {
+ int length =
ctx->shader_info->info.ps.num_input_clips_culls;
+
+ for (unsigned j = 0; j < length; j += 4) {
+ inputs = ctx->inputs + ac_llvm_reg_index_soa(i,
j);
+
+ interp_param = *inputs;
+ interp_fs_input(ctx, index, interp_param,
+ ctx->abi.prim_mask, inputs);
+ ++index;
+ }
} else if (i == VARYING_SLOT_POS) {
for(int i = 0; i < 3; ++i)
inputs[i] = ctx->abi.frag_pos[i];
@@ -2482,6 +2503,13 @@ handle_vs_outputs_post(struct radv_shader_context *ctx,
memcpy(_args[target - V_008DFC_SQ_EXP_POS],
, sizeof(args));
+ /* Export the clip/cull distances values to the next stage. */
+ radv_export_param(ctx, param_count, [0], 0xf);
+ outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST0] =
param_count++;
+ if (ctx->num_output_clips + ctx->num_output_culls > 4) {
+ radv_export_param(ctx, param_count, [4], 0xf);
+
outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST1] = param_count++;
+ }
}
LLVMValueRef pos_values[4] = {ctx->ac.f32_0, ctx->ac.f32_0,
ctx->ac.f32_0, ctx->ac.f32_1};
diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index e63c481d1e..0303642d7e 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -3052,6 +3052,22 @@ radv_pipeline_generate_ps_inputs(struct radeon_cmdbuf
*cs,
ps_offset++;
}
+ if (ps->info.info.ps.num_input_clips_culls) {
+ unsigned vs_offset;
+
+ vs_offset =
outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST0];
+ if (vs_offset != AC_EXP_PARAM_UNDEFINED) {
+ ps_input_cntl[ps_offset] =
offset_to_ps_input(vs_offset, true);
+ ++ps_offset;
+ }
+
+ vs_offset =
outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST1];
+ if (vs_offset != AC_EXP_PARAM_UNDEFINED) {
&& ps->info.info.ps.num_input_clips_culls > 4 ? Otherwise ps_offset
can be
Re: [Mesa-dev] [PATCH v2 4/5] gitlab-ci: update base + llvm images with scheduled pipelines
On Wed, 2018-08-29 at 14:16 +0100, Emil Velikov wrote: > On 29 August 2018 at 11:12, Juan A. Suarez Romero wrote: > > Use scheduled pipelines to update both the base and the LLVM images. > > > > This way allows to have an updated version of the base images even when > > the respect Rockerfiles keep the same. > > > > Grammar seems off. > After re-reading it, you're right :) > Please include an example when a scheduled pipeline is needed/used. > As base images are only re-built when there's a change in the Dockerfile, if Ubuntu updates the base image or some of the packages we won't get them until we change the Dockerfile, which is something that doesn't happen very frequently. Thus, we use scheduled pipelines: every week or so, we force the full re-build of the base images, that are re-built even if the Dockerfile dind't change. > Thanks > Emil > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2] Check if the window is non-NULL before setting swap interval.
On Thursday, 2018-08-30 17:55:14 +0900, Tomasz Figa wrote: > Hi Erik, Emil, Eric, > > On Tue, Jul 10, 2018 at 12:41 AM Erik Faye-Lund wrote: > > > > On Thu, Jul 5, 2018 at 7:02 PM Emil Velikov > > wrote: > > > > > > On 5 July 2018 at 17:17, Eric Engestrom wrote: > > > > On Thursday, 2018-07-05 14:43:02 +0100, Emil Velikov wrote: > > > >> On 5 July 2018 at 10:53, Eric Engestrom > > > >> wrote: > > > >> > On Monday, 2018-07-02 14:12:44 +0530, samiuddi wrote: > > > >> >> This fixes crash due to NULL window when swap interval is set > > > >> >> for pbuffer surface. > > > >> >> > > > >> >> Test: CtsDisplayTestCases pass > > > >> >> > > > >> >> Signed-off-by: samiuddi > > > >> >> --- > > > >> >> > > > >> >> Kindly ignore this patch > > > >> >> https://lists.freedesktop.org/archives/mesa-dev/2018-July/199098.html > > > >> >> > > > >> >> src/egl/drivers/dri2/platform_android.c | 2 +- > > > >> >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > >> >> > > > >> >> diff --git a/src/egl/drivers/dri2/platform_android.c > > > >> >> b/src/egl/drivers/dri2/platform_android.c > > > >> >> index ca8708a..b5b960a 100644 > > > >> >> --- a/src/egl/drivers/dri2/platform_android.c > > > >> >> +++ b/src/egl/drivers/dri2/platform_android.c > > > >> >> @@ -485,7 +485,7 @@ droid_swap_interval(_EGLDriver *drv, > > > >> >> _EGLDisplay *dpy, > > > >> >> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); > > > >> >> struct ANativeWindow *window = dri2_surf->window; > > > >> >> > > > >> >> - if (window->setSwapInterval(window, interval)) > > > >> >> + if (window && window->setSwapInterval(window, interval)) > > > >> >>return EGL_FALSE; > > > >> > > > > >> > Shouldn't we return false if we couldn't set the swap interval? > > > >> > > > > >> > I think this should be: > > > >> >if (!window || window->setSwapInterval(window, interval)) > > > >> > return EGL_FALSE; > > > >> > > > > >> Changing the patch as above will lead to eglSwapInterval consistently > > > >> failing for pbuffer surfaces. > > > > > > > > I'm not that familiar with pbuffers, but does SwapInterval really make > > > > sense for them? I thought you couldn't swap a pbuffer. > > > > > > > > If so, failing an invalid op seems like the right thing to do. > > > > Otherwise, I guess sure, no-op returning success is fine. > > > > > > > Looking at eglSwapInterval manpage [1] (with my annotation): > > > "eglSwapInterval — specifies the minimum number of video frame periods > > > per buffer swap for the _window_ associated with the current context." > > > While the spec does not mention window/pbuffer/pixmap at all - it > > > extensively refers to eglSwapBuffers. > > > > > > Wrt eglSwapBuffers manpage [2] and spec are consistent: > > > > > > "If surface is a pixel buffer or a pixmap, eglSwapBuffers has no > > > effect, and no error is generated." > > > > > > Perhaps it's wrong to set eglSwapInterval for !window surfaces, but > > > its sibling (eglSwapBuffers) does not error out. > > > Hence I doubt many users make a distinction between window and pbuffer > > > surfaces for eglSwap*. > > > > > > Worth bringing to the WG meeting - it' planned for 1st August. > > > > > > > As I pointed out when I proposed this variant here: > > https://patchwork.freedesktop.org/patch/219313/ > > > > "Also, I don't think EGL_FALSE is the right return-value, as it doesn't > > seem the EGL 1.5 spec defines any such error. Also, for instance > > dri2_swap_interval returns EGL_TRUE when there's no driver-function, > > which further backs the "silent failure" in this case IMO." > > > > So I think EGL_TRUE is the correct return-value for now. If the spec > > gets changed, we can of course update our implementation. > > What happens to this patch in the end? It looks like we're observing a > similar problem in Chrome OS. > > Emil, was there any follow-up on the WG meeting? Meeting was cancelled, but I raised the issue here: https://gitlab.khronos.org/egl/API/merge_requests/17 Right now we have ARM saying they return false + error and NVIDIA saying they return true + no error and that ARM has a bug. I think another party adding their opinion might nudge it forward :) > > Best regards, > Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] radv: fix passing clip/cull distances from VS to PS
On Wed, Aug 29, 2018 at 10:55 PM Samuel Pitoiset
wrote:
>
> CTS doesn't test input clip/cull distances for the fragment
> shader stage, which explains why this was totally broken. I
> wrote a simple test locally that works now.
>
> This fixes a crash with GTA V and DXVK.
>
> Note that we are exporting unused parameters from the vertex
> shader now, but this can't be optimized easily because we don't
> keep the fragment shader info...
>
> Cc: mesa-sta...@lists.freedesktop.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107477
> Signed-off-by: Samuel Pitoiset
> ---
> src/amd/vulkan/radv_nir_to_llvm.c | 30 +-
> src/amd/vulkan/radv_pipeline.c| 16
> src/amd/vulkan/radv_shader.h | 1 +
> src/amd/vulkan/radv_shader_info.c | 4
> 4 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/src/amd/vulkan/radv_nir_to_llvm.c
> b/src/amd/vulkan/radv_nir_to_llvm.c
> index 4940e3230f..d7cd8cc069 100644
> --- a/src/amd/vulkan/radv_nir_to_llvm.c
> +++ b/src/amd/vulkan/radv_nir_to_llvm.c
> @@ -2098,9 +2098,10 @@ handle_fs_input_decl(struct radv_shader_context *ctx,
> int idx = variable->data.location;
> unsigned attrib_count = glsl_count_attribute_slots(variable->type,
> false);
> LLVMValueRef interp = NULL;
> + uint64_t mask;
>
> variable->data.driver_location = idx * 4;
> - ctx->input_mask |= ((1ull << attrib_count) - 1) <<
> variable->data.location;
> + mask = ((1ull << attrib_count) - 1) << variable->data.location;
>
> if (glsl_get_base_type(glsl_without_array(variable->type)) ==
> GLSL_TYPE_FLOAT) {
> unsigned interp_type;
> @@ -2121,6 +2122,15 @@ handle_fs_input_decl(struct radv_shader_context *ctx,
> for (unsigned i = 0; i < attrib_count; ++i)
> ctx->inputs[ac_llvm_reg_index_soa(idx + i, 0)] = interp;
>
> + if (idx == VARYING_SLOT_CLIP_DIST0) {
> + /* Do not account for the number of components inside the
> array
> +* of clip/cull distances because this might wrongly set other
> +* bits like primitive ID or layer.
> +*/
> + mask = 1ull << VARYING_SLOT_CLIP_DIST0;
> + }
> +
> + ctx->input_mask |= mask;
> }
>
> static void
> @@ -2187,6 +2197,17 @@ handle_fs_inputs(struct radv_shader_context *ctx,
> if (LLVMIsUndef(interp_param))
> ctx->shader_info->fs.flat_shaded_mask |= 1u
> << index;
> ++index;
> + } else if (i == VARYING_SLOT_CLIP_DIST0) {
> + int length =
> ctx->shader_info->info.ps.num_input_clips_culls;
> +
> + for (unsigned j = 0; j < length; j += 4) {
> + inputs = ctx->inputs +
> ac_llvm_reg_index_soa(i, j);
> +
> + interp_param = *inputs;
> + interp_fs_input(ctx, index, interp_param,
> + ctx->abi.prim_mask, inputs);
> + ++index;
> + }
> } else if (i == VARYING_SLOT_POS) {
> for(int i = 0; i < 3; ++i)
> inputs[i] = ctx->abi.frag_pos[i];
> @@ -2482,6 +2503,13 @@ handle_vs_outputs_post(struct radv_shader_context *ctx,
> memcpy(_args[target - V_008DFC_SQ_EXP_POS],
>, sizeof(args));
>
> + /* Export the clip/cull distances values to the next stage. */
> + radv_export_param(ctx, param_count, [0], 0xf);
> + outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST0] =
> param_count++;
> + if (ctx->num_output_clips + ctx->num_output_culls > 4) {
> + radv_export_param(ctx, param_count, [4], 0xf);
> +
> outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST1] = param_count++;
> + }
> }
>
> LLVMValueRef pos_values[4] = {ctx->ac.f32_0, ctx->ac.f32_0,
> ctx->ac.f32_0, ctx->ac.f32_1};
> diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
> index e63c481d1e..0303642d7e 100644
> --- a/src/amd/vulkan/radv_pipeline.c
> +++ b/src/amd/vulkan/radv_pipeline.c
> @@ -3052,6 +3052,22 @@ radv_pipeline_generate_ps_inputs(struct radeon_cmdbuf
> *cs,
> ps_offset++;
> }
>
> + if (ps->info.info.ps.num_input_clips_culls) {
> + unsigned vs_offset;
> +
> + vs_offset =
> outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST0];
> + if (vs_offset != AC_EXP_PARAM_UNDEFINED) {
> + ps_input_cntl[ps_offset] =
> offset_to_ps_input(vs_offset, true);
> + ++ps_offset;
> + }
> +
> + vs_offset =
>
Re: [Mesa-dev] [PATCH v2 1/5] gitlab-ci: build Mesa using GitLab CI
On Wed, 2018-08-29 at 14:11 +0100, Emil Velikov wrote:
> Hi Juan,
>
> I've shared a number of suggestions. I'll leave that to you if they
> will be in v3 or patches on top.
>
> On 29 August 2018 at 11:12, Juan A. Suarez Romero wrote:
>
> > In order to build the images, Rocker is used. This is a tool that
> > extends the Dockerfiles with new features that are quite interested
> > here. The main features we use is the support for templating, and the
> > support for mounting external directories during the image building.
> > This help to use tools like ccache to improve the build speed.
> >
>
> I think that gitlab-ci supports templating - not sure about mounting
> external directories.
We need the templating in the Dockerfile, not in the gitlab-ci itself. Same for
mounting external directories: we need mounting (not a real requirement, but
speeds up the build step) inside the docker build process.
> But as everyone has said - one could toggle to another tool at a later
> stage... if needed.
>
> > v2:
> > - Review dependencies (Eric Anholt)
> > - Install libdrm in base image with Meson (Eric Anholt)
> > - Do system changes before apt to speed up installation (Daniel Stone)
> >
> > Signed-off-by: Juan A. Suarez Romero
> > Acked-by: Daniel Stone
> > ---
> > .gitlab-ci.yml| 177
> > gitlab-ci/Rockerfile.base | 185 ++
> > gitlab-ci/Rockerfile.llvm | 57
> > gitlab-ci/Rockerfile.mesa | 132 +++
> > 4 files changed, 551 insertions(+)
> > create mode 100644 .gitlab-ci.yml
> > create mode 100644 gitlab-ci/Rockerfile.base
> > create mode 100644 gitlab-ci/Rockerfile.llvm
> > create mode 100644 gitlab-ci/Rockerfile.mesa
> >
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > new file mode 100644
> > index 000..5cee333dd45
> > --- /dev/null
> > +++ b/.gitlab-ci.yml
> > @@ -0,0 +1,177 @@
> > +image: docker:latest
> > +
> > +services:
> > + - docker:dind
> > +
> > +stages:
> > + - base
> > + - llvm
> > + - mesa
> > +
> > +variables:
> > + DOCKER_IMAGE: $CI_REGISTRY_IMAGE
> > + CCACHE_DIR: $CI_PROJECT_DIR/../ccache
> > + LLVM: "6.0"
> > +
> > +cache:
> > + paths:
> > +- ccache/
> > + key: "$CI_JOB_STAGE"
> > +
> > +before_script:
> > + - mkdir -p ccache
> > + - rm -fr ../ccache
> > + - mv ccache ../
>
> nit: how does this look
> rm -rf ../ccache
> mkdir -p ../ccache
>
It wouldn't work.
> Although why do we .. in the first place? Mind adding a comment?
Yup. First of all, we cache the ccache directory in GitLab, so when re-building
a Docker image we save time.
GitLab only caches directories that are in the same path as the git clone/fetch
is done. That is, it restores/saves directories that are in the same place as
the full Mesa code.
But we don't want to have the ccache there, because when building the image we
add all the cloned content. And this would add inside the image the full ccache.
We don't want this, but just mount it externally.
Thus, we move the restored ccache to a different place at the beginning (the "mv
ccache ../" in the before_script) and move back at the end so GitLab can store
the new cache (the "mv ../ccache ./" in the after_script).
As the very first time there is no ccache to restore, we ensure there's always a
ccache directory (hence, the mkdir call). The "rmdir ../ccache" is to ensure
there's nothing there before doing the move. I think this is a safe measure, as
usually there shouldn't be nothing there.
>
> > + - export MAKEFLAGS=-j$(nproc)
> > + - apk --no-cache add libc6-compat
> > + - wget
> > https://github.com/grammarly/rocker/releases/download/1.3.1/rocker-1.3.1-linux_amd64.tar.gz
> > + - tar xvf rocker-1.3.1-linux_amd64.tar.gz
> > + - rm rocker-1.3.1-linux_amd64.tar.gz
> > + - mv rocker ..
> > + - docker login -u gitlab-ci-token -p $CI_BUILD_TOKEN $CI_REGISTRY
> > +
> > +after_script:
> > + - mv ../ccache ./
> > +
> > +.build_llvm: _llvm
> > + stage: llvm
> > + cache: {}
> > + script:
> > +- ../rocker build -f gitlab-ci/Rockerfile.llvm --var LLVM=$LLVM
> > +- docker push $CI_REGISTRY_IMAGE:llvm-$LLVM
> > +
>
> I think we can drop most, if not all the special LLVM handling. More
> details below.
>
>
> > +
> > +llvm:3.3:
> > + variables:
> > +LLVM: "3.3"
> > + <<: *build_llvm
> > +
> > +llvm:3.6:
> > + variables:
> > +LLVM: "3.6"
> > + <<: *build_llvm
> > +
> > +llvm:3.8:
> > + variables:
> > +LLVM: "3.8"
> > + <<: *build_llvm
> > +
>
> I'm fairly sure we can drop anything older than 3.9 through the series.
> Jose was pretty clear that they're aiming/moved to llvm 5.0
>
I'm fine with this. We test so many versions because those are the minimum
required versions according to configure.ac.
If we think it is not needed to check older versions, we can remove them. On the
other hand, we could just keep them when testing release branches, and remove
when testing master.
>
> >
[Mesa-dev] [PATCH] st/dri: use the FREE wrapper for the dri screen
From: Emil Velikov The memory is allocated with the uppercase wrapper. Tear down should match that. Cc: mesa-sta...@lists.freedesktop.org Signed-off-by: Emil Velikov --- src/gallium/state_trackers/dri/dri_screen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/state_trackers/dri/dri_screen.c b/src/gallium/state_trackers/dri/dri_screen.c index 3e4de59a433..98b02678c81 100644 --- a/src/gallium/state_trackers/dri/dri_screen.c +++ b/src/gallium/state_trackers/dri/dri_screen.c @@ -484,7 +484,7 @@ dri_destroy_screen(__DRIscreen * sPriv) pipe_loader_release(>dev, 1); - free(screen); + FREE(screen); sPriv->driverPrivate = NULL; sPriv->extensions = NULL; } -- 2.18.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC 00/10] Let us welcome EGLDevice
On Tuesday, 28 August 2018 16:22:33 CEST Emil Velikov wrote:
> > From a higher point of view the approach looks good.
> >
> > To sum up, you basically associate an _EGLDevice with each _EGLDisplay
> > where the _EGLDevice only contains the meta information where to
> > find the device and the _EGLDisplay later contains open file descriptors
> > to work with ...
>
> It's the other way around - I associate each EGLDIsplay with a given
> EGLDevice. The EGL_EXT_device_enumeration spec removed that (imho) bogus
> relation:
>
> "Because the EGLDeviceEXT is a property of , any use of an
> associated EGLDeviceEXT after has been terminated gives
> undefined results. Querying an EGL_DEVICE_EXT from after a
> call to eglTerminate() (and subsequent re-initialization) may
> return a different value."
Ok, wording difference. The _EGLDisplay has a member that points to an
_EGLDevice. So each display has a device.
But the list of _EGLDevices exists independent of the list of _EGLDisplays.
Sounds plausible to me.
> > Nevertheless, running the tests and proof of concept programs that I used
> > back in the day brought up some questions and one crash.
> >
> > At first the Crash: The attached eglcontext-pbuffer.c which goes the
> > pbuffer approach instead of going surfaceless just dumps core in pbuffer
> > creation using the patch series. I believe that it is legal what the
> > program does, but may be you want to double check that too.
>
> Off the top of my head, I think that's due to eglCreatePbufferSurface
> instead of eglCreatePlatformPbufferSurfaceEXT.
> I think we could wire that legacy API up, although the whole thing is
> _really_ fiddly.
>
> See my piglit patch 85e3b32b3260184853ec20b938574c26a0f39dd8 for some
> details.
Can you explain that in more detail?
Don't we have a initialized _EGLDisplay in our hands that does not require any
further 'guessing of the underlying platform'.
May be I am missing the latest development in this area, but I could not find
any pointer to eglCreatePlatformPbufferSurfaceEXT in the wild. May be you can
help out here?
Anyhow, IMO it must not crash with a null pointer dereference even with the
first patch series.
> > Then if I use the patch series on an account that has no DISPLAY set and
> > no
> > 'display server' running, eglInitialize fails in device_probe_device due
> > to at first opening the '.../card0' device and bailing out when this does
> > not work. Means the current patch series goes via opening the primary
> > node which shall not be accessible by everyone and from that derives the
> > rendernode device which is finally used for _EGLDisplay initialization.
> > Can we alternatively go directly to the rendernode in some way?
>
> I think we could. Can you elaborate a bit more or provide an example
> on the topic though.
> I'm quite curious what's happening here - is there no card0 node,
> open() fails, other?
It's just the file permissions:
$ ls -l /dev/dri/{card*,render*}
crw-rw+ 1 root video 226, 0 Aug 30 08:28 /dev/dri/card0
crw-rw-rw- 1 root render 226, 128 Aug 30 08:28 /dev/dri/renderD128
which is pretty much what I expect from the basic idea of render nodes.
As long as you are the one logged into the console you can access card0 via an
dynamically added acl. But if you are not logged into the console, which is at
least one of the major driving use cases of the device enumeration extension,
the current implementation fails with opening card0.
> > For patch #7, can you explain why dri already provides the right format?
> > It's probably correct, but I am missing some bits of that context creation
> > big picture to give a review.
>
> The format used when to create a surface is passed to the driver,
> which then calls back the loader with the exact same one.
> Admittedly there's a ton of conversion back and forth (within the
> driver) so it's not always clear.
>
> Pretty much every other platform respects the provided format, hence
> my cleanup patches ;-)
> That said, I'm perfectly fine with pulling those patches (and updating
> platform_device.c) if it will make things easier.
Thanks for that explanation.
I see, it is the getBuffers call that routes that back into the loader.
This is to align with the behavior of loader_dri3_get_buffers.
I am quite sure there are others here that can way better judge about possible
sideeffects of such changes. The change sounds plausible to me and if this
would be the only thing that holds back EGLDevices I can give my name for
that...
> > And finally, in patch #2 you mention that you want to avoid larger patches
> > and the announced extensions are not yet working as expected.
> > May be you can announce the extensions in a separate patch that follows
> > all the implementation patches? That probably helps people that want to
> > bisect something using piglit.
>
> It's actually patch 1/10 which "enables" the extensions.
Yes, may be put that hunk into a patch 3.5/10.
Then
[Mesa-dev] [PATCH] radv: add missing support for protected memory properties
Fixes Vulkan CTS CL#2849. Similar to the ANV driver.
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_device.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/amd/vulkan/radv_device.c b/src/amd/vulkan/radv_device.c
index 79dbbd886d..c300c88468 100644
--- a/src/amd/vulkan/radv_device.c
+++ b/src/amd/vulkan/radv_device.c
@@ -1139,6 +1139,12 @@ void radv_GetPhysicalDeviceProperties2(
properties->maxDescriptorSetUpdateAfterBindInputAttachments =
max_descriptor_set_size;
break;
}
+ case
VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROTECTED_MEMORY_PROPERTIES: {
+ VkPhysicalDeviceProtectedMemoryProperties *properties =
+ (VkPhysicalDeviceProtectedMemoryProperties
*)ext;
+ properties->protectedNoFault = false;
+ break;
+ }
default:
break;
}
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/7] radv: gather the output usage mask for clip/cull distances correctly
It's a special case because both are combined into a single array.
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_shader_info.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/amd/vulkan/radv_shader_info.c
b/src/amd/vulkan/radv_shader_info.c
index 5925fd924c..6262acb1a6 100644
--- a/src/amd/vulkan/radv_shader_info.c
+++ b/src/amd/vulkan/radv_shader_info.c
@@ -128,6 +128,14 @@ set_output_usage_mask(const nir_shader *nir, const
nir_intrinsic_instr *instr,
get_deref_offset(deref_instr, _offset);
+ if (idx == VARYING_SLOT_CLIP_DIST0) {
+ /* Special case for clip/cull distances because there are
+* combined into a single array that contains both.
+*/
+ output_usage_mask[idx] |= 1 << const_offset;
+ return;
+ }
+
for (unsigned i = 0; i < attrib_count; i++) {
output_usage_mask[idx + i + const_offset] |=
instr->const_index[0] << comp;
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 5/7] radv: adjust the cull dist mask in scan_shader_output_decl()
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_nir_to_llvm.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/amd/vulkan/radv_nir_to_llvm.c
b/src/amd/vulkan/radv_nir_to_llvm.c
index 0c7b238e10..069fb53b68 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -2248,10 +2248,12 @@ scan_shader_output_decl(struct radv_shader_context *ctx,
if (stage == MESA_SHADER_VERTEX) {
ctx->shader_info->vs.outinfo.clip_dist_mask =
(1 << shader->info.clip_distance_array_size) - 1;
ctx->shader_info->vs.outinfo.cull_dist_mask =
(1 << shader->info.cull_distance_array_size) - 1;
+ ctx->shader_info->vs.outinfo.cull_dist_mask <<=
shader->info.clip_distance_array_size;
}
if (stage == MESA_SHADER_TESS_EVAL) {
ctx->shader_info->tes.outinfo.clip_dist_mask =
(1 << shader->info.clip_distance_array_size) - 1;
ctx->shader_info->tes.outinfo.cull_dist_mask =
(1 << shader->info.cull_distance_array_size) - 1;
+ ctx->shader_info->tes.outinfo.cull_dist_mask
<<= shader->info.clip_distance_array_size;
}
if (length > 4)
@@ -2496,9 +2498,6 @@ handle_vs_outputs_post(struct radv_shader_context *ctx,
length = util_last_bit(output_usage_mask);
- if (outinfo->cull_dist_mask)
- outinfo->cull_dist_mask <<= ctx->num_output_clips;
-
i = VARYING_SLOT_CLIP_DIST0;
for (j = 0; j < length; j++)
slots[j] = ac_to_float(>ac, radv_load_output(ctx,
i, j));
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 6/7] radv: remove radv_shader_context::num_output_{clips, culls}
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_nir_to_llvm.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/src/amd/vulkan/radv_nir_to_llvm.c
b/src/amd/vulkan/radv_nir_to_llvm.c
index 069fb53b68..b2411fe38b 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -91,8 +91,6 @@ struct radv_shader_context {
uint64_t input_mask;
uint64_t output_mask;
- uint8_t num_output_clips;
- uint8_t num_output_culls;
bool is_gs_copy_shader;
LLVMValueRef gs_next_vertex;
@@ -3289,8 +3287,6 @@ LLVMModuleRef ac_translate_nir_to_llvm(struct
ac_llvm_compiler *ac_llvm,
for(int i = 0; i < shader_count; ++i) {
ctx.stage = shaders[i]->info.stage;
ctx.output_mask = 0;
- ctx.num_output_clips =
shaders[i]->info.clip_distance_array_size;
- ctx.num_output_culls =
shaders[i]->info.cull_distance_array_size;
if (shaders[i]->info.stage == MESA_SHADER_GEOMETRY) {
ctx.gs_next_vertex = ac_build_alloca(,
ctx.ac.i32, "gs_next_vertex");
@@ -3683,9 +3679,6 @@ radv_compile_gs_copy_shader(struct ac_llvm_compiler
*ac_llvm,
ctx.gs_max_out_vertices = geom_shader->info.gs.vertices_out;
ac_setup_rings();
- ctx.num_output_clips = geom_shader->info.clip_distance_array_size;
- ctx.num_output_culls = geom_shader->info.cull_distance_array_size;
-
nir_foreach_variable(variable, _shader->outputs) {
scan_shader_output_decl(, variable, geom_shader,
MESA_SHADER_VERTEX);
ac_handle_shader_output_decl(, , geom_shader,
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/7] radv: get length of the clip/cull distances array from usage mask
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_nir_to_llvm.c | 49 +--
1 file changed, 40 insertions(+), 9 deletions(-)
diff --git a/src/amd/vulkan/radv_nir_to_llvm.c
b/src/amd/vulkan/radv_nir_to_llvm.c
index 17c76332e1..0c7b238e10 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -1738,7 +1738,7 @@ visit_emit_vertex(struct ac_shader_abi *abi, unsigned
stream, LLVMValueRef *addr
if (i == VARYING_SLOT_CLIP_DIST0) {
/* pack clip and cull into a single set of slots */
- length = ctx->num_output_clips + ctx->num_output_culls;
+ length = util_last_bit(output_usage_mask);
if (length > 4)
slot_inc = 2;
}
@@ -2477,20 +2477,36 @@ handle_vs_outputs_post(struct radv_shader_context *ctx,
sizeof(outinfo->vs_output_param_offset));
if (ctx->output_mask & (1ull << VARYING_SLOT_CLIP_DIST0)) {
+ unsigned output_usage_mask, length;
LLVMValueRef slots[8];
unsigned j;
+ if (ctx->stage == MESA_SHADER_VERTEX &&
+ !ctx->is_gs_copy_shader) {
+ output_usage_mask =
+
ctx->shader_info->info.vs.output_usage_mask[VARYING_SLOT_CLIP_DIST0];
+ } else if (ctx->stage == MESA_SHADER_TESS_EVAL) {
+ output_usage_mask =
+
ctx->shader_info->info.tes.output_usage_mask[VARYING_SLOT_CLIP_DIST0];
+ } else {
+ assert(ctx->is_gs_copy_shader);
+ output_usage_mask =
+
ctx->shader_info->info.gs.output_usage_mask[VARYING_SLOT_CLIP_DIST0];
+ }
+
+ length = util_last_bit(output_usage_mask);
+
if (outinfo->cull_dist_mask)
outinfo->cull_dist_mask <<= ctx->num_output_clips;
i = VARYING_SLOT_CLIP_DIST0;
- for (j = 0; j < ctx->num_output_clips + ctx->num_output_culls;
j++)
+ for (j = 0; j < length; j++)
slots[j] = ac_to_float(>ac, radv_load_output(ctx,
i, j));
- for (i = ctx->num_output_clips + ctx->num_output_culls; i < 8;
i++)
+ for (i = length; i < 8; i++)
slots[i] = LLVMGetUndef(ctx->ac.f32);
- if (ctx->num_output_clips + ctx->num_output_culls > 4) {
+ if (length > 4) {
target = V_008DFC_SQ_EXP_POS + 3;
si_llvm_init_export_args(ctx, [4], 0xf, target,
);
memcpy(_args[target - V_008DFC_SQ_EXP_POS],
@@ -2505,7 +2521,7 @@ handle_vs_outputs_post(struct radv_shader_context *ctx,
/* Export the clip/cull distances values to the next stage. */
radv_export_param(ctx, param_count, [0], 0xf);
outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST0] =
param_count++;
- if (ctx->num_output_clips + ctx->num_output_culls > 4) {
+ if (length > 4) {
radv_export_param(ctx, param_count, [4], 0xf);
outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST1] = param_count++;
}
@@ -2662,14 +2678,24 @@ handle_es_outputs_post(struct radv_shader_context *ctx,
LLVMValueRef lds_base = NULL;
for (unsigned i = 0; i < AC_LLVM_MAX_OUTPUTS; ++i) {
+ unsigned output_usage_mask;
int param_index;
int length = 4;
if (!(ctx->output_mask & (1ull << i)))
continue;
+ if (ctx->stage == MESA_SHADER_VERTEX) {
+ output_usage_mask =
+ ctx->shader_info->info.vs.output_usage_mask[i];
+ } else {
+ assert(ctx->stage == MESA_SHADER_TESS_EVAL);
+ output_usage_mask =
+ ctx->shader_info->info.tes.output_usage_mask[i];
+ }
+
if (i == VARYING_SLOT_CLIP_DIST0)
- length = ctx->num_output_clips + ctx->num_output_culls;
+ length = util_last_bit(output_usage_mask);
param_index = shader_io_get_unique_index(i);
@@ -2711,7 +2737,7 @@ handle_es_outputs_post(struct radv_shader_context *ctx,
}
if (i == VARYING_SLOT_CLIP_DIST0)
- length = ctx->num_output_clips + ctx->num_output_culls;
+ length = util_last_bit(output_usage_mask);
param_index = shader_io_get_unique_index(i);
@@ -2758,6 +2784,8 @@ handle_ls_outputs_post(struct radv_shader_context *ctx)
[Mesa-dev] [PATCH 3/7] radv: do not recompute the output usage mask for clipdist twice
The shader info pass takes care of this now.
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_nir_to_llvm.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/amd/vulkan/radv_nir_to_llvm.c
b/src/amd/vulkan/radv_nir_to_llvm.c
index d7cd8cc069..17c76332e1 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -1741,7 +1741,6 @@ visit_emit_vertex(struct ac_shader_abi *abi, unsigned
stream, LLVMValueRef *addr
length = ctx->num_output_clips + ctx->num_output_culls;
if (length > 4)
slot_inc = 2;
- output_usage_mask = (1 << length) - 1;
}
for (unsigned j = 0; j < length; j++) {
@@ -2711,10 +2710,8 @@ handle_es_outputs_post(struct radv_shader_context *ctx,
ctx->shader_info->info.tes.output_usage_mask[i];
}
- if (i == VARYING_SLOT_CLIP_DIST0) {
+ if (i == VARYING_SLOT_CLIP_DIST0)
length = ctx->num_output_clips + ctx->num_output_culls;
- output_usage_mask = (1 << length) - 1;
- }
param_index = shader_io_get_unique_index(i);
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 7/7] radv: remove dead code in scan_shader_output_decl()
Never used.
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_nir_to_llvm.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/src/amd/vulkan/radv_nir_to_llvm.c
b/src/amd/vulkan/radv_nir_to_llvm.c
index b2411fe38b..af34c548c1 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -2241,8 +2241,6 @@ scan_shader_output_decl(struct radv_shader_context *ctx,
stage == MESA_SHADER_TESS_EVAL ||
stage == MESA_SHADER_GEOMETRY) {
if (idx == VARYING_SLOT_CLIP_DIST0) {
- int length = shader->info.clip_distance_array_size +
-shader->info.cull_distance_array_size;
if (stage == MESA_SHADER_VERTEX) {
ctx->shader_info->vs.outinfo.clip_dist_mask =
(1 << shader->info.clip_distance_array_size) - 1;
ctx->shader_info->vs.outinfo.cull_dist_mask =
(1 << shader->info.cull_distance_array_size) - 1;
@@ -2254,10 +2252,6 @@ scan_shader_output_decl(struct radv_shader_context *ctx,
ctx->shader_info->tes.outinfo.cull_dist_mask
<<= shader->info.clip_distance_array_size;
}
- if (length > 4)
- attrib_count = 2;
- else
- attrib_count = 1;
mask_attribs = 1ull << idx;
}
}
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/7] radv: add set_output_usage_mask() helper
Signed-off-by: Samuel Pitoiset
---
src/amd/vulkan/radv_shader_info.c | 43 +++
1 file changed, 26 insertions(+), 17 deletions(-)
diff --git a/src/amd/vulkan/radv_shader_info.c
b/src/amd/vulkan/radv_shader_info.c
index a45c847c46..5925fd924c 100644
--- a/src/amd/vulkan/radv_shader_info.c
+++ b/src/amd/vulkan/radv_shader_info.c
@@ -114,6 +114,26 @@ gather_intrinsic_load_deref_info(const nir_shader *nir,
}
}
+static void
+set_output_usage_mask(const nir_shader *nir, const nir_intrinsic_instr *instr,
+ uint8_t *output_usage_mask)
+{
+ nir_deref_instr *deref_instr =
+ nir_instr_as_deref(instr->src[0].ssa->parent_instr);
+ nir_variable *var = nir_deref_instr_get_variable(deref_instr);
+ unsigned attrib_count = glsl_count_attribute_slots(var->type, false);
+ unsigned idx = var->data.location;
+ unsigned comp = var->data.location_frac;
+ unsigned const_offset = 0;
+
+ get_deref_offset(deref_instr, _offset);
+
+ for (unsigned i = 0; i < attrib_count; i++) {
+ output_usage_mask[idx + i + const_offset] |=
+ instr->const_index[0] << comp;
+ }
+}
+
static void
gather_intrinsic_store_deref_info(const nir_shader *nir,
const nir_intrinsic_instr *instr,
@@ -122,31 +142,20 @@ gather_intrinsic_store_deref_info(const nir_shader *nir,
nir_variable *var =
nir_deref_instr_get_variable(nir_instr_as_deref(instr->src[0].ssa->parent_instr));
if (var->data.mode == nir_var_shader_out) {
- unsigned attrib_count = glsl_count_attribute_slots(var->type,
false);
unsigned idx = var->data.location;
- unsigned comp = var->data.location_frac;
- unsigned const_offset = 0;
-
-
get_deref_offset(nir_instr_as_deref(instr->src[0].ssa->parent_instr),
_offset);
switch (nir->info.stage) {
case MESA_SHADER_VERTEX:
- for (unsigned i = 0; i < attrib_count; i++) {
- info->vs.output_usage_mask[idx + i +
const_offset] |=
- instr->const_index[0] << comp;
- }
+ set_output_usage_mask(nir, instr,
+ info->vs.output_usage_mask);
break;
case MESA_SHADER_GEOMETRY:
- for (unsigned i = 0; i < attrib_count; i++) {
- info->gs.output_usage_mask[idx + i +
const_offset] |=
- instr->const_index[0] << comp;
- }
+ set_output_usage_mask(nir, instr,
+ info->gs.output_usage_mask);
break;
case MESA_SHADER_TESS_EVAL:
- for (unsigned i = 0; i < attrib_count; i++) {
- info->tes.output_usage_mask[idx + i +
const_offset] |=
- instr->const_index[0] << comp;
- }
+ set_output_usage_mask(nir, instr,
+ info->tes.output_usage_mask);
break;
case MESA_SHADER_TESS_CTRL: {
unsigned param = shader_io_get_unique_index(idx);
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2] Check if the window is non-NULL before setting swap interval.
Hi Erik, Emil, Eric, On Tue, Jul 10, 2018 at 12:41 AM Erik Faye-Lund wrote: > > On Thu, Jul 5, 2018 at 7:02 PM Emil Velikov wrote: > > > > On 5 July 2018 at 17:17, Eric Engestrom wrote: > > > On Thursday, 2018-07-05 14:43:02 +0100, Emil Velikov wrote: > > >> On 5 July 2018 at 10:53, Eric Engestrom wrote: > > >> > On Monday, 2018-07-02 14:12:44 +0530, samiuddi wrote: > > >> >> This fixes crash due to NULL window when swap interval is set > > >> >> for pbuffer surface. > > >> >> > > >> >> Test: CtsDisplayTestCases pass > > >> >> > > >> >> Signed-off-by: samiuddi > > >> >> --- > > >> >> > > >> >> Kindly ignore this patch > > >> >> https://lists.freedesktop.org/archives/mesa-dev/2018-July/199098.html > > >> >> > > >> >> src/egl/drivers/dri2/platform_android.c | 2 +- > > >> >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> >> > > >> >> diff --git a/src/egl/drivers/dri2/platform_android.c > > >> >> b/src/egl/drivers/dri2/platform_android.c > > >> >> index ca8708a..b5b960a 100644 > > >> >> --- a/src/egl/drivers/dri2/platform_android.c > > >> >> +++ b/src/egl/drivers/dri2/platform_android.c > > >> >> @@ -485,7 +485,7 @@ droid_swap_interval(_EGLDriver *drv, _EGLDisplay > > >> >> *dpy, > > >> >> struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); > > >> >> struct ANativeWindow *window = dri2_surf->window; > > >> >> > > >> >> - if (window->setSwapInterval(window, interval)) > > >> >> + if (window && window->setSwapInterval(window, interval)) > > >> >>return EGL_FALSE; > > >> > > > >> > Shouldn't we return false if we couldn't set the swap interval? > > >> > > > >> > I think this should be: > > >> >if (!window || window->setSwapInterval(window, interval)) > > >> > return EGL_FALSE; > > >> > > > >> Changing the patch as above will lead to eglSwapInterval consistently > > >> failing for pbuffer surfaces. > > > > > > I'm not that familiar with pbuffers, but does SwapInterval really make > > > sense for them? I thought you couldn't swap a pbuffer. > > > > > > If so, failing an invalid op seems like the right thing to do. > > > Otherwise, I guess sure, no-op returning success is fine. > > > > > Looking at eglSwapInterval manpage [1] (with my annotation): > > "eglSwapInterval — specifies the minimum number of video frame periods > > per buffer swap for the _window_ associated with the current context." > > While the spec does not mention window/pbuffer/pixmap at all - it > > extensively refers to eglSwapBuffers. > > > > Wrt eglSwapBuffers manpage [2] and spec are consistent: > > > > "If surface is a pixel buffer or a pixmap, eglSwapBuffers has no > > effect, and no error is generated." > > > > Perhaps it's wrong to set eglSwapInterval for !window surfaces, but > > its sibling (eglSwapBuffers) does not error out. > > Hence I doubt many users make a distinction between window and pbuffer > > surfaces for eglSwap*. > > > > Worth bringing to the WG meeting - it' planned for 1st August. > > > > As I pointed out when I proposed this variant here: > https://patchwork.freedesktop.org/patch/219313/ > > "Also, I don't think EGL_FALSE is the right return-value, as it doesn't > seem the EGL 1.5 spec defines any such error. Also, for instance > dri2_swap_interval returns EGL_TRUE when there's no driver-function, > which further backs the "silent failure" in this case IMO." > > So I think EGL_TRUE is the correct return-value for now. If the spec > gets changed, we can of course update our implementation. What happens to this patch in the end? It looks like we're observing a similar problem in Chrome OS. Emil, was there any follow-up on the WG meeting? Best regards, Tomasz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radv/meta: Set num_components on image_store intrinsics
On Thu, Aug 30, 2018 at 9:17 AM Samuel Pitoiset wrote: > > > > On 8/30/18 2:49 AM, Jason Ekstrand wrote: > > Now that image load/store intrinsics are variable-width, we need to set > > num_components accordingly. In 15d39f474b890, both glsl_to_nir and > > spirv_to_nir were updated to properly set num_components but radv meta > > was left behind. > > > > Fixes: 15d39f474b890 "nir: Make image load/store intrinsics..." > > --- > > src/amd/vulkan/radv_meta_bufimage.c | 4 > > src/amd/vulkan/radv_meta_fast_clear.c | 1 + > > src/amd/vulkan/radv_meta_resolve_cs.c | 1 + > > 3 files changed, 6 insertions(+) > > > > diff --git a/src/amd/vulkan/radv_meta_bufimage.c > > b/src/amd/vulkan/radv_meta_bufimage.c > > index aa17c25833b..8a9ac7a8ea4 100644 > > --- a/src/amd/vulkan/radv_meta_bufimage.c > > +++ b/src/amd/vulkan/radv_meta_bufimage.c > > @@ -116,6 +116,7 @@ build_nir_itob_compute_shader(struct radv_device *dev, > > bool is_3d) > > > > nir_ssa_def *outval = >dest.ssa; > > nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, > > nir_intrinsic_image_deref_store); > > + store->num_components = 4; > > store->src[0] = nir_src_for_ssa(_build_deref_var(, > > output_img)->dest.ssa); > > store->src[1] = nir_src_for_ssa(coord); > > store->src[2] = nir_src_for_ssa(nir_ssa_undef(, 1, 32)); > > @@ -342,6 +343,7 @@ build_nir_btoi_compute_shader(struct radv_device *dev, > > bool is_3d) > > > > nir_ssa_def *outval = >dest.ssa; > > nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, > > nir_intrinsic_image_deref_store); > > + store->num_components = 4; > > store->src[0] = nir_src_for_ssa(_build_deref_var(, > > output_img)->dest.ssa); > > store->src[1] = nir_src_for_ssa(img_coord); > > store->src[2] = nir_src_for_ssa(nir_ssa_undef(, 1, 32)); > > @@ -557,6 +559,7 @@ build_nir_itoi_compute_shader(struct radv_device *dev, > > bool is_3d) > > > > nir_ssa_def *outval = >dest.ssa; > > nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, > > nir_intrinsic_image_deref_store); > > +store->num_components = 4; > > Indentation issue, otherwise: > > Reviewed-by: Samuel Pitoiset Thanks for fixing this while I was asleep! Reviewed-by: Bas Nieuwenhuizen Tested-by: Bas Nieuwenhuizen > > > store->src[0] = nir_src_for_ssa(_build_deref_var(, > > output_img)->dest.ssa); > > store->src[1] = nir_src_for_ssa(dst_coord); > > store->src[2] = nir_src_for_ssa(nir_ssa_undef(, 1, 32)); > > @@ -753,6 +756,7 @@ build_nir_cleari_compute_shader(struct radv_device > > *dev, bool is_3d) > > global_id = nir_vec(, comps, 4); > > > > nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, > > nir_intrinsic_image_deref_store); > > + store->num_components = 4; > > store->src[0] = nir_src_for_ssa(_build_deref_var(, > > output_img)->dest.ssa); > > store->src[1] = nir_src_for_ssa(global_id); > > store->src[2] = nir_src_for_ssa(nir_ssa_undef(, 1, 32)); > > diff --git a/src/amd/vulkan/radv_meta_fast_clear.c > > b/src/amd/vulkan/radv_meta_fast_clear.c > > index b4cc900028e..9544ee94f5c 100644 > > --- a/src/amd/vulkan/radv_meta_fast_clear.c > > +++ b/src/amd/vulkan/radv_meta_fast_clear.c > > @@ -92,6 +92,7 @@ build_dcc_decompress_compute_shader(struct radv_device > > *dev) > > > > nir_ssa_def *outval = >dest.ssa; > > nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, > > nir_intrinsic_image_deref_store); > > + store->num_components = 4; > > store->src[0] = nir_src_for_ssa(_build_deref_var(, > > output_img)->dest.ssa); > > store->src[1] = nir_src_for_ssa(global_id); > > store->src[2] = nir_src_for_ssa(nir_ssa_undef(, 1, 32)); > > diff --git a/src/amd/vulkan/radv_meta_resolve_cs.c > > b/src/amd/vulkan/radv_meta_resolve_cs.c > > index fca49a01bb0..2fcfb0aaeff 100644 > > --- a/src/amd/vulkan/radv_meta_resolve_cs.c > > +++ b/src/amd/vulkan/radv_meta_resolve_cs.c > > @@ -136,6 +136,7 @@ build_resolve_compute_shader(struct radv_device *dev, > > bool is_integer, bool is_s > > > > nir_ssa_def *coord = nir_iadd(, global_id, _offset->dest.ssa); > > nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, > > nir_intrinsic_image_deref_store); > > + store->num_components = 4; > > store->src[0] = nir_src_for_ssa(_build_deref_var(, > > output_img)->dest.ssa); > > store->src[1] = nir_src_for_ssa(coord); > > store->src[2] = nir_src_for_ssa(nir_ssa_undef(, 1, 32)); > > > ___ > 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] radv/meta: Set num_components on image_store intrinsics
On 8/30/18 2:49 AM, Jason Ekstrand wrote: Now that image load/store intrinsics are variable-width, we need to set num_components accordingly. In 15d39f474b890, both glsl_to_nir and spirv_to_nir were updated to properly set num_components but radv meta was left behind. Fixes: 15d39f474b890 "nir: Make image load/store intrinsics..." --- src/amd/vulkan/radv_meta_bufimage.c | 4 src/amd/vulkan/radv_meta_fast_clear.c | 1 + src/amd/vulkan/radv_meta_resolve_cs.c | 1 + 3 files changed, 6 insertions(+) diff --git a/src/amd/vulkan/radv_meta_bufimage.c b/src/amd/vulkan/radv_meta_bufimage.c index aa17c25833b..8a9ac7a8ea4 100644 --- a/src/amd/vulkan/radv_meta_bufimage.c +++ b/src/amd/vulkan/radv_meta_bufimage.c @@ -116,6 +116,7 @@ build_nir_itob_compute_shader(struct radv_device *dev, bool is_3d) nir_ssa_def *outval = >dest.ssa; nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, nir_intrinsic_image_deref_store); + store->num_components = 4; store->src[0] = nir_src_for_ssa(_build_deref_var(, output_img)->dest.ssa); store->src[1] = nir_src_for_ssa(coord); store->src[2] = nir_src_for_ssa(nir_ssa_undef(, 1, 32)); @@ -342,6 +343,7 @@ build_nir_btoi_compute_shader(struct radv_device *dev, bool is_3d) nir_ssa_def *outval = >dest.ssa; nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, nir_intrinsic_image_deref_store); + store->num_components = 4; store->src[0] = nir_src_for_ssa(_build_deref_var(, output_img)->dest.ssa); store->src[1] = nir_src_for_ssa(img_coord); store->src[2] = nir_src_for_ssa(nir_ssa_undef(, 1, 32)); @@ -557,6 +559,7 @@ build_nir_itoi_compute_shader(struct radv_device *dev, bool is_3d) nir_ssa_def *outval = >dest.ssa; nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, nir_intrinsic_image_deref_store); +store->num_components = 4; Indentation issue, otherwise: Reviewed-by: Samuel Pitoiset store->src[0] = nir_src_for_ssa(_build_deref_var(, output_img)->dest.ssa); store->src[1] = nir_src_for_ssa(dst_coord); store->src[2] = nir_src_for_ssa(nir_ssa_undef(, 1, 32)); @@ -753,6 +756,7 @@ build_nir_cleari_compute_shader(struct radv_device *dev, bool is_3d) global_id = nir_vec(, comps, 4); nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, nir_intrinsic_image_deref_store); + store->num_components = 4; store->src[0] = nir_src_for_ssa(_build_deref_var(, output_img)->dest.ssa); store->src[1] = nir_src_for_ssa(global_id); store->src[2] = nir_src_for_ssa(nir_ssa_undef(, 1, 32)); diff --git a/src/amd/vulkan/radv_meta_fast_clear.c b/src/amd/vulkan/radv_meta_fast_clear.c index b4cc900028e..9544ee94f5c 100644 --- a/src/amd/vulkan/radv_meta_fast_clear.c +++ b/src/amd/vulkan/radv_meta_fast_clear.c @@ -92,6 +92,7 @@ build_dcc_decompress_compute_shader(struct radv_device *dev) nir_ssa_def *outval = >dest.ssa; nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, nir_intrinsic_image_deref_store); + store->num_components = 4; store->src[0] = nir_src_for_ssa(_build_deref_var(, output_img)->dest.ssa); store->src[1] = nir_src_for_ssa(global_id); store->src[2] = nir_src_for_ssa(nir_ssa_undef(, 1, 32)); diff --git a/src/amd/vulkan/radv_meta_resolve_cs.c b/src/amd/vulkan/radv_meta_resolve_cs.c index fca49a01bb0..2fcfb0aaeff 100644 --- a/src/amd/vulkan/radv_meta_resolve_cs.c +++ b/src/amd/vulkan/radv_meta_resolve_cs.c @@ -136,6 +136,7 @@ build_resolve_compute_shader(struct radv_device *dev, bool is_integer, bool is_s nir_ssa_def *coord = nir_iadd(, global_id, _offset->dest.ssa); nir_intrinsic_instr *store = nir_intrinsic_instr_create(b.shader, nir_intrinsic_image_deref_store); + store->num_components = 4; store->src[0] = nir_src_for_ssa(_build_deref_var(, output_img)->dest.ssa); store->src[1] = nir_src_for_ssa(coord); store->src[2] = nir_src_for_ssa(nir_ssa_undef(, 1, 32)); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
