Adds support to handle FPGA/PL power Domain to optimize the PL power
consumption.
Changes for v2:
-Rebased the patch series on 5.18v
Nava kishore Manne (3):
dt-bindings: zynqmp: Add new PD_PL macro
fpga: region: Add fpga-region 'power-domains' property
fpga: region: Add runtime PM support
.../devicetree/bindings/fpga/fpga-region.txt | 14 ++++++++++++++
drivers/fpga/of-fpga-region.c | 14 ++++++++++++++
include/dt-bindings/power/xlnx-zynqmp-power.h | 1 +
3 files changed, 29 insertions(+)
--
2.25.1
Add support to handle FPGA/PL power domain. With this patch,
the PL power domain will be turned on before loading the bitstream
into the targeted region and turned off while removing/unloading the
bitstream from the targeted region using overlays. This can be achieved
by adding the runtime PM support to the fpga regions.
Signed-off-by: Nava kishore Manne <[email protected]>
---
Changes for v2:
- Updated commit message.
- Updated runtime PM handling logic to fix the PM ref count
imbalance issues.
drivers/fpga/of-fpga-region.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
index ae82532fc127..f14bb5916d97 100644
--- a/drivers/fpga/of-fpga-region.c
+++ b/drivers/fpga/of-fpga-region.c
@@ -15,6 +15,7 @@
#include <linux/of_platform.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
+#include <linux/pm_runtime.h>
static const struct of_device_id fpga_region_of_match[] = {
{ .compatible = "fpga-region", },
@@ -301,10 +302,17 @@ static int of_fpga_region_notify_pre_apply(struct fpga_region *region,
return -EINVAL;
}
+ ret = pm_runtime_resume_and_get(dev->parent);
+ if (ret < 0) {
+ fpga_image_info_free(info);
+ return ret;
+ }
+
region->info = info;
ret = fpga_region_program_fpga(region);
if (ret) {
/* error; reject overlay */
+ pm_runtime_put_sync(dev->parent);
fpga_image_info_free(info);
region->info = NULL;
}
@@ -324,10 +332,13 @@ static int of_fpga_region_notify_pre_apply(struct fpga_region *region,
static void of_fpga_region_notify_post_remove(struct fpga_region *region,
struct of_overlay_notify_data *nd)
{
+ struct device *dev = ®ion->dev;
+
fpga_bridges_disable(®ion->bridge_list);
fpga_bridges_put(®ion->bridge_list);
fpga_image_info_free(region->info);
region->info = NULL;
+ pm_runtime_put_sync(dev->parent);
}
/**
@@ -411,6 +422,8 @@ static int of_fpga_region_probe(struct platform_device *pdev)
goto eprobe_mgr_put;
}
+ pm_runtime_enable(&pdev->dev);
+
of_platform_populate(np, fpga_region_of_match, NULL, ®ion->dev);
platform_set_drvdata(pdev, region);
@@ -430,6 +443,7 @@ static int of_fpga_region_remove(struct platform_device *pdev)
fpga_region_unregister(region);
fpga_mgr_put(mgr);
+ pm_runtime_disable(region->dev.parent);
return 0;
}
--
2.25.1
Add fpga-region 'power-domains' property to allow to handle
the FPGA/PL power domains.
Signed-off-by: Nava kishore Manne <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Changes for v2:
- Updated power-domains description.
.../devicetree/bindings/fpga/fpga-region.txt | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt b/Documentation/devicetree/bindings/fpga/fpga-region.txt
index 7d3515264838..f299c3749505 100644
--- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
+++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
@@ -196,6 +196,20 @@ Optional properties:
- config-complete-timeout-us : The maximum time in microseconds time for the
FPGA to go to operating mode after the region has been programmed.
- child nodes : devices in the FPGA after programming.
+- power-domains : A phandle and power domain specifier pair to the power domain
+ which is responsible for turning on/off the power to the FPGA/PL region.
+Example:
+ fpga_full: fpga-full {
+ compatible = "fpga-region";
+ fpga-mgr = <&zynqmp_pcap>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+ power-domains = <&zynqmp_firmware PL_PD>;
+ };
+
+ The PL_PD power domain will be turned on before loading the bitstream
+and turned off while removing/unloading the bitstream using overlays.
In the example below, when an overlay is applied targeting fpga-region0,
fpga_mgr is used to program the FPGA. Two bridges are controlled during
--
2.25.1
Add new power domain for PL region.
Signed-off-by: Nava kishore Manne <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Changes for v2:
- None.
include/dt-bindings/power/xlnx-zynqmp-power.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/dt-bindings/power/xlnx-zynqmp-power.h b/include/dt-bindings/power/xlnx-zynqmp-power.h
index 0d9a412fd5e0..5f718759c38f 100644
--- a/include/dt-bindings/power/xlnx-zynqmp-power.h
+++ b/include/dt-bindings/power/xlnx-zynqmp-power.h
@@ -35,5 +35,6 @@
#define PD_CAN_1 48
#define PD_GPU 58
#define PD_PCIE 59
+#define PD_PL 69
#endif
--
2.25.1
Ping!
> -----Original Message-----
> From: Nava kishore Manne <[email protected]>
> Sent: Monday, May 23, 2022 7:15 PM
> To: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Michal Simek <[email protected]>; Nava kishore Manne
> <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-arm-
> [email protected]; git <[email protected]>
> Subject: [PATCH v2 3/3] fpga: region: Add runtime PM support
>
> Add support to handle FPGA/PL power domain. With this patch, the PL
> power domain will be turned on before loading the bitstream into the
> targeted region and turned off while removing/unloading the bitstream from
> the targeted region using overlays. This can be achieved by adding the
> runtime PM support to the fpga regions.
>
> Signed-off-by: Nava kishore Manne <[email protected]>
> ---
> Changes for v2:
> - Updated commit message.
> - Updated runtime PM handling logic to fix the PM ref count
> imbalance issues.
>
> drivers/fpga/of-fpga-region.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
> index ae82532fc127..f14bb5916d97 100644
> --- a/drivers/fpga/of-fpga-region.c
> +++ b/drivers/fpga/of-fpga-region.c
> @@ -15,6 +15,7 @@
> #include <linux/of_platform.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> +#include <linux/pm_runtime.h>
>
> static const struct of_device_id fpga_region_of_match[] = {
> { .compatible = "fpga-region", },
> @@ -301,10 +302,17 @@ static int of_fpga_region_notify_pre_apply(struct
> fpga_region *region,
> return -EINVAL;
> }
>
> + ret = pm_runtime_resume_and_get(dev->parent);
> + if (ret < 0) {
> + fpga_image_info_free(info);
> + return ret;
> + }
> +
> region->info = info;
> ret = fpga_region_program_fpga(region);
> if (ret) {
> /* error; reject overlay */
> + pm_runtime_put_sync(dev->parent);
> fpga_image_info_free(info);
> region->info = NULL;
> }
> @@ -324,10 +332,13 @@ static int of_fpga_region_notify_pre_apply(struct
> fpga_region *region, static void of_fpga_region_notify_post_remove(struct
> fpga_region *region,
> struct of_overlay_notify_data
> *nd) {
> + struct device *dev = ®ion->dev;
> +
> fpga_bridges_disable(®ion->bridge_list);
> fpga_bridges_put(®ion->bridge_list);
> fpga_image_info_free(region->info);
> region->info = NULL;
> + pm_runtime_put_sync(dev->parent);
> }
>
> /**
> @@ -411,6 +422,8 @@ static int of_fpga_region_probe(struct
> platform_device *pdev)
> goto eprobe_mgr_put;
> }
>
> + pm_runtime_enable(&pdev->dev);
> +
> of_platform_populate(np, fpga_region_of_match, NULL, ®ion-
> >dev);
> platform_set_drvdata(pdev, region);
>
> @@ -430,6 +443,7 @@ static int of_fpga_region_remove(struct
> platform_device *pdev)
>
> fpga_region_unregister(region);
> fpga_mgr_put(mgr);
> + pm_runtime_disable(region->dev.parent);
>
> return 0;
> }
> --
> 2.25.1
On Mon, Jun 20, 2022 at 05:38:13AM +0000, Manne, Nava kishore wrote:
> Ping!
Sorry I missed this one. Will try to catch up.
Thanks,
Yilun
>
> > -----Original Message-----
> > From: Nava kishore Manne <[email protected]>
> > Sent: Monday, May 23, 2022 7:15 PM
> > To: [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > Michal Simek <[email protected]>; Nava kishore Manne
> > <[email protected]>; [email protected];
> > [email protected]; [email protected]; linux-arm-
> > [email protected]; git <[email protected]>
> > Subject: [PATCH v2 3/3] fpga: region: Add runtime PM support
> >
> > Add support to handle FPGA/PL power domain. With this patch, the PL
> > power domain will be turned on before loading the bitstream into the
> > targeted region and turned off while removing/unloading the bitstream from
> > the targeted region using overlays. This can be achieved by adding the
> > runtime PM support to the fpga regions.
> >
> > Signed-off-by: Nava kishore Manne <[email protected]>
> > ---
> > Changes for v2:
> > - Updated commit message.
> > - Updated runtime PM handling logic to fix the PM ref count
> > imbalance issues.
> >
> > drivers/fpga/of-fpga-region.c | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/fpga/of-fpga-region.c b/drivers/fpga/of-fpga-region.c
> > index ae82532fc127..f14bb5916d97 100644
> > --- a/drivers/fpga/of-fpga-region.c
> > +++ b/drivers/fpga/of-fpga-region.c
> > @@ -15,6 +15,7 @@
> > #include <linux/of_platform.h>
> > #include <linux/slab.h>
> > #include <linux/spinlock.h>
> > +#include <linux/pm_runtime.h>
> >
> > static const struct of_device_id fpga_region_of_match[] = {
> > { .compatible = "fpga-region", },
> > @@ -301,10 +302,17 @@ static int of_fpga_region_notify_pre_apply(struct
> > fpga_region *region,
> > return -EINVAL;
> > }
> >
> > + ret = pm_runtime_resume_and_get(dev->parent);
> > + if (ret < 0) {
> > + fpga_image_info_free(info);
> > + return ret;
> > + }
> > +
> > region->info = info;
> > ret = fpga_region_program_fpga(region);
> > if (ret) {
> > /* error; reject overlay */
> > + pm_runtime_put_sync(dev->parent);
> > fpga_image_info_free(info);
> > region->info = NULL;
> > }
> > @@ -324,10 +332,13 @@ static int of_fpga_region_notify_pre_apply(struct
> > fpga_region *region, static void of_fpga_region_notify_post_remove(struct
> > fpga_region *region,
> > struct of_overlay_notify_data
> > *nd) {
> > + struct device *dev = ®ion->dev;
> > +
> > fpga_bridges_disable(®ion->bridge_list);
> > fpga_bridges_put(®ion->bridge_list);
> > fpga_image_info_free(region->info);
> > region->info = NULL;
> > + pm_runtime_put_sync(dev->parent);
> > }
> >
> > /**
> > @@ -411,6 +422,8 @@ static int of_fpga_region_probe(struct
> > platform_device *pdev)
> > goto eprobe_mgr_put;
> > }
> >
> > + pm_runtime_enable(&pdev->dev);
> > +
> > of_platform_populate(np, fpga_region_of_match, NULL, ®ion-
> > >dev);
> > platform_set_drvdata(pdev, region);
> >
> > @@ -430,6 +443,7 @@ static int of_fpga_region_remove(struct
> > platform_device *pdev)
> >
> > fpga_region_unregister(region);
> > fpga_mgr_put(mgr);
> > + pm_runtime_disable(region->dev.parent);
> >
> > return 0;
> > }
> > --
> > 2.25.1
On Mon, May 23, 2022 at 07:15:16PM +0530, Nava kishore Manne wrote:
> Add fpga-region 'power-domains' property to allow to handle
> the FPGA/PL power domains.
>
> Signed-off-by: Nava kishore Manne <[email protected]>
> Acked-by: Rob Herring <[email protected]>
> ---
> Changes for v2:
> - Updated power-domains description.
>
> .../devicetree/bindings/fpga/fpga-region.txt | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> index 7d3515264838..f299c3749505 100644
> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> @@ -196,6 +196,20 @@ Optional properties:
> - config-complete-timeout-us : The maximum time in microseconds time for the
> FPGA to go to operating mode after the region has been programmed.
> - child nodes : devices in the FPGA after programming.
> +- power-domains : A phandle and power domain specifier pair to the power domain
> + which is responsible for turning on/off the power to the FPGA/PL region.
Could you help explain what is PL?
> +Example:
> + fpga_full: fpga-full {
> + compatible = "fpga-region";
> + fpga-mgr = <&zynqmp_pcap>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + power-domains = <&zynqmp_firmware PL_PD>;
> + };
> +
> + The PL_PD power domain will be turned on before loading the bitstream
> +and turned off while removing/unloading the bitstream using overlays.
I think the single power-domain may not cover some use cases that
of-fpga-region driver supports. It is possible there are already
devices in fpga-region for static OF tree, or an overlay with no
'firmware-name' but 'external-fpga-config'. In these cases power domains
may still be needed, is it?
Another case is the fpga-region may need multiple power domains?
Since the of-fpga-region driver is a generic fpga-region driver, we may
investigate more for a compatible power-domain solution.
Thanks,
Yilun
>
> In the example below, when an overlay is applied targeting fpga-region0,
> fpga_mgr is used to program the FPGA. Two bridges are controlled during
> --
> 2.25.1
Hi Yilun,
Please find my response inline.
> -----Original Message-----
> From: Xu Yilun <[email protected]>
> Sent: Friday, June 24, 2022 9:58 PM
> To: Nava kishore Manne <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-arm-
> [email protected]; [email protected]; Rob Herring <[email protected]>
> Subject: Re: [PATCH v2 2/3] fpga: region: Add fpga-region 'power-domains'
> property
>
> CAUTION: This message has originated from an External Source. Please use
> proper judgment and caution when opening attachments, clicking links, or
> responding to this email.
>
>
> On Mon, May 23, 2022 at 07:15:16PM +0530, Nava kishore Manne wrote:
> > Add fpga-region 'power-domains' property to allow to handle the
> > FPGA/PL power domains.
> >
> > Signed-off-by: Nava kishore Manne <[email protected]>
> > Acked-by: Rob Herring <[email protected]>
> > ---
> > Changes for v2:
> > - Updated power-domains description.
> >
> > .../devicetree/bindings/fpga/fpga-region.txt | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > index 7d3515264838..f299c3749505 100644
> > --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > @@ -196,6 +196,20 @@ Optional properties:
> > - config-complete-timeout-us : The maximum time in microseconds time
> for the
> > FPGA to go to operating mode after the region has been programmed.
> > - child nodes : devices in the FPGA after programming.
> > +- power-domains : A phandle and power domain specifier pair to the
> power domain
> > + which is responsible for turning on/off the power to the FPGA/PL
> region.
>
> Could you help explain what is PL?
>
> > +Example:
> > + fpga_full: fpga-full {
> > + compatible = "fpga-region";
> > + fpga-mgr = <&zynqmp_pcap>;
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
> > + power-domains = <&zynqmp_firmware PL_PD>;
> > + };
> > +
> > + The PL_PD power domain will be turned on before loading the
> > +bitstream and turned off while removing/unloading the bitstream using
> overlays.
>
> I think the single power-domain may not cover some use cases that of-fpga-
> region driver supports.
I am not sure which use case you are talking about. Can you please point me the exact use case here?
> It is possible there are already devices in fpga-region
> for static OF tree, or an overlay with no 'firmware-name' but 'external-fpga-
> config'. In these cases power domains may still be needed, is it?
>
It's an optional property user can decide whether he needs this support or not for 'external-fpga-config
Use case.
> Another case is the fpga-region may need multiple power domains?
>
In our use case full region and relevant partial regions have different power domains and
this patch is capable of handle different power domain regions (full and partial regions)
> Since the of-fpga-region driver is a generic fpga-region driver, we may
> investigate more for a compatible power-domain solution.
>
Please share your thoughts here we will try to align with it.
Regards,
Navakishore.
On Mon, Jul 18, 2022 at 06:46:32AM +0000, Manne, Nava kishore wrote:
> Hi Yilun,
>
> Please find my response inline.
>
> > -----Original Message-----
> > From: Xu Yilun <[email protected]>
> > Sent: Friday, June 24, 2022 9:58 PM
> > To: Nava kishore Manne <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; linux-arm-
> > [email protected]; [email protected]; Rob Herring <[email protected]>
> > Subject: Re: [PATCH v2 2/3] fpga: region: Add fpga-region 'power-domains'
> > property
> >
> > CAUTION: This message has originated from an External Source. Please use
> > proper judgment and caution when opening attachments, clicking links, or
> > responding to this email.
> >
> >
> > On Mon, May 23, 2022 at 07:15:16PM +0530, Nava kishore Manne wrote:
> > > Add fpga-region 'power-domains' property to allow to handle the
> > > FPGA/PL power domains.
> > >
> > > Signed-off-by: Nava kishore Manne <[email protected]>
> > > Acked-by: Rob Herring <[email protected]>
> > > ---
> > > Changes for v2:
> > > - Updated power-domains description.
> > >
> > > .../devicetree/bindings/fpga/fpga-region.txt | 14 ++++++++++++++
> > > 1 file changed, 14 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > index 7d3515264838..f299c3749505 100644
> > > --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > @@ -196,6 +196,20 @@ Optional properties:
> > > - config-complete-timeout-us : The maximum time in microseconds time
> > for the
> > > FPGA to go to operating mode after the region has been programmed.
> > > - child nodes : devices in the FPGA after programming.
> > > +- power-domains : A phandle and power domain specifier pair to the
> > power domain
> > > + which is responsible for turning on/off the power to the FPGA/PL
> > region.
> >
> > Could you help explain what is PL?
> >
> > > +Example:
> > > + fpga_full: fpga-full {
> > > + compatible = "fpga-region";
> > > + fpga-mgr = <&zynqmp_pcap>;
> > > + #address-cells = <2>;
> > > + #size-cells = <2>;
> > > + ranges;
> > > + power-domains = <&zynqmp_firmware PL_PD>;
> > > + };
> > > +
> > > + The PL_PD power domain will be turned on before loading the
> > > +bitstream and turned off while removing/unloading the bitstream using
> > overlays.
> >
> > I think the single power-domain may not cover some use cases that of-fpga-
> > region driver supports.
>
> I am not sure which use case you are talking about. Can you please point me the exact use case here?
>
> > It is possible there are already devices in fpga-region
> > for static OF tree, or an overlay with no 'firmware-name' but 'external-fpga-
> > config'. In these cases power domains may still be needed, is it?
> >
>
> It's an optional property user can decide whether he needs this support or not for 'external-fpga-config
> Use case.
If an external-fpga-config FPGA region needs to enable a power domain before
sub devices population, how could it config the DT? I assume in this
patch "power-domains" property is only used before & after
reconfiguration but external-fpga-config FPGA region may need no
reconfiguration.
>
> > Another case is the fpga-region may need multiple power domains?
> >
>
> In our use case full region and relevant partial regions have different power domains and
> this patch is capable of handle different power domain regions (full and partial regions)
If a FPGA region needs 2 or more power domains for partial reconfiguration,
how could we find out and enable them all?
From the 2 cases, I see as a generic driver, there may be need to enable
different power domains at different moments. And I'm afraid a simple
implementation of pm_runtime_get before reconfiguration may limit the
usage of "power-domains" property for of-fpga-region.
Thanks,
Yilun
>
> > Since the of-fpga-region driver is a generic fpga-region driver, we may
> > investigate more for a compatible power-domain solution.
> >
>
> Please share your thoughts here we will try to align with it.
>
> Regards,
> Navakishore.
On 23/05/2022 15:45, Nava kishore Manne wrote:
> Add fpga-region 'power-domains' property to allow to handle
> the FPGA/PL power domains.
>
> Signed-off-by: Nava kishore Manne <[email protected]>
> Acked-by: Rob Herring <[email protected]>
> ---
> Changes for v2:
> - Updated power-domains description.
>
> .../devicetree/bindings/fpga/fpga-region.txt | 14 ++++++++++++++
Please use subject matching the file, so you have to include dt-bindings
as one of the prefixes.
Best regards,
Krzysztof
Hi Yilun,
Please find my response inline.
> -----Original Message-----
> From: Xu Yilun <[email protected]>
> Sent: Thursday, July 21, 2022 12:25 PM
> To: Manne, Nava kishore <[email protected]>
> Cc: Nava kishore Manne <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; Rob Herring <[email protected]>
> Subject: Re: [PATCH v2 2/3] fpga: region: Add fpga-region 'power-domains'
> property
>
> On Mon, Jul 18, 2022 at 06:46:32AM +0000, Manne, Nava kishore wrote:
> > Hi Yilun,
> >
> > Please find my response inline.
> >
> > > -----Original Message-----
> > > From: Xu Yilun <[email protected]>
> > > Sent: Friday, June 24, 2022 9:58 PM
> > > To: Nava kishore Manne <[email protected]>
> > > Cc: [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected]; linux-arm-
> > > [email protected]; [email protected]; Rob Herring
> > > <[email protected]>
> > > Subject: Re: [PATCH v2 2/3] fpga: region: Add fpga-region 'power-
> domains'
> > > property
> > >
> > > CAUTION: This message has originated from an External Source. Please
> > > use proper judgment and caution when opening attachments, clicking
> > > links, or responding to this email.
> > >
> > >
> > > On Mon, May 23, 2022 at 07:15:16PM +0530, Nava kishore Manne wrote:
> > > > Add fpga-region 'power-domains' property to allow to handle the
> > > > FPGA/PL power domains.
> > > >
> > > > Signed-off-by: Nava kishore Manne <[email protected]>
> > > > Acked-by: Rob Herring <[email protected]>
> > > > ---
> > > > Changes for v2:
> > > > - Updated power-domains description.
> > > >
> > > > .../devicetree/bindings/fpga/fpga-region.txt | 14 ++++++++++++++
> > > > 1 file changed, 14 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > index 7d3515264838..f299c3749505 100644
> > > > --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > @@ -196,6 +196,20 @@ Optional properties:
> > > > - config-complete-timeout-us : The maximum time in microseconds
> > > > time
> > > for the
> > > > FPGA to go to operating mode after the region has been
> programmed.
> > > > - child nodes : devices in the FPGA after programming.
> > > > +- power-domains : A phandle and power domain specifier pair to
> > > > +the
> > > power domain
> > > > + which is responsible for turning on/off the power to the
> > > > + FPGA/PL
> > > region.
> > >
> > > Could you help explain what is PL?
> > >
> > > > +Example:
> > > > + fpga_full: fpga-full {
> > > > + compatible = "fpga-region";
> > > > + fpga-mgr = <&zynqmp_pcap>;
> > > > + #address-cells = <2>;
> > > > + #size-cells = <2>;
> > > > + ranges;
> > > > + power-domains = <&zynqmp_firmware PL_PD>;
> > > > + };
> > > > +
> > > > + The PL_PD power domain will be turned on before loading the
> > > > +bitstream and turned off while removing/unloading the bitstream
> > > > +using
> > > overlays.
> > >
> > > I think the single power-domain may not cover some use cases that
> > > of-fpga- region driver supports.
> >
> > I am not sure which use case you are talking about. Can you please point
> me the exact use case here?
> >
> > > It is possible there are already devices in fpga-region for static
> > > OF tree, or an overlay with no 'firmware-name' but 'external-fpga-
> > > config'. In these cases power domains may still be needed, is it?
> > >
> >
> > It's an optional property user can decide whether he needs this
> > support or not for 'external-fpga-config Use case.
>
> If an external-fpga-config FPGA region needs to enable a power domain
> before sub devices population, how could it config the DT? I assume in this
> patch "power-domains" property is only used before & after reconfiguration
> but external-fpga-config FPGA region may need no reconfiguration.
>
I agree, For 'external-fpga-config' use case this patch is not capable of supporting the dynamic PM.
I Will explore and get back to you to handle this use case.
> >
> > > Another case is the fpga-region may need multiple power domains?
> > >
> >
> > In our use case full region and relevant partial regions have
> > different power domains and this patch is capable of handle different
> > power domain regions (full and partial regions)
>
> If a FPGA region needs 2 or more power domains for partial reconfiguration,
> how could we find out and enable them all?
>
> From the 2 cases, I see as a generic driver, there may be need to enable
> different power domains at different moments. And I'm afraid a simple
> implementation of pm_runtime_get before reconfiguration may limit the
> usage of "power-domains" property for of-fpga-region.
>
Can you please point me any references to handle the multiple power domains?
This implementation is region specific which means it can handle both full and partial power domains
Independently based on the power domains info exists in the DT.
For example:
Base DT: For full
fpga_full: fpga-full {
compatible = "fpga-region";
fpga-mgr = <&zynqmp_pcap>;
#address-cells = <2>;
#size-cells = <2>;
ranges;
power-domains = <&zynqmp_firmware PL_PD>;
};
Overlay DT: For Partial
fragment@0 {
target = <&fpga_full>;
#address-cells = <2>;
#size-cells = <2>;
__overlay__ {
#address-cells = <2>;
#size-cells = <2>;
firmware-name = "full.bin";
fpga_PR0: fpga-PR0 {
compatible = "fpga-region";
#address-cells = <2>;
#size-cells = <2>;
ranges;
power-domains = <&zynqmp_firmware PL_PR_PD>;
};
};
};
Regards,
Navakishore.
On 2022-08-16 at 10:22:28 +0000, Manne, Nava kishore wrote:
> Hi Yilun,
>
> Please find my response inline.
>
> > -----Original Message-----
> > From: Xu Yilun <[email protected]>
> > Sent: Thursday, July 21, 2022 12:25 PM
> > To: Manne, Nava kishore <[email protected]>
> > Cc: Nava kishore Manne <[email protected]>; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; linux-
> > [email protected]; [email protected];
> > [email protected]; Rob Herring <[email protected]>
> > Subject: Re: [PATCH v2 2/3] fpga: region: Add fpga-region 'power-domains'
> > property
> >
> > On Mon, Jul 18, 2022 at 06:46:32AM +0000, Manne, Nava kishore wrote:
> > > Hi Yilun,
> > >
> > > Please find my response inline.
> > >
> > > > -----Original Message-----
> > > > From: Xu Yilun <[email protected]>
> > > > Sent: Friday, June 24, 2022 9:58 PM
> > > > To: Nava kishore Manne <[email protected]>
> > > > Cc: [email protected]; [email protected]; [email protected];
> > > > [email protected]; [email protected];
> > > > [email protected]; [email protected];
> > > > [email protected]; [email protected]; linux-arm-
> > > > [email protected]; [email protected]; Rob Herring
> > > > <[email protected]>
> > > > Subject: Re: [PATCH v2 2/3] fpga: region: Add fpga-region 'power-
> > domains'
> > > > property
> > > >
> > > > CAUTION: This message has originated from an External Source. Please
> > > > use proper judgment and caution when opening attachments, clicking
> > > > links, or responding to this email.
> > > >
> > > >
> > > > On Mon, May 23, 2022 at 07:15:16PM +0530, Nava kishore Manne wrote:
> > > > > Add fpga-region 'power-domains' property to allow to handle the
> > > > > FPGA/PL power domains.
> > > > >
> > > > > Signed-off-by: Nava kishore Manne <[email protected]>
> > > > > Acked-by: Rob Herring <[email protected]>
> > > > > ---
> > > > > Changes for v2:
> > > > > - Updated power-domains description.
> > > > >
> > > > > .../devicetree/bindings/fpga/fpga-region.txt | 14 ++++++++++++++
> > > > > 1 file changed, 14 insertions(+)
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > > b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > > index 7d3515264838..f299c3749505 100644
> > > > > --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > > +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > > @@ -196,6 +196,20 @@ Optional properties:
> > > > > - config-complete-timeout-us : The maximum time in microseconds
> > > > > time
> > > > for the
> > > > > FPGA to go to operating mode after the region has been
> > programmed.
> > > > > - child nodes : devices in the FPGA after programming.
> > > > > +- power-domains : A phandle and power domain specifier pair to
> > > > > +the
> > > > power domain
> > > > > + which is responsible for turning on/off the power to the
> > > > > + FPGA/PL
> > > > region.
> > > >
> > > > Could you help explain what is PL?
> > > >
> > > > > +Example:
> > > > > + fpga_full: fpga-full {
> > > > > + compatible = "fpga-region";
> > > > > + fpga-mgr = <&zynqmp_pcap>;
> > > > > + #address-cells = <2>;
> > > > > + #size-cells = <2>;
> > > > > + ranges;
> > > > > + power-domains = <&zynqmp_firmware PL_PD>;
> > > > > + };
> > > > > +
> > > > > + The PL_PD power domain will be turned on before loading the
> > > > > +bitstream and turned off while removing/unloading the bitstream
> > > > > +using
> > > > overlays.
> > > >
> > > > I think the single power-domain may not cover some use cases that
> > > > of-fpga- region driver supports.
> > >
> > > I am not sure which use case you are talking about. Can you please point
> > me the exact use case here?
> > >
> > > > It is possible there are already devices in fpga-region for static
> > > > OF tree, or an overlay with no 'firmware-name' but 'external-fpga-
> > > > config'. In these cases power domains may still be needed, is it?
> > > >
> > >
> > > It's an optional property user can decide whether he needs this
> > > support or not for 'external-fpga-config Use case.
> >
> > If an external-fpga-config FPGA region needs to enable a power domain
> > before sub devices population, how could it config the DT? I assume in this
> > patch "power-domains" property is only used before & after reconfiguration
> > but external-fpga-config FPGA region may need no reconfiguration.
> >
> I agree, For 'external-fpga-config' use case this patch is not capable of supporting the dynamic PM.
> I Will explore and get back to you to handle this use case.
>
> > >
> > > > Another case is the fpga-region may need multiple power domains?
> > > >
> > >
> > > In our use case full region and relevant partial regions have
> > > different power domains and this patch is capable of handle different
> > > power domain regions (full and partial regions)
> >
> > If a FPGA region needs 2 or more power domains for partial reconfiguration,
> > how could we find out and enable them all?
> >
> > From the 2 cases, I see as a generic driver, there may be need to enable
> > different power domains at different moments. And I'm afraid a simple
> > implementation of pm_runtime_get before reconfiguration may limit the
> > usage of "power-domains" property for of-fpga-region.
> >
>
> Can you please point me any references to handle the multiple power domains?
Some devices may need multiple domains to work properly, for example:
power-domains = <&pd_disp>, <&pd_pci>;
power-domain-names = "pcie", "pcie_phy";
I'm not sure if there is need for some fpga region, but think it could be
a valid usecase.
Thanks,
Yilun
>
> This implementation is region specific which means it can handle both full and partial power domains
> Independently based on the power domains info exists in the DT.
> For example:
>
> Base DT: For full
> fpga_full: fpga-full {
> compatible = "fpga-region";
> fpga-mgr = <&zynqmp_pcap>;
> #address-cells = <2>;
> #size-cells = <2>;
> ranges;
> power-domains = <&zynqmp_firmware PL_PD>;
> };
>
> Overlay DT: For Partial
> fragment@0 {
> target = <&fpga_full>;
> #address-cells = <2>;
> #size-cells = <2>;
> __overlay__ {
> #address-cells = <2>;
> #size-cells = <2>;
> firmware-name = "full.bin";
> fpga_PR0: fpga-PR0 {
> compatible = "fpga-region";
> #address-cells = <2>;
> #size-cells = <2>;
> ranges;
> power-domains = <&zynqmp_firmware PL_PR_PD>;
> };
> };
> };
>
> Regards,
> Navakishore.