Re: [PATCH] platform/x86/amd/pmf: install notify handler after acpi init

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 9/23/22 15:17, Shyam Sundar S K wrote:
> It is observed that when thinkpad_acpi driver loads before amd-pmf
> driver, thinkpad_acpi driver sends the AMT "on" event and the request
> immediately will be part of the PMF BIOS "pending requests".
> 
> With the current amd-pmf code, as soon as the amd-pmf driver gets
> probed, it calls apmf_acpi_init() where the notify handler will be
> installed. Handler callback would call amd_pmf_handle_amt() where the
> amd_pmf_set_automode() shall update the auto-mode thermals.
> In this case, the auto-mode config_store shall have "zeros", as the
> auto mode init gets called during the later stage.
> 
> To fix this, change the order of the acpi notifer install and call it
> after the auto mode initialization is done.
> 
> Fixes: 7d77dcc83ada ("platform/x86/amd/pmf: Handle AMT and CQL events for Auto mode")
> Cc: Mario Limonciello <mario.limonciello@amd.com>
> Cc: Mark Pearson <markpearson@lenovo.com>
> Cc: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>  drivers/platform/x86/amd/pmf/acpi.c | 38 +++++++++++++++++------------
>  drivers/platform/x86/amd/pmf/core.c |  1 +
>  drivers/platform/x86/amd/pmf/pmf.h  |  1 +
>  3 files changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index 05a2b8a056fc..b6453157a59d 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -243,6 +243,28 @@ int apmf_get_dyn_slider_def_dc(struct amd_pmf_dev *pdev, struct apmf_dyn_slider_
>  	return apmf_if_call_store_buffer(pdev, APMF_FUNC_DYN_SLIDER_DC, data, sizeof(*data));
>  }
>  
> +int apmf_install_handler(struct amd_pmf_dev *pmf_dev)
> +{
> +	acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
> +	acpi_status status;
> +
> +	/* Install the APMF Notify handler */
> +	if (is_apmf_func_supported(pmf_dev, APMF_FUNC_AUTO_MODE) &&
> +	    is_apmf_func_supported(pmf_dev, APMF_FUNC_SBIOS_REQUESTS)) {
> +		status = acpi_install_notify_handler(ahandle, ACPI_ALL_NOTIFY,
> +						     apmf_event_handler, pmf_dev);
> +		if (ACPI_FAILURE(status)) {
> +			dev_err(pmf_dev->dev, "failed to install notify handler\n");
> +			return -ENODEV;
> +		}
> +
> +	/* Call the handler once manually to catch up with possibly missed notifies. */
> +	apmf_event_handler(ahandle, 0, pmf_dev);
> +}
> +
> +return 0;

Note something has gone wrong with the indentation of the 5 new lines above, they
need an extra tab at the beginning of the line to be properly indented.

I have fixed this up while merging this and added the patch to:

https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=platform-drivers-x86-amd-pmf

Which is where all the AMD-PMF AMT enablement patches are.

I will also merge this branch into review-hans/for-next again so that
this patch will find its way into 6.1-rc1 together with the other AMT
patches.

Regards,

Hans



> +}
> +
>  void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev)
>  {
>  	acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
> @@ -257,8 +279,6 @@ void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev)
>  
>  int apmf_acpi_init(struct amd_pmf_dev *pmf_dev)
>  {
> -	acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
> -	acpi_status status;
>  	int ret;
>  
>  	ret = apmf_if_verify_interface(pmf_dev);
> @@ -279,20 +299,6 @@ int apmf_acpi_init(struct amd_pmf_dev *pmf_dev)
>  		schedule_delayed_work(&pmf_dev->heart_beat, 0);
>  	}
>  
> -	/* Install the APMF Notify handler */
> -	if (is_apmf_func_supported(pmf_dev, APMF_FUNC_AUTO_MODE) &&
> -	    is_apmf_func_supported(pmf_dev, APMF_FUNC_SBIOS_REQUESTS)) {
> -		status = acpi_install_notify_handler(ahandle,
> -						     ACPI_ALL_NOTIFY,
> -						     apmf_event_handler, pmf_dev);
> -		if (ACPI_FAILURE(status)) {
> -			dev_err(pmf_dev->dev, "failed to install notify handler\n");
> -			return -ENODEV;
> -		}
> -		/* Call the handler once manually to catch up with possibly missed notifies. */
> -		apmf_event_handler(ahandle, 0, pmf_dev);
> -	}
> -
>  out:
>  	return ret;
>  }
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index 44fe30726b62..a5f5a4bcff6d 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -369,6 +369,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
>  	apmf_acpi_init(dev);
>  	platform_set_drvdata(pdev, dev);
>  	amd_pmf_init_features(dev);
> +	apmf_install_handler(dev);
>  	amd_pmf_dbgfs_register(dev);
>  
>  	mutex_init(&dev->lock);
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index e5dc3ae238c7..84bbe2c6ea61 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -381,6 +381,7 @@ int is_apmf_func_supported(struct amd_pmf_dev *pdev, unsigned long index);
>  int amd_pmf_send_cmd(struct amd_pmf_dev *dev, u8 message, bool get, u32 arg, u32 *data);
>  int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev);
>  int amd_pmf_get_power_source(void);
> +int apmf_install_handler(struct amd_pmf_dev *pmf_dev);
>  
>  /* SPS Layer */
>  int amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf);