diff mbox series

interconnect: qcom: msm8939: Use icc_sync_state

Message ID 20220416012634.479617-1-leo.yan@linaro.org (mailing list archive)
State Not Applicable, archived
Headers show
Series interconnect: qcom: msm8939: Use icc_sync_state | expand

Commit Message

Leo Yan April 16, 2022, 1:26 a.m. UTC
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(+)

Comments

Georgi Djakov April 28, 2022, 7:19 a.m. UTC | #1
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);
Leo Yan April 28, 2022, 8:41 a.m. UTC | #2
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);
>
Leo Yan July 5, 2022, 7:31 a.m. UTC | #3
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
>
Georgi Djakov July 5, 2022, 1:58 p.m. UTC | #4
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 mbox series

Patch

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);