[PATCH] drm/amdgpu/vcn: fix idle work handler for VCN 2.5

[Alex Deucher]

On Tue, Mar 4, 2025 at 11:29 AM Alex Deucher <alexander.deucher at amd.com> wrote:
>
> VCN 2.5 uses the PG callback to enable VCN DPM which is
> a global state.  As such, we need to make sure all instances
> are in the same state.

Actually ref counting won't work because the gate and ungate calls may
not be balanced.  I just sent a v3 which just adds a new work handler
for vcn 2.5.

Alex

>
> v2: switch to a ref count (Lijo)
>
> Fixes: 4ce4fe27205c ("drm/amdgpu/vcn: use per instance callbacks for idle work handler")
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  4 +++
>  drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c   | 41 +++++++++++++++++++------
>  2 files changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 26c9c2d90f455..3bc4fe4aeb481 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -358,6 +358,10 @@ struct amdgpu_vcn {
>
>         bool                    per_inst_fw;
>         unsigned                fw_version;
> +       /* VCN 2.5 global PG handling */
> +       struct mutex            global_pg_lock;
> +       unsigned int            global_pg_count;
> +       enum amd_powergating_state global_pg_state;
>  };
>
>  struct amdgpu_fw_shared_rb_ptrs_struct {
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> index dff1a88590363..972f0842ea47b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> @@ -172,6 +172,8 @@ static int vcn_v2_5_sw_init(struct amdgpu_ip_block *ip_block)
>         uint32_t *ptr;
>         struct amdgpu_device *adev = ip_block->adev;
>
> +       mutex_init(&adev->vcn.global_pg_lock);
> +
>         for (j = 0; j < adev->vcn.num_vcn_inst; j++) {
>                 volatile struct amdgpu_fw_shared *fw_shared;
>
> @@ -1853,21 +1855,42 @@ static int vcn_v2_5_set_pg_state(struct amdgpu_vcn_inst *vinst,
>                                  enum amd_powergating_state state)
>  {
>         struct amdgpu_device *adev = vinst->adev;
> -       int ret;
> +       struct amdgpu_vcn_inst *v;
> +       int ret = 0, i;
>
>         if (amdgpu_sriov_vf(adev))
>                 return 0;
>
> -       if (state == vinst->cur_state)
> -               return 0;
> +       mutex_lock(&adev->vcn.global_pg_lock);
> +       if (state == AMD_PG_STATE_GATE) {
> +               if (adev->vcn.global_pg_count == 0)
> +                       goto unlock;
> +               adev->vcn.global_pg_count--;
> +               if (adev->vcn.global_pg_count == 0 &&
> +                   adev->vcn.global_pg_state == AMD_PG_STATE_UNGATE) {
> +                       for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> +                               v = &adev->vcn.inst[i];
> +
> +                               ret = vcn_v2_5_stop(v);
> +                       }
> +                       adev->vcn.global_pg_state = AMD_PG_STATE_GATE;
> +               }
> +       } else {
> +               if (adev->vcn.global_pg_count == 0 &&
> +                   adev->vcn.global_pg_state == AMD_PG_STATE_GATE) {
> +                       for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
> +                               v = &adev->vcn.inst[i];
>
> -       if (state == AMD_PG_STATE_GATE)
> -               ret = vcn_v2_5_stop(vinst);
> -       else
> -               ret = vcn_v2_5_start(vinst);
> +                               ret = vcn_v2_5_start(v);
> +                       }
> +                       adev->vcn.global_pg_state = AMD_PG_STATE_UNGATE;
> +               }
> +               adev->vcn.global_pg_count++;
> +       }
>
> -       if (!ret)
> -               vinst->cur_state = state;
> +unlock:
> +       vinst->cur_state = state;
> +       mutex_unlock(&adev->vcn.global_pg_lock);
>
>         return ret;
>  }
> --
> 2.48.1
>