platform-driver-x86.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] platform/x86/amd/pmf: install notify handler after acpi init
@ 2022-09-23 13:17 Shyam Sundar S K
  2022-09-23 15:15 ` Limonciello, Mario
  2022-09-27 12:50 ` Hans de Goede
  0 siblings, 2 replies; 3+ messages in thread
From: Shyam Sundar S K @ 2022-09-23 13:17 UTC (permalink / raw)
  To: hdegoede, markgross, mario.limonciello
  Cc: platform-driver-x86, Patil.Reddy, bnocera, Shyam Sundar S K,
	Mark Pearson

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;
+}
+
 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);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] platform/x86/amd/pmf: install notify handler after acpi init
  2022-09-23 13:17 [PATCH] platform/x86/amd/pmf: install notify handler after acpi init Shyam Sundar S K
@ 2022-09-23 15:15 ` Limonciello, Mario
  2022-09-27 12:50 ` Hans de Goede
  1 sibling, 0 replies; 3+ messages in thread
From: Limonciello, Mario @ 2022-09-23 15:15 UTC (permalink / raw)
  To: Shyam Sundar S K, hdegoede, markgross
  Cc: platform-driver-x86, Patil.Reddy, bnocera, Mark Pearson

On 9/23/2022 08: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>

LGTM, thanks.

Reviewed-by: Mario Limonciello <mario.limonciello@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;
> +}
> +
>   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);


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] platform/x86/amd/pmf: install notify handler after acpi init
  2022-09-23 13:17 [PATCH] platform/x86/amd/pmf: install notify handler after acpi init Shyam Sundar S K
  2022-09-23 15:15 ` Limonciello, Mario
@ 2022-09-27 12:50 ` Hans de Goede
  1 sibling, 0 replies; 3+ messages in thread
From: Hans de Goede @ 2022-09-27 12:50 UTC (permalink / raw)
  To: Shyam Sundar S K, markgross, mario.limonciello
  Cc: platform-driver-x86, Patil.Reddy, bnocera, Mark Pearson

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);


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-27 12:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-23 13:17 [PATCH] platform/x86/amd/pmf: install notify handler after acpi init Shyam Sundar S K
2022-09-23 15:15 ` Limonciello, Mario
2022-09-27 12:50 ` Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).