All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND v3 0/3] add some more functionality to RPMH
@ 2019-02-21 12:18 Raju P.L.S.S.S.N
  2019-02-21 12:18 ` [PATCH RESEND v3 1/3] drivers: qcom: rpmh-rsc: simplify TCS locking Raju P.L.S.S.S.N
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Raju P.L.S.S.S.N @ 2019-02-21 12:18 UTC (permalink / raw)
  To: andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, linux-pm, sboyd, evgreen,
	dianders, mka, ilina, Raju P.L.S.S.S.N

Resending the patches. Kindly review.

Changes in v3:
 - Simplify TCS locking - added a new patch
 - Add lock check to avoid potential race as suggested by Matthias
 - Add functionality to RSC controller to disallow active requests
   if solver mode is not set 
 - Removed independent patches and posted as separate series[2]

Changes in v2:
 - Remove unnecessary EXPORT_SYMBOL in rpmh-rsc

This set of patches add additional functionality to RPMH drivers[1].

PM drivers can choose to disallow idle modes when RSC controller is busy
sending or processing requests. The patches add necessary functions to
query the controller status.

The controllers may be in 'solver' state, where they could be in autonomous
mode executing low power modes for their hardware and as such are not
available for sending active votes. Functionality to get notified about
such state and disallow requests for state change in that case is added
in these patches.

Please consider reviewing this patchset.

This series is based on other patchset[2] 


v1:https://lkml.org/lkml/2018/7/19/213
v2:https://patchwork.kernel.org/cover/10546863/


[1] https://lkml.org/lkml/2018/6/20/519
[2] https://lkml.org/lkml/2019/2/18/714


Lina Iyer (2):
  drivers: qcom: rpmh-rsc: return if the controller is idle
  drivers: qcom: rpmh: disallow active requests in solver mode

Raju P.L.S.S.S.N (1):
  drivers: qcom: rpmh-rsc: simplify TCS locking

 drivers/soc/qcom/rpmh-internal.h |  8 ++-
 drivers/soc/qcom/rpmh-rsc.c      | 91 +++++++++++++++++++++++---------
 drivers/soc/qcom/rpmh.c          | 84 ++++++++++++++++++++++++-----
 include/soc/qcom/rpmh.h          | 10 ++++
 4 files changed, 155 insertions(+), 38 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of the Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH RESEND v3 1/3] drivers: qcom: rpmh-rsc: simplify TCS locking
  2019-02-21 12:18 [PATCH RESEND v3 0/3] add some more functionality to RPMH Raju P.L.S.S.S.N
@ 2019-02-21 12:18 ` Raju P.L.S.S.S.N
  2019-02-21 12:18 ` [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle Raju P.L.S.S.S.N
  2019-02-21 12:18 ` [PATCH RESEND v3 3/3] drivers: qcom: rpmh: disallow active requests in solver mode Raju P.L.S.S.S.N
  2 siblings, 0 replies; 12+ messages in thread
From: Raju P.L.S.S.S.N @ 2019-02-21 12:18 UTC (permalink / raw)
  To: andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, linux-pm, sboyd, evgreen,
	dianders, mka, ilina, Raju P.L.S.S.S.N

tcs->lock was introduced to serialize access with in TCS group. But
even without tcs->lock, drv->lock is serving the same purpose. So
use single drv->lock.

Other optimizations include -
 - Remove locking around clear_bit() in IRQ handler. clear_bit() is
   atomic.
 - Remove redundant read of TCS registers.
 - Use spin_lock instead of _irq variants as the locks are not held
   in interrupt context

Suggested-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
---
 drivers/soc/qcom/rpmh-internal.h |  2 --
 drivers/soc/qcom/rpmh-rsc.c      | 37 +++++++++++---------------------
 drivers/soc/qcom/rpmh.c          | 20 +++++++----------
 3 files changed, 21 insertions(+), 38 deletions(-)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 8c316b4d36dc..2e3ffcdf220e 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -28,7 +28,6 @@ struct rsc_drv;
  * @offset:    start of the TCS group relative to the TCSes in the RSC
  * @num_tcs:   number of TCSes in this type
  * @ncpt:      number of commands in each TCS
- * @lock:      lock for synchronizing this TCS writes
  * @req:       requests that are sent from the TCS
  * @cmd_cache: flattened cache of cmds in sleep/wake TCS
  * @slots:     indicates which of @cmd_addr are occupied
@@ -40,7 +39,6 @@ struct tcs_group {
 	u32 offset;
 	int num_tcs;
 	int ncpt;
-	spinlock_t lock;
 	const struct tcs_request *req[MAX_TCS_PER_TYPE];
 	u32 *cmd_cache;
 	DECLARE_BITMAP(slots, MAX_TCS_SLOTS);
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 73d5b9802a29..d6b834eeeb37 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -98,8 +98,7 @@ static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int tcs_id,
 
 static bool tcs_is_free(struct rsc_drv *drv, int tcs_id)
 {
-	return !test_bit(tcs_id, drv->tcs_in_use) &&
-	       read_tcs_reg(drv, RSC_DRV_STATUS, tcs_id, 0);
+	return !test_bit(tcs_id, drv->tcs_in_use);
 }
 
 static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type)
@@ -109,29 +108,28 @@ static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type)
 
 static int tcs_invalidate(struct rsc_drv *drv, int type)
 {
-	int m;
+	int m, ret = 0;
 	struct tcs_group *tcs;
 
 	tcs = get_tcs_of_type(drv, type);
 
-	spin_lock(&tcs->lock);
-	if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) {
-		spin_unlock(&tcs->lock);
-		return 0;
-	}
+	spin_lock(&drv->lock);
+	if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS))
+		goto done;
 
 	for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
 		if (!tcs_is_free(drv, m)) {
-			spin_unlock(&tcs->lock);
-			return -EAGAIN;
+			ret = -EAGAIN;
+			goto done;
 		}
 		write_tcs_reg_sync(drv, RSC_DRV_CMD_ENABLE, m, 0);
 		write_tcs_reg_sync(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, m, 0);
 	}
 	bitmap_zero(tcs->slots, MAX_TCS_SLOTS);
-	spin_unlock(&tcs->lock);
 
