diff mbox series

[v1,1/5] dt-bindings: arm: mediatek: mmsys: add mt8195 SoC binding

Message ID 20210722092624.14401-2-jason-jh.lin@mediatek.com (mailing list archive)
State New, archived
Headers show
Series add mt8195 SoC DRM binding | expand

Commit Message

Jason-JH.Lin July 22, 2021, 9:26 a.m. UTC
There are 2 display hardware path in mt8195, namely vdosys0 and
vdosys1, so add their definition in mtk-mmsys documentation.

Signed-off-by: jason-jh.lin <jason-jh.lin@mediatek.com>
---
this patch is base on [1][2]

[1] dt-bindings: arm: mediatek: mmsys: convert to YAML format
- https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-1-fparent@baylibre.com/
[2] dt-bindings: arm: mediatek: mmsys: add MT8365 SoC binding
- https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-2-fparent@baylibre.com/
---
 .../devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml        | 2 ++
 1 file changed, 2 insertions(+)

Comments

Enric Balletbo Serra July 23, 2021, 11:13 a.m. UTC | #1
Hi Jason,

Thank you for your patch.

Missatge de jason-jh.lin <jason-jh.lin@mediatek.com> del dia dj., 22
de jul. 2021 a les 11:26:
>
> There are 2 display hardware path in mt8195, namely vdosys0 and
> vdosys1, so add their definition in mtk-mmsys documentation.
>

Just having 2 display hardware paths is not a reason to have two
compatibles, isn't the IP block the same? Why do you need to introduce
the two compatibles?

Thanks,
  Enric

> Signed-off-by: jason-jh.lin <jason-jh.lin@mediatek.com>
> ---
> this patch is base on [1][2]
>
> [1] dt-bindings: arm: mediatek: mmsys: convert to YAML format
> - https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-1-fparent@baylibre.com/
> [2] dt-bindings: arm: mediatek: mmsys: add MT8365 SoC binding
> - https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-2-fparent@baylibre.com/
> ---
>  .../devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml        | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> index 2d4ff0ce387b..0789a9614f12 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> @@ -30,6 +30,8 @@ properties:
>                - mediatek,mt8173-mmsys
>                - mediatek,mt8183-mmsys
>                - mediatek,mt8365-mmsys
> +              - mediatek,mt8195-vdosys0
> +              - mediatek,mt8195-vdosys1
>            - const: syscon
>        - items:
>            - const: mediatek,mt7623-mmsys
> --
> 2.18.0
>
Jason-JH.Lin July 26, 2021, 7:02 a.m. UTC | #2
On Fri, 2021-07-23 at 13:13 +0200, Enric Balletbo Serra wrote:
> Hi Jason,
> 
> Thank you for your patch.
> 
> Missatge de jason-jh.lin <jason-jh.lin@mediatek.com> del dia dj., 22
> de jul. 2021 a les 11:26:
> > 
> > There are 2 display hardware path in mt8195, namely vdosys0 and
> > vdosys1, so add their definition in mtk-mmsys documentation.
> > 
> 
> Just having 2 display hardware paths is not a reason to have two
> compatibles, isn't the IP block the same? Why do you need to
> introduce
> the two compatibles?
> 
> Thanks,
>   Enric
> 

Hi Enric,

Thanks for reviewing my patch.

The reason for using two compatibles is that vdosys0 and vdosys1 are
different IP blocks.

Because mmsys provides clock control, other display function blocks may
use them as clock provider.

E.g.
1. mmsys with compatible="mediatek,mt8195-vdosys0"
[v4,1/6] arm64: dts: mt8195: add display node for vdosys0

https://patchwork.kernel.org/project/linux-mediatek/patch/20210723090233.24007-2-jason-jh.lin@mediatek.com/

ovl0: disp_ovl@1c000000 {
	...
	clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
	...
};

2. mmsys with compatible="mediatek,mt8195-vdosys1"
[v2,06/14] arm64: dts: mt8195: add display node for vdosys1

https://patchwork.kernel.org/project/linux-mediatek/patch/20210722094551.15255-7-nancy.lin@mediatek.com/

vdo1_rdma0: vdo1_rdma@1c104000 {
	...
	clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>;
	...
};

Regards,
Jason-JH.Lin

