[Mesa-dev] [PATCH] radeonsi: Lazily re-set sampler views after disabling DCC

Nicolai Hähnle nhaehnle at gmail.com
Thu Mar 10 16:23:45 UTC 2016


On 09.03.2016 16:12, Bas Nieuwenhuizen wrote:
> Clear DCC flags if necessary when binding a new sampler_view. Also
> rebind all sampler views so that the sampler views that were already
> bound are also up to date.

Seems mostly reasonable to me and should cover all the cases.

I don't think rebinding the sampler views is really necessary: during 
DCC decompression, the DCC buffer should be cleared. This means that a 
currently bound sampler view will still result in correct data, just at 
a slightly increased bandwidth cost (because the DCC metadata is 
unnecessarily loaded). When the sampler view is bound the next time, 
that additional cost will go away. So you could simplify the patch a 
bit, which is probably beneficial in the long run. It is a minor quibble 
though.

One more comment below.

>
> Signed-off-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> ---
>   src/gallium/drivers/radeon/r600_texture.c     |  2 --
>   src/gallium/drivers/radeonsi/si_descriptors.c | 22 +++++++++++++++++++---
>   src/gallium/drivers/radeonsi/si_state.h       |  1 +
>   src/gallium/drivers/radeonsi/si_state_draw.c  |  1 +
>   4 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/src/gallium/drivers/radeon/r600_texture.c b/src/gallium/drivers/radeon/r600_texture.c
> index 1a8822c..07118fc 100644
> --- a/src/gallium/drivers/radeon/r600_texture.c
> +++ b/src/gallium/drivers/radeon/r600_texture.c
> @@ -307,14 +307,12 @@ static void r600_texture_disable_dcc(struct r600_common_screen *rscreen,
>   	/* Disable DCC. */
>   	rtex->dcc_offset = 0;
>   	rtex->cb_color_info &= ~VI_S_028C70_DCC_ENABLE(1);
>
>   	/* Notify all contexts about the change. */
>   	r600_dirty_all_framebuffer_states(rscreen);
> -
> -	/* TODO: re-set all sampler views and images, but how? */
>   }
>
>   static boolean r600_texture_get_handle(struct pipe_screen* screen,
>   				       struct pipe_resource *resource,
>   				       struct winsys_handle *whandle,
>                                          unsigned usage)
> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c
> index 37b9d68..5838e24 100644
> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> @@ -182,18 +182,22 @@ static void si_sampler_views_begin_new_cs(struct si_context *sctx,
>   }
>
>   static void si_set_sampler_view(struct si_context *sctx,
>   				struct si_sampler_views *views,
>   				unsigned slot, struct pipe_sampler_view *view)
>   {
> -	if (views->views[slot] == view)
> +	struct si_sampler_view *rview = (struct si_sampler_view*)view;
> +
> +	if (view && G_008F28_COMPRESSION_EN(rview->state[6]) &&
> +	    ((struct r600_texture*)rview->base.texture)->dcc_offset == 0) {
> +		rview->state[6] &= C_008F28_COMPRESSION_EN &
> +		                   C_008F28_ALPHA_IS_ON_MSB;
> +	} else if (views->views[slot] == view)
>   		return;
>
>   	if (view) {
> -		struct si_sampler_view *rview =
> -			(struct si_sampler_view*)view;
>   		struct r600_texture *rtex = (struct r600_texture *)view->texture;
>
>   		si_sampler_view_add_buffer(sctx, view->texture);
>
>   		pipe_sampler_view_reference(&views->views[slot], view);
>   		memcpy(views->desc.list + slot * 16, rview->state, 8*4);
> @@ -267,12 +271,24 @@ static void si_set_sampler_views(struct pipe_context *ctx,
>   			samplers->depth_texture_mask &= ~(1 << slot);
>   			samplers->compressed_colortex_mask &= ~(1 << slot);
>   		}
>   	}
>   }
>
> +void si_reset_sampler_views(struct si_context *sctx) {
> +	unsigned shader, sampler;
> +
> +	for (shader = 0; shader < SI_NUM_SHADERS; ++shader) {
> +		struct si_sampler_views *views = &sctx->samplers[shader].views;
> +		for (sampler = 0; sampler < SI_NUM_SAMPLERS; ++sampler) {
> +			si_set_sampler_view(sctx, views, sampler,
> +					    views->views[sampler]);
> +		}
> +	}
> +}

Please use a u_bit_scan loop over the enabled_mask like in other places.

Cheers,
Nicolai

> +
>   /* SAMPLER STATES */
>
>   static void si_bind_sampler_states(struct pipe_context *ctx, unsigned shader,
>                                      unsigned start, unsigned count, void **states)
>   {
>   	struct si_context *sctx = (struct si_context *)ctx;
> diff --git a/src/gallium/drivers/radeonsi/si_state.h b/src/gallium/drivers/radeonsi/si_state.h
> index fb16d0f..dab94e5 100644
> --- a/src/gallium/drivers/radeonsi/si_state.h
> +++ b/src/gallium/drivers/radeonsi/si_state.h
> @@ -243,12 +243,13 @@ void si_set_ring_buffer(struct pipe_context *ctx, uint shader, uint slot,
>   			bool add_tid, bool swizzle,
>   			unsigned element_size, unsigned index_stride, uint64_t offset);
>   void si_init_all_descriptors(struct si_context *sctx);
>   bool si_upload_shader_descriptors(struct si_context *sctx);
>   void si_release_all_descriptors(struct si_context *sctx);
>   void si_all_descriptors_begin_new_cs(struct si_context *sctx);
> +void si_reset_sampler_views(struct si_context *sctx);
>   void si_upload_const_buffer(struct si_context *sctx, struct r600_resource **rbuffer,
>   			    const uint8_t *ptr, unsigned size, uint32_t *const_offset);
>   void si_shader_change_notify(struct si_context *sctx);
>   void si_emit_shader_userdata(struct si_context *sctx, struct r600_atom *atom);
>
>   /* si_state.c */
> diff --git a/src/gallium/drivers/radeonsi/si_state_draw.c b/src/gallium/drivers/radeonsi/si_state_draw.c
> index 5d094c7..585148d 100644
> --- a/src/gallium/drivers/radeonsi/si_state_draw.c
> +++ b/src/gallium/drivers/radeonsi/si_state_draw.c
> @@ -787,12 +787,13 @@ void si_draw_vbo(struct pipe_context *ctx, const struct pipe_draw_info *info)
>   	if (dirty_fb_counter != sctx->b.last_dirty_fb_counter) {
>   		sctx->b.last_dirty_fb_counter = dirty_fb_counter;
>   		sctx->framebuffer.dirty_cbufs |=
>   			((1 << sctx->framebuffer.state.nr_cbufs) - 1);
>   		sctx->framebuffer.dirty_zsbuf = true;
>   		si_mark_atom_dirty(sctx, &sctx->framebuffer.atom);
> +		si_reset_sampler_views(sctx);
>   	}
>
>   	si_decompress_textures(sctx);
>
>   	/* Set the rasterization primitive type.
>   	 *
>


More information about the mesa-dev mailing list