2023-06-26 03:56:32

by Yunfei Dong (董云飞)

[permalink] [raw]
Subject: [PATCH v4,03/11] media: mediatek: vcodec: Removing struct 'mtk_vcodec_ctx/dev' for shared interface

The shared struct 'mtk_vcodec_ctx/dev' will be changed to 'mtk_vcodec_enc_ctx/dev'
and 'mtk_vcodec_dec_ctx/dev' in order to separate encoder and decoder. Removing
common struct 'mtk_vcodec_ctx/dev' for shared interface which encoder and decoder
used at the same time. Then encoder and decoder can call the same interface
independently.

Signed-off-by: Yunfei Dong <[email protected]>
---
.../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;

/* 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);

#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);

--
2.18.0



2023-07-03 20:16:15

by Nicolas Dufresne

[permalink] [raw]
Subject: Re: [PATCH v4,03/11] media: mediatek: vcodec: Removing struct 'mtk_vcodec_ctx/dev' for shared interface

Le lundi 26 juin 2023 à 11:53 +0800, Yunfei Dong a écrit :
> The shared struct 'mtk_vcodec_ctx/dev' will be changed to 'mtk_vcodec_enc_ctx/dev'
> and 'mtk_vcodec_dec_ctx/dev' in order to separate encoder and decoder. Removing
> common struct 'mtk_vcodec_ctx/dev' for shared interface which encoder and decoder
> used at the same time. Then encoder and decoder can call the same interface
> independently.
>
> Signed-off-by: Yunfei Dong <[email protected]>
> ---
> .../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;
>
> /* 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);
>
> #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)
> {

I'd like to suggest:

+ u32 reg_base = inst->ctx->dev->reg_base

> - 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);

+ inst->reg_base.top = mtk_vcodec_get_reg_addr(reg_base, VDEC_TOP);
+ inst->reg_base.cm = mtk_vcodec_get_reg_addr(reg_base, VDEC_CM);
+ inst->reg_base.hwd = mtk_vcodec_get_reg_addr(reg_base, VDEC_HWD);
+ inst->reg_base.sys = mtk_vcodec_get_reg_addr(reg_base, VDEC_SYS);
+ inst->reg_base.misc = mtk_vcodec_get_reg_addr(reg_base, VDEC_MISC);
+ inst->reg_base.ld = mtk_vcodec_get_reg_addr(reg_base, VDEC_LD);
+ inst->reg_base.hwb = mtk_vcodec_get_reg_addr(reg_base, VDEC_HWB);

With such minor stylistic change:

Reviewed-by: Nicolas Dufresne <[email protected]>

> }
>
> 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);
>


2023-07-04 11:14:25

by Yunfei Dong (董云飞)

[permalink] [raw]
Subject: Re: [PATCH v4,03/11] media: mediatek: vcodec: Removing struct 'mtk_vcodec_ctx/dev' for shared interface

Hi Nicolas,

Thanks for you suggestion.
On Mon, 2023-07-03 at 16:09 -0400, Nicolas Dufresne wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Le lundi 26 juin 2023 à 11:53 +0800, Yunfei Dong a écrit :
> > The shared struct 'mtk_vcodec_ctx/dev' will be changed to
> 'mtk_vcodec_enc_ctx/dev'
> > and 'mtk_vcodec_dec_ctx/dev' in order to separate encoder and
> decoder. Removing
> > common struct 'mtk_vcodec_ctx/dev' for shared interface which
> encoder and decoder
> > used at the same time. Then encoder and decoder can call the same
> interface
> > independently.
> >
> > Signed-off-by: Yunfei Dong <[email protected]>
> > ---
> > .../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;
> >
> > /* 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);
> >
> > #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)
> > {
>
> I'd like to suggest:
>
> +u32 reg_base = inst->ctx->dev->reg_base
>
> > -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);
>
> +inst->reg_base.top = mtk_vcodec_get_reg_addr(reg_base, VDEC_TOP);
> +inst->reg_base.cm = mtk_vcodec_get_reg_addr(reg_base, VDEC_CM);
> +inst->reg_base.hwd = mtk_vcodec_get_reg_addr(reg_base, VDEC_HWD);
> +inst->reg_base.sys = mtk_vcodec_get_reg_addr(reg_base, VDEC_SYS);
> +inst->reg_base.misc = mtk_vcodec_get_reg_addr(reg_base, VDEC_MISC);
> +inst->reg_base.ld = mtk_vcodec_get_reg_addr(reg_base, VDEC_LD);
> +inst->reg_base.hwb = mtk_vcodec_get_reg_addr(reg_base, VDEC_HWB);
>
> With such minor stylistic change:
>
> Reviewed-by: Nicolas Dufresne <[email protected]>
>

Fixed in patch v5, please help to review it again.

Best Regards,
Yunfei Dong
> > }
> >
> > 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);
> >
>