[Mesa-dev] [PATCH] radeon/uvd: fix feedback buffer handling

[Michel Dänzer]

On Mon, 2014-02-03 at 11:33 +0100, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
> 
> Without the correct feedback buffer size UVD runs
> into an error on each frame, reducing the maximum FPS.
> 
> Signed-off-by: Christian König <christian.koenig at amd.com>

Some minor comments below, other than that

Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>


> diff --git a/src/gallium/drivers/radeon/radeon_uvd.c b/src/gallium/drivers/radeon/radeon_uvd.c
> index 95757e3..6ac2199 100644
> --- a/src/gallium/drivers/radeon/radeon_uvd.c
> +++ b/src/gallium/drivers/radeon/radeon_uvd.c
> @@ -132,15 +136,20 @@ static void send_cmd(struct ruvd_decoder *dec, unsigned cmd,
>  }
>  
>  /* map the next available message buffer */
> -static void map_msg_buf(struct ruvd_decoder *dec)
> +static void map_msg_fb_buf(struct ruvd_decoder *dec)

Maybe the function comment should be updated as well, e.g. to:

        /* map the next available message/feedback buffer */


> +	void *ptr;
>  
> -	/* grap the current message buffer */
> +	/* grap the current message/feedback buffer */

While you're at it, why not fix the spelling: 'grab'


> -	/* copy the message into it */
> -	dec->msg = dec->ws->buffer_map(buf->cs_handle, dec->cs, PIPE_TRANSFER_WRITE);
> +	/* and map it for CPU access */
> +	ptr = dec->ws->buffer_map(buf->cs_handle, dec->cs, PIPE_TRANSFER_WRITE);
> +
> +	/* calc buffer offsets */
> +	dec->msg = ptr;
> +	dec->fb = ptr + FB_BUFFER_OFFSET;
>  }

This is pointer arithmetic on a void* pointer, which is not defined by
the C standard and not supported by all compilers. Maybe make ptr a
char* instead, or just cast it to that for the assignment.


> @@ -898,7 +913,8 @@ struct pipe_video_codec *ruvd_create_decoder(struct pipe_context *context,
>  
>  	bs_buf_size = width * height * 512 / (16 * 16);
>  	for (i = 0; i < NUM_BUFFERS; ++i) {
> -		unsigned msg_fb_size = align(sizeof(struct ruvd_msg), 0x1000) + 0x1000;
> +		unsigned msg_fb_size = FB_BUFFER_OFFSET + FB_BUFFER_SIZE;
> +		assert(sizeof(struct ruvd_msg) <= FB_BUFFER_OFFSET);

This looks like it could be a STATIC_ASSERT.


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer