Hi Kent,
Loosely following this thread. Was there a decision on whether to leave
this in debugfs or sysfs?
I'd like to throw the version string in umr's config output (umr -c) :-)
Cheers,
Tom
On 24/08/17 05:30 PM, Russell, Kent wrote:
The plan is for the vbios version to be available through the ROCM-SMI
utility. We have the GPU power usage listed in the debugfs currently. If
they are both to be used for a userspace utility, should we be moving
both of those to sysfs instead?
Kent
KENT RUSSELL
Software Engineer | Vertical Workstation/Compute
1 Commerce Valley Drive East
Markham, ON L3T 7X6
Canada
O +(1) 289-695-2122 | Ext x72122
------------------------------------------------------------------------
*From:* Christian König <deathsim...@vodafone.de>
*Sent:* Thursday, August 24, 2017 2:10:35 PM
*To:* Russell, Kent; Alex Deucher; Kuehling, Felix
*Cc:* amd-gfx@lists.freedesktop.org
*Subject:* Re: [PATCH] drm/amdgpu: Add sysfs file for VBIOS
Actually the main difference is not root vs. user, but rather unstable
vs stable interface.
If you want a stable interface for an userspace tool you should use
sysfs, if you don't care about that you should use debugfs.
Christian.
Am 24.08.2017 um 18:37 schrieb Russell, Kent:
We already access the GPU Power usage via debugfs, which is why I didn't think
it was a huge deal to switch it over, since we already need root access for the
SMI due to that.
Kent
-----Original Message-----
From: Alex Deucher [mailto:alexdeuc...@gmail.com]
Sent: Thursday, August 24, 2017 11:39 AM
To: Kuehling, Felix
Cc: Russell, Kent; Christian König; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: Add sysfs file for VBIOS
On Thu, Aug 24, 2017 at 11:35 AM, Kuehling, Felix <felix.kuehl...@amd.com>
wrote:
Debugfs is only accessible by Root. The BIOS version is already reported in the
kernel log, which is visible to everyone. So why hide this away in debugfs?
I think Kent's intention is to add VBIOS version reporting to the rocm-smi
tool. I'd prefer using a stable interface like sysfs, and one that doesn't
require root access in a tool like rocm-smi.
Ah, ok, sysfs is fine in that case. I thought this was just general debugging
stuff.
Alex
Regards,
Felix
________________________________________
From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> on behalf of
Russell, Kent <kent.russ...@amd.com>
Sent: Thursday, August 24, 2017 9:06:22 AM
To: Alex Deucher
Cc: Christian König; amd-gfx@lists.freedesktop.org
Subject: RE: [PATCH] drm/amdgpu: Add sysfs file for VBIOS
I can definitely add that as well.
Kent
-----Original Message-----
From: Alex Deucher [mailto:alexdeuc...@gmail.com]
Sent: Thursday, August 24, 2017 8:56 AM
To: Russell, Kent
Cc: Christian König; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: Add sysfs file for VBIOS
On Thu, Aug 24, 2017 at 5:58 AM, Russell, Kent <kent.russ...@amd.com> wrote:
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!
While you are at it, can you expose the vbios binary itself via debugfs?
That's been on by todo list for a while.
Alex
Kent
-----Original Message-----
From: Christian König [mailto:deathsim...@vodafone.de]
Sent: Thursday, August 24, 2017 2:22 AM
To: Russell, Kent; amd-gfx@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.russ...@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;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
