Hi, thanks for the patch

On 14.07.21 04:56, Yong Wu wrote:

After adding device_link between the consumer with the smi-larbs, if the consumer call its owner pm_runtime_get(_sync), the pm_runtime_get(_sync) of smi-larb and smi-common will be called automatically. Thus, the consumer don't need the property.

And IOMMU also know which larb this consumer connects with from iommu id in the "iommus=" property.

Signed-off-by: Yong Wu yong.wu@mediatek.com Reviewed-by: Rob Herring robh@kernel.org Reviewed-by: Evan Green evgreen@chromium.org

---

.../bindings/display/mediatek/mediatek,disp.txt | 9 --------- .../devicetree/bindings/media/mediatek-jpeg-decoder.yaml | 9 --------- .../devicetree/bindings/media/mediatek-jpeg-encoder.yaml | 9 ---------

On which repo are these patches based on ? In linux-next the file mediatek-jpeg-encoder.yaml don't exist

Thanks, Dafna

Documentation/devicetree/bindings/media/mediatek-mdp.txt | 8 -------- .../devicetree/bindings/media/mediatek-vcodec.txt | 4 ---- 5 files changed, 39 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt index fbb59c9ddda6..867bd82e2f03 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt @@ -61,8 +61,6 @@ Required properties (DMA function blocks): "mediatek,<chip>-disp-rdma" "mediatek,<chip>-disp-wdma" the supported chips are mt2701, mt8167 and mt8173. -- larb: Should contain a phandle pointing to the local arbiter device as defined

• in Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
  • iommus: Should point to the respective IOMMU block with master port as
  argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for details.

@@ -91,7 +89,6 @@ ovl0: ovl@1400c000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_OVL0>; iommus = <&iommu M4U_PORT_DISP_OVL0>;

• mediatek,larb = <&larb0>; };

  ovl1: ovl@1400d000 {

@@ -101,7 +98,6 @@ ovl1: ovl@1400d000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_OVL1>; iommus = <&iommu M4U_PORT_DISP_OVL1>;

• mediatek,larb = <&larb4>; };

  rdma0: rdma@1400e000 {

@@ -111,7 +107,6 @@ rdma0: rdma@1400e000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_RDMA0>; iommus = <&iommu M4U_PORT_DISP_RDMA0>;

• mediatek,larb = <&larb0>; mediatek,rdma-fifosize = <8192>; };

@@ -122,7 +117,6 @@ rdma1: rdma@1400f000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_RDMA1>; iommus = <&iommu M4U_PORT_DISP_RDMA1>;

• mediatek,larb = <&larb4>; };

  rdma2: rdma@14010000 {

@@ -132,7 +126,6 @@ rdma2: rdma@14010000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_RDMA2>; iommus = <&iommu M4U_PORT_DISP_RDMA2>;

• mediatek,larb = <&larb4>; };

  wdma0: wdma@14011000 {

@@ -142,7 +135,6 @@ wdma0: wdma@14011000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_WDMA0>; iommus = <&iommu M4U_PORT_DISP_WDMA0>;

• mediatek,larb = <&larb0>; };

  wdma1: wdma@14012000 {

@@ -152,7 +144,6 @@ wdma1: wdma@14012000 { power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_WDMA1>; iommus = <&iommu M4U_PORT_DISP_WDMA1>;

• mediatek,larb = <&larb4>; };

  color0: color@14013000 {

diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml index 9b87f036f178..052e752157b4 100644 --- a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml +++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml @@ -42,13 +42,6 @@ properties: power-domains: maxItems: 1

• mediatek,larb:
• $ref: '/schemas/types.yaml#/definitions/phandle'
• description: |

•  Must contain the local arbiters in the current Socs, see
  

•  Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
  

•  for details.
  

• iommus: maxItems: 2 description: |

@@ -63,7 +56,6 @@ required: - clocks - clock-names - power-domains

• • mediatek,larb
  • iommus

  additionalProperties: false

@@ -83,7 +75,6 @@ examples: clock-names = "jpgdec-smi", "jpgdec"; power-domains = <&scpsys MT2701_POWER_DOMAIN_ISP>;

•  mediatek,larb = <&larb2>;
    iommus = <&iommu MT2701_M4U_PORT_JPGDEC_WDMA>,
             <&iommu MT2701_M4U_PORT_JPGDEC_BSDMA>;
  };
  

diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml index fcd9b829e036..8bfdfdfaba59 100644 --- a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml +++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml @@ -35,13 +35,6 @@ properties: power-domains: maxItems: 1

• mediatek,larb:
• $ref: '/schemas/types.yaml#/definitions/phandle'
• description: |

•  Must contain the local arbiters in the current Socs, see
  

•  Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml
  

•  for details.
  

• iommus: maxItems: 2 description: |

@@ -56,7 +49,6 @@ required: - clocks - clock-names - power-domains

• • mediatek,larb
  • iommus

  additionalProperties: false

@@ -75,7 +67,6 @@ examples: clocks = <&imgsys CLK_IMG_VENC>; clock-names = "jpgenc"; power-domains = <&scpsys MT2701_POWER_DOMAIN_ISP>;

•  mediatek,larb = <&larb2>;
    iommus = <&iommu MT2701_M4U_PORT_JPGENC_RDMA>,
             <&iommu MT2701_M4U_PORT_JPGENC_BSDMA>;
  };
  

