[Public]

You also need to take care of atom firmware file change. The purpose of version 
specific file is to keep IP specific logic and avoid littering common code with 
IP version checks.

Thanks,
Lijo
________________________________
From: Xie, Patrick <[email protected]>
Sent: Tuesday, March 24, 2026 7:14:46 AM
To: Lazar, Lijo <[email protected]>; [email protected] 
<[email protected]>
Cc: Zhang, Hawking <[email protected]>; Zhou1, Tao <[email protected]>
Subject: RE: [PATCH] drm/amdgpu: add support to query vram info from firmware

[AMD Official Use Only - AMD Internal Distribution Only]

Thanks, will do two changes:
        1. change vram_type to HBM as you pointed out.
        2.will add multi-aid check.

-----Original Message-----
From: Lazar, Lijo <[email protected]>
Sent: Monday, March 23, 2026 11:30 PM
To: Xie, Patrick <[email protected]>; [email protected]
Cc: Zhang, Hawking <[email protected]>; Zhou1, Tao <[email protected]>
Subject: Re: [PATCH] drm/amdgpu: add support to query vram info from firmware



On 23-Mar-26 1:35 PM, Gangliang Xie wrote:
> add support to query vram info from firmware
>
> Signed-off-by: Gangliang Xie <[email protected]>
> ---
>   .../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c  | 13 ++++-
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c         | 50 +++++++++++--------
>   2 files changed, 40 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> index 7f4751e5caaf..504b5f0a74ab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> @@ -399,6 +399,9 @@ amdgpu_atomfirmware_get_vram_info(struct amdgpu_device 
> *adev,
>               switch (amdgpu_ip_version(adev, GC_HWIP, 0)) {
>               case IP_VERSION(12, 0, 0):
>               case IP_VERSION(12, 0, 1):
> +             case IP_VERSION(9, 5, 0):
> +             case IP_VERSION(9, 4, 4):
> +             case IP_VERSION(9, 4, 3):
>                       index = 
> get_index_into_master_table(atom_master_list_of_data_tables_v2_1, umc_info);
>                       break;
>               default:
> @@ -475,6 +478,9 @@ amdgpu_atomfirmware_get_vram_info(struct amdgpu_device 
> *adev,
>                       switch (amdgpu_ip_version(adev, GC_HWIP, 0)) {
>                       case IP_VERSION(12, 0, 0):
>                       case IP_VERSION(12, 0, 1):
> +                     case IP_VERSION(9, 5, 0):
> +                     case IP_VERSION(9, 4, 4):
> +                     case IP_VERSION(9, 4, 3):
>                               umc_info = (union umc_info 
> *)(mode_info->atom_context->bios +
> data_offset);
>
>                               if (frev == 4) {
> @@ -488,8 +494,13 @@ amdgpu_atomfirmware_get_vram_info(struct amdgpu_device 
> *adev,
>                                                       *vram_vendor = 
> mem_vendor;
>                                               if (vram_type)
>                                                       *vram_type = 
> convert_atom_mem_type_to_vram_type(adev, mem_type);
> -                                             if (vram_width)
> +                                             if (vram_width) {
>                                                       *vram_width = 
> mem_channel_number * (1 << mem_channel_width);
> +                                                     if 
> (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 5, 0) ||
> +                                                         
> amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 4) ||
> +                                                         
> amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4,
> +3))

Suggest to move this logic inside gmc_v9_0.c after fetching vram info from 
firwmare. Then within gmc_v9, you may wrap the check with 
amdgpu_is_multi_aid(adev).

> +                                                             *vram_width *= 
> 4;
> +                                             }
>                                               break;
>                                       default:
>                                               return -EINVAL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 1ca0202cfdea..e6bb04dd0830 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1823,24 +1823,37 @@ static void gmc_v9_0_save_registers(struct 
> amdgpu_device *adev)
>               adev->gmc.sdpif_register = RREG32_SOC15(DCE, 0, 
> mmDCHUBBUB_SDPIF_MMIO_CNTRL_0);
>   }
>
> -static void gmc_v9_4_3_init_vram_info(struct amdgpu_device *adev)
> +static void gmc_v9_0_init_vram_info(struct amdgpu_device *adev)
>   {
>       static const u32 regBIF_BIOS_SCRATCH_4 = 0x50;
> +     int dev_var = adev->pdev->device & 0xF;
>       u32 vram_info;
>
> -     adev->gmc.vram_type = AMDGPU_VRAM_TYPE_HBM;
> -     adev->gmc.vram_width = 128 * 64;
> -
> -     if (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 5, 0))
> +     if (adev->gmc.is_app_apu) {
>               adev->gmc.vram_type = AMDGPU_VRAM_TYPE_HBM3E;

This is just AMDGPU_VRAM_TYPE_HBM.

> +             adev->gmc.vram_width = 128 * 64;
> +     } else if (adev->flags & AMD_IS_APU) {
> +             adev->gmc.vram_type = AMDGPU_VRAM_TYPE_DDR4;
> +             adev->gmc.vram_width = 64 * 64;
> +     } else {
> +             adev->gmc.vram_type = AMDGPU_VRAM_TYPE_HBM;
> +             adev->gmc.vram_width = 128 * 64;
>
> -     if (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 4) &&
> -             adev->rev_id == 0x3)
> -             adev->gmc.vram_type = AMDGPU_VRAM_TYPE_HBM3E;
> +             if (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 5, 0))
> +                     adev->gmc.vram_type = AMDGPU_VRAM_TYPE_HBM3E;
> +
> +             if (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 4) 
> &&
> +                 adev->rev_id == 0x3)
> +                     adev->gmc.vram_type = AMDGPU_VRAM_TYPE_HBM3E;
> +
> +             if (amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 3) 
> &&
> +                 (dev_var == 0x5))
> +                     adev->gmc.vram_type = AMDGPU_VRAM_TYPE_HBM3E;

For an easier lookup, may be wrap these inside a small inline function -
        if (gmc_v9_0_is_hbm3e(adev))
                adev->gmc.vram_type = AMDGPU_VRAM_TYPE_HBM3E;
        else
                adev->gmc.vram_type = AMDGPU_VRAM_TYPE_HBM;
>
> -     if (!(adev->flags & AMD_IS_APU) && !amdgpu_sriov_vf(adev)) {
> -             vram_info = RREG32(regBIF_BIOS_SCRATCH_4);
> -             adev->gmc.vram_vendor = vram_info & 0xF;
> +             if (!(adev->flags & AMD_IS_APU) && !amdgpu_sriov_vf(adev)) {
> +                     vram_info = RREG32(regBIF_BIOS_SCRATCH_4);
> +                     adev->gmc.vram_vendor = vram_info & 0xF;

This is specific to multi-aid SOCs. You may add that check here.

Thanks,
Lijo

> +             }
>       }
>   }
>
> @@ -1856,19 +1869,11 @@ static int gmc_v9_0_sw_init(struct
> amdgpu_ip_block *ip_block)
>
>       spin_lock_init(&adev->gmc.invalidate_lock);
>
> -     if (amdgpu_is_multi_aid(adev)) {
> -             gmc_v9_4_3_init_vram_info(adev);
> -     } else if (!adev->bios) {
> -             if (adev->flags & AMD_IS_APU) {
> -                     adev->gmc.vram_type = AMDGPU_VRAM_TYPE_DDR4;
> -                     adev->gmc.vram_width = 64 * 64;
> -             } else {
> -                     adev->gmc.vram_type = AMDGPU_VRAM_TYPE_HBM;
> -                     adev->gmc.vram_width = 128 * 64;
> -             }
> +     if (!adev->bios) {
> +             gmc_v9_0_init_vram_info(adev);
>       } else {
>               r = amdgpu_atomfirmware_get_vram_info(adev,
> -                     &vram_width, &vram_type, &vram_vendor);
> +                             &vram_width, &vram_type, &vram_vendor);
>               if (amdgpu_sriov_vf(adev))
>                       /* For Vega10 SR-IOV, vram_width can't be read from 
> ATOM as RAVEN,
>                        * and DF related registers is not readable, seems 
> hardcord is
> the @@ -1896,6 +1901,7 @@ static int gmc_v9_0_sw_init(struct amdgpu_ip_block 
> *ip_block)
>               adev->gmc.vram_type = vram_type;
>               adev->gmc.vram_vendor = vram_vendor;
>       }
> +
>       switch (amdgpu_ip_version(adev, GC_HWIP, 0)) {
>       case IP_VERSION(9, 1, 0):
>       case IP_VERSION(9, 2, 2):

Reply via email to