[PATCH 0/3] pmdomain: ti_sci: collect and send low-power mode constraints

[PATCH 0/3] pmdomain: ti_sci: collect and send low-power mode constraints

[Kevin Hilman]

The latest (10.x) version of the firmware for the PM co-processor (aka
device manager, or DM) adds support for a "managed" mode, where the DM
firmware will select the specific low power state which is entered
when Linux requests a system-wide suspend.

In this mode, the DM will always attempt the deepest low-power state
available for the SoC.

However, Linux (or OSes running on other cores) may want to constrain
the DM for certain use cases.  For example, the deepest state may have
a wakeup/resume latency that is too long for certain use cases.  Or,
some wakeup-capable devices may potentially be powered off in deep
low-power states, but if one of those devices is enabled as a wakeup
source, it should not be powered off.

These kinds of constraints are are already known in Linux by the use
of existing APIs such as per-device PM QoS and device wakeup APIs, but
now we need to communicate these constraints to the DM.

For TI SoCs with TI SCI support, all DM-managed devices will be
connected to a TI SCI PM domain.  So the goal of this series is to use
the PM domain driver for TI SCI devices to collect constraints, and
communicate them to the DM via the new TI SCI APIs.

This is all managed by TI SCI PM domain code.  No new APIs are needed
by Linux drivers.  Any device that is managed by TI SCI will be
checked for QoS constraints or wakeup capability and the constraints
will be collected and sent to the DM.

This series depends on the support for the new TI SCI APIs here:
https://lore.kernel.org/r/20240801195422.2296347-1-msp@baylibre.com

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
Kevin Hilman (3):
      pmdomain: ti_sci: add per-device latency constraint management
      pmdomain: ti_sci: add wakeup constraint management
      pmdomain: ti_sci: handle wake IRQs for IO daisy chain wakeups

 drivers/pmdomain/ti/ti_sci_pm_domains.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 123 insertions(+), 1 deletion(-)
---
base-commit: ad7eb1b6b92ee0c959a0a6ae846ddadd7a79ea64
change-id: 20240802-lpm-v6-10-constraints-pmdomain-f33df5aef449
prerequisite-message-id: <20240801195422.2296347-1-msp@baylibre.com>
prerequisite-patch-id: 57f574b3f686c62cc0e255f52b2b8b96cd8b1661
prerequisite-patch-id: 0dc60150f10bbd6e0d7ae19001075b859d7af2f1
prerequisite-patch-id: f4b971122cc54df199c0d95a6c4ed47db1dae027

Best regards,
-- 
Kevin Hilman <khilman@baylibre.com>

[PATCH 1/3] pmdomain: ti_sci: add per-device latency constraint management

[Kevin Hilman]

For each device in a TI SCI PM domain, check whether the device has
any resume latency constraints set via per-device PM QoS.  If
constraints are set, send them to DM via the new SCI constraints API.

Checking for constraints happen:

1) before SCI PM domain power off (->power_off() hook)
2) before system-wide suspend (via ->suspend() hook)

For TI SCI devices that are runtime PM enabled, check (1) will be the
primary method, and will happen when the TI SCI PM domain is powered
off (e.g. when the runtime PM usecount of the last device in that
domain goes to zero.)

For devices that are either not runtime PM enabled, or are not yet
runtime suspended (e.g. due to being used during the suspend path),
the constraints check will happen by check(2).

Since constraints can be sent by either (1) or (2), driver keeps track
of whether a valid constraint has been sent already.

An important detail here is that the PM domain driver inserts itself
into the path of both the ->suspend() and ->resume() hook path
of *all* devices in the PM domain.  This allows generic PM domain code
to handle the constraint management and communication with TI SCI.

Further, this allows device drivers to use existing PM QoS APIs to
add/update constraints.

DM firmware clears constraints during its resume, so Linux has
to check/update/send constraints each time system suspends.

Co-developed-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
Signed-off-by: Dhruva Gole <d-gole@ti.com>
---
 drivers/pmdomain/ti/ti_sci_pm_domains.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 91 insertions(+), 1 deletion(-)

diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
index 1510d5ddae3d..4dc48a97f9b8 100644
--- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
+++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
@@ -13,6 +13,8 @@
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
 #include <linux/slab.h>
+#include <linux/pm_qos.h>
+#include <linux/pm_runtime.h>
 #include <linux/soc/ti/ti_sci_protocol.h>
 #include <dt-bindings/soc/ti,sci_pm_domain.h>
 
@@ -47,10 +49,46 @@ struct ti_sci_pm_domain {
 	struct generic_pm_domain pd;
 	struct list_head node;
 	struct ti_sci_genpd_provider *parent;
+	s32 lat_constraint;
+	bool constraint_sent;
 };
 
 #define genpd_to_ti_sci_pd(gpd) container_of(gpd, struct ti_sci_pm_domain, pd)
 
+static inline bool ti_sci_pd_is_valid_constraint(s32 val)
+{
+	return val != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
+}
+
+static int ti_sci_pd_send_constraint(struct device *dev, s32 val)
+{
+	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
+	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
+	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
+	int ret;
+
+	ret = ti_sci->ops.pm_ops.set_latency_constraint(ti_sci, val, TISCI_MSG_CONSTRAINT_SET);
+	if (!ret) {
+		pd->constraint_sent = true;
+		dev_dbg(dev, "ti_sci_pd: ID:%d set latency constraint %d\n",
+			pd->idx, val);
+	} else {
+		dev_err(dev, "ti_sci_pd: set latency constraint failed: ret=%d\n",
+			ret);
+	}
+
+	return ret;
+}
+
+static inline void ti_sci_pd_clear_constraints(struct device *dev)
+{
+	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
+	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
+
+	pd->lat_constraint = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
+	pd->constraint_sent = false;
+}
+
 /*
  * ti_sci_pd_power_off(): genpd power down hook
  * @domain: pointer to the powerdomain to power off
@@ -59,6 +97,18 @@ static int ti_sci_pd_power_off(struct generic_pm_domain *domain)
 {
 	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(domain);
 	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
+	struct pm_domain_data *pdd;
+
+	list_for_each_entry(pdd, &domain->dev_list, list_node) {
+		struct device *dev = pdd->dev;
+		s32 val;
+
+		/* If device has any resume latency constraints, send 'em */
+		val = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
+		if (ti_sci_pd_is_valid_constraint(val) && !pd->constraint_sent)
+			ti_sci_pd_send_constraint(dev, val);
+		pd->lat_constraint = val;
+	}
 
 	return ti_sci->ops.dev_ops.put_device(ti_sci, pd->idx);
 }
