Message-ID: <e66d8392-3d8b-4c96-8717-34e51867064b@gmx.de>
Date: Thu, 9 Jan 2025 00:13:14 +0100
From: Armin Wolf <W_Armin@....de>
To: Joshua Grisham <josh@...huagrisham.com>
Cc: 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

Am 08.01.25 um 23:00 schrieb Joshua Grisham:

> 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?

OK, i can live with that. Looking forward to the v5 revision :).

Thanks,
Armin Wolf

>> [...]
>> Thanks,
>> Armin Wolf
>>
> Thank you!
> Joshua
>

Powered by blists - more mailing lists