| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a8c3e22c-8f22-2fb9-ff23-f394c5c34366@collabora.com>
Date: Fri, 24 Jun 2022 11:00:57 +0200
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: Irui Wang <irui.wang@...iatek.com>,
Hans Verkuil <hverkuil-cisco@...all.nl>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Matthias Brugger <matthias.bgg@...il.com>,
Tomasz Figa <tfiga@...gle.com>,
Tzung-Bi Shih <tzungbi@...omium.org>,
Alexandre Courbot <acourbot@...omium.org>,
Tiffany Lin <tiffany.lin@...iatek.com>,
Andrew-CT Chen <andrew-ct.chen@...iatek.com>,
nicolas.dufresne@...labora.com
Cc: Hsin-Yi Wang <hsinyi@...omium.org>,
Maoguang Meng <maoguang.meng@...iatek.com>,
Longfei Wang <longfei.wang@...iatek.com>,
Yunfei Dong <yunfei.dong@...iatek.com>,
linux-media@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
srv_heupstream@...iatek.com, linux-mediatek@...ts.infradead.org,
Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v4, 4/8] media: mediatek: vcodec: Add more extra
processing for multi-core encoding
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@...iatek.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
Powered by blists - more mailing lists