| Message ID | 20240326111205.510019-2-boris.brezillon@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/3] drm/panthor: Fix IO-page mmap() for 32-bit userspace on 64-bit kernel | expand |
On Tue, Mar 26, 2024 at 12:12:04PM +0100, Boris Brezillon wrote: > Make sure we set suspended=true last to avoid generating an irq storm > in the unlikely case where an IRQ happens between the suspended=true > assignment and the _INT_MASK update. > > We also move the mask=0 assignment before writing to the _INT_MASK > register to prevent the thread handler from unmasking the interrupt > behind our back. This means we might lose events if there were some > pending when we get to suspend the IRQ, but that's fine. > The synchronize_irq() we have in the _irq_suspend() path was not > there to make sure all IRQs are processed, just to make sure we don't > have registers accesses coming from the irq handlers after > _irq_suspend() has been called. If there's a need to have all pending > IRQs processed, it should happen before _irq_suspend() is called. > > v3: > - Add Steve's R-b > > v2: > - New patch > > Fixes: 5fe909cae118 ("drm/panthor: Add the device logical block") > Reported-by: Steven Price <steven.price@arm.com> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > Reviewed-by: Steven Price <steven.price@arm.com> Acked-by: Liviu Dudau <liviu.dudau@arm.com> Best regards, Liviu > --- > drivers/gpu/drm/panthor/panthor_device.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h > index 7ee4987a3796..3a930a368ae1 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.h > +++ b/drivers/gpu/drm/panthor/panthor_device.h > @@ -325,7 +325,7 @@ static inline void panthor_ ## __name ## _irq_suspend(struct panthor_irq *pirq) > { \ > int cookie; \ > \ > - atomic_set(&pirq->suspended, true); \ > + pirq->mask = 0; \ > \ > if (drm_dev_enter(&pirq->ptdev->base, &cookie)) { \ > gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, 0); \ > @@ -333,7 +333,7 @@ static inline void panthor_ ## __name ## _irq_suspend(struct panthor_irq *pirq) > drm_dev_exit(cookie); \ > } \ > \ > - pirq->mask = 0; \ > + atomic_set(&pirq->suspended, true); \ > } \ > \ > static inline void panthor_ ## __name ## _irq_resume(struct panthor_irq *pirq, u32 mask) \ > -- > 2.44.0 >
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h index 7ee4987a3796..3a930a368ae1 100644 --- a/drivers/gpu/drm/panthor/panthor_device.h +++ b/drivers/gpu/drm/panthor/panthor_device.h @@ -325,7 +325,7 @@ static inline void panthor_ ## __name ## _irq_suspend(struct panthor_irq *pirq) { \ int cookie; \ \ - atomic_set(&pirq->suspended, true); \ + pirq->mask = 0; \ \ if (drm_dev_enter(&pirq->ptdev->base, &cookie)) { \ gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, 0); \ @@ -333,7 +333,7 @@ static inline void panthor_ ## __name ## _irq_suspend(struct panthor_irq *pirq) drm_dev_exit(cookie); \ } \ \ - pirq->mask = 0; \ + atomic_set(&pirq->suspended, true); \ } \ \ static inline void panthor_ ## __name ## _irq_resume(struct panthor_irq *pirq, u32 mask) \
Make sure we set suspended=true last to avoid generating an irq storm in the unlikely case where an IRQ happens between the suspended=true assignment and the _INT_MASK update. We also move the mask=0 assignment before writing to the _INT_MASK register to prevent the thread handler from unmasking the interrupt behind our back. This means we might lose events if there were some pending when we get to suspend the IRQ, but that's fine. The synchronize_irq() we have in the _irq_suspend() path was not there to make sure all IRQs are processed, just to make sure we don't have registers accesses coming from the irq handlers after _irq_suspend() has been called. If there's a need to have all pending IRQs processed, it should happen before _irq_suspend() is called. v3: - Add Steve's R-b v2: - New patch Fixes: 5fe909cae118 ("drm/panthor: Add the device logical block") Reported-by: Steven Price <steven.price@arm.com> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> Reviewed-by: Steven Price <steven.price@arm.com> --- drivers/gpu/drm/panthor/panthor_device.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)