> > Signed-off-by: jason-jh.lin <jason-jh.lin@mediatek.com>
> > ---
> > this patch is base on [1][2]
> > 
> > [1] dt-bindings: arm: mediatek: mmsys: convert to YAML format
> > - 
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-1-fparent@baylibre.com/__;!!CTRNKA9wMg0ARbw!ycgPEK4yBDojiiZJC2E9mGwvxJbaLqhyUxzJIq0ckEP-JVteBcjFdc6ixkNbmknH8f2P$
> >  
> > [2] dt-bindings: arm: mediatek: mmsys: add MT8365 SoC binding
> > - 
> > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-2-fparent@baylibre.com/__;!!CTRNKA9wMg0ARbw!ycgPEK4yBDojiiZJC2E9mGwvxJbaLqhyUxzJIq0ckEP-JVteBcjFdc6ixkNbmju2GBrD$
> >  
> > ---
> >  .../devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml        |
> > 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > l
> > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > l
> > index 2d4ff0ce387b..0789a9614f12 100644
> > ---
> > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > l
> > +++
> > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > l
> > @@ -30,6 +30,8 @@ properties:
> >                - mediatek,mt8173-mmsys
> >                - mediatek,mt8183-mmsys
> >                - mediatek,mt8365-mmsys
> > +              - mediatek,mt8195-vdosys0
> > +              - mediatek,mt8195-vdosys1
> >            - const: syscon
> >        - items:
> >            - const: mediatek,mt7623-mmsys
> > --
> > 2.18.0
> >
Enric Balletbo Serra July 26, 2021, 10:08 a.m. UTC | #3
Hi Jason,

Missatge de Jason-JH Lin <jason-jh.lin@mediatek.com> del dia dl., 26
de jul. 2021 a les 9:02:
>
> On Fri, 2021-07-23 at 13:13 +0200, Enric Balletbo Serra wrote:
> > Hi Jason,
> >
> > Thank you for your patch.
> >
> > Missatge de jason-jh.lin <jason-jh.lin@mediatek.com> del dia dj., 22
> > de jul. 2021 a les 11:26:
> > >
> > > There are 2 display hardware path in mt8195, namely vdosys0 and
> > > vdosys1, so add their definition in mtk-mmsys documentation.
> > >
> >
> > Just having 2 display hardware paths is not a reason to have two
> > compatibles, isn't the IP block the same? Why do you need to
> > introduce
> > the two compatibles?
> >
> > Thanks,
> >   Enric
> >
>
> Hi Enric,
>
> Thanks for reviewing my patch.
>
> The reason for using two compatibles is that vdosys0 and vdosys1 are
> different IP blocks.
>

With that there are different IP blocks, what do you mean? Do you mean
that there are two completely different blocks with completely
different functionalities?

Or that there is the same IP block twice? I mean, of course, the
registers are different but has exactly the same functionality.

> Because mmsys provides clock control, other display function blocks may
> use them as clock provider.
>
> E.g.
> 1. mmsys with compatible="mediatek,mt8195-vdosys0"
> [v4,1/6] arm64: dts: mt8195: add display node for vdosys0
>
> https://patchwork.kernel.org/project/linux-mediatek/patch/20210723090233.24007-2-jason-jh.lin@mediatek.com/
>
> ovl0: disp_ovl@1c000000 {
>         ...
>         clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
>         ...
> };
>
> 2. mmsys with compatible="mediatek,mt8195-vdosys1"
> [v2,06/14] arm64: dts: mt8195: add display node for vdosys1
>
> https://patchwork.kernel.org/project/linux-mediatek/patch/20210722094551.15255-7-nancy.lin@mediatek.com/
>
> vdo1_rdma0: vdo1_rdma@1c104000 {
>         ...
>         clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>;
>         ...
> };
>

Note that I am talking without knowing the hardware in detail, but I
am wondering why I can't have something like this, where every mmsys
is a clock and reset controller provider.

vdosys0: syscon@14000000 {
          compatible = "mediatek,mt8195-mmsys", "syscon";
          reg = <0 0x14000000 0 0x1000>;
          #clock-cells = <1>;
          #reset-cells = <1>;
};

vdosys1: syscon@15000000 {
          compatible = "mediatek,mt8195-mmsys", "syscon";
          reg = <0 0x15000000 0 0x1000>;
          #clock-cells = <1>;
          #reset-cells = <1>;
};

ovl0: disp_ovl@1c000000 {
        ...
       clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
        ...
};

vdo1_rdma0: vdo1_rdma@1c104000 {
        ...
        clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>;
        ...
};

What are the differences between vdosys0 and vdosys1 from a hardware
point of view?

Cheers,
  Enric

