2022-10-01 03:35:16

by Irui Wang (王瑞)

[permalink] [raw]
Subject: [PATCH v6, 6/8] media: mediatek: vcodec: Refactor encoder clock on/off function

when enable multi-core encoding, encoder cores use their own clock,
refactor clock management functions with used encoder hardware id.

Signed-off-by: Irui Wang <[email protected]>
---
.../mediatek/vcodec/mtk_vcodec_enc_pm.c | 89 ++++++++++++++++---
.../mediatek/vcodec/mtk_vcodec_enc_pm.h | 6 +-
.../platform/mediatek/vcodec/venc_drv_if.c | 4 +-
3 files changed, 84 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.c
index 213c3f50e9eb..2f83aade779a 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.c
@@ -60,7 +60,9 @@ EXPORT_SYMBOL_GPL(mtk_vcodec_init_enc_clk);
static int mtk_enc_core_power_on(struct mtk_vcodec_ctx *ctx)
{
struct mtk_venc_hw_dev *sub_core;
+ struct mtk_vcodec_clk *clk;
int ret, i;
+ int j = 0;

/* multi-core encoding need power on all available cores */
for (i = 0; i < MTK_VENC_HW_MAX; i++) {
@@ -73,12 +75,27 @@ static int mtk_enc_core_power_on(struct mtk_vcodec_ctx *ctx)
mtk_v4l2_err("power on sub_core[%d] fail %d", i, ret);
goto pm_on_fail;
}
+
+ clk = &sub_core->pm.venc_clk;
+ for (j = 0; j < clk->clk_num; j++) {
+ ret = clk_prepare(clk->clk_info[j].vcodec_clk);
+ if (ret) {
+ mtk_v4l2_err("prepare clk [%s] fail %d",
+ clk->clk_info[j].clk_name, ret);
+ goto pm_on_fail;
+ }
+ }
}
return ret;

