[igt-dev] [PATCH i-g-t v2 1/2] lib/rendercopy_gen7: Reorganizing batch allocations

[Daniele Ceraolo Spurio]

On 16/04/18 08:22, Lukasz Kalamarz wrote:
> This lib was written in a different manner than all other libs,
> which was causing some issues during refactoring. Previous
> implementation was allocating data only in a state part of batchbuffer.
> New implementation take usage of splitting batch into two parts,
> one for batch commands and second for various states.
> 

This commit message is a bit unclear. There are no changes in how data 
is allocated/written as both the old and the new implementations split 
the batch; the difference is that the old style used 2 different 
pointers (ptr and state) to access the 2 parts at the same time while 
the new implementation re-uses the same pointer (ptr) and only access 
one section at a time.

Also, if I'm not missing anything after this patch there are no more 
users of batch->state (grep only returns results in null_state_gen, 
which has its own definition of intel_batchbuffer), so we can remove it 
from the structure.

> Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz at intel.com>
> Cc: Katarzyna Dec <katarzyna.dec at intel.com>
> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> ---

<snip>

> @@ -535,12 +531,27 @@ void gen7_render_copyfunc(struct intel_batchbuffer *batch,
>   			  unsigned width, unsigned height,
>   			  struct igt_buf *dst, unsigned dst_x, unsigned dst_y)
>   {
> +	uint32_t ps_binding_table, ps_sampler_off, ps_kernel_off;
> +	uint32_t blend_state, cc_viewport;
> +	uint32_t vertex_buffer;
>   	uint32_t batch_end;
>   
>   	intel_batchbuffer_flush_with_context(batch, context);
>   
> -	batch->state = &batch->buffer[BATCH_STATE_SPLIT];
> +	batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
>   
> +
> +	blend_state = gen7_create_blend_state(batch);
> +	cc_viewport = gen7_create_cc_viewport(batch);
> +	ps_sampler_off = gen7_create_sampler(batch);
> +	ps_kernel_off = batch_copy(batch, ps_kernel, sizeof(ps_kernel), 64);
> +	vertex_buffer = gen7_create_vertex_buffer(batch,
> +						  src_x, src_y,
> +						  dst_x, dst_y,
> +						  width, height);
> +	ps_binding_table = gen7_bind_surfaces(batch, src, dst);
> +

igt_assert(batch->ptr < &batch->buffer[4095]);

> +	batch->ptr = batch->buffer;
>   	OUT_BATCH(GEN7_PIPELINE_SELECT | PIPELINE_SELECT_3D);
>   
>   	gen7_emit_state_base_address(batch);
> @@ -556,18 +567,18 @@ void gen7_render_copyfunc(struct intel_batchbuffer *batch,
>   	gen7_emit_wm(batch);
>   	gen7_emit_streamout(batch);
>   	gen7_emit_null_depth_buffer(batch);
> -
> -	gen7_emit_cc(batch);
> -        gen7_emit_sampler(batch);
> +	gen7_emit_cc(batch, blend_state, cc_viewport);
> +	gen7_emit_sampler(batch, ps_sampler_off);
>           gen7_emit_sbe(batch);
> -        gen7_emit_ps(batch);
> +	gen7_emit_ps(batch, ps_kernel_off);
>           gen7_emit_vertex_elements(batch);
> -        gen7_emit_vertex_buffer(batch,
> -				src_x, src_y, dst_x, dst_y, width, height);
> -	gen7_emit_binding_table(batch, src, dst);
> +	gen7_emit_vertex_buffer(batch, src_x, src_y,
> +				dst_x, dst_y, width,
> +				height, vertex_buffer);
> +	gen7_emit_binding_table(batch, src, dst, ps_binding_table);
>   	gen7_emit_drawing_rectangle(batch, dst);
>   
> -        OUT_BATCH(GEN7_3DPRIMITIVE | (7- 2));
> +	OUT_BATCH(GEN7_3DPRIMITIVE | (7 - 2));
>           OUT_BATCH(GEN7_3DPRIMITIVE_VERTEX_SEQUENTIAL | _3DPRIM_RECTLIST);
>           OUT_BATCH(3);
>           OUT_BATCH(0);
> 

It might be worth adding a patch to change the spaces to tabs before 
this one to have cleaner result. Not a blocker since it isn't strictly 
related to this change.

Daniele