Re: [PATCH 1/3] PM / QoS: Make it possible to expose PM QoS latency constraints

[Kevin Hilman <khilman@ti.com>]

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

> On Friday, March 09, 2012, Kevin Hilman wrote:
>> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
>> 
>> > On Thursday, March 08, 2012, Kevin Hilman wrote:
>> >> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
>> >> 
>> >> > On Thursday, March 08, 2012, Kevin Hilman wrote:
>> >> >> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
>> >> >> 
>> >> >> > From: Rafael J. Wysocki <rjw@sisk.pl>
>> >> >> >
>> >> >> > A runtime suspend of a device (e.g. an MMC controller) belonging to
>> >> >> > a power domain or, in a more complicated scenario, a runtime suspend
>> >> >> > of another device in the same power domain, may cause power to be
>> >> >> > removed from the entire domain.  In that case, the amount of time
>> >> >> > necessary to runtime-resume the given device (e.g. the MMC
>> >> >> > controller) is often substantially greater than the time needed to
>> >> >> > run its driver's runtime resume callback.  That may hurt performance
>> >> >> > in some situations, because user data may need to wait for the
>> >> >> > device to become operational, so we should make it possible to
>> >> >> > prevent that from happening.
>> >> >> >
>> >> >> > For this reason, introduce a new sysfs attribute for devices,
>> >> >> > power/pm_qos_latency_us, allowing user space to specify the upper
>> >> >> 
>> >> >> If we're expecting to have more of these knobs, maybe having a pm_qos
>> >> >> subdir under power will keep down the clutter in /sys/devices/.../power.
>> >> >> This knob would then be /sys/devices/.../power/pm_qos/pm_qos_latency_us.
>> >> >
>> >> > I'm not sure how difficult it is to create a subdir in sysfs under something
>> >> > that is not a kobject.
>> >> >
>> >> > Besides, this follows the convention already used by wakeup and runtime PM
>> >> > attributes that don't have their own subdirs (although there may be a number
>> >> > of them in each category).
>> >> 
>> >> OK
>> >> 
>> >> >> I think 'latency' alone is a bit too vague (wakeup latency?  interrupt
>> >> >> latency?  I think wakeup latency is clearer.  Another possibility is
>> >> >> resume latency, IMO, that will lead to confusion about whether this
>> >> >> field also affects system suspend/resume.
>> >> >
>> >> > I think "wakeup latency" will lead to more confusion because of the
>> >> > wakeup-related attributes.  
>> >> 
>> >> What confusion?  All of those are related to device wakeups from some
>> >> low power state, and so is this proposed latency attribute.  So I don't
>> >> understand the potential confusion.
>> >
>> > The word "wakeup" may refer to many different things, as well as the word
>> > "resume". :-)
>> 
>> Yes, but what's the confusion in this case?
>> 
>> IMO, The existing /sys/devices/.../power/wakeup* meaning is the same
>> meaning as as for the wakeup latency in this patch,
>
> No, it is not.  They refer to system wakeup. :-)

OK, now I'm confused (again).  I thought those could be used for runtime
PM wakeups also.  At least I was planning on using them for any kind of
wakeup.

>> so I don't understand where the confusion would be.
>
> See above. ;-)

Sheesh, this is getting ugly.

So wakeup*  attributes refer to system resume and resume* attribues
refer to runtime PM.

Yuck.

Kevin