> Regards,
> Jason-JH.Lin
>
> > > Signed-off-by: jason-jh.lin <jason-jh.lin@mediatek.com>
> > > ---
> > > this patch is base on [1][2]
> > >
> > > [1] dt-bindings: arm: mediatek: mmsys: convert to YAML format
> > > -
> > > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-1-fparent@baylibre.com/__;!!CTRNKA9wMg0ARbw!ycgPEK4yBDojiiZJC2E9mGwvxJbaLqhyUxzJIq0ckEP-JVteBcjFdc6ixkNbmknH8f2P$
> > >
> > > [2] dt-bindings: arm: mediatek: mmsys: add MT8365 SoC binding
> > > -
> > > https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-2-fparent@baylibre.com/__;!!CTRNKA9wMg0ARbw!ycgPEK4yBDojiiZJC2E9mGwvxJbaLqhyUxzJIq0ckEP-JVteBcjFdc6ixkNbmju2GBrD$
> > >
> > > ---
> > >  .../devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml        |
> > > 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > > l
> > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > > l
> > > index 2d4ff0ce387b..0789a9614f12 100644
> > > ---
> > > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > > l
> > > +++
> > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > > l
> > > @@ -30,6 +30,8 @@ properties:
> > >                - mediatek,mt8173-mmsys
> > >                - mediatek,mt8183-mmsys
> > >                - mediatek,mt8365-mmsys
> > > +              - mediatek,mt8195-vdosys0
> > > +              - mediatek,mt8195-vdosys1
> > >            - const: syscon
> > >        - items:
> > >            - const: mediatek,mt7623-mmsys
> > > --
> > > 2.18.0
> > >
> --
Jason-JH.Lin July 27, 2021, 2:53 a.m. UTC | #4
Hi Enric,

On Mon, 2021-07-26 at 12:08 +0200, Enric Balletbo Serra wrote:
> Hi Jason,
> 
> Missatge de Jason-JH Lin <jason-jh.lin@mediatek.com> del dia dl., 26
> de jul. 2021 a les 9:02:
> > 
> > On Fri, 2021-07-23 at 13:13 +0200, Enric Balletbo Serra wrote:
> > > Hi Jason,
> > > 
> > > Thank you for your patch.
> > > 
> > > Missatge de jason-jh.lin <jason-jh.lin@mediatek.com> del dia dj.,
> > > 22
> > > de jul. 2021 a les 11:26:
> > > > 
> > > > There are 2 display hardware path in mt8195, namely vdosys0 and
> > > > vdosys1, so add their definition in mtk-mmsys documentation.
> > > > 
> > > 
> > > Just having 2 display hardware paths is not a reason to have two
> > > compatibles, isn't the IP block the same? Why do you need to
> > > introduce
> > > the two compatibles?
> > > 
> > > Thanks,
> > >   Enric
> > > 
> > 
> > Hi Enric,
> > 
> > Thanks for reviewing my patch.
> > 
> > The reason for using two compatibles is that vdosys0 and vdosys1
> > are
> > different IP blocks.
> > 
> 
> With that there are different IP blocks, what do you mean? Do you
> mean
> that there are two completely different blocks with completely
> different functionalities?
> 
> Or that there is the same IP block twice? I mean, of course, the
> registers are different but has exactly the same functionality.
> 

They are not the same IP block twice.
Although both vdosys0 and vdosys1 will probe meiatek-drm driver, but
the components on their hardware path are different and their output
panel are also different.

> > Because mmsys provides clock control, other display function blocks
> > may
> > use them as clock provider.
> > 
> > E.g.
> > 1. mmsys with compatible="mediatek,mt8195-vdosys0"
> > [v4,1/6] arm64: dts: mt8195: add display node for vdosys0
> > 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210723090233.24007-2-jason-jh.lin@mediatek.com/__;!!CTRNKA9wMg0ARbw!xHjKwv34W7ETFcmQPXRViylF2LbV7C7pE8OeJeNvA93jDdzr_ZBRRm8aIUCvAHSD_qGo$
> >  
> > 
> > ovl0: disp_ovl@1c000000 {
> >         ...
> >         clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
> >         ...
> > };
> > 
> > 2. mmsys with compatible="mediatek,mt8195-vdosys1"
> > [v2,06/14] arm64: dts: mt8195: add display node for vdosys1
> > 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210722094551.15255-7-nancy.lin@mediatek.com/__;!!CTRNKA9wMg0ARbw!xHjKwv34W7ETFcmQPXRViylF2LbV7C7pE8OeJeNvA93jDdzr_ZBRRm8aIUCvAP0FOfkc$
> >  
> > 
> > vdo1_rdma0: vdo1_rdma@1c104000 {
> >         ...
> >         clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>;
> >         ...
> > };
> > 
> 
> Note that I am talking without knowing the hardware in detail, but I
> am wondering why I can't have something like this, where every mmsys
> is a clock and reset controller provider.
> 
> vdosys0: syscon@14000000 {
>           compatible = "mediatek,mt8195-mmsys", "syscon";
>           reg = <0 0x14000000 0 0x1000>;
>           #clock-cells = <1>;
>           #reset-cells = <1>;
> };
> 
> vdosys1: syscon@15000000 {
>           compatible = "mediatek,mt8195-mmsys", "syscon";
>           reg = <0 0x15000000 0 0x1000>;
>           #clock-cells = <1>;
>           #reset-cells = <1>;
> };
> 
> ovl0: disp_ovl@1c000000 {
>         ...
>        clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
>         ...
> };
> 
> vdo1_rdma0: vdo1_rdma@1c104000 {
>         ...
>         clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>;
>         ...
> };
> 
> What are the differences between vdosys0 and vdosys1 from a hardware
> point of view?
> 
> Cheers,
>   Enric
> 
> > Regards,
> > Jason-JH.Lin
> > 

