Re: [PATCH v2 00/12] Clean up "mediatek,larb" after adding device_link

2019-07-27 Thread Yong Wu
On Tue, 2019-07-23 at 10:31 +0800, CK Hu wrote:
> Hi, Yong:
> 
> I've added log  in mtk_smi_clk_enable() and mtk_smi_clk_disable(), and I
> boot MT8183 with display, the log is
> 
> [4.020340] mtk-smi-common 14019000.smi: mtk_smi_clk_enable()
> [4.331371] mtk-smi-common 14019000.smi: mtk_smi_clk_disable()
> [4.429578] mtk-smi-common 14019000.smi: mtk_smi_clk_enable()
> [4.719743] mtk-smi-common 14019000.smi: mtk_smi_clk_disable()
> [5.084770] mtk-smi-common 14019000.smi: mtk_smi_clk_enable()
> [5.904310] mtk-smi-common 14019000.smi: mtk_smi_clk_disable()
> 
> From the log, the clock is finally turned off, but the display works
> normally. This is because scpsys has turn the clock on,
> 
>   scpsys: syscon@10006000 {
>   compatible = "mediatek,mt8183-scpsys", "syscon";
>   #power-domain-cells = <1>;
>   reg = <0 0x10006000 0 0x1000>;
>   clocks = <&topckgen CLK_TOP_MUX_AUD_INTBUS>,
><&mmsys CLK_MM_SMI_COMMON>,
><&mmsys CLK_MM_GALS_COMM0>,
><&mmsys CLK_MM_GALS_COMM1>,
>   clock-names = "audio","mm-0",
> "mm-1", "mm-2";
>   }
> 
> I'm worried that for MT8173, scpsys would not turn on subsys clock, this
> series would let display work abnormally, so I think smi common should
> not depend on scpsys to turn on the clock.
> 
> You could simply remove the clock parameter in scpsys device node, and
> you would see the display works abnormally.

Thanks for the report. I have reproduced this issue by removing the smi
clock in scpsys node.

Now the probe sequence is like this:
a) mtk scpsys (genpd) probe
b) smi probe
c) iommu probe
d) display and the other MM probe.

The log above are called from device_link_add and rpm_idle which is
called between iommu-probe and display-probe.

At the beginning, all the clocks are enabled. But after this
mtk_smi_clk_enable and mtk_smi_clk_disable, the display clock will be
turned off. If the fastlogo is displayed before kernel, then the display
HW will work abnormally.

I have no mt8173 evb board now, I remembered that the screen only is
showed in display driver of kernel in mt8173, It may be ok in that case.
But It really is a problem when the fastlogo is showed before kernel.

This clock issue happened before display driver, It looks display driver
can not help this. I will try to fix it in SMI(smi try to help
display(larb0) to pm_runtime_get_sync one time and put later).


> 
> Regards,
> CK
> 
> 
> On Mon, 2019-06-10 at 20:55 +0800, Yong Wu wrote:
> > 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.
> > 
> > MediaTek IOMMU don't have its power-domain. 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.
> > 
> > This patchset depends on "MT8183 IOMMU SUPPORT"[1].
> > 
> > [1] https://lists.linuxfoundation.org/pipermail/iommu/2019-June/036552.html
> > 
> > Change notes:
> > v2:
> >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://lists.linuxfoundation.org/pipermail/iommu/2019-January/032387.html
> > 
> > Yong Wu (12):
> >   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: Get rid of mtk_smi_larb_get/put
> >   media: mtk-mdp: Get rid of mtk_smi_larb_get/put
> >   media: mtk-vcodec: Get rid of mtk_smi_larb_get/put
> >   drm/mediatek: Get rid of mtk_smi_larb_get/put
> >   memory: mtk-smi: Get rid of mtk_smi_larb_get/put
> >   iommu/mediatek: Use builtin_platform_driver
> >   arm: dts: mediatek: Get rid of mediatek,larb for MM nodes
> >   arm64: dts: mediatek: Get rid of mediatek,larb for MM nodes
> > 
> >  .../bindings/display/mediatek/mediatek,disp.txt|  9 -
> >  .../bindings/media/mediatek-jpeg-decoder.txt   |  4 --

Re: [PATCH v2 00/12] Clean up "mediatek,larb" after adding device_link

2019-07-22 Thread CK Hu
Hi, Yong:

I've added log  in mtk_smi_clk_enable() and mtk_smi_clk_disable(), and I
boot MT8183 with display, the log is

[4.020340] mtk-smi-common 14019000.smi: mtk_smi_clk_enable()
[4.331371] mtk-smi-common 14019000.smi: mtk_smi_clk_disable()
[4.429578] mtk-smi-common 14019000.smi: mtk_smi_clk_enable()
[4.719743] mtk-smi-common 14019000.smi: mtk_smi_clk_disable()
[5.084770] mtk-smi-common 14019000.smi: mtk_smi_clk_enable()
[5.904310] mtk-smi-common 14019000.smi: mtk_smi_clk_disable()

>From the log, the clock is finally turned off, but the display works
normally. This is because scpsys has turn the clock on,

