Re: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory training(V4)

"Yin, Tianci (Rico)" <Tianci.Yin@xxxxxxx> · Mon, 23 Dec 2019 02:10:56 +0000

[AMD Official Use Only - Internal Distribution Only]

Thank you very much for your review!

From: Tuikov, Luben <Luben.Tuikov@xxxxxxx>

Sent: Saturday, December 21, 2019 5:52

To: Alex Deucher <alexdeucher@xxxxxxxxx>; Yin, Tianci (Rico) <Tianci.Yin@xxxxxxx>

Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>; Long, Gang <Gang.Long@xxxxxxx>; Xu, Feifei <Feifei.Xu@xxxxxxx>; Wang, Kevin(Yang) <Kevin1.Wang@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Zhang, Hawking <Hawking.Zhang@xxxxxxx>;
 Koenig, Christian <Christian.Koenig@xxxxxxx>; Yuan, Xiaojie <Xiaojie.Yuan@xxxxxxx>

Subject: Re: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory training(V4)

On 2019-12-20 10:27 a.m., Alex Deucher wrote:

> On Thu, Dec 19, 2019 at 9:00 PM Yin, Tianci (Rico) <Tianci.Yin@xxxxxxx> wrote:

>>

>> [AMD Official Use Only - Internal Distribution Only]

>>

>>

>> Hi Luben,

>>

>> May I have your Review-by?

>>

If you'd like--it's completely up to you. If you choose to, like Alex's below.

Regards,

Luben

> 

> Series is:

> Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>

> 

>> Thanks a lot!

>> Rico

>> ________________________________

>> From: Tuikov, Luben <Luben.Tuikov@xxxxxxx>

>> Sent: Friday, December 20, 2019 3:47

>> To: Yin, Tianci (Rico) <Tianci.Yin@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>

>> Cc: Koenig, Christian <Christian.Koenig@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Zhang, Hawking <Hawking.Zhang@xxxxxxx>; Xu, Feifei <Feifei.Xu@xxxxxxx>; Yuan, Xiaojie <Xiaojie.Yuan@xxxxxxx>; Long, Gang <Gang.Long@xxxxxxx>; Wang, Kevin(Yang)
 <Kevin1.Wang@xxxxxxx>

>> Subject: Re: [PATCH 1/2] drm/amdgpu: update the method to get fb_loc of memory training(V4)

>>

>> Yep! That's perfect--good job!

>>

>> Regards,

>> Luben

>>

>> On 2019-12-19 04:16, Tianci Yin wrote:

>>> From: "Tianci.Yin" <tianci.yin@xxxxxxx>

>>>

>>> The method of getting fb_loc changed from parsing VBIOS to

>>> taking certain offset from top of VRAM

>>>

>>> Change-Id: I053b42fdb1d822722fa7980b2cd9f86b3fdce539

>>> Signed-off-by: Tianci.Yin <tianci.yin@xxxxxxx>

>>> ---

>>>  drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  3 +-

>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c  |  2 +-

>>>  .../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c  | 38 ++-----------------

>>>  .../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h  |  2 +-

>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c       | 10 ++++-

>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h       |  7 ++++

>>>  drivers/gpu/drm/amd/include/atomfirmware.h    | 14 -------

>>>  7 files changed, 23 insertions(+), 53 deletions(-)

>>>

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

>>> index a78a363b1d71..fa2cf8e7bc07 100644

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

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

>>> @@ -642,9 +642,8 @@ struct amdgpu_fw_vram_usage {

>>>        struct amdgpu_bo *reserved_bo;

>>>        void *va;

>>>

>>> -     /* Offset on the top of VRAM, used as c2p write buffer.

>>> +     /* GDDR6 training support flag.

>>>        */

>>> -     u64 mem_train_fb_loc;

>>>        bool mem_train_support;

>>>  };

>>>

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

>>> index 9ba80d828876..fdd52d86a4d7 100644

>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c

>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c

>>> @@ -2022,7 +2022,7 @@ int amdgpu_atombios_init(struct amdgpu_device *adev)