From a hardware point of view, the components and the ouptut panel of
vdosys0 and vdosys1:
1. The components on meiatek-drm of vdosys0 are OVL0, RDMA0, COLOR0,
CCORR, AAL0, GAMMA, DITHER, DSC0, MERGE0, DP_INTF0 and its output panel
is eDP.

2. The components on meiatek-drm of vdosys1 are PSEUDO_OVL, MERGE5,
DP_INTF1 and its ouptut panel is DP.


The resaon for using two compatibales is that we use different driver
data in mtk-mmsys.c and mtk_drm_drv.c to identify the corresponding 
mmsys is vdosys0 or vdosys1.

Their driver data in mtk_drm_drv.c is defined here:
[v4,4/6] drm/mediatek: add mediatek-drm of vdosys0 support for mt8195

https://patchwork.kernel.org/project/linux-mediatek/patch/20210723090233.24007-5-jason-jh.lin@mediatek.com/
[v2,14/14] drm/mediatek: add mediatek-drm of vdosys1 support for MT8195

https://patchwork.kernel.org/project/linux-mediatek/patch/20210722094551.15255-15-nancy.lin@mediatek.com/

I think using the same compatible is unable to do this. Or do you have
other suggestions to do this with the same compatibe?

Regards,
Jason-JH.Lin

> > > > Signed-off-by: jason-jh.lin <jason-jh.lin@mediatek.com>
> > > > ---
> > > > this patch is base on [1][2]
> > > > 
> > > > [1] dt-bindings: arm: mediatek: mmsys: convert to YAML format
> > > > -
> > > > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-1-fparent@baylibre.com/__;!!CTRNKA9wMg0ARbw!ycgPEK4yBDojiiZJC2E9mGwvxJbaLqhyUxzJIq0ckEP-JVteBcjFdc6ixkNbmknH8f2P$
> > > > 
> > > > [2] dt-bindings: arm: mediatek: mmsys: add MT8365 SoC binding
> > > > -
> > > > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-2-fparent@baylibre.com/__;!!CTRNKA9wMg0ARbw!ycgPEK4yBDojiiZJC2E9mGwvxJbaLqhyUxzJIq0ckEP-JVteBcjFdc6ixkNbmju2GBrD$
> > > > 
> > > > ---
> > > >  .../devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml      
> > > >   |
> > > > 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys
> > > > .yam
> > > > l
> > > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys
> > > > .yam
> > > > l
> > > > index 2d4ff0ce387b..0789a9614f12 100644
> > > > ---
> > > > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys
> > > > .yam
> > > > l
> > > > +++
> > > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys
> > > > .yam
> > > > l
> > > > @@ -30,6 +30,8 @@ properties:
> > > >                - mediatek,mt8173-mmsys
> > > >                - mediatek,mt8183-mmsys
> > > >                - mediatek,mt8365-mmsys
> > > > +              - mediatek,mt8195-vdosys0
> > > > +              - mediatek,mt8195-vdosys1
> > > >            - const: syscon
> > > >        - items:
> > > >            - const: mediatek,mt7623-mmsys
> > > > --
> > > > 2.18.0
> > > > 
> > 
> > --
Enric Balletbo Serra July 28, 2021, 10:56 a.m. UTC | #5
Hi Jason,

Missatge de Jason-JH Lin <jason-jh.lin@mediatek.com> del dia dt., 27
de jul. 2021 a les 4:53:
>
> Hi Enric,
>
> On Mon, 2021-07-26 at 12:08 +0200, Enric Balletbo Serra wrote:
> > Hi Jason,
> >
> > Missatge de Jason-JH Lin <jason-jh.lin@mediatek.com> del dia dl., 26
> > de jul. 2021 a les 9:02:
> > >
> > > On Fri, 2021-07-23 at 13:13 +0200, Enric Balletbo Serra wrote:
> > > > Hi Jason,
> > > >
> > > > Thank you for your patch.
> > > >
> > > > Missatge de jason-jh.lin <jason-jh.lin@mediatek.com> del dia dj.,
> > > > 22
> > > > de jul. 2021 a les 11:26:
> > > > >
> > > > > There are 2 display hardware path in mt8195, namely vdosys0 and
> > > > > vdosys1, so add their definition in mtk-mmsys documentation.
> > > > >
> > > >
> > > > Just having 2 display hardware paths is not a reason to have two
> > > > compatibles, isn't the IP block the same? Why do you need to
> > > > introduce
> > > > the two compatibles?
> > > >
> > > > Thanks,
> > > >   Enric
> > > >
> > >
> > > Hi Enric,
> > >
> > > Thanks for reviewing my patch.
> > >
> > > The reason for using two compatibles is that vdosys0 and vdosys1
> > > are
> > > different IP blocks.
> > >
> >
> > With that there are different IP blocks, what do you mean? Do you
> > mean
> > that there are two completely different blocks with completely
> > different functionalities?
> >
> > Or that there is the same IP block twice? I mean, of course, the
> > registers are different but has exactly the same functionality.
> >
>
> They are not the same IP block twice.
> Although both vdosys0 and vdosys1 will probe meiatek-drm driver, but
> the components on their hardware path are different and their output
> panel are also different.
>
> > > Because mmsys provides clock control, other display function blocks
> > > may
> > > use them as clock provider.
> > >
> > > E.g.
> > > 1. mmsys with compatible="mediatek,mt8195-vdosys0"
> > > [v4,1/6] arm64: dts: mt8195: add display node for vdosys0
> > >
> > >
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210723090233.24007-2-jason-jh.lin@mediatek.com/__;!!CTRNKA9wMg0ARbw!xHjKwv34W7ETFcmQPXRViylF2LbV7C7pE8OeJeNvA93jDdzr_ZBRRm8aIUCvAHSD_qGo$
> > >
> > >
> > > ovl0: disp_ovl@1c000000 {
> > >         ...
> > >         clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
> > >         ...
> > > };
> > >
> > > 2. mmsys with compatible="mediatek,mt8195-vdosys1"
> > > [v2,06/14] arm64: dts: mt8195: add display node for vdosys1
> > >
> > >
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210722094551.15255-7-nancy.lin@mediatek.com/__;!!CTRNKA9wMg0ARbw!xHjKwv34W7ETFcmQPXRViylF2LbV7C7pE8OeJeNvA93jDdzr_ZBRRm8aIUCvAP0FOfkc$
> > >
> > >
> > > vdo1_rdma0: vdo1_rdma@1c104000 {
> > >         ...
> > >         clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>;
> > >         ...
> > > };
> > >
> >
> > Note that I am talking without knowing the hardware in detail, but I
> > am wondering why I can't have something like this, where every mmsys
> > is a clock and reset controller provider.
> >
> > vdosys0: syscon@14000000 {
> >           compatible = "mediatek,mt8195-mmsys", "syscon";
> >           reg = <0 0x14000000 0 0x1000>;
> >           #clock-cells = <1>;
> >           #reset-cells = <1>;
> > };
> >
> > vdosys1: syscon@15000000 {
> >           compatible = "mediatek,mt8195-mmsys", "syscon";
> >           reg = <0 0x15000000 0 0x1000>;
> >           #clock-cells = <1>;
> >           #reset-cells = <1>;
> > };
> >
> > ovl0: disp_ovl@1c000000 {
> >         ...
> >        clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
> >         ...
> > };
> >
> > vdo1_rdma0: vdo1_rdma@1c104000 {
> >         ...
> >         clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>;
> >         ...
> > };
> >
> > What are the differences between vdosys0 and vdosys1 from a hardware
> > point of view?
> >
> > Cheers,
> >   Enric
> >
> > > Regards,
> > > Jason-JH.Lin
> > >
>
> From a hardware point of view, the components and the ouptut panel of
> vdosys0 and vdosys1:
> 1. The components on meiatek-drm of vdosys0 are OVL0, RDMA0, COLOR0,
> CCORR, AAL0, GAMMA, DITHER, DSC0, MERGE0, DP_INTF0 and its output panel
> is eDP.
>
> 2. The components on meiatek-drm of vdosys1 are PSEUDO_OVL, MERGE5,
> DP_INTF1 and its ouptut panel is DP.
>
>
> The resaon for using two compatibales is that we use different driver
> data in mtk-mmsys.c and mtk_drm_drv.c to identify the corresponding
> mmsys is vdosys0 or vdosys1.
>

