Re: [PATCH v4 2/2] media: cedrus: Add H264 decoding support
From: Jernej Åkrabec
Date: Thu Feb 21 2019 - 13:21:58 EST
Hi,
Dne sreda, 20. februar 2019 ob 18:50:54 CET je Jernej Åkrabec napisal(a):
> Hi!
>
> I really wanted to do another review on previous series but got distracted
> by analyzing one particulary troublesome H264 sample. It still doesn't work
> correctly, so I would ask you if you can test it with your stack (it might
> be userspace issue):
>
> http://jernej.libreelec.tv/videos/problematic/test.mkv
>
> Please take a look at my comments below.
>
> Dne sreda, 20. februar 2019 ob 15:17:34 CET je Maxime Ripard napisal(a):
> > Introduce some basic H264 decoding support in cedrus. So far, only the
> > baseline profile videos have been tested, and some more advanced features
> > used in higher profiles are not even implemented.
>
> What is not yet implemented? Multi slice frame decoding, interlaced frames
> and decoding frames with width > 2048. Anything else?
>
> > Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxx>
> > ---
> >
> > drivers/staging/media/sunxi/cedrus/Makefile | 3 +-
> > drivers/staging/media/sunxi/cedrus/cedrus.c | 30 +-
> > drivers/staging/media/sunxi/cedrus/cedrus.h | 38 +-
> > drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 13 +-
> > drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 584 +++++++++++++++-
> > drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 4 +-
> > drivers/staging/media/sunxi/cedrus/cedrus_regs.h | 91 ++-
> > drivers/staging/media/sunxi/cedrus/cedrus_video.c | 9 +-
> > 8 files changed, 770 insertions(+), 2 deletions(-)
> > create mode 100644 drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/Makefile
> > b/drivers/staging/media/sunxi/cedrus/Makefile index
> > e9dc68b7bcb6..aaf141fc58b6 100644
> > --- a/drivers/staging/media/sunxi/cedrus/Makefile
> > +++ b/drivers/staging/media/sunxi/cedrus/Makefile
> > @@ -1,3 +1,4 @@
> >
> > obj-$(CONFIG_VIDEO_SUNXI_CEDRUS) += sunxi-cedrus.o
> >
> > -sunxi-cedrus-y = cedrus.o cedrus_video.o cedrus_hw.o cedrus_dec.o
> > cedrus_mpeg2.o +sunxi-cedrus-y = cedrus.o cedrus_video.o cedrus_hw.o
> > cedrus_dec.o \ + cedrus_mpeg2.o cedrus_h264.o
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus.c index
> > ff11cbeba205..c1607142d998 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> > @@ -40,6 +40,35 @@ static const struct cedrus_control cedrus_controls[] =
> > {
> >
> > .codec = CEDRUS_CODEC_MPEG2,
> > .required = false,
> >
> > },
> >
> > + {
> > + .id =
>
> V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS,
>
> > + .elem_size = sizeof(struct
>
> v4l2_ctrl_h264_decode_param),
>
> > + .codec = CEDRUS_CODEC_H264,
> > + .required = true,
> > + },
> > + {
> > + .id =
>
> V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS,
>
> > + .elem_size = sizeof(struct
v4l2_ctrl_h264_slice_param),
> > + .codec = CEDRUS_CODEC_H264,
> > + .required = true,
> > + },
> > + {
> > + .id = V4L2_CID_MPEG_VIDEO_H264_SPS,
> > + .elem_size = sizeof(struct v4l2_ctrl_h264_sps),
> > + .codec = CEDRUS_CODEC_H264,
> > + .required = true,
> > + },
> > + {
> > + .id = V4L2_CID_MPEG_VIDEO_H264_PPS,
> > + .elem_size = sizeof(struct v4l2_ctrl_h264_pps),
> > + .codec = CEDRUS_CODEC_H264,
> > + .required = true,
> > + },
> > + {
> > + .id =
>
> V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX,
>
> > + .elem_size = sizeof(struct
>
> v4l2_ctrl_h264_scaling_matrix),
>
> > + .codec = CEDRUS_CODEC_H264,
> > + },
> >
> > };
> >
> > #define CEDRUS_CONTROLS_COUNT ARRAY_SIZE(cedrus_controls)
> >
> > @@ -278,6 +307,7 @@ static int cedrus_probe(struct platform_device *pdev)
> >
> > }
> >
> > dev->dec_ops[CEDRUS_CODEC_MPEG2] = &cedrus_dec_ops_mpeg2;
> >
> > + dev->dec_ops[CEDRUS_CODEC_H264] = &cedrus_dec_ops_h264;
> >
> > mutex_init(&dev->dev_mutex);
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > b/drivers/staging/media/sunxi/cedrus/cedrus.h index
> > 4aedd24a9848..8c64f9a27e9d 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> > @@ -30,7 +30,7 @@
> >
> > enum cedrus_codec {
> >
> > CEDRUS_CODEC_MPEG2,
> >
> > -
> > + CEDRUS_CODEC_H264,
> >
> > CEDRUS_CODEC_LAST,
> >
> > };
> >
> > @@ -40,6 +40,12 @@ enum cedrus_irq_status {
> >
> > CEDRUS_IRQ_OK,
> >
> > };
> >
> > +enum cedrus_h264_pic_type {
> > + CEDRUS_H264_PIC_TYPE_FRAME = 0,
> > + CEDRUS_H264_PIC_TYPE_FIELD,
> > + CEDRUS_H264_PIC_TYPE_MBAFF,
> > +};
> > +
> >
> > struct cedrus_control {
> >
> > u32 id;
> > u32 elem_size;
> >
> > @@ -47,6 +53,14 @@ struct cedrus_control {
> >
> > unsigned char required:1;
> >
> > };
> >
> > +struct cedrus_h264_run {
> > + const struct v4l2_ctrl_h264_decode_param *decode_param;
> > + const struct v4l2_ctrl_h264_pps *pps;
> > + const struct v4l2_ctrl_h264_scaling_matrix
*scaling_matrix;
> > + const struct v4l2_ctrl_h264_slice_param *slice_param;
> > + const struct v4l2_ctrl_h264_sps *sps;
> > +};
> > +
> >
> > struct cedrus_mpeg2_run {
> >
> > const struct v4l2_ctrl_mpeg2_slice_params *slice_params;
> > const struct v4l2_ctrl_mpeg2_quantization *quantization;
> >
> > @@ -57,12 +71,20 @@ struct cedrus_run {
> >
> > struct vb2_v4l2_buffer *dst;
> >
> > union {
> >
> > + struct cedrus_h264_run h264;
> >
> > struct cedrus_mpeg2_run mpeg2;
> >
> > };
> >
> > };
> >
> > struct cedrus_buffer {
> >
> > struct v4l2_m2m_buffer m2m_buf;
> >
> > +
> > + union {
> > + struct {
> > + unsigned int position;
> > + enum cedrus_h264_pic_type pic_type;
> > + } h264;
> > + } codec;
> >
> > };
> >
> > struct cedrus_ctx {
> >
> > @@ -77,6 +99,19 @@ struct cedrus_ctx {
> >
> > struct v4l2_ctrl **ctrls;
> >
> > struct vb2_buffer *dst_bufs[VIDEO_MAX_FRAME];
> >
> > +
> > + union {
> > + struct {
> > + void *mv_col_buf;
> > + dma_addr_t mv_col_buf_dma;
> > + ssize_t mv_col_buf_field_size;
> > + ssize_t mv_col_buf_size;
> > + void *pic_info_buf;
> > + dma_addr_t pic_info_buf_dma;
> > + void *neighbor_info_buf;
> > + dma_addr_t neighbor_info_buf_dma;
> > + } h264;
> > + } codec;
> >
> > };
> >
> > struct cedrus_dec_ops {
> >
> > @@ -118,6 +153,7 @@ struct cedrus_dev {
> >
> > };
> >
> > extern struct cedrus_dec_ops cedrus_dec_ops_mpeg2;
> >
> > +extern struct cedrus_dec_ops cedrus_dec_ops_h264;
> >
> > static inline void cedrus_write(struct cedrus_dev *dev, u32 reg, u32 val)
> > {
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index
> > 4d6d602cdde6..a290ae1b8f4d 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> > @@ -46,6 +46,19 @@ void cedrus_device_run(void *priv)
> >
> > V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION);
> >
> > break;
> >
> > + case V4L2_PIX_FMT_H264_SLICE:
> > + run.h264.decode_param = cedrus_find_control_data(ctx,
> > + V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAMS);
> > + run.h264.pps = cedrus_find_control_data(ctx,
> > + V4L2_CID_MPEG_VIDEO_H264_PPS);
> > + run.h264.scaling_matrix = cedrus_find_control_data(ctx,
> > + V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX);
> > + run.h264.slice_param = cedrus_find_control_data(ctx,
> > + V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAMS);
> > + run.h264.sps = cedrus_find_control_data(ctx,
> > + V4L2_CID_MPEG_VIDEO_H264_SPS);
> > + break;
> > +
> >
> > default:
> > break;
> >
> > }
> >
> > 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..51e5f57120a2
> > --- /dev/null
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> > @@ -0,0 +1,584 @@
> > +// 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;
> > +} __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);
> > +
> > + do {
> > + cedrus_write(dev, VE_AVC_SRAM_PORT_DATA, *buffer++);
> > + } while (--count);
>
> Above loop will still write one word for count = 0. I propose following:
>
> 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);
>
> > +}
> > +
> > +static void cedrus_write_frame_list(struct cedrus_ctx *ctx,
> > + struct cedrus_run *run)
> > +{
> > + struct cedrus_h264_sram_ref_pic pic_list[CEDRUS_H264_FRAME_NUM];
> > + const struct v4l2_ctrl_h264_decode_param *dec_param =
> > run->h264.decode_param; + const struct v4l2_ctrl_h264_slice_param
>
> *slice =
>
> > run->h264.slice_param; + const struct v4l2_ctrl_h264_sps *sps =
> > run->h264.sps;
> > + const struct vb2_buffer *dst_buf = &run->dst->vb2_buf;
> > + struct vb2_queue *cap_q = &ctx->fh.m2m_ctx->cap_q_ctx.q;
> > + struct cedrus_buffer *output_buf;
> > + struct cedrus_dev *dev = ctx->dev;
> > + unsigned long used_dpbs = 0;
> > + unsigned int position;
> > + unsigned int output = 0;
> > + unsigned int i;
> > +
> > + memset(pic_list, 0, sizeof(pic_list));
> > +
> > + for (i = 0; i < ARRAY_SIZE(dec_param->dpb); i++) {
> > + const struct v4l2_h264_dpb_entry *dpb = &dec_param-
> >
> >dpb[i];
> >
> > + struct cedrus_buffer *cedrus_buf;
> > + int buf_idx;
> > +
> > + if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_VALID))
> > + continue;
> > +
> > + buf_idx = vb2_find_timestamp(cap_q, dpb->timestamp, 0);
> > + if (buf_idx < 0)
> > + continue;
> > +
> > + cedrus_buf = vb2_to_cedrus_buffer(ctx-
> >
> >dst_bufs[buf_idx]);
> >
> > + position = cedrus_buf->codec.h264.position;
> > + used_dpbs |= BIT(position);
> > +
> > + if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
> > + continue;
> > +
> > + cedrus_fill_ref_pic(ctx, cedrus_buf,
> > + dpb->top_field_order_cnt,
> > + dpb->bottom_field_order_cnt,
> > + &pic_list[position]);
> > +
> > + output = max(position, output);
> > + }
> > +
> > + position = find_next_zero_bit(&used_dpbs, CEDRUS_H264_FRAME_NUM,
> > + output);
> > + if (position >= CEDRUS_H264_FRAME_NUM)
> > + position = find_first_zero_bit(&used_dpbs,
>
> CEDRUS_H264_FRAME_NUM);
>
> I guess you didn't try any interlaced videos? Sometimes it happens that
> buffer is reference and output at the same time. In such cases, above code
> would make two entries, which doesn't work based on Kwiboo's and my
> experiments.
>
> I guess decoding interlaced videos is out of scope at this time?
>
> > +
> > + output_buf = vb2_to_cedrus_buffer(&run->dst->vb2_buf);
> > + output_buf->codec.h264.position = position;
> > +
> > + if (slice->flags & V4L2_H264_SLICE_FLAG_FIELD_PIC)
> > + output_buf->codec.h264.pic_type =
>
> CEDRUS_H264_PIC_TYPE_FIELD;
>
> > + else if (sps->flags & V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD)
> > + output_buf->codec.h264.pic_type =
>
> CEDRUS_H264_PIC_TYPE_MBAFF;
>
> > + else
> > + output_buf->codec.h264.pic_type =
>
> CEDRUS_H264_PIC_TYPE_FRAME;
>
> > +
> > + cedrus_fill_ref_pic(ctx, output_buf,
> > + dec_param->top_field_order_cnt,
> > + dec_param->bottom_field_order_cnt,
> > + &pic_list[position]);
> > +
> > + cedrus_h264_write_sram(dev, CEDRUS_SRAM_H264_FRAMEBUFFER_LIST,
> > + pic_list, sizeof(pic_list));
> > +
> > + cedrus_write(dev, VE_H264_OUTPUT_FRAME_IDX, position);
> > +}
> > +
> > +#define CEDRUS_MAX_REF_IDX 32
> > +
> > +static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
> > + struct cedrus_run *run,
> > + const u8 *ref_list, u8
num_ref,
> > + enum cedrus_h264_sram_off sram)
> > +{
> > + const struct v4l2_ctrl_h264_decode_param *decode = run-
> >
> >h264.decode_param;
> >
> > + struct vb2_queue *cap_q = &ctx->fh.m2m_ctx->cap_q_ctx.q;
> > + const struct vb2_buffer *dst_buf = &run->dst->vb2_buf;
> > + struct cedrus_dev *dev = ctx->dev;
> > + u8 sram_array[CEDRUS_MAX_REF_IDX];
> > + unsigned int i;
> > + size_t size;
> > +
> > + memset(sram_array, 0, sizeof(sram_array));
> > +
> > + for (i = 0; i < num_ref; i++) {
> > + const struct v4l2_h264_dpb_entry *dpb;
> > + const struct cedrus_buffer *cedrus_buf;
> > + const struct vb2_v4l2_buffer *ref_buf;
> > + unsigned int position;
> > + int buf_idx;
> > + u8 dpb_idx;
> > +
> > + dpb_idx = ref_list[i];
> > + dpb = &decode->dpb[dpb_idx];
> > +
> > + if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
> > + continue;
> > +
> > + buf_idx = vb2_find_timestamp(cap_q, dpb->timestamp, 0);
> > + if (buf_idx < 0)
> > + continue;
> > +
> > + ref_buf = to_vb2_v4l2_buffer(ctx->dst_bufs[buf_idx]);
> > + cedrus_buf = vb2_v4l2_to_cedrus_buffer(ref_buf);
> > + position = cedrus_buf->codec.h264.position;
> > +
> > + sram_array[i] |= position << 1;
> > + if (ref_buf->field == V4L2_FIELD_BOTTOM)
>
> I'm still not convinced that checking buffer field is appropriate solution
> here. IMO this bit defines top or bottom reference and same buffer could be
> used for both.
>
> But I guess this belongs for follow up patch which will fix decoding
> interlaced videos.
>
> > + sram_array[i] |= BIT(0);
> > + }
> > +
> > + size = min_t(size_t, ALIGN(num_ref, 4), sizeof(sram_array));
> > + cedrus_h264_write_sram(dev, sram, &sram_array, size);
> > +}
> > +
> > +static void cedrus_write_ref_list0(struct cedrus_ctx *ctx,
> > + struct cedrus_run *run)
> > +{
> > + const struct v4l2_ctrl_h264_slice_param *slice = run-
> >
> >h264.slice_param;
> >
> > +
> > + _cedrus_write_ref_list(ctx, run,
> > + slice->ref_pic_list0,
> > + slice->num_ref_idx_l0_active_minus1 +
>
> 1,
>
> > + CEDRUS_SRAM_H264_REF_LIST_0);
> > +}
> > +
> > +static void cedrus_write_ref_list1(struct cedrus_ctx *ctx,
> > + struct cedrus_run *run)
> > +{
> > + const struct v4l2_ctrl_h264_slice_param *slice = run-
> >
> >h264.slice_param;
> >
> > +
> > + _cedrus_write_ref_list(ctx, run,
> > + slice->ref_pic_list1,
> > + slice->num_ref_idx_l1_active_minus1 +
>
> 1,
>
> > + CEDRUS_SRAM_H264_REF_LIST_1);
> > +}
> > +
> > +static void cedrus_write_scaling_lists(struct cedrus_ctx *ctx,
> > + struct cedrus_run *run)
> > +{
> > + const struct v4l2_ctrl_h264_scaling_matrix *scaling =
> > + run->h264.scaling_matrix;
> > + struct cedrus_dev *dev = ctx->dev;
> > +
> > + if (!scaling)
> > + return;
> > +
> > + cedrus_h264_write_sram(dev, CEDRUS_SRAM_H264_SCALING_LIST_8x8_0,
> > + scaling->scaling_list_8x8[0],
> > + sizeof(scaling-
>scaling_list_8x8[0]));
> > +
> > + cedrus_h264_write_sram(dev, CEDRUS_SRAM_H264_SCALING_LIST_8x8_1,
> > + scaling->scaling_list_8x8[1],
> > + sizeof(scaling-
>scaling_list_8x8[1]));
>
> Index above should be 3. IIRC 1 and 3 are used by 4:2:0 chroma subsampling,
> but currently I'm unable to find reference to that in standard.
I actually meant index 0 and 3. While I still can't pinpoint exact chapter in
specs, I found comment in ffmpeg what each index represents:
0 - Intra, Y
1 - Intra, Cr
2 - Intra, Cb
3 - Inter, Y
4 - Inter, Cr
5 - Inter, Cb
So 0 and 3 makes sense.
>
> > +
> > + cedrus_h264_write_sram(dev, CEDRUS_SRAM_H264_SCALING_LIST_4x4,
> > + scaling->scaling_list_4x4,
> > + sizeof(scaling->scaling_list_4x4));
> > +}
> > +
> > +static void cedrus_write_pred_weight_table(struct cedrus_ctx *ctx,
> > + struct cedrus_run
>
> *run)
>
> > +{
> > + const struct v4l2_ctrl_h264_slice_param *slice =
> > + run->h264.slice_param;
> > + const struct v4l2_h264_pred_weight_table *pred_weight =
> > + &slice->pred_weight_table;
> > + struct cedrus_dev *dev = ctx->dev;
> > + int i, j, k;
> > +
> > + cedrus_write(dev, VE_H264_SHS_WP,
> > + ((pred_weight->chroma_log2_weight_denom & 0xf) <<
>
> 4) |
>
> > + ((pred_weight->luma_log2_weight_denom & 0xf) <<
>
> 0));
>
> Denominators are only in range of 0-7, so mask should be 0x7. CedarX code
> also specify those two fields 3 bits wide.
>
> > +
> > + cedrus_write(dev, VE_AVC_SRAM_PORT_OFFSET,
> > + CEDRUS_SRAM_H264_PRED_WEIGHT_TABLE << 2);
> > +
> > + for (i = 0; i < ARRAY_SIZE(pred_weight->weight_factors); i++) {
> > + const struct v4l2_h264_weight_factors *factors =
> > + &pred_weight->weight_factors[i];
> > +
> > + for (j = 0; j < ARRAY_SIZE(factors->luma_weight); j++)
{
> > + u32 val;
> > +
> > + val = ((factors->luma_offset[j] & 0x1ff) <<
16)
> >
> > + (factors->luma_weight[j] & 0x1ff);
> > + cedrus_write(dev, VE_AVC_SRAM_PORT_DATA,
>
> val);
>
> You should cast offset varible to wider type. Currently some videos which
> use prediction weight table don't work for me, unless offset is casted to
> u32 first. Shifting 8 bit variable for 16 places gives you 0 every time.
>
> Luma offset and weight are defined as s8, so having wider mask doesn't
> really make sense. However, I think weight should be s16 anyway, because
> standard says that it's value could be 2^denominator for default value or
> in range -128..127. Worst case would be 2^7 = 128 and -128. To cover both
> values you need at least 9 bits.
>
> I guess there is a way to detect when default values need to be written (if
> at all) and that can be handled separately. But I don't see any reason why
> bigger type can't be used for just in case. Offset being s8 is fine and you
> can drop mask for it.
>
> Everything applies for chroma below too.
>
> > + }
> > +
> > + for (j = 0; j < ARRAY_SIZE(factors->chroma_weight); j+
+)
>
> {
>
> > + for (k = 0; k < ARRAY_SIZE(factors-
> >
> >chroma_weight[0]); k++) {
> >
> > + u32 val;
> > +
> > + val = ((factors->chroma_offset[j]
>
> [k] & 0x1ff) << 16) |
>
> > + (factors-
> >
> >chroma_weight[j][k] & 0x1ff);
> >
> > + cedrus_write(dev,
>
> VE_AVC_SRAM_PORT_DATA, val);
>
> > + }
> > + }
> > + }
> > +}
> > +
> > +static void cedrus_set_params(struct cedrus_ctx *ctx,
> > + struct cedrus_run *run)
> > +{
> > + const struct v4l2_ctrl_h264_scaling_matrix *scaling =
> > run->h264.scaling_matrix; + const struct v4l2_ctrl_h264_decode_param
> > *decode = run->h264.decode_param; + const struct
>
> v4l2_ctrl_h264_slice_param
>
> > *slice = run->h264.slice_param; + const struct v4l2_ctrl_h264_pps *pps =
> > run->h264.pps;
> > + const struct v4l2_ctrl_h264_sps *sps = run->h264.sps;
> > + struct vb2_buffer *src_buf = &run->src->vb2_buf;
> > + struct cedrus_dev *dev = ctx->dev;
> > + dma_addr_t src_buf_addr;
> > + u32 offset = slice->header_bit_size;
> > + u32 len = (slice->size * 8) - offset;
> > + u32 reg;
> > +
> > + cedrus_write(dev, VE_H264_VLD_LEN, len);
> > + cedrus_write(dev, VE_H264_VLD_OFFSET, offset);
> > +
> > + src_buf_addr = vb2_dma_contig_plane_dma_addr(src_buf, 0);
> > + cedrus_write(dev, VE_H264_VLD_END,
> > + src_buf_addr + vb2_get_plane_payload(src_buf, 0));
> > + cedrus_write(dev, VE_H264_VLD_ADDR,
> > + VE_H264_VLD_ADDR_VAL(src_buf_addr) |
> > + VE_H264_VLD_ADDR_FIRST | VE_H264_VLD_ADDR_VALID |
> > + VE_H264_VLD_ADDR_LAST);
> > +
> > + /*
> > + * FIXME: Since the bitstream parsing is done in software, and
> > + * in userspace, this shouldn't be needed anymore. But it
> > + * turns out that removing it breaks the decoding process,
> > + * without any clear indication why.
> > + */
> > + cedrus_write(dev, VE_H264_TRIGGER_TYPE,
> > + VE_H264_TRIGGER_TYPE_INIT_SWDEC);
> > +
> > + if (((pps->flags & V4L2_H264_PPS_FLAG_WEIGHTED_PRED) &&
> > + (slice->slice_type == V4L2_H264_SLICE_TYPE_P ||
> > + slice->slice_type == V4L2_H264_SLICE_TYPE_SP)) ||
> > + (pps->weighted_bipred_idc == 1 &&
> > + slice->slice_type == V4L2_H264_SLICE_TYPE_B))
> > + cedrus_write_pred_weight_table(ctx, run);
> > +
> > + if ((slice->slice_type == V4L2_H264_SLICE_TYPE_P) ||
> > + (slice->slice_type == V4L2_H264_SLICE_TYPE_SP) ||
> > + (slice->slice_type == V4L2_H264_SLICE_TYPE_B))
> > + cedrus_write_ref_list0(ctx, run);
> > +
> > + if (slice->slice_type == V4L2_H264_SLICE_TYPE_B)
> > + cedrus_write_ref_list1(ctx, run);
> > +
> > + // picture parameters
> > + reg = 0;
> > + /*
> > + * FIXME: the kernel headers are allowing the default value to
> > + * be passed, but the libva doesn't give us that.
> > + */
> > + reg |= (slice->num_ref_idx_l0_active_minus1 & 0x1f) << 10;
> > + reg |= (slice->num_ref_idx_l1_active_minus1 & 0x1f) << 5;
> > + reg |= (pps->weighted_bipred_idc & 0x3) << 2;
> > + if (pps->flags & V4L2_H264_PPS_FLAG_ENTROPY_CODING_MODE)
> > + reg |= VE_H264_PPS_ENTROPY_CODING_MODE;
> > + if (pps->flags & V4L2_H264_PPS_FLAG_WEIGHTED_PRED)
> > + reg |= VE_H264_PPS_WEIGHTED_PRED;
> > + if (pps->flags & V4L2_H264_PPS_FLAG_CONSTRAINED_INTRA_PRED)
> > + reg |= VE_H264_PPS_CONSTRAINED_INTRA_PRED;
> > + if (pps->flags & V4L2_H264_PPS_FLAG_TRANSFORM_8X8_MODE)
> > + reg |= VE_H264_PPS_TRANSFORM_8X8_MODE;
> > + cedrus_write(dev, VE_H264_PPS, reg);
> > +
> > + // sequence parameters
> > + reg = 0;
> > + reg |= (sps->chroma_format_idc & 0x7) << 19;
> > + reg |= (sps->pic_width_in_mbs_minus1 & 0xff) << 8;
> > + reg |= sps->pic_height_in_map_units_minus1 & 0xff;
> > + if (sps->flags & V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY)
> > + reg |= VE_H264_SPS_MBS_ONLY;
> > + if (sps->flags & V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD)
> > + reg |= VE_H264_SPS_MB_ADAPTIVE_FRAME_FIELD;
> > + if (sps->flags & V4L2_H264_SPS_FLAG_DIRECT_8X8_INFERENCE)
> > + reg |= VE_H264_SPS_DIRECT_8X8_INFERENCE;
> > + cedrus_write(dev, VE_H264_SPS, reg);
> > +
> > + // slice parameters
> > + reg = 0;
> > + reg |= decode->nal_ref_idc ? BIT(12) : 0;
BIT(12) should be a flag.
> > + reg |= (slice->slice_type & 0xf) << 8;
> > + reg |= slice->cabac_init_idc & 0x3;
> > + reg |= VE_H264_SHS_FIRST_SLICE_IN_PIC;
> > + if (slice->flags & V4L2_H264_SLICE_FLAG_FIELD_PIC)
> > + reg |= VE_H264_SHS_FIELD_PIC;
> > + if (slice->flags & V4L2_H264_SLICE_FLAG_BOTTOM_FIELD)
> > + reg |= VE_H264_SHS_BOTTOM_FIELD;
> > + if (slice->flags & V4L2_H264_SLICE_FLAG_DIRECT_SPATIAL_MV_PRED)
> > + reg |= VE_H264_SHS_DIRECT_SPATIAL_MV_PRED;
> > + cedrus_write(dev, VE_H264_SHS, reg);
> > +
> > + reg = 0;
> > + reg |= VE_H264_SHS2_NUM_REF_IDX_ACTIVE_OVRD;
> > + reg |= (slice->num_ref_idx_l0_active_minus1 & 0x1f) << 24;
> > + reg |= (slice->num_ref_idx_l1_active_minus1 & 0x1f) << 16;
> > + reg |= (slice->disable_deblocking_filter_idc & 0x3) << 8;
> > + reg |= (slice->slice_alpha_c0_offset_div2 & 0xf) << 4;
> > + reg |= slice->slice_beta_offset_div2 & 0xf;
> > + cedrus_write(dev, VE_H264_SHS2, reg);
> > +
> > + reg = 0;
> > + if (!(scaling && (pps->flags &
> > V4L2_H264_PPS_FLAG_PIC_SCALING_MATRIX_PRESENT))) + reg |=
> > VE_H264_SHS_QP_SCALING_MATRIX_DEFAULT;
> > + reg |= (pps->second_chroma_qp_index_offset & 0x3f) << 16;
> > + reg |= (pps->chroma_qp_index_offset & 0x3f) << 8;
> > + reg |= (pps->pic_init_qp_minus26 + 26 + slice->slice_qp_delta) &
>
> 0x3f;
>
> > + cedrus_write(dev, VE_H264_SHS_QP, reg);
> > +
> > + // clear status flags
> > + cedrus_write(dev, VE_H264_STATUS, cedrus_read(dev,
>
> VE_H264_STATUS));
>
> I'm not sure clearing status here is needed. Do you have any case where it
> is need? Maybe if some error happened before and cedrus_h264_irq_clear()
> wasn't cleared. I'm fine either way.
>
> > +
> > + // enable int
> > + reg = cedrus_read(dev, VE_H264_CTRL);
> > + cedrus_write(dev, VE_H264_CTRL, reg |
> > + VE_H264_CTRL_SLICE_DECODE_INT |
> > + VE_H264_CTRL_DECODE_ERR_INT |
> > + VE_H264_CTRL_VLD_DATA_REQ_INT);
>
> Since this is the only place where you set VE_H264_CTRL, I wouldn't preserve
> previous content. This mode is also capable of decoding VP8 and AVS. So in
> theory, if user would want to decode H264 and VP8 videos at the same time,
> preserving content will probably corrupt your output. I would just set all
> other bits to 0. What do you think? I tested this without preservation and
> it works fine.
>
> > +}
> > +
> > +static enum cedrus_irq_status
> > +cedrus_h264_irq_status(struct cedrus_ctx *ctx)
> > +{
> > + struct cedrus_dev *dev = ctx->dev;
> > + u32 reg = cedrus_read(dev, VE_H264_STATUS);
> > +
> > + if (reg & (VE_H264_STATUS_DECODE_ERR_INT |
> > + VE_H264_STATUS_VLD_DATA_REQ_INT))
> > + return CEDRUS_IRQ_ERROR;
> > +
> > + if (reg & VE_H264_CTRL_SLICE_DECODE_INT)
> > + return CEDRUS_IRQ_OK;
> > +
> > + return CEDRUS_IRQ_NONE;
> > +}
> > +
> > +static void cedrus_h264_irq_clear(struct cedrus_ctx *ctx)
> > +{
> > + struct cedrus_dev *dev = ctx->dev;
> > +
> > + cedrus_write(dev, VE_H264_STATUS,
> > + VE_H264_STATUS_INT_MASK);
> > +}
> > +
> > +static void cedrus_h264_irq_disable(struct cedrus_ctx *ctx)
> > +{
> > + struct cedrus_dev *dev = ctx->dev;
> > + u32 reg = cedrus_read(dev, VE_H264_CTRL);
> > +
> > + cedrus_write(dev, VE_H264_CTRL,
> > + reg & ~VE_H264_CTRL_INT_MASK);
> > +}
> > +
> > +static void cedrus_h264_setup(struct cedrus_ctx *ctx,
> > + struct cedrus_run *run)
> > +{
> > + struct cedrus_dev *dev = ctx->dev;
> > +
> > + cedrus_engine_enable(dev, CEDRUS_CODEC_H264);
> > +
> > + cedrus_write(dev, VE_H264_SDROT_CTRL, 0);
> > + cedrus_write(dev, VE_H264_EXTRA_BUFFER1,
> > + ctx->codec.h264.pic_info_buf_dma);
> > + cedrus_write(dev, VE_H264_EXTRA_BUFFER2,
> > + ctx->codec.h264.neighbor_info_buf_dma);
> > +
> > + cedrus_write_scaling_lists(ctx, run);
> > + cedrus_write_frame_list(ctx, run);
> > +
> > + cedrus_set_params(ctx, run);
> > +}
> > +
> > +static int cedrus_h264_start(struct cedrus_ctx *ctx)
> > +{
> > + struct cedrus_dev *dev = ctx->dev;
> > + unsigned int field_size;
> > + unsigned int mv_col_size;
> > + int ret;
> > +
> > + /*
> > + * FIXME: It seems that the H6 cedarX code is using a formula
> > + * here based on the size of the frame, while all the older
> > + * code is using a fixed size, so that might need to be
> > + * changed at some point.
> > + */
> > + ctx->codec.h264.pic_info_buf =
> > + dma_alloc_coherent(dev->dev, CEDRUS_PIC_INFO_BUF_SIZE,
> > + &ctx-
> >
> >codec.h264.pic_info_buf_dma,
> >
> > + GFP_KERNEL);
> > + if (!ctx->codec.h264.pic_info_buf)
> > + return -ENOMEM;
> > +
> > + /*
> > + * That buffer is supposed to be 16kiB in size, and be aligned
> > + * on 16kiB as well. However, dma_alloc_coherent provides the
> > + * guarantee that we'll have a CPU and DMA address aligned on
> > + * the smallest page order that is greater to the requested
> > + * size, so we don't have to overallocate.
> > + */
> > + ctx->codec.h264.neighbor_info_buf =
> > + dma_alloc_coherent(dev->dev,
>
> CEDRUS_NEIGHBOR_INFO_BUF_SIZE,
>
> > + &ctx-
> >
> >codec.h264.neighbor_info_buf_dma,
> >
> > + GFP_KERNEL);
> > + if (!ctx->codec.h264.neighbor_info_buf) {
> > + ret = -ENOMEM;
> > + goto err_pic_buf;
> > + }
> > +
> > + field_size = DIV_ROUND_UP(ctx->src_fmt.width, 16) *
> > + DIV_ROUND_UP(ctx->src_fmt.height, 16) * 16;
> > +
> > + /*
> > + * FIXME: This is actually conditional to
> > + * V4L2_H264_SPS_FLAG_DIRECT_8X8_INFERENCE not being set, we
> > + * might have to rework this if memory efficiency ever is
> > + * something we need to work on.
> > + */
> > + field_size = field_size * 2;
> > +
> > + /*
> > + * FIXME: This is actually conditional to
> > + * V4L2_H264_SPS_FLAG_FRAME_MBS_ONLY not being set, we might
> > + * have to rework this if memory efficiency ever is something
> > + * we need to work on.
> > + */
> > + field_size = field_size * 2;
> > + ctx->codec.h264.mv_col_buf_field_size = field_size;
>
> CedarX code aligns this buffer to 1024. Should we do it too just to be on
> the safe side? I don't think it cost us anything due to
> dma_alloc_coherent() alignments.
I mixed up this a bit. While I still think we should do it, it cost us just a
little extra memory.
It wouldn't cost us anything if they would be separately allocated (due to
alignment), but that's not the case here.
Best regards,
Jernej
>
> Sorry again for a bit late in-depth review.
>
> Best regards,
> Jernej
>
> > +
> > + mv_col_size = field_size * 2 * CEDRUS_H264_FRAME_NUM;
> > + ctx->codec.h264.mv_col_buf_size = mv_col_size;
> > + ctx->codec.h264.mv_col_buf = dma_alloc_coherent(dev->dev,
> > +
>
> ctx->codec.h264.mv_col_buf_size,
>
> > +
>
> &ctx->codec.h264.mv_col_buf_dma,
>
> > +
>
> GFP_KERNEL);
>
> > + if (!ctx->codec.h264.mv_col_buf) {
> > + ret = -ENOMEM;
> > + goto err_neighbor_buf;
> > + }
> > +
> > + return 0;
> > +
> > +err_neighbor_buf:
> > + dma_free_coherent(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE,
> > + ctx->codec.h264.neighbor_info_buf,
> > + ctx->codec.h264.neighbor_info_buf_dma);
> > +
> > +err_pic_buf:
> > + dma_free_coherent(dev->dev, CEDRUS_PIC_INFO_BUF_SIZE,
> > + ctx->codec.h264.pic_info_buf,
> > + ctx->codec.h264.pic_info_buf_dma);
> > + return ret;
> > +}
> > +
> > +static void cedrus_h264_stop(struct cedrus_ctx *ctx)
> > +{
> > + struct cedrus_dev *dev = ctx->dev;
> > +
> > + dma_free_coherent(dev->dev, ctx->codec.h264.mv_col_buf_size,
> > + ctx->codec.h264.mv_col_buf,
> > + ctx->codec.h264.mv_col_buf_dma);
> > + dma_free_coherent(dev->dev, CEDRUS_NEIGHBOR_INFO_BUF_SIZE,
> > + ctx->codec.h264.neighbor_info_buf,
> > + ctx->codec.h264.neighbor_info_buf_dma);
> > + dma_free_coherent(dev->dev, CEDRUS_PIC_INFO_BUF_SIZE,
> > + ctx->codec.h264.pic_info_buf,
> > + ctx->codec.h264.pic_info_buf_dma);
> > +}
> > +
> > +static void cedrus_h264_trigger(struct cedrus_ctx *ctx)
> > +{
> > + struct cedrus_dev *dev = ctx->dev;
> > +
> > + cedrus_write(dev, VE_H264_TRIGGER_TYPE,
> > + VE_H264_TRIGGER_TYPE_AVC_SLICE_DECODE);
> > +}
> > +
> > +struct cedrus_dec_ops cedrus_dec_ops_h264 = {
> > + .irq_clear = cedrus_h264_irq_clear,
> > + .irq_disable = cedrus_h264_irq_disable,
> > + .irq_status = cedrus_h264_irq_status,
> > + .setup = cedrus_h264_setup,
> > + .start = cedrus_h264_start,
> > + .stop = cedrus_h264_stop,
> > + .trigger = cedrus_h264_trigger,
> > +};
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c index
> > 0acf219a8c91..ab402b0cac4e 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> > @@ -46,6 +46,10 @@ int cedrus_engine_enable(struct cedrus_dev *dev, enum
> > cedrus_codec codec) reg |= VE_MODE_DEC_MPEG;
> >
> > break;
> >
> > + case CEDRUS_CODEC_H264:
> > + reg |= VE_MODE_DEC_H264;
> > + break;
> > +
> >
> > default:
> > return -EINVAL;
> >
> > }
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
> > b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h index
> > de2d6b6f64bf..3e9931416e45 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
> > @@ -232,4 +232,95 @@
> >
> > #define VE_DEC_MPEG_ROT_LUMA (VE_ENGINE_DEC_MPEG +
>
> 0xcc)
>
> > #define VE_DEC_MPEG_ROT_CHROMA
>
> (VE_ENGINE_DEC_MPEG + 0xd0)
>
> > +#define VE_H264_SPS 0x200
> > +#define VE_H264_SPS_MBS_ONLY BIT(18)
> > +#define VE_H264_SPS_MB_ADAPTIVE_FRAME_FIELD BIT(17)
> > +#define VE_H264_SPS_DIRECT_8X8_INFERENCE BIT(16)
> > +
> > +#define VE_H264_PPS 0x204
> > +#define VE_H264_PPS_ENTROPY_CODING_MODE BIT(15)
> > +#define VE_H264_PPS_WEIGHTED_PRED BIT(4)
> > +#define VE_H264_PPS_CONSTRAINED_INTRA_PRED BIT(1)
> > +#define VE_H264_PPS_TRANSFORM_8X8_MODE BIT(0)
> > +
> > +#define VE_H264_SHS 0x208
> > +#define VE_H264_SHS_FIRST_SLICE_IN_PIC BIT(5)
> > +#define VE_H264_SHS_FIELD_PIC BIT(4)
> > +#define VE_H264_SHS_BOTTOM_FIELD BIT(3)
> > +#define VE_H264_SHS_DIRECT_SPATIAL_MV_PRED BIT(2)
> > +
> > +#define VE_H264_SHS2 0x20c
> > +#define VE_H264_SHS2_NUM_REF_IDX_ACTIVE_OVRD BIT(12)
> > +
> > +#define VE_H264_SHS_WP 0x210
> > +
> > +#define VE_H264_SHS_QP 0x21c
> > +#define VE_H264_SHS_QP_SCALING_MATRIX_DEFAULT BIT(24)
> > +
> > +#define VE_H264_CTRL 0x220
> > +#define VE_H264_CTRL_VLD_DATA_REQ_INT BIT(2)
> > +#define VE_H264_CTRL_DECODE_ERR_INT BIT(1)
> > +#define VE_H264_CTRL_SLICE_DECODE_INT BIT(0)
> > +
> > +#define VE_H264_CTRL_INT_MASK (VE_H264_CTRL_VLD_DATA_REQ_INT |
\
> > +
>
> VE_H264_CTRL_DECODE_ERR_INT | \
>
> > +
>
> VE_H264_CTRL_SLICE_DECODE_INT)
>
> > +
> > +#define VE_H264_TRIGGER_TYPE 0x224
> > +#define VE_H264_TRIGGER_TYPE_AVC_SLICE_DECODE (8 << 0)
> > +#define VE_H264_TRIGGER_TYPE_INIT_SWDEC (7 << 0)
> > +
> > +#define VE_H264_STATUS 0x228
> > +#define VE_H264_STATUS_VLD_DATA_REQ_INT
>
> VE_H264_CTRL_VLD_DATA_REQ_INT
>
> > +#define VE_H264_STATUS_DECODE_ERR_INT
>
> VE_H264_CTRL_DECODE_ERR_INT
>
> > +#define VE_H264_STATUS_SLICE_DECODE_INT
>
> VE_H264_CTRL_SLICE_DECODE_INT
>
> > +
> > +#define VE_H264_STATUS_INT_MASK
>
> VE_H264_CTRL_INT_MASK
>
> > +
> > +#define VE_H264_CUR_MB_NUM 0x22c
> > +
> > +#define VE_H264_VLD_ADDR 0x230
> > +#define VE_H264_VLD_ADDR_FIRST BIT(30)
> > +#define VE_H264_VLD_ADDR_LAST BIT(29)
> > +#define VE_H264_VLD_ADDR_VALID BIT(28)
> > +#define VE_H264_VLD_ADDR_VAL(x) (((x) &
0x0ffffff0) |
>
> ((x) >> 28))
>
> > +
> > +#define VE_H264_VLD_OFFSET 0x234
> > +#define VE_H264_VLD_LEN 0x238
> > +#define VE_H264_VLD_END 0x23c
> > +#define VE_H264_SDROT_CTRL 0x240
> > +#define VE_H264_OUTPUT_FRAME_IDX 0x24c
> > +#define VE_H264_EXTRA_BUFFER1 0x250
> > +#define VE_H264_EXTRA_BUFFER2 0x254
> > +#define VE_H264_BASIC_BITS 0x2dc
> > +#define VE_AVC_SRAM_PORT_OFFSET 0x2e0
> > +#define VE_AVC_SRAM_PORT_DATA 0x2e4
> > +
> > +#define VE_ISP_INPUT_SIZE 0xa00
> > +#define VE_ISP_INPUT_STRIDE 0xa04
> > +#define VE_ISP_CTRL 0xa08
> > +#define VE_ISP_INPUT_LUMA 0xa78
> > +#define VE_ISP_INPUT_CHROMA 0xa7c
> > +
> > +#define VE_AVC_PARAM 0xb04
> > +#define VE_AVC_QP 0xb08
> > +#define VE_AVC_MOTION_EST 0xb10
> > +#define VE_AVC_CTRL 0xb14
> > +#define VE_AVC_TRIGGER 0xb18
> > +#define VE_AVC_STATUS 0xb1c
> > +#define VE_AVC_BASIC_BITS 0xb20
> > +#define VE_AVC_UNK_BUF 0xb60
> > +#define VE_AVC_VLE_ADDR 0xb80
> > +#define VE_AVC_VLE_END 0xb84
> > +#define VE_AVC_VLE_OFFSET 0xb88
> > +#define VE_AVC_VLE_MAX 0xb8c
> > +#define VE_AVC_VLE_LENGTH 0xb90
> > +#define VE_AVC_REF_LUMA 0xba0
> > +#define VE_AVC_REF_CHROMA 0xba4
> > +#define VE_AVC_REC_LUMA 0xbb0
> > +#define VE_AVC_REC_CHROMA 0xbb4
> > +#define VE_AVC_REF_SLUMA 0xbb8
> > +#define VE_AVC_REC_SLUMA 0xbbc
> > +#define VE_AVC_MB_INFO 0xbc0
> > +
> >
> > #endif
> >
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_video.c index
> > b5cc79389d67..67062900f87a 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > @@ -38,6 +38,10 @@ static struct cedrus_format cedrus_formats[] = {
> >
> > .directions = CEDRUS_DECODE_SRC,
> >
> > },
> > {
> >
> > + .pixelformat = V4L2_PIX_FMT_H264_SLICE,
> > + .directions = CEDRUS_DECODE_SRC,
> > + },
> > + {
> >
> > .pixelformat = V4L2_PIX_FMT_SUNXI_TILED_NV12,
> > .directions = CEDRUS_DECODE_DST,
> >
> > },
> >
> > @@ -100,6 +104,7 @@ static void cedrus_prepare_format(struct
> > v4l2_pix_format *pix_fmt)
> >
> > switch (pix_fmt->pixelformat) {
> >
> > case V4L2_PIX_FMT_MPEG2_SLICE:
> > + case V4L2_PIX_FMT_H264_SLICE:
> > /* Zero bytes per line for encoded source. */
> > bytesperline = 0;
> >
> > @@ -454,6 +459,10 @@ static int cedrus_start_streaming(struct vb2_queue
> > *vq, unsigned int count) ctx->current_codec = CEDRUS_CODEC_MPEG2;
> >
> > break;
> >
> > + case V4L2_PIX_FMT_H264_SLICE:
> > + ctx->current_codec = CEDRUS_CODEC_H264;
> > + break;
> > +
> >
> > default:
> > return -EINVAL;
> >
> > }