pm_on_fail:
for (i -= 1; i >= 0; i--) {
sub_core = (struct mtk_venc_hw_dev *)ctx->dev->enc_hw_dev[i];
+
+ clk = &sub_core->pm.venc_clk;
+ for (j -= 1; j >= 0; j--)
+ clk_unprepare(clk->clk_info[j].vcodec_clk);
+
pm_runtime_put_sync(&sub_core->plat_dev->dev);
}
return ret;
@@ -87,7 +104,9 @@ static int mtk_enc_core_power_on(struct mtk_vcodec_ctx *ctx)
int mtk_vcodec_enc_power_on(struct mtk_vcodec_ctx *ctx)
{
struct mtk_vcodec_pm *pm = &ctx->dev->pm;
+ struct mtk_vcodec_clk *clk;
int ret;
+ int i = 0;

ret = pm_runtime_resume_and_get(pm->dev);
if (ret) {
@@ -95,6 +114,16 @@ int mtk_vcodec_enc_power_on(struct mtk_vcodec_ctx *ctx)
return ret;
}

+ clk = &pm->venc_clk;
+ for (i = 0; i < clk->clk_num; i++) {
+ ret = clk_prepare(clk->clk_info[i].vcodec_clk);
+ if (ret) {
+ mtk_v4l2_err("prepare clk [%s] fail %d",
+ clk->clk_info[i].clk_name, ret);
+ goto clk_error;
+ }
+ }
+
if (IS_VENC_MULTICORE(ctx->dev->enc_capability)) {
ret = mtk_enc_core_power_on(ctx);
if (ret) {
@@ -104,6 +133,9 @@ int mtk_vcodec_enc_power_on(struct mtk_vcodec_ctx *ctx)
}
return ret;

+clk_error:
+ for (i -= 1; i >= 0; i--)
+ clk_unprepare(clk->clk_info[i].vcodec_clk);
core_error:
pm_runtime_put_sync(pm->dev);
return ret;
@@ -112,7 +144,8 @@ int mtk_vcodec_enc_power_on(struct mtk_vcodec_ctx *ctx)
static void mtk_enc_core_power_off(struct mtk_vcodec_ctx *ctx)
{
struct mtk_venc_hw_dev *sub_core;
- int ret, i;
+ struct mtk_vcodec_clk *clk;
+ int ret, i, j;

/* multi-core encoding need power off all available cores */
for (i = 0; i < MTK_VENC_HW_MAX; i++) {
@@ -120,6 +153,10 @@ static void mtk_enc_core_power_off(struct mtk_vcodec_ctx *ctx)
if (!sub_core)
continue;

+ clk = &sub_core->pm.venc_clk;
+ for (j = clk->clk_num - 1; j >= 0; j--)
+ clk_unprepare(clk->clk_info[j].vcodec_clk);
+
ret = pm_runtime_put_sync(&sub_core->plat_dev->dev);
if (ret)
mtk_v4l2_err("power off sub_core[%d] fail %d", i, ret);
@@ -129,26 +166,44 @@ static void mtk_enc_core_power_off(struct mtk_vcodec_ctx *ctx)
void mtk_vcodec_enc_power_off(struct mtk_vcodec_ctx *ctx)
{
struct mtk_vcodec_pm *pm = &ctx->dev->pm;
- int ret;
+ struct mtk_vcodec_clk *clk;
+ int ret, i;

if (IS_VENC_MULTICORE(ctx->dev->enc_capability))
mtk_enc_core_power_off(ctx);

+ clk = &pm->venc_clk;
+ for (i = clk->clk_num - 1; i >= 0; i--)
+ clk_unprepare(clk->clk_info[i].vcodec_clk);
+
ret = pm_runtime_put_sync(pm->dev);
if (ret)
mtk_v4l2_err("pm_runtime_put_sync fail %d", ret);
}

-void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm)
+void mtk_vcodec_enc_clock_on(struct mtk_vcodec_dev *dev,
+ enum mtk_venc_hw_id hw_id)
{
- struct mtk_vcodec_clk *enc_clk = &pm->venc_clk;
+ struct mtk_venc_hw_dev *sub_core;
+ struct mtk_vcodec_clk *enc_clk;
+
int ret, i = 0;

+ if (hw_id == MTK_VENC_CORE_0) {
+ enc_clk = &dev->pm.venc_clk;
+ } else if (hw_id == MTK_VENC_CORE_1) {
+ sub_core = (struct mtk_venc_hw_dev *)dev->enc_hw_dev[hw_id];
+ enc_clk = &sub_core->pm.venc_clk;
+ } else {
+ mtk_v4l2_err("invalid hw id : %d", hw_id);
+ return;
+ }
+
for (i = 0; i < enc_clk->clk_num; i++) {
- ret = clk_prepare_enable(enc_clk->clk_info[i].vcodec_clk);
+ ret = clk_enable(enc_clk->clk_info[i].vcodec_clk);
if (ret) {
- mtk_v4l2_err("venc clk_prepare_enable %d %s fail %d", i,
- enc_clk->clk_info[i].clk_name, ret);
+ mtk_v4l2_err("venc clk_enable %d %s fail %d", i,
+ enc_clk->clk_info[i].clk_name, ret);
goto clkerr;
}
}
@@ -157,14 +212,26 @@ void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm)

clkerr:
for (i -= 1; i >= 0; i--)
- clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk);
+ clk_disable(enc_clk->clk_info[i].vcodec_clk);
}

-void mtk_vcodec_enc_clock_off(struct mtk_vcodec_pm *pm)
+void mtk_vcodec_enc_clock_off(struct mtk_vcodec_dev *dev,
+ enum mtk_venc_hw_id hw_id)
{
- struct mtk_vcodec_clk *enc_clk = &pm->venc_clk;
+ struct mtk_venc_hw_dev *sub_core;
+ struct mtk_vcodec_clk *enc_clk;
int i = 0;

+ if (hw_id == MTK_VENC_CORE_0) {
+ enc_clk = &dev->pm.venc_clk;
+ } else if (hw_id == MTK_VENC_CORE_1) {
+ sub_core = (struct mtk_venc_hw_dev *)dev->enc_hw_dev[hw_id];
+ enc_clk = &sub_core->pm.venc_clk;
+ } else {
+ mtk_v4l2_err("invalid hw id : %d", hw_id);
+ return;
+ }
+
for (i = enc_clk->clk_num - 1; i >= 0; i--)
- clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk);
+ clk_disable(enc_clk->clk_info[i].vcodec_clk);
}
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.h
index 9065dec4ed4f..a2906d2971ee 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.h
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.h
@@ -14,7 +14,9 @@ int mtk_vcodec_init_enc_clk(struct platform_device *pdev,

int mtk_vcodec_enc_power_on(struct mtk_vcodec_ctx *ctx);
void mtk_vcodec_enc_power_off(struct mtk_vcodec_ctx *ctx);
-void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm);
-void mtk_vcodec_enc_clock_off(struct mtk_vcodec_pm *pm);
+void mtk_vcodec_enc_clock_on(struct mtk_vcodec_dev *dev,
+ enum mtk_venc_hw_id hw_id);
+void mtk_vcodec_enc_clock_off(struct mtk_vcodec_dev *dev,
+ enum mtk_venc_hw_id hw_id);

