Re: [PATCH v7 00/12] Introduce PMF Smart PC Solution Builder Feature

From: Hans de Goede <hdegoede@redhat.com>
To: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>,
	markgross@kernel.org, ilpo.jarvinen@linux.intel.com,
	basavaraj.natikar@amd.com, jikos@kernel.org,
	benjamin.tissoires@redhat.com
Cc: Patil.Reddy@amd.com, mario.limonciello@amd.com,
	platform-driver-x86@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH v7 00/12] Introduce PMF Smart PC Solution Builder Feature
Date: Mon, 18 Dec 2023 13:05:47 +0100	[thread overview]
Message-ID: <a98c0c5a-caab-4cff-85de-b1b43c19b736@redhat.com> (raw)
In-Reply-To: <20231212014705.2017474-1-Shyam-sundar.S-k@amd.com>

Hi,

On 12/12/23 02:46, Shyam Sundar S K wrote:
> Smart PC Solutions Builder allows for OEM to define a large number of
> custom system states to dynamically switch to. The system states are
> referred to as policies, and multiple policies can be loaded onto the
> system at any given time, however only one policy can be active at a
> given time.
> 
> Policy is a combination of PMF input and output capabilities. The inputs
> are the incoming information from the other kernel subsystems like LID
> state, Sensor info, GPU info etc and the actions are the updating the 
> power limits of SMU etc.
> 
> The policy binary is signed and encrypted by a special key from AMD. This
> policy binary shall have the inputs and outputs which the OEMs can build
> for the platform customization that can enhance the user experience and
> system behavior.
> 
> This series adds the initial support for Smart PC solution to PMF driver.
> 
> Note that, on platforms where CnQF and Smart PC is advertised, Smart PC
> shall have higher precedence and same applies for Auto Mode.

Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans

> 
> v6->v7:
> ---------
>  - handle buffer free during suspend/resume
>  - Move Smart PC checks within Smart PC init function
>  - realloc a updated buffer size during the side load.
>  - Drop patches from 13/15 to 15/15 of V6 series
> 
> v5->v6:
> ---------
>  - Add Ilpo's and Mario's Reviewed-by tags
>  - Drop 13/17 and 14/17 patches from this series which are GPU centric
>  - Drop separate checks for battery handling.
>  - Handle SFH failure cases
> 
> v4->v5:
> ---------
>  - Remove PMF-GPU interface from amdgpu driver and add DRM/backlight
>    changes within PMF
>  - Add module_softdep for AMDGPU
>  - remove error checks for debugfs_create_file()
>  - Add "Reviewed-by:" tags
>  - Add kerneldoc for kernel-wide headers
>  - Add checks for acpi_backlight_native
>  - Add early return for SFH path
>  - other cosmetic changes
>  
> v3->v4:
> ---------
> - Split v3 9/16 into 2 patches, that addresses using generic fn names
> - Add softdep [Ilpo] instead of request_module()
> - return proper ACPI status [Mario]
> - Update comments in code [Mario]
> - Remove missed double _ remarks
> - handle battery status branches [Ilpo]
> - Address KASAN problems 
> 
> v2->v3:
> ---------
> - Remove pci_get_device() for getting gpu handle
> - add .suspend handler for pmf driver
> - remove unwanted type caste
> - Align comments, spaces etc.
> - add wrapper for print_hex_dump_debug()
> - Remove lkp tags in commit-msg
> - Add macros for magic numbers
> - use right format specifiers for printing
> - propagate error codes back to the caller
> - remove unwanted comments
> 
> v1->v2:
> ---------
> - Remove __func__ macros
> - Remove manual function names inside prints
> - Handle tee_shm_get_va() failure
> - Remove double _
> - Add meaningful prints
> - pass amd_pmf_set_dram_addr() failure errors
> - Add more information to commit messages
> - use right format specifiers
> - use devm_ioremap() instead of ioremap()
> - address unsigned long vs u32 problems
> - Fix lkp reported issues
> - Add amd_pmf_remove_pb() to remove the debugfs files created(if any).
> - Make amd_pmf_open_pb() as static.
> - Add cooling device APIs for controlling amdgpu backlight
> - handle amd_pmf_apply_policies() failures
> - Split v1 14/15 into 2 patches further
> - use linux/units.h for better handling
> - add "depends on" AMD_SFH_HID for interaction with SFH
> - other cosmetic remarks
> 
> Shyam Sundar S K (12):
>   platform/x86/amd/pmf: Add PMF TEE interface
>   platform/x86/amd/pmf: Add support for PMF-TA interaction
>   platform/x86/amd/pmf: Change return type of amd_pmf_set_dram_addr()
>   platform/x86/amd/pmf: Add support for PMF Policy Binary
>   platform/x86/amd/pmf: change amd_pmf_init_features() call sequence
>   platform/x86/amd/pmf: Add support to get inputs from other subsystems
>   platform/x86/amd/pmf: Add support update p3t limit
>   platform/x86/amd/pmf: Add support to update system state
>   platform/x86/amd/pmf: Make source_as_str() as non-static
>   platform/x86/amd/pmf: Add facility to dump TA inputs
>   platform/x86/amd/pmf: Add capability to sideload of policy binary
>   platform/x86/amd/pmf: dump policy binary data
> 
>  Documentation/admin-guide/index.rst   |   1 +
>  Documentation/admin-guide/pmf.rst     |  24 ++
>  drivers/platform/x86/amd/pmf/Kconfig  |   1 +
>  drivers/platform/x86/amd/pmf/Makefile |   3 +-
>  drivers/platform/x86/amd/pmf/acpi.c   |  37 ++
>  drivers/platform/x86/amd/pmf/core.c   |  52 ++-
>  drivers/platform/x86/amd/pmf/pmf.h    | 203 +++++++++++
>  drivers/platform/x86/amd/pmf/spc.c    | 158 +++++++++
>  drivers/platform/x86/amd/pmf/sps.c    |   5 +-
>  drivers/platform/x86/amd/pmf/tee-if.c | 469 ++++++++++++++++++++++++++
>  10 files changed, 936 insertions(+), 17 deletions(-)
>  create mode 100644 Documentation/admin-guide/pmf.rst
>  create mode 100644 drivers/platform/x86/amd/pmf/spc.c
>  create mode 100644 drivers/platform/x86/amd/pmf/tee-if.c
> 