So IIUC the two mmsys block ares basically the same, they provide the
same, a reset controller, a clock controller and being able to write
to the routing registers, and what you only need to identify is which
one is is vdosys0 and which one is vdosys1 to apply different routing
tables? Can these routing tables change at runtime? Or are they
hardware fixed?

Regards,
 Enric

> Their driver data in mtk_drm_drv.c is defined here:
> [v4,4/6] drm/mediatek: add mediatek-drm of vdosys0 support for mt8195
>
> https://patchwork.kernel.org/project/linux-mediatek/patch/20210723090233.24007-5-jason-jh.lin@mediatek.com/
> [v2,14/14] drm/mediatek: add mediatek-drm of vdosys1 support for MT8195
>
> https://patchwork.kernel.org/project/linux-mediatek/patch/20210722094551.15255-15-nancy.lin@mediatek.com/
>
> I think using the same compatible is unable to do this. Or do you have
> other suggestions to do this with the same compatibe?
>
> Regards,
> Jason-JH.Lin
>
> > > > > Signed-off-by: jason-jh.lin <jason-jh.lin@mediatek.com>
> > > > > ---
> > > > > this patch is base on [1][2]
> > > > >
> > > > > [1] dt-bindings: arm: mediatek: mmsys: convert to YAML format
> > > > > -
> > > > >
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-1-fparent@baylibre.com/__;!!CTRNKA9wMg0ARbw!ycgPEK4yBDojiiZJC2E9mGwvxJbaLqhyUxzJIq0ckEP-JVteBcjFdc6ixkNbmknH8f2P$
> > > > >
> > > > > [2] dt-bindings: arm: mediatek: mmsys: add MT8365 SoC binding
> > > > > -
> > > > >
> https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-2-fparent@baylibre.com/__;!!CTRNKA9wMg0ARbw!ycgPEK4yBDojiiZJC2E9mGwvxJbaLqhyUxzJIq0ckEP-JVteBcjFdc6ixkNbmju2GBrD$
> > > > >
> > > > > ---
> > > > >  .../devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> > > > >   |
> > > > > 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys
> > > > > .yam
> > > > > l
> > > > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys
> > > > > .yam
> > > > > l
> > > > > index 2d4ff0ce387b..0789a9614f12 100644
> > > > > ---
> > > > > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys
> > > > > .yam
> > > > > l
> > > > > +++
> > > > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys
> > > > > .yam
> > > > > l
> > > > > @@ -30,6 +30,8 @@ properties:
> > > > >                - mediatek,mt8173-mmsys
> > > > >                - mediatek,mt8183-mmsys
> > > > >                - mediatek,mt8365-mmsys
> > > > > +              - mediatek,mt8195-vdosys0
> > > > > +              - mediatek,mt8195-vdosys1
> > > > >            - const: syscon
> > > > >        - items:
> > > > >            - const: mediatek,mt7623-mmsys
> > > > > --
> > > > > 2.18.0
> > > > >
> > >
> > > --
> --
> Jason-JH Lin <jason-jh.lin@mediatek.com>
Jason-JH.Lin July 29, 2021, 9:31 a.m. UTC | #6
Hi Enric,

Thanks for your review.

