* Re: System PM QoS patches
2011-06-20 19:28 ` System PM QoS patches Rafael J. Wysocki
@ 2011-06-21 13:27 ` Pihet-XID, Jean
0 siblings, 0 replies; 2+ messages in thread
From: Pihet-XID, Jean @ 2011-06-21 13:27 UTC (permalink / raw)
To: Rafael J. Wysocki, Paul Walmsley, Kevin Hilman, Magnus Damm
Cc: Linux PM mailing list
On Mon, Jun 20, 2011 at 9:28 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> Hi,
>
> [CCing linux-pm]
>
> On Monday, June 20, 2011, Pihet-XID, Jean wrote:
>> Hi,
>>
>> On Fri, Jun 3, 2011 at 12:23 AM, Paul Walmsley <paul@pwsan.com> wrote:
>> > On Thu, 2 Jun 2011, Kevin Hilman wrote:
>> >
>> >> The main idea is to start from the PM QoS API, and just make all those
>> >> APIs take a 'struct device *', and the constraints would be assoicated
>> >> with a struct device. The current system-wide constratints would then
>> >> be a special case of the per-device constratints. Maybe when the struct
>> >> device pointer is NULL, that means set a system-wide constraint. The
>> >> existing users of PM QoS could then be easily converted to the new API
>> >> by just passing in NULL.
>> >
>> > There are so few current in-tree users, I'm not sure if that's needed.
>> >
>> > Of the three system-wide PM QoS parameters, CPU_DMA_LATENCY is the only
>> > one that is really set by in-kernel drivers. And there are only five
>> > drivers that use it (fgrep -r PM_QOS_CPU .).
>> >
>> > mac80211 uses network latency, but only as a way to get some requirements
>> > from userspace. And that's almost certainly the wrong way to do it, since
>> > not all network devices have the same latency requirements. Network
>> > throughput isn't used at all.
>> >
>> >
>> > - Paul
>> >
>>
>> I see 3 implementation options:
>>
>> 1) Add an extra parameter to the PM QoS API to take an extra parameter
>> for the device ptr, e.g. changing
>> void pm_qos_add_request(struct pm_qos_request_list *dep, int
>> pm_qos_class, s32 value)
>> to
>> void pm_qos_add_request(struct pm_qos_request_list *dep, struct
>> device *dev, int pm_qos_class, s32 value)
>>
>> 2) Use a constraints parameter instead of the current parameter list
>> of the PM QoS API, e.g. changing
>> void pm_qos_add_request(struct pm_qos_request_list *dep, int
>> pm_qos_class, s32 value)
>> to
>> void pm_qos_add_request(struct pm_qos_request_list *dep, struct
>> pm_qos_params *params), with:
>> struct pm_qos_params { device *dev; int pm_qos_class; s32 value };
>>
>> 3) Keep the current API and add a new API for the device wake-up
>> latency constraints.
>
> I'm opting for 3, with a twist that the new implementation would use the
> majority of the exsiting code (with some modifications).
>
>> The options 1 & 2 have the advantage of keeping the code generic while
>> option 3 means duplicating most of the code in kernel/pm_qos_params.c.
>
> You don't need to duplicate it. Just modify it so that it works with both
> the existing API and the new one.
OK I took the approach of combining the options 2 and 3, so that we
have the API extended with the wake-up latency constraints without
duplication and (hopefully) not too much intrusion in the existing
code.
>
>> Also the options 1 & 2 are impacting the current API users but as Paul
>> reported it there are so few users so it is not an issue to do so.
>> Option 3 has the advantage of being flexible when more parameters will
>> be needed in the future.
>>
>> What do you think?
>>
>> I am currently trying to implement the option 2. More to come!
>
> Good, looking forward to seeing the code. ;-)
The API changes have been sent to you and the linux-pm ML, as
'[RFC/PATCH 0/2] PM QoS: add a per-device wake-up latency constraint
class'. Cf. PATCH 0/2 for more details on the implementation choices.
The next patches are in preparation.
Feedback is welcome!
>
> Thanks,
> Rafael
>
Regards,
Jean
^ permalink raw reply [flat|nested] 2+ messages in thread