From: "Khatri, Sunil" <sunil.khatri@amd.com>
To: "Christian König" <ckoenig.leichtzumerken@gmail.com>,
"Alex Deucher" <alexander.deucher@amd.com>,
"Christian König" <christian.koenig@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH v4 07/15] drm/amdgpu: validate resume before function call
Date: Thu, 17 Oct 2024 18:16:00 +0530 [thread overview]
Message-ID: <f45a4aff-c772-3344-5b16-1aa147b9ca09@amd.com> (raw)
In-Reply-To: <bd5cebef-9423-4cd2-a965-208735bca523@gmail.com>
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@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;
>
next prev parent reply other threads:[~2024-10-17 12:46 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-17 10:06 [PATCH v4 00/15] validate/clean the functions of ip funcs Sunil Khatri
2024-10-17 10:06 ` [PATCH v4 01/15] drm/amdgpu: validate sw_init before function call Sunil Khatri
2024-10-17 10:06 ` [PATCH v4 02/15] drm/amdgpu: clean the dummy sw_init functions Sunil Khatri
2024-10-17 10:06 ` [PATCH v4 03/15] drm/amdgpu: validate sw_fini before function call Sunil Khatri
2024-10-17 10:06 ` [PATCH v4 04/15] drm/amdgpu: clean the dummy sw_fini functions Sunil Khatri
2024-10-17 10:06 ` [PATCH v4 05/15] drm/amdgpu: validate hw_fini before function call Sunil Khatri
2024-10-17 11:52 ` Christian König
2024-10-17 12:44 ` Khatri, Sunil
2024-10-17 10:06 ` [PATCH v4 06/15] drm/amdgpu: validate suspend " Sunil Khatri
2024-10-17 11:54 ` Christian König
2024-10-17 12:45 ` Khatri, Sunil
2024-10-17 10:06 ` [PATCH v4 07/15] drm/amdgpu: validate resume " Sunil Khatri
2024-10-17 11:55 ` Christian König
2024-10-17 12:46 ` Khatri, Sunil [this message]
2024-10-17 10:06 ` [PATCH v4 08/15] drm/amdgpu: validate wait_for_idle " Sunil Khatri
2024-10-17 10:06 ` [PATCH v4 09/15] drm/amdgpu: clean the dummy resume functions Sunil Khatri
2024-10-17 10:06 ` [PATCH v4 10/15] drm/amdgpu: clean the dummy suspend functions Sunil Khatri
2024-10-17 10:06 ` [PATCH v4 11/15] drm/amdgpu: clean the dummy hw_fini functions Sunil Khatri
2024-10-17 11:58 ` Christian König
2024-10-17 12:47 ` Khatri, Sunil
2024-10-17 10:06 ` [PATCH v4 12/15] drm/amdgpu: clean the dummy wait_for_idle functions Sunil Khatri
2024-10-17 10:06 ` [PATCH v4 13/15] drm/amdgpu: clean the dummy soft_reset functions Sunil Khatri
2024-10-17 10:06 ` [PATCH v4 14/15] drm/amdgpu: clean unused functions of amd_ip_funcs Sunil Khatri
2024-10-17 12:18 ` Christian König
2024-10-17 12:49 ` Khatri, Sunil
2024-10-17 10:06 ` [PATCH v4 15/15] drm/amdgpu: validate get_clockgating_state before use Sunil Khatri
2024-10-17 12:20 ` Christian König
2024-10-17 12:55 ` Khatri, Sunil
2024-10-17 13:15 ` Christian König
2024-10-17 12:23 ` [PATCH v4 00/15] validate/clean the functions of ip funcs Christian König
2024-10-17 12:57 ` Khatri, Sunil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f45a4aff-c772-3344-5b16-1aa147b9ca09@amd.com \
--to=sunil.khatri@amd.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=ckoenig.leichtzumerken@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox