Messages in this thread | | | Date | Wed, 24 May 2023 09:35:54 +0200 | Subject | Re: [PATCH v5,2/2] media: mediatek: vcodec: support stateless hevc decoder | From | AngeloGioacchino Del Regno <> |
| |
Il 24/05/23 04:16, Yunfei Dong ha scritto: > Add mediatek hevc decoder linux driver which use the stateless API in MT8195. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > Tested-by: Nathan Hebert <nhebert@chromium.org> > --- > .../media/platform/mediatek/vcodec/Makefile | 1 + > .../vcodec/mtk_vcodec_dec_stateless.c | 59 +- > .../platform/mediatek/vcodec/mtk_vcodec_drv.h | 1 + > .../vcodec/vdec/vdec_hevc_req_multi_if.c | 1101 +++++++++++++++++ > .../platform/mediatek/vcodec/vdec_drv_if.c | 4 + > .../platform/mediatek/vcodec/vdec_drv_if.h | 1 + > 6 files changed, 1166 insertions(+), 1 deletion(-) > create mode 100644 drivers/media/platform/mediatek/vcodec/vdec/vdec_hevc_req_multi_if.c >
..snip..
> diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_hevc_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_hevc_req_multi_if.c > new file mode 100644 > index 000000000000..9a96547af33c > --- /dev/null > +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_hevc_req_multi_if.c > @@ -0,0 +1,1101 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2023 MediaTek Inc. > + * Author: Yunfei Dong <yunfei.dong@mediatek.com> > + */ > +
..snip..
> + > +/** > + * struct vdec_hevc_slice_share_info - shared information used to exchange > + * message between lat and core > + * > + * @sps: sequence header information from user space > + * @dec_params: decoder params from user space > + * @hevc_slice_params: decoder params used for hardware > + * @trans_start: trans start dma address > + * @trans_end: trans end dma address
Wrong documentation, there's no trans_start, trans_end, but just `trans`.
> + */ > +struct vdec_hevc_slice_share_info { > + struct v4l2_ctrl_hevc_sps sps; > + struct v4l2_ctrl_hevc_decode_params dec_params; > + struct vdec_hevc_slice_lat_dec_param hevc_slice_params; > + struct vdec_hevc_slice_mem trans; > +}; > + > +/** > + * struct vdec_hevc_slice_inst - hevc decoder instance > + * > + * @slice_dec_num: how many picture be decoded > + * @ctx: point to mtk_vcodec_ctx > + * @pred_buf: HW working predication buffer
pred_buf is not present in this structure; either add it and make use of it, or remove the documentation for it.
> + * @mv_buf: HW working motion vector buffer > + * @vpu: VPU instance > + * @vsi: vsi used for lat > + * @vsi_core: vsi used for core > + * @wrap_addr: wrap address used for hevc > + * > + * @hevc_slice_param: the parameters that hardware use to decode > + * > + * @resolution_changed: resolution changed > + * @realloc_mv_buf: reallocate mv buffer > + * @cap_num_planes: number of capture queue plane > + */ > +struct vdec_hevc_slice_inst { > + unsigned int slice_dec_num; > + struct mtk_vcodec_ctx *ctx; > + struct mtk_vcodec_mem mv_buf[HEVC_MAX_MV_NUM]; > + struct vdec_vpu_inst vpu; > + struct vdec_hevc_slice_vsi *vsi; > + struct vdec_hevc_slice_vsi *vsi_core; > + struct mtk_vcodec_mem wrap_addr; > + > + struct vdec_hevc_slice_lat_dec_param hevc_slice_param; > + > + unsigned int resolution_changed; > + unsigned int realloc_mv_buf; > + unsigned int cap_num_planes; > +}; > + > +static unsigned int vdec_hevc_get_mv_buf_size(unsigned int width, unsigned int height) > +{ > + const int unit_size = (width / 16) * (height / 16) + 8;
This is supposed to be `const unsigned int`, otherwise you may overflow here (even if it's unlikely to, but still....!)
> + > + return 64 * unit_size; > +} > +
..snip..
> +static int vdec_hevc_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs, > + struct vdec_fb *fb, bool *res_chg) > +{ > + struct vdec_hevc_slice_inst *inst = h_vdec; > + struct vdec_vpu_inst *vpu = &inst->vpu; > +
Please remove this extra empty line.
> + int err, timeout = 0; > + unsigned int data[2]; > + struct vdec_lat_buf *lat_buf; > + struct vdec_hevc_slice_share_info *share_info; > +
...after which:
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
| |