Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Tue, Aug 07, 2018 at 10:35:37AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 07일 09:23, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Tue, Aug 07, 2018 at 07:31:16AM +0900, Chanwoo Choi wrote: > >> Hi Matthias, > >> > >> On 2018년 08월 07일 04:21, Matthias Kaehlcke wrote: > >>> Hi Chanwoo, > >>> > >>> On Fri, Aug 03, 2018 at 09:14:46AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: > > On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: > >> Hi Chanwoo, > >> > >> On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > >>> Hi Matthias, > >>> > >>> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > > On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > >> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > >>> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > > >> Firstly, > >> I'm not sure why devfreq needs the > >> devfreq_verify_within_limits() function. > >> > >> devfreq already used the OPP interface as default. It means > >> that > >> the outside of 'drivers/devfreq' can disable/enable the > >> frequency > >> such as drivers/thermal/devfreq_cooling.c. Also, when some > >> device > >> drivers disable/enable the specific frequency, the devfreq core > >> consider them. > >> > >> So, devfreq doesn't need to devfreq_verify_within_limits() > >> because > >> already support some interface to change the minimum/maximum > >> frequency > >> of devfreq device. > >> > >> In case of cpufreq subsystem, cpufreq only provides > >> 'cpufreq_verify_with_limits()' > >> to change the minimum/maximum frequency of cpu. some device > >> driver cannot > >> change the minimum/maximum frequency through OPP interface. > >> > >> But, in case of devfreq subsystem, as I explained already, > >> devfreq support > >> the OPP interface as default way. devfreq subsystem doesn't > >> need to add > >> other way to change the minimum/maximum frequency. > > > > Using the OPP interface exclusively works as long as a > > enabling/disabling of OPPs is limited to a single driver > > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > > involved you need a way to resolve conflicts, that's the > > purpose of > > devfreq_verify_within_limits(). Please let me know if there are > > existing mechanisms for conflict resolution that I overlooked. > > > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to > > use > > devfreq_verify_within_limits() instead of the OPP interface if > > desired, however this seems beyond the scope of this series. > > Actually, if we uses this approach, it doesn't support the > multiple drivers too. > If non throttler drivers uses devfreq_verify_within_limits(), > the conflict > happen. > >>> > >>> As long as drivers limit the max freq there is no conflict, the > >>> lowest > >>> max freq wins. I expect this to be the usual case, apparently it > >>> worked for cpufreq for 10+ years. > >>> > >>> However it is correct that there would be a conflict if a driver > >>> requests a min freq that is higher than the max freq requested by > >>> another. In this case devfreq_verify_within_limits() resolves the > >>> conflict by raising p->max to the min freq. Not sure if this is > >>> something that would ever occur in practice though. > >>> > >>> If we are really concerned about this case it would also be an > >>> option > >>> to limit the adjustment to the max frequency. > >>> > To resolve the conflict for multiple device driver, maybe OPP > interface > have to support 'usage_count' such as clk_enable/disable(). > >>> > >>> This would require supporting negative usage count values, since > >>> a OPP > >>> should not be enabled if e.g. thermal enables it but the
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Tue, Aug 07, 2018 at 10:35:37AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 07일 09:23, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Tue, Aug 07, 2018 at 07:31:16AM +0900, Chanwoo Choi wrote: > >> Hi Matthias, > >> > >> On 2018년 08월 07일 04:21, Matthias Kaehlcke wrote: > >>> Hi Chanwoo, > >>> > >>> On Fri, Aug 03, 2018 at 09:14:46AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: > > On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: > >> Hi Chanwoo, > >> > >> On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > >>> Hi Matthias, > >>> > >>> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > > On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > >> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > >>> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > > >> Firstly, > >> I'm not sure why devfreq needs the > >> devfreq_verify_within_limits() function. > >> > >> devfreq already used the OPP interface as default. It means > >> that > >> the outside of 'drivers/devfreq' can disable/enable the > >> frequency > >> such as drivers/thermal/devfreq_cooling.c. Also, when some > >> device > >> drivers disable/enable the specific frequency, the devfreq core > >> consider them. > >> > >> So, devfreq doesn't need to devfreq_verify_within_limits() > >> because > >> already support some interface to change the minimum/maximum > >> frequency > >> of devfreq device. > >> > >> In case of cpufreq subsystem, cpufreq only provides > >> 'cpufreq_verify_with_limits()' > >> to change the minimum/maximum frequency of cpu. some device > >> driver cannot > >> change the minimum/maximum frequency through OPP interface. > >> > >> But, in case of devfreq subsystem, as I explained already, > >> devfreq support > >> the OPP interface as default way. devfreq subsystem doesn't > >> need to add > >> other way to change the minimum/maximum frequency. > > > > Using the OPP interface exclusively works as long as a > > enabling/disabling of OPPs is limited to a single driver > > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > > involved you need a way to resolve conflicts, that's the > > purpose of > > devfreq_verify_within_limits(). Please let me know if there are > > existing mechanisms for conflict resolution that I overlooked. > > > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to > > use > > devfreq_verify_within_limits() instead of the OPP interface if > > desired, however this seems beyond the scope of this series. > > Actually, if we uses this approach, it doesn't support the > multiple drivers too. > If non throttler drivers uses devfreq_verify_within_limits(), > the conflict > happen. > >>> > >>> As long as drivers limit the max freq there is no conflict, the > >>> lowest > >>> max freq wins. I expect this to be the usual case, apparently it > >>> worked for cpufreq for 10+ years. > >>> > >>> However it is correct that there would be a conflict if a driver > >>> requests a min freq that is higher than the max freq requested by > >>> another. In this case devfreq_verify_within_limits() resolves the > >>> conflict by raising p->max to the min freq. Not sure if this is > >>> something that would ever occur in practice though. > >>> > >>> If we are really concerned about this case it would also be an > >>> option > >>> to limit the adjustment to the max frequency. > >>> > To resolve the conflict for multiple device driver, maybe OPP > interface > have to support 'usage_count' such as clk_enable/disable(). > >>> > >>> This would require supporting negative usage count values, since > >>> a OPP > >>> should not be enabled if e.g. thermal enables it but the
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 08월 07일 09:23, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Tue, Aug 07, 2018 at 07:31:16AM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> On 2018년 08월 07일 04:21, Matthias Kaehlcke wrote: >>> Hi Chanwoo, >>> >>> On Fri, Aug 03, 2018 at 09:14:46AM +0900, Chanwoo Choi wrote: Hi Matthias, On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: > On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: >> Hi Chanwoo, >> >> On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: >>> Hi Matthias, >>> >>> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: Hi Chanwoo, On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: >> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: >>> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: Hi Matthias, On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > >> Firstly, >> I'm not sure why devfreq needs the >> devfreq_verify_within_limits() function. >> >> devfreq already used the OPP interface as default. It means that >> the outside of 'drivers/devfreq' can disable/enable the frequency >> such as drivers/thermal/devfreq_cooling.c. Also, when some device >> drivers disable/enable the specific frequency, the devfreq core >> consider them. >> >> So, devfreq doesn't need to devfreq_verify_within_limits() >> because >> already support some interface to change the minimum/maximum >> frequency >> of devfreq device. >> >> In case of cpufreq subsystem, cpufreq only provides >> 'cpufreq_verify_with_limits()' >> to change the minimum/maximum frequency of cpu. some device >> driver cannot >> change the minimum/maximum frequency through OPP interface. >> >> But, in case of devfreq subsystem, as I explained already, >> devfreq support >> the OPP interface as default way. devfreq subsystem doesn't need >> to add >> other way to change the minimum/maximum frequency. > > Using the OPP interface exclusively works as long as a > enabling/disabling of OPPs is limited to a single driver > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > involved you need a way to resolve conflicts, that's the purpose > of > devfreq_verify_within_limits(). Please let me know if there are > existing mechanisms for conflict resolution that I overlooked. > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to > use > devfreq_verify_within_limits() instead of the OPP interface if > desired, however this seems beyond the scope of this series. Actually, if we uses this approach, it doesn't support the multiple drivers too. If non throttler drivers uses devfreq_verify_within_limits(), the conflict happen. >>> >>> As long as drivers limit the max freq there is no conflict, the >>> lowest >>> max freq wins. I expect this to be the usual case, apparently it >>> worked for cpufreq for 10+ years. >>> >>> However it is correct that there would be a conflict if a driver >>> requests a min freq that is higher than the max freq requested by >>> another. In this case devfreq_verify_within_limits() resolves the >>> conflict by raising p->max to the min freq. Not sure if this is >>> something that would ever occur in practice though. >>> >>> If we are really concerned about this case it would also be an >>> option >>> to limit the adjustment to the max frequency. >>> To resolve the conflict for multiple device driver, maybe OPP interface have to support 'usage_count' such as clk_enable/disable(). >>> >>> This would require supporting negative usage count values, since a >>> OPP >>> should not be enabled if e.g. thermal enables it but the throttler >>> disabled it or viceversa. >>> >>> Theoretically there could also be conflicts, like one driver >>> disabling >>> the higher OPPs and another the lower ones, with the outcome of all >>> OPPs being disabled, which would be a more drastic conflict >>> resolution >>>
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 08월 07일 09:23, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Tue, Aug 07, 2018 at 07:31:16AM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> On 2018년 08월 07일 04:21, Matthias Kaehlcke wrote: >>> Hi Chanwoo, >>> >>> On Fri, Aug 03, 2018 at 09:14:46AM +0900, Chanwoo Choi wrote: Hi Matthias, On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: > On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: >> Hi Chanwoo, >> >> On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: >>> Hi Matthias, >>> >>> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: Hi Chanwoo, On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: >> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: >>> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: Hi Matthias, On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > >> Firstly, >> I'm not sure why devfreq needs the >> devfreq_verify_within_limits() function. >> >> devfreq already used the OPP interface as default. It means that >> the outside of 'drivers/devfreq' can disable/enable the frequency >> such as drivers/thermal/devfreq_cooling.c. Also, when some device >> drivers disable/enable the specific frequency, the devfreq core >> consider them. >> >> So, devfreq doesn't need to devfreq_verify_within_limits() >> because >> already support some interface to change the minimum/maximum >> frequency >> of devfreq device. >> >> In case of cpufreq subsystem, cpufreq only provides >> 'cpufreq_verify_with_limits()' >> to change the minimum/maximum frequency of cpu. some device >> driver cannot >> change the minimum/maximum frequency through OPP interface. >> >> But, in case of devfreq subsystem, as I explained already, >> devfreq support >> the OPP interface as default way. devfreq subsystem doesn't need >> to add >> other way to change the minimum/maximum frequency. > > Using the OPP interface exclusively works as long as a > enabling/disabling of OPPs is limited to a single driver > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > involved you need a way to resolve conflicts, that's the purpose > of > devfreq_verify_within_limits(). Please let me know if there are > existing mechanisms for conflict resolution that I overlooked. > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to > use > devfreq_verify_within_limits() instead of the OPP interface if > desired, however this seems beyond the scope of this series. Actually, if we uses this approach, it doesn't support the multiple drivers too. If non throttler drivers uses devfreq_verify_within_limits(), the conflict happen. >>> >>> As long as drivers limit the max freq there is no conflict, the >>> lowest >>> max freq wins. I expect this to be the usual case, apparently it >>> worked for cpufreq for 10+ years. >>> >>> However it is correct that there would be a conflict if a driver >>> requests a min freq that is higher than the max freq requested by >>> another. In this case devfreq_verify_within_limits() resolves the >>> conflict by raising p->max to the min freq. Not sure if this is >>> something that would ever occur in practice though. >>> >>> If we are really concerned about this case it would also be an >>> option >>> to limit the adjustment to the max frequency. >>> To resolve the conflict for multiple device driver, maybe OPP interface have to support 'usage_count' such as clk_enable/disable(). >>> >>> This would require supporting negative usage count values, since a >>> OPP >>> should not be enabled if e.g. thermal enables it but the throttler >>> disabled it or viceversa. >>> >>> Theoretically there could also be conflicts, like one driver >>> disabling >>> the higher OPPs and another the lower ones, with the outcome of all >>> OPPs being disabled, which would be a more drastic conflict >>> resolution >>>
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Tue, Aug 07, 2018 at 07:31:16AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 07일 04:21, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Fri, Aug 03, 2018 at 09:14:46AM +0900, Chanwoo Choi wrote: > >> Hi Matthias, > >> > >> On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: > >>> On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > > Hi Matthias, > > > > On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > >> Hi Chanwoo, > >> > >> On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > >>> On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > > On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > >> Hi Matthias, > >> > >> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > >>> Hi Chanwoo, > >>> > >>> On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > >>> > Firstly, > I'm not sure why devfreq needs the > devfreq_verify_within_limits() function. > > devfreq already used the OPP interface as default. It means that > the outside of 'drivers/devfreq' can disable/enable the frequency > such as drivers/thermal/devfreq_cooling.c. Also, when some device > drivers disable/enable the specific frequency, the devfreq core > consider them. > > So, devfreq doesn't need to devfreq_verify_within_limits() > because > already support some interface to change the minimum/maximum > frequency > of devfreq device. > > In case of cpufreq subsystem, cpufreq only provides > 'cpufreq_verify_with_limits()' > to change the minimum/maximum frequency of cpu. some device > driver cannot > change the minimum/maximum frequency through OPP interface. > > But, in case of devfreq subsystem, as I explained already, > devfreq support > the OPP interface as default way. devfreq subsystem doesn't need > to add > other way to change the minimum/maximum frequency. > >>> > >>> Using the OPP interface exclusively works as long as a > >>> enabling/disabling of OPPs is limited to a single driver > >>> (drivers/thermal/devfreq_cooling.c). When multiple drivers are > >>> involved you need a way to resolve conflicts, that's the purpose > >>> of > >>> devfreq_verify_within_limits(). Please let me know if there are > >>> existing mechanisms for conflict resolution that I overlooked. > >>> > >>> Possibly drivers/thermal/devfreq_cooling.c could be migrated to > >>> use > >>> devfreq_verify_within_limits() instead of the OPP interface if > >>> desired, however this seems beyond the scope of this series. > >> > >> Actually, if we uses this approach, it doesn't support the > >> multiple drivers too. > >> If non throttler drivers uses devfreq_verify_within_limits(), the > >> conflict > >> happen. > > > > As long as drivers limit the max freq there is no conflict, the > > lowest > > max freq wins. I expect this to be the usual case, apparently it > > worked for cpufreq for 10+ years. > > > > However it is correct that there would be a conflict if a driver > > requests a min freq that is higher than the max freq requested by > > another. In this case devfreq_verify_within_limits() resolves the > > conflict by raising p->max to the min freq. Not sure if this is > > something that would ever occur in practice though. > > > > If we are really concerned about this case it would also be an > > option > > to limit the adjustment to the max frequency. > > > >> To resolve the conflict for multiple device driver, maybe OPP > >> interface > >> have to support 'usage_count' such as clk_enable/disable(). > > > > This would require supporting negative usage count values, since a > > OPP > > should not be enabled if e.g. thermal enables it but the throttler > > disabled it or viceversa. > > > > Theoretically there could also be conflicts, like one driver > > disabling > > the higher OPPs and another the lower ones, with the outcome of all > > OPPs being disabled, which would be a more drastic conflict > > resolution > > than that of devfreq_verify_within_limits(). > > > >
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Tue, Aug 07, 2018 at 07:31:16AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 07일 04:21, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Fri, Aug 03, 2018 at 09:14:46AM +0900, Chanwoo Choi wrote: > >> Hi Matthias, > >> > >> On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: > >>> On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > > Hi Matthias, > > > > On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > >> Hi Chanwoo, > >> > >> On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > >>> On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > > On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > >> Hi Matthias, > >> > >> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > >>> Hi Chanwoo, > >>> > >>> On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > >>> > Firstly, > I'm not sure why devfreq needs the > devfreq_verify_within_limits() function. > > devfreq already used the OPP interface as default. It means that > the outside of 'drivers/devfreq' can disable/enable the frequency > such as drivers/thermal/devfreq_cooling.c. Also, when some device > drivers disable/enable the specific frequency, the devfreq core > consider them. > > So, devfreq doesn't need to devfreq_verify_within_limits() > because > already support some interface to change the minimum/maximum > frequency > of devfreq device. > > In case of cpufreq subsystem, cpufreq only provides > 'cpufreq_verify_with_limits()' > to change the minimum/maximum frequency of cpu. some device > driver cannot > change the minimum/maximum frequency through OPP interface. > > But, in case of devfreq subsystem, as I explained already, > devfreq support > the OPP interface as default way. devfreq subsystem doesn't need > to add > other way to change the minimum/maximum frequency. > >>> > >>> Using the OPP interface exclusively works as long as a > >>> enabling/disabling of OPPs is limited to a single driver > >>> (drivers/thermal/devfreq_cooling.c). When multiple drivers are > >>> involved you need a way to resolve conflicts, that's the purpose > >>> of > >>> devfreq_verify_within_limits(). Please let me know if there are > >>> existing mechanisms for conflict resolution that I overlooked. > >>> > >>> Possibly drivers/thermal/devfreq_cooling.c could be migrated to > >>> use > >>> devfreq_verify_within_limits() instead of the OPP interface if > >>> desired, however this seems beyond the scope of this series. > >> > >> Actually, if we uses this approach, it doesn't support the > >> multiple drivers too. > >> If non throttler drivers uses devfreq_verify_within_limits(), the > >> conflict > >> happen. > > > > As long as drivers limit the max freq there is no conflict, the > > lowest > > max freq wins. I expect this to be the usual case, apparently it > > worked for cpufreq for 10+ years. > > > > However it is correct that there would be a conflict if a driver > > requests a min freq that is higher than the max freq requested by > > another. In this case devfreq_verify_within_limits() resolves the > > conflict by raising p->max to the min freq. Not sure if this is > > something that would ever occur in practice though. > > > > If we are really concerned about this case it would also be an > > option > > to limit the adjustment to the max frequency. > > > >> To resolve the conflict for multiple device driver, maybe OPP > >> interface > >> have to support 'usage_count' such as clk_enable/disable(). > > > > This would require supporting negative usage count values, since a > > OPP > > should not be enabled if e.g. thermal enables it but the throttler > > disabled it or viceversa. > > > > Theoretically there could also be conflicts, like one driver > > disabling > > the higher OPPs and another the lower ones, with the outcome of all > > OPPs being disabled, which would be a more drastic conflict > > resolution > > than that of devfreq_verify_within_limits(). > > > >
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Viresh Kumar, I have a question about dev_pm_opp_enable() and dev_pm_opp_disable(). Two functions have 'available' field to indicate the status of specific OPP. If different device drivers try to control the same OPP, dev_pm_opp_enable() and dev_pm_opp_disable() will consider only last operation. It means that OPP should be enabled/disabled by only one device driver. For example, opp_table of driver a(dev_a) - 500Mhz - 400Mhz - 300Mhz - 200Mhz - 100Mhz Driver B, opp_disable(dev_a, 500) Driver C, opp_enable(dev_a, 500) -> 500Mhz is enabled. But, driver B might want to enable 500Mhz at this time such as cooling. I think that if OPP support the use of multiple device drivers, dev_pm_opp_enable() and dev_pm_opp_disable() should support the usage count such as regulator/clock. I would like your opinion. Regards, Chanwoo Choi On 2018년 08월 07일 07:31, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 07일 04:21, Matthias Kaehlcke wrote: >> Hi Chanwoo, >> >> On Fri, Aug 03, 2018 at 09:14:46AM +0900, Chanwoo Choi wrote: >>> Hi Matthias, >>> >>> On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: >>> Hi Chanwoo, >>> >>> On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: >> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: >>> Hi Matthias, >>> >>> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: Hi Chanwoo, On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > Firstly, > I'm not sure why devfreq needs the devfreq_verify_within_limits() > function. > > devfreq already used the OPP interface as default. It means that > the outside of 'drivers/devfreq' can disable/enable the frequency > such as drivers/thermal/devfreq_cooling.c. Also, when some device > drivers disable/enable the specific frequency, the devfreq core > consider them. > > So, devfreq doesn't need to devfreq_verify_within_limits() because > already support some interface to change the minimum/maximum > frequency > of devfreq device. > > In case of cpufreq subsystem, cpufreq only provides > 'cpufreq_verify_with_limits()' > to change the minimum/maximum frequency of cpu. some device > driver cannot > change the minimum/maximum frequency through OPP interface. > > But, in case of devfreq subsystem, as I explained already, > devfreq support > the OPP interface as default way. devfreq subsystem doesn't need > to add > other way to change the minimum/maximum frequency. Using the OPP interface exclusively works as long as a enabling/disabling of OPPs is limited to a single driver (drivers/thermal/devfreq_cooling.c). When multiple drivers are involved you need a way to resolve conflicts, that's the purpose of devfreq_verify_within_limits(). Please let me know if there are existing mechanisms for conflict resolution that I overlooked. Possibly drivers/thermal/devfreq_cooling.c could be migrated to use devfreq_verify_within_limits() instead of the OPP interface if desired, however this seems beyond the scope of this series. >>> >>> Actually, if we uses this approach, it doesn't support the multiple >>> drivers too. >>> If non throttler drivers uses devfreq_verify_within_limits(), the >>> conflict >>> happen. >> >> As long as drivers limit the max freq there is no conflict, the >> lowest >> max freq wins. I expect this to be the usual case, apparently it >> worked for cpufreq for 10+ years. >> >> However it is correct that there would be a conflict if a driver >> requests a min freq that is higher than the max freq requested by >> another. In this case devfreq_verify_within_limits() resolves the >> conflict by raising p->max to the min freq. Not sure if this is >> something that would ever occur in practice though. >> >> If we are really concerned about this case it would also be an option >> to limit the adjustment to the max frequency. >> >>> To resolve the conflict for multiple device driver, maybe OPP >>>
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Viresh Kumar, I have a question about dev_pm_opp_enable() and dev_pm_opp_disable(). Two functions have 'available' field to indicate the status of specific OPP. If different device drivers try to control the same OPP, dev_pm_opp_enable() and dev_pm_opp_disable() will consider only last operation. It means that OPP should be enabled/disabled by only one device driver. For example, opp_table of driver a(dev_a) - 500Mhz - 400Mhz - 300Mhz - 200Mhz - 100Mhz Driver B, opp_disable(dev_a, 500) Driver C, opp_enable(dev_a, 500) -> 500Mhz is enabled. But, driver B might want to enable 500Mhz at this time such as cooling. I think that if OPP support the use of multiple device drivers, dev_pm_opp_enable() and dev_pm_opp_disable() should support the usage count such as regulator/clock. I would like your opinion. Regards, Chanwoo Choi On 2018년 08월 07일 07:31, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 07일 04:21, Matthias Kaehlcke wrote: >> Hi Chanwoo, >> >> On Fri, Aug 03, 2018 at 09:14:46AM +0900, Chanwoo Choi wrote: >>> Hi Matthias, >>> >>> On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: >>> Hi Chanwoo, >>> >>> On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: >> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: >>> Hi Matthias, >>> >>> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: Hi Chanwoo, On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > Firstly, > I'm not sure why devfreq needs the devfreq_verify_within_limits() > function. > > devfreq already used the OPP interface as default. It means that > the outside of 'drivers/devfreq' can disable/enable the frequency > such as drivers/thermal/devfreq_cooling.c. Also, when some device > drivers disable/enable the specific frequency, the devfreq core > consider them. > > So, devfreq doesn't need to devfreq_verify_within_limits() because > already support some interface to change the minimum/maximum > frequency > of devfreq device. > > In case of cpufreq subsystem, cpufreq only provides > 'cpufreq_verify_with_limits()' > to change the minimum/maximum frequency of cpu. some device > driver cannot > change the minimum/maximum frequency through OPP interface. > > But, in case of devfreq subsystem, as I explained already, > devfreq support > the OPP interface as default way. devfreq subsystem doesn't need > to add > other way to change the minimum/maximum frequency. Using the OPP interface exclusively works as long as a enabling/disabling of OPPs is limited to a single driver (drivers/thermal/devfreq_cooling.c). When multiple drivers are involved you need a way to resolve conflicts, that's the purpose of devfreq_verify_within_limits(). Please let me know if there are existing mechanisms for conflict resolution that I overlooked. Possibly drivers/thermal/devfreq_cooling.c could be migrated to use devfreq_verify_within_limits() instead of the OPP interface if desired, however this seems beyond the scope of this series. >>> >>> Actually, if we uses this approach, it doesn't support the multiple >>> drivers too. >>> If non throttler drivers uses devfreq_verify_within_limits(), the >>> conflict >>> happen. >> >> As long as drivers limit the max freq there is no conflict, the >> lowest >> max freq wins. I expect this to be the usual case, apparently it >> worked for cpufreq for 10+ years. >> >> However it is correct that there would be a conflict if a driver >> requests a min freq that is higher than the max freq requested by >> another. In this case devfreq_verify_within_limits() resolves the >> conflict by raising p->max to the min freq. Not sure if this is >> something that would ever occur in practice though. >> >> If we are really concerned about this case it would also be an option >> to limit the adjustment to the max frequency. >> >>> To resolve the conflict for multiple device driver, maybe OPP >>>
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 08월 07일 04:21, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Fri, Aug 03, 2018 at 09:14:46AM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: >>> On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: Hi Chanwoo, On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: >> Hi Chanwoo, >> >> On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: >>> On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: >>> Hi Chanwoo, >>> >>> On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: >>> Firstly, I'm not sure why devfreq needs the devfreq_verify_within_limits() function. devfreq already used the OPP interface as default. It means that the outside of 'drivers/devfreq' can disable/enable the frequency such as drivers/thermal/devfreq_cooling.c. Also, when some device drivers disable/enable the specific frequency, the devfreq core consider them. So, devfreq doesn't need to devfreq_verify_within_limits() because already support some interface to change the minimum/maximum frequency of devfreq device. In case of cpufreq subsystem, cpufreq only provides 'cpufreq_verify_with_limits()' to change the minimum/maximum frequency of cpu. some device driver cannot change the minimum/maximum frequency through OPP interface. But, in case of devfreq subsystem, as I explained already, devfreq support the OPP interface as default way. devfreq subsystem doesn't need to add other way to change the minimum/maximum frequency. >>> >>> Using the OPP interface exclusively works as long as a >>> enabling/disabling of OPPs is limited to a single driver >>> (drivers/thermal/devfreq_cooling.c). When multiple drivers are >>> involved you need a way to resolve conflicts, that's the purpose of >>> devfreq_verify_within_limits(). Please let me know if there are >>> existing mechanisms for conflict resolution that I overlooked. >>> >>> Possibly drivers/thermal/devfreq_cooling.c could be migrated to use >>> devfreq_verify_within_limits() instead of the OPP interface if >>> desired, however this seems beyond the scope of this series. >> >> Actually, if we uses this approach, it doesn't support the multiple >> drivers too. >> If non throttler drivers uses devfreq_verify_within_limits(), the >> conflict >> happen. > > As long as drivers limit the max freq there is no conflict, the lowest > max freq wins. I expect this to be the usual case, apparently it > worked for cpufreq for 10+ years. > > However it is correct that there would be a conflict if a driver > requests a min freq that is higher than the max freq requested by > another. In this case devfreq_verify_within_limits() resolves the > conflict by raising p->max to the min freq. Not sure if this is > something that would ever occur in practice though. > > If we are really concerned about this case it would also be an option > to limit the adjustment to the max frequency. > >> To resolve the conflict for multiple device driver, maybe OPP >> interface >> have to support 'usage_count' such as clk_enable/disable(). > > This would require supporting negative usage count values, since a OPP > should not be enabled if e.g. thermal enables it but the throttler > disabled it or viceversa. > > Theoretically there could also be conflicts, like one driver disabling > the higher OPPs and another the lower ones, with the outcome of all > OPPs being disabled, which would be a more drastic conflict resolution > than that of devfreq_verify_within_limits(). > > Viresh, what do you think about an OPP usage count? Ping, can we try to reach a conclusion on this or at least keep the discussion going? Not that it matters, but my preferred solution continues to be devfreq_verify_within_limits(). It solves conflicts in some way (which could be adjusted if needed) and has proven to work
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 08월 07일 04:21, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Fri, Aug 03, 2018 at 09:14:46AM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: >>> On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: Hi Chanwoo, On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: >> Hi Chanwoo, >> >> On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: >>> On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: >>> Hi Chanwoo, >>> >>> On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: >>> Firstly, I'm not sure why devfreq needs the devfreq_verify_within_limits() function. devfreq already used the OPP interface as default. It means that the outside of 'drivers/devfreq' can disable/enable the frequency such as drivers/thermal/devfreq_cooling.c. Also, when some device drivers disable/enable the specific frequency, the devfreq core consider them. So, devfreq doesn't need to devfreq_verify_within_limits() because already support some interface to change the minimum/maximum frequency of devfreq device. In case of cpufreq subsystem, cpufreq only provides 'cpufreq_verify_with_limits()' to change the minimum/maximum frequency of cpu. some device driver cannot change the minimum/maximum frequency through OPP interface. But, in case of devfreq subsystem, as I explained already, devfreq support the OPP interface as default way. devfreq subsystem doesn't need to add other way to change the minimum/maximum frequency. >>> >>> Using the OPP interface exclusively works as long as a >>> enabling/disabling of OPPs is limited to a single driver >>> (drivers/thermal/devfreq_cooling.c). When multiple drivers are >>> involved you need a way to resolve conflicts, that's the purpose of >>> devfreq_verify_within_limits(). Please let me know if there are >>> existing mechanisms for conflict resolution that I overlooked. >>> >>> Possibly drivers/thermal/devfreq_cooling.c could be migrated to use >>> devfreq_verify_within_limits() instead of the OPP interface if >>> desired, however this seems beyond the scope of this series. >> >> Actually, if we uses this approach, it doesn't support the multiple >> drivers too. >> If non throttler drivers uses devfreq_verify_within_limits(), the >> conflict >> happen. > > As long as drivers limit the max freq there is no conflict, the lowest > max freq wins. I expect this to be the usual case, apparently it > worked for cpufreq for 10+ years. > > However it is correct that there would be a conflict if a driver > requests a min freq that is higher than the max freq requested by > another. In this case devfreq_verify_within_limits() resolves the > conflict by raising p->max to the min freq. Not sure if this is > something that would ever occur in practice though. > > If we are really concerned about this case it would also be an option > to limit the adjustment to the max frequency. > >> To resolve the conflict for multiple device driver, maybe OPP >> interface >> have to support 'usage_count' such as clk_enable/disable(). > > This would require supporting negative usage count values, since a OPP > should not be enabled if e.g. thermal enables it but the throttler > disabled it or viceversa. > > Theoretically there could also be conflicts, like one driver disabling > the higher OPPs and another the lower ones, with the outcome of all > OPPs being disabled, which would be a more drastic conflict resolution > than that of devfreq_verify_within_limits(). > > Viresh, what do you think about an OPP usage count? Ping, can we try to reach a conclusion on this or at least keep the discussion going? Not that it matters, but my preferred solution continues to be devfreq_verify_within_limits(). It solves conflicts in some way (which could be adjusted if needed) and has proven to work
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 08월 07일 03:46, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Fri, Aug 03, 2018 at 08:56:57AM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> On 2018년 08월 03일 08:13, Matthias Kaehlcke wrote: >>> Hi Chanwoo, >>> >>> On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: Hi Matthias, On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: >> On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: >>> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: >> Hi Chanwoo, >> >> On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: >> >>> Firstly, >>> I'm not sure why devfreq needs the devfreq_verify_within_limits() >>> function. >>> >>> devfreq already used the OPP interface as default. It means that >>> the outside of 'drivers/devfreq' can disable/enable the frequency >>> such as drivers/thermal/devfreq_cooling.c. Also, when some device >>> drivers disable/enable the specific frequency, the devfreq core >>> consider them. >>> >>> So, devfreq doesn't need to devfreq_verify_within_limits() because >>> already support some interface to change the minimum/maximum >>> frequency >>> of devfreq device. >>> >>> In case of cpufreq subsystem, cpufreq only provides >>> 'cpufreq_verify_with_limits()' >>> to change the minimum/maximum frequency of cpu. some device driver >>> cannot >>> change the minimum/maximum frequency through OPP interface. >>> >>> But, in case of devfreq subsystem, as I explained already, devfreq >>> support >>> the OPP interface as default way. devfreq subsystem doesn't need to >>> add >>> other way to change the minimum/maximum frequency. >> >> Using the OPP interface exclusively works as long as a >> enabling/disabling of OPPs is limited to a single driver >> (drivers/thermal/devfreq_cooling.c). When multiple drivers are >> involved you need a way to resolve conflicts, that's the purpose of >> devfreq_verify_within_limits(). Please let me know if there are >> existing mechanisms for conflict resolution that I overlooked. >> >> Possibly drivers/thermal/devfreq_cooling.c could be migrated to use >> devfreq_verify_within_limits() instead of the OPP interface if >> desired, however this seems beyond the scope of this series. > > Actually, if we uses this approach, it doesn't support the multiple > drivers too. > If non throttler drivers uses devfreq_verify_within_limits(), the > conflict > happen. As long as drivers limit the max freq there is no conflict, the lowest max freq wins. I expect this to be the usual case, apparently it worked for cpufreq for 10+ years. However it is correct that there would be a conflict if a driver requests a min freq that is higher than the max freq requested by another. In this case devfreq_verify_within_limits() resolves the conflict by raising p->max to the min freq. Not sure if this is something that would ever occur in practice though. If we are really concerned about this case it would also be an option to limit the adjustment to the max frequency. > To resolve the conflict for multiple device driver, maybe OPP > interface > have to support 'usage_count' such as clk_enable/disable(). This would require supporting negative usage count values, since a OPP should not be enabled if e.g. thermal enables it but the throttler disabled it or viceversa. Theoretically there could also be conflicts, like one driver disabling the higher OPPs and another the lower ones, with the outcome of all OPPs being disabled, which would be a more drastic conflict resolution than that of devfreq_verify_within_limits(). Viresh, what do you think about an OPP usage count? >>> >>> Ping, can we try to reach a conclusion on this or at least keep the >>> discussion going? >>> >>> Not that it matters, but my preferred solution continues to be >>> devfreq_verify_within_limits(). It solves conflicts in some way (which >>> could be adjusted if needed) and has proven to work in practice for >>> 10+ years in a very similar sub-system. >> >> It is not true. Current cpufreq subsystem doesn't support external OPP >> control to
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 08월 07일 03:46, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Fri, Aug 03, 2018 at 08:56:57AM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> On 2018년 08월 03일 08:13, Matthias Kaehlcke wrote: >>> Hi Chanwoo, >>> >>> On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: Hi Matthias, On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: >> On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: >>> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: >> Hi Chanwoo, >> >> On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: >> >>> Firstly, >>> I'm not sure why devfreq needs the devfreq_verify_within_limits() >>> function. >>> >>> devfreq already used the OPP interface as default. It means that >>> the outside of 'drivers/devfreq' can disable/enable the frequency >>> such as drivers/thermal/devfreq_cooling.c. Also, when some device >>> drivers disable/enable the specific frequency, the devfreq core >>> consider them. >>> >>> So, devfreq doesn't need to devfreq_verify_within_limits() because >>> already support some interface to change the minimum/maximum >>> frequency >>> of devfreq device. >>> >>> In case of cpufreq subsystem, cpufreq only provides >>> 'cpufreq_verify_with_limits()' >>> to change the minimum/maximum frequency of cpu. some device driver >>> cannot >>> change the minimum/maximum frequency through OPP interface. >>> >>> But, in case of devfreq subsystem, as I explained already, devfreq >>> support >>> the OPP interface as default way. devfreq subsystem doesn't need to >>> add >>> other way to change the minimum/maximum frequency. >> >> Using the OPP interface exclusively works as long as a >> enabling/disabling of OPPs is limited to a single driver >> (drivers/thermal/devfreq_cooling.c). When multiple drivers are >> involved you need a way to resolve conflicts, that's the purpose of >> devfreq_verify_within_limits(). Please let me know if there are >> existing mechanisms for conflict resolution that I overlooked. >> >> Possibly drivers/thermal/devfreq_cooling.c could be migrated to use >> devfreq_verify_within_limits() instead of the OPP interface if >> desired, however this seems beyond the scope of this series. > > Actually, if we uses this approach, it doesn't support the multiple > drivers too. > If non throttler drivers uses devfreq_verify_within_limits(), the > conflict > happen. As long as drivers limit the max freq there is no conflict, the lowest max freq wins. I expect this to be the usual case, apparently it worked for cpufreq for 10+ years. However it is correct that there would be a conflict if a driver requests a min freq that is higher than the max freq requested by another. In this case devfreq_verify_within_limits() resolves the conflict by raising p->max to the min freq. Not sure if this is something that would ever occur in practice though. If we are really concerned about this case it would also be an option to limit the adjustment to the max frequency. > To resolve the conflict for multiple device driver, maybe OPP > interface > have to support 'usage_count' such as clk_enable/disable(). This would require supporting negative usage count values, since a OPP should not be enabled if e.g. thermal enables it but the throttler disabled it or viceversa. Theoretically there could also be conflicts, like one driver disabling the higher OPPs and another the lower ones, with the outcome of all OPPs being disabled, which would be a more drastic conflict resolution than that of devfreq_verify_within_limits(). Viresh, what do you think about an OPP usage count? >>> >>> Ping, can we try to reach a conclusion on this or at least keep the >>> discussion going? >>> >>> Not that it matters, but my preferred solution continues to be >>> devfreq_verify_within_limits(). It solves conflicts in some way (which >>> could be adjusted if needed) and has proven to work in practice for >>> 10+ years in a very similar sub-system. >> >> It is not true. Current cpufreq subsystem doesn't support external OPP >> control to
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Fri, Aug 03, 2018 at 09:14:46AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: > > On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: > >> Hi Chanwoo, > >> > >> On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > >>> Hi Matthias, > >>> > >>> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > > On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > >> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > >>> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > > >> Firstly, > >> I'm not sure why devfreq needs the devfreq_verify_within_limits() > >> function. > >> > >> devfreq already used the OPP interface as default. It means that > >> the outside of 'drivers/devfreq' can disable/enable the frequency > >> such as drivers/thermal/devfreq_cooling.c. Also, when some device > >> drivers disable/enable the specific frequency, the devfreq core > >> consider them. > >> > >> So, devfreq doesn't need to devfreq_verify_within_limits() because > >> already support some interface to change the minimum/maximum > >> frequency > >> of devfreq device. > >> > >> In case of cpufreq subsystem, cpufreq only provides > >> 'cpufreq_verify_with_limits()' > >> to change the minimum/maximum frequency of cpu. some device driver > >> cannot > >> change the minimum/maximum frequency through OPP interface. > >> > >> But, in case of devfreq subsystem, as I explained already, devfreq > >> support > >> the OPP interface as default way. devfreq subsystem doesn't need > >> to add > >> other way to change the minimum/maximum frequency. > > > > Using the OPP interface exclusively works as long as a > > enabling/disabling of OPPs is limited to a single driver > > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > > involved you need a way to resolve conflicts, that's the purpose of > > devfreq_verify_within_limits(). Please let me know if there are > > existing mechanisms for conflict resolution that I overlooked. > > > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > > devfreq_verify_within_limits() instead of the OPP interface if > > desired, however this seems beyond the scope of this series. > > Actually, if we uses this approach, it doesn't support the multiple > drivers too. > If non throttler drivers uses devfreq_verify_within_limits(), the > conflict > happen. > >>> > >>> As long as drivers limit the max freq there is no conflict, the lowest > >>> max freq wins. I expect this to be the usual case, apparently it > >>> worked for cpufreq for 10+ years. > >>> > >>> However it is correct that there would be a conflict if a driver > >>> requests a min freq that is higher than the max freq requested by > >>> another. In this case devfreq_verify_within_limits() resolves the > >>> conflict by raising p->max to the min freq. Not sure if this is > >>> something that would ever occur in practice though. > >>> > >>> If we are really concerned about this case it would also be an option > >>> to limit the adjustment to the max frequency. > >>> > To resolve the conflict for multiple device driver, maybe OPP > interface > have to support 'usage_count' such as clk_enable/disable(). > >>> > >>> This would require supporting negative usage count values, since a OPP > >>> should not be enabled if e.g. thermal enables it but the throttler > >>> disabled it or viceversa. > >>> > >>> Theoretically there could also be conflicts, like one driver disabling > >>> the higher OPPs and another the lower ones, with the outcome of all > >>> OPPs being disabled, which would be a more drastic conflict resolution > >>> than that of devfreq_verify_within_limits(). > >>> > >>> Viresh, what do you think about an OPP usage count? > >> > >> Ping, can we try to reach a conclusion on this or at least keep the > >> discussion going? > >> > >> Not that it matters, but my preferred solution continues to be > >> devfreq_verify_within_limits(). It solves conflicts in some way (which > >> could be adjusted if needed) and has proven to work in practice for > >> 10+ years in a very similar sub-system. > >
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Fri, Aug 03, 2018 at 09:14:46AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: > > On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: > >> Hi Chanwoo, > >> > >> On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > >>> Hi Matthias, > >>> > >>> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > > On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > >> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > >>> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > > >> Firstly, > >> I'm not sure why devfreq needs the devfreq_verify_within_limits() > >> function. > >> > >> devfreq already used the OPP interface as default. It means that > >> the outside of 'drivers/devfreq' can disable/enable the frequency > >> such as drivers/thermal/devfreq_cooling.c. Also, when some device > >> drivers disable/enable the specific frequency, the devfreq core > >> consider them. > >> > >> So, devfreq doesn't need to devfreq_verify_within_limits() because > >> already support some interface to change the minimum/maximum > >> frequency > >> of devfreq device. > >> > >> In case of cpufreq subsystem, cpufreq only provides > >> 'cpufreq_verify_with_limits()' > >> to change the minimum/maximum frequency of cpu. some device driver > >> cannot > >> change the minimum/maximum frequency through OPP interface. > >> > >> But, in case of devfreq subsystem, as I explained already, devfreq > >> support > >> the OPP interface as default way. devfreq subsystem doesn't need > >> to add > >> other way to change the minimum/maximum frequency. > > > > Using the OPP interface exclusively works as long as a > > enabling/disabling of OPPs is limited to a single driver > > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > > involved you need a way to resolve conflicts, that's the purpose of > > devfreq_verify_within_limits(). Please let me know if there are > > existing mechanisms for conflict resolution that I overlooked. > > > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > > devfreq_verify_within_limits() instead of the OPP interface if > > desired, however this seems beyond the scope of this series. > > Actually, if we uses this approach, it doesn't support the multiple > drivers too. > If non throttler drivers uses devfreq_verify_within_limits(), the > conflict > happen. > >>> > >>> As long as drivers limit the max freq there is no conflict, the lowest > >>> max freq wins. I expect this to be the usual case, apparently it > >>> worked for cpufreq for 10+ years. > >>> > >>> However it is correct that there would be a conflict if a driver > >>> requests a min freq that is higher than the max freq requested by > >>> another. In this case devfreq_verify_within_limits() resolves the > >>> conflict by raising p->max to the min freq. Not sure if this is > >>> something that would ever occur in practice though. > >>> > >>> If we are really concerned about this case it would also be an option > >>> to limit the adjustment to the max frequency. > >>> > To resolve the conflict for multiple device driver, maybe OPP > interface > have to support 'usage_count' such as clk_enable/disable(). > >>> > >>> This would require supporting negative usage count values, since a OPP > >>> should not be enabled if e.g. thermal enables it but the throttler > >>> disabled it or viceversa. > >>> > >>> Theoretically there could also be conflicts, like one driver disabling > >>> the higher OPPs and another the lower ones, with the outcome of all > >>> OPPs being disabled, which would be a more drastic conflict resolution > >>> than that of devfreq_verify_within_limits(). > >>> > >>> Viresh, what do you think about an OPP usage count? > >> > >> Ping, can we try to reach a conclusion on this or at least keep the > >> discussion going? > >> > >> Not that it matters, but my preferred solution continues to be > >> devfreq_verify_within_limits(). It solves conflicts in some way (which > >> could be adjusted if needed) and has proven to work in practice for > >> 10+ years in a very similar sub-system. > >
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Fri, Aug 03, 2018 at 08:56:57AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 03일 08:13, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > >> Hi Matthias, > >> > >> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > >>> Hi Chanwoo, > >>> > >>> On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > > On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > >> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > >>> Hi Matthias, > >>> > >>> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > > Firstly, > > I'm not sure why devfreq needs the devfreq_verify_within_limits() > > function. > > > > devfreq already used the OPP interface as default. It means that > > the outside of 'drivers/devfreq' can disable/enable the frequency > > such as drivers/thermal/devfreq_cooling.c. Also, when some device > > drivers disable/enable the specific frequency, the devfreq core > > consider them. > > > > So, devfreq doesn't need to devfreq_verify_within_limits() because > > already support some interface to change the minimum/maximum > > frequency > > of devfreq device. > > > > In case of cpufreq subsystem, cpufreq only provides > > 'cpufreq_verify_with_limits()' > > to change the minimum/maximum frequency of cpu. some device driver > > cannot > > change the minimum/maximum frequency through OPP interface. > > > > But, in case of devfreq subsystem, as I explained already, devfreq > > support > > the OPP interface as default way. devfreq subsystem doesn't need to > > add > > other way to change the minimum/maximum frequency. > > Using the OPP interface exclusively works as long as a > enabling/disabling of OPPs is limited to a single driver > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > involved you need a way to resolve conflicts, that's the purpose of > devfreq_verify_within_limits(). Please let me know if there are > existing mechanisms for conflict resolution that I overlooked. > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > devfreq_verify_within_limits() instead of the OPP interface if > desired, however this seems beyond the scope of this series. > >>> > >>> Actually, if we uses this approach, it doesn't support the multiple > >>> drivers too. > >>> If non throttler drivers uses devfreq_verify_within_limits(), the > >>> conflict > >>> happen. > >> > >> As long as drivers limit the max freq there is no conflict, the lowest > >> max freq wins. I expect this to be the usual case, apparently it > >> worked for cpufreq for 10+ years. > >> > >> However it is correct that there would be a conflict if a driver > >> requests a min freq that is higher than the max freq requested by > >> another. In this case devfreq_verify_within_limits() resolves the > >> conflict by raising p->max to the min freq. Not sure if this is > >> something that would ever occur in practice though. > >> > >> If we are really concerned about this case it would also be an option > >> to limit the adjustment to the max frequency. > >> > >>> To resolve the conflict for multiple device driver, maybe OPP > >>> interface > >>> have to support 'usage_count' such as clk_enable/disable(). > >> > >> This would require supporting negative usage count values, since a OPP > >> should not be enabled if e.g. thermal enables it but the throttler > >> disabled it or viceversa. > >> > >> Theoretically there could also be conflicts, like one driver disabling > >> the higher OPPs and another the lower ones, with the outcome of all > >> OPPs being disabled, which would be a more drastic conflict resolution > >> than that of devfreq_verify_within_limits(). > >> > >> Viresh, what do you think about an OPP usage count? > > > > Ping, can we try to reach a conclusion on this or at least keep the > > discussion going? > > > > Not that it matters, but my preferred solution continues to be > > devfreq_verify_within_limits(). It solves conflicts in some way (which > > could be adjusted if needed) and has proven to work in practice for > > 10+ years in a very similar sub-system. > > It is not true. Current cpufreq subsystem doesn't support external OPP > control to enable/disable the OPP entry. If some device driver > controls the
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Fri, Aug 03, 2018 at 08:56:57AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 03일 08:13, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > >> Hi Matthias, > >> > >> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > >>> Hi Chanwoo, > >>> > >>> On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > > On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > >> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > >>> Hi Matthias, > >>> > >>> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > > Firstly, > > I'm not sure why devfreq needs the devfreq_verify_within_limits() > > function. > > > > devfreq already used the OPP interface as default. It means that > > the outside of 'drivers/devfreq' can disable/enable the frequency > > such as drivers/thermal/devfreq_cooling.c. Also, when some device > > drivers disable/enable the specific frequency, the devfreq core > > consider them. > > > > So, devfreq doesn't need to devfreq_verify_within_limits() because > > already support some interface to change the minimum/maximum > > frequency > > of devfreq device. > > > > In case of cpufreq subsystem, cpufreq only provides > > 'cpufreq_verify_with_limits()' > > to change the minimum/maximum frequency of cpu. some device driver > > cannot > > change the minimum/maximum frequency through OPP interface. > > > > But, in case of devfreq subsystem, as I explained already, devfreq > > support > > the OPP interface as default way. devfreq subsystem doesn't need to > > add > > other way to change the minimum/maximum frequency. > > Using the OPP interface exclusively works as long as a > enabling/disabling of OPPs is limited to a single driver > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > involved you need a way to resolve conflicts, that's the purpose of > devfreq_verify_within_limits(). Please let me know if there are > existing mechanisms for conflict resolution that I overlooked. > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > devfreq_verify_within_limits() instead of the OPP interface if > desired, however this seems beyond the scope of this series. > >>> > >>> Actually, if we uses this approach, it doesn't support the multiple > >>> drivers too. > >>> If non throttler drivers uses devfreq_verify_within_limits(), the > >>> conflict > >>> happen. > >> > >> As long as drivers limit the max freq there is no conflict, the lowest > >> max freq wins. I expect this to be the usual case, apparently it > >> worked for cpufreq for 10+ years. > >> > >> However it is correct that there would be a conflict if a driver > >> requests a min freq that is higher than the max freq requested by > >> another. In this case devfreq_verify_within_limits() resolves the > >> conflict by raising p->max to the min freq. Not sure if this is > >> something that would ever occur in practice though. > >> > >> If we are really concerned about this case it would also be an option > >> to limit the adjustment to the max frequency. > >> > >>> To resolve the conflict for multiple device driver, maybe OPP > >>> interface > >>> have to support 'usage_count' such as clk_enable/disable(). > >> > >> This would require supporting negative usage count values, since a OPP > >> should not be enabled if e.g. thermal enables it but the throttler > >> disabled it or viceversa. > >> > >> Theoretically there could also be conflicts, like one driver disabling > >> the higher OPPs and another the lower ones, with the outcome of all > >> OPPs being disabled, which would be a more drastic conflict resolution > >> than that of devfreq_verify_within_limits(). > >> > >> Viresh, what do you think about an OPP usage count? > > > > Ping, can we try to reach a conclusion on this or at least keep the > > discussion going? > > > > Not that it matters, but my preferred solution continues to be > > devfreq_verify_within_limits(). It solves conflicts in some way (which > > could be adjusted if needed) and has proven to work in practice for > > 10+ years in a very similar sub-system. > > It is not true. Current cpufreq subsystem doesn't support external OPP > control to enable/disable the OPP entry. If some device driver > controls the
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: > On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: >> Hi Chanwoo, >> >> On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: >>> Hi Matthias, >>> >>> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: Hi Chanwoo, On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: >> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: >>> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: Hi Matthias, On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > >> Firstly, >> I'm not sure why devfreq needs the devfreq_verify_within_limits() >> function. >> >> devfreq already used the OPP interface as default. It means that >> the outside of 'drivers/devfreq' can disable/enable the frequency >> such as drivers/thermal/devfreq_cooling.c. Also, when some device >> drivers disable/enable the specific frequency, the devfreq core >> consider them. >> >> So, devfreq doesn't need to devfreq_verify_within_limits() because >> already support some interface to change the minimum/maximum >> frequency >> of devfreq device. >> >> In case of cpufreq subsystem, cpufreq only provides >> 'cpufreq_verify_with_limits()' >> to change the minimum/maximum frequency of cpu. some device driver >> cannot >> change the minimum/maximum frequency through OPP interface. >> >> But, in case of devfreq subsystem, as I explained already, devfreq >> support >> the OPP interface as default way. devfreq subsystem doesn't need to >> add >> other way to change the minimum/maximum frequency. > > Using the OPP interface exclusively works as long as a > enabling/disabling of OPPs is limited to a single driver > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > involved you need a way to resolve conflicts, that's the purpose of > devfreq_verify_within_limits(). Please let me know if there are > existing mechanisms for conflict resolution that I overlooked. > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > devfreq_verify_within_limits() instead of the OPP interface if > desired, however this seems beyond the scope of this series. Actually, if we uses this approach, it doesn't support the multiple drivers too. If non throttler drivers uses devfreq_verify_within_limits(), the conflict happen. >>> >>> As long as drivers limit the max freq there is no conflict, the lowest >>> max freq wins. I expect this to be the usual case, apparently it >>> worked for cpufreq for 10+ years. >>> >>> However it is correct that there would be a conflict if a driver >>> requests a min freq that is higher than the max freq requested by >>> another. In this case devfreq_verify_within_limits() resolves the >>> conflict by raising p->max to the min freq. Not sure if this is >>> something that would ever occur in practice though. >>> >>> If we are really concerned about this case it would also be an option >>> to limit the adjustment to the max frequency. >>> To resolve the conflict for multiple device driver, maybe OPP interface have to support 'usage_count' such as clk_enable/disable(). >>> >>> This would require supporting negative usage count values, since a OPP >>> should not be enabled if e.g. thermal enables it but the throttler >>> disabled it or viceversa. >>> >>> Theoretically there could also be conflicts, like one driver disabling >>> the higher OPPs and another the lower ones, with the outcome of all >>> OPPs being disabled, which would be a more drastic conflict resolution >>> than that of devfreq_verify_within_limits(). >>> >>> Viresh, what do you think about an OPP usage count? >> >> Ping, can we try to reach a conclusion on this or at least keep the >> discussion going? >> >> Not that it matters, but my preferred solution continues to be >> devfreq_verify_within_limits(). It solves conflicts in some way (which >> could be adjusted if needed) and has proven to work in practice for >> 10+ years in a very similar sub-system. > > It is not true. Current cpufreq subsystem doesn't support external OPP > control to enable/disable the OPP entry. If some device driver > controls the OPP entry of cpufreq driver with opp_disable/enable(), > the operation is not working. Because cpufreq considers the
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 08월 03일 08:48, Matthias Kaehlcke wrote: > On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: >> Hi Chanwoo, >> >> On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: >>> Hi Matthias, >>> >>> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: Hi Chanwoo, On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: >> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: >>> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: Hi Matthias, On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > >> Firstly, >> I'm not sure why devfreq needs the devfreq_verify_within_limits() >> function. >> >> devfreq already used the OPP interface as default. It means that >> the outside of 'drivers/devfreq' can disable/enable the frequency >> such as drivers/thermal/devfreq_cooling.c. Also, when some device >> drivers disable/enable the specific frequency, the devfreq core >> consider them. >> >> So, devfreq doesn't need to devfreq_verify_within_limits() because >> already support some interface to change the minimum/maximum >> frequency >> of devfreq device. >> >> In case of cpufreq subsystem, cpufreq only provides >> 'cpufreq_verify_with_limits()' >> to change the minimum/maximum frequency of cpu. some device driver >> cannot >> change the minimum/maximum frequency through OPP interface. >> >> But, in case of devfreq subsystem, as I explained already, devfreq >> support >> the OPP interface as default way. devfreq subsystem doesn't need to >> add >> other way to change the minimum/maximum frequency. > > Using the OPP interface exclusively works as long as a > enabling/disabling of OPPs is limited to a single driver > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > involved you need a way to resolve conflicts, that's the purpose of > devfreq_verify_within_limits(). Please let me know if there are > existing mechanisms for conflict resolution that I overlooked. > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > devfreq_verify_within_limits() instead of the OPP interface if > desired, however this seems beyond the scope of this series. Actually, if we uses this approach, it doesn't support the multiple drivers too. If non throttler drivers uses devfreq_verify_within_limits(), the conflict happen. >>> >>> As long as drivers limit the max freq there is no conflict, the lowest >>> max freq wins. I expect this to be the usual case, apparently it >>> worked for cpufreq for 10+ years. >>> >>> However it is correct that there would be a conflict if a driver >>> requests a min freq that is higher than the max freq requested by >>> another. In this case devfreq_verify_within_limits() resolves the >>> conflict by raising p->max to the min freq. Not sure if this is >>> something that would ever occur in practice though. >>> >>> If we are really concerned about this case it would also be an option >>> to limit the adjustment to the max frequency. >>> To resolve the conflict for multiple device driver, maybe OPP interface have to support 'usage_count' such as clk_enable/disable(). >>> >>> This would require supporting negative usage count values, since a OPP >>> should not be enabled if e.g. thermal enables it but the throttler >>> disabled it or viceversa. >>> >>> Theoretically there could also be conflicts, like one driver disabling >>> the higher OPPs and another the lower ones, with the outcome of all >>> OPPs being disabled, which would be a more drastic conflict resolution >>> than that of devfreq_verify_within_limits(). >>> >>> Viresh, what do you think about an OPP usage count? >> >> Ping, can we try to reach a conclusion on this or at least keep the >> discussion going? >> >> Not that it matters, but my preferred solution continues to be >> devfreq_verify_within_limits(). It solves conflicts in some way (which >> could be adjusted if needed) and has proven to work in practice for >> 10+ years in a very similar sub-system. > > It is not true. Current cpufreq subsystem doesn't support external OPP > control to enable/disable the OPP entry. If some device driver > controls the OPP entry of cpufreq driver with opp_disable/enable(), > the operation is not working. Because cpufreq considers the
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 08월 03일 08:13, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: >>> Hi Chanwoo, >>> >>> On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: >> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: >>> Hi Matthias, >>> >>> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: Hi Chanwoo, On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > Firstly, > I'm not sure why devfreq needs the devfreq_verify_within_limits() > function. > > devfreq already used the OPP interface as default. It means that > the outside of 'drivers/devfreq' can disable/enable the frequency > such as drivers/thermal/devfreq_cooling.c. Also, when some device > drivers disable/enable the specific frequency, the devfreq core > consider them. > > So, devfreq doesn't need to devfreq_verify_within_limits() because > already support some interface to change the minimum/maximum frequency > of devfreq device. > > In case of cpufreq subsystem, cpufreq only provides > 'cpufreq_verify_with_limits()' > to change the minimum/maximum frequency of cpu. some device driver > cannot > change the minimum/maximum frequency through OPP interface. > > But, in case of devfreq subsystem, as I explained already, devfreq > support > the OPP interface as default way. devfreq subsystem doesn't need to > add > other way to change the minimum/maximum frequency. Using the OPP interface exclusively works as long as a enabling/disabling of OPPs is limited to a single driver (drivers/thermal/devfreq_cooling.c). When multiple drivers are involved you need a way to resolve conflicts, that's the purpose of devfreq_verify_within_limits(). Please let me know if there are existing mechanisms for conflict resolution that I overlooked. Possibly drivers/thermal/devfreq_cooling.c could be migrated to use devfreq_verify_within_limits() instead of the OPP interface if desired, however this seems beyond the scope of this series. >>> >>> Actually, if we uses this approach, it doesn't support the multiple >>> drivers too. >>> If non throttler drivers uses devfreq_verify_within_limits(), the >>> conflict >>> happen. >> >> As long as drivers limit the max freq there is no conflict, the lowest >> max freq wins. I expect this to be the usual case, apparently it >> worked for cpufreq for 10+ years. >> >> However it is correct that there would be a conflict if a driver >> requests a min freq that is higher than the max freq requested by >> another. In this case devfreq_verify_within_limits() resolves the >> conflict by raising p->max to the min freq. Not sure if this is >> something that would ever occur in practice though. >> >> If we are really concerned about this case it would also be an option >> to limit the adjustment to the max frequency. >> >>> To resolve the conflict for multiple device driver, maybe OPP interface >>> have to support 'usage_count' such as clk_enable/disable(). >> >> This would require supporting negative usage count values, since a OPP >> should not be enabled if e.g. thermal enables it but the throttler >> disabled it or viceversa. >> >> Theoretically there could also be conflicts, like one driver disabling >> the higher OPPs and another the lower ones, with the outcome of all >> OPPs being disabled, which would be a more drastic conflict resolution >> than that of devfreq_verify_within_limits(). >> >> Viresh, what do you think about an OPP usage count? > > Ping, can we try to reach a conclusion on this or at least keep the > discussion going? > > Not that it matters, but my preferred solution continues to be > devfreq_verify_within_limits(). It solves conflicts in some way (which > could be adjusted if needed) and has proven to work in practice for > 10+ years in a very similar sub-system. It is not true. Current cpufreq subsystem doesn't support external OPP control to enable/disable the OPP entry. If some device driver controls the OPP entry of cpufreq driver with opp_disable/enable(), the operation is not working. Because cpufreq considers the limit through 'cpufreq_verify_with_limits()' only. >>> >>> Ok, we can probably agree that using cpufreq_verify_with_limits() >>> exclusively seems to have worked well for cpufreq,
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 08월 03일 08:13, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: >> Hi Matthias, >> >> On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: >>> Hi Chanwoo, >>> >>> On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: >> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: >>> Hi Matthias, >>> >>> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: Hi Chanwoo, On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > Firstly, > I'm not sure why devfreq needs the devfreq_verify_within_limits() > function. > > devfreq already used the OPP interface as default. It means that > the outside of 'drivers/devfreq' can disable/enable the frequency > such as drivers/thermal/devfreq_cooling.c. Also, when some device > drivers disable/enable the specific frequency, the devfreq core > consider them. > > So, devfreq doesn't need to devfreq_verify_within_limits() because > already support some interface to change the minimum/maximum frequency > of devfreq device. > > In case of cpufreq subsystem, cpufreq only provides > 'cpufreq_verify_with_limits()' > to change the minimum/maximum frequency of cpu. some device driver > cannot > change the minimum/maximum frequency through OPP interface. > > But, in case of devfreq subsystem, as I explained already, devfreq > support > the OPP interface as default way. devfreq subsystem doesn't need to > add > other way to change the minimum/maximum frequency. Using the OPP interface exclusively works as long as a enabling/disabling of OPPs is limited to a single driver (drivers/thermal/devfreq_cooling.c). When multiple drivers are involved you need a way to resolve conflicts, that's the purpose of devfreq_verify_within_limits(). Please let me know if there are existing mechanisms for conflict resolution that I overlooked. Possibly drivers/thermal/devfreq_cooling.c could be migrated to use devfreq_verify_within_limits() instead of the OPP interface if desired, however this seems beyond the scope of this series. >>> >>> Actually, if we uses this approach, it doesn't support the multiple >>> drivers too. >>> If non throttler drivers uses devfreq_verify_within_limits(), the >>> conflict >>> happen. >> >> As long as drivers limit the max freq there is no conflict, the lowest >> max freq wins. I expect this to be the usual case, apparently it >> worked for cpufreq for 10+ years. >> >> However it is correct that there would be a conflict if a driver >> requests a min freq that is higher than the max freq requested by >> another. In this case devfreq_verify_within_limits() resolves the >> conflict by raising p->max to the min freq. Not sure if this is >> something that would ever occur in practice though. >> >> If we are really concerned about this case it would also be an option >> to limit the adjustment to the max frequency. >> >>> To resolve the conflict for multiple device driver, maybe OPP interface >>> have to support 'usage_count' such as clk_enable/disable(). >> >> This would require supporting negative usage count values, since a OPP >> should not be enabled if e.g. thermal enables it but the throttler >> disabled it or viceversa. >> >> Theoretically there could also be conflicts, like one driver disabling >> the higher OPPs and another the lower ones, with the outcome of all >> OPPs being disabled, which would be a more drastic conflict resolution >> than that of devfreq_verify_within_limits(). >> >> Viresh, what do you think about an OPP usage count? > > Ping, can we try to reach a conclusion on this or at least keep the > discussion going? > > Not that it matters, but my preferred solution continues to be > devfreq_verify_within_limits(). It solves conflicts in some way (which > could be adjusted if needed) and has proven to work in practice for > 10+ years in a very similar sub-system. It is not true. Current cpufreq subsystem doesn't support external OPP control to enable/disable the OPP entry. If some device driver controls the OPP entry of cpufreq driver with opp_disable/enable(), the operation is not working. Because cpufreq considers the limit through 'cpufreq_verify_with_limits()' only. >>> >>> Ok, we can probably agree that using cpufreq_verify_with_limits() >>> exclusively seems to have worked well for cpufreq,
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > > Hi Matthias, > > > > On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > > > Hi Chanwoo, > > > > > > On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > > >> On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > > >>> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > > On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > > > Hi Matthias, > > > > > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > > >> Hi Chanwoo, > > >> > > >> On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > >> > > >>> Firstly, > > >>> I'm not sure why devfreq needs the devfreq_verify_within_limits() > > >>> function. > > >>> > > >>> devfreq already used the OPP interface as default. It means that > > >>> the outside of 'drivers/devfreq' can disable/enable the frequency > > >>> such as drivers/thermal/devfreq_cooling.c. Also, when some device > > >>> drivers disable/enable the specific frequency, the devfreq core > > >>> consider them. > > >>> > > >>> So, devfreq doesn't need to devfreq_verify_within_limits() because > > >>> already support some interface to change the minimum/maximum > > >>> frequency > > >>> of devfreq device. > > >>> > > >>> In case of cpufreq subsystem, cpufreq only provides > > >>> 'cpufreq_verify_with_limits()' > > >>> to change the minimum/maximum frequency of cpu. some device driver > > >>> cannot > > >>> change the minimum/maximum frequency through OPP interface. > > >>> > > >>> But, in case of devfreq subsystem, as I explained already, devfreq > > >>> support > > >>> the OPP interface as default way. devfreq subsystem doesn't need to > > >>> add > > >>> other way to change the minimum/maximum frequency. > > >> > > >> Using the OPP interface exclusively works as long as a > > >> enabling/disabling of OPPs is limited to a single driver > > >> (drivers/thermal/devfreq_cooling.c). When multiple drivers are > > >> involved you need a way to resolve conflicts, that's the purpose of > > >> devfreq_verify_within_limits(). Please let me know if there are > > >> existing mechanisms for conflict resolution that I overlooked. > > >> > > >> Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > > >> devfreq_verify_within_limits() instead of the OPP interface if > > >> desired, however this seems beyond the scope of this series. > > > > > > Actually, if we uses this approach, it doesn't support the multiple > > > drivers too. > > > If non throttler drivers uses devfreq_verify_within_limits(), the > > > conflict > > > happen. > > > > As long as drivers limit the max freq there is no conflict, the lowest > > max freq wins. I expect this to be the usual case, apparently it > > worked for cpufreq for 10+ years. > > > > However it is correct that there would be a conflict if a driver > > requests a min freq that is higher than the max freq requested by > > another. In this case devfreq_verify_within_limits() resolves the > > conflict by raising p->max to the min freq. Not sure if this is > > something that would ever occur in practice though. > > > > If we are really concerned about this case it would also be an option > > to limit the adjustment to the max frequency. > > > > > To resolve the conflict for multiple device driver, maybe OPP > > > interface > > > have to support 'usage_count' such as clk_enable/disable(). > > > > This would require supporting negative usage count values, since a OPP > > should not be enabled if e.g. thermal enables it but the throttler > > disabled it or viceversa. > > > > Theoretically there could also be conflicts, like one driver disabling > > the higher OPPs and another the lower ones, with the outcome of all > > OPPs being disabled, which would be a more drastic conflict resolution > > than that of devfreq_verify_within_limits(). > > > > Viresh, what do you think about an OPP usage count? > > >>> > > >>> Ping, can we try to reach a conclusion on this or at least keep the > > >>> discussion going? > > >>> > > >>> Not that it matters, but my preferred solution continues to be > > >>> devfreq_verify_within_limits(). It solves conflicts in some way (which > > >>> could be adjusted if needed) and has proven to work in practice for > > >>> 10+ years in a very similar sub-system. > > >> > > >> It is not true. Current cpufreq subsystem doesn't support external OPP > > >> control to enable/disable the OPP entry. If some device driver > > >> controls the OPP entry of cpufreq driver with opp_disable/enable(), > > >> the operation is not
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
On Thu, Aug 02, 2018 at 04:13:43PM -0700, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > > Hi Matthias, > > > > On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > > > Hi Chanwoo, > > > > > > On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > > >> On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > > >>> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > > On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > > > Hi Matthias, > > > > > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > > >> Hi Chanwoo, > > >> > > >> On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > >> > > >>> Firstly, > > >>> I'm not sure why devfreq needs the devfreq_verify_within_limits() > > >>> function. > > >>> > > >>> devfreq already used the OPP interface as default. It means that > > >>> the outside of 'drivers/devfreq' can disable/enable the frequency > > >>> such as drivers/thermal/devfreq_cooling.c. Also, when some device > > >>> drivers disable/enable the specific frequency, the devfreq core > > >>> consider them. > > >>> > > >>> So, devfreq doesn't need to devfreq_verify_within_limits() because > > >>> already support some interface to change the minimum/maximum > > >>> frequency > > >>> of devfreq device. > > >>> > > >>> In case of cpufreq subsystem, cpufreq only provides > > >>> 'cpufreq_verify_with_limits()' > > >>> to change the minimum/maximum frequency of cpu. some device driver > > >>> cannot > > >>> change the minimum/maximum frequency through OPP interface. > > >>> > > >>> But, in case of devfreq subsystem, as I explained already, devfreq > > >>> support > > >>> the OPP interface as default way. devfreq subsystem doesn't need to > > >>> add > > >>> other way to change the minimum/maximum frequency. > > >> > > >> Using the OPP interface exclusively works as long as a > > >> enabling/disabling of OPPs is limited to a single driver > > >> (drivers/thermal/devfreq_cooling.c). When multiple drivers are > > >> involved you need a way to resolve conflicts, that's the purpose of > > >> devfreq_verify_within_limits(). Please let me know if there are > > >> existing mechanisms for conflict resolution that I overlooked. > > >> > > >> Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > > >> devfreq_verify_within_limits() instead of the OPP interface if > > >> desired, however this seems beyond the scope of this series. > > > > > > Actually, if we uses this approach, it doesn't support the multiple > > > drivers too. > > > If non throttler drivers uses devfreq_verify_within_limits(), the > > > conflict > > > happen. > > > > As long as drivers limit the max freq there is no conflict, the lowest > > max freq wins. I expect this to be the usual case, apparently it > > worked for cpufreq for 10+ years. > > > > However it is correct that there would be a conflict if a driver > > requests a min freq that is higher than the max freq requested by > > another. In this case devfreq_verify_within_limits() resolves the > > conflict by raising p->max to the min freq. Not sure if this is > > something that would ever occur in practice though. > > > > If we are really concerned about this case it would also be an option > > to limit the adjustment to the max frequency. > > > > > To resolve the conflict for multiple device driver, maybe OPP > > > interface > > > have to support 'usage_count' such as clk_enable/disable(). > > > > This would require supporting negative usage count values, since a OPP > > should not be enabled if e.g. thermal enables it but the throttler > > disabled it or viceversa. > > > > Theoretically there could also be conflicts, like one driver disabling > > the higher OPPs and another the lower ones, with the outcome of all > > OPPs being disabled, which would be a more drastic conflict resolution > > than that of devfreq_verify_within_limits(). > > > > Viresh, what do you think about an OPP usage count? > > >>> > > >>> Ping, can we try to reach a conclusion on this or at least keep the > > >>> discussion going? > > >>> > > >>> Not that it matters, but my preferred solution continues to be > > >>> devfreq_verify_within_limits(). It solves conflicts in some way (which > > >>> could be adjusted if needed) and has proven to work in practice for > > >>> 10+ years in a very similar sub-system. > > >> > > >> It is not true. Current cpufreq subsystem doesn't support external OPP > > >> control to enable/disable the OPP entry. If some device driver > > >> controls the OPP entry of cpufreq driver with opp_disable/enable(), > > >> the operation is not
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > >> On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > >>> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > > Hi Matthias, > > > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > >> Hi Chanwoo, > >> > >> On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > >> > >>> Firstly, > >>> I'm not sure why devfreq needs the devfreq_verify_within_limits() > >>> function. > >>> > >>> devfreq already used the OPP interface as default. It means that > >>> the outside of 'drivers/devfreq' can disable/enable the frequency > >>> such as drivers/thermal/devfreq_cooling.c. Also, when some device > >>> drivers disable/enable the specific frequency, the devfreq core > >>> consider them. > >>> > >>> So, devfreq doesn't need to devfreq_verify_within_limits() because > >>> already support some interface to change the minimum/maximum frequency > >>> of devfreq device. > >>> > >>> In case of cpufreq subsystem, cpufreq only provides > >>> 'cpufreq_verify_with_limits()' > >>> to change the minimum/maximum frequency of cpu. some device driver > >>> cannot > >>> change the minimum/maximum frequency through OPP interface. > >>> > >>> But, in case of devfreq subsystem, as I explained already, devfreq > >>> support > >>> the OPP interface as default way. devfreq subsystem doesn't need to > >>> add > >>> other way to change the minimum/maximum frequency. > >> > >> Using the OPP interface exclusively works as long as a > >> enabling/disabling of OPPs is limited to a single driver > >> (drivers/thermal/devfreq_cooling.c). When multiple drivers are > >> involved you need a way to resolve conflicts, that's the purpose of > >> devfreq_verify_within_limits(). Please let me know if there are > >> existing mechanisms for conflict resolution that I overlooked. > >> > >> Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > >> devfreq_verify_within_limits() instead of the OPP interface if > >> desired, however this seems beyond the scope of this series. > > > > Actually, if we uses this approach, it doesn't support the multiple > > drivers too. > > If non throttler drivers uses devfreq_verify_within_limits(), the > > conflict > > happen. > > As long as drivers limit the max freq there is no conflict, the lowest > max freq wins. I expect this to be the usual case, apparently it > worked for cpufreq for 10+ years. > > However it is correct that there would be a conflict if a driver > requests a min freq that is higher than the max freq requested by > another. In this case devfreq_verify_within_limits() resolves the > conflict by raising p->max to the min freq. Not sure if this is > something that would ever occur in practice though. > > If we are really concerned about this case it would also be an option > to limit the adjustment to the max frequency. > > > To resolve the conflict for multiple device driver, maybe OPP interface > > have to support 'usage_count' such as clk_enable/disable(). > > This would require supporting negative usage count values, since a OPP > should not be enabled if e.g. thermal enables it but the throttler > disabled it or viceversa. > > Theoretically there could also be conflicts, like one driver disabling > the higher OPPs and another the lower ones, with the outcome of all > OPPs being disabled, which would be a more drastic conflict resolution > than that of devfreq_verify_within_limits(). > > Viresh, what do you think about an OPP usage count? > >>> > >>> Ping, can we try to reach a conclusion on this or at least keep the > >>> discussion going? > >>> > >>> Not that it matters, but my preferred solution continues to be > >>> devfreq_verify_within_limits(). It solves conflicts in some way (which > >>> could be adjusted if needed) and has proven to work in practice for > >>> 10+ years in a very similar sub-system. > >> > >> It is not true. Current cpufreq subsystem doesn't support external OPP > >> control to enable/disable the OPP entry. If some device driver > >> controls the OPP entry of cpufreq driver with opp_disable/enable(), > >> the operation is not working. Because cpufreq considers the limit > >> through 'cpufreq_verify_with_limits()' only. > > > > Ok, we can probably agree that using cpufreq_verify_with_limits() > > exclusively seems to have worked well for cpufreq, and that in their > > overall purpose cpufreq and devfreq are similar
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Thu, Aug 02, 2018 at 10:58:59AM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > >> On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > >>> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > > Hi Matthias, > > > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > >> Hi Chanwoo, > >> > >> On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > >> > >>> Firstly, > >>> I'm not sure why devfreq needs the devfreq_verify_within_limits() > >>> function. > >>> > >>> devfreq already used the OPP interface as default. It means that > >>> the outside of 'drivers/devfreq' can disable/enable the frequency > >>> such as drivers/thermal/devfreq_cooling.c. Also, when some device > >>> drivers disable/enable the specific frequency, the devfreq core > >>> consider them. > >>> > >>> So, devfreq doesn't need to devfreq_verify_within_limits() because > >>> already support some interface to change the minimum/maximum frequency > >>> of devfreq device. > >>> > >>> In case of cpufreq subsystem, cpufreq only provides > >>> 'cpufreq_verify_with_limits()' > >>> to change the minimum/maximum frequency of cpu. some device driver > >>> cannot > >>> change the minimum/maximum frequency through OPP interface. > >>> > >>> But, in case of devfreq subsystem, as I explained already, devfreq > >>> support > >>> the OPP interface as default way. devfreq subsystem doesn't need to > >>> add > >>> other way to change the minimum/maximum frequency. > >> > >> Using the OPP interface exclusively works as long as a > >> enabling/disabling of OPPs is limited to a single driver > >> (drivers/thermal/devfreq_cooling.c). When multiple drivers are > >> involved you need a way to resolve conflicts, that's the purpose of > >> devfreq_verify_within_limits(). Please let me know if there are > >> existing mechanisms for conflict resolution that I overlooked. > >> > >> Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > >> devfreq_verify_within_limits() instead of the OPP interface if > >> desired, however this seems beyond the scope of this series. > > > > Actually, if we uses this approach, it doesn't support the multiple > > drivers too. > > If non throttler drivers uses devfreq_verify_within_limits(), the > > conflict > > happen. > > As long as drivers limit the max freq there is no conflict, the lowest > max freq wins. I expect this to be the usual case, apparently it > worked for cpufreq for 10+ years. > > However it is correct that there would be a conflict if a driver > requests a min freq that is higher than the max freq requested by > another. In this case devfreq_verify_within_limits() resolves the > conflict by raising p->max to the min freq. Not sure if this is > something that would ever occur in practice though. > > If we are really concerned about this case it would also be an option > to limit the adjustment to the max frequency. > > > To resolve the conflict for multiple device driver, maybe OPP interface > > have to support 'usage_count' such as clk_enable/disable(). > > This would require supporting negative usage count values, since a OPP > should not be enabled if e.g. thermal enables it but the throttler > disabled it or viceversa. > > Theoretically there could also be conflicts, like one driver disabling > the higher OPPs and another the lower ones, with the outcome of all > OPPs being disabled, which would be a more drastic conflict resolution > than that of devfreq_verify_within_limits(). > > Viresh, what do you think about an OPP usage count? > >>> > >>> Ping, can we try to reach a conclusion on this or at least keep the > >>> discussion going? > >>> > >>> Not that it matters, but my preferred solution continues to be > >>> devfreq_verify_within_limits(). It solves conflicts in some way (which > >>> could be adjusted if needed) and has proven to work in practice for > >>> 10+ years in a very similar sub-system. > >> > >> It is not true. Current cpufreq subsystem doesn't support external OPP > >> control to enable/disable the OPP entry. If some device driver > >> controls the OPP entry of cpufreq driver with opp_disable/enable(), > >> the operation is not working. Because cpufreq considers the limit > >> through 'cpufreq_verify_with_limits()' only. > > > > Ok, we can probably agree that using cpufreq_verify_with_limits() > > exclusively seems to have worked well for cpufreq, and that in their > > overall purpose cpufreq and devfreq are similar
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: >> On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: >>> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: >> Hi Chanwoo, >> >> On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: >> >>> Firstly, >>> I'm not sure why devfreq needs the devfreq_verify_within_limits() >>> function. >>> >>> devfreq already used the OPP interface as default. It means that >>> the outside of 'drivers/devfreq' can disable/enable the frequency >>> such as drivers/thermal/devfreq_cooling.c. Also, when some device >>> drivers disable/enable the specific frequency, the devfreq core >>> consider them. >>> >>> So, devfreq doesn't need to devfreq_verify_within_limits() because >>> already support some interface to change the minimum/maximum frequency >>> of devfreq device. >>> >>> In case of cpufreq subsystem, cpufreq only provides >>> 'cpufreq_verify_with_limits()' >>> to change the minimum/maximum frequency of cpu. some device driver >>> cannot >>> change the minimum/maximum frequency through OPP interface. >>> >>> But, in case of devfreq subsystem, as I explained already, devfreq >>> support >>> the OPP interface as default way. devfreq subsystem doesn't need to add >>> other way to change the minimum/maximum frequency. >> >> Using the OPP interface exclusively works as long as a >> enabling/disabling of OPPs is limited to a single driver >> (drivers/thermal/devfreq_cooling.c). When multiple drivers are >> involved you need a way to resolve conflicts, that's the purpose of >> devfreq_verify_within_limits(). Please let me know if there are >> existing mechanisms for conflict resolution that I overlooked. >> >> Possibly drivers/thermal/devfreq_cooling.c could be migrated to use >> devfreq_verify_within_limits() instead of the OPP interface if >> desired, however this seems beyond the scope of this series. > > Actually, if we uses this approach, it doesn't support the multiple > drivers too. > If non throttler drivers uses devfreq_verify_within_limits(), the conflict > happen. As long as drivers limit the max freq there is no conflict, the lowest max freq wins. I expect this to be the usual case, apparently it worked for cpufreq for 10+ years. However it is correct that there would be a conflict if a driver requests a min freq that is higher than the max freq requested by another. In this case devfreq_verify_within_limits() resolves the conflict by raising p->max to the min freq. Not sure if this is something that would ever occur in practice though. If we are really concerned about this case it would also be an option to limit the adjustment to the max frequency. > To resolve the conflict for multiple device driver, maybe OPP interface > have to support 'usage_count' such as clk_enable/disable(). This would require supporting negative usage count values, since a OPP should not be enabled if e.g. thermal enables it but the throttler disabled it or viceversa. Theoretically there could also be conflicts, like one driver disabling the higher OPPs and another the lower ones, with the outcome of all OPPs being disabled, which would be a more drastic conflict resolution than that of devfreq_verify_within_limits(). Viresh, what do you think about an OPP usage count? >>> >>> Ping, can we try to reach a conclusion on this or at least keep the >>> discussion going? >>> >>> Not that it matters, but my preferred solution continues to be >>> devfreq_verify_within_limits(). It solves conflicts in some way (which >>> could be adjusted if needed) and has proven to work in practice for >>> 10+ years in a very similar sub-system. >> >> It is not true. Current cpufreq subsystem doesn't support external OPP >> control to enable/disable the OPP entry. If some device driver >> controls the OPP entry of cpufreq driver with opp_disable/enable(), >> the operation is not working. Because cpufreq considers the limit >> through 'cpufreq_verify_with_limits()' only. > > Ok, we can probably agree that using cpufreq_verify_with_limits() > exclusively seems to have worked well for cpufreq, and that in their > overall purpose cpufreq and devfreq are similar subsystems. > > The current throttler series with devfreq_verify_within_limits() takes > the enabled OPPs into account, the lowest and highest OPP are used as > a starting point for the frequency adjustment and (in theory) the > frequency range should only be narrowed by >
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 08월 02일 02:08, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: >> On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: >>> On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: >> Hi Chanwoo, >> >> On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: >> >>> Firstly, >>> I'm not sure why devfreq needs the devfreq_verify_within_limits() >>> function. >>> >>> devfreq already used the OPP interface as default. It means that >>> the outside of 'drivers/devfreq' can disable/enable the frequency >>> such as drivers/thermal/devfreq_cooling.c. Also, when some device >>> drivers disable/enable the specific frequency, the devfreq core >>> consider them. >>> >>> So, devfreq doesn't need to devfreq_verify_within_limits() because >>> already support some interface to change the minimum/maximum frequency >>> of devfreq device. >>> >>> In case of cpufreq subsystem, cpufreq only provides >>> 'cpufreq_verify_with_limits()' >>> to change the minimum/maximum frequency of cpu. some device driver >>> cannot >>> change the minimum/maximum frequency through OPP interface. >>> >>> But, in case of devfreq subsystem, as I explained already, devfreq >>> support >>> the OPP interface as default way. devfreq subsystem doesn't need to add >>> other way to change the minimum/maximum frequency. >> >> Using the OPP interface exclusively works as long as a >> enabling/disabling of OPPs is limited to a single driver >> (drivers/thermal/devfreq_cooling.c). When multiple drivers are >> involved you need a way to resolve conflicts, that's the purpose of >> devfreq_verify_within_limits(). Please let me know if there are >> existing mechanisms for conflict resolution that I overlooked. >> >> Possibly drivers/thermal/devfreq_cooling.c could be migrated to use >> devfreq_verify_within_limits() instead of the OPP interface if >> desired, however this seems beyond the scope of this series. > > Actually, if we uses this approach, it doesn't support the multiple > drivers too. > If non throttler drivers uses devfreq_verify_within_limits(), the conflict > happen. As long as drivers limit the max freq there is no conflict, the lowest max freq wins. I expect this to be the usual case, apparently it worked for cpufreq for 10+ years. However it is correct that there would be a conflict if a driver requests a min freq that is higher than the max freq requested by another. In this case devfreq_verify_within_limits() resolves the conflict by raising p->max to the min freq. Not sure if this is something that would ever occur in practice though. If we are really concerned about this case it would also be an option to limit the adjustment to the max frequency. > To resolve the conflict for multiple device driver, maybe OPP interface > have to support 'usage_count' such as clk_enable/disable(). This would require supporting negative usage count values, since a OPP should not be enabled if e.g. thermal enables it but the throttler disabled it or viceversa. Theoretically there could also be conflicts, like one driver disabling the higher OPPs and another the lower ones, with the outcome of all OPPs being disabled, which would be a more drastic conflict resolution than that of devfreq_verify_within_limits(). Viresh, what do you think about an OPP usage count? >>> >>> Ping, can we try to reach a conclusion on this or at least keep the >>> discussion going? >>> >>> Not that it matters, but my preferred solution continues to be >>> devfreq_verify_within_limits(). It solves conflicts in some way (which >>> could be adjusted if needed) and has proven to work in practice for >>> 10+ years in a very similar sub-system. >> >> It is not true. Current cpufreq subsystem doesn't support external OPP >> control to enable/disable the OPP entry. If some device driver >> controls the OPP entry of cpufreq driver with opp_disable/enable(), >> the operation is not working. Because cpufreq considers the limit >> through 'cpufreq_verify_with_limits()' only. > > Ok, we can probably agree that using cpufreq_verify_with_limits() > exclusively seems to have worked well for cpufreq, and that in their > overall purpose cpufreq and devfreq are similar subsystems. > > The current throttler series with devfreq_verify_within_limits() takes > the enabled OPPs into account, the lowest and highest OPP are used as > a starting point for the frequency adjustment and (in theory) the > frequency range should only be narrowed by >
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > > On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > >> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > >>> Hi Matthias, > >>> > >>> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > > Firstly, > > I'm not sure why devfreq needs the devfreq_verify_within_limits() > > function. > > > > devfreq already used the OPP interface as default. It means that > > the outside of 'drivers/devfreq' can disable/enable the frequency > > such as drivers/thermal/devfreq_cooling.c. Also, when some device > > drivers disable/enable the specific frequency, the devfreq core > > consider them. > > > > So, devfreq doesn't need to devfreq_verify_within_limits() because > > already support some interface to change the minimum/maximum frequency > > of devfreq device. > > > > In case of cpufreq subsystem, cpufreq only provides > > 'cpufreq_verify_with_limits()' > > to change the minimum/maximum frequency of cpu. some device driver > > cannot > > change the minimum/maximum frequency through OPP interface. > > > > But, in case of devfreq subsystem, as I explained already, devfreq > > support > > the OPP interface as default way. devfreq subsystem doesn't need to add > > other way to change the minimum/maximum frequency. > > Using the OPP interface exclusively works as long as a > enabling/disabling of OPPs is limited to a single driver > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > involved you need a way to resolve conflicts, that's the purpose of > devfreq_verify_within_limits(). Please let me know if there are > existing mechanisms for conflict resolution that I overlooked. > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > devfreq_verify_within_limits() instead of the OPP interface if > desired, however this seems beyond the scope of this series. > >>> > >>> Actually, if we uses this approach, it doesn't support the multiple > >>> drivers too. > >>> If non throttler drivers uses devfreq_verify_within_limits(), the conflict > >>> happen. > >> > >> As long as drivers limit the max freq there is no conflict, the lowest > >> max freq wins. I expect this to be the usual case, apparently it > >> worked for cpufreq for 10+ years. > >> > >> However it is correct that there would be a conflict if a driver > >> requests a min freq that is higher than the max freq requested by > >> another. In this case devfreq_verify_within_limits() resolves the > >> conflict by raising p->max to the min freq. Not sure if this is > >> something that would ever occur in practice though. > >> > >> If we are really concerned about this case it would also be an option > >> to limit the adjustment to the max frequency. > >> > >>> To resolve the conflict for multiple device driver, maybe OPP interface > >>> have to support 'usage_count' such as clk_enable/disable(). > >> > >> This would require supporting negative usage count values, since a OPP > >> should not be enabled if e.g. thermal enables it but the throttler > >> disabled it or viceversa. > >> > >> Theoretically there could also be conflicts, like one driver disabling > >> the higher OPPs and another the lower ones, with the outcome of all > >> OPPs being disabled, which would be a more drastic conflict resolution > >> than that of devfreq_verify_within_limits(). > >> > >> Viresh, what do you think about an OPP usage count? > > > > Ping, can we try to reach a conclusion on this or at least keep the > > discussion going? > > > > Not that it matters, but my preferred solution continues to be > > devfreq_verify_within_limits(). It solves conflicts in some way (which > > could be adjusted if needed) and has proven to work in practice for > > 10+ years in a very similar sub-system. > > It is not true. Current cpufreq subsystem doesn't support external OPP > control to enable/disable the OPP entry. If some device driver > controls the OPP entry of cpufreq driver with opp_disable/enable(), > the operation is not working. Because cpufreq considers the limit > through 'cpufreq_verify_with_limits()' only. Ok, we can probably agree that using cpufreq_verify_with_limits() exclusively seems to have worked well for cpufreq, and that in their overall purpose cpufreq and devfreq are similar subsystems. The current throttler series with devfreq_verify_within_limits() takes the enabled OPPs into account, the lowest and highest OPP are used as a starting point for the frequency adjustment and (in theory) the frequency range should only be narrowed by devfreq_verify_within_limits(). > As I already commented[1], there is different between cpufreq and devfreq. >
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Wed, Aug 01, 2018 at 10:22:16AM +0900, Chanwoo Choi wrote: > On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > > On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > >> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > >>> Hi Matthias, > >>> > >>> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > > Firstly, > > I'm not sure why devfreq needs the devfreq_verify_within_limits() > > function. > > > > devfreq already used the OPP interface as default. It means that > > the outside of 'drivers/devfreq' can disable/enable the frequency > > such as drivers/thermal/devfreq_cooling.c. Also, when some device > > drivers disable/enable the specific frequency, the devfreq core > > consider them. > > > > So, devfreq doesn't need to devfreq_verify_within_limits() because > > already support some interface to change the minimum/maximum frequency > > of devfreq device. > > > > In case of cpufreq subsystem, cpufreq only provides > > 'cpufreq_verify_with_limits()' > > to change the minimum/maximum frequency of cpu. some device driver > > cannot > > change the minimum/maximum frequency through OPP interface. > > > > But, in case of devfreq subsystem, as I explained already, devfreq > > support > > the OPP interface as default way. devfreq subsystem doesn't need to add > > other way to change the minimum/maximum frequency. > > Using the OPP interface exclusively works as long as a > enabling/disabling of OPPs is limited to a single driver > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > involved you need a way to resolve conflicts, that's the purpose of > devfreq_verify_within_limits(). Please let me know if there are > existing mechanisms for conflict resolution that I overlooked. > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > devfreq_verify_within_limits() instead of the OPP interface if > desired, however this seems beyond the scope of this series. > >>> > >>> Actually, if we uses this approach, it doesn't support the multiple > >>> drivers too. > >>> If non throttler drivers uses devfreq_verify_within_limits(), the conflict > >>> happen. > >> > >> As long as drivers limit the max freq there is no conflict, the lowest > >> max freq wins. I expect this to be the usual case, apparently it > >> worked for cpufreq for 10+ years. > >> > >> However it is correct that there would be a conflict if a driver > >> requests a min freq that is higher than the max freq requested by > >> another. In this case devfreq_verify_within_limits() resolves the > >> conflict by raising p->max to the min freq. Not sure if this is > >> something that would ever occur in practice though. > >> > >> If we are really concerned about this case it would also be an option > >> to limit the adjustment to the max frequency. > >> > >>> To resolve the conflict for multiple device driver, maybe OPP interface > >>> have to support 'usage_count' such as clk_enable/disable(). > >> > >> This would require supporting negative usage count values, since a OPP > >> should not be enabled if e.g. thermal enables it but the throttler > >> disabled it or viceversa. > >> > >> Theoretically there could also be conflicts, like one driver disabling > >> the higher OPPs and another the lower ones, with the outcome of all > >> OPPs being disabled, which would be a more drastic conflict resolution > >> than that of devfreq_verify_within_limits(). > >> > >> Viresh, what do you think about an OPP usage count? > > > > Ping, can we try to reach a conclusion on this or at least keep the > > discussion going? > > > > Not that it matters, but my preferred solution continues to be > > devfreq_verify_within_limits(). It solves conflicts in some way (which > > could be adjusted if needed) and has proven to work in practice for > > 10+ years in a very similar sub-system. > > It is not true. Current cpufreq subsystem doesn't support external OPP > control to enable/disable the OPP entry. If some device driver > controls the OPP entry of cpufreq driver with opp_disable/enable(), > the operation is not working. Because cpufreq considers the limit > through 'cpufreq_verify_with_limits()' only. Ok, we can probably agree that using cpufreq_verify_with_limits() exclusively seems to have worked well for cpufreq, and that in their overall purpose cpufreq and devfreq are similar subsystems. The current throttler series with devfreq_verify_within_limits() takes the enabled OPPs into account, the lowest and highest OPP are used as a starting point for the frequency adjustment and (in theory) the frequency range should only be narrowed by devfreq_verify_within_limits(). > As I already commented[1], there is different between cpufreq and devfreq. >
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: >> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: >>> Hi Matthias, >>> >>> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: Hi Chanwoo, On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > Firstly, > I'm not sure why devfreq needs the devfreq_verify_within_limits() > function. > > devfreq already used the OPP interface as default. It means that > the outside of 'drivers/devfreq' can disable/enable the frequency > such as drivers/thermal/devfreq_cooling.c. Also, when some device > drivers disable/enable the specific frequency, the devfreq core > consider them. > > So, devfreq doesn't need to devfreq_verify_within_limits() because > already support some interface to change the minimum/maximum frequency > of devfreq device. > > In case of cpufreq subsystem, cpufreq only provides > 'cpufreq_verify_with_limits()' > to change the minimum/maximum frequency of cpu. some device driver cannot > change the minimum/maximum frequency through OPP interface. > > But, in case of devfreq subsystem, as I explained already, devfreq support > the OPP interface as default way. devfreq subsystem doesn't need to add > other way to change the minimum/maximum frequency. Using the OPP interface exclusively works as long as a enabling/disabling of OPPs is limited to a single driver (drivers/thermal/devfreq_cooling.c). When multiple drivers are involved you need a way to resolve conflicts, that's the purpose of devfreq_verify_within_limits(). Please let me know if there are existing mechanisms for conflict resolution that I overlooked. Possibly drivers/thermal/devfreq_cooling.c could be migrated to use devfreq_verify_within_limits() instead of the OPP interface if desired, however this seems beyond the scope of this series. >>> >>> Actually, if we uses this approach, it doesn't support the multiple drivers >>> too. >>> If non throttler drivers uses devfreq_verify_within_limits(), the conflict >>> happen. >> >> As long as drivers limit the max freq there is no conflict, the lowest >> max freq wins. I expect this to be the usual case, apparently it >> worked for cpufreq for 10+ years. >> >> However it is correct that there would be a conflict if a driver >> requests a min freq that is higher than the max freq requested by >> another. In this case devfreq_verify_within_limits() resolves the >> conflict by raising p->max to the min freq. Not sure if this is >> something that would ever occur in practice though. >> >> If we are really concerned about this case it would also be an option >> to limit the adjustment to the max frequency. >> >>> To resolve the conflict for multiple device driver, maybe OPP interface >>> have to support 'usage_count' such as clk_enable/disable(). >> >> This would require supporting negative usage count values, since a OPP >> should not be enabled if e.g. thermal enables it but the throttler >> disabled it or viceversa. >> >> Theoretically there could also be conflicts, like one driver disabling >> the higher OPPs and another the lower ones, with the outcome of all >> OPPs being disabled, which would be a more drastic conflict resolution >> than that of devfreq_verify_within_limits(). >> >> Viresh, what do you think about an OPP usage count? > > Ping, can we try to reach a conclusion on this or at least keep the > discussion going? > > Not that it matters, but my preferred solution continues to be > devfreq_verify_within_limits(). It solves conflicts in some way (which > could be adjusted if needed) and has proven to work in practice for > 10+ years in a very similar sub-system. It is not true. Current cpufreq subsystem doesn't support external OPP control to enable/disable the OPP entry. If some device driver controls the OPP entry of cpufreq driver with opp_disable/enable(), the operation is not working. Because cpufreq considers the limit through 'cpufreq_verify_with_limits()' only. As I already commented[1], there is different between cpufreq and devfreq. [1] https://lkml.org/lkml/2018/7/4/80 Already, subsystem already used OPP interface in order to control specific OPP entry. I don't want to provide two outside method to control the frequency of devfreq driver. It might make the confusion. I want to use only OPP interface to enable/disable frequency even if we have to modify the OPP interface. -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
On 2018년 08월 01일 04:39, Matthias Kaehlcke wrote: > On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: >> On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: >>> Hi Matthias, >>> >>> On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: Hi Chanwoo, On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > Firstly, > I'm not sure why devfreq needs the devfreq_verify_within_limits() > function. > > devfreq already used the OPP interface as default. It means that > the outside of 'drivers/devfreq' can disable/enable the frequency > such as drivers/thermal/devfreq_cooling.c. Also, when some device > drivers disable/enable the specific frequency, the devfreq core > consider them. > > So, devfreq doesn't need to devfreq_verify_within_limits() because > already support some interface to change the minimum/maximum frequency > of devfreq device. > > In case of cpufreq subsystem, cpufreq only provides > 'cpufreq_verify_with_limits()' > to change the minimum/maximum frequency of cpu. some device driver cannot > change the minimum/maximum frequency through OPP interface. > > But, in case of devfreq subsystem, as I explained already, devfreq support > the OPP interface as default way. devfreq subsystem doesn't need to add > other way to change the minimum/maximum frequency. Using the OPP interface exclusively works as long as a enabling/disabling of OPPs is limited to a single driver (drivers/thermal/devfreq_cooling.c). When multiple drivers are involved you need a way to resolve conflicts, that's the purpose of devfreq_verify_within_limits(). Please let me know if there are existing mechanisms for conflict resolution that I overlooked. Possibly drivers/thermal/devfreq_cooling.c could be migrated to use devfreq_verify_within_limits() instead of the OPP interface if desired, however this seems beyond the scope of this series. >>> >>> Actually, if we uses this approach, it doesn't support the multiple drivers >>> too. >>> If non throttler drivers uses devfreq_verify_within_limits(), the conflict >>> happen. >> >> As long as drivers limit the max freq there is no conflict, the lowest >> max freq wins. I expect this to be the usual case, apparently it >> worked for cpufreq for 10+ years. >> >> However it is correct that there would be a conflict if a driver >> requests a min freq that is higher than the max freq requested by >> another. In this case devfreq_verify_within_limits() resolves the >> conflict by raising p->max to the min freq. Not sure if this is >> something that would ever occur in practice though. >> >> If we are really concerned about this case it would also be an option >> to limit the adjustment to the max frequency. >> >>> To resolve the conflict for multiple device driver, maybe OPP interface >>> have to support 'usage_count' such as clk_enable/disable(). >> >> This would require supporting negative usage count values, since a OPP >> should not be enabled if e.g. thermal enables it but the throttler >> disabled it or viceversa. >> >> Theoretically there could also be conflicts, like one driver disabling >> the higher OPPs and another the lower ones, with the outcome of all >> OPPs being disabled, which would be a more drastic conflict resolution >> than that of devfreq_verify_within_limits(). >> >> Viresh, what do you think about an OPP usage count? > > Ping, can we try to reach a conclusion on this or at least keep the > discussion going? > > Not that it matters, but my preferred solution continues to be > devfreq_verify_within_limits(). It solves conflicts in some way (which > could be adjusted if needed) and has proven to work in practice for > 10+ years in a very similar sub-system. It is not true. Current cpufreq subsystem doesn't support external OPP control to enable/disable the OPP entry. If some device driver controls the OPP entry of cpufreq driver with opp_disable/enable(), the operation is not working. Because cpufreq considers the limit through 'cpufreq_verify_with_limits()' only. As I already commented[1], there is different between cpufreq and devfreq. [1] https://lkml.org/lkml/2018/7/4/80 Already, subsystem already used OPP interface in order to control specific OPP entry. I don't want to provide two outside method to control the frequency of devfreq driver. It might make the confusion. I want to use only OPP interface to enable/disable frequency even if we have to modify the OPP interface. -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > > Hi Matthias, > > > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > > > Hi Chanwoo, > > > > > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > > > > >> Firstly, > > >> I'm not sure why devfreq needs the devfreq_verify_within_limits() > > >> function. > > >> > > >> devfreq already used the OPP interface as default. It means that > > >> the outside of 'drivers/devfreq' can disable/enable the frequency > > >> such as drivers/thermal/devfreq_cooling.c. Also, when some device > > >> drivers disable/enable the specific frequency, the devfreq core > > >> consider them. > > >> > > >> So, devfreq doesn't need to devfreq_verify_within_limits() because > > >> already support some interface to change the minimum/maximum frequency > > >> of devfreq device. > > >> > > >> In case of cpufreq subsystem, cpufreq only provides > > >> 'cpufreq_verify_with_limits()' > > >> to change the minimum/maximum frequency of cpu. some device driver cannot > > >> change the minimum/maximum frequency through OPP interface. > > >> > > >> But, in case of devfreq subsystem, as I explained already, devfreq > > >> support > > >> the OPP interface as default way. devfreq subsystem doesn't need to add > > >> other way to change the minimum/maximum frequency. > > > > > > Using the OPP interface exclusively works as long as a > > > enabling/disabling of OPPs is limited to a single driver > > > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > > > involved you need a way to resolve conflicts, that's the purpose of > > > devfreq_verify_within_limits(). Please let me know if there are > > > existing mechanisms for conflict resolution that I overlooked. > > > > > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > > > devfreq_verify_within_limits() instead of the OPP interface if > > > desired, however this seems beyond the scope of this series. > > > > Actually, if we uses this approach, it doesn't support the multiple drivers > > too. > > If non throttler drivers uses devfreq_verify_within_limits(), the conflict > > happen. > > As long as drivers limit the max freq there is no conflict, the lowest > max freq wins. I expect this to be the usual case, apparently it > worked for cpufreq for 10+ years. > > However it is correct that there would be a conflict if a driver > requests a min freq that is higher than the max freq requested by > another. In this case devfreq_verify_within_limits() resolves the > conflict by raising p->max to the min freq. Not sure if this is > something that would ever occur in practice though. > > If we are really concerned about this case it would also be an option > to limit the adjustment to the max frequency. > > > To resolve the conflict for multiple device driver, maybe OPP interface > > have to support 'usage_count' such as clk_enable/disable(). > > This would require supporting negative usage count values, since a OPP > should not be enabled if e.g. thermal enables it but the throttler > disabled it or viceversa. > > Theoretically there could also be conflicts, like one driver disabling > the higher OPPs and another the lower ones, with the outcome of all > OPPs being disabled, which would be a more drastic conflict resolution > than that of devfreq_verify_within_limits(). > > Viresh, what do you think about an OPP usage count? Ping, can we try to reach a conclusion on this or at least keep the discussion going? Not that it matters, but my preferred solution continues to be devfreq_verify_within_limits(). It solves conflicts in some way (which could be adjusted if needed) and has proven to work in practice for 10+ years in a very similar sub-system.
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
On Mon, Jul 16, 2018 at 10:50:50AM -0700, Matthias Kaehlcke wrote: > On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > > Hi Matthias, > > > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > > > Hi Chanwoo, > > > > > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > > > > >> Firstly, > > >> I'm not sure why devfreq needs the devfreq_verify_within_limits() > > >> function. > > >> > > >> devfreq already used the OPP interface as default. It means that > > >> the outside of 'drivers/devfreq' can disable/enable the frequency > > >> such as drivers/thermal/devfreq_cooling.c. Also, when some device > > >> drivers disable/enable the specific frequency, the devfreq core > > >> consider them. > > >> > > >> So, devfreq doesn't need to devfreq_verify_within_limits() because > > >> already support some interface to change the minimum/maximum frequency > > >> of devfreq device. > > >> > > >> In case of cpufreq subsystem, cpufreq only provides > > >> 'cpufreq_verify_with_limits()' > > >> to change the minimum/maximum frequency of cpu. some device driver cannot > > >> change the minimum/maximum frequency through OPP interface. > > >> > > >> But, in case of devfreq subsystem, as I explained already, devfreq > > >> support > > >> the OPP interface as default way. devfreq subsystem doesn't need to add > > >> other way to change the minimum/maximum frequency. > > > > > > Using the OPP interface exclusively works as long as a > > > enabling/disabling of OPPs is limited to a single driver > > > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > > > involved you need a way to resolve conflicts, that's the purpose of > > > devfreq_verify_within_limits(). Please let me know if there are > > > existing mechanisms for conflict resolution that I overlooked. > > > > > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > > > devfreq_verify_within_limits() instead of the OPP interface if > > > desired, however this seems beyond the scope of this series. > > > > Actually, if we uses this approach, it doesn't support the multiple drivers > > too. > > If non throttler drivers uses devfreq_verify_within_limits(), the conflict > > happen. > > As long as drivers limit the max freq there is no conflict, the lowest > max freq wins. I expect this to be the usual case, apparently it > worked for cpufreq for 10+ years. > > However it is correct that there would be a conflict if a driver > requests a min freq that is higher than the max freq requested by > another. In this case devfreq_verify_within_limits() resolves the > conflict by raising p->max to the min freq. Not sure if this is > something that would ever occur in practice though. > > If we are really concerned about this case it would also be an option > to limit the adjustment to the max frequency. > > > To resolve the conflict for multiple device driver, maybe OPP interface > > have to support 'usage_count' such as clk_enable/disable(). > > This would require supporting negative usage count values, since a OPP > should not be enabled if e.g. thermal enables it but the throttler > disabled it or viceversa. > > Theoretically there could also be conflicts, like one driver disabling > the higher OPPs and another the lower ones, with the outcome of all > OPPs being disabled, which would be a more drastic conflict resolution > than that of devfreq_verify_within_limits(). > > Viresh, what do you think about an OPP usage count? Ping, can we try to reach a conclusion on this or at least keep the discussion going? Not that it matters, but my preferred solution continues to be devfreq_verify_within_limits(). It solves conflicts in some way (which could be adjusted if needed) and has proven to work in practice for 10+ years in a very similar sub-system.
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > > >> Firstly, > >> I'm not sure why devfreq needs the devfreq_verify_within_limits() function. > >> > >> devfreq already used the OPP interface as default. It means that > >> the outside of 'drivers/devfreq' can disable/enable the frequency > >> such as drivers/thermal/devfreq_cooling.c. Also, when some device > >> drivers disable/enable the specific frequency, the devfreq core > >> consider them. > >> > >> So, devfreq doesn't need to devfreq_verify_within_limits() because > >> already support some interface to change the minimum/maximum frequency > >> of devfreq device. > >> > >> In case of cpufreq subsystem, cpufreq only provides > >> 'cpufreq_verify_with_limits()' > >> to change the minimum/maximum frequency of cpu. some device driver cannot > >> change the minimum/maximum frequency through OPP interface. > >> > >> But, in case of devfreq subsystem, as I explained already, devfreq support > >> the OPP interface as default way. devfreq subsystem doesn't need to add > >> other way to change the minimum/maximum frequency. > > > > Using the OPP interface exclusively works as long as a > > enabling/disabling of OPPs is limited to a single driver > > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > > involved you need a way to resolve conflicts, that's the purpose of > > devfreq_verify_within_limits(). Please let me know if there are > > existing mechanisms for conflict resolution that I overlooked. > > > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > > devfreq_verify_within_limits() instead of the OPP interface if > > desired, however this seems beyond the scope of this series. > > Actually, if we uses this approach, it doesn't support the multiple drivers > too. > If non throttler drivers uses devfreq_verify_within_limits(), the conflict > happen. As long as drivers limit the max freq there is no conflict, the lowest max freq wins. I expect this to be the usual case, apparently it worked for cpufreq for 10+ years. However it is correct that there would be a conflict if a driver requests a min freq that is higher than the max freq requested by another. In this case devfreq_verify_within_limits() resolves the conflict by raising p->max to the min freq. Not sure if this is something that would ever occur in practice though. If we are really concerned about this case it would also be an option to limit the adjustment to the max frequency. > To resolve the conflict for multiple device driver, maybe OPP interface > have to support 'usage_count' such as clk_enable/disable(). This would require supporting negative usage count values, since a OPP should not be enabled if e.g. thermal enables it but the throttler disabled it or viceversa. Theoretically there could also be conflicts, like one driver disabling the higher OPPs and another the lower ones, with the outcome of all OPPs being disabled, which would be a more drastic conflict resolution than that of devfreq_verify_within_limits(). Viresh, what do you think about an OPP usage count? Thanks Matthias
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
On Thu, Jul 12, 2018 at 05:44:33PM +0900, Chanwoo Choi wrote: > Hi Matthias, > > On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > > Hi Chanwoo, > > > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > > > >> Firstly, > >> I'm not sure why devfreq needs the devfreq_verify_within_limits() function. > >> > >> devfreq already used the OPP interface as default. It means that > >> the outside of 'drivers/devfreq' can disable/enable the frequency > >> such as drivers/thermal/devfreq_cooling.c. Also, when some device > >> drivers disable/enable the specific frequency, the devfreq core > >> consider them. > >> > >> So, devfreq doesn't need to devfreq_verify_within_limits() because > >> already support some interface to change the minimum/maximum frequency > >> of devfreq device. > >> > >> In case of cpufreq subsystem, cpufreq only provides > >> 'cpufreq_verify_with_limits()' > >> to change the minimum/maximum frequency of cpu. some device driver cannot > >> change the minimum/maximum frequency through OPP interface. > >> > >> But, in case of devfreq subsystem, as I explained already, devfreq support > >> the OPP interface as default way. devfreq subsystem doesn't need to add > >> other way to change the minimum/maximum frequency. > > > > Using the OPP interface exclusively works as long as a > > enabling/disabling of OPPs is limited to a single driver > > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > > involved you need a way to resolve conflicts, that's the purpose of > > devfreq_verify_within_limits(). Please let me know if there are > > existing mechanisms for conflict resolution that I overlooked. > > > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > > devfreq_verify_within_limits() instead of the OPP interface if > > desired, however this seems beyond the scope of this series. > > Actually, if we uses this approach, it doesn't support the multiple drivers > too. > If non throttler drivers uses devfreq_verify_within_limits(), the conflict > happen. As long as drivers limit the max freq there is no conflict, the lowest max freq wins. I expect this to be the usual case, apparently it worked for cpufreq for 10+ years. However it is correct that there would be a conflict if a driver requests a min freq that is higher than the max freq requested by another. In this case devfreq_verify_within_limits() resolves the conflict by raising p->max to the min freq. Not sure if this is something that would ever occur in practice though. If we are really concerned about this case it would also be an option to limit the adjustment to the max frequency. > To resolve the conflict for multiple device driver, maybe OPP interface > have to support 'usage_count' such as clk_enable/disable(). This would require supporting negative usage count values, since a OPP should not be enabled if e.g. thermal enables it but the throttler disabled it or viceversa. Theoretically there could also be conflicts, like one driver disabling the higher OPPs and another the lower ones, with the outcome of all OPPs being disabled, which would be a more drastic conflict resolution than that of devfreq_verify_within_limits(). Viresh, what do you think about an OPP usage count? Thanks Matthias
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > >> Firstly, >> I'm not sure why devfreq needs the devfreq_verify_within_limits() function. >> >> devfreq already used the OPP interface as default. It means that >> the outside of 'drivers/devfreq' can disable/enable the frequency >> such as drivers/thermal/devfreq_cooling.c. Also, when some device >> drivers disable/enable the specific frequency, the devfreq core >> consider them. >> >> So, devfreq doesn't need to devfreq_verify_within_limits() because >> already support some interface to change the minimum/maximum frequency >> of devfreq device. >> >> In case of cpufreq subsystem, cpufreq only provides >> 'cpufreq_verify_with_limits()' >> to change the minimum/maximum frequency of cpu. some device driver cannot >> change the minimum/maximum frequency through OPP interface. >> >> But, in case of devfreq subsystem, as I explained already, devfreq support >> the OPP interface as default way. devfreq subsystem doesn't need to add >> other way to change the minimum/maximum frequency. > > Using the OPP interface exclusively works as long as a > enabling/disabling of OPPs is limited to a single driver > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > involved you need a way to resolve conflicts, that's the purpose of > devfreq_verify_within_limits(). Please let me know if there are > existing mechanisms for conflict resolution that I overlooked. > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > devfreq_verify_within_limits() instead of the OPP interface if > desired, however this seems beyond the scope of this series. Actually, if we uses this approach, it doesn't support the multiple drivers too. If non throttler drivers uses devfreq_verify_within_limits(), the conflict happen. To resolve the conflict for multiple device driver, maybe OPP interface have to support 'usage_count' such as clk_enable/disable(). > >> Secondly, >> This patch send the 'struct devfreq_policy' instance as the data >> when sending the notification as following: >> >> srcu_notifier_call_chain(>policy_notifier_list, >> DEVFREQ_ADJUST, policy); >> >> But, I think that if devfreq core sends the 'struct devfreq_freq_limits' >> instance instead of 'struct devfreq_policy', it is enough. >> Because receiver of DEVFREQ_ADJUST just will use the min_freq/max_freq >> variables. >> >> So, I tried to find the cpufreq's case. The some device drivers using >> CPUFREQ_POLICY_NOTIFIER uses following variables of 'struct cpufreq_policy'. >> It means that receiver of CPUFREQ_POLICY_NOTIFIER don't need to other >> information/variables except for min/max frequency. >> >> - policy->min >> - policy->max >> - policy->cpuinfo.max_freq >> - policy->cpuinfo.min_freq >> - policy->cpu : not related to devfreq) >> - policy->related_cpus : not related to devfreq) >> >> - list of device drivers using CPUFREQ_POLICY_NOTIFIER (linux kernel is >> v4.18-rc1) >> $ grep -rn "CPUFREQ_POLICY_NOTIFIER" . >> ./drivers/macintosh/windfarm_cpufreq_clamp.c >> ./drivers/thermal/cpu_cooling.c >> ./drivers/thermal/cpu_cooling.c >> ./drivers/acpi/processor_thermal.c >> ./drivers/acpi/processor_thermal.c >> ./drivers/acpi/processor_perflib.c >> ./drivers/acpi/processor_perflib.c >> ./drivers/base/arch_topology.c >> ./drivers/base/arch_topology.c >> ./drivers/video/fbdev/sa1100fb.c >> ./drivers/video/fbdev/pxafb.c >> ./drivers/cpufreq/ppc_cbe_cpufreq_pmi.c >> ./drivers/cpufreq/cpufreq.c >> ./drivers/cpufreq/cpufreq.c >> ./drivers/cpufreq/cpufreq.c >> ./drivers/cpufreq/cpufreq.c > > Thanks for your investigation. > > I decided to mirror the cpufreq interface for consistency, but I agree > that 'struct devfreq_freq_limits' could be passed instead of the > policy object. I'm fine with changing that. > >> On 2018년 07월 04일 08:46, Matthias Kaehlcke wrote: >>> Policy notifiers are called before a frequency change and may narrow >>> the min/max frequency range in devfreq_policy, which is used to adjust >>> the target frequency if it is beyond this range. >>> >>> Also add a few helpers: >>> - devfreq_verify_within_[dev_]limits() >>> - should be used by the notifiers for policy adjustments. >>> - dev_to_devfreq() >>> - lookup a devfreq strict from a device pointer >>> >>> Signed-off-by: Matthias Kaehlcke >>> Reviewed-by: Brian Norris >>> --- >>> Changes in v5: >>> - none >>> >>> Changes in v4: >>> - Fixed typo in commit message: devfreg => devfreq >>> - added 'Reviewed-by: Brian Norris ' tag >>> >>> Changes in v3: >>> - devfreq.h: fixed misspelling of struct devfreq_policy >>> >>> Changes in v2: >>> - performance, powersave and simpleondemand governors don't need changes >>> with "PM / devfreq: Don't adjust to user limits in governors" >>> - formatting fixes >>> --- >>> drivers/devfreq/devfreq.c | 48 ++--- >>> include/linux/devfreq.h
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias, On 2018년 07월 07일 02:53, Matthias Kaehlcke wrote: > Hi Chanwoo, > > On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > >> Firstly, >> I'm not sure why devfreq needs the devfreq_verify_within_limits() function. >> >> devfreq already used the OPP interface as default. It means that >> the outside of 'drivers/devfreq' can disable/enable the frequency >> such as drivers/thermal/devfreq_cooling.c. Also, when some device >> drivers disable/enable the specific frequency, the devfreq core >> consider them. >> >> So, devfreq doesn't need to devfreq_verify_within_limits() because >> already support some interface to change the minimum/maximum frequency >> of devfreq device. >> >> In case of cpufreq subsystem, cpufreq only provides >> 'cpufreq_verify_with_limits()' >> to change the minimum/maximum frequency of cpu. some device driver cannot >> change the minimum/maximum frequency through OPP interface. >> >> But, in case of devfreq subsystem, as I explained already, devfreq support >> the OPP interface as default way. devfreq subsystem doesn't need to add >> other way to change the minimum/maximum frequency. > > Using the OPP interface exclusively works as long as a > enabling/disabling of OPPs is limited to a single driver > (drivers/thermal/devfreq_cooling.c). When multiple drivers are > involved you need a way to resolve conflicts, that's the purpose of > devfreq_verify_within_limits(). Please let me know if there are > existing mechanisms for conflict resolution that I overlooked. > > Possibly drivers/thermal/devfreq_cooling.c could be migrated to use > devfreq_verify_within_limits() instead of the OPP interface if > desired, however this seems beyond the scope of this series. Actually, if we uses this approach, it doesn't support the multiple drivers too. If non throttler drivers uses devfreq_verify_within_limits(), the conflict happen. To resolve the conflict for multiple device driver, maybe OPP interface have to support 'usage_count' such as clk_enable/disable(). > >> Secondly, >> This patch send the 'struct devfreq_policy' instance as the data >> when sending the notification as following: >> >> srcu_notifier_call_chain(>policy_notifier_list, >> DEVFREQ_ADJUST, policy); >> >> But, I think that if devfreq core sends the 'struct devfreq_freq_limits' >> instance instead of 'struct devfreq_policy', it is enough. >> Because receiver of DEVFREQ_ADJUST just will use the min_freq/max_freq >> variables. >> >> So, I tried to find the cpufreq's case. The some device drivers using >> CPUFREQ_POLICY_NOTIFIER uses following variables of 'struct cpufreq_policy'. >> It means that receiver of CPUFREQ_POLICY_NOTIFIER don't need to other >> information/variables except for min/max frequency. >> >> - policy->min >> - policy->max >> - policy->cpuinfo.max_freq >> - policy->cpuinfo.min_freq >> - policy->cpu : not related to devfreq) >> - policy->related_cpus : not related to devfreq) >> >> - list of device drivers using CPUFREQ_POLICY_NOTIFIER (linux kernel is >> v4.18-rc1) >> $ grep -rn "CPUFREQ_POLICY_NOTIFIER" . >> ./drivers/macintosh/windfarm_cpufreq_clamp.c >> ./drivers/thermal/cpu_cooling.c >> ./drivers/thermal/cpu_cooling.c >> ./drivers/acpi/processor_thermal.c >> ./drivers/acpi/processor_thermal.c >> ./drivers/acpi/processor_perflib.c >> ./drivers/acpi/processor_perflib.c >> ./drivers/base/arch_topology.c >> ./drivers/base/arch_topology.c >> ./drivers/video/fbdev/sa1100fb.c >> ./drivers/video/fbdev/pxafb.c >> ./drivers/cpufreq/ppc_cbe_cpufreq_pmi.c >> ./drivers/cpufreq/cpufreq.c >> ./drivers/cpufreq/cpufreq.c >> ./drivers/cpufreq/cpufreq.c >> ./drivers/cpufreq/cpufreq.c > > Thanks for your investigation. > > I decided to mirror the cpufreq interface for consistency, but I agree > that 'struct devfreq_freq_limits' could be passed instead of the > policy object. I'm fine with changing that. > >> On 2018년 07월 04일 08:46, Matthias Kaehlcke wrote: >>> Policy notifiers are called before a frequency change and may narrow >>> the min/max frequency range in devfreq_policy, which is used to adjust >>> the target frequency if it is beyond this range. >>> >>> Also add a few helpers: >>> - devfreq_verify_within_[dev_]limits() >>> - should be used by the notifiers for policy adjustments. >>> - dev_to_devfreq() >>> - lookup a devfreq strict from a device pointer >>> >>> Signed-off-by: Matthias Kaehlcke >>> Reviewed-by: Brian Norris >>> --- >>> Changes in v5: >>> - none >>> >>> Changes in v4: >>> - Fixed typo in commit message: devfreg => devfreq >>> - added 'Reviewed-by: Brian Norris ' tag >>> >>> Changes in v3: >>> - devfreq.h: fixed misspelling of struct devfreq_policy >>> >>> Changes in v2: >>> - performance, powersave and simpleondemand governors don't need changes >>> with "PM / devfreq: Don't adjust to user limits in governors" >>> - formatting fixes >>> --- >>> drivers/devfreq/devfreq.c | 48 ++--- >>> include/linux/devfreq.h
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > Firstly, > I'm not sure why devfreq needs the devfreq_verify_within_limits() function. > > devfreq already used the OPP interface as default. It means that > the outside of 'drivers/devfreq' can disable/enable the frequency > such as drivers/thermal/devfreq_cooling.c. Also, when some device > drivers disable/enable the specific frequency, the devfreq core > consider them. > > So, devfreq doesn't need to devfreq_verify_within_limits() because > already support some interface to change the minimum/maximum frequency > of devfreq device. > > In case of cpufreq subsystem, cpufreq only provides > 'cpufreq_verify_with_limits()' > to change the minimum/maximum frequency of cpu. some device driver cannot > change the minimum/maximum frequency through OPP interface. > > But, in case of devfreq subsystem, as I explained already, devfreq support > the OPP interface as default way. devfreq subsystem doesn't need to add > other way to change the minimum/maximum frequency. Using the OPP interface exclusively works as long as a enabling/disabling of OPPs is limited to a single driver (drivers/thermal/devfreq_cooling.c). When multiple drivers are involved you need a way to resolve conflicts, that's the purpose of devfreq_verify_within_limits(). Please let me know if there are existing mechanisms for conflict resolution that I overlooked. Possibly drivers/thermal/devfreq_cooling.c could be migrated to use devfreq_verify_within_limits() instead of the OPP interface if desired, however this seems beyond the scope of this series. > Secondly, > This patch send the 'struct devfreq_policy' instance as the data > when sending the notification as following: > > srcu_notifier_call_chain(>policy_notifier_list, > DEVFREQ_ADJUST, policy); > > But, I think that if devfreq core sends the 'struct devfreq_freq_limits' > instance instead of 'struct devfreq_policy', it is enough. > Because receiver of DEVFREQ_ADJUST just will use the min_freq/max_freq > variables. > > So, I tried to find the cpufreq's case. The some device drivers using > CPUFREQ_POLICY_NOTIFIER uses following variables of 'struct cpufreq_policy'. > It means that receiver of CPUFREQ_POLICY_NOTIFIER don't need to other > information/variables except for min/max frequency. > > - policy->min > - policy->max > - policy->cpuinfo.max_freq > - policy->cpuinfo.min_freq > - policy->cpu : not related to devfreq) > - policy->related_cpus : not related to devfreq) > > - list of device drivers using CPUFREQ_POLICY_NOTIFIER (linux kernel is > v4.18-rc1) > $ grep -rn "CPUFREQ_POLICY_NOTIFIER" . > ./drivers/macintosh/windfarm_cpufreq_clamp.c > ./drivers/thermal/cpu_cooling.c > ./drivers/thermal/cpu_cooling.c > ./drivers/acpi/processor_thermal.c > ./drivers/acpi/processor_thermal.c > ./drivers/acpi/processor_perflib.c > ./drivers/acpi/processor_perflib.c > ./drivers/base/arch_topology.c > ./drivers/base/arch_topology.c > ./drivers/video/fbdev/sa1100fb.c > ./drivers/video/fbdev/pxafb.c > ./drivers/cpufreq/ppc_cbe_cpufreq_pmi.c > ./drivers/cpufreq/cpufreq.c > ./drivers/cpufreq/cpufreq.c > ./drivers/cpufreq/cpufreq.c > ./drivers/cpufreq/cpufreq.c Thanks for your investigation. I decided to mirror the cpufreq interface for consistency, but I agree that 'struct devfreq_freq_limits' could be passed instead of the policy object. I'm fine with changing that. > On 2018년 07월 04일 08:46, Matthias Kaehlcke wrote: > > Policy notifiers are called before a frequency change and may narrow > > the min/max frequency range in devfreq_policy, which is used to adjust > > the target frequency if it is beyond this range. > > > > Also add a few helpers: > > - devfreq_verify_within_[dev_]limits() > > - should be used by the notifiers for policy adjustments. > > - dev_to_devfreq() > > - lookup a devfreq strict from a device pointer > > > > Signed-off-by: Matthias Kaehlcke > > Reviewed-by: Brian Norris > > --- > > Changes in v5: > > - none > > > > Changes in v4: > > - Fixed typo in commit message: devfreg => devfreq > > - added 'Reviewed-by: Brian Norris ' tag > > > > Changes in v3: > > - devfreq.h: fixed misspelling of struct devfreq_policy > > > > Changes in v2: > > - performance, powersave and simpleondemand governors don't need changes > > with "PM / devfreq: Don't adjust to user limits in governors" > > - formatting fixes > > --- > > drivers/devfreq/devfreq.c | 48 ++--- > > include/linux/devfreq.h | 65 +++ > > 2 files changed, 102 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > > index 21604d6ae2b8..4cbaa7ad1972 100644 > > --- a/drivers/devfreq/devfreq.c > > +++ b/drivers/devfreq/devfreq.c > > @@ -72,6 +72,21 @@ static struct devfreq *find_device_devfreq(struct device > > *dev) > > return ERR_PTR(-ENODEV); > > } > > > > +/** > > + *
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Chanwoo, On Wed, Jul 04, 2018 at 03:41:46PM +0900, Chanwoo Choi wrote: > Firstly, > I'm not sure why devfreq needs the devfreq_verify_within_limits() function. > > devfreq already used the OPP interface as default. It means that > the outside of 'drivers/devfreq' can disable/enable the frequency > such as drivers/thermal/devfreq_cooling.c. Also, when some device > drivers disable/enable the specific frequency, the devfreq core > consider them. > > So, devfreq doesn't need to devfreq_verify_within_limits() because > already support some interface to change the minimum/maximum frequency > of devfreq device. > > In case of cpufreq subsystem, cpufreq only provides > 'cpufreq_verify_with_limits()' > to change the minimum/maximum frequency of cpu. some device driver cannot > change the minimum/maximum frequency through OPP interface. > > But, in case of devfreq subsystem, as I explained already, devfreq support > the OPP interface as default way. devfreq subsystem doesn't need to add > other way to change the minimum/maximum frequency. Using the OPP interface exclusively works as long as a enabling/disabling of OPPs is limited to a single driver (drivers/thermal/devfreq_cooling.c). When multiple drivers are involved you need a way to resolve conflicts, that's the purpose of devfreq_verify_within_limits(). Please let me know if there are existing mechanisms for conflict resolution that I overlooked. Possibly drivers/thermal/devfreq_cooling.c could be migrated to use devfreq_verify_within_limits() instead of the OPP interface if desired, however this seems beyond the scope of this series. > Secondly, > This patch send the 'struct devfreq_policy' instance as the data > when sending the notification as following: > > srcu_notifier_call_chain(>policy_notifier_list, > DEVFREQ_ADJUST, policy); > > But, I think that if devfreq core sends the 'struct devfreq_freq_limits' > instance instead of 'struct devfreq_policy', it is enough. > Because receiver of DEVFREQ_ADJUST just will use the min_freq/max_freq > variables. > > So, I tried to find the cpufreq's case. The some device drivers using > CPUFREQ_POLICY_NOTIFIER uses following variables of 'struct cpufreq_policy'. > It means that receiver of CPUFREQ_POLICY_NOTIFIER don't need to other > information/variables except for min/max frequency. > > - policy->min > - policy->max > - policy->cpuinfo.max_freq > - policy->cpuinfo.min_freq > - policy->cpu : not related to devfreq) > - policy->related_cpus : not related to devfreq) > > - list of device drivers using CPUFREQ_POLICY_NOTIFIER (linux kernel is > v4.18-rc1) > $ grep -rn "CPUFREQ_POLICY_NOTIFIER" . > ./drivers/macintosh/windfarm_cpufreq_clamp.c > ./drivers/thermal/cpu_cooling.c > ./drivers/thermal/cpu_cooling.c > ./drivers/acpi/processor_thermal.c > ./drivers/acpi/processor_thermal.c > ./drivers/acpi/processor_perflib.c > ./drivers/acpi/processor_perflib.c > ./drivers/base/arch_topology.c > ./drivers/base/arch_topology.c > ./drivers/video/fbdev/sa1100fb.c > ./drivers/video/fbdev/pxafb.c > ./drivers/cpufreq/ppc_cbe_cpufreq_pmi.c > ./drivers/cpufreq/cpufreq.c > ./drivers/cpufreq/cpufreq.c > ./drivers/cpufreq/cpufreq.c > ./drivers/cpufreq/cpufreq.c Thanks for your investigation. I decided to mirror the cpufreq interface for consistency, but I agree that 'struct devfreq_freq_limits' could be passed instead of the policy object. I'm fine with changing that. > On 2018년 07월 04일 08:46, Matthias Kaehlcke wrote: > > Policy notifiers are called before a frequency change and may narrow > > the min/max frequency range in devfreq_policy, which is used to adjust > > the target frequency if it is beyond this range. > > > > Also add a few helpers: > > - devfreq_verify_within_[dev_]limits() > > - should be used by the notifiers for policy adjustments. > > - dev_to_devfreq() > > - lookup a devfreq strict from a device pointer > > > > Signed-off-by: Matthias Kaehlcke > > Reviewed-by: Brian Norris > > --- > > Changes in v5: > > - none > > > > Changes in v4: > > - Fixed typo in commit message: devfreg => devfreq > > - added 'Reviewed-by: Brian Norris ' tag > > > > Changes in v3: > > - devfreq.h: fixed misspelling of struct devfreq_policy > > > > Changes in v2: > > - performance, powersave and simpleondemand governors don't need changes > > with "PM / devfreq: Don't adjust to user limits in governors" > > - formatting fixes > > --- > > drivers/devfreq/devfreq.c | 48 ++--- > > include/linux/devfreq.h | 65 +++ > > 2 files changed, 102 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > > index 21604d6ae2b8..4cbaa7ad1972 100644 > > --- a/drivers/devfreq/devfreq.c > > +++ b/drivers/devfreq/devfreq.c > > @@ -72,6 +72,21 @@ static struct devfreq *find_device_devfreq(struct device > > *dev) > > return ERR_PTR(-ENODEV); > > } > > > > +/** > > + *
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias,
Firstly,
I'm not sure why devfreq needs the devfreq_verify_within_limits() function.
devfreq already used the OPP interface as default. It means that
the outside of 'drivers/devfreq' can disable/enable the frequency
such as drivers/thermal/devfreq_cooling.c. Also, when some device
drivers disable/enable the specific frequency, the devfreq core
consider them.
So, devfreq doesn't need to devfreq_verify_within_limits() because
already support some interface to change the minimum/maximum frequency
of devfreq device.
In case of cpufreq subsystem, cpufreq only provides
'cpufreq_verify_with_limits()'
to change the minimum/maximum frequency of cpu. some device driver cannot
change the minimum/maximum frequency through OPP interface.
But, in case of devfreq subsystem, as I explained already, devfreq support
the OPP interface as default way. devfreq subsystem doesn't need to add
other way to change the minimum/maximum frequency.
Secondly,
This patch send the 'struct devfreq_policy' instance as the data
when sending the notification as following:
srcu_notifier_call_chain(>policy_notifier_list,
DEVFREQ_ADJUST, policy);
But, I think that if devfreq core sends the 'struct devfreq_freq_limits'
instance instead of 'struct devfreq_policy', it is enough.
Because receiver of DEVFREQ_ADJUST just will use the min_freq/max_freq
variables.
So, I tried to find the cpufreq's case. The some device drivers using
CPUFREQ_POLICY_NOTIFIER uses following variables of 'struct cpufreq_policy'.
It means that receiver of CPUFREQ_POLICY_NOTIFIER don't need to other
information/variables except for min/max frequency.
- policy->min
- policy->max
- policy->cpuinfo.max_freq
- policy->cpuinfo.min_freq
- policy->cpu : not related to devfreq)
- policy->related_cpus : not related to devfreq)
- list of device drivers using CPUFREQ_POLICY_NOTIFIER (linux kernel is
v4.18-rc1)
$ grep -rn "CPUFREQ_POLICY_NOTIFIER" .
./drivers/macintosh/windfarm_cpufreq_clamp.c
./drivers/thermal/cpu_cooling.c
./drivers/thermal/cpu_cooling.c
./drivers/acpi/processor_thermal.c
./drivers/acpi/processor_thermal.c
./drivers/acpi/processor_perflib.c
./drivers/acpi/processor_perflib.c
./drivers/base/arch_topology.c
./drivers/base/arch_topology.c
./drivers/video/fbdev/sa1100fb.c
./drivers/video/fbdev/pxafb.c
./drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
./drivers/cpufreq/cpufreq.c
./drivers/cpufreq/cpufreq.c
./drivers/cpufreq/cpufreq.c
./drivers/cpufreq/cpufreq.c
On 2018년 07월 04일 08:46, Matthias Kaehlcke wrote:
> Policy notifiers are called before a frequency change and may narrow
> the min/max frequency range in devfreq_policy, which is used to adjust
> the target frequency if it is beyond this range.
>
> Also add a few helpers:
> - devfreq_verify_within_[dev_]limits()
> - should be used by the notifiers for policy adjustments.
> - dev_to_devfreq()
> - lookup a devfreq strict from a device pointer
>
> Signed-off-by: Matthias Kaehlcke
> Reviewed-by: Brian Norris
> ---
> Changes in v5:
> - none
>
> Changes in v4:
> - Fixed typo in commit message: devfreg => devfreq
> - added 'Reviewed-by: Brian Norris ' tag
>
> Changes in v3:
> - devfreq.h: fixed misspelling of struct devfreq_policy
>
> Changes in v2:
> - performance, powersave and simpleondemand governors don't need changes
> with "PM / devfreq: Don't adjust to user limits in governors"
> - formatting fixes
> ---
> drivers/devfreq/devfreq.c | 48 ++---
> include/linux/devfreq.h | 65 +++
> 2 files changed, 102 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 21604d6ae2b8..4cbaa7ad1972 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -72,6 +72,21 @@ static struct devfreq *find_device_devfreq(struct device
> *dev)
> return ERR_PTR(-ENODEV);
> }
>
> +/**
> + * dev_to_devfreq() - find devfreq struct using device pointer
> + * @dev: device pointer used to lookup device devfreq.
> + */
> +struct devfreq *dev_to_devfreq(struct device *dev)
> +{
> + struct devfreq *devfreq;
> +
> + mutex_lock(_list_lock);
> + devfreq = find_device_devfreq(dev);
> + mutex_unlock(_list_lock);
> +
> + return devfreq;
> +}
> +
> static unsigned long find_available_min_freq(struct devfreq *devfreq)
> {
> struct dev_pm_opp *opp;
> @@ -269,20 +284,21 @@ int update_devfreq(struct devfreq *devfreq)
> if (!policy->governor)
> return -EINVAL;
>
> + policy->min = policy->devinfo.min_freq;
> + policy->max = policy->devinfo.max_freq;
Why don't you consider 'policy->user.max/min_freq' as following?
As I already commented, I think that 'struct devfreq_freq_limits' is enough
instead of 'struct devfreq_policy'.
->max_freq = MIN(policy->devinfo.max_freq, policy->user.max_freq);
->min_freq = MAX(policy->devinfo.min_freq, policy->user.min_freq);
> +
> +
Re: [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Hi Matthias,
Firstly,
I'm not sure why devfreq needs the devfreq_verify_within_limits() function.
devfreq already used the OPP interface as default. It means that
the outside of 'drivers/devfreq' can disable/enable the frequency
such as drivers/thermal/devfreq_cooling.c. Also, when some device
drivers disable/enable the specific frequency, the devfreq core
consider them.
So, devfreq doesn't need to devfreq_verify_within_limits() because
already support some interface to change the minimum/maximum frequency
of devfreq device.
In case of cpufreq subsystem, cpufreq only provides
'cpufreq_verify_with_limits()'
to change the minimum/maximum frequency of cpu. some device driver cannot
change the minimum/maximum frequency through OPP interface.
But, in case of devfreq subsystem, as I explained already, devfreq support
the OPP interface as default way. devfreq subsystem doesn't need to add
other way to change the minimum/maximum frequency.
Secondly,
This patch send the 'struct devfreq_policy' instance as the data
when sending the notification as following:
srcu_notifier_call_chain(>policy_notifier_list,
DEVFREQ_ADJUST, policy);
But, I think that if devfreq core sends the 'struct devfreq_freq_limits'
instance instead of 'struct devfreq_policy', it is enough.
Because receiver of DEVFREQ_ADJUST just will use the min_freq/max_freq
variables.
So, I tried to find the cpufreq's case. The some device drivers using
CPUFREQ_POLICY_NOTIFIER uses following variables of 'struct cpufreq_policy'.
It means that receiver of CPUFREQ_POLICY_NOTIFIER don't need to other
information/variables except for min/max frequency.
- policy->min
- policy->max
- policy->cpuinfo.max_freq
- policy->cpuinfo.min_freq
- policy->cpu : not related to devfreq)
- policy->related_cpus : not related to devfreq)
- list of device drivers using CPUFREQ_POLICY_NOTIFIER (linux kernel is
v4.18-rc1)
$ grep -rn "CPUFREQ_POLICY_NOTIFIER" .
./drivers/macintosh/windfarm_cpufreq_clamp.c
./drivers/thermal/cpu_cooling.c
./drivers/thermal/cpu_cooling.c
./drivers/acpi/processor_thermal.c
./drivers/acpi/processor_thermal.c
./drivers/acpi/processor_perflib.c
./drivers/acpi/processor_perflib.c
./drivers/base/arch_topology.c
./drivers/base/arch_topology.c
./drivers/video/fbdev/sa1100fb.c
./drivers/video/fbdev/pxafb.c
./drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
./drivers/cpufreq/cpufreq.c
./drivers/cpufreq/cpufreq.c
./drivers/cpufreq/cpufreq.c
./drivers/cpufreq/cpufreq.c
On 2018년 07월 04일 08:46, Matthias Kaehlcke wrote:
> Policy notifiers are called before a frequency change and may narrow
> the min/max frequency range in devfreq_policy, which is used to adjust
> the target frequency if it is beyond this range.
>
> Also add a few helpers:
> - devfreq_verify_within_[dev_]limits()
> - should be used by the notifiers for policy adjustments.
> - dev_to_devfreq()
> - lookup a devfreq strict from a device pointer
>
> Signed-off-by: Matthias Kaehlcke
> Reviewed-by: Brian Norris
> ---
> Changes in v5:
> - none
>
> Changes in v4:
> - Fixed typo in commit message: devfreg => devfreq
> - added 'Reviewed-by: Brian Norris ' tag
>
> Changes in v3:
> - devfreq.h: fixed misspelling of struct devfreq_policy
>
> Changes in v2:
> - performance, powersave and simpleondemand governors don't need changes
> with "PM / devfreq: Don't adjust to user limits in governors"
> - formatting fixes
> ---
> drivers/devfreq/devfreq.c | 48 ++---
> include/linux/devfreq.h | 65 +++
> 2 files changed, 102 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 21604d6ae2b8..4cbaa7ad1972 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -72,6 +72,21 @@ static struct devfreq *find_device_devfreq(struct device
> *dev)
> return ERR_PTR(-ENODEV);
> }
>
> +/**
> + * dev_to_devfreq() - find devfreq struct using device pointer
> + * @dev: device pointer used to lookup device devfreq.
> + */
> +struct devfreq *dev_to_devfreq(struct device *dev)
> +{
> + struct devfreq *devfreq;
> +
> + mutex_lock(_list_lock);
> + devfreq = find_device_devfreq(dev);
> + mutex_unlock(_list_lock);
> +
> + return devfreq;
> +}
> +
> static unsigned long find_available_min_freq(struct devfreq *devfreq)
> {
> struct dev_pm_opp *opp;
> @@ -269,20 +284,21 @@ int update_devfreq(struct devfreq *devfreq)
> if (!policy->governor)
> return -EINVAL;
>
> + policy->min = policy->devinfo.min_freq;
> + policy->max = policy->devinfo.max_freq;
Why don't you consider 'policy->user.max/min_freq' as following?
As I already commented, I think that 'struct devfreq_freq_limits' is enough
instead of 'struct devfreq_policy'.
->max_freq = MIN(policy->devinfo.max_freq, policy->user.max_freq);
->min_freq = MAX(policy->devinfo.min_freq, policy->user.min_freq);
> +
> +
[PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Policy notifiers are called before a frequency change and may narrow
the min/max frequency range in devfreq_policy, which is used to adjust
the target frequency if it is beyond this range.
Also add a few helpers:
- devfreq_verify_within_[dev_]limits()
- should be used by the notifiers for policy adjustments.
- dev_to_devfreq()
- lookup a devfreq strict from a device pointer
Signed-off-by: Matthias Kaehlcke
Reviewed-by: Brian Norris
---
Changes in v5:
- none
Changes in v4:
- Fixed typo in commit message: devfreg => devfreq
- added 'Reviewed-by: Brian Norris ' tag
Changes in v3:
- devfreq.h: fixed misspelling of struct devfreq_policy
Changes in v2:
- performance, powersave and simpleondemand governors don't need changes
with "PM / devfreq: Don't adjust to user limits in governors"
- formatting fixes
---
drivers/devfreq/devfreq.c | 48 ++---
include/linux/devfreq.h | 65 +++
2 files changed, 102 insertions(+), 11 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 21604d6ae2b8..4cbaa7ad1972 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -72,6 +72,21 @@ static struct devfreq *find_device_devfreq(struct device
*dev)
return ERR_PTR(-ENODEV);
}
+/**
+ * dev_to_devfreq() - find devfreq struct using device pointer
+ * @dev: device pointer used to lookup device devfreq.
+ */
+struct devfreq *dev_to_devfreq(struct device *dev)
+{
+ struct devfreq *devfreq;
+
+ mutex_lock(_list_lock);
+ devfreq = find_device_devfreq(dev);
+ mutex_unlock(_list_lock);
+
+ return devfreq;
+}
+
static unsigned long find_available_min_freq(struct devfreq *devfreq)
{
struct dev_pm_opp *opp;
@@ -269,20 +284,21 @@ int update_devfreq(struct devfreq *devfreq)
if (!policy->governor)
return -EINVAL;
+ policy->min = policy->devinfo.min_freq;
+ policy->max = policy->devinfo.max_freq;
+
+ srcu_notifier_call_chain(>policy_notifier_list,
+ DEVFREQ_ADJUST, policy);
+
/* Reevaluate the proper frequency */
err = policy->governor->get_target_freq(devfreq, );
if (err)
return err;
- /*
-* Adjust the frequency with user freq, QoS and available freq.
-*
-* List from the highest priority
-* max_freq
-* min_freq
-*/
- max_freq = MIN(policy->devinfo.max_freq, policy->user.max_freq);
- min_freq = MAX(policy->devinfo.min_freq, policy->user.min_freq);
+ /* Adjust the frequency */
+
+ max_freq = MIN(policy->max, policy->user.max_freq);
+ min_freq = MAX(policy->min, policy->user.min_freq);
if (freq < min_freq) {
freq = min_freq;
@@ -645,6 +661,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
devfreq->last_stat_updated = jiffies;
srcu_init_notifier_head(>transition_notifier_list);
+ srcu_init_notifier_head(>policy_notifier_list);
mutex_unlock(>lock);
@@ -1445,7 +1462,7 @@ EXPORT_SYMBOL(devm_devfreq_unregister_opp_notifier);
* devfreq_register_notifier() - Register a driver with devfreq
* @devfreq: The devfreq object.
* @nb:The notifier block to register.
- * @list: DEVFREQ_TRANSITION_NOTIFIER.
+ * @list: DEVFREQ_TRANSITION_NOTIFIER or DEVFREQ_POLICY_NOTIFIER.
*/
int devfreq_register_notifier(struct devfreq *devfreq,
struct notifier_block *nb,
@@ -1461,6 +1478,10 @@ int devfreq_register_notifier(struct devfreq *devfreq,
ret = srcu_notifier_chain_register(
>transition_notifier_list, nb);
break;
+ case DEVFREQ_POLICY_NOTIFIER:
+ ret = srcu_notifier_chain_register(
+ >policy_notifier_list, nb);
+ break;
default:
ret = -EINVAL;
}
@@ -1473,7 +1494,7 @@ EXPORT_SYMBOL(devfreq_register_notifier);
* devfreq_unregister_notifier() - Unregister a driver with devfreq
* @devfreq: The devfreq object.
* @nb:The notifier block to be unregistered.
- * @list: DEVFREQ_TRANSITION_NOTIFIER.
+ * @list: DEVFREQ_TRANSITION_NOTIFIER or DEVFREQ_POLICY_NOTIFIER.
*/
int devfreq_unregister_notifier(struct devfreq *devfreq,
struct notifier_block *nb,
@@ -1489,6 +1510,11 @@ int devfreq_unregister_notifier(struct devfreq *devfreq,
ret = srcu_notifier_chain_unregister(
>transition_notifier_list, nb);
break;
+ case DEVFREQ_POLICY_NOTIFIER:
+ ret = srcu_notifier_chain_unregister(
+ >policy_notifier_list, nb);
+ break;
+
default:
ret = -EINVAL;
}
diff --git a/include/linux/devfreq.h
[PATCH v5 05/12] PM / devfreq: Add support for policy notifiers
Policy notifiers are called before a frequency change and may narrow
the min/max frequency range in devfreq_policy, which is used to adjust
the target frequency if it is beyond this range.
Also add a few helpers:
- devfreq_verify_within_[dev_]limits()
- should be used by the notifiers for policy adjustments.
- dev_to_devfreq()
- lookup a devfreq strict from a device pointer
Signed-off-by: Matthias Kaehlcke
Reviewed-by: Brian Norris
---
Changes in v5:
- none
Changes in v4:
- Fixed typo in commit message: devfreg => devfreq
- added 'Reviewed-by: Brian Norris ' tag
Changes in v3:
- devfreq.h: fixed misspelling of struct devfreq_policy
Changes in v2:
- performance, powersave and simpleondemand governors don't need changes
with "PM / devfreq: Don't adjust to user limits in governors"
- formatting fixes
---
drivers/devfreq/devfreq.c | 48 ++---
include/linux/devfreq.h | 65 +++
2 files changed, 102 insertions(+), 11 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 21604d6ae2b8..4cbaa7ad1972 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -72,6 +72,21 @@ static struct devfreq *find_device_devfreq(struct device
*dev)
return ERR_PTR(-ENODEV);
}
+/**
+ * dev_to_devfreq() - find devfreq struct using device pointer
+ * @dev: device pointer used to lookup device devfreq.
+ */
+struct devfreq *dev_to_devfreq(struct device *dev)
+{
+ struct devfreq *devfreq;
+
+ mutex_lock(_list_lock);
+ devfreq = find_device_devfreq(dev);
+ mutex_unlock(_list_lock);
+
+ return devfreq;
+}
+
static unsigned long find_available_min_freq(struct devfreq *devfreq)
{
struct dev_pm_opp *opp;
@@ -269,20 +284,21 @@ int update_devfreq(struct devfreq *devfreq)
if (!policy->governor)
return -EINVAL;
+ policy->min = policy->devinfo.min_freq;
+ policy->max = policy->devinfo.max_freq;
+
+ srcu_notifier_call_chain(>policy_notifier_list,
+ DEVFREQ_ADJUST, policy);
+
/* Reevaluate the proper frequency */
err = policy->governor->get_target_freq(devfreq, );
if (err)
return err;
- /*
-* Adjust the frequency with user freq, QoS and available freq.
-*
-* List from the highest priority
-* max_freq
-* min_freq
-*/
- max_freq = MIN(policy->devinfo.max_freq, policy->user.max_freq);
- min_freq = MAX(policy->devinfo.min_freq, policy->user.min_freq);
+ /* Adjust the frequency */
+
+ max_freq = MIN(policy->max, policy->user.max_freq);
+ min_freq = MAX(policy->min, policy->user.min_freq);
if (freq < min_freq) {
freq = min_freq;
@@ -645,6 +661,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
devfreq->last_stat_updated = jiffies;
srcu_init_notifier_head(>transition_notifier_list);
+ srcu_init_notifier_head(>policy_notifier_list);
mutex_unlock(>lock);
@@ -1445,7 +1462,7 @@ EXPORT_SYMBOL(devm_devfreq_unregister_opp_notifier);
* devfreq_register_notifier() - Register a driver with devfreq
* @devfreq: The devfreq object.
* @nb:The notifier block to register.
- * @list: DEVFREQ_TRANSITION_NOTIFIER.
+ * @list: DEVFREQ_TRANSITION_NOTIFIER or DEVFREQ_POLICY_NOTIFIER.
*/
int devfreq_register_notifier(struct devfreq *devfreq,
struct notifier_block *nb,
@@ -1461,6 +1478,10 @@ int devfreq_register_notifier(struct devfreq *devfreq,
ret = srcu_notifier_chain_register(
>transition_notifier_list, nb);
break;
+ case DEVFREQ_POLICY_NOTIFIER:
+ ret = srcu_notifier_chain_register(
+ >policy_notifier_list, nb);
+ break;
default:
ret = -EINVAL;
}
@@ -1473,7 +1494,7 @@ EXPORT_SYMBOL(devfreq_register_notifier);
* devfreq_unregister_notifier() - Unregister a driver with devfreq
* @devfreq: The devfreq object.
* @nb:The notifier block to be unregistered.
- * @list: DEVFREQ_TRANSITION_NOTIFIER.
+ * @list: DEVFREQ_TRANSITION_NOTIFIER or DEVFREQ_POLICY_NOTIFIER.
*/
int devfreq_unregister_notifier(struct devfreq *devfreq,
struct notifier_block *nb,
@@ -1489,6 +1510,11 @@ int devfreq_unregister_notifier(struct devfreq *devfreq,
ret = srcu_notifier_chain_unregister(
>transition_notifier_list, nb);
break;
+ case DEVFREQ_POLICY_NOTIFIER:
+ ret = srcu_notifier_chain_unregister(
+ >policy_notifier_list, nb);
+ break;
+
default:
ret = -EINVAL;
}
diff --git a/include/linux/devfreq.h
