On 3/10/2022 2:21 AM, Ilpo Järvinen wrote:
On Wed, 23 Feb 2022, Ricardo Martinez wrote:
From: Haijun Liu <haijun.liu@xxxxxxxxxxxx>
Introduce the mechanism to lock/unlock the device 'deep sleep' mode.
When the PCIe link state is L1.2 or L2, the host side still can keep
the device is in D0 state from the host side point of view. At the same
time, if the device's 'deep sleep' mode is unlocked, the device will
go to 'deep sleep' while it is still in D0 state on the host side.
Signed-off-by: Haijun Liu <haijun.liu@xxxxxxxxxxxx>
Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@xxxxxxxxx>
Co-developed-by: Ricardo Martinez <ricardo.martinez@xxxxxxxxxxxxxxx>
Signed-off-by: Ricardo Martinez <ricardo.martinez@xxxxxxxxxxxxxxx>
---
...
+int t7xx_pci_sleep_disable_complete(struct t7xx_pci_dev *t7xx_dev)
+{
+ struct device *dev = &t7xx_dev->pdev->dev;
+ int ret;
+
+ ret = wait_for_completion_timeout(&t7xx_dev->sleep_lock_acquire,
+ msecs_to_jiffies(PM_SLEEP_DIS_TIMEOUT_MS));
+ if (!ret)
+ dev_err_ratelimited(dev, "Resource wait complete timed out\n");
+
+ return ret;
+}
+
+/**
+ * t7xx_pci_disable_sleep() - Disable deep sleep capability.
+ * @t7xx_dev: MTK device.
+ *
+ * Lock the deep sleep capability, note that the device can still go into deep sleep
+ * state while device is in D0 state, from the host's point-of-view.
+ *
+ * If device is in deep sleep state, wake up the device and disable deep sleep capability.
+ */
+void t7xx_pci_disable_sleep(struct t7xx_pci_dev *t7xx_dev)
+{
+ unsigned long flags;
+
+ if (atomic_read(&t7xx_dev->md_pm_state) < MTK_PM_RESUMED) {
+ atomic_inc(&t7xx_dev->sleep_disable_count);
+ complete_all(&t7xx_dev->sleep_lock_acquire);
+ return;
+ }
+
+ spin_lock_irqsave(&t7xx_dev->md_pm_lock, flags);
+ if (atomic_inc_return(&t7xx_dev->sleep_disable_count) == 1) {
+ u32 deep_sleep_enabled;
+
+ reinit_completion(&t7xx_dev->sleep_lock_acquire);
You might want to check that there's a mechanism that prevents this
racing with wait_for_completion_timeout() in t7xx_pci_sleep_disable_complete().
I couldn't prove it myself but there are probably aspect in the PM side of
things I wasn't able to take fully into account (that is, which call
paths are not possible to occur).
Those functions are called in the following order:
1.- t7xx_pci_disable_sleep()
2.- t7xx_pci_sleep_disable_complete()
3.- t7xx_pci_enable_sleep()
That sequence and md_pm_lock protect against a race condition between
wait_for_completion_timeout() and reinit_completion().
On the other hand, there could be a race condition between
t7xx_pci_disable_sleep() and t7xx_pci_enable_sleep() which may cause
sleep to get enabled while one thread expects it to be disabled.
The fix would be to protect sleep_disable_count with md_pm_lock, then
sleep_disable_count doesn't need to be declared as atomic.
The next version will include cleanup in this area.
+ t7xx_dev_set_sleep_capability(t7xx_dev, false);
+
+ deep_sleep_enabled = ioread32(IREG_BASE(t7xx_dev) + T7XX_PCIE_RESOURCE_STATUS);
+ deep_sleep_enabled &= T7XX_PCIE_RESOURCE_STS_MSK;
+ if (deep_sleep_enabled) {
+ spin_unlock_irqrestore(&t7xx_dev->md_pm_lock, flags);
+ complete_all(&t7xx_dev->sleep_lock_acquire);
+ return;
+ }
+
+ t7xx_mhccif_h2d_swint_trigger(t7xx_dev, H2D_CH_DS_LOCK);
+ }
+ spin_unlock_irqrestore(&t7xx_dev->md_pm_lock, flags);
+}