Re: [PATCH v8 01/13] Documentation: x86: Add AMD Hardware Feedback Interface documentation

[Mario Limonciello <superm1@kernel.org>]

On 3/21/2025 08:58, Ilpo Järvinen wrote:
> On Thu, 20 Mar 2025, Mario Limonciello wrote:
> 
>> On 3/19/2025 09:01, Ilpo Järvinen wrote:
>>> On Tue, 18 Feb 2025, Mario Limonciello wrote:
>>>
>>>> From: Perry Yuan <Perry.Yuan@amd.com>
>>>>
>>>> Introduce a new documentation file, `amd_hfi.rst`, which delves into the
>>>> implementation details of the AMD Hardware Feedback Interface and its
>>>> associated driver, `amd_hfi`. This documentation describes how the
>>>> driver provides hint to the OS scheduling which depends on the capability
>>>> of core performance and efficiency ranking data.
>>>>
>>>> This documentation describes
>>>> * The design of the driver
>>>> * How the driver provides hints to the OS scheduling
>>>> * How the driver interfaces with the kernel for efficiency ranking data.
>>>>
>>>> Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
>>>> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
>>>> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>> ---
>>>>    Documentation/arch/x86/amd-hfi.rst | 127 +++++++++++++++++++++++++++++
>>>>    Documentation/arch/x86/index.rst   |   1 +
>>>>    2 files changed, 128 insertions(+)
>>>>    create mode 100644 Documentation/arch/x86/amd-hfi.rst
>>>>
>>>> diff --git a/Documentation/arch/x86/amd-hfi.rst
>>>> b/Documentation/arch/x86/amd-hfi.rst
>>>> new file mode 100644
>>>> index 0000000000000..5d204688470e3
>>>> --- /dev/null
>>>> +++ b/Documentation/arch/x86/amd-hfi.rst
>>>> @@ -0,0 +1,127 @@
>>>> +.. SPDX-License-Identifier: GPL-2.0
>>>> +
>>>> +======================================================================
>>>> +Hardware Feedback Interface For Hetero Core Scheduling On AMD Platform
>>>> +======================================================================
>>>> +
>>>> +:Copyright: 2024 Advanced Micro Devices, Inc. All Rights Reserved.
>>>> +
>>>> +:Author: Perry Yuan <perry.yuan@amd.com>
>>>> +:Author: Mario Limonciello <mario.limonciello@amd.com>
>>>> +
>>>> +Overview
>>>> +--------
>>>> +
>>>> +AMD Heterogeneous Core implementations are comprised of more than one
>>>> +architectural class and CPUs are comprised of cores of various efficiency
>>>> and
>>>> +power capabilities: performance-oriented *classic cores* and
>>>> power-efficient
>>>> +*dense cores*. As such, power management strategies must be designed to
>>>> +accommodate the complexities introduced by incorporating different core
>>>> types.
>>>> +Heterogeneous systems can also extend to more than two architectural
>>>> classes as
>>>> +well. The purpose of the scheduling feedback mechanism is to provide
>>>> +information to the operating system scheduler in real time such that the
>>>> +scheduler can direct threads to the optimal core.
>>>> +
>>>> +The goal of AMD's heterogeneous architecture is to attain power benefit
>>>> by sending
>>>> +background thread to the dense cores while sending high priority threads
>>>> to the classic
>>>> +cores. From a performance perspective, sending background threads to
>>>> dense cores can free
>>>> +up power headroom and allow the classic cores to optimally service
>>>> demanding threads.
>>>> +Furthermore, the area optimized nature of the dense cores allows for an
>>>> increasing
>>>> +number of physical cores. This improved core density will have positive
>>>> multithreaded
>>>> +performance impact.
>>>
>>> Hi Mario,
>>>
>>> Please fold these paragraphs to 80 characters so that they're easier to
>>> read as textfiles (the table can obviously exceed that but there should be
>>> no reason for the text paragraphs to have excessively long lines).
>>>
>>> My apologies for taking so long to get to review this series.
>>
>> No problem.  Thanks for looking.  I'll get a new version ready to put out
>> after the next merge window.
>>
>>> Most of my
>>> comments are quite minor but there's also 1-2 things that seem more
>>> important. It seemed to me that there is some disconnetion between the
>>> promises made in the Kconfig description and what is provided by the patch
>>> series.
>>
>> Some of the series was pared down to go in multiple parts to make it easier to
>> review with follow ups for the dynamic stuff planned for the next iteration.
>>
>> You see some artifacts of that comments and Kconfig.  I figured it was better
>> to leave as is for those given they get to the intent, but I can change if you
>> think it's better to adjust them when the next part lands instead.
> 
> Okay, I thought that might be because such a split to multiple series. I
> think you can leave those as is as I assume to intention is to immediately
> follow up with the other parts (and not like wait a few kernel releases
> or so)?
> 

The next part was going to be submitted by another team.  Let me check 
offline with them on their intended timing and I will make a call what 
to do.