On Thu, 30 Jul 2015 13:35:41 +0530 Viresh Kumar <viresh.ku...@linaro.org> wrote:
Hi Viresh, > Cc'ing Rafael as well.. > > On 29-07-15, 17:46, Punit Agrawal wrote: > > [ adding Viresh ] > > Thanks. That earned me few more patches ;) > > > Radivoje Jovanovic <radivoje.jovano...@linux.intel.com> writes: > > > > > Hi Agarwal, > > > > > > On Fri, 24 Jul 2015 16:26:12 +0100 > > > Punit Agrawal <punit.agra...@arm.com> wrote: > > > > > >> Radivoje Jovanovic <radivoje.jovano...@linux.intel.com> writes: > > >> > > >> > From: Radivoje Jovanovic <radivoje.jovano...@intel.com> > > >> > > > >> > there is no need to keep local state variable. if another > > >> > driver changes the policy under our feet the cpu_cooling > > >> > driver will have the wrong state. Get current state from the > > >> > policy directly instead > > >> > > > >> > > >> Although the patch below looks good, it does add additional > > >> processing. I was wondering in what situation do you observe the > > >> problem $SUBJECT solves? > > >> > > >> Presumably, the policy caps are tighter than those imposed by > > >> the cpu cooling device (cpufreq_thermal_notifier should take > > >> care of this). > > > > > > we are using this solution on the platfrom which has user space > > > component control cpufreq throttling. However, user space > > > component has its limitations so we are using cpu_cooling as a > > > critical backup. Due to this cpu_cooling does not have correct > > > state as a current state so when the change is needed cpu_cooling > > > does not make the change since it believes it is in the "correct" > > > state. I agree that there is slight increase in processing, but > > > in the case when user space is changing the policy the notifier > > > will not have access to the current state of the cpu_cooling to > > > change it appropriately. > > > > > > > Makes sense. Thanks for the explanation. > > Sorry, but with what I understood it doesn't make sense. And I can be > wrong here, so please don't laugh at me :) > > So, we have two external suppliers to policy->max here: > - user space: which decides the maximum frequency the policy can ever > achieve. > - thermal: which decides the maximum safe frequency the policy should > ever be set to. > > We need to set policy->max based on what user requested, but keeping > in mind the thermal limitations. > > So if the clipped-freq from thermal is higher than what user has > requested, we don't need to do anything. > > But if the clipped-freq is lower than what user has requested, then > we need to correct that to keep the system in safe range. > > That's what the code is doing as well. > > Now coming to the change you made. What you are saying is, we should > report current state based on the value of policy->max. But why? > > policy->max can be lesser than clipped-freq (set by thermal), and the > current state of thermal clipped-freq isn't what policy->max gives. > > Now, I didn't understood when you said "cpu_cooling doesn't change > the state since it believes it is in correct state". Can you please > explain that with some example? > In this case both userspace thermal solution and cpu_cooling are changing policy->max and the userspace solution will let governor or HW (depends on architecture) decide the clipped-freq. Now let us say that cpu_cooling has 4 available states 0-3 and let us say that cpu_cooling has set the state 1 as the last state. Now userspace component comes in and changes the state of the system that matches cpu_cooling state 0. cpu_cooling is unaware of this change and does not change the local cur_state. Now the temperature changes and cpu_cooling should change the system state to 1 (userspace component malfunctioned and is not picking up this change) but since the cur_state is already at 1 cpu_cooling will not do anything since it believes it is in the correct state. Hope this explains it better -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
