Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The refactored `panfrost_perfcnt_gpu_disable()` helper reverses the register write sequence when disabling performance counters, potentially breaking the HW_ISSUE_8186 (PRLAM-8186) hardware workaround and risking hardware state corruption. --
commit b1a0eb47a40de5889be52c5109cc3ee780f78598 Author: Adrián Larumbe <[email protected]> drm/panfrost: Move perfcnt GPU disable sequence into a helper This commit refactors the performance counter disable sequences into a single helper function to avoid repetition. > diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > index 7020c0192e189..ad1156678e91c 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c > @@ -35,6 +35,16 @@ struct panfrost_perfcnt { > struct completion dump_comp; > }; > > +static void panfrost_perfcnt_gpu_disable(struct panfrost_device *pfdev) > +{ > + gpu_write(pfdev, GPU_PERFCNT_CFG, > + GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF)); > + gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0x0); > + gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0x0); > + gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0x0); > + gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0); > +} [Severity: High] Does this new helper reverse the hardware disable sequence when called on actively running counters? In the original panfrost_perfcnt_disable_locked(), the block enable registers (like GPU_PRFCNT_TILER_EN) were explicitly cleared to 0 before the master configuration GPU_PERFCNT_CFG was turned off. The new helper reverses this order. Could shutting down the master configuration while sub-blocks are still actively generating events cause hardware state corruption or bus hangs? Modifying the main configuration while the Tiler counters are actively enabled might also trigger hardware bugs (e.g., HW_ISSUE_8186), which the original sequence safely avoided. [ ... ] > @@ -193,12 +203,7 @@ static int panfrost_perfcnt_disable_locked(struct > panfrost_device *pfdev, > if (user != perfcnt->user) > return -EINVAL; > > - gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0x0); > - gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0x0); > - gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0x0); > - gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0); > - gpu_write(pfdev, GPU_PERFCNT_CFG, > - GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF)); > + panfrost_perfcnt_gpu_disable(pfdev); -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
