Hi Maxime,
While rebasing my cedrus-h264 pull request and running sparse over the
newly rebased code I found these sparse warnings:
SPARSE:drivers/staging/media/sunxi/cedrus/cedrus_h264.c drivers/staging/media/sunxi/cedrus/cedrus_h264.c:80:34: warning: incorrect type in assignment (different base types)
SPARSE:drivers/staging/media/sunxi/cedrus/cedrus_h264.c drivers/staging/media/sunxi/cedrus/cedrus_h264.c:81:37: warning: incorrect type in assignment (different base types)
SPARSE:drivers/staging/media/sunxi/cedrus/cedrus_h264.c drivers/staging/media/sunxi/cedrus/cedrus_h264.c:82:25: warning: incorrect type in assignment (different base types)
SPARSE:drivers/staging/media/sunxi/cedrus/cedrus_h264.c drivers/staging/media/sunxi/cedrus/cedrus_h264.c:84:23: warning: incorrect type in assignment (different base types)
SPARSE:drivers/staging/media/sunxi/cedrus/cedrus_h264.c drivers/staging/media/sunxi/cedrus/cedrus_h264.c:85:25: warning: incorrect type in assignment (different base types)
SPARSE:drivers/staging/media/sunxi/cedrus/cedrus_h264.c drivers/staging/media/sunxi/cedrus/cedrus_h264.c:86:29: warning: incorrect type in assignment (different base types)
SPARSE:drivers/staging/media/sunxi/cedrus/cedrus_h264.c drivers/staging/media/sunxi/cedrus/cedrus_h264.c:87:29: warning: incorrect type in assignment (different base types)
These warnings seem valid:
On 4/11/19 11:37 AM, Maxime Ripard wrote:
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> new file mode 100644
> index 000000000000..2c98a3e46d2b
> --- /dev/null
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> @@ -0,0 +1,574 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Cedrus VPU driver
> + *
> + * Copyright (c) 2013 Jens Kuske <jenskuske@xxxxxxxxx>
> + * Copyright (c) 2018 Bootlin
> + */
> +
> +#include <linux/types.h>
> +
> +#include <media/videobuf2-dma-contig.h>
> +
> +#include "cedrus.h"
> +#include "cedrus_hw.h"
> +#include "cedrus_regs.h"
> +
> +enum cedrus_h264_sram_off {
> + CEDRUS_SRAM_H264_PRED_WEIGHT_TABLE = 0x000,
> + CEDRUS_SRAM_H264_FRAMEBUFFER_LIST = 0x100,
> + CEDRUS_SRAM_H264_REF_LIST_0 = 0x190,
> + CEDRUS_SRAM_H264_REF_LIST_1 = 0x199,
> + CEDRUS_SRAM_H264_SCALING_LIST_8x8_0 = 0x200,
> + CEDRUS_SRAM_H264_SCALING_LIST_8x8_1 = 0x210,
> + CEDRUS_SRAM_H264_SCALING_LIST_4x4 = 0x220,
> +};
> +
> +struct cedrus_h264_sram_ref_pic {
> + __le32 top_field_order_cnt;
> + __le32 bottom_field_order_cnt;
> + __le32 frame_info;
> + __le32 luma_ptr;
> + __le32 chroma_ptr;
> + __le32 mv_col_top_ptr;
> + __le32 mv_col_bot_ptr;
> + __le32 reserved;
These types are __le32, but...
> +} __packed;
> +
> +#define CEDRUS_H264_FRAME_NUM 18
> +
> +#define CEDRUS_NEIGHBOR_INFO_BUF_SIZE (16 * SZ_1K)
> +#define CEDRUS_PIC_INFO_BUF_SIZE (128 * SZ_1K)
> +
> +static void cedrus_h264_write_sram(struct cedrus_dev *dev,
> + enum cedrus_h264_sram_off off,
> + const void *data, size_t len)
> +{
> + const u32 *buffer = data;
> + size_t count = DIV_ROUND_UP(len, 4);
> +
> + cedrus_write(dev, VE_AVC_SRAM_PORT_OFFSET, off << 2);
> +
> + while (count--)
> + cedrus_write(dev, VE_AVC_SRAM_PORT_DATA, *buffer++);
> +}
> +
> +static dma_addr_t cedrus_h264_mv_col_buf_addr(struct cedrus_ctx *ctx,
> + unsigned int position,
> + unsigned int field)
> +{
> + dma_addr_t addr = ctx->codec.h264.mv_col_buf_dma;
> +
> + /* Adjust for the position */
> + addr += position * ctx->codec.h264.mv_col_buf_field_size * 2;
> +
> + /* Adjust for the field */
> + addr += field * ctx->codec.h264.mv_col_buf_field_size;
> +
> + return addr;
> +}
> +
> +static void cedrus_fill_ref_pic(struct cedrus_ctx *ctx,
> + struct cedrus_buffer *buf,
> + unsigned int top_field_order_cnt,
> + unsigned int bottom_field_order_cnt,
> + struct cedrus_h264_sram_ref_pic *pic)
> +{
> + struct vb2_buffer *vbuf = &buf->m2m_buf.vb.vb2_buf;
> + unsigned int position = buf->codec.h264.position;
> +
> + pic->top_field_order_cnt = top_field_order_cnt;
> + pic->bottom_field_order_cnt = bottom_field_order_cnt;
> + pic->frame_info = buf->codec.h264.pic_type << 8;
> +
> + pic->luma_ptr = cedrus_buf_addr(vbuf, &ctx->dst_fmt, 0);
> + pic->chroma_ptr = cedrus_buf_addr(vbuf, &ctx->dst_fmt, 1);
> + pic->mv_col_top_ptr = cedrus_h264_mv_col_buf_addr(ctx, position, 0);
> + pic->mv_col_bot_ptr = cedrus_h264_mv_col_buf_addr(ctx, position, 1);
... here regular types are assigned to the pic fields.
I assume that cpu_to_le32() is needed here.
I'm not sure how I missed this when I made the initial pull request,
but I think it would make sense if you post a rebased and fixed v10.
Regards,
Hans