@@ -79,6 +129,38 @@ static int ti_sci_pd_power_on(struct generic_pm_domain *domain)
 		return ti_sci->ops.dev_ops.get_device(ti_sci, pd->idx);
 }
 
+static int ti_sci_pd_resume(struct device *dev)
+{
+	ti_sci_pd_clear_constraints(dev);
+	return pm_generic_resume(dev);
+}
+
+static int ti_sci_pd_suspend(struct device *dev)
+{
+	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
+	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
+	s32 val;
+	int ret;
+
+	ret = pm_generic_suspend(dev);
+	if (ret)
+		return ret;
+
+	/* Check if device has any resume latency constraints */
+	val = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
+	if (ti_sci_pd_is_valid_constraint(val) && !pd->constraint_sent) {
+		if (genpd && genpd->status == GENPD_STATE_OFF)
+			dev_warn(dev, "%s: %s: already off.\n", genpd->name, __func__);
+		else if (pm_runtime_suspended(dev))
+			dev_warn(dev, "%s: %s: already RPM suspended.\n", genpd->name, __func__);
+		else
+			ti_sci_pd_send_constraint(dev, val);
+	}
+	pd->lat_constraint = val;
+
+	return 0;
+}
+
 /*
  * ti_sci_pd_xlate(): translation service for TI SCI genpds
  * @genpdspec: DT identification data for the genpd
@@ -188,7 +270,15 @@ static int ti_sci_pm_domain_probe(struct platform_device *pdev)
 				pd->pd.power_on = ti_sci_pd_power_on;
 				pd->idx = args.args[0];
 				pd->parent = pd_provider;
-
+				pd->lat_constraint = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
+				/*
+				 * If SCI constraint functions are present, then firmware
+				 * supports the constraints API.
+				 */
+				if (pd_provider->ti_sci->ops.pm_ops.set_device_constraint) {
+					pd->pd.domain.ops.resume = ti_sci_pd_resume;
+					pd->pd.domain.ops.suspend = ti_sci_pd_suspend;
+				}
 				pm_genpd_init(&pd->pd, NULL, true);
 
 				list_add(&pd->node, &pd_provider->pd_list);

-- 
2.46.0

Re: [PATCH 1/3] pmdomain: ti_sci: add per-device latency constraint management

[Markus Schneider-Pargmann]

Hi Kevin,

On Mon, Aug 05, 2024 at 04:38:39PM GMT, Kevin Hilman wrote:
> For each device in a TI SCI PM domain, check whether the device has
> any resume latency constraints set via per-device PM QoS.  If
> constraints are set, send them to DM via the new SCI constraints API.
> 
> Checking for constraints happen:
> 
> 1) before SCI PM domain power off (->power_off() hook)
> 2) before system-wide suspend (via ->suspend() hook)
> 
> For TI SCI devices that are runtime PM enabled, check (1) will be the
> primary method, and will happen when the TI SCI PM domain is powered
> off (e.g. when the runtime PM usecount of the last device in that
> domain goes to zero.)
> 
> For devices that are either not runtime PM enabled, or are not yet
> runtime suspended (e.g. due to being used during the suspend path),
> the constraints check will happen by check(2).
> 
> Since constraints can be sent by either (1) or (2), driver keeps track
> of whether a valid constraint has been sent already.
> 
> An important detail here is that the PM domain driver inserts itself
> into the path of both the ->suspend() and ->resume() hook path
> of *all* devices in the PM domain.  This allows generic PM domain code
> to handle the constraint management and communication with TI SCI.
> 
> Further, this allows device drivers to use existing PM QoS APIs to
> add/update constraints.
> 
> DM firmware clears constraints during its resume, so Linux has
> to check/update/send constraints each time system suspends.
> 
> Co-developed-by: Vibhore Vardhan <vibhore@ti.com>
> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Dhruva Gole <d-gole@ti.com>

Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>

In general this looks good, two small things below.

