On Mon, 2014-02-03 at 11:33 +0100, Christian König wrote:
> From: Christian König <christian.koe...@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.koe...@amd.com>
Some minor comments below, other than that
Reviewed-by: Michel Dänzer <michel.daen...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
