From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/sysfb: Support basic power management Date: Sun, 12 Apr 2026 12:33:54 +1000 Message-ID: In-Reply-To: <20260408120722.328769-1-tzimmermann@suse.de> References: <20260408120722.328769-1-tzimmermann@suse.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Overall Series Review Subject: drm/sysfb: Support basic power management Author: Thomas Zimmermann 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