Currently, the NO_D3 flag is set in ish_probe(), the intel-ish-ipc driver
puts the ISH into D0i3 when system enter both suspend-to-idle(S0ix) and
suspend-to-mem(S3). These patches are to put the ISH into D3 when system
enter S3 and put the ISH into D0i3 when system enter S0ix.
I tested these patches on the following platforms:
ISH 5.2: ICL
ISH 5.0: CNL CFL WHL CML
ISH 4.0: GLK
ISH 3.0: KBL
Test steps:
* Run the IIO Sensor tool to read the accel_3d sensor data
* Run cmd "echo mem > /sys/power/state" to suspend-to-mem
* Press the keyboard to wake up OS.
* Check if the tool can get the sensor data after OS resume.
* Run cmd "echo freeze > /sys/power/state" to suspend-to-idle
* Press the keyboard to wake up OS.
* Check if the tool can get the sensor data after OS resume.
Test results:
The tool can get the accel_3d sensor data after resuming from both
suspend-to-mem and suspend-to-idle.
Changes from v1:
* Fix the indentation issue
* Elaborate the reason to remove the NO_D3 flag
* Split the PATCH v1 to three changes, and try to minimize the lines change
Zhang Lixu (3):
HID: intel-ish-hid: ipc: set NO_D3 flag only when needed
HID: intel-ish-hid: ipc: make ish suspend paths clear
HID: intel-ish-hid: ipc: check the NO_D3 flag to distinguish resume
paths
drivers/hid/intel-ish-hid/ipc/hw-ish.h | 1 +
drivers/hid/intel-ish-hid/ipc/ipc.c | 2 +-
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 95 +++++++++++++++----------
3 files changed, 61 insertions(+), 37 deletions(-)
--
2.17.1
Currently, the NO_D3 flag is set in ish_probe(), and cleared in
ish_remove(). So even if the system goes into S3, ISH is still
in D0i3 state. It makes more sense that put ISH into D3 as system
goes into S3 and put ISH into D0i3 as system goes into suspend-to-idle.
I remove the NO_D3 setting in ish_probe(), so that ISH can enter
D3 state when system enters S3. Only set N0_D3 flag when system
enters the suspend-to-idle or platform specified, and clear it
when system resume.
When the ISH enters D3, the FW will check the DMA bit status.
If the DMA bit is set, the FW will reset automatically. So the
DMA bit need be clear before putting ISH into D3 state.
Signed-off-by: Zhang Lixu <[email protected]>
---
drivers/hid/intel-ish-hid/ipc/hw-ish.h | 1 +
drivers/hid/intel-ish-hid/ipc/ipc.c | 2 +-
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 21 +++++++++++++++++++--
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ipc/hw-ish.h b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
index 1065692f90e2..ddd8e8e87cfa 100644
--- a/drivers/hid/intel-ish-hid/ipc/hw-ish.h
+++ b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
@@ -77,5 +77,6 @@ irqreturn_t ish_irq_handler(int irq, void *dev_id);
struct ishtp_device *ish_dev_init(struct pci_dev *pdev);
int ish_hw_start(struct ishtp_device *dev);
void ish_device_disable(struct ishtp_device *dev);
+int ish_disable_dma(struct ishtp_device *dev);
#endif /* _ISHTP_HW_ISH_H_ */
diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index 18fe8af89aad..8f8dfdf64833 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -672,7 +672,7 @@ irqreturn_t ish_irq_handler(int irq, void *dev_id)
*
* Return: 0 for success else error code.
*/
-static int ish_disable_dma(struct ishtp_device *dev)
+int ish_disable_dma(struct ishtp_device *dev)
{
unsigned int dma_delay;
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index aa80b4d3b740..89b14d2edd0b 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -14,6 +14,7 @@
#include <linux/types.h>
#include <linux/pci.h>
#include <linux/sched.h>
+#include <linux/suspend.h>
#include <linux/interrupt.h>
#include <linux/workqueue.h>
#define CREATE_TRACE_POINTS
@@ -97,6 +98,11 @@ static const struct pci_device_id ish_invalid_pci_ids[] = {
{}
};
+static inline bool ish_should_enter_d0i3(struct pci_dev *pdev)
+{
+ return !pm_suspend_via_firmware() || pdev->device == CHV_DEVICE_ID;
+}
+
/**
* ish_probe() - PCI driver probe callback
* @pdev: pci device
@@ -147,7 +153,6 @@ static int ish_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
/* mapping IO device memory */
hw->mem_addr = pcim_iomap_table(pdev)[0];
ishtp->pdev = pdev;
- pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
/* request and enable interrupt */
ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
@@ -184,7 +189,6 @@ static void ish_remove(struct pci_dev *pdev)
struct ishtp_device *ishtp_dev = pci_get_drvdata(pdev);
ishtp_bus_remove_all_clients(ishtp_dev, false);
- pdev->dev_flags &= ~PCI_DEV_FLAGS_NO_D3;
ish_device_disable(ishtp_dev);
}
@@ -209,6 +213,8 @@ static void __maybe_unused ish_resume_handler(struct work_struct *work)
uint32_t fwsts;
int ret;
+ pdev->dev_flags &= ~PCI_DEV_FLAGS_NO_D3;
+
/* Get ISH FW status */
fwsts = IPC_GET_ISH_FWSTS(dev->ops->get_fw_status(dev));
@@ -267,6 +273,17 @@ static int __maybe_unused ish_suspend(struct device *device)
!dev->suspend_flag,
msecs_to_jiffies(25));
+ if (ish_should_enter_d0i3(pdev)) {
+ /* Set the NO_D3 flag, the ISH would enter D0i3 */
+ pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
+ } else {
+ /*
+ * Clear the DMA bit before putting ISH into D3,
+ * or ISH FW would reset automatically.
+ */
+ ish_disable_dma(dev);
+ }
+
return 0;
}
--
2.17.1
On Thu, 2019-08-08 at 18:21 +0800, Zhang Lixu wrote:
> Currently, the NO_D3 flag is set in ish_probe(), the intel-ish-ipc
> driver
> puts the ISH into D0i3 when system enter both suspend-to-idle(S0ix)
> and
> suspend-to-mem(S3). These patches are to put the ISH into D3 when
> system
> enter S3 and put the ISH into D0i3 when system enter S0ix.
>
> I tested these patches on the following platforms:
> ISH 5.2: ICL
> ISH 5.0: CNL CFL WHL CML
> ISH 4.0: GLK
> ISH 3.0: KBL
>
> Test steps:
> * Run the IIO Sensor tool to read the accel_3d sensor data
> * Run cmd "echo mem > /sys/power/state" to suspend-to-mem
> * Press the keyboard to wake up OS.
> * Check if the tool can get the sensor data after OS resume.
> * Run cmd "echo freeze > /sys/power/state" to suspend-to-idle
> * Press the keyboard to wake up OS.
> * Check if the tool can get the sensor data after OS resume.
>
> Test results:
> The tool can get the accel_3d sensor data after resuming from both
> suspend-to-mem and suspend-to-idle.
>
> Changes from v1:
> * Fix the indentation issue
> * Elaborate the reason to remove the NO_D3 flag
> * Split the PATCH v1 to three changes, and try to minimize the lines
> change
>
> Zhang Lixu (3):
> HID: intel-ish-hid: ipc: set NO_D3 flag only when needed
> HID: intel-ish-hid: ipc: make ish suspend paths clear
> HID: intel-ish-hid: ipc: check the NO_D3 flag to distinguish resume
> paths
>
For the series:
Acked-by: Srinivas Pandruvada <[email protected]>
> drivers/hid/intel-ish-hid/ipc/hw-ish.h | 1 +
> drivers/hid/intel-ish-hid/ipc/ipc.c | 2 +-
> drivers/hid/intel-ish-hid/ipc/pci-ish.c | 95 +++++++++++++++------
> ----
> 3 files changed, 61 insertions(+), 37 deletions(-)
>
On Thu, 8 Aug 2019, Zhang Lixu wrote:
> Currently, the NO_D3 flag is set in ish_probe(), the intel-ish-ipc driver
> puts the ISH into D0i3 when system enter both suspend-to-idle(S0ix) and
> suspend-to-mem(S3). These patches are to put the ISH into D3 when system
> enter S3 and put the ISH into D0i3 when system enter S0ix.
>
> I tested these patches on the following platforms:
> ISH 5.2: ICL
> ISH 5.0: CNL CFL WHL CML
> ISH 4.0: GLK
> ISH 3.0: KBL
>
> Test steps:
> * Run the IIO Sensor tool to read the accel_3d sensor data
> * Run cmd "echo mem > /sys/power/state" to suspend-to-mem
> * Press the keyboard to wake up OS.
> * Check if the tool can get the sensor data after OS resume.
> * Run cmd "echo freeze > /sys/power/state" to suspend-to-idle
> * Press the keyboard to wake up OS.
> * Check if the tool can get the sensor data after OS resume.
>
> Test results:
> The tool can get the accel_3d sensor data after resuming from both
> suspend-to-mem and suspend-to-idle.
>
> Changes from v1:
> * Fix the indentation issue
> * Elaborate the reason to remove the NO_D3 flag
> * Split the PATCH v1 to three changes, and try to minimize the lines change
Applied to for-5.4/ish. Thanks,
--
Jiri Kosina
SUSE Labs