[PATCH v4 07/15] drm/amdgpu: validate resume before function call

[Khatri, Sunil]

On 10/17/2024 5:25 PM, Christian König wrote:
> Same comment as patch #5, please add an amdgpu_ip_block_resume() 
> function.
Sure
>
> Regards,
> Christian.
>
> Am 17.10.24 um 12:06 schrieb Sunil Khatri:
>> Before making a function call to resume, validate
>> the function pointer like we do in sw_init.
>>
>> Signed-off-by: Sunil Khatri <sunil.khatri at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/aldebaran.c      | 23 ++++++----
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  | 47 ++++++++++++---------
>>   drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c | 28 ++++++------
>>   drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c   | 14 +++---
>>   4 files changed, 66 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/aldebaran.c 
>> b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
>> index e55d680d95ce..4dd9af51210d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/aldebaran.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/aldebaran.c
>> @@ -247,9 +247,11 @@ static int aldebaran_mode2_restore_ip(struct 
>> amdgpu_device *adev)
>>           dev_err(adev->dev, "Failed to get BIF handle\n");
>>           return -EINVAL;
>>       }
>> -    r = cmn_block->version->funcs->resume(cmn_block);
>> -    if (r)
>> -        return r;
>> +    if (cmn_block->version->funcs->resume) {
>> +        r = cmn_block->version->funcs->resume(cmn_block);
>> +        if (r)
>> +            return r;
>> +    }
>>         /* Reinit GFXHUB */
>>       adev->gfxhub.funcs->init(adev);
>> @@ -283,12 +285,15 @@ static int aldebaran_mode2_restore_ip(struct 
>> amdgpu_device *adev)
>>                 adev->ip_blocks[i].version->type ==
>>                     AMD_IP_BLOCK_TYPE_SDMA))
>>               continue;
>> -        r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> -        if (r) {
>> -            dev_err(adev->dev,
>> -                "resume of IP block <%s> failed %d\n",
>> - adev->ip_blocks[i].version->funcs->name, r);
>> -            return r;
>> +
>> +        if (adev->ip_blocks[i].version->funcs->resume) {
>> +            r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> +            if (r) {
>> +                dev_err(adev->dev,
>> +                    "resume of IP block <%s> failed %d\n",
>> + adev->ip_blocks[i].version->funcs->name, r);
>> +                return r;
>> +            }
>>           }
>>             adev->ip_blocks[i].status.hw = true;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index fda89aeb4d1b..aa7314c14db8 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2744,11 +2744,15 @@ static int amdgpu_device_fw_loading(struct 
>> amdgpu_device *adev)
>>                   break;
>>                 if (amdgpu_in_reset(adev) || adev->in_suspend) {
>> -                r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> -                if (r) {
>> -                    DRM_ERROR("resume of IP block <%s> failed %d\n",
>> - adev->ip_blocks[i].version->funcs->name, r);
>> -                    return r;
>> +                if (adev->ip_blocks[i].version->funcs->resume) {
>> +                    r = adev->ip_blocks[i].version->funcs->resume(
>> +                        &adev->ip_blocks[i]);
>> +                    if (r) {
>> +                        DRM_ERROR("resume of IP block <%s> failed 
>> %d\n",
>> + adev->ip_blocks[i].version->funcs
>> +                            ->name, r);
>> +                        return r;
>> +                    }
>>                   }
>>               } else {
>>                   r = 
>> adev->ip_blocks[i].version->funcs->hw_init(&adev->ip_blocks[i]);
>> @@ -3683,10 +3687,12 @@ static int 
>> amdgpu_device_ip_reinit_late_sriov(struct amdgpu_device *adev)
>>                   block->status.hw)
>>                   continue;
>>   -            if (block->version->type == AMD_IP_BLOCK_TYPE_SMC)
>> -                r = block->version->funcs->resume(&adev->ip_blocks[i]);
>> -            else
>> +            if (block->version->type == AMD_IP_BLOCK_TYPE_SMC) {
>> +                if (adev->ip_blocks[i].version->funcs->resume)
>> +                    r = 
>> block->version->funcs->resume(&adev->ip_blocks[i]);
>> +            } else {
>>                   r = 
>> block->version->funcs->hw_init(&adev->ip_blocks[i]);
>> +            }
>>                 DRM_INFO("RE-INIT-late: %s %s\n", 
>> block->version->funcs->name, r?"failed":"succeeded");
>>               if (r)
>> @@ -3721,12 +3727,13 @@ static int 
>> amdgpu_device_ip_resume_phase1(struct amdgpu_device *adev)
>>               adev->ip_blocks[i].version->type == 
>> AMD_IP_BLOCK_TYPE_GMC ||
>>               adev->ip_blocks[i].version->type == 
>> AMD_IP_BLOCK_TYPE_IH ||
>>               (adev->ip_blocks[i].version->type == 
>> AMD_IP_BLOCK_TYPE_PSP && amdgpu_sriov_vf(adev))) {
>> -
>> -            r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> -            if (r) {
>> -                DRM_ERROR("resume of IP block <%s> failed %d\n",
>> - adev->ip_blocks[i].version->funcs->name, r);
>> -                return r;
>> +            if (adev->ip_blocks[i].version->funcs->resume) {
>> +                r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> +                if (r) {
>> +                    DRM_ERROR("resume of IP block <%s> failed %d\n",
>> + adev->ip_blocks[i].version->funcs->name, r);
>> +                    return r;
>> +                }
>>               }
>>               adev->ip_blocks[i].status.hw = true;
>>           }
>> @@ -3760,11 +3767,13 @@ static int 
>> amdgpu_device_ip_resume_phase2(struct amdgpu_device *adev)
>>               adev->ip_blocks[i].version->type == 
>> AMD_IP_BLOCK_TYPE_IH ||
>>               adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_PSP)
>>               continue;
>> -        r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> -        if (r) {
>> -            DRM_ERROR("resume of IP block <%s> failed %d\n",
>> - adev->ip_blocks[i].version->funcs->name, r);
>> -            return r;
>> +        if (adev->ip_blocks[i].version->funcs->resume) {
>> +            r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> +            if (r) {
>> +                DRM_ERROR("resume of IP block <%s> failed %d\n",
>> + adev->ip_blocks[i].version->funcs->name, r);
>> +                return r;
>> +            }
>>           }
>>           adev->ip_blocks[i].status.hw = true;
>>       }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c 
>> b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
>> index 10dece12509f..082a69babe62 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/sienna_cichlid.c
>> @@ -176,12 +176,14 @@ static int 
>> sienna_cichlid_mode2_restore_ip(struct amdgpu_device *adev)
>>         for (i = 0; i < adev->num_ip_blocks; i++) {
>>           if (adev->ip_blocks[i].version->type == 
>> AMD_IP_BLOCK_TYPE_IH) {
>> -            r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> -            if (r) {
>> -                dev_err(adev->dev,
>> -                    "resume of IP block <%s> failed %d\n",
>> - adev->ip_blocks[i].version->funcs->name, r);
>> -                return r;
>> +            if (adev->ip_blocks[i].version->funcs->resume) {
>> +                r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> +                if (r) {
>> +                    dev_err(adev->dev,
>> +                        "resume of IP block <%s> failed %d\n",
>> + adev->ip_blocks[i].version->funcs->name, r);
>> +                    return r;
>> +                }
>>               }
>>                 adev->ip_blocks[i].status.hw = true;
>> @@ -194,12 +196,14 @@ static int 
>> sienna_cichlid_mode2_restore_ip(struct amdgpu_device *adev)
>>                 adev->ip_blocks[i].version->type ==
>>                     AMD_IP_BLOCK_TYPE_SDMA))
>>               continue;
>> -        r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> -        if (r) {
>> -            dev_err(adev->dev,
>> -                "resume of IP block <%s> failed %d\n",
>> - adev->ip_blocks[i].version->funcs->name, r);
>> -            return r;
>> +        if (adev->ip_blocks[i].version->funcs->resume) {
>> +            r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> +            if (r) {
>> +                dev_err(adev->dev,
>> +                    "resume of IP block <%s> failed %d\n",
>> + adev->ip_blocks[i].version->funcs->name, r);
>> +                return r;
>> +            }
>>           }
>>             adev->ip_blocks[i].status.hw = true;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c 
>> b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
>> index ab049f0b4d39..2e041424ef9f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/smu_v13_0_10.c
>> @@ -187,12 +187,14 @@ static int smu_v13_0_10_mode2_restore_ip(struct 
>> amdgpu_device *adev)
>>                 adev->ip_blocks[i].version->type ==
>>                     AMD_IP_BLOCK_TYPE_SDMA))
>>               continue;
>> -        r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> -        if (r) {
>> -            dev_err(adev->dev,
>> -                "resume of IP block <%s> failed %d\n",
>> - adev->ip_blocks[i].version->funcs->name, r);
>> -            return r;
>> +        if (adev->ip_blocks[i].version->funcs->resume) {
>> +            r = 
>> adev->ip_blocks[i].version->funcs->resume(&adev->ip_blocks[i]);
>> +            if (r) {
>> +                dev_err(adev->dev,
>> +                    "resume of IP block <%s> failed %d\n",
>> + adev->ip_blocks[i].version->funcs->name, r);
>> +                return r;
>> +            }
>>           }
>>             adev->ip_blocks[i].status.hw = true;
>