On Thu, Feb 17, 2022 at 2:20 PM StDenis, Tom <Tom.StDenis at amd.com> wrote:
>
> [AMD Official Use Only]
>
> I can do IP name substitutions but that's kinda not the point. We put headers called "vcn_x_y_z*.h" in the source tree but call the IP block UVD.
>
> As for SDMA ... the IP discovery has entries for SDMA0 and SDMA1 IP blocks on my navi10...
>
> [root at fedora umr]# cat /sys/class/drm/card0/device/ip_discovery/die/0/SDMA0/0/major
> 5
> [root at fedora umr]# cat /sys/class/drm/card0/device/ip_discovery/die/0/SDMA1/0/major
> 5
>
> If they're "behind the GC block" then there really isn't an SDMA block right?
Sort of; different hardware teams own them, but then the IP gets
integrated into the GC hardware block on the SoC, so the team that
generates the register headers sees it all as GC rather than as GC and
SDMA. I guess from the perspective of something like umr, you can
just use GC for both SDMA and GC. The memory hubs are similar. GFXHUB
is part of GC so it's registers are part of GC, but MMHUB is it's own
block and has its own register headers.
Alex
>
> Right now umr just ignores anything it can't pair up but I imagine it'll cause some confusion later on.
>
> Tom
>
> ________________________________________
> From: Alex Deucher <alexdeucher at gmail.com>
> Sent: Thursday, February 17, 2022 13:32
> To: StDenis, Tom
> Cc: Tuikov, Luben; amd-gfx at lists.freedesktop.org; Deucher, Alexander
> Subject: Re: [PATCH] drm/amdgpu: Dynamically initialize IP instance attributes
>
> On Thu, Feb 17, 2022 at 1:25 PM StDenis, Tom <Tom.StDenis at amd.com> wrote:
> >
> > [AMD Official Use Only]
> >
> > This works, now I'm facing other issues. Like we call the VCN block on navi10 a "uvd 2.0.0". Originally I was only accepting exact major hits which meant my navi10 model had no video enc/dec block once I programmed it to take the closest match it hit UVD 4.0.0 (pre-soc15/etc) and that's obviously wrong.
> > Shouldn't IP discovered video enc/dec be name VCN?
>
> They used the UVD name since VCN was derived from UVD. Not sure why
> they didn't rename it. Still, all asics that use IP discovery use
> VCN, so no need to worry about UVD/VCE for IP discovery.
>
> >
> > On a semi related note we have no SDMA v5.x.y headers in the tree but navi10+ uses it. I guess the driver just uses SDMA 4.x.y headers because they're close enough but it means umr won't load in an SDMA model.
> >
>
> For navi10+, SDMA moved into the graphics block so the SDMA registers
> are part of the GC headers on navi1x and newer.
>
> > Which then gets us into weird conditions. Right now I'm programming umr to find the closet abs() major hit, then minor, then revision. But I imagine we'll hit cases where say version X.Y+1.Z is better than X.Y-1.Z and vice versa (etc with the other fields). There's no way for umr to know if a newer or older header is better.
> >
> > Tom
> >
> > ________________________________________
> > From: Tuikov, Luben <Luben.Tuikov at amd.com>
> > Sent: Thursday, February 17, 2022 11:35
> > To: amd-gfx at lists.freedesktop.org
> > Cc: Tuikov, Luben; Deucher, Alexander; StDenis, Tom
> > Subject: [PATCH] drm/amdgpu: Dynamically initialize IP instance attributes
> >
> > Dynamically initialize IP instance attributes. This eliminates bugs
> > stemming from adding new attributes to an IP instance.
> >
> > Cc: Alex Deucher <Alexander.Deucher at amd.com>
> > Reported-by: Tom StDenis <tom.stdenis at amd.com>
> > Fixes: c10b6aa7417b0a ("drm/amdgpu: Add "harvest" to IP discovery sysfs")
> > Signed-off-by: Luben Tuikov <luben.tuikov at amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 17 ++++++-----------
> > 1 file changed, 6 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > index 6c7ec058125e1d..5848fec5c39251 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > @@ -482,16 +482,7 @@ static struct ip_hw_instance_attr ip_hw_attr[] = {
> > __ATTR_RO(base_addr),
> > };
> >
> > -static struct attribute *ip_hw_instance_attrs[] = {
> > - &ip_hw_attr[0].attr,
> > - &ip_hw_attr[1].attr,
> > - &ip_hw_attr[2].attr,
> > - &ip_hw_attr[3].attr,
> > - &ip_hw_attr[4].attr,
> > - &ip_hw_attr[5].attr,
> > - &ip_hw_attr[6].attr,
> > - NULL,
> > -};
> > +static struct attribute *ip_hw_instance_attrs[ARRAY_SIZE(ip_hw_attr) + 1];
> > ATTRIBUTE_GROUPS(ip_hw_instance);
> >
> > #define to_ip_hw_instance(x) container_of(x, struct ip_hw_instance, kobj)
> > @@ -789,7 +780,7 @@ static int amdgpu_discovery_sysfs_recurse(struct amdgpu_device *adev)
> > static int amdgpu_discovery_sysfs_init(struct amdgpu_device *adev)
> > {
> > struct kset *die_kset;
> > - int res;
> > + int res, ii;
> >
> > adev->ip_top = kzalloc(sizeof(*adev->ip_top), GFP_KERNEL);
> > if (!adev->ip_top)
> > @@ -814,6 +805,10 @@ static int amdgpu_discovery_sysfs_init(struct amdgpu_device *adev)
> > goto Err;
> > }
> >
> > + for (ii = 0; ii < ARRAY_SIZE(ip_hw_attr); ii++)
> > + ip_hw_instance_attrs[ii] = &ip_hw_attr[ii].attr;
> > + ip_hw_instance_attrs[ii] = NULL;
> > +
> > res = amdgpu_discovery_sysfs_recurse(adev);
> >
> > return res;
> >
> > base-commit: f736148ca7bf82654141a4411409c2d7a9e2269b
> > --
> > 2.35.1.129.gb80121027d
> >