Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] panfrost: potential hw bug trigger due to reversed perfcnt disable 
sequence
--

commit 955ad49004b82d4c7b4dae93b28817ea9431d397
Author: Adrián Larumbe <[email protected]>

drm/panfrost: Move perfcnt GPU disable sequence into a helper

This commit introduces a helper function panfrost_perfcnt_hw_disable to
handle the GPU performance counter disable sequence. It updates the existing
disable and init/fini paths to use this new helper to avoid code repetition.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c 
> b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
> index 7020c0192e189..f71534e741b65 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_hw_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: Medium]
Could this sequence reversal cause a hardware lockup or undefined behavior?

The new panfrost_perfcnt_hw_disable() helper reverses the previous order by
setting GPU_PERFCNT_CFG to OFF while the enable registers are still enabled.

Modifying GPU_PERFCNT_CFG while TILER_EN is actively enabled during the
teardown path might risk triggering hardware malfunctions similar to the
PRLAM-8186 issue, which requires TILER_EN to be 0 before modifying
GPU_PERFCNT_CFG to enable counters.

Does this disable sequence need to preserve the original ordering seen in
panfrost_perfcnt_disable_locked()?

[ ... ]

> @@ -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_hw_disable(pfdev);
>  
>       perfcnt->user = NULL;
>       drm_gem_vunmap(&perfcnt->mapping->obj->base.base, &map);

-- 
Sashiko AI review · 
https://sashiko.dev/#/patchset/[email protected]?part=11

Reply via email to