AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Kuehling, Felix" <felix.kuehling@amd.com>
To: Philip Yang <yangp@amd.com>,
	"Russell, Kent" <Kent.Russell@amd.com>,
	"Yang, Philip" <Philip.Yang@amd.com>,
	"amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>
Cc: "Koenig, Christian" <Christian.Koenig@amd.com>,
	"Yat Sin, David" <David.YatSin@amd.com>
Subject: Re: [PATCH v2 2/4] drm/amdkfd: Add domain parameter to alloc kernel BO
Date: Wed, 3 Dec 2025 12:45:18 -0500	[thread overview]
Message-ID: <fd6eb40c-cf75-430e-ac44-db6244d48762@amd.com> (raw)
In-Reply-To: <1256b740-65b5-48ad-ad65-4149d3b880e6@amd.com>


On 2025-12-01 15:31, Philip Yang wrote:
>
>
> On 2025-12-01 12:38, Russell, Kent wrote:
>> [Public]
>>
>> I know that it makes the change more complicated, but should we be 
>> renaming the function since we're no longer guaranteeing a GTT memory 
>> allocation in the alloc_gtt_mem function?
> yes, you are right, I have the patch to 
> s/alloc_gtt_mem/alloc_kernel_mem/g, s/free_gtt_mem/free_kernel_mem/g, 
> and found it is too much changes,

I see amdgpu_amdkfd_alloc_gtt_mem being called in 7 places in KFD. I 
don't think that's too much churn. You need to change all those places 
anyway to add a new parameter, so you might as well change the function 
name while you're there.

Regards,
   Felix


>  not related to the purpose of this patch series, so I dropped that 
> patch. We can cleanup this in the followup patch, probably remove the 
> alloc/free_gtt_mem functions and use amdgpu_bo_create_kernel instead.
>
> Regards,
> Philip
>>
>>   Kent
>>
>>> -----Original Message-----
>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of 
>>> Philip Yang
>>> Sent: Monday, December 1, 2025 9:29 AM
>>> To: amd-gfx@lists.freedesktop.org
>>> Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian
>>> <Christian.Koenig@amd.com>; Yat Sin, David <David.YatSin@amd.com>; 
>>> Yang,
>>> Philip <Philip.Yang@amd.com>
>>> Subject: [PATCH v2 2/4] drm/amdkfd: Add domain parameter to alloc 
>>> kernel BO
>>>
>>> To allocate kernel BO from VRAM domain for MQD in the following patch.
>>> No functional change because kernel BO allocate are all from GTT 
>>> domain so far.
>>>
>>> amdgpu_amdkfd_alloc_gtt_mem has many duplicate code as
>>> function amdgpu_bo_create_kernel, with one extra flag MQD_GFX9
>>> to bind MQD and control stack in GART with different mtype,
>>> the duplicate code could be removed in future.
>>>
>>> Signed-off-by: Philip Yang <Philip.Yang@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c             | 6 +++---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h             | 2 +-
>>>   drivers/gpu/drm/amd/amdkfd/kfd_debug.c                 | 1 +
>>>   drivers/gpu/drm/amd/amdkfd/kfd_device.c                | 3 ++-
>>>   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  | 1 +
>>>   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c        | 1 +
>>>   drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 2 ++
>>>   7 files changed, 11 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> index a2879d2b7c8e..9cd1660b8f60 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
>>> @@ -317,7 +317,7 @@ void amdgpu_amdkfd_gpu_reset(struct amdgpu_device
>>> *adev)
>>>   }
>>>
>>>   int amdgpu_amdkfd_alloc_gtt_mem(struct amdgpu_device *adev, size_t 
>>> size,
>>> -                             void **mem_obj, uint64_t *gpu_addr,
>>> +                             u32 domain, void **mem_obj, uint64_t 
>>> *gpu_addr,
>>>                                void **cpu_ptr, bool cp_mqd_gfx9)
>>>   {
>>>        struct amdgpu_bo *bo = NULL;
>>> @@ -328,7 +328,7 @@ int amdgpu_amdkfd_alloc_gtt_mem(struct 
>>> amdgpu_device
>>> *adev, size_t size,
>>>        memset(&bp, 0, sizeof(bp));
>>>        bp.size = size;
>>>        bp.byte_align = PAGE_SIZE;
>>> -     bp.domain = AMDGPU_GEM_DOMAIN_GTT;
>>> +     bp.domain = domain;
>>>        bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC;
>>>        bp.type = ttm_bo_type_kernel;
>>>        bp.resv = NULL;
>>> @@ -351,7 +351,7 @@ int amdgpu_amdkfd_alloc_gtt_mem(struct 
>>> amdgpu_device
>>> *adev, size_t size,
>>>                goto allocate_mem_reserve_bo_failed;
>>>        }
>>>
>>> -     r = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT);
>>> +     r = amdgpu_bo_pin(bo, domain);
>>>        if (r) {
>>>                dev_err(adev->dev, "(%d) failed to pin bo for 
>>> amdkfd\n", r);
>>>                goto allocate_mem_pin_bo_failed;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>>> index 335d733751cb..14b5ab6fa051 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
>>> @@ -237,7 +237,7 @@ int amdgpu_amdkfd_bo_validate_and_fence(struct
>>> amdgpu_bo *bo,
>>>   #endif
>>>   /* Shared API */
>>>   int amdgpu_amdkfd_alloc_gtt_mem(struct amdgpu_device *adev, size_t 
>>> size,
>>> -                             void **mem_obj, uint64_t *gpu_addr,
>>> +                             u32 domain, void **mem_obj, uint64_t 
>>> *gpu_addr,
>>>                                void **cpu_ptr, bool mqd_gfx9);
>>>   void amdgpu_amdkfd_free_gtt_mem(struct amdgpu_device *adev, void 
>>> **mem_obj);
>>>   int amdgpu_amdkfd_alloc_gws(struct amdgpu_device *adev, size_t size,
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
>>> index ba9a09b6589a..494a563e072e 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debug.c
>>> @@ -359,6 +359,7 @@ int kfd_dbg_set_mes_debug_mode(struct
>>> kfd_process_device *pdd, bool sq_trap_en)
>>>        if (!pdd->proc_ctx_cpu_ptr) {
>>>                r = amdgpu_amdkfd_alloc_gtt_mem(adev,
>>>                        AMDGPU_MES_PROC_CTX_SIZE,
>>> +                     AMDGPU_GEM_DOMAIN_GTT,
>>>                        &pdd->proc_ctx_bo,
>>>                        &pdd->proc_ctx_gpu_addr,
>>>                        &pdd->proc_ctx_cpu_ptr,
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> index 9f183d27a0ef..b0cfad750919 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> @@ -787,7 +787,8 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
>>>        size += 512 * 1024;
>>>
>>>        if (amdgpu_amdkfd_alloc_gtt_mem(
>>> -                     kfd->adev, size, &kfd->gtt_mem,
>>> +                     kfd->adev, size, AMDGPU_GEM_DOMAIN_GTT,
>>> +                     &kfd->gtt_mem,
>>>                        &kfd->gtt_start_gpu_addr, 
>>> &kfd->gtt_start_cpu_ptr,
>>>                        false)) {
>>>                dev_err(kfd_device, "Could not allocate %d bytes\n", 
>>> size);
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>> index 36ac35d64126..028fca1d2755 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>> @@ -2906,6 +2906,7 @@ static int allocate_hiq_sdma_mqd(struct
>>> device_queue_manager *dqm)
>>>                NUM_XCC(dqm->dev->xcc_mask));
>>>
>>>        retval = amdgpu_amdkfd_alloc_gtt_mem(dev->adev, size,
>>> +             AMDGPU_GEM_DOMAIN_GTT,
>>>                &(mem_obj->gtt_mem), &(mem_obj->gpu_addr),
>>>                (void *)&(mem_obj->cpu_ptr), false);
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
>>> index 2e9b6bcf2704..a489d43d5f64 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
>>> @@ -139,6 +139,7 @@ static struct kfd_mem_obj *allocate_mqd(struct 
>>> kfd_node
>>> *node,
>>>                        (ALIGN(q->ctl_stack_size, PAGE_SIZE) +
>>>                        ALIGN(sizeof(struct v9_mqd), PAGE_SIZE)) *
>>>                        NUM_XCC(node->xcc_mask),
>>> +                     AMDGPU_GEM_DOMAIN_GTT,
>>>                        &(mqd_mem_obj->gtt_mem),
>>>                        &(mqd_mem_obj->gpu_addr),
>>>                        (void *)&(mqd_mem_obj->cpu_ptr), true);
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>>> index 5f8cda4733f9..e0c5ec7e2fe9 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>>> @@ -266,6 +266,7 @@ static int init_user_queue(struct 
>>> process_queue_manager
>>> *pqm,
>>>        if (dev->kfd->shared_resources.enable_mes) {
>>>                retval = amdgpu_amdkfd_alloc_gtt_mem(dev->adev,
>>> AMDGPU_MES_GANG_CTX_SIZE,
>>> + AMDGPU_GEM_DOMAIN_GTT,
>>> &(*q)->gang_ctx_bo,
>>> &(*q)->gang_ctx_gpu_addr,
>>> &(*q)->gang_ctx_cpu_ptr,
>>> @@ -369,6 +370,7 @@ int pqm_create_queue(struct process_queue_manager
>>> *pqm,
>>>        if (!pdd->proc_ctx_cpu_ptr && 
>>> dev->kfd->shared_resources.enable_mes) {
>>>                retval = amdgpu_amdkfd_alloc_gtt_mem(dev->adev,
>>> AMDGPU_MES_PROC_CTX_SIZE,
>>> + AMDGPU_GEM_DOMAIN_GTT,
>>> &pdd->proc_ctx_bo,
>>> &pdd->proc_ctx_gpu_addr,
>>> &pdd->proc_ctx_cpu_ptr,
>>> -- 
>>> 2.50.1
>

  parent reply	other threads:[~2025-12-03 17:45 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-01 14:28 [PATCH v2 0/4] drm/amdkfd: Move gfx9 MQD to HBM Philip Yang
2025-12-01 14:28 ` [PATCH v2 1/4] drm/amdkfd: Bind gfx9 MQD in GART with mtype RW Philip Yang
2025-12-03 17:40   ` Kuehling, Felix
2025-12-04 15:10     ` Philip Yang
2025-12-01 14:28 ` [PATCH v2 2/4] drm/amdkfd: Add domain parameter to alloc kernel BO Philip Yang
2025-12-01 17:38   ` Russell, Kent
2025-12-01 20:31     ` Philip Yang
2025-12-01 22:48       ` Russell, Kent
2025-12-03 17:45       ` Kuehling, Felix [this message]
2025-12-01 14:28 ` [PATCH v2 3/4] drm/amdkfd: Move gfx9 MQD to VRAM domain Philip Yang
2025-12-03 17:55   ` Kuehling, Felix
2025-12-04 19:28     ` Philip Yang
2025-12-04 22:51     ` Philip Yang
2025-12-05 19:46       ` Felix Kuehling
2025-12-05 21:22         ` Philip Yang
2025-12-01 14:28 ` [PATCH v2 4/4] drm/amdkfd: Map VRAM MQD on GART Philip Yang
2025-12-03 16:06   ` Christian König
2025-12-04 22:43     ` Philip Yang
2025-12-05  9:30       ` Christian König
2025-12-03 15:52 ` [PATCH v2 0/4] drm/amdkfd: Move gfx9 MQD to HBM Philip Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fd6eb40c-cf75-430e-ac44-db6244d48762@amd.com \
    --to=felix.kuehling@amd.com \
    --cc=Christian.Koenig@amd.com \
    --cc=David.YatSin@amd.com \
    --cc=Kent.Russell@amd.com \
    --cc=Philip.Yang@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=yangp@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox