* [PATCH v2 1/3] amd64-agp: convert to generic power management
2021-12-01 2:54 [PATCH v2 RESEND 0/3] agp: convert to generic power management Bjorn Helgaas
@ 2021-12-01 2:54 ` Bjorn Helgaas
2021-12-02 17:43 ` Bjorn Helgaas
2021-12-01 2:54 ` [PATCH v2 2/3] sis-agp: " Bjorn Helgaas
2021-12-01 2:54 ` [PATCH v2 3/3] via-agp: " Bjorn Helgaas
2 siblings, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2021-12-01 2:54 UTC (permalink / raw)
To: David Airlie
Cc: Vaibhav Gupta, Vaibhav Gupta, Rafael J . Wysocki, linux-kernel,
linux-pci, Bjorn Helgaas
From: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Convert amd64-agp from legacy PCI power management to the generic power
management framework.
Previously, amd64-agp used legacy PCI power management.
agp_amd64_suspend() looked like this:
agp_amd64_suspend
pci_save_state(pdev)
pci_set_power_state(pdev, pci_choose_state(pdev, state))
With generic power management, these are both done by the PCI core in
pci_pm_runtime_suspend(), so drop agp_amd64_suspend() completely.
agp_amd64_resume() looked like this:
agp_amd64_resume
pci_set_power_state(pdev, PCI_D0)
pci_restore_state(pdev)
...
With generic power management, the PCI parts are done by
pci_pm_runtime_resume(), so drop those from agp_amd64_resume().
[bhelgaas: commit log]
Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/char/agp/amd64-agp.c | 24 ++++++------------------
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
index b40edae32817..dc78a4fb879e 100644
--- a/drivers/char/agp/amd64-agp.c
+++ b/drivers/char/agp/amd64-agp.c
@@ -588,20 +588,11 @@ static void agp_amd64_remove(struct pci_dev *pdev)
agp_bridges_found--;
}
-#ifdef CONFIG_PM
+#define agp_amd64_suspend NULL
-static int agp_amd64_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused agp_amd64_resume(struct device *dev)
{
- pci_save_state(pdev);
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
- return 0;
-}
-
-static int agp_amd64_resume(struct pci_dev *pdev)
-{
- pci_set_power_state(pdev, PCI_D0);
- pci_restore_state(pdev);
+ struct pci_dev *pdev = to_pci_dev(dev);
if (pdev->vendor == PCI_VENDOR_ID_NVIDIA)
nforce3_agp_init(pdev);
@@ -609,8 +600,6 @@ static int agp_amd64_resume(struct pci_dev *pdev)
return amd_8151_configure();
}
-#endif /* CONFIG_PM */
-
static const struct pci_device_id agp_amd64_pci_table[] = {
{
.class = (PCI_CLASS_BRIDGE_HOST << 8),
@@ -738,15 +727,14 @@ static const struct pci_device_id agp_amd64_pci_promisc_table[] = {
{ }
};
+static SIMPLE_DEV_PM_OPS(agp_amd64_pm_ops, agp_amd64_suspend, agp_amd64_resume);
+
static struct pci_driver agp_amd64_pci_driver = {
.name = "agpgart-amd64",
.id_table = agp_amd64_pci_table,
.probe = agp_amd64_probe,
.remove = agp_amd64_remove,
-#ifdef CONFIG_PM
- .suspend = agp_amd64_suspend,
- .resume = agp_amd64_resume,
-#endif
+ .driver.pm = &agp_amd64_pm_ops,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/3] amd64-agp: convert to generic power management
2021-12-01 2:54 ` [PATCH v2 1/3] amd64-agp: " Bjorn Helgaas
@ 2021-12-02 17:43 ` Bjorn Helgaas
0 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2021-12-02 17:43 UTC (permalink / raw)
To: David Airlie, Rafael J . Wysocki
Cc: Vaibhav Gupta, Vaibhav Gupta, linux-kernel, linux-pci,
Bjorn Helgaas
[cc->to: Rafael: help :)]
On Tue, Nov 30, 2021 at 08:54:17PM -0600, Bjorn Helgaas wrote:
> From: Vaibhav Gupta <vaibhavgupta40@gmail.com>
>
> Convert amd64-agp from legacy PCI power management to the generic power
> management framework.
>
> Previously, amd64-agp used legacy PCI power management.
> agp_amd64_suspend() looked like this:
>
> agp_amd64_suspend
> pci_save_state(pdev)
> pci_set_power_state(pdev, pci_choose_state(pdev, state))
>
> With generic power management, these are both done by the PCI core in
> pci_pm_runtime_suspend(), so drop agp_amd64_suspend() completely.
I think the *patch* is correct, but my explanation is wrong. Would
appreciate any corrections!
Prior to this patch, agp_amd64_suspend() is a pci_driver.suspend()
method and is called in this path:
pci_pm_suspend
pci_legacy_suspend
drv->suspend
After this patch, agp_amd64_suspend() is not implemented at all, and
we do the pci_save_state() and pci_set_power_state() in PCI generic
code.
But I think those actually happen in pci_pm_suspend_noirq(), not in
pci_pm_runtime_suspend(), i.e., in this path:
suspend_devices_and_enter
dpm_suspend_start(PMSG_SUSPEND)
dpm_suspend(PMSG_SUSPEND)
device_suspend
__device_suspend
callback = pm_op(dev->bus->pm, state)
dpm_run_callback(callback)
pci_pm_suspend # PCI bus method
dev->driver->pm->suspend
agp_amd64_suspend # <-- no longer needed
suspend_enter
dpm_suspend_noirq(PMSG_SUSPEND)
dpm_noirq_suspend_devices(PMSG_SUSPEND)
device_suspend_noirq
__device_suspend_noirq
callback = pm_noirq_op(dev->bus->pm, state)
dpm_run_callback(callback)
pci_pm_suspend_noirq # PCI bus method
pci_save_state # <-- now done here
pci_prepare_to_sleep
pci_set_power_state # <-- and here
I got confused because I couldn't find the call chain leading to
pci_pm_suspend_noirq(). pm_op() and dpm_run_callback() essentially
break the call chain, which makes it a little hard to follow.
> agp_amd64_resume() looked like this:
>
> agp_amd64_resume
> pci_set_power_state(pdev, PCI_D0)
> pci_restore_state(pdev)
> ...
>
> With generic power management, the PCI parts are done by
> pci_pm_runtime_resume(), so drop those from agp_amd64_resume().
>
> [bhelgaas: commit log]
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/char/agp/amd64-agp.c | 24 ++++++------------------
> 1 file changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
> index b40edae32817..dc78a4fb879e 100644
> --- a/drivers/char/agp/amd64-agp.c
> +++ b/drivers/char/agp/amd64-agp.c
> @@ -588,20 +588,11 @@ static void agp_amd64_remove(struct pci_dev *pdev)
> agp_bridges_found--;
> }
>
> -#ifdef CONFIG_PM
> +#define agp_amd64_suspend NULL
>
> -static int agp_amd64_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused agp_amd64_resume(struct device *dev)
> {
> - pci_save_state(pdev);
> - pci_set_power_state(pdev, pci_choose_state(pdev, state));
> -
> - return 0;
> -}
> -
> -static int agp_amd64_resume(struct pci_dev *pdev)
> -{
> - pci_set_power_state(pdev, PCI_D0);
> - pci_restore_state(pdev);
> + struct pci_dev *pdev = to_pci_dev(dev);
>
> if (pdev->vendor == PCI_VENDOR_ID_NVIDIA)
> nforce3_agp_init(pdev);
> @@ -609,8 +600,6 @@ static int agp_amd64_resume(struct pci_dev *pdev)
> return amd_8151_configure();
> }
>
> -#endif /* CONFIG_PM */
> -
> static const struct pci_device_id agp_amd64_pci_table[] = {
> {
> .class = (PCI_CLASS_BRIDGE_HOST << 8),
> @@ -738,15 +727,14 @@ static const struct pci_device_id agp_amd64_pci_promisc_table[] = {
> { }
> };
>
> +static SIMPLE_DEV_PM_OPS(agp_amd64_pm_ops, agp_amd64_suspend, agp_amd64_resume);
> +
> static struct pci_driver agp_amd64_pci_driver = {
> .name = "agpgart-amd64",
> .id_table = agp_amd64_pci_table,
> .probe = agp_amd64_probe,
> .remove = agp_amd64_remove,
> -#ifdef CONFIG_PM
> - .suspend = agp_amd64_suspend,
> - .resume = agp_amd64_resume,
> -#endif
> + .driver.pm = &agp_amd64_pm_ops,
> };
>
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/3] sis-agp: convert to generic power management
2021-12-01 2:54 [PATCH v2 RESEND 0/3] agp: convert to generic power management Bjorn Helgaas
2021-12-01 2:54 ` [PATCH v2 1/3] amd64-agp: " Bjorn Helgaas
@ 2021-12-01 2:54 ` Bjorn Helgaas
2021-12-01 2:54 ` [PATCH v2 3/3] via-agp: " Bjorn Helgaas
2 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2021-12-01 2:54 UTC (permalink / raw)
To: David Airlie
Cc: Vaibhav Gupta, Vaibhav Gupta, Rafael J . Wysocki, linux-kernel,
linux-pci, Bjorn Helgaas
From: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Convert sis-agp from legacy PCI power management to the generic power
management framework.
Previously, sis-agp used legacy PCI power management. agp_sis_suspend()
looked like this:
agp_sis_suspend
pci_save_state(pdev)
pci_set_power_state(pdev, pci_choose_state(pdev, state))
With generic power management, these are both done by the PCI core in
pci_pm_runtime_suspend(), so drop agp_sis_suspend() completely.
agp_sis_resume() looked like this:
agp_sis_resume
pci_set_power_state(pdev, PCI_D0)
pci_restore_state(pdev)
sis_driver.configure()
With generic power management, the PCI parts are done by
pci_pm_runtime_resume(), so drop those from agp_sis_resume().
[bhelgaas: commit log]
Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/char/agp/sis-agp.c | 25 ++++++-------------------
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/char/agp/sis-agp.c b/drivers/char/agp/sis-agp.c
index 14909fc5d767..f8a02f4bef1b 100644
--- a/drivers/char/agp/sis-agp.c
+++ b/drivers/char/agp/sis-agp.c
@@ -217,26 +217,14 @@ static void agp_sis_remove(struct pci_dev *pdev)
agp_put_bridge(bridge);
}
-#ifdef CONFIG_PM
+#define agp_sis_suspend NULL
-static int agp_sis_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused agp_sis_resume(
+ __attribute__((unused)) struct device *dev)
{
- pci_save_state(pdev);
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
- return 0;
-}
-
-static int agp_sis_resume(struct pci_dev *pdev)
-{
- pci_set_power_state(pdev, PCI_D0);
- pci_restore_state(pdev);
-
return sis_driver.configure();
}
-#endif /* CONFIG_PM */
-
static const struct pci_device_id agp_sis_pci_table[] = {
{
.class = (PCI_CLASS_BRIDGE_HOST << 8),
@@ -419,15 +407,14 @@ static const struct pci_device_id agp_sis_pci_table[] = {
MODULE_DEVICE_TABLE(pci, agp_sis_pci_table);
+static SIMPLE_DEV_PM_OPS(agp_sis_pm_ops, agp_sis_suspend, agp_sis_resume);
+
static struct pci_driver agp_sis_pci_driver = {
.name = "agpgart-sis",
.id_table = agp_sis_pci_table,
.probe = agp_sis_probe,
.remove = agp_sis_remove,
-#ifdef CONFIG_PM
- .suspend = agp_sis_suspend,
- .resume = agp_sis_resume,
-#endif
+ .driver.pm = &agp_sis_pm_ops,
};
static int __init agp_sis_init(void)
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 3/3] via-agp: convert to generic power management
2021-12-01 2:54 [PATCH v2 RESEND 0/3] agp: convert to generic power management Bjorn Helgaas
2021-12-01 2:54 ` [PATCH v2 1/3] amd64-agp: " Bjorn Helgaas
2021-12-01 2:54 ` [PATCH v2 2/3] sis-agp: " Bjorn Helgaas
@ 2021-12-01 2:54 ` Bjorn Helgaas
2 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2021-12-01 2:54 UTC (permalink / raw)
To: David Airlie
Cc: Vaibhav Gupta, Vaibhav Gupta, Rafael J . Wysocki, linux-kernel,
linux-pci, Bjorn Helgaas
From: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Convert via-agp from legacy PCI power management to the generic power
management framework.
Previously, via-agp used legacy PCI power management. agp_via_suspend()
looked like this:
agp_via_suspend
pci_save_state(pdev)
pci_set_power_state(pdev, pci_choose_state(pdev, state))
With generic power management, these are both done by the PCI core in
pci_pm_runtime_suspend(), so drop agp_via_suspend() completely.
agp_via_resume() looked like this:
agp_via_resume
pci_set_power_state(pdev, PCI_D0)
pci_restore_state(pdev)
...
With generic power management, the PCI parts are done by
pci_pm_runtime_resume(), so drop those from agp_via_resume().
[bhelgaas: commit log]
Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/char/agp/via-agp.c | 25 +++++--------------------
1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/drivers/char/agp/via-agp.c b/drivers/char/agp/via-agp.c
index 87a92a044570..a460ae352772 100644
--- a/drivers/char/agp/via-agp.c
+++ b/drivers/char/agp/via-agp.c
@@ -492,22 +492,11 @@ static void agp_via_remove(struct pci_dev *pdev)
agp_put_bridge(bridge);
}
-#ifdef CONFIG_PM
+#define agp_via_suspend NULL
-static int agp_via_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused agp_via_resume(struct device *dev)
{
- pci_save_state (pdev);
- pci_set_power_state (pdev, PCI_D3hot);
-
- return 0;
-}
-
-static int agp_via_resume(struct pci_dev *pdev)
-{
- struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
-
- pci_set_power_state (pdev, PCI_D0);
- pci_restore_state(pdev);
+ struct agp_bridge_data *bridge = dev_get_drvdata(dev);
if (bridge->driver == &via_agp3_driver)
return via_configure_agp3();
@@ -517,8 +506,6 @@ static int agp_via_resume(struct pci_dev *pdev)
return 0;
}
-#endif /* CONFIG_PM */
-
/* must be the same order as name table above */
static const struct pci_device_id agp_via_pci_table[] = {
#define ID(x) \
@@ -567,16 +554,14 @@ static const struct pci_device_id agp_via_pci_table[] = {
MODULE_DEVICE_TABLE(pci, agp_via_pci_table);
+static SIMPLE_DEV_PM_OPS(agp_via_pm_ops, agp_via_suspend, agp_via_resume);
static struct pci_driver agp_via_pci_driver = {
.name = "agpgart-via",
.id_table = agp_via_pci_table,
.probe = agp_via_probe,
.remove = agp_via_remove,
-#ifdef CONFIG_PM
- .suspend = agp_via_suspend,
- .resume = agp_via_resume,
-#endif
+ .driver.pm = &agp_via_pm_ops,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread