Kudos should go to Nirmoy who mentioned that tool in one of our discussions. But, yeah it is rather useful :) Regards, Christian. Am 20.05.21 um 03:30 schrieb Gu, JiaWei (Will): > > [AMD Official Use Only - Internal Distribution Only] > > Thanks Christian! > > Happy to learn new tricks. > > Best regards, > > Jiawei > > *From:* Christian König > *Sent:* Wednesday, May 19, 2021 9:23 PM > *To:* Deucher, Alexander ; Gu, JiaWei > (Will) ; Nieto, David M ; > Koenig, Christian ; > amd-gfx@lists.freedesktop.org; maraeo@gmail.com > *Cc:* Deng, Emily > *Subject:* Re: [PATCH] drm/amdgpu: Add vbios info ioctl interface > > Good point. > > If you want to double check the alignment you can use something like > "pahole drivers/gpu/drm/amd/amdgpu/amdgpu.ko -C drm_amdgpu_info_vbios" > after building the kernel module. > > Regards, > Christian. > > Am 19.05.21 um 15:09 schrieb Deucher, Alexander: > > [Public] > > The structure is not 64 bit aligned.  I think you want something like: > > > +struct drm_amdgpu_info_vbios { > > +     __u8 name[64]; > > +     __u8 vbios_pn[64]; > > +     __u32 version; > > > +     __u32 pad; > > > +     __u8 vbios_ver_str[32]; > > +     __u8 date[32]; > > +}; > > ------------------------------------------------------------------------ > > *From:*Gu, JiaWei (Will) > > *Sent:* Tuesday, May 18, 2021 1:58 AM > *To:* Nieto, David M > ; Koenig, Christian > ; > amd-gfx@lists.freedesktop.org > > > ; maraeo@gmail.com > > ; Deucher, Alexander > > *Cc:* Deng, Emily > *Subject:* RE: [PATCH] drm/amdgpu: Add vbios info ioctl interface > > [Public] > > Hi all, > > Then the struct looks like: > > > +struct drm_amdgpu_info_vbios { > > +     __u8 name[64]; > > +     __u8 vbios_pn[64]; > > +     __u32 version; > > +     __u8 vbios_ver_str[32]; > > +     __u8 date[32]; > > +}; > > Sample output: > > vbios name : NAVI12 A0 XT D30501 8GB EVAL 1150e/334m HYN/SAM > vbios pn : 113-D3050100-104 > vbios version : 285409288 > vbios ver_str : 017.003.000.008.016956 > vbios date : 2021/05/03 23:32 > > Please help double confirm that we’re all fine with it and there’s > no need to add & remove anything. > > Best regards, > > Jiawei > > *From:* Nieto, David M > > *Sent:* Tuesday, May 18, 2021 12:40 PM > *To:* Gu, JiaWei (Will) > ; Koenig, Christian > ; > amd-gfx@lists.freedesktop.org > ; maraeo@gmail.com > ; Deucher, Alexander > > *Cc:* Deng, Emily > *Subject:* Re: [PATCH] drm/amdgpu: Add vbios info ioctl interface > > [Public] > > Yes, let's remove that too, > > Thanks, > > David > > ------------------------------------------------------------------------ > > *From:*Gu, JiaWei (Will) > > *Sent:* Monday, May 17, 2021 8:07 PM > *To:* Nieto, David M >; Koenig, Christian > >; > amd-gfx@lists.freedesktop.org > > >; maraeo@gmail.com > >; Deucher, Alexander > > > *Cc:* Deng, Emily > > *Subject:* RE: [PATCH] drm/amdgpu: Add vbios info ioctl interface > > [AMD Official Use Only - Internal Distribution Only] > > OK let’s remove serial. > > dbdf comes from this: > > vbios_info.dbdf = PCI_DEVID(adev->pdev->bus->number, > adev->pdev->devfn); > > I think we can remove dbdf as well. > > Best regards, > > Jiawei > > *From:* Nieto, David M > > *Sent:* Tuesday, May 18, 2021 10:45 AM > *To:* Gu, JiaWei (Will) >; Koenig, Christian > >; > amd-gfx@lists.freedesktop.org > ; maraeo@gmail.com > ; Deucher, Alexander > > > *Cc:* Deng, Emily > > *Subject:* Re: [PATCH] drm/amdgpu: Add vbios info ioctl interface > > [AMD Official Use Only - Internal Distribution Only] > > The serial number is ASIC information, not VBIOS information, and > it is still available as a sysfs node... I don't think we should > put it there. > > Not sure what dbdf stands for. > > ------------------------------------------------------------------------ > > *From:*Gu, JiaWei (Will) > > *Sent:* Monday, May 17, 2021 7:11 PM > *To:* Koenig, Christian >; amd-gfx@lists.freedesktop.org > > >; Nieto, David M > >; > maraeo@gmail.com >; Deucher, Alexander > > > *Cc:* Deng, Emily > > *Subject:* RE: [PATCH] drm/amdgpu: Add vbios info ioctl interface > > [AMD Official Use Only - Internal Distribution Only] > > So I guess the dbdf is also needed to be removed? > And how about serial? > > > +struct drm_amdgpu_info_vbios { > > +     __u8 name[64]; > > +     __u32 dbdf; // do we need this? > > +     __u8 vbios_pn[64]; > > +     __u32 version; > > +     __u8 vbios_ver_str[32]; > > +     __u8 date[32]; > > +     __u64 serial; // do we need this? > > +}; > > Best regards, > Jiawei > > -----Original Message----- > From: Koenig, Christian > > Sent: Monday, May 17, 2021 8:26 PM > To: Gu, JiaWei (Will) >; amd-gfx@lists.freedesktop.org > ; Nieto, David M > >; > maraeo@gmail.com ; Deucher, Alexander > > > Cc: Deng, Emily > > Subject: Re: [PATCH] drm/amdgpu: Add vbios info ioctl interface > > I'm not very familiar with the technical background why we have > the fields here once more. > > But of hand we should at least remove everything which is also > available from the PCI information. > > E.g. dev_id, rev_id, sub_dev_id, sub_ved_id. > > Regards, > Christian. > > Am 17.05.21 um 14:17 schrieb Gu, JiaWei (Will): > > [AMD Official Use Only - Internal Distribution Only] > > > > Hi all, > > > > Thanks Christian's suggestion. > > I reverted the previous patches and squash them into this single > one. > > > > As this patch shows, the current uapi change looks like this: > > > > +struct drm_amdgpu_info_vbios { > > +     __u8 name[64]; > > +     __u32 dbdf; > > +     __u8 vbios_pn[64]; > > +     __u32 version; > > +     __u8 vbios_ver_str[32]; > > +     __u8 date[32]; > > +     __u64 serial; > > +     __u32 dev_id; > > +     __u32 rev_id; > > +     __u32 sub_dev_id; > > +     __u32 sub_ved_id; > > +}; > > > > As we know there's some redundant info in this struct. > > Please feel free to give any comments or suggestion about what > it should & shouldn't include. > > > > Best regards, > > Jiawei > > > > -----Original Message----- > > From: Jiawei Gu > > > Sent: Monday, May 17, 2021 8:08 PM > > To: amd-gfx@lists.freedesktop.org > ; Koenig, Christian > > >; > Nieto, David M >; > > maraeo@gmail.com ; Deucher, Alexander > > > > Cc: Deng, Emily >; Gu, JiaWei (Will) > > > > > Subject: [PATCH] drm/amdgpu: Add vbios info ioctl interface > > > > Add AMDGPU_INFO_VBIOS_INFO subquery id for detailed vbios info. > > > > Provides a way for the user application to get the VBIOS > information without having to parse the binary. > > 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. > > > > V2: > > Use numeric serial. > > Parse and expose vbios version string. > > > > Signed-off-by: Jiawei Gu > > > Acked-by: Christian König > > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  21 +++ > > drivers/gpu/drm/amd/amdgpu/atom.c | 174 +++++++++++++++++++++ > > drivers/gpu/drm/amd/amdgpu/atom.h |  12 ++ > > drivers/gpu/drm/amd/include/atomfirmware.h |   5 + > > include/uapi/drm/amdgpu_drm.h |  16 ++ > >   5 files changed, 228 insertions(+) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > index 8d12e474745a..30e4fed3de22 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > > @@ -861,6 +861,27 @@ 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.vbios_ver_str, atom_context->vbios_ver_str, > > + sizeof(atom_context->vbios_ver_str)); > > + memcpy(vbios_info.date, atom_context->date, > sizeof(atom_context->date)); > > + vbios_info.serial = adev->unique_id; > > + 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..542b2c2414e4 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,168 @@ 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; > > +     } > > +} > > + > > +static void atom_get_vbios_version(struct atom_context *ctx) { > > +     unsigned char *vbios_ver; > > + > > +     /* find anchor ATOMBIOSBK-AMD */ > > +     vbios_ver = atom_find_str_in_rom(ctx, BIOS_VERSION_PREFIX, > 3, 1024, 64); > > +     if (vbios_ver != NULL) { > > +             /* skip ATOMBIOSBK-AMD VER */ > > +             vbios_ver += 18; > > + memcpy(ctx->vbios_ver_str, vbios_ver, STRLEN_NORMAL); > > +     } else { > > + ctx->vbios_ver_str[0] = '\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 +1510,23 @@ 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); > > +     atom_get_vbios_version(ctx); > > > >        return ctx; > >   } > > diff --git a/drivers/gpu/drm/amd/amdgpu/atom.h > > b/drivers/gpu/drm/amd/amdgpu/atom.h > > index d279759cab47..6463ce6e756d 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,14 @@ 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 vbios_ver_str[STRLEN_NORMAL]; > > +     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 275468e4be60..28deecc2f990 100644 > > --- a/drivers/gpu/drm/amd/include/atomfirmware.h > > +++ b/drivers/gpu/drm/amd/include/atomfirmware.h > > @@ -197,6 +197,9 @@ enum atom_dp_vs_preemph_def{ > >     DP_VS_LEVEL0_PREEMPH_LEVEL3 = 0x18, > >   }; > > > > +#define BIOS_ATOM_PREFIX "ATOMBIOS" > > +#define BIOS_VERSION_PREFIX "ATOMBIOSBK-AMD" > > +#define BIOS_STRING_LENGTH 43 > > > >   /* > >   enum atom_string_def{ > > @@ -215,6 +218,8 @@ enum atombios_image_offset{ > > 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 9169df7fadee..e0f98ca9a755 > > 100644 > > --- a/include/uapi/drm/amdgpu_drm.h > > +++ b/include/uapi/drm/amdgpu_drm.h > > @@ -756,6 +756,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 */ @@ -949,6 +951,20 @@ > struct > > drm_amdgpu_info_firmware { > >        __u32 feature; > >   }; > > > > +struct drm_amdgpu_info_vbios { > > +     __u8 name[64]; > > +     __u32 dbdf; > > +     __u8 vbios_pn[64]; > > +     __u32 version; > > +     __u8 vbios_ver_str[32]; > > +     __u8 date[32]; > > +     __u64 serial; > > +     __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 > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx >