Re: [PATCH 09/29] drm/amdgpu: track instances of the same IP block

[Boyuan Zhang <Boyuan.Zhang@amd.com>]

On 2024-10-28 16:05, Alex Deucher wrote:
> On Mon, Oct 28, 2024 at 3:53 PM Boyuan Zhang <Boyuan.Zhang@amd.com> wrote:
>>
>> On 2024-10-28 15:27, Alex Deucher wrote:
>>> On Thu, Oct 24, 2024 at 10:48 PM <boyuan.zhang@amd.com> wrote:
>>>> From: Boyuan Zhang <boyuan.zhang@amd.com>
>>>>
>>>> Add a new function to count the number of instance of the same IP block
>>>> in the current ip_block list, then use the returned count value to set
>>>> the newly defined instance variable in ip_block, to track the instance
>>>> number of each ip_block.
>>>>
>>>> Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> Suggested-by: Christian König <christian.koenig@amd.com>
>>>> Reviewed-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 +
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 25 +++++++++++++++++++++-
>>>>    2 files changed, 25 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> index fba10ad44be9..2e2c6a556cc8 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> @@ -390,6 +390,7 @@ struct amdgpu_ip_block {
>>>>           struct amdgpu_ip_block_status status;
>>>>           const struct amdgpu_ip_block_version *version;
>>>>           struct amdgpu_device *adev;
>>>> +       unsigned int instance;
>>> Thinking towards future work, we should add a `bool harvested;` member
>>> to the structure so that we can skip harvested instances in the common
>>> code going forward.
>>>
>>> Alex
>>
>> OK, so do you suggest to add it in this patch set, or a separated patch
>> set when we implement it later on?
> Later on.  Just thinking out loud for when we clean up adev->vcn.
> I.e., we can remove all of the checks for (harvest & (1 << inst))
> because we can set ip_block->harvested = true in the common code and
> then in amdgpu_device_ip_early_init() we can do:
>
> if (ip_block->harvested)
>     adev->ip_blocks[i].status.valid = false;
>
> and we won't have to check for harvested instances in any of the runtime code.
>
> Alex


Really good idea! Sure, will clean up this part later on! Thanks!

Boyuan


>
>> Boyuan
>>
>>
>>>>    };
>>>>
>>>>    int amdgpu_device_ip_block_version_cmp(struct amdgpu_device *adev,
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> index 7c06e3a9146c..065463b5d6a9 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -2322,6 +2322,28 @@ int amdgpu_device_ip_block_version_cmp(struct amdgpu_device *adev,
>>>>           return 1;
>>>>    }
>>>>
>>>> +/**
>>>> + * amdgpu_device_ip_get_num_instances - get number of instances of an IP block
>>>> + *
>>>> + * @adev: amdgpu_device pointer
>>>> + * @type: Type of hardware IP (SMU, GFX, UVD, etc.)
>>>> + *
>>>> + * Returns the count of the hardware IP blocks structure for that type.
>>>> + */
>>>> +static unsigned int
>>>> +amdgpu_device_ip_get_num_instances(struct amdgpu_device *adev,
>>>> +                                   enum amd_ip_block_type type)
>>>> +{
>>>> +       unsigned int i, count = 0;
>>>> +
>>>> +       for (i = 0; i < adev->num_ip_blocks; i++) {
>>>> +               if (adev->ip_blocks[i].version->type == type)
>>>> +                       count++;
>>>> +       }
>>>> +
>>>> +       return count;
>>>> +}
>>>> +
>>>>    /**
>>>>     * amdgpu_device_ip_block_add
>>>>     *
>>>> @@ -2354,7 +2376,8 @@ int amdgpu_device_ip_block_add(struct amdgpu_device *adev,
>>>>                     ip_block_version->funcs->name);
>>>>
>>>>           adev->ip_blocks[adev->num_ip_blocks].adev = adev;
>>>> -
>>>> +       adev->ip_blocks[adev->num_ip_blocks].instance =
>>>> +               amdgpu_device_ip_get_num_instances(adev, ip_block_version->type);
>>>>           adev->ip_blocks[adev->num_ip_blocks++].version = ip_block_version;
>>>>
>>>>           return 0;
>>>> --
>>>> 2.34.1
>>>>