Messages in this thread | ![/](/images/icornerl.gif) | | Date | Sat, 14 Jan 2023 01:40:19 +0000 | Subject | Re: [PATCH] interconnect: Skip call into provider if initial bw is zero | From | Bryan O'Donoghue <> |
| |
On 14/01/2023 01:24, Bryan O'Donoghue wrote: > On 13/01/2023 22:07, Vivek Aknurwar wrote: >> Currently framework sets bw even when init bw requirements are zero >> during >> provider registration, thus resulting bulk of set bw to hw. >> Avoid this behaviour by skipping provider set bw calls if init bw is >> zero. >> >> Signed-off-by: Vivek Aknurwar <quic_viveka@quicinc.com> >> --- >> drivers/interconnect/core.c | 17 ++++++++++------- >> 1 file changed, 10 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c >> index 25debde..43ed595 100644 >> --- a/drivers/interconnect/core.c >> +++ b/drivers/interconnect/core.c >> @@ -977,14 +977,17 @@ void icc_node_add(struct icc_node *node, struct >> icc_provider *provider) >> node->avg_bw = node->init_avg; >> node->peak_bw = node->init_peak; >> - if (provider->pre_aggregate) >> - provider->pre_aggregate(node); >> - >> - if (provider->aggregate) >> - provider->aggregate(node, 0, node->init_avg, node->init_peak, >> - &node->avg_bw, &node->peak_bw); >> + if (node->avg_bw || node->peak_bw) { >> + if (provider->pre_aggregate) >> + provider->pre_aggregate(node); >> + >> + if (provider->aggregate) >> + provider->aggregate(node, 0, node->init_avg, >> node->init_peak, >> + &node->avg_bw, &node->peak_bw); >> + if (provider->set) >> + provider->set(node, node); >> + } >> - provider->set(node, node); >> node->avg_bw = 0; >> node->peak_bw = 0; > > I have the same comment/question for this patch that I had for the qcom > arch specific version of it. This patch seems to be doing at a higher > level what the patch below was doing at a lower level. > > https://lore.kernel.org/lkml/1039a507-c4cd-e92f-dc29-1e2169ce5078@linaro.org/T/#m0c90588d0d1e2ab88c39be8f5f3a8f0b61396349 > > what happens to earlier silicon - qcom silicon which previously made > explicit zero requests ? > > https://lore.kernel.org/lkml/1039a507-c4cd-e92f-dc29-1e2169ce5078@linaro.org/T/#m589e8280de470e038249bb362634221771d845dd > > https://lkml.org/lkml/2023/1/3/1232 > > Isn't it a better idea to let lower layer drivers differentiate what > they do ? > > For example on pre 5.4 qcom kernel silicon we might choose to set the > value to zero "because that's what the reference code did" but on newer > silicon we might opt to skip the zero configuration ? > > I'm happy to be shown the error of my ways but, absent testing to *show* > it doesn't impact existing legacy silicon, I think we should be wary of > this change. > > --- > bod
Oh, and what is the effect on Samsung and i.MX silicon interconnect providers of skipping the zero set ?
--- bod
| ![\](/images/icornerr.gif) |