[PATCH v3] dt-bindings: dma: xilinx: Add power-domains to xlnx,zynqmp-dpdma

[PATCH v3] dt-bindings: dma: xilinx: Add power-domains to xlnx,zynqmp-dpdma

[Michal Simek]

DP DMA has own power domain that's why describe required power-domain
property.

Signed-off-by: Michal Simek <michal.simek@amd.com>
---

Changes in v3:
- make power-domains as required property
- also update commit message

Changes in v2:
- rewrite commit message - requested by Krzysztof

The commit b06112cd5e08 ("arm64: dts: zynqmp: Add power domain for the
DisplayPort DMA controller") added this property already in Linux that's
why the patch is also fixing dts_check warnings.

In v2 I got ack from Krzysztof but not adding it because of additional
discussion about required property in v3.
https://lore.kernel.org/r/029ba923-d13e-ea7c-018d-95e179dda2e5@linaro.org

---
 .../devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml   | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
index d6cbd95ec26d..2128f4645c98 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
+++ b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
@@ -41,6 +41,9 @@ properties:
   clock-names:
     const: axi_clk
 
+  power-domains:
+    maxItems: 1
+
 required:
   - "#dma-cells"
   - compatible
@@ -48,12 +51,14 @@ required:
   - interrupts
   - clocks
   - clock-names
+  - power-domains
 
 additionalProperties: false
 
 examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/xlnx-zynqmp-power.h>
 
     dma: dma-controller@fd4c0000 {
       compatible = "xlnx,zynqmp-dpdma";
@@ -63,6 +68,7 @@ examples:
       clocks = <&dpdma_clk>;
       clock-names = "axi_clk";
       #dma-cells = <1>;
+      power-domains = <&zynqmp_firmware PD_DP>;
     };
 
 ...
-- 
2.36.1

Re: [PATCH v3] dt-bindings: dma: xilinx: Add power-domains to xlnx,zynqmp-dpdma

[Laurent Pinchart]

Hi Michal,

Thank you for the patch.

On Fri, May 12, 2023 at 01:40:33PM +0200, Michal Simek wrote:
> DP DMA has own power domain that's why describe required power-domain
> property.
> 
> Signed-off-by: Michal Simek <michal.simek@amd.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> 
> Changes in v3:
> - make power-domains as required property
> - also update commit message
> 
> Changes in v2:
> - rewrite commit message - requested by Krzysztof
> 
> The commit b06112cd5e08 ("arm64: dts: zynqmp: Add power domain for the
> DisplayPort DMA controller") added this property already in Linux that's
> why the patch is also fixing dts_check warnings.
> 
> In v2 I got ack from Krzysztof but not adding it because of additional
> discussion about required property in v3.
> https://lore.kernel.org/r/029ba923-d13e-ea7c-018d-95e179dda2e5@linaro.org
> 
> ---
>  .../devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml   | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
> index d6cbd95ec26d..2128f4645c98 100644
> --- a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
> +++ b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
> @@ -41,6 +41,9 @@ properties:
>    clock-names:
>      const: axi_clk
>  
> +  power-domains:
> +    maxItems: 1
> +
>  required:
>    - "#dma-cells"
>    - compatible
> @@ -48,12 +51,14 @@ required:
>    - interrupts
>    - clocks
>    - clock-names
> +  - power-domains
>  
>  additionalProperties: false
>  
>  examples:
>    - |
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/power/xlnx-zynqmp-power.h>
>  
>      dma: dma-controller@fd4c0000 {
>        compatible = "xlnx,zynqmp-dpdma";
> @@ -63,6 +68,7 @@ examples:
>        clocks = <&dpdma_clk>;
>        clock-names = "axi_clk";
>        #dma-cells = <1>;
> +      power-domains = <&zynqmp_firmware PD_DP>;
>      };
>  
>  ...

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3] dt-bindings: dma: xilinx: Add power-domains to xlnx,zynqmp-dpdma

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2505 bytes --]

