Message-ID: <CAMF+KeZ0xOdu3LADEeAHAC22=hZFQXnkHM3oxko7E7urNisfGw@mail.gmail.com>
Date: Wed, 8 Jan 2025 23:00:52 +0100
From: Joshua Grisham <josh@...huagrisham.com>
To: Armin Wolf <W_Armin@....de>
Cc: Joshua Grisham <josh@...huagrisham.com>, ilpo.jarvinen@...ux.intel.com, 
	hdegoede@...hat.com, platform-driver-x86@...r.kernel.org, corbet@....net, 
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] platform/x86: samsung-galaxybook: Add
 samsung-galaxybook driver

Den tis 7 jan. 2025 kl 19:56 skrev Armin Wolf <W_Armin@....de>:
>
> Is this non-ultra performance mode any different than the ultra performance mode
> in terms of performance gains, fan speed, etc?
>

>From what I can tell it ramps up the performance even more and might
even also ramp up the performance of the GPU (these devices have a
second dedicated GPU) a bit more. My understanding is that even for
these models, "Performance" is considered a high performance mode, but
that "Ultra" is like "super performance" ?

Fan speed I think is mostly controlled based on temperature but it
could also be that some thresholds are adjusted etc. All of this is
unfortunately embedded within the EC so you cannot really see any
voltage, clock, etc, or other differences on the CPU when these modes
are used, even though it is clear from basic stress testing especially
with the "Silent" / low-power mode that the CPU has been severely
limited.

>
> PLATFORM_PROFILE_CUSTOM is meant to signal that the platform is not in a well-defined
> profile state, usually due to manual tuning. So please do not use it for ULTRA.
>

Thank you yes I also realized this a bit more when I read through some
of the proposed changes to other drivers in the mailing list!

> > If this is possible, then my best guess for the logic for this mapping
> > in samsung-galaxybook could be changed to loop the "supported modes"
> > forwards instead of backwards, and just let the "legacy" modes be
> > written first (as they seem to always come first in the list), and
> > then in case the non-legacy mode exists later in the array, it will
> > just replace the already-mapped legacy value with the new non-legacy
> > value, and thus skip any kind of condition-based checking/mapping
> > entirely. Is that sort of more like what you had in mind?
>
> Can you be sure that legacy performance modes are always placed before non-legacy
> performance modes?
>
> If no then i suggest that you iterate over all supported modes and if you encounter
> a legacy performance mode you check if the associated platform profile slot was already
> taken by a non-legacy performance mode. If that is the case you ignore that legacy performance
> mode.
>
> If you are sure that the order is always the same then you can of course simplify this by
> iterating forward. I will leave it to you to choose which one to implement, as you seem
> to have more knowledge about the underlying hardware than me.
>

So far the order has always been the same for all devices I have seen
from users in the community, it is just that certain modes are or are
not present in the list depending on their support. However, based on
your comment I think it is maybe safe to add a bit more logic just in
case the modes suddenly come in a different or random order on some
new device. I have also now simplified the mapping so it is mostly 1:1
with one exception: if Ultra is found, then I map it to performance
and re-map what was Performance to balanced-performance. Otherwise and
for all other devices without Ultra, it is 1:1.

I have also tightened up and streamlined the logic a tiny bit so
hopefully it will feel slightly more straight-forward in the new v5.
This feels like an ok compromise if we should be using exactly the
profiles which are currently available .. how does this sound?

> [...]
> Thanks,
> Armin Wolf
>

Thank you!
Joshua

Powered by blists - more mailing lists