Re: [PATCH v7, 03/15] media: mtk-vcodec: Refactor vcodec pm interface
From: "yunfei.dong@mediatek.com" <yunfei.dong@mediatek.com>
Date: 2021-10-29 03:29:45
Also in:
dri-devel, linux-arm-kernel, linux-media, linux-mediatek, lkml
Hi Dafna, Thanks for your suggestion. On Thu, 2021-10-14 at 15:44 +0200, Dafna Hirschfeld wrote:
On 11.10.21 09:02, Yunfei Dong wrote:quoted
Using the needed param for pm init/release function and remove unused param mtkdev in 'struct mtk_vcodec_pm'.I see that there is a lot of code duplication between mtk_vcodec_release_dec_pm.c and mtk_vcodec_release_enc_pm.c I think if you bother to factor the decoder you should do the same factor to the encoder, but actually the much better thing to do is to unify all code duplication between these two files, just for example of identical functions: mtk_vcodec_enc/dec_clock_on/off mtk_vcodec_release_enc_pm mtk_vcodec_init_dec_pm In addition, the function mtk_vcodec_dec_pw_on can be remove since it only calls pm_runtime_resume_and_get. It would be much better to call pm_runtime_resume_and_get directly and not hide it in a different function
I will fix it in next patch series. It's not very reasonable in this patch series.
Thanks, Dafna
Thanks, Yunfei Dong
quoted
Reviewed-by: Tzung-Bi Shih <redacted> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> --- .../platform/mtk-vcodec/mtk_vcodec_dec_drv.c | 6 ++--- .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 22 ++++++++---- ------- .../platform/mtk-vcodec/mtk_vcodec_dec_pm.h | 5 +++-- .../platform/mtk-vcodec/mtk_vcodec_drv.h | 1 - .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 1 - 5 files changed, 15 insertions(+), 20 deletions(-)diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.cb/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c index 8db9cdc66043..dd749d41c75a 100644--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_drv.c@@ -249,7 +249,7 @@ static int mtk_vcodec_probe(structplatform_device *pdev) if (IS_ERR(dev->fw_handler)) return PTR_ERR(dev->fw_handler); - ret = mtk_vcodec_init_dec_pm(dev); + ret = mtk_vcodec_init_dec_pm(dev->plat_dev, &dev->pm); if (ret < 0) { dev_err(&pdev->dev, "Failed to get mt vcodec clock source"); goto err_dec_pm;@@ -379,7 +379,7 @@ static int mtk_vcodec_probe(structplatform_device *pdev) err_dec_alloc: v4l2_device_unregister(&dev->v4l2_dev); err_res: - mtk_vcodec_release_dec_pm(dev); + mtk_vcodec_release_dec_pm(&dev->pm); err_dec_pm: mtk_vcodec_fw_release(dev->fw_handler); return ret;@@ -422,7 +422,7 @@ static int mtk_vcodec_dec_remove(structplatform_device *pdev) video_unregister_device(dev->vfd_dec); v4l2_device_unregister(&dev->v4l2_dev); - mtk_vcodec_release_dec_pm(dev); + mtk_vcodec_release_dec_pm(&dev->pm); mtk_vcodec_fw_release(dev->fw_handler); return 0; }diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.cb/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c index 6038db96f71c..20bd157a855c 100644--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c@@ -13,18 +13,15 @@ #include "mtk_vcodec_dec_pm.h" #include "mtk_vcodec_util.h" -int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *mtkdev) +int mtk_vcodec_init_dec_pm(struct platform_device *pdev, + struct mtk_vcodec_pm *pm) { struct device_node *node; - struct platform_device *pdev; - struct mtk_vcodec_pm *pm; + struct platform_device *larb_pdev; struct mtk_vcodec_clk *dec_clk; struct mtk_vcodec_clk_info *clk_info; int i = 0, ret = 0; - pdev = mtkdev->plat_dev; - pm = &mtkdev->pm; - pm->mtkdev = mtkdev; dec_clk = &pm->vdec_clk; node = of_parse_phandle(pdev->dev.of_node, "mediatek,larb", 0); if (!node) {@@ -32,13 +29,12 @@ int mtk_vcodec_init_dec_pm(structmtk_vcodec_dev *mtkdev) return -1; } - pdev = of_find_device_by_node(node); + larb_pdev = of_find_device_by_node(node); of_node_put(node); - if (WARN_ON(!pdev)) { + if (WARN_ON(!larb_pdev)) { return -1; } - pm->larbvdec = &pdev->dev; - pdev = mtkdev->plat_dev; + pm->larbvdec = &larb_pdev->dev; pm->dev = &pdev->dev; dec_clk->clk_num =@@ -82,10 +78,10 @@ int mtk_vcodec_init_dec_pm(structmtk_vcodec_dev *mtkdev) return ret; } -void mtk_vcodec_release_dec_pm(struct mtk_vcodec_dev *dev) +void mtk_vcodec_release_dec_pm(struct mtk_vcodec_pm *pm) { - pm_runtime_disable(dev->pm.dev); - put_device(dev->pm.larbvdec); + pm_runtime_disable(pm->dev); + put_device(pm->larbvdec); } int mtk_vcodec_dec_pw_on(struct mtk_vcodec_pm *pm)diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.hb/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.h index 280aeaefdb65..a3df6aef6cb9 100644--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.h +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.h@@ -9,8 +9,9 @@ #include "mtk_vcodec_drv.h" -int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *dev); -void mtk_vcodec_release_dec_pm(struct mtk_vcodec_dev *dev); +int mtk_vcodec_init_dec_pm(struct platform_device *pdev, + struct mtk_vcodec_pm *pm); +void mtk_vcodec_release_dec_pm(struct mtk_vcodec_pm *pm); int mtk_vcodec_dec_pw_on(struct mtk_vcodec_pm *pm); void mtk_vcodec_dec_pw_off(struct mtk_vcodec_pm *pm);diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.hb/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h index 3b1e5e3a450e..973b0b3649c6 100644--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h@@ -195,7 +195,6 @@ struct mtk_vcodec_pm { struct mtk_vcodec_clk venc_clk; struct device *larbvenc; struct device *dev; - struct mtk_vcodec_dev *mtkdev; }; /**diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.cb/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c index 1b2e4930ed27..0c8c8f86788c 100644--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c@@ -26,7 +26,6 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev*mtkdev) pdev = mtkdev->plat_dev; pm = &mtkdev->pm; memset(pm, 0, sizeof(struct mtk_vcodec_pm)); - pm->mtkdev = mtkdev; pm->dev = &pdev->dev; dev = &pdev->dev; enc_clk = &pm->venc_clk;