linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] interconnect: qcom: Misc bcm-voter changes and fixes
@ 2020-06-23  4:08 Mike Tipton
  2020-06-23  4:08 ` [PATCH 1/4] interconnect: qcom: Support bcm-voter-specific TCS wait behavior Mike Tipton
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Mike Tipton @ 2020-06-23  4:08 UTC (permalink / raw)
  To: georgi.djakov
  Cc: bjorn.andersson, agross, linux-pm, linux-arm-msm, linux-kernel,
	Mike Tipton

These changes are mostly unrelated, but there are some dependencies
between them.

Mike Tipton (4):
  interconnect: qcom: Support bcm-voter-specific TCS wait behavior
  interconnect: qcom: Only wait for completion in AMC/WAKE by default
  interconnect: qcom: Add support for per-BCM scaling factors
  interconnect: qcom: Fix small BW votes being truncated to zero

 drivers/interconnect/qcom/bcm-voter.c | 63 ++++++++++++++++++---------
 drivers/interconnect/qcom/icc-rpmh.c  |  3 ++
 drivers/interconnect/qcom/icc-rpmh.h  |  2 +
 3 files changed, 47 insertions(+), 21 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH 1/4] interconnect: qcom: Support bcm-voter-specific TCS wait behavior
  2020-06-23  4:08 [PATCH 0/4] interconnect: qcom: Misc bcm-voter changes and fixes Mike Tipton
@ 2020-06-23  4:08 ` Mike Tipton
  2020-07-02  9:02   ` Georgi Djakov
  2020-06-23  4:08 ` [PATCH 2/4] interconnect: qcom: Only wait for completion in AMC/WAKE by default Mike Tipton
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Mike Tipton @ 2020-06-23  4:08 UTC (permalink / raw)
  To: georgi.djakov
  Cc: bjorn.andersson, agross, linux-pm, linux-arm-msm, linux-kernel,
	Mike Tipton

Currently, all bcm-voters set tcs_cmd::wait=true for the last VCD
command in each TCS (AMC, WAKE, and SLEEP). However, some bcm-voters
don't need the completion and instead need to optimize for latency. For
instance, disabling wait-for-completion in the WAKE set can decrease
resume latency and allow for certain operations to occur in parallel
with the WAKE TCS triggering. This is only safe in very specific
situations. Keep the default behavior of always waiting, but allow it to
be overridden in devicetree.

Signed-off-by: Mike Tipton <mdtipton@codeaurora.org>
---
 drivers/interconnect/qcom/bcm-voter.c | 32 ++++++++++++++++++---------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c
index 2a11a63e7217..e9f66f5c8a91 100644
--- a/drivers/interconnect/qcom/bcm-voter.c
+++ b/drivers/interconnect/qcom/bcm-voter.c
@@ -27,6 +27,7 @@ static DEFINE_MUTEX(bcm_voter_lock);
  * @commit_list: list containing bcms to be committed to hardware
  * @ws_list: list containing bcms that have different wake/sleep votes
  * @voter_node: list of bcm voters
+ * @tcs_wait: mask for which buckets require TCS completion
  */
 struct bcm_voter {
 	struct device *dev;
@@ -35,6 +36,7 @@ struct bcm_voter {
 	struct list_head commit_list;
 	struct list_head ws_list;
 	struct list_head voter_node;
+	u32 tcs_wait;
 };
 
 static int cmp_vcd(void *priv, struct list_head *a, struct list_head *b)
@@ -89,7 +91,7 @@ static void bcm_aggregate(struct qcom_icc_bcm *bcm)
 }
 
 static inline void tcs_cmd_gen(struct tcs_cmd *cmd, u64 vote_x, u64 vote_y,
-			       u32 addr, bool commit)
+			       u32 addr, bool commit, bool wait)
 {
 	bool valid = true;
 
@@ -114,15 +116,16 @@ static inline void tcs_cmd_gen(struct tcs_cmd *cmd, u64 vote_x, u64 vote_y,
 	 * Set the wait for completion flag on command that need to be completed
 	 * before the next command.
 	 */
-	cmd->wait = commit;
+	cmd->wait = wait;
 }
 
-static void tcs_list_gen(struct list_head *bcm_list, int bucket,
-			 struct tcs_cmd tcs_list[MAX_BCMS],
+static void tcs_list_gen(struct bcm_voter *voter, int bucket,
+			 struct tcs_cmd tcs_list[MAX_VCD],
 			 int n[MAX_VCD + 1])
 {
+	struct list_head *bcm_list = &voter->commit_list;
 	struct qcom_icc_bcm *bcm;
-	bool commit;
+	bool commit, wait;
 	size_t idx = 0, batch = 0, cur_vcd_size = 0;
 
 	memset(n, 0, sizeof(int) * (MAX_VCD + 1));
@@ -135,8 +138,11 @@ static void tcs_list_gen(struct list_head *bcm_list, int bucket,
 			commit = true;
 			cur_vcd_size = 0;
 		}
+
+		wait = commit && (voter->tcs_wait & BIT(bucket));
+
 		tcs_cmd_gen(&tcs_list[idx], bcm->vote_x[bucket],
-			    bcm->vote_y[bucket], bcm->addr, commit);
+			    bcm->vote_y[bucket], bcm->addr, commit, wait);
 		idx++;
 		n[batch]++;
 		/*
@@ -261,8 +267,7 @@ int qcom_icc_bcm_voter_commit(struct bcm_voter *voter)
 	 * Construct the command list based on a pre ordered list of BCMs
 	 * based on VCD.
 	 */
-	tcs_list_gen(&voter->commit_list, QCOM_ICC_BUCKET_AMC, cmds, commit_idx);
-
+	tcs_list_gen(voter, QCOM_ICC_BUCKET_AMC, cmds, commit_idx);
 	if (!commit_idx[0])
 		goto out;
 
@@ -302,7 +307,7 @@ int qcom_icc_bcm_voter_commit(struct bcm_voter *voter)
 
 	list_sort(NULL, &voter->commit_list, cmp_vcd);
 
-	tcs_list_gen(&voter->commit_list, QCOM_ICC_BUCKET_WAKE, cmds, commit_idx);
+	tcs_list_gen(voter, QCOM_ICC_BUCKET_WAKE, cmds, commit_idx);
 
 	ret = rpmh_write_batch(voter->dev, RPMH_WAKE_ONLY_STATE, cmds, commit_idx);
 	if (ret) {
@@ -310,7 +315,7 @@ int qcom_icc_bcm_voter_commit(struct bcm_voter *voter)
 		goto out;
 	}
 
-	tcs_list_gen(&voter->commit_list, QCOM_ICC_BUCKET_SLEEP, cmds, commit_idx);
+	tcs_list_gen(voter, QCOM_ICC_BUCKET_SLEEP, cmds, commit_idx);
 
 	ret = rpmh_write_batch(voter->dev, RPMH_SLEEP_STATE, cmds, commit_idx);
 	if (ret) {
@@ -329,6 +334,7 @@ EXPORT_SYMBOL_GPL(qcom_icc_bcm_voter_commit);
 
 static int qcom_icc_bcm_voter_probe(struct platform_device *pdev)
 {
+	struct device_node *np = pdev->dev.of_node;
 	struct bcm_voter *voter;
 
 	voter = devm_kzalloc(&pdev->dev, sizeof(*voter), GFP_KERNEL);
@@ -336,7 +342,11 @@ static int qcom_icc_bcm_voter_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	voter->dev = &pdev->dev;
-	voter->np = pdev->dev.of_node;
+	voter->np = np;
+
+	if (of_property_read_u32(np, "qcom,tcs-wait", &voter->tcs_wait))
+		voter->tcs_wait = QCOM_ICC_TAG_ALWAYS;
+
 	mutex_init(&voter->lock);
 	INIT_LIST_HEAD(&voter->commit_list);
 	INIT_LIST_HEAD(&voter->ws_list);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH 2/4] interconnect: qcom: Only wait for completion in AMC/WAKE by default
  2020-06-23  4:08 [PATCH 0/4] interconnect: qcom: Misc bcm-voter changes and fixes Mike Tipton
  2020-06-23  4:08 ` [PATCH 1/4] interconnect: qcom: Support bcm-voter-specific TCS wait behavior Mike Tipton
@ 2020-06-23  4:08 ` Mike Tipton
  2020-06-23  4:08 ` [PATCH 3/4] interconnect: qcom: Add support for per-BCM scaling factors Mike Tipton
  2020-06-23  4:08 ` [PATCH 4/4] interconnect: qcom: Fix small BW votes being truncated to zero Mike Tipton
  3 siblings, 0 replies; 10+ messages in thread
From: Mike Tipton @ 2020-06-23  4:08 UTC (permalink / raw)
  To: georgi.djakov
  Cc: bjorn.andersson, agross, linux-pm, linux-arm-msm, linux-kernel,
	Mike Tipton

Change the default TCS wait behavior to only wait for completion in AMC
and WAKE. Waiting isn't necessary in the SLEEP TCS, since votes are only
being removed in this case. Resources can be safely disabled
asynchronously in parallel with the rest of the power collapse sequence.
This reduces the sleep entry latency.

Signed-off-by: Mike Tipton <mdtipton@codeaurora.org>
---
 drivers/interconnect/qcom/bcm-voter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c
index e9f66f5c8a91..bb83ce7554b7 100644
--- a/drivers/interconnect/qcom/bcm-voter.c
+++ b/drivers/interconnect/qcom/bcm-voter.c
@@ -345,7 +345,7 @@ static int qcom_icc_bcm_voter_probe(struct platform_device *pdev)
 	voter->np = np;
 
 	if (of_property_read_u32(np, "qcom,tcs-wait", &voter->tcs_wait))
-		voter->tcs_wait = QCOM_ICC_TAG_ALWAYS;
+		voter->tcs_wait = QCOM_ICC_TAG_ACTIVE_ONLY;
 
 	mutex_init(&voter->lock);
 	INIT_LIST_HEAD(&voter->commit_list);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH 3/4] interconnect: qcom: Add support for per-BCM scaling factors
  2020-06-23  4:08 [PATCH 0/4] interconnect: qcom: Misc bcm-voter changes and fixes Mike Tipton
  2020-06-23  4:08 ` [PATCH 1/4] interconnect: qcom: Support bcm-voter-specific TCS wait behavior Mike Tipton
  2020-06-23  4:08 ` [PATCH 2/4] interconnect: qcom: Only wait for completion in AMC/WAKE by default Mike Tipton
@ 2020-06-23  4:08 ` Mike Tipton
  2020-06-23  4:08 ` [PATCH 4/4] interconnect: qcom: Fix small BW votes being truncated to zero Mike Tipton
  3 siblings, 0 replies; 10+ messages in thread
From: Mike Tipton @ 2020-06-23  4:08 UTC (permalink / raw)
  To: georgi.djakov
  Cc: bjorn.andersson, agross, linux-pm, linux-arm-msm, linux-kernel,
	Mike Tipton

Currently, bcm-voter always assumes requests are made in KBps and that
BCM HW always wants them in Bps, so it always scales the requests by
1000. However, certain use cases and BCMs may use different units.
Thus, add support for BCM-specific scaling factors.

Signed-off-by: Mike Tipton <mdtipton@codeaurora.org>
---
 drivers/interconnect/qcom/bcm-voter.c | 4 ++--
 drivers/interconnect/qcom/icc-rpmh.c  | 3 +++
 drivers/interconnect/qcom/icc-rpmh.h  | 2 ++
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c
index bb83ce7554b7..a68c858ca6b7 100644
--- a/drivers/interconnect/qcom/bcm-voter.c
+++ b/drivers/interconnect/qcom/bcm-voter.c
@@ -72,11 +72,11 @@ static void bcm_aggregate(struct qcom_icc_bcm *bcm)
 			agg_peak[bucket] = max(agg_peak[bucket], temp);
 		}
 
-		temp = agg_avg[bucket] * 1000ULL;
+		temp = agg_avg[bucket] * bcm->vote_scale;
 		do_div(temp, bcm->aux_data.unit);
 		bcm->vote_x[bucket] = temp;
 
-		temp = agg_peak[bucket] * 1000ULL;
+		temp = agg_peak[bucket] * bcm->vote_scale;
 		do_div(temp, bcm->aux_data.unit);
 		bcm->vote_y[bucket] = temp;
 	}
diff --git a/drivers/interconnect/qcom/icc-rpmh.c b/drivers/interconnect/qcom/icc-rpmh.c
index 3ac5182c9ab2..008846c17bec 100644
--- a/drivers/interconnect/qcom/icc-rpmh.c
+++ b/drivers/interconnect/qcom/icc-rpmh.c
@@ -136,6 +136,9 @@ int qcom_icc_bcm_init(struct qcom_icc_bcm *bcm, struct device *dev)
 	INIT_LIST_HEAD(&bcm->list);
 	INIT_LIST_HEAD(&bcm->ws_list);
 
+	if (!bcm->vote_scale)
+		bcm->vote_scale = 1000;
+
 	/* Link Qnodes to their respective BCMs */
 	for (i = 0; i < bcm->num_nodes; i++) {
 		qn = bcm->nodes[i];
diff --git a/drivers/interconnect/qcom/icc-rpmh.h b/drivers/interconnect/qcom/icc-rpmh.h
index 903d25e61984..200e98be5926 100644
--- a/drivers/interconnect/qcom/icc-rpmh.h
+++ b/drivers/interconnect/qcom/icc-rpmh.h
@@ -94,6 +94,7 @@ struct qcom_icc_node {
  * @addr: address offsets used when voting to RPMH
  * @vote_x: aggregated threshold values, represents sum_bw when @type is bw bcm
  * @vote_y: aggregated threshold values, represents peak_bw when @type is bw bcm
+ * @vote_scale: scaling factor for vote_x and vote_y
  * @dirty: flag used to indicate whether the bcm needs to be committed
  * @keepalive: flag used to indicate whether a keepalive is required
  * @aux_data: auxiliary data used when calculating threshold values and
@@ -109,6 +110,7 @@ struct qcom_icc_bcm {
 	u32 addr;
 	u64 vote_x[QCOM_ICC_NUM_BUCKETS];
 	u64 vote_y[QCOM_ICC_NUM_BUCKETS];
+	u64 vote_scale;
 	bool dirty;
 	bool keepalive;
 	struct bcm_db aux_data;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH 4/4] interconnect: qcom: Fix small BW votes being truncated to zero
  2020-06-23  4:08 [PATCH 0/4] interconnect: qcom: Misc bcm-voter changes and fixes Mike Tipton
                   ` (2 preceding siblings ...)
  2020-06-23  4:08 ` [PATCH 3/4] interconnect: qcom: Add support for per-BCM scaling factors Mike Tipton
@ 2020-06-23  4:08 ` Mike Tipton
  2020-07-02 11:11   ` Georgi Djakov
  2020-07-06 16:45   ` kernel test robot
  3 siblings, 2 replies; 10+ messages in thread
From: Mike Tipton @ 2020-06-23  4:08 UTC (permalink / raw)
  To: georgi.djakov
  Cc: bjorn.andersson, agross, linux-pm, linux-arm-msm, linux-kernel,
	Mike Tipton

Small BW votes that translate to less than a single BCM unit are
currently truncated to zero. Ensure that non-zero BW requests always
result in at least a vote of 1 to BCM.

Fixes: 976daac4a1c5 ("interconnect: qcom: Consolidate interconnect RPMh support")
Signed-off-by: Mike Tipton <mdtipton@codeaurora.org>
---
 drivers/interconnect/qcom/bcm-voter.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c
index a68c858ca6b7..9e2612fe7fad 100644
--- a/drivers/interconnect/qcom/bcm-voter.c
+++ b/drivers/interconnect/qcom/bcm-voter.c
@@ -54,8 +54,20 @@ static int cmp_vcd(void *priv, struct list_head *a, struct list_head *b)
 		return 1;
 }
 
+static u64 bcm_div(u64 num, u64 base)
+{
+	/* Ensure that small votes aren't lost. */
+	if (num && num < base)
+		return 1;
+
+	do_div(num, base);
+
+	return num;
+}
+
 static void bcm_aggregate(struct qcom_icc_bcm *bcm)
 {
+	struct qcom_icc_node *node;
 	size_t i, bucket;
 	u64 agg_avg[QCOM_ICC_NUM_BUCKETS] = {0};
 	u64 agg_peak[QCOM_ICC_NUM_BUCKETS] = {0};
@@ -63,22 +75,21 @@ static void bcm_aggregate(struct qcom_icc_bcm *bcm)
 
 	for (bucket = 0; bucket < QCOM_ICC_NUM_BUCKETS; bucket++) {
 		for (i = 0; i < bcm->num_nodes; i++) {
-			temp = bcm->nodes[i]->sum_avg[bucket] * bcm->aux_data.width;
-			do_div(temp, bcm->nodes[i]->buswidth * bcm->nodes[i]->channels);
+			node = bcm->nodes[i];
+			temp = bcm_div(node->sum_avg[bucket] * bcm->aux_data.width,
+				       node->buswidth * node->channels);
 			agg_avg[bucket] = max(agg_avg[bucket], temp);
 
-			temp = bcm->nodes[i]->max_peak[bucket] * bcm->aux_data.width;
-			do_div(temp, bcm->nodes[i]->buswidth);
+			temp = bcm_div(node->max_peak[bucket] * bcm->aux_data.width,
+				       node->buswidth);
 			agg_peak[bucket] = max(agg_peak[bucket], temp);
 		}
 
 		temp = agg_avg[bucket] * bcm->vote_scale;
-		do_div(temp, bcm->aux_data.unit);
-		bcm->vote_x[bucket] = temp;
+		bcm->vote_x[bucket] = bcm_div(temp, bcm->aux_data.unit);
 
 		temp = agg_peak[bucket] * bcm->vote_scale;
-		do_div(temp, bcm->aux_data.unit);
-		bcm->vote_y[bucket] = temp;
+		bcm->vote_y[bucket] = bcm_div(temp, bcm->aux_data.unit);
 	}
 
 	if (bcm->keepalive && bcm->vote_x[QCOM_ICC_BUCKET_AMC] == 0 &&
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* Re: [PATCH 1/4] interconnect: qcom: Support bcm-voter-specific TCS wait behavior
  2020-06-23  4:08 ` [PATCH 1/4] interconnect: qcom: Support bcm-voter-specific TCS wait behavior Mike Tipton
@ 2020-07-02  9:02   ` Georgi Djakov
  2020-07-02 21:01     ` Mike Tipton
  0 siblings, 1 reply; 10+ messages in thread
From: Georgi Djakov @ 2020-07-02  9:02 UTC (permalink / raw)
  To: Mike Tipton
  Cc: bjorn.andersson, agross, linux-pm, linux-arm-msm, linux-kernel

Hi Mike,

On 6/23/20 07:08, Mike Tipton wrote:
> Currently, all bcm-voters set tcs_cmd::wait=true for the last VCD
> command in each TCS (AMC, WAKE, and SLEEP). However, some bcm-voters
> don't need the completion and instead need to optimize for latency. For
> instance, disabling wait-for-completion in the WAKE set can decrease
> resume latency and allow for certain operations to occur in parallel
> with the WAKE TCS triggering. This is only safe in very specific
> situations. Keep the default behavior of always waiting, but allow it to
> be overridden in devicetree.
> 
> Signed-off-by: Mike Tipton <mdtipton@codeaurora.org>
> ---
>  drivers/interconnect/qcom/bcm-voter.c | 32 ++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c
[..]
> @@ -336,7 +342,11 @@ static int qcom_icc_bcm_voter_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	voter->dev = &pdev->dev;
> -	voter->np = pdev->dev.of_node;
> +	voter->np = np;
> +
> +	if (of_property_read_u32(np, "qcom,tcs-wait", &voter->tcs_wait))

This DT property needs to be documented.

Thanks,
Georgi

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

* Re: [PATCH 4/4] interconnect: qcom: Fix small BW votes being truncated to zero
  2020-06-23  4:08 ` [PATCH 4/4] interconnect: qcom: Fix small BW votes being truncated to zero Mike Tipton
@ 2020-07-02 11:11   ` Georgi Djakov
  2020-07-02 21:02     ` Mike Tipton
  2020-07-06 16:45   ` kernel test robot
  1 sibling, 1 reply; 10+ messages in thread
From: Georgi Djakov @ 2020-07-02 11:11 UTC (permalink / raw)
  To: Mike Tipton
  Cc: bjorn.andersson, agross, linux-pm, linux-arm-msm, linux-kernel

Hi Mike,

On 6/23/20 07:08, Mike Tipton wrote:
> Small BW votes that translate to less than a single BCM unit are
> currently truncated to zero. Ensure that non-zero BW requests always
> result in at least a vote of 1 to BCM.
> 
> Fixes: 976daac4a1c5 ("interconnect: qcom: Consolidate interconnect RPMh support")
> Signed-off-by: Mike Tipton <mdtipton@codeaurora.org>
> ---
>  drivers/interconnect/qcom/bcm-voter.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c
> index a68c858ca6b7..9e2612fe7fad 100644
> --- a/drivers/interconnect/qcom/bcm-voter.c
> +++ b/drivers/interconnect/qcom/bcm-voter.c
> @@ -54,8 +54,20 @@ static int cmp_vcd(void *priv, struct list_head *a, struct list_head *b)
>  		return 1;
>  }
>  
> +static u64 bcm_div(u64 num, u64 base)
> +{
> +	/* Ensure that small votes aren't lost. */
> +	if (num && num < base)
> +		return 1;
> +
> +	do_div(num, base);

do_div() does a 64-by-32 division, which will truncate these to 32-bit.

> +
> +	return num;
> +}
> +
>  static void bcm_aggregate(struct qcom_icc_bcm *bcm)
>  {
> +	struct qcom_icc_node *node;
>  	size_t i, bucket;
>  	u64 agg_avg[QCOM_ICC_NUM_BUCKETS] = {0};
>  	u64 agg_peak[QCOM_ICC_NUM_BUCKETS] = {0};
> @@ -63,22 +75,21 @@ static void bcm_aggregate(struct qcom_icc_bcm *bcm)
>  
>  	for (bucket = 0; bucket < QCOM_ICC_NUM_BUCKETS; bucket++) {
>  		for (i = 0; i < bcm->num_nodes; i++) {
> -			temp = bcm->nodes[i]->sum_avg[bucket] * bcm->aux_data.width;
> -			do_div(temp, bcm->nodes[i]->buswidth * bcm->nodes[i]->channels);
> +			node = bcm->nodes[i];
> +			temp = bcm_div(node->sum_avg[bucket] * bcm->aux_data.width,
> +				       node->buswidth * node->channels);
>  			agg_avg[bucket] = max(agg_avg[bucket], temp);
>  
> -			temp = bcm->nodes[i]->max_peak[bucket] * bcm->aux_data.width;
> -			do_div(temp, bcm->nodes[i]->buswidth);
> +			temp = bcm_div(node->max_peak[bucket] * bcm->aux_data.width,
> +				       node->buswidth);
>  			agg_peak[bucket] = max(agg_peak[bucket], temp);
>  		}
>  
>  		temp = agg_avg[bucket] * bcm->vote_scale;
> -		do_div(temp, bcm->aux_data.unit);
> -		bcm->vote_x[bucket] = temp;
> +		bcm->vote_x[bucket] = bcm_div(temp, bcm->aux_data.unit);
>  
>  		temp = agg_peak[bucket] * bcm->vote_scale;
> -		do_div(temp, bcm->aux_data.unit);
> -		bcm->vote_y[bucket] = temp;
> +		bcm->vote_y[bucket] = bcm_div(temp, bcm->aux_data.unit);
>  	}
>  
>  	if (bcm->keepalive && bcm->vote_x[QCOM_ICC_BUCKET_AMC] == 0 &&
> 

The rest looks good.

Thanks,
Georgi

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

* Re: [PATCH 1/4] interconnect: qcom: Support bcm-voter-specific TCS wait behavior
  2020-07-02  9:02   ` Georgi Djakov
@ 2020-07-02 21:01     ` Mike Tipton
  0 siblings, 0 replies; 10+ messages in thread
From: Mike Tipton @ 2020-07-02 21:01 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: bjorn.andersson, agross, linux-pm, linux-arm-msm, linux-kernel

On 7/2/2020 2:02 AM, Georgi Djakov wrote:
> Hi Mike,
> 
> On 6/23/20 07:08, Mike Tipton wrote:
>> Currently, all bcm-voters set tcs_cmd::wait=true for the last VCD
>> command in each TCS (AMC, WAKE, and SLEEP). However, some bcm-voters
>> don't need the completion and instead need to optimize for latency. For
>> instance, disabling wait-for-completion in the WAKE set can decrease
>> resume latency and allow for certain operations to occur in parallel
>> with the WAKE TCS triggering. This is only safe in very specific
>> situations. Keep the default behavior of always waiting, but allow it to
>> be overridden in devicetree.
>>
>> Signed-off-by: Mike Tipton <mdtipton@codeaurora.org>
>> ---
>>   drivers/interconnect/qcom/bcm-voter.c | 32 ++++++++++++++++++---------
>>   1 file changed, 21 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c
> [..]
>> @@ -336,7 +342,11 @@ static int qcom_icc_bcm_voter_probe(struct platform_device *pdev)
>>   		return -ENOMEM;
>>   
>>   	voter->dev = &pdev->dev;
>> -	voter->np = pdev->dev.of_node;
>> +	voter->np = np;
>> +
>> +	if (of_property_read_u32(np, "qcom,tcs-wait", &voter->tcs_wait))
> 
> This DT property needs to be documented.
Whoops, will do.

> 
> Thanks,
> Georgi
> 

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

* Re: [PATCH 4/4] interconnect: qcom: Fix small BW votes being truncated to zero
  2020-07-02 11:11   ` Georgi Djakov
@ 2020-07-02 21:02     ` Mike Tipton
  0 siblings, 0 replies; 10+ messages in thread
From: Mike Tipton @ 2020-07-02 21:02 UTC (permalink / raw)
  To: Georgi Djakov
  Cc: bjorn.andersson, agross, linux-pm, linux-arm-msm, linux-kernel

On 7/2/2020 4:11 AM, Georgi Djakov wrote:
> Hi Mike,
> 
> On 6/23/20 07:08, Mike Tipton wrote:
>> Small BW votes that translate to less than a single BCM unit are
>> currently truncated to zero. Ensure that non-zero BW requests always
>> result in at least a vote of 1 to BCM.
>>
>> Fixes: 976daac4a1c5 ("interconnect: qcom: Consolidate interconnect RPMh support")
>> Signed-off-by: Mike Tipton <mdtipton@codeaurora.org>
>> ---
>>   drivers/interconnect/qcom/bcm-voter.c | 27 +++++++++++++++++++--------
>>   1 file changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c
>> index a68c858ca6b7..9e2612fe7fad 100644
>> --- a/drivers/interconnect/qcom/bcm-voter.c
>> +++ b/drivers/interconnect/qcom/bcm-voter.c
>> @@ -54,8 +54,20 @@ static int cmp_vcd(void *priv, struct list_head *a, struct list_head *b)
>>   		return 1;
>>   }
>>   
>> +static u64 bcm_div(u64 num, u64 base)
>> +{
>> +	/* Ensure that small votes aren't lost. */
>> +	if (num && num < base)
>> +		return 1;
>> +
>> +	do_div(num, base);
> 
> do_div() does a 64-by-32 division, which will truncate these to 32-bit.
I can change base to a u32. It doesn't need anything more than that.

> 
>> +
>> +	return num;
>> +}
>> +
>>   static void bcm_aggregate(struct qcom_icc_bcm *bcm)
>>   {
>> +	struct qcom_icc_node *node;
>>   	size_t i, bucket;
>>   	u64 agg_avg[QCOM_ICC_NUM_BUCKETS] = {0};
>>   	u64 agg_peak[QCOM_ICC_NUM_BUCKETS] = {0};
>> @@ -63,22 +75,21 @@ static void bcm_aggregate(struct qcom_icc_bcm *bcm)
>>   
>>   	for (bucket = 0; bucket < QCOM_ICC_NUM_BUCKETS; bucket++) {
>>   		for (i = 0; i < bcm->num_nodes; i++) {
>> -			temp = bcm->nodes[i]->sum_avg[bucket] * bcm->aux_data.width;
>> -			do_div(temp, bcm->nodes[i]->buswidth * bcm->nodes[i]->channels);
>> +			node = bcm->nodes[i];
>> +			temp = bcm_div(node->sum_avg[bucket] * bcm->aux_data.width,
>> +				       node->buswidth * node->channels);
>>   			agg_avg[bucket] = max(agg_avg[bucket], temp);
>>   
>> -			temp = bcm->nodes[i]->max_peak[bucket] * bcm->aux_data.width;
>> -			do_div(temp, bcm->nodes[i]->buswidth);
>> +			temp = bcm_div(node->max_peak[bucket] * bcm->aux_data.width,
>> +				       node->buswidth);
>>   			agg_peak[bucket] = max(agg_peak[bucket], temp);
>>   		}
>>   
>>   		temp = agg_avg[bucket] * bcm->vote_scale;
>> -		do_div(temp, bcm->aux_data.unit);
>> -		bcm->vote_x[bucket] = temp;
>> +		bcm->vote_x[bucket] = bcm_div(temp, bcm->aux_data.unit);
>>   
>>   		temp = agg_peak[bucket] * bcm->vote_scale;
>> -		do_div(temp, bcm->aux_data.unit);
>> -		bcm->vote_y[bucket] = temp;
>> +		bcm->vote_y[bucket] = bcm_div(temp, bcm->aux_data.unit);
>>   	}
>>   
>>   	if (bcm->keepalive && bcm->vote_x[QCOM_ICC_BUCKET_AMC] == 0 &&
>>
> 
> The rest looks good.
> 
> Thanks,
> Georgi
> 

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

* Re: [PATCH 4/4] interconnect: qcom: Fix small BW votes being truncated to zero
  2020-06-23  4:08 ` [PATCH 4/4] interconnect: qcom: Fix small BW votes being truncated to zero Mike Tipton
  2020-07-02 11:11   ` Georgi Djakov
@ 2020-07-06 16:45   ` kernel test robot
  1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2020-07-06 16:45 UTC (permalink / raw)
  To: Mike Tipton, georgi.djakov
  Cc: kbuild-all, bjorn.andersson, agross, linux-pm, linux-arm-msm,
	linux-kernel, Mike Tipton

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

Hi Mike,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.8-rc4 next-20200706]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mike-Tipton/interconnect-qcom-Misc-bcm-voter-changes-and-fixes/20200623-121301
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd0d718152e4c65b173070d48ea9dfc06894c3e5
config: arm64-randconfig-s032-20200706 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-14-g8fce3d7a-dirty
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

   drivers/interconnect/qcom/bcm-voter.c:79:77: sparse: sparse: restricted __le16 degrades to integer
   drivers/interconnect/qcom/bcm-voter.c:83:78: sparse: sparse: restricted __le16 degrades to integer
>> drivers/interconnect/qcom/bcm-voter.c:89:66: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned long long [usertype] base @@     got restricted __le32 [usertype] unit @@
>> drivers/interconnect/qcom/bcm-voter.c:89:66: sparse:     expected unsigned long long [usertype] base
   drivers/interconnect/qcom/bcm-voter.c:89:66: sparse:     got restricted __le32 [usertype] unit
   drivers/interconnect/qcom/bcm-voter.c:92:66: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned long long [usertype] base @@     got restricted __le32 [usertype] unit @@
   drivers/interconnect/qcom/bcm-voter.c:92:66: sparse:     expected unsigned long long [usertype] base
   drivers/interconnect/qcom/bcm-voter.c:92:66: sparse:     got restricted __le32 [usertype] unit
   drivers/interconnect/qcom/bcm-voter.c:124:21: sparse: sparse: restricted __le32 degrades to integer
   drivers/interconnect/qcom/bcm-voter.c:124:21: sparse: sparse: restricted __le32 degrades to integer

vim +89 drivers/interconnect/qcom/bcm-voter.c

    67	
    68	static void bcm_aggregate(struct qcom_icc_bcm *bcm)
    69	{
    70		struct qcom_icc_node *node;
    71		size_t i, bucket;
    72		u64 agg_avg[QCOM_ICC_NUM_BUCKETS] = {0};
    73		u64 agg_peak[QCOM_ICC_NUM_BUCKETS] = {0};
    74		u64 temp;
    75	
    76		for (bucket = 0; bucket < QCOM_ICC_NUM_BUCKETS; bucket++) {
    77			for (i = 0; i < bcm->num_nodes; i++) {
    78				node = bcm->nodes[i];
    79				temp = bcm_div(node->sum_avg[bucket] * bcm->aux_data.width,
    80					       node->buswidth * node->channels);
    81				agg_avg[bucket] = max(agg_avg[bucket], temp);
    82	
    83				temp = bcm_div(node->max_peak[bucket] * bcm->aux_data.width,
    84					       node->buswidth);
    85				agg_peak[bucket] = max(agg_peak[bucket], temp);
    86			}
    87	
    88			temp = agg_avg[bucket] * bcm->vote_scale;
  > 89			bcm->vote_x[bucket] = bcm_div(temp, bcm->aux_data.unit);
    90	
    91			temp = agg_peak[bucket] * bcm->vote_scale;
    92			bcm->vote_y[bucket] = bcm_div(temp, bcm->aux_data.unit);
    93		}
    94	
    95		if (bcm->keepalive && bcm->vote_x[QCOM_ICC_BUCKET_AMC] == 0 &&
    96		    bcm->vote_y[QCOM_ICC_BUCKET_AMC] == 0) {
    97			bcm->vote_x[QCOM_ICC_BUCKET_AMC] = 1;
    98			bcm->vote_x[QCOM_ICC_BUCKET_WAKE] = 1;
    99			bcm->vote_y[QCOM_ICC_BUCKET_AMC] = 1;
   100			bcm->vote_y[QCOM_ICC_BUCKET_WAKE] = 1;
   101		}
   102	}
   103	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37856 bytes --]

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

end of thread, other threads:[~2020-07-06 16:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-23  4:08 [PATCH 0/4] interconnect: qcom: Misc bcm-voter changes and fixes Mike Tipton
2020-06-23  4:08 ` [PATCH 1/4] interconnect: qcom: Support bcm-voter-specific TCS wait behavior Mike Tipton
2020-07-02  9:02   ` Georgi Djakov
2020-07-02 21:01     ` Mike Tipton
2020-06-23  4:08 ` [PATCH 2/4] interconnect: qcom: Only wait for completion in AMC/WAKE by default Mike Tipton
2020-06-23  4:08 ` [PATCH 3/4] interconnect: qcom: Add support for per-BCM scaling factors Mike Tipton
2020-06-23  4:08 ` [PATCH 4/4] interconnect: qcom: Fix small BW votes being truncated to zero Mike Tipton
2020-07-02 11:11   ` Georgi Djakov
2020-07-02 21:02     ` Mike Tipton
2020-07-06 16:45   ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).