Re: [PATCH] drm/amdgpu: Add vbios info ioctl interface

["StDenis, Tom" <Tom.StDenis@amd.com>]

[AMD Official Use Only - Internal Distribution Only]

Hi Will,

I've merged in your v2 patch last week.  If that's still the latest you should be good to go.

Tom

________________________________________
From: Gu, JiaWei (Will) <JiaWei.Gu@amd.com>
Sent: Wednesday, April 28, 2021 06:38
To: Christian König; Nieto, David M; amd-gfx@lists.freedesktop.org; StDenis, Tom
Cc: Deucher, Alexander
Subject: RE: [PATCH] drm/amdgpu: Add vbios info ioctl interface

[AMD Official Use Only - Internal Distribution Only]

Hi @StDenis, Tom<mailto:Tom.StDenis@amd.com>,

We have merged vbios info ioctl patch.
Could you help re-merge the UMR side one again if it was reverted before?

Thanks in advance!
Jiawei

From: Gu, JiaWei (Will)
Sent: Wednesday, April 28, 2021 4:21 PM
To: Christian König <ckoenig.leichtzumerken@gmail.com>; Nieto, David M <David.Nieto@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; StDenis, Tom <Tom.StDenis@amd.com>
Subject: RE: [PATCH] drm/amdgpu: Add vbios info ioctl interface


[AMD Official Use Only - Internal Distribution Only]

Thanks Christian,

I amended the commit message and resend the patch out.
Please feel free to let me know if message is not clear enough.

Best regards,
Jiawei

From: Christian König <ckoenig.leichtzumerken@gmail.com<mailto:ckoenig.leichtzumerken@gmail.com>>
Sent: Wednesday, April 28, 2021 3:43 PM
To: Nieto, David M <David.Nieto@amd.com<mailto:David.Nieto@amd.com>>; Gu, JiaWei (Will) <JiaWei.Gu@amd.com<mailto:JiaWei.Gu@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com<mailto:Alexander.Deucher@amd.com>>; StDenis, Tom <Tom.StDenis@amd.com<mailto:Tom.StDenis@amd.com>>
Subject: Re: [PATCH] drm/amdgpu: Add vbios info ioctl interface

Yeah, makes sense. Please note that in the commit message.

With that feel free to put an Acked-by: Christian König <christian.koenig@amd.com><mailto:christian.koenig@amd.com> on it.

Regards,
Christian.
Am 28.04.21 um 09:25 schrieb Nieto, David M:
I think this change may be orthogonal to that. Here we want to provide a way for the user application to get the VBIOS information without having to parse the binary…

And I agree that we should not have strong dependencies unless the encounter buggy VBIOS on the field, but I still think it is useful for the user to be able to display in a simple way the VBIOS version in their system if they happen to encounter an issue.

Regards,
David

From: Christian König <ckoenig.leichtzumerken@gmail.com><mailto:ckoenig.leichtzumerken@gmail.com>
Date: Wednesday, April 28, 2021 at 12:15 AM
To: "Nieto, David M" <David.Nieto@amd.com><mailto:David.Nieto@amd.com>, "Gu, JiaWei (Will)" <JiaWei.Gu@amd.com><mailto:JiaWei.Gu@amd.com>, "amd-gfx@lists.freedesktop.org"<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org><mailto:amd-gfx@lists.freedesktop.org>
Cc: "Deucher, Alexander" <Alexander.Deucher@amd.com><mailto:Alexander.Deucher@amd.com>, "StDenis, Tom" <Tom.StDenis@amd.com><mailto:Tom.StDenis@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Add vbios info ioctl interface

Well displaying the VBIOS information along with the other firmware in userspace is certainly useful.

We should just avoid making strong dependencies on that.

Firmware and VBIOS must always be backward compatible and the driver must always work with any released firmware and VBIOS combination.

What we can do is to display a warning when we detect and/or old/buggy firmware.

Regards,
Christian.
Am 28.04.21 um 08:47 schrieb Nieto, David M:
Besides system management, it provides parseable details on the VBIOS version and information. This is useful renderer information as the GPU behavior depends not only on the driver version but also on the firmwares running on the GPU.

The AMDGPU_INFO ioctl contains a method to get the versions of the ucode for all the IPs, but for VBIOS, only a way to dump the entire image is provided. While it Is possible to implement the whole logic of VBIOS parsing on userspace, it requires the application to have details on the header and table formats to parse the data. Moreover there is no guarantee that the format and header locations will remain constant across ASIC generations, so the maintainance cost and complexity seems unreasonable.

Providing a simple, and stable interface to the application seems to us like a sensible choice.

Thanks,
David

From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org><mailto:amd-gfx-bounces@lists.freedesktop.org> on behalf of "Gu, JiaWei (Will)" <JiaWei.Gu@amd.com><mailto:JiaWei.Gu@amd.com>
Date: Thursday, April 22, 2021 at 8:25 PM
To: Christian König <ckoenig.leichtzumerken@gmail.com><mailto:ckoenig.leichtzumerken@gmail.com>, "amd-gfx@lists.freedesktop.org"<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org><mailto:amd-gfx@lists.freedesktop.org>
Cc: "Deucher, Alexander" <Alexander.Deucher@amd.com><mailto:Alexander.Deucher@amd.com>, "StDenis, Tom" <Tom.StDenis@amd.com><mailto:Tom.StDenis@amd.com>, "Nieto, David M" <David.Nieto@amd.com><mailto:David.Nieto@amd.com>
Subject: RE: [PATCH] drm/amdgpu: Add vbios info ioctl interface


[AMD Official Use Only - Internal Distribution Only]

Hi Christian,
It will be used by a user space SMI-lib for GPU status query.

Hi David, please feel free to correct my statement since I’m not sure I have a view of whole picture.

Thanks,
Jiawei

From: Christian König <ckoenig.leichtzumerken@gmail.com><mailto:ckoenig.leichtzumerken@gmail.com>
Sent: Thursday, April 22, 2021 9:27 PM
To: Gu, JiaWei (Will) <JiaWei.Gu@amd.com><mailto:JiaWei.Gu@amd.com>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com><mailto:Alexander.Deucher@amd.com>; StDenis, Tom <Tom.StDenis@amd.com><mailto:Tom.StDenis@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Add vbios info ioctl interface

Is that useful to Vulkan/OpenGL/other clients in any way?

Christian.
Am 22.04.21 um 15:18 schrieb Gu, JiaWei (Will):



CC Tom.



On Apr 22, 2021, at 21:09, Gu, JiaWei (Will) <JiaWei.Gu@amd.com><mailto:JiaWei.Gu@amd.com> wrote:



<[v2][umr]add-vbios-info-query.patch>

UMR patch which calls this new IOCTL attached.



Best regards,

Jiawei



On Apr 22, 2021, at 10:34, Jiawei Gu <JiaWei.Gu@amd.com><mailto:JiaWei.Gu@amd.com> wrote:



Add AMDGPU_INFO_VBIOS_INFO subquery id for detailed vbios info.



Signed-off-by: Jiawei Gu <Jiawei.Gu@amd.com><mailto:Jiawei.Gu@amd.com>

---

drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    |  19 +++

drivers/gpu/drm/amd/amdgpu/atom.c          | 158 +++++++++++++++++++++

drivers/gpu/drm/amd/amdgpu/atom.h          |  11 ++

drivers/gpu/drm/amd/include/atomfirmware.h |  16 ++-

include/uapi/drm/amdgpu_drm.h              |  15 ++

5 files changed, 213 insertions(+), 6 deletions(-)



diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

index 39ee88d29cca..a20b016b05ab 100644

--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

@@ -861,6 +861,25 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)

                                       min((size_t)size, (size_t)(bios_size - bios_offset)))

                                   ? -EFAULT : 0;

             }

+            case AMDGPU_INFO_VBIOS_INFO: {

+                   struct drm_amdgpu_info_vbios vbios_info = {};

+                   struct atom_context *atom_context;

+

+                   atom_context = adev->mode_info.atom_context;

+                 memcpy(vbios_info.name, atom_context->name, sizeof(atom_context->name));

+                 vbios_info.dbdf = PCI_DEVID(adev->pdev->bus->number, adev->pdev->devfn);

+                 memcpy(vbios_info.vbios_pn, atom_context->vbios_pn, sizeof(atom_context->vbios_pn));

+                 vbios_info.version = atom_context->version;

+                 memcpy(vbios_info.date, atom_context->date, sizeof(atom_context->date));

+                 memcpy(vbios_info.serial, adev->serial, sizeof(adev->serial));

+                 vbios_info.dev_id = adev->pdev->device;

+                 vbios_info.rev_id = adev->pdev->revision;

+                 vbios_info.sub_dev_id = atom_context->sub_dev_id;

+                 vbios_info.sub_ved_id = atom_context->sub_ved_id;

+

+                   return copy_to_user(out, &vbios_info,

+                                  min((size_t)size, sizeof(vbios_info))) ? -EFAULT : 0;

+            }

             default:

                  DRM_DEBUG_KMS("Invalid request %d\n",

                            info->vbios_info.type);

diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c

index 3dcb8b32f48b..0e2f0ea13b40 100644

--- a/drivers/gpu/drm/amd/amdgpu/atom.c

+++ b/drivers/gpu/drm/amd/amdgpu/atom.c

@@ -31,6 +31,7 @@



#define ATOM_DEBUG



+#include "atomfirmware.h"

#include "atom.h"

#include "atom-names.h"

#include "atom-bits.h"

@@ -1299,12 +1300,153 @@ static void atom_index_iio(struct atom_context *ctx, int base)

     }

}



+static void atom_get_vbios_name(struct atom_context *ctx)

+{

+    unsigned char *p_rom;

+    unsigned char str_num;

+    unsigned short off_to_vbios_str;

+    unsigned char *c_ptr;

+    int name_size;

+    int i;

+

+    const char *na = "--N/A--";

+    char *back;

+

+    p_rom = ctx->bios;

+

+    str_num = *(p_rom + OFFSET_TO_GET_ATOMBIOS_NUMBER_OF_STRINGS);

+    if (str_num != 0) {

+            off_to_vbios_str =

+                   *(unsigned short *)(p_rom + OFFSET_TO_GET_ATOMBIOS_STRING_START);

+

+            c_ptr = (unsigned char *)(p_rom + off_to_vbios_str);

+    } else {

+            /* do not know where to find name */

+            memcpy(ctx->name, na, 7);

+            ctx->name[7] = 0;

+            return;

+    }

+

+    /*

+     * skip the atombios strings, usually 4

+     * 1st is P/N, 2nd is ASIC, 3rd is PCI type, 4th is Memory type

+     */

+    for (i = 0; i < str_num; i++) {

+            while (*c_ptr != 0)

+                   c_ptr++;

+            c_ptr++;

+    }

+

+    /* skip the following 2 chars: 0x0D 0x0A */

+    c_ptr += 2;

+

+    name_size = strnlen(c_ptr, STRLEN_LONG - 1);

+    memcpy(ctx->name, c_ptr, name_size);

+    back = ctx->name + name_size;

+    while ((*--back) == ' ')

+            ;

+    *(back + 1) = '\0';

+}

+

+static void atom_get_vbios_date(struct atom_context *ctx)

+{

+    unsigned char *p_rom;

+    unsigned char *date_in_rom;

+

+    p_rom = ctx->bios;

+

+    date_in_rom = p_rom + OFFSET_TO_VBIOS_DATE;

+

+    ctx->date[0] = '2';

+    ctx->date[1] = '0';

+    ctx->date[2] = date_in_rom[6];

+    ctx->date[3] = date_in_rom[7];

+    ctx->date[4] = '/';

+    ctx->date[5] = date_in_rom[0];

+    ctx->date[6] = date_in_rom[1];

+    ctx->date[7] = '/';

+    ctx->date[8] = date_in_rom[3];

+    ctx->date[9] = date_in_rom[4];

+    ctx->date[10] = ' ';

+    ctx->date[11] = date_in_rom[9];

+    ctx->date[12] = date_in_rom[10];

+    ctx->date[13] = date_in_rom[11];

+    ctx->date[14] = date_in_rom[12];

+    ctx->date[15] = date_in_rom[13];

+    ctx->date[16] = '\0';

+}

+

+static unsigned char *atom_find_str_in_rom(struct atom_context *ctx, char *str, int start,

+                                     int end, int maxlen)

+{

+    unsigned long str_off;

+    unsigned char *p_rom;

+    unsigned short str_len;

+

+    str_off = 0;

+    str_len = strnlen(str, maxlen);

+    p_rom = ctx->bios;

+

+    for (; start <= end; ++start) {

+            for (str_off = 0; str_off < str_len; ++str_off) {

+                   if (str[str_off] != *(p_rom + start + str_off))

+                        break;

+            }

+

+            if (str_off == str_len || str[str_off] == 0)

+                   return p_rom + start;

+    }

+    return NULL;

+}

