On 10/9/26 00:11, Geramy Loveless wrote:
> KFD sub-allocates the MQD of every GFX12 user compute queue from its
> GTT pool, so the MQD is in system memory. The CP accesses the MQD while
> it runs the queue. A dispatch that exactly fills a shader engine is
> retired through a path that accesses the MQD, so with the MQD in system
> memory that dispatch pays the PCIe/host-link latency.
> 
> Measured with an empty kernel, median of 200 dispatches, on a Radeon AI
> PRO R9700 (gfx1201), for a dispatch of 253-256 workgroups x 256
> threads:
> 
>   - stock Linux (MQD in GTT): 26 us, against 7.9 us for 252
>     workgroups.
>   - Thunderbolt-attached R9700 to MacOS running mac_linuxgpu running the same 
> amdgpu code under a
>     different host OS: 51 us with the MQD in system memory, 17 us with
>     the MQD in VRAM.
> 
> On dGPUs, allocate GFX12 compute queue MQDs with
> amdgpu_bo_create_kernel() in VRAM. That sets CPU_ACCESS_REQUIRED, so
> the BO is placed in the CPU-visible part of VRAM, which is needed
> because the CPU writes the MQD. Flush HDP after CPU writes to the MQD:
> at the end of update_mqd() (init_mqd() ends with it), in restore_mqd(),
> and in a new update_mqd_gpu_addr().
> 
> Add GC 12.0.0 and 12.0.1 to mqd_on_vram() so the existing S4 handling
> (unpin at suspend, repin on resume) covers these MQDs, and implement
> update_mqd_gpu_addr() for v12 so the MQD base address is fixed up after
> a repin.
> 
> This follows existing practice: KFD already places compute queue MQDs
> in VRAM on GFX 9.4.2, 9.4.3, 9.4.4 and 9.5.0 (mqd_on_vram()), and
> amdgpu places its kernel compute queue MQDs in VRAM on GFX10+
> (amdgpu_gfx_mqd_sw_init()).
> 
> Only GFX12 (GC 12.0.x) is affected. SDMA, HIQ and DIQ MQDs are
> unchanged, as are APUs that prefer GTT.
> 
> Testing: compile-tested on amd-staging-drm-next. The patched kernel has
> not been measured on Linux yet; the VRAM number above is from the other
> host OS.
> 
> for macos r9700 support reference where i prove the gains:
> https://github.com/lemonade-sdk/mac_linuxgpu

Absolutely clear NAK!

You are missing tons and I mean litterally hundreds of technical pre-requisites 
of doing this.

Keep in mind that VRAM is not coherent, you need HDP flushes and iosys_map() 
handling for all memory accesses to the MQD for that to work correctly.

We have some internal patches to do this, but IIRC they are still not full 
stable.

Regards,
Christian.

