| 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: <e6a1ed5937c3a2182ba6b7a90572f905a62269f3.camel@collabora.com>
Date: Mon, 19 Jun 2023 10:53:38 -0400
From: Nicolas Dufresne <nicolas.dufresne@...labora.com>
To: Yunfei Dong <yunfei.dong@...iatek.com>,
Nícolas "F . R . A . Prado"
<nfraprado@...labora.com>, Hans Verkuil <hverkuil-cisco@...all.nl>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>,
Benjamin Gaignard <benjamin.gaignard@...labora.com>,
Nathan Hebert <nhebert@...omium.org>
Cc: Chen-Yu Tsai <wenst@...omium.org>,
Hsin-Yi Wang <hsinyi@...omium.org>,
Fritz Koenig <frkoenig@...omium.org>,
Daniel Vetter <daniel@...ll.ch>,
Steve Cho <stevecho@...omium.org>, linux-media@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org,
Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v3,03/11] media: mediatek: vcodec: re-write shared
interface
Hi Yunfei,
Le samedi 17 juin 2023 à 18:32 +0800, Yunfei Dong a écrit :
> Re-write shared interface which encoder and decoder used at
> the same time. Using the common struct as the parameter of
> these interface in order to remove the depedency.
>
> Signed-off-by: Yunfei Dong <yunfei.dong@...iatek.com>
> ---
> .../mediatek/vcodec/mtk_vcodec_intr.c | 30 ++++++++++++-------
> .../mediatek/vcodec/mtk_vcodec_intr.h | 3 +-
> .../mediatek/vcodec/mtk_vcodec_util.c | 19 +++++-------
> .../mediatek/vcodec/mtk_vcodec_util.h | 9 ++----
> .../mediatek/vcodec/vdec/vdec_vp8_if.c | 14 ++++-----
> .../mediatek/vcodec/venc/venc_h264_if.c | 2 +-
> .../mediatek/vcodec/venc/venc_vp8_if.c | 2 +-
> 7 files changed, 39 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_intr.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_intr.c
> index 552b4c93d972..daa44f635727 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_intr.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_intr.c
> @@ -11,32 +11,40 @@
> #include "mtk_vcodec_intr.h"
> #include "mtk_vcodec_util.h"
>
> -int mtk_vcodec_wait_for_done_ctx(struct mtk_vcodec_ctx *ctx,
> - int command, unsigned int timeout_ms,
> +int mtk_vcodec_wait_for_done_ctx(void *priv, int command, unsigned int timeout_ms,
> unsigned int hw_id)
> {
> + struct mtk_vcodec_ctx *ctx = priv;
> long timeout_jiff, ret;
> - int status = 0;
> + int status = 0, ctx_id, ctx_type;
> + int *ctx_int_cond, *ctx_int_type;
> + wait_queue_head_t *ctx_queue;
> +
> + ctx_id = ctx->id;
> + ctx_type = ctx->type;
> + ctx_int_cond = ctx->int_cond;
> + ctx_int_type = ctx->int_type;
> + ctx_queue = ctx->queue;
>
> timeout_jiff = msecs_to_jiffies(timeout_ms);
> - ret = wait_event_interruptible_timeout(ctx->queue[hw_id],
> - ctx->int_cond[hw_id],
> + ret = wait_event_interruptible_timeout(ctx_queue[hw_id],
> + ctx_int_cond[hw_id],
> timeout_jiff);
>
> if (!ret) {
> status = -1; /* timeout */
> mtk_v4l2_err("[%d] cmd=%d, type=%d, dec timeout=%ums (%d %d)",
> - ctx->id, command, ctx->type, timeout_ms,
> - ctx->int_cond[hw_id], ctx->int_type[hw_id]);
> + ctx_id, command, ctx_type, timeout_ms,
> + ctx_int_cond[hw_id], ctx_int_type[hw_id]);
> } else if (-ERESTARTSYS == ret) {
> status = -1;
> mtk_v4l2_err("[%d] cmd=%d, type=%d, dec inter fail (%d %d)",
> - ctx->id, command, ctx->type,
> - ctx->int_cond[hw_id], ctx->int_type[hw_id]);
> + ctx_id, command, ctx_type,
> + ctx_int_cond[hw_id], ctx_int_type[hw_id]);
> }
>
> - ctx->int_cond[hw_id] = 0;
> - ctx->int_type[hw_id] = 0;
> + ctx_int_cond[hw_id] = 0;
> + ctx_int_type[hw_id] = 0;
>
> return status;
> }
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_intr.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_intr.h
> index 9681f492813b..11bf0ef94d5d 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_intr.h
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_intr.h
> @@ -12,8 +12,7 @@
> struct mtk_vcodec_ctx;
You have a forward declaration here.
>
> /* timeout is ms */
> -int mtk_vcodec_wait_for_done_ctx(struct mtk_vcodec_ctx *ctx,
> - int command, unsigned int timeout_ms,
> +int mtk_vcodec_wait_for_done_ctx(void *priv, int command, unsigned int timeout_ms,
> unsigned int hw_id);
So has the CTX is only uses has a pointer, its hard to follow why you need to
hide the type here. At least its not clear to me how this helps with the goal
set in the commit message and would simply like to understand before giving an
r-b.
>
> #endif /* _MTK_VCODEC_INTR_H_ */
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.c
> index f214e6f67005..847e321f4fcc 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.c
> @@ -21,24 +21,20 @@ int mtk_v4l2_dbg_level;
> EXPORT_SYMBOL(mtk_v4l2_dbg_level);
> #endif
>
> -void __iomem *mtk_vcodec_get_reg_addr(struct mtk_vcodec_ctx *data,
> - unsigned int reg_idx)
> +void __iomem *mtk_vcodec_get_reg_addr(void __iomem **reg_base, unsigned int reg_idx)
> {
> - struct mtk_vcodec_ctx *ctx = (struct mtk_vcodec_ctx *)data;
> -
> - if (!data || reg_idx >= NUM_MAX_VCODEC_REG_BASE) {
> + if (reg_idx >= NUM_MAX_VCODEC_REG_BASE) {
> mtk_v4l2_err("Invalid arguments, reg_idx=%d", reg_idx);
> return NULL;
> }
> - return ctx->dev->reg_base[reg_idx];
> + return reg_base[reg_idx];
> }
> EXPORT_SYMBOL(mtk_vcodec_get_reg_addr);
>
> -int mtk_vcodec_mem_alloc(struct mtk_vcodec_ctx *data,
> - struct mtk_vcodec_mem *mem)
> +int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem)
> {
> unsigned long size = mem->size;
> - struct mtk_vcodec_ctx *ctx = (struct mtk_vcodec_ctx *)data;
> + struct mtk_vcodec_ctx *ctx = priv;
> struct device *dev = &ctx->dev->plat_dev->dev;
>
> mem->va = dma_alloc_coherent(dev, size, &mem->dma_addr, GFP_KERNEL);
> @@ -57,11 +53,10 @@ int mtk_vcodec_mem_alloc(struct mtk_vcodec_ctx *data,
> }
> EXPORT_SYMBOL(mtk_vcodec_mem_alloc);
>
> -void mtk_vcodec_mem_free(struct mtk_vcodec_ctx *data,
> - struct mtk_vcodec_mem *mem)
> +void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem)
> {
> unsigned long size = mem->size;
> - struct mtk_vcodec_ctx *ctx = (struct mtk_vcodec_ctx *)data;
> + struct mtk_vcodec_ctx *ctx = priv;
> struct device *dev = &ctx->dev->plat_dev->dev;
>
> if (!mem->va) {
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> index 88d389b65f13..827937bcb4b4 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> @@ -68,12 +68,9 @@ extern int mtk_vcodec_dbg;
> #define mtk_vcodec_debug_enter(h) mtk_vcodec_debug(h, "+")
> #define mtk_vcodec_debug_leave(h) mtk_vcodec_debug(h, "-")
>
> -void __iomem *mtk_vcodec_get_reg_addr(struct mtk_vcodec_ctx *data,
> - unsigned int reg_idx);
> -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,
> - struct mtk_vcodec_mem *mem);
> +void __iomem *mtk_vcodec_get_reg_addr(void __iomem **reg_base, unsigned int reg_idx);
> +int mtk_vcodec_mem_alloc(void *priv, struct mtk_vcodec_mem *mem);
> +void mtk_vcodec_mem_free(void *priv, struct mtk_vcodec_mem *mem);
> void mtk_vcodec_set_curr_ctx(struct mtk_vcodec_dev *vdec_dev,
> struct mtk_vcodec_ctx *ctx, int hw_idx);
> struct mtk_vcodec_ctx *mtk_vcodec_get_curr_ctx(struct mtk_vcodec_dev *vdec_dev,
> diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp8_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp8_if.c
> index 88c046731754..5edbccc9ae68 100644
> --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp8_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp8_if.c
> @@ -167,13 +167,13 @@ struct vdec_vp8_inst {
>
> static void get_hw_reg_base(struct vdec_vp8_inst *inst)
> {
> - inst->reg_base.top = mtk_vcodec_get_reg_addr(inst->ctx, VDEC_TOP);
> - inst->reg_base.cm = mtk_vcodec_get_reg_addr(inst->ctx, VDEC_CM);
> - inst->reg_base.hwd = mtk_vcodec_get_reg_addr(inst->ctx, VDEC_HWD);
> - inst->reg_base.sys = mtk_vcodec_get_reg_addr(inst->ctx, VDEC_SYS);
> - inst->reg_base.misc = mtk_vcodec_get_reg_addr(inst->ctx, VDEC_MISC);
> - inst->reg_base.ld = mtk_vcodec_get_reg_addr(inst->ctx, VDEC_LD);
> - inst->reg_base.hwb = mtk_vcodec_get_reg_addr(inst->ctx, VDEC_HWB);
> + inst->reg_base.top = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VDEC_TOP);
> + inst->reg_base.cm = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VDEC_CM);
> + inst->reg_base.hwd = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VDEC_HWD);
> + inst->reg_base.sys = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VDEC_SYS);
> + inst->reg_base.misc = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VDEC_MISC);
> + inst->reg_base.ld = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VDEC_LD);
> + inst->reg_base.hwb = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VDEC_HWB);
> }
>
> static void write_hw_segmentation_data(struct vdec_vp8_inst *inst)
> 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 60fd165c0d94..10365c95ebbe 100644
> --- a/drivers/media/platform/mediatek/vcodec/venc/venc_h264_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/venc/venc_h264_if.c
> @@ -612,7 +612,7 @@ static int h264_enc_init(struct mtk_vcodec_ctx *ctx)
> inst->ctx = ctx;
> inst->vpu_inst.ctx = ctx;
> inst->vpu_inst.id = is_ext ? SCP_IPI_VENC_H264 : IPI_VENC_H264;
> - inst->hw_base = mtk_vcodec_get_reg_addr(inst->ctx, VENC_SYS);
> + inst->hw_base = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VENC_SYS);
>
> mtk_vcodec_debug_enter(inst);
>
> diff --git a/drivers/media/platform/mediatek/vcodec/venc/venc_vp8_if.c b/drivers/media/platform/mediatek/vcodec/venc/venc_vp8_if.c
> index 56ce58f761f1..73ebc35d7c99 100644
> --- a/drivers/media/platform/mediatek/vcodec/venc/venc_vp8_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/venc/venc_vp8_if.c
> @@ -336,7 +336,7 @@ static int vp8_enc_init(struct mtk_vcodec_ctx *ctx)
> inst->ctx = ctx;
> inst->vpu_inst.ctx = ctx;
> inst->vpu_inst.id = IPI_VENC_VP8;
> - inst->hw_base = mtk_vcodec_get_reg_addr(inst->ctx, VENC_LT_SYS);
> + inst->hw_base = mtk_vcodec_get_reg_addr(inst->ctx->dev->reg_base, VENC_LT_SYS);
>
> mtk_vcodec_debug_enter(inst);
>
Powered by blists - more mailing lists