#endif /* _MTK_VCODEC_ENC_PM_H_ */
diff --git a/drivers/media/platform/mediatek/vcodec/venc_drv_if.c b/drivers/media/platform/mediatek/vcodec/venc_drv_if.c
index 65a27e39ef5b..6cbdb7e30bb3 100644
--- a/drivers/media/platform/mediatek/vcodec/venc_drv_if.c
+++ b/drivers/media/platform/mediatek/vcodec/venc_drv_if.c
@@ -64,10 +64,10 @@ int venc_if_encode(struct mtk_vcodec_ctx *ctx,
ctx->dev->curr_ctx = ctx;
spin_unlock_irqrestore(&ctx->dev->irqlock, flags);

- mtk_vcodec_enc_clock_on(&ctx->dev->pm);
+ mtk_vcodec_enc_clock_on(ctx->dev, 0);
ret = ctx->enc_if->encode(ctx->drv_handle, opt, frm_buf,
bs_buf, result);
- mtk_vcodec_enc_clock_off(&ctx->dev->pm);
+ mtk_vcodec_enc_clock_off(ctx->dev, 0);

spin_lock_irqsave(&ctx->dev->irqlock, flags);
ctx->dev->curr_ctx = NULL;
--
2.18.0


2022-10-04 10:48:19

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH v6, 6/8] media: mediatek: vcodec: Refactor encoder clock on/off function

On Sat, 1 Oct 2022, Irui Wang wrote:

> when enable multi-core encoding, encoder cores use their own clock,
> refactor clock management functions with used encoder hardware id.
>
> Signed-off-by: Irui Wang <[email protected]>
> ---
> .../mediatek/vcodec/mtk_vcodec_enc_pm.c | 89 ++++++++++++++++---
> .../mediatek/vcodec/mtk_vcodec_enc_pm.h | 6 +-
> .../platform/mediatek/vcodec/venc_drv_if.c | 4 +-
> 3 files changed, 84 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.c
> index 213c3f50e9eb..2f83aade779a 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.c
> @@ -60,7 +60,9 @@ EXPORT_SYMBOL_GPL(mtk_vcodec_init_enc_clk);
> static int mtk_enc_core_power_on(struct mtk_vcodec_ctx *ctx)
> {
> struct mtk_venc_hw_dev *sub_core;
> + struct mtk_vcodec_clk *clk;
> int ret, i;
> + int j = 0;
>
> /* multi-core encoding need power on all available cores */
> for (i = 0; i < MTK_VENC_HW_MAX; i++) {
> @@ -73,12 +75,27 @@ static int mtk_enc_core_power_on(struct mtk_vcodec_ctx *ctx)
> mtk_v4l2_err("power on sub_core[%d] fail %d", i, ret);
> goto pm_on_fail;
> }
> +
> + clk = &sub_core->pm.venc_clk;
> + for (j = 0; j < clk->clk_num; j++) {
> + ret = clk_prepare(clk->clk_info[j].vcodec_clk);
> + if (ret) {
> + mtk_v4l2_err("prepare clk [%s] fail %d",
> + clk->clk_info[j].clk_name, ret);
> + goto pm_on_fail;
> + }
> + }
> }
> return ret;
>
> pm_on_fail:
> for (i -= 1; i >= 0; i--) {
> sub_core = (struct mtk_venc_hw_dev *)ctx->dev->enc_hw_dev[i];
> +
> + clk = &sub_core->pm.venc_clk;
> + for (j -= 1; j >= 0; j--)
> + clk_unprepare(clk->clk_info[j].vcodec_clk);
> +
> pm_runtime_put_sync(&sub_core->plat_dev->dev);

There's more than one thing wrong here.

pm_runtime_put_sync() won't be called for the ith entry when the later
goto pm_on_fail is taken because the loop decrements i right at the
start.

Similarly, i and j will mismatch for the ith entry because i was
decremented.

Third, j does not start from clk->clk_num - 1 for the other entries
(for those lower "i"s).


--
i.