Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing rollback of GT suspend operations on error paths in PM suspend callbacks leaves previously suspended GTs in a powered-down state. -- commit beb79b15731f1c3df49e1a0d1e8cf997804da5af Author: Vinod Govindapillai <[email protected]> Subject: drm/xe/pm: plumb PME capability to runtime suspend handler If the device is PME capable, we don't need to start the HPD polling during the runtime suspend. This patch plumbs the pme_capable flag through the runtime suspend stack. > diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c > index 1488c4dfcc055..c3a53c5f8b330 100644 > --- a/drivers/gpu/drm/xe/xe_pm.c > +++ b/drivers/gpu/drm/xe/xe_pm.c [ ... ] > @@ -639,9 +640,9 @@ int xe_pm_runtime_suspend(struct xe_device *xe) > goto out_resume; ^^^^^^^^^^^^^^^ [Severity: High] This isn't a bug introduced by this patch, but modifying this function highlights incomplete error handling on this path. If suspending a GT fails (e.g., due to a forcewake timeout), the out_resume error path skips resuming the GTs that successfully suspended prior to the failure: out_resume: xe_display_pm_runtime_resume(xe); xe_pxp_pm_resume(xe->pxp); out: xe_rpm_lockmap_release(xe); xe_pm_write_callback_task(xe, NULL); return err; Since the PM core considers the device to be active when the suspend callback returns an error, won't subsequent accesses to the successfully suspended GTs cause hardware hangs or kernel oops? Also, a similar issue appears in the system suspend path in xe_pm_suspend(): for_each_gt(gt, xe, id) { err = xe_gt_suspend(gt); if (err) goto err_display; } Could we ensure previously suspended GTs are properly resumed in these error paths? > } > > - xe_irq_suspend(xe, false); > + xe_irq_suspend(xe, pme_capable); > > - xe_display_pm_runtime_suspend_late(xe); > + xe_display_pm_runtime_suspend_late(xe, pme_capable); > > xe_i2c_pm_suspend(xe); -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