On Wed, 2021-07-28 at 12:56 +0200, Enric Balletbo Serra wrote:
> Hi Jason,
> 
> Missatge de Jason-JH Lin <jason-jh.lin@mediatek.com> del dia dt., 27
> de jul. 2021 a les 4:53:
> > 
> > Hi Enric,
> > 
> > On Mon, 2021-07-26 at 12:08 +0200, Enric Balletbo Serra wrote:
> > > Hi Jason,
> > > 
> > > Missatge de Jason-JH Lin <jason-jh.lin@mediatek.com> del dia dl.,
> > > 26
> > > de jul. 2021 a les 9:02:
> > > > 
> > > > On Fri, 2021-07-23 at 13:13 +0200, Enric Balletbo Serra wrote:
> > > > > Hi Jason,
> > > > > 
> > > > > Thank you for your patch.
> > > > > 
> > > > > Missatge de jason-jh.lin <jason-jh.lin@mediatek.com> del dia
> > > > > dj.,
> > > > > 22
> > > > > de jul. 2021 a les 11:26:
> > > > > > 
> > > > > > There are 2 display hardware path in mt8195, namely vdosys0
> > > > > > and
> > > > > > vdosys1, so add their definition in mtk-mmsys
> > > > > > documentation.
> > > > > > 
> > > > > 
> > > > > Just having 2 display hardware paths is not a reason to have
> > > > > two
> > > > > compatibles, isn't the IP block the same? Why do you need to
> > > > > introduce
> > > > > the two compatibles?
> > > > > 
> > > > > Thanks,
> > > > >   Enric
> > > > > 
> > > > 
> > > > Hi Enric,
> > > > 
> > > > Thanks for reviewing my patch.
> > > > 
> > > > The reason for using two compatibles is that vdosys0 and
> > > > vdosys1
> > > > are
> > > > different IP blocks.
> > > > 
> > > 
> > > With that there are different IP blocks, what do you mean? Do you
> > > mean
> > > that there are two completely different blocks with completely
> > > different functionalities?
> > > 
> > > Or that there is the same IP block twice? I mean, of course, the
> > > registers are different but has exactly the same functionality.
> > > 
> > 
> > They are not the same IP block twice.
> > Although both vdosys0 and vdosys1 will probe meiatek-drm driver,
> > but
> > the components on their hardware path are different and their
> > output
> > panel are also different.
> > 
> > > > Because mmsys provides clock control, other display function
> > > > blocks
> > > > may
> > > > use them as clock provider.
> > > > 
> > > > E.g.
> > > > 1. mmsys with compatible="mediatek,mt8195-vdosys0"
> > > > [v4,1/6] arm64: dts: mt8195: add display node for vdosys0
> > > > 
> > > > 
> > 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210723090233.24007-2-jason-jh.lin@mediatek.com/__;!!CTRNKA9wMg0ARbw!xHjKwv34W7ETFcmQPXRViylF2LbV7C7pE8OeJeNvA93jDdzr_ZBRRm8aIUCvAHSD_qGo$
> > > > 
> > > > 
> > > > ovl0: disp_ovl@1c000000 {
> > > >         ...
> > > >         clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
> > > >         ...
> > > > };
> > > > 
> > > > 2. mmsys with compatible="mediatek,mt8195-vdosys1"
> > > > [v2,06/14] arm64: dts: mt8195: add display node for vdosys1
> > > > 
> > > > 
> > 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210722094551.15255-7-nancy.lin@mediatek.com/__;!!CTRNKA9wMg0ARbw!xHjKwv34W7ETFcmQPXRViylF2LbV7C7pE8OeJeNvA93jDdzr_ZBRRm8aIUCvAP0FOfkc$
> > > > 
> > > > 
> > > > vdo1_rdma0: vdo1_rdma@1c104000 {
> > > >         ...
> > > >         clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>;
> > > >         ...
> > > > };
> > > > 
> > > 
> > > Note that I am talking without knowing the hardware in detail,
> > > but I
> > > am wondering why I can't have something like this, where every
> > > mmsys
> > > is a clock and reset controller provider.
> > > 
> > > vdosys0: syscon@14000000 {
> > >           compatible = "mediatek,mt8195-mmsys", "syscon";
> > >           reg = <0 0x14000000 0 0x1000>;
> > >           #clock-cells = <1>;
> > >           #reset-cells = <1>;
> > > };
> > > 
> > > vdosys1: syscon@15000000 {
> > >           compatible = "mediatek,mt8195-mmsys", "syscon";
> > >           reg = <0 0x15000000 0 0x1000>;
> > >           #clock-cells = <1>;
> > >           #reset-cells = <1>;
> > > };
> > > 
> > > ovl0: disp_ovl@1c000000 {
> > >         ...
> > >        clocks = <&vdosys0 CLK_VDO0_DISP_OVL0>;
> > >         ...
> > > };
> > > 
> > > vdo1_rdma0: vdo1_rdma@1c104000 {
> > >         ...
> > >         clocks = <&vdosys1 CLK_VDO1_MDP_RDMA0>;
> > >         ...
> > > };
> > > 
> > > What are the differences between vdosys0 and vdosys1 from a
> > > hardware
> > > point of view?
> > > 
> > > Cheers,
> > >   Enric
> > > 
> > > > Regards,
> > > > Jason-JH.Lin
> > > > 
> > 
> > From a hardware point of view, the components and the ouptut panel
> > of
> > vdosys0 and vdosys1:
> > 1. The components on meiatek-drm of vdosys0 are OVL0, RDMA0,
> > COLOR0,
> > CCORR, AAL0, GAMMA, DITHER, DSC0, MERGE0, DP_INTF0 and its output
> > panel
> > is eDP.
> > 
> > 2. The components on meiatek-drm of vdosys1 are PSEUDO_OVL, MERGE5,
> > DP_INTF1 and its ouptut panel is DP.
> > 
> > 
> > The resaon for using two compatibales is that we use different
> > driver
> > data in mtk-mmsys.c and mtk_drm_drv.c to identify the corresponding
> > mmsys is vdosys0 or vdosys1.
> > 
> 
> So IIUC the two mmsys block ares basically the same, they provide the
> same, a reset controller, a clock controller and being able to write
> to the routing registers, and what you only need to identify is which
> one is is vdosys0 and which one is vdosys1 to apply different routing
> tables? Can these routing tables change at runtime? Or are they
> hardware fixed?
> 
> Regards,
>  Enric
> 
In the case of vdosys1 DP_TX switch to HDMI_TX, routing tables will
change at runtime.

