Hello Kurt!
Den lör 11 jan. 2025 kl 18:15 skrev Kurt Borja <kuurtb@xxxxxxxxx>:
>
> > I am glad you bring this up, as it forces me think through this a few
> > more rounds... basically what happens is that the device will always
> > come up from a fresh boot with the value of 0x0 as the "current
> > performance mode" as response from the ACPI method, even though for
> > most devices value 0x0 is the "legacy" optimized value that should not
> > be used.
>
> Is this 0x0 mode real or just a placeholder value? i.e. the device always
> starts with legacy low-power?
>
The performance mode 0 is a "real" value in Samsung's ACPI and
settings services/apps, which maps to what we are calling the "Legacy"
Optimized value. It is the only Optimized value on older devices, but
most newer devices have the newer "Optimized" mode of 0x2. The "funny"
thing is that upon a fresh system start, the ACPI method to get the
current performance mode always returns 0. In Windows, the Samsung
background service/app somehow stores/caches the user's last-used
profile mode and then just sets it to that on startup, so basically
their background app will very quickly change the 0 to one of the
devices "mapped" values upon startup.
My thinking with this driver was to do similar -- use the modes which
Samsung has said "should" be used on these devices (with thinking that
they have put more testing into these modes and support them for all
of their users in Windows, so it feels safer and less likely we could
harm our devices due to overheating etc if we stick to the same modes
that they are supporting and using in Windows ;) ). More on this / a
big simplification IMO with my v6 patch (see below!).
> I don't know if this has been discussed before. If it was you should
> follow their advice instead.
>
> The platform_profile module doesn't enforce selected choices when
> getting the current profile. Choices are only enforced when setting it.
>
> I suggest that galaxybook_platform_profile_get() should map all known
> values to valid platform profile options. Something like:
>
> switch() {
> case GB_PERFORMANCE_MODE_SILENT:
> *profile = PLATFORM_PROFILE_LOW_POWER;
> break;
> case GB_PERFORMANCE_MODE_QUIET:
> *profile = PLATFORM_PROFILE_QUIET;
> break;
> case GB_PERFORMANCE_MODE_OPTIMIZED:
> case GB_PERFORMANCE_MODE_OPTIMIZED_LEGACY:
> *profile = PLATFORM_PROFILE_BALANCED;
> break;
> case GB_PERFORMANCE_MODE_PERFORMANCE:
> case GB_PERFORMANCE_MODE_PERFORMANCE_LEGACY:
> if (ultra)
> *profile = PLATFORM_PROFILE_BALANCED_PERFORMANCE;
> else
> *profile = PLATFORM_PROFILE_PERFORMANCE;
> break;
> case GB_PERFORMANCE_MODE_ULTRA:
> *profile = PLAFORM_PROFILE_PERFORMANCE;
> break;
> default:
> return -ENODATA;
> }
>
Thanks very much for bringing this up! I think for me this has been
the kind of problem that I was just too close to and a bit stuck in
the details, so it is good to have a total re-think like this, and
this kind of feedback is very helpful IMO! I was actually thinking
something very similar already, and then to use this function to help
drive the mapping both for init and for the "get" function during
runtime, with a much higher reliance on the built in facilities from
platform_profile itself to drive the behavior (based on the bits set
on the profile handler choices for example). I have a draft of this
working on my device now and will try to clean it up and get it sent
as a v6 of the patch shortly!
> Also a quick question. Why isn't silent mapped to
> PLATFORM_PROFILE_QUIET. Are there devices that support both
> GB_PERFORMANCE_MODE_SILENT and GB_PERFORMANCE_MODE_QUIET? Are this modes
> different in nature?
>
Yes, they are different modes and often both present together on devices.
Silent = CPU (and possibly others?) become severely crippled and the
fans turn off almost completely. Due to this it feels like "low-power"
is in fact most appropriate for the Samsung "Silent" mode?
Quiet = CPU still works mostly the same as other modes (think we
already start to hit Intel's thermal throttling here, actually...) but
it is apparent that the fans do not come on as often or spin as fast
as it does in other modes (e.g. Optimized, Performance, etc).
In fact the only case I have seen where they both are not present
together is with the Ultra devices -- they usually do not seem to have
the "Silent" mode at all -- assume this is to help with their image of
being "ultra performance" plus maybe to help with overheating risks ?
> I don't think CUSTOM should be used as a placeholder value.
>
Noted and agree, I tried this as a quick test and saw very quickly
that it was a bad idea :)
v6 patch coming soon-ish (today I think/hope)!
Best,
Joshua
