Claude review: drm/sysfb: Support basic power management

[Claude Code Review Bot <claude-review@example.com>]

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