2023-03-21 17:42:50

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v1] arm64: dts: mt8195: add display node for vdosys1

On 21/03/2023 14:12, Nancy.Lin wrote:
> Add display node for vdosys1.
>
> Signed-off-by: Nancy.Lin <[email protected]>
> Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
>
> ---
> 1. The dts patch is separated from the following patch:
> - [v21,25/25] arm64: dts: mt8195: add display node for vdosys1
> [email protected]
>
> 2. This patch is based on the following series:
> - Add MediaTek SoC DRM (vdosys1) support for mt8195
> [email protected]
> ---
> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 223 +++++++++++++++++++++++
> 1 file changed, 223 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 8fc527570791..c6b625350583 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -24,6 +24,22 @@
> aliases {
> gce0 = &gce0;
> gce1 = &gce1;
> + ethdr0 = &ethdr0;
> + mutex0 = &mutex;
> + mutex1 = &mutex1;
> + merge1 = &merge1;
> + merge2 = &merge2;
> + merge3 = &merge3;
> + merge4 = &merge4;
> + merge5 = &merge5;
> + vdo1-rdma0 = &vdo1_rdma0;
> + vdo1-rdma1 = &vdo1_rdma1;
> + vdo1-rdma2 = &vdo1_rdma2;
> + vdo1-rdma3 = &vdo1_rdma3;
> + vdo1-rdma4 = &vdo1_rdma4;
> + vdo1-rdma5 = &vdo1_rdma5;
> + vdo1-rdma6 = &vdo1_rdma6;
> + vdo1-rdma7 = &vdo1_rdma7;
> };
>
> cpus {
> @@ -2575,7 +2591,10 @@
> vdosys1: syscon@1c100000 {
> compatible = "mediatek,mt8195-vdosys1", "syscon";
> reg = <0 0x1c100000 0 0x1000>;
> + mboxes = <&gce0 1 CMDQ_THR_PRIO_4>;
> + mediatek,gce-client-reg = <&gce0 SUBSYS_1c10XXXX 0x0000 0x1000>;
> #clock-cells = <1>;
> + #reset-cells = <1>;
> };
>
> smi_common_vdo: smi@1c01b000 {
> @@ -2604,6 +2623,17 @@
> power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS0>;
> };
>
> + mutex1: mutex0@1c101000 {

So is it mutex1 or mutex0? Or maybe hwlock?

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation



Best regards,
Krzysztof



2023-03-22 02:13:22

by Nancy Lin (林欣螢)

[permalink] [raw]
Subject: Re: [PATCH v1] arm64: dts: mt8195: add display node for vdosys1

Hi Krzysztof,

Thanks for the review.

On Tue, 2023-03-21 at 18:42 +0100, Krzysztof Kozlowski wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> On 21/03/2023 14:12, Nancy.Lin wrote:
> > Add display node for vdosys1.
> >
> > Signed-off-by: Nancy.Lin <[email protected]>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > [email protected]>
> >
> > ---
> > 1. The dts patch is separated from the following patch:
> > - [v21,25/25] arm64: dts: mt8195: add display node for vdosys1
> > [email protected]
> >
> > 2. This patch is based on the following series:
> > - Add MediaTek SoC DRM (vdosys1) support for mt8195
> > [email protected]
> > ---
> > arch/arm64/boot/dts/mediatek/mt8195.dtsi | 223
> > +++++++++++++++++++++++
> > 1 file changed, 223 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > index 8fc527570791..c6b625350583 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > @@ -24,6 +24,22 @@
> > aliases {
> > gce0 = &gce0;
> > gce1 = &gce1;
> > + ethdr0 = &ethdr0;
> > + mutex0 = &mutex;
> > + mutex1 = &mutex1;
> > + merge1 = &merge1;
> > + merge2 = &merge2;
> > + merge3 = &merge3;
> > + merge4 = &merge4;
> > + merge5 = &merge5;
> > + vdo1-rdma0 = &vdo1_rdma0;
> > + vdo1-rdma1 = &vdo1_rdma1;
> > + vdo1-rdma2 = &vdo1_rdma2;
> > + vdo1-rdma3 = &vdo1_rdma3;
> > + vdo1-rdma4 = &vdo1_rdma4;
> > + vdo1-rdma5 = &vdo1_rdma5;
> > + vdo1-rdma6 = &vdo1_rdma6;
> > + vdo1-rdma7 = &vdo1_rdma7;
> > };
> >
> > cpus {
> > @@ -2575,7 +2591,10 @@
> > vdosys1: syscon@1c100000 {
> > compatible = "mediatek,mt8195-vdosys1",
> > "syscon";
> > reg = <0 0x1c100000 0 0x1000>;
> > + mboxes = <&gce0 1 CMDQ_THR_PRIO_4>;
> > + mediatek,gce-client-reg = <&gce0
> > SUBSYS_1c10XXXX 0x0000 0x1000>;
> > #clock-cells = <1>;
> > + #reset-cells = <1>;
> > };
> >
> > smi_common_vdo: smi@1c01b000 {
> > @@ -2604,6 +2623,17 @@
> > power-domains = <&spm
> > MT8195_POWER_DOMAIN_VDOSYS0>;
> > };
> >
> > + mutex1: mutex0@1c101000 {
>
> So is it mutex1 or mutex0? Or maybe hwlock?
>
> Node names should be generic.
>
https://urldefense.com/v3/__https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html*generic-names-recommendation__;Iw!!CTRNKA9wMg0ARbw!kTTPHVkRSKyI_lzJLpTAADr2d_0OCzH083AMTTqsEM7lgT7R-0ES2YPejeJUbqGSPR7CYLALr6FsNZ-VLOjIpMAFkOsv$
>
>
>
> Best regards,
> Krzysztof


Mutex is used to send the triggers signals called Star Of
Frame(SOF)/End Of Frame(EOF) to each sub-modules on the display data
path[1]. MT8195 have two mutex, one is for vdosys0[2] and the other is
for vdosys1.

This is typo, the node name is mutex. I will change to
=> mutex1: mutex@1c101000 {


[1]mutex binding yaml

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml

[2] arm64: dts: mt8195: Add display node for vdosys0

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/mediatek/mt8195.dtsi?id=b852ee68fd722294145d86bb4d383d4ad4ae270f


Regards,
Nancy



>