> ---
>  drivers/pmdomain/ti/ti_sci_pm_domains.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 91 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
> index 1510d5ddae3d..4dc48a97f9b8 100644
> --- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
> +++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
> @@ -13,6 +13,8 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
>  #include <linux/slab.h>
> +#include <linux/pm_qos.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/soc/ti/ti_sci_protocol.h>
>  #include <dt-bindings/soc/ti,sci_pm_domain.h>
>  
> @@ -47,10 +49,46 @@ struct ti_sci_pm_domain {
>  	struct generic_pm_domain pd;
>  	struct list_head node;
>  	struct ti_sci_genpd_provider *parent;
> +	s32 lat_constraint;
> +	bool constraint_sent;
>  };
>  
>  #define genpd_to_ti_sci_pd(gpd) container_of(gpd, struct ti_sci_pm_domain, pd)
>  
> +static inline bool ti_sci_pd_is_valid_constraint(s32 val)
> +{
> +	return val != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> +}
> +
> +static int ti_sci_pd_send_constraint(struct device *dev, s32 val)
> +{
> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
> +	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
> +	int ret;
> +
> +	ret = ti_sci->ops.pm_ops.set_latency_constraint(ti_sci, val, TISCI_MSG_CONSTRAINT_SET);
> +	if (!ret) {
> +		pd->constraint_sent = true;
> +		dev_dbg(dev, "ti_sci_pd: ID:%d set latency constraint %d\n",
> +			pd->idx, val);
> +	} else {
> +		dev_err(dev, "ti_sci_pd: set latency constraint failed: ret=%d\n",
> +			ret);
> +	}
> +
> +	return ret;
> +}
> +
> +static inline void ti_sci_pd_clear_constraints(struct device *dev)
> +{
> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
> +
> +	pd->lat_constraint = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> +	pd->constraint_sent = false;
> +}
> +
>  /*
>   * ti_sci_pd_power_off(): genpd power down hook
>   * @domain: pointer to the powerdomain to power off
> @@ -59,6 +97,18 @@ static int ti_sci_pd_power_off(struct generic_pm_domain *domain)
>  {
>  	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(domain);
>  	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
> +	struct pm_domain_data *pdd;
> +
> +	list_for_each_entry(pdd, &domain->dev_list, list_node) {
> +		struct device *dev = pdd->dev;
> +		s32 val;
> +
> +		/* If device has any resume latency constraints, send 'em */
> +		val = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
> +		if (ti_sci_pd_is_valid_constraint(val) && !pd->constraint_sent)
> +			ti_sci_pd_send_constraint(dev, val);
> +		pd->lat_constraint = val;
> +	}
>  
>  	return ti_sci->ops.dev_ops.put_device(ti_sci, pd->idx);
>  }
> @@ -79,6 +129,38 @@ static int ti_sci_pd_power_on(struct generic_pm_domain *domain)
>  		return ti_sci->ops.dev_ops.get_device(ti_sci, pd->idx);
>  }
>  
> +static int ti_sci_pd_resume(struct device *dev)
> +{
> +	ti_sci_pd_clear_constraints(dev);
> +	return pm_generic_resume(dev);
> +}
> +
> +static int ti_sci_pd_suspend(struct device *dev)
> +{
> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
> +	s32 val;
> +	int ret;
> +
> +	ret = pm_generic_suspend(dev);
> +	if (ret)
> +		return ret;
> +
> +	/* Check if device has any resume latency constraints */
> +	val = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
> +	if (ti_sci_pd_is_valid_constraint(val) && !pd->constraint_sent) {
> +		if (genpd && genpd->status == GENPD_STATE_OFF)
> +			dev_warn(dev, "%s: %s: already off.\n", genpd->name, __func__);
> +		else if (pm_runtime_suspended(dev))
> +			dev_warn(dev, "%s: %s: already RPM suspended.\n", genpd->name, __func__);
> +		else
> +			ti_sci_pd_send_constraint(dev, val);
> +	}
> +	pd->lat_constraint = val;

Could you get rid of pd->constraint_sent? I don't really see a situation
where it would be necessary.

> +
> +	return 0;
> +}
> +
>  /*
>   * ti_sci_pd_xlate(): translation service for TI SCI genpds
>   * @genpdspec: DT identification data for the genpd
> @@ -188,7 +270,15 @@ static int ti_sci_pm_domain_probe(struct platform_device *pdev)
>  				pd->pd.power_on = ti_sci_pd_power_on;
>  				pd->idx = args.args[0];
>  				pd->parent = pd_provider;
> -
> +				pd->lat_constraint = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;

Optionally you could use ti_sci_pd_clear_constraints() here instead.

Best,
Markus

> +				/*
> +				 * If SCI constraint functions are present, then firmware
> +				 * supports the constraints API.
> +				 */
> +				if (pd_provider->ti_sci->ops.pm_ops.set_device_constraint) {
> +					pd->pd.domain.ops.resume = ti_sci_pd_resume;
> +					pd->pd.domain.ops.suspend = ti_sci_pd_suspend;
> +				}
>  				pm_genpd_init(&pd->pd, NULL, true);
>  
>  				list_add(&pd->node, &pd_provider->pd_list);
> 
> -- 
> 2.46.0
> 

Re: [PATCH 1/3] pmdomain: ti_sci: add per-device latency constraint management

[Kevin Hilman]

Markus Schneider-Pargmann <msp@baylibre.com> writes:

> Hi Kevin,
>
> On Mon, Aug 05, 2024 at 04:38:39PM GMT, Kevin Hilman wrote:
>> For each device in a TI SCI PM domain, check whether the device has
>> any resume latency constraints set via per-device PM QoS.  If
>> constraints are set, send them to DM via the new SCI constraints API.
>> 
>> Checking for constraints happen:
>> 
>> 1) before SCI PM domain power off (->power_off() hook)
>> 2) before system-wide suspend (via ->suspend() hook)
>> 
>> For TI SCI devices that are runtime PM enabled, check (1) will be the
>> primary method, and will happen when the TI SCI PM domain is powered
>> off (e.g. when the runtime PM usecount of the last device in that
>> domain goes to zero.)
>> 
>> For devices that are either not runtime PM enabled, or are not yet
>> runtime suspended (e.g. due to being used during the suspend path),
>> the constraints check will happen by check(2).
>> 
>> Since constraints can be sent by either (1) or (2), driver keeps track
>> of whether a valid constraint has been sent already.
>> 
>> An important detail here is that the PM domain driver inserts itself
>> into the path of both the ->suspend() and ->resume() hook path
>> of *all* devices in the PM domain.  This allows generic PM domain code
>> to handle the constraint management and communication with TI SCI.
>> 
>> Further, this allows device drivers to use existing PM QoS APIs to
>> add/update constraints.
>> 
>> DM firmware clears constraints during its resume, so Linux has
>> to check/update/send constraints each time system suspends.
>> 
>> Co-developed-by: Vibhore Vardhan <vibhore@ti.com>
>> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>> Signed-off-by: Dhruva Gole <d-gole@ti.com>
>
> Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
>
> In general this looks good, two small things below.
>
>> ---
>>  drivers/pmdomain/ti/ti_sci_pm_domains.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 91 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
>> index 1510d5ddae3d..4dc48a97f9b8 100644
>> --- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
>> +++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
>> @@ -13,6 +13,8 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_domain.h>
>>  #include <linux/slab.h>
>> +#include <linux/pm_qos.h>
>> +#include <linux/pm_runtime.h>
>>  #include <linux/soc/ti/ti_sci_protocol.h>
>>  #include <dt-bindings/soc/ti,sci_pm_domain.h>
>>  
>> @@ -47,10 +49,46 @@ struct ti_sci_pm_domain {
>>  	struct generic_pm_domain pd;
>>  	struct list_head node;
>>  	struct ti_sci_genpd_provider *parent;
>> +	s32 lat_constraint;
>> +	bool constraint_sent;
>>  };
>>  
>>  #define genpd_to_ti_sci_pd(gpd) container_of(gpd, struct ti_sci_pm_domain, pd)
>>  
>> +static inline bool ti_sci_pd_is_valid_constraint(s32 val)
>> +{
>> +	return val != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>> +}
>> +
>> +static int ti_sci_pd_send_constraint(struct device *dev, s32 val)
>> +{
>> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
>> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
>> +	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
>> +	int ret;
>> +
>> +	ret = ti_sci->ops.pm_ops.set_latency_constraint(ti_sci, val, TISCI_MSG_CONSTRAINT_SET);
>> +	if (!ret) {
>> +		pd->constraint_sent = true;
>> +		dev_dbg(dev, "ti_sci_pd: ID:%d set latency constraint %d\n",
>> +			pd->idx, val);
>> +	} else {
>> +		dev_err(dev, "ti_sci_pd: set latency constraint failed: ret=%d\n",
>> +			ret);
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static inline void ti_sci_pd_clear_constraints(struct device *dev)
>> +{
>> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
>> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
>> +
>> +	pd->lat_constraint = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>> +	pd->constraint_sent = false;
>> +}
>> +
>>  /*
>>   * ti_sci_pd_power_off(): genpd power down hook
>>   * @domain: pointer to the powerdomain to power off
>> @@ -59,6 +97,18 @@ static int ti_sci_pd_power_off(struct generic_pm_domain *domain)
>>  {
>>  	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(domain);
>>  	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
>> +	struct pm_domain_data *pdd;
>> +
>> +	list_for_each_entry(pdd, &domain->dev_list, list_node) {
>> +		struct device *dev = pdd->dev;
>> +		s32 val;
>> +
>> +		/* If device has any resume latency constraints, send 'em */
>> +		val = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
>> +		if (ti_sci_pd_is_valid_constraint(val) && !pd->constraint_sent)
>> +			ti_sci_pd_send_constraint(dev, val);
>> +		pd->lat_constraint = val;
>> +	}
>>  
>>  	return ti_sci->ops.dev_ops.put_device(ti_sci, pd->idx);
>>  }
>> @@ -79,6 +129,38 @@ static int ti_sci_pd_power_on(struct generic_pm_domain *domain)
>>  		return ti_sci->ops.dev_ops.get_device(ti_sci, pd->idx);
>>  }
>>  
>> +static int ti_sci_pd_resume(struct device *dev)
>> +{
>> +	ti_sci_pd_clear_constraints(dev);
>> +	return pm_generic_resume(dev);
>> +}
>> +
>> +static int ti_sci_pd_suspend(struct device *dev)
>> +{
>> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
>> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
>> +	s32 val;
>> +	int ret;
>> +
>> +	ret = pm_generic_suspend(dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Check if device has any resume latency constraints */
>> +	val = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
>> +	if (ti_sci_pd_is_valid_constraint(val) && !pd->constraint_sent) {
>> +		if (genpd && genpd->status == GENPD_STATE_OFF)
>> +			dev_warn(dev, "%s: %s: already off.\n", genpd->name, __func__);
>> +		else if (pm_runtime_suspended(dev))
>> +			dev_warn(dev, "%s: %s: already RPM suspended.\n", genpd->name, __func__);
>> +		else
>> +			ti_sci_pd_send_constraint(dev, val);
>> +	}
>> +	pd->lat_constraint = val;
>
> Could you get rid of pd->constraint_sent? I don't really see a situation
> where it would be necessary.

It is necessary for runtime PM enabled devices that also use
constraints.  For a device like that, if a constraint is present it will
be sent when the PM domain is powered off (right after the runtime PM
usecount goes to zero.)  Later, during system-wide suspend, the suspend
hook for that same device might try to send the constraint again if we
don't keep track of whether the constraint was already sent.

>> +
>> +	return 0;
>> +}
>> +
>>  /*
>>   * ti_sci_pd_xlate(): translation service for TI SCI genpds
>>   * @genpdspec: DT identification data for the genpd
>> @@ -188,7 +270,15 @@ static int ti_sci_pm_domain_probe(struct platform_device *pdev)
>>  				pd->pd.power_on = ti_sci_pd_power_on;
>>  				pd->idx = args.args[0];
>>  				pd->parent = pd_provider;
>> -
>> +				pd->lat_constraint = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>
> Optionally you could use ti_sci_pd_clear_constraints() here instead.

hmm, yes.  Good catch.

Kevin

Re: [PATCH 1/3] pmdomain: ti_sci: add per-device latency constraint management

[Markus Schneider-Pargmann]

On Tue, Aug 06, 2024 at 03:53:49PM GMT, Kevin Hilman wrote:
> Markus Schneider-Pargmann <msp@baylibre.com> writes:
> 
> > Hi Kevin,
> >
> > On Mon, Aug 05, 2024 at 04:38:39PM GMT, Kevin Hilman wrote:
> >> For each device in a TI SCI PM domain, check whether the device has
> >> any resume latency constraints set via per-device PM QoS.  If
> >> constraints are set, send them to DM via the new SCI constraints API.
> >> 
> >> Checking for constraints happen:
> >> 
> >> 1) before SCI PM domain power off (->power_off() hook)
> >> 2) before system-wide suspend (via ->suspend() hook)
> >> 
> >> For TI SCI devices that are runtime PM enabled, check (1) will be the
> >> primary method, and will happen when the TI SCI PM domain is powered
> >> off (e.g. when the runtime PM usecount of the last device in that
> >> domain goes to zero.)
> >> 
> >> For devices that are either not runtime PM enabled, or are not yet
> >> runtime suspended (e.g. due to being used during the suspend path),
> >> the constraints check will happen by check(2).
> >> 
> >> Since constraints can be sent by either (1) or (2), driver keeps track
> >> of whether a valid constraint has been sent already.
> >> 
> >> An important detail here is that the PM domain driver inserts itself
> >> into the path of both the ->suspend() and ->resume() hook path
> >> of *all* devices in the PM domain.  This allows generic PM domain code
> >> to handle the constraint management and communication with TI SCI.
> >> 
> >> Further, this allows device drivers to use existing PM QoS APIs to
> >> add/update constraints.
> >> 
> >> DM firmware clears constraints during its resume, so Linux has
> >> to check/update/send constraints each time system suspends.
> >> 
> >> Co-developed-by: Vibhore Vardhan <vibhore@ti.com>
> >> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> >> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> >> Signed-off-by: Dhruva Gole <d-gole@ti.com>
> >
> > Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
> >
> > In general this looks good, two small things below.
> >
> >> ---
> >>  drivers/pmdomain/ti/ti_sci_pm_domains.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 91 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
> >> index 1510d5ddae3d..4dc48a97f9b8 100644
> >> --- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
> >> +++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
> >> @@ -13,6 +13,8 @@
> >>  #include <linux/platform_device.h>
> >>  #include <linux/pm_domain.h>
> >>  #include <linux/slab.h>
> >> +#include <linux/pm_qos.h>
> >> +#include <linux/pm_runtime.h>
> >>  #include <linux/soc/ti/ti_sci_protocol.h>
> >>  #include <dt-bindings/soc/ti,sci_pm_domain.h>
> >>  
> >> @@ -47,10 +49,46 @@ struct ti_sci_pm_domain {
> >>  	struct generic_pm_domain pd;
> >>  	struct list_head node;
> >>  	struct ti_sci_genpd_provider *parent;
> >> +	s32 lat_constraint;
> >> +	bool constraint_sent;
> >>  };
> >>  
> >>  #define genpd_to_ti_sci_pd(gpd) container_of(gpd, struct ti_sci_pm_domain, pd)
> >>  
> >> +static inline bool ti_sci_pd_is_valid_constraint(s32 val)
> >> +{
> >> +	return val != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> >> +}
> >> +
> >> +static int ti_sci_pd_send_constraint(struct device *dev, s32 val)
> >> +{
> >> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
> >> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
> >> +	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
> >> +	int ret;
> >> +
> >> +	ret = ti_sci->ops.pm_ops.set_latency_constraint(ti_sci, val, TISCI_MSG_CONSTRAINT_SET);
> >> +	if (!ret) {
> >> +		pd->constraint_sent = true;
> >> +		dev_dbg(dev, "ti_sci_pd: ID:%d set latency constraint %d\n",
> >> +			pd->idx, val);
> >> +	} else {
> >> +		dev_err(dev, "ti_sci_pd: set latency constraint failed: ret=%d\n",
> >> +			ret);
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static inline void ti_sci_pd_clear_constraints(struct device *dev)
> >> +{
> >> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
> >> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
> >> +
> >> +	pd->lat_constraint = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> >> +	pd->constraint_sent = false;
> >> +}
> >> +
> >>  /*
> >>   * ti_sci_pd_power_off(): genpd power down hook
> >>   * @domain: pointer to the powerdomain to power off
> >> @@ -59,6 +97,18 @@ static int ti_sci_pd_power_off(struct generic_pm_domain *domain)
> >>  {
> >>  	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(domain);
> >>  	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
> >> +	struct pm_domain_data *pdd;
> >> +
> >> +	list_for_each_entry(pdd, &domain->dev_list, list_node) {
> >> +		struct device *dev = pdd->dev;
> >> +		s32 val;
> >> +
> >> +		/* If device has any resume latency constraints, send 'em */
> >> +		val = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
> >> +		if (ti_sci_pd_is_valid_constraint(val) && !pd->constraint_sent)
> >> +			ti_sci_pd_send_constraint(dev, val);
> >> +		pd->lat_constraint = val;
> >> +	}
> >>  
> >>  	return ti_sci->ops.dev_ops.put_device(ti_sci, pd->idx);
> >>  }
> >> @@ -79,6 +129,38 @@ static int ti_sci_pd_power_on(struct generic_pm_domain *domain)
> >>  		return ti_sci->ops.dev_ops.get_device(ti_sci, pd->idx);
> >>  }
> >>  
> >> +static int ti_sci_pd_resume(struct device *dev)
> >> +{
> >> +	ti_sci_pd_clear_constraints(dev);
> >> +	return pm_generic_resume(dev);
> >> +}
> >> +
> >> +static int ti_sci_pd_suspend(struct device *dev)
> >> +{
> >> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
> >> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
> >> +	s32 val;
> >> +	int ret;
> >> +
> >> +	ret = pm_generic_suspend(dev);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	/* Check if device has any resume latency constraints */
> >> +	val = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
> >> +	if (ti_sci_pd_is_valid_constraint(val) && !pd->constraint_sent) {
> >> +		if (genpd && genpd->status == GENPD_STATE_OFF)
> >> +			dev_warn(dev, "%s: %s: already off.\n", genpd->name, __func__);
> >> +		else if (pm_runtime_suspended(dev))
> >> +			dev_warn(dev, "%s: %s: already RPM suspended.\n", genpd->name, __func__);
> >> +		else
> >> +			ti_sci_pd_send_constraint(dev, val);
> >> +	}
> >> +	pd->lat_constraint = val;
> >
> > Could you get rid of pd->constraint_sent? I don't really see a situation
> > where it would be necessary.
> 
> It is necessary for runtime PM enabled devices that also use
> constraints.  For a device like that, if a constraint is present it will
> be sent when the PM domain is powered off (right after the runtime PM
> usecount goes to zero.)  Later, during system-wide suspend, the suspend
> hook for that same device might try to send the constraint again if we
> don't keep track of whether the constraint was already sent.

But wouldn't in that case pd->lat_constraint also be valid as you set it
only after sending the constraint? So if you check for that instead of
pd->constraint_sent, could you remove constraint_sent? On resume it is
always reset as well.

It just looked to me like if it is valid then it would have also been
sent, which is the same semantic as the bool.

Best
Markus

> 
> >> +
> >> +	return 0;
> >> +}
> >> +
> >>  /*
> >>   * ti_sci_pd_xlate(): translation service for TI SCI genpds
> >>   * @genpdspec: DT identification data for the genpd
> >> @@ -188,7 +270,15 @@ static int ti_sci_pm_domain_probe(struct platform_device *pdev)
> >>  				pd->pd.power_on = ti_sci_pd_power_on;
> >>  				pd->idx = args.args[0];
> >>  				pd->parent = pd_provider;
> >> -
> >> +				pd->lat_constraint = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> >
> > Optionally you could use ti_sci_pd_clear_constraints() here instead.
> 
> hmm, yes.  Good catch.
> 
> Kevin

Re: [PATCH 1/3] pmdomain: ti_sci: add per-device latency constraint management

[Kevin Hilman]

Markus Schneider-Pargmann <msp@baylibre.com> writes:

> On Tue, Aug 06, 2024 at 03:53:49PM GMT, Kevin Hilman wrote:
>> Markus Schneider-Pargmann <msp@baylibre.com> writes:
>> 
>> > Hi Kevin,
>> >
>> > On Mon, Aug 05, 2024 at 04:38:39PM GMT, Kevin Hilman wrote:
>> >> For each device in a TI SCI PM domain, check whether the device has
>> >> any resume latency constraints set via per-device PM QoS.  If
>> >> constraints are set, send them to DM via the new SCI constraints API.
>> >> 
>> >> Checking for constraints happen:
>> >> 
>> >> 1) before SCI PM domain power off (->power_off() hook)
>> >> 2) before system-wide suspend (via ->suspend() hook)
>> >> 
>> >> For TI SCI devices that are runtime PM enabled, check (1) will be the
>> >> primary method, and will happen when the TI SCI PM domain is powered
>> >> off (e.g. when the runtime PM usecount of the last device in that
>> >> domain goes to zero.)
>> >> 
>> >> For devices that are either not runtime PM enabled, or are not yet
>> >> runtime suspended (e.g. due to being used during the suspend path),
>> >> the constraints check will happen by check(2).
>> >> 
>> >> Since constraints can be sent by either (1) or (2), driver keeps track
>> >> of whether a valid constraint has been sent already.
>> >> 
>> >> An important detail here is that the PM domain driver inserts itself
>> >> into the path of both the ->suspend() and ->resume() hook path
>> >> of *all* devices in the PM domain.  This allows generic PM domain code
>> >> to handle the constraint management and communication with TI SCI.
>> >> 
>> >> Further, this allows device drivers to use existing PM QoS APIs to
>> >> add/update constraints.
>> >> 
>> >> DM firmware clears constraints during its resume, so Linux has
>> >> to check/update/send constraints each time system suspends.
>> >> 
>> >> Co-developed-by: Vibhore Vardhan <vibhore@ti.com>
>> >> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
>> >> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>> >> Signed-off-by: Dhruva Gole <d-gole@ti.com>
>> >
>> > Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
>> >
>> > In general this looks good, two small things below.
>> >
>> >> ---
>> >>  drivers/pmdomain/ti/ti_sci_pm_domains.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> >>  1 file changed, 91 insertions(+), 1 deletion(-)
>> >> 
>> >> diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
>> >> index 1510d5ddae3d..4dc48a97f9b8 100644
>> >> --- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
>> >> +++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
>> >> @@ -13,6 +13,8 @@
>> >>  #include <linux/platform_device.h>
>> >>  #include <linux/pm_domain.h>
>> >>  #include <linux/slab.h>
>> >> +#include <linux/pm_qos.h>
>> >> +#include <linux/pm_runtime.h>
>> >>  #include <linux/soc/ti/ti_sci_protocol.h>
>> >>  #include <dt-bindings/soc/ti,sci_pm_domain.h>
>> >>  
>> >> @@ -47,10 +49,46 @@ struct ti_sci_pm_domain {
>> >>  	struct generic_pm_domain pd;
>> >>  	struct list_head node;
>> >>  	struct ti_sci_genpd_provider *parent;
>> >> +	s32 lat_constraint;
>> >> +	bool constraint_sent;
>> >>  };
>> >>  
>> >>  #define genpd_to_ti_sci_pd(gpd) container_of(gpd, struct ti_sci_pm_domain, pd)
>> >>  
>> >> +static inline bool ti_sci_pd_is_valid_constraint(s32 val)
>> >> +{
>> >> +	return val != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>> >> +}
>> >> +
>> >> +static int ti_sci_pd_send_constraint(struct device *dev, s32 val)
>> >> +{
>> >> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
>> >> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
>> >> +	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
>> >> +	int ret;
>> >> +
>> >> +	ret = ti_sci->ops.pm_ops.set_latency_constraint(ti_sci, val, TISCI_MSG_CONSTRAINT_SET);
>> >> +	if (!ret) {
>> >> +		pd->constraint_sent = true;
>> >> +		dev_dbg(dev, "ti_sci_pd: ID:%d set latency constraint %d\n",
>> >> +			pd->idx, val);
>> >> +	} else {
>> >> +		dev_err(dev, "ti_sci_pd: set latency constraint failed: ret=%d\n",
>> >> +			ret);
>> >> +	}
>> >> +
>> >> +	return ret;
>> >> +}
>> >> +
>> >> +static inline void ti_sci_pd_clear_constraints(struct device *dev)
>> >> +{
>> >> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
>> >> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
>> >> +
>> >> +	pd->lat_constraint = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>> >> +	pd->constraint_sent = false;
>> >> +}
>> >> +
>> >>  /*
>> >>   * ti_sci_pd_power_off(): genpd power down hook
>> >>   * @domain: pointer to the powerdomain to power off
>> >> @@ -59,6 +97,18 @@ static int ti_sci_pd_power_off(struct generic_pm_domain *domain)
>> >>  {
>> >>  	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(domain);
>> >>  	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
>> >> +	struct pm_domain_data *pdd;
>> >> +
>> >> +	list_for_each_entry(pdd, &domain->dev_list, list_node) {
>> >> +		struct device *dev = pdd->dev;
>> >> +		s32 val;
>> >> +
>> >> +		/* If device has any resume latency constraints, send 'em */
>> >> +		val = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
>> >> +		if (ti_sci_pd_is_valid_constraint(val) && !pd->constraint_sent)
>> >> +			ti_sci_pd_send_constraint(dev, val);
>> >> +		pd->lat_constraint = val;
>> >> +	}
>> >>  
>> >>  	return ti_sci->ops.dev_ops.put_device(ti_sci, pd->idx);
>> >>  }
>> >> @@ -79,6 +129,38 @@ static int ti_sci_pd_power_on(struct generic_pm_domain *domain)
>> >>  		return ti_sci->ops.dev_ops.get_device(ti_sci, pd->idx);
>> >>  }
>> >>  
>> >> +static int ti_sci_pd_resume(struct device *dev)
>> >> +{
>> >> +	ti_sci_pd_clear_constraints(dev);
>> >> +	return pm_generic_resume(dev);
>> >> +}
>> >> +
>> >> +static int ti_sci_pd_suspend(struct device *dev)
>> >> +{
>> >> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
>> >> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
>> >> +	s32 val;
>> >> +	int ret;
>> >> +
>> >> +	ret = pm_generic_suspend(dev);
>> >> +	if (ret)
>> >> +		return ret;
>> >> +
>> >> +	/* Check if device has any resume latency constraints */
>> >> +	val = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY);
>> >> +	if (ti_sci_pd_is_valid_constraint(val) && !pd->constraint_sent) {
>> >> +		if (genpd && genpd->status == GENPD_STATE_OFF)
>> >> +			dev_warn(dev, "%s: %s: already off.\n", genpd->name, __func__);
>> >> +		else if (pm_runtime_suspended(dev))
>> >> +			dev_warn(dev, "%s: %s: already RPM suspended.\n", genpd->name, __func__);
>> >> +		else
>> >> +			ti_sci_pd_send_constraint(dev, val);
>> >> +	}
>> >> +	pd->lat_constraint = val;
>> >
>> > Could you get rid of pd->constraint_sent? I don't really see a situation
>> > where it would be necessary.
>> 
>> It is necessary for runtime PM enabled devices that also use
>> constraints.  For a device like that, if a constraint is present it will
>> be sent when the PM domain is powered off (right after the runtime PM
>> usecount goes to zero.)  Later, during system-wide suspend, the suspend
>> hook for that same device might try to send the constraint again if we
>> don't keep track of whether the constraint was already sent.
>
> But wouldn't in that case pd->lat_constraint also be valid as you set it
> only after sending the constraint? So if you check for that instead of
> pd->constraint_sent, could you remove constraint_sent? On resume it is
> always reset as well.
>
> It just looked to me like if it is valid then it would have also been
> sent, which is the same semantic as the bool.

hmm, yeah.  I think you're right that constraint valid and constraint
sent are the same thing.

In earlier versions of the firmware, there was slightly different
behavior about how constraints were cleared, but with how it works now,
I think it can be removed.

I'll test with that for the next version.

Thanks for the review!

Kevin

[PATCH 2/3] pmdomain: ti_sci: add wakeup constraint management

[Kevin Hilman]

During system-wide suspend, check all devices connected to PM domain
to see if they are wakeup-enabled.  If so, set a TI SCI device
constraint.

Note: DM firmware clears all constraints on resume.

Co-developed-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
Signed-off-by: Dhruva Gole <d-gole@ti.com>
---
 drivers/pmdomain/ti/ti_sci_pm_domains.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
index 4dc48a97f9b8..7cd6ae957289 100644
--- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
+++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
@@ -51,6 +51,7 @@ struct ti_sci_pm_domain {
 	struct ti_sci_genpd_provider *parent;
 	s32 lat_constraint;
 	bool constraint_sent;
+	bool wkup_constraint;
 };
 
 #define genpd_to_ti_sci_pd(gpd) container_of(gpd, struct ti_sci_pm_domain, pd)
@@ -87,6 +88,26 @@ static inline void ti_sci_pd_clear_constraints(struct device *dev)
 
 	pd->lat_constraint = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
 	pd->constraint_sent = false;