On Fri, May 12, 2023 at 01:40:33PM +0200, Michal Simek wrote:
> DP DMA has own power domain that's why describe required power-domain
> property.
> 
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> ---
> 
> Changes in v3:
> - make power-domains as required property
> - also update commit message
> 
> Changes in v2:
> - rewrite commit message - requested by Krzysztof
> 
> The commit b06112cd5e08 ("arm64: dts: zynqmp: Add power domain for the
> DisplayPort DMA controller") added this property already in Linux that's
> why the patch is also fixing dts_check warnings.
> 
> In v2 I got ack from Krzysztof but not adding it because of additional
> discussion about required property in v3.
> https://lore.kernel.org/r/029ba923-d13e-ea7c-018d-95e179dda2e5@linaro.org

It was not entirely clear reading that whether the driver actually
functions correctly if the property is omitted.
Is it the case that if the property is omitted, and the power domain
controller driver is enabled, that the dma controller will not operate
correctly?
If I am understanding correctly, it very much does sound like it
should be listed as required, so:
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> 
> ---
>  .../devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml   | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
> index d6cbd95ec26d..2128f4645c98 100644
> --- a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
> +++ b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml
> @@ -41,6 +41,9 @@ properties:
>    clock-names:
>      const: axi_clk
>  
> +  power-domains:
> +    maxItems: 1
> +
>  required:
>    - "#dma-cells"
>    - compatible
> @@ -48,12 +51,14 @@ required:
>    - interrupts
>    - clocks
>    - clock-names
> +  - power-domains
>  
>  additionalProperties: false
>  
>  examples:
>    - |
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/power/xlnx-zynqmp-power.h>
>  
>      dma: dma-controller@fd4c0000 {
>        compatible = "xlnx,zynqmp-dpdma";
> @@ -63,6 +68,7 @@ examples:
>        clocks = <&dpdma_clk>;
>        clock-names = "axi_clk";
>        #dma-cells = <1>;
> +      power-domains = <&zynqmp_firmware PD_DP>;
>      };
>  
>  ...
> -- 
> 2.36.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3] dt-bindings: dma: xilinx: Add power-domains to xlnx,zynqmp-dpdma

[Michal Simek]

On 5/12/23 22:40, Conor Dooley wrote:
> On Fri, May 12, 2023 at 01:40:33PM +0200, Michal Simek wrote:
>> DP DMA has own power domain that's why describe required power-domain
>> property.
>>
>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>> ---
>>
>> Changes in v3:
>> - make power-domains as required property
>> - also update commit message
>>
>> Changes in v2:
>> - rewrite commit message - requested by Krzysztof
>>
>> The commit b06112cd5e08 ("arm64: dts: zynqmp: Add power domain for the
>> DisplayPort DMA controller") added this property already in Linux that's
>> why the patch is also fixing dts_check warnings.
>>
>> In v2 I got ack from Krzysztof but not adding it because of additional
>> discussion about required property in v3.
>> https://lore.kernel.org/r/029ba923-d13e-ea7c-018d-95e179dda2e5@linaro.org
> 
> It was not entirely clear reading that whether the driver actually
> functions correctly if the property is omitted.
> Is it the case that if the property is omitted, and the power domain
> controller driver is enabled, that the dma controller will not operate
> correctly?
> If I am understanding correctly, it very much does sound like it
> should be listed as required, so:
> Acked-by: Conor Dooley <conor.dooley@microchip.com>

As was discussed there are 3 things here. One thing is if property is required 
from simple description point of view. In silicon there is own power domain for 
pretty much every IP separately.
The second if driver is using it. The third if power domains are enabled in the 
kernel configuration.

On our SOC pretty much we should look at every dt binding and make 
power-domains, smmus, etc all required because they are there (it is not said 
how exactly described but HW have them).

And SOC basic setup for 2nd/3rd point above. By default they are enabled and 
power domain driver has capability to disable it to save power.

Thanks,
Michal

Re: [PATCH v3] dt-bindings: dma: xilinx: Add power-domains to xlnx,zynqmp-dpdma

[Vinod Koul]

On 12-05-23, 13:40, Michal Simek wrote:
> DP DMA has own power domain that's why describe required power-domain
> property.

Applied, thanks

-- 
~Vinod