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

[Alex Deucher]

On Mon, Oct 28, 2024 at 3:53 PM Boyuan Zhang <Boyuan.Zhang at amd.com> wrote:
>
>
> On 2024-10-28 15:27, Alex Deucher wrote:
> > On Thu, Oct 24, 2024 at 10:48 PM <boyuan.zhang at amd.com> wrote:
> >> From: Boyuan Zhang <boyuan.zhang at 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 at amd.com>
> >> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> >> Suggested-by: Christian König <christian.koenig at amd.com>
> >> Reviewed-by: Christian König <christian.koenig at 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

>
> 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
> >>