Christian was saying that the VBIOS binary should remain in debugfs (on the other thread). I'll let you two discuss, since I have zero vested interest in the location of the VBIOS binary :) Kent -----Original Message----- From: Alex Deucher [mailto:alexdeucher@xxxxxxxxx] Sent: Friday, August 25, 2017 10:41 AM To: Russell, Kent Cc: amd-gfx list Subject: Re: [PATCH] drm/amdgpu: Move VBIOS version to sysfs On Fri, Aug 25, 2017 at 9:43 AM, Kent Russell <kent.russell at amd.com> wrote: > sysfs is more stable, and doesn't require root to access > > Signed-off-by: Kent Russell <kent.russell at amd.com> Might as well move the vbios binary itself to sysfs as well. It should be a stable interface :) Alex > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 54 > +++++++++++++----------------- > 1 file changed, 23 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index c041496..77929e4 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -67,7 +67,6 @@ static int amdgpu_debugfs_regs_init(struct > amdgpu_device *adev); static void amdgpu_debugfs_regs_cleanup(struct > amdgpu_device *adev); static int > amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device *adev); static > int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev); > -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device > *adev); > > static const char *amdgpu_asic_name[] = { > "TAHITI", > @@ -890,6 +889,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 > * > @@ -909,6 +922,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); > } > > /** > @@ -925,6 +939,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; > @@ -961,6 +976,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; > } > > @@ -2256,10 +2278,6 @@ int amdgpu_device_init(struct amdgpu_device *adev, > if (r) > DRM_ERROR("Creating vbios dump debugfs failed > (%d).\n", r); > > - r = amdgpu_debugfs_vbios_version_init(adev); > - if (r) > - DRM_ERROR("Creating vbios version debugfs failed (%d).\n", r); > - > if ((amdgpu_testing & 1)) { > if (adev->accel_working) > amdgpu_test_moves(adev); @@ -3851,39 +3869,17 > @@ static int amdgpu_debugfs_get_vbios_dump(struct seq_file *m, void *data) > return 0; > } > > -static int amdgpu_debugfs_get_vbios_version(struct seq_file *m, void > *data) -{ > - struct drm_info_node *node = (struct drm_info_node *) m->private; > - struct drm_device *dev = node->minor->dev; > - struct amdgpu_device *adev = dev->dev_private; > - struct atom_context *ctx = adev->mode_info.atom_context; > - > - seq_printf(m, "%s\n", ctx->vbios_version); > - return 0; > -} > - > static const struct drm_info_list amdgpu_vbios_dump_list[] = { > {"amdgpu_vbios", > amdgpu_debugfs_get_vbios_dump, > 0, NULL}, > }; > > -static const struct drm_info_list amdgpu_vbios_version_list[] = { > - {"amdgpu_vbios_version", > - amdgpu_debugfs_get_vbios_version, > - 0, NULL}, > -}; > - > static int amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev) > { > return amdgpu_debugfs_add_files(adev, > amdgpu_vbios_dump_list, 1); } > -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device > *adev) -{ > - return amdgpu_debugfs_add_files(adev, > - amdgpu_vbios_version_list, 1); > -} > #else > static int amdgpu_debugfs_test_ib_ring_init(struct amdgpu_device > *adev) { @@ -3897,9 +3893,5 @@ static int > amdgpu_debugfs_vbios_dump_init(struct amdgpu_device *adev) { > return 0; > } > -static int amdgpu_debugfs_vbios_version_init(struct amdgpu_device > *adev) -{ > - return 0; > -} > static void amdgpu_debugfs_regs_cleanup(struct amdgpu_device *adev) { > } #endif > -- > 2.7.4 > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx