Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm: race condition in panfrost_gpu_irq_handler causes lost perfcnt interrupts --
commit 54ca68cf4bae3920aff016a6d2b492d879450341 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 after a performance counter sample becomes available when the GPU and CPU are fully coherent. It does this by completing the dump immediately rather than issuing a cache clean command. > 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 > + complete(&pfdev->perfcnt->dump_comp); [Severity: Medium] Since complete() is now called before the IRQ handler clears the interrupt, could this cause a race condition where a subsequent perfcnt dump request times out? If the waiting process wakes up and immediately issues another perfcnt dump request on another CPU, it might write GPU_CMD_PERFCNT_SAMPLE and trigger a new GPU_IRQ_PERFCNT_SAMPLE_COMPLETED interrupt before the original IRQ handler finishes. The original IRQ handler in panfrost_gpu_irq_handler() finishes by writing the previously read state back: drivers/gpu/drm/panfrost/panfrost_gpu.c:panfrost_gpu_irq_handler() { ... state = gpu_read(pfdev, GPU_INT_STAT); ... if (state & GPU_IRQ_PERFCNT_SAMPLE_COMPLETED) panfrost_perfcnt_sample_done(pfdev); ... gpu_write(pfdev, GPU_INT_CLEAR, state); return IRQ_HANDLED; } Could this gpu_write(pfdev, GPU_INT_CLEAR, state) clear the newly asserted interrupt, causing the second dump request to hang and return -ETIMEDOUT to userspace? > } > > static int panfrost_perfcnt_dump_locked(struct panfrost_device *pfdev) -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13
