Re: [PATCH v5 02/12] drm/amdgpu: add helper function amdgpu_ip_block_suspend

["Khatri, Sunil" <sunil.khatri@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 2346 bytes --]


On 10/18/2024 4:40 PM, Christian König wrote:
> Am 17.10.24 um 18:25 schrieb Sunil Khatri:
>> Use the helper function amdgpu_ip_block_suspend where
>> same checks and calls are repeated.
>
> I strongly suggest to squash this patch and the next one together.
Sure. Noted
>
>> Signed-off-by: Sunil Khatri<sunil.khatri@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 17 +++++++++++++++++
>>   2 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 48c9b9b06905..df57efa019ba 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -364,6 +364,7 @@ int amdgpu_device_ip_wait_for_idle(struct amdgpu_device *adev,
>>   				   enum amd_ip_block_type block_type);
>>   bool amdgpu_device_ip_is_valid(struct amdgpu_device *adev,
>>   			      enum amd_ip_block_type block_type);
>> +int amdgpu_ip_block_suspend(struct amdgpu_ip_block *ip_block);
>>   
>>   #define AMDGPU_MAX_IP_NUM 16
>>   
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index b7277bef7463..f69aba68e7b1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -272,6 +272,23 @@ void amdgpu_reg_state_sysfs_fini(struct amdgpu_device *adev)
>>   	sysfs_remove_bin_file(&adev->dev->kobj, &bin_attr_reg_state);
>>   }
>>   
>> +int amdgpu_ip_block_suspend(struct amdgpu_ip_block *ip_block)
>> +{
>> +	int r;
>> +
>> +	if (ip_block->version->funcs->suspend) {
>> +		r = ip_block->version->funcs->suspend(ip_block);
>> +		if (r) {
>> +			dev_err(ip_block->adev->dev,
>> +				"suspend of IP block <%s> failed %d\n",
>> +				ip_block->version->funcs->name, r);
>> +			return r;
>> +		}
>> +	}
>
> Please add "ip_blocks->status.hw = false;" and remove that from the 
> callers as well. I did not add that in the amdgpu_ip_block_suspendas 
> on failure we do return without setting status.hw=false. In case 
> suspend fail and we return then in that case as per the current logic
> we dont set that flag. And with the current logic that is maintained.
> Apart from that looks good to me, Christian.
>> +
>> +	return 0;
>> +}
>> +
>>   /**
>>    * DOC: board_info
>>    *
>

[-- Attachment #2: Type: text/html, Size: 3598 bytes --]