On 9/17/2020 1:03 PM, Hans de Goede wrote:
Hi Mark,
On 9/17/20 6:58 PM, Mark Pearson wrote:
On 9/17/2020 10:10 AM, Benjamin Berg wrote:
Hi,
On Thu, 2020-09-17 at 15:54 +0200, Hans de Goede wrote:
Hi,
On 9/17/20 3:50 PM, Benjamin Berg wrote:
On Thu, 2020-09-17 at 14:51 +0200, Hans de Goede wrote:
Compared to the WIP lenovo-dytc "perfmode" driver, we're missing
something to advertise the unavailability of a profile, and the
reason
for that unavailability.
UGh, do we really need to export this though. We have the lap_mode
thing
already; and that is something which we will need for other
reasons in
the future too. Any UI for selecting performance modes can display a
warning when lap_mode is true saying that: "The laptop has
detected that it
is sitting on someone's lap and that performance may be limited
because of this." (feel free to improve the text).
Well, for dytc_perfmode there are actually always the three states
L/M/H. It just happens that the kernel will write "H*" (was "M*" until
yesterday) when the performance mode is degraded due to lap detection.
Think of dytc_perfmode as a profile that sets a number of things:
* Thermal Limits
* Fan Behaviour
* possibly more
While dytc_lapmode will only enforce a change to the thermal limit.
So "performance" (H) is technically a valid mode even when the lap is
detected.
I guess we could split the "value" attribute from my reply to
Benjamin's
email into "configured_value" (rw) and "actual_value" (rw)
attributes.
If we have the info we might as well export it I guess,.
I consider the "*" purely a curtsey to users that read the attribute
directly using e.g. cat to help with the interpretation. It
probably is
not interesting to userspace applications/daemons.
So if there is a difference between M and H and H* then I think we
should
just do the KISS thing and only have a single value attribute and in
the
new interface handle the H* like H (p-p-d can still check the lap_mode
attribute to differentiate the 2 if it wants to).
I guess you are saying to drop "H*" and only have "L"/"M"/"H"? If so,
fine with me, but we probably need that input in reply to
https://patchwork.kernel.org/patch/11730133/#23618881
then :)
In principle it could be useful for userspace to know that performance
is or would be dramatically impacted. i.e. when dytc_lapmode is 1, then
you might want to say something like:
performance states >= 75 are impacted due to "lapmode"
But, not sure if a kernel interface for that is useful or whether we
should just put that kind of knowledge into userspace.
Benjamin
I don't have a strong opinion on this but the kernel driver is already
knowledgeable about the quirks of what does and doesn't work on the
system so it seems like a good place to have that logic.
What if we have an API for "configured" and "actual" - and if they
differ userspace knows it should figure out why (likely lapmode, but
if the HW vendor adds a new setting related to "position of sun in the
sky" or "how much money is in your account and can you afford the
electricity bill?" that could be added too....)
As I understand the problem with the configured and actual
value/performance_level ideas is that if I understand things correctly
that H* is not the same as M,
it behaves close-ish to M because of the lower thermal-limits from
lapmode, but if I understood Benjamin correctly is is not exactly the
same, so if we were
to advertise "M" in the actual_performance_level sysfs-attribute then
that would not really be correct ?
Just a small clarification - in our case High performance is only for
deskmode. It drops to Medium in lapmode.
Medium mode is slightly different in power rating between lap and desk
mode (e.g on X1Carbon 14.5W on lap, 15W on desk). I haven't really
worried about this in my patch implementation - it's still "medium"
Does that make it better or more confusing?
Mark
