Re: [RFC v2 08/15] drm/amdgpu: make IP block state machine works in stack like way

[Lazar, Lijo]

On 1/13/2025 7:12 AM, Jiang Liu wrote:
> There are some mismatches between IP block state machine and its
> associated status flags, especially about the meaning of
> `status.late_initialized`. So let's make the state machine and
> associated status flas work in stack-like way as below:
> Callback      Status
> early_init:   valid = true
> sw_init:      sw = true
> hw_init:      hw = true
> late_init:    late_initialized = true
> early_fini:   late_initialized = false

Changing the state like this is confusing. The intention of late_fini is
to reverse the steps in late_init. It's straight forward read like if
the ip is not late_initialized, no need to late_fini. This is making
that complicated.

Thanks,
Lijo

> hw_fini:      hw = false
> sw_fini:      sw = false
> late_fini:    valid = false
> 
> Signed-off-by: Jiang Liu <ge...@linux.alibaba.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 6cbd19ad0fa5..6b503fb7e366 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3376,6 +3376,8 @@ static int amdgpu_device_ip_fini_early(struct 
> amdgpu_device *adev)
>                       DRM_DEBUG("early_fini of IP block <%s> failed %d\n",
>                                 adev->ip_blocks[i].version->funcs->name, r);
>               }
> +
> +             adev->ip_blocks[i].status.late_initialized = false;
>       }
>  
>       amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE);
> @@ -3445,15 +3447,14 @@ static int amdgpu_device_ip_fini(struct amdgpu_device 
> *adev)
>                       }
>               }
>               adev->ip_blocks[i].status.sw = false;
> -             adev->ip_blocks[i].status.valid = false;
>       }
>  
>       for (i = adev->num_ip_blocks - 1; i >= 0; i--) {
> -             if (!adev->ip_blocks[i].status.late_initialized)
> +             if (!adev->ip_blocks[i].status.valid)
>                       continue;
>               if (adev->ip_blocks[i].version->funcs->late_fini)
>                       
> adev->ip_blocks[i].version->funcs->late_fini(&adev->ip_blocks[i]);
> -             adev->ip_blocks[i].status.late_initialized = false;
> +             adev->ip_blocks[i].status.valid = false;
>       }
>  
>       amdgpu_ras_fini(adev);