| Message ID | 20210427111526.1772293-1-acourbot@chromium.org |
|---|---|
| Headers | show |
| Series | media: mtk-vcodec: support for MT8183 decoder | expand |
On 27/04/2021 13:15, Alexandre Courbot wrote: > Add Mediatek's non-compressed 8 bit block video mode. This format is > produced by the MT8183 codec and can be converted to a non-proprietary > format by the MDP3 component. > > Signed-off-by: Alexandre Courbot <acourbot@chromium.org> > --- > Documentation/userspace-api/media/v4l/pixfmt-reserved.rst | 7 +++++++ > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > include/uapi/linux/videodev2.h | 1 + > 3 files changed, 9 insertions(+) > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > index 0b879c0da713..42357b0b3535 100644 > --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > @@ -246,6 +246,13 @@ please make a proposal on the linux-media mailing list. > It is an opaque intermediate format and the MDP hardware must be > used to convert ``V4L2_PIX_FMT_MT21C`` to ``V4L2_PIX_FMT_NV12M``, > ``V4L2_PIX_FMT_YUV420M`` or ``V4L2_PIX_FMT_YVU420``. > + * .. _V4L2-PIX-FMT-MM21: > + > + - ``V4L2_PIX_FMT_MM21`` > + - 'MM21' > + - Non-compressed, tiled two-planar format used by Mediatek MT8183. > + This is an opaque intermediate format and the MDP3 hardware can be > + used to convert it to other formats. > * .. _V4L2-PIX-FMT-SUNXI-TILED-NV12: > > - ``V4L2_PIX_FMT_SUNXI_TILED_NV12`` > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > index 6a5d1c6d11d6..608e3ddc0f42 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -1385,6 +1385,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) > case V4L2_PIX_FMT_TM6000: descr = "A/V + VBI Mux Packet"; break; > case V4L2_PIX_FMT_CIT_YYVYUY: descr = "GSPCA CIT YYVYUY"; break; > case V4L2_PIX_FMT_KONICA420: descr = "GSPCA KONICA420"; break; > + case V4L2_PIX_FMT_MM21: descr = "Mediatek 8-bit block format"; break; block format -> Block Format (to be consistent with the other descriptions) Regards, Hans > case V4L2_PIX_FMT_HSV24: descr = "24-bit HSV 8-8-8"; break; > case V4L2_PIX_FMT_HSV32: descr = "32-bit XHSV 8-8-8-8"; break; > case V4L2_SDR_FMT_CU8: descr = "Complex U8"; break; > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 311a01cc5775..db04e37da1a8 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -733,6 +733,7 @@ struct v4l2_pix_format { > #define V4L2_PIX_FMT_Y12I v4l2_fourcc('Y', '1', '2', 'I') /* Greyscale 12-bit L/R interleaved */ > #define V4L2_PIX_FMT_Z16 v4l2_fourcc('Z', '1', '6', ' ') /* Depth data 16-bit */ > #define V4L2_PIX_FMT_MT21C v4l2_fourcc('M', 'T', '2', '1') /* Mediatek compressed block mode */ > +#define V4L2_PIX_FMT_MM21 v4l2_fourcc('M', 'M', '2', '1') /* Mediatek 8-bit block mode, two non-contiguous planes */ > #define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Planar Greyscale 10-bit and Depth 16-bit */ > #define V4L2_PIX_FMT_SUNXI_TILED_NV12 v4l2_fourcc('S', 'T', '1', '2') /* Sunxi Tiled NV12 Format */ > #define V4L2_PIX_FMT_CNF4 v4l2_fourcc('C', 'N', 'F', '4') /* Intel 4-bit packed depth confidence information */ >
On 27/04/2021 13:15, Alexandre Courbot wrote: > From: Yunfei Dong <yunfei.dong@mediatek.com> > > Support the stateless codec API that will be used by MT8183. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > [acourbot: refactor, cleanup and split] > Co-developed-by: Alexandre Courbot <acourbot@chromium.org> > Signed-off-by: Alexandre Courbot <acourbot@chromium.org> > --- > drivers/media/platform/mtk-vcodec/Makefile | 1 + > .../platform/mtk-vcodec/mtk_vcodec_dec.c | 66 +++- > .../platform/mtk-vcodec/mtk_vcodec_dec.h | 9 +- > .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 370 ++++++++++++++++++ > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 + > 5 files changed, 446 insertions(+), 3 deletions(-) > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > > diff --git a/drivers/media/platform/mtk-vcodec/Makefile b/drivers/media/platform/mtk-vcodec/Makefile > index 9c3cbb5b800e..4ba93d838ab6 100644 > --- a/drivers/media/platform/mtk-vcodec/Makefile > +++ b/drivers/media/platform/mtk-vcodec/Makefile > @@ -12,6 +12,7 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \ > vdec_vpu_if.o \ > mtk_vcodec_dec.o \ > mtk_vcodec_dec_stateful.o \ > + mtk_vcodec_dec_stateless.o \ > mtk_vcodec_dec_pm.o \ > > mtk-vcodec-enc-y := venc/venc_vp8_if.o \ > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c > index 4ad2662a43b2..01c5333d6cff 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c > @@ -46,6 +46,13 @@ static struct mtk_q_data *mtk_vdec_get_q_data(struct mtk_vcodec_ctx *ctx, > static int vidioc_try_decoder_cmd(struct file *file, void *priv, > struct v4l2_decoder_cmd *cmd) > { > + struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv); > + > + /* Use M2M stateless helper if relevant */ > + if (ctx->dev->vdec_pdata->uses_stateless_api) > + return v4l2_m2m_ioctl_stateless_try_decoder_cmd(file, priv, > + cmd); > + > switch (cmd->cmd) { > case V4L2_DEC_CMD_STOP: > case V4L2_DEC_CMD_START: > @@ -72,6 +79,10 @@ static int vidioc_decoder_cmd(struct file *file, void *priv, > if (ret) > return ret; > > + /* Use M2M stateless helper if relevant */ > + if (ctx->dev->vdec_pdata->uses_stateless_api) > + return v4l2_m2m_ioctl_stateless_decoder_cmd(file, priv, cmd); > + > mtk_v4l2_debug(1, "decoder cmd=%u", cmd->cmd); > dst_vq = v4l2_m2m_get_vq(ctx->m2m_ctx, > V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > @@ -414,7 +425,8 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv, > * Setting OUTPUT format after OUTPUT buffers are allocated is invalid > * if using the stateful API. > */ > - if ((f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) && > + if (!dec_pdata->uses_stateless_api && > + (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) && > vb2_is_busy(&ctx->m2m_ctx->out_q_ctx.q)) { > mtk_v4l2_err("out_q_ctx buffers already requested"); > ret = -EBUSY; > @@ -457,6 +469,7 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv, > ctx->quantization = pix_mp->quantization; > ctx->xfer_func = pix_mp->xfer_func; > > + ctx->current_codec = fmt->fourcc; > if (ctx->state == MTK_STATE_FREE) { > ret = vdec_if_init(ctx, q_data->fmt->fourcc); > if (ret) { > @@ -468,6 +481,49 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv, > } > } > > + /* > + * If using the stateless API, S_FMT should have the effect of setting > + * the CAPTURE queue resolution no matter which queue it was called on. > + */ > + if (dec_pdata->uses_stateless_api) { > + ctx->picinfo.pic_w = pix_mp->width; > + ctx->picinfo.pic_h = pix_mp->height; > + > + ret = vdec_if_get_param(ctx, GET_PARAM_PIC_INFO, &ctx->picinfo); > + if (ret) { > + mtk_v4l2_err("[%d]Error!! Get GET_PARAM_PICTURE_INFO Fail", > + ctx->id); > + return -EINVAL; > + } > + > + ctx->last_decoded_picinfo = ctx->picinfo; > + > + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 1) { > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] = > + ctx->picinfo.fb_sz[0] + > + ctx->picinfo.fb_sz[1]; > + ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] = > + ctx->picinfo.buf_w; > + } else { > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] = > + ctx->picinfo.fb_sz[0]; > + ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] = > + ctx->picinfo.buf_w; > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[1] = > + ctx->picinfo.fb_sz[1]; > + ctx->q_data[MTK_Q_DATA_DST].bytesperline[1] = > + ctx->picinfo.buf_w; > + } > + > + ctx->q_data[MTK_Q_DATA_DST].coded_width = ctx->picinfo.buf_w; > + ctx->q_data[MTK_Q_DATA_DST].coded_height = ctx->picinfo.buf_h; > + mtk_v4l2_debug(2, "[%d] vdec_if_init() num_plane = %d wxh=%dx%d pic wxh=%dx%d sz[0]=0x%x sz[1]=0x%x", > + ctx->id, pix_mp->num_planes, > + ctx->picinfo.buf_w, ctx->picinfo.buf_h, > + ctx->picinfo.pic_w, ctx->picinfo.pic_h, > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[0], > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[1]); > + } > return 0; > } > > @@ -765,9 +821,15 @@ void vb2ops_vdec_stop_streaming(struct vb2_queue *q) > while ((src_buf = v4l2_m2m_src_buf_remove(ctx->m2m_ctx))) { > struct mtk_video_dec_buf *buf_info = container_of( > src_buf, struct mtk_video_dec_buf, m2m_buf.vb); > - if (!buf_info->lastframe) > + if (!buf_info->lastframe) { > + struct media_request *req = > + src_buf->vb2_buf.req_obj.req; > v4l2_m2m_buf_done(src_buf, > VB2_BUF_STATE_ERROR); > + if (req) > + v4l2_ctrl_request_complete(req, > + &ctx->ctrl_hdl); > + } > } > return; > } > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h > index 6a18cb3bfe07..6b29d7d9ae15 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h > @@ -45,6 +45,7 @@ struct vdec_fb { > * @lastframe: Intput buffer is last buffer - EOS > * @error: An unrecoverable error occurs on this buffer. > * @frame_buffer: Decode status, and buffer information of Capture buffer > + * @bs_buffer: Output buffer info > * > * Note : These status information help us track and debug buffer state > */ > @@ -55,12 +56,18 @@ struct mtk_video_dec_buf { > bool queued_in_vb2; > bool queued_in_v4l2; > bool lastframe; > + > bool error; > - struct vdec_fb frame_buffer; > + > + union { > + struct vdec_fb frame_buffer; > + struct mtk_vcodec_mem bs_buffer; > + }; > }; > > extern const struct v4l2_ioctl_ops mtk_vdec_ioctl_ops; > extern const struct v4l2_m2m_ops mtk_vdec_m2m_ops; > +extern const struct media_device_ops mtk_vcodec_media_ops; > > > /* > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > new file mode 100644 > index 000000000000..75ddf53e2876 > --- /dev/null > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > @@ -0,0 +1,370 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include "media/videobuf2-v4l2.h" > +#include <media/videobuf2-dma-contig.h> > +#include <media/v4l2-event.h> > +#include <media/v4l2-mem2mem.h> > +#include <linux/module.h> > + > +#include "mtk_vcodec_drv.h" > +#include "mtk_vcodec_dec.h" > +#include "mtk_vcodec_intr.h" > +#include "mtk_vcodec_util.h" > +#include "vdec_drv_if.h" > +#include "mtk_vcodec_dec_pm.h" > + > +/** > + * struct mtk_stateless_control - CID control type > + * @cfg: control configuration > + * @codec_type: codec type (V4L2 pixel format) for CID control type > + */ > +struct mtk_stateless_control { > + struct v4l2_ctrl_config cfg; > + int codec_type; > +}; > + > +static const struct mtk_stateless_control mtk_stateless_controls[] = { > + { > + .cfg = { > + .id = V4L2_CID_STATELESS_H264_SPS, > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + }, > + { > + .cfg = { > + .id = V4L2_CID_STATELESS_H264_PPS, > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + }, > + { > + .cfg = { > + .id = V4L2_CID_STATELESS_H264_SCALING_MATRIX, > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + }, > + { > + .cfg = { > + .id = V4L2_CID_STATELESS_H264_DECODE_PARAMS, > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + }, > + { > + .cfg = { > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, > + .menu_skip_mask = > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) | > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + }, > + { > + .cfg = { > + .id = V4L2_CID_STATELESS_H264_DECODE_MODE, > + .min = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED, > + .def = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED, > + .max = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED, > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + }, > + { > + .cfg = { > + .id = V4L2_CID_STATELESS_H264_START_CODE, > + .min = V4L2_STATELESS_H264_START_CODE_ANNEX_B, > + .def = V4L2_STATELESS_H264_START_CODE_ANNEX_B, > + .max = V4L2_STATELESS_H264_START_CODE_ANNEX_B, > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + } > +}; > +#define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls) > + > +static const struct mtk_video_fmt mtk_video_formats[] = { > + { > + .fourcc = V4L2_PIX_FMT_H264_SLICE, > + .type = MTK_FMT_DEC, > + .num_planes = 1, > + }, > + { > + .fourcc = V4L2_PIX_FMT_MM21, > + .type = MTK_FMT_FRAME, > + .num_planes = 2, > + }, > +}; > +#define NUM_FORMATS ARRAY_SIZE(mtk_video_formats) > +#define DEFAULT_OUT_FMT_IDX 0 > +#define DEFAULT_CAP_FMT_IDX 1 > + > +static const struct mtk_codec_framesizes mtk_vdec_framesizes[] = { > + { > + .fourcc = V4L2_PIX_FMT_H264_SLICE, > + .stepwise = { MTK_VDEC_MIN_W, MTK_VDEC_MAX_W, 16, > + MTK_VDEC_MIN_H, MTK_VDEC_MAX_H, 16 }, > + }, > +}; > + > +#define NUM_SUPPORTED_FRAMESIZE ARRAY_SIZE(mtk_vdec_framesizes) > + > +static void mtk_vdec_stateless_set_dst_payload(struct mtk_vcodec_ctx *ctx, > + struct vdec_fb *fb) > +{ > + struct mtk_video_dec_buf *vdec_frame_buf = > + container_of(fb, struct mtk_video_dec_buf, frame_buffer); > + struct vb2_v4l2_buffer *vb = &vdec_frame_buf->m2m_buf.vb; > + unsigned int cap_y_size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[0]; > + > + vb2_set_plane_payload(&vb->vb2_buf, 0, cap_y_size); > + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2) { > + unsigned int cap_c_size = > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[1]; > + > + vb2_set_plane_payload(&vb->vb2_buf, 1, cap_c_size); > + } > +} > + > +static struct vdec_fb *vdec_get_cap_buffer(struct mtk_vcodec_ctx *ctx, > + struct vb2_v4l2_buffer *vb2_v4l2) > +{ > + struct mtk_video_dec_buf *framebuf = > + container_of(vb2_v4l2, struct mtk_video_dec_buf, m2m_buf.vb); > + struct vdec_fb *pfb = &framebuf->frame_buffer; > + struct vb2_buffer *dst_buf = &vb2_v4l2->vb2_buf; > + > + pfb = &framebuf->frame_buffer; > + pfb->base_y.va = NULL; > + pfb->base_y.dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0); > + pfb->base_y.size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[0]; > + > + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2) { > + pfb->base_c.va = NULL; > + pfb->base_c.dma_addr = > + vb2_dma_contig_plane_dma_addr(dst_buf, 1); > + pfb->base_c.size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[1]; > + } > + mtk_v4l2_debug(1, > + "id=%d Framebuf pfb=%p VA=%p Y_DMA=%pad C_DMA=%pad Size=%zx frame_count = %d", > + dst_buf->index, pfb, > + pfb->base_y.va, &pfb->base_y.dma_addr, > + &pfb->base_c.dma_addr, pfb->base_y.size, > + ctx->decoded_frame_cnt); > + > + return pfb; > +} > + > +static void vb2ops_vdec_buf_request_complete(struct vb2_buffer *vb) > +{ > + struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); > + > + v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->ctrl_hdl); > +} > + > +static int fops_media_request_validate(struct media_request *mreq) I think this function should be moved to just before mtk_vcodec_media_ops. It's a bit weird to have this in the middle of the code. > +{ > + const unsigned int buffer_cnt = vb2_request_buffer_cnt(mreq); > + > + switch (buffer_cnt) { > + case 1: > + /* We expect exactly one buffer with the request */ > + break; > + case 0: > + mtk_v4l2_err("No buffer provided with the request"); > + return -ENOENT; > + default: > + mtk_v4l2_err("Too many buffers (%d) provided with the request", > + buffer_cnt); These aren't errors: user errors are not something that should be reported in the kernel log, only driver/hw errors should be reported there. Use _debug instead. > + return -EINVAL; > + } > + > + return vb2_request_validate(mreq); > +} > + > +static void mtk_vdec_worker(struct work_struct *work) > +{ > + struct mtk_vcodec_ctx *ctx = > + container_of(work, struct mtk_vcodec_ctx, decode_work); > + struct mtk_vcodec_dev *dev = ctx->dev; > + struct vb2_v4l2_buffer *vb2_v4l2_src, *vb2_v4l2_dst; > + struct vb2_buffer *vb2_src; > + struct mtk_vcodec_mem *bs_src; > + struct mtk_video_dec_buf *dec_buf_src; > + struct media_request *src_buf_req; > + struct vdec_fb *dst_buf; > + bool res_chg = false; > + int ret; > + > + vb2_v4l2_src = v4l2_m2m_next_src_buf(ctx->m2m_ctx); > + if (vb2_v4l2_src == NULL) { > + v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx); > + mtk_v4l2_debug(1, "[%d] no available source buffer", ctx->id); > + return; > + } > + > + vb2_v4l2_dst = v4l2_m2m_next_dst_buf(ctx->m2m_ctx); > + if (vb2_v4l2_dst == NULL) { > + v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx); > + mtk_v4l2_debug(1, "[%d] no available destination buffer", ctx->id); > + return; > + } > + > + vb2_src = &vb2_v4l2_src->vb2_buf; > + dec_buf_src = container_of(vb2_v4l2_src, struct mtk_video_dec_buf, > + m2m_buf.vb); > + bs_src = &dec_buf_src->bs_buffer; > + > + mtk_v4l2_debug(3, "[%d] (%d) id=%d, vb=%p buf_info = %p", > + ctx->id, src_buf->vb2_queue->type, > + src_buf->index, src_buf, src_buf_info); > + > + bs_src->va = NULL; > + bs_src->dma_addr = vb2_dma_contig_plane_dma_addr(vb2_src, 0); > + bs_src->size = (size_t)vb2_src->planes[0].bytesused; > + > + mtk_v4l2_debug(3, "[%d] Bitstream VA=%p DMA=%pad Size=%zx vb=%p", > + ctx->id, buf->va, &buf->dma_addr, buf->size, src_buf); > + /* Apply request controls. */ > + src_buf_req = vb2_src->req_obj.req; > + if (src_buf_req) > + v4l2_ctrl_request_setup(src_buf_req, &ctx->ctrl_hdl); > + else > + mtk_v4l2_err("vb2 buffer media request is NULL"); > + > + dst_buf = vdec_get_cap_buffer(ctx, vb2_v4l2_dst); > + v4l2_m2m_buf_copy_metadata(vb2_v4l2_src, vb2_v4l2_dst, true); > + ret = vdec_if_decode(ctx, bs_src, dst_buf, &res_chg); > + if (ret) { > + mtk_v4l2_err( > + " <===[%d], src_buf[%d] sz=0x%zx pts=%llu vdec_if_decode() ret=%d res_chg=%d===>", > + ctx->id, vb2_src->index, bs_src->size, > + vb2_src->timestamp, ret, res_chg); > + if (ret == -EIO) { > + mutex_lock(&ctx->lock); > + dec_buf_src->error = true; > + mutex_unlock(&ctx->lock); > + } > + } > + > + mtk_vdec_stateless_set_dst_payload(ctx, dst_buf); > + > + v4l2_m2m_buf_done_and_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx, > + ret ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE); > + > + v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl); > +} > + > +static void vb2ops_vdec_stateless_buf_queue(struct vb2_buffer *vb) > +{ > + struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); > + struct vb2_v4l2_buffer *vb2_v4l2 = to_vb2_v4l2_buffer(vb); > + > + mtk_v4l2_debug(3, "[%d] (%d) id=%d, vb=%p", > + ctx->id, vb->vb2_queue->type, > + vb->index, vb); > + > + mutex_lock(&ctx->lock); > + v4l2_m2m_buf_queue(ctx->m2m_ctx, vb2_v4l2); > + mutex_unlock(&ctx->lock); > + if (vb->vb2_queue->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) > + return; > + > + mtk_v4l2_debug(3, "(%d) id=%d, bs=%p", > + vb->vb2_queue->type, vb->index, src_buf); > + > + /* If an OUTPUT buffer, we may need to update the state */ > + if (ctx->state == MTK_STATE_INIT) { > + ctx->state = MTK_STATE_HEADER; > + mtk_v4l2_debug(1, "Init driver from init to header."); > + } else { > + mtk_v4l2_debug(3, "[%d] already init driver %d", > + ctx->id, ctx->state); > + } > +} > + > +static int mtk_vdec_flush_decoder(struct mtk_vcodec_ctx *ctx) > +{ > + bool res_chg; > + > + return vdec_if_decode(ctx, NULL, NULL, &res_chg); > +} > + > +static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_ctx *ctx) > +{ > + unsigned int i; > + > + v4l2_ctrl_handler_init(&ctx->ctrl_hdl, NUM_CTRLS); > + if (ctx->ctrl_hdl.error) { > + mtk_v4l2_err("v4l2_ctrl_handler_init failed\n"); > + return ctx->ctrl_hdl.error; > + } > + > + for (i = 0; i < NUM_CTRLS; i++) { > + struct v4l2_ctrl_config cfg = mtk_stateless_controls[i].cfg; > + > + v4l2_ctrl_new_custom(&ctx->ctrl_hdl, &cfg, NULL); > + if (ctx->ctrl_hdl.error) { > + mtk_v4l2_err("Adding control %d failed %d", > + i, ctx->ctrl_hdl.error); > + return ctx->ctrl_hdl.error; > + } > + } > + > + v4l2_ctrl_handler_setup(&ctx->ctrl_hdl); > + > + return 0; > +} > + > +const struct media_device_ops mtk_vcodec_media_ops = { > + .req_validate = fops_media_request_validate, > + .req_queue = v4l2_m2m_request_queue, > +}; > + > +static void mtk_init_vdec_params(struct mtk_vcodec_ctx *ctx) > +{ > + struct vb2_queue *src_vq; > + > + src_vq = v4l2_m2m_get_vq(ctx->m2m_ctx, > + V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); > + > + /* Support request api for output plane */ > + src_vq->supports_requests = true; > + src_vq->requires_requests = true; > +} > + > +static int vb2ops_vdec_out_buf_validate(struct vb2_buffer *vb) > +{ > + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); > + > + vbuf->field = V4L2_FIELD_NONE; > + return 0; > +} > + > +static struct vb2_ops mtk_vdec_request_vb2_ops = { > + .queue_setup = vb2ops_vdec_queue_setup, > + .buf_prepare = vb2ops_vdec_buf_prepare, Should be with the other .buf ops. > + .wait_prepare = vb2_ops_wait_prepare, > + .wait_finish = vb2_ops_wait_finish, > + .start_streaming = vb2ops_vdec_start_streaming, > + > + .buf_queue = vb2ops_vdec_stateless_buf_queue, > + .buf_out_validate = vb2ops_vdec_out_buf_validate, > + .buf_init = vb2ops_vdec_buf_init, > + .buf_finish = vb2ops_vdec_buf_finish, > + .stop_streaming = vb2ops_vdec_stop_streaming, Shouldn't stop_streaming be just after .start_streaming? It's weird to see them separated by other ops. > + .buf_request_complete = vb2ops_vdec_buf_request_complete, > +}; > + > +const struct mtk_vcodec_dec_pdata mtk_vdec_8183_pdata = { > + .chip = MTK_MT8183, > + .init_vdec_params = mtk_init_vdec_params, > + .ctrls_setup = mtk_vcodec_dec_ctrls_setup, > + .vdec_vb2_ops = &mtk_vdec_request_vb2_ops, > + .vdec_formats = mtk_video_formats, > + .num_formats = NUM_FORMATS, > + .default_out_fmt = &mtk_video_formats[DEFAULT_OUT_FMT_IDX], > + .default_cap_fmt = &mtk_video_formats[DEFAULT_CAP_FMT_IDX], > + .vdec_framesizes = mtk_vdec_framesizes, > + .num_framesizes = NUM_SUPPORTED_FRAMESIZE, > + .uses_stateless_api = true, > + .worker = mtk_vdec_worker, > + .flush_decoder = mtk_vdec_flush_decoder, > +}; > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > index e5b8309785e1..78d4a7728ddf 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > @@ -250,6 +250,7 @@ struct vdec_pic_info { > * @encode_work: worker for the encoding > * @last_decoded_picinfo: pic information get from latest decode > * @empty_flush_buf: a fake size-0 capture buffer that indicates flush > + * @current_codec: current set input codec, in V4L2 pixel format > * > * @colorspace: enum v4l2_colorspace; supplemental to pixelformat > * @ycbcr_enc: enum v4l2_ycbcr_encoding, Y'CbCr encoding > @@ -289,6 +290,8 @@ struct mtk_vcodec_ctx { > struct vdec_pic_info last_decoded_picinfo; > struct mtk_video_dec_buf *empty_flush_buf; > > + u32 current_codec; > + > enum v4l2_colorspace colorspace; > enum v4l2_ycbcr_encoding ycbcr_enc; > enum v4l2_quantization quantization; > Regards, Hans
On 27/04/2021 13:15, Alexandre Courbot wrote: > From: Yunfei Dong <yunfei.dong@mediatek.com> > > The stateless API requires a media device for issuing requests. Add one > if we are being instantiated as a stateless decoder. Why for the stateless decoder only? Why not create one for all? It's not a blocker, but I would recommend looking at this. Regards, Hans > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > [acourbot: refactor, cleanup and split] > Co-developed-by: Alexandre Courbot <acourbot@chromium.org> > Signed-off-by: Alexandre Courbot <acourbot@chromium.org> > --- > drivers/media/platform/Kconfig | 1 + > .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 39 +++++++++++++++++++ > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 2 + > 3 files changed, 42 insertions(+) > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index ae1468aa1b4e..4154fdec2efb 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -315,6 +315,7 @@ config VIDEO_MEDIATEK_VCODEC > select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU > select VIDEO_MEDIATEK_VCODEC_SCP if MTK_SCP > select V4L2_H264 > + select MEDIA_CONTROLLER > help > Mediatek video codec driver provides HW capability to > encode and decode in a range of video formats on MT8173 > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > index 533781d4680a..e942e28f96fe 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > @@ -14,6 +14,7 @@ > #include <media/v4l2-event.h> > #include <media/v4l2-mem2mem.h> > #include <media/videobuf2-dma-contig.h> > +#include <media/v4l2-device.h> > > #include "mtk_vcodec_drv.h" > #include "mtk_vcodec_dec.h" > @@ -324,6 +325,31 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > goto err_event_workq; > } > > + if (dev->vdec_pdata->uses_stateless_api) { > + dev->mdev_dec.dev = &pdev->dev; > + strscpy(dev->mdev_dec.model, MTK_VCODEC_DEC_NAME, > + sizeof(dev->mdev_dec.model)); > + > + media_device_init(&dev->mdev_dec); > + dev->mdev_dec.ops = &mtk_vcodec_media_ops; > + dev->v4l2_dev.mdev = &dev->mdev_dec; > + > + ret = v4l2_m2m_register_media_controller(dev->m2m_dev_dec, > + dev->vfd_dec, MEDIA_ENT_F_PROC_VIDEO_DECODER); > + if (ret) { > + mtk_v4l2_err("Failed to register media controller"); > + goto err_reg_cont; > + } > + > + ret = media_device_register(&dev->mdev_dec); > + if (ret) { > + mtk_v4l2_err("Failed to register media device"); > + goto err_media_reg; > + } > + > + mtk_v4l2_debug(0, "media registered as /dev/media%d", > + vfd_dec->num); > + } > ret = video_register_device(vfd_dec, VFL_TYPE_VIDEO, 0); > if (ret) { > mtk_v4l2_err("Failed to register video device"); > @@ -336,6 +362,12 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > return 0; > > err_dec_reg: > + if (dev->vdec_pdata->uses_stateless_api) > + media_device_unregister(&dev->mdev_dec); > +err_media_reg: > + if (dev->vdec_pdata->uses_stateless_api) > + v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec); > +err_reg_cont: > destroy_workqueue(dev->decode_workqueue); > err_event_workq: > v4l2_m2m_release(dev->m2m_dev_dec); > @@ -368,6 +400,13 @@ static int mtk_vcodec_dec_remove(struct platform_device *pdev) > > flush_workqueue(dev->decode_workqueue); > destroy_workqueue(dev->decode_workqueue); > + > + if (media_devnode_is_registered(dev->mdev_dec.devnode)) { > + media_device_unregister(&dev->mdev_dec); > + v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec); > + media_device_cleanup(&dev->mdev_dec); > + } > + > if (dev->m2m_dev_dec) > v4l2_m2m_release(dev->m2m_dev_dec); > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > index 78d4a7728ddf..10edd238c939 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > @@ -383,6 +383,7 @@ struct mtk_vcodec_enc_pdata { > * struct mtk_vcodec_dev - driver data > * @v4l2_dev: V4L2 device to register video devices for. > * @vfd_dec: Video device for decoder > + * @mdev_dec: Media device for decoder > * @vfd_enc: Video device for encoder. > * > * @m2m_dev_dec: m2m device for decoder > @@ -418,6 +419,7 @@ struct mtk_vcodec_enc_pdata { > struct mtk_vcodec_dev { > struct v4l2_device v4l2_dev; > struct video_device *vfd_dec; > + struct media_device mdev_dec; > struct video_device *vfd_enc; > > struct v4l2_m2m_dev *m2m_dev_dec; >
On 27/04/2021 13:15, Alexandre Courbot wrote: > This series adds support for the stateless API into mtk-vcodec, by first > separating the stateful ops into their own source file, and introducing > a new set of ops suitable for stateless decoding. As such, support for > stateful decoders should remain completely unaffected. > > This series has been tested with both MT8183 and MT8173. Decoding was > working for both chips, and in the case of MT8173 no regression has been > noticed. > > Patches 1-9 add MT8183 support to the decoder using the stateless API. > MT8183 only support H.264 acceleration. > > Patches 10-15 are follow-ups that further improve compliance for the > decoder and encoder, by fixing support for commands on both. Patch 11 > also makes sure that supported H.264 profiles are exported on MT8173. For a v5 I would recommend that - where possible - these 'improve compliance' patches are moved to the beginning of the series. That way they can be picked up quickly without having to wait for the whole series to be accepted. Regards, Hans > > Changes since v3: > * Stop checking that controls are set for every request. > * Add V4L2_CID_STATELESS_H264_START_CODE control. > * Stop mapping OUTPUT buffers and getting the NAL type from them, use the > nal_ref_idc field instead. > * Make V4L2_CID_MIN_BUFFERS_FOR_CAPTURE control stateful-only. > * Set vb2_buffer's field to V4L2_FIELD_NONE in buffer validation hook. > > Changes since v2: > * Add follow-up patches fixing support for START/STOP commands for the > encoder, and stateful decoder. > > Alexandre Courbot (8): > media: mtk-vcodec: vdec: handle firmware version field > media: mtk-vcodec: support version 2 of decoder firmware ABI > media: add Mediatek's MM21 format > dt-bindings: media: document mediatek,mt8183-vcodec-dec > media: mtk-vcodec: vdec: use helpers in VIDIOC_(TRY_)DECODER_CMD > media: mtk-vcodec: vdec: clamp OUTPUT resolution to hardware limits > media: mtk-vcodec: make flush buffer reusable by encoder > media: mtk-vcodec: venc: support START and STOP commands > > Hirokazu Honda (1): > media: mtk-vcodec: vdec: Support H264 profile control > > Hsin-Yi Wang (1): > media: mtk-vcodec: venc: make sure buffer exists in list before > removing > > Yunfei Dong (5): > media: mtk-vcodec: vdec: move stateful ops into their own file > media: mtk-vcodec: vdec: support stateless API > media: mtk-vcodec: vdec: support stateless H.264 decoding > media: mtk-vcodec: vdec: add media device if using stateless api > media: mtk-vcodec: enable MT8183 decoder > > .../bindings/media/mediatek-vcodec.txt | 1 + > .../media/v4l/pixfmt-reserved.rst | 7 + > drivers/media/platform/Kconfig | 2 + > drivers/media/platform/mtk-vcodec/Makefile | 3 + > .../platform/mtk-vcodec/mtk_vcodec_dec.c | 818 +++--------------- > .../platform/mtk-vcodec/mtk_vcodec_dec.h | 28 +- > .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 66 +- > .../mtk-vcodec/mtk_vcodec_dec_stateful.c | 667 ++++++++++++++ > .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 370 ++++++++ > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 58 +- > .../platform/mtk-vcodec/mtk_vcodec_enc.c | 135 ++- > .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 4 + > .../mtk-vcodec/vdec/vdec_h264_req_if.c | 780 +++++++++++++++++ > .../media/platform/mtk-vcodec/vdec_drv_if.c | 3 + > .../media/platform/mtk-vcodec/vdec_drv_if.h | 1 + > .../media/platform/mtk-vcodec/vdec_ipi_msg.h | 23 +- > .../media/platform/mtk-vcodec/vdec_vpu_if.c | 43 +- > .../media/platform/mtk-vcodec/vdec_vpu_if.h | 5 + > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > include/uapi/linux/videodev2.h | 1 + > 20 files changed, 2293 insertions(+), 723 deletions(-) > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateful.c > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > create mode 100644 drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c > > -- > 2.31.1.498.g6c1eba8ee3d-goog >
Hi, On 27.04.21 13:15, Alexandre Courbot wrote: > This series adds support for the stateless API into mtk-vcodec, by first > separating the stateful ops into their own source file, and introducing > a new set of ops suitable for stateless decoding. As such, support for > stateful decoders should remain completely unaffected. > > This series has been tested with both MT8183 and MT8173. Decoding was > working for both chips, and in the case of MT8173 no regression has been > noticed. I am trying to test the decoder with this patchset using v4l2-ctl on mt8173. I am trying to decode an h264 file into V4L2_PIX_FMT_MT21C format. I am not able to do it. It seems that the driver expects each buffer to start with a nal starting code, and the v4l2-ctl command just read the files into buffers without any parsing. Can you share the command and the files you used for testing? Thank you, Dafna > > Patches 1-9 add MT8183 support to the decoder using the stateless API. > MT8183 only support H.264 acceleration. > > Patches 10-15 are follow-ups that further improve compliance for the > decoder and encoder, by fixing support for commands on both. Patch 11 > also makes sure that supported H.264 profiles are exported on MT8173. > > Changes since v3: > * Stop checking that controls are set for every request. > * Add V4L2_CID_STATELESS_H264_START_CODE control. > * Stop mapping OUTPUT buffers and getting the NAL type from them, use the > nal_ref_idc field instead. > * Make V4L2_CID_MIN_BUFFERS_FOR_CAPTURE control stateful-only. > * Set vb2_buffer's field to V4L2_FIELD_NONE in buffer validation hook. > > Changes since v2: > * Add follow-up patches fixing support for START/STOP commands for the > encoder, and stateful decoder. > > Alexandre Courbot (8): > media: mtk-vcodec: vdec: handle firmware version field > media: mtk-vcodec: support version 2 of decoder firmware ABI > media: add Mediatek's MM21 format > dt-bindings: media: document mediatek,mt8183-vcodec-dec > media: mtk-vcodec: vdec: use helpers in VIDIOC_(TRY_)DECODER_CMD > media: mtk-vcodec: vdec: clamp OUTPUT resolution to hardware limits > media: mtk-vcodec: make flush buffer reusable by encoder > media: mtk-vcodec: venc: support START and STOP commands > > Hirokazu Honda (1): > media: mtk-vcodec: vdec: Support H264 profile control > > Hsin-Yi Wang (1): > media: mtk-vcodec: venc: make sure buffer exists in list before > removing > > Yunfei Dong (5): > media: mtk-vcodec: vdec: move stateful ops into their own file > media: mtk-vcodec: vdec: support stateless API > media: mtk-vcodec: vdec: support stateless H.264 decoding > media: mtk-vcodec: vdec: add media device if using stateless api > media: mtk-vcodec: enable MT8183 decoder > > .../bindings/media/mediatek-vcodec.txt | 1 + > .../media/v4l/pixfmt-reserved.rst | 7 + > drivers/media/platform/Kconfig | 2 + > drivers/media/platform/mtk-vcodec/Makefile | 3 + > .../platform/mtk-vcodec/mtk_vcodec_dec.c | 818 +++--------------- > .../platform/mtk-vcodec/mtk_vcodec_dec.h | 28 +- > .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 66 +- > .../mtk-vcodec/mtk_vcodec_dec_stateful.c | 667 ++++++++++++++ > .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 370 ++++++++ > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 58 +- > .../platform/mtk-vcodec/mtk_vcodec_enc.c | 135 ++- > .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 4 + > .../mtk-vcodec/vdec/vdec_h264_req_if.c | 780 +++++++++++++++++ > .../media/platform/mtk-vcodec/vdec_drv_if.c | 3 + > .../media/platform/mtk-vcodec/vdec_drv_if.h | 1 + > .../media/platform/mtk-vcodec/vdec_ipi_msg.h | 23 +- > .../media/platform/mtk-vcodec/vdec_vpu_if.c | 43 +- > .../media/platform/mtk-vcodec/vdec_vpu_if.h | 5 + > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > include/uapi/linux/videodev2.h | 1 + > 20 files changed, 2293 insertions(+), 723 deletions(-) > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateful.c > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > create mode 100644 drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c > > -- > 2.31.1.498.g6c1eba8ee3d-goog >
On 27.04.21 13:15, Alexandre Courbot wrote: > From: Yunfei Dong <yunfei.dong@mediatek.com> > > Support the stateless codec API that will be used by MT8183. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > [acourbot: refactor, cleanup and split] > Co-developed-by: Alexandre Courbot <acourbot@chromium.org> > Signed-off-by: Alexandre Courbot <acourbot@chromium.org> > --- > drivers/media/platform/mtk-vcodec/Makefile | 1 + > .../platform/mtk-vcodec/mtk_vcodec_dec.c | 66 +++- > .../platform/mtk-vcodec/mtk_vcodec_dec.h | 9 +- > .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 370 ++++++++++++++++++ > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 + > 5 files changed, 446 insertions(+), 3 deletions(-) > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > > diff --git a/drivers/media/platform/mtk-vcodec/Makefile b/drivers/media/platform/mtk-vcodec/Makefile > index 9c3cbb5b800e..4ba93d838ab6 100644 > --- a/drivers/media/platform/mtk-vcodec/Makefile > +++ b/drivers/media/platform/mtk-vcodec/Makefile > @@ -12,6 +12,7 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \ > vdec_vpu_if.o \ > mtk_vcodec_dec.o \ > mtk_vcodec_dec_stateful.o \ > + mtk_vcodec_dec_stateless.o \ > mtk_vcodec_dec_pm.o \ > > mtk-vcodec-enc-y := venc/venc_vp8_if.o \ > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c > index 4ad2662a43b2..01c5333d6cff 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c > @@ -46,6 +46,13 @@ static struct mtk_q_data *mtk_vdec_get_q_data(struct mtk_vcodec_ctx *ctx, > static int vidioc_try_decoder_cmd(struct file *file, void *priv, > struct v4l2_decoder_cmd *cmd) > { > + struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv); > + > + /* Use M2M stateless helper if relevant */ > + if (ctx->dev->vdec_pdata->uses_stateless_api) > + return v4l2_m2m_ioctl_stateless_try_decoder_cmd(file, priv, > + cmd); > + > switch (cmd->cmd) { > case V4L2_DEC_CMD_STOP: > case V4L2_DEC_CMD_START: > @@ -72,6 +79,10 @@ static int vidioc_decoder_cmd(struct file *file, void *priv, > if (ret) > return ret; > > + /* Use M2M stateless helper if relevant */ > + if (ctx->dev->vdec_pdata->uses_stateless_api) > + return v4l2_m2m_ioctl_stateless_decoder_cmd(file, priv, cmd); > + > mtk_v4l2_debug(1, "decoder cmd=%u", cmd->cmd); > dst_vq = v4l2_m2m_get_vq(ctx->m2m_ctx, > V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > @@ -414,7 +425,8 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv, > * Setting OUTPUT format after OUTPUT buffers are allocated is invalid > * if using the stateful API. > */ > - if ((f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) && > + if (!dec_pdata->uses_stateless_api && > + (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) && > vb2_is_busy(&ctx->m2m_ctx->out_q_ctx.q)) { > mtk_v4l2_err("out_q_ctx buffers already requested"); > ret = -EBUSY; > @@ -457,6 +469,7 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv, > ctx->quantization = pix_mp->quantization; > ctx->xfer_func = pix_mp->xfer_func; > > + ctx->current_codec = fmt->fourcc; > if (ctx->state == MTK_STATE_FREE) { > ret = vdec_if_init(ctx, q_data->fmt->fourcc); > if (ret) { > @@ -468,6 +481,49 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv, > } > } > > + /* > + * If using the stateless API, S_FMT should have the effect of setting > + * the CAPTURE queue resolution no matter which queue it was called on. > + */ > + if (dec_pdata->uses_stateless_api) { > + ctx->picinfo.pic_w = pix_mp->width; > + ctx->picinfo.pic_h = pix_mp->height; > + > + ret = vdec_if_get_param(ctx, GET_PARAM_PIC_INFO, &ctx->picinfo); > + if (ret) { > + mtk_v4l2_err("[%d]Error!! Get GET_PARAM_PICTURE_INFO Fail", > + ctx->id); > + return -EINVAL; > + } > + > + ctx->last_decoded_picinfo = ctx->picinfo; > + > + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 1) { > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] = > + ctx->picinfo.fb_sz[0] + > + ctx->picinfo.fb_sz[1]; > + ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] = > + ctx->picinfo.buf_w; > + } else { > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] = > + ctx->picinfo.fb_sz[0]; > + ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] = > + ctx->picinfo.buf_w; > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[1] = > + ctx->picinfo.fb_sz[1]; > + ctx->q_data[MTK_Q_DATA_DST].bytesperline[1] = > + ctx->picinfo.buf_w; > + } > + > + ctx->q_data[MTK_Q_DATA_DST].coded_width = ctx->picinfo.buf_w; > + ctx->q_data[MTK_Q_DATA_DST].coded_height = ctx->picinfo.buf_h; > + mtk_v4l2_debug(2, "[%d] vdec_if_init() num_plane = %d wxh=%dx%d pic wxh=%dx%d sz[0]=0x%x sz[1]=0x%x", > + ctx->id, pix_mp->num_planes, > + ctx->picinfo.buf_w, ctx->picinfo.buf_h, > + ctx->picinfo.pic_w, ctx->picinfo.pic_h, > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[0], > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[1]); > + } > return 0; > } > > @@ -765,9 +821,15 @@ void vb2ops_vdec_stop_streaming(struct vb2_queue *q) > while ((src_buf = v4l2_m2m_src_buf_remove(ctx->m2m_ctx))) { > struct mtk_video_dec_buf *buf_info = container_of( > src_buf, struct mtk_video_dec_buf, m2m_buf.vb); > - if (!buf_info->lastframe) > + if (!buf_info->lastframe) { > + struct media_request *req = > + src_buf->vb2_buf.req_obj.req; > v4l2_m2m_buf_done(src_buf, > VB2_BUF_STATE_ERROR); > + if (req) > + v4l2_ctrl_request_complete(req, > + &ctx->ctrl_hdl); > + } > } > return; > } > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h > index 6a18cb3bfe07..6b29d7d9ae15 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h > @@ -45,6 +45,7 @@ struct vdec_fb { > * @lastframe: Intput buffer is last buffer - EOS > * @error: An unrecoverable error occurs on this buffer. > * @frame_buffer: Decode status, and buffer information of Capture buffer > + * @bs_buffer: Output buffer info > * > * Note : These status information help us track and debug buffer state > */ > @@ -55,12 +56,18 @@ struct mtk_video_dec_buf { > bool queued_in_vb2; > bool queued_in_v4l2; > bool lastframe; > + > bool error; > - struct vdec_fb frame_buffer; > + > + union { > + struct vdec_fb frame_buffer; > + struct mtk_vcodec_mem bs_buffer; > + }; > }; > > extern const struct v4l2_ioctl_ops mtk_vdec_ioctl_ops; > extern const struct v4l2_m2m_ops mtk_vdec_m2m_ops; > +extern const struct media_device_ops mtk_vcodec_media_ops; > > > /* > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > new file mode 100644 > index 000000000000..75ddf53e2876 > --- /dev/null > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c Hi, when compiling with -DDEBUG flag, I got some compilation errors for this file. Thanks, Dafna > @@ -0,0 +1,370 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include "media/videobuf2-v4l2.h" > +#include <media/videobuf2-dma-contig.h> > +#include <media/v4l2-event.h> > +#include <media/v4l2-mem2mem.h> > +#include <linux/module.h> > + > +#include "mtk_vcodec_drv.h" > +#include "mtk_vcodec_dec.h" > +#include "mtk_vcodec_intr.h" > +#include "mtk_vcodec_util.h" > +#include "vdec_drv_if.h" > +#include "mtk_vcodec_dec_pm.h" > + > +/** > + * struct mtk_stateless_control - CID control type > + * @cfg: control configuration > + * @codec_type: codec type (V4L2 pixel format) for CID control type > + */ > +struct mtk_stateless_control { > + struct v4l2_ctrl_config cfg; > + int codec_type; > +}; > + > +static const struct mtk_stateless_control mtk_stateless_controls[] = { > + { > + .cfg = { > + .id = V4L2_CID_STATELESS_H264_SPS, > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + }, > + { > + .cfg = { > + .id = V4L2_CID_STATELESS_H264_PPS, > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + }, > + { > + .cfg = { > + .id = V4L2_CID_STATELESS_H264_SCALING_MATRIX, > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + }, > + { > + .cfg = { > + .id = V4L2_CID_STATELESS_H264_DECODE_PARAMS, > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + }, > + { > + .cfg = { > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, > + .menu_skip_mask = > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) | > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + }, > + { > + .cfg = { > + .id = V4L2_CID_STATELESS_H264_DECODE_MODE, > + .min = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED, > + .def = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED, > + .max = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED, > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + }, > + { > + .cfg = { > + .id = V4L2_CID_STATELESS_H264_START_CODE, > + .min = V4L2_STATELESS_H264_START_CODE_ANNEX_B, > + .def = V4L2_STATELESS_H264_START_CODE_ANNEX_B, > + .max = V4L2_STATELESS_H264_START_CODE_ANNEX_B, > + }, > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > + } > +}; > +#define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls) > + > +static const struct mtk_video_fmt mtk_video_formats[] = { > + { > + .fourcc = V4L2_PIX_FMT_H264_SLICE, > + .type = MTK_FMT_DEC, > + .num_planes = 1, > + }, > + { > + .fourcc = V4L2_PIX_FMT_MM21, > + .type = MTK_FMT_FRAME, > + .num_planes = 2, > + }, > +}; > +#define NUM_FORMATS ARRAY_SIZE(mtk_video_formats) > +#define DEFAULT_OUT_FMT_IDX 0 > +#define DEFAULT_CAP_FMT_IDX 1 > + > +static const struct mtk_codec_framesizes mtk_vdec_framesizes[] = { > + { > + .fourcc = V4L2_PIX_FMT_H264_SLICE, > + .stepwise = { MTK_VDEC_MIN_W, MTK_VDEC_MAX_W, 16, > + MTK_VDEC_MIN_H, MTK_VDEC_MAX_H, 16 }, > + }, > +}; > + > +#define NUM_SUPPORTED_FRAMESIZE ARRAY_SIZE(mtk_vdec_framesizes) > + > +static void mtk_vdec_stateless_set_dst_payload(struct mtk_vcodec_ctx *ctx, > + struct vdec_fb *fb) > +{ > + struct mtk_video_dec_buf *vdec_frame_buf = > + container_of(fb, struct mtk_video_dec_buf, frame_buffer); > + struct vb2_v4l2_buffer *vb = &vdec_frame_buf->m2m_buf.vb; > + unsigned int cap_y_size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[0]; > + > + vb2_set_plane_payload(&vb->vb2_buf, 0, cap_y_size); > + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2) { > + unsigned int cap_c_size = > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[1]; > + > + vb2_set_plane_payload(&vb->vb2_buf, 1, cap_c_size); > + } > +} > + > +static struct vdec_fb *vdec_get_cap_buffer(struct mtk_vcodec_ctx *ctx, > + struct vb2_v4l2_buffer *vb2_v4l2) > +{ > + struct mtk_video_dec_buf *framebuf = > + container_of(vb2_v4l2, struct mtk_video_dec_buf, m2m_buf.vb); > + struct vdec_fb *pfb = &framebuf->frame_buffer; > + struct vb2_buffer *dst_buf = &vb2_v4l2->vb2_buf; > + > + pfb = &framebuf->frame_buffer; > + pfb->base_y.va = NULL; > + pfb->base_y.dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0); > + pfb->base_y.size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[0]; > + > + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2) { > + pfb->base_c.va = NULL; > + pfb->base_c.dma_addr = > + vb2_dma_contig_plane_dma_addr(dst_buf, 1); > + pfb->base_c.size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[1]; > + } > + mtk_v4l2_debug(1, > + "id=%d Framebuf pfb=%p VA=%p Y_DMA=%pad C_DMA=%pad Size=%zx frame_count = %d", > + dst_buf->index, pfb, > + pfb->base_y.va, &pfb->base_y.dma_addr, > + &pfb->base_c.dma_addr, pfb->base_y.size, > + ctx->decoded_frame_cnt); > + > + return pfb; > +} > + > +static void vb2ops_vdec_buf_request_complete(struct vb2_buffer *vb) > +{ > + struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); > + > + v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->ctrl_hdl); > +} > + > +static int fops_media_request_validate(struct media_request *mreq) > +{ > + const unsigned int buffer_cnt = vb2_request_buffer_cnt(mreq); > + > + switch (buffer_cnt) { > + case 1: > + /* We expect exactly one buffer with the request */ > + break; > + case 0: > + mtk_v4l2_err("No buffer provided with the request"); > + return -ENOENT; > + default: > + mtk_v4l2_err("Too many buffers (%d) provided with the request", > + buffer_cnt); > + return -EINVAL; > + } > + > + return vb2_request_validate(mreq); > +} > + > +static void mtk_vdec_worker(struct work_struct *work) > +{ > + struct mtk_vcodec_ctx *ctx = > + container_of(work, struct mtk_vcodec_ctx, decode_work); > + struct mtk_vcodec_dev *dev = ctx->dev; > + struct vb2_v4l2_buffer *vb2_v4l2_src, *vb2_v4l2_dst; > + struct vb2_buffer *vb2_src; > + struct mtk_vcodec_mem *bs_src; > + struct mtk_video_dec_buf *dec_buf_src; > + struct media_request *src_buf_req; > + struct vdec_fb *dst_buf; > + bool res_chg = false; > + int ret; > + > + vb2_v4l2_src = v4l2_m2m_next_src_buf(ctx->m2m_ctx); > + if (vb2_v4l2_src == NULL) { > + v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx); > + mtk_v4l2_debug(1, "[%d] no available source buffer", ctx->id); > + return; > + } > + > + vb2_v4l2_dst = v4l2_m2m_next_dst_buf(ctx->m2m_ctx); > + if (vb2_v4l2_dst == NULL) { > + v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx); > + mtk_v4l2_debug(1, "[%d] no available destination buffer", ctx->id); > + return; > + } > + > + vb2_src = &vb2_v4l2_src->vb2_buf; > + dec_buf_src = container_of(vb2_v4l2_src, struct mtk_video_dec_buf, > + m2m_buf.vb); > + bs_src = &dec_buf_src->bs_buffer; > + > + mtk_v4l2_debug(3, "[%d] (%d) id=%d, vb=%p buf_info = %p", > + ctx->id, src_buf->vb2_queue->type, > + src_buf->index, src_buf, src_buf_info); > + > + bs_src->va = NULL; > + bs_src->dma_addr = vb2_dma_contig_plane_dma_addr(vb2_src, 0); > + bs_src->size = (size_t)vb2_src->planes[0].bytesused; > + > + mtk_v4l2_debug(3, "[%d] Bitstream VA=%p DMA=%pad Size=%zx vb=%p", > + ctx->id, buf->va, &buf->dma_addr, buf->size, src_buf); > + /* Apply request controls. */ > + src_buf_req = vb2_src->req_obj.req; > + if (src_buf_req) > + v4l2_ctrl_request_setup(src_buf_req, &ctx->ctrl_hdl); > + else > + mtk_v4l2_err("vb2 buffer media request is NULL"); > + > + dst_buf = vdec_get_cap_buffer(ctx, vb2_v4l2_dst); > + v4l2_m2m_buf_copy_metadata(vb2_v4l2_src, vb2_v4l2_dst, true); > + ret = vdec_if_decode(ctx, bs_src, dst_buf, &res_chg); > + if (ret) { > + mtk_v4l2_err( > + " <===[%d], src_buf[%d] sz=0x%zx pts=%llu vdec_if_decode() ret=%d res_chg=%d===>", > + ctx->id, vb2_src->index, bs_src->size, > + vb2_src->timestamp, ret, res_chg); > + if (ret == -EIO) { > + mutex_lock(&ctx->lock); > + dec_buf_src->error = true; > + mutex_unlock(&ctx->lock); > + } > + } > + > + mtk_vdec_stateless_set_dst_payload(ctx, dst_buf); > + > + v4l2_m2m_buf_done_and_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx, > + ret ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE); > + > + v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl); > +} > + > +static void vb2ops_vdec_stateless_buf_queue(struct vb2_buffer *vb) > +{ > + struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); > + struct vb2_v4l2_buffer *vb2_v4l2 = to_vb2_v4l2_buffer(vb); > + > + mtk_v4l2_debug(3, "[%d] (%d) id=%d, vb=%p", > + ctx->id, vb->vb2_queue->type, > + vb->index, vb); > + > + mutex_lock(&ctx->lock); > + v4l2_m2m_buf_queue(ctx->m2m_ctx, vb2_v4l2); > + mutex_unlock(&ctx->lock); > + if (vb->vb2_queue->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) > + return; > + > + mtk_v4l2_debug(3, "(%d) id=%d, bs=%p", > + vb->vb2_queue->type, vb->index, src_buf); > + > + /* If an OUTPUT buffer, we may need to update the state */ > + if (ctx->state == MTK_STATE_INIT) { > + ctx->state = MTK_STATE_HEADER; > + mtk_v4l2_debug(1, "Init driver from init to header."); > + } else { > + mtk_v4l2_debug(3, "[%d] already init driver %d", > + ctx->id, ctx->state); > + } > +} > + > +static int mtk_vdec_flush_decoder(struct mtk_vcodec_ctx *ctx) > +{ > + bool res_chg; > + > + return vdec_if_decode(ctx, NULL, NULL, &res_chg); > +} > + > +static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_ctx *ctx) > +{ > + unsigned int i; > + > + v4l2_ctrl_handler_init(&ctx->ctrl_hdl, NUM_CTRLS); > + if (ctx->ctrl_hdl.error) { > + mtk_v4l2_err("v4l2_ctrl_handler_init failed\n"); > + return ctx->ctrl_hdl.error; > + } > + > + for (i = 0; i < NUM_CTRLS; i++) { > + struct v4l2_ctrl_config cfg = mtk_stateless_controls[i].cfg; > + > + v4l2_ctrl_new_custom(&ctx->ctrl_hdl, &cfg, NULL); > + if (ctx->ctrl_hdl.error) { > + mtk_v4l2_err("Adding control %d failed %d", > + i, ctx->ctrl_hdl.error); > + return ctx->ctrl_hdl.error; > + } > + } > + > + v4l2_ctrl_handler_setup(&ctx->ctrl_hdl); > + > + return 0; > +} > + > +const struct media_device_ops mtk_vcodec_media_ops = { > + .req_validate = fops_media_request_validate, > + .req_queue = v4l2_m2m_request_queue, > +}; > + > +static void mtk_init_vdec_params(struct mtk_vcodec_ctx *ctx) > +{ > + struct vb2_queue *src_vq; > + > + src_vq = v4l2_m2m_get_vq(ctx->m2m_ctx, > + V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); > + > + /* Support request api for output plane */ > + src_vq->supports_requests = true; > + src_vq->requires_requests = true; > +} > + > +static int vb2ops_vdec_out_buf_validate(struct vb2_buffer *vb) > +{ > + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); > + > + vbuf->field = V4L2_FIELD_NONE; > + return 0; > +} > + > +static struct vb2_ops mtk_vdec_request_vb2_ops = { > + .queue_setup = vb2ops_vdec_queue_setup, > + .buf_prepare = vb2ops_vdec_buf_prepare, > + .wait_prepare = vb2_ops_wait_prepare, > + .wait_finish = vb2_ops_wait_finish, > + .start_streaming = vb2ops_vdec_start_streaming, > + > + .buf_queue = vb2ops_vdec_stateless_buf_queue, > + .buf_out_validate = vb2ops_vdec_out_buf_validate, > + .buf_init = vb2ops_vdec_buf_init, > + .buf_finish = vb2ops_vdec_buf_finish, > + .stop_streaming = vb2ops_vdec_stop_streaming, > + .buf_request_complete = vb2ops_vdec_buf_request_complete, > +}; > + > +const struct mtk_vcodec_dec_pdata mtk_vdec_8183_pdata = { > + .chip = MTK_MT8183, > + .init_vdec_params = mtk_init_vdec_params, > + .ctrls_setup = mtk_vcodec_dec_ctrls_setup, > + .vdec_vb2_ops = &mtk_vdec_request_vb2_ops, > + .vdec_formats = mtk_video_formats, > + .num_formats = NUM_FORMATS, > + .default_out_fmt = &mtk_video_formats[DEFAULT_OUT_FMT_IDX], > + .default_cap_fmt = &mtk_video_formats[DEFAULT_CAP_FMT_IDX], > + .vdec_framesizes = mtk_vdec_framesizes, > + .num_framesizes = NUM_SUPPORTED_FRAMESIZE, > + .uses_stateless_api = true, > + .worker = mtk_vdec_worker, > + .flush_decoder = mtk_vdec_flush_decoder, > +}; > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > index e5b8309785e1..78d4a7728ddf 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > @@ -250,6 +250,7 @@ struct vdec_pic_info { > * @encode_work: worker for the encoding > * @last_decoded_picinfo: pic information get from latest decode > * @empty_flush_buf: a fake size-0 capture buffer that indicates flush > + * @current_codec: current set input codec, in V4L2 pixel format > * > * @colorspace: enum v4l2_colorspace; supplemental to pixelformat > * @ycbcr_enc: enum v4l2_ycbcr_encoding, Y'CbCr encoding > @@ -289,6 +290,8 @@ struct mtk_vcodec_ctx { > struct vdec_pic_info last_decoded_picinfo; > struct mtk_video_dec_buf *empty_flush_buf; > > + u32 current_codec; > + > enum v4l2_colorspace colorspace; > enum v4l2_ycbcr_encoding ycbcr_enc; > enum v4l2_quantization quantization; >
Hi Hans, thanks for the review! On Thu, Apr 29, 2021 at 4:28 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote: > > On 27/04/2021 13:15, Alexandre Courbot wrote: > > From: Yunfei Dong <yunfei.dong@mediatek.com> > > > > The stateless API requires a media device for issuing requests. Add one > > if we are being instantiated as a stateless decoder. > > Why for the stateless decoder only? Why not create one for all? > > It's not a blocker, but I would recommend looking at this. Would there be any use in creating a media device for a stateful decoder that does not need to use requests? Cheers, Alex.
On Thu, Apr 29, 2021 at 4:35 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote: > > On 27/04/2021 13:15, Alexandre Courbot wrote: > > This series adds support for the stateless API into mtk-vcodec, by first > > separating the stateful ops into their own source file, and introducing > > a new set of ops suitable for stateless decoding. As such, support for > > stateful decoders should remain completely unaffected. > > > > This series has been tested with both MT8183 and MT8173. Decoding was > > working for both chips, and in the case of MT8173 no regression has been > > noticed. > > > > Patches 1-9 add MT8183 support to the decoder using the stateless API. > > MT8183 only support H.264 acceleration. > > > > Patches 10-15 are follow-ups that further improve compliance for the > > decoder and encoder, by fixing support for commands on both. Patch 11 > > also makes sure that supported H.264 profiles are exported on MT8173. > > For a v5 I would recommend that - where possible - these 'improve compliance' > patches are moved to the beginning of the series. That way they can be picked > up quickly without having to wait for the whole series to be accepted. Makes sense, the current order reflects the chronology these patches have been written, but I agree that improving compliance should be merged first. Let me try to reorder things a bit. Cheers, Alex.
Hi Dafna, On Thu, Apr 29, 2021 at 9:07 PM Dafna Hirschfeld <dafna.hirschfeld@collabora.com> wrote: > > Hi, > > On 27.04.21 13:15, Alexandre Courbot wrote: > > This series adds support for the stateless API into mtk-vcodec, by first > > separating the stateful ops into their own source file, and introducing > > a new set of ops suitable for stateless decoding. As such, support for > > stateful decoders should remain completely unaffected. > > > > This series has been tested with both MT8183 and MT8173. Decoding was > > working for both chips, and in the case of MT8173 no regression has been > > noticed. > > I am trying to test the decoder with this patchset using v4l2-ctl on mt8173. > > I am trying to decode an h264 file into V4L2_PIX_FMT_MT21C format. > I am not able to do it. It seems that the driver expects each buffer to start > with a nal starting code, and the v4l2-ctl command just read the files into > buffers without any parsing. > > Can you share the command and the files you used for testing? I have been using the Chromium test suite (aka video_decode_accelerator_tests). I had doubts after reading your email but I tested the series again using this tool and confirmed it was working. Unfortunately this test is not easy to build, but maybe if you have a Chromium environment ready you can run it. mtk-vcodec does expect the input buffers to be split by NAL units (as per the spec [1] IIUC), so that would explain why v4l2-ctl failed (I assume that it also fails without this series?). Maybe ffmpeg can also be used to exercice this driver with properly split NAL units, but I am not familiar with its use with V4L2. I'm also not sure it would be happy about the MT21C format that the driver outputs, or that it could pick the MDP to convert it to something readable... Cheers, Alex. [1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/pixfmt-compressed.html#compressed-formats
Hi Hans, On Thu, Apr 29, 2021 at 4:27 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote: > > On 27/04/2021 13:15, Alexandre Courbot wrote: > > From: Yunfei Dong <yunfei.dong@mediatek.com> > > > > Support the stateless codec API that will be used by MT8183. > > > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > [acourbot: refactor, cleanup and split] > > Co-developed-by: Alexandre Courbot <acourbot@chromium.org> > > Signed-off-by: Alexandre Courbot <acourbot@chromium.org> > > --- > > drivers/media/platform/mtk-vcodec/Makefile | 1 + > > .../platform/mtk-vcodec/mtk_vcodec_dec.c | 66 +++- > > .../platform/mtk-vcodec/mtk_vcodec_dec.h | 9 +- > > .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 370 ++++++++++++++++++ > > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 + > > 5 files changed, 446 insertions(+), 3 deletions(-) > > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > > > > diff --git a/drivers/media/platform/mtk-vcodec/Makefile b/drivers/media/platform/mtk-vcodec/Makefile > > index 9c3cbb5b800e..4ba93d838ab6 100644 > > --- a/drivers/media/platform/mtk-vcodec/Makefile > > +++ b/drivers/media/platform/mtk-vcodec/Makefile > > @@ -12,6 +12,7 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \ > > vdec_vpu_if.o \ > > mtk_vcodec_dec.o \ > > mtk_vcodec_dec_stateful.o \ > > + mtk_vcodec_dec_stateless.o \ > > mtk_vcodec_dec_pm.o \ > > > > mtk-vcodec-enc-y := venc/venc_vp8_if.o \ > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c > > index 4ad2662a43b2..01c5333d6cff 100644 > > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c > > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c > > @@ -46,6 +46,13 @@ static struct mtk_q_data *mtk_vdec_get_q_data(struct mtk_vcodec_ctx *ctx, > > static int vidioc_try_decoder_cmd(struct file *file, void *priv, > > struct v4l2_decoder_cmd *cmd) > > { > > + struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv); > > + > > + /* Use M2M stateless helper if relevant */ > > + if (ctx->dev->vdec_pdata->uses_stateless_api) > > + return v4l2_m2m_ioctl_stateless_try_decoder_cmd(file, priv, > > + cmd); > > + > > switch (cmd->cmd) { > > case V4L2_DEC_CMD_STOP: > > case V4L2_DEC_CMD_START: > > @@ -72,6 +79,10 @@ static int vidioc_decoder_cmd(struct file *file, void *priv, > > if (ret) > > return ret; > > > > + /* Use M2M stateless helper if relevant */ > > + if (ctx->dev->vdec_pdata->uses_stateless_api) > > + return v4l2_m2m_ioctl_stateless_decoder_cmd(file, priv, cmd); > > + > > mtk_v4l2_debug(1, "decoder cmd=%u", cmd->cmd); > > dst_vq = v4l2_m2m_get_vq(ctx->m2m_ctx, > > V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > > @@ -414,7 +425,8 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv, > > * Setting OUTPUT format after OUTPUT buffers are allocated is invalid > > * if using the stateful API. > > */ > > - if ((f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) && > > + if (!dec_pdata->uses_stateless_api && > > + (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) && > > vb2_is_busy(&ctx->m2m_ctx->out_q_ctx.q)) { > > mtk_v4l2_err("out_q_ctx buffers already requested"); > > ret = -EBUSY; > > @@ -457,6 +469,7 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv, > > ctx->quantization = pix_mp->quantization; > > ctx->xfer_func = pix_mp->xfer_func; > > > > + ctx->current_codec = fmt->fourcc; > > if (ctx->state == MTK_STATE_FREE) { > > ret = vdec_if_init(ctx, q_data->fmt->fourcc); > > if (ret) { > > @@ -468,6 +481,49 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv, > > } > > } > > > > + /* > > + * If using the stateless API, S_FMT should have the effect of setting > > + * the CAPTURE queue resolution no matter which queue it was called on. > > + */ > > + if (dec_pdata->uses_stateless_api) { > > + ctx->picinfo.pic_w = pix_mp->width; > > + ctx->picinfo.pic_h = pix_mp->height; > > + > > + ret = vdec_if_get_param(ctx, GET_PARAM_PIC_INFO, &ctx->picinfo); > > + if (ret) { > > + mtk_v4l2_err("[%d]Error!! Get GET_PARAM_PICTURE_INFO Fail", > > + ctx->id); > > + return -EINVAL; > > + } > > + > > + ctx->last_decoded_picinfo = ctx->picinfo; > > + > > + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 1) { > > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] = > > + ctx->picinfo.fb_sz[0] + > > + ctx->picinfo.fb_sz[1]; > > + ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] = > > + ctx->picinfo.buf_w; > > + } else { > > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] = > > + ctx->picinfo.fb_sz[0]; > > + ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] = > > + ctx->picinfo.buf_w; > > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[1] = > > + ctx->picinfo.fb_sz[1]; > > + ctx->q_data[MTK_Q_DATA_DST].bytesperline[1] = > > + ctx->picinfo.buf_w; > > + } > > + > > + ctx->q_data[MTK_Q_DATA_DST].coded_width = ctx->picinfo.buf_w; > > + ctx->q_data[MTK_Q_DATA_DST].coded_height = ctx->picinfo.buf_h; > > + mtk_v4l2_debug(2, "[%d] vdec_if_init() num_plane = %d wxh=%dx%d pic wxh=%dx%d sz[0]=0x%x sz[1]=0x%x", > > + ctx->id, pix_mp->num_planes, > > + ctx->picinfo.buf_w, ctx->picinfo.buf_h, > > + ctx->picinfo.pic_w, ctx->picinfo.pic_h, > > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[0], > > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[1]); > > + } > > return 0; > > } > > > > @@ -765,9 +821,15 @@ void vb2ops_vdec_stop_streaming(struct vb2_queue *q) > > while ((src_buf = v4l2_m2m_src_buf_remove(ctx->m2m_ctx))) { > > struct mtk_video_dec_buf *buf_info = container_of( > > src_buf, struct mtk_video_dec_buf, m2m_buf.vb); > > - if (!buf_info->lastframe) > > + if (!buf_info->lastframe) { > > + struct media_request *req = > > + src_buf->vb2_buf.req_obj.req; > > v4l2_m2m_buf_done(src_buf, > > VB2_BUF_STATE_ERROR); > > + if (req) > > + v4l2_ctrl_request_complete(req, > > + &ctx->ctrl_hdl); > > + } > > } > > return; > > } > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h > > index 6a18cb3bfe07..6b29d7d9ae15 100644 > > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h > > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h > > @@ -45,6 +45,7 @@ struct vdec_fb { > > * @lastframe: Intput buffer is last buffer - EOS > > * @error: An unrecoverable error occurs on this buffer. > > * @frame_buffer: Decode status, and buffer information of Capture buffer > > + * @bs_buffer: Output buffer info > > * > > * Note : These status information help us track and debug buffer state > > */ > > @@ -55,12 +56,18 @@ struct mtk_video_dec_buf { > > bool queued_in_vb2; > > bool queued_in_v4l2; > > bool lastframe; > > + > > bool error; > > - struct vdec_fb frame_buffer; > > + > > + union { > > + struct vdec_fb frame_buffer; > > + struct mtk_vcodec_mem bs_buffer; > > + }; > > }; > > > > extern const struct v4l2_ioctl_ops mtk_vdec_ioctl_ops; > > extern const struct v4l2_m2m_ops mtk_vdec_m2m_ops; > > +extern const struct media_device_ops mtk_vcodec_media_ops; > > > > > > /* > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > > new file mode 100644 > > index 000000000000..75ddf53e2876 > > --- /dev/null > > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > > @@ -0,0 +1,370 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +#include "media/videobuf2-v4l2.h" > > +#include <media/videobuf2-dma-contig.h> > > +#include <media/v4l2-event.h> > > +#include <media/v4l2-mem2mem.h> > > +#include <linux/module.h> > > + > > +#include "mtk_vcodec_drv.h" > > +#include "mtk_vcodec_dec.h" > > +#include "mtk_vcodec_intr.h" > > +#include "mtk_vcodec_util.h" > > +#include "vdec_drv_if.h" > > +#include "mtk_vcodec_dec_pm.h" > > + > > +/** > > + * struct mtk_stateless_control - CID control type > > + * @cfg: control configuration > > + * @codec_type: codec type (V4L2 pixel format) for CID control type > > + */ > > +struct mtk_stateless_control { > > + struct v4l2_ctrl_config cfg; > > + int codec_type; > > +}; > > + > > +static const struct mtk_stateless_control mtk_stateless_controls[] = { > > + { > > + .cfg = { > > + .id = V4L2_CID_STATELESS_H264_SPS, > > + }, > > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > > + }, > > + { > > + .cfg = { > > + .id = V4L2_CID_STATELESS_H264_PPS, > > + }, > > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > > + }, > > + { > > + .cfg = { > > + .id = V4L2_CID_STATELESS_H264_SCALING_MATRIX, > > + }, > > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > > + }, > > + { > > + .cfg = { > > + .id = V4L2_CID_STATELESS_H264_DECODE_PARAMS, > > + }, > > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > > + }, > > + { > > + .cfg = { > > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE, > > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN, > > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, > > + .menu_skip_mask = > > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) | > > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED), > > + }, > > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > > + }, > > + { > > + .cfg = { > > + .id = V4L2_CID_STATELESS_H264_DECODE_MODE, > > + .min = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED, > > + .def = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED, > > + .max = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED, > > + }, > > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > > + }, > > + { > > + .cfg = { > > + .id = V4L2_CID_STATELESS_H264_START_CODE, > > + .min = V4L2_STATELESS_H264_START_CODE_ANNEX_B, > > + .def = V4L2_STATELESS_H264_START_CODE_ANNEX_B, > > + .max = V4L2_STATELESS_H264_START_CODE_ANNEX_B, > > + }, > > + .codec_type = V4L2_PIX_FMT_H264_SLICE, > > + } > > +}; > > +#define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls) > > + > > +static const struct mtk_video_fmt mtk_video_formats[] = { > > + { > > + .fourcc = V4L2_PIX_FMT_H264_SLICE, > > + .type = MTK_FMT_DEC, > > + .num_planes = 1, > > + }, > > + { > > + .fourcc = V4L2_PIX_FMT_MM21, > > + .type = MTK_FMT_FRAME, > > + .num_planes = 2, > > + }, > > +}; > > +#define NUM_FORMATS ARRAY_SIZE(mtk_video_formats) > > +#define DEFAULT_OUT_FMT_IDX 0 > > +#define DEFAULT_CAP_FMT_IDX 1 > > + > > +static const struct mtk_codec_framesizes mtk_vdec_framesizes[] = { > > + { > > + .fourcc = V4L2_PIX_FMT_H264_SLICE, > > + .stepwise = { MTK_VDEC_MIN_W, MTK_VDEC_MAX_W, 16, > > + MTK_VDEC_MIN_H, MTK_VDEC_MAX_H, 16 }, > > + }, > > +}; > > + > > +#define NUM_SUPPORTED_FRAMESIZE ARRAY_SIZE(mtk_vdec_framesizes) > > + > > +static void mtk_vdec_stateless_set_dst_payload(struct mtk_vcodec_ctx *ctx, > > + struct vdec_fb *fb) > > +{ > > + struct mtk_video_dec_buf *vdec_frame_buf = > > + container_of(fb, struct mtk_video_dec_buf, frame_buffer); > > + struct vb2_v4l2_buffer *vb = &vdec_frame_buf->m2m_buf.vb; > > + unsigned int cap_y_size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[0]; > > + > > + vb2_set_plane_payload(&vb->vb2_buf, 0, cap_y_size); > > + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2) { > > + unsigned int cap_c_size = > > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[1]; > > + > > + vb2_set_plane_payload(&vb->vb2_buf, 1, cap_c_size); > > + } > > +} > > + > > +static struct vdec_fb *vdec_get_cap_buffer(struct mtk_vcodec_ctx *ctx, > > + struct vb2_v4l2_buffer *vb2_v4l2) > > +{ > > + struct mtk_video_dec_buf *framebuf = > > + container_of(vb2_v4l2, struct mtk_video_dec_buf, m2m_buf.vb); > > + struct vdec_fb *pfb = &framebuf->frame_buffer; > > + struct vb2_buffer *dst_buf = &vb2_v4l2->vb2_buf; > > + > > + pfb = &framebuf->frame_buffer; > > + pfb->base_y.va = NULL; > > + pfb->base_y.dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0); > > + pfb->base_y.size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[0]; > > + > > + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2) { > > + pfb->base_c.va = NULL; > > + pfb->base_c.dma_addr = > > + vb2_dma_contig_plane_dma_addr(dst_buf, 1); > > + pfb->base_c.size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[1]; > > + } > > + mtk_v4l2_debug(1, > > + "id=%d Framebuf pfb=%p VA=%p Y_DMA=%pad C_DMA=%pad Size=%zx frame_count = %d", > > + dst_buf->index, pfb, > > + pfb->base_y.va, &pfb->base_y.dma_addr, > > + &pfb->base_c.dma_addr, pfb->base_y.size, > > + ctx->decoded_frame_cnt); > > + > > + return pfb; > > +} > > + > > +static void vb2ops_vdec_buf_request_complete(struct vb2_buffer *vb) > > +{ > > + struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); > > + > > + v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->ctrl_hdl); > > +} > > + > > +static int fops_media_request_validate(struct media_request *mreq) > > I think this function should be moved to just before mtk_vcodec_media_ops. > It's a bit weird to have this in the middle of the code. > > > +{ > > + const unsigned int buffer_cnt = vb2_request_buffer_cnt(mreq); > > + > > + switch (buffer_cnt) { > > + case 1: > > + /* We expect exactly one buffer with the request */ > > + break; > > + case 0: > > + mtk_v4l2_err("No buffer provided with the request"); > > + return -ENOENT; > > + default: > > + mtk_v4l2_err("Too many buffers (%d) provided with the request", > > + buffer_cnt); > > These aren't errors: user errors are not something that should be reported > in the kernel log, only driver/hw errors should be reported there. > > Use _debug instead. > > > + return -EINVAL; > > + } > > + > > + return vb2_request_validate(mreq); > > +} > > + > > +static void mtk_vdec_worker(struct work_struct *work) > > +{ > > + struct mtk_vcodec_ctx *ctx = > > + container_of(work, struct mtk_vcodec_ctx, decode_work); > > + struct mtk_vcodec_dev *dev = ctx->dev; > > + struct vb2_v4l2_buffer *vb2_v4l2_src, *vb2_v4l2_dst; > > + struct vb2_buffer *vb2_src; > > + struct mtk_vcodec_mem *bs_src; > > + struct mtk_video_dec_buf *dec_buf_src; > > + struct media_request *src_buf_req; > > + struct vdec_fb *dst_buf; > > + bool res_chg = false; > > + int ret; > > + > > + vb2_v4l2_src = v4l2_m2m_next_src_buf(ctx->m2m_ctx); > > + if (vb2_v4l2_src == NULL) { > > + v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx); > > + mtk_v4l2_debug(1, "[%d] no available source buffer", ctx->id); > > + return; > > + } > > + > > + vb2_v4l2_dst = v4l2_m2m_next_dst_buf(ctx->m2m_ctx); > > + if (vb2_v4l2_dst == NULL) { > > + v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx); > > + mtk_v4l2_debug(1, "[%d] no available destination buffer", ctx->id); > > + return; > > + } > > + > > + vb2_src = &vb2_v4l2_src->vb2_buf; > > + dec_buf_src = container_of(vb2_v4l2_src, struct mtk_video_dec_buf, > > + m2m_buf.vb); > > + bs_src = &dec_buf_src->bs_buffer; > > + > > + mtk_v4l2_debug(3, "[%d] (%d) id=%d, vb=%p buf_info = %p", > > + ctx->id, src_buf->vb2_queue->type, > > + src_buf->index, src_buf, src_buf_info); > > + > > + bs_src->va = NULL; > > + bs_src->dma_addr = vb2_dma_contig_plane_dma_addr(vb2_src, 0); > > + bs_src->size = (size_t)vb2_src->planes[0].bytesused; > > + > > + mtk_v4l2_debug(3, "[%d] Bitstream VA=%p DMA=%pad Size=%zx vb=%p", > > + ctx->id, buf->va, &buf->dma_addr, buf->size, src_buf); > > + /* Apply request controls. */ > > + src_buf_req = vb2_src->req_obj.req; > > + if (src_buf_req) > > + v4l2_ctrl_request_setup(src_buf_req, &ctx->ctrl_hdl); > > + else > > + mtk_v4l2_err("vb2 buffer media request is NULL"); > > + > > + dst_buf = vdec_get_cap_buffer(ctx, vb2_v4l2_dst); > > + v4l2_m2m_buf_copy_metadata(vb2_v4l2_src, vb2_v4l2_dst, true); > > + ret = vdec_if_decode(ctx, bs_src, dst_buf, &res_chg); > > + if (ret) { > > + mtk_v4l2_err( > > + " <===[%d], src_buf[%d] sz=0x%zx pts=%llu vdec_if_decode() ret=%d res_chg=%d===>", > > + ctx->id, vb2_src->index, bs_src->size, > > + vb2_src->timestamp, ret, res_chg); > > + if (ret == -EIO) { > > + mutex_lock(&ctx->lock); > > + dec_buf_src->error = true; > > + mutex_unlock(&ctx->lock); > > + } > > + } > > + > > + mtk_vdec_stateless_set_dst_payload(ctx, dst_buf); > > + > > + v4l2_m2m_buf_done_and_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx, > > + ret ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE); > > + > > + v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl); > > +} > > + > > +static void vb2ops_vdec_stateless_buf_queue(struct vb2_buffer *vb) > > +{ > > + struct mtk_vcodec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); > > + struct vb2_v4l2_buffer *vb2_v4l2 = to_vb2_v4l2_buffer(vb); > > + > > + mtk_v4l2_debug(3, "[%d] (%d) id=%d, vb=%p", > > + ctx->id, vb->vb2_queue->type, > > + vb->index, vb); > > + > > + mutex_lock(&ctx->lock); > > + v4l2_m2m_buf_queue(ctx->m2m_ctx, vb2_v4l2); > > + mutex_unlock(&ctx->lock); > > + if (vb->vb2_queue->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) > > + return; > > + > > + mtk_v4l2_debug(3, "(%d) id=%d, bs=%p", > > + vb->vb2_queue->type, vb->index, src_buf); > > + > > + /* If an OUTPUT buffer, we may need to update the state */ > > + if (ctx->state == MTK_STATE_INIT) { > > + ctx->state = MTK_STATE_HEADER; > > + mtk_v4l2_debug(1, "Init driver from init to header."); > > + } else { > > + mtk_v4l2_debug(3, "[%d] already init driver %d", > > + ctx->id, ctx->state); > > + } > > +} > > + > > +static int mtk_vdec_flush_decoder(struct mtk_vcodec_ctx *ctx) > > +{ > > + bool res_chg; > > + > > + return vdec_if_decode(ctx, NULL, NULL, &res_chg); > > +} > > + > > +static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_ctx *ctx) > > +{ > > + unsigned int i; > > + > > + v4l2_ctrl_handler_init(&ctx->ctrl_hdl, NUM_CTRLS); > > + if (ctx->ctrl_hdl.error) { > > + mtk_v4l2_err("v4l2_ctrl_handler_init failed\n"); > > + return ctx->ctrl_hdl.error; > > + } > > + > > + for (i = 0; i < NUM_CTRLS; i++) { > > + struct v4l2_ctrl_config cfg = mtk_stateless_controls[i].cfg; > > + > > + v4l2_ctrl_new_custom(&ctx->ctrl_hdl, &cfg, NULL); > > + if (ctx->ctrl_hdl.error) { > > + mtk_v4l2_err("Adding control %d failed %d", > > + i, ctx->ctrl_hdl.error); > > + return ctx->ctrl_hdl.error; > > + } > > + } > > + > > + v4l2_ctrl_handler_setup(&ctx->ctrl_hdl); > > + > > + return 0; > > +} > > + > > +const struct media_device_ops mtk_vcodec_media_ops = { > > + .req_validate = fops_media_request_validate, > > + .req_queue = v4l2_m2m_request_queue, > > +}; > > + > > +static void mtk_init_vdec_params(struct mtk_vcodec_ctx *ctx) > > +{ > > + struct vb2_queue *src_vq; > > + > > + src_vq = v4l2_m2m_get_vq(ctx->m2m_ctx, > > + V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); > > + > > + /* Support request api for output plane */ > > + src_vq->supports_requests = true; > > + src_vq->requires_requests = true; > > +} > > + > > +static int vb2ops_vdec_out_buf_validate(struct vb2_buffer *vb) > > +{ > > + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); > > + > > + vbuf->field = V4L2_FIELD_NONE; > > + return 0; > > +} > > + > > +static struct vb2_ops mtk_vdec_request_vb2_ops = { > > + .queue_setup = vb2ops_vdec_queue_setup, > > + .buf_prepare = vb2ops_vdec_buf_prepare, > > Should be with the other .buf ops. > > > + .wait_prepare = vb2_ops_wait_prepare, > > + .wait_finish = vb2_ops_wait_finish, > > + .start_streaming = vb2ops_vdec_start_streaming, > > + > > + .buf_queue = vb2ops_vdec_stateless_buf_queue, > > + .buf_out_validate = vb2ops_vdec_out_buf_validate, > > + .buf_init = vb2ops_vdec_buf_init, > > + .buf_finish = vb2ops_vdec_buf_finish, > > + .stop_streaming = vb2ops_vdec_stop_streaming, > > Shouldn't stop_streaming be just after .start_streaming? It's weird to see > them separated by other ops. All suggestions applied, thanks! Cheers, Alex.
Hi Dafna, On Thu, Apr 29, 2021 at 9:10 PM Dafna Hirschfeld <dafna.hirschfeld@collabora.com> wrote: > > > > On 27.04.21 13:15, Alexandre Courbot wrote: > > From: Yunfei Dong <yunfei.dong@mediatek.com> > > > > Support the stateless codec API that will be used by MT8183. > > > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > [acourbot: refactor, cleanup and split] > > Co-developed-by: Alexandre Courbot <acourbot@chromium.org> > > Signed-off-by: Alexandre Courbot <acourbot@chromium.org> > > --- > > drivers/media/platform/mtk-vcodec/Makefile | 1 + > > .../platform/mtk-vcodec/mtk_vcodec_dec.c | 66 +++- > > .../platform/mtk-vcodec/mtk_vcodec_dec.h | 9 +- > > .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 370 ++++++++++++++++++ > > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 + > > 5 files changed, 446 insertions(+), 3 deletions(-) > > create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > > > > diff --git a/drivers/media/platform/mtk-vcodec/Makefile b/drivers/media/platform/mtk-vcodec/Makefile > > index 9c3cbb5b800e..4ba93d838ab6 100644 > > --- a/drivers/media/platform/mtk-vcodec/Makefile > > +++ b/drivers/media/platform/mtk-vcodec/Makefile > > @@ -12,6 +12,7 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \ > > vdec_vpu_if.o \ > > mtk_vcodec_dec.o \ > > mtk_vcodec_dec_stateful.o \ > > + mtk_vcodec_dec_stateless.o \ > > mtk_vcodec_dec_pm.o \ > > > > mtk-vcodec-enc-y := venc/venc_vp8_if.o \ > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c > > index 4ad2662a43b2..01c5333d6cff 100644 > > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c > > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c > > @@ -46,6 +46,13 @@ static struct mtk_q_data *mtk_vdec_get_q_data(struct mtk_vcodec_ctx *ctx, > > static int vidioc_try_decoder_cmd(struct file *file, void *priv, > > struct v4l2_decoder_cmd *cmd) > > { > > + struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv); > > + > > + /* Use M2M stateless helper if relevant */ > > + if (ctx->dev->vdec_pdata->uses_stateless_api) > > + return v4l2_m2m_ioctl_stateless_try_decoder_cmd(file, priv, > > + cmd); > > + > > switch (cmd->cmd) { > > case V4L2_DEC_CMD_STOP: > > case V4L2_DEC_CMD_START: > > @@ -72,6 +79,10 @@ static int vidioc_decoder_cmd(struct file *file, void *priv, > > if (ret) > > return ret; > > > > + /* Use M2M stateless helper if relevant */ > > + if (ctx->dev->vdec_pdata->uses_stateless_api) > > + return v4l2_m2m_ioctl_stateless_decoder_cmd(file, priv, cmd); > > + > > mtk_v4l2_debug(1, "decoder cmd=%u", cmd->cmd); > > dst_vq = v4l2_m2m_get_vq(ctx->m2m_ctx, > > V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > > @@ -414,7 +425,8 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv, > > * Setting OUTPUT format after OUTPUT buffers are allocated is invalid > > * if using the stateful API. > > */ > > - if ((f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) && > > + if (!dec_pdata->uses_stateless_api && > > + (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) && > > vb2_is_busy(&ctx->m2m_ctx->out_q_ctx.q)) { > > mtk_v4l2_err("out_q_ctx buffers already requested"); > > ret = -EBUSY; > > @@ -457,6 +469,7 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv, > > ctx->quantization = pix_mp->quantization; > > ctx->xfer_func = pix_mp->xfer_func; > > > > + ctx->current_codec = fmt->fourcc; > > if (ctx->state == MTK_STATE_FREE) { > > ret = vdec_if_init(ctx, q_data->fmt->fourcc); > > if (ret) { > > @@ -468,6 +481,49 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv, > > } > > } > > > > + /* > > + * If using the stateless API, S_FMT should have the effect of setting > > + * the CAPTURE queue resolution no matter which queue it was called on. > > + */ > > + if (dec_pdata->uses_stateless_api) { > > + ctx->picinfo.pic_w = pix_mp->width; > > + ctx->picinfo.pic_h = pix_mp->height; > > + > > + ret = vdec_if_get_param(ctx, GET_PARAM_PIC_INFO, &ctx->picinfo); > > + if (ret) { > > + mtk_v4l2_err("[%d]Error!! Get GET_PARAM_PICTURE_INFO Fail", > > + ctx->id); > > + return -EINVAL; > > + } > > + > > + ctx->last_decoded_picinfo = ctx->picinfo; > > + > > + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 1) { > > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] = > > + ctx->picinfo.fb_sz[0] + > > + ctx->picinfo.fb_sz[1]; > > + ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] = > > + ctx->picinfo.buf_w; > > + } else { > > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] = > > + ctx->picinfo.fb_sz[0]; > > + ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] = > > + ctx->picinfo.buf_w; > > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[1] = > > + ctx->picinfo.fb_sz[1]; > > + ctx->q_data[MTK_Q_DATA_DST].bytesperline[1] = > > + ctx->picinfo.buf_w; > > + } > > + > > + ctx->q_data[MTK_Q_DATA_DST].coded_width = ctx->picinfo.buf_w; > > + ctx->q_data[MTK_Q_DATA_DST].coded_height = ctx->picinfo.buf_h; > > + mtk_v4l2_debug(2, "[%d] vdec_if_init() num_plane = %d wxh=%dx%d pic wxh=%dx%d sz[0]=0x%x sz[1]=0x%x", > > + ctx->id, pix_mp->num_planes, > > + ctx->picinfo.buf_w, ctx->picinfo.buf_h, > > + ctx->picinfo.pic_w, ctx->picinfo.pic_h, > > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[0], > > + ctx->q_data[MTK_Q_DATA_DST].sizeimage[1]); > > + } > > return 0; > > } > > > > @@ -765,9 +821,15 @@ void vb2ops_vdec_stop_streaming(struct vb2_queue *q) > > while ((src_buf = v4l2_m2m_src_buf_remove(ctx->m2m_ctx))) { > > struct mtk_video_dec_buf *buf_info = container_of( > > src_buf, struct mtk_video_dec_buf, m2m_buf.vb); > > - if (!buf_info->lastframe) > > + if (!buf_info->lastframe) { > > + struct media_request *req = > > + src_buf->vb2_buf.req_obj.req; > > v4l2_m2m_buf_done(src_buf, > > VB2_BUF_STATE_ERROR); > > + if (req) > > + v4l2_ctrl_request_complete(req, > > + &ctx->ctrl_hdl); > > + } > > } > > return; > > } > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h > > index 6a18cb3bfe07..6b29d7d9ae15 100644 > > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h > > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.h > > @@ -45,6 +45,7 @@ struct vdec_fb { > > * @lastframe: Intput buffer is last buffer - EOS > > * @error: An unrecoverable error occurs on this buffer. > > * @frame_buffer: Decode status, and buffer information of Capture buffer > > + * @bs_buffer: Output buffer info > > * > > * Note : These status information help us track and debug buffer state > > */ > > @@ -55,12 +56,18 @@ struct mtk_video_dec_buf { > > bool queued_in_vb2; > > bool queued_in_v4l2; > > bool lastframe; > > + > > bool error; > > - struct vdec_fb frame_buffer; > > + > > + union { > > + struct vdec_fb frame_buffer; > > + struct mtk_vcodec_mem bs_buffer; > > + }; > > }; > > > > extern const struct v4l2_ioctl_ops mtk_vdec_ioctl_ops; > > extern const struct v4l2_m2m_ops mtk_vdec_m2m_ops; > > +extern const struct media_device_ops mtk_vcodec_media_ops; > > > > > > /* > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > > new file mode 100644 > > index 000000000000..75ddf53e2876 > > --- /dev/null > > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c > > Hi, when compiling with -DDEBUG flag, I got some compilation errors for this file. Nice catch, I've made sure the whole driver compiles with -DDEBUG. Thanks, Alex.
Le jeudi 13 mai 2021 à 17:05 +0900, Alexandre Courbot a écrit : > Hi Hans, thanks for the review! > > On Thu, Apr 29, 2021 at 4:28 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote: > > > > On 27/04/2021 13:15, Alexandre Courbot wrote: > > > From: Yunfei Dong <yunfei.dong@mediatek.com> > > > > > > The stateless API requires a media device for issuing requests. Add one > > > if we are being instantiated as a stateless decoder. > > > > Why for the stateless decoder only? Why not create one for all? > > > > It's not a blocker, but I would recommend looking at this. > > Would there be any use in creating a media device for a stateful > decoder that does not need to use requests? The only thing I can think of is classification. In GStreamer support for stateless decoders, I use the topology to classify what is a decoder by walking the toplogy and making sure it's what I expect, and that there is no other branches or functions that I may not support. This makes it more strict, so less likely to confuse driver function. Note that v4l2-compliance just use the same old heuristic we have used for stateful, which is to check that one side have only RAW format, and the other side have only encoded formats. It works too, it's just less strict. > > Cheers, > Alex.
On Tue, Apr 27, 2021 at 7:16 PM Alexandre Courbot <acourbot@chromium.org> wrote: > > From: Yunfei Dong <yunfei.dong@mediatek.com> > > The stateless API requires a media device for issuing requests. Add one > if we are being instantiated as a stateless decoder. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > [acourbot: refactor, cleanup and split] > Co-developed-by: Alexandre Courbot <acourbot@chromium.org> > Signed-off-by: Alexandre Courbot <acourbot@chromium.org> > --- > drivers/media/platform/Kconfig | 1 + > .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 39 +++++++++++++++++++ > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 2 + > 3 files changed, 42 insertions(+) > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > index ae1468aa1b4e..4154fdec2efb 100644 > --- a/drivers/media/platform/Kconfig > +++ b/drivers/media/platform/Kconfig > @@ -315,6 +315,7 @@ config VIDEO_MEDIATEK_VCODEC > select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU > select VIDEO_MEDIATEK_VCODEC_SCP if MTK_SCP > select V4L2_H264 > + select MEDIA_CONTROLLER Should this also select MEDIA_CONTROLLER_REQUEST_API config? Thanks > help > Mediatek video codec driver provides HW capability to > encode and decode in a range of video formats on MT8173 > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > index 533781d4680a..e942e28f96fe 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c > @@ -14,6 +14,7 @@ > #include <media/v4l2-event.h> > #include <media/v4l2-mem2mem.h> > #include <media/videobuf2-dma-contig.h> > +#include <media/v4l2-device.h> > > #include "mtk_vcodec_drv.h" > #include "mtk_vcodec_dec.h" > @@ -324,6 +325,31 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > goto err_event_workq; > } > > + if (dev->vdec_pdata->uses_stateless_api) { > + dev->mdev_dec.dev = &pdev->dev; > + strscpy(dev->mdev_dec.model, MTK_VCODEC_DEC_NAME, > + sizeof(dev->mdev_dec.model)); > + > + media_device_init(&dev->mdev_dec); > + dev->mdev_dec.ops = &mtk_vcodec_media_ops; > + dev->v4l2_dev.mdev = &dev->mdev_dec; > + > + ret = v4l2_m2m_register_media_controller(dev->m2m_dev_dec, > + dev->vfd_dec, MEDIA_ENT_F_PROC_VIDEO_DECODER); > + if (ret) { > + mtk_v4l2_err("Failed to register media controller"); > + goto err_reg_cont; > + } > + > + ret = media_device_register(&dev->mdev_dec); > + if (ret) { > + mtk_v4l2_err("Failed to register media device"); > + goto err_media_reg; > + } > + > + mtk_v4l2_debug(0, "media registered as /dev/media%d", > + vfd_dec->num); > + } > ret = video_register_device(vfd_dec, VFL_TYPE_VIDEO, 0); > if (ret) { > mtk_v4l2_err("Failed to register video device"); > @@ -336,6 +362,12 @@ static int mtk_vcodec_probe(struct platform_device *pdev) > return 0; > > err_dec_reg: > + if (dev->vdec_pdata->uses_stateless_api) > + media_device_unregister(&dev->mdev_dec); > +err_media_reg: > + if (dev->vdec_pdata->uses_stateless_api) > + v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec); > +err_reg_cont: > destroy_workqueue(dev->decode_workqueue); > err_event_workq: > v4l2_m2m_release(dev->m2m_dev_dec); > @@ -368,6 +400,13 @@ static int mtk_vcodec_dec_remove(struct platform_device *pdev) > > flush_workqueue(dev->decode_workqueue); > destroy_workqueue(dev->decode_workqueue); > + > + if (media_devnode_is_registered(dev->mdev_dec.devnode)) { > + media_device_unregister(&dev->mdev_dec); > + v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec); > + media_device_cleanup(&dev->mdev_dec); > + } > + > if (dev->m2m_dev_dec) > v4l2_m2m_release(dev->m2m_dev_dec); > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > index 78d4a7728ddf..10edd238c939 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > @@ -383,6 +383,7 @@ struct mtk_vcodec_enc_pdata { > * struct mtk_vcodec_dev - driver data > * @v4l2_dev: V4L2 device to register video devices for. > * @vfd_dec: Video device for decoder > + * @mdev_dec: Media device for decoder > * @vfd_enc: Video device for encoder. > * > * @m2m_dev_dec: m2m device for decoder > @@ -418,6 +419,7 @@ struct mtk_vcodec_enc_pdata { > struct mtk_vcodec_dev { > struct v4l2_device v4l2_dev; > struct video_device *vfd_dec; > + struct media_device mdev_dec; > struct video_device *vfd_enc; > > struct v4l2_m2m_dev *m2m_dev_dec; > -- > 2.31.1.498.g6c1eba8ee3d-goog > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Mon, May 17, 2021 at 1:35 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > On Tue, Apr 27, 2021 at 7:16 PM Alexandre Courbot <acourbot@chromium.org> wrote: > > > > From: Yunfei Dong <yunfei.dong@mediatek.com> > > > > The stateless API requires a media device for issuing requests. Add one > > if we are being instantiated as a stateless decoder. > > > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > > [acourbot: refactor, cleanup and split] > > Co-developed-by: Alexandre Courbot <acourbot@chromium.org> > > Signed-off-by: Alexandre Courbot <acourbot@chromium.org> > > --- > > drivers/media/platform/Kconfig | 1 + > > .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 39 +++++++++++++++++++ > > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 2 + > > 3 files changed, 42 insertions(+) > > > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > > index ae1468aa1b4e..4154fdec2efb 100644 > > --- a/drivers/media/platform/Kconfig > > +++ b/drivers/media/platform/Kconfig > > @@ -315,6 +315,7 @@ config VIDEO_MEDIATEK_VCODEC > > select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU > > select VIDEO_MEDIATEK_VCODEC_SCP if MTK_SCP > > select V4L2_H264 > > + select MEDIA_CONTROLLER > > Should this also select MEDIA_CONTROLLER_REQUEST_API config? Yup, it probably should. hantro and rkvdec also select it, so let's do the same. Thanks! Alex.
On 13.05.21 10:21, Alexandre Courbot wrote: > Hi Dafna, > > On Thu, Apr 29, 2021 at 9:07 PM Dafna Hirschfeld > <dafna.hirschfeld@collabora.com> wrote: >> >> Hi, >> >> On 27.04.21 13:15, Alexandre Courbot wrote: >>> This series adds support for the stateless API into mtk-vcodec, by first >>> separating the stateful ops into their own source file, and introducing >>> a new set of ops suitable for stateless decoding. As such, support for >>> stateful decoders should remain completely unaffected. >>> >>> This series has been tested with both MT8183 and MT8173. Decoding was >>> working for both chips, and in the case of MT8173 no regression has been >>> noticed. >> >> I am trying to test the decoder with this patchset using v4l2-ctl on mt8173. >> >> I am trying to decode an h264 file into V4L2_PIX_FMT_MT21C format. >> I am not able to do it. It seems that the driver expects each buffer to start >> with a nal starting code, and the v4l2-ctl command just read the files into >> buffers without any parsing. >> >> Can you share the command and the files you used for testing? > > I have been using the Chromium test suite (aka > video_decode_accelerator_tests). I had doubts after reading your email > but I tested the series again using this tool and confirmed it was > working. Hi, I have a chromeos userspace that I compiled. I use it to test the codec drivers on latest 5.14. Could you share the exact command you use for your tests? I used the command: tast -verbose run -build=false 10.42.0.175 video.DecodeAccel*h264* With that commands, all the tests that ends with 'VD' fail , the other tests pass. The docs in [1] says that the command video_decode_accelerator_tests does not require the "full Chrome browser stack", does that mean that it can be compiled and run on userspace other than chromeos? I could not find the instructions of how to compile and install that command. Could you instruct me to it? The mt8173 doc [2] says that the supported video decoders are "H.264, H.265 / HEVC, MPEG-1/2/4" But running 'v4l2-ctl --list-formats-out -d0' shows: ioctl: VIDIOC_ENUM_FMT Type: Video Output Multiplanar [0]: 'H264' (H.264, compressed) [1]: 'VP80' (VP8, compressed) [2]: 'VP90' (VP9, compressed) So the source code shows support for vp8, vp9 which is not stated in the official doc. Do you know the explanation for that difference? Do you know if elm device should support vp8, vp9? [1] https://chromium.googlesource.com/chromium/src/+/HEAD/docs/media/gpu/video_decoder_test_usage.md [2] https://www.mediatek.com/products/tablets/mt8173 Thanks, Dafna > > Unfortunately this test is not easy to build, but maybe if you have a > Chromium environment ready you can run it. mtk-vcodec does expect the > input buffers to be split by NAL units (as per the spec [1] IIUC), so > that would explain why v4l2-ctl failed (I assume that it also fails > without this series?). > > Maybe ffmpeg can also be used to exercice this driver with properly > split NAL units, but I am not familiar with its use with V4L2. I'm > also not sure it would be happy about the MT21C format that the driver > outputs, or that it could pick the MDP to convert it to something > readable... > > Cheers, > Alex. > > [1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/pixfmt-compressed.html#compressed-formats >
This series adds support for the stateless API into mtk-vcodec, by first separating the stateful ops into their own source file, and introducing a new set of ops suitable for stateless decoding. As such, support for stateful decoders should remain completely unaffected. This series has been tested with both MT8183 and MT8173. Decoding was working for both chips, and in the case of MT8173 no regression has been noticed. Patches 1-9 add MT8183 support to the decoder using the stateless API. MT8183 only support H.264 acceleration. Patches 10-15 are follow-ups that further improve compliance for the decoder and encoder, by fixing support for commands on both. Patch 11 also makes sure that supported H.264 profiles are exported on MT8173. Changes since v3: * Stop checking that controls are set for every request. * Add V4L2_CID_STATELESS_H264_START_CODE control. * Stop mapping OUTPUT buffers and getting the NAL type from them, use the nal_ref_idc field instead. * Make V4L2_CID_MIN_BUFFERS_FOR_CAPTURE control stateful-only. * Set vb2_buffer's field to V4L2_FIELD_NONE in buffer validation hook. Changes since v2: * Add follow-up patches fixing support for START/STOP commands for the encoder, and stateful decoder. Alexandre Courbot (8): media: mtk-vcodec: vdec: handle firmware version field media: mtk-vcodec: support version 2 of decoder firmware ABI media: add Mediatek's MM21 format dt-bindings: media: document mediatek,mt8183-vcodec-dec media: mtk-vcodec: vdec: use helpers in VIDIOC_(TRY_)DECODER_CMD media: mtk-vcodec: vdec: clamp OUTPUT resolution to hardware limits media: mtk-vcodec: make flush buffer reusable by encoder media: mtk-vcodec: venc: support START and STOP commands Hirokazu Honda (1): media: mtk-vcodec: vdec: Support H264 profile control Hsin-Yi Wang (1): media: mtk-vcodec: venc: make sure buffer exists in list before removing Yunfei Dong (5): media: mtk-vcodec: vdec: move stateful ops into their own file media: mtk-vcodec: vdec: support stateless API media: mtk-vcodec: vdec: support stateless H.264 decoding media: mtk-vcodec: vdec: add media device if using stateless api media: mtk-vcodec: enable MT8183 decoder .../bindings/media/mediatek-vcodec.txt | 1 + .../media/v4l/pixfmt-reserved.rst | 7 + drivers/media/platform/Kconfig | 2 + drivers/media/platform/mtk-vcodec/Makefile | 3 + .../platform/mtk-vcodec/mtk_vcodec_dec.c | 818 +++--------------- .../platform/mtk-vcodec/mtk_vcodec_dec.h | 28 +- .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 66 +- .../mtk-vcodec/mtk_vcodec_dec_stateful.c | 667 ++++++++++++++ .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 370 ++++++++ .../platform/mtk-vcodec/mtk_vcodec_drv.h | 58 +- .../platform/mtk-vcodec/mtk_vcodec_enc.c | 135 ++- .../platform/mtk-vcodec/mtk_vcodec_enc_drv.c | 4 + .../mtk-vcodec/vdec/vdec_h264_req_if.c | 780 +++++++++++++++++ .../media/platform/mtk-vcodec/vdec_drv_if.c | 3 + .../media/platform/mtk-vcodec/vdec_drv_if.h | 1 + .../media/platform/mtk-vcodec/vdec_ipi_msg.h | 23 +- .../media/platform/mtk-vcodec/vdec_vpu_if.c | 43 +- .../media/platform/mtk-vcodec/vdec_vpu_if.h | 5 + drivers/media/v4l2-core/v4l2-ioctl.c | 1 + include/uapi/linux/videodev2.h | 1 + 20 files changed, 2293 insertions(+), 723 deletions(-) create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateful.c create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c create mode 100644 drivers/media/platform/mtk-vcodec/vdec/vdec_h264_req_if.c -- 2.31.1.498.g6c1eba8ee3d-goog