* [PATCH 1/5] drm/sysfb: corebootdrm: Support power management
2026-04-08 12:03 [PATCH 0/5] drm/sysfb: Support basic power management Thomas Zimmermann
@ 2026-04-08 12:03 ` Thomas Zimmermann
2026-04-12 2:33 ` Claude review: " Claude Code Review Bot
2026-04-08 12:03 ` [PATCH 2/5] drm/sysfb: efidrm: " Thomas Zimmermann
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2026-04-08 12:03 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, Thomas Zimmermann
Set PM ops for the corebootdrm driver. Suspend and resume the DRM
state on systems that support it.
Many systems lose the hardware's framebuffer settings on suspend,
hence resuming doesn't work there. Yet some systems, most notably
emulators, keep the hardware state across suspend/resume cycles.
There, DRM's suspend and resume helpers bring back the display on
resume.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/sysfb/corebootdrm.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/sysfb/corebootdrm.c b/drivers/gpu/drm/sysfb/corebootdrm.c
index 5dc6f3c76f7b..853da9a609df 100644
--- a/drivers/gpu/drm/sysfb/corebootdrm.c
+++ b/drivers/gpu/drm/sysfb/corebootdrm.c
@@ -4,6 +4,7 @@
#include <linux/coreboot.h>
#include <linux/minmax.h>
#include <linux/platform_device.h>
+#include <linux/pm.h>
#include <drm/clients/drm_client_setup.h>
#include <drm/drm_atomic.h>
@@ -18,6 +19,7 @@
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_gem_shmem_helper.h>
#include <drm/drm_managed.h>
+#include <drm/drm_modeset_helper.h>
#include <drm/drm_modeset_helper_vtables.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
@@ -286,6 +288,24 @@ static struct drm_driver corebootdrm_drm_driver = {
* Coreboot driver
*/
+static int corebootdrm_pm_suspend(struct device *dev)
+{
+ struct corebootdrm_device *cdev = dev_get_drvdata(dev);
+ struct drm_device *drm = &cdev->sysfb.dev;
+
+ return drm_mode_config_helper_suspend(drm);
+}
+
+static int corebootdrm_pm_resume(struct device *dev)
+{
+ struct corebootdrm_device *cdev = dev_get_drvdata(dev);
+ struct drm_device *drm = &cdev->sysfb.dev;
+
+ return drm_mode_config_helper_resume(drm);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(corebootdrm_pm_ops, corebootdrm_pm_suspend, corebootdrm_pm_resume);
+
static int corebootdrm_probe(struct platform_device *pdev)
{
const struct lb_framebuffer *fb = dev_get_platdata(&pdev->dev);
@@ -423,6 +443,7 @@ static void corebootdrm_remove(struct platform_device *pdev)
static struct platform_driver corebootdrm_platform_driver = {
.driver = {
.name = "coreboot-framebuffer",
+ .pm = pm_sleep_ptr(&corebootdrm_pm_ops),
},
.probe = corebootdrm_probe,
.remove = corebootdrm_remove,
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Claude review: drm/sysfb: corebootdrm: Support power management
2026-04-08 12:03 ` [PATCH 1/5] drm/sysfb: corebootdrm: Support " Thomas Zimmermann
@ 2026-04-12 2:33 ` Claude Code Review Bot
0 siblings, 0 replies; 12+ messages in thread
From: Claude Code Review Bot @ 2026-04-12 2:33 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correct.** The probe stores `cdev` (a `struct corebootdrm_device *`) as drvdata:
```c
platform_set_drvdata(pdev, cdev); /* corebootdrm.c:329 */
```
The PM callbacks correctly retrieve the same type and navigate to the drm_device:
```c
struct corebootdrm_device *cdev = dev_get_drvdata(dev);
struct drm_device *drm = &cdev->sysfb.dev;
```
Includes are correct: `<linux/pm.h>` for PM macros, `<drm/drm_modeset_helper.h>` for `drm_mode_config_helper_suspend/resume`.
No issues.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/5] drm/sysfb: efidrm: Support power management
2026-04-08 12:03 [PATCH 0/5] drm/sysfb: Support basic power management Thomas Zimmermann
2026-04-08 12:03 ` [PATCH 1/5] drm/sysfb: corebootdrm: Support " Thomas Zimmermann
@ 2026-04-08 12:03 ` Thomas Zimmermann
2026-04-12 2:33 ` Claude review: " Claude Code Review Bot
2026-04-08 12:03 ` [PATCH 3/5] drm/sysfb: ofdrm: " Thomas Zimmermann
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2026-04-08 12:03 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, Thomas Zimmermann
Set PM ops for the efidrm driver. Suspend and resume the DRM state
on systems that support it.
Many systems lose the hardware's framebuffer settings on suspend,
hence resuming doesn't work there. Yet some systems, most notably
emulators, keep the hardware state across suspend/resume cycles.
There, DRM's suspend and resume helpers bring back the display on
resume.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/sysfb/efidrm.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
index a335c94a7bd7..1a1e36700976 100644
--- a/drivers/gpu/drm/sysfb/efidrm.c
+++ b/drivers/gpu/drm/sysfb/efidrm.c
@@ -5,6 +5,7 @@
#include <linux/limits.h>
#include <linux/platform_device.h>
#include <linux/sysfb.h>
+#include <linux/pm.h>
#include <drm/clients/drm_client_setup.h>
#include <drm/drm_atomic.h>
@@ -20,6 +21,7 @@
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_gem_shmem_helper.h>
#include <drm/drm_managed.h>
+#include <drm/drm_modeset_helper.h>
#include <drm/drm_modeset_helper_vtables.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
@@ -371,6 +373,22 @@ static struct drm_driver efidrm_driver = {
* Platform driver
*/
+static int efidrm_pm_suspend(struct device *dev)
+{
+ struct drm_device *drm = dev_get_drvdata(dev);
+
+ return drm_mode_config_helper_suspend(drm);
+}
+
+static int efidrm_pm_resume(struct device *dev)
+{
+ struct drm_device *drm = dev_get_drvdata(dev);
+
+ return drm_mode_config_helper_resume(drm);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(efidrm_pm_ops, efidrm_pm_suspend, efidrm_pm_resume);
+
static int efidrm_probe(struct platform_device *pdev)
{
struct efidrm_device *efi;
@@ -403,6 +421,7 @@ static void efidrm_remove(struct platform_device *pdev)
static struct platform_driver efidrm_platform_driver = {
.driver = {
.name = "efi-framebuffer",
+ .pm = pm_sleep_ptr(&efidrm_pm_ops),
},
.probe = efidrm_probe,
.remove = efidrm_remove,
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/5] drm/sysfb: ofdrm: Support power management
2026-04-08 12:03 [PATCH 0/5] drm/sysfb: Support basic power management Thomas Zimmermann
2026-04-08 12:03 ` [PATCH 1/5] drm/sysfb: corebootdrm: Support " Thomas Zimmermann
2026-04-08 12:03 ` [PATCH 2/5] drm/sysfb: efidrm: " Thomas Zimmermann
@ 2026-04-08 12:03 ` Thomas Zimmermann
2026-04-12 2:33 ` Claude review: " Claude Code Review Bot
2026-04-08 12:03 ` [PATCH 4/5] drm/sysfb: simpledrm: " Thomas Zimmermann
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2026-04-08 12:03 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, Thomas Zimmermann
Set PM ops for the ofdrm driver. Suspend and resume the DRM state
on systems that support it.
Many systems lose the hardware's framebuffer settings on suspend,
hence resuming doesn't work there. Yet some systems, most notably
emulators, keep the hardware state across suspend/resume cycles.
There, DRM's suspend and resume helpers bring back the display on
resume.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/sysfb/ofdrm.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/sysfb/ofdrm.c b/drivers/gpu/drm/sysfb/ofdrm.c
index d38ba70f4e0d..8822ebcbc081 100644
--- a/drivers/gpu/drm/sysfb/ofdrm.c
+++ b/drivers/gpu/drm/sysfb/ofdrm.c
@@ -4,6 +4,7 @@
#include <linux/of_address.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
+#include <linux/pm.h>
#include <drm/clients/drm_client_setup.h>
#include <drm/drm_atomic.h>
@@ -20,6 +21,7 @@
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_gem_shmem_helper.h>
#include <drm/drm_managed.h>
+#include <drm/drm_modeset_helper.h>
#include <drm/drm_modeset_helper_vtables.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
@@ -1095,6 +1097,22 @@ static struct drm_driver ofdrm_driver = {
* Platform driver
*/
+static int ofdrm_pm_suspend(struct device *dev)
+{
+ struct drm_device *drm = dev_get_drvdata(dev);
+
+ return drm_mode_config_helper_suspend(drm);
+}
+
+static int ofdrm_pm_resume(struct device *dev)
+{
+ struct drm_device *drm = dev_get_drvdata(dev);
+
+ return drm_mode_config_helper_resume(drm);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(ofdrm_pm_ops, ofdrm_pm_suspend, ofdrm_pm_resume);
+
static int ofdrm_probe(struct platform_device *pdev)
{
struct ofdrm_device *odev;
@@ -1134,6 +1152,7 @@ static struct platform_driver ofdrm_platform_driver = {
.driver = {
.name = "of-display",
.of_match_table = ofdrm_of_match_display,
+ .pm = pm_sleep_ptr(&ofdrm_pm_ops),
},
.probe = ofdrm_probe,
.remove = ofdrm_remove,
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/5] drm/sysfb: simpledrm: Support power management
2026-04-08 12:03 [PATCH 0/5] drm/sysfb: Support basic power management Thomas Zimmermann
` (2 preceding siblings ...)
2026-04-08 12:03 ` [PATCH 3/5] drm/sysfb: ofdrm: " Thomas Zimmermann
@ 2026-04-08 12:03 ` Thomas Zimmermann
2026-04-12 2:33 ` Claude review: " Claude Code Review Bot
2026-04-08 12:03 ` [PATCH 5/5] drm/sysfb: vesadrm: " Thomas Zimmermann
2026-04-12 2:33 ` Claude review: drm/sysfb: Support basic " Claude Code Review Bot
5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2026-04-08 12:03 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, Thomas Zimmermann
Set PM ops for the simpledrm driver. Suspend and resume the DRM
state on systems that support it.
Many systems lose the hardware's framebuffer settings on suspend,
hence resuming doesn't work there. Yet some systems, most notably
emulators, keep the hardware state across suspend/resume cycles.
There, DRM's suspend and resume helpers bring back the display on
resume.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/sysfb/simpledrm.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/gpu/drm/sysfb/simpledrm.c b/drivers/gpu/drm/sysfb/simpledrm.c
index 7a95d2dacd9d..fc168920f2c6 100644
--- a/drivers/gpu/drm/sysfb/simpledrm.c
+++ b/drivers/gpu/drm/sysfb/simpledrm.c
@@ -8,6 +8,7 @@
#include <linux/of_reserved_mem.h>
#include <linux/platform_data/simplefb.h>
#include <linux/platform_device.h>
+#include <linux/pm.h>
#include <linux/pm_domain.h>
#include <linux/regulator/consumer.h>
@@ -24,6 +25,7 @@
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_gem_shmem_helper.h>
#include <drm/drm_managed.h>
+#include <drm/drm_modeset_helper.h>
#include <drm/drm_modeset_helper_vtables.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
@@ -834,6 +836,24 @@ static struct drm_driver simpledrm_driver = {
* Platform driver
*/
+static int simpledrm_pm_suspend(struct device *dev)
+{
+ struct simpledrm_device *sdev = dev_get_drvdata(dev);
+ struct drm_device *drm = &sdev->sysfb.dev;
+
+ return drm_mode_config_helper_suspend(drm);
+}
+
+static int simpledrm_pm_resume(struct device *dev)
+{
+ struct simpledrm_device *sdev = dev_get_drvdata(dev);
+ struct drm_device *drm = &sdev->sysfb.dev;
+
+ return drm_mode_config_helper_resume(drm);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(simpledrm_pm_ops, simpledrm_pm_suspend, simpledrm_pm_resume);
+
static int simpledrm_probe(struct platform_device *pdev)
{
struct simpledrm_device *sdev;
@@ -874,6 +894,7 @@ static struct platform_driver simpledrm_platform_driver = {
.driver = {
.name = "simple-framebuffer", /* connect to sysfb */
.of_match_table = simpledrm_of_match_table,
+ .pm = pm_sleep_ptr(&simpledrm_pm_ops),
},
.probe = simpledrm_probe,
.remove = simpledrm_remove,
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Claude review: drm/sysfb: simpledrm: Support power management
2026-04-08 12:03 ` [PATCH 4/5] drm/sysfb: simpledrm: " Thomas Zimmermann
@ 2026-04-12 2:33 ` Claude Code Review Bot
0 siblings, 0 replies; 12+ messages in thread
From: Claude Code Review Bot @ 2026-04-12 2:33 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correct.** Same pattern as corebootdrm -- probe stores the outer struct:
```c
platform_set_drvdata(pdev, sdev); /* simpledrm.c:621 */
```
PM callbacks correctly cast back:
```c
struct simpledrm_device *sdev = dev_get_drvdata(dev);
struct drm_device *drm = &sdev->sysfb.dev;
```
No issues.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/5] drm/sysfb: vesadrm: Support power management
2026-04-08 12:03 [PATCH 0/5] drm/sysfb: Support basic power management Thomas Zimmermann
` (3 preceding siblings ...)
2026-04-08 12:03 ` [PATCH 4/5] drm/sysfb: simpledrm: " Thomas Zimmermann
@ 2026-04-08 12:03 ` Thomas Zimmermann
2026-04-12 2:33 ` Claude review: " Claude Code Review Bot
2026-04-12 2:33 ` Claude review: drm/sysfb: Support basic " Claude Code Review Bot
5 siblings, 1 reply; 12+ messages in thread
From: Thomas Zimmermann @ 2026-04-08 12:03 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, airlied, simona
Cc: dri-devel, Thomas Zimmermann
Set PM ops for the vesadrm driver. Suspend and resume the DRM state
on systems that support it.
Many systems lose the hardware's framebuffer settings on suspend,
hence resuming doesn't work there. Yet some systems, most notably
emulators, keep the hardware state across suspend/resume cycles.
There, DRM's suspend and resume helpers bring back the display on
resume.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/gpu/drm/sysfb/vesadrm.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/sysfb/vesadrm.c b/drivers/gpu/drm/sysfb/vesadrm.c
index 4e00113e5c77..255b90a200b0 100644
--- a/drivers/gpu/drm/sysfb/vesadrm.c
+++ b/drivers/gpu/drm/sysfb/vesadrm.c
@@ -5,6 +5,7 @@
#include <linux/limits.h>
#include <linux/platform_device.h>
#include <linux/sysfb.h>
+#include <linux/pm.h>
#include <drm/clients/drm_client_setup.h>
#include <drm/drm_atomic.h>
@@ -21,6 +22,7 @@
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_gem_shmem_helper.h>
#include <drm/drm_managed.h>
+#include <drm/drm_modeset_helper.h>
#include <drm/drm_modeset_helper_vtables.h>
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
@@ -617,6 +619,22 @@ static struct drm_driver vesadrm_driver = {
* Platform driver
*/
+static int vesadrm_pm_suspend(struct device *dev)
+{
+ struct drm_device *drm = dev_get_drvdata(dev);
+
+ return drm_mode_config_helper_suspend(drm);
+}
+
+static int vesadrm_pm_resume(struct device *dev)
+{
+ struct drm_device *drm = dev_get_drvdata(dev);
+
+ return drm_mode_config_helper_resume(drm);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(vesadrm_pm_ops, vesadrm_pm_suspend, vesadrm_pm_resume);
+
static int vesadrm_probe(struct platform_device *pdev)
{
struct vesadrm_device *vesa;
@@ -649,6 +667,7 @@ static void vesadrm_remove(struct platform_device *pdev)
static struct platform_driver vesadrm_platform_driver = {
.driver = {
.name = "vesa-framebuffer",
+ .pm = pm_sleep_ptr(&vesadrm_pm_ops),
},
.probe = vesadrm_probe,
.remove = vesadrm_remove,
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Claude review: drm/sysfb: vesadrm: Support power management
2026-04-08 12:03 ` [PATCH 5/5] drm/sysfb: vesadrm: " Thomas Zimmermann
@ 2026-04-12 2:33 ` Claude Code Review Bot
0 siblings, 0 replies; 12+ messages in thread
From: Claude Code Review Bot @ 2026-04-12 2:33 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correct.** Same pattern as efidrm/ofdrm -- probe stores `dev`:
```c
platform_set_drvdata(pdev, dev); /* vesadrm.c:438 */
```
PM callbacks match.
No issues. This is also the one Thomas notes was tested with qemu's bochs device.
---
**Summary:** All five patches are correct and consistent with each driver's existing drvdata convention. The only potential improvement would be consolidating the identical PM logic into a shared helper, but that would require first unifying the drvdata convention across the five drivers. Reviewed-by can be given for the series as-is.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 12+ messages in thread
* Claude review: drm/sysfb: Support basic power management
2026-04-08 12:03 [PATCH 0/5] drm/sysfb: Support basic power management Thomas Zimmermann
` (4 preceding siblings ...)
2026-04-08 12:03 ` [PATCH 5/5] drm/sysfb: vesadrm: " Thomas Zimmermann
@ 2026-04-12 2:33 ` Claude Code Review Bot
5 siblings, 0 replies; 12+ messages in thread
From: Claude Code Review Bot @ 2026-04-12 2:33 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/sysfb: Support basic power management
Author: Thomas Zimmermann <tzimmermann@suse.de>
Patches: 6
Reviewed: 2026-04-12T12:33:54.714983
---
This is a clean, straightforward 5-patch series by Thomas Zimmermann adding basic PM (suspend/resume) support to all five sysfb DRM drivers: corebootdrm, efidrm, ofdrm, simpledrm, and vesadrm.
Each patch follows an identical pattern: add PM callbacks that call `drm_mode_config_helper_suspend()`/`drm_mode_config_helper_resume()`, wire them up via `DEFINE_SIMPLE_DEV_PM_OPS` and `pm_sleep_ptr()`. The approach is correct -- all five drivers use `DRIVER_ATOMIC` and set up `mode_config_funcs` with `drm_atomic_helper_commit`, which is required for the config helper suspend/resume to work.
I verified that `dev_get_drvdata()` retrieval in each PM callback matches what each driver's probe function stores via `platform_set_drvdata()`. The series is correct.
**One design observation:** All five patches are nearly identical boilerplate. Since every sysfb driver embeds a `struct drm_sysfb_device` (which embeds `struct drm_device`), shared PM ops could be factored into `drm_sysfb_helper.h`, avoiding ~20 lines of duplication in each driver. The blocker is that drvdata is inconsistent across the drivers: corebootdrm and simpledrm store their outer device struct, while efidrm, ofdrm, and vesadrm store the `struct drm_device *` directly. Standardizing the drvdata convention would enable a single shared `drm_sysfb_pm_ops`. This is a nice-to-have, not a blocker.
**Verdict:** Series looks good.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 12+ messages in thread