Message-ID: <b6a4e31f-610a-42f0-8e3e-970d3610384b@amd.com>
Date: Mon, 18 Nov 2024 22:13:21 -0600
From: Mario Limonciello <mario.limonciello@....com>
To: Armin Wolf <W_Armin@....de>, Hans de Goede <hdegoede@...hat.com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: "Rafael J . Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
 Maximilian Luz <luzmaximilian@...il.com>, Lee Chun-Yi <jlee@...e.com>,
 Shyam Sundar S K <Shyam-sundar.S-k@....com>,
 Corentin Chary <corentin.chary@...il.com>, "Luke D . Jones"
 <luke@...nes.dev>, Ike Panhc <ike.pan@...onical.com>,
 Henrique de Moraes Holschuh <hmh@....eng.br>,
 Alexis Belmonte <alexbelm48@...il.com>,
 Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
 Ai Chao <aichao@...inos.cn>, Gergo Koteles <soyer@....hu>,
 open list <linux-kernel@...r.kernel.org>,
 "open list:ACPI" <linux-acpi@...r.kernel.org>,
 "open list:MICROSOFT SURFACE PLATFORM PROFILE DRIVER"
 <platform-driver-x86@...r.kernel.org>,
 "open list:THINKPAD ACPI EXTRAS DRIVER"
 <ibm-acpi-devel@...ts.sourceforge.net>,
 Mark Pearson <mpearson-lenovo@...ebb.ca>,
 Matthew Schwartz <matthew.schwartz@...ux.dev>
Subject: Re: [PATCH v6 00/22] Add support for binding ACPI platform profile to
 multiple drivers

On 11/18/2024 15:05, Armin Wolf wrote:
> Am 14.11.24 um 22:57 schrieb Armin Wolf:
> 
>> Am 09.11.24 um 05:41 schrieb Mario Limonciello:
>>
>>> Currently there are a number of ASUS products on the market that
>>> happen to
>>> have ACPI objects for amd-pmf to bind to as well as an ACPI platform
>>> profile provided by asus-wmi.
>>>
>>> The ACPI platform profile support created by amd-pmf on these ASUS
>>> products is "Function 9" which is specifically for "BIOS or EC
>>> notification" of power slider position. This feature is actively used
>>> by some designs such as Framework 13 and Framework 16.
>>>
>>> On these ASUS designs we keep on quirking more and more of them to turn
>>> off this notification so that asus-wmi can bind.
>>>
>>> This however isn't how Windows works.  "Multiple" things are notified
>>> for
>>> the power slider position. This series adjusts Linux to behave
>>> similarly.
>>>
>>> Multiple drivers can now register an ACPI platform profile and will
>>> react
>>> to set requests.
>>>
>>> To avoid chaos, only positions that are common to both drivers are
>>> accepted when the legacy /sys/firmware/acpi/platform_profile interface
>>> is used.
>>>
>>> This series also adds a new concept of a "custom" profile.  This allows
>>> userspace to discover that there are multiple driver handlers that are
>>> configured differently.
>>>
>>> This series also allows dropping all of the PMF quirks from amd-pmf.
>>
>> Sorry for taking a bit long to respond, i am currently quite busy. I
>> will try to review this series
>> in the coming days.
>>
>> Thanks,
>> Armin Wolf
>>
> So far the patch series looks quite good, but a single issue remains: 
> the locking around the class attributes.
> Maybe someone with some knowledge about sysfs can help us here.
> 
> Also can you please rebase the patch series onto the current for-net 
> branch? This would solve a merge conflict
> inside the asus-wmi driver.

Yeah; I've done this locally.  I was going to wait until 6.13-rc1 to 
send it out, but I guess if we don't have any other merge conflicts 
coming in this code I'll send it after we are in agreement on the 
locking and I do some more testing.

> 
> Thanks,
> Armin WOlf
> 
>>> ---
>>> v6:
>>>   * Add patch dev patch but don't make mandatory
>>>   * See other patches changelogs for individualized changes
>>>
>>> Mario Limonciello (22):
>>>    ACPI: platform-profile: Add a name member to handlers
>>>    platform/x86/dell: dell-pc: Create platform device
>>>    ACPI: platform_profile: Add device pointer into platform profile
>>>      handler
>>>    ACPI: platform_profile: Add platform handler argument to
>>>      platform_profile_remove()
>>>    ACPI: platform_profile: Pass the profile handler into
>>>      platform_profile_notify()
>>>    ACPI: platform_profile: Move sanity check out of the mutex
>>>    ACPI: platform_profile: Move matching string for new profile out of
>>>      mutex
>>>    ACPI: platform_profile: Use guard(mutex) for register/unregister
>>>    ACPI: platform_profile: Use `scoped_cond_guard`
>>>    ACPI: platform_profile: Create class for ACPI platform profile
>>>    ACPI: platform_profile: Add name attribute to class interface
>>>    ACPI: platform_profile: Add choices attribute for class interface
>>>    ACPI: platform_profile: Add profile attribute for class interface
>>>    ACPI: platform_profile: Notify change events on register and
>>>      unregister
>>>    ACPI: platform_profile: Only show profiles common for all handlers
>>>    ACPI: platform_profile: Add concept of a "custom" profile
>>>    ACPI: platform_profile: Make sure all profile handlers agree on
>>>      profile
>>>    ACPI: platform_profile: Check all profile handler to calculate next
>>>    ACPI: platform_profile: Notify class device from
>>>      platform_profile_notify()
>>>    ACPI: platform_profile: Allow multiple handlers
>>>    platform/x86/amd: pmf: Drop all quirks
>>>    Documentation: Add documentation about class interface for platform
>>>      profiles
>>>
>>>   .../ABI/testing/sysfs-platform_profile        |   5 +
>>>   .../userspace-api/sysfs-platform_profile.rst  |  28 +
>>>   drivers/acpi/platform_profile.c               | 537 ++++++++++++++----
>>>   .../surface/surface_platform_profile.c        |   8 +-
>>>   drivers/platform/x86/acer-wmi.c               |  12 +-
>>>   drivers/platform/x86/amd/pmf/Makefile         |   2 +-
>>>   drivers/platform/x86/amd/pmf/core.c           |   1 -
>>>   drivers/platform/x86/amd/pmf/pmf-quirks.c     |  66 ---
>>>   drivers/platform/x86/amd/pmf/pmf.h            |   3 -
>>>   drivers/platform/x86/amd/pmf/sps.c            |   4 +-
>>>   drivers/platform/x86/asus-wmi.c               |  10 +-
>>>   drivers/platform/x86/dell/alienware-wmi.c     |   8 +-
>>>   drivers/platform/x86/dell/dell-pc.c           |  36 +-
>>>   drivers/platform/x86/hp/hp-wmi.c              |   8 +-
>>>   drivers/platform/x86/ideapad-laptop.c         |   6 +-
>>>   .../platform/x86/inspur_platform_profile.c    |   7 +-
>>>   drivers/platform/x86/thinkpad_acpi.c          |  16 +-
>>>   include/linux/platform_profile.h              |   9 +-
>>>   18 files changed, 553 insertions(+), 213 deletions(-)
>>>   delete mode 100644 drivers/platform/x86/amd/pmf/pmf-quirks.c
>>>
>>>
>>> base-commit: d68cb6023356af3bd3193983ad4ec03954a0b3e2
>>

Powered by blists - more mailing lists