diff --git a/Documentation/devicetree/bindings/media/mediatek-mdp.txt b/Documentation/devicetree/bindings/media/mediatek-mdp.txt index caa24943da33..53ef26e2c857 100644 --- a/Documentation/devicetree/bindings/media/mediatek-mdp.txt +++ b/Documentation/devicetree/bindings/media/mediatek-mdp.txt @@ -27,9 +27,6 @@ Required properties (DMA function blocks, child node):

• iommus: should point to the respective IOMMU block with master port as argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for details.

-- mediatek,larb: must contain the local arbiters in the current Socs, see

• Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml

• for details.

  Example: mdp_rdma0: rdma@14001000 {

@@ -40,7 +37,6 @@ Example: <&mmsys CLK_MM_MUTEX_32K>; power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; iommus = <&iommu M4U_PORT_MDP_RDMA0>;

• mediatek,larb = <&larb0>;
  

  mediatek,vpu = <&vpu>; };

@@ -51,7 +47,6 @@ Example: <&mmsys CLK_MM_MUTEX_32K>; power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; iommus = <&iommu M4U_PORT_MDP_RDMA1>;

• mediatek,larb = <&larb4>;
  

  };

  mdp_rsz0: rsz@14003000 {

@@ -81,7 +76,6 @@ Example: clocks = <&mmsys CLK_MM_MDP_WDMA>; power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; iommus = <&iommu M4U_PORT_MDP_WDMA>;

• mediatek,larb = <&larb0>;
  

  };

  mdp_wrot0: wrot@14007000 {

@@ -90,7 +84,6 @@ Example: clocks = <&mmsys CLK_MM_MDP_WROT0>; power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; iommus = <&iommu M4U_PORT_MDP_WROT0>;

• mediatek,larb = <&larb0>;
  

  };

  mdp_wrot1: wrot@14008000 {

@@ -99,5 +92,4 @@ Example: clocks = <&mmsys CLK_MM_MDP_WROT1>; power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; iommus = <&iommu M4U_PORT_MDP_WROT1>;

• mediatek,larb = <&larb4>;
  

  };

diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt index ad1321e5a22d..71237355cc7e 100644 --- a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt +++ b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt @@ -13,7 +13,6 @@ Required properties:

• reg : Physical base address of the video codec registers and length of memory mapped region.
• interrupts : interrupt number to the cpu.

-- mediatek,larb : must contain the local arbiters in the current Socs.

• clocks : list of clock specifiers, corresponding to entries in the clock-names property.
• clock-names: avc encoder must contain "venc_sel", vp8 encoder must

@@ -46,7 +45,6 @@ vcodec_dec: vcodec@16000000 { <0 0x16027800 0 0x800>, /*VP8_VL*/ <0 0x16028400 0 0x400>; /*VP9_VD*/ interrupts = <GIC_SPI 204 IRQ_TYPE_LEVEL_LOW>;

• mediatek,larb = <&larb1>; iommus = <&iommu M4U_PORT_HW_VDEC_MC_EXT>, <&iommu M4U_PORT_HW_VDEC_PP_EXT>, <&iommu M4U_PORT_HW_VDEC_AVC_MV_EXT>,

@@ -99,7 +97,6 @@ vcodec_enc_avc: vcodec@18002000 { <&iommu M4U_PORT_VENC_REF_CHROMA>, <&iommu M4U_PORT_VENC_NBM_RDMA>, <&iommu M4U_PORT_VENC_NBM_WDMA>;

• mediatek,larb = <&larb3>; mediatek,vpu = <&vpu>; clocks = <&topckgen CLK_TOP_VENC_SEL>; clock-names = "venc_sel";

@@ -120,7 +117,6 @@ vcodec_enc_vp8: vcodec@19002000 { <&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>, <&iommu M4U_PORT_VENC_REF_LUMA_SET2>, <&iommu M4U_PORT_VENC_REC_CHROMA_SET2>;

• mediatek,larb = <&larb5>; mediatek,vpu = <&vpu>; clocks = <&topckgen CLK_TOP_VENC_LT_SEL>; clock-names = "venc_lt_sel";

On 14.07.21 04:56, Yong Wu wrote:

MediaTek IOMMU-SMI diagram is like below. all the consumer connect with smi-larb, then connect with smi-common.

     M4U
      |
 smi-common
      |

---

| | ... | | larb1 larb2 | | vdec venc

When the consumer works, it should enable the smi-larb's power which also need enable the smi-common's power firstly.

Thus, First of all, use the device link connect the consumer and the smi-larbs. then add device link between the smi-larb and smi-common.

This patch adds device_link between the consumer and the larbs.

When device_link_add, I add the flag DL_FLAG_STATELESS to avoid calling pm_runtime_xx to keep the original status of clocks. It can avoid two issues:

1. Display HW show fastlogo abnormally reported in [1]. At the beggining,

all the clocks are enabled before entering kernel, but the clocks for display HW(always in larb0) will be gated after clk_enable and clk_disable called from device_link_add(->pm_runtime_resume) and rpm_idle. The clock operation happened before display driver probe. At that time, the display HW will be abnormal.

2. A deadlock issue reported in [2]. Use DL_FLAG_STATELESS to skip

pm_runtime_xx to avoid the deadlock.

Corresponding, DL_FLAG_AUTOREMOVE_CONSUMER can't be added, then device_link_removed should be added explicitly.

[1] https://lore.kernel.org/linux-mediatek/1564213888.22908.4.camel@mhfsdcap03/ [2] https://lore.kernel.org/patchwork/patch/1086569/

Suggested-by: Tomasz Figa tfiga@chromium.org Signed-off-by: Yong Wu yong.wu@mediatek.com

---

drivers/iommu/mtk_iommu.c | 22 ++++++++++++++++++++++ drivers/iommu/mtk_iommu_v1.c | 20 +++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index a02dde094788..ee742900cf4b 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.c @@ -571,22 +571,44 @@ static struct iommu_device *mtk_iommu_probe_device(struct device *dev) { struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); struct mtk_iommu_data *data;

• struct device_link *link;

• struct device *larbdev;

• unsigned int larbid;

  if (!fwspec || fwspec->ops != &mtk_iommu_ops) return ERR_PTR(-ENODEV); /* Not a iommu client device */

  data = dev_iommu_priv_get(dev);

• /*

• * Link the consumer device with the smi-larb device(supplier)
  

• * The device in each a larb is a independent HW. thus only link
  

• * one larb here.
  

• */
  

• larbid = MTK_M4U_TO_LARB(fwspec->ids[0]);

• larbdev = data->larb_imu[larbid].dev;

• link = device_link_add(dev, larbdev,

• 	       DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
  

• if (!link)

• dev_err(dev, "Unable to link %s\n", dev_name(larbdev));
  

shoudn't ERR_PTR be returned in case of failure?

Thanks, Dafna

return &data->iommu; }

static void mtk_iommu_release_device(struct device *dev) { struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);

• struct mtk_iommu_data *data;

• struct device *larbdev;

• unsigned int larbid;

  if (!fwspec || fwspec->ops != &mtk_iommu_ops) return;

• data = dev_iommu_priv_get(dev);

• larbid = MTK_M4U_TO_LARB(fwspec->ids[0]);

• larbdev = data->larb_imu[larbid].dev;

• device_link_remove(dev, larbdev);

• iommu_fwspec_free(dev); }

diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c index d9365a3d8dc9..d2a7c66b8239 100644 --- a/drivers/iommu/mtk_iommu_v1.c +++ b/drivers/iommu/mtk_iommu_v1.c @@ -424,7 +424,9 @@ static struct iommu_device *mtk_iommu_probe_device(struct device *dev) struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); struct of_phandle_args iommu_spec; struct mtk_iommu_data *data;

• int err, idx = 0;

• int err, idx = 0, larbid;

• struct device_link *link;

• struct device *larbdev;

  while (!of_parse_phandle_with_args(dev->of_node, "iommus", "#iommu-cells",

@@ -445,6 +447,14 @@ static struct iommu_device *mtk_iommu_probe_device(struct device *dev)

data = dev_iommu_priv_get(dev);

• /* Link the consumer device with the smi-larb device(supplier) */
• larbid = mt2701_m4u_to_larb(fwspec->ids[0]);
• larbdev = data->larb_imu[larbid].dev;
• link = device_link_add(dev, larbdev,

• 	       DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
  

• if (!link)

• dev_err(dev, "Unable to link %s\n", dev_name(larbdev));
  

• return &data->iommu; }

@@ -465,10 +475,18 @@ static void mtk_iommu_probe_finalize(struct device *dev) static void mtk_iommu_release_device(struct device *dev) { struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);

• struct mtk_iommu_data *data;

• struct device *larbdev;

• unsigned int larbid;

  if (!fwspec || fwspec->ops != &mtk_iommu_ops) return;

• data = dev_iommu_priv_get(dev);

• larbid = mt2701_m4u_to_larb(fwspec->ids[0]);

• larbdev = data->larb_imu[larbid].dev;

• device_link_remove(dev, larbdev);

• iommu_fwspec_free(dev); }

On Wed, 2021-07-14 at 10:44 +0200, Dafna Hirschfeld wrote:

On 14.07.21 04:56, Yong Wu wrote:

...

From: Yongqiang Niu yongqiang.niu@mediatek.com

Prepare for smi cleaning up "mediatek,larb".

Display use the dispsys device to call pm_rumtime_get_sync before. This patch add pm_runtime_xx with ovl and rdma device whose nodes has "iommus" property, then display could help pm_runtime_get for smi via ovl or rdma device.

CC: CK Hu ck.hu@mediatek.com Signed-off-by: Yongqiang Niu yongqiang.niu@mediatek.com Signed-off-by: Yong Wu yong.wu@mediatek.com (Yong: Use pm_runtime_resume_and_get instead of pm_runtime_get_sync) Acked-by: Chun-Kuang Hu chunkuang.hu@kernel.org

---

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 | 12 +++++++++++- 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index fa9d79963cd3..ea5760f856ec 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -11,6 +11,7 @@ #include <linux/of_device.h> #include <linux/of_irq.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/soc/mediatek/mtk-cmdq.h>

#include "mtk_disp_drv.h" @@ -414,15 +415,21 @@ static int mtk_disp_ovl_probe(struct platform_device *pdev) return ret; }

• pm_runtime_enable(dev);
• ret = component_add(dev, &mtk_disp_ovl_component_ops);

• if (ret)

• if (ret) {

• pm_runtime_disable(dev);
  

  dev_err(dev, "Failed to add component: %d\n", ret);

• }

  return ret; }

  static int mtk_disp_ovl_remove(struct platform_device *pdev) {

• pm_runtime_disable(&pdev->dev);

• return 0; }

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c index 705f28ceb4dd..0f31d1c8e37c 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c @@ -9,6 +9,7 @@ #include <linux/of_device.h> #include <linux/of_irq.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/soc/mediatek/mtk-cmdq.h>

#include "mtk_disp_drv.h" @@ -327,9 +328,13 @@ static int mtk_disp_rdma_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, priv);

• pm_runtime_enable(dev);
• ret = component_add(dev, &mtk_disp_rdma_component_ops);

• if (ret)

• if (ret) {

• pm_runtime_disable(dev);
  

  dev_err(dev, "Failed to add component: %d\n", ret);

• }

  return ret; }

@@ -338,6 +343,8 @@ static int mtk_disp_rdma_remove(struct platform_device *pdev) { component_del(&pdev->dev, &mtk_disp_rdma_component_ops);

• pm_runtime_disable(&pdev->dev);
• return 0; }

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 474efb844249..08e3f352377d 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -557,9 +557,15 @@ static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc, return; }

• ret = pm_runtime_resume_and_get(comp->dev);
• if (ret < 0)

• DRM_DEV_ERROR(comp->dev, "Failed to enable power domain: %d\n",
  

• 	      ret);
  

shouldn't the code return in case of failure here?

After confirm with yongqiang, We will fix this in next version.

Thanks.

Thanks, Dafna

...

• ret = mtk_crtc_ddp_hw_init(mtk_crtc); if (ret) { mtk_smi_larb_put(comp->larb_dev);

• pm_runtime_put(comp->dev);
  

  return; }

@@ -572,7 +578,7 @@ static void mtk_drm_crtc_atomic_disable(struct drm_crtc *crtc, { struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0];

• int i;

• int i, ret;

  DRM_DEBUG_DRIVER("%s %d\n", __func__, crtc->base.id); if (!mtk_crtc->enabled)

@@ -596,6 +602,10 @@ 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",
  

• 	      ret);
  
  

  mtk_crtc->enabled = false; }

---

Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek

On 14.07.21 04:56, 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

Reviewed-by: Dafna Hirschfeld dafna.hirschfeld@collabora.com

---

drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 9 ------ drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 36 ++------------------- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 1 - drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 +-- 4 files changed, 3 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index 08e3f352377d..d046abcf66ce 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> @@ -551,12 +550,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",

@@ -564,7 +557,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; }

@@ -601,7 +593,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..7d240218d4c7 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c @@ -449,37 +449,15 @@ 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;

+#if IS_REACHABLE(CONFIG_MTK_CMDQ) int ret; +#endif

if (comp_id < 0 || comp_id >= DDP_COMPONENT_ID_MAX) return -EINVAL; @@ -495,16 +473,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 b46bdb8985da..0d5ef3d8d081 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -577,11 +577,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 14.07.21 04:56, Yong Wu 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

Reviewed-by: Dafna Hirschfeld dafna.hirschfeld@collabora.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 | 44 +++---------------- 4 files changed, 10 insertions(+), 75 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 6038db96f71c..d0bf9aa3b29d 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); }

  int mtk_vcodec_dec_pw_on(struct mtk_vcodec_pm *pm)

@@ -122,11 +101,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:

@@ -139,7 +113,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 c6c7672fecfb..64b73dd880ce 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 416f356af363..9a1515cf862d 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..dffb190267ed 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,11 +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:

@@ -125,7 +92,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); }

On 14.07.21 04:56, 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: Krzysztof Kozlowski krzysztof.kozlowski@canonical.com Acked-by: Matthias Brugger matthias.bgg@gmail.com

Reviewed-by: Dafna Hirschfeld dafna.hirschfeld@collabora.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