| Message ID | 20210410091128.31823-1-yong.wu@mediatek.com |
|---|---|
| Headers | show |
| Series | Clean up "mediatek,larb" | expand |
On 10/04/2021 11:11, Yong Wu wrote: > Normally, If the smi-larb HW need work, we should enable the smi-common > HW power and clock firstly. > This patch adds device-link between the smi-larb dev and the smi-common > dev. then If pm_runtime_get_sync(smi-larb-dev), the pm_runtime_get_sync > (smi-common-dev) will be called automatically. > > Also, Add DL_FLAG_STATELESS to avoid the smi-common clocks be gated when > probe. > > CC: Matthias Brugger <matthias.bgg@gmail.com> > Suggested-by: Tomasz Figa <tfiga@chromium.org> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > drivers/memory/mtk-smi.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) I understood this is a dependency for other patches, so: Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> If I am wrong and I can take it via memory tree, let me know. Best regards, Krzysztof
On Sat, 2021-04-10 at 14:40 +0200, Krzysztof Kozlowski wrote: > On 10/04/2021 11:11, Yong Wu wrote: > > Normally, If the smi-larb HW need work, we should enable the smi-common > > HW power and clock firstly. > > This patch adds device-link between the smi-larb dev and the smi-common > > dev. then If pm_runtime_get_sync(smi-larb-dev), the pm_runtime_get_sync > > (smi-common-dev) will be called automatically. > > > > Also, Add DL_FLAG_STATELESS to avoid the smi-common clocks be gated when > > probe. > > > > CC: Matthias Brugger <matthias.bgg@gmail.com> > > Suggested-by: Tomasz Figa <tfiga@chromium.org> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > --- > > drivers/memory/mtk-smi.c | 19 ++++++++++--------- > > 1 file changed, 10 insertions(+), 9 deletions(-) > > I understood this is a dependency for other patches, so: > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > > If I am wrong and I can take it via memory tree, let me know. Hi Krzysztof, Thanks very much for your quickly review. I think it is ok if it go through memory tree. In the original patch, we pm_runtime_get(smi-common-dev) in the smi-larb's pm resume callback. This patch only use device-link do this. thus, this patch have no function change. it only adjusts the SMI internal code flow. In addition, [14/16] expects your Acked-by. and that one should be merged with the others. About the others patches, I'm not sure which tree they should go through. they cross several trees, dt-binding/iommu/media/drm/dts. Not sure if Matthias could have time to review and give some suggestion. > > Best regards, > Krzysztof > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
On 10/04/2021 11:11, Yong Wu wrote: > After adding device_link between the iommu consumer and smi-larb, > the pm_runtime_get(_sync) of smi-larb and smi-common will be called > automatically. we can get rid of mtk_smi_larb_get/put. > > CC: Matthias Brugger <matthias.bgg@gmail.com> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > Reviewed-by: Evan Green <evgreen@chromium.org> > --- > drivers/memory/mtk-smi.c | 14 -------------- > include/soc/mediatek/smi.h | 20 -------------------- > 2 files changed, 34 deletions(-) > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> Best regards, Krzysztof
On 13/04/2021 08:04, Yong Wu wrote: > On Sat, 2021-04-10 at 14:40 +0200, Krzysztof Kozlowski wrote: >> On 10/04/2021 11:11, Yong Wu wrote: >>> Normally, If the smi-larb HW need work, we should enable the smi-common >>> HW power and clock firstly. >>> This patch adds device-link between the smi-larb dev and the smi-common >>> dev. then If pm_runtime_get_sync(smi-larb-dev), the pm_runtime_get_sync >>> (smi-common-dev) will be called automatically. >>> >>> Also, Add DL_FLAG_STATELESS to avoid the smi-common clocks be gated when >>> probe. >>> >>> CC: Matthias Brugger <matthias.bgg@gmail.com> >>> Suggested-by: Tomasz Figa <tfiga@chromium.org> >>> Signed-off-by: Yong Wu <yong.wu@mediatek.com> >>> --- >>> drivers/memory/mtk-smi.c | 19 ++++++++++--------- >>> 1 file changed, 10 insertions(+), 9 deletions(-) >> >> I understood this is a dependency for other patches, so: >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> >> >> If I am wrong and I can take it via memory tree, let me know. > > Hi Krzysztof, > > Thanks very much for your quickly review. > > I think it is ok if it go through memory tree. In the original patch, we > pm_runtime_get(smi-common-dev) in the smi-larb's pm resume callback. > This patch only use device-link do this. thus, this patch have no > function change. it only adjusts the SMI internal code flow. Hm, okay, I took it then for v5.13. > > In addition, [14/16] expects your Acked-by. and that one should be > merged with the others. Thanks for reminder. Best regards, Krzysztof
On Sat, 10 Apr 2021 17:11:12 +0800, Yong Wu wrote: > MediaTek IOMMU block diagram always like below: > > M4U > | > smi-common > | > ------------- > | | ... > | | > larb1 larb2 > | | > vdec venc > > [...] Applied, thanks! [04/16] memory: mtk-smi: Add device-link between smi-larb and smi-common commit: 6ce2c05b21189eb17b3aa26720cc5841acf9dce8 Best regards,
On Sat, Apr 10, 2021 at 5:14 PM Yong Wu <yong.wu@mediatek.com> wrote: > > MediaTek IOMMU has already added the device_link between the consumer > and smi-larb device. If the vcodec device call the pm_runtime_get_sync, > the smi-larb's pm_runtime_get_sync also be called automatically. > > CC: Tiffany Lin <tiffany.lin@mediatek.com> > CC: Irui Wang <irui.wang@mediatek.com> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > Reviewed-by: Evan Green <evgreen@chromium.org> > Acked-by: Tiffany Lin <tiffany.lin@mediatek.com> > --- > .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 37 ++------------- > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 -- > .../platform/mtk-vcodec/mtk_vcodec_enc.c | 1 - > .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 46 ++----------------- > 4 files changed, 10 insertions(+), 77 deletions(-) > > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c > index 32e1858e9f1d..2b3562e47f4f 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c > @@ -8,14 +8,12 @@ > #include <linux/of_address.h> > #include <linux/of_platform.h> > #include <linux/pm_runtime.h> > -#include <soc/mediatek/smi.h> > > #include "mtk_vcodec_dec_pm.h" > #include "mtk_vcodec_util.h" > > int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *mtkdev) > { > - struct device_node *node; > struct platform_device *pdev; > struct mtk_vcodec_pm *pm; > struct mtk_vcodec_clk *dec_clk; > @@ -26,18 +24,7 @@ int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *mtkdev) > pm = &mtkdev->pm; > pm->mtkdev = mtkdev; > dec_clk = &pm->vdec_clk; > - node = of_parse_phandle(pdev->dev.of_node, "mediatek,larb", 0); > - if (!node) { > - mtk_v4l2_err("of_parse_phandle mediatek,larb fail!"); > - return -1; > - } > > - pdev = of_find_device_by_node(node); > - of_node_put(node); > - if (WARN_ON(!pdev)) { > - return -1; > - } > - pm->larbvdec = &pdev->dev; > pdev = mtkdev->plat_dev; > pm->dev = &pdev->dev; > > @@ -47,14 +34,11 @@ int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *mtkdev) > dec_clk->clk_info = devm_kcalloc(&pdev->dev, > dec_clk->clk_num, sizeof(*clk_info), > GFP_KERNEL); > - if (!dec_clk->clk_info) { > - ret = -ENOMEM; > - goto put_device; > - } > + if (!dec_clk->clk_info) > + return -ENOMEM; > } else { > mtk_v4l2_err("Failed to get vdec clock count"); > - ret = -EINVAL; > - goto put_device; > + return -EINVAL; > } > > for (i = 0; i < dec_clk->clk_num; i++) { > @@ -63,29 +47,24 @@ int mtk_vcodec_init_dec_pm(struct mtk_vcodec_dev *mtkdev) > "clock-names", i, &clk_info->clk_name); > if (ret) { > mtk_v4l2_err("Failed to get clock name id = %d", i); > - goto put_device; > + return ret; > } > clk_info->vcodec_clk = devm_clk_get(&pdev->dev, > clk_info->clk_name); > if (IS_ERR(clk_info->vcodec_clk)) { > mtk_v4l2_err("devm_clk_get (%d)%s fail", i, > clk_info->clk_name); > - ret = PTR_ERR(clk_info->vcodec_clk); > - goto put_device; > + return PTR_ERR(clk_info->vcodec_clk); > } > } > > pm_runtime_enable(&pdev->dev); > return 0; > -put_device: > - put_device(pm->larbvdec); > - return ret; > } > > void mtk_vcodec_release_dec_pm(struct mtk_vcodec_dev *dev) > { > pm_runtime_disable(dev->pm.dev); > - put_device(dev->pm.larbvdec); > } > > void mtk_vcodec_dec_pw_on(struct mtk_vcodec_pm *pm) > @@ -121,11 +100,6 @@ void mtk_vcodec_dec_clock_on(struct mtk_vcodec_pm *pm) > } > } > > - ret = mtk_smi_larb_get(pm->larbvdec); > - if (ret) { > - mtk_v4l2_err("mtk_smi_larb_get larbvdec fail %d", ret); > - goto error; > - } > return; > > error: > @@ -138,7 +112,6 @@ void mtk_vcodec_dec_clock_off(struct mtk_vcodec_pm *pm) > struct mtk_vcodec_clk *dec_clk = &pm->vdec_clk; > int i = 0; > > - mtk_smi_larb_put(pm->larbvdec); > for (i = dec_clk->clk_num - 1; i >= 0; i--) > clk_disable_unprepare(dec_clk->clk_info[i].vcodec_clk); > } > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > index 869d958d2b99..659790398809 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h > @@ -189,10 +189,7 @@ struct mtk_vcodec_clk { > */ > struct mtk_vcodec_pm { > struct mtk_vcodec_clk vdec_clk; > - struct device *larbvdec; > - > 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.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c > index 4831052f475d..59816981735b 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c > @@ -8,7 +8,6 @@ > #include <media/v4l2-event.h> > #include <media/v4l2-mem2mem.h> > #include <media/videobuf2-dma-contig.h> > -#include <soc/mediatek/smi.h> > #include <linux/pm_runtime.h> > > #include "mtk_vcodec_drv.h" > diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c > index 1b2e4930ed27..78b99ff882ae 100644 > --- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c > +++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c > @@ -8,58 +8,36 @@ > #include <linux/of_address.h> > #include <linux/of_platform.h> > #include <linux/pm_runtime.h> > -#include <soc/mediatek/smi.h> > > #include "mtk_vcodec_enc_pm.h" > #include "mtk_vcodec_util.h" > > int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev) > { > - struct device_node *node; > struct platform_device *pdev; > struct mtk_vcodec_pm *pm; > struct mtk_vcodec_clk *enc_clk; > struct mtk_vcodec_clk_info *clk_info; > int ret = 0, i = 0; > - struct device *dev; > > 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; > > - node = of_parse_phandle(dev->of_node, "mediatek,larb", 0); > - if (!node) { > - mtk_v4l2_err("no mediatek,larb found"); > - return -ENODEV; > - } > - pdev = of_find_device_by_node(node); > - of_node_put(node); > - if (!pdev) { > - mtk_v4l2_err("no mediatek,larb device found"); > - return -ENODEV; > - } > - pm->larbvenc = &pdev->dev; > - pdev = mtkdev->plat_dev; > - pm->dev = &pdev->dev; > - > enc_clk->clk_num = of_property_count_strings(pdev->dev.of_node, > "clock-names"); > if (enc_clk->clk_num > 0) { > enc_clk->clk_info = devm_kcalloc(&pdev->dev, > enc_clk->clk_num, sizeof(*clk_info), > GFP_KERNEL); > - if (!enc_clk->clk_info) { > - ret = -ENOMEM; > - goto put_larbvenc; > - } > + if (!enc_clk->clk_info) > + return -ENOMEM; > } else { > mtk_v4l2_err("Failed to get venc clock count"); > - ret = -EINVAL; > - goto put_larbvenc; > + return -EINVAL; > } > > for (i = 0; i < enc_clk->clk_num; i++) { > @@ -68,29 +46,23 @@ int mtk_vcodec_init_enc_pm(struct mtk_vcodec_dev *mtkdev) > "clock-names", i, &clk_info->clk_name); > if (ret) { > mtk_v4l2_err("venc failed to get clk name %d", i); > - goto put_larbvenc; > + return ret; > } > clk_info->vcodec_clk = devm_clk_get(&pdev->dev, > clk_info->clk_name); > if (IS_ERR(clk_info->vcodec_clk)) { > mtk_v4l2_err("venc devm_clk_get (%d)%s fail", i, > clk_info->clk_name); > - ret = PTR_ERR(clk_info->vcodec_clk); > - goto put_larbvenc; > + return PTR_ERR(clk_info->vcodec_clk); > } > } > > return 0; > - > -put_larbvenc: > - put_device(pm->larbvenc); > - return ret; > } > > void mtk_vcodec_release_enc_pm(struct mtk_vcodec_dev *mtkdev) > { > pm_runtime_disable(mtkdev->pm.dev); > - put_device(mtkdev->pm.larbvenc); > } > > > @@ -108,13 +80,6 @@ void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm) > } > } > > - ret = mtk_smi_larb_get(pm->larbvenc); > - if (ret) { > - mtk_v4l2_err("mtk_smi_larb_get larb3 fail %d", ret); > - goto clkerr; > - } > - return; You can't delete the return; here, otherwise vcodec_clk will be turned off immediately after they are turned on. > - > clkerr: > for (i -= 1; i >= 0; i--) > clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk); > @@ -125,7 +90,6 @@ void mtk_vcodec_enc_clock_off(struct mtk_vcodec_pm *pm) > struct mtk_vcodec_clk *enc_clk = &pm->venc_clk; > int i = 0; > > - mtk_smi_larb_put(pm->larbvenc); > for (i = enc_clk->clk_num - 1; i >= 0; i--) > clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk); > } > -- > 2.18.0 >
On Wed, 2021-05-12 at 17:20 +0800, Hsin-Yi Wang wrote: > On Sat, Apr 10, 2021 at 5:14 PM Yong Wu <yong.wu@mediatek.com> wrote: > > > > MediaTek IOMMU has already added the device_link between the consumer > > and smi-larb device. If the vcodec device call the pm_runtime_get_sync, > > the smi-larb's pm_runtime_get_sync also be called automatically. > > > > CC: Tiffany Lin <tiffany.lin@mediatek.com> > > CC: Irui Wang <irui.wang@mediatek.com> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > Reviewed-by: Evan Green <evgreen@chromium.org> > > Acked-by: Tiffany Lin <tiffany.lin@mediatek.com> > > --- > > .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 37 ++------------- > > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 -- > > .../platform/mtk-vcodec/mtk_vcodec_enc.c | 1 - > > .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 46 ++----------------- > > 4 files changed, 10 insertions(+), 77 deletions(-) [...] > > @@ -108,13 +80,6 @@ void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm) > > } > > } > > > > - ret = mtk_smi_larb_get(pm->larbvenc); > > - if (ret) { > > - mtk_v4l2_err("mtk_smi_larb_get larb3 fail %d", ret); > > - goto clkerr; > > - } > > - return; > > You can't delete the return; here, otherwise vcodec_clk will be turned > off immediately after they are turned on. Thanks very much for your review. Sorry for this. You are quite right. I checked this, it was introduced in v4 when I rebase the code. I will fix it in next time. > > > - > > clkerr: > > for (i -= 1; i >= 0; i--) > > clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk); > > @@ -125,7 +90,6 @@ void mtk_vcodec_enc_clock_off(struct mtk_vcodec_pm *pm) > > struct mtk_vcodec_clk *enc_clk = &pm->venc_clk; > > int i = 0; > > > > - mtk_smi_larb_put(pm->larbvenc); > > for (i = enc_clk->clk_num - 1; i >= 0; i--) > > clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk); > > } > > -- > > 2.18.0 > >
Hi On 10.04.21 12:11, Yong Wu wrote: > MediaTek IOMMU has already added the device_link between the consumer > and smi-larb device. If the drm device call the pm_runtime_get_sync, > the smi-larb's pm_runtime_get_sync also be called automatically. > > CC: CK Hu <ck.hu@mediatek.com> > CC: Philipp Zabel <p.zabel@pengutronix.de> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > Reviewed-by: Evan Green <evgreen@chromium.org> > Acked-by: Chun-Kuang Hu <chunkuang.hu@kernel.org> > --- > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 9 ------ > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 35 --------------------- > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 - > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 +-- > 4 files changed, 1 insertion(+), 49 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > index 971ef58ac1dc..d59353af4019 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > @@ -10,7 +10,6 @@ > #include <linux/soc/mediatek/mtk-mutex.h> > > #include <asm/barrier.h> > -#include <soc/mediatek/smi.h> > > #include <drm/drm_atomic.h> > #include <drm/drm_atomic_helper.h> > @@ -544,12 +543,6 @@ static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc, > > DRM_DEBUG_DRIVER("%s %d\n", __func__, crtc->base.id); > > - ret = mtk_smi_larb_get(comp->larb_dev); > - if (ret) { > - DRM_ERROR("Failed to get larb: %d\n", ret); > - return; > - } > - > ret = pm_runtime_resume_and_get(comp->dev); > if (ret < 0) > DRM_DEV_ERROR(comp->dev, "Failed to enable power domain: %d\n", > @@ -557,7 +550,6 @@ static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc, > > ret = mtk_crtc_ddp_hw_init(mtk_crtc); > if (ret) { > - mtk_smi_larb_put(comp->larb_dev); > pm_runtime_put(comp->dev); > return; > } > @@ -594,7 +586,6 @@ static void mtk_drm_crtc_atomic_disable(struct drm_crtc *crtc, > > drm_crtc_vblank_off(crtc); > mtk_crtc_ddp_hw_fini(mtk_crtc); > - mtk_smi_larb_put(comp->larb_dev); > ret = pm_runtime_put(comp->dev); > if (ret < 0) > DRM_DEV_ERROR(comp->dev, "Failed to disable power domain: %d\n", > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > index 75bc00e17fc4..6c01492ba4df 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > @@ -449,37 +449,12 @@ unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm, > return ret; > } > > -static int mtk_ddp_get_larb_dev(struct device_node *node, struct mtk_ddp_comp *comp, > - struct device *dev) > -{ > - struct device_node *larb_node; > - struct platform_device *larb_pdev; > - > - larb_node = of_parse_phandle(node, "mediatek,larb", 0); > - if (!larb_node) { > - dev_err(dev, "Missing mediadek,larb phandle in %pOF node\n", node); > - return -EINVAL; > - } > - > - larb_pdev = of_find_device_by_node(larb_node); > - if (!larb_pdev) { > - dev_warn(dev, "Waiting for larb device %pOF\n", larb_node); > - of_node_put(larb_node); > - return -EPROBE_DEFER; > - } > - of_node_put(larb_node); > - comp->larb_dev = &larb_pdev->dev; > - > - return 0; > -} > - > int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp, > enum mtk_ddp_comp_id comp_id) > { > struct platform_device *comp_pdev; > enum mtk_ddp_comp_type type; > struct mtk_ddp_comp_dev *priv; > - int ret; Hi, This 'ret' is also used inside `if IS_REACHABLE(CONFIG_MTK_CMDQ)` so it should not be removed. Thanks, Dafna > > if (comp_id < 0 || comp_id >= DDP_COMPONENT_ID_MAX) > return -EINVAL; > @@ -495,16 +470,6 @@ int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp, > } > comp->dev = &comp_pdev->dev; > > - /* Only DMA capable components need the LARB property */ > - if (type == MTK_DISP_OVL || > - type == MTK_DISP_OVL_2L || > - type == MTK_DISP_RDMA || > - type == MTK_DISP_WDMA) { > - ret = mtk_ddp_get_larb_dev(node, comp, comp->dev); > - if (ret) > - return ret; > - } > - > if (type == MTK_DISP_BLS || > type == MTK_DISP_CCORR || > type == MTK_DISP_COLOR || > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > index bb914d976cf5..1b582262b682 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > @@ -70,7 +70,6 @@ struct mtk_ddp_comp_funcs { > struct mtk_ddp_comp { > struct device *dev; > int irq; > - struct device *larb_dev; > enum mtk_ddp_comp_id id; > const struct mtk_ddp_comp_funcs *funcs; > }; > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > index b013d56d2777..622de47239eb 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > @@ -576,11 +576,8 @@ static int mtk_drm_probe(struct platform_device *pdev) > pm_runtime_disable(dev); > err_node: > of_node_put(private->mutex_node); > - for (i = 0; i < DDP_COMPONENT_ID_MAX; i++) { > + for (i = 0; i < DDP_COMPONENT_ID_MAX; i++) > of_node_put(private->comp_node[i]); > - if (private->ddp_comp[i].larb_dev) > - put_device(private->ddp_comp[i].larb_dev); > - } > return ret; > } > >
On Sat, 2021-04-10 at 17:11 +0800, Yong Wu wrote: > MediaTek IOMMU has already added the device_link between the consumer > and smi-larb device. If the mdp device call the pm_runtime_get_sync, > the smi-larb's pm_runtime_get_sync also be called automatically. > > CC: Minghsiu Tsai <minghsiu.tsai@mediatek.com> > CC: Houlong Wei <houlong.wei@mediatek.com> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > Reviewed-by: Evan Green <evgreen@chromium.org> > --- > drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 40 ------------------- > drivers/media/platform/mtk-mdp/mtk_mdp_comp.h | 2 - > drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 1 - > 3 files changed, 43 deletions(-) > > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c > index b3426a551bea..1e3833f1c9ae 100644 > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c > @@ -9,7 +9,6 @@ > #include <linux/of.h> > #include <linux/of_address.h> > #include <linux/of_platform.h> > -#include <soc/mediatek/smi.h> > > #include "mtk_mdp_comp.h" > > @@ -18,14 +17,6 @@ void mtk_mdp_comp_clock_on(struct device *dev, struct mtk_mdp_comp *comp) > { > int i, err; > > - if (comp->larb_dev) { > - err = mtk_smi_larb_get(comp->larb_dev); > - if (err) > - dev_err(dev, > - "failed to get larb, err %d. type:%d\n", > - err, comp->type); > - } > - > for (i = 0; i < ARRAY_SIZE(comp->clk); i++) { > if (IS_ERR(comp->clk[i])) > continue; > @@ -46,17 +37,12 @@ void mtk_mdp_comp_clock_off(struct device *dev, struct mtk_mdp_comp *comp) > continue; > clk_disable_unprepare(comp->clk[i]); > } > - > - if (comp->larb_dev) > - mtk_smi_larb_put(comp->larb_dev); > } > > int mtk_mdp_comp_init(struct device *dev, struct device_node *node, > struct mtk_mdp_comp *comp, > enum mtk_mdp_comp_type comp_type) > { > - struct device_node *larb_node; > - struct platform_device *larb_pdev; > int ret; > int i; > > @@ -77,32 +63,6 @@ int mtk_mdp_comp_init(struct device *dev, struct device_node *node, > break; > } > > - /* Only DMA capable components need the LARB property */ > - comp->larb_dev = NULL; > - if (comp->type != MTK_MDP_RDMA && > - comp->type != MTK_MDP_WDMA && > - comp->type != MTK_MDP_WROT) > - return 0; > - > - larb_node = of_parse_phandle(node, "mediatek,larb", 0); > - if (!larb_node) { > - dev_err(dev, > - "Missing mediadek,larb phandle in %pOF node\n", node); > - ret = -EINVAL; > - goto put_dev; > - } > - > - larb_pdev = of_find_device_by_node(larb_node); > - if (!larb_pdev) { > - dev_warn(dev, "Waiting for larb device %pOF\n", larb_node); > - of_node_put(larb_node); > - ret = -EPROBE_DEFER; > - goto put_dev; > - } > - of_node_put(larb_node); > - > - comp->larb_dev = &larb_pdev->dev; > - > return 0; > > put_dev: > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h > index 1bf0242cce46..36bc1b8f6222 100644 > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h > @@ -27,14 +27,12 @@ enum mtk_mdp_comp_type { > * @node: list node to track sibing MDP components > * @dev_node: component device node > * @clk: clocks required for component > - * @larb_dev: SMI device required for component > * @type: component type > */ > struct mtk_mdp_comp { > struct list_head node; > struct device_node *dev_node; > struct clk *clk[2]; > - struct device *larb_dev; > enum mtk_mdp_comp_type type; > }; > > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c > index 976aa1f4829b..70a8eab16863 100644 > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c > @@ -17,7 +17,6 @@ > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/workqueue.h> > -#include <soc/mediatek/smi.h> > > #include "mtk_mdp_core.h" > #include "mtk_mdp_m2m.h" Reviewed-by: Houlong Wei <houlong.wei@mediatek.com>
On 10/04/2021 11:11, Yong Wu wrote: > pm_runtime_get_sync will increment pm usage counter even it failed. > This patch use pm_runtime_resume_and_get instead of pm_runtime_get > to keep usage counter balanced. > > CC: Xia Jiang <xia.jiang@mediatek.com> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > --- > drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c > index 88a23bce569d..a89c7b206eef 100644 > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c > @@ -920,7 +920,7 @@ static void mtk_jpeg_enc_device_run(void *priv) > src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx); > dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx); > > - ret = pm_runtime_get_sync(jpeg->dev); > + ret = pm_runtime_resume_and_get(jpeg->dev); > if (ret < 0) > goto enc_end; > > @@ -973,7 +973,7 @@ static void mtk_jpeg_dec_device_run(void *priv) > return; > } > > - ret = pm_runtime_get_sync(jpeg->dev); > + ret = pm_runtime_resume_and_get(jpeg->dev); > if (ret < 0) > goto dec_end; > >
On 10/04/2021 11:11, Yong Wu wrote: > pm_runtime_get_sync will increment pm usage counter even it failed. > This patch use pm_runtime_resume_and_get instead of pm_runtime_get > to keep usage counter balanced. > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > --- > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > index 8b0de90156c6..69d23ce56d2c 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > @@ -259,7 +259,7 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc) > drm_connector_list_iter_end(&conn_iter); > } > > - ret = pm_runtime_get_sync(crtc->dev->dev); > + ret = pm_runtime_resume_and_get(crtc->dev->dev); > if (ret < 0) { > DRM_ERROR("Failed to enable power domain: %d\n", ret); > return ret; >
On 10/04/2021 11:11, Yong Wu wrote: > After adding device_link between the iommu consumer and smi-larb, > the pm_runtime_get(_sync) of smi-larb and smi-common will be called > automatically. we can get rid of mtk_smi_larb_get/put. > > CC: Matthias Brugger <matthias.bgg@gmail.com> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > Reviewed-by: Evan Green <evgreen@chromium.org> Acked-by: Matthias Brugger <matthias.bgg@gmail.com> > --- > drivers/memory/mtk-smi.c | 14 -------------- > include/soc/mediatek/smi.h | 20 -------------------- > 2 files changed, 34 deletions(-) > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c > index c5fb51f73b34..7c61c924e220 100644 > --- a/drivers/memory/mtk-smi.c > +++ b/drivers/memory/mtk-smi.c > @@ -134,20 +134,6 @@ static void mtk_smi_clk_disable(const struct mtk_smi *smi) > clk_disable_unprepare(smi->clk_apb); > } > > -int mtk_smi_larb_get(struct device *larbdev) > -{ > - int ret = pm_runtime_resume_and_get(larbdev); > - > - return (ret < 0) ? ret : 0; > -} > -EXPORT_SYMBOL_GPL(mtk_smi_larb_get); > - > -void mtk_smi_larb_put(struct device *larbdev) > -{ > - pm_runtime_put_sync(larbdev); > -} > -EXPORT_SYMBOL_GPL(mtk_smi_larb_put); > - > static int > mtk_smi_larb_bind(struct device *dev, struct device *master, void *data) > { > diff --git a/include/soc/mediatek/smi.h b/include/soc/mediatek/smi.h > index 15e3397cec58..11f7d6b59642 100644 > --- a/include/soc/mediatek/smi.h > +++ b/include/soc/mediatek/smi.h > @@ -19,26 +19,6 @@ struct mtk_smi_larb_iommu { > unsigned char bank[32]; > }; > > -/* > - * mtk_smi_larb_get: Enable the power domain and clocks for this local arbiter. > - * It also initialize some basic setting(like iommu). > - * mtk_smi_larb_put: Disable the power domain and clocks for this local arbiter. > - * Both should be called in non-atomic context. > - * > - * Returns 0 if successful, negative on failure. > - */ > -int mtk_smi_larb_get(struct device *larbdev); > -void mtk_smi_larb_put(struct device *larbdev); > - > -#else > - > -static inline int mtk_smi_larb_get(struct device *larbdev) > -{ > - return 0; > -} > - > -static inline void mtk_smi_larb_put(struct device *larbdev) { } > - > #endif > > #endif >
Hi Yong, On 12/05/2021 14:29, Yong Wu wrote: > On Wed, 2021-05-12 at 17:20 +0800, Hsin-Yi Wang wrote: >> On Sat, Apr 10, 2021 at 5:14 PM Yong Wu <yong.wu@mediatek.com> wrote: >>> >>> MediaTek IOMMU has already added the device_link between the consumer >>> and smi-larb device. If the vcodec device call the pm_runtime_get_sync, >>> the smi-larb's pm_runtime_get_sync also be called automatically. >>> >>> CC: Tiffany Lin <tiffany.lin@mediatek.com> >>> CC: Irui Wang <irui.wang@mediatek.com> >>> Signed-off-by: Yong Wu <yong.wu@mediatek.com> >>> Reviewed-by: Evan Green <evgreen@chromium.org> >>> Acked-by: Tiffany Lin <tiffany.lin@mediatek.com> >>> --- >>> .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 37 ++------------- >>> .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 -- >>> .../platform/mtk-vcodec/mtk_vcodec_enc.c | 1 - >>> .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 46 ++----------------- >>> 4 files changed, 10 insertions(+), 77 deletions(-) > > [...] > >>> @@ -108,13 +80,6 @@ void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm) >>> } >>> } >>> >>> - ret = mtk_smi_larb_get(pm->larbvenc); >>> - if (ret) { >>> - mtk_v4l2_err("mtk_smi_larb_get larb3 fail %d", ret); >>> - goto clkerr; >>> - } >>> - return; >> >> You can't delete the return; here, otherwise vcodec_clk will be turned >> off immediately after they are turned on. > > Thanks very much for your review. > > Sorry for this. You are quite right. > > I checked this, it was introduced in v4 when I rebase the code. I will > fix it in next time. > Please also make sure that you add all maintainers. I realized that at least for the media/platform drivers we miss the maintainer and the corresponding mailing list. This is especially important in this series, as it spans several subsystems. Thanks a lot, Matthias >> >>> - >>> clkerr: >>> for (i -= 1; i >= 0; i--) >>> clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk); >>> @@ -125,7 +90,6 @@ void mtk_vcodec_enc_clock_off(struct mtk_vcodec_pm *pm) >>> struct mtk_vcodec_clk *enc_clk = &pm->venc_clk; >>> int i = 0; >>> >>> - mtk_smi_larb_put(pm->larbvenc); >>> for (i = enc_clk->clk_num - 1; i >= 0; i--) >>> clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk); >>> } >>> -- >>> 2.18.0 >>> >
On Wed, 2021-05-26 at 08:41 +0300, Dafna Hirschfeld wrote: > Hi > > On 10.04.21 12:11, Yong Wu wrote: > > MediaTek IOMMU has already added the device_link between the consumer > > and smi-larb device. If the drm device call the pm_runtime_get_sync, > > the smi-larb's pm_runtime_get_sync also be called automatically. > > > > CC: CK Hu <ck.hu@mediatek.com> > > CC: Philipp Zabel <p.zabel@pengutronix.de> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > Reviewed-by: Evan Green <evgreen@chromium.org> > > Acked-by: Chun-Kuang Hu <chunkuang.hu@kernel.org> > > --- > > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 9 ------ > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 35 --------------------- > > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 - > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 +-- > > 4 files changed, 1 insertion(+), 49 deletions(-) > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > index 971ef58ac1dc..d59353af4019 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c > > @@ -10,7 +10,6 @@ > > #include <linux/soc/mediatek/mtk-mutex.h> > > > > #include <asm/barrier.h> > > -#include <soc/mediatek/smi.h> > > > > #include <drm/drm_atomic.h> > > #include <drm/drm_atomic_helper.h> > > @@ -544,12 +543,6 @@ static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc, > > > > DRM_DEBUG_DRIVER("%s %d\n", __func__, crtc->base.id); > > > > - ret = mtk_smi_larb_get(comp->larb_dev); > > - if (ret) { > > - DRM_ERROR("Failed to get larb: %d\n", ret); > > - return; > > - } > > - > > ret = pm_runtime_resume_and_get(comp->dev); > > if (ret < 0) > > DRM_DEV_ERROR(comp->dev, "Failed to enable power domain: %d\n", > > @@ -557,7 +550,6 @@ static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc, > > > > ret = mtk_crtc_ddp_hw_init(mtk_crtc); > > if (ret) { > > - mtk_smi_larb_put(comp->larb_dev); > > pm_runtime_put(comp->dev); > > return; > > } > > @@ -594,7 +586,6 @@ static void mtk_drm_crtc_atomic_disable(struct drm_crtc *crtc, > > > > drm_crtc_vblank_off(crtc); > > mtk_crtc_ddp_hw_fini(mtk_crtc); > > - mtk_smi_larb_put(comp->larb_dev); > > ret = pm_runtime_put(comp->dev); > > if (ret < 0) > > DRM_DEV_ERROR(comp->dev, "Failed to disable power domain: %d\n", > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > index 75bc00e17fc4..6c01492ba4df 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c > > @@ -449,37 +449,12 @@ unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm, > > return ret; > > } > > > > -static int mtk_ddp_get_larb_dev(struct device_node *node, struct mtk_ddp_comp *comp, > > - struct device *dev) > > -{ > > - struct device_node *larb_node; > > - struct platform_device *larb_pdev; > > - > > - larb_node = of_parse_phandle(node, "mediatek,larb", 0); > > - if (!larb_node) { > > - dev_err(dev, "Missing mediadek,larb phandle in %pOF node\n", node); > > - return -EINVAL; > > - } > > - > > - larb_pdev = of_find_device_by_node(larb_node); > > - if (!larb_pdev) { > > - dev_warn(dev, "Waiting for larb device %pOF\n", larb_node); > > - of_node_put(larb_node); > > - return -EPROBE_DEFER; > > - } > > - of_node_put(larb_node); > > - comp->larb_dev = &larb_pdev->dev; > > - > > - return 0; > > -} > > - > > int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp, > > enum mtk_ddp_comp_id comp_id) > > { > > struct platform_device *comp_pdev; > > enum mtk_ddp_comp_type type; > > struct mtk_ddp_comp_dev *priv; > > - int ret; > Hi, > > This 'ret' is also used inside `if IS_REACHABLE(CONFIG_MTK_CMDQ)` > so it should not be removed. Hi Dafna, Thanks very much for pointing out. Apparently I didn't enable this to build. Sorry for this. I will fix it in next version. > > Thanks, > Dafna > > > > > if (comp_id < 0 || comp_id >= DDP_COMPONENT_ID_MAX) > > return -EINVAL; > > @@ -495,16 +470,6 @@ int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp, > > } > > comp->dev = &comp_pdev->dev; > > > > - /* Only DMA capable components need the LARB property */ > > - if (type == MTK_DISP_OVL || > > - type == MTK_DISP_OVL_2L || > > - type == MTK_DISP_RDMA || > > - type == MTK_DISP_WDMA) { > > - ret = mtk_ddp_get_larb_dev(node, comp, comp->dev); > > - if (ret) > > - return ret; > > - } > > - > > if (type == MTK_DISP_BLS || > > type == MTK_DISP_CCORR || > > type == MTK_DISP_COLOR || > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > > index bb914d976cf5..1b582262b682 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h > > @@ -70,7 +70,6 @@ struct mtk_ddp_comp_funcs { > > struct mtk_ddp_comp { > > struct device *dev; > > int irq; > > - struct device *larb_dev; > > enum mtk_ddp_comp_id id; > > const struct mtk_ddp_comp_funcs *funcs; > > }; > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > index b013d56d2777..622de47239eb 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > @@ -576,11 +576,8 @@ static int mtk_drm_probe(struct platform_device *pdev) > > pm_runtime_disable(dev); > > err_node: > > of_node_put(private->mutex_node); > > - for (i = 0; i < DDP_COMPONENT_ID_MAX; i++) { > > + for (i = 0; i < DDP_COMPONENT_ID_MAX; i++) > > of_node_put(private->comp_node[i]); > > - if (private->ddp_comp[i].larb_dev) > > - put_device(private->ddp_comp[i].larb_dev); > > - } > > return ret; > > } > > > >
On Thu, 2021-06-10 at 09:53 +0200, Matthias Brugger wrote: > Hi Yong, > > On 12/05/2021 14:29, Yong Wu wrote: > > On Wed, 2021-05-12 at 17:20 +0800, Hsin-Yi Wang wrote: > >> On Sat, Apr 10, 2021 at 5:14 PM Yong Wu <yong.wu@mediatek.com> wrote: > >>> > >>> MediaTek IOMMU has already added the device_link between the consumer > >>> and smi-larb device. If the vcodec device call the pm_runtime_get_sync, > >>> the smi-larb's pm_runtime_get_sync also be called automatically. > >>> > >>> CC: Tiffany Lin <tiffany.lin@mediatek.com> > >>> CC: Irui Wang <irui.wang@mediatek.com> > >>> Signed-off-by: Yong Wu <yong.wu@mediatek.com> > >>> Reviewed-by: Evan Green <evgreen@chromium.org> > >>> Acked-by: Tiffany Lin <tiffany.lin@mediatek.com> > >>> --- > >>> .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 37 ++------------- > >>> .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 -- > >>> .../platform/mtk-vcodec/mtk_vcodec_enc.c | 1 - > >>> .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 46 ++----------------- > >>> 4 files changed, 10 insertions(+), 77 deletions(-) > > > > [...] > > > >>> @@ -108,13 +80,6 @@ void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm) > >>> } > >>> } > >>> > >>> - ret = mtk_smi_larb_get(pm->larbvenc); > >>> - if (ret) { > >>> - mtk_v4l2_err("mtk_smi_larb_get larb3 fail %d", ret); > >>> - goto clkerr; > >>> - } > >>> - return; > >> > >> You can't delete the return; here, otherwise vcodec_clk will be turned > >> off immediately after they are turned on. > > > > Thanks very much for your review. > > > > Sorry for this. You are quite right. > > > > I checked this, it was introduced in v4 when I rebase the code. I will > > fix it in next time. > > > > Please also make sure that you add all maintainers. I realized that at least for > the media/platform drivers we miss the maintainer and the corresponding mailing > list. > This is especially important in this series, as it spans several subsystems. Thanks for hint. I only added the file maintainer here. I will add linux-media in next version. By the way, this patchset cross several trees, then which tree should it go through. Do you have some suggestion? > > Thanks a lot, > Matthias > > >> > >>> - > >>> clkerr: > >>> for (i -= 1; i >= 0; i--) > >>> clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk); > >>> @@ -125,7 +90,6 @@ void mtk_vcodec_enc_clock_off(struct mtk_vcodec_pm *pm) > >>> struct mtk_vcodec_clk *enc_clk = &pm->venc_clk; > >>> int i = 0; > >>> > >>> - mtk_smi_larb_put(pm->larbvenc); > >>> for (i = enc_clk->clk_num - 1; i >= 0; i--) > >>> clk_disable_unprepare(enc_clk->clk_info[i].vcodec_clk); > >>> } > >>> -- > >>> 2.18.0 > >>> > >
On 10/06/2021 14:02, Yong Wu wrote: > On Thu, 2021-06-10 at 09:53 +0200, Matthias Brugger wrote: >> Hi Yong, >> >> On 12/05/2021 14:29, Yong Wu wrote: >>> On Wed, 2021-05-12 at 17:20 +0800, Hsin-Yi Wang wrote: >>>> On Sat, Apr 10, 2021 at 5:14 PM Yong Wu <yong.wu@mediatek.com> wrote: >>>>> >>>>> MediaTek IOMMU has already added the device_link between the consumer >>>>> and smi-larb device. If the vcodec device call the pm_runtime_get_sync, >>>>> the smi-larb's pm_runtime_get_sync also be called automatically. >>>>> >>>>> CC: Tiffany Lin <tiffany.lin@mediatek.com> >>>>> CC: Irui Wang <irui.wang@mediatek.com> >>>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com> >>>>> Reviewed-by: Evan Green <evgreen@chromium.org> >>>>> Acked-by: Tiffany Lin <tiffany.lin@mediatek.com> >>>>> --- >>>>> .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 37 ++------------- >>>>> .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 -- >>>>> .../platform/mtk-vcodec/mtk_vcodec_enc.c | 1 - >>>>> .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 46 ++----------------- >>>>> 4 files changed, 10 insertions(+), 77 deletions(-) >>> >>> [...] >>> >>>>> @@ -108,13 +80,6 @@ void mtk_vcodec_enc_clock_on(struct mtk_vcodec_pm *pm) >>>>> } >>>>> } >>>>> >>>>> - ret = mtk_smi_larb_get(pm->larbvenc); >>>>> - if (ret) { >>>>> - mtk_v4l2_err("mtk_smi_larb_get larb3 fail %d", ret); >>>>> - goto clkerr; >>>>> - } >>>>> - return; >>>> >>>> You can't delete the return; here, otherwise vcodec_clk will be turned >>>> off immediately after they are turned on. >>> >>> Thanks very much for your review. >>> >>> Sorry for this. You are quite right. >>> >>> I checked this, it was introduced in v4 when I rebase the code. I will >>> fix it in next time. >>> >> >> Please also make sure that you add all maintainers. I realized that at least for >> the media/platform drivers we miss the maintainer and the corresponding mailing >> list. >> This is especially important in this series, as it spans several subsystems. > > Thanks for hint. I only added the file maintainer here. I will add > linux-media in next version. > > By the way, this patchset cross several trees, then which tree should it > go through. Do you have some suggestion? > That's a good question. I think the media tree would be a good candidate, as it has the biggest bunch of patches. But that would mean that Joerg is fine that. The DTS part could still go through my tree. Regards, Matthias
On Fri, Jun 11, 2021 at 12:07:24PM +0200, Matthias Brugger wrote: > That's a good question. I think the media tree would be a good > candidate, as it has the biggest bunch of patches. But that would mean > that Joerg is fine that. The DTS part could still go through my tree. IOMMU changes are only a minor part of this, so it should not go through the IOMMU tree. When Matthias has reviewed the IOMMU changes, feel free to add my Acked-by: Joerg Roedel <jroedel@suse.de> to them. Regards, Joerg
MediaTek IOMMU block diagram always like below: M4U | smi-common | ------------- | | ... | | larb1 larb2 | | vdec venc All the consumer connect with smi-larb, then connect with smi-common. When the consumer works, it should enable the smi-larb's power which also need enable the smi-common's power firstly. Thus, Firstly, use the device link connect the consumer and the smi-larbs. then add device link between the smi-larb and smi-common. After adding the device_link, then "mediatek,larb" property can be removed. the iommu consumer don't need call the mtk_smi_larb_get/put to enable the power and clock of smi-larb and smi-common. About the MM dt-bindings/dtsi patches, I guess they will go through Matthias's tree, so I don't split them for each a MM module and each a SoC. This patchset base on v5.12-rc2 and a vcodec patchset(separating its node)[1] which has already been in linux-next. [1] https://lore.kernel.org/linux-mediatek/20210325122625.15100-1-irui.wang@mediatek.com/ Change notes: v5: 1) Base on v5.12-rc2. 2) Remove changing the mtk-iommu to module_platform_driver patch, It have already been a independent patch. 3) Add several patches for the MM drivers with pm_runtime_resume_and_get instead of pm_runtime_get_sync since smi already use it. v4: https://lore.kernel.org/linux-mediatek/1590826218-23653-1-git-send-email-yong.wu@mediatek.com/ base on v5.7-rc1. 1) Move drm PM patch before smi patchs. 2) Change builtin_platform_driver to module_platform_driver since we may need build as module. 3) Rebase many patchset as above. v3: https://lore.kernel.org/linux-iommu/1567503456-24725-1-git-send-email-yong.wu@mediatek.com/ 1) rebase on v5.3-rc1 and the latest mt8183 patchset. 2) Use device_is_bound to check whether the driver is ready from Matthias. 3) Add DL_FLAG_STATELESS flag when calling device_link_add and explain the reason in the commit message[3/14]. 4) Add a display patch[12/14] into this series. otherwise it may affect display HW fastlogo even though it don't happen in mt8183. v2: https://lore.kernel.org/linux-iommu/1560171313-28299-1-git-send-email-yong.wu@mediatek.com/ 1) rebase on v5.2-rc1. 2) Move adding device_link between the consumer and smi-larb into iommu_add_device from Robin. 3) add DL_FLAG_AUTOREMOVE_CONSUMER even though the smi is built-in from Evan. 4) Remove the shutdown callback in iommu. v1: https://lore.kernel.org/linux-iommu/1546318276-18993-1-git-send-email-yong.wu@mediatek.com/ Yong Wu (15): dt-binding: mediatek: Get rid of mediatek,larb for multimedia HW iommu/mediatek: Add probe_defer for smi-larb iommu/mediatek: Add device_link between the consumer and the larb devices memory: mtk-smi: Add device-link between smi-larb and smi-common media: mtk-jpeg: Use pm_runtime_resume_and_get for PM get_sync media: mtk-jpeg: Get rid of mtk_smi_larb_get/put media: mtk-mdp: Use pm_runtime_resume_and_get for PM get_sync media: mtk-mdp: Get rid of mtk_smi_larb_get/put drm/mediatek: Use pm_runtime_resume_and_get for PM get_sync drm/mediatek: Get rid of mtk_smi_larb_get/put media: mtk-vcodec: Use pm_runtime_resume_and_get for PM get_sync media: mtk-vcodec: Get rid of mtk_smi_larb_get/put memory: mtk-smi: Get rid of mtk_smi_larb_get/put arm: dts: mediatek: Get rid of mediatek,larb for MM nodes arm64: dts: mediatek: Get rid of mediatek,larb for MM nodes Yongqiang Niu (1): drm/mediatek: Add pm runtime support for ovl and rdma .../display/mediatek/mediatek,disp.txt | 9 ---- .../bindings/media/mediatek-jpeg-decoder.txt | 4 -- .../bindings/media/mediatek-jpeg-encoder.txt | 4 -- .../bindings/media/mediatek-mdp.txt | 8 ---- .../bindings/media/mediatek-vcodec.txt | 4 -- arch/arm/boot/dts/mt2701.dtsi | 2 - arch/arm/boot/dts/mt7623n.dtsi | 5 -- arch/arm64/boot/dts/mediatek/mt8173.dtsi | 16 ------- arch/arm64/boot/dts/mediatek/mt8183.dtsi | 5 -- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 9 +++- drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 9 +++- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 21 +++++---- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 35 -------------- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 - drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 +- drivers/iommu/mtk_iommu.c | 24 +++++++++- drivers/iommu/mtk_iommu_v1.c | 22 ++++++++- .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 34 ++------------ .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 2 - drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 40 ---------------- drivers/media/platform/mtk-mdp/mtk_mdp_comp.h | 2 - drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 1 - drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 4 +- .../platform/mtk-vcodec/mtk_vcodec_dec_pm.c | 42 ++++------------- .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 -- .../platform/mtk-vcodec/mtk_vcodec_enc.c | 1 - .../platform/mtk-vcodec/mtk_vcodec_enc_pm.c | 46 ++----------------- drivers/memory/mtk-smi.c | 33 ++++--------- include/soc/mediatek/smi.h | 20 -------- 29 files changed, 99 insertions(+), 312 deletions(-)