scpsys: syscon@10006000 {
compatible = "mediatek,mt8183-scpsys", "syscon";
#power-domain-cells = <1>;
reg = <0 0x10006000 0 0x1000>;
clocks = <&topckgen CLK_TOP_MUX_AUD_INTBUS>,
 <&mmsys CLK_MM_SMI_COMMON>,
 <&mmsys CLK_MM_GALS_COMM0>,
 <&mmsys CLK_MM_GALS_COMM1>,
clock-names = "audio","mm-0",
  "mm-1", "mm-2";
}

I'm worried that for MT8173, scpsys would not turn on subsys clock, this
series would let display work abnormally, so I think smi common should
not depend on scpsys to turn on the clock.

You could simply remove the clock parameter in scpsys device node, and
you would see the display works abnormally.

Regards,
CK


On Mon, 2019-06-10 at 20:55 +0800, Yong Wu wrote:
> 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.
> 
> MediaTek IOMMU don't have its power-domain. 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.
> 
> This patchset depends on "MT8183 IOMMU SUPPORT"[1].
> 
> [1] https://lists.linuxfoundation.org/pipermail/iommu/2019-June/036552.html
> 
> Change notes:
> v2:
>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://lists.linuxfoundation.org/pipermail/iommu/2019-January/032387.html
> 
> Yong Wu (12):
>   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: Get rid of mtk_smi_larb_get/put
>   media: mtk-mdp: Get rid of mtk_smi_larb_get/put
>   media: mtk-vcodec: Get rid of mtk_smi_larb_get/put
>   drm/mediatek: Get rid of mtk_smi_larb_get/put
>   memory: mtk-smi: Get rid of mtk_smi_larb_get/put
>   iommu/mediatek: Use builtin_platform_driver
>   arm: dts: mediatek: Get rid of mediatek,larb for MM nodes
>   arm64: dts: mediatek: Get rid of mediatek,larb for MM nodes
> 
>  .../bindings/display/mediatek/mediatek,disp.txt|  9 -
>  .../bindings/media/mediatek-jpeg-decoder.txt   |  4 --
>  .../devicetree/bindings/media/mediatek-mdp.txt |  8 
>  .../devicetree/bindings/media/mediatek-vcodec.txt  |  4 --
>  arch/arm/boot/dts/mt2701.dtsi  |  1 -
>  arch/arm/boot/dts/mt7623.dtsi  |  1 -
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi   | 15 ---
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c| 11 -
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c| 26 
>  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h|  1 -
>  drivers/iommu/mtk_iommu.c  | 45 +++--
>  drivers/iommu/mtk_iommu_v1.c   | 39 +++---
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c| 22 --
>  drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h|  2 -
>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.c  | 38 -
>  drivers/media/platform/mtk-mdp/mtk_mdp_comp.h  |  2 -
>  drivers/media/platform/mtk-mdp/mtk_mdp_core.c  |  1 -
>  .../media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c  | 21 --
>  drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h |  3 --
>  drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c |  1 -
>  .../media/plat

[PATCH v2 00/12] Clean up "mediatek,larb" after adding device_link

2019-06-10 Thread Yong Wu
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.

MediaTek IOMMU don't have its power-domain. 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.

This patchset depends on "MT8183 IOMMU SUPPORT"[1].

[1] https://lists.linuxfoundation.org/pipermail/iommu/2019-June/036552.html

Change notes:
v2:
   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://lists.linuxfoundation.org/pipermail/iommu/2019-January/032387.html

Yong Wu (12):
  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: Get rid of mtk_smi_larb_get/put
  media: mtk-mdp: Get rid of mtk_smi_larb_get/put
  media: mtk-vcodec: Get rid of mtk_smi_larb_get/put
  drm/mediatek: Get rid of mtk_smi_larb_get/put
  memory: mtk-smi: Get rid of mtk_smi_larb_get/put
  iommu/mediatek: Use builtin_platform_driver
  arm: dts: mediatek: Get rid of mediatek,larb for MM nodes
  arm64: dts: mediatek: Get rid of mediatek,larb for MM nodes

 .../bindings/display/mediatek/mediatek,disp.txt|  9 -
 .../bindings/media/mediatek-jpeg-decoder.txt   |  4 --
 .../devicetree/bindings/media/mediatek-mdp.txt |  8 
 .../devicetree/bindings/media/mediatek-vcodec.txt  |  4 --
 arch/arm/boot/dts/mt2701.dtsi  |  1 -
 arch/arm/boot/dts/mt7623.dtsi  |  1 -
 arch/arm64/boot/dts/mediatek/mt8173.dtsi   | 15 ---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c| 11 -
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c| 26 
 drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h|  1 -
 drivers/iommu/mtk_iommu.c  | 45 +++--
 drivers/iommu/mtk_iommu_v1.c   | 39 +++---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c| 22 --
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h|  2 -
 drivers/media/platform/mtk-mdp/mtk_mdp_comp.c  | 38 -
 drivers/media/platform/mtk-mdp/mtk_mdp_comp.h  |  2 -
 drivers/media/platform/mtk-mdp/mtk_mdp_core.c  |  1 -
 .../media/platform/mtk-vcodec/mtk_vcodec_dec_pm.c  | 21 --
 drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h |  3 --
 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c |  1 -
 .../media/platform/mtk-vcodec/mtk_vcodec_enc_pm.c  | 47 --
 drivers/memory/mtk-smi.c   | 31 --
 include/soc/mediatek/smi.h | 20 -
 23 files changed, 36 insertions(+), 316 deletions(-)

-- 
1.9.1