[PATCH] drm/amdgpu: Add sysfs file for VBIOS
Alex Deucher
alexdeucher at gmail.com
Thu Aug 24 15:39:06 UTC 2017
On Thu, Aug 24, 2017 at 11:35 AM, Kuehling, Felix
<Felix.Kuehling at 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-bounces at lists.freedesktop.org> on behalf of Russell, Kent <Kent.Russell at amd.com>
> Sent: Thursday, August 24, 2017 9:06:22 AM
> To: Alex Deucher
> Cc: Christian König; amd-gfx at 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:alexdeucher at gmail.com]
> Sent: Thursday, August 24, 2017 8:56 AM
> To: Russell, Kent
> Cc: Christian König; amd-gfx at 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.Russell at 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:deathsimple at vodafone.de]
>> 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;
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list