Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Waking up the user thread before clearing the GPU interrupt introduces a race condition that can clear the interrupt of a subsequent performance counter dump, causing it to time out. --
commit f36520c5a42efb4216abf8dab5756d0a6134f2a4 Author: Adrián Larumbe <[email protected]> drm/panfrost: Avoid cache flush after perfcnt sample in fully coherent systems This commit skips the GPU cache flush when the system is fully coherent (ACE). Instead of sending a cache clean command, it immediately signals completion of the performance counter sample. > diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > index ffc77121070e2..b3f71d7fd82a9 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > @@ -52,7 +52,10 @@ void panfrost_perfcnt_clean_cache_done(struct > panfrost_device *pfdev) > > void panfrost_perfcnt_sample_done(struct panfrost_device *pfdev) > { > - gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES); > + if (pfdev->features.selected_coherency != COHERENCY_ACE) > + gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES); > + else [Severity: Medium] Does waking up the user thread before clearing the GPU interrupt introduce a race condition that can drop subsequent sample interrupts? If panfrost_perfcnt_sample_done() calls complete() and wakes up the user thread, the thread can return to userspace and immediately issue a new PERFCNT_DUMP ioctl, starting a new hardware sample. If the IRQ handler on the original CPU is delayed or preempted (e.g., by a hypervisor or NMI) before it writes to the clear register: drivers/gpu/drm/panfrost/panfrost_gpu.c:panfrost_gpu_irq_handler() { ... if (state & GPU_IRQ_PERFCNT_SAMPLE_COMPLETED) panfrost_perfcnt_sample_done(pfdev); ... gpu_write(pfdev, GPU_INT_CLEAR, state); } Could it write the original state (which has the completion bit set) to GPU_INT_CLEAR after the new sample has already started? This would clear the interrupt for the newly submitted sample, causing the new dump's wait to time out with -ETIMEDOUT. Prior to this patch, the CLEAN_CACHES step prevented this race because the delayed completion meant the interrupt clear could not hit a new sample. > + complete(&pfdev->perfcnt->dump_comp); > } > > static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev) -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13
