| Message ID | 20210810173006.202673615@linuxfoundation.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
On 10.08.21 20:31, Greg Kroah-Hartman wrote: > From: Mike Tipton <mdtipton@codeaurora.org> > > commit f84f5b6f72e68bbaeb850b58ac167e4a3a47532a 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. > > Fixes: 976daac4a1c5 ("interconnect: qcom: Consolidate interconnect RPMh support") > Signed-off-by: Mike Tipton <mdtipton@codeaurora.org> > Link: https://lore.kernel.org/r/20210721175432.2119-5-mdtipton@codeaurora.org > Signed-off-by: Georgi Djakov <djakov@kernel.org> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Hello Greg, Please drop this patch, as people are reporting issues on some platforms. So please do not apply it to any stable trees yet (5.10 and 5.13). I will send a revert (or other fix) to you soon. Thanks, Georgi > --- > drivers/interconnect/qcom/icc-rpmh.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > --- 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_n > { > 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 * > { > 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 * > *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); > >
On Wed, Aug 11, 2021 at 06:50:12PM +0300, Georgi Djakov wrote: > On 10.08.21 20:31, Greg Kroah-Hartman wrote: > > From: Mike Tipton <mdtipton@codeaurora.org> > > > > commit f84f5b6f72e68bbaeb850b58ac167e4a3a47532a 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. > > > > Fixes: 976daac4a1c5 ("interconnect: qcom: Consolidate interconnect RPMh support") > > Signed-off-by: Mike Tipton <mdtipton@codeaurora.org> > > Link: https://lore.kernel.org/r/20210721175432.2119-5-mdtipton@codeaurora.org > > Signed-off-by: Georgi Djakov <djakov@kernel.org> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Hello Greg, > > Please drop this patch, as people are reporting issues on some > platforms. So please do not apply it to any stable trees yet > (5.10 and 5.13). I will send a revert (or other fix) to you soon. Now dropped from both queues, thanks. greg k-h
--- 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_n { 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 * { 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 * *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);