<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> </head> <body> <div>I literally brought this up in our initial discussion....</div> <div> </div> <div>Frankly from umrs point of view a single file is easier.</div> <div> </div> <div>But I can't code anything until it's in the tree...</div> <div> </div> <div> </div> <div>Tom</div> <div> </div> <div> </div> <div> </div> <div> <div style="font-family:Tahoma; font-size:13px"> </div> </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Alex Deucher <alexdeucher@gmail.com> Sent: Tuesday, January 25, 2022 11:39 To: StDenis, Tom <Tom.StDenis@amd.com> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org> Subject: Re: [PATCH] drm/amd/amdgpu: Add ip_discovery_text sysfs entry <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">On Mon, Jan 24, 2022 at 1:07 PM Tom St Denis <tom.stdenis@amd.com> wrote: > > Newer hardware has a discovery table in hardware that the kernel will > rely on instead of header files for things like IP offsets. This > sysfs entry adds a simple to parse table of IP instances and segment > offsets. > > Produces output that looks like: > > $ cat ip_discovery_text > ATHUB{0} v2.0.0: 00000c00 02408c00 > CLKA{0} v11.0.0: 00016c00 02401800 > CLKA{1} v11.0.0: 00016e00 02401c00 > CLKA{2} v11.0.0: 00017000 02402000 > CLKA{3} v11.0.0: 00017200 02402400 > CLKA{4} v11.0.0: 0001b000 0242d800 > CLKB{0} v11.0.0: 00017e00 0240bc00 > DBGU_NBIO{0} v3.0.0: 000001c0 02409000 > DBGU0{0} v3.0.0: 00000180 02409800 > DBGU1{0} v3.0.0: 000001a0 02409c00 > DF{0} v3.0.0: 00007000 0240b800 > DFX{0} v4.1.0: 00000580 02409400 > DFX_DAP{0} v2.0.0: 000005a0 00b80000 0240c400 > DMU{0} v2.0.2: 00000012 000000c0 000034c0 00009000 02403c00 > FUSE{0} v11.0.0: 00017400 02401400 > GC{0} v10.1.10: 00001260 0000a000 02402c00 I'm not opposed to this, but the general rule is one value per file with sysfs. Maybe something like: $ ls ip_discovery ATHUB CLKA CLKB DBGU_NBIO DBGU0 DBGU1 $ ls ip_discovery/CLKA 0 1 2 3 4 $ ls ip_discovery/CLKA/0 version offset0 offset1 Alex > > Signed-off-by: Tom St Denis <tom.stdenis@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 77 ++++++++++++++++++- > 2 files changed, 77 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index 3bc76759c143..6920f73bbe73 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -1019,6 +1019,7 @@ struct amdgpu_device { > struct amdgpu_ip_block ip_blocks[AMDGPU_MAX_IP_NUM]; > uint32_t harvest_ip_mask; > int num_ip_blocks; > + char *ip_discovery_text; > struct mutex mn_lock; > DECLARE_HASHTABLE(mn_hash, 7); > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > index 285811509d94..c64cab0ad18e 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c > @@ -267,6 +267,19 @@ static void amdgpu_discovery_harvest_config_quirk(struct amdgpu_device *adev) > } > } > > +static ssize_t ip_discovery_text_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct drm_device *ddev = dev_get_drvdata(dev); > + struct amdgpu_device *adev = drm_to_adev(ddev); > + > + return sysfs_emit(buf, "%s", adev->ip_discovery_text); > +} > + > +static DEVICE_ATTR(ip_discovery_text, S_IRUGO, > + ip_discovery_text_show, NULL); > + > + > static int amdgpu_discovery_init(struct amdgpu_device *adev) > { > struct table_info *info; > @@ -351,6 +364,11 @@ static int amdgpu_discovery_init(struct amdgpu_device *adev) > goto out; > } > > + // init sysfs for ip_discovery > + r = sysfs_create_file(&adev->dev->kobj, &dev_attr_ip_discovery_text.attr); > + if (r) > + dev_err(adev->dev, "Could not create amdgpu device attr\n"); > + > return 0; > > out: > @@ -363,7 +381,11 @@ static int amdgpu_discovery_init(struct amdgpu_device *adev) > void amdgpu_discovery_fini(struct amdgpu_device *adev) > { > kfree(adev->mman.discovery_bin); > + kfree(adev->ip_discovery_text); > + sysfs_remove_file(&adev->dev->kobj, &dev_attr_ip_discovery_text.attr); > + > adev->mman.discovery_bin = NULL; > + adev->ip_discovery_text = NULL; > } > > static int amdgpu_discovery_validate_ip(const struct ip *ip) > @@ -382,6 +404,20 @@ static int amdgpu_discovery_validate_ip(const struct ip *ip) > return 0; > } > > +static int add_string(char **dst, unsigned *size, char *src) > +{ > + if (strlen(src) + strlen(*dst) >= *size) { > + void *tmp = krealloc(*dst, *size + 4096, GFP_KERNEL); > + if (!tmp) { > + return -1; > + } > + *dst = tmp; > + *size = *size + 4096; > + } > + strcat(*dst, src); > + return 0; > +} > + > int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev) > { > struct binary_header *bhdr; > @@ -396,6 +432,8 @@ int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev) > int hw_ip; > int i, j, k; > int r; > + unsigned txt_size = 4096; > + char *linebuf; > > r = amdgpu_discovery_init(adev); > if (r) { > @@ -410,6 +448,15 @@ int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev) > > DRM_DEBUG("number of dies: %d\n", num_dies); > > + adev->ip_discovery_text = kzalloc(txt_size, GFP_KERNEL); > + linebuf = kzalloc(4096, GFP_KERNEL); > + if (!adev->ip_discovery_text || !linebuf) { > + DRM_ERROR("Out of memory\n"); > + kfree(linebuf); > + kfree(adev->ip_discovery_text); > + return -ENOMEM; > + } > + > for (i = 0; i < num_dies; i++) { > die_offset = le16_to_cpu(ihdr->die_info[i].die_offset); > dhdr = (struct die_header *)(adev->mman.discovery_bin + die_offset); > @@ -419,6 +466,8 @@ int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev) > if (le16_to_cpu(dhdr->die_id) != i) { > DRM_ERROR("invalid die id %d, expected %d\n", > le16_to_cpu(dhdr->die_id), i); > + kfree(linebuf); > + kfree(adev->ip_discovery_text); > return -EINVAL; > } > > @@ -458,6 +507,19 @@ int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev) > le16_to_cpu(ip->hw_id) == SDMA3_HWID) > adev->sdma.num_instances++; > > + snprintf(linebuf, 4096, "%s{%d} v%d.%d.%d: ", > + hw_id_names[le16_to_cpu(ip->hw_id)], > + ip->number_instance, > + ip->major, ip->minor, > + ip->revision); > + if (add_string(&adev->ip_discovery_text, &txt_size, linebuf)) { > + DRM_ERROR("Out of memory\n"); > + kfree(linebuf); > + kfree(adev->ip_discovery_text); > + return -ENOMEM; > + } > + > + > for (k = 0; k < num_base_address; k++) { > /* > * convert the endianness of base addresses in place, > @@ -465,6 +527,19 @@ int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev) > */ > ip->base_address[k] = le32_to_cpu(ip->base_address[k]); > DRM_DEBUG("\t0x%08x\n", ip->base_address[k]); > + snprintf(linebuf, 4096, "%08lx ", (unsigned long)ip->base_address[k]); > + if (add_string(&adev->ip_discovery_text, &txt_size, linebuf)) { > + DRM_ERROR("Out of memory\n"); > + kfree(linebuf); > + kfree(adev->ip_discovery_text); > + return -ENOMEM; > + } > + } > + if (add_string(&adev->ip_discovery_text, &txt_size, "\n")) { > + DRM_ERROR("Out of memory\n"); > + kfree(linebuf); > + kfree(adev->ip_discovery_text); > + return -ENOMEM; > } > > for (hw_ip = 0; hw_ip < MAX_HWIP; hw_ip++) { > @@ -491,7 +566,7 @@ int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev) > ip_offset += sizeof(*ip) + 4 * (ip->num_base_address - 1); > } > } > - > + kfree(linebuf); > return 0; > } > > -- > 2.32.0 > </div> </div> </body> </html>