Re: [5.10 PATCH] interconnect: qcom: icc-rpmh: Add BCMs to commit list in pre_aggregate

[Alex Elder]

On 9/22/22 4:18 PM, Douglas Anderson wrote:
> From: Mike Tipton <mdtipton@xxxxxxxxxxxxxx>
>
> commit b95b668eaaa2574e8ee72f143c52075e9955177e upstream.
>
> We're only adding BCMs to the commit list in aggregate(), but there are
> cases where pre_aggregate() is called without subsequently calling
> aggregate(). In particular, in icc_sync_state() when a node with initial
> BW has zero requests. Since BCMs aren't added to the commit list in
> these cases, we don't actually send the zero BW request to HW. So the
> resources remain on unnecessarily.
>
> Add BCMs to the commit list in pre_aggregate() instead, which is always
> called even when there are no requests.
>
> Signed-off-by: Mike Tipton <mdtipton@xxxxxxxxxxxxxx>
> [georgi: remove icc_sync_state for platforms with incomplete support]
> Link: https://lore.kernel.org/r/20211125174751.25317-1-djakov@xxxxxxxxxx
> Signed-off-by: Georgi Djakov <djakov@xxxxxxxxxx>
> [dianders: dropped sm8350.c which isn't present in 5.10]
> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>

Whoops, sorry about that.

Acked-by: Alex Elder <elder@xxxxxxxxxx>

> ---
> This should have been included in Alex Elder's request for patches
> picked to 5.10 [1] but it was missed. Let's finally pick it up.
>
> [1] https://lore.kernel.org/r/20220608205415.185248-3-elder@xxxxxxxxxx
>
>   drivers/interconnect/qcom/icc-rpmh.c | 10 +++++-----
>   drivers/interconnect/qcom/sm8150.c   |  1 -
>   drivers/interconnect/qcom/sm8250.c   |  1 -
>   3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/interconnect/qcom/icc-rpmh.c b/drivers/interconnect/qcom/icc-rpmh.c
> index f6fae64861ce..27cc5f03611c 100644
> --- a/drivers/interconnect/qcom/icc-rpmh.c
> +++ b/drivers/interconnect/qcom/icc-rpmh.c
> @@ -20,13 +20,18 @@ void qcom_icc_pre_aggregate(struct icc_node *node)
>   {
>   	size_t i;
>   	struct qcom_icc_node *qn;
> +	struct qcom_icc_provider *qp;
>   
>   	qn = node->data;
> +	qp = to_qcom_provider(node->provider);
>   
>   	for (i = 0; i < QCOM_ICC_NUM_BUCKETS; i++) {
>   		qn->sum_avg[i] = 0;
>   		qn->max_peak[i] = 0;
>   	}
> +
> +	for (i = 0; i < qn->num_bcms; i++)
> +		qcom_icc_bcm_voter_add(qp->voter, qn->bcms[i]);
>   }
>   EXPORT_SYMBOL_GPL(qcom_icc_pre_aggregate);
>   
> @@ -44,10 +49,8 @@ int qcom_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
>   {
>   	size_t i;
>   	struct qcom_icc_node *qn;
> -	struct qcom_icc_provider *qp;
>   
>   	qn = node->data;
> -	qp = to_qcom_provider(node->provider);
>   
>   	if (!tag)
>   		tag = QCOM_ICC_TAG_ALWAYS;
> @@ -67,9 +70,6 @@ int qcom_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
>   	*agg_avg += avg_bw;
>   	*agg_peak = max_t(u32, *agg_peak, peak_bw);
>   
> -	for (i = 0; i < qn->num_bcms; i++)
> -		qcom_icc_bcm_voter_add(qp->voter, qn->bcms[i]);
> -
>   	return 0;
>   }
>   EXPORT_SYMBOL_GPL(qcom_icc_aggregate);
> diff --git a/drivers/interconnect/qcom/sm8150.c b/drivers/interconnect/qcom/sm8150.c
> index c76b2c7f9b10..b936196c229c 100644
> --- a/drivers/interconnect/qcom/sm8150.c
> +++ b/drivers/interconnect/qcom/sm8150.c
> @@ -627,7 +627,6 @@ static struct platform_driver qnoc_driver = {
>   	.driver = {
>   		.name = "qnoc-sm8150",
>   		.of_match_table = qnoc_of_match,
> -		.sync_state = icc_sync_state,
>   	},
>   };
>   module_platform_driver(qnoc_driver);
> diff --git a/drivers/interconnect/qcom/sm8250.c b/drivers/interconnect/qcom/sm8250.c
> index cc558fec74e3..40820043c8d3 100644
> --- a/drivers/interconnect/qcom/sm8250.c
> +++ b/drivers/interconnect/qcom/sm8250.c
> @@ -643,7 +643,6 @@ static struct platform_driver qnoc_driver = {
>   	.driver = {
>   		.name = "qnoc-sm8250",
>   		.of_match_table = qnoc_of_match,
> -		.sync_state = icc_sync_state,
>   	},
>   };
>   module_platform_driver(qnoc_driver);