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

["Lazar, Lijo" <lijo.lazar@amd.com>]

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 <gerry@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);