+

+static void atom_get_vbios_pn(struct atom_context *ctx)

+{

+    unsigned char *p_rom;

+    unsigned short off_to_vbios_str;

+    unsigned char *vbios_str;

+    int count;

+

+    off_to_vbios_str = 0;

+    p_rom = ctx->bios;

+

+    if (*(p_rom + OFFSET_TO_GET_ATOMBIOS_NUMBER_OF_STRINGS) != 0) {

+            off_to_vbios_str =

+                   *(unsigned short *)(p_rom + OFFSET_TO_GET_ATOMBIOS_STRING_START);

+

+            vbios_str = (unsigned char *)(p_rom + off_to_vbios_str);

+    } else {

+            vbios_str = p_rom + OFFSET_TO_VBIOS_PART_NUMBER;

+    }

+

+    if (*vbios_str == 0) {

+            vbios_str = atom_find_str_in_rom(ctx, BIOS_ATOM_PREFIX, 3, 1024, 64);

+            if (vbios_str == NULL)

+                   vbios_str += sizeof(BIOS_ATOM_PREFIX) - 1;

+    }

+    if (vbios_str != NULL && *vbios_str == 0)

+            vbios_str++;

+

+    if (vbios_str != NULL) {

+            count = 0;

+            while ((count < BIOS_STRING_LENGTH) && vbios_str[count] >= ' ' &&

+                   vbios_str[count] <= 'z') {

+                 ctx->vbios_pn[count] = vbios_str[count];

+                   count++;

+            }

+

+            ctx->vbios_pn[count] = 0;

+    }

+}

+

struct atom_context *amdgpu_atom_parse(struct card_info *card, void *bios)

{

     int base;

     struct atom_context *ctx =

         kzalloc(sizeof(struct atom_context), GFP_KERNEL);

     char *str;

+    struct _ATOM_ROM_HEADER *atom_rom_header;

+    struct _ATOM_MASTER_DATA_TABLE *master_table;

+    struct _ATOM_FIRMWARE_INFO *atom_fw_info;

     u16 idx;



     if (!ctx)

@@ -1353,6 +1495,22 @@ struct atom_context *amdgpu_atom_parse(struct card_info *card, void *bios)

          strlcpy(ctx->vbios_version, str, sizeof(ctx->vbios_version));

     }



+    atom_rom_header = (struct _ATOM_ROM_HEADER *)CSTR(base);

+    ctx->sub_dev_id = atom_rom_header->usSubsystemVendorID;

+    ctx->sub_ved_id = atom_rom_header->usSubsystemID;

+    if (atom_rom_header->usMasterDataTableOffset != 0) {

+            master_table = (struct _ATOM_MASTER_DATA_TABLE *)

+                    CSTR(atom_rom_header->usMasterDataTableOffset);

+            if (master_table->ListOfDataTables.FirmwareInfo != 0) {

+                   atom_fw_info = (struct _ATOM_FIRMWARE_INFO *)

+                           CSTR(master_table->ListOfDataTables.FirmwareInfo);

+                   ctx->version = atom_fw_info->ulFirmwareRevision;

+            }

+    }

+

+    atom_get_vbios_name(ctx);

+    atom_get_vbios_pn(ctx);

+    atom_get_vbios_date(ctx);



     return ctx;

}

diff --git a/drivers/gpu/drm/amd/amdgpu/atom.h b/drivers/gpu/drm/amd/amdgpu/atom.h

index d279759cab47..c729f7ceba4f 100644

--- a/drivers/gpu/drm/amd/amdgpu/atom.h

+++ b/drivers/gpu/drm/amd/amdgpu/atom.h

@@ -112,6 +112,10 @@ struct drm_device;

#define ATOM_IO_SYSIO              2

#define ATOM_IO_IIO         0x80



+#define STRLEN_NORMAL             32

+#define STRLEN_LONG        64

+#define STRLEN_VERYLONG           254

+

struct card_info {

     struct drm_device *dev;

     void (* reg_write)(struct card_info *, uint32_t, uint32_t);   /*  filled by driver */

@@ -140,6 +144,13 @@ struct atom_context {

     uint32_t *scratch;

     int scratch_size_bytes;

     char vbios_version[20];

+

+    uint8_t name[STRLEN_LONG];

+    uint8_t vbios_pn[STRLEN_LONG];

+    uint32_t version;

+    uint8_t date[STRLEN_NORMAL];

+    uint32_t sub_dev_id;

+    uint32_t sub_ved_id;

};



extern int amdgpu_atom_debug;

diff --git a/drivers/gpu/drm/amd/include/atomfirmware.h b/drivers/gpu/drm/amd/include/atomfirmware.h

index c77ed38c20fb..3a8e404099b0 100644

--- a/drivers/gpu/drm/amd/include/atomfirmware.h

+++ b/drivers/gpu/drm/amd/include/atomfirmware.h

@@ -197,6 +197,8 @@ enum atom_dp_vs_preemph_def{

 DP_VS_LEVEL0_PREEMPH_LEVEL3 = 0x18,

};



+#define BIOS_ATOM_PREFIX   "ATOMBIOS"

+#define BIOS_STRING_LENGTH 43



/*

enum atom_string_def{

@@ -209,12 +211,14 @@ atom_bios_string          = "ATOM"

#pragma pack(1)                          /* BIOS data must use byte aligment*/



enum atombios_image_offset{

-OFFSET_TO_ATOM_ROM_HEADER_POINTER          =0x00000048,

-OFFSET_TO_ATOM_ROM_IMAGE_SIZE              =0x00000002,

-OFFSET_TO_ATOMBIOS_ASIC_BUS_MEM_TYPE       =0x94,

-MAXSIZE_OF_ATOMBIOS_ASIC_BUS_MEM_TYPE      =20,  /*including the terminator 0x0!*/

-OFFSET_TO_GET_ATOMBIOS_NUMBER_OF_STRINGS   =0x2f,

-OFFSET_TO_GET_ATOMBIOS_STRING_START        =0x6e,

+  OFFSET_TO_ATOM_ROM_HEADER_POINTER          = 0x00000048,

+  OFFSET_TO_ATOM_ROM_IMAGE_SIZE              = 0x00000002,

+  OFFSET_TO_ATOMBIOS_ASIC_BUS_MEM_TYPE       = 0x94,

+  MAXSIZE_OF_ATOMBIOS_ASIC_BUS_MEM_TYPE      = 20,  /*including the terminator 0x0!*/

+  OFFSET_TO_GET_ATOMBIOS_NUMBER_OF_STRINGS   = 0x2f,

+  OFFSET_TO_GET_ATOMBIOS_STRING_START        = 0x6e,

+  OFFSET_TO_VBIOS_PART_NUMBER                = 0x80,

+  OFFSET_TO_VBIOS_DATE                       = 0x50,

};



/****************************************************************************

diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h

index 8b832f7458f2..77980445d486 100644

--- a/include/uapi/drm/amdgpu_drm.h

+++ b/include/uapi/drm/amdgpu_drm.h

@@ -758,6 +758,8 @@ struct drm_amdgpu_cs_chunk_data {

     #define AMDGPU_INFO_VBIOS_SIZE             0x1

     /* Subquery id: Query vbios image */

     #define AMDGPU_INFO_VBIOS_IMAGE            0x2

+    /* Subquery id: Query vbios info */

+    #define AMDGPU_INFO_VBIOS_INFO             0x3

/* Query UVD handles */

#define AMDGPU_INFO_NUM_HANDLES                    0x1C

/* Query sensor related information */

@@ -951,6 +953,19 @@ struct drm_amdgpu_info_firmware {

     __u32 feature;

};



+struct drm_amdgpu_info_vbios {

+    __u8 name[64];

+    __u32 dbdf;

+    __u8 vbios_pn[64];

+    __u32 version;

+    __u8 date[32];

+    __u8 serial[16];

+    __u32 dev_id;

+    __u32 rev_id;

+    __u32 sub_dev_id;

+    __u32 sub_ved_id;

+};

+

#define AMDGPU_VRAM_TYPE_UNKNOWN 0

#define AMDGPU_VRAM_TYPE_GDDR1 1

#define AMDGPU_VRAM_TYPE_DDR2  2

--

2.17.1








_______________________________________________

amd-gfx mailing list

amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>

https://lists.freedesktop.org/mailman/listinfo/amd-gfx<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7CJiaWei.Gu%40amd.com%7Cfc8d3420d88745c257bf08d90a1934d6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637551925654186287%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Y9%2B4WCgVSr0pS%2Bdf5PzTCddGhjitFaCsNhp%2Fu%2F%2FnmII%3D&reserved=0>



_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx