| Message ID | 20220416012634.479617-1-leo.yan@linaro.org |
|---|---|
| State | Accepted |
| Commit | 76a748e2c1aa976d0c7fef872fa6ff93ce334a8a |
| Headers | show |
| Series | interconnect: qcom: msm8939: Use icc_sync_state | expand |
On 16.04.22 4:26, Leo Yan wrote: > It's fashion to use the icc_sync_state callback to notify the framework > when all consumers are probed, so that the bandwidth request doesn't > need to stay on maximum value. > > Do the same thing for msm8939 driver. I assume that you tested this with some out of tree DT? Is it public? If the consumers are not described as such in DT and/or the support in the client drivers is missing, paths might get disabled. Thanks, Georgi > Signed-off-by: Leo Yan <leo.yan@linaro.org> > --- > drivers/interconnect/qcom/msm8939.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/interconnect/qcom/msm8939.c b/drivers/interconnect/qcom/msm8939.c > index f9c2d7d3100d..ca5f611d33b0 100644 > --- a/drivers/interconnect/qcom/msm8939.c > +++ b/drivers/interconnect/qcom/msm8939.c > @@ -1423,6 +1423,7 @@ static struct platform_driver msm8939_noc_driver = { > .driver = { > .name = "qnoc-msm8939", > .of_match_table = msm8939_noc_of_match, > + .sync_state = icc_sync_state, > }, > }; > module_platform_driver(msm8939_noc_driver);
On Thu, Apr 28, 2022 at 10:19:55AM +0300, Georgi Djakov wrote: > On 16.04.22 4:26, Leo Yan wrote: > > It's fashion to use the icc_sync_state callback to notify the framework > > when all consumers are probed, so that the bandwidth request doesn't > > need to stay on maximum value. > > > > Do the same thing for msm8939 driver. > > I assume that you tested this with some out of tree DT? Is it public? Yes, Bryan is upstreaming for DT binding patch, see: https://lore.kernel.org/all/20220419010903.3109514-3-bryan.odonoghue@linaro.org/ > If the consumers are not described as such in DT and/or the support > in the client drivers is missing, paths might get disabled. Indeed, when tested the mainline kernel on msm8939 (with several offline patches for enabling msm8939), I observed that GPU and display drivers are not enabled yet, so some interconnect paths are failed. In this case, the interconnect clock stays on maximum frequency. But I think this doesn't impact this patch; if without this patch, icc_sync_state() will never be called and the global variable 'synced_state' is always false. In other words, based on this patch and after initiailize all client drivers, the clients' bandwdith request will be respected. Thanks, Leo > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > --- > > drivers/interconnect/qcom/msm8939.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/interconnect/qcom/msm8939.c b/drivers/interconnect/qcom/msm8939.c > > index f9c2d7d3100d..ca5f611d33b0 100644 > > --- a/drivers/interconnect/qcom/msm8939.c > > +++ b/drivers/interconnect/qcom/msm8939.c > > @@ -1423,6 +1423,7 @@ static struct platform_driver msm8939_noc_driver = { > > .driver = { > > .name = "qnoc-msm8939", > > .of_match_table = msm8939_noc_of_match, > > + .sync_state = icc_sync_state, > > }, > > }; > > module_platform_driver(msm8939_noc_driver); >
Hi Georgi, Bjorn, On Sat, Apr 16, 2022 at 09:26:34AM +0800, Leo Yan wrote: > It's fashion to use the icc_sync_state callback to notify the framework > when all consumers are probed, so that the bandwidth request doesn't > need to stay on maximum value. > > Do the same thing for msm8939 driver. Ping for this patch. This patch is needed for enabling ICC driver on msm8939, I verified it based on Bryan O'Donoghue's DT binding patches. Please see the branch which contains complete patches: https://git.linaro.org/people/leo.yan/linux.git/log/?h=v5.19-rc4%2bicc_sleep_clock_v2 Thanks, Leo > Signed-off-by: Leo Yan <leo.yan@linaro.org> > --- > drivers/interconnect/qcom/msm8939.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/interconnect/qcom/msm8939.c b/drivers/interconnect/qcom/msm8939.c > index f9c2d7d3100d..ca5f611d33b0 100644 > --- a/drivers/interconnect/qcom/msm8939.c > +++ b/drivers/interconnect/qcom/msm8939.c > @@ -1423,6 +1423,7 @@ static struct platform_driver msm8939_noc_driver = { > .driver = { > .name = "qnoc-msm8939", > .of_match_table = msm8939_noc_of_match, > + .sync_state = icc_sync_state, > }, > }; > module_platform_driver(msm8939_noc_driver); > -- > 2.25.1 >
On 5.07.22 10:31, Leo Yan wrote: > Hi Georgi, Bjorn, > > On Sat, Apr 16, 2022 at 09:26:34AM +0800, Leo Yan wrote: >> It's fashion to use the icc_sync_state callback to notify the framework >> when all consumers are probed, so that the bandwidth request doesn't >> need to stay on maximum value. >> >> Do the same thing for msm8939 driver. > > Ping for this patch. This patch is needed for enabling ICC driver on > msm8939, I verified it based on Bryan O'Donoghue's DT binding patches. > > Please see the branch which contains complete patches: > https://git.linaro.org/people/leo.yan/linux.git/log/?h=v5.19-rc4%2bicc_sleep_clock_v2 > Ok, thanks for verifying it. Merged. BR, Georgi
diff --git a/drivers/interconnect/qcom/msm8939.c b/drivers/interconnect/qcom/msm8939.c index f9c2d7d3100d..ca5f611d33b0 100644 --- a/drivers/interconnect/qcom/msm8939.c +++ b/drivers/interconnect/qcom/msm8939.c @@ -1423,6 +1423,7 @@ static struct platform_driver msm8939_noc_driver = { .driver = { .name = "qnoc-msm8939", .of_match_table = msm8939_noc_of_match, + .sync_state = icc_sync_state, }, }; module_platform_driver(msm8939_noc_driver);
It's fashion to use the icc_sync_state callback to notify the framework when all consumers are probed, so that the bandwidth request doesn't need to stay on maximum value. Do the same thing for msm8939 driver. Signed-off-by: Leo Yan <leo.yan@linaro.org> --- drivers/interconnect/qcom/msm8939.c | 1 + 1 file changed, 1 insertion(+)