| Message ID | 20220922141725.5.10.1.I791715539cae1355e21827ca738b0b523a4a0f53@changeid (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | [5.10] interconnect: qcom: icc-rpmh: Add BCMs to commit list in pre_aggregate | expand |
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);
On Thu, Sep 22, 2022 at 04:28:25PM -0500, Alex Elder wrote: > 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> Now queued up, thanks. greg k-h
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);