All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.