Refer to mt8173.dtsi:

https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt8173.dtsi
I forgot to put the power domain property in vdosys0 dts node.
So I'll add this into DRM vdosys0 series at the next version:
vdosys0: syscon@1c01a000 {
 	compatible = "mediatek,mt8195-vdosys0", "syscon";
 	reg = <0 0x1c01a000 0 0x1000>;
	power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
	...
};
I'll also tell Nancy to add this:
vdosys1: syscon@1c100000 {
	compatible = "mediatek,mt8195-vdosys1", "syscon";
	reg = <0 0x1c100000 0 0x1000>;
	power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS1>;	...
};
I'll also add the power-domian description into this yaml.
power-domains:
description: A phandle and PM domain specifier as defined by bindings
of the power controller specified by phandle. See
Documentation/devicetree/bindings/power/power-domain.yaml for details.

In the SoC before, such as mt8173, it has 2 pipelines binding to one
mmsys with the same clock driver and the same power domain.

In mt8195, 2 pipelines are binding to different mmsys, such as vdosys0
and vdosys1. Each mmsys uses different clock drivers and different
power domain.
So I think it is more appropriate to use 2 compatibles to identify
which mmsys represents the pipeline.


Regards,
Jason-JH.Lin

> > Their driver data in mtk_drm_drv.c is defined here:
> > [v4,4/6] drm/mediatek: add mediatek-drm of vdosys0 support for
> > mt8195
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210723090233.24007-5-jason-jh.lin@mediatek.com/
> >  
> > [v2,14/14] drm/mediatek: add mediatek-drm of vdosys1 support for
> > MT8195
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210722094551.15255-15-nancy.lin@mediatek.com
> > 
> > 
> > I think using the same compatible is unable to do this. Or do you
> > have
> > other suggestions to do this with the same compatibe?
> > 
> > Regards,
> > Jason-JH.Lin
> > 
> > > > > > Signed-off-by: jason-jh.lin <jason-jh.lin@mediatek.com>
> > > > > > ---
> > > > > > this patch is base on [1][2]
> > > > > > 
> > > > > > [1] dt-bindings: arm: mediatek: mmsys: convert to YAML
> > > > > > format
> > > > > > -
> > > > > > 
> > 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-1-fparent@baylibre.com/
> > > > > > 
> > > > > > [2] dt-bindings: arm: mediatek: mmsys: add MT8365 SoC
> > > > > > binding
> > > > > > -
> > > > > > 
> > 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20210519161847.3747352-2-fparent@baylibre.com/
> > > > > > 
> > > > > > ---
> > > > > >  .../devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> > > > > >   |
> > > > > > 2 ++
> > > > > >  1 file changed, 2 insertions(+)
> > > > > > 
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,m
> > > > > > msys
> > > > > > .yam
> > > > > > l
> > > > > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,m
> > > > > > msys
> > > > > > .yam
> > > > > > l
> > > > > > index 2d4ff0ce387b..0789a9614f12 100644
> > > > > > ---
> > > > > > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,m
> > > > > > msys
> > > > > > .yam
> > > > > > l
> > > > > > +++
> > > > > > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,m
> > > > > > msys
> > > > > > .yam
> > > > > > l
> > > > > > @@ -30,6 +30,8 @@ properties:
> > > > > >                - mediatek,mt8173-mmsys
> > > > > >                - mediatek,mt8183-mmsys
> > > > > >                - mediatek,mt8365-mmsys
> > > > > > +              - mediatek,mt8195-vdosys0
> > > > > > +              - mediatek,mt8195-vdosys1
> > > > > >            - const: syscon
> > > > > >        - items:
> > > > > >            - const: mediatek,mt7623-mmsys
> > > > > > --
> > > > > > 2.18.0
> > > > > > 
> > > > 
> > > > --
> > 
> > --
> > Jason-JH Lin <jason-jh.lin@mediatek.com>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
index 2d4ff0ce387b..0789a9614f12 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
@@ -30,6 +30,8 @@  properties:
               - mediatek,mt8173-mmsys
               - mediatek,mt8183-mmsys
               - mediatek,mt8365-mmsys
+              - mediatek,mt8195-vdosys0
+              - mediatek,mt8195-vdosys1
           - const: syscon
       - items:
           - const: mediatek,mt7623-mmsys