| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <59ab4749-8fb9-c0d5-1f7e-d5fceda85063@amd.com>
Date: Wed, 4 Jan 2023 10:24:07 +0530
From: "Lazar, Lijo" <lijo.lazar@....com>
To: Mario Limonciello <mario.limonciello@....com>,
Alex Deucher <alexander.deucher@....com>,
linux-kernel@...r.kernel.org
Cc: Javier Martinez Canillas <javierm@...hat.com>,
Carlos Soriano Sanchez <csoriano@...hat.com>,
amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>, christian.koenig@....com,
"Pan, Xinhui" <Xinhui.Pan@....com>
Subject: Re: [PATCH v4 07/27] drm/amd: Convert SDMA to use
`amdgpu_ucode_ip_version_decode`
On 1/4/2023 3:48 AM, Mario Limonciello wrote:
> Simplifies the code so that all SDMA versions will get the firmware
> name from `amdgpu_ucode_ip_version_decode`.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> ---
> v3->v4:
> * Move out of IP discovery and instead simplify early_init
> v2->v3:
> * Fix dGPU naming scheme
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 7 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h | 4 +-
> drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 47 +-------------------
> drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 30 +------------
> drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 55 +-----------------------
> drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 25 +----------
> 6 files changed, 13 insertions(+), 155 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> index 9e85a078d918..83e8f0dae647 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> @@ -200,15 +200,18 @@ void amdgpu_sdma_destroy_inst_ctx(struct amdgpu_device *adev,
> }
>
> int amdgpu_sdma_init_microcode(struct amdgpu_device *adev,
> - char *fw_name, u32 instance,
> - bool duplicate)
> + u32 instance, bool duplicate)
> {
> struct amdgpu_firmware_info *info = NULL;
> const struct common_firmware_header *header = NULL;
> int err = 0, i;
> const struct sdma_firmware_header_v2_0 *sdma_hdr;
> uint16_t version_major;
> + char ucode_prefix[30];
> + char fw_name[40];
>
> + amdgpu_ucode_ip_version_decode(adev, SDMA0_HWIP, ucode_prefix, sizeof(ucode_prefix));
> + snprintf(fw_name, sizeof(fw_name), "amdgpu/%s%s.bin", ucode_prefix, !instance ? "" : "1");
It is safer to keep the original logic with instance number as suffix
rather than hardcoding to 1.
Thanks,
Lijo
> err = amdgpu_ucode_load(adev, &adev->sdma.instance[instance].fw, fw_name);
> if (err)
> goto out;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> index 7d99205c2e01..2d16e6d36728 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> @@ -124,8 +124,8 @@ int amdgpu_sdma_process_ras_data_cb(struct amdgpu_device *adev,
> int amdgpu_sdma_process_ecc_irq(struct amdgpu_device *adev,
> struct amdgpu_irq_src *source,
> struct amdgpu_iv_entry *entry);
> -int amdgpu_sdma_init_microcode(struct amdgpu_device *adev,
> - char *fw_name, u32 instance, bool duplicate);
> +int amdgpu_sdma_init_microcode(struct amdgpu_device *adev, u32 instance,
> + bool duplicate);
> void amdgpu_sdma_destroy_inst_ctx(struct amdgpu_device *adev,
> bool duplicate);
> void amdgpu_sdma_unset_buffer_funcs_helper(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 4d780e4430e7..017ae298558e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -575,60 +575,17 @@ static void sdma_v4_0_setup_ulv(struct amdgpu_device *adev)
> // vega10 real chip need to use PSP to load firmware
> static int sdma_v4_0_init_microcode(struct amdgpu_device *adev)
> {
> - const char *chip_name;
> - char fw_name[30];
> int ret, i;
>
> - DRM_DEBUG("\n");
> -
> - switch (adev->ip_versions[SDMA0_HWIP][0]) {
> - case IP_VERSION(4, 0, 0):
> - chip_name = "vega10";
> - break;
> - case IP_VERSION(4, 0, 1):
> - chip_name = "vega12";
> - break;
> - case IP_VERSION(4, 2, 0):
> - chip_name = "vega20";
> - break;
> - case IP_VERSION(4, 1, 0):
> - case IP_VERSION(4, 1, 1):
> - if (adev->apu_flags & AMD_APU_IS_RAVEN2)
> - chip_name = "raven2";
> - else if (adev->apu_flags & AMD_APU_IS_PICASSO)
> - chip_name = "picasso";
> - else
> - chip_name = "raven";
> - break;
> - case IP_VERSION(4, 2, 2):
> - chip_name = "arcturus";
> - break;
> - case IP_VERSION(4, 1, 2):
> - if (adev->apu_flags & AMD_APU_IS_RENOIR)
> - chip_name = "renoir";
> - else
> - chip_name = "green_sardine";
> - break;
> - case IP_VERSION(4, 4, 0):
> - chip_name = "aldebaran";
> - break;
> - default:
> - BUG();
> - }
> -
> for (i = 0; i < adev->sdma.num_instances; i++) {
> - if (i == 0)
> - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_sdma.bin", chip_name);
> - else
> - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_sdma%d.bin", chip_name, i);
> if (adev->ip_versions[SDMA0_HWIP][0] == IP_VERSION(4, 2, 2) ||
> adev->ip_versions[SDMA0_HWIP][0] == IP_VERSION(4, 4, 0)) {
> /* Acturus & Aldebaran will leverage the same FW memory
> for every SDMA instance */
> - ret = amdgpu_sdma_init_microcode(adev, fw_name, 0, true);
> + ret = amdgpu_sdma_init_microcode(adev, 0, true);
> break;
> } else {
> - ret = amdgpu_sdma_init_microcode(adev, fw_name, i, false);
> + ret = amdgpu_sdma_init_microcode(adev, i, false);
> if (ret)
> return ret;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> index d4d9f196db83..1941b3b7c5d9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> @@ -237,39 +237,13 @@ static void sdma_v5_0_init_golden_registers(struct amdgpu_device *adev)
> // emulation only, won't work on real chip
> // navi10 real chip need to use PSP to load firmware
> static int sdma_v5_0_init_microcode(struct amdgpu_device *adev)
> -{
> - const char *chip_name;
> - char fw_name[40];
> - int ret, i;
> +{ int ret, i;
>
> if (amdgpu_sriov_vf(adev) && (adev->ip_versions[SDMA0_HWIP][0] == IP_VERSION(5, 0, 5)))
> return 0;
>
> - DRM_DEBUG("\n");
> -
> - switch (adev->ip_versions[SDMA0_HWIP][0]) {
> - case IP_VERSION(5, 0, 0):
> - chip_name = "navi10";
> - break;
> - case IP_VERSION(5, 0, 2):
> - chip_name = "navi14";
> - break;
> - case IP_VERSION(5, 0, 5):
> - chip_name = "navi12";
> - break;
> - case IP_VERSION(5, 0, 1):
> - chip_name = "cyan_skillfish2";
> - break;
> - default:
> - BUG();
> - }
> -
> for (i = 0; i < adev->sdma.num_instances; i++) {
> - if (i == 0)
> - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_sdma.bin", chip_name);
> - else
> - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_sdma1.bin", chip_name);
> - ret = amdgpu_sdma_init_microcode(adev, fw_name, i, false);
> + ret = amdgpu_sdma_init_microcode(adev, i, false);
> if (ret)
> return ret;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> index 65e7a710298d..8e445eb9dd49 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> @@ -89,59 +89,6 @@ static u32 sdma_v5_2_get_reg_offset(struct amdgpu_device *adev, u32 instance, u3
> return base + internal_offset;
> }
>
> -/**
> - * sdma_v5_2_init_microcode - load ucode images from disk
> - *
> - * @adev: amdgpu_device pointer
> - *
> - * Use the firmware interface to load the ucode images into
> - * the driver (not loaded into hw).
> - * Returns 0 on success, error on failure.
> - */
> -
> -// emulation only, won't work on real chip
> -// navi10 real chip need to use PSP to load firmware
> -static int sdma_v5_2_init_microcode(struct amdgpu_device *adev)
> -{
> - const char *chip_name;
> - char fw_name[40];
> -
> - DRM_DEBUG("\n");
> -
> - switch (adev->ip_versions[SDMA0_HWIP][0]) {
> - case IP_VERSION(5, 2, 0):
> - chip_name = "sienna_cichlid_sdma";
> - break;
> - case IP_VERSION(5, 2, 2):
> - chip_name = "navy_flounder_sdma";
> - break;
> - case IP_VERSION(5, 2, 1):
> - chip_name = "vangogh_sdma";
> - break;
> - case IP_VERSION(5, 2, 4):
> - chip_name = "dimgrey_cavefish_sdma";
> - break;
> - case IP_VERSION(5, 2, 5):
> - chip_name = "beige_goby_sdma";
> - break;
> - case IP_VERSION(5, 2, 3):
> - chip_name = "yellow_carp_sdma";
> - break;
> - case IP_VERSION(5, 2, 6):
> - chip_name = "sdma_5_2_6";
> - break;
> - case IP_VERSION(5, 2, 7):
> - chip_name = "sdma_5_2_7";
> - break;
> - default:
> - BUG();
> - }
> -
> - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", chip_name);
> -
> - return amdgpu_sdma_init_microcode(adev, fw_name, 0, true);
> -}
> -
> static unsigned sdma_v5_2_ring_init_cond_exec(struct amdgpu_ring *ring)
> {
> unsigned ret;
> @@ -1288,7 +1235,7 @@ static int sdma_v5_2_sw_init(void *handle)
> return r;
> }
>
> - r = sdma_v5_2_init_microcode(adev);
> + r = amdgpu_sdma_init_microcode(adev, 0, true);
> if (r) {
> DRM_ERROR("Failed to load sdma firmware!\n");
> return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> index 049c26a45d85..bf1fa5e8d2f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> @@ -78,29 +78,6 @@ static u32 sdma_v6_0_get_reg_offset(struct amdgpu_device *adev, u32 instance, u3
> return base + internal_offset;
> }
>
> -/**
> - * sdma_v6_0_init_microcode - load ucode images from disk
> - *
> - * @adev: amdgpu_device pointer
> - *
> - * Use the firmware interface to load the ucode images into
> - * the driver (not loaded into hw).
> - * Returns 0 on success, error on failure.
> - */
> -static int sdma_v6_0_init_microcode(struct amdgpu_device *adev)
> -{
> - char fw_name[30];
> - char ucode_prefix[30];
> -
> - DRM_DEBUG("\n");
> -
> - amdgpu_ucode_ip_version_decode(adev, SDMA0_HWIP, ucode_prefix, sizeof(ucode_prefix));
> -
> - snprintf(fw_name, sizeof(fw_name), "amdgpu/%s.bin", ucode_prefix);
> -
> - return amdgpu_sdma_init_microcode(adev, fw_name, 0, true);
> -}
> -
> static unsigned sdma_v6_0_ring_init_cond_exec(struct amdgpu_ring *ring)
> {
> unsigned ret;
> @@ -1260,7 +1237,7 @@ static int sdma_v6_0_sw_init(void *handle)
> if (r)
> return r;
>
> - r = sdma_v6_0_init_microcode(adev);
> + r = amdgpu_sdma_init_microcode(adev, 0, true);
> if (r) {
> DRM_ERROR("Failed to load sdma firmware!\n");
> return r;
Powered by blists - more mailing lists