diff mbox series

[v5,13/16] media: mtk-vcodec: Get rid of mtk_smi_larb_get/put

Message ID 20210410091128.31823-14-yong.wu@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Clean up "mediatek,larb" | expand

Commit Message

Yong Wu (吴勇) April 10, 2021, 9:11 a.m. UTC
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(-)

Comments

Hsin-Yi Wang May 12, 2021, 9:20 a.m. UTC | #1
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
>
Yong Wu (吴勇) May 12, 2021, 12:29 p.m. UTC | #2
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
> >
Matthias Brugger June 10, 2021, 7:53 a.m. UTC | #3
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
>>>
>
Yong Wu (吴勇) June 10, 2021, 12:02 p.m. UTC | #4
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
> >>>
> >
Matthias Brugger June 11, 2021, 10:07 a.m. UTC | #5
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
Joerg Roedel June 11, 2021, 10:46 a.m. UTC | #6
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
diff mbox series

Patch

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