No real reason for sysfs instead of debugfs, I just picked the one that was more familiar with. I can definitely move it to debugfs instead. I will also clean up the commit message per Michel's comments. Thank you! Kent -----Original Message----- From: Christian König [mailto:deathsimple@xxxxxxxxxxx] Sent: Thursday, August 24, 2017 2:22 AM To: Russell, Kent; amd-gfx at lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu: Add sysfs file for VBIOS Am 23.08.2017 um 20:12 schrieb Kent Russell: > This won't change after initialization, so we can add the information > when we parse the atombios information. This ensures that we can find > out the VBIOS, even when the dmesg buffer fills up, and makes it > easier to associate which VBIOS is for which GPU on mGPU > configurations. Set the size to 20 characters in case of some weird > VBIOS suffix that exceeds the expected 17 character format (3-8-3\0) Is there any reason that needs to be sysfs? Sounds more like an use case for debugfs. Christian. > > Signed-off-by: Kent Russell <kent.russell at amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 23 +++++++++++++++++++++++ > drivers/gpu/drm/amd/amdgpu/atom.c | 5 ++++- > drivers/gpu/drm/amd/amdgpu/atom.h | 1 + > 3 files changed, 28 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index a1f9424..f40be71 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -888,6 +888,20 @@ static uint32_t cail_ioreg_read(struct card_info *info, uint32_t reg) > return r; > } > > +static ssize_t amdgpu_atombios_get_vbios_version(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct drm_device *ddev = dev_get_drvdata(dev); > + struct amdgpu_device *adev = ddev->dev_private; > + struct atom_context *ctx = adev->mode_info.atom_context; > + > + return snprintf(buf, PAGE_SIZE, "%s\n", ctx->vbios_version); } > + > +static DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version, > + NULL); > + > /** > * amdgpu_atombios_fini - free the driver info and callbacks for atombios > * > @@ -907,6 +921,7 @@ static void amdgpu_atombios_fini(struct amdgpu_device *adev) > adev->mode_info.atom_context = NULL; > kfree(adev->mode_info.atom_card_info); > adev->mode_info.atom_card_info = NULL; > + device_remove_file(adev->dev, &dev_attr_vbios_version); > } > > /** > @@ -923,6 +938,7 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev) > { > struct card_info *atom_card_info = > kzalloc(sizeof(struct card_info), GFP_KERNEL); > + int ret; > > if (!atom_card_info) > return -ENOMEM; > @@ -959,6 +975,13 @@ static int amdgpu_atombios_init(struct amdgpu_device *adev) > amdgpu_atombios_scratch_regs_init(adev); > amdgpu_atombios_allocate_fb_scratch(adev); > } > + > + ret = device_create_file(adev->dev, &dev_attr_vbios_version); > + if (ret) { > + DRM_ERROR("Failed to create device file for VBIOS version\n"); > + return ret; > + } > + > return 0; > } > > diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c > b/drivers/gpu/drm/amd/amdgpu/atom.c > index d69aa2e..69500a8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/atom.c > +++ b/drivers/gpu/drm/amd/amdgpu/atom.c > @@ -1343,8 +1343,11 @@ struct atom_context *amdgpu_atom_parse(struct card_info *card, void *bios) > idx = 0x80; > > str = CSTR(idx); > - if (*str != '\0') > + if (*str != '\0') { > pr_info("ATOM BIOS: %s\n", str); > + strlcpy(ctx->vbios_version, str, sizeof(ctx->vbios_version)); > + } > + > > return ctx; > } > diff --git a/drivers/gpu/drm/amd/amdgpu/atom.h > b/drivers/gpu/drm/amd/amdgpu/atom.h > index ddd8045..a391709 100644 > --- a/drivers/gpu/drm/amd/amdgpu/atom.h > +++ b/drivers/gpu/drm/amd/amdgpu/atom.h > @@ -140,6 +140,7 @@ struct atom_context { > int io_mode; > uint32_t *scratch; > int scratch_size_bytes; > + char vbios_version[20]; > }; > > extern int amdgpu_atom_debug;