Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <00eb82ca-8bf6-c744-d04d-96b97ce06b17@linaro.org>
Date:   Thu, 22 Sep 2022 16:28:25 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [5.10 PATCH] interconnect: qcom: icc-rpmh: Add BCMs to commit
 list in pre_aggregate
Content-Language: en-US
To:     Douglas Anderson <dianders@chromium.org>, stable@vger.kernel.org,
        gregkh@linuxfoundation.org
Cc:     swboyd@chromium.org, Mike Tipton <mdtipton@codeaurora.org>,
        Georgi Djakov <djakov@kernel.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Georgi Djakov <georgi.djakov@linaro.org>,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-pm@vger.kernel.org
References: <20220922141725.5.10.1.I791715539cae1355e21827ca738b0b523a4a0f53@changeid>
From:   Alex Elder <elder@linaro.org>
In-Reply-To: <20220922141725.5.10.1.I791715539cae1355e21827ca738b0b523a4a0f53@changeid>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 9/22/22 4:18 PM, Douglas Anderson wrote:
> From: Mike Tipton <mdtipton@codeaurora.org>
> 
> 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@codeaurora.org>
> [georgi: remove icc_sync_state for platforms with incomplete support]
> Link: https://lore.kernel.org/r/20211125174751.25317-1-djakov@kernel.org
> Signed-off-by: Georgi Djakov <djakov@kernel.org>
> [dianders: dropped sm8350.c which isn't present in 5.10]
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Whoops, sorry about that.

Acked-by: Alex Elder <elder@linaro.org>

> ---
> 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@linaro.org
> 
>   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);