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