[Mesa-dev] [PATCH] radv: fix begin/end transform feedback with 0 counter buffers.

[Samuel Pitoiset]

On 11/1/18 12:59 AM, Dave Airlie wrote:
> From: Dave Airlie <airlied at redhat.com>
> 
> If the user gives 0 counterBuffers then the driver should still
> enable transform feedback on all targets. This changes the
> driver to always enable xfb, and use counter buffers where
> one is defined for the target in question.
> 
> Fixes: b4eb029062 (radv: implement VK_EXT_transform_feedback)
> ---
>   src/amd/vulkan/radv_cmd_buffer.c | 30 ++++++++++++++++--------------
>   1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/src/amd/vulkan/radv_cmd_buffer.c b/src/amd/vulkan/radv_cmd_buffer.c
> index 77680268897..ac72a25d272 100644
> --- a/src/amd/vulkan/radv_cmd_buffer.c
> +++ b/src/amd/vulkan/radv_cmd_buffer.c
> @@ -4743,13 +4743,14 @@ void radv_CmdBeginTransformFeedbackEXT(
>   	struct radv_streamout_binding *sb = cmd_buffer->streamout_bindings;
>   	struct radv_streamout_state *so = &cmd_buffer->state.streamout;
>   	struct radeon_cmdbuf *cs = cmd_buffer->cs;
> -
> +	uint32_t i;

Please, add a new line here (and in End()).

With that, patch is:

Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

>   	radv_flush_vgt_streamout(cmd_buffer);
>   
>   	assert(firstCounterBuffer + counterBufferCount <= MAX_SO_BUFFERS);
> -	for (uint32_t i = firstCounterBuffer; i < counterBufferCount; i++) {
> -		if (!(so->enabled_mask & (1 << i)))
> -			continue;
> +	for_each_bit(i, so->enabled_mask) {
> +		int32_t counter_buffer_idx = i - firstCounterBuffer;
> +		if (counter_buffer_idx >= 0 && counter_buffer_idx > counterBufferCount)
> +			counter_buffer_idx = -1;
>   
>   		/* SI binds streamout buffers as shader resources.
>   		 * VGT only counts primitives and tells the shader through
> @@ -4759,12 +4760,12 @@ void radv_CmdBeginTransformFeedbackEXT(
>   		radeon_emit(cs, (sb[i].offset + sb[i].size) >> 2);	/* BUFFER_SIZE (in DW) */
>   		radeon_emit(cs, so->stride_in_dw[i]);			/* VTX_STRIDE (in DW) */
>   
> -		if (pCounterBuffers && pCounterBuffers[i]) {
> +		if (counter_buffer_idx >= 0 && pCounterBuffers && pCounterBuffers[counter_buffer_idx]) {
>   			/* The array of counter buffers is optional. */
> -			RADV_FROM_HANDLE(radv_buffer, buffer, pCounterBuffers[i]);
> +			RADV_FROM_HANDLE(radv_buffer, buffer, pCounterBuffers[counter_buffer_idx]);
>   			uint64_t va = radv_buffer_get_va(buffer->bo);
>   
> -			va += buffer->offset + pCounterBufferOffsets[i];
> +			va += buffer->offset + pCounterBufferOffsets[counter_buffer_idx];
>   
>   			/* Append */
>   			radeon_emit(cs, PKT3(PKT3_STRMOUT_BUFFER_UPDATE, 4, 0));
> @@ -4803,20 +4804,21 @@ void radv_CmdEndTransformFeedbackEXT(
>   	RADV_FROM_HANDLE(radv_cmd_buffer, cmd_buffer, commandBuffer);
>   	struct radv_streamout_state *so = &cmd_buffer->state.streamout;
>   	struct radeon_cmdbuf *cs = cmd_buffer->cs;
> -
> +	uint32_t i;
>   	radv_flush_vgt_streamout(cmd_buffer);
>   
>   	assert(firstCounterBuffer + counterBufferCount <= MAX_SO_BUFFERS);
> -	for (uint32_t i = firstCounterBuffer; i < counterBufferCount; i++) {
> -		if (!(so->enabled_mask & (1 << i)))
> -			continue;
> +	for_each_bit(i, so->enabled_mask) {
> +		int32_t counter_buffer_idx = i - firstCounterBuffer;
> +		if (counter_buffer_idx >= 0 && counter_buffer_idx > counterBufferCount)
> +			counter_buffer_idx = -1;
>   
> -		if (pCounterBuffers && pCounterBuffers[i]) {
> +		if (counter_buffer_idx >= 0 && pCounterBuffers && pCounterBuffers[counter_buffer_idx]) {
>   			/* The array of counters buffer is optional. */
> -			RADV_FROM_HANDLE(radv_buffer, buffer, pCounterBuffers[i]);
> +			RADV_FROM_HANDLE(radv_buffer, buffer, pCounterBuffers[counter_buffer_idx]);
>   			uint64_t va = radv_buffer_get_va(buffer->bo);
>   
> -			va += buffer->offset + pCounterBufferOffsets[i];
> +			va += buffer->offset + pCounterBufferOffsets[counter_buffer_idx];
>   
>   			radeon_emit(cs, PKT3(PKT3_STRMOUT_BUFFER_UPDATE, 4, 0));
>   			radeon_emit(cs, STRMOUT_SELECT_BUFFER(i) |
>