Re: [External] Re: [PATCH] [RFC] Documentation: Add documentation for new platform_profile sysfs attribute

[Hans de Goede]

Hi Mark,

On 10/27/20 1:28 PM, Mark Pearson wrote:
> Hi Elia
> 
> On 27/10/2020 05:19, Elia Devito wrote:
>> Hi to all,
>>
>> In data martedì 27 ottobre 2020 08:54:44 CET, Hans de Goede ha scritto:
>>> Hi,
>>>
>>> On 10/26/20 8:55 PM, Mark Pearson wrote:
>>>> Thanks Hans
>>>>
>>>> On 26/10/2020 14:33, Hans de Goede wrote:
>>>>> Hi Mark,
>>>>>
>>>>> Thank you for this new version.
>>>>>
>>>>> On 10/26/20 6:44 PM, Mark Pearson wrote:
>>>>>> From: Hans de Goede <hdegoede@xxxxxxxxxx>
>>>>
>>>> <snip>
>>>>
>>>>>> +
>>>>>> +If for some reason there is no good match when mapping then a new
>>>>>> profile-name +may be added. Drivers which wish to introduce new
>>>>>> profile-names must: +1. Have very good reasons to do so.
>>>>>> +2. Add the new profile-name to this document, so that future drivers
>>>>>> which also +   have a similar problem can use the same new.
>>>>>
>>>>> s/same new/same name/
>>>>
>>>> I've read this document so many times...I'm not sure how I missed that
>>>> one. Thanks.>
>>>>>> + Usually new profile-names will
>>>>>> +   be added to the "extra profile-names" section of this document. But
>>>>>> in some +   cases the set of standard profile-names may be extended.
>>>>>
>>>>> With the change from a more generic API to this new one more targeted
>>>>> towards DPTF I would drop this part.
>>>>
>>>> OK - I have some questions then related to this change, below
>>>>
>>>>>> +
>>>>>> +What:        /sys/firmware/acpi/platform_profile_choices
>>>>>> +Date:        October 2020
>>>>>> +Contact:    Hans de Goede <hdegoede@xxxxxxxxxx>
>>>>>> +Description:
>>>>>> +        Reading this file gives a space separated list of profiles
>>>>>> +        supported for this device.
>>>>>> +
>>>>>> +        Drivers must use the following standard profile-names whenever
>>>>>> +        possible:
>>>>>> +
>>>>>> +        low-power:        Emphasises low power consumption
>>>>>> +        quiet:            Offers quieter operation (lower fan
>>>>>> +                    speed but with higher performance and
>>>>>> +                    temperatures then seen in low-power
>>>>>
>>>>> I think the description here is a bit too specific, this may cause
>>>>> userspace to have expectations which are not necessary true. I would
>>>>> describe this as just:
>>>>>
>>>>>          quiet:            Emphasises quieter operation
>>>>
>>>> Agreed. I'll update
>>>>
>>>>>> +        balanced:        Balance between low power consumption
>>>>>> +                    and performance
>>>>>> +        performance:        Emphasises performance (and may lead to
>>>>>> +                    higher temperatures and fan speeds)
>>>>>> +
>>>>>> +        Userspace may expect drivers to offer at least several of these
>>>>>> +        standard profile-names! If none of the above are a good match
>>>>>> +        for some of the drivers profiles, then drivers may use one of
>>>>>> +        these extra profile-names:
>>>>>> +        <reserved for future use>
>>>>>> +
>>>>
>>>> If we remove the extra profile-names section above then I think it should
>>>> be removed here too. If someone wants to add a new 'mode' then it would
>>>> be added to the list of 'standard names', and becomes a new option.
>>>> Wanted to check I'm not missing something important.
>>> You are completely right, any references to an extra profile-names section
>>> should be removed here too. I did intend to add that it should be removed
>>> here too, but I forgot.
>>>
>>>>>> +What:        /sys/firmware/acpi/platform_profile
>>>>>> +Date:        October 2020
>>>>>> +Contact:    Hans de Goede <hdegoede@xxxxxxxxxx>
>>>>>> +Description:
>>>>>> +        Reading this file gives the current selected profile for this
>>>>>> +        device. Writing this file with one of the strings from
>>>>>> +        available_profiles changes the profile to the new value.
>>>>>
>>>>> The part about custom profiles below may be dropped. That was intended
>>>>> for use with e.g. GPUs but since this now strictly is a system-level
>>>>> profile API, the part below can be dropped now.
>>>>
>>>> Agreed
>>>>
>>>>>> +
>>>>>> +        Reading this file may also return "custom". This is intended
>>>>>> for
>>>>>> +        drivers which have and export multiple knobs. Such drivers may
>>>>>> +        very well still want to offer a set of profiles for easy of use
>>>>>> +        and to be able to offer a consistent standard API (this API) to
>>>>>> +        userspace for configuring their performance. The "custom" value
>>>>>> +        is intended for when ai user has directly configured the knobs
>>>>>> +        (through e.g. some advanced control-panel for a GPU) and the
>>>>>> +        knob values do not match any of the presets represented by the
>>>>>> +        platform-profiles. In this case writing this file will
>>>>>> +        override the modifications and restore the selected presets.
>>>>>> +
>>>>>
>>>>> Regards,
>>>>>
>>>>> Hans
>>>>
>>>> Thanks!
>>>> mark
>>>
>>> Regards,
>>>
>>> Hans
>>
>> This look good,
>> only thing is that hp-wmi driver need a cool profile (Emphasises the computer
>> cool to touch), if you can add it would be perfect.
>>
>> Regards
>> Elia
>>
>>
>>
> Is low-power is different to cool? I figured low-power was going to be cool so combined them.
> I could call it low-power-cool if that helps? It seems a little clunky but not too bad. I'm sure the user space folks can put sunglasses on it or something ;)

IIRC we already had this discussion, cool means cool-to-touch, so could be done by
e.g. extra aggressive ramping up of the fans, so this is not necessarily the same
as low-power.

Yes this is all somewhat confusing. Luckily (for us kernel folks) we have already
sorta decided to just use the profile-names from the vendors more or less as is and
leave figuring this out further to userspace.

The reason to use the enum + try to have a fixed list of choices is to try and
limit the proliferation of profile-names to keep things somewhat manageable.

But as I discussed previously with Elia (*) we really need all 3 of low-power
cool and quiet.

Regards,

Hans



*) I was coming at this discussion from the same angle you (Mark) are