[PATCH v4 03/18] drm/panthor: Make panthor_device::pm::state non-atomic
From: Boris Brezillon
Date: Wed Aug 26 2026 - 11:01:18 EST
Now that the reset logic has been reworked to use disable/enable_work(),
there's no need for panthor_device::pm::state to be an atomic. It can
simply be accessed under the same lock we use to touch MMIO mappings.
While at it, rename the lock to make it clear it protects more than just
the MMIO logic, and transition locked sections to scoped_guard().
Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
---
drivers/gpu/drm/panthor/panthor_device.c | 97 +++++++++++++++++---------------
drivers/gpu/drm/panthor/panthor_device.h | 16 ++++--
2 files changed, 63 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
index 2974f4bc0bb1..133e3895cd0a 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -147,8 +147,10 @@ static void panthor_device_reset_work(struct work_struct *work)
/* If the device is entering suspend, we don't reset. A slow reset will
* be forced at resume time instead.
*/
- if (atomic_read(&ptdev->pm.state) != PANTHOR_DEVICE_PM_STATE_ACTIVE)
- return;
+ scoped_guard(mutex, &ptdev->pm.lock) {
+ if (ptdev->pm.state != PANTHOR_DEVICE_PM_STATE_ACTIVE)
+ return;
+ }
if (!drm_dev_enter(&ptdev->base, &cookie))
return;
@@ -204,7 +206,7 @@ int panthor_device_init(struct panthor_device *ptdev)
if (ret)
return ret;
- ret = drmm_mutex_init(&ptdev->base, &ptdev->pm.mmio_lock);
+ ret = drmm_mutex_init(&ptdev->base, &ptdev->pm.lock);
if (ret)
return ret;
@@ -213,7 +215,7 @@ int panthor_device_init(struct panthor_device *ptdev)
INIT_LIST_HEAD(&ptdev->gems.node);
#endif
- atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED);
+ ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_SUSPENDED;
p = alloc_page(GFP_KERNEL | __GFP_ZERO);
if (!p)
return -ENOMEM;
@@ -432,40 +434,39 @@ static vm_fault_t panthor_mmio_vm_fault(struct vm_fault *vmf)
{
struct vm_area_struct *vma = vmf->vma;
struct panthor_device *ptdev = vma->vm_private_data;
- u64 offset = (u64)vma->vm_pgoff << PAGE_SHIFT;
- unsigned long pfn;
- pgprot_t pgprot;
vm_fault_t ret;
- bool active;
int cookie;
if (!drm_dev_enter(&ptdev->base, &cookie))
return VM_FAULT_SIGBUS;
- mutex_lock(&ptdev->pm.mmio_lock);
- active = atomic_read(&ptdev->pm.state) == PANTHOR_DEVICE_PM_STATE_ACTIVE;
+ scoped_guard(mutex, &ptdev->pm.lock) {
+ bool active = ptdev->pm.state == PANTHOR_DEVICE_PM_STATE_ACTIVE;
+ u64 offset = (u64)vma->vm_pgoff << PAGE_SHIFT;
+ unsigned long pfn;
+ pgprot_t pgprot;
- switch (offset) {
- case DRM_PANTHOR_USER_FLUSH_ID_MMIO_OFFSET:
+ switch (offset) {
+ case DRM_PANTHOR_USER_FLUSH_ID_MMIO_OFFSET:
+ if (active)
+ pfn = __phys_to_pfn(ptdev->phys_addr + CSF_GPU_LATEST_FLUSH_ID);
+ else
+ pfn = page_to_pfn(ptdev->pm.dummy_latest_flush);
+ break;
+
+ default:
+ ret = VM_FAULT_SIGBUS;
+ goto out_dev_exit;
+ }
+
+ pgprot = vma->vm_page_prot;
if (active)
- pfn = __phys_to_pfn(ptdev->phys_addr + CSF_GPU_LATEST_FLUSH_ID);
- else
- pfn = page_to_pfn(ptdev->pm.dummy_latest_flush);
- break;
+ pgprot = pgprot_noncached(pgprot);
- default:
- ret = VM_FAULT_SIGBUS;
- goto out_unlock;
+ ret = vmf_insert_pfn_prot(vma, vmf->address, pfn, pgprot);
}
- pgprot = vma->vm_page_prot;
- if (active)
- pgprot = pgprot_noncached(pgprot);
-
- ret = vmf_insert_pfn_prot(vma, vmf->address, pfn, pgprot);
-
-out_unlock:
- mutex_unlock(&ptdev->pm.mmio_lock);
+out_dev_exit:
drm_dev_exit(cookie);
return ret;
}
@@ -526,10 +527,13 @@ int panthor_device_resume(struct device *dev)
struct panthor_device *ptdev = dev_get_drvdata(dev);
int ret, cookie;
- if (atomic_read(&ptdev->pm.state) != PANTHOR_DEVICE_PM_STATE_SUSPENDED)
- return -EINVAL;
+ scoped_guard(mutex, &ptdev->pm.lock) {
+ if (ptdev->pm.state != PANTHOR_DEVICE_PM_STATE_SUSPENDED)
+ return -EINVAL;
+
+ ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_RESUMING;
+ }
- atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_RESUMING);
ret = clk_prepare_enable(ptdev->clks.core);
if (ret)
@@ -574,11 +578,11 @@ int panthor_device_resume(struct device *dev)
* are removed and the real iomem mapping will be restored on next
* access.
*/
- mutex_lock(&ptdev->pm.mmio_lock);
+ mutex_lock(&ptdev->pm.lock);
unmap_mapping_range(ptdev->base.anon_inode->i_mapping,
DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
- atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_ACTIVE);
- mutex_unlock(&ptdev->pm.mmio_lock);
+ ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_ACTIVE;
+ mutex_unlock(&ptdev->pm.lock);
/* Now that everything is resumed, we can re-enable the reset work. */
enable_resets(ptdev);
@@ -595,7 +599,9 @@ int panthor_device_resume(struct device *dev)
clk_disable_unprepare(ptdev->clks.core);
err_set_suspended:
- atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED);
+ scoped_guard(mutex, &ptdev->pm.lock)
+ ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_SUSPENDED;
+
atomic_set(&ptdev->pm.recovery_needed, 1);
return ret;
}
@@ -605,21 +611,21 @@ int panthor_device_suspend(struct device *dev)
struct panthor_device *ptdev = dev_get_drvdata(dev);
int cookie;
- if (atomic_read(&ptdev->pm.state) != PANTHOR_DEVICE_PM_STATE_ACTIVE)
- return -EINVAL;
-
/* Clear all IOMEM mappings pointing to this device before we
* shutdown the power-domain and clocks. Failing to do that results
* in external aborts when the process accesses the iomem region.
* We change the state and call unmap_mapping_range() with the
- * mmio_lock held to make sure the vm_fault handler won't set up
+ * lock held to make sure the vm_fault handler won't set up
* invalid mappings.
*/
- mutex_lock(&ptdev->pm.mmio_lock);
- atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDING);
- unmap_mapping_range(ptdev->base.anon_inode->i_mapping,
- DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
- mutex_unlock(&ptdev->pm.mmio_lock);
+ scoped_guard(mutex, &ptdev->pm.lock) {
+ if (ptdev->pm.state != PANTHOR_DEVICE_PM_STATE_ACTIVE)
+ return -EINVAL;
+
+ ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_SUSPENDING;
+ unmap_mapping_range(ptdev->base.anon_inode->i_mapping,
+ DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1);
+ }
/* Make sure we're not interrupted by resets after that point
* until the GPU is resumed.
@@ -644,6 +650,9 @@ int panthor_device_suspend(struct device *dev)
clk_disable_unprepare(ptdev->clks.coregroup);
clk_disable_unprepare(ptdev->clks.stacks);
clk_disable_unprepare(ptdev->clks.core);
- atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED);
+
+ scoped_guard(mutex, &ptdev->pm.lock)
+ ptdev->pm.state = PANTHOR_DEVICE_PM_STATE_SUSPENDED;
+
return 0;
}
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index f55baa21b25e..217eec811bdb 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -298,18 +298,22 @@ struct panthor_device {
/** @pm: Power management related data. */
struct {
- /** @state: Power state. */
- atomic_t state;
+ /**
+ * @state: Power state.
+ *
+ * Must be accessed with the panthor_device::pm::lock held.
+ */
+ enum panthor_device_pm_state state;
/**
- * @mmio_lock: Lock protecting MMIO userspace CPU mappings.
+ * @lock: Lock protecting PM related fields.
*
* This is needed to ensure we map the dummy IO pages when
* the device is being suspended, and the real IO pages when
- * the device is being resumed. We can't just do with the
- * state atomicity to deal with this race.
+ * the device is being resumed. We can't just do with an
+ * atomic state to deal with this race.
*/
- struct mutex mmio_lock;
+ struct mutex lock;
/**
* @dummy_latest_flush: Dummy LATEST_FLUSH page.
--
2.55.0