[PATCH v4, 4/8] media: mediatek: vcodec: Add more extra processing for multi-core encoding

AngeloGioacchino Del Regno angelogioacchino.delregno at collabora.com
Fri Jun 24 02:00:57 PDT 2022


Il 24/06/22 10:23, Irui Wang ha scritto:
> Add a bit for indicating support multi-core encoding, because multi-core
> encoding need more working buffers for encoder hardware. The working
> buffers are allocated from kernel side then pass to scp firmware side
> through shared memory, the struct definition must be kept align between
> kernel and scp firmware side. New another shared memory struct for
> multi-core encoding.
> 
> Signed-off-by: Irui Wang <irui.wang at mediatek.com>
> ---
>   .../platform/mediatek/vcodec/mtk_vcodec_drv.h |   2 +
>   .../mediatek/vcodec/mtk_vcodec_util.c         |  19 ++
>   .../mediatek/vcodec/mtk_vcodec_util.h         |   2 +
>   .../mediatek/vcodec/venc/venc_h264_if.c       | 171 +++++++++++++++---
>   4 files changed, 170 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> index 8919bdf2eef5..3a291cac6b5e 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> @@ -29,6 +29,8 @@
>   #define WAIT_INTR_TIMEOUT_MS	1000
>   #define IS_VDEC_LAT_ARCH(hw_arch) ((hw_arch) >= MTK_VDEC_LAT_SINGLE_CORE)
>   #define IS_VDEC_INNER_RACING(capability) ((capability) & MTK_VCODEC_INNER_RACING)
> +#define MTK_VENC_MULTICORE_ENABLE BIT(1)
> +#define IS_VENC_MULTICORE(capability) ((capability) & MTK_VENC_MULTICORE_ENABLE)
>   
>   /*
>    * enum mtk_hw_reg_idx - MTK hw register base index
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.c
> index ace78c4b5b9e..a723243626c0 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.c
> @@ -11,6 +11,7 @@
>   
>   #include "mtk_vcodec_dec_hw.h"
>   #include "mtk_vcodec_drv.h"
> +#include "mtk_vcodec_enc_hw.h"
>   #include "mtk_vcodec_util.h"
>   
>   void __iomem *mtk_vcodec_get_reg_addr(struct mtk_vcodec_ctx *data,
> @@ -26,6 +27,24 @@ void __iomem *mtk_vcodec_get_reg_addr(struct mtk_vcodec_ctx *data,
>   }
>   EXPORT_SYMBOL(mtk_vcodec_get_reg_addr);
>   
> +void __iomem *mtk_venc_get_core_reg_addr(struct mtk_vcodec_ctx *ctx,
> +					 unsigned int hw_id)

This is enum mtk_venc_hw_id.....

> +{
> +	struct mtk_venc_hw_dev *sub_core;
> +
> +	if (hw_id >= MTK_VENC_HW_MAX) {
> +		mtk_v4l2_err("Invalid hw_id = %d", hw_id);
> +		return NULL;
> +	}
> +
> +	sub_core = (struct mtk_venc_hw_dev *)ctx->dev->enc_hw_dev[hw_id];
> +	if (!sub_core)
> +		return NULL;
> +
> +	return sub_core->reg_base;
> +}
> +EXPORT_SYMBOL(mtk_venc_get_core_reg_addr);
> +
>   int mtk_vcodec_mem_alloc(struct mtk_vcodec_ctx *data,
>   			struct mtk_vcodec_mem *mem)
>   {
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> index 71956627a0e2..a74c98aa355b 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> @@ -50,6 +50,8 @@ struct mtk_vcodec_dev;
>   
>   void __iomem *mtk_vcodec_get_reg_addr(struct mtk_vcodec_ctx *data,
>   				unsigned int reg_idx);
> +void __iomem *mtk_venc_get_core_reg_addr(struct mtk_vcodec_ctx *data,
> +					 unsigned int hw_id);
>   int mtk_vcodec_mem_alloc(struct mtk_vcodec_ctx *data,
>   				struct mtk_vcodec_mem *mem);
>   void mtk_vcodec_mem_free(struct mtk_vcodec_ctx *data,
> diff --git a/drivers/media/platform/mediatek/vcodec/venc/venc_h264_if.c b/drivers/media/platform/mediatek/vcodec/venc/venc_h264_if.c
> index 4d9b8798dffe..888cee39d324 100644
> --- a/drivers/media/platform/mediatek/vcodec/venc/venc_h264_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/venc/venc_h264_if.c

..snip..

> @@ -143,8 +184,8 @@ struct venc_h264_vsi {
>    * @ctx: context for v4l2 layer integration
>    */
>   struct venc_h264_inst {
> -	void __iomem *hw_base;
> -	struct mtk_vcodec_mem work_bufs[VENC_H264_VPU_WORK_BUF_MAX];
> +	void __iomem *hw_base[MTK_VENC_HW_MAX];
> +	struct mtk_vcodec_mem work_bufs[VENC_MULTI_CORE_WORK_BUF_MAX];
>   	struct mtk_vcodec_mem pps_buf;
>   	bool work_buf_allocated;
>   	unsigned int frm_cnt;
> @@ -152,12 +193,13 @@ struct venc_h264_inst {
>   	unsigned int prepend_hdr;
>   	struct venc_vpu_inst vpu_inst;
>   	struct venc_h264_vsi *vsi;
> +	struct venc_multi_core_vsi *core_vsi;

You're adding a new struct member without adding it to the documentation,
please add it there too.

>   	struct mtk_vcodec_ctx *ctx;
>   };
>   
>   static inline u32 h264_read_reg(struct venc_h264_inst *inst, u32 addr)
>   {
> -	return readl(inst->hw_base + addr);
> +	return readl(inst->hw_base[MTK_VENC_CORE_0] + addr);
>   }
>   
>   static unsigned int h264_get_profile(struct venc_h264_inst *inst,
> @@ -228,13 +270,20 @@ static unsigned int h264_get_level(struct venc_h264_inst *inst,
>   static void h264_enc_free_work_buf(struct venc_h264_inst *inst)
>   {
>   	int i;
> +	struct mtk_vcodec_ctx *ctx = inst->ctx;
> +	int max_work_buf;

int i, max_work_buf;

>   
>   	mtk_vcodec_debug_enter(inst);
>   
> +	if (IS_VENC_MULTICORE(ctx->dev->enc_capability))
> +		max_work_buf = VENC_MULTI_CORE_WORK_BUF_MAX;
> +	else
> +		max_work_buf = VENC_H264_VPU_WORK_BUF_MAX;
> +
>   	/* Except the SKIP_FRAME buffers,
>   	 * other buffers need to be freed by AP.
>   	 */
> -	for (i = 0; i < VENC_H264_VPU_WORK_BUF_MAX; i++) {
> +	for (i = 0; i < max_work_buf; i++) {
>   		if (i != VENC_H264_VPU_WORK_BUF_SKIP_FRAME)
>   			mtk_vcodec_mem_free(inst->ctx, &inst->work_bufs[i]);
>   	}
> @@ -248,11 +297,21 @@ static int h264_enc_alloc_work_buf(struct venc_h264_inst *inst)
>   {
>   	int i;
>   	int ret = 0;
> -	struct venc_h264_vpu_buf *wb = inst->vsi->work_bufs;
> +	struct mtk_vcodec_ctx *ctx = inst->ctx;
> +	struct venc_h264_vpu_buf *wb;
> +	int max_work_buf;

While at it, can you please fix the order of these declarations?

	struct mtk_vcodec_ctx *ctx = inst->ctx;
	struct venc_h264_vpu_buf *wb;
	int i, max_work_buf;
	int ret = 0;

>   
>   	mtk_vcodec_debug_enter(inst);
>   
> -	for (i = 0; i < VENC_H264_VPU_WORK_BUF_MAX; i++) {
> +	if (IS_VENC_MULTICORE(ctx->dev->enc_capability)) {
> +		wb = inst->core_vsi->work_bufs;
> +		max_work_buf = VENC_MULTI_CORE_WORK_BUF_MAX;
> +	} else {
> +		wb = inst->vsi->work_bufs;
> +		max_work_buf = VENC_H264_VPU_WORK_BUF_MAX;
> +	}
> +
> +	for (i = 0; i < max_work_buf; i++) {
>   		/*
>   		 * This 'wb' structure is set by VPU side and shared to AP for
>   		 * buffer allocation and IO virtual addr mapping. For most of
> @@ -358,6 +417,26 @@ static int h264_frame_type(struct venc_h264_inst *inst)
>   		return VENC_H264_P_FRM;  /* Note: B frames are not supported */
>   	}
>   }
> +
> +static int h264_core_frame_type(struct venc_h264_inst *inst)
> +{
> +	struct venc_multi_core_vsi *vsi = inst->core_vsi;
> +
> +	if ((vsi->config.gop_size != 0 &&
> +	     (inst->frm_cnt % vsi->config.gop_size) == 0) ||
> +	    (inst->frm_cnt == 0 && vsi->config.gop_size == 0)) {
> +		/* IDR frame */
> +		return VENC_H264_IDR_FRM;

You can get to a maximum of 100 columns and, thinking about readability...:

if ((vsi->config.gop_size != 0 && (inst->frm_cnt % vsi->config.gop_size) == 0) ||
     (inst->frm_cnt == 0 && vsi->config.gop_size == 0)) {

this gets to 90 columns, so it's acceptable (and more readable).


> +	} else if ((vsi->config.intra_period != 0 &&
> +		    (inst->frm_cnt % vsi->config.intra_period) == 0) ||
> +		   (inst->frm_cnt == 0 && vsi->config.intra_period == 0)) {

..but, unfortunately, this doesn't, so we have to live with it.

> +		/* I frame */
> +		return VENC_H264_I_FRM;
> +	} else {
> +		return VENC_H264_P_FRM;  /* Note: B frames are not supported */
> +	}
> +}
> +
>   static int h264_encode_sps(struct venc_h264_inst *inst,
>   			   struct mtk_vcodec_mem *bs_buf,
>   			   unsigned int *bs_size)
> @@ -440,12 +519,16 @@ static int h264_encode_frame(struct venc_h264_inst *inst,
>   	int ret = 0;
>   	unsigned int irq_status;
>   	struct venc_frame_info frame_info;
> +	struct mtk_vcodec_ctx *ctx = inst->ctx;
>   
>   	mtk_vcodec_debug_enter(inst);
>   	mtk_vcodec_debug(inst, "frm_cnt = %d\n ", inst->frm_cnt);
>   	frame_info.frm_count = inst->frm_cnt;
>   	frame_info.skip_frm_count = inst->skip_frm_cnt;
> -	frame_info.frm_type = h264_frame_type(inst);
> +	if (IS_VENC_MULTICORE(ctx->dev->enc_capability))
> +		frame_info.frm_type = h264_core_frame_type(inst);
> +	else
> +		frame_info.frm_type = h264_frame_type(inst);
>   	mtk_vcodec_debug(inst, "frm_count = %d,skip_frm_count =%d,frm_type=%d.\n",
>   			 frame_info.frm_count, frame_info.skip_frm_count,
>   			 frame_info.frm_type);
> @@ -501,7 +584,7 @@ static void h264_encode_filler(struct venc_h264_inst *inst, void *buf,
>   static int h264_enc_init(struct mtk_vcodec_ctx *ctx)
>   {
>   	const bool is_ext = MTK_ENC_CTX_IS_EXT(ctx);
> -	int ret = 0;
> +	int ret, i;

Can you also move this after `inst` please?

>   	struct venc_h264_inst *inst;
>   

Cheers,
Angelo



More information about the Linux-mediatek mailing list