-	return 0;
+done:
+	spin_unlock(&drv->lock);
+	return ret;
 }
 
 /**
@@ -298,9 +296,7 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
 		write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, i, 0);
 		write_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, i, 0);
 		write_tcs_reg(drv, RSC_DRV_IRQ_CLEAR, 0, BIT(i));
-		spin_lock(&drv->lock);
 		clear_bit(i, drv->tcs_in_use);
-		spin_unlock(&drv->lock);
 		if (req)
 			rpmh_tx_done(req, err);
 	}
@@ -383,14 +379,12 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg)
 {
 	struct tcs_group *tcs;
 	int tcs_id;
-	unsigned long flags;
 	int ret;
 
 	tcs = get_tcs_for_msg(drv, msg);
 	if (IS_ERR(tcs))
 		return PTR_ERR(tcs);
 
-	spin_lock_irqsave(&tcs->lock, flags);
 	spin_lock(&drv->lock);
 	/*
 	 * The h/w does not like if we send a request to the same address,
@@ -398,14 +392,12 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg)
 	 */
 	ret = check_for_req_inflight(drv, tcs, msg);
 	if (ret) {
-		spin_unlock(&drv->lock);
 		goto done_write;
 	}
 
 	tcs_id = find_free_tcs(tcs);
 	if (tcs_id < 0) {
 		ret = tcs_id;
-		spin_unlock(&drv->lock);
 		goto done_write;
 	}
 
@@ -413,13 +405,12 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg)
 	set_bit(tcs_id, drv->tcs_in_use);
 	if (msg->state == RPMH_ACTIVE_ONLY_STATE && tcs->type != ACTIVE_TCS)
 		enable_tcs_irq(drv, tcs_id, true);
-	spin_unlock(&drv->lock);
 
 	__tcs_buffer_write(drv, tcs_id, 0, msg);
 	__tcs_trigger(drv, tcs_id, true);
 
 done_write:
-	spin_unlock_irqrestore(&tcs->lock, flags);
+	spin_unlock(&drv->lock);
 	return ret;
 }
 
@@ -517,19 +508,18 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)
 {
 	struct tcs_group *tcs;
 	int tcs_id = 0, cmd_id = 0;
-	unsigned long flags;
 	int ret;
 
 	tcs = get_tcs_for_msg(drv, msg);
 	if (IS_ERR(tcs))
 		return PTR_ERR(tcs);
 
-	spin_lock_irqsave(&tcs->lock, flags);
+	spin_lock(&drv->lock);
 	/* find the TCS id and the command in the TCS to write to */
 	ret = find_slots(tcs, msg, &tcs_id, &cmd_id);
 	if (!ret)
 		__tcs_buffer_write(drv, tcs_id, cmd_id, msg);
-	spin_unlock_irqrestore(&tcs->lock, flags);
+	spin_unlock(&drv->lock);
 
 	return ret;
 }
@@ -638,7 +628,6 @@ static int rpmh_probe_tcs_config(struct platform_device *pdev,
 		tcs->type = tcs_cfg[i].type;
 		tcs->num_tcs = tcs_cfg[i].n;
 		tcs->ncpt = ncpt;
-		spin_lock_init(&tcs->lock);
 
 		if (!tcs->num_tcs || tcs->type == CONTROL_TCS)
 			continue;
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 71a76eaa4b63..297d6cc4e395 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -117,9 +117,8 @@ static struct cache_req *cache_rpm_request(struct rpmh_ctrlr *ctrlr,
 					   struct tcs_cmd *cmd)
 {
 	struct cache_req *req;
-	unsigned long flags;
 
-	spin_lock_irqsave(&ctrlr->cache_lock, flags);
+	spin_lock(&ctrlr->cache_lock);
 	req = __find_req(ctrlr, cmd->addr);
 	if (req)
 		goto existing;
@@ -153,7 +152,7 @@ static struct cache_req *cache_rpm_request(struct rpmh_ctrlr *ctrlr,
 
 	ctrlr->dirty = true;
 unlock:
-	spin_unlock_irqrestore(&ctrlr->cache_lock, flags);
+	spin_unlock(&ctrlr->cache_lock);
 
 	return req;
 }
@@ -283,23 +282,21 @@ EXPORT_SYMBOL(rpmh_write);
 
 static void cache_batch(struct rpmh_ctrlr *ctrlr, struct batch_cache_req *req)
 {
-	unsigned long flags;
 
-	spin_lock_irqsave(&ctrlr->cache_lock, flags);
+	spin_lock(&ctrlr->cache_lock);
 	list_add_tail(&req->list, &ctrlr->batch_cache);
-	spin_unlock_irqrestore(&ctrlr->cache_lock, flags);
+	spin_unlock(&ctrlr->cache_lock);
 }
 
 static int flush_batch(struct rpmh_ctrlr *ctrlr)
 {
 	struct batch_cache_req *req;
 	const struct rpmh_request *rpm_msg;
-	unsigned long flags;
 	int ret = 0;
 	int i;
 
 	/* Send Sleep/Wake requests to the controller, expect no response */
-	spin_lock_irqsave(&ctrlr->cache_lock, flags);
+	spin_lock(&ctrlr->cache_lock);
 	list_for_each_entry(req, &ctrlr->batch_cache, list) {
 		for (i = 0; i < req->count; i++) {
 			rpm_msg = req->rpm_msgs + i;
@@ -309,7 +306,7 @@ static int flush_batch(struct rpmh_ctrlr *ctrlr)
 				break;
 		}
 	}
-	spin_unlock_irqrestore(&ctrlr->cache_lock, flags);
+	spin_unlock(&ctrlr->cache_lock);
 
 	return ret;
 }
@@ -317,13 +314,12 @@ static int flush_batch(struct rpmh_ctrlr *ctrlr)
 static void invalidate_batch(struct rpmh_ctrlr *ctrlr)
 {
 	struct batch_cache_req *req, *tmp;
-	unsigned long flags;
 
-	spin_lock_irqsave(&ctrlr->cache_lock, flags);
+	spin_lock(&ctrlr->cache_lock);
 	list_for_each_entry_safe(req, tmp, &ctrlr->batch_cache, list)
 		kfree(req);
 	INIT_LIST_HEAD(&ctrlr->batch_cache);
-	spin_unlock_irqrestore(&ctrlr->cache_lock, flags);
+	spin_unlock(&ctrlr->cache_lock);
 }
 
 /**
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of the Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle
  2019-02-21 12:18 [PATCH RESEND v3 0/3] add some more functionality to RPMH Raju P.L.S.S.S.N
  2019-02-21 12:18 ` [PATCH RESEND v3 1/3] drivers: qcom: rpmh-rsc: simplify TCS locking Raju P.L.S.S.S.N
@ 2019-02-21 12:18 ` Raju P.L.S.S.S.N
  2019-02-27  0:49     ` Stephen Boyd
  2019-02-21 12:18 ` [PATCH RESEND v3 3/3] drivers: qcom: rpmh: disallow active requests in solver mode Raju P.L.S.S.S.N
  2 siblings, 1 reply; 12+ messages in thread
From: Raju P.L.S.S.S.N @ 2019-02-21 12:18 UTC (permalink / raw)
  To: andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, linux-pm, sboyd, evgreen,
	dianders, mka, ilina, Raju P . L . S . S . S . N

From: Lina Iyer <ilina@codeaurora.org>

Allow the controller status be queried. The controller is busy if it is
actively processing request. Also allow the controller state be read by
platform drivers. This is useful for PM drivers which can choose to
disallow idle modes when the controller is busy.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
---
 drivers/soc/qcom/rpmh-internal.h |  1 +
 drivers/soc/qcom/rpmh-rsc.c      | 24 ++++++++++++++++++++++++
 drivers/soc/qcom/rpmh.c          | 13 +++++++++++++
 include/soc/qcom/rpmh.h          |  5 +++++
 4 files changed, 43 insertions(+)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 2e3ffcdf220e..4b891c23b4cd 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -109,6 +109,7 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
 			     const struct tcs_request *msg);
 int rpmh_rsc_invalidate(struct rsc_drv *drv);
 int rpmh_rsc_write_pdc_data(struct rsc_drv *drv, const struct tcs_request *msg);
+bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv);
 void rpmh_tx_done(const struct tcs_request *msg, int r);
 
 #endif /* __RPM_INTERNAL_H__ */
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index d6b834eeeb37..9cc303e88a06 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -524,6 +524,30 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)
 	return ret;
 }
 
+/**
+ *  rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
+ *
+ *  @drv: The controller
+ *
+ *  Returns true if the TCSes are engaged in handling requests.
+ */
+bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv)
+{
+	int m;
+	struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS);
+
+	spin_lock(&drv->lock);
+	for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
+		if (!tcs_is_free(drv, m)) {
+			spin_unlock(&drv->lock);
+			return false;
+		}
+	}
+	spin_unlock(&drv->lock);
+
+	return true;
+}
+
 /**
  * rpmh_rsc_write_ctrl_data: Write request to the controller
  *
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 297d6cc4e395..43eb9816041a 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -535,3 +535,16 @@ int rpmh_invalidate(const struct device *dev)
 	return ret;
 }
 EXPORT_SYMBOL(rpmh_invalidate);
+
+/**
+ * rpmh_ctrlr_idle: Return the controller idle status
+ *
+ * @dev: the device making the request
+ */
+int rpmh_ctrlr_idle(const struct device *dev)
+{
+	struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
+
+	return rpmh_rsc_ctrlr_is_idle(ctrlr_to_drv(ctrlr));
+}
+EXPORT_SYMBOL(rpmh_ctrlr_idle);
diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h
index b05e31aaf047..4c4b013f0226 100644
--- a/include/soc/qcom/rpmh.h
+++ b/include/soc/qcom/rpmh.h
@@ -27,6 +27,8 @@ int rpmh_invalidate(const struct device *dev);
 int rpmh_write_pdc_data(const struct device *dev,
 			const struct tcs_cmd *cmd, u32 n);
 
+int rpmh_ctrlr_idle(const struct device *dev);
+
 #else
 
 static inline int rpmh_write(const struct device *dev, enum rpmh_state state,
@@ -53,6 +55,9 @@ static inline int rpmh_write_pdc_data(const struct device *dev,
 				      const struct tcs_cmd *cmd, u32 n)
 { return -ENODEV; }
 
+static inline int rpmh_ctrlr_idle(const struct device *dev)
+{ return -ENODEV; }
+
 #endif /* CONFIG_QCOM_RPMH */
 
 #endif /* __SOC_QCOM_RPMH_H__ */
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of the Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH RESEND v3 3/3] drivers: qcom: rpmh: disallow active requests in solver mode
  2019-02-21 12:18 [PATCH RESEND v3 0/3] add some more functionality to RPMH Raju P.L.S.S.S.N
  2019-02-21 12:18 ` [PATCH RESEND v3 1/3] drivers: qcom: rpmh-rsc: simplify TCS locking Raju P.L.S.S.S.N
  2019-02-21 12:18 ` [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle Raju P.L.S.S.S.N
@ 2019-02-21 12:18 ` Raju P.L.S.S.S.N
  2 siblings, 0 replies; 12+ messages in thread
From: Raju P.L.S.S.S.N @ 2019-02-21 12:18 UTC (permalink / raw)
  To: andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, linux-pm, sboyd, evgreen,
	dianders, mka, ilina, Raju P . L . S . S . S . N

From: Lina Iyer <ilina@codeaurora.org>

Controllers may be in 'solver' state, where they could be in autonomous
mode executing low power modes for their hardware and as such are not
available for sending active votes. Device driver may notify RPMH API
that the controller is in solver mode and when in such mode, disallow
requests from platform drivers for state change using the RSC.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
---
 drivers/soc/qcom/rpmh-internal.h |  5 ++++
 drivers/soc/qcom/rpmh-rsc.c      | 30 +++++++++++++++++++
 drivers/soc/qcom/rpmh.c          | 51 ++++++++++++++++++++++++++++++++
 include/soc/qcom/rpmh.h          |  5 ++++
 4 files changed, 91 insertions(+)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 4b891c23b4cd..0923c45216a8 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -70,12 +70,14 @@ struct rpmh_request {
  * @cache_lock: synchronize access to the cache data
  * @dirty: was the cache updated since flush
  * @batch_cache: Cache sleep and wake requests sent as batch
+ * @in_solver_mode: Controller is busy in solver mode
  */
 struct rpmh_ctrlr {
 	struct list_head cache;
 	spinlock_t cache_lock;
 	bool dirty;
 	struct list_head batch_cache;
+	bool in_solver_mode;
 };
 
 /**
@@ -86,6 +88,7 @@ struct rpmh_ctrlr {
  * @base:       start address of the RSC's DRV registers
  * @tcs_base:   start address of the TCS registers in this controller
  * @id:         instance id in the controller (Direct Resource Voter)
+ * @in_solver_mode: Controller is in solver mode
  * @num_tcs:    number of TCSes in this DRV
  * @tcs:        TCS groups
  * @tcs_in_use: s/w state of the TCS
@@ -97,6 +100,7 @@ struct rsc_drv {
 	void __iomem *base;
 	void __iomem *tcs_base;
 	int id;
+	bool in_solver_mode;
 	int num_tcs;
 	struct tcs_group tcs[TCS_TYPE_NR];
 	DECLARE_BITMAP(tcs_in_use, MAX_TCS_NR);
@@ -107,6 +111,7 @@ struct rsc_drv {
 int rpmh_rsc_send_data(struct rsc_drv *drv, const struct tcs_request *msg);
 int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
 			     const struct tcs_request *msg);
+void rpmh_rsc_mode_solver_set(struct rsc_drv *drv, bool enable);
 int rpmh_rsc_invalidate(struct rsc_drv *drv);
 int rpmh_rsc_write_pdc_data(struct rsc_drv *drv, const struct tcs_request *msg);
 bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv);
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index 9cc303e88a06..10b3102298c7 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -386,6 +386,11 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg)
 		return PTR_ERR(tcs);
 
 	spin_lock(&drv->lock);
+
+	if (msg->state == RPMH_ACTIVE_ONLY_STATE && drv->in_solver_mode) {
+		ret = -EINVAL;
+		goto done_write;
+	}
 	/*
 	 * The h/w does not like if we send a request to the same address,
 	 * when one is already in-flight or being processed.
@@ -524,6 +529,30 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)
 	return ret;
 }
 
+/**
+ *  rpmh_rsc_mode_solver_set: Enable/disable solver mode
+ *
+ *  @drv: The controller
+ *
+ *  enable: boolean state to be set - true/false
+ */
+void rpmh_rsc_mode_solver_set(struct rsc_drv *drv, bool enable)
+{
+	int m;
+	struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS);
+
+again:
+	spin_lock(&drv->lock);
+	for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
+		if (!tcs_is_free(drv, m)) {
+			spin_unlock(&drv->lock);
+			goto again;
+		}
+	}
+	drv->in_solver_mode = enable;
+	spin_unlock(&drv->lock);
+}
+
 /**
  *  rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
  *
@@ -718,6 +747,7 @@ static int rpmh_rsc_probe(struct platform_device *pdev)
 		return ret;
 
 	spin_lock_init(&drv->lock);
+	drv->in_solver_mode = false;
 	bitmap_zero(drv->tcs_in_use, MAX_TCS_NR);
 
 	irq = platform_get_irq(pdev, drv->id);
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 43eb9816041a..e6e98d461121 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -5,6 +5,7 @@
 
 #include <linux/atomic.h>
 #include <linux/bug.h>
+#include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
@@ -75,6 +76,42 @@ static struct rpmh_ctrlr *get_rpmh_ctrlr(const struct device *dev)
 	return &drv->client;
 }
 
+static int check_ctrlr_state(struct rpmh_ctrlr *ctrlr, enum rpmh_state state)
+{
+	int ret = 0;
+
+	/* Do not allow setting active votes when in solver mode */
+	spin_lock(&ctrlr->cache_lock);
+	if (ctrlr->in_solver_mode && state == RPMH_ACTIVE_ONLY_STATE)
+		ret = -EBUSY;
+	spin_unlock(&ctrlr->cache_lock);
+
+	return ret;
+}
+
+/**
+ * rpmh_mode_solver_set: Indicate that the RSC controller hardware has
+ * been configured to be in solver mode
+ *
+ * @dev: the device making the request
+ * @enable: Boolean value indicating if the controller is in solver mode.
+ *
+ * When solver mode is enabled, passthru API will not be able to send wake
+ * votes, just awake and active votes.
+ */
+int rpmh_mode_solver_set(const struct device *dev, bool enable)
+{
+	struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
+
+	spin_lock(&ctrlr->cache_lock);
+	rpmh_rsc_mode_solver_set(ctrlr_to_drv(ctrlr), enable);
+	ctrlr->in_solver_mode = enable;
+	spin_unlock(&ctrlr->cache_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(rpmh_mode_solver_set);
+
 void rpmh_tx_done(const struct tcs_request *msg, int r)
 {
 	struct rpmh_request *rpm_msg = container_of(msg, struct rpmh_request,
@@ -230,8 +267,13 @@ int rpmh_write_async(const struct device *dev, enum rpmh_state state,
 		     const struct tcs_cmd *cmd, u32 n)
 {
 	struct rpmh_request *rpm_msg;
+	struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
 	int ret;
 
+	ret = check_ctrlr_state(ctrlr, state);
+	if (ret)
+		return ret;
+
 	rpm_msg = kzalloc(sizeof(*rpm_msg), GFP_ATOMIC);
 	if (!rpm_msg)
 		return -ENOMEM;
@@ -262,11 +304,16 @@ int rpmh_write(const struct device *dev, enum rpmh_state state,
 {
 	DECLARE_COMPLETION_ONSTACK(compl);
 	DEFINE_RPMH_MSG_ONSTACK(dev, state, &compl, rpm_msg);
+	struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
 	int ret;
 
 	if (!cmd || !n || n > MAX_RPMH_PAYLOAD)
 		return -EINVAL;
 
+	ret = check_ctrlr_state(ctrlr, state);
+	if (ret)
+		return ret;
+
 	memcpy(rpm_msg.cmd, cmd, n * sizeof(*cmd));
 	rpm_msg.msg.num_cmds = n;
 
@@ -353,6 +400,10 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state,
 	if (!cmd || !n)
 		return -EINVAL;
 
+	ret = check_ctrlr_state(ctrlr, state);
+	if (ret)
+		return ret;
+
 	while (n[count] > 0)
 		count++;
 	if (!count)
diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h
index 4c4b013f0226..e8356b659c7d 100644
--- a/include/soc/qcom/rpmh.h
+++ b/include/soc/qcom/rpmh.h
@@ -29,6 +29,8 @@ int rpmh_write_pdc_data(const struct device *dev,
 
 int rpmh_ctrlr_idle(const struct device *dev);
 
+int rpmh_mode_solver_set(const struct device *dev, bool enable);
+
 #else
 
 static inline int rpmh_write(const struct device *dev, enum rpmh_state state,
@@ -58,6 +60,9 @@ static inline int rpmh_write_pdc_data(const struct device *dev,
 static inline int rpmh_ctrlr_idle(const struct device *dev)
 { return -ENODEV; }
 
+static inline int rpmh_mode_solver_set(const struct device *dev, bool enable)
+{ return -ENODEV; }
+
 #endif /* CONFIG_QCOM_RPMH */
 
 #endif /* __SOC_QCOM_RPMH_H__ */
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of the Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle
  2019-02-21 12:18 ` [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle Raju P.L.S.S.S.N
@ 2019-02-27  0:49     ` Stephen Boyd
  0 siblings, 0 replies; 12+ messages in thread
From: Stephen Boyd @ 2019-02-27  0:49 UTC (permalink / raw)
  To: andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, linux-pm, evgreen,
	dianders, mka, ilina, Raju P . L . S . S . S . N

Quoting Raju P.L.S.S.S.N (2019-02-21 04:18:26)
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index d6b834eeeb37..9cc303e88a06 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -524,6 +524,30 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)
>         return ret;
>  }
>  
> +/**
> + *  rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
> + *
> + *  @drv: The controller
> + *
> + *  Returns true if the TCSes are engaged in handling requests.
> + */
> +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv)
> +{

This API seems inherently racy. How do we know that nothing else is
going to be inserted into the TCS after this function returns true? Do
you have a user of this API? It would be good to know how it is used
instead of adding some code that never gets called.

> +       int m;
> +       struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS);
> +
> +       spin_lock(&drv->lock);
> +       for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
> +               if (!tcs_is_free(drv, m)) {
> +                       spin_unlock(&drv->lock);
> +                       return false;
> +               }
> +       }
> +       spin_unlock(&drv->lock);
> +
> +       return true;
> +}
> +
>  /**
>   * rpmh_rsc_write_ctrl_data: Write request to the controller
>   *

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle
@ 2019-02-27  0:49     ` Stephen Boyd
  0 siblings, 0 replies; 12+ messages in thread
From: Stephen Boyd @ 2019-02-27  0:49 UTC (permalink / raw)
  To: Raju P.L.S.S.S.N, andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, linux-pm, evgreen,
	dianders, mka, ilina, Raju P . L . S . S . S . N

Quoting Raju P.L.S.S.S.N (2019-02-21 04:18:26)
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index d6b834eeeb37..9cc303e88a06 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -524,6 +524,30 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)
>         return ret;
>  }
>  
> +/**
> + *  rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
> + *
> + *  @drv: The controller
> + *
> + *  Returns true if the TCSes are engaged in handling requests.
> + */
> +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv)
> +{

This API seems inherently racy. How do we know that nothing else is
going to be inserted into the TCS after this function returns true? Do
you have a user of this API? It would be good to know how it is used
instead of adding some code that never gets called.

> +       int m;
> +       struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS);
> +
> +       spin_lock(&drv->lock);
> +       for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
> +               if (!tcs_is_free(drv, m)) {
> +                       spin_unlock(&drv->lock);
> +                       return false;
> +               }
> +       }
> +       spin_unlock(&drv->lock);
> +
> +       return true;
> +}
> +
>  /**
>   * rpmh_rsc_write_ctrl_data: Write request to the controller
>   *


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle
  2019-02-27  0:49     ` Stephen Boyd
  (?)
@ 2019-02-27 22:29     ` Lina Iyer
  2019-03-01 17:58       ` Stephen Boyd
  -1 siblings, 1 reply; 12+ messages in thread
From: Lina Iyer @ 2019-02-27 22:29 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Raju P.L.S.S.S.N, andy.gross, david.brown, linux-arm-msm,
	linux-soc, rnayak, bjorn.andersson, linux-kernel, linux-pm,
	evgreen, dianders, mka

Hi Stephen,

On Tue, Feb 26 2019 at 17:49 -0700, Stephen Boyd wrote:
>Quoting Raju P.L.S.S.S.N (2019-02-21 04:18:26)
>> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
>> index d6b834eeeb37..9cc303e88a06 100644
>> --- a/drivers/soc/qcom/rpmh-rsc.c
>> +++ b/drivers/soc/qcom/rpmh-rsc.c
>> @@ -524,6 +524,30 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)
>>         return ret;
>>  }
>>
>> +/**
>> + *  rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
>> + *
>> + *  @drv: The controller
>> + *
>> + *  Returns true if the TCSes are engaged in handling requests.
>> + */
>> +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv)
>> +{
>
>This API seems inherently racy. How do we know that nothing else is
>going to be inserted into the TCS after this function returns true? Do
>you have a user of this API? It would be good to know how it is used
>instead of adding some code that never gets called.
>
This API is called from the last CPU that is powering down in an
interrupt locked context (say during suspend). If we are waiting on a
request, we would bail out of the suspend process. There can be no issue
requested during the last step in suspend. The PM driver itself does not
make any TCS request. Currently, this API is used by the downstream code
in its last man activities. The usage by platform coordinated mode is
still under discussion.

-- Lina
>> +       int m;
>> +       struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS);
>> +
>> +       spin_lock(&drv->lock);
>> +       for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
>> +               if (!tcs_is_free(drv, m)) {
>> +                       spin_unlock(&drv->lock);
>> +                       return false;
>> +               }
>> +       }
>> +       spin_unlock(&drv->lock);
>> +
>> +       return true;
>> +}
>> +
>>  /**
>>   * rpmh_rsc_write_ctrl_data: Write request to the controller
>>   *
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle
  2019-02-27 22:29     ` Lina Iyer
@ 2019-03-01 17:58       ` Stephen Boyd
  2019-03-04 17:14         ` Lina Iyer
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Boyd @ 2019-03-01 17:58 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Raju P.L.S.S.S.N, andy.gross, david.brown, linux-arm-msm,
	linux-soc, rnayak, bjorn.andersson, linux-kernel, linux-pm,
	evgreen, dianders, mka

Quoting Lina Iyer (2019-02-27 14:29:13)
> Hi Stephen,
> 
> On Tue, Feb 26 2019 at 17:49 -0700, Stephen Boyd wrote:
> >Quoting Raju P.L.S.S.S.N (2019-02-21 04:18:26)
> >> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> >> index d6b834eeeb37..9cc303e88a06 100644
> >> --- a/drivers/soc/qcom/rpmh-rsc.c
> >> +++ b/drivers/soc/qcom/rpmh-rsc.c
> >> @@ -524,6 +524,30 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)
> >>         return ret;
> >>  }
> >>
> >> +/**
> >> + *  rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
> >> + *
> >> + *  @drv: The controller
> >> + *
> >> + *  Returns true if the TCSes are engaged in handling requests.

By the way, this says AMCs are busy and then TCSes are engaged. Which
one is it?

> >> + */
> >> +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv)
> >> +{
> >
> >This API seems inherently racy. How do we know that nothing else is
> >going to be inserted into the TCS after this function returns true? Do
> >you have a user of this API? It would be good to know how it is used
> >instead of adding some code that never gets called.
> >
> This API is called from the last CPU that is powering down in an
> interrupt locked context (say during suspend). If we are waiting on a
> request, we would bail out of the suspend process. There can be no issue
> requested during the last step in suspend. The PM driver itself does not
> make any TCS request. Currently, this API is used by the downstream code
> in its last man activities. The usage by platform coordinated mode is
> still under discussion.
> 

Ok, can you explain why it's even a problem for the TCSes to be active
during suspend? I would hope that for suspend/resume, if this is
actually a problem, the RPMh driver itself can block suspend with a
driver suspend callback that checks for idleness. But I suspect that in
the system wide suspend/resume case, any callers that could make TCS
requests are child devices of the RPMh controller and therefore they
would already be suspended if they didn't have anything pending they're
waiting for a response on or they would be blocking suspend themselves
if they're waiting for the response. So why are we even checking the
TCSes in system suspend path at all? Assume that callers know what
they're doing and will block suspend if they care?

Following that same logic, is this more of an API that is planned for
use by CPU idle? Where the case is much more of a runtime PM design.
Even then, I don't get it. A device that's runtime active and making
RPMh requests might need to block some forms of CPU idle states because
a request hasn't been processed yet that may change the decision for
certain deep idle states?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle
  2019-03-01 17:58       ` Stephen Boyd
@ 2019-03-04 17:14         ` Lina Iyer
  2019-03-06 22:12           ` Stephen Boyd
  0 siblings, 1 reply; 12+ messages in thread
From: Lina Iyer @ 2019-03-04 17:14 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Raju P.L.S.S.S.N, andy.gross, david.brown, linux-arm-msm,
	linux-soc, rnayak, bjorn.andersson, linux-kernel, linux-pm,
	evgreen, dianders, mka

On Fri, Mar 01 2019 at 10:58 -0700, Stephen Boyd wrote:
>Quoting Lina Iyer (2019-02-27 14:29:13)
>> Hi Stephen,
>>
>> On Tue, Feb 26 2019 at 17:49 -0700, Stephen Boyd wrote:
>> >Quoting Raju P.L.S.S.S.N (2019-02-21 04:18:26)
>> >> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
>> >> index d6b834eeeb37..9cc303e88a06 100644
>> >> --- a/drivers/soc/qcom/rpmh-rsc.c
>> >> +++ b/drivers/soc/qcom/rpmh-rsc.c
>> >> @@ -524,6 +524,30 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)
>> >>         return ret;
>> >>  }
>> >>
>> >> +/**
>> >> + *  rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
>> >> + *
>> >> + *  @drv: The controller
>> >> + *
>> >> + *  Returns true if the TCSes are engaged in handling requests.
>
>By the way, this says AMCs are busy and then TCSes are engaged. Which
>one is it?
>
>> >> + */
>> >> +bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv)
>> >> +{
>> >
>> >This API seems inherently racy. How do we know that nothing else is
>> >going to be inserted into the TCS after this function returns true? Do
>> >you have a user of this API? It would be good to know how it is used
>> >instead of adding some code that never gets called.
>> >
>> This API is called from the last CPU that is powering down in an
>> interrupt locked context (say during suspend). If we are waiting on a
>> request, we would bail out of the suspend process. There can be no issue
>> requested during the last step in suspend. The PM driver itself does not
>> make any TCS request. Currently, this API is used by the downstream code
>> in its last man activities. The usage by platform coordinated mode is
>> still under discussion.
>>
>
>Ok, can you explain why it's even a problem for the TCSes to be active
>during suspend? I would hope that for suspend/resume, if this is
>actually a problem, the RPMh driver itself can block suspend with a
>driver suspend callback that checks for idleness.
The RSC can transmit TCS executed from Linux and when all the CPUs have
powered down, could execute a firmware in the RSC to deliver the sleep
state requests. The firmware cannot run when there are active requests
being processed. To ensure that case, we bail out of sleep or suspend,
when the last CPU is powering down, if there are active requests.

>But I suspect that in
>the system wide suspend/resume case, any callers that could make TCS
>requests are child devices of the RPMh controller and therefore they
>would already be suspended if they didn't have anything pending they're
>waiting for a response on or they would be blocking suspend themselves
>if they're waiting for the response. So why are we even checking the
>TCSes in system suspend path at all? Assume that callers know what
>they're doing and will block suspend if they care?
>
In suspend, they probably would do what you mention above. All CPUs
might conincidentally be idle at the same idle, when a request is being
processed.

>Following that same logic, is this more of an API that is planned for
>use by CPU idle? Where the case is much more of a runtime PM design.
>Even then, I don't get it. A device that's runtime active and making
>RPMh requests might need to block some forms of CPU idle states because
>a request hasn't been processed yet that may change the decision for
>certain deep idle states?
>
A process waiting on a RPMH request, may let the CPU go to sleep and
therefore this is a possibility.

--Lina

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle
  2019-03-04 17:14         ` Lina Iyer
@ 2019-03-06 22:12           ` Stephen Boyd
  2019-03-06 22:19             ` Lina Iyer
  0 siblings, 1 reply; 12+ messages in thread
From: Stephen Boyd @ 2019-03-06 22:12 UTC (permalink / raw)
  To: Lina Iyer
  Cc: Raju P.L.S.S.S.N, andy.gross, david.brown, linux-arm-msm,
	linux-soc, rnayak, bjorn.andersson, linux-kernel, linux-pm,
	evgreen, dianders, mka

Quoting Lina Iyer (2019-03-04 09:14:50)
> On Fri, Mar 01 2019 at 10:58 -0700, Stephen Boyd wrote:
> >Quoting Lina Iyer (2019-02-27 14:29:13)
> >> Hi Stephen,
> >>
> >> On Tue, Feb 26 2019 at 17:49 -0700, Stephen Boyd wrote:
> >
> >Ok, can you explain why it's even a problem for the TCSes to be active
> >during suspend? I would hope that for suspend/resume, if this is
> >actually a problem, the RPMh driver itself can block suspend with a
> >driver suspend callback that checks for idleness.
> The RSC can transmit TCS executed from Linux and when all the CPUs have
> powered down, could execute a firmware in the RSC to deliver the sleep
> state requests. The firmware cannot run when there are active requests
> being processed. To ensure that case, we bail out of sleep or suspend,
> when the last CPU is powering down, if there are active requests.

Ok, do we actually bail out or just pick a shallower idle state that
wouldn't trigger the firmware to run something that may conflict with
the active requests (i.e. some light CPU sleep mode)? The commit text
seems to imply we block certain idle states.

> 
> >But I suspect that in
> >the system wide suspend/resume case, any callers that could make TCS
> >requests are child devices of the RPMh controller and therefore they
> >would already be suspended if they didn't have anything pending they're
> >waiting for a response on or they would be blocking suspend themselves
> >if they're waiting for the response. So why are we even checking the
> >TCSes in system suspend path at all? Assume that callers know what
> >they're doing and will block suspend if they care?
> >
> In suspend, they probably would do what you mention above. All CPUs
> might conincidentally be idle at the same idle, when a request is being
> processed.
> 
> >Following that same logic, is this more of an API that is planned for
> >use by CPU idle? Where the case is much more of a runtime PM design.
> >Even then, I don't get it. A device that's runtime active and making
> >RPMh requests might need to block some forms of CPU idle states because
> >a request hasn't been processed yet that may change the decision for
> >certain deep idle states?
> >
> A process waiting on a RPMH request, may let the CPU go to sleep and
> therefore this is a possibility.
> 

Ok thanks for the info. Can these details be included in the commit text
so we don't lose sight of the bigger picture? And can this patch series
be combined with a larger cpuidle/suspend patch series so we don't have
to review this in isolation? I don't understand the need to add more
APIs that aren't used yet.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle
  2019-03-06 22:12           ` Stephen Boyd
@ 2019-03-06 22:19             ` Lina Iyer
  0 siblings, 0 replies; 12+ messages in thread
From: Lina Iyer @ 2019-03-06 22:19 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Raju P.L.S.S.S.N, andy.gross, david.brown, linux-arm-msm,
	linux-soc, rnayak, bjorn.andersson, linux-kernel, linux-pm,
	evgreen, dianders, mka

On Wed, Mar 06 2019 at 15:12 -0700, Stephen Boyd wrote:
>Quoting Lina Iyer (2019-03-04 09:14:50)
>> On Fri, Mar 01 2019 at 10:58 -0700, Stephen Boyd wrote:
>> >Quoting Lina Iyer (2019-02-27 14:29:13)
>> >> Hi Stephen,
>> >>
>> >> On Tue, Feb 26 2019 at 17:49 -0700, Stephen Boyd wrote:
>> >
>> >Ok, can you explain why it's even a problem for the TCSes to be active
>> >during suspend? I would hope that for suspend/resume, if this is
>> >actually a problem, the RPMh driver itself can block suspend with a
>> >driver suspend callback that checks for idleness.
>> The RSC can transmit TCS executed from Linux and when all the CPUs have
>> powered down, could execute a firmware in the RSC to deliver the sleep
>> state requests. The firmware cannot run when there are active requests
>> being processed. To ensure that case, we bail out of sleep or suspend,
>> when the last CPU is powering down, if there are active requests.
>
>Ok, do we actually bail out or just pick a shallower idle state that
>wouldn't trigger the firmware to run something that may conflict with
>the active requests (i.e. some light CPU sleep mode)? The commit text
>seems to imply we block certain idle states.
>
We bail out of idle and let cpuidle determine the state again. We don't
go into a shallower state.
>>
>> >But I suspect that in
>> >the system wide suspend/resume case, any callers that could make TCS
>> >requests are child devices of the RPMh controller and therefore they
>> >would already be suspended if they didn't have anything pending they're
>> >waiting for a response on or they would be blocking suspend themselves
>> >if they're waiting for the response. So why are we even checking the
>> >TCSes in system suspend path at all? Assume that callers know what
>> >they're doing and will block suspend if they care?
>> >
>> In suspend, they probably would do what you mention above. All CPUs
>> might conincidentally be idle at the same idle, when a request is being
>> processed.
>>
>> >Following that same logic, is this more of an API that is planned for
>> >use by CPU idle? Where the case is much more of a runtime PM design.
>> >Even then, I don't get it. A device that's runtime active and making
>> >RPMh requests might need to block some forms of CPU idle states because
>> >a request hasn't been processed yet that may change the decision for
>> >certain deep idle states?
>> >
>> A process waiting on a RPMH request, may let the CPU go to sleep and
>> therefore this is a possibility.
>>
>
>Ok thanks for the info. Can these details be included in the commit text
>so we don't lose sight of the bigger picture? And can this patch series
>be combined with a larger cpuidle/suspend patch series so we don't have
>to review this in isolation? I don't understand the need to add more
>APIs that aren't used yet.
>
Agreed.

--Lina

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle
  2018-10-09  6:36 [PATCH RESEND v3 0/3] drivers/qcom: add additional functionality to RPMH Raju P.L.S.S.S.N
@ 2018-10-09  6:36 ` Raju P.L.S.S.S.N
  0 siblings, 0 replies; 12+ messages in thread
From: Raju P.L.S.S.S.N @ 2018-10-09  6:36 UTC (permalink / raw)
  To: andy.gross, david.brown, linux-arm-msm, linux-soc
  Cc: rnayak, bjorn.andersson, linux-kernel, linux-pm, sboyd, evgreen,
	dianders, mka, ilina, Raju P.L.S.S.S.N

From: Lina Iyer <ilina@codeaurora.org>

Allow the controller status be queried. The controller is busy if it is
actively processing request. Also allow the controller state be read by
platform drivers. This is useful for PM drivers which can choose to
disallow idle modes when the controller is busy.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
---
changes in v3
- Add lock check to avoid potential race as suggested by Matthias
---
 drivers/soc/qcom/rpmh-internal.h |  1 +
 drivers/soc/qcom/rpmh-rsc.c      | 24 ++++++++++++++++++++++++
 drivers/soc/qcom/rpmh.c          | 13 +++++++++++++
 include/soc/qcom/rpmh.h          |  5 +++++
 4 files changed, 43 insertions(+)

diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h
index 2e3ffcd..4b891c23 100644
--- a/drivers/soc/qcom/rpmh-internal.h
+++ b/drivers/soc/qcom/rpmh-internal.h
@@ -109,6 +109,7 @@ int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv,
 			     const struct tcs_request *msg);
 int rpmh_rsc_invalidate(struct rsc_drv *drv);
 int rpmh_rsc_write_pdc_data(struct rsc_drv *drv, const struct tcs_request *msg);
+bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv);
 void rpmh_tx_done(const struct tcs_request *msg, int r);
 
 #endif /* __RPM_INTERNAL_H__ */
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index d6b834e..9cc303e 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -525,6 +525,30 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg)
 }
 
 /**
+ *  rpmh_rsc_ctrlr_is_idle: Check if any of the AMCs are busy.
+ *
+ *  @drv: The controller
+ *
+ *  Returns true if the TCSes are engaged in handling requests.
+ */
+bool rpmh_rsc_ctrlr_is_idle(struct rsc_drv *drv)
+{
+	int m;
+	struct tcs_group *tcs = get_tcs_of_type(drv, ACTIVE_TCS);
+
+	spin_lock(&drv->lock);
+	for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
+		if (!tcs_is_free(drv, m)) {
+			spin_unlock(&drv->lock);
+			return false;
+		}
+	}
+	spin_unlock(&drv->lock);
+
+	return true;
+}
+
+/**
  * rpmh_rsc_write_ctrl_data: Write request to the controller
  *
  * @drv: the controller
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 297d6cc..43eb981 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -535,3 +535,16 @@ int rpmh_invalidate(const struct device *dev)
 	return ret;
 }
 EXPORT_SYMBOL(rpmh_invalidate);
+
+/**
+ * rpmh_ctrlr_idle: Return the controller idle status
+ *
+ * @dev: the device making the request
+ */
+int rpmh_ctrlr_idle(const struct device *dev)
+{
+	struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
+
+	return rpmh_rsc_ctrlr_is_idle(ctrlr_to_drv(ctrlr));
+}
+EXPORT_SYMBOL(rpmh_ctrlr_idle);
diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h
index b05e31a..4c4b013 100644
--- a/include/soc/qcom/rpmh.h
+++ b/include/soc/qcom/rpmh.h
@@ -27,6 +27,8 @@ int rpmh_write_batch(const struct device *dev, enum rpmh_state state,
 int rpmh_write_pdc_data(const struct device *dev,
 			const struct tcs_cmd *cmd, u32 n);
 
+int rpmh_ctrlr_idle(const struct device *dev);
+
 #else
 
 static inline int rpmh_write(const struct device *dev, enum rpmh_state state,
@@ -53,6 +55,9 @@ static inline int rpmh_write_pdc_data(const struct device *dev,
 				      const struct tcs_cmd *cmd, u32 n)
 { return -ENODEV; }
 
+static inline int rpmh_ctrlr_idle(const struct device *dev)
+{ return -ENODEV; }
+
 #endif /* CONFIG_QCOM_RPMH */
 
 #endif /* __SOC_QCOM_RPMH_H__ */
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of the Code Aurora Forum, hosted by The Linux Foundation.

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-03-06 22:19 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-21 12:18 [PATCH RESEND v3 0/3] add some more functionality to RPMH Raju P.L.S.S.S.N
2019-02-21 12:18 ` [PATCH RESEND v3 1/3] drivers: qcom: rpmh-rsc: simplify TCS locking Raju P.L.S.S.S.N
2019-02-21 12:18 ` [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle Raju P.L.S.S.S.N
2019-02-27  0:49   ` Stephen Boyd
2019-02-27  0:49     ` Stephen Boyd
2019-02-27 22:29     ` Lina Iyer
2019-03-01 17:58       ` Stephen Boyd
2019-03-04 17:14         ` Lina Iyer
2019-03-06 22:12           ` Stephen Boyd
2019-03-06 22:19             ` Lina Iyer
2019-02-21 12:18 ` [PATCH RESEND v3 3/3] drivers: qcom: rpmh: disallow active requests in solver mode Raju P.L.S.S.S.N
  -- strict thread matches above, loose matches on Subject: below --
2018-10-09  6:36 [PATCH RESEND v3 0/3] drivers/qcom: add additional functionality to RPMH Raju P.L.S.S.S.N
2018-10-09  6:36 ` [PATCH RESEND v3 2/3] drivers: qcom: rpmh-rsc: return if the controller is idle Raju P.L.S.S.S.N

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.