Re: [Intel-gfx] [PATCH v8 06/16] mei: gsc: use polling instead of interrupts

2022-09-07 Thread Ceraolo Spurio, Daniele




On 9/7/2022 8:58 AM, Tomas Winkler wrote:

A work-around for a HW issue in XEHPSDV that manifests itself when SW reads
a gsc register when gsc is sending an interrupt. The work-around is
to disable interrupts and to use polling instead.

Cc: James Ausmus 
Signed-off-by: Vitaly Lubart 
Signed-off-by: Tomas Winkler 
Signed-off-by: Alexander Usyskin 


The only changes from the previously reviewed rev are extra comments and 
doc. Those look good, so my r-b stands:


Reviewed-by: Daniele Ceraolo Spurio 

Daniele


---
  drivers/misc/mei/gsc-me.c | 48 ++-
  drivers/misc/mei/hw-me.c  | 80 ---
  drivers/misc/mei/hw-me.h  | 15 +++-
  3 files changed, 128 insertions(+), 15 deletions(-)

diff --git a/drivers/misc/mei/gsc-me.c b/drivers/misc/mei/gsc-me.c
index c8145e9b62b6..2caba3a9ac35 100644
--- a/drivers/misc/mei/gsc-me.c
+++ b/drivers/misc/mei/gsc-me.c
@@ -13,6 +13,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include "mei_dev.h"

  #include "hw-me.h"
@@ -66,13 +67,28 @@ static int mei_gsc_probe(struct auxiliary_device *aux_dev,
  
  	dev_set_drvdata(device, dev);
  
-	ret = devm_request_threaded_irq(device, hw->irq,

-   mei_me_irq_quick_handler,
-   mei_me_irq_thread_handler,
-   IRQF_ONESHOT, KBUILD_MODNAME, dev);
-   if (ret) {
-   dev_err(device, "irq register failed %d\n", ret);
-   goto err;
+   /* use polling */
+   if (mei_me_hw_use_polling(hw)) {
+   mei_disable_interrupts(dev);
+   mei_clear_interrupts(dev);
+   init_waitqueue_head(>wait_active);
+   hw->is_active = true; /* start in active mode for 
initialization */
+   hw->polling_thread = kthread_run(mei_me_polling_thread, dev,
+"kmegscirqd/%s", 
dev_name(device));
+   if (IS_ERR(hw->polling_thread)) {
+   ret = PTR_ERR(hw->polling_thread);
+   dev_err(device, "unable to create kernel thread: %d\n", 
ret);
+   goto err;
+   }
+   } else {
+   ret = devm_request_threaded_irq(device, hw->irq,
+   mei_me_irq_quick_handler,
+   mei_me_irq_thread_handler,
+   IRQF_ONESHOT, KBUILD_MODNAME, 
dev);
+   if (ret) {
+   dev_err(device, "irq register failed %d\n", ret);
+   goto err;
+   }
}
  
  	pm_runtime_get_noresume(device);

@@ -98,7 +114,8 @@ static int mei_gsc_probe(struct auxiliary_device *aux_dev,
  
  register_err:

mei_stop(dev);
-   devm_free_irq(device, hw->irq, dev);
+   if (!mei_me_hw_use_polling(hw))
+   devm_free_irq(device, hw->irq, dev);
  
  err:

dev_err(device, "probe failed: %d\n", ret);
@@ -119,12 +136,17 @@ static void mei_gsc_remove(struct auxiliary_device 
*aux_dev)
  
  	mei_stop(dev);
  
+	hw = to_me_hw(dev);

+   if (mei_me_hw_use_polling(hw))
+   kthread_stop(hw->polling_thread);
+
mei_deregister(dev);
  
  	pm_runtime_disable(_dev->dev);
  
  	mei_disable_interrupts(dev);

-   devm_free_irq(_dev->dev, hw->irq, dev);
+   if (!mei_me_hw_use_polling(hw))
+   devm_free_irq(_dev->dev, hw->irq, dev);
  }
  
  static int __maybe_unused mei_gsc_pm_suspend(struct device *device)

@@ -185,6 +207,9 @@ static int  __maybe_unused 
mei_gsc_pm_runtime_suspend(struct device *device)
if (mei_write_is_idle(dev)) {
hw = to_me_hw(dev);
hw->pg_state = MEI_PG_ON;
+
+   if (mei_me_hw_use_polling(hw))
+   hw->is_active = false;
ret = 0;
} else {
ret = -EAGAIN;
@@ -209,6 +234,11 @@ static int __maybe_unused mei_gsc_pm_runtime_resume(struct 
device *device)
hw = to_me_hw(dev);
hw->pg_state = MEI_PG_OFF;
  
+	if (mei_me_hw_use_polling(hw)) {

+   hw->is_active = true;
+   wake_up(>wait_active);
+   }
+
mutex_unlock(>device_lock);
  
  	irq_ret = mei_me_irq_thread_handler(1, dev);

diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 3a95fe7d4e33..23ad53efbcb7 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -1,6 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0
  /*
- * Copyright (c) 2003-2020, Intel Corporation. All rights reserved.
+ * Copyright (c) 2003-2022, Intel Corporation. All rights reserved.
   * Intel Management Engine Interface (Intel MEI) Linux driver
   */
  
@@ -10,6 +10,7 @@

  #include 
  #include 
  #include 
+#include 
  
  #include "mei_dev.h"

  #include "hbm.h"
@@ -327,9 +328,12 @@ static void mei_me_intr_clear(struct mei_device *dev)
  

[Intel-gfx] [PATCH v8 06/16] mei: gsc: use polling instead of interrupts

2022-09-07 Thread Tomas Winkler
A work-around for a HW issue in XEHPSDV that manifests itself when SW reads
a gsc register when gsc is sending an interrupt. The work-around is
to disable interrupts and to use polling instead.

Cc: James Ausmus 
Signed-off-by: Vitaly Lubart 
Signed-off-by: Tomas Winkler 
Signed-off-by: Alexander Usyskin 
---
 drivers/misc/mei/gsc-me.c | 48 ++-
 drivers/misc/mei/hw-me.c  | 80 ---
 drivers/misc/mei/hw-me.h  | 15 +++-
 3 files changed, 128 insertions(+), 15 deletions(-)

diff --git a/drivers/misc/mei/gsc-me.c b/drivers/misc/mei/gsc-me.c
index c8145e9b62b6..2caba3a9ac35 100644
--- a/drivers/misc/mei/gsc-me.c
+++ b/drivers/misc/mei/gsc-me.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "mei_dev.h"
 #include "hw-me.h"
@@ -66,13 +67,28 @@ static int mei_gsc_probe(struct auxiliary_device *aux_dev,
 
dev_set_drvdata(device, dev);
 
-   ret = devm_request_threaded_irq(device, hw->irq,
-   mei_me_irq_quick_handler,
-   mei_me_irq_thread_handler,
-   IRQF_ONESHOT, KBUILD_MODNAME, dev);
-   if (ret) {
-   dev_err(device, "irq register failed %d\n", ret);
-   goto err;
+   /* use polling */
+   if (mei_me_hw_use_polling(hw)) {
+   mei_disable_interrupts(dev);
+   mei_clear_interrupts(dev);
+   init_waitqueue_head(>wait_active);
+   hw->is_active = true; /* start in active mode for 
initialization */
+   hw->polling_thread = kthread_run(mei_me_polling_thread, dev,
+"kmegscirqd/%s", 
dev_name(device));
+   if (IS_ERR(hw->polling_thread)) {
+   ret = PTR_ERR(hw->polling_thread);
+   dev_err(device, "unable to create kernel thread: %d\n", 
ret);
+   goto err;
+   }
+   } else {
+   ret = devm_request_threaded_irq(device, hw->irq,
+   mei_me_irq_quick_handler,
+   mei_me_irq_thread_handler,
+   IRQF_ONESHOT, KBUILD_MODNAME, 
dev);
+   if (ret) {
+   dev_err(device, "irq register failed %d\n", ret);
+   goto err;
+   }
}
 
pm_runtime_get_noresume(device);
@@ -98,7 +114,8 @@ static int mei_gsc_probe(struct auxiliary_device *aux_dev,
 
 register_err:
mei_stop(dev);
-   devm_free_irq(device, hw->irq, dev);
+   if (!mei_me_hw_use_polling(hw))
+   devm_free_irq(device, hw->irq, dev);
 
 err:
dev_err(device, "probe failed: %d\n", ret);
@@ -119,12 +136,17 @@ static void mei_gsc_remove(struct auxiliary_device 
*aux_dev)
 
mei_stop(dev);
 
+   hw = to_me_hw(dev);
+   if (mei_me_hw_use_polling(hw))
+   kthread_stop(hw->polling_thread);
+
mei_deregister(dev);
 
pm_runtime_disable(_dev->dev);
 
mei_disable_interrupts(dev);
-   devm_free_irq(_dev->dev, hw->irq, dev);
+   if (!mei_me_hw_use_polling(hw))
+   devm_free_irq(_dev->dev, hw->irq, dev);
 }
 
 static int __maybe_unused mei_gsc_pm_suspend(struct device *device)
@@ -185,6 +207,9 @@ static int  __maybe_unused 
mei_gsc_pm_runtime_suspend(struct device *device)
if (mei_write_is_idle(dev)) {
hw = to_me_hw(dev);
hw->pg_state = MEI_PG_ON;
+
+   if (mei_me_hw_use_polling(hw))
+   hw->is_active = false;
ret = 0;
} else {
ret = -EAGAIN;
@@ -209,6 +234,11 @@ static int __maybe_unused mei_gsc_pm_runtime_resume(struct 
device *device)
hw = to_me_hw(dev);
hw->pg_state = MEI_PG_OFF;
 
+   if (mei_me_hw_use_polling(hw)) {
+   hw->is_active = true;
+   wake_up(>wait_active);
+   }
+
mutex_unlock(>device_lock);
 
irq_ret = mei_me_irq_thread_handler(1, dev);
diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 3a95fe7d4e33..23ad53efbcb7 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (c) 2003-2020, Intel Corporation. All rights reserved.
+ * Copyright (c) 2003-2022, Intel Corporation. All rights reserved.
  * Intel Management Engine Interface (Intel MEI) Linux driver
  */
 
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "mei_dev.h"
 #include "hbm.h"
@@ -327,9 +328,12 @@ static void mei_me_intr_clear(struct mei_device *dev)
  */
 static void mei_me_intr_enable(struct mei_device *dev)
 {
-   u32 hcsr = mei_hcsr_read(dev);
+   u32 hcsr;
+
+   if (mei_me_hw_use_polling(to_me_hw(dev)))
+   return;
 
-   hcsr |=