Re: [Mesa-dev] [PATCH] radeonsi: Reinitialize all descriptors in CE preamble.

2016-06-08 Thread Marek Olšák 
Reviewed-by: Marek Olšák 

Marek

On Mon, Jun 6, 2016 at 10:49 PM, Bas Nieuwenhuizen
 wrote:
> This fixes a problem with the CE preamble and restoring only stuff in the
> preamble when needed.
>
> To illustrate suppose we have two graphics IB's 1 and 2, which  are submitted 
> in
> that order. Furthermore suppose IB 1 does not use CE ram, but IB 2 does, and 
> we
> have a context switch at the start of IB 1, but not between IB 1 and IB 2.
>
> The old code put the CE RAM loads in the preamble of IB 2. As the preamble of
> IB 1 does not have the loads and the preamble of IB 2 does not get executed, 
> the
> old values are not load into CE RAM.
>
> Fix this by always restoring the entire CE RAM.
>
> v2: - Just load all descriptor set buffers instead of load and store the 
> entire
>   CE RAM.
> - Leave the ce_ram_dirty tracking in place for the non-preamble case.
>
> Signed-off-by: Bas Nieuwenhuizen 
> Cc: "12.0" 
> ---
>
> Replaces "radeonsi: Save and restore entire CE RAM."
>
>  src/gallium/drivers/radeonsi/si_descriptors.c | 15 +--
>  src/gallium/drivers/radeonsi/si_hw_context.c  |  3 +++
>  src/gallium/drivers/radeonsi/si_state.h   |  1 +
>  3 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
> b/src/gallium/drivers/radeonsi/si_descriptors.c
> index baddc5f..b32032e 100644
> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> @@ -160,7 +160,7 @@ static bool si_ce_upload(struct si_context *sctx, 
> unsigned ce_offset, unsigned s
> return true;
>  }
>
> -static void si_reinitialize_ce_ram(struct si_context *sctx,
> +static void si_ce_reinitialize_descriptors(struct si_context *sctx,
>  struct si_descriptors *desc)
>  {
> if (desc->buffer) {
> @@ -186,6 +186,17 @@ static void si_reinitialize_ce_ram(struct si_context 
> *sctx,
> desc->ce_ram_dirty = false;
>  }
>
> +void si_ce_reinitialize_all_descriptors(struct si_context *sctx)
> +{
> +   for (int i = 0; i < SI_NUM_SHADERS; i++) {
> +si_ce_reinitialize_descriptors(sctx, 
> >const_buffers[i].desc);
> +si_ce_reinitialize_descriptors(sctx, 
> >shader_buffers[i].desc);
> +si_ce_reinitialize_descriptors(sctx, 
> >samplers[i].views.desc);
> +si_ce_reinitialize_descriptors(sctx, >images[i].desc);
> +   }
> +si_ce_reinitialize_descriptors(sctx, >rw_buffers.desc);
> +}
> +
>  void si_ce_enable_loads(struct radeon_winsys_cs *ib)
>  {
> radeon_emit(ib, PKT3(PKT3_CONTEXT_CONTROL, 1, 0));
> @@ -207,7 +218,7 @@ static bool si_upload_descriptors(struct si_context *sctx,
> uint32_t const* list = (uint32_t const*)desc->list;
>
> if (desc->ce_ram_dirty)
> -   si_reinitialize_ce_ram(sctx, desc);
> +   si_ce_reinitialize_descriptors(sctx, desc);
>
> while(desc->dirty_mask) {
> int begin, count;
> diff --git a/src/gallium/drivers/radeonsi/si_hw_context.c 
> b/src/gallium/drivers/radeonsi/si_hw_context.c
> index fa6a2cb..d1b9851 100644
> --- a/src/gallium/drivers/radeonsi/si_hw_context.c
> +++ b/src/gallium/drivers/radeonsi/si_hw_context.c
> @@ -213,6 +213,9 @@ void si_begin_new_cs(struct si_context *ctx)
> else if (ctx->ce_ib)
> si_ce_enable_loads(ctx->ce_ib);
>
> +   if (ctx->ce_preamble_ib)
> +   si_ce_reinitialize_all_descriptors(ctx);
> +
> ctx->framebuffer.dirty_cbufs = (1 << 8) - 1;
> ctx->framebuffer.dirty_zsbuf = true;
> si_mark_atom_dirty(ctx, >framebuffer.atom);
> diff --git a/src/gallium/drivers/radeonsi/si_state.h 
> b/src/gallium/drivers/radeonsi/si_state.h
> index e5795eb..c1c3ca5 100644
> --- a/src/gallium/drivers/radeonsi/si_state.h
> +++ b/src/gallium/drivers/radeonsi/si_state.h
> @@ -250,6 +250,7 @@ struct si_buffer_resources {
> } while(0)
>
>  /* si_descriptors.c */
> +void si_ce_reinitialize_all_descriptors(struct si_context *sctx);
>  void si_ce_enable_loads(struct radeon_winsys_cs *ib);
>  void si_set_mutable_tex_desc_fields(struct r600_texture *tex,
> const struct radeon_surf_level 
> *base_level_info,
> --
> 2.8.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] radeonsi: Reinitialize all descriptors in CE preamble.

2016-06-06 Thread Bas Nieuwenhuizen 
This fixes a problem with the CE preamble and restoring only stuff in the
preamble when needed.

To illustrate suppose we have two graphics IB's 1 and 2, which  are submitted in
that order. Furthermore suppose IB 1 does not use CE ram, but IB 2 does, and we
have a context switch at the start of IB 1, but not between IB 1 and IB 2.

The old code put the CE RAM loads in the preamble of IB 2. As the preamble of
IB 1 does not have the loads and the preamble of IB 2 does not get executed, the
old values are not load into CE RAM.

Fix this by always restoring the entire CE RAM.

v2: - Just load all descriptor set buffers instead of load and store the entire
  CE RAM.
- Leave the ce_ram_dirty tracking in place for the non-preamble case.

Signed-off-by: Bas Nieuwenhuizen 
Cc: "12.0" 
---

Replaces "radeonsi: Save and restore entire CE RAM."

 src/gallium/drivers/radeonsi/si_descriptors.c | 15 +--
 src/gallium/drivers/radeonsi/si_hw_context.c  |  3 +++
 src/gallium/drivers/radeonsi/si_state.h   |  1 +
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c 
b/src/gallium/drivers/radeonsi/si_descriptors.c
index baddc5f..b32032e 100644
--- a/src/gallium/drivers/radeonsi/si_descriptors.c
+++ b/src/gallium/drivers/radeonsi/si_descriptors.c
@@ -160,7 +160,7 @@ static bool si_ce_upload(struct si_context *sctx, unsigned 
ce_offset, unsigned s
return true;
 }
 
-static void si_reinitialize_ce_ram(struct si_context *sctx,
+static void si_ce_reinitialize_descriptors(struct si_context *sctx,
 struct si_descriptors *desc)
 {
if (desc->buffer) {
@@ -186,6 +186,17 @@ static void si_reinitialize_ce_ram(struct si_context *sctx,
desc->ce_ram_dirty = false;
 }
 
+void si_ce_reinitialize_all_descriptors(struct si_context *sctx)
+{
+   for (int i = 0; i < SI_NUM_SHADERS; i++) {
+si_ce_reinitialize_descriptors(sctx, 
>const_buffers[i].desc);
+si_ce_reinitialize_descriptors(sctx, 
>shader_buffers[i].desc);
+si_ce_reinitialize_descriptors(sctx, 
>samplers[i].views.desc);
+si_ce_reinitialize_descriptors(sctx, >images[i].desc);
+   }
+si_ce_reinitialize_descriptors(sctx, >rw_buffers.desc);
+}
+
 void si_ce_enable_loads(struct radeon_winsys_cs *ib)
 {
radeon_emit(ib, PKT3(PKT3_CONTEXT_CONTROL, 1, 0));
@@ -207,7 +218,7 @@ static bool si_upload_descriptors(struct si_context *sctx,
uint32_t const* list = (uint32_t const*)desc->list;
 
if (desc->ce_ram_dirty)
-   si_reinitialize_ce_ram(sctx, desc);
+   si_ce_reinitialize_descriptors(sctx, desc);
 
while(desc->dirty_mask) {
int begin, count;
diff --git a/src/gallium/drivers/radeonsi/si_hw_context.c 
b/src/gallium/drivers/radeonsi/si_hw_context.c
index fa6a2cb..d1b9851 100644
--- a/src/gallium/drivers/radeonsi/si_hw_context.c
+++ b/src/gallium/drivers/radeonsi/si_hw_context.c
@@ -213,6 +213,9 @@ void si_begin_new_cs(struct si_context *ctx)
else if (ctx->ce_ib)
si_ce_enable_loads(ctx->ce_ib);
 
+   if (ctx->ce_preamble_ib)
+   si_ce_reinitialize_all_descriptors(ctx);
+
ctx->framebuffer.dirty_cbufs = (1 << 8) - 1;
ctx->framebuffer.dirty_zsbuf = true;
si_mark_atom_dirty(ctx, >framebuffer.atom);
diff --git a/src/gallium/drivers/radeonsi/si_state.h 
b/src/gallium/drivers/radeonsi/si_state.h
index e5795eb..c1c3ca5 100644
--- a/src/gallium/drivers/radeonsi/si_state.h
+++ b/src/gallium/drivers/radeonsi/si_state.h
@@ -250,6 +250,7 @@ struct si_buffer_resources {
} while(0)
 
 /* si_descriptors.c */
+void si_ce_reinitialize_all_descriptors(struct si_context *sctx);
 void si_ce_enable_loads(struct radeon_winsys_cs *ib);
 void si_set_mutable_tex_desc_fields(struct r600_texture *tex,
const struct radeon_surf_level 
*base_level_info,
-- 
2.8.3

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