On Monday 08 of October 2012 10:48:24 MyungJoo Ham wrote:
On 8 October 2012 03:31, Rafael J. Wysocki rjw@sisk.pl wrote:
On Thursday 04 of October 2012 14:58:33 Rajagopal Venkat wrote:
Add devfreq suspend/resume apis for devfreq users. This patch supports suspend and resume of devfreq load monitoring, required for devices which can idle.
Signed-off-by: Rajagopal Venkat rajagopal.venkat@linaro.org Acked-by: MyungJoo Ham myungjoo.ham@samsung.com
Well, I wonder if this may be tied in to the runtime PM framework, so that, for example, pm_runtime_suspend() will automatically suspend devfreq on success (and the runtime resume of the device will resume devfreq)?
That's a good idea. If you agree, we can handle this as separate patch on top this patchset.
Sure.
I guess implementing the idea may be not so obvious; thus, seperating the patchset would be appropriate.
When we implement the idea, we may need to implement at the pm_runtime core. Because devfreq->dev is a child of the target dev, not a parent, runtime-pm event of the target dev does not automatically invoke a cascaded action on the devfreq->dev.
I'm not exactly sure what you mean, care to explain?
When a device "p" creates devfreq, devfreq->dev->parent = p. And, devfreq's functions need to react to the "p"'s runtime-pm events.
However, when "p"'s runtime-pm-suspend is being invoked, devfreq->dev's runtime-pm-suspend won't be automatically invoked.
Thus, we will need a mechanism that invokes devfreq->dev's runtime-pm callbacks when p's runtime-pm is invoked. (at the runtime-pm core) Or A mechanism that one can notify others (including its children) when the one's runtime-pm is invoked. (probably at the runtime-pm core, too)
Without such support, it appears that the current implementation (calling runtime-pm suspend/resume equivalent devfreq functions manually at device drivers) seems to be inevitable.
Anyway, if devfreq->dev is a parent of "p", runtime-pm core will do the required task automatically; however, it isn't and I don't think it'd be appropriate.
Maybe either
- capability to allow a child to monitor the power state of a parent
(I remember Inki Dae once suggested to add notifiers at runtime-pm, but it seems to be not merged) or 2) letting runtime-pm be aware of devfreq (doesn't feel alright with this??) is required?
I'm not a big fan of notifiers, so I'd prefer to avoid using them, if possible.
Thanks, Rafael
-- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
Cheers, MyungJoo
On Tuesday 16 of October 2012 06:40:19 MyungJoo Ham wrote:
On Monday 08 of October 2012 10:48:24 MyungJoo Ham wrote:
On 8 October 2012 03:31, Rafael J. Wysocki rjw@sisk.pl wrote:
On Thursday 04 of October 2012 14:58:33 Rajagopal Venkat wrote:
Add devfreq suspend/resume apis for devfreq users. This patch supports suspend and resume of devfreq load monitoring, required for devices which can idle.
Signed-off-by: Rajagopal Venkat rajagopal.venkat@linaro.org Acked-by: MyungJoo Ham myungjoo.ham@samsung.com
Well, I wonder if this may be tied in to the runtime PM framework, so that, for example, pm_runtime_suspend() will automatically suspend devfreq on success (and the runtime resume of the device will resume devfreq)?
That's a good idea. If you agree, we can handle this as separate patch on top this patchset.
Sure.
I guess implementing the idea may be not so obvious; thus, seperating the patchset would be appropriate.
When we implement the idea, we may need to implement at the pm_runtime core. Because devfreq->dev is a child of the target dev, not a parent, runtime-pm event of the target dev does not automatically invoke a cascaded action on the devfreq->dev.
I'm not exactly sure what you mean, care to explain?
When a device "p" creates devfreq, devfreq->dev->parent = p.
This is wrong. It shouldn't have been designed this way in the first place and it was my mistake to merge it, apparently.
And, devfreq's functions need to react to the "p"'s runtime-pm events.
However, when "p"'s runtime-pm-suspend is being invoked, devfreq->dev's runtime-pm-suspend won't be automatically invoked.
Well, the parent should not be able to suspend at all before the children have suspended and your design doesn't seem to match that principle.
Thus, we will need a mechanism that invokes devfreq->dev's runtime-pm callbacks when p's runtime-pm is invoked. (at the runtime-pm core)
No, this would be going backwards.
Or A mechanism that one can notify others (including its children) when the one's runtime-pm is invoked. (probably at the runtime-pm core, too)
You don't appear to understand how the runtime PM framework works.
There's no need to notify a child about the parent's runtime suspend, because normally the parent won't be suspended before all of its children have been suspended.
Without such support, it appears that the current implementation (calling runtime-pm suspend/resume equivalent devfreq functions manually at device drivers) seems to be inevitable.
Which only means that the devfreq design in incorrect, so please fix it.
Anyway, if devfreq->dev is a parent of "p", runtime-pm core will do the required task automatically; however, it isn't and I don't think it'd be appropriate.
No, it wouldn't. Actually, I'm not sure what you need the dev in struct devfreq for at all.
As for finding the given device's devfreq structure, for now we can just walk the devfreq_list. To avoid doing that for devices that don't have a struct devfreq attached to them, we can add a devfreq_in_use flag in struct dev_pm_info. That may even help some functions in devfreq.c as far as I can say.
Thanks, Rafael