+	pd->wkup_constraint = false;
+}
+
+static inline bool ti_sci_pd_check_wkup_constraint(struct device *dev)
+{
+	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
+	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
+	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
+	int ret;
+
+	if (device_may_wakeup(dev)) {
+		ret = ti_sci->ops.pm_ops.set_device_constraint(ti_sci, pd->idx,
+							       TISCI_MSG_CONSTRAINT_SET);
+		if (!ret) {
+			pd->wkup_constraint = true;
+			dev_dbg(dev, "ti_sci_pd: ID:%d set device constraint.\n", pd->idx);
+		}
+	}
+
+	return pd->wkup_constraint;
 }
 
 /*
@@ -158,6 +179,8 @@ static int ti_sci_pd_suspend(struct device *dev)
 	}
 	pd->lat_constraint = val;
 
+	ti_sci_pd_check_wkup_constraint(dev);
+
 	return 0;
 }
 

-- 
2.46.0

Re: [PATCH 2/3] pmdomain: ti_sci: add wakeup constraint management

[Markus Schneider-Pargmann]

On Mon, Aug 05, 2024 at 04:38:40PM GMT, Kevin Hilman wrote:
> During system-wide suspend, check all devices connected to PM domain
> to see if they are wakeup-enabled.  If so, set a TI SCI device
> constraint.
> 
> Note: DM firmware clears all constraints on resume.
> 
> Co-developed-by: Vibhore Vardhan <vibhore@ti.com>
> Signed-off-by: Vibhore Vardhan <vibhore@ti.com>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> Signed-off-by: Dhruva Gole <d-gole@ti.com>
> ---
>  drivers/pmdomain/ti/ti_sci_pm_domains.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
> index 4dc48a97f9b8..7cd6ae957289 100644
> --- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
> +++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
> @@ -51,6 +51,7 @@ struct ti_sci_pm_domain {
>  	struct ti_sci_genpd_provider *parent;
>  	s32 lat_constraint;
>  	bool constraint_sent;
> +	bool wkup_constraint;
>  };
>  
>  #define genpd_to_ti_sci_pd(gpd) container_of(gpd, struct ti_sci_pm_domain, pd)
> @@ -87,6 +88,26 @@ static inline void ti_sci_pd_clear_constraints(struct device *dev)
>  
>  	pd->lat_constraint = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>  	pd->constraint_sent = false;
> +	pd->wkup_constraint = false;
> +}
> +
> +static inline bool ti_sci_pd_check_wkup_constraint(struct device *dev)

'check' in the function name sounds like a passive function. Maybe
ti_sci_pd_send_wkup_constraint() would indicate its purpose better?

> +{
> +	struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
> +	struct ti_sci_pm_domain *pd = genpd_to_ti_sci_pd(genpd);
> +	const struct ti_sci_handle *ti_sci = pd->parent->ti_sci;
> +	int ret;
> +
> +	if (device_may_wakeup(dev)) {
> +		ret = ti_sci->ops.pm_ops.set_device_constraint(ti_sci, pd->idx,
> +							       TISCI_MSG_CONSTRAINT_SET);
> +		if (!ret) {
> +			pd->wkup_constraint = true;
> +			dev_dbg(dev, "ti_sci_pd: ID:%d set device constraint.\n", pd->idx);
> +		}
> +	}
> +
> +	return pd->wkup_constraint;

Is this return value used anywhere?

Best
Markus

>  }
>  
>  /*
> @@ -158,6 +179,8 @@ static int ti_sci_pd_suspend(struct device *dev)
>  	}
>  	pd->lat_constraint = val;
>  
> +	ti_sci_pd_check_wkup_constraint(dev);
> +
>  	return 0;
>  }
>  
> 
> -- 
> 2.46.0
> 

[PATCH 3/3] pmdomain: ti_sci: handle wake IRQs for IO daisy chain wakeups

[Kevin Hilman]

When a device supports IO daisy-chain wakeups, it uses a dedicated
wake IRQ.  Devices with IO daisy-chain wakeups enabled should not set
wakeup constraints since these can happen even from deep power states,
so should not prevent the DM from picking deep power states.

Wake IRQs are set with dev_pm_set_wake_irq() or
dev_pm_set_dedicated_wake_irq().  The latter is used by the serial
driver used on K3 platforms (drivers/tty/serial/8250/8250_omap.c)
when the interrupts-extended property is used to describe the
dedicated wakeup interrupt.

Detect these wake IRQs in the suspend path, and if set, skip sending
constraint.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 drivers/pmdomain/ti/ti_sci_pm_domains.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/pmdomain/ti/ti_sci_pm_domains.c b/drivers/pmdomain/ti/ti_sci_pm_domains.c
index 7cd6ae957289..4c85ce50510f 100644
--- a/drivers/pmdomain/ti/ti_sci_pm_domains.c
+++ b/drivers/pmdomain/ti/ti_sci_pm_domains.c
@@ -99,6 +99,15 @@ static inline bool ti_sci_pd_check_wkup_constraint(struct device *dev)
 	int ret;
 
 	if (device_may_wakeup(dev)) {
+		/*
+		 * If device can wakeup using IO daisy chain wakeups,
+		 * we do not want to set a constraint.
+		 */
+		if (dev->power.wakeirq) {
+			dev_dbg(dev, "%s: has wake IRQ, not setting constraints\n", __func__);
+			return false;
+		}
+
 		ret = ti_sci->ops.pm_ops.set_device_constraint(ti_sci, pd->idx,
 							       TISCI_MSG_CONSTRAINT_SET);
 		if (!ret) {

-- 
2.46.0