Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.117.161 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.117.161; helo=mail.nvidia.com; pr=C
Message-ID: <af03ee80-9e72-eecf-2a81-78a35eb4a07c@nvidia.com>
Date:   Tue, 10 Oct 2023 13:21:16 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [Patch v4 2/2] ACPI: processor: reduce CPUFREQ thermal reduction
 pctg for Tegra241
Content-Language: en-US
To:     <rafael@kernel.org>, <rui.zhang@intel.com>, <lenb@kernel.org>,
        <linux-acpi@vger.kernel.org>, <linux-tegra@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
CC:     <treding@nvidia.com>, <jonathanh@nvidia.com>, <bbasu@nvidia.com>,
        <sanjayc@nvidia.com>, <ksitaraman@nvidia.com>,
        <srikars@nvidia.com>, <jbrasen@nvidia.com>,
        Sumit Gupta <sumitg@nvidia.com>
References: <20231009171839.12267-1-sumitg@nvidia.com>
 <20231009171839.12267-3-sumitg@nvidia.com>
From:   Sumit Gupta <sumitg@nvidia.com>
In-Reply-To: <20231009171839.12267-3-sumitg@nvidia.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Oct 2023 07:51:34.1750
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: e6d3d4eb-08d5-48e5-d633-08dbc965b973
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.117.161];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: DS3PEPF000099DA.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR12MB6408
X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Tue, 10 Oct 2023 00:52:02 -0700 (PDT)


On 09/10/23 22:48, Sumit Gupta wrote:
> From: Srikar Srimath Tirumala <srikars@nvidia.com>
> 
> Current implementation of processor_thermal performs software throttling
> in fixed steps of "20%" which can be too coarse for some platforms.
> We observed some performance gain after reducing the throttle percentage.
> Change the CPUFREQ thermal reduction percentage and maximum thermal steps
> to be configurable. Also, update the default values of both for Nvidia
> Tegra241 (Grace) SoC. The thermal reduction percentage is reduced to "5%"
> and accordingly the maximum number of thermal steps are increased as they
> are derived from the reduction percentage.
> 
> Signed-off-by: Srikar Srimath Tirumala <srikars@nvidia.com>
> Co-developed-by: Sumit Gupta <sumitg@nvidia.com>
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> ---
>   drivers/acpi/arm64/Makefile          |  1 +
>   drivers/acpi/arm64/thermal_cpufreq.c | 20 ++++++++++++++++
>   drivers/acpi/processor_thermal.c     | 35 +++++++++++++++++++++++++---
>   include/linux/acpi.h                 |  9 +++++++
>   4 files changed, 62 insertions(+), 3 deletions(-)
>   create mode 100644 drivers/acpi/arm64/thermal_cpufreq.c
> 
> diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile
> index 143debc1ba4a..3f181d8156cc 100644
> --- a/drivers/acpi/arm64/Makefile
> +++ b/drivers/acpi/arm64/Makefile
> @@ -5,3 +5,4 @@ obj-$(CONFIG_ACPI_GTDT) 	+= gtdt.o
>   obj-$(CONFIG_ACPI_APMT) 	+= apmt.o
>   obj-$(CONFIG_ARM_AMBA)		+= amba.o
>   obj-y				+= dma.o init.o
> +obj-$(CONFIG_ACPI)		+= thermal_cpufreq.o
> diff --git a/drivers/acpi/arm64/thermal_cpufreq.c b/drivers/acpi/arm64/thermal_cpufreq.c
> new file mode 100644
> index 000000000000..de834fb013e7
> --- /dev/null
> +++ b/drivers/acpi/arm64/thermal_cpufreq.c
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/acpi.h>
> +
> +#ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY
> +#define SMCCC_SOC_ID_T241      0x036b0241
> +
> +int acpi_thermal_cpufreq_pctg(void)
> +{
> +	s32 soc_id = arm_smccc_get_soc_id_version();
> +
> +	/*
> +	 * Check JEP106 code for NVIDIA Tegra241 chip (036b:0241) and
> +	 * reduce the CPUFREQ Thermal reduction percentage to 5%.
> +	 */
> +	if (soc_id == SMCCC_SOC_ID_T241)
> +		return 5;
> +
> +	return 0;
> +}
> +#endif
> diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
> index b7c6287eccca..52f316e4e260 100644
> --- a/drivers/acpi/processor_thermal.c
> +++ b/drivers/acpi/processor_thermal.c
> @@ -26,7 +26,16 @@
>    */
>   
>   #define CPUFREQ_THERMAL_MIN_STEP 0
> -#define CPUFREQ_THERMAL_MAX_STEP 3
> +
> +static int cpufreq_thermal_max_step __read_mostly = 3;
> +
> +/*
> + * Minimum throttle percentage for processor_thermal cooling device.
> + * The processor_thermal driver uses it to calculate the percentage amount by
> + * which cpu frequency must be reduced for each cooling state. This is also used
> + * to calculate the maximum number of throttling steps or cooling states.
> + */
> +static int cpufreq_thermal_pctg __read_mostly = 20;
>   
>   static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
>   
> @@ -71,7 +80,7 @@ static int cpufreq_get_max_state(unsigned int cpu)
>   	if (!cpu_has_cpufreq(cpu))
>   		return 0;
>   
> -	return CPUFREQ_THERMAL_MAX_STEP;
> +	return cpufreq_thermal_max_step;
>   }
>   
>   static int cpufreq_get_cur_state(unsigned int cpu)
> @@ -113,7 +122,8 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
>   		if (!policy)
>   			return -EINVAL;
>   
> -		max_freq = (policy->cpuinfo.max_freq * (100 - reduction_pctg(i) * 20)) / 100;
> +		max_freq = (policy->cpuinfo.max_freq *
> +			    (100 - reduction_pctg(i) * cpufreq_thermal_pctg)) / 100;
>   
>   		cpufreq_cpu_put(policy);
>   
> @@ -126,10 +136,29 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
>   	return 0;
>   }
>   
> +static void acpi_thermal_cpufreq_config(void)
> +{
> +	int cpufreq_pctg = acpi_thermal_cpufreq_pctg();
> +
> +	if (!cpufreq_pctg)
> +		return;
> +
> +	cpufreq_thermal_pctg = cpufreq_pctg;
> +
> +	/*
> +	 * Derive the MAX_STEP from minimum throttle percentage so that the reduction
> +	 * percentage doesn't end up becoming negative. Also, cap the MAX_STEP so that
> +	 * the CPU performance doesn't become 0.
> +	 */
> +	cpufreq_thermal_max_step = (100 / cpufreq_thermal_pctg) - 1;
> +}
> +
>   void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy)
>   {
>   	unsigned int cpu;
>   
> +	acpi_thermal_cpufreq_config();
> +
>   	for_each_cpu(cpu, policy->related_cpus) {
>   		struct acpi_processor *pr = per_cpu(processors, cpu);
>   		int ret;
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index ba3f601b6e3d..052c90d7479a 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1541,4 +1541,13 @@ static inline void acpi_device_notify(struct device *dev) { }
>   static inline void acpi_device_notify_remove(struct device *dev) { }
>   #endif
>   
> +#ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY
> +int acpi_thermal_cpufreq_pctg(void);
> +#else
> +inline int acpi_thermal_cpufreq_pctg(void)
> +{
> +	return 0;
> +}
> +#endif
> +
>   #endif	/*_LINUX_ACPI_H*/

Hi Rafael,

The below change fixes the issue reported by the kernel test robot.
If there are no further comments, then I can send a v5 with this change?

+++ b/include/linux/acpi.h
@@ -1542,7 +1542,7 @@ static inline void 
acpi_device_notify_remove(struct device *dev) { }
  #ifdef CONFIG_HAVE_ARM_SMCCC_DISCOVERY
  int acpi_thermal_cpufreq_pctg(void);
  #else
-inline int acpi_thermal_cpufreq_pctg(void)
+static inline int acpi_thermal_cpufreq_pctg(void)
  {
         return 0;
  }

Best Regards,
Sumit Gupta