[Mesa-dev] [PATCH 3/7] radeonsi: add sampler view BOs to the BO list last

[Nicolai Hähnle]

On 13.09.2016 00:44, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> This might not be stricly needed, but better safe than sorry.

I think it's not needed, but it is a bit cleaner perhaps? If 
si_sampler_view_add_buffer ends up flushing, then the code in 
begin_new_cs would previously have added the buffer(s) for whatever was 
previously bound to that slot. Now it would add only the new buffer.

Patches 1-3:

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

> ---
>  src/gallium/drivers/radeonsi/si_descriptors.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c
> index b9fae79..b1a8594 100644
> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> @@ -403,24 +403,20 @@ static void si_set_sampler_view(struct si_context *sctx,
>  	struct si_sampler_view *rview = (struct si_sampler_view*)view;
>  	struct si_descriptors *descs = si_sampler_descriptors(sctx, shader);
>
>  	if (views->views[slot] == view && !disallow_early_out)
>  		return;
>
>  	if (view) {
>  		struct r600_texture *rtex = (struct r600_texture *)view->texture;
>  		uint32_t *desc = descs->list + slot * 16;
>
> -		si_sampler_view_add_buffer(sctx, view->texture,
> -					   RADEON_USAGE_READ,
> -					   rview->is_stencil_sampler, true);
> -
>  		pipe_sampler_view_reference(&views->views[slot], view);
>  		memcpy(desc, rview->state, 8*4);
>
>  		if (view->texture && view->texture->target != PIPE_BUFFER) {
>  			bool is_separate_stencil =
>  				rtex->db_compatible &&
>  				rview->is_stencil_sampler;
>
>  			si_set_mutable_tex_desc_fields(rtex,
>  						       rview->base_level_info,
> @@ -439,20 +435,26 @@ static void si_set_sampler_view(struct si_context *sctx,
>  			/* Disable FMASK and bind sampler state in [12:15]. */
>  			memcpy(desc + 8,
>  			       null_texture_descriptor, 4*4);
>
>  			if (views->sampler_states[slot])
>  				memcpy(desc + 12,
>  				       views->sampler_states[slot], 4*4);
>  		}
>
>  		views->enabled_mask |= 1u << slot;
> +
> +		/* Since this can flush, it must be done after enabled_mask is
> +		 * updated. */
> +		si_sampler_view_add_buffer(sctx, view->texture,
> +					   RADEON_USAGE_READ,
> +					   rview->is_stencil_sampler, true);
>  	} else {
>  		pipe_sampler_view_reference(&views->views[slot], NULL);
>  		memcpy(descs->list + slot*16, null_texture_descriptor, 8*4);
>  		/* Only clear the lower dwords of FMASK. */
>  		memcpy(descs->list + slot*16 + 8, null_texture_descriptor, 4*4);
>  		views->enabled_mask &= ~(1u << slot);
>  	}
>
>  	descs->dirty_mask |= 1u << slot;
>  	sctx->descriptors_dirty |= 1u << si_sampler_descriptors_idx(shader);
> @@ -620,23 +622,20 @@ static void si_set_shader_image(struct si_context *ctx,
>  	if (!view || !view->resource) {
>  		si_disable_shader_image(ctx, shader, slot);
>  		return;
>  	}
>
>  	res = (struct r600_resource *)view->resource;
>
>  	if (&images->views[slot] != view)
>  		util_copy_image_view(&images->views[slot], view);
>
> -	si_sampler_view_add_buffer(ctx, &res->b.b,
> -				   RADEON_USAGE_READWRITE, false, true);
> -
>  	if (res->b.b.target == PIPE_BUFFER) {
>  		if (view->access & PIPE_IMAGE_ACCESS_WRITE)
>  			si_mark_image_range_valid(view);
>
>  		si_make_buffer_descriptor(screen, res,
>  					  view->format,
>  					  view->u.buf.offset,
>  					  view->u.buf.size,
>  					  descs->list + slot * 8);
>  		images->compressed_colortex_mask &= ~(1 << slot);
> @@ -695,20 +694,24 @@ static void si_set_shader_image(struct si_context *ctx,
>  					   desc, NULL);
>  		si_set_mutable_tex_desc_fields(tex, &tex->surface.level[level],
>  					       level, level,
>  					       util_format_get_blockwidth(view->format),
>  					       false, desc);
>  	}
>
>  	images->enabled_mask |= 1u << slot;
>  	descs->dirty_mask |= 1u << slot;
>  	ctx->descriptors_dirty |= 1u << si_image_descriptors_idx(shader);
> +
> +	/* Since this can flush, it must be done after enabled_mask is updated. */
> +	si_sampler_view_add_buffer(ctx, &res->b.b,
> +				   RADEON_USAGE_READWRITE, false, true);
>  }
>
>  static void
>  si_set_shader_images(struct pipe_context *pipe,
>  		     enum pipe_shader_type shader,
>  		     unsigned start_slot, unsigned count,
>  		     const struct pipe_image_view *views)
>  {
>  	struct si_context *ctx = (struct si_context *)pipe;
>  	unsigned i, slot;
>