[PATCH 2/2] drm/amdgpu/mes: Add cleaner shader fence address handling in MES for GFX12

[Liu, Shaoyun]

[AMD Official Use Only - AMD Internal Distribution Only]

I believe this could be a mistake , from MES side , the set_hw_resource_1  should always be called  no matter it's unified or none-unified . I think we should remove it for the unified check at first place .

Regards
Shaoyun.liu

-----Original Message-----
From: Alex Deucher <alexdeucher at gmail.com>
Sent: Tuesday, February 11, 2025 10:24 AM
To: Liu, Shaoyun <Shaoyun.Liu at amd.com>
Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; amd-gfx at lists.freedesktop.org; Koenig, Christian <Christian.Koenig at amd.com>; SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com>
Subject: Re: [PATCH 2/2] drm/amdgpu/mes: Add cleaner shader fence address handling in MES for GFX12

On Mon, Feb 10, 2025 at 8:48 PM Liu, Shaoyun <Shaoyun.Liu at amd.com> wrote:
>
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> Why only free the mem when  unified mes is enabled , I remembered that set_hw_resource_1 will also be called in none-unified mes mode .

mes_v12_0_set_hw_resources_1() is currently only called when unified mes mode is enabled so it makes sense to align the free side as well.

>
> Another thing is eventually we probably should allocate total 129 pages the same as GFX11  (128  for hw_resource_1 and  one  extra page for MES internal usage) .  But seems SRIOV still not use that 128 pages, It can  be  updated later when  I update the API for gfx12 to enable that .
>

yeah, we can add that later if we need it.  The current mes 12 packet doesn't seem to support that feature yet.

Alex

> Regards
> Shaoyun.liu
>
> -----Original Message-----
> From: Deucher, Alexander <Alexander.Deucher at amd.com>
> Sent: Monday, February 10, 2025 4:19 PM
> To: amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Koenig, Christian
> <Christian.Koenig at amd.com>; SHANMUGAM, SRINIVASAN
> <SRINIVASAN.SHANMUGAM at amd.com>; Liu, Shaoyun <Shaoyun.Liu at amd.com>
> Subject: [PATCH 2/2] drm/amdgpu/mes: Add cleaner shader fence address
> handling in MES for GFX12
>
> This commit introduces enhancements to the handling of the cleaner shader fence in the AMDGPU MES driver:
>
> - The MES (Microcode Execution Scheduler) now sends a PM4 packet to the
>   KIQ (Kernel Interface Queue) to request the cleaner shader, ensuring
>   that requests are handled in a controlled manner and avoiding the
>   race conditions.
> - The CP (Compute Processor) firmware has been updated to use a private
>   bus for accessing specific registers, avoiding unnecessary operations
>   that could lead to issues in VF (Virtual Function) mode.
> - The cleaner shader fence memory address is now set correctly in the
>   `mes_set_hw_res_pkt` structure, allowing for proper synchronization of
>   the cleaner shader execution.
>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> Suggested-by: Shaoyun Liu <shaoyun.liu at amd.com>
>
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> index dcbc31279f29b..8dbab3834d82d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
> @@ -678,6 +678,9 @@ static int mes_v12_0_misc_op(struct amdgpu_mes
> *mes,
>
>  static int mes_v12_0_set_hw_resources_1(struct amdgpu_mes *mes, int
> pipe)  {
> +       unsigned int alloc_size = AMDGPU_GPU_PAGE_SIZE;
> +       int ret = 0;
> +       struct amdgpu_device *adev = mes->adev;
>         union MESAPI_SET_HW_RESOURCES_1 mes_set_hw_res_1_pkt;
>
>         memset(&mes_set_hw_res_1_pkt, 0, sizeof(mes_set_hw_res_1_pkt)); @@ -687,6 +690,19 @@ static int mes_v12_0_set_hw_resources_1(struct amdgpu_mes *mes, int pipe)
>         mes_set_hw_res_1_pkt.header.dwsize = API_FRAME_SIZE_IN_DWORDS;
>         mes_set_hw_res_1_pkt.mes_kiq_unmap_timeout = 0xa;
>
> +       ret = amdgpu_bo_create_kernel(adev, alloc_size, PAGE_SIZE,
> +                               AMDGPU_GEM_DOMAIN_VRAM,
> +                               &mes->resource_1,
> +                               &mes->resource_1_gpu_addr,
> +                               &mes->resource_1_addr);
> +       if (ret) {
> +               dev_err(adev->dev, "(%d) failed to create mes resource_1 bo\n", ret);
> +               return ret;
> +       }
> +
> +       mes_set_hw_res_1_pkt.cleaner_shader_fence_mc_addr =
> +               mes->resource_1_gpu_addr;
> +
>         return mes_v12_0_submit_pkt_and_poll_completion(mes, pipe,
>                         &mes_set_hw_res_1_pkt, sizeof(mes_set_hw_res_1_pkt),
>                         offsetof(union MESAPI_SET_HW_RESOURCES_1,
> api_status)); @@ -1770,6 +1786,12 @@ static int
> mes_v12_0_hw_init(struct amdgpu_ip_block *ip_block)
>
>  static int mes_v12_0_hw_fini(struct amdgpu_ip_block *ip_block)  {
> +       struct amdgpu_device *adev = ip_block->adev;
> +
> +       if (adev->enable_uni_mes)
> +               amdgpu_bo_free_kernel(&adev->mes.resource_1,
> +                                     &adev->mes.resource_1_gpu_addr,
> +                                     &adev->mes.resource_1_addr);
>         return 0;
>  }
>
> --
> 2.48.1
>