Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Ignoring the return value of pm_runtime_get_sync() can lead to unclocked register accesses and system crashes. - [High] Driver teardown races with in-flight GPU jobs, leading to unhandled hardware faults. -- commit 75e6d8fc2d8a39088a19c0381af15b43a9a4c041 Author: Adrián Larumbe <[email protected]> drm/panfrost: Fix PM refcnt and autosuspend issues at device probe/remove During device probe(), failure to do a PM get() will leave the usage_count set to 0, which is the value assigned at device creation time. That means when the autosuspend delay expires, runtime suspend callback won't be invoked, so the device will remain powered on forever. > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c > b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 55fc22e8d4d4e..a3eff77add557 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c [ ... ] > @@ -884,9 +888,11 @@ static void panfrost_remove(struct platform_device *pdev) > drm_dev_unregister(&pfdev->base); > > pm_runtime_get_sync(pfdev->base.dev); > + pm_runtime_dont_use_autosuspend(pfdev->base.dev); > + pm_runtime_put_noidle(pfdev->base.dev); > pm_runtime_disable(pfdev->base.dev); > - panfrost_device_fini(pfdev); > pm_runtime_set_suspended(pfdev->base.dev); > + panfrost_device_fini(pfdev); [Severity: High] This is a pre-existing issue, but does panfrost_device_fini() ignore the return value of pm_runtime_get_sync() and risk system crashes? Looking at panfrost_device.c:panfrost_device_fini(), it issues a warning but unconditionally proceeds with hardware teardown: drm_WARN_ON(&pfdev->base, pm_runtime_get_sync(pfdev->base.dev) < 0); ... panfrost_jm_fini(pfdev); panfrost_mmu_fini(pfdev); panfrost_gpu_fini(pfdev); If pm_runtime_get_sync() fails (e.g., due to regulator or clock failure), the device remains physically suspended with clocks disabled. Can unconditionally writing to MMIO registers in panfrost_jm_fini(), panfrost_mmu_fini(), and panfrost_gpu_fini() with clocks off cause a bus fault or kernel panic? [Severity: High] This is a pre-existing issue, but does panfrost_device_fini() tear down hardware while GPU jobs might still be executing? During device unbind, the driver does not appear to wait for actively executing jobs on the physical GPU hardware to complete or abort. It proceeds to unmap pagetables and power off the GPU: panfrost_jm_fini(pfdev); panfrost_mmu_fini(pfdev); panfrost_gpu_fini(pfdev); Can pulling the memory and clocks out from under a running GPU job during a hot-unplug or unbind cause catastrophic hardware faults or kernel panics? > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
