devfreq: Add suspend and resume apis
Quoted 22 lines · On Monday 08 of October 2012 10:48:24 MyungJoo Ham wrote:
Quoted 9 lines
Quoted 9 lines · Rafael J. Wysocki
Quoted 8 lines · On Thursday 04 of October 2012 14:58:33 Rajagopal Venkat wrote:
Quoted 6 lines
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 <[email protected]>Acked-by MyungJoo Ham <[email protected]>Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
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.
Quoted 3 lines
I guess implementing the idea may be not so obvious; thus, seperating the patchset would be appropriate.
Quoted 6 lines
Quoted 4 lines
When we implement the idea, we may need to implement at the
pm_runtimecore. Becausedevfreq->devis a child of the target dev, not a parent, runtime-pm event of the target dev does not automatically invoke a cascaded action on thedevfreq->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.
Quoted 10 lines
Quoted 9 lines
Maybe either
1) 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?
Quoted 11 lines
I'm not a big fan of notifiers, so I'd prefer to avoid using them, if possible.
Thanks,
RafaelSignature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
Cheers,
MyungJoo �淸�{.n�+돴윯돪�+%듚�깁負쪐w��{.n�+돴쪐{깸G쳺��{ay� ���,j�f＂톒쉹��묎�z_溫(�쉸듶줷"얎�m����G쳺�◀?솳鈺�&ｘ�~뤳턨O뺞촾퇉v�^�m��� �띠�◀?뻀�
Quoted 22 lines · On Tuesday 16 of October 2012 06:40:19 MyungJoo Ham wrote:
Quoted 22 lines · On Monday 08 of October 2012 10:48:24 MyungJoo Ham wrote:
Quoted 9 lines
Quoted 9 lines · Rafael J. Wysocki
Quoted 8 lines · On Thursday 04 of October 2012 14:58:33 Rajagopal Venkat wrote:
Quoted 6 lines
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 <[email protected]>Acked-by MyungJoo Ham <[email protected]>Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
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.
Quoted 3 lines
I guess implementing the idea may be not so obvious; thus, seperating the patchset would be appropriate.
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
When we implement the idea, we may need to implement at the
pm_runtimecore. Becausedevfreq->devis a child of the target dev, not a parent, runtime-pm event of the target dev does not automatically invoke a cascaded action on thedevfreq->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.
Quoted 4 lines
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.
Quoted 2 lines
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.
Quoted 3 lines
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.
Quoted 3 lines
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.
Quoted 3 lines
Anyway, if
devfreq->devis 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
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.