diff mbox series

[04/57] media: atomisp: Move power-management over to a custom pm-domain

Message ID 20230123125205.622152-5-hdegoede@redhat.com
State Accepted
Commit 553a64b7e7cef7690203bb07bd0280dd142c1015
Headers show
Series media: atomisp: Big power-management changes + lots of fixes | expand

Commit Message

Hans de Goede Jan. 23, 2023, 12:51 p.m. UTC
The atomisp does not use standard PCI power-management through the
PCI config space. Instead this driver directly tells the P-Unit to
disable the ISP over the IOSF. The standard PCI subsystem pm_ops will
try to access the config space before (resume) / after (suspend) this
driver has turned the ISP on / off, resulting in the following errors:

 Unable to change power state from D0 to D3hot, device inaccessible
 Unable to change power state from D3cold to D0, device inaccessible

Getting logged into dmesg a whole bunch of time during boot as well as
every time the camera is used.

To avoid these errors use a custom pm_domain instead of standard driver
pm-callbacks so that all the PCI subsys suspend / resume handling is
skipped and call pci_save_state() / pci_restore_state() ourselves.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../media/atomisp/pci/atomisp_internal.h      |  1 +
 .../staging/media/atomisp/pci/atomisp_v4l2.c  | 44 ++++++++++++++-----
 2 files changed, 33 insertions(+), 12 deletions(-)
diff mbox series

Patch

diff --git a/drivers/staging/media/atomisp/pci/atomisp_internal.h b/drivers/staging/media/atomisp/pci/atomisp_internal.h
index 675007d7d9af..fa38d91420cf 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_internal.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_internal.h
@@ -210,6 +210,7 @@  struct atomisp_device {
 	void __iomem *base;
 	const struct firmware *firmware;
 
+	struct dev_pm_domain pm_domain;
 	struct pm_qos_request pm_qos;
 	s32 max_isr_latency;
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index e786b81921da..e994a4a5284e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -19,6 +19,7 @@ 
  */
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/pm_qos.h>
 #include <linux/timer.h>
@@ -524,7 +525,7 @@  static int atomisp_save_iunit_reg(struct atomisp_device *isp)
 	return 0;
 }
 
-static int __maybe_unused atomisp_restore_iunit_reg(struct atomisp_device *isp)
+static int atomisp_restore_iunit_reg(struct atomisp_device *isp)
 {
 	struct pci_dev *pdev = to_pci_dev(isp->dev);
 
@@ -662,6 +663,7 @@  static void punit_ddr_dvfs_enable(bool enable)
 
 static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
 {
+	struct pci_dev *pdev = to_pci_dev(isp->dev);
 	unsigned long timeout;
 	u32 val = enable ? MRFLD_ISPSSPM0_IUNIT_POWER_ON :
 			   MRFLD_ISPSSPM0_IUNIT_POWER_OFF;
@@ -703,6 +705,7 @@  static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
 		tmp = (tmp >> MRFLD_ISPSSPM0_ISPSSS_OFFSET) & MRFLD_ISPSSPM0_ISPSSC_MASK;
 		if (tmp == val) {
 			trace_ipu_cstate(enable);
+			pdev->current_state = enable ? PCI_D0 : PCI_D3cold;
 			return 0;
 		}
 
@@ -743,6 +746,7 @@  int atomisp_power_off(struct device *dev)
 	pci_write_config_dword(pdev, MRFLD_PCI_CSI_CONTROL, reg);
 
 	cpu_latency_qos_update_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE);
+	pci_save_state(pdev);
 	return atomisp_mrfld_power(isp, false);
 }
 
@@ -756,6 +760,7 @@  int atomisp_power_on(struct device *dev)
 	if (ret)
 		return ret;
 
+	pci_restore_state(to_pci_dev(dev));
 	cpu_latency_qos_update_request(&isp->pm_qos, isp->max_isr_latency);
 
 	/*restore register values for iUnit and iUnitPHY registers*/
@@ -767,7 +772,7 @@  int atomisp_power_on(struct device *dev)
 	return atomisp_css_init(isp);
 }
 
-static int __maybe_unused atomisp_suspend(struct device *dev)
+static int atomisp_suspend(struct device *dev)
 {
 	struct atomisp_device *isp = (struct atomisp_device *)
 				     dev_get_drvdata(dev);
@@ -790,10 +795,12 @@  static int __maybe_unused atomisp_suspend(struct device *dev)
 	}
 	spin_unlock_irqrestore(&isp->lock, flags);
 
+	pm_runtime_resume(dev);
+
 	return atomisp_power_off(dev);
 }
 
-static int __maybe_unused atomisp_resume(struct device *dev)
+static int atomisp_resume(struct device *dev)
 {
 	return atomisp_power_on(dev);
 }
@@ -1603,6 +1610,26 @@  static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	/* save the iunit context only once after all the values are init'ed. */
 	atomisp_save_iunit_reg(isp);
 
+	/*
+	 * The atomisp does not use standard PCI power-management through the
+	 * PCI config space. Instead this driver directly tells the P-Unit to
+	 * disable the ISP over the IOSF. The standard PCI subsystem pm_ops will
+	 * try to access the config space before (resume) / after (suspend) this
+	 * driver has turned the ISP on / off, resulting in the following errors:
+	 *
+	 * "Unable to change power state from D0 to D3hot, device inaccessible"
+	 * "Unable to change power state from D3cold to D0, device inaccessible"
+	 *
+	 * To avoid these errors override the pm_domain so that all the PCI
+	 * subsys suspend / resume handling is skipped.
+	 */
+	isp->pm_domain.ops.runtime_suspend = atomisp_power_off;
+	isp->pm_domain.ops.runtime_resume = atomisp_power_on;
+	isp->pm_domain.ops.suspend = atomisp_suspend;
+	isp->pm_domain.ops.resume = atomisp_resume;
+
+	dev_pm_domain_set(&pdev->dev, &isp->pm_domain);
+
 	pm_runtime_put_noidle(&pdev->dev);
 	pm_runtime_allow(&pdev->dev);
 
@@ -1645,6 +1672,7 @@  static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 request_irq_fail:
 	hmm_cleanup();
 	pm_runtime_get_noresume(&pdev->dev);
+	dev_pm_domain_set(&pdev->dev, NULL);
 	atomisp_unregister_entities(isp);
 register_entities_fail:
 	atomisp_uninitialize_modules(isp);
@@ -1697,6 +1725,7 @@  static void atomisp_pci_remove(struct pci_dev *pdev)
 
 	pm_runtime_forbid(&pdev->dev);
 	pm_runtime_get_noresume(&pdev->dev);
+	dev_pm_domain_set(&pdev->dev, NULL);
 	cpu_latency_qos_remove_request(&isp->pm_qos);
 
 	atomisp_msi_irq_uninit(isp);
@@ -1721,17 +1750,8 @@  static const struct pci_device_id atomisp_pci_tbl[] = {
 
 MODULE_DEVICE_TABLE(pci, atomisp_pci_tbl);
 
-static const struct dev_pm_ops atomisp_pm_ops = {
-	.runtime_suspend = atomisp_power_off,
-	.runtime_resume = atomisp_power_on,
-	.suspend = atomisp_suspend,
-	.resume = atomisp_resume,
-};
 
 static struct pci_driver atomisp_pci_driver = {
-	.driver = {
-		.pm = &atomisp_pm_ops,
-	},
 	.name = "atomisp-isp2",
 	.id_table = atomisp_pci_tbl,
 	.probe = atomisp_pci_probe,