>>>        if (adev->is_atom_fw) {

>>>                amdgpu_atomfirmware_scratch_regs_init(adev);

>>>                amdgpu_atomfirmware_allocate_fb_scratch(adev);

>>> -             ret = amdgpu_atomfirmware_get_mem_train_fb_loc(adev);

>>> +             ret = amdgpu_atomfirmware_get_mem_train_info(adev);

>>>                if (ret) {

>>>                        DRM_ERROR("Failed to get mem train fb location.\n");

>>>                        return ret;

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

>>> index ff4eb96bdfb5..58f9d8c3a17a 100644

>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c

>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c

>>> @@ -525,16 +525,12 @@ static int gddr6_mem_train_support(struct amdgpu_device *adev)

>>>        return ret;

>>>  }

>>>

>>> -int amdgpu_atomfirmware_get_mem_train_fb_loc(struct amdgpu_device *adev)

>>> +int amdgpu_atomfirmware_get_mem_train_info(struct amdgpu_device *adev)

>>>  {

>>>        struct atom_context *ctx = adev->mode_info.atom_context;

>>> -     unsigned char *bios = ctx->bios;

>>> -     struct vram_reserve_block *reserved_block;

>>> -     int index, block_number;

>>> +     int index;

>>>        uint8_t frev, crev;

>>>        uint16_t data_offset, size;

>>> -     uint32_t start_address_in_kb;

>>> -     uint64_t offset;

>>>        int ret;

>>>

>>>        adev->fw_vram_usage.mem_train_support = false;

>>> @@ -569,32 +565,6 @@ int amdgpu_atomfirmware_get_mem_train_fb_loc(struct amdgpu_device *adev)

>>>                return -EINVAL;

>>>        }

>>>

>>> -     reserved_block = (struct vram_reserve_block *)

>>> -             (bios + data_offset + sizeof(struct atom_common_table_header));

>>> -     block_number = ((unsigned int)size - sizeof(struct atom_common_table_header))

>>> -             / sizeof(struct vram_reserve_block);

>>> -     reserved_block += (block_number > 0) ? block_number-1 : 0;

>>> -     DRM_DEBUG("block_number:0x%04x, last block: 0x%08xkb sz, %dkb fw, %dkb drv.\n",

>>> -               block_number,

>>> -               le32_to_cpu(reserved_block->start_address_in_kb),

>>> -               le16_to_cpu(reserved_block->used_by_firmware_in_kb),

>>> -               le16_to_cpu(reserved_block->used_by_driver_in_kb));

>>> -     if (reserved_block->used_by_firmware_in_kb > 0) {

>>> -             start_address_in_kb = le32_to_cpu(reserved_block->start_address_in_kb);

>>> -             offset = (uint64_t)start_address_in_kb * ONE_KiB;

>>> -             if ((offset & (ONE_MiB - 1)) < (4 * ONE_KiB + 1) ) {

>>> -                     offset -= ONE_MiB;

>>> -             }

>>> -

>>> -             offset &= ~(ONE_MiB - 1);

>>> -             adev->fw_vram_usage.mem_train_fb_loc = offset;

>>> -             adev->fw_vram_usage.mem_train_support = true;

>>> -             DRM_DEBUG("mem_train_fb_loc:0x%09llx.\n", offset);

>>> -             ret = 0;

>>> -     } else {

>>> -             DRM_ERROR("used_by_firmware_in_kb is 0!\n");

>>> -             ret = -EINVAL;

>>> -     }

>>> -

>>> -     return ret;

>>> +     adev->fw_vram_usage.mem_train_support = true;

>>> +     return 0;

>>>  }

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

>>> index f871af5ea6f3..434fe2fa0089 100644

>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h

>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.h

>>> @@ -31,7 +31,7 @@ void amdgpu_atomfirmware_scratch_regs_init(struct amdgpu_device *adev);

>>>  int amdgpu_atomfirmware_allocate_fb_scratch(struct amdgpu_device *adev);

>>>  int amdgpu_atomfirmware_get_vram_info(struct amdgpu_device *adev,

>>>        int *vram_width, int *vram_type, int *vram_vendor);

>>> -int amdgpu_atomfirmware_get_mem_train_fb_loc(struct amdgpu_device *adev);

>>> +int amdgpu_atomfirmware_get_mem_train_info(struct amdgpu_device *adev);

>>>  int amdgpu_atomfirmware_get_clock_info(struct amdgpu_device *adev);

>>>  int amdgpu_atomfirmware_get_gfx_info(struct amdgpu_device *adev);

>>>  bool amdgpu_atomfirmware_mem_ecc_supported(struct amdgpu_device *adev);

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

>>> index 2ff63d0414c9..1515413fd356 100644

>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c

>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c

>>> @@ -1687,6 +1687,14 @@ static int amdgpu_ttm_training_reserve_vram_fini(struct amdgpu_device *adev)

>>>        return 0;

>>>  }

>>>

>>> +static u64 amdgpu_ttm_training_get_c2p_offset(u64 vram_size)

>>> +{

>>> +       if ((vram_size & (ONE_MiB - 1)) < (4 * ONE_KiB + 1) )

>>> +               vram_size -= ONE_MiB;

>>> +

>>> +       return ALIGN(vram_size, ONE_MiB);

>>> +}

>>> +

>>>  /**

>>>   * amdgpu_ttm_training_reserve_vram_init - create bo vram reservation from memory training

>>>   *

>>> @@ -1705,7 +1713,7 @@ static int amdgpu_ttm_training_reserve_vram_init(struct amdgpu_device *adev)

>>>                return 0;

>>>        }

>>>

>>> -     ctx->c2p_train_data_offset = adev->fw_vram_usage.mem_train_fb_loc;

>>> +     ctx->c2p_train_data_offset = amdgpu_ttm_training_get_c2p_offset(adev->gmc.mc_vram_size);

>>>        ctx->p2c_train_data_offset = (adev->gmc.mc_vram_size - GDDR6_MEM_TRAINING_OFFSET);

>>>        ctx->train_data_size = GDDR6_MEM_TRAINING_DATA_SIZE_IN_BYTES;

>>>

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

>>> index f1ebd424510c..19eb3e8456c7 100644

>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h

>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h

>>> @@ -66,6 +66,13 @@ struct amdgpu_copy_mem {

>>>        unsigned long                   offset;

>>>  };

>>>

>>> +/* Definitions for constance */

>>> +enum amdgpu_internal_constants

>>> +{

>>> +     _ONE_KiB_ = 0x400,

>>> +     _ONE_MiB_ = 0x100000,

>>> +};

>>> +

>>>  extern const struct ttm_mem_type_manager_func amdgpu_gtt_mgr_func;

>>>  extern const struct ttm_mem_type_manager_func amdgpu_vram_mgr_func;

>>>

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

>>> index dd7cbc00a0aa..70146518174c 100644

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

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

>>> @@ -672,20 +672,6 @@ struct vram_usagebyfirmware_v2_1

>>>    uint16_t  used_by_driver_in_kb;

>>>  };

>>>

>>> -/* This is part of vram_usagebyfirmware_v2_1 */

>>> -struct vram_reserve_block

>>> -{

>>> -     uint32_t start_address_in_kb;

>>> -     uint16_t used_by_firmware_in_kb;

>>> -     uint16_t used_by_driver_in_kb;

>>> -};

>>> -

>>> -/* Definitions for constance */

>>> -enum atomfirmware_internal_constants

>>> -{

>>> -     _ONE_KiB_ = 0x400,

>>> -     _ONE_MiB_ = 0x100000,

>>> -};

>>>

>>>  /*

>>>    ***************************************************************************

>>>

>>

>> _______________________________________________

>> amd-gfx mailing list

>> amd-gfx@xxxxxxxxxxxxxxxxxxxxx

>> 
https://nam11.safelinks.protection.outlook.com/?url="">

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