From: Bjorn Helgaas <[email protected]>
Vaibhav has converted around 180 drivers to generic power management, and
over 100 of those conversions have made it upstream. If we can finish off
the remaining ones, we'll be able to remove quite a bit of ugly legacy code
from the PCI core.
This is a repost of Vaibhav's patches for AGP. I rebased them to v5.16-rc1
and updated the commit logs to try to make it easier to verify them.
In the most recent posting here:
https://lore.kernel.org/linux-pci/[email protected]/
my commit log updates were incorrect. This v3 has updates that I believe
to be correct, but of course I'd be grateful for more corrections.
Vaibhav Gupta (3):
amd64-agp: convert to generic power management
sis-agp: convert to generic power management
via-agp: convert to generic power management
drivers/char/agp/amd64-agp.c | 24 ++++++------------------
drivers/char/agp/sis-agp.c | 25 ++++++-------------------
drivers/char/agp/via-agp.c | 25 +++++--------------------
3 files changed, 17 insertions(+), 57 deletions(-)
--
2.25.1
From: Vaibhav Gupta <[email protected]>
Convert amd64-agp from legacy PCI power management to the generic power
management framework.
Previously, amd64-agp used legacy PCI power management, and
agp_amd64_suspend() and agp_amd64_resume() were responsible for both
device-specific things and generic PCI things:
agp_amd64_suspend
pci_save_state(pdev) <-- generic PCI
pci_set_power_state(pdev, pci_choose_state(pdev, state)) <-- generic PCI
agp_amd64_resume
pci_set_power_state(pdev, PCI_D0) <-- generic PCI
pci_restore_state(pdev) <-- generic PCI
nforce3_agp_init() <-- device-specific
amd_8151_configure() <-- device-specific
With generic power management, the PCI bus PM methods do the generic PCI
things, and the driver needs only the device-specific part, i.e.,
suspend_devices_and_enter
dpm_suspend_start(PMSG_SUSPEND)
pci_pm_suspend # PCI bus .suspend() method
agp_amd64_suspend <-- not needed at all; removed
suspend_enter
dpm_suspend_noirq(PMSG_SUSPEND)
pci_pm_suspend_noirq # PCI bus .suspend_noirq() method
pci_save_state <-- generic PCI
pci_prepare_to_sleep <-- generic PCI
pci_set_power_state
...
dpm_resume_end(PMSG_RESUME)
pci_pm_resume # PCI bus .resume() method
pci_restore_standard_config
pci_set_power_state(PCI_D0) <-- generic PCI
pci_restore_state <-- generic PCI
agp_amd64_resume # dev->driver->pm->resume
nforce3_agp_init() <-- device-specific
amd_8151_configure() <-- device-specific
[bhelgaas: commit log]
Signed-off-by: Vaibhav Gupta <[email protected]>
Signed-off-by: Bjorn Helgaas <[email protected]>
---
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
From: Vaibhav Gupta <[email protected]>
Convert sis-agp from legacy PCI power management to the generic power
management framework.
Previously, sis-agp used legacy PCI power management, and agp_sis_suspend()
and agp_sis_resume() were responsible for both device-specific things and
generic PCI things:
agp_sis_suspend
pci_save_state(pdev) <-- generic PCI
pci_set_power_state(pdev, pci_choose_state(pdev, state)) <-- generic PCI
agp_sis_resume
pci_set_power_state(pdev, PCI_D0) <-- generic PCI
pci_restore_state(pdev) <-- generic PCI
sis_driver.configure() <-- device-specific
With generic power management, the PCI bus PM methods do the generic PCI
things, and the driver needs only the device-specific part, i.e.,
suspend_devices_and_enter
dpm_suspend_start(PMSG_SUSPEND)
pci_pm_suspend # PCI bus .suspend() method
agp_sis_suspend <-- not needed at all; removed
suspend_enter
dpm_suspend_noirq(PMSG_SUSPEND)
pci_pm_suspend_noirq # PCI bus .suspend_noirq() method
pci_save_state <-- generic PCI
pci_prepare_to_sleep <-- generic PCI
pci_set_power_state
...
dpm_resume_end(PMSG_RESUME)
pci_pm_resume # PCI bus .resume() method
pci_restore_standard_config
pci_set_power_state(PCI_D0) <-- generic PCI
pci_restore_state <-- generic PCI
agp_sis_resume # dev->driver->pm->resume
sis_driver.configure() <-- device-specific
[bhelgaas: commit log]
Signed-off-by: Vaibhav Gupta <[email protected]>
Signed-off-by: Bjorn Helgaas <[email protected]>
---
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
From: Vaibhav Gupta <[email protected]>
Convert via-agp from legacy PCI power management to the generic power
management framework.
Previously, via-agp used legacy PCI power management, and agp_via_suspend()
and agp_via_resume() were responsible for both device-specific things and
generic PCI things:
agp_via_suspend
pci_save_state(pdev) <-- generic PCI
pci_set_power_state(pdev, pci_choose_state(pdev, state)) <-- generic PCI
agp_via_resume
pci_set_power_state(pdev, PCI_D0) <-- generic PCI
pci_restore_state(pdev) <-- generic PCI
via_configure_agp3() <-- device-specific
via_configure() <-- device-specific
With generic power management, the PCI bus PM methods do the generic PCI
things, and the driver needs only the device-specific part, i.e.,
suspend_devices_and_enter
dpm_suspend_start(PMSG_SUSPEND)
pci_pm_suspend # PCI bus .suspend() method
agp_via_suspend <-- not needed at all; removed
suspend_enter
dpm_suspend_noirq(PMSG_SUSPEND)
pci_pm_suspend_noirq # PCI bus .suspend_noirq() method
pci_save_state <-- generic PCI
pci_prepare_to_sleep <-- generic PCI
pci_set_power_state
...
dpm_resume_end(PMSG_RESUME)
pci_pm_resume # PCI bus .resume() method
pci_restore_standard_config
pci_set_power_state(PCI_D0) <-- generic PCI
pci_restore_state <-- generic PCI
agp_via_resume # dev->driver->pm->resume
via_configure_agp3() <-- device-specific
via_configure() <-- device-specific
[bhelgaas: commit log]
Signed-off-by: Vaibhav Gupta <[email protected]>
Signed-off-by: Bjorn Helgaas <[email protected]>
---
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
On Wed, 8 Dec 2021, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <[email protected]>
>
> Vaibhav has converted around 180 drivers to generic power management, and
> over 100 of those conversions have made it upstream. If we can finish off
> the remaining ones, we'll be able to remove quite a bit of ugly legacy code
> from the PCI core.
>
> This is a repost of Vaibhav's patches for AGP. I rebased them to v5.16-rc1
> and updated the commit logs to try to make it easier to verify them.
>
> In the most recent posting here:
>
> https://lore.kernel.org/linux-pci/[email protected]/
>
> my commit log updates were incorrect. This v3 has updates that I believe
> to be correct, but of course I'd be grateful for more corrections.
Hi Bjorn,
Do you want to merge these via your tree?
if so,
Acked-by: Dave Airlie <[email protected]>
Dave.
>
> Vaibhav Gupta (3):
> amd64-agp: convert to generic power management
> sis-agp: convert to generic power management
> via-agp: convert to generic power management
>
> drivers/char/agp/amd64-agp.c | 24 ++++++------------------
> drivers/char/agp/sis-agp.c | 25 ++++++-------------------
> drivers/char/agp/via-agp.c | 25 +++++--------------------
> 3 files changed, 17 insertions(+), 57 deletions(-)
>
>
On Wed, Dec 08, 2021 at 07:55:08PM +0000, Dave Airlie wrote:
> On Wed, 8 Dec 2021, Bjorn Helgaas wrote:
>
> > From: Bjorn Helgaas <[email protected]>
> >
> > Vaibhav has converted around 180 drivers to generic power management, and
> > over 100 of those conversions have made it upstream. If we can finish off
> > the remaining ones, we'll be able to remove quite a bit of ugly legacy code
> > from the PCI core.
> >
> > This is a repost of Vaibhav's patches for AGP. I rebased them to v5.16-rc1
> > and updated the commit logs to try to make it easier to verify them.
> >
> > In the most recent posting here:
> >
> > https://lore.kernel.org/linux-pci/[email protected]/
> >
> > my commit log updates were incorrect. This v3 has updates that I believe
> > to be correct, but of course I'd be grateful for more corrections.
>
> Hi Bjorn,
>
> Do you want to merge these via your tree?
>
> if so,
> Acked-by: Dave Airlie <[email protected]>
Either way is fine; I'll merge them to save you the trouble. I put
them on pci/legacy-pm-removal for v5.17 with your ack. Thanks!
> > Vaibhav Gupta (3):
> > amd64-agp: convert to generic power management
> > sis-agp: convert to generic power management
> > via-agp: convert to generic power management
> >
> > drivers/char/agp/amd64-agp.c | 24 ++++++------------------
> > drivers/char/agp/sis-agp.c | 25 ++++++-------------------
> > drivers/char/agp/via-agp.c | 25 +++++--------------------
> > 3 files changed, 17 insertions(+), 57 deletions(-)
> >
> >