Re: [PATCH] drm/amd: Avoid reading the VBIOS part number twice
On Tue, Jul 18, 2023 at 2:03 PM Mario Limonciello
wrote:
>
> The VBIOS part number is read both in amdgpu_atom_parse() as well
> as in atom_get_vbios_pn() and stored twice in the `struct atom_context`
> structure. Remove the first unnecessary read and move the `pr_info`
> line from that read into the second.
>
> Signed-off-by: Mario Limonciello
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +-
> .../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c| 20 +--
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 8
> .../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 8
> drivers/gpu/drm/amd/amdgpu/atom.c | 13 ++--
> drivers/gpu/drm/amd/amdgpu/atom.h | 2 --
> 7 files changed, 22 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> index f4e3c133a16ca..dce9e7d5e4ec6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
> @@ -1776,7 +1776,7 @@ static ssize_t amdgpu_atombios_get_vbios_version(struct
> device *dev,
> struct amdgpu_device *adev = drm_to_adev(ddev);
> struct atom_context *ctx = adev->mode_info.atom_context;
>
> - return sysfs_emit(buf, "%s\n", ctx->vbios_version);
> + return sysfs_emit(buf, "%s\n", ctx->vbios_pn);
> }
>
> static DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> index 4620c4712ce32..c9f16eab0f3d0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
> @@ -60,10 +60,10 @@ static bool is_fru_eeprom_supported(struct amdgpu_device
> *adev, u32 *fru_addr)
> switch (adev->asic_type) {
> case CHIP_VEGA20:
> /* D161 and D163 are the VG20 server SKUs */
> - if (strnstr(atom_ctx->vbios_version, "D161",
> - sizeof(atom_ctx->vbios_version)) ||
> - strnstr(atom_ctx->vbios_version, "D163",
> - sizeof(atom_ctx->vbios_version))) {
> + if (strnstr(atom_ctx->vbios_pn, "D161",
> + sizeof(atom_ctx->vbios_pn)) ||
> + strnstr(atom_ctx->vbios_pn, "D163",
> + sizeof(atom_ctx->vbios_pn))) {
> if (fru_addr)
> *fru_addr = FRU_EEPROM_MADDR_6;
> return true;
> @@ -72,16 +72,16 @@ static bool is_fru_eeprom_supported(struct amdgpu_device
> *adev, u32 *fru_addr)
> }
> case CHIP_ALDEBARAN:
> /* All Aldebaran SKUs have an FRU */
> - if (!strnstr(atom_ctx->vbios_version, "D673",
> -sizeof(atom_ctx->vbios_version)))
> + if (!strnstr(atom_ctx->vbios_pn, "D673",
> +sizeof(atom_ctx->vbios_pn)))
> if (fru_addr)
> *fru_addr = FRU_EEPROM_MADDR_6;
> return true;
> case CHIP_SIENNA_CICHLID:
> - if (strnstr(atom_ctx->vbios_version, "D603",
> - sizeof(atom_ctx->vbios_version))) {
> - if (strnstr(atom_ctx->vbios_version, "D603GLXE",
> - sizeof(atom_ctx->vbios_version))) {
> + if (strnstr(atom_ctx->vbios_pn, "D603",
> + sizeof(atom_ctx->vbios_pn))) {
> + if (strnstr(atom_ctx->vbios_pn, "D603GLXE",
> + sizeof(atom_ctx->vbios_pn))) {
> return false;
> } else {
> if (fru_addr)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index cca5a495611f3..46844082762a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -1719,7 +1719,7 @@ static int amdgpu_debugfs_firmware_info_show(struct
> seq_file *m, void *unused)
> seq_printf(m, "MES feature version: %u, firmware version: 0x%08x\n",
>fw_info.feature, fw_info.ver);
>
> - seq_printf(m, "VBIOS version: %s\n", ctx->vbios_version);
> + seq_printf(m, "VBIOS version: %s\n", ctx->vbios_pn);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 8aaa427f8c0f6..5055c14d6b426 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -2440,10 +2440,10 @@ static void amdgpu_ras_get_quirks(struct
> amdgpu_device *adev)
> if (!ctx)
>
[PATCH] drm/amd: Avoid reading the VBIOS part number twice
The VBIOS part number is read both in amdgpu_atom_parse() as well
as in atom_get_vbios_pn() and stored twice in the `struct atom_context`
structure. Remove the first unnecessary read and move the `pr_info`
line from that read into the second.
Signed-off-by: Mario Limonciello
---
drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c | 2 +-
.../gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c| 20 +--
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 8
.../gpu/drm/amd/amdgpu/amdgpu_ras_eeprom.c| 8
drivers/gpu/drm/amd/amdgpu/atom.c | 13 ++--
drivers/gpu/drm/amd/amdgpu/atom.h | 2 --
7 files changed, 22 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
index f4e3c133a16ca..dce9e7d5e4ec6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c
@@ -1776,7 +1776,7 @@ static ssize_t amdgpu_atombios_get_vbios_version(struct
device *dev,
struct amdgpu_device *adev = drm_to_adev(ddev);
struct atom_context *ctx = adev->mode_info.atom_context;
- return sysfs_emit(buf, "%s\n", ctx->vbios_version);
+ return sysfs_emit(buf, "%s\n", ctx->vbios_pn);
}
static DEVICE_ATTR(vbios_version, 0444, amdgpu_atombios_get_vbios_version,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
index 4620c4712ce32..c9f16eab0f3d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fru_eeprom.c
@@ -60,10 +60,10 @@ static bool is_fru_eeprom_supported(struct amdgpu_device
*adev, u32 *fru_addr)
switch (adev->asic_type) {
case CHIP_VEGA20:
/* D161 and D163 are the VG20 server SKUs */
- if (strnstr(atom_ctx->vbios_version, "D161",
- sizeof(atom_ctx->vbios_version)) ||
- strnstr(atom_ctx->vbios_version, "D163",
- sizeof(atom_ctx->vbios_version))) {
+ if (strnstr(atom_ctx->vbios_pn, "D161",
+ sizeof(atom_ctx->vbios_pn)) ||
+ strnstr(atom_ctx->vbios_pn, "D163",
+ sizeof(atom_ctx->vbios_pn))) {
if (fru_addr)
*fru_addr = FRU_EEPROM_MADDR_6;
return true;
@@ -72,16 +72,16 @@ static bool is_fru_eeprom_supported(struct amdgpu_device
*adev, u32 *fru_addr)
}
case CHIP_ALDEBARAN:
/* All Aldebaran SKUs have an FRU */
- if (!strnstr(atom_ctx->vbios_version, "D673",
-sizeof(atom_ctx->vbios_version)))
+ if (!strnstr(atom_ctx->vbios_pn, "D673",
+sizeof(atom_ctx->vbios_pn)))
if (fru_addr)
*fru_addr = FRU_EEPROM_MADDR_6;
return true;
case CHIP_SIENNA_CICHLID:
- if (strnstr(atom_ctx->vbios_version, "D603",
- sizeof(atom_ctx->vbios_version))) {
- if (strnstr(atom_ctx->vbios_version, "D603GLXE",
- sizeof(atom_ctx->vbios_version))) {
+ if (strnstr(atom_ctx->vbios_pn, "D603",
+ sizeof(atom_ctx->vbios_pn))) {
+ if (strnstr(atom_ctx->vbios_pn, "D603GLXE",
+ sizeof(atom_ctx->vbios_pn))) {
return false;
} else {
if (fru_addr)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index cca5a495611f3..46844082762a6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1719,7 +1719,7 @@ static int amdgpu_debugfs_firmware_info_show(struct
seq_file *m, void *unused)
seq_printf(m, "MES feature version: %u, firmware version: 0x%08x\n",
fw_info.feature, fw_info.ver);
- seq_printf(m, "VBIOS version: %s\n", ctx->vbios_version);
+ seq_printf(m, "VBIOS version: %s\n", ctx->vbios_pn);
return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 8aaa427f8c0f6..5055c14d6b426 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -2440,10 +2440,10 @@ static void amdgpu_ras_get_quirks(struct amdgpu_device
*adev)
if (!ctx)
return;
- if (strnstr(ctx->vbios_version, "D16406",
- sizeof(ctx->vbios_version)) ||
- strnstr(ctx->vbios_version, "D36002",
- sizeof(ctx->vbios_version)))
+ if (strnstr(ctx->vbios_pn, "D16406",
+