> 
> Signed-off-by: Geramy Loveless <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c  |  2 +
>  .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c  | 54 ++++++++++++++++++-
>  2 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c
> index 9b7859a77..e6b45950e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c
> @@ -326,6 +326,8 @@ bool mqd_on_vram(struct amdgpu_device *adev)
>       case IP_VERSION(9, 4, 3):
>       case IP_VERSION(9, 4, 4):
>       case IP_VERSION(9, 5, 0):
> +     case IP_VERSION(12, 0, 0):
> +     case IP_VERSION(12, 0, 1):
>               return true;
>       default:
>               return false;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c
> index 6ea09b031..d3a8c2aed 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c
> @@ -92,6 +92,37 @@ static struct kfd_mem_obj *allocate_mqd(struct mqd_manager 
> *mm,
>       return mqd_mem_obj;
>  }
>  
> +/*
> + * The CP accesses the MQD while the queue is running, so keep compute queue
> + * MQDs in VRAM. The CPU writes the MQD, so it must be CPU visible.
> + */
> +static struct kfd_mem_obj *allocate_cp_mqd(struct mqd_manager *mm,
> +                                        struct queue_properties *q)
> +{
> +     u32 mqd_size = AMDGPU_MQD_SIZE_ALIGN(mm->mqd_size);
> +     struct amdgpu_device *adev = mm->dev->adev;
> +     struct kfd_mem_obj *mqd_mem_obj;
> +     struct amdgpu_bo *bo = NULL;
> +
> +     if (!mqd_on_vram(adev))
> +             return allocate_mqd(mm, q);
> +
> +     mqd_mem_obj = kzalloc_obj(*mqd_mem_obj);
> +     if (!mqd_mem_obj)
> +             return NULL;
> +
> +     if (amdgpu_bo_create_kernel(adev, mqd_size, PAGE_SIZE,
> +                                 AMDGPU_GEM_DOMAIN_VRAM, &bo,
> +                                 &mqd_mem_obj->gpu_addr,
> +                                 (void **)&mqd_mem_obj->cpu_ptr)) {
> +             kfree(mqd_mem_obj);
> +             return NULL;
> +     }
> +     mqd_mem_obj->mem = bo;
> +
> +     return mqd_mem_obj;
> +}
> +
>  static void init_mqd(struct mqd_manager *mm, void **mqd,
>                       struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>                       struct queue_properties *q)
> @@ -242,6 +273,23 @@ static void update_mqd(struct mqd_manager *mm, void *mqd,
>       set_priority(m, q);
>  
>       q->is_active = QUEUE_IS_ACTIVE(*q);
> +
> +     if (mqd_on_vram(mm->dev->adev))
> +             amdgpu_device_flush_hdp(mm->dev->adev, NULL);
> +}
> +
> +static void update_mqd_gpu_addr(struct mqd_manager *mm, void *mqd,
> +                             struct kfd_mem_obj *mqd_mem_obj,
> +                             struct queue_properties *qp)
> +{
> +     struct v12_compute_mqd *m = get_mqd(mqd);
> +     u64 addr = mqd_mem_obj->gpu_addr;
> +
> +     m->cp_mqd_base_addr_lo = lower_32_bits(addr);
> +     m->cp_mqd_base_addr_hi = upper_32_bits(addr);
> +
> +     if (mqd_on_vram(mm->dev->adev))
> +             amdgpu_device_flush_hdp(mm->dev->adev, NULL);
>  }
>  
>  static bool check_preemption_failed(struct mqd_manager *mm, void *mqd)
> @@ -409,6 +457,9 @@ static void restore_mqd(struct mqd_manager *mm, void 
> **mqd,
>               *gart_addr = addr;
>  
>       qp->is_active = 0;
> +
> +     if (mqd_on_vram(mm->dev->adev))
> +             amdgpu_device_flush_hdp(mm->dev->adev, NULL);
>  }
>  
>  static void restore_mqd_sdma(struct mqd_manager *mm, void **mqd,
> @@ -454,7 +505,7 @@ struct mqd_manager *mqd_manager_init_v12(enum 
> KFD_MQD_TYPE type,
>       switch (type) {
>       case KFD_MQD_TYPE_CP:
>               pr_debug("%s@%i\n", __func__, __LINE__);
> -             mqd->allocate_mqd = allocate_mqd;
> +             mqd->allocate_mqd = allocate_cp_mqd;
>               mqd->init_mqd = init_mqd;
>               mqd->free_mqd = kfd_free_mqd_cp;
>               mqd->load_mqd = load_mqd;
> @@ -465,6 +516,7 @@ struct mqd_manager *mqd_manager_init_v12(enum 
> KFD_MQD_TYPE type,
>               mqd->get_wave_state = get_wave_state;
>               mqd->mqd_stride = kfd_mqd_stride;
>               mqd->restore_mqd = restore_mqd;
> +             mqd->update_mqd_gpu_addr = update_mqd_gpu_addr;
>  #if defined(CONFIG_DEBUG_FS)
>               mqd->debugfs_show_mqd = debugfs_show_mqd;
>  #endif

Reply via email to