Linux Kernel Mentee: Remove Legacy Power Management.
The purpose of this patch series is to upgrade power management in video fbdev
drivers. This has been done by upgrading .suspend() and .resume() callbacks.
The upgrade makes sure that the involvement of PCI Core does not change the
order of operations executed in a driver. Thus, does not change its behavior.
In general, drivers with legacy PM, .suspend() and .resume() make use of PCI
helper functions like pci_enable/disable_device_mem(), pci_set_power_state(),
pci_save/restore_state(), pci_enable/disable_device(), etc. to complete
their job.
The conversion requires the removal of those function calls, change the
callbacks' definition accordingly and make use of dev_pm_ops structure.
All patches are compile-tested only.
Test tools:
- Compiler: gcc (GCC) 10.1.0
- allmodconfig build: make -j$(nproc) W=1 all
Vaibhav Gupta (12):
fbdev: gxfb: use generic power management
fbdev: lxfb: use generic power management
fbdev: via-core: use generic power management
fbdev: aty: use generic power management
fbdev: aty128fb: use generic power management
fbdev: nvidia: use generic power management
fbdev: savagefb: use generic power management
fbdev: cyber2000fb: use generic power management
fbdev: i740fb: use generic power management
fbdev: vt8623fb: use generic power management
fbdev: s3fb: use generic power management
fbdev: arkfb: use generic power management
drivers/video/fbdev/arkfb.c | 41 ++++++-------
drivers/video/fbdev/aty/aty128fb.c | 51 ++++++++++------
drivers/video/fbdev/aty/atyfb_base.c | 50 ++++++++++-----
drivers/video/fbdev/cyber2000fb.c | 13 ++--
drivers/video/fbdev/geode/gxfb.h | 5 --
drivers/video/fbdev/geode/gxfb_core.c | 36 ++++++-----
drivers/video/fbdev/geode/lxfb.h | 5 --
drivers/video/fbdev/geode/lxfb_core.c | 37 +++++------
drivers/video/fbdev/geode/lxfb_ops.c | 4 --
drivers/video/fbdev/geode/suspend_gx.c | 4 --
drivers/video/fbdev/i740fb.c | 40 +++++-------
drivers/video/fbdev/nvidia/nvidia.c | 64 +++++++++++---------
drivers/video/fbdev/s3fb.c | 39 +++++-------
drivers/video/fbdev/savage/savagefb_driver.c | 52 ++++++++++------
drivers/video/fbdev/via/via-core.c | 39 +++++-------
drivers/video/fbdev/vt8623fb.c | 41 ++++++-------
include/linux/via-core.h | 2 -
17 files changed, 267 insertions(+), 256 deletions(-)
--
2.28.0
Drivers should do only device-specific jobs. But in general, drivers using
legacy PCI PM framework for .suspend()/.resume() have to manage many PCI
PM-related tasks themselves which can be done by PCI Core itself. This
brings extra load on the driver and it directly calls PCI helper functions
to handle them.
Switch to the new generic framework by updating function signatures and
define a "struct dev_pm_ops" variable to bind PM callbacks. Also, remove
unnecessary calls to the PCI Helper functions along with the legacy
.suspend & .resume bindings.
The i740fb_suspend() is not designed to function in the case of Freeze.
Thus, the code checked for "if (state.event == PM_EVENT_FREEZE....)". This
is because, in the legacy framework, this callback was invoked even in the
event of Freeze. Hence, added the load of unnecessary function-call.
The goal can be achieved by binding the callback with only ".suspend" and
".poweroff" in the "i740fb_pm_ops" const variable. This also avoids the
step of checking "if (state.event == PM_EVENT_FREEZE....)" every time the
callback is invoked.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/video/fbdev/i740fb.c | 40 +++++++++++++++---------------------
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/drivers/video/fbdev/i740fb.c b/drivers/video/fbdev/i740fb.c
index c65ec7386e87..8d7f06fc8a5a 100644
--- a/drivers/video/fbdev/i740fb.c
+++ b/drivers/video/fbdev/i740fb.c
@@ -1175,16 +1175,11 @@ static void i740fb_remove(struct pci_dev *dev)
}
}
-#ifdef CONFIG_PM
-static int i740fb_suspend(struct pci_dev *dev, pm_message_t state)
+static int __maybe_unused i740fb_suspend(struct device *dev)
{
- struct fb_info *info = pci_get_drvdata(dev);
+ struct fb_info *info = dev_get_drvdata(dev);
struct i740fb_par *par = info->par;
- /* don't disable console during hibernation and wakeup from it */
- if (state.event == PM_EVENT_FREEZE || state.event == PM_EVENT_PRETHAW)
- return 0;
-
console_lock();
mutex_lock(&(par->open_lock));
@@ -1197,19 +1192,15 @@ static int i740fb_suspend(struct pci_dev *dev, pm_message_t state)
fb_set_suspend(info, 1);
- pci_save_state(dev);
- pci_disable_device(dev);
- pci_set_power_state(dev, pci_choose_state(dev, state));
-
mutex_unlock(&(par->open_lock));
console_unlock();
return 0;
}
-static int i740fb_resume(struct pci_dev *dev)
+static int __maybe_unused i740fb_resume(struct device *dev)
{
- struct fb_info *info = pci_get_drvdata(dev);
+ struct fb_info *info = dev_get_drvdata(dev);
struct i740fb_par *par = info->par;
console_lock();
@@ -1218,11 +1209,6 @@ static int i740fb_resume(struct pci_dev *dev)
if (par->ref_count == 0)
goto fail;
- pci_set_power_state(dev, PCI_D0);
- pci_restore_state(dev);
- if (pci_enable_device(dev))
- goto fail;
-
i740fb_set_par(info);
fb_set_suspend(info, 0);
@@ -1231,10 +1217,17 @@ static int i740fb_resume(struct pci_dev *dev)
console_unlock();
return 0;
}
-#else
-#define i740fb_suspend NULL
-#define i740fb_resume NULL
-#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops i740fb_pm_ops = {
+#ifdef CONFIG_PM_SLEEP
+ .suspend = i740fb_suspend,
+ .resume = i740fb_resume,
+ .freeze = NULL,
+ .thaw = i740fb_resume,
+ .poweroff = i740fb_suspend,
+ .restore = i740fb_resume,
+#endif /* CONFIG_PM_SLEEP */
+};
#define I740_ID_PCI 0x00d1
#define I740_ID_AGP 0x7800
@@ -1251,8 +1244,7 @@ static struct pci_driver i740fb_driver = {
.id_table = i740fb_id_table,
.probe = i740fb_probe,
.remove = i740fb_remove,
- .suspend = i740fb_suspend,
- .resume = i740fb_resume,
+ .driver.pm = &i740fb_pm_ops,
};
#ifndef MODULE
--
2.28.0
Drivers should do only device-specific jobs. But in general, drivers using
legacy PCI PM framework for .suspend()/.resume() have to manage many PCI
PM-related tasks themselves which can be done by PCI Core itself. This
brings extra load on the driver and it directly calls PCI helper functions
to handle them.
Switch to the new generic framework by updating function signatures and
define a "struct dev_pm_ops" variable to bind PM callbacks. Also, remove
unnecessary calls to the PCI Helper functions along with the legacy
.suspend & .resume bindings.
The vt8623_pci_suspend() is not designed to function in the case of Freeze.
Thus, the code checked for "if (state.event == PM_EVENT_FREEZE....)". This
is because, in the legacy framework, this callback was invoked even in the
event of Freeze. Hence, added the load of unnecessary function-call.
The goal can be achieved by binding the callback with only ".suspend" and
".poweroff" in the "vt8623_pci_pm_ops" const variable. This also avoids the
step of checking "state.event == PM_EVENT_FREEZE" every time the callback
is invoked.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/video/fbdev/vt8623fb.c | 41 ++++++++++++++--------------------
1 file changed, 17 insertions(+), 24 deletions(-)
diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
index 7b3eef1b893f..c488e0117758 100644
--- a/drivers/video/fbdev/vt8623fb.c
+++ b/drivers/video/fbdev/vt8623fb.c
@@ -815,12 +815,11 @@ static void vt8623_pci_remove(struct pci_dev *dev)
}
-#ifdef CONFIG_PM
/* PCI suspend */
-static int vt8623_pci_suspend(struct pci_dev* dev, pm_message_t state)
+static int __maybe_unused vt8623_pci_suspend(struct device *dev)
{
- struct fb_info *info = pci_get_drvdata(dev);
+ struct fb_info *info = dev_get_drvdata(dev);
struct vt8623fb_info *par = info->par;
dev_info(info->device, "suspend\n");
@@ -828,7 +827,7 @@ static int vt8623_pci_suspend(struct pci_dev* dev, pm_message_t state)
console_lock();
mutex_lock(&(par->open_lock));
- if ((state.event == PM_EVENT_FREEZE) || (par->ref_count == 0)) {
+ if (par->ref_count == 0) {
mutex_unlock(&(par->open_lock));
console_unlock();
return 0;
@@ -836,10 +835,6 @@ static int vt8623_pci_suspend(struct pci_dev* dev, pm_message_t state)
fb_set_suspend(info, 1);
- pci_save_state(dev);
- pci_disable_device(dev);
- pci_set_power_state(dev, pci_choose_state(dev, state));
-
mutex_unlock(&(par->open_lock));
console_unlock();
@@ -849,9 +844,9 @@ static int vt8623_pci_suspend(struct pci_dev* dev, pm_message_t state)
/* PCI resume */
-static int vt8623_pci_resume(struct pci_dev* dev)
+static int __maybe_unused vt8623_pci_resume(struct device *dev)
{
- struct fb_info *info = pci_get_drvdata(dev);
+ struct fb_info *info = dev_get_drvdata(dev);
struct vt8623fb_info *par = info->par;
dev_info(info->device, "resume\n");
@@ -862,14 +857,6 @@ static int vt8623_pci_resume(struct pci_dev* dev)
if (par->ref_count == 0)
goto fail;
- pci_set_power_state(dev, PCI_D0);
- pci_restore_state(dev);
-
- if (pci_enable_device(dev))
- goto fail;
-
- pci_set_master(dev);
-
vt8623fb_set_par(info);
fb_set_suspend(info, 0);
@@ -879,10 +866,17 @@ static int vt8623_pci_resume(struct pci_dev* dev)
return 0;
}
-#else
-#define vt8623_pci_suspend NULL
-#define vt8623_pci_resume NULL
-#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops vt8623_pci_pm_ops = {
+#ifdef CONFIG_PM_SLEEP
+ .suspend = vt8623_pci_suspend,
+ .resume = vt8623_pci_resume,
+ .freeze = NULL,
+ .thaw = vt8623_pci_resume,
+ .poweroff = vt8623_pci_suspend,
+ .restore = vt8623_pci_resume,
+#endif /* CONFIG_PM_SLEEP */
+};
/* List of boards that we are trying to support */
@@ -898,8 +892,7 @@ static struct pci_driver vt8623fb_pci_driver = {
.id_table = vt8623_devices,
.probe = vt8623_pci_probe,
.remove = vt8623_pci_remove,
- .suspend = vt8623_pci_suspend,
- .resume = vt8623_pci_resume,
+ .driver.pm = &vt8623_pci_pm_ops,
};
/* Cleanup */
--
2.28.0
Drivers should do only device-specific jobs. But in general, drivers using
legacy PCI PM framework for .suspend()/.resume() have to manage many PCI
PM-related tasks themselves which can be done by PCI Core itself. This
brings extra load on the driver and it directly calls PCI helper functions
to handle them.
Switch to the new generic framework by updating function signatures and
define a "struct dev_pm_ops" variable to bind PM callbacks. Also, remove
unnecessary calls to the PCI Helper functions along with the legacy
.suspend & .resume bindings.
The ark_pci_suspend() is not designed to function in the case of Freeze.
Thus, the code checked for "if (state.event == PM_EVENT_FREEZE....)". This
is because, in the legacy framework, this callback was invoked even in the
event of Freeze. Hence, added the load of unnecessary function-call.
The goal can be achieved by binding the callback with only ".suspend" and
".poweroff" in the "ark_pci_pm_ops" const variable. This also avoids the
step of checking "state.event == PM_EVENT_FREEZE" every time the callback
is invoked.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/video/fbdev/arkfb.c | 41 +++++++++++++++----------------------
1 file changed, 17 insertions(+), 24 deletions(-)
diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index 11ab9a153860..edf169d0816e 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -1085,12 +1085,11 @@ static void ark_pci_remove(struct pci_dev *dev)
}
-#ifdef CONFIG_PM
/* PCI suspend */
-static int ark_pci_suspend (struct pci_dev* dev, pm_message_t state)
+static int __maybe_unused ark_pci_suspend(struct device *dev)
{
- struct fb_info *info = pci_get_drvdata(dev);
+ struct fb_info *info = dev_get_drvdata(dev);
struct arkfb_info *par = info->par;
dev_info(info->device, "suspend\n");
@@ -1098,7 +1097,7 @@ static int ark_pci_suspend (struct pci_dev* dev, pm_message_t state)
console_lock();
mutex_lock(&(par->open_lock));
- if ((state.event == PM_EVENT_FREEZE) || (par->ref_count == 0)) {
+ if (par->ref_count == 0) {
mutex_unlock(&(par->open_lock));
console_unlock();
return 0;
@@ -1106,10 +1105,6 @@ static int ark_pci_suspend (struct pci_dev* dev, pm_message_t state)
fb_set_suspend(info, 1);
- pci_save_state(dev);
- pci_disable_device(dev);
- pci_set_power_state(dev, pci_choose_state(dev, state));
-
mutex_unlock(&(par->open_lock));
console_unlock();
@@ -1119,9 +1114,9 @@ static int ark_pci_suspend (struct pci_dev* dev, pm_message_t state)
/* PCI resume */
-static int ark_pci_resume (struct pci_dev* dev)
+static int __maybe_unused ark_pci_resume(struct device *dev)
{
- struct fb_info *info = pci_get_drvdata(dev);
+ struct fb_info *info = dev_get_drvdata(dev);
struct arkfb_info *par = info->par;
dev_info(info->device, "resume\n");
@@ -1132,14 +1127,6 @@ static int ark_pci_resume (struct pci_dev* dev)
if (par->ref_count == 0)
goto fail;
- pci_set_power_state(dev, PCI_D0);
- pci_restore_state(dev);
-
- if (pci_enable_device(dev))
- goto fail;
-
- pci_set_master(dev);
-
arkfb_set_par(info);
fb_set_suspend(info, 0);
@@ -1148,10 +1135,17 @@ static int ark_pci_resume (struct pci_dev* dev)
console_unlock();
return 0;
}
-#else
-#define ark_pci_suspend NULL
-#define ark_pci_resume NULL
-#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops ark_pci_pm_ops = {
+#ifdef CONFIG_PM_SLEEP
+ .suspend = ark_pci_suspend,
+ .resume = ark_pci_resume,
+ .freeze = NULL,
+ .thaw = ark_pci_resume,
+ .poweroff = ark_pci_suspend,
+ .restore = ark_pci_resume,
+#endif
+};
/* List of boards that we are trying to support */
@@ -1168,8 +1162,7 @@ static struct pci_driver arkfb_pci_driver = {
.id_table = ark_devices,
.probe = ark_pci_probe,
.remove = ark_pci_remove,
- .suspend = ark_pci_suspend,
- .resume = ark_pci_resume,
+ .driver.pm = &ark_pci_pm_ops,
};
/* Cleanup */
--
2.28.0
Drivers should do only device-specific jobs. But in general, drivers using
legacy PCI PM framework for .suspend()/.resume() have to manage many PCI
PM-related tasks themselves which can be done by PCI Core itself. This
brings extra load on the driver and it directly calls PCI helper functions
to handle them.
Switch to the new generic framework by updating function signatures and
define a "struct dev_pm_ops" variable to bind PM callbacks. Also, remove
unnecessary calls to the PCI Helper functions along with the legacy
.suspend & .resume bindings.
The s3_pci_suspend() is not designed to function in the case of Freeze.
Thus, the code checked for "if (state.event == PM_EVENT_FREEZE....)". This
is because, in the legacy framework, this callback was invoked even in the
event of Freeze. Hence, added the load of unnecessary function-call.
The goal can be achieved by binding the callback with only ".suspend" and
".poweroff" in the "s3_pci_pm_ops" const variable. This also avoids the
step of checking "state.event == PM_EVENT_FREEZE" every time the callback
is invoked.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/video/fbdev/s3fb.c | 39 ++++++++++++++++----------------------
1 file changed, 16 insertions(+), 23 deletions(-)
diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
index 60c424fae988..5c74253e7b2c 100644
--- a/drivers/video/fbdev/s3fb.c
+++ b/drivers/video/fbdev/s3fb.c
@@ -1410,9 +1410,9 @@ static void s3_pci_remove(struct pci_dev *dev)
/* PCI suspend */
-static int s3_pci_suspend(struct pci_dev* dev, pm_message_t state)
+static int __maybe_unused s3_pci_suspend(struct device *dev)
{
- struct fb_info *info = pci_get_drvdata(dev);
+ struct fb_info *info = dev_get_drvdata(dev);
struct s3fb_info *par = info->par;
dev_info(info->device, "suspend\n");
@@ -1420,7 +1420,7 @@ static int s3_pci_suspend(struct pci_dev* dev, pm_message_t state)
console_lock();
mutex_lock(&(par->open_lock));
- if ((state.event == PM_EVENT_FREEZE) || (par->ref_count == 0)) {
+ if (par->ref_count == 0) {
mutex_unlock(&(par->open_lock));
console_unlock();
return 0;
@@ -1428,10 +1428,6 @@ static int s3_pci_suspend(struct pci_dev* dev, pm_message_t state)
fb_set_suspend(info, 1);
- pci_save_state(dev);
- pci_disable_device(dev);
- pci_set_power_state(dev, pci_choose_state(dev, state));
-
mutex_unlock(&(par->open_lock));
console_unlock();
@@ -1441,11 +1437,10 @@ static int s3_pci_suspend(struct pci_dev* dev, pm_message_t state)
/* PCI resume */
-static int s3_pci_resume(struct pci_dev* dev)
+static int __maybe_unused s3_pci_resume(struct device *dev)
{
- struct fb_info *info = pci_get_drvdata(dev);
+ struct fb_info *info = dev_get_drvdata(dev);
struct s3fb_info *par = info->par;
- int err;
dev_info(info->device, "resume\n");
@@ -1458,17 +1453,6 @@ static int s3_pci_resume(struct pci_dev* dev)
return 0;
}
- pci_set_power_state(dev, PCI_D0);
- pci_restore_state(dev);
- err = pci_enable_device(dev);
- if (err) {
- mutex_unlock(&(par->open_lock));
- console_unlock();
- dev_err(info->device, "error %d enabling device for resume\n", err);
- return err;
- }
- pci_set_master(dev);
-
s3fb_set_par(info);
fb_set_suspend(info, 0);
@@ -1478,6 +1462,16 @@ static int s3_pci_resume(struct pci_dev* dev)
return 0;
}
+static const struct dev_pm_ops s3_pci_pm_ops = {
+#ifdef CONFIG_PM_SLEEP
+ .suspend = s3_pci_suspend,
+ .resume = s3_pci_resume,
+ .freeze = NULL,
+ .thaw = s3_pci_resume,
+ .poweroff = s3_pci_suspend,
+ .restore = s3_pci_resume,
+#endif
+};
/* List of boards that we are trying to support */
@@ -1510,8 +1504,7 @@ static struct pci_driver s3fb_pci_driver = {
.id_table = s3_devices,
.probe = s3_pci_probe,
.remove = s3_pci_remove,
- .suspend = s3_pci_suspend,
- .resume = s3_pci_resume,
+ .driver.pm = &s3_pci_pm_ops,
};
/* Parse user specified options */
--
2.28.0
Switch to the new generic framework by updating function signatures and
define a "struct dev_pm_ops" variable to bind PM callbacks. This way we can
remove the legacy .suspend & .resume bindings from "lxfb_driver".
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/video/fbdev/cyber2000fb.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/cyber2000fb.c b/drivers/video/fbdev/cyber2000fb.c
index 42d37bed518a..d45355b9a58c 100644
--- a/drivers/video/fbdev/cyber2000fb.c
+++ b/drivers/video/fbdev/cyber2000fb.c
@@ -1810,7 +1810,7 @@ static void cyberpro_pci_remove(struct pci_dev *dev)
}
}
-static int cyberpro_pci_suspend(struct pci_dev *dev, pm_message_t state)
+static int __maybe_unused cyberpro_pci_suspend(struct device *dev)
{
return 0;
}
@@ -1818,9 +1818,9 @@ static int cyberpro_pci_suspend(struct pci_dev *dev, pm_message_t state)
/*
* Re-initialise the CyberPro hardware
*/
-static int cyberpro_pci_resume(struct pci_dev *dev)
+static int __maybe_unused cyberpro_pci_resume(struct device *dev)
{
- struct cfb_info *cfb = pci_get_drvdata(dev);
+ struct cfb_info *cfb = dev_get_drvdata(dev);
if (cfb) {
cyberpro_pci_enable_mmio(cfb);
@@ -1846,12 +1846,15 @@ static struct pci_device_id cyberpro_pci_table[] = {
MODULE_DEVICE_TABLE(pci, cyberpro_pci_table);
+static SIMPLE_DEV_PM_OPS(cyberpro_pci_pm_ops,
+ cyberpro_pci_suspend,
+ cyberpro_pci_resume);
+
static struct pci_driver cyberpro_driver = {
.name = "CyberPro",
.probe = cyberpro_pci_probe,
.remove = cyberpro_pci_remove,
- .suspend = cyberpro_pci_suspend,
- .resume = cyberpro_pci_resume,
+ .driver.pm = &cyberpro_pci_pm_ops,
.id_table = cyberpro_pci_table
};
--
2.28.0
Drivers should do only device-specific jobs. But in general, drivers using
legacy PCI PM framework for .suspend()/.resume() have to manage many PCI
PM-related tasks themselves which can be done by PCI Core itself. This
brings extra load on the driver and it directly calls PCI helper functions
to handle them.
Switch to the new generic framework by updating function signatures and
define a "struct dev_pm_ops" variable to bind PM callbacks. Also, remove
unnecessary calls to the PCI Helper functions along with the legacy
.suspend & .resume bindings.
Now,
- savagefb_suspend() had a "pm_message_t" type parameter as per legacy
PCI PM framework that got deprecated in generic.
- Rename the callback as savagefb_suspend_late() and preserve the
parameter.
- Define 3 new callbacks as:
* savagefb_suspend()
* savagefb_freeze()
* savagefb_hibernate()
which in turn call savagefb_suspend_late() by passing appropriate value
for "pm_message_t" type parameter.
- Bind the callbacks in "struct dev_pm_ops" type variable
"savagefb_pm_ops".
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/video/fbdev/savage/savagefb_driver.c | 52 ++++++++++++--------
1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/drivers/video/fbdev/savage/savagefb_driver.c b/drivers/video/fbdev/savage/savagefb_driver.c
index 3c8ae87f0ea7..d6aae759e90f 100644
--- a/drivers/video/fbdev/savage/savagefb_driver.c
+++ b/drivers/video/fbdev/savage/savagefb_driver.c
@@ -2346,9 +2346,9 @@ static void savagefb_remove(struct pci_dev *dev)
}
}
-static int savagefb_suspend(struct pci_dev *dev, pm_message_t mesg)
+static int savagefb_suspend_late(struct device *dev, pm_message_t mesg)
{
- struct fb_info *info = pci_get_drvdata(dev);
+ struct fb_info *info = dev_get_drvdata(dev);
struct savagefb_par *par = info->par;
DBG("savagefb_suspend");
@@ -2356,7 +2356,7 @@ static int savagefb_suspend(struct pci_dev *dev, pm_message_t mesg)
if (mesg.event == PM_EVENT_PRETHAW)
mesg.event = PM_EVENT_FREEZE;
par->pm_state = mesg.event;
- dev->dev.power.power_state = mesg;
+ dev->power.power_state = mesg;
/*
* For PM_EVENT_FREEZE, do not power down so the console
@@ -2374,17 +2374,29 @@ static int savagefb_suspend(struct pci_dev *dev, pm_message_t mesg)
savagefb_blank(FB_BLANK_POWERDOWN, info);
savage_set_default_par(par, &par->save);
savage_disable_mmio(par);
- pci_save_state(dev);
- pci_disable_device(dev);
- pci_set_power_state(dev, pci_choose_state(dev, mesg));
console_unlock();
return 0;
}
-static int savagefb_resume(struct pci_dev* dev)
+static int __maybe_unused savagefb_suspend(struct device *dev)
{
- struct fb_info *info = pci_get_drvdata(dev);
+ return savagefb_suspend_late(dev, PMSG_SUSPEND);
+}
+
+static int __maybe_unused savagefb_hibernate(struct device *dev)
+{
+ return savagefb_suspend_late(dev, PMSG_HIBERNATE);
+}
+
+static int __maybe_unused savagefb_freeze(struct device *dev)
+{
+ return savagefb_suspend_late(dev, PMSG_FREEZE);
+}
+
+static int __maybe_unused savagefb_resume(struct device *dev)
+{
+ struct fb_info *info = dev_get_drvdata(dev);
struct savagefb_par *par = info->par;
int cur_state = par->pm_state;
@@ -2396,20 +2408,11 @@ static int savagefb_resume(struct pci_dev* dev)
* The adapter was not powered down coming back from a
* PM_EVENT_FREEZE.
*/
- if (cur_state == PM_EVENT_FREEZE) {
- pci_set_power_state(dev, PCI_D0);
+ if (cur_state == PM_EVENT_FREEZE)
return 0;
- }
console_lock();
- pci_set_power_state(dev, PCI_D0);
- pci_restore_state(dev);
-
- if (pci_enable_device(dev))
- DBG("err");
-
- pci_set_master(dev);
savage_enable_mmio(par);
savage_init_hw(par);
savagefb_set_par(info);
@@ -2420,6 +2423,16 @@ static int savagefb_resume(struct pci_dev* dev)
return 0;
}
+static const struct dev_pm_ops savagefb_pm_ops = {
+#ifdef CONFIG_PM_SLEEP
+ .suspend = savagefb_suspend,
+ .resume = savagefb_resume,
+ .freeze = savagefb_freeze,
+ .thaw = savagefb_resume,
+ .poweroff = savagefb_hibernate,
+ .restore = savagefb_resume,
+#endif
+};
static const struct pci_device_id savagefb_devices[] = {
{PCI_VENDOR_ID_S3, PCI_CHIP_SUPSAV_MX128,
@@ -2500,8 +2513,7 @@ static struct pci_driver savagefb_driver = {
.name = "savagefb",
.id_table = savagefb_devices,
.probe = savagefb_probe,
- .suspend = savagefb_suspend,
- .resume = savagefb_resume,
+ .driver.pm = &savagefb_pm_ops,
.remove = savagefb_remove,
};
--
2.28.0
On 8/19/20 8:56 PM, Vaibhav Gupta wrote:
> Linux Kernel Mentee: Remove Legacy Power Management.
>
> The purpose of this patch series is to upgrade power management in video fbdev
> drivers. This has been done by upgrading .suspend() and .resume() callbacks.
>
> The upgrade makes sure that the involvement of PCI Core does not change the
> order of operations executed in a driver. Thus, does not change its behavior.
>
> In general, drivers with legacy PM, .suspend() and .resume() make use of PCI
> helper functions like pci_enable/disable_device_mem(), pci_set_power_state(),
> pci_save/restore_state(), pci_enable/disable_device(), etc. to complete
> their job.
>
> The conversion requires the removal of those function calls, change the
> callbacks' definition accordingly and make use of dev_pm_ops structure.
>
> All patches are compile-tested only.
>
> Test tools:
> - Compiler: gcc (GCC) 10.1.0
> - allmodconfig build: make -j$(nproc) W=1 all
>
> Vaibhav Gupta (12):
> fbdev: gxfb: use generic power management
> fbdev: lxfb: use generic power management
> fbdev: via-core: use generic power management
> fbdev: aty: use generic power management
> fbdev: aty128fb: use generic power management
> fbdev: nvidia: use generic power management
> fbdev: savagefb: use generic power management
> fbdev: cyber2000fb: use generic power management
> fbdev: i740fb: use generic power management
> fbdev: vt8623fb: use generic power management
> fbdev: s3fb: use generic power management
> fbdev: arkfb: use generic power management
>
> drivers/video/fbdev/arkfb.c | 41 ++++++-------
> drivers/video/fbdev/aty/aty128fb.c | 51 ++++++++++------
> drivers/video/fbdev/aty/atyfb_base.c | 50 ++++++++++-----
> drivers/video/fbdev/cyber2000fb.c | 13 ++--
> drivers/video/fbdev/geode/gxfb.h | 5 --
> drivers/video/fbdev/geode/gxfb_core.c | 36 ++++++-----
> drivers/video/fbdev/geode/lxfb.h | 5 --
> drivers/video/fbdev/geode/lxfb_core.c | 37 +++++------
> drivers/video/fbdev/geode/lxfb_ops.c | 4 --
> drivers/video/fbdev/geode/suspend_gx.c | 4 --
> drivers/video/fbdev/i740fb.c | 40 +++++-------
> drivers/video/fbdev/nvidia/nvidia.c | 64 +++++++++++---------
> drivers/video/fbdev/s3fb.c | 39 +++++-------
> drivers/video/fbdev/savage/savagefb_driver.c | 52 ++++++++++------
> drivers/video/fbdev/via/via-core.c | 39 +++++-------
> drivers/video/fbdev/vt8623fb.c | 41 ++++++-------
> include/linux/via-core.h | 2 -
> 17 files changed, 267 insertions(+), 256 deletions(-)
Applied to drm-misc-next tree, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics