Re: [Mesa-dev] [PATCH 15/20] pan/midgard: Use type-appropriate swizzle for texture coordinate
Thanks, fixed in v2.
On Fri, Aug 16, 2019 at 11:55:59AM -0400, Ilia Mirkin wrote:
> On Fri, Aug 16, 2019 at 11:38 AM Alyssa Rosenzweig
> wrote:
> >
> > The texture coordinate for a 2D texture could be a vec2 or a vec3,
> > depending if it's an array texture or not. If it's vec2 (non-array
> > texture), we should not reference the z component; otherwise, liveness
> > analysis will get very confused when z is never written.
> >
> > Signed-off-by: Alyssa Rosenzweig
> > ---
> > src/panfrost/midgard/midgard_compile.c | 8 +++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/panfrost/midgard/midgard_compile.c
> > b/src/panfrost/midgard/midgard_compile.c
> > index 67c5c848214..0be9cc32dc2 100644
> > --- a/src/panfrost/midgard/midgard_compile.c
> > +++ b/src/panfrost/midgard/midgard_compile.c
> > @@ -1753,6 +1753,7 @@ emit_texop_native(compiler_context *ctx,
> > nir_tex_instr *instr,
> > for (unsigned i = 0; i < instr->num_srcs; ++i) {
> > int index = nir_src_index(ctx, &instr->src[i].src);
> > midgard_vector_alu_src alu_src = blank_alu_src;
> > +unsigned nr_components =
> > nir_src_num_components(instr->src[i].src);
> >
> > switch (instr->src[i].src_type) {
> > case nir_tex_src_coord: {
> > @@ -1804,7 +1805,12 @@ emit_texop_native(compiler_context *ctx,
> > nir_tex_instr *instr,
> >
> > if (instr->sampler_dim == GLSL_SAMPLER_DIM_2D) {
> > /* Array component in w but NIR wants it
> > in z */
> > -ins.texture.in_reg_swizzle = SWIZZLE_XYZZ;
> > +if (nr_components == 3)
> > +ins.texture.in_reg_swizzle =
> > SWIZZLE_XYZZ;
> > +else if (nr_components == 2)
> > +ins.texture.in_reg_swizzle =
> > SWIZZLE_XYXX;
> > +else
> > +unreachable("Invalid texture 2D
> > componens");
>
> Just a drive-by review: components
>
> -ilia
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 15/20] pan/midgard: Use type-appropriate swizzle for texture coordinate
On Fri, Aug 16, 2019 at 11:38 AM Alyssa Rosenzweig
wrote:
>
> The texture coordinate for a 2D texture could be a vec2 or a vec3,
> depending if it's an array texture or not. If it's vec2 (non-array
> texture), we should not reference the z component; otherwise, liveness
> analysis will get very confused when z is never written.
>
> Signed-off-by: Alyssa Rosenzweig
> ---
> src/panfrost/midgard/midgard_compile.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/panfrost/midgard/midgard_compile.c
> b/src/panfrost/midgard/midgard_compile.c
> index 67c5c848214..0be9cc32dc2 100644
> --- a/src/panfrost/midgard/midgard_compile.c
> +++ b/src/panfrost/midgard/midgard_compile.c
> @@ -1753,6 +1753,7 @@ emit_texop_native(compiler_context *ctx, nir_tex_instr
> *instr,
> for (unsigned i = 0; i < instr->num_srcs; ++i) {
> int index = nir_src_index(ctx, &instr->src[i].src);
> midgard_vector_alu_src alu_src = blank_alu_src;
> +unsigned nr_components =
> nir_src_num_components(instr->src[i].src);
>
> switch (instr->src[i].src_type) {
> case nir_tex_src_coord: {
> @@ -1804,7 +1805,12 @@ emit_texop_native(compiler_context *ctx, nir_tex_instr
> *instr,
>
> if (instr->sampler_dim == GLSL_SAMPLER_DIM_2D) {
> /* Array component in w but NIR wants it in
> z */
> -ins.texture.in_reg_swizzle = SWIZZLE_XYZZ;
> +if (nr_components == 3)
> +ins.texture.in_reg_swizzle =
> SWIZZLE_XYZZ;
> +else if (nr_components == 2)
> +ins.texture.in_reg_swizzle =
> SWIZZLE_XYXX;
> +else
> +unreachable("Invalid texture 2D
> componens");
Just a drive-by review: components
-ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 16/20] pan/midgard: Clamp st_vary swizzle by number of components
Same issue with liveness analysis. If we store out a vec3, we should not
reference the .w component.
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/midgard_compile.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/panfrost/midgard/midgard_compile.c
b/src/panfrost/midgard/midgard_compile.c
index 0be9cc32dc2..73ba3173eb3 100644
--- a/src/panfrost/midgard/midgard_compile.c
+++ b/src/panfrost/midgard/midgard_compile.c
@@ -1577,11 +1577,12 @@ emit_intrinsic(compiler_context *ctx,
nir_intrinsic_instr *instr)
emit_explicit_constant(ctx, reg, reg);
unsigned component = nir_intrinsic_component(instr);
+unsigned nr_comp =
nir_src_num_components(instr->src[0]);
midgard_instruction st = m_st_vary_32(reg, offset);
st.load_store.arg_1 = 0x9E;
st.load_store.arg_2 = 0x1E;
-st.load_store.swizzle = SWIZZLE_XYZW << (2*component);
+st.load_store.swizzle = swizzle_of(nr_comp) <<
(2*component);
emit_mir_instruction(ctx, st);
} else {
DBG("Unknown store\n");
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 10/20] pan/midgard: Index blocks for printing
Better than having pointers flying about.
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/compiler.h| 6 ++
src/panfrost/midgard/midgard_compile.c | 2 ++
src/panfrost/midgard/midgard_print.c | 4 ++--
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h
index 217ab317090..1fddd3f1fe8 100644
--- a/src/panfrost/midgard/compiler.h
+++ b/src/panfrost/midgard/compiler.h
@@ -160,6 +160,9 @@ typedef struct midgard_block {
/* List of midgard_instructions emitted for the current block */
struct list_head instructions;
+/* Index of the block in source order */
+unsigned source_id;
+
bool is_scheduled;
/* List of midgard_bundles emitted (after the scheduler has run) */
@@ -229,6 +232,9 @@ typedef struct compiler_context {
int block_count;
struct list_head blocks;
+/* TODO merge with block_count? */
+unsigned block_source_count;
+
/* List of midgard_instructions emitted for the current block */
midgard_block *current_block;
diff --git a/src/panfrost/midgard/midgard_compile.c
b/src/panfrost/midgard/midgard_compile.c
index 12af84d17d6..67c5c848214 100644
--- a/src/panfrost/midgard/midgard_compile.c
+++ b/src/panfrost/midgard/midgard_compile.c
@@ -2264,6 +2264,8 @@ create_empty_block(compiler_context *ctx)
_mesa_hash_pointer,
_mesa_key_pointer_equal);
+blk->source_id = ctx->block_source_count++;
+
return blk;
}
diff --git a/src/panfrost/midgard/midgard_print.c
b/src/panfrost/midgard/midgard_print.c
index 5d19f6f6800..240780a1f7d 100644
--- a/src/panfrost/midgard/midgard_print.c
+++ b/src/panfrost/midgard/midgard_print.c
@@ -162,7 +162,7 @@ mir_print_instruction(midgard_instruction *ins)
void
mir_print_block(midgard_block *block)
{
-printf("%p: {\n", block);
+printf("block%d: {\n", block->source_id);
mir_foreach_instr_in_block(block, ins) {
mir_print_instruction(ins);
@@ -173,7 +173,7 @@ mir_print_block(midgard_block *block)
if (block->nr_successors) {
printf(" -> ");
for (unsigned i = 0; i < block->nr_successors; ++i) {
-printf("%p%s", block->successors[i],
+printf("block%d%s", block->successors[i]->source_id,
(i + 1) != block->nr_successors ? ", "
: "");
}
}
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 07/20] pan/midgard: Add mir_foreach_successor helper
Now we should be able to walk the control-flow graph naturally. Signed-off-by: Alyssa Rosenzweig --- src/panfrost/midgard/compiler.h | 8 1 file changed, 8 insertions(+) diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h index f196389c36a..a78d933e9e4 100644 --- a/src/panfrost/midgard/compiler.h +++ b/src/panfrost/midgard/compiler.h @@ -360,6 +360,14 @@ mir_next_op(struct midgard_instruction *ins) mir_foreach_block(ctx, v_block) \ mir_foreach_instr_in_block_safe(v_block, v) +#define mir_foreach_successor(blk, v) \ +struct midgard_block *v; \ +struct midgard_block **_v; \ +for (_v = &blk->successors[0], \ +v = *_v; \ +v != NULL && _v < &blk->successors[2]; \ +_v++, v = *_v) \ + /* Based on set_foreach, expanded with automatic type casts */ #define mir_foreach_predecessor(blk, v) \ -- 2.23.0.rc1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 09/20] pan/midgard: Add mir_foreach_src
This is repeated often enough.
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/compiler.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h
index 6f953d48cb8..217ab317090 100644
--- a/src/panfrost/midgard/compiler.h
+++ b/src/panfrost/midgard/compiler.h
@@ -381,6 +381,9 @@ mir_next_op(struct midgard_instruction *ins)
_entry_##v = _mesa_set_next_entry(blk->predecessors,
_entry_##v), \
v = (struct midgard_block *) (_entry_##v ? _entry_##v->key :
NULL))
+#define mir_foreach_src(ins, v) \
+for (unsigned v = 0; v < ARRAY_SIZE(ins->ssa_args.src); ++v)
+
static inline midgard_instruction *
mir_last_in_block(struct midgard_block *block)
{
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/20] pan/midgard: Add mir_foreach_predecessor utility
It's ugly, but c'est la vie.
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/compiler.h | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h
index e51ea8ba602..f196389c36a 100644
--- a/src/panfrost/midgard/compiler.h
+++ b/src/panfrost/midgard/compiler.h
@@ -360,6 +360,17 @@ mir_next_op(struct midgard_instruction *ins)
mir_foreach_block(ctx, v_block) \
mir_foreach_instr_in_block_safe(v_block, v)
+/* Based on set_foreach, expanded with automatic type casts */
+
+#define mir_foreach_predecessor(blk, v) \
+struct set_entry *_entry_##v; \
+struct midgard_block *v; \
+for (_entry_##v = _mesa_set_next_entry(blk->predecessors, NULL), \
+v = (struct midgard_block *) (_entry_##v ? _entry_##v->key :
NULL); \
+_entry_##v != NULL; \
+_entry_##v = _mesa_set_next_entry(blk->predecessors,
_entry_##v), \
+v = (struct midgard_block *) (_entry_##v ? _entry_##v->key :
NULL))
+
static inline midgard_instruction *
mir_last_in_block(struct midgard_block *block)
{
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/20] pan/midgard: Maintain block predecessor set
While we already compute the successors array, for backwards data flow
analysis, it is useful to walk the control flow graph backwards based on
predecessors, so let's compute that information as well.
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/compiler.h| 2 ++
src/panfrost/midgard/midgard_compile.c | 21 ++---
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h
index 61fe8a92b2e..fb47c3475bf 100644
--- a/src/panfrost/midgard/compiler.h
+++ b/src/panfrost/midgard/compiler.h
@@ -173,6 +173,8 @@ typedef struct midgard_block {
struct midgard_block *successors[2];
unsigned nr_successors;
+struct set *predecessors;
+
/* The successors pointer form a graph, and in the case of
* complex control flow, this graph has a cycles. To aid
* traversal during liveness analysis, we have a visited?
diff --git a/src/panfrost/midgard/midgard_compile.c
b/src/panfrost/midgard/midgard_compile.c
index 4a010c97443..f08f60fc328 100644
--- a/src/panfrost/midgard/midgard_compile.c
+++ b/src/panfrost/midgard/midgard_compile.c
@@ -89,6 +89,9 @@ midgard_block_add_successor(midgard_block *block,
midgard_block *successor)
block->successors[block->nr_successors++] = successor;
assert(block->nr_successors <= ARRAY_SIZE(block->successors));
+
+/* Note the predecessor in the other direction */
+_mesa_set_add(successor->predecessors, block);
}
/* Helpers to generate midgard_instruction's using macro magic, since every
@@ -2252,6 +2255,18 @@ emit_fragment_epilogue(compiler_context *ctx)
EMIT(alu_br_compact_cond, midgard_jmp_writeout_op_writeout, TAG_ALU_4,
-1, midgard_condition_always);
}
+static midgard_block *
+create_empty_block(compiler_context *ctx)
+{
+midgard_block *blk = rzalloc(ctx, midgard_block);
+
+blk->predecessors = _mesa_set_create(blk,
+_mesa_hash_pointer,
+_mesa_key_pointer_equal);
+
+return blk;
+}
+
static midgard_block *
emit_block(compiler_context *ctx, nir_block *block)
{
@@ -2259,7 +2274,7 @@ emit_block(compiler_context *ctx, nir_block *block)
ctx->after_block = NULL;
if (!this_block)
-this_block = rzalloc(ctx, midgard_block);
+this_block = create_empty_block(ctx);
list_addtail(&this_block->link, &ctx->blocks);
@@ -2342,7 +2357,7 @@ emit_if(struct compiler_context *ctx, nir_if *nif)
/* Wire up the successors */
-ctx->after_block = rzalloc(ctx, midgard_block);
+ctx->after_block = create_empty_block(ctx);
midgard_block_add_successor(before_block, then_block);
midgard_block_add_successor(before_block, else_block);
@@ -2381,7 +2396,7 @@ emit_loop(struct compiler_context *ctx, nir_loop *nloop)
/* Fix up the break statements we emitted to point to the right place,
* now that we can allocate a block number for them */
-ctx->after_block = rzalloc(ctx, midgard_block);
+ctx->after_block = create_empty_block(ctx);
list_for_each_entry_from(struct midgard_block, block, start_block,
&ctx->blocks, link) {
mir_foreach_instr_in_block(block, ins) {
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 15/20] pan/midgard: Use type-appropriate swizzle for texture coordinate
The texture coordinate for a 2D texture could be a vec2 or a vec3,
depending if it's an array texture or not. If it's vec2 (non-array
texture), we should not reference the z component; otherwise, liveness
analysis will get very confused when z is never written.
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/midgard_compile.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/panfrost/midgard/midgard_compile.c
b/src/panfrost/midgard/midgard_compile.c
index 67c5c848214..0be9cc32dc2 100644
--- a/src/panfrost/midgard/midgard_compile.c
+++ b/src/panfrost/midgard/midgard_compile.c
@@ -1753,6 +1753,7 @@ emit_texop_native(compiler_context *ctx, nir_tex_instr
*instr,
for (unsigned i = 0; i < instr->num_srcs; ++i) {
int index = nir_src_index(ctx, &instr->src[i].src);
midgard_vector_alu_src alu_src = blank_alu_src;
+unsigned nr_components =
nir_src_num_components(instr->src[i].src);
switch (instr->src[i].src_type) {
case nir_tex_src_coord: {
@@ -1804,7 +1805,12 @@ emit_texop_native(compiler_context *ctx, nir_tex_instr
*instr,
if (instr->sampler_dim == GLSL_SAMPLER_DIM_2D) {
/* Array component in w but NIR wants it in z
*/
-ins.texture.in_reg_swizzle = SWIZZLE_XYZZ;
+if (nr_components == 3)
+ins.texture.in_reg_swizzle =
SWIZZLE_XYZZ;
+else if (nr_components == 2)
+ins.texture.in_reg_swizzle =
SWIZZLE_XYXX;
+else
+unreachable("Invalid texture 2D
componens");
}
break;
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 05/20] pan/midgard: Link exit block
The exit block has been 'dangling' in the successors graph, so let's ensure it's linked in. Signed-off-by: Alyssa Rosenzweig --- src/panfrost/midgard/midgard_compile.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/panfrost/midgard/midgard_compile.c b/src/panfrost/midgard/midgard_compile.c index f08f60fc328..12af84d17d6 100644 --- a/src/panfrost/midgard/midgard_compile.c +++ b/src/panfrost/midgard/midgard_compile.c @@ -2575,7 +2575,15 @@ midgard_compile_shader_nir(struct midgard_screen *screen, nir_shader *nir, midga ctx->func = func; emit_cf_list(ctx, &func->impl->body); -emit_block(ctx, func->impl->end_block); + +/* Emit empty exit block with successor */ + +struct midgard_block *semi_end = ctx->current_block; + +struct midgard_block *end = +emit_block(ctx, func->impl->end_block); + +midgard_block_add_successor(semi_end, end); break; /* TODO: Multi-function shaders */ } -- 2.23.0.rc1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 04/20] pan/midgard: Add mir_exit_block helper
The exit block is gauranteed to be empty, signaling the end of the
program.
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/compiler.h | 13 +
1 file changed, 13 insertions(+)
diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h
index fb47c3475bf..e51ea8ba602 100644
--- a/src/panfrost/midgard/compiler.h
+++ b/src/panfrost/midgard/compiler.h
@@ -377,6 +377,19 @@ mir_get_block(compiler_context *ctx, int idx)
return (struct midgard_block *) lst;
}
+static inline midgard_block *
+mir_exit_block(struct compiler_context *ctx)
+{
+midgard_block *last = list_last_entry(&ctx->blocks,
+struct midgard_block, link);
+
+/* The last block must be empty (the exit block) */
+assert(list_empty(&last->instructions));
+assert(last->nr_successors == 0);
+
+return last;
+}
+
static inline bool
mir_is_alu_bundle(midgard_bundle *bundle)
{
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 12/20] pan/midgard: Add mir_rewrite_index_dst_single helper
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/compiler.h | 1 +
src/panfrost/midgard/mir.c | 9 +++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h
index 1fddd3f1fe8..1cbebdbef2e 100644
--- a/src/panfrost/midgard/compiler.h
+++ b/src/panfrost/midgard/compiler.h
@@ -478,6 +478,7 @@ void mir_rewrite_index(compiler_context *ctx, unsigned old,
unsigned new);
void mir_rewrite_index_src(compiler_context *ctx, unsigned old, unsigned new);
void mir_rewrite_index_dst(compiler_context *ctx, unsigned old, unsigned new);
void mir_rewrite_index_dst_tag(compiler_context *ctx, unsigned old, unsigned
new, unsigned tag);
+void mir_rewrite_index_dst_single(midgard_instruction *ins, unsigned old,
unsigned new);
void mir_rewrite_index_src_single(midgard_instruction *ins, unsigned old,
unsigned new);
void mir_rewrite_index_src_tag(compiler_context *ctx, unsigned old, unsigned
new, unsigned tag);
void mir_rewrite_index_src_swizzle(compiler_context *ctx, unsigned old,
unsigned new, unsigned swizzle);
diff --git a/src/panfrost/midgard/mir.c b/src/panfrost/midgard/mir.c
index 97921a419f2..9e269629131 100644
--- a/src/panfrost/midgard/mir.c
+++ b/src/panfrost/midgard/mir.c
@@ -32,6 +32,12 @@ void mir_rewrite_index_src_single(midgard_instruction *ins,
unsigned old, unsign
}
}
+void mir_rewrite_index_dst_single(midgard_instruction *ins, unsigned old,
unsigned new)
+{
+if (ins->ssa_args.dest == old)
+ins->ssa_args.dest = new;
+}
+
static unsigned
mir_get_swizzle(midgard_instruction *ins, unsigned idx)
{
@@ -181,8 +187,7 @@ void
mir_rewrite_index_dst(compiler_context *ctx, unsigned old, unsigned new)
{
mir_foreach_instr_global(ctx, ins) {
-if (ins->ssa_args.dest == old)
-ins->ssa_args.dest = new;
+mir_rewrite_index_dst_single(ins, old, new);
}
}
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 18/20] pan/midgard: Treat cubemaps "stores" as loads
It's always been ambiguous which they are, but their primary register is
their output, not their input; therefore, they are loads.
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/helpers.h | 5 ++---
src/panfrost/midgard/midgard.h | 9 -
src/panfrost/midgard/midgard_compile.c | 14 +++---
src/panfrost/midgard/midgard_ops.c | 2 +-
src/panfrost/midgard/midgard_ra.c | 4 +---
5 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/src/panfrost/midgard/helpers.h b/src/panfrost/midgard/helpers.h
index afed7d97e2d..ffcf9d84de1 100644
--- a/src/panfrost/midgard/helpers.h
+++ b/src/panfrost/midgard/helpers.h
@@ -47,8 +47,7 @@
)
#define OP_IS_STORE(op) (\
-OP_IS_STORE_R26(op) || \
-op == midgard_op_st_cubemap_coords \
+OP_IS_STORE_R26(op) \
)
#define OP_IS_PROJECTION(op) ( \
@@ -58,7 +57,7 @@
#define OP_IS_VEC4_ONLY(op) ( \
OP_IS_PROJECTION(op) || \
-op == midgard_op_st_cubemap_coords \
+op == midgard_op_ld_cubemap_coords \
)
#define OP_IS_MOVE(op) ( \
diff --git a/src/panfrost/midgard/midgard.h b/src/panfrost/midgard/midgard.h
index a43b7f309ed..753da1c064e 100644
--- a/src/panfrost/midgard/midgard.h
+++ b/src/panfrost/midgard/midgard.h
@@ -391,12 +391,11 @@ midgard_writeout;
typedef enum {
midgard_op_ld_st_noop = 0x03,
-/* Unclear why this is on the L/S unit, but (with an address of 0,
- * appropriate swizzle, magic constant 0x24, and xy mask?) moves fp32
cube
- * map coordinates in r27 to its cube map texture coordinate
- * destination (e.g r29). 0x4 magic for lding from fp16 instead */
+/* Unclear why this is on the L/S unit, but moves fp32 cube map
+ * coordinates in r27 to its cube map texture coordinate destination
+ * (e.g r29). */
-midgard_op_st_cubemap_coords = 0x0E,
+midgard_op_ld_cubemap_coords = 0x0E,
/* Loads a global/local/group ID, depending on arguments */
midgard_op_ld_compute_id = 0x10,
diff --git a/src/panfrost/midgard/midgard_compile.c
b/src/panfrost/midgard/midgard_compile.c
index 705c3b32778..2b6e851167b 100644
--- a/src/panfrost/midgard/midgard_compile.c
+++ b/src/panfrost/midgard/midgard_compile.c
@@ -190,7 +190,7 @@ M_STORE(st_int4);
M_LOAD(ld_color_buffer_8);
//M_STORE(st_vary_16);
M_STORE(st_vary_32);
-M_LOAD(st_cubemap_coords);
+M_LOAD(ld_cubemap_coords);
M_LOAD(ld_compute_id);
static midgard_instruction
@@ -1792,12 +1792,12 @@ emit_texop_native(compiler_context *ctx, nir_tex_instr
*instr,
* texture register */
unsigned temp = make_compiler_temp(ctx);
-midgard_instruction st =
m_st_cubemap_coords(temp, 0);
-st.ssa_args.src[0] = index;
-st.mask = 0x3; /* xy */
-st.load_store.arg_1 = 0x20;
-st.load_store.swizzle = alu_src.swizzle;
-emit_mir_instruction(ctx, st);
+midgard_instruction ld =
m_ld_cubemap_coords(temp, 0);
+ld.ssa_args.src[0] = index;
+ld.mask = 0x3; /* xy */
+ld.load_store.arg_1 = 0x20;
+ld.load_store.swizzle = alu_src.swizzle;
+emit_mir_instruction(ctx, ld);
ins.ssa_args.src[0] = temp;
ins.texture.in_reg_swizzle = SWIZZLE_XYXX;
diff --git a/src/panfrost/midgard/midgard_ops.c
b/src/panfrost/midgard/midgard_ops.c
index d8fd80df277..dbc87386bc0 100644
--- a/src/panfrost/midgard/midgard_ops.c
+++ b/src/panfrost/midgard/midgard_ops.c
@@ -170,7 +170,7 @@ struct mir_op_props alu_opcode_props[256] = {
};
const char *load_store_opcode_names[256] = {
-[midgard_op_st_cubemap_coords] = "st_cubemap_coords",
+[midgard_op_ld_cubemap_coords] = "ld_cubemap_coords",
[midgard_op_ld_compute_id] = "ld_compute_id",
[midgard_op_ldst_perspective_division_z] =
"ldst_perspective_division_z",
[midgard_op_ldst_perspective_division_w] =
"ldst_perspective_division_w",
diff --git a/src/panfrost/midgard/midgard_ra.c
b/src/panfrost/midgard/midgard_ra.c
index 6a417d48c91..ebd085cd9a3 100644
--- a/src/panfrost/midgard/midgard_ra.c
+++ b/src/panfrost/midgard/midgard_ra.c
@@ -752,9 +752,7 @@ install_registers_instr(
* whether we are loading or storing -- think about the
* logical dataflow */
-bool encodes_src =
-OP_IS_STORE(ins->load_store.op) &&
-ins->load_store.op != midgard_op_st_cubemap_coords;
+bool encodes_src =
[Mesa-dev] [PATCH 14/20] pan/midgard: Set mask for lowered read-hazard moves
If we need to lower a move for a read from a vec2 texture coordinate, we
shouldn't write zw, even incidentally.
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/midgard_ra.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/panfrost/midgard/midgard_ra.c
b/src/panfrost/midgard/midgard_ra.c
index 5df8ecf0df0..6a417d48c91 100644
--- a/src/panfrost/midgard/midgard_ra.c
+++ b/src/panfrost/midgard/midgard_ra.c
@@ -501,6 +501,7 @@ mir_lower_special_reads(compiler_context *ctx)
} else {
idx = spill_idx++;
m = v_mov(i, blank_alu_src, idx);
+m.mask =
mir_mask_of_read_components(pre_use, i);
mir_insert_instruction_before(pre_use,
m);
mir_rewrite_index_src_single(pre_use,
i, idx);
}
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 13/20] pan/midgard: Fix texw lowering with complex control flow
Fixes shaders with control flow like:
out = 0;
if (A) {
if (B)
out = texture(A, ...)
} else {
out = texture(B, ...)
}
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/midgard_ra.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/panfrost/midgard/midgard_ra.c
b/src/panfrost/midgard/midgard_ra.c
index 4754971acb2..5df8ecf0df0 100644
--- a/src/panfrost/midgard/midgard_ra.c
+++ b/src/panfrost/midgard/midgard_ra.c
@@ -497,7 +497,7 @@ mir_lower_special_reads(compiler_context *ctx)
midgard_instruction *use =
mir_next_op(pre_use);
assert(use);
mir_insert_instruction_before(use, m);
-mir_rewrite_index_dst_tag(ctx, i, idx,
classes[j]);
+mir_rewrite_index_dst_single(pre_use,
i, idx);
} else {
idx = spill_idx++;
m = v_mov(i, blank_alu_src, idx);
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 17/20] pan/midgard: Clamp cubemap swizzle to XYXX
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/midgard_compile.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/panfrost/midgard/midgard_compile.c
b/src/panfrost/midgard/midgard_compile.c
index 73ba3173eb3..705c3b32778 100644
--- a/src/panfrost/midgard/midgard_compile.c
+++ b/src/panfrost/midgard/midgard_compile.c
@@ -1800,6 +1800,7 @@ emit_texop_native(compiler_context *ctx, nir_tex_instr
*instr,
emit_mir_instruction(ctx, st);
ins.ssa_args.src[0] = temp;
+ins.texture.in_reg_swizzle = SWIZZLE_XYXX;
} else {
ins.ssa_args.src[0] = index;
}
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 20/20] pan/midgard: Compute liveness per-block
Rather than using a regalloc based on live internals, computed hastily
with repeated invocations of a forward-analysis pass, we switch to
compute liveness information on a per-block basis.
Within a given basic block, we compute liveness backwards with a
linear-time algorithm; for common shaders, this may help RA terminate
quicker.
Across blocks, we use a work list (really a work set) and check if we're
making progress. This isn't terribly efficient, but it gets the job
done. Point is, we get the live_in/live_out for each block.
From there, it's simple to rerun the linear-time update algorithm to
compute the interference graph.
The benefit of this technique is the ability to ignore "gaps" in
liveness across intermediate blocks that are never executed. On simple
shaders like the loops in glmark, this results in a minor reduction in
register pressure. The motivation was a complex shader in Krita that
failed register allocation due to an unfortunate interaction between
texture pipeline registers and control flow. This shader now compiles
successfully.
total instructions in shared programs: 3439 -> 3438 (-0.03%)
instructions in affected programs: 22 -> 21 (-4.55%)
helped: 1
HURT: 0
total bundles in shared programs: 2077 -> 2076 (-0.05%)
bundles in affected programs: 12 -> 11 (-8.33%)
helped: 1
HURT: 0
total quadwords in shared programs: 3457 -> 3456 (-0.03%)
quadwords in affected programs: 20 -> 19 (-5.00%)
helped: 1
HURT: 0
total registers in shared programs: 341 -> 338 (-0.88%)
registers in affected programs: 9 -> 6 (-33.33%)
helped: 3
HURT: 0
helped stats (abs) min: 1 max: 1 x̄: 1.00 x̃: 1
helped stats (rel) min: 33.33% max: 33.33% x̄: 33.33% x̃: 33.33%
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/compiler.h | 8 ++
src/panfrost/midgard/midgard_ra.c | 231 +-
2 files changed, 169 insertions(+), 70 deletions(-)
diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h
index 1cbebdbef2e..edf0c105a19 100644
--- a/src/panfrost/midgard/compiler.h
+++ b/src/panfrost/midgard/compiler.h
@@ -184,6 +184,14 @@ typedef struct midgard_block {
* boolean for passes to use as they see fit, provided they
* clean up later */
bool visited;
+
+/* In liveness analysis, these are live masks (per-component) for
+ * indices for the block. Scalar compilers have the luxury of using
+ * simple bit fields, but for us, liveness is a vector idea. We use
+ * 8-bit to allow finegrained tracking up to vec8. If you're
+ * implementing vec16 on Panfrost... I'm sorry. */
+uint8_t *live_in;
+uint8_t *live_out;
} midgard_block;
typedef struct midgard_bundle {
diff --git a/src/panfrost/midgard/midgard_ra.c
b/src/panfrost/midgard/midgard_ra.c
index ebd085cd9a3..9d38054f691 100644
--- a/src/panfrost/midgard/midgard_ra.c
+++ b/src/panfrost/midgard/midgard_ra.c
@@ -26,6 +26,7 @@
#include "midgard_ops.h"
#include "util/register_allocate.h"
#include "util/u_math.h"
+#include "util/u_memory.h"
/* For work registers, we can subdivide in various ways. So we create
* classes for the various sizes and conflict accordingly, keeping in
@@ -516,6 +517,165 @@ mir_lower_special_reads(compiler_context *ctx)
free(texw);
}
+/* Routines for liveness analysis */
+
+static void
+liveness_gen(uint8_t *live, unsigned node, unsigned max, unsigned mask)
+{
+if ((node < 0) || (node >= max))
+return;
+
+live[node] |= mask;
+}
+
+static void
+liveness_kill(uint8_t *live, unsigned node, unsigned max, unsigned mask)
+{
+if ((node < 0) || (node >= max))
+return;
+
+live[node] &= ~mask;
+}
+
+/* Updates live_in for a single instruction */
+
+static void
+liveness_ins_update(uint8_t *live, midgard_instruction *ins, unsigned max)
+{
+/* live_in[s] = GEN[s] + (live_out[s] - KILL[s]) */
+
+liveness_kill(live, ins->ssa_args.dest, max, ins->mask);
+
+mir_foreach_src(ins, src) {
+unsigned node = ins->ssa_args.src[src];
+unsigned mask = mir_mask_of_read_components(ins, node);
+
+liveness_gen(live, node, max, mask);
+}
+}
+
+/* live_out[s] = sum { p in succ[s] } ( live_in[p] ) */
+
+static void
+liveness_block_live_out(compiler_context *ctx, midgard_block *blk)
+{
+mir_foreach_successor(blk, succ) {
+for (unsigned i = 0; i < ctx->temp_count; ++i)
+blk->live_out[i] |= succ->live_in[i];
+}
+}
+
+/* Liveness analysis is a backwards-may dataflow analysis pass. Within a block,
+ * we compute live_out from live_in. The intrablock pass is linear-time. It
+ * returns whether progress was made. */
+
+static bool
+liveness_block_update(compiler_context *ctx, midgard_block *blk)
+{
+bool progress = false;
+
+liveness_block_live_out(ctx, blk);
+
+uint8_t *live = mem_dup(blk->live_out, ctx->temp_
[Mesa-dev] [PATCH 11/20] pan/midgard: Print predecessors in MIR
Just as a sanity check.
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/midgard_print.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/panfrost/midgard/midgard_print.c
b/src/panfrost/midgard/midgard_print.c
index 240780a1f7d..1f8bd12680e 100644
--- a/src/panfrost/midgard/midgard_print.c
+++ b/src/panfrost/midgard/midgard_print.c
@@ -178,6 +178,11 @@ mir_print_block(midgard_block *block)
}
}
+printf(" from { ");
+mir_foreach_predecessor(block, pred)
+printf("block%d ", pred->source_id);
+printf("}");
+
printf("\n\n");
}
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 19/20] pan/midgard: Analyze load/store for swizzle propagation
If there's a nontrivial swizzle fed into an extra (shortened) argument,
we bail on copyprop. No glmark changes (since it doesn't use fancy
texturing/loads).
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/midgard_opt_copy_prop.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/src/panfrost/midgard/midgard_opt_copy_prop.c
b/src/panfrost/midgard/midgard_opt_copy_prop.c
index dc5579c4d46..68c1a4d0d55 100644
--- a/src/panfrost/midgard/midgard_opt_copy_prop.c
+++ b/src/panfrost/midgard/midgard_opt_copy_prop.c
@@ -52,13 +52,31 @@ midgard_opt_copy_prop(compiler_context *ctx, midgard_block
*block)
if (mir_nontrivial_source2_mod_simple(ins)) continue;
if (mir_nontrivial_outmod(ins)) continue;
-/* Texture ops have some weirdness around bias */
+/* Shortened arguments (bias for textures, extra load/store
+ * arguments, etc.) do not get a swizzlw, only a start
+ * component and even that is restricted. */
bool skip = false;
mir_foreach_instr_global(ctx, q) {
-if (q->ssa_args.src[1] != to) continue;
-if (q->type == TAG_TEXTURE_4) skip = true;
+bool is_tex = q->type == TAG_TEXTURE_4;
+bool is_ldst = q->type == TAG_LOAD_STORE_4;
+
+if (!(is_tex || is_ldst)) continue;
+
+/* For textures, we get one real swizzle. For stores,
+ * we also get one. For loads, we get none. */
+
+unsigned start =
+is_tex ? 1 :
+OP_IS_STORE(q->load_store.op) ? 1 : 0;
+
+mir_foreach_src(q, s) {
+if ((s >= start) && q->ssa_args.src[s] == to) {
+skip = true;
+break;
+}
+}
}
if (skip)
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 08/20] pan/midgard: Add mir_foreach_instr_in_block_rev
Signed-off-by: Alyssa Rosenzweig --- src/panfrost/midgard/compiler.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h index a78d933e9e4..6f953d48cb8 100644 --- a/src/panfrost/midgard/compiler.h +++ b/src/panfrost/midgard/compiler.h @@ -336,6 +336,8 @@ mir_next_op(struct midgard_instruction *ins) #define mir_foreach_instr_in_block(block, v) \ list_for_each_entry(struct midgard_instruction, v, &block->instructions, link) +#define mir_foreach_instr_in_block_rev(block, v) \ +list_for_each_entry_rev(struct midgard_instruction, v, &block->instructions, link) #define mir_foreach_instr_in_block_safe(block, v) \ list_for_each_entry_safe(struct midgard_instruction, v, &block->instructions, link) -- 2.23.0.rc1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 01/20] pan/midgard: Shrink successors[] to 2 length
A block can't have more.
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/compiler.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h
index 942d288a326..61fe8a92b2e 100644
--- a/src/panfrost/midgard/compiler.h
+++ b/src/panfrost/midgard/compiler.h
@@ -170,7 +170,7 @@ typedef struct midgard_block {
/* Succeeding blocks. The compiler should not necessarily rely on
* source-order traversal */
-struct midgard_block *successors[4];
+struct midgard_block *successors[2];
unsigned nr_successors;
/* The successors pointer form a graph, and in the case of
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 00/20] panfrost: Fix Midgard liveness analysis
This series was motivated by RA failures in a Krita shader. The fix -- rewriting liveness analysis entirely -- turned out to be rather more involved than I expected. Ah well, I think it was worth it (as the issue fixed really does apply to GLES2 even, it was just missed in dEQP). Alyssa Rosenzweig (20): pan/midgard: Shrink successors[] to 2 length pan/midgard: Use ralloc on ctx/blocks pan/midgard: Maintain block predecessor set pan/midgard: Add mir_exit_block helper pan/midgard: Link exit block pan/midgard: Add mir_foreach_predecessor utility pan/midgard: Add mir_foreach_successor helper pan/midgard: Add mir_foreach_instr_in_block_rev pan/midgard: Add mir_foreach_src pan/midgard: Index blocks for printing pan/midgard: Print predecessors in MIR pan/midgard: Add mir_rewrite_index_dst_single helper pan/midgard: Fix texw lowering with complex control flow pan/midgard: Set mask for lowered read-hazard moves pan/midgard: Use type-appropriate swizzle for texture coordinate pan/midgard: Clamp st_vary swizzle by number of components pan/midgard: Clamp cubemap swizzle to XYXX pan/midgard: Treat cubemaps "stores" as loads pan/midgard: Analyze load/store for swizzle propagation pan/midgard: Compute liveness per-block src/panfrost/midgard/compiler.h | 56 - src/panfrost/midgard/helpers.h | 5 +- src/panfrost/midgard/midgard.h | 9 +- src/panfrost/midgard/midgard_compile.c | 79 -- src/panfrost/midgard/midgard_ops.c | 2 +- src/panfrost/midgard/midgard_opt_copy_prop.c | 24 +- src/panfrost/midgard/midgard_print.c | 9 +- src/panfrost/midgard/midgard_ra.c| 238 +-- src/panfrost/midgard/mir.c | 9 +- 9 files changed, 315 insertions(+), 116 deletions(-) -- 2.23.0.rc1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 02/20] pan/midgard: Use ralloc on ctx/blocks
This will allow us to get some level of automatic memory management.
Signed-off-by: Alyssa Rosenzweig
---
src/panfrost/midgard/midgard_compile.c | 26 +++---
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/src/panfrost/midgard/midgard_compile.c
b/src/panfrost/midgard/midgard_compile.c
index 59a323c828b..4a010c97443 100644
--- a/src/panfrost/midgard/midgard_compile.c
+++ b/src/panfrost/midgard/midgard_compile.c
@@ -2259,7 +2259,7 @@ emit_block(compiler_context *ctx, nir_block *block)
ctx->after_block = NULL;
if (!this_block)
-this_block = calloc(sizeof(midgard_block), 1);
+this_block = rzalloc(ctx, midgard_block);
list_addtail(&this_block->link, &ctx->blocks);
@@ -2342,7 +2342,7 @@ emit_if(struct compiler_context *ctx, nir_if *nif)
/* Wire up the successors */
-ctx->after_block = calloc(sizeof(midgard_block), 1);
+ctx->after_block = rzalloc(ctx, midgard_block);
midgard_block_add_successor(before_block, then_block);
midgard_block_add_successor(before_block, else_block);
@@ -2381,7 +2381,7 @@ emit_loop(struct compiler_context *ctx, nir_loop *nloop)
/* Fix up the break statements we emitted to point to the right place,
* now that we can allocate a block number for them */
-ctx->after_block = calloc(sizeof(midgard_block), 1);
+ctx->after_block = rzalloc(ctx, midgard_block);
list_for_each_entry_from(struct midgard_block, block, start_block,
&ctx->blocks, link) {
mir_foreach_instr_in_block(block, ins) {
@@ -2477,19 +2477,14 @@ midgard_compile_shader_nir(struct midgard_screen
*screen, nir_shader *nir, midga
midgard_debug = debug_get_option_midgard_debug();
-compiler_context ictx = {
-.nir = nir,
-.screen = screen,
-.stage = nir->info.stage,
-.temp_alloc = 0,
+/* TODO: Bound against what? */
+compiler_context *ctx = rzalloc(NULL, compiler_context);
-.is_blend = is_blend,
-.blend_constant_offset = 0,
-
-.alpha_ref = program->alpha_ref
-};
-
-compiler_context *ctx = &ictx;
+ctx->nir = nir;
+ctx->screen = screen;
+ctx->stage = nir->info.stage;
+ctx->is_blend = is_blend;
+ctx->alpha_ref = program->alpha_ref;
/* Start off with a safe cutoff, allowing usage of all 16 work
* registers. Later, we'll promote uniform reads to uniform registers
@@ -2823,6 +2818,7 @@ midgard_compile_shader_nir(struct midgard_screen *screen,
nir_shader *nir, midga
ctx->spills, ctx->fills);
}
+ralloc_free(ctx);
return 0;
}
--
2.23.0.rc1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] panfrost: Add madvise support to BO cache
Both patches look good to me.
Reviewed-by: Tomeu Vizoso
Thanks!
Tomeu
On Fri, 9 Aug 2019 at 21:53, Rob Herring wrote:
>
> The kernel now supports madvise ioctl to indicate which BOs can be freed
> when there is memory pressure. Mark BOs purgeable when they are in the
> BO cache. The BOs must also be munmapped when they are in the cache or
> they cannot be purged.
>
> We could optimize avoiding the madvise ioctl on older kernels once the
> driver version bump lands, but probably not worth it given the other
> driver features also being added.
>
> Signed-off-by: Rob Herring
> ---
> src/gallium/drivers/panfrost/pan_bo_cache.c | 21 +
> src/gallium/drivers/panfrost/pan_drm.c | 4 ++--
> 2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_bo_cache.c
> b/src/gallium/drivers/panfrost/pan_bo_cache.c
> index 7378d0a8abea..239ea0b46cb2 100644
> --- a/src/gallium/drivers/panfrost/pan_bo_cache.c
> +++ b/src/gallium/drivers/panfrost/pan_bo_cache.c
> @@ -23,6 +23,8 @@
> * Authors (Collabora):
> * Alyssa Rosenzweig
> */
> +#include
> +#include "drm-uapi/panfrost_drm.h"
>
> #include "pan_screen.h"
> #include "util/u_math.h"
> @@ -88,9 +90,21 @@ panfrost_bo_cache_fetch(
> list_for_each_entry_safe(struct panfrost_bo, entry, bucket, link) {
> if (entry->size >= size &&
> entry->flags == flags) {
> +int ret;
> +struct drm_panfrost_madvise madv;
> +
> /* This one works, splice it out of the cache */
> list_del(&entry->link);
>
> +madv.handle = entry->gem_handle;
> +madv.madv = PANFROST_MADV_WILLNEED;
> + madv.retained = 0;
> +
> +ret = drmIoctl(screen->fd,
> DRM_IOCTL_PANFROST_MADVISE, &madv);
> + if (!ret && !madv.retained) {
> + panfrost_drm_release_bo(screen, entry, false);
> + continue;
> + }
> /* Let's go! */
> return entry;
> }
> @@ -109,6 +123,13 @@ panfrost_bo_cache_put(
> struct panfrost_bo *bo)
> {
> struct list_head *bucket = pan_bucket(screen, bo->size);
> +struct drm_panfrost_madvise madv;
> +
> +madv.handle = bo->gem_handle;
> +madv.madv = PANFROST_MADV_DONTNEED;
> + madv.retained = 0;
> +
> +drmIoctl(screen->fd, DRM_IOCTL_PANFROST_MADVISE, &madv);
>
> /* Add us to the bucket */
> list_addtail(&bo->link, bucket);
> diff --git a/src/gallium/drivers/panfrost/pan_drm.c
> b/src/gallium/drivers/panfrost/pan_drm.c
> index 36a6b975680a..28a4287202bd 100644
> --- a/src/gallium/drivers/panfrost/pan_drm.c
> +++ b/src/gallium/drivers/panfrost/pan_drm.c
> @@ -163,6 +163,8 @@ panfrost_drm_release_bo(struct panfrost_screen *screen,
> struct panfrost_bo *bo,
> /* Rather than freeing the BO now, we'll cache the BO for later
> * allocations if we're allowed to */
>
> +panfrost_drm_munmap_bo(screen, bo);
> +
> if (cacheable) {
> bool cached = panfrost_bo_cache_put(screen, bo);
>
> @@ -172,8 +174,6 @@ panfrost_drm_release_bo(struct panfrost_screen *screen,
> struct panfrost_bo *bo,
>
> /* Otherwise, if the BO wasn't cached, we'll legitimately free the
> BO */
>
> -panfrost_drm_munmap_bo(screen, bo);
> -
> ret = drmIoctl(screen->fd, DRM_IOCTL_GEM_CLOSE, &gem_close);
> if (ret) {
> fprintf(stderr, "DRM_IOCTL_GEM_CLOSE failed: %m\n");
> --
> 2.20.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] Concern about proposed patch "egl/android: remove HAL_PIXEL_FORMAT_BGRA_8888 support"
OK, Actually android-x86 has a "workaround" at android side to enable BRGA here: http://git.osdn.net/view?p=android-x86/frameworks-native.git;a=blobdiff;f=opengl/libs/EGL/eglApi.cpp;h=01eeb6489dd1db98fde2e8c5cda2e0bd36e83f39;hp=94dfe6a9de4b7d3a0064b8cc60e8b856da1be5d3;hb=519828c5b649e5e83f18444666f0672ab7852518;hpb=792c8dc009bd3a0c44eb39e757a95e099c03b54c Not sure how difficult it is to push this to aosp android. On Thu, Aug 15, 2019 at 3:31 PM Mauro Rossi wrote: > > Hi Lepton, > > On Thu, Aug 15, 2019 at 9:58 PM Lepton Wu wrote: >> >> That's interesting. The android frame work will hard coded to use >> RGBA_ and set it as window format here: >> >> https://android.googlesource.com/platform/frameworks/native/+/refs/heads/master/opengl/libs/EGL/eglApi.cpp#738 >> >> Do you have a custom version of android which do different code here? > > > Affirmative, you may check [1] for reference > > [1] > http://git.osdn.net/view?p=android-x86/frameworks-native.git;a=commit;h=792c8dc009bd3a0c44eb39e757a95e099c03b54c > >> >> >> For other GPU, like intel, if we choose a EGL config with BGRA , >> finally we got a wrong color on screen. > > > android-x86 implementation selects BGRA_ only when RGBA_ not > available, > in a similar way to a pre-existing Workaround 10194508 that was removed from > AOSP code > > Removing HAL_PIXEL_FORMAT_BGRA_ support from egl/android > SurfaceFlinger will just crash with SIGABRT on nouveau > >> >> >> On Thu, Aug 15, 2019 at 12:49 PM Mauro Rossi wrote: >> > >> > Hi, >> > >> > sorry if I'm communicating in dedicated thread, but I don't know how to >> > reply to existing one. >> > >> > The proposed patch is based on the wrong assumption that all drivers used >> > with Android have RGBA_, which is not correct, for example nouveau >> > does not support RGBA on primary planes, infact we have a fallback to >> > simpler EGL query in android-x86 and we use BGRA_ (pixel format = 5) >> > >> > If patch does not solve a specific problem and it will cause a regression >> > at least in nouveau, it is recommended to abandon the patch >> > >> > Thanks for your feedbacks